From 7027b9acf2bc9c9164d5a425e805abcaf4620c62 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 4 Sep 2026 13:21:34 +0100 Subject: [PATCH 1/2] Add GET /moderation/all-extensions for a moderator's full extension list Lists every extension across every developer, regardless of status, filterable by status (published/delisted/unpublished) and by a case-insensitive substring match on id (q). Distinct from the existing GET /moderation/extensions, which lists revisions awaiting review - an extension's own status and its having a pending edit are independent. --- src/services/extensions/v2/README.md | 6 + src/services/extensions/v2/db/extensions.ts | 74 ++++++++++ .../extensions/v2/routes/moderation.ts | 64 ++++++++- .../extensions/v2/schemas/extensions.ts | 22 +++ .../services/extensions/v2/moderation.test.ts | 136 ++++++++++++++++++ 5 files changed, 301 insertions(+), 1 deletion(-) diff --git a/src/services/extensions/v2/README.md b/src/services/extensions/v2/README.md index 3e6cebae..c27e80ba 100644 --- a/src/services/extensions/v2/README.md +++ b/src/services/extensions/v2/README.md @@ -41,6 +41,12 @@ against it. `GET /extensions/mine/{id}`, including `delisted`. Without it, a moderator could delist an extension but never see why (their own or another moderator's) again short of digging through `GET /extensions/{id}/revisions`. +- `GET /moderation/all-extensions` lists every extension across every + developer, filterable by `status` (`published`, `delisted`, `unpublished`; + omitted means all) and `q` (case-insensitive substring match on the id). + Distinct from `GET /moderation/extensions` above, which lists _revisions_ + awaiting review - an extension's own status and its having a pending edit + are independent. 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 diff --git a/src/services/extensions/v2/db/extensions.ts b/src/services/extensions/v2/db/extensions.ts index 9f691030..952cc511 100644 --- a/src/services/extensions/v2/db/extensions.ts +++ b/src/services/extensions/v2/db/extensions.ts @@ -336,6 +336,74 @@ export class ExtensionsDatabase { }; } + // The moderator equivalent of listOwned(): every extension regardless of + // who owns it, filterable by the same published/delisted/unpublished states + // rather than scoped to one developerId. Shares ownedListQuery and its + // parser so a moderator's list can never disagree with an owner's about + // what one row means. + async listForModeration( + filters: { + status?: "published" | "delisted" | "unpublished"; + type?: string; + q?: string; + limit?: number; + cursor?: string; + } = {} + ): Promise> { + const limit = filters.limit ?? 50; + const conditions = []; + if (filters.status === "published") { + conditions.push( + isNotNull(extensions.publishedAt), + isNull(extensions.delistedAt) + ); + } else if (filters.status === "delisted") { + conditions.push(isNotNull(extensions.delistedAt)); + } else if (filters.status === "unpublished") { + conditions.push(isNull(extensions.publishedAt)); + } + if (filters.q) { + const pattern = `%${escapeLikePattern(filters.q.toLowerCase())}%`; + conditions.push(sql`LOWER(${extensions.id}) LIKE ${pattern} ESCAPE '\\'`); + } + if (filters.type) { + conditions.push( + sql`COALESCE( + ${extensions.type}, + json_extract(${PENDING.content}, '$.type'), + json_extract(${REVIEWED.content}, '$.type') + ) = ${filters.type}` + ); + } + if (filters.cursor) { + const cursor = decodeCursor(filters.cursor); + if (!cursor) return invalidCursor(); + conditions.push(keysetAfter(cursor)); + } + + let rows: OwnedListRow[]; + try { + const query = ownedListQuery(this.db); + rows = await (conditions.length ? query.where(and(...conditions)) : query) + .orderBy(asc(sql`LOWER(${extensions.id})`), asc(extensions.id)) + .limit(limit + 1); + } catch (error) { + return databaseError("listForModeration", error); + } + + const hasMore = rows.length > limit; + const pageRows = rows.slice(0, limit); + const last = pageRows.at(-1); + return { + data: { + items: pageRows.map(parseOwnedListRow), + hasMore, + nextCursor: hasMore && last ? encodeCursor(last.id) : null + }, + error: null + }; + } + // Returns the owner view plus the two ids a route needs to authorise the // caller, so a detail read is one query rather than a fetch-then-check. async getOwned( @@ -625,6 +693,12 @@ export class ExtensionsDatabase { } } +// Escapes SQLite LIKE metacharacters in a caller-supplied search term so a +// literal "%" or "_" in it is matched literally rather than as a wildcard. +function escapeLikePattern(value: string): string { + return value.replace(/[\\%_]/g, (char) => `\\${char}`); +} + function invalidCursor(): DatabaseResult { return { data: null, diff --git a/src/services/extensions/v2/routes/moderation.ts b/src/services/extensions/v2/routes/moderation.ts index eed3f7b3..16ab83bb 100644 --- a/src/services/extensions/v2/routes/moderation.ts +++ b/src/services/extensions/v2/routes/moderation.ts @@ -26,7 +26,11 @@ import { RevisionIdParamSchema, RevisionQueueQuerySchema } from "../schemas/revisions"; -import { OwnedExtensionSchema } from "../schemas/extensions"; +import { + ModerationExtensionListQuerySchema, + OwnedExtensionListResponseSchema, + OwnedExtensionSchema +} from "../schemas/extensions"; import { DeveloperProfilesDatabase } from "../db/developer-profiles"; import { ExtensionsDatabase } from "../db/extensions"; import { ExtensionRevisionsDatabase } from "../db/revisions"; @@ -92,6 +96,64 @@ export function registerModerationRoutes(app: ExtensionsV2App): void { ); }); + // Distinct from queueRoute above: that lists *revisions* awaiting review, + // this lists *extensions* by their own published/delisted/unpublished + // state - the two are independent (an extension can be published with a + // pending edit, or delisted with none). Named /all-extensions rather than + // nested under /moderation/extensions to avoid colliding with the {id} + // route below - see isReservedExtensionId for why a static sibling segment + // there would need its own reservation. + const allExtensionsRoute = createRoute({ + method: "get", + path: "/moderation/all-extensions", + tags: ["Moderation"], + summary: "List every extension regardless of status", + security: [{ Bearer: [] }], + middleware: [requireModerator()] as const, + request: { query: ModerationExtensionListQuerySchema }, + responses: { + 200: { + content: { + "application/json": { schema: OwnedExtensionListResponseSchema } + }, + description: + "Extensions matching the requested status (default: all), alphabetical by id" + }, + 401: errorResponse("Missing or invalid bearer token"), + 403: { + ...ActiveAccountRequiredResponse, + description: "The account is inactive or the caller is not a moderator" + }, + 422: errorResponse("Query params failed validation"), + 500: errorResponse("Database error") + } + }); + + app.openapi(allExtensionsRoute, async (c) => { + const db = new ExtensionsDatabase(getExtensionsDb(c.env.DB_EXTENSIONS)); + const { status, type, q, limit, cursor } = c.req.valid("query"); + const { data, error } = await db.listForModeration({ + status, + type, + q, + limit, + cursor + }); + if (error || !data) { + return c.json( + errorBody(error, "Unable to load extensions"), + error?.code === "INVALID_CURSOR" ? 422 : 500 + ); + } + return c.json( + { + result: data.items, + pagination: { next_cursor: data.nextCursor, has_more: data.hasMore } + }, + 200 + ); + }); + // The only full-record read a moderator has for an extension they don't // own. Namespaced under /moderation rather than reusing GET /extensions/{id} // (public, published-only) or GET /extensions/mine/{id} (owner-only, 403s diff --git a/src/services/extensions/v2/schemas/extensions.ts b/src/services/extensions/v2/schemas/extensions.ts index 6f75e8bd..e734cb9e 100644 --- a/src/services/extensions/v2/schemas/extensions.ts +++ b/src/services/extensions/v2/schemas/extensions.ts @@ -290,6 +290,28 @@ export const ExtensionMineListQuerySchema = ExtensionListQuerySchema.omit({ developer_id: true }); +// A moderator's view of the whole catalogue, not just the public one: every +// status a developer can be in, filterable by the same states OwnedExtension +// itself distinguishes (see its comment) rather than a derived label. +export const ModerationExtensionListQuerySchema = ExtensionListQuerySchema.omit( + { developer_id: true } +).extend({ + status: z + .enum(["published", "delisted", "unpublished"]) + .optional() + .openapi({ param: { name: "status", in: "query" } }), + q: z + .string() + .trim() + .min(1) + .max(200) + .optional() + .openapi({ + param: { name: "q", in: "query" }, + description: "Case-insensitive substring match on the extension id" + }) +}); + export const ExtensionListResponseSchema = z .object({ result: z.array(ExtensionListItemSchema), diff --git a/test/services/extensions/v2/moderation.test.ts b/test/services/extensions/v2/moderation.test.ts index 0b3e419d..5c8f10b4 100644 --- a/test/services/extensions/v2/moderation.test.ts +++ b/test/services/extensions/v2/moderation.test.ts @@ -172,6 +172,142 @@ describe("Extensions API v2", () => { }); }); + describe("GET /moderation/all-extensions", () => { + it("requires moderator access", async () => { + await seedDeveloper("new-developer", "user-1"); + const res = await get( + "/extensions/v2/moderation/all-extensions", + await authHeaders("user-1") + ); + expect(res.status).toBe(403); + }); + + it("lists extensions across every status, unlike the public catalogue", 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" + }); + await insertUnpublishedExtension(db, { + id: "draft-ext", + developer_id: "new-developer" + }); + await post( + "/extensions/v2/extensions/live-ext/delist", + await authHeaders("mod-1"), + { reason: "Upstream source removed" } + ); + await seedDeveloper("other-developer", "user-2"); + await insertExtension(db, { + id: "other-ext", + developer_id: "other-developer" + }); + + const res = await get( + "/extensions/v2/moderation/all-extensions", + await authHeaders("mod-1") + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { result: Array<{ id: string }> }; + expect(body.result.map((r) => r.id)).toEqual([ + "draft-ext", + "live-ext", + "other-ext" + ]); + }); + + it("filters by status", 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" + }); + await insertExtension(db, { + id: "delisted-ext", + developer_id: "new-developer" + }); + await insertUnpublishedExtension(db, { + id: "draft-ext", + developer_id: "new-developer" + }); + await post( + "/extensions/v2/extensions/delisted-ext/delist", + await authHeaders("mod-1"), + { reason: "Upstream source removed" } + ); + + const published = await get( + "/extensions/v2/moderation/all-extensions?status=published", + await authHeaders("mod-1") + ); + expect(await published.json()).toMatchObject({ + result: [{ id: "live-ext" }] + }); + + const delisted = await get( + "/extensions/v2/moderation/all-extensions?status=delisted", + await authHeaders("mod-1") + ); + expect(await delisted.json()).toMatchObject({ + result: [{ id: "delisted-ext" }] + }); + + const unpublished = await get( + "/extensions/v2/moderation/all-extensions?status=unpublished", + await authHeaders("mod-1") + ); + expect(await unpublished.json()).toMatchObject({ + result: [{ id: "draft-ext" }] + }); + }); + + it("searches by a case-insensitive id substring", async () => { + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + await seedDeveloper("new-developer", "user-1"); + await insertExtension(db, { + id: "paygate", + developer_id: "new-developer" + }); + await insertExtension(db, { + id: "other-gateway", + developer_id: "new-developer" + }); + await insertExtension(db, { + id: "unrelated", + developer_id: "new-developer" + }); + + const res = await get( + "/extensions/v2/moderation/all-extensions?q=GATE", + await authHeaders("mod-1") + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { result: Array<{ id: string }> }; + expect(body.result.map((r) => r.id).sort()).toEqual([ + "other-gateway", + "paygate" + ]); + }); + + it("treats % and _ in a search term literally rather than as wildcards", async () => { + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + await seedDeveloper("new-developer", "user-1"); + await insertExtension(db, { + id: "pay-gate", + developer_id: "new-developer" + }); + + const res = await get( + `/extensions/v2/moderation/all-extensions?${new URLSearchParams({ q: "pay%gate" })}`, + await authHeaders("mod-1") + ); + expect(res.status).toBe(200); + await expect(res.json()).resolves.toMatchObject({ result: [] }); + }); + }); + describe("approve / reject", () => { it("does not approve a former owner's content when ownership changes at approval", async () => { await insertUser(db, { id: "mod-1", is_moderator: 1 }); From 030b6a14e5727f8f2c1537299648109680a9661c Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 4 Sep 2026 13:37:18 +0100 Subject: [PATCH 2/2] Address cubic review: fold q search with SQLite's LOWER() on both sides, cover the _ wildcard case toLowerCase() in JS is Unicode-aware while SQLite's LOWER() only folds ASCII, so pre-lowering the search term could desync from LOWER(id) for a non-ASCII id. Wrapping the term in LOWER() too keeps both sides folded identically. Also adds the missing '_' assertion the test's own title already claimed to cover. --- src/services/extensions/v2/db/extensions.ts | 10 ++++++++-- test/services/extensions/v2/moderation.test.ts | 17 ++++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/services/extensions/v2/db/extensions.ts b/src/services/extensions/v2/db/extensions.ts index 952cc511..c3bfd886 100644 --- a/src/services/extensions/v2/db/extensions.ts +++ b/src/services/extensions/v2/db/extensions.ts @@ -363,8 +363,14 @@ export class ExtensionsDatabase { conditions.push(isNull(extensions.publishedAt)); } if (filters.q) { - const pattern = `%${escapeLikePattern(filters.q.toLowerCase())}%`; - conditions.push(sql`LOWER(${extensions.id}) LIKE ${pattern} ESCAPE '\\'`); + // LOWER() on both sides rather than lowercasing the term in JS first: + // JS's toLowerCase() is Unicode-aware, but SQLite's LOWER() only folds + // ASCII, so pre-folding just the term could desync from what LOWER(id) + // produces for a non-ASCII id. + const pattern = `%${escapeLikePattern(filters.q)}%`; + conditions.push( + sql`LOWER(${extensions.id}) LIKE LOWER(${pattern}) ESCAPE '\\'` + ); } if (filters.type) { conditions.push( diff --git a/test/services/extensions/v2/moderation.test.ts b/test/services/extensions/v2/moderation.test.ts index 5c8f10b4..60f848ac 100644 --- a/test/services/extensions/v2/moderation.test.ts +++ b/test/services/extensions/v2/moderation.test.ts @@ -299,12 +299,23 @@ describe("Extensions API v2", () => { developer_id: "new-developer" }); - const res = await get( + const percentRes = await get( `/extensions/v2/moderation/all-extensions?${new URLSearchParams({ q: "pay%gate" })}`, await authHeaders("mod-1") ); - expect(res.status).toBe(200); - await expect(res.json()).resolves.toMatchObject({ result: [] }); + expect(percentRes.status).toBe(200); + await expect(percentRes.json()).resolves.toMatchObject({ result: [] }); + + // Unescaped, "_" is a single-character wildcard that would match the + // "-" in "pay-gate" - this must not happen either. + const underscoreRes = await get( + `/extensions/v2/moderation/all-extensions?${new URLSearchParams({ q: "pay_gate" })}`, + await authHeaders("mod-1") + ); + expect(underscoreRes.status).toBe(200); + await expect(underscoreRes.json()).resolves.toMatchObject({ + result: [] + }); }); });