fix(registry): re-review publisher updates / 发布者更新重新进入审核 (#6830)

* fix(registry): re-review publisher updates

Problem:
Publishers could replace the source, manifest, or metadata of an active package while it stayed live and retained its verified badge.

Root cause:
Registry moderation was triggered only when the public package kind changed, although every accepted republish creates a new immutable package version.

Fix:
Return every non-admin package update to pending review, clear verification, and cover active, hidden, rejected, and trusted-admin update paths.

Verification:
- workers/crash-report: npm test (35 passed)
- workers/crash-report: npm run typecheck
- git diff --check

* fix(registry): fence approvals to reviewed revision

Problem:
A stale moderation page could approve a package version submitted after the reviewer loaded the queue.

Root cause:
Approval requests carried only the slug and action, while both admin backends performed unconditional status updates.

Fix:
Bind approvals to the reviewed version, update timestamp, and status with one atomic D1 conditional update. Stale or old-page requests now fail closed across both admin interfaces.

Verification:
- worker tests: 41 passed
- worker typecheck
- site tests: 26 passed
- site production build
- git diff --check
This commit is contained in:
SivanCola authored and GitHub committed 2026-07-22 21:24:27 +08:00
1 parent d97dfdcfbf
commit d9dd4aa0e5
9 files changed
+473 -56

No files matched your search

+20 -4
View File
@@ -482,7 +482,7 @@ const desc = "Browse, publish, and install community skills, plugins, and MCP se
if (!pkgs.length) { reviewList.innerHTML = ''; show(reviewEmpty, true); return; }
show(reviewEmpty, false);
reviewList.innerHTML = pkgs.map((p) => `
<div class="review-row" data-slug="${esc(p.handle)}/${esc(p.name)}">
<div class="review-row" data-slug="${esc(p.handle)}/${esc(p.name)}" data-version="${esc(p.latestVersion)}" data-updated-at="${esc(p.updatedAt)}" data-status="${esc(p.status)}">
<div class="rv-main">
<div class="rv-name"><span class="scope">${esc(p.handle)}/</span>${esc(p.name)} <span class="${kindClass(p.kind)}">${esc(p.kind)}</span></div>
<div class="rv-sum">${esc(p.summary) || '(no summary)'}</div>
@@ -509,11 +509,27 @@ const desc = "Browse, publish, and install community skills, plugins, and MCP se
const row = btn.closest('.review-row');
row.querySelectorAll('.rv-btn').forEach((b) => (b.disabled = true));
try {
const r = await fetch(`${REGISTRY}/v1/admin/packages/${row.dataset.slug}/${btn.dataset.act}`, { method: 'POST', credentials: 'include' });
if (!r.ok) throw new Error();
const r = await fetch(`${REGISTRY}/v1/admin/packages/${row.dataset.slug}/${btn.dataset.act}`, {
method: 'POST', credentials: 'include', headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
expectedVersion: row.dataset.version,
expectedUpdatedAt: row.dataset.updatedAt,
expectedStatus: row.dataset.status,
}),
});
const data = await r.json().catch(() => ({}));
if (!r.ok) {
row.querySelectorAll('.rv-btn').forEach((b) => (b.disabled = false));
setMsg(data?.error?.message || 'Review action failed.', 'err');
if (r.status === 409) loadReview();
return;
}
row.remove();
loadReview(); loadPackages(); loadFeed();
} catch { row.querySelectorAll('.rv-btn').forEach((b) => (b.disabled = false)); }
} catch {
row.querySelectorAll('.rv-btn').forEach((b) => (b.disabled = false));
setMsg('Registry is unreachable right now.', 'err');
}
});
// Nav account state, via the site's shared account client. Signed out → a
@@ -34,3 +34,15 @@ test("registry copy requests preserve the reviewed package kind", async () => {
assert.match(page, /data-copy="\$\{esc\(installRequest\(p\)\)\}"/);
assert.doesNotMatch(page, /data-copy="\$\{esc\(p\.source\)\}"/);
});
test("admin approval is bound to the package revision shown in the review queue", async () => {
const page = await source();
assert.match(page, /data-version="\$\{esc\(p\.latestVersion\)\}"/);
assert.match(page, /data-updated-at="\$\{esc\(p\.updatedAt\)\}"/);
assert.match(page, /data-status="\$\{esc\(p\.status\)\}"/);
assert.match(page, /expectedVersion: row\.dataset\.version/);
assert.match(page, /expectedUpdatedAt: row\.dataset\.updatedAt/);
assert.match(page, /expectedStatus: row\.dataset\.status/);
assert.match(page, /if \(r\.status === 409\) loadReview\(\)/);
});
@@ -0,0 +1,45 @@
import { describe, expect, it } from "vitest";
import type { User } from "./auth";
import { renderCommunity } from "./community";
import type { PackageRow } from "./registry/types";
const admin: User = {
id: 1,
email: "[email protected]",
role: "admin",
created_at: "2026-07-22T00:00:00.000Z",
approved_at: "2026-07-22T00:00:00.000Z",
};
const pending: PackageRow = {
id: 42,
kind: "plugin",
scope_handle: "publisher",
name: "devkit",
slug: "publisher/devkit",
summary: "Developer tools",
description: "",
source: "https://github.com/o/r",
install_kind: "plugin",
homepage: "",
repo_url: "https://github.com/o/r",
tags: "tool",
latest_version: "2.7.1",
install_count: 0,
star_count: 0,
verified: 0,
status: "pending",
publisher_id: 7,
created_at: "2026-07-22T00:00:00.000Z",
updated_at: "2026-07-22T00:30:00.000Z",
};
describe("renderCommunity", () => {
it("binds moderation forms to the rendered package revision", () => {
const html = renderCommunity(admin, [pending], "pending");
expect(html).toContain('name="expectedVersion" value="2.7.1"');
expect(html).toContain('name="expectedUpdatedAt" value="2026-07-22T00:30:00.000Z"');
expect(html).toContain('name="expectedStatus" value="pending"');
});
});
+5 -1
View File
@@ -14,7 +14,11 @@ const STATUS_TABS = [
function actionForm(pkg: PackageRow, action: string, label: string, cls: string, backStatus: string, confirm?: string): string {
const onsubmit = confirm ? ` onsubmit="return confirm('${esc(confirm)}')"` : "";
return `<form method="post" action="/community/${esc(pkg.scope_handle)}/${esc(pkg.name)}/${action}" class="inline"${onsubmit}>
<input type="hidden" name="status" value="${esc(backStatus)}"><button class="btn ${cls} sm" type="submit">${esc(label)}</button></form>`;
<input type="hidden" name="status" value="${esc(backStatus)}">
<input type="hidden" name="expectedVersion" value="${esc(pkg.latest_version)}">
<input type="hidden" name="expectedUpdatedAt" value="${esc(pkg.updated_at)}">
<input type="hidden" name="expectedStatus" value="${esc(pkg.status)}">
<button class="btn ${cls} sm" type="submit">${esc(label)}</button></form>`;
}
function rowActions(pkg: PackageRow, backStatus: string): string {
+27 -11
View File
@@ -1121,19 +1121,35 @@ async function handleCommunityAction(
return back;
}
if (action === "approve") {
const before = await repo.bySlug(slug);
const row = await repo.setStatus(slug, "active", now);
// Emit the publish event on first approval so the feed only announces
// packages that actually went public.
if (row && before && before.status !== "active") {
await new EventRepo(env.REGISTRY_DB).log({
type: "publish",
packageId: row.id,
actorHandle: row.scope_handle,
summary: `published ${row.slug}@${row.latest_version}`,
now,
const expectedStatus = ["pending", "hidden", "rejected"].includes(form.expectedStatus)
? form.expectedStatus
: "";
if (!form.expectedVersion || !form.expectedUpdatedAt || !expectedStatus) {
return new Response("Package review revision is missing. Refresh the review page and try again.", {
status: 409,
});
}
const row = await repo.setStatusIfCurrent(
slug,
"active",
form.expectedVersion,
form.expectedUpdatedAt,
expectedStatus,
now,
);
if (!row) {
return new Response("Package changed since it was reviewed. Refresh and review the latest version.", {
status: 409,
});
}
// Emit the publish event only after the reviewed revision becomes public.
await new EventRepo(env.REGISTRY_DB).log({
type: "publish",
packageId: row.id,
actorHandle: row.scope_handle,
summary: `published ${row.slug}@${row.latest_version}`,
now,
});
await logAction(env, admin, "pkg_approve", slug);
return back;
}
@@ -112,7 +112,78 @@ describe("PackageRepo.publish", () => {
expect(updates[0].values[11]).toBe(0);
});
it("keeps ordinary updates to an active package live and verified", async () => {
it("returns a same-kind active package update to review", async () => {
const active: PackageRow = { ...existing, status: "active", verified: 1 };
const updated: PackageRow = {
...active,
summary: "new summary",
source: "https://github.com/o/r2",
repo_url: "https://github.com/o/r2",
install_kind: "mcp",
latest_version: "2.7.1",
status: "pending",
verified: 0,
};
const { db, updates } = fakePackageDB([active, updated]);
const input = PublishSchema.parse({
kind: "mcp",
name: "devkit",
summary: "new summary",
source: "https://github.com/o/r2",
repoUrl: "https://github.com/o/r2",
version: "2.7.1",
});
const result = await new PackageRepo(db).publish(user, input, now);
expect(result.row.status).toBe("pending");
expect(result.row.verified).toBe(0);
expect(updates[0].values[4]).toBe("mcp");
expect(updates[0].values[10]).toBe("pending");
expect(updates[0].values[11]).toBe(0);
});
it("returns a hidden package update to review and clears verification", async () => {
const hidden: PackageRow = { ...existing, status: "hidden", verified: 1 };
const updated: PackageRow = {
...hidden,
kind: "plugin",
install_kind: "plugin",
latest_version: "2.7.1",
status: "pending",
verified: 0,
};
const { db, updates } = fakePackageDB([hidden, updated]);
const result = await new PackageRepo(db).publish(user, pluginInput(), now);
expect(result.row.status).toBe("pending");
expect(result.row.verified).toBe(0);
expect(updates[0].values[10]).toBe("pending");
expect(updates[0].values[11]).toBe(0);
});
it("returns a rejected package update to review", async () => {
const rejected: PackageRow = { ...existing, status: "rejected", verified: 0 };
const requeued: PackageRow = { ...rejected, latest_version: "2.7.1", status: "pending" };
const { db, updates } = fakePackageDB([rejected, requeued]);
const input = PublishSchema.parse({
kind: "mcp",
name: "devkit",
source: "https://github.com/o/r",
repoUrl: "https://github.com/o/r",
version: "2.7.1",
});
const result = await new PackageRepo(db).publish(user, input, now);
expect(result.row.status).toBe("pending");
expect(updates[0].values[10]).toBe("pending");
expect(updates[0].values[11]).toBe(0);
});
it("preserves status and verification for trusted admin updates", async () => {
const admin: RegistryUser = { ...user, role: "admin" };
const active: PackageRow = { ...existing, status: "active", verified: 1 };
const updated: PackageRow = { ...active, install_kind: "mcp", latest_version: "2.7.1" };
const { db, updates } = fakePackageDB([active, updated]);
@@ -124,31 +195,88 @@ describe("PackageRepo.publish", () => {
version: "2.7.1",
});
const result = await new PackageRepo(db).publish(user, input, now);
const result = await new PackageRepo(db).publish(admin, input, now);
expect(result.row.status).toBe("active");
expect(result.row.verified).toBe(1);
expect(updates[0].values[4]).toBe("mcp");
expect(updates[0].values[10]).toBe("active");
expect(updates[0].values[11]).toBe(1);
});
});
it("clears verification when a non-active package changes kind", async () => {
const hidden: PackageRow = { ...existing, status: "hidden", verified: 1 };
const updated: PackageRow = {
...hidden,
kind: "plugin",
install_kind: "plugin",
latest_version: "2.7.1",
verified: 0,
};
const { db, updates } = fakePackageDB([hidden, updated]);
describe("PackageRepo.setStatusIfCurrent", () => {
it("approves only the exact package revision the admin reviewed", async () => {
const approvedAt = "2026-07-22T01:00:00.000Z";
const approved: PackageRow = { ...existing, status: "active", updated_at: approvedAt };
const statements: { sql: string; values: unknown[] }[] = [];
const db = {
prepare(sql: string) {
let values: unknown[] = [];
const statement = {
bind(...bound: unknown[]) {
values = bound;
return statement;
},
async first<T>() {
statements.push({ sql, values });
return approved as T;
},
};
return statement;
},
} as unknown as D1Database;
const result = await new PackageRepo(db).publish(user, pluginInput(), now);
const row = await new PackageRepo(db).setStatusIfCurrent(
existing.slug,
"active",
existing.latest_version,
existing.updated_at,
existing.status,
approvedAt,
);
expect(result.row.status).toBe("hidden");
expect(result.row.verified).toBe(0);
expect(updates[0].values[10]).toBe("hidden");
expect(updates[0].values[11]).toBe(0);
expect(row).toEqual(approved);
expect(statements[0].sql).toContain("latest_version = ?4 AND updated_at = ?5 AND status = ?6");
expect(statements[0].sql).toContain("RETURNING *");
expect(statements[0].values).toEqual([
"active",
approvedAt,
existing.slug,
existing.latest_version,
existing.updated_at,
existing.status,
]);
});
it("returns null when a newer package revision no longer matches", async () => {
const statements: { sql: string; values: unknown[] }[] = [];
const db = {
prepare(sql: string) {
let values: unknown[] = [];
const statement = {
bind(...bound: unknown[]) {
values = bound;
return statement;
},
async first<T>() {
statements.push({ sql, values });
return null as T | null;
},
};
return statement;
},
} as unknown as D1Database;
const row = await new PackageRepo(db).setStatusIfCurrent(
existing.slug,
"active",
existing.latest_version,
existing.updated_at,
existing.status,
"2026-07-22T01:00:00.000Z",
);
expect(row).toBeNull();
expect(statements).toHaveLength(1);
});
});
@@ -77,10 +77,11 @@ export class PackageRepo {
return res.results ?? [];
}
// Create a new package or append a version to an owned one. New packages from
// non-admins land as 'pending' (hidden until an admin approves); versions of an
// already-approved package stay live. Republishing an existing version is
// refused (409) so version history stays immutable.
// Create a new package or append a version to an owned one. New packages and
// updates from non-admins land as 'pending' (hidden until an admin approves).
// Every accepted update appends an immutable version, so its source, manifest,
// metadata, and capability kind must all cross the same moderation boundary.
// Republishing an existing version is refused (409).
async publish(user: RegistryUser, input: PublishInput, now: string): Promise<PublishResult> {
const slug = `${user.handle}/${input.name}`;
const existing = await this.bySlug(slug);
@@ -89,12 +90,12 @@ export class PackageRepo {
if (existing.publisher_id !== user.id && user.role !== "admin") {
throw new ApiError(403, "not_owner", "That name belongs to another publisher.");
}
// Changing an approved package's capability class changes what users
// consent to install. Non-admin publishers may request that change, but
// it must return to moderation and must not retain a verified badge.
const publisherChangedKind = user.role !== "admin" && existing.kind !== input.kind;
const status = publisherChangedKind && existing.status === "active" ? "pending" : existing.status;
const verified = publisherChangedKind ? 0 : existing.verified;
// A new version may change executable source or manifest content even
// when its public kind stays the same. Only trusted admin updates bypass
// re-review; publisher updates always lose verification until approved.
const publisherNeedsReview = user.role !== "admin";
const status = publisherNeedsReview ? "pending" : existing.status;
const verified = publisherNeedsReview ? 0 : existing.verified;
const version = input.version || nextPatch(existing.latest_version);
await this.insertVersion(existing.id, version, input, now);
await this.db
@@ -227,6 +228,28 @@ export class PackageRepo {
return this.bySlug(slug);
}
// Admin approval must be bound to the exact row the reviewer inspected.
// The version protects publisher updates, while updated_at + status also
// fence concurrent moderation actions. D1 evaluates the predicate and write
// atomically, so a package cannot change between a preflight read and approval.
async setStatusIfCurrent(
slug: string,
status: string,
expectedVersion: string,
expectedUpdatedAt: string,
expectedStatus: string,
now: string,
): Promise<PackageRow | null> {
return this.db
.prepare(
`UPDATE packages SET status = ?1, updated_at = ?2
WHERE slug = ?3 AND latest_version = ?4 AND updated_at = ?5 AND status = ?6
RETURNING *`,
)
.bind(status, now, slug, expectedVersion, expectedUpdatedAt, expectedStatus)
.first<PackageRow>();
}
// Admin: grant or revoke the verified trust badge.
async setVerified(slug: string, verified: boolean, now: string): Promise<PackageRow | null> {
const res = await this.db
@@ -0,0 +1,150 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import registryApp from "../app";
import type { Bindings } from "../env";
import type { PackageRow } from "../types";
const oldRevision: PackageRow = {
id: 42,
kind: "plugin",
scope_handle: "publisher",
name: "devkit",
slug: "publisher/devkit",
summary: "Developer tools",
description: "",
source: "https://github.com/o/r",
install_kind: "plugin",
homepage: "",
repo_url: "https://github.com/o/r",
tags: "tool",
latest_version: "2.7.1",
install_count: 0,
star_count: 0,
verified: 0,
status: "pending",
publisher_id: 7,
created_at: "2026-07-22T00:00:00.000Z",
updated_at: "2026-07-22T00:30:00.000Z",
};
function approvalDB(current: PackageRow, approved: PackageRow | null) {
const statements: { sql: string; values: unknown[] }[] = [];
const db = {
prepare(sql: string) {
let values: unknown[] = [];
const statement = {
bind(...bound: unknown[]) {
values = bound;
return statement;
},
async first<T>() {
statements.push({ sql, values });
if (sql.startsWith("UPDATE packages SET status")) return approved as T | null;
if (sql.startsWith("SELECT * FROM packages")) return current as T;
return null;
},
async run() {
statements.push({ sql, values });
return { meta: { changes: 1 } };
},
};
return statement;
},
};
return { db: db as unknown as D1Database, statements };
}
function bindings(db: D1Database): Bindings {
return {
DB: db,
ACCOUNTS_ORIGIN: "https://id.reasonix.test",
APP_ORIGIN: "https://reasonix.test",
ALLOWED_ORIGINS: "https://reasonix.test",
};
}
function approvalRequest(body: object): Request {
return new Request("https://registry.reasonix.test/v1/admin/packages/publisher/devkit/approve", {
method: "POST",
headers: { cookie: "rxid=test", "content-type": "application/json" },
body: JSON.stringify(body),
});
}
afterEach(() => vi.unstubAllGlobals());
describe("admin package approval", () => {
it("fails closed when an older review page omits the revision", async () => {
vi.stubGlobal(
"fetch",
vi.fn(async () =>
Response.json({ user: { id: 1, handle: "admin", role: "admin", emailVerified: true } }),
),
);
const { db, statements } = approvalDB(oldRevision, null);
const response = await registryApp.fetch(approvalRequest({}), bindings(db));
expect(response.status).toBe(400);
await expect(response.json()).resolves.toEqual({
error: {
code: "invalid_review_revision",
message: "Approval requires the reviewed package revision.",
},
});
expect(statements).toHaveLength(0);
});
it("rejects a stale review after the publisher submits a newer version", async () => {
vi.stubGlobal(
"fetch",
vi.fn(async () =>
Response.json({ user: { id: 1, handle: "admin", role: "admin", emailVerified: true } }),
),
);
const current = { ...oldRevision, latest_version: "2.7.2", updated_at: "2026-07-22T00:45:00.000Z" };
const { db, statements } = approvalDB(current, null);
const response = await registryApp.fetch(
approvalRequest({
expectedVersion: oldRevision.latest_version,
expectedUpdatedAt: oldRevision.updated_at,
expectedStatus: oldRevision.status,
}),
bindings(db),
);
expect(response.status).toBe(409);
await expect(response.json()).resolves.toEqual({
error: {
code: "stale_review",
message: "Package changed since it was reviewed. Refresh and review the latest version.",
},
});
expect(statements.some(({ sql }) => sql.startsWith("INSERT INTO events"))).toBe(false);
});
it("publishes when the submitted review revision still matches", async () => {
vi.stubGlobal(
"fetch",
vi.fn(async () =>
Response.json({ user: { id: 1, handle: "admin", role: "admin", emailVerified: true } }),
),
);
const approved = { ...oldRevision, status: "active", updated_at: "2026-07-22T01:00:00.000Z" };
const { db, statements } = approvalDB(oldRevision, approved);
const response = await registryApp.fetch(
approvalRequest({
expectedVersion: oldRevision.latest_version,
expectedUpdatedAt: oldRevision.updated_at,
expectedStatus: oldRevision.status,
}),
bindings(db),
);
expect(response.status).toBe(200);
const body = (await response.json()) as { package: { status: string; latestVersion: string } };
expect(body.package).toMatchObject({ status: "active", latestVersion: "2.7.1" });
expect(statements.some(({ sql }) => sql.startsWith("INSERT INTO events"))).toBe(true);
});
});
@@ -5,11 +5,18 @@ import { repos } from "../db";
import { requireAdmin } from "../http/auth";
import { writeRateLimit } from "../http/ratelimit";
import { ApiError } from "../http/errors";
import { z } from "zod";
const admin = new Hono<AppEnv>();
const now = () => new Date().toISOString();
const ApprovalRevisionSchema = z.object({
expectedVersion: z.string().min(1).max(64),
expectedUpdatedAt: z.string().min(1).max(64),
expectedStatus: z.enum(["pending", "hidden", "rejected"]),
});
admin.use("*", requireAdmin);
// Review queue. ?status defaults to pending; also accepts rejected/hidden/active.
@@ -24,19 +31,35 @@ admin.get("/packages", async (c) => {
admin.post("/packages/:handle/:name/approve", writeRateLimit, async (c) => {
const slug = `${c.req.param("handle")}/${c.req.param("name")}`;
const { packages: repo, events } = repos(c.env);
const before = await repo.bySlug(slug);
if (!before) throw new ApiError(404, "not_found", "No such package.");
const row = await repo.setStatus(slug, "active", now());
if (!row) throw new ApiError(404, "not_found", "No such package.");
if (before.status !== "active") {
await events.log({
type: "publish",
packageId: row.id,
actorHandle: row.scope_handle,
summary: `published ${row.slug}@${row.latest_version}`,
now: now(),
});
const revision = ApprovalRevisionSchema.safeParse(await c.req.json().catch(() => null));
if (!revision.success) {
throw new ApiError(400, "invalid_review_revision", "Approval requires the reviewed package revision.");
}
const approvedAt = now();
const row = await repo.setStatusIfCurrent(
slug,
"active",
revision.data.expectedVersion,
revision.data.expectedUpdatedAt,
revision.data.expectedStatus,
approvedAt,
);
if (!row) {
const current = await repo.bySlug(slug);
if (!current) throw new ApiError(404, "not_found", "No such package.");
throw new ApiError(
409,
"stale_review",
"Package changed since it was reviewed. Refresh and review the latest version.",
);
}
await events.log({
type: "publish",
packageId: row.id,
actorHandle: row.scope_handle,
summary: `published ${row.slug}@${row.latest_version}`,
now: approvedAt,
});
return c.json({ package: toPackageDTO(row) });
});