From e49b03a7188ffbbd7bd5cc54e43d74329ecda3d3 Mon Sep 17 00:00:00 2001 From: mchwang Date: Thu, 1 Oct 2026 17:47:09 -0700 Subject: [PATCH 1/5] D: empty read-only mounts at gitlink paths and a pre-launch tree check (#81) checkTaskTree runs the no-follow inspection with no declared links just before an execute or fix launch. It refuses (TaskTreeRefused) unless the work tree is exactly the tree of the recorded head, and each declared operation fits that tree: add and rename destinations free, sources a file or symlink, no declared path beneath a file, symlink or gitlink. It also refuses a gitlink it cannot mount (over 256, or a name with a comma or quote), and a tree it cannot read whole. The result is a one-shot capability: an execute or fix profile requires a check made for its own storage and head, mounts an empty read-only tmpfs at every gitlink, and the validator allows exactly those mounts. F2b's prepare runs the check last; a refusal is a safety finding. A parent of a nested gitlink can still be renamed during the run; the post-run inspection catches it. plan-format.md records this, and #99 tracks enforcing it during the run. Co-Authored-By: Claude Opus 5.5 --- agents/adapters/types.ts | 3 + agents/container/changes.ts | 159 ++++++++++++++++++++++++ agents/container/profile.ts | 22 +++- agents/container/run.ts | 20 ++- agents/container/tree-script.ts | 26 +++- agents/policy.ts | 10 +- docs/implementation/agent-isolation.md | 26 ++++ docs/implementation/runner-lifecycle.md | 2 +- docs/plan-format.md | 2 +- runner/execution.ts | 31 ++++- runner/workspace.ts | 5 +- test/agent-container.test.ts | 148 +++++++++++++++++++++- test/runner-execution.test.ts | 51 ++++++-- 13 files changed, 472 insertions(+), 33 deletions(-) diff --git a/agents/adapters/types.ts b/agents/adapters/types.ts index dd580309..62d204d3 100644 --- a/agents/adapters/types.ts +++ b/agents/adapters/types.ts @@ -1,5 +1,6 @@ import type { InvocationInput } from '../contract.ts'; import type { TaskFilesystems } from '../container/storage.ts'; +import type { TaskTreeCheck } from '../container/changes.ts'; import { assertBuiltAgentImage } from '../container/image.ts'; import { assertResourceOwner } from '../labels.ts'; import type { CaptureLimits } from './supervisor.ts'; @@ -15,6 +16,8 @@ export interface AgentAdapterRequest { * call so recovery can match them to the attempt (#51 item 3). */ readonly networkAllocationId: string; + /** Execute and fix only, and required there: `checkTaskTree`'s result for `filesystems`, made just before this start. */ + readonly treeCheck?: TaskTreeCheck; } /** Check a start call's synchronous input, so invalid input throws before a handle exists. */ export function assertAdapterRequest(request: AgentAdapterRequest): void { diff --git a/agents/container/changes.ts b/agents/container/changes.ts index 43c9e15f..17d7a245 100644 --- a/agents/container/changes.ts +++ b/agents/container/changes.ts @@ -1,5 +1,6 @@ import { constants } from 'node:buffer'; import { createHash } from 'node:crypto'; +import { DockerError } from '../docker.ts'; import { resolveMetadataBaseline, runStorageScript, type RecoveredTaskStorage, type StorageScriptOptions, type TaskFilesystems } from './storage.ts'; import { MAXIMUM_CHANGES, MAXIMUM_DECLARED_LINKS, MAXIMUM_NAME_BYTES, MAXIMUM_TARGET_ENTRIES, MAXIMUM_TREE_OUTPUT, @@ -350,6 +351,164 @@ export async function inspectTaskChanges(storage: TaskFilesystems | RecoveredTas return manifestOf(options.base, input, JSON.parse(stdout)); } +/** One file operation a plan item declares, as the plan names it (`renamedFrom` only for a rename). */ +export interface DeclaredOperation { + readonly kind: 'add' | 'edit' | 'delete' | 'rename'; + readonly path: string; + readonly renamedFrom?: string | null; +} +export interface TreeCheckOptions extends StorageScriptOptions { + /** The recorded head the storage was seeded from: the clone's head. */ + readonly base: string; + /** The item's file operations, checked against the tree of `base`. */ + readonly operations: readonly DeclaredOperation[]; + /** As for `inspectTaskChanges`. */ + readonly metadataBaseline?: string; +} +/** + * What `checkTaskTree` found, for the execute or fix profile of the same storage. Each check is one-shot: one profile + * uses it, and the next invocation needs a new check. + */ +export interface TaskTreeCheck { + readonly base: string; + /** Every gitlink of `base`, each an empty directory now; each gets an empty read-only mount. */ + readonly gitlinks: readonly string[]; +} +/** + * The item must not launch on this storage: the tree is not a clean copy of the head, an operation does not fit the + * head, or a gitlink cannot be mounted. Each entry of `differences` says why. A person must look; retrying the same + * head and plan gives the same answer. + */ +export class TaskTreeRefused extends Error { + readonly differences: readonly string[]; + constructor(differences: readonly string[]) { + const shown = differences.slice(0, 10).join('; '); + super(`The pre-launch tree check refused: ${shown}${differences.length > 10 + ? `; and ${differences.length - 10} more` : ''}.`); + this.name = 'TaskTreeRefused'; + this.differences = Object.freeze([...differences]); + } +} +/** The most gitlinks one profile mounts; a head with more refuses the launch. */ +export const MAXIMUM_GITLINK_MOUNTS = 256; +// The storage each check was made over, and whether a profile has used it. +const treeChecks = new WeakMap(); +// Docker reads `--mount` as comma-separated fields that a double quote can open: a name with either cannot be mounted +// exactly where it is. +const unmountable = (path: string) => /[,"]/.test(path); +const KINDS = new Set(['add', 'edit', 'delete', 'rename']); +interface DeclaredFact { readonly path: string; readonly type: EntryType | 'absent'; readonly blockedBy?: string; + readonly blockedByType?: EntryType } + +// Why the storage is not a fresh copy of `base`: every change, nested content, commit or metadata difference. +function treeDifferences(manifest: TaskChangeManifest): string[] { + if (manifest.metadataChanged) return ['the Git metadata differs from what the seeder recorded']; + const q = JSON.stringify, found: string[] = []; + if (manifest.agentCommits.length) found.push('HEAD is not the recorded head'); + for (const change of manifest.changes) { + if (change.kind === 'add') found.push(`${q(change.path)} is a ${change.newType} the head does not have`); + else if (change.kind === 'delete') found.push(`${q(change.path)} is missing`); + else if (change.kind === 'rename') found.push(`${q(change.path)} is ${q(change.oldPath)} moved`); + else if (change.oldType !== change.newType) found.push(`${q(change.path)} is a ${change.newType} where the head has a ${change.oldType}`); + else found.push(`${q(change.path)} differs from the head`); + } + for (const path of manifest.nestedGitlinkContent) found.push(`gitlink ${q(path)} has content or cannot be read`); + return found; +} + +/** + * Check task storage immediately before an execute or fix invocation (plan-format.md, "Clean invocation state" and + * "Submodules in version 1"): read without following links, the work tree must be exactly the tree of `base` (no + * untracked or ignored entry, no changed type or content, nothing in a gitlink directory, HEAD at `base`, metadata as + * seeded), and each declared operation must fit that tree: an add or rename destination is free, an edit, delete or + * rename source is a file or symlink, and no declared path lies beneath a file, symlink or gitlink. It refuses with + * `TaskTreeRefused` otherwise, and when a gitlink cannot be given its empty read-only mount (more than + * `MAXIMUM_GITLINK_MOUNTS`, or a name Docker cannot take). Runs the inspection's read-only, no-network container. + * The result is the capability `createContainerProfile` needs for an execute or fix profile over this storage. + */ +export async function checkTaskTree(storage: TaskFilesystems, options: TreeCheckOptions): Promise { + if (typeof options.base !== 'string' || !COMMIT_ID.test(options.base)) throw new Error('base must be a full commit ID.'); + const baseline = resolveMetadataBaseline(storage, options.metadataBaseline); + if (!Array.isArray(options.operations)) throw new Error('Declared operations must be a list.'); + const operations = options.operations.map(operation => { + const { kind, path, renamedFrom } = operation ?? {} as DeclaredOperation; + if (!KINDS.has(kind)) throw new Error(`Declared operation kind ${JSON.stringify(kind)} is not add, edit, delete or rename.`); + assertDeclaredPath(path); + if (kind === 'rename') assertDeclaredPath(renamedFrom); + else if (renamedFrom !== undefined && renamedFrom !== null) throw new Error('Only a rename has renamedFrom.'); + return { kind, path, renamedFrom: kind === 'rename' ? renamedFrom! : undefined }; + }); + const paths = [...new Set(operations.flatMap(operation => [operation.path, ...operation.renamedFrom ? [operation.renamedFrom] : []]))]; + if (paths.length > MAXIMUM_DECLARED_LINKS) throw new Error(`At most ${MAXIMUM_DECLARED_LINKS} declared paths are checked.`); + assertArguments(paths); + let stdout: string; + try { + stdout = await runStorageScript(storage, { kind: 'inspect', operation: 'Pre-launch tree check', + consequence: 'the task tree cannot be checked', entrypoint: 'perl', maxOutputBytes: MAXIMUM_TREE_OUTPUT + OUTPUT_SLACK, + memory: INSPECTION_MEMORY, tmpBytes: INSPECTION_TMP, + args: ['-e', TREE_SCRIPT, 'prelaunch', baseline, options.base, ...paths] }, + { ...options, timeoutMs: options.timeoutMs ?? 120_000 }); + } catch (error) { + // The script found the tree itself unfit: something it cannot read (6), a name the manifest cannot carry (8), or + // too many changes or too much output (9). A fresh copy of the head has none of these, so each is a refusal like + // any other difference, not a failure to run. Its message escapes every name it shows. + if (error instanceof DockerError && [6, 8, 9].includes(error.status ?? -1)) { + const reason = error.stderr.trim().split('\n').at(-1)!.slice(0, 512); + throw new TaskTreeRefused([`the tree cannot be checked whole (${JSON.stringify(reason)})`]); + } + throw error; + } + const output = JSON.parse(stdout) as InspectOutput & { metadataOnly?: boolean; gitlinks?: unknown; declared?: unknown }; + const manifest = manifestOf(options.base, { baseline, links: [], recorded: {}, linkArgs: [] }, output); + const differences = treeDifferences(manifest); + if (manifest.metadataChanged) throw new TaskTreeRefused(differences); + const gitlinks = output.gitlinks, declared = output.declared as DeclaredFact[] | undefined; + if (!Array.isArray(gitlinks) || gitlinks.some(path => { try { assertDeclaredPath(path); return false; } catch { return true; } }) + || !Array.isArray(declared) + || declared.length !== paths.length || declared.some((fact, index) => fact?.path !== paths[index])) + throw new Error('The pre-launch tree check returned an unexpected result.'); + const facts = new Map(declared.map(fact => [fact.path, fact])); + const q = JSON.stringify; + for (const operation of operations) { + const sources = operation.kind === 'add' ? [] : [operation.renamedFrom ?? operation.path]; + const destinations = operation.kind === 'add' || operation.kind === 'rename' ? [operation.path] : []; + for (const path of [...sources, ...destinations]) { + const fact = facts.get(path)!; + if (fact.blockedBy !== undefined) differences.push(`${q(path)} lies beneath the ${fact.blockedByType} ${q(fact.blockedBy)}`); + } + for (const path of sources) { + const type = facts.get(path)!.type; + if (type === 'gitlink') differences.push(`${q(path)} is a gitlink, which a plan item cannot change`); + else if (type !== 'file' && type !== 'symlink') differences.push(`${operation.kind} source ${q(path)} is ${type === 'absent' ? 'missing' : `a ${type}`}`); + } + for (const path of destinations) { + const type = facts.get(path)!.type; + if (type !== 'absent') differences.push(`${operation.kind} destination ${q(path)} is occupied by a ${type}`); + } + } + if (gitlinks.length > MAXIMUM_GITLINK_MOUNTS) + differences.push(`the head has ${gitlinks.length} gitlinks, more than the ${MAXIMUM_GITLINK_MOUNTS} that can be mounted`); + for (const path of gitlinks as string[]) if (unmountable(path)) differences.push(`gitlink ${q(path)} has a name Docker cannot mount`); + if (differences.length) throw new TaskTreeRefused([...new Set(differences)]); + const check = Object.freeze({ base: options.base, gitlinks: Object.freeze([...gitlinks as string[]]) }); + treeChecks.set(check, { filesystems: storage, used: false }); + return check; +} + +/** + * For the profile builder: the gitlinks of a check `checkTaskTree` made over these filesystems at `base`, marking it + * used. Refuses a copied, reused or mismatched check. + */ +export function useTaskTreeCheck(check: TaskTreeCheck | undefined, filesystems: TaskFilesystems, base: string): readonly string[] { + const record = check ? treeChecks.get(check) : undefined; + if (!record) throw new Error('An execute or fix profile requires a pre-launch tree check from checkTaskTree.'); + if (record.filesystems !== filesystems || check!.base !== base) + throw new Error('The pre-launch tree check was made over other task storage or another head.'); + if (record.used) throw new Error('The pre-launch tree check was already used; check the tree again before each invocation.'); + record.used = true; + return check!.gitlinks; +} + /** The most bundle bytes a runner commit returns; a larger one fails the commit. */ export const MAXIMUM_BUNDLE_BYTES = 64 * 1024 * 1024; const MAXIMUM_MESSAGE_BYTES = 64 * 1024; diff --git a/agents/container/profile.ts b/agents/container/profile.ts index f654dba3..cefeb13b 100644 --- a/agents/container/profile.ts +++ b/agents/container/profile.ts @@ -6,6 +6,7 @@ import { join } from 'node:path'; import { assertCapturedInvocation, type InvocationInput, type Phase, type UnreleasedResource } from '../contract.ts'; import { assertBuiltAgentImage } from './image.ts'; import { assertTaskFilesystems, taskFilesystemOwner, type TaskFilesystems } from './storage.ts'; +import { useTaskTreeCheck, type TaskTreeCheck } from './changes.ts'; import { ownerLabelArgs, type ResourceOwner } from '../labels.ts'; import { assertVendorNetwork, removeVendorNetwork, vendorNetworkResources, type VendorNetwork } from '../network/network.ts'; @@ -24,6 +25,8 @@ export interface ContainerProfile { readonly network: VendorNetwork; readonly policy: PhasePolicy; readonly deferredOutput: boolean; + /** Execute and fix: every gitlink path, each covered by an empty read-only tmpfs. Empty in read-only phases. */ + readonly gitlinks: readonly string[]; } export interface ProfileOptions { readonly invocation: InvocationInput; @@ -36,6 +39,11 @@ export interface ProfileOptions { readonly network: VendorNetwork; readonly policy: PhasePolicy; readonly deferredOutput?: boolean; + /** + * Execute and fix only, and required there: what `checkTaskTree` returned for these filesystems at the clone's head, + * just before this launch. Each check serves one profile. + */ + readonly treeCheck?: TaskTreeCheck; /** Remaining invocation budget for Docker-backed profile validation. */ readonly timeoutMs?: number; /** Cancels creation; whatever was staged is removed before the promise rejects. */ @@ -214,6 +222,9 @@ const safeName = (value: string) => { const prefix = value.replace(/[^a-zA-Z0-9_.-]/g, '-').slice(0, 24); return `${prefix}-${createHash('sha256').update(value).digest('hex').slice(0, 16)}`; }; +/** The size and mode of the empty read-only tmpfs at each gitlink path. */ +export const GITLINK_MOUNT_BYTES = 4096; +export const GITLINK_MOUNT_MODE = '0555'; const mount = (parts: Record) => Object.entries(parts) .map(([key, value]) => value === true ? key : `${key}=${value}`).join(','); const mountSource = (path: string, kind: string) => { @@ -277,6 +288,10 @@ export async function createContainerProfile(options: ProfileOptions): Promise ['--mount', mount({ type: 'tmpfs', target: `/work/${path}`, readonly: true, + 'tmpfs-mode': GITLINK_MOUNT_MODE, 'tmpfs-size': String(GITLINK_MOUNT_BYTES) })])]; if (options.deferredOutput) { if (invocation.vendor !== 'codex') throw new Error('Deferred output is available only for Codex.'); args.push('--env', 'CODEBOOST_DEFERRED_OUTPUT=1', @@ -311,7 +329,7 @@ export async function createContainerProfile(options: ProfileOptions): Promise | null; CgroupParent: string; RestartPolicy?: { Name?: string; MaximumRetryCount?: number } | null; Runtime: string; Devices: unknown[] | null; DeviceRequests: unknown[] | null; Tmpfs: Record | null; - Mounts: Array<{ Type: string; Source: string; Target: string; ReadOnly: boolean }> | null; Dns: string[]; + Mounts: Array<{ Type: string; Source: string; Target: string; ReadOnly: boolean; + TmpfsOptions?: { SizeBytes?: number; Mode?: number; Options?: unknown } }> | null; Dns: string[]; DnsOptions: string[]; DnsSearch: string[]; ExtraHosts: string[] | null; PortBindings: Record | null; PublishAllPorts: boolean }; Mounts: Array<{ Type: string; Name?: string; Source: string; Destination: string; RW: boolean }>; @@ -254,10 +255,23 @@ export async function validateContainer(container: string, profile: ContainerPro if (!hasExactOptions(tmpfs[path], expected)) throw new Error(`Container tmpfs ${path} options changed.`); } const mounts = new Map(inspect.Mounts.map(item => [item.Destination, item])); - const allowedMounts = new Set(['/work', '/work/.git', '/run/codeboost-input', + const gitlinkMounts = profile.gitlinks.map(path => `/work/${path}`); + const allowedMounts = new Set(['/work', '/work/.git', '/run/codeboost-input', ...gitlinkMounts, ...(profile.vendor === 'codex' ? ['/run/codeboost-auth/codex/auth.json'] : [])]); if (inspect.Mounts.some(item => !allowedMounts.has(item.Destination))) throw new Error('Container includes an unexpected external mount.'); + // Every gitlink is covered by its own empty, read-only, bounded tmpfs, exactly as the profile asked. + for (const target of gitlinkMounts) { + const actual = inspect.Mounts.filter(item => item.Destination === target); + const requested = (host.Mounts ?? []).filter(item => item.Target === target); + if (actual.length !== 1 || actual[0]!.Type !== 'tmpfs' || actual[0]!.RW || requested.length !== 1 + || requested[0]!.Type !== 'tmpfs' || !requested[0]!.ReadOnly + || requested[0]!.TmpfsOptions?.SizeBytes !== GITLINK_MOUNT_BYTES + || requested[0]!.TmpfsOptions?.Mode !== Number.parseInt(GITLINK_MOUNT_MODE, 8) + || ![undefined, null].includes(requested[0]!.TmpfsOptions?.Options as null | undefined) + && !(Array.isArray(requested[0]!.TmpfsOptions?.Options) && (requested[0]!.TmpfsOptions!.Options as unknown[]).length === 0)) + throw new Error('A gitlink path is not covered by its empty read-only mount.'); + } const work = mounts.get('/work'), metadata = mounts.get('/work/.git'), input = mounts.get('/run/codeboost-input'); if (work?.Type !== 'volume' || work.RW !== ['execute', 'fix'].includes(profile.phase) || metadata?.Type !== 'volume' || metadata.RW || input?.Type !== 'bind' || input.RW) diff --git a/agents/container/tree-script.ts b/agents/container/tree-script.ts index 690ff004..2d3e620f 100644 --- a/agents/container/tree-script.ts +++ b/agents/container/tree-script.ts @@ -1,5 +1,5 @@ /** - * The Perl program D runs inside task storage to read it without following links (#66). One program serves five modes, + * The Perl program D runs inside task storage to read it without following links (#66). One program serves six modes, * so the seeder's checks and a later inspection read storage the same way: * * - `links ` (run by the seeder, as root, over what it copied) refuses, with exit 11, any link in the @@ -13,6 +13,8 @@ * - `inspect ... ...` (in /work) compares the work tree with the tree of * `base`, hashing every file as a commit would store it, resolves each declared link again (without walking its * target), and reports the state now of each target the snapshot recorded, the metadata digest and HEAD. + * - `prelaunch ...` (in /work) runs the inspection with no declared links, and also reports + * every gitlink of `base` and, for each declared path, what `base` has there and any leaf of `base` on its way. * - `commit ... ...` (in /work) runs the inspection, then, * from the same reads, builds the runner commit on top of `base` from base plus the changes it found. It reads the * message and identities as JSON on stdin and writes every object to a scratch object store in /tmp: both volumes @@ -22,7 +24,7 @@ * a commit cannot hold (a directory, a fifo or other special file, a path under a `.git` part, or a symlink named * `.gitmodules`); the runner refuses those. * - * Snapshot, inspect and commit first compare the metadata digest with `baseline`, before any Git command: if it differs, a snapshot refuses + * Snapshot, inspect, prelaunch and commit first compare the metadata digest with `baseline`, before any Git command: if it differs, a snapshot refuses * (exit 10) and an inspection prints only the digest and `metadataOnly`. * * Output is one JSON document on stdout, at most MAXIMUM_TREE_OUTPUT bytes (a commit adds its own line and the bundle). A reported path or link target that is not @@ -381,7 +383,7 @@ if ($mode eq "snapshot") { emit({ links => \@links, targets => { map { (text($_, "target") => target_state($_)) } keys %resolved_targets } }); } -fail(2, "unknown mode") unless $mode eq "inspect" || $mode eq "commit"; +fail(2, "unknown mode") unless $mode eq "inspect" || $mode eq "commit" || $mode eq "prelaunch"; my $base = shift @ARGV; fail(3, "base is not a full commit ID") unless $base =~ /^[0-9a-f]{40}([0-9a-f]{24})?$/; my $bundle_limit = $mode eq "commit" ? shift @ARGV : 0; @@ -393,7 +395,9 @@ my $exists = do { }; fail(4, "git rev-parse failed (" . exit_reason($?) . ")") if $? == -1 || ($? & 127) || ($? >> 8) > 1; fail(3, "base $base is not a commit in this task storage") if $exists != 0; -my $link_count = shift @ARGV; +# A pre-launch check passes no links: everything after base is a path the item declares. +my @declared = $mode eq "prelaunch" ? splice(@ARGV, 0) : (); +my $link_count = $mode eq "prelaunch" ? 0 : shift @ARGV; fail(2, "bad link count") unless defined $link_count && $link_count =~ /^\d+$/ && $link_count <= @ARGV; my @links = splice @ARGV, 0, $link_count; my @targets = @ARGV; @@ -648,6 +652,20 @@ my %targets = map { (text($_, "target") => target_state($_)) } @targets; my %result = (metadataDigest => $metadata_digest, head => $head, agentCommits => \@agent_commits, changes => \@changes, nestedGitlinkContent => [map { text($_, "path") } sort @nested], links => \@fresh, targets => \%targets); emit(\%result) if $mode eq "inspect"; +# Before launch, also every gitlink of base (each gets an empty read-only mount), and for each declared path what base +# has there and the first part on the way that base holds as a leaf (a file, symlink or gitlink), if any. The work tree +# was just compared with base, so where it matches, these are what the agent will find. +if ($mode eq "prelaunch") { + $result{gitlinks} = [map { text($_, "gitlink path") } sort grep { $base{$_}{type} eq "gitlink" } keys %base]; + $result{declared} = [map { + my $path = $_; my @parts = split m{/}, $path; pop @parts; my ($at, $blocked) = (""); + for my $part (@parts) { $at = join_path($at, $part); if ($base{$at}) { $blocked = $at; last } } + my $type = $base{$path} ? $base{$path}{type} : $base_directory{$path} ? "directory" : "absent"; + +{ path => text($path, "declared path"), type => $type, + defined $blocked ? (blockedBy => text($blocked, "path"), blockedByType => $base{$blocked}{type}) : () }; + } @declared]; + emit(\%result); +} # A commit reports the inspection as encoded now, before anything below reads it: nothing that builds the commit can # change what the runner checks. # It goes out at once, on one line (JSON::PP escapes every newline inside a string), so the build below does not hold diff --git a/agents/policy.ts b/agents/policy.ts index 4e0e083a..82f1615a 100644 --- a/agents/policy.ts +++ b/agents/policy.ts @@ -144,7 +144,7 @@ export type IsolationProbe = 'noop' | 'phase-worktree' | 'read-only-isolation' | | 'oversized-output' | 'fifo-output' | 'invalid-utf8-output' | 'invalid-utf8-stderr' | 'truncated-utf8-stderr' | 'replace-output-directory' | 'nonzero-output' | 'duplicate-protocol' | 'newline-free-deferred-output' | 'scratch-capacity' | 'metadata-alias' - | 'hostile-repo'; + | 'hostile-repo' | 'gitlink-write'; // `set -e` ignores a failing `! command`, so a negated check could never fail a probe. `deny` exits instead when a // forbidden action succeeds, and names the breach. Both streams of the attempted command are discarded, so a breach @@ -220,6 +220,14 @@ export function createIsolationProbeCommand(policy: PhasePolicy, probe: Isolatio + 'case "$resolved" in /work|/work/*) ;; ' + '*) echo "isolation breach: link leaves the checkout $link" >&2; exit 1;; esac; done\' sh {} +; ' + 'deny grep -rqs codeboost-host-secret /work /tmp "$HOME"; printf hostile-repo-contained', + // Every gitlink in the index (the head's, in a fresh copy) is its own empty read-only mount: nothing can be written beneath it, and it cannot be + // removed or moved aside. The rest of the work tree stays writable. + 'gitlink-write': `${deny}set -eu; n=0; for p in $(git ls-files --stage | awk '$1 == "160000" { print $4 }'); do ` + + 'n=$((n+1)); test "$(findmnt --noheadings --output TARGET --target "/work/$p")" = "/work/$p"; ' + + 'findmnt --noheadings --output OPTIONS --target "/work/$p" | tr , "\\n" | grep -Fxq ro; ' + + 'test -z "$(ls -A "/work/$p")"; deny touch "/work/$p/x"; deny mkdir "/work/$p/.git"; ' + + 'deny rmdir "/work/$p"; deny mv "/work/$p" "/work/$p.moved"; done; test "$n" -gt 0; ' + + 'printf ok > /work/beside.txt; printf gitlink-protected', }; return command(policy, probe === 'noop' ? ['true'] : ['sh', '-c', scripts[probe]]); } diff --git a/docs/implementation/agent-isolation.md b/docs/implementation/agent-isolation.md index 1f75a00e..cfbf5e98 100644 --- a/docs/implementation/agent-isolation.md +++ b/docs/implementation/agent-isolation.md @@ -36,6 +36,7 @@ Each row is a T9 requirement for the Docker suite. The suite fails if any row fa | Hard links and alias writes from `.git/config` and objects fail, and metadata stays unchanged | `agent-container`: metadata alias probe in planning, review and execute, with a digest of `.git` before and after | | Mountpoint replacement fails | `agent-container`: metadata and metadata alias probes (`mv` and `rm -rf` of `.git`) | | Both vendor startup probes read the schema and return bounded valid output through their documented channel | `agent-supervisor` live probes (credentials required): Codex in review through its output file; Claude in questions through its stdout envelope; Claude in planning returns a schema-constrained answer as bare JSON. **Known failure:** the Codex probes (here and in `agent-container`) fail, because Codex cannot read files in any phase (see [Codex in read-only phases](#codex-in-read-only-phases)). | +| Nothing can be written beneath a gitlink, and execute and fix launch only on a clean tree (#81) | `agent-container`: the gitlink probe (writes, `mkdir`, `rmdir` and `mv` at every gitlink fail; the rest of `/work` stays writable); a pre-populated nested checkout, a restart with an untracked symlink parent and a restart with an occupied add destination are refused; a gitlink name Docker cannot mount, more than 256 gitlinks and an unreadable tree are refused; each check serves one profile over its own storage and head; the validator rejects a gitlink mount that is missing, writable, or of another size or mode | | Hostile input stays inside the boundary | `agent-container`: repositories with links that leave the checkout are refused, links inside the checkout still work, oversized repositories fail closed; `agent-policy`: option-like prompts; `agent-proxy`: hostile CONNECT traffic; `agent-supervisor`: hostile output | ## Why the gate can fail @@ -110,6 +111,24 @@ Use only these entry points to run an agent: the target and everything beneath it. A link on the way, a target that is a link, or a link inside a directory target shows as `through-link`. Do not launch an item with a `through-link` declared link: a write through it lands somewhere its target does not cover. Keep the result. + - **Just before launch (execute and fix)**, call `checkTaskTree(storage, { base, operations, imageId })` with the + item's file operations (`{ kind, path, renamedFrom }`) and `base`, the clone's head (#81). It runs the inspection + below with no declared links, and refuses with `TaskTreeRefused` unless the work tree is exactly the tree of + `base`: no untracked or ignored entry, no changed content, mode or type (a directory replaced by a symlink shows + here), nothing in a gitlink directory, `HEAD` at `base`, and the metadata as seeded. It then checks each operation + against that tree: an add or rename destination is free, an edit, delete or rename source is a file or symlink + (never a gitlink), and no declared path lies beneath a file, symlink or gitlink. It also refuses when a gitlink + cannot get its mount: more than 256 gitlinks, or a name with a comma or a double quote, which Docker's `--mount` + cannot take. A tree it cannot read whole (an unreadable entry, a name the manifest cannot carry, more than + 10,000 changes) is refused too. `differences` lists every reason, with paths quoted. A refusal gives the same + answer on retry, so a person must look. Any other error is the check failing to run. + + Pass the result as `treeCheck` in the start call's request. An execute or fix profile requires it; a read-only + phase refuses one. The profile adds an empty read-only tmpfs (`--mount type=tmpfs,target=/work/,readonly, + tmpfs-mode=0555,tmpfs-size=4096`) at every gitlink the check listed, and the validator allows exactly those mounts, + each read-only with that size and mode. A check serves one profile, over the storage and head it was made for: a + copy, a second use, other storage or another head is refused, so check again before every invocation, retries + included. - **After the handle settles**, call `inspectTaskChanges(storage, { base, linkSnapshot, imageId })`. It returns the change manifest: every difference between the work tree and `base`, read without following links, with content IDs as a commit would store them. New ignored files, fifos and entries under a `.git` part are listed; a new @@ -359,3 +378,10 @@ against real Docker, including two reviews with different Ask owners on one daem fail until Codex can read files in its phases; that failure is expected, not a regression. - T9 as a whole is complete only when lane F runs every suite in required CI (F6). +- Gitlink mounts and the pre-launch check apply to execute and fix only. In planning, questions and review, `/work` + is a read-only volume, so nothing can be written beneath a gitlink there; that includes `check` attempts, which run + as review. Running an item's `cmd` in a writable task filesystem is not built yet; when it is, each run must call + `checkTaskTree` and get the same mounts before it starts. +- The mount keeps a gitlink directory empty, but an agent can still rename a parent directory of a nested gitlink + (the mount moves with it) and create a new directory at the old path. The inspection after the run reports that + directory's content as nested gitlink content, so the task goes to needs human. #99 tracks enforcing this during the run. diff --git a/docs/implementation/runner-lifecycle.md b/docs/implementation/runner-lifecycle.md index c7d65cb7..ef4cfb53 100644 --- a/docs/implementation/runner-lifecycle.md +++ b/docs/implementation/runner-lifecycle.md @@ -207,7 +207,7 @@ The server computes `retryable` and sends it to the UI. The UI never works it ou ## Publishing a result 1. Wait for `settled`. -2. Validate the output (schema, size, file-scope audit for writable attempts). Do not await anything between the last check and the transaction. For a writable attempt, the audit reads D's change manifest (#66): F calls `snapshotDeclaredLinks` before launch and keeps the snapshot, and after `settled` calls `inspectTaskChanges` with it and the storage's `metadataBaseline`, which F recorded at allocation. Any agent commit, metadata change, change to a declared link's target, nested gitlink content, new file that base's own ignore rules ignore (#87 decision 2), or an inspection that refuses, sends the task to needs human with no tests and no commit. A `retargeted` entry for a declared link is that link's own edit when the manifest lists it, which plan-format.md permits at declared paths; F2a judges it by the plan's link rules, not as a target change. Without that edit in the manifest it is a target change. A declared link whose snapshot status is `through-link` is refused before launch. +2. Validate the output (schema, size, file-scope audit for writable attempts). Do not await anything between the last check and the transaction. For a writable attempt, the audit reads D's change manifest (#66): F calls `snapshotDeclaredLinks` before launch and keeps the snapshot, and after `settled` calls `inspectTaskChanges` with it and the storage's `metadataBaseline`, which F recorded at allocation. Any agent commit, metadata change, change to a declared link's target, nested gitlink content, new file that base's own ignore rules ignore (#87 decision 2), or an inspection that refuses, sends the task to needs human with no tests and no commit. A `retargeted` entry for a declared link is that link's own edit when the manifest lists it, which plan-format.md permits at declared paths; F2a judges it by the plan's link rules, not as a target change. Without that edit in the manifest it is a target change. A declared link whose snapshot status is `through-link` is refused before launch. Last before launch, F calls D's `checkTaskTree` with the clone's head and the item's file operations (#81) and passes the result to the start call, which mounts every gitlink empty and read-only. A `TaskTreeRefused` (the tree is not a clean copy of the head, or an operation does not fit it) is a safety finding that sends the task to needs human; any other check failure fails the attempt. 3. **If the job holds an in-memory first reason** (its earlier write failed), skip publication: settle the row through the first-reason mapping in rule 2, passing that reason. Otherwise, in one `Store` transaction: confirm that the attempt ID is the task's current attempt, the state is `running`, there is no first reason, the captured context is still current, the task's status is `running`, and `tasks.cancel_requested` is null (the same guard as the `running → completed` row). Do not compare the task's state version here; the attempt's own transitions have increased it. Then write `completed` and the result, and increase the state version. 4. If step 3 refuses, reread durable state and settle the row by the rules above. Keep the original diagnostic. 5. For a completed writable attempt, F2's result extraction is split in two: diff --git a/docs/plan-format.md b/docs/plan-format.md index 8e25d38a..a9a3180d 100644 --- a/docs/plan-format.md +++ b/docs/plan-format.md @@ -128,7 +128,7 @@ The schema checks the shape. codeboost then checks the meaning. A **failure** bl | A completed agent invocation changes a dependency or a script codeboost will run. | Before its own installation or script invocation, codeboost stops in "needs approval". This post-invocation gate cannot prevent an agent from executing a changed script during its invocation; container and network restrictions must already contain that execution. | | `questions` is not empty. | The plan shows the questions at the top; answer them or approve anyway | -**Submodules in version 1.** Gitlinks are review-only metadata leaves: the linking engine can display and attribute externally produced pointer changes, but v1 plans cannot author, add, delete, or rename a gitlink. Reject declarations targeting an existing gitlink and reject agent-produced gitlinks. Authoring them requires a future typed target-commit field and runner-controlled operation. Do not initialize or update nested submodule worktrees. Before each agent or check invocation, reject an initialized/populated gitlink directory and bind an empty read-only mount at every existing gitlink path; refuse the invocation if that protection cannot be enforced. Before tests and before committing, inspect gitlink locations without following links and reject nested content, including content under newly introduced gitlinks. A top-level Git diff is not enough. Reviewing submodule pointers reads the Git entry only; the agent never receives an editable nested checkout. Tests must attempt writes beneath a gitlink and provide a pre-populated nested checkout, and both must be refused. +**Submodules in version 1.** Gitlinks are review-only metadata leaves: the linking engine can display and attribute externally produced pointer changes, but v1 plans cannot author, add, delete, or rename a gitlink. Reject declarations targeting an existing gitlink and reject agent-produced gitlinks. Authoring them requires a future typed target-commit field and runner-controlled operation. Do not initialize or update nested submodule worktrees. Before each agent or check invocation, reject an initialized/populated gitlink directory and bind an empty read-only mount at every existing gitlink path; refuse the invocation if that protection cannot be enforced. In v1 the mount fully protects a top-level gitlink; a gitlink below the top level is protected against writes into it, but an agent can rename one of its parent directories (the mount moves with it) and create a new directory at the old path, so for those the inspection below is the enforcement and its finding sends the task to needs human (follow-up #99: enforce it during the run). Before tests and before committing, inspect gitlink locations without following links and reject nested content, including content under newly introduced gitlinks. A top-level Git diff is not enough. Reviewing submodule pointers reads the Git entry only; the agent never receives an editable nested checkout. Tests must attempt writes beneath a gitlink and provide a pre-populated nested checkout, and both must be refused. **Accepted state versus runtime containment.** The symlink target rules and snapshots validate the pre-run and accepted post-run repository states; they do not prove that an arbitrary program never created, used, and restored a transient link. A proposed target discovered after execution is never retroactive authorization for access. During execution, the container mount/permission and network boundaries govern access: no unrelated host paths are mounted, metadata/schema/auth mounts are read-only, and only bounded worktree/scratch are writable. A link cannot confer permissions beyond that namespace. The selected vendor credential is deliberately readable by its CLI inside that namespace, including by a compromised CLI; merely spelling that same mounted path through a symlink is not an additional privilege. We do not promise that arbitrary execution cannot read its own vendor credential or access writable scratch. Tests must attempt transient links to unmounted host paths and writes to protected mounts, restore the links, and still confirm the runtime boundary denied those accesses. Do not use a final snapshot as evidence that transient access was blocked. diff --git a/runner/execution.ts b/runner/execution.ts index fc38eb2e..07a52c25 100644 --- a/runner/execution.ts +++ b/runner/execution.ts @@ -4,7 +4,7 @@ import type { InvocationContext, InvocationHandle, InvocationInput, TaskClone } import { prepareExecution } from '../core/execution-prompt.ts'; import { neutralizeMentions, neutralizeReferences } from '../core/pull-request-body.ts'; import { auditRun, type ChangeManifest } from '../core/run-audit.ts'; -import type { DeclaredLinkSnapshot } from '../agents/container/changes.ts'; +import { TaskTreeRefused, type DeclaredLinkSnapshot, type DeclaredOperation, type TaskTreeCheck } from '../agents/container/changes.ts'; import { saveDiagnostic } from './diagnostics.ts'; import { ownerOnlyDirectory } from './runner-repository.ts'; import { FinishFailure, NEEDS_RESTART, PreparationFailure, type PreparedAttempt, type RunnerCoordinator, type RunnerDeps } from './coordinator.ts'; @@ -24,6 +24,12 @@ export interface TaskWorkspace { * path the item declares: an earlier item may have renamed or added links, so only D sees the actual entries. */ snapshotDeclaredLinks(workspace: WorkspaceRef, paths: readonly string[], signal: AbortSignal): Promise; + /** + * D's pre-launch tree check (#81, `checkTaskTree`), the last step before launch: the work tree is exactly the tree of + * `baseHead` and the item's operations fit it. It rejects with `TaskTreeRefused` when they do not. The result is the + * capability the agent's profile needs to mount every gitlink empty and read-only; it serves one launch. + */ + checkTree(workspace: WorkspaceRef, input: { baseHead: string; operations: readonly DeclaredOperation[] }, signal: AbortSignal): Promise; /** D's change manifest after the agent settled (`TaskChangeManifest`), with the digest the commit step must match. */ inspectChanges(workspace: WorkspaceRef, input: { baseHead: string; linkSnapshot: DeclaredLinkSnapshot }, signal: AbortSignal): Promise; /** @@ -44,8 +50,8 @@ export interface TaskWorkspace { /** Remove host-side preparation files (the staging clone). Called before the terminal write when D never ran, after it otherwise. */ cleanupPreparation?(attempt: AttemptRecord): Promise; } -/** D's start call for an execute/fix phase with this prompt; returns at once (see #51). */ -export type AgentLauncher = (input: InvocationInput, prompt: string, workspace: WorkspaceRef) => InvocationHandle; +/** D's start call for an execute/fix phase with this prompt and the tree check made for it; returns at once (see #51). */ +export type AgentLauncher = (input: InvocationInput, prompt: string, workspace: WorkspaceRef, treeCheck: TaskTreeCheck) => InvocationHandle; /** Trusted runner-side sources for a task. Issue text and lessons are untrusted data inside the prompt. */ export interface ExecutionSources { planContext(identity: PlanIdentity): PlanContext; @@ -88,7 +94,8 @@ export class SafetyFindings { settle(attemptId: string): void { this.#unsaved.delete(attemptId); } } export interface ExecutionResult { head: string; unchanged: boolean; inScope: string[]; outOfScope: string[] } -interface Private { workspace: WorkspaceRef; prompt: string; baseHead: string; linkSnapshot: DeclaredLinkSnapshot | undefined } +interface Private { workspace: WorkspaceRef; prompt: string; baseHead: string; linkSnapshot: DeclaredLinkSnapshot | undefined; + treeCheck: TaskTreeCheck | undefined } /** * RunnerDeps for execute attempts: fresh workspace, prompt, agent, then audit and the runner's own commit. @@ -139,7 +146,7 @@ export function executionDeps(store: Store, workspace: TaskWorkspace, launch: Ag const declaredPaths = [...new Set(item.files.flatMap(file => [file.path, ...(file.renamed_from ? [file.renamed_from] : [])]))]; const ws = await workspace.materialize(attempt, baseHead, signal); // From here task storage exists: a failure hands it to the coordinator, which removes it after the terminal write. - const data: Private = { workspace: ws, prompt: request.prompt, baseHead, linkSnapshot: undefined }; + const data: Private = { workspace: ws, prompt: request.prompt, baseHead, linkSnapshot: undefined, treeCheck: undefined }; const prepared = { clone: ws.clone, vendor, approvedArgv: request.approvedArgv, private: data }; try { data.linkSnapshot = await workspace.snapshotDeclaredLinks(ws, declaredPaths, signal); } catch (error) { throw new PreparationFailure(error, prepared); } @@ -152,11 +159,23 @@ export function executionDeps(store: Store, workspace: TaskWorkspace, launch: Ag findings.record(attempt.id, text); throw new PreparationFailure(new Error(text), prepared); } + // Last, just before launch (plan-format.md, "Clean invocation state"): no agent runs until the actual tree is the + // head's and the item's operations fit it. A refusal (a tree that is not a clean copy, an operation the head does + // not allow, a gitlink that cannot be mounted) gives the same answer on retry, so it goes to a person. + const operations = item.files.map(file => ({ kind: file.kind, path: file.path, renamedFrom: file.renamed_from })); + try { data.treeCheck = await workspace.checkTree(ws, { baseHead, operations }, signal); } + catch (error) { + if (!(error instanceof TaskTreeRefused)) throw new PreparationFailure(error, prepared); + // The differences name agent-reachable paths: D quotes each one. + const text = bounded(`${SAFETY_VIOLATION} ${error.message}`); + findings.record(attempt.id, text); + throw new PreparationFailure(new Error(text), prepared); + } return prepared; }, // Host-side files only (the staging clone); task storage waits for release. async cleanupPreparation(attempt) { await workspace.cleanupPreparation?.(attempt); }, - start(input, prepared) { const data = prepared.private as Private; return launch(input, data.prompt, data.workspace); }, + start(input, prepared) { const data = prepared.private as Private; return launch(input, data.prompt, data.workspace, data.treeCheck!); }, validate() { throw new Error('Execute attempts publish through finish().'); }, async finish(attempt, _result, prepared, signal) { const data = prepared.private as Private, identity = identityOf(attempt); diff --git a/runner/workspace.ts b/runner/workspace.ts index 19689e9a..f6bce7fb 100644 --- a/runner/workspace.ts +++ b/runner/workspace.ts @@ -1,7 +1,7 @@ import { randomUUID } from 'node:crypto'; import { lstatSync, mkdirSync, rmSync } from 'node:fs'; import { join } from 'node:path'; -import { commitTaskChanges, inspectTaskChanges, snapshotDeclaredLinks } from '../agents/container/changes.ts'; +import { checkTaskTree, commitTaskChanges, inspectTaskChanges, snapshotDeclaredLinks } from '../agents/container/changes.ts'; import { exportTaskDiff, prepareTaskFilesystemsAsync, removeTaskFilesystemsAsync, type TaskFilesystems, type TaskStorageLimits } from '../agents/container/storage.ts'; import type { ProcessGroup } from '../agents/process-group.ts'; import { identityKey, type PlanIdentity } from '../core/identity.ts'; @@ -70,6 +70,9 @@ export function createTaskWorkspace(options: WorkspaceOptions): TaskWorkspace { async snapshotDeclaredLinks(workspace, paths, signal) { return snapshotDeclaredLinks(held(workspace).filesystems, paths, { imageId, signal }); }, + async checkTree(workspace, input, signal) { + return checkTaskTree(held(workspace).filesystems, { base: input.baseHead, operations: input.operations, imageId, signal }); + }, async inspectChanges(workspace, input, signal) { return inspectTaskChanges(held(workspace).filesystems, { base: input.baseHead, linkSnapshot: input.linkSnapshot, imageId, signal }); }, diff --git a/test/agent-container.test.ts b/test/agent-container.test.ts index 925d7635..f6ec381f 100644 --- a/test/agent-container.test.ts +++ b/test/agent-container.test.ts @@ -15,8 +15,8 @@ import { createTaskClone } from '../git/clone.ts'; import { hasOwnerLabels } from '../agents/labels.ts'; import { exportTaskDiff, isRecoveredTaskStorage, prepareTaskFilesystemsAsync, taskFilesystemOwner, EXPORT_SCRIPT, UnusableRepositoryError } from '../agents/container/storage.ts'; -import { commitTaskChanges, inspectTaskChanges, manifestDigest, MAXIMUM_CHANGES, MAXIMUM_NAME_BYTES, snapshotDeclaredLinks, - TASK_COMMIT_REF, TaskCommitRefused, type TaskChangeManifest } from '../agents/container/changes.ts'; +import { checkTaskTree, commitTaskChanges, inspectTaskChanges, manifestDigest, MAXIMUM_CHANGES, MAXIMUM_NAME_BYTES, snapshotDeclaredLinks, + TASK_COMMIT_REF, TaskCommitRefused, TaskTreeRefused, type TaskChangeManifest, type TaskTreeCheck } from '../agents/container/changes.ts'; import { TREE_SCRIPT } from '../agents/container/tree-script.ts'; import { recoverLeftovers } from '../agents/recovery.ts'; import { createVendorNetwork, removeVendorNetwork, VendorNetworkCreationCleanupError, @@ -81,14 +81,18 @@ const governed = async (captured: InvocationInput, probe: IsolationProbe = 'noop async function profile(data: ReturnType, phase: Phase, command: IsolationProbe | ((policy: ReturnType) => AgentCommand), options: { vendor?: 'codex' | 'claude'; authProbe?: boolean; codexAuthFile?: string; claudeToken?: string; deadlineMs?: number; + treeCheck?: TaskTreeCheck; } = {}) { const vendor = options.vendor ?? 'codex'; const captured = invocation(data.clone, phase, vendor, options.deadlineMs); const policy = createPhasePolicy(captured), network = await createVendorNetwork(captured, imageId, randomUUID()); vendorNetworks.push(network); const trustedCommand = typeof command === 'string' ? createIsolationProbeCommand(policy, command) : command(policy); + // Execute and fix launch only on a tree checked just before (#81). + const treeCheck = phase === 'execute' || phase === 'fix' + ? options.treeCheck ?? await checkTaskTree(data.filesystems, { base: data.clone.head, operations: [], imageId }) : undefined; const base = await createContainerProfile({ invocation: captured, policy, network, filesystems: data.filesystems, - inputDirectory: data.input, command: trustedCommand, imageId, + inputDirectory: data.input, command: trustedCommand, imageId, treeCheck, codexAuthFile: vendor === 'codex' ? (options.codexAuthFile ?? data.fakeAuth) : undefined, claudeToken: vendor === 'claude' ? options.claudeToken : undefined }); profiles.push(base); @@ -276,7 +280,8 @@ describe('real Docker agent isolation', () => { it('persists execution changes while replacing HOME and scratch for each invocation', async () => { const data = fixture(); expect(await runContainer(await profile(data, 'execute', 'persist-write'))).toBe('first'); - const output = await runContainer(await profile(data, 'execute', 'persist-read')); + // Read back by a read-only phase: an execute or fix launch needs a clean tree (#81). + const output = await runContainer(await profile(data, 'review', 'persist-read')); expect(output).toContain('?? generated.txt'); }, 60_000); @@ -1678,6 +1683,141 @@ describe('real Docker agent isolation', () => { }, 300_000); }); + describe('pre-launch tree check and gitlink mounts (#81)', () => { + // A base commit with gitlinks, which the non-recursive task clone leaves as empty directories. + const withGitlinks = (paths: readonly string[], extra?: (source: string) => void, beforeSeed?: (clone: string) => void) => + fixture({ beforeSeed, hostile: source => { + mkdirSync(join(source, 'src')); writeFileSync(join(source, 'src', 'a.ts'), 'a\n'); + for (const [index, path] of paths.entries()) { + mkdirSync(join(source, path), { recursive: true }); + git(source, 'update-index', '--add', '--cacheinfo', `160000,${String(index + 1).repeat(40)},${path}`); + } + extra?.(source); + } }); + const refusal = async (promise: Promise) => { + const error = await promise.then(() => undefined, (caught: unknown) => caught); + expect(error).toBeInstanceOf(TaskTreeRefused); + return (error as TaskTreeRefused).differences; + }; + + it('mounts every gitlink empty and read-only, so an agent\'s write beneath one is refused', async () => { + const data = withGitlinks(['sm', 'deps/inner']); + const check = await checkTaskTree(data.filesystems, { base: data.clone.head, imageId, + operations: [{ kind: 'edit', path: 'src/a.ts' }, { kind: 'add', path: 'src/b.ts' }] }); + expect(check).toEqual({ base: data.clone.head, gitlinks: ['deps/inner', 'sm'] }); + const valid = await profile(data, 'execute', 'gitlink-write', { treeCheck: check }); + expect(valid.gitlinks).toEqual(['deps/inner', 'sm']); + expect(await runContainer(valid)).toBe('gitlink-protected'); + // What the agent could write is still inspected, and nothing reached a gitlink directory. + const manifest = await inspectTaskChanges(data.filesystems, { base: data.clone.head, + linkSnapshot: await snapshotDeclaredLinks(data.filesystems, [], { imageId }), imageId }); + expect(manifest.nestedGitlinkContent).toEqual([]); + expect(manifest.changes.map(change => change.path)).toEqual(['beside.txt']); + // The mount cannot stop a parent of a nested gitlink being renamed (the mount moves with it) and a new directory + // made at the old path; the inspection after the run reports that content. + asAgent(data.filesystems, 'mv deps deps2 && mkdir -p deps/inner && printf nested > deps/inner/x'); + const moved = await inspectTaskChanges(data.filesystems, { base: data.clone.head, + linkSnapshot: await snapshotDeclaredLinks(data.filesystems, [], { imageId }), imageId }); + expect(moved.nestedGitlinkContent).toEqual(['deps/inner']); + }, 180_000); + + it('refuses a pre-populated nested checkout before any profile exists', async () => { + const data = withGitlinks(['sm'], undefined, clone => { + mkdirSync(join(clone, 'sm', '.git'), { recursive: true }); writeFileSync(join(clone, 'sm', 'lib.c'), 'nested\n'); + }); + expect(await refusal(checkTaskTree(data.filesystems, { base: data.clone.head, operations: [], imageId }))) + .toContain('gitlink "sm" has content or cannot be read'); + // Without a check, no execute or fix profile is built. + await expect(createContainerProfile({ ...await governed(invocation(data.clone, 'execute')), filesystems: data.filesystems, + inputDirectory: data.input, codexAuthFile: data.fakeAuth, imageId })).rejects.toThrow('requires a pre-launch tree check'); + }, 180_000); + + it('refuses a restart with an untracked symlink parent, and a declared path beneath a tracked symlink', async () => { + const data = withGitlinks([], source => { mkdirSync(join(source, 'd')); symlinkSync('d', join(source, 'link')); }); + // An earlier attempt left src replaced by a link to a directory of its own. + asAgent(data.filesystems, 'mv src src-real && ln -s src-real src'); + const differences = await refusal(checkTaskTree(data.filesystems, { base: data.clone.head, imageId, + operations: [{ kind: 'edit', path: 'src/a.ts' }] })); + expect(differences).toContain('"src" is a symlink the head does not have'); + expect(differences).toContain('"src-real/a.ts" is "src/a.ts" moved'); + // On a clean tree, a declared path the head puts beneath a link is refused all the same. + const clean = withGitlinks([], source => { mkdirSync(join(source, 'd')); symlinkSync('d', join(source, 'link')); }); + expect(await refusal(checkTaskTree(clean.filesystems, { base: clean.clone.head, imageId, + operations: [{ kind: 'add', path: 'link/new.ts' }, { kind: 'rename', path: 'n.ts', renamedFrom: 'link' }] }))) + .toEqual(['"link/new.ts" lies beneath the symlink "link"']); + }, 180_000); + + it('refuses a restart with an occupied add destination, untracked or in the head', async () => { + const data = withGitlinks(['sm']); + asAgent(data.filesystems, 'printf left > src/b.ts'); + expect(await refusal(checkTaskTree(data.filesystems, { base: data.clone.head, imageId, + operations: [{ kind: 'add', path: 'src/b.ts' }] }))).toEqual(['"src/b.ts" is a file the head does not have']); + const clean = withGitlinks(['sm']); + expect(await refusal(checkTaskTree(clean.filesystems, { base: clean.clone.head, imageId, operations: [ + { kind: 'add', path: 'file.txt' }, { kind: 'rename', path: 'src', renamedFrom: 'file.txt' }, + { kind: 'edit', path: 'sm' }, { kind: 'delete', path: 'gone.ts' }, { kind: 'add', path: 'sm/x' }] }))).toEqual([ + 'add destination "file.txt" is occupied by a file', 'rename destination "src" is occupied by a directory', + '"sm" is a gitlink, which a plan item cannot change', 'delete source "gone.ts" is missing', + '"sm/x" lies beneath the gitlink "sm"']); + }, 180_000); + + it('refuses a gitlink Docker cannot mount where it is, and more gitlinks than a profile mounts', async () => { + const data = withGitlinks(['a,b', 'q"x']); + expect(await refusal(checkTaskTree(data.filesystems, { base: data.clone.head, operations: [], imageId }))) + .toEqual(['gitlink "a,b" has a name Docker cannot mount', 'gitlink "q\\"x" has a name Docker cannot mount']); + const many = withGitlinks(Array.from({ length: 257 }, (_, index) => `m/${index}`)); + expect(await refusal(checkTaskTree(many.filesystems, { base: many.clone.head, operations: [], imageId }))) + .toEqual(['the head has 257 gitlinks, more than the 256 that can be mounted']); + }, 180_000); + + it('refuses a tree it cannot read whole as a refusal, not a failure to run', async () => { + const data = withGitlinks([]); + asAgent(data.filesystems, 'touch "$(printf \'bad\\001name\')"'); + expect(await refusal(checkTaskTree(data.filesystems, { base: data.clone.head, operations: [], imageId }))) + .toEqual([expect.stringMatching(/^the tree cannot be checked whole \(".*bad\\\\x01name.*"\)$/)]); + }, 180_000); + + it('uses each check for one profile only, over its own storage and head', async () => { + const data = withGitlinks(['sm']), other = withGitlinks(['sm']); + const check = await checkTaskTree(data.filesystems, { base: data.clone.head, operations: [], imageId }); + await expect(profile(other, 'execute', 'noop', { treeCheck: check })).rejects.toThrow('other task storage'); + await expect(createContainerProfile({ ...await governed(invocation(data.clone, 'review')), filesystems: data.filesystems, + inputDirectory: data.input, codexAuthFile: data.fakeAuth, imageId, treeCheck: check })).rejects.toThrow('Only an execute or fix profile'); + await expect(profile(data, 'execute', 'noop', { treeCheck: { ...check } })).rejects.toThrow('requires a pre-launch tree check'); + await profile(data, 'execute', 'noop', { treeCheck: check }); + await expect(profile(data, 'execute', 'noop', { treeCheck: check })).rejects.toThrow('already used'); + // A check made at another head (a commit with the same tree, on top of the recorded one) is refused too. + let later = ''; + const moved = withGitlinks(['sm'], undefined, clone => { + git(clone, '-c', 'user.name=t', '-c', 'user.email=t@example.com', 'commit', '--allow-empty', '-qm', 'later'); + later = git(clone, 'rev-parse', 'HEAD'); + }); + const elsewhere = await checkTaskTree(moved.filesystems, { base: later, operations: [], imageId }); + await expect(profile(moved, 'execute', 'noop', { treeCheck: elsewhere })).rejects.toThrow('another head'); + }, 180_000); + + it('rejects a container whose gitlink mount is missing or writable after Docker resolves it', async () => { + const data = withGitlinks(['sm']); + const notCovered = 'A gitlink path is not covered by its empty read-only mount.'; + for (const [replace, refusal] of [ + [(value: string) => value.replace(',readonly,tmpfs-mode', ',tmpfs-mode'), notCovered], + [(value: string) => value.replace('tmpfs-mode=0555', 'tmpfs-mode=0777'), notCovered], + [(value: string) => value.replace('tmpfs-size=4096', 'tmpfs-size=8192'), notCovered], + [(value: string) => value.replace('target=/work/sm,', 'target=/work/elsewhere,'), 'unexpected external mount'], + // Dropped altogether: the mount and its --mount flag go. + [() => undefined, notCovered]] as const) { + const valid = await profile(data, 'execute', 'must-not-run'); + const args = valid.args.flatMap((value, index) => valid.args[index + 1]?.startsWith('type=tmpfs,target=/work/sm,') + && replace(valid.args[index + 1]!) === undefined ? [] : value.startsWith('type=tmpfs,target=/work/sm,') + ? [replace(value)].filter((item): item is string => item !== undefined) : [value]); + docker(...args); + containers.add(valid.name); + await expect(validateContainer(valid.name, valid)).rejects.toThrow(refusal); + docker('rm', '--force', valid.name); containers.delete(valid.name); + } + }, 180_000); + }); + it('names what the diff cannot show: a submodule directory with content, and attributes that rewrite bytes', async () => { // The base commit has a submodule, which the non-recursive task clone leaves as an empty directory, and a tracked // file with an $Id$ keyword. diff --git a/test/runner-execution.test.ts b/test/runner-execution.test.ts index 50eb4211..e2369f43 100644 --- a/test/runner-execution.test.ts +++ b/test/runner-execution.test.ts @@ -11,6 +11,7 @@ import { MAX_REASON, ShuttingDownError, type ShutdownCapability } from '../runne import type { ChangeManifest, ManifestChange } from '../core/run-audit.ts'; import type { InvocationResult } from '../agents/contract.ts'; import type { Plan, PlanContext } from '../core/plan.ts'; +import { TaskTreeRefused } from '../agents/container/changes.ts'; const oid = (n: number) => n.toString(16).padStart(40, '0'); const RUNNER_OWNER = '0123456789abcdef0123456789abcdef'; @@ -28,7 +29,7 @@ const manifest = (changes: ManifestChange[], over: Partial; exit?: Record>; commit?: (item: string) => Promise; release?: () => Promise; startError?: Error; - inspect?: (item: string, signal: AbortSignal) => Promise; snapshotError?: Error; capability?: (store: Store) => ShutdownCapability; settleError?: boolean; + inspect?: (item: string, signal: AbortSignal) => Promise; snapshotError?: Error; checkError?: Error; capability?: (store: Store) => ShutdownCapability; settleError?: boolean; plan?: Plan; commitHead?: string; pathKeyError?: Error; materializeError?: Error; /** The durable save of a safety finding fails, so the executor must act on it from memory. */ findingSaveError?: boolean; @@ -48,6 +49,13 @@ function setup(options: { manifests?: Record `${o.kind} ${o.path}`).join(',')}]`); + if (options.checkError) throw options.checkError; + return { item: itemOf(ws), base: input.baseHead, gitlinks: [] } as never; + }, async inspectChanges(ws, input, signal) { log.push(`inspect ${itemOf(ws)} @${input.baseHead.slice(-3)}`); await options.inspect?.(itemOf(ws), signal); return options.manifests?.[itemOf(ws)] ?? manifest([change(itemOf(ws) === 'P1' ? 'a.ts' : 'b.ts')]); @@ -72,20 +80,20 @@ function setup(options: { manifests?: Record { throw options.pathKeyError; } } : context; const sources: ExecutionSources = { planContext: () => auditContext, issue: () => ({ number: 1, title: 'Issue', body: 'Please fix', comments: [] }), lessons: () => [], vendor: () => 'claude' }; - const prompts: string[] = [], argv: (readonly (readonly string[])[])[] = [], owners: string[] = []; + const prompts: string[] = [], argv: (readonly (readonly string[])[])[] = [], owners: string[] = [], checks: unknown[] = []; const capability = options.capability?.(store), findings = new SafetyFindings(store, capability); if (options.settleError) store.settleAttempt = () => { throw Object.assign(new Error('disk full'), { code: 'ERR_SQLITE_ERROR' }); }; if (options.findingSaveError) store.recordSafetyFinding = () => { throw Object.assign(new Error('disk full'), { code: 'ERR_SQLITE_ERROR' }); }; - const deps = executionDeps(store, workspace, (input, prompt, ws) => { + const deps = executionDeps(store, workspace, (input, prompt, ws, treeCheck) => { if (options.startError) throw options.startError; - log.push(`start ${itemOf(ws)}`); prompts.push(prompt); argv.push(input.approvedArgv); owners.push(input.runnerOwner); + log.push(`start ${itemOf(ws)}`); checks.push(treeCheck); prompts.push(prompt); argv.push(input.approvedArgv); owners.push(input.runnerOwner); const settled = Promise.resolve().then(() => options.onLaunch?.(input.attemptId)).then(() => ({ attemptId: options.foreignResult ? randomUUID() : input.attemptId, context: input.context, exitCode: 0, signal: null, stdout: 'done', stderr: '', ...options.exit?.[itemOf(ws)] })); return { attemptId: input.attemptId, settled, cancel: () => undefined }; }, sources, RUNNER_OWNER, findings, { diagnostics: { directory: join(dir, 'diagnostics'), capBytes: options.diagnosticsCap }, exportDeadlineMs: options.exportDeadlineMs }); const runner = new RunnerCoordinator(store, deps, undefined, capability); cleanups.push(async () => { await runner.close(); store.close(); }); - return { store, path, workspace, findings, runner, executor: new ItemExecutor(store, runner, sources, findings, { capability }), log, commits, prompts, argv, owners }; + return { store, path, workspace, findings, runner, executor: new ItemExecutor(store, runner, sources, findings, { capability }), log, commits, prompts, argv, owners, checks }; } describe('item execution', () => { @@ -100,8 +108,8 @@ describe('item execution', () => { { sha: oid(100), owner: 'P1', origin: 'owned', sourceSha: null }, { sha: oid(101), owner: 'P2', origin: 'owned', sourceSha: null }])); expect(store.getSnapshot(identity).head).toBe(oid(101)); expect(log).toEqual([ - 'materialize P1 @002', 'snapshot P1 [a.ts]', 'start P1', 'inspect P1 @002', 'commit P1 -> 064', 'release P1 after completed', - 'materialize P2 @064', 'snapshot P2 [b.ts]', 'start P2', 'inspect P2 @064', 'commit P2 -> 065', 'release P2 after completed']); + 'materialize P1 @002', 'snapshot P1 [a.ts]', 'check P1 @002 [edit a.ts]', 'start P1', 'inspect P1 @002', 'commit P1 -> 064', 'release P1 after completed', + 'materialize P2 @064', 'snapshot P2 [b.ts]', 'check P2 @064 [edit b.ts]', 'start P2', 'inspect P2 @064', 'commit P2 -> 065', 'release P2 after completed']); expect(prompts[0]).toContain(''); expect(argv[0]).toEqual([['npm', 'test']]); expect(owners[0]).toBe(RUNNER_OWNER); @@ -425,6 +433,29 @@ describe('item execution', () => { // The storage preparation allocated is still released after the terminal write. expect(h.log.some(line => line.startsWith('release P1 after failed'))).toBe(true); }); + it('never launches an item on a tree the pre-launch check refused, and sends the task to a person (#81)', async () => { + const h = setup({ checkError: new TaskTreeRefused(['"a.ts" lies beneath the symlink "via"', 'add destination "n.ts" is occupied by a file']) }); + expect(await h.executor.runTask(identity)).toMatchObject({ kind: 'needs human', item: 'P1', + reason: expect.stringContaining('The pre-launch tree check refused: "a.ts" lies beneath the symlink "via"; add destination "n.ts" is occupied by a file.') }); + expect(h.log).toEqual(['materialize P1 @002', 'snapshot P1 [a.ts]', 'check P1 @002 [edit a.ts]', 'release P1 after failed']); + expect(h.store.getTask(identity).status).toBe('needs human'); + expect(h.store.getAttempts(identity)[0]!.safetyFinding).toContain(SAFETY_VIOLATION); + }); + it('fails the attempt without a finding when the pre-launch check itself fails (#81)', async () => { + const { store, executor, log } = setup({ checkError: new Error('docker run failed') }); + expect(await executor.runTask(identity)).toMatchObject({ kind: 'stopped', item: 'P1', state: 'failed', reason: 'Preparation failed: "docker run failed"' }); + expect(log).toEqual(['materialize P1 @002', 'snapshot P1 [a.ts]', 'check P1 @002 [edit a.ts]', 'release P1 after failed']); + expect(store.getAttempts(identity)[0]!.safetyFinding ?? null).toBeNull(); + expect(store.getTask(identity).status).toBe('running'); + }); + it('checks every declared operation, rename sources included, and launches with that item\'s check (#81)', async () => { + const renaming: Plan = { ...plan, items: [{ ...plan.items[0]!, files: [ + { path: 'n.ts', kind: 'rename', renamed_from: 'a.ts', change: 'move' }, { path: 'c.ts', kind: 'add', renamed_from: null, change: 'new' }] }] }; + const h = setup({ plan: renaming }); + expect(await h.executor.runTask(identity)).toMatchObject({ kind: 'executed', items: ['P1'] }); + expect(h.log.slice(0, 4)).toEqual(['materialize P1 @002', 'snapshot P1 [n.ts,a.ts,c.ts]', 'check P1 @002 [rename n.ts,add c.ts]', 'start P1']); + expect(h.checks).toEqual([{ item: 'P1', base: oid(2), gitlinks: [] }]); + }); it('acts on a saved finding at the terminal write, before release can set a gate (#87)', async () => { let store!: Store, atRelease: string | undefined; const h = setup({ manifests: { P1: manifest([change('a.ts')], { metadataChanged: true }) }, @@ -555,7 +586,7 @@ describe('item execution', () => { store.setAssignment(identity, store.getTask(identity).stateVersion, 'reassigned', 'hash-2'); return snapshot(ws, paths, signal); }; expect(await h.executor.runTask(identity)).toMatchObject({ kind: 'stopped', item: 'P1', state: 'stale' }); - expect(h.log).toEqual(['materialize P1 @002', 'snapshot P1 [a.ts]', 'release P1 after stale']); + expect(h.log).toEqual(['materialize P1 @002', 'snapshot P1 [a.ts]', 'check P1 @002 [edit a.ts]', 'release P1 after stale']); }); it('refuses a scope pause whose executed prefix does not match the plan at the item\'s revision', () => { const { store } = setup(); @@ -580,7 +611,7 @@ describe('item execution', () => { store.cancelTask(identity, store.getTask(identity).stateVersion, randomUUID()); return snapshot(ws, paths, signal); }; expect(await h.executor.runTask(identity)).toMatchObject({ kind: 'stopped', item: 'P1', state: 'cancelled' }); - expect(h.log).toEqual(['materialize P1 @002', 'snapshot P1 [a.ts]', 'release P1 after cancelled']); + expect(h.log).toEqual(['materialize P1 @002', 'snapshot P1 [a.ts]', 'check P1 @002 [edit a.ts]', 'release P1 after cancelled']); expect(store.getTask(identity).status).toBe('cancelled'); }); it('keeps a safety finding owed when moving to needs human fails, and escalates it before the next run launches anything', async () => { @@ -773,7 +804,7 @@ describe('item execution', () => { return snapshot(ws, paths, signal); }; expect(await h.executor.runTask(identity)).toMatchObject({ kind: 'stopped', item: 'P1', state: 'cancelled' }); - expect(h.log).toEqual(['materialize P1 @002', 'snapshot P1 [a.ts]', 'release P1 after cancelled']); + expect(h.log).toEqual(['materialize P1 @002', 'snapshot P1 [a.ts]', 'check P1 @002 [edit a.ts]', 'release P1 after cancelled']); }); it('finds an older checkpoint by its head after a newer one', async () => { const h = setup({ manifests: { P1: manifest([change('a.ts'), change('extra.ts', { kind: 'add', oldType: undefined })]) } }); From db9ce56ddc79eef2b7a4268b8c44ce8bd220829a Mon Sep 17 00:00:00 2001 From: mchwang Date: Thu, 1 Oct 2026 21:44:21 -0700 Subject: [PATCH 2/5] Test: give each fixture gitlink a 40-digit object ID String(index + 1).repeat(40) made an 80-character ID from the tenth gitlink on, which git update-index --cacheinfo refuses; only the 257-gitlink case reached it. Co-Authored-By: Claude Opus 5.5 --- test/agent-container.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/agent-container.test.ts b/test/agent-container.test.ts index f6ec381f..109fbbd9 100644 --- a/test/agent-container.test.ts +++ b/test/agent-container.test.ts @@ -1690,7 +1690,7 @@ describe('real Docker agent isolation', () => { mkdirSync(join(source, 'src')); writeFileSync(join(source, 'src', 'a.ts'), 'a\n'); for (const [index, path] of paths.entries()) { mkdirSync(join(source, path), { recursive: true }); - git(source, 'update-index', '--add', '--cacheinfo', `160000,${String(index + 1).repeat(40)},${path}`); + git(source, 'update-index', '--add', '--cacheinfo', `160000,${(index + 1).toString(16).padStart(40, '0')},${path}`); } extra?.(source); } }); From 1dd118232466af7e2c1197d43900c8982399e30e Mon Sep 17 00:00:00 2001 From: mchwang Date: Fri, 2 Oct 2026 09:51:22 -0700 Subject: [PATCH 3/5] Test the check's exit 6 and 9 refusals; say what the rename test proves (review round 2) Co-Authored-By: Claude Opus 5.5 --- test/agent-container.test.ts | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/test/agent-container.test.ts b/test/agent-container.test.ts index 109fbbd9..6db63092 100644 --- a/test/agent-container.test.ts +++ b/test/agent-container.test.ts @@ -1685,8 +1685,9 @@ describe('real Docker agent isolation', () => { describe('pre-launch tree check and gitlink mounts (#81)', () => { // A base commit with gitlinks, which the non-recursive task clone leaves as empty directories. - const withGitlinks = (paths: readonly string[], extra?: (source: string) => void, beforeSeed?: (clone: string) => void) => - fixture({ beforeSeed, hostile: source => { + const withGitlinks = (paths: readonly string[], extra?: (source: string) => void, beforeSeed?: (clone: string) => void, + limits?: Parameters[1]) => + fixture({ beforeSeed, limits, hostile: source => { mkdirSync(join(source, 'src')); writeFileSync(join(source, 'src', 'a.ts'), 'a\n'); for (const [index, path] of paths.entries()) { mkdirSync(join(source, path), { recursive: true }); @@ -1713,8 +1714,8 @@ describe('real Docker agent isolation', () => { linkSnapshot: await snapshotDeclaredLinks(data.filesystems, [], { imageId }), imageId }); expect(manifest.nestedGitlinkContent).toEqual([]); expect(manifest.changes.map(change => change.path)).toEqual(['beside.txt']); - // The mount cannot stop a parent of a nested gitlink being renamed (the mount moves with it) and a new directory - // made at the old path; the inspection after the run reports that content. + // Inspection only: content at a nested gitlink path, as an agent can leave by renaming a parent during the run + // (#99), is reported after it. This runs without the mounts, so it does not test the runtime gap itself. asAgent(data.filesystems, 'mv deps deps2 && mkdir -p deps/inner && printf nested > deps/inner/x'); const moved = await inspectTaskChanges(data.filesystems, { base: data.clone.head, linkSnapshot: await snapshotDeclaredLinks(data.filesystems, [], { imageId }), imageId }); @@ -1777,6 +1778,20 @@ describe('real Docker agent isolation', () => { .toEqual([expect.stringMatching(/^the tree cannot be checked whole \(".*bad\\\\x01name.*"\)$/)]); }, 180_000); + it('refuses an unreadable directory and more changes than it reports, as refusals too', async () => { + // Exit 6: a directory the check cannot list may hold anything. + const locked = withGitlinks([]); + asAgent(locked.filesystems, 'mkdir hidden && touch hidden/x && chmod 000 hidden'); + expect(await refusal(checkTaskTree(locked.filesystems, { base: locked.clone.head, operations: [], imageId }))) + .toEqual([expect.stringMatching(/^the tree cannot be checked whole \(".*could not read directory hidden.*"\)$/)]); + // Exit 9: more than MAXIMUM_CHANGES untracked entries cannot be listed whole. + const crowded = withGitlinks([], undefined, undefined, + { workBytes: 64 * 1024 * 1024, workInodes: MAXIMUM_CHANGES + 2048, metadataBytes: 16 * 1024 * 1024, metadataInodes: 512 }); + asAgent(crowded.filesystems, `mkdir many && cd many && seq 0 ${MAXIMUM_CHANGES} | xargs touch`); + expect(await refusal(checkTaskTree(crowded.filesystems, { base: crowded.clone.head, operations: [], imageId }))) + .toEqual([expect.stringMatching(/^the tree cannot be checked whole \(".*more than 10000 new entries.*"\)$/)]); + }, 300_000); + it('uses each check for one profile only, over its own storage and head', async () => { const data = withGitlinks(['sm']), other = withGitlinks(['sm']); const check = await checkTaskTree(data.filesystems, { base: data.clone.head, operations: [], imageId }); From 3d51d184282c40cf188d203ddd753c20743ba1a4 Mon Sep 17 00:00:00 2001 From: mchwang Date: Fri, 2 Oct 2026 10:17:53 -0700 Subject: [PATCH 4/5] D: pin every directory above a nested gitlink during execute and fix (#99) The read-only tmpfs at a nested gitlink (#81) moved with its parent when the agent renamed that parent, so the old path could be refilled during the run. Each directory above a nested gitlink is now mounted again from the task's work volume at its own path (volume-subpath). A mountpoint cannot be renamed, so the gitlink's mount stays put; what is inside stays writable, nosuid and nodev, on the same volume and limits. checkTaskTree refuses more than 256 pinned directories or more than 128 KiB of mount paths. The validator derives the pins from the gitlinks and allows exactly those: the task's own work volume, writable, at that subpath. The probe checks each pin and that renaming it fails. plan-format.md no longer lists the exception. Co-Authored-By: Claude Opus 5.5 --- agents/container/changes.ts | 32 ++++++++++++++- agents/container/profile.ts | 13 +++++- agents/container/run.ts | 25 +++++++++++- agents/policy.ts | 11 +++++- docs/implementation/agent-isolation.md | 16 +++++--- docs/plan-format.md | 2 +- test/agent-container.test.ts | 55 +++++++++++++++++++++++--- 7 files changed, 136 insertions(+), 18 deletions(-) diff --git a/agents/container/changes.ts b/agents/container/changes.ts index 17d7a245..df88f27e 100644 --- a/agents/container/changes.ts +++ b/agents/container/changes.ts @@ -371,7 +371,10 @@ export interface TreeCheckOptions extends StorageScriptOptions { */ export interface TaskTreeCheck { readonly base: string; - /** Every gitlink of `base`, each an empty directory now; each gets an empty read-only mount. */ + /** + * Every gitlink of `base`, each an empty directory now; each gets an empty read-only mount, and every directory above + * one (as `gitlinkParents(gitlinks)` lists them) is pinned with a mount of its own. + */ readonly gitlinks: readonly string[]; } /** @@ -389,8 +392,26 @@ export class TaskTreeRefused extends Error { this.differences = Object.freeze([...differences]); } } -/** The most gitlinks one profile mounts; a head with more refuses the launch. */ +/** The most gitlinks one profile mounts, and the most parent directories it pins; a head with more refuses the launch. */ export const MAXIMUM_GITLINK_MOUNTS = 256; +/** + * The most bytes of path the gitlink mounts add to the `docker create` arguments (each pin names its directory twice, + * each gitlink once), well inside what one exec can carry; a head that needs more refuses the launch. + */ +export const MAXIMUM_GITLINK_MOUNT_BYTES = 128 * 1024; +/** + * Every directory above a gitlink below the top level, parents before children (#99). Each is mounted again at its own + * path from the same work volume, so it is a mountpoint: renaming or removing it fails (EBUSY), and the gitlink's own + * mount cannot be moved away with it. What is inside stays writable. + */ +export function gitlinkParents(gitlinks: readonly string[]): readonly string[] { + const parents = new Set(); + for (const path of gitlinks) { + const parts = path.split('/'); + for (let index = 1; index < parts.length; index++) parents.add(parts.slice(0, index).join('/')); + } + return Object.freeze([...parents].sort((a, b) => a.split('/').length - b.split('/').length || (a < b ? -1 : a > b ? 1 : 0))); +} // The storage each check was made over, and whether a profile has used it. const treeChecks = new WeakMap(); // Docker reads `--mount` as comma-separated fields that a double quote can open: a name with either cannot be mounted @@ -488,6 +509,13 @@ export async function checkTaskTree(storage: TaskFilesystems, options: TreeCheck } if (gitlinks.length > MAXIMUM_GITLINK_MOUNTS) differences.push(`the head has ${gitlinks.length} gitlinks, more than the ${MAXIMUM_GITLINK_MOUNTS} that can be mounted`); + const parents = gitlinkParents(gitlinks as string[]).length; + if (parents > MAXIMUM_GITLINK_MOUNTS) + differences.push(`the head's gitlinks are below ${parents} directories, more than the ${MAXIMUM_GITLINK_MOUNTS} that can be pinned`); + const mountBytes = (gitlinks as string[]).reduce((total, path) => total + Buffer.byteLength(path), 0) + + gitlinkParents(gitlinks as string[]).reduce((total, path) => total + 2 * Buffer.byteLength(path), 0); + if (mountBytes > MAXIMUM_GITLINK_MOUNT_BYTES) + differences.push(`the gitlink mounts would name ${mountBytes} bytes of paths, more than the ${MAXIMUM_GITLINK_MOUNT_BYTES} a launch passes`); for (const path of gitlinks as string[]) if (unmountable(path)) differences.push(`gitlink ${q(path)} has a name Docker cannot mount`); if (differences.length) throw new TaskTreeRefused([...new Set(differences)]); const check = Object.freeze({ base: options.base, gitlinks: Object.freeze([...gitlinks as string[]]) }); diff --git a/agents/container/profile.ts b/agents/container/profile.ts index cefeb13b..177b7ef1 100644 --- a/agents/container/profile.ts +++ b/agents/container/profile.ts @@ -6,7 +6,7 @@ import { join } from 'node:path'; import { assertCapturedInvocation, type InvocationInput, type Phase, type UnreleasedResource } from '../contract.ts'; import { assertBuiltAgentImage } from './image.ts'; import { assertTaskFilesystems, taskFilesystemOwner, type TaskFilesystems } from './storage.ts'; -import { useTaskTreeCheck, type TaskTreeCheck } from './changes.ts'; +import { gitlinkParents, useTaskTreeCheck, type TaskTreeCheck } from './changes.ts'; import { ownerLabelArgs, type ResourceOwner } from '../labels.ts'; import { assertVendorNetwork, removeVendorNetwork, vendorNetworkResources, type VendorNetwork } from '../network/network.ts'; @@ -27,6 +27,8 @@ export interface ContainerProfile { readonly deferredOutput: boolean; /** Execute and fix: every gitlink path, each covered by an empty read-only tmpfs. Empty in read-only phases. */ readonly gitlinks: readonly string[]; + /** Execute and fix: every directory above a gitlink below the top level, each remounted from the work volume (#99). */ + readonly gitlinkParents: readonly string[]; } export interface ProfileOptions { readonly invocation: InvocationInput; @@ -292,6 +294,7 @@ export async function createContainerProfile(options: ProfileOptions): Promise ['--mount', mount({ type: 'volume', source: filesystems.workVolume, target: `/work/${path}`, + 'volume-subpath': path })]), // Each gitlink is an empty directory (the check proved it): an empty read-only tmpfs over it keeps it empty. ...gitlinks.flatMap(path => ['--mount', mount({ type: 'tmpfs', target: `/work/${path}`, readonly: true, 'tmpfs-mode': GITLINK_MOUNT_MODE, 'tmpfs-size': String(GITLINK_MOUNT_BYTES) })])]; @@ -329,7 +338,7 @@ export async function createContainerProfile(options: ProfileOptions): Promise | null; Mounts: Array<{ Type: string; Source: string; Target: string; ReadOnly: boolean; - TmpfsOptions?: { SizeBytes?: number; Mode?: number; Options?: unknown } }> | null; Dns: string[]; + TmpfsOptions?: { SizeBytes?: number; Mode?: number; Options?: unknown }; + VolumeOptions?: { Subpath?: string; NoCopy?: boolean; DriverConfig?: unknown; Labels?: unknown } }> | null; Dns: string[]; DnsOptions: string[]; DnsSearch: string[]; ExtraHosts: string[] | null; PortBindings: Record | null; PublishAllPorts: boolean }; Mounts: Array<{ Type: string; Name?: string; Source: string; Destination: string; RW: boolean }>; @@ -256,7 +258,12 @@ export async function validateContainer(container: string, profile: ContainerPro } const mounts = new Map(inspect.Mounts.map(item => [item.Destination, item])); const gitlinkMounts = profile.gitlinks.map(path => `/work/${path}`); - const allowedMounts = new Set(['/work', '/work/.git', '/run/codeboost-input', ...gitlinkMounts, + // Derived again from the gitlinks, so the pins checked are exactly the ones those gitlinks need. + const pins = gitlinkParents(profile.gitlinks); + if (JSON.stringify(pins) !== JSON.stringify(profile.gitlinkParents)) + throw new Error('A directory above a gitlink is not pinned by its own work-volume mount.'); + const parentMounts = pins.map(path => `/work/${path}`); + const allowedMounts = new Set(['/work', '/work/.git', '/run/codeboost-input', ...gitlinkMounts, ...parentMounts, ...(profile.vendor === 'codex' ? ['/run/codeboost-auth/codex/auth.json'] : [])]); if (inspect.Mounts.some(item => !allowedMounts.has(item.Destination))) throw new Error('Container includes an unexpected external mount.'); @@ -272,11 +279,25 @@ export async function validateContainer(container: string, profile: ContainerPro && !(Array.isArray(requested[0]!.TmpfsOptions?.Options) && (requested[0]!.TmpfsOptions!.Options as unknown[]).length === 0)) throw new Error('A gitlink path is not covered by its empty read-only mount.'); } + // Every directory above a nested gitlink is the task's own work volume again, writable, at exactly its own subpath. + for (const path of pins) { + const target = `/work/${path}`; + const actual = inspect.Mounts.filter(item => item.Destination === target); + const requested = (host.Mounts ?? []).filter(item => item.Target === target); + const options = requested[0]?.VolumeOptions; + if (actual.length !== 1 || actual[0]!.Type !== 'volume' || actual[0]!.Name !== profile.filesystems.workVolume + || !actual[0]!.RW || requested.length !== 1 || requested[0]!.Type !== 'volume' + || requested[0]!.Source !== profile.filesystems.workVolume || requested[0]!.ReadOnly || options?.Subpath !== path + || options.NoCopy) + throw new Error('A directory above a gitlink is not pinned by its own work-volume mount.'); + } const work = mounts.get('/work'), metadata = mounts.get('/work/.git'), input = mounts.get('/run/codeboost-input'); if (work?.Type !== 'volume' || work.RW !== ['execute', 'fix'].includes(profile.phase) || metadata?.Type !== 'volume' || metadata.RW || input?.Type !== 'bind' || input.RW) throw new Error('Container mounts do not match the phase isolation profile.'); const requestedMounts = new Map((host.Mounts ?? []).map(item => [item.Target, item])); + // /work is the whole work volume: a subpath there would put another directory's content at the root. + if (requestedMounts.get('/work')?.VolumeOptions?.Subpath) throw new Error('Container mounts do not match the phase isolation profile.'); const requestedInput = requestedMounts.get('/run/codeboost-input'); if (requestedInput?.Type !== 'bind' || canonicalDockerBindSource(requestedInput.Source) !== profile.inputDirectory || canonicalDockerBindSource(input.Source) !== profile.inputDirectory diff --git a/agents/policy.ts b/agents/policy.ts index 82f1615a..2eaa644c 100644 --- a/agents/policy.ts +++ b/agents/policy.ts @@ -226,7 +226,16 @@ export function createIsolationProbeCommand(policy: PhasePolicy, probe: Isolatio + 'n=$((n+1)); test "$(findmnt --noheadings --output TARGET --target "/work/$p")" = "/work/$p"; ' + 'findmnt --noheadings --output OPTIONS --target "/work/$p" | tr , "\\n" | grep -Fxq ro; ' + 'test -z "$(ls -A "/work/$p")"; deny touch "/work/$p/x"; deny mkdir "/work/$p/.git"; ' - + 'deny rmdir "/work/$p"; deny mv "/work/$p" "/work/$p.moved"; done; test "$n" -gt 0; ' + + 'deny rmdir "/work/$p"; deny mv "/work/$p" "/work/$p.moved"; ' + // Each directory above it is pinned (#99): its own writable nosuid,nodev mountpoint, which cannot be moved aside, + // so the gitlink's mount cannot be carried off and its path refilled. What is inside stays writable. + + 'd=$(dirname "$p"); while [ "$d" != . ]; do ' + + 'test "$(findmnt --noheadings --output TARGET --target "/work/$d")" = "/work/$d"; ' + + 'o=$(findmnt --noheadings --output OPTIONS --target "/work/$d" | tr , "\\n"); ' + + 'for x in rw nosuid nodev; do printf "%s\\n" "$o" | grep -Fxq "$x"; done; ' + + 'deny mv "/work/$d" "/work/$d.moved"; test -d "/work/$p"; ' + + 'touch "/work/$d/.pinned-write"; test -f "/work/$d/.pinned-write"; rm "/work/$d/.pinned-write"; ' + + 'd=$(dirname "$d"); done; done; test "$n" -gt 0; ' + 'printf ok > /work/beside.txt; printf gitlink-protected', }; return command(policy, probe === 'noop' ? ['true'] : ['sh', '-c', scripts[probe]]); diff --git a/docs/implementation/agent-isolation.md b/docs/implementation/agent-isolation.md index cfbf5e98..045c4a02 100644 --- a/docs/implementation/agent-isolation.md +++ b/docs/implementation/agent-isolation.md @@ -36,7 +36,7 @@ Each row is a T9 requirement for the Docker suite. The suite fails if any row fa | Hard links and alias writes from `.git/config` and objects fail, and metadata stays unchanged | `agent-container`: metadata alias probe in planning, review and execute, with a digest of `.git` before and after | | Mountpoint replacement fails | `agent-container`: metadata and metadata alias probes (`mv` and `rm -rf` of `.git`) | | Both vendor startup probes read the schema and return bounded valid output through their documented channel | `agent-supervisor` live probes (credentials required): Codex in review through its output file; Claude in questions through its stdout envelope; Claude in planning returns a schema-constrained answer as bare JSON. **Known failure:** the Codex probes (here and in `agent-container`) fail, because Codex cannot read files in any phase (see [Codex in read-only phases](#codex-in-read-only-phases)). | -| Nothing can be written beneath a gitlink, and execute and fix launch only on a clean tree (#81) | `agent-container`: the gitlink probe (writes, `mkdir`, `rmdir` and `mv` at every gitlink fail; the rest of `/work` stays writable); a pre-populated nested checkout, a restart with an untracked symlink parent and a restart with an occupied add destination are refused; a gitlink name Docker cannot mount, more than 256 gitlinks and an unreadable tree are refused; each check serves one profile over its own storage and head; the validator rejects a gitlink mount that is missing, writable, or of another size or mode | +| Nothing can be written beneath a gitlink, and execute and fix launch only on a clean tree (#81) | `agent-container`: the gitlink probe (writes, `mkdir`, `rmdir` and `mv` at every gitlink fail; the rest of `/work` stays writable); a pre-populated nested checkout, a restart with an untracked symlink parent and a restart with an occupied add destination are refused; a gitlink name Docker cannot mount, more than 256 gitlinks and an unreadable tree are refused; each check serves one profile over its own storage and head; the validator rejects a gitlink mount that is missing, writable, or of another size or mode; every directory above a nested gitlink is pinned, writable and `nosuid,nodev`, so renaming one fails (#99); the validator rejects a pin that is missing, read-only, at another subpath, from another volume, `volume-nocopy`, or not a volume | | Hostile input stays inside the boundary | `agent-container`: repositories with links that leave the checkout are refused, links inside the checkout still work, oversized repositories fail closed; `agent-policy`: option-like prompts; `agent-proxy`: hostile CONNECT traffic; `agent-supervisor`: hostile output | ## Why the gate can fail @@ -126,7 +126,16 @@ Use only these entry points to run an agent: Pass the result as `treeCheck` in the start call's request. An execute or fix profile requires it; a read-only phase refuses one. The profile adds an empty read-only tmpfs (`--mount type=tmpfs,target=/work/,readonly, tmpfs-mode=0555,tmpfs-size=4096`) at every gitlink the check listed, and the validator allows exactly those mounts, - each read-only with that size and mode. A check serves one profile, over the storage and head it was made for: a + each read-only with that size and mode. Every directory above a gitlink below the top level is pinned (#99): the + profile mounts the work volume again at that path (`--mount type=volume,source=,target=/work/, + volume-subpath=`, which needs Docker 26 or later). A mountpoint cannot be renamed, so the gitlink's mount + cannot be moved away with its parent and the path refilled; what is inside stays writable and `nosuid,nodev`, on + the same volume and limits. A pin is a mount boundary: `rename(2)` and hard links across it fail with `EXDEV`, so + `mv` copies instead, and a tool that writes a temporary file elsewhere and renames it into a pinned directory + fails. Docker resolves the subpath when the container starts, following any link there, so nothing may write the + work volume between the check and the start (nothing does). More than 256 pinned directories, or more than + 128 KiB of paths in these mounts, is refused. The validator allows exactly these mounts: the task's own + work volume, writable, at that subpath. A check serves one profile, over the storage and head it was made for: a copy, a second use, other storage or another head is refused, so check again before every invocation, retries included. - **After the handle settles**, call `inspectTaskChanges(storage, { base, linkSnapshot, imageId })`. It returns the @@ -382,6 +391,3 @@ against real Docker, including two reviews with different Ask owners on one daem is a read-only volume, so nothing can be written beneath a gitlink there; that includes `check` attempts, which run as review. Running an item's `cmd` in a writable task filesystem is not built yet; when it is, each run must call `checkTaskTree` and get the same mounts before it starts. -- The mount keeps a gitlink directory empty, but an agent can still rename a parent directory of a nested gitlink - (the mount moves with it) and create a new directory at the old path. The inspection after the run reports that - directory's content as nested gitlink content, so the task goes to needs human. #99 tracks enforcing this during the run. diff --git a/docs/plan-format.md b/docs/plan-format.md index a9a3180d..5dc18f66 100644 --- a/docs/plan-format.md +++ b/docs/plan-format.md @@ -128,7 +128,7 @@ The schema checks the shape. codeboost then checks the meaning. A **failure** bl | A completed agent invocation changes a dependency or a script codeboost will run. | Before its own installation or script invocation, codeboost stops in "needs approval". This post-invocation gate cannot prevent an agent from executing a changed script during its invocation; container and network restrictions must already contain that execution. | | `questions` is not empty. | The plan shows the questions at the top; answer them or approve anyway | -**Submodules in version 1.** Gitlinks are review-only metadata leaves: the linking engine can display and attribute externally produced pointer changes, but v1 plans cannot author, add, delete, or rename a gitlink. Reject declarations targeting an existing gitlink and reject agent-produced gitlinks. Authoring them requires a future typed target-commit field and runner-controlled operation. Do not initialize or update nested submodule worktrees. Before each agent or check invocation, reject an initialized/populated gitlink directory and bind an empty read-only mount at every existing gitlink path; refuse the invocation if that protection cannot be enforced. In v1 the mount fully protects a top-level gitlink; a gitlink below the top level is protected against writes into it, but an agent can rename one of its parent directories (the mount moves with it) and create a new directory at the old path, so for those the inspection below is the enforcement and its finding sends the task to needs human (follow-up #99: enforce it during the run). Before tests and before committing, inspect gitlink locations without following links and reject nested content, including content under newly introduced gitlinks. A top-level Git diff is not enough. Reviewing submodule pointers reads the Git entry only; the agent never receives an editable nested checkout. Tests must attempt writes beneath a gitlink and provide a pre-populated nested checkout, and both must be refused. +**Submodules in version 1.** Gitlinks are review-only metadata leaves: the linking engine can display and attribute externally produced pointer changes, but v1 plans cannot author, add, delete, or rename a gitlink. Reject declarations targeting an existing gitlink and reject agent-produced gitlinks. Authoring them requires a future typed target-commit field and runner-controlled operation. Do not initialize or update nested submodule worktrees. Before each agent or check invocation, reject an initialized/populated gitlink directory and bind an empty read-only mount at every existing gitlink path; refuse the invocation if that protection cannot be enforced. Every directory above a gitlink is pinned for the invocation too (mounted again at its own path, so it cannot be renamed), so the gitlink's mount cannot be moved aside and its path refilled. Before tests and before committing, inspect gitlink locations without following links and reject nested content, including content under newly introduced gitlinks. A top-level Git diff is not enough. Reviewing submodule pointers reads the Git entry only; the agent never receives an editable nested checkout. Tests must attempt writes beneath a gitlink and provide a pre-populated nested checkout, and both must be refused. **Accepted state versus runtime containment.** The symlink target rules and snapshots validate the pre-run and accepted post-run repository states; they do not prove that an arbitrary program never created, used, and restored a transient link. A proposed target discovered after execution is never retroactive authorization for access. During execution, the container mount/permission and network boundaries govern access: no unrelated host paths are mounted, metadata/schema/auth mounts are read-only, and only bounded worktree/scratch are writable. A link cannot confer permissions beyond that namespace. The selected vendor credential is deliberately readable by its CLI inside that namespace, including by a compromised CLI; merely spelling that same mounted path through a symlink is not an additional privilege. We do not promise that arbitrary execution cannot read its own vendor credential or access writable scratch. Tests must attempt transient links to unmounted host paths and writes to protected mounts, restore the links, and still confirm the runtime boundary denied those accesses. Do not use a final snapshot as evidence that transient access was blocked. diff --git a/test/agent-container.test.ts b/test/agent-container.test.ts index 6db63092..3a9a5e4f 100644 --- a/test/agent-container.test.ts +++ b/test/agent-container.test.ts @@ -1702,20 +1702,22 @@ describe('real Docker agent isolation', () => { }; it('mounts every gitlink empty and read-only, so an agent\'s write beneath one is refused', async () => { - const data = withGitlinks(['sm', 'deps/inner']); + const data = withGitlinks(['sm', 'deps/inner', 'x/y/z']); const check = await checkTaskTree(data.filesystems, { base: data.clone.head, imageId, operations: [{ kind: 'edit', path: 'src/a.ts' }, { kind: 'add', path: 'src/b.ts' }] }); - expect(check).toEqual({ base: data.clone.head, gitlinks: ['deps/inner', 'sm'] }); + expect(check).toEqual({ base: data.clone.head, gitlinks: ['deps/inner', 'sm', 'x/y/z'] }); const valid = await profile(data, 'execute', 'gitlink-write', { treeCheck: check }); - expect(valid.gitlinks).toEqual(['deps/inner', 'sm']); + expect(valid.gitlinks).toEqual(['deps/inner', 'sm', 'x/y/z']); + // Every directory above a nested gitlink is pinned, so the probe's renames of deps, x and x/y all fail (#99). + expect(valid.gitlinkParents).toEqual(['deps', 'x', 'x/y']); expect(await runContainer(valid)).toBe('gitlink-protected'); // What the agent could write is still inspected, and nothing reached a gitlink directory. const manifest = await inspectTaskChanges(data.filesystems, { base: data.clone.head, linkSnapshot: await snapshotDeclaredLinks(data.filesystems, [], { imageId }), imageId }); expect(manifest.nestedGitlinkContent).toEqual([]); expect(manifest.changes.map(change => change.path)).toEqual(['beside.txt']); - // Inspection only: content at a nested gitlink path, as an agent can leave by renaming a parent during the run - // (#99), is reported after it. This runs without the mounts, so it does not test the runtime gap itself. + // Inspection only, without the mounts: content at a nested gitlink path is still reported after a run, should any + // way around the pinned parents remain. asAgent(data.filesystems, 'mv deps deps2 && mkdir -p deps/inner && printf nested > deps/inner/x'); const moved = await inspectTaskChanges(data.filesystems, { base: data.clone.head, linkSnapshot: await snapshotDeclaredLinks(data.filesystems, [], { imageId }), imageId }); @@ -1811,6 +1813,49 @@ describe('real Docker agent isolation', () => { await expect(profile(moved, 'execute', 'noop', { treeCheck: elsewhere })).rejects.toThrow('another head'); }, 180_000); + it('refuses gitlinks below more directories than a profile pins', async () => { + const deep = withGitlinks(Array.from({ length: 130 }, (_, index) => `d${index}/e/g`), undefined, undefined, + { workBytes: 16 * 1024 * 1024, workInodes: 2048, metadataBytes: 16 * 1024 * 1024, metadataInodes: 512 }); + expect(await refusal(checkTaskTree(deep.filesystems, { base: deep.clone.head, operations: [], imageId }))) + .toEqual(['the head\'s gitlinks are below 260 directories, more than the 256 that can be pinned']); + }, 180_000); + + it('refuses gitlink mounts that would name more paths than a launch passes', async () => { + const stem = ['a', 'b', 'c', 'd'].map(letter => letter.repeat(200)).join('/'); + const paths = Array.from({ length: 200 }, (_, index) => `${stem}/g${index}`); + const parents = ['a', 'b', 'c', 'd'].map((_, index) => stem.split('/').slice(0, index + 1).join('/')); + const bytes = paths.reduce((total, path) => total + Buffer.byteLength(path), 0) + + parents.reduce((total, path) => total + 2 * Buffer.byteLength(path), 0); + expect(bytes).toBeGreaterThan(128 * 1024); + const long = withGitlinks(paths); + expect(await refusal(checkTaskTree(long.filesystems, { base: long.clone.head, operations: [], imageId }))) + .toEqual([`the gitlink mounts would name ${bytes} bytes of paths, more than the ${128 * 1024} a launch passes`]); + }, 180_000); + + it('rejects a container whose pin above a nested gitlink is missing, read-only or elsewhere', async () => { + const data = withGitlinks(['deps/inner']); + const pinned = (value: string) => value.startsWith('type=volume,') && value.endsWith(',target=/work/deps,volume-subpath=deps'); + const notPinned = 'A directory above a gitlink is not pinned by its own work-volume mount.'; + for (const [replace, refusal] of [ + [(value: string) => `${value},readonly`, notPinned], + [(value: string) => value.replace('volume-subpath=deps', 'volume-subpath=src'), notPinned], + [(value: string) => value.replace(',target=/work/deps,', ',target=/work/src,'), 'unexpected external mount'], + [(value: string) => value.replace(`source=${data.filesystems.workVolume},`, `source=${data.filesystems.metadataVolume},`), notPinned], + [(value: string) => `${value},volume-nocopy`, notPinned], + [() => 'type=tmpfs,target=/work/deps', notPinned], + [() => undefined, notPinned]] as const) { + const valid = await profile(data, 'execute', 'must-not-run'); + expect(valid.args.filter(pinned)).toHaveLength(1); + const args = valid.args.flatMap((value, index) => valid.args[index + 1] !== undefined && pinned(valid.args[index + 1]!) + && replace(valid.args[index + 1]!) === undefined ? [] : pinned(value) + ? [replace(value)].filter((item): item is string => item !== undefined) : [value]); + docker(...args); + containers.add(valid.name); + await expect(validateContainer(valid.name, valid)).rejects.toThrow(refusal); + docker('rm', '--force', valid.name); containers.delete(valid.name); + } + }, 180_000); + it('rejects a container whose gitlink mount is missing or writable after Docker resolves it', async () => { const data = withGitlinks(['sm']); const notCovered = 'A gitlink path is not covered by its empty read-only mount.'; From 86bccd02e79dc9a40a978cbdb708db5d61bf2098 Mon Sep 17 00:00:00 2001 From: mchwang Date: Fri, 2 Oct 2026 17:14:45 -0700 Subject: [PATCH 5/5] Docs: gitlink masking applies to writable invocations (review round 3) plan-format.md asked for it before every invocation; #81 decided on execute and fix (and future cmd runs) only. Read-only invocations get a fresh non-recursive clone mounted read-only, so a gitlink there stays empty. Co-Authored-By: Claude Opus 5.5 --- docs/plan-format.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/plan-format.md b/docs/plan-format.md index 5dc18f66..323a4e59 100644 --- a/docs/plan-format.md +++ b/docs/plan-format.md @@ -128,7 +128,7 @@ The schema checks the shape. codeboost then checks the meaning. A **failure** bl | A completed agent invocation changes a dependency or a script codeboost will run. | Before its own installation or script invocation, codeboost stops in "needs approval". This post-invocation gate cannot prevent an agent from executing a changed script during its invocation; container and network restrictions must already contain that execution. | | `questions` is not empty. | The plan shows the questions at the top; answer them or approve anyway | -**Submodules in version 1.** Gitlinks are review-only metadata leaves: the linking engine can display and attribute externally produced pointer changes, but v1 plans cannot author, add, delete, or rename a gitlink. Reject declarations targeting an existing gitlink and reject agent-produced gitlinks. Authoring them requires a future typed target-commit field and runner-controlled operation. Do not initialize or update nested submodule worktrees. Before each agent or check invocation, reject an initialized/populated gitlink directory and bind an empty read-only mount at every existing gitlink path; refuse the invocation if that protection cannot be enforced. Every directory above a gitlink is pinned for the invocation too (mounted again at its own path, so it cannot be renamed), so the gitlink's mount cannot be moved aside and its path refilled. Before tests and before committing, inspect gitlink locations without following links and reject nested content, including content under newly introduced gitlinks. A top-level Git diff is not enough. Reviewing submodule pointers reads the Git entry only; the agent never receives an editable nested checkout. Tests must attempt writes beneath a gitlink and provide a pre-populated nested checkout, and both must be refused. +**Submodules in version 1.** Gitlinks are review-only metadata leaves: the linking engine can display and attribute externally produced pointer changes, but v1 plans cannot author, add, delete, or rename a gitlink. Reject declarations targeting an existing gitlink and reject agent-produced gitlinks. Authoring them requires a future typed target-commit field and runner-controlled operation. Do not initialize or update nested submodule worktrees. Before each invocation that can write the task filesystem (an execute or fix agent, and a check that runs an item's `cmd` there), reject an initialized/populated gitlink directory and bind an empty read-only mount at every existing gitlink path; refuse the invocation if that protection cannot be enforced. A read-only invocation (planning, questions, review, and a check that runs as review) needs neither: its task filesystem is a fresh non-recursive clone, so every gitlink directory is empty, and it is mounted read-only, so nothing can fill one. Every directory above a gitlink is pinned for the invocation too (mounted again at its own path, so it cannot be renamed), so the gitlink's mount cannot be moved aside and its path refilled. Before tests and before committing, inspect gitlink locations without following links and reject nested content, including content under newly introduced gitlinks. A top-level Git diff is not enough. Reviewing submodule pointers reads the Git entry only; the agent never receives an editable nested checkout. Tests must attempt writes beneath a gitlink and provide a pre-populated nested checkout, and both must be refused. **Accepted state versus runtime containment.** The symlink target rules and snapshots validate the pre-run and accepted post-run repository states; they do not prove that an arbitrary program never created, used, and restored a transient link. A proposed target discovered after execution is never retroactive authorization for access. During execution, the container mount/permission and network boundaries govern access: no unrelated host paths are mounted, metadata/schema/auth mounts are read-only, and only bounded worktree/scratch are writable. A link cannot confer permissions beyond that namespace. The selected vendor credential is deliberately readable by its CLI inside that namespace, including by a compromised CLI; merely spelling that same mounted path through a symlink is not an additional privilege. We do not promise that arbitrary execution cannot read its own vendor credential or access writable scratch. Tests must attempt transient links to unmounted host paths and writes to protected mounts, restore the links, and still confirm the runtime boundary denied those accesses. Do not use a final snapshot as evidence that transient access was blocked.