diff --git a/changelog.d/fixes/pack-entrypoint-closures.md b/changelog.d/fixes/pack-entrypoint-closures.md new file mode 100644 index 0000000000..09e114614d --- /dev/null +++ b/changelog.d/fixes/pack-entrypoint-closures.md @@ -0,0 +1 @@ +- **Packaging**: pack-artifact closure tests now cover EVERY npm-shipped dist wrapper (derived from `EXTRA_MODULE_ENTRIES`) and the `bin/omniroute.mjs` CLI entry — including dynamic `import()` and `require()` forms the original server-ws test missed — requiring each local import in both the prepublish prune allowlist and `check:pack-artifact`; `bin/cli/data-dir.mjs` and `bin/cli/utils/storageKeyProvision.mjs` are now required tarball paths (#7065 class hardening, WS1.1) diff --git a/scripts/build/pack-artifact-policy.ts b/scripts/build/pack-artifact-policy.ts index 88329052a1..9adde22c1f 100644 --- a/scripts/build/pack-artifact-policy.ts +++ b/scripts/build/pack-artifact-policy.ts @@ -160,6 +160,12 @@ export const PACK_ARTIFACT_REQUIRED_PATHS: string[] = [ "dist/head-response-guard.cjs", "dist/webdav-handler.mjs", "bin/cli/program.mjs", + // Direct imports of bin/omniroute.mjs — bin/cli/ is only an allowlist PREFIX, so a + // file vanishing from the tarball never fails the unexpected-paths check; only these + // required entries make its absence loud (#7065 class; derived + enforced by + // tests/unit/pack-artifact-entrypoint-closures.test.ts). + "bin/cli/data-dir.mjs", + "bin/cli/utils/storageKeyProvision.mjs", "bin/mcp-server.mjs", "bin/nodeRuntimeSupport.mjs", "bin/omniroute.mjs", diff --git a/tests/unit/pack-artifact-entrypoint-closures.test.ts b/tests/unit/pack-artifact-entrypoint-closures.test.ts new file mode 100644 index 0000000000..78b4826417 --- /dev/null +++ b/tests/unit/pack-artifact-entrypoint-closures.test.ts @@ -0,0 +1,125 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { + APP_STAGING_ALLOWED_EXACT_PATHS, + PACK_ARTIFACT_REQUIRED_PATHS, +} from "../../scripts/build/pack-artifact-policy.ts"; + +// Generalization of pack-artifact-server-ws-closure.test.ts (#7065 class, 3rd recurrence: +// tls-options/3.8.41, head-response-guard VPS #7040 + npm #7065). assembleStandalone copies +// wrapper modules to the dist ROOT; the prepublish prune then deletes anything not in +// APP_STAGING_ALLOWED_EXACT_PATHS, and check:pack-artifact only fails for entries in +// PACK_ARTIFACT_REQUIRED_PATHS. The original test hardcoded ONE wrapper (server-ws.mjs) +// and ONE import form (static `from "./x"`). This suite derives the full closure from the +// sources of truth — EXTRA_MODULE_ENTRIES in assembleStandalone.mjs plus each wrapper's own +// imports (static, dynamic import() and require()) — so adding an import to ANY npm-shipped +// wrapper without updating both lists fails here instead of shipping a boot-crashing tarball. + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); +const ASSEMBLE = path.join(ROOT, "scripts", "build", "assembleStandalone.mjs"); +const BIN_ENTRY = path.join(ROOT, "bin", "omniroute.mjs"); + +interface WrapperEntry { + src: string; + dest: string; +} + +// EXTRA_MODULE_ENTRIES entries whose dest is a bare module filename at the dist root — +// these are the boot-path wrappers the prune can silently drop (the #7065 shape). +function distRootWrappers(): WrapperEntry[] { + const text = fs.readFileSync(ASSEMBLE, "utf8"); + const entries = [...text.matchAll(/src:\s*\[([^\]]+)\],?\s*dest:\s*\[([^\]]+)\]/gs)]; + const toPath = (segmentList: string) => + segmentList + .split(",") + .map((s) => s.trim().replace(/^"|"$/g, "")) + .filter(Boolean) + .join("/"); + return entries + .map((m) => ({ src: toPath(m[1]), dest: toPath(m[2]) })) + .filter((e) => !e.dest.includes("/") && /\.(mjs|cjs|js)$/.test(e.dest)); +} + +// Local sibling imports of a module: static `from "./x"`, dynamic `import("./x")`, +// and CommonJS `require("./x")`. The original test missed the dynamic form — server-ws +// boots dist/server.js via `await import("./server.js")`. +function localImports(filePath: string): string[] { + const src = fs.readFileSync(filePath, "utf8"); + const patterns = [ + /from\s+["']\.\/([^"']+)["']/g, + /import\(\s*["']\.\/([^"']+)["']\s*\)/g, + /require\(\s*["']\.\/([^"']+)["']\s*\)/g, + ]; + return [...new Set(patterns.flatMap((re) => [...src.matchAll(re)].map((m) => m[1])))]; +} + +// Wrappers that ship in the npm channel are exactly those whose dest survives the prune. +// Wrappers intentionally outside the npm tarball (e.g. healthcheck.mjs, Docker-only) are +// excluded: their imports live or die with them, consistently. +function npmShippedWrappers(): WrapperEntry[] { + return distRootWrappers().filter((e) => APP_STAGING_ALLOWED_EXACT_PATHS.includes(e.dest)); +} + +test("sanity: EXTRA_MODULE_ENTRIES parsing finds the known dist-root wrappers", () => { + const dests = distRootWrappers().map((e) => e.dest); + for (const known of ["server-ws.mjs", "peer-stamp.mjs", "head-response-guard.cjs"]) { + assert.ok(dests.includes(known), `parser lost known wrapper ${known}: got ${dests.join(", ")}`); + } + assert.ok(dests.length >= 7, `parsed only ${dests.length} dist-root wrappers`); +}); + +test("every local import of every npm-shipped wrapper survives the prune (allowlist)", () => { + for (const wrapper of npmShippedWrappers()) { + const srcPath = path.join(ROOT, wrapper.src); + assert.ok(fs.existsSync(srcPath), `EXTRA_MODULE_ENTRIES src missing on disk: ${wrapper.src}`); + const missing = localImports(srcPath).filter( + (f) => !APP_STAGING_ALLOWED_EXACT_PATHS.includes(f) + ); + assert.deepEqual( + missing, + [], + `${wrapper.dest}: add to APP_STAGING_ALLOWED_EXACT_PATHS: ${missing.join(", ")}` + ); + } +}); + +test("every local import of every npm-shipped wrapper is enforced by check:pack-artifact", () => { + for (const wrapper of npmShippedWrappers()) { + const missing = localImports(path.join(ROOT, wrapper.src)).filter( + (f) => !PACK_ARTIFACT_REQUIRED_PATHS.includes(`dist/${f}`) + ); + assert.deepEqual( + missing, + [], + `${wrapper.dest}: add dist/ to PACK_ARTIFACT_REQUIRED_PATHS: ${missing.join(", ")}` + ); + } +}); + +test("dynamic import() closure is covered (server-ws boots dist/server.js)", () => { + const serverWs = distRootWrappers().find((e) => e.dest === "server-ws.mjs"); + assert.ok(serverWs, "server-ws.mjs wrapper not found in EXTRA_MODULE_ENTRIES"); + const imports = localImports(path.join(ROOT, serverWs.src)); + assert.ok( + imports.includes("server.js"), + `dynamic import extraction broken — server.js not among: ${imports.join(", ")}` + ); +}); + +test("every bin/omniroute.mjs local import is enforced by check:pack-artifact", () => { + // The CLI boot path (bin/omniroute.mjs → bin/cli/*) is covered by allowlist PREFIXES, + // so a file vanishing from the tarball never fails the unexpected-paths check — only + // PACK_ARTIFACT_REQUIRED_PATHS makes its absence loud. Derive the requirement from + // the entrypoint's own imports. + const missing = localImports(BIN_ENTRY).filter( + (f) => !PACK_ARTIFACT_REQUIRED_PATHS.includes(`bin/${f}`) + ); + assert.deepEqual( + missing, + [], + `add bin/ to PACK_ARTIFACT_REQUIRED_PATHS: ${missing.join(", ")}` + ); +}); diff --git a/tests/unit/pack-artifact-policy.test.ts b/tests/unit/pack-artifact-policy.test.ts index a5f78e2235..ae24d23dea 100644 --- a/tests/unit/pack-artifact-policy.test.ts +++ b/tests/unit/pack-artifact-policy.test.ts @@ -105,7 +105,9 @@ test("findMissingArtifactPaths flags missing root runtime files in the tarball", // alphabetically (bin/ < dist/ < scripts/ < src/), minus the paths present // above (dist/server.js, bin/omniroute.mjs, package.json, the postinstall scripts). assert.deepEqual(missingPaths, [ + "bin/cli/data-dir.mjs", "bin/cli/program.mjs", + "bin/cli/utils/storageKeyProvision.mjs", "bin/mcp-server.mjs", "bin/nodeRuntimeSupport.mjs", "dist/head-response-guard.cjs",