fix(auth): log in when the browser is not on this machine - #1418
fix(auth): log in when the browser is not on this machine#1418cristifalcas wants to merge 1 commit into
Conversation
`aspect auth login` opened the browser by shelling out to `open`, then waited
for the OAuth code on a loopback listener. Both halves fail away from a desktop:
- `open` is macOS. On Linux it is at best Debian's xdg-utils symlink, so a
login either recited `www-browser: not found` six times or died with
"failed to spawn command open" and an AXL traceback (ENG-2098).
- Worse, the redirect is a loopback address. Paste the URL into a browser on
another machine and the code is delivered to *that* machine's port —
ERR_CONNECTION_REFUSED for the user and a CLI that waits forever. No amount
of browser-opening fixes it; over SSH the code has to come back by hand.
Opening a browser moves into the runtime as `ctx.aspect.auth.open_browser`,
which carries the platform table (`$BROWSER`, then xdg-open/gio/gnome-open/
kde-open/x-www-browser/wslview on Linux, `open` on macOS, rundll32 on Windows —
it takes the URL as one argument, where `cmd /c start` would hand an
`&`-separated authorize URL back to cmd.exe to re-parse). Launcher output is
discarded; the task prints one message that says what to do instead.
It reports "headless" without trying at all when this is an SSH session with no
forwarded display. A display counts only if it names a host (`localhost:10.0`,
what ssh -X sets) — a bare `:0` is the remote machine's own screen, which is
what a Cloud Workstation image sets and where a browser would open in front of
nobody.
`AuthSession.redeem(pasted)` is then the login path for those sessions: it takes
the browser's full address-bar URL, a bare query string, or the bare code.
`wait()` and `redeem()` now share one `complete()`, so both reach an identical
token exchange; `state` is still validated whenever it is present, and a stale
paste from an earlier run is rejected. Off a TTY there is nothing to paste with,
so the listener is still waited on — a forwarded port delivers.
`--no-browser` forces the paste flow on both `login` and `configure`. The port
is only worth forwarding for the Aspect account, which binds a fixed 19556; a
self-hosted deployment takes a fresh port each run, so that caveat lives in the
flag's help text rather than as a hint the user cannot act on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ef89800e5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for entry in browser_env.unwrap_or_default().split(':') { | ||
| let argv: Vec<String> = entry.split_whitespace().map(str::to_string).collect(); |
There was a problem hiding this comment.
Preserve quoting when parsing BROWSER commands
When $BROWSER contains a quoted command line such as firefox --new-window "%s" or sh -c 'open-browser %s', split_whitespace() leaves the quote characters in the arguments and splits shell expressions incorrectly. spawn_launcher then substitutes the authorize URL into those malformed arguments; if the launcher still exits successfully, the CLI reports that the browser opened and waits indefinitely even though it received an invalid URL. Parse each colon-separated entry with shell-aware argument parsing rather than raw whitespace splitting.
Useful? React with 👍 / 👎.
Aspect Workflows Tasks📅 Tue Sep 1 14:12:48 UTC 2026 Task Results
Fix❌ buildifier (buildifier-gha-debug · buildifier-gha)Install Reproduce❌ buildifier (buildifier-gha-debug · buildifier-gha)❌ delivery (delivery-uncacheable · delivery-gha-debug · delivery-gha)Install ⏱ Last updated Tue Sep 1 14:35:40 UTC 2026 · 📊 GitHub API quota 782/15,000 (5% used, resets in 36m) |
Fixes
aspect auth loginon Linux and over SSH. Closes ENG-2098.The two faults
1. The opener was macOS-only.
_run_browser_loginshelled out toopen. On Linux that is at best Debian's xdg-utils symlink, so a login either recitedwww-browser: not foundsix times or died outright:2. The one that actually blocks people. The redirect is a loopback address. Paste the URL into a browser on another machine and the code is delivered to that machine's port:
…and the CLI waits forever. No amount of browser-opening fixes this. For a self-hosted deployment the edge's own callback page is explicit about it (
jwt-validator/files/envoy.yaml:106):The consolation:
location.searchis forwarded whole, so the address bar already holds the code. It is stranded, not lost.What this does
Opening a browser moves into the runtime as
ctx.aspect.auth.open_browser, carrying the platform table:$BROWSERfirst, thenxdg-open/gio open/gnome-open/kde-open/x-www-browser/wslviewon Linux,openon macOS, andrundll32 url.dll,FileProtocolHandleron Windows — that last because it takes the URL as one argument, wherecmd /c startwould hand an&-separated authorize URL back tocmd.exeto re-parse. Launcher output is discarded, so the six-linenot foundrecital is replaced by one message that says what to do instead.It doesn't try at all under SSH. A display counts only if it names a host (
localhost:10.0, whatssh -Xsets); a bare:0is the remote machine's own screen — which is what a Cloud Workstation image sets, and where a browser opens in front of nobody.AuthSession.redeem(pasted)is the login path for those sessions. It accepts the browser's full address-bar URL, a bare query string, or the bare code.wait()andredeem()now share onecomplete(), so both reach an identical token exchange.--no-browserforces the paste flow onloginandconfigure.Notes on the choices
stateis validated whenever the paste carries it; a stale paste from an earlier run is rejected. A bare code arrives without one, and there the paste is the user's intent, sorequire_stateis false for that path only.--no-browser's help text instead.open/webbrowsercrate would mean Cargo.lock and crate_universe churn for less.Test plan
538 tests pass (
cargo test -p axl-runtime -p aspect-cli), including new unit coverage forparse_pasted_callback,is_remote_session,is_forwarded_displayandbrowser_launchers.Driven by hand under a pty, since the branch that matters needs a TTY:
DISPLAY=:0(workstation shape)DISPLAY=localhost:10.0--no-browsercurlto the listener)127.0.0.1URL,--deployment silo-gcpcodeparamBoth flows fail identically on an invented code, which is as close as this gets to proving
redeemandwaitare equivalent without a real credential.Also verified directly on
gcloud workstations sshto anaspect-internal-devworkstation:SSH_CONNECTIONset,DISPLAYempty (detection fires), and the workstation reachesauth.aspect.build/oauth/token, so the exchange completes from there.Not verified: a real end-to-end login with a live code (needs a browser identity), and the Linux launcher table on Linux — though on a workstation it is never reached, since detection short-circuits first.
Aside, for the SaaS launch
auth.aspect.buildadvertisesdevice_authorization_endpointand thedevice_codegrant — RFC 8628, the flowghuses, and the properly correct answer to this whole class of problem. The client is currently refused withInvalid client_id, so it wants enabling on the Frontegg side; not something the CLI can land on its own.🤖 Generated with Claude Code