From 250531aa4306acdf2d23cdc046f7cb405ef8556a Mon Sep 17 00:00:00 2001 From: DongHyeonka Date: Fri, 14 Aug 2026 16:08:32 +0900 Subject: [PATCH] fix: stop test fixtures from deleting the repository's dependencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four fixtures linked the installed dependencies into a throwaway root with a single directory symlink at /node_modules, then ran pnpm inside that root. pnpm does not recognise the modules directory it finds there and purges it; with CI=true it does so without a prompt. The purge followed the symlink and deleted the repository's own node_modules mid-run, so a test suite uninstalled the workspace it was running in. That is what produced the cascading, file-unrelated failures a full test:unit run reported, and it happened twice while running the suites for the adapter re-review. scripts/lib/fixture-node-modules.ts replaces all four sites: node_modules is a real directory whose entries are individual symlinks, so a recursive delete unlinks the fixture's own links instead of walking through one link into the shared tree. Resolution is unchanged. tests/unit/fixture-node-modules.test.ts performs the exact recursive delete pnpm performs and asserts the source tree survives, and check:adapter-inventory now fails on any reintroduction of the directory-symlink form — verified by putting the old line back and watching the gate reject it. A full tests/unit + tests/integration run now leaves the dependencies intact. removal-fixture, supply-chain and security-followup-archive, the three suites that had to be excluded before, pass in that run. Co-Authored-By: Claude Opus 5 (1M context) --- docs/operations/adapter-remediation-ledger.md | 46 +++++++++- scripts/check-adapter-inventory.ts | 35 +++++++- .../check-supply-chain-provider-fixtures.ts | 4 +- scripts/lib/fixture-node-modules.ts | 31 +++++++ scripts/lib/removal-fixture.ts | 4 +- .../security-followup-archive.test.ts | 5 +- tests/unit/ci-artifact-contract.test.ts | 3 +- tests/unit/fixture-node-modules.test.ts | 89 +++++++++++++++++++ 8 files changed, 207 insertions(+), 10 deletions(-) create mode 100644 scripts/lib/fixture-node-modules.ts create mode 100644 tests/unit/fixture-node-modules.test.ts diff --git a/docs/operations/adapter-remediation-ledger.md b/docs/operations/adapter-remediation-ledger.md index 9fe8da0..cacb948 100644 --- a/docs/operations/adapter-remediation-ledger.md +++ b/docs/operations/adapter-remediation-ledger.md @@ -314,8 +314,50 @@ Run on the landed tree. Only what actually passed is claimed as passing. | --- | --- | --- | | `tests/unit/ci-workflow-generation.test.ts` | 82 failed / 325 passed | Identical on the pre-change baseline (`git stash` comparison). The subprocess gates it spawns cannot run in this sandbox. | | `tests/unit/ci-artifact-contract.test.ts` | fails | Unchanged pre-existing sandbox, cgroup and `/tmp` permission behaviour. | -| `tests/unit/removal-fixture.test.ts`, `tests/unit/supply-chain.test.ts`, `tests/integration/security-followup-archive.test.ts` | destructive | `scripts/lib/removal-fixture.ts:45` and `scripts/check-supply-chain-provider-fixtures.ts:62` symlink the real `node_modules` into a temp fixture root and then run `corepack pnpm` there with `CI=true`. pnpm purges the modules directory it does not recognise, **through the symlink**, deleting the repository's own dependencies mid-run. This is a pre-existing repository hazard, outside the 38 findings, and it is why a full `test:unit` run reports cascading failures. | -| `tests/unit/security-followup.test.ts` | flaky under load | Passes in a fresh process; the process-group reaping assertion is timing sensitive. | +| `tests/unit/security-followup.test.ts`, `tests/unit/provider-guardian-transaction.test.ts` | flaky under full-suite load | Both pass in a fresh process (49 passed). They spawn and reap process groups, so their timing assertions are load sensitive. | + +### Destructive fixture hazard — fixed + +Outside the 38 findings, and found while running the suites for them. + +Four sites linked the repository's installed dependencies into a throwaway +fixture with a single directory symlink at `/node_modules`: + +- `scripts/lib/removal-fixture.ts` +- `scripts/check-supply-chain-provider-fixtures.ts` +- `tests/integration/security-followup-archive.test.ts` +- `tests/unit/ci-artifact-contract.test.ts` + +Each fixture then runs `pnpm` inside itself. pnpm does not recognise the modules +directory it finds there and purges it; with `CI=true` it does so without a +prompt. The purge followed the symlink and deleted the **repository's own** +`node_modules` mid-run — a test suite uninstalling the workspace it was running +in. That is what produced the cascading, file-unrelated failures a full +`test:unit` run reported, and it happened twice during this work. + +`scripts/lib/fixture-node-modules.ts` replaces all four: `node_modules` is a +real directory whose entries are individual symlinks, so a recursive delete +unlinks the fixture's own links instead of walking through one link into the +shared tree. `tests/unit/fixture-node-modules.test.ts` performs the exact +recursive delete pnpm performs and asserts the source tree survives, and +`corepack pnpm check:adapter-inventory` fails on any reintroduction of the +directory-symlink form. + +After the fix a full `tests/unit` + `tests/integration` run leaves the +dependencies intact and its failures are confined to the two environmental +files above plus the two flaky-under-load ones: + +| File | Failed | Attribution | +| --- | ---: | --- | +| `tests/unit/ci-workflow-generation.test.ts` | 82 | Identical on the pre-change baseline (`git stash` comparison). Its subprocess gates cannot run in this sandbox. | +| `tests/unit/ci-artifact-contract.test.ts` | 19 | Unchanged pre-existing sandbox, cgroup and `/tmp` permission behaviour. | +| `tests/unit/security-followup.test.ts` | 2 | Passes in isolation. | +| `tests/unit/provider-guardian-transaction.test.ts` | 1 | Passes in isolation. | + +1619 passed / 1723 total, and `tests/unit/removal-fixture.test.ts`, +`tests/unit/supply-chain.test.ts` and +`tests/integration/security-followup-archive.test.ts` — the three that had to be +excluded before — now pass in the full run. ## Rules for updating this ledger diff --git a/scripts/check-adapter-inventory.ts b/scripts/check-adapter-inventory.ts index ba1f8c1..3ad921f 100644 --- a/scripts/check-adapter-inventory.ts +++ b/scripts/check-adapter-inventory.ts @@ -91,6 +91,38 @@ async function main(): Promise { } } + // A fixture that links the repository's node_modules with a single directory + // symlink is destructive: pnpm running inside that fixture purges the modules + // directory it does not recognise, follows the link, and deletes the real + // dependencies mid-run. `linkFixtureNodeModules` is the only sanctioned form. + const sources = spawnSync( + "git", + ["grep", "-n", "-e", 'symlink(', "--", "scripts", "tests"], + { encoding: "utf8" }, + ); + if (sources.status === 0) { + for (const line of sources.stdout.split("\n").filter(Boolean)) { + if (!line.includes("node_modules")) continue; + if (line.startsWith("scripts/lib/fixture-node-modules.ts:")) continue; + problems.push( + `fixture node_modules: use linkFixtureNodeModules instead — ${line}`, + ); + } + } + const linkedFixtures = spawnSync( + "git", + ["grep", "-l", "linkFixtureNodeModules", "--", "scripts", "tests"], + { encoding: "utf8" }, + ); + if ( + linkedFixtures.status !== 0 || + linkedFixtures.stdout.split("\n").filter(Boolean).length < 2 + ) { + problems.push( + "fixture node_modules: the shared linker has no callers, so it is not the sanctioned path", + ); + } + if (problems.length > 0) { for (const problem of problems) console.error(problem); process.exitCode = 1; @@ -100,7 +132,8 @@ async function main(): Promise { `Adapter inventory: ${tracked.length} files PASS; ` + `service worker asset table: ${ Object.keys(CACHEABLE_ASSET_CONTENT_TYPES).length - } shared extensions PASS`, + } shared extensions PASS; ` + + `fixture node_modules linking PASS`, ); } diff --git a/scripts/check-supply-chain-provider-fixtures.ts b/scripts/check-supply-chain-provider-fixtures.ts index 04e8f8f..69cbef8 100644 --- a/scripts/check-supply-chain-provider-fixtures.ts +++ b/scripts/check-supply-chain-provider-fixtures.ts @@ -12,7 +12,6 @@ import { readFile, readdir, rm, - symlink, writeFile, } from "node:fs/promises"; import { tmpdir } from "node:os"; @@ -42,6 +41,7 @@ import { releaseCandidateManifestSchema, } from "./lib/release-candidate.ts"; import { writeValidatedJsonArtifact } from "./lib/validated-json-artifact.ts"; +import { linkFixtureNodeModules } from "./lib/fixture-node-modules.ts"; const repositoryRoot = process.cwd(); const fixtureRoot = await mkdtemp(path.join(tmpdir(), "provider-exact-five-fixture-")); @@ -59,7 +59,7 @@ try { recursive: true, }); await rm(path.join(fixtureRoot, "artifacts/release"), { recursive: true, force: true }); - await symlink(path.join(repositoryRoot, "node_modules"), path.join(fixtureRoot, "node_modules"), "dir"); + await linkFixtureNodeModules(fixtureRoot, repositoryRoot); const git = spawnSync("git", ["show", "-s", "--format=%H%n%ct", "HEAD"], { cwd: repositoryRoot, encoding: "utf8", diff --git a/scripts/lib/fixture-node-modules.ts b/scripts/lib/fixture-node-modules.ts new file mode 100644 index 0000000..2b21f27 --- /dev/null +++ b/scripts/lib/fixture-node-modules.ts @@ -0,0 +1,31 @@ +import { lstat, mkdir, readdir, symlink } from "node:fs/promises"; +import path from "node:path"; + +/** + * Links the repository's installed dependencies into a throwaway fixture root. + * + * The obvious form — one directory symlink at `/node_modules` — is + * destructive. A fixture runs `pnpm` inside itself, pnpm does not recognise the + * modules directory it finds there, and its purge follows the symlink and + * deletes the *repository's* real dependencies mid-run. With `CI=true` that + * happens without a prompt, so a test suite silently uninstalls the workspace + * it is running in. + * + * `node_modules` is therefore a real directory here, and every entry inside it + * is an individual symlink. Package resolution is unchanged, but a recursive + * delete unlinks the fixture's own symlinks instead of walking through one link + * into the shared tree. + */ +export async function linkFixtureNodeModules( + fixtureRoot: string, + sourceRoot: string = process.cwd(), +): Promise { + const source = path.join(sourceRoot, "node_modules"); + const target = path.join(fixtureRoot, "node_modules"); + await mkdir(target, { recursive: true }); + for (const entry of await readdir(source)) { + const from = path.join(source, entry); + const stats = await lstat(from); + await symlink(from, path.join(target, entry), stats.isDirectory() ? "dir" : "file"); + } +} diff --git a/scripts/lib/removal-fixture.ts b/scripts/lib/removal-fixture.ts index e325fb6..f78bdfb 100644 --- a/scripts/lib/removal-fixture.ts +++ b/scripts/lib/removal-fixture.ts @@ -5,7 +5,6 @@ import { readFile, readdir, rm, - symlink, writeFile, } from "node:fs/promises"; import path from "node:path"; @@ -14,6 +13,7 @@ import { loadCiGateContract, parseCiGateContract, } from "../contracts/ci-gates.ts"; +import { linkFixtureNodeModules } from "./fixture-node-modules.ts"; import { generateCiWorkflow } from "../generate-ci-workflow.ts"; export const REMOVAL_FIXTURE_COPY_TARGETS = Object.freeze([ @@ -42,7 +42,7 @@ export async function prepareRemovalFixture( for (const target of copyTargets) { await cp(target, path.join(root, target), { recursive: true }); } - await symlink(path.resolve("node_modules"), path.join(root, "node_modules"), "dir"); + await linkFixtureNodeModules(root); } export function runRemovalFixturePnpm( diff --git a/tests/integration/security-followup-archive.test.ts b/tests/integration/security-followup-archive.test.ts index cc8492f..a1260ee 100644 --- a/tests/integration/security-followup-archive.test.ts +++ b/tests/integration/security-followup-archive.test.ts @@ -1,5 +1,5 @@ import { spawnSync } from "node:child_process"; -import { cp, mkdir, mkdtemp, readFile, rm, symlink, writeFile } from "node:fs/promises"; +import { cp, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -15,6 +15,7 @@ import { RELEASE_CANDIDATE_MANIFEST_PATH, releaseCandidateManifestSchema, } from "../../scripts/lib/release-candidate.ts"; +import { linkFixtureNodeModules } from "../../scripts/lib/fixture-node-modules.ts"; it( "builds a real candidate assessment and passes the default archived verifier from the captured archive", @@ -43,7 +44,7 @@ it( recursive: true, force: true, }); - await symlink(path.join(sourceRoot, "node_modules"), path.join(fixtureRoot, "node_modules"), "dir"); + await linkFixtureNodeModules(fixtureRoot, sourceRoot); const git = spawnSync("git", ["show", "-s", "--format=%H%n%ct", "HEAD"], { cwd: sourceRoot, encoding: "utf8", diff --git a/tests/unit/ci-artifact-contract.test.ts b/tests/unit/ci-artifact-contract.test.ts index 12d2903..9b52fd8 100644 --- a/tests/unit/ci-artifact-contract.test.ts +++ b/tests/unit/ci-artifact-contract.test.ts @@ -19,6 +19,7 @@ import { validateCiArtifact, } from "../../scripts/lib/ci-artifact-validator.ts"; import { writeCiGateLogAtomic } from "../../scripts/lib/ci-gate-log.ts"; +import { linkFixtureNodeModules } from "../../scripts/lib/fixture-node-modules.ts"; import { captureCiCandidateArchive, verifyCiCandidateArchive } from "../../scripts/lib/ci-candidate-archive.ts"; import { CANDIDATE_ARCHIVE_USAGE, @@ -2131,7 +2132,7 @@ async function ensureProviderBaseFixture(): Promise { recursive: true, }); await rm(path.join(root, "artifacts/release"), { recursive: true, force: true }); - await symlink(path.join(sourceRoot, "node_modules"), path.join(root, "node_modules"), "dir"); + await linkFixtureNodeModules(root, sourceRoot); const git = spawnSync("git", ["show", "-s", "--format=%H%n%ct", "HEAD"], { cwd: sourceRoot, encoding: "utf8", diff --git a/tests/unit/fixture-node-modules.test.ts b/tests/unit/fixture-node-modules.test.ts new file mode 100644 index 0000000..3059092 --- /dev/null +++ b/tests/unit/fixture-node-modules.test.ts @@ -0,0 +1,89 @@ +import { mkdtemp, mkdir, readdir, readFile, rm, stat, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { linkFixtureNodeModules } from "../../scripts/lib/fixture-node-modules.ts"; + +/** + * A removal or provider fixture runs `pnpm` inside a throwaway copy of the + * repository. pnpm does not recognise the modules directory it finds there and + * purges it, and with `CI=true` it does so without a prompt. When + * `/node_modules` was a single directory symlink, that purge followed + * the link and deleted the repository's own installed dependencies mid-run — + * a test suite uninstalling the workspace it was running in. + */ +describe("fixture node_modules linking", () => { + async function sourceTree() { + const source = await mkdtemp(path.join(tmpdir(), "fixture-source-")); + const modules = path.join(source, "node_modules"); + await mkdir(path.join(modules, "left", "lib"), { recursive: true }); + await mkdir(path.join(modules, "@scope", "right"), { recursive: true }); + await mkdir(path.join(modules, ".bin"), { recursive: true }); + await writeFile(path.join(modules, "left", "lib", "index.js"), "export default 1;\n"); + await writeFile(path.join(modules, "@scope", "right", "package.json"), "{}\n"); + await writeFile(path.join(modules, ".modules.yaml"), "{}\n"); + return source; + } + + it("survives a recursive delete of the fixture's node_modules", async () => { + const source = await sourceTree(); + const fixture = await mkdtemp(path.join(tmpdir(), "fixture-root-")); + try { + await linkFixtureNodeModules(fixture, source); + + // The fixture resolves the same packages... + expect( + await readFile( + path.join(fixture, "node_modules/left/lib/index.js"), + "utf8", + ), + ).toContain("export default 1;"); + expect( + (await readdir(path.join(fixture, "node_modules"))).sort(), + ).toEqual([".bin", ".modules.yaml", "@scope", "left"]); + + // ...and this is exactly what pnpm's purge does. + await rm(path.join(fixture, "node_modules"), { + recursive: true, + force: true, + }); + + // The repository's real dependencies are untouched. + expect( + await readFile( + path.join(source, "node_modules/left/lib/index.js"), + "utf8", + ), + ).toContain("export default 1;"); + expect( + await stat(path.join(source, "node_modules/@scope/right/package.json")), + ).toBeDefined(); + expect( + (await readdir(path.join(source, "node_modules"))).sort(), + ).toEqual([".bin", ".modules.yaml", "@scope", "left"]); + } finally { + await rm(source, { recursive: true, force: true }); + await rm(fixture, { recursive: true, force: true }); + } + }); + + it("creates node_modules as a real directory, never a symlink", async () => { + const source = await sourceTree(); + const fixture = await mkdtemp(path.join(tmpdir(), "fixture-root-")); + try { + await linkFixtureNodeModules(fixture, source); + const { lstat } = await import("node:fs/promises"); + const entry = await lstat(path.join(fixture, "node_modules")); + expect(entry.isSymbolicLink()).toBe(false); + expect(entry.isDirectory()).toBe(true); + expect( + (await lstat(path.join(fixture, "node_modules/left"))).isSymbolicLink(), + ).toBe(true); + } finally { + await rm(source, { recursive: true, force: true }); + await rm(fixture, { recursive: true, force: true }); + } + }); +});