Skip to content

feat(cli): cross-device login via OAuth code + PKCE - #3965

Merged
saddlepaddle merged 4 commits into
mainfrom
superset-cli-cross-device
May 1, 2026
Merged

saddlepaddle merged 4 commits into
mainfrom
superset-cli-cross-device

Conversation

@saddlepaddle

@saddlepaddle saddlepaddle commented May 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Replaces the loopback-only /cli/authorize + /api/cli/exchange flow with Better Auth's first-class oauth-provider plugin (PKCE S256 + state + resource indicator)
  • CLI tries a loopback listener first (one-click happy path on local machines); falls back to a copy-paste page at /cli/auth/code (showing <code>#<state>) when loopback isn't reachable (SSH, CI, sandboxed envs) — same pattern as Claude Code's CLI
  • superset-cli OAuth client is seeded idempotently at API boot with both loopback ports (127.0.0.1:51789-93/callback) and the paste redirect URI
  • tRPC context now accepts the OAuth-issued JWT as a Bearer token via JWKS verification, mirroring the existing MCP route's auth path

Why now

superset auth login previously bound a local HTTP server and required the browser to redirect to it. That breaks for any CLI not running on the same machine as the browser — SSH hosts, dev containers, CI runners, cloud sandboxes — which is most of the realistic deploy surface for a developer CLI.

The OAuth code + PKCE + paste flow (rather than the OAuth device-grant alternative) was chosen for parity with Claude Code's UX: short user codes are phishable and the paste flow feels more polished to technical users.

Test plan

  • bun run lint passes
  • bun run typecheck passes (all 27 packages)
  • Loopback path: superset auth login on a local Mac binds 127.0.0.1:51789, browser auto-redirects, no paste needed
  • Paste path: SSH_CONNECTION=1 superset auth login (or run from an SSH'd host) prints the URL → browser → consent → copy code from /cli/auth/code → paste back into CLI
  • Multi-org consent picker appears for users with 2+ orgs (covered by existing consent page)
  • Token works: superset auth whoami and any tRPC-backed command succeed with the returned bearer
  • Cross-origin dev: 127.0.0.1:5760 no longer blocks HMR (added allowedDevOrigins)

Summary by cubic

Adds cross-device CLI login using OAuth 2.0 Authorization Code + PKCE via better-auth/oauth2, replacing the loopback-only flow. The API now accepts Bearer JWTs, and a copy-paste page enables SSH/CI login.

  • New Features

    • CLI login uses loopback (127.0.0.1 ports 51789–93) or copy-paste at /cli/auth/code (<code>#<state>), with an in-terminal paste prompt; uses SUPERSET_WEB_URL for the paste redirect and drops the offline_access scope.
    • OAuth uses PKCE S256, state, and resource, exchanging via /api/auth/oauth2/token; browser opens only when possible.
    • tRPC context verifies OAuth-issued JWTs via JWKS and accepts Bearer tokens; removed /cli/authorize and /api/cli/{create-code,exchange}; added public /cli/auth/code.
  • Migration

    • Deploy API/Web first, then update the superset CLI (old endpoints are removed).
    • Ensure a superset-cli OAuth client exists with PKCE, the paste redirect, and loopback ports; runtime seeding was removed.

Written for commit 7afd198. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • CLI authentication now uses standard OAuth2 with PKCE support for improved security.
    • Added authorization code paste flow as a fallback when browser-based login is unavailable.
  • Chores

    • Removed legacy CLI authorization endpoints and updated authentication workflow.
    • Improved CLI login prompts with better error handling and user guidance.
    • Added support for SUPERSET_WEB_URL environment configuration.

Replaces the loopback-only `cli/authorize`+`/api/cli/exchange` flow
with Better Auth's `oauth-provider`. The CLI tries a loopback
listener first (one-click happy path on local machines); when that
isn't reachable (SSH, CI, sandboxed envs), it falls back to a
copy-paste page at `/cli/auth/code` showing `<code>#<state>` —
matching how Claude Code's CLI handles cross-device.

PKCE (S256) + state binding + `resource` indicator so Better Auth
issues a JWT, which tRPC's context now accepts as Bearer via JWKS
verification (mirrors the existing MCP route's auth path).

The `superset-cli` OAuth client is seeded idempotently at API boot
with both loopback ports (51789–93) and the paste redirect URI.
@coderabbitai

coderabbitai Bot commented May 1, 2026 •

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The pull request removes deprecated CLI authentication endpoints and introduces a new OAuth2 PKCE-based authorization flow. The old flow via /api/cli/create-code and /api/cli/exchange is deleted. Session creation now supports JWT bearer token validation. The CLI login command is updated to implement PKCE, support loopback and paste modes, and integrate with the new authorization flow.

Changes

Cohort / File(s) Summary
Removed Legacy CLI Endpoints
apps/api/src/app/api/cli/create-code/route.ts, apps/api/src/app/api/cli/cli/exchange/route.ts
Deleted POST handlers that issued temporary CLI codes and exchanged them for session tokens. Removes authentication logic, Redis persistence, and code generation.
New CLI Auth Display
apps/web/src/app/cli/auth/code/page.tsx, apps/web/src/app/cli/auth/code/components/CliAuthCodeDisplay/CliAuthCodeDisplay.tsx, apps/web/src/app/cli/auth/code/components/CliAuthCodeDisplay/index.ts
Adds new OAuth callback handler page and reusable component for displaying auth code with clipboard copy functionality. Handles query parameters (code, state, error) from OAuth flow.
Removed Old CLI Authorize UI
apps/web/src/app/cli/authorize/page.tsx, apps/web/src/app/cli/authorize/components/CliAuthorizeForm/CliAuthorizeForm.tsx, apps/web/src/app/cli/authorize/components/CliAuthorizeForm/index.ts
Deletes deprecated authorization page and form component that previously orchestrated code creation and organization selection flow.
Session & Auth Infrastructure
apps/api/src/trpc/context.ts, packages/cli/src/lib/auth.ts, packages/cli/src/lib/env.ts
Adds OAuth bearer token fallback in session creation with JWT validation via JWKS. Updates auth library to implement PKCE flow, OAuth2 token exchange, and loopback/paste modes. Adds SUPERSET_WEB_URL environment variable.
CLI Login Command
packages/cli/src/commands/auth/login/command.ts
Enhances login flow with PKCE support, conditional spinner management for callback waiting, and user prompts for pasting authorization codes in non-loopback scenarios.
Routing & Configuration
apps/web/src/proxy.ts, apps/web/next.config.ts
Marks /cli/auth/code as public route to bypass authentication. Adds development origin allowlist configuration.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 The CLI hops down the OAuth path,
With PKCE shields from aftermath,
Old endpoints fade, new tokens bloom,
JWT bearers light the room,
Paste or loop—authentication blooms! 🌸

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(cli): cross-device login via OAuth code + PKCE' accurately captures the main objective—replacing loopback-only auth with OAuth Authorization Code + PKCE supporting both loopback and cross-device paste flows.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering summary, motivation, test plan, and implementation details with clear sections.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch superset-cli-cross-device

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented May 1, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

Replaces the bespoke loopback-only /cli/authorize + Redis exchange flow with a standards-based OAuth 2.0 Authorization Code + PKCE flow backed by Better Auth's oauth-provider plugin, adding a copy-paste fallback page (/cli/auth/code) for SSH/CI environments. The tRPC context is extended to accept the resulting JWT as a Bearer token via JWKS verification.

Two functional gaps need attention before this lands:

  • JWT lacks org context: sessionFromOAuthBearer reads payload.organizationId to populate activeOrganizationId, but Better Auth's oauth-provider does not emit a custom organizationId claim by default. Every CLI-issued JWT will have activeOrganizationId: null, silently breaking any tRPC procedure that scopes data to the active organization.
  • Refresh token discarded: offline_access is requested and the exchange returns a refresh_token, but command.ts never writes it to config.auth, meaning users must re-authenticate once the access token expires.

Confidence Score: 3/5

Not ready to merge — two P1 issues cause silently broken org-scoped tRPC calls and wasted refresh tokens for all CLI sessions.

Two independent P1 findings on the core auth path: missing org claim in JWT sessions and discarded refresh token. Either alone would cap the score at 4, but both together on the primary feature path warrants 3.

apps/api/src/trpc/context.ts (organizationId claim), packages/cli/src/commands/auth/login/command.ts (refresh token storage)

Important Files Changed

Filename Overview
apps/api/src/trpc/context.ts Adds JWT bearer auth path to tRPC context; P1: activeOrganizationId silently null if JWT lacks custom organizationId claim, breaking all org-scoped operations for CLI sessions
packages/cli/src/commands/auth/login/command.ts Wires loopback/paste callbacks to CLI UX; P1: refresh token from LoginResult is not persisted to config, wasting the offline_access scope
packages/cli/src/lib/auth.ts Core OAuth PKCE + loopback/paste dual-path login flow; P2: paste redirect URI derived via heuristic that may differ from server-seeded value
packages/auth/src/lib/seed-cli-oauth-client.ts Idempotent OAuth client seed (PKCE-only, public client); overwrites config on every boot which may clobber manual admin changes
apps/web/src/app/cli/auth/code/page.tsx New paste-flow redirect page; renders code+state from query params with proper error handling, no issues
apps/web/src/app/cli/auth/code/components/CliAuthCodeDisplay/CliAuthCodeDisplay.tsx Copy-to-clipboard component for auth code display; uses deprecated execCommand fallback (P2), primary clipboard API usage is correct
apps/api/src/instrumentation.ts Calls seedCliOAuthClient at API boot with error swallowing; straightforward and safe
apps/web/src/proxy.ts Adds /cli/auth/code to public routes — correct, this page must be accessible without a session
apps/web/next.config.ts Adds allowedDevOrigins for 127.0.0.1 to unblock HMR from loopback callback server

Sequence Diagram

sequenceDiagram
    participant CLI
    participant Browser
    participant API as API (Better Auth oauth-provider)
    participant WebApp as Web App (/cli/auth/code)

    CLI->>CLI: generate codeVerifier, codeChallenge (S256), state
    alt Loopback path (local machine)
        CLI->>CLI: bindLoopbackServer() on 127.0.0.1:5178x
        CLI->>Browser: open authorize URL with redirect_uri=127.0.0.1:5178x/callback
        Browser->>API: GET /api/auth/oauth2/authorize (PKCE + state)
        API->>Browser: consent page
        Browser->>API: user approves
        API->>CLI: GET /callback?code=...&state=...
        CLI->>CLI: verify state
        CLI->>API: POST /api/auth/oauth2/token (code + code_verifier)
        API->>CLI: access_token + refresh_token
    else Paste path (SSH / CI / no loopback)
        CLI->>Browser: open authorize URL with redirect_uri=/cli/auth/code
        Browser->>API: GET /api/auth/oauth2/authorize (PKCE + state)
        API->>Browser: consent page
        Browser->>API: user approves
        API->>WebApp: redirect /cli/auth/code?code=...&state=...
        WebApp->>Browser: display code#state for copy
        Browser->>CLI: user pastes code#state
        CLI->>CLI: verify state
        CLI->>API: POST /api/auth/oauth2/token (code + code_verifier)
        API->>CLI: access_token + refresh_token
    end
    CLI->>API: tRPC calls with Bearer access_token (JWT)
    API->>API: JWKS verify then sessionFromOAuthBearer then activeOrganizationId=null
Loading

Comments Outside Diff (1)

  1. apps/web/src/app/cli/auth/code/components/CliAuthCodeDisplay/CliAuthCodeDisplay.tsx, line 267-268 (link)

    P2 document.execCommand('copy') is deprecated

    The execCommand("copy") API is deprecated and may be removed from future browsers. Since this is used as a fallback for when navigator.clipboard is unavailable, it's acceptable for now, but you could simplify the fallback to just return false — modern browsers that support navigator.clipboard will handle 99%+ of cases, and the code/state text is always selectable for manual copy.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/web/src/app/cli/auth/code/components/CliAuthCodeDisplay/CliAuthCodeDisplay.tsx
    Line: 267-268
    
    Comment:
    **`document.execCommand('copy')` is deprecated**
    
    The `execCommand("copy")` API is deprecated and may be removed from future browsers. Since this is used as a fallback for when `navigator.clipboard` is unavailable, it's acceptable for now, but you could simplify the fallback to just `return false` — modern browsers that support `navigator.clipboard` will handle 99%+ of cases, and the code/state text is always selectable for manual copy.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
apps/api/src/trpc/context.ts:45-46
**`organizationId` claim may never be present — all org-scoped tRPC calls will see `null`**

Better Auth's oauth-provider issues standard JWT claims (`sub`, `iat`, `exp`, `iss`, `aud`). The custom `organizationId` claim is not a standard claim and is not emitted unless the oauth-provider plugin is explicitly configured to include it. As a result `activeOrganizationId` will almost always be `null` for CLI bearer sessions, and any tRPC procedure that reads the active org from the session context will silently behave as if no org is selected — even after the user chose one during login.

The `config.organizationId` written in `command.ts` is never forwarded back to the API in the JWT; you'd need either (a) a custom JWT "access-token claims" hook in your Better Auth config that embeds `organizationId`, or (b) the CLI passing the org explicitly on each request (e.g. a tRPC header), or (c) a separate token exchange step. Without one of these, `superset auth whoami` and every org-scoped command will return wrong/empty data.

### Issue 2 of 4
packages/cli/src/commands/auth/login/command.ts:56-59
**Refresh token silently discarded — re-login required on expiry**

`offline_access` is included in the OAuth scope, so the token endpoint returns a refresh token and `LoginResult.refreshToken` carries it. However the assignment at lines 56-59 only writes `accessToken` and `expiresAt` to config — the refresh token is never persisted. Once the access token expires, users must re-run `superset auth login` from scratch, entirely defeating the purpose of requesting `offline_access`.

The `refreshToken` field needs to be added to the config write here, added to the config type, and a silent-refresh path needs to consume it before the token expires.

### Issue 3 of 4
packages/cli/src/lib/auth.ts:53-55
**`getWebUrl()` string-replace heuristic can produce a mismatched redirect URI**

`getWebUrl()` derives the web URL by replacing `api.superset.sh` with `app.superset.sh` in `SUPERSET_API_URL`. The OAuth client is seeded with `NEXT_PUBLIC_WEB_URL` as the paste redirect URI. If a user sets `SUPERSET_WEB_URL` (or the heuristic produces a different value), the `redirect_uri` sent during authorization and token-exchange won't match the registered URI and the exchange will fail with a 400.

Consider a `SUPERSET_WEB_URL` default pre-built into the CLI at publish time (matching `NEXT_PUBLIC_WEB_URL`), or deriving it from a `/.well-known/oauth-authorization-server` metadata document to guarantee parity.

### Issue 4 of 4
apps/web/src/app/cli/auth/code/components/CliAuthCodeDisplay/CliAuthCodeDisplay.tsx:267-268
**`document.execCommand('copy')` is deprecated**

The `execCommand("copy")` API is deprecated and may be removed from future browsers. Since this is used as a fallback for when `navigator.clipboard` is unavailable, it's acceptable for now, but you could simplify the fallback to just `return false` — modern browsers that support `navigator.clipboard` will handle 99%+ of cases, and the code/state text is always selectable for manual copy.

Reviews (1): Last reviewed commit: "feat(cli): cross-device login via OAuth ..." | Re-trigger Greptile

Comment on lines +45 to +46
const activeOrganizationId =
typeof payload.organizationId === "string" ? payload.organizationId : null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 organizationId claim may never be present — all org-scoped tRPC calls will see null

Better Auth's oauth-provider issues standard JWT claims (sub, iat, exp, iss, aud). The custom organizationId claim is not a standard claim and is not emitted unless the oauth-provider plugin is explicitly configured to include it. As a result activeOrganizationId will almost always be null for CLI bearer sessions, and any tRPC procedure that reads the active org from the session context will silently behave as if no org is selected — even after the user chose one during login.

The config.organizationId written in command.ts is never forwarded back to the API in the JWT; you'd need either (a) a custom JWT "access-token claims" hook in your Better Auth config that embeds organizationId, or (b) the CLI passing the org explicitly on each request (e.g. a tRPC header), or (c) a separate token exchange step. Without one of these, superset auth whoami and every org-scoped command will return wrong/empty data.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/api/src/trpc/context.ts
Line: 45-46

Comment:
**`organizationId` claim may never be present — all org-scoped tRPC calls will see `null`**

Better Auth's oauth-provider issues standard JWT claims (`sub`, `iat`, `exp`, `iss`, `aud`). The custom `organizationId` claim is not a standard claim and is not emitted unless the oauth-provider plugin is explicitly configured to include it. As a result `activeOrganizationId` will almost always be `null` for CLI bearer sessions, and any tRPC procedure that reads the active org from the session context will silently behave as if no org is selected — even after the user chose one during login.

The `config.organizationId` written in `command.ts` is never forwarded back to the API in the JWT; you'd need either (a) a custom JWT "access-token claims" hook in your Better Auth config that embeds `organizationId`, or (b) the CLI passing the org explicitly on each request (e.g. a tRPC header), or (c) a separate token exchange step. Without one of these, `superset auth whoami` and every org-scoped command will return wrong/empty data.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 56 to 59
config.auth = {
accessToken: result.accessToken,
expiresAt: result.expiresAt,
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Refresh token silently discarded — re-login required on expiry

offline_access is included in the OAuth scope, so the token endpoint returns a refresh token and LoginResult.refreshToken carries it. However the assignment at lines 56-59 only writes accessToken and expiresAt to config — the refresh token is never persisted. Once the access token expires, users must re-run superset auth login from scratch, entirely defeating the purpose of requesting offline_access.

The refreshToken field needs to be added to the config write here, added to the config type, and a silent-refresh path needs to consume it before the token expires.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/commands/auth/login/command.ts
Line: 56-59

Comment:
**Refresh token silently discarded — re-login required on expiry**

`offline_access` is included in the OAuth scope, so the token endpoint returns a refresh token and `LoginResult.refreshToken` carries it. However the assignment at lines 56-59 only writes `accessToken` and `expiresAt` to config — the refresh token is never persisted. Once the access token expires, users must re-run `superset auth login` from scratch, entirely defeating the purpose of requesting `offline_access`.

The `refreshToken` field needs to be added to the config write here, added to the config type, and a silent-refresh path needs to consume it before the token expires.

How can I resolve this? If you propose a fix, please make it concise.

Comment thread packages/cli/src/lib/auth.ts Outdated
Comment on lines +53 to +55
export function getWebUrl(): string {
if (process.env.SUPERSET_WEB_URL) return process.env.SUPERSET_WEB_URL;
return env.SUPERSET_API_URL.replace("api.superset.sh", "app.superset.sh");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 getWebUrl() string-replace heuristic can produce a mismatched redirect URI

getWebUrl() derives the web URL by replacing api.superset.sh with app.superset.sh in SUPERSET_API_URL. The OAuth client is seeded with NEXT_PUBLIC_WEB_URL as the paste redirect URI. If a user sets SUPERSET_WEB_URL (or the heuristic produces a different value), the redirect_uri sent during authorization and token-exchange won't match the registered URI and the exchange will fail with a 400.

Consider a SUPERSET_WEB_URL default pre-built into the CLI at publish time (matching NEXT_PUBLIC_WEB_URL), or deriving it from a /.well-known/oauth-authorization-server metadata document to guarantee parity.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/lib/auth.ts
Line: 53-55

Comment:
**`getWebUrl()` string-replace heuristic can produce a mismatched redirect URI**

`getWebUrl()` derives the web URL by replacing `api.superset.sh` with `app.superset.sh` in `SUPERSET_API_URL`. The OAuth client is seeded with `NEXT_PUBLIC_WEB_URL` as the paste redirect URI. If a user sets `SUPERSET_WEB_URL` (or the heuristic produces a different value), the `redirect_uri` sent during authorization and token-exchange won't match the registered URI and the exchange will fail with a 400.

Consider a `SUPERSET_WEB_URL` default pre-built into the CLI at publish time (matching `NEXT_PUBLIC_WEB_URL`), or deriving it from a `/.well-known/oauth-authorization-server` metadata document to guarantee parity.

How can I resolve this? If you propose a fix, please make it concise.

@github-actions

github-actions Bot commented May 1, 2026 •

Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch

Thank you for your contribution! 🎉

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/src/proxy.ts (1)

11-15: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid prefix-matching the new public callback route.

startsWith("/cli/auth/code") also exempts paths like /cli/auth/code-anything, so this change widens the unauthenticated surface beyond the single callback page. Match on equality or a slash boundary instead.

Suggested fix
 function isPublicRoute(pathname: string): boolean {
-	return publicRoutes.some((route) => pathname.startsWith(route));
+	return publicRoutes.some(
+		(route) => pathname === route || pathname.startsWith(`${route}/`),
+	);
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/proxy.ts` around lines 11 - 15, The publicRoutes prefix check is
too permissive: update isPublicRoute to avoid matching paths like
"/cli/auth/code-anything" for the callback route by changing the match from a
simple startsWith to either exact equality or a slash-boundary check;
specifically, in the isPublicRoute function (and where publicRoutes is defined)
replace the predicate with something like pathname === route ||
pathname.startsWith(route + "/") so callback entries such as "/cli/auth/code"
only match themselves or subpaths under that exact segment.
🧹 Nitpick comments (1)
packages/cli/src/lib/auth.ts (1)

239-251: ⚡ Quick win

Validate token response shape before use.

The type assertion on the JSON response doesn't provide runtime guarantees. If the server returns an unexpected shape (e.g., error response with different structure), data.access_token could be undefined, resulting in an invalid token being stored and used for subsequent API calls.

♻️ Suggested validation
 	const data = (await response.json()) as {
 		access_token: string;
 		token_type: string;
 		expires_in?: number;
 		refresh_token?: string;
 	};

+	if (!data.access_token || typeof data.access_token !== "string") {
+		throw new CLIError(
+			"Invalid token response",
+			"Server returned an unexpected response. Run `superset auth login` again.",
+		);
+	}
+
 	const expiresIn = data.expires_in ?? 60 * 60 * 24 * 30;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/lib/auth.ts` around lines 239 - 251, The JSON response is
being blindly asserted into the expected shape which can yield undefined tokens;
modify the code around the response.json() call to perform runtime validation on
the parsed object (check that data is an object and data.access_token is a
non-empty string, and validate optional fields like data.expires_in and
data.refresh_token if present), and if validation fails throw or return a clear
error instead of returning an invalid token; update the return logic that builds
accessToken, expiresAt, and refreshToken to only use validated fields (e.g., use
a default expiresIn only after confirming its numericity) so you never store or
return undefined accessToken or malformed expiresAt from the current function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/api/src/instrumentation.ts`:
- Around line 6-13: The dynamic import of "@superset/auth/seed-cli-oauth-client"
is outside the try/catch so import-time failures still abort startup; move the
await import(...) inside the existing try block (or wrap it with its own try) so
both the module resolution/evaluation and the call to seedCliOAuthClient() are
caught; update the code paths around seedCliOAuthClient (the imported symbol) in
the register() initialization so any import or execution error is logged and
does not cause register() to throw.

In
`@apps/web/src/app/cli/auth/code/components/CliAuthCodeDisplay/CliAuthCodeDisplay.tsx`:
- Around line 39-46: The useCopiedFlag hook sets a timeout in flash but never
clears it, which can cause setState on an unmounted component; update
useCopiedFlag to store the timeout id (e.g. in a ref) and clearTimeout on
unmount (using useEffect cleanup) and also clear any previous timer when flash
runs again to avoid overlapping timers; reference the useCopiedFlag function and
the flash/setTimeout call when making these changes.

In `@packages/auth/src/lib/seed-cli-oauth-client.ts`:
- Around line 35-48: The seed logic using db.query.oauthClients.findFirst +
conditional insert can race on concurrent startups; replace it with a single
atomic upsert or a transaction with a unique constraint on clientId so two
processes cannot both insert. Concretely, change the flow around
db.query.oauthClients.findFirst / db.update(oauthClients).set /
db.insert(oauthClients).values to use the ORM's
upsert/insert...onConflictDoUpdate (or db.insert(...).onConflict(...).merge)
keyed on oauthClients.clientId (CLIENT_ID) so you perform an atomic
insert-or-update by clientId, or alternatively wrap the find/insert/update in a
serializable transaction that enforces the unique key on clientId. Ensure the
final operation updates the same fields currently in config when a conflict
occurs.

---

Outside diff comments:
In `@apps/web/src/proxy.ts`:
- Around line 11-15: The publicRoutes prefix check is too permissive: update
isPublicRoute to avoid matching paths like "/cli/auth/code-anything" for the
callback route by changing the match from a simple startsWith to either exact
equality or a slash-boundary check; specifically, in the isPublicRoute function
(and where publicRoutes is defined) replace the predicate with something like
pathname === route || pathname.startsWith(route + "/") so callback entries such
as "/cli/auth/code" only match themselves or subpaths under that exact segment.

---

Nitpick comments:
In `@packages/cli/src/lib/auth.ts`:
- Around line 239-251: The JSON response is being blindly asserted into the
expected shape which can yield undefined tokens; modify the code around the
response.json() call to perform runtime validation on the parsed object (check
that data is an object and data.access_token is a non-empty string, and validate
optional fields like data.expires_in and data.refresh_token if present), and if
validation fails throw or return a clear error instead of returning an invalid
token; update the return logic that builds accessToken, expiresAt, and
refreshToken to only use validated fields (e.g., use a default expiresIn only
after confirming its numericity) so you never store or return undefined
accessToken or malformed expiresAt from the current function.
🪄 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: ff54795e-7c77-4785-ae17-50a127da3ec4

📥 Commits

Reviewing files that changed from the base of the PR and between b7a2558 and deee0a1.

📒 Files selected for processing (16)
  • apps/api/src/app/api/cli/create-code/route.ts
  • apps/api/src/app/api/cli/exchange/route.ts
  • apps/api/src/instrumentation.ts
  • apps/api/src/trpc/context.ts
  • apps/web/next.config.ts
  • apps/web/src/app/cli/auth/code/components/CliAuthCodeDisplay/CliAuthCodeDisplay.tsx
  • apps/web/src/app/cli/auth/code/components/CliAuthCodeDisplay/index.ts
  • apps/web/src/app/cli/auth/code/page.tsx
  • apps/web/src/app/cli/authorize/components/CliAuthorizeForm/CliAuthorizeForm.tsx
  • apps/web/src/app/cli/authorize/components/CliAuthorizeForm/index.ts
  • apps/web/src/app/cli/authorize/page.tsx
  • apps/web/src/proxy.ts
  • packages/auth/package.json
  • packages/auth/src/lib/seed-cli-oauth-client.ts
  • packages/cli/src/commands/auth/login/command.ts
  • packages/cli/src/lib/auth.ts
💤 Files with no reviewable changes (5)
  • apps/web/src/app/cli/authorize/components/CliAuthorizeForm/index.ts
  • apps/api/src/app/api/cli/exchange/route.ts
  • apps/web/src/app/cli/authorize/page.tsx
  • apps/api/src/app/api/cli/create-code/route.ts
  • apps/web/src/app/cli/authorize/components/CliAuthorizeForm/CliAuthorizeForm.tsx

Comment thread apps/api/src/instrumentation.ts Outdated
Comment on lines +6 to +13
const { seedCliOAuthClient } = await import(
"@superset/auth/seed-cli-oauth-client"
);
try {
await seedCliOAuthClient();
} catch (error) {
console.error("[instrumentation] Failed to seed CLI OAuth client", error);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Wrap the dynamic import in the same failure path.

If @superset/auth/seed-cli-oauth-client fails to resolve or throws during module evaluation, register() still aborts because the try only covers seedCliOAuthClient(). That turns this best-effort seed into a startup failure.

Suggested fix
-		const { seedCliOAuthClient } = await import(
-			"@superset/auth/seed-cli-oauth-client"
-		);
 		try {
+			const { seedCliOAuthClient } = await import(
+				"@superset/auth/seed-cli-oauth-client"
+			);
 			await seedCliOAuthClient();
 		} catch (error) {
 			console.error("[instrumentation] Failed to seed CLI OAuth client", error);
 		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { seedCliOAuthClient } = await import(
"@superset/auth/seed-cli-oauth-client"
);
try {
await seedCliOAuthClient();
} catch (error) {
console.error("[instrumentation] Failed to seed CLI OAuth client", error);
}
try {
const { seedCliOAuthClient } = await import(
"@superset/auth/seed-cli-oauth-client"
);
await seedCliOAuthClient();
} catch (error) {
console.error("[instrumentation] Failed to seed CLI OAuth client", error);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/api/src/instrumentation.ts` around lines 6 - 13, The dynamic import of
"@superset/auth/seed-cli-oauth-client" is outside the try/catch so import-time
failures still abort startup; move the await import(...) inside the existing try
block (or wrap it with its own try) so both the module resolution/evaluation and
the call to seedCliOAuthClient() are caught; update the code paths around
seedCliOAuthClient (the imported symbol) in the register() initialization so any
import or execution error is logged and does not cause register() to throw.

The superset-cli OAuth client is one-time bootstrap data, not
something that should re-run on every API boot. The prod row was
applied manually with the matching shape (paste URI + 5 loopback
ports + require_pkce: true), so we no longer need the seed in code.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 16 files

- bake SUPERSET_WEB_URL into env.ts (already defined by cli.config.ts at
  build time) and drop the host-substitution heuristic in getWebUrl()
- drop offline_access scope: refresh tokens were being requested but
  silently discarded; re-add the scope alongside actual refresh logic
- clear the copied-flag timer on unmount in CliAuthCodeDisplay
@saddlepaddle
saddlepaddle merged commit b30c859 into main May 1, 2026
15 checks passed
saddlepaddle added a commit that referenced this pull request May 2, 2026
Bundles the 5 feat(cli) commits since v0.2.2:
- automations + tasks alignment with SDK and MCP-v2 (#3966)
- cross-device login via OAuth code + PKCE (#3965)
- 'automations prompt' split into 'prompt get' / 'prompt set' (#3959)
- --name filter on automations list (#3952)
- --assignee filter on tasks list (#3972)

Push cli-v0.2.3 after this lands to fire the release pipeline.
@Kitenite
Kitenite deleted the superset-cli-cross-device branch May 6, 2026 04:51

This branch was previously deployed

1 inactive deployment
preview — 7afd198a Deployed May 1, 2026 by saddlepaddle via Deploy Admin #4711
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant