feat(proxy): serve on a Unix domain socket (--uds) - #3151
Conversation
PR governanceThis PR does not yet satisfy the required template fields:
Please update the PR body, or move the PR back to draft while it is still in progress. |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
JerrettDavis
left a comment
There was a problem hiding this comment.
Thanks for keeping the transport change focused. Two concrete blockers remain on the latest commit:\n\n1. prepare_uds_path() unconditionally calls parent.chmod(0o700) even when the parent already existed. That can silently change permissions on a caller-owned shared directory and disrupt unrelated users/files. Please only apply 0700 to a directory this function actually creates (or validate/refuse an unsafe existing parent without mutating it), and add a regression test proving an existing parent's mode is preserved.\n\n2. CI's est (3) shard fails because the new HEADROOM_UDS Click env var is absent from settings_store.SETTINGS. Please either register it with the appropriate setting metadata or explicitly add it to the intentional-exclusion mechanism with a documented rationale, as the registry contract requires.\n\nThe branch is also currently conflicted with main; resolving that should rerun the suite. I did not treat the coverage comment alone as a blocker.
|
Running this on Linux and it still doesn't seem to allow remote control - in fact it seems to push you to the API subscription... I ran with In the terminal this message comes up on start up:
I try to send a message:
I log in using my subscription but it stil doesn't work and in the logs I see:
|
4127d5e to
bd02453
Compare
bd02453 to
656dd28
Compare
`headroom proxy --uds PATH` binds an AF_UNIX socket instead of a TCP port. Request handling is untouched; this is the transport shell only. No port to collide with, nothing listening on the network, and access governed by filesystem permissions. Containers and systemd units can share a socket through a bind mount or runtime directory without publishing a port. Path handling refuses more than it deletes. It clears a stale socket left by a crashed proxy, but refuses to start when something is still listening (two proxies would split traffic), when the path exists as a non-socket (far more likely a typo'd argument over real data than a leftover), or when the path exceeds the platform's sun_path limit, which otherwise fails inside bind() with an error that names nothing. The parent directory is created 0700, which is the whole access-control boundary since a Unix socket carries no credentials of its own. POSIX only: Windows exposes no socket.AF_UNIX to Python and asyncio has no Windows UDS transport, so the CLI rejects --uds up front and points at --port. HEADROOM_UDS is registered in settings_store.SETTINGS alongside HEADROOM_HOST and HEADROOM_PORT, as the registry drift guard in test_settings_store.py requires of every HEADROOM_* Click envvar. It is manifest_managed for the same reason the other two are: on docker/service installs the bind target belongs to the install manifest. This branch originally proposed the socket as a way to restore Claude Code's Remote Control behind the proxy (headroomlabs-ai#1779). A Linux field test disproved that, and the docs now record the negative result instead. ANTHROPIC_UNIX_SOCKET does satisfy Remote Control's api.anthropic.com host check, but it is reserved for `claude ssh`, where the process on the far end of the socket supplies credentials; Claude Code 2.1.198 carries the diagnostic "ANTHROPIC_UNIX_SOCKET is set (claude ssh remote), and the local proxy is API-key-authed." Setting it classifies the session as API-key auth, while Remote Control separately requires claude.ai subscription auth -- one variable opens one gate and closes the other. Sessions start "Not logged in", and a subscription login still ends in "Could not resolve authentication method" from the SDK. No proxy-side change can repair that, because the decision is made in the client before any request leaves it. The socket stands on its own merits as a transport. It does not close headroomlabs-ai#1779. Refs headroomlabs-ai#1779
656dd28 to
1da7ffd
Compare
prepare_uds_path() chmod'd the socket's parent to 0700 unconditionally, so pointing --uds at an existing shared directory silently tightened it and locked out whatever else was using it. Review feedback on headroomlabs-ai#3151. The 0700 now applies only to directories this call brings into existence, tracked level by level via _missing_ancestors() so an existing root above a newly created subtree keeps its mode. An existing parent is validated instead of modified: startup refuses when it is group- or world-writable without the sticky bit, since any other local user could then replace the socket. Sticky directories such as /tmp are accepted -- others may create their own entries but cannot unlink or rename ours. An unreadable parent is left to bind() to report authoritatively. Tests cover the mode policy as pure logic (injected st_mode, so it runs on Windows too, where chmod is a no-op) plus AF_UNIX-backed regressions proving an existing 0755 parent and a bystander file inside it survive untouched.
|
@JerrettDavis Thanks — the 1. You were right about the failure mode:
On the regression test you asked for — the direct one needs
2. 3. Conflict with main — resolved in 1da7ffd. Rebased onto Local state: One thing worth flagging separately, in case you reviewed before I updated the description: the Remote Control premise this branch opened with turned out to be wrong, and a field test on Linux is what killed it. So this PR no longer claims anything about #1779 beyond recording the negative result in |
JerrettDavis
left a comment
There was a problem hiding this comment.
Thanks for addressing both earlier blockers: the updated parent-directory policy preserves caller-owned modes, rejects unsafe non-sticky writable parents, and the regression coverage is strong; HEADROOM_UDS is also now intentionally accounted for. One actionable correctness issue remains in the latest head: the PR now correctly documents that ANTHROPIC_UNIX_SOCKET cannot make Claude Code work through this transport, but headroom proxy --uds still prints that exact non-working Claude Code command under Usage (ANTHROPIC_UNIX_SOCKET=... ANTHROPIC_BASE_URL=http://api.anthropic.com claude). That turns a known-negative field result into first-party runtime guidance and will send users into the authentication failure described in this PR. Please remove that Claude Code recipe from the UDS banner and replace it with transport-neutral guidance (for example, state that the client must natively support HTTP over UDS, optionally pointing to the docs), with a banner regression asserting the unsupported recipe is absent. After that, the transport implementation itself looks ready.
Both startup banners printed an ANTHROPIC_UNIX_SOCKET=... claude line on a socket bind. That configuration is the one this branch's own docs document as broken: it satisfies Claude Code's api.anthropic.com host check but reclassifies the session as API-key auth, so the session then fails to authenticate. Runtime output was contradicting the docs and walking users into the failure. Review feedback on headroomlabs-ai#3151. The socket branch of both banners now comes from uds.socket_usage_lines(), a single agent-neutral block: the socket path, the transport requirement (the client must speak HTTP over a Unix socket natively), a curl example, and a link to the docs page that carries the per-client detail. The legacy banner's hardcoded "Claude Code:" label is now computed alongside it, so the socket bind no longer addresses one agent by name; both branches still render to the same 70-column inner width. Per-client wiring stays in the docs, where it can be qualified. Three tests pin the contract: the unsupported recipe is absent, the replacement states the requirement and links the docs, and no agent is named at all. They are pure string assertions, so they run on every platform rather than only where AF_UNIX exists.
|
@JerrettDavis Good catch, and the framing is right — I corrected the docs to record the negative result and left the runtime output still handing users the exact configuration those docs call broken. Fixed in 73f77da. Both banners, not one. The recipe was in the click CLI banner and in The legacy banner's label was hardcoded The function carries a comment explaining why it must stay agent-neutral, so the next person to add a convenience line here sees the field result rather than rediscovering it. Regression coverage, as requested — three tests, all pure string assertions so they run everywhere rather than only where
One thing I caught while writing this: my first draft of the banner linked Local state: I cannot re-request review from this account (no write access on the base repo — the API 404s), so tagging you here instead. |
JerrettDavis
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 73f77da after the requested correction. The startup surfaces no longer advertise the nonworking Claude Code configuration: both banners now stay transport-neutral, the detailed CLI banner points to qualified documentation, and the regression tests explicitly prevent reintroducing agent/env guidance. I also rechecked the UDS bind/path lifecycle and default TCP path against current main. Local validation: 50 passed, 14 platform skips; Ruff passed. Exact-head CI is fully green. The earlier blocker is resolved; ready for final human merge review.
Uh oh!
There was an error while loading. Please reload this page.