diff --git a/src/lib/db/batches.ts b/src/lib/db/batches.ts index 4c24e408e0..bb70071426 100644 --- a/src/lib/db/batches.ts +++ b/src/lib/db/batches.ts @@ -1,5 +1,5 @@ import { getDbInstance, rowToCamel, objToSnake } from "./core"; -import { deleteFile } from "./files"; +import { deleteFile, deleteFileOwnedBy } from "./files"; import { v4 as uuidv4 } from "uuid"; import { logger } from "../../../open-sse/utils/logger.ts"; @@ -439,6 +439,10 @@ export type DeleteCompletedBatchesScope = { apiKeyId: string } | { allTenants: t * sweep must never reach records the key does not own, so unowned batches are * only swept by `{ allTenants: true }`. * + * In key mode the file half is owner-scoped too: only files whose api_key_id is + * the caller's are soft-deleted; a referenced file another tenant owns (or an + * unowned one) is left intact and is not counted in deletedFiles. + * * 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. @@ -484,7 +488,11 @@ export function deleteCompletedBatches(scope: DeleteCompletedBatchesScope): { let deletedFiles = 0; for (const fid of fileIds) { try { - if (deleteFile(fid)) deletedFiles++; + // Key mode: only the key's OWN files. A batch may reference a file + // another tenant (or nobody) owns; a bulk destructive sweep must not + // reach it (SEC-C). Instance mode keeps the unconditional soft delete. + const removed = allTenants ? deleteFile(fid) : deleteFileOwnedBy(fid, apiKeyId as string); + if (removed) deletedFiles++; } catch (err) { log.warn("deleteCompletedBatches: file soft-delete failed", { fid, diff --git a/tests/unit/batches-delete-completed-ownership-wvxc.test.ts b/tests/unit/batches-delete-completed-ownership-wvxc.test.ts index cc562e5f19..8701342db1 100644 --- a/tests/unit/batches-delete-completed-ownership-wvxc.test.ts +++ b/tests/unit/batches-delete-completed-ownership-wvxc.test.ts @@ -189,6 +189,50 @@ describe("deleteCompletedBatches — ownership boundary (GHSA-wvxc-jp3v-5mg5)", assert.strictEqual(getFile(unowned.file.id), null, "allTenants soft-deletes its file too"); }); + it("SEC-C: a key-scoped sweep never nulls a file another key owns, even when its own batch references it", () => { + const foreignFile = createFile({ + bytes: 7, + filename: "foreign.jsonl", + purpose: "batch", + content: Buffer.from("foreign"), + apiKeyId: "key-other", + }); + const unownedFile = createFile({ + bytes: 7, + filename: "unowned.jsonl", + purpose: "batch", + content: Buffer.from("unowned"), + apiKeyId: null, + }); + const own = seedCompletedBatch("key-secc", "secc-own"); + const cross = createBatch({ + endpoint: "/v1/chat/completions", + completionWindow: "24h", + inputFileId: foreignFile.id, + outputFileId: unownedFile.id, + status: "completed", + apiKeyId: "key-secc", + }); + + const result = deleteCompletedBatches({ apiKeyId: "key-secc" }); + + assert.strictEqual(result.deletedBatches, 2, "both of the key's completed batches are swept"); + assert.strictEqual(result.deletedFiles, 1, "only the key's OWN file is soft-deleted"); + assert.strictEqual(getBatch(own.batch.id), null); + assert.strictEqual(getBatch(cross.id), null); + assert.strictEqual(getFileContent(own.file.id), null, "own file content nulled"); + assert.strictEqual( + getFileContent(foreignFile.id)?.toString(), + "foreign", + "another key's file intact" + ); + assert.strictEqual( + getFileContent(unownedFile.id)?.toString(), + "unowned", + "unowned file intact" + ); + }); + 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");