Skip to content

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
kirkmarple-clockgrove merged 2 commits into
mainfrom
fix/185-relay-dir
Nov 14, 2026
Merged

kirkmarple-clockgrove merged 2 commits into
mainfrom
fix/185-relay-dir

Conversation

@kirkmarple-clockgrove

@kirkmarple-clockgrove kirkmarple-clockgrove commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

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:

  1. The protection against swapping the per-user directory between Factory's checks and its listen relies on /tmp being root's and sticky, and nothing checked that. On a non-sticky /tmp another 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.
  2. Another local user could make /tmp/factory-relay-<uid> first, and every relayed stage of that user then Blocked until root or a reboot cleared it.
  3. A stage that crashed left its socket directory behind until the next boot.

What changes

  • The root is checked (checkSocketRoot, lib/worker-relay.mjs). The socket goes in /tmp only 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 (or is not owned by root). No fallback escapes this: any directory in a non-sticky /tmp could be swapped the same way.
  • Checked before the listen, confirmed after it (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.
  • A private fallback (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 by mkdtemp (mode 700, a name no one can take first). The log says which directory was refused and why.
  • Handed from the loop to the stage, locally (handSocketDir, local.mjs stageEnv). For a work or review stage that opts in AI_GATEWAY_API_KEY, serve makes 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 in FACTORY_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. Only serve sets that variable. It is dropped from the workflow's env, listed in FACTORY_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.
  • Reaped with a dead stage (reapSocketDirs, called from reapHere). serve records the directory it made in the stage's usage directory (relay-socket.json, with serve's own process record). It removes the directory when the stage ends and on every failed start. When serve itself 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, runs checkSocketDir first: 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 /tmp is never followed. The per-user directory itself stays.
  • The sun_path check is unchanged and covers the fallback too.
  • docs/PLAN.md (Worker secrets, "How commands reach it") states the new guarantees.

Why the isolation holds

  • Nothing a sandbox can reach changes. The outer profile still keeps exactly one socket directory reachable and never writable: this stage's, now whichever one serve made. Another stage's bound socket is still hidden like any host socket.
  • The fallback is no weaker than the per-user directory. mkdtemp makes 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.
  • The swap the review describes needs a non-sticky root, which is now refused. If a root passes the check and the directory still changes, the confirmation after the listen catches it before anything is handed over.
  • Removal (reap and stage end) follows 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.
  • A stage takes only the directory recorded for it in its usage directory, never another stage's.

Tests

In plugin/test/gateway-relay.test.mjs:

  • Root: this machine's /tmp passes. A non-sticky stand-in root is refused with has no sticky bit (mode 777), and a sticky one not owned by root with is 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's handSocketDir refuses it too.
  • Fallback: with a file at the per-user name, a developer's session gets factory-relay-<uid>-XXXXXX (700, socket 600), removed with the stage. Locally with nothing handed, the stage Blocks naming the refusal. serve's handSocketDir makes 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.
  • After the listen: an open that swaps the checked directory before the listen Blocks with was replaced after it was checked. One that swaps it back after the listen Blocks with no socket of this user's is at … after the listen. In both, the key is out of the shell.
  • Reap: a dead stage's directory and one whose loop died before the stage started are removed. A live stage's, one a live loop is starting, a recorded path of another shape and the per-user parent stay. A second reap removes nothing.
  • Only relaying stages get one: work and review stages that opt in AI_GATEWAY_API_KEY, nothing else.
  • No removal through a link: a dead stage's record names <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 nor dropSocketDir at the stage's end removes anything, and both records go.
  • Only its own directory: a stage handed another stage's live directory, or a directory with no record, Blocks with 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. If handSocketDir cannot write its record, the directory it made is removed.
  • Two existing Local: the relay's Unix socket path can exceed the OS limit, so a stage that relays a worker key Blocks at start #182 tests now use a sticky stand-in root.

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):

  • the sticky check removed: the root test fails;
  • the fallback removed: the fallback test fails;
  • the confirmation after the listen removed: the swap test fails;
  • the reap disabled, the liveness check removed, or the shape check on removal removed: the reap test fails;
  • the stage filter removed: the last test fails;
  • dropSocketDir's earlier string-shape and last-level check put back: the link test fails, because it removes precious.txt;
  • the record match removed, or the removal on a failed record write removed: the only-its-own-directory test fails.

Live, on this machine: stageEnv for a Codex work stage that opts in the key made /tmp/factory-relay-1000/<12 hex>, handed it in FACTORY_RELAY_SOCKET_DIR, and the outer srt profile left it reachable. A merge stage got none, and dropSocketDir removed 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

…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
kirkmarple-clockgrove added this pull request to the merge queue Nov 14, 2026
Merged via the queue into main with commit 7be11ee Nov 14, 2026
4 checks passed
@kirkmarple-clockgrove
kirkmarple-clockgrove deleted the fix/185-relay-dir branch November 14, 2026 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Relay socket directory: check /tmp is sticky, fall back when the per-user directory is taken, clean up after a crash

1 participant