Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions packages/cli/test/artifact-pinned-boot.e2e.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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');
Expand DownExpand Up@@ -122,8 +123,15 @@ function runServe(env: Record<string, string>, 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')}`,
Expand All@@ -136,7 +144,7 @@ function runServe(env: Record<string, string>, opts: { migrateAndExit?: boolean
// container carrying no app does not have.
OS_ARTIFACT_PATH: join(cwd, 'dist/objectstack.json'),
...env,
},
}),
stdio: ['ignore', 'pipe', 'pipe'],
},
);
Expand Down
145 changes: 142 additions & 3 deletions packages/cli/test/helpers/serve-process.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<string, string | undefined> = {},
): Record<string, string | undefined> {
const env: Record<string, string | undefined> = {};
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;
Expand DownExpand Up@@ -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 = '';
Expand Down
19 changes: 15 additions & 4 deletions packages/cli/test/serve-mcp-capability-collision.e2e.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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. */
Expand DownExpand Up@@ -109,16 +109,27 @@ function boot(env: Record<string, string | undefined>, 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);

Expand Down
19 changes: 15 additions & 4 deletions packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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. */
Expand DownExpand Up@@ -104,17 +104,28 @@ function boot(env: Record<string, string | undefined>, 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);

Expand Down
42 changes: 31 additions & 11 deletions packages/cli/test/serve-node-env-production-default.e2e.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<null, Readable, Readable>;
Expand DownExpand Up@@ -210,8 +211,7 @@ async function probeOriginCheck(env: Record<string, string | undefined>): 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',
Expand All@@ -221,27 +221,47 @@ async function probeOriginCheck(env: Record<string, string | undefined>): 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);

Expand Down
Loading
Loading