Repository navigation
Relay socket directory: /tmp must be root's and sticky, a private fallback when the per-user directory is refused, dead stages' directories reaped - #193
Merged
Conversation
…lback when the per-user directory is refused, dead stages' directories reaped (#185) - checkSocketRoot refuses a socket root that is not root's with the sticky bit, naming why; confirmSocketDir checks after the listen that the socket is in the very directory checked (device, inode, real path, socket owner). - A refused per-user directory falls back to a private mkdtemp directory (/tmp/factory-relay-<uid>-XXXXXX). Locally serve makes it before the stage, names it in the outer sandbox and hands it over (FACTORY_RELAY_SOCKET_DIR). - serve removes a socket directory a dead stage left when it reaps the stage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CS53Gu2D3bE8mKgFN92XP5
…nly its recorded directory (#185 review) - dropSocketDir (reap and stage end) removes only a directory that passes checkSocketDir, so a per-user parent that became a link is never followed. - A local stage takes the handed directory only when its usage directory's record names it. - handSocketDir removes the directory when its record cannot be written. - A stage that relays nothing keeps no socket directory reachable in the outer sandbox. - Comment names reapSocketDirs; PLAN.md says the above. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CS53Gu2D3bE8mKgFN92XP5
kirkmarple-clockgrove
force-pushed
the
fix/185-relay-dir
branch
from
November 14, 2026 01:37
7461aa2 to
1eee5d8
Compare
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.
What was wrong
A security review of #183 found the relay's socket directory,
/tmp/factory-relay-<uid>/<12 hex>, clean on a normal host, with three follow-ups:listenrelies on/tmpbeing root's and sticky, and nothing checked that. On a non-sticky/tmpanother user could rename the directory away and put one of theirs in its place, and the relay would listen there: the commands' calls, with the stage's token, would go to them./tmp/factory-relay-<uid>first, and every relayed stage of that user then Blocked until root or a reboot cleared it.What changes
checkSocketRoot,lib/worker-relay.mjs). The socket goes in/tmponly when its real path is a directory owned by root with the sticky bit. Otherwise the stage Blocks before the run, naming why:the gateway relay for AI_GATEWAY_API_KEY could not start: its socket's directory: /tmp has no sticky bit (mode 777), so another user could swap the socket's directory there for one of theirs(oris not owned by root). No fallback escapes this: any directory in a non-sticky/tmpcould be swapped the same way.checkSocketDir,confirmSocketDir). Before: the path is one of the two shapes this user's socket directories take under the root, and each level is a real directory of this user's, mode 700. After: the directory is still the same one (device and inode), still passes those checks, is the socket's real directory, and holds a socket of this user's at the path. Anything else closes the relay and Blocks before any command starts or any token is handed out.stageSocketDir). When the per-user directory is refused (a file, a link, another user's directory, or one open to others), the socket goes in/tmp/factory-relay-<uid>-XXXXXX, made bymkdtemp(mode 700, a name no one can take first). The log says which directory was refused and why.handSocketDir,local.mjsstageEnv). For a work or review stage that opts inAI_GATEWAY_API_KEY,servemakes the directory before it writes the outer sandbox's profile, names that directory as the one socket directory it keeps reachable, and hands it to the stage inFACTORY_RELAY_SOCKET_DIR. If the record can't be written, the directory is removed at once. The stage takes the handed directory only when the usage directory's record names that same directory; no sandbox can write the usage directory. Another stage's live directory, or one with no record, is refused. A stage that relays nothing now keeps no socket directory reachable in the outer profile. Before this, it kept its own hash directory reachable, though nothing ever listened there. Onlyservesets that variable. It is dropped from the workflow's env, listed inFACTORY_CONTROL_KEYS(scrubbed from every child), and the stage checks it like any other directory. A local stage with nothing handed never makes a fallback of its own, since the outer sandbox would not name it; it uses the per-user directory or Blocks naming the refusal. A developer's own session (no outer sandbox) falls back by itself.reapSocketDirs, called fromreapHere).serverecords the directory it made in the stage's usage directory (relay-socket.json, withserve's own process record). It removes the directory when the stage ends and on every failed start. Whenserveitself died, the next loop to reap the Objective removes it if the stage's process is gone, or, where the stage never started, if the loop that made it is gone. It never removes a live stage's directory, or one whose process cannot be told. Removal, at the end of a stage and by the reap alike, runscheckSocketDirfirst: the root must still be root's and sticky, the path must have one of the two shapes, and every level must be a real directory of this user's, mode 700. So a per-user parent that became a link after a reboot cleared/tmpis never followed. The per-user directory itself stays.docs/PLAN.md(Worker secrets, "How commands reach it") states the new guarantees.Why the isolation holds
servemade. Another stage's bound socket is still hidden like any host socket.mkdtempmakes it 700 under a random name, and the sticky root keeps other users from renaming or removing it. The same check before the listen and confirmation after it apply to both.checkSocketDir: only paths of the two shapes under a root-owned sticky root, with every level this user's own, mode 700 and not a link. It removes only the stage's own level.Tests
In
plugin/test/gateway-relay.test.mjs:/tmppasses. A non-sticky stand-in root is refused withhas no sticky bit (mode 777), and a sticky one not owned by root withis not owned by root. A stage on the non-sticky root Blocks, locally and in a developer's session, with the key out of the shell and no directory made, not even a fallback.serve'shandSocketDirrefuses it too.factory-relay-<uid>-XXXXXX(700, socket 600), removed with the stage. Locally with nothing handed, the stage Blocks naming the refusal.serve'shandSocketDirmakes the fallback and records it, and the outer profile keeps it reachable. A stage handed it listens there, and it is gone with the stage. A handed path of another shape is refused. With a deep stand-in root, the fallback's socket path over the limit Blocks with the named limit and the private directory is removed.openthat swaps the checked directory before the listen Blocks withwas replaced after it was checked. One that swaps it back after the listen Blocks withno socket of this user's is at … after the listen. In both, the key is out of the shell.AI_GATEWAY_API_KEY, nothing else.<root>/factory-relay-<uid>/<hash>, and the per-user name is then a link to a directory elsewhere that holds<hash>/precious.txt. Neither the reap nordropSocketDirat the stage's end removes anything, and both records go.is not the directory made for this stage, with the key out of the shell. Handed its own, it listens there, and the other stage's directory is untouched. IfhandSocketDircannot write its record, the directory it made is removed.Each new test fails without its piece of the fix (checked one at a time, each test alone with the rest of the fix kept):
dropSocketDir's earlier string-shape and last-level check put back: the link test fails, because it removesprecious.txt;Live, on this machine:
stageEnvfor a Codex work stage that opts in the key made/tmp/factory-relay-1000/<12 hex>, handed it inFACTORY_RELAY_SOCKET_DIR, and the outer srt profile left it reachable. A merge stage got none, anddropSocketDirremoved it. For a merge stage, a socket bound in its own hash directory is now hidden by the outer profile. Before this change it was kept reachable.node --test plugin/test/*.test.mjs bench/test/*.test.mjs: 791/791 pass. The relay test file passed two more runs in a row with no temp directory left behind.node bench/skills.mjs: exit 0; only the timestamp changed, so it was reverted.Fixes #185
🤖 Generated with Claude Code
https://claude.ai/code/session_01CS53Gu2D3bE8mKgFN92XP5