From 641b9e7f315ad2324bd1c40dabebfbbd2f82395b Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 4 Sep 2026 08:39:03 +0100 Subject: [PATCH 1/6] Add moderator delisting for published extensions The Paygate extension (paygate.love, github.com/hanihiyoze/paygate-fossbilling) has gone dark: its source repo is deleted and its site is down, so its catalogue listing links to a dead download with no way to reach the developer. There was no way to remove a published extension from the catalogue at all - withdraw() only ever worked pre-publish, and moderation routes could approve/reject revisions but never touch a live one. Adds extensions.delisted_at/delist_reason (plain ADD COLUMN, no table rebuild - see the schema.ts comment for why a CHECK constraint here isn't worth the rebuild's FK risk) and POST /extensions/{id}/delist (moderator-only). Delisted rows are hidden from both public catalogue reads but keep their content and history, so the owner can still see why via GET /extensions/mine and a moderator can re-list by hand later. No email/notification on delist yet - the reason is visible to the owner in their dashboard; actually notifying them needs picking an email provider, which is out of scope here. --- src/services/extensions/v2/README.md | 11 +- src/services/extensions/v2/db/extensions.ts | 101 +- .../0022_add_extensions_delisted_at.sql | 6 + .../v2/db/migrations/meta/0022_snapshot.json | 1135 +++++++++++++++++ .../v2/db/migrations/meta/_journal.json | 7 + src/services/extensions/v2/db/schema.ts | 28 +- .../extensions/v2/routes/moderation.ts | 65 + src/services/extensions/v2/schemas/common.ts | 7 + .../extensions/v2/schemas/extensions.ts | 20 +- test/services/extensions/v2/db-fixtures.ts | 11 +- .../services/extensions/v2/migrations.test.ts | 32 +- .../services/extensions/v2/moderation.test.ts | 125 ++ .../extensions/v2/public-extensions.test.ts | 23 + 13 files changed, 1541 insertions(+), 30 deletions(-) create mode 100644 src/services/extensions/v2/db/migrations/0022_add_extensions_delisted_at.sql create mode 100644 src/services/extensions/v2/db/migrations/meta/0022_snapshot.json diff --git a/src/services/extensions/v2/README.md b/src/services/extensions/v2/README.md index 3a2d688a..a52de4c8 100644 --- a/src/services/extensions/v2/README.md +++ b/src/services/extensions/v2/README.md @@ -30,6 +30,12 @@ against it. - `POST /extensions/{id}/revisions/{revisionId}/approve` publishes the revision's content. `reject` leaves the published content untouched and the extension available to edit and resubmit. +- `POST /extensions/{id}/delist` pulls an already-published extension out of + the public catalogue for cause (its upstream source disappearing, for + example). Moderator-only, and the inverse of neither `approve` nor + `reject`: content and history are kept, so the owner can still see and edit + the extension, and a moderator can re-list it by hand later. There is no + `relist` endpoint yet - see `ExtensionsDatabase.delist()`. 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 @@ -54,7 +60,10 @@ state and a derived enum could only disagree with them: The adopted row is the one worth reading twice: migration 0021 published every extension that already existed, and those have no revisions at all, so a live extension with no review history is normal rather than a gap. `published` -being set is the only thing that means "in the catalogue". +being set is the only thing that means "in the catalogue" - except a fourth, +independent field, `delisted`: set once a moderator removes a published +extension for cause, it hides the row from both public catalogue reads +without touching `published`, `pending_revision` or `last_review`. These are separate routes from the public `GET /extensions` and `GET /extensions/{id}`, which only ever return published content. A single path diff --git a/src/services/extensions/v2/db/extensions.ts b/src/services/extensions/v2/db/extensions.ts index 3cf9058f..08fb0b2e 100644 --- a/src/services/extensions/v2/db/extensions.ts +++ b/src/services/extensions/v2/db/extensions.ts @@ -1,4 +1,4 @@ -import { and, asc, eq, isNotNull, or, sql } from "drizzle-orm"; +import { and, asc, eq, isNotNull, isNull, or, sql } from "drizzle-orm"; import { alias } from "drizzle-orm/sqlite-core"; import { DatabaseError, DatabaseResult } from "../../../../lib/interfaces"; import { ExtensionsDb } from "../../../../lib/db"; @@ -113,6 +113,8 @@ const { const OWNED_LIST_COLUMNS = { id: extensions.id, publishedAt: extensions.publishedAt, + delistedAt: extensions.delistedAt, + delistReason: extensions.delistReason, createdAt: extensions.createdAt, updatedAt: extensions.updatedAt, ...CARD_CONTENT_COLUMNS, @@ -221,7 +223,10 @@ export class ExtensionsDatabase { filters: ExtensionListFilters = {} ): Promise> { const limit = filters.limit ?? 50; - const conditions = [isNotNull(extensions.publishedAt)]; + const conditions = [ + isNotNull(extensions.publishedAt), + isNull(extensions.delistedAt) + ]; if (filters.type) conditions.push(eq(extensions.type, filters.type)); if (filters.developerId) conditions.push(eq(extensions.developerId, filters.developerId)); @@ -268,7 +273,8 @@ export class ExtensionsDatabase { .where( and( sql`LOWER(${extensions.id}) = LOWER(${id})`, - isNotNull(extensions.publishedAt) + isNotNull(extensions.publishedAt), + isNull(extensions.delistedAt) ) )) as PublishedRow[]; } catch (error) { @@ -526,6 +532,90 @@ export class ExtensionsDatabase { } }; } + + // A moderator's decision to pull an already-published extension from the + // catalogue for cause - its upstream source disappearing, for example. + // Content and history are kept (unlike withdraw(), which deletes a + // never-published row outright), so an owner can still see why and a + // moderator can re-list it later without the developer resubmitting from + // scratch. `AND delisted_at IS NULL` makes this a single atomic + // check-and-set, the same way ExtensionRevisionsDatabase.reject() guards + // on `status = 'pending'`. + async delist( + id: string, + moderatorId: string, + reason: string + ): Promise> { + let result; + try { + result = await this.db + .update(extensions) + .set({ + delistedAt: sql`CURRENT_TIMESTAMP`, + delistReason: reason + }) + .where( + and( + sql`LOWER(${extensions.id}) = LOWER(${id})`, + isNotNull(extensions.publishedAt), + isNull(extensions.delistedAt), + sql`EXISTS ( + SELECT 1 FROM ${users} + WHERE ${users.id} = ${moderatorId} AND ${users.deletedAt} IS NULL + )` + ) + ); + } catch (error) { + return databaseError("delist", error); + } + + if (!result.meta?.changes) { + return this.delistBlockedError(id, moderatorId); + } + + return { data: { id }, error: null }; + } + + // Separates the three ways delist()'s guard can affect no rows, so the + // route can answer 404/409 rather than one opaque failure. + private async delistBlockedError( + id: string, + moderatorId: string + ): Promise> { + const inactive = await inactiveActorError(this.db, moderatorId); + if (inactive) return { data: null, error: inactive }; + + const [existing] = await this.db + .select({ + publishedAt: extensions.publishedAt, + delistedAt: extensions.delistedAt + }) + .from(extensions) + .where(sql`LOWER(${extensions.id}) = LOWER(${id})`); + if (!existing) return notFound(id); + if (!existing.publishedAt) { + return { + data: null, + error: { + message: "Only a published extension can be delisted", + code: "CONFLICT" + } + }; + } + if (existing.delistedAt) { + return { + data: null, + error: { + message: "This extension is already delisted", + code: "CONFLICT" + } + }; + } + return { + data: null, + error: { message: "Extension could not be delisted", code: "CONFLICT" } + }; + } } function invalidCursor(): DatabaseResult { @@ -655,6 +745,11 @@ function parseOwnedListRow(row: OwnedListRow): OwnedExtensionListItem { reviewed_at: row.reviewedAt } : null, + // delistReason is only ever null alongside delistedAt - delist() sets + // both in the same statement - so the cast below just states that. + delisted: row.delistedAt + ? { reason: row.delistReason as string, at: row.delistedAt } + : null, created_at: row.createdAt, updated_at: row.updatedAt }; diff --git a/src/services/extensions/v2/db/migrations/0022_add_extensions_delisted_at.sql b/src/services/extensions/v2/db/migrations/0022_add_extensions_delisted_at.sql new file mode 100644 index 00000000..5e80f288 --- /dev/null +++ b/src/services/extensions/v2/db/migrations/0022_add_extensions_delisted_at.sql @@ -0,0 +1,6 @@ +DROP INDEX `idx_extensions_catalogue_order`;--> statement-breakpoint +DROP INDEX `idx_extensions_type_catalogue_order`;--> statement-breakpoint +ALTER TABLE `extensions` ADD `delisted_at` text;--> statement-breakpoint +ALTER TABLE `extensions` ADD `delist_reason` text;--> statement-breakpoint +CREATE INDEX `idx_extensions_catalogue_order` ON `extensions` (lower("id"),`id`) WHERE "extensions"."published_at" IS NOT NULL AND "extensions"."delisted_at" IS NULL;--> statement-breakpoint +CREATE INDEX `idx_extensions_type_catalogue_order` ON `extensions` (`type`,lower("id"),`id`) WHERE "extensions"."published_at" IS NOT NULL AND "extensions"."delisted_at" IS NULL; \ No newline at end of file diff --git a/src/services/extensions/v2/db/migrations/meta/0022_snapshot.json b/src/services/extensions/v2/db/migrations/meta/0022_snapshot.json new file mode 100644 index 00000000..7f07c079 --- /dev/null +++ b/src/services/extensions/v2/db/migrations/meta/0022_snapshot.json @@ -0,0 +1,1135 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "d9c02d59-2b8a-4e8b-9a7c-f97ab0260be4", + "prevId": "463ee99b-e53e-43a0-8b47-ae34518b5940", + "tables": { + "developer_claims": { + "name": "developer_claims", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "claimant_id": { + "name": "claimant_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'pending'" + }, + "note": { + "name": "note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "review_note": { + "name": "review_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "reviewer_id": { + "name": "reviewer_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "reviewed_at": { + "name": "reviewed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_org_verified": { + "name": "github_org_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verification_note": { + "name": "github_verification_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developer_claims_developer": { + "name": "idx_developer_claims_developer", + "columns": [ + "developer_id" + ], + "isUnique": false + }, + "idx_developer_claims_claimant": { + "name": "idx_developer_claims_claimant", + "columns": [ + "claimant_id" + ], + "isUnique": false + }, + "idx_developer_claims_pending_unique": { + "name": "idx_developer_claims_pending_unique", + "columns": [ + "developer_id", + "claimant_id" + ], + "isUnique": true, + "where": "\"developer_claims\".\"status\" = 'pending'" + }, + "idx_developer_claims_pending_queue": { + "name": "idx_developer_claims_pending_queue", + "columns": [ + "created_at" + ], + "isUnique": false, + "where": "\"developer_claims\".\"status\" = 'pending'" + } + }, + "foreignKeys": { + "developer_claims_developer_id_developers_id_fk": { + "name": "developer_claims_developer_id_developers_id_fk", + "tableFrom": "developer_claims", + "tableTo": "developers", + "columnsFrom": [ + "developer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_claims_claimant_id_users_id_fk": { + "name": "developer_claims_claimant_id_users_id_fk", + "tableFrom": "developer_claims", + "tableTo": "users", + "columnsFrom": [ + "claimant_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_claims_reviewer_id_users_id_fk": { + "name": "developer_claims_reviewer_id_users_id_fk", + "tableFrom": "developer_claims", + "tableTo": "users", + "columnsFrom": [ + "reviewer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "developer_claims_status_check": { + "name": "developer_claims_status_check", + "value": "\"developer_claims\".\"status\" IN ('pending', 'approved', 'rejected')" + }, + "developer_claims_github_org_verified_check": { + "name": "developer_claims_github_org_verified_check", + "value": "\"developer_claims\".\"github_org_verified\" IN (0, 1)" + } + } + }, + "developer_history": { + "name": "developer_history", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "changed_by": { + "name": "changed_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "changed_at": { + "name": "changed_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + } + }, + "indexes": { + "idx_developer_history_developer_changed_at": { + "name": "idx_developer_history_developer_changed_at", + "columns": [ + "developer_id", + "changed_at" + ], + "isUnique": false + } + }, + "foreignKeys": { + "developer_history_changed_by_users_id_fk": { + "name": "developer_history_changed_by_users_id_fk", + "tableFrom": "developer_history", + "tableTo": "users", + "columnsFrom": [ + "changed_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "developer_transfers": { + "name": "developer_transfers", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "token_hash": { + "name": "token_hash", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_by": { + "name": "created_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "expires_at": { + "name": "expires_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "accepted_by": { + "name": "accepted_by", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "accepted_at": { + "name": "accepted_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "revoked_at": { + "name": "revoked_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developer_transfers_token": { + "name": "idx_developer_transfers_token", + "columns": [ + "token_hash" + ], + "isUnique": true + }, + "idx_developer_transfers_pending": { + "name": "idx_developer_transfers_pending", + "columns": [ + "developer_id" + ], + "isUnique": true, + "where": "\"developer_transfers\".\"accepted_at\" IS NULL AND \"developer_transfers\".\"revoked_at\" IS NULL" + } + }, + "foreignKeys": { + "developer_transfers_developer_id_developers_id_fk": { + "name": "developer_transfers_developer_id_developers_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "developers", + "columnsFrom": [ + "developer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_transfers_created_by_users_id_fk": { + "name": "developer_transfers_created_by_users_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "users", + "columnsFrom": [ + "created_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_transfers_accepted_by_users_id_fk": { + "name": "developer_transfers_accepted_by_users_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "users", + "columnsFrom": [ + "accepted_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "developers": { + "name": "developers", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "owner_user_id": { + "name": "owner_user_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "approved_at": { + "name": "approved_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'1970-01-01T00:00:00.000Z'" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'1970-01-01T00:00:00.000Z'" + }, + "avatar_url": { + "name": "avatar_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "contact_email": { + "name": "contact_email", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "ownership_epoch": { + "name": "ownership_epoch", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "content_revision": { + "name": "content_revision", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "approved_revision": { + "name": "approved_revision", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "approved_by": { + "name": "approved_by", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_org_verified": { + "name": "github_org_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verification_note": { + "name": "github_verification_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verified_at": { + "name": "github_verified_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_url_verified": { + "name": "github_url_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "url_check_cooldown_until": { + "name": "url_check_cooldown_until", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developers_owner_unique": { + "name": "idx_developers_owner_unique", + "columns": [ + "owner_user_id" + ], + "isUnique": true + }, + "idx_developers_approved": { + "name": "idx_developers_approved", + "columns": [ + "approved_at" + ], + "isUnique": false + } + }, + "foreignKeys": { + "developers_owner_user_id_users_id_fk": { + "name": "developers_owner_user_id_users_id_fk", + "tableFrom": "developers", + "tableTo": "users", + "columnsFrom": [ + "owner_user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "developers_ownership_epoch_check": { + "name": "developers_ownership_epoch_check", + "value": "\"developers\".\"ownership_epoch\" >= 1" + }, + "developers_content_revision_check": { + "name": "developers_content_revision_check", + "value": "\"developers\".\"content_revision\" >= 1" + }, + "developers_github_org_verified_check": { + "name": "developers_github_org_verified_check", + "value": "\"developers\".\"github_org_verified\" IN (0, 1)" + }, + "developers_github_url_verified_check": { + "name": "developers_github_url_verified_check", + "value": "\"developers\".\"github_url_verified\" = 1" + } + } + }, + "extension_revisions": { + "name": "extension_revisions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "extension_id": { + "name": "extension_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "submitted_by": { + "name": "submitted_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'pending'" + }, + "content": { + "name": "content", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "reviewer_id": { + "name": "reviewer_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "review_note": { + "name": "review_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "reviewed_at": { + "name": "reviewed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "ownership_epoch": { + "name": "ownership_epoch", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + } + }, + "indexes": { + "idx_extension_revisions_submitted_by": { + "name": "idx_extension_revisions_submitted_by", + "columns": [ + "submitted_by" + ], + "isUnique": false + }, + "idx_extension_revisions_developer": { + "name": "idx_extension_revisions_developer", + "columns": [ + "developer_id" + ], + "isUnique": false + }, + "idx_extension_revisions_pending": { + "name": "idx_extension_revisions_pending", + "columns": [ + "extension_id" + ], + "isUnique": true, + "where": "\"extension_revisions\".\"status\" = 'pending'" + }, + "idx_extension_revisions_extension_page": { + "name": "idx_extension_revisions_extension_page", + "columns": [ + "extension_id", + "\"created_at\" desc", + "\"id\" desc" + ], + "isUnique": false + }, + "idx_extension_revisions_submitter_page": { + "name": "idx_extension_revisions_submitter_page", + "columns": [ + "submitted_by", + "\"created_at\" desc", + "\"id\" desc" + ], + "isUnique": false + }, + "idx_extension_revisions_queue_page": { + "name": "idx_extension_revisions_queue_page", + "columns": [ + "status", + "created_at", + "id" + ], + "isUnique": false + } + }, + "foreignKeys": { + "extension_revisions_extension_id_extensions_id_fk": { + "name": "extension_revisions_extension_id_extensions_id_fk", + "tableFrom": "extension_revisions", + "tableTo": "extensions", + "columnsFrom": [ + "extension_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "extension_revisions_submitted_by_users_id_fk": { + "name": "extension_revisions_submitted_by_users_id_fk", + "tableFrom": "extension_revisions", + "tableTo": "users", + "columnsFrom": [ + "submitted_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "extension_revisions_reviewer_id_users_id_fk": { + "name": "extension_revisions_reviewer_id_users_id_fk", + "tableFrom": "extension_revisions", + "tableTo": "users", + "columnsFrom": [ + "reviewer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "extension_revisions_status_check": { + "name": "extension_revisions_status_check", + "value": "\"extension_revisions\".\"status\" IN ('pending', 'approved', 'rejected')" + }, + "extension_revisions_ownership_epoch_check": { + "name": "extension_revisions_ownership_epoch_check", + "value": "\"extension_revisions\".\"ownership_epoch\" >= 1" + } + } + }, + "extensions": { + "name": "extensions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "published_at": { + "name": "published_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "published_revision_id": { + "name": "published_revision_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "releases": { + "name": "releases", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "website": { + "name": "website", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "license": { + "name": "license", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "icon_url": { + "name": "icon_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "readme": { + "name": "readme", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "source": { + "name": "source", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "version": { + "name": "version", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "download_url": { + "name": "download_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "delisted_at": { + "name": "delisted_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "delist_reason": { + "name": "delist_reason", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_extensions_id_nocase": { + "name": "idx_extensions_id_nocase", + "columns": [ + "lower(\"id\")" + ], + "isUnique": true + }, + "idx_extensions_developer_order": { + "name": "idx_extensions_developer_order", + "columns": [ + "developer_id", + "lower(\"id\")", + "id" + ], + "isUnique": false + }, + "idx_extensions_catalogue_order": { + "name": "idx_extensions_catalogue_order", + "columns": [ + "lower(\"id\")", + "id" + ], + "isUnique": false, + "where": "\"extensions\".\"published_at\" IS NOT NULL AND \"extensions\".\"delisted_at\" IS NULL" + }, + "idx_extensions_type_catalogue_order": { + "name": "idx_extensions_type_catalogue_order", + "columns": [ + "type", + "lower(\"id\")", + "id" + ], + "isUnique": false, + "where": "\"extensions\".\"published_at\" IS NOT NULL AND \"extensions\".\"delisted_at\" IS NULL" + } + }, + "foreignKeys": { + "extensions_developer_id_developers_id_fk": { + "name": "extensions_developer_id_developers_id_fk", + "tableFrom": "extensions", + "tableTo": "developers", + "columnsFrom": [ + "developer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "extensions_published_content_check": { + "name": "extensions_published_content_check", + "value": "\"extensions\".\"published_at\" IS NULL OR (\n \"extensions\".\"type\" IS NOT NULL AND \"extensions\".\"name\" IS NOT NULL AND\n \"extensions\".\"description\" IS NOT NULL AND \"extensions\".\"releases\" IS NOT NULL AND\n \"extensions\".\"website\" IS NOT NULL AND \"extensions\".\"license\" IS NOT NULL AND\n \"extensions\".\"readme\" IS NOT NULL AND \"extensions\".\"source\" IS NOT NULL AND\n \"extensions\".\"version\" IS NOT NULL AND \"extensions\".\"download_url\" IS NOT NULL\n )" + } + } + }, + "users": { + "name": "users", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "email": { + "name": "email", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "email_verified": { + "name": "email_verified", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "picture": { + "name": "picture", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "is_moderator": { + "name": "is_moderator", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "display_name": { + "name": "display_name", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_login": { + "name": "github_login", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_orgs": { + "name": "github_orgs", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_orgs_expires_at": { + "name": "github_orgs_expires_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "deleted_at": { + "name": "deleted_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": { + "idx_extension_revisions_extension_page": { + "columns": { + "\"created_at\" desc": { + "isExpression": true + }, + "\"id\" desc": { + "isExpression": true + } + } + }, + "idx_extension_revisions_submitter_page": { + "columns": { + "\"created_at\" desc": { + "isExpression": true + }, + "\"id\" desc": { + "isExpression": true + } + } + }, + "idx_extensions_id_nocase": { + "columns": { + "lower(\"id\")": { + "isExpression": true + } + } + }, + "idx_extensions_developer_order": { + "columns": { + "lower(\"id\")": { + "isExpression": true + } + } + }, + "idx_extensions_catalogue_order": { + "columns": { + "lower(\"id\")": { + "isExpression": true + } + } + }, + "idx_extensions_type_catalogue_order": { + "columns": { + "lower(\"id\")": { + "isExpression": true + } + } + } + } + } +} \ No newline at end of file diff --git a/src/services/extensions/v2/db/migrations/meta/_journal.json b/src/services/extensions/v2/db/migrations/meta/_journal.json index 1b19840e..fc682ee9 100644 --- a/src/services/extensions/v2/db/migrations/meta/_journal.json +++ b/src/services/extensions/v2/db/migrations/meta/_journal.json @@ -57,6 +57,13 @@ "when": 1785916611194, "tag": "0021_restructure_extensions_revisions", "breakpoints": true + }, + { + "idx": 22, + "version": "6", + "when": 1788507117476, + "tag": "0022_add_extensions_delisted_at", + "breakpoints": true } ] } diff --git a/src/services/extensions/v2/db/schema.ts b/src/services/extensions/v2/db/schema.ts index 27dd1f1e..f1cc624d 100644 --- a/src/services/extensions/v2/db/schema.ts +++ b/src/services/extensions/v2/db/schema.ts @@ -69,7 +69,15 @@ export const extensions = sqliteTable( .default(sql`CURRENT_TIMESTAMP`), updatedAt: text("updated_at") .notNull() - .default(sql`CURRENT_TIMESTAMP`) + .default(sql`CURRENT_TIMESTAMP`), + // A moderator's decision to pull an already-published extension from the + // catalogue for cause (its upstream source disappearing, for example). + // Distinct from never having been published and from an owner's + // pre-publish withdraw(): published_at and delisted_at can both be set at + // once, so the fact that this was live and then pulled is never lost, and + // an owner can still see and edit the row afterwards. + delistedAt: text("delisted_at"), + delistReason: text("delist_reason") }, (table) => [ // Case-insensitive id uniqueness. The id is a lowercase slug by schema, @@ -89,14 +97,18 @@ export const extensions = sqliteTable( table.id ), // These two are partial: every read that uses them filters on - // published_at IS NOT NULL, so unpublished rows would only bloat the - // index the catalogue scans. + // published_at IS NOT NULL AND delisted_at IS NULL, so an unpublished or + // delisted row would only bloat the index the catalogue scans. index("idx_extensions_catalogue_order") .on(sql`lower(${table.id})`, table.id) - .where(sql`${table.publishedAt} IS NOT NULL`), + .where( + sql`${table.publishedAt} IS NOT NULL AND ${table.delistedAt} IS NULL` + ), index("idx_extensions_type_catalogue_order") .on(table.type, sql`lower(${table.id})`, table.id) - .where(sql`${table.publishedAt} IS NOT NULL`), + .where( + sql`${table.publishedAt} IS NOT NULL AND ${table.delistedAt} IS NULL` + ), // "Published" must mean every column the public contract declares // non-optional is present. icon_url is genuinely optional and is left out. check( @@ -109,6 +121,12 @@ export const extensions = sqliteTable( ${table.version} IS NOT NULL AND ${table.downloadUrl} IS NOT NULL )` ) + // No CHECK tying delisted_at to published_at: SQLite cannot add a CHECK + // without a full table rebuild, and this table has a child + // (extension_revisions) that a rebuild's DROP TABLE cannot carry through + // a real transaction (see migration 0021's header). delist() is the only + // writer and already guards on published_at IS NOT NULL, so the invariant + // holds without spending a rebuild on it. ] ); diff --git a/src/services/extensions/v2/routes/moderation.ts b/src/services/extensions/v2/routes/moderation.ts index 46c33a7d..7cecbae4 100644 --- a/src/services/extensions/v2/routes/moderation.ts +++ b/src/services/extensions/v2/routes/moderation.ts @@ -9,6 +9,7 @@ import { } from "./errors"; import { ActiveAccountRequiredResponse, + DelistReasonSchema, IdParamSchema, PaginationSchema, ReviewNoteOptionalSchema, @@ -26,6 +27,7 @@ import { RevisionQueueQuerySchema } from "../schemas/revisions"; import { DeveloperProfilesDatabase } from "../db/developer-profiles"; +import { ExtensionsDatabase } from "../db/extensions"; import { ExtensionRevisionsDatabase } from "../db/revisions"; import { ExtensionsV2App } from "./app"; @@ -215,6 +217,69 @@ export function registerModerationRoutes(app: ExtensionsV2App): void { return c.json({ result: data }, 200); }); + // Distinct from reject: reject leaves a pending edit unpublished, delist + // pulls an already-published extension out of the catalogue entirely. See + // ExtensionsDatabase.delist() for why content and history are kept rather + // than cleared. + const delistRoute = createRoute({ + method: "post", + path: "/extensions/{id}/delist", + tags: ["Moderation"], + summary: "Remove a published extension from the public catalogue", + security: [{ Bearer: [] }], + middleware: [requireModerator()] as const, + request: { + params: IdParamSchema, + body: { + content: { "application/json": { schema: DelistReasonSchema } } + } + }, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ + result: z.object({ + id: z.string(), + status: z.literal("delisted") + }) + }) + } + }, + description: + "Extension removed from the public catalogue. Its content and " + + "history are kept, and its owner can still see and edit it." + }, + 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 not published, or is already delisted"), + 422: errorResponse("Path params or reason body failed validation"), + 500: errorResponse("Database error") + } + }); + + app.openapi(delistRoute, async (c) => { + const auth = getAuth(c); + const { id } = c.req.valid("param"); + const { reason } = c.req.valid("json"); + const db = new ExtensionsDatabase(getExtensionsDb(c.env.DB_EXTENSIONS)); + const { data, error } = await db.delist(id, auth.userId, reason); + if (error || !data) { + return c.json( + errorBody(error, "Unable to delist extension"), + statusFromWriteErrorCode(error?.code) + ); + } + return c.json( + { result: { id: data.id, status: "delisted" as const } }, + 200 + ); + }); + const allDevelopersRoute = createRoute({ method: "get", path: "/developers", diff --git a/src/services/extensions/v2/schemas/common.ts b/src/services/extensions/v2/schemas/common.ts index fc4d8ebf..7b043ff6 100644 --- a/src/services/extensions/v2/schemas/common.ts +++ b/src/services/extensions/v2/schemas/common.ts @@ -73,6 +73,13 @@ export const ReviewNoteRequiredSchema = z .strict() .openapi("ReviewNoteRequired"); +export const DelistReasonSchema = z + .object({ + reason: z.string().min(1).max(2000) + }) + .strict() + .openapi("DelistReason"); + export const PaginationSchema = z .object({ next_cursor: z.string().nullable(), diff --git a/src/services/extensions/v2/schemas/extensions.ts b/src/services/extensions/v2/schemas/extensions.ts index 346850f8..6f75e8bd 100644 --- a/src/services/extensions/v2/schemas/extensions.ts +++ b/src/services/extensions/v2/schemas/extensions.ts @@ -207,9 +207,22 @@ const PendingRevisionRefSchema = z }) .openapi("PendingRevisionRef"); -// published, pending_revision and last_review are independent — a live -// extension with an unreviewed edit has all three. There is deliberately no -// derived `status` field on top; see the README for how they map to a UI. +// Set once a moderator pulls an already-published extension from the +// catalogue for cause (its upstream source disappearing, for example). +// Content and history are kept, so the owner can still see and edit the +// extension - they just cannot get it back into the catalogue without a +// moderator re-listing it. +export const DelistedInfoSchema = z + .object({ + reason: z.string(), + at: z.string() + }) + .openapi("DelistedInfo"); + +// published, pending_revision, last_review and delisted are independent — a +// live extension with an unreviewed edit has all three of the first, and a +// delisted one keeps whichever of them it already had. There is deliberately +// no derived `status` field on top; see the README for how they map to a UI. export const OwnedExtensionListItemSchema = z .object({ id: z.string(), @@ -217,6 +230,7 @@ export const OwnedExtensionListItemSchema = z published: ExtensionCardContentSchema.nullable(), pending_revision: PendingRevisionRefSchema.nullable(), last_review: RevisionReviewSchema.nullable(), + delisted: DelistedInfoSchema.nullable(), created_at: z.string(), updated_at: z.string() }) diff --git a/test/services/extensions/v2/db-fixtures.ts b/test/services/extensions/v2/db-fixtures.ts index 4f6b8595..0b775a5a 100644 --- a/test/services/extensions/v2/db-fixtures.ts +++ b/test/services/extensions/v2/db-fixtures.ts @@ -43,6 +43,8 @@ export interface ExtensionRow { download_url: string | null; created_at: string; updated_at: string; + delisted_at: string | null; + delist_reason: string | null; } export interface RevisionRow { @@ -249,8 +251,9 @@ export async function insertExtension( `INSERT INTO extensions (id, developer_id, published_at, published_revision_id, type, name, description, releases, website, license, icon_url, readme, source, - version, download_url, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` + version, download_url, created_at, updated_at, delisted_at, + delist_reason) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` ) .bind( row.id, @@ -269,7 +272,9 @@ export async function insertExtension( row.version ?? "1.0.0", row.download_url ?? "https://e.com/d.zip", row.created_at ?? now, - row.updated_at ?? now + row.updated_at ?? now, + row.delisted_at ?? null, + row.delist_reason ?? null ) .run(); } diff --git a/test/services/extensions/v2/migrations.test.ts b/test/services/extensions/v2/migrations.test.ts index 88843028..139b7b1a 100644 --- a/test/services/extensions/v2/migrations.test.ts +++ b/test/services/extensions/v2/migrations.test.ts @@ -80,14 +80,17 @@ function seedSubmissionFixture(db: DatabaseSync): void { // it: foreign keys enforced, and 0021 inside the transaction wrangler wraps // each migration file in. That combination is what a local apply cannot see, // and it is what let a broken 0021 reach production. +// +// Held back by `candidate < "0021"` rather than `!== "0021"`: this file seeds +// pre-restructure rows and watches them cross 0021 specifically, so anything +// numbered after it (e.g. 0022, which assumes 0021's published_at/developer_id +// already exist) must stay held back too, not just 0021 itself. function applyAllAsD1( db: DatabaseSync, seed?: (db: DatabaseSync) => void ): void { db.exec("PRAGMA foreign_keys = ON;"); - for (const name of migrationNames.filter( - (candidate) => !candidate.startsWith("0021") - )) { + for (const name of migrationNames.filter((candidate) => candidate < "0021")) { db.exec(migration(name)); } seed?.(db); @@ -134,13 +137,12 @@ describe("Extensions D1 migrations", () => { // assertions prove that the adoption migration is the only schema // change needed for the old split-owned database. 0021 is held back // with it so this test can seed pre-0021 rows and then watch them - // migrate; the assertions after it cover the restructure. - const heldBack = new Set([ - "0019_add_user_deleted_at.sql", - "0021_restructure_extensions_revisions.sql" - ]); + // migrate; the assertions after it cover the restructure. Anything + // numbered 0021 or later (not just 0021 by name) stays out of this + // first pass too, since it may assume 0021 already ran. + const heldBack = new Set(["0019_add_user_deleted_at.sql"]); for (const name of migrationNames.filter( - (candidate) => !heldBack.has(candidate) + (candidate) => !heldBack.has(candidate) && candidate < "0021" )) { db.exec(migration(name)); } @@ -264,7 +266,7 @@ describe("Extensions D1 migrations", () => { try { for (const name of migrationNames.filter( - (candidate) => !candidate.startsWith("0021") + (candidate) => candidate < "0021" )) { db.exec(migration(name)); } @@ -368,7 +370,7 @@ describe("Extensions D1 migrations", () => { try { for (const name of migrationNames.filter( - (candidate) => !candidate.startsWith("0021") + (candidate) => candidate < "0021" )) { db.exec(migration(name)); } @@ -405,7 +407,7 @@ describe("Extensions D1 migrations", () => { try { for (const name of migrationNames.filter( - (candidate) => !candidate.startsWith("0021") + (candidate) => candidate < "0021" )) { db.exec(migration(name)); } @@ -450,7 +452,7 @@ describe("Extensions D1 migrations", () => { try { for (const name of migrationNames.filter( - (candidate) => !candidate.startsWith("0021") + (candidate) => candidate < "0021" )) { db.exec(migration(name)); } @@ -487,7 +489,7 @@ describe("Extensions D1 migrations", () => { try { for (const name of migrationNames.filter( - (candidate) => !candidate.startsWith("0021") + (candidate) => candidate < "0021" )) { db.exec(migration(name)); } @@ -614,7 +616,7 @@ describe("Extensions D1 migrations", () => { try { db.exec("PRAGMA foreign_keys = ON;"); for (const name of migrationNames.filter( - (candidate) => !candidate.startsWith("0021") + (candidate) => candidate < "0021" )) { db.exec(migration(name)); } diff --git a/test/services/extensions/v2/moderation.test.ts b/test/services/extensions/v2/moderation.test.ts index f39ab1af..452b35be 100644 --- a/test/services/extensions/v2/moderation.test.ts +++ b/test/services/extensions/v2/moderation.test.ts @@ -602,6 +602,131 @@ describe("Extensions API v2", () => { }); }); + describe("POST /extensions/{id}/delist", () => { + it("requires moderator access", async () => { + 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/delist", + await authHeaders("user-1"), + { reason: "Upstream source removed" } + ); + expect(res.status).toBe(403); + }); + + 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/delist", + await authHeaders("mod-1"), + { reason: "Upstream source removed" } + ); + expect(res.status).toBe(404); + }); + + it("409s for an extension that was never published", 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/delist", + await authHeaders("mod-1"), + { reason: "Upstream source removed" } + ); + expect(res.status).toBe(409); + }); + + it("422s on an empty reason", 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/delist", + await authHeaders("mod-1"), + { reason: "" } + ); + expect(res.status).toBe(422); + }); + + it("removes a published extension from the public catalogue and records why", 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/delist", + await authHeaders("mod-1"), + { reason: "Upstream source removed" } + ); + expect(res.status).toBe(200); + await expect(res.json()).resolves.toEqual({ + result: { id: "live-ext", status: "delisted" } + }); + + // Case-insensitively addressed, like every other extension route. + expect(await getExtension(db, "LIVE-ext")).toMatchObject({ + delist_reason: "Upstream source removed" + }); + + expect((await get("/extensions/v2/extensions", {})).status).toBe(200); + await expect( + (await get("/extensions/v2/extensions", {})).json() + ).resolves.toMatchObject({ result: [] }); + expect((await get("/extensions/v2/extensions/live-ext", {})).status).toBe( + 404 + ); + + // The owner can still see it, plus why it was pulled. + const mine = await get( + "/extensions/v2/extensions/mine/live-ext", + await authHeaders("user-1") + ); + await expect(mine.json()).resolves.toMatchObject({ + result: { + delisted: { reason: "Upstream source removed" }, + published: { name: "Extension" } + } + }); + }); + + it("409s on a second delist of the same extension", 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 mod = await authHeaders("mod-1"); + await post("/extensions/v2/extensions/live-ext/delist", mod, { + reason: "First reason" + }); + + const res = await post("/extensions/v2/extensions/live-ext/delist", mod, { + reason: "Second reason" + }); + expect(res.status).toBe(409); + expect(await getExtension(db, "live-ext")).toMatchObject({ + delist_reason: "First reason" + }); + }); + }); + describe("developer moderation", () => { it("binds approval to the exact profile revision reviewed", async () => { await put( diff --git a/test/services/extensions/v2/public-extensions.test.ts b/test/services/extensions/v2/public-extensions.test.ts index 656a89a3..05144dd9 100644 --- a/test/services/extensions/v2/public-extensions.test.ts +++ b/test/services/extensions/v2/public-extensions.test.ts @@ -217,5 +217,28 @@ describe("Extensions API v2", () => { const res = await get("/extensions/v2/extensions/no-such-extension", {}); expect(res.status).toBe(404); }); + + it("404s for a delisted extension, even though it is still published", async () => { + await insertDeveloper(db, { + id: "catalogue-developer", + type: "user", + name: "Catalogue Developer", + url: null, + owner_user_id: null + }); + await insertExtension(db, { + id: "delisted-ext", + developer_id: "catalogue-developer", + delisted_at: "2026-01-01T00:00:00.000Z", + delist_reason: "Upstream source removed" + }); + + expect( + (await get("/extensions/v2/extensions/delisted-ext", {})).status + ).toBe(404); + + const list = await get("/extensions/v2/extensions", {}); + await expect(list.json()).resolves.toMatchObject({ result: [] }); + }); }); }); From 838bd1be832198e2d6b3ccb54ad206ec7ec25a30 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 4 Sep 2026 08:53:34 +0100 Subject: [PATCH 2/6] Deslop: match moderation.ts's error-handling style for the new route --- src/services/extensions/v2/routes/moderation.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/services/extensions/v2/routes/moderation.ts b/src/services/extensions/v2/routes/moderation.ts index 7cecbae4..82b49f69 100644 --- a/src/services/extensions/v2/routes/moderation.ts +++ b/src/services/extensions/v2/routes/moderation.ts @@ -269,10 +269,8 @@ export function registerModerationRoutes(app: ExtensionsV2App): void { const db = new ExtensionsDatabase(getExtensionsDb(c.env.DB_EXTENSIONS)); const { data, error } = await db.delist(id, auth.userId, reason); if (error || !data) { - return c.json( - errorBody(error, "Unable to delist extension"), - statusFromWriteErrorCode(error?.code) - ); + const status = statusFromWriteErrorCode(error?.code); + return c.json(errorBody(error, "Unable to delist extension"), status); } return c.json( { result: { id: data.id, status: "delisted" as const } }, From d3cb3d778f59f543274057de32d7a6b7e7b3b0c1 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 4 Sep 2026 09:03:03 +0100 Subject: [PATCH 3/6] Address cubic review: v1 catalogue leak, stale updated_at, whitespace reasons - v1's /extensions/v1/list and /:id endpoints share the extensions table with v2 but filtered only on published_at, so a delisted extension (e.g. the Paygate case this PR exists for) stayed visible to FOSSBilling installs even after v2 hid it. Both v1 queries now also require delisted_at IS NULL, with regression tests. - delist() left extensions.updated_at untouched, unlike every other write to this table (approve()'s publish statement, for one). Now bumps it. - delistBlockedError()'s diagnostic SELECT wasn't wrapped in try/catch, so a transient DB error there would throw out of delist() as a raw exception instead of the structured DatabaseResult the moderation route expects. Wrapped, matching the rest of this file's error handling. - DelistReasonSchema's min(1) let a whitespace-only reason through, which would then be shown to the owner as "why" verbatim. Added .trim(). - README's 'Reading Owner State' intro still said 'three independent fields' after the delisted paragraph introduced a fourth; fixed the count. Not changed: cubic also flagged delist() returning the caller-supplied id verbatim instead of resolving to canonical casing. Checked against withdraw() and create() in the same file - both do exactly the same thing (echo the input id, not a re-fetched canonical one), so this is consistent with, not a deviation from, how every other id-addressed write here already behaves. Replying on the review thread with this instead of changing it. Also not changed here: ReviewNoteRequiredSchema (used by reject and by developer-claim rejection) has the identical missing-trim gap DelistReasonSchema had. Pre-existing and outside this PR's diff; flagging separately rather than expanding scope. --- src/services/extensions/v1/database.ts | 15 ++++++--- src/services/extensions/v2/README.md | 7 ++-- src/services/extensions/v2/db/extensions.ts | 23 ++++++++----- src/services/extensions/v2/schemas/common.ts | 4 ++- test/services/extensions/v1/index.test.ts | 33 +++++++++++++++++++ .../services/extensions/v2/moderation.test.ts | 22 ++++++++++++- 6 files changed, 88 insertions(+), 16 deletions(-) diff --git a/src/services/extensions/v1/database.ts b/src/services/extensions/v1/database.ts index a388a6c3..2098f3ae 100644 --- a/src/services/extensions/v1/database.ts +++ b/src/services/extensions/v1/database.ts @@ -1,4 +1,4 @@ -import { and, eq, isNotNull, sql } from "drizzle-orm"; +import { and, eq, isNotNull, isNull, sql } from "drizzle-orm"; import { ExtensionsDb } from "../../../lib/db"; import { extensions, developers } from "../v2/db/schema"; import { DatabaseResult } from "../../../lib/interfaces"; @@ -39,7 +39,10 @@ const EXTENSION_COLUMNS = { // non-null here because both queries below filter on published_at IS NOT NULL // and extensions_published_content_check makes that filter sufficient - a // published row cannot be missing any of them. That filter is also what keeps -// unreviewed extensions out of the v1 catalogue. +// unreviewed extensions out of the v1 catalogue. delisted_at IS NULL is the +// other half: v1 shares this table with v2's public catalogue and must stay +// in sync with what v2 hides, or a moderator delisting an extension would +// pull it from the v2 catalogue while it stayed visible here. interface ExtensionRow { id: string; type: string; @@ -65,7 +68,10 @@ export class ExtensionsDatabase { async getAllExtensions(type?: string): Promise> { let rows: ExtensionRow[]; try { - const published = isNotNull(extensions.publishedAt); + const published = and( + isNotNull(extensions.publishedAt), + isNull(extensions.delistedAt) + ); rows = (await this.db .select(EXTENSION_COLUMNS) .from(extensions) @@ -96,7 +102,8 @@ export class ExtensionsDatabase { .where( and( sql`LOWER(${extensions.id}) = LOWER(${id})`, - isNotNull(extensions.publishedAt) + isNotNull(extensions.publishedAt), + isNull(extensions.delistedAt) ) )) as ExtensionRow[]; } catch (error) { diff --git a/src/services/extensions/v2/README.md b/src/services/extensions/v2/README.md index a52de4c8..4102503b 100644 --- a/src/services/extensions/v2/README.md +++ b/src/services/extensions/v2/README.md @@ -44,9 +44,12 @@ most one developer profile, so no request body names one. ### Reading Owner State -`GET /extensions/mine` and `GET /extensions/mine/{id}` return three independent +`GET /extensions/mine` and `GET /extensions/mine/{id}` return four independent fields rather than a single derived status, because together they are the -state and a derived enum could only disagree with them: +state and a derived enum could only disagree with them. The table below +covers three of them - `published`, `pending_revision` and `last_review` - the +fourth, `delisted`, is documented separately just below since it is orthogonal +to all three: | `published` | `pending_revision` | `last_review` | Meaning | | ----------- | ------------------ | ------------- | --------------------------------------- | diff --git a/src/services/extensions/v2/db/extensions.ts b/src/services/extensions/v2/db/extensions.ts index 08fb0b2e..9f691030 100644 --- a/src/services/extensions/v2/db/extensions.ts +++ b/src/services/extensions/v2/db/extensions.ts @@ -552,7 +552,8 @@ export class ExtensionsDatabase { .update(extensions) .set({ delistedAt: sql`CURRENT_TIMESTAMP`, - delistReason: reason + delistReason: reason, + updatedAt: sql`CURRENT_TIMESTAMP` }) .where( and( @@ -585,13 +586,19 @@ export class ExtensionsDatabase { const inactive = await inactiveActorError(this.db, moderatorId); if (inactive) return { data: null, error: inactive }; - const [existing] = await this.db - .select({ - publishedAt: extensions.publishedAt, - delistedAt: extensions.delistedAt - }) - .from(extensions) - .where(sql`LOWER(${extensions.id}) = LOWER(${id})`); + 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("delist", error); + } if (!existing) return notFound(id); if (!existing.publishedAt) { return { diff --git a/src/services/extensions/v2/schemas/common.ts b/src/services/extensions/v2/schemas/common.ts index 7b043ff6..643702fb 100644 --- a/src/services/extensions/v2/schemas/common.ts +++ b/src/services/extensions/v2/schemas/common.ts @@ -75,7 +75,9 @@ export const ReviewNoteRequiredSchema = z export const DelistReasonSchema = z .object({ - reason: z.string().min(1).max(2000) + // Shown to the owner as-is, so a whitespace-only value must not satisfy + // min(1) the way an untrimmed string would. + reason: z.string().trim().min(1).max(2000) }) .strict() .openapi("DelistReason"); diff --git a/test/services/extensions/v1/index.test.ts b/test/services/extensions/v1/index.test.ts index 08bd07db..e8d128c4 100644 --- a/test/services/extensions/v1/index.test.ts +++ b/test/services/extensions/v1/index.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, beforeAll, beforeEach } from "vitest"; +import { eq } from "drizzle-orm"; import { createExecutionContext, waitOnExecutionContext @@ -138,6 +139,24 @@ describe("Extensions API v1", () => { expect(res.status).toBe(301); }); + // v1 and v2 share the extensions table and must agree on what counts as + // published - a moderator delisting an extension in v2 must also pull it + // from here, or FOSSBilling installs would keep seeing it. + it("should exclude a delisted extension", async () => { + const db = getExtensionsDb(env.DB_EXTENSIONS); + await db + .update(extensions) + .set({ delistedAt: "2026-01-01T00:00:00.000Z" }) + .where(eq(extensions.id, "Example")); + + const ctx = createExecutionContext(); + const res = await app.request("/extensions/v1/list", {}, env, ctx); + await waitOnExecutionContext(ctx); + + const data = (await res.json()) as { result: Array<{ id: string }> }; + expect(data.result.map((e) => e.id)).toEqual(["TestTheme"]); + }); + it("should parse releases in descending order", async () => { const ctx = createExecutionContext(); const res = await app.request("/extensions/v1/list", {}, env, ctx); @@ -186,6 +205,20 @@ describe("Extensions API v1", () => { expect(data.error.message).toContain("nonexistent"); }); + it("should return 404 for a delisted extension", async () => { + const db = getExtensionsDb(env.DB_EXTENSIONS); + await db + .update(extensions) + .set({ delistedAt: "2026-01-01T00:00:00.000Z" }) + .where(eq(extensions.id, "Example")); + + const ctx = createExecutionContext(); + const res = await app.request("/extensions/v1/Example", {}, env, ctx); + await waitOnExecutionContext(ctx); + + expect(res.status).toBe(404); + }); + it("should include parsed author object", async () => { const ctx = createExecutionContext(); const res = await app.request("/extensions/v1/Example", {}, env, ctx); diff --git a/test/services/extensions/v2/moderation.test.ts b/test/services/extensions/v2/moderation.test.ts index 452b35be..71977392 100644 --- a/test/services/extensions/v2/moderation.test.ts +++ b/test/services/extensions/v2/moderation.test.ts @@ -661,12 +661,29 @@ describe("Extensions API v2", () => { expect(res.status).toBe(422); }); + it("422s on a whitespace-only reason", 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/delist", + await authHeaders("mod-1"), + { reason: " " } + ); + expect(res.status).toBe(422); + }); + it("removes a published extension from the public catalogue and records why", 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" + developer_id: "new-developer", + updated_at: "2020-01-01T00:00:00.000Z" }); const res = await post( @@ -683,6 +700,9 @@ describe("Extensions API v2", () => { expect(await getExtension(db, "LIVE-ext")).toMatchObject({ delist_reason: "Upstream source removed" }); + expect((await getExtension(db, "LIVE-ext"))?.updated_at).not.toBe( + "2020-01-01T00:00:00.000Z" + ); expect((await get("/extensions/v2/extensions", {})).status).toBe(200); await expect( From df8dd820f26c3da816ce99a7b6c4ec12ba21bc8d Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 4 Sep 2026 09:10:24 +0100 Subject: [PATCH 4/6] Trim ReviewNoteRequiredSchema the same way DelistReasonSchema was fixed Follow-up to the cubic finding on DelistReasonSchema's whitespace-only reason: review_note (used by reject and by developer-claim rejection) had the identical gap. Added .trim() before .min(1) there too, with a whitespace-only regression test on both routes. --- src/services/extensions/v2/schemas/common.ts | 4 +++- .../services/extensions/v2/moderation.test.ts | 13 ++++++++++++ test/services/extensions/v2/ownership.test.ts | 20 +++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/services/extensions/v2/schemas/common.ts b/src/services/extensions/v2/schemas/common.ts index 643702fb..78e9e89b 100644 --- a/src/services/extensions/v2/schemas/common.ts +++ b/src/services/extensions/v2/schemas/common.ts @@ -68,7 +68,9 @@ export const ReviewNoteOptionalSchema = z export const ReviewNoteRequiredSchema = z .object({ - review_note: z.string().min(1).max(2000) + // Shown to the submitter as-is, so a whitespace-only value must not + // satisfy min(1) the way an untrimmed string would - see DelistReasonSchema. + review_note: z.string().trim().min(1).max(2000) }) .strict() .openapi("ReviewNoteRequired"); diff --git a/test/services/extensions/v2/moderation.test.ts b/test/services/extensions/v2/moderation.test.ts index 71977392..f23bf160 100644 --- a/test/services/extensions/v2/moderation.test.ts +++ b/test/services/extensions/v2/moderation.test.ts @@ -517,6 +517,19 @@ describe("Extensions API v2", () => { expect(res.status).toBe(422); }); + it("rejects a whitespace-only review_note", async () => { + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + await seedDeveloper("new-developer", "user-1"); + const { id, revisionId } = await createPending("user-1"); + + const res = await post( + reviewPath(id, revisionId, "reject"), + await authHeaders("mod-1"), + { review_note: " " } + ); + expect(res.status).toBe(422); + }); + it("rejects a revision with a note and leaves the extension unpublished", async () => { await insertUser(db, { id: "mod-1", is_moderator: 1 }); await seedDeveloper("new-developer", "user-1"); diff --git a/test/services/extensions/v2/ownership.test.ts b/test/services/extensions/v2/ownership.test.ts index 0a1c164c..103c6998 100644 --- a/test/services/extensions/v2/ownership.test.ts +++ b/test/services/extensions/v2/ownership.test.ts @@ -1023,6 +1023,26 @@ describe("Extensions API v2", () => { ).toBeNull(); }); + it("rejects a whitespace-only review_note when rejecting a claim", async () => { + await seedUnownedDeveloper("legacy-developer"); + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + + const claim = await post( + "/extensions/v2/developers/legacy-developer/claim", + await authHeaders("user-1"), + {} + ); + const claimId = ((await claim.json()) as { result: { id: string } }) + .result.id; + + const reject = await post( + `/extensions/v2/developers/claims/${claimId}/reject`, + await authHeaders("mod-1"), + { review_note: " " } + ); + expect(reject.status).toBe(422); + }); + it("verifies a claim when the claimant's linked GitHub org matches the developer id", async () => { await insertDeveloper(db, { id: "legacy-developer", From c8d02c265f5b95277268986366c25228a62c3830 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 4 Sep 2026 09:35:15 +0100 Subject: [PATCH 5/6] Add access-control regression test for delisted extensions Confirms the current state: the owner can still see a delisted extension's full record (including the delist reason) via GET /extensions/mine/{id}, unaffected by delisted state since getOwned() never filters on it. A moderator's only access to a delisted extension besides delisting it is GET /extensions/{id}/revisions (the one owner-extensions.ts route with a moderator bypass on the ownership check) - which returns revision history, not the delist reason/timestamp itself. Anyone else gets 403 from both routes. --- .../services/extensions/v2/moderation.test.ts | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/test/services/extensions/v2/moderation.test.ts b/test/services/extensions/v2/moderation.test.ts index f23bf160..4c8419f0 100644 --- a/test/services/extensions/v2/moderation.test.ts +++ b/test/services/extensions/v2/moderation.test.ts @@ -758,6 +758,55 @@ describe("Extensions API v2", () => { delist_reason: "First reason" }); }); + + // Public reads are covered in public-extensions.test.ts. This is about + // who can still reach a delisted extension's full record once it is out + // of the catalogue: the owner via GET /extensions/mine/{id} (ownership + // check, unaffected by delisted state - see getOwned()), a moderator via + // GET /extensions/{id}/revisions (the only route a moderator has that + // isn't scoped to their own developer profile), and no one else. + it("only the owner or a moderator can still reach a delisted extension", 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 post( + "/extensions/v2/extensions/live-ext/delist", + await authHeaders("mod-1"), + { + reason: "Upstream source removed" + } + ); + + const owner = await get( + "/extensions/v2/extensions/mine/live-ext", + await authHeaders("user-1") + ); + expect(owner.status).toBe(200); + await expect(owner.json()).resolves.toMatchObject({ + result: { delisted: { reason: "Upstream source removed" } } + }); + + const moderator = await get( + "/extensions/v2/extensions/live-ext/revisions", + await authHeaders("mod-1") + ); + expect(moderator.status).toBe(200); + + const stranger = await get( + "/extensions/v2/extensions/mine/live-ext", + await authHeaders("user-2") + ); + expect(stranger.status).toBe(403); + + const strangerRevisions = await get( + "/extensions/v2/extensions/live-ext/revisions", + await authHeaders("user-2") + ); + expect(strangerRevisions.status).toBe(403); + }); }); describe("developer moderation", () => { From 421f7e6010ba686f0b8156143f830e719017c533 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 4 Sep 2026 10:00:23 +0100 Subject: [PATCH 6/6] Add GET /moderation/extensions/{id}: a moderator's full-record read Closes the gap the delisted-extension access-control check surfaced: a moderator could delist an extension but had no dedicated way to see the delisted state (reason/timestamp) afterward - only the owner could, via GET /extensions/mine/{id}. Adds a moderator-only equivalent under /moderation rather than reusing GET /extensions/{id} (public, published-only) or GET /extensions/mine/{id} (owner-only, 403s everyone else) - same OwnedExtension response shape, resolved via the existing getOwned() rather than a new query. Namespaced /moderation/extensions/{id} specifically to avoid colliding with public-extensions.ts's GET /extensions/{id}, which is registered on the same app and would otherwise win or lose ordering-dependently. Updated the access-control regression test to use this instead of the GET /extensions/{id}/revisions workaround it previously relied on to prove moderator access existed at all. --- src/services/extensions/v2/README.md | 5 ++ .../extensions/v2/routes/moderation.ts | 49 +++++++++++ .../services/extensions/v2/moderation.test.ts | 83 ++++++++++++++++++- 3 files changed, 134 insertions(+), 3 deletions(-) diff --git a/src/services/extensions/v2/README.md b/src/services/extensions/v2/README.md index 4102503b..3e6cebae 100644 --- a/src/services/extensions/v2/README.md +++ b/src/services/extensions/v2/README.md @@ -36,6 +36,11 @@ against it. `reject`: content and history are kept, so the owner can still see and edit the extension, and a moderator can re-list it by hand later. There is no `relist` endpoint yet - see `ExtensionsDatabase.delist()`. +- `GET /moderation/extensions/{id}` is a moderator's only full-record read of + an extension they don't own - the same `OwnedExtension` shape as + `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`. 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/routes/moderation.ts b/src/services/extensions/v2/routes/moderation.ts index 82b49f69..eed3f7b3 100644 --- a/src/services/extensions/v2/routes/moderation.ts +++ b/src/services/extensions/v2/routes/moderation.ts @@ -26,6 +26,7 @@ import { RevisionIdParamSchema, RevisionQueueQuerySchema } from "../schemas/revisions"; +import { OwnedExtensionSchema } from "../schemas/extensions"; import { DeveloperProfilesDatabase } from "../db/developer-profiles"; import { ExtensionsDatabase } from "../db/extensions"; import { ExtensionRevisionsDatabase } from "../db/revisions"; @@ -91,6 +92,54 @@ export function registerModerationRoutes(app: ExtensionsV2App): void { ); }); + // 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 + // anyone else) - a moderator needs the owner's full view, including + // `delisted`, for an extension that isn't theirs. Same shape as + // GET /extensions/mine/{id} for that reason. + const getExtensionRoute = createRoute({ + method: "get", + path: "/moderation/extensions/{id}", + tags: ["Moderation"], + summary: "Get any extension's full record, including a delisted one", + security: [{ Bearer: [] }], + middleware: [requireModerator()] as const, + request: { params: IdParamSchema }, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ result: OwnedExtensionSchema }) + } + }, + description: + "The extension's live content, its unreviewed edit if any, the last moderator decision, and its delist state" + }, + 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"), + 422: errorResponse("id param failed validation"), + 500: errorResponse("Database error") + } + }); + + app.openapi(getExtensionRoute, async (c) => { + const { id } = c.req.valid("param"); + const db = new ExtensionsDatabase(getExtensionsDb(c.env.DB_EXTENSIONS)); + const { data, error } = await db.getOwned(id); + if (error || !data) { + return c.json( + errorBody(error, "Extension not found"), + statusFromErrorCode(error?.code, false) + ); + } + return c.json({ result: data.extension }, 200); + }); + // Reviews are addressed through the extension they belong to. The revision // id alone would be enough to find the row, but scoping the path to the // extension means a moderator acting from a queue entry cannot approve a diff --git a/test/services/extensions/v2/moderation.test.ts b/test/services/extensions/v2/moderation.test.ts index 4c8419f0..0b3e419d 100644 --- a/test/services/extensions/v2/moderation.test.ts +++ b/test/services/extensions/v2/moderation.test.ts @@ -103,6 +103,75 @@ describe("Extensions API v2", () => { }); }); + describe("GET /moderation/extensions/{id}", () => { + it("requires moderator access", async () => { + await seedDeveloper("new-developer", "user-1"); + await insertExtension(db, { + id: "live-ext", + developer_id: "new-developer" + }); + + const res = await get( + "/extensions/v2/moderation/extensions/live-ext", + await authHeaders("user-1") + ); + expect(res.status).toBe(403); + }); + + it("404s for an unknown extension", async () => { + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + + const res = await get( + "/extensions/v2/moderation/extensions/no-such-extension", + await authHeaders("mod-1") + ); + expect(res.status).toBe(404); + }); + + it("gets a published extension's full record, case-insensitively", 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 get( + "/extensions/v2/moderation/extensions/live-ext", + await authHeaders("mod-1") + ); + expect(res.status).toBe(200); + const data = (await res.json()) as { + result: { id: string; published: { name: string } | null }; + }; + expect(data.result.id).toBe("LIVE-ext"); + expect(data.result.published?.name).toBe("Extension"); + }); + + it("gets a delisted extension's record, including why it was delisted", 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 post( + "/extensions/v2/extensions/live-ext/delist", + await authHeaders("mod-1"), + { reason: "Upstream source removed" } + ); + + const res = await get( + "/extensions/v2/moderation/extensions/live-ext", + await authHeaders("mod-1") + ); + expect(res.status).toBe(200); + await expect(res.json()).resolves.toMatchObject({ + result: { delisted: { reason: "Upstream source removed" } } + }); + }); + }); + 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 }); @@ -763,8 +832,7 @@ describe("Extensions API v2", () => { // who can still reach a delisted extension's full record once it is out // of the catalogue: the owner via GET /extensions/mine/{id} (ownership // check, unaffected by delisted state - see getOwned()), a moderator via - // GET /extensions/{id}/revisions (the only route a moderator has that - // isn't scoped to their own developer profile), and no one else. + // GET /moderation/extensions/{id}, and no one else. it("only the owner or a moderator can still reach a delisted extension", async () => { await insertUser(db, { id: "mod-1", is_moderator: 1 }); await seedDeveloper("new-developer", "user-1"); @@ -790,10 +858,13 @@ describe("Extensions API v2", () => { }); const moderator = await get( - "/extensions/v2/extensions/live-ext/revisions", + "/extensions/v2/moderation/extensions/live-ext", await authHeaders("mod-1") ); expect(moderator.status).toBe(200); + await expect(moderator.json()).resolves.toMatchObject({ + result: { delisted: { reason: "Upstream source removed" } } + }); const stranger = await get( "/extensions/v2/extensions/mine/live-ext", @@ -801,6 +872,12 @@ describe("Extensions API v2", () => { ); expect(stranger.status).toBe(403); + const strangerModerationRead = await get( + "/extensions/v2/moderation/extensions/live-ext", + await authHeaders("user-2") + ); + expect(strangerModerationRead.status).toBe(403); + const strangerRevisions = await get( "/extensions/v2/extensions/live-ext/revisions", await authHeaders("user-2")