Skip to content

Commit 2b9a522

Browse files
os-trumpclaude
andauthored
docs(cli): fix stale detectMode quote in serve-process.ts header (#12170)
The header's "VITEST is NOT cosmetic" section quoted local-crypto-provider.ts's detectMode with an `env.VITEST ||` arm that #11448 (a58eac3) already removed, and predicted a crypto-posture flip that can no longer happen. Comment-only rewrite: the read existed, #11448 removed it, check:runner-env-posture now holds the class, and the strip in childEnv() stays as defence-in-depth over a gated class rather than the fix for a live read. OS_SECRET_KEY's justification in runServe() is re-derived from NODE_ENV (bin/run-dev.js pins development before argv is parsed) instead of VITEST. No executable line changed. Co-authored-by: Claude <noreply@anthropic.com>
1 parent ce14581 commit 2b9a522

1 file changed

Lines changed: 29 additions & 25 deletions

File tree

packages/cli/test/helpers/serve-process.ts

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -66,41 +66,45 @@ export function randomPort(): string {
6666
* The third row is the isolation for THAT probe: `TEST` alone is what
6767
* better-auth reads.
6868
*
69-
* ## ⚠️ `VITEST` is NOT cosmetic either — a claim this file got wrong once
69+
* ## VITEST is stripped as defence-in-depth now, not for a live product read
7070
*
7171
* The first revision of this header said the `VITEST*` entries were stripped
72-
* as hygiene, "nothing in `os serve` reads them today". That was **false**, and
73-
* CI found the counterexample:
72+
* as hygiene, "nothing in `os serve` reads them today". That was **false**
73+
* for a while: `detectMode` in `local-crypto-provider.ts` read `env.VITEST`
74+
* directly, so an inherited `VITEST=true` put a spawned child's crypto layer
75+
* in `test` mode — ephemeral key, never touches disk, never refuses — no
76+
* matter what posture the rest of the boot was in. #11448 (`a58eac3e`,
77+
* merged 2026-08-23) removed that arm; `detectMode` today reads only
78+
* `NODE_ENV`:
7479
*
7580
* ```ts
76-
* // packages/services/service-settings/src/local-crypto-provider.ts:133
81+
* // packages/services/service-settings/src/local-crypto-provider.ts:185
7782
* const detectMode = (env: EnvMap): CryptoMode => {
78-
* if (env.VITEST || env.NODE_ENV === 'test') return 'test';
83+
* if (env.NODE_ENV === 'test') return 'test';
7984
* if (env.NODE_ENV === 'production') return 'production';
8085
* return 'development';
8186
* };
8287
* ```
8388
*
84-
* So an inherited `VITEST=true` put every spawned child's crypto layer in
85-
* `test` mode — ephemeral key, never touches disk, never refuses — no matter
86-
* what posture the rest of the boot was in. That is the SAME defect class as
87-
* the `TEST` leak one layer over: a security-relevant gate (here, stable
88-
* encryption-key enforcement) softened by a variable the child inherited from
89-
* the test runner rather than by anything the code under test decided.
90-
* Stripping `VITEST` is therefore load-bearing in its own right, and the
91-
* `serve-node-env-production-default` pin going red the moment it stopped
92-
* leaking is the gate working, not the gate misfiring: that fixture's
93-
* "production posture" had been genuine for auth and fake for crypto.
94-
*
95-
* The consequence is why `OS_SECRET_KEY` is a default below. Once the child
96-
* stops claiming to be a vitest worker, `detectMode` answers `development`
97-
* for the ordinary boots here, and development mode **persists** a minted key
98-
* to `$HOME/.objectstack/dev-crypto-key`. Measured: with that file absent a
99-
* production-posture boot refuses to start, and with it present — put there by
100-
* any earlier dev-mode boot in the same run — the same boot succeeds. That is
101-
* a cross-test ordering coupling through the runner's home directory, and
102-
* under vitest's parallel workers it is nondeterministic. An explicit key
103-
* removes both halves: nothing is written, and nothing is depended on.
89+
* No product source reads `VITEST` any more, and `pnpm check:runner-env-posture`
90+
* is the gate that keeps that class shut. The strip below stays anyway — now
91+
* as **defence-in-depth over a gated class**, not as the fix for a live read:
92+
* the choke point here should not depend on product source staying that way.
93+
*
94+
* The consequence this drove — `OS_SECRET_KEY` being a default below — no
95+
* longer follows from a VITEST leak; re-derive it from `NODE_ENV`, which is
96+
* what `detectMode` actually reads. `bin/run-dev.js` pins
97+
* `process.env.NODE_ENV = 'development'` before argv is even parsed, and
98+
* `NODE_ENV` is deliberately outside this strip family (below), so every
99+
* child spawned through this helper is ALREADY in `development` crypto
100+
* posture — with or without a leaked `VITEST`. Development mode **persists**
101+
* a minted key to `$HOME/.objectstack/dev-crypto-key`. Measured: with that
102+
* file absent a production-posture boot refuses to start, and with it
103+
* present — put there by any earlier dev-mode boot in the same run — the
104+
* same boot succeeds. That is a cross-test ordering coupling through the
105+
* runner's home directory, and under vitest's parallel workers it is
106+
* nondeterministic. An explicit key removes both halves: nothing is written,
107+
* and nothing is depended on.
104108
*
105109
* ⛔ `NODE_ENV` is deliberately NOT in this family. The vitest worker exports
106110
* `NODE_ENV=test` too, but every caller here already pins the child's

0 commit comments

Comments
 (0)