Resolve the last component of a write, not just the directory - #103
Closed
beardthelion wants to merge 1 commit into
Closed
Resolve the last component of a write, not just the directory#103beardthelion wants to merge 1 commit into
beardthelion wants to merge 1 commit into
Conversation
Path confinement is described here as three layers, and the third one, resolving symlinks, stops one component short on the write path. `resolvePath` resolves the directory a write lands in and then returns the lexical target, so a link sitting where the file goes is followed by `writeFile` and the bytes land wherever it points. A workspace holding `notes.md -> /etc/crontab` takes a write to `notes.md` and puts it in `/etc/crontab`, as root, with the request refused by nothing. Nothing in this API creates a link, which is why it reads as covered. The links come from everywhere else: a shell session, an archive a Bot unpacked, a volume left over from an earlier deployment, and in a container shared between Bots, the other Bot. A dangling link is refused rather than resolved. It points at a file that does not exist yet, so there is nothing to compare against the root, and writing through one creates the file it names, which is the same escape with an extra step. Links pointing back inside still work, for writes as well as reads. The guard has to confine rather than forbid; refusing every link would pass every refusal test here and break what links are legitimately for.
beardthelion
requested review from
MikeRyanDev,
davidmckayv,
guidovizoso and
tylerslaton
as code owners
August 21, 2026 20:04
Contributor
Author
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
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.
Closes #100.
resolvePathresolves the directory a write lands in and returns the lexical target, so a symlink sitting at the last component is followed bywriteFileand the bytes land wherever it points, as root. The module documents three confinement layers and names the symlink layer as the one people miss; on the write path it covers everything except the name being written to.What it does
Resolves the last component too, when it is a link:
lstatthe target. Only a symlink is treated specially, so an ordinary file or a name that does not exist yet takes the path it took before.assertInsideas every other layer.Links pointing back inside still work, for writes as well as reads. The guard has to confine rather than forbid: refusing every link would pass every refusal test in the suite and break what links are legitimately for.
Append takes the same resolved path and is covered by the same check.
Verification
Four new cases in
agent-computer/tests/workspace.test.ts, against real inodes in a temporary directory, because a symlink test with a fake filesystem tests the fake. Three fail before the change and pass after:The fourth extends the existing "points back INSIDE still works" case to write as well as read, which is the assertion that would catch a fix that simply forbade links.
153 tests pass in
agent-computer.bunx tsc --noEmitandbunx biome checkclean on the changed files. Whole-treebun testfailure set is identical tomain(the integration tests wanting a Postgres).Note on the shared arrangement
Without
COMPUTER_SUPERVISOR_URLevery Bot shares one container and one/workspace, so this is also the path by which one Bot plants a link and another writes through it. That arrangement has other reasons not to be used for mutually untrusted Bots, andshell.tssays so already; this closes the file boundary either way.