Skip to content

Commit b1e9db8

Browse files
test(create-objectstack): converge the third GIT_* allowlist carrier onto gitFreeEnv() (#19391)
Fixes #19171 Clause-②: no ## What this does The hand-maintained ten-name `GIT_*` location allowlist survived in one more file after PR #19168 converged the two `packages/spec` carriers. This retires it the same way, so the repo carries **one** spelling of the strip and no "either is fine" transitional state — the #16644 ruling's 「不要保留「两种都行」的过渡态」. In `packages/create-objectstack/src/template-consistency.test.ts`, `REPO_READ_ENV` becomes `gitFreeEnv()`. The ten names it replaced are byte-identical to the allowlist #19168 deleted from both spec carriers — the same shape, not a variant — and the constant is not re-spelled anywhere in the file, so a census of the retired shape does not match the paragraph that explains it. ### The header prose at `:42-54` is kept, deliberately That paragraph records why this carrier stops short of the fixture harnesses' `/dev/null` config pins: closing global config would also discard `safe.directory`, which is what lets git read a checkout owned by another user inside a container, and it would trade a passing read for `detected dubious ownership`. `gitFreeEnv()` does not reintroduce that risk, and the reason is mechanical rather than a judgement call: it **deletes** keys instead of pinning any of them to `/dev/null`, so it never sets `GIT_CONFIG_GLOBAL`. Global and system config stay open, `safe.directory` with them. The concern is preserved, and more strictly — deletion moves git toward its own defaults rather than substituting an empty config file. The widening is free here for the boundary the module's own header draws: `gitFreeEnv()` is for a **local** child and never for one that talks to a remote. Both children in this file are local reads against `cwd` (`ls-files` and `git grep`), so the transport settings the blanket strip also removes are nothing either one needs. Measured on this container: the ambient environment carries 11 `GIT_*` keys, **none** of them among the ten the allowlist named, and the `GIT_CONFIG_*` triple carries only `credential.interactive` and two `url.insteadOf` remote rewrites. There is also landed precedent for a local read of the **real** checkout taking this environment — `scripts/check-type-check-coverage.mjs` spells it `env: gitFreeEnv(), // LOCAL-ONLY (#16644)`. ## The two companion files are FORCED, not chosen The import is a relative ES-module specifier that escapes the package, and `check-cross-package-test-inputs` treats an undeclared escaping import as a red gate by design. This is not an argument — it was measured on the conversion commit before the declarations existed: ```text FAIL: cross-package test inputs are not declared consistently. - create-objectstack names path(s) no declared glob covers, so a change to them would not re-run its tests: scripts/git-env.mjs (named in packages/create-objectstack/src/template-consistency.test.ts) Widen the package's globs to cover them. ``` So `scripts/cross-package-test-inputs.mjs` and `turbo.json` gain `scripts/git-env.mjs`. Same shape PR #19168's own addendum recorded for its two extra files — both forced rather than chosen. The alternative — re-spelling the blanket strip inline to stay inside one file — is refused: it would be a **fourth** spelling of the retired shape, which is what the ruling forbids. Only the `.mjs` is declared, not its `.d.mts` sibling: vitest resolves and loads the module at run time, which is what makes it a live input to this package's `test` verdict, while the declaration is type-only and the generic `typecheck` turbo task takes `$TURBO_DEFAULT$` with no repo-root additions at all. See the acceptance note below. ## Does the test still fail for the reason it was written to catch? Yes — measured, not assumed. A conversion that compiles, runs green and no longer asserts the property is the failure mode this card is about, so the deliverable is the ablation rather than the diff. The battery builds a **decoy** repository under `mktemp -d` (never the real checkout) and proves a firing control before any verdict: ```text absolute-git-dir leak=[/tmp/os19171-decoy-WNXIGS/.git] absolute-git-dir real=[/home/user/objectstack/.git/worktrees/objectstack-issue-19171] ls-files '*SKILL.md' leak=0 real=15 ``` Under a leaked `GIT_DIR` the exact command this test runs answers for the decoy and reports **0** SKILL.md files — at **exit 0**, silently. That is the #9068 exposure class the file exists to close. A second probe confirms the leak survives the pnpm/exec spawn chain, so legs 2 and 3 are not void. | Leg | Strip | Leak | Exit | Tests | |:---|:---|:---|:---|:---| | L1 negative control | present | absent | 0 | 37 passed | | L2 guard | present | **present** | 0 | 37 passed | | L3 ablation | **removed** | **present** | **1** | **1 failed**, 36 passed | | L4 ablation control | **removed** | absent | 0 | 37 passed | L3's red is the vacuous-green guard itself: ```text FAIL src/template-consistency.test.ts > skills catalog boundary > finds the curated skills/ catalog (sanity) AssertionError: expected 0 to be greater than 0 ``` L4 is the leg that makes the red mean something: with the strip removed but no leak present, all 37 still pass — so the red in L3 comes from the leak, not from the mutation. The mutation was landed through `scripts/ablation-replace.mjs` (anchor 1 to 0, blob `36ab445bd902` to `6b125ae11ffb`), and the restore is proven on disk rather than by an exit code: blob equals HEAD and `git diff HEAD` is empty. ## Verification Run at `0e2e1065b0`, the commit this PR lands. - `pnpm --filter create-objectstack typecheck` — **exit 0**. This was the open question: the module sits outside the package's `rootDir: "src"`, and TS6059 was the failure mode. It resolves to the declaration instead, and that is a reading rather than an inference — `tsc --noEmit --listFiles` puts `scripts/git-env.d.mts` in the program at entry 178 of 308, with the test file itself at 179. - **Reverse verification that the green is real**: the declaration is *effective*, not silently `any`. Mutating the call to `gitFreeEnv` (the missing parentheses that the `.d.mts` header names as the costliest mistake here) turns the same typecheck red with `TS2322: Type '(base?: ProcessEnv | undefined) => ProcessEnv' is not assignable to type 'ProcessEnv'`, then restores to blob equal to HEAD. - `pnpm --filter create-objectstack test` — **16 files, 203 tests, all passed**. - `pnpm --filter create-objectstack build` — green; the dependency closure is empty by construction (this package has no workspace dependencies), so that leg is an empty run. - `pnpm check:cross-package-test-inputs` — 255 self-test cases pass, then **OK**. - Derived gate family via `scripts/pm/dispatch-gates.mjs`: **68 derived, 65 green, 0 unrun**. Three are NOT MEASURED, each exiting 3 on its own prerequisite (`check:dual-build-cjs-loads`, `check:lean-entry-closure`, `check:type-check-debt` all need a full-repo build, which is CI's). Re-derived after `git fetch origin main`: still 68, no new families. - `pnpm lint` (`eslint . --no-inline-config`, repo-wide, not narrowed) — **exit 0**. No changeset: measured rather than assumed. This package's `files[]` is `dist`, `README.md`, `CHANGELOG.md`; after a build, `gitFreeEnv`, `REPO_READ_ENV` and `LEAKED_GIT_ENV` have **zero** hits across those paths while the positive controls `syncObjectStackDeps`, `SKILLS_INSTALL_COMMAND` and `TEMPLATES` all hit. The changed test file is not a build entry point, and the other two files are repo-root tooling and config. ## Acceptance notes Out of scope, noted and deliberately not filed: - `scripts/git-env.d.mts`'s header names only the two `packages/spec` fixtures as its TypeScript consumers. A third now exists, so that sentence is stale. It is prose, not a gate — `check:declaration-mirrors` discovers its own corpus and is green — so nothing enforces it and nothing breaks. Flagged by the dispatching seat as explicitly outside this fence. - `@objectstack/vitest-filter-preflight#test` declares `scripts/js-comment-mask.mjs` without the `.d.mts` sibling that thirteen other turbo entries pair with it, while its `test/config-wiring-sweep.test.ts` does import the module from TypeScript. On inspection this is consistent rather than wrong — a `test` task is vitest, which never type-checks, so the declaration is genuinely irrelevant to that task's hash — and it is the same reasoning applied here for `git-env`. Recorded because it reads as an inconsistency until you work that out; no successor is blocked by it. --- _Generated by [Claude Code](https://claude.ai/code/session_01QCdUBjM47SxioST9z5Zwdf)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 03008c7 commit b1e9db8

3 files changed

Lines changed: 42 additions & 17 deletions

File tree

‎packages/create-objectstack/src/template-consistency.test.ts‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { fileURLToPath } from 'node:url';
1414
import { syncObjectStackDeps } from './pkg-utils.js';
1515
import { copyDir, TEMPLATE_FILE_ALIASES } from './template-copy.js';
1616
import { TEMPLATES } from './template-registry.js';
17+
import { gitFreeEnv } from '../../../scripts/git-env.mjs';
1718
import { SKILLS_CATALOG, SKILLS_INSTALL_COMMAND } from './skills-install.js';
1819

1920
const pkgRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
@@ -52,26 +53,32 @@ const REGISTRY_SOURCE = fs.readFileSync(path.join(pkgRoot, 'src', 'index.ts'), '
5253
// read a checkout owned by another user inside a container. Closing it could
5354
// turn a passing read into `detected dubious ownership`, which is a
5455
// regression this file gets no isolation benefit in exchange for.
55-
const LEAKED_GIT_ENV = [
56-
'GIT_DIR',
57-
'GIT_WORK_TREE',
58-
'GIT_COMMON_DIR',
59-
'GIT_INDEX_FILE',
60-
'GIT_OBJECT_DIRECTORY',
61-
'GIT_ALTERNATE_OBJECT_DIRECTORIES',
62-
'GIT_NAMESPACE',
63-
'GIT_CEILING_DIRECTORIES',
64-
'GIT_TEMPLATE_DIR',
65-
'GIT_CONFIG',
66-
] as const;
56+
//
57+
// #16644 — the hand-maintained allowlist of ten location variables that used to
58+
// stand here is retired in favour of the blanket strip in `scripts/git-env.mjs`,
59+
// and it is not re-spelled anywhere in this file so that a census of the retired
60+
// shape does not match this paragraph. The reason the list goes rather than gets
61+
// one more entry: it had to be kept level with git's own list of location
62+
// variables, and its failure mode is that THE KEY IT MISSES IS THE KEY THAT
63+
// BITES. ⛔ One spelling in the repo, not two — no "either is fine" transition
64+
// state.
65+
//
66+
// `gitFreeEnv()` removes every `GIT_`-prefixed key, which is strictly wider than
67+
// the list it replaces and still inside the boundary drawn above. It DELETES
68+
// keys rather than pinning any of them to `/dev/null`, so it never sets
69+
// `GIT_CONFIG_GLOBAL`: global and system config stay open and `safe.directory`
70+
// with them, which is the whole of the concern in the bullet above — preserved,
71+
// and more strictly, because the deletion moves git toward its own defaults
72+
// instead of substituting an empty config file. The widening is free here for
73+
// the other reason the module's header names: both children below are LOCAL
74+
// reads — `ls-files` and `git grep` against `cwd` — so the transport settings
75+
// the strip also takes (`GIT_CONFIG_*` rewriting remotes, `GIT_SSL_*`) are
76+
// nothing either one needs. ⛔ A child that fetches, clones or pushes would not
77+
// be entitled to this environment.
6778

6879
/** The environment the repo-reading git calls below get: this process's, minus
6980
* every variable that could aim them at a different repository. */
70-
const REPO_READ_ENV: NodeJS.ProcessEnv = (() => {
71-
const env = { ...process.env };
72-
for (const key of LEAKED_GIT_ENV) delete env[key];
73-
return env;
74-
})();
81+
const REPO_READ_ENV: NodeJS.ProcessEnv = gitFreeEnv();
7582

7683
// ── Declared version surfaces, per bundled template (#9264) ─────────────────
7784
//

‎scripts/cross-package-test-inputs.mjs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1393,6 +1393,23 @@ export const CROSS_PACKAGE_TEST_INPUTS = {
13931393
// with ERR_MODULE_NOT_FOUND), which is exactly the trigger radius this
13941394
// declaration exists to keep honest.
13951395
'scripts/invoked-as.mjs',
1396+
// The git-environment strip, IMPORTED by src/template-consistency.test.ts.
1397+
// That file's skills-catalog block asks git two questions whose answers ARE
1398+
// its verdict — `ls-files '*SKILL.md'` and a `git grep` over the
1399+
// customer-facing surfaces — and both run against the REAL checkout, so a
1400+
// `GIT_DIR` or `GIT_INDEX_FILE` inherited from a hook would make them answer
1401+
// for a DIFFERENT repository while `cwd` still reads as this one. The test
1402+
// used to spell a hand-maintained ten-name allowlist inline; `gitFreeEnv()`
1403+
// replaces it, which is the #16644 convergence onto one spelling.
1404+
//
1405+
// FORCED rather than chosen, and the same shape as the `.d.mts` pair above:
1406+
// the import is a relative ES-module specifier vitest RESOLVES AND LOADS at
1407+
// run time, so the module is a live input to this package's verdict, and an
1408+
// undeclared escaping import is a red gate by design. Measured on the
1409+
// conversion commit before this line existed — the gate printed
1410+
// `scripts/git-env.mjs (named in packages/create-objectstack/src/template-consistency.test.ts)`
1411+
// and exited 1.
1412+
'scripts/git-env.mjs',
13961413
'.github/workflows/scaffold-e2e.yml',
13971414
'packages/cli/src/commands/serve.ts',
13981415
'scripts/gen-sdui-manifest.sh',

‎turbo.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,7 @@
502502
"$TURBO_ROOT$/content/**",
503503
"$TURBO_ROOT$/scripts/sync-template-versions.mjs",
504504
"$TURBO_ROOT$/scripts/invoked-as.mjs",
505+
"$TURBO_ROOT$/scripts/git-env.mjs",
505506
"$TURBO_ROOT$/.github/workflows/scaffold-e2e.yml",
506507
"$TURBO_ROOT$/packages/cli/src/commands/serve.ts",
507508
"$TURBO_ROOT$/scripts/gen-sdui-manifest.sh",

0 commit comments

Comments
 (0)