Auth: CLI device-code login#732
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 268a64fada
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| CREATE TABLE IF NOT EXISTS device_auth_grant ( | ||
| id uuid PRIMARY KEY DEFAULT gen_random_uuid(), | ||
| device_code_hash text NOT NULL UNIQUE, | ||
| user_code varchar(6) NOT NULL UNIQUE, |
There was a problem hiding this comment.
Because user_code is globally unique, every approved, denied, expired, or consumed device grant permanently removes one of only 1,000,000 possible CLI codes. reserveUserCode only checks active pending grants, so once the random generator picks any historical code the insert fails with a unique-constraint error and CreateDeviceCode returns 500; the collision rate grows quickly as login attempts accumulate. Use a partial uniqueness strategy for active grants or retry on historical-code conflicts.
Useful? React with 👍 / 👎.
| r.Post("/device/code", h.CreateDeviceCode) | ||
| r.Post("/device/token", h.PollDeviceToken) | ||
| r.Get("/device/grant", h.GetDeviceGrant) | ||
| r.Post("/device/grant", h.UpdateDeviceGrant) |
There was a problem hiding this comment.
UpdateDeviceGrant performs a cookie-authenticated state change by calling BrowserSession, but this route is mounted with the public auth routes in router.go, so it skips the authMiddleware.Require path that applies the Origin/Referer CSRF check and workspace IP restrictions for other mutating browser-session requests. In browser contexts where the session cookie is sent to /api/auth/device/grant, a forged POST can approve or deny a device code without those protections; move this endpoint behind the protected middleware or apply the same checks here.
Useful? React with 👍 / 👎.
| const child = spawn(command, args, { detached: true, stdio: "ignore" }); | ||
| child.unref(); |
There was a problem hiding this comment.
When xdg-open/open is missing or BROWSER points to a bad command, Node's spawn() reports ENOENT via the child process error event rather than throwing from this call. With no error listener, default expn login can terminate on an unhandled error instead of returning false and showing the manual copy/paste instructions, which is common in headless shells and minimal containers.
Useful? React with 👍 / 👎.
|
Controller disposition for current head
Controller evidence:
Required before merge: add/run the missing device-flow Playwright approval+denial coverage and web/account-security revocation visibility coverage, then refresh this PR head. |
1 participant
Summary
cliscope/auth/deviceapproval/denial UI and update login-page CLI pairing copyexpn logindefault to browser device flow while preserving--token pat_...fallbackVerification
gobinary is not installed in this lane image; failed atpnpm api:build)Remaining