Add passkey, WebAuthn, and FIDO2 support to browser pane#2660
Merged
austinywang merged 5 commits intomainfrom Apr 7, 2026
Merged
Add passkey, WebAuthn, and FIDO2 support to browser pane#2660austinywang merged 5 commits intomainfrom
austinywang merged 5 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This review could not be run because your cubic account has exceeded the monthly review limit. If you need help restoring access, please contact contact@cubic.dev. |
📝 WalkthroughWalkthroughAdds passkey/WebAuthn authorization prompting for trustworthy navigations, separates app vs embedded entitlements and new entitlement files, updates signing workflows/scripts to use and assert the passkey entitlement, adds Bluetooth usage localization, and adds unit tests and a validation script. Changes
Sequence DiagramsequenceDiagram
participant WV as WebView
participant ND as BrowserNavigationDelegate
participant PM as BrowserPasskeyAuthorizationManager
participant ASAuth as ASAuthorizationWebBrowserPublicKeyCredentialManager
participant System as System/User
WV->>ND: webView(decidePolicyFor: navigationAction)
ND->>ND: ensure main-frame & extract URL
ND->>PM: requestAuthorizationIfNeeded(url, source)
PM->>PM: check trust, auth state, pending task, session prompt
alt Should request authorization
PM->>ASAuth: requestAuthorizationForPublicKeyCredentials()
ASAuth->>System: show system prompt
System-->>ASAuth: grant / deny
ASAuth-->>PM: result/state
else Skip request
PM->>PM: no-op / skip
end
PM-->>ND: authorization complete/skip
ND->>WV: allow navigation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Greptile SummaryThis PR adds passkey/WebAuthn/FIDO2 support to the cmux browser pane by wiring Confidence Score: 5/5Safe to merge; all remaining findings are P2 localization gaps. The passkey authorization logic is correct, entitlement scoping is intentional, task cleanup is handled, and unit tests cover the core decision logic. The only finding is a missing translation for NSBluetoothAlwaysUsageDescription — a localization gap, not a correctness issue. Resources/InfoPlist.xcstrings — add the missing locale translations for NSBluetoothAlwaysUsageDescription. Important Files Changed
Sequence DiagramsequenceDiagram
participant Nav as WKNavigationDelegate
participant Mgr as BrowserPasskeyAuthorizationManager
participant Support as BrowserPasskeyAuthorizationSupport
participant AS as ASAuthorizationWebBrowserPublicKeyCredentialManager
Nav->>Mgr: requestAuthorizationIfNeeded(url, source)
Mgr->>AS: authorizationStateForPlatformCredentials
AS-->>Mgr: state
Mgr->>Support: shouldRequestAuthorization(url, state, pending, prompted)
Support->>Support: isPotentiallyTrustworthy(url)
Support-->>Mgr: true / false
alt should request
Mgr->>Mgr: didPromptThisSession = true
Mgr->>AS: requestAuthorizationForPublicKeyCredentials
AS-->>Mgr: authorized / denied / notDetermined
Mgr->>Mgr: authorizationTask = nil
alt result == notDetermined
Mgr->>Mgr: didPromptThisSession = false
end
end
Reviews (1): Last reviewed commit: "Fix popup access to shared passkey autho..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+5
to
+20
| "NSBluetoothAlwaysUsageDescription": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "cmux uses Bluetooth for cross-device passkey sign-in in embedded browser authentication flows." | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "cmux は内蔵ブラウザの認証フローでクロスデバイスのパスキーサインインに Bluetooth を使用します。" | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
This new key only includes en and ja translations, but every other key in this file (NSMicrophoneUsageDescription, New $(PRODUCT_NAME) Workspace Here, etc.) carries 16 locales (zh-Hans, zh-Hant, ko, de, es, fr, it, da, pl, ru, bs, ar, nb, pt-BR, th, tr). Users on those locales will see the English fallback in the macOS Bluetooth permission dialog when a cross-device passkey flow is triggered.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/nightly.yml:
- Around line 396-408: The codesigning order should sign embedded binaries first
and then sign the app without --deep: for each APP_PATH (using APP_ENTITLEMENTS,
EMBEDDED_ENTITLEMENTS, CLI_PATH, HELPER_PATH), run codesign on CLI_PATH and
HELPER_PATH first using --force --options runtime --timestamp --sign
"$APPLE_SIGNING_IDENTITY" --entitlements "$EMBEDDED_ENTITLEMENTS", then sign the
top-level APP_PATH last using --force --options runtime --timestamp --sign
"$APPLE_SIGNING_IDENTITY" --entitlements "$APP_ENTITLEMENTS" (omit the --deep
flag); keep the existing file-existence checks for CLI_PATH and HELPER_PATH and
preserve the same flags otherwise.
In `@Resources/InfoPlist.xcstrings`:
- Around line 5-20: The NSBluetoothAlwaysUsageDescription entry is missing the
full set of localized keys; update the NSBluetoothAlwaysUsageDescription
stringUnit to include the same locale list used by
NSCameraUsageDescription/NSMicrophoneUsageDescription (add the missing locale
keys such as de, fr, es, it, ko, zh-Hans, zh-Hant, etc.), using the existing
English value as the fallback for lower-confidence locales where you don't have
translations, and ensure each added locale follows the same structure
("localizations" -> "<locale>" -> "stringUnit" -> { "state": "translated" or
"untranslated", "value": "<text>" }) to match the catalog format.
In `@scripts/build-sign-upload.sh`:
- Around line 97-103: Sign embedded executables (CLI_PATH and HELPER_PATH) first
using SIGN_HASH and EMBEDDED_ENTITLEMENTS if they exist, then sign the app
bundle (APP_PATH) last using SIGN_HASH and APP_ENTITLEMENTS without the --deep
flag; this ensures you don’t reseal the parent after signing nested binaries
(remove --deep from the codesign call that targets APP_PATH and move that call
to after the conditional codesign calls for CLI_PATH and HELPER_PATH).
In `@scripts/reload.sh`:
- Line 433: The codesign invocation that signs TAG_APP_PATH with
cmux.entitlements should be moved so it runs after the helper binaries (cmuxd
and ghostty) are copied into Contents/Resources/bin (i.e., after the copy steps
that mutate the bundle), and the redirection/silencing (the >/dev/null 2>&1 ||
true) must be removed so any codesign failures surface; update the script to
call /usr/bin/codesign --force --sign - --timestamp=none
--generate-entitlement-der --entitlements "$PWD/cmux.entitlements"
"$TAG_APP_PATH" after the helper-copy logic and allow non-zero exit to fail the
script or log the error.
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 1825-1842: The debug logging in BrowserPanel.swift currently uses
url?.absoluteString (see variables requestURL and calls to dlog in the passkey
authorization skip and begin branches), which can leak OIDC state, auth codes,
or user identifiers; change these dlog calls to avoid logging the full URL by
logging only non-sensitive metadata such as url?.host (or scheme + host), a
redacted placeholder (e.g., "<redacted_url>"), and a byte/length indicator if
useful, and keep the rest of the fields (source, state.cmuxDebugName,
pending/prompted) unchanged; update both occurrences around the passkey
authorization skip and begin blocks (and the similar 1861-1865 section) to use
the redacted/origin value instead of absoluteString.
- Around line 1763-1777: isLoopbackHost(_: ) currently only recognizes
localhost, ::1 and 127.* but misses the remote loopback alias used by
remoteProxyLoopbackAliasURL(for:). Update isLoopbackHost to also treat the
remote loopback alias host (e.g. "cmux-loopback.localtest.me") and its
subdomains as loopback: add checks for normalized ==
"cmux-loopback.localtest.me" and/or
normalized.hasSuffix(".cmux-loopback.localtest.me"). Keep existing logic
(localhost, .localhost, ::1, 127.*) and ensure you reference the same
normalizedHost(_) usage in this function.
🪄 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: dfe8a62d-d7df-4a70-b19c-8559b17a3706
📒 Files selected for processing (11)
.github/workflows/nightly.yml.github/workflows/release.ymlResources/Info.plistResources/InfoPlist.xcstringsSources/Panels/BrowserPanel.swiftSources/Panels/BrowserPopupWindowController.swiftcmux.embedded.entitlementscmux.entitlementscmuxTests/BrowserConfigTests.swiftscripts/build-sign-upload.shscripts/reload.sh
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/reload.sh (1)
439-445:⚠️ Potential issue | 🟠 MajorRe-sign still happens before bundle mutation, and failures are still masked.
At Line 441/444, the app is signed before Lines 481-488 copy
cmuxd/ghosttyinto the bundle, which invalidates the seal. Also,>/dev/null 2>&1 || truehides signing failures, so entitlement regressions are silent.Suggested fix (move signing after helper copy, fail loud)
- APPLE_DEVELOPMENT_IDENTITY="$(find_apple_development_identity || true)" - if [[ -n "${APPLE_DEVELOPMENT_IDENTITY:-}" ]]; then - /usr/bin/codesign --force --sign "$APPLE_DEVELOPMENT_IDENTITY" --timestamp=none --generate-entitlement-der --entitlements "$PWD/cmux.entitlements" "$TAG_APP_PATH" >/dev/null 2>&1 || true - else - echo "No Apple Development signing identity found; local tagged app will launch without passkey entitlements." - /usr/bin/codesign --force --sign - --timestamp=none --generate-entitlement-der "$TAG_APP_PATH" >/dev/null 2>&1 || true - fi + : @@ if [[ -x "$GHOSTTY_HELPER_SRC" ]]; then BIN_DIR="$APP_PATH/Contents/Resources/bin" mkdir -p "$BIN_DIR" cp "$GHOSTTY_HELPER_SRC" "$BIN_DIR/ghostty" chmod +x "$BIN_DIR/ghostty" fi + +if [[ -n "$TAG" && -d "$APP_PATH" ]]; then + APPLE_DEVELOPMENT_IDENTITY="$(find_apple_development_identity || true)" + if [[ -n "${APPLE_DEVELOPMENT_IDENTITY:-}" ]]; then + /usr/bin/codesign --force --sign "$APPLE_DEVELOPMENT_IDENTITY" --timestamp=none --generate-entitlement-der --entitlements "$PWD/cmux.entitlements" "$APP_PATH" + else + echo "No Apple Development signing identity found; local tagged app will launch without passkey entitlements." + /usr/bin/codesign --force --sign - --timestamp=none --generate-entitlement-der "$APP_PATH" + fi +fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/reload.sh` around lines 439 - 445, The current codesign step runs before the helper binaries are copied (so later bundle mutations break the signature) and its failures are silenced; move the entire codesign block that uses APPLE_DEVELOPMENT_IDENTITY and TAG_APP_PATH to execute after the section that copies cmuxd and ghostty into the app bundle, and remove the "|| true" (and avoid redirecting stderr/stdout to /dev/null) so any codesign failure causes the script to fail loudly; ensure both the branch that signs with "$APPLE_DEVELOPMENT_IDENTITY" and the fallback branch that signs with "-" are relocated and behave the same (no suppression) so entitlement/signing regressions surface immediately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/reload.sh`:
- Around line 439-445: The current codesign step runs before the helper binaries
are copied (so later bundle mutations break the signature) and its failures are
silenced; move the entire codesign block that uses APPLE_DEVELOPMENT_IDENTITY
and TAG_APP_PATH to execute after the section that copies cmuxd and ghostty into
the app bundle, and remove the "|| true" (and avoid redirecting stderr/stdout to
/dev/null) so any codesign failure causes the script to fail loudly; ensure both
the branch that signs with "$APPLE_DEVELOPMENT_IDENTITY" and the fallback branch
that signs with "-" are relocated and behave the same (no suppression) so
entitlement/signing regressions surface immediately.
- Skip the system passkey authorization prompt when running under automated UI tests; the dialog blocked the main thread and caused testCmdFFocusesBrowserFindFieldAfterCmdDCmdLNavigation to time out. - Treat the cmux-loopback.localtest.me alias (and subdomains) as a loopback host so WebAuthn works in remote workspaces where local pages are proxied through that hostname. - Redact passkey authorization debug logs to log scheme/host only, avoiding leaks of OIDC state, auth codes, or user identifiers. - Add the broader privacy-prompt locale set for NSBluetoothAlwaysUsageDescription so non-English builds do not fall back to English in the macOS Bluetooth permission dialog. - Update BrowserPasskeyAuthorizationSupport tests to cover the new loopback alias and the URL-redaction helper.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is kicking off a free cloud agent to fix this issue. This run is complimentary, but you can enable autofix for all future PRs in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6458077. Configure here.
| // WKWebView handles WebAuthn itself for browser apps; this support code only | ||
| // determines when cmux should request browser passkey access from macOS. | ||
| enum BrowserPasskeyAuthorizationSupport { | ||
| static let remoteLoopbackAliasHost = "cmux-loopback.localtest.me" |
There was a problem hiding this comment.
Duplicate loopback alias host constant risks future drift
Low Severity
The magic string "cmux-loopback.localtest.me" is now defined as BrowserPasskeyAuthorizationSupport.remoteLoopbackAliasHost in addition to the existing BrowserPanel.remoteLoopbackProxyAliasHost (and a third copy in Workspace). If one copy is updated without the others, the passkey trustworthiness check and the proxy rewriting logic would silently disagree on which hosts are treated as loopback, creating a security-relevant inconsistency.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6458077. Configure here.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/Panels/BrowserPanel.swift (1)
1734-1736: Share the loopback alias constant.The remote-proxy rewrite and passkey trust check both key off this exact value, but the string now lives in two separate constants in the same file. Keeping one source of truth would reduce the chance that those paths drift apart again.
♻️ Proposed refactor
+private let browserRemoteLoopbackAliasHost = "cmux-loopback.localtest.me" + enum BrowserPasskeyAuthorizationSupport { - static let remoteLoopbackAliasHost = "cmux-loopback.localtest.me" + static let remoteLoopbackAliasHost = browserRemoteLoopbackAliasHost @@ final class BrowserPanel: Panel, ObservableObject { - private static let remoteLoopbackProxyAliasHost = "cmux-loopback.localtest.me" + private static let remoteLoopbackProxyAliasHost = browserRemoteLoopbackAliasHost🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/BrowserPanel.swift` around lines 1734 - 1736, The duplicate loopback alias string is defined twice; consolidate it to a single shared constant by exposing BrowserPasskeyAuthorizationSupport.remoteLoopbackAliasHost as the single source of truth and replace any other occurrences or duplicate constants (e.g., the remote-proxy rewrite and passkey trust check constants) to reference BrowserPasskeyAuthorizationSupport.remoteLoopbackAliasHost so both subsystems use the exact same value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 1734-1736: The duplicate loopback alias string is defined twice;
consolidate it to a single shared constant by exposing
BrowserPasskeyAuthorizationSupport.remoteLoopbackAliasHost as the single source
of truth and replace any other occurrences or duplicate constants (e.g., the
remote-proxy rewrite and passkey trust check constants) to reference
BrowserPasskeyAuthorizationSupport.remoteLoopbackAliasHost so both subsystems
use the exact same value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d6133a5-5e8e-4d6d-8bc4-c971f223988d
📒 Files selected for processing (3)
Resources/InfoPlist.xcstringsSources/Panels/BrowserPanel.swiftcmuxTests/BrowserConfigTests.swift
✅ Files skipped from review due to trivial changes (2)
- Resources/InfoPlist.xcstrings
- cmuxTests/BrowserConfigTests.swift
austinywang
added a commit
that referenced
this pull request
Apr 7, 2026
…)" This reverts commit f550206.
3 tasks
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements full passkey / WebAuthn / FIDO2 support for the cmux browser pane.
Refs umbrella issue: #2657
Also covers:
What changed:
com.apple.developer.web-browser.public-key-credentialentitlement to the app signing path, while keeping embedded CLI/helper binaries on a narrower entitlement set.This intentionally keeps WebKit handling native
navigator.credentialsWebAuthn flows instead of reimplementing them in JavaScript/native glue. That aligns cmux with Apple's browser-app passkey model for platform passkeys, cross-device handoff, hardware security keys, and enterprise SSO step-up flows.Local tests were not run per repo policy.
Note
Medium Risk
Introduces new passkey authorization prompting and adds the
com.apple.developer.web-browser.public-key-credentialentitlement, plus updates CI/release signing paths; mistakes could break login flows or macOS code-signing/notarization behavior.Overview
Enables passkey/WebAuthn support in the embedded browser by requesting
ASAuthorizationWebBrowserPublicKeyCredentialManagerauthorization on trustworthy main-frame navigations (HTTPS or loopback HTTP) in both the main browser panel and auth popups, with session-level gating to avoid repeated prompts.Updates macOS packaging/signing to include the new
com.apple.developer.web-browser.public-key-credentialapp entitlement, splits entitlements so embedded CLI/helper binaries usecmux.embedded.entitlements, and adds a CI/script check (scripts/assert-passkey-entitlement.sh) to fail builds if the signed app is missing the entitlement.Adds the required
NSBluetoothAlwaysUsageDescription(localized) for cross-device passkey flows, and adds unit tests covering trustworthy-origin detection and authorization-request conditions.Reviewed by Cursor Bugbot for commit a767c32. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds passkey/WebAuthn/FIDO2 to the in‑app browser so users can sign in with platform passkeys, cross‑device handoff, and hardware security keys in tabs and auth popups. Prompts for passkey access only on trustworthy main‑frame navigations. Addresses #2657 (also #124, #1056, #1278).
New Features
WKWebView(WebKit handlesnavigator.credentials).cmux-loopback.localtest.me; single prompt per session.com.apple.developer.web-browser.public-key-credentialto app entitlements; newcmux.embedded.entitlementsfor CLI/helpers.NSBluetoothAlwaysUsageDescriptionwith expanded localizations.Bug Fixes
scripts/assert-passkey-entitlement.shinrelease/nightlyandscripts/build-sign-upload.sh.scripts/reload.shto prefer an Apple Development identity for tagged builds, falling back to ad‑hoc with a clear note if not.Written for commit 6458077. Summary will update on new commits.
Summary by CodeRabbit
New Features
Privacy / Permissions
Localization
Tests
Chores
Follow-up: CI fix and review responses (commit 6458077)
testCmdFFocusesBrowserFindFieldAfterCmdDCmdLNavigationwas timing out because the new passkey authorization request, triggered on the first main-frame navigation, opened a system permission dialog that blocked the main thread. Skip the prompt under automated UI tests viaSessionRestorePolicy.isRunningUnderAutomatedTests().cmux-loopback.localtest.me(and subdomains) as a loopback host so WebAuthn works in remote workspaces where local pages are proxied through that alias.redactedURLDescription(for:)helper, avoiding leaks of OIDC state, auth codes, or user identifiers.NSBluetoothAlwaysUsageDescription(zh-Hans, zh-Hant, ko, de, es, fr, it, da, pl, ru, bs, ar, nb, pt-BR, th, tr) so non-English builds do not fall back to English in the macOS Bluetooth permission dialog.release.yml,nightly.yml, andscripts/build-sign-upload.shis already correct (CLI → helper → app, no--deepon the outer sign). The bot reports were inaccurate.