From 08e805c5504e124b1e9dbac6a1d6a5b42f542c14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sun, 11 Jan 2026 21:20:39 +0100 Subject: [PATCH] prefetch-npm-deps: normalize packuments using whitelist approach Switch from stripping known volatile fields to using an explicit whitelist of allowed fields. This is more robust against upstream changes that add new fields which could affect hash stability. Top-level: only name and versions (dist-tags and time not needed for lockfile installs where versions are already resolved) Version-level: identity, all dependency types, dist, bin, platform constraints (engines/os/cpu), scripts, and deprecated flag. Based on analysis of pacote, npm-pick-manifest, npm-install-checks, and arborist - only fields actually read during npm install are included. --- .../node/prefetch-npm-deps/src/main.rs | 65 ++++++++++++------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/pkgs/build-support/node/prefetch-npm-deps/src/main.rs b/pkgs/build-support/node/prefetch-npm-deps/src/main.rs index 7666474f9fe1..53cc075bfdc0 100644 --- a/pkgs/build-support/node/prefetch-npm-deps/src/main.rs +++ b/pkgs/build-support/node/prefetch-npm-deps/src/main.rs @@ -33,8 +33,42 @@ fn get_packument_url(registry: &str, package_name: &str) -> anyhow::Result /// Normalize packument data to ensure determinism. /// -/// Strips volatile fields like `_rev`, `time`, and `modified`. -/// Filters the `versions` map to only include versions requested in the lockfile. +/// Filters to whitelisted fields and requested versions only. +/// Allowed top-level fields in normalized packuments. +/// +/// For lockfile-based installs, versions are exact (e.g., "4.17.21") so npm-pick-manifest +/// just does a direct `versions[ver]` lookup. Tarballs are fetched via the resolved URL. +const ALLOWED_TOP_LEVEL_FIELDS: &[&str] = &["name", "versions"]; + +/// Allowed fields in version objects. +/// +/// Based on analysis of pacote, npm-pick-manifest, npm-install-checks, and arborist. +/// Only fields actually read during `npm install` are included. +const ALLOWED_VERSION_FIELDS: &[&str] = &[ + "name", + "version", + // Dependencies + "dependencies", + "devDependencies", + "peerDependencies", + "peerDependenciesMeta", + "optionalDependencies", + "bundleDependencies", + "bundledDependencies", + // Distribution (tarball URL and integrity) + "dist", + // Executables + "bin", + // Platform constraints (npm-install-checks) + "engines", + "os", + "cpu", + // Lifecycle scripts + "scripts", + // Version selection hint (npm-pick-manifest) + "deprecated", +]; + fn normalize_packument( package_name: &str, data: &[u8], @@ -47,37 +81,22 @@ fn normalize_packument( .as_object_mut() .ok_or_else(|| anyhow!("packument for {package_name} is not a JSON object"))?; - // Strip volatile top-level fields - obj.remove("_rev"); - obj.remove("time"); - obj.remove("modified"); + // Keep only whitelisted top-level fields to ensure determinism + obj.retain(|key, _| ALLOWED_TOP_LEVEL_FIELDS.contains(&key.as_str())); - // Filter versions to only those in lockfile + // Filter and normalize versions if let Some(Value::Object(versions)) = obj.get_mut("versions") { + // Only keep versions that are in the lockfile versions.retain(|version, _| requested_versions.contains(version)); - // Normalize each version object + // Normalize each version object to only include necessary fields for version_val in versions.values_mut() { if let Some(version_obj) = version_val.as_object_mut() { - // Strip fields starting with underscore (volatile/internal) - version_obj.retain(|key, _| !key.starts_with('_')); - // Strip other often-volatile fields - version_obj.remove("gitHead"); + version_obj.retain(|key, _| ALLOWED_VERSION_FIELDS.contains(&key.as_str())); } } } - // Filter dist-tags to only point to versions we kept - if let Some(Value::Object(tags)) = obj.get_mut("dist-tags") { - tags.retain(|_, version_val| { - if let Some(version) = version_val.as_str() { - requested_versions.contains(version) - } else { - false - } - }); - } - serde_json::to_vec(&json) .map_err(|e| anyhow!("failed to re-serialize packument for {package_name}: {e}")) }