Repository navigation
Keep shell shims, wait-for signals and debug logs in private per-user paths - #15179
austinywang wants to merge 10 commits into
Conversation
…ectories The claude shim directory and bash's PR-hint and history files are created under TMPDIR with mkdir -p and shell redirection, which accept a directory another user created and follow symlinks. These cases pre-create shared, group-writable and symlinked directories and check that zsh, bash and fish leave them alone, keep them off PATH and never run what they contain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ories The zsh, bash and fish integrations now write and trust the claude CLI shim only from a per-user directory that is ours, not a symlink, and not group- or other-writable. A missing directory is created 0700. An inherited shim root that fails the check falls back to a fresh private root, and if none can be made the shim is skipped and the bundled wrapper runs instead. The claude function only runs the shim this shell verified. The bash PR-hint and history scratch files move into the same kind of private directory, and the tracked background job no longer needs a pid file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Moves the wait-for signal file handling out of cmux.swift, unchanged, into TmuxWaitForSignal so a small driver can compile and exercise it. The new test fails today: signals land in the shared /tmp, and a waiter accepts a symlink at the signal path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signal files move from the shared /tmp into cmux-wait-for under the per-user temporary directory, which comes from confstr so both ends agree regardless of TMPDIR. The directory is created 0700 and must be a real directory we own with no group or other write bit. Signals are created with O_NOFOLLOW relative to that directory, and a waiter only accepts a regular file we own. The wait uses a kqueue on the verified directory instead of the path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Moves the three /tmp debug log writers onto one OwnedLogFile helper with their current behavior, and adds tests that a log path which is a symlink, a hard link or a missing file is handled privately. They fail until the helper is hardened. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The /tmp debug logs (cmux-bg.log, cmux-ghostty-init.log and cmux-panel-debug.log) now open with O_NOFOLLOW and are kept only when the result is a regular file this user owns with a single link. New log files are created 0600. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 1 minute. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (15)
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. Comment |
|
All contributors have signed the CLA ✍️ ✅ |
This comment has been minimized.
This comment has been minimized.
CI failure attributionCI passes on Written by |
|
Automatic catch-up couldn't merge Label |
tests/test-execution.toml: keep this branch's shell shim test and main's workspace shim root owner test (#15185) as separate macos-shell entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
The package-conventions lint rejects all-static namespace types. OwnedLogFile is now a struct holding the log path and the user that must own it, like PrivateDirectoryCheck, and callers open it with OwnedLogFile(path:).openForAppending(). The injected owner also lets the tests cover a file another user owns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The wake-on-later-signal test slept half a second and then asserted on elapsed time, which the test-determinism guard rejects. wait(timeout:) now takes a callback that runs once the directory watch is registered and the first check found no signal. The fixture reports it on stderr, the test signals only after reading it, and the waiter's own timeout is far past communicate()'s, so its OK can only come from the signal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Dogfood tours of
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Automatic catch-up couldn't merge Label |
|
Closing as superseded by merged PR #15116, which consolidated this SSH/security/local-state hardening into main. |










Summary
Some of the files cmux shell integration, the CLI and the app debug logs write sit at fixed names in shared temporary directories. Another local user could create one of those paths first, or point it somewhere else. This PR uses only a private per-user location for these files, and opens each file without following links.
Claude command shim (zsh, bash, fish).
_cmux_install_cli_command_shimnow puts shims in a directory that must be:Missing directories are created 0700. The parent is checked before the child.
The same check runs before an existing shim root is trusted for lookup. If a root fails the check, the shim is skipped: nothing falls back to a shared path. An inherited root that fails the check is also taken out of
PATH.Remote hosts get the same fix, because
RemoteInteractiveShellBootstrapBuilderembeds these integration files.Bash hint and history state. These files used to go straight into
${TMPDIR:-/tmp}. They now go in…/cmux-bash-$EUID, which is held to the same private-directory rule.cmux wait-for. Signal files move out of/tmp/cmux-wait-for-<name>.sigintocmux-wait-forunder the per-user temporary directory (confstr(_CS_DARWIN_USER_TEMP_DIR)). That directory doesn't depend onTMPDIR, so the signalling side and the waiting side resolve the same path.-Screates the signal file withopenat(O_NOFOLLOW), relative to the verified directory.fstatat(AT_SYMLINK_NOFOLLOW), and then removes it.App debug logs. Three logs share a new helper,
OwnedLogFile.openForAppending:/tmp/cmux-bg.log/tmp/cmux-ghostty-init.log/tmp/cmux-panel-debug.logThe helper opens with
O_NOFOLLOWand keeps the file only if it's a regular file owned by this user with a single link. New log files are created 0600.Out of scope, as follow-ups:
TerminalSurface+AgentCommandShims.swift) should get the same owner and symlink checks.ROOTPATHprepend inWorkspaceInitialCommandLoginShellshould get the same checks.#14642 edits the same three shell-integration files, so whichever lands second will need a rebase.
Testing
Each fix follows its failing test in the commit history. For each pair below, the red and green results come from the same command.
python3 tests/test_shell_cli_shim_private_dir.py54952742ca: exit 1, 10 tests,FAILED (failures=38)51add4c847: exit 0, 10 tests, OKcmux wait-forpython3 tests/test_cli_tmux_wait_for_private_dir.pyac1c6004ec: exit 1, 6 tests,FAILED (failures=3)9e5b2c58ff: exit 0, 6 tests, OKswift test --package-path Packages/macOS/CmuxFoundation --filter OwnedLogFileTests7556730023: exit 1, 5 tests, 4 failed with 6 issuesaf076601a4: exit 0, 5 passedNotes on the red commits:
CLI/TmuxWaitForSignal.swiftunchanged, so the test can compile it on its own.OwnedLogFilewith their old behavior.At head
d780d95035:python3 scripts/verify-local.py --affected origin/main --swift-changed origin/mainpassed 15/15 checks.These related shell tests also pass on this branch:
test_claude_wrapper_shim_root_survives_tmpdir_changetest_issue_9356_bash_shim_noclobbertest_issue_6714_zsh_shim_noclobbertest_claude_wrapper_mutual_shim_looptest_issue_2448_shell_claude_wrapper_dispatchtest_issue_13343_claude_integration_toggletest_claude_wrapper_user_binary_resolutiontest_shell_first_prompt_spawnstest_bash_integration_no_done_notificationstest_shell_no_git_watchtest_nushell_shim_path_refrontwas skipped becausenuisn't installed.Not verified:
FileBackgroundLogLineSinkchange haven't been compiled. The wait-for code was compiled only on its own, withswiftc -swift-version 5.cmux-wait-forto 0770 and restores it infinally.Known limits:
TMPDIRis world-writable and not sticky, another user could still rename the checked directory between the check and its use./tmp/cmux-cli-shims, the shim is disabled instead of falling back.Localization: no user-facing UI strings changed. The new wait-for errors are non-localized
CLIErrormessages, like the existing wait-for errors.Changelog
Fixed: The Claude command shim, bash history state,
cmux wait-forsignals and debug logs no longer use shared temporary paths that another local user could create or redirect.Checklist
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Moves shell command shims,
cmux wait-forsignal files, bash history state, and debug logs out of shared temporary paths into private per-user locations, so another local user can no longer pre-create or redirect those paths.Behavior
PATH. If no private shim root can be made, the shim is skipped and the bundled wrapper runs instead.<TMPDIR>/cmux-bash-<euid>, held to the same private-directory rule.cmux wait-forsignal files move to the per-user temp directory, resolved viaconfstrso both ends agree regardless ofTMPDIR; files are created and consumed without following symlinks, and the waiter uses a kqueue on the verified directory instead of polling the file path./tmpdebug logs (cmux-bg.log,cmux-ghostty-init.log,cmux-panel-debug.log) open withO_NOFOLLOWand are kept only when owned by the current user with a single link; new files are 0600.Notes
ROOTPATHprepend are follow-ups.TMPDIRis world-writable and not sticky, a check-to-use race remains; on a shared host an already-owned shim directory disables the shim rather than replacing it.OwnedLogFilehelper are covered by tests.Written for commit 9d58d1d. Summary will update on new commits.