Fix cmux ssh against hosts configured with RemoteCommand/RequestTTY - #7359
Conversation
…and/RequestTTY cmux ssh against a host alias whose ssh_config sets `RequestTTY yes` and `RemoteCommand sudo su -` exits 255 with OpenSSH's "Cannot execute command-line and remote command." and loops the reconnect banner (issue #7246): every cmux-controlled invocation that supplies its own remote command (foreground auth `true`, bootstrap installer hop, daemon stdio transport, coordinator batch plumbing, ssh-tmux control commands) inherits the host RemoteCommand instead of overriding it. Covers, all red without the fix: - cmuxTests/SSHConfiguredRemoteCommandHostTests: end-to-end `cmux ssh` startup scripts (persistent-PTY foreground-auth flow and bootstrap install flow) against a fake ssh that mirrors OpenSSH's rule, plus the app-side SSHPTYAttachStartupCommandBuilder foreground auth argv. - cmuxTests/RemoteTmuxHostRemoteCommandOverrideTests: shared ssh-tmux control args, interactive auth, and tmux -CC control-mode argv. - CmuxCoreTests: daemonTransportArguments (cmuxd stdio transport). - CmuxRemoteSessionTests: coordinator batch exec argv (port scan) and override/RequestTTY ordering ahead of caller-configured options. Part 1 of 2 (test-only, expected red); the fix lands separately so CI proves these tests catch the bug. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (17)
✨ 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 |
Greptile SummaryThis PR fixes a crash loop where
Confidence Score: 5/5Safe to merge — the fix is well-scoped, thoroughly tested with fake-ssh harnesses that model real OpenSSH semantics, and does not touch interactive or -N/-O/-G invocations. The change is purely additive argv surgery on cold SSH process-launch paths. Every patched builder is covered by new regression tests using a fake ssh that enforces first-value-wins and none-clears. Interactive and non-command invocations are explicitly excluded and untouched. The one inconsistency (override placement after user options in SSHPTYAttachStartupCommandBuilder) only matters for a user who has RemoteCommand in their cmux SSH settings rather than in ~/.ssh/config, and does not regress the reported bug scenario. Sources/SSHPTYAttachStartupCommandBuilder.swift — override is appended after the user-options loop rather than before it, unlike every other patched site. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant cmux
participant SSH as ssh binary
participant Config as ~/.ssh/config
participant Host as Remote Host
Note over User,Host: Before fix – host has RemoteCommand in ssh_config
User->>cmux: cmux ssh dev-host
cmux->>SSH: ssh [options] dev-host true
SSH->>Config: "read RemoteCommand=sudo su -"
SSH-->>cmux: fatal: Cannot execute command-line and remote command. (exit 255)
cmux-->>User: reconnect loop (1/20)
Note over User,Host: After fix – -o RemoteCommand=none inserted first
User->>cmux: cmux ssh dev-host
cmux->>SSH: "ssh -o RemoteCommand=none [options] dev-host true"
SSH->>Config: "read RemoteCommand=sudo su - (overridden by first -o)"
SSH->>Host: exec true (exit 0 – auth hop succeeds)
cmux->>SSH: "ssh [session -o RemoteCommand=bootstrap] dev-host"
SSH->>Host: exec bootstrap (persistent PTY attached)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant cmux
participant SSH as ssh binary
participant Config as ~/.ssh/config
participant Host as Remote Host
Note over User,Host: Before fix – host has RemoteCommand in ssh_config
User->>cmux: cmux ssh dev-host
cmux->>SSH: ssh [options] dev-host true
SSH->>Config: "read RemoteCommand=sudo su -"
SSH-->>cmux: fatal: Cannot execute command-line and remote command. (exit 255)
cmux-->>User: reconnect loop (1/20)
Note over User,Host: After fix – -o RemoteCommand=none inserted first
User->>cmux: cmux ssh dev-host
cmux->>SSH: "ssh -o RemoteCommand=none [options] dev-host true"
SSH->>Config: "read RemoteCommand=sudo su - (overridden by first -o)"
SSH->>Host: exec true (exit 0 – auth hop succeeds)
cmux->>SSH: "ssh [session -o RemoteCommand=bootstrap] dev-host"
SSH->>Host: exec bootstrap (persistent PTY attached)
Reviews (3): Last reviewed commit: "Make SSHHostConfiguredRemoteCommand an i..." | Re-trigger Greptile |
| XCTAssertFalse(startupResult.timedOut, startupResult.stderr) | ||
| XCTAssertFalse( | ||
| startupResult.stderr.contains("Cannot execute command-line and remote command."), | ||
| "The bootstrap installer hop must override a host-configured RemoteCommand; stderr: \(startupResult.stderr)" | ||
| ) | ||
| XCTAssertEqual(startupResult.status, 0, startupResult.stderr) |
There was a problem hiding this comment.
The bootstrap-install test checks for the "Cannot execute command-line and remote command." string but omits the reconnect-banner assertion (
[cmux] ssh exited with status) that the default-flow test above uses to confirm no retry loop was entered. A fatal exit 255 on the installer hop would still produce the banner, and the missing check would leave that failure mode silent.
| XCTAssertFalse(startupResult.timedOut, startupResult.stderr) | |
| XCTAssertFalse( | |
| startupResult.stderr.contains("Cannot execute command-line and remote command."), | |
| "The bootstrap installer hop must override a host-configured RemoteCommand; stderr: \(startupResult.stderr)" | |
| ) | |
| XCTAssertEqual(startupResult.status, 0, startupResult.stderr) | |
| XCTAssertFalse(startupResult.timedOut, startupResult.stderr) | |
| XCTAssertFalse( | |
| startupResult.stderr.contains("Cannot execute command-line and remote command."), | |
| "The bootstrap installer hop must override a host-configured RemoteCommand; stderr: \(startupResult.stderr)" | |
| ) | |
| XCTAssertFalse( | |
| startupResult.stderr.contains("[cmux] ssh exited with status"), | |
| startupResult.stderr | |
| ) | |
| XCTAssertEqual(startupResult.status, 0, startupResult.stderr) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Good catch — applied. The bootstrap-install test now also asserts the reconnect banner ([cmux] ssh exited with status) is absent, matching the default-flow test; it lands with the fix commit so the red commit stays as-pushed while CI records the failure.
…ions
Fixes `cmux ssh` (and every other cmux-built ssh exec) against host
aliases whose ssh_config sets `RemoteCommand` (typically with
`RequestTTY yes`): OpenSSH refuses a command-line remote command while a
configured RemoteCommand is in effect ("Cannot execute command-line and
remote command.", exit 255), so the foreground auth hop died before the
session ever started and the pane looped reconnect attempts
(issue #7246).
New shared CmuxFoundation constant `SSHHostConfiguredRemoteCommand`
(`-o RemoteCommand=none`, OpenSSH >= 7.6 — macOS has shipped newer
clients since 10.13.2) applied at every builder that appends its own
remote command:
- CLI `cmux ssh`: foreground-auth hop, bootstrap installer hop, and the
`cmux ssh <dest> -- <command>` passthrough branch (inserted right
after `ssh`, so it also wins over caller-supplied options under
OpenSSH's first-value-per-option rule). The interactive session hop
keeps carrying cmux's own `-o RemoteCommand=<bootstrap>`, which
already overrides the host config; bare interactive invocations (VM
attach) are untouched.
- App restore/reattach: SSHPTYAttachStartupCommandBuilder foreground
auth.
- Coordinator batch plumbing (bootstrap probes/install, BootstrapTTY,
port scans, upload cleanup, relay metadata, stale-listener cleanup):
sshCommonArguments(batchMode:) now also pins `-o RequestTTY=no` so a
host `RequestTTY force` cannot CRLF-corrupt parsed pipes.
- CmuxCore daemonTransportArguments (cmuxd stdio transport).
- ssh-tmux stack via RemoteTmuxHost.sshControlArguments (interactive
auth, `tmux -CC` control mode — which keeps its forced `-tt` — and
one-shot discovery/mutation commands).
- File explorer listing, remote git status, and drag-drop upload
cleanup argv builders.
Invocations with no remote command (`-N` forwards, `-O` control ops,
`-G` config dumps, plain interactive shells) are unchanged, and hosts
without a configured RemoteCommand see identical behavior — the
override is inert there.
The CLIRemoteShellStartupPerformanceTests fake ssh now mirrors
OpenSSH's real RemoteCommand semantics (first value wins, `none`
clears) so the installer hop's new override falls through to the
positional command exactly like real ssh.
Fixes #7246
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e conventions The package-conventions lint forbids all-static public namespace types in packages; follow the SSHAgentSocketResolver pattern (public struct with a public initializer) and access the override via an instance at every call site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… ssh fixes) Notable: #7393 moves macOS-15 CI jobs off the dead Blacksmith pool (cures the tests-build-and-lag runner failure), remote workspace package test stabilization, #7359 ssh RemoteCommand/RequestTTY fix, #7255 client config API, #7174 NIGHTLY updater fix, and the sidebar inline-rename feature. Conflicts resolved keeping HEAD's refactored structure: - RemoteTmuxHost: union imports (main's CmuxFoundation + HEAD's CmuxRemoteSession). - TerminalSSHSessionDetector: took main's scpArguments addition (#7359). - FileExplorerStore: HEAD tombstone kept — the CmuxFoundation package copy of SSHFileExplorerProvider already carries main's stateLock/State shape. - ContentView (2 regions): kept HEAD's extracted SidebarWorkspaceRowContent row. main's inline-rename edits target the inline row body the refactor extracted; the feature's six implementation files + tests auto-merged in and the row- architecture port follows as a bounded task (rename-port) before merge. - budget.tsv regenerated; pbxproj union-dedup + normalize (SidebarScrim.swift ref pruned: whole-file-lifted into CmuxSidebarUI earlier, unreferenced on main too). Test-wiring/budget/conventions lints green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem
cmux ssh dev-hostfails and loops the reconnect banner when the host alias is configured for interactive logins:OpenSSH refuses a command-line remote command while the host config sets
RemoteCommand(reproducible outside cmux:ssh dev-host true→ same fatal, exit 255). cmux-controlled invocations pass their own remote commands — the defaultcmux sshflow dies on its very first hop, the foreground-authssh … <dest> true.Fix
New shared
CmuxFoundation.SSHHostConfiguredRemoteCommand(-o RemoteCommand=none, OpenSSH ≥ 7.6 — macOS has shipped newer clients since 10.13.2), applied inside every builder that appends a cmux-supplied remote command:cmux ssh: foreground-auth hop, bootstrap installer hop, and thecmux ssh <dest> -- <command>passthrough branch. Inserted right afterssh, so under OpenSSH's first-value-per-option rule it also wins over caller-supplied options. The session hop keeps carrying cmux's own-o RemoteCommand=<bootstrap>(which already overrides the config), and bare interactive invocations (VM attach) are untouched, so hosts without a configuredRemoteCommandbehave identically.SSHPTYAttachStartupCommandBuilderforeground auth.sshCommonArguments(batchMode:), which now also pins-o RequestTTY=noso a hostRequestTTY forcecannot CRLF-corrupt parsed pipes.WorkspaceRemoteConfiguration.daemonTransportArguments(this is what the persistent remote PTY rides, so PTY attach works on these hosts).RemoteTmuxHost.sshControlArguments(interactive auth,tmux -CCcontrol mode — which keeps its forced-tt— and one-shot discovery/mutation commands).Invocations with no remote command (
-Nforwards,-Ocontrol ops,-Gconfig dumps, plain interactive shells) are unchanged.scp/sftpalready pass-oRemoteCommand=nonethemselves since OpenSSH 7.6.Two-commit structure (regression test policy)
cmuxTests/SSHConfiguredRemoteCommandHostTests: end-to-endcmux sshstartup scripts (both the persistent-PTY foreground-auth flow and the bootstrap-install flow) executed against a fakesshthat mirrors OpenSSH's real rule (positional command + noRemoteCommandoverride → the exact fatal + exit 255; first-o RemoteCommandwins;noneclears), plus the app-side foreground-auth argv.cmuxTests/RemoteTmuxHostRemoteCommandOverrideTests,CmuxCoreTests(daemon transport),CmuxRemoteSessionTests(coordinator batch argv + option ordering).WorkspaceRemoteConfigurationSSHBatchCommandsTestsexact arrays; theCLIRemoteShellStartupPerformanceTestsfake ssh now models first-wins/none-clears), and aGitStatusProvider.fetchStatusSSHargv test.Verification
swift testfor CmuxCore (8/8) and CmuxRemoteSession (42/42) — red on commit 1 for the new suites, green with the fix.ssh-pty-attachbridge handoff) once the override is present.noneclearing, first-value-wins,-Nexemption) verified empirically against OpenSSH 10.2 with a synthetic ssh_config.Fixes #7246
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes cmux ssh failing on hosts that set RemoteCommand/RequestTTY by clearing the host RemoteCommand when cmux sends its own command and disabling TTY for batch ops to prevent reconnect loops and CRLF issues. Applies across default, bootstrap, tmux, daemon, file explorer, and git status flows.
Bug Fixes
CmuxFoundationSSHHostConfiguredRemoteCommandto all cmux-controlled ssh calls that pass a command (foreground auth, bootstrap installer, passthrough, daemon transport, coordinator batch, ssh-tmux, file explorer, remote git status); place it before the destination and ahead of user -o options.Refactors
SSHHostConfiguredRemoteCommandan instantiable struct (per package conventions) and update call sites; no behavior change.Written for commit 759c48c. Summary will update on new commits.