Uh oh!
There was an error while loading. Please reload this page.
test(cli,scripts): enforce the childEnv() convention over packages/cli/test spawners - #11599
Merged
Merged
Conversation
…t spawners
`childEnv()` (packages/cli/test/helpers/serve-process.ts) is the choke point
that keeps the vitest worker's `TEST`/`VITEST*` family out of a spawned CLI
child. It has been a CONVENTION only: the next author writing an e2e test in
that directory reaches for `{ ...process.env, ... }` because that is what the
neighbouring files still look like, and nothing fails when they do.
Twice measured, in two subsystems, one week apart: an inherited `TEST=true`
switched better-auth's origin/CSRF validation off inside a spawned `os serve`
(#11267), and an inherited `VITEST` put the settings crypto provider in its
never-refuses posture (#11352). Neither showed up as a red test -- the failure
mode is a security assertion that cannot go red for the reason it exists.
check-cli-test-child-env.mjs makes the convention an invariant. A finding is a
BULK `process.env` reference (used as a whole value, as opposed to reading one
variable off it) in a SPAWNER file (one that imports a process-creating API).
Both halves are load-bearing: the spawner filter is what keeps an ordinary
in-process save/restore from being flagged, and the bulk/member distinction is
what keeps `childEnv({ HOME: process.env.HOME })` green.
The 18 leaks that predate the gate are baselined shrink-only; the ratchet
refuses new members outright and fails on a stale ceiling. The two sites that
copy the environment ON PURPOSE -- the choke point, and #11267's pin leg that
proves the leak does something -- are DELIBERATE entries pinned in BOTH
directions, so "cleaning up" that evidence now fails.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR… and its ratchet The baseline had no owner named anywhere in the tree, so a reader hitting a stale-ceiling failure had nothing to follow. #11596 tracks the burn-down; #11595 is the sibling gap this gate deliberately does not cover (a spawn passing no env option at all). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
os-zhuang
marked this pull request as ready for review
August 24, 2026 07:45
This was referenced Aug 24, 2026
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#11341
childEnv()inpackages/cli/test/helpers/serve-process.tskeeps the vitest worker'sTEST/VITEST*family out of a spawned CLI child. Until now it was a convention: the next author writing an e2e test in that directory reaches for{ ...process.env, ... }because that is what the neighbouring files still look like, and nothing failed when they did.This lands the enforcement. It does not repair the existing violations — those are baselined, and burning the baseline down is follow-up work, one card per neighbourhood, tracked on #11596.
The rule, stated exactly
A finding is a bulk
process.envreference in a spawner file underpackages/cli/test/**.node:child_process(spawn,spawnSync,exec,execSync,execFile,execFileSync,fork; named, namespace or default) orWorkerfromnode:worker_threads.Workeris on the roster with no instance in the population, deliberately: the class has to close for the next spelling, not the last one.process.envused as a whole value: a spread, anObject.assign/Object.entriessource, an argument, an alias. Reading one variable off it is never flagged, sochildEnv({ HOME: process.env.HOME })stays green. Writing to it (process.env = saved) is not a copy and is not flagged either.Both halves are load-bearing. The spawner filter is what keeps an ordinary in-process save/restore from being flagged; the bulk/member distinction is what keeps the correct repair shape green. A gate that flagged every
...process.envin the package would be carved out into uselessness.Measured on
origin/mainat2dc0a770b: 82 sources in the population, 28 of them spawners, and zero bulk references outside a spawner file. So the filter costs no recall today and its precision claim is a measurement rather than a hope.Why the site is not anchored to the spawn CALL
The sharper-looking rule — find the
env:option of a spawn call and look inside it — was written, measured against this population, and rejected. It cannot see the one site in the directory that matters most:serve-process-child-env.e2e.test.tsbuilds its leaked environment inleakedEnv(), hands it toprobeOrigin(env), and the spawn'senvis a function parameter. Chasing that needs same-file interprocedural data flow, and every hop it cannot follow is a silent zero. The file-anchored rule has no such hop.Edges, both directions, named rather than implied
False positive, by construction: a spawner file that copies the whole environment for a reason that is not a child. Zero instances today beyond the two declared below; a new one is a
DELIBERATEentry carrying its reason.False negatives, three, each measured:
envoption at all inheritsprocess.envverbatim — the same leak in a purer form, and this gate is silent on it. 8 such call sites exist today. Out of scope deliberately: whether atsx-on-a-probe-script unit test owes thechildEnv()convention is a question about the convention's reach, not about this defect. Filed as finding: 8packages/cli/testspawners pass noenvat all, so the child inherits the vitest worker environment verbatim — the purer form of #11341's leak, and the new gate is silent on it #11595.process.env.The two registries, and why they are different kinds
DELIBERATE(in the gate) is a declaration registry — the sites that copy the whole environment on purpose, pinned in both directions. A stale entry fails. That is the carve-out discipline the choke point documents, expressed where a machine can hold it:serve-process-child-env.e2e.test.ts's header says "Do not clean it up ... that would delete the only evidence in the repo that the leak does anything", and a header is a request. Two entries today:helpers/serve-process.ts::childEnvserve-process-child-env.e2e.test.ts::leakedEnvscripts/cli-test-child-env.baseline.jsonis a ratchet — 18 latent leaks across 17 files, as per-file counts rather than a path list (a path list silences the whole file, so a new leak rides an existing entry in silence — the #4251 lesson the slot-lookup ratchet paid for). Shrink-only in both dimensions, closed to new entries, and a ceiling left above reality fails as stale.Population coverage: the two files #11441 names
#11441 reports that the hand-built worklist preceding this gate was two files short. A gate has no worklist, so both are ordinary members of the enumerated population, and both are in the baseline:
Two self-test cases pin that by name, because "the derivation covers them" is exactly the kind of claim that rots silently. #11441 itself stays open for triage to grade — nothing here changes its state. #11464 is a different defect and is out of scope here; it stays open too.
generate-skill.e2e.test.tsandgenerate-agent-retired.e2e.test.tsare baselined and untouched — they belong to #11071's neighbourhood, in flight in a parallel seat this same round.Anti-vacuity: the self-test can fail, shown four ways
54 cases. The positive control runs out of process through the real entry point, because "exits non-zero" is the claim and no process can observe its own exit status:
childEnv()form through the same entry point exits zero;DELIBERATEsite fails;Ablation against the live tree, each leg confirmed on disk before the reading was taken and restored under a trap:
exit=1— 1 file builds a child's environment from the whole of process.envexit=1— 2 bulk reference(s), ceiling 1exit=1— 1 stale baseline entryDELIBERATEpin "cleaned up"exit=1— 1 DELIBERATE site no longer presentThe tree was restored and the gate returned to green after each.
No changeset
Nothing published changes. The diff is
scripts/**,.github/workflows/lint.ymland the rootpackage.json(private,@objectstack/spec-monorepo) — no workspace package source, nopackages/cli/testfile.skip-changesetapplies.Gates
Derived with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(19 families) atacf5f14e, and the whole union re-run on that head after the last commit, pluscheck:nul-bytes. All green, each quoted from the gate's own verdict line:check:type-check-debtfirst refused rather than failing (--re-measure cannot run: 56 workspace dependencies have no built type entry point on disk) — a property of a fresh worktree, not of this diff. Re-run to a real verdict afterturbo run build --filter='./packages/*' --filter='./packages/*/*'; the line above is that run.Farm-wide
pnpm lint(eslint . --no-inline-config) also run in full rather than narrowed:VERDICT command-exit 0 · held the lock 79s.The gate itself registers in
check-ratchet-remedy-authority's control corpus asrefused— the sweep classified it that way on its own before the entry was written, which is the classification being confirmed rather than asserted.Generated by Claude Code