Files
OmniRoute/tests/integration/files-api-limit-validation.test.ts
Diego Rodrigues de Sa e Souza e7f9fec251 fix(api): enforce API-key ownership on files and batches — null-owner records and anonymous listing (#13749)
GHSA-2jm2-mpx8-6523 and GHSA-m3hp-hq9g-fpmv, one root cause.

`getApiKeyRequestScope()` never rejects: with the default REQUIRE_API_KEY=false
the client-api policy admits both a missing and an invalid bearer as anonymous,
and the scope comes back `{ apiKeyId: null, isSessionAuth: false }`. The
`/v1/files` and `/v1/batches` routes then treated "null" as permissive in two
different ways:

- GHSA-m3hp — the list routes coerced `apiKeyId || undefined`, and the DB layer
  reads `undefined` as "no owner filter", so an anonymous or invalid-bearer
  caller got every tenant's file and batch metadata, the same unfiltered view as
  the operator's dashboard.
- GHSA-2jm2 — the single-record checks were `record.apiKeyId !== null && …`, so
  a record with no owner short-circuited to "allowed" for any caller: read,
  download raw content, delete, cancel, or use as a batch input. Null-owner
  records are common — every dashboard-session upload, and every batch output
  file inheriting a session batch's owner, which carries model responses.

`api_key_id` has existed since the table was created (migration 028), so a null
owner is not a legacy row; it is an unattributable write. No doc described it
as shared — API_REFERENCE says files are scoped per key — and batch_api.test.ts
pinned the by-id exposure as expected behaviour.

One rule now, in `_helpers/apiKeyScope.ts`:

- `canAccessOwnedRecord(scope, owner)`: a dashboard session is the instance
  operator and may act on any record; an API key acts on its own records only;
  a null owner is denied to every non-session caller. Applied to files
  GET / DELETE / content, batches GET / DELETE / cancel, and the batch-create
  input-file check.
- `resolveListScope(scope)`: an explicit union for list/count reads — scoped to
  the presented key (a key wins even alongside a session cookie), instance-wide
  only for a session without a key, and 401 otherwise, including for a bearer
  that does not resolve to a key. There is no default that widens a read.

This follows the GHSA-wvxc shape already used by the delete-completed sweep.

Behaviour change: the anonymous upload → batch → download flow no longer works
without an API key, because a null owner cannot be attributed.

Subsumes #13683: it moved `scopeCheck` into the shared helper so a session can
cancel any batch — kept, and its test ported — but it also kept null-owner
records open on the premise they predate ownership tracking, which migration
028 contradicts.

Tests are red-first. batch_api's by-id case is flipped to 404 with a negative
assertion; batch-deletion-route-logic now imports the real helper instead of a
local copy that had silently diverged from production; the two integration
tests present a real key, since their subject is limits and rate logging, not
auth.

Co-authored-by: Markus Hartung <mail@hartmark.se>
2026-09-15 13:25:13 -03:00

109 lines
3.7 KiB
TypeScript

import { describe, it, before } from "node:test";
import assert from "node:assert";
// `GET /v1/files` fails closed for a caller that is neither an API key nor a
// dashboard session (GHSA-m3hp-hq9g-fpmv), so the HTTP cases below present a
// real key: the subject here is limit validation, not auth.
process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "files-limit-validation-secret";
const { createFile, deleteFile } = await import("@/lib/db/files");
const { createApiKey } = await import("@/lib/db/apiKeys");
const { GET, parseFilesListQuery } = await import("@/app/api/v1/files/route");
let authHeaders: Record<string, string> = {};
let apiKeyId = "";
describe("GET /v1/files limit validation", () => {
before(async () => {
const key = await createApiKey("files-limit-validation", "machine-files-limit", []);
apiKeyId = key.id;
authHeaders = { Authorization: `Bearer ${key.key}` };
});
it("defaults to 20 when limit is absent", () => {
const parsed = parseFilesListQuery(new URLSearchParams("order=asc"));
assert.equal(parsed.ok, true);
if (!parsed.ok) return;
assert.equal(parsed.limit, 20);
});
it("parses an explicit positive integer limit", () => {
const parsed = parseFilesListQuery(new URLSearchParams("limit=2&order=asc&purpose=batch"));
assert.equal(parsed.ok, true);
if (!parsed.ok) return;
assert.equal(parsed.limit, 2);
assert.equal(parsed.order, "asc");
assert.equal(parsed.purpose, "batch");
});
it("rejects non-integer, zero, and oversized limits", async () => {
for (const rawLimit of ["abc", "1.5", "-1", "0", "10001"]) {
const parsed = parseFilesListQuery(
new URLSearchParams(`limit=${encodeURIComponent(rawLimit)}`)
);
assert.equal(parsed.ok, false, `limit=${rawLimit} should be rejected`);
if (parsed.ok) continue;
assert.equal(parsed.response.status, 400);
const body = await parsed.response.json();
assert.equal(body.error.type, "invalid_request_error");
}
});
it("returns only the requested number of files over HTTP", async () => {
const created = [
createFile({
bytes: 1,
filename: "test-files-limit-http-a.txt",
purpose: "assistants",
content: Buffer.from("a"),
mimeType: "text/plain",
apiKeyId,
}),
createFile({
bytes: 1,
filename: "test-files-limit-http-b.txt",
purpose: "assistants",
content: Buffer.from("b"),
mimeType: "text/plain",
apiKeyId,
}),
];
try {
const response = await GET(
new Request("http://localhost/v1/files?limit=1&purpose=assistants", {
headers: authHeaders,
})
);
assert.equal(response.status, 200);
const body = await response.json();
assert.equal(body.object, "list");
assert.equal(body.data.length, 1);
assert.equal(body.has_more, true);
} finally {
for (const file of created) deleteFile(file.id);
}
});
it("returns 400 over HTTP for an invalid limit instead of listing files", async () => {
const response = await GET(
new Request("http://localhost/v1/files?limit=-1", { headers: authHeaders })
);
assert.equal(response.status, 400);
const body = await response.json();
assert.equal(body.error.type, "invalid_request_error");
});
it("rejects an anonymous list with 401 before the limit is even looked at (GHSA-m3hp-hq9g-fpmv)", async () => {
const response = await GET(new Request("http://localhost/v1/files?limit=1"));
assert.equal(response.status, 401);
const body = await response.json();
assert.equal(body.error.message, "Authentication required");
assert.equal(body.error.type, "authentication_error");
});
});