From 8808dbe422a49462c4a00e4e3cc41c39d11d36b7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 17:29:53 +0000 Subject: [PATCH 1/2] fix(auth): gate localhost trusted-origin substitution to non-production 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 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .../plugin-auth/src/auth-manager.test.ts | 140 ++++++++++++++++++ .../plugins/plugin-auth/src/auth-manager.ts | 23 ++- 2 files changed, 162 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-auth/src/auth-manager.test.ts b/packages/plugins/plugin-auth/src/auth-manager.test.ts index e50da10dd1..8db4c64313 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.test.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.test.ts @@ -1315,6 +1315,146 @@ describe('AuthManager', () => { }); }); + // #10366 — the localhost-wildcard trio is a DEVELOPMENT convenience and is + // gated on `NODE_ENV !== 'production'`. Before the gate the condition tested + // only emptiness, so a production deployment whose trusted-origin list + // resolved empty silently CSRF-trusted every `localhost` / `*.localhost` + // origin. These pins enforce the boundary in BOTH directions: production + // must not substitute, and non-production must still substitute. + describe('trustedOrigins localhost substitution is non-production only (#10366)', () => { + const TRIO = ['http://localhost:*', 'http://*.localhost:*', 'https://*.localhost:*']; + + const ENV_KEYS = ['NODE_ENV', 'OS_CORS_ORIGIN', 'CORS_ORIGIN', 'OS_SSO_ENABLED'] as const; + let saved: Record; + + beforeEach(() => { + saved = Object.fromEntries(ENV_KEYS.map(k => [k, process.env[k]])); + for (const k of ENV_KEYS) delete process.env[k]; + }); + + afterEach(() => { + for (const k of ENV_KEYS) { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]!; + } + }); + + async function captureConfig(config: Record): Promise { + let capturedConfig: any; + (betterAuth as any).mockImplementation((c: any) => { + capturedConfig = c; + return { handler: vi.fn(), api: {} }; + }); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const manager = new AuthManager({ + secret: 'test-secret-at-least-32-chars-long', + baseUrl: 'https://app.example.com', + ...config, + } as any); + await manager.getAuthInstance(); + warnSpy.mockRestore(); + return capturedConfig; + } + + describe('production does not substitute', () => { + it('omits the trustedOrigins key entirely when none is provided', async () => { + process.env.NODE_ENV = 'production'; + const cfg = await captureConfig({}); + + expect(cfg.trustedOrigins).toBeUndefined(); + // Absent, not merely empty — the key must not appear at all, which is + // the shape better-auth receives. + expect('trustedOrigins' in cfg).toBe(false); + }); + + it('omits the trustedOrigins key entirely when an empty array is provided', async () => { + process.env.NODE_ENV = 'production'; + const cfg = await captureConfig({ trustedOrigins: [] }); + + expect(cfg.trustedOrigins).toBeUndefined(); + expect('trustedOrigins' in cfg).toBe(false); + }); + }); + + describe('non-production still substitutes', () => { + it("substitutes the trio under NODE_ENV='development'", async () => { + process.env.NODE_ENV = 'development'; + const cfg = await captureConfig({}); + + expect(cfg.trustedOrigins).toEqual(TRIO); + }); + + // Guards against "hardening" the predicate to `=== 'development'`, which + // would be a STRICTER boundary than ruled and would break `test` and + // unset-NODE_ENV development flows. + it("substitutes the trio under NODE_ENV='test'", async () => { + process.env.NODE_ENV = 'test'; + const cfg = await captureConfig({}); + + expect(cfg.trustedOrigins).toEqual(TRIO); + }); + + it('substitutes the trio when NODE_ENV is unset', async () => { + delete process.env.NODE_ENV; + const cfg = await captureConfig({}); + + expect(cfg.trustedOrigins).toEqual(TRIO); + }); + }); + + // LOAD-BEARING: without these two legs, a change that broke ALL origin + // trust in production would still pass a substitution-only suite. + describe('production leaves real configured trust intact', () => { + it('forwards an explicitly configured trustedOrigins list unchanged', async () => { + process.env.NODE_ENV = 'production'; + const cfg = await captureConfig({ + trustedOrigins: ['https://app.example.com', 'https://*.example.com'], + }); + + expect(cfg.trustedOrigins).toEqual([ + 'https://app.example.com', + 'https://*.example.com', + ]); + }); + + it('forwards an OS_CORS_ORIGIN-derived list unchanged', async () => { + process.env.NODE_ENV = 'production'; + process.env.OS_CORS_ORIGIN = 'https://app.example.com,https://admin.example.com'; + const cfg = await captureConfig({}); + + expect(cfg.trustedOrigins).toEqual([ + 'https://app.example.com', + 'https://admin.example.com', + ]); + }); + }); + + // The SSO branch returns `trustedOrigins` as a per-request FUNCTION built + // from a copy of the same `origins` array, so gating the push at the source + // covers this shape too. That is true today and is exactly the kind of + // coupling a future refactor breaks silently — pin it. + describe('SSO per-request function shape', () => { + it('production: the resolved list contains no localhost wildcard', async () => { + process.env.NODE_ENV = 'production'; + const cfg = await captureConfig({ plugins: { sso: true } }); + + expect(typeof cfg.trustedOrigins).toBe('function'); + const resolved: string[] = await cfg.trustedOrigins(undefined); + for (const entry of TRIO) expect(resolved).not.toContain(entry); + expect(resolved.some(o => o.includes('localhost'))).toBe(false); + }); + + it('non-production: the resolved list still contains the trio', async () => { + process.env.NODE_ENV = 'development'; + const cfg = await captureConfig({ plugins: { sso: true } }); + + expect(typeof cfg.trustedOrigins).toBe('function'); + const resolved: string[] = await cfg.trustedOrigins(undefined); + for (const entry of TRIO) expect(resolved).toContain(entry); + }); + }); + }); + describe('setRuntimeBaseUrl', () => { it('should update baseURL before auth instance is created', async () => { let capturedConfig: any; diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 2e63bcada1..d6bdba962a 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -1905,7 +1905,28 @@ export class AuthManager { // `*.localhost` subdomains so per-project tenant subdomains (the dev // default root domain — see project-provisioning.ts) pass CSRF checks // without operators having to configure trustedOrigins manually. - if (!origins.length && (!corsOrigin || corsOrigin === '*')) { + // + // NON-PRODUCTION ONLY (#10366). This substitution is a development + // convenience and is now gated on the same `NODE_ENV` dev signal used + // by the fallback auth secret and the dev Origin synthesis below, so + // the boundary this comment claims is the boundary that is enforced. + // Previously the condition tested only emptiness, so a production + // deployment whose trusted-origin list resolved empty silently + // CSRF-trusted every `localhost` / `*.localhost` origin. + // + // What production gets instead: `trustedOrigins` is omitted from the + // better-auth config entirely (see the tail of this block). 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 empty list and an omitted key are equivalent and + // both leave exactly the deployment's own origin trusted; every other + // origin is refused with `403 INVALID_ORIGIN`. Measured against + // better-auth 1.7.1 (`getTrustedOrigins` in `dist/context/helpers.mjs`, + // `validateOrigin` in `dist/api/middlewares/origin-check.mjs`). + if ( + process.env.NODE_ENV !== 'production' && + !origins.length && (!corsOrigin || corsOrigin === '*') + ) { origins.push('http://localhost:*'); origins.push('http://*.localhost:*'); origins.push('https://*.localhost:*'); From 26a8967130614f4867b83c1cd1fcf10e867caba9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 17:42:03 +0000 Subject: [PATCH 2/2] docs(changeset): localhost trusted-origin substitution is non-production only Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .changeset/csrf-localhost-trio-dev-only.md | 45 ++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 .changeset/csrf-localhost-trio-dev-only.md diff --git a/.changeset/csrf-localhost-trio-dev-only.md b/.changeset/csrf-localhost-trio-dev-only.md new file mode 100644 index 0000000000..1ee559580c --- /dev/null +++ b/.changeset/csrf-localhost-trio-dev-only.md @@ -0,0 +1,45 @@ +--- +"@objectstack/plugin-auth": patch +--- + +Gate the localhost trusted-origin substitution to non-production (#10366). + +`AuthManager`'s `trustedOrigins` block substituted a localhost wildcard trio +(`http://localhost:*`, `http://*.localhost:*`, `https://*.localhost:*`) whenever +the resolved trusted-origin list came out empty and `OS_CORS_ORIGIN` was unset +or `*`. Its own comment described this as a development convenience, but the +condition tested only emptiness — it carried no `NODE_ENV` term, no dev-mode +term, nothing. A production deployment that reached it with an empty list +CSRF-trusted every `localhost` and `*.localhost` origin. The declared boundary +and the enforced boundary disagreed, and only the declared one was visible in +the file. + +The substitution is now gated on `NODE_ENV !== 'production'`, the same dev +signal already used by the fallback auth secret and by the dev `Origin` +synthesis in the same file. The property enforced: **a development convenience +exists only outside production.** + +**What production receives instead.** With the trio gated off and the list +empty, the block's tail omits `trustedOrigins` from the better-auth config +entirely. That is not an absent policy. Measured against the installed +better-auth 1.7.1: `getTrustedOrigins` +(`dist/context/helpers.mjs`) unconditionally seeds the trusted set from the +resolved `baseURL` origin and treats `options.trustedOrigins` as purely +**additive**, so an omitted key and an empty array are equivalent — both leave +exactly the deployment's own origin trusted, and `validateOrigin` +(`dist/api/middlewares/origin-check.mjs`) refuses everything else with +`403 INVALID_ORIGIN`. + +**Who is affected.** Deployments with an explicitly configured `trustedOrigins`, +or one derived from `OS_CORS_ORIGIN`, are unchanged in production — the +substitution never fired for them. Non-production behaviour is unchanged, +including under `NODE_ENV=test` and when `NODE_ENV` is unset. A production +deployment that was relying on the substitution to reach its own login page +now receives a loud `403` rather than silent over-trust; the remedy is to set +`OS_TRUSTED_ORIGINS`, or to fix the base URL that resolved unusable (PR #10369's +boot diagnostic already names that condition at startup). + +Both existing pins keep their dev-only assertions verbatim; new pins cover the +production omission, the non-production legs, the SSO per-request-function +shape, and — load-bearing — that explicitly configured and `OS_CORS_ORIGIN`-derived +trust survives in production.