fix(auth): gate the localhost trusted-origin substitution to non-production - #11115
Conversation
The trustedOrigins fallback substituted a localhost wildcard trio whenever the resolved origin list was empty. Its own comment described this as a development convenience, but the condition tested only emptiness, so a production deployment whose trusted-origin list resolved empty CSRF-trusted every localhost and *.localhost origin. Gate the substitution on NODE_ENV !== 'production', the same dev signal used by the fallback auth secret and the dev Origin synthesis, so the boundary the comment claims is the boundary that is enforced. In production the key is now omitted from the better-auth config. That is not an absent policy: better-auth seeds its trusted set from the resolved baseURL origin and treats trustedOrigins as purely additive, so an omitted key and an empty array are equivalent and both leave exactly the deployment's own origin trusted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
…ion only Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
📓 Docs Drift Check1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅ What this run could not see
Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin ffe466ca78ae31a7d17e82d934d1aedbc0a24b7e && git checkout ffe466ca78ae31a7d17e82d934d1aedbc0a24b7e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 13c0b166b8ed498ce7d6db7fd5ae70f1761e848c 26a8967130614f4867b83c1cd1fcf10e867caba9 && git checkout -B drift-repro 13c0b166b8ed498ce7d6db7fd5ae70f1761e848c && git merge --no-ff 26a8967130614f4867b83c1cd1fcf10e867caba9
node scripts/docs-audit/affected-docs.mjs --json 13c0b166b8ed498ce7d6db7fd5ae70f1761e848c |
|
CI is green on Verified by enumerating jobs rather than reading a check-suite conclusion — two ⛔ This PR stays draft on purpose and is NOT waiting on me. The card carries Two things for whoever reviews it, both already in the PR body but easy to miss:
Generated by Claude Code |
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 32596638385 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Fixes #10366
Implements the maintainer ruling of 2026-08-22 (Option A — non-production only), verbatim.
What changed
packages/plugins/plugin-auth/src/auth-manager.ts— thetrustedOriginsblock substituted a localhost wildcard trio whenever the resolved origin list came out empty andOS_CORS_ORIGINwas unset or*. Its own comment called this a development convenience, but the condition carried noNODE_ENVterm, no dev-mode term, nothing — it tested only emptiness. The declared boundary and the enforced boundary disagreed, and only the declared one was visible in the file.The substitution is now gated on the ruling's literal predicate,
NODE_ENV !== 'production'.Property enforced: a development convenience exists only outside production.
Predicate spelling — chosen, not invented
This file already contains several spellings of the environment test:
!== 'production',=== 'production',=== 'development', and a!== 'production'living inside an unrelated method rather than a reusable predicate. I enumerated them and added none. The form used here is the file's existing sibling-CSRF-gate spelling (the devOriginsynthesis), which is also the form used by the fallback-auth-secret gate on the same config-build path — provingprocessis available there without aglobalThisguard.Not hardened to
=== 'development': that is a stricter boundary than ruled and would breaktestand unset-NODE_ENVdevelopment flows. A pin now holds that line in both directions.Measured, not assumed: what production actually receives
The block's tail returns
trustedOriginsonly when the computed list is non-empty, and otherwise returns an object with notrustedOriginskey at all. So in production, once the trio is gated off and the list is empty, better-auth receives notrustedOriginskey — not an empty array. Those are different inputs, and the ruling's premise depended on the answer. It was unmeasured on the card. It is measured now.Measured against the installed better-auth 1.7.1, by reading the resolver and by booting real instances:
getTrustedOrigins(dist/context/helpers.mjs) unconditionally seeds the trusted set from the resolvedbaseURLorigin, then treatsoptions.trustedOriginsas purely additive (trustedOrigins.push(...)).validateOrigin(dist/api/middlewares/origin-check.mjs) refuses anything unmatched with403 INVALID_ORIGIN.Booted probe results (
500= the origin check passed and reached the credential path against a schema-less memory adapter;403= refused):tenant.localhostevil.example.nethttps://app.example.comhttps://app.example.com[]https://app.example.comThe absent-key default is a refusal, not a permissive fallback. It leaves exactly the deployment's own origin trusted. The ruling's assumption holds, and no sentinel value, synthesized origin or forced empty array was needed — each of those would have been a different trust surface and a different decision.
A correction to the card's reachability read
The severity read predicted that in the bare-host shape (
OS_AUTH_URLset to a host with no scheme) the deployment's real origin would still be refused. Measured, it is not:getCanonicalOriginrepairs the bare host by prepending the scheme, and that repaired value is whatauth-manager.tshands better-auth asbaseURL, which better-auth then seeds into its trusted set. So in the bare-host shape the real origin is already trusted today; the defect there is only the extra trio. This change removes the extra without touching the real origin.The two empty-list causes compose as follows in production after this change:
getCanonicalOriginfalls back to itshttp://localhost:3000default, so the accept set becomes that. Still a refusal for the trio and for the real origin; the operator's remedy is to setOS_TRUSTED_ORIGINSor fix the base URL. PR fix(cli): serve says so when the auth base URL is unusable, instead of swallowing it #10369's boot diagnostic already names that condition at startup for both causes, and is not duplicated here.Pins
Both existing tests keep their dev-only assertions verbatim — untouched. New pins added alongside:
trustedOriginsabsent from the config (asserted as key-absent, not merelyundefined)development, undertest, and withNODE_ENVunsettrustedOriginsand anOS_CORS_ORIGIN-derived list are forwarded unchanged. Without these two legs, a change that broke all origin trust in production would pass a substitution-only suite.trustedOriginsas a per-request async function built from a copy of the sameoriginsarray, so gating the push at the source covers both shapes. That is true today and is exactly what a future refactor breaks silently — pinned in both directions (production: trio absent from what the function returns; non-production: trio present).Verification
Ablation with the signature predicted in writing before running: removing the gate reddens exactly three cases — the two production-omission pins and the SSO production pin — while both load-bearing legs and every non-production leg stay green. Observed: exactly those three,
3 failed | 6 passed. Restore proved byte-identical bygit hash-object(d6bdba96…before and after) and re-run to a real green verdict.The ablation also settles
srcvsdist: the test importsAuthManagerby relative path with no vitest alias, so it resolves package source. Held falsifiable — a src-only edit with no rebuild flipped the verdict, which could not happen if the suite readdist.Gate union derived on the final commit
26a89671, clean tree,dispatch-gates.mjswith no path arguments (exit 0, 3 paths vs merge base24043c290). All 11 path-matched and 5 convention-triggered gates run to real verdicts, exit 0, each read from the gate's own printed verdict line rather than a pipe status.check:type-check-debt --re-measurewas run against a built workspace closure — plugin-auth's TEST_DEBT ceiling is 109 and tsc now reports 97, so the new test code adds zero type errors. That 12-error surplus is pre-existing and already tracked in #6376; not touched here, and no baseline was written.Note: this package's
tsconfig.jsonexcludes**/*.test.ts, so the new pins are invisible to its owntypecheck. They were typechecked explicitly via a test-inclusive tsc program — zero errors in the added range.Scope
Out of scope and deliberately untouched: the
''-coalescing question (parked), PR #10369's boot diagnostic (not duplicated), andcloud(outside this session's repo scope — triage already recorded that its per-project factory builds the list explicitly and passesundefinedrather than relying on this fallback, so the cloud production path does not depend on the trio). Nopackages/specchange was needed.Declared residual, filed not fixed: with
NODE_ENVunset,undefined !== 'production'is true, so anos servedeployment that never sets it still receives the trio. The ruling's literal text governs, so it is implemented as ruled and the gap is recorded in #11113 rather than silently tightened. Measured while filing:os startalready forcesNODE_ENV=productionwhen unset (start.ts:347);os servedoes not.Every line number in this PR was re-derived in the worktree; the ones on the card and its comments are stale.
Review state
needs:contract-reviewremains on #10366. This PR stays draft — not marked ready, auto-merge not armed, not merged, and the label not cleared by me.Generated by Claude Code