Skip to content

cmux ssh: require a per-tunnel credential on the browser proxy - #15132

Closed
austinywang wants to merge 6 commits into
mainfrom
ssh-browser-proxy-auth
Closed

austinywang wants to merge 6 commits into
mainfrom
ssh-browser-proxy-auth

Conversation

@austinywang

@austinywang austinywang commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The ssh workspace browser reaches remote services through a loopback SOCKS5 and HTTP CONNECT listener. Each accepted handshake opens a daemon proxy.open stream on the remote host. Until now the listener accepted any local client.

Each tunnel start now mints a random BrowserProxyCredential: username cmux and a 64-hex-digit password. The listener checks it on every handshake, with a constant-time compare, before any daemon stream opens:

  • SOCKS5: only the username/password method (RFC 1929) is offered. A greeting without it gets 05 FF, and a wrong credential gets 01 01. Either way the connection closes.
  • HTTP CONNECT: the request must carry Proxy-Authorization: Basic …. Without it, the listener answers 407 with Proxy-Authenticate: Basic realm="cmux" and closes.

The browser applies the credential to both ProxyConfigurations on its WKWebsiteDataStore. The favicon URLSession switches from connectionProxyDictionary to the same proxyConfigurations, so it carries the credential too.

A refused credential never turns into a password prompt. Without a delegate, a proxy 407 reaches the system, which asks the user for a proxy password and would send the answer to whatever holds the loopback port. That can happen after a tunnel restart rotates the credential, or when another process takes a freed port. The new ManagedProxySessionDelegate cancels proxy challenges instead. It is used on the favicon session, and the same check runs in the web view and popup navigation delegates of remote and cloud panes. Local panes keep the default handling for their system proxy.

The credential stays out of logs, argv and workspace.remote.status. BrowserProxyCredential and BrowserProxyEndpoint print as redacted descriptions. The status payload still reports host, port and a socks5:// URL, without the credential.

Testing

Regression test, run the same way at each commit:

swift test --package-path Packages/macOS/CmuxRemoteWorkspace --filter RemoteDaemonProxySessionAuthenticationTests
  • Red at 409a6bc (test only): exit 1, "Test run with 8 tests in 1 suite failed after 30.257 seconds with 23 issues". Handshakes without the credential got 05 00 or 200 Connection Established, and daemon streams opened.
  • Green at 645d033 (fix): exit 0, "Test run with 8 tests in 1 suite passed after 5.406 seconds". The test files are the same in both commits.

Proxy prompt follow-up, run the same way at each commit:

swift test --package-path Packages/macOS/CmuxCore --filter ManagedProxySessionDelegateTests
  • Red at 0fd0b21 (test only): "Test run with 2 tests in 1 suite failed after 0.007 seconds with 2 issues". Both proxy challenge cases got .performDefaultHandling instead of .cancelAuthenticationChallenge.
  • Green at f39305d (fix): "Test run with 2 tests in 1 suite passed after 0.005 seconds".

On f39305d, RemoteDaemonProxySessionAuthenticationTests passed 8 tests in 0.276 seconds. Its URLSession now goes through the delegate, so the no-credential CONNECT case is refused without opening the system proxy-password dialog. verify-local.py --affected origin/main --swift-changed origin/main passed 15/15 checks.

The suite covers raw SOCKS5 and CONNECT handshakes with a missing, wrong or correct credential. It also drives URLSession through Network.framework ProxyConfiguration, the stack WKWebView uses, with and without the credential.

Full package runs on 645d033 all passed:

Package Tests Suites
CmuxCore 86 14
CmuxRemoteWorkspace 164 26
CmuxRemoteSession 199 34

CmuxCore adds BrowserProxyCredentialTests, which checks:

  • The password has the expected length and differs between mints.
  • matches needs the exact bytes.
  • No description or reflection contains the password.

python3 scripts/verify-local.py with automatic selection passed 5/5 checks: swift-syntax, wire-app-sources, test-wiring, package-groups and feature-flags.

Not run:

  • App target: the app was not built and app tests were not run locally. That leaves these to the CI app build and app test lanes: Sources/Panels/BrowserPanel.swift, BrowserNavigationDelegate.swift, BrowserPopupWindowController.swift and the three GhosttyConfigTests call sites. No app test drives a web view proxy challenge.
  • Live ssh workspace: the browser was not checked against a real one.
  • Docker scripts in tests_v2: test_ssh_remote_docker_forwarding.py, test_ssh_remote_docker_reconnect.py and test_ssh_remote_proxy_bind_conflict.py send unauthenticated SOCKS5 and CONNECT to the endpoint from workspace.remote.status. The listener now refuses those requests, and the status payload deliberately omits the credential. These scripts are not in CI and were not updated here.

No user-facing strings changed, so there is no localization impact.

Changelog

Fixed: The ssh workspace browser proxy now requires a per-tunnel credential, so other local accounts can't route connections through the remote host

Checklist

  • Behavior changes have added or updated tests, or Testing says why not
  • User-facing docs updated if needed (docs/remote-daemon-spec.md, daemon/remote/README.md)
  • Reviewed with a subagent before merge (cmux-review), and all bot and human review comments resolved

🤖 Generated with Claude Code

austinywang and others added 2 commits September 27, 2026 20:38
The loopback SOCKS5 and HTTP CONNECT listener that serves the ssh
workspace browser accepts any local client. Add a per-tunnel
BrowserProxyCredential and an optional session seam, then test that a
handshake without it is refused before a daemon stream opens and that the
credentialed handshake still relays bytes. The Network.framework cases
drive URLSession through ProxyConfiguration, the stack WKWebView uses.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The loopback SOCKS5 and HTTP CONNECT listener for the ssh workspace
browser now requires the credential its tunnel minted. SOCKS5 only
accepts username/password authentication (RFC 1929), CONNECT needs a
matching Proxy-Authorization: Basic header, and both compare in constant
time before any daemon stream opens. A refused client gets 05 FF, 01 01
or 407 and the connection closes.

The broker mints a fresh credential per tunnel start and hands it to the
tunnel and the BrowserProxyEndpoint. The browser applies it to its
WKWebsiteDataStore SOCKS5 and CONNECT proxy configurations, and the
favicon session moves from the legacy proxy dictionary, which cannot
carry a credential, to a credentialed ProxyConfiguration. Descriptions
of the credential and endpoint are redacted, and workspace.remote.status
keeps reporting the endpoint without it.

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

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 2 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: 21e9fec5-c95e-48f4-8065-fb8608cdaf39

📥 Commits

Reviewing files that changed from the base of the PR and between ce5cb45 and 89cef27.

📒 Files selected for processing (28)
  • Packages/macOS/CmuxCore/Sources/CmuxCore/Remote/BrowserProxyCredential.swift
  • Packages/macOS/CmuxCore/Sources/CmuxCore/Remote/BrowserProxyEndpoint.swift
  • Packages/macOS/CmuxCore/Sources/CmuxCore/Remote/ManagedProxySessionDelegate.swift
  • Packages/macOS/CmuxCore/Tests/CmuxCoreTests/BrowserProxyCredentialTests.swift
  • Packages/macOS/CmuxCore/Tests/CmuxCoreTests/ManagedProxySessionDelegateTests.swift
  • Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/IntentionalCleanupTestTunnelProvider.swift
  • Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/NeverReadyProxyTunnelProvider.swift
  • Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemotePTYIntentionalCleanupTests.swift
  • Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemotePortScanGatingTests.swift
  • Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteReconnectPolicyTests.swift
  • Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionReadinessParkingTests.swift
  • Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Broker/RemoteDaemonProxyTunnelProvider.swift
  • Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Broker/RemoteProxyBroker.swift
  • Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Broker/RemoteProxyTunnelProviding.swift
  • Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Tunnel/RemoteDaemonProxySession.swift
  • Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Tunnel/RemoteDaemonProxyTunnel.swift
  • Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/BlockingCloseTunnelProvider.swift
  • Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/BridgeTestClient.swift
  • Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/ProxyStreamTestRPCClient.swift
  • Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonProxySessionAuthenticationTests.swift
  • Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonProxyTunnelPTYBridgeTests.swift
  • Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteProxyBrokerTests.swift
  • Sources/Panels/BrowserNavigationDelegate.swift
  • Sources/Panels/BrowserPanel.swift
  • Sources/Panels/BrowserPopupWindowController.swift
  • cmuxTests/GhosttyConfigTests.swift
  • daemon/remote/README.md
  • docs/remote-daemon-spec.md

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

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

CI failure attribution

CI passes on 89cef27732 (run 36484987966 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.

austinywang and others added 4 commits September 27, 2026 21:46
A proxy 407 on a URLSession without a delegate reaches the system,
which asks the user for a proxy password. Route the proxy test's
session through a delegate and test that it cancels proxy challenges
while leaving server challenges to the default handling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The remote proxy credential rotates on each tunnel start, and another
process can take a freed loopback port. A stale or wrong credential
then reaches the system, which asks the user for a proxy password and
would send what they type to that port.

Cancel proxy challenges on the favicon session and in the web view and
popup delegates of remote and cloud panes, whose only proxies are cmux
proxies that already carry their credential. Local panes keep the
default handling for their system proxy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Catch-up merge by scripts/ci/catch_up_pr.py (RFC #14631).
Merged by scripts/merge-main.sh: origin/main at ce5cb45.

Catch-up-previous-head: f39305d
Catch-up-base: ce5cb45
ProxyConfiguration is a struct over a reference, and applyCredential is
non-mutating, so the var bindings added with the per-tunnel credential
drew two "never mutated" warnings in BrowserPanel.swift. The macOS
compile admission gate refuses new Swift warnings over the budget, which
declined the PR. The test helper had the same binding.

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

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Dogfood tours of 89cef277

browser-notifications-tour at 89cef277: passed (run)

browser-notifications-tour at 89cef277

Key frames of browser-notifications-tour at 89cef27 04-browser-split 12-omnibar-suggestions 20-notifications-menu 27-end

Tours are picked by the paths globs in dogfood/scenarios/*.json; a Dogfood-tours: a, b line in the description picks them instead (none turns this off). Look at every frame before merging: a green tour only means no step failed.

@austinywang

Copy link
Copy Markdown
Contributor Author

Closing as superseded by merged PR #15116, which consolidated this SSH/security/local-state hardening into main.

@austinywang austinywang closed this Oct 8, 2026
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.

1 participant