fix: read socket-control-password from XDG state dir (cmux 0.64+)#8
fix: read socket-control-password from XDG state dir (cmux 0.64+)#8JoyciAkira wants to merge 1 commit into
Conversation
cmux 0.64 moved its state from ~/Library/Application Support/cmux to ~/.local/state/cmux and writes socket-control-password there when socketControlMode=password is configured. The relay was only checking the legacy Application Support path, so auth.login was never called and cmux rejected every connection with 'Access denied'. Fix cmuxSocketPassword() to check ~/.local/state/cmux/socket-control-password first (modern convention, matching the existing cmuxSocketPath() logic), then fall back to ~/Library/Application Support/cmux/ for older cmux builds. Also isolate existing password tests from the live host state dir, add three new tests covering the XDG-first behaviour, and document the socketControlMode=password prerequisite in connection-guide.en.md. Co-authored-by: Cursor <[email protected]>
📝 WalkthroughWalkthroughSocket password lookup now checks an optional state directory before falling back to Application Support, while keeping environment-variable precedence. Tests cover the new ordering and missing/blank cases. The connection guide adds a cmux 0.64+ socket access-denied troubleshooting entry. ChangesSocket password resolution and guidance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/connection-guide.en.md`:
- Around line 155-164: The heredoc in the connection guide is quoted, so the
`$(openssl rand -hex 24)` expression in the `cmux.json` example is written
literally instead of being evaluated. Update the `cmux.json` generation snippet
to use an unquoted heredoc in that example so the `socketPassword` field is
populated with a real random secret, and keep the rest of the `automation`
config unchanged.
- Around line 174-176: Remove the “No relay restart is needed” claim from the
connection guide and update the surrounding text to reflect that the relay only
reads the password once via cmuxSocketPassword() during CmuxConnection
initialization; since the SIGHUP path only reloads relay config and does not
rebuild the connection, any password file change requires reconnecting or
restarting the process.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7823fec5-f6ab-472c-8836-9146ed36fcfe
📒 Files selected for processing (3)
Sources/CMUXClient/UnixSocketChannel.swiftTests/CMUXClientTests/CmuxSocketPathTests.swiftdocs/connection-guide.en.md
| ```bash | ||
| mkdir -p ~/.config/cmux | ||
| cat > ~/.config/cmux/cmux.json << 'EOF' | ||
| { | ||
| "automation": { | ||
| "socketControlMode": "password", | ||
| "socketPassword": "$(openssl rand -hex 24)" | ||
| } | ||
| } | ||
| EOF |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove the quoted heredoc here.
Line 157 uses << 'EOF', so Line 161 writes the literal text $(openssl rand -hex 24) into cmux.json instead of a random secret. That makes the documented password predictable.
Suggested fix
mkdir -p ~/.config/cmux
-cat > ~/.config/cmux/cmux.json << 'EOF'
+cat > ~/.config/cmux/cmux.json <<EOF
{
"automation": {
"socketControlMode": "password",
"socketPassword": "$(openssl rand -hex 24)"
}
}
EOF📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```bash | |
| mkdir -p ~/.config/cmux | |
| cat > ~/.config/cmux/cmux.json << 'EOF' | |
| { | |
| "automation": { | |
| "socketControlMode": "password", | |
| "socketPassword": "$(openssl rand -hex 24)" | |
| } | |
| } | |
| EOF |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/connection-guide.en.md` around lines 155 - 164, The heredoc in the
connection guide is quoted, so the `$(openssl rand -hex 24)` expression in the
`cmux.json` example is written literally instead of being evaluated. Update the
`cmux.json` generation snippet to use an unquoted heredoc in that example so the
`socketPassword` field is populated with a real random secret, and keep the rest
of the `automation` config unchanged.
| The relay reads the password automatically from | ||
| `~/.local/state/cmux/socket-control-password` (written by cmux on | ||
| reload). No relay restart is needed. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Outline the downstream consumer =="
ast-grep outline Sources/RelayCore/CmuxConnection.swift --view expanded || true
echo
echo "== Find CmuxConnection construction sites =="
rg -nC3 '\bCmuxConnection\s*\(' Sources Tests
echo
echo "== Find cmuxSocketPassword call sites =="
rg -nC3 '\bcmuxSocketPassword\s*\(' Sources TestsRepository: NewTurn2017/cmux-remote
Length of output: 8394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== CmuxConnection implementation =="
sed -n '1,220p' Sources/RelayCore/CmuxConnection.swift
echo
echo "== CmuxRelay startup flow =="
sed -n '1,180p' Sources/RelayServer/CmuxRelay.swift
echo
echo "== Search for relay reload/recreate paths =="
rg -nC2 'reload-config|reload\(|invalidateEvents\(|connectForEvents\(|observe\(bootInfo\)' Sources TestsRepository: NewTurn2017/cmux-remote
Length of output: 16507
Drop the “No relay restart is needed” claim
CmuxConnection reads cmuxSocketPassword() once during initialization and reuses that cached value for all reconnects, and the SIGHUP handler only reloads relay config — it does not recreate the connection. If the password file changes after startup, the running relay won’t pick it up until the connection is rebuilt or the process restarts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/connection-guide.en.md` around lines 174 - 176, Remove the “No relay
restart is needed” claim from the connection guide and update the surrounding
text to reflect that the relay only reads the password once via
cmuxSocketPassword() during CmuxConnection initialization; since the SIGHUP path
only reloads relay config and does not rebuild the connection, any password file
change requires reconnecting or restarting the process.
Problem
cmux 0.64+ introduced
socketControlMode(default:cmuxOnly) which restricts socket access to processes launched inside cmux. When configured topasswordmode, cmux writes the password to~/.local/state/cmux/socket-control-password(XDG state dir). The relay was only checking~/Library/Application Support/cmux/socket-control-password(legacy path), soauth.loginwas never called and cmux rejected every relay connection withAccess denied — only processes started inside cmux can connect.Fix
Update
cmuxSocketPassword()to check~/.local/state/cmux/socket-control-passwordfirst, then fall back to~/Library/Application Support/cmux/for older cmux builds. This mirrors the exact same two-path strategy already used bycmuxSocketPath()for the socket itself.Required cmux configuration
Users on cmux 0.64+ must add
~/.config/cmux/cmux.json:{ "automation": { "socketControlMode": "password", "socketPassword": "<any-strong-password>" } }Then reload from inside a cmux terminal:
cmux reload-config. The relay picks up the password automatically — no restart needed.Changes
Sources/CMUXClient/UnixSocketChannel.swift—cmuxSocketPassword()now acceptsstateDirectory:and checks XDG state dir firstTests/CMUXClientTests/CmuxSocketPathTests.swift— isolate existing tests from live host state dir; add 3 new tests for XDG-first behaviourdocs/connection-guide.en.md— document thesocketControlMode=passwordprerequisite with step-by-step fix in troubleshooting section ③aTested
CmuxSocketPathTestspassAccess denied)Made with Cursor
Summary by CodeRabbit
New Features
Bug Fixes
Documentation