fix(ai-review): harden BYOK — provider timeout, rate-limit, maintainer-only panel guard, key validation

apps/gittensory-ui/src/components/site/app-panels/ai-review-settings.tsx

@@ -102,17 +102,33 @@ export function AiReviewSettings({ reviewability }: { reviewability: Array<{ pr:
 
   async function saveKey() {
     if (!base) return;
-    if (keyInput.trim().length < 20) {
+    const trimmed = keyInput.trim();
+    if (trimmed.length < 20) {
       setMessage({ kind: "err", text: "Enter a valid provider API key." });
       return;
     }
+    // Mirror the server-side prefix check so an obvious provider/key mismatch is caught before the round-trip.
+    const matchesProvider =
+      provider === "anthropic"
+        ? trimmed.startsWith("sk-ant-")
+        : trimmed.startsWith("sk-") && !trimmed.startsWith("sk-ant-");
+    if (!matchesProvider) {
+      setMessage({
+        kind: "err",
+        text:
+          provider === "anthropic"
+            ? "Anthropic keys start with sk-ant-."
+            : "OpenAI keys start with sk- (and not sk-ant-).",
+      });
+      return;
+    }
     setBusy(true);
     const result = await apiFetch<AiKeyStatus>(`${base}/ai-key`, {
       method: "POST",
       label: "Save provider key",
       credentials: "include",
       headers: JSON_HEADERS,
-      body: JSON.stringify({ provider, key: keyInput.trim(), model: model.trim() || null }),
+      body: JSON.stringify({ provider, key: trimmed, model: model.trim() || null }),
     });
     setBusy(false);
     if (result.ok) {

apps/gittensory-ui/src/components/site/app-panels/maintainer-panel.tsx

@@ -19,10 +19,11 @@ import {
 } from "@/components/site/control-primitives";
 import { AiReviewSettings } from "@/components/site/app-panels/ai-review-settings";
 import { StatCard } from "@/components/site/primitives";
-import { StateBoundary } from "@/components/site/state-views";
+import { EmptyState, LoadingState, StateBoundary } from "@/components/site/state-views";
 import { apiFetch } from "@/lib/api/request";
 import { getApiOrigin } from "@/lib/api/origin";
 import { useApiResource } from "@/lib/api/use-api-resource";
+import { useSession } from "@/lib/api/session";
 import {
   PREVIEW_SCENARIOS,
   buildSettingsPreviewRequest,
@@ -143,7 +144,34 @@ type SettingsPreviewResponse = {
   summary: string;
 };
 
+const MAINTAINER_ROLES = ["maintainer", "owner", "operator"] as const;
+
+/**
+ * Role gate. The maintainer console — including the AI review / BYOK key panel — is shown ONLY to
+ * verified maintainers/owners/operators. This mirrors the server gate (GET /v1/app/maintainer-dashboard
+ * 403s `insufficient_role`, and every BYOK route re-checks per-repo maintainer access), but stops the
+ * dashboard query and the BYOK form from ever mounting for a non-maintainer (defense-in-depth + a clean
+ * message instead of a raw 403). The backend remains the source of truth.
+ */
 export function MaintainerPanel() {
+  const { session, hydrated } = useSession();
+  const isMaintainer = (session?.roles ?? []).some((role) =>
+    MAINTAINER_ROLES.includes(role as (typeof MAINTAINER_ROLES)[number]),
+  );
+
+  if (!hydrated) return <LoadingState title="Checking maintainer access…" />;
+  if (!isMaintainer) {
+    return (
+      <EmptyState
+        title="Maintainer access required"
+        description="This console is available to verified repository maintainers, owners, and operators. Sign in with a GitHub account that maintains an installed repository to manage AI review and BYOK provider keys."
+      />
+    );
+  }
+  return <MaintainerDashboardView />;
+}
+
+function MaintainerDashboardView() {
   const dashboard = useApiResource<MaintainerDashboard>(
     "/v1/app/maintainer-dashboard",
     "Maintainer dashboard",

src/api/routes.ts

@@ -534,12 +534,19 @@ const repositorySettingsSchema = z.object({
 });
 
 // Maintainer BYOK provider key. Write-only: the key is encrypted at rest and never returned. A loose
-// shape check (sk-ant-… / sk-…) catches obvious paste errors without coupling to provider key formats.
-const repositoryAiKeySchema = z.object({
-  provider: z.enum(["anthropic", "openai"]),
-  key: z.string().trim().min(20).max(400),
-  model: z.string().trim().min(1).max(120).nullable().optional(),
-});
+// prefix check catches the common provider/key mismatch (e.g. pasting an OpenAI key under Anthropic)
+// without coupling to exact provider key formats: Anthropic keys start with `sk-ant-`; OpenAI keys
+// start with `sk-` but never `sk-ant-`.
+const repositoryAiKeySchema = z
+  .object({
+    provider: z.enum(["anthropic", "openai"]),
+    key: z.string().trim().min(20).max(400),
+    model: z.string().trim().min(1).max(120).nullable().optional(),
+  })
+  .refine((value) => (value.provider === "anthropic" ? value.key.startsWith("sk-ant-") : value.key.startsWith("sk-") && !value.key.startsWith("sk-ant-")), {
+    message: "API key does not match the selected provider (Anthropic keys start with sk-ant-, OpenAI keys start with sk-).",
+    path: ["key"],
+  });
 
 // Maintainer-settable AI-review config (the non-secret subset of settings). The secret key is set
 // separately via the ai-key route; never here.

src/auth/rate-limit.ts

@@ -104,6 +104,8 @@ export function routeClassForPath(path: string): RateLimitClass {
     path.includes("/scoring/preview") ||
     path.includes("/decision-pack") ||
     path.includes("/open-pr-monitor") ||
+    // Maintainer BYOK config: POST /ai-key runs PBKDF2 (100k iters) + an encrypted D1 upsert per request.
+    /\/ai-(?:key|review)$/.test(path) ||
     /^\/v1\/installations\/[^/]+\/repair\/refresh$/.test(path) ||
     path.includes("/upstream/") ||
     path.includes("/internal/jobs/generate-signal-snapshots") ||

src/services/ai-review.ts

@@ -212,8 +212,18 @@ const PROVIDER_DEFAULT_MODEL: Record<AiReviewProviderKey["provider"], string> =
   openai: "gpt-4o",
 };
 
-/** Run the maintainer's BYOK frontier model for the advisory write-up. Never throws; null on any error. */
-async function runProviderReview(providerKey: AiReviewProviderKey, system: string, user: string, maxTokens: number): Promise<ModelReview | null> {
+/** Hard cap on a single BYOK provider request. Without it a slow/half-open Anthropic/OpenAI connection
+ *  would stall the queue worker for as long as the platform allows; a bounded timeout turns the hang into
+ *  the existing fail-safe null path. Mirrors the github/gittensor fetch-timeout convention. */
+const AI_PROVIDER_TIMEOUT_MS = 20_000;
+
+/** Why a BYOK advisory call produced no review — surfaced in the audit event for observability (never a key). */
+type ProviderFailure = "timeout" | "http_error" | "exception";
+type ProviderReviewOutcome = { review: ModelReview | null; failure?: ProviderFailure };
+
+/** Run the maintainer's BYOK frontier model for the advisory write-up. Never throws; the review is null on
+ *  any error and `failure` names the reason (timeout/http_error/exception) for the audit trail. */
+async function runProviderReview(providerKey: AiReviewProviderKey, system: string, user: string, maxTokens: number): Promise<ProviderReviewOutcome> {
   const model = providerKey.model || PROVIDER_DEFAULT_MODEL[providerKey.provider];
   try {
     let response: Response;
@@ -222,6 +232,7 @@ async function runProviderReview(providerKey: AiReviewProviderKey, system: strin
         method: "POST",
         headers: { "content-type": "application/json", "x-api-key": providerKey.key, "anthropic-version": "2023-06-01" },
         body: JSON.stringify({ model, max_tokens: maxTokens, system, messages: [{ role: "user", content: user }] }),
+        signal: AbortSignal.timeout(AI_PROVIDER_TIMEOUT_MS),
       });
     } else {
       response = await fetch("https://api.openai.com/v1/chat/completions", {
@@ -235,12 +246,15 @@ async function runProviderReview(providerKey: AiReviewProviderKey, system: strin
             { role: "user", content: user },
           ],
         }),
+        signal: AbortSignal.timeout(AI_PROVIDER_TIMEOUT_MS),
       });
     }
-    if (!response.ok) return null;
-    return parseModelReview(coerceAiText(await response.json()));
-  } catch {
-    return null;
+    if (!response.ok) return { review: null, failure: "http_error" };
+    return { review: parseModelReview(coerceAiText(await response.json())) };
+  } catch (error) {
+    // AbortSignal.timeout rejects with a TimeoutError; everything else is a network/parse exception.
+    const failure: ProviderFailure = (error as { name?: string } | null)?.name === "TimeoutError" ? "timeout" : "exception";
+    return { review: null, failure };
   }
 }
 
@@ -302,9 +316,15 @@ export async function runGittensoryAiReview(env: Env, input: GittensoryAiReviewI
   }
 
   // Advisory write-up: BYOK frontier model if configured, else the free Workers-AI primary (with fallback).
-  const advisoryReview = input.providerKey
-    ? await runProviderReview(input.providerKey, REVIEW_SYSTEM_PROMPT, user, maxTokens)
-    : await runWorkersOpinion(env, BEST_REVIEW_MODELS[0], RELIABLE_FALLBACK_MODELS[0], REVIEW_SYSTEM_PROMPT, user, maxTokens);
+  let byokFailure: ProviderFailure | undefined;
+  let advisoryReview: ModelReview | null;
+  if (input.providerKey) {
+    const outcome = await runProviderReview(input.providerKey, REVIEW_SYSTEM_PROMPT, user, maxTokens);
+    advisoryReview = outcome.review;
+    byokFailure = outcome.failure;
+  } else {
+    advisoryReview = await runWorkersOpinion(env, BEST_REVIEW_MODELS[0], RELIABLE_FALLBACK_MODELS[0], REVIEW_SYSTEM_PROMPT, user, maxTokens);
+  }
 
   let consensusDefect: AiConsensusDefect | null = null;
   let secondReview: ModelReview | null = null;
@@ -325,6 +345,7 @@ export async function runGittensoryAiReview(env: Env, input: GittensoryAiReviewI
     mode: input.mode,
     byok: Boolean(input.providerKey),
     consensus: Boolean(consensusDefect),
+    ...(byokFailure ? { byokFailure } : {}),
   });
   return { status: "ok", advisoryNotes, consensusDefect, estimatedNeurons };
 }

test/unit/ai-review.test.ts

@@ -134,14 +134,35 @@ describe("BYOK provider dispatch", () => {
     const result = await runGittensoryAiReview(env, { ...baseInput, providerKey: { provider: "anthropic", key: "sk-ant-secret" } });
     expect(result.status === "ok" && result.advisoryNotes).toContain("BYOK review.");
     expect(fetchMock.mock.calls[0]?.[0]).toBe("https://api.anthropic.com/v1/messages");
+    // The provider fetch must carry a timeout signal so a hung provider can't stall the queue worker.
+    expect((fetchMock.mock.calls[0]?.[1] as RequestInit | undefined)?.signal).toBeInstanceOf(AbortSignal);
     expect(run).not.toHaveBeenCalled(); // advisory mode + BYOK → no Workers AI call
   });
 
-  it("falls back to no notes when the provider returns a non-200", async () => {
+  it("falls back to no notes when the provider returns a non-200 and records the failure reason", async () => {
     vi.stubGlobal("fetch", vi.fn(async () => new Response("nope", { status: 401 })));
     const env = createTestEnv({ AI: { run: vi.fn() } as unknown as Ai, AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true", AI_DAILY_NEURON_BUDGET: "100000" });
     const result = await runGittensoryAiReview(env, { ...baseInput, providerKey: { provider: "openai", key: "sk-secret" } });
     expect(result.status === "ok" && result.advisoryNotes).toBeNull();
+    // The audit event names the failure (observability) and NEVER includes key material.
+    const row = await env.DB.prepare("select metadata_json from ai_usage_events where feature = ? order by rowid desc limit 1").bind("ai_review_pr").first<{ metadata_json: string }>();
+    expect(JSON.parse(row?.metadata_json ?? "{}").byokFailure).toBe("http_error");
+    expect(row?.metadata_json ?? "").not.toContain("sk-secret");
+  });
+
+  it("records a timeout failure when the provider fetch aborts", async () => {
+    vi.stubGlobal(
+      "fetch",
+      vi.fn(async () => {
+        // Mirror AbortSignal.timeout's rejection (a TimeoutError DOMException-shaped error).
+        throw Object.assign(new Error("The operation timed out."), { name: "TimeoutError" });
+      }),
+    );
+    const env = createTestEnv({ AI: { run: vi.fn() } as unknown as Ai, AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true", AI_DAILY_NEURON_BUDGET: "100000" });
+    const result = await runGittensoryAiReview(env, { ...baseInput, providerKey: { provider: "anthropic", key: "sk-ant-secret" } });
+    expect(result.status === "ok" && result.advisoryNotes).toBeNull();
+    const row = await env.DB.prepare("select metadata_json from ai_usage_events where feature = ? order by rowid desc limit 1").bind("ai_review_pr").first<{ metadata_json: string }>();
+    expect(JSON.parse(row?.metadata_json ?? "{}").byokFailure).toBe("timeout");
   });
 
   it("falls back to no notes when the provider fetch throws, and honors a model override", async () => {

test/unit/auth.test.ts

@@ -91,6 +91,9 @@ describe("private-beta auth and rate limiting", () => {
     expect(routeClassForPath("/v1/internal/jobs/build-contributor-decision-packs")).toBe("expensive");
     expect(routeClassForPath("/v1/internal/jobs/refresh-upstream-drift")).toBe("expensive");
     expect(routeClassForPath("/v1/internal/queue-intelligence")).toBe("expensive");
+    // Maintainer BYOK config writes run PBKDF2 + an encrypted upsert; they are rate-limited as expensive.
+    expect(routeClassForPath("/v1/repos/acme/widgets/ai-key")).toBe("expensive");
+    expect(routeClassForPath("/v1/repos/acme/widgets/ai-review")).toBe("expensive");
     expect(routeClassForPath("/v1/repos")).toBe("normal");
   });
 

test/unit/routes-ai-byok.test.ts

@@ -78,6 +78,19 @@ describe("maintainer BYOK key route", () => {
     expect(res.status).toBe(400);
   });
 
+  it("rejects a key whose prefix does not match the selected provider (400)", async () => {
+    const app = createApp();
+    const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET });
+    // An OpenAI-shaped key stored under Anthropic, and an Anthropic key stored under OpenAI — both rejected.
+    const wrongAnthropic = await app.request(`/v1/repos/${REPO}/ai-key`, { method: "POST", headers: apiHeaders(env), body: JSON.stringify({ provider: "anthropic", key: "sk-openai-not-anthropic-123456" }) }, env);
+    expect(wrongAnthropic.status).toBe(400);
+    const wrongOpenai = await app.request(`/v1/repos/${REPO}/ai-key`, { method: "POST", headers: apiHeaders(env), body: JSON.stringify({ provider: "openai", key: "sk-ant-not-openai-1234567890" }) }, env);
+    expect(wrongOpenai.status).toBe(400);
+    // An OpenAI key that doesn't start with sk- at all is also rejected.
+    const noPrefix = await app.request(`/v1/repos/${REPO}/ai-key`, { method: "POST", headers: apiHeaders(env), body: JSON.stringify({ provider: "openai", key: "ghp-not-a-provider-key-12345" }) }, env);
+    expect(noPrefix.status).toBe(400);
+  });
+
   it("reports 503 when key storage (encryption secret) is unavailable", async () => {
     const app = createApp();
     const env = createTestEnv({});