Skip to content

fix(install): set the homepage without a login only on unclaimed environments - #249

Closed
nicknisi wants to merge 1 commit into
mainfrom
nicknisi/fix-api-key-homepage
Closed

nicknisi wants to merge 1 commit into
mainfrom
nicknisi/fix-api-key-homepage

Conversation

@nicknisi

Copy link
Copy Markdown
Member

Follow-up to #244.

Problem

Without a dashboard session, Next.js setup registers the callback with the API key, then writes the default homepage only if none is set (preserveExisting). To check, it reads GET /user_management/app_homepage_url. That route doesn't exist: it returns the same 404 as a made-up path.

GET /user_management/app_homepage_url       → 404 {"message":"The requested resource was not found","error":"Not Found"}
GET /user_management/definitely_not_a_route → 404 {"message":"The requested resource was not found","error":"Not Found"}

The read never succeeds, so every install without a login (including every freshly provisioned unclaimed environment) ends with:

Application setup is incomplete: Callback registered using the API key, but homepage setup failed.

It also leaves no homepage set. Seen in a real workos integrate run.

Fix

With only an API key, the current homepage can't be known. So the installer writes it only where nothing can be overwritten:

  • --homepage-url given: written, as before.
  • Unclaimed environment (the stored environment is unclaimed and its key is the one in use): written. Nobody can open its dashboard until it's claimed. This covers every fresh install, whose sign-out then has a homepage to fall back to.
  • Otherwise: left alone, and listed with the Sign-out URI and Initiate login URI for the dashboard.

The unused preserveExisting read path in setHomepageUrl is removed. The path with a dashboard session is unchanged: it reads appHomepageUrl over GraphQL, which works.

Tests

The API-key tests mocked a homepage read ({ url: null }) that the API never returns. They now mock the real 404 and cover:

  • an unclaimed environment;
  • a claimed environment, another environment's key, and no stored environment;
  • a keyring error;
  • an explicit --homepage-url;
  • PUT failures.

bun run test, typecheck, lint, format, build and scripts/command-smoke.sh all pass.

…ronments

Without a dashboard session, Next.js setup registers the callback with the API
key and then writes the default homepage only if none is set. That check read
`GET /user_management/app_homepage_url`, which doesn't exist (it answers 404,
exactly like an unknown route), so the read always failed and every such
install reported "homepage setup failed" with no homepage set.

With only an API key the current homepage can't be known, so it's now
written only where nothing can be overwritten:
- the homepage the user asked for (--homepage-url), as before
- an unclaimed environment, whose dashboard nobody can open until it's
  claimed: this covers every fresh install

Otherwise it's left alone and listed with the sign-out and initiate login URIs
for the dashboard. The dead `preserveExisting` read path is removed, and the
tests now mock the API's real 404 instead of a homepage read it never returns.
@nicknisi
nicknisi marked this pull request as ready for review September 24, 2026 20:53
@greptile-apps

greptile-apps Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

This PR should not merge until an externally claimed environment cannot have its dashboard homepage overwritten by a stale local profile.

Findings

  1. P1 Stale profile overwrites homepage ▶
Fix with agent prompt
### Issue 1
src/lib/authkit-application-setup.ts:42-43
If an environment is claimed in the dashboard but its local profile still says `unclaimed`, a later API-key-only install treats the matching key as permission to write the default homepage. Install does not refresh the claim status first, and the REST homepage read returns 404, so the PUT can silently replace a homepage set after the claim. Check the current claim status before allowing this write.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR replaces an unavailable REST homepage read with a conditional write during API-key-only AuthKit setup.

  • It writes an explicit homepage or a default for a locally unclaimed environment, and leaves other homepages for dashboard setup.
  • It updates API-key tests to reflect the REST endpoint’s 404 response.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Callback registered with API key] --> B{Explicit homepage?}
  B -- Yes --> D[PUT homepage]
  B -- No --> C{Local active profile unclaimed and key matches?}
  C -- Yes --> D
  C -- No --> E[Leave homepage for dashboard setup]
  D --> F[Return pending dashboard steps]
  E --> F
Loading

Reviews (1) · Last reviewed commit: "fix(install): set the homepage without a..."

Comment on lines +42 to +43
const environment = getActiveEnvironment();
return !!environment && isUnclaimedEnvironment(environment) && environment.apiKey === apiKey;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Stale profile overwrites homepage

If an environment is claimed in the dashboard but its local profile still says unclaimed, a later API-key-only install treats the matching key as permission to write the default homepage. Install does not refresh the claim status first, and the REST homepage read returns 404, so the PUT can silently replace a homepage set after the claim. Check the current claim status before allowing this write.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/authkit-application-setup.ts
Line: 42-43

Comment:
**Stale profile overwrites homepage**

If an environment is claimed in the dashboard but its local profile still says `unclaimed`, a later API-key-only install treats the matching key as permission to write the default homepage. Install does not refresh the claim status first, and the REST homepage read returns 404, so the PUT can silently replace a homepage set after the claim. Check the current claim status before allowing this write.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Addressed on main by merged #252 (60578d0), following #250. The shared isUnclaimedEnvironmentKey helper now confirms live claim status via createClaimNonce after matching the stored key and, when available, the expected client ID. Both implicit-homepage write paths await it; claimed or unavailable status leaves the homepage unchanged, while explicit --homepage-url remains supported.

Regression coverage includes an externally claimed environment, HTTP 409, invalid tokens, missing environments, rate limits, server/network failures, malformed responses, and client-ID mismatches. Rechecked the main-equivalent files today: 105 relevant tests pass. This is a best-effort preflight, not an atomic claim/write guarantee.

Closing this older PR as superseded rather than merging its stale duplicate implementation. The unused preserveExisting option cleanup is the only non-behavioral remainder and is not needed for this fix.

nicknisi added a commit that referenced this pull request Sep 25, 2026
…overwritten

The API-key setup step wrote the homepage URL on every sandbox. The
homepage is one value per environment, and the REST API can set it but not
read it (its GET answers 404), so the write replaced whatever a teammate
had set on a shared sandbox.

It now follows the rule #249 set for the Next.js API-key path: write it only
when the user asked for it (--homepage-url) or the key belongs to the
stored unclaimed environment, whose dashboard nobody has had. Otherwise the
Homepage URL row says it wasn't changed. With a login, the later dashboard
step reads the current homepage and fills an empty one.
nicknisi added a commit that referenced this pull request Sep 25, 2026
…overwritten

The API-key setup step wrote the homepage URL on every sandbox. The
homepage is one value per environment, and the REST API can set it but not
read it (its GET answers 404), so the write replaced whatever a teammate
had set on a shared sandbox.

It now follows the rule #249 set for the Next.js API-key path: write it only
when the user asked for it (--homepage-url) or the key belongs to the
stored unclaimed environment, whose dashboard nobody has had. Otherwise the
Homepage URL row says it wasn't changed. With a login, the later dashboard
step reads the current homepage and fills an empty one.
nicknisi added a commit that referenced this pull request Sep 25, 2026
…overwritten

The API-key setup step wrote the homepage URL on every sandbox. The
homepage is one value per environment, and the REST API can set it but not
read it (its GET answers 404), so the write replaced whatever a teammate
had set on a shared sandbox.

It now follows the rule #249 set for the Next.js API-key path: write it only
when the user asked for it (--homepage-url) or the key belongs to the
stored unclaimed environment, whose dashboard nobody has had. Otherwise the
Homepage URL row says it wasn't changed. With a login, the later dashboard
step reads the current homepage and fills an empty one.
nicknisi added a commit that referenced this pull request Sep 25, 2026
* fix(install): register redirect URIs for every server-side SDK

The installer machine only called autoConfigureWorkOSEnvironment for
TanStack Start and React Router, and returned early for non-JavaScript
SDKs. Each integration's run() skips its own call when the machine passes
credentials, so SvelteKit, Node, PHP, Laravel, Python, Ruby, Go, .NET,
Kotlin and Elixir never registered their redirect URI, CORS origin or
homepage URL.

Key the check on the integration's requiresApiKey flag (Next.js keeps its
post-validation path), and register URLs before the non-JavaScript return.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* fix(install): register redirect URIs and CORS origins for client-only SDKs

React and vanilla JS (Vite) apps declare requiresApiKey: false, so the
previous fix still skipped them, though the install usually holds an API
key from the staging credentials. A client-only app needs its callback and,
above all, its CORS origin registered. Register URLs for every SDK except
Next.js whenever an API key is available.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* feat(install): save sign-out and initiate login URIs for every SDK

Only Next.js set the Sign-out URI and Initiate login URI, through the
dashboard GraphQL path. The REST API has no endpoint for either, so other
SDKs left both for the user to set by hand.

- Run configureAuthkitApplication after the agent for every SDK. For SDKs
  other than Next.js, a failure leaves the settings for the dashboard
  instead of failing a finished install.
- Save the origin as the Sign-out URI everywhere. Save an Initiate login
  URI only where the SDK guide fixes the route (signInPath in
  cli.config.ts); otherwise report it as not set.
- React and vanilla JS apps get a /sign-in client route that calls
  signIn() on load, checked by validation before it is saved.
- Pin the agent to that exact route in the prompt.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* fix(install): align saved AuthKit URLs with each SDK's docs

Checked the saved Initiate login and callback paths against the WorkOS
docs and SDK READMEs.

- React and vanilla JS use /login, as the authkit-react docs do, and must
  pass WORKOS_REDIRECT_URI as redirectUri: the SDK defaults to the page
  origin, which the installer never registers. Validation checks both.
- Save the documented sign-in routes for React Router (/login), TanStack
  Start (/api/auth/sign-in), SvelteKit (/sign-in), Node (/login, which also
  fixes its outro copy) and plain PHP (/login.php).
- SvelteKit defaults to Vite's 5173 and /callback, per its README.
- Pin the route in the custom Ruby, Go, .NET and Elixir prompts too, so
  the agent cannot fall back to a path the dashboard does not point at.
- Drop the query-parameter pass-through: AuthKit keeps password-reset and
  invitation details through the redirect.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* refactor(install): simplify AuthKit URL setup across SDKs

- Build the default redirect URI in one resolveRedirectUri() helper.
- Derive client-only SDKs from requiresApiKey instead of a second list.
- Scan client source once, in parallel, for both the sign-in route and
  redirectUri; check only that route after the agent instead of rerunning
  the full validator.
- Report why an initiate login URI was skipped (no fixed route vs. a
  missing route) and show an unusable callback URL in the checklist.
- Give the Python prompt the pinned sign-in route like the other SDKs.
- Read the Node outro routes from config.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* fix(install): tighten client-only route and redirect URI checks

Address review feedback on the client-only validation:

- A link to /login no longer counts as the route. Require a route
  declaration (router path, createFileRoute, or a pathname check) or a
  static page that calls signIn().
- Check that the prefixed env var the client reads as redirectUri (for
  example VITE_WORKOS_REDIRECT_URI) is set, and flag an unprefixed one
  the build tool never exposes. The installer writes only the unprefixed
  WORKOS_REDIRECT_URI.
- Read source files in bounded batches instead of all at once.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* refactor(install): write client env vars under the bundler prefix

For client-only apps, the installer now writes VITE_ (or REACT_APP_ for
Create React App) WORKOS_CLIENT_ID and WORKOS_REDIRECT_URI itself, instead
of asking the agent to copy them and validating the copy. The validator
only checks that the client reads the variable the installer wrote.

Also share one route check between validation and the save step, build
the route pattern from a named list, and read static pages in parallel.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* fix(install): check only browser code for the client redirect URI var

A Vite app may read the unprefixed WORKOS_REDIRECT_URI in server-side
files such as vite.config.ts. Check Vite browser code through
import.meta.env, and Create React App through process.env in src/, so
those files no longer fail validation.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* refactor(install): key browser env access rules by bundler prefix

Replace two prefix ternaries with one table typed against the prefixes
getClientEnvPrefix returns, so a new bundler cannot fall through to the
Create React App rules.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* fix(install): flag process.env redirect URI reads in Vite browser code

Vite browser code has no process.env, but the previous change checked
only import.meta.env reads for Vite apps. Match the full access
expression instead: count every import.meta.env read and process.env
reads in src/, and require the one expression the bundler exposes.
vite.config.ts and scripts may still read the unprefixed var.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* refactor(install): define client browser code once for all checks

Read only browser code for the client-only checks: leave out config
files, scripts, server code and tests, which run in Node. The sign-in
route, redirectUri and env checks now share one file set, and the env
check needs no per-accessor path rule.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* fix(install): scope client browser code by bundler

Create React App compiles only src/, so a root server.js that reads
the unprefixed redirect URI no longer fails validation. For Vite and
other client apps, leave out only the Node files at the project root
(bundler config, a server, scripts), so a browser config such as
src/auth.config.ts is still checked.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* refactor(install): read client browser code with one glob

Use one fast-glob call whose root depends on the bundler, look up the
bundler prefix once per validation, and add an envReadIssue spec helper.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* fix(install): leave nested server code out of the client browser scan

Server code in a nested folder such as src/server/ may read the
unprefixed redirect URI. Exclude server files and folders at any depth;
bundler config and scripts stay root-only so browser files in src/ are
still checked.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* refactor(install): classify redirect URI reads instead of guessing server paths

Every server-path exclusion existed to hide one read: Node code reading
process.env.WORKOS_REDIRECT_URI. Exempt that read for Vite, where the
same read in browser code throws at page load (no process global), and
drop the server/ and scripts/ exclusions. Create React App keeps flagging
it, because CRA defines process.env in the browser and the read comes
back undefined without an error. The silent cases (import.meta.env with
the wrong name, process.env with a browser prefix) are still flagged in
any file.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* fix(install): accept a /login pathname check through any variable

The route check only recognised `pathname === '/login'`. The agent
followed the prompt and wrote `const path = window.location.pathname;
if (path === '/login')`, which failed validation in a real Vite app.
Accept a comparison against any identifier, in either order.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* refactor(install): name the accepted /login route forms in the prompt

The agent prompt and the route check had no shared contract, so the
agent could write a form the validator did not know. The prompt now names
the forms the validator accepts, and the route regex uses String.raw
like its declarations.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018EgeNr2FXXVcJ5o165U5Lx

* fix(install): never write localhost URLs with a production API key

The API-key setup step registers a localhost redirect URI and CORS origin
and sets the homepage URL. It ran for any key, and a production key can
reach the installer three ways: the --api-key flag or WORKOS_API_KEY, the
project's env file, or an active profile of type production (the
environment picker lists every profile with a key). With this PR running
the step for every SDK, a Go, Rails, or Django install with a live key
would add localhost URLs to production and replace its homepage.

autoConfigureWorkOSEnvironment now does nothing unless the key starts with
sk_test_, and reports the redirect URI and CORS origin as skipped with the
reason, so the checklist and summary show them. The check sits in the
function, not a caller, because the installer machine, agent-runner, and
some integrations' own run() all reach it. It is the same sk_test_ test
the Next.js API-key path already uses, so every write path now agrees.

* fix(install): leave a sandbox's homepage alone unless nothing can be overwritten

The API-key setup step wrote the homepage URL on every sandbox. The
homepage is one value per environment, and the REST API can set it but not
read it (its GET answers 404), so the write replaced whatever a teammate
had set on a shared sandbox.

It now follows the rule #249 set for the Next.js API-key path: write it only
when the user asked for it (--homepage-url) or the key belongs to the
stored unclaimed environment, whose dashboard nobody has had. Otherwise the
Homepage URL row says it wasn't changed. With a login, the later dashboard
step reads the current homepage and fills an empty one.

* fix(install): check the client code the browser loads, not every file

The client-only checks scanned every source file in a Vite project, so they
could not tell browser code from a server or script beside it. Each fix moved
the gap: a /login route in server.js counted as the client route, and
exempting server reads hid browser code that read
process.env.WORKOS_REDIRECT_URI, which Vite leaves undefined. The route check
also counted any `x === '/login'` comparison as a route.

The scan now traces what the browser loads: every HTML page and its script
srcs (Vite's index.html, a plain app's pages), or Create React App's
src/index, then their relative, root and @/ imports. A server, a script or
the bundler config is never reached from there, wherever it lives, so traced
browser code that reads the unprefixed var is flagged again. A project with
no entry keeps the old whole-project scan and its exemption.

A comparison counts as the route only when it checks the pathname:
`location.pathname === '/login'`, a variable read from it, or
`case '/login':` in a switch on it. `path = '/login'` (an assignment) no
longer passes as `path="/login"`. These are the forms the agent prompt names.

* revert: restore client validation before browser-source tracing

Revert df2d1f4. The scanner introduced false negatives for inline module imports and custom aliases without reliably proving that a route starts sign-in. Restore the previous validator and leave those review findings open. Keep the production-key guard and homepage protection unchanged.

* ci: diagnose smoke authentication with a direct read-only request

When authenticated command smoke fails, issue a single GET /connections using the same smoke credential and configured API host through curl instead of the compiled CLI. Discard the body, do not follow redirects, and log only HTTP status, curl exit status, and the request ID. Keep the original failed check red.

* fix(install): bound client checks and exclude server-only sources

Check conventional client roots without an import resolver, excluding server, API, function, script and other workspace paths. Flag Node env reads in Vite client code rather than globally exempting them. Restrict route evidence to supported router forms or actual pathname comparisons and require a sign-in call. Limit reads to 256 files, 256 KiB per file and 4 MiB total; incomplete scans cannot authorize saving a sign-in destination. Add regression cases for the review findings and previously broken inline/custom-alias layouts.

* fix(install): report incomplete URL setup without claiming verification

Use the established explicit-or-unclaimed homepage policy in the API-only fallback too. Authorized writes no longer require the unsupported homepage GET to succeed; unknown existing values remain untouched with an honest reason. A missing initiate-login destination leaves setup unverified even when other writes succeed. Add real GET-404 and missing-route regression coverage.

* fix(install): tie sign-in evidence to the matching pathname branch

Fail required validation when bounded scans are incomplete. Replace disconnected route/signIn string matches with parsed startup or React mount-effect branches: the matching positive pathname branch must directly call signIn. Ignore comments, strings, click callbacks and unrelated routes. Standardize this conservative source-evidence form in the client prompt; no import graph or bundler resolution. Promote the already-locked Babel parser to a direct dependency and add regression coverage.

* ci: distinguish unusable smoke credentials from CLI regressions

Preflight live smoke with one direct read-only API request. Missing credentials or direct HTTP401 report live checks NOT RUN with an explicit warning/summary instead of misclassifying a CLI regression. HTTP200 enables the unchanged authenticated CLI checks, whose failures still fail CI. All other HTTP or transport failures remain fatal. Localhost-only tests cover status handling, non-disclosure, no redirects, and propagation of CLI failures.

* fix(install): recognize sign-in effects bound to React login routes

Tie JSX and object-route targets to their lexical component definitions and mount effects, including relative default/named exports from the bounded source set. A routed component does not need a redundant pathname guard. Reject unrelated, shadowed, reassigned, click-only and unresolved component bindings. Preserve the bounded scan and existing guarded startup path; no disk-expanding import traversal.

* fix(install): configure all application URLs through one target

Defer non-Next.js URL writes until post-agent setup chooses the client-ID-matched dashboard application or the API-key-only fallback. Never write early through API key A then later through dashboard environment B. Include CORS in that choice: preserve existing dashboard origins with dry-run/recheck/readback, or add through REST only on the API-key path. An unregistered callback cannot be reported as successful setup. Cover mismatched credentials, late sessions, production refusal, CORS preservation and incomplete results.

* ci: warn explicitly when smoke credentials are unavailable

* Revert "ci: warn explicitly when smoke credentials are unavailable"

This reverts commit 3052708.

* Revert "ci: distinguish unusable smoke credentials from CLI regressions"

This reverts commit eaae2f4.

* Revert "ci: diagnose smoke authentication with a direct read-only request"

This reverts commit 6313223.

* refactor(install): leave client login verification to browser testing

Remove the client-route AST analyzer and its direct Babel dependency. Keep generating a public login route using app conventions, but leave the client-only Initiate login URI unchanged until browser verification. Treat scan limits as warnings while preserving callback/env checks and mandatory registration.\n\nRetain sandbox/homepage protections and single-target writes. Derive CORS from the callback origin and report read-back sign-out independently of pending login verification. CI smoke changes are split onto ci/smoke-auth-preflight.

* ci: rerun checks after retargeting installer fixes to main

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Nick Nisi <nick.nisi@workos.com>
@nicknisi

Copy link
Copy Markdown
Member Author

Superseded by merged #250 and #252. The homepage policy and live-claim safeguard are already on main, including regression tests for the review finding. No additional behavior needs to merge from this conflicted branch; retaining the branch/history for reference.

@nicknisi nicknisi closed this Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant