fix(fleet): bind secure reads to descriptors
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -68,6 +68,10 @@ the shell sidecar only provides fallback state when the native marker is stale o
|
|||||||
|
|
||||||
`mosaic agent comms-block <exact-member>` can inspect that exact roster member's resolved Fleet-Comms
|
`mosaic agent comms-block <exact-member>` 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.
|
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
|
## Current M2 boundary
|
||||||
|
|
||||||
|
|||||||
@@ -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.
|
- 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.
|
- 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.
|
- 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 <fixture> 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.
|
||||||
|
|||||||
@@ -1,7 +1,42 @@
|
|||||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||||
import { chmodSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
|
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 { tmpdir } from 'node:os';
|
||||||
import { join } from 'node:path';
|
import { join } from 'node:path';
|
||||||
|
|
||||||
|
interface FilesystemRaceState {
|
||||||
|
afterLstat?: (path: string) => void;
|
||||||
|
afterOpen?: (path: string) => void;
|
||||||
|
}
|
||||||
|
|
||||||
|
const filesystemRaceState = vi.hoisted<FilesystemRaceState>(() => ({}));
|
||||||
|
|
||||||
|
vi.mock('node:fs', async (importOriginal) => {
|
||||||
|
const actual = await importOriginal<typeof NodeFs>();
|
||||||
|
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';
|
import { assertCanonicalContainment, readRegularFileSecure } from './secure-file.js';
|
||||||
|
|
||||||
describe('secure file reads', () => {
|
describe('secure file reads', () => {
|
||||||
@@ -9,9 +44,15 @@ describe('secure file reads', () => {
|
|||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
root = mkdtempSync(join(tmpdir(), 'mosaic-secure-file-'));
|
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', () => {
|
it('rejects canonical path escape', () => {
|
||||||
expect(() => assertCanonicalContainment(root, join(root, '..', 'outside'))).toThrow(
|
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', () => {
|
it('uses effective-identity execute access after regular-file validation', () => {
|
||||||
const file = join(root, 'helper.sh');
|
const file = join(root, 'helper.sh');
|
||||||
writeFileSync(file, '#!/bin/sh\n', { mode: 0o644 });
|
writeFileSync(file, '#!/bin/sh\n', { mode: 0o644 });
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import {
|
|||||||
openSync,
|
openSync,
|
||||||
readFileSync,
|
readFileSync,
|
||||||
} from 'node:fs';
|
} from 'node:fs';
|
||||||
|
import { platform } from 'node:os';
|
||||||
import { dirname, isAbsolute, relative, resolve, sep } from 'node:path';
|
import { dirname, isAbsolute, relative, resolve, sep } from 'node:path';
|
||||||
|
|
||||||
export interface SecureFileReadOptions {
|
export interface SecureFileReadOptions {
|
||||||
@@ -30,6 +31,109 @@ function sameIdentity(
|
|||||||
return left.dev === right.dev && left.ino === right.ino;
|
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 {
|
export function assertCanonicalContainment(root: string, target: string): void {
|
||||||
const canonicalRoot = resolve(root);
|
const canonicalRoot = resolve(root);
|
||||||
const canonicalTarget = resolve(target);
|
const canonicalTarget = resolve(target);
|
||||||
@@ -99,43 +203,32 @@ export function readRegularFileSecure(
|
|||||||
path: string,
|
path: string,
|
||||||
options: SecureFileReadOptions,
|
options: SecureFileReadOptions,
|
||||||
): SecureFileSnapshot {
|
): SecureFileSnapshot {
|
||||||
assertCanonicalContainment(options.root, path);
|
const openedFile = openFileBeneathRoot(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);
|
|
||||||
try {
|
try {
|
||||||
const opened = fstatSync(fd);
|
const opened = fstatSync(openedFile.fd);
|
||||||
if (!opened.isFile() || !sameIdentity(before, opened)) {
|
if (!opened.isFile()) throw new Error('managed file is not a regular file');
|
||||||
throw new Error(`file changed during secure open: ${path}`);
|
if (options.maxBytes !== undefined && opened.size > options.maxBytes) {
|
||||||
|
throw new Error(`managed file exceeds ${options.maxBytes} bytes`);
|
||||||
}
|
}
|
||||||
if (options.executable) {
|
if (options.executable) {
|
||||||
accessSync(path, constants.X_OK);
|
try {
|
||||||
const afterAccess = lstatSync(path);
|
accessSync(procDescriptorPath(openedFile.fd), constants.X_OK);
|
||||||
if (
|
} catch (error) {
|
||||||
afterAccess.isSymbolicLink() ||
|
throw secureFilesystemError('managed file is not executable', error);
|
||||||
!afterAccess.isFile() ||
|
}
|
||||||
!sameIdentity(opened, afterAccess)
|
const afterAccess = fstatSync(openedFile.fd);
|
||||||
) {
|
if (!afterAccess.isFile() || !sameIdentity(opened, afterAccess)) {
|
||||||
throw new Error(`file changed during executable access check: ${path}`);
|
throw new Error('managed file changed during executable access check');
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
const content = readFileSync(fd);
|
|
||||||
const after = fstatSync(fd);
|
const content = readFileSync(openedFile.fd);
|
||||||
const current = lstatSync(path);
|
const after = fstatSync(openedFile.fd);
|
||||||
if (
|
if (!after.isFile() || !sameIdentity(opened, after)) {
|
||||||
!after.isFile() ||
|
throw new Error('managed file changed during secure read');
|
||||||
!sameIdentity(opened, after) ||
|
}
|
||||||
current.isSymbolicLink() ||
|
if (options.maxBytes !== undefined && content.byteLength > options.maxBytes) {
|
||||||
!current.isFile() ||
|
throw new Error(`managed file exceeds ${options.maxBytes} bytes`);
|
||||||
!sameIdentity(opened, current)
|
|
||||||
) {
|
|
||||||
throw new Error(`file changed during secure read: ${path}`);
|
|
||||||
}
|
}
|
||||||
return {
|
return {
|
||||||
content,
|
content,
|
||||||
@@ -144,6 +237,6 @@ export function readRegularFileSecure(
|
|||||||
ino: opened.ino,
|
ino: opened.ino,
|
||||||
};
|
};
|
||||||
} finally {
|
} finally {
|
||||||
closeSync(fd);
|
closeDescriptors(openedFile.descriptors);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user