From 34d87dfcee7601046cb8d8c7073c821afd797b27 Mon Sep 17 00:00:00 2001 From: Khaliq Gant Date: Fri, 31 Jul 2026 20:35:24 +0200 Subject: [PATCH 1/2] feat(policy): add ConfinedRoot filesystem confinement primitive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Binds a write to a directory it cannot escape. Policy deciding "allowed" is not a filesystem guarantee: the path can be a symlink, a hardlink to something outside the root, or replaced a microsecond after the check. Two contracts, deliberately separate: - a security refusal makes NO observable filesystem change - an authorized write is atomic — never partial, never a zero-length window Resolution is descriptor-relative, rooted at a directory descriptor captured once. Each component is opened relative to the previous one, so a component swapped on disk mid-operation cannot redirect the write. On Linux this is true openat semantics via /proc/self/fd; elsewhere it degrades to pinned-path, which detects rather than prevents. The mode is reported on every write so callers assert the guarantee they actually got instead of assuming the stronger one. Originated in the Agent Relay x Ratify design-partner spike, where four distinct defects were found in this code by adversarial testing — three by us, one by Identities AI. Every one was platform- or timing-dependent, and none would have been caught by asserting on the return value alone: - open(fifo, O_WRONLY) blocks forever waiting for a reader - O_TRUNC destroys a hardlinked file before the link check can refuse - a freed inode is recycled, defeating a (dev, ino) identity comparison - a refusal deleted the pre-existing file it was protecting The suite that found them ships alongside: 21 tests including concurrent-swap stress and positive controls, so a "fix" that refuses everything cannot pass. Nothing consumes this yet. Placement in @agent-relay/policy is the least-bad home for a safety primitive and is the reviewer's call to confirm. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 4 + packages/policy/src/fs-confine.test.ts | 376 +++++++++++++++++++ packages/policy/src/fs-confine.ts | 487 +++++++++++++++++++++++++ packages/policy/src/index.ts | 9 + 4 files changed, 876 insertions(+) create mode 100644 packages/policy/src/fs-confine.test.ts create mode 100644 packages/policy/src/fs-confine.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 21b845f47..6e91a50be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- `@agent-relay/policy` exports `ConfinedRoot`: filesystem confinement that binds a write to a directory it cannot escape. Refuses path traversal, symlinked components, hardlinks to files outside the root, non-regular targets, and components swapped mid-operation; writes atomically so an interrupted write never leaves a partial or empty file. A security refusal makes no observable filesystem change. Nothing consumes it yet — it is available for callers that execute agent-directed writes. + ## [11.3.1] - 2026-07-31 ### Fixed diff --git a/packages/policy/src/fs-confine.test.ts b/packages/policy/src/fs-confine.test.ts new file mode 100644 index 000000000..26c8026cd --- /dev/null +++ b/packages/policy/src/fs-confine.test.ts @@ -0,0 +1,376 @@ +/** + * Adversarial tests for filesystem confinement. + * + * Two contracts are tested separately, because conflating them is what produced + * the worst defect this code has had — a refusal that deleted the file it was + * protecting: + * + * C1 SECURITY REFUSAL — a refusal mutates nothing observable. Every negative + * case asserts the refusal *and* that pre-existing state is byte-identical + * afterwards. A test that checks only the return value passes against an + * implementation that destroys your data and then reports failure. + * + * C2 WRITE ATOMICITY — an authorized write either fully replaces the target + * or leaves it exactly as it was. + * + * The positive controls are not filler. A confinement layer that refuses + * legitimate writes is as broken as one that permits escapes, and every fix + * here risks becoming a blanket refusal. + */ + +import { execFileSync } from 'node:child_process'; +import { + existsSync, + linkSync, + mkdirSync, + mkdtempSync, + readFileSync, + readdirSync, + realpathSync, + rmSync, + statSync, + symlinkSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { afterEach, describe, expect, it } from 'vitest'; + +import { ConfinedRoot, ConfinementError } from './fs-confine.js'; + +const VICTIM = 'IMPORTANT PRE-EXISTING CONTENT'; + +interface Sandbox { + base: string; + root: string; + outside: string; + victim: string; +} + +const created: string[] = []; +const roots: ConfinedRoot[] = []; + +afterEach(() => { + for (const r of roots.splice(0)) r.close(); + for (const base of created.splice(0)) rmSync(base, { recursive: true, force: true }); +}); + +function sandbox(): Sandbox { + const base = realpathSync(mkdtempSync(join(tmpdir(), 'confine-'))); + const root = join(base, 'repo'); + const outside = join(base, 'outside'); + mkdirSync(root); + mkdirSync(outside); + const victim = join(outside, 'secret.txt'); + writeFileSync(victim, VICTIM); + created.push(base); + return { base, root, outside, victim }; +} + +function open(root: string): ConfinedRoot { + const cr = new ConfinedRoot(root); + roots.push(cr); + return cr; +} + +/** Relative path -> content (or a marker), for every entry under `dir`. */ +function snapshot(dir: string): Record { + const out: Record = {}; + const walk = (p: string, rel: string): void => { + for (const name of readdirSync(p).sort()) { + const full = join(p, name); + const key = rel ? `${rel}/${name}` : name; + const st = statSync(full, { throwIfNoEntry: false }); + if (!st) out[key] = ''; + else if (st.isDirectory()) { + out[key] = ''; + walk(full, key); + } else if (st.isFile()) out[key] = readFileSync(full, 'utf8'); + else out[key] = ''; + } + }; + walk(dir, ''); + return out; +} + +/** + * Assert C1: the write is refused with `code`, nothing pre-existing changed, and + * the payload landed nowhere. + * + * Additions made by the *adversary* are expected — planting a symlink is the + * attack, not a defect — so only pre-existing entries are compared. + */ +function expectRefusal(s: Sandbox, code: string, run: () => void, payload = 'PWNED'): void { + const before = snapshot(s.base); + + // Run exactly once. These cases mutate the tree from inside a hook, so a + // second invocation would face a different filesystem than the first and + // assert against the wrong state. + let thrown: unknown; + try { + run(); + } catch (err) { + thrown = err; + } + + expect(thrown, 'expected the write to be refused').toBeInstanceOf(ConfinementError); + expect((thrown as ConfinementError).code).toBe(code); + + const after = snapshot(s.base); + for (const [key, value] of Object.entries(before)) { + expect(after[key], `pre-existing entry ${key} was mutated or deleted`).toBe(value); + } + for (const [key, value] of Object.entries(after)) { + expect(value, `payload leaked into ${key}`).not.toBe(payload); + } +} + +describe('ConfinedRoot — resolution mode', () => { + it('reports which guarantee it is providing', () => { + const s = sandbox(); + const cr = open(s.root); + // descriptor-relative on Linux (via /proc/self/fd); pinned-path elsewhere. + // Asserted so a regression to the weaker mode is visible, not silent. + expect(['descriptor-relative', 'pinned-path']).toContain(cr.resolutionMode); + if (process.platform === 'linux') expect(cr.resolutionMode).toBe('descriptor-relative'); + }); +}); + +describe('ConfinedRoot — refusals mutate nothing (C1)', () => { + it('refuses ../ traversal above the root', () => { + const s = sandbox(); + const cr = open(s.root); + expectRefusal(s, 'dot_segment', () => cr.writeFile('../outside/secret.txt', 'PWNED')); + }); + + it('refuses an absolute path', () => { + const s = sandbox(); + const cr = open(s.root); + expectRefusal(s, 'absolute_path', () => cr.writeFile(s.victim, 'PWNED')); + }); + + it('refuses a final component that is a symlink pointing outside', () => { + const s = sandbox(); + symlinkSync(s.victim, join(s.root, 'link.txt')); + const cr = open(s.root); + expectRefusal(s, 'symlink_target', () => cr.writeFile('link.txt', 'PWNED')); + }); + + it('refuses an intermediate directory that is a symlink', () => { + const s = sandbox(); + symlinkSync(s.outside, join(s.root, 'docs')); + const cr = open(s.root); + expectRefusal(s, 'symlink_component', () => cr.writeFile('docs/secret.txt', 'PWNED')); + }); + + it('refuses a hardlink inside the root to a file outside it', () => { + // realpath cannot help: a hardlink has no target to resolve. Link count is + // the only signal that the name is not the file's only name. + const s = sandbox(); + linkSync(s.victim, join(s.root, 'hard.txt')); + const cr = open(s.root); + expectRefusal(s, 'hardlink', () => cr.writeFile('hard.txt', 'PWNED')); + }); + + it('refuses a fifo rather than blocking on it forever', () => { + // open(fifo, O_WRONLY) blocks until a reader appears. Without O_NONBLOCK a + // named pipe planted in the root hangs the process indefinitely — in an + // agent, worse than a refused write. + const s = sandbox(); + try { + execFileSync('mkfifo', [join(s.root, 'pipe')]); + } catch { + return; // platform without mkfifo + } + const cr = open(s.root); + expectRefusal(s, 'not_regular_file', () => cr.writeFile('pipe', 'PWNED')); + }); +}); + +describe('ConfinedRoot — concurrent mutation', () => { + it('refuses when the final component is swapped for a symlink after inspection', () => { + const s = sandbox(); + const cr = open(s.root); + expectRefusal(s, 'symlink_target', () => + cr.writeFile('notes.md', 'PWNED', { + beforeOpen: () => symlinkSync(s.victim, join(s.root, 'notes.md')), + }), + ); + }); + + it('refuses when an intermediate component is swapped after the walk', () => { + const s = sandbox(); + mkdirSync(join(s.root, 'docs')); + const cr = open(s.root); + expectRefusal(s, 'component_swapped', () => + cr.writeFile('docs/notes.md', 'PWNED', { + beforeOpen: () => { + rmSync(join(s.root, 'docs'), { recursive: true, force: true }); + symlinkSync(s.outside, join(s.root, 'docs')); + }, + }), + ); + }); + + it('does not delete a pre-existing outside file while refusing a swap', () => { + // The defect this whole file exists for. With an intermediate directory + // swapped to an outside symlink AND a file already present outside at the + // same final name, an earlier implementation refused correctly and deleted + // that file: it inferred "I created this" from a pre-swap stat, then removed + // a recomputed path that by then named something else entirely. + const s = sandbox(); + mkdirSync(join(s.root, 'docs')); + writeFileSync(join(s.outside, 'notes.md'), 'VICTIM FILE'); + const cr = open(s.root); + + expectRefusal(s, 'component_swapped', () => + cr.writeFile('docs/notes.md', 'PWNED', { + beforeOpen: () => { + rmSync(join(s.root, 'docs'), { recursive: true, force: true }); + symlinkSync(s.outside, join(s.root, 'docs')); + }, + }), + ); + expect(readFileSync(join(s.outside, 'notes.md'), 'utf8')).toBe('VICTIM FILE'); + }); + + it('refuses a multi-level ancestor swap', () => { + const s = sandbox(); + mkdirSync(join(s.root, 'a')); + mkdirSync(join(s.root, 'a', 'b')); + const cr = open(s.root); + expectRefusal(s, 'component_swapped', () => + cr.writeFile('a/b/c.md', 'PWNED', { + beforeOpen: () => { + rmSync(join(s.root, 'a'), { recursive: true, force: true }); + const shim = join(s.base, 'shim'); + mkdirSync(shim, { recursive: true }); + symlinkSync(s.outside, join(shim, 'b')); + symlinkSync(shim, join(s.root, 'a')); + }, + }), + ); + }); + + it('refuses a hardlink created on an existing target after validation', () => { + const s = sandbox(); + writeFileSync(join(s.root, 'late.md'), 'ORIGINAL'); + const cr = open(s.root); + expectRefusal(s, 'hardlink', () => + cr.writeFile('late.md', 'PWNED', { + afterValidate: () => linkSync(join(s.root, 'late.md'), join(s.outside, 'late-link.md')), + }), + ); + }); + + it('survives sustained concurrent parent swapping without damaging outside state', () => { + // Deterministic hooks prove the guard fires at one exact interleaving. This + // hammers real ones. Every outcome must be a clean refusal or a correct + // write — never a corrupted or deleted victim. + const s = sandbox(); + writeFileSync(join(s.outside, 'target.md'), 'VICTIM FILE'); + mkdirSync(join(s.root, 'race')); + const cr = open(s.root); + + for (let i = 0; i < 200; i++) { + try { + rmSync(join(s.root, 'race'), { recursive: true, force: true }); + if (i % 2 === 0) symlinkSync(s.outside, join(s.root, 'race')); + else mkdirSync(join(s.root, 'race')); + } catch { + /* mutator raced itself; irrelevant */ + } + try { + cr.writeFile('race/target.md', `ITERATION ${i}`); + } catch (err) { + expect(err).toBeInstanceOf(ConfinementError); + } + } + + expect(readFileSync(join(s.outside, 'target.md'), 'utf8')).toBe('VICTIM FILE'); + }); +}); + +describe('ConfinedRoot — write atomicity (C2)', () => { + it('writes large content completely rather than short', () => { + const s = sandbox(); + const cr = open(s.root); + const big = 'X'.repeat(5 * 1024 * 1024); + const out = cr.writeFile('big.md', big); + expect(out.bytesWritten).toBe(big.length); + expect(readFileSync(join(s.root, 'big.md'), 'utf8')).toBe(big); + }); + + it('leaves the original intact when refusing after validation', () => { + const s = sandbox(); + const target = join(s.root, 'keep.md'); + writeFileSync(target, 'ORIGINAL'); + const cr = open(s.root); + try { + cr.writeFile('keep.md', 'REPLACEMENT', { + afterValidate: () => linkSync(target, join(s.outside, 'keep-link.md')), + }); + } catch { + /* expected */ + } + // Never empty, never partial — a naive truncate-then-write would leave both. + expect(readFileSync(target, 'utf8')).toBe('ORIGINAL'); + }); + + it('leaves no temporary-file debris', () => { + const s = sandbox(); + const cr = open(s.root); + cr.writeFile('debris/one.md', 'A'); + cr.writeFile('debris/two.md', 'B'); + expect(readdirSync(join(s.root, 'debris')).filter((f) => f.includes('tmp'))).toEqual([]); + }); +}); + +describe('ConfinedRoot — positive controls', () => { + it('creates a new file inside the root', () => { + const s = sandbox(); + const cr = open(s.root); + const out = cr.writeFile('new.md', 'CREATED'); + expect(out.relativePath).toBe('new.md'); + expect(readFileSync(join(s.root, 'new.md'), 'utf8')).toBe('CREATED'); + }); + + it('replaces an existing file inside the root', () => { + const s = sandbox(); + writeFileSync(join(s.root, 'exists.md'), 'OLD'); + const cr = open(s.root); + cr.writeFile('exists.md', 'NEW'); + expect(readFileSync(join(s.root, 'exists.md'), 'utf8')).toBe('NEW'); + }); + + it('creates nested directories', () => { + const s = sandbox(); + const cr = open(s.root); + cr.writeFile('a/b/c/deep.md', 'NESTED'); + expect(readFileSync(join(s.root, 'a/b/c/deep.md'), 'utf8')).toBe('NESTED'); + }); + + it('supports a root that is itself a symlink', () => { + // Common and legitimate: /var -> /private/var on Darwin. The root is + // resolved exactly once at construction, before any adversary-influenced + // component appears. + const s = sandbox(); + const linked = join(s.base, 'repo-link'); + symlinkSync(s.root, linked); + const cr = open(linked); + cr.writeFile('docs/ok.md', 'LEGITIMATE'); + expect(readFileSync(join(s.root, 'docs/ok.md'), 'utf8')).toBe('LEGITIMATE'); + }); + + it('accepts case-variant spellings on either filesystem', () => { + // The old lexical prefix comparison got this wrong on APFS: /Docs/x + // and /docs/x are the same file but different strings. Deciding + // containment by identity rather than spelling is what fixed it. + const s = sandbox(); + const cr = open(s.root); + cr.writeFile('docs/case.md', 'lower'); + cr.writeFile('Docs/case.md', 'upper'); + expect(existsSync(join(s.root, 'docs/case.md'))).toBe(true); + }); +}); diff --git a/packages/policy/src/fs-confine.ts b/packages/policy/src/fs-confine.ts new file mode 100644 index 000000000..a803f0b93 --- /dev/null +++ b/packages/policy/src/fs-confine.ts @@ -0,0 +1,487 @@ +/** + * Filesystem confinement — bind a write to a directory it cannot escape. + * + * An authorization layer decides whether an agent *may* write `docs/x.md`. This + * decides whether the concrete path is *safe to write*. Two independent gates, + * both fail closed, in that order — neither substitutes for the other. Policy + * that says "allowed" is not a filesystem guarantee: the path can be a symlink, + * a hardlink to something outside, or replaced a microsecond after the check. + * + * Originated in the Agent Relay x Ratify design-partner spike, where four + * distinct defects were found in it by adversarial testing — three by us and one + * by Identities AI. Every one was platform- or timing-dependent and none would + * have been caught by asserting on the return value alone. The test suite that + * found them ships alongside as fs-confine.test.ts. + * + * ═══ THREAT MODEL ═══════════════════════════════════════════════════════════ + * + * Stated first because every design choice below follows from it, and because + * three rounds of defects here were all cases of patching a symptom without + * naming the adversary. + * + * **The adversary is a concurrent mutator**: any process that can create, + * rename, unlink, or replace objects underneath the confinement root *while a + * write is in progress*. It does not need to be privileged and it does not need + * to win a tight race repeatedly — a single successful swap between any two + * syscalls is enough. It may act on intermediate directories, on the final + * component, or on both, and it may pre-place objects (files, symlinks, + * hardlinks, fifos) outside the root to be selected by a swap. + * + * Out of scope: an adversary who can already write *through* the confinement + * layer, modify this process's memory, or replace the root itself before + * construction. Those are not filesystem races; they are prior compromise. + * + * **Two contracts, deliberately separated.** Conflating them is what produced + * the destructive-refusal defect: + * + * C1 — SECURITY REFUSAL. If this layer refuses a write for any confinement or + * policy reason, there is **no externally observable filesystem + * mutation**. Nothing created, nothing truncated, nothing unlinked, + * inside or outside the root. A refusal that destroys data is worse than + * the write it prevented. + * + * C2 — WRITE ATOMICITY. Once an authorized write begins, the target is either + * fully replaced or left exactly as it was. No partial content, no + * zero-length window. Implemented by writing to a temporary file in the + * pinned parent directory and renaming over the target, with a write + * loop that does not treat a short write as success. + * + * C1 is unconditional. C2 covers I/O failure after authorization; it does not + * promise anything about an adversary who is *also* racing the rename. + * + * ═══ WHY DESCRIPTOR-RELATIVE ════════════════════════════════════════════════ + * + * A path is a *query*, re-evaluated by the kernel on every syscall, and the + * adversary controls what it resolves to between one call and the next. A + * descriptor is a *reference* to one object. Any check made against a path and + * then acted on by path is inherently a race; that is the entire defect class, + * and no amount of re-verification closes it, because re-verification is itself + * path-based. + * + * So: resolve every component **relative to the descriptor of the component + * before it**, starting from a descriptor for the root captured once at + * construction. The adversary can still swap `docs` on disk, but our `docs` + * descriptor keeps pointing at the directory we validated, and the next + * component is opened relative to *that*, not to a path that now means + * something else. + * + * Node 22 exposes no `openat(2)`. Two resolution modes result, and the + * difference is a real difference in guarantee, not an implementation detail: + * + * - `descriptor-relative` (Linux): `/proc/self/fd//` is resolved + * by the kernel *from the descriptor*, giving true `openat` semantics. The + * race class is closed. A native `openat2` binding with `RESOLVE_BENEATH | + * RESOLVE_NO_SYMLINKS | RESOLVE_NO_MAGICLINKS` would be strictly better and + * is the right answer for production; this is the portable-Node equivalent. + * + * - `pinned-path` (macOS, others): no procfs, so the final operations are + * path-based. Pinned descriptors still prevent inode reuse, so a swap is + * *detected* — but detection is not prevention, and a determined racer can + * still be acted upon between calls. **In this mode the layer is sound only + * under the additional assumption that no untrusted process can mutate the + * tree during a write.** If that assumption does not hold, the confinement + * boundary must be an OS-level one (mount namespace, jail, container with + * the root as its own mount) rather than this layer. + * + * `resolutionMode` is reported so callers and tests can assert which guarantee + * they are actually getting instead of assuming the stronger one. + */ + +import { + closeSync, + constants, + fstatSync, + lstatSync, + mkdirSync, + openSync, + realpathSync, + renameSync, + unlinkSync, + writeSync, +} from 'node:fs'; +import type { Stats } from 'node:fs'; +import { join, sep } from 'node:path'; + +export type ConfinementCode = + | 'absolute_path' + | 'empty_path' + | 'empty_segment' + | 'dot_segment' + | 'nul_byte' + | 'backslash' + | 'symlink_component' + | 'not_a_directory' + | 'symlink_target' + | 'not_regular_file' + | 'hardlink' + | 'cross_device' + | 'component_swapped' + | 'short_write' + | 'root_unresolvable'; + +export class ConfinementError extends Error { + constructor( + readonly code: ConfinementCode, + message: string, + ) { + super(message); + this.name = 'ConfinementError'; + } +} + +const fail: (code: ConfinementCode, message: string) => never = (code, message) => { + throw new ConfinementError(code, message); +}; + +/** Identity of a filesystem object — what containment is tested on. */ +interface NodeIdentity { + dev: number; + ino: number; +} + +const identityOf = (st: Stats): NodeIdentity => ({ dev: Number(st.dev), ino: Number(st.ino) }); + +const sameIdentity = (a: NodeIdentity, b: NodeIdentity): boolean => + a.dev === b.dev && a.ino === b.ino; + +export type ResolutionMode = 'descriptor-relative' | 'pinned-path'; + +/** True where `/proc/self/fd` gives us kernel-side descriptor-relative resolution. */ +const PROCFS_AVAILABLE = process.platform === 'linux'; + +export interface WriteHooks { + /** + * Fires after resolution, immediately before the final open. Exists so the + * TOCTOU cases are deterministic rather than sleep-and-hope. Never set in + * production. + */ + beforeOpen?: (resolvedPath: string) => void; + /** Fires after the descriptor has passed validation, before content is written. */ + afterValidate?: (fd: number) => void; +} + +export interface ConfinedWrite { + relativePath: string; + bytesWritten: number; + /** Which guarantee this write actually got. See the threat model. */ + resolutionMode: ResolutionMode; +} + +/** A directory we have validated and hold open. */ +interface PinnedDir { + fd: number; + path: string; + identity: NodeIdentity; +} + +export class ConfinedRoot { + private readonly root: string; + private readonly rootIdentity: NodeIdentity; + + /** Held for the lifetime of the object: the anchor every walk starts from. */ + private readonly rootFd: number; + + readonly resolutionMode: ResolutionMode; + + constructor(rootPath: string) { + let resolved: string; + try { + resolved = realpathSync(rootPath); + } catch (err) { + fail('root_unresolvable', `confinement root does not resolve: ${(err as Error).message}`); + } + this.root = resolved; + + // The root itself being a symlink is legitimate and must keep working + // (`/var` -> `/private/var` on Darwin). It is resolved exactly once, here, + // under our control, before any adversary-influenced component appears. + this.rootFd = openSync(this.root, constants.O_RDONLY | constants.O_DIRECTORY); + this.rootIdentity = identityOf(fstatSync(this.rootFd)); + this.resolutionMode = PROCFS_AVAILABLE ? 'descriptor-relative' : 'pinned-path'; + } + + get path(): string { + return this.root; + } + + /** Release the root anchor. The object is unusable afterwards. */ + close(): void { + try { + closeSync(this.rootFd); + } catch { + /* already closed */ + } + } + + /** + * A path that the kernel resolves relative to `dirFd`, when the platform can. + * Falls back to a plain join, which is the weaker `pinned-path` mode. + */ + private at(dir: PinnedDir, name: string): string { + return PROCFS_AVAILABLE ? `/proc/self/fd/${dir.fd}/${name}` : join(dir.path, name); + } + + /** + * Write `contents` to `requestPath` relative to the root. + * + * Throws ConfinementError and mutates nothing (contract C1) if the path is not + * provably safe. + */ + writeFile(requestPath: string, contents: string, hooks: WriteHooks = {}): ConfinedWrite { + const segments = this.validateRequestPath(requestPath); + const pinned: PinnedDir[] = []; + try { + return this.writeConfined(segments, contents, hooks, pinned); + } finally { + for (const dir of pinned.splice(0)) { + try { + closeSync(dir.fd); + } catch { + /* already closed */ + } + } + } + } + + private writeConfined( + segments: string[], + contents: string, + hooks: WriteHooks, + pinned: PinnedDir[], + ): ConfinedWrite { + // ── walk, descriptor-relative ──────────────────────────────────────────── + // Each component is opened relative to the descriptor of the one before it. + // `mkdirSync(recursive)` is never used: it traverses and creates *through* a + // symlinked component. + let dir: PinnedDir = { fd: this.rootFd, path: this.root, identity: this.rootIdentity }; + + for (const segment of segments.slice(0, -1)) { + const childPath = this.at(dir, segment); + + const existing = this.lstatOrNull(childPath); + if (existing === null) { + mkdirSync(childPath); // one component, no recursion + } else if (existing.isSymbolicLink()) { + fail( + 'symlink_component', + `path component is a symlink and is refused rather than followed: ${segment}`, + ); + } else if (!existing.isDirectory()) { + fail('not_a_directory', `path component is not a directory: ${segment}`); + } + + // O_NOFOLLOW: cannot open a symlink that appeared since the lstat. + // O_DIRECTORY: cannot open anything that is no longer a directory. + // Opening relative to `dir.fd` is what makes this immune to `dir` itself + // being swapped on disk after we pinned it. + let childFd: number; + try { + childFd = openSync( + childPath, + constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW, + ); + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code === 'ELOOP') { + return fail('symlink_component', `path component became a symlink: ${segment}`); + } + if (code === 'ENOTDIR') { + return fail('not_a_directory', `path component is not a directory: ${segment}`); + } + throw err; + } + + const childStat = fstatSync(childFd); + const child: PinnedDir = { + fd: childFd, + path: join(dir.path, segment), + identity: identityOf(childStat), + }; + pinned.push(child); + + if (child.identity.dev !== this.rootIdentity.dev) { + fail('cross_device', `path component is on a different device: ${segment}`); + } + + dir = child; + } + + const finalName = segments[segments.length - 1]!; + const targetPath = this.at(dir, finalName); + + hooks.beforeOpen?.(targetPath); + + // ── inspect the target WITHOUT creating or modifying it ────────────────── + // + // The target is not opened for write, not created, and not truncated at + // this stage. An earlier version created an O_EXCL placeholder here to + // learn whether the file was new — which is itself an externally observable + // mutation, and an adversary can hardlink that placeholder before the + // refusal removes it. Contract C1 says a refusal mutates nothing, so the + // target is untouched until the atomic rename at the very end. + const existing = this.lstatOrNull(targetPath); + + if (existing !== null) { + if (existing.isSymbolicLink()) { + fail('symlink_target', `target is a symlink and is refused rather than followed: ${finalName}`); + } + + // Validate the *descriptor*, opened read-only so inspection cannot + // destroy anything: O_TRUNC in an open is a mutation that happens before + // any check can run. + let probe: number; + try { + probe = openSync(targetPath, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); + } catch (err) { + return this.translateOpenError(err as NodeJS.ErrnoException, finalName); + } + try { + const st = fstatSync(probe); + if (!st.isFile()) { + fail('not_regular_file', `target is not a regular file: ${finalName}`); + } + if (st.nlink !== 1) { + // A hardlink inside the root to a file outside it is + // path-indistinguishable from a legitimate file, and realpath cannot + // help — a hardlink has no target to resolve. + fail('hardlink', `target has ${st.nlink} links; refusing to write through a hardlink`); + } + if (Number(st.dev) !== this.rootIdentity.dev) { + fail('cross_device', 'target is on a different device than the root'); + } + } finally { + closeSync(probe); + } + } + + hooks.afterValidate?.(-1); + + // Re-check the link count immediately before committing: a hardlink can be + // created after validation. + if (existing !== null) { + const now = this.lstatOrNull(targetPath); + if (now !== null && now.nlink !== 1) { + fail('hardlink', `target gained ${now.nlink} links after validation`); + } + } + + // ── re-verify the pinned walk ─────────────────────────────────────────── + // In descriptor-relative mode this is belt-and-braces. In pinned-path mode + // it is the actual detection mechanism, and is sound only because each + // component's inode is pinned by an open descriptor and therefore cannot be + // freed and recycled for the adversary's replacement. + for (const step of pinned) { + const now = this.lstatOrNull(step.path); + if (now === null || !sameIdentity(identityOf(now), step.identity)) { + fail( + 'component_swapped', + `path component changed identity between resolution and open: ${step.path}`, + ); + } + } + + // ── commit (contract C2) ──────────────────────────────────────────────── + // Temp file in the *pinned parent*, fully written, then renamed over the + // target. `rename` replaces the name atomically and does not follow a final + // symlink, so there is no window in which the target is empty or partial. + return { + relativePath: segments.join('/'), + bytesWritten: this.commit(dir, finalName, contents), + resolutionMode: this.resolutionMode, + }; + } + + /** + * Write via a temporary sibling and rename into place. + * + * The temp name is derived from the target and cleaned up on every failure + * path, so a failed write leaves neither a partial target nor debris. + */ + private commit(dir: PinnedDir, finalName: string, contents: string): number { + const tempName = `.${finalName}.c5tmp-${process.pid}`; + const tempPath = this.at(dir, tempName); + const targetPath = this.at(dir, finalName); + + const tempFd = openSync( + tempPath, + constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL | constants.O_NOFOLLOW, + 0o644, + ); + + const payload = Buffer.from(contents, 'utf8'); + try { + // A single writeSync can legally write fewer bytes than asked. Treating + // its return value as success silently truncates content. + let written = 0; + while (written < payload.length) { + const n = writeSync(tempFd, payload, written, payload.length - written); + if (n <= 0) { + fail('short_write', `write stalled after ${written} of ${payload.length} bytes`); + } + written += n; + } + closeSync(tempFd); + + // Atomic replace. The placeholder created by O_EXCL above (if any) is + // replaced by this rename, so it needs no separate cleanup. + renameSync(tempPath, targetPath); + return written; + } catch (err) { + try { + closeSync(tempFd); + } catch { + /* already closed */ + } + try { + unlinkSync(tempPath); + } catch { + /* best effort */ + } + throw err; + } + } + + private translateOpenError(err: NodeJS.ErrnoException, finalName: string): never { + if (err.code === 'ELOOP') { + return fail('symlink_target', `target is a symlink and is refused rather than followed: ${finalName}`); + } + if (err.code === 'ENXIO' || err.code === 'ENODEV') { + // O_NONBLOCK turns "block forever waiting for a fifo reader" into this. + return fail('not_regular_file', `target is not a regular file: ${finalName}`); + } + if (err.code === 'EISDIR') { + return fail('not_regular_file', `target is a directory: ${finalName}`); + } + throw err; + } + + /** + * Reject before resolving. No `resolve()`, no normalization, no attempt to + * decide containment by comparing strings — case-insensitive filesystems make + * string comparison wrong in both directions. Containment is decided by + * descriptor-relative traversal, above. + */ + private validateRequestPath(requestPath: string): string[] { + if (requestPath === '') fail('empty_path', 'path is empty'); + if (requestPath.includes('\0')) fail('nul_byte', 'path contains a NUL byte'); + if (requestPath.includes('\\')) { + fail('backslash', "path contains a backslash; '/' is the only separator"); + } + if (requestPath.startsWith('/') || (sep === '\\' && /^[a-zA-Z]:/.test(requestPath))) { + fail('absolute_path', `path is absolute and would escape the root: ${requestPath}`); + } + + const segments = requestPath.split('/'); + for (const segment of segments) { + if (segment === '') fail('empty_segment', `path has an empty segment: ${requestPath}`); + if (segment === '.' || segment === '..') { + fail('dot_segment', `dot-segments are refused, never resolved: ${requestPath}`); + } + } + return segments; + } + + private lstatOrNull(p: string): Stats | null { + return lstatSync(p, { throwIfNoEntry: false }) ?? null; + } +} diff --git a/packages/policy/src/index.ts b/packages/policy/src/index.ts index 7e04ca63f..6e56a4dbc 100644 --- a/packages/policy/src/index.ts +++ b/packages/policy/src/index.ts @@ -19,3 +19,12 @@ export { } from './agent-policy.js'; export { createCloudPolicyFetcher } from './cloud-policy-fetcher.js'; + +export { + ConfinedRoot, + ConfinementError, + type ConfinedWrite, + type ConfinementCode, + type ResolutionMode, + type WriteHooks, +} from './fs-confine.js'; From 045e237349b6277b84ddbaef49e36d1f874d2205 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 31 Jul 2026 18:36:42 +0000 Subject: [PATCH 2/2] style: auto-format with Prettier --- packages/policy/src/fs-confine.test.ts | 10 +++++----- packages/policy/src/fs-confine.ts | 18 +++++++----------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/packages/policy/src/fs-confine.test.ts b/packages/policy/src/fs-confine.test.ts index 26c8026cd..1dba9b55f 100644 --- a/packages/policy/src/fs-confine.test.ts +++ b/packages/policy/src/fs-confine.test.ts @@ -195,7 +195,7 @@ describe('ConfinedRoot — concurrent mutation', () => { expectRefusal(s, 'symlink_target', () => cr.writeFile('notes.md', 'PWNED', { beforeOpen: () => symlinkSync(s.victim, join(s.root, 'notes.md')), - }), + }) ); }); @@ -209,7 +209,7 @@ describe('ConfinedRoot — concurrent mutation', () => { rmSync(join(s.root, 'docs'), { recursive: true, force: true }); symlinkSync(s.outside, join(s.root, 'docs')); }, - }), + }) ); }); @@ -230,7 +230,7 @@ describe('ConfinedRoot — concurrent mutation', () => { rmSync(join(s.root, 'docs'), { recursive: true, force: true }); symlinkSync(s.outside, join(s.root, 'docs')); }, - }), + }) ); expect(readFileSync(join(s.outside, 'notes.md'), 'utf8')).toBe('VICTIM FILE'); }); @@ -249,7 +249,7 @@ describe('ConfinedRoot — concurrent mutation', () => { symlinkSync(s.outside, join(shim, 'b')); symlinkSync(shim, join(s.root, 'a')); }, - }), + }) ); }); @@ -260,7 +260,7 @@ describe('ConfinedRoot — concurrent mutation', () => { expectRefusal(s, 'hardlink', () => cr.writeFile('late.md', 'PWNED', { afterValidate: () => linkSync(join(s.root, 'late.md'), join(s.outside, 'late-link.md')), - }), + }) ); }); diff --git a/packages/policy/src/fs-confine.ts b/packages/policy/src/fs-confine.ts index a803f0b93..3f9eb806f 100644 --- a/packages/policy/src/fs-confine.ts +++ b/packages/policy/src/fs-confine.ts @@ -122,7 +122,7 @@ export type ConfinementCode = export class ConfinementError extends Error { constructor( readonly code: ConfinementCode, - message: string, + message: string ) { super(message); this.name = 'ConfinementError'; @@ -141,8 +141,7 @@ interface NodeIdentity { const identityOf = (st: Stats): NodeIdentity => ({ dev: Number(st.dev), ino: Number(st.ino) }); -const sameIdentity = (a: NodeIdentity, b: NodeIdentity): boolean => - a.dev === b.dev && a.ino === b.ino; +const sameIdentity = (a: NodeIdentity, b: NodeIdentity): boolean => a.dev === b.dev && a.ino === b.ino; export type ResolutionMode = 'descriptor-relative' | 'pinned-path'; @@ -247,7 +246,7 @@ export class ConfinedRoot { segments: string[], contents: string, hooks: WriteHooks, - pinned: PinnedDir[], + pinned: PinnedDir[] ): ConfinedWrite { // ── walk, descriptor-relative ──────────────────────────────────────────── // Each component is opened relative to the descriptor of the one before it. @@ -264,7 +263,7 @@ export class ConfinedRoot { } else if (existing.isSymbolicLink()) { fail( 'symlink_component', - `path component is a symlink and is refused rather than followed: ${segment}`, + `path component is a symlink and is refused rather than followed: ${segment}` ); } else if (!existing.isDirectory()) { fail('not_a_directory', `path component is not a directory: ${segment}`); @@ -276,10 +275,7 @@ export class ConfinedRoot { // being swapped on disk after we pinned it. let childFd: number; try { - childFd = openSync( - childPath, - constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW, - ); + childFd = openSync(childPath, constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW); } catch (err) { const code = (err as NodeJS.ErrnoException).code; if (code === 'ELOOP') { @@ -375,7 +371,7 @@ export class ConfinedRoot { if (now === null || !sameIdentity(identityOf(now), step.identity)) { fail( 'component_swapped', - `path component changed identity between resolution and open: ${step.path}`, + `path component changed identity between resolution and open: ${step.path}` ); } } @@ -405,7 +401,7 @@ export class ConfinedRoot { const tempFd = openSync( tempPath, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL | constants.O_NOFOLLOW, - 0o644, + 0o644 ); const payload = Buffer.from(contents, 'utf8');