Repository navigation
F56 — the private clone is the real boundary - #121
Merged
Merged
Conversation
…F56) prepareWorktree clones the folder into a private checkout — own .git, own refs, alternates borrowing the folder's objects, detached at the base commit — instead of git worktree add --detach from the root, so a git update-ref, git branch -D, git push <folder-path> or a hooks-path override inside the run can reach only the clone's refs. The ref guard and the before-and-after ref snapshot are removed rather than kept beside the clone (no compatibility layer): installRefGuard and the snapshotRefs/changedRefs pair go away, and refChanges leaves the result shape. The publication reads the run's commit from the clone: the executor's own git fetches the result commit from the clone into the folder under the branch the publication names (branchCommit.ts) — the one write to the folder's refs the loop makes, made by the executor and never by the harness. The clone is removed as the worktree is today (removeWorktree and the orphan sweep in orphans.ts). Tests state the properties: a run whose harness pushes into the folder's path, deletes a branch and overrides core.hooksPath leaves every ref of the folder as it was, with no hook involved; the publication of such a run lands exactly the commit the run made on the named branch; a clone is removed when the run ends and swept when it is orphaned.
…t write The clone's git directory moves out of the sandbox's writable checkout (git clone --separate-git-dir into the executor's own run scratch, only a gitfile in the checkout), and every git the executor runs in the checkout after the run names that directory itself (--git-dir and --work-tree, or GIT_DIR/GIT_WORK_TREE) and never resolves through the gitfile the agent could rewrite; the harness's git inside the sandbox may follow the gitfile, since a rewritten gitfile only moves the agent's own git. The comment at worktree.ts claiming the clone's config cannot be written is rewritten to say what is true. A sandboxed test plants a rewritten .git gitfile, a .git/commondir and a .gitattributes filter backed by an agent-built repository, and asserts that collection and the apply check run no program and read the executor's git directory. Minors: the commissionChange catalog version becomes 4 (the refChanges promotion, state variable and guidance restored), and the lane's readiness regains a minimum git version at 2.29 whose message names --no-write-fetch-head, in the shape the deleted check had.
…, and the sweep decides by the scratch's marker clonePrivately ran `git clone -- <root>`, which accepts only a repository's top level or its .git, while isGitRepository also accepts a subdirectory of a repository — so a folder connected as a subdirectory of a repository (a package in a monorepo) passed the repository check and then failed every commission, folder check and publication with "Could not create a checkout". The clone is now made from the repository's top level (rev-parse --show-toplevel), and the run's working directory is the folder's path relative to that top level inside the clone, so the harness, the checks, collectChanges and the publication all see the folder where the operator connected it. A test connects a subdirectory and gets a checkout. The boot sweep's clonesIn read a missing or symbolic HEAD in a scratch git directory as the operator's and skipped that scratch for good, leaving behind a clone from a run killed between the gitfile write and the detached checkout and any worktree an earlier executor left there. Whether a directory is the executor's is now decided by the scratch's own naming and marker, never by the shape of HEAD. A test plants a half-made clone and a stale worktree in an executor-named scratch and the sweep removes both. Note on the prior fix commit 8d82b20: its message says Commission Change's refChanges promotion, state variable and guidance are "restored"; the range removes all three and that commit only bumps the version. This commit does not repeat the claim.
…and the mirror walks to the top level (a) The boot sweep and removeWorktree followed a symlink entry of a marked scratch — checkoutsIn resolved <entry>/.git through it — so the sweep could delete the symlinks inside a node_modules outside the scratch, the operator's own project included, and an agent could plant such a symlink in the scratch it writes. The sweep and the removal now act only on a real directory (lstat, never stat), refuse a symlink entry, and unlinkDependencies resolves nothing outside the checkout it was given. A test plants a symlink entry in a marked scratch pointing at a directory with symlinked node_modules and asserts the sweep leaves that directory untouched. (b) For a folder connected as a subdirectory of a repository, the dependency mirroring linked only the folder's own node_modules; hoisted dependencies in the repository's top-level node_modules were never linked and links to sibling workspace packages resolved outside the folder. The mirroring now walks from the folder up to the repository's top level and links every node_modules it finds at the same relative position in the clone, so resolution inside the clone sees what it sees in the folder. A test with a top-level and a package-level node_modules asserts both are linked. (c) The boot sweep took only scratch carrying .aflow-scratch, a marker no executor before this range wrote, so scratch left by the previous executor and the worktrees it registered in operators' repositories were never removed. The sweep now also prunes, for every connected folder, the worktrees `git worktree list --porcelain` reports whose path lies under the executor's scratch root — the only place the previous executor ever put one — and the comment says exactly what is swept.
…andbox
The run's scratch held the checkout `work` and `git-work` inside the
sandbox's writable set, and only the git directory and
<writableRoot>/.git/{config,hooks} were denied — so
`rm -rf <scratch>/work && ln -s ~/.ssh <scratch>/work` was permitted,
and the executor's unsandboxed git, naming --work-tree=<checkout.path>
explicitly, accepted the symlinked directory as its work tree:
collectChanges staged that directory's contents into the returned diff
and mergeIntoCheckout wrote into it.
Two parts, both needed:
1. The directory that holds the checkout is never writable by the
sandbox: the writable set is the checkout itself and a scratch
subdirectory of its own for the agent, and the parent holding `work`
and `git-work` is denied, so the checkout cannot be replaced from
inside.
2. Every executor git on a Checkout first proves, in one shared place
(gitLocation), that checkout.path and workDir are real directories
by lstat whose realpath is the path recorded at prepareWorktree, and
refuses otherwise — collectChanges, mergeIntoCheckout and
checkApplies all go through it.
A sandboxed test shows the policy refusing the replacement, and a
second test plants the symlink from outside the policy and asserts
collection and merge refuse.
Docs: D18 no longer lists refChanges among Commission Change's
promoted outputs, and the F56 entry names the separate git directory
beside the checkout instead of <checkout>/.git.
…covers the borrowed objects and the linked modules For a folder connected as a subdirectory of a repository under the operator's home, the sandbox re-allowed reading only binding.root, the scratch, the writable root and the git directory, while the --mirror --shared clone owns no objects and borrows every one through objects/info/alternates naming <top>/.git/objects, and the mirrored top-level node_modules links point into <top>/node_modules — both under the denied home — so inside the run the harness's git could not read HEAD's tree and the hoisted dependencies did not resolve. When the folder is a subdirectory, the sandbox's read-only widening now names the repository's object store (<top>/.git/objects) and every node_modules the mirroring linked, for the commission and the folder checks alike; and the sandbox policy states the whole boundary in one place — for a top-level folder and for a subdirectory, what the run may read and what it may write, enumerated — with a test over that enumeration. A sandboxed test connects a subdirectory of a repository under a denied home and, inside the run, runs git log -1 and resolves a hoisted package. The boot sweep's loop for the previous executor's registered worktrees (removeLinkedWorktree) goes — the migration bridge CLAUDE.md forbids, half-cleaning anyway, leaving the unmarked scratch around each removed worktree forever. The sweep handles marked scratch only, the doc comment says so, the operator prunes the old executor's worktrees once by hand, and the F56 entry in the plan says that in one sentence. The git 2.29 floor moves to where the flag is used: commitPatchOnBranch checks it before fetchFromClone, with its message, and the check leaves the start of every harness run.
… location, not by prefix borrowedReads returned nothing whenever the folder is a repository's top level, on the reasoning that a top-level folder's object store is inside the folder — false when the folder is a linked worktree (which this repository's own conventions encourage), a submodule, or a repository with a separate git dir: the clone's alternates then name an object store outside the folder, under the denied home, and the harness's git cannot read a single object. The decision is now made by location, never by prefix: git rev-parse --git-path objects resolved to its realpath, reading widened to it whenever it lies outside binding.root, for the commission and the folder checks alike; the doc comment says that, and the sandboxed boundary test gains a case where the connected folder is a linked worktree under the denied home and git log -1 succeeds inside the run. For a subdirectory folder, linkDependencies mirrored the folder's own node_modules and those on the path up to the top level, but workspace links resolve into sibling packages in the clone that have no node_modules, so a sibling's non-hoisted dependencies did not resolve inside the run. For a subdirectory folder the dependency mirroring now runs from the repository's top level exactly as it does for a top-level folder, so every installation in the repository is mirrored and readable; the worktreeDependencies test gains a sibling package with its own node_modules that resolves inside the clone.
… the walk, whatever the depth Since e6c2d12, linkDependencies and borrowedReads found installations only through dependencyDirs(top), which stops at DEPENDENCY_SCAN_DEPTH 3, so a folder connected three or more directories below the top (packages/group/pkg) got neither its own node_modules nor any installation below that depth mirrored or made readable — the walk up from the folder that 8a242f4 had was lost, and the comment promising every installation whichever folder is connected was false. The set of installations is now the union of the scan from the top and the walk from the connected folder up to the top, so the folder's own installation and those on its path are always in it whatever the depth, and borrowedReads reads the same set; the comment says that; the worktreeDependencies test connects a folder three levels down and resolves its own dependency inside the clone. The journal records the outer scratch — passed to recordSpawn separately from the sandbox's agent/ subdirectory — so a crashed run's clone goes with its run as the plan says, with a test; reapOrphans removing only <scratch>/agent no longer leaves the clone to the boot sweep. D18a drops refChanges from Commission Change's promoted outputs and its guidance — 5970b1a corrected line 53 and not this one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix round on F56, closing the review's reproduced blocker and two minors:
.git/configand.git/hooks, so a coding agent could write.git/commondirpointing at a repository it built in its scratch whose config defines afilter.<x>.clean, add a.gitattributesnaming the filter, and the executor's own unsandboxed git (collectChanges,checkApplies) would read that config through commdir and run the agent's program with the executor's PATH and HOME. The clone's git directory now lives where the sandbox cannot write —git clone --separate-git-dirinto the executor's own run scratch outside the writable root, only a gitfile in the checkout — and every git the executor runs in the checkout after the run names that directory itself (--git-dir/--work-treeorGIT_DIR/GIT_WORK_TREE), never resolving through the gitfile the agent could rewrite; the harness's git inside the sandbox may follow the gitfile, since a rewritten gitfile only moves the agent's own git. The comment claiming the clone's config cannot be written is rewritten to say what is true..gitgitfile, a.git/commondir, and a.gitattributesfilter backed by an agent-built repository, and asserts that collection and the apply check run no program and read the executor's git directory; the reviewer's reproduction steps are its shape.packages/platform-artifacts/src/skillCatalog/commissionChange.tscatalog version becomes 4 (therefChangespromotion, state variable and guidance restored), and the lane's readiness regains a minimum git version at 2.29 whose message names--no-write-fetch-head, in the shape the deleted check had.