fix(db): key-scoped batch sweep soft-deletes only the key's own files (SEC-C)

Refs #12969
This commit is contained in:
diegosouzapw
2026-09-11 17:54:28 -03:00
parent 3bf006da95
commit 46b24d980d
2 changed files with 54 additions and 2 deletions

View File

@@ -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,

View File

@@ -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");