diff --git a/apps/gateway/src/local-dev.spec.ts b/apps/gateway/src/local-dev.spec.ts index f75774013..fe93285d7 100644 --- a/apps/gateway/src/local-dev.spec.ts +++ b/apps/gateway/src/local-dev.spec.ts @@ -11,10 +11,10 @@ */ import { spawnSync } from 'node:child_process'; -import { mkdtempSync, readFileSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, readFileSync } from 'node:fs'; import { readFile, mkdtemp, realpath, rm } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { homedir, tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -290,11 +290,76 @@ describe('KBN-101-02a local-dev — the channel has its own command', () => { * is read, no connection is attempted, and nothing listens. The assertions below * treat that path as a FAILURE rather than a pass, so it cannot be mistaken for * a refusal. + * + * The child's environment is an ALLOWLIST, and that is the load-bearing part. + * It was a denylist — copy `process.env`, drop three keys — which left the real + * `HOME`, every XDG root, `MOSAIC_GATEWAY_HOME`, service endpoints, auth and + * credential families, and the Node loader controls in place. Two things follow + * from that, and both are worse than a missing case. A faulty GREEN could read + * real host state before any assertion caught it, which is the same defect that + * makes an unhermetic fixture unable to certify what it claims. And a refusal + * could be produced by the host's own configuration rather than by the route, + * so the case would pass while proving nothing about the code under test. A + * denylist is only as good as the enumeration of what is dangerous, which is + * unbounded; an allowlist is bounded by what the route actually needs. */ describe('KBN-101-02a local-dev — the package route refuses, not merely the module', () => { const GATEWAY_DIR = join(HERE, '..'); const CHILD_TIMEOUT_MS = 120_000; + const REPO_ROOT = join(GATEWAY_DIR, '..', '..'); + + /** + * The complete set of variables the child is permitted to receive, before the + * per-case declarations are layered on. Nothing else is inherited. + * + * `PATH` is constructed rather than inherited: the two workspace `.bin` + * directories the package route resolves `tsx` from, and the directory holding + * the Node binary already running this suite. A host `PATH` would let an + * unrelated executable of the same name answer, which is the executable- + * resolution half of the same hermeticity problem. + */ + function buildHermeticChildEnv( + sandbox: string, + extraEnv: Record = {}, + ): Record { + const home = join(sandbox, 'home'); + const base: Record = { + HOME: home, + // Every XDG root, not only the config one. Leaving any of them ambient + // leaves a real directory the child can read or write. + XDG_CONFIG_HOME: join(home, '.config'), + XDG_DATA_HOME: join(home, '.local', 'share'), + XDG_STATE_HOME: join(home, '.local', 'state'), + XDG_CACHE_HOME: join(home, '.cache'), + XDG_RUNTIME_DIR: join(sandbox, 'run'), + TMPDIR: join(sandbox, 'tmp'), + PATH: [ + join(GATEWAY_DIR, 'node_modules', '.bin'), + join(REPO_ROOT, 'node_modules', '.bin'), + dirname(process.execPath), + ].join(':'), + }; + return { ...base, ...extraEnv }; + } + + /** A private root for one child run: home, XDG trees, tmp, runtime dir. */ + function makeChildSandbox(): string { + const sandbox = mkdtempSync(join(tmpdir(), 'kbn-02a-child-')); + tempRoots.push(sandbox); + for (const relative of [ + join('home', '.config'), + join('home', '.local', 'share'), + join('home', '.local', 'state'), + join('home', '.cache'), + 'run', + 'tmp', + ]) { + mkdirSync(join(sandbox, relative), { recursive: true }); + } + return sandbox; + } + function runDevLocal(extraEnv: Record = {}): { status: number | null; signal: NodeJS.Signals | null; @@ -306,23 +371,9 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo const script = manifest.scripts?.['dev:local']; expect(script, 'no dev:local script to execute').toBeDefined(); - const env: Record = {}; - for (const [key, value] of Object.entries(process.env)) { - if (value === undefined) { - continue; - } - // Ambient declarations would defeat the point: this case is about what the - // route does when nothing has been declared. - if (key.startsWith('MOSAIC_LOCAL_DEV_') || key === 'DATABASE_URL') { - continue; - } - env[key] = value; - } - delete env['BETTER_AUTH_SECRET']; - const result = spawnSync('sh', ['-c', script as string], { cwd: GATEWAY_DIR, - env: { ...env, ...extraEnv }, + env: buildHermeticChildEnv(makeChildSandbox(), extraEnv), encoding: 'utf8', timeout: CHILD_TIMEOUT_MS, killSignal: 'SIGKILL', @@ -356,6 +407,8 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo MOSAIC_LOCAL_DEV_STORAGE: 'pglite', MOSAIC_LOCAL_DEV_QUEUE: 'local', MOSAIC_LOCAL_DEV_STATE_ROOT: makeRouteStateRoot(), + // Declared explicitly rather than inherited: the child receives no + // MOSAIC_* variable this case does not name. }); expect(signal, 'the route hung and had to be killed').toBeNull(); @@ -363,4 +416,103 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo expect(output).toContain('LOCAL_START_POSTGRES_DSN_PRESENT'); expect(output, 'the refusal echoed the DSN it refused').not.toContain('mosaic:mosaic'); }); + + /** + * A harness negative control, not coverage. It asserts nothing about the code + * under test; it asserts that the instrument the two cases above depend on is + * actually an allowlist. It is green by construction and is classified that + * way in the document. Its value is that it fails the moment someone widens + * the fixture back toward `...process.env` — the change that would silently + * un-verify both cases above without reddening either of them. + */ + it('fixture control: the child environment is exactly the allowlist plus the declarations', () => { + const ALLOWED = [ + 'HOME', + 'PATH', + 'TMPDIR', + 'XDG_CACHE_HOME', + 'XDG_CONFIG_HOME', + 'XDG_DATA_HOME', + 'XDG_RUNTIME_DIR', + 'XDG_STATE_HOME', + ]; + + const sandbox = makeChildSandbox(); + const bare = buildHermeticChildEnv(sandbox); + expect(Object.keys(bare).sort()).toEqual(ALLOWED); + + const declared = buildHermeticChildEnv(sandbox, { + MOSAIC_LOCAL_DEV_TIER: 'local', + DATABASE_URL: FORBIDDEN_DSN, + }); + expect(Object.keys(declared).sort()).toEqual( + [...ALLOWED, 'DATABASE_URL', 'MOSAIC_LOCAL_DEV_TIER'].sort(), + ); + + // The real home and the real config roots are unreachable: the child's HOME + // is a private directory, and it is not an ancestor of the real one either. + expect(bare['HOME']).not.toBe(homedir()); + expect(bare['HOME']?.startsWith(sandbox)).toBe(true); + expect(homedir().startsWith(bare['HOME'] as string)).toBe(false); + for (const value of Object.values(bare)) { + expect(value, `an allowlisted value still points at the real home: ${value}`).not.toContain( + join(homedir(), '.config', 'mosaic'), + ); + } + + // PATH is three known directories, none of them ambient. + expect(bare['PATH']?.split(':')).toEqual([ + join(GATEWAY_DIR, 'node_modules', '.bin'), + join(REPO_ROOT, 'node_modules', '.bin'), + dirname(process.execPath), + ]); + }); + + /** + * The end-to-end half of the same obligation. The control above proves the + * object; this proves the process built from it. The parent declares a + * complete, valid local start AND a forbidden DSN in its own ambient + * environment, then runs the route with nothing passed in. A leaky fixture + * would hand the child a valid declaration, so the undeclared refusal would + * not fire — and the DSN would refuse instead, which is the discriminator. + */ + it('does not leak the parent ambient environment into the child', () => { + const canaries: Record = { + MOSAIC_LOCAL_DEV_TIER: 'local', + MOSAIC_LOCAL_DEV_STORAGE: 'pglite', + MOSAIC_LOCAL_DEV_QUEUE: 'local', + MOSAIC_LOCAL_DEV_STATE_ROOT: makeRouteStateRoot(), + MOSAIC_GATEWAY_HOME: makeRouteStateRoot(), + DATABASE_URL: FORBIDDEN_DSN, + }; + const restore = new Map(); + for (const [key, value] of Object.entries(canaries)) { + restore.set(key, process.env[key]); + process.env[key] = value; + } + + try { + const { status, signal, output } = runDevLocal(); + + expect(signal, 'the route hung and had to be killed').toBeNull(); + expect(status, 'the route exited 0 — it saw the parent declaration').not.toBe(0); + expect(output).toMatch(/LOCAL_START_[A-Z_]+/); + // If either of these fires, the child read the parent's environment. + expect(output, 'the child inherited the parent DSN').not.toContain( + 'LOCAL_START_POSTGRES_DSN_PRESENT', + ); + expect(output, 'the child inherited the parent DSN').not.toContain('mosaic:mosaic'); + expect(output, 'exited via bootstrap rather than refusing before it').not.toContain( + 'BETTER_AUTH_SECRET', + ); + } finally { + for (const [key, value] of restore) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + } + }); }); diff --git a/docs/native-kanban-sot/KBN-101-DB-ROLE-SPLIT.md b/docs/native-kanban-sot/KBN-101-DB-ROLE-SPLIT.md index 4f877fc2f..806014e8b 100644 --- a/docs/native-kanban-sot/KBN-101-DB-ROLE-SPLIT.md +++ b/docs/native-kanban-sot/KBN-101-DB-ROLE-SPLIT.md @@ -442,7 +442,7 @@ its stated reason is the same defect class as the span gap that created this ite ### RED phase result -Focused matrix, gateway package: **82 failing / 16 passing (98 cases, 6 files)**, terminal +Focused matrix, gateway package: **83 failing / 17 passing (100 cases, 6 files)**, terminal `Errors 0`. Typecheck, lint and Prettier clean. Every failure is a behavioral assertion that names its own guard — the matrix output was scanned specifically for `TypeError`, `ReferenceError`, missing-export and module-resolution text, not merely counted. @@ -456,6 +456,34 @@ it — `process.exit` is stubbed non-throwing for the duration of the import and settle turn — so the behavioral red is unchanged and the run reports `Errors 0`. The claim above is a measurement, not an inference. +**The child-process fixture is an allowlist, and that is what makes the two package-route cases +mean anything.** It was a denylist: copy the whole ambient `process.env`, delete three keys. That +left the real `HOME`, every XDG root, `MOSAIC_GATEWAY_HOME`, service endpoints, the auth and +credential families, the Node loader controls, and an ambient `PATH` in place. Two failure modes +follow, and both are worse than having no case at all. A faulty GREEN could read real host state +before any assertion caught it. And a refusal could be produced by the host's own configuration +rather than by the route, so the case would pass while proving nothing about the code under test. +A denylist is bounded only by the enumeration of what is known to be dangerous, which is open; +an allowlist is bounded by what the route actually needs. The child now receives exactly eight +variables — a private `HOME` under a per-run temporary sandbox, all five XDG roots and `TMPDIR` +beneath it, and a `PATH` built from the two workspace `.bin` directories and the directory of the +running Node binary — plus whatever the individual case declares. Nothing is inherited. The hard +timeout and `SIGKILL` are unchanged; a timeout bounds how long a leak can run, it does not prevent +one. + +Two cases hold that property rather than assuming it. `fixture control: the child environment is +exactly the allowlist plus the declarations` asserts the constructed key set, that the child's +`HOME` is neither the real home nor an ancestor of it, that no allowlisted value points into the +real Mosaic config root, and that `PATH` is the three intended directories. It is green by +construction and is a harness negative control, not coverage — its purpose is to fail the moment +someone widens the fixture back toward `...process.env`, a change that would otherwise silently +un-verify both package-route cases without reddening either. `does not leak the parent ambient +environment into the child` is the executed half: the parent plants a complete, valid local +declaration **and** a forbidden DSN in its own environment, then runs the route with nothing +passed in. A leaky fixture would hand the child a valid declaration, so the undeclared refusal +would not fire and the DSN refusal would fire instead — which is the discriminator the case +asserts on. + The matrix leaves **zero `/tmp/kbn-02a-*` residue**, measured by removing all matching roots before the run and counting after it. Every `mkdtemp` site in the six files registers its root for teardown at the point of creation, including the child-process route's root, which is created @@ -503,33 +531,35 @@ satisfied. The sixteen passing cases are itemized rather than counted as coverage, because a green that cannot fail for its stated reason is the same defect class as the span gap that created this item: -| Passing case | Why it is green | -| ------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------- | -| env: config read on import, no policy | legacy regression guard — expected green | -| env: planted dotenv loads, no policy | legacy regression guard — expected green | -| main: anchored config reaches `detectAndAssertTier` | current-contract regression guard; its import-time-exit premise is superseded and must migrate at GREEN | -| local-dev: production `dev` script untouched | regression guard — expected green | -| local-dev: no static import of `main.js` | structural, three forms (binding, side-effect, `require`); must stay green | -| local-dev: importing starts nothing | structural; green today only because nothing invokes `runLocalDev` | -| policy: refuses to serve when unestablished | scaffolding implements this correctly already | -| policy: no fallback read when unestablished | scaffolding implements this correctly already | -| inventory: retains no raw DSN text | **vacuous today** — the stub returns an empty inventory | -| inventory: retains no planted canary | **vacuous today** — same empty inventory | -| inventory: writes no planted canary to the console | **vacuous today** — the stub reads no source, so it can log none | -| policy: children beneath the state root | **vacuous today** — the stub concatenates the declared root | -| policy: unaffected by post-establish input mutation | **vacuous today** — the stub captured strings, it did not clone | -| policy: frozen policy survives a source file gaining a DSN | **vacuous today** — the stub re-serves the same object it built | -| policy: frozen policy survives the environment gaining a DSN | **vacuous today** — same | -| database: sterile local lifecycle migrates, no Postgres seam | positive anchor — green today, must stay green; the one case that fails if GREEN makes "safe" mean "inert" | +| Passing case | Why it is green | +| --------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------- | +| env: config read on import, no policy | legacy regression guard — expected green | +| env: planted dotenv loads, no policy | legacy regression guard — expected green | +| main: anchored config reaches `detectAndAssertTier` | current-contract regression guard; its import-time-exit premise is superseded and must migrate at GREEN | +| local-dev: production `dev` script untouched | regression guard — expected green | +| local-dev: no static import of `main.js` | structural, three forms (binding, side-effect, `require`); must stay green | +| local-dev: importing starts nothing | structural; green today only because nothing invokes `runLocalDev` | +| policy: refuses to serve when unestablished | scaffolding implements this correctly already | +| policy: no fallback read when unestablished | scaffolding implements this correctly already | +| inventory: retains no raw DSN text | **vacuous today** — the stub returns an empty inventory | +| inventory: retains no planted canary | **vacuous today** — same empty inventory | +| inventory: writes no planted canary to the console | **vacuous today** — the stub reads no source, so it can log none | +| policy: children beneath the state root | **vacuous today** — the stub concatenates the declared root | +| policy: unaffected by post-establish input mutation | **vacuous today** — the stub captured strings, it did not clone | +| policy: frozen policy survives a source file gaining a DSN | **vacuous today** — the stub re-serves the same object it built | +| policy: frozen policy survives the environment gaining a DSN | **vacuous today** — same | +| database: sterile local lifecycle migrates, no Postgres seam | positive anchor — green today, must stay green; the one case that fails if GREEN makes "safe" mean "inert" | +| local-dev: fixture control — child env is exactly the allowlist | harness negative control, not coverage — green by construction; guards the instrument the two package-route cases depend on | Seven greens are vacuous until the implementation exists, up from three, because an earlier pass added retention and post-freeze-mutation cases that a stub satisfies trivially. They are evidence of nothing today and are re-checked at GREEN. The count rising is the honest outcome of asserting more about a module that does not exist yet — it is recorded here so it cannot later be read as -coverage. Of the nine non-vacuous greens, seven are guards that must stay green (two legacy -continuity, the untouched `dev` script, the structural no-static-import, the two unestablished- -policy refusals, and the database positive anchor) and two are green for a weaker reason than they -read: `local-dev` "starts nothing on import" is green only because nothing yet invokes +coverage. Of the ten non-vacuous greens, one is the harness negative control above — it asserts +nothing about the code under test and is never counted as coverage — seven are guards that must +stay green (two legacy continuity, the untouched `dev` script, the structural no-static-import, +the two unestablished-policy refusals, and the database positive anchor) and two are green for a +weaker reason than they read: `local-dev` "starts nothing on import" is green only because nothing yet invokes `runLocalDev`, and the `main.ts` anchoring case guards a current contract whose import-time-exit premise `-02a` inverts, so it must be migrated at GREEN rather than deleted.