Repository navigation
Fix tagged native auth callback handoff - #5742
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR implements a secure native-to-web sign-in handoff mechanism with per-attempt callback state validation, refactored web routing, and enhanced debugging. The core change threads callback state through ChangesNative Sign-In Handoff Flow
🎯 3 (Moderate) | ⏱️ ~25 minutes
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (18 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5782c5c. Configure here.
| } | ||
|
|
||
| function requestIsSecure(): boolean { | ||
| return process.env.NODE_ENV === "production"; |
There was a problem hiding this comment.
Handoff cookie secure mismatch
Low Severity
The native handoff cookie is set with secure tied to the request scheme in native-sign-in, but cleared in after-sign-in using NODE_ENV === "production". On HTTPS dev hosts, the clear cookie may not match the original Secure flag, so the nonce cookie can outlive auto-open.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5782c5c. Configure here.
Greptile SummaryThis PR hardens native auth callback routing for tagged dev builds by registering per-tag
Confidence Score: 4/5The core auth callback routing is well-structured — state matching, nonce gating, scheme allowlisting, and double-resume prevention are all implemented correctly. The two findings are scoped to edge cases: a cookie-attribute mismatch visible only in HTTPS non-production deployments, and a side-channel state coupling that can produce a stale fallback URL if Settings login fires between two rapid CLI socket calls. The new auth flow is thoughtfully tested and the security-critical pieces (nonce verification, state matching, allowed-scheme gating) are correct. The
Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI
participant App as Native App (MainActor)
participant NativeSignIn as /handler/native-sign-in
participant Stack as Stack /handler/sign-in
participant AfterSignIn as /handler/after-sign-in
CLI->>App: auth.sign_in_url (socket)
App->>App: manualSignInURL generates state, sets pendingManualCallbackState
App-->>CLI: url with cmux_auth_state embedded
CLI->>CLI: print fallback URL
CLI->>App: auth.begin_sign_in (socket)
App->>App: startAttempt() consumes pendingManualCallbackState as activeCallbackState
App->>App: scheduleAttemptTimeout 10 min
App->>NativeSignIn: ASWebAuthenticationSession GET
NativeSignIn->>NativeSignIn: "canSetAutoHandoff=true, nonce=UUID"
NativeSignIn->>NativeSignIn: append cmux_auth_handoff nonce to after-sign-in URL
NativeSignIn-->>App: 302 to Stack sign-in, Set-Cookie handoff nonce httpOnly
App->>Stack: User authenticates
alt Popup path
Stack-->>App: 302 to after-sign-in with native_app_return_to and nonce
App->>AfterSignIn: GET with handoff cookie
AfterSignIn->>AfterSignIn: verifiedAutoOpen cookie matches nonce
AfterSignIn-->>App: meta-refresh to cmux-dev-tag callback URL with tokens
App->>App: handleCallbackURL state matches activeCallbackState, completeCallback
else Fallback path
CLI->>NativeSignIn: user opens fallback URL manually
NativeSignIn-->>AfterSignIn: same nonce flow
AfterSignIn-->>App: URL scheme callback, state matches activeCallbackState
end
App->>App: cancelAttemptTimeout, finishAttempt
App-->>CLI: auth.begin_sign_in signed in
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function requestIsSecure(): boolean { | ||
| return process.env.NODE_ENV === "production"; | ||
| } |
There was a problem hiding this comment.
requestIsSecure() helper uses NODE_ENV === "production" to set the secure flag when clearing the handoff cookie, but the sibling native-sign-in route sets the same cookie using request.nextUrl.protocol === "https:". In an HTTPS staging or preview environment where NODE_ENV is not "production", the cookie would have been written with secure: true (by native-sign-in) but the deletion attempt here would use secure: false, causing browsers to silently ignore the Set-Cookie header and leaving a stale nonce in the browser until it naturally expires (10 min).
| function requestIsSecure(): boolean { | |
| return process.env.NODE_ENV === "production"; | |
| } | |
| function requestIsSecure(request: NextRequest): boolean { | |
| return request.nextUrl.protocol === "https:"; | |
| } |
| /// The hosted sign-in URL for manual fallback when the browser handoff does | ||
| /// not return to the native app. | ||
| public var manualSignInURL: URL { | ||
| let state = makeCallbackState() | ||
| pendingManualCallbackState = state | ||
| return makeSignInURL(state) | ||
| } |
There was a problem hiding this comment.
manualSignInURL writes pendingManualCallbackState as a side effect, which is then consumed by the very next startAttempt() call. The invariant holds for the CLI's sequential auth.sign_in_url → auth.begin_sign_in path, but breaks if beginSignIn() fires between those two socket calls (e.g., the user clicks the Settings login button). In that case, startAttempt() consumes state-N for the Settings popup, the subsequent auth.begin_sign_in call generates a fresh state-N+1 for the CLI popup, and the already-printed fallback URL (holding state-N) will be rejected as a state mismatch when the user tries to use it. Consider making the state transfer explicit by having the caller own the UUID and pass it through both the URL generator and the sign-in call, rather than leaving it in shared mutable side-channel state.
Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@cmuxTests/AuthEnvironmentTests.swift`:
- Around line 40-45: The test signInURLEntersNativeWrapper currently only checks
that the query contains "after_auth_return_to="; update it to parse the URL's
query items (use URLComponents(url: _, resolvingAgainstBaseURL:
false).queryItems) find the "after_auth_return_to" value, URL-decode it and
parse it as a nested URLComponents, and then assert that the nested queryItems
include "native_app_return_to" and "cmux_auth_state" with the expected value
("state-1"); reference AuthEnvironment.signInURL(callbackState:) and the test
signInURLEntersNativeWrapper to locate the code to change.
In
`@Packages/CmuxAuthRuntime/Sources/CmuxAuthRuntime/BrowserSignIn/HostBrowserSignInFlow.swift`:
- Around line 81-84: manualSignInURL currently creates a new callback state
every read and overwrites pendingManualCallbackState, invalidating earlier URLs;
change the getter so it first checks if pendingManualCallbackState already
exists and, if so, returns makeSignInURL(existingState), otherwise generate a
new state via makeCallbackState(), assign it to pendingManualCallbackState, and
then return makeSignInURL(state), keeping pendingManualCallbackState stable
until it is consumed (e.g., by the existing consumption code that reads/clears
pendingManualCallbackState).
In `@scripts/reload.sh`:
- Around line 322-327: The helper set_plist_url_scheme currently only tries Set
:CFBundleURLTypes:1:CFBundleURLSchemes:0 and swallows failures; change it to try
a robust sequence of updates (attempt index 1, then index 0, then scan
CFBundleURLTypes entries to locate the first CFBundleURLSchemes array and set
its 0th element) and if all attempts fail emit a clear error to stderr and exit
non‑zero instead of swallowing the PlistBuddy error; update the function name
set_plist_url_scheme to return failure on error and ensure callers handle that
non‑zero exit so the reload fails fast when the auth URL scheme rewrite does not
land.
In `@web/app/handler/after-sign-in/route.ts`:
- Around line 26-29: The host-parsing in isLocalRequest incorrectly splits on
":" so bracketed IPv6 hosts like "[::1]:3000" become "[" and fail the local
check; update isLocalRequest to robustly extract the hostname rather than using
simple split — for example, detect and strip an IPv6 bracketed address (remove
surrounding "[" and "]" and any trailing ":port"), or use a URL/hostname parser
to get request host, then compare the resulting hostname (e.g., "::1",
"127.0.0.1", "localhost") against the local list; ensure the change targets the
isLocalRequest function and preserves the existing comparisons.
- Around line 80-101: The decodeAccessCookie and decodeRefreshCookie functions
call decodeURIComponent on untrusted cookie strings which can throw URIError for
malformed percent-escapes; wrap the decodeURIComponent call in a try/catch (or
perform a safeDecode helper used by both functions) so that if
decodeURIComponent throws you fall back to the raw value instead of propagating
the error, preserving existing behavior (return accessToken/refreshToken or
undefined) and preventing a 500 from malformed cookies.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: aa671515-1a32-46ee-82c8-0a095321b5ab
📒 Files selected for processing (39)
CLI/cmux.swiftPackages/CmuxAuthRuntime/Sources/CmuxAuthRuntime/BrowserSignIn/ASWebBrowserAuthSessionFactory.swiftPackages/CmuxAuthRuntime/Sources/CmuxAuthRuntime/BrowserSignIn/HostBrowserSignInFlow.swiftPackages/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Diagnostics/AuthDebugLog.swiftPackages/CmuxAuthRuntime/Tests/CmuxAuthRuntimeTests/AuthDebugLogTests.swiftPackages/CmuxAuthRuntime/Tests/CmuxAuthRuntimeTests/HostBrowserSignInFlowTests.swiftPackages/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swiftPackages/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandExecutionPolicyTests.swiftSources/AppDelegate.swiftSources/Auth/AuthEnvironment.swiftSources/Auth/MacAuthComposition.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AuthEnvironmentTests.swiftscripts/reload.shweb/app/handler/after-sign-in/OpenNativeClient.tsxweb/app/handler/after-sign-in/page.tsxweb/app/handler/after-sign-in/route.tsweb/app/handler/native-sign-in/route.tsweb/messages/ar.jsonweb/messages/bs.jsonweb/messages/da.jsonweb/messages/de.jsonweb/messages/en.jsonweb/messages/es.jsonweb/messages/fr.jsonweb/messages/it.jsonweb/messages/ja.jsonweb/messages/km.jsonweb/messages/ko.jsonweb/messages/no.jsonweb/messages/pl.jsonweb/messages/pt-BR.jsonweb/messages/ru.jsonweb/messages/th.jsonweb/messages/tr.jsonweb/messages/uk.jsonweb/messages/zh-CN.jsonweb/messages/zh-TW.json
💤 Files with no reviewable changes (2)
- web/app/handler/after-sign-in/OpenNativeClient.tsx
- web/app/handler/after-sign-in/page.tsx
| @Test("sign-in URL enters native wrapper") | ||
| func signInURLEntersNativeWrapper() { | ||
| let url = AuthEnvironment.signInURL(callbackState: "state-1") | ||
| #expect(url.path == "/handler/native-sign-in") | ||
| #expect(url.query?.contains("after_auth_return_to=") == true) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Strengthen nested callback assertions in the wrapper URL test.
The current check can pass even if the nested after_auth_return_to payload is malformed. Parse query items and assert the nested URL contains the expected native_app_return_to (and cmux_auth_state) contract.
Suggested test hardening
`@Test`("sign-in URL enters native wrapper")
func signInURLEntersNativeWrapper() {
let url = AuthEnvironment.signInURL(callbackState: "state-1")
`#expect`(url.path == "/handler/native-sign-in")
- `#expect`(url.query?.contains("after_auth_return_to=") == true)
+ let outer = URLComponents(url: url, resolvingAgainstBaseURL: false)
+ let afterAuthReturnTo = outer?.queryItems?.first(where: { $0.name == "after_auth_return_to" })?.value
+ `#expect`(afterAuthReturnTo != nil)
+
+ let afterSignInURL = URL(string: afterAuthReturnTo!)
+ let afterSignIn = URLComponents(url: afterSignInURL!, resolvingAgainstBaseURL: false)
+ let nativeAppReturnTo = afterSignIn?.queryItems?.first(where: { $0.name == "native_app_return_to" })?.value
+ `#expect`(nativeAppReturnTo != nil)
+ `#expect`(nativeAppReturnTo?.contains("cmux_auth_state=state-1") == true)
}🤖 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 `@cmuxTests/AuthEnvironmentTests.swift` around lines 40 - 45, The test
signInURLEntersNativeWrapper currently only checks that the query contains
"after_auth_return_to="; update it to parse the URL's query items (use
URLComponents(url: _, resolvingAgainstBaseURL: false).queryItems) find the
"after_auth_return_to" value, URL-decode it and parse it as a nested
URLComponents, and then assert that the nested queryItems include
"native_app_return_to" and "cmux_auth_state" with the expected value
("state-1"); reference AuthEnvironment.signInURL(callbackState:) and the test
signInURLEntersNativeWrapper to locate the code to change.
| public var manualSignInURL: URL { | ||
| let state = makeCallbackState() | ||
| pendingManualCallbackState = state | ||
| return makeSignInURL(state) |
There was a problem hiding this comment.
Make manualSignInURL stable for the current pending attempt.
Line 82 generates a fresh callback state on every read and overwrites pendingManualCallbackState. Any caller that touches this property twice before Line 195 consumes it will invalidate the first URL, so the earlier link's callback is rejected as stale.
Suggested fix
public var manualSignInURL: URL {
- let state = makeCallbackState()
- pendingManualCallbackState = state
- return makeSignInURL(state)
+ let state = pendingManualCallbackState ?? makeCallbackState()
+ pendingManualCallbackState = state
+ return makeSignInURL(state)
}🤖 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
`@Packages/CmuxAuthRuntime/Sources/CmuxAuthRuntime/BrowserSignIn/HostBrowserSignInFlow.swift`
around lines 81 - 84, manualSignInURL currently creates a new callback state
every read and overwrites pendingManualCallbackState, invalidating earlier URLs;
change the getter so it first checks if pendingManualCallbackState already
exists and, if so, returns makeSignInURL(existingState), otherwise generate a
new state via makeCallbackState(), assign it to pendingManualCallbackState, and
then return makeSignInURL(state), keeping pendingManualCallbackState stable
until it is consumed (e.g., by the existing consumption code that reads/clears
pendingManualCallbackState).
| set_plist_url_scheme() { | ||
| local plist="$1" | ||
| local scheme="$2" | ||
| /usr/libexec/PlistBuddy -c "Set :CFBundleURLTypes:1:CFBundleURLSchemes:0 \"${scheme}\"" "$plist" 2>/dev/null \ | ||
| || true | ||
| } |
There was a problem hiding this comment.
Fail fast if the auth URL scheme rewrite does not land.
This helper only tries CFBundleURLTypes:1:CFBundleURLSchemes:0 and then swallows any PlistBuddy failure. If that slot ever moves or is missing, the staged app stays registered for the old callback scheme while Lines 913-923 and web/app/handler/after-sign-in/route.ts now require cmux-dev-${TAG_SLUG}. The reload still reports success, but tagged native auth callbacks no longer reopen the app.
🩹 Suggested fix
set_plist_url_scheme() {
local plist="$1"
local scheme="$2"
- /usr/libexec/PlistBuddy -c "Set :CFBundleURLTypes:1:CFBundleURLSchemes:0 \"${scheme}\"" "$plist" 2>/dev/null \
- || true
+ if ! /usr/libexec/PlistBuddy -c "Set :CFBundleURLTypes:1:CFBundleURLSchemes:0 \"${scheme}\"" "$plist" 2>/dev/null; then
+ echo "error: failed to update auth callback URL scheme in $plist" >&2
+ return 1
+ fi
}📝 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.
| set_plist_url_scheme() { | |
| local plist="$1" | |
| local scheme="$2" | |
| /usr/libexec/PlistBuddy -c "Set :CFBundleURLTypes:1:CFBundleURLSchemes:0 \"${scheme}\"" "$plist" 2>/dev/null \ | |
| || true | |
| } | |
| set_plist_url_scheme() { | |
| local plist="$1" | |
| local scheme="$2" | |
| if ! /usr/libexec/PlistBuddy -c "Set :CFBundleURLTypes:1:CFBundleURLSchemes:0 \"${scheme}\"" "$plist" 2>/dev/null; then | |
| echo "error: failed to update auth callback URL scheme in $plist" >&2 | |
| return 1 | |
| fi | |
| } |
🤖 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 `@scripts/reload.sh` around lines 322 - 327, The helper set_plist_url_scheme
currently only tries Set :CFBundleURLTypes:1:CFBundleURLSchemes:0 and swallows
failures; change it to try a robust sequence of updates (attempt index 1, then
index 0, then scan CFBundleURLTypes entries to locate the first
CFBundleURLSchemes array and set its 0th element) and if all attempts fail emit
a clear error to stderr and exit non‑zero instead of swallowing the PlistBuddy
error; update the function name set_plist_url_scheme to return failure on error
and ensure callers handle that non‑zero exit so the reload fails fast when the
auth URL scheme rewrite does not land.
| function isLocalRequest(request: NextRequest): boolean { | ||
| const hostHeader = request.headers.get("host"); | ||
| const host = (hostHeader?.split(":")[0] ?? request.nextUrl.hostname).toLowerCase(); | ||
| return host === "localhost" || host === "127.0.0.1" || host === "::1"; |
There was a problem hiding this comment.
[::1] never passes the local-request check.
Line 28 splits the Host header on :, so a loopback IPv6 host like [::1]:3000 becomes "[". That makes isLocalRequest() false and rejects cmux-dev / tagged callback schemes on a local dev server reached via IPv6.
🤖 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 `@web/app/handler/after-sign-in/route.ts` around lines 26 - 29, The
host-parsing in isLocalRequest incorrectly splits on ":" so bracketed IPv6 hosts
like "[::1]:3000" become "[" and fail the local check; update isLocalRequest to
robustly extract the hostname rather than using simple split — for example,
detect and strip an IPv6 bracketed address (remove surrounding "[" and "]" and
any trailing ":port"), or use a URL/hostname parser to get request host, then
compare the resulting hostname (e.g., "::1", "127.0.0.1", "localhost") against
the local list; ensure the change targets the isLocalRequest function and
preserves the existing comparisons.
| function decodeAccessCookie(value: string | undefined): { refreshToken?: string; accessToken?: string } { | ||
| if (!value) return {}; | ||
| const decoded = value.includes("%") ? decodeURIComponent(value) : value; | ||
| if (!decoded.startsWith("[")) return { accessToken: decoded }; | ||
| try { | ||
| const arr = JSON.parse(decoded) as unknown[]; | ||
| if (Array.isArray(arr) && arr.length >= 2) { | ||
| return { refreshToken: arr[0] as string, accessToken: arr[1] as string }; | ||
| } | ||
| } catch {} | ||
| return {}; | ||
| } | ||
|
|
||
| function decodeRefreshCookie(value: string | undefined): string | undefined { | ||
| if (!value) return undefined; | ||
| const decoded = value.includes("%") ? decodeURIComponent(value) : value; | ||
| if (!decoded.startsWith("{")) return decoded; | ||
| try { | ||
| const obj = JSON.parse(decoded) as Record<string, unknown>; | ||
| if (typeof obj.refresh_token === "string") return obj.refresh_token; | ||
| } catch {} | ||
| return undefined; |
There was a problem hiding this comment.
Guard malformed cookie decoding.
Lines 82 and 95 call decodeURIComponent on raw cookie values. Those cookies are request-controlled input; a stray % or truncated escape throws URIError and turns /handler/after-sign-in into a 500 during sign-in completion.
Suggested fix
+function safeDecodeCookie(value: string): string | null {
+ try {
+ return value.includes("%") ? decodeURIComponent(value) : value;
+ } catch {
+ return null;
+ }
+}
+
function decodeAccessCookie(value: string | undefined): { refreshToken?: string; accessToken?: string } {
if (!value) return {};
- const decoded = value.includes("%") ? decodeURIComponent(value) : value;
+ const decoded = safeDecodeCookie(value);
+ if (decoded === null) return {};
if (!decoded.startsWith("[")) return { accessToken: decoded };
try {
const arr = JSON.parse(decoded) as unknown[];
@@
function decodeRefreshCookie(value: string | undefined): string | undefined {
if (!value) return undefined;
- const decoded = value.includes("%") ? decodeURIComponent(value) : value;
+ const decoded = safeDecodeCookie(value);
+ if (decoded === null) return undefined;
if (!decoded.startsWith("{")) return decoded;
try {🤖 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 `@web/app/handler/after-sign-in/route.ts` around lines 80 - 101, The
decodeAccessCookie and decodeRefreshCookie functions call decodeURIComponent on
untrusted cookie strings which can throw URIError for malformed percent-escapes;
wrap the decodeURIComponent call in a try/catch (or perform a safeDecode helper
used by both functions) so that if decodeURIComponent throws you fall back to
the raw value instead of propagating the error, preserving existing behavior
(return accessToken/refreshToken or undefined) and preventing a 500 from
malformed cookies.
Summary
cmux-dev-<tag>callbackauth.sign_in_urlover the control socketMechanism
The normal macOS popup path stays on
ASWebAuthenticationSession. Tagged dev builds now registercmux-dev-<tag>in Info.plist and pass that same scheme through the native auth URL. The web handoff enters/handler/native-sign-in, mints an httpOnly handoff cookie, and only auto-opens the native callback when the after-sign-in route sees both the expected stateful callback and matching handoff nonce. Direct/manual fallback links remain click-only.Verification
/Users/lawrence/fun/cmuxterm-hq/skills/autoreview/scripts/autoreview --mode local/Users/lawrence/fun/cmuxterm-hq/skills/autoreview/scripts/autoreview --mode branch --base origin/mainswift testinPackages/CmuxAuthRuntimebunx tsc --noEmitinwebbash -n scripts/reload.sh && git diff --checkxcodebuild -project cmux.xcodeproj -scheme cmux-unit -configuration Debug -destination "platform=macOS" -derivedDataPath /tmp/cmux-safauth-cli-test -only-testing:cmuxTests/AuthEnvironmentTests testCMUX_PORT=4577 CMUX_PORT_RANGE=1 CMUX_PORT_END=4577 ./scripts/reload.sh --tag safauthcmux-dev-safauthandCMUX_AUTH_CALLBACK_SCHEME=cmux-dev-safauth/handler/native-sign-inonhttp://localhost:4577and confirmed the handoff cookie + Stack redirectNeed help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
High Risk
Touches authentication token handoff, callback validation, and web/native redirect security; regressions could block sign-in or accept wrong callbacks.
Overview
Tagged dev native auth now uses per-tag callback schemes (
cmux-dev-<tag>):AuthEnvironmentderives them fromCMUX_TAG,scripts/reload.shwires Info.plist URL schemes and env, and sign-in URLs embedcmux_auth_stateon the native return URL.Browser sign-in flow is hardened in
HostBrowserSignInFlow: per-attemptcmux_auth_state,manualSignInURLfor CLI/socket fallback, stricter external callback routing (state match, stale/sign-out rejection), non-authASWebAuthenticationSessioncompletions that wait for a real callback, guarded continuation resumes, and abandoned-attempt timeouts without killing in-flight token validation. Diagnostics add structured logging and broader redaction (includingcmux_auth_state).Control surface: new socket method
auth.sign_in_url; CLIcmux auth loginprints a fallback sign-in URL before opening the popup.Web handoff: sign-in enters
/handler/native-sign-in(httpOnly handoff nonce) instead of Stack sign-in directly; after-sign-in moves to a localized serverroute.tsthat only auto-opens native when scheme, state, and nonce align; manual fallback stays click-only. i18n strings added for the return page.Tests expand
HostBrowserSignInFlow,AuthDebugLog,AuthEnvironment, and execution-policy coverage.Reviewed by Cursor Bugbot for commit 1f1cb27. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes native auth handoff for tagged dev builds and hardens the browser sign-in flow. Tagged schemes now round-trip reliably, and Safari/ASWebAuthenticationSession callbacks are state-checked and nonce-gated.
New Features
cmux-dev-<tag>;scripts/reload.shwrites the Info.plist URL scheme and setsCMUX_AUTH_CALLBACK_SCHEME/CMUX_TAG./handler/native-sign-inwrapper and a server/handler/after-sign-inroute with an httpOnly handoff cookie andcmux_auth_handoffnonce; auto-open only when state and nonce verify.auth.sign_in_urlon the control socket; the CLI prints a fallback sign-in URL before opening the popup.HostBrowserSignInFlowgenerates a statefulmanualSignInURL, embedscmux_auth_statein native return URLs, and gates external callbacks by state.ASWebAuthenticationSession/runtime logs, optionalCMUX_DEBUG_LOGpath(s), and broader redaction (covers encodedcmux_auth_stateand token values).Bug Fixes
cmux_auth_state; rejects stale/mismatched callbacks and late callbacks after sign-out.ASWebAuthenticationSession, waits for external callbacks, prevents double resumes, and times out abandoned attempts without canceling in-flight validation.cmux,cmux-nightly, and localcmux-dev[-tag]) to prevent unintended launches.Written for commit 1f1cb27. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Tests