diff --git a/apps/gateway/src/local-dev.spec.ts b/apps/gateway/src/local-dev.spec.ts index fe93285d7..75f3b6915 100644 --- a/apps/gateway/src/local-dev.spec.ts +++ b/apps/gateway/src/local-dev.spec.ts @@ -11,11 +11,12 @@ */ import { spawnSync } from 'node:child_process'; -import { mkdirSync, mkdtempSync, readFileSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, statSync } from 'node:fs'; import { readFile, mkdtemp, realpath, rm } from 'node:fs/promises'; import { homedir, tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { types as utilTypes } from 'node:util'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type * as LocalDevNamespace from './local-dev.js'; @@ -76,14 +77,13 @@ let envSnapshot: NodeJS.ProcessEnv; * Every temp root this file creates, removed after the case that made it. * * Both the in-process beds and the child-process route beds register here. The - * child route's root is created synchronously inside the spawn arguments, which - * is precisely the placement most likely to be missed — it is not next to any - * other setup. + * child route's sandbox is created synchronously in `spawnHermetic`, immediately + * before the child is spawned. */ const tempRoots: string[] = []; beforeEach(() => { - envSnapshot = { ...process.env }; + envSnapshot = Object.assign({}, process.env); vi.clearAllMocks(); }); @@ -283,10 +283,12 @@ describe('KBN-101-02a local-dev — the channel has its own command', () => { * started nothing and refused nothing. A route that silently succeeds is worse * than one that is missing, because the operator's shell says it worked. * - * Safety of running the real route in-band: the child is given no - * BETTER_AUTH_SECRET and no DATABASE_URL. If a future GREEN wrongly reached - * `main.ts`, `bootstrap()` throws on the missing secret at its first statement, - * before `loadConfig()` and before `detectAndAssertTier()` — so no configuration + * Safety of running the real route in-band: the child inherits nothing and + * receives only the base allowlist plus names a case explicitly declares. No + * case declares BETTER_AUTH_SECRET; the DSN route case deliberately declares + * DATABASE_URL. If a future GREEN wrongly reached `main.ts`, `bootstrap()` + * constructs its logger, then throws on the missing secret before `loadConfig()` + * and before `detectAndAssertTier()` — so no configuration * 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. @@ -310,20 +312,96 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo 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. + * The POSIX shell the packaged route is executed through, as a validated + * absolute path. + * + * A literal `'sh'` resolved here only because this host keeps Node and the + * shell in the same directory, so the constructed `PATH` happened to contain + * both. That is a coincidence of layout, not a property of the fixture: on a + * host where Node lives elsewhere the constructed `PATH` would not reach a + * shell at all, and widening `PATH` to find one would reopen the executable- + * resolution hole the allowlist exists to close. + */ + function resolveTrustedShell(): string { + for (const candidate of ['/bin/sh', '/usr/bin/sh']) { + if (existsSync(candidate) && statSync(candidate).isFile()) { + return candidate; + } + } + throw new Error('no trusted POSIX shell found at /bin/sh or /usr/bin/sh'); + } + + const TRUSTED_SHELL = resolveTrustedShell(); + + /** + * Every variable name the child is permitted to receive, before the per-case + * declarations are layered on. + */ + const ALLOWED_CHILD_ENV = Object.freeze([ + 'HOME', + 'PATH', + 'TMPDIR', + 'XDG_CACHE_HOME', + 'XDG_CONFIG_DIRS', + 'XDG_CONFIG_HOME', + 'XDG_DATA_DIRS', + 'XDG_DATA_HOME', + 'XDG_RUNTIME_DIR', + 'XDG_STATE_HOME', + ]); + + /** + * Names a POSIX shell sets for itself. They are derived from the invocation — + * `PWD` from `cwd`, `SHLVL` from a counter, `_` from the last word of the + * command — not inherited from this process, so they carry no host state. They + * are enumerated rather than tolerated: an unexpected name appearing in a + * shell-mediated child is a leak, and the only way to say that is to say which + * names are not. + */ + const SHELL_GENERATED_ENV = Object.freeze(['IFS', 'OLDPWD', 'PWD', 'SHLVL', '_']); + + /** + * The exact route inputs cases may add to the child environment. Adding a new + * name is a deliberate change to this list, not an open-ended prefix match. + */ + const DECLARABLE_CASE_ENV_NAMES = Object.freeze([ + 'MOSAIC_LOCAL_DEV_TIER', + 'MOSAIC_LOCAL_DEV_STORAGE', + 'MOSAIC_LOCAL_DEV_QUEUE', + 'MOSAIC_LOCAL_DEV_STATE_ROOT', + 'DATABASE_URL', + ]); + + /** + * The complete base environment for one child. Built from nothing; nothing is + * inherited. A case may add only the explicitly declarable route inputs. * * `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. + * directories the package route resolves `tsx` from, the directory holding the + * Node binary already running this suite, and the trusted shell's own + * directory (which is what supplies the coreutils a `#!/usr/bin/env node` + * shebang needs). A host `PATH` would let an unrelated executable of the same + * name answer, which is the executable-resolution half of the same problem. + * + * `XDG_CONFIG_DIRS` and `XDG_DATA_DIRS` are set to private empty directories + * rather than left unset. Unset is not neutral for the XDG directory *lists*: + * they default to `/etc/xdg` and `/usr/local/share:/usr/share`, so leaving + * them out would have left two real system config/data roots readable while + * the eight-key set claimed full isolation. Empty *strings* would not do + * either — the specification says a set-but-empty list falls back to the same + * defaults, so the value has to be a real directory that happens to be empty. */ function buildHermeticChildEnv( sandbox: string, extraEnv: Record = {}, ): Record { const home = join(sandbox, 'home'); + const pathEntries = [ + join(GATEWAY_DIR, 'node_modules', '.bin'), + join(REPO_ROOT, 'node_modules', '.bin'), + dirname(process.execPath), + dirname(TRUSTED_SHELL), + ]; const base: Record = { HOME: home, // Every XDG root, not only the config one. Leaving any of them ambient @@ -333,12 +411,10 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo XDG_STATE_HOME: join(home, '.local', 'state'), XDG_CACHE_HOME: join(home, '.cache'), XDG_RUNTIME_DIR: join(sandbox, 'run'), + XDG_CONFIG_DIRS: join(sandbox, 'xdg', 'config-dirs'), + XDG_DATA_DIRS: join(sandbox, 'xdg', 'data-dirs'), TMPDIR: join(sandbox, 'tmp'), - PATH: [ - join(GATEWAY_DIR, 'node_modules', '.bin'), - join(REPO_ROOT, 'node_modules', '.bin'), - dirname(process.execPath), - ].join(':'), + PATH: [...new Set(pathEntries)].join(':'), }; return { ...base, ...extraEnv }; } @@ -354,12 +430,124 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo join('home', '.cache'), 'run', 'tmp', + join('xdg', 'config-dirs'), + join('xdg', 'data-dirs'), ]) { mkdirSync(join(sandbox, relative), { recursive: true }); } return sandbox; } + /** + * Enumerates a case's declarations once, then validates and snapshots that + * same entry array. The single enumeration removes the double-read that two + * earlier versions had; the exact-name and string-value checks reject ambient + * copies and malformed declarations. A case that patches the globals this + * helper calls, or that writes `process.execPath`, can still make the checked + * value differ from the used value, and nothing here detects that. These cases + * share a closure with the guard, so the guard is maintainer discipline + * enforced mechanically, not a sandbox. `isProxy` is secondary defense. + */ + function snapshotCaseEnv(extraEnv: Record): Record { + const inputIsProxy = utilTypes.isProxy(extraEnv); + const entries = Object.entries(extraEnv); + const snapshot = Object.fromEntries(entries); + expect( + inputIsProxy, + 'extraEnv must not be a Proxy; use a plain record so its validated snapshot is stable', + ).toBe(false); + const offendingKeys = entries + .map(([key]) => key) + .filter((key) => !DECLARABLE_CASE_ENV_NAMES.includes(key)); + expect( + offendingKeys, + `extraEnv contains undeclarable keys (${offendingKeys.join(', ')}); accepted names are exactly: ${DECLARABLE_CASE_ENV_NAMES.join(', ')}`, + ).toEqual([]); + const nonStringKeys = entries + .filter(([, value]) => typeof value !== 'string') + .map(([key]) => key); + expect( + nonStringKeys, + `extraEnv must contain only string values; non-string keys: ${nonStringKeys.join(', ')}`, + ).toEqual([]); + return snapshot; + } + + /** + * The ONE place a child process is created in this file, and the only place + * its environment is chosen. + * + * That is the point, not tidiness. A control that exercised its own spawn + * would leave the route's spawn free to be reverted to an ambient copy without + * reddening anything, while real host state leaked alongside the expected + * refusal. The single site keeps both arms on one environment builder, and the + * single-enumeration validation of per-case inputs against an exact name list + * rejects an ambient copy passed as a declaration, which is the mistake a + * maintainer actually makes. + */ + function spawnHermetic( + command: string, + args: string[], + extraEnv: Record = {}, + ): { + status: number | null; + signal: NodeJS.Signals | null; + output: string; + sandbox: string; + } { + const extraEnvSnapshot = snapshotCaseEnv(extraEnv); + const sandbox = makeChildSandbox(); + const result = spawnSync(command, args, { + cwd: GATEWAY_DIR, + env: buildHermeticChildEnv(sandbox, extraEnvSnapshot), + encoding: 'utf8', + timeout: CHILD_TIMEOUT_MS, + killSignal: 'SIGKILL', + }); + + return { + status: result.status, + signal: result.signal, + output: `${result.stdout ?? ''}${result.stderr ?? ''}`, + sandbox, + }; + } + + /** A child that reports the environment it was actually given, and exits. */ + const ENV_MARKER = 'KBN02A_ENV:'; + const ENV_MARKER_SCRIPT = `process.stdout.write(${JSON.stringify(ENV_MARKER)} + JSON.stringify(process.env));`; + + function hasOnlyStringValues(value: object): value is Record { + return Object.values(value).every((entry) => typeof entry === 'string'); + } + + function parseObservedEnv(output: string): Record { + const at = output.indexOf(ENV_MARKER); + expect(at, `the marker child printed no environment: ${output}`).toBeGreaterThanOrEqual(0); + + const parsed: unknown = JSON.parse(output.slice(at + ENV_MARKER.length)); + const isRecord = typeof parsed === 'object' && parsed !== null && !Array.isArray(parsed); + expect(isRecord, 'the marker child environment must be a non-null, non-array object').toBe( + true, + ); + if (!isRecord) { + return {}; + } + + const nonStringKeys = Object.entries(parsed) + .filter(([, value]) => typeof value !== 'string') + .map(([key]) => key); + expect( + nonStringKeys, + `the marker child environment must contain only string values; non-string keys: ${nonStringKeys.join(', ')}`, + ).toEqual([]); + if (!hasOnlyStringValues(parsed)) { + return {}; + } + + return parsed; + } + function runDevLocal(extraEnv: Record = {}): { status: number | null; signal: NodeJS.Signals | null; @@ -369,21 +557,11 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo scripts?: Record; }; const script = manifest.scripts?.['dev:local']; - expect(script, 'no dev:local script to execute').toBeDefined(); + if (typeof script !== 'string') { + expect.fail('the package manifest has no dev:local script yet'); + } - const result = spawnSync('sh', ['-c', script as string], { - cwd: GATEWAY_DIR, - env: buildHermeticChildEnv(makeChildSandbox(), extraEnv), - encoding: 'utf8', - timeout: CHILD_TIMEOUT_MS, - killSignal: 'SIGKILL', - }); - - return { - status: result.status, - signal: result.signal, - output: `${result.stdout ?? ''}${result.stderr ?? ''}`, - }; + return spawnHermetic(TRUSTED_SHELL, ['-c', script], extraEnv); } it('exits nonzero with a refusal code when nothing is declared', () => { @@ -393,8 +571,9 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo expect(status, 'the route exited 0 with nothing declared').not.toBe(0); // Nonzero alone would be satisfied by a crash, a missing dependency, or the // BETTER_AUTH_SECRET throw inside bootstrap — all of which mean the refusal - // did NOT happen and the start was merely broken. - expect(output).toMatch(/LOCAL_START_[A-Z_]+/); + // did NOT happen and the start was merely broken. The code is named exactly: + // any LOCAL_START_* would let an unrelated refusal stand in for this one. + expect(output).toContain('LOCAL_START_TIER_NOT_EXPLICIT'); expect(output, 'exited via bootstrap rather than refusing before it').not.toContain( 'BETTER_AUTH_SECRET', ); @@ -418,63 +597,139 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo }); /** - * 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. + * A harness control, and the only case in this describe that runs a child at + * RED head. It asserts nothing about the code under test: it asserts that the + * spawn site the route arm depends on delivers the allowlist, measured from + * inside a real child rather than from the object handed to the spawn site. + * + * Measuring the object would not have been enough. Reverting only the route's + * spawn to an ambient copy leaves an assertion on the builder green, and that + * revert is invisible from the outside — the ambient copy strips the planted + * declarations, so the canary below still sees the refusal it expects while + * real host state leaks alongside it. Both observations here go through the + * same single spawn site the route goes through, so no such divergence exists + * to be measured. */ - 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', - ]; + it('fixture control: the spawn site delivers exactly the allowlist to a real child', () => { + // Source-text heuristics guard the named import, direct call count, and + // ambient-copy spelling. They cannot see a child created through an + // indirection that this source does not name, so this is not a proof that + // every possible child-process path is covered. + const ownSource = readFileSync(fileURLToPath(import.meta.url), 'utf8'); + const spawnIdentifier = ['spawn', 'Sync'].join(''); + const childProcessModule = ['node', 'child_process'].join(':'); + expect( + ownSource.match(new RegExp(childProcessModule, 'g'))?.length ?? 0, + 'the child-process module must appear as exactly one import source', + ).toBe(1); + const childProcessImports = + ownSource.match(new RegExp(`^import[^\\n]*['"]${childProcessModule}['"];?$`, 'gm')) ?? []; + expect( + childProcessImports, + 'the child-process module must be imported exactly once, using only its named synchronous spawn', + ).toEqual([`import { ${spawnIdentifier} } from '${childProcessModule}';`]); + expect( + ownSource.match(new RegExp(`\\b${spawnIdentifier}\\b`, 'g'))?.length ?? 0, + 'the synchronous spawn identifier must appear only in its import and single call site', + ).toBe(2); + expect( + /\.\.\.process\.env/.test(ownSource), + 'a child environment is being built by copying the ambient environment', + ).toBe(false); - const sandbox = makeChildSandbox(); - const bare = buildHermeticChildEnv(sandbox); - expect(Object.keys(bare).sort()).toEqual(ALLOWED); + // Executed directly: no shell, so the child's environment is the builder's + // set exactly. + const direct = spawnHermetic(process.execPath, ['-e', ENV_MARKER_SCRIPT]); + expect(direct.signal, 'the marker child hung').toBeNull(); + expect(direct.status, `the marker child failed: ${direct.output}`).toBe(0); - 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(), - ); + const observed = parseObservedEnv(direct.output); + expect(Object.keys(observed).sort()).toEqual(ALLOWED_CHILD_ENV); - // 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)) { + // The real home and the real Mosaic config root are unreachable. + expect(observed['HOME']).not.toBe(homedir()); + expect(observed['HOME']?.startsWith(direct.sandbox)).toBe(true); + expect(homedir().startsWith(observed['HOME'] as string)).toBe(false); + for (const value of Object.values(observed)) { 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 XDG directory lists name private empty directories. An unset list + // falls back to system roots, while a non-empty directory can expose + // prepopulated state, so either condition would defeat the isolation. + for (const key of ['XDG_CONFIG_DIRS', 'XDG_DATA_DIRS']) { + const value = observed[key]; + const isolationReason = `${key} must name a private empty directory because an unset list or a non-empty directory would expose external state and defeat isolation`; + expect(value?.startsWith(direct.sandbox), isolationReason).toBe(true); + expect(value, isolationReason).toBeDefined(); + if (value === undefined) { + continue; + } + expect(existsSync(value), isolationReason).toBe(true); + expect(statSync(value).isDirectory(), isolationReason).toBe(true); + expect(readdirSync(value), isolationReason).toEqual([]); + } + + // PATH is the intended directories, in order, and nothing else. + expect(observed['PATH']?.split(':')).toEqual([ + ...new Set([ + join(GATEWAY_DIR, 'node_modules', '.bin'), + join(REPO_ROOT, 'node_modules', '.bin'), + dirname(process.execPath), + dirname(TRUSTED_SHELL), + ]), ]); + + // Executed through the trusted shell, which is the route arm's own shape. + // The shell adds names of its own; the claim is not that the final process + // receives the builder's set exactly, it is that every difference is the + // shell's own bookkeeping and none of it is inherited. + const viaShell = spawnHermetic(TRUSTED_SHELL, [ + '-c', + `exec ${JSON.stringify(process.execPath)} -e ${JSON.stringify(ENV_MARKER_SCRIPT)}`, + ]); + expect(viaShell.signal, 'the shell-mediated marker child hung').toBeNull(); + expect(viaShell.status, `the shell-mediated marker child failed: ${viaShell.output}`).toBe(0); + + const shellObserved = parseObservedEnv(viaShell.output); + expect(ALLOWED_CHILD_ENV.filter((key) => !(key in shellObserved))).toEqual([]); + const expectedShellBase = buildHermeticChildEnv(viaShell.sandbox); + for (const key of ALLOWED_CHILD_ENV) { + expect( + shellObserved[key], + `the shell-mediated child changed the builder's value for ${key}`, + ).toBe(expectedShellBase[key]); + } + const added = Object.keys(shellObserved).filter((key) => !ALLOWED_CHILD_ENV.includes(key)); + expect( + added.filter((key) => !SHELL_GENERATED_ENV.includes(key)), + "the shell-mediated child received a variable that is not the shell's own bookkeeping", + ).toEqual([]); + for (const key of added) { + expect( + shellObserved[key], + `a shell-added value points at the real home: ${key}=${shellObserved[key]}`, + ).not.toContain(join(homedir(), '.config', 'mosaic')); + } }); /** - * 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. + * The route-arm half of the same obligation, and it is PENDING EXECUTION until + * GREEN. At RED head there is no `dev:local` script, so `runDevLocal` fails on + * the manifest assertion before any child is created — this case is red for + * that reason and its hermeticity claim is not yet measured by it. The control + * above is what is measured today; this is what will measure the route once + * the route exists. Saying otherwise would classify a case that never spawned + * as evidence about a subprocess, which is the same overclaim the vacuous + * greens are itemized to prevent. + * + * What it will then assert: 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 = { @@ -496,7 +751,9 @@ describe('KBN-101-02a local-dev — the package route refuses, not merely the mo 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_]+/); + // The exact code, not the family: an unrelated refusal would otherwise + // satisfy a canary whose whole claim is that THIS refusal fired. + expect(output).toContain('LOCAL_START_TIER_NOT_EXPLICIT'); // 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', 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 806014e8b..cddcdd4c8 100644 --- a/docs/native-kanban-sot/KBN-101-DB-ROLE-SPLIT.md +++ b/docs/native-kanban-sot/KBN-101-DB-ROLE-SPLIT.md @@ -442,10 +442,20 @@ its stated reason is the same defect class as the span gap that created this ite ### RED phase result -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. +Focused matrix, gateway package: **83 failing / 17 passing (100 cases, 6 files)**, and the terminal +summary carries **no `Errors` row at all**. Typecheck, lint and Prettier clean. Every failure is an +assertion that names what it wanted — the matrix output was scanned specifically for `TypeError`, +`ReferenceError`, missing-export and module-resolution text, not merely counted. Most of those +assertions are behavioral, but not all of them: the structural guards over this file's own source +text are assertions too, and they are counted in the same 83. The claim is that nothing failed by +accident, not that every failure describes runtime behavior. + +An earlier version of this section reported that figure as "terminal `Errors 0`", which is not a +thing vitest prints. It emits an `Errors` row **only** when an out-of-band error is attributed to +no case; at zero the row is absent. The measured claim is the absence of the row, and the wording +above says so. The distinction matters because "`Errors 0`" reads as an observed zero, and a reader +who went looking for it would find nothing and could not tell whether the run was clean or the +claim was invented. **Correction to the previous report's error claim.** It said "zero non-assertion failures", and that was wrong at the time it was written: the run also carried one out-of-band @@ -453,10 +463,10 @@ that was wrong at the time it was written: the run also carried one out-of-band rejecting after the importing case had already returned. Vitest attributes such an error to no case and warns it can produce false positives. It is now captured inside the case that provokes it — `process.exit` is stubbed non-throwing for the duration of the import and the case awaits a -settle turn — so the behavioral red is unchanged and the run reports `Errors 0`. The claim above -is a measurement, not an inference. +settle turn — so the behavioral red is unchanged and the run no longer emits the row. 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 +**The child-process fixture is an allowlist, and that is what makes the three 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 @@ -464,31 +474,146 @@ follow, and both are worse than having no case at all. A faulty GREEN could read 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. +an allowlist is bounded by what the route actually needs. The child now receives exactly ten +variables — a private `HOME` under a per-run temporary sandbox, the four `XDG_*_HOME` roots and +`XDG_RUNTIME_DIR` plus both XDG _directory lists_ and `TMPDIR` beneath it, and an explicit +`PATH` — 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. +Three details in that set are not obvious, and each was a hole before it was closed. + +`XDG_CONFIG_DIRS` and `XDG_DATA_DIRS` are set to private empty directories rather than omitted. +Omission is not neutral for the XDG _lists_: unset, they mean `/etc/xdg` and +`/usr/local/share:/usr/share`, so an eight-key allowlist that left them out still exposed two real +system config and data roots while claiming full isolation. Setting them to the empty _string_ is +no better — the specification says a set-but-empty list falls back to the same defaults — so the +value has to be a real directory that happens to be empty. + +The shell is resolved to a validated absolute path (`/bin/sh` or `/usr/bin/sh`, checked to exist +and to be a file) instead of the literal `sh`. The literal resolved only because this host keeps +Node and the shell in the same directory, so the constructed `PATH` happened to contain both. That +is a property of the host's layout, not of the fixture; where Node lives elsewhere the constructed +`PATH` would reach no shell at all, and widening `PATH` to find one would reopen the +executable-resolution hole the allowlist exists to close. `PATH` itself is the two workspace `.bin` +directories, the directory of the running Node binary, and the trusted shell's own directory, +deduplicated. + +The claim is **not** that the final route process receives the builder's key set exactly. The route +is executed through that shell, and a POSIX shell sets some of its own bookkeeping names. The +permitted set is five — `IFS`, `OLDPWD`, `PWD`, `SHLVL`, `_` — and in the observed runs the shell +added exactly two of them, `PWD` and `SHLVL`. The permitted list is deliberately wider than the +observed additions, because which of the five a given `/bin/sh` sets is a property of that shell, +not of this fixture; the assertion requires only that any addition be drawn from the five. Each is +derived from the invocation — `PWD` from `cwd`, `SHLVL` from a counter, `_` from the last word of +the command — so none carries host state, but they are additions and the document says so rather +than rounding them away. The measured claim is: the shell-mediated child receives every allowlisted +name with the value the builder constructed for it, and every name beyond the allowlist is one of +those five. + +**There is exactly one place in the file where a child process is created**, and that is the +mechanism, not tidiness. A control that made its own spawn would leave the route's spawn free to be +reverted to an ambient copy while the control stayed green — and that revert is invisible from the +outside, because an ambient copy also strips the planted declarations, so the canary still gets the +refusal it expects while real host state leaks alongside it. + +**One site is necessary and was not sufficient, and the gap was a check/use gap.** The first +version of this argument said the control and the route received the same environment _by +construction_, because there was only one place an environment could be built. That was too strong, +and the counterexample is exact rather than theoretical: the site validated the caller's per-case +object and then spread it, which enumerated that object **twice**. A `Proxy` returning one legal +name to the validating enumeration and the whole ambient environment to the spread passes the guard +and reaches the child — verified in a standalone probe, where validation reported no offending keys +while the merge copied ambient names and overrode `HOME`. One spawn site constrains _where_ an +environment is built; it says nothing about _what_ the caller may hand that site. + +The first repair of that was itself too weak, and the second counterexample is worth recording +because it is the same defect wearing different clothes. That repair snapshotted the caller's +object and then validated the snapshot with a fresh `Object.keys` call — two calls again. A +**non-Proxy** object defeats it: a getter on a declarable key replaces the global `Object.keys` +while `Object.entries` is still collecting, so the forbidden names land in the snapshot and the +later lookup reports none. Also verified by probe. Detecting `Proxy` did not help, because the +input was not one. + +What holds it now is that the site enumerates the caller's object exactly **once**, into an entries +array, and derives **both** the validated name list and the merged object from that same array. The +caller's object is never read again, so neither of the two exploits above works: there is no second +read of caller-controlled data for an accessor to sit between. The snapshot's keys must be drawn +from an exact five-name list — the four the route reads plus `DATABASE_URL` — not an open +`MOSAIC_LOCAL_DEV_*` prefix, because a prefix is wide enough to carry host variables the route +never reads; and its values must all be strings, so a getter cannot place a non-string in the child +environment. The list is frozen, so a case cannot extend it before declaring. Adding a sixth name +is an edit to that list. A `Proxy` is additionally rejected outright, but that is secondary defense +and is documented as such. + +**The third statement of this claim was also too strong, and the bound is now stated instead of +narrowed again.** An earlier version of this paragraph said no second call to any replaceable +global stands between the check and the use, so the check and the use could not describe different +values whatever the caller did. That is false. The helper calls `Object.fromEntries`, `map` and +`filter` after the enumeration, `buildHermeticChildEnv` later reads the writable `process.execPath` +and calls `Set` — and a getter firing during the single enumeration can replace any of them. The +guard is therefore not a sandbox and cannot become one at this layer, because the cases share a +closure with it: an author who can patch a global can also patch the guard. What the guard actually +does is reject an ambient copy or a stray name **passed as a declaration**, which is the mistake a +maintainer makes by accident. That is worth having, and it is all that is claimed. Three +consecutive repairs tried to make it more than that; the third one added a callback to the spawn +site and had to be reverted, which is the evidence that the escalation was the wrong move rather +than an incomplete one. + +Both exploits above were re-run against the final guard and neither reaches a child. They are +rejected by different assertions, which is worth stating precisely because an earlier version of +this paragraph claimed both were named by the offending-keys message: the ambient copy is, and the +`Proxy` is stopped one assertion earlier by the `isProxy` check, whose message names no keys at +all. That proof runs through the spawn site **directly**, not through the package route, because at +RED head the absent `dev:local` script legitimately stops the route arm before it can spawn. An intermediate version reordered that — it added a `beforeSpawn` +callback to the spawn site so route setup assertions ran _after_ the input guard, purely so the +mutation failed on the guard rather than on the missing script. That was removed. It bought nothing +but a more legible proof, and it cost a caller-supplied function executing inside the one guarded +site, plus an `as string` cast that put `undefined` in the argument array and was safe only because +the callback happened to throw first. How a proof reads is not a property of the artifact, and the +mechanism this whole item rests on is the last place to accept new surface for it. + +A structural assertion guards the shape the argument depends on: the spec asserts against its own +source that `node:child_process` is imported exactly once as the named form `{ spawnSync }`, that +the `spawnSync` identifier occurs exactly twice (its import and its one call), and that +`...process.env` occurs nowhere. That is aimed at the one revert behavioral assertions cannot see, +since a behavioral case only ever observes the spawn it makes itself. It is a source-text +heuristic and is documented as one: a child created through an indirection it does not name would +not redden it. It is stated here as a guard, not as a proof. + +`fixture control: the spawn site delivers exactly the allowlist to a real child` is the only case +in the package-route arm that runs a child at RED head, and it measures the environment **from +inside the child** rather than from the object handed to `spawnSync`. It spawns twice through that +one site: directly, where the observed key set must equal the allowlist exactly and the child's +`HOME` must be neither the real home nor an ancestor of it; and through the trusted shell, which is +the route's own shape, where every allowlisted name must be present, must still carry the value the +builder constructed for that spawn's own sandbox, and every added name must be one of the shell's +bookkeeping names. Presence alone was not enough: a shell-mediated child that changed `HOME`, +`PATH` or an XDG root would keep every name and stay green. It is green by construction and is a **harness control, not +coverage** — it asserts nothing about the code under test. Its purpose is to redden if the fixture +is widened back toward the ambient environment. + +`does not leak the parent ambient environment into the child` is the route-arm half, and it is +**pending execution until GREEN**. At RED head there is no `dev:local` script, so the manifest +assertion fires before any child is created: the case is red for that reason, and its hermeticity +claim is not yet measured by it. Recording it as an executed proof would classify a case that never +spawned as evidence about a subprocess, which is the overclaim the vacuous-green itemization exists +to prevent. What it will assert once the route exists: the parent plants a complete, valid local +declaration **and** a forbidden DSN in its own environment, then runs the route with nothing passed +in, and requires `LOCAL_START_TIER_NOT_EXPLICIT` specifically. A leaky fixture would hand the child +a valid declaration, so that refusal would not fire and the DSN refusal would fire instead — the +discriminator. Both route-arm cases name the exact refusal code rather than matching +`/LOCAL_START_[A-Z_]+/`; the family pattern let an unrelated refusal stand in for the one under +test, which is the same defect as asserting a raw source value against a classification union. 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 -inside the spawn arguments and was the one most easily missed. The earlier passes leaked one -directory per case. +before the run and counting after it. Every `mkdtemp` site in `local-dev.spec.ts` registers its +root for teardown at the point of creation, including the child sandbox, which is created in +`spawnHermetic` immediately before the spawn call and was the one most easily missed. One root +elsewhere in the matrix — `main.spec.ts:58`, outside the `kbn-02a-` prefix the residue count +measures — is removed by its own `finally` rather than by registration; that is a second cleanup +path, not a leak, and it is named here so the sentence is not read as covering more than it does. +The earlier passes leaked one directory per case. Ten of the sixteen fenced paths are touched in RED. The six GREEN targets (`package.json`, `env.ts`, `app.module.ts`, `config/config.module.ts`, `database/database.module.ts`, `main.ts`) @@ -507,9 +632,12 @@ only against scaffolding. This pass closes the first gap and narrows the second: - `database.module.ts` now has REDs that drive the real `onModuleInit()`, asserting that migration and adapter-migrate never run (bars Z3 and Z4) — still through constructor substitution, not through a full application boot. -- `package.json` now has a RED that executes the packaged `dev:local` route as a child process, - requiring a `LOCAL_START_*` code in its output and the absence of the `BETTER_AUTH_SECRET` - throw, so a crash or an unrelated failure cannot satisfy it. +- `package.json` now has a RED that _will_ execute the packaged `dev:local` route as a child + process, requiring a `LOCAL_START_*` code in its output and the absence of the + `BETTER_AUTH_SECRET` throw, so a crash or an unrelated failure cannot satisfy it. At RED head it + does not get that far: the script does not exist yet, so the case stops on the missing manifest + entry before any child is created. It is red for the right reason, and the child-process + behavior it specifies is unmeasured until GREEN adds the script. - `env.ts` remains covered only by legacy-continuity regression guards, by design. The bars are therefore proven through the actual entrypoint for the probe and application-create @@ -528,34 +656,34 @@ The second is green today and must stay green; it replaced a case that awaited `onModuleInit().catch(() => undefined)` and asserted only an absence, which a total failure satisfied. -The sixteen passing cases are itemized rather than counted as coverage, because a green that +The seventeen 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" | -| 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 | +| 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 — spawn site delivers the allowlist | harness control, not coverage — green by construction; the only case that runs a child at RED head, and the guard on the instrument the other three 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 ten non-vacuous greens, one is the harness negative control above — it asserts +coverage. Of the ten non-vacuous greens, one is the harness 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