Skip to content

fix(service-settings): select crypto posture from the deployment signal, never the test runner - #11448

Merged
os-sam merged 4 commits into
mainfrom
claude/issue-11352-crypto-mode-deployment-env
Aug 23, 2026
Merged

fix(service-settings): select crypto posture from the deployment signal, never the test runner#11448
os-sam merged 4 commits into
mainfrom
claude/issue-11352-crypto-mode-deployment-env

Conversation

@claude

@claudeclaudeBot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes#11352

The defect

packages/services/service-settings/src/local-crypto-provider.ts selected its crypto posture like this:

constdetectMode=(env: EnvMap): CryptoMode=>{if(env.VITEST||env.NODE_ENV==='test')return'test';if(env.NODE_ENV==='production')return'production';return'development';};

'test' there is not a softer flavour of 'production'. It is the branch that takes an ephemeral key, never touches disk, and never refuses to boot. The refusal is the gate — so that one line let a test-runner variable decide whether a security gate ran at all.

Runner variables are inherited. Vitest sets TEST, VITEST, VITEST_MODE, VITEST_WORKER_ID and VITEST_POOL_ID on its worker, and every process that worker spawns with { ...process.env } receives them. So an os serve boot that merely happened to have VITEST in its environment silently took an ephemeral key, wrote nothing to disk, and stopped refusing to boot without a stable key.

Zone 2 — the dispatch's three assumptions, measured

(1) detectMode is the only posture-selection site — CONFIRMED, whole-tree. Every VITEST occurrence in tracked files (git grep -n VITEST -- . ':!**/dist/**' ':!pnpm-lock.yaml') is one of: this file (2 — one code, one prose), packages/cli/test/** (the harness that strips the family), examples/app-showcase/test/** (same), scripts/check-test-source-alias.mjs (a config-filename list). Widened to VITEST_WORKER_ID|VITEST_POOL_ID|JEST_WORKER_ID|npm_lifecycle_event and to a bare env.TEST across packages/, apps/, examples/ excluding tests: one product site, this one. Nothing else needed changing, so nothing else was changed.

(2) The deployment signal is NODE_ENV — CONFIRMED, and it is the repo's only one.packages/metadata-protocol/src/seed-loader.ts states it in prose ("NODE_ENV is this repo's ONE established environment source"), Prime Directive #9 lists it as a third-party exception, and discovery.zod.ts / seed-loader.ts both fold it. No OS_* deployment switch exists. Re-derived on origin/main: since #11113 / PR #11268, serve.ts itself defaults process.env.NODE_ENV to 'production' when unset, and its comment names "service-settings' crypto-key mode" as one of the gates that must observe that default — this change is what makes that sentence true.

(3) "There may be no reproducible spawn-leak path left today" — FALSIFIED.packages/cli/test/serve-app-anchored-optional-import.e2e.test.ts:160 spawns a real os serve with a bare ...process.env, deliberately bypassing childEnv() (its header explains why: the helper always sets cwd). serve-host-fallback-base.e2e.test.ts:187 does the same for a probe script. Neither is named in #11341's enumerated worklist — both landed after it was measured. Filed as #11441 (sub-issue of #11341); not fixed here, per the dispatch.

The fix

The VITEST read is deleted, not narrowed.

Its documented purpose — in-process unit tests get test posture, so they neither mint a key file in $HOME nor fail on a machine without one — is preserved rather than dropped, because vitest sets both variables on the same worker. Measured in vitest 4.1.10's own source, prepareVitest():

process.env.TEST="true";process.env.VITEST="true";process.env.NODE_ENV??="test";

and repeated in the env handed to each worker: NODE_ENV: process.env.NODE_ENV || "test". An in-process test therefore already satisfies NODE_ENV === 'test' and lands on the first line without VITEST participating at all.

In-process the two spellings are indistinguishable. They differ only for an inheriting child — which is precisely the defect.

⚠️Deliberate divergence from the dispatch's literal wording, flagged for review. The dispatch says "Reading VITEST for in-process unit tests is intentional and documented — do NOT remove it." This PR removes the literal env.VITEST read while preserving the behaviour it existed to provide, and pins that behaviour directly instead. The alternative — keeping a VITEST read gated on a non-inheritable in-process marker such as globalThis.__vitest_worker__ — was rejected: it keeps a runner concept in product code (against triage's ruled shape), depends on vitest internals, and is behaviourally identical to this change except in the one case where a runner variable beats an explicit deployment declaration, which is the defect class itself. If the PM reads the ruling as requiring the literal read, say so and it will be reshaped.

Clause ② — this changes accept/reject, in the tightening direction

A process that boots with NODE_ENV=production, no OS_SECRET_KEY / OS_DEV_CRYPTO_KEY, no persisted key file, no OS_CRYPTO_AUTOKEY — and a runner variable in its environment — now refuses to start where it previously booted on an ephemeral key. needs:contract-review is on the PR and it stays draft.

Measured — before

Provider-level posture matrix, real construction, no mocks (node --import tsx, each case an explicit env map, OS_HOME at an empty temp dir):

REFUSED [LocalCryptoProvider] Refusing to start in production… :: NODE_ENV=production, no key, no runner var
BOOTED keySource=ephemeral :: NODE_ENV=production, no key, VITEST=true ← the defect
BOOTED keySource=ephemeral :: NODE_ENV=production, no key, VITEST + TEST
BOOTED keySource=ephemeral :: NODE_ENV=test (in-process unit tests)

Line 2 is the card: the same deployment, the same missing key, and the refusal is simply gone.

…and end to end, on a real os serve boot

Not only at the provider. The same pair, driven through the shipped entrypoint (packages/cli/bin/run.js, so serve.ts comes from dist) against examples/app-crm, child env NODE_ENV=production + VITEST/TEST/VITEST_WORKER_ID/VITEST_POOL_ID/VITEST_MODE present, every key variable unset, HOME and OS_HOME at an empty temp dir:

BEFORE (pre-fix line restored, service-settings rebuilt) → BOOTED "Server is ready" — no crypto line printed at all
AFTER (the fix, service-settings rebuilt) → REFUSED "Refusing to start in production without a stable encryption key"

Both legs were rebuilt, because a spawned server resolves @objectstack/service-settings through exports to its dist — an unrebuilt ablation here stays silently green. Each leg's artifact was verified on disk with node scripts/ablation-dist-preflight.mjs @objectstack/service-settings 'env.VITEST' (--absent on the fix and restore legs): marker present in 2 built files on the BEFORE leg, marker absent from all 6 built files on the AFTER and restore legs. Source restored byte-identical (git hash-object 7525e844eb32c6163c53f29d6f00286269a9d4e8 both sides).

⚠️A first attempt at this probe measured nothing and said so rather than being quietly re-run. It used bin/run-dev.js, which hard-sets process.env.NODE_ENV = 'development' on line 27 — so both legs booted in development posture and the "after" leg printed "generated a new AES-256-GCM key and persisted it" instead of refusing. The deployment signal under test had been overwritten by the entrypoint. Redone through bin/run.js with packages/cli built, which is the run quoted above.

Measured — after: the refusal is back, and in-process posture did not move

packages/services/service-settings/src/crypto-posture-deployment-signal.test.ts (22 cases) pins both halves against a copy of the real worker environment{ ...process.env }, runner variables and all — rather than a hand-written fixture, because that copy is literally what a spawned child receives. One deliberate mutation per case:

  • NODE_ENV=production + the worker's own env (VITEST present) ⇒ throws/Refusing to start in production/, and no key file is minted.
  • the worker's own env unchanged ⇒ keySource === 'ephemeral', disk untouched — in-process posture intact, carried by NODE_ENV=test.
  • a table over TEST · VITEST · VITEST_WORKER_ID · VITEST_POOL_ID · VITEST_MODE · JEST_WORKER_ID × production · development · test: none of the 18 combinations moves the answer.
  • an anti-vacuity control asserts the worker really does export runner variables, so the 18 cannot pass by measuring nothing.
pnpm --filter @objectstack/service-settings test
Test Files 28 passed (28)
Tests 505 passed (505)

Ablation — prediction written before the run, signature matched exactly

Subject resolution first: the pin imports ./local-crypto-provider.js, a relative specifier, so vitest resolves the sibling .ts source and no rebuild leg is required for this ablation (@objectstack/service-settings appears 0 times in the file).

Predicted, in writing, before running: 3 red — "a deployment in production posture REFUSES…" (expected function to throw), "NODE_ENV=production is unchanged by VITEST" (booted:ephemeral vs refused:…), "NODE_ENV=development is unchanged by VITEST" (booted:ephemeral vs booted:generated-file); everything else green, including the whole table for the five non-VITEST tokens.

Observed:

× a deployment in production posture REFUSES to boot without a stable key
× NODE_ENV=production is unchanged by VITEST
× NODE_ENV=development is unchanged by VITEST
AssertionError: expected [Function] to throw an error
AssertionError: expected 'booted:ephemeral' to be 'refused:[LocalCryptoProvider] Refusin…'
AssertionError: expected 'booted:ephemeral' to be 'booted:generated-file'
Test Files 1 failed (1)
Tests 3 failed | 19 passed (22)

On-disk confirmation in both directions with grep -F on literals, and restore proven byte-identical:

BASELINE clean=1 mutant=1 hash 7525e844eb32c6163c53f29d6f00286269a9d4e8
MUTATED clean=0 mutant=2 hash 2d7b52db878a1090b0def9ccc9dae20992e0a845
RESTORED clean=1 mutant=1 hash 7525e844eb32c6163c53f29d6f00286269a9d4e8 identical: yes

⚠️ The mutant=1 baseline is not contamination: the new header quotes the removed line, and grep -F matches substrings. The deltas are what carry the signal (+1/−1 on the mutant, −1/+1 on the clean), and the gate below proves the quote is not read as code.

The guard — closing the class, not the instance

scripts/check-runner-env-posture.mjs / pnpm check:runner-env-posture, wired into lint.yml. It refuses TEST, VITEST/VITEST_* and JEST_WORKER_ID anywhere in product source (every src tree under packages/, apps/, examples/; test files and test directories excluded, where naming the family is the job).

NODE_ENV is deliberately not banned. That is the whole rule: a deployment may declare itself a test deployment; a runner may not declare it on the deployment's behalf.

Comments and string/template/regex literals are masked through the shared js-comment-mask.mjs, so the fixed file's header can keep quoting the banned line — deleting the explanation is how a defect like this comes back.

Reverse control, on real source rather than self-test fixtures — a zero-hit is not evidence on its own:

clean tree → ✓ 1907 product source file(s), no test-runner variable read. exit 0
pre-fix line restored → ✗ packages/services/service-settings/src/local-crypto-provider.ts:186 VITEST exit 1

One finding, and none of the six prose mentions of VITEST in that same file's header.

The bracket pass exists because the self-test caught its absence: with literals masked, env['VITEST'] vanished entirely and the gate reported a confident zero about the first spelling an author would reach for if the dotted one were rejected. An array literal naming the family — what the code that strips these variables must write — is still not a read. 34 self-test cases, including the wiring pins, so unwiring the gate reddens here rather than going quiet.

Docs

No hand-written doc enumerates how crypto posture is selected, so nothing became false. Checked rather than assumed: content/docs/protocol/kernel/config-resolution.mdx and packages/services/service-settings/README.md both describe the gate as NODE_ENV=production and stay accurate; a whole-tree git grep VITEST returns no .md/.mdx hit at all. What did enumerate it is the module header ("test/VITEST → ephemeral"), and that is repaired in this PR — docs-drift's symbol-anchor green is not evidence either way here.

Verification

Gate union derived with node scripts/pm/dispatch-gates.mjs, no hand-supplied paths, re-derived after the changeset existed. 30 gates, every one to a real verdict, exit captured before any pipe — all 0, including check:cross-package-test-inputs, check:entry-guard, check:parse-guard, check:test-source-alias, check:type-check-coverage, check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:published-files, check:nul-bytes, docs-audit/check-affected-docs, and the new check:runner-env-posture (which discovered its own wiring).

pnpm --filter @objectstack/service-settings typecheck — green (it caught a real TS2339 in the new pin first, fixed in c3b2e7a).

ESLint, narrowed and declared: 0 errors, 2 warnings over the 5 changed files, the 2 being "ignored file" notices for lint.yml and package.json. The narrowing is a measurement, not a skip — (i) scope read from eslint's own config via --print-config per file, (ii) file count read from --format json (results.length === 5), (iii) eslint.config.mjs enables no type-aware linting anywhere (parserOptions.project / projectService occurrences: 0, and the config says so in prose at line 326), so this diff cannot move any untouched file's verdict.

check:type-check-debt --re-measure was not run locally: it needs the whole workspace built and re-runs tsc per ledger entry. service-settings appears in neither the DEBT nor the TEST_DEBT ledger — it is one of the 65 fully type-checked packages — so this diff has no ledger entry to move, and the structural half (check:type-check-coverage, the half a new test file moves) ran green. CI runs the farm regardless.

Green-gate union re-run at c3b2e7a5d3 — the final commit on this branch, tree clean — so the 30/30 above is a reading of the tree this PR actually proposes, not of an earlier one.


Generated by Claude Code

…al only
`detectMode` read `env.VITEST` — a test-RUNNER variable — as a vote for
`'test'` posture. `'test'` is the branch that takes an ephemeral key, never
touches disk, and never refuses to boot, so a runner variable decided whether
the fail-loud production gate ran at all. Runner variables are inherited by
every process the runner spawns, so a real `os serve` spawned from a vitest
worker with `{ ...process.env }` booted with production auth and test crypto.
The read is deleted rather than narrowed. Its documented purpose — in-process
unit tests get test posture — is preserved, because vitest sets BOTH variables
on the same worker (vitest 4.1.10, `prepareVitest()`:
`process.env.VITEST = "true"; process.env.NODE_ENV ??= "test";`, repeated as
`NODE_ENV: process.env.NODE_ENV || "test"` in each worker's env). In-process the
two spellings are indistinguishable; they differ only for an INHERITING child,
which is the defect.
`crypto-posture-deployment-signal.test.ts` pins both halves against a COPY of
the real worker environment rather than a hand-written fixture, so it pins
whatever the runner actually exports.
Part of #11352
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
Closes the class the crypto-posture defect belonged to rather than only its one
member. `check:runner-env-posture` scans every `src` tree under packages/, apps/
and examples/ and refuses `TEST`, `VITEST`/`VITEST_*` and `JEST_WORKER_ID`.
`NODE_ENV` is deliberately not banned: it describes the DEPLOYMENT, and a
deployment may declare itself a test deployment — a runner may not declare it on
the deployment's behalf.
Two variables, two subsystems, one week, the same shape: `TEST` leaking into
better-auth's origin check, `VITEST` leaking into crypto posture. Nothing
mechanical stops the third.
Comments and string/template/regex literals are masked through the shared
`js-comment-mask.mjs`, so the fixed file's header can keep QUOTING the banned
line — deleting the explanation is how a defect like this comes back.
The bracket pass exists because the self-test caught its absence: with literals
masked, `env['VITEST']` vanished and the gate reported a confident zero about
the first spelling an author would reach for if the dotted one were rejected. An
array literal naming the family (what the code that STRIPS these variables must
do) is still not a read.
Verified both directions on real source, not only on self-test fixtures:
restoring the pre-fix line to `local-crypto-provider.ts` makes the gate exit 1
naming `local-crypto-provider.ts:186 VITEST` — one finding, and none of the six
prose mentions of `VITEST` in that file's own header.
Part of #11352
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
…vMap
`pnpm --filter @objectstack/service-settings typecheck` reported
TS2339 "Property 'VITEST' does not exist on type '{ NODE_ENV: string; }'":
spreading the `EnvMap`-typed base into an unannotated object literal dropped
the index signature, so the anti-vacuity assertion that the leak is still in
the map did not compile. Annotating the literal keeps it.
Also adds the changeset for the behaviour change.
Part of #11352
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
@github-actions

github-actionsBot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 7 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json e63348c984a77b6a2463e95d428e7db78406fc2epackageMentionDocs.

Which tree this was computed on

This run read content/docs from d9789803b6557bd91960ff49f6e18fa29492a803 — the merge of head 1b86bcb713a6d1c8546a981cc8cfeaefefa0f7b8 into base e63348c984a77b6a2463e95d428e7db78406fc2e, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d9789803b6557bd91960ff49f6e18fa29492a803 && git checkout d9789803b6557bd91960ff49f6e18fa29492a803
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e63348c984a77b6a2463e95d428e7db78406fc2e 1b86bcb713a6d1c8546a981cc8cfeaefefa0f7b8 && git checkout -B drift-repro e63348c984a77b6a2463e95d428e7db78406fc2e && git merge --no-ff 1b86bcb713a6d1c8546a981cc8cfeaefefa0f7b8
node scripts/docs-audit/affected-docs.mjs --json e63348c984a77b6a2463e95d428e7db78406fc2e

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

`bare-root-worklist --self-test` reddened on this branch: the new gate's
SCANNED_ROOTS holds three bare single-segment words, so it joined the invisible
bare-root species unjudged. That meta-gate is asking exactly the question this
card is about — a guard that quietly stops guarding — so the verdict is recorded
rather than the question silenced.
REFUSE-UNSPELLABLE on all three roots, measured, not estimated:
packages 1757 of 5049 (35%)
examples 150 of 240 (63%)
apps 0 of 35 (0%)
Unspellable rather than merely wide, and the distinction is the `src` SEGMENT:
the true population is `packages/**/src/**`, and `collapseHint` reduces that to
`packages`. So the only declaration the idiom can express also claims every
manifest, changelog, fixture and the 2658 test files this gate deliberately
skips. Its nearest neighbour `check:authz-resolver` is REFUSE-WIDE at a similar
39% because ITS population really is every non-test source under the root; this
one is not.
The apps leg is the load-bearing one: at 0 of 35 a subtree declaration would not
be imprecise but FALSE — pasting this gate into every apps card to reach
nothing. `apps` stays in SCANNED_ROOTS deliberately so an apps package that
grows a src tree is covered the day it lands.
Data only: pure insertion, no logic in the meta-gate touched, and
`check:runner-env-posture` itself is unchanged — it is the deliverable, not the
thing to narrow.
node scripts/pm/bare-root-worklist.mjs --self-test
OK self-test: 37 live row(s), 34 unreachable as spelled, 34 recorded
verdict(s) — none stale, none missing.
Part of #11352
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cddependenciesPull requests that update a dependency filedocumentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding: crypto mode is selected from the VITEST runner variable in product code, so a spawned server can inherit test posture

2 participants

@os-sam@claude