diff --git a/docs/fleet/FLEET-LAUNCH.md b/docs/fleet/FLEET-LAUNCH.md index 3979fd4..c6e2167 100644 --- a/docs/fleet/FLEET-LAUNCH.md +++ b/docs/fleet/FLEET-LAUNCH.md @@ -68,6 +68,10 @@ the shell sidecar only provides fallback state when the native marker is stale o `mosaic agent comms-block ` can inspect that exact roster member's resolved Fleet-Comms block. It is a read-only inspection tool and fails loudly for an unknown exact member or missing roster. +On Linux, the installed roster, TOOLS contract, and executable helper are opened through a held +descriptor chain rooted at `/`; every managed path component uses no-follow traversal, and content plus +execute validation stay bound to the same opened file. Systems without Linux `/proc/self/fd` support +fail closed rather than falling back to pathname revalidation. ## Current M2 boundary diff --git a/docs/scratchpads/issue-766-exact-fleet-comms.md b/docs/scratchpads/issue-766-exact-fleet-comms.md index 3bad5dc..cdad958 100644 --- a/docs/scratchpads/issue-766-exact-fleet-comms.md +++ b/docs/scratchpads/issue-766-exact-fleet-comms.md @@ -71,3 +71,10 @@ Replace inference-prone fleet onboarding guidance with one roster-resolved contr - Final remediated gates passed: four-runtime regression 39/39; six changed-suite matrix 345/345; connector schema regression PASS; Mosaic package 910/910; installer migration 21/21; `agent-send.test.sh` 11/11; named-socket isolation PASS; Matrix/tmux transport 12/12; repository format PASS; typecheck 42/42 tasks; lint 23/23 tasks; tests 42/42 tasks; build 23/23 tasks. - No live tmux/session/fleet mutation, commit, push, PR mutation, issue mutation, context mutation, or reviewer launch performed. - Exact synthetic-tree reconstruction and frozen evidence are included in the coordinator handoff. +- Sole-remediation preflight reverified the clean committed checkout at head `0dc47cac92c93a3ffd39ba9dd6685ac4165f6361`, tree `538de6ccce1f8c44ba288a7493286e63a3413e75`, branch `fix/766-exact-fleet-comms`; issue and PR state were read only through Mosaic wrappers. +- Deterministic red-first ancestor substitution swapped validated `root/tools` for an external symlink immediately after `lstat`; current head returned `external marker` (`1 failed, 4 passed`) before implementation. +- Secure reads now hold `/` and every root/descendant directory descriptor, traverse appended components through Linux `/proc/self/fd` with `O_DIRECTORY|O_NOFOLLOW`, and read plus effective-identity execute-check the same final descriptor. Non-Linux or unavailable proc-fd capability fails closed; stable errors redact managed paths while retaining Node `code` compatibility for missing/non-executable repair behavior. +- Added deterministic root-selection, descendant-ancestor, and final-target substitution coverage. All return trusted descriptor-bound bytes after rename/symlink replacement; the race suite passed 50/50 repeated runs. +- Isolated CLI verification drove `mosaic agent --mosaic-home comms-block self` while repeatedly swapping `fleet/` with an external symlink: `trusted=2 fail_closed=10 external_marker=0`; a persistent symlink ancestor exited 1 with a redacted unsafe-ancestor error. No live fleet state was used or mutated. +- Remediation gates: focused secure-file/comms/launch/tmux/Matrix `110/110`; full `@mosaicstack/mosaic` `914/914`; package and repository typecheck pass (`42/42` repository tasks); package and repository lint pass (`23/23` repository tasks); repository format check and `git diff --check` pass. +- Independent review found one production hardening blocker (nonblocking final open), one redacted-error blocker, and a deterministic ancestor-test gap. Remediation added `O_NONBLOCK`, normalized execute errors while preserving errno, proved the ancestor hook fires, and added final-target substitution coverage; post-remediation review evidence is clean on the production invariant. diff --git a/packages/mosaic/src/fleet/secure-file.spec.ts b/packages/mosaic/src/fleet/secure-file.spec.ts index 95a7049..7d97384 100644 --- a/packages/mosaic/src/fleet/secure-file.spec.ts +++ b/packages/mosaic/src/fleet/secure-file.spec.ts @@ -1,7 +1,42 @@ -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { chmodSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + chmodSync, + mkdirSync, + mkdtempSync, + renameSync, + rmSync, + symlinkSync, + writeFileSync, + type PathLike, +} from 'node:fs'; +import type * as NodeFs from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; + +interface FilesystemRaceState { + afterLstat?: (path: string) => void; + afterOpen?: (path: string) => void; +} + +const filesystemRaceState = vi.hoisted(() => ({})); + +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + lstatSync: (path: PathLike) => { + const result = actual.lstatSync(path); + filesystemRaceState.afterLstat?.(String(path)); + return result; + }, + openSync: (path: PathLike, flags: string | number, mode?: number) => { + const fd = actual.openSync(path, flags, mode); + filesystemRaceState.afterOpen?.(String(path)); + return fd; + }, + }; +}); + import { assertCanonicalContainment, readRegularFileSecure } from './secure-file.js'; describe('secure file reads', () => { @@ -9,9 +44,15 @@ describe('secure file reads', () => { beforeEach(() => { root = mkdtempSync(join(tmpdir(), 'mosaic-secure-file-')); + filesystemRaceState.afterLstat = undefined; + filesystemRaceState.afterOpen = undefined; }); - afterEach(() => rmSync(root, { recursive: true, force: true })); + afterEach(() => { + filesystemRaceState.afterLstat = undefined; + filesystemRaceState.afterOpen = undefined; + rmSync(root, { recursive: true, force: true }); + }); it('rejects canonical path escape', () => { expect(() => assertCanonicalContainment(root, join(root, '..', 'outside'))).toThrow( @@ -40,6 +81,91 @@ describe('secure file reads', () => { ); }); + it('keeps ancestor traversal bound when an opened directory is substituted', () => { + const tools = join(root, 'tools'); + const displacedTools = join(root, 'tools.displaced'); + const external = join(root, 'external'); + const helper = join(tools, 'helper.sh'); + mkdirSync(tools); + mkdirSync(external); + writeFileSync(helper, 'trusted\n', { mode: 0o755 }); + writeFileSync(join(external, 'helper.sh'), 'external marker\n', { mode: 0o755 }); + + let substituted = false; + filesystemRaceState.afterOpen = (openedPath: string): void => { + if (substituted || !openedPath.startsWith('/proc/self/fd/')) return; + if (openedPath.split('/').at(-1) !== 'tools') return; + substituted = true; + renameSync(tools, displacedTools); + symlinkSync(external, tools); + }; + + const result = readRegularFileSecure(helper, { root, executable: true }); + expect(substituted).toBe(true); + expect(result.content.toString('utf8')).toBe('trusted\n'); + }); + + it('keeps root selection bound when the opened root is substituted', () => { + const displacedRoot = `${root}.displaced`; + const externalRoot = `${root}.external`; + const helper = join(root, 'helper.sh'); + mkdirSync(externalRoot); + writeFileSync(helper, 'trusted root\n', { mode: 0o755 }); + writeFileSync(join(externalRoot, 'helper.sh'), 'external root marker\n', { mode: 0o755 }); + + let substituted = false; + filesystemRaceState.afterOpen = (openedPath: string): void => { + if (substituted || !openedPath.startsWith('/proc/self/fd/')) return; + const match = openedPath.match(/\/([^/]+)$/); + if (match?.[1] !== root.split('/').filter(Boolean).at(-1)) return; + substituted = true; + renameSync(root, displacedRoot); + symlinkSync(externalRoot, root); + }; + + const result = readRegularFileSecure(helper, { root, executable: true }); + expect(substituted).toBe(true); + expect(result.content.toString('utf8')).toBe('trusted root\n'); + filesystemRaceState.afterOpen = undefined; + rmSync(root); + renameSync(displacedRoot, root); + }); + + it('keeps target read and execute validation bound to the opened file', () => { + const file = join(root, 'helper.sh'); + const displaced = join(root, 'helper.displaced.sh'); + const external = join(root, 'external-helper.sh'); + writeFileSync(file, 'trusted target\n', { mode: 0o755 }); + writeFileSync(external, 'external target marker\n', { mode: 0o755 }); + + let substituted = false; + filesystemRaceState.afterOpen = (openedPath: string): void => { + if (substituted || !openedPath.startsWith('/proc/self/fd/')) return; + if (openedPath.split('/').at(-1) !== 'helper.sh') return; + substituted = true; + renameSync(file, displaced); + symlinkSync(external, file); + }; + + const result = readRegularFileSecure(file, { root, executable: true }); + expect(substituted).toBe(true); + expect(result.content.toString('utf8')).toBe('trusted target\n'); + }); + + it('uses a stable redacted executable error while retaining the error code', () => { + const file = join(root, 'helper.sh'); + writeFileSync(file, '#!/bin/sh\n', { mode: 0o644 }); + + try { + readRegularFileSecure(file, { root, executable: true }); + throw new Error('expected executable validation to fail'); + } catch (error) { + expect(error).toMatchObject({ message: 'managed file is not executable', code: 'EACCES' }); + expect(String(error)).not.toContain('/proc/self/fd/'); + expect(String(error)).not.toContain(root); + } + }); + it('uses effective-identity execute access after regular-file validation', () => { const file = join(root, 'helper.sh'); writeFileSync(file, '#!/bin/sh\n', { mode: 0o644 }); diff --git a/packages/mosaic/src/fleet/secure-file.ts b/packages/mosaic/src/fleet/secure-file.ts index b07fbb0..82f2720 100644 --- a/packages/mosaic/src/fleet/secure-file.ts +++ b/packages/mosaic/src/fleet/secure-file.ts @@ -8,6 +8,7 @@ import { openSync, readFileSync, } from 'node:fs'; +import { platform } from 'node:os'; import { dirname, isAbsolute, relative, resolve, sep } from 'node:path'; export interface SecureFileReadOptions { @@ -30,6 +31,109 @@ function sameIdentity( return left.dev === right.dev && left.ino === right.ino; } +function secureFilesystemError(message: string, cause: unknown): Error { + const error = new Error(message); + if (cause instanceof Error && 'code' in cause && typeof cause.code === 'string') { + Object.defineProperty(error, 'code', { value: cause.code, enumerable: true }); + } + return error; +} + +function closeDescriptors(descriptors: number[]): void { + for (const fd of descriptors.reverse()) { + try { + closeSync(fd); + } catch { + // Best-effort cleanup must not replace the security decision already made. + } + } +} + +function procDescriptorPath(fd: number, component?: string): string { + const descriptor = `/proc/self/fd/${fd}`; + return component === undefined ? descriptor : `${descriptor}/${component}`; +} + +/** + * Hold each directory while opening its child through Linux proc-fd. The only + * symlink followed is the kernel-owned descriptor link; O_NOFOLLOW protects + * every appended filesystem component from substitution. + */ +function openDirectoryChain(absoluteDirectory: string): { fd: number; descriptors: number[] } { + if (platform() !== 'linux') { + throw new Error('secure descriptor traversal is unsupported on this platform'); + } + + const descriptors: number[] = []; + try { + let fd = openSync(sep, constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW); + descriptors.push(fd); + for (const component of absoluteDirectory.split(sep).filter(Boolean)) { + fd = openSync( + procDescriptorPath(fd, component), + constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW, + ); + descriptors.push(fd); + if (!fstatSync(fd).isDirectory()) { + throw new Error('secure descriptor traversal encountered a non-directory component'); + } + } + return { fd, descriptors }; + } catch (error) { + closeDescriptors(descriptors); + throw secureFilesystemError( + 'secure descriptor traversal failed: symbolic link, unavailable, or not a directory', + error, + ); + } +} + +function openFileBeneathRoot(root: string, target: string): { fd: number; descriptors: number[] } { + const canonicalRoot = resolve(root); + const canonicalTarget = resolve(target); + assertCanonicalContainment(canonicalRoot, canonicalTarget); + const components = relative(canonicalRoot, canonicalTarget).split(sep).filter(Boolean); + const fileName = components.pop(); + if (fileName === undefined) throw new Error('managed file path names the managed root'); + + const rootChain = openDirectoryChain(canonicalRoot); + try { + let parentFd = rootChain.fd; + for (const component of components) { + try { + parentFd = openSync( + procDescriptorPath(parentFd, component), + constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW, + ); + } catch (error) { + throw secureFilesystemError( + 'path ancestor is a symbolic link, unavailable, or not a directory', + error, + ); + } + rootChain.descriptors.push(parentFd); + if (!fstatSync(parentFd).isDirectory()) { + throw new Error('path ancestor is a symbolic link or not a directory'); + } + } + let fd: number; + try { + fd = openSync( + procDescriptorPath(parentFd, fileName), + constants.O_RDONLY | constants.O_NONBLOCK | constants.O_NOFOLLOW, + ); + } catch (error) { + throw secureFilesystemError('file is a symbolic link or unavailable', error); + } + rootChain.descriptors.push(fd); + return { fd, descriptors: rootChain.descriptors }; + } catch (error) { + closeDescriptors(rootChain.descriptors); + if (error instanceof Error) throw error; + throw new Error('secure managed file open failed'); + } +} + export function assertCanonicalContainment(root: string, target: string): void { const canonicalRoot = resolve(root); const canonicalTarget = resolve(target); @@ -99,43 +203,32 @@ export function readRegularFileSecure( path: string, options: SecureFileReadOptions, ): SecureFileSnapshot { - assertCanonicalContainment(options.root, path); - assertNoSymlinkAncestors(path); - const before = lstatSync(path); - if (before.isSymbolicLink()) throw new Error(`file is a symbolic link: ${path}`); - if (!before.isFile()) throw new Error(`file is not a regular file: ${path}`); - if (options.maxBytes !== undefined && before.size > options.maxBytes) { - throw new Error(`file exceeds ${options.maxBytes} bytes: ${path}`); - } - - const fd = openSync(path, constants.O_RDONLY | constants.O_NOFOLLOW); + const openedFile = openFileBeneathRoot(options.root, path); try { - const opened = fstatSync(fd); - if (!opened.isFile() || !sameIdentity(before, opened)) { - throw new Error(`file changed during secure open: ${path}`); + const opened = fstatSync(openedFile.fd); + if (!opened.isFile()) throw new Error('managed file is not a regular file'); + if (options.maxBytes !== undefined && opened.size > options.maxBytes) { + throw new Error(`managed file exceeds ${options.maxBytes} bytes`); } if (options.executable) { - accessSync(path, constants.X_OK); - const afterAccess = lstatSync(path); - if ( - afterAccess.isSymbolicLink() || - !afterAccess.isFile() || - !sameIdentity(opened, afterAccess) - ) { - throw new Error(`file changed during executable access check: ${path}`); + try { + accessSync(procDescriptorPath(openedFile.fd), constants.X_OK); + } catch (error) { + throw secureFilesystemError('managed file is not executable', error); + } + const afterAccess = fstatSync(openedFile.fd); + if (!afterAccess.isFile() || !sameIdentity(opened, afterAccess)) { + throw new Error('managed file changed during executable access check'); } } - const content = readFileSync(fd); - const after = fstatSync(fd); - const current = lstatSync(path); - if ( - !after.isFile() || - !sameIdentity(opened, after) || - current.isSymbolicLink() || - !current.isFile() || - !sameIdentity(opened, current) - ) { - throw new Error(`file changed during secure read: ${path}`); + + const content = readFileSync(openedFile.fd); + const after = fstatSync(openedFile.fd); + if (!after.isFile() || !sameIdentity(opened, after)) { + throw new Error('managed file changed during secure read'); + } + if (options.maxBytes !== undefined && content.byteLength > options.maxBytes) { + throw new Error(`managed file exceeds ${options.maxBytes} bytes`); } return { content, @@ -144,6 +237,6 @@ export function readRegularFileSecure( ino: opened.ino, }; } finally { - closeSync(fd); + closeDescriptors(openedFile.descriptors); } }