fix(installer): warn on a failed credentials/ chmod instead of swallowing it

scooby's review flag 1 on #1242. The other three chmods warn; this one was
`|| true`. It is the one directory holding secrets, so a chmod that fails
silently there is the failure most worth a line in the output.

Comment-and-warn only. No behaviour change on the success path.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01WYgWocp36goy8hj2ui6ps1

eslint.config.mjs

@@ -33,7 +33,6 @@ 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',
           ],

packages/mosaic/framework/install.sh

@@ -35,6 +35,18 @@ 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.
@@ -696,6 +708,52 @@ 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.

packages/mosaic/src/test-support/host-lease-env.spec.ts

@@ -1,64 +0,0 @@
-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([]);
-  });
-});

packages/mosaic/src/test-support/host-lease-env.ts

@@ -1,41 +0,0 @@
-/**
- * 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();
-}

packages/mosaic/vitest.config.ts

@@ -5,7 +5,6 @@ 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'],

packages/mosaic/vitest.setup.ts

@@ -1,5 +0,0 @@
-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);