From 43aba7dd7e2f4a25eb46907b282f2fd055fd4506 Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:39:36 -0300 Subject: [PATCH] fix(api): explicit sweep scope, audit log and atomic delete for completed batches Follow-up to #12969 (GHSA-wvxc-jp3v-5mg5) from the omni-code-review battery (LEDGER-1/2/3/4/7/8/9/10): - deleteCompletedBatches takes an explicit scope `{ apiKeyId } | { allTenants: true }`; a missing/empty id throws instead of silently sweeping the whole instance - route branches on the dashboard session explicitly; keys only sweep their own batches; batches with no owner stay out of a key-scoped sweep on purpose (JSDoc) - every sweep is logged (warn for instance-wide, info for key-scoped); a failing sweep returns a sanitized 500 via buildErrorBody - the file soft-deletes, checkpoint delete and batch delete run in one transaction; the empty catch around deleteFile now logs the failure - route-level regression test through the real handler; test files self-isolate their DATA_DIR so the single-file command never touches ~/.omniroute - changelog.d fragment --- ...2969-batches-delete-completed-ownership.md | 1 + .../api/v1/batches/delete-completed/route.ts | 54 +++++- src/lib/db/batches.ts | 114 ++++++++---- tests/unit/batch-deletion.test.ts | 6 +- ...es-delete-completed-ownership-wvxc.test.ts | 168 +++++++++++++++-- ...tches-delete-completed-route-scope.test.ts | 175 ++++++++++++++++++ 6 files changed, 454 insertions(+), 64 deletions(-) create mode 100644 changelog.d/fixes/12969-batches-delete-completed-ownership.md create mode 100644 tests/unit/batches-delete-completed-route-scope.test.ts diff --git a/changelog.d/fixes/12969-batches-delete-completed-ownership.md b/changelog.d/fixes/12969-batches-delete-completed-ownership.md new file mode 100644 index 0000000000..77f6015eba --- /dev/null +++ b/changelog.d/fixes/12969-batches-delete-completed-ownership.md @@ -0,0 +1 @@ +- **fix(api):** `DELETE /v1/batches/delete-completed` now sweeps only the calling API key's own completed batches (batches with no owner stay out of a key-scoped sweep on purpose), with an explicit instance-wide mode reserved for authenticated dashboard sessions, audit logging of every sweep, a sanitized 500 on failure and an atomic sweep so a mid-way error never leaves a batch pointing at a nulled file (GHSA-wvxc-jp3v-5mg5) diff --git a/src/app/api/v1/batches/delete-completed/route.ts b/src/app/api/v1/batches/delete-completed/route.ts index af0c53b79c..fc9fac6de0 100644 --- a/src/app/api/v1/batches/delete-completed/route.ts +++ b/src/app/api/v1/batches/delete-completed/route.ts @@ -1,7 +1,11 @@ import { CORS_HEADERS, handleCorsOptions } from "@/shared/utils/cors"; -import { deleteCompletedBatches } from "@/lib/db/batches"; +import { deleteCompletedBatches, type DeleteCompletedBatchesScope } from "@/lib/db/batches"; import { NextResponse } from "next/server"; import { getApiKeyRequestScope } from "@/app/api/v1/_helpers/apiKeyScope"; +import { buildErrorBody } from "@omniroute/open-sse/utils/error"; +import * as log from "@/sse/utils/logger"; + +const LOG_ROUTE = "batches/delete-completed"; export async function OPTIONS() { return handleCorsOptions(); @@ -11,19 +15,53 @@ export async function DELETE(request: Request) { const scope = await getApiKeyRequestScope(request); if (scope.rejection) return scope.rejection; - // Allow session-authenticated (dashboard) requests; for API-key requests, require a key - if (!scope.isSessionAuth && !scope.apiKeyId) { + // Only an authenticated dashboard session sweeps the whole instance. Every + // other caller is an inference key and only sweeps its own completed batches, + // like the list/count siblings do — otherwise an ordinary key would delete + // every tenant's completed batches and null out their file contents + // (GHSA-wvxc-jp3v-5mg5). A caller that is neither gets 401; there is no + // fallback that silently widens the sweep. + let sweepScope: DeleteCompletedBatchesScope; + if (scope.isSessionAuth) { + sweepScope = { allTenants: true }; + } else if (scope.apiKeyId) { + sweepScope = { apiKeyId: scope.apiKeyId }; + } else { return NextResponse.json( { error: { message: "Authentication required", type: "invalid_request_error" } }, { status: 401, headers: CORS_HEADERS } ); } + const mode: "instance" | "api_key" = scope.isSessionAuth ? "instance" : "api_key"; - // Scope the sweep to the caller's own batches, like the list/count siblings do. - // Only a dashboard session (apiKeyId === null) sweeps the whole instance — - // otherwise an ordinary inference key would delete every tenant's completed - // batches and null out their file contents (GHSA-wvxc-jp3v-5mg5). - const result = deleteCompletedBatches(scope.apiKeyId || undefined); + let result: ReturnType; + try { + result = deleteCompletedBatches(sweepScope); + } catch (err) { + log.error("BATCHES", "delete-completed sweep failed", { + route: LOG_ROUTE, + mode, + apiKeyId: scope.apiKeyId, + error: err instanceof Error ? err.message : String(err), + }); + return NextResponse.json(buildErrorBody(500, "Failed to delete completed batches"), { + status: 500, + headers: CORS_HEADERS, + }); + } + + const audit = { + route: LOG_ROUTE, + mode, + apiKeyId: scope.apiKeyId, + deletedBatches: result.deletedBatches, + deletedFiles: result.deletedFiles, + }; + if (mode === "instance") { + log.warn("BATCHES", "instance-wide completed-batch sweep", audit); + } else { + log.info("BATCHES", "completed-batch sweep", audit); + } return NextResponse.json( { deleted: true, deletedBatches: result.deletedBatches, deletedFiles: result.deletedFiles }, diff --git a/src/lib/db/batches.ts b/src/lib/db/batches.ts index 12d95e0251..3f0aedbe64 100644 --- a/src/lib/db/batches.ts +++ b/src/lib/db/batches.ts @@ -1,6 +1,9 @@ import { getDbInstance, rowToCamel, objToSnake } from "./core"; import { deleteFile } from "./files"; import { v4 as uuidv4 } from "uuid"; +import { logger } from "../../../open-sse/utils/logger.ts"; + +const log = logger("DB_BATCHES"); function parseBatchRow(row: any): BatchRecord { const camel = rowToCamel(row) as any; @@ -411,57 +414,90 @@ export function deleteBatch(id: string): boolean { return result.changes > 0; } +/** + * Scope of a `deleteCompletedBatches` sweep. The intent is explicit on purpose: + * a caller either names the API key whose batches it may sweep, or states + * `allTenants: true` — there is no default that widens to the whole instance. + */ +export type DeleteCompletedBatchesScope = { apiKeyId: string } | { allTenants: true }; + /** * Delete completed batches and the files they reference. * - * `apiKeyId` scopes the sweep to that key's own batches, exactly like - * `listBatches`/`countBatches`. Omitting it sweeps the whole instance and is - * reserved for an authenticated dashboard session — an ordinary inference key - * that reached this without its own id would otherwise delete every tenant's - * completed batches and null out their file contents (GHSA-wvxc-jp3v-5mg5). + * `{ apiKeyId }` scopes the sweep to that key's own batches, exactly like + * `listBatches`/`countBatches`. `{ allTenants: true }` sweeps the whole instance + * and is reserved for an authenticated dashboard session — an ordinary inference + * key that reached this without its own id would otherwise delete every tenant's + * completed batches and null out their file contents (GHSA-wvxc-jp3v-5mg5). A + * missing/empty `apiKeyId` without `allTenants` throws instead of silently + * widening the sweep. + * + * Batches whose `api_key_id` IS NULL are intentionally OUT of a key-scoped sweep. + * This diverges from `scopeCheck` in `src/app/api/v1/batches/[id]/route.ts`, + * which lets any key read/delete a single unowned batch by id: a bulk destructive + * sweep must never reach records the key does not own, so unowned batches are + * only swept by `{ allTenants: true }`. + * + * The file soft-deletes, the checkpoint DELETE and the batches DELETE run in one + * transaction, so a mid-sweep failure rolls everything back — no batch row is + * left pointing at a file whose content was already nulled. */ -export function deleteCompletedBatches(apiKeyId?: string): { +export function deleteCompletedBatches(scope: DeleteCompletedBatchesScope): { deletedBatches: number; deletedFiles: number; } { + const scopeObj = scope && typeof scope === "object" ? scope : {}; + const allTenants = "allTenants" in scopeObj && scopeObj.allTenants === true; + const apiKeyId = "apiKeyId" in scopeObj ? scopeObj.apiKeyId : undefined; + if (!allTenants && !apiKeyId) { + throw new Error("deleteCompletedBatches: apiKeyId required unless allTenants"); + } + const db = getDbInstance(); - const ownershipClause = apiKeyId ? " AND api_key_id = ?" : ""; - const ownershipArgs = apiKeyId ? [apiKeyId] : []; + const ownershipClause = allTenants ? "" : " AND api_key_id = ?"; + const ownershipArgs = allTenants ? [] : [apiKeyId]; - // Collect unique file IDs from the completed batches in scope - const rows = db - .prepare( - `SELECT input_file_id, output_file_id, error_file_id FROM batches WHERE status = 'completed'${ownershipClause}` - ) - .all(...ownershipArgs) as Array<{ - input_file_id: string | null; - output_file_id: string | null; - error_file_id: string | null; - }>; + const sweep = db.transaction(() => { + // Collect unique file IDs from the completed batches in scope + const rows = db + .prepare( + `SELECT input_file_id, output_file_id, error_file_id FROM batches WHERE status = 'completed'${ownershipClause}` + ) + .all(...ownershipArgs) as Array<{ + input_file_id: string | null; + output_file_id: string | null; + error_file_id: string | null; + }>; - const fileIds = new Set(); - for (const row of rows) { - if (row.input_file_id) fileIds.add(row.input_file_id); - if (row.output_file_id) fileIds.add(row.output_file_id); - if (row.error_file_id) fileIds.add(row.error_file_id); - } - - let deletedFiles = 0; - for (const fid of fileIds) { - try { - if (deleteFile(fid)) deletedFiles++; - } catch { - /* ignore */ + const fileIds = new Set(); + for (const row of rows) { + if (row.input_file_id) fileIds.add(row.input_file_id); + if (row.output_file_id) fileIds.add(row.output_file_id); + if (row.error_file_id) fileIds.add(row.error_file_id); } - } - db.prepare( - `DELETE FROM batch_item_checkpoints WHERE batch_id IN (SELECT id FROM batches WHERE status = 'completed'${ownershipClause})` - ).run(...ownershipArgs); + let deletedFiles = 0; + for (const fid of fileIds) { + try { + if (deleteFile(fid)) deletedFiles++; + } catch (err) { + log.warn("deleteCompletedBatches: file soft-delete failed", { + fid, + err: err instanceof Error ? err.message : String(err), + }); + } + } - const result = db - .prepare(`DELETE FROM batches WHERE status = 'completed'${ownershipClause}`) - .run(...ownershipArgs); - return { deletedBatches: result.changes, deletedFiles }; + db.prepare( + `DELETE FROM batch_item_checkpoints WHERE batch_id IN (SELECT id FROM batches WHERE status = 'completed'${ownershipClause})` + ).run(...ownershipArgs); + + const result = db + .prepare(`DELETE FROM batches WHERE status = 'completed'${ownershipClause}`) + .run(...ownershipArgs); + return { deletedBatches: result.changes, deletedFiles }; + }); + + return sweep(); } diff --git a/tests/unit/batch-deletion.test.ts b/tests/unit/batch-deletion.test.ts index 1149c07730..3998ccb552 100644 --- a/tests/unit/batch-deletion.test.ts +++ b/tests/unit/batch-deletion.test.ts @@ -180,7 +180,7 @@ describe("deleteCompletedBatches", () => { assert.ok(getFile(liveInput.id)); // Delete all completed (may include pre-existing ones from other tests) - const result = deleteCompletedBatches(); + const result = deleteCompletedBatches({ allTenants: true }); assert.ok(result.deletedBatches >= 3, `expected >=3, got ${result.deletedBatches}`); assert.ok(result.deletedFiles >= 3, `expected >=3, got ${result.deletedFiles}`); @@ -194,7 +194,7 @@ describe("deleteCompletedBatches", () => { }); it("should return zero counts when no completed batches exist", () => { - const result = deleteCompletedBatches(); + const result = deleteCompletedBatches({ allTenants: true }); assert.strictEqual(result.deletedBatches, 0); assert.strictEqual(result.deletedFiles, 0); }); @@ -224,7 +224,7 @@ describe("deleteCompletedBatches", () => { assert.ok(getBatch(batchB.id)); assert.ok(getFile(sharedFile.id)); - const result = deleteCompletedBatches(); + const result = deleteCompletedBatches({ allTenants: true }); assert.ok(result.deletedBatches >= 2); assert.ok(result.deletedFiles >= 1, "shared file should be counted once"); diff --git a/tests/unit/batches-delete-completed-ownership-wvxc.test.ts b/tests/unit/batches-delete-completed-ownership-wvxc.test.ts index 79daa2898c..7fdfa7a857 100644 --- a/tests/unit/batches-delete-completed-ownership-wvxc.test.ts +++ b/tests/unit/batches-delete-completed-ownership-wvxc.test.ts @@ -4,19 +4,39 @@ * * `listBatches(apiKeyId?)` / `countBatches(apiKeyId?)` take the caller's key and * scope the SQL to `api_key_id = ?`, falling back to instance-wide only when the - * caller is an authenticated dashboard session (which passes `undefined`). - * `deleteCompletedBatches()` took no such argument, so any valid inference key — - * including one with `scopes: []` — deleted every completed batch on the instance - * and nulled the content of the files those batches referenced. + * caller is an authenticated dashboard session. `deleteCompletedBatches()` took + * no such argument, so any valid inference key — including one with + * `scopes: []` — deleted every completed batch on the instance and nulled the + * content of the files those batches referenced. * - * These tests pin both halves of the contract: scoped deletion for a key, and the - * unchanged instance-wide sweep for the dashboard session. + * These tests pin the DB-level contract: + * - `{ apiKeyId }` sweeps only that key's completed batches; + * - `{ allTenants: true }` is the ONLY way to sweep the whole instance — an + * 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; + * - a file soft-delete failure is logged, not swallowed, and the batch rows + * are still swept. + * + * Self-isolating: DATA_DIR is pointed at a fresh temp dir BEFORE any `@/lib/db/*` + * module loads (dynamic imports below), so this file never touches ~/.omniroute + * even when run without tests/_setup/isolateDataDir.ts. */ -import { describe, it, after } from "node:test"; +import { describe, it, after, mock } from "node:test"; import assert from "node:assert"; -import { createFile, getFile } from "@/lib/db/files"; -import { createBatch, getBatch, deleteCompletedBatches } from "@/lib/db/batches"; -import { resetDbInstance } from "@/lib/db/core"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "wvxc-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const { createFile, getFile, getFileContent } = await import("../../src/lib/db/files.ts"); +const { createBatch, getBatch, deleteCompletedBatches } = + await import("../../src/lib/db/batches.ts"); +const { getDbInstance, resetDbInstance } = await import("../../src/lib/db/core.ts"); /** One completed batch owned by `apiKeyId`, with its input file. */ function seedCompletedBatch(apiKeyId: string | null, label: string) { @@ -40,13 +60,14 @@ function seedCompletedBatch(apiKeyId: string | null, label: string) { describe("deleteCompletedBatches — ownership boundary (GHSA-wvxc-jp3v-5mg5)", () => { after(() => { resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); }); it("deletes only the caller's completed batches, never another key's", () => { const attacker = seedCompletedBatch("key_attacker_wvxc", "wvxc-attacker"); const victim = seedCompletedBatch("key_victim_wvxc", "wvxc-victim"); - const result = deleteCompletedBatches("key_attacker_wvxc"); + const result = deleteCompletedBatches({ apiKeyId: "key_attacker_wvxc" }); assert.strictEqual( getBatch(attacker.batch.id), @@ -81,19 +102,138 @@ describe("deleteCompletedBatches — ownership boundary (GHSA-wvxc-jp3v-5mg5)", apiKeyId: "key_owner_wvxc", }); - deleteCompletedBatches("key_owner_wvxc"); + deleteCompletedBatches({ apiKeyId: "key_owner_wvxc" }); assert.strictEqual(getBatch(own.batch.id), null, "completed batch of the caller goes"); assert.ok(getBatch(inProgress.id), "an in-progress batch is never swept"); }); - it("keeps the instance-wide sweep for a dashboard session (no apiKeyId)", () => { + it("keeps the instance-wide sweep only for the explicit allTenants scope", () => { const a = seedCompletedBatch("key_a_wvxc_global", "wvxc-global-a"); const b = seedCompletedBatch("key_b_wvxc_global", "wvxc-global-b"); - deleteCompletedBatches(); + deleteCompletedBatches({ allTenants: true }); assert.strictEqual(getBatch(a.batch.id), null, "session sweep clears every key"); assert.strictEqual(getBatch(b.batch.id), null, "session sweep clears every key"); }); + + it("throws on a missing/empty apiKeyId instead of silently sweeping the instance", () => { + const survivor = seedCompletedBatch("key_survivor_wvxc", "wvxc-survivor"); + + assert.throws( + () => deleteCompletedBatches({ apiKeyId: "" }), + /apiKeyId required unless allTenants/ + ); + assert.throws( + // A caller that forgot the scope entirely (JS caller / `any` cast) must not + // fall through to an instance-wide sweep either. + () => (deleteCompletedBatches as unknown as (s?: unknown) => unknown)(), + /apiKeyId required unless allTenants/ + ); + assert.throws( + () => + (deleteCompletedBatches as unknown as (s: unknown) => unknown)({ + apiKeyId: null, + allTenants: false, + }), + /apiKeyId required unless allTenants/ + ); + + assert.ok(getBatch(survivor.batch.id), "a rejected call must not delete anything"); + assert.strictEqual( + getFileContent(survivor.file.id)?.toString(), + "wvxc-survivor", + "a rejected call must not null file content" + ); + + deleteCompletedBatches({ apiKeyId: "key_survivor_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"); + + const scoped = deleteCompletedBatches({ apiKeyId: "key_strict_wvxc" }); + + assert.strictEqual(scoped.deletedBatches, 1, "only the key's own batch is counted"); + assert.strictEqual(getBatch(own.batch.id), null, "the key's own batch goes"); + assert.ok(getBatch(unowned.batch.id), "the NULL-owned batch survives a key-scoped sweep"); + assert.ok(getFile(unowned.file.id), "the NULL-owned batch's file survives"); + assert.strictEqual( + getFileContent(unowned.file.id)?.toString(), + "wvxc-unowned", + "the NULL-owned batch's file content is intact" + ); + + const instanceWide = deleteCompletedBatches({ allTenants: true }); + + assert.ok(instanceWide.deletedBatches >= 1, "the allTenants sweep reaches unowned batches"); + assert.strictEqual(getBatch(unowned.batch.id), null, "allTenants removes the NULL-owned batch"); + assert.strictEqual(getFile(unowned.file.id), null, "allTenants soft-deletes its file too"); + }); + + it("ATOMIC: a failure after the file soft-deletes rolls the file content back", () => { + const db = getDbInstance(); + const own = seedCompletedBatch("key_atomic_wvxc", "wvxc-atomic"); + + // Test-side failure injection: make the final `DELETE FROM batches` abort. The + // file soft-deletes and the checkpoint DELETE run before it, so without a + // transaction the batch row would survive pointing at a nulled file. + db.exec( + "CREATE TRIGGER wvxc_abort_batch_delete BEFORE DELETE ON batches BEGIN SELECT RAISE(ABORT, 'wvxc injected failure'); END" + ); + try { + assert.throws( + () => deleteCompletedBatches({ apiKeyId: "key_atomic_wvxc" }), + /wvxc injected failure/ + ); + } finally { + db.exec("DROP TRIGGER IF EXISTS wvxc_abort_batch_delete"); + } + + const batch = getBatch(own.batch.id); + assert.ok(batch, "the batch row is still there after the failed sweep"); + assert.ok( + getFile(own.file.id), + "the referenced file is not soft-deleted after the failed sweep" + ); + assert.strictEqual( + getFileContent(own.file.id)?.toString(), + "wvxc-atomic", + "the file content was rolled back — no batch row points at a nulled file" + ); + + // Sanity: without the injected failure the sweep completes normally. + const result = deleteCompletedBatches({ apiKeyId: "key_atomic_wvxc" }); + assert.strictEqual(result.deletedBatches, 1); + assert.strictEqual(result.deletedFiles, 1); + assert.strictEqual(getFile(own.file.id), null); + }); + + it("logs (does not swallow) a file soft-delete failure and still sweeps the batch rows", () => { + const db = getDbInstance(); + const own = seedCompletedBatch("key_filefail_wvxc", "wvxc-filefail"); + const warn = mock.method(console, "warn", () => {}); + + db.exec( + "CREATE TRIGGER wvxc_abort_file_update BEFORE UPDATE ON files BEGIN SELECT RAISE(ABORT, 'wvxc file failure'); END" + ); + let result: { deletedBatches: number; deletedFiles: number }; + try { + result = deleteCompletedBatches({ apiKeyId: "key_filefail_wvxc" }); + } finally { + db.exec("DROP TRIGGER IF EXISTS wvxc_abort_file_update"); + warn.mock.restore(); + } + + assert.strictEqual(result.deletedBatches, 1, "batch rows are swept even when a file fails"); + assert.strictEqual(result.deletedFiles, 0, "a failed soft-delete is not counted"); + assert.strictEqual(getBatch(own.batch.id), null); + const logged = warn.mock.calls + .map((c) => c.arguments.map((a) => String(a)).join(" ")) + .find((line) => line.includes("deleteCompletedBatches: file soft-delete failed")); + assert.ok(logged, "the failure is logged at warn level"); + assert.ok(logged!.includes(own.file.id), "the log line names the file id"); + }); }); diff --git a/tests/unit/batches-delete-completed-route-scope.test.ts b/tests/unit/batches-delete-completed-route-scope.test.ts new file mode 100644 index 0000000000..6f7d4620c1 --- /dev/null +++ b/tests/unit/batches-delete-completed-route-scope.test.ts @@ -0,0 +1,175 @@ +/** + * GHSA-wvxc-jp3v-5mg5 — route-level regression guard for + * `DELETE /api/v1/batches/delete-completed`. + * + * The DB-level contract lives in batches-delete-completed-ownership-wvxc.test.ts. + * This file drives the REAL route handler with REAL credentials (API keys created + * through `createApiKey`, a dashboard session via a signed `auth_token` cookie) so + * it fails whenever the route stops translating the caller's scope into the + * matching `deleteCompletedBatches` scope: + * + * - an inference key must only sweep its own completed batches and the response + * must report 0 deletions when it owns none — proven against an un-scoped + * `{ allTenants: true }` regression (the "flip" recorded in the PR notes); + * - an authenticated dashboard session sweeps the whole instance, even when the + * request ALSO carries an API key (isSessionAuth wins); + * - 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). + * + * Self-isolating: DATA_DIR points at a fresh temp dir BEFORE any `@/lib/db/*` + * module loads (dynamic imports below), so this file never touches ~/.omniroute. + */ +import { describe, it, after } from "node:test"; +import assert from "node:assert"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { SignJWT } from "jose"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "wvxc-route-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "wvxc-route-api-secret"; +process.env.JWT_SECRET = "wvxc-route-jwt-secret"; + +const { getDbInstance, resetDbInstance } = await import("../../src/lib/db/core.ts"); +const { createApiKey } = await import("../../src/lib/db/apiKeys.ts"); +const { createFile, getFile, getFileContent } = await import("../../src/lib/db/files.ts"); +const { createBatch, getBatch } = await import("../../src/lib/db/batches.ts"); +const { DELETE } = await import("../../src/app/api/v1/batches/delete-completed/route.ts"); + +const ROUTE_URL = "http://localhost/api/v1/batches/delete-completed"; + +async function sessionCookie(): Promise { + const secret = new TextEncoder().encode(process.env.JWT_SECRET); + const jwt = await new SignJWT({ sub: "admin" }) + .setProtectedHeader({ alg: "HS256" }) + .setExpirationTime("1h") + .sign(secret); + return `auth_token=${jwt}`; +} + +function seedCompletedBatch(apiKeyId: string | null, label: string) { + const file = createFile({ + bytes: 8, + filename: `${label}.jsonl`, + purpose: "batch", + content: Buffer.from(label), + apiKeyId, + }); + const batch = createBatch({ + endpoint: "/v1/chat/completions", + completionWindow: "24h", + inputFileId: file.id, + status: "completed", + apiKeyId, + }); + return { file, batch }; +} + +async function callDelete(headers: Record) { + const res = await DELETE(new Request(ROUTE_URL, { method: "DELETE", headers })); + const body = (await res.json()) as { + deleted?: boolean; + deletedBatches?: number; + deletedFiles?: number; + error?: { message: string; type?: string; code?: string }; + }; + return { res, body }; +} + +describe("DELETE /api/v1/batches/delete-completed — caller scope (GHSA-wvxc-jp3v-5mg5)", () => { + after(() => { + resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + }); + + it("an inference key sweeps only its own batches: another key's batch survives and the response reports 0", async () => { + const keyA = await createApiKey("wvxc-route-key-a", "machine-wvxc-a", []); + const keyB = await createApiKey("wvxc-route-key-b", "machine-wvxc-b", []); + const victim = seedCompletedBatch(keyB.id, "wvxc-route-victim"); + + const { res, body } = await callDelete({ Authorization: `Bearer ${keyA.key}` }); + + assert.strictEqual(res.status, 200); + assert.strictEqual(body.deleted, true); + assert.strictEqual(body.deletedBatches, 0, "key A owns no completed batch — nothing to sweep"); + assert.strictEqual(body.deletedFiles, 0); + assert.ok(getBatch(victim.batch.id), "key B's completed batch must survive key A's sweep"); + assert.strictEqual( + getFileContent(victim.file.id)?.toString(), + "wvxc-route-victim", + "key B's file content must not be nulled by key A" + ); + + // And key A's own batch IS swept by key A. + const own = seedCompletedBatch(keyA.id, "wvxc-route-own"); + const second = await callDelete({ Authorization: `Bearer ${keyA.key}` }); + assert.strictEqual(second.res.status, 200); + assert.strictEqual(second.body.deletedBatches, 1); + assert.strictEqual(second.body.deletedFiles, 1); + assert.strictEqual(getBatch(own.batch.id), null, "key A's own completed batch is swept"); + assert.ok(getBatch(victim.batch.id), "key B's batch still survives"); + }); + + it("a dashboard session sweeps the whole instance — even when the request also carries an API key", async () => { + const keyA = await createApiKey("wvxc-route-session-a", "machine-wvxc-sa", []); + const keyB = await createApiKey("wvxc-route-session-b", "machine-wvxc-sb", []); + const other = seedCompletedBatch(keyB.id, "wvxc-route-session-other"); + const unowned = seedCompletedBatch(null, "wvxc-route-session-unowned"); + + const { res, body } = await callDelete({ + Authorization: `Bearer ${keyA.key}`, + cookie: await sessionCookie(), + }); + + assert.strictEqual(res.status, 200); + assert.ok( + body.deletedBatches! >= 2, + `session sweep must reach every tenant, got ${body.deletedBatches}` + ); + assert.strictEqual(getBatch(other.batch.id), null, "session sweep removes another key's batch"); + assert.strictEqual(getBatch(unowned.batch.id), null, "session sweep removes the unowned batch"); + assert.strictEqual(getFile(other.file.id), null, "session sweep soft-deletes the files too"); + }); + + 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"); + + const { res, body } = await callDelete({}); + + assert.strictEqual(res.status, 401); + assert.strictEqual(body.error?.message, "Authentication required"); + assert.ok(getBatch(seeded.batch.id), "nothing is swept without credentials"); + }); + + it("returns a sanitized 500 (no stack trace, no raw SQLite message) when the sweep throws, and deletes nothing", async () => { + const keyA = await createApiKey("wvxc-route-500-a", "machine-wvxc-500", []); + const own = seedCompletedBatch(keyA.id, "wvxc-route-500"); + const db = getDbInstance(); + + db.exec( + "CREATE TRIGGER wvxc_route_abort_batch_delete BEFORE DELETE ON batches BEGIN SELECT RAISE(ABORT, 'wvxc route injected failure at /secret/path.ts:1'); END" + ); + let outcome: Awaited>; + try { + outcome = await callDelete({ Authorization: `Bearer ${keyA.key}` }); + } finally { + db.exec("DROP TRIGGER IF EXISTS wvxc_route_abort_batch_delete"); + } + + assert.strictEqual(outcome.res.status, 500); + const message = outcome.body.error?.message ?? ""; + assert.ok(message.length > 0, "a 500 still carries a human-readable message"); + assert.ok(!message.includes("at /"), `stack trace leaked: ${message}`); + assert.ok(!message.includes("wvxc route injected failure"), `raw DB error leaked: ${message}`); + assert.ok(!message.includes("SQLITE"), `raw SQLite code leaked: ${message}`); + assert.ok(getBatch(own.batch.id), "a failed sweep leaves the batch row in place"); + assert.strictEqual( + getFileContent(own.file.id)?.toString(), + "wvxc-route-500", + "a failed sweep rolls the file content back" + ); + }); +});