diff --git a/src/services/extensions/v2/README.md b/src/services/extensions/v2/README.md index 81fe86f..9f51f87 100644 --- a/src/services/extensions/v2/README.md +++ b/src/services/extensions/v2/README.md @@ -37,6 +37,17 @@ against it. the extension. `POST /extensions/{id}/relist` restores it (optional `review_note`, same `?notify` opt-out); see `ExtensionsDatabase.delist()` and `relist()`. +- `POST /extensions/{id}/moderator-correct` corrects a published extension's + live content as a moderator (api#251): truncated readmes, broken links, or + other corruption that should not wait for the author to resubmit. One write + inserts an already-approved revision row (`submitted_by` and `reviewer_id` + both the moderator, `correction_note` as the `review_note`) and publishes + it, leaving `published_at` and ownership untouched; the catalogue is + revalidated afterwards. Requires a published, listed extension with no + pending revision (409 otherwise — approve or reject the pending edit + first, never supersede it). Unlike every other moderation write there is no + `?notify` query and no author email: the correction is recorded in revision + history and surfaced by the directory UI. See `ExtensionsDatabase.moderatorCorrect()`. - `GET /extensions/{id}` is role-aware: anonymous and unrelated callers get the published projection (or 404, which hides existence for drafts and delisted rows); the owner and moderators get the full `OwnedExtension`, @@ -70,6 +81,23 @@ missing mail credentials skip the send before the response and report `https://smtpapi.mxroute.com/`, `resend`, or `disabled`) and the root README for the `EXTENSIONS_V2_EMAIL_*` configuration. +### Correcting Live Content + +Three paths, in order of preference (api#251): + +- **Moderator edit** (`POST /extensions/{id}/moderator-correct`): live + catalogue corruption — a truncated readme, a broken link — on a published, + listed extension. Immediate, audited (an approved revision row), no author + email. +- **Ask the author to resubmit**: wording disputes, new releases, unpublished + or delisted extensions, or anything while a pending edit exists. The normal + propose/review queue, with its notification emails. +- **Direct D1 edit (break-glass only)**: the API is down, or the stored data + violates a constraint the API cannot express a fix through. Back up the row + first, write down why in the change record, and trigger a catalogue + revalidate manually afterwards — the audit trail and purge that the API + would have done do not happen by themselves. + The id and the developer are properties of the extension, not of a revision: an edit cannot rename an extension or move it to another developer, and approving one no longer rewrites the developer profile as a side effect. A user owns at diff --git a/src/services/extensions/v2/db/extensions.ts b/src/services/extensions/v2/db/extensions.ts index 3577d2e..a6d13fa 100644 --- a/src/services/extensions/v2/db/extensions.ts +++ b/src/services/extensions/v2/db/extensions.ts @@ -12,6 +12,7 @@ import { encodeCursor as encode, decodeCursor as decode } from "./cursor"; import { Extension, ExtensionContent, + ExtensionContentSchema, ExtensionListItem, License, OwnedExtension, @@ -886,6 +887,215 @@ export class ExtensionsDatabase { error: { message: "Extension could not be relisted", code: "CONFLICT" } }; } + + // Moderator correction of live catalogue content (api#251): a truncated + // readme or broken link the owner should not have to resubmit to fix. One + // D1 batch() like approve(): the first statement inserts an + // already-approved revision row (submitted_by and reviewer both the + // moderator, ownership_epoch carried from the owner row), the second + // publishes it gated on `changes() = 1`. published_at is left untouched + // and ownership never written; guards on published, listed, no pending + // revision, and active moderator. + async moderatorCorrect( + id: string, + moderatorId: string, + content: ExtensionContent, + correctionNote: string + ): Promise> { + const parsed = ExtensionContentSchema.safeParse(content); + if (!parsed.success) { + return { + data: null, + error: { + message: "Extension content failed validation", + code: "CONFLICT" + } + }; + } + const valid = parsed.data; + const revisionId = crypto.randomUUID(); + + let results; + try { + const correctStmt = toD1Statement(this.db.$client, { + sql: `INSERT INTO extension_revisions + (id, extension_id, developer_id, submitted_by, status, content, + reviewer_id, review_note, reviewed_at, ownership_epoch) + SELECT ?, e.id, e.developer_id, ?, 'approved', ?, ?, ?, CURRENT_TIMESTAMP, d.ownership_epoch + FROM extensions e + JOIN developers d ON d.id = e.developer_id + WHERE LOWER(e.id) = LOWER(?) + AND e.published_at IS NOT NULL + AND e.delisted_at IS NULL + AND NOT EXISTS ( + SELECT 1 FROM extension_revisions r + WHERE r.extension_id = e.id AND r.status = 'pending' + ) + AND EXISTS ( + SELECT 1 FROM users u + WHERE u.id = ? AND u.deleted_at IS NULL AND u.is_moderator = 1 + )`, + params: [ + revisionId, + moderatorId, + JSON.stringify(valid), + moderatorId, + correctionNote, + id, + moderatorId + ] + }); + + const publishStmt = toD1Statement(this.db.$client, { + sql: `UPDATE extensions + SET type = ?, name = ?, description = ?, releases = ?, website = ?, + license = ?, icon_url = ?, readme = ?, source = ?, version = ?, + download_url = ?, + published_revision_id = ?, + updated_at = CURRENT_TIMESTAMP + WHERE changes() = 1 AND LOWER(id) = LOWER(?)`, + params: [ + valid.type, + valid.name, + valid.description, + JSON.stringify(valid.releases), + valid.website, + JSON.stringify(valid.license), + valid.icon_url ?? null, + valid.readme, + JSON.stringify(valid.source), + valid.version, + valid.download_url, + revisionId, + id + ] + }); + + results = await this.db.$client.batch([correctStmt, publishStmt]); + } catch (error) { + return databaseError("moderatorCorrect", error); + } + + if (!results[0]?.meta?.changes) { + return this.moderatorCorrectBlockedError(id, moderatorId); + } + + // Canonical id for the response (the path param may differ in case). + // Inside error handling like every other read here: the correction is + // already committed, so a failure must report a database error rather + // than throw past the route. + let row: { canonicalId: string } | undefined; + try { + [row] = await this.db + .select({ canonicalId: extensions.id }) + .from(extensions) + .where(sql`LOWER(${extensions.id}) = LOWER(${id})`); + } catch (error) { + return databaseError("moderatorCorrect", error); + } + return { + data: { id: row?.canonicalId ?? id, revisionId }, + error: null + }; + } + + // Separates the ways moderatorCorrect()'s guard can affect no rows, so the + // route can answer 403/404/409 rather than one opaque failure. Mirrors + // relistBlockedError's actor check first: a moderator deactivated or + // demoted after requireModerator() ran fails the write and must be told so. + private async moderatorCorrectBlockedError( + id: string, + moderatorId: string + ): Promise> { + const access = await new UsersDatabase(this.db).moderatorAccess( + moderatorId + ); + if (access.error || !access.data) { + return { + data: null, + error: access.error ?? { + message: "Active account required", + code: "ACCOUNT_INACTIVE" + } + }; + } + if (!access.data.active) { + return { + data: null, + error: { + message: "Active account required", + code: "ACCOUNT_INACTIVE" + } + }; + } + if (!access.data.moderator) { + return { + data: null, + error: { message: "Moderator access required", code: "FORBIDDEN" } + }; + } + + let existing: + { publishedAt: string | null; delistedAt: string | null } | undefined; + try { + [existing] = await this.db + .select({ + publishedAt: extensions.publishedAt, + delistedAt: extensions.delistedAt + }) + .from(extensions) + .where(sql`LOWER(${extensions.id}) = LOWER(${id})`); + } catch (error) { + return databaseError("moderatorCorrect", error); + } + if (!existing) return notFound(id); + if (!existing.publishedAt) { + return { + data: null, + error: { + message: "Only a published extension can be corrected", + code: "CONFLICT" + } + }; + } + if (existing.delistedAt) { + return { + data: null, + error: { + message: "A delisted extension cannot be corrected; relist it first", + code: "CONFLICT" + } + }; + } + + try { + const [pending] = await this.db + .select({ one: sql`1` }) + .from(extensionRevisions) + .where( + and( + sql`LOWER(${extensionRevisions.extensionId}) = LOWER(${id})`, + eq(extensionRevisions.status, "pending") + ) + ); + if (pending) { + return { + data: null, + error: { + message: + "An edit to this extension is already awaiting review; approve or reject it first", + code: "CONFLICT" + } + }; + } + } catch (error) { + return databaseError("moderatorCorrect", error); + } + return { + data: null, + error: { message: "Extension could not be corrected", code: "CONFLICT" } + }; + } } // Escapes SQLite LIKE metacharacters in a caller-supplied search term so a diff --git a/src/services/extensions/v2/routes/moderation.ts b/src/services/extensions/v2/routes/moderation.ts index 6ee3603..a84f1be 100644 --- a/src/services/extensions/v2/routes/moderation.ts +++ b/src/services/extensions/v2/routes/moderation.ts @@ -25,6 +25,7 @@ import { DeveloperHistoryEntrySchema } from "../schemas/developers"; import { RevisionIdParamSchema } from "../schemas/revisions"; +import { ExtensionUpdateSchema } from "../schemas/extensions"; import { DeveloperProfilesDatabase } from "../db/developer-profiles"; import { ExtensionsDatabase } from "../db/extensions"; import { ExtensionRevisionsDatabase } from "../db/revisions"; @@ -352,6 +353,93 @@ export function registerModerationRoutes(app: ExtensionsV2App): void { ); }); + // Moderator correction of live content (api#251): unlike approve, which + // only publishes what an author proposed, this inserts an already-approved + // revision row and publishes it at once. No ?notify query and no mail + // path - the correction is recorded in history, and the author is not + // emailed. + const correctRoute = createRoute({ + method: "post", + path: "/extensions/{id}/moderator-correct", + tags: ["Moderation"], + summary: "Correct a published extension's live content as a moderator", + security: [{ Bearer: [] }], + middleware: [requireModerator()] as const, + request: { + params: IdParamSchema, + body: { + content: { + "application/json": { + schema: ExtensionUpdateSchema.extend({ + correction_note: z.string().trim().min(1).max(2000) + }) + .strict() + .openapi("ModeratorCorrect") + } + } + } + }, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ + result: z.object({ + id: z.string(), + revision_id: z.string(), + status: z.literal("approved") + }) + }) + } + }, + description: + "Content corrected and published. Recorded as an approved moderator revision; no author email is sent." + }, + 401: errorResponse("Missing or invalid bearer token"), + 403: { + ...ActiveAccountRequiredResponse, + description: "The account is inactive or the caller is not a moderator" + }, + 404: errorResponse("No such extension"), + 409: errorResponse( + "Extension is unpublished or delisted, or an edit is already awaiting review" + ), + 422: errorResponse( + "Path params, content, or correction_note failed validation" + ), + 500: errorResponse("Database error") + } + }); + + app.openapi(correctRoute, async (c) => { + const auth = getAuth(c); + const { id } = c.req.valid("param"); + const { correction_note, ...content } = c.req.valid("json"); + const extDb = getExtensionsDb(c.env.DB_EXTENSIONS); + const db = new ExtensionsDatabase(extDb); + const { data, error } = await db.moderatorCorrect( + id, + auth.userId, + content, + correction_note + ); + if (error || !data) { + const status = statusFromWriteErrorCode(error?.code); + return c.json(errorBody(error, "Unable to correct extension"), status); + } + revalidateCatalogue(c); + return c.json( + { + result: { + id: data.id, + revision_id: data.revisionId, + status: "approved" as const + } + }, + 200 + ); + }); + const approveDeveloperRoute = createRoute({ method: "post", path: "/developers/{id}/approve", diff --git a/test/services/extensions/v2/moderation-correct.test.ts b/test/services/extensions/v2/moderation-correct.test.ts new file mode 100644 index 0000000..dd9882f --- /dev/null +++ b/test/services/extensions/v2/moderation-correct.test.ts @@ -0,0 +1,303 @@ +import { describe, it, expect, vi } from "vitest"; +import { env } from "cloudflare:workers"; +import { wrapD1WithHook } from "./db-interceptor"; +import { + setupExtensionsV2Tests, + db, + authHeaders, + post, + put, + sampleContent, + sampleCreate, + seedDeveloper +} from "./harness"; +import { + insertUser, + insertExtension, + insertUnpublishedExtension, + countRevisions, + getDeveloper, + getExtension, + getRevision, + listRevisions +} from "./db-fixtures"; + +// Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the +// default "not found" behaviour in beforeEach and documents why. +vi.mock("@octokit/request", async () => + (await import("../../../mocks/octokit")).octokitRequestMock() +); + +setupExtensionsV2Tests(); + +const PATH = "/extensions/v2/extensions/live-ext/moderator-correct"; + +function correctBody(overrides?: Record) { + return { + ...sampleContent(), + correction_note: "Fix truncated readme", + ...overrides + }; +} + +async function seedPublished(): Promise { + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + await seedDeveloper("new-developer", "user-1"); + await insertExtension(db, { + id: "live-ext", + developer_id: "new-developer", + readme: "old readme" + }); +} + +describe("POST /extensions/{id}/moderator-correct (api#251)", () => { + it("requires auth", async () => { + const res = await post( + PATH, + { "Content-Type": "application/json" }, + correctBody() + ); + expect(res.status).toBe(401); + }); + + it("blocks non-moderators", async () => { + await seedPublished(); + const res = await post(PATH, await authHeaders("user-1"), correctBody()); + expect(res.status).toBe(403); + await expect(res.json()).resolves.toMatchObject({ + error: { code: "FORBIDDEN" } + }); + }); + + it("reports 403 when the moderator is deactivated mid-correct", async () => { + await seedPublished(); + const headers = await authHeaders("mod-1"); + + let done = false; + env.DB_EXTENSIONS = wrapD1WithHook(db, async (sql) => { + if (!done && sql.includes("extension_revisions")) { + done = true; + await db + .prepare("UPDATE users SET deleted_at = ? WHERE id = ?") + .bind(new Date().toISOString(), "mod-1") + .run(); + } + }); + + const res = await post(PATH, headers, correctBody()); + expect(res.status).toBe(403); + await expect(res.json()).resolves.toMatchObject({ + error: { code: "ACCOUNT_INACTIVE" } + }); + expect((await getExtension(db, "live-ext"))?.readme).toBe("old readme"); + // The guarded INSERT never ran, so no orphan revision row is left behind. + expect(await countRevisions(db)).toBe(0); + }); + + it("404s for an unknown extension", async () => { + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + const res = await post( + "/extensions/v2/extensions/no-such-extension/moderator-correct", + await authHeaders("mod-1"), + correctBody() + ); + expect(res.status).toBe(404); + await expect(res.json()).resolves.toMatchObject({ + error: { code: "NOT_FOUND" } + }); + }); + + it("409s for an unpublished extension", async () => { + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + await seedDeveloper("new-developer", "user-1"); + await insertUnpublishedExtension(db, { + id: "draft-ext", + developer_id: "new-developer" + }); + const res = await post( + "/extensions/v2/extensions/draft-ext/moderator-correct", + await authHeaders("mod-1"), + correctBody() + ); + expect(res.status).toBe(409); + await expect(res.json()).resolves.toMatchObject({ + error: { code: "CONFLICT" } + }); + expect(await countRevisions(db)).toBe(0); + }); + + it("409s for a delisted extension", async () => { + await seedPublished(); + await db + .prepare( + "UPDATE extensions SET delisted_at = ?, delist_reason = ? WHERE id = ?" + ) + .bind(new Date().toISOString(), "cause", "live-ext") + .run(); + const res = await post(PATH, await authHeaders("mod-1"), correctBody()); + expect(res.status).toBe(409); + await expect(res.json()).resolves.toMatchObject({ + error: { code: "CONFLICT" } + }); + expect(await countRevisions(db)).toBe(0); + }); + + it("409s when a pending revision exists", async () => { + await seedPublished(); + const edit = await put( + "/extensions/v2/extensions/live-ext", + await authHeaders("user-1"), + sampleContent({ name: "Author Edit" }) + ); + expect(edit.status).toBe(202); + + const res = await post(PATH, await authHeaders("mod-1"), correctBody()); + expect(res.status).toBe(409); + await expect(res.json()).resolves.toMatchObject({ + error: { code: "CONFLICT" } + }); + // Neither the live content nor the pending edit moves. + expect((await getExtension(db, "live-ext"))?.readme).toBe("old readme"); + // Only the author's pending revision exists: the failed correction added + // no row of its own. + expect(await countRevisions(db)).toBe(1); + }); + + it("422s on a blank correction_note", async () => { + await seedPublished(); + const res = await post( + PATH, + await authHeaders("mod-1"), + correctBody({ correction_note: " " }) + ); + expect(res.status).toBe(422); + }); + + it("422s on an overlong correction_note", async () => { + await seedPublished(); + const res = await post( + PATH, + await authHeaders("mod-1"), + correctBody({ correction_note: "x".repeat(2001) }) + ); + expect(res.status).toBe(422); + expect(await countRevisions(db)).toBe(0); + }); + + it("422s on invalid content", async () => { + await seedPublished(); + const res = await post( + PATH, + await authHeaders("mod-1"), + correctBody({ readme: "" }) + ); + expect(res.status).toBe(422); + }); + + it("corrects live content and records an approved moderator revision", async () => { + await seedPublished(); + const before = await getExtension(db, "live-ext"); + expect(before?.published_at).not.toBeNull(); + + const res = await post( + PATH, + await authHeaders("mod-1"), + correctBody({ name: "Fixed Extension", readme: "# Fixed" }) + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { id: string; revision_id: string; status: string }; + }; + expect(body.result).toMatchObject({ id: "live-ext", status: "approved" }); + expect(typeof body.result.revision_id).toBe("string"); + // No notification envelope on this route (api#251: history only). + expect("notified" in body.result).toBe(false); + + const after = await getExtension(db, "live-ext"); + expect(after?.name).toBe("Fixed Extension"); + expect(after?.readme).toBe("# Fixed"); + expect(after?.published_at).toBe(before?.published_at); + expect(after?.published_revision_id).toBe(body.result.revision_id); + + const revision = await getRevision(db, body.result.revision_id); + expect(revision).toMatchObject({ + extension_id: "live-ext", + status: "approved", + submitted_by: "mod-1", + reviewer_id: "mod-1", + review_note: "Fix truncated readme" + }); + expect(JSON.parse(revision!.content)).toMatchObject({ + name: "Fixed Extension", + readme: "# Fixed" + }); + + // Ownership is untouched: developer, owner, and epoch survive. + expect(await getDeveloper(db, "new-developer")).toMatchObject({ + id: "new-developer", + owner_user_id: "user-1", + ownership_epoch: 1 + }); + expect( + (await listRevisions(db)).filter((r) => r.status === "pending") + ).toHaveLength(0); + }); + + it("matches ids case-insensitively and returns the canonical id", async () => { + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + await seedDeveloper("new-developer", "user-1"); + await insertExtension(db, { + id: "LIVE-ext", + developer_id: "new-developer" + }); + const res = await post( + "/extensions/v2/extensions/live-EXT/moderator-correct", + await authHeaders("mod-1"), + correctBody() + ); + expect(res.status).toBe(200); + await expect(res.json()).resolves.toMatchObject({ + result: { id: "LIVE-ext", status: "approved" } + }); + }); + + it("creates through the full owner flow then corrects, preserving history", async () => { + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + await seedDeveloper("new-developer", "user-1"); + const created = await post( + "/extensions/v2/extensions", + await authHeaders("user-1"), + sampleCreate({ extensionId: "history-ext" }) + ); + expect(created.status).toBe(201); + const { result: pending } = (await created.json()) as { + result: { id: string; revision_id: string }; + }; + expect( + ( + await post( + `/extensions/v2/extensions/${pending.id}/revisions/${pending.revision_id}/approve?notify=false`, + await authHeaders("mod-1"), + {} + ) + ).status + ).toBe(200); + + const res = await post( + "/extensions/v2/extensions/history-ext/moderator-correct", + await authHeaders("mod-1"), + { ...sampleContent(), readme: "# Corrected", correction_note: "typo" } + ); + expect(res.status).toBe(200); + + // The owner's approval and the moderator's correction are both in + // history as approved revisions. + const revisions = await listRevisions(db); + expect( + revisions + .filter((r) => r.extension_id === "history-ext") + .map((r) => r.status) + .sort() + ).toEqual(["approved", "approved"]); + }); +}); diff --git a/test/services/extensions/v2/moderation-revalidate.test.ts b/test/services/extensions/v2/moderation-revalidate.test.ts index 3973fed..64b5b6e 100644 --- a/test/services/extensions/v2/moderation-revalidate.test.ts +++ b/test/services/extensions/v2/moderation-revalidate.test.ts @@ -7,6 +7,7 @@ import { post, put, del, + sampleContent, sampleCreate, sampleDeveloper } from "./harness"; @@ -148,6 +149,19 @@ describe("CDN cache revalidation on catalogue mutations", () => { expect(fetchCalls(fetcher)).toHaveLength(1); }); + it("purges after a moderator correction", async () => { + await seedModAndExtension(); + const fetcher = stubFrontend(); + + const res = await post( + "/extensions/v2/extensions/live-ext/moderator-correct", + await authHeaders("mod-1"), + { ...sampleContent(), correction_note: "Fix truncated readme" } + ); + expect(res.status).toBe(200); + expect(fetchCalls(fetcher)).toHaveLength(1); + }); + it("purges after a revision rejection", async () => { await seedModAndExtension(); const created = await post(