fix(db): keep test runs off the operator's real DATA_DIR (#10432)

Any process that opened the DB without setting DATA_DIR resolved to ~/.omniroute/storage.sqlite — the operator's live database, provider credentials included. tests/_setup/isolateDataDir.ts only covers the npm scripts; the documented single-file test command and ad-hoc probes bypassed it (one did exactly that during #10334).

resolveWritableDataDir now redirects a test-context process with no DATA_DIR to a throwaway temp dir, stable per process. Redirect rather than throw, so the documented single-file command keeps working; OMNIROUTE_ALLOW_DEFAULT_DATA_DIR=1 opts back in and records the intent.

Closes #10428
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-08-15 01:51:01 -03:00
committed by GitHub
parent 50cb187511
commit 2a04b2415a
6 changed files with 179 additions and 0 deletions

View File

@@ -45,6 +45,12 @@ INITIAL_PASSWORD=CHANGEME
# executor's on-disk thread-sticky session cache. Leave unset to rely on DATA_DIR.
# OMNIROUTE_DATA_DIR=/var/lib/omniroute
# Escape hatch for the test-context DATA_DIR guard (#10428). A test run that never
# chose a DATA_DIR is redirected to a throwaway temp dir so it cannot open the
# operator's real database. Set to 1 only for a deliberate run against the real
# DATA_DIR — never for CI. Used by: src/lib/dataPaths.ts
# OMNIROUTE_ALLOW_DEFAULT_DATA_DIR=1
# Encryption key for SQLite database encryption at rest.
# Used by: src/lib/db/encryption.ts — encrypts the entire SQLite database.
# Generate: openssl rand -hex 32 | Leave empty to disable DB encryption.

View File

@@ -83,6 +83,7 @@ OmniRoute uses **SQLite** (via `better-sqlite3`) for all persistence. These vari
| Variable | Default | Source File | Description |
| -------------------------------------- | -------------------- | ----------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `DATA_DIR` | `~/.omniroute/` | `src/lib/db/core.ts` | Root directory for SQLite DB, backups, and data files. Override for Docker volumes or custom paths. |
| `OMNIROUTE_ALLOW_DEFAULT_DATA_DIR` | _(unset)_ | `src/lib/dataPaths.ts` | Escape hatch for the test-context DATA_DIR guard (#10428). Test runs with no `DATA_DIR` are redirected to a throwaway temp dir so they cannot open the operator's real database; set to `1` to opt back in to the real directory. |
| `OMNIROUTE_DATA_DIR` | _(unset)_ | `open-sse/executors/promptql/threadSticky.ts` | **Fallback alias** for `DATA_DIR`, checked only when `DATA_DIR` is unset. Used to locate the PromptQL executor's on-disk thread-sticky session cache (`<dir>/promptql-thread-sessions.json`); if neither var is set, the cache stays in-memory only (not persisted across restarts). |
| `STORAGE_ENCRYPTION_KEY` | _(empty = disabled)_ | `src/lib/db/encryption.ts` | AES key for full SQLite database encryption at rest. Generate with `openssl rand -hex 32`. |
| `STORAGE_ENCRYPTION_KEY_VERSION` | `v1` | `scripts/build/bootstrap-env.mjs`, `electron/main.js` | Version label for the encryption key. Increment when performing key rotation to support decryption of old backups. |

View File

@@ -83,12 +83,54 @@ export function resolveDataDir({ isCloud = false }: { isCloud?: boolean } = {}):
* Use this only at the single startup site that owns directory creation
* (currently `db/core.ts`); everywhere else keep using the pure resolver.
*/
/**
* #10428: true when this process looks like a test run rather than a server start.
*
* `NODE_TEST_CONTEXT` is set by `node --test` in every spawned test process, `VITEST` by
* vitest, and `NODE_ENV=test` by the npm scripts — between them they cover both runners
* plus the AGENTS.md single-file command, which does NOT load
* `tests/_setup/isolateDataDir.ts`.
*/
function isTestContext(): boolean {
return (
process.env.NODE_ENV === "test" ||
!!process.env.VITEST ||
!!process.env.NODE_TEST_CONTEXT ||
process.execArgv.includes("--test") ||
process.argv.includes("--test")
);
}
/** Process-wide redirect target, so repeated calls share one DB instead of one per call. */
let testContextDataDir: string | null = null;
export function resolveWritableDataDir({ isCloud = false }: { isCloud?: boolean } = {}): string {
const resolved = resolveDataDir({ isCloud });
// Cloud/serverless never owns a writable home dir; leave its sentinel alone.
if (isCloud) return resolved;
// #10428: a test/ad-hoc run that never chose a DATA_DIR would otherwise open the
// OPERATOR'S REAL database (~/.omniroute/storage.sqlite — live provider credentials).
// Redirect to a throwaway dir instead of throwing: the documented single-file command
// (`node --import tsx/esm --test tests/unit/x.test.ts`) does not load the isolation
// setup, and a hard failure there would only teach people to disable the guard.
// `OMNIROUTE_ALLOW_DEFAULT_DATA_DIR=1` opts back in, so the intent is recorded.
if (
!process.env.DATA_DIR &&
isTestContext() &&
process.env.OMNIROUTE_ALLOW_DEFAULT_DATA_DIR !== "1"
) {
if (!testContextDataDir) {
testContextDataDir = fs.mkdtempSync(path.join(os.tmpdir(), `${APP_NAME}-testctx-`));
console.warn(
`[DATA_DIR] test context without DATA_DIR → using '${testContextDataDir}' instead of ` +
`'${resolved}'. Set DATA_DIR explicitly (or load tests/_setup/isolateDataDir.ts) to silence this.`
);
}
return testContextDataDir;
}
// No explicit override → already the default user dir; nothing to fall back to.
const configured = normalizeConfiguredPath(process.env.DATA_DIR);
if (!configured) return resolved;

View File

@@ -84,6 +84,11 @@ test("resolveWritableDataDir falls back to the default dir when DATA_DIR is not
test("resolveWritableDataDir returns the default dir (no probe) when DATA_DIR is unset", async () => {
await withTempEnv(() => {
delete process.env.DATA_DIR;
// #10428: this asserts the SERVER path. Since the test-context guard now redirects a
// DATA_DIR-less test process to a temp dir (so a test can never open the operator's
// real DB), opt back in explicitly here — otherwise this test would be asserting the
// guard's behavior instead of the server's.
process.env.OMNIROUTE_ALLOW_DEFAULT_DATA_DIR = "1";
const resolved = resolveWritableDataDir();
assert.equal(resolved, getDefaultDataDir());
// Matches the pure resolver when no override is present.

View File

@@ -0,0 +1,120 @@
import test from "node:test";
import assert from "node:assert/strict";
import os from "node:os";
import path from "node:path";
import fs from "node:fs";
/**
* #10428 — a script or test that opens the DB without setting DATA_DIR resolves to the
* operator's REAL database (`~/.omniroute/storage.sqlite`, credentials included).
* `tests/_setup/isolateDataDir.ts` protects the npm test scripts, but it is opt-in per
* invocation: the AGENTS.md-documented single-file command
* (`node --import tsx/esm --test tests/unit/x.test.ts`) does NOT load it, and neither does
* an ad-hoc `node --import tsx probe.ts`.
*
* The guard therefore lives at the one place that actually opens the DB
* (`resolveWritableDataDir`, consumed only by `src/lib/db/core.ts`): in a test context
* pointing at the default user data dir, it redirects to a throwaway temp dir instead of
* touching the real one. Redirecting rather than throwing keeps the documented
* single-file command working — a hard failure there would just teach people to unset the
* guard.
*/
const { resolveWritableDataDir, getDefaultDataDir } = await import("../../src/lib/dataPaths.ts");
function withEnv(overrides: Record<string, string | undefined>, run: () => void) {
const saved: Record<string, string | undefined> = {};
for (const [key, value] of Object.entries(overrides)) {
saved[key] = process.env[key];
if (value === undefined) delete process.env[key];
else process.env[key] = value;
}
try {
run();
} finally {
for (const [key, value] of Object.entries(saved)) {
if (value === undefined) delete process.env[key];
else process.env[key] = value;
}
}
}
test("G1: a test context with no DATA_DIR never resolves to the operator's real data dir", () => {
withEnv({ DATA_DIR: undefined, NODE_ENV: "test", OMNIROUTE_ALLOW_DEFAULT_DATA_DIR: undefined }, () => {
const resolved = resolveWritableDataDir();
assert.notEqual(
resolved,
getDefaultDataDir(),
"a test run must never be handed the operator's real DATA_DIR"
);
assert.ok(
resolved.startsWith(os.tmpdir()),
`expected a throwaway temp dir, got ${resolved}`
);
assert.ok(fs.existsSync(resolved), "the redirected dir must exist and be usable");
});
});
test("G2: an explicit DATA_DIR still wins inside a test context", () => {
const explicit = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-explicit-"));
withEnv({ DATA_DIR: explicit, NODE_ENV: "test" }, () => {
assert.equal(resolveWritableDataDir(), explicit);
});
fs.rmSync(explicit, { recursive: true, force: true });
});
test("G3: the escape hatch restores the old behavior for deliberate runs", () => {
withEnv(
{ DATA_DIR: undefined, NODE_ENV: "test", OMNIROUTE_ALLOW_DEFAULT_DATA_DIR: "1" },
() => {
assert.equal(
resolveWritableDataDir(),
getDefaultDataDir(),
"an explicit opt-in must still reach the real dir, so the intent is recorded"
);
}
);
});
test("G4: a normal server run (no test markers) is untouched", () => {
withEnv(
{
DATA_DIR: undefined,
NODE_ENV: "production",
VITEST: undefined,
NODE_TEST_CONTEXT: undefined,
OMNIROUTE_ALLOW_DEFAULT_DATA_DIR: undefined,
},
() => {
assert.equal(
resolveWritableDataDir(),
getDefaultDataDir(),
"the server must keep resolving to the real data dir"
);
}
);
});
test("G5: node:test subprocesses are detected through NODE_TEST_CONTEXT too", () => {
withEnv(
{
DATA_DIR: undefined,
NODE_ENV: undefined,
NODE_TEST_CONTEXT: "child-v8",
OMNIROUTE_ALLOW_DEFAULT_DATA_DIR: undefined,
},
() => {
const resolved = resolveWritableDataDir();
assert.notEqual(resolved, getDefaultDataDir());
assert.ok(resolved.startsWith(os.tmpdir()));
}
);
});
test("G6: the redirect is stable within a process (same dir on repeated calls)", () => {
withEnv({ DATA_DIR: undefined, NODE_ENV: "test" }, () => {
const first = resolveWritableDataDir();
const second = resolveWritableDataDir();
assert.equal(first, second, "a per-call temp dir would split the DB across handles");
});
});

View File

@@ -464,6 +464,11 @@ test(
HOME: fakeHome,
USERPROFILE: fakeHome,
APPDATA: undefined,
// #10428: this pins the SERVER fallback (home data dir). The test-context guard
// would otherwise redirect this DATA_DIR-less process to a temp dir — correct for
// real test runs, but it would turn this assertion into a test of the guard rather
// than of the home-dir fallback. `fakeHome` already keeps the real DB out of reach.
OMNIROUTE_ALLOW_DEFAULT_DATA_DIR: "1",
},
async () => {
const core = await importFresh("src/lib/db/core.ts");