diff --git a/src/services/extensions/v2/db/migrations/0011_harden_review_workflows.sql b/src/services/extensions/v2/db/migrations/0011_harden_review_workflows.sql new file mode 100644 index 0000000..9768b4c --- /dev/null +++ b/src/services/extensions/v2/db/migrations/0011_harden_review_workflows.sql @@ -0,0 +1,33 @@ +-- Bind moderation decisions to the ownership/content state that was reviewed. +ALTER TABLE developers + ADD COLUMN ownership_epoch INTEGER NOT NULL DEFAULT 1 CHECK (ownership_epoch >= 1); +ALTER TABLE developers + ADD COLUMN content_revision INTEGER NOT NULL DEFAULT 1 CHECK (content_revision >= 1); +ALTER TABLE developers + ADD COLUMN approved_revision INTEGER; +ALTER TABLE developers + ADD COLUMN approved_by TEXT; + +-- Preserve approvals that predate revision tracking. +UPDATE developers +SET approved_revision = content_revision +WHERE approved_at IS NOT NULL; + +ALTER TABLE extension_submissions + ADD COLUMN ownership_epoch INTEGER NOT NULL DEFAULT 1 CHECK (ownership_epoch >= 1); +ALTER TABLE extension_submissions + ADD COLUMN target_key TEXT; + +UPDATE extension_submissions +SET target_key = LOWER(COALESCE(extension_id, json_extract(payload, '$.extension.id'))); + +-- Deployments with duplicate pending rows must reconcile them before applying +-- this migration; silently choosing one would mutate moderation state. +CREATE UNIQUE INDEX idx_extension_submissions_pending_target + ON extension_submissions(target_key) + WHERE status = 'pending'; + +CREATE INDEX idx_extension_submissions_submitter_page + ON extension_submissions(submitted_by, created_at DESC, id DESC); +CREATE INDEX idx_extension_submissions_queue_page + ON extension_submissions(status, created_at ASC, id ASC); diff --git a/src/services/extensions/v2/developers-database.ts b/src/services/extensions/v2/developers-database.ts index afc42b8..450f033 100644 --- a/src/services/extensions/v2/developers-database.ts +++ b/src/services/extensions/v2/developers-database.ts @@ -54,7 +54,12 @@ function parseDeveloperRow(row: Record): DeveloperProfile { URL: (row.url as string | null) ?? undefined, avatar_url: (row.avatar_url as string | null) ?? undefined, contact_email: (row.contact_email as string | null) ?? undefined, - approved: row.approved_at !== null && row.approved_at !== undefined + approved: + row.approved_at !== null && + row.approved_at !== undefined && + (row.approved_revision == null || + Number(row.approved_revision) === Number(row.content_revision ?? 1)), + content_revision: Number(row.content_revision ?? 1) }; } @@ -133,8 +138,12 @@ export class DevelopersDatabase { // approval no longer applies. Not worth diffing old vs. new values. mainStmt = this.db .prepare( - `UPDATE developers SET type = ?, name = ?, url = ?, avatar_url = ?, contact_email = ?, approved_at = NULL, updated_at = CURRENT_TIMESTAMP - WHERE id = ?` + `UPDATE developers + SET type = ?, name = ?, url = ?, avatar_url = ?, contact_email = ?, + content_revision = content_revision + 1, + approved_at = NULL, approved_revision = NULL, approved_by = NULL, + updated_at = CURRENT_TIMESTAMP + WHERE id = ? AND owner_user_id = ?` ) .bind( developer.type, @@ -142,7 +151,8 @@ export class DevelopersDatabase { developer.URL ?? null, developer.avatar_url ?? null, developer.contact_email ?? null, - developer.id + developer.id, + userId ); } @@ -156,7 +166,8 @@ export class DevelopersDatabase { const historyStmt = this.db .prepare( `INSERT INTO developer_history (id, developer_id, type, name, url, changed_by, changed_at) - VALUES (?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)` + SELECT ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP + WHERE changes() = 1` ) .bind( crypto.randomUUID(), @@ -172,6 +183,7 @@ export class DevelopersDatabase { results = (await this.db.batch([mainStmt, historyStmt])) as Array<{ success: boolean; error?: string; + meta?: { changes?: number }; }>; } catch (error) { if (isOwnerConflict(error instanceof Error ? error.message : "")) { @@ -203,7 +215,30 @@ export class DevelopersDatabase { ); } - return this.getById(developer.id); + if (!results[0]?.meta?.changes) { + return { + data: null, + error: { + message: "Developer ownership changed while updating the profile", + code: "CONFLICT" + } + }; + } + + const current = await this.db + .prepare("SELECT * FROM developers WHERE id = ? AND owner_user_id = ?") + .bind(developer.id, userId) + .first>(); + if (!current) { + return { + data: null, + error: { + message: "Developer ownership changed while updating the profile", + code: "CONFLICT" + } + }; + } + return { data: parseDeveloperRow(current), error: null }; } catch (error) { return databaseError("upsertOwn", error); } @@ -461,15 +496,21 @@ export class DevelopersDatabase { } async approve( - id: string + id: string, + expectedRevision: number, + reviewerId: string ): Promise> { let result; try { result = await this.db .prepare( - "UPDATE developers SET approved_at = CURRENT_TIMESTAMP WHERE id = ?" + `UPDATE developers + SET approved_at = CURRENT_TIMESTAMP, + approved_revision = content_revision, + approved_by = ? + WHERE id = ? AND content_revision = ?` ) - .bind(id) + .bind(reviewerId, id, expectedRevision) .run(); } catch (error) { return databaseError("approve", error); @@ -483,13 +524,23 @@ export class DevelopersDatabase { } if (!result.meta?.changes) { - return { - data: null, - error: { - message: `Cannot find developer by id: ${id}`, - code: "NOT_FOUND" - } - }; + const existing = await this.getById(id); + return existing.error + ? { + data: null, + error: { + message: `Cannot find developer by id: ${id}`, + code: "NOT_FOUND" + } + } + : { + data: null, + error: { + message: + "Developer profile changed after it was reviewed; reload it and approve the current revision", + code: "CONFLICT" + } + }; } return { data: { id, approved: true }, error: null }; @@ -564,9 +615,7 @@ export class DevelopersDatabase { crypto.randomUUID().replace(/-/g, "") + crypto.randomUUID().replace(/-/g, ""); const tokenHash = await sha256Hex(token); - const expiresAt = toSqliteDatetime( - new Date(Date.now() + 24 * 60 * 60 * 1000) - ); + const expiresAt = toSqliteDatetime(new Date(Date.now() + 60 * 60 * 1000)); if (!this.db.batch) { return databaseError( @@ -730,7 +779,12 @@ export class DevelopersDatabase { .bind(userId, tokenHash, userId); const updateDeveloperStmt = this.db .prepare( - `UPDATE developers SET owner_user_id = ?, approved_at = NULL, updated_at = CURRENT_TIMESTAMP + `UPDATE developers + SET owner_user_id = ?, + ownership_epoch = ownership_epoch + 1, + content_revision = content_revision + 1, + approved_at = NULL, approved_revision = NULL, approved_by = NULL, + updated_at = CURRENT_TIMESTAMP WHERE changes() = 1 AND id = ( SELECT developer_id FROM developer_transfers @@ -739,11 +793,27 @@ export class DevelopersDatabase { ) .bind(userId, tokenHash, userId); + const rejectPendingStmt = this.db + .prepare( + `UPDATE extension_submissions + SET status = 'rejected', + review_note = 'Ownership changed before review', + reviewed_at = CURRENT_TIMESTAMP + WHERE changes() = 1 + AND developer_id = ( + SELECT developer_id FROM developer_transfers + WHERE token_hash = ? AND accepted_by = ? AND accepted_at IS NOT NULL + ) + AND status = 'pending'` + ) + .bind(tokenHash, userId); + let results; try { results = (await this.db.batch([ claimStmt, - updateDeveloperStmt + updateDeveloperStmt, + rejectPendingStmt ])) as Array<{ success: boolean; error?: string; @@ -1125,7 +1195,12 @@ export class DevelopersDatabase { try { const developerStmt = this.db .prepare( - `UPDATE developers SET owner_user_id = ?, approved_at = NULL, updated_at = CURRENT_TIMESTAMP + `UPDATE developers + SET owner_user_id = ?, + ownership_epoch = ownership_epoch + 1, + content_revision = content_revision + 1, + approved_at = NULL, approved_revision = NULL, approved_by = NULL, + updated_at = CURRENT_TIMESTAMP WHERE id = ? AND owner_user_id IS NULL` ) .bind(claim.claimant_id, claim.developer_id); diff --git a/src/services/extensions/v2/index.ts b/src/services/extensions/v2/index.ts index 4452656..017f39e 100644 --- a/src/services/extensions/v2/index.ts +++ b/src/services/extensions/v2/index.ts @@ -8,6 +8,7 @@ import { getAuth, requireAuth } from "../../../lib/auth"; import { ClaimNoteSchema, DeveloperClaimSchema, + DeveloperApprovalSchema, DeveloperHistoryEntrySchema, DeveloperProfileSchema, DeveloperSchema, @@ -17,13 +18,15 @@ import { ExtensionSchema, IdParamSchema, PendingDeveloperClaimSchema, + PaginationSchema, PublicDeveloperSchema, QueueQuerySchema, ReviewNoteOptionalSchema, ReviewNoteRequiredSchema, SubmissionPayloadSchema, + SubmissionPageQuerySchema, SubmissionSchema, - TokenParamSchema, + TransferAcceptanceSchema, toPublicDeveloper } from "./interfaces"; import { DevelopersDatabase } from "./developers-database"; @@ -214,6 +217,11 @@ const createSubmissionRoute = createRoute({ content: { "application/json": { schema: ErrorResponseSchema } }, description: "Caller does not own the target developer or extension" }, + 409: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: + "Ownership or target changed, a duplicate is pending, or the pending limit was reached" + }, 422: { content: { "application/json": { schema: ErrorResponseSchema } }, description: "Payload failed validation" @@ -247,6 +255,7 @@ extensionsV2.openapi(createSubmissionRoute, async (c) => { const created = await db.create({ extensionId: ownership.data.extensionId, developerId: ownership.data.developerId, + ownershipEpoch: ownership.data.ownershipEpoch, submittedBy: auth.userId, payload }); @@ -255,10 +264,10 @@ extensionsV2.openapi(createSubmissionRoute, async (c) => { { error: { message: created.error?.message ?? "Unable to create submission", - code: "DATABASE_ERROR" + code: created.error?.code ?? "DATABASE_ERROR" } }, - 500 + created.error?.code === "CONFLICT" ? 409 : 500 ); } @@ -275,11 +284,15 @@ const mineRoute = createRoute({ summary: "List the caller's own submissions, in any status", security: [{ Bearer: [] }], middleware: [requireAuth()] as const, + request: { query: SubmissionPageQuerySchema }, responses: { 200: { content: { "application/json": { - schema: z.object({ result: z.array(SubmissionSchema) }) + schema: z.object({ + result: z.array(SubmissionSchema), + pagination: PaginationSchema + }) } }, description: "The caller's submissions" @@ -288,6 +301,10 @@ const mineRoute = createRoute({ content: { "application/json": { schema: ErrorResponseSchema } }, description: "Missing or invalid bearer token" }, + 422: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Pagination query failed validation" + }, 500: { content: { "application/json": { schema: ErrorResponseSchema } }, description: "Database error" @@ -297,10 +314,11 @@ const mineRoute = createRoute({ extensionsV2.openapi(mineRoute, async (c) => { const auth = getAuth(c); + const { limit, cursor } = c.req.valid("query"); const platform = getPlatform(c); const db = new SubmissionsDatabase(platform.getDatabase("DB_EXTENSIONS")); - const { data, error } = await db.listBySubmitter(auth.userId); + const { data, error } = await db.listBySubmitter(auth.userId, limit, cursor); if (error || !data) { return c.json( { @@ -309,11 +327,20 @@ extensionsV2.openapi(mineRoute, async (c) => { code: "DATABASE_ERROR" } }, - 500 + error?.code === "INVALID_CURSOR" ? 422 : 500 ); } - return c.json({ result: data }, 200); + return c.json( + { + result: data.items, + pagination: { + next_cursor: data.nextCursor, + has_more: data.hasMore + } + }, + 200 + ); }); const queueRoute = createRoute({ @@ -328,7 +355,10 @@ const queueRoute = createRoute({ 200: { content: { "application/json": { - schema: z.object({ result: z.array(SubmissionSchema) }) + schema: z.object({ + result: z.array(SubmissionSchema), + pagination: PaginationSchema + }) } }, description: @@ -357,9 +387,13 @@ extensionsV2.openapi(queueRoute, async (c) => { const platform = getPlatform(c); const db = new SubmissionsDatabase(platform.getDatabase("DB_EXTENSIONS")); - const { status } = c.req.valid("query"); + const { status, limit, cursor } = c.req.valid("query"); - const { data, error } = await db.listQueue(status ?? "pending"); + const { data, error } = await db.listQueue( + status ?? "pending", + limit, + cursor + ); if (error || !data) { return c.json( { @@ -368,11 +402,20 @@ extensionsV2.openapi(queueRoute, async (c) => { code: "DATABASE_ERROR" } }, - 500 + error?.code === "INVALID_CURSOR" ? 422 : 500 ); } - return c.json({ result: data }, 200); + return c.json( + { + result: data.items, + pagination: { + next_cursor: data.nextCursor, + has_more: data.hasMore + } + }, + 200 + ); }); const approveRoute = createRoute({ @@ -1090,12 +1133,16 @@ extensionsV2.openapi(revokeTransferRoute, async (c) => { const acceptTransferRoute = createRoute({ method: "post", - path: "/developers/transfers/{token}/accept", + path: "/developers/transfers/accept", tags: ["Developers"], summary: "Accept a developer profile transfer using its single-use token", security: [{ Bearer: [] }], middleware: [requireAuth()] as const, - request: { params: TokenParamSchema }, + request: { + body: { + content: { "application/json": { schema: TransferAcceptanceSchema } } + } + }, responses: { 200: { content: { @@ -1119,7 +1166,7 @@ const acceptTransferRoute = createRoute({ }, 422: { content: { "application/json": { schema: ErrorResponseSchema } }, - description: "token param failed validation" + description: "token body failed validation" }, 500: { content: { "application/json": { schema: ErrorResponseSchema } }, @@ -1130,7 +1177,7 @@ const acceptTransferRoute = createRoute({ extensionsV2.openapi(acceptTransferRoute, async (c) => { const auth = getAuth(c); - const { token } = c.req.valid("param"); + const { token } = c.req.valid("json"); const platform = getPlatform(c); const db = new DevelopersDatabase(platform.getDatabase("DB_EXTENSIONS")); @@ -1318,7 +1365,12 @@ const approveDeveloperRoute = createRoute({ summary: "Mark a developer profile as reviewed/approved", security: [{ Bearer: [] }], middleware: [requireAuth(), requireModerator()] as const, - request: { params: IdParamSchema }, + request: { + params: IdParamSchema, + body: { + content: { "application/json": { schema: DeveloperApprovalSchema } } + } + }, responses: { 200: { content: { @@ -1342,6 +1394,10 @@ const approveDeveloperRoute = createRoute({ content: { "application/json": { schema: ErrorResponseSchema } }, description: "No developer with that id" }, + 409: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Profile changed after the reviewed revision" + }, 422: { content: { "application/json": { schema: ErrorResponseSchema } }, description: "id param failed validation" @@ -1354,13 +1410,15 @@ const approveDeveloperRoute = createRoute({ }); extensionsV2.openapi(approveDeveloperRoute, async (c) => { + const auth = getAuth(c); const { id } = c.req.valid("param"); + const { expected_revision } = c.req.valid("json"); const platform = getPlatform(c); const db = new DevelopersDatabase(platform.getDatabase("DB_EXTENSIONS")); - const { data, error } = await db.approve(id); + const { data, error } = await db.approve(id, expected_revision, auth.userId); if (error || !data) { - const status = error?.code === "NOT_FOUND" ? 404 : 500; + const status = statusFromErrorCode(error?.code); return c.json( { error: { diff --git a/src/services/extensions/v2/interfaces.ts b/src/services/extensions/v2/interfaces.ts index 2210f23..96bc34d 100644 --- a/src/services/extensions/v2/interfaces.ts +++ b/src/services/extensions/v2/interfaces.ts @@ -27,6 +27,7 @@ const lowercaseId = (label: string) => const httpUrl = () => z .string() + .max(2048) .url() .refine((value) => /^https?:\/\//i.test(value), { message: "must use http or https" @@ -49,10 +50,10 @@ export const DeveloperSchema = z .object({ id: developerId(), type: z.enum(["user", "organization"]), - name: z.string().min(1), + name: z.string().min(1).max(120), URL: httpUrl().optional(), avatar_url: httpUrl().optional(), - contact_email: z.string().email().optional() + contact_email: z.string().email().max(254).optional() }) .openapi("Developer"); @@ -73,12 +74,13 @@ export const SubmissionDeveloperSchema = DeveloperSchema.pick({ export const ReleaseSchema = z .object({ - tag: z.string().min(1), - date: z.string().min(1), + tag: z.string().min(1).max(100), + date: z.string().min(1).max(64), download_url: httpUrl(), changelog_url: httpUrl().optional(), - min_fossbilling_version: z.string().min(1) + min_fossbilling_version: z.string().min(1).max(100) }) + .strict() .openapi("Release"); export type Release = z.infer; @@ -86,17 +88,19 @@ export type Release = z.infer; export const RepositorySchema = z .object({ type: z.enum(["github", "gitlab", "custom"]), - repo: z.string().min(1) + repo: z.string().min(1).max(500) }) + .strict() .openapi("Repository"); export type Repository = z.infer; export const LicenseSchema = z .object({ - name: z.string().min(1), + name: z.string().min(1).max(100), URL: httpUrl().optional() }) + .strict() .openapi("License"); export type License = z.infer; @@ -105,17 +109,18 @@ export const ExtensionPayloadSchema = z .object({ id: lowercaseId("extension"), type: z.enum(EXTENSION_TYPES), - name: z.string().min(1), - description: z.string().min(1), - releases: z.array(ReleaseSchema).min(1), + name: z.string().min(1).max(120), + description: z.string().min(1).max(4000), + releases: z.array(ReleaseSchema).min(1).max(100), website: httpUrl(), license: LicenseSchema, icon_url: httpUrl().optional(), - readme: z.string().min(1), + readme: z.string().min(1).max(100_000), source: RepositorySchema, - version: z.string().min(1), + version: z.string().min(1).max(100), download_url: httpUrl() }) + .strict() .openapi("ExtensionPayload"); export const SubmissionPayloadSchema = z @@ -123,12 +128,23 @@ export const SubmissionPayloadSchema = z developer: SubmissionDeveloperSchema, extension: ExtensionPayloadSchema }) + .strict() + .superRefine((payload, ctx) => { + const size = new TextEncoder().encode(JSON.stringify(payload)).byteLength; + if (size > 256 * 1024) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: "Submission payload must not exceed 256 KiB" + }); + } + }) .openapi("SubmissionPayload"); export type SubmissionPayload = z.infer; export const DeveloperProfileSchema = DeveloperSchema.extend({ - approved: z.boolean() + approved: z.boolean(), + content_revision: z.number().int().positive() }).openapi("DeveloperProfile"); export type DeveloperProfile = z.infer; @@ -137,7 +153,8 @@ export type DeveloperProfile = z.infer; // DeveloperProfile except contact_email, which exists for moderator/owner // communication and was never meant to be broadcast to anonymous callers. export const PublicDeveloperSchema = DeveloperProfileSchema.omit({ - contact_email: true + contact_email: true, + content_revision: true }).openapi("PublicDeveloper"); export type PublicDeveloper = z.infer; @@ -189,13 +206,13 @@ export type DeveloperHistoryEntry = z.infer; export const ReviewNoteOptionalSchema = z .object({ - review_note: z.string().optional() + review_note: z.string().max(2000).optional() }) .openapi("ReviewNoteOptional"); export const ReviewNoteRequiredSchema = z .object({ - review_note: z.string().min(1) + review_note: z.string().min(1).max(2000) }) .openapi("ReviewNoteRequired"); @@ -241,14 +258,15 @@ export const IdParamSchema = z.object({ }) }); -export const TokenParamSchema = z.object({ - token: z - .string() - .min(1) - .openapi({ - param: { name: "token", in: "path" } - }) -}); +export const TransferAcceptanceSchema = z + .object({ token: z.string().min(64).max(128) }) + .strict() + .openapi("TransferAcceptance"); + +export const DeveloperApprovalSchema = z + .object({ expected_revision: z.number().int().positive() }) + .strict() + .openapi("DeveloperApproval"); export const DeveloperTransferSchema = z .object({ @@ -291,5 +309,33 @@ export const ClaimNoteSchema = z export const QueueQuerySchema = z.object({ status: SubmissionStatusSchema.optional().openapi({ param: { name: "status", in: "query" } - }) + }), + limit: z.coerce + .number() + .int() + .min(1) + .max(100) + .default(50) + .openapi({ + param: { name: "limit", in: "query" } + }), + cursor: z + .string() + .max(1000) + .optional() + .openapi({ + param: { name: "cursor", in: "query" } + }) +}); + +export const SubmissionPageQuerySchema = QueueQuerySchema.pick({ + limit: true, + cursor: true }); + +export const PaginationSchema = z + .object({ + next_cursor: z.string().nullable(), + has_more: z.boolean() + }) + .openapi("Pagination"); diff --git a/src/services/extensions/v2/submissions-database.ts b/src/services/extensions/v2/submissions-database.ts index 5779d22..4088f49 100644 --- a/src/services/extensions/v2/submissions-database.ts +++ b/src/services/extensions/v2/submissions-database.ts @@ -5,17 +5,55 @@ import { Submission, SubmissionPayload, SubmissionStatus } from "./interfaces"; interface OwnershipResolution { extensionId: string | null; developerId: string; + ownershipEpoch: number; } interface CreateInput { extensionId: string | null; developerId: string; + ownershipEpoch: number; submittedBy: string; payload: SubmissionPayload; } -function parseSubmissionRow(row: Record): Submission { - return { +export interface SubmissionPage { + items: Submission[]; + nextCursor: string | null; + hasMore: boolean; +} + +interface StoredSubmission extends Submission { + ownershipEpoch: number; +} + +const MAX_PENDING_SUBMISSIONS_PER_USER = 10; + +function isPendingTargetConflict(error: unknown): boolean { + return ( + error instanceof Error && + /UNIQUE constraint failed.*extension_submissions/i.test(error.message) + ); +} + +function encodeCursor(createdAt: string, id: string): string { + return btoa(JSON.stringify([createdAt, id])); +} + +function decodeCursor(cursor: string): [string, string] | null { + try { + const value = JSON.parse(atob(cursor)) as unknown; + return Array.isArray(value) && + value.length === 2 && + value.every((part) => typeof part === "string") + ? (value as [string, string]) + : null; + } catch { + return null; + } +} + +function parseSubmissionRow(row: Record): StoredSubmission { + const submission = { id: row.id as string, extension_id: (row.extension_id as string | null) ?? null, developer_id: row.developer_id as string, @@ -26,7 +64,12 @@ function parseSubmissionRow(row: Record): Submission { review_note: (row.review_note as string | null) ?? null, created_at: row.created_at as string, reviewed_at: (row.reviewed_at as string | null) ?? null - }; + } as StoredSubmission; + Object.defineProperty(submission, "ownershipEpoch", { + value: Number(row.ownership_epoch ?? 1), + enumerable: false + }); + return submission; } export class SubmissionsDatabase { @@ -56,9 +99,11 @@ export class SubmissionsDatabase { if (existingExtension) { const existingDeveloper = await this.db - .prepare("SELECT owner_user_id FROM developers WHERE id = ?") + .prepare( + "SELECT owner_user_id, ownership_epoch FROM developers WHERE id = ?" + ) .bind(existingExtension.author_id) - .first<{ owner_user_id: string | null }>(); + .first<{ owner_user_id: string | null; ownership_epoch: number }>(); if ( !existingDeveloper || @@ -77,9 +122,11 @@ export class SubmissionsDatabase { } const payloadDeveloper = await this.db - .prepare("SELECT owner_user_id FROM developers WHERE id = ?") + .prepare( + "SELECT owner_user_id, ownership_epoch FROM developers WHERE id = ?" + ) .bind(payload.developer.id) - .first<{ owner_user_id: string | null }>(); + .first<{ owner_user_id: string | null; ownership_epoch: number }>(); if (!payloadDeveloper || payloadDeveloper.owner_user_id !== callerId) { return { @@ -93,7 +140,11 @@ export class SubmissionsDatabase { } return { - data: { extensionId, developerId: payload.developer.id }, + data: { + extensionId, + developerId: payload.developer.id, + ownershipEpoch: Number(payloadDeveloper.ownership_epoch ?? 1) + }, error: null }; } catch (error) { @@ -108,18 +159,54 @@ export class SubmissionsDatabase { try { result = await this.db .prepare( - `INSERT INTO extension_submissions (id, extension_id, developer_id, submitted_by, status, payload) - VALUES (?, ?, ?, ?, 'pending', ?)` + `INSERT INTO extension_submissions + (id, extension_id, developer_id, submitted_by, status, payload, ownership_epoch, target_key) + SELECT ?, ?, ?, ?, 'pending', ?, d.ownership_epoch, LOWER(?) + FROM developers d + WHERE d.id = ? AND d.owner_user_id = ? AND d.ownership_epoch = ? + AND ( + SELECT COUNT(*) FROM extension_submissions + WHERE submitted_by = ? AND status = 'pending' + ) < ? + AND ( + (? IS NULL AND NOT EXISTS ( + SELECT 1 FROM extensions WHERE LOWER(id) = LOWER(?) + )) + OR + (? IS NOT NULL AND EXISTS ( + SELECT 1 FROM extensions + WHERE id = ? AND author_id = d.id + )) + )` ) .bind( id, input.extensionId, input.developerId, input.submittedBy, - JSON.stringify(input.payload) + JSON.stringify(input.payload), + input.payload.extension.id, + input.developerId, + input.submittedBy, + input.ownershipEpoch, + input.submittedBy, + MAX_PENDING_SUBMISSIONS_PER_USER, + input.extensionId, + input.payload.extension.id, + input.extensionId, + input.extensionId ) .run(); } catch (error) { + if (isPendingTargetConflict(error)) { + return { + data: null, + error: { + message: "A submission for this extension is already pending", + code: "CONFLICT" + } + }; + } return databaseError("create", error); } @@ -130,18 +217,51 @@ export class SubmissionsDatabase { ); } + if (!result.meta?.changes) { + return { + data: null, + error: { + message: + "Submission could not be created because ownership changed, the target changed, or the pending-submission limit was reached", + code: "CONFLICT" + } + }; + } + return { data: { id }, error: null }; } - async listBySubmitter(userId: string): Promise> { + async listBySubmitter( + userId: string, + limit: number, + cursor?: string + ): Promise> { + const decoded = cursor ? decodeCursor(cursor) : null; + if (cursor && !decoded) { + return { + data: null, + error: { message: "Invalid pagination cursor", code: "INVALID_CURSOR" } + }; + } let result; try { - result = await this.db - .prepare( - "SELECT * FROM extension_submissions WHERE submitted_by = ? ORDER BY created_at DESC" - ) - .bind(userId) - .all>(); + const statement = decoded + ? this.db + .prepare( + `SELECT * FROM extension_submissions + WHERE submitted_by = ? + AND (created_at < ? OR (created_at = ? AND id < ?)) + ORDER BY created_at DESC, id DESC LIMIT ?` + ) + .bind(userId, decoded[0], decoded[0], decoded[1], limit + 1) + : this.db + .prepare( + `SELECT * FROM extension_submissions + WHERE submitted_by = ? + ORDER BY created_at DESC, id DESC LIMIT ?` + ) + .bind(userId, limit + 1); + result = await statement.all>(); } catch (error) { return databaseError("listBySubmitter", error); } @@ -153,23 +273,52 @@ export class SubmissionsDatabase { ); } + const rows = result.results ?? []; + const hasMore = rows.length > limit; + const items = rows.slice(0, limit).map(parseSubmissionRow); + const last = items.at(-1); return { - data: (result.results ?? []).map(parseSubmissionRow), + data: { + items, + hasMore, + nextCursor: + hasMore && last ? encodeCursor(last.created_at, last.id) : null + }, error: null }; } async listQueue( - status: SubmissionStatus = "pending" - ): Promise> { + status: SubmissionStatus, + limit: number, + cursor?: string + ): Promise> { + const decoded = cursor ? decodeCursor(cursor) : null; + if (cursor && !decoded) { + return { + data: null, + error: { message: "Invalid pagination cursor", code: "INVALID_CURSOR" } + }; + } let result; try { - result = await this.db - .prepare( - "SELECT * FROM extension_submissions WHERE status = ? ORDER BY created_at ASC" - ) - .bind(status) - .all>(); + const statement = decoded + ? this.db + .prepare( + `SELECT * FROM extension_submissions + WHERE status = ? + AND (created_at > ? OR (created_at = ? AND id > ?)) + ORDER BY created_at ASC, id ASC LIMIT ?` + ) + .bind(status, decoded[0], decoded[0], decoded[1], limit + 1) + : this.db + .prepare( + `SELECT * FROM extension_submissions + WHERE status = ? + ORDER BY created_at ASC, id ASC LIMIT ?` + ) + .bind(status, limit + 1); + result = await statement.all>(); } catch (error) { return databaseError("listQueue", error); } @@ -181,13 +330,22 @@ export class SubmissionsDatabase { ); } + const rows = result.results ?? []; + const hasMore = rows.length > limit; + const items = rows.slice(0, limit).map(parseSubmissionRow); + const last = items.at(-1); return { - data: (result.results ?? []).map(parseSubmissionRow), + data: { + items, + hasMore, + nextCursor: + hasMore && last ? encodeCursor(last.created_at, last.id) : null + }, error: null }; } - async getById(id: string): Promise> { + async getById(id: string): Promise> { let row; try { row = await this.db @@ -211,26 +369,6 @@ export class SubmissionsDatabase { return { data: parseSubmissionRow(row), error: null }; } - // Best-effort compensation: if the write-through after a successful claim - // fails, put the submission back to 'pending' rather than leaving it - // permanently 'approved' with no matching developer/extension write. If - // this itself fails, the submission is stuck 'approved' and needs manual - // fixup — surfaced via the DATABASE_ERROR the caller already returns. - private async revertToPending(id: string): Promise { - try { - await this.db - .prepare( - `UPDATE extension_submissions - SET status = 'pending', reviewer_id = NULL, review_note = NULL, reviewed_at = NULL - WHERE id = ?` - ) - .bind(id) - .run(); - } catch { - // best-effort only - } - } - // Notes what happened to an id-scoped write that didn't affect any rows: // either it never existed, or someone else already moved it off 'pending'. private async explainNoOpTransition( @@ -311,64 +449,7 @@ export class SubmissionsDatabase { }; } - const recheck = await this.resolveOwnership( - submission.payload, - submission.submitted_by - ); - if (recheck.error || !recheck.data) { - return { - data: null, - error: { - message: recheck.error?.message ?? "Unable to re-validate ownership", - code: "CONFLICT" - } - }; - } - - const wasEdit = submission.extension_id !== null; - const isStillEdit = recheck.data.extensionId !== null; - if (wasEdit !== isStillEdit) { - return { - data: null, - error: { - message: wasEdit - ? "Extension no longer exists" - : "Extension id is now taken", - code: "CONFLICT" - } - }; - } - - // Claim the transition atomically before writing anything through. If - // this affects no rows, a concurrent approve/reject already won the - // race — report CONFLICT without having touched developers/extensions. - let claim; - try { - claim = await this.db - .prepare( - `UPDATE extension_submissions - SET status = 'approved', reviewer_id = ?, review_note = ?, reviewed_at = CURRENT_TIMESTAMP - WHERE id = ? AND status = 'pending'` - ) - .bind(reviewerId, reviewNote ?? null, id) - .run(); - } catch (error) { - return databaseError("approve", error); - } - - if (!claim.success) { - return databaseError( - "approve", - new Error(claim.error || "Database query failed") - ); - } - - if (!claim.meta?.changes) { - return this.explainNoOpTransition(id); - } - if (!this.db.batch) { - await this.revertToPending(id); return databaseError( "approve", new Error("Database adapter does not support batch operations") @@ -376,32 +457,63 @@ export class SubmissionsDatabase { } const { developer, extension } = submission.payload; - // Edits must update the existing row even if it was stored under a - // different case (e.g. legacy "Example" edited via "example") — using - // the payload's id here would insert a second row instead. - const extensionId = recheck.data.extensionId ?? extension.id; + const extensionId = submission.extension_id ?? extension.id; let results; try { + // D1 executes a batch as one transaction. The first statement binds + // the moderation transition to the submitter's captured ownership + // epoch; transfer and approval therefore cannot interleave. + const claimStmt = this.db + .prepare( + `UPDATE extension_submissions + SET status = 'approved', reviewer_id = ?, review_note = ?, reviewed_at = CURRENT_TIMESTAMP + WHERE id = ? AND status = 'pending' + AND EXISTS ( + SELECT 1 FROM developers d + WHERE d.id = extension_submissions.developer_id + AND d.owner_user_id = extension_submissions.submitted_by + AND d.ownership_epoch = extension_submissions.ownership_epoch + ) + AND ( + (extension_id IS NULL AND NOT EXISTS ( + SELECT 1 FROM extensions e + WHERE LOWER(e.id) = LOWER(?) + )) + OR + (extension_id IS NOT NULL AND EXISTS ( + SELECT 1 FROM extensions e + WHERE e.id = extension_submissions.extension_id + AND e.author_id = extension_submissions.developer_id + )) + )` + ) + .bind(reviewerId, reviewNote ?? null, id, extension.id); + const developerStmt = this.db .prepare( - `INSERT INTO developers (id, type, name, url, owner_user_id) - VALUES (?, ?, ?, ?, ?) - ON CONFLICT(id) DO UPDATE SET - type = excluded.type, name = excluded.name, url = excluded.url` + `UPDATE developers + SET type = ?, name = ?, url = ?, + content_revision = content_revision + 1, + approved_at = NULL, approved_revision = NULL, approved_by = NULL, + updated_at = CURRENT_TIMESTAMP + WHERE changes() = 1 AND id = ? AND owner_user_id = ? + AND ownership_epoch = ?` ) .bind( - developer.id, developer.type, developer.name, developer.URL ?? null, - submission.submitted_by + developer.id, + submission.submitted_by, + submission.ownershipEpoch ); const extensionStmt = this.db .prepare( `INSERT INTO extensions (id, type, author_id, name, description, releases, website, license, icon_url, readme, source, version, download_url) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + SELECT ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? + WHERE changes() = 1 ON CONFLICT(id) DO UPDATE SET type = excluded.type, author_id = excluded.author_id, name = excluded.name, description = excluded.description, releases = excluded.releases, @@ -426,24 +538,38 @@ export class SubmissionsDatabase { extension.download_url ); - results = (await this.db.batch([developerStmt, extensionStmt])) as Array<{ + results = (await this.db.batch([ + claimStmt, + developerStmt, + extensionStmt + ])) as Array<{ success: boolean; error?: string; + meta?: { changes?: number }; }>; } catch (error) { - await this.revertToPending(id); return databaseError("approve", error); } const failed = results.find((r) => !r.success); if (failed) { - await this.revertToPending(id); return databaseError( "approve", new Error(failed.error || "Database write failed") ); } + if (!results[0]?.meta?.changes) { + return { + data: null, + error: { + message: + "Submission is not pending, ownership changed, or the extension target changed", + code: "CONFLICT" + } + }; + } + return { data: { id, status: "approved" }, error: null }; } } diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index 005c800..e842c6b 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -297,6 +297,143 @@ describe("Extensions API v2", () => { expect(res.status).toBe(403); expect(tables.extension_submissions.size).toBe(0); }); + + it("bounds payload size and the number of releases", async () => { + seedDeveloper("new-developer", "user-1"); + const payload = samplePayload(); + const oversized = await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + { + ...payload, + extension: { ...payload.extension, readme: "x".repeat(100_001) } + } + ); + expect(oversized.status).toBe(422); + + const unknownExtensionField = await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + { + ...payload, + extension: { + ...payload.extension, + padding: "x" + } + } + ); + expect(unknownExtensionField.status).toBe(422); + const unknownExtensionBody = (await unknownExtensionField.json()) as { + error: { details: Array<{ code: string; path: PropertyKey[] }> }; + }; + expect(unknownExtensionBody.error.details).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "unrecognized_keys", + path: ["extension"] + }) + ]) + ); + + const unknownReleaseField = await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + { + ...payload, + extension: { + ...payload.extension, + releases: [ + { + ...payload.extension.releases[0], + padding: "x" + } + ] + } + } + ); + expect(unknownReleaseField.status).toBe(422); + const unknownReleaseBody = (await unknownReleaseField.json()) as { + error: { details: Array<{ code: string; path: PropertyKey[] }> }; + }; + expect(unknownReleaseBody.error.details).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "unrecognized_keys", + path: ["extension", "releases", 0] + }) + ]) + ); + + const tooManyReleases = await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + { + ...payload, + extension: { + ...payload.extension, + releases: Array.from( + { length: 101 }, + () => payload.extension.releases[0] + ) + } + } + ); + expect(tooManyReleases.status).toBe(422); + }); + + it("preserves compatibility with stored slug ids over 100 characters", async () => { + const developerId = "d".repeat(120); + const extensionId = "e".repeat(120); + seedDeveloper(developerId, "user-1"); + + const res = await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + samplePayload({ developerId, extensionId }) + ); + + expect(res.status).toBe(201); + }); + + it("rejects duplicate pending targets and caps each user's backlog", async () => { + seedDeveloper("new-developer", "user-1"); + const headers = await authHeaders("user-1"); + expect( + (await post("/extensions/v2/submissions", headers, samplePayload())) + .status + ).toBe(201); + expect( + (await post("/extensions/v2/submissions", headers, samplePayload())) + .status + ).toBe(409); + + seedDeveloper("other-developer", "user-2"); + expect( + ( + await post( + "/extensions/v2/submissions", + await authHeaders("user-2"), + samplePayload({ developerId: "other-developer" }) + ) + ).status + ).toBe(409); + + for (let index = 1; index < 10; index++) { + const result = await post( + "/extensions/v2/submissions", + headers, + samplePayload({ extensionId: `new-ext-${index}` }) + ); + expect(result.status).toBe(201); + } + const overLimit = await post( + "/extensions/v2/submissions", + headers, + samplePayload({ extensionId: "new-ext-over-limit" }) + ); + expect(overLimit.status).toBe(409); + expect(tables.extension_submissions.size).toBe(10); + }); }); describe("GET /submissions/mine", () => { @@ -330,6 +467,47 @@ describe("Extensions API v2", () => { const res = await get("/extensions/v2/submissions/mine", {}); expect(res.status).toBe(401); }); + + it("paginates deterministically with an opaque cursor", async () => { + seedDeveloper("new-developer", "user-1"); + const headers = await authHeaders("user-1"); + for (const extensionId of ["page-a", "page-b", "page-c"]) { + expect( + ( + await post( + "/extensions/v2/submissions", + headers, + samplePayload({ extensionId }) + ) + ).status + ).toBe(201); + } + + const first = await get( + "/extensions/v2/submissions/mine?limit=2", + headers + ); + const firstBody = (await first.json()) as { + result: unknown[]; + pagination: { has_more: boolean; next_cursor: string }; + }; + expect(firstBody.result).toHaveLength(2); + expect(firstBody.pagination.has_more).toBe(true); + + const second = await get( + `/extensions/v2/submissions/mine?limit=2&cursor=${encodeURIComponent(firstBody.pagination.next_cursor)}`, + headers + ); + const secondBody = (await second.json()) as { + result: unknown[]; + pagination: { has_more: boolean; next_cursor: null }; + }; + expect(secondBody.result).toHaveLength(1); + expect(secondBody.pagination).toEqual({ + has_more: false, + next_cursor: null + }); + }); }); describe("GET /submissions/queue", () => { @@ -362,6 +540,29 @@ describe("Extensions API v2", () => { }); describe("approve / reject", () => { + it("does not approve a former owner's payload when ownership changes at approval", async () => { + tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + seedDeveloper("new-developer", "user-1"); + const created = await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + samplePayload() + ); + const { result } = (await created.json()) as { result: { id: string } }; + + tables.raceOwnerChangeOnSubmissionApprovalTo = "user-2"; + const approved = await post( + `/extensions/v2/submissions/${result.id}/approve`, + await authHeaders("mod-1"), + {} + ); + expect(approved.status).toBe(409); + expect(tables.extension_submissions.get(result.id)?.status).toBe( + "pending" + ); + expect(tables.extensions.size).toBe(0); + }); + it("reverts to pending if the write-through fails after a successful claim", async () => { tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); seedDeveloper("new-developer", "user-1"); @@ -678,7 +879,8 @@ describe("Extensions API v2", () => { tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); const approved = await post( "/extensions/v2/developers/dev-developer/approve", - await authHeaders("mod-1") + await authHeaders("mod-1"), + { expected_revision: 1 } ); expect(approved.status).toBe(200); const approvedBody = (await approved.json()) as { @@ -697,6 +899,34 @@ describe("Extensions API v2", () => { expect(data.result.approved).toBe(false); }); + it("does not update a profile after ownership changes mid-request", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + tables.raceOwnerChangeOnProfileUpdateTo = "user-2"; + + const raced = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper({ name: "Former owner write" }) + ); + + expect(raced.status).toBe(409); + expect(tables.developers.get("dev-developer")?.name).toBe( + "Dev Developer" + ); + expect(tables.developers.get("dev-developer")?.owner_user_id).toBe( + "user-2" + ); + expect( + [...tables.developer_history.values()].filter( + (row) => row.developer_id === "dev-developer" + ) + ).toHaveLength(1); + }); + it("round-trips avatar_url and contact_email", async () => { const headers = await authHeaders("user-1"); const res = await put("/extensions/v2/developers/me", headers, { @@ -917,6 +1147,33 @@ describe("Extensions API v2", () => { }); describe("developer moderation", () => { + it("binds approval to the exact profile revision reviewed", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper({ name: "Revision two" }) + ); + tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + + const stale = await post( + "/extensions/v2/developers/dev-developer/approve", + await authHeaders("mod-1"), + { expected_revision: 1 } + ); + expect(stale.status).toBe(409); + + const current = await post( + "/extensions/v2/developers/dev-developer/approve", + await authHeaders("mod-1"), + { expected_revision: 2 } + ); + expect(current.status).toBe(200); + }); it("approves a developer and removes it from the unapproved list", async () => { await put( "/extensions/v2/developers/me", @@ -927,7 +1184,8 @@ describe("Extensions API v2", () => { const approve = await post( "/extensions/v2/developers/dev-developer/approve", - await authHeaders("mod-1") + await authHeaders("mod-1"), + { expected_revision: 1 } ); expect(approve.status).toBe(200); const approveBody = (await approve.json()) as { @@ -956,7 +1214,8 @@ describe("Extensions API v2", () => { const res = await post( "/extensions/v2/developers/no-such-developer/approve", - await authHeaders("mod-1") + await authHeaders("mod-1"), + { expected_revision: 1 } ); expect(res.status).toBe(404); }); @@ -983,7 +1242,8 @@ describe("Extensions API v2", () => { tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); await post( "/extensions/v2/developers/dev-developer/approve", - await authHeaders("mod-1") + await authHeaders("mod-1"), + { expected_revision: 1 } ); const res = await get( @@ -1023,7 +1283,8 @@ describe("Extensions API v2", () => { const res = await post( "/extensions/v2/developers/dev-developer/approve", - await authHeaders("user-1") + await authHeaders("user-1"), + { expected_revision: 1 } ); expect(res.status).toBe(403); }); @@ -1136,6 +1397,14 @@ describe("Extensions API v2", () => { }); describe("developer transfers", () => { + it("does not accept transfer capabilities in URL paths", async () => { + const res = await post( + "/extensions/v2/developers/transfers/secret-token/accept", + await authHeaders("user-2") + ); + expect(res.status).toBe(404); + }); + it("initiating a second transfer revokes the first token", async () => { await put( "/extensions/v2/developers/me", @@ -1158,8 +1427,9 @@ describe("Extensions API v2", () => { expect(second.status).toBe(200); const acceptFirst = await post( - `/extensions/v2/developers/transfers/${firstToken}/accept`, - await authHeaders("user-2") + "/extensions/v2/developers/transfers/accept", + await authHeaders("user-2"), + { token: firstToken } ); expect(acceptFirst.status).toBe(404); }); @@ -1173,7 +1443,8 @@ describe("Extensions API v2", () => { tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); await post( "/extensions/v2/developers/dev-developer/approve", - await authHeaders("mod-1") + await authHeaders("mod-1"), + { expected_revision: 1 } ); const initiate = await post( @@ -1184,8 +1455,9 @@ describe("Extensions API v2", () => { .result.token; const accept = await post( - `/extensions/v2/developers/transfers/${token}/accept`, - await authHeaders("user-2") + "/extensions/v2/developers/transfers/accept", + await authHeaders("user-2"), + { token } ); expect(accept.status).toBe(200); const accepted = (await accept.json()) as { @@ -1198,8 +1470,9 @@ describe("Extensions API v2", () => { ); const acceptAgain = await post( - `/extensions/v2/developers/transfers/${token}/accept`, - await authHeaders("user-3") + "/extensions/v2/developers/transfers/accept", + await authHeaders("user-3"), + { token } ); expect(acceptAgain.status).toBe(404); expect(tables.developers.get("dev-developer")?.owner_user_id).toBe( @@ -1222,8 +1495,9 @@ describe("Extensions API v2", () => { .result.token; const accept1 = await post( - `/extensions/v2/developers/transfers/${token1}/accept`, - await authHeaders("user-2") + "/extensions/v2/developers/transfers/accept", + await authHeaders("user-2"), + { token: token1 } ); expect(accept1.status).toBe(200); expect(tables.developers.get("dev-developer")?.owner_user_id).toBe( @@ -1238,8 +1512,9 @@ describe("Extensions API v2", () => { const token2 = ((await initiate2.json()) as { result: { token: string } }) .result.token; const accept2 = await post( - `/extensions/v2/developers/transfers/${token2}/accept`, - await authHeaders("user-3") + "/extensions/v2/developers/transfers/accept", + await authHeaders("user-3"), + { token: token2 } ); expect(accept2.status).toBe(200); expect(tables.developers.get("dev-developer")?.owner_user_id).toBe( @@ -1249,8 +1524,9 @@ describe("Extensions API v2", () => { // Replaying the *first* (already-used) token, by the same user who // originally accepted it, must not silently reassign ownership back. const replay = await post( - `/extensions/v2/developers/transfers/${token1}/accept`, - await authHeaders("user-2") + "/extensions/v2/developers/transfers/accept", + await authHeaders("user-2"), + { token: token1 } ); expect(replay.status).toBe(404); expect(tables.developers.get("dev-developer")?.owner_user_id).toBe( @@ -1277,8 +1553,9 @@ describe("Extensions API v2", () => { } const accept = await post( - `/extensions/v2/developers/transfers/${token}/accept`, - await authHeaders("user-2") + "/extensions/v2/developers/transfers/accept", + await authHeaders("user-2"), + { token } ); expect(accept.status).toBe(404); }); @@ -1304,8 +1581,9 @@ describe("Extensions API v2", () => { expect(revoke.status).toBe(200); const accept = await post( - `/extensions/v2/developers/transfers/${token}/accept`, - await authHeaders("user-2") + "/extensions/v2/developers/transfers/accept", + await authHeaders("user-2"), + { token } ); expect(accept.status).toBe(404); }); @@ -1330,8 +1608,9 @@ describe("Extensions API v2", () => { .result.token; const accept = await post( - `/extensions/v2/developers/transfers/${token}/accept`, - await authHeaders("user-2") + "/extensions/v2/developers/transfers/accept", + await authHeaders("user-2"), + { token } ); expect(accept.status).toBe(409); expect(tables.developers.get("dev-developer")?.owner_user_id).toBe( @@ -1354,8 +1633,9 @@ describe("Extensions API v2", () => { .result.token; const accept = await post( - `/extensions/v2/developers/transfers/${token}/accept`, - await authHeaders("user-1") + "/extensions/v2/developers/transfers/accept", + await authHeaders("user-1"), + { token } ); expect(accept.status).toBe(409); expect(tables.developers.get("dev-developer")?.owner_user_id).toBe( @@ -1748,7 +2028,7 @@ describe("Extensions API v2", () => { "/developers/{id}/approve", "/developers/{id}/transfer", "/developers/{id}/transfer/revoke", - "/developers/transfers/{token}/accept", + "/developers/transfers/accept", "/developers/{id}/claim", "/developers/claims/mine", "/developers/claims", diff --git a/test/services/extensions/v2/mock-db.ts b/test/services/extensions/v2/mock-db.ts index e3ac91f..555eaef 100644 --- a/test/services/extensions/v2/mock-db.ts +++ b/test/services/extensions/v2/mock-db.ts @@ -15,6 +15,9 @@ export interface MockTables { // fires (once) the first time that lookup runs, mutating the row so the // guard sees a different owner than the caller who's mid-request. raceOwnerChangeTo?: string; + // Fires immediately before a moderator's guarded submission approval. + raceOwnerChangeOnSubmissionApprovalTo?: string; + raceOwnerChangeOnProfileUpdateTo?: string; } export function createTables(): MockTables { @@ -203,7 +206,12 @@ class MockStatement implements D1PreparedStatement { return [{ count }]; } - if (q.startsWith("SELECT owner_user_id FROM developers WHERE id = ?")) { + if ( + q.startsWith( + "SELECT owner_user_id, ownership_epoch FROM developers WHERE id = ?" + ) || + q.startsWith("SELECT owner_user_id FROM developers WHERE id = ?") + ) { const row = this.tables.developers.get(String(p[0])); return row ? [row] : []; } @@ -226,6 +234,15 @@ class MockStatement implements D1PreparedStatement { return row ? [row] : []; } + if ( + q.startsWith( + "SELECT * FROM developers WHERE id = ? AND owner_user_id = ?" + ) + ) { + const row = this.tables.developers.get(String(p[0])); + return row && row.owner_user_id === p[1] ? [row] : []; + } + if (q.startsWith("SELECT * FROM developers WHERE id = ?")) { const row = this.tables.developers.get(String(p[0])); return row ? [row] : []; @@ -250,15 +267,7 @@ class MockStatement implements D1PreparedStatement { "INSERT INTO developers (id, type, name, url, avatar_url, contact_email, owner_user_id, approved_at, created_at, updated_at)" ) ) { - const [ - id, - type, - name, - url, - avatar_url, - contact_email, - owner_user_id - ] = p; + const [id, type, name, url, avatar_url, contact_email, owner_user_id] = p; // Mirrors idx_developers_owner_unique: one profile per non-null owner. const ownerTaken = [...this.tables.developers.values()].some( (r) => owner_user_id !== null && r.owner_user_id === owner_user_id @@ -278,26 +287,42 @@ class MockStatement implements D1PreparedStatement { contact_email, owner_user_id, approved_at: null, + ownership_epoch: 1, + content_revision: 1, + approved_revision: null, + approved_by: null, created_at: now, updated_at: now }); + this.changes = 1; return []; } if ( q.startsWith( - "UPDATE developers SET type = ?, name = ?, url = ?, avatar_url = ?, contact_email = ?, approved_at = NULL" + "UPDATE developers SET type = ?, name = ?, url = ?, avatar_url = ?, contact_email = ?" ) ) { - const [type, name, url, avatar_url, contact_email, id] = p; + const [type, name, url, avatar_url, contact_email, id, owner_user_id] = p; const row = this.tables.developers.get(String(id)); - if (row) { + if (row && this.tables.raceOwnerChangeOnProfileUpdateTo !== undefined) { + row.owner_user_id = this.tables.raceOwnerChangeOnProfileUpdateTo; + row.ownership_epoch = Number(row.ownership_epoch ?? 1) + 1; + this.tables.raceOwnerChangeOnProfileUpdateTo = undefined; + } + if ( + row && + (owner_user_id === undefined || row.owner_user_id === owner_user_id) + ) { row.type = type; row.name = name; row.url = url; row.avatar_url = avatar_url; row.contact_email = contact_email; row.approved_at = null; + row.approved_revision = null; + row.approved_by = null; + row.content_revision = Number(row.content_revision ?? 1) + 1; row.updated_at = new Date().toISOString(); this.changes = 1; } @@ -322,6 +347,31 @@ class MockStatement implements D1PreparedStatement { return []; } + if ( + q.startsWith( + "UPDATE developers SET type = ?, name = ?, url = ?, content_revision = content_revision + 1" + ) + ) { + const [type, name, url, id, owner_user_id, ownership_epoch] = p; + const row = this.tables.developers.get(String(id)); + if ( + row && + row.owner_user_id === owner_user_id && + Number(row.ownership_epoch ?? 1) === ownership_epoch + ) { + row.type = type; + row.name = name; + row.url = url; + row.content_revision = Number(row.content_revision ?? 1) + 1; + row.approved_at = null; + row.approved_revision = null; + row.approved_by = null; + row.updated_at = new Date().toISOString(); + this.changes = 1; + } + return []; + } + if ( q.startsWith( "SELECT developer_id, type, name, url, changed_by, changed_at FROM developer_history WHERE developer_id = ?" @@ -453,9 +503,8 @@ class MockStatement implements D1PreparedStatement { } if ( - q.startsWith( - "UPDATE developers SET owner_user_id = ?, approved_at = NULL, updated_at = CURRENT_TIMESTAMP WHERE changes() = 1 AND id = ( SELECT developer_id FROM developer_transfers" - ) + q.startsWith("UPDATE developers SET owner_user_id = ?") && + q.includes("SELECT developer_id FROM developer_transfers") ) { const [owner_user_id, token_hash, accepted_by] = p; const transfer = [...this.tables.developer_transfers.values()].find( @@ -469,7 +518,11 @@ class MockStatement implements D1PreparedStatement { : undefined; if (row) { row.owner_user_id = owner_user_id; + row.ownership_epoch = Number(row.ownership_epoch ?? 1) + 1; + row.content_revision = Number(row.content_revision ?? 1) + 1; row.approved_at = null; + row.approved_revision = null; + row.approved_by = null; row.updated_at = new Date().toISOString(); this.changes = 1; } else { @@ -480,13 +533,15 @@ class MockStatement implements D1PreparedStatement { if ( q.startsWith( - "UPDATE developers SET approved_at = CURRENT_TIMESTAMP WHERE id = ?" + "UPDATE developers SET approved_at = CURRENT_TIMESTAMP, approved_revision = content_revision, approved_by = ? WHERE id = ? AND content_revision = ?" ) ) { - const [id] = p; + const [reviewer_id, id, expected_revision] = p; const row = this.tables.developers.get(String(id)); - if (row) { + if (row && Number(row.content_revision ?? 1) === expected_revision) { row.approved_at = new Date().toISOString(); + row.approved_revision = Number(row.content_revision ?? 1); + row.approved_by = reviewer_id; this.changes = 1; } return []; @@ -604,15 +659,18 @@ class MockStatement implements D1PreparedStatement { } if ( - q.startsWith( - "UPDATE developers SET owner_user_id = ?, approved_at = NULL, updated_at = CURRENT_TIMESTAMP WHERE id = ? AND owner_user_id IS NULL" - ) + q.startsWith("UPDATE developers SET owner_user_id = ?") && + q.includes("WHERE id = ? AND owner_user_id IS NULL") ) { const [owner_user_id, id] = p; const row = this.tables.developers.get(String(id)); if (row && row.owner_user_id === null) { row.owner_user_id = owner_user_id; + row.ownership_epoch = Number(row.ownership_epoch ?? 1) + 1; + row.content_revision = Number(row.content_revision ?? 1) + 1; row.approved_at = null; + row.approved_revision = null; + row.approved_by = null; row.updated_at = new Date().toISOString(); this.changes = 1; } else { @@ -684,10 +742,39 @@ class MockStatement implements D1PreparedStatement { if ( q.startsWith( - "INSERT INTO extension_submissions (id, extension_id, developer_id, submitted_by, status, payload)" + "INSERT INTO extension_submissions (id, extension_id, developer_id, submitted_by, status, payload, ownership_epoch, target_key)" ) ) { - const [id, extension_id, developer_id, submitted_by, payload] = p; + const [ + id, + extension_id, + developer_id, + submitted_by, + payload, + target_key, + checkDeveloperId, + checkOwner, + checkEpoch + ] = p; + const developer = this.tables.developers.get(String(checkDeveloperId)); + const allPending = [...this.tables.extension_submissions.values()].filter( + (row) => row.status === "pending" + ); + const submitterPending = allPending.filter( + (row) => row.submitted_by === submitted_by + ); + if ( + !developer || + developer.owner_user_id !== checkOwner || + Number(developer.ownership_epoch ?? 1) !== checkEpoch || + submitterPending.length >= 10 || + allPending.some( + (row) => row.target_key === String(target_key).toLowerCase() + ) + ) { + this.changes = 0; + return []; + } this.tables.extension_submissions.set(String(id), { id, extension_id, @@ -695,30 +782,57 @@ class MockStatement implements D1PreparedStatement { submitted_by, status: "pending", payload, + ownership_epoch: Number(developer.ownership_epoch ?? 1), + target_key: String(target_key).toLowerCase(), reviewer_id: null, review_note: null, created_at: new Date().toISOString(), reviewed_at: null }); + this.changes = 1; return []; } if ( q.startsWith("SELECT * FROM extension_submissions WHERE submitted_by = ?") ) { - return [...this.tables.extension_submissions.values()] + let rows = [...this.tables.extension_submissions.values()] .filter((r) => r.submitted_by === p[0]) - .sort((a, b) => - String(b.created_at).localeCompare(String(a.created_at)) + .sort((a, b) => { + const byDate = String(b.created_at).localeCompare( + String(a.created_at) + ); + return byDate || String(b.id).localeCompare(String(a.id)); + }); + if (q.includes("created_at < ?")) { + const [, createdAt, , id] = p; + rows = rows.filter( + (r) => + String(r.created_at) < String(createdAt) || + (r.created_at === createdAt && String(r.id) < String(id)) ); + } + return rows.slice(0, Number(p.at(-1))); } if (q.startsWith("SELECT * FROM extension_submissions WHERE status = ?")) { - return [...this.tables.extension_submissions.values()] + let rows = [...this.tables.extension_submissions.values()] .filter((r) => r.status === p[0]) - .sort((a, b) => - String(a.created_at).localeCompare(String(b.created_at)) + .sort((a, b) => { + const byDate = String(a.created_at).localeCompare( + String(b.created_at) + ); + return byDate || String(a.id).localeCompare(String(b.id)); + }); + if (q.includes("created_at > ?")) { + const [, createdAt, , id] = p; + rows = rows.filter( + (r) => + String(r.created_at) > String(createdAt) || + (r.created_at === createdAt && String(r.id) > String(id)) ); + } + return rows.slice(0, Number(p.at(-1))); } if (q.startsWith("SELECT * FROM extension_submissions WHERE id = ?")) { @@ -726,6 +840,34 @@ class MockStatement implements D1PreparedStatement { return row ? [row] : []; } + if ( + q.startsWith("UPDATE extension_submissions SET status = 'rejected'") && + q.includes("Ownership changed before review") + ) { + const [token_hash, accepted_by] = p; + const transfer = [...this.tables.developer_transfers.values()].find( + (r) => + r.token_hash === token_hash && + r.accepted_by === accepted_by && + r.accepted_at !== null + ); + let changes = 0; + for (const row of this.tables.extension_submissions.values()) { + if ( + transfer && + row.developer_id === transfer.developer_id && + row.status === "pending" + ) { + row.status = "rejected"; + row.review_note = "Ownership changed before review"; + row.reviewed_at = new Date().toISOString(); + changes++; + } + } + this.changes = changes; + return []; + } + if (q.startsWith("UPDATE extension_submissions SET status = 'rejected'")) { const [reviewer_id, review_note, id] = p; const row = this.tables.extension_submissions.get(String(id)); @@ -742,7 +884,29 @@ class MockStatement implements D1PreparedStatement { if (q.startsWith("UPDATE extension_submissions SET status = 'approved'")) { const [reviewer_id, review_note, id] = p; const row = this.tables.extension_submissions.get(String(id)); - if (row && row.status === "pending") { + if ( + this.tables.raceOwnerChangeOnSubmissionApprovalTo !== undefined && + row + ) { + const developer = this.tables.developers.get(String(row.developer_id)); + if (developer) { + developer.owner_user_id = + this.tables.raceOwnerChangeOnSubmissionApprovalTo; + developer.ownership_epoch = + Number(developer.ownership_epoch ?? 1) + 1; + } + this.tables.raceOwnerChangeOnSubmissionApprovalTo = undefined; + } + const developer = row + ? this.tables.developers.get(String(row.developer_id)) + : undefined; + if ( + row && + row.status === "pending" && + developer?.owner_user_id === row.submitted_by && + Number(developer?.ownership_epoch ?? 1) === + Number(row.ownership_epoch ?? 1) + ) { row.status = "approved"; row.reviewer_id = reviewer_id; row.review_note = review_note; @@ -906,6 +1070,15 @@ export function createMockD1(tables: MockTables): D1Database { async batch( statements: D1PreparedStatement[] ): Promise[]> { + const snapshots = { + developers: structuredClone(tables.developers), + extensions: structuredClone(tables.extensions), + extension_submissions: structuredClone(tables.extension_submissions), + developer_history: structuredClone(tables.developer_history), + developer_transfers: structuredClone(tables.developer_transfers), + developer_claims: structuredClone(tables.developer_claims), + users: structuredClone(tables.users) + }; const results: D1Result[] = []; // Mirrors SQL's changes(): the mock has no other way to expose "how // many rows did the immediately preceding statement change" across @@ -915,15 +1088,27 @@ export function createMockD1(tables: MockTables): D1Database { // past) — a statement referencing that gate is skipped here unless // the previous statement's change count was exactly 1. let lastChanges = 0; - for (const stmt of statements) { - const query = stmt instanceof MockStatement ? stmt.normalizedQuery : ""; - if (query.includes("WHERE changes() = 1") && lastChanges !== 1) { - results.push(ok([], 0)); - continue; + try { + for (const stmt of statements) { + const query = + stmt instanceof MockStatement ? stmt.normalizedQuery : ""; + if (query.includes("WHERE changes() = 1") && lastChanges !== 1) { + results.push(ok([], 0)); + continue; + } + const result = await stmt.run(); + lastChanges = result.meta?.changes ?? 0; + results.push(result); } - const result = await stmt.run(); - lastChanges = result.meta?.changes ?? 0; - results.push(result); + } catch (error) { + tables.developers = snapshots.developers; + tables.extensions = snapshots.extensions; + tables.extension_submissions = snapshots.extension_submissions; + tables.developer_history = snapshots.developer_history; + tables.developer_transfers = snapshots.developer_transfers; + tables.developer_claims = snapshots.developer_claims; + tables.users = snapshots.users; + throw error; } return results; },