Skip to content

Commit bbe643c

Browse files
os-warrenclaude
andauthored
fix(auth): gate the localhost trusted-origin substitution to non-production (#11115)
* 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx * docs(changeset): localhost trusted-origin substitution is non-production only Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 795ea05 commit bbe643c

3 files changed

Lines changed: 207 additions & 1 deletion

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
---
4+
5+
Gate the localhost trusted-origin substitution to non-production (#10366).
6+
7+
`AuthManager`'s `trustedOrigins` block substituted a localhost wildcard trio
8+
(`http://localhost:*`, `http://*.localhost:*`, `https://*.localhost:*`) whenever
9+
the resolved trusted-origin list came out empty and `OS_CORS_ORIGIN` was unset
10+
or `*`. Its own comment described this as a development convenience, but the
11+
condition tested only emptiness — it carried no `NODE_ENV` term, no dev-mode
12+
term, nothing. A production deployment that reached it with an empty list
13+
CSRF-trusted every `localhost` and `*.localhost` origin. The declared boundary
14+
and the enforced boundary disagreed, and only the declared one was visible in
15+
the file.
16+
17+
The substitution is now gated on `NODE_ENV !== 'production'`, the same dev
18+
signal already used by the fallback auth secret and by the dev `Origin`
19+
synthesis in the same file. The property enforced: **a development convenience
20+
exists only outside production.**
21+
22+
**What production receives instead.** With the trio gated off and the list
23+
empty, the block's tail omits `trustedOrigins` from the better-auth config
24+
entirely. That is not an absent policy. Measured against the installed
25+
better-auth 1.7.1: `getTrustedOrigins`
26+
(`dist/context/helpers.mjs`) unconditionally seeds the trusted set from the
27+
resolved `baseURL` origin and treats `options.trustedOrigins` as purely
28+
**additive**, so an omitted key and an empty array are equivalent — both leave
29+
exactly the deployment's own origin trusted, and `validateOrigin`
30+
(`dist/api/middlewares/origin-check.mjs`) refuses everything else with
31+
`403 INVALID_ORIGIN`.
32+
33+
**Who is affected.** Deployments with an explicitly configured `trustedOrigins`,
34+
or one derived from `OS_CORS_ORIGIN`, are unchanged in production — the
35+
substitution never fired for them. Non-production behaviour is unchanged,
36+
including under `NODE_ENV=test` and when `NODE_ENV` is unset. A production
37+
deployment that was relying on the substitution to reach its own login page
38+
now receives a loud `403` rather than silent over-trust; the remedy is to set
39+
`OS_TRUSTED_ORIGINS`, or to fix the base URL that resolved unusable (PR #10369's
40+
boot diagnostic already names that condition at startup).
41+
42+
Both existing pins keep their dev-only assertions verbatim; new pins cover the
43+
production omission, the non-production legs, the SSO per-request-function
44+
shape, and — load-bearing — that explicitly configured and `OS_CORS_ORIGIN`-derived
45+
trust survives in production.

packages/plugins/plugin-auth/src/auth-manager.test.ts

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,6 +1315,146 @@ describe('AuthManager', () => {
13151315
});
13161316
});
13171317

1318+
// #10366 — the localhost-wildcard trio is a DEVELOPMENT convenience and is
1319+
// gated on `NODE_ENV !== 'production'`. Before the gate the condition tested
1320+
// only emptiness, so a production deployment whose trusted-origin list
1321+
// resolved empty silently CSRF-trusted every `localhost` / `*.localhost`
1322+
// origin. These pins enforce the boundary in BOTH directions: production
1323+
// must not substitute, and non-production must still substitute.
1324+
describe('trustedOrigins localhost substitution is non-production only (#10366)', () => {
1325+
const TRIO = ['http://localhost:*', 'http://*.localhost:*', 'https://*.localhost:*'];
1326+
1327+
const ENV_KEYS = ['NODE_ENV', 'OS_CORS_ORIGIN', 'CORS_ORIGIN', 'OS_SSO_ENABLED'] as const;
1328+
let saved: Record<string, string | undefined>;
1329+
1330+
beforeEach(() => {
1331+
saved = Object.fromEntries(ENV_KEYS.map(k => [k, process.env[k]]));
1332+
for (const k of ENV_KEYS) delete process.env[k];
1333+
});
1334+
1335+
afterEach(() => {
1336+
for (const k of ENV_KEYS) {
1337+
if (saved[k] === undefined) delete process.env[k];
1338+
else process.env[k] = saved[k]!;
1339+
}
1340+
});
1341+
1342+
async function captureConfig(config: Record<string, unknown>): Promise<any> {
1343+
let capturedConfig: any;
1344+
(betterAuth as any).mockImplementation((c: any) => {
1345+
capturedConfig = c;
1346+
return { handler: vi.fn(), api: {} };
1347+
});
1348+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
1349+
const manager = new AuthManager({
1350+
secret: 'test-secret-at-least-32-chars-long',
1351+
baseUrl: 'https://app.example.com',
1352+
...config,
1353+
} as any);
1354+
await manager.getAuthInstance();
1355+
warnSpy.mockRestore();
1356+
return capturedConfig;
1357+
}
1358+
1359+
describe('production does not substitute', () => {
1360+
it('omits the trustedOrigins key entirely when none is provided', async () => {
1361+
process.env.NODE_ENV = 'production';
1362+
const cfg = await captureConfig({});
1363+
1364+
expect(cfg.trustedOrigins).toBeUndefined();
1365+
// Absent, not merely empty — the key must not appear at all, which is
1366+
// the shape better-auth receives.
1367+
expect('trustedOrigins' in cfg).toBe(false);
1368+
});
1369+
1370+
it('omits the trustedOrigins key entirely when an empty array is provided', async () => {
1371+
process.env.NODE_ENV = 'production';
1372+
const cfg = await captureConfig({ trustedOrigins: [] });
1373+
1374+
expect(cfg.trustedOrigins).toBeUndefined();
1375+
expect('trustedOrigins' in cfg).toBe(false);
1376+
});
1377+
});
1378+
1379+
describe('non-production still substitutes', () => {
1380+
it("substitutes the trio under NODE_ENV='development'", async () => {
1381+
process.env.NODE_ENV = 'development';
1382+
const cfg = await captureConfig({});
1383+
1384+
expect(cfg.trustedOrigins).toEqual(TRIO);
1385+
});
1386+
1387+
// Guards against "hardening" the predicate to `=== 'development'`, which
1388+
// would be a STRICTER boundary than ruled and would break `test` and
1389+
// unset-NODE_ENV development flows.
1390+
it("substitutes the trio under NODE_ENV='test'", async () => {
1391+
process.env.NODE_ENV = 'test';
1392+
const cfg = await captureConfig({});
1393+
1394+
expect(cfg.trustedOrigins).toEqual(TRIO);
1395+
});
1396+
1397+
it('substitutes the trio when NODE_ENV is unset', async () => {
1398+
delete process.env.NODE_ENV;
1399+
const cfg = await captureConfig({});
1400+
1401+
expect(cfg.trustedOrigins).toEqual(TRIO);
1402+
});
1403+
});
1404+
1405+
// LOAD-BEARING: without these two legs, a change that broke ALL origin
1406+
// trust in production would still pass a substitution-only suite.
1407+
describe('production leaves real configured trust intact', () => {
1408+
it('forwards an explicitly configured trustedOrigins list unchanged', async () => {
1409+
process.env.NODE_ENV = 'production';
1410+
const cfg = await captureConfig({
1411+
trustedOrigins: ['https://app.example.com', 'https://*.example.com'],
1412+
});
1413+
1414+
expect(cfg.trustedOrigins).toEqual([
1415+
'https://app.example.com',
1416+
'https://*.example.com',
1417+
]);
1418+
});
1419+
1420+
it('forwards an OS_CORS_ORIGIN-derived list unchanged', async () => {
1421+
process.env.NODE_ENV = 'production';
1422+
process.env.OS_CORS_ORIGIN = 'https://app.example.com,https://admin.example.com';
1423+
const cfg = await captureConfig({});
1424+
1425+
expect(cfg.trustedOrigins).toEqual([
1426+
'https://app.example.com',
1427+
'https://admin.example.com',
1428+
]);
1429+
});
1430+
});
1431+
1432+
// The SSO branch returns `trustedOrigins` as a per-request FUNCTION built
1433+
// from a copy of the same `origins` array, so gating the push at the source
1434+
// covers this shape too. That is true today and is exactly the kind of
1435+
// coupling a future refactor breaks silently — pin it.
1436+
describe('SSO per-request function shape', () => {
1437+
it('production: the resolved list contains no localhost wildcard', async () => {
1438+
process.env.NODE_ENV = 'production';
1439+
const cfg = await captureConfig({ plugins: { sso: true } });
1440+
1441+
expect(typeof cfg.trustedOrigins).toBe('function');
1442+
const resolved: string[] = await cfg.trustedOrigins(undefined);
1443+
for (const entry of TRIO) expect(resolved).not.toContain(entry);
1444+
expect(resolved.some(o => o.includes('localhost'))).toBe(false);
1445+
});
1446+
1447+
it('non-production: the resolved list still contains the trio', async () => {
1448+
process.env.NODE_ENV = 'development';
1449+
const cfg = await captureConfig({ plugins: { sso: true } });
1450+
1451+
expect(typeof cfg.trustedOrigins).toBe('function');
1452+
const resolved: string[] = await cfg.trustedOrigins(undefined);
1453+
for (const entry of TRIO) expect(resolved).toContain(entry);
1454+
});
1455+
});
1456+
});
1457+
13181458
describe('setRuntimeBaseUrl', () => {
13191459
it('should update baseURL before auth instance is created', async () => {
13201460
let capturedConfig: any;

packages/plugins/plugin-auth/src/auth-manager.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1905,7 +1905,28 @@ export class AuthManager {
19051905
// `*.localhost` subdomains so per-project tenant subdomains (the dev
19061906
// default root domain — see project-provisioning.ts) pass CSRF checks
19071907
// without operators having to configure trustedOrigins manually.
1908-
if (!origins.length && (!corsOrigin || corsOrigin === '*')) {
1908+
//
1909+
// NON-PRODUCTION ONLY (#10366). This substitution is a development
1910+
// convenience and is now gated on the same `NODE_ENV` dev signal used
1911+
// by the fallback auth secret and the dev Origin synthesis below, so
1912+
// the boundary this comment claims is the boundary that is enforced.
1913+
// Previously the condition tested only emptiness, so a production
1914+
// deployment whose trusted-origin list resolved empty silently
1915+
// CSRF-trusted every `localhost` / `*.localhost` origin.
1916+
//
1917+
// What production gets instead: `trustedOrigins` is omitted from the
1918+
// better-auth config entirely (see the tail of this block). That is
1919+
// NOT an absent policy — better-auth seeds its trusted set from the
1920+
// resolved `baseURL` origin and treats `trustedOrigins` as purely
1921+
// ADDITIVE, so an empty list and an omitted key are equivalent and
1922+
// both leave exactly the deployment's own origin trusted; every other
1923+
// origin is refused with `403 INVALID_ORIGIN`. Measured against
1924+
// better-auth 1.7.1 (`getTrustedOrigins` in `dist/context/helpers.mjs`,
1925+
// `validateOrigin` in `dist/api/middlewares/origin-check.mjs`).
1926+
if (
1927+
process.env.NODE_ENV !== 'production' &&
1928+
!origins.length && (!corsOrigin || corsOrigin === '*')
1929+
) {
19091930
origins.push('http://localhost:*');
19101931
origins.push('http://*.localhost:*');
19111932
origins.push('https://*.localhost:*');

0 commit comments

Comments
 (0)