Skip to content

Commit f4b8336

Browse files
mchwangclaude
andauthored
D: make the runner's commit from audited task changes (#66, part 2) (#90)
* D: make the runner's commit from audited task changes (#66, part 2) commitTaskChanges runs the change inspection again in one container with both task volumes read-only, builds the commit from the same reads (base plus exactly the manifest's changes, each file stored as hashed) into a scratch object store in /tmp, and returns it as a bounded git bundle. Node keeps the bundle only when the manifest digest matches the approved one, refuses agent commits, metadata and link target changes, gitlink content and changes a tree cannot hold, and checks the commit ID against the one its inputs describe. Replaces the planned exportTaskCommit. Also: stdin input for process groups and storage scripts, recovery of leftover commit containers, neutralised issue references in F's runner commit message, and a fix for a SIGTERM race in the fake-Docker tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Accept a .gitattributes over 100 MB again: Git ignores it from the index too The refusal added in review broke part 1's pinned behaviour. Every .gitattributes leaves the scratch index, and Git 2.39.1+ ignores one this large read from the index as well, so hashing does not depend on order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent aafd153 commit f4b8336

15 files changed

Lines changed: 804 additions & 80 deletions

‎AGENTS.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ Every reproduced race requires a failing-before and passing-after regression. As
124124
- Never infer that agent-written content is unchanged from timestamps, sizes or an index's stat cache: not every write moves them (tmpfs, for one). Compare the content itself.
125125
- Hand agent-chosen names to tools as literal data: disable or sidestep pathspec magic (a leading `:`), quote them in the tool's own input format (a leading `"` in `--stdin-paths`), pass them as bytes, and keep empty fields when splitting NUL-separated output. Check a name's safety only where it is reported, never refusing unchanged input for a name the report never carries.
126126
- Check a trust root before anything reads it. When a tool reads config or rules from storage an agent could have reached (Git and the metadata volume), compare that storage with its trusted baseline first, and run nothing if it changed.
127+
- When one run reports a result and then acts on the same data, serialize the report before the action reads it: a read that changes data (Perl autovivification, a lazy default) must not change what the caller checks.
128+
- When the runner predicts what an external tool will produce (a commit ID from its inputs), refuse up front any input the tool would normalize (Git writes a `-0000` offset as `+0000` and re-encodes Unicode noncharacters), so the prediction cannot fail later on valid-looking input.
127129

128130
## Blinded experiments
129131

‎agents/container/changes.ts‎

Lines changed: 196 additions & 24 deletions
Large diffs are not rendered by default.

‎agents/container/storage.ts‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,14 @@ const createDeadline = (timeoutMs: number) => {
8585
};
8686
/** One Docker call a storage step needs run, a wait, or a point where a long walk lets other work (and an abort) in. */
8787
type StorageStep = { readonly args: readonly string[]; readonly timeoutMs: number; readonly cancellable: boolean;
88-
readonly maxBuffer?: number }
88+
readonly maxBuffer?: number; readonly input?: Buffer }
8989
| { readonly sleepMs: number } | typeof PAUSE;
9090
type Steps<T> = Generator<StorageStep, T, DockerOutcome | undefined>;
9191
const PAUSE = Symbol('pause');
9292
/** Ask the driver to run one Docker call. Allocation calls are cancellable; cleanup calls never are. */
9393
function* run(args: readonly string[], timeoutMs: number, cancellable: boolean,
94-
maxBuffer?: number): Steps<DockerOutcome> {
95-
return (yield { args, timeoutMs, cancellable, maxBuffer })!;
94+
maxBuffer?: number, input?: Buffer): Steps<DockerOutcome> {
95+
return (yield { args, timeoutMs, cancellable, maxBuffer, input })!;
9696
}
9797
/** Run one Docker call that must succeed; its failure throws a `DockerError` that `createOutcomeUnknown` reads. */
9898
function* must(args: readonly string[], timeoutMs: number): Steps<string> {
@@ -111,7 +111,8 @@ function runSteps<T>(steps: Steps<T>): T {
111111
else if ('sleepMs' in step) { sleep(step.sleepMs); next = steps.next(); }
112112
else {
113113
const result = spawnSync('docker', [...step.args], { encoding: 'utf8', timeout: step.timeoutMs, killSignal: 'SIGKILL',
114-
env: dockerEnvironment(), stdio: ['ignore', 'pipe', 'pipe'], maxBuffer: step.maxBuffer ?? 16 * 1024 * 1024 });
114+
env: dockerEnvironment(), stdio: [step.input ? 'pipe' : 'ignore', 'pipe', 'pipe'], input: step.input,
115+
maxBuffer: step.maxBuffer ?? 16 * 1024 * 1024 });
115116
const error = result.error ?? (result.status === null
116117
? new Error(`docker ${step.args[0] ?? ''} was killed by ${result.signal}.`) : undefined);
117118
next = steps.next({ status: error ? null : result.status, stdout: String(result.stdout ?? ''),
@@ -145,7 +146,7 @@ async function runStepsAsync<T>(steps: Steps<T>, options: PreparationOptions): P
145146
} else {
146147
next = steps.next(await runInProcessGroup('docker', step.args, { env: dockerEnvironment(),
147148
timeoutMs: step.timeoutMs, signal: step.cancellable ? options.signal : undefined,
148-
onProcessGroup: options.onProcessGroup, maxBuffer: step.maxBuffer }));
149+
onProcessGroup: options.onProcessGroup, maxBuffer: step.maxBuffer, input: step.input }));
149150
}
150151
}
151152
return next.value;
@@ -223,8 +224,8 @@ export function taskMetadataBaseline(storage: TaskFilesystems | RecoveredTaskSto
223224
}
224225
/**
225226
* The baseline to check the metadata against: D's own for storage this process allocated (a given one must match it),
226-
* or the one F recorded, which a recovery handle needs. Nothing but a runner commit (#66 part 2) may change the
227-
* metadata after seeding, and that commit is the last step on a storage: no check runs after it.
227+
* or the one F recorded, which a recovery handle needs. Nothing changes the metadata after seeding: agents mount it
228+
* read-only, and so does every runner operation, the runner commit (#66 part 2) included.
228229
*/
229230
export function resolveMetadataBaseline(storage: TaskFilesystems | RecoveredTaskStorage, given: string | undefined): string {
230231
const known = taskMetadataBaseline(storage);
@@ -729,7 +730,7 @@ export const EXPORT_SCRIPT = [
729730
/** A container run over both volumes of task storage, read-only, as the storage user, with no network. */
730731
interface StorageScript {
731732
/** The `io.codeboost.task-storage` kind the container carries; recovery removes a leftover one. */
732-
readonly kind: 'export' | 'inspect';
733+
readonly kind: 'export' | 'inspect' | 'commit';
733734
/** Names the operation in messages, for example "Task diff export". */
734735
readonly operation: string;
735736
/** Ends "Task work volume is missing; ..." when a volume is gone. */
@@ -742,6 +743,8 @@ interface StorageScript {
742743
readonly memory?: '256m' | '1g';
743744
/** The size of its /tmp. Default 64m. */
744745
readonly tmpBytes?: '64m' | '512m';
746+
/** Written to the script's stdin (`docker run -i`), which is then closed; without it stdin is not connected. */
747+
readonly input?: Buffer;
745748
}
746749
export interface StorageScriptOptions extends PreparationOptions {
747750
/** The immutable ID of the built agent image, whose tools run the script. */
@@ -765,14 +768,14 @@ function* storageScriptSteps(workVolume: string, metadataVolume: string, owner:
765768
const name = `codeboost-${script.kind}-${randomUUID()}`;
766769
let unsettled = false;
767770
try {
768-
const args = ['run', '--rm', '--name', name, '--label', `io.codeboost.task-storage=${script.kind}`, ...ownerLabelArgs(owner),
771+
const args = ['run', '--rm', ...script.input ? ['-i'] : [], '--name', name, '--label', `io.codeboost.task-storage=${script.kind}`, ...ownerLabelArgs(owner),
769772
'--read-only', '--user', '10001:10001', '--network=none', '--cap-drop=ALL', '--security-opt=no-new-privileges',
770773
'--security-opt=seccomp=builtin', '--runtime=runc', '--pids-limit=64', `--memory=${script.memory ?? '256m'}`, '--cpus=.5',
771774
'--tmpfs', `/tmp:rw,nosuid,nodev,noexec,size=${script.tmpBytes ?? '64m'}`,
772775
'--mount', `type=volume,source=${workVolume},target=/work,readonly`,
773776
'--mount', `type=volume,source=${metadataVolume},target=/work/.git,readonly`,
774777
'--entrypoint', script.entrypoint, imageId, ...script.args];
775-
const outcome = yield* run(args, remaining(), true, script.maxOutputBytes);
778+
const outcome = yield* run(args, remaining(), true, script.maxOutputBytes, script.input);
776779
if (outcome.status !== 0) {
777780
const error = new DockerError(args, outcome);
778781
// A client stopped before the daemon answered may still have started the container.

0 commit comments

Comments
 (0)