mirror of
https://github.com/mvschwarz/openrig.git
synced 2026-11-07 05:56:21 -10:00
test: isolate inherited OpenRig instance selectors across Node suites (#780)
Node test suites run inside an OpenRig seat no longer inherit that seat's instance. The daemon and TUI suites now use the same fixture-home and fetch-guard setup as the CLI suite (test/hermetic-env.setup.ts). OPENRIG_* and RIGGED_* instance selectors are cleared before product imports, so tests cannot reach the person's OpenRig home, database or daemon endpoint. Ordinary runs also redirect HOME and the XDG directories and clear CODEX_HOME. With OPENRIG_E2E_REAL_CODEX=1, the real HOME, CODEX_HOME and XDG directories are kept, so the opt-in real-Codex tests run as before. Explicit fixture overrides set after setup still apply. Test-only; no product code changes. Controls against disposable outer audit, SQLite and HTTP targets fail with the old setup and pass with the new one. Not covered: subprocess globals and hardcoded filesystem targets outside the fetch guard.
This commit is contained in:
1 parent
aa4ee793cd
commit
33eaeda30f
7 files changed
+215
-72
No files matched your search
@@ -1,71 +1,2 @@
|
|||||||
// D12 base-health + WRITE-leak containment (broadcast-leak atom).
|
// Shared Node-suite isolation runs before any product module captures its home.
|
||||||
//
|
import "../../../test/hermetic-env.setup.js";
|
||||||
// The cli test suite MUST NOT inherit the seat's live-daemon connection. Two holes:
|
|
||||||
//
|
|
||||||
// 1. Connection-redirect env (OPENRIG_URL / OPENRIG_PORT / OPENRIG_HOST + RIGGED_*
|
|
||||||
// aliases) — honored as a real operator override in production, but ambient inside
|
|
||||||
// a seat it bypasses each test's injected mock daemon (up.test.ts: 29 fails ambient
|
|
||||||
// vs 1 hermetic). Scrubbed below. Production behavior is untouched.
|
|
||||||
//
|
|
||||||
// 2. The DEFAULT STATE_FILE discovery — `OPENRIG_HOME/daemon.json` (daemon-lifecycle
|
|
||||||
// `OPENRIG_DIR = OPENRIG_HOME`). Scrubbing the env is NOT enough: with URL/PORT
|
|
||||||
// unset, the CLI falls back to the live daemon's state file. This is exactly how
|
|
||||||
// `broadcast.test.ts` emitted REAL 'System maintenance' broadcasts to 14 seats.
|
|
||||||
// The READ direction (results pollution) was known; the WRITE direction (emitting
|
|
||||||
// into the live topology) is strictly worse.
|
|
||||||
//
|
|
||||||
// FORCE + ASSERT (unwritable-by-omission — the guarantee lives where it cannot be
|
|
||||||
// skipped, so a new test file inherits safety from the setup that runs first, not
|
|
||||||
// from a runner or a per-file convention that 145/164 files already omit):
|
|
||||||
// FORCE — establish a fixture-scoped OPENRIG_HOME (a fresh temp dir carrying the
|
|
||||||
// `.openrig-fixture` marker) BEFORE any daemon-resolving module loads and
|
|
||||||
// captures the eager `OPENRIG_HOME` const, so default discovery finds no
|
|
||||||
// live daemon.
|
|
||||||
// ASSERT — `assertFixtureScopedHome` throws LOUD if the home is NOT fixture-scoped
|
|
||||||
// (mkdtemp failure, or OPENRIG_HOME captured before this ran via an
|
|
||||||
// import-order regression). Forcing alone silently fixes and proves
|
|
||||||
// nothing; the assert makes it evidence. Its known-negative lives in
|
|
||||||
// live-daemon-guard.test.ts.
|
|
||||||
import fs from "node:fs";
|
|
||||||
import os from "node:os";
|
|
||||||
import path from "node:path";
|
|
||||||
|
|
||||||
// (1) Scrub the connection-redirect vars (desk-authoritative set).
|
|
||||||
for (const key of ["OPENRIG_URL", "OPENRIG_PORT", "RIGGED_URL", "RIGGED_PORT", "OPENRIG_HOST_SELECTED", "OPENRIG_HOST", "OPENRIG_BIND_HOST"]) {
|
|
||||||
delete process.env[key];
|
|
||||||
}
|
|
||||||
|
|
||||||
// (2) FORCE a fixture-scoped home — BEFORE importing openrig-compat (whose
|
|
||||||
// OPENRIG_HOME const is eager). Node builtins above do not load it; the dynamic
|
|
||||||
// imports below run only AFTER this assignment, so the const captures the fixture.
|
|
||||||
// ONE stable fixture home per worker. setupFiles runs per test FILE, but
|
|
||||||
// openrig-compat's OPENRIG_HOME const is captured ONCE per worker — a fresh per-file
|
|
||||||
// home would diverge from that captured const (env says home N, the const holds home
|
|
||||||
// 1), which flakes any test reading one vs the other under suite ordering. So reuse a
|
|
||||||
// fixture home already established in this worker. Named as a realistic `.openrig`-style
|
|
||||||
// home (still fixture-scoped via the marker) so path-shape assertions hold against it.
|
|
||||||
const existingHome = process.env["OPENRIG_HOME"];
|
|
||||||
const fixtureHome =
|
|
||||||
existingHome && fs.existsSync(path.join(existingHome, ".openrig-fixture"))
|
|
||||||
? existingHome
|
|
||||||
: (() => {
|
|
||||||
const h = fs.mkdtempSync(path.join(os.tmpdir(), ".openrig-cli-fixture-home-"));
|
|
||||||
fs.writeFileSync(path.join(h, ".openrig-fixture"), "");
|
|
||||||
return h;
|
|
||||||
})();
|
|
||||||
process.env["OPENRIG_HOME"] = fixtureHome;
|
|
||||||
delete process.env["RIGGED_HOME"];
|
|
||||||
|
|
||||||
// (3) ASSERT — capture the resolved home + fail loud if it is not fixture-scoped.
|
|
||||||
const { OPENRIG_HOME } = await import("../src/openrig-compat.js");
|
|
||||||
const { assertFixtureScopedHome } = await import("./live-daemon-guard.js");
|
|
||||||
assertFixtureScopedHome(OPENRIG_HOME);
|
|
||||||
|
|
||||||
// (4) P37 REQUEST-LAYER GUARD — the universal outbound-request chokepoint. The two
|
|
||||||
// guards above close the DISCOVERY paths (connection env, state-file home); this closes
|
|
||||||
// the REQUEST layer, where a hardcoded :7433 literal, a mocked URL getter with an
|
|
||||||
// un-mocked client, or a production default constant still issues a real request that
|
|
||||||
// no env-scrub or fixture-home can catch. Fail-closed on any unregistered target; an
|
|
||||||
// in-process fixture registers its origin (allowFetchTarget) when it binds.
|
|
||||||
const { installFetchGuard } = await import("./fetch-guard.js");
|
|
||||||
installFetchGuard();
|
|
||||||
@@ -0,0 +1,45 @@
|
|||||||
|
import { describe, expect, it, vi } from "vitest";
|
||||||
|
import { mkdirSync, mkdtempSync, rmSync } from "node:fs";
|
||||||
|
import { dirname, join } from "node:path";
|
||||||
|
import { tmpdir } from "node:os";
|
||||||
|
import { createDb } from "../src/db/connection.js";
|
||||||
|
import { ConfigStore } from "../../cli/src/config-store.js";
|
||||||
|
import { DaemonClient } from "../../cli/src/client.js";
|
||||||
|
|
||||||
|
describe("test instance environment", () => {
|
||||||
|
// scripts/test-instance-isolation.test.mjs supplies only a disposable outer
|
||||||
|
// home/database/HTTP server. Never run this discriminator against a real seat.
|
||||||
|
it.skipIf(!process.env.OPENRIG_TEST_OUTER_INSTANCE)("exercises default database and endpoint routing", async () => {
|
||||||
|
const file = new ConfigStore().resolve().db.path;
|
||||||
|
mkdirSync(dirname(file), { recursive: true });
|
||||||
|
const db = createDb(file);
|
||||||
|
try { db.exec("CREATE TABLE IF NOT EXISTS isolation_probe (value TEXT)"); }
|
||||||
|
finally { db.close(); }
|
||||||
|
try {
|
||||||
|
const response = await new DaemonClient().get("/api/test-isolation");
|
||||||
|
expect(response.status).toBe(200);
|
||||||
|
} catch (error) {
|
||||||
|
// The isolated default is not a fixture server; the existing fetch guard
|
||||||
|
// must reject it before any network request. The parent reaches the outer server.
|
||||||
|
expect(String(error)).toContain("FETCH GUARD");
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("preserves explicit fixture overrides made after setup", async () => {
|
||||||
|
const root = mkdtempSync(join(tmpdir(), "instance-override-"));
|
||||||
|
try {
|
||||||
|
vi.stubEnv("OPENRIG_HOME", root);
|
||||||
|
vi.stubEnv("OPENRIG_DB", join(root, "chosen.sqlite"));
|
||||||
|
vi.stubEnv("OPENRIG_URL", "http://fixture.invalid:12345");
|
||||||
|
expect(new ConfigStore().resolve().db.path).toBe(join(root, "chosen.sqlite"));
|
||||||
|
const fetchImpl = vi.fn(async () => new Response("{}", { status: 200 }));
|
||||||
|
const client = new DaemonClient(undefined, { fetchImpl });
|
||||||
|
expect(client.baseUrl).toBe("http://fixture.invalid:12345");
|
||||||
|
await client.get("/api/test-isolation");
|
||||||
|
expect(fetchImpl).toHaveBeenCalled();
|
||||||
|
} finally {
|
||||||
|
vi.unstubAllEnvs();
|
||||||
|
rmSync(root, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -2,6 +2,7 @@ import { defineConfig } from "vitest/config";
|
|||||||
|
|
||||||
export default defineConfig({
|
export default defineConfig({
|
||||||
test: {
|
test: {
|
||||||
|
setupFiles: ["../../test/hermetic-env.setup.ts"],
|
||||||
include: ["test/**/*.test.ts"],
|
include: ["test/**/*.test.ts"],
|
||||||
// P6/D12-residue: many daemon suites spawn REAL child processes (stub-runner-*,
|
// P6/D12-residue: many daemon suites spawn REAL child processes (stub-runner-*,
|
||||||
// precompact-hook, bridge, restore-from-jsonl — 20+ files). Under fold-gate
|
// precompact-hook, bridge, restore-from-jsonl — 20+ files). Under fold-gate
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import { defineConfig } from "vitest/config";
|
|||||||
|
|
||||||
export default defineConfig({
|
export default defineConfig({
|
||||||
test: {
|
test: {
|
||||||
|
setupFiles: ["../../test/hermetic-env.setup.ts"],
|
||||||
include: ["test/**/*.test.ts"],
|
include: ["test/**/*.test.ts"],
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
@@ -0,0 +1,70 @@
|
|||||||
|
import { test } from "node:test";
|
||||||
|
import assert from "node:assert/strict";
|
||||||
|
import { spawn } from "node:child_process";
|
||||||
|
import { createServer } from "node:http";
|
||||||
|
import { once } from "node:events";
|
||||||
|
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, rmSync } from "node:fs";
|
||||||
|
import { tmpdir } from "node:os";
|
||||||
|
import { join, dirname } from "node:path";
|
||||||
|
import { fileURLToPath } from "node:url";
|
||||||
|
|
||||||
|
const repo = dirname(dirname(fileURLToPath(import.meta.url)));
|
||||||
|
|
||||||
|
for (const prefix of ["OPENRIG", "RIGGED"]) {
|
||||||
|
test(`npm daemon tests leave an inherited ${prefix} instance untouched`, { timeout: 60000 }, async () => {
|
||||||
|
const outer = mkdtempSync(join(tmpdir(), "test-outer-instance-"));
|
||||||
|
const home = join(outer, "user"), state = join(outer, "state");
|
||||||
|
mkdirSync(home); mkdirSync(state);
|
||||||
|
const audit = join(state, "bundle-audit.jsonl");
|
||||||
|
const database = join(outer, "outer.sqlite");
|
||||||
|
writeFileSync(audit, "outer sentinel\n");
|
||||||
|
const requests = [];
|
||||||
|
const server = createServer((req, res) => {
|
||||||
|
requests.push(req.url);
|
||||||
|
res.writeHead(200, { "content-type": "application/json" });
|
||||||
|
res.end("{}");
|
||||||
|
});
|
||||||
|
server.listen(0, "127.0.0.1");
|
||||||
|
await once(server, "listening");
|
||||||
|
const port = server.address().port;
|
||||||
|
// Both state-file and explicit-env discovery point only at this disposable server.
|
||||||
|
writeFileSync(join(state, "daemon.json"), JSON.stringify({ pid: process.pid, port, host: "127.0.0.1" }));
|
||||||
|
const env = { ...process.env, HOME: home, OPENRIG_TEST_OUTER_INSTANCE: outer };
|
||||||
|
for (const key of Object.keys(env)) {
|
||||||
|
if (/^(OPENRIG|RIGGED)_(HOME|URL|DB|PORT|HOST|HOST_SELECTED)$/.test(key)) delete env[key];
|
||||||
|
}
|
||||||
|
Object.assign(env, {
|
||||||
|
[`${prefix}_HOME`]: state, [`${prefix}_DB`]: database,
|
||||||
|
[`${prefix}_URL`]: `http://127.0.0.1:${port}`, [`${prefix}_PORT`]: String(port),
|
||||||
|
[`${prefix}_HOST`]: "127.0.0.1", OPENRIG_HOST_SELECTED: "outer-fixture",
|
||||||
|
});
|
||||||
|
let output = "";
|
||||||
|
try {
|
||||||
|
const child = spawn("npm", ["test", "-w", "packages/daemon", "--",
|
||||||
|
"test/bundle-routes.test.ts", "test/instance-environment.test.ts", "--maxWorkers=1",
|
||||||
|
"-t", "v2 bundle enters pod-aware path|test instance environment"],
|
||||||
|
{ cwd: repo, env, detached: process.platform !== "win32", stdio: ["ignore", "pipe", "pipe"] });
|
||||||
|
child.stdout.on("data", chunk => { output += chunk; });
|
||||||
|
child.stderr.on("data", chunk => { output += chunk; });
|
||||||
|
const timeout = setTimeout(() => {
|
||||||
|
if (process.platform === "win32") child.kill("SIGKILL");
|
||||||
|
else process.kill(-child.pid, "SIGKILL"); // Only this owned npm/Vitest process group.
|
||||||
|
}, 50000);
|
||||||
|
const [code, signal] = await once(child, "close").finally(() => clearTimeout(timeout));
|
||||||
|
const effects = {
|
||||||
|
auditUnchanged: readFileSync(audit, "utf8") === "outer sentinel\n",
|
||||||
|
databaseUntouched: !existsSync(database),
|
||||||
|
routedRequests: requests,
|
||||||
|
};
|
||||||
|
// Inspect effects before the child exit, so the parent discriminator names
|
||||||
|
// the actual leak even if another assertion in that run also fails.
|
||||||
|
assert.deepEqual(effects, { auditUnchanged: true, databaseUntouched: true, routedRequests: [] });
|
||||||
|
assert.equal(signal, null, output);
|
||||||
|
assert.equal(code, 0, output);
|
||||||
|
} finally {
|
||||||
|
server.closeAllConnections();
|
||||||
|
await new Promise(resolve => server.close(resolve));
|
||||||
|
rmSync(outer, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
@@ -0,0 +1,95 @@
|
|||||||
|
// D12 base-health + WRITE-leak containment (broadcast-leak atom).
|
||||||
|
//
|
||||||
|
// The Node test suites MUST NOT inherit the seat's live-daemon connection. Two holes:
|
||||||
|
//
|
||||||
|
// 1. Connection-redirect env (OPENRIG_URL / OPENRIG_PORT / OPENRIG_HOST + RIGGED_*
|
||||||
|
// aliases) — honored as a real operator override in production, but ambient inside
|
||||||
|
// a seat it bypasses each test's injected mock daemon (up.test.ts: 29 fails ambient
|
||||||
|
// vs 1 hermetic). Scrubbed below. Production behavior is untouched.
|
||||||
|
//
|
||||||
|
// 2. The DEFAULT STATE_FILE discovery — `OPENRIG_HOME/daemon.json` (daemon-lifecycle
|
||||||
|
// `OPENRIG_DIR = OPENRIG_HOME`). Scrubbing the env is NOT enough: with URL/PORT
|
||||||
|
// unset, the CLI falls back to the live daemon's state file. This is exactly how
|
||||||
|
// `broadcast.test.ts` emitted REAL 'System maintenance' broadcasts to 14 seats.
|
||||||
|
// The READ direction (results pollution) was known; the WRITE direction (emitting
|
||||||
|
// into the live topology) is strictly worse.
|
||||||
|
//
|
||||||
|
// FORCE + ASSERT (unwritable-by-omission — the guarantee lives where it cannot be
|
||||||
|
// skipped, so a new test file inherits safety from the setup that runs first, not
|
||||||
|
// from a runner or a per-file convention that 145/164 files already omit):
|
||||||
|
// FORCE — establish a fixture-scoped OPENRIG_HOME (a fresh temp dir carrying the
|
||||||
|
// `.openrig-fixture` marker) BEFORE any daemon-resolving module loads and
|
||||||
|
// captures the eager `OPENRIG_HOME` const, so default discovery finds no
|
||||||
|
// live daemon.
|
||||||
|
// ASSERT — `assertFixtureScopedHome` throws LOUD if the home is NOT fixture-scoped
|
||||||
|
// (mkdtemp failure, or OPENRIG_HOME captured before this ran via an
|
||||||
|
// import-order regression). Forcing alone silently fixes and proves
|
||||||
|
// nothing; the assert makes it evidence. Its known-negative lives in
|
||||||
|
// live-daemon-guard.test.ts.
|
||||||
|
import fs from "node:fs";
|
||||||
|
import os from "node:os";
|
||||||
|
import path from "node:path";
|
||||||
|
|
||||||
|
// Scrub inherited instance selectors before product modules capture their defaults.
|
||||||
|
// Tests can still set explicit overrides after setup; test/provider opt-in flags
|
||||||
|
// are not instance selectors and remain untouched.
|
||||||
|
for (const key of [
|
||||||
|
"URL", "PORT", "HOST", "BIND_HOST", "HOST_SELECTED", "DB",
|
||||||
|
"TRANSCRIPTS_PATH", "WORKSPACE_ROOT", "WORKSPACE_SLICES_ROOT",
|
||||||
|
"WORKSPACE_STEERING_PATH", "WORKSPACE_SPECS_ROOT", "WORKSPACE_PROJECTS_ROOT",
|
||||||
|
"WORKSPACE_CATALOG_PATH", "TOPOLOGY_ROOT", "CONTEXT_ROOT", "CONTEXT_PACKS_ROOT",
|
||||||
|
"SKILLS_ROOT", "SHARED_DOCS_ROOT", "FILES_ALLOWLIST", "PROGRESS_SCAN_ROOTS",
|
||||||
|
"SESSION_NAME", "NODE_ID", "RIG_ID", "AUTH_BEARER_TOKEN", "TERMINAL_BEARER_TOKEN",
|
||||||
|
]) {
|
||||||
|
delete process.env[`OPENRIG_${key}`];
|
||||||
|
delete process.env[`RIGGED_${key}`];
|
||||||
|
}
|
||||||
|
|
||||||
|
// (2) FORCE a fixture-scoped home — BEFORE importing openrig-compat (whose
|
||||||
|
// OPENRIG_HOME const is eager). Node builtins above do not load it; the dynamic
|
||||||
|
// imports below run only AFTER this assignment, so the const captures the fixture.
|
||||||
|
// ONE stable fixture home per worker. setupFiles runs per test FILE, but
|
||||||
|
// openrig-compat's OPENRIG_HOME const is captured ONCE per worker — a fresh per-file
|
||||||
|
// home would diverge from that captured const (env says home N, the const holds home
|
||||||
|
// 1), which flakes any test reading one vs the other under suite ordering. So reuse a
|
||||||
|
// fixture home already established in this worker. Named as a realistic `.openrig`-style
|
||||||
|
// home (still fixture-scoped via the marker) so path-shape assertions hold against it.
|
||||||
|
const existingHome = process.env["OPENRIG_HOME"];
|
||||||
|
const fixtureHome =
|
||||||
|
existingHome && fs.existsSync(path.join(existingHome, ".openrig-fixture"))
|
||||||
|
? existingHome
|
||||||
|
: (() => {
|
||||||
|
const h = fs.mkdtempSync(path.join(os.tmpdir(), ".openrig-test-fixture-home-"));
|
||||||
|
fs.writeFileSync(path.join(h, ".openrig-fixture"), "");
|
||||||
|
return h;
|
||||||
|
})();
|
||||||
|
process.env["OPENRIG_HOME"] = fixtureHome;
|
||||||
|
delete process.env["RIGGED_HOME"];
|
||||||
|
// Legacy ~/.rigged discovery and ordinary home-based config must also stay local
|
||||||
|
// to this fixture. Merely changing HOME would not override OPENRIG_HOME or DB.
|
||||||
|
// Keep ordinary user files outside the state root, as in a real installation.
|
||||||
|
// Otherwise file-transfer correctly rejects every ~/ path as OpenRig state.
|
||||||
|
if (process.env["OPENRIG_E2E_REAL_CODEX"] !== "1") {
|
||||||
|
const userHome = `${fixtureHome}-user-home`;
|
||||||
|
fs.mkdirSync(userHome, { recursive: true });
|
||||||
|
process.env["HOME"] = userHome;
|
||||||
|
delete process.env["CODEX_HOME"];
|
||||||
|
for (const [key, suffix] of Object.entries({
|
||||||
|
XDG_CONFIG_HOME: ".config", XDG_STATE_HOME: ".local/state",
|
||||||
|
XDG_DATA_HOME: ".local/share", XDG_CACHE_HOME: ".cache",
|
||||||
|
})) process.env[key] = path.join(userHome, suffix);
|
||||||
|
}
|
||||||
|
|
||||||
|
// (3) ASSERT — capture the resolved home + fail loud if it is not fixture-scoped.
|
||||||
|
const { OPENRIG_HOME } = await import("../packages/cli/src/openrig-compat.js");
|
||||||
|
const { assertFixtureScopedHome } = await import("../packages/cli/test/live-daemon-guard.js");
|
||||||
|
assertFixtureScopedHome(OPENRIG_HOME);
|
||||||
|
|
||||||
|
// (4) P37 REQUEST-LAYER GUARD — the universal outbound-request chokepoint. The two
|
||||||
|
// guards above close the DISCOVERY paths (connection env, state-file home); this closes
|
||||||
|
// the REQUEST layer, where a hardcoded :7433 literal, a mocked URL getter with an
|
||||||
|
// un-mocked client, or a production default constant still issues a real request that
|
||||||
|
// no env-scrub or fixture-home can catch. Fail-closed on any unregistered target; an
|
||||||
|
// in-process fixture registers its origin (allowFetchTarget) when it binds.
|
||||||
|
const { installFetchGuard } = await import("../packages/cli/test/fetch-guard.js");
|
||||||
|
installFetchGuard();
|
||||||
+1
-1
@@ -1,5 +1,5 @@
|
|||||||
// B15 — root-invocation hermeticity (r1-reviewed shape: test.projects, NOT the deprecated
|
// B15 — root-invocation hermeticity (r1-reviewed shape: test.projects, NOT the deprecated
|
||||||
// workspace file). The per-package vitest configs carry the hermetic-env setup (D12: scrub
|
// workspace file). The Node per-package vitest configs carry the hermetic-env setup (D12: scrub
|
||||||
// connection env, fixture-scoped OPENRIG_HOME, fetch guard), but a bare
|
// connection env, fixture-scoped OPENRIG_HOME, fetch guard), but a bare
|
||||||
// `npx vitest run packages/cli/test/x.test.ts` from the REPO ROOT found no config and ran
|
// `npx vitest run packages/cli/test/x.test.ts` from the REPO ROOT found no config and ran
|
||||||
// UNGUARDED — inside a seat that meant the suites escaped their mocks to the live daemon and
|
// UNGUARDED — inside a seat that meant the suites escaped their mocks to the live daemon and
|
||||||
|
|||||||
Reference in new issue
Block a user