Skip to content

fix(cli): serve says so when the auth base URL is unusable, instead of swallowing it - #10369

Merged
os-elon merged 2 commits into
mainfrom
claude/issue-10202-empty-auth-url-trusted-origins
Aug 20, 2026
Merged

fix(cli): serve says so when the auth base URL is unusable, instead of swallowing it#10369
os-elon merged 2 commits into
mainfrom
claude/issue-10202-empty-auth-url-trusted-origins

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#10202

The measurement came first

The card filed this as not measured — read from source, never observed — and named the specific way it could be wrong: "whether better-auth has any behaviour of its own that masks an empty baseURL." So the first deliverable was a real boot, not a patch.

examples/app-todo under os serve, NODE_ENV=production, OS_AUTH_SECRET and OS_SECRET_KEY set, OS_TRUSTED_ORIGINS / OS_ROOT_DOMAIN / OS_BASE_URL / OS_CORS_ORIGIN / preview mode all unset. Probed with POST /api/v1/auth/sign-in/email and deliberately wrong credentials, so a trusted origin answers 401 INVALID_EMAIL_OR_PASSWORD and an untrusted one answers 403 INVALID_ORIGIN — two distinct codes, neither of which can be confused with the 429 a repeated sign-up probe earns.

OriginA: OS_AUTH_URL= (empty)B: OS_AUTH_URL=https://app.example.comC: OS_AUTH_URL unset
https://app.example.com403401403
http://localhost:PORT401403401
http://tenant.localhost:PORT401403403
https://tenant.localhost403403403
https://evil.example.net403403403
/api/v1/health · /api/v1/ready200 · 200200 · 200200 · 200

Confirmed. With a set-but-empty OS_AUTH_URL the deployment's own origin is refused while health and ready keep answering 200. Authentication is dead and nothing says so. The three language semantics the card named all hold at head: readEnvWithDeprecation returns '' for a present-but-empty variable (packages/types/src/env.ts, preferredValue !== undefined), '' ?? x is '', and new URL('') throws.

Corrected. The card predicted trustedOrigins would be [] and that "every origin is refused". It is not, and they are not. serve passes trustedOrigins.length ? trustedOrigins : undefined, and AuthManager substitutes a localhost wildcard trio (http://localhost:*, http://*.localhost:*, https://*.localhost:*) for an absent list — pre-existing behaviour, already pinned by auth-manager.test.ts. So better-auth receives a non-empty allow-list. The masking layer the filer suspected is real; it is ObjectStack's own AuthManager, not better-auth. (better-auth never sees '' either — getCanonicalOrigin() masks it to http://localhost:3000 via ||.)

Worse than claimed. Column A versus column C is the finding the card did not have: http://tenant.localhost:PORT is trusted when the variable is empty and refused when it is unset. Set-but-empty is strictly more permissive than unset — an env template rendering an absent key to the empty string silently widens a production CSRF allow-list to every localhost subdomain. Filed separately as #10366, unassigned.

The fix — direction 2 only

The catch was the only witness that the configured base URL was unusable, and it discarded the witness:

try{constu=newURL(baseUrl);…push…}catch{/* ignore malformed baseUrl */}

Resolution is now extracted into an exported seam — resolveAuthBaseUrl() reports the value, which variable supplied it, and whether it parses; formatUnusableAuthBaseUrlDiagnostic() produces the sentence. The call site pushes an origin when there is one and warns when there is not.

What is resolved did not change. Same chain, same precedence, same ${protocol}//${host} origin spelling (kept over URL.origin, which answers the string "null" for a non-special scheme). A set-but-empty OS_AUTH_URL still stops the chain exactly as before — that behaviour is now pinned by a test rather than left implicit.

Direction 1 — treating empty as unset inside readEnvWithDeprecation — is not touched, per the card and the dispatch. See the census below for the input that decision was missing.

Per the dispatch, the diagnostic is a warning, not a refusal to boot: a deployment running set-but-empty today keeps starting, and now says why authentication will not work.

Verified reaching an operator, on a real boot

The dispatch made this conditional: if the message cannot reach an operator in the deployment shapes that produce the defect, stop and report the fork. It reaches them. Rebuilt CLI, same env as column A:

 ⚠ Auth base URL is unusable — its origin was NOT added to the CSRF allow-list.
OS_AUTH_URL is set but EMPTY ("").
An empty value is NOT the same as an unset one: the fallback chain skips only
UNSET variables, so OS_BASE_URL and the built-in http://localhost:PORT default were never consulted.
Sign-in and sign-up will answer 403 INVALID_ORIGIN for this deployment's own
origin, while /api/v1/health and /api/v1/ready keep answering 200.
Fix: set OS_AUTH_URL to this deployment's public origin (e.g.
https://app.example.com), or remove the variable entirely to fall back to
OS_BASE_URL / http://localhost:PORT.

It lands on stderr next to the existing StorageServicePlugin production warning — the channel operators already read. Re-probing that same boot reproduced column A's status codes exactly, confirming the change is diagnostic-only.

Census — reported, not fixed

The card named an unexamined population: how many other readEnvWithDeprecation call sites feed a ?? chain. 24 call sites outside the helper and its tests; 9 feed a ?? chain, of which 2 are unmitigated:

SiteChainEmpty value's fate
cli/src/commands/serve.tsOS_AUTH_URL?? OS_BASE_URL ?? http://localhost:PORTunmitigated — this card
mcp/src/plugin.tsOS_MCP_SERVER_TRANSPORT?? options.transport ?? 'stdio'unmitigated'' is neither stdio nor http
cli/src/commands/serve.tsOS_PORT (flag default)?? '3000'reaches parseInt('')NaN (not traced further)
cli/src/commands/dev.tsOS_PORT?? '3000'same shape (not traced further)
cli/src/commands/start.tsOS_PORT (banner)?? 3000cosmetic — banner URL only
cli/src/commands/serve.tsOS_AUTH_SECRET?? (isDev ? dev-secret : undefined)caught downstream by !secret
cli/src/commands/start.tsOS_AUTH_SECRET?? readOrCreateAuthSecret(homeDir)falsy secret, not traced further
mcp/src/plugin.tsOS_MCP_SERVER_NAME?? options.name ?? 'objectstack'cosmetic
cli/src/utils/log-level.tsOS_LOG_LEVEL?? DEFAULT_LOG_LEVEL (in resolveLogLevel)self-healing — whitelist rejects ''

The other 15 neutralise empty at or near the read (|| '', if (value), === 'false' / !== 'false', ?.trim() then truthiness). One near-shape worth naming: resolveStorageLocalRootEnv() guards with value === undefined, so '' passes through — but its consumer resolveStorageCapabilityArg uses envRoot?.trim() || default, so it lands safely.

Nine sites, two unmitigated. That is a small number, and it is an argument against reaching into the shared helper on this card's behalf.

Verification

Gate families re-derived with node scripts/pm/dispatch-gates.mjs and no path arguments, against the real diff. Full results in the dev report on #10202. packages/cli suite: 138 files / 1525 tests passed — 138 test files exist in the worktree against 137 on main, so the new file is the +1 and did run.

Not addressed here

The localhost-wildcard-in-production behaviour that column A exposes is AuthManager's pre-existing default and is out of scope here — filed separately as its own card, unassigned. No other issue is addressed by this PR.


Verified at 76de54c34. Full gate results in the dev report on #10202.

Generated by Claude Code


Generated by Claude Code

…f swallowing it (#10202)
`serve` resolved the auth base URL through a `??` chain and parsed it inside
`try { new URL(baseUrl) } catch { /* ignore malformed baseUrl */ }`. That catch
was the only witness that the configured value could not be parsed, and it
discarded the witness: the deployment's own origin never reached the CSRF
allow-list, boot continued, and `/api/v1/health` kept answering 200 while
sign-in answered 403 INVALID_ORIGIN naming nothing.
Extract the resolution into an exported seam (`resolveAuthBaseUrl`) that reports
which variable supplied the value and whether it parses, plus
`formatUnusableAuthBaseUrlDiagnostic` for the sentence. Resolution itself is
byte-for-byte unchanged — same chain, same precedence, same
`${protocol}//${host}` origin spelling.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
@github-actionsgithub-actionsBot added size/m documentation Improvements or additions to documentation tests tooling labels Aug 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 11 documentable anchor(s).

23 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json d23e3a00c0ad431320f598e168b40dda7bc74131.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json d23e3a00c0ad431320f598e168b40dda7bc74131packageMentionDocs.

Which tree this was computed on

This run read content/docs from 58f8e362f55abe4a1f91bd9ca0a3ea88cd849ab4 — the merge of head 76de54c3430c1b2d79274674534c593587ea90c7 into base d23e3a00c0ad431320f598e168b40dda7bc74131, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 58f8e362f55abe4a1f91bd9ca0a3ea88cd849ab4 && git checkout 58f8e362f55abe4a1f91bd9ca0a3ea88cd849ab4
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d23e3a00c0ad431320f598e168b40dda7bc74131 76de54c3430c1b2d79274674534c593587ea90c7 && git checkout -B drift-repro d23e3a00c0ad431320f598e168b40dda7bc74131 && git merge --no-ff 76de54c3430c1b2d79274674534c593587ea90c7
node scripts/docs-audit/affected-docs.mjs --json d23e3a00c0ad431320f598e168b40dda7bc74131

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs d23e3a00c0ad431320f598e168b40dda7bc74131 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-elon
os-elon marked this pull request as ready for review August 20, 2026 17:26
@os-elon
os-elon added this pull request to the merge queueAug 20, 2026
Merged via the queue into main with commit 621a487Aug 20, 2026
25 checks passed
@os-elon
os-elon deleted the claude/issue-10202-empty-auth-url-trusted-origins branch August 20, 2026 18:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-elon@claude