Skip to content

fix(auth): gate the localhost trusted-origin substitution to non-production - #11115

Merged
os-warren merged 2 commits into
mainfrom
claude/issue-10366-csrf-localhost-trio-dev-only
Aug 22, 2026
Merged

fix(auth): gate the localhost trusted-origin substitution to non-production#11115
os-warren merged 2 commits into
mainfrom
claude/issue-10366-csrf-localhost-trio-dev-only

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

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 — the trustedOrigins block substituted a localhost wildcard trio whenever the resolved origin list came out empty and OS_CORS_ORIGIN was unset or *. Its own comment called this a development convenience, but the condition carried no NODE_ENV term, 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 dev Origin synthesis), which is also the form used by the fallback-auth-secret gate on the same config-build path — proving process is available there without a globalThis guard.

Not hardened to === 'development': that is a stricter boundary than ruled and would break test and unset-NODE_ENV development flows. A pin now holds that line in both directions.

Measured, not assumed: what production actually receives

The block's tail returns trustedOrigins only when the computed list is non-empty, and otherwise returns an object with no trustedOrigins key at all. So in production, once the trio is gated off and the list is empty, better-auth receives no trustedOrigins key — 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 resolved baseURL origin, then treats options.trustedOrigins as purely additive (trustedOrigins.push(...)).
  • Therefore an omitted key and an empty array are equivalent — confirmed by boot, identical outcomes on every probe.
  • validateOrigin (dist/api/middlewares/origin-check.mjs) refuses anything unmatched with 403 INVALID_ORIGIN.

Booted probe results (500 = the origin check passed and reached the credential path against a schema-less memory adapter; 403 = refused):

baseURL handed to better-auth key tenant.localhost evil.example.net own origin
https://app.example.com absent 403 403 500 (trusted)
https://app.example.com [] 403 403 500 (trusted)
https://app.example.com trio (today) 500 (trusted) 403 500 (trusted)

The 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_URL set to a host with no scheme) the deployment's real origin would still be refused. Measured, it is not: getCanonicalOrigin repairs the bare host by prepending the scheme, and that repaired value is what auth-manager.ts hands better-auth as baseURL, 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:

  • bare host — accept set becomes the deployment's real origin. Strictly better than today.
  • empty string — getCanonicalOrigin falls back to its http://localhost:3000 default, so the accept set becomes that. Still a refusal for the trio and for the real origin; the operator's remedy is to set OS_TRUSTED_ORIGINS or 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:

  • production, no key provided ⇒ trustedOrigins absent from the config (asserted as key-absent, not merely undefined)
  • production, empty array provided ⇒ same
  • non-production still substitutes, under development, under test, and with NODE_ENV unset
  • load-bearing: in production an explicitly configured trustedOrigins and an OS_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.
  • SSO shape: when the SSO RP is wired the block returns trustedOrigins as a per-request async function built from a copy of the same origins array, 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 by git hash-object (d6bdba96… before and after) and re-run to a real green verdict.

The ablation also settles src vs dist: the test imports AuthManager by 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 read dist.

Gate union derived on the final commit 26a89671, clean tree, dispatch-gates.mjs with no path arguments (exit 0, 3 paths vs merge base 24043c290). 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-measure was 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.json excludes **/*.test.ts, so the new pins are invisible to its own typecheck. 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), and cloud (outside this session's repo scope — triage already recorded that its per-project factory builds the list explicitly and passes undefined rather than relying on this fallback, so the cloud production path does not depend on the trio). No packages/spec change was needed.

Declared residual, filed not fixed: with NODE_ENV unset, undefined !== 'production' is true, so an os serve deployment 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 start already forces NODE_ENV=production when unset (start.ts:347); os serve does 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

⚠️ This changes the runtime accept set (Clause-② confirmed). needs:contract-review remains 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

claude added 2 commits August 22, 2026 17:29
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
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • 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 — 11 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 13c0b166b8ed498ce7d6db7fd5ae70f1761e848cpackageMentionDocs.

Which tree this was computed on

This run read content/docs from ffe466ca78ae31a7d17e82d934d1aedbc0a24b7e — the merge of head 26a8967130614f4867b83c1cd1fcf10e867caba9 into base 13c0b166b8ed498ce7d6db7fd5ae70f1761e848c, 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 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

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

Copy link
Copy Markdown
Collaborator Author

CI is green on 26a89671, verified by job name. All 31 check runs terminal; every one success except Console Pin Gate and Build Docs, both skipped.

Verified by enumerating jobs rather than reading a check-suite conclusion — two check_suite.completed: success events arrived on this head while jobs were still running, which has held on every PR in this lane today.

This PR stays draft on purpose and is NOT waiting on me. The card carries needs:contract-review; draft is the compensating control for developing a Clause-② card below CONTRACT_REVIEW_TIER. It waits on a non-dispatching seat's contract review — nothing else blocks it. As the PM seat that dispatched the card, I will not mark it ready, arm auto-merge, or clear the label.

Two things for whoever reviews it, both already in the PR body but easy to miss:

  1. The pre-write measurement is the load-bearing part. Gating the trio off in production leaves better-auth with no trustedOrigins key at all, not an empty array — and the fix is only correct because an absent key was measured to be a refusal (better-auth seeds its trusted set from the resolved baseURL origin and treats the option as purely additive). That was established by booting real betterAuth instances, and it is the thing to re-check if better-auth is ever upgraded.
  2. The unset-NODE_ENV residual is deliberate, not an oversight: implemented exactly as ruled (NODE_ENV !== 'production'), filed separately as Dev-only gates spelled NODE_ENV !== 'production' still open in a real production deployment that never sets NODE_ENV (os serve does not force it, os start does) #11113, and pinned so a later tightening to === 'development' reddens rather than silently becoming stricter than the maintainer ruled.

Generated by Claude Code

@os-warren
os-warren marked this pull request as ready for review August 22, 2026 20:25
@os-warren
os-warren added this pull request to the merge queue Aug 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32596638385 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 6.16s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️ 本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 97 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

Merged via the queue into main with commit bbe643c Aug 22, 2026
32 checks passed
@os-warren
os-warren deleted the claude/issue-10366-csrf-localhost-trio-dev-only branch August 22, 2026 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AuthManager trusts localhost wildcards for CSRF in production whenever trustedOrigins arrives empty — measured

2 participants