Skip to content

Commit 2f665a1

Browse files
os-litantclaude
andauthored
docs(cli): past-tense the deleted detectMode VITEST arm in two serve e2e headers (#12495)
Two `packages/cli/test` headers quoted `detectMode`'s `env.VITEST ||` arm in the PRESENT tense and cited `local-crypto-provider.ts:133`. That arm was deleted by a58eac3 (2026-08-23) and the live `detectMode` sits at `local-crypto-provider.ts:185`, reading `NODE_ENV` and nothing else. The conclusion each header draws survives; the mechanism it predicts does not: - `serve-process-child-env.e2e.test.ts` implied the strip still moves crypto posture. Children spawned through `helpers/serve-process.ts` run `bin/run-dev.js`, which pins `NODE_ENV = 'development'` before argv is parsed, and `NODE_ENV` sits outside `childEnv()`'s strip family — their posture is `development` with or without a leaked `VITEST`, so the `test` -> `development` flip the old wording predicted cannot happen. - `serve-node-env-production-default.e2e.test.ts` tied its explicit `OS_SECRET_KEY` to `childEnv()` stripping `VITEST`. The unset-`NODE_ENV` leg selects production posture from `NODE_ENV` alone; production refuses without a stable key whether or not a `VITEST` leaks in. Both restatements quote the removed line in the PAST TENSE and say so, which is the discriminator this class needs: `local-crypto-provider.ts` and four other sites quote the same string CORRECTLY, as retired code, and a string-level sweep would corrupt them. Comment prose only, no behaviour change. Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3d8d2f1 commit 2f665a1

2 files changed

Lines changed: 33 additions & 7 deletions

File tree

packages/cli/test/serve-node-env-production-default.e2e.test.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,13 +244,24 @@ async function probeOriginCheck(env: Record<string, string | undefined>): Promis
244244
// file measures, so the key is supplied explicitly.
245245
//
246246
// ⚠️ It was NOT needed before #11267 — and that is the finding, not an
247-
// inconvenience. `local-crypto-provider.ts:133` reads
247+
// inconvenience. What follows is quoted in the PAST TENSE on purpose:
248+
// the code it quotes is GONE. `detectMode` used to read
248249
// `if (env.VITEST || env.NODE_ENV === 'test') return 'test'`, so while
249250
// this fixture still inherited the vitest worker's `VITEST=true`, its
250251
// crypto layer sat in TEST mode (ephemeral key, no disk, no refusal)
251252
// while the rest of the boot was in production posture. The production
252253
// posture this file exists to pin was genuine for auth and fake for
253-
// crypto. Supplying the key is what makes it genuine for both.
254+
// crypto.
255+
//
256+
// #11448 (`a58eac3e`, merged 2026-08-23) deleted that arm. The live
257+
// `detectMode` (`local-crypto-provider.ts:185`) reads `NODE_ENV` and
258+
// nothing else, so the key requirement no longer depends on `childEnv()`
259+
// stripping anything: the unset-`NODE_ENV` leg selects production
260+
// posture from `NODE_ENV` alone, and production refuses without a stable
261+
// key whether or not a `VITEST` leaks in. The strip stays anyway, now as
262+
// defence-in-depth over a class `pnpm check:runner-env-posture` holds
263+
// shut in product source. Supplying the key is what makes the posture
264+
// genuine for both halves.
254265
OS_SECRET_KEY: E2E_SECRET_KEY,
255266
// The base default for every call: truly unset, unless overridden by
256267
// `env` below. Node's spawn omits an `undefined`-valued entry rather

packages/cli/test/serve-process-child-env.e2e.test.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,26 @@
3636
* `VITEST_WORKER_ID`, `VITEST_POOL_ID`, `VITEST_MODE` in place) also answers
3737
* `403 INVALID_ORIGIN`, so `TEST` alone is what better-auth reads.
3838
*
39-
* ⚠️ `VITEST` is not merely hygiene either, though this file said so in its
40-
* first revision and was wrong: `local-crypto-provider.ts:133` reads it
41-
* (`if (env.VITEST || env.NODE_ENV === 'test') return 'test'`) and an
42-
* inherited one silently put a spawned child's crypto layer in test mode. Same
43-
* class, different gate. `helpers/serve-process.ts` carries the measurement.
39+
* ⚠️ `VITEST` was not merely hygiene either, though this file said so in its
40+
* first revision and was wrong. What follows is quoted in the PAST TENSE on
41+
* purpose — the code it quotes is GONE. `detectMode` in
42+
* `local-crypto-provider.ts` used to read
43+
* `if (env.VITEST || env.NODE_ENV === 'test') return 'test'`, so an inherited
44+
* `VITEST=true` silently put a spawned child's crypto layer in test mode.
45+
* #11448 (`a58eac3e`, merged 2026-08-23) deleted that arm; the live
46+
* `detectMode` (`local-crypto-provider.ts:185`) reads `NODE_ENV` and nothing
47+
* else.
48+
*
49+
* ⛔ So do not read the strip as still moving crypto posture: children
50+
* spawned through this helper run `bin/run-dev.js`, which pins
51+
* `process.env.NODE_ENV = 'development'` before argv is parsed, and `NODE_ENV`
52+
* is deliberately outside `childEnv()`'s strip family — their posture is
53+
* `development` with or without a leaked `VITEST`. The old wording predicted a
54+
* `test` → `development` flip that cannot happen, and that prediction has
55+
* already cost one dispatch (#11596) its scoping assumption. The strip stays
56+
* anyway, now as defence-in-depth over a class `pnpm check:runner-env-posture`
57+
* holds shut in product source. Same class, different gate;
58+
* `helpers/serve-process.ts` carries the measurement.
4459
*
4560
* ## ⚠️ The first boot deliberately builds the env the WRONG way
4661
*

0 commit comments

Comments
 (0)