Check the owner of the Claude shim directory in the app, workspace commands and nushell

[austinywang]

Summary

This follows #15179, which added owner checks for the Claude command shim directory (cmux-cli-shims/<surface>) in the zsh, bash and fish integrations. It covers the three places #15179 left as follow-ups. Each of them still trusted that directory:

• the app, which writes the shims into it;
• a workspace's initial command, which puts it first on PATH;
• the nushell bootstrap, which moves it to the front of PATH.

In a shared temporary directory, another user could create cmux-cli-shims or a surface directory first, or put a symlink there. Now each place checks the directory first. If the check fails, it skips the shim and leaves PATH alone. Nothing moves to a new location.

App shim writer. TerminalSurface.installAgentCommandShimsIfPossible now checks three directories: the shim parent, the staging directory and the final surface directory. It uses a new PrivateDirectoryCheck helper in CmuxFoundation, which:

• opens the path with O_NOFOLLOW | O_DIRECTORY;
• keeps it only if it's a directory owned by the effective user, then sets it to 0700 with fchmod;
• confirms with lstat that the path still names that directory and that group and others can't write to it.

If any directory fails, the installer returns nil and the surface starts without the shim. The old code set permissions with FileManager.setAttributes, which follows symlinks. The helper is in CmuxFoundation so its tests can run without GhosttyKit.

Workspace initial command. WorkspaceInitialCommandLoginShell prepends $CMUX_CLAUDE_WRAPPER_SHIM_ROOT only when it's a directory, isn't a symlink and belongs to this user. In POSIX shells the check is [ -d ] && [ ! -L ] && [ -O ]. In fish it's test -d, not test -L and test -O.

Nushell. Nushell has no built-in owner check, so the bootstrap asks /usr/bin/stat. Without -L, stat doesn't follow a symlink. The bootstrap used to move every PATH entry containing cmux-cli-shims to the front. Now it moves only the surface's shim root, and only when stat reports a directory owned by id -u. Other entries keep their order.

The bootstrap looks for the shim root in two variables:

• $CMUX_AGENT_COMMAND_SHIM_ROOT, which the app sets whenever any agent shim is installed;
• $CMUX_CLAUDE_WRAPPER_SHIM_ROOT, which the app sets only for the Claude shim.

With Claude integration off, the Codex, Pi, Amp and Hermes shims keep their place ahead of the user's PATH prepends.

Known limits:

• The workspace-command and nushell checks don't look at group or other write bits. The app writer already makes the directory 0700.
• The workspace initial command still prepends only $CMUX_CLAUDE_WRAPPER_SHIM_ROOT, the same as on main.

#14642 edits TerminalSurface+AgentCommandShims.swift, its permissions tests and tests/test-execution.toml. Whichever PR lands second will need a rebase.

Testing

Each fix comes after its failing test in the commit history. For each pair below, the red and green results come from the same command.

Area	Command	Red	Green
App shim writer (helper)	swift test --package-path Packages/macOS/CmuxFoundation --filter PrivateDirectoryCheckTests	b97e33bbf7: 5 tests, 3 failed with 6 issues (symlink, other owner, regular file)	8df112be57: 5 passed
Workspace initial command	python3 tests/test_workspace_initial_command_shim_root_owner.py	d6ea06f94b: 5 tests, FAILED (failures=12); the symlink and other-owner cases failed in zsh, bash, sh, ksh, dash and fish	1f92ca694f: 5 tests, OK
Nushell (owner checks)	CMUX_TEST_NU_BIN=<nu 0.113.1> python3 tests/test_nushell_shim_path_refront.py	main's bootstrap: failed at the symlinked-root case	5905e00cfd: all cases passed
Nushell (without Claude integration)	same command	be36a5785b: failed; which claude found the user's decoy, not the shim	c6fd61da22: all 7 cases passed

About the tests:

• Workspace-command test. It compiles WorkspaceInitialCommandLoginShell.swift with a small driver and runs the wrapped command in each available login shell. It's registered in the macos-shell lane. Its other-owner case uses /usr/share, which root owns.

• Nushell test. It ran with nushell 0.113.1, the checksum-verified release binary CI pins. It has four new cases:

  • a symlinked root;
  • a root another user owns (/usr/share);
  • other cmux-cli-shims entries staying where they are;
  • a shim root that only $CMUX_AGENT_COMMAND_SHIM_ROOT names.

  The other-owner case passes on main too, because the old bootstrap never moved /usr/share. tests/test_nushell_integration_hooks.py and tests/test_nushell_resume_command_dialect.py also pass with the new bootstrap.

At 5905e00cfd:

• python3 scripts/verify-local.py --affected origin/main --swift-changed origin/main passed 15/15 checks.
• python3 scripts/ci/validate_test_execution_registry.py passed.
• python3 tests/test_workspace_initial_command_shim_root_owner.py passed.

At head c6fd61da22, python3 scripts/verify-local.py --affected origin/main passed 14/14 checks.

Not run:

• CmuxTerminal package tests. These include the two new symlink cases in TerminalSurfaceCommandShimPermissionsTests. GhosttyKit.xcframework isn't available locally, so the TerminalSurface+AgentCommandShims.swift change hasn't been compiled either.
• cmuxTests. The six exact-string wrap tests in WorkspaceCreateWorkingDirectoryTests were updated but not run. Their expected strings were checked against the compiled driver's output.
• No app build. Nothing was checked live in a tagged build.

No user-facing strings changed.

Changelog

• Fixed: cmux no longer trusts a Claude command shim directory another user owns.

Checklist

• Behavior changes have added or updated tests, or Testing says why not
• Reviewed with a subagent before merge (cmux-review), and all bot and human review comments resolved

🤖 Generated with Claude Code

---

Summary by cubic

This extends the Claude shim directory owner checks from the existing shell integrations to the three places that still trusted it: the app that writes the shims, a workspace's initial command that prepends them to PATH, and the nushell bootstrap that refronts them. Each now uses the directory only when it is a real directory this user owns; otherwise the surface starts without the shim and PATH keeps its order.

Behavior changes

• The app shim writer uses a new PrivateDirectoryCheck helper that opens paths without following symlinks, sets mode 0700 via fchmod, and re-verifies ownership with lstat; it replaces FileManager.setAttributes, which followed symlinks.
• Workspace login-shell wrappers (POSIX and fish) prepend $CMUX_CLAUDE_WRAPPER_SHIM_ROOT only when directory, not-symlink, and owned-by-user checks pass.
• Nushell moves only $CMUX_CLAUDE_WRAPPER_SHIM_ROOT to the front of PATH, and only when /usr/bin/stat (which doesn't follow symlinks) reports a directory owned by the user; other cmux-cli-shims entries stay in place.

Limits

• The workspace and nushell checks don't test group or other write bits; the app writer sets the directory to 0700 first.
• When Claude integration is off, CMUX_CLAUDE_WRAPPER_SHIM_ROOT isn't set, so nushell no longer moves that directory to the front of PATH.
• Keep per-pane Claude shims outside TMPDIR #14642 edits the same app shim files; whichever PR merges second needs a rebase.

^{Written for commit 5905e00. Summary will update on new commits.}

Summary by CodeRabbit

• Security
  • Shim directories are now checked to ensure they are real directories owned by the current user before they are installed or added to the shell search path.
  • Symlinked, missing, incorrectly owned, or non-directory paths are rejected, helping prevent unsafe shim use.
  • Shell path order is preserved except when a validated shim directory is moved to the front.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 4a4b2831-95c9-48b1-9f32-d50041749641

📥 Commits

Reviewing files that changed from the base of the PR and between 47a223c and c6fd61d.

📒 Files selected for processing (11)

• Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/PrivateDirectoryCheck.swift
• Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/PrivateDirectoryCheckTests.swift
• Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Spawn/TerminalSurface+AgentCommandShims.swift
• Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceCommandShimPermissionsTests.swift
• Resources/shell-integration/nushell/cmux-nushell-bootstrap.nu
• Sources/WorkspaceInitialCommandLoginShell.swift
• cmuxTests/WorkspaceCreateWorkingDirectoryTests.swift
• tests/fixtures/WorkspaceInitialCommandLoginShellFixture.swift
• tests/test-execution.toml
• tests/test_nushell_shim_path_refront.py
• tests/test_workspace_initial_command_shim_root_owner.py

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

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

[github-actions]

CI failure attribution

CI passes on c6fd61da22 (run 36384441972 attempt 2).

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

[teamleaderleo]

Review: security review. Merging this, it is a clear net improvement over main, which had no owner check at all. One divergence between the three implementations is worth tightening, but it is narrower than what the PR closes and you already name it in Known limits, so I would rather have this in than hold it.

The divergence, for the record

The app's check is the strong one, PrivateDirectoryCheck.swift:46:

&& current.st_uid == owner
&& current.st_mode & (S_IWGRP | S_IWOTH) == 0

plus it forces the directory to 0700 with fchmod before trusting it.

The two shell-level checks stop at ownership. WorkspaceInitialCommandLoginShell.swift:48,67:

[ -d "$ROOT" ] && [ ! -L "$ROOT" ] && [ -O "$ROOT" ]

and nushell compares stat -f "%HT:%u" against Directory:<uid> and nothing else.

So a shim root owned by the invoking user but group- or world-writable is refused by the app's own writer and trusted by both shells. That matters on a shared Mac because macOS puts every local account in staff, so another local user in that group can drop a claude file, or a symlink to one, into a group-writable shim root, and it runs as the victim at the next login shell or nushell start. In practice the app creates these directories and forces 0700, so the reachable case is a leftover directory from a build that predates this hardening, or one loosened by something else. Narrow, but it is the classic "three implementations, the weakest one is the policy" shape and the reason I would close it rather than leave it.

Porting the bit test is small: stat -f %Lp in nushell, and an explicit permission check alongside the existing tests in the POSIX and fish payloads.

What I checked that holds up

Fails closed everywhere. PrivateDirectoryCheck returns false on any failure across open, fstat, fchmod and lstat. The POSIX and fish && and and chains short-circuit to not-prepending on any failed test. The nushell version wraps the whole computation in try { } catch { false } and returns false immediately for a non-absolute root.

Owner acceptance is correctly narrow: all three require the current euid only, and none of them accept root or any other owner. Your /usr/share test case covers that.

Symlink handling is genuinely careful, and this is the part I liked most. The app opens with O_NOFOLLOW|O_DIRECTORY, and TerminalSurface+AgentCommandShims.swift:173-174 re-checks the destination with makePrivate immediately before replaceItemAt, which is exactly the right move since replaceItemAt will otherwise happily follow a symlink at the destination. installSkipsSymlinkedShimParent and installSkipsSymlinkedSurfaceDirectory cover it, and test_symlink_is_not_prepended covers the shell side.

Nothing legitimate breaks. The PR only touches app-generated per-surface directories under the user's private temp directory, never Homebrew's paths or root-owned ones, so Homebrew installs, admin-group setups and multi-user Macs are unaffected.

Also worth noting, not a defect

Trust is established once at shell start and never re-verified when claude is actually resolved from PATH later, and the -d/-L/-O tests are three separate non-atomic stats. That stat-then-exec window only matters if the parent directory is writable by someone else, which collapses back into the group-writable case above, so fixing that one closes this too.

The workspace-command wrapper only prepends CMUX_CLAUDE_WRAPPER_SHIM_ROOT while nushell refronts both roots. That is a scope difference, not a weaker check.

Fixed: nothing needed to merge.

Left: the group and other write-bit check in the POSIX, fish and nushell payloads, plus a test with an owned-but-group-writable directory for each, since none of the current tests cover that shape. Worth a follow-up issue so it does not get lost.

[github-actions]

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