Skip to content

cmux-tui: only connect to derived local sockets served by this user - #15144

Merged
teamleaderleo merged 2 commits into
mainfrom
cmux-tui-private-socket-fallback
Sep 28, 2026
Merged

teamleaderleo merged 2 commits into
mainfrom
cmux-tui-private-socket-fallback

Conversation

@austinywang

@austinywang austinywang commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

When the preferred runtime path is too long, cmux-tui derives a session socket in a /tmp/cmux-tui-<uid> or hashed fallback directory. Clients connected to that path without checking the directory or who was listening. The remote daemon's ensure_daemon also passed its derived mux path to the mux owner as an explicit --socket, which skipped the owner's own directory check.

Derived session sockets are now used only when they are in a private directory owned by this user and are served by this user:

  • One shared helper, cmux_tui_core::server::connect_session_socket, checks the directory, then connects with platform::transport::connect_same_user. It refuses the listener before writing anything if the listener's peer uid isn't this user. Attach, relay, launch, the detached owner, the raw/wire/lifecycle CLI, local-owner startup and the machine agent all use it.
  • The Unix listener refuses clients from other users. Root is still admitted.
  • ensure_daemon now lets the mux owner derive its own socket instead of passing --socket. The remote daemon, sidecar and mux monitor check that the mux or terminal-host listener belongs to this user. That includes CMUX_MUX_SOCKET, because the daemon starts that owner itself.
  • Terminal hosts keep /tmp/cmux-th-<uid> owned by this user at 0700 and refuse a symlink. They check the host's uid before sending the owner capability.

Nothing changes for an explicit --socket or env socket. Windows keeps a plain connect.

Compatibility

There's no protocol change.

  • Servers: every existing server already creates the derived directory at 0700 and the socket at 0600, so a same-user server passes the new client checks. The only clients the new accept check refuses are other non-root users, and they couldn't open a 0600 socket anyway.
  • Remote daemon: ensure_daemon, the sidecar and the mux owner are the same binary, so they derive the same path.
  • Mixed versions and missing checks: a new client talking to a server run by another user fails with PermissionDenied, and so does a Unix target without peer-credential support.
  • Root: root can no longer reuse another user's /tmp/cmux-th-<uid>.

Not changed

  • Hook and agent-browser sockets come only from explicit env.
  • The machine-provider socket is configured by the operator.
  • schema_socket_owner sends only an identify probe.

Testing

Focused command: ./scripts/verify-cmux-tui-hosted.sh --filter private_socket (hosted cargo test --workspace --locked private_socket, Linux and macOS).

  • Red, c8a4a1d (run): failed on Linux and macOS. remote_cli::tests::private_socket_remote_mux_owner_derives_its_own_socket failed because the mux owner got --socket. Cargo stops at the first failing test binary, so the cmux-tui-core regressions in that run were compiled but not executed. Lint failed as expected on the red stub's unused peer_may_connect.
  • Green, b087153 (run): passed. All five private_socket tests passed on Linux and macOS, as did lint (Linux, macOS), Rust MSRV 1.91 and the release-path artifact build.
    • remote_cli::tests::private_socket_remote_mux_owner_derives_its_own_socket
    • platform::tests::private_socket_listener_admits_only_the_owner_and_root
    • platform::tests::private_socket_peer_uid_must_match_the_expected_user
    • server::tests::private_socket_connect_requires_a_private_derived_parent
    • terminal_host_runtime::unix::tests::private_socket_terminal_host_endpoint_dir_refuses_a_symlink
  • python3 scripts/verify-local.py: passed (portable checks only; no native build).

Not run:

  • --full hosted verification, including Windows.
  • Tagged-build dogfood.
  • A live remote host.

Changelog

  • Fixed: cmux-tui clients, the remote daemon and terminal hosts only use derived local sockets served by the same user.

🤖 Generated with Claude Code

austinywang and others added 2 commits September 27, 2026 20:49
Add failing tests for the checks a client should make before it writes to
a socket at a path it derived: the socket directory is a private one this
user owns, the listener runs as this user, the listener refuses other
users, terminal hosts refuse a symlinked endpoint directory, and the remote
daemon lets the mux owner derive its own socket.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A socket path cmux derives from a session name can fall back to a shared
/tmp name. Before a client writes to one it now checks that the socket's
directory is a private one this user owns and that the listener runs as
this user. Explicit --socket and environment-selected paths keep their
current behavior.

The listener refuses clients from other users (root is still allowed).
The remote daemon lets the mux owner derive its own socket instead of
passing it back as an explicit --socket, and it and its sidecar only talk
to mux and terminal-host listeners running as the same user. Terminal
hosts keep their shared /tmp endpoint directory owned by this user and
private, and check the host's uid before sending the owner capability.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cursor

cursor Bot commented Sep 28, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 4 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 79fe52d2-22bf-4c6d-91f4-cb8b05b8ca45

📥 Commits

Reviewing files that changed from the base of the PR and between 55b4049 and b087153.

📒 Files selected for processing (15)
  • cmux-tui/crates/cmux-remote/src/services.rs
  • cmux-tui/crates/cmux-tui-core/src/platform.rs
  • cmux-tui/crates/cmux-tui-core/src/provider_management.rs
  • cmux-tui/crates/cmux-tui-core/src/server.rs
  • cmux-tui/crates/cmux-tui-core/src/terminal_host_runtime.rs
  • cmux-tui/crates/cmux-tui/src/cli/lifecycle.rs
  • cmux-tui/crates/cmux-tui/src/cli/raw.rs
  • cmux-tui/crates/cmux-tui/src/cli/wire.rs
  • cmux-tui/crates/cmux-tui/src/local_owner.rs
  • cmux-tui/crates/cmux-tui/src/machine_agent/mod.rs
  • cmux-tui/crates/cmux-tui/src/machine_agent/transport.rs
  • cmux-tui/crates/cmux-tui/src/main.rs
  • cmux-tui/crates/cmux-tui/src/remote_cli.rs
  • cmux-tui/crates/cmux-tui/src/session/remote.rs
  • cmux-tui/crates/cmux-tui/tests/cli.rs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Review (subagent, correctness-first; I re-verified the load-bearing check and the cross-PR detail myself)

Findings: none blocking.

The question I cared about was whether this is an fd check or a path check, because a pre-connect path stat would be TOCTOU and would not actually close anything. It is an fd check. connect_same_user (cmux-tui/crates/cmux-tui-core/src/platform.rs:76-80) connects, then calls require_unix_peer_uid(&stream, effective_uid()) before returning the stream to the caller, so nothing is written first. That reads SO_PEERCRED on Linux (:198-222) or getpeereid on BSD/macOS (:224-239), which is kernel-reported and cannot be spoofed by symlink or directory games. Listener::accept (:82-94) mirrors it on the serving side via peer_may_connect. The pre-connect verify_private_socket_directory in server.rs:5018-5033 is then just a cheap early exit, which is the right role for it.

Also confirmed:

  • Directory hardening is correct, including the narrow race. prepare_runtime_socket_directory (server.rs:4907-4950) and prepare_endpoint_dir (terminal_host_runtime.rs:3079-3111) both re-verify with symlink_metadata after create_dir_all/chmod, which catches the case where mkdir's AlreadyExists + is_dir() fallback followed a planted symlink. Derived dirs are uid-suffixed (/tmp/cmux-tui-<uid>, /tmp/cmux-th-<uid>), so two real uids cannot collide on a name.
  • Client and daemon agree. connect_owned_unix_socket (cmux-remote/src/services.rs:1526-1533) reuses admin::verify_unix_peer_owner, the same SO_PEERCRED exact-uid check the admin socket already used. And ensure_daemon no longer hands --socket to the mux owner it spawns on a derived path (mux_owner_args, remote_cli.rs:2497-2509), so the owner re-derives and checks its own path instead of trusting one passed in. That was the actual bug.
  • Nothing ordinary breaks. The check is purely uid-based, so containers, multiple login sessions and differing ttys under the same uid are unaffected. I looked for a fail-closed path that makes cmux-tui unusable and did not find one.
  • Not stale: zero commits on main touch any of the 15 changed files since the merge-base, despite main being 55 ahead.

Coverage, checked rather than assumed: the five private_socket_* tests ran and passed on head b08715363fda in run 36376260009, in both test (linux) and test (macos), with TEST_FILTER: private_socket. Real filtered cargo lane, not a skipped job. The focused hosted verification gate reports MODE: focused with Windows/valgrind/CDP/bindings/web skipped, which matches your own "Not run: --full hosted verification" disclosure.

Fixed: nothing needed.

Left, none blocking:

  1. Root can no longer transparently reuse another user's derived socket and must pass an explicit --socket. You disclose this in the Compatibility section and the escape hatch works (connect_session_socket skips the new checks when is_derived=false), so this is intended hardening, just flagging it as the one real behavior change.
  2. Cross-PR inconsistency worth a follow-up, and I checked this rather than assuming: chatmux-relay: keep cmux-tui sockets and journal cursors private to this user #15156, which I merged a few minutes ago, implements its own require_peer_uid in chatmux-relay/src/control.rs and passes libc::getuid() (real uid, control.rs:119 on main), while this PR uses libc::geteuid() (effective uid, platform.rs:169-171) throughout. Under sudo those differ, so the two halves of the sweep would disagree about who the owner is. Neither is wrong on its own and it does not block either PR, but the sweep should settle on one.
  3. Windows keeps a plain connect (platform.rs:136-138) since Windows sockets do not expose peer credentials. Disclosed, and a known platform gap rather than something introduced here.
  4. CMUX_TUI_SOCKET/CMUX_MUX_SOCKET are treated as explicit by resolve_socket_with_origin (cli/wire.rs:751-773) so CLI commands skip the peer check, while ensure_daemon's internal use of the same env var is checked. Two trust models for one variable name. Unchanged from before, but easy to trip over later.

Merging :)

@teamleaderleo
teamleaderleo merged commit c307ab0 into main Sep 28, 2026
79 of 80 checks passed
@teamleaderleo
teamleaderleo deleted the cmux-tui-private-socket-fallback branch September 28, 2026 08:47
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for b08715363f: every check was green at merge (12 verified; 16 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 28, 2026
ba94a13 CI: let Iroh release gate reuse unchanged TUI artifact
71a921c fix(web): stop orphaned Cloud VM alert pages (manaflow-ai#15138)
9971c2c Keep newer iOS connections alive when a recovery is superseded (manaflow-ai#15141)
c307ab0 cmux-tui: only connect to derived local sockets served by this user (manaflow-ai#15144)
1220252 codex-teams: keep the watcher's socket password out of its arguments (manaflow-ai#15140)
b3a73f0 chatmux-relay: keep cmux-tui sockets and journal cursors private to this user (manaflow-ai#15156)
b0d5083 ci: dispatch UI tests from a default-branch workflow; PR CI keeps no write token (manaflow-ai#15226)
1255448 test: fix three app-host tests that keep main red (manaflow-ai#15204)
0fc4975 test: pin the fixture PATH inside the zsh watcher sleep test (manaflow-ai#15237)
758aaeb fix(ios): clear read notifications on foreground return (manaflow-ai#14725)
4c15bb3 cmux-browser: stop requiring GPL for web/package.json (manaflow-ai#15231)
97fe6b4 test: keep the Cloud notification harness workspace unselected (manaflow-ai#15215)
61083e3 test: keep workspace cwd inheritance tests off the shared standard defaults (manaflow-ai#15227)
eae4994 Pin password badge actions to their source runtime (manaflow-ai#14921)
fd96369 Check the owner of the Claude shim directory in the app, workspace commands and nushell (manaflow-ai#15185)
0ebf8d7 Fix main-thread freeze during SSH paste detection (manaflow-ai#15113)
a98c560 test: pin font magnification in the Cloud outline attention test (manaflow-ai#15213)
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.

2 participants