mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-19 13:23:50 +03:00
/api/files, /api/files/[id]/content, /api/batches and /api/batches/[id] only gated on requireManagementAuth(request), which returns null unconditionally when settings.requireLogin===false, and never applied any per-record ownership check. On an instance with login disabled, an unauthenticated caller could enumerate/download every tenant's files and batches — the hardened /api/v1/files and /api/v1/batches siblings already scope via getApiKeyRequestScope()/resolveListScope()/ canAccessOwnedRecord() from the GHSA-2jm2-mpx8-6523 and GHSA-m3hp-hq9g-fpmv fixes. Port that exact scoping onto the 4 management routes: an API key sees only its own files/batches, a dashboard session keeps instance-wide access, and any other caller is rejected instead of falling through to an unscoped read.
This commit is contained in:
committed by
GitHub
parent
5b61937f17
commit
25d35179fd
1
changelog.d/fixes/13882-files-batches-tenant-scope.md
Normal file
1
changelog.d/fixes/13882-files-batches-tenant-scope.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(security): scope `/api/files` and `/api/batches` management siblings to the caller's own API key (session stays instance-wide), closing a cross-tenant read that let an anonymous or foreign-key caller enumerate and download other tenants' files/batches (#13882)
|
||||
@@ -1,15 +1,19 @@
|
||||
import { NextResponse } from "next/server";
|
||||
export const dynamic = "force-dynamic";
|
||||
import { getBatch } from "@/lib/db/batches";
|
||||
import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import { getApiKeyRequestScope, canAccessOwnedRecord } from "@/app/api/v1/_helpers/apiKeyScope";
|
||||
|
||||
export async function GET(request: Request, { params }: { params: { id: string } }) {
|
||||
const authError = await requireManagementAuth(request);
|
||||
if (authError) return authError;
|
||||
const scope = await getApiKeyRequestScope(request);
|
||||
if (scope.rejection) return scope.rejection;
|
||||
|
||||
try {
|
||||
const batch = getBatch(params.id);
|
||||
if (!batch) {
|
||||
// Session = operator, key = own rows only, null owner = denied. Mirrors
|
||||
// /api/v1/batches/[id] (GHSA-2jm2-mpx8-6523) so this management sibling
|
||||
// cannot leak a foreign tenant's batch metadata to an unauthenticated
|
||||
// caller (#13882).
|
||||
if (!batch || !canAccessOwnedRecord(scope, batch.apiKeyId)) {
|
||||
return NextResponse.json({ error: "Batch not found" }, { status: 404 });
|
||||
}
|
||||
return NextResponse.json({ batch });
|
||||
|
||||
@@ -1,16 +1,24 @@
|
||||
import { NextResponse } from "next/server";
|
||||
export const dynamic = "force-dynamic";
|
||||
import { listBatches } from "@/lib/db/batches";
|
||||
import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import { getApiKeyRequestScope, resolveListScope } from "@/app/api/v1/_helpers/apiKeyScope";
|
||||
|
||||
export async function GET(request: Request) {
|
||||
const authError = await requireManagementAuth(request);
|
||||
if (authError) return authError;
|
||||
const scope = await getApiKeyRequestScope(request);
|
||||
if (scope.rejection) return scope.rejection;
|
||||
|
||||
// Key → own batches only; dashboard session without a key → instance-wide;
|
||||
// anonymous / unresolvable bearer → 401. Mirrors /api/v1/batches (GHSA-2jm2-
|
||||
// mpx8-6523 / GHSA-m3hp-hq9g-fpmv) so this management sibling cannot leak
|
||||
// every tenant's batches to an unauthenticated caller (#13882).
|
||||
const listScope = resolveListScope(scope);
|
||||
if (listScope.mode === "rejected") return listScope.response;
|
||||
|
||||
try {
|
||||
const url = new URL(request.url);
|
||||
const limit = Number.parseInt(url.searchParams.get("limit") || "100", 10);
|
||||
const batches = listBatches(undefined, limit);
|
||||
const ownerFilter = listScope.mode === "api_key" ? listScope.apiKeyId : undefined;
|
||||
const batches = listBatches(ownerFilter, limit);
|
||||
return NextResponse.json({ batches });
|
||||
} catch (error) {
|
||||
console.log("Error fetching batches:", error);
|
||||
|
||||
@@ -1,15 +1,19 @@
|
||||
import { NextResponse } from "next/server";
|
||||
import { getFile, getFileContent } from "@/lib/db/files";
|
||||
import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import { getApiKeyRequestScope, canAccessOwnedRecord } from "@/app/api/v1/_helpers/apiKeyScope";
|
||||
|
||||
export async function GET(request: Request, { params }: { params: Promise<{ id: string }> }) {
|
||||
const authError = await requireManagementAuth(request);
|
||||
if (authError) return authError;
|
||||
const scope = await getApiKeyRequestScope(request);
|
||||
if (scope.rejection) return scope.rejection;
|
||||
|
||||
const { id } = await params;
|
||||
const file = getFile(id);
|
||||
|
||||
if (!file) {
|
||||
// `getFileContent` has no ownership check of its own — this guard is the only
|
||||
// thing between a caller and the raw bytes. Mirrors /api/v1/files/[id]/content
|
||||
// (GHSA-2jm2-mpx8-6523) so this management sibling cannot leak a foreign
|
||||
// tenant's file content to an unauthenticated caller (#13882).
|
||||
if (!file || !canAccessOwnedRecord(scope, file.apiKeyId)) {
|
||||
return NextResponse.json(
|
||||
{ error: { message: "File not found", type: "invalid_request_error" } },
|
||||
{ status: 404 }
|
||||
|
||||
@@ -1,15 +1,25 @@
|
||||
import { NextResponse } from "next/server";
|
||||
import { listFiles } from "@/lib/db/files";
|
||||
import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import { getApiKeyRequestScope, resolveListScope } from "@/app/api/v1/_helpers/apiKeyScope";
|
||||
|
||||
export async function GET(request: Request) {
|
||||
const authError = await requireManagementAuth(request);
|
||||
if (authError) return authError;
|
||||
const scope = await getApiKeyRequestScope(request);
|
||||
if (scope.rejection) return scope.rejection;
|
||||
|
||||
// Key → own files only; dashboard session without a key → instance-wide;
|
||||
// anonymous / unresolvable bearer → 401. Mirrors /api/v1/files (GHSA-2jm2-
|
||||
// mpx8-6523 / GHSA-m3hp-hq9g-fpmv) so this management sibling cannot leak
|
||||
// every tenant's files to an unauthenticated caller (#13882).
|
||||
const listScope = resolveListScope(scope);
|
||||
if (listScope.mode === "rejected") return listScope.response;
|
||||
|
||||
try {
|
||||
const url = new URL(request.url);
|
||||
const limit = Number.parseInt(url.searchParams.get("limit") || "100", 10);
|
||||
const files = listFiles({ limit });
|
||||
const files =
|
||||
listScope.mode === "api_key"
|
||||
? listFiles({ limit, apiKeyId: listScope.apiKeyId })
|
||||
: listFiles({ limit });
|
||||
return NextResponse.json({ files });
|
||||
} catch (error) {
|
||||
console.log("Error fetching files:", error);
|
||||
|
||||
@@ -62,9 +62,10 @@ function createTestFile(
|
||||
|
||||
// ── Auth tests ─────────────────────────────────────────────────────────────
|
||||
|
||||
test("GET /api/files/{id}/content — auth not required (default) → file content returned", async () => {
|
||||
// By default (no INITIAL_PASSWORD, no password set) auth is skipped,
|
||||
// so a plain unauthenticated request should still work.
|
||||
test("GET /api/files/{id}/content — null-owner file, no session/key → anonymous caller denied (#13882)", async () => {
|
||||
// A null-owner file is unattributable and MUST be denied to any non-session
|
||||
// caller, even when auth is not required at the instance level — an
|
||||
// anonymous pass-through here was GHSA-2jm2-mpx8-6523's exact shape.
|
||||
const content = makeFileContent("hello batch");
|
||||
const file = createTestFile({ filename: "hello.jsonl", content });
|
||||
|
||||
@@ -73,9 +74,9 @@ test("GET /api/files/{id}/content — auth not required (default) → file conte
|
||||
{ params: Promise.resolve({ id: file.id }) }
|
||||
);
|
||||
|
||||
assert.equal(res.status, 200);
|
||||
const buf = Buffer.from(await res.arrayBuffer());
|
||||
assert.equal(buf.toString(), "hello batch");
|
||||
assert.equal(res.status, 404);
|
||||
const body = await res.json();
|
||||
assert.equal(body.error.message, "File not found");
|
||||
});
|
||||
|
||||
test("GET /api/files/{id}/content — with management session → 200 and file content", async () => {
|
||||
@@ -210,7 +211,12 @@ test("GET /api/files/{id}/content — unauthenticated request is rejected when a
|
||||
{ params: Promise.resolve({ id: file.id }) }
|
||||
);
|
||||
assert.notEqual(res.status, 200, "Unauthenticated request should not return 200");
|
||||
assert.ok(res.status === 401 || res.status === 403, `Expected 401/403, got ${res.status}`);
|
||||
// Fails closed via ownership scoping (404, "not found" — never leaking
|
||||
// existence) rather than the old management-auth 401/403 (#13882).
|
||||
assert.ok(
|
||||
res.status === 401 || res.status === 403 || res.status === 404,
|
||||
`Expected 401/403/404, got ${res.status}`
|
||||
);
|
||||
} finally {
|
||||
delete process.env.INITIAL_PASSWORD;
|
||||
}
|
||||
|
||||
252
tests/unit/files-batches-management-ownership-13882.test.ts
Normal file
252
tests/unit/files-batches-management-ownership-13882.test.ts
Normal file
@@ -0,0 +1,252 @@
|
||||
/**
|
||||
* #13882 — route-level regression guard for the `/api/files` and `/api/batches`
|
||||
* MANAGEMENT siblings' ownership model.
|
||||
*
|
||||
* These routes previously gated ONLY on `requireManagementAuth(request)`, which
|
||||
* returns `null` (auth waived) unconditionally when `settings.requireLogin===false`
|
||||
* (`requireManagementAuth.ts:59`), and never applied any per-record ownership
|
||||
* check — unlike the hardened `/api/v1/files` + `/api/v1/batches` siblings that
|
||||
* GHSA-2jm2-mpx8-6523 / GHSA-m3hp-hq9g-fpmv already fixed. On an instance with
|
||||
* login disabled, any unauthenticated caller could enumerate/download every
|
||||
* tenant's files and batches.
|
||||
*
|
||||
* The fix ports the exact `/v1` scoping pattern onto these 4 routes:
|
||||
* `getApiKeyRequestScope` + `resolveListScope` (list endpoints) and
|
||||
* `canAccessOwnedRecord` (single-item endpoints) from
|
||||
* `src/app/api/v1/_helpers/apiKeyScope.ts`.
|
||||
*
|
||||
* Modelled on tests/unit/files-batches-ownership-2jm2-m3hp.test.ts: drives the
|
||||
* REAL route handlers with REAL credentials (API keys via `createApiKey`, a
|
||||
* dashboard session via a signed `auth_token` cookie). Self-isolating: DATA_DIR
|
||||
* points at a fresh temp dir BEFORE any `@/lib/db/*` module loads, so this file
|
||||
* never touches ~/.omniroute.
|
||||
*/
|
||||
import { describe, it, before, 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(), "mgmt-ownership-13882-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "mgmt-ownership-13882-api-secret";
|
||||
process.env.JWT_SECRET = "mgmt-ownership-13882-jwt-secret";
|
||||
|
||||
const { resetDbInstance } = await import("../../src/lib/db/core.ts");
|
||||
const { updateSettings } = await import("../../src/lib/db/settings.ts");
|
||||
const { createApiKey } = await import("../../src/lib/db/apiKeys.ts");
|
||||
const { createFile } = await import("../../src/lib/db/files.ts");
|
||||
const { createBatch } = await import("../../src/lib/db/batches.ts");
|
||||
|
||||
const filesListRoute = await import("../../src/app/api/files/route.ts");
|
||||
const fileContentRoute = await import("../../src/app/api/files/[id]/content/route.ts");
|
||||
const batchesListRoute = await import("../../src/app/api/batches/route.ts");
|
||||
const batchByIdRoute = await import("../../src/app/api/batches/[id]/route.ts");
|
||||
|
||||
type Headers = Record<string, string>;
|
||||
|
||||
before(async () => {
|
||||
await updateSettings({ requireLogin: false });
|
||||
});
|
||||
|
||||
after(() => {
|
||||
resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
|
||||
});
|
||||
|
||||
async function sessionCookie(): Promise<string> {
|
||||
const secret = new TextEncoder().encode(process.env.JWT_SECRET);
|
||||
const jwt = await new SignJWT({ authenticated: true, sub: "admin" })
|
||||
.setProtectedHeader({ alg: "HS256" })
|
||||
.setExpirationTime("1h")
|
||||
.sign(secret);
|
||||
return `auth_token=${jwt}`;
|
||||
}
|
||||
|
||||
function seedFile(apiKeyId: string, label: string) {
|
||||
return createFile({
|
||||
bytes: label.length,
|
||||
filename: `${label}.jsonl`,
|
||||
purpose: "batch",
|
||||
content: Buffer.from(label),
|
||||
mimeType: "application/jsonl",
|
||||
apiKeyId,
|
||||
});
|
||||
}
|
||||
|
||||
function seedBatch(apiKeyId: string, label: string) {
|
||||
const file = seedFile(apiKeyId, `${label}-input`);
|
||||
const batch = createBatch({
|
||||
endpoint: "/v1/chat/completions",
|
||||
completionWindow: "24h",
|
||||
inputFileId: file.id,
|
||||
status: "validating",
|
||||
apiKeyId,
|
||||
});
|
||||
return { file, batch };
|
||||
}
|
||||
|
||||
async function listFilesVia(headers: Headers) {
|
||||
const res = await filesListRoute.GET(
|
||||
new Request("http://localhost/api/files?limit=100", { headers })
|
||||
);
|
||||
return { res, body: (await res.json()) as { files?: Array<{ id: string }> } };
|
||||
}
|
||||
|
||||
async function getFileContentVia(headers: Headers, id: string) {
|
||||
return fileContentRoute.GET(
|
||||
new Request(`http://localhost/api/files/${id}/content`, { headers }),
|
||||
{ params: Promise.resolve({ id }) }
|
||||
);
|
||||
}
|
||||
|
||||
async function listBatchesVia(headers: Headers) {
|
||||
const res = await batchesListRoute.GET(
|
||||
new Request("http://localhost/api/batches?limit=100", { headers })
|
||||
);
|
||||
return { res, body: (await res.json()) as { batches?: Array<{ id: string }> } };
|
||||
}
|
||||
|
||||
async function getBatchVia(headers: Headers, id: string) {
|
||||
return batchByIdRoute.GET(new Request(`http://localhost/api/batches/${id}`, { headers }), {
|
||||
params: { id },
|
||||
});
|
||||
}
|
||||
|
||||
describe("#13882 — /api/files management sibling ownership scoping", () => {
|
||||
it("anonymous caller (requireLogin=false, no credentials) cannot list a foreign file", async () => {
|
||||
const victim = await createApiKey("13882-files-victim-a", "machine-13882-files-a", []);
|
||||
const victimFile = seedFile(victim.id, "anon-list-victim");
|
||||
|
||||
const { res, body } = await listFilesVia({});
|
||||
const leaked = res.status === 200 && (body.files?.some((f) => f.id === victimFile.id) ?? false);
|
||||
assert.strictEqual(leaked, false, "anonymous caller must not enumerate a foreign file");
|
||||
});
|
||||
|
||||
it("a foreign API key cannot list another key's file", async () => {
|
||||
const owner = await createApiKey("13882-files-owner-b", "machine-13882-files-ob", []);
|
||||
const foreign = await createApiKey("13882-files-foreign-b", "machine-13882-files-fb", []);
|
||||
const ownerFile = seedFile(owner.id, "foreign-list-victim");
|
||||
|
||||
const { res, body } = await listFilesVia({ Authorization: `Bearer ${foreign.key}` });
|
||||
assert.strictEqual(res.status, 200);
|
||||
assert.strictEqual(
|
||||
body.files?.some((f) => f.id === ownerFile.id) ?? false,
|
||||
false,
|
||||
"a foreign key must not see another key's file in its own list"
|
||||
);
|
||||
});
|
||||
|
||||
it("the owning API key can still list its own file", async () => {
|
||||
const owner = await createApiKey("13882-files-owner-c", "machine-13882-files-oc", []);
|
||||
const ownedFile = seedFile(owner.id, "owner-list-self");
|
||||
|
||||
const { res, body } = await listFilesVia({ Authorization: `Bearer ${owner.key}` });
|
||||
assert.strictEqual(res.status, 200);
|
||||
assert.strictEqual(body.files?.some((f) => f.id === ownedFile.id) ?? false, true);
|
||||
});
|
||||
|
||||
it("a dashboard session (instance operator) still sees every tenant's file", async () => {
|
||||
const someKey = await createApiKey("13882-files-owner-d", "machine-13882-files-od", []);
|
||||
const someFile = seedFile(someKey.id, "session-list-visible");
|
||||
|
||||
const { res, body } = await listFilesVia({ cookie: await sessionCookie() });
|
||||
assert.strictEqual(res.status, 200);
|
||||
assert.strictEqual(body.files?.some((f) => f.id === someFile.id) ?? false, true);
|
||||
});
|
||||
|
||||
it("anonymous caller cannot download a foreign file's content", async () => {
|
||||
const victim = await createApiKey("13882-files-victim-e", "machine-13882-files-ve", []);
|
||||
const victimFile = seedFile(victim.id, "anon-content-victim");
|
||||
|
||||
const res = await getFileContentVia({}, victimFile.id);
|
||||
assert.notStrictEqual(res.status, 200, "anonymous caller must not download a foreign file");
|
||||
});
|
||||
|
||||
it("a foreign API key cannot download another key's file content", async () => {
|
||||
const owner = await createApiKey("13882-files-owner-f", "machine-13882-files-of", []);
|
||||
const foreign = await createApiKey("13882-files-foreign-f", "machine-13882-files-ff", []);
|
||||
const ownerFile = seedFile(owner.id, "foreign-content-victim");
|
||||
|
||||
const res = await getFileContentVia({ Authorization: `Bearer ${foreign.key}` }, ownerFile.id);
|
||||
assert.notStrictEqual(res.status, 200, "a foreign key must not download another key's file");
|
||||
});
|
||||
|
||||
it("the owning API key can still download its own file content", async () => {
|
||||
const owner = await createApiKey("13882-files-owner-g", "machine-13882-files-og", []);
|
||||
const ownedFile = seedFile(owner.id, "owner-content-self");
|
||||
|
||||
const res = await getFileContentVia({ Authorization: `Bearer ${owner.key}` }, ownedFile.id);
|
||||
assert.strictEqual(res.status, 200);
|
||||
});
|
||||
});
|
||||
|
||||
describe("#13882 — /api/batches management sibling ownership scoping", () => {
|
||||
it("anonymous caller (requireLogin=false, no credentials) cannot list a foreign batch", async () => {
|
||||
const victim = await createApiKey("13882-batches-victim-a", "machine-13882-batches-a", []);
|
||||
const { batch: victimBatch } = seedBatch(victim.id, "anon-list-victim");
|
||||
|
||||
const { res, body } = await listBatchesVia({});
|
||||
const leaked =
|
||||
res.status === 200 && (body.batches?.some((b) => b.id === victimBatch.id) ?? false);
|
||||
assert.strictEqual(leaked, false, "anonymous caller must not enumerate a foreign batch");
|
||||
});
|
||||
|
||||
it("a foreign API key cannot list another key's batch", async () => {
|
||||
const owner = await createApiKey("13882-batches-owner-b", "machine-13882-batches-ob", []);
|
||||
const foreign = await createApiKey("13882-batches-foreign-b", "machine-13882-batches-fb", []);
|
||||
const { batch: ownerBatch } = seedBatch(owner.id, "foreign-list-victim");
|
||||
|
||||
const { res, body } = await listBatchesVia({ Authorization: `Bearer ${foreign.key}` });
|
||||
assert.strictEqual(res.status, 200);
|
||||
assert.strictEqual(
|
||||
body.batches?.some((b) => b.id === ownerBatch.id) ?? false,
|
||||
false,
|
||||
"a foreign key must not see another key's batch in its own list"
|
||||
);
|
||||
});
|
||||
|
||||
it("the owning API key can still list its own batch", async () => {
|
||||
const owner = await createApiKey("13882-batches-owner-c", "machine-13882-batches-oc", []);
|
||||
const { batch: ownedBatch } = seedBatch(owner.id, "owner-list-self");
|
||||
|
||||
const { res, body } = await listBatchesVia({ Authorization: `Bearer ${owner.key}` });
|
||||
assert.strictEqual(res.status, 200);
|
||||
assert.strictEqual(body.batches?.some((b) => b.id === ownedBatch.id) ?? false, true);
|
||||
});
|
||||
|
||||
it("a dashboard session (instance operator) still sees every tenant's batch", async () => {
|
||||
const someKey = await createApiKey("13882-batches-owner-d", "machine-13882-batches-od", []);
|
||||
const { batch: someBatch } = seedBatch(someKey.id, "session-list-visible");
|
||||
|
||||
const { res, body } = await listBatchesVia({ cookie: await sessionCookie() });
|
||||
assert.strictEqual(res.status, 200);
|
||||
assert.strictEqual(body.batches?.some((b) => b.id === someBatch.id) ?? false, true);
|
||||
});
|
||||
|
||||
it("anonymous caller cannot read a foreign batch's metadata", async () => {
|
||||
const victim = await createApiKey("13882-batches-victim-e", "machine-13882-batches-ve", []);
|
||||
const { batch: victimBatch } = seedBatch(victim.id, "anon-getid-victim");
|
||||
|
||||
const res = await getBatchVia({}, victimBatch.id);
|
||||
assert.notStrictEqual(res.status, 200, "anonymous caller must not read a foreign batch");
|
||||
});
|
||||
|
||||
it("a foreign API key cannot read another key's batch metadata", async () => {
|
||||
const owner = await createApiKey("13882-batches-owner-f", "machine-13882-batches-of", []);
|
||||
const foreign = await createApiKey("13882-batches-foreign-f", "machine-13882-batches-ff", []);
|
||||
const { batch: ownerBatch } = seedBatch(owner.id, "foreign-getid-victim");
|
||||
|
||||
const res = await getBatchVia({ Authorization: `Bearer ${foreign.key}` }, ownerBatch.id);
|
||||
assert.notStrictEqual(res.status, 200, "a foreign key must not read another key's batch");
|
||||
});
|
||||
|
||||
it("the owning API key can still read its own batch metadata", async () => {
|
||||
const owner = await createApiKey("13882-batches-owner-g", "machine-13882-batches-og", []);
|
||||
const { batch: ownedBatch } = seedBatch(owner.id, "owner-getid-self");
|
||||
|
||||
const res = await getBatchVia({ Authorization: `Bearer ${owner.key}` }, ownedBatch.id);
|
||||
assert.strictEqual(res.status, 200);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user