🔓 fix: Let Linked-Worktree Lanes Commit When the Checkout Lives in the Worker Home - #274
Conversation
…d-denied home A read grant on the whole common Git directory masked the write binds beneath it whenever the checkout lived inside the worker home, which SRT re-binds under a tmpfs (writes first, then reads). Lanes could not create their own index.lock or ref locks. Grant each entry off the writable paths instead, and add a real-SRT regression test with a Linux CI job.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c90e44aa49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| metadata, | ||
| ]; | ||
| return { root, identity, checkoutRoot: checkout, commonGitDir, writableGitPaths }; | ||
| const readableGitPaths = (await gitPathsBeside(commonGitDir, writableGitPaths)).sort(); |
There was a problem hiding this comment.
On Linux with the checkout beneath the read-denied worker home, regular files returned here become individual read-only bind mounts, but the command-root fingerprint records only their path names. Git commonly replaces files such as packed-refs and config by renaming a lockfile over the existing file; because that leaves readableGitPaths unchanged, subsequent requests reuse the sandbox and its bind to the old inode indefinitely. A lane or replay probe can therefore observe stale refs/config, and may lose refs after another process packs and removes the corresponding loose refs. Include identities for individually granted files in the fingerprint or otherwise recreate their grants after replacement.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3828aa1: there are no individual file binds any more. The common directory is bound as a directory, so packed-refs, config and similar files replaced by rename are seen live.
| /** Never the common directory itself: its read bind would mask the writable binds beneath it. */ | ||
| const laneGitPaths = [...readableGitPaths, ...writableGitPaths]; |
There was a problem hiding this comment.
When the checkout is beneath the read-denied worker home and packed-refs does not yet exist, omitting the common directory leaves the reconstructed .git parent as writable tmpfs while refs remains a writable host-backed bind. The lane Git guard permits git pack-refs, so Git can create packed-refs.lock and packed-refs successfully in tmpfs and then delete the real loose refs through the writable refs bind; when the sandbox exits, the packed file disappears and checkout or sibling branch refs are lost. Make the structural tmpfs parent non-writable or reject commands such as pack-refs whose transaction combines ungranted common-root files with writable descendants.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3828aa1 by changing the mechanism. The lane is again granted read access to the whole common directory, so under the worker home .git is a real read-only host directory, not a tmpfs placeholder. On Linux, each existing writable Git directory is also listed as a deeper read-deny, which SRT processes after the home tmpfs and re-binds writable on top. pack-refs now fails to create packed-refs.lock (EROFS) before touching any loose ref. The live test runs git pack-refs --all in a lane and asserts every branch still resolves on the host.
…anting entries Per-entry read grants left the common Git directory as a writable tmpfs placeholder inside the worker home (pack-refs could strand refs there) and pinned replaced files to stale inodes. Grant the whole directory read-only again and, on Linux, list each existing writable Git directory as a deeper read-deny so SRT re-binds its write mount after the ancestor read bind.
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
A linked-worktree lane (#270) could not commit when its checkout sat inside the worker's home directory, which is where a personal machine keeps its repositories.
git addfailed withUnable to create '.git/worktrees/<name>/index.lock': Read-only file system, and ref updates failed the same way. The Skynet worker canary hit this on its first lane commit, so the worker was rolled back.The cause is how the sandbox runtime rebuilds a read-denied directory on Linux. The worker's home is
denyRead, so bwrap mounts a tmpfs over it and then restores the granted paths: write binds first, then read binds. A lane is granted read access to the whole common.gitand write access toobjects,refs,logs/refs,lfsand its ownworktrees/<name>. The read-only bind of.gittherefore landed on top of those write binds and masked them. Checkouts outside the home directory were unaffected, which is why the fake-manager tests passed.On Linux, each existing writable Git directory is now also listed as a deeper read-deny. SRT processes read-denies shallowest first and re-binds each one's write paths, so these deeper entries restore the write binds after the ancestor read bind. The common directory stays a live, read-only host directory. A regression test runs the real sandbox in both layouts, and a new Linux CI job runs it with bubblewrap.
Related: #268, #270, #272.
How it works
Mount order inside the worker home, with this change (Linux):
Only directories that exist are listed. Git creates neither
lfsnorlogs/refsinside a lane, which matches the behavior outside the home directory. Nothing changes on macOS, where Seatbelt applies subpath rules instead of bind mounts.This keeps the common directory a real directory rather than a tmpfs placeholder, which matters for two reasons:
.git, for examplepacked-refsduringgit pack-refsor aMERGE_HEAD, so a Git transaction can never split between vanishing tmpfs files and host-backed refs.packed-refsorconfig, are seen live, because the whole directory is bound rather than individual files.Testing
$HOMEfailsgit addonindex.lockandgit branchon the ref lock.HEAD,index,configandMERGE_HEAD, and to sibling metadata, fail withEROFSin both layouts.linked-worktrees-live.test.tsruns the real sandbox, gated onLIBRECHAT_CODE_LIVE_SRT_TESTS=1, with the checkout both beneath and outside the worker home:git pack-refs --all.Linux Native Sandbox Testsinstalls bubblewrap, lifts Ubuntu 24.04's AppArmor restriction on unprivileged user namespaces, and runs the live test.native-sandbox.test.tschecks that the writable Git directories become deeper read-denies on Linux only, and that a missinglfsis not listed.packages/code:tscis clean, andnpm testshows no new failures.