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);