Skip to content

fix(login): device-flow link re-anchored onto the environment console; prompt names the deployment (#134) - #135

Open
tonychang04 wants to merge 2 commits into
mainfrom
fix/device-login-staging-host
Open

fix(login): device-flow link re-anchored onto the environment console; prompt names the deployment (#134)#135
tonychang04 wants to merge 2 commits into
mainfrom
fix/device-login-staging-host

Conversation

@tonychang04

@tonychang04 tonychang04 commented Aug 24, 2026

Copy link
Copy Markdown
Member

Fixes #134.

Root cause

The CLI never builds the device-approval URL — the platform does, from its own INSTA_CONSOLE_URL (Better Auth deviceAuthorization({ verificationUri: ${cfg.consoleUrl}/device }) in insta-platform). The staging deployment's INSTA_CONSOLE_URL points at the staging landing host instead of the staging console, so staging mints approval links onto a host that serves no /device page.

Probe evidence (2026-08-24):

  • POST https://api.staging.instacloud.com/api/auth/device/code"verification_uri": "https://staging.instacloud.com/device" — the landing host.
  • POST https://api.instacloud.com/api/auth/device/code"verification_uri": "https://console.instacloud.com/device" — the console host.
  • https://console.instacloud.com/device?user_code=TEST307 /signin?next=%2Fdevice%3Fuser_code%3DTEST — the route exists on the prod console.
  • https://instacloud.com/device?user_code=TEST308 → www.instacloud.com404 — the landing site serves no /device anywhere, so staging's link can only 404 (behind Vercel SSO, exactly as reported).
  • The /device approval page lives in insta-frontend (src/app/device/page.tsx), which is present on its staging branch (deployed to console.staging.instacloud.com per that repo's README) — so the staging console does serve the route; the platform just points at the wrong host.

The lasting fix is one env var on the staging platform deployment: INSTA_CONSOLE_URL=https://console.staging.instacloud.com (currently the landing origin). This PR makes the CLI robust to that class of misconfiguration and closes the UX nit.

What this PR does

  1. Re-anchors the approval link onto the environment's console host (deviceVerificationUrl). Each env in ENVS now carries its console origin (probed above). For a known env, only the URL's origin is replaced — path and user_code query stay exactly what the server minted, so a correctly configured platform is a no-op (prod already is). Custom/self-hosted --api-url targets (env === null) are the user's deliberate choice and are never rewritten; an unparseable server string is shown as-is.
  2. Labels the deployment in the prompt (the issue's UX nit): to log in to STAGING, open this link… for named envs, to log in to https://insta.local:7130, … for custom hosts.

Tests

  • deviceVerificationUrl: wrong-host staging link re-anchored with user_code preserved; already-correct prod link untouched; custom host (env null) untouched; unparseable string passed through.
  • deviceGrant output: staging run prints to log in to STAGING + the console-staging link and never the 404 landing link; custom-host run prints the apiUrl label and the server's link verbatim.
  • env table: prod/staging console hosts pinned to the probed working values.

Ran npm run typecheck and the full vitest suite against this branch: the two touched files pass 63/63; the suite's only failures (5, in setup-agent/ensure-skills) are identical on pristine main on the same machine (ambient local agent config), i.e. pre-existing and unrelated.

Not in this PR

  • The staging platform's INSTA_CONSOLE_URL should still be corrected to https://console.staging.instacloud.com (deployment env, not in any repo I can PR). Until then, this CLI fix alone makes insta login --device --env staging print a working link.

🤖 Generated with Claude Code


Summary by cubic

Fixes device login by re-anchoring the device-approval link to the environment’s console and naming the target deployment in the prompt. Previously, staging links pointed at the landing host (no /device) and 404’d; now known environments rewrite only the link origin while preserving the server-provided path and user_code.

  • Known envs: origin is replaced with the env console host; prod is unchanged; custom/self-hosted targets are never rewritten.
  • Prompt now labels the target (“to log in to STAGING …” or the apiUrl for custom hosts) to reduce wrong-environment approvals.
  • Adds console hosts to the env table and introduces a small helper to compute the approval URL; tests cover rewriting, prompting, and env table.
  • No migration required. Recommended follow-up: set INSTA_CONSOLE_URL=https://console.staging.instacloud.com on the staging platform.

Written for commit 6c282fe. Summary will update on new commits.

Review in cubic

…; prompt names the deployment (#134)

The staging platform mints its RFC 8628 verification_uri from its own
INSTA_CONSOLE_URL, which points at the staging landing site - a host that
serves no /device page (the link passes Vercel SSO, then 404s). For a known
environment the CLI knows the console host itself, so re-anchor the link's
origin there (path + user_code stay the server's; a correctly configured
platform is a no-op, custom/self-hosted hosts are never rewritten). Also
name the deployment in the prompt ("to log in to STAGING") so a code can't
be approved against the wrong environment.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/commands/auth.ts Outdated

@jwfing jwfing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Summary
No blocking issues found; the device-flow URL rewrite and prompt labeling are scoped to the reported staging-login failure.

Requirements Context
I used the PR description, linked issue #134 (staging insta login --device --env staging prints a landing-host /device URL that 404s and should label STAGING), README environment/authentication docs, and the repo dev notes in .claude/skills/developing-insta-cli/SKILL.md. The documented intent is to keep prod/staging separated, preserve explicit custom INSTA_API_URL / --api-url targets, and improve the device-login prompt without changing command flags.

Findings

Critical

(none)

Suggestion

(none)

Information

  • src/commands/auth.ts:61-156, src/env.ts:36-47, test/device-login.test.ts:147-210: Software engineering/functionality review found the implementation aligned with the stated requirements. Known envs are inferred from the resolved API URL, only the approval URL origin is rewritten, and tests cover staging wrong-host rewriting, prod no-op behavior, custom-host pass-through, unparseable strings, prompt labels, and pinned console hosts.
  • src/commands/auth.ts:130-156: Security review found no new token/secret exposure or auth weakening. The code prints the existing device approval URL/user code flow and constrains known-env approval links to the pinned console origin.
  • src/commands/auth.ts:130-156: Performance review found no relevant risk; the added work is one URL parse/reconstruction during interactive login, with no new hot-path loops, blocking I/O, dependencies, or network calls.
  • package.json:35-37: Verification note: npm run typecheck could not run in this checkout because tsc is unavailable (sh: tsc: not found). I did not run Vitest because the review request was explicitly read-only and the suite creates temporary files.

Verdict
Approved per the requested rule: zero Critical findings. This is a bot-review approval verdict, not a GitHub green-check approval.

@jwfing jwfing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Summary

Tightly scoped, well-tested fix: the device-login approval link is re-anchored onto the environment's own console origin and the prompt now names the deployment — no Critical findings, three non-blocking Suggestions (one real test-coverage gap, mutation-proved below).

Requirements context

No matching spec/plan found — this repo has no /docs/superpowers/ (or any docs/) directory at head 31d3e8a. Assessed against issue #134 and the PR body.

Issue #134 asks for two things, and both are delivered:

  1. "The staging device-approval page must exist at whatever URL the CLI prints (either add the route to the staging frontend, or point the CLI at the host that serves it)" → this PR takes the second option.
  2. "the device-flow output should label the environment explicitly ('log in to STAGING')"src/commands/auth.ts:152-155.

I independently verified the PR's root-cause chain rather than taking the body's probe log at face value:

  • src/app/device/page.tsx exists on both main and staging of InsForge/insta-frontend (GitHub contents API, HTTP 200 on each ref) — so the route does ship to staging.
  • insta-frontend@staging .github/workflows/deploy-staging.yml:41 sets STAGING_HOST: console.staging.instacloud.com, and the workflow aliases the staging deployment to it. So the pinned ENVS.staging.console value is the host that actually serves the staging branch. (This host assignment landed in insta-frontend#237, which also deliberately reassigned staging.instacloud.com to the landing/marketing staging site — exactly the PR's diagnosis.)
  • Issue #134 explicitly left this unverified ("console.staging.instacloud.com/device is behind the same SSO (unverified whether the route exists there)"). The two checks above close that gap; the PR is pointing at the right host.

Both hosts are behind Vercel deployment-protection SSO, so /device cannot be probed anonymously on staging — the branch+workflow evidence is what settles it.

Findings

Critical

(none) — no correctness bug, no security hole, no requirement violation, no regression risk I could construct. npx tsc --noEmit clean and npx vitest run 490/490 across 38 files green in a fresh clone of 31d3e8a.


Suggestion

1. Functionality / test coverage — the production wiring is untested; the whole feature can regress silently

src/commands/auth.ts:61-65, test/device-login.test.ts:142-213

Both new behavioural tests call deviceGrant(post, wait, target) directly. Nothing exercises loginDevice's job of constructing that target from envForApiUrl(api.apiUrl). Because target?: DeviceTarget is optional (auth.ts:141), dropping it at the call site is not a type error either.

Mutation-proved in the workspace — I replaced the call site with the pre-PR form:

const token = await deviceGrant((path, body) => api.request('POST', path, body, { auth: false }))

Result: npx tsc --noEmit clean, npx vitest run 490/490 passing. Every user-visible effect of this PR is gone and the gate stays green.

Cheapest fix: make target required on deviceGrant (it is an internal export with exactly one production call site — grep confirms), so the compiler owns the wiring. Or add a loginDevice-level test with a fake ApiClient asserting the target handed to the injected grant.

For contrast, the behaviours that are covered are genuinely load-bearing — I mutation-tested them too:

  • deleting ${url.hash} from auth.ts:135 → 1 failure (the fragment test at test/device-login.test.ts:194-197 is real, not decorative);
  • short-circuiting deviceVerificationUrl to always return raw → 3 failures;
  • dropping the to ${where} label from auth.ts:155 → 2 failures.

2. Functionality — the rewrite silently papers over the platform misconfiguration it exists to survive

src/commands/auth.ts:130-136

When the rewrite fires, the user sees a working link and no one learns the platform is misconfigured. That is the whole point in the moment, but it also means INSTA_CONSOLE_URL can stay wrong indefinitely with zero signal — and the PR's own "Not in this PR" follow-up is the kind of item that only gets done if something keeps complaining.

This is not hypothetical: staging has already been corrected server-side. Live probe today (2026-08-25):

POST https://api.staging.instacloud.com/api/auth/device/code
  → "verification_uri_complete": "https://console.staging.instacloud.com/device?user_code=…"
POST https://api.instacloud.com/api/auth/device/code
  → "verification_uri_complete": "https://console.instacloud.com/device?user_code=…"

So the rewrite is a no-op on both environments as of now — which is the strongest possible argument for making the non-no-op case loud. Suggest a one-line stderr note when url.origin !== consoleOrigin, e.g. note: the staging platform minted an approval link on https://…; using the known console host instead.

The same argument covers the staleness direction: pinning console origins client-side means a published CLI will rewrite a legitimately-changed future console host back to the stale one. ENVS[*].console should join the release checklist alongside api/mcp.

3. Security — arbitrary server-supplied path/query/fragment is grafted onto the trusted origin

src/commands/auth.ts:135

return url.origin === consoleOrigin ? raw : `${consoleOrigin}${url.pathname}${url.search}${url.hash}`

Everything except the origin comes from the server, and it is then rendered under a host the user has every reason to trust. The console does carry a redirect parameter — probing https://console.instacloud.com/device?user_code=TEST returns 307 → /signin?next=%2Fdevice%3Fuser_code%3DTEST — so a platform minting …/signin?next=https://evil.example would have the CLI print a link on the genuine console.instacloud.com origin.

Non-blocking, and this is strictly better than the pre-PR behaviour (which printed the hostile origin verbatim): the precondition is a hostile or badly-templated auth server, which already owns the session anyway. But since the CLI now knows the console host, it costs nothing to also pin the path and carry only the code — nothing server-supplied would land on the trusted origin except user_code:

const u = new URL('/device', ENVS[env].console)
const code = url.searchParams.get('user_code')
if (code) u.searchParams.set('user_code', code)

Note this changes the "unknown env-null target" path not at all, and would need the verification_uri (non-complete) case to skip the query.

Everything else on the security dimension is clean: no new dependencies; no new user input reaching SQL/shell/HTTP; device_code is still never printed (only user_code, which RFC 8628 §3.3 intends to be displayed, and which was already printed pre-PR at auth.ts:157); no auth check weakened — the poll loop, token validation and error handling at auth.ts:159-188 are untouched. The non-http(s) case is safe by accident and in the right direction: new URL('javascript:…') has origin "null", so it never matches consoleOrigin and gets defused into a harmless https://console…/… path.


Information

  1. The PR body's stated bug no longer reproduces. Per the probe above, staging now mints the console host, so the "Not in this PR" follow-up (INSTA_CONSOLE_URL=https://console.staging.instacloud.com) appears to have already been applied. Worth saying so when closing #134, so the issue records the actual root-cause fix (a platform env var) rather than reading as if the CLI change was the cure.
  2. The PR's "5 pre-existing local failures" did not reproduce. In a clean clone + npm ci, the full suite is 490/490 / 38 files green — consistent with the author's ambient-local-agent-config explanation. No action.
  3. Library-API check. Verified deviceAuthorization's server-side verificationUri option and the optional verification_uri_complete / interval response fields against the Better Auth v1.6.23 docs. The root-cause account and the verification_uri_complete ?? verification_uri fallback at auth.ts:156 both match the current API — no stale or invented usage.
  4. Signature shape. target as a third positional after a defaulted wait forces the undefined placeholder at auth.ts:63. An options object would read better, but a 3-arg internal helper is well within the repo's existing style. Pure taste.
  5. Conventions are respected. .js ESM import specifiers, type-only import for EnvName (auth.ts:4), DI-fake tests with no global module mocking, and comments that explain why, matching the density of the surrounding file. AGENTS.md rule 4 (mirror command/flag changes into skills/insta/cli-reference.md) does not apply — no command or flag surface changed, only output text. Worth a glance only if that reference quotes the device-login output verbatim.
  6. The no-op guard at auth.ts:135 is not what its test proves. test/device-login.test.ts:199-202 would still pass if url.origin === consoleOrigin ? raw : were deleted, because reassembling an already-correct URL yields the identical string. The ternary does earn its keep on round-trip fidelity (a bare https://host would otherwise gain a trailing /), but that isn't what the test name claims. Minor.

Performance

No performance-relevant changes. Two new URL() parses per insta login --device invocation, on a path that then blocks on a human for up to 15 minutes. No loops, no I/O, no allocation growth, no DB queries.

Verdict

approved — zero Critical findings. (Informational: the green checkmark is a separate human action; this bot only posts COMMENT.) The three Suggestions are all worth a follow-up, and #1 (making target required so the wiring is compiler-enforced) is a genuinely cheap win.

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.

staging device-login URL 404s: /device route missing on staging frontend

2 participants