Repository navigation
Security: loopback CLI login bound to the approving browser - #22
Closed
ndbroadbent wants to merge 3 commits into
Closed
ndbroadbent wants to merge 3 commits into
ndbroadbent wants to merge 3 commits into
Conversation
The CLI login used to be a device flow without a user code: whoever held the gateway-issued `state` got the 90-day session. /auth/cli/complete never checked the code_verifier, states never expired, `state` and the OAuth code were written to the request log, and nothing tied the browser that approved the login to the CLI that polled. A log reader or anyone who got a user to open their login link could take over the session. Now: - The CLI generates the PKCE verifier and its own state, listens on 127.0.0.1 with a random port, and sends only the S256 challenge, state and loopback redirect URI to /auth/cli/start. - The browser that completes Google OAuth is bound to the login with an HttpOnly cookie. The MFA form, MFA submit and return steps all require it, so a third party holding the state can't use it or burn the user's MFA attempts. - After MFA, the browser is redirected to the CLI's loopback listener with a single-use login code (stored hashed, 10-minute TTL, consumed atomically). /auth/cli/complete needs the code AND the verifier. The polling endpoint and the old state-as-credential path are gone. - The MFA page shows the initiating IP and device name. A completed CLI login sends a notification. - state, code and login_code query params are redacted from request and audit logs. - The CLI only opens https auth URLs, and on Windows uses rundll32 rather than `cmd /c start` (which split URLs on '&'). E2E harness: the simulated browser stays on the gateway's hostname (the binding cookie is host-scoped, as in a real browser), and commands run with stdin from /dev/null. `env set` reads extra input from a non-terminal stdin until EOF, so an inherited open pipe hung the suite.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| JavaScript |  |
Oct 9, 2026 11:09p.m. | Review ↗ |
| Go |  |
Oct 9, 2026 11:09p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
# Conflicts: # scripts/lib/cli-e2e/cli_helpers.sh # web/e2e/cli-login-webui.spec.ts
Findings from the PR #22 review: - A first factor enrolled during an enrollment-required CLI login only completes the login if the browser that approved it verified its own session after the enrollment. - The Google code is exchanged in the callback and the browser is bound only after it succeeds, so a junk-code callback can't claim the login, and the Google code is no longer stored. - Failures after binding, provider errors and exchange failures go back to the CLI's loopback listener, so the CLI exits instead of waiting out its timeout. "Cancel Login" now cancels on the server (POST /auth/cli/cancel) and tells the CLI. - The CLI always prints the login URL, accepts any https identity provider, and reports version skew clearly in both directions. - Login codes are issued once; the CLI-login email dedup uses the client IP instead of the client-supplied device name. - Docs describe the loopback flow, the same-machine requirement (ssh -L for remote hosts) and the CLI upgrade needed after a gateway upgrade.
Member
Author
|
Merged to main as part of #25 (combined security audit release). |
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.
This PR is stacked on #19. It fixes RG-10 (AUTH-2, CLI-2, CLI-3, MFA-9) from the 2026-10-09 audit.
Problem
CLI login was effectively a device flow with no user code, so whoever held the gateway-issued
stategot the 90-day session:/auth/cli/completenever checked thecode_verifier.stateand the OAuth code were logged in plaintext.Anyone who could read the gateway logs, or get you to open their login link, could take over the session.
Fix (RFC 8252 loopback)
127.0.0.1:<random port>./auth/cli/startreceives only the S256 challenge, the state and the loopback redirect URI. The redirect URI is strictly validated: http, loopback host,/callback./auth/cli/complete. The polling path and the state-as-credential path are removed.Also in this PR:
state,codeandlogin_codeare redacted from request and audit logs.rundll32instead ofcmd /c start, which split URLs on&.Testing
/dev/nullas stdin.env setreads a non-terminal stdin until EOF, so an inherited pipe hung the suite.task go:test,task web:testand lint all pass, web E2E is 56/56, and CLI E2E is green.Deploy note
The login protocol changes, so after deploying a gateway with this change you need a new CLI build to log in to it. CircleCI uses API tokens rather than login, so it's unaffected.