From b9e5638043cc66232c443292df990f2ec0d202fe Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 24 Jul 2026 09:02:38 +0100 Subject: [PATCH 1/5] Add extensions API v2 with submissions and auth --- package-lock.json | 206 ++++++++- package.json | 7 +- src/app/index.ts | 2 + src/lib/auth/bearer-assertion.ts | 86 ++++ src/lib/auth/index.ts | 3 + src/lib/auth/interfaces.ts | 13 + src/lib/auth/middleware.ts | 54 +++ .../v2/db/migrations/0001_add_v2_tables.sql | 33 ++ src/services/extensions/v2/index.ts | 417 ++++++++++++++++++ src/services/extensions/v2/interfaces.ts | 137 ++++++ .../extensions/v2/submissions-database.ts | 388 ++++++++++++++++ src/services/extensions/v2/users-database.ts | 22 + test/lib/auth/assertion-helper.ts | 51 +++ test/lib/auth/bearer-assertion.test.ts | 94 ++++ test/services/extensions/v2/index.test.ts | 397 +++++++++++++++++ test/services/extensions/v2/mock-db.ts | 281 ++++++++++++ vitest.config.ts | 3 +- wrangler.jsonc | 3 +- 18 files changed, 2189 insertions(+), 8 deletions(-) create mode 100644 src/lib/auth/bearer-assertion.ts create mode 100644 src/lib/auth/index.ts create mode 100644 src/lib/auth/interfaces.ts create mode 100644 src/lib/auth/middleware.ts create mode 100644 src/services/extensions/v2/db/migrations/0001_add_v2_tables.sql create mode 100644 src/services/extensions/v2/index.ts create mode 100644 src/services/extensions/v2/interfaces.ts create mode 100644 src/services/extensions/v2/submissions-database.ts create mode 100644 src/services/extensions/v2/users-database.ts create mode 100644 test/lib/auth/assertion-helper.ts create mode 100644 test/lib/auth/bearer-assertion.test.ts create mode 100644 test/services/extensions/v2/index.test.ts create mode 100644 test/services/extensions/v2/mock-db.ts diff --git a/package-lock.json b/package-lock.json index d66ca83..2b2ce14 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7,11 +7,14 @@ "name": "api", "license": "AGPL-3.0-only", "dependencies": { + "@hono/zod-openapi": "^1.5.1", "@octokit/graphql": "^9.0.3", "@octokit/request": "^10.0.2", + "@scalar/hono-api-reference": "^0.11.11", "badge-maker": "^6.0.0", "hono": "^4.11.4", - "semver": "^7.7.2" + "semver": "^7.7.2", + "zod": "^4.4.3" }, "devDependencies": { "@cloudflare/vitest-pool-workers": "0.18.6", @@ -33,6 +36,18 @@ "wrangler": "4.112.0" } }, + "node_modules/@asteasolutions/zod-to-openapi": { + "version": "8.5.0", + "resolved": "https://registry.npmjs.org/@asteasolutions/zod-to-openapi/-/zod-to-openapi-8.5.0.tgz", + "integrity": "sha512-SABbKiObg5dLRiTFnqiW1WWwGcg1BJfmHtT2asIBnBHg6Smy/Ms2KHc650+JI4Hw7lSkdiNebEGXpwoxfben8Q==", + "license": "MIT", + "dependencies": { + "openapi3-ts": "^4.1.2" + }, + "peerDependencies": { + "zod": "^4.0.0" + } + }, "node_modules/@babel/code-frame": { "version": "7.29.7", "resolved": "https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.29.7.tgz", @@ -338,6 +353,16 @@ "vitest": "^4.1.0" } }, + "node_modules/@cloudflare/vitest-pool-workers/node_modules/zod": { + "version": "3.25.76", + "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", + "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", + "dev": true, + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/colinhacks" + } + }, "node_modules/@cloudflare/workerd-darwin-64": { "version": "1.20260714.1", "resolved": "https://registry.npmjs.org/@cloudflare/workerd-darwin-64/-/workerd-darwin-64-1.20260714.1.tgz", @@ -1038,6 +1063,34 @@ "node": "^20.19.0 || ^22.13.0 || >=24" } }, + "node_modules/@hono/zod-openapi": { + "version": "1.5.1", + "resolved": "https://registry.npmjs.org/@hono/zod-openapi/-/zod-openapi-1.5.1.tgz", + "integrity": "sha512-ZaDdEIkn6PEGjIYHXJeyByg6yhvLzI+UXn968pWtahpwcQ6HMjQYXI3zNffTl0Wl9QZ79nUnGqiN1erEIu23fA==", + "license": "MIT", + "dependencies": { + "@asteasolutions/zod-to-openapi": "^8.5.0", + "@hono/zod-validator": "^0.9.0", + "openapi3-ts": "^4.5.0" + }, + "engines": { + "node": ">=16.0.0" + }, + "peerDependencies": { + "hono": ">=4.10.0", + "zod": "^4.0.0" + } + }, + "node_modules/@hono/zod-validator": { + "version": "0.9.0", + "resolved": "https://registry.npmjs.org/@hono/zod-validator/-/zod-validator-0.9.0.tgz", + "integrity": "sha512-n0ZSXmCiHVIp4Y5wlOOyZCeTd/rsawA/qW1cipB8QOYKZ9N8Tk0nZUZCXho9cu374AN4JpDNKioNKBJ/W+LBug==", + "license": "MIT", + "peerDependencies": { + "hono": ">=4.11.2", + "zod": "^3.25.0 || ^4.0.0" + } + }, "node_modules/@humanfs/core": { "version": "0.19.2", "resolved": "https://registry.npmjs.org/@humanfs/core/-/core-0.19.2.tgz", @@ -2126,6 +2179,99 @@ "dev": true, "license": "MIT" }, + "node_modules/@scalar/client-side-rendering": { + "version": "0.3.4", + "resolved": "https://registry.npmjs.org/@scalar/client-side-rendering/-/client-side-rendering-0.3.4.tgz", + "integrity": "sha512-kb3B+FGjvAUr2DU0fe9dVKBLwot1TjOO+iHCTmY8r8FUJySmYKGQYCicRzzilOyTjvKspiZBCEr03PWaWW+1gw==", + "license": "MIT", + "dependencies": { + "@scalar/schemas": "0.7.4", + "@scalar/types": "0.16.4", + "@scalar/validation": "0.6.2" + }, + "engines": { + "node": ">=22" + } + }, + "node_modules/@scalar/helpers": { + "version": "0.9.2", + "resolved": "https://registry.npmjs.org/@scalar/helpers/-/helpers-0.9.2.tgz", + "integrity": "sha512-hjyMpMZjTBZQhyByZmz5oUgRKUQJO5V5AOiJxsVEGbUmgA7sJRQeTrXLB+BEwzaKS5nm2opJeNyMBYLFNK4hiQ==", + "license": "MIT", + "engines": { + "node": ">=22" + } + }, + "node_modules/@scalar/hono-api-reference": { + "version": "0.11.11", + "resolved": "https://registry.npmjs.org/@scalar/hono-api-reference/-/hono-api-reference-0.11.11.tgz", + "integrity": "sha512-yvJ2oqyG9MC4C55/Xvi/Jny5qBYDxDvoVleABhgNG24A93u9ar17qsdSppli94sQOUdX31Qw/yCTm89LHbBRiQ==", + "license": "MIT", + "dependencies": { + "@scalar/client-side-rendering": "0.3.4" + }, + "engines": { + "node": ">=22" + }, + "peerDependencies": { + "hono": "^4.12.5" + } + }, + "node_modules/@scalar/schemas": { + "version": "0.7.4", + "resolved": "https://registry.npmjs.org/@scalar/schemas/-/schemas-0.7.4.tgz", + "integrity": "sha512-Or31zxR+ceGGhkVU5XBO2Zv7oyGDtjsJOjM2Rr0WRZ1/tRsQc2/FsVv+9kPmCA7sqFL8BKWlNfTXcPITI7WRHw==", + "license": "MIT", + "dependencies": { + "@scalar/helpers": "0.9.2", + "@scalar/validation": "0.6.2" + }, + "engines": { + "node": ">=22" + } + }, + "node_modules/@scalar/types": { + "version": "0.16.4", + "resolved": "https://registry.npmjs.org/@scalar/types/-/types-0.16.4.tgz", + "integrity": "sha512-fLf0ANAC3iQq0sIVdmH6aGM+pFSLyD8GfGBxSWcLpI0jE0iFAzX2A3p0qVZm7vqBT4TQnn0fSwg5U+h+FZ52BA==", + "license": "MIT", + "dependencies": { + "@scalar/helpers": "0.9.2", + "nanoid": "^5.1.6", + "type-fest": "^5.3.1", + "zod": "^4.3.5" + }, + "engines": { + "node": ">=22" + } + }, + "node_modules/@scalar/types/node_modules/nanoid": { + "version": "5.1.16", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-5.1.16.tgz", + "integrity": "sha512-kVrnsrJqMR8+oLJnGEmSWw9BivK5mt7H3FZatVRjrc5wGqFYuBxX1yG7+A7Gi5AefkX6t/oCkizcQgpu0cY1dQ==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/ai" + } + ], + "license": "MIT", + "bin": { + "nanoid": "bin/nanoid.js" + }, + "engines": { + "node": "^18 || >=20" + } + }, + "node_modules/@scalar/validation": { + "version": "0.6.2", + "resolved": "https://registry.npmjs.org/@scalar/validation/-/validation-0.6.2.tgz", + "integrity": "sha512-Sc1TkcwGV6aVCO51AyKeaGiP8gpwAHxEtO5d3tZzPV+KsnlC/YokQxFxwBrbIXw73k9hmcExnJyGu3k5i6n6VA==", + "license": "MIT", + "engines": { + "node": ">=20" + } + }, "node_modules/@sindresorhus/is": { "version": "7.2.0", "resolved": "https://registry.npmjs.org/@sindresorhus/is/-/is-7.2.0.tgz", @@ -4032,6 +4178,15 @@ "node": ">=12.20.0" } }, + "node_modules/openapi3-ts": { + "version": "4.6.0", + "resolved": "https://registry.npmjs.org/openapi3-ts/-/openapi3-ts-4.6.0.tgz", + "integrity": "sha512-a4sfn6L2sIShhtzJqmjGrARvxAW/3F2BJDdyRVvNF9VhAsZSh5hSyI3a9TNvmzBxXmq66nY5LNT5bQcBxYAZZg==", + "license": "MIT", + "dependencies": { + "yaml": "^2.9.0" + } + }, "node_modules/optionator": { "version": "0.9.4", "resolved": "https://registry.npmjs.org/optionator/-/optionator-0.9.4.tgz", @@ -4359,6 +4514,18 @@ "node": ">=8" } }, + "node_modules/tagged-tag": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/tagged-tag/-/tagged-tag-1.0.0.tgz", + "integrity": "sha512-yEFYrVhod+hdNyx7g5Bnkkb0G6si8HJurOoOEgC8B/O0uXLHlaey/65KRv6cuWBNhBgHKAROVpc7QyYqE5gFng==", + "license": "MIT", + "engines": { + "node": ">=20" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/tinybench": { "version": "2.9.0", "resolved": "https://registry.npmjs.org/tinybench/-/tinybench-2.9.0.tgz", @@ -4456,6 +4623,21 @@ "node": ">= 0.8.0" } }, + "node_modules/type-fest": { + "version": "5.8.0", + "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-5.8.0.tgz", + "integrity": "sha512-YGYEVz3Fm5iy/AybuA0oyNFq7H4CgQNfRp/qfe8nurE1kuCeNm3/vfm9X4Mtl+qLyaKJUh5xrFZwogr41SMjYA==", + "license": "(MIT OR CC0-1.0)", + "dependencies": { + "tagged-tag": "^1.0.0" + }, + "engines": { + "node": ">=20" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/typescript": { "version": "6.0.3", "resolved": "https://registry.npmjs.org/typescript/-/typescript-6.0.3.tgz", @@ -4865,6 +5047,21 @@ "dev": true, "license": "ISC" }, + "node_modules/yaml": { + "version": "2.9.0", + "resolved": "https://registry.npmjs.org/yaml/-/yaml-2.9.0.tgz", + "integrity": "sha512-2AvhNX3mb8zd6Zy7INTtSpl1F15HW6Wnqj0srWlkKLcpYl/gMIMJiyuGq2KeI2YFxUPjdlB+3Lc10seMLtL4cA==", + "license": "ISC", + "bin": { + "yaml": "bin.mjs" + }, + "engines": { + "node": ">= 14.6" + }, + "funding": { + "url": "https://github.com/sponsors/eemeli" + } + }, "node_modules/yocto-queue": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", @@ -4904,10 +5101,9 @@ } }, "node_modules/zod": { - "version": "3.25.76", - "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", - "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", - "dev": true, + "version": "4.4.3", + "resolved": "https://registry.npmjs.org/zod/-/zod-4.4.3.tgz", + "integrity": "sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ==", "license": "MIT", "funding": { "url": "https://github.com/sponsors/colinhacks" diff --git a/package.json b/package.json index 5386c24..b79e8cc 100644 --- a/package.json +++ b/package.json @@ -9,6 +9,8 @@ "format": "prettier --write .", "format:check": "prettier --check .", "init:db": "npx tsx src/services/central-alerts/v1/scripts/init-db.ts", + "migrate:extensions-v2": "wrangler d1 migrations apply DB_EXTENSIONS", + "migrate:extensions-v2:list": "wrangler d1 migrations list DB_EXTENSIONS", "lint": "eslint", "lint:fix": "eslint --fix", "test": "vitest run", @@ -18,11 +20,14 @@ "typecheck": "tsc --noEmit" }, "dependencies": { + "@hono/zod-openapi": "^1.5.1", "@octokit/graphql": "^9.0.3", "@octokit/request": "^10.0.2", + "@scalar/hono-api-reference": "^0.11.11", "badge-maker": "^6.0.0", "hono": "^4.11.4", - "semver": "^7.7.2" + "semver": "^7.7.2", + "zod": "^4.4.3" }, "devDependencies": { "@cloudflare/vitest-pool-workers": "0.18.6", diff --git a/src/app/index.ts b/src/app/index.ts index 589fb6b..4716414 100644 --- a/src/app/index.ts +++ b/src/app/index.ts @@ -3,6 +3,7 @@ import { contextStorage } from "hono/context-storage"; import { HTTPException } from "hono/http-exception"; import centralAlertsV1 from "../services/central-alerts/v1"; import extensionsV1 from "../services/extensions/v1"; +import extensionsV2 from "../services/extensions/v2"; import versionsV1 from "../services/versions/v1"; import statsV1 from "../services/stats/v1"; import { platformMiddleware } from "../lib/middleware"; @@ -21,6 +22,7 @@ app.use("*", async (c, next) => { app.route("/central-alerts/v1", centralAlertsV1); app.route("/extensions/v1", extensionsV1); +app.route("/extensions/v2", extensionsV2); app.route("/versions/v1", versionsV1); app.route("/stats/v1", statsV1); diff --git a/src/lib/auth/bearer-assertion.ts b/src/lib/auth/bearer-assertion.ts new file mode 100644 index 0000000..a0b341b --- /dev/null +++ b/src/lib/auth/bearer-assertion.ts @@ -0,0 +1,86 @@ +import { AuthPrincipal, TokenVerifier } from "./interfaces"; + +const CLOCK_SKEW_SECONDS = 5; + +interface AssertionPayload { + sub: string; + iat: number; + exp: number; +} + +function base64UrlDecode(input: string): Uint8Array { + const normalized = input.replace(/-/g, "+").replace(/_/g, "/"); + const padded = + normalized.length % 4 === 0 + ? normalized + : normalized + "=".repeat(4 - (normalized.length % 4)); + const binary = atob(padded); + const bytes = new Uint8Array(binary.length); + for (let i = 0; i < binary.length; i++) { + bytes[i] = binary.charCodeAt(i); + } + return bytes; +} + +function isAssertionPayload(value: unknown): value is AssertionPayload { + if (typeof value !== "object" || value === null) return false; + const record = value as Record; + return ( + typeof record.sub === "string" && + record.sub.length > 0 && + typeof record.iat === "number" && + typeof record.exp === "number" + ); +} + +// Verifies a compact HS256 assertion (header.payload.signature). The header +// is never parsed or trusted to pick the algorithm, which avoids alg-confusion. +export const bearerAssertionVerifier: TokenVerifier = { + async verify(token, platform): Promise { + const secret = platform.getEnv("ASSERTION_SIGNING_SECRET"); + if (!secret) return null; + + const parts = token.split("."); + if (parts.length !== 3) return null; + const [headerB64, payloadB64, signatureB64] = parts; + + let payload: unknown; + try { + payload = JSON.parse( + new TextDecoder().decode(base64UrlDecode(payloadB64)) + ); + } catch { + return null; + } + if (!isAssertionPayload(payload)) return null; + + let signature: Uint8Array; + try { + signature = base64UrlDecode(signatureB64); + } catch { + return null; + } + + const key = await crypto.subtle.importKey( + "raw", + new TextEncoder().encode(secret), + { name: "HMAC", hash: "SHA-256" }, + false, + ["verify"] + ); + + const valid = await crypto.subtle.verify( + "HMAC", + key, + signature, + new TextEncoder().encode(`${headerB64}.${payloadB64}`) + ); + if (!valid) return null; + + const now = Math.floor(Date.now() / 1000); + if (payload.exp <= now) return null; + if (payload.iat > now + CLOCK_SKEW_SECONDS) return null; + + return { userId: payload.sub, scope: "assertion" }; + } +}; diff --git a/src/lib/auth/index.ts b/src/lib/auth/index.ts new file mode 100644 index 0000000..dcb2748 --- /dev/null +++ b/src/lib/auth/index.ts @@ -0,0 +1,3 @@ +export type { AuthPrincipal, TokenVerifier } from "./interfaces"; +export { bearerAssertionVerifier } from "./bearer-assertion"; +export { requireAuth, getAuth } from "./middleware"; diff --git a/src/lib/auth/interfaces.ts b/src/lib/auth/interfaces.ts new file mode 100644 index 0000000..115b4af --- /dev/null +++ b/src/lib/auth/interfaces.ts @@ -0,0 +1,13 @@ +import { PlatformContext } from "../context"; + +export interface AuthPrincipal { + userId: string; + scope: "assertion" | "api_key"; +} + +export interface TokenVerifier { + verify( + token: string, + platform: PlatformContext + ): Promise; +} diff --git a/src/lib/auth/middleware.ts b/src/lib/auth/middleware.ts new file mode 100644 index 0000000..9f65644 --- /dev/null +++ b/src/lib/auth/middleware.ts @@ -0,0 +1,54 @@ +import { Context, MiddlewareHandler } from "hono"; +import { getPlatform } from "../middleware"; +import { AuthPrincipal, TokenVerifier } from "./interfaces"; +import { bearerAssertionVerifier } from "./bearer-assertion"; + +declare module "hono" { + interface ContextVariableMap { + auth: AuthPrincipal; + } +} + +// Ordered list of verifiers tried for an incoming bearer token. Adding the +// future long-lived API-key verifier means appending here — route handlers +// and requireAuth() itself don't change. +const verifiers: TokenVerifier[] = [bearerAssertionVerifier]; + +export function requireAuth(): MiddlewareHandler { + return async (c, next) => { + const header = c.req.header("Authorization"); + const token = header?.startsWith("Bearer ") + ? header.slice("Bearer ".length).trim() + : null; + + if (!token) { + return c.json( + { error: { message: "Missing bearer token", code: "UNAUTHORIZED" } }, + 401 + ); + } + + const platform = getPlatform(c); + for (const verifier of verifiers) { + const principal = await verifier.verify(token, platform); + if (principal) { + c.set("auth", principal); + await next(); + return; + } + } + + return c.json( + { error: { message: "Invalid or expired token", code: "UNAUTHORIZED" } }, + 401 + ); + }; +} + +export function getAuth(c: Context): AuthPrincipal { + const auth = c.get("auth"); + if (!auth) { + throw new Error("Auth principal not found"); + } + return auth; +} diff --git a/src/services/extensions/v2/db/migrations/0001_add_v2_tables.sql b/src/services/extensions/v2/db/migrations/0001_add_v2_tables.sql new file mode 100644 index 0000000..9f20b24 --- /dev/null +++ b/src/services/extensions/v2/db/migrations/0001_add_v2_tables.sql @@ -0,0 +1,33 @@ +-- Migration number: 0001 2026-07-24T06:49:28.535Z +-- +-- v2: self-service submissions, ownership, moderation. +-- Adds to the v1-owned `authors` table (../../v1/db/schema.sql) and creates a new +-- v2-owned table. +-- +-- NOTE: `users` referenced below is owned by the FOSSBilling/extensions repo +-- (src/lib/db/users.sql there), NOT this repo, but lives in the same DB_EXTENSIONS +-- database. If that schema changes, update fossbilling/api AND that file. Assumed +-- columns used here: users.id (TEXT, = auth `sub` claim), users.is_moderator +-- (INTEGER 0/1). + +ALTER TABLE authors ADD COLUMN owner_user_id TEXT REFERENCES users(id); +CREATE INDEX IF NOT EXISTS idx_authors_owner ON authors(owner_user_id); + +CREATE TABLE IF NOT EXISTS extension_submissions ( + id TEXT PRIMARY KEY NOT NULL, + extension_id TEXT REFERENCES extensions(id), -- NULL = new extension, set = edit + author_id TEXT NOT NULL, -- not a hard FK: may name an author + -- that doesn't exist yet (created on approval) + submitted_by TEXT NOT NULL REFERENCES users(id), + status TEXT NOT NULL DEFAULT 'pending' CHECK (status IN ('pending', 'approved', 'rejected')), + payload TEXT NOT NULL, -- JSON: { author: {...}, extension: {...} } + reviewer_id TEXT REFERENCES users(id), + review_note TEXT, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + reviewed_at DATETIME +); + +CREATE INDEX IF NOT EXISTS idx_submissions_status ON extension_submissions(status); +CREATE INDEX IF NOT EXISTS idx_submissions_submitted_by ON extension_submissions(submitted_by); +CREATE INDEX IF NOT EXISTS idx_submissions_author ON extension_submissions(author_id); +CREATE INDEX IF NOT EXISTS idx_submissions_extension ON extension_submissions(extension_id); diff --git a/src/services/extensions/v2/index.ts b/src/services/extensions/v2/index.ts new file mode 100644 index 0000000..54af9b5 --- /dev/null +++ b/src/services/extensions/v2/index.ts @@ -0,0 +1,417 @@ +import { OpenAPIHono, createRoute, z } from "@hono/zod-openapi"; +import { cors } from "hono/cors"; +import { trimTrailingSlash } from "hono/trailing-slash"; +import { MiddlewareHandler } from "hono"; +import { Scalar } from "@scalar/hono-api-reference"; +import { getPlatform } from "../../../lib/middleware"; +import { getAuth, requireAuth } from "../../../lib/auth"; +import { + ErrorResponseSchema, + IdParamSchema, + QueueQuerySchema, + ReviewNoteOptionalSchema, + ReviewNoteRequiredSchema, + SubmissionPayloadSchema, + SubmissionSchema +} from "./interfaces"; +import { SubmissionsDatabase } from "./submissions-database"; +import { UsersDatabase } from "./users-database"; + +const extensionsV2 = new OpenAPIHono<{ Bindings: CloudflareBindings }>({ + defaultHook: (result, c) => { + if (!result.success) { + return c.json( + { + error: { + message: "Invalid request", + code: "VALIDATION_ERROR", + details: result.error.issues + } + }, + 422 + ); + } + } +}); + +extensionsV2.use("/*", cors({ origin: "*" })); +extensionsV2.use("/*", trimTrailingSlash()); + +extensionsV2.openAPIRegistry.registerComponent("securitySchemes", "Bearer", { + type: "http", + scheme: "bearer" +}); + +function statusFromErrorCode(code?: string): 404 | 409 | 500 { + if (code === "NOT_FOUND") return 404; + if (code === "CONFLICT") return 409; + return 500; +} + +function requireModerator(): MiddlewareHandler { + return async (c, next) => { + const auth = getAuth(c); + const platform = getPlatform(c); + const users = new UsersDatabase(platform.getDatabase("DB_EXTENSIONS")); + + const isModerator = await users.isModerator(auth.userId); + if (!isModerator) { + return c.json( + { error: { message: "Moderator access required", code: "FORBIDDEN" } }, + 403 + ); + } + + await next(); + }; +} + +const createSubmissionRoute = createRoute({ + method: "post", + path: "/submissions", + tags: ["Submissions"], + summary: "Submit a new extension, or an edit to one you own", + security: [{ Bearer: [] }], + middleware: [requireAuth()] as const, + request: { + body: { + content: { "application/json": { schema: SubmissionPayloadSchema } } + } + }, + responses: { + 201: { + content: { + "application/json": { + schema: z.object({ + result: z.object({ id: z.string(), status: z.literal("pending") }) + }) + } + }, + description: "Submission created and pending moderator review" + }, + 401: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Missing or invalid bearer token" + }, + 403: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Caller does not own the target author or extension" + }, + 422: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Payload failed validation" + }, + 500: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Database error" + } + } +}); + +extensionsV2.openapi(createSubmissionRoute, async (c) => { + const auth = getAuth(c); + const payload = c.req.valid("json"); + const platform = getPlatform(c); + const db = new SubmissionsDatabase(platform.getDatabase("DB_EXTENSIONS")); + + const ownership = await db.resolveOwnership(payload, auth.userId); + if (ownership.error || !ownership.data) { + return c.json( + { + error: { + message: ownership.error?.message ?? "Unable to validate ownership", + code: ownership.error?.code ?? "DATABASE_ERROR" + } + }, + ownership.error?.code === "FORBIDDEN" ? 403 : 500 + ); + } + + const created = await db.create({ + extensionId: ownership.data.extensionId, + authorId: ownership.data.authorId, + submittedBy: auth.userId, + payload + }); + if (created.error || !created.data) { + return c.json( + { + error: { + message: created.error?.message ?? "Unable to create submission", + code: "DATABASE_ERROR" + } + }, + 500 + ); + } + + return c.json( + { result: { id: created.data.id, status: "pending" as const } }, + 201 + ); +}); + +const mineRoute = createRoute({ + method: "get", + path: "/submissions/mine", + tags: ["Submissions"], + summary: "List the caller's own submissions, in any status", + security: [{ Bearer: [] }], + middleware: [requireAuth()] as const, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ result: z.array(SubmissionSchema) }) + } + }, + description: "The caller's submissions" + }, + 401: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Missing or invalid bearer token" + }, + 500: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Database error" + } + } +}); + +extensionsV2.openapi(mineRoute, async (c) => { + const auth = getAuth(c); + const platform = getPlatform(c); + const db = new SubmissionsDatabase(platform.getDatabase("DB_EXTENSIONS")); + + const { data, error } = await db.listBySubmitter(auth.userId); + if (error || !data) { + return c.json( + { + error: { + message: error?.message ?? "Unable to load submissions", + code: "DATABASE_ERROR" + } + }, + 500 + ); + } + + return c.json({ result: data }, 200); +}); + +const queueRoute = createRoute({ + method: "get", + path: "/submissions/queue", + tags: ["Moderation"], + summary: "List submissions in the moderation queue", + security: [{ Bearer: [] }], + middleware: [requireAuth(), requireModerator()] as const, + request: { query: QueueQuerySchema }, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ result: z.array(SubmissionSchema) }) + } + }, + description: + "Submissions matching the requested status (default: pending)" + }, + 401: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Missing or invalid bearer token" + }, + 403: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Caller is not a moderator" + }, + 500: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Database error" + } + } +}); + +extensionsV2.openapi(queueRoute, async (c) => { + const platform = getPlatform(c); + const db = new SubmissionsDatabase(platform.getDatabase("DB_EXTENSIONS")); + + const { status } = c.req.valid("query"); + + const { data, error } = await db.listQueue(status ?? "pending"); + if (error || !data) { + return c.json( + { + error: { + message: error?.message ?? "Unable to load queue", + code: "DATABASE_ERROR" + } + }, + 500 + ); + } + + return c.json({ result: data }, 200); +}); + +const approveRoute = createRoute({ + method: "post", + path: "/submissions/{id}/approve", + tags: ["Moderation"], + summary: "Approve a pending submission", + security: [{ Bearer: [] }], + middleware: [requireAuth(), requireModerator()] as const, + request: { + params: IdParamSchema, + body: { + content: { "application/json": { schema: ReviewNoteOptionalSchema } } + } + }, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ + result: z.object({ id: z.string(), status: z.literal("approved") }) + }) + } + }, + description: + "Submission approved and written through to the live extension/author" + }, + 401: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Missing or invalid bearer token" + }, + 403: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Caller is not a moderator" + }, + 404: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "No submission with that id" + }, + 409: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: + "Submission is not pending, or ownership has changed since it was submitted" + }, + 500: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Database error" + } + } +}); + +extensionsV2.openapi(approveRoute, async (c) => { + const auth = getAuth(c); + const { id } = c.req.valid("param"); + const { review_note } = c.req.valid("json"); + const platform = getPlatform(c); + const db = new SubmissionsDatabase(platform.getDatabase("DB_EXTENSIONS")); + + const { data, error } = await db.approve(id, auth.userId, review_note); + if (error || !data) { + const status = statusFromErrorCode(error?.code); + return c.json( + { + error: { + message: error?.message ?? "Unable to approve submission", + code: error?.code ?? "DATABASE_ERROR" + } + }, + status + ); + } + + return c.json({ result: data }, 200); +}); + +const rejectRoute = createRoute({ + method: "post", + path: "/submissions/{id}/reject", + tags: ["Moderation"], + summary: "Reject a pending submission", + security: [{ Bearer: [] }], + middleware: [requireAuth(), requireModerator()] as const, + request: { + params: IdParamSchema, + body: { + content: { "application/json": { schema: ReviewNoteRequiredSchema } } + } + }, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ + result: z.object({ id: z.string(), status: z.literal("rejected") }) + }) + } + }, + description: "Submission rejected" + }, + 401: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Missing or invalid bearer token" + }, + 403: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Caller is not a moderator" + }, + 404: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "No submission with that id" + }, + 409: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Submission is not pending" + }, + 422: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "review_note is required" + }, + 500: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Database error" + } + } +}); + +extensionsV2.openapi(rejectRoute, async (c) => { + const auth = getAuth(c); + const { id } = c.req.valid("param"); + const { review_note } = c.req.valid("json"); + const platform = getPlatform(c); + const db = new SubmissionsDatabase(platform.getDatabase("DB_EXTENSIONS")); + + const { data, error } = await db.reject(id, auth.userId, review_note); + if (error || !data) { + const status = statusFromErrorCode(error?.code); + return c.json( + { + error: { + message: error?.message ?? "Unable to reject submission", + code: error?.code ?? "DATABASE_ERROR" + } + }, + status + ); + } + + return c.json({ result: data }, 200); +}); + +extensionsV2.doc("/openapi.json", { + openapi: "3.1.0", + info: { + title: "FOSSBilling Extensions API v2", + version: "2.0.0", + description: + "Self-service extension submission, ownership, and moderation. Read-only listings remain at /extensions/v1." + } +}); + +extensionsV2.get("/docs", Scalar({ url: "/extensions/v2/openapi.json" })); + +export default extensionsV2; diff --git a/src/services/extensions/v2/interfaces.ts b/src/services/extensions/v2/interfaces.ts new file mode 100644 index 0000000..42412df --- /dev/null +++ b/src/services/extensions/v2/interfaces.ts @@ -0,0 +1,137 @@ +import { z } from "@hono/zod-openapi"; + +export const EXTENSION_TYPES = [ + "mod", + "theme", + "payment-gateway", + "server-manager", + "domain-registrar", + "hook", + "translation" +] as const; + +const lowercaseId = (label: string) => + z + .string() + .min(1) + .refine((value) => value === value.toLowerCase(), { + message: `${label} id must be lowercase` + }); + +export const AuthorSchema = z + .object({ + id: lowercaseId("author"), + type: z.enum(["user", "organization"]), + name: z.string().min(1), + url: z.string().url().optional() + }) + .openapi("Author"); + +export const ReleaseSchema = z + .object({ + tag: z.string().min(1), + date: z.string().min(1), + download_url: z.string().url(), + changelog_url: z.string().url().optional(), + min_fossbilling_version: z.string().min(1) + }) + .openapi("Release"); + +export const RepositorySchema = z + .object({ + type: z.enum(["github", "gitlab", "custom"]), + repo: z.string().min(1) + }) + .openapi("Repository"); + +export const LicenseSchema = z + .object({ + name: z.string().min(1), + URL: z.string().url().optional() + }) + .openapi("License"); + +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), + website: z.string().url(), + license: LicenseSchema, + icon_url: z.string().url().optional(), + readme: z.string().min(1), + source: RepositorySchema, + version: z.string().min(1), + download_url: z.string().url() + }) + .openapi("ExtensionPayload"); + +export const SubmissionPayloadSchema = z + .object({ + author: AuthorSchema, + extension: ExtensionPayloadSchema + }) + .openapi("SubmissionPayload"); + +export type SubmissionPayload = z.infer; + +export const ReviewNoteOptionalSchema = z + .object({ + review_note: z.string().optional() + }) + .openapi("ReviewNoteOptional"); + +export const ReviewNoteRequiredSchema = z + .object({ + review_note: z.string().min(1) + }) + .openapi("ReviewNoteRequired"); + +export const SubmissionStatusSchema = z.enum([ + "pending", + "approved", + "rejected" +]); + +export type SubmissionStatus = z.infer; + +export const SubmissionSchema = z + .object({ + id: z.string(), + extension_id: z.string().nullable(), + author_id: z.string(), + submitted_by: z.string(), + status: SubmissionStatusSchema, + payload: SubmissionPayloadSchema, + reviewer_id: z.string().nullable(), + review_note: z.string().nullable(), + created_at: z.string(), + reviewed_at: z.string().nullable() + }) + .openapi("Submission"); + +export type Submission = z.infer; + +export const ErrorResponseSchema = z + .object({ + error: z.object({ + message: z.string(), + code: z.string() + }) + }) + .openapi("Error"); + +export const IdParamSchema = z.object({ + id: z.string().openapi({ + param: { name: "id", in: "path" }, + example: "b6e2c9c4-3f1a-4e9b-9c3a-2e4b1a2f9d10" + }) +}); + +export const QueueQuerySchema = z.object({ + status: SubmissionStatusSchema.optional().openapi({ + param: { name: "status", in: "query" } + }) +}); diff --git a/src/services/extensions/v2/submissions-database.ts b/src/services/extensions/v2/submissions-database.ts new file mode 100644 index 0000000..58b5899 --- /dev/null +++ b/src/services/extensions/v2/submissions-database.ts @@ -0,0 +1,388 @@ +import { DatabaseResult, IDatabase } from "../../../lib/interfaces"; +import { Submission, SubmissionPayload, SubmissionStatus } from "./interfaces"; + +interface OwnershipResolution { + extensionId: string | null; + authorId: string; +} + +interface CreateInput { + extensionId: string | null; + authorId: string; + submittedBy: string; + payload: SubmissionPayload; +} + +function databaseError(error: unknown): DatabaseResult { + return { + data: null, + error: { + message: error instanceof Error ? error.message : String(error), + code: "DATABASE_ERROR" + } + }; +} + +function parseSubmissionRow(row: Record): Submission { + return { + id: row.id as string, + extension_id: (row.extension_id as string | null) ?? null, + author_id: row.author_id as string, + submitted_by: row.submitted_by as string, + status: row.status as SubmissionStatus, + payload: JSON.parse(row.payload as string) as SubmissionPayload, + reviewer_id: (row.reviewer_id as string | null) ?? null, + 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 + }; +} + +export class SubmissionsDatabase { + private db: IDatabase; + + constructor(db: IDatabase) { + this.db = db; + } + + // Edits require owning the extension's current author; the author named in + // the payload (create, or an edit naming a different author) must be owned + // by the caller if it already exists, or is free to claim if it doesn't. + async resolveOwnership( + payload: SubmissionPayload, + callerId: string + ): Promise> { + try { + const existingExtension = await this.db + .prepare( + "SELECT id, author_id FROM extensions WHERE LOWER(id) = LOWER(?)" + ) + .bind(payload.extension.id) + .first<{ id: string; author_id: string }>(); + + let extensionId: string | null = null; + + if (existingExtension) { + const existingAuthor = await this.db + .prepare("SELECT owner_user_id FROM authors WHERE id = ?") + .bind(existingExtension.author_id) + .first<{ owner_user_id: string | null }>(); + + if (!existingAuthor || existingAuthor.owner_user_id !== callerId) { + return { + data: null, + error: { + message: "You do not own the author of this extension", + code: "FORBIDDEN" + } + }; + } + + extensionId = existingExtension.id; + } + + const payloadAuthor = await this.db + .prepare("SELECT owner_user_id FROM authors WHERE id = ?") + .bind(payload.author.id) + .first<{ owner_user_id: string | null }>(); + + if (payloadAuthor && payloadAuthor.owner_user_id !== callerId) { + return { + data: null, + error: { message: "You do not own this author", code: "FORBIDDEN" } + }; + } + + return { + data: { extensionId, authorId: payload.author.id }, + error: null + }; + } catch (error) { + return databaseError(error); + } + } + + async create(input: CreateInput): Promise> { + const id = crypto.randomUUID(); + + try { + const result = await this.db + .prepare( + `INSERT INTO extension_submissions (id, extension_id, author_id, submitted_by, status, payload) + VALUES (?, ?, ?, ?, 'pending', ?)` + ) + .bind( + id, + input.extensionId, + input.authorId, + input.submittedBy, + JSON.stringify(input.payload) + ) + .run(); + + if (!result.success) { + return { + data: null, + error: { + message: result.error || "Database query failed", + code: "DATABASE_ERROR" + } + }; + } + } catch (error) { + return databaseError(error); + } + + return { data: { id }, error: null }; + } + + async listBySubmitter(userId: string): Promise> { + let result; + try { + result = await this.db + .prepare( + "SELECT * FROM extension_submissions WHERE submitted_by = ? ORDER BY created_at DESC" + ) + .bind(userId) + .all>(); + } catch (error) { + return databaseError(error); + } + + if (!result.success) { + return { + data: null, + error: { + message: result.error || "Database query failed", + code: "DATABASE_ERROR" + } + }; + } + + return { + data: (result.results ?? []).map(parseSubmissionRow), + error: null + }; + } + + async listQueue( + status: SubmissionStatus = "pending" + ): Promise> { + let result; + try { + result = await this.db + .prepare( + "SELECT * FROM extension_submissions WHERE status = ? ORDER BY created_at ASC" + ) + .bind(status) + .all>(); + } catch (error) { + return databaseError(error); + } + + if (!result.success) { + return { + data: null, + error: { + message: result.error || "Database query failed", + code: "DATABASE_ERROR" + } + }; + } + + return { + data: (result.results ?? []).map(parseSubmissionRow), + error: null + }; + } + + async getById(id: string): Promise> { + let row; + try { + row = await this.db + .prepare("SELECT * FROM extension_submissions WHERE id = ?") + .bind(id) + .first>(); + } catch (error) { + return databaseError(error); + } + + if (!row) { + return { + data: null, + error: { + message: `Cannot find submission by id: ${id}`, + code: "NOT_FOUND" + } + }; + } + + return { data: parseSubmissionRow(row), error: null }; + } + + async reject( + id: string, + reviewerId: string, + reviewNote: string + ): Promise> { + const existing = await this.getById(id); + if (existing.error || !existing.data) { + return { + data: null, + error: existing.error ?? { + message: `Cannot find submission by id: ${id}`, + code: "NOT_FOUND" + } + }; + } + + if (existing.data.status !== "pending") { + return { + data: null, + error: { message: "Submission is not pending", code: "CONFLICT" } + }; + } + + try { + await this.db + .prepare( + `UPDATE extension_submissions + SET status = 'rejected', reviewer_id = ?, review_note = ?, reviewed_at = CURRENT_TIMESTAMP + WHERE id = ?` + ) + .bind(reviewerId, reviewNote, id) + .run(); + } catch (error) { + return databaseError(error); + } + + return { data: { id, status: "rejected" }, error: null }; + } + + // Re-checks ownership/availability (may have shifted since submission), + // then upserts through to authors/extensions in one atomic batch — the + // upsert form means create vs. edit needs no branching in the SQL. + async approve( + id: string, + reviewerId: string, + reviewNote?: string + ): Promise> { + const existing = await this.getById(id); + if (existing.error || !existing.data) { + return { + data: null, + error: existing.error ?? { + message: `Cannot find submission by id: ${id}`, + code: "NOT_FOUND" + } + }; + } + const submission = existing.data; + + if (submission.status !== "pending") { + return { + data: null, + error: { message: "Submission is not pending", code: "CONFLICT" } + }; + } + + 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" + } + }; + } + + const { author, extension } = submission.payload; + + if (!this.db.batch) { + return { + data: null, + error: { + message: "Database adapter does not support batch operations", + code: "DATABASE_ERROR" + } + }; + } + + try { + const authorStmt = this.db + .prepare( + `INSERT INTO authors (id, type, name, url, owner_user_id) + VALUES (?, ?, ?, ?, ?) + ON CONFLICT(id) DO UPDATE SET + type = excluded.type, name = excluded.name, url = excluded.url` + ) + .bind( + author.id, + author.type, + author.name, + author.url ?? null, + submission.submitted_by + ); + + 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 (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ON CONFLICT(id) DO UPDATE SET + type = excluded.type, author_id = excluded.author_id, name = excluded.name, + description = excluded.description, releases = excluded.releases, + website = excluded.website, license = excluded.license, + icon_url = excluded.icon_url, readme = excluded.readme, + source = excluded.source, version = excluded.version, + download_url = excluded.download_url` + ) + .bind( + extension.id, + extension.type, + author.id, + extension.name, + extension.description, + JSON.stringify(extension.releases), + extension.website, + JSON.stringify(extension.license), + extension.icon_url ?? null, + extension.readme, + JSON.stringify(extension.source), + extension.version, + extension.download_url + ); + + const submissionStmt = this.db + .prepare( + `UPDATE extension_submissions + SET status = 'approved', reviewer_id = ?, review_note = ?, reviewed_at = CURRENT_TIMESTAMP + WHERE id = ?` + ) + .bind(reviewerId, reviewNote ?? null, id); + + await this.db.batch([authorStmt, extensionStmt, submissionStmt]); + } catch (error) { + return databaseError(error); + } + + return { data: { id, status: "approved" }, error: null }; + } +} diff --git a/src/services/extensions/v2/users-database.ts b/src/services/extensions/v2/users-database.ts new file mode 100644 index 0000000..c3fac4c --- /dev/null +++ b/src/services/extensions/v2/users-database.ts @@ -0,0 +1,22 @@ +import { IDatabase } from "../../../lib/interfaces"; + +// `users` is owned by the FOSSBilling/extensions repo (src/lib/db/users.sql there), +// NOT this repo, but lives in the same DB_EXTENSIONS database. If that schema +// changes (columns renamed/dropped), update fossbilling/api AND that file. Assumed +// columns used here: users.id (TEXT, = auth `sub` claim), users.is_moderator +// (INTEGER 0/1). +export class UsersDatabase { + private db: IDatabase; + + constructor(db: IDatabase) { + this.db = db; + } + + async isModerator(userId: string): Promise { + const row = await this.db + .prepare("SELECT is_moderator FROM users WHERE id = ?") + .bind(userId) + .first<{ is_moderator: number }>(); + return row?.is_moderator === 1; + } +} diff --git a/test/lib/auth/assertion-helper.ts b/test/lib/auth/assertion-helper.ts new file mode 100644 index 0000000..845d8df --- /dev/null +++ b/test/lib/auth/assertion-helper.ts @@ -0,0 +1,51 @@ +function base64UrlEncodeBytes(bytes: Uint8Array): string { + let binary = ""; + for (const byte of bytes) binary += String.fromCharCode(byte); + return btoa(binary) + .replace(/\+/g, "-") + .replace(/\//g, "_") + .replace(/=+$/, ""); +} + +export function base64UrlEncodeString(value: string): string { + return base64UrlEncodeBytes(new TextEncoder().encode(value)); +} + +export interface AssertionOverrides { + sub?: string; + iat?: number; + exp?: number; +} + +/** Mints a compact HS256 assertion matching what bearer-assertion.ts verifies. */ +export async function signAssertion( + secret: string, + overrides: AssertionOverrides = {} +): Promise { + const now = Math.floor(Date.now() / 1000); + const payload = { + sub: overrides.sub ?? "user-1", + iat: overrides.iat ?? now, + exp: overrides.exp ?? now + 60 + }; + + const headerB64 = base64UrlEncodeString( + JSON.stringify({ alg: "HS256", typ: "JWT" }) + ); + const payloadB64 = base64UrlEncodeString(JSON.stringify(payload)); + + const key = await crypto.subtle.importKey( + "raw", + new TextEncoder().encode(secret), + { name: "HMAC", hash: "SHA-256" }, + false, + ["sign"] + ); + const signature = await crypto.subtle.sign( + "HMAC", + key, + new TextEncoder().encode(`${headerB64}.${payloadB64}`) + ); + + return `${headerB64}.${payloadB64}.${base64UrlEncodeBytes(new Uint8Array(signature))}`; +} diff --git a/test/lib/auth/bearer-assertion.test.ts b/test/lib/auth/bearer-assertion.test.ts new file mode 100644 index 0000000..0211e65 --- /dev/null +++ b/test/lib/auth/bearer-assertion.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect } from "vitest"; +import { bearerAssertionVerifier } from "../../../src/lib/auth/bearer-assertion"; +import { PlatformContext } from "../../../src/lib/context"; +import { base64UrlEncodeString, signAssertion } from "./assertion-helper"; + +const SECRET = "test-secret"; + +function platformWithSecret(secret: string | undefined): PlatformContext { + return { + getDatabase: () => { + throw new Error("not implemented"); + }, + getCache: () => { + throw new Error("not implemented"); + }, + getEnv: (key: string) => + key === "ASSERTION_SIGNING_SECRET" ? secret : undefined, + raw: undefined as unknown as PlatformContext["raw"] + }; +} + +describe("bearerAssertionVerifier", () => { + it("accepts a validly signed, non-expired assertion", async () => { + const token = await signAssertion(SECRET, { sub: "user-42" }); + const principal = await bearerAssertionVerifier.verify( + token, + platformWithSecret(SECRET) + ); + + expect(principal).toEqual({ userId: "user-42", scope: "assertion" }); + }); + + it("rejects an expired assertion", async () => { + const now = Math.floor(Date.now() / 1000); + const token = await signAssertion(SECRET, { + iat: now - 120, + exp: now - 60 + }); + + const principal = await bearerAssertionVerifier.verify( + token, + platformWithSecret(SECRET) + ); + + expect(principal).toBeNull(); + }); + + it("rejects a token signed with the wrong secret", async () => { + const token = await signAssertion("wrong-secret"); + const principal = await bearerAssertionVerifier.verify( + token, + platformWithSecret(SECRET) + ); + + expect(principal).toBeNull(); + }); + + it("rejects a malformed token", async () => { + const principal = await bearerAssertionVerifier.verify( + "not-a-jwt", + platformWithSecret(SECRET) + ); + + expect(principal).toBeNull(); + }); + + it("rejects a token with a tampered payload", async () => { + const token = await signAssertion(SECRET, { sub: "user-1" }); + const [header, , signature] = token.split("."); + + const now = Math.floor(Date.now() / 1000); + const tamperedPayload = base64UrlEncodeString( + JSON.stringify({ sub: "user-attacker", iat: now, exp: now + 60 }) + ); + const tampered = `${header}.${tamperedPayload}.${signature}`; + + const principal = await bearerAssertionVerifier.verify( + tampered, + platformWithSecret(SECRET) + ); + + expect(principal).toBeNull(); + }); + + it("rejects when ASSERTION_SIGNING_SECRET is not configured", async () => { + const token = await signAssertion(SECRET); + const principal = await bearerAssertionVerifier.verify( + token, + platformWithSecret(undefined) + ); + + expect(principal).toBeNull(); + }); +}); diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts new file mode 100644 index 0000000..2e2d5ab --- /dev/null +++ b/test/services/extensions/v2/index.test.ts @@ -0,0 +1,397 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { + createExecutionContext, + waitOnExecutionContext +} from "cloudflare:test"; +import { env } from "cloudflare:workers"; +import app from "../../../../src/app"; +import { createMockD1, createTables, MockTables } from "./mock-db"; +import { signAssertion } from "../../../lib/auth/assertion-helper"; + +// Matches the ASSERTION_SIGNING_SECRET binding configured in vitest.config.ts. +const SECRET = "test-assertion-signing-secret"; + +let tables: MockTables; + +beforeEach(() => { + tables = createTables(); + env.DB_EXTENSIONS = createMockD1(tables); +}); + +async function authHeaders(sub: string): Promise> { + const token = await signAssertion(SECRET, { sub }); + return { + Authorization: `Bearer ${token}`, + "Content-Type": "application/json" + }; +} + +function samplePayload(overrides?: { + extensionId?: string; + authorId?: string; +}) { + return { + author: { + id: overrides?.authorId ?? "new-author", + type: "user", + name: "Some Author", + url: "https://example.com" + }, + extension: { + id: overrides?.extensionId ?? "new-ext", + type: "mod", + name: "New Extension", + description: "A new extension", + releases: [ + { + tag: "1.0.0", + date: "2026-01-01T00:00:00Z", + download_url: "https://example.com/download.zip", + min_fossbilling_version: "0.6" + } + ], + website: "https://example.com", + license: { name: "MIT" }, + readme: "# Readme", + source: { type: "github", repo: "example/new-ext" }, + version: "1.0.0", + download_url: "https://example.com/download.zip" + } + }; +} + +function seedOwnedExtension(): void { + tables.authors.set("owner-author", { + id: "owner-author", + type: "user", + name: "Owner", + url: null, + owner_user_id: "owner-1" + }); + tables.extensions.set("existing-ext", { + id: "existing-ext", + type: "mod", + author_id: "owner-author", + name: "Existing", + description: "d", + releases: "[]", + website: "https://e.com", + license: '{"name":"MIT"}', + icon_url: null, + readme: "r", + source: '{"type":"github","repo":"example/existing"}', + version: "1.0.0", + download_url: "https://e.com/d.zip" + }); +} + +async function post( + path: string, + headers: Record, + body?: unknown +) { + const ctx = createExecutionContext(); + const res = await app.request( + path, + { + method: "POST", + headers, + body: body !== undefined ? JSON.stringify(body) : undefined + }, + env, + ctx + ); + await waitOnExecutionContext(ctx); + return res; +} + +async function get(path: string, headers: Record) { + const ctx = createExecutionContext(); + const res = await app.request(path, { headers }, env, ctx); + await waitOnExecutionContext(ctx); + return res; +} + +describe("Extensions API v2", () => { + describe("POST /submissions", () => { + it("requires auth", async () => { + const res = await post( + "/extensions/v2/submissions", + { + "Content-Type": "application/json" + }, + samplePayload() + ); + expect(res.status).toBe(401); + }); + + it("rejects an invalid payload", async () => { + const headers = await authHeaders("user-1"); + const res = await post("/extensions/v2/submissions", headers, { + author: {}, + extension: {} + }); + expect(res.status).toBe(422); + const data = (await res.json()) as { error: { code: string } }; + expect(data.error.code).toBe("VALIDATION_ERROR"); + }); + + it("creates a pending submission for a brand-new extension and author", async () => { + const headers = await authHeaders("user-1"); + const res = await post( + "/extensions/v2/submissions", + headers, + samplePayload() + ); + + expect(res.status).toBe(201); + const data = (await res.json()) as { + result: { id: string; status: string }; + }; + expect(data.result.status).toBe("pending"); + expect(tables.extension_submissions.size).toBe(1); + + const stored = [...tables.extension_submissions.values()][0]; + expect(stored.extension_id).toBeNull(); + expect(stored.submitted_by).toBe("user-1"); + }); + + it("rejects editing an extension not owned by the caller", async () => { + seedOwnedExtension(); + const headers = await authHeaders("intruder"); + + const res = await post( + "/extensions/v2/submissions", + headers, + samplePayload({ extensionId: "existing-ext", authorId: "owner-author" }) + ); + + expect(res.status).toBe(403); + expect(tables.extension_submissions.size).toBe(0); + }); + + it("allows editing an extension owned by the caller", async () => { + seedOwnedExtension(); + const headers = await authHeaders("owner-1"); + + const res = await post( + "/extensions/v2/submissions", + headers, + samplePayload({ extensionId: "existing-ext", authorId: "owner-author" }) + ); + + expect(res.status).toBe(201); + const stored = [...tables.extension_submissions.values()][0]; + expect(stored.extension_id).toBe("existing-ext"); + }); + + it("rejects claiming an author already owned by someone else", async () => { + seedOwnedExtension(); + const headers = await authHeaders("intruder"); + + const res = await post( + "/extensions/v2/submissions", + headers, + samplePayload({ + extensionId: "another-new-ext", + authorId: "owner-author" + }) + ); + + expect(res.status).toBe(403); + }); + }); + + describe("GET /submissions/mine", () => { + it("returns only the caller's own submissions", async () => { + await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + samplePayload({ extensionId: "ext-a", authorId: "author-a" }) + ); + await post( + "/extensions/v2/submissions", + await authHeaders("user-2"), + samplePayload({ extensionId: "ext-b", authorId: "author-b" }) + ); + + const res = await get( + "/extensions/v2/submissions/mine", + await authHeaders("user-1") + ); + expect(res.status).toBe(200); + const data = (await res.json()) as { + result: Array<{ submitted_by: string }>; + }; + expect(data.result).toHaveLength(1); + expect(data.result[0].submitted_by).toBe("user-1"); + }); + + it("requires auth", async () => { + const res = await get("/extensions/v2/submissions/mine", {}); + expect(res.status).toBe(401); + }); + }); + + describe("GET /submissions/queue", () => { + it("requires moderator access", async () => { + const res = await get( + "/extensions/v2/submissions/queue", + await authHeaders("user-1") + ); + expect(res.status).toBe(403); + }); + + it("returns pending submissions for a moderator", async () => { + tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + samplePayload() + ); + + const res = await get( + "/extensions/v2/submissions/queue", + await authHeaders("mod-1") + ); + expect(res.status).toBe(200); + const data = (await res.json()) as { result: Array<{ status: string }> }; + expect(data.result).toHaveLength(1); + expect(data.result[0].status).toBe("pending"); + }); + }); + + describe("approve / reject", () => { + it("approves a submission and it becomes visible via the v1 read path", async () => { + tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + + const created = await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + samplePayload() + ); + const { result } = (await created.json()) as { result: { id: string } }; + + const approved = await post( + `/extensions/v2/submissions/${result.id}/approve`, + await authHeaders("mod-1"), + {} + ); + expect(approved.status).toBe(200); + const approvedBody = (await approved.json()) as { + result: { status: string }; + }; + expect(approvedBody.result.status).toBe("approved"); + + const v1Res = await get("/extensions/v1/new-ext", {}); + expect(v1Res.status).toBe(200); + const v1Body = (await v1Res.json()) as { + result: { id: string; author: { id: string } }; + }; + expect(v1Body.result.id).toBe("new-ext"); + expect(v1Body.result.author.id).toBe("new-author"); + }); + + it("blocks non-moderators from approving", async () => { + const created = await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + samplePayload() + ); + const { result } = (await created.json()) as { result: { id: string } }; + + const res = await post( + `/extensions/v2/submissions/${result.id}/approve`, + await authHeaders("user-1"), + {} + ); + expect(res.status).toBe(403); + }); + + it("rejects approving a submission that is not pending", async () => { + tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + const created = await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + samplePayload() + ); + const { result } = (await created.json()) as { result: { id: string } }; + + await post( + `/extensions/v2/submissions/${result.id}/approve`, + await authHeaders("mod-1"), + {} + ); + const secondApprove = await post( + `/extensions/v2/submissions/${result.id}/approve`, + await authHeaders("mod-1"), + {} + ); + expect(secondApprove.status).toBe(409); + }); + + it("requires a review_note to reject", async () => { + tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + const created = await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + samplePayload() + ); + const { result } = (await created.json()) as { result: { id: string } }; + + const res = await post( + `/extensions/v2/submissions/${result.id}/reject`, + await authHeaders("mod-1"), + {} + ); + expect(res.status).toBe(422); + }); + + it("rejects a submission with a note", async () => { + tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + const created = await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + samplePayload() + ); + const { result } = (await created.json()) as { result: { id: string } }; + + const res = await post( + `/extensions/v2/submissions/${result.id}/reject`, + await authHeaders("mod-1"), + { review_note: "Needs a valid license URL" } + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { result: { status: string } }; + expect(body.result.status).toBe("rejected"); + expect(tables.extensions.size).toBe(0); + }); + }); + + describe("OpenAPI docs", () => { + it("serves a generated OpenAPI document", async () => { + const res = await get("/extensions/v2/openapi.json", {}); + expect(res.status).toBe(200); + const spec = (await res.json()) as { + openapi: string; + paths: Record; + }; + expect(spec.openapi).toBe("3.1.0"); + expect(Object.keys(spec.paths)).toEqual( + expect.arrayContaining([ + "/submissions", + "/submissions/mine", + "/submissions/queue", + "/submissions/{id}/approve", + "/submissions/{id}/reject" + ]) + ); + }); + + it("serves the Scalar API reference UI", async () => { + const res = await get("/extensions/v2/docs", {}); + expect(res.status).toBe(200); + expect(res.headers.get("Content-Type")).toContain("text/html"); + }); + }); +}); diff --git a/test/services/extensions/v2/mock-db.ts b/test/services/extensions/v2/mock-db.ts new file mode 100644 index 0000000..5fb7f63 --- /dev/null +++ b/test/services/extensions/v2/mock-db.ts @@ -0,0 +1,281 @@ +type Row = Record; + +export interface MockTables { + authors: Map; + extensions: Map; + extension_submissions: Map; + users: Map; +} + +export function createTables(): MockTables { + return { + authors: new Map(), + extensions: new Map(), + extension_submissions: new Map(), + users: new Map() + }; +} + +function ok>(results: T[] = []): D1Result { + return { + success: true, + results, + meta: { + duration: 0, + last_row_id: 0, + changes: results.length, + served_by: "mock", + size_after: 0, + rows_read: 0, + rows_written: 0, + changed_db: false + } + }; +} + +class MockStatement implements D1PreparedStatement { + private params: unknown[] = []; + + constructor( + private tables: MockTables, + private query: string + ) {} + + bind(...params: unknown[]): D1PreparedStatement { + this.params = params; + return this; + } + + raw: D1PreparedStatement["raw"] = (() => { + throw new Error("not implemented"); + }) as D1PreparedStatement["raw"]; + + private execute(): Row[] { + const q = this.query.replace(/\s+/g, " ").trim(); + const p = this.params; + + // v1's SELECT_EXTENSIONS join (database.ts), for cross-service verification + // that an approved v2 submission is visible via the v1 read path. + if (q.startsWith("SELECT e.id, e.type, e.author_id,")) { + let rows = [...this.tables.extensions.values()]; + if (q.includes("WHERE e.type = ?")) { + rows = rows.filter((r) => r.type === p[0]); + } else if (q.includes("WHERE LOWER(e.id) = LOWER(?)")) { + const id = String(p[0]).toLowerCase(); + rows = rows.filter((r) => String(r.id).toLowerCase() === id); + } + return rows.map((r) => { + const author = this.tables.authors.get(String(r.author_id)); + return { + id: r.id, + type: r.type, + author_id: r.author_id, + author_type: author?.type ?? "user", + author_name: author?.name ?? "", + author_url: author?.url ?? null, + name: r.name, + description: r.description, + releases: r.releases, + website: r.website, + license: r.license, + icon_url: r.icon_url, + readme: r.readme, + source: r.source, + version: r.version, + download_url: r.download_url + }; + }); + } + + if ( + q.startsWith( + "SELECT id, author_id FROM extensions WHERE LOWER(id) = LOWER(?)" + ) + ) { + const id = String(p[0]).toLowerCase(); + const row = [...this.tables.extensions.values()].find( + (r) => String(r.id).toLowerCase() === id + ); + return row ? [row] : []; + } + + if (q.startsWith("SELECT owner_user_id FROM authors WHERE id = ?")) { + const row = this.tables.authors.get(String(p[0])); + return row ? [row] : []; + } + + if (q.startsWith("SELECT is_moderator FROM users WHERE id = ?")) { + const row = this.tables.users.get(String(p[0])); + return row ? [row] : []; + } + + if ( + q.startsWith( + "INSERT INTO extension_submissions (id, extension_id, author_id, submitted_by, status, payload)" + ) + ) { + const [id, extension_id, author_id, submitted_by, payload] = p; + this.tables.extension_submissions.set(String(id), { + id, + extension_id, + author_id, + submitted_by, + status: "pending", + payload, + reviewer_id: null, + review_note: null, + created_at: new Date().toISOString(), + reviewed_at: null + }); + return []; + } + + if ( + q.startsWith("SELECT * FROM extension_submissions WHERE submitted_by = ?") + ) { + return [...this.tables.extension_submissions.values()] + .filter((r) => r.submitted_by === p[0]) + .sort((a, b) => + String(b.created_at).localeCompare(String(a.created_at)) + ); + } + + if (q.startsWith("SELECT * FROM extension_submissions WHERE status = ?")) { + return [...this.tables.extension_submissions.values()] + .filter((r) => r.status === p[0]) + .sort((a, b) => + String(a.created_at).localeCompare(String(b.created_at)) + ); + } + + if (q.startsWith("SELECT * FROM extension_submissions WHERE id = ?")) { + const row = this.tables.extension_submissions.get(String(p[0])); + return row ? [row] : []; + } + + 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)); + if (row) { + row.status = "rejected"; + row.reviewer_id = reviewer_id; + row.review_note = review_note; + row.reviewed_at = new Date().toISOString(); + } + return []; + } + + 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 = "approved"; + row.reviewer_id = reviewer_id; + row.review_note = review_note; + row.reviewed_at = new Date().toISOString(); + } + return []; + } + + if ( + q.startsWith("INSERT INTO authors (id, type, name, url, owner_user_id)") + ) { + const [id, type, name, url, owner_user_id] = p; + const existing = this.tables.authors.get(String(id)); + this.tables.authors.set(String(id), { + id, + type, + name, + url, + owner_user_id: existing ? existing.owner_user_id : owner_user_id + }); + return []; + } + + if ( + q.startsWith( + "INSERT INTO extensions (id, type, author_id, name, description, releases, website, license, icon_url, readme, source, version, download_url)" + ) + ) { + const [ + id, + type, + author_id, + name, + description, + releases, + website, + license, + icon_url, + readme, + source, + version, + download_url + ] = p; + this.tables.extensions.set(String(id), { + id, + type, + author_id, + name, + description, + releases, + website, + license, + icon_url, + readme, + source, + version, + download_url + }); + return []; + } + + throw new Error(`MockStatement: unhandled query: ${q}`); + } + + async all(): Promise> { + return ok(this.execute() as unknown as T[]); + } + + async first(column?: string): Promise { + const rows = this.execute(); + if (rows.length === 0) return null; + if (column) return (rows[0][column] as T) ?? null; + return rows[0] as unknown as T; + } + + async run>(): Promise> { + this.execute(); + return ok([]); + } +} + +export function createMockD1(tables: MockTables): D1Database { + return { + prepare(query: string): D1PreparedStatement { + return new MockStatement(tables, query); + }, + + dump(): Promise { + throw new Error("not implemented"); + }, + + async batch( + statements: D1PreparedStatement[] + ): Promise[]> { + const results: D1Result[] = []; + for (const stmt of statements) { + results.push(await stmt.run()); + } + return results; + }, + + exec(_query: string): Promise { + throw new Error("not implemented"); + }, + + withSession(_constraintOrBookmark?: string): D1DatabaseSession { + throw new Error("not implemented"); + } + }; +} diff --git a/vitest.config.ts b/vitest.config.ts index 44c67eb..5e23548 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -9,7 +9,8 @@ export default defineConfig({ // Add test environment variables bindings: { GITHUB_TOKEN: "test-github-token", - UPDATE_TOKEN: "test-update-token" + UPDATE_TOKEN: "test-update-token", + ASSERTION_SIGNING_SECRET: "test-assertion-signing-secret" } } }) diff --git a/wrangler.jsonc b/wrangler.jsonc index 8c73600..9fb548f 100644 --- a/wrangler.jsonc +++ b/wrangler.jsonc @@ -32,7 +32,8 @@ }, { "binding": "DB_EXTENSIONS", - "database_id": "b99e8d34-4856-4f92-837d-5413869d3d83" + "database_id": "b99e8d34-4856-4f92-837d-5413869d3d83", + "migrations_dir": "src/services/extensions/v2/db/migrations" } ], "kv_namespaces": [ From 94690b186278793dd429f8c99b49651b975a435c Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 24 Jul 2026 09:41:57 +0100 Subject: [PATCH 2/5] Address automated PR review findings Fixes several real correctness/security issues flagged by cubic-dev-ai on PR #149: - Concurrent moderators could race approve/reject; the status transition is now an atomic conditional UPDATE (WHERE status = 'pending'), so only one wins and the other gets a clean 409. - Editing an extension whose stored id differs only by case (legacy v1 data) inserted a duplicate row instead of updating the existing one; approvals now bind the resolved canonical id for edits. - A failed approval batch (or a failed reject UPDATE) could be reported as success; both now check per-statement/result success. - Non-finite exp/iat (e.g. 1e999 -> Infinity) passed the bearer assertion's payload guard, producing a token that never expires. - Extension/author ids had no charset restriction; author/website/ icon_url/download_url URLs accepted any scheme (javascript: etc). Restricted ids to a lowercase slug pattern and URLs to http(s). - author.URL (matching v1's Author.URL) was named `url` in the v2 schema, silently dropping the field for clients following the v1 convention. - DB exception text was returned directly to API callers; now logged server-side via the existing logger and replaced with a generic message across submissions-database.ts and users-database.ts. - UsersDatabase.isModerator() didn't distinguish a DB failure from "not a moderator", so a DB error surfaced as a bare exception instead of the API's structured error shape. - Bearer auth was case-sensitive on the "Bearer" scheme and didn't send WWW-Authenticate on 401s; both now follow RFC 6750. - OpenAPI doc gaps: missing `servers` entry for the /extensions/v2 mount path, missing 422 responses on two routes that can actually return one, and ErrorResponseSchema didn't include the `details` field the validation hook actually sends. - migrate:extensions-v2 silently defaulted to local D1 (wrangler's default when neither --local/--remote is passed); split into explicit :local/:remote script variants. - Documented ASSERTION_SIGNING_SECRET in AGENTS.md (.dev.vars is gitignored, so it wasn't visible to reviewers or new contributors). - assertion-helper.ts's default exp was computed from `now` rather than the effective `iat`, so overriding iat alone could produce an already-expired test token. Not changed, by design (see PR reply for reasoning): free-text name fields aren't charset-restricted (that would break legitimate names; output escaping belongs to the rendering consumer), legacy authors with no owner remain unclaimable via v2 this pass, and list-endpoint pagination is deferred as a feature addition rather than a bug fix. --- AGENTS.md | 6 + package.json | 6 +- src/lib/auth/bearer-assertion.ts | 4 +- src/lib/auth/middleware.ts | 10 +- .../v2/db/migrations/0001_add_v2_tables.sql | 4 + src/services/extensions/v2/errors.ts | 17 ++ src/services/extensions/v2/index.ts | 19 +- src/services/extensions/v2/interfaces.ts | 34 ++-- .../extensions/v2/submissions-database.ts | 186 ++++++++++-------- src/services/extensions/v2/users-database.ts | 19 +- test/lib/auth/assertion-helper.ts | 6 +- test/services/extensions/v2/index.test.ts | 49 ++++- test/services/extensions/v2/mock-db.ts | 16 +- 13 files changed, 261 insertions(+), 115 deletions(-) create mode 100644 src/services/extensions/v2/errors.ts diff --git a/AGENTS.md b/AGENTS.md index b66fa62..038afc9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,6 +43,12 @@ - Local secrets go in `.dev.vars` (for example `GITHUB_TOKEN="..."`). - Bindings for D1/KV are defined in `wrangler.jsonc`; keep names aligned with `CloudflareBindings`. - Use Wrangler secrets for production tokens instead of committing them. +- `ASSERTION_SIGNING_SECRET`: HMAC key the extensions v2 API uses to verify short-lived + bearer assertions minted by the extensions site (`src/lib/auth/bearer-assertion.ts`). + Not sent over the wire — only signs/verifies server-side in each Worker. Add + `ASSERTION_SIGNING_SECRET="..."` to `.dev.vars` for local dev; set via + `wrangler secret put ASSERTION_SIGNING_SECRET` in production, matching the value + configured in the extensions site's Worker. ## Stats API v1 diff --git a/package.json b/package.json index b79e8cc..3a37d91 100644 --- a/package.json +++ b/package.json @@ -9,8 +9,10 @@ "format": "prettier --write .", "format:check": "prettier --check .", "init:db": "npx tsx src/services/central-alerts/v1/scripts/init-db.ts", - "migrate:extensions-v2": "wrangler d1 migrations apply DB_EXTENSIONS", - "migrate:extensions-v2:list": "wrangler d1 migrations list DB_EXTENSIONS", + "migrate:extensions-v2:local": "wrangler d1 migrations apply DB_EXTENSIONS --local", + "migrate:extensions-v2:remote": "wrangler d1 migrations apply DB_EXTENSIONS --remote", + "migrate:extensions-v2:list:local": "wrangler d1 migrations list DB_EXTENSIONS --local", + "migrate:extensions-v2:list:remote": "wrangler d1 migrations list DB_EXTENSIONS --remote", "lint": "eslint", "lint:fix": "eslint --fix", "test": "vitest run", diff --git a/src/lib/auth/bearer-assertion.ts b/src/lib/auth/bearer-assertion.ts index a0b341b..9129c0a 100644 --- a/src/lib/auth/bearer-assertion.ts +++ b/src/lib/auth/bearer-assertion.ts @@ -29,7 +29,9 @@ function isAssertionPayload(value: unknown): value is AssertionPayload { typeof record.sub === "string" && record.sub.length > 0 && typeof record.iat === "number" && - typeof record.exp === "number" + Number.isFinite(record.iat) && + typeof record.exp === "number" && + Number.isFinite(record.exp) ); } diff --git a/src/lib/auth/middleware.ts b/src/lib/auth/middleware.ts index 9f65644..f924fa5 100644 --- a/src/lib/auth/middleware.ts +++ b/src/lib/auth/middleware.ts @@ -17,14 +17,13 @@ const verifiers: TokenVerifier[] = [bearerAssertionVerifier]; export function requireAuth(): MiddlewareHandler { return async (c, next) => { const header = c.req.header("Authorization"); - const token = header?.startsWith("Bearer ") - ? header.slice("Bearer ".length).trim() - : null; + const token = header?.match(/^Bearer\s+(.+)$/i)?.[1]?.trim() || null; if (!token) { return c.json( { error: { message: "Missing bearer token", code: "UNAUTHORIZED" } }, - 401 + 401, + { "WWW-Authenticate": "Bearer" } ); } @@ -40,7 +39,8 @@ export function requireAuth(): MiddlewareHandler { return c.json( { error: { message: "Invalid or expired token", code: "UNAUTHORIZED" } }, - 401 + 401, + { "WWW-Authenticate": "Bearer" } ); }; } diff --git a/src/services/extensions/v2/db/migrations/0001_add_v2_tables.sql b/src/services/extensions/v2/db/migrations/0001_add_v2_tables.sql index 9f20b24..ce5e507 100644 --- a/src/services/extensions/v2/db/migrations/0001_add_v2_tables.sql +++ b/src/services/extensions/v2/db/migrations/0001_add_v2_tables.sql @@ -9,6 +9,10 @@ -- database. If that schema changes, update fossbilling/api AND that file. Assumed -- columns used here: users.id (TEXT, = auth `sub` claim), users.is_moderator -- (INTEGER 0/1). +-- +-- Bootstrap order for a fresh database: v1's schema.sql (../../v1/db/schema.sql, +-- creates `authors`/`extensions`) and the extensions repo's `users` table must +-- both exist before this migration runs, since it ALTERs/references them. ALTER TABLE authors ADD COLUMN owner_user_id TEXT REFERENCES users(id); CREATE INDEX IF NOT EXISTS idx_authors_owner ON authors(owner_user_id); diff --git a/src/services/extensions/v2/errors.ts b/src/services/extensions/v2/errors.ts new file mode 100644 index 0000000..d3bba19 --- /dev/null +++ b/src/services/extensions/v2/errors.ts @@ -0,0 +1,17 @@ +import { DatabaseResult } from "../../../lib/interfaces"; +import { logError } from "../../../lib/logger"; + +// Logs the real error server-side and returns a generic message to the +// caller — DB exception text can leak schema/backend details otherwise. +export function databaseError( + context: string, + error: unknown +): DatabaseResult { + logError("extensions-v2", context, { + error: error instanceof Error ? error.message : String(error) + }); + return { + data: null, + error: { message: "A database error occurred", code: "DATABASE_ERROR" } + }; +} diff --git a/src/services/extensions/v2/index.ts b/src/services/extensions/v2/index.ts index 54af9b5..be95b7d 100644 --- a/src/services/extensions/v2/index.ts +++ b/src/services/extensions/v2/index.ts @@ -54,7 +54,13 @@ function requireModerator(): MiddlewareHandler { const platform = getPlatform(c); const users = new UsersDatabase(platform.getDatabase("DB_EXTENSIONS")); - const isModerator = await users.isModerator(auth.userId); + const { data: isModerator, error } = await users.isModerator(auth.userId); + if (error) { + return c.json( + { error: { message: error.message, code: error.code } }, + 500 + ); + } if (!isModerator) { return c.json( { error: { message: "Moderator access required", code: "FORBIDDEN" } }, @@ -225,6 +231,10 @@ const queueRoute = createRoute({ content: { "application/json": { schema: ErrorResponseSchema } }, description: "Caller is not a moderator" }, + 422: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "status query param failed validation" + }, 500: { content: { "application/json": { schema: ErrorResponseSchema } }, description: "Database error" @@ -296,6 +306,10 @@ const approveRoute = createRoute({ description: "Submission is not pending, or ownership has changed since it was submitted" }, + 422: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "id param or review_note body failed validation" + }, 500: { content: { "application/json": { schema: ErrorResponseSchema } }, description: "Database error" @@ -409,7 +423,8 @@ extensionsV2.doc("/openapi.json", { version: "2.0.0", description: "Self-service extension submission, ownership, and moderation. Read-only listings remain at /extensions/v1." - } + }, + servers: [{ url: "/extensions/v2" }] }); extensionsV2.get("/docs", Scalar({ url: "/extensions/v2/openapi.json" })); diff --git a/src/services/extensions/v2/interfaces.ts b/src/services/extensions/v2/interfaces.ts index 42412df..8a00ee1 100644 --- a/src/services/extensions/v2/interfaces.ts +++ b/src/services/extensions/v2/interfaces.ts @@ -10,12 +10,23 @@ export const EXTENSION_TYPES = [ "translation" ] as const; +// Lowercase alphanumeric slug (hyphens allowed, no leading/trailing hyphen) — +// matches the shape of existing ids (e.g. "fossbilling") and rules out +// anything that isn't safe to use as a URL path segment or DOM identifier. const lowercaseId = (label: string) => + z.string().regex(/^[a-z0-9]+(-[a-z0-9]+)*$/, { + message: `${label} id must be a lowercase alphanumeric slug` + }); + +// Restricts to http(s) — z.string().url() alone accepts any scheme, +// including javascript:/data:, which is unsafe for fields a consumer may +// render as a link or image src. +const httpUrl = () => z .string() - .min(1) - .refine((value) => value === value.toLowerCase(), { - message: `${label} id must be lowercase` + .url() + .refine((value) => /^https?:\/\//i.test(value), { + message: "must use http or https" }); export const AuthorSchema = z @@ -23,7 +34,7 @@ export const AuthorSchema = z id: lowercaseId("author"), type: z.enum(["user", "organization"]), name: z.string().min(1), - url: z.string().url().optional() + URL: httpUrl().optional() }) .openapi("Author"); @@ -31,8 +42,8 @@ export const ReleaseSchema = z .object({ tag: z.string().min(1), date: z.string().min(1), - download_url: z.string().url(), - changelog_url: z.string().url().optional(), + download_url: httpUrl(), + changelog_url: httpUrl().optional(), min_fossbilling_version: z.string().min(1) }) .openapi("Release"); @@ -47,7 +58,7 @@ export const RepositorySchema = z export const LicenseSchema = z .object({ name: z.string().min(1), - URL: z.string().url().optional() + URL: httpUrl().optional() }) .openapi("License"); @@ -58,13 +69,13 @@ export const ExtensionPayloadSchema = z name: z.string().min(1), description: z.string().min(1), releases: z.array(ReleaseSchema).min(1), - website: z.string().url(), + website: httpUrl(), license: LicenseSchema, - icon_url: z.string().url().optional(), + icon_url: httpUrl().optional(), readme: z.string().min(1), source: RepositorySchema, version: z.string().min(1), - download_url: z.string().url() + download_url: httpUrl() }) .openapi("ExtensionPayload"); @@ -118,7 +129,8 @@ export const ErrorResponseSchema = z .object({ error: z.object({ message: z.string(), - code: z.string() + code: z.string(), + details: z.array(z.unknown()).optional() }) }) .openapi("Error"); diff --git a/src/services/extensions/v2/submissions-database.ts b/src/services/extensions/v2/submissions-database.ts index 58b5899..0c8f442 100644 --- a/src/services/extensions/v2/submissions-database.ts +++ b/src/services/extensions/v2/submissions-database.ts @@ -1,4 +1,5 @@ import { DatabaseResult, IDatabase } from "../../../lib/interfaces"; +import { databaseError } from "./errors"; import { Submission, SubmissionPayload, SubmissionStatus } from "./interfaces"; interface OwnershipResolution { @@ -13,16 +14,6 @@ interface CreateInput { payload: SubmissionPayload; } -function databaseError(error: unknown): DatabaseResult { - return { - data: null, - error: { - message: error instanceof Error ? error.message : String(error), - code: "DATABASE_ERROR" - } - }; -} - function parseSubmissionRow(row: Record): Submission { return { id: row.id as string, @@ -98,15 +89,16 @@ export class SubmissionsDatabase { error: null }; } catch (error) { - return databaseError(error); + return databaseError("resolveOwnership", error); } } async create(input: CreateInput): Promise> { const id = crypto.randomUUID(); + let result; try { - const result = await this.db + result = await this.db .prepare( `INSERT INTO extension_submissions (id, extension_id, author_id, submitted_by, status, payload) VALUES (?, ?, ?, ?, 'pending', ?)` @@ -119,18 +111,15 @@ export class SubmissionsDatabase { JSON.stringify(input.payload) ) .run(); - - if (!result.success) { - return { - data: null, - error: { - message: result.error || "Database query failed", - code: "DATABASE_ERROR" - } - }; - } } catch (error) { - return databaseError(error); + return databaseError("create", error); + } + + if (!result.success) { + return databaseError( + "create", + new Error(result.error || "Database query failed") + ); } return { data: { id }, error: null }; @@ -146,17 +135,14 @@ export class SubmissionsDatabase { .bind(userId) .all>(); } catch (error) { - return databaseError(error); + return databaseError("listBySubmitter", error); } if (!result.success) { - return { - data: null, - error: { - message: result.error || "Database query failed", - code: "DATABASE_ERROR" - } - }; + return databaseError( + "listBySubmitter", + new Error(result.error || "Database query failed") + ); } return { @@ -177,17 +163,14 @@ export class SubmissionsDatabase { .bind(status) .all>(); } catch (error) { - return databaseError(error); + return databaseError("listQueue", error); } if (!result.success) { - return { - data: null, - error: { - message: result.error || "Database query failed", - code: "DATABASE_ERROR" - } - }; + return databaseError( + "listQueue", + new Error(result.error || "Database query failed") + ); } return { @@ -204,7 +187,7 @@ export class SubmissionsDatabase { .bind(id) .first>(); } catch (error) { - return databaseError(error); + return databaseError("getById", error); } if (!row) { @@ -220,11 +203,11 @@ export class SubmissionsDatabase { return { data: parseSubmissionRow(row), error: null }; } - async reject( - id: string, - reviewerId: string, - reviewNote: string - ): Promise> { + // 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( + id: string + ): Promise> { const existing = await this.getById(id); if (existing.error || !existing.data) { return { @@ -235,33 +218,47 @@ export class SubmissionsDatabase { } }; } + return { + data: null, + error: { message: "Submission is not pending", code: "CONFLICT" } + }; + } - if (existing.data.status !== "pending") { - return { - data: null, - error: { message: "Submission is not pending", code: "CONFLICT" } - }; - } - + // The `AND status = 'pending'` guard makes this a single atomic + // check-and-set: if two moderators race, only one's update affects a row. + async reject( + id: string, + reviewerId: string, + reviewNote: string + ): Promise> { + let result; try { - await this.db + result = await this.db .prepare( `UPDATE extension_submissions SET status = 'rejected', reviewer_id = ?, review_note = ?, reviewed_at = CURRENT_TIMESTAMP - WHERE id = ?` + WHERE id = ? AND status = 'pending'` ) .bind(reviewerId, reviewNote, id) .run(); } catch (error) { - return databaseError(error); + return databaseError("reject", error); + } + + if (!result.success) { + return databaseError( + "reject", + new Error(result.error || "Database query failed") + ); + } + + if (!result.meta?.changes) { + return this.explainNoOpTransition(id); } return { data: { id, status: "rejected" }, error: null }; } - // Re-checks ownership/availability (may have shifted since submission), - // then upserts through to authors/extensions in one atomic batch — the - // upsert form means create vs. edit needs no branching in the SQL. async approve( id: string, reviewerId: string, @@ -314,18 +311,48 @@ export class SubmissionsDatabase { }; } - const { author, extension } = submission.payload; + // 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 authors/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) { - return { - data: null, - error: { - message: "Database adapter does not support batch operations", - code: "DATABASE_ERROR" - } - }; + return databaseError( + "approve", + new Error("Database adapter does not support batch operations") + ); } + const { author, 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; + + let results; try { const authorStmt = this.db .prepare( @@ -338,7 +365,7 @@ export class SubmissionsDatabase { author.id, author.type, author.name, - author.url ?? null, + author.URL ?? null, submission.submitted_by ); @@ -355,7 +382,7 @@ export class SubmissionsDatabase { download_url = excluded.download_url` ) .bind( - extension.id, + extensionId, extension.type, author.id, extension.name, @@ -370,17 +397,20 @@ export class SubmissionsDatabase { extension.download_url ); - const submissionStmt = this.db - .prepare( - `UPDATE extension_submissions - SET status = 'approved', reviewer_id = ?, review_note = ?, reviewed_at = CURRENT_TIMESTAMP - WHERE id = ?` - ) - .bind(reviewerId, reviewNote ?? null, id); - - await this.db.batch([authorStmt, extensionStmt, submissionStmt]); + results = (await this.db.batch([authorStmt, extensionStmt])) as Array<{ + success: boolean; + error?: string; + }>; } catch (error) { - return databaseError(error); + return databaseError("approve", error); + } + + const failed = results.find((r) => !r.success); + if (failed) { + return databaseError( + "approve", + new Error(failed.error || "Database write failed") + ); } return { data: { id, status: "approved" }, error: null }; diff --git a/src/services/extensions/v2/users-database.ts b/src/services/extensions/v2/users-database.ts index c3fac4c..5381884 100644 --- a/src/services/extensions/v2/users-database.ts +++ b/src/services/extensions/v2/users-database.ts @@ -1,4 +1,5 @@ -import { IDatabase } from "../../../lib/interfaces"; +import { DatabaseResult, IDatabase } from "../../../lib/interfaces"; +import { databaseError } from "./errors"; // `users` is owned by the FOSSBilling/extensions repo (src/lib/db/users.sql there), // NOT this repo, but lives in the same DB_EXTENSIONS database. If that schema @@ -12,11 +13,15 @@ export class UsersDatabase { this.db = db; } - async isModerator(userId: string): Promise { - const row = await this.db - .prepare("SELECT is_moderator FROM users WHERE id = ?") - .bind(userId) - .first<{ is_moderator: number }>(); - return row?.is_moderator === 1; + async isModerator(userId: string): Promise> { + try { + const row = await this.db + .prepare("SELECT is_moderator FROM users WHERE id = ?") + .bind(userId) + .first<{ is_moderator: number }>(); + return { data: row?.is_moderator === 1, error: null }; + } catch (error) { + return databaseError("isModerator", error); + } } } diff --git a/test/lib/auth/assertion-helper.ts b/test/lib/auth/assertion-helper.ts index 845d8df..522ea19 100644 --- a/test/lib/auth/assertion-helper.ts +++ b/test/lib/auth/assertion-helper.ts @@ -22,11 +22,11 @@ export async function signAssertion( secret: string, overrides: AssertionOverrides = {} ): Promise { - const now = Math.floor(Date.now() / 1000); + const iat = overrides.iat ?? Math.floor(Date.now() / 1000); const payload = { sub: overrides.sub ?? "user-1", - iat: overrides.iat ?? now, - exp: overrides.exp ?? now + 60 + iat, + exp: overrides.exp ?? iat + 60 }; const headerB64 = base64UrlEncodeString( diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index 2e2d5ab..3fbe62b 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -35,7 +35,7 @@ function samplePayload(overrides?: { id: overrides?.authorId ?? "new-author", type: "user", name: "Some Author", - url: "https://example.com" + URL: "https://example.com" }, extension: { id: overrides?.extensionId ?? "new-ext", @@ -328,6 +328,53 @@ describe("Extensions API v2", () => { {} ); expect(secondApprove.status).toBe(409); + // The second (raced) approve must not write through again. + expect(tables.extensions.size).toBe(1); + }); + + it("updates the existing row instead of duplicating it when an edit's id differs only by case", async () => { + tables.authors.set("owner-author", { + id: "owner-author", + type: "user", + name: "Owner", + url: null, + owner_user_id: "owner-1" + }); + // Legacy v1 data can have mixed-case ids; v2 submissions must be lowercase. + tables.extensions.set("Existing-Ext", { + id: "Existing-Ext", + type: "mod", + author_id: "owner-author", + name: "Existing", + description: "d", + releases: "[]", + website: "https://e.com", + license: '{"name":"MIT"}', + icon_url: null, + readme: "r", + source: '{"type":"github","repo":"example/existing"}', + version: "1.0.0", + download_url: "https://e.com/d.zip" + }); + tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); + + const created = await post( + "/extensions/v2/submissions", + await authHeaders("owner-1"), + samplePayload({ extensionId: "existing-ext", authorId: "owner-author" }) + ); + const { result } = (await created.json()) as { result: { id: string } }; + + const approved = await post( + `/extensions/v2/submissions/${result.id}/approve`, + await authHeaders("mod-1"), + {} + ); + expect(approved.status).toBe(200); + + expect(tables.extensions.size).toBe(1); + const stored = tables.extensions.get("Existing-Ext"); + expect(stored?.name).toBe("New Extension"); }); it("requires a review_note to reject", async () => { diff --git a/test/services/extensions/v2/mock-db.ts b/test/services/extensions/v2/mock-db.ts index 5fb7f63..42d89ec 100644 --- a/test/services/extensions/v2/mock-db.ts +++ b/test/services/extensions/v2/mock-db.ts @@ -16,14 +16,17 @@ export function createTables(): MockTables { }; } -function ok>(results: T[] = []): D1Result { +function ok>( + results: T[] = [], + changes = results.length +): D1Result { return { success: true, results, meta: { duration: 0, last_row_id: 0, - changes: results.length, + changes, served_by: "mock", size_after: 0, rows_read: 0, @@ -35,6 +38,7 @@ function ok>(results: T[] = []): D1Result { class MockStatement implements D1PreparedStatement { private params: unknown[] = []; + private changes = 0; constructor( private tables: MockTables, @@ -156,11 +160,12 @@ class MockStatement implements D1PreparedStatement { 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)); - if (row) { + if (row && row.status === "pending") { row.status = "rejected"; row.reviewer_id = reviewer_id; row.review_note = review_note; row.reviewed_at = new Date().toISOString(); + this.changes = 1; } return []; } @@ -168,11 +173,12 @@ 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) { + if (row && row.status === "pending") { row.status = "approved"; row.reviewer_id = reviewer_id; row.review_note = review_note; row.reviewed_at = new Date().toISOString(); + this.changes = 1; } return []; } @@ -246,7 +252,7 @@ class MockStatement implements D1PreparedStatement { async run>(): Promise> { this.execute(); - return ok([]); + return ok([], this.changes); } } From 3fe45288066fcaadb5f472acf3dcc66c98ae15a4 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 24 Jul 2026 09:58:53 +0100 Subject: [PATCH 3/5] Revert approval to pending on write-through failure; fix schema.sql path - submissions-database.ts: if the author/extension write-through fails after a successful atomic claim (approve()), best-effort revert the submission's status back to 'pending' instead of leaving it stuck 'approved' with no matching live data. Added a test that forces a write failure, confirms the revert, then confirms a retry succeeds cleanly. - 0001_add_v2_tables.sql: fixed the relative path to v1's schema.sql in the header comment (was two directories up, needed three from db/migrations/). Addresses the two new cubic-dev-ai findings from the re-review of 94690b1. --- .../v2/db/migrations/0001_add_v2_tables.sql | 4 +-- .../extensions/v2/submissions-database.ts | 23 +++++++++++++ test/services/extensions/v2/index.test.ts | 33 +++++++++++++++++++ test/services/extensions/v2/mock-db.ts | 17 ++++++++++ 4 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/services/extensions/v2/db/migrations/0001_add_v2_tables.sql b/src/services/extensions/v2/db/migrations/0001_add_v2_tables.sql index ce5e507..7178105 100644 --- a/src/services/extensions/v2/db/migrations/0001_add_v2_tables.sql +++ b/src/services/extensions/v2/db/migrations/0001_add_v2_tables.sql @@ -1,7 +1,7 @@ -- Migration number: 0001 2026-07-24T06:49:28.535Z -- -- v2: self-service submissions, ownership, moderation. --- Adds to the v1-owned `authors` table (../../v1/db/schema.sql) and creates a new +-- Adds to the v1-owned `authors` table (../../../v1/db/schema.sql) and creates a new -- v2-owned table. -- -- NOTE: `users` referenced below is owned by the FOSSBilling/extensions repo @@ -10,7 +10,7 @@ -- columns used here: users.id (TEXT, = auth `sub` claim), users.is_moderator -- (INTEGER 0/1). -- --- Bootstrap order for a fresh database: v1's schema.sql (../../v1/db/schema.sql, +-- Bootstrap order for a fresh database: v1's schema.sql (../../../v1/db/schema.sql, -- creates `authors`/`extensions`) and the extensions repo's `users` table must -- both exist before this migration runs, since it ALTERs/references them. diff --git a/src/services/extensions/v2/submissions-database.ts b/src/services/extensions/v2/submissions-database.ts index 0c8f442..b061f43 100644 --- a/src/services/extensions/v2/submissions-database.ts +++ b/src/services/extensions/v2/submissions-database.ts @@ -203,6 +203,26 @@ 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 author/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( @@ -340,6 +360,7 @@ export class SubmissionsDatabase { } if (!this.db.batch) { + await this.revertToPending(id); return databaseError( "approve", new Error("Database adapter does not support batch operations") @@ -402,11 +423,13 @@ export class SubmissionsDatabase { error?: string; }>; } 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") diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index 3fbe62b..9b4e0a0 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -262,6 +262,39 @@ describe("Extensions API v2", () => { }); describe("approve / reject", () => { + 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 }); + + const created = await post( + "/extensions/v2/submissions", + await authHeaders("user-1"), + samplePayload() + ); + const { result } = (await created.json()) as { result: { id: string } }; + + tables.forceExtensionWriteFailure = true; + const approved = await post( + `/extensions/v2/submissions/${result.id}/approve`, + await authHeaders("mod-1"), + {} + ); + expect(approved.status).toBe(500); + expect(tables.extensions.size).toBe(0); + + const stored = tables.extension_submissions.get(result.id); + expect(stored?.status).toBe("pending"); + + // Recovers cleanly once the underlying failure is gone. + tables.forceExtensionWriteFailure = false; + const retried = await post( + `/extensions/v2/submissions/${result.id}/approve`, + await authHeaders("mod-1"), + {} + ); + expect(retried.status).toBe(200); + expect(tables.extensions.size).toBe(1); + }); + it("approves a submission and it becomes visible via the v1 read path", async () => { tables.users.set("mod-1", { id: "mod-1", is_moderator: 1 }); diff --git a/test/services/extensions/v2/mock-db.ts b/test/services/extensions/v2/mock-db.ts index 42d89ec..b2f4832 100644 --- a/test/services/extensions/v2/mock-db.ts +++ b/test/services/extensions/v2/mock-db.ts @@ -5,6 +5,8 @@ export interface MockTables { extensions: Map; extension_submissions: Map; users: Map; + // Test-only seam for simulating a write-through failure during approve(). + forceExtensionWriteFailure?: boolean; } export function createTables(): MockTables { @@ -183,6 +185,18 @@ class MockStatement implements D1PreparedStatement { return []; } + if (q.startsWith("UPDATE extension_submissions SET status = 'pending'")) { + const [id] = p; + const row = this.tables.extension_submissions.get(String(id)); + if (row) { + row.status = "pending"; + row.reviewer_id = null; + row.review_note = null; + row.reviewed_at = null; + } + return []; + } + if ( q.startsWith("INSERT INTO authors (id, type, name, url, owner_user_id)") ) { @@ -203,6 +217,9 @@ class MockStatement implements D1PreparedStatement { "INSERT INTO extensions (id, type, author_id, name, description, releases, website, license, icon_url, readme, source, version, download_url)" ) ) { + if (this.tables.forceExtensionWriteFailure) { + throw new Error("simulated write-through failure"); + } const [ id, type, From 4e0b70c753b04d1a1807f8a53cf74e98a54555ef Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 24 Jul 2026 11:00:12 +0100 Subject: [PATCH 4/5] Regenerate Cloudflare worker type definitions --- worker-configuration.d.ts | 255 ++++++++++++++++++++++++++++++++++---- 1 file changed, 232 insertions(+), 23 deletions(-) diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index 4b8f0c4..9a0e109 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,11 +1,13 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types --env-interface=CloudflareBindings` (hash: 0005c28ca80b7c3953acb0d6707876e2) -// Runtime types generated with workerd@1.20260617.1 2026-04-21 nodejs_compat +// Generated by Wrangler by running `wrangler types --env-interface=CloudflareBindings` (hash: b6302fdeb71a78b1dfc437234b4bcc3b) +// Runtime types generated with workerd@1.20260714.1 2026-06-24 nodejs_compat interface __BaseEnv_CloudflareBindings { AUTH_KV: KVNamespace; CACHE_KV: KVNamespace; DB_CENTRAL_ALERTS: D1Database; DB_EXTENSIONS: D1Database; + GITHUB_TOKEN: string; + ASSERTION_SIGNING_SECRET: string; } declare namespace Cloudflare { interface GlobalProps { @@ -14,6 +16,12 @@ declare namespace Cloudflare { interface Env extends __BaseEnv_CloudflareBindings {} } interface CloudflareBindings extends __BaseEnv_CloudflareBindings {} +type StringifyValues> = { + [Binding in keyof EnvType]: EnvType[Binding] extends string ? EnvType[Binding] : string; +}; +declare namespace NodeJS { + interface ProcessEnv extends StringifyValues> {} +} // Begin runtime types /*! ***************************************************************************** @@ -438,7 +446,8 @@ interface ExecutionContext { readonly exports: Cloudflare.Exports; readonly props: Props; cache?: CacheContext; - tracing?: Tracing; + readonly access?: CloudflareAccessContext; + tracing: Tracing; } type ExportedHandlerFetchHandler = (request: Request>, env: Env, ctx: ExecutionContext) => Response | Promise; type ExportedHandlerConnectHandler = (socket: Socket, env: Env, ctx: ExecutionContext) => void | Promise; @@ -494,6 +503,10 @@ interface CachePurgeOptions { interface CacheContext { purge(options: CachePurgeOptions): Promise; } +interface CloudflareAccessContext { + readonly aud: string; + getIdentity(): Promise; +} declare abstract class ColoLocalActorNamespace { get(actorId: string): Fetcher; } @@ -523,11 +536,11 @@ declare abstract class DurableObjectNamespace; jurisdiction(jurisdiction: DurableObjectJurisdiction): DurableObjectNamespace; } -type DurableObjectJurisdiction = "eu" | "fedramp" | "fedramp-high"; +type DurableObjectJurisdiction = "eu" | "fedramp" | "fedramp-high" | "us"; interface DurableObjectNamespaceNewUniqueIdOptions { jurisdiction?: DurableObjectJurisdiction; } -type DurableObjectLocationHint = "wnam" | "enam" | "sam" | "weur" | "eeur" | "apac" | "oc" | "afr" | "me"; +type DurableObjectLocationHint = "wnam" | "enam" | "sam" | "weur" | "eeur" | "apac" | "apac-ne" | "apac-se" | "oc" | "afr" | "me"; type DurableObjectRoutingMode = "primary-only"; interface DurableObjectNamespaceGetDurableObjectOptions { locationHint?: DurableObjectLocationHint; @@ -623,6 +636,7 @@ interface DurableObjectFacets { get(name: string, getStartupOptions: () => FacetStartupOptions | Promise>): Fetcher; abort(name: string, reason: any): void; delete(name: string): void; + clone(src: string, dst: string): void; } interface FacetStartupOptions { id?: DurableObjectId | string; @@ -3298,6 +3312,28 @@ interface EventSourceEventSourceInit { withCredentials?: boolean; fetcher?: Fetcher; } +interface ExecOutput { + readonly stdout: ArrayBuffer; + readonly stderr: ArrayBuffer; + readonly exitCode: number; +} +interface ContainerExecOptions { + cwd?: string; + env?: Record; + user?: string; + stdin?: ReadableStream | "pipe"; + stdout?: "pipe" | "ignore"; + stderr?: "pipe" | "ignore" | "combined"; +} +interface ExecProcess { + readonly stdin: WritableStream | null; + readonly stdout: ReadableStream | null; + readonly stderr: ReadableStream | null; + readonly pid: number; + readonly exitCode: Promise; + output(): Promise; + kill(signal?: number): void; +} interface Container { get running(): boolean; start(options?: ContainerStartupOptions): void; @@ -3311,6 +3347,7 @@ interface Container { snapshotDirectory(options: ContainerDirectorySnapshotOptions): Promise; snapshotContainer(options: ContainerSnapshotOptions): Promise; interceptOutboundHttps(addr: string, binding: Fetcher): Promise; + exec(cmd: string[], options?: ContainerExecOptions): Promise; } interface ContainerDirectorySnapshot { id: string; @@ -3481,11 +3518,58 @@ declare abstract class Performance { } interface Tracing { enterSpan(name: string, callback: (span: Span, ...args: A) => T, ...args: A): T; + startActiveSpan(name: string, callback: (span: Span, ...args: A) => T, ...args: A): T; Span: typeof Span; } declare abstract class Span { get isTraced(): boolean; setAttribute(key: string, value?: (boolean | number | string)): void; + end(): void; +} +/** + * Represents the identity of a user authenticated via Cloudflare Access. + * This matches the result of calling /cdn-cgi/access/get-identity. + * + * The exact structure of the returned object depends on the identity provider + * configuration for the Access application. The fields below represent commonly + * available properties, but additional provider-specific fields may be present. + */ +interface CloudflareAccessIdentity extends Record { + /** The user's email address, if available from the identity provider. */ + email?: string; + /** The user's display name. */ + name?: string; + /** The user's unique identifier. */ + user_uuid?: string; + /** The Cloudflare account ID. */ + account_id?: string; + /** Login timestamp (Unix epoch seconds). */ + iat?: number; + /** The user's IP address at authentication time. */ + ip?: string; + /** Authentication methods used (e.g., "pwd"). */ + amr?: string[]; + /** Identity provider information. */ + idp?: { + id: string; + type: string; + }; + /** Geographic information about where the user authenticated. */ + geo?: { + country: string; + }; + /** Group memberships from the identity provider. */ + groups?: Array<{ + id: string; + name: string; + email?: string; + }>; + /** Device posture check results, keyed by check ID. */ + devicePosture?: Record; + /** True if the user connected via Cloudflare WARP. */ + is_warp?: boolean; + /** True if the user is authenticated via Cloudflare Gateway. */ + is_gateway?: boolean; } // ============================================================================ // Agent Memory @@ -11116,6 +11200,8 @@ interface RequestInitCfProperties extends Record { * (e.g. { '200-299': 86400, '404': 1, '500-599': 0 }) */ cacheTtlByStatus?: Record; + /** Controls how responses with a `Vary` header are cached for this request. */ + vary?: RequestInitCfPropertiesVary; /** * Explicit Cache-Control header value to set on the response stored in cache. * This gives full control over cache directives (e.g. 'public, max-age=3600, s-maxage=86400'). @@ -11153,6 +11239,17 @@ interface RequestInitCfProperties extends Record { cacheReserveMinimumFileSize?: number; scrapeShield?: boolean; apps?: boolean; + /** + * Controls whether an outbound gRPC-web subrequest from this Worker is + * converted to gRPC at the Cloudflare edge. + * + * - `"passthrough"`: forward the subrequest unchanged as gRPC-web (default). + * - `"convert"`: convert the gRPC-web subrequest to gRPC at the edge. + * + * Provides per-request control over the same edge conversion behavior + * gated by the `auto_grpc_convert` compatibility flag. + */ + grpcWeb?: "passthrough" | "convert"; image?: RequestInitCfPropertiesImage; minify?: RequestInitCfPropertiesImageMinify; mirage?: boolean; @@ -11173,6 +11270,63 @@ interface RequestInitCfProperties extends Record { */ resolveOverride?: string; } +/** + * Controls how Workers Standard Vary handles a request header listed by an + * origin `Vary` response header: + * + * - `"normalize"`: normalize the request header value before it is used in the + * cache variance key. + * - `"passthrough"`: use the raw request header value in the cache variance + * key. + * - `"bypass"`: bypass cache when the header appears in the origin `Vary` + * response header. + */ +type RequestInitCfPropertiesVaryAction = "normalize" | "passthrough" | "bypass"; +/** Configuration for Workers Standard Vary support. */ +interface RequestInitCfPropertiesVary { + /** The fallback action for varied request headers not listed in `headers`. */ + default: RequestInitCfPropertiesVaryHeader; + /** + * Lowercase request header names and their Vary configuration. + * + * The `accept` header can include `media_types`, the `accept-language` + * header can include `languages`, and other headers support only `action`. + */ + headers?: RequestInitCfPropertiesVaryHeaders; +} +/** Common Vary behavior for a single request header. */ +interface RequestInitCfPropertiesVaryHeader { + /** How this request header contributes to cache variance. */ + action: RequestInitCfPropertiesVaryAction; +} +/** Vary behavior for the `accept` request header. */ +interface RequestInitCfPropertiesVaryAcceptHeader extends RequestInitCfPropertiesVaryHeader { + /** + * Media types to keep when normalizing the `Accept` request header. + * + * Named `media_types` to match the serialized `cf.vary` configuration. + */ + media_types?: string[]; +} +/** Vary behavior for the `accept-language` request header. */ +interface RequestInitCfPropertiesVaryAcceptLanguageHeader extends RequestInitCfPropertiesVaryHeader { + /** + * Language tags to keep when normalizing the `Accept-Language` request + * header. + */ + languages?: string[]; +} +/** + * Lowercase request header names and their Vary behavior. + * + * The index signature allows arbitrary custom request headers beyond the + * well-known `accept` and `accept-language` specializations. + */ +interface RequestInitCfPropertiesVaryHeaders { + accept?: RequestInitCfPropertiesVaryAcceptHeader; + "accept-language"?: RequestInitCfPropertiesVaryAcceptLanguageHeader; + [header: string]: RequestInitCfPropertiesVaryHeader | RequestInitCfPropertiesVaryAcceptHeader | RequestInitCfPropertiesVaryAcceptLanguageHeader | undefined; +} interface BasicImageTransformations { /** * Maximum width in image pixels. The value must be an integer. @@ -12077,6 +12231,13 @@ interface ForwardableEmailMessage extends EmailMessage { * @returns A promise that resolves when the email message is replied. */ reply(message: EmailMessage): Promise; + /** + * Reply to the sender of this email message with a message built from the given + * fields. Threading headers (In-Reply-To/References) are set automatically. + * @param builder The reply message contents. + * @returns A promise that resolves when the email message is replied. + */ + reply(builder: EmailReplyMessageBuilder): Promise; } /** A file attachment for an email message */ type EmailAttachment = { @@ -12097,23 +12258,46 @@ interface EmailAddress { name: string; email: string; } +/** + * Recipient fields for `SendEmail.send()`. At least one of `to`, `cc`, or + * `bcc` must be provided. + */ +type EmailDestinations = { + to?: string | EmailAddress | (string | EmailAddress)[]; + cc?: string | EmailAddress | (string | EmailAddress)[]; + bcc?: string | EmailAddress | (string | EmailAddress)[]; +} & ({ + to: string | EmailAddress | (string | EmailAddress)[]; +} | { + cc: string | EmailAddress | (string | EmailAddress)[]; +} | { + bcc: string | EmailAddress | (string | EmailAddress)[]; +}); +/** + * Fields shared by all composed emails (no recipients). Used directly by + * `ForwardableEmailMessage.reply()`, which always replies to the original + * sender, and extended by `EmailMessageBuilder` for `SendEmail.send()`. + */ +interface EmailReplyMessageBuilder { + from: string | EmailAddress; + subject: string; + replyTo?: string | EmailAddress; + headers?: Record; + text?: string; + html?: string; + attachments?: EmailAttachment[]; +} +/** + * Fields for composing an email without constructing raw MIME, for + * `SendEmail.send()`. Requires at least one of `to`, `cc`, or `bcc`. + */ +type EmailMessageBuilder = EmailReplyMessageBuilder & EmailDestinations; /** * A binding that allows a Worker to send email messages. */ interface SendEmail { send(message: EmailMessage): Promise; - send(builder: { - from: string | EmailAddress; - to: string | EmailAddress | (string | EmailAddress)[]; - subject: string; - replyTo?: string | EmailAddress; - cc?: string | EmailAddress | (string | EmailAddress)[]; - bcc?: string | EmailAddress | (string | EmailAddress)[]; - headers?: Record; - text?: string; - html?: string; - attachments?: EmailAttachment[]; - }): Promise; + send(builder: EmailMessageBuilder): Promise; } declare abstract class EmailEvent extends ExtendableEvent { readonly message: ForwardableEmailMessage; @@ -12935,6 +13119,11 @@ declare namespace CloudflareWorkersModule { export type WorkflowDurationLabel = 'second' | 'minute' | 'hour' | 'day' | 'week' | 'month' | 'year'; export type WorkflowSleepDuration = `${number} ${WorkflowDurationLabel}${'s' | ''}` | number; export type WorkflowDelayDuration = WorkflowSleepDuration; + export type WorkflowDynamicDelayContext = { + ctx: WorkflowStepContext; + error: Error; + }; + export type WorkflowDelayFunction = (input: WorkflowDynamicDelayContext) => WorkflowDelayDuration | Promise; export type WorkflowTimeoutDuration = WorkflowSleepDuration; export type WorkflowRetentionDuration = WorkflowSleepDuration; export type WorkflowBackoff = 'constant' | 'linear' | 'exponential'; @@ -12942,7 +13131,7 @@ declare namespace CloudflareWorkersModule { export type WorkflowStepConfig = { retries?: { limit: number; - delay: WorkflowDelayDuration | number; + delay: WorkflowDelayDuration | number | WorkflowDelayFunction; backoff?: WorkflowBackoff; }; timeout?: WorkflowTimeoutDuration | number; @@ -12968,13 +13157,22 @@ declare namespace CloudflareWorkersModule { type: string; sensitive?: WorkflowStepSensitivity; }; - export type WorkflowStepContext = { + export type WorkflowStepContext = { step: { name: string; count: number; }; attempt: number; - config: WorkflowStepConfig; + config: { + retries?: { + limit: number; + backoff?: WorkflowBackoff; + } & (Delay extends WorkflowDelayFunction ? {} : { + delay: WorkflowDelayDuration | number; + }); + timeout?: WorkflowTimeoutDuration | number; + sensitive?: WorkflowStepSensitivity; + }; }; export type WorkflowRollbackContext = { ctx: WorkflowStepContext; @@ -12990,7 +13188,9 @@ declare namespace CloudflareWorkersModule { }; export abstract class WorkflowStep { do>(name: string, callback: (ctx: WorkflowStepContext) => Promise, rollbackOptions?: WorkflowStepRollbackOptions): Promise; - do>(name: string, config: WorkflowStepConfig, callback: (ctx: WorkflowStepContext) => Promise, rollbackOptions?: WorkflowStepRollbackOptions): Promise; + do, const C extends WorkflowStepConfig>(name: string, config: C, callback: (ctx: WorkflowStepContext) => Promise, rollbackOptions?: WorkflowStepRollbackOptions): Promise; sleep: (name: string, duration: WorkflowSleepDuration) => Promise; sleepUntil: (name: string, timestamp: Date | number) => Promise; waitForEvent>(name: string, options: { @@ -13881,7 +14081,7 @@ declare namespace TailStream { interface ConnectEventInfo { readonly type: "connect"; } - type EventOutcome = "ok" | "canceled" | "exception" | "unknown" | "killSwitch" | "daemonDown" | "exceededCpu" | "exceededMemory" | "loadShed" | "responseStreamDisconnected" | "scriptNotFound" | "internalError"; + type EventOutcome = "ok" | "canceled" | "exception" | "unknown" | "killSwitch" | "daemonDown" | "exceededCpu" | "exceededMemory" | "loadShed" | "responseStreamDisconnected" | "scriptNotFound" | "internalError" | "exceededWallTime"; interface ScriptVersion { readonly id: string; readonly tag?: string; @@ -13900,6 +14100,7 @@ declare namespace TailStream { readonly dispatchNamespace?: string; readonly entrypoint?: string; readonly executionModel: string; + readonly durableObjectId?: string; readonly scriptName?: string; readonly scriptTags?: string[]; readonly scriptVersion?: ScriptVersion; @@ -14454,6 +14655,13 @@ interface WorkflowError { code?: number; message: string; } +interface WorkflowInstanceTerminateOptions { + /** + * If true, run registered rollback handlers before terminating the instance. + * Only steps that registered rollback handlers are rolled back. + */ + rollback?: boolean; +} interface WorkflowInstanceRestartOptions { /** * Restart from a specific step. If omitted, the instance restarts from the beginning. @@ -14487,8 +14695,9 @@ declare abstract class WorkflowInstance { public resume(): Promise; /** * Terminate the instance. If it is errored, terminated or complete, an error will be thrown. + * @param options Options for termination, including whether registered rollback handlers should run. */ - public terminate(): Promise; + public terminate(options?: WorkflowInstanceTerminateOptions): Promise; /** * Restart the instance. Optionally restart from a specific step, preserving * cached results for all steps before it. From 479a2e4fc15b63cd43c7560e37702cf2e715f76c Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 24 Jul 2026 11:16:38 +0100 Subject: [PATCH 5/5] Update Extensions v2 API docs titles --- src/services/extensions/v2/index.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/services/extensions/v2/index.ts b/src/services/extensions/v2/index.ts index be95b7d..ae85427 100644 --- a/src/services/extensions/v2/index.ts +++ b/src/services/extensions/v2/index.ts @@ -419,7 +419,7 @@ extensionsV2.openapi(rejectRoute, async (c) => { extensionsV2.doc("/openapi.json", { openapi: "3.1.0", info: { - title: "FOSSBilling Extensions API v2", + title: "FOSSBilling Extensions API (v2)", version: "2.0.0", description: "Self-service extension submission, ownership, and moderation. Read-only listings remain at /extensions/v1." @@ -427,6 +427,9 @@ extensionsV2.doc("/openapi.json", { servers: [{ url: "/extensions/v2" }] }); -extensionsV2.get("/docs", Scalar({ url: "/extensions/v2/openapi.json" })); +extensionsV2.get("/docs", Scalar({ + url: "/extensions/v2/openapi.json", + pageTitle: 'FOSSBilling Extensions API (v2)', +})); export default extensionsV2;