Harden preview budgets, cache variants, and moderator guards

src/lib/adapters/cloudflare/preview-github-budget.ts

@@ -1,8 +1,8 @@
 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.
+// Token buckets permit 60 calls globally and 12 per client in a burst,
+// then refill at 1000/hour globally and 120/hour per client. Every upstream call (not just each lookup) costs one.
 export class PreviewGitHubBudget extends DurableObject<CloudflareBindings> {
   constructor(ctx: DurableObjectState, env: CloudflareBindings) {
     super(ctx, env);
@@ -14,24 +14,28 @@ export class PreviewGitHubBudget extends DurableObject<CloudflareBindings> {
   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.
+      // A client bucket is full after six minutes; retaining thirty minutes
+      // of idle state avoids granting an early reset.
       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 }) => {
+        { key: "global", capacity: 60, rate: 1000 / 3600000 },
+        { key: `client:${client}`, capacity: 12, rate: 120 / 3600000 }
+      ].map(({ key, capacity, 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;
+          ? Math.min(
+              capacity,
+              row.tokens + Math.max(0, now - row.updated_at) * rate
+            )
+          : capacity;
         return { key, tokens };
       });
       if (buckets.some(({ tokens }) => tokens < 1)) return false;

src/services/extensions/v2/db/developer-claims.ts

@@ -5,6 +5,7 @@ import { encodeCursor as encode, decodeCursor as decode } from "./cursor";
 import { developerClaims, developers, users } from "./schema";
 import {
   databaseError,
+  moderatorActorError,
   isDeveloperOwnerConflict,
   isOwnershipEpochRollback
 } from "./errors";
@@ -430,8 +431,12 @@ export class DeveloperClaimsDatabase {
   }
 
   private async explainClaimApprovalNoOp(
-    claim: DeveloperClaim
+    claim: DeveloperClaim,
+    reviewerId: string
   ): Promise<DatabaseResult<DeveloperProfile>> {
+    const actorError = await moderatorActorError(this.db, reviewerId);
+    if (actorError) return { data: null, error: actorError };
+
     const latestClaim = await this.getClaimById(claim.id);
     if (latestClaim.error || latestClaim.data?.status !== "pending") {
       return {
@@ -524,7 +529,7 @@ export class DeveloperClaimsDatabase {
                 )
                 AND EXISTS (
                   SELECT 1 FROM users
-                  WHERE users.id = ? AND users.deleted_at IS NULL
+                  WHERE users.id = ? AND users.deleted_at IS NULL AND users.is_moderator = 1
                 )`,
         params: [reviewerId, claimId, reviewerId]
       });
@@ -575,7 +580,7 @@ export class DeveloperClaimsDatabase {
     } catch (error) {
       if (isOwnershipEpochRollback(error)) {
         return this.claimApprovalOutcome(
-          await this.explainClaimApprovalNoOp(claim),
+          await this.explainClaimApprovalNoOp(claim, reviewerId),
           claim
         );
       }
@@ -596,7 +601,7 @@ export class DeveloperClaimsDatabase {
       // Diagnose only after the guarded transaction. These reads improve the
       // response without participating in (or weakening) its race safety.
       return this.claimApprovalOutcome(
-        await this.explainClaimApprovalNoOp(claim),
+        await this.explainClaimApprovalNoOp(claim, reviewerId),
         claim
       );
     }
@@ -644,6 +649,7 @@ export class DeveloperClaimsDatabase {
             sql`EXISTS (
               SELECT 1 FROM ${users}
               WHERE ${users.id} = ${reviewerId} AND ${users.deletedAt} IS NULL
+                AND ${users.isModerator} = 1
             )`
           )
         );
@@ -652,6 +658,9 @@ export class DeveloperClaimsDatabase {
     }
 
     if (!result.meta?.changes) {
+      const actorError = await moderatorActorError(this.db, reviewerId);
+      if (actorError) return { data: null, error: actorError };
+
       return {
         data: null,
         error: {

src/services/extensions/v2/db/developer-profiles.ts

@@ -9,7 +9,7 @@ import {
   extensions,
   users
 } from "./schema";
-import { databaseError } from "./errors";
+import { databaseError, moderatorActorError } from "./errors";
 import { toD1Statement } from "./batch";
 import { optionalBool } from "./columns";
 import {
@@ -833,6 +833,7 @@ export class DeveloperProfilesDatabase {
             sql`EXISTS (
               SELECT 1 FROM ${users}
               WHERE ${users.id} = ${reviewerId} AND ${users.deletedAt} IS NULL
+                AND ${users.isModerator} = 1
             )`
           )
         );
@@ -841,24 +842,8 @@ export class DeveloperProfilesDatabase {
     }
 
     if (!result.meta?.changes) {
-      let reviewer: { deletedAt: string | null } | undefined;
-      try {
-        [reviewer] = await this.db
-          .select({ deletedAt: users.deletedAt })
-          .from(users)
-          .where(eq(users.id, reviewerId));
-      } catch (error) {
-        return databaseError("approve", error);
-      }
-      if (!reviewer || reviewer.deletedAt !== null) {
-        return {
-          data: null,
-          error: {
-            code: "ACCOUNT_INACTIVE",
-            message: "Active account required"
-          }
-        };
-      }
+      const actorError = await moderatorActorError(this.db, reviewerId);
+      if (actorError) return { data: null, error: actorError };
 
       const existing = await this.getById(id);
       if (existing.error) {

src/services/extensions/v2/db/errors.ts

@@ -86,3 +86,19 @@ export async function inactiveActorError(
     ? null
     : { message: "Active account required", code: "ACCOUNT_INACTIVE" };
 }
+
+// Diagnose a failed commit-time moderator guard before workflow conflicts.
+// Activity takes precedence so deactivation retains ACCOUNT_INACTIVE.
+export async function moderatorActorError(
+  db: ExtensionsDb,
+  userId: string
+): Promise<DatabaseError | null> {
+  const { data, error } = await new UsersDatabase(db).moderatorAccess(userId);
+  if (error) return error;
+  if (!data?.active) {
+    return { message: "Active account required", code: "ACCOUNT_INACTIVE" };
+  }
+  return data.moderator
+    ? null
+    : { message: "Moderator access required", code: "FORBIDDEN" };
+}

src/services/extensions/v2/db/extensions.ts

@@ -5,7 +5,11 @@ import { ExtensionsDb } from "../../../../lib/db";
 import { sortReleasesDescending } from "../../../../lib/releases";
 import { parseJSON } from "../../../../lib/json";
 import { extensions, extensionRevisions, developers, users } from "./schema";
-import { databaseError, inactiveActorError } from "./errors";
+import {
+  databaseError,
+  inactiveActorError,
+  moderatorActorError
+} from "./errors";
 import { UsersDatabase } from "./users";
 import { toD1Statement } from "./batch";
 import { encodeCursor as encode, decodeCursor as decode } from "./cursor";
@@ -706,6 +710,7 @@ export class ExtensionsDatabase {
             sql`EXISTS (
               SELECT 1 FROM ${users}
               WHERE ${users.id} = ${moderatorId} AND ${users.deletedAt} IS NULL
+                AND ${users.isModerator} = 1
             )`
           )
         );
@@ -726,8 +731,8 @@ export class ExtensionsDatabase {
     id: string,
     moderatorId: string
   ): Promise<DatabaseResult<never>> {
-    const inactive = await inactiveActorError(this.db, moderatorId);
-    if (inactive) return { data: null, error: inactive };
+    const actorError = await moderatorActorError(this.db, moderatorId);
+    if (actorError) return { data: null, error: actorError };
 
     let existing:
       { publishedAt: string | null; delistedAt: string | null } | undefined;

src/services/extensions/v2/db/revisions.ts

@@ -2,7 +2,11 @@ import { and, asc, desc, eq, gt, lt, or, sql, SQL } from "drizzle-orm";
 import { DatabaseError, DatabaseResult } from "../../../../lib/interfaces";
 import { ExtensionsDb } from "../../../../lib/db";
 import { extensionRevisions, developers, extensions, users } from "./schema";
-import { databaseError, inactiveActorError } from "./errors";
+import {
+  databaseError,
+  inactiveActorError,
+  moderatorActorError
+} from "./errors";
 import { toD1Statement } from "./batch";
 import { encodeCursor as encode, decodeCursor as decode } from "./cursor";
 import { MAX_PENDING_REVISIONS_PER_USER, parseContent } from "./extensions";
@@ -351,8 +355,8 @@ export class ExtensionRevisionsDatabase {
     id: string,
     reviewerId: string
   ): Promise<DatabaseResult<never>> {
-    const inactive = await inactiveActorError(this.db, reviewerId);
-    if (inactive) return { data: null, error: inactive };
+    const actorError = await moderatorActorError(this.db, reviewerId);
+    if (actorError) return { data: null, error: actorError };
 
     const existing = await this.getById(extensionId, id);
     if (existing.error || !existing.data) {
@@ -393,6 +397,7 @@ export class ExtensionRevisionsDatabase {
             sql`EXISTS (
               SELECT 1 FROM ${users}
               WHERE ${users.id} = ${reviewerId} AND ${users.deletedAt} IS NULL
+                AND ${users.isModerator} = 1
             )`
           )
         );
@@ -475,7 +480,7 @@ export class ExtensionRevisionsDatabase {
                 )
                 AND EXISTS (
                   SELECT 1 FROM users u
-                  WHERE u.id = ? AND u.deleted_at IS NULL
+                  WHERE u.id = ? AND u.deleted_at IS NULL AND u.is_moderator = 1
                 )`,
         params: [
           reviewerId,
@@ -521,10 +526,10 @@ export class ExtensionRevisionsDatabase {
     }
 
     if (!results[0]?.meta?.changes) {
-      const inactive = await inactiveActorError(this.db, reviewerId);
+      const actorError = await moderatorActorError(this.db, reviewerId);
       return {
         data: null,
-        error: inactive ?? {
+        error: actorError ?? {
           message:
             "Revision is not pending, or ownership changed since it was proposed",
           code: "CONFLICT"

src/services/extensions/v2/routes/moderation.ts

@@ -3,11 +3,7 @@ import { getExtensionsDb } from "../../../../lib/db";
 import { getPlatform } from "../../../../lib/middleware";
 import { getAuth } from "../../../../lib/auth";
 import { createRoute, z } from "@hono/zod-openapi";
-import {
-  errorBody,
-  statusFromErrorCode,
-  statusFromWriteErrorCode
-} from "./errors";
+import { errorBody, statusFromWriteErrorCode } from "./errors";
 import {
   ActiveAccountRequiredResponse,
   CursorPaginationQuerySchema,
@@ -494,10 +490,7 @@ export function registerModerationRoutes(app: ExtensionsV2App): void {
       auth.userId
     );
     if (error || !data) {
-      const status =
-        error?.code === "ACCOUNT_INACTIVE"
-          ? 403
-          : statusFromErrorCode(error?.code);
+      const status = statusFromWriteErrorCode(error?.code);
       return c.json(errorBody(error, "Unable to approve developer"), status);
     }
     revalidateCatalogue(c);

src/services/extensions/v2/routes/ownership.ts

@@ -325,7 +325,7 @@ export function registerOwnershipRoutes(app: ExtensionsV2App): void {
     if (error || !data) {
       return c.json(
         errorBody(error, "Unable to approve claim"),
-        statusFromErrorCode(error?.code)
+        statusFromWriteErrorCode(error?.code)
       );
     }
     revalidateCatalogue(c);
@@ -399,7 +399,10 @@ export function registerOwnershipRoutes(app: ExtensionsV2App): void {
     const db = new DeveloperClaimsDatabase(extDb);
     const { data, error } = await db.rejectClaim(id, auth.userId, review_note);
     if (error || !data) {
-      const status = statusFromErrorCode(error?.code, false);
+      const status =
+        error?.code === "FORBIDDEN" || error?.code === "ACCOUNT_INACTIVE"
+          ? 403
+          : statusFromErrorCode(error?.code, false);
       return c.json(errorBody(error, "Unable to reject claim"), status);
     }
     revalidateCatalogue(c);

src/services/previews/v1/README.md

@@ -33,8 +33,13 @@ Endpoints are not listed here. The service publishes its own contract:
 
 - `GET /main` and `GET /pr/{number}` are **pointers** - they always resolve
   to whatever is current.
-- `GET /commit/{sha}` is a **fixed point** - one commit, one build,
+- `GET /commit/{sha}` with a full SHA is a **fixed point** - one commit,
   permanently addressable (until GitHub's artifact retention expires it).
+  Abbreviated SHAs (7+ hex characters) are accepted only when a complete
+  artifact scan finds one distinct live preview commit. Multiple commits
+  return HTTP 409 (`AMBIGUOUS_COMMIT`); use the full SHA to disambiguate.
+  Budget or page-cap exhaustion returns 503; GitHub rate limits return 429.
+  An incomplete scan never returns a selected artifact or a 404.
 - `pr/{number}`'s handler resolves the PR to its head SHA
   (`GET /pulls/{number}`) and delegates to the same resolver `commit/{sha}`
   uses - one GitHub-facing code path, not two.
@@ -71,7 +76,7 @@ Endpoints are not listed here. The service publishes its own contract:
   metadata route already cached) and `GET /main` (`preview:main`) use the
   60s default, matching how often a moving pointer can realistically
   change.
-  `GET /commit/{sha}` (`preview:commit:{sha}`, likewise shared with
+  Full-SHA `GET /commit/{sha}` (`preview:commit:{sha}`, likewise shared with
   `/commit/{sha}/download`) uses 3600s instead - a commit's build never
   changes once it exists, so there's no correctness reason to re-check it
   every minute. That 3600s is capped at the artifact's own remaining
@@ -82,6 +87,10 @@ Endpoints are not listed here. The service publishes its own contract:
   minimum TTL, so those requests (and any more before the artifact expires
   or a request refreshes it) are just served live instead of cached - a
   short burst of extra GitHub calls right at the end, never stale data.
+- Abbreviated commit lookups bypass KV reads and writes, including legacy
+  prefix entries: a newly built colliding commit must be detected on the
+  next lookup. Concurrent prefix lookups share the in-flight scan, but its
+  result is never retained. Full-SHA cache behavior is unchanged.
 - `GET /pr/{number}/download` and `GET /commit/{sha}/download` always
   resolve GitHub's signed redirect URL live, never cached - it expires in
   about a minute, and Cloudflare KV's 60s minimum TTL leaves no safe margin
@@ -98,9 +107,18 @@ Every previews GitHub request reserves a token from the singleton
 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.
+Each client shares a second bucket with a 12-call burst and 120 calls/hour
+refill, so one client cannot spend the whole global burst. IPv4 addresses use
+one allowance per address; IPv6 addresses share an allowance per /64, with
+IPv4-mapped IPv6 addresses using their IPv4 allowance. Only a SHA-256 digest of
+this normalized identity from Cloudflare's connecting IP is stored. Missing or
+invalid addresses share one allowance. Idle client buckets expire after 30 minutes.
+
+Each incoming request also has an eight-call ceiling shared across PR head
+resolution, artifact fallback, main enrichment and live download resolution.
+Exhausting this ceiling returns unavailable rather than a false not-found result.
+A match beyond the ceiling requires a cheaper lookup (such as a full commit SHA)
+or a later cached result; repeating a cold lookup does not resume its scan.
 
 Abbreviated SHAs and fork-PR resolution remain supported within these budgets.
 Exhaustion or budget-service failure stops before another GitHub request and