Compare commits

..
Author SHA1 Message Date
fred 982f4fc8f9 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, <the few
vars the case sets> }`. 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.
2026-08-15 13:28:39 -05:00
6 changed files with 112 additions and 58 deletions
+1
View File
@@ -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',
],
-58
View File
@@ -35,18 +35,6 @@ SOURCE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
TARGET_DIR="${MOSAIC_HOME:-$HOME/.config/mosaic}"
INSTALL_MODE="${MOSAIC_INSTALL_MODE:-prompt}"
# Normalize the ambient umask so directory modes are a property of the installer
# and not of whatever shell invoked it (#1236). Debian/Ubuntu ship umask 002, so
# every `mkdir -p` below yielded 0775 — and the fleet env boundary rejects any
# managed directory with `mode & 0o022`, which made `mosaic fleet init --write`
# impossible on a stock install of those distros. Fedora/RHEL ship 022 and did
# not trip it, so the product worked or did not depending on the operator's
# login shell. 022 is what this script already assumes it produces: see the
# umask note in make_durable_snapshot, which restores to the ambient value
# precisely so "every later sync copy and new framework dir" gets 0644/0755.
# Now that value is 022 rather than whatever was inherited.
umask 022
# Deliberately parsed from "$@" (a real, explicit, per-invocation argument) —
# never an environment variable — so this opt-out can never sit silently
# inherited in a shell profile. See #869 Point-1 C2.
@@ -708,52 +696,6 @@ sync_framework
mkdir -p "$TARGET_DIR/memory"
mkdir -p "$TARGET_DIR/credentials"
# Three directories must be 0700, not merely not-group-writable (#1236).
# The fleet code guards them with two different masks in two different
# languages, and the strict one wins:
#
# assertPrivateManagedDirectory (fleet-reconciler.js, `mode & 0o077`)
# -> MOSAIC_HOME and MOSAIC_HOME/fleet, checked before the roster lock is
# taken, so every mutating `mosaic fleet` command dies at 0755.
# assert_private_directory (tools/fleet/start-agent-session.sh, `mode & 077`)
# -> MOSAIC_HOME/fleet/agents, checked before a pane is ever spawned.
#
# Their laxer siblings (`mode & 0o022`) accept 0755, which is why normalizing
# the umask above is necessary and not sufficient — a correct umask-022 install
# still produces 0755 and still cannot run `mosaic fleet init --write`. Say the
# strict modes outright rather than inferring them from a umask.
#
# Only these. The rest of the tree is content, stays 0755, and is only ever
# reached by the 0o022 checks, which 0755 satisfies.
chmod 700 "$TARGET_DIR" 2>/dev/null || \
warn "Could not set 0700 on $TARGET_DIR — 'mosaic fleet' mutations will fail as unsafe-permissions."
if [[ -d "$TARGET_DIR/fleet" ]]; then
chmod 700 "$TARGET_DIR/fleet" 2>/dev/null || \
warn "Could not set 0700 on $TARGET_DIR/fleet — 'mosaic fleet' mutations will fail as unsafe-permissions."
fi
# fleet/agents does not exist on a first install — the CLI creates it 0700 on
# demand. It is chmod'd here for the UPGRADE case: a tree built under umask 002
# has it at 0775, and the repair sweep below cannot rescue it, because stripping
# group/other write from 0755 leaves 0750 and `mode & 077` is still non-zero.
if [[ -d "$TARGET_DIR/fleet/agents" ]]; then
chmod 700 "$TARGET_DIR/fleet/agents" 2>/dev/null || \
warn "Could not set 0700 on $TARGET_DIR/fleet/agents — agent sessions will fail to start as unsafe-permissions."
fi
# credentials/ holds secrets and was never meant to be group-readable either.
# It is not on the fleet boundary, so a failure here breaks nothing — but it is
# the one directory where a silently-failed chmod leaves secrets group-readable,
# which is precisely the failure worth a line in the output.
chmod 700 "$TARGET_DIR/credentials" 2>/dev/null || \
warn "Could not set 0700 on $TARGET_DIR/credentials — stored secrets may be readable by other users on this host."
# Repair an existing tree. The umask above only governs directories this run
# creates, so a host installed under umask 002 before this fix keeps its 0775
# dirs through every upgrade and stays broken. Strips group/other WRITE only —
# never read or execute — so it can repair the boundary violation without
# changing who can traverse or read anything. Scoped to directories: file modes
# are the manifest's business, not this fix's.
find "$TARGET_DIR" -type d -perm /022 -exec chmod go-w {} + 2>/dev/null || true
# Reconcile contract files from defaults/ into the framework root: framework-owned
# files (CONSTITUTION/AGENTS/STANDARDS) are overwritten every upgrade (a divergent
# copy is backed up once); user-seeded files (TOOLS) are written on first install only.
@@ -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([]);
});
});
@@ -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, <the
* few vars this case cares about> }`. 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();
}
+1
View File
@@ -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'],
+5
View File
@@ -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);