diff --git a/apps/gittensory-ui/src/components/site/app-panels/ai-review-settings.tsx b/apps/gittensory-ui/src/components/site/app-panels/ai-review-settings.tsx index 1cd9f16084..c04919610a 100644 --- a/apps/gittensory-ui/src/components/site/app-panels/ai-review-settings.tsx +++ b/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(`${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) { diff --git a/apps/gittensory-ui/src/components/site/app-panels/maintainer-panel.tsx b/apps/gittensory-ui/src/components/site/app-panels/maintainer-panel.tsx index fc5938844f..3ad1294417 100644 --- a/apps/gittensory-ui/src/components/site/app-panels/maintainer-panel.tsx +++ b/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 ; + if (!isMaintainer) { + return ( + + ); + } + return ; +} + +function MaintainerDashboardView() { const dashboard = useApiResource( "/v1/app/maintainer-dashboard", "Maintainer dashboard", diff --git a/src/api/routes.ts b/src/api/routes.ts index dd05f9f25b..51b9b9a49e 100644 --- a/src/api/routes.ts +++ b/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. diff --git a/src/auth/rate-limit.ts b/src/auth/rate-limit.ts index 2e88caa76f..f744937b87 100644 --- a/src/auth/rate-limit.ts +++ b/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") || diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 53a25af62e..adf366eb0c 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -212,8 +212,18 @@ const PROVIDER_DEFAULT_MODEL: Record = 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 { +/** 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 { 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 }; } diff --git a/test/unit/ai-review.test.ts b/test/unit/ai-review.test.ts index 9502b6b100..c67d7b9dcc 100644 --- a/test/unit/ai-review.test.ts +++ b/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 () => { diff --git a/test/unit/auth.test.ts b/test/unit/auth.test.ts index f493f93550..f5e1a9dfe0 100644 --- a/test/unit/auth.test.ts +++ b/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"); }); diff --git a/test/unit/routes-ai-byok.test.ts b/test/unit/routes-ai-byok.test.ts index 186567e576..39f22042b2 100644 --- a/test/unit/routes-ai-byok.test.ts +++ b/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({});