diff --git a/packages/cli/test/artifact-pinned-boot.e2e.test.ts b/packages/cli/test/artifact-pinned-boot.e2e.test.ts index 82fe269fa5..561b32f9d5 100644 --- a/packages/cli/test/artifact-pinned-boot.e2e.test.ts +++ b/packages/cli/test/artifact-pinned-boot.e2e.test.ts @@ -40,6 +40,7 @@ import { tmpdir } from 'node:os'; import { join, resolve, dirname } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; import { PROTOCOL_MAJOR } from '@objectstack/spec/kernel'; +import { childEnv } from './helpers/serve-process.js'; const HERE = dirname(fileURLToPath(import.meta.url)); const CLI = resolve(HERE, '../bin/run-dev.js'); @@ -122,8 +123,15 @@ function runServe(env: Record, opts: { migrateAndExit?: boolean ['--import', TSX_LOADER, CLI, 'serve'], { cwd, - env: { - ...process.env, + // `childEnv`, not a bare `...process.env`: the vitest worker + // exports `TEST=true`, and better-auth 1.7.1 reads it directly + // to switch its own origin/CSRF validation OFF in the child — + // see `helpers/serve-process.ts` for the measurement (#11267). + // This boot exits at `kernel:ready` (`OS_MIGRATE_AND_EXIT`) and + // never answers a request, so nothing here CHANGES; it is the + // hygiene half, so the next assertion added to this file starts + // from a child that is not lying about being a test runner. + env: childEnv({ NODE_ENV: 'production', OS_HOME: home, OS_DATABASE_URL: `file:${join(home, 'e2e.db')}`, @@ -136,7 +144,7 @@ function runServe(env: Record, opts: { migrateAndExit?: boolean // container carrying no app does not have. OS_ARTIFACT_PATH: join(cwd, 'dist/objectstack.json'), ...env, - }, + }), stdio: ['ignore', 'pipe', 'pipe'], }, ); diff --git a/packages/cli/test/helpers/serve-process.ts b/packages/cli/test/helpers/serve-process.ts index 3c9dffa506..b23b38d5ea 100644 --- a/packages/cli/test/helpers/serve-process.ts +++ b/packages/cli/test/helpers/serve-process.ts @@ -25,6 +25,139 @@ export function randomPort(): string { return String(40000 + Math.floor(Math.random() * 20000)); } +/** + * The variables vitest sets on its own WORKER process, which must never reach a + * spawned `os serve` child (#11267). + * + * ## Why this exists — measured, not defensive + * + * A child built with `{ ...process.env, … }` inherits the **vitest worker's** + * environment, and vitest sets `TEST=true` on that worker unconditionally, + * independent of `NODE_ENV`. better-auth 1.7.1 reads `TEST` **directly**: + * + * ```js + * // @better-auth/core/dist/env/env-impl.mjs:36 + * const isTest = () => nodeENV === "test" || toBoolean(env.TEST); + * // better-auth/dist/context/create-context.mjs:210 + * skipOriginCheck: options.advanced?.disableOriginCheck !== void 0 + * ? options.advanced.disableOriginCheck + * : isTest() ? true : false, + * ``` + * + * So an inherited `TEST=true` disables better-auth's origin/CSRF validation + * **entirely**, one layer below anything `serve.ts` or `plugin-auth` decide, + * and independent of whatever `NODE_ENV` the caller sets on the child. The + * dangerous direction is not a red test: it is a security-posture assertion + * that can never go red for the reason it exists, which reads as coverage. + * + * MEASURED on a real boot through this helper's own spawn recipe — same + * fixture, same code, the five variables below the only difference. Probe: + * `POST /api/v1/auth/sign-in/email` with `Origin: https://evil.example.com` + * (untrusted under every branch of `serve.ts`'s trusted-origin assembly, + * including the `isDev` `http://localhost:*` convenience that `run-dev.js` + * always turns on): + * + * | child env | answer | + * |---|---| + * | `{ ...process.env }` (this helper, before #11267) | `401 INVALID_EMAIL_OR_PASSWORD` — origin ACCEPTED, validation never ran | + * | family below stripped | `403 INVALID_ORIGIN` — validation ran and rejected | + * | only `TEST` stripped | `403 INVALID_ORIGIN` | + * + * The third row is the isolation for THAT probe: `TEST` alone is what + * better-auth reads. + * + * ## ⚠️ `VITEST` is NOT cosmetic either — a claim this file got wrong once + * + * The first revision of this header said the `VITEST*` entries were stripped + * as hygiene, "nothing in `os serve` reads them today". That was **false**, and + * CI found the counterexample: + * + * ```ts + * // packages/services/service-settings/src/local-crypto-provider.ts:133 + * const detectMode = (env: EnvMap): CryptoMode => { + * if (env.VITEST || env.NODE_ENV === 'test') return 'test'; + * if (env.NODE_ENV === 'production') return 'production'; + * return 'development'; + * }; + * ``` + * + * So an inherited `VITEST=true` put every spawned child's crypto layer in + * `test` mode — ephemeral key, never touches disk, never refuses — no matter + * what posture the rest of the boot was in. That is the SAME defect class as + * the `TEST` leak one layer over: a security-relevant gate (here, stable + * encryption-key enforcement) softened by a variable the child inherited from + * the test runner rather than by anything the code under test decided. + * Stripping `VITEST` is therefore load-bearing in its own right, and the + * `serve-node-env-production-default` pin going red the moment it stopped + * leaking is the gate working, not the gate misfiring: that fixture's + * "production posture" had been genuine for auth and fake for crypto. + * + * The consequence is why `OS_SECRET_KEY` is a default below. Once the child + * stops claiming to be a vitest worker, `detectMode` answers `development` + * for the ordinary boots here, and development mode **persists** a minted key + * to `$HOME/.objectstack/dev-crypto-key`. Measured: with that file absent a + * production-posture boot refuses to start, and with it present — put there by + * any earlier dev-mode boot in the same run — the same boot succeeds. That is + * a cross-test ordering coupling through the runner's home directory, and + * under vitest's parallel workers it is nondeterministic. An explicit key + * removes both halves: nothing is written, and nothing is depended on. + * + * ⛔ `NODE_ENV` is deliberately NOT in this family. The vitest worker exports + * `NODE_ENV=test` too, but every caller here already pins the child's + * `NODE_ENV` explicitly (`bin/run-dev.js` sets `development` before argv is + * even parsed; the `bin/run.js` spawners pass it in `env`), so stripping it + * would change which entrypoint those tests resolve through rather than remove + * a leak. That is a different defect with its own card (#11317) — ⛔ do not + * fold it in here. + */ +/** + * A fixed, obviously-synthetic 32-byte key (64 hex chars) for spawned children, + * so no test boot has to mint one — see `runServe()` and the header above. + * ⛔ Test fixtures only; it is in the repo in plaintext and encrypts nothing + * anyone keeps. + */ +export const E2E_SECRET_KEY = '0e2e'.repeat(16); + +export const VITEST_WORKER_ENV_KEYS = [ + 'TEST', + 'VITEST', + 'VITEST_WORKER_ID', + 'VITEST_POOL_ID', + 'VITEST_MODE', +] as const; + +/** `TEST` exactly, or any `VITEST`-prefixed variable — see `childEnv()`. */ +function isVitestWorkerKey(key: string): boolean { + return key === 'TEST' || key === 'VITEST' || key.startsWith('VITEST_'); +} + +/** + * Build the environment for a spawned CLI child: this process's environment + * minus the vitest worker family above, plus `overrides`. + * + * The strip is a **class**, not the fixed list: `TEST` exactly, plus anything + * matching `VITEST`/`VITEST_*`. `VITEST_WORKER_ENV_KEYS` names the five that + * vitest 4 exports today (and is what the pin asserts against), but a future + * runner variable in that namespace is caught without anyone having to + * rediscover this trap first. + * + * `overrides` is applied AFTER the strip, so a test that genuinely wants one of + * these set in its child can still say so explicitly — the point is that + * nothing arrives by accident. An `undefined` value UNSETS a variable for the + * child: Node's `spawn()` omits `undefined`-valued entries rather than + * stringifying them, which `''` would not do. + */ +export function childEnv( + overrides: Record = {}, +): Record { + const env: Record = {}; + for (const [key, value] of Object.entries(process.env)) { + if (isVitestWorkerKey(key)) continue; + env[key] = value; + } + return { ...env, ...overrides }; +} + export interface ServeRun { stdout: string; stderr: string; @@ -56,16 +189,22 @@ export function runServe( return new Promise((resolveRun, rejectRun) => { const child = spawn(TSX, [CLI, 'serve', opts.config ?? 'objectstack.config.ts', ...args], { cwd, - env: { - ...process.env, + // `childEnv`, never a bare `...process.env` — see its header for the + // measured reason (#11267). + env: childEnv({ NO_COLOR: '1', // Keep the fixture self-contained: no file written, no port conflict // with another agent's dev server, no inherited log level. OS_DATABASE_URL: ':memory:', OS_LOG_LEVEL: '', OS_DISABLE_CONSOLE: '1', + // Same "no file written" rule, extended to the crypto key — see the + // header. Without this the child mints one and PERSISTS it to + // `$HOME/.objectstack/dev-crypto-key`, which both litters the runner's + // home directory and couples unrelated tests to each other through it. + OS_SECRET_KEY: E2E_SECRET_KEY, ...(opts.env ?? {}), - }, + }), }); let stdout = ''; diff --git a/packages/cli/test/serve-mcp-capability-collision.e2e.test.ts b/packages/cli/test/serve-mcp-capability-collision.e2e.test.ts index 017b87609f..311d6f4b39 100644 --- a/packages/cli/test/serve-mcp-capability-collision.e2e.test.ts +++ b/packages/cli/test/serve-mcp-capability-collision.e2e.test.ts @@ -48,7 +48,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { randomPort } from './helpers/serve-process.js'; +import { E2E_SECRET_KEY, childEnv, randomPort } from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); /** `bin/run.js` — the SHIPPED entrypoint, i.e. the one the card's repro names. */ @@ -109,16 +109,27 @@ function boot(env: Record, waitFor: RegExp): Promise const child = spawn(process.execPath, [CLI, 'serve', '-p', port, '--dev'], { cwd: dir, stdio: ['pipe', 'pipe', 'pipe'], - env: { - ...process.env, + // `childEnv`, not a bare `...process.env`: the vitest worker exports + // `TEST=true`, which better-auth 1.7.1 reads directly and answers by + // switching its own origin/CSRF validation OFF in the child — see + // `helpers/serve-process.ts` for the measurement (#11267). This file + // signs in for real, so it is a child that actually reaches that code. + env: childEnv({ NO_COLOR: '1', OS_LOG_LEVEL: 'info', OS_DISABLE_CONSOLE: '1', + // Explicit, not minted: with `VITEST` no longer inherited (#11267), + // `local-crypto-provider.ts`'s detectMode answers `development` for + // this child instead of `test`, and development mode PERSISTS a minted + // key to `$HOME/.objectstack/dev-crypto-key`. Supplying one keeps this + // boot from writing to the runner's home directory and from coupling + // itself to whatever other test got there first. + OS_SECRET_KEY: E2E_SECRET_KEY, // The dev-admin seed the key mint signs in as is gated on this, and // vitest exports `test`. NODE_ENV: 'development', ...env, - }, + }), }) as ChildProcessWithoutNullStreams; children.push(child); diff --git a/packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts b/packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts index 7d3de7ff75..eb3bd9f5bf 100644 --- a/packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts +++ b/packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts @@ -47,7 +47,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { randomPort } from './helpers/serve-process.js'; +import { E2E_SECRET_KEY, childEnv, randomPort } from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); /** `bin/run.js` — the SHIPPED entrypoint, i.e. the one the card's repro names. */ @@ -104,17 +104,28 @@ function boot(env: Record, waitFor: RegExp): Promise const child = spawn(process.execPath, [CLI, 'serve', '-p', port, '--dev'], { cwd: dir, stdio: ['pipe', 'pipe', 'pipe'], - env: { - ...process.env, + // `childEnv`, not a bare `...process.env`: the vitest worker exports + // `TEST=true`, which better-auth 1.7.1 reads directly and answers by + // switching its own origin/CSRF validation OFF in the child — see + // `helpers/serve-process.ts` for the measurement (#11267). This file + // signs in for real, so it is a child that actually reaches that code. + env: childEnv({ NO_COLOR: '1', OS_LOG_LEVEL: 'info', OS_DISABLE_CONSOLE: '1', + // Explicit, not minted: with `VITEST` no longer inherited (#11267), + // `local-crypto-provider.ts`'s detectMode answers `development` for + // this child instead of `test`, and development mode PERSISTS a minted + // key to `$HOME/.objectstack/dev-crypto-key`. Supplying one keeps this + // boot from writing to the runner's home directory and from coupling + // itself to whatever other test got there first. + OS_SECRET_KEY: E2E_SECRET_KEY, // Explicit, not inherited: the dev-admin seed this fixture signs in as // is hard-gated on `NODE_ENV === 'development'`, and vitest exports // `test`, which would leave the DB user-less and the mint unauthorized. NODE_ENV: 'development', ...env, - }, + }), }) as ChildProcessWithoutNullStreams; children.push(child); diff --git a/packages/cli/test/serve-node-env-production-default.e2e.test.ts b/packages/cli/test/serve-node-env-production-default.e2e.test.ts index bfd3e838ae..87a8e819f3 100644 --- a/packages/cli/test/serve-node-env-production-default.e2e.test.ts +++ b/packages/cli/test/serve-node-env-production-default.e2e.test.ts @@ -130,6 +130,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { join, resolve } from 'node:path'; import type { Readable } from 'node:stream'; import { fileURLToPath } from 'node:url'; +import { childEnv, E2E_SECRET_KEY } from './helpers/serve-process.js'; /** What `spawn(..., { stdio: ['ignore', 'pipe', 'pipe'] })` actually returns — no `stdin`. */ type ProbeChild = ChildProcessByStdio; @@ -210,8 +211,7 @@ async function probeOriginCheck(env: Record): Promis const child = spawn(process.execPath, [CLI, 'serve', '-p', String(port)], { cwd: dir, stdio: ['ignore', 'pipe', 'pipe'], - env: { - ...process.env, + env: childEnv({ NO_COLOR: '1', OS_LOG_LEVEL: 'warn', OS_DISABLE_CONSOLE: '1', @@ -221,27 +221,47 @@ async function probeOriginCheck(env: Record): Promis // "AuthPlugin.init() throws: secret is required" path regardless of // which NODE_ENV state this call is probing. OS_AUTH_SECRET: 'e2e-node-env-default-probe-secret-not-for-real-use', + // EXACTLY the argument the line above makes, for the sibling gate that + // #11267 exposed. The unset-`NODE_ENV` leg is — by this file's whole + // design — a PRODUCTION boot, and `LocalCryptoProvider` refuses to start + // in production without a stable key rather than mint one that would + // make every `sys_secret` value undecryptable after a restart. That + // refusal is a boot failure, not a signal about the origin gate this + // file measures, so the key is supplied explicitly. + // + // ⚠️ It was NOT needed before #11267 — and that is the finding, not an + // inconvenience. `local-crypto-provider.ts:133` reads + // `if (env.VITEST || env.NODE_ENV === 'test') return 'test'`, so while + // this fixture still inherited the vitest worker's `VITEST=true`, its + // crypto layer sat in TEST mode (ephemeral key, no disk, no refusal) + // while the rest of the boot was in production posture. The production + // posture this file exists to pin was genuine for auth and fake for + // crypto. Supplying the key is what makes it genuine for both. + OS_SECRET_KEY: E2E_SECRET_KEY, // The base default for every call: truly unset, unless overridden by // `env` below. Node's spawn omits an `undefined`-valued entry rather // than inheriting whatever this test RUNNER's own process (vitest sets // NODE_ENV=test) happened to have. NODE_ENV: undefined, - // MEASURED TRAP, worth stating explicitly: `...process.env` above is - // THIS FILE's own process env — the vitest WORKER's — and vitest's - // worker carries `TEST=true` (and `VITEST=true`) regardless of - // `NODE_ENV`. better-auth 1.7.1 reads `TEST` directly, independent of - // `NODE_ENV`: `create-context.mjs` defaults + // MEASURED TRAP, and the reason the base above is `childEnv()` rather + // than `...process.env`: this file's own process env is the vitest + // WORKER's, and that worker carries `TEST=true` (and `VITEST=true`) + // regardless of `NODE_ENV`. better-auth 1.7.1 reads `TEST` directly, + // independent of `NODE_ENV`: `create-context.mjs` defaults // `skipOriginCheck: … isTest() ? true : false`, and // `isTest = () => nodeENV === 'test' || toBoolean(env.TEST)`. Left // alone, that inherited `TEST=true` makes better-auth skip origin // validation ENTIRELY — a false GREEN that has nothing to do with // `serve.ts`'s own gate and stays green with the fix reverted, which is // exactly the vacuity this card's anti-vacuity section warns against, - // one layer further down than the one it names. Unset it the same way - // `NODE_ENV` is unset above, for the same reason. - TEST: undefined, + // one layer further down than the one it names. This file used to unset + // `TEST` by hand right here; #11267 moved that into `childEnv()` so + // every spawner in this directory gets it without having to know, and + // widened it to the whole `VITEST*` family. The behaviour of this + // fixture is unchanged — `childEnv()` removes a superset of what the + // hand-written `TEST: undefined` removed. ...env, - }, + }), }) as ProbeChild; children.push(child); diff --git a/packages/cli/test/serve-process-child-env.e2e.test.ts b/packages/cli/test/serve-process-child-env.e2e.test.ts new file mode 100644 index 0000000000..4b3b63ec29 --- /dev/null +++ b/packages/cli/test/serve-process-child-env.e2e.test.ts @@ -0,0 +1,258 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #11267 — a spawned `os serve` child must not inherit the vitest worker's + * `TEST=true`, because better-auth switches its own origin/CSRF validation OFF + * when it sees it. + * + * ## What this file pins, and why it takes a real boot to pin it + * + * The fix itself is four lines in `helpers/serve-process.ts` (`childEnv()` + * drops `TEST` and the `VITEST*` family before the overrides go on). A pin that + * only asserted "the returned object has no `TEST` key" would be true and + * almost worthless: it says nothing about whether that key ever mattered, and + * this card exists precisely because that distinction was invisible. So the + * structural assertions below are followed by two REAL boots that differ in + * nothing but those variables. + * + * ## The measurement, which is what the two boots re-run on every CI pass + * + * Probe: `POST /api/v1/auth/sign-in/email` carrying + * `Origin: https://evil.example.com`, no cookie, no `Sec-Fetch-*`. That is the + * shape `validateFormCsrf` forces an origin check for (`origin-check.mjs`: no + * cookie and no Fetch-Metadata, but an `origin` header present ⇒ + * `validateOrigin(ctx, true)`). The origin is not localhost, so it is untrusted + * under EVERY branch of `serve.ts`'s trusted-origin assembly — including the + * `isDev` `http://localhost:*` convenience, which `bin/run-dev.js` always turns + * on by setting `NODE_ENV=development` before argv is parsed. A pass therefore + * cannot be explained by the deployment trusting its own origin. + * + * | child env | answer | + * |---|---| + * | `{ ...process.env, … }` — this directory's shape before #11267 | `401 INVALID_EMAIL_OR_PASSWORD` — origin ACCEPTED, validation never ran | + * | `childEnv({ … })` | `403 INVALID_ORIGIN` — validation ran and rejected | + * + * Isolated when this was measured: stripping ONLY `TEST` (leaving `VITEST`, + * `VITEST_WORKER_ID`, `VITEST_POOL_ID`, `VITEST_MODE` in place) also answers + * `403 INVALID_ORIGIN`, so `TEST` alone is what better-auth reads. + * + * ⚠️ `VITEST` is not merely hygiene either, though this file said so in its + * first revision and was wrong: `local-crypto-provider.ts:133` reads it + * (`if (env.VITEST || env.NODE_ENV === 'test') return 'test'`) and an + * inherited one silently put a spawned child's crypto layer in test mode. Same + * class, different gate. `helpers/serve-process.ts` carries the measurement. + * + * ## ⚠️ The first boot deliberately builds the env the WRONG way + * + * `leakedEnv()` below is a bare `...process.env` spread on purpose — it is the + * pre-#11267 recipe, kept executable so the repair stays distinguishable from a + * no-op. ⛔ Do not "clean it up" to `childEnv()`: that would delete the only + * evidence in the repo that the leak does anything, and leave a green suite + * behind. It is also the canary on the dependency — if better-auth stops + * reading `TEST`, that leg goes red, and the answer is to re-read this header + * and re-measure, not to silence it. + * + * Cost: two real `os serve` boots, ~18s together when measured on this + * container. Both children are killed in `afterAll` regardless of outcome. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { spawn, type ChildProcessByStdio } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { Readable } from 'node:stream'; +import { CLI, TSX, E2E_SECRET_KEY, childEnv, randomPort, VITEST_WORKER_ENV_KEYS } from './helpers/serve-process.js'; + +/** What `spawn(…, { stdio: ['ignore', 'pipe', 'pipe'] })` actually returns — no `stdin`. */ +type ProbeChild = ChildProcessByStdio; + +/** + * No `plugins` key: this is the shape that leaves `serve.ts` to auto-inject + * `AuthPlugin` (`!hasAuthPlugin && tierEnabled('auth')`), which is what mounts + * better-auth at `/api/v1/auth/*`. The object exists only so the stack is a + * valid app; nothing below reads it. + */ +const CONFIG = ` +export default { + manifest: { + id: 'com.example.childenv', + namespace: 'childenv', + version: '1.0.0', + type: 'app', + name: 'childEnv origin-validation probe', + }, + objects: [{ + name: 'childenv_task', + label: 'Task', + sharingModel: 'public', + fields: { title: { type: 'text', label: 'Title' } }, + }], +}; +`; + +/** The overrides both legs share, so the ONLY difference between them is the base. */ +const OVERRIDES = { + NO_COLOR: '1', + OS_DATABASE_URL: ':memory:', + OS_LOG_LEVEL: 'warn', + OS_DISABLE_CONSOLE: '1', + // Explicit rather than leaning on serve.ts's isDev fallback secret, so a + // change to that fallback can never turn this into a boot-failure test. + OS_AUTH_SECRET: 'e2e-child-env-probe-secret-not-for-real-use', + // Explicit for the same reason, and it stays in the SHARED overrides so the + // two legs still differ in nothing but the vitest env family — which is the + // property the whole comparison rests on. + OS_SECRET_KEY: E2E_SECRET_KEY, +}; + +/** + * ⚠️ The PRE-#11267 recipe, on purpose. See this file's header before touching + * it — it is the leg that proves the leak does something. + */ +function leakedEnv(): Record { + return { ...process.env, ...OVERRIDES }; +} + +let dir: string; +const children: ProbeChild[] = []; + +async function stop(child: ProbeChild): Promise { + if (child.exitCode !== null || child.signalCode !== null) return; + await new Promise((done) => { + const give = setTimeout(() => { + try { child.kill('SIGKILL'); } catch { /* already gone */ } + done(); + }, 10_000); + child.once('exit', () => { clearTimeout(give); done(); }); + try { + child.kill('SIGTERM'); + } catch { + clearTimeout(give); + done(); + } + }); +} + +/** + * Boot the real `os serve` through the same entrypoint `runServe()` uses, probe + * the origin check while it is UP, then stop it. `runServe()` itself cannot + * stand in here: it kills the child the moment `waitFor` matches, so there is + * no window in which to send a request. + */ +async function probeOrigin(env: Record): Promise<{ status: number; code: unknown }> { + const port = randomPort(); + const child = spawn(TSX, [CLI, 'serve', 'objectstack.config.ts', '--port', port], { + cwd: dir, + stdio: ['ignore', 'pipe', 'pipe'], + env, + }) as ProbeChild; + children.push(child); + + let out = ''; + let err = ''; + await new Promise((ready, fail) => { + const timer = setTimeout( + () => fail(new Error(`serve never became ready\n--- stdout ---\n${out}\n--- stderr ---\n${err}`)), + 150_000, + ); + const onData = () => { + if (/Press Ctrl\+C to stop|Server is ready/.test(out + err)) { + clearTimeout(timer); + ready(); + } + }; + child.stdout.on('data', (d) => { out += String(d); onData(); }); + child.stderr.on('data', (d) => { err += String(d); onData(); }); + child.on('exit', (code) => { + clearTimeout(timer); + fail(new Error(`serve exited ${code} before it was ready\n--- stdout ---\n${out}\n--- stderr ---\n${err}`)); + }); + }); + + try { + const res = await fetch(`http://localhost:${port}/api/v1/auth/sign-in/email`, { + method: 'POST', + headers: { + 'content-type': 'application/json', + // Not localhost, so untrusted under every branch of serve.ts's + // trusted-origin assembly. No cookie and no Sec-Fetch-* header: that is + // the shape validateFormCsrf forces an origin check for. + origin: 'https://evil.example.com', + }, + body: JSON.stringify({ email: 'nobody@example.com', password: 'definitely-wrong-password' }), + }); + let body: any = null; + try { body = await res.json(); } catch { /* non-JSON body, fall through */ } + return { status: res.status, code: body?.code }; + } finally { + await stop(child); + } +} + +describe('#11267: childEnv() keeps the vitest worker out of a spawned os serve', () => { + it('drops every variable in the vitest worker family', () => { + const env = childEnv(); + for (const key of VITEST_WORKER_ENV_KEYS) { + expect(Object.hasOwn(env, key), `childEnv() still carries ${key}`).toBe(false); + } + }); + + it('drops the whole VITEST namespace, not just the five names known today', () => { + const env = childEnv(); + const leaked = Object.keys(env).filter((k) => k === 'TEST' || k.startsWith('VITEST')); + expect(leaked).toEqual([]); + }); + + it('still carries the rest of the environment, and lets an override win', () => { + const env = childEnv({ TEST: 'deliberate', OS_LOG_LEVEL: 'warn' }); + // PATH is the one variable a spawned child cannot do without. + expect(env.PATH).toBe(process.env.PATH); + // Overrides are applied AFTER the strip: a test that genuinely wants one of + // these can still say so — the point is that nothing arrives by accident. + expect(env.TEST).toBe('deliberate'); + expect(env.OS_LOG_LEVEL).toBe('warn'); + }); + + it('an `undefined` override survives as an own key, so spawn() unsets it', () => { + const env = childEnv({ NODE_ENV: undefined }); + expect(Object.hasOwn(env, 'NODE_ENV')).toBe(true); + expect(env.NODE_ENV).toBeUndefined(); + }); + + describe('and that is not cosmetic — the same boot, the same probe, two answers', () => { + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'os-child-env-probe-')); + writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8'); + }); + + afterAll(async () => { + for (const child of children) await stop(child); + if (dir) rmSync(dir, { recursive: true, force: true }); + }, 60_000); + + it( + 'inheriting the worker env (the pre-#11267 shape): the untrusted origin is ACCEPTED', + async () => { + // ⛔ Do NOT convert this leg to childEnv() — see the file header. + const { status, code } = await probeOrigin(leakedEnv()); + expect(status).not.toBe(403); + expect(code).not.toBe('INVALID_ORIGIN'); + // Positive control: the request really did reach the sign-in handler, + // rather than failing for some unrelated reason that also is not a 403. + expect(code).toBe('INVALID_EMAIL_OR_PASSWORD'); + }, + 180_000, + ); + + it( + 'through childEnv(): the untrusted origin is REJECTED', + async () => { + const { status, code } = await probeOrigin(childEnv(OVERRIDES)); + expect(status).toBe(403); + expect(code).toBe('INVALID_ORIGIN'); + }, + 180_000, + ); + }); +}); diff --git a/packages/cli/test/serve-stdio-stdout-purity.e2e.test.ts b/packages/cli/test/serve-stdio-stdout-purity.e2e.test.ts index 183935def1..b557e0a81b 100644 --- a/packages/cli/test/serve-stdio-stdout-purity.e2e.test.ts +++ b/packages/cli/test/serve-stdio-stdout-purity.e2e.test.ts @@ -44,7 +44,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { randomPort } from './helpers/serve-process.js'; +import { E2E_SECRET_KEY, childEnv, randomPort } from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); /** `bin/run.js` — the SHIPPED entrypoint, i.e. the one the card's repro names. */ @@ -96,19 +96,30 @@ function boot(env: Record, waitFor: RegExp): Promise const child = spawn(process.execPath, [CLI, 'serve', '-p', port, '--dev'], { cwd: dir, stdio: ['pipe', 'pipe', 'pipe'], - env: { - ...process.env, + // `childEnv`, not a bare `...process.env`: the vitest worker exports + // `TEST=true`, which better-auth 1.7.1 reads directly and answers by + // switching its own origin/CSRF validation OFF in the child — see + // `helpers/serve-process.ts` for the measurement (#11267). This file + // signs in for real, so it is a child that actually reaches that code. + env: childEnv({ NO_COLOR: '1', // The card's own capture level. `warn` (the default) still puts the // banner and the WARN records on the channel, so `info` is the same // defect with more of it — the strictest setting this pin can run at. OS_LOG_LEVEL: 'info', OS_DISABLE_CONSOLE: '1', + // Explicit, not minted: with `VITEST` no longer inherited (#11267), + // `local-crypto-provider.ts`'s detectMode answers `development` for + // this child instead of `test`, and development mode PERSISTS a minted + // key to `$HOME/.objectstack/dev-crypto-key`. Supplying one keeps this + // boot from writing to the runner's home directory and from coupling + // itself to whatever other test got there first. + OS_SECRET_KEY: E2E_SECRET_KEY, // Explicit, not inherited: the dev-admin seed the mint signs in as is // hard-gated on `NODE_ENV === 'development'`, and vitest exports `test`. NODE_ENV: 'development', ...env, - }, + }), }) as ChildProcessWithoutNullStreams; children.push(child);