diff --git a/.env.example b/.env.example index 23cfc66856..5d1f674607 100644 --- a/.env.example +++ b/.env.example @@ -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. diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index 1325c9e5f6..2cab810ee9 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -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 (`/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. | diff --git a/src/lib/dataPaths.ts b/src/lib/dataPaths.ts index 5ad61ddbee..c73d139f36 100644 --- a/src/lib/dataPaths.ts +++ b/src/lib/dataPaths.ts @@ -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; diff --git a/tests/unit/data-dir-writable-fallback.test.ts b/tests/unit/data-dir-writable-fallback.test.ts index 77661b530c..70421108f5 100644 --- a/tests/unit/data-dir-writable-fallback.test.ts +++ b/tests/unit/data-dir-writable-fallback.test.ts @@ -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. diff --git a/tests/unit/datadir-test-context-guard-10428.test.ts b/tests/unit/datadir-test-context-guard-10428.test.ts new file mode 100644 index 0000000000..e957a490fa --- /dev/null +++ b/tests/unit/datadir-test-context-guard-10428.test.ts @@ -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, run: () => void) { + const saved: Record = {}; + 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"); + }); +}); diff --git a/tests/unit/db-core-init.test.ts b/tests/unit/db-core-init.test.ts index af26c11c51..553795757f 100644 --- a/tests/unit/db-core-init.test.ts +++ b/tests/unit/db-core-init.test.ts @@ -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");