Files
OmniRoute/tests/unit/authz/client-api-policy-fallback.test.ts
anhtahaylove fde6241d41 test: close the database before removing temp DATA_DIR (#13290) (#13292)
* test: close the database before removing temp DATA_DIR (#13290)

Tests that set their own DATA_DIR and removed it in test.after() failed on
Windows with EPERM: nothing closed the SQLite connection, so the directory
still had an open handle and the -shm/-wal sidecars kept it locked. maxRetries
could not help because every retry hit the same open handle.

Adds tests/_setup/tempDataDir.ts with cleanupTempDataDir()/createTempDataDir(),
which close the DB singleton (lazily imported, so tests that never touch the
database do not pull in the DB layer) and then remove the directory
best-effort. Applies it to the five suites confirmed failing.

The helper's own test proves the ordering matters: skipping the close makes it
fail with 'cleanup must remove the directory'.

* test: close the database before removing temp DATA_DIR (15 more suites)

Converts the suites that measurably emitted EPERM during a full run to the
shared cleanupTempDataDir helper from #13292.

Measured on the same 15 files:
  base   -> 22 fail, 40 EPERM lines
  branch ->  7 fail, 10 EPERM lines

The 7 remaining failures are pre-existing and unrelated to teardown:
rtk-learn-discover-routes and executor-map-golden already fail on a clean
base (6 and 3 failures respectively).

* test: close the database before removing temp DATA_DIR (final 9 suites)

Completes the #13290 sweep. Two teardown shapes needed the helper:

- after()/t.after() hooks that removed DATA_DIR directly
- beforeEach() hooks that wiped DATA_DIR between tests while the previous
  test's connection was still open. These failed *before* the test body ran,
  so every test in the file reported the same EPERM path.

Three of them already called core.resetDbInstance() right before rmSync and
still leaked, which is the product-side connection leak tracked in #13303.

Measured per file, EPERM lines now 0 across all nine. Remaining failures are
pre-existing on a clean base (firefly 4->1, driverFactory 1, responses-* 1
each) and unrelated to teardown.

* test: add the missing cleanupTempDataDir import to two responses suites

The previous commit swapped rmSync for cleanupTempDataDir in these two files but
did not add the import, so both suites died with
ReferenceError: cleanupTempDataDir is not defined before running any test.

responses-parse-once-4041:            0 pass / 1 fail -> 4 pass / 0 fail
responses-route-early-keepalive-wiring: 0 pass / 1 fail -> 3 pass / 0 fail

Both now report 0 EPERM.

* test: close SQLite handles in three silently-leaking suites

These three suites requested DATA_DIR cleanup but the delete failed on
Windows because a SQLite connection was still open. They pass today, so
the leak is invisible: they carry state between tests and would surface
later as an unrelated-looking assertion, as #13303 already did in the
Firefly suite (a 500 instead of a 401).

agentbridge-mitm-router-key-6403 and agent-bridge-bypass-flow removed
their own temp dir in test.after() without closing the DB first; both now
use the shared cleanupTempDataDir helper, which closes the singleton
before removing the directory.

issue-agent-route-execution is a different case: it has no teardown at
all, so the connection stayed open until process exit and the
isolateDataDir cleanup hook then hit EPERM. It now closes the DB in
test.after().

Verified with a probe on fs.rmSync: all three reported a failed delete
before, and zero across three consecutive runs after, while the same
probe still reports four leaks in the Firefly suite.

* test: remove temp DATA_DIR in five suites that never cleaned up

These five suites create their own mkdtemp DATA_DIR, open the SQLite DB and
never remove the directory, so every run leaves a storage.sqlite behind in the
OS temp dir. Each dir is private to its suite, so this leaked disk space rather
than corrupting results - but the churn is pointless.

Each now closes the DB and removes its directory through the shared
cleanupTempDataDir helper.

Verified with an exit-time probe that lists storage.sqlite* still present in
DATA_DIR: it fired for these suites before the change and is silent after,
with the same test counts (22/14/5/3/3 passing).
2026-09-17 02:31:53 -03:00

268 lines
10 KiB
TypeScript

/**
* Issue #2257 — clientApi policy behavior when an invalid Bearer is sent and
* REQUIRE_API_KEY=false.
*
* The existing `client-api-policy.test.ts` shares a DB-backed setup via
* `resetStorage()` and `apiKeysDb` that has SQLite migration races on this
* branch. This standalone file mocks `validateApiKey` to test the policy's
* fallback branch in isolation — no DB, no migration runner.
*/
import test from "node:test";
import assert from "node:assert/strict";
import Module from "node:module";
import { cleanupTempDataDir } from "../../_setup/tempDataDir.ts";
// ─── Mock validateApiKey via require interception (so the dynamic import in
// the policy module returns our stub instead of hitting the real DB module) ─
type ValidateFn = (key: string) => boolean | Promise<boolean>;
let mockValidateApiKey: ValidateFn = () => false;
const originalResolve = (Module as unknown as { _resolveFilename: typeof Module._resolveFilename })
._resolveFilename;
// Intercept require() / import() resolution for the apiKeys DB module and
// substitute it for our stub. This runs only for the exact path the policy
// imports — production code paths are unaffected.
const POLICY_IMPORT_TARGET = "src/lib/db/apiKeys";
(Module as unknown as { _resolveFilename: typeof Module._resolveFilename })._resolveFilename =
function patched(this: unknown, request: string, ...rest: unknown[]) {
if (request.includes(POLICY_IMPORT_TARGET)) {
// Resolve to a stub file we create below
const stubPath = new URL("./__stub_apiKeys.mjs", import.meta.url).pathname;
// @ts-expect-error - rest spread to original
return originalResolve.call(this, stubPath, ...rest);
}
// @ts-expect-error - rest spread to original
return originalResolve.call(this, request, ...rest);
};
// Write the stub file ad-hoc (Node's loader needs a real file)
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { fileURLToPath } from "node:url";
const ORIGINAL_DATA_DIR = process.env.DATA_DIR;
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omr-clientapi-policy-fallback-"));
process.env.DATA_DIR = TEST_DATA_DIR;
const __dirname = path.dirname(fileURLToPath(import.meta.url));
const STUB_PATH = path.join(__dirname, "__stub_apiKeys.mjs");
fs.writeFileSync(
STUB_PATH,
`export const validateApiKey = (key) => globalThis.__mockValidateApiKey(key);\n`
);
// Wire the stub to our local variable
(globalThis as unknown as { __mockValidateApiKey: ValidateFn }).__mockValidateApiKey = (key) =>
mockValidateApiKey(key);
test.after(async () => {
try {
fs.unlinkSync(STUB_PATH);
} catch {
/* ignore */
}
await cleanupTempDataDir(TEST_DATA_DIR);
if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR;
else process.env.DATA_DIR = ORIGINAL_DATA_DIR;
});
// ─── Load policy fresh (after the interceptor is in place) ────────────────
async function loadPolicy() {
const mod = await import(`../../../src/server/authz/policies/clientApi.ts?ts=${Date.now()}`);
return mod.clientApiPolicy;
}
function ctx(headers: Headers, normalizedPath = "/api/v1/chat/completions") {
return {
request: { method: "POST", headers, url: `http://localhost${normalizedPath}` },
classification: {
routeClass: "CLIENT_API" as const,
reason: "client_api_v1" as const,
normalizedPath,
},
requestId: "req_test",
};
}
// ─── Tests ────────────────────────────────────────────────────────────────
test.beforeEach(() => {
// Default to "every key fails" — individual tests override as needed.
mockValidateApiKey = () => false;
delete process.env.REQUIRE_API_KEY;
});
test("#2257 — invalid bearer + REQUIRE_API_KEY=true → 401", async () => {
process.env.REQUIRE_API_KEY = "true";
const policy = await loadPolicy();
const headers = new Headers({ authorization: "Bearer sk-stub-bogus" });
const out = await policy.evaluate(ctx(headers));
assert.equal(out.allow, false);
if (!out.allow) {
assert.equal(out.status, 401);
assert.equal(out.code, "AUTH_002");
}
});
test("#2257 — invalid bearer + REQUIRE_API_KEY=false → anonymous (with warning log)", async () => {
const originalWarn = console.warn;
const warnings: string[] = [];
console.warn = (msg: string) => warnings.push(String(msg));
try {
const policy = await loadPolicy();
const headers = new Headers({ authorization: "Bearer sk-stub-bogus" });
const out = await policy.evaluate(ctx(headers));
assert.equal(out.allow, true);
if (out.allow) {
assert.equal(out.subject.kind, "anonymous");
assert.equal(out.subject.id, "local");
}
assert.ok(
warnings.some((w) => w.includes("[clientApiPolicy]") && w.includes("REQUIRE_API_KEY=false")),
"expected a warning about the fallback"
);
} finally {
console.warn = originalWarn;
}
});
test("#2257 — invalid x-api-key + REQUIRE_API_KEY=false → anonymous (with warning log)", async () => {
const originalWarn = console.warn;
const warnings: string[] = [];
console.warn = (msg: string) => warnings.push(String(msg));
try {
const policy = await loadPolicy();
const headers = new Headers({ "x-api-key": "sk-stub-bogus" });
const out = await policy.evaluate(ctx(headers));
assert.equal(out.allow, true);
if (out.allow) {
assert.equal(out.subject.kind, "anonymous");
assert.equal(out.subject.id, "local");
}
assert.ok(
warnings.some((w) => w.includes("[clientApiPolicy]") && w.includes("REQUIRE_API_KEY=false")),
"expected a warning about the fallback"
);
} finally {
console.warn = originalWarn;
}
});
test("#2257 — fallback warning masks the x-api-key (only last-4 in log)", async () => {
const originalWarn = console.warn;
const warnings: string[] = [];
console.warn = (msg: string) => warnings.push(String(msg));
try {
const policy = await loadPolicy();
const headers = new Headers({ "x-api-key": "sk-secretprefix-secretmiddle-XYZW" });
const out = await policy.evaluate(ctx(headers));
assert.equal(out.allow, true);
assert.ok(
warnings.every((w) => !w.includes("secretprefix") && !w.includes("secretmiddle")),
"warning leaked the full bearer; only masked key id should be logged"
);
assert.ok(
warnings.some((w) => w.includes("key_XYZW")),
"expected masked key id (last-4) in the warning"
);
} finally {
console.warn = originalWarn;
}
});
test("#2257 — fallback warning masks the bearer (only last-4 in log)", async () => {
const originalWarn = console.warn;
const warnings: string[] = [];
console.warn = (msg: string) => warnings.push(String(msg));
try {
const policy = await loadPolicy();
const headers = new Headers({ authorization: "Bearer sk-secretprefix-secretmiddle-XYZW" });
const out = await policy.evaluate(ctx(headers));
assert.equal(out.allow, true);
assert.ok(
warnings.every((w) => !w.includes("secretprefix") && !w.includes("secretmiddle")),
"warning leaked the full bearer; only masked key id should be logged"
);
assert.ok(
warnings.some((w) => w.includes("key_XYZW")),
"expected masked key id (last-4) in the warning"
);
} finally {
console.warn = originalWarn;
}
});
test("#2257 — no bearer + REQUIRE_API_KEY=false → anonymous (unchanged, no fallback warning)", async () => {
const originalWarn = console.warn;
const warnings: string[] = [];
console.warn = (msg: string) => warnings.push(String(msg));
try {
const policy = await loadPolicy();
const out = await policy.evaluate(ctx(new Headers()));
assert.equal(out.allow, true);
if (out.allow) {
assert.equal(out.subject.kind, "anonymous");
}
// No warning should fire when no bearer is sent in the first place —
// the warning is specifically for the "invalid-bearer-fell-through" case.
assert.ok(
warnings.every((w) => !w.includes("[clientApiPolicy]")),
"no fallback warning expected when no bearer was sent"
);
} finally {
console.warn = originalWarn;
}
});
// ─── #3504 — non-usable Authorization must NOT short-circuit the URL path token ─
// VS Code Copilot sends its own (empty / non-OmniRoute) Authorization header even
// when the OmniRoute key lives in the URL path of a /vscode tokenized endpoint.
// A non-"Bearer <token>" Authorization must fall through to the URL token instead
// of returning null and 401'ing under REQUIRE_API_KEY=true.
// validateApiKey is the real (no-DB → always-false) implementation here, so we
// distinguish "URL token was extracted" from "no token found" by the rejection
// MESSAGE: an extracted-but-unknown token → "Invalid API key"; nothing extracted
// → "Authentication required". On the pre-fix code a non-Bearer Authorization
// returned null, so these would all 401 with "Authentication required".
test("#3504 — empty 'Bearer ' Authorization falls through to the URL path token", async () => {
process.env.REQUIRE_API_KEY = "true";
const policy = await loadPolicy();
const headers = new Headers({ authorization: "Bearer " });
const out = await policy.evaluate(ctx(headers, "/api/v1/vscode/sk-url-token/chat/completions"));
assert.equal(out.allow, false);
if (!out.allow) {
assert.equal(out.status, 401);
assert.equal(
out.message,
"Invalid API key",
"URL token must be extracted (→ 'Invalid API key'), not skipped (→ 'Authentication required')"
);
}
});
test("#3504 — a non-Bearer scheme (Basic) also falls through to the URL token", async () => {
process.env.REQUIRE_API_KEY = "true";
const policy = await loadPolicy();
const headers = new Headers({ authorization: "Basic Zm9vOmJhcg==" });
const out = await policy.evaluate(ctx(headers, "/api/v1/vscode/sk-url-token/chat/completions"));
assert.equal(out.allow, false);
if (!out.allow) assert.equal(out.message, "Invalid API key");
});
test("#3504 — non-Bearer Authorization with NO URL token still rejects as unauthenticated", async () => {
process.env.REQUIRE_API_KEY = "true";
const policy = await loadPolicy();
const headers = new Headers({ authorization: "Bearer " });
const out = await policy.evaluate(ctx(headers, "/api/v1/chat/completions"));
assert.equal(out.allow, false);
if (!out.allow) assert.equal(out.message, "Authentication required");
});