From ec60915d9170e9947d2b5a627162b6dd37323eed Mon Sep 17 00:00:00 2001 From: Fouad Salkini Date: Thu, 17 Sep 2026 22:26:42 +0300 Subject: [PATCH] fix(api): accept blockedModels in the key permissions schema (#13666) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(api): accept blockedModels in the key permissions schema `PATCH /api/keys/[id]` already destructures `blockedModels`, forwards it into the update payload, and `updateApiKeyPermissions()` writes it to the `blocked_models` column. Only the first link was missing: `updateKeyPermissionsSchema` never declared the field, so Zod stripped it from the parsed body and the destructured value was always `undefined`. The request answered 200 and wrote nothing. The API Manager permissions modal sends `blockedModels` on every save (ApiManagerPageClient.tsx), so the Claude-Code family-blocking control silently did nothing and an existing deny-list could not be cleared. `blockedModels` is the deny-list half of the model policy — read by `isModelAllowedForKey()` before the allow-list and winning over it — so that half was only reachable by editing the database by hand. Declare the field mirroring `allowedModels` (trimmed, non-empty, max 1000) and count it in the "No valid fields to update" guard so a body carrying only `blockedModels` is a valid update. Left out of `createKeySchema` deliberately: the create route does not read `blockedModels`, so declaring it there would be dead weight. * docs(changelog): add fragment for blockedModels key schema fix Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --- .../fixes/api-key-blocked-models-schema.md | 1 + src/shared/validation/schemas/keys.ts | 2 + .../api-keys-blocked-models-schema.test.ts | 126 ++++++++++++++++++ 3 files changed, 129 insertions(+) create mode 100644 changelog.d/fixes/api-key-blocked-models-schema.md create mode 100644 tests/unit/api-keys-blocked-models-schema.test.ts diff --git a/changelog.d/fixes/api-key-blocked-models-schema.md b/changelog.d/fixes/api-key-blocked-models-schema.md new file mode 100644 index 0000000000..6541967f69 --- /dev/null +++ b/changelog.d/fixes/api-key-blocked-models-schema.md @@ -0,0 +1 @@ +- **fix(api):** accept `blockedModels` in the key permissions update schema so the deny-list half of per-key model policy is no longer silently stripped before it reaches the route diff --git a/src/shared/validation/schemas/keys.ts b/src/shared/validation/schemas/keys.ts index 1441c8851f..f301158d19 100644 --- a/src/shared/validation/schemas/keys.ts +++ b/src/shared/validation/schemas/keys.ts @@ -125,6 +125,7 @@ export const updateKeyPermissionsSchema = z modelAccessMode: z.enum(["all", "restricted"]).optional(), connectionAccessMode: z.enum(["all", "restricted"]).optional(), allowedModels: z.array(z.string().trim().min(1)).max(1000).optional(), + blockedModels: z.array(z.string().trim().min(1)).max(1000).optional(), allowedCombos: z.array(z.string().trim().min(1).max(200)).max(500).optional(), allowedConnections: z.array(z.string().uuid()).max(100).optional(), noLog: z.boolean().optional(), @@ -191,6 +192,7 @@ export const updateKeyPermissionsSchema = z value.modelAccessMode === undefined && value.connectionAccessMode === undefined && value.allowedModels === undefined && + value.blockedModels === undefined && value.allowedCombos === undefined && value.allowedConnections === undefined && value.noLog === undefined && diff --git a/tests/unit/api-keys-blocked-models-schema.test.ts b/tests/unit/api-keys-blocked-models-schema.test.ts new file mode 100644 index 0000000000..9c27cc6b5f --- /dev/null +++ b/tests/unit/api-keys-blocked-models-schema.test.ts @@ -0,0 +1,126 @@ +/** + * `blockedModels` is plumbed end-to-end but unreachable through the API. + * + * `PATCH /api/keys/[id]` destructures `blockedModels` from `validation.data` + * (src/app/api/keys/[id]/route.ts), forwards it into the payload, and + * `updateApiKeyPermissions()` writes it to the `blocked_models` column + * (src/lib/db/apiKeys.ts). Every link exists EXCEPT the first one: + * `updateKeyPermissionsSchema` never declared the field, so Zod stripped it + * from the parsed body and the destructured value was always `undefined`. + * The request answered 200 and wrote nothing — a silent no-op. + * + * Observed on a live gateway: PATCH with + * { modelAccessMode: "all", allowedModels: [], blockedModels: [] } + * returned 200 and echoed modelAccessMode/allowedModels back, while the + * `blocked_models` column stayed NULL. + * + * `blockedModels` is the deny-list half of the model policy — it is read by + * `isModelAllowedForKey()` (src/lib/db/apiKeys.ts) BEFORE the allow-list and + * wins over it, which is what lets an operator keep a broad scope like `cc/*` + * while excluding specific families. With the field unsettable, that half of + * the policy could only ever be written by seeding the database by hand. + * + * Rules: + * R1 The schema preserves `blockedModels` verbatim. + * R2 `blockedModels` alone is a valid update (the "No valid fields" guard counts it). + * R3 It coexists with the allow-list, including under modelAccessMode "all". + * R4 Entry-level validation matches `allowedModels` (trimmed, non-empty, max 1000). + * R5 An empty array is preserved — that is how a deny-list is cleared. + * R6 The update route still forwards the field it destructures. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; + +const schemas = await import("../../src/shared/validation/schemas.ts"); + +test("R1: the PATCH schema preserves blockedModels verbatim", () => { + const parsed = schemas.updateKeyPermissionsSchema.safeParse({ + modelAccessMode: "restricted", + allowedModels: ["cc/*"], + blockedModels: ["*/*", "gpt-5.6-sol-xhigh"], + }); + assert.equal(parsed.success, true, "a body carrying blockedModels must parse"); + if (!parsed.success) return; + assert.deepEqual( + parsed.data.blockedModels, + ["*/*", "gpt-5.6-sol-xhigh"], + "blockedModels must survive parsing — stripping it makes the PATCH a silent no-op" + ); +}); + +test("R2: blockedModels on its own is a valid update", () => { + const parsed = schemas.updateKeyPermissionsSchema.safeParse({ + blockedModels: ["anthropic/*"], + }); + assert.equal( + parsed.success, + true, + "the 'No valid fields to update' guard must count blockedModels as a field" + ); + if (!parsed.success) return; + assert.deepEqual(parsed.data.blockedModels, ["anthropic/*"]); +}); + +test("R3: blockedModels coexists with the allow-list and with modelAccessMode 'all'", () => { + // The allow-list must stay empty under "all"; the deny-list has no such rule, + // because deny-over-broad-scope is exactly its purpose. + const parsed = schemas.updateKeyPermissionsSchema.safeParse({ + modelAccessMode: "all", + allowedModels: [], + blockedModels: ["openai/*"], + }); + assert.equal(parsed.success, true, "a deny-list must be settable on an allow-all key"); + if (!parsed.success) return; + assert.deepEqual(parsed.data.blockedModels, ["openai/*"]); + assert.deepEqual(parsed.data.allowedModels, []); +}); + +test("R4: entry validation matches allowedModels", () => { + const trimmed = schemas.updateKeyPermissionsSchema.safeParse({ + blockedModels: [" openai/* "], + }); + assert.equal(trimmed.success, true); + if (trimmed.success) { + assert.deepEqual(trimmed.data.blockedModels, ["openai/*"], "entries must be trimmed"); + } + + assert.equal( + schemas.updateKeyPermissionsSchema.safeParse({ blockedModels: [""] }).success, + false, + "an empty pattern must be rejected" + ); + assert.equal( + schemas.updateKeyPermissionsSchema.safeParse({ blockedModels: "openai/*" }).success, + false, + "a bare string must be rejected — the column stores a list" + ); + assert.equal( + schemas.updateKeyPermissionsSchema.safeParse({ + blockedModels: Array.from({ length: 1001 }, (_, i) => `m${i}`), + }).success, + false, + "more than 1000 entries must be rejected, as for allowedModels" + ); +}); + +test("R5: an empty blockedModels array is preserved so a deny-list can be cleared", () => { + const parsed = schemas.updateKeyPermissionsSchema.safeParse({ blockedModels: [] }); + assert.equal(parsed.success, true, "clearing the deny-list must be a valid update"); + if (!parsed.success) return; + assert.deepEqual( + parsed.data.blockedModels, + [], + "an empty array must survive as [] — undefined would skip the column write" + ); +}); + +test("R6: the update route forwards the blockedModels it destructures", () => { + const route = fs.readFileSync(path.join(process.cwd(), "src/app/api/keys/[id]/route.ts"), "utf8"); + assert.ok( + route.includes("if (blockedModels !== undefined) payload.blockedModels = blockedModels"), + "the route must keep forwarding blockedModels into the update payload" + ); +});