diff --git a/scripts/generate-npm-package-lock.mts b/scripts/generate-npm-package-lock.mts index 9261c249b6dc..e2753e7708ed 100644 --- a/scripts/generate-npm-package-lock.mts +++ b/scripts/generate-npm-package-lock.mts @@ -1158,24 +1158,51 @@ type OverrideViolation = { path: string; }; -// pnpm overrides cannot replace npm's bundled dependencies. Keep these exact -// exceptions bound to the reviewed npm tarball and its bundled package markers. -// Exact paths, versions, and bundle markers keep this exception out of other trees. -const NPM_12_1_0_BUNDLED_EXCEPTIONS = new Map([ - ["node_modules/npm/node_modules/minimatch", "10.2.5"], - ["node_modules/npm/node_modules/brace-expansion", "5.0.9"], - ["node_modules/npm/node_modules/ip-address", "10.5.0"], +type NpmBundledDependencyPolicy = { + allowMissingBundleMarker: boolean; + exceptions: Map; +}; + +// Trusted release tooling validates frozen targets as well as current main. Keep each +// reviewed npm tarball exact here until no supported frozen target can reference it. +const NPM_BUNDLED_DEPENDENCY_POLICIES = new Map([ + [ + "11.20.0", + { + allowMissingBundleMarker: true, + exceptions: new Map([ + ["node_modules/npm/node_modules/minimatch", "10.2.5"], + ["node_modules/npm/node_modules/brace-expansion", "5.0.9"], + ["node_modules/npm/node_modules/ip-address", "10.5.0"], + ]), + }, + ], + [ + "12.1.0", + { + allowMissingBundleMarker: false, + exceptions: new Map([ + ["node_modules/npm/node_modules/minimatch", "10.2.5"], + ["node_modules/npm/node_modules/brace-expansion", "5.0.9"], + ["node_modules/npm/node_modules/ip-address", "10.5.0"], + ]), + }, + ], ]); function isApprovedNpmBundledDependency(packages: UnknownRecord, lockPath: string) { - const expectedVersion = NPM_12_1_0_BUNDLED_EXCEPTIONS.get(lockPath); const npm = recordAt(packages, "node_modules/npm"); + const npmVersion = typeof npm?.version === "string" ? npm.version : undefined; + const policy = npmVersion ? NPM_BUNDLED_DEPENDENCY_POLICIES.get(npmVersion) : undefined; + const expectedVersion = policy?.exceptions.get(lockPath); const dependency = recordAt(packages, lockPath); return ( expectedVersion !== undefined && - npm?.version === "12.1.0" && + npm !== undefined && (npm.name === undefined || npm.name === "npm") && - dependency?.inBundle === true && + dependency !== undefined && + (dependency.inBundle === true || + (policy?.allowMissingBundleMarker === true && dependency.inBundle === undefined)) && dependency.version === expectedVersion && (dependency.name === undefined || dependency.name === lockPath.split("/").at(-1)) ); @@ -1621,10 +1648,13 @@ function normalizeNpmVersionDrift(lockfile: T): T { if (!packages) { return lockfile; } - for (const metadata of Object.values(packages)) { + for (const [lockPath, metadata] of Object.entries(packages)) { if (!isRecord(metadata)) { continue; } + if (metadata.inBundle === undefined && isApprovedNpmBundledDependency(packages, lockPath)) { + metadata.inBundle = true; + } // npm versions and mutable registry metadata disagree on these package-lock // fields. None affect resolution, so keep generated npm locks stable. delete metadata.deprecated; @@ -1691,13 +1721,14 @@ export function generateNpmPackageLock(packageDir: string, options: NpmLockOptio ), ); let npmBundleTarball: Buffer | undefined; - if (recordAt(recordAt(generated, "packages"), "node_modules/npm")?.version === "12.1.0") { + const npmVersion = recordAt(recordAt(generated, "packages"), "node_modules/npm")?.version; + if (typeof npmVersion === "string" && NPM_BUNDLED_DEPENDENCY_POLICIES.has(npmVersion)) { runNpm( - ["pack", "npm@12.1.0", "--ignore-scripts", "--pack-destination", tempDir], + ["pack", `npm@${npmVersion}`, "--ignore-scripts", "--pack-destination", tempDir], tempDir, env, ); - npmBundleTarball = readFileSync(path.join(tempDir, "npm-12.1.0.tgz")); + npmBundleTarball = readFileSync(path.join(tempDir, `npm-${npmVersion}.tgz`)); } assertNpmLockMatchesPnpmLock(generated, localPackageArtifacts, npmBundleTarball); return `${JSON.stringify(generated, null, 2)}\n`; @@ -1718,15 +1749,18 @@ function verifiedNpmBundlePackages( return manifests; } const npm = recordAt(packages, "node_modules/npm"); + const npmVersion = typeof npm?.version === "string" ? npm.version : undefined; + const policy = npmVersion ? NPM_BUNDLED_DEPENDENCY_POLICIES.get(npmVersion) : undefined; const integrity = `sha512-${createHash("sha512").update(tarball).digest("base64")}`; if ( - npm?.version !== "12.1.0" || + !policy || + !npm || (npm.name !== undefined && npm.name !== "npm") || npm.integrity !== integrity || - !pnpmIntegrities.get("npm@12.1.0")?.has(integrity) + !pnpmIntegrities.get(`npm@${npmVersion}`)?.has(integrity) ) { throw new Error( - "npm bundled dependency tarball does not match the pnpm-locked npm@12.1.0 integrity", + `npm bundled dependency tarball does not match the pnpm-locked npm@${npmVersion ?? "unknown"} integrity`, ); } let parseError: Error | undefined; @@ -1763,7 +1797,7 @@ function verifiedNpmBundlePackages( throw parseError; } const root = manifests.get(""); - if (root?.name !== "npm" || root.version !== "12.1.0") { + if (root?.name !== "npm" || root.version !== npmVersion) { throw new Error("npm bundled dependency tarball has an unexpected package identity"); } return new Map( diff --git a/test/scripts/generate-npm-package-lock.test.ts b/test/scripts/generate-npm-package-lock.test.ts index 1c1a9d90881b..76f9024f1b0b 100644 --- a/test/scripts/generate-npm-package-lock.test.ts +++ b/test/scripts/generate-npm-package-lock.test.ts @@ -786,13 +786,17 @@ describe("generate-npm-package-lock", () => { ).toEqual([]); }); - it.each([ - { name: "minimatch", version: "10.2.5", required: "10.2.6" }, - { name: "brace-expansion", version: "5.0.9", required: "5.0.12" }, - { name: "ip-address", version: "10.5.0", required: "10.7.2" }, - ])( - "limits the npm bundled exception for $name to its approved occurrence", - ({ name, version, required }) => { + it.each( + [ + { name: "minimatch", version: "10.2.5", required: "10.2.6" }, + { name: "brace-expansion", version: "5.0.9", required: "5.0.12" }, + { name: "ip-address", version: "10.5.0", required: "10.7.2" }, + ].flatMap((entry) => + ["11.20.0", "12.1.0"].map((npmVersion) => Object.assign({ npmVersion }, entry)), + ), + )( + "limits the npm@$npmVersion bundled exception for $name to its approved occurrence", + ({ name, version, required, npmVersion }) => { for (const change of [ "approved", "bundle version", @@ -800,20 +804,30 @@ describe("generate-npm-package-lock", () => { "path", "unbundled", ] as const) { - const npmVersion = change === "npm version" ? "12.1.1" : "12.1.0"; + const installedNpmVersion = + change === "npm version" + ? `${npmVersion.slice(0, npmVersion.lastIndexOf(".") + 1)}1` + : npmVersion; const bundledVersion = change === "bundle version" ? "0.0.1" : version; const npmPath = change === "path" ? "node_modules/other/node_modules/npm" : "node_modules/npm"; const childPath = `${npmPath}/node_modules/${name}`; const lockfile = { packages: { - "": { dependencies: { npm: npmVersion } }, + "": { dependencies: { npm: installedNpmVersion } }, [npmPath]: { - version: npmVersion, + version: installedNpmVersion, dependencies: { [name]: bundledVersion }, hasShrinkwrap: true, }, - [childPath]: { version: bundledVersion, inBundle: change !== "unbundled" }, + [childPath]: { + version: bundledVersion, + ...(change === "unbundled" + ? { inBundle: false } + : npmVersion === "12.1.0" + ? { inBundle: true } + : {}), + }, }, }; const rules = { [name]: required }; @@ -825,7 +839,7 @@ describe("generate-npm-package-lock", () => { expect( collectPnpmLockViolations( lockfile, - new Set([`npm@${npmVersion}`, `${name}@${required}`]), + new Set([`npm@${installedNpmVersion}`, `${name}@${required}`]), new Map(), ).map((entry) => entry.path), change, @@ -837,59 +851,62 @@ describe("generate-npm-package-lock", () => { }, ); - it("authenticates bundled lock entries against the pnpm-locked npm archive", () => { - const root = tempDirs.make("openclaw-npm-bundle-"); - const child = "node_modules/@npmcli/arborist"; - const childPath = `node_modules/npm/${child}`; - mkdirSync(path.join(root, "package", child), { recursive: true }); - writeFileSync( - path.join(root, "package/package.json"), - JSON.stringify({ name: "npm", version: "12.1.0" }), - ); - writeFileSync( - path.join(root, "package", child, "package.json"), - JSON.stringify({ name: "@npmcli/arborist", version: "9.9.2" }), - ); - const archive = path.join(root, "npm.tgz"); - createTar({ cwd: root, file: archive, sync: true, gzip: true }, ["package"]); - const bytes = readFileSync(archive); - const integrity = `sha512-${createHash("sha512").update(bytes).digest("base64")}`; - const lockfile = { - packages: { - "node_modules/npm": { version: "12.1.0", integrity }, - [childPath]: { version: "9.9.2", inBundle: true }, - }, - }; - const pins = new Set(["npm@12.1.0"]); - const integrities = new Map([["npm@12.1.0", new Set([integrity])]]); - const check = ( - value: Parameters[0] = lockfile, - tarball = bytes, - hashes = integrities, - ) => collectPnpmLockViolations(value, pins, hashes, [], tarball); - expect(check()).toEqual([]); - expect(() => check(lockfile, Buffer.concat([bytes, Buffer.from("changed")]))).toThrow( - "integrity", - ); - expect(() => check(lockfile, bytes, new Map())).toThrow("integrity"); - for (const change of ["version", "path", "unbundled", "name"]) { - const entryPath = change === "path" ? `node_modules/other/${child}` : childPath; - const changed = { + it.each(["11.20.0", "12.1.0"])( + "authenticates npm@%s bundled lock entries against the pnpm-locked archive", + (npmVersion) => { + const root = tempDirs.make("openclaw-npm-bundle-"); + const child = "node_modules/@npmcli/arborist"; + const childPath = `node_modules/npm/${child}`; + mkdirSync(path.join(root, "package", child), { recursive: true }); + writeFileSync( + path.join(root, "package/package.json"), + JSON.stringify({ name: "npm", version: npmVersion }), + ); + writeFileSync( + path.join(root, "package", child, "package.json"), + JSON.stringify({ name: "@npmcli/arborist", version: "9.9.2" }), + ); + const archive = path.join(root, "npm.tgz"); + createTar({ cwd: root, file: archive, sync: true, gzip: true }, ["package"]); + const bytes = readFileSync(archive); + const integrity = `sha512-${createHash("sha512").update(bytes).digest("base64")}`; + const lockfile = { packages: { - "node_modules/npm": lockfile.packages["node_modules/npm"], - [entryPath]: { - version: change === "version" ? "9.9.1" : "9.9.2", - inBundle: change !== "unbundled", - ...(change === "name" ? { name: "other" } : {}), - }, + "node_modules/npm": { version: npmVersion, integrity }, + [childPath]: { version: "9.9.2", inBundle: true }, }, }; - expect( - check(changed).map((entry) => entry.path), - change, - ).toEqual([entryPath]); - } - }); + const pins = new Set([`npm@${npmVersion}`]); + const integrities = new Map([[`npm@${npmVersion}`, new Set([integrity])]]); + const check = ( + value: Parameters[0] = lockfile, + tarball = bytes, + hashes = integrities, + ) => collectPnpmLockViolations(value, pins, hashes, [], tarball); + expect(check()).toEqual([]); + expect(() => check(lockfile, Buffer.concat([bytes, Buffer.from("changed")]))).toThrow( + "integrity", + ); + expect(() => check(lockfile, bytes, new Map())).toThrow("integrity"); + for (const change of ["version", "path", "unbundled", "name"]) { + const entryPath = change === "path" ? `node_modules/other/${child}` : childPath; + const changed = { + packages: { + "node_modules/npm": lockfile.packages["node_modules/npm"], + [entryPath]: { + version: change === "version" ? "9.9.1" : "9.9.2", + inBundle: change !== "unbundled", + ...(change === "name" ? { name: "other" } : {}), + }, + }, + }; + expect( + check(changed).map((entry) => entry.path), + change, + ).toEqual([entryPath]); + } + }, + ); it("detects npm package-lock entries that bypass the pnpm lock", () => { const lockfile = {