From c0f92ec98a981cc13c25e44048a04b33c62a3112 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Tue, 15 Sep 2026 01:06:16 -0300 Subject: [PATCH] fix(security): resolve findings from omni-code-sec battery (fix/batches-delete-completed-authz) (#13684) Batch sweep enforces the caller's API-key policy (allowedEndpoints/schedule/usage/rate limit; the /api/v1 pathname now resolves its endpoint category for every /v1 route), commits per 200-batch chunk in key mode, guards against no-progress loops, rejects a scope naming both a key and allTenants; 8 covering tests registered for the mutation gate. Remaining CI reds are release base-reds (#12732), reproduced identically on the base tip. Refs #12969, #13680, #13681, #13685, #13377 --- .../13684-batches-sweep-policy-and-lock.md | 1 + .../api/v1/batches/delete-completed/route.ts | 16 ++- src/lib/db/batches.ts | 99 ++++++++++--------- src/shared/utils/apiKeyPolicy.ts | 7 +- stryker.conf.json | 10 +- ...es-delete-completed-ownership-wvxc.test.ts | 78 ++++++++++++--- ...tches-delete-completed-route-scope.test.ts | 29 +++++- tests/unit/endpoint-categories.test.ts | 9 +- .../unit/endpoint-restrictions-policy.test.ts | 17 ++++ 9 files changed, 201 insertions(+), 65 deletions(-) create mode 100644 changelog.d/fixes/13684-batches-sweep-policy-and-lock.md diff --git a/changelog.d/fixes/13684-batches-sweep-policy-and-lock.md b/changelog.d/fixes/13684-batches-sweep-policy-and-lock.md new file mode 100644 index 0000000000..746eb6db91 --- /dev/null +++ b/changelog.d/fixes/13684-batches-sweep-policy-and-lock.md @@ -0,0 +1 @@ +- **fix(api):** `DELETE /v1/batches/delete-completed` now applies the caller's API-key policy (endpoint allowlist, schedule, usage cap, rate limit) like every other `/v1` route, commits the key-scoped sweep per 200-batch chunk instead of holding one write lock for the whole sweep, refuses to loop without progress, and rejects a scope that names both a key and `allTenants` ([#13684](https://github.com/diegosouzapw/OmniRoute/pull/13684)) diff --git a/src/app/api/v1/batches/delete-completed/route.ts b/src/app/api/v1/batches/delete-completed/route.ts index 6f6448f92a..fa60d98d61 100644 --- a/src/app/api/v1/batches/delete-completed/route.ts +++ b/src/app/api/v1/batches/delete-completed/route.ts @@ -3,6 +3,7 @@ import { deleteCompletedBatches, type DeleteCompletedBatchesScope } from "@/lib/ import { validateApiKey } from "@/lib/db/apiKeys"; import { NextResponse } from "next/server"; import { getApiKeyRequestScope } from "@/app/api/v1/_helpers/apiKeyScope"; +import { enforceApiKeyPolicy } from "@/shared/utils/apiKeyPolicy"; import { buildErrorBody } from "@omniroute/open-sse/utils/error"; import * as log from "@/sse/utils/logger"; @@ -24,7 +25,11 @@ export async function DELETE(request: Request) { // gate (is_active, revoked_at, is_banned, expires_at) — and neither case may // fall through to the session branch and widen the sweep to the whole instance. if (scope.apiKey && (!scope.apiKeyId || !(await validateApiKey(scope.apiKey)))) { - log.warn("BATCHES", "delete-completed: presented API key rejected", { + // `info`, not `warn`: any caller can reach this branch by presenting any + // string as a key, so a warn-level line per attempt is a log-flooding lever + // (LEDGER-12). The 401 itself is the audit signal; the real sweeps below + // keep their warn-level audit lines. + log.info("BATCHES", "delete-completed: presented API key rejected", { route: LOG_ROUTE, reason: scope.apiKeyId ? "invalid" : "unresolved", apiKeyId: scope.apiKeyId, @@ -36,6 +41,15 @@ export async function DELETE(request: Request) { }); } + // The per-key operator policy every other `/v1` route applies (endpoint + // allowlist, access schedule, usage cap, rate limit — LEDGER-9/13/16). Runs + // after the lifecycle gate above (the enforcer's own status check does not + // look at `revoked_at`) and before the sweep scope is chosen, so a restricted + // key is refused with the enforcer's own rejection and nothing is swept. A + // session-only caller carries no key and passes through untouched. + const policy = await enforceApiKeyPolicy(request, null); + if (policy.rejection) return policy.rejection; + // A presented API key always scopes the sweep to that key — even when the // request also carries a dashboard session cookie — exactly like the // list/count siblings (`apiKeyId || undefined`), so a leaked or over-shared diff --git a/src/lib/db/batches.ts b/src/lib/db/batches.ts index 0f03cb47ce..9d3a8e4531 100644 --- a/src/lib/db/batches.ts +++ b/src/lib/db/batches.ts @@ -421,7 +421,7 @@ export function deleteBatch(id: string): boolean { */ export type DeleteCompletedBatchesScope = { apiKeyId: string } | { allTenants: true }; -/** Instance-wide sweeps commit in chunks of this many batches (SEC-D). */ +/** Both sweep modes commit in chunks of this many batches (SEC-D, LEDGER-4). */ export const INSTANCE_SWEEP_CHUNK = 200; /** @@ -449,18 +449,23 @@ export const INSTANCE_SWEEP_CHUNK = 200; * The file soft-deletes, the checkpoint DELETE and the batches DELETE for a set * of batch ids run in one transaction, so a mid-sweep failure rolls that set back * — within a chunk, no batch row is left pointing at a file whose content was - * already nulled. Both modes walk the key's/instance's completed batches in - * chunks of `INSTANCE_SWEEP_CHUNK` ids and run that unit once per chunk. - * Key mode wraps the whole chunk loop in ONE outer transaction (a nested - * transaction call is a savepoint on every adapter), so the key sweep stays - * all-or-nothing: a failure in any chunk rolls every earlier chunk back too. - * Instance mode (`allTenants`) commits per chunk (SEC-D): a large sweep never - * holds one write lock over the whole table, each chunk stays atomic, and a - * failure inside chunk N leaves chunks < N committed, chunk N fully rolled - * back, and rethrows. Inherent to per-chunk commits: a file shared by batches - * in two different chunks can be nulled by chunk 1 before chunk 2 fails; the - * surviving batch row is swept by the next run. The returned totals sum the - * chunks. + * already nulled. BOTH modes walk the key's/instance's completed batches in + * chunks of `INSTANCE_SWEEP_CHUNK` ids and commit that unit once per chunk + * (SEC-D; key mode since the omni-code-sec proof run, LEDGER-4/20/21): the + * SQLite write lock is held for one chunk at a time and never across chunks, + * so the lowest-privilege caller — any valid API key — cannot hold the + * instance's single writer for the length of its whole sweep. Each chunk stays + * atomic: a failure inside chunk N leaves chunks < N committed, chunk N fully + * rolled back, and rethrows. Inherent to per-chunk commits, in either mode: a + * file shared by batches in two different chunks can be nulled by chunk 1 + * before chunk 2 fails; the surviving batch row is swept by the next run. The + * returned totals sum the chunks. + * + * The loop must make progress: it remembers the first id of the previous chunk + * and throws if the next chunk starts with the same id — the DELETE removed + * nothing (e.g. a trigger ignored it), and re-selecting the same rows would + * spin forever (LEDGER-22). Rows vanishing under a concurrent deleter are fine: + * the next chunk then starts with a different id or is empty. * * The ids of a unit are bound as `IN (?, …)` placeholders. No statement ever * binds more than `INSTANCE_SWEEP_CHUNK` ids in either mode, so a tenant with @@ -477,17 +482,18 @@ export function deleteCompletedBatches(scope: DeleteCompletedBatchesScope): { if (!allTenants && (typeof apiKeyId !== "string" || apiKeyId.trim() === "")) { throw new Error("deleteCompletedBatches: apiKeyId required unless allTenants"); } - if (allTenants && apiKeyId) { + // Presence, not truthiness: `{ allTenants: true, apiKeyId: "" }` (or null) is a + // caller that named both fields and must be refused, not widened (LEDGER-18). + if (allTenants && "apiKeyId" in scopeObj) { throw new Error("deleteCompletedBatches: apiKeyId and allTenants are mutually exclusive"); } const db = getDbInstance(); // One consistent unit: file soft-deletes → checkpoints → batch rows for a - // given set of batch ids. Both modes run it per chunk of INSTANCE_SWEEP_CHUNK - // ids; key mode nests the chunks in one outer transaction, instance mode - // commits each chunk so a large sweep never holds one write-lock for the - // whole table (SEC-D) while each chunk stays atomic. + // given set of batch ids. Both modes run — and commit — it once per chunk of + // INSTANCE_SWEEP_CHUNK ids, so a large sweep never holds one write lock for + // the whole table (SEC-D, LEDGER-4) while each chunk stays atomic. const sweepIds = db.transaction((ids: string[]) => { if (ids.length === 0) return { deletedBatches: 0, deletedFiles: 0 }; const marks = ids.map(() => "?").join(","); @@ -529,41 +535,42 @@ export function deleteCompletedBatches(scope: DeleteCompletedBatchesScope): { return { deletedBatches: result.changes, deletedFiles }; }); + // The one chunk loop both modes share: select the next chunk of ids, sweep + // it in its own committed transaction, sum. The only difference between the + // modes is the SELECT that produces the next chunk. No outer transaction — + // the write lock is released between chunks (LEDGER-4/20/21). + const sweepLoop = (nextIds: () => string[]) => { + const totals = { deletedBatches: 0, deletedFiles: 0 }; + let previousFirstId: string | null = null; + for (;;) { + const ids = nextIds(); + if (ids.length === 0) break; + // Forward-progress guard (LEDGER-22): the chunk is re-selected from the + // table after each commit, so a repeated first id means the previous + // DELETE removed nothing and the loop would spin forever. A concurrent + // deleter only makes rows vanish, which yields a different first id. + if (ids[0] === previousFirstId) { + throw new Error(`deleteCompletedBatches: no progress — chunk repeated (id ${ids[0]})`); + } + previousFirstId = ids[0]; + const part = sweepIds(ids); + totals.deletedBatches += part.deletedBatches; + totals.deletedFiles += part.deletedFiles; + } + return totals; + }; + + const toIds = (rows: unknown[]) => (rows as Array<{ id: string }>).map((r) => r.id); + if (!allTenants) { - // One outer transaction so the key sweep stays all-or-nothing; inside it, - // the same 200-id unit as instance mode (nested transaction calls become - // savepoints on every adapter), so no statement ever binds more than - // INSTANCE_SWEEP_CHUNK ids — a tenant with tens of thousands of completed - // batches must not hit SQLite's 32766 bound-parameter ceiling. const keyChunk = db.prepare( "SELECT id FROM batches WHERE status = 'completed' AND api_key_id = ? ORDER BY rowid LIMIT ?" ); - const sweepKey = db.transaction(() => { - const totals = { deletedBatches: 0, deletedFiles: 0 }; - for (;;) { - const ids = (keyChunk.all(apiKeyId, INSTANCE_SWEEP_CHUNK) as Array<{ id: string }>).map( - (r) => r.id - ); - if (ids.length === 0) break; - const part = sweepIds(ids); - totals.deletedBatches += part.deletedBatches; - totals.deletedFiles += part.deletedFiles; - } - return totals; - }); - return sweepKey(); + return sweepLoop(() => toIds(keyChunk.all(apiKeyId, INSTANCE_SWEEP_CHUNK))); } - const totals = { deletedBatches: 0, deletedFiles: 0 }; const nextChunk = db.prepare( "SELECT id FROM batches WHERE status = 'completed' ORDER BY rowid LIMIT ?" ); - for (;;) { - const ids = (nextChunk.all(INSTANCE_SWEEP_CHUNK) as Array<{ id: string }>).map((r) => r.id); - if (ids.length === 0) break; - const part = sweepIds(ids); - totals.deletedBatches += part.deletedBatches; - totals.deletedFiles += part.deletedFiles; - } - return totals; + return sweepLoop(() => toIds(nextChunk.all(INSTANCE_SWEEP_CHUNK))); } diff --git a/src/shared/utils/apiKeyPolicy.ts b/src/shared/utils/apiKeyPolicy.ts index 67d9b4b70e..ad82e10ad7 100644 --- a/src/shared/utils/apiKeyPolicy.ts +++ b/src/shared/utils/apiKeyPolicy.ts @@ -472,7 +472,12 @@ function validateEndpointAccess(context: PolicyContext): Response | null { const { request, apiKeyInfo } = context; if (!apiKeyInfo.allowedEndpoints?.length) return null; try { - const category = resolveEndpointCategory(new URL(request.url).pathname); + // A route handler sees the client's original URL: `/v1/…` when the + // `/v1/:path*` rewrite fired, but `/api/v1/…` when the client hit the App + // Router path directly (no rewrite). The category prefixes are `/v1/…`, so + // strip the `/api` shape or a restricted key silently passes on that path. + const pathname = new URL(request.url).pathname.replace(/^\/api(?=\/v1\/)/, ""); + const category = resolveEndpointCategory(pathname); if (category && !apiKeyInfo.allowedEndpoints.includes(category)) { return errorResponse( HTTP_STATUS.FORBIDDEN, diff --git a/stryker.conf.json b/stryker.conf.json index c54c51d59b..bdfc4aea09 100644 --- a/stryker.conf.json +++ b/stryker.conf.json @@ -440,7 +440,15 @@ "tests/unit/felo-web-runtime-block.test.ts", "tests/unit/microsoft-designer-web-runtime-block.test.ts", "tests/unit/qwen-web-runtime-block.test.ts", - "tests/unit/tunnel-routes-error-sanitization.test.ts" + "tests/unit/tunnel-routes-error-sanitization.test.ts", + "tests/unit/auth-grok-cli-402-shared-wallet.test.ts", + "tests/unit/cline-401-oauth-12594.test.ts", + "tests/unit/cliproxyapi-unknown-provider-400-12800.test.ts", + "tests/unit/combo/combo-skipped-targets-summary.test.ts", + "tests/unit/context-handoff-native-passthrough-bug.test.ts", + "tests/unit/guardrails/visionBridge12111Repro.test.ts", + "tests/unit/guardrails/visionBridgeRouter.test.ts", + "tests/unit/issue-11912-opencode-roundrobin-collapse.test.ts" ], "nodeArgs": [ "--import", diff --git a/tests/unit/batches-delete-completed-ownership-wvxc.test.ts b/tests/unit/batches-delete-completed-ownership-wvxc.test.ts index 9c12fd59eb..c3817a8da9 100644 --- a/tests/unit/batches-delete-completed-ownership-wvxc.test.ts +++ b/tests/unit/batches-delete-completed-ownership-wvxc.test.ts @@ -15,8 +15,14 @@ * empty/missing apiKeyId throws instead of silently widening the sweep; * - batches with `api_key_id IS NULL` are deliberately OUT of a key-scoped * sweep (strict — diverges from the single-batch `scopeCheck` on purpose); - * - the sweep is atomic: a failure after the file soft-deletes rolls the file - * content back, so no batch row is left pointing at a nulled file; + * - each chunk is atomic: a failure after the file soft-deletes rolls the file + * content of that chunk back, so no batch row is left pointing at a nulled + * file; BOTH modes commit per chunk of INSTANCE_SWEEP_CHUNK ids, so the + * write lock is never held across chunks (omni-code-sec LEDGER-4/20/21); + * - the chunk loop must make progress: a DELETE that silently removes nothing + * throws instead of re-selecting the same chunk forever (LEDGER-22); + * - a scope naming BOTH `apiKeyId` and `allTenants` is rejected by the + * presence of the field, not its truthiness (LEDGER-18); * - a file soft-delete failure is logged, not swallowed, and the batch rows * are still swept. * @@ -166,6 +172,25 @@ describe("deleteCompletedBatches — ownership boundary (GHSA-wvxc-jp3v-5mg5)", deleteCompletedBatches({ apiKeyId: "key_mixed_wvxc" }); }); + it("rejects allTenants combined with an EMPTY or NULL apiKeyId too — the field's presence decides, not its truthiness (LEDGER-18)", () => { + const survivor = seedCompletedBatch("key_presence_wvxc", "wvxc-presence"); + assert.throws( + () => deleteCompletedBatches({ allTenants: true, apiKeyId: "" } as never), + /mutually exclusive/ + ); + assert.throws( + () => deleteCompletedBatches({ allTenants: true, apiKeyId: null } as never), + /mutually exclusive/ + ); + assert.ok(getBatch(survivor.batch.id), "a rejected mixed scope must not sweep the instance"); + assert.strictEqual( + getFileContent(survivor.file.id)?.toString(), + "wvxc-presence", + "a rejected mixed scope must not null file content" + ); + deleteCompletedBatches({ apiKeyId: "key_presence_wvxc" }); + }); + it("STRICT: a batch with api_key_id NULL stays out of a key-scoped sweep (diverges from scopeCheck on purpose)", () => { const unowned = seedCompletedBatch(null, "wvxc-unowned"); const own = seedCompletedBatch("key_strict_wvxc", "wvxc-strict-own"); @@ -354,7 +379,7 @@ describe("deleteCompletedBatches — ownership boundary (GHSA-wvxc-jp3v-5mg5)", } }); - it("SEC-D: a key with more than INSTANCE_SWEEP_CHUNK completed batches is swept in one call, atomically", () => { + it("SEC-D: a key with more than INSTANCE_SWEEP_CHUNK completed batches is swept in one call, one commit per chunk", () => { const total = INSTANCE_SWEEP_CHUNK + 1; const own = Array.from({ length: total }, (_, i) => seedCompletedBatch("key-big", `big-${i}`)); const other = seedCompletedBatch("key-small", "big-other"); @@ -376,32 +401,63 @@ describe("deleteCompletedBatches — ownership boundary (GHSA-wvxc-jp3v-5mg5)", } assert.strictEqual(result.deletedBatches, total); assert.strictEqual(result.deletedFiles, total); - assert.ok(runs >= 3, `outer transaction + 2 chunk units expected, got ${runs}`); + assert.strictEqual(runs, 2, "two chunk units (200 + 1) and NO outer transaction"); for (const s of own) assert.strictEqual(getBatch(s.batch.id), null); assert.ok(getBatch(other.batch.id), "another key's batch survives"); }); - it("SEC-D: a failure in the key sweep's second chunk rolls the WHOLE key sweep back (single atomic transaction)", () => { + it("a failure in the key sweep's second chunk keeps chunk 1 committed and rolls chunk 2 back (per-chunk atomicity, same as instance mode)", () => { const own = Array.from({ length: INSTANCE_SWEEP_CHUNK + 5 }, (_, i) => - seedCompletedBatch("key-atomic", `atomic-${i}`) + seedCompletedBatch("key-perchunk", `perchunk-${i}`) ); - const poison = own[INSTANCE_SWEEP_CHUNK + 2].batch.id; + const first = own.slice(0, INSTANCE_SWEEP_CHUNK); + const second = own.slice(INSTANCE_SWEEP_CHUNK); + const poison = second[2].batch.id; const db = getDbInstance(); // DDL cannot take bound parameters in SQLite; the value is createBatch's generated id. db.exec( `CREATE TRIGGER wvxc_key_poison BEFORE DELETE ON batches WHEN OLD.id = '${poison}' BEGIN SELECT RAISE(ABORT, 'key poison'); END` ); try { - assert.throws(() => deleteCompletedBatches({ apiKeyId: "key-atomic" }), /key poison/); + assert.throws(() => deleteCompletedBatches({ apiKeyId: "key-perchunk" }), /key poison/); } finally { db.exec("DROP TRIGGER IF EXISTS wvxc_key_poison"); } - for (const s of own) { - assert.ok(getBatch(s.batch.id), "key sweep is all-or-nothing: chunk 1 rolled back too"); + for (const s of first) { + assert.strictEqual(getBatch(s.batch.id), null, "chunk 1 committed before chunk 2 failed"); + assert.strictEqual(getFile(s.file.id), null, "chunk 1's file soft-deleted with it"); + } + for (const s of second) { + assert.ok(getBatch(s.batch.id), "chunk 2 rolled back as a unit"); assert.strictEqual( getFileContent(s.file.id)?.toString(), - s.file.filename.replace(".jsonl", "") + s.file.filename.replace(".jsonl", ""), + "chunk 2 file content restored" ); } + // The survivors are swept normally once the poison is gone. + const rest = deleteCompletedBatches({ apiKeyId: "key-perchunk" }); + assert.strictEqual(rest.deletedBatches, second.length); + }); + + // Kept LAST on purpose: without the guard this call never returns (a + // synchronous loop that node:test's timeout cannot interrupt), so every + // earlier result still prints before a hung run is killed. + it("FORWARD PROGRESS: a chunk whose DELETE silently removes nothing throws instead of looping forever (LEDGER-22)", () => { + const db = getDbInstance(); + const own = seedCompletedBatch("key-noprog", "noprog-0"); + // RAISE(IGNORE) turns the DELETE into a silent no-op: the row survives, the + // next chunk selects the same id again, and the loop would never end. + db.exec( + "CREATE TRIGGER wvxc_noprog BEFORE DELETE ON batches WHEN OLD.api_key_id = 'key-noprog' BEGIN SELECT RAISE(IGNORE); END" + ); + try { + assert.throws(() => deleteCompletedBatches({ apiKeyId: "key-noprog" }), /no progress/); + } finally { + db.exec("DROP TRIGGER IF EXISTS wvxc_noprog"); + } + assert.ok(getBatch(own.batch.id), "the row the DELETE ignored is still there"); + const result = deleteCompletedBatches({ apiKeyId: "key-noprog" }); + assert.strictEqual(result.deletedBatches, 1, "sweeps normally once the DELETE works again"); }); }); diff --git a/tests/unit/batches-delete-completed-route-scope.test.ts b/tests/unit/batches-delete-completed-route-scope.test.ts index 421db2064d..6e36914e94 100644 --- a/tests/unit/batches-delete-completed-route-scope.test.ts +++ b/tests/unit/batches-delete-completed-route-scope.test.ts @@ -19,6 +19,12 @@ * - a presented key that resolves but is no longer VALID (revoked, deactivated, * banned or expired) is rejected with 401 too — existence of the row is not * authorization (CWE-613); the 401 body is the `buildErrorBody()` shape; + * - a VALID key still goes through the per-key operator policy + * (`enforceApiKeyPolicy`: endpoint allowlist, schedule, usage cap, rate + * limit) like every other `/v1` route — a key whose `allowedEndpoints` + * excludes `batches` is rejected by the enforcer and sweeps nothing, for + * BOTH path shapes the handler can see (`/v1/…` via the rewrite and the + * App Router's own `/api/v1/…`) (omni-code-sec LEDGER-9/13/16); * - no credentials at all → 401; * - a sweep that throws → sanitized 500 (no stack trace, no raw SQLite message) * and nothing deleted (the sweep is atomic). @@ -74,8 +80,8 @@ function seedCompletedBatch(apiKeyId: string | null, label: string) { return { file, batch }; } -async function callDelete(headers: Record) { - const res = await DELETE(new Request(ROUTE_URL, { method: "DELETE", headers })); +async function callDelete(headers: Record, url: string = ROUTE_URL) { + const res = await DELETE(new Request(url, { method: "DELETE", headers })); const body = (await res.json()) as { deleted?: boolean; deletedBatches?: number; @@ -250,6 +256,25 @@ describe("DELETE /api/v1/batches/delete-completed — caller scope (GHSA-wvxc-jp assert.ok(getBatch(unowned.batch.id), "the session branch is never reached"); }); + it("applies the caller's API-key policy: a key whose allowedEndpoints excludes 'batches' is rejected by the enforcer and sweeps nothing (both /api/v1 and /v1 path shapes)", async () => { + const keyA = await createApiKey("wvxc-route-policy-a", "machine-wvxc-pa", []); + await updateApiKeyPermissions(keyA.id, { allowedEndpoints: ["chat"] }); + const own = seedCompletedBatch(keyA.id, "wvxc-route-policy-own"); + + for (const url of [ROUTE_URL, "http://localhost/v1/batches/delete-completed"]) { + const { res, body } = await callDelete({ Authorization: `Bearer ${keyA.key}` }, url); + + assert.strictEqual(res.status, 403, `${url}: the enforcer's endpoint-allowlist rejection`); + assert.match(body.error?.message ?? "", /batches/, `${url}: names the blocked category`); + assert.ok(getBatch(own.batch.id), `${url}: nothing is swept when the policy rejects`); + assert.strictEqual( + getFileContent(own.file.id)?.toString(), + "wvxc-route-policy-own", + `${url}: file content is intact when the policy rejects` + ); + } + }); + it("rejects an unauthenticated request with 401 and deletes nothing", async () => { const keyB = await createApiKey("wvxc-route-401-b", "machine-wvxc-401", []); const seeded = seedCompletedBatch(keyB.id, "wvxc-route-401"); diff --git a/tests/unit/endpoint-categories.test.ts b/tests/unit/endpoint-categories.test.ts index fd973573d5..cb8179ab19 100644 --- a/tests/unit/endpoint-categories.test.ts +++ b/tests/unit/endpoint-categories.test.ts @@ -12,9 +12,8 @@ import assert from "node:assert/strict"; // ─── resolveEndpointCategory: pure function tests ───────────────────────── // Import the pure resolver without DB dependencies -const { resolveEndpointCategory } = await import( - "../../src/shared/constants/endpointCategories.ts" -); +const { resolveEndpointCategory } = + await import("../../src/shared/constants/endpointCategories.ts"); test("resolveEndpointCategory: maps /v1/chat/completions to 'chat'", () => { assert.equal(resolveEndpointCategory("/v1/chat/completions"), "chat"); @@ -133,3 +132,7 @@ test("resolveEndpointCategory: handles sub-paths under category", () => { assert.equal(resolveEndpointCategory("/v1/batches/batch-123"), "batches"); assert.equal(resolveEndpointCategory("/v1/responses/some/path"), "chat"); }); + +test("resolveEndpointCategory: maps /v1/batches/delete-completed to 'batches' (bulk sweep is policy-gated)", () => { + assert.equal(resolveEndpointCategory("/v1/batches/delete-completed"), "batches"); +}); diff --git a/tests/unit/endpoint-restrictions-policy.test.ts b/tests/unit/endpoint-restrictions-policy.test.ts index 24ac4812e4..dea603c947 100644 --- a/tests/unit/endpoint-restrictions-policy.test.ts +++ b/tests/unit/endpoint-restrictions-policy.test.ts @@ -117,6 +117,23 @@ test("search-only key blocks /v1/chat/completions", async () => { assert.ok(msg.includes("chat"), `Error message should mention 'chat', got: ${msg}`); }); +test("search-only key blocks /api/v1/chat/completions too — the App Router path shape must not fail open", async () => { + // A client may call the App Router path directly (no `/v1/:path*` rewrite + // fires), and the handler then sees `/api/v1/…` in `request.url`. The + // category prefixes are `/v1/…`, so without normalization the allowlist + // silently failed open for that shape (omni-code-sec LEDGER-9/16). + const policy = await loadPolicy("search-blocks-api-chat"); + const key = await createKeyWithEndpoints(["search"]); + + const request = makeRequest("http://localhost/api/v1/chat/completions", key.key); + const result = await policy.enforceApiKeyPolicy(request, "gpt-4"); + + assert.ok(result.rejection, "Should reject the /api/v1 shape as well"); + assert.equal(result.rejection.status, 403); + const msg = await readErrorMessage(result.rejection); + assert.ok(msg.includes("chat"), `Error message should mention 'chat', got: ${msg}`); +}); + test("chat+embeddings key allows /v1/embeddings", async () => { const policy = await loadPolicy("chat-emb-allowed"); const key = await createKeyWithEndpoints(["chat", "embeddings"]);