login: harden the browser sign-in fallbacks - #1403
Merged
toothbrush merged 4 commits intoJun 10, 2026
Merged
Conversation
The device flow is bounded by the AS's expires_in (capped at 15m), but the browser flow waited on the loopback redirect with no deadline — a closed tab left `entire login` hanging until Ctrl-C. Bound the wait at 5 minutes, starting after the Enter prompt so reading time isn't counted, and point the timeout error at `--device` as the escape hatch. Parent-context cancellation (Ctrl-C) still surfaces as cancellation, not as a timeout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 8979d0646ea0
Over SSH with a TTY, CanPromptInteractively() is true so the browser flow was chosen — but the loopback listener binds 127.0.0.1 on the remote host, where the user's local browser can't reach it; even the printed fallback URL can't complete. Detect SSH sessions via the SSH_CONNECTION/SSH_CLIENT/SSH_TTY vars sshd sets and route them to the device-code flow with a one-line explanation, the same way gh and gcloud do. Extract the flow choice from the cobra RunE into runLoginAuto, taking a startBrowser func and the environment facts as plain values, so the selection and its stderr commentary are unit-testable with fakes. The browser-flow integration test blanks the SSH_* vars it inherits from os.Environ() so it keeps exercising the browser path when a developer runs the suite over SSH. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 4aa664e559dc
If StartBrowserAuth couldn't bind the 127.0.0.1 listener (sandboxing, firewall policy, exhausted ports), `entire login` errored out even though the device-code flow would have worked fine. Warn on stderr with the bind error and continue with the device flow instead of stranding the user. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 247398f9ecc7
Cleanups from a reuse/simplification/efficiency/altitude review of the browser-flow hardening commits; no behavior change: - Collapse runLoginAuto's three trailing bools into a loginFlowFacts struct, so call sites are self-documenting instead of relying on /* useDevice */-style comments. - Pass canPrompt into runLogin instead of re-probing /dev/tty via interactive.CanPromptInteractively() a second time on the device path. - Slim the startBrowser adapter closure to a direct return — the typed-nil interface on error is harmless since callers check err first. - Move the SSH_* env blanking from the one browser-flow integration test into startLoginProcess, so future login subprocess tests are SSH-safe by default (same centralization idea as GitIsolatedEnv). - Share one noopOpenURL test helper instead of seven inline lambdas. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 224fb4e17934
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens entire login’s default browser (loopback) sign-in behavior by adding bounded waiting and more robust, user-friendly fallbacks to the device-code flow across SSH/headless and listener-bind failure scenarios.
Changes:
- Add a 5-minute timeout to the browser-flow loopback redirect wait, with a timeout error that points users to
entire login --device. - Route SSH sessions and headless/non-interactive environments to the device-code flow, emitting a one-line stderr explanation (except when
--deviceis explicitly set). - Fall back to the device-code flow (with warning) when the browser-flow listener fails to start; add unit + integration test hardening (clearing inherited
SSH_*env vars for subprocess tests).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cmd/entire/cli/login.go | Adds runLoginAuto flow selection with SSH/headless detection, browser wait timeout, and device-flow fallbacks on browser listener start failures. |
| cmd/entire/cli/login_test.go | Expands unit tests to cover flow selection, SSH detection, browser-start failure fallback messaging, and timeout vs cancellation behavior. |
| cmd/entire/cli/integration_test/login_test.go | Stabilizes browser-flow subprocess integration tests by blanking inherited SSH_* environment variables by default. |
toothbrush
approved these changes
Jun 10, 2026
toothbrush
left a comment
toothbrush
left a comment
Contributor
There was a problem hiding this comment.
Ah, yes, these are all very sensible improvements i think 👍
Soph
marked this pull request as ready for review
June 10, 2026 14:38
This was referenced Jul 10, 2026
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
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.
https://entire.io/gh/entireio/cli/trails/550
What
Hardening follow-ups to #1366 (browser sign-in default), from its code review. Stacked on
login-loopback-browser-flow— review/merge that first.expires_in; the browser flow waited forever if the user closed the tab. The timeout error points atentire login --deviceas the escape hatch, and Ctrl-C still reports as cancellation, not a timeout.ssh -tthe browser flow was chosen (a TTY is present), but the loopback listener binds 127.0.0.1 on the remote host where the user's browser can't reach it. Detect SSH via theSSH_CONNECTION/SSH_CLIENT/SSH_TTYvars sshd sets — same asgh/gcloud— and explain the fallback in one line on stderr.StartBrowserAuthcan't bind (sandboxing, firewall, exhausted ports), warn and continue with the device flow instead of erroring out.How
The flow choice moved out of the cobra
RunEintorunLoginAuto, which takes aloginFlowFactsstruct (useDevice/canPrompt/sshSession, detected once at entry) plus astartBrowserfunc, so the selection logic and all three fallback paths are unit-tested with fakes.startLoginProcessin the integration tests now blanks the inheritedSSH_*vars centrally, so browser-flow subprocess tests stay on the browser path even when the developer runs the suite over SSH.Testing
--deviceproduces no commentary,isSSHSession.mise run checkgreen.🤖 Generated with Claude Code
Note
Medium Risk
Changes authentication flow selection and timeout behavior for
entire login, which is security-sensitive but limited to CLI UX and fallbacks with broad unit test coverage.Overview
Hardens
entire loginbrowser sign-in with timeouts, SSH-aware routing, and automatic fallbacks to the device-code flow.Browser wait is capped at 5 minutes (
browserLoginTimeout). Loopback redirect wait uses a context timeout starting after the Enter prompt; deadline exceeded yields an error that suggestsentire login --device, while parent cancellation still surfaces ascontext.Canceled, not a timeout.Flow selection moves into
runLoginAutowith aloginFlowFactsstruct (--device, interactive TTY, SSH session). Browser flow is skipped when SSH env vars indicate a remote session (one-line stderr explanation), when headless, or when--deviceis set. IfStartBrowserAuthfails to bind loopback, the CLI warns and falls back to device flow instead of failing outright.Integration
startLoginProcessclears inheritedSSH_*vars so browser-flow subprocess tests stay stable when the suite runs over SSH. Unit tests cover SSH/headless/listener-failure paths, timeout vs cancel, and--devicesilence on stderr commentary.Reviewed by Cursor Bugbot for commit 334df98. Configure here.