From 982f4fc8f9d6a9b75131cd013c928a0f4b490718 Mon Sep 17 00:00:00 2001 From: fred Date: Sat, 15 Aug 2026 13:28:39 -0500 Subject: [PATCH] test(lease): stop the host's lease identity leaking into spawned hooks Four cases in mutator-gate.acceptance.spec.ts fail whenever the suite runs inside a Mosaic-managed agent seat, and pass everywhere else. They are not a product defect: the gate denies in all four, it just denies for the wrong reason. The specs build each spawned hook's environment as `{ ...process.env, }`. Inside a managed seat, process.env already carries that seat's live lease identity, and whatever the spread does not override survives into the child. MOSAIC_LEASE_GENERATION_FILE is the one that bites: read_runtime_generation() prefers that file over MOSAIC_RUNTIME_GENERATION, so a case that carefully sets MOSAIC_RUNTIME_GENERATION=1 is silently overruled by the host's generation counter -- 388 on the seat this was found on. The revoke client reads 388 and sends it, the test broker advances the session to that generation, and every later authorize() in the case sends generation 1, is now behind, and is denied STALE_GENERATION instead of the asserted MUTATOR_UNVERIFIED. The fourth failure (runtime gate status 2 rather than 0) is the same cause. Fixed once, centrally: a vitest setupFile scrubs the host lease variables from process.env before any spec in the package runs, so all twelve spread sites are covered and a thirteenth cannot reintroduce it. Scrubbing is by prefix rather than by an explicit list, because a lease variable added later leaks by exactly the same route and a list would have to be remembered. Measured, not assumed. Red on origin/next: 4 failed / 16 passed. The same spec re-run with only those five variables stripped and no code change: 20/20. With this commit: typecheck clean, package build clean, 1537 tests passed across 85 files, 0 failed. Falsified by unregistering the setup file, which turns the wiring assertion red. The eslint change is mechanical: the project service needs root-level config files listed in allowDefaultProject, which already carries the sibling packages/mosaic/vitest.config.ts. Worth recording why it lasted: on a clean checkout and in CI these variables are unset, so the suite is green and the leak is invisible. It only reproduces in the one environment nobody runs the suite in. --- eslint.config.mjs | 1 + .../src/test-support/host-lease-env.spec.ts | 64 +++++++++++++++++++ .../mosaic/src/test-support/host-lease-env.ts | 41 ++++++++++++ packages/mosaic/vitest.config.ts | 1 + packages/mosaic/vitest.setup.ts | 5 ++ 5 files changed, 112 insertions(+) create mode 100644 packages/mosaic/src/test-support/host-lease-env.spec.ts create mode 100644 packages/mosaic/src/test-support/host-lease-env.ts create mode 100644 packages/mosaic/vitest.setup.ts diff --git a/eslint.config.mjs b/eslint.config.mjs index bcfe1995..f889463f 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -33,6 +33,7 @@ export default tseslint.config( 'packages/db/vitest.config.ts', 'packages/storage/vitest.config.ts', 'packages/mosaic/vitest.config.ts', + 'packages/mosaic/vitest.setup.ts', 'packages/mosaic/__tests__/*.ts', 'tools/federation-harness/*.ts', ], diff --git a/packages/mosaic/src/test-support/host-lease-env.spec.ts b/packages/mosaic/src/test-support/host-lease-env.spec.ts new file mode 100644 index 00000000..98c0d81b --- /dev/null +++ b/packages/mosaic/src/test-support/host-lease-env.spec.ts @@ -0,0 +1,64 @@ +import { describe, expect, test } from 'vitest'; + +import { isHostLeaseVariable, scrubHostLeaseEnv } from './host-lease-env.js'; + +describe('host lease environment scrubbing', () => { + test('removes every lease variable and reports what it removed', () => { + const environment = { + MOSAIC_LEASE_GENERATION_FILE: '/run/user/1001/mosaic-lease/generation-abc.state', + MOSAIC_LEASE_SESSION_ID: 'a'.repeat(64), + MOSAIC_LEASE_BROKER_SOCKET: '/run/user/1001/mosaic-lease/broker.sock', + MOSAIC_LEASE_RUNTIME: 'claude', + MOSAIC_RUNTIME_GENERATION: '388', + PATH: '/usr/bin', + MOSAIC_AGENT_NAME: 'fred', + } as NodeJS.ProcessEnv; + + expect(scrubHostLeaseEnv(environment)).toEqual([ + 'MOSAIC_LEASE_BROKER_SOCKET', + 'MOSAIC_LEASE_GENERATION_FILE', + 'MOSAIC_LEASE_RUNTIME', + 'MOSAIC_LEASE_SESSION_ID', + 'MOSAIC_RUNTIME_GENERATION', + ]); + expect(environment).toEqual({ PATH: '/usr/bin', MOSAIC_AGENT_NAME: 'fred' }); + }); + + test('a lease variable added later is scrubbed without being listed anywhere', () => { + // The prefix rule is the point: this is the case a hand-maintained list would miss. + const environment = { MOSAIC_LEASE_SOMETHING_NEW: 'x' } as NodeJS.ProcessEnv; + expect(scrubHostLeaseEnv(environment)).toEqual(['MOSAIC_LEASE_SOMETHING_NEW']); + expect(environment).toEqual({}); + }); + + test('leaves unrelated variables alone', () => { + const environment = { + MOSAIC_AGENT_NAME: 'fred', + MOSAIC_HOME: '/home/fred/.mosaic', + HOME: '/home/fred', + } as NodeJS.ProcessEnv; + + expect(scrubHostLeaseEnv(environment)).toEqual([]); + expect(environment).toEqual({ + MOSAIC_AGENT_NAME: 'fred', + MOSAIC_HOME: '/home/fred/.mosaic', + HOME: '/home/fred', + }); + }); + + test('classifies by prefix, not by an exact list', () => { + expect(isHostLeaseVariable('MOSAIC_LEASE_ANYTHING')).toBe(true); + expect(isHostLeaseVariable('MOSAIC_RUNTIME_GENERATION')).toBe(true); + expect(isHostLeaseVariable('MOSAIC_RUNTIME')).toBe(false); + expect(isHostLeaseVariable('LEASE_MOSAIC_X')).toBe(false); + }); + + // Wiring check. On a clean checkout or in CI these variables are unset, so this + // passes whether or not vitest.setup.ts is registered -- it is worth little there and + // is not claimed to be. Its value is inside a Mosaic-managed agent seat, where the + // variables ARE set and this is the assertion that catches the setup file being + // dropped from vitest.config.ts. That is the environment the leak was found in. + test('the suite does not run with the host lease identity in scope', () => { + expect(Object.keys(process.env).filter(isHostLeaseVariable)).toEqual([]); + }); +}); diff --git a/packages/mosaic/src/test-support/host-lease-env.ts b/packages/mosaic/src/test-support/host-lease-env.ts new file mode 100644 index 00000000..62e255aa --- /dev/null +++ b/packages/mosaic/src/test-support/host-lease-env.ts @@ -0,0 +1,41 @@ +/** + * Remove the host's live lease identity from an environment before tests run. + * + * The lease specs start their own broker on a private socket and then spawn the real + * hook scripts against it, building each child's environment as `{ ...process.env, }`. That spread is the problem: when the suite runs + * inside a Mosaic-managed agent seat, `process.env` already carries that seat's real + * lease identity, and the parts the spread does not override survive into the child. + * + * `MOSAIC_LEASE_GENERATION_FILE` is the one that bites. `read_runtime_generation()` + * prefers that file over `MOSAIC_RUNTIME_GENERATION`, so a case that carefully sets + * `MOSAIC_RUNTIME_GENERATION: '1'` is silently overruled by the host's generation + * counter -- which on a long-lived seat is in the hundreds. The revoke client reads it, + * sends it, and the test broker advances the session to that generation. Every later + * `authorize` in the case sends generation 1, is now behind, and is denied with + * `STALE_GENERATION` instead of the `MUTATOR_UNVERIFIED` the case asserts. The gate + * still denies, so this is not a hole in the product -- but it turns four acceptance + * tests red for a reason that has nothing to do with the code under test. + * + * It only reproduces inside a managed seat. On a clean checkout or in CI these vars are + * unset, the suite is green, and the leak is invisible -- which is why it survived. + * + * Scrubbing by prefix rather than by an explicit list is deliberate: any lease variable + * added later leaks by exactly the same route, and a list would have to be remembered. + */ + +const HOST_LEASE_PREFIX = 'MOSAIC_LEASE_'; +const HOST_LEASE_EXTRA = ['MOSAIC_RUNTIME_GENERATION']; + +export function isHostLeaseVariable(name: string): boolean { + return name.startsWith(HOST_LEASE_PREFIX) || HOST_LEASE_EXTRA.includes(name); +} + +/** Deletes the host lease variables from `environment`; returns the names removed. */ +export function scrubHostLeaseEnv(environment: NodeJS.ProcessEnv): string[] { + const removed = Object.keys(environment).filter(isHostLeaseVariable); + for (const name of removed) { + delete environment[name]; + } + return removed.sort(); +} diff --git a/packages/mosaic/vitest.config.ts b/packages/mosaic/vitest.config.ts index f542764b..098eb78c 100644 --- a/packages/mosaic/vitest.config.ts +++ b/packages/mosaic/vitest.config.ts @@ -5,6 +5,7 @@ export default defineConfig({ globals: true, environment: 'node', testTimeout: 30_000, + setupFiles: ['./vitest.setup.ts'], coverage: { provider: 'v8', include: ['src/commands/skill.ts', 'src/lease-broker/broker-test-client.ts'], diff --git a/packages/mosaic/vitest.setup.ts b/packages/mosaic/vitest.setup.ts new file mode 100644 index 00000000..af909958 --- /dev/null +++ b/packages/mosaic/vitest.setup.ts @@ -0,0 +1,5 @@ +import { scrubHostLeaseEnv } from './src/test-support/host-lease-env.js'; + +// Runs before every spec file in this package. See src/test-support/host-lease-env.ts +// for why the host's lease identity must not reach a spawned hook process. +scrubHostLeaseEnv(process.env);