stack-cli: support self-hosted URLs and tighten CLI auth polling#1419
stack-cli: support self-hosted URLs and tighten CLI auth polling#1419mantrakp04 merged 5 commits intodevfrom
Conversation
- Read STACK_API_URL / STACK_DASHBOARD_URL from env in stack-cli so the published CLI can talk to self-hosted Stack Auth installs without a custom build. Surface the vars (and the existing STACK_CLI_PUBLISHABLE_CLIENT_KEY) in docker/server/.env.example. - Reduce the CLI auth polling-code TTL: default 2h -> 2min, max 24h -> 15min. The code is only used while a user is actively waiting in `stack login`, so a tight window limits the value of a leak. - Convert the CLI auth poll handler to raw SQL against the tenancy source-of-truth schema, matching the pattern already used by the initiate handler. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughMoves CLI auth polling from Prisma model methods to schema-qualified raw SQL, changes CLI request timing defaults to 2 minutes (max 15 minutes), refines anon refresh token presence and expiry checks, and replaces the ORM update with an atomic SQL UPDATE that returns the refresh token. ChangesCLI Auth Endpoints Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
… hours in route.tsx
There was a problem hiding this comment.
Pull request overview
Adds self-hosting support for the published stack-cli by allowing API/Dashboard URL overrides via environment variables, updates the Docker env example to surface those knobs, and tightens CLI-auth polling code lifetimes while aligning the poll handler with the tenancy source-of-truth schema via raw SQL.
Changes:
- Allow
stack-clito readSTACK_API_URL/STACK_DASHBOARD_URLfrom environment variables. - Document CLI-specific env vars in
docker/server/.env.example. - Reduce CLI auth polling code TTL defaults/max and switch the poll handler to tenancy-schema raw SQL access.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/stack-cli/src/lib/auth.ts | Uses env overrides for default API/Dashboard URLs to support self-hosted backends. |
| docker/server/.env.example | Documents CLI-relevant env vars for self-hosted/docker operators. |
| apps/backend/src/app/api/latest/auth/cli/route.tsx | Tightens expires_in_millis default/max for CLI auth initiation. |
| apps/backend/src/app/api/latest/auth/cli/poll/route.tsx | Uses raw SQL against tenancy schema for polling and marks attempts as used. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR adds anonymous refresh token support to the CLI auth initiation endpoint, converts the poll handler from Prisma ORM to raw SQL (matching the initiate handler's pattern), and tightens CLI auth polling-code TTL defaults. The poll handler also addresses a previous TOCTOU concern by using an atomic
Confidence Score: 4/5Safe to merge with the max-TTL caveat resolved — the atomic claim fix and anon-token handling are correct, but the 24-hour max expiry contradicts the stated security hardening. The poll handler's atomic UPDATE correctly closes the concurrent-claim window, and the anon refresh token validation in the initiate handler is well-structured. However, the initiate handler's yupNumber().max() constraint was not reduced from 24 hours to the 15 minutes stated in the PR — a client can still request a day-long polling code, leaving the blast-radius argument incomplete. apps/backend/src/app/api/latest/auth/cli/route.tsx — the max TTL guard needs to be updated to match the stated intent. Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as CLI Client
participant Init as POST /auth/cli (initiate)
participant Poll as POST /auth/cli/poll
participant DB as Tenancy DB
CLI->>Init: "POST {expires_in_millis, anon_refresh_token?}"
Init->>DB: Validate anon_refresh_token (global DB)
Init->>DB: INSERT CliAuthAttempt (pollingCode, loginCode, expiresAt)
Init-->>CLI: "{polling_code, login_code, expires_at}"
loop Poll until success/expired/used
CLI->>Poll: "POST {polling_code}"
Poll->>DB: SELECT id, refreshToken, expiresAt, usedAt
alt expired
Poll-->>CLI: "{status: expired}"
else "usedAt != null"
Poll-->>CLI: "{status: used}"
else "refreshToken == null"
Poll-->>CLI: "{status: waiting}"
else refreshToken set
Poll->>DB: "UPDATE SET usedAt=NOW() WHERE usedAt IS NULL RETURNING refreshToken"
alt claimed atomic
Poll-->>CLI: "201 {status: success, refresh_token}"
else already claimed by concurrent request
Poll-->>CLI: "{status: used}"
end
end
end
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/backend/src/app/api/latest/auth/cli/route.tsx:28
**Max TTL not reduced — contradicts PR's security claim**
The PR description says the max expiry is being tightened from 24 h to 15 min, but the `yupNumber().max(...)` guard is unchanged at `1000 * 60 * 60 * 24` (24 hours). A client can still explicitly pass `expires_in_millis=86400000` and obtain a polling code valid for a full day, which negates the "limit the blast radius of a leaked code" argument in the PR. The default is also 10 minutes here, not 2 minutes as stated in the PR description. Both the max and the default need to match the stated security intent if the hardening is the rationale for the change.
Reviews (2): Last reviewed commit: "Update default expiration time for CLI a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/backend/src/app/api/latest/auth/cli/poll/route.tsx (2)
66-88: ⚡ Quick winMake "mark as used" atomic to close a TOCTOU window on the polling code.
The SELECT at L51 and the UPDATE at L81 are independent statements with no row lock and no
usedAt IS NULLguard. Two concurrent successful polls of the samepolling_code(e.g., the legitimate CLI plus a leaked/mirrored request) can both pass theusedAt/refreshTokenchecks and both receive the refresh token, with the second update simply re-stampingusedAt. Since the file is now raw SQL, an atomicUPDATE … WHERE "usedAt" IS NULL RETURNING …plus a row-count check is a minimal, defense-in-depth fix and also lets you drop the in-memorycliAuth.usedAtbranch.🔒 Proposed atomic claim of the polling code
- if (cliAuth.usedAt) { - return createResponse('used'); - } - - if (!cliAuth.refreshToken) { + if (cliAuth.refreshToken == null) { return createResponse('waiting'); } // Mark as used - await prisma.$executeRaw(Prisma.sql` - UPDATE ${sqlQuoteIdent(schema)}."CliAuthAttempt" - SET - "usedAt" = NOW(), - "updatedAt" = NOW() - WHERE "tenancyId" = ${tenancy.id}::UUID - AND "id" = ${cliAuth.id}::UUID - `); - - return createResponse('success', cliAuth.refreshToken); + const claimed = await prisma.$queryRaw<{ refreshToken: string }[]>(Prisma.sql` + UPDATE ${sqlQuoteIdent(schema)}."CliAuthAttempt" + SET + "usedAt" = NOW(), + "updatedAt" = NOW() + WHERE "tenancyId" = ${tenancy.id}::UUID + AND "id" = ${cliAuth.id}::UUID + AND "usedAt" IS NULL + RETURNING "refreshToken" + `); + if (claimed.length === 0) { + return createResponse('used'); + } + return createResponse('success', claimed[0].refreshToken);🤖 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/backend/src/app/api/latest/auth/cli/poll/route.tsx` around lines 66 - 88, The polling code has a TOCTOU gap between selecting cliAuth and later marking it used; change the raw UPDATE executed in prisma.$executeRaw (the one that updates "CliAuthAttempt") to atomically set "usedAt" = NOW() only when "usedAt" IS NULL and RETURNING the refreshToken (i.e. add WHERE "usedAt" IS NULL and a RETURNING clause), then check the returned row count/result to decide whether to return the refresh token or a 'used' response; remove the separate in-memory cliAuth.usedAt branch and rely on the UPDATE RETURNING result (use schema, tenancy.id, cliAuth.id and sqlQuoteIdent as in the current query to locate the statement).
72-78: 💤 Low valueUse explicit null checks for
usedAt/refreshToken.
usedAtisDate | nullandrefreshTokenisstring | nullperCliAuthAttemptRow; prefer explicit null comparisons over truthiness so that, e.g., empty-stringrefreshTokenwould still be treated as not-yet-set rather than as an invalid token. (If you adopt the atomic-claim refactor above, theusedAtbranch can be removed and only therefreshToken == nullcheck remains.)♻️ Proposed change
- if (cliAuth.usedAt) { + if (cliAuth.usedAt != null) { return createResponse('used'); } - if (!cliAuth.refreshToken) { + if (cliAuth.refreshToken == null) { return createResponse('waiting'); }As per coding guidelines: "Unless very clearly equivalent from types, prefer explicit null/undefinedness checks over boolean checks (e.g.,
foo == nullinstead of!foo)".🤖 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/backend/src/app/api/latest/auth/cli/poll/route.tsx` around lines 72 - 78, The truthy checks on cliAuth.usedAt and cliAuth.refreshToken should be replaced with explicit null checks: change the first branch to test if cliAuth.usedAt != null (or === null depending on desired behavior) and change the second branch to test cliAuth.refreshToken == null so an empty-string refreshToken is treated as "waiting"; update the branches that call createResponse('used') and createResponse('waiting') accordingly (or remove the usedAt branch if you adopt the atomic-claim refactor and only check refreshToken == null).apps/backend/src/app/api/latest/auth/cli/route.tsx (1)
44-62: 💤 Low valueUse explicit null/undefined checks instead of truthiness.
Per the coding guidelines, prefer
foo == null(or!= null) over boolean checks when the value isn't a boolean. Hereanon_refresh_tokenisstring | undefinedandrefreshTokenObj.expiresAtisDate | null, so the truthiness checks would also branch on the empty string / a future epoch-zero date even though those should not be treated specially.♻️ Proposed change
- if (anon_refresh_token) { + if (anon_refresh_token != null) { const refreshTokenRows = await globalPrismaClient.$queryRaw<RefreshTokenRow[]>(Prisma.sql` @@ - if (refreshTokenObj.expiresAt && refreshTokenObj.expiresAt < new Date()) { + if (refreshTokenObj.expiresAt != null && refreshTokenObj.expiresAt < new Date()) { throw new StatusError(400, "The provided anon refresh token has expired"); }As per coding guidelines: "Unless very clearly equivalent from types, prefer explicit null/undefinedness checks over boolean checks (e.g.,
foo == nullinstead of!foo)".🤖 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/backend/src/app/api/latest/auth/cli/route.tsx` around lines 44 - 62, Replace truthiness checks with explicit null/undefined checks: change the anon_refresh_token branch to test anon_refresh_token != null (since its type is string | undefined) and change the expiry check to test refreshTokenObj.expiresAt != null && refreshTokenObj.expiresAt < new Date() (since expiresAt is Date | null) so empty string or epoch-like dates are not misinterpreted; update the conditions around anon_refresh_token and refreshTokenObj.expiresAt in the route handler accordingly.
🤖 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.
Nitpick comments:
In `@apps/backend/src/app/api/latest/auth/cli/poll/route.tsx`:
- Around line 66-88: The polling code has a TOCTOU gap between selecting cliAuth
and later marking it used; change the raw UPDATE executed in prisma.$executeRaw
(the one that updates "CliAuthAttempt") to atomically set "usedAt" = NOW() only
when "usedAt" IS NULL and RETURNING the refreshToken (i.e. add WHERE "usedAt" IS
NULL and a RETURNING clause), then check the returned row count/result to decide
whether to return the refresh token or a 'used' response; remove the separate
in-memory cliAuth.usedAt branch and rely on the UPDATE RETURNING result (use
schema, tenancy.id, cliAuth.id and sqlQuoteIdent as in the current query to
locate the statement).
- Around line 72-78: The truthy checks on cliAuth.usedAt and
cliAuth.refreshToken should be replaced with explicit null checks: change the
first branch to test if cliAuth.usedAt != null (or === null depending on desired
behavior) and change the second branch to test cliAuth.refreshToken == null so
an empty-string refreshToken is treated as "waiting"; update the branches that
call createResponse('used') and createResponse('waiting') accordingly (or remove
the usedAt branch if you adopt the atomic-claim refactor and only check
refreshToken == null).
In `@apps/backend/src/app/api/latest/auth/cli/route.tsx`:
- Around line 44-62: Replace truthiness checks with explicit null/undefined
checks: change the anon_refresh_token branch to test anon_refresh_token != null
(since its type is string | undefined) and change the expiry check to test
refreshTokenObj.expiresAt != null && refreshTokenObj.expiresAt < new Date()
(since expiresAt is Date | null) so empty string or epoch-like dates are not
misinterpreted; update the conditions around anon_refresh_token and
refreshTokenObj.expiresAt in the route handler accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8bd27d59-5cc0-4402-bef9-1432804d4f42
📒 Files selected for processing (4)
apps/backend/src/app/api/latest/auth/cli/poll/route.tsxapps/backend/src/app/api/latest/auth/cli/route.tsxdocker/server/.env.examplepackages/stack-cli/src/lib/auth.ts
…tes and improve null checks in route handlers
|
@greptile-ai I've addressed all the review feedback:
Please take another look. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/backend/src/app/api/latest/auth/cli/route.tsx`:
- Line 28: The expires_in_millis schema currently allows up to 24 hours; tighten
the TTL by changing the yupNumber().max(...) cap from 1000 * 60 * 60 * 24 to a
short ceiling (e.g., 1000 * 60 * 15 for 15 minutes) while keeping the default at
1000 * 60 * 10 (10 minutes) so the expires_in_millis field enforces the intended
short polling window.
🪄 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: edad35e1-8dd4-45f2-8ac8-60353e408c4f
📒 Files selected for processing (2)
apps/backend/src/app/api/latest/auth/cli/poll/route.tsxapps/backend/src/app/api/latest/auth/cli/route.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/backend/src/app/api/latest/auth/cli/poll/route.tsx
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/backend/src/app/api/latest/auth/cli/route.tsx (1)
28-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore the intended default polling TTL to 10 minutes.
Line 28 currently defaults to 2 minutes, but this PR’s agreed behavior is 10 minutes (with 15-minute max). Keep the tighter max, but update the default to avoid premature CLI auth expiry.
Suggested patch
- expires_in_millis: yupNumber().max(1000 * 60 * 15).default(1000 * 60 * 2), // Default: 2 minutes, max: 15 minutes + expires_in_millis: yupNumber().max(1000 * 60 * 15).default(1000 * 60 * 10), // Default: 10 minutes, max: 15 minutes🤖 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/backend/src/app/api/latest/auth/cli/route.tsx` at line 28, The default polling TTL for CLI auth is set incorrectly; update the default value in the expires_in_millis schema (where expires_in_millis uses yupNumber().max(1000 * 60 * 15).default(...)) to 10 minutes by changing the default from 1000 * 60 * 2 to 1000 * 60 * 10 while keeping the existing max at 1000 * 60 * 15.
🤖 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.
Duplicate comments:
In `@apps/backend/src/app/api/latest/auth/cli/route.tsx`:
- Line 28: The default polling TTL for CLI auth is set incorrectly; update the
default value in the expires_in_millis schema (where expires_in_millis uses
yupNumber().max(1000 * 60 * 15).default(...)) to 10 minutes by changing the
default from 1000 * 60 * 2 to 1000 * 60 * 10 while keeping the existing max at
1000 * 60 * 15.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 939ac83e-d289-4e52-a37e-8ca4b9241f74
📒 Files selected for processing (1)
apps/backend/src/app/api/latest/auth/cli/route.tsx
Summary
STACK_API_URL/STACK_DASHBOARD_URLfrom env instack-cliso the published CLI can talk to self-hosted Stack Auth installs without a custom build. The existingSTACK_CLI_PUBLISHABLE_CLIENT_KEYoverride is kept as-is.docker/server/.env.exampleso self-host operators see them.2h -> 2min, max24h -> 15minfor the CLI auth polling code. The code is only valid while a user is actively waiting instack login, so a tight window limits the blast radius of a leaked code.apps/backend/src/app/api/latest/auth/cli/poll/route.tsxfromprisma.cliAuthAttempt.*to raw SQL targeted at the tenancy source-of-truth schema, matching the pattern already used by the initiate handler inapps/backend/src/app/api/latest/auth/cli/route.tsx.Test plan
pnpm typecheckpnpm lintpnpm test run(focus on CLI-auth tests if any)stack loginagainst a local backendwaiting/success/used/expiredbranches still return correct status codes and bodiesstack-cliagainst a self-hosted backend withSTACK_API_URL/STACK_DASHBOARD_URLset, end-to-end loginMade with Cursor
Summary by CodeRabbit
Improvements
Changes