Repository navigation
feat: device authorization for the CLI - #5
Merged
Merged
Conversation
Enable better-auth deviceAuthorization (verification page at the web app's /auth/device) and the bearer plugin so the CLI can authenticate with the session token. Add the device_code drizzle model, WEB_APP_URL env var, and a migration creating the table. Rename migrations to descriptive tags and route drizzle/auth scripts through dotenvx so they read apps/server/.env. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the deviceAuthorization client plugin and a /auth/device route where a signed-in user enters the code from their terminal to approve or deny a device, bouncing through GitHub sign-in first when needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 30 minutes and 28 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (15)
📝 WalkthroughWalkthroughAdds device authorization across the server, CLI, and web app. The server gains device auth plugins, a ChangesOAuth Device Authorization Flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
biome.json (1)
29-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow this ignore to the generated migration metadata path.
!**/metaexcludes everymetadirectory in the repo, not justapps/server/src/db/migrations/meta. That can silently remove unrelated source from linting/formatting later.🤖 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 `@biome.json` at line 29, The ignore rule is too broad because `!**/meta` unignores every `meta` directory in the repo instead of only the generated migration metadata path. Update the Biome ignore entry to target the specific migrations metadata location used by the server DB migrations, so only `apps/server/src/db/migrations/meta` is excluded and unrelated `meta` folders remain covered.apps/cli/src/index.ts (1)
13-13: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse
parseAsync()with these async Commander actions.Commander documents that async action handlers should be paired with
.parseAsync()rather than.parse(). (github.com)Suggested change
-program.parse(Bun.argv); +await program.parseAsync(Bun.argv);🤖 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 `@apps/cli/src/index.ts` at line 13, The CLI entrypoint is using program.parse while it has async Commander actions, so update the program setup in index.ts to use program.parseAsync instead. Locate the top-level command parsing call on the Commander program instance and switch it to the async parsing API so async action handlers are awaited correctly.apps/cli/src/utils/style.ts (1)
21-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis printer API is what forced the broad
noUnusedExpressionswaiver.Using
print.dim\...`as an expression statement is whybiome.jsonnow disablessuspicious.noUnusedExpressionsforapps/cli/**. That turns off a useful bug-catching rule across the whole CLI. Prefer callable printers (print.dim("...")`) or scope the override down to the handful of files that truly need tagged templates.🤖 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 `@apps/cli/src/utils/style.ts` around lines 21 - 42, The current printer API in style.ts forces tagged-template usage, which is why noUnusedExpressions had to be broadly waived. Update the printer/print API so the exported helpers like print.dim, print.success, and print.error are callable functions instead of tag-only template printers, and adjust any local uses in this module accordingly. If template support must remain, keep it isolated and avoid requiring the broad suspicious.noUnusedExpressions override in biome.json.
🤖 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 `@apps/cli/src/commands/auth/logout.ts`:
- Around line 5-12: The logout flow in logout() is treating readToken() like a
synchronous guard and also leaves clearToken() outside a failure-safe path.
Await readToken() before deciding whether to return early, and make sure
authClient.signOut() and clearToken() are handled so the local token is always
removed in a finally block even if signOut() fails. Keep the success message
only after the async cleanup completes successfully.
In `@apps/cli/src/commands/auth/whoami.ts`:
- Around line 11-15: In whoami’s session check, avoid calling clearToken() for
every authClient.getSession() error, since transient network/5xx failures should
not wipe a still-valid cached credential. Update the logic around
authClient.getSession() to only clear the token when the response clearly
indicates an invalid/unauthenticated session, and for other errors surface the
error while preserving the token; keep the existing not-logged-in handling in
the whoami command.
In `@apps/cli/src/index.ts`:
- Line 10: The `logout` command is wired to a broken handler in `logout` from
`apps/cli/src/commands/auth/logout.ts`; fix the token check and deletion flow so
it only signs out when a token exists. Update the `logout` function to await
`clearToken()` before returning, and verify the condition around `readToken()`
so it does not always take the sign-out path when no token is present.
In `@apps/cli/src/utils/store.ts`:
- Around line 19-27: The readToken function is collapsing non-ENOENT token-store
failures into a null token by using Result.tryPromise(...).unwrapOr("null"), and
YAML.parse can still throw on bad content. Update readToken in store.ts so only
a missing file returns null, while other file read or parse failures are
propagated as actionable errors; keep the logic around file.exists(),
file.text(), and YAML.parse, but distinguish ENOENT from corruption/read
failures instead of treating them as logged-out state.
- Around line 12-16: The saveToken flow currently writes the token file via
Bun.file.write and then fixes permissions afterward, which leaves a brief window
with unsafe defaults. Update saveToken in store.ts to use a Node fs-based
create/write path that opens or creates TOKEN_FILE_PATH with restrictive 0600
permissions from the start, while keeping the YAML.stringify({ token } satisfies
Stored) content generation intact.
In `@apps/server/package.json`:
- Line 13: The auth:generate script currently writes Better Auth output into the
same auth model file that also contains the custom deviceCode table, which risks
overwriting hand-written schema. Update the package.json script to generate auth
tables into a dedicated file, or move custom tables like deviceCode out of the
generated target, and then make sure src/db/models/index.ts still imports the
correct split model files via the auth/index-related symbols.
In `@apps/server/src/db/migrations/0001_device_authorization.sql`:
- Around line 1-12: The device_code table definition is missing uniqueness
enforcement for the device_code and user_code fields, which can allow duplicate
grants to be resolved incorrectly. Update the migration to add UNIQUE
constraints or unique indexes for these columns, and if expired rows must be
retained then use partial unique indexes instead. Keep the change within the
device_code table migration so the constraints are enforced when device polling
and approval look up grants.
In `@apps/server/src/db/migrations/meta/_journal.json`:
- Around line 9-17: Restore the original idx 0 migration tag in _journal.json so
the initial migration identity stays unchanged; update the journal entry back to
the existing tag used by the first migration and leave the subsequent
0001_device_authorization entry intact. Use the migration metadata in the
journal array (especially the first object’s tag field and idx 0) to locate and
revert only that identity, without changing the rest of the migration sequence.
In `@apps/server/src/db/models/auth.ts`:
- Around line 83-94: In deviceCode, add database constraints for the lookup keys
used by the auth flow: make userCode and deviceCode unique and indexed so web
approvals and CLI polling resolve a single record efficiently. Update the
pgTable("device_code", ...) definition to include UNIQUE/INDEX metadata for
these columns, using the existing deviceCode symbol and the related
userCode/deviceCode fields.
In `@apps/web/src/routes/auth.device.tsx`:
- Around line 79-101: Handle failures from the claim step in decide before the
handler exits, because authClient.$fetch("/device") can reject for
invalid/expired codes or network errors and leave the page stuck in busy state.
Wrap the claim-and-approve/deny flow in try/finally (or equivalent) so
setBusy(false) always runs, and catch the claim-step error to show a toast via
errorMessage instead of letting the exception escape. Use decide,
authClient.$fetch("/device"), and setBusy(false) as the key locations to update.
---
Nitpick comments:
In `@apps/cli/src/index.ts`:
- Line 13: The CLI entrypoint is using program.parse while it has async
Commander actions, so update the program setup in index.ts to use
program.parseAsync instead. Locate the top-level command parsing call on the
Commander program instance and switch it to the async parsing API so async
action handlers are awaited correctly.
In `@apps/cli/src/utils/style.ts`:
- Around line 21-42: The current printer API in style.ts forces tagged-template
usage, which is why noUnusedExpressions had to be broadly waived. Update the
printer/print API so the exported helpers like print.dim, print.success, and
print.error are callable functions instead of tag-only template printers, and
adjust any local uses in this module accordingly. If template support must
remain, keep it isolated and avoid requiring the broad
suspicious.noUnusedExpressions override in biome.json.
In `@biome.json`:
- Line 29: The ignore rule is too broad because `!**/meta` unignores every
`meta` directory in the repo instead of only the generated migration metadata
path. Update the Biome ignore entry to target the specific migrations metadata
location used by the server DB migrations, so only
`apps/server/src/db/migrations/meta` is excluded and unrelated `meta` folders
remain covered.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46b31281-8708-4a9a-9977-fb340a931814
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockmise.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
apps/cli/package.jsonapps/cli/src/commands/auth/login.tsapps/cli/src/commands/auth/logout.tsapps/cli/src/commands/auth/whoami.tsapps/cli/src/index.tsapps/cli/src/lib/auth.tsapps/cli/src/lib/env.tsapps/cli/src/paths.tsapps/cli/src/serve.tsapps/cli/src/utils/store.tsapps/cli/src/utils/style.tsapps/cli/tsconfig.jsonapps/server/package.jsonapps/server/src/auth/index.tsapps/server/src/config/env.tsapps/server/src/db/migrations/0000_db_init.sqlapps/server/src/db/migrations/0001_device_authorization.sqlapps/server/src/db/migrations/meta/0001_snapshot.jsonapps/server/src/db/migrations/meta/_journal.jsonapps/server/src/db/models/auth.tsapps/server/src/db/models/index.tsapps/web/src/lib/auth.tsapps/web/src/routeTree.gen.tsapps/web/src/routes/auth.device.tsxbiome.json
💤 Files with no reviewable changes (2)
- apps/cli/src/serve.ts
- apps/cli/src/paths.ts
soorya-u
added a commit
that referenced
this pull request
Jun 28, 2026
- logout: await readToken and always clear the local token in finally so a valid bearer token never lingers after a successful sign-out message - whoami: only clear the token on an empty session, not on transient/server errors that would otherwise force an unnecessary relogin - store: create the token file with 0600 atomically (Bun.write can't set the mode), and stop swallowing read/parse errors as a logged-out state — only a missing file maps to null Addresses CodeRabbit review on #5. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
soorya-u
added a commit
that referenced
this pull request
Jun 28, 2026
Wrap the claim/approve/deny calls so setBusy(false) always runs and a rejected request shows a toast instead of throwing and leaving the buttons disabled. Addresses CodeRabbit review on #5. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/cli/src/utils/store.ts (1)
1-18: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAvoid in-place truncation when saving the token.
writeFile(TOKEN_FILE_PATH, ...)updates the live config in place. If the process dies mid-write,config.ymlcan be left partial/corrupt and later auth commands will fail until the file is repaired. Write to a0600temp file in the same directory and rename it into place after the write succeeds.Suggested fix
-import { chmod, mkdir, writeFile } from "node:fs/promises"; +import { chmod, mkdir, rename, writeFile } from "node:fs/promises"; +import { randomUUID } from "node:crypto"; +import { join } from "node:path"; export async function saveToken(token: string) { - await mkdir(dirname(TOKEN_FILE_PATH), { recursive: true, mode: 0o700 }); + const dir = dirname(TOKEN_FILE_PATH); + await mkdir(dir, { recursive: true, mode: 0o700 }); const content = YAML.stringify({ token } satisfies Stored); - await writeFile(TOKEN_FILE_PATH, content, { mode: 0o600 }); + const tempPath = join(dir, `.config.${randomUUID()}.tmp`); + await writeFile(tempPath, content, { mode: 0o600, flag: "wx" }); + await rename(tempPath, TOKEN_FILE_PATH); await chmod(TOKEN_FILE_PATH, 0o600); }🤖 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 `@apps/cli/src/utils/store.ts` around lines 1 - 18, The saveToken flow in store.ts currently writes directly to TOKEN_FILE_PATH via writeFile, which can leave config.yml partial if the process dies mid-write. Update saveToken to write the YAML content to a temporary 0600 file in the same directory first, then rename it into TOKEN_FILE_PATH after the write succeeds; keep the existing directory creation and permissions handling around TOKEN_FILE_PATH and the saveToken helper.
♻️ Duplicate comments (1)
apps/cli/src/utils/store.ts (1)
27-28: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate the parsed YAML shape before returning it.
This cast does not enforce
{ token: string }at runtime. A valid YAML file liketoken: 123ortoken: {}can cross the store boundary as a non-string value, which breaks the token contract instead of surfacing corruption.Suggested fix
const content = await file.text(); - return (YAML.parse(content) as Stored | null)?.token ?? null; + const parsed = YAML.parse(content); + if ( + !parsed || + typeof parsed !== "object" || + !("token" in parsed) || + typeof parsed.token !== "string" + ) { + throw new Error(`Invalid token store format at ${TOKEN_FILE_PATH}`); + } + return parsed.token; }🤖 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 `@apps/cli/src/utils/store.ts` around lines 27 - 28, The YAML parsing in store.ts is only using a type cast, so invalid shapes like a non-string token can slip through at runtime. Update the logic around the existing YAML.parse call in the store read helper to explicitly validate that the parsed value is an object with a string token before returning it, and otherwise return null or handle corruption; use the store function that reads the token as the place to enforce this contract.
🤖 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.
Outside diff comments:
In `@apps/cli/src/utils/store.ts`:
- Around line 1-18: The saveToken flow in store.ts currently writes directly to
TOKEN_FILE_PATH via writeFile, which can leave config.yml partial if the process
dies mid-write. Update saveToken to write the YAML content to a temporary 0600
file in the same directory first, then rename it into TOKEN_FILE_PATH after the
write succeeds; keep the existing directory creation and permissions handling
around TOKEN_FILE_PATH and the saveToken helper.
---
Duplicate comments:
In `@apps/cli/src/utils/store.ts`:
- Around line 27-28: The YAML parsing in store.ts is only using a type cast, so
invalid shapes like a non-string token can slip through at runtime. Update the
logic around the existing YAML.parse call in the store read helper to explicitly
validate that the parsed value is an object with a string token before returning
it, and otherwise return null or handle corruption; use the store function that
reads the token as the place to enforce this contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1d740d86-0f45-442d-936f-fbd7181741b2
📒 Files selected for processing (4)
apps/cli/src/commands/auth/logout.tsapps/cli/src/commands/auth/whoami.tsapps/cli/src/utils/store.tsapps/web/src/routes/auth.device.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/cli/src/commands/auth/logout.ts
- apps/web/src/routes/auth.device.tsx
Drive the OAuth 2.0 device flow from the CLI: login requests a device code, polls for the token, and persists it (0600 file via Bun/YAML); whoami and logout use it as a bearer token. Add a Bun-native color/print helper, env config (@t3-oss/env-core), and the @/* path alias; organize commands under src/commands. Disable noUnusedExpressions for apps/cli so print tagged templates pass lint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- logout: await readToken and always clear the local token in finally so a valid bearer token never lingers after a successful sign-out message - whoami: only clear the token on an empty session, not on transient/server errors that would otherwise force an unnecessary relogin - store: create the token file with 0600 atomically (Bun.write can't set the mode), and stop swallowing read/parse errors as a logged-out state — only a missing file maps to null Addresses CodeRabbit review on #5. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wrap the claim/approve/deny calls so setBusy(false) always runs and a rejected request shows a toast instead of throwing and leaving the buttons disabled. Addresses CodeRabbit review on #5. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the raw `$fetch("/device", …)` escape hatch with the generated typed
client method `authClient.device({ query: { user_code } })` — same GET request,
but type-checked and without a hardcoded path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AuthProvider expects the shared TanStack queryClient; wire it from the QueryClientProvider via useQueryClient(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
soorya-u
force-pushed
the
feat/device-authorization
branch
from
June 28, 2026 16:38
312cf46 to
ff72be5
Compare
This was referenced Jul 8, 2026
This was referenced Jul 25, 2026
This branch was successfully deployed
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.
Summary
Adds OAuth 2.0 Device Authorization (RFC 8628) so the
cyrusCLI can sign in via the browser, plus the supporting server endpoints and web verification page.Server
deviceAuthorization(verification page → web app/auth/device) and thebearerplugin so the CLI authenticates with its session token.device_codedrizzle model +WEB_APP_URLenv var, and a migration creating the table.db_init,device_authorization); route drizzle/auth scripts throughdotenvxso they readapps/server/.env.Web
deviceAuthorizationclient plugin + a/auth/deviceroute where a signed-in user approves/denies a device by entering the code from their terminal (bounces through GitHub sign-in when needed).CLI
login(device-code flow + polling),whoami, andlogoutcommands.0600YAML file ($CYRUS_HOME/config.yml); sent as a bearer token on subsequent calls.printhelper (nochalk),@t3-oss/env-coreconfig,@/*path alias, commands undersrc/commands.Verification
bun check(Biome) andbun check:types(turbo) both pass.login→ approve →whoami→logout.Notes
get-session, logout) was verified directly.noUnusedExpressionsis disabled forapps/clionly, so theprinttagged-template API (print.error\…``) passes lint.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes