From 910dd9d2ee8ac2e672f3ec2f211ba5b199974f16 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sun, 4 Oct 2026 19:12:59 +0100 Subject: [PATCH 1/9] Harden identity sync assertions Introduce a dedicated `identity-sync` bearer assertion flow with a required signed `body_sha256` claim, and allow `requireAuth` to accept route-specific verifiers. The extensions identity sync middleware now validates exact request bytes against the signed digest, keeps these proofs scoped to the sync endpoint, and enforces a one-hour cap on stored GitHub org evidence freshness. Documentation and tests were expanded to cover the new auth model, tamper cases, raw-body integrity, secret rotation, and helper utilities. --- src/lib/auth/bearer-assertion.ts | 99 ++++---- src/lib/auth/interfaces.ts | 7 +- src/lib/auth/middleware.ts | 6 +- src/services/extensions/v2/README.md | 17 ++ src/services/extensions/v2/db/users.ts | 8 +- src/services/extensions/v2/middleware.ts | 58 +++-- src/services/extensions/v2/routes/account.ts | 6 +- test/lib/auth/assertion-helper.ts | 4 + test/lib/auth/bearer-assertion.test.ts | 83 ++++++- test/services/extensions/v2/account.test.ts | 231 +++++++++++++++---- test/services/extensions/v2/harness.ts | 25 ++ 11 files changed, 439 insertions(+), 105 deletions(-) diff --git a/src/lib/auth/bearer-assertion.ts b/src/lib/auth/bearer-assertion.ts index 52ce112a..65f333e0 100644 --- a/src/lib/auth/bearer-assertion.ts +++ b/src/lib/auth/bearer-assertion.ts @@ -23,7 +23,8 @@ interface AssertionPayload { exp: number; iss: typeof ASSERTION_ISSUER; aud: typeof ASSERTION_AUDIENCE; - purpose: typeof ASSERTION_PURPOSE; + purpose: typeof ASSERTION_PURPOSE | "identity-sync"; + body_sha256?: string; ver: typeof ASSERTION_VERSION; } @@ -31,7 +32,10 @@ function isInteger(value: unknown): value is number { return typeof value === "number" && Number.isInteger(value); } -function isAssertionPayload(value: unknown): value is AssertionPayload { +function isAssertionPayload( + value: unknown, + purpose: AssertionPayload["purpose"] +): value is AssertionPayload { if (typeof value !== "object" || value === null) return false; const record = value as Record; return ( @@ -41,7 +45,10 @@ function isAssertionPayload(value: unknown): value is AssertionPayload { isInteger(record.exp) && record.iss === ASSERTION_ISSUER && record.aud === ASSERTION_AUDIENCE && - record.purpose === ASSERTION_PURPOSE && + record.purpose === purpose && + (purpose !== "identity-sync" || + (typeof record.body_sha256 === "string" && + /^[a-f0-9]{64}$/.test(record.body_sha256))) && record.ver === ASSERTION_VERSION ); } @@ -77,44 +84,58 @@ function importedKeyFor(secret: string): Promise { // (header.payload.signature). Hono performs JWT parsing and signature // verification with the algorithm pinned by ASSERTION_VERIFY_OPTIONS; the // checks below are specific to this assertion profile. -export const bearerAssertionVerifier: TokenVerifier = { - async verify(token, platform): Promise { - const secrets = [ - platform.getEnv("ASSERTION_SIGNING_SECRET"), - platform.getEnv("ASSERTION_SIGNING_SECRET_PREVIOUS") - ].filter((secret): secret is string => Boolean(secret)); - if (secrets.length === 0) return null; +function assertionVerifier( + purpose: AssertionPayload["purpose"] +): TokenVerifier { + return { + async verify(token, platform): Promise { + const secrets = [ + platform.getEnv("ASSERTION_SIGNING_SECRET"), + platform.getEnv("ASSERTION_SIGNING_SECRET_PREVIOUS") + ].filter((secret): secret is string => Boolean(secret)); + if (secrets.length === 0) return null; - for (const secret of secrets) { - let payload: unknown; - try { - payload = await verifyJwt( - token, - await importedKeyFor(secret), - ASSERTION_VERIFY_OPTIONS - ); - } catch { - continue; - } - if (!isAssertionPayload(payload)) continue; + for (const secret of secrets) { + let payload: unknown; + try { + payload = await verifyJwt( + token, + await importedKeyFor(secret), + ASSERTION_VERIFY_OPTIONS + ); + } catch { + continue; + } + if (!isAssertionPayload(payload, purpose)) continue; - const now = Math.floor(Date.now() / 1000); - if (payload.iat > now + CLOCK_SKEW_SECONDS) continue; - if (payload.exp <= payload.iat) continue; - if (payload.exp - payload.iat > ASSERTION_TTL_SECONDS) continue; + const now = Math.floor(Date.now() / 1000); + if (payload.iat > now + CLOCK_SKEW_SECONDS) continue; + if (payload.exp <= payload.iat) continue; + if (payload.exp - payload.iat > ASSERTION_TTL_SECONDS) continue; - return { userId: payload.sub, scope: "assertion" }; - } + return purpose === "identity-sync" + ? { + userId: payload.sub, + scope: "identity_sync", + bodySha256: payload.body_sha256! + } + : { userId: payload.sub, scope: "assertion" }; + } - // A consistent failure across every configured secret is the only - // signal a misconfigured ASSERTION_SIGNING_SECRET produces. - const now = Date.now(); - if (now - lastAuthWarnAt >= WARN_INTERVAL_MS) { - lastAuthWarnAt = now; - logWarn("auth", "Bearer assertion failed verification", { - secretsTried: secrets.length - }); + // A consistent failure across every configured secret is the only + // signal a misconfigured ASSERTION_SIGNING_SECRET produces. + const now = Date.now(); + if (now - lastAuthWarnAt >= WARN_INTERVAL_MS) { + lastAuthWarnAt = now; + logWarn("auth", "Bearer assertion failed verification", { + secretsTried: secrets.length + }); + } + return null; } - return null; - } -}; + }; +} + +export const bearerAssertionVerifier = assertionVerifier(ASSERTION_PURPOSE); +// Service proofs are not registered in the general bearer verifier list. +export const identitySyncAssertionVerifier = assertionVerifier("identity-sync"); diff --git a/src/lib/auth/interfaces.ts b/src/lib/auth/interfaces.ts index 115b4afe..aaace9a0 100644 --- a/src/lib/auth/interfaces.ts +++ b/src/lib/auth/interfaces.ts @@ -1,9 +1,8 @@ import { PlatformContext } from "../context"; -export interface AuthPrincipal { - userId: string; - scope: "assertion" | "api_key"; -} +export type AuthPrincipal = + | { userId: string; scope: "assertion" | "api_key" } + | { userId: string; scope: "identity_sync"; bodySha256: string }; export interface TokenVerifier { verify( diff --git a/src/lib/auth/middleware.ts b/src/lib/auth/middleware.ts index 28681d48..0304c345 100644 --- a/src/lib/auth/middleware.ts +++ b/src/lib/auth/middleware.ts @@ -14,7 +14,9 @@ declare module "hono" { // and requireAuth() itself don't change. const verifiers: TokenVerifier[] = [bearerAssertionVerifier]; -export function requireAuth(): MiddlewareHandler { +export function requireAuth( + tokenVerifiers: readonly TokenVerifier[] = verifiers +): MiddlewareHandler { return async (c, next) => { const header = c.req.header("Authorization"); const token = header?.match(/^Bearer\s+(.+)$/i)?.[1]?.trim() || null; @@ -28,7 +30,7 @@ export function requireAuth(): MiddlewareHandler { } const platform = getPlatform(c); - for (const verifier of verifiers) { + for (const verifier of tokenVerifiers) { const principal = await verifier.verify(token, platform); if (principal) { c.set("auth", principal); diff --git a/src/services/extensions/v2/README.md b/src/services/extensions/v2/README.md index 9f51f876..6cedbc45 100644 --- a/src/services/extensions/v2/README.md +++ b/src/services/extensions/v2/README.md @@ -166,6 +166,23 @@ Requests carry a short-lived bearer assertion minted by the Extensions site and Assertions use HS256 and include the exact issuer `fossbilling-extensions`, audience `fossbilling-api/extensions-v2`, purpose `user-authentication`, and protocol version `1`. They are valid for at most 60 seconds. +`PUT /users/me/identity` instead requires purpose `identity-sync` with a +`body_sha256` claim containing the lowercase hexadecimal SHA-256 digest of the +exact UTF-8 JSON request bytes. Other claims and lifetime requirements stay the +same. The site must derive the projection from trusted provider data, serialize +once, hash those bytes, sign, and send the same bytes. Never mint identity proofs +for browser-supplied projections or expose these proofs to clients. +Ordinary user assertions and mismatched bodies receive 403 on this endpoint; +missing or invalid credentials receive 401. Identity proofs cannot authorize +other API routes. Schema validation remains 422 after proof verification. + +Deploy the site's new identity-sync signer with this API change; the previous +unsigned-body sync protocol is intentionally rejected. Membership expiry is +capped to one hour from synchronization and never extended past the supplied +expiry. Invalid or stale evidence remains unavailable for automatic verification. +Review historical identity projections and derived verification records if prior +abuse is suspected; this ingress fix does not attest to previously stored data. + ### Rotating the Shared Secret `ASSERTION_SIGNING_SECRET_PREVIOUS` is an optional second secret accepted only as a temporary rotation window. To rotate without interrupting requests: diff --git a/src/services/extensions/v2/db/users.ts b/src/services/extensions/v2/db/users.ts index 94c9e3a7..45d4490a 100644 --- a/src/services/extensions/v2/db/users.ts +++ b/src/services/extensions/v2/db/users.ts @@ -144,8 +144,14 @@ export class UsersDatabase { updatedAt: now, githubLogin: input.githubLogin, githubOrgs: hasFreshGithubOrgs ? JSON.stringify(input.githubOrgs) : null, + // Bound even a trusted service's membership snapshot to one hour. githubOrgsExpiresAt: hasFreshGithubOrgs - ? input.githubOrgsExpiresAt + ? new Date( + Math.min( + Date.parse(input.githubOrgsExpiresAt!), + Date.parse(now) + 60 * 60 * 1000 + ) + ).toISOString() : null, deletedAt: null }; diff --git a/src/services/extensions/v2/middleware.ts b/src/services/extensions/v2/middleware.ts index 82dc0192..3d9ebfa8 100644 --- a/src/services/extensions/v2/middleware.ts +++ b/src/services/extensions/v2/middleware.ts @@ -1,4 +1,8 @@ import { type Context, type MiddlewareHandler } from "hono"; +import { + bearerAssertionVerifier, + identitySyncAssertionVerifier +} from "../../../lib/auth/bearer-assertion"; import { getAuth, requireAuth } from "../../../lib/auth"; import type { AuthPrincipal } from "../../../lib/auth"; import { getExtensionsDb } from "../../../lib/db"; @@ -27,8 +31,10 @@ export function getOptionalAuth(c: Context): AuthPrincipal | null { type AuthenticatedCheck = (c: Context) => Promise; -function withAuthenticatedCheck(check: AuthenticatedCheck): MiddlewareHandler { - const authenticate = requireAuth(); +function withAuthenticatedCheck( + check: AuthenticatedCheck, + authenticate = requireAuth() +): MiddlewareHandler { return async (c, next) => { let response: Response | undefined; const authenticationResult = await authenticate(c, async () => { @@ -60,19 +66,43 @@ export function requireActiveAuth(): MiddlewareHandler { } export function requireIdentitySync(): MiddlewareHandler { - return withAuthenticatedCheck(async (c) => { - if (getAuth(c).scope !== "assertion") { - return c.json( - { - error: { - message: "Identity synchronization requires a trusted assertion", - code: "FORBIDDEN" - } - }, - 403 + return withAuthenticatedCheck( + async (c) => { + const auth = getAuth(c); + if (auth.scope !== "identity_sync") { + return c.json( + { + error: { + message: "Identity synchronization requires a trusted assertion", + code: "FORBIDDEN" + } + }, + 403 + ); + } + // Hash the exact bytes before JSON parsing. Hono caches this buffer so + // validation and persistence consume the same authenticated body. + const digest = await crypto.subtle.digest( + "SHA-256", + await c.req.arrayBuffer() ); - } - }); + const hex = Array.from(new Uint8Array(digest), (byte) => + byte.toString(16).padStart(2, "0") + ).join(""); + if (hex !== auth.bodySha256) { + return c.json( + { + error: { + message: "Identity payload does not match assertion", + code: "FORBIDDEN" + } + }, + 403 + ); + } + }, + requireAuth([identitySyncAssertionVerifier, bearerAssertionVerifier]) + ); } // Moderator routes list this alone, not behind requireActiveAuth(): it diff --git a/src/services/extensions/v2/routes/account.ts b/src/services/extensions/v2/routes/account.ts index 9c928c01..0ad9bc50 100644 --- a/src/services/extensions/v2/routes/account.ts +++ b/src/services/extensions/v2/routes/account.ts @@ -64,10 +64,8 @@ export function registerAccountRoutes(app: ExtensionsV2App): void { }); app.openapi(syncIdentityRoute, async (c) => { - // requireIdentitySync has already verified the HMAC assertion minted by - // the trusted Extensions site. The projection fields below therefore - // represent the site's OIDC callback, while authorization state remains - // API-owned and is never accepted from the request body. + // requireIdentitySync verified a dedicated service assertion and the + // digest of the exact body validated here. Authorization stays API-owned. const auth = getAuth(c); const body = c.req.valid("json"); const users = new UsersDatabase(getExtensionsDb(c.env.DB_EXTENSIONS)); diff --git a/test/lib/auth/assertion-helper.ts b/test/lib/auth/assertion-helper.ts index d39cca0f..3b19222c 100644 --- a/test/lib/auth/assertion-helper.ts +++ b/test/lib/auth/assertion-helper.ts @@ -13,6 +13,7 @@ export function base64UrlEncodeString(value: string): string { export interface AssertionOverrides { sub?: string; + bodySha256?: string; iat?: number; exp?: number; iss?: string; @@ -43,6 +44,9 @@ export async function signAssertion( }); } + if (overrides.bodySha256 !== undefined) + payload.body_sha256 = overrides.bodySha256; + const headerB64 = base64UrlEncodeString( JSON.stringify(overrides.header ?? { alg: "HS256", typ: "JWT" }) ); diff --git a/test/lib/auth/bearer-assertion.test.ts b/test/lib/auth/bearer-assertion.test.ts index 3aba88d5..958ae7ac 100644 --- a/test/lib/auth/bearer-assertion.test.ts +++ b/test/lib/auth/bearer-assertion.test.ts @@ -1,5 +1,8 @@ import { describe, it, expect } from "vitest"; -import { bearerAssertionVerifier } from "../../../src/lib/auth/bearer-assertion"; +import { + bearerAssertionVerifier, + identitySyncAssertionVerifier +} from "../../../src/lib/auth/bearer-assertion"; import { PlatformContext } from "../../../src/lib/context"; import { base64UrlEncodeString, signAssertion } from "./assertion-helper"; @@ -207,3 +210,81 @@ describe("bearerAssertionVerifier", () => { expect(principal).toBeNull(); }); }); + +describe("identitySyncAssertionVerifier", () => { + it("requires the dedicated purpose and a signed SHA-256 digest", async () => { + expect( + await identitySyncAssertionVerifier.verify( + await signAssertion(SECRET), + platformWithSecret(SECRET) + ) + ).toBeNull(); + const digest = "a".repeat(64); + const token = await signAssertion(SECRET, { + purpose: "identity-sync", + bodySha256: digest + }); + expect( + await identitySyncAssertionVerifier.verify( + token, + platformWithSecret(SECRET) + ) + ).toEqual({ userId: "user-1", scope: "identity_sync", bodySha256: digest }); + expect( + await bearerAssertionVerifier.verify(token, platformWithSecret(SECRET)) + ).toBeNull(); + }); + it.each([undefined, "", "A".repeat(64), "a".repeat(63), "z".repeat(64)])( + "rejects an invalid digest %s", + async (bodySha256) => { + const token = await signAssertion(SECRET, { + purpose: "identity-sync", + bodySha256 + }); + expect( + await identitySyncAssertionVerifier.verify( + token, + platformWithSecret(SECRET) + ) + ).toBeNull(); + } + ); + it.each([ + { iss: "wrong" }, + { aud: "wrong" }, + { ver: 2 }, + { exp: Math.floor(Date.now() / 1000) - 1 }, + { iat: Math.floor(Date.now() / 1000) + 10 }, + { exp: Math.floor(Date.now() / 1000) + 120 } + ])("rejects invalid identity proof metadata %j", async (overrides) => { + const token = await signAssertion(SECRET, { + purpose: "identity-sync", + bodySha256: "a".repeat(64), + ...overrides + }); + expect( + await identitySyncAssertionVerifier.verify( + token, + platformWithSecret(SECRET) + ) + ).toBeNull(); + }); + it("preserves secret rotation for service proofs", async () => { + const token = await signAssertion("previous", { + purpose: "identity-sync", + bodySha256: "a".repeat(64) + }); + expect( + await identitySyncAssertionVerifier.verify( + token, + platformWithSecret(SECRET, "previous") + ) + ).not.toBeNull(); + expect( + await identitySyncAssertionVerifier.verify( + token, + platformWithSecret(SECRET) + ) + ).toBeNull(); + }); +}); diff --git a/test/services/extensions/v2/account.test.ts b/test/services/extensions/v2/account.test.ts index cd1ef86e..04e9607c 100644 --- a/test/services/extensions/v2/account.test.ts +++ b/test/services/extensions/v2/account.test.ts @@ -1,8 +1,17 @@ +import app from "../../../../src/app"; +import { env } from "cloudflare:workers"; +import { + createExecutionContext, + waitOnExecutionContext +} from "cloudflare:test"; +import { signAssertion } from "../../../lib/auth/assertion-helper"; import { describe, it, expect, vi } from "vitest"; import { setupExtensionsV2Tests, db, authHeaders, + identityHeaders, + syncIdentity, get, put, patch, @@ -40,9 +49,163 @@ setupExtensionsV2Tests(); describe("Extensions API v2", () => { describe("API-owned account projection", () => { + const identity = { + name: "Trusted User", + email: "trusted@example.com", + email_verified: true, + picture: "https://example.com/trusted.png", + github_login: "trusted", + github_orgs: ["trusted-org"], + github_orgs_expires_at: "2099-01-01T00:00:00.000Z" + }; + + it("rejects ordinary assertions before storing attacker-selected identity", async () => { + const response = await put( + "/extensions/v2/users/me/identity", + await authHeaders("attacker"), + identity + ); + expect(response.status).toBe(403); + const row = await db + .prepare("SELECT github_login FROM users WHERE id = ?") + .bind("attacker") + .first(); + expect(row).toEqual({ github_login: null }); + }); + + it.each([ + ["name", "Forged"], + ["email", "forged@example.com"], + ["email_verified", false], + ["picture", null], + ["github_login", "victim"], + ["github_orgs", ["victim-org"]], + ["github_orgs_expires_at", "2099-02-01T00:00:00.000Z"] + ])("rejects tampering with signed %s", async (field, value) => { + const headers = await identityHeaders("new-identity", identity); + const response = await put("/extensions/v2/users/me/identity", headers, { + ...identity, + [field as string]: value + }); + expect(response.status).toBe(403); + expect( + await db + .prepare("SELECT id FROM users WHERE id = ?") + .bind("new-identity") + .first() + ).toBeNull(); + }); + + it.each([ + " " + JSON.stringify(identity), + JSON.stringify(identity).replace( + '"github_login":"trusted"', + '"github_login":"trusted","github_login":"victim"' + ), + JSON.stringify(identity).replace('"github_login"', '"github_\\u006cogin"') + ])( + "rejects changed JSON bytes, including aliases and duplicate fields", + async (rawBody) => { + const ctx = createExecutionContext(); + const res = await app.request( + "/extensions/v2/users/me/identity", + { + method: "PUT", + headers: await identityHeaders("raw-body", identity), + body: rawBody + }, + env, + ctx + ); + await waitOnExecutionContext(ctx); + expect(res.status).toBe(403); + } + ); + + it("accepts exact signed UTF-8 JSON bytes and preserves schema errors", async () => { + for (const [body, status] of [ + [{ ...identity, name: "Renée" }, 200], + [{ ...identity, extra: true }, 422] + ] as const) { + const rawBody = JSON.stringify(body, null, 2); + const digest = await crypto.subtle.digest( + "SHA-256", + new TextEncoder().encode(rawBody) + ); + const bodySha256 = Array.from(new Uint8Array(digest), (byte) => + byte.toString(16).padStart(2, "0") + ).join(""); + const token = await signAssertion("test-assertion-signing-secret", { + sub: "utf8-body", + purpose: "identity-sync", + bodySha256 + }); + const ctx = createExecutionContext(); + const res = await app.request( + "/extensions/v2/users/me/identity", + { + method: "PUT", + headers: { + Authorization: `Bearer ${token}`, + "Content-Type": "application/json" + }, + body: rawBody + }, + env, + ctx + ); + await waitOnExecutionContext(ctx); + expect(res.status).toBe(status); + } + }); + + it("keeps identity proofs out of user API authorization", async () => { + const response = await get( + "/extensions/v2/users/me", + await identityHeaders("service-sub", identity) + ); + expect(response.status).toBe(401); + }); + + it("creates a new account from signed identity and caps membership freshness", async () => { + const before = Date.now(); + const response = await syncIdentity("new-signed-user", identity); + expect(response.status).toBe(200); + const row = await db + .prepare( + "SELECT github_login, github_orgs_expires_at FROM users WHERE id = ?" + ) + .bind("new-signed-user") + .first<{ github_login: string; github_orgs_expires_at: string }>(); + expect(row?.github_login).toBe("trusted"); + expect(Date.parse(row!.github_orgs_expires_at)).toBeGreaterThanOrEqual( + before + 3600000 + ); + expect(Date.parse(row!.github_orgs_expires_at)).toBeLessThanOrEqual( + Date.now() + 3600000 + ); + }); + + it("does not extend a shorter signed membership expiry", async () => { + const expiry = new Date(Date.now() + 60000).toISOString(); + expect( + ( + await syncIdentity("short-evidence", { + ...identity, + github_orgs_expires_at: expiry + }) + ).status + ).toBe(200); + const row = await db + .prepare("SELECT github_orgs_expires_at FROM users WHERE id = ?") + .bind("short-evidence") + .first(); + expect(row).toEqual({ github_orgs_expires_at: expiry }); + }); + it("syncs identity, exposes owner state, and lists owned extensions", async () => { const headers = await authHeaders("account-1"); - const synced = await put("/extensions/v2/users/me/identity", headers, { + const synced = await syncIdentity("account-1", { name: "Account User", email: "account@example.com", email_verified: true, @@ -128,19 +291,15 @@ describe("Extensions API v2", () => { }); it("only reports GitHub as linked when both login and fresh evidence exist", async () => { - const res = await put( - "/extensions/v2/users/me/identity", - await authHeaders("github-evidence-without-login"), - { - name: "No Login", - email: "no-login@example.com", - email_verified: true, - picture: null, - github_login: null, - github_orgs: ["fossbilling"], - github_orgs_expires_at: "2099-01-01T00:00:00.000Z" - } - ); + const res = await syncIdentity("github-evidence-without-login", { + name: "No Login", + email: "no-login@example.com", + email_verified: true, + picture: null, + github_login: null, + github_orgs: ["fossbilling"], + github_orgs_expires_at: "2099-01-01T00:00:00.000Z" + }); expect(res.status).toBe(200); expect(await res.json()).toMatchObject({ result: { github_linked: false } @@ -154,19 +313,15 @@ describe("Extensions API v2", () => { ])( "does not treat %s as usable organization evidence", async (_description, github_orgs_expires_at) => { - const res = await put( - "/extensions/v2/users/me/identity", - await authHeaders("impossible-org-date"), - { - name: "Impossible Date", - email: "impossible-date@example.com", - email_verified: true, - picture: null, - github_login: "someone", - github_orgs: ["fossbilling"], - github_orgs_expires_at - } - ); + const res = await syncIdentity("impossible-org-date", { + name: "Impossible Date", + email: "impossible-date@example.com", + email_verified: true, + picture: null, + github_login: "someone", + github_orgs: ["fossbilling"], + github_orgs_expires_at + }); expect(res.status).toBe(200); expect(await res.json()).toMatchObject({ @@ -239,19 +394,15 @@ describe("Extensions API v2", () => { error: { code: "ACCOUNT_INACTIVE" } }); - const reactivated = await put( - "/extensions/v2/users/me/identity", - headers, - { - name: "Reactivated", - email: "reactivated@example.com", - email_verified: true, - picture: null, - github_login: null, - github_orgs: null, - github_orgs_expires_at: null - } - ); + const reactivated = await syncIdentity("delete-me", { + name: "Reactivated", + email: "reactivated@example.com", + email_verified: true, + picture: null, + github_login: null, + github_orgs: null, + github_orgs_expires_at: null + }); expect(reactivated.status).toBe(200); expect(await reactivated.json()).toMatchObject({ result: { active: true, display_name: null } diff --git a/test/services/extensions/v2/harness.ts b/test/services/extensions/v2/harness.ts index cd1dae25..9537010d 100644 --- a/test/services/extensions/v2/harness.ts +++ b/test/services/extensions/v2/harness.ts @@ -96,6 +96,31 @@ export async function authHeaders( }; } +export async function identityHeaders( + sub: string, + body: unknown +): Promise> { + const digest = await crypto.subtle.digest( + "SHA-256", + new TextEncoder().encode(JSON.stringify(body)) + ); + const bodySha256 = Array.from(new Uint8Array(digest), (byte) => + byte.toString(16).padStart(2, "0") + ).join(""); + return { + Authorization: `Bearer ${await signAssertion(SECRET, { sub, purpose: "identity-sync", bodySha256 })}`, + "Content-Type": "application/json" + }; +} + +export async function syncIdentity(sub: string, body: unknown) { + return put( + "/extensions/v2/users/me/identity", + await identityHeaders(sub, body), + body + ); +} + // The PUT /extensions/{id} body: content only, no id and no developer. Both // are now properties of the extension record rather than of the edit. export function sampleContent(overrides?: { name?: string }) { From e6aceb8c796f0143e3b43b65c985c8081944765b Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sun, 4 Oct 2026 19:25:15 +0100 Subject: [PATCH 2/9] Add durable claim verification budgets Introduce a D1-backed claim verification budget system for ownership claims, with atomic reservation across three limits: per-account, per-normalized-developer, and a global hourly cap. Claim creation now checks this budget before GitHub verification and returns RATE_LIMITED/429 when exhausted, while preserving existing duplicate-claim behavior. This adds migration 0024, schema/table wiring, and a new budget reservation helper, updates ownership route 429 docs and service README guidance (including deployment order), and expands tests to cover mismatches, cancellation, upstream failures, fail-closed writes, normalization sharing, aggregate limits, concurrency atomicity, expiry renewal, and fixture cleanup. --- src/services/extensions/v2/README.md | 9 + .../v2/db/claim-verification-budget.ts | 34 + .../extensions/v2/db/developer-claims.ts | 14 + .../0024_claim_verification_budgets.sql | 7 + .../v2/db/migrations/meta/0024_snapshot.json | 1085 +++++++++++++++++ .../v2/db/migrations/meta/_journal.json | 7 + src/services/extensions/v2/db/schema.ts | 13 + .../extensions/v2/routes/ownership.ts | 4 +- .../extensions/v2/claim-budget.test.ts | 167 +++ test/services/extensions/v2/db-fixtures.ts | 1 + 10 files changed, 1340 insertions(+), 1 deletion(-) create mode 100644 src/services/extensions/v2/db/claim-verification-budget.ts create mode 100644 src/services/extensions/v2/db/migrations/0024_claim_verification_budgets.sql create mode 100644 src/services/extensions/v2/db/migrations/meta/0024_snapshot.json create mode 100644 test/services/extensions/v2/claim-budget.test.ts diff --git a/src/services/extensions/v2/README.md b/src/services/extensions/v2/README.md index 6cedbc45..2c455a8c 100644 --- a/src/services/extensions/v2/README.md +++ b/src/services/extensions/v2/README.md @@ -257,3 +257,12 @@ Migration `0021` also drops any submission filed under a developer that no longe ## Code Layout See `AGENTS.md` for what belongs in `routes/`, `db/`, `schemas/`, `github/`, and `middleware.ts`. This service is the reference layout for larger services. + +Claim verification is limited before contacting GitHub: 3 attempts per account +and per normalized developer ID per 60 seconds, plus 300 aggregate claim +verification attempts per hour. D1 reserves all budgets atomically. Mismatches, +upstream failures, and cancellation do not refund attempts. Exhaustion returns +429 (`RATE_LIMITED`); pending duplicates still return 409 without spending quota. +Deploy migration `0024_claim_verification_budgets.sql` before the Worker update. +The aggregate budget covers claims only and leaves shared GitHub capacity for +other workflows; it is not a budget for every GitHub consumer. diff --git a/src/services/extensions/v2/db/claim-verification-budget.ts b/src/services/extensions/v2/db/claim-verification-budget.ts new file mode 100644 index 00000000..cd6c9a8b --- /dev/null +++ b/src/services/extensions/v2/db/claim-verification-budget.ts @@ -0,0 +1,34 @@ +import { sql } from "drizzle-orm"; +import { ExtensionsDb } from "../../../../lib/db"; + +// Claim attempts retain their budget regardless of verification outcome or +// claim cancellation. D1 serializes this single conditional write across +// isolates: either all three budgets are charged or none are. +export async function reserveClaimVerification( + db: ExtensionsDb, + claimantId: string, + developerId: string +): Promise { + await db.run(sql`DELETE FROM claim_verification_budgets + WHERE expires_at <= unixepoch()`); + const rows = await db.all<{ key: string }>(sql` + WITH requested(key, allowance, period) AS ( + VALUES (${`account:${claimantId}`}, 3, 60), + (${`developer:${developerId.toLowerCase()}`}, 3, 60), + ('global', 300, 3600) + ) + INSERT INTO claim_verification_budgets (key, attempts, expires_at) + SELECT key, 1, unixepoch() + period FROM requested + WHERE NOT EXISTS ( + SELECT 1 FROM requested r + JOIN claim_verification_budgets b ON b.key = r.key + WHERE b.expires_at > unixepoch() AND b.attempts >= r.allowance + ) + ON CONFLICT (key) DO UPDATE SET + attempts = CASE WHEN expires_at <= unixepoch() THEN 1 ELSE attempts + 1 END, + expires_at = CASE WHEN expires_at <= unixepoch() + THEN excluded.expires_at ELSE expires_at END + RETURNING key + `); + return rows.length === 3; +} diff --git a/src/services/extensions/v2/db/developer-claims.ts b/src/services/extensions/v2/db/developer-claims.ts index c4dce3f5..234a75f7 100644 --- a/src/services/extensions/v2/db/developer-claims.ts +++ b/src/services/extensions/v2/db/developer-claims.ts @@ -15,6 +15,8 @@ import { DeveloperClaim, PendingDeveloperClaim } from "../schemas/ownership"; import { DeveloperProfilesDatabase } from "./developer-profiles"; import { verifyGithubOwnership } from "../github/identity"; +import { reserveClaimVerification } from "./claim-verification-budget"; + type ClaimRow = typeof developerClaims.$inferSelect; function parseClaimRow(row: ClaimRow): DeveloperClaim { @@ -166,6 +168,18 @@ export class DeveloperClaimsDatabase { }; } + if ( + !(await reserveClaimVerification(this.db, claimantId, developerId)) + ) { + return { + data: null, + error: { + code: "RATE_LIMITED", + message: "Claim verification allowance exhausted; try again later" + } + }; + } + const check = await verifyGithubOwnership( this.db, developerId, diff --git a/src/services/extensions/v2/db/migrations/0024_claim_verification_budgets.sql b/src/services/extensions/v2/db/migrations/0024_claim_verification_budgets.sql new file mode 100644 index 00000000..bf19123e --- /dev/null +++ b/src/services/extensions/v2/db/migrations/0024_claim_verification_budgets.sql @@ -0,0 +1,7 @@ +CREATE TABLE claim_verification_budgets ( + key TEXT PRIMARY KEY NOT NULL, + attempts INTEGER NOT NULL, + expires_at INTEGER NOT NULL +); +--> statement-breakpoint +CREATE INDEX idx_claim_verification_budgets_expiry ON claim_verification_budgets (expires_at); diff --git a/src/services/extensions/v2/db/migrations/meta/0024_snapshot.json b/src/services/extensions/v2/db/migrations/meta/0024_snapshot.json new file mode 100644 index 00000000..115a3e48 --- /dev/null +++ b/src/services/extensions/v2/db/migrations/meta/0024_snapshot.json @@ -0,0 +1,1085 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "8ca0b40f-64d5-441f-97e3-0283d5cc1192", + "prevId": "94fe44c4-3101-4040-b57c-a72e4d543066", + "tables": { + "claim_verification_budgets": { + "name": "claim_verification_budgets", + "columns": { + "key": { + "name": "key", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "attempts": { + "name": "attempts", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "expires_at": { + "name": "expires_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "idx_claim_verification_budgets_expiry": { + "name": "idx_claim_verification_budgets_expiry", + "columns": ["expires_at"], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "developer_claims": { + "name": "developer_claims", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "claimant_id": { + "name": "claimant_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'pending'" + }, + "note": { + "name": "note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "review_note": { + "name": "review_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "reviewer_id": { + "name": "reviewer_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "reviewed_at": { + "name": "reviewed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_org_verified": { + "name": "github_org_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verification_note": { + "name": "github_verification_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developer_claims_developer": { + "name": "idx_developer_claims_developer", + "columns": ["developer_id"], + "isUnique": false + }, + "idx_developer_claims_claimant": { + "name": "idx_developer_claims_claimant", + "columns": ["claimant_id"], + "isUnique": false + }, + "idx_developer_claims_pending_unique": { + "name": "idx_developer_claims_pending_unique", + "columns": ["developer_id", "claimant_id"], + "isUnique": true, + "where": "\"developer_claims\".\"status\" = 'pending'" + }, + "idx_developer_claims_pending_queue": { + "name": "idx_developer_claims_pending_queue", + "columns": ["created_at"], + "isUnique": false, + "where": "\"developer_claims\".\"status\" = 'pending'" + } + }, + "foreignKeys": { + "developer_claims_developer_id_developers_id_fk": { + "name": "developer_claims_developer_id_developers_id_fk", + "tableFrom": "developer_claims", + "tableTo": "developers", + "columnsFrom": ["developer_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_claims_claimant_id_users_id_fk": { + "name": "developer_claims_claimant_id_users_id_fk", + "tableFrom": "developer_claims", + "tableTo": "users", + "columnsFrom": ["claimant_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_claims_reviewer_id_users_id_fk": { + "name": "developer_claims_reviewer_id_users_id_fk", + "tableFrom": "developer_claims", + "tableTo": "users", + "columnsFrom": ["reviewer_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "developer_claims_status_check": { + "name": "developer_claims_status_check", + "value": "\"developer_claims\".\"status\" IN ('pending', 'approved', 'rejected')" + }, + "developer_claims_github_org_verified_check": { + "name": "developer_claims_github_org_verified_check", + "value": "\"developer_claims\".\"github_org_verified\" IN (0, 1)" + } + } + }, + "developer_history": { + "name": "developer_history", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "changed_by": { + "name": "changed_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "changed_at": { + "name": "changed_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + } + }, + "indexes": { + "idx_developer_history_developer_changed_at": { + "name": "idx_developer_history_developer_changed_at", + "columns": ["developer_id", "changed_at"], + "isUnique": false + } + }, + "foreignKeys": { + "developer_history_changed_by_users_id_fk": { + "name": "developer_history_changed_by_users_id_fk", + "tableFrom": "developer_history", + "tableTo": "users", + "columnsFrom": ["changed_by"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "developer_transfers": { + "name": "developer_transfers", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "token_hash": { + "name": "token_hash", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_by": { + "name": "created_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "expires_at": { + "name": "expires_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "accepted_by": { + "name": "accepted_by", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "accepted_at": { + "name": "accepted_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "revoked_at": { + "name": "revoked_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developer_transfers_token": { + "name": "idx_developer_transfers_token", + "columns": ["token_hash"], + "isUnique": true + }, + "idx_developer_transfers_pending": { + "name": "idx_developer_transfers_pending", + "columns": ["developer_id"], + "isUnique": true, + "where": "\"developer_transfers\".\"accepted_at\" IS NULL AND \"developer_transfers\".\"revoked_at\" IS NULL" + } + }, + "foreignKeys": { + "developer_transfers_developer_id_developers_id_fk": { + "name": "developer_transfers_developer_id_developers_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "developers", + "columnsFrom": ["developer_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_transfers_created_by_users_id_fk": { + "name": "developer_transfers_created_by_users_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "users", + "columnsFrom": ["created_by"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_transfers_accepted_by_users_id_fk": { + "name": "developer_transfers_accepted_by_users_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "users", + "columnsFrom": ["accepted_by"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "developers": { + "name": "developers", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "owner_user_id": { + "name": "owner_user_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "approved_at": { + "name": "approved_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'1970-01-01T00:00:00.000Z'" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'1970-01-01T00:00:00.000Z'" + }, + "avatar_url": { + "name": "avatar_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "contact_email": { + "name": "contact_email", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "ownership_epoch": { + "name": "ownership_epoch", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "content_revision": { + "name": "content_revision", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "approved_revision": { + "name": "approved_revision", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "approved_by": { + "name": "approved_by", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_org_verified": { + "name": "github_org_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verification_note": { + "name": "github_verification_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verified_at": { + "name": "github_verified_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_url_verified": { + "name": "github_url_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "url_check_cooldown_until": { + "name": "url_check_cooldown_until", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developers_owner_unique": { + "name": "idx_developers_owner_unique", + "columns": ["owner_user_id"], + "isUnique": true + }, + "idx_developers_approved": { + "name": "idx_developers_approved", + "columns": ["approved_at"], + "isUnique": false + } + }, + "foreignKeys": { + "developers_owner_user_id_users_id_fk": { + "name": "developers_owner_user_id_users_id_fk", + "tableFrom": "developers", + "tableTo": "users", + "columnsFrom": ["owner_user_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "developers_ownership_epoch_check": { + "name": "developers_ownership_epoch_check", + "value": "\"developers\".\"ownership_epoch\" >= 1" + }, + "developers_content_revision_check": { + "name": "developers_content_revision_check", + "value": "\"developers\".\"content_revision\" >= 1" + }, + "developers_github_org_verified_check": { + "name": "developers_github_org_verified_check", + "value": "\"developers\".\"github_org_verified\" IN (0, 1)" + }, + "developers_github_url_verified_check": { + "name": "developers_github_url_verified_check", + "value": "\"developers\".\"github_url_verified\" = 1" + } + } + }, + "extension_revisions": { + "name": "extension_revisions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "extension_id": { + "name": "extension_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "submitted_by": { + "name": "submitted_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'pending'" + }, + "content": { + "name": "content", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "reviewer_id": { + "name": "reviewer_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "review_note": { + "name": "review_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "reviewed_at": { + "name": "reviewed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "ownership_epoch": { + "name": "ownership_epoch", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + } + }, + "indexes": { + "idx_extension_revisions_submitted_by": { + "name": "idx_extension_revisions_submitted_by", + "columns": ["submitted_by"], + "isUnique": false + }, + "idx_extension_revisions_developer": { + "name": "idx_extension_revisions_developer", + "columns": ["developer_id"], + "isUnique": false + }, + "idx_extension_revisions_pending": { + "name": "idx_extension_revisions_pending", + "columns": ["extension_id"], + "isUnique": true, + "where": "\"extension_revisions\".\"status\" = 'pending'" + }, + "idx_extension_revisions_extension_page": { + "name": "idx_extension_revisions_extension_page", + "columns": ["extension_id", "\"created_at\" desc", "\"id\" desc"], + "isUnique": false + }, + "idx_extension_revisions_submitter_page": { + "name": "idx_extension_revisions_submitter_page", + "columns": ["submitted_by", "\"created_at\" desc", "\"id\" desc"], + "isUnique": false + }, + "idx_extension_revisions_queue_page": { + "name": "idx_extension_revisions_queue_page", + "columns": ["status", "created_at", "id"], + "isUnique": false + }, + "idx_extension_revisions_reviewed": { + "name": "idx_extension_revisions_reviewed", + "columns": ["extension_id", "reviewed_at"], + "isUnique": false, + "where": "\"extension_revisions\".\"status\" IN ('approved', 'rejected')" + }, + "idx_extension_revisions_submitter_pending": { + "name": "idx_extension_revisions_submitter_pending", + "columns": ["submitted_by"], + "isUnique": false, + "where": "\"extension_revisions\".\"status\" = 'pending'" + } + }, + "foreignKeys": { + "extension_revisions_extension_id_extensions_id_fk": { + "name": "extension_revisions_extension_id_extensions_id_fk", + "tableFrom": "extension_revisions", + "tableTo": "extensions", + "columnsFrom": ["extension_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "extension_revisions_submitted_by_users_id_fk": { + "name": "extension_revisions_submitted_by_users_id_fk", + "tableFrom": "extension_revisions", + "tableTo": "users", + "columnsFrom": ["submitted_by"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "extension_revisions_reviewer_id_users_id_fk": { + "name": "extension_revisions_reviewer_id_users_id_fk", + "tableFrom": "extension_revisions", + "tableTo": "users", + "columnsFrom": ["reviewer_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "extension_revisions_status_check": { + "name": "extension_revisions_status_check", + "value": "\"extension_revisions\".\"status\" IN ('pending', 'approved', 'rejected')" + }, + "extension_revisions_ownership_epoch_check": { + "name": "extension_revisions_ownership_epoch_check", + "value": "\"extension_revisions\".\"ownership_epoch\" >= 1" + } + } + }, + "extensions": { + "name": "extensions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "published_at": { + "name": "published_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "published_revision_id": { + "name": "published_revision_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "releases": { + "name": "releases", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "website": { + "name": "website", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "license": { + "name": "license", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "icon_url": { + "name": "icon_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "readme": { + "name": "readme", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "source": { + "name": "source", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "version": { + "name": "version", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "download_url": { + "name": "download_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "delisted_at": { + "name": "delisted_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "delist_reason": { + "name": "delist_reason", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_extensions_id_nocase": { + "name": "idx_extensions_id_nocase", + "columns": ["lower(\"id\")"], + "isUnique": true + }, + "idx_extensions_developer_order": { + "name": "idx_extensions_developer_order", + "columns": ["developer_id", "lower(\"id\")", "id"], + "isUnique": false + }, + "idx_extensions_catalogue_order": { + "name": "idx_extensions_catalogue_order", + "columns": ["lower(\"id\")", "id"], + "isUnique": false, + "where": "\"extensions\".\"published_at\" IS NOT NULL AND \"extensions\".\"delisted_at\" IS NULL" + }, + "idx_extensions_type_catalogue_order": { + "name": "idx_extensions_type_catalogue_order", + "columns": ["type", "lower(\"id\")", "id"], + "isUnique": false, + "where": "\"extensions\".\"published_at\" IS NOT NULL AND \"extensions\".\"delisted_at\" IS NULL" + } + }, + "foreignKeys": { + "extensions_developer_id_developers_id_fk": { + "name": "extensions_developer_id_developers_id_fk", + "tableFrom": "extensions", + "tableTo": "developers", + "columnsFrom": ["developer_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "extensions_published_content_check": { + "name": "extensions_published_content_check", + "value": "\"extensions\".\"published_at\" IS NULL OR (\n \"extensions\".\"type\" IS NOT NULL AND \"extensions\".\"name\" IS NOT NULL AND\n \"extensions\".\"description\" IS NOT NULL AND \"extensions\".\"releases\" IS NOT NULL AND\n \"extensions\".\"website\" IS NOT NULL AND \"extensions\".\"license\" IS NOT NULL AND\n \"extensions\".\"readme\" IS NOT NULL AND \"extensions\".\"source\" IS NOT NULL AND\n \"extensions\".\"version\" IS NOT NULL AND \"extensions\".\"download_url\" IS NOT NULL\n )" + } + } + }, + "users": { + "name": "users", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "email": { + "name": "email", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "email_verified": { + "name": "email_verified", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "picture": { + "name": "picture", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "is_moderator": { + "name": "is_moderator", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "display_name": { + "name": "display_name", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_login": { + "name": "github_login", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_orgs": { + "name": "github_orgs", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_orgs_expires_at": { + "name": "github_orgs_expires_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "deleted_at": { + "name": "deleted_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": { + "idx_extension_revisions_extension_page": { + "columns": { + "\"created_at\" desc": { + "isExpression": true + }, + "\"id\" desc": { + "isExpression": true + } + } + }, + "idx_extension_revisions_submitter_page": { + "columns": { + "\"created_at\" desc": { + "isExpression": true + }, + "\"id\" desc": { + "isExpression": true + } + } + }, + "idx_extensions_id_nocase": { + "columns": { + "lower(\"id\")": { + "isExpression": true + } + } + }, + "idx_extensions_developer_order": { + "columns": { + "lower(\"id\")": { + "isExpression": true + } + } + }, + "idx_extensions_catalogue_order": { + "columns": { + "lower(\"id\")": { + "isExpression": true + } + } + }, + "idx_extensions_type_catalogue_order": { + "columns": { + "lower(\"id\")": { + "isExpression": true + } + } + } + } + } +} diff --git a/src/services/extensions/v2/db/migrations/meta/_journal.json b/src/services/extensions/v2/db/migrations/meta/_journal.json index 3703688d..05d7f3b1 100644 --- a/src/services/extensions/v2/db/migrations/meta/_journal.json +++ b/src/services/extensions/v2/db/migrations/meta/_journal.json @@ -71,6 +71,13 @@ "when": 1790009840510, "tag": "0023_fast_makkari", "breakpoints": true + }, + { + "idx": 24, + "version": "6", + "when": 1791060505627, + "tag": "0024_claim_verification_budgets", + "breakpoints": true } ] } diff --git a/src/services/extensions/v2/db/schema.ts b/src/services/extensions/v2/db/schema.ts index a02de17b..c13acb7d 100644 --- a/src/services/extensions/v2/db/schema.ts +++ b/src/services/extensions/v2/db/schema.ts @@ -394,3 +394,16 @@ export const developerClaims = sqliteTable( ) ] ); + +// No identity foreign keys: deleting an account or claim must not reset quota. +export const claimVerificationBudgets = sqliteTable( + "claim_verification_budgets", + { + key: text("key").primaryKey(), + attempts: integer("attempts").notNull(), + expiresAt: integer("expires_at").notNull() + }, + (table) => [ + index("idx_claim_verification_budgets_expiry").on(table.expiresAt) + ] +); diff --git a/src/services/extensions/v2/routes/ownership.ts b/src/services/extensions/v2/routes/ownership.ts index 14133eeb..8f703c50 100644 --- a/src/services/extensions/v2/routes/ownership.ts +++ b/src/services/extensions/v2/routes/ownership.ts @@ -67,7 +67,9 @@ export function registerOwnershipRoutes(app: ExtensionsV2App): void { 409: errorResponse( "Profile is already owned, caller already owns a different profile, or already has a pending claim on this one" ), - 429: errorResponse("GitHub verification is temporarily rate limited"), + 429: errorResponse( + "Claim allowance exhausted or GitHub verification is temporarily rate limited" + ), 503: errorResponse("GitHub verification is temporarily unavailable"), 422: errorResponse( "The request failed validation, or the GitHub account type is unsupported" diff --git a/test/services/extensions/v2/claim-budget.test.ts b/test/services/extensions/v2/claim-budget.test.ts new file mode 100644 index 00000000..bbfc445b --- /dev/null +++ b/test/services/extensions/v2/claim-budget.test.ts @@ -0,0 +1,167 @@ +import { describe, it, expect, vi } from "vitest"; +import { env } from "cloudflare:workers"; +import { wrapD1WithHook } from "./db-interceptor"; +import { request as ghRequest } from "@octokit/request"; +import { getExtensionsDb } from "../../../../src/lib/db"; +import { reserveClaimVerification } from "../../../../src/services/extensions/v2/db/claim-verification-budget"; +import { + setupExtensionsV2Tests, + db, + authHeaders, + seedUnownedDeveloper, + mockGithubEntity, + post +} from "./harness"; + +vi.mock("@octokit/request", async () => + (await import("../../../mocks/octokit")).octokitRequestMock() +); +setupExtensionsV2Tests(); +const path = "/extensions/v2/developers/target/claim"; + +describe("durable claim verification budgets", () => { + it("stops mismatches before GitHub, including after changing targets", async () => { + const headers = await authHeaders("attacker"); + await db + .prepare("UPDATE users SET github_login = 'other' WHERE id = 'attacker'") + .run(); + mockGithubEntity("User"); + for (let i = 0; i < 4; i++) { + await seedUnownedDeveloper(`target-${i}`); + const response = await post( + `/extensions/v2/developers/target-${i}/claim`, + headers, + {} + ); + expect(response.status).toBe(i < 3 ? 403 : 429); + } + expect(ghRequest).toHaveBeenCalledTimes(3); + }); + + it("retains quota after cancellation while preserving manual review", async () => { + const headers = await authHeaders("claimant"); + await seedUnownedDeveloper("target"); + for (let i = 0; i < 3; i++) { + const response = await post(path, headers, {}); + expect(response.status).toBe(201); + const body = (await response.json()) as { result: { id: string } }; + expect( + ( + await post( + `/extensions/v2/developers/claims/${body.result.id}/cancel`, + headers + ) + ).status + ).toBe(200); + } + expect((await post(path, headers, {})).status).toBe(429); + expect(ghRequest).toHaveBeenCalledTimes(3); + }); + + it("retains attempts on upstream failures and fails closed when the aggregate budget is full", async () => { + const headers = await authHeaders("claimant"); + await seedUnownedDeveloper("target"); + vi.mocked(ghRequest).mockRejectedValue( + Object.assign(new Error("Upstream failure"), { status: 500 }) + ); + for (let i = 0; i < 3; i++) + expect((await post(path, headers, {})).status).toBe(503); + expect((await post(path, headers, {})).status).toBe(429); + expect(ghRequest).toHaveBeenCalledTimes(3); + await db + .prepare( + "UPDATE claim_verification_budgets SET attempts = 300 WHERE key = 'global'" + ) + .run(); + await seedUnownedDeveloper("independent-target"); + expect( + ( + await post( + "/extensions/v2/developers/independent-target/claim", + await authHeaders("independent"), + {} + ) + ).status + ).toBe(429); + expect(ghRequest).toHaveBeenCalledTimes(3); + }); + + it("fails closed when the budget write fails", async () => { + const headers = await authHeaders("claimant"); + await seedUnownedDeveloper("target"); + env.DB_EXTENSIONS = wrapD1WithHook(db, (query) => { + if (query.includes("INSERT INTO claim_verification_budgets")) + throw new Error("budget write failed"); + }); + expect((await post(path, headers, {})).status).toBe(500); + expect(ghRequest).not.toHaveBeenCalled(); + }); + + it("shares the normalized developer budget between accounts", async () => { + const database = getExtensionsDb(db); + for (let i = 0; i < 3; i++) { + expect( + await reserveClaimVerification(database, `user-${i}`, "Target") + ).toBe(true); + } + expect(await reserveClaimVerification(database, "another", "target")).toBe( + false + ); + }); + + it("bounds independent accounts and targets by the aggregate budget", async () => { + const database = getExtensionsDb(db); + await db + .prepare( + "INSERT INTO claim_verification_budgets VALUES ('global', 299, unixepoch() + 3600)" + ) + .run(); + expect(await reserveClaimVerification(database, "user-1", "target-1")).toBe( + true + ); + expect(await reserveClaimVerification(database, "user-2", "target-2")).toBe( + false + ); + const row = await db + .prepare( + "SELECT COUNT(*) AS count FROM claim_verification_budgets WHERE key = 'account:user-2'" + ) + .first<{ count: number }>(); + expect(row?.count).toBe(0); + }); + + it("atomically admits only remaining capacity under concurrency", async () => { + const database = getExtensionsDb(db); + const results = await Promise.all( + Array.from({ length: 8 }, (_, i) => + reserveClaimVerification(database, "same-user", `target-${i}`) + ) + ); + expect(results.filter(Boolean)).toHaveLength(3); + const row = await db + .prepare( + "SELECT attempts FROM claim_verification_budgets WHERE key = 'global'" + ) + .first<{ attempts: number }>(); + expect(row?.attempts).toBe(3); + }); + + it("renews expired budgets using database time", async () => { + const database = getExtensionsDb(db); + for (let i = 0; i < 3; i++) + expect(await reserveClaimVerification(database, "user", "target")).toBe( + true + ); + expect(await reserveClaimVerification(database, "user", "target")).toBe( + false + ); + await db + .prepare( + "UPDATE claim_verification_budgets SET expires_at = unixepoch() - 1 WHERE key != 'global'" + ) + .run(); + expect(await reserveClaimVerification(database, "user", "target")).toBe( + true + ); + }); +}); diff --git a/test/services/extensions/v2/db-fixtures.ts b/test/services/extensions/v2/db-fixtures.ts index 9530f72a..b6ae1b3a 100644 --- a/test/services/extensions/v2/db-fixtures.ts +++ b/test/services/extensions/v2/db-fixtures.ts @@ -131,6 +131,7 @@ async function clearDeveloperHistory(db: D1Database): Promise { } export async function resetExtensionsDb(db: D1Database): Promise { + await db.prepare("DELETE FROM claim_verification_budgets").run(); await clearDeveloperHistory(db); for (const table of [ "extension_revisions", From 04420451684e0f6ebe50b1d2e7c6ba4b59512be3 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sun, 4 Oct 2026 19:36:27 +0100 Subject: [PATCH 3/9] Add GitHub request budget for previews Introduce a Durable Object-backed token bucket for previews GitHub traffic, with shared global and per-client limits enforced before upstream requests. Thread the budgeted GitHub helper through preview lookup and download routes, document the behavior, add coverage for exhaustion and refill cases, and register the new binding/migration in Wrangler and worker types. --- src/app/index.ts | 2 + .../cloudflare/preview-github-budget.ts | 49 ++++++ src/services/previews/v1/README.md | 21 +++ src/services/previews/v1/github/artifacts.ts | 69 +++++---- src/services/previews/v1/github/request.ts | 56 +++++++ src/services/previews/v1/resolve.ts | 5 +- src/services/previews/v1/routes/commit.ts | 11 +- src/services/previews/v1/routes/main.ts | 7 +- src/services/previews/v1/routes/pr.ts | 17 +- src/services/previews/v1/routes/respond.ts | 5 +- test/services/previews/v1/budget.test.ts | 146 ++++++++++++++++++ worker-configuration.d.ts | 4 +- wrangler.jsonc | 11 ++ 13 files changed, 350 insertions(+), 53 deletions(-) create mode 100644 src/lib/adapters/cloudflare/preview-github-budget.ts create mode 100644 src/services/previews/v1/github/request.ts create mode 100644 test/services/previews/v1/budget.test.ts diff --git a/src/app/index.ts b/src/app/index.ts index 39e9423f..e436c107 100644 --- a/src/app/index.ts +++ b/src/app/index.ts @@ -70,3 +70,5 @@ app.all("/*", (c) => { }); export default app; + +export { PreviewGitHubBudget } from "../lib/adapters/cloudflare/preview-github-budget"; diff --git a/src/lib/adapters/cloudflare/preview-github-budget.ts b/src/lib/adapters/cloudflare/preview-github-budget.ts new file mode 100644 index 00000000..8558600a --- /dev/null +++ b/src/lib/adapters/cloudflare/preview-github-budget.ts @@ -0,0 +1,49 @@ +import { DurableObject } from "cloudflare:workers"; + +// One singleton for all preview traffic, independent of URL, client and colo. +// Token buckets permit 60 calls in a burst, then 1000/hour globally and +// 120/hour per client. Every upstream call (not just each lookup) costs one. +export class PreviewGitHubBudget extends DurableObject { + constructor(ctx: DurableObjectState, env: CloudflareBindings) { + super(ctx, env); + this.ctx.storage.sql.exec(`CREATE TABLE IF NOT EXISTS buckets ( + key TEXT PRIMARY KEY, tokens REAL NOT NULL, updated_at INTEGER NOT NULL + )`); + } + + reserve(client: string): boolean { + const now = Date.now(); + return this.ctx.storage.transactionSync(() => { + // A client bucket is full after 30 minutes; discard only older state. + this.ctx.storage.sql.exec( + "DELETE FROM buckets WHERE key != 'global' AND updated_at < ?", + now - 1800000 + ); + const buckets = [ + { key: "global", rate: 1000 / 3600000 }, + { key: `client:${client}`, rate: 120 / 3600000 } + ].map(({ key, rate }) => { + const row = this.ctx.storage.sql + .exec<{ tokens: number; updated_at: number }>( + "SELECT tokens, updated_at FROM buckets WHERE key = ?", + key + ) + .toArray()[0]; + const tokens = row + ? Math.min(60, row.tokens + Math.max(0, now - row.updated_at) * rate) + : 60; + return { key, tokens }; + }); + if (buckets.some(({ tokens }) => tokens < 1)) return false; + for (const { key, tokens } of buckets) { + this.ctx.storage.sql.exec( + "INSERT OR REPLACE INTO buckets (key, tokens, updated_at) VALUES (?, ?, ?)", + key, + tokens - 1, + now + ); + } + return true; + }); + } +} diff --git a/src/services/previews/v1/README.md b/src/services/previews/v1/README.md index 23c27740..6fc767ce 100644 --- a/src/services/previews/v1/README.md +++ b/src/services/previews/v1/README.md @@ -90,3 +90,24 @@ Endpoints are not listed here. The service publishes its own contract: `versions/v1`). - `DOWNLOAD_BUCKET` (R2 binding) backs `/main` - see `wrangler.jsonc` for the bucket this points at and why. + +### GitHub request budgets + +Every previews GitHub request reserves a token from the singleton +`PREVIEW_GITHUB_BUDGET` Durable Object before contacting GitHub. This includes +fallback pages, PR head lookups, live download redirects and main enrichment. +Metadata cache hits and R2 downloads spend no tokens. All locations and lookup +keys share a token bucket with a 60-call burst and a refill of 1000 calls/hour. +Each client IP shares a second bucket with a 60-call burst and 120 calls/hour +refill; only a SHA-256 digest of Cloudflare's connecting IP is stored. Missing +addresses share one allowance. Idle client buckets expire after 30 minutes. + +Abbreviated SHAs and fork-PR resolution remain supported within these budgets. +Exhaustion or budget-service failure stops before another GitHub request and +returns the existing unavailable (503) response, without negative caching an +incomplete lookup. Main enrichment remains optional. These caps bound preview +consumption, rather than guaranteeing remaining quota when other token consumers +are busy. Raising them requires reviewing the shared token's quota allocation. + +The binding and SQLite class migration in `wrangler.jsonc` are required when +deploying this change; do not deploy only the application source. diff --git a/src/services/previews/v1/github/artifacts.ts b/src/services/previews/v1/github/artifacts.ts index 692a7e59..0dafbb9c 100644 --- a/src/services/previews/v1/github/artifacts.ts +++ b/src/services/previews/v1/github/artifacts.ts @@ -1,4 +1,4 @@ -import { request as ghRequest } from "@octokit/request"; +import { PreviewGitHub, previewRequest } from "./request"; import { classifyGitHubError, GitHubError, @@ -68,11 +68,12 @@ function unavailable( } async function listArtifacts( - githubToken: string, + github: PreviewGitHub, name: string | undefined, page: number = 1 ): Promise { - const result = await ghRequest( + const result = await previewRequest( + github, "GET /repos/{owner}/{repo}/actions/artifacts", { owner: REPO_OWNER, @@ -80,7 +81,7 @@ async function listArtifacts( ...(name ? { name } : {}), per_page: 100, page, - headers: { Authorization: `Bearer ${githubToken}` } + headers: { Authorization: `Bearer ${github.token}` } } ); return result.data.artifacts as RawArtifact[]; @@ -103,11 +104,11 @@ const MAX_FALLBACK_PAGES = 50; // through until GitHub returns a page short of per_page - the real "no // more results" signal - or a match is found, whichever happens first. async function findInFallbackPages( - githubToken: string, + github: PreviewGitHub, shaLower: string ): Promise { for (let page = 1; page <= MAX_FALLBACK_PAGES; page++) { - const artifacts = await listArtifacts(githubToken, undefined, page); + const artifacts = await listArtifacts(github, undefined, page); const match = matchArtifact( artifacts.filter((artifact) => artifact.name?.startsWith(ARTIFACT_NAME_PREFIX) @@ -127,17 +128,18 @@ async function findInFallbackPages( const MAX_RUN_ARTIFACT_LISTINGS = 50; async function listRunArtifacts( - githubToken: string, + github: PreviewGitHub, runId: number ): Promise { - const result = await ghRequest( + const result = await previewRequest( + github, "GET /repos/{owner}/{repo}/actions/runs/{run_id}/artifacts", { owner: REPO_OWNER, repo: REPO_NAME, run_id: runId, per_page: 100, - headers: { Authorization: `Bearer ${githubToken}` } + headers: { Authorization: `Bearer ${github.token}` } } ); return result.data.artifacts as RawArtifact[]; @@ -153,20 +155,24 @@ async function listRunArtifacts( // workflow_run is synthesized from the run at hand when absent, so // matchArtifact can rely on it. async function findArtifactByRunHeadSha( - githubToken: string, + github: PreviewGitHub, shaLower: string ): Promise { let listingsLeft = MAX_RUN_ARTIFACT_LISTINGS; for (let page = 1; ; page++) { - const result = await ghRequest("GET /repos/{owner}/{repo}/actions/runs", { - owner: REPO_OWNER, - repo: REPO_NAME, - head_sha: shaLower, - per_page: 100, - page, - headers: { Authorization: `Bearer ${githubToken}` } - }); + const result = await previewRequest( + github, + "GET /repos/{owner}/{repo}/actions/runs", + { + owner: REPO_OWNER, + repo: REPO_NAME, + head_sha: shaLower, + per_page: 100, + page, + headers: { Authorization: `Bearer ${github.token}` } + } + ); const runs = (result.data.workflow_runs ?? []) as Array<{ id: number; head_sha: string; @@ -184,7 +190,7 @@ async function findArtifactByRunHeadSha( "workflow-run artifact scan exhausted its listing budget before matching" ); } - const artifacts = await listRunArtifacts(githubToken, run.id); + const artifacts = await listRunArtifacts(github, run.id); const match = matchArtifact( artifacts .filter((artifact) => artifact.name?.startsWith(ARTIFACT_NAME_PREFIX)) @@ -264,23 +270,20 @@ function toPreviewArtifact(match: ArtifactMatch): PreviewArtifact { // size); short prefixes stay on the page scan (findInFallbackPages), // since the runs API can't be trusted to match on a partial SHA. export async function findPreviewArtifactByCommitSha( - githubToken: string, + github: PreviewGitHub, sha: string ): Promise> { const shaLower = sha.toLowerCase(); const url = `https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/actions/artifacts`; try { - const exact = await listArtifacts( - githubToken, - artifactNameForSha(shaLower) - ); + const exact = await listArtifacts(github, artifactNameForSha(shaLower)); let match = matchArtifact(exact, shaLower); if (!match) { match = shaLower.length === 40 - ? await findArtifactByRunHeadSha(githubToken, shaLower) - : await findInFallbackPages(githubToken, shaLower); + ? await findArtifactByRunHeadSha(github, shaLower) + : await findInFallbackPages(github, shaLower); } if (!match) { @@ -294,18 +297,19 @@ export async function findPreviewArtifactByCommitSha( } export async function resolvePullRequestHeadSha( - githubToken: string, + github: PreviewGitHub, prNumber: number ): Promise> { const url = `https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/pulls/${prNumber}`; try { - const result = await ghRequest( + const result = await previewRequest( + github, "GET /repos/{owner}/{repo}/pulls/{pull_number}", { owner: REPO_OWNER, repo: REPO_NAME, pull_number: prNumber, - headers: { Authorization: `Bearer ${githubToken}` } + headers: { Authorization: `Bearer ${github.token}` } } ); return { status: "found", data: result.data.head.sha }; @@ -320,12 +324,13 @@ export async function resolvePullRequestHeadSha( // download-worker/src/preview.ts's getArtifactDownloadUrl - GitHub answers // with a 302 to a signed, temporary URL rather than the file itself. export async function getArtifactDownloadUrl( - githubToken: string, + github: PreviewGitHub, artifactId: number ): Promise> { const url = `https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/actions/artifacts/${artifactId}/zip`; try { - const result = await ghRequest( + const result = await previewRequest( + github, "GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/{archive_format}", { owner: REPO_OWNER, @@ -333,7 +338,7 @@ export async function getArtifactDownloadUrl( artifact_id: artifactId, archive_format: "zip", request: { redirect: "manual" }, - headers: { Authorization: `Bearer ${githubToken}` } + headers: { Authorization: `Bearer ${github.token}` } } ); diff --git a/src/services/previews/v1/github/request.ts b/src/services/previews/v1/github/request.ts new file mode 100644 index 00000000..bd5f8604 --- /dev/null +++ b/src/services/previews/v1/github/request.ts @@ -0,0 +1,56 @@ +import { Context } from "hono"; +import { request } from "@octokit/request"; +import { GitHubError } from "../../../../lib/github-errors"; + +export interface PreviewGitHub { + token: string; + reserve: () => Promise; +} + +export function previewGitHub( + c: Context<{ Bindings: CloudflareBindings }> +): PreviewGitHub { + // CF supplies this header at the edge; never trust X-Forwarded-For. + // Missing addresses share a conservative allowance. + const client = c.req.header("CF-Connecting-IP") ?? "unknown"; + return { + token: c.env.GITHUB_TOKEN, + reserve: async () => { + const digest = await crypto.subtle.digest( + "SHA-256", + new TextEncoder().encode(client) + ); + const key = Array.from(new Uint8Array(digest), (b) => + b.toString(16).padStart(2, "0") + ).join(""); + return c.env.PREVIEW_GITHUB_BUDGET.getByName("previews").reserve(key); + } + }; +} + +export async function previewRequest( + github: PreviewGitHub, + route: string, + parameters: Record +) { + let allowed: boolean; + try { + allowed = await github.reserve(); + } catch { + throw new GitHubError( + "Preview GitHub budget unavailable", + 503, + "preview_budget_unavailable" + ); + } + if (!allowed) { + // Existing lookup error handling returns 503, without negative-caching + // an incomplete scan. Binding failures also fail closed before GitHub. + throw new GitHubError( + "Preview GitHub request budget exhausted", + 503, + "preview_budget_exhausted" + ); + } + return request(route, parameters); +} diff --git a/src/services/previews/v1/resolve.ts b/src/services/previews/v1/resolve.ts index 1f74f56e..7a77e1a8 100644 --- a/src/services/previews/v1/resolve.ts +++ b/src/services/previews/v1/resolve.ts @@ -1,3 +1,4 @@ +import { PreviewGitHub } from "./github/request"; import { ArtifactPreview } from "./schemas/previews"; import { findPreviewArtifactByCommitSha, @@ -12,11 +13,11 @@ export type PreviewLookupResult = GithubLookupResult; // land; a specific commit's build does not, so that's the one stable link // to hand back regardless of which route resolved it. export async function resolveArtifactPreview( - githubToken: string, + github: PreviewGitHub, sha: string, prNumber: number | null ): Promise { - const found = await findPreviewArtifactByCommitSha(githubToken, sha); + const found = await findPreviewArtifactByCommitSha(github, sha); if (found.status !== "found") return found; const { data } = found; diff --git a/src/services/previews/v1/routes/commit.ts b/src/services/previews/v1/routes/commit.ts index 65847649..c5ac98c0 100644 --- a/src/services/previews/v1/routes/commit.ts +++ b/src/services/previews/v1/routes/commit.ts @@ -1,3 +1,4 @@ +import { previewGitHub } from "../github/request"; import { createRoute } from "@hono/zod-openapi"; import { ArtifactPreview, @@ -79,12 +80,12 @@ export function registerCommitRoutes(app: PreviewsV1App): void { app.openapi(commitRoute, async (c) => { const { sha } = c.req.valid("param"); - const githubToken = c.env.GITHUB_TOKEN; + const github = previewGitHub(c); const result = await cachedLookup( c.env.CACHE_KV, cacheKeyForSha(sha), - () => resolveArtifactPreview(githubToken, sha, null), + () => resolveArtifactPreview(github, sha, null), ttlForArtifact, (p) => c.executionCtx.waitUntil(p) ); @@ -114,7 +115,7 @@ export function registerCommitRoutes(app: PreviewsV1App): void { app.openapi(commitDownloadRoute, async (c) => { const { sha } = c.req.valid("param"); - const githubToken = c.env.GITHUB_TOKEN; + const github = previewGitHub(c); // Shares the metadata route's cache entry for which artifact to // download - only the signed URL itself (resolved inside @@ -123,13 +124,13 @@ export function registerCommitRoutes(app: PreviewsV1App): void { const artifact = await cachedLookup( c.env.CACHE_KV, cacheKeyForSha(sha), - () => resolveArtifactPreview(githubToken, sha, null), + () => resolveArtifactPreview(github, sha, null), ttlForArtifact, (p) => c.executionCtx.waitUntil(p) ); return respondWithDownloadRedirect( c, - githubToken, + github, artifact, `No preview artifact exists for commit ${sha}.` ); diff --git a/src/services/previews/v1/routes/main.ts b/src/services/previews/v1/routes/main.ts index 447b9804..361c1c69 100644 --- a/src/services/previews/v1/routes/main.ts +++ b/src/services/previews/v1/routes/main.ts @@ -1,3 +1,4 @@ +import { PreviewGitHub, previewGitHub } from "../github/request"; import { createRoute } from "@hono/zod-openapi"; import { Context } from "hono"; import { @@ -26,7 +27,7 @@ const MAIN_NEGATIVE_CACHE_VALUE = "__main_preview_missing__"; // leaves them null; it never fails or degrades the response, since // download_url/digest below are R2-sourced and don't depend on this. async function resolveArtifactFields( - githubToken: string, + github: PreviewGitHub, commitSha: string | null ): Promise< Pick @@ -39,7 +40,7 @@ async function resolveArtifactFields( }; if (!commitSha) return empty; - const artifact = await findPreviewArtifactByCommitSha(githubToken, commitSha); + const artifact = await findPreviewArtifactByCommitSha(github, commitSha); if (artifact.status !== "found") return empty; return { @@ -114,7 +115,7 @@ async function resolveMainPreview( } const artifactFields = await resolveArtifactFields( - c.env.GITHUB_TOKEN, + previewGitHub(c), object.commitSha ); diff --git a/src/services/previews/v1/routes/pr.ts b/src/services/previews/v1/routes/pr.ts index a60ece1f..218f90fb 100644 --- a/src/services/previews/v1/routes/pr.ts +++ b/src/services/previews/v1/routes/pr.ts @@ -1,3 +1,4 @@ +import { PreviewGitHub, previewGitHub } from "../github/request"; import { createRoute } from "@hono/zod-openapi"; import { ArtifactPreviewResponseSchema, @@ -19,18 +20,18 @@ import { PreviewsV1App } from "./app"; // commit route's shape); the PR number is overlaid here, after the shared // read. async function resolvePrPreview( - githubToken: string, + github: PreviewGitHub, prNumber: number, kv: KVNamespace, waitUntil?: (promise: Promise) => void ): Promise { - const head = await resolvePullRequestHeadSha(githubToken, prNumber); + const head = await resolvePullRequestHeadSha(github, prNumber); if (head.status !== "found") return head; const commit = await cachedLookup( kv, cacheKeyForSha(head.data), - () => resolveArtifactPreview(githubToken, head.data, null), + () => resolveArtifactPreview(github, head.data, null), ttlForArtifact, waitUntil ); @@ -68,13 +69,13 @@ export function registerPrRoutes(app: PreviewsV1App): void { app.openapi(prRoute, async (c) => { const { number } = c.req.valid("param"); - const githubToken = c.env.GITHUB_TOKEN; + const github = previewGitHub(c); const result = await cachedLookup( c.env.CACHE_KV, `preview:pr:${number}`, () => - resolvePrPreview(githubToken, number, c.env.CACHE_KV, (p) => + resolvePrPreview(github, number, c.env.CACHE_KV, (p) => c.executionCtx.waitUntil(p) ), DEFAULT_CACHE_TTL_SECONDS, @@ -102,7 +103,7 @@ export function registerPrRoutes(app: PreviewsV1App): void { app.openapi(prDownloadRoute, async (c) => { const { number } = c.req.valid("param"); - const githubToken = c.env.GITHUB_TOKEN; + const github = previewGitHub(c); // Shares the metadata route's cache entry - see the equivalent comment // in routes/commit.ts. Without this, every download hit would cost 3 @@ -112,7 +113,7 @@ export function registerPrRoutes(app: PreviewsV1App): void { c.env.CACHE_KV, `preview:pr:${number}`, () => - resolvePrPreview(githubToken, number, c.env.CACHE_KV, (p) => + resolvePrPreview(github, number, c.env.CACHE_KV, (p) => c.executionCtx.waitUntil(p) ), DEFAULT_CACHE_TTL_SECONDS, @@ -120,7 +121,7 @@ export function registerPrRoutes(app: PreviewsV1App): void { ); return respondWithDownloadRedirect( c, - githubToken, + github, artifact, notFoundMessage(number) ); diff --git a/src/services/previews/v1/routes/respond.ts b/src/services/previews/v1/routes/respond.ts index e1c6c0a9..49426a11 100644 --- a/src/services/previews/v1/routes/respond.ts +++ b/src/services/previews/v1/routes/respond.ts @@ -1,3 +1,4 @@ +import { PreviewGitHub } from "../github/request"; import { Context } from "hono"; import { getArtifactDownloadUrl } from "../github/artifacts"; import { PreviewLookupResult } from "../resolve"; @@ -30,7 +31,7 @@ export function respondWithLookup( // and abandoned. export async function respondWithDownloadRedirect( c: Context, - githubToken: string, + github: PreviewGitHub, artifact: PreviewLookupResult, notFoundMessage: string ) { @@ -45,7 +46,7 @@ export async function respondWithDownloadRedirect( } const redirect = await getArtifactDownloadUrl( - githubToken, + github, artifact.data.artifact_id ); if (redirect.status === "not_found") { diff --git a/test/services/previews/v1/budget.test.ts b/test/services/previews/v1/budget.test.ts new file mode 100644 index 00000000..e5afed11 --- /dev/null +++ b/test/services/previews/v1/budget.test.ts @@ -0,0 +1,146 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { env } from "cloudflare:workers"; +import { + createExecutionContext, + runInDurableObject, + waitOnExecutionContext +} from "cloudflare:test"; +import app from "../../../../src/app"; +import { previewRequest } from "../../../../src/services/previews/v1/github/request"; + +vi.mock("@octokit/request", async () => + (await import("../../../mocks/octokit")).octokitRequestMock() +); +import { request } from "@octokit/request"; + +const budget = () => env.PREVIEW_GITHUB_BUDGET.getByName("previews"); + +async function get(path: string, ip = "192.0.2.1") { + const ctx = createExecutionContext(); + const response = await app.request( + path, + { headers: { "CF-Connecting-IP": ip } }, + env, + ctx + ); + await waitOnExecutionContext(ctx); + return response; +} + +beforeEach(async () => { + vi.clearAllMocks(); + await runInDurableObject(budget(), (_instance, state) => { + state.storage.sql.exec("DELETE FROM buckets"); + }); + const keys = await env.CACHE_KV.list({ prefix: "preview:" }); + await Promise.all(keys.keys.map(({ name }) => env.CACHE_KV.delete(name))); +}); + +describe("preview GitHub budget", () => { + it("atomically caps concurrent reservations across distinct clients", async () => { + const results = await Promise.all( + Array.from({ length: 100 }, (_, i) => budget().reserve(`client-${i}`)) + ); + expect(results.filter(Boolean)).toHaveLength(60); + expect(await budget().reserve("another-client")).toBe(false); + }); + + it("enforces the client bucket even when global capacity remains", async () => { + await runInDurableObject(budget(), (instance, state) => { + for (let i = 0; i < 60; i++) + expect(instance.reserve("one-client")).toBe(true); + state.storage.sql.exec("DELETE FROM buckets WHERE key = 'global'"); + expect(instance.reserve("one-client")).toBe(false); + expect(instance.reserve("other-client")).toBe(true); + }); + }); + + it("refills persisted buckets and expires idle client state", async () => { + await runInDurableObject(budget(), (instance, state) => { + const now = Date.now(); + state.storage.sql.exec( + "INSERT INTO buckets VALUES ('global', 0, ?)", + now - 36000 + ); + state.storage.sql.exec( + "INSERT INTO buckets VALUES ('client:active', 0, ?)", + now - 36000 + ); + state.storage.sql.exec( + "INSERT INTO buckets VALUES ('client:idle', 0, ?)", + now - 1800001 + ); + expect(instance.reserve("active")).toBe(true); + expect(instance.reserve("active")).toBe(false); + expect( + state.storage.sql + .exec("SELECT * FROM buckets WHERE key = 'client:idle'") + .toArray() + ).toHaveLength(0); + }); + }); + + it("bounds distinct short SHA misses including every fallback page", async () => { + vi.mocked(request).mockResolvedValue({ + data: { + artifacts: Array.from({ length: 100 }, () => ({ + name: "other", + expired: false + })) + } + } as never); + expect((await get("/previews/v1/commit/aaaaaaa")).status).toBe(404); + expect(request).toHaveBeenCalledTimes(51); + expect((await get("/previews/v1/commit/bbbbbbb", "192.0.2.2")).status).toBe( + 503 + ); + expect(request).toHaveBeenCalledTimes(60); + expect(await env.CACHE_KV.get("preview:commit:bbbbbbb")).toBeNull(); + expect((await get("/previews/v1/commit/ccccccc", "192.0.2.3")).status).toBe( + 503 + ); + expect(request).toHaveBeenCalledTimes(60); + }); + + it("bounds distinct PR keys before contacting GitHub", async () => { + vi.mocked(request).mockRejectedValue( + Object.assign(new Error("Not found"), { status: 404 }) + ); + for (let i = 1; i <= 60; i++) { + expect((await get(`/previews/v1/pr/${i}`, `192.0.2.${i}`)).status).toBe( + 404 + ); + } + expect((await get("/previews/v1/pr/61")).status).toBe(503); + expect(request).toHaveBeenCalledTimes(60); + expect(await env.CACHE_KV.get("preview:pr:61")).toBeNull(); + }); + + it("serves cached metadata but denies live downloads when exhausted", async () => { + const sha = "a".repeat(40); + await env.CACHE_KV.put( + `preview:commit:${sha}`, + JSON.stringify({ commit_sha: sha, artifact_id: 123 }) + ); + for (let i = 0; i < 60; i++) await budget().reserve(`client-${i}`); + expect((await get(`/previews/v1/commit/${sha}`)).status).toBe(200); + expect((await get(`/previews/v1/commit/${sha}/download`)).status).toBe(503); + expect(request).not.toHaveBeenCalled(); + }); + + it("fails closed if reservation fails", async () => { + await expect( + previewRequest( + { + token: "test", + reserve: async () => { + throw new Error("budget unavailable"); + } + }, + "GET /repos/{owner}/{repo}/actions/artifacts", + {} + ) + ).rejects.toThrow("Preview GitHub budget unavailable"); + expect(request).not.toHaveBeenCalled(); + }); +}); diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index 24aeb6c5..ee19e1a2 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,5 +1,5 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types --env-interface=CloudflareBindings` (hash: 0a920b6605a52fe8bb765a1b2b09184d) +// Generated by Wrangler by running `wrangler types --env-interface=CloudflareBindings` (hash: 9acc9d39a37fd9c6a402ae4b208ea835) // Runtime types generated with workerd@1.20260911.1 2026-06-24 nodejs_compat interface __BaseEnv_CloudflareBindings { AUTH_KV: KVNamespace; @@ -15,11 +15,13 @@ interface __BaseEnv_CloudflareBindings { ASSERTION_SIGNING_SECRET: string; EXTENSIONS_V2_MXROUTE_PASSWORD: string; EXTENSIONS_REVALIDATE_SECRET: string; + PREVIEW_GITHUB_BUDGET: DurableObjectNamespace; EXTENSIONS_FRONTEND: Fetcher /* extensions */; } declare namespace Cloudflare { interface GlobalProps { mainModule: typeof import("./src/app/index"); + durableNamespaces: "PreviewGitHubBudget"; } interface Env extends __BaseEnv_CloudflareBindings {} } diff --git a/wrangler.jsonc b/wrangler.jsonc index ad16a425..577e0624 100644 --- a/wrangler.jsonc +++ b/wrangler.jsonc @@ -67,6 +67,17 @@ "service": "extensions" } ], + "durable_objects": { + "bindings": [ + { "name": "PREVIEW_GITHUB_BUDGET", "class_name": "PreviewGitHubBudget" } + ] + }, + "migrations": [ + { + "tag": "preview-github-budget-v1", + "new_sqlite_classes": ["PreviewGitHubBudget"] + } + ], "ratelimits": [ { "name": "PROFILE_CREATION_RATE_LIMITER", From 1ac9a4bc7316654fcc8776a4da55db39b752e226 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sun, 4 Oct 2026 19:49:18 +0100 Subject: [PATCH 4/9] Normalize central alerts pagination and cache This change makes GET /central-alerts/v1/list behave as a public, cacheable endpoint. It normalizes equivalent pagination inputs to the same effective page, validates offset/limit rules before lookup, and caches successful 200 responses by page value instead of by arbitrary auth headers. It also ensures validation errors and DB failures are uncached and non-public, while preserving the legacy full-list contract when no usable pagination is supplied. The README documents the newly supported pagination and cache semantics. --- src/services/central-alerts/v1/README.md | 10 +- src/services/central-alerts/v1/index.ts | 160 ++++++------ test/services/central-alerts/v1/index.test.ts | 246 ++++++++++++++---- 3 files changed, 294 insertions(+), 122 deletions(-) diff --git a/src/services/central-alerts/v1/README.md b/src/services/central-alerts/v1/README.md index 16a3ff61..f9a71e0e 100644 --- a/src/services/central-alerts/v1/README.md +++ b/src/services/central-alerts/v1/README.md @@ -8,7 +8,15 @@ The Central Alerts service provides targeted notifications to FOSSBilling instal ### GET `/list` -Retrieve all alerts in the system. +Retrieve all alerts in the system. Optional `limit` (integer 1–100) and +`offset` enable pagination. An unusable limit keeps the full-list response; +`offset` without a usable limit returns 422. An omitted or unusable offset +with a usable limit defaults to zero. + +Successful responses are edge-cached for 60 seconds by the effective page. +Unknown query parameters and equivalent pagination spellings reuse the same +entry. This endpoint is public: Authorization does not affect its response or +bypass its cache. Validation failures and database errors are not cached. **Response:** diff --git a/src/services/central-alerts/v1/index.ts b/src/services/central-alerts/v1/index.ts index daddd068..1a424170 100644 --- a/src/services/central-alerts/v1/index.ts +++ b/src/services/central-alerts/v1/index.ts @@ -1,5 +1,4 @@ import { Hono } from "hono"; -import { cache } from "hono/cache"; import { cors } from "hono/cors"; import { trimTrailingSlash } from "hono/trailing-slash"; import { CentralAlertsDatabase } from "./database"; @@ -10,88 +9,97 @@ const centralAlertsV1 = new Hono<{ Bindings: CloudflareBindings }>(); centralAlertsV1.use("/*", cors({ origin: "*" }), trimTrailingSlash()); -// Admin panels poll this route constantly; the alert set changes at human -// speed, so an edge-cached response with a short window keeps those polls -// off D1. Authorization-bearing requests skip the cache (hono default), and -// only 200s are stored, so validation failures and D1 errors stay live. The -// key is the full URL: unlike the versions routes, the limit/offset query -// changes the response body, so a query-stripped key would serve page one -// to every pagination window for a minute. -centralAlertsV1.get( - "/list", - cache({ - cacheName: "central-alerts-v1", - cacheControl: "max-age=60", - keyGenerator: (c) => c.req.url - }), - async (c) => { - const db = new CentralAlertsDatabase( - getCentralAlertsDb(c.env.DB_CENTRAL_ALERTS) +// Admin panels poll this public representation constantly. Parse once before +// cache lookup so ignored inputs cannot create new entries for the same page. +centralAlertsV1.get("/list", async (c) => { + // Opt-in pagination: absent params keep the full-list contract. A + // non-numeric limit is treated as absent rather than a 400 - this route + // has never validated query params and FOSSBilling's client passes none. + // offset is the exception: only a caller opting into pagination can send + // it, so offset without a usable limit is a 422 (matching the v2 + // pagination endpoints) rather than a silently ignored param. + const limitParam = Number(c.req.query("limit")); + const hasValidLimit = + Number.isInteger(limitParam) && limitParam >= 1 && limitParam <= 100; + const rawOffset = c.req.query("offset"); + if (rawOffset !== undefined && !hasValidLimit) { + return c.json( + { + result: null, + error: { message: "offset requires limit", code: "VALIDATION_ERROR" } + }, + 422 ); + } + const offsetParam = rawOffset === undefined ? 0 : Number(rawOffset); + const page = hasValidLimit + ? { + limit: limitParam, + offset: + Number.isInteger(offsetParam) && offsetParam >= 0 ? offsetParam : 0 + } + : undefined; - // Opt-in pagination: absent params keep the full-list contract. A - // non-numeric limit is treated as absent rather than a 400 - this route - // has never validated query params and FOSSBilling's client passes none. - // offset is the exception: only a caller opting into pagination can send - // it, so offset without a usable limit is a 422 (matching the v2 - // pagination endpoints) rather than a silently ignored param. - const limitParam = Number(c.req.query("limit")); - const hasValidLimit = - Number.isInteger(limitParam) && limitParam >= 1 && limitParam <= 100; - const rawOffset = c.req.query("offset"); - if (rawOffset !== undefined && !hasValidLimit) { - return c.json( - { - result: null, - error: { message: "offset requires limit", code: "VALIDATION_ERROR" } - }, - 422 - ); - } - const offsetParam = rawOffset === undefined ? 0 : Number(rawOffset); - const page = hasValidLimit - ? { - limit: limitParam, - offset: - Number.isInteger(offsetParam) && offsetParam >= 0 ? offsetParam : 0 - } + const cacheUrl = new URL(c.req.url); + // Hono decodes path aliases before routing; key that same routed path. + cacheUrl.pathname = c.req.path; + cacheUrl.search = ""; + cacheUrl.hash = ""; + if (page) { + cacheUrl.searchParams.set("limit", String(page.limit)); + cacheUrl.searchParams.set("offset", String(page.offset)); + } + // This route never authenticates or varies by Authorization. Use a + // header-free key so that arbitrary credentials cannot force a D1 read. + const edgeCache = + typeof caches !== "undefined" + ? await caches.open("central-alerts-v1") : undefined; + const cached = await edgeCache?.match(cacheUrl.href); + if (cached) return new Response(cached.body, cached); - const { data, error } = await db.getAllAlerts(page); + const db = new CentralAlertsDatabase( + getCentralAlertsDb(c.env.DB_CENTRAL_ALERTS) + ); + const { data, error } = await db.getAllAlerts(page); - if (error) { - logError("central-alerts", "Failed to list central alerts", { - message: error.message, - code: error.code - }); - return c.json( - { - result: null, - error: { - message: "Unable to load central alerts", - code: error.code || "DATABASE_ERROR" - } - }, - 500 - ); - } + if (error) { + logError("central-alerts", "Failed to list central alerts", { + message: error.message, + code: error.code + }); + return c.json( + { + result: null, + error: { + message: "Unable to load central alerts", + code: error.code || "DATABASE_ERROR" + } + }, + 500 + ); + } - return c.json({ - result: { - alerts: data?.alerts || [], - ...(page && data - ? { - pagination: { - limit: page.limit, - offset: page.offset, - has_more: data.hasMore - } + c.header("Cache-Control", "max-age=60"); + const response = c.json({ + result: { + alerts: data?.alerts || [], + ...(page && data + ? { + pagination: { + limit: page.limit, + offset: page.offset, + has_more: data.hasMore } - : {}) - }, - error: null - }); + } + : {}) + }, + error: null + }); + if (edgeCache) { + c.executionCtx.waitUntil(edgeCache.put(cacheUrl.href, response.clone())); } -); + return response; +}); export default centralAlertsV1; diff --git a/test/services/central-alerts/v1/index.test.ts b/test/services/central-alerts/v1/index.test.ts index ad2c6062..aa6f91d9 100644 --- a/test/services/central-alerts/v1/index.test.ts +++ b/test/services/central-alerts/v1/index.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeAll } from "vitest"; +import { describe, it, expect, beforeAll, beforeEach } from "vitest"; import { createExecutionContext, waitOnExecutionContext @@ -8,10 +8,7 @@ import app from "../../../../src/app"; import type { CentralAlertsResponse } from "../../../utils/test-types"; import { applyTestMigrations } from "../../../utils/apply-migrations"; -// Authorization header makes hono's cache middleware skip (same pattern as -// the stats tests) so each request reaches the handler; the cache path is -// covered by the dedicated cache test below, which omits the header. -const BYPASS_CACHE = { authorization: "test-bypass-cache" } as const; +let requestHost: string; // No fixture-insertion setup needed beyond migrations: migrations already // seed exactly the row these tests assert on (see @@ -19,13 +16,17 @@ const BYPASS_CACHE = { authorization: "test-bypass-cache" } as const; // against the real local D1. describe("Central Alerts API v1", () => { beforeAll(applyTestMigrations); + beforeEach(() => { + // Isolate edge entries without relying on a client-controlled bypass. + requestHost = `${crypto.randomUUID()}.example.com`; + }); describe("GET /list", () => { it("should return list of central alerts", async () => { const ctx = createExecutionContext(); const response = await app.request( - "/central-alerts/v1/list", - { headers: BYPASS_CACHE }, + `https://${requestHost}/central-alerts/v1/list`, + {}, env, ctx ); @@ -40,14 +41,12 @@ describe("Central Alerts API v1", () => { expect(Array.isArray(data.result.alerts)).toBe(true); }); - // Deliberately omits BYPASS_CACHE so hono's cache middleware - // participates: the second identical request is served from the edge - // cache without reaching D1. + // The second identical request must avoid D1. it("should serve a repeated request from the edge cache", async () => { const requestOnce = async () => { const ctx = createExecutionContext(); const response = await app.request( - "/central-alerts/v1/list", + `https://${requestHost}/central-alerts/v1/list`, {}, env, ctx @@ -79,14 +78,162 @@ describe("Central Alerts API v1", () => { } }); + it.each([ + { + initial: "", + variants: [ + "?nonce=one", + "?nonce=two", + "?limit=bad", + "?limit=0", + "?limit=101", + "?limit=", + "?limit=bad&limit=1" + ] + }, + { + initial: "?limit=1", + variants: [ + "?nonce=one&offset=0&limit=01", + "?limit=1e0&offset=bad", + "?limit=0x1&offset=-1", + "?limit=%201%20&offset=0.0", + "?%6cimit=1", + "?%6cimit=2&limit=1", + "?limit=1&limit=2" + ] + }, + { + initial: "?limit=2&offset=1", + variants: ["?offset=01&nonce=one&limit=2.0", "?limit=2&offset=1e0"] + } + ])( + "should reuse the effective page for $initial", + async ({ initial, variants }) => { + const requestList = async (query: string, authorization?: string) => { + const ctx = createExecutionContext(); + const response = await app.request( + `https://${requestHost}/central-alerts/v1/list${query}`, + { headers: authorization === undefined ? {} : { authorization } }, + env, + ctx + ); + await waitOnExecutionContext(ctx); + return response; + }; + const first = await requestList(initial); + expect(first.status).toBe(200); + const body = await first.text(); + const realDb = env.DB_CENTRAL_ALERTS; + env.DB_CENTRAL_ALERTS = { + prepare() { + throw new Error("unexpected D1 read"); + } + } as unknown as D1Database; + try { + for (const query of variants) { + for (const authorization of [undefined, "x", "Bearer arbitrary"]) { + const response = await requestList(query, authorization); + expect(response.status).toBe(200); + await expect(response.text()).resolves.toBe(body); + expect(response.headers.get("Access-Control-Allow-Origin")).toBe( + "*" + ); + } + } + // Validation must run before lookup, even when the full list is warm. + const invalid = await requestList("?offset=0&limit=bad", "x"); + expect(invalid.status).toBe(422); + expect(invalid.headers.get("cache-control")).toBeNull(); + } finally { + env.DB_CENTRAL_ALERTS = realDb; + } + } + ); + + it("should share cached responses across encoded route aliases", async () => { + const requestPath = async (path: string) => { + const ctx = createExecutionContext(); + const response = await app.request( + `https://${requestHost}${path}`, + {}, + env, + ctx + ); + await waitOnExecutionContext(ctx); + return response; + }; + const first = await requestPath("/central-alerts/v1/list"); + expect(first.status).toBe(200); + const body = await first.text(); + const realDb = env.DB_CENTRAL_ALERTS; + env.DB_CENTRAL_ALERTS = { + prepare() { + throw new Error("unexpected D1 read"); + } + } as unknown as D1Database; + try { + for (const path of [ + "/central-alerts/v1/%6cist", + "/central-alerts/v1/%6Cist", + "/%63entral-alerts/v1/list?nonce=one", + "/central-alerts/%761/list" + ]) { + const response = await requestPath(path); + expect(response.status).toBe(200); + await expect(response.text()).resolves.toBe(body); + } + } finally { + env.DB_CENTRAL_ALERTS = realDb; + } + }); + + it("should keep the full list and different pagination windows distinct", async () => { + const bodies = []; + for (const query of ["", "?limit=1", "?limit=2", "?limit=1&offset=1"]) { + const ctx = createExecutionContext(); + const response = await app.request( + `https://${requestHost}/central-alerts/v1/list${query}`, + {}, + env, + ctx + ); + await waitOnExecutionContext(ctx); + expect(response.status).toBe(200); + bodies.push( + (await response.json()) as { + result: { + alerts: unknown[]; + pagination?: { limit: number; offset: number }; + }; + } + ); + } + expect(bodies[0].result).not.toHaveProperty("pagination"); + expect(bodies[1].result.pagination).toMatchObject({ + limit: 1, + offset: 0 + }); + expect(bodies[2].result.pagination).toMatchObject({ + limit: 2, + offset: 0 + }); + expect(bodies[3].result.pagination).toMatchObject({ + limit: 1, + offset: 1 + }); + expect(bodies[1].result.alerts).toHaveLength(1); + expect(bodies[3].result.alerts).toHaveLength(0); + }); + // offset is only meaningful alongside a limit: a stray offset alone // must not silently fall through to the full legacy response the way // it would if the param were simply ignored. it("should reject offset without limit with 422", async () => { const ctx = createExecutionContext(); const response = await app.request( - "/central-alerts/v1/list?offset=1", - { headers: BYPASS_CACHE }, + `https://${requestHost}/central-alerts/v1/list?offset=1`, + {}, env, ctx ); @@ -105,8 +252,8 @@ describe("Central Alerts API v1", () => { it("should return alerts from static data", async () => { const ctx = createExecutionContext(); const response = await app.request( - "/central-alerts/v1/list", - { headers: BYPASS_CACHE }, + `https://${requestHost}/central-alerts/v1/list`, + {}, env, ctx ); @@ -125,8 +272,8 @@ describe("Central Alerts API v1", () => { it("should include buttons in alerts when present", async () => { const ctx = createExecutionContext(); const response = await app.request( - "/central-alerts/v1/list", - { headers: BYPASS_CACHE }, + `https://${requestHost}/central-alerts/v1/list`, + {}, env, ctx ); @@ -150,8 +297,8 @@ describe("Central Alerts API v1", () => { it("should redirect trailing slash to non-trailing slash path", async () => { const ctx = createExecutionContext(); const response = await app.request( - "/central-alerts/v1/list/", - { headers: BYPASS_CACHE }, + `https://${requestHost}/central-alerts/v1/list/`, + {}, env, ctx ); @@ -166,8 +313,8 @@ describe("Central Alerts API v1", () => { it("should return consistent data on multiple requests", async () => { const ctx1 = createExecutionContext(); const response1 = await app.request( - "/central-alerts/v1/list", - { headers: BYPASS_CACHE }, + `https://${requestHost}/central-alerts/v1/list`, + {}, env, ctx1 ); @@ -176,8 +323,8 @@ describe("Central Alerts API v1", () => { const ctx2 = createExecutionContext(); const response2 = await app.request( - "/central-alerts/v1/list", - { headers: BYPASS_CACHE }, + `https://${requestHost}/central-alerts/v1/list`, + {}, env, ctx2 ); @@ -190,8 +337,8 @@ describe("Central Alerts API v1", () => { it("should return valid ISO 8601 datetime", async () => { const ctx = createExecutionContext(); const response = await app.request( - "/central-alerts/v1/list", - { headers: BYPASS_CACHE }, + `https://${requestHost}/central-alerts/v1/list`, + {}, env, ctx ); @@ -209,35 +356,44 @@ describe("Central Alerts API v1", () => { }); describe("Error Cases", () => { - it("should not expose database exception details", async () => { + it("should not expose or cache database errors", async () => { + const realDb = env.DB_CENTRAL_ALERTS; env.DB_CENTRAL_ALERTS = { prepare() { throw new Error("secret schema detail"); } } as unknown as D1Database; - - const ctx = createExecutionContext(); - const response = await app.request( - "/central-alerts/v1/list", - { headers: BYPASS_CACHE }, - env, - ctx - ); - await waitOnExecutionContext(ctx); - - expect(response.status).toBe(500); - const data = (await response.json()) as { - error: { message: string; code: string }; + const requestList = async () => { + const ctx = createExecutionContext(); + const response = await app.request( + `https://${requestHost}/central-alerts/v1/list`, + {}, + env, + ctx + ); + await waitOnExecutionContext(ctx); + return response; }; - expect(data.error.message).toBe("Unable to load central alerts"); - expect(data.error.message).not.toContain("secret schema detail"); + try { + const response = await requestList(); + expect(response.status).toBe(500); + expect(response.headers.get("cache-control")).toBeNull(); + const data = (await response.json()) as { + error: { message: string; code: string }; + }; + expect(data.error.message).toBe("Unable to load central alerts"); + expect(data.error.message).not.toContain("secret schema detail"); + } finally { + env.DB_CENTRAL_ALERTS = realDb; + } + expect((await requestList()).status).toBe(200); }); it("should return 404 for unknown routes", async () => { const ctx = createExecutionContext(); const response = await app.request( - "/central-alerts/v1/unknown", - { headers: BYPASS_CACHE }, + `https://${requestHost}/central-alerts/v1/unknown`, + {}, env, ctx ); @@ -249,8 +405,8 @@ describe("Central Alerts API v1", () => { it("should redirect root path with trailing slash", async () => { const ctx = createExecutionContext(); const response = await app.request( - "/central-alerts/v1/", - { headers: BYPASS_CACHE }, + `https://${requestHost}/central-alerts/v1/`, + {}, env, ctx ); From c0e79ce540a40b1a3d51b669509e011fdddd434a Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sun, 4 Oct 2026 20:00:03 +0100 Subject: [PATCH 5/9] Rate-limit developer profile writes Add a 20-write rolling 24-hour budget for `PUT /developers/me`, enforced atomically from `developer_history` with a new `(changed_by, changed_at)` index and migration. No-op profile replays now return success without changing approval, writing audit history, or triggering catalogue revalidation, while real edits still invalidate approval as needed. Tests and service docs were updated to cover concurrency, field comparison, retained budget after deletion, and the new 429 `PROFILE_MUTATION_RATE_LIMITED` response. --- src/services/extensions/v2/README.md | 12 ++ .../extensions/v2/db/developer-profiles.ts | 89 +++++++++-- .../0024_profile_history_budget.sql | 2 + src/services/extensions/v2/db/schema.ts | 4 + .../v2/routes/developer-profiles.ts | 12 +- .../extensions/v2/developer-profiles.test.ts | 148 ++++++++++++++++++ .../v2/moderation-revalidate.test.ts | 23 +++ 7 files changed, 270 insertions(+), 20 deletions(-) create mode 100644 src/services/extensions/v2/db/migrations/0024_profile_history_budget.sql diff --git a/src/services/extensions/v2/README.md b/src/services/extensions/v2/README.md index 2c455a8c..eadce0f8 100644 --- a/src/services/extensions/v2/README.md +++ b/src/services/extensions/v2/README.md @@ -266,3 +266,15 @@ upstream failures, and cancellation do not refund attempts. Exhaustion returns Deploy migration `0024_claim_verification_budgets.sql` before the Worker update. The aggregate budget covers claims only and leaves shared GitHub capacity for other workflows; it is not a budget for every GitHub consumer. +### Profile write budget + +`PUT /developers/me` permits 20 successful profile writes per account in a +rolling 24-hour window, including creation. Exhaustion returns 429 with +`PROFILE_MUTATION_RATE_LIMITED` and a conservative `Retry-After: 86400`. +Unchanged submissions return the current profile without changing approval, +revision, audit history, or revalidating the catalogue. All five editable fields +participate in the comparison; omitted optional fields mean null. The budget is +enforced atomically with the write using indexed, immutable history and survives +profile deletion, recreation, and ownership transfer. Audit records remain +append-only; this bounds growth per account per day rather than total retention. +Apply migration `0024_profile_history_budget.sql` before deploying. diff --git a/src/services/extensions/v2/db/developer-profiles.ts b/src/services/extensions/v2/db/developer-profiles.ts index 7c4deadb..280d9266 100644 --- a/src/services/extensions/v2/db/developer-profiles.ts +++ b/src/services/extensions/v2/db/developer-profiles.ts @@ -30,6 +30,27 @@ import { UsersDatabase } from "./users"; const URL_CHECK_COOLDOWN_SECONDS = 60; +// Audit history survives profile deletion, so the budget follows the account. +const PROFILE_WRITES_PER_DAY = 20; +function profileBudgetAvailable(userId: string) { + return sql`NOT EXISTS ( + SELECT 1 FROM ${developerHistory} + WHERE ${developerHistory.changedBy} = ${userId} + AND ${developerHistory.changedAt} > datetime('now', '-1 day') + LIMIT 1 OFFSET ${PROFILE_WRITES_PER_DAY - 1} + )`; +} + +function profileMatches(row: DeveloperRow, developer: Developer) { + return ( + row.type === developer.type && + row.name === developer.name && + row.url === (developer.URL ?? null) && + row.avatarUrl === (developer.avatar_url ?? null) && + row.contactEmail === (developer.contact_email ?? null) + ); +} + type DeveloperRow = typeof developers.$inferSelect; function parseDeveloperRow(row: DeveloperRow): DeveloperProfile { @@ -144,7 +165,7 @@ export class DeveloperProfilesDatabase { developer: Developer, githubToken?: string, allowCreationAttempt: () => Promise = async () => true - ): Promise> { + ): Promise & { changed?: boolean }> { try { // Independent lookups - "does this caller already own a profile" and // "is this id taken" - so they go out together rather than costing two @@ -241,6 +262,11 @@ export class DeveloperProfilesDatabase { SELECT 1 FROM users WHERE id = ? AND deleted_at IS NULL ) + AND NOT EXISTS ( + SELECT 1 FROM developer_history + WHERE changed_by = ? AND changed_at > datetime('now', '-1 day') + LIMIT 1 OFFSET ? + ) ON CONFLICT DO NOTHING`, params: [ developer.id, @@ -254,7 +280,9 @@ export class DeveloperProfilesDatabase { githubVerificationNote, githubOrgVerified, githubUrlVerified, - userId + userId, + userId, + PROFILE_WRITES_PER_DAY - 1 ] }); } else { @@ -268,19 +296,8 @@ export class DeveloperProfilesDatabase { }; } - // approved_at is normally cleared here, even if nothing meaningful - // changed — the reviewed content just got overwritten, so the old - // approval no longer applies. Not worth diffing old vs. new field - // values for that. The one exception: a profile that's currently - // GitHub org/user verified keeps its approval across edits — that - // verification is an independently-computed identity signal (this - // write never touches githubOrgVerified, except when the id's type - // changes below) strong enough on its own that re-queuing for - // manual review on every edit isn't worth the moderator load. - // approvedRevision is bumped in lockstep with contentRevision in - // that branch so the existing approval keeps matching (see - // parseDeveloperRow) instead of silently going stale. - // + // Meaningful edits invalidate manual approval. GitHub identity + // verification keeps approval unless the profile type changes. // A type change invalidates the existing GitHub verification // outright — matchesClaimant() compares differently per type (org // membership vs. username), so a signal computed for the old type @@ -323,6 +340,16 @@ export class DeveloperProfilesDatabase { and( eq(developers.id, developer.id), eq(developers.ownerUserId, userId), + // Pin approval/verification decisions to the version read above. + eq(developers.contentRevision, existingOwn.contentRevision), + profileBudgetAvailable(userId), + sql`NOT ( + ${developers.type} IS ${developer.type} AND + ${developers.name} IS ${developer.name} AND + ${developers.url} IS ${developer.URL ?? null} AND + ${developers.avatarUrl} IS ${developer.avatar_url ?? null} AND + ${developers.contactEmail} IS ${developer.contact_email ?? null} + )`, sql`EXISTS ( SELECT 1 FROM ${users} WHERE ${users.id} = ${userId} AND ${users.deletedAt} IS NULL @@ -363,6 +390,26 @@ export class DeveloperProfilesDatabase { } if (!results[0]?.meta?.changes) { + // A replay is successful without writing, but only while this caller + // still owns the profile and has an active account. + const [unchanged] = await this.db + .select() + .from(developers) + .where( + and( + eq(developers.id, developer.id), + eq(developers.ownerUserId, userId), + sql`EXISTS (SELECT 1 FROM ${users} WHERE ${users.id} = ${userId} + AND ${users.deletedAt} IS NULL)` + ) + ); + if (!isCreating && unchanged && profileMatches(unchanged, developer)) { + return { + data: parseDeveloperRow(unchanged), + error: null, + changed: false + }; + } return { data: null, error: await this.upsertBlockedError(userId, developer.id, isCreating) @@ -387,7 +434,7 @@ export class DeveloperProfilesDatabase { } }; } - return { data: parseDeveloperRow(current), error: null }; + return { data: parseDeveloperRow(current), error: null, changed: true }; } catch (error) { return databaseError("upsertOwn", error); } @@ -436,6 +483,16 @@ export class DeveloperProfilesDatabase { } } + const [budget] = await this.db + .select({ available: profileBudgetAvailable(userId) }) + .from(users) + .where(eq(users.id, userId)); + if (budget && !budget.available) { + return { + message: "Profile write allowance exhausted; try again in 24 hours", + code: "PROFILE_MUTATION_RATE_LIMITED" + }; + } return { message: "Developer ownership changed while updating the profile", code: "CONFLICT" diff --git a/src/services/extensions/v2/db/migrations/0024_profile_history_budget.sql b/src/services/extensions/v2/db/migrations/0024_profile_history_budget.sql new file mode 100644 index 00000000..10d9323c --- /dev/null +++ b/src/services/extensions/v2/db/migrations/0024_profile_history_budget.sql @@ -0,0 +1,2 @@ +CREATE INDEX IF NOT EXISTS idx_developer_history_account_changed_at +ON developer_history (changed_by, changed_at); diff --git a/src/services/extensions/v2/db/schema.ts b/src/services/extensions/v2/db/schema.ts index c13acb7d..70775949 100644 --- a/src/services/extensions/v2/db/schema.ts +++ b/src/services/extensions/v2/db/schema.ts @@ -321,6 +321,10 @@ export const developerHistory = sqliteTable( index("idx_developer_history_developer_changed_at").on( table.developerId, table.changedAt + ), + index("idx_developer_history_account_changed_at").on( + table.changedBy, + table.changedAt ) ] ); diff --git a/src/services/extensions/v2/routes/developer-profiles.ts b/src/services/extensions/v2/routes/developer-profiles.ts index b41bc2ce..3d004fb6 100644 --- a/src/services/extensions/v2/routes/developer-profiles.ts +++ b/src/services/extensions/v2/routes/developer-profiles.ts @@ -186,7 +186,7 @@ export function registerDeveloperProfileRoutes(app: ExtensionsV2App): void { "Developer id already taken by someone else, or id was changed on an existing profile" ), 429: errorResponse( - "The account exhausted its profile-creation allowance, or GitHub verification is temporarily rate limited" + "The account exhausted its profile creation or daily write allowance, or GitHub verification is temporarily rate limited" ), 503: errorResponse("GitHub verification is temporarily unavailable"), 422: errorResponse( @@ -203,7 +203,7 @@ export function registerDeveloperProfileRoutes(app: ExtensionsV2App): void { const db = new DeveloperProfilesDatabase( getExtensionsDb(c.env.DB_EXTENSIONS) ); - const { data, error } = await db.upsertOwn( + const { data, error, changed } = await db.upsertOwn( auth.userId, body, platform.getEnv("GITHUB_TOKEN"), @@ -222,7 +222,8 @@ export function registerDeveloperProfileRoutes(app: ExtensionsV2App): void { const status = error?.code === "GITHUB_MISMATCH" || error?.code === "ACCOUNT_INACTIVE" ? 403 - : error?.code === "PROFILE_CREATION_RATE_LIMITED" + : error?.code === "PROFILE_CREATION_RATE_LIMITED" || + error?.code === "PROFILE_MUTATION_RATE_LIMITED" ? 429 : error?.code === "CONFLICT" || error?.code === "DEVELOPER_ID_TAKEN" ? 409 @@ -234,11 +235,14 @@ export function registerDeveloperProfileRoutes(app: ExtensionsV2App): void { if (error?.code === "PROFILE_CREATION_RATE_LIMITED") { response.headers.set("Retry-After", "60"); } + if (error?.code === "PROFILE_MUTATION_RATE_LIMITED") { + response.headers.set("Retry-After", "86400"); + } return response; } // Profile edits apply immediately (no moderation staging) and change // catalogue-visible fields (developer name/URL), so purge here too. - revalidateCatalogue(c); + if (changed) revalidateCatalogue(c); return c.json({ result: data }, 200); }); diff --git a/test/services/extensions/v2/developer-profiles.test.ts b/test/services/extensions/v2/developer-profiles.test.ts index abd9b9ab..7917dc60 100644 --- a/test/services/extensions/v2/developer-profiles.test.ts +++ b/test/services/extensions/v2/developer-profiles.test.ts @@ -42,6 +42,154 @@ setupExtensionsV2Tests(); describe("Extensions API v2", () => { describe("PUT /developers/me", () => { + it("coalesces identical and concurrent replays without changing approval or history", async () => { + const headers = await authHeaders("replay-owner"); + const profile = sampleDeveloper({ id: "replay-profile" }); + expect( + (await put("/extensions/v2/developers/me", headers, profile)).status + ).toBe(200); + await db + .prepare( + "UPDATE developers SET approved_at = CURRENT_TIMESTAMP, approved_revision = content_revision WHERE id = ?" + ) + .bind(profile.id) + .run(); + const before = await getDeveloper(db, profile.id); + const responses = await Promise.all( + Array.from({ length: 6 }, () => + put("/extensions/v2/developers/me", headers, profile) + ) + ); + expect(responses.map((r) => r.status)).toEqual(Array(6).fill(200)); + expect(await getDeveloper(db, profile.id)).toEqual(before); + expect(await listDeveloperHistory(db)).toHaveLength(1); + }); + + it("budgets real edits atomically and retains the account budget after deletion", async () => { + const headers = await authHeaders("budget-owner"); + const profile = sampleDeveloper({ id: "budget-profile" }); + expect( + (await put("/extensions/v2/developers/me", headers, profile)).status + ).toBe(200); + for (let i = 0; i < 18; i++) { + await db + .prepare( + "INSERT INTO developer_history (id, developer_id, type, name, changed_by) VALUES (?, ?, 'user', 'Budget', ?)" + ) + .bind(`budget-${i}`, profile.id, "budget-owner") + .run(); + } + const raced = await Promise.all( + ["Last A", "Last B"].map((name) => + put("/extensions/v2/developers/me", headers, { ...profile, name }) + ) + ); + expect(raced.filter((r) => r.status === 200)).toHaveLength(1); + expect(raced.filter((r) => r.status !== 200)).toHaveLength(1); + expect(await listDeveloperHistory(db)).toHaveLength(20); + const current = await getDeveloper(db, profile.id); + expect( + ( + await put("/extensions/v2/developers/me", headers, { + ...profile, + name: current!.name + }) + ).status + ).toBe(200); + const denied = await put("/extensions/v2/developers/me", headers, { + ...profile, + name: "Over quota" + }); + expect(denied.status).toBe(429); + expect(denied.headers.get("Retry-After")).toBe("86400"); + expect(await denied.json()).toMatchObject({ + error: { code: "PROFILE_MUTATION_RATE_LIMITED" } + }); + expect((await del("/extensions/v2/developers/me", headers)).status).toBe( + 200 + ); + expect( + (await put("/extensions/v2/developers/me", headers, profile)).status + ).toBe(429); + expect(await listDeveloperHistory(db)).toHaveLength(20); + expect( + ( + await put( + "/extensions/v2/developers/me", + await authHeaders("independent-owner"), + sampleDeveloper({ id: "independent-profile" }) + ) + ).status + ).toBe(200); + }); + + it("allows writes after old history leaves the window and audits all editable fields", async () => { + const headers = await authHeaders("expired-budget-owner"); + let profile = sampleDeveloper({ id: "expired-profile" }); + for (let i = 0; i < 20; i++) { + await db + .prepare( + "INSERT INTO developer_history (id, developer_id, type, name, changed_by, changed_at) VALUES (?, ?, 'user', 'Old', ?, datetime('now', '-2 days'))" + ) + .bind(`old-${i}`, profile.id, "expired-budget-owner") + .run(); + } + expect( + (await put("/extensions/v2/developers/me", headers, profile)).status + ).toBe(200); + for (const edit of [ + { name: "New name" }, + { type: "organization" as const }, + { URL: "https://example.com/new" }, + { avatar_url: "https://example.com/avatar.png" }, + { contact_email: "new@example.com" } + ]) { + profile = { ...profile, ...edit }; + expect( + (await put("/extensions/v2/developers/me", headers, profile)).status + ).toBe(200); + } + expect(await listDeveloperHistory(db)).toHaveLength(26); + expect((await getDeveloper(db, profile.id))?.content_revision).toBe(6); + }); + + it("treats omitted optional fields as null and audits clearing them once", async () => { + const headers = await authHeaders("null-profile-owner"); + const profile = { + id: "null-profile", + type: "user", + name: "Null profile" + }; + expect( + (await put("/extensions/v2/developers/me", headers, profile)).status + ).toBe(200); + expect( + (await put("/extensions/v2/developers/me", headers, profile)).status + ).toBe(200); + expect(await listDeveloperHistory(db)).toHaveLength(1); + expect( + ( + await put("/extensions/v2/developers/me", headers, { + ...profile, + URL: "https://example.com", + avatar_url: "https://example.com/avatar", + contact_email: "owner@example.com" + }) + ).status + ).toBe(200); + expect( + (await put("/extensions/v2/developers/me", headers, profile)).status + ).toBe(200); + expect( + (await put("/extensions/v2/developers/me", headers, profile)).status + ).toBe(200); + expect(await listDeveloperHistory(db)).toHaveLength(3); + const current = await getDeveloper(db, profile.id); + expect(current?.url).toBeNull(); + expect(current?.avatar_url).toBeNull(); + expect(current?.contact_email).toBeNull(); + }); + it("limits creation attempts per account before GitHub and database writes", async () => { mockGithubEntity("Organization"); const firstHeaders = await authHeaders("rate-limited-account"); diff --git a/test/services/extensions/v2/moderation-revalidate.test.ts b/test/services/extensions/v2/moderation-revalidate.test.ts index 64b5b6e5..51a12bc1 100644 --- a/test/services/extensions/v2/moderation-revalidate.test.ts +++ b/test/services/extensions/v2/moderation-revalidate.test.ts @@ -82,6 +82,29 @@ async function delist(as: string): Promise { } describe("CDN cache revalidation on catalogue mutations", () => { + it("does not revalidate unchanged profile replays", async () => { + const fetcher = stubFrontend(); + const headers = await authHeaders("noop-revalidate-owner"); + const profile = sampleDeveloper({ id: "noop-revalidate-profile" }); + expect( + (await put("/extensions/v2/developers/me", headers, profile)).status + ).toBe(200); + expect(fetcher.fetch).toHaveBeenCalledTimes(1); + expect( + (await put("/extensions/v2/developers/me", headers, profile)).status + ).toBe(200); + expect(fetcher.fetch).toHaveBeenCalledTimes(1); + expect( + ( + await put("/extensions/v2/developers/me", headers, { + ...profile, + name: "Changed" + }) + ).status + ).toBe(200); + expect(fetcher.fetch).toHaveBeenCalledTimes(2); + }); + it("purges the catalogue tags after a successful delist", async () => { await seedModAndExtension(); const fetcher = stubFrontend(); From 45b20a64a63bd8d711c0051a0c308bfe832f2a2e Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sun, 4 Oct 2026 20:26:43 +0100 Subject: [PATCH 6/9] Cache public GETs despite auth headers Add a new `publicResponseCache` helper that strips `Authorization` only for cache keying, then restores the original request for handlers and error paths. Versions, stats, and central-alerts now use this middleware so public responses still hit edge cache even when clients send unused credentials, and cached routes apply `etag()` before caching to keep conditional requests working on cache hits. Tests were updated to reflect the new behavior, including a shared `isolateEdgeCache` utility that namespaces Cache API entries per test to avoid cross-test contamination. Service READMEs were also updated to document that unused Authorization headers no longer bypass public edge caching. --- src/lib/cache.ts | 30 ++++- src/services/central-alerts/v1/README.md | 2 + src/services/stats/v1/README.md | 2 + src/services/stats/v1/index.ts | 7 +- src/services/versions/v1/README.md | 2 +- src/services/versions/v1/index.ts | 13 +- test/app/index.test.ts | 9 +- test/integration/app.test.ts | 9 +- test/integration/versions/index.test.ts | 4 + test/lib/cache.test.ts | 74 ++++++++++- test/services/stats/v1/index.test.ts | 56 ++++++-- test/services/versions/v1/errors.test.ts | 41 +++--- test/services/versions/v1/index.test.ts | 131 ++++++++++++------- test/services/versions/v1/middleware.test.ts | 44 ++++--- test/utils/isolate-edge-cache.ts | 16 +++ 15 files changed, 325 insertions(+), 115 deletions(-) create mode 100644 test/utils/isolate-edge-cache.ts diff --git a/src/lib/cache.ts b/src/lib/cache.ts index 2d8779df..19a83307 100644 --- a/src/lib/cache.ts +++ b/src/lib/cache.ts @@ -1,4 +1,5 @@ -import type { Context } from "hono"; +import type { Context, MiddlewareHandler } from "hono"; +import { cache } from "hono/cache"; export function normalizePublicCacheKey(url: string): string { const cacheUrl = new URL(url); @@ -28,3 +29,30 @@ export function singleFlight( inflight.set(key, promise); return promise; } + +// Only for public GET representations that never depend on credentials. +// Keep Hono's key/Vary and response privacy safeguards, but prevent an +// unused Authorization header from turning off backend-protective caching. +// Downstream handlers always see the original request (including credentials). +export function publicResponseCache( + options: Parameters[0] +): MiddlewareHandler { + const middleware = cache(options); + return async (c, next) => { + const original = c.req.raw; + if (c.req.method !== "GET" || !original.headers.has("Authorization")) { + return middleware(c, next); + } + const headers = new Headers(original.headers); + headers.delete("Authorization"); + c.req.raw = new Request(original, { headers }); + try { + return await middleware(c, async () => { + c.req.raw = original; + await next(); + }); + } finally { + c.req.raw = original; + } + }; +} diff --git a/src/services/central-alerts/v1/README.md b/src/services/central-alerts/v1/README.md index f9a71e0e..7f155499 100644 --- a/src/services/central-alerts/v1/README.md +++ b/src/services/central-alerts/v1/README.md @@ -67,3 +67,5 @@ The `type` field accepts: `success`, `info`, `warning`, `danger` ## Database Uses D1 database binding `DB_CENTRAL_ALERTS`. Initialize with the setup script in `src/services/central-alerts/v1/scripts/`. + +Public GET responses use the edge cache even when an unused Authorization header is supplied; these representations do not depend on credentials. diff --git a/src/services/stats/v1/README.md b/src/services/stats/v1/README.md index 88a51f5f..1d4f9394 100644 --- a/src/services/stats/v1/README.md +++ b/src/services/stats/v1/README.md @@ -24,3 +24,5 @@ Returns the aggregated statistics behind those charts as JSON. ## Caching Stats are cached with a 24-hour TTL and follow the same caching patterns as the versions service, including its graceful handling of GitHub API errors — a failed refresh serves the previous data rather than erroring. + +Public GET responses use the edge cache even when an unused Authorization header is supplied; these representations do not depend on credentials. diff --git a/src/services/stats/v1/index.ts b/src/services/stats/v1/index.ts index 97a04e86..62f330b0 100644 --- a/src/services/stats/v1/index.ts +++ b/src/services/stats/v1/index.ts @@ -1,5 +1,4 @@ import { Hono } from "hono"; -import { cache } from "hono/cache"; import { cors } from "hono/cors"; import { etag } from "hono/etag"; import { prettyJSON } from "hono/pretty-json"; @@ -10,7 +9,7 @@ import { Releases } from "../../versions/v1/interfaces"; import { StatsData, ReleasesPerYearData } from "./interfaces"; import { getPlatform } from "../../../lib/middleware"; import { ICache } from "../../../lib/interfaces"; -import { publicCacheKey } from "../../../lib/cache"; +import { publicCacheKey, publicResponseCache } from "../../../lib/cache"; import { logError, logInfo } from "../../../lib/logger"; import { GitHubError } from "../../../lib/github-errors"; @@ -37,7 +36,9 @@ function registerCachedRoute

( ) { return statsV1.get( path, - cache({ + // Honor conditional requests on cache hits; the inner etag stamps entries. + etag(), + publicResponseCache({ cacheName: STATS_CACHE_NAME, cacheControl: STATS_CACHE_CONTROL, keyGenerator: publicCacheKey diff --git a/src/services/versions/v1/README.md b/src/services/versions/v1/README.md index 3bd3c347..a9f51fc8 100644 --- a/src/services/versions/v1/README.md +++ b/src/services/versions/v1/README.md @@ -4,7 +4,7 @@ Provides release metadata from the FOSSBilling GitHub repo. Responses are cached in `CACHE_KV` for 24 hours. -Successful GET responses are also edge-cached (Cache API, per-PoP) for 5 minutes, so repeat requests don't wake the Worker at all; the window deliberately bounds how long a post-`/update` refresh takes to become visible everywhere. Authorization-bearing requests and non-200s bypass that cache, and 4xx/5xx responses are stamped `Cache-Control: no-store` so a transient failure can't be pinned client-side. `/`, `/latest`, and `/{version}` bodies vary by mirror trust (derived from `User-Agent`), which they declare with `Vary: User-Agent`; the edge cache folds that header into its key. +Successful GET responses are also edge-cached (Cache API, per-PoP) for 5 minutes, so repeat requests don't wake the Worker at all; the window deliberately bounds how long a post-`/update` refresh takes to become visible everywhere. Unused Authorization headers do not bypass this public cache. Non-200s bypass it, and 4xx/5xx responses are stamped `Cache-Control: no-store` so a transient failure can't be pinned client-side. `/`, `/latest`, and `/{version}` bodies vary by mirror trust (derived from `User-Agent`), which they declare with `Vary: User-Agent`; the edge cache folds that header into its key. ## Authentication diff --git a/src/services/versions/v1/index.ts b/src/services/versions/v1/index.ts index 52d26034..a93c4e32 100644 --- a/src/services/versions/v1/index.ts +++ b/src/services/versions/v1/index.ts @@ -1,6 +1,5 @@ import { bearerAuth } from "hono/bearer-auth"; import { Hono, type Context, type Handler } from "hono"; -import { cache } from "hono/cache"; import { cors } from "hono/cors"; import { etag } from "hono/etag"; import { prettyJSON } from "hono/pretty-json"; @@ -17,7 +16,11 @@ import { Releases, ReleaseDetails, ResolvedReleaseDetails } from "./interfaces"; import { getReleaseR2Object, ReleaseR2Object } from "./r2"; import { getPlatform } from "../../../lib/middleware"; import { ICache } from "../../../lib/interfaces"; -import { publicCacheKey, singleFlight } from "../../../lib/cache"; +import { + publicCacheKey, + publicResponseCache, + singleFlight +} from "../../../lib/cache"; import { logError, logWarn, logInfo } from "../../../lib/logger"; import { GitHubError, @@ -80,7 +83,7 @@ async function getUpdateToken(cache: ICache): Promise { // auto-cache Worker responses regardless of Cache-Control, so without this // every request wakes the isolate and pays the KV read + parse + stringify // + etag hash for a payload that only changes when a release lands. The -// middleware skips Authorization-bearing requests and caches only 200s. +// middleware ignores unused Authorization headers and caches only 200s. const VERSIONS_CACHE_NAME = "versions-api-v1"; // Transient failures must not be client-cached: FOSSBilling's Update.php @@ -121,7 +124,9 @@ function registerCachedRoute

( ) { return versionsV1.get( path, - cache({ + // Honor conditional requests on cache hits; the inner etag stamps entries. + etag(), + publicResponseCache({ cacheName: VERSIONS_CACHE_NAME, cacheControl: RELEASES_CACHE_CONTROL, keyGenerator, diff --git a/test/app/index.test.ts b/test/app/index.test.ts index 8c59eab3..b92dc20c 100644 --- a/test/app/index.test.ts +++ b/test/app/index.test.ts @@ -1,3 +1,4 @@ +import { isolateEdgeCache } from "../utils/isolate-edge-cache"; /** * Tests for FOSSBilling API Worker - Main Application * @@ -12,10 +13,7 @@ import { } from "cloudflare:test"; import app from "../../src/app/index"; -// Requests carrying an Authorization header bypass hono's cache middleware so -// tests that assert KV writes or handler runs reach the live handler rather -// than an edge-cached response from an earlier test. /update calls keep -// their exact auth headers and are excluded. +// Arbitrary credentials use the public cache; isolate entries between tests. const BYPASS_CACHE = { authorization: "test-bypass-cache" } as const; import { ApiResponse, @@ -72,6 +70,8 @@ const mockComposerJson = { } }; +const resetEdgeCache = isolateEdgeCache(); + describe("FOSSBilling API Worker - Main App", () => { beforeAll(applyTestMigrations); @@ -86,6 +86,7 @@ describe("FOSSBilling API Worker - Main App", () => { // Reset all mocks vi.clearAllMocks(); + resetEdgeCache(); // Setup default GitHub API mock responses (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( diff --git a/test/integration/app.test.ts b/test/integration/app.test.ts index 60766e66..05f6535d 100644 --- a/test/integration/app.test.ts +++ b/test/integration/app.test.ts @@ -1,3 +1,4 @@ +import { isolateEdgeCache } from "../utils/isolate-edge-cache"; import { describe, it, expect, beforeAll, beforeEach, vi } from "vitest"; import { createExecutionContext, @@ -6,10 +7,7 @@ import { import { env } from "cloudflare:workers"; import app from "../../src/app/index"; -// Requests carrying an Authorization header bypass hono's cache middleware so -// tests that assert KV writes or handler runs reach the live handler rather -// than an edge-cached response from an earlier test. /update calls keep -// their exact auth headers and are excluded. +// Arbitrary credentials use the public cache; isolate entries between tests. const BYPASS_CACHE = { authorization: "test-bypass-cache" } as const; import { mockGitHubReleases, mockComposerJson } from "../mocks/github-releases"; import { setupGitHubApiMock } from "../utils/mock-helpers"; @@ -40,6 +38,8 @@ import { request as ghRequest } from "@octokit/request"; import { graphql } from "@octokit/graphql"; import { resetUpdateTokenCache } from "../../src/services/versions/v1/index"; +const resetEdgeCache = isolateEdgeCache(); + describe("FOSSBilling API Worker - Full App Integration", () => { beforeAll(applyTestMigrations); @@ -49,6 +49,7 @@ describe("FOSSBilling API Worker - Full App Integration", () => { await env.AUTH_KV.put("UPDATE_TOKEN", "test-update-token-12345"); vi.clearAllMocks(); + resetEdgeCache(); setupGitHubApiMock( vi.mocked(ghRequest) as MockGitHubRequest, vi.mocked(graphql) as unknown as MockGitHubGraphQL, diff --git a/test/integration/versions/index.test.ts b/test/integration/versions/index.test.ts index 64065a10..1a64384c 100644 --- a/test/integration/versions/index.test.ts +++ b/test/integration/versions/index.test.ts @@ -1,3 +1,4 @@ +import { isolateEdgeCache } from "../../utils/isolate-edge-cache"; import { describe, it, expect, beforeEach, vi } from "vitest"; import { createExecutionContext, @@ -37,6 +38,8 @@ import { request as ghRequest } from "@octokit/request"; import { graphql } from "@octokit/graphql"; import { resetUpdateTokenCache } from "../../../src/services/versions/v1/index"; +const resetEdgeCache = isolateEdgeCache(); + describe("Versions API v1 - Integration Tests", () => { beforeEach(async () => { await env.CACHE_KV.delete("gh-fossbilling-releases"); @@ -44,6 +47,7 @@ describe("Versions API v1 - Integration Tests", () => { await env.AUTH_KV.put("UPDATE_TOKEN", "test-update-token-12345"); vi.resetAllMocks(); + resetEdgeCache(); setupGitHubApiMock( vi.mocked(ghRequest) as MockGitHubRequest, vi.mocked(graphql) as unknown as MockGitHubGraphQL, diff --git a/test/lib/cache.test.ts b/test/lib/cache.test.ts index 6d02af5a..9c14bf37 100644 --- a/test/lib/cache.test.ts +++ b/test/lib/cache.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "vitest"; -import { normalizePublicCacheKey } from "../../src/lib/cache"; +import { Hono } from "hono"; +import { + normalizePublicCacheKey, + publicResponseCache +} from "../../src/lib/cache"; describe("cache helpers", () => { it("normalizes public cache keys by removing query strings and fragments", () => { @@ -18,3 +22,71 @@ describe("cache helpers", () => { ); }); }); + +describe("public response cache", () => { + it("preserves the original request for live handlers and error handling", async () => { + const app = new Hono(); + const original = new Request("https://example.test/public", { + headers: { Authorization: "x" } + }); + app.get( + "/public", + publicResponseCache({ cacheName: crypto.randomUUID(), wait: true }), + (c) => { + expect(c.req.raw).toBe(original); + expect(c.req.header("Authorization")).toBe("x"); + throw new Error("handler failed"); + } + ); + app.onError((_error, c) => { + expect(c.req.raw).toBe(original); + return c.text("failure", 500); + }); + expect((await app.fetch(original)).status).toBe(500); + }); + + it("restores the request when cache key generation fails", async () => { + const app = new Hono(); + const original = new Request("https://example.test/public", { + headers: { Authorization: "x" } + }); + app.get( + "/public", + publicResponseCache({ + cacheName: crypto.randomUUID(), + keyGenerator: () => { + throw new Error("key failed"); + } + }), + (c) => c.text("unused") + ); + app.onError((_error, c) => { + expect(c.req.raw).toBe(original); + return c.text("failure", 500); + }); + expect((await app.fetch(original)).status).toBe(500); + }); + + it.each(["no-store", "private", "no-cache"])( + "keeps %s responses out of the cache", + async (control) => { + const app = new Hono(); + let calls = 0; + app.get( + "/public", + publicResponseCache({ cacheName: crypto.randomUUID(), wait: true }), + (c) => { + calls++; + c.header("Cache-Control", control); + return c.text("public"); + } + ); + for (let i = 0; i < 2; i++) { + await app.request("https://example.test/public", { + headers: { Authorization: "x" } + }); + } + expect(calls).toBe(2); + } + ); +}); diff --git a/test/services/stats/v1/index.test.ts b/test/services/stats/v1/index.test.ts index 4bf7f86c..ff48adff 100644 --- a/test/services/stats/v1/index.test.ts +++ b/test/services/stats/v1/index.test.ts @@ -1,3 +1,4 @@ +import { isolateEdgeCache } from "../../../utils/isolate-edge-cache"; import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { env, @@ -50,12 +51,11 @@ vi.mock("@octokit/graphql", () => ({ let restoreConsole: (() => void) | null = null; -// Requests carrying an Authorization header bypass the hono cache middleware -// entirely, so tests that mock specific GitHub responses and assert on the -// handler output are guaranteed to execute the handler rather than read an -// entry a previous test cached. The cache path itself is covered by -// "should cache statistics data", which deliberately omits the header. -const NO_CACHE_HEADERS = { authorization: "test-bypass-cache" } as const; +// Arbitrary credentials must behave like anonymous public requests. +// Edge entries are cleared between tests to exercise live handlers. +const PUBLIC_HEADERS = { authorization: "test-bypass-cache" } as const; + +const resetEdgeCache = isolateEdgeCache(); describe("Stats API v1", () => { beforeEach(async () => { @@ -68,6 +68,7 @@ describe("Stats API v1", () => { await env.AUTH_KV.put("UPDATE_TOKEN", testUpdateToken); vi.clearAllMocks(); + resetEdgeCache(); setupGitHubApiMock( vi.mocked(ghRequest) as MockGitHubRequest, vi.mocked(graphql) as unknown as MockGitHubGraphQL, @@ -80,12 +81,47 @@ describe("Stats API v1", () => { if (restoreConsole) restoreConsole(); }); + it.each(["/stats/v1", "/stats/v1/data"])( + "shares the public edge entry across credentials for %s", + async (path) => { + const requestAs = async (authorization?: string) => { + const ctx = createExecutionContext(); + const response = await app.request( + path, + { + headers: authorization === undefined ? {} : { authorization } + }, + env, + ctx + ); + await waitOnExecutionContext(ctx); + return response; + }; + const first = await requestAs("Bearer arbitrary-cold"); + expect(first.status).toBe(200); + const body = await first.text(); + const get = vi + .spyOn(env.CACHE_KV, "get") + .mockRejectedValue(new Error("backend must not be read")); + try { + for (const authorization of [undefined, "x", "Bearer other", ""]) { + const response = await requestAs(authorization); + expect(response.status).toBe(200); + await expect(response.text()).resolves.toBe(body); + } + expect(get).not.toHaveBeenCalled(); + } finally { + get.mockRestore(); + } + } + ); + describe("GET /stats/v1/data", () => { it("should return aggregated statistics", async () => { const ctx = createExecutionContext(); const response = await app.fetch( new Request("http://localhost/stats/v1/data", { - headers: NO_CACHE_HEADERS + headers: PUBLIC_HEADERS }), env, ctx @@ -176,7 +212,7 @@ describe("Stats API v1", () => { const ctx = createExecutionContext(); const response = await app.fetch( new Request("http://localhost/stats/v1/data", { - headers: NO_CACHE_HEADERS + headers: PUBLIC_HEADERS }), env, ctx @@ -247,7 +283,7 @@ describe("Stats API v1", () => { const ctx = createExecutionContext(); const response = await app.fetch( new Request("http://localhost/stats/v1/data", { - headers: NO_CACHE_HEADERS + headers: PUBLIC_HEADERS }), env, ctx @@ -310,7 +346,7 @@ describe("Stats API v1", () => { const ctx = createExecutionContext(); const response = await app.fetch( new Request("http://localhost/stats/v1/data", { - headers: NO_CACHE_HEADERS + headers: PUBLIC_HEADERS }), env, ctx diff --git a/test/services/versions/v1/errors.test.ts b/test/services/versions/v1/errors.test.ts index 3567ff62..00a070e4 100644 --- a/test/services/versions/v1/errors.test.ts +++ b/test/services/versions/v1/errors.test.ts @@ -1,3 +1,4 @@ +import { isolateEdgeCache } from "../../../utils/isolate-edge-cache"; import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { createExecutionContext, @@ -39,13 +40,14 @@ import { request as ghRequest } from "@octokit/request"; import { graphql } from "@octokit/graphql"; import { resetUpdateTokenCache } from "../../../../src/services/versions/v1/index"; -// Requests carrying an Authorization header bypass hono's cache middleware -// (same pattern as index.test.ts) so each test sees live handler output -// instead of a response an earlier test cached. -const BYPASS_CACHE = { authorization: "test-bypass-cache" } as const; +// Arbitrary credentials must behave like anonymous public requests. +// Edge entries are cleared between tests to exercise live handlers. +const PUBLIC_HEADERS = { authorization: "test-bypass-cache" } as const; let restoreConsole: (() => void) | null = null; +const resetEdgeCache = isolateEdgeCache(); + describe("Versions API v1 - Error Handling", () => { beforeEach(async () => { restoreConsole = suppressConsole(); @@ -54,6 +56,7 @@ describe("Versions API v1 - Error Handling", () => { await env.AUTH_KV.put("UPDATE_TOKEN", "test-update-token-12345"); vi.resetAllMocks(); + resetEdgeCache(); (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( async (route: string) => { if (route === "GET /repos/{owner}/{repo}/releases") { @@ -232,7 +235,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -272,7 +275,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -297,7 +300,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -341,7 +344,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -364,7 +367,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -385,7 +388,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -400,7 +403,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -415,7 +418,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -433,7 +436,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -476,7 +479,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -518,7 +521,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -561,7 +564,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -580,7 +583,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/0.5.0", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -601,7 +604,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/0.5.0", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -622,7 +625,7 @@ describe("Versions API v1 - Error Handling", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/0.5.0", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); diff --git a/test/services/versions/v1/index.test.ts b/test/services/versions/v1/index.test.ts index 64a10541..09b3ba96 100644 --- a/test/services/versions/v1/index.test.ts +++ b/test/services/versions/v1/index.test.ts @@ -1,3 +1,4 @@ +import { isolateEdgeCache } from "../../../utils/isolate-edge-cache"; import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { compare as semverCompare } from "semver"; import { @@ -27,12 +28,9 @@ import { VersionsResponse } from "../../../utils/test-types"; -// Requests carrying an Authorization header bypass hono's cache middleware -// (same pattern as the stats tests) so each test sees live handler output -// instead of a response a previous test cached. The cache path itself is -// covered by the dedicated "edge cache" describe block, which omits the -// header. -const BYPASS_CACHE = { authorization: "test-bypass-cache" } as const; +// Arbitrary credentials must behave like anonymous public requests. +// Edge entries are cleared between tests to exercise live handlers. +const PUBLIC_HEADERS = { authorization: "test-bypass-cache" } as const; vi.mock("@octokit/request", () => { const endpoint = { DEFAULTS: {} }; @@ -55,6 +53,8 @@ import { resetUpdateTokenCache } from "../../../../src/services/versions/v1/inde let restoreConsole: (() => void) | null = null; let originalKVPut: typeof env.CACHE_KV.put | null = null; +const resetEdgeCache = isolateEdgeCache(); + describe("Versions API v1", () => { beforeEach(async () => { restoreConsole = suppressConsole(); @@ -68,6 +68,7 @@ describe("Versions API v1", () => { await env.AUTH_KV.put("UPDATE_TOKEN", testUpdateToken); vi.clearAllMocks(); + resetEdgeCache(); setupGitHubApiMock( vi.mocked(ghRequest) as MockGitHubRequest, vi.mocked(graphql) as unknown as MockGitHubGraphQL, @@ -92,7 +93,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -113,7 +114,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -170,7 +171,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -188,7 +189,7 @@ describe("Versions API v1", () => { it("should cache releases data", async () => { const ctx = createExecutionContext(); - await app.request("/versions/v1", { headers: BYPASS_CACHE }, env, ctx); + await app.request("/versions/v1", { headers: PUBLIC_HEADERS }, env, ctx); await waitOnExecutionContext(ctx); const cached = await env.CACHE_KV.get("gh-fossbilling-releases"); @@ -198,7 +199,7 @@ describe("Versions API v1", () => { it("should return cached data on subsequent requests", async () => { const ctx1 = createExecutionContext(); - await app.request("/versions/v1", { headers: BYPASS_CACHE }, env, ctx1); + await app.request("/versions/v1", { headers: PUBLIC_HEADERS }, env, ctx1); await waitOnExecutionContext(ctx1); ( @@ -208,7 +209,7 @@ describe("Versions API v1", () => { const ctx2 = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx2 ); @@ -225,7 +226,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/latest", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -300,7 +301,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "FOSSBilling/0.8.7", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -309,7 +310,9 @@ describe("Versions API v1", () => { await waitOnExecutionContext(ctx); expect(response.status).toBe(200); - expect(response.headers.get("Vary")).toContain("User-Agent"); + expect(response.headers.get("Vary")?.toLowerCase()).toContain( + "user-agent" + ); const data: ApiResponse = await response.json(); if (!data.result) { throw new Error("Expected latest release data"); @@ -329,7 +332,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "FOSSBilling/0.8.7", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -373,7 +376,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "FOSSBilling/0.8.7", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -402,7 +405,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "FOSSBilling/0.8.6", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -430,7 +433,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/latest", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -454,7 +457,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "curl/8.0.0", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -480,7 +483,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "FOSSBilling/0.8.7", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -509,7 +512,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "FOSSBilling/0.8.6", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -532,7 +535,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "FOSSBilling/0.8.7", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -564,7 +567,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "FOSSBilling/0.8.7", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -587,7 +590,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "FOSSBilling/0.8.6", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -645,7 +648,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "FOSSBilling/0.8.7", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -704,7 +707,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "FOSSBilling/0.8.6", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -731,7 +734,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "FOSSBilling/0.8.7", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -791,7 +794,7 @@ describe("Versions API v1", () => { { headers: { "User-Agent": "FOSSBilling/0.8.7", - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -820,7 +823,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/0.5.0", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -842,7 +845,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/999.999.999", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -860,7 +863,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/latest", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -881,7 +884,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/build_changelog/0.5.0", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -900,7 +903,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/build_changelog/0.6.0", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -916,7 +919,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/build_changelog/invalid", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -936,7 +939,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/count", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -954,14 +957,14 @@ describe("Versions API v1", () => { it("should serve cached count when available", async () => { const ctx1 = createExecutionContext(); - await app.request("/versions/v1", { headers: BYPASS_CACHE }, env, ctx1); + await app.request("/versions/v1", { headers: PUBLIC_HEADERS }, env, ctx1); await waitOnExecutionContext(ctx1); // Make a second request - should use cache const ctx2 = createExecutionContext(); const response = await app.request( "/versions/v1/count", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx2 ); @@ -1100,7 +1103,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -1121,7 +1124,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/0.5.0", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -1153,7 +1156,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/0.6.0", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -1183,7 +1186,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/0.6.0", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -1210,7 +1213,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/latest", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -1241,7 +1244,7 @@ describe("Versions API v1", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/latest", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -1276,7 +1279,7 @@ describe("Versions API v1", () => { ); const ctx = createExecutionContext(); - await app.request("/versions/v1", { headers: BYPASS_CACHE }, env, ctx); + await app.request("/versions/v1", { headers: PUBLIC_HEADERS }, env, ctx); await waitOnExecutionContext(ctx); expect(env.CACHE_KV.put).toHaveBeenCalled(); @@ -1287,8 +1290,7 @@ describe("Versions API v1", () => { expect(putCall![2]!).toHaveProperty("expirationTtl", 86400); }); - // Edge (Cache API) response caching. These tests deliberately omit - // BYPASS_CACHE so the hono cache middleware participates; each test + // Edge (Cache API) response caching. Each test // uses a different route so it can't observe a response cached by // another test (or by an earlier run of its own, on retry). describe("edge cache", () => { @@ -1310,6 +1312,41 @@ describe("Versions API v1", () => { await expect(second.text()).resolves.toBe(firstBody); }); + it.each(["", "/latest", "/count", "/0.6.0", "/build_changelog/0.5.0"])( + "shares the public edge entry across credentials for %s", + async (path) => { + const requestAs = async (authorization?: string) => { + const ctx = createExecutionContext(); + const response = await app.request( + `/versions/v1${path}`, + { + headers: authorization === undefined ? {} : { authorization } + }, + env, + ctx + ); + await waitOnExecutionContext(ctx); + return response; + }; + const first = await requestAs("Bearer arbitrary-cold"); + expect(first.status).toBe(200); + const body = await first.text(); + const get = vi + .spyOn(env.CACHE_KV, "get") + .mockRejectedValue(new Error("backend must not be read")); + try { + for (const authorization of [undefined, "x", "Bearer other", ""]) { + const response = await requestAs(authorization); + expect(response.status).toBe(200); + await expect(response.text()).resolves.toBe(body); + } + expect(get).not.toHaveBeenCalled(); + } finally { + get.mockRestore(); + } + } + ); + it("keys mirror-trust variants separately on the same URL", async () => { setupGitHubApiMock( vi.mocked(ghRequest) as MockGitHubRequest, diff --git a/test/services/versions/v1/middleware.test.ts b/test/services/versions/v1/middleware.test.ts index 80cd5a19..03c6a69d 100644 --- a/test/services/versions/v1/middleware.test.ts +++ b/test/services/versions/v1/middleware.test.ts @@ -1,3 +1,4 @@ +import { isolateEdgeCache } from "../../../utils/isolate-edge-cache"; import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { createExecutionContext, @@ -36,14 +37,14 @@ import { request as ghRequest } from "@octokit/request"; import { graphql } from "@octokit/graphql"; import { resetUpdateTokenCache } from "../../../../src/services/versions/v1/index"; -// Requests carrying an Authorization header bypass hono's cache middleware, -// so each test exercises the full middleware chain on a live handler run -// (the /update tests are excluded - their auth semantics depend on the -// exact Authorization header). /update itself is never edge-cached. -const BYPASS_CACHE = { authorization: "test-bypass-cache" } as const; +// Arbitrary credentials must behave like anonymous public requests. +// Edge entries are cleared between tests to exercise live handlers. +const PUBLIC_HEADERS = { authorization: "test-bypass-cache" } as const; let restoreConsole: (() => void) | null = null; +const resetEdgeCache = isolateEdgeCache(); + describe("Versions API v1 - Middleware", () => { beforeEach(async () => { restoreConsole = suppressConsole(); @@ -52,6 +53,7 @@ describe("Versions API v1 - Middleware", () => { await env.AUTH_KV.put("UPDATE_TOKEN", "test-update-token-12345"); vi.clearAllMocks(); + resetEdgeCache(); setupGitHubApiMock( vi.mocked(ghRequest) as MockGitHubRequest, vi.mocked(graphql) as unknown as MockGitHubGraphQL, @@ -72,7 +74,7 @@ describe("Versions API v1 - Middleware", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -85,7 +87,7 @@ describe("Versions API v1 - Middleware", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/nonexistent-version", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -116,7 +118,7 @@ describe("Versions API v1 - Middleware", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -132,7 +134,7 @@ describe("Versions API v1 - Middleware", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -145,7 +147,7 @@ describe("Versions API v1 - Middleware", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/latest/", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -163,7 +165,7 @@ describe("Versions API v1 - Middleware", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -178,7 +180,7 @@ describe("Versions API v1 - Middleware", () => { const ctx1 = createExecutionContext(); const response1 = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx1 ); @@ -188,7 +190,7 @@ describe("Versions API v1 - Middleware", () => { const ctx2 = createExecutionContext(); const response2 = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx2 ); @@ -202,7 +204,7 @@ describe("Versions API v1 - Middleware", () => { const ctx1 = createExecutionContext(); const response1 = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx1 ); @@ -216,7 +218,7 @@ describe("Versions API v1 - Middleware", () => { { headers: { "If-None-Match": etag as string, - authorization: BYPASS_CACHE.authorization + authorization: PUBLIC_HEADERS.authorization } }, env, @@ -233,7 +235,7 @@ describe("Versions API v1 - Middleware", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -248,7 +250,7 @@ describe("Versions API v1 - Middleware", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/latest", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -262,7 +264,7 @@ describe("Versions API v1 - Middleware", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/build_changelog/0.5.0", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -280,7 +282,7 @@ describe("Versions API v1 - Middleware", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -357,7 +359,7 @@ describe("Versions API v1 - Middleware", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); @@ -374,7 +376,7 @@ describe("Versions API v1 - Middleware", () => { const ctx = createExecutionContext(); const response = await app.request( "/versions/v1/", - { headers: BYPASS_CACHE }, + { headers: PUBLIC_HEADERS }, env, ctx ); diff --git a/test/utils/isolate-edge-cache.ts b/test/utils/isolate-edge-cache.ts new file mode 100644 index 00000000..39452d60 --- /dev/null +++ b/test/utils/isolate-edge-cache.ts @@ -0,0 +1,16 @@ +import { afterEach, vi } from "vitest"; + +// Cloudflare has no CacheStorage.delete(). Give each test a fresh namespace +// while exercising the real Cache API, including waitUntil cache writes. +export function isolateEdgeCache() { + const open = caches.open.bind(caches); + let restore: () => void; + afterEach(() => restore()); + return () => { + const suffix = crypto.randomUUID(); + const spy = vi + .spyOn(caches, "open") + .mockImplementation((name) => open(`${name}-${suffix}`)); + restore = () => spy.mockRestore(); + }; +} From d8d91ff699d969c983e3d111feff43637e5a4ffe Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sun, 4 Oct 2026 20:32:31 +0100 Subject: [PATCH 7/9] Tighten auth and user input checks --- src/lib/auth/bearer-assertion.ts | 15 ++++++++------- src/services/central-alerts/v1/README.md | 2 -- src/services/extensions/v2/README.md | 3 +++ src/services/extensions/v2/db/users.ts | 17 +++++++++-------- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/lib/auth/bearer-assertion.ts b/src/lib/auth/bearer-assertion.ts index 65f333e0..d90b469c 100644 --- a/src/lib/auth/bearer-assertion.ts +++ b/src/lib/auth/bearer-assertion.ts @@ -113,13 +113,14 @@ function assertionVerifier( if (payload.exp <= payload.iat) continue; if (payload.exp - payload.iat > ASSERTION_TTL_SECONDS) continue; - return purpose === "identity-sync" - ? { - userId: payload.sub, - scope: "identity_sync", - bodySha256: payload.body_sha256! - } - : { userId: payload.sub, scope: "assertion" }; + if (purpose === "identity-sync") { + // isAssertionPayload() verified body_sha256 above; re-check here + // so the type narrows without a non-null assertion. + const bodySha256 = payload.body_sha256; + if (typeof bodySha256 !== "string") continue; + return { userId: payload.sub, scope: "identity_sync", bodySha256 }; + } + return { userId: payload.sub, scope: "assertion" }; } // A consistent failure across every configured secret is the only diff --git a/src/services/central-alerts/v1/README.md b/src/services/central-alerts/v1/README.md index 7f155499..f9a71e0e 100644 --- a/src/services/central-alerts/v1/README.md +++ b/src/services/central-alerts/v1/README.md @@ -67,5 +67,3 @@ The `type` field accepts: `success`, `info`, `warning`, `danger` ## Database Uses D1 database binding `DB_CENTRAL_ALERTS`. Initialize with the setup script in `src/services/central-alerts/v1/scripts/`. - -Public GET responses use the edge cache even when an unused Authorization header is supplied; these representations do not depend on credentials. diff --git a/src/services/extensions/v2/README.md b/src/services/extensions/v2/README.md index eadce0f8..a3a47ce3 100644 --- a/src/services/extensions/v2/README.md +++ b/src/services/extensions/v2/README.md @@ -258,6 +258,8 @@ Migration `0021` also drops any submission filed under a developer that no longe See `AGENTS.md` for what belongs in `routes/`, `db/`, `schemas/`, `github/`, and `middleware.ts`. This service is the reference layout for larger services. +### Claim verification budget + Claim verification is limited before contacting GitHub: 3 attempts per account and per normalized developer ID per 60 seconds, plus 300 aggregate claim verification attempts per hour. D1 reserves all budgets atomically. Mismatches, @@ -266,6 +268,7 @@ upstream failures, and cancellation do not refund attempts. Exhaustion returns Deploy migration `0024_claim_verification_budgets.sql` before the Worker update. The aggregate budget covers claims only and leaves shared GitHub capacity for other workflows; it is not a budget for every GitHub consumer. + ### Profile write budget `PUT /developers/me` permits 20 successful profile writes per account in a diff --git a/src/services/extensions/v2/db/users.ts b/src/services/extensions/v2/db/users.ts index 45d4490a..818d6559 100644 --- a/src/services/extensions/v2/db/users.ts +++ b/src/services/extensions/v2/db/users.ts @@ -145,14 +145,15 @@ export class UsersDatabase { githubLogin: input.githubLogin, githubOrgs: hasFreshGithubOrgs ? JSON.stringify(input.githubOrgs) : null, // Bound even a trusted service's membership snapshot to one hour. - githubOrgsExpiresAt: hasFreshGithubOrgs - ? new Date( - Math.min( - Date.parse(input.githubOrgsExpiresAt!), - Date.parse(now) + 60 * 60 * 1000 - ) - ).toISOString() - : null, + githubOrgsExpiresAt: + hasFreshGithubOrgs && typeof input.githubOrgsExpiresAt === "string" + ? new Date( + Math.min( + Date.parse(input.githubOrgsExpiresAt), + Date.parse(now) + 60 * 60 * 1000 + ) + ).toISOString() + : null, deletedAt: null }; From fcee973ce48e0afb6588cef648775ab3bafe902b Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sun, 4 Oct 2026 20:48:52 +0100 Subject: [PATCH 8/9] Address cubic review findings on fix/several Suppress the shared auth warning for sibling-purpose assertions, harden the identity-sync body read, coalesce main enrichment per commit SHA, fix drizzle bookkeeping via generate (0025), and tighten tests and docs. --- src/lib/auth/bearer-assertion.ts | 14 +- src/services/central-alerts/v1/README.md | 13 + src/services/extensions/v2/README.md | 8 +- .../0024_profile_history_budget.sql | 2 - .../0025_luxuriant_franklin_richards.sql | 1 + .../v2/db/migrations/meta/0025_snapshot.json | 1199 +++++++++++++++++ .../v2/db/migrations/meta/_journal.json | 9 +- src/services/extensions/v2/middleware.ts | 19 +- src/services/previews/v1/routes/main.ts | 13 +- test/lib/auth/bearer-assertion.test.ts | 43 +- test/lib/cache.test.ts | 9 +- .../extensions/v2/developer-profiles.test.ts | 7 +- test/utils/isolate-edge-cache.ts | 4 +- 13 files changed, 1319 insertions(+), 22 deletions(-) delete mode 100644 src/services/extensions/v2/db/migrations/0024_profile_history_budget.sql create mode 100644 src/services/extensions/v2/db/migrations/0025_luxuriant_franklin_richards.sql create mode 100644 src/services/extensions/v2/db/migrations/meta/0025_snapshot.json diff --git a/src/lib/auth/bearer-assertion.ts b/src/lib/auth/bearer-assertion.ts index d90b469c..df38571b 100644 --- a/src/lib/auth/bearer-assertion.ts +++ b/src/lib/auth/bearer-assertion.ts @@ -87,6 +87,11 @@ function importedKeyFor(secret: string): Promise { function assertionVerifier( purpose: AssertionPayload["purpose"] ): TokenVerifier { + // The sibling purpose's verifier runs alongside this one (see + // requireIdentitySync): an assertion minted for it is expected traffic + // here, not a misconfiguration, so it must not trip the warning below. + const sibling: AssertionPayload["purpose"] = + purpose === "identity-sync" ? ASSERTION_PURPOSE : "identity-sync"; return { async verify(token, platform): Promise { const secrets = [ @@ -95,6 +100,7 @@ function assertionVerifier( ].filter((secret): secret is string => Boolean(secret)); if (secrets.length === 0) return null; + let sawSiblingPurpose = false; for (const secret of secrets) { let payload: unknown; try { @@ -106,7 +112,10 @@ function assertionVerifier( } catch { continue; } - if (!isAssertionPayload(payload, purpose)) continue; + if (!isAssertionPayload(payload, purpose)) { + if (isAssertionPayload(payload, sibling)) sawSiblingPurpose = true; + continue; + } const now = Math.floor(Date.now() / 1000); if (payload.iat > now + CLOCK_SKEW_SECONDS) continue; @@ -125,8 +134,9 @@ function assertionVerifier( // A consistent failure across every configured secret is the only // signal a misconfigured ASSERTION_SIGNING_SECRET produces. + // Sibling-purpose assertions are expected here, so they skip it. const now = Date.now(); - if (now - lastAuthWarnAt >= WARN_INTERVAL_MS) { + if (!sawSiblingPurpose && now - lastAuthWarnAt >= WARN_INTERVAL_MS) { lastAuthWarnAt = now; logWarn("auth", "Bearer assertion failed verification", { secretsTried: secrets.length diff --git a/src/services/central-alerts/v1/README.md b/src/services/central-alerts/v1/README.md index f9a71e0e..a6cf5fe3 100644 --- a/src/services/central-alerts/v1/README.md +++ b/src/services/central-alerts/v1/README.md @@ -18,6 +18,19 @@ Unknown query parameters and equivalent pagination spellings reuse the same entry. This endpoint is public: Authorization does not affect its response or bypass its cache. Validation failures and database errors are not cached. +When `limit` is usable the response adds a `pagination` object next to +`alerts`: + +```json +{ + "result": { + "alerts": [], + "pagination": { "limit": 1, "offset": 0, "has_more": true } + }, + "error": null +} +``` + **Response:** ```json diff --git a/src/services/extensions/v2/README.md b/src/services/extensions/v2/README.md index a3a47ce3..84dd5457 100644 --- a/src/services/extensions/v2/README.md +++ b/src/services/extensions/v2/README.md @@ -264,8 +264,10 @@ Claim verification is limited before contacting GitHub: 3 attempts per account and per normalized developer ID per 60 seconds, plus 300 aggregate claim verification attempts per hour. D1 reserves all budgets atomically. Mismatches, upstream failures, and cancellation do not refund attempts. Exhaustion returns -429 (`RATE_LIMITED`); pending duplicates still return 409 without spending quota. -Deploy migration `0024_claim_verification_budgets.sql` before the Worker update. +429 (`RATE_LIMITED`); sequential replays of a pending claim short-circuit to +409 before spending quota, but two requests racing each other can each spend +one attempt before the unique index decides the winner. Deploy migration +`0024_claim_verification_budgets.sql` before the Worker update. The aggregate budget covers claims only and leaves shared GitHub capacity for other workflows; it is not a budget for every GitHub consumer. @@ -280,4 +282,4 @@ participate in the comparison; omitted optional fields mean null. The budget is enforced atomically with the write using indexed, immutable history and survives profile deletion, recreation, and ownership transfer. Audit records remain append-only; this bounds growth per account per day rather than total retention. -Apply migration `0024_profile_history_budget.sql` before deploying. +Apply migration `0025_luxuriant_franklin_richards.sql` before deploying. diff --git a/src/services/extensions/v2/db/migrations/0024_profile_history_budget.sql b/src/services/extensions/v2/db/migrations/0024_profile_history_budget.sql deleted file mode 100644 index 10d9323c..00000000 --- a/src/services/extensions/v2/db/migrations/0024_profile_history_budget.sql +++ /dev/null @@ -1,2 +0,0 @@ -CREATE INDEX IF NOT EXISTS idx_developer_history_account_changed_at -ON developer_history (changed_by, changed_at); diff --git a/src/services/extensions/v2/db/migrations/0025_luxuriant_franklin_richards.sql b/src/services/extensions/v2/db/migrations/0025_luxuriant_franklin_richards.sql new file mode 100644 index 00000000..9b44a8a3 --- /dev/null +++ b/src/services/extensions/v2/db/migrations/0025_luxuriant_franklin_richards.sql @@ -0,0 +1 @@ +CREATE INDEX `idx_developer_history_account_changed_at` ON `developer_history` (`changed_by`,`changed_at`); \ No newline at end of file diff --git a/src/services/extensions/v2/db/migrations/meta/0025_snapshot.json b/src/services/extensions/v2/db/migrations/meta/0025_snapshot.json new file mode 100644 index 00000000..b12ff402 --- /dev/null +++ b/src/services/extensions/v2/db/migrations/meta/0025_snapshot.json @@ -0,0 +1,1199 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "2a49cad7-5b9b-4280-aa04-04c373cfe355", + "prevId": "8ca0b40f-64d5-441f-97e3-0283d5cc1192", + "tables": { + "claim_verification_budgets": { + "name": "claim_verification_budgets", + "columns": { + "key": { + "name": "key", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "attempts": { + "name": "attempts", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "expires_at": { + "name": "expires_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "idx_claim_verification_budgets_expiry": { + "name": "idx_claim_verification_budgets_expiry", + "columns": [ + "expires_at" + ], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "developer_claims": { + "name": "developer_claims", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "claimant_id": { + "name": "claimant_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'pending'" + }, + "note": { + "name": "note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "review_note": { + "name": "review_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "reviewer_id": { + "name": "reviewer_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "reviewed_at": { + "name": "reviewed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_org_verified": { + "name": "github_org_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verification_note": { + "name": "github_verification_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developer_claims_developer": { + "name": "idx_developer_claims_developer", + "columns": [ + "developer_id" + ], + "isUnique": false + }, + "idx_developer_claims_claimant": { + "name": "idx_developer_claims_claimant", + "columns": [ + "claimant_id" + ], + "isUnique": false + }, + "idx_developer_claims_pending_unique": { + "name": "idx_developer_claims_pending_unique", + "columns": [ + "developer_id", + "claimant_id" + ], + "isUnique": true, + "where": "\"developer_claims\".\"status\" = 'pending'" + }, + "idx_developer_claims_pending_queue": { + "name": "idx_developer_claims_pending_queue", + "columns": [ + "created_at" + ], + "isUnique": false, + "where": "\"developer_claims\".\"status\" = 'pending'" + } + }, + "foreignKeys": { + "developer_claims_developer_id_developers_id_fk": { + "name": "developer_claims_developer_id_developers_id_fk", + "tableFrom": "developer_claims", + "tableTo": "developers", + "columnsFrom": [ + "developer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_claims_claimant_id_users_id_fk": { + "name": "developer_claims_claimant_id_users_id_fk", + "tableFrom": "developer_claims", + "tableTo": "users", + "columnsFrom": [ + "claimant_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_claims_reviewer_id_users_id_fk": { + "name": "developer_claims_reviewer_id_users_id_fk", + "tableFrom": "developer_claims", + "tableTo": "users", + "columnsFrom": [ + "reviewer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "developer_claims_status_check": { + "name": "developer_claims_status_check", + "value": "\"developer_claims\".\"status\" IN ('pending', 'approved', 'rejected')" + }, + "developer_claims_github_org_verified_check": { + "name": "developer_claims_github_org_verified_check", + "value": "\"developer_claims\".\"github_org_verified\" IN (0, 1)" + } + } + }, + "developer_history": { + "name": "developer_history", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "changed_by": { + "name": "changed_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "changed_at": { + "name": "changed_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + } + }, + "indexes": { + "idx_developer_history_developer_changed_at": { + "name": "idx_developer_history_developer_changed_at", + "columns": [ + "developer_id", + "changed_at" + ], + "isUnique": false + }, + "idx_developer_history_account_changed_at": { + "name": "idx_developer_history_account_changed_at", + "columns": [ + "changed_by", + "changed_at" + ], + "isUnique": false + } + }, + "foreignKeys": { + "developer_history_changed_by_users_id_fk": { + "name": "developer_history_changed_by_users_id_fk", + "tableFrom": "developer_history", + "tableTo": "users", + "columnsFrom": [ + "changed_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "developer_transfers": { + "name": "developer_transfers", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "token_hash": { + "name": "token_hash", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_by": { + "name": "created_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "expires_at": { + "name": "expires_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "accepted_by": { + "name": "accepted_by", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "accepted_at": { + "name": "accepted_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "revoked_at": { + "name": "revoked_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developer_transfers_token": { + "name": "idx_developer_transfers_token", + "columns": [ + "token_hash" + ], + "isUnique": true + }, + "idx_developer_transfers_pending": { + "name": "idx_developer_transfers_pending", + "columns": [ + "developer_id" + ], + "isUnique": true, + "where": "\"developer_transfers\".\"accepted_at\" IS NULL AND \"developer_transfers\".\"revoked_at\" IS NULL" + } + }, + "foreignKeys": { + "developer_transfers_developer_id_developers_id_fk": { + "name": "developer_transfers_developer_id_developers_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "developers", + "columnsFrom": [ + "developer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_transfers_created_by_users_id_fk": { + "name": "developer_transfers_created_by_users_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "users", + "columnsFrom": [ + "created_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_transfers_accepted_by_users_id_fk": { + "name": "developer_transfers_accepted_by_users_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "users", + "columnsFrom": [ + "accepted_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "developers": { + "name": "developers", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "owner_user_id": { + "name": "owner_user_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "approved_at": { + "name": "approved_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'1970-01-01T00:00:00.000Z'" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'1970-01-01T00:00:00.000Z'" + }, + "avatar_url": { + "name": "avatar_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "contact_email": { + "name": "contact_email", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "ownership_epoch": { + "name": "ownership_epoch", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "content_revision": { + "name": "content_revision", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "approved_revision": { + "name": "approved_revision", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "approved_by": { + "name": "approved_by", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_org_verified": { + "name": "github_org_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verification_note": { + "name": "github_verification_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verified_at": { + "name": "github_verified_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_url_verified": { + "name": "github_url_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "url_check_cooldown_until": { + "name": "url_check_cooldown_until", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developers_owner_unique": { + "name": "idx_developers_owner_unique", + "columns": [ + "owner_user_id" + ], + "isUnique": true + }, + "idx_developers_approved": { + "name": "idx_developers_approved", + "columns": [ + "approved_at" + ], + "isUnique": false + } + }, + "foreignKeys": { + "developers_owner_user_id_users_id_fk": { + "name": "developers_owner_user_id_users_id_fk", + "tableFrom": "developers", + "tableTo": "users", + "columnsFrom": [ + "owner_user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "developers_ownership_epoch_check": { + "name": "developers_ownership_epoch_check", + "value": "\"developers\".\"ownership_epoch\" >= 1" + }, + "developers_content_revision_check": { + "name": "developers_content_revision_check", + "value": "\"developers\".\"content_revision\" >= 1" + }, + "developers_github_org_verified_check": { + "name": "developers_github_org_verified_check", + "value": "\"developers\".\"github_org_verified\" IN (0, 1)" + }, + "developers_github_url_verified_check": { + "name": "developers_github_url_verified_check", + "value": "\"developers\".\"github_url_verified\" = 1" + } + } + }, + "extension_revisions": { + "name": "extension_revisions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "extension_id": { + "name": "extension_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "submitted_by": { + "name": "submitted_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'pending'" + }, + "content": { + "name": "content", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "reviewer_id": { + "name": "reviewer_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "review_note": { + "name": "review_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "reviewed_at": { + "name": "reviewed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "ownership_epoch": { + "name": "ownership_epoch", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + } + }, + "indexes": { + "idx_extension_revisions_submitted_by": { + "name": "idx_extension_revisions_submitted_by", + "columns": [ + "submitted_by" + ], + "isUnique": false + }, + "idx_extension_revisions_developer": { + "name": "idx_extension_revisions_developer", + "columns": [ + "developer_id" + ], + "isUnique": false + }, + "idx_extension_revisions_pending": { + "name": "idx_extension_revisions_pending", + "columns": [ + "extension_id" + ], + "isUnique": true, + "where": "\"extension_revisions\".\"status\" = 'pending'" + }, + "idx_extension_revisions_extension_page": { + "name": "idx_extension_revisions_extension_page", + "columns": [ + "extension_id", + "\"created_at\" desc", + "\"id\" desc" + ], + "isUnique": false + }, + "idx_extension_revisions_submitter_page": { + "name": "idx_extension_revisions_submitter_page", + "columns": [ + "submitted_by", + "\"created_at\" desc", + "\"id\" desc" + ], + "isUnique": false + }, + "idx_extension_revisions_queue_page": { + "name": "idx_extension_revisions_queue_page", + "columns": [ + "status", + "created_at", + "id" + ], + "isUnique": false + }, + "idx_extension_revisions_reviewed": { + "name": "idx_extension_revisions_reviewed", + "columns": [ + "extension_id", + "reviewed_at" + ], + "isUnique": false, + "where": "\"extension_revisions\".\"status\" IN ('approved', 'rejected')" + }, + "idx_extension_revisions_submitter_pending": { + "name": "idx_extension_revisions_submitter_pending", + "columns": [ + "submitted_by" + ], + "isUnique": false, + "where": "\"extension_revisions\".\"status\" = 'pending'" + } + }, + "foreignKeys": { + "extension_revisions_extension_id_extensions_id_fk": { + "name": "extension_revisions_extension_id_extensions_id_fk", + "tableFrom": "extension_revisions", + "tableTo": "extensions", + "columnsFrom": [ + "extension_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "extension_revisions_submitted_by_users_id_fk": { + "name": "extension_revisions_submitted_by_users_id_fk", + "tableFrom": "extension_revisions", + "tableTo": "users", + "columnsFrom": [ + "submitted_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "extension_revisions_reviewer_id_users_id_fk": { + "name": "extension_revisions_reviewer_id_users_id_fk", + "tableFrom": "extension_revisions", + "tableTo": "users", + "columnsFrom": [ + "reviewer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "extension_revisions_status_check": { + "name": "extension_revisions_status_check", + "value": "\"extension_revisions\".\"status\" IN ('pending', 'approved', 'rejected')" + }, + "extension_revisions_ownership_epoch_check": { + "name": "extension_revisions_ownership_epoch_check", + "value": "\"extension_revisions\".\"ownership_epoch\" >= 1" + } + } + }, + "extensions": { + "name": "extensions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "published_at": { + "name": "published_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "published_revision_id": { + "name": "published_revision_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "releases": { + "name": "releases", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "website": { + "name": "website", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "license": { + "name": "license", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "icon_url": { + "name": "icon_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "readme": { + "name": "readme", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "source": { + "name": "source", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "version": { + "name": "version", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "download_url": { + "name": "download_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "delisted_at": { + "name": "delisted_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "delist_reason": { + "name": "delist_reason", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_extensions_id_nocase": { + "name": "idx_extensions_id_nocase", + "columns": [ + "lower(\"id\")" + ], + "isUnique": true + }, + "idx_extensions_developer_order": { + "name": "idx_extensions_developer_order", + "columns": [ + "developer_id", + "lower(\"id\")", + "id" + ], + "isUnique": false + }, + "idx_extensions_catalogue_order": { + "name": "idx_extensions_catalogue_order", + "columns": [ + "lower(\"id\")", + "id" + ], + "isUnique": false, + "where": "\"extensions\".\"published_at\" IS NOT NULL AND \"extensions\".\"delisted_at\" IS NULL" + }, + "idx_extensions_type_catalogue_order": { + "name": "idx_extensions_type_catalogue_order", + "columns": [ + "type", + "lower(\"id\")", + "id" + ], + "isUnique": false, + "where": "\"extensions\".\"published_at\" IS NOT NULL AND \"extensions\".\"delisted_at\" IS NULL" + } + }, + "foreignKeys": { + "extensions_developer_id_developers_id_fk": { + "name": "extensions_developer_id_developers_id_fk", + "tableFrom": "extensions", + "tableTo": "developers", + "columnsFrom": [ + "developer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "extensions_published_content_check": { + "name": "extensions_published_content_check", + "value": "\"extensions\".\"published_at\" IS NULL OR (\n \"extensions\".\"type\" IS NOT NULL AND \"extensions\".\"name\" IS NOT NULL AND\n \"extensions\".\"description\" IS NOT NULL AND \"extensions\".\"releases\" IS NOT NULL AND\n \"extensions\".\"website\" IS NOT NULL AND \"extensions\".\"license\" IS NOT NULL AND\n \"extensions\".\"readme\" IS NOT NULL AND \"extensions\".\"source\" IS NOT NULL AND\n \"extensions\".\"version\" IS NOT NULL AND \"extensions\".\"download_url\" IS NOT NULL\n )" + } + } + }, + "users": { + "name": "users", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "email": { + "name": "email", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "email_verified": { + "name": "email_verified", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "picture": { + "name": "picture", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "is_moderator": { + "name": "is_moderator", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "display_name": { + "name": "display_name", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_login": { + "name": "github_login", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_orgs": { + "name": "github_orgs", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_orgs_expires_at": { + "name": "github_orgs_expires_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "deleted_at": { + "name": "deleted_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": { + "idx_extension_revisions_extension_page": { + "columns": { + "\"created_at\" desc": { + "isExpression": true + }, + "\"id\" desc": { + "isExpression": true + } + } + }, + "idx_extension_revisions_submitter_page": { + "columns": { + "\"created_at\" desc": { + "isExpression": true + }, + "\"id\" desc": { + "isExpression": true + } + } + }, + "idx_extensions_id_nocase": { + "columns": { + "lower(\"id\")": { + "isExpression": true + } + } + }, + "idx_extensions_developer_order": { + "columns": { + "lower(\"id\")": { + "isExpression": true + } + } + }, + "idx_extensions_catalogue_order": { + "columns": { + "lower(\"id\")": { + "isExpression": true + } + } + }, + "idx_extensions_type_catalogue_order": { + "columns": { + "lower(\"id\")": { + "isExpression": true + } + } + } + } + } +} \ No newline at end of file diff --git a/src/services/extensions/v2/db/migrations/meta/_journal.json b/src/services/extensions/v2/db/migrations/meta/_journal.json index 05d7f3b1..1f688a7c 100644 --- a/src/services/extensions/v2/db/migrations/meta/_journal.json +++ b/src/services/extensions/v2/db/migrations/meta/_journal.json @@ -78,6 +78,13 @@ "when": 1791060505627, "tag": "0024_claim_verification_budgets", "breakpoints": true + }, + { + "idx": 25, + "version": "6", + "when": 1791143160901, + "tag": "0025_luxuriant_franklin_richards", + "breakpoints": true } ] -} +} \ No newline at end of file diff --git a/src/services/extensions/v2/middleware.ts b/src/services/extensions/v2/middleware.ts index 3d9ebfa8..6ccf2ad7 100644 --- a/src/services/extensions/v2/middleware.ts +++ b/src/services/extensions/v2/middleware.ts @@ -82,10 +82,21 @@ export function requireIdentitySync(): MiddlewareHandler { } // Hash the exact bytes before JSON parsing. Hono caches this buffer so // validation and persistence consume the same authenticated body. - const digest = await crypto.subtle.digest( - "SHA-256", - await c.req.arrayBuffer() - ); + let rawBody: ArrayBuffer; + try { + rawBody = await c.req.arrayBuffer(); + } catch { + return c.json( + { + error: { + message: "Unable to read request body", + code: "BAD_REQUEST" + } + }, + 400 + ); + } + const digest = await crypto.subtle.digest("SHA-256", rawBody); const hex = Array.from(new Uint8Array(digest), (byte) => byte.toString(16).padStart(2, "0") ).join(""); diff --git a/src/services/previews/v1/routes/main.ts b/src/services/previews/v1/routes/main.ts index 361c1c69..96e5d50a 100644 --- a/src/services/previews/v1/routes/main.ts +++ b/src/services/previews/v1/routes/main.ts @@ -114,10 +114,15 @@ async function resolveMainPreview( return null; } - const artifactFields = await resolveArtifactFields( - previewGitHub(c), - object.commitSha - ); + const artifactFields = await (object.commitSha + ? // Enrichment is keyed by commit: concurrent cold requests share one + // GitHub lookup (charged once to the shared budget) without mixing + // SHAs, and the R2 head above is already single-flighted so racing + // requests observe the same object. + singleFlight(`previews:main:enrich:${object.commitSha}`, () => + resolveArtifactFields(previewGitHub(c), object.commitSha) + ) + : resolveArtifactFields(previewGitHub(c), null)); const result = buildMainPreview(object, artifactFields); diff --git a/test/lib/auth/bearer-assertion.test.ts b/test/lib/auth/bearer-assertion.test.ts index 958ae7ac..ece8bfcc 100644 --- a/test/lib/auth/bearer-assertion.test.ts +++ b/test/lib/auth/bearer-assertion.test.ts @@ -1,11 +1,18 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi, afterEach, beforeEach } from "vitest"; import { bearerAssertionVerifier, identitySyncAssertionVerifier } from "../../../src/lib/auth/bearer-assertion"; +import { logWarn } from "../../../src/lib/logger"; import { PlatformContext } from "../../../src/lib/context"; import { base64UrlEncodeString, signAssertion } from "./assertion-helper"; +vi.mock("../../../src/lib/logger", () => ({ + logError: vi.fn(), + logWarn: vi.fn(), + logInfo: vi.fn() +})); + const SECRET = "test-secret"; function platformWithSecret( @@ -288,3 +295,37 @@ describe("identitySyncAssertionVerifier", () => { ).toBeNull(); }); }); + +describe("verification warnings", () => { + beforeEach(() => { + // Push past the warn throttle so assertions below are meaningful + // rather than an artifact of earlier tests warning first. + vi.useFakeTimers(); + vi.setSystemTime(Date.now() + 61_000); + vi.mocked(logWarn).mockClear(); + }); + afterEach(() => { + vi.useRealTimers(); + }); + + it("does not warn for a sibling-purpose assertion", async () => { + const token = await signAssertion(SECRET, { sub: "user-42" }); + expect( + await identitySyncAssertionVerifier.verify( + token, + platformWithSecret(SECRET) + ) + ).toBeNull(); + expect(logWarn).not.toHaveBeenCalled(); + }); + + it("still warns for tokens valid for neither purpose", async () => { + expect( + await identitySyncAssertionVerifier.verify( + "not-a-jwt", + platformWithSecret(SECRET) + ) + ).toBeNull(); + expect(logWarn).toHaveBeenCalledTimes(1); + }); +}); diff --git a/test/lib/cache.test.ts b/test/lib/cache.test.ts index 9c14bf37..9a3fd74c 100644 --- a/test/lib/cache.test.ts +++ b/test/lib/cache.test.ts @@ -29,12 +29,15 @@ describe("public response cache", () => { const original = new Request("https://example.test/public", { headers: { Authorization: "x" } }); + // Asserted after fetch: an expect thrown inside the handler would be + // routed to onError's 500, making the status assertion pass vacuously. + const observed: { raw?: unknown; authorization?: string | null } = {}; app.get( "/public", publicResponseCache({ cacheName: crypto.randomUUID(), wait: true }), (c) => { - expect(c.req.raw).toBe(original); - expect(c.req.header("Authorization")).toBe("x"); + observed.raw = c.req.raw; + observed.authorization = c.req.header("Authorization"); throw new Error("handler failed"); } ); @@ -43,6 +46,8 @@ describe("public response cache", () => { return c.text("failure", 500); }); expect((await app.fetch(original)).status).toBe(500); + expect(observed.raw).toBe(original); + expect(observed.authorization).toBe("x"); }); it("restores the request when cache key generation fails", async () => { diff --git a/test/services/extensions/v2/developer-profiles.test.ts b/test/services/extensions/v2/developer-profiles.test.ts index 7917dc60..a10ce911 100644 --- a/test/services/extensions/v2/developer-profiles.test.ts +++ b/test/services/extensions/v2/developer-profiles.test.ts @@ -85,7 +85,12 @@ describe("Extensions API v2", () => { ) ); expect(raced.filter((r) => r.status === 200)).toHaveLength(1); - expect(raced.filter((r) => r.status !== 200)).toHaveLength(1); + const losers = raced.filter((r) => r.status !== 200); + expect(losers).toHaveLength(1); + expect(losers[0].status).toBe(429); + expect(await losers[0].json()).toMatchObject({ + error: { code: "PROFILE_MUTATION_RATE_LIMITED" } + }); expect(await listDeveloperHistory(db)).toHaveLength(20); const current = await getDeveloper(db, profile.id); expect( diff --git a/test/utils/isolate-edge-cache.ts b/test/utils/isolate-edge-cache.ts index 39452d60..27565706 100644 --- a/test/utils/isolate-edge-cache.ts +++ b/test/utils/isolate-edge-cache.ts @@ -4,8 +4,8 @@ import { afterEach, vi } from "vitest"; // while exercising the real Cache API, including waitUntil cache writes. export function isolateEdgeCache() { const open = caches.open.bind(caches); - let restore: () => void; - afterEach(() => restore()); + let restore: (() => void) | undefined; + afterEach(() => restore?.()); return () => { const suffix = crypto.randomUUID(); const spy = vi From 0449150e5c7883dae4d70a44f873d4e4c520d29e Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sun, 4 Oct 2026 20:59:07 +0100 Subject: [PATCH 9/9] Declare 400 response on PUT /users/me/identity --- src/services/extensions/v2/routes/account.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services/extensions/v2/routes/account.ts b/src/services/extensions/v2/routes/account.ts index 0ad9bc50..f242fbe9 100644 --- a/src/services/extensions/v2/routes/account.ts +++ b/src/services/extensions/v2/routes/account.ts @@ -53,6 +53,7 @@ export function registerAccountRoutes(app: ExtensionsV2App): void { }, description: "Identity projection synchronized" }, + 400: errorResponse("Request body could not be read"), 401: errorResponse("Missing or invalid bearer token"), 403: { ...ActiveAccountRequiredResponse,