From 0e2e1065b00b522d76164440751f80c4caa37f70 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 15:17:32 +0000 Subject: [PATCH] test(create-objectstack): converge the third GIT_* allowlist carrier onto gitFreeEnv() The hand-maintained ten-name `GIT_*` location allowlist survived in one more file after the two spec carriers were converged. It is retired here the same way, so the repo carries one spelling of the strip and no transitional state. `REPO_READ_ENV` is now `gitFreeEnv()`. The blanket strip is strictly wider than the list it replaces and stays inside the boundary this file's header draws: it DELETES keys rather than pinning any to `/dev/null`, so it never sets `GIT_CONFIG_GLOBAL` and global/system config -- `safe.directory` with it -- stays open. Both children here are local reads against `cwd` (`ls-files` and `git grep`), so the transport settings the strip also removes are nothing either one needs. The `:42-54` prose is kept: it records why this carrier stops short of the fixture harnesses' `/dev/null` config pins. The import is a relative ES-module specifier that escapes the package, which forces two companion declarations -- `scripts/cross-package-test-inputs.mjs` and `turbo.json` -- so a change to the strip re-runs this package's suite instead of replaying a cached green over it. Measured before they landed: the gate named `scripts/git-env.mjs` and exited 1. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01QCdUBjM47SxioST9z5Zwdf --- .../src/template-consistency.test.ts | 41 +++++++++++-------- scripts/cross-package-test-inputs.mjs | 17 ++++++++ turbo.json | 1 + 3 files changed, 42 insertions(+), 17 deletions(-) diff --git a/packages/create-objectstack/src/template-consistency.test.ts b/packages/create-objectstack/src/template-consistency.test.ts index 466f305d7a6..36ab445bd90 100644 --- a/packages/create-objectstack/src/template-consistency.test.ts +++ b/packages/create-objectstack/src/template-consistency.test.ts @@ -14,6 +14,7 @@ import { fileURLToPath } from 'node:url'; import { syncObjectStackDeps } from './pkg-utils.js'; import { copyDir, TEMPLATE_FILE_ALIASES } from './template-copy.js'; import { TEMPLATES } from './template-registry.js'; +import { gitFreeEnv } from '../../../scripts/git-env.mjs'; import { SKILLS_CATALOG, SKILLS_INSTALL_COMMAND } from './skills-install.js'; 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'), ' // read a checkout owned by another user inside a container. Closing it could // turn a passing read into `detected dubious ownership`, which is a // regression this file gets no isolation benefit in exchange for. -const LEAKED_GIT_ENV = [ - 'GIT_DIR', - 'GIT_WORK_TREE', - 'GIT_COMMON_DIR', - 'GIT_INDEX_FILE', - 'GIT_OBJECT_DIRECTORY', - 'GIT_ALTERNATE_OBJECT_DIRECTORIES', - 'GIT_NAMESPACE', - 'GIT_CEILING_DIRECTORIES', - 'GIT_TEMPLATE_DIR', - 'GIT_CONFIG', -] as const; +// +// #16644 — the hand-maintained allowlist of ten location variables that used to +// stand here is retired in favour of the blanket strip in `scripts/git-env.mjs`, +// and it is not re-spelled anywhere in this file so that a census of the retired +// shape does not match this paragraph. The reason the list goes rather than gets +// one more entry: it had to be kept level with git's own list of location +// variables, and its failure mode is that THE KEY IT MISSES IS THE KEY THAT +// BITES. ⛔ One spelling in the repo, not two — no "either is fine" transition +// state. +// +// `gitFreeEnv()` removes every `GIT_`-prefixed key, which is strictly wider than +// the list it replaces and still inside the boundary drawn above. It DELETES +// keys rather than pinning any of them to `/dev/null`, so it never sets +// `GIT_CONFIG_GLOBAL`: global and system config stay open and `safe.directory` +// with them, which is the whole of the concern in the bullet above — preserved, +// and more strictly, because the deletion moves git toward its own defaults +// instead of substituting an empty config file. The widening is free here for +// the other reason the module's header names: both children below are LOCAL +// reads — `ls-files` and `git grep` against `cwd` — so the transport settings +// the strip also takes (`GIT_CONFIG_*` rewriting remotes, `GIT_SSL_*`) are +// nothing either one needs. ⛔ A child that fetches, clones or pushes would not +// be entitled to this environment. /** The environment the repo-reading git calls below get: this process's, minus * every variable that could aim them at a different repository. */ -const REPO_READ_ENV: NodeJS.ProcessEnv = (() => { - const env = { ...process.env }; - for (const key of LEAKED_GIT_ENV) delete env[key]; - return env; -})(); +const REPO_READ_ENV: NodeJS.ProcessEnv = gitFreeEnv(); // ── Declared version surfaces, per bundled template (#9264) ───────────────── // diff --git a/scripts/cross-package-test-inputs.mjs b/scripts/cross-package-test-inputs.mjs index b2b9cb54b56..b790c2e0c59 100644 --- a/scripts/cross-package-test-inputs.mjs +++ b/scripts/cross-package-test-inputs.mjs @@ -1393,6 +1393,23 @@ export const CROSS_PACKAGE_TEST_INPUTS = { // with ERR_MODULE_NOT_FOUND), which is exactly the trigger radius this // declaration exists to keep honest. 'scripts/invoked-as.mjs', + // The git-environment strip, IMPORTED by src/template-consistency.test.ts. + // That file's skills-catalog block asks git two questions whose answers ARE + // its verdict — `ls-files '*SKILL.md'` and a `git grep` over the + // customer-facing surfaces — and both run against the REAL checkout, so a + // `GIT_DIR` or `GIT_INDEX_FILE` inherited from a hook would make them answer + // for a DIFFERENT repository while `cwd` still reads as this one. The test + // used to spell a hand-maintained ten-name allowlist inline; `gitFreeEnv()` + // replaces it, which is the #16644 convergence onto one spelling. + // + // FORCED rather than chosen, and the same shape as the `.d.mts` pair above: + // the import is a relative ES-module specifier vitest RESOLVES AND LOADS at + // run time, so the module is a live input to this package's verdict, and an + // undeclared escaping import is a red gate by design. Measured on the + // conversion commit before this line existed — the gate printed + // `scripts/git-env.mjs (named in packages/create-objectstack/src/template-consistency.test.ts)` + // and exited 1. + 'scripts/git-env.mjs', '.github/workflows/scaffold-e2e.yml', 'packages/cli/src/commands/serve.ts', 'scripts/gen-sdui-manifest.sh', diff --git a/turbo.json b/turbo.json index 1761f9f818e..4deb60b4fb3 100644 --- a/turbo.json +++ b/turbo.json @@ -502,6 +502,7 @@ "$TURBO_ROOT$/content/**", "$TURBO_ROOT$/scripts/sync-template-versions.mjs", "$TURBO_ROOT$/scripts/invoked-as.mjs", + "$TURBO_ROOT$/scripts/git-env.mjs", "$TURBO_ROOT$/.github/workflows/scaffold-e2e.yml", "$TURBO_ROOT$/packages/cli/src/commands/serve.ts", "$TURBO_ROOT$/scripts/gen-sdui-manifest.sh",