From 65c296d430d0386972f69bff809c0d0d7fd692a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 21 Oct 2026 15:07:06 +0200 Subject: [PATCH] fix(ai): audit setting and removing a stored provider key Removing a provider key, like setting one, left no audit row, unlike every other credential change. Each key slot a PATCH touches now writes user.ai-provider.key.set or .remove with the slot and never the value. --- .../user/ai-provider/__tests__/route.test.ts | 37 +++++++++++++++++++ src/app/api/user/ai-provider/route.ts | 25 +++++++++++++ .../ai-provider-key-removal.test.ts | 11 ++++++ 3 files changed, 73 insertions(+) diff --git a/src/app/api/user/ai-provider/__tests__/route.test.ts b/src/app/api/user/ai-provider/__tests__/route.test.ts index 0eb415494..e7ea24c49 100644 --- a/src/app/api/user/ai-provider/__tests__/route.test.ts +++ b/src/app/api/user/ai-provider/__tests__/route.test.ts @@ -63,6 +63,10 @@ vi.mock("@/lib/ai/consent-guard", () => ({ hasActiveConsentForSurface: vi.fn(), })); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn(), +})); + import { GET, PATCH } from "../route"; import { prisma } from "@/lib/db"; import { resolveProviderAvailability } from "@/lib/ai/provider"; @@ -72,6 +76,7 @@ import { readServerProviderHealth } from "@/lib/ai/server-provider-health"; import { getAssistantFlags } from "@/lib/feature-flags"; import { providerCredentialPolicy } from "@/lib/sharing/provider-work-authority"; import { hasActiveConsentForSurface } from "@/lib/ai/consent-guard"; +import { auditLog } from "@/lib/auth/audit"; function patchRequest(body: unknown): Request { return new Request("http://localhost/api/user/ai-provider", { @@ -243,6 +248,38 @@ describe("PATCH /api/user/ai-provider — v1.22 fields", () => { }); }); +describe("PATCH /api/user/ai-provider — key changes are audited", () => { + const patch = PATCH as (req: Request) => Promise; + + it("audits a removed key, naming its slot and never a value", async () => { + const res = await patch(patchRequest({ anthropicKey: null })); + expect(res.status).toBe(200); + expect(vi.mocked(auditLog)).toHaveBeenCalledTimes(1); + expect(vi.mocked(auditLog)).toHaveBeenCalledWith( + "user.ai-provider.key.remove", + expect.objectContaining({ + userId: "u-1", + details: { slot: "anthropic" }, + }), + ); + }); + + it("audits a stored key without its value", async () => { + await patch(patchRequest({ openaiKey: "sk-test-123", compatKey: "" })); + const calls = vi.mocked(auditLog).mock.calls; + expect(calls.map(([action, opts]) => [action, opts?.details])).toEqual([ + ["user.ai-provider.key.set", { slot: "openai" }], + ["user.ai-provider.key.remove", { slot: "openai-compatible" }], + ]); + expect(JSON.stringify(calls)).not.toContain("sk-test-123"); + }); + + it("writes no key audit for a change that touches no key", async () => { + await patch(patchRequest({ responseTimeoutSeconds: 120 })); + expect(vi.mocked(auditLog)).not.toHaveBeenCalled(); + }); +}); + // v1.33.1 (#470) — the OpenAI-compatible gateway writes three columns of its // own. What matters at this layer: the SSRF floor applies to the gateway's // base URL exactly as it does to the Local one, and the column is nobody diff --git a/src/app/api/user/ai-provider/route.ts b/src/app/api/user/ai-provider/route.ts index 1b976d5ec..ecac527a3 100644 --- a/src/app/api/user/ai-provider/route.ts +++ b/src/app/api/user/ai-provider/route.ts @@ -4,9 +4,11 @@ import { prisma } from "@/lib/db"; import { apiSuccess, apiError, + getClientIp, returnAllZodIssues, safeJson, } from "@/lib/api-response"; +import { auditLog } from "@/lib/auth/audit"; import { isPublicUrl } from "@/lib/validations/notifications"; import { isLocalAiHostAllowed } from "@/lib/ai/local-host-allowlist"; import { encrypt, decrypt } from "@/lib/crypto"; @@ -374,5 +376,28 @@ export const PATCH = apiHandler(async (request: NextRequest) => { await prisma.user.update({ where: { id: user.id }, data: updates }); } + // A stored provider key is a credential: setting or removing one is + // audited like every other credential change, with the slot it went into + // and never the value. A removal used to leave no trace at all. + const keySlots = [ + ["anthropic", body.anthropicKey], + ["openai", body.openaiKey], + ["local", body.localKey], + ["openai-compatible", body.compatKey], + ] as const; + for (const [slot, value] of keySlots) { + if (value === undefined) continue; + await auditLog( + value === null || value === "" + ? "user.ai-provider.key.remove" + : "user.ai-provider.key.set", + { + userId: user.id, + ipAddress: getClientIp(request), + details: { slot }, + }, + ); + } + return apiSuccess({ updated: true }); }); diff --git a/tests/integration/ai-provider-key-removal.test.ts b/tests/integration/ai-provider-key-removal.test.ts index 3eae163bf..c1944acf7 100644 --- a/tests/integration/ai-provider-key-removal.test.ts +++ b/tests/integration/ai-provider-key-removal.test.ts @@ -111,6 +111,17 @@ describe("PATCH /api/user/ai-provider — removing one saved key", () => { expect(row.aiCompatKeyEncrypted).not.toBeNull(); expect(row.aiProvider).toBe("OPENAI"); expect(row.aiModel).toBe("gpt-4o-mini"); + // The removal is audited, naming the slot and no value. + const audits = await getPrismaClient().auditLog.findMany({ + where: { userId, action: { startsWith: "user.ai-provider.key." } }, + select: { action: true, details: true }, + }); + expect(audits).toEqual([ + { + action: "user.ai-provider.key.remove", + details: JSON.stringify({ slot: "openai" }), + }, + ]); }); it("the read the form seeds from reports the key gone", async () => {