diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 9b37242d4..96eb5f433 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1490,6 +1490,7 @@ command's integration tests check its output. | `hosted_wiring_contested` | top-level `error.code` (list: warning when it can still list) | rollback / remove / vendor eject / list (v5.0): a lockfile mentions a recognized hosted patch uuid that discovery rejected (or a pin with no lockfile), so the hosted set is not known exactly; refused with nothing touched, exit 1. Remedy: fix or `git checkout` the named lockfile. | | `vendor_pnpm_settings_elsewhere` | `failed` | vendor / scan / get `--mode vendored` (pnpm, v9 lock): the project directory is a pnpm workspace member (listed by the `packages:` globs of the nearest ancestor `pnpm-workspace.yaml`, read as for `redirect_pnpm_settings_elsewhere`) with its own `pnpm-lock.yaml` and no `pnpm-workspace.yaml` of its own; pnpm reads `overrides:` only from the nearest ancestor `pnpm-workspace.yaml`, so an override wired into the member (its `package.json` or a nested workspace file) would be ignored, failing frozen installs on pnpm >= 11 and silently reinstalling the unpatched package on a plain install. Refused before any write (the pre-download preflight and `--dry-run` included); the detail names the governing file; remedy: `--mode hosted`. A directory the globs do not list is a standalone project: the override is wired into a new `pnpm-workspace.yaml` of its own. | | `vendor_pnpm_lock_multi_document` | `failed` | vendor / scan / get `--mode vendored` (pnpm, v9 lock): `pnpm-lock.yaml` has several YAML documents but is not pnpm's two-document layout — a `---`, an env document whose importers carry only `configDependencies` / `packageManagerDependencies` (pnpm >= 11 with config dependencies, pnpm 12 with `packageManager` set), a `---`, then the project lock with its own `lockfileVersion: '9.0'`. The vendored backend edits only that project document (the env document is written back byte-for-byte, #466) and will not guess for any other layout (three or more documents, a `...` end marker, a byte-order mark before a separator, an unrecognised first document). Refused before any write (the pre-download preflight and `--dry-run` included); remedy: `--mode hosted`. A vendored pnpm revert over such a lock (`vendor --revert`, `rollback`, the vendored → hosted takeover) fails as a revert failure whose detail names the lock, dry run included, before touching any surface, rather than reverting `package.json` / `pnpm-workspace.yaml` around a lock left wired to the artifact. | +| `vendor_pnpm_build_approval_lost` | `skipped` (warning) | vendor / scan / get `--mode vendored` (pnpm, v9 lock, #1597): the project decides the vendored package's install script (`allowBuilds` / `onlyBuiltDependencies`, in `pnpm-workspace.yaml` or `package.json#pnpm`) by a key pnpm 10.34.2+ / 11.5.3+ no longer matches against the vendored `name@file:` id (pnpm 10 skips the script, pnpm 11+ fail every install with `ERR_PNPM_IGNORED_BUILDS`), and the decision was not carried: its value is flow-style or not a boolean, or no pnpm pin (package.json `packageManager` / `devEngines` / `engines.pnpm`, `node_modules/.modules.yaml`) says the project runs pnpm 10.34.2+ (within 10.x), 11.5.3+ (within 11.x) or 12+ (pnpm 10.20–10.34.1 and 11.0–11.5.2 refuse such a key with `ERR_PNPM_INVALID_VERSION_UNION`). The setting is left untouched and the detail names the key to add. With a block-style boolean decision and a pin admitting only such a pnpm the decision is carried to the vendored key automatically (recorded in the ledger, kept in step with the user's decision on re-runs, removed on revert), with no warning; a pin admitting only older pnpm needs nothing. | | `vendor_config_dependency_unpatched` | `skipped` (warning) | vendor / scan / get `--mode vendored` (pnpm, v9 lock): the vendored package is also a pnpm config dependency (its registry key is in the lock's env document). Vendoring wires the project's copy only; the copy pnpm installs under `node_modules/.pnpm-config` keeps the registry bytes. Hosted mode pins both. | | `vendor_pnpm_git_branch_lockfile` | `failed` | vendor / scan / get `--mode vendored` (pnpm, every lockfile version): `gitBranchLockfile` is on and a `pnpm-lock..yaml` exists at the root (or in a workspace member that installs from its own lock under `sharedWorkspaceLockfile: false`), so pnpm installs the branch from that lock while vendoring wires `pnpm-lock.yaml` only (every `--frozen-lockfile` install would then fail on the overrides mismatch). Refused before any write, by the pre-flight too; also raised in place of `vendor_lockfile_missing` when the branch lock is the only pnpm lock. Remedy: turn the setting off and run `pnpm install --merge-git-branch-lockfiles`. Hosted mode's twin is the `redirect_pnpm_git_branch_lockfile` warning (#556). | | `vendor_dir_symlink_unsupported` | `failed` | vendor / scan / get `--mode vendored` (every ecosystem): `.socket` itself (#887), `.socket/vendor`, `.socket/vendor/` or the patch's `` dir is a symlink or junction. socket-patch creates those directories itself and never writes links, so a linked one is not ours; its target may be another project's vendor store. Refused before any write. The vendored revert (`vendor --revert`, `rollback`, `remove`, the vendored → hosted takeover) fails on the same check with the same detail before it edits a lock or deletes anything, so it never deletes another project's artifacts through the link. The detail names the linked path. Remedy: replace the link with a real directory and re-run. | diff --git a/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs index d494fa30b..6619312f8 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs @@ -2568,3 +2568,191 @@ fn pnpm_vendor_refuses_quoted_scoped_alias_references() { ); } } + +/// #1597: pnpm 10.34.2+ matches build approvals against the id it installs +/// a package as, and the vendored copy installs as +/// `sp-build@file:.socket/vendor/npm//…`. A name-keyed approval used +/// to stop applying after vendoring: pnpm 10 skipped the approved +/// `postinstall`, pnpm 11 (strictDepBuilds) failed every frozen install +/// with ERR_PNPM_IGNORED_BUILDS. Vendoring now carries the approval to the +/// vendored key: a fresh frozen offline install of the committed files +/// lands the patched bytes AND runs the script, and `vendor --revert` +/// byte-restores the workspace file. pnpm 10.30 and 11.5.2 (which refuse +/// a `file:` key with ERR_PNPM_INVALID_VERSION_UNION and still match the +/// name) get no key, and their script runs too. +#[tokio::test(flavor = "multi_thread")] +async fn pnpm_vendor_keeps_approved_build_scripts_running() { + for (pm, approval, carried) in [ + (PNPM_TERTIARY, "allowBuilds:\n sp-build: true\n", true), + (PNPM_PRIMARY, "onlyBuiltDependencies:\n - sp-build\n", true), + ( + "pnpm@10.30.0", + "onlyBuiltDependencies:\n - sp-build\n", + false, + ), + // 11.5.3 is where the id matching landed (10.34.2 is its backport); + // 11.5.2 still refuses a `file:` key. + ("pnpm@11.5.3", "allowBuilds:\n sp-build: true\n", true), + ("pnpm@11.5.2", "allowBuilds:\n sp-build: true\n", false), + ] { + if !has_corepack_pm(pm) { + println!("SKIP: `corepack {pm}` unavailable"); + continue; + } + run_build_approval_capstone(pm, approval, carried).await; + } +} + +async fn run_build_approval_capstone(pm: &str, approval: &str, carried: bool) { + const NAME: &str = "sp-build"; + let original: &[u8] = b"module.exports = 'orig';\n"; + let patched: Vec = [MARKER.as_bytes(), original].concat(); + let manifest = serde_json::json!({ + "name": NAME, "version": "1.0.0", "main": "index.js", + "scripts": {"postinstall": "node post.js"}, + }); + let mut builder = tar::Builder::new(flate2::write::GzEncoder::new( + Vec::new(), + flate2::Compression::default(), + )); + for (file, bytes) in [ + ("package.json", manifest.to_string().into_bytes()), + ("index.js", original.to_vec()), + ( + "post.js", + b"require('fs').writeFileSync('built.txt', 'built');\n".to_vec(), + ), + ] { + let mut header = tar::Header::new_gnu(); + header.set_size(bytes.len() as u64); + header.set_mode(0o644); + header.set_cksum(); + builder + .append_data(&mut header, format!("package/{file}"), bytes.as_slice()) + .unwrap(); + } + let tgz = builder.into_inner().unwrap().finish().unwrap(); + + let server = MockServer::start().await; + let route = format!("/{NAME}/-/{NAME}-1.0.0.tgz"); + let mut record = manifest.clone(); + record["dist"] = serde_json::json!({ + "tarball": format!("{}{route}", server.uri()), + "integrity": format!("sha512-{}", b64(&sha2::Sha512::digest(&tgz))), + }); + let packument = serde_json::json!({ + "name": NAME, "dist-tags": {"latest": "1.0.0"}, "versions": {"1.0.0": record}, + }); + Mock::given(method("GET")) + .and(path(route.as_str())) + .respond_with(ResponseTemplate::new(200).set_body_bytes(tgz)) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(format!("/{NAME}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(packument)) + .mount(&server) + .await; + + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(&proj).unwrap(); + std::fs::write( + proj.join("package.json"), + format!( + "{{\"name\":\"approvals\",\"version\":\"0.0.0\",\"private\":true,\ + \"dependencies\":{{\"{NAME}\":\"1.0.0\"}}}}\n" + ), + ) + .unwrap(); + std::fs::write( + proj.join(".npmrc"), + format!("registry={}/\nfetch-retries=0\n", server.uri()), + ) + .unwrap(); + let ws_before = format!("packages:\n - '.'\nminimumReleaseAge: 0\n{approval}"); + std::fs::write(proj.join("pnpm-workspace.yaml"), &ws_before).unwrap(); + + let store = tmp.path().join("pnpm-store"); + let install = corepack( + &proj, + pm, + &["install", "--store-dir", store.to_str().unwrap()], + ); + assert!( + install.status.success(), + "{pm} fixture install: {install:?}" + ); + let built = |root: &Path| root.join("node_modules").join(NAME).join("built.txt"); + assert!(built(&proj).exists(), "{pm}: the approved script ran"); + let lock_before = std::fs::read_to_string(proj.join("pnpm-lock.yaml")).unwrap(); + + let purl = format!("pkg:npm/{NAME}@1.0.0"); + stage_patch(&proj, &purl, "package/index.js", original, &patched); + let cwd = proj.to_str().unwrap(); + let (code, stdout, stderr) = + run_socket(&proj, &["vendor", "--json", "--offline", "--cwd", cwd]); + assert_eq!(code, 0, "{pm} vendor failed.\n{stdout}\n{stderr}"); + let env = parse_envelope(&stdout); + assert_eq!(env["summary"]["applied"], 1, "{env}"); + let ws_after = std::fs::read_to_string(proj.join("pnpm-workspace.yaml")).unwrap(); + assert_eq!( + ws_after.contains(&format!( + "{NAME}@file:.socket/vendor/npm/{UUID}/{NAME}-1.0.0.tgz" + )), + carried, + "{pm}: the approval is carried to the vendored key iff pnpm reads it:\n{ws_after}" + ); + assert!( + !stdout.contains("vendor_pnpm_build_approval_lost"), + "{pm}: the installed pnpm is known, so nothing is lost:\n{stdout}" + ); + + // Fresh checkout of the committable files: empty store, no registry. + let fresh = tmp.path().join("fresh"); + std::fs::create_dir_all(&fresh).unwrap(); + for file in ["package.json", "pnpm-lock.yaml", "pnpm-workspace.yaml"] { + std::fs::copy(proj.join(file), fresh.join(file)).unwrap(); + } + copy_dir_recursive(&proj.join(".socket"), &fresh.join(".socket")); + std::fs::write(fresh.join(".npmrc"), "registry=http://127.0.0.1:1/\n").unwrap(); + let fresh_store = tmp.path().join("fresh-pnpm-store"); + let ci = corepack( + &fresh, + pm, + &[ + "install", + "--frozen-lockfile", + "--offline", + "--store-dir", + fresh_store.to_str().unwrap(), + ], + ); + assert!( + ci.status.success(), + "{pm}: fresh frozen install must keep the approval.\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&ci.stdout), + String::from_utf8_lossy(&ci.stderr), + ); + assert_eq!( + std::fs::read(fresh.join("node_modules").join(NAME).join("index.js")).unwrap(), + patched, + "{pm}: the patched bytes are installed" + ); + assert!( + built(&fresh).exists(), + "{pm}: the approved postinstall still runs on the vendored copy" + ); + + let (code, stdout, stderr) = run_socket(&proj, &["vendor", "--revert", "--json", "--cwd", cwd]); + assert_eq!(code, 0, "{pm} revert failed.\n{stdout}\n{stderr}"); + assert_eq!( + std::fs::read_to_string(proj.join("pnpm-workspace.yaml")).unwrap(), + ws_before, + "{pm}: workspace file byte-restored" + ); + assert_eq!( + std::fs::read_to_string(proj.join("pnpm-lock.yaml")).unwrap(), + lock_before + ); +} diff --git a/crates/socket-patch-core/src/formats/pnpm/mod.rs b/crates/socket-patch-core/src/formats/pnpm/mod.rs index 4a4798a40..2bfd3033c 100644 --- a/crates/socket-patch-core/src/formats/pnpm/mod.rs +++ b/crates/socket-patch-core/src/formats/pnpm/mod.rs @@ -30,7 +30,7 @@ pub(crate) mod lines; mod version; pub(crate) mod workspace; -pub use version::root_only_workspace_breaks_add; +pub use version::{matches_file_id_build_keys, root_only_workspace_breaks_add}; pub(crate) use grammar::{ entry_bundled, entry_field, is_pnpm_lock_text, Bundled, Entry, Resolution, diff --git a/crates/socket-patch-core/src/formats/pnpm/version.rs b/crates/socket-patch-core/src/formats/pnpm/version.rs index 036b96762..139003766 100644 --- a/crates/socket-patch-core/src/formats/pnpm/version.rs +++ b/crates/socket-patch-core/src/formats/pnpm/version.rs @@ -31,11 +31,37 @@ fn breaks_root_only_workspace((lo, hi): (MajorMinor, MajorMinor)) -> bool { lo >= (9, 0) && hi < (10, 5) } +/// `(major, minor, patch)`. +type Release = (u64, u64, u64); + +/// Whether pnpm `release` matches a build decision (`allowBuilds`, +/// `onlyBuiltDependencies`) against a `file:` tarball's own id +/// (`name@file:`), which is also the first release that accepts such +/// a key at all. The change landed in 11.5.3 and was backported to +/// 10.34.2: older releases match the package name, and from at least 10.20 +/// on refuse a `file:` key with ERR_PNPM_INVALID_VERSION_UNION (#1597; +/// measured on 9.15.9, 10.0.0, 10.20.0, 10.30.0, 10.34.1, 10.34.2, 10.34.7, +/// 11.5.2, 11.5.3, 11.28.7 and 12.15.0). +fn matches_file_ids(release: Release) -> bool { + match release.0 { + ..=9 => false, + 10 => release >= (10, 34, 2), + 11 => release >= (11, 5, 3), + _ => true, + } +} + /// The inclusive `(major, minor)` span a version spec admits: an exact /// version (`9.15.9`, `=9.15.9`, `v9.15.9`, a prerelease), `^X.Y.Z`, /// `~X.Y.Z`, or a wildcard (`9`, `9.x`, `10.4.x`, `9.*`). `None` for /// anything else (comparators, unions, hyphen ranges, junk). fn version_span(spec: &str) -> Option<(MajorMinor, MajorMinor)> { + version_bounds(spec).map(|(lo, hi)| ((lo.0, lo.1), (hi.0, hi.1))) +} + +/// [`version_span`] at patch precision. A prerelease counts as its +/// release. +fn version_bounds(spec: &str) -> Option<(Release, Release)> { let spec = spec.trim(); let (op, rest) = match spec.as_bytes().first()? { b'^' => ('^', &spec[1..]), @@ -53,18 +79,22 @@ fn version_span(spec: &str) -> Option<(MajorMinor, MajorMinor)> { Some(part) if wild(part) => None, Some(part) => Some(part.parse::().ok()?), }; - if let Some(patch) = parts.next() { - if !(wild(patch) || patch.parse::().is_ok()) { - return None; - } - } + let patch = match parts.next() { + None => None, + Some(part) if wild(part) => None, + Some(part) => Some(part.parse::().ok()?), + }; if parts.next().is_some() { return None; } - Some(match (op, minor) { - (_, None) => ((major, 0), (major, u64::MAX)), - ('^', Some(minor)) if major > 0 => ((major, minor), (major, u64::MAX)), - (_, Some(minor)) => ((major, minor), (major, minor)), + const MAX: u64 = u64::MAX; + Some(match (op, minor, patch) { + (_, None, _) => ((major, 0, 0), (major, MAX, MAX)), + ('^', Some(minor), patch) if major > 0 => { + ((major, minor, patch.unwrap_or(0)), (major, MAX, MAX)) + } + ('=', Some(minor), Some(patch)) => ((major, minor, patch), (major, minor, patch)), + (_, Some(minor), patch) => ((major, minor, patch.unwrap_or(0)), (major, minor, MAX)), }) } @@ -148,10 +178,97 @@ pub fn root_only_workspace_breaks_add( }) } +/// Whether the pnpm the project runs matches build decisions against a +/// vendored `file:` id (see [`matches_file_ids`]): `Some(true)` when every +/// pin that parses admits only such a release, `Some(false)` when every one +/// admits only older ones, `None` without a parseable pin or when they +/// disagree. +pub fn matches_file_id_build_keys( + package_json: Option<&str>, + modules_yaml: Option<&str>, +) -> Option { + // A span decides when both ends agree within one major (the answer is + // monotonic inside a major, not across the 10 → 11 boundary). + let decided = |(lo, hi): (Release, Release)| { + (lo.0 == hi.0 && matches_file_ids(lo) == matches_file_ids(hi)).then(|| matches_file_ids(lo)) + }; + let mut answers = pnpm_pins(package_json, modules_yaml) + .iter() + .filter_map(|(_, spec)| version_bounds(spec)) + .map(decided) + .collect::>() + .into_iter(); + let first = answers.next()??; + answers.all(|a| a == Some(first)).then_some(first) +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn file_id_build_keys_start_at_10_34_2_and_11_5_3() { + let pm = |spec: &str| format!("{{\"packageManager\":\"pnpm@{spec}\"}}"); + for (spec, want) in [ + ("10.34.2", Some(true)), + ("10.34.7+sha512.abc", Some(true)), + ("11.28.7", Some(true)), + ("11.5.3", Some(true)), + ("11.5.2", Some(false)), + ("11.2.0", Some(false)), + ("11.0.0", Some(false)), + ("12.0.0-rc.1", Some(true)), + ("10.34.1", Some(false)), + ("10.20.0", Some(false)), + ("9.15.9", Some(false)), + ] { + assert_eq!( + matches_file_id_build_keys(Some(&pm(spec)), None), + want, + "{spec}" + ); + } + // Ranges admitting both sides, or no pin, decide nothing. + let engines = |range: &str| format!("{{\"engines\":{{\"pnpm\":\"{range}\"}}}}"); + assert_eq!( + matches_file_id_build_keys(Some(&engines("^10.30.0")), None), + None + ); + assert_eq!( + matches_file_id_build_keys(Some(&engines("^10.34.2")), None), + Some(true) + ); + assert_eq!( + matches_file_id_build_keys(Some(&engines("~10.34.1")), None), + None + ); + // 11.0–11.5.2 predate the change despite sorting after 10.34.2. + assert_eq!( + matches_file_id_build_keys(Some(&engines("^11.0.0")), None), + None + ); + assert_eq!( + matches_file_id_build_keys(Some(&engines("^11.5.3")), None), + Some(true) + ); + assert_eq!( + matches_file_id_build_keys(Some(&engines("~11.4.0")), None), + Some(false) + ); + assert_eq!( + matches_file_id_build_keys(Some(&engines(">=10")), None), + None + ); + assert_eq!(matches_file_id_build_keys(Some("{}"), None), None); + // The pnpm that last installed counts; a disagreeing pin undoes it. + let modules = r#"{"packageManager":"pnpm@11.28.7"}"#; + assert_eq!(matches_file_id_build_keys(None, Some(modules)), Some(true)); + assert_eq!( + matches_file_id_build_keys(Some(&pm("10.20.0")), Some(modules)), + None + ); + } + #[test] fn version_span_reads_exact_caret_tilde_and_wildcards() { for (spec, span) in [ diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index ba29bb238..5453323ee 100644 --- a/crates/socket-patch-core/src/vendor/mod.rs +++ b/crates/socket-patch-core/src/vendor/mod.rs @@ -81,6 +81,7 @@ mod npm_pack; pub(crate) mod nuget_config; pub mod nuget_feed; pub(crate) mod parse_memo; +mod pnpm_build_approvals; pub mod pnpm_lock; pub mod pnpm_lock_legacy; pub mod prestage; diff --git a/crates/socket-patch-core/src/vendor/pnpm_build_approvals.rs b/crates/socket-patch-core/src/vendor/pnpm_build_approvals.rs new file mode 100644 index 000000000..cc12b1987 --- /dev/null +++ b/crates/socket-patch-core/src/vendor/pnpm_build_approvals.rs @@ -0,0 +1,848 @@ +//! pnpm build approvals for a vendored `file:` tarball (#1597). +//! +//! Vendoring wires a pnpm project to `name@file:.socket/vendor/npm//…`, +//! and pnpm installs the package under that id. pnpm 10.34.2+ / 11.5.3+ match a +//! build decision (`allowBuilds`, `onlyBuiltDependencies`) against the id, +//! so a key spelling the registry package (`name`, `name@1.2.3`, +//! `name@1.2.3 || 1.2.4`) stops matching the vendored copy: pnpm 10 skips +//! the approved install script and pnpm 11/12 (`strictDepBuilds` on by +//! default) fail every install with `ERR_PNPM_IGNORED_BUILDS`. Only a key +//! spelling the whole `name@file:` matches. +//! +//! So when the project already decides the package's build, the same +//! decision is added under the vendored key, next to the user's own entry, +//! on the surface that holds it: `pnpm-workspace.yaml` (`allowBuilds:` +//! block mapping, `onlyBuiltDependencies:` block sequence) or +//! `package.json#pnpm` (the same two fields, read by pnpm <= 10). A key a +//! superseded patch left for the same `name@version` is replaced, and its +//! decision carried when it is the only one; an added key follows the +//! user's decision when it changes. Each added key is a ledger record that +//! `vendor --revert` / `rollback` / `remove` take out again. The user's own +//! entries are never edited. +//! +//! The key is written only when the project's pnpm pins say it runs +//! 10.34.2+ (11.5.3+ on pnpm 11): older releases still match the name, and +//! refuse a `file:` key outright (ERR_PNPM_INVALID_VERSION_UNION). Without a usable +//! pin, or for a decision the line surgery cannot extend (a flow-style +//! value, a non-boolean one), nothing is written and the +//! `vendor_pnpm_build_approval_lost` warning names the key to add. + +use serde_json::Value; + +use super::path::{parse_vendor_path, VendorPathParts}; +use super::state::{WiringAction, WiringRecord}; +use super::VendorWarning; +use crate::formats::pnpm::lines::{ + indent_of, parse_key_line, split_lines, unquote_value, yaml_key, +}; +use crate::formats::pnpm::workspace::{block_section_bounds, strip_comment, top_level_key}; +use crate::utils::pnpm_workspace::PNPM_WORKSPACE; + +const PACKAGE_JSON: &str = "package.json"; + +/// The two pnpm settings that hold per-package build decisions. +const ALLOW_BUILDS: &str = "allowBuilds"; +const ONLY_BUILT: &str = "onlyBuiltDependencies"; + +/// Wiring kinds: one vendored-key approval added to a surface. +pub(super) const KIND_WS_ALLOW_BUILD: &str = "pnpm_ws_allow_build"; +pub(super) const KIND_WS_ONLY_BUILT: &str = "pnpm_ws_only_built"; +pub(super) const KIND_PKG_ALLOW_BUILD: &str = "pnpm_pkg_allow_build"; +pub(super) const KIND_PKG_ONLY_BUILT: &str = "pnpm_pkg_only_built"; + +/// Warning code: the project decides the package's build in a way this +/// rewriter does not carry to the vendored key, so pnpm 10.34.2+ / 11.5.3+ stop +/// applying that decision. +pub(crate) const VENDOR_PNPM_BUILD_APPROVAL_LOST: &str = "vendor_pnpm_build_approval_lost"; + +/// Whether `kind` is one of this module's `pnpm-workspace.yaml` records. +pub(super) fn is_ws_kind(kind: &str) -> bool { + matches!(kind, KIND_WS_ALLOW_BUILD | KIND_WS_ONLY_BUILT) +} + +/// Whether `kind` is one of this module's `package.json` records. +pub(super) fn is_pkg_kind(kind: &str) -> bool { + matches!(kind, KIND_PKG_ALLOW_BUILD | KIND_PKG_ONLY_BUILT) +} + +/// The package being vendored and the key pnpm matches its copy by. +pub(super) struct Target<'a> { + name: &'a str, + version: &'a str, + /// `name@file:`, the id pnpm installs the vendored copy as. + key: String, + /// The vendored tarball's path, parsed. + ours: Option, +} + +impl<'a> Target<'a> { + /// `rel_tgz` is the vendored tarball's root-relative path. + pub(super) fn new(name: &'a str, version: &'a str, rel_tgz: &str) -> Self { + Target { + name, + version, + key: format!("{name}@file:{rel_tgz}"), + ours: parse_vendor_path(rel_tgz), + } + } + + /// How strongly `key` decides this package's build: a version-pinned + /// key beats a name-only one, and both beat a vendored key a superseded + /// patch left for the same `name@version`. `None` for any other key, + /// including the vendored key itself. + fn rank(&self, key: &str) -> Option { + if key == self.name { + return Some(Rank::Name); + } + let rest = key.strip_prefix(self.name)?.strip_prefix('@')?; + if let Some(path) = rest.strip_prefix("file:") { + let ours = self.ours.as_ref()?; + return parse_vendor_path(path) + .filter(|p| p.eco == "npm" && p.leaf == ours.leaf && p.uuid != ours.uuid) + .map(|_| Rank::Stale); + } + // pnpm's version specs: exact versions joined by `||`. + rest.split("||") + .any(|v| v.trim() == self.version) + .then_some(Rank::Pinned) + } + + /// Where the decision for this package sits among `keys`, `None` when + /// none decides it. + fn select<'k>(&self, keys: impl Iterator) -> Option { + let mut sel = Selection::default(); + let mut best = None; + for (i, key) in keys.enumerate() { + if key == self.key { + sel.ours = Some(i); + continue; + } + let Some(rank) = self.rank(key) else { + continue; + }; + if best.is_none_or(|b| rank > b) { + sel.source = i; + best = Some(rank); + } + if rank == Rank::Stale { + sel.stale.push(i); + } + sel.anchor = i; + } + best.map(|_| sel) + } +} + +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +enum Rank { + Stale, + Name, + Pinned, +} + +/// [`Target::select`]'s answer, as indices into the keys it was given. +#[derive(Default)] +struct Selection { + /// The entry whose decision is carried. + source: usize, + /// The vendored key's own entry. + ours: Option, + /// Keys a superseded patch left, ascending. + stale: Vec, + /// The last deciding entry: the vendored key goes right after it. + anchor: usize, +} + +/// What [`carry`] changed. +#[derive(Default)] +pub(super) struct Carried { + /// `package.json` (the `pkg` value passed in) was edited. + pub pkg_changed: bool, + /// The new `pnpm-workspace.yaml` text, when it was edited. + pub ws_text: Option, +} + +/// Add the vendored key beside every build decision the project holds for +/// `target` (see the module doc), pushing one record per added key. +/// `ws_text` is the workspace file as the override wiring left it. +/// `file_keys` is whether the project's pnpm matches build decisions +/// against a `file:` id ([`crate::formats::pnpm::matches_file_id_build_keys`]). +pub(super) fn carry( + target: &Target<'_>, + ws_text: Option<&str>, + pkg: &mut Value, + file_keys: Option, + wiring: &mut Vec, + warnings: &mut Vec, +) -> Carried { + if file_keys == Some(false) { + // Older pnpm matches the name, which the vendored copy keeps. + return Carried::default(); + } + let mut trial = pkg.clone(); + let mut added = Vec::new(); + let mut lost = Vec::new(); + let carried = carry_all(target, ws_text, &mut trial, &mut added, &mut lost); + for surface in &lost { + warnings.push(lost_warning(target, surface, LostWhy::Shape)); + } + if file_keys == Some(true) { + *pkg = trial; + wiring.extend(added); + return carried; + } + if let Some(rec) = added.first() { + warnings.push(lost_warning(target, &rec.file, LostWhy::UnknownPnpm)); + } + Carried::default() +} + +fn carry_all( + target: &Target<'_>, + ws_text: Option<&str>, + pkg: &mut Value, + wiring: &mut Vec, + lost: &mut Vec, +) -> Carried { + let mut carried = Carried::default(); + if let Some(text) = ws_text { + let mut lines = split_lines(text); + let mut changed = false; + for (field, kind) in [ + (ALLOW_BUILDS, KIND_WS_ALLOW_BUILD), + (ONLY_BUILT, KIND_WS_ONLY_BUILT), + ] { + match carry_ws(&mut lines, field, target) { + WsCarry::Unchanged => {} + WsCarry::Edited(added) => { + changed = true; + if let Some(value) = added { + wiring.push(record(PNPM_WORKSPACE, kind, target, value)); + } + } + WsCarry::Unsupported => lost.push(format!("{PNPM_WORKSPACE} `{field}`")), + } + } + if changed { + carried.ws_text = Some(lines.join("\n")); + } + } + if let Some(table) = pkg.get_mut("pnpm").and_then(Value::as_object_mut) { + if let Some(map) = table.get_mut(ALLOW_BUILDS).and_then(Value::as_object_mut) { + match carry_pkg_map(map, target) { + Some((changed, added)) => { + carried.pkg_changed |= changed; + if let Some(value) = added { + wiring.push(record(PACKAGE_JSON, KIND_PKG_ALLOW_BUILD, target, value)); + } + } + None => lost.push(format!("{PACKAGE_JSON} `pnpm.{ALLOW_BUILDS}`")), + } + } + if let Some(list) = table.get_mut(ONLY_BUILT).and_then(Value::as_array_mut) { + let (changed, added) = carry_pkg_list(list, target); + carried.pkg_changed |= changed; + if let Some(value) = added { + wiring.push(record(PACKAGE_JSON, KIND_PKG_ONLY_BUILT, target, value)); + } + } + } + carried +} + +fn record(file: &str, kind: &str, target: &Target<'_>, value: Value) -> WiringRecord { + WiringRecord { + file: file.to_string(), + kind: kind.to_string(), + action: WiringAction::Added, + key: Some(target.key.clone()), + original: None, + new: Some(value), + } +} + +/// Why a decision was not carried. +enum LostWhy { + /// The value is not one the line surgery can extend. + Shape, + /// No pnpm pin says whether the key would be accepted. + UnknownPnpm, +} + +fn lost_warning(target: &Target<'_>, surface: &str, why: LostWhy) -> VendorWarning { + let (name, version, key) = (target.name, target.version, &target.key); + let reason = match why { + LostWhy::Shape => format!( + "its value is not a block mapping/sequence of true/false this rewriter can \ + extend; add `{key}` there with the same decision" + ), + LostWhy::UnknownPnpm => format!( + "no pnpm pin (package.json `packageManager`, `devEngines`, `engines.pnpm`, or \ + node_modules/.modules.yaml) says which pnpm installs, and pnpm 10.20–10.34.1 and \ + 11.0–11.5.2 refuse that key (ERR_PNPM_INVALID_VERSION_UNION); on pnpm 10.34.2+ / \ + 11.5.3+ add \ + `{key}` there with the same decision, or pin pnpm and re-run to have it added" + ), + }; + VendorWarning::new( + VENDOR_PNPM_BUILD_APPROVAL_LOST, + format!( + "{surface} decides {name}@{version}'s install script by a key pnpm 10.34.2+ / 11.5.3+ \ + no longer match once the package is vendored, so pnpm 10 skips the script \ + and pnpm 11+ fail every install with ERR_PNPM_IGNORED_BUILDS; {reason}" + ), + ) +} + +/// The outcome of one `pnpm-workspace.yaml` field. +enum WsCarry { + Unchanged, + /// Lines were edited; `Some` carries the decision of a key added. + Edited(Option), + /// The field decides the package in a form not carried. + Unsupported, +} + +/// One entry of a block mapping or sequence: its line, key and value. +struct WsEntry { + line: usize, + key: String, + value: String, +} + +/// The entries of the top-level block `field`, with the indent its entries +/// use. A sequence's items may sit at column 0 (YAML allows that under a +/// mapping key), which ends [`block_section_bounds`]'s section, so items +/// are read here line by line. +fn ws_entries(lines: &[String], field: &str) -> Option<(usize, Vec)> { + let (start, end) = block_section_bounds(lines, field)?; + let body = |l: &str| { + let t = l.trim_start_matches(' '); + !(t.is_empty() || t == "\r" || t.starts_with('#')) + }; + let first = lines[start + 1..].iter().position(|l| body(l))? + start + 1; + let indent = indent_of(&lines[first]); + let mut entries = Vec::new(); + if field == ONLY_BUILT { + for (i, line) in lines.iter().enumerate().skip(first) { + let line = line.strip_suffix('\r').unwrap_or(line); + if !body(line) { + continue; + } + let Some(item) = line + .strip_prefix(&" ".repeat(indent)) + .and_then(|s| s.strip_prefix('-')) + .filter(|s| s.is_empty() || s.starts_with(' ')) + else { + break; + }; + entries.push(WsEntry { + line: i, + key: unquote_value(strip_comment(item).trim()).to_string(), + value: String::new(), + }); + } + } else { + for (i, line) in lines.iter().enumerate().take(end).skip(start + 1) { + let line = line.strip_suffix('\r').unwrap_or(line); + if let Some((key, _repr, rest)) = parse_key_line(line, indent) { + entries.push(WsEntry { + line: i, + key: key.to_string(), + value: strip_comment(rest).trim().to_string(), + }); + } + } + } + Some((indent, entries)) +} + +/// Whether a top-level `field` with a flow (inline) value decides the +/// package: the value runs from the key's line until its brackets close. +fn flow_value_decides(lines: &[String], field: &str, target: &Target<'_>) -> bool { + let Some(start) = lines.iter().position(|l| { + top_level_key(l).is_some_and(|(key, value)| key == field && !value.is_empty()) + }) else { + return false; + }; + let mut text = String::new(); + for (n, line) in lines[start..].iter().enumerate() { + if n == 0 { + text.push_str(top_level_key(line).map_or("", |(_, value)| value)); + } else { + text.push(' '); + text.push_str(strip_comment(line)); + } + let depth: i32 = text + .chars() + .map(|c| match c { + '[' | '{' => 1, + ']' | '}' => -1, + _ => 0, + }) + .sum(); + if depth <= 0 { + break; + } + } + text.split([',', '[', ']', '{', '}']) + .map(|item| { + let item = item.trim(); + // A mapping item's key ends at the first `: `. + let key = item.find(": ").map_or(item, |at| &item[..at]); + unquote_value(key.trim()) + }) + .any(|key| target.rank(key).is_some()) +} + +fn carry_ws(lines: &mut Vec, field: &str, target: &Target<'_>) -> WsCarry { + let Some((indent, entries)) = ws_entries(lines, field) else { + return if flow_value_decides(lines, field, target) { + WsCarry::Unsupported + } else { + WsCarry::Unchanged + }; + }; + let Some(sel) = target.select(entries.iter().map(|e| e.key.as_str())) else { + return WsCarry::Unchanged; + }; + let source = &entries[sel.source]; + let map = field == ALLOW_BUILDS; + if map && !matches!(source.value.as_str(), "true" | "false") { + return WsCarry::Unsupported; + } + let pad = " ".repeat(indent); + let eol = if lines[source.line].ends_with('\r') { + "\r" + } else { + "" + }; + let new_line = if map { + format!("{pad}{}: {}{eol}", yaml_key(&target.key), source.value) + } else { + format!("{pad}- '{}'{eol}", target.key) + }; + let mut edited = !sel.stale.is_empty(); + let mut added = None; + match sel.ours.map(|i| &entries[i]) { + // The added key follows the user's decision when it changes. + Some(ours) if map && ours.value != source.value => { + lines[ours.line] = new_line; + edited = true; + } + Some(_) => {} + None => { + lines.insert(entries[sel.anchor].line + 1, new_line); + added = Some(Value::String(if map { + source.value.clone() + } else { + target.key.clone() + })); + edited = true; + } + } + // Stale lines all sit at or before the anchor, so the insert leaves + // their indices alone; drop them last-first. + for &i in sel.stale.iter().rev() { + lines.remove(entries[i].line); + } + if edited { + WsCarry::Edited(added) + } else { + WsCarry::Unchanged + } +} + +/// `package.json#pnpm.allowBuilds`: `(changed, the decision of a key +/// added)`, `None` when the deciding value is not a boolean. +fn carry_pkg_map( + map: &mut serde_json::Map, + target: &Target<'_>, +) -> Option<(bool, Option)> { + let keys: Vec = map.keys().cloned().collect(); + let Some(sel) = target.select(keys.iter().map(String::as_str)) else { + return Some((false, None)); + }; + let value = map[&keys[sel.source]].clone(); + if !value.is_boolean() { + return None; + } + let changed = sel.ours.is_none_or(|i| map[&keys[i]] != value) || !sel.stale.is_empty(); + // Rebuild in order: the vendored key goes right after the last + // deciding entry (or stays where it is), stale keys go. + let old = std::mem::take(map); + for (i, (key, v)) in old.into_iter().enumerate() { + if Some(i) == sel.ours { + map.insert(key, value.clone()); + } else if !sel.stale.contains(&i) { + map.insert(key, v); + } + if i == sel.anchor && sel.ours.is_none() { + map.insert(target.key.clone(), value.clone()); + } + } + Some((changed, sel.ours.is_none().then_some(value))) +} + +/// `package.json#pnpm.onlyBuiltDependencies`. +fn carry_pkg_list(list: &mut Vec, target: &Target<'_>) -> (bool, Option) { + let keys: Vec<&str> = list.iter().map(|v| v.as_str().unwrap_or("")).collect(); + let Some(sel) = target.select(keys.into_iter()) else { + return (false, None); + }; + let ours = sel.ours.is_some(); + if !ours { + list.insert(sel.anchor + 1, Value::String(target.key.clone())); + } + // The insert sits after every stale index. + for &i in sel.stale.iter().rev() { + list.remove(i); + } + if ours { + (!sel.stale.is_empty(), None) + } else { + (true, Some(Value::String(target.key.clone()))) + } +} + +/// Take the keys `records` added out of `pnpm-workspace.yaml`'s lines. A +/// key already gone is converged. The key names this patch's own artifact, +/// which the revert removes, so a decision the user flipped since goes too. +pub(super) fn revert_ws(lines: &mut Vec, records: &[&WiringRecord]) -> bool { + let mut dirty = false; + for rec in records { + let Some(key) = rec.key.as_deref() else { + continue; + }; + let field = if rec.kind == KIND_WS_ALLOW_BUILD { + ALLOW_BUILDS + } else { + ONLY_BUILT + }; + let Some((_, entries)) = ws_entries(lines, field) else { + continue; + }; + if let Some(entry) = entries.iter().rev().find(|e| e.key == key) { + lines.remove(entry.line); + dirty = true; + } + } + dirty +} + +/// Take the key `rec` added out of `package.json#pnpm`; `true` when the +/// document changed. +pub(super) fn revert_pkg(doc: &mut Value, rec: &WiringRecord) -> bool { + let Some(key) = rec.key.as_deref() else { + return false; + }; + let Some(table) = doc.get_mut("pnpm").and_then(Value::as_object_mut) else { + return false; + }; + if rec.kind == KIND_PKG_ALLOW_BUILD { + table + .get_mut(ALLOW_BUILDS) + .and_then(Value::as_object_mut) + .is_some_and(|map| map.shift_remove(key).is_some()) + } else { + table + .get_mut(ONLY_BUILT) + .and_then(Value::as_array_mut) + .is_some_and(|list| { + let before = list.len(); + list.retain(|v| v.as_str() != Some(key)); + list.len() != before + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const UUID: &str = "9f6b2c4e-1d3a-4f6b-8c2d-7e5a9b1c3d5f"; + const OLD: &str = "11111111-2222-4333-8444-555555555555"; + + fn vendored_rel(uuid: &str) -> String { + format!(".socket/vendor/npm/{uuid}/sp-build-1.0.0.tgz") + } + + fn target() -> Target<'static> { + Target::new("sp-build", "1.0.0", &vendored_rel(UUID)) + } + + fn key(uuid: &str) -> String { + format!("sp-build@file:{}", vendored_rel(uuid)) + } + + fn run_ws_with( + text: &str, + file_keys: Option, + ) -> (Option, Vec, Vec) { + let (mut wiring, mut warnings) = (Vec::new(), Vec::new()); + let mut pkg = serde_json::json!({}); + let out = carry( + &target(), + Some(text), + &mut pkg, + file_keys, + &mut wiring, + &mut warnings, + ); + assert!(!out.pkg_changed); + (out.ws_text, wiring, warnings) + } + + fn run_ws(text: &str) -> (Option, Vec, Vec) { + run_ws_with(text, Some(true)) + } + + #[test] + fn rank_reads_pnpm_version_specs() { + let t = target(); + assert!(t.rank("sp-build") == Some(Rank::Name)); + assert!(t.rank("sp-build@1.0.0") == Some(Rank::Pinned)); + assert!(t.rank("sp-build@0.9.0 || 1.0.0") == Some(Rank::Pinned)); + assert!(t.rank("sp-build@2.0.0").is_none()); + assert!(t.rank("sp-build-native").is_none()); + assert!(t.rank(&key(OLD)) == Some(Rank::Stale)); + assert!(t.rank(&key(UUID)).is_none()); + assert!(t + .rank("sp-build@file:.socket/vendor/npm/x/other-1.0.0.tgz") + .is_none()); + } + + #[test] + fn allow_builds_true_and_false_are_carried_after_the_users_entry() { + for value in ["true", "false"] { + let text = format!("packages:\n - '.'\nallowBuilds:\n esbuild: true\n sp-build: {value} # native\n zz: false\n"); + let (ws, wiring, warnings) = run_ws(&text); + let key = key(UUID); + assert_eq!( + ws.unwrap(), + text.replace( + &format!(" sp-build: {value} # native\n"), + &format!(" sp-build: {value} # native\n {key}: {value}\n") + ) + ); + assert!(warnings.is_empty()); + assert_eq!(wiring.len(), 1); + assert_eq!(wiring[0].kind, KIND_WS_ALLOW_BUILD); + assert_eq!(wiring[0].key.as_deref(), Some(key.as_str())); + } + } + + #[test] + fn pinned_decision_beats_name_only() { + let text = "allowBuilds:\n sp-build@1.0.0: false\n sp-build: true\n"; + let (ws, _, _) = run_ws(text); + assert_eq!(ws.unwrap(), format!("{text} {}: false\n", key(UUID))); + } + + #[test] + fn only_built_list_gets_a_quoted_item_even_at_column_zero() { + let key = key(UUID); + for text in [ + "onlyBuiltDependencies:\n - esbuild\n - sp-build\npackages:\n - '.'\n", + "onlyBuiltDependencies:\n- 'sp-build'\npackages:\n- '.'\n", + ] { + let (ws, wiring, _) = run_ws(text); + let pad = if text.contains("\n - esbuild") { + " " + } else { + "" + }; + let item = if pad.is_empty() { + "- 'sp-build'\n" + } else { + " - sp-build\n" + }; + assert_eq!( + ws.unwrap(), + text.replacen(item, &format!("{item}{pad}- '{key}'\n"), 1) + ); + assert_eq!(wiring[0].kind, KIND_WS_ONLY_BUILT); + } + } + + #[test] + fn no_decision_for_the_package_leaves_the_file_alone() { + let text = "allowBuilds:\n esbuild: true\nonlyBuiltDependencies:\n - sharp\n"; + let (ws, wiring, warnings) = run_ws(text); + assert!(ws.is_none() && wiring.is_empty() && warnings.is_empty()); + } + + #[test] + fn rerun_is_in_sync_and_a_superseded_key_is_replaced() { + let key = key(UUID); + let synced = format!("allowBuilds:\n sp-build: true\n {key}: true\n"); + let (ws, wiring, _) = run_ws(&synced); + assert!(ws.is_none() && wiring.is_empty()); + + // Only the superseded patch's key decides: its decision carries. + let old = self::key(OLD); + let (ws, wiring, _) = run_ws(&format!("allowBuilds:\n {old}: true\n")); + assert_eq!(ws.unwrap(), format!("allowBuilds:\n {key}: true\n")); + assert_eq!(wiring.len(), 1); + let (ws, _, _) = run_ws(&format!( + "onlyBuiltDependencies:\n - sp-build\n - '{old}'\n" + )); + assert_eq!( + ws.unwrap(), + format!("onlyBuiltDependencies:\n - sp-build\n - '{key}'\n") + ); + } + + /// The user flips their decision after vendoring: the re-run brings + /// the added key along, without a new record (the first one still + /// takes the key out on revert). + #[test] + fn added_key_follows_a_flipped_decision() { + let key = key(UUID); + let (ws, wiring, _) = run_ws(&format!("allowBuilds:\n sp-build: false\n {key}: true\n")); + assert_eq!( + ws.unwrap(), + format!("allowBuilds:\n sp-build: false\n {key}: false\n") + ); + assert!(wiring.is_empty()); + + let mut pkg = serde_json::json!({"pnpm": {"allowBuilds": {"sp-build": false, &key: true}}}); + let (mut wiring, mut warnings) = (Vec::new(), Vec::new()); + let out = carry( + &target(), + None, + &mut pkg, + Some(true), + &mut wiring, + &mut warnings, + ); + assert!(out.pkg_changed && wiring.is_empty()); + assert_eq!(pkg["pnpm"]["allowBuilds"][&key], Value::Bool(false)); + } + + #[test] + fn flow_or_non_boolean_values_warn_and_write_nothing() { + for text in [ + "onlyBuiltDependencies: [sp-build]\n", + "onlyBuiltDependencies: [\n esbuild,\n 'sp-build', # native\n]\npackages: ['.']\n", + "allowBuilds: {sp-build: true}\n", + "allowBuilds:\n sp-build: &yes true\n", + "allowBuilds:\n sp-build:\n", + ] { + let (ws, wiring, warnings) = run_ws(text); + assert!(ws.is_none() && wiring.is_empty(), "{text}"); + assert_eq!(warnings.len(), 1, "{text}"); + assert_eq!(warnings[0].code, VENDOR_PNPM_BUILD_APPROVAL_LOST); + assert!( + warnings[0].detail.contains(&key(UUID)), + "{}", + warnings[0].detail + ); + } + // Other packages whose names contain the target's decide nothing. + for text in [ + "onlyBuiltDependencies: [sp-build-native, '@x/sp-build']\n", + "allowBuilds: {sp-builder: true}\n", + ] { + let (_, _, warnings) = run_ws(text); + assert!(warnings.is_empty(), "{text}: {warnings:?}"); + } + } + + /// pnpm 10.20–10.34.1 refuse a `file:` key, older pnpm still matches + /// the name: only a pin admitting nothing but 10.34.2+ / 11.5.3+ gets the key. + #[test] + fn the_key_is_written_only_for_a_pnpm_that_reads_it() { + let text = "allowBuilds:\n sp-build: true\n"; + let (ws, wiring, warnings) = run_ws_with(text, Some(false)); + assert!(ws.is_none() && wiring.is_empty() && warnings.is_empty()); + + let (ws, wiring, warnings) = run_ws_with(text, None); + assert!(ws.is_none() && wiring.is_empty()); + assert_eq!(warnings.len(), 1); + assert_eq!(warnings[0].code, VENDOR_PNPM_BUILD_APPROVAL_LOST); + assert!( + warnings[0] + .detail + .contains("ERR_PNPM_INVALID_VERSION_UNION"), + "{}", + warnings[0].detail + ); + + // Nothing to carry: silent whatever the pnpm. + let (_, _, warnings) = run_ws_with("allowBuilds:\n esbuild: true\n", None); + assert!(warnings.is_empty()); + } + + #[test] + fn package_json_fields_are_carried_and_reverted() { + let t = target(); + let key = key(UUID); + let before = serde_json::json!({ + "pnpm": { + "onlyBuiltDependencies": ["sp-build", "esbuild"], + "allowBuilds": {"sp-build@1.0.0": false, "esbuild": true} + } + }); + let mut pkg = before.clone(); + let (mut wiring, mut warnings) = (Vec::new(), Vec::new()); + let out = carry(&t, None, &mut pkg, Some(true), &mut wiring, &mut warnings); + assert!(out.pkg_changed && out.ws_text.is_none()); + assert_eq!( + pkg["pnpm"]["onlyBuiltDependencies"], + serde_json::json!(["sp-build", key, "esbuild"]) + ); + let order: Vec<&String> = pkg["pnpm"]["allowBuilds"] + .as_object() + .unwrap() + .keys() + .collect(); + assert_eq!( + order, + ["sp-build@1.0.0", &key, "esbuild"], + "next to the decision" + ); + assert_eq!(pkg["pnpm"]["allowBuilds"][&key], Value::Bool(false)); + assert_eq!(wiring.len(), 2); + + // In sync on a re-run. + let mut again = Vec::new(); + assert!(!carry(&t, None, &mut pkg, Some(true), &mut again, &mut warnings).pkg_changed); + assert!(again.is_empty()); + + for rec in &wiring { + assert!(is_pkg_kind(&rec.kind)); + assert!(revert_pkg(&mut pkg, rec)); + } + assert_eq!(pkg, before); + } + + #[test] + fn revert_ws_removes_only_the_added_keys() { + let text = "packages:\n - '.'\nallowBuilds:\n sp-build: true\nonlyBuiltDependencies:\n - sp-build\n"; + let (ws, wiring, _) = run_ws(text); + assert_eq!(wiring.len(), 2); + let mut lines = split_lines(&ws.unwrap()); + let refs: Vec<&WiringRecord> = wiring.iter().collect(); + assert!(refs.iter().all(|r| is_ws_kind(&r.kind))); + assert!(revert_ws(&mut lines, &refs)); + assert_eq!(lines.join("\n"), text); + // Converged on a second pass. + assert!(!revert_ws(&mut lines, &refs)); + } + + #[test] + fn scoped_package_keys_are_quoted() { + let rel = ".socket/vendor/npm/9f6b2c4e-1d3a-4f6b-8c2d-7e5a9b1c3d5f/@sp/pad-1.0.0.tgz"; + let t = Target::new("@sp/pad", "1.0.0", rel); + let mut lines = split_lines("allowBuilds:\n '@sp/pad': false\n"); + assert!(matches!( + carry_ws(&mut lines, ALLOW_BUILDS, &t), + WsCarry::Edited(Some(_)) + )); + assert_eq!( + lines.join("\n"), + format!("allowBuilds:\n '@sp/pad': false\n '@sp/pad@file:{rel}': false\n") + ); + } +} diff --git a/crates/socket-patch-core/src/vendor/pnpm_lock.rs b/crates/socket-patch-core/src/vendor/pnpm_lock.rs index a1cf14876..997ddfce1 100644 --- a/crates/socket-patch-core/src/vendor/pnpm_lock.rs +++ b/crates/socket-patch-core/src/vendor/pnpm_lock.rs @@ -68,6 +68,7 @@ use super::npm_common::{ }; use super::parse_memo::ParseMemo; use super::path::parse_vendor_path; +use super::pnpm_build_approvals as build_approvals; use super::revert::{self, KeepPolicy}; use super::source::PackageSource; use super::state::{PnpmMeta, VendorEntry, WiringAction, WiringRecord}; @@ -304,7 +305,7 @@ impl NpmLockBackend for PnpmBackend { // keys, which pnpm reads the same, #1096) pinned to pnpm 9.0–10.4: those read package.json `pnpm.overrides` (wired above) // and refuse `pnpm add` in the root-only workspace a created file // would make (#734). A later run on pnpm >= 10.5 adds the mirror. - let ws_edit = if dialect == PnpmDialect::V9 + let mut ws_edit = if dialect == PnpmDialect::V9 && !(ws_text.as_deref().is_none_or(workspace::is_keyless) && (pinned_pre_10_5(project_root, &pkg_bytes).await // A keyless file the lock was installed from as a @@ -319,6 +320,23 @@ impl NpmLockBackend for PnpmBackend { WorkspaceEdit::default() }; + // pnpm 10.34.2+ / 11.5.3+ match build decisions against the vendored + // `name@file:` id, so carry the project's decision over to it + // (#1597). Legacy locks are installed by pnpm <= 8, which matches + // by name. + let mut pkg_changed = pkg_changed; + if dialect == PnpmDialect::V9 { + let target = build_approvals::Target::new(&cx.coords.name, &cx.coords.version, rel_tgz); + let base = ws_edit.new_text.as_deref().or(ws_text.as_deref()); + let file_keys = matches_file_id_build_keys(project_root, &pkg_bytes).await; + let carried = + build_approvals::carry(&target, base, &mut pkg, file_keys, &mut wiring, warnings); + pkg_changed |= carried.pkg_changed; + if carried.ws_text.is_some() { + ws_edit.new_text = carried.ws_text; + } + } + if !pkg_changed && !lock_changed && ws_edit.new_text.is_none() { // Everything already carries this uuid + the packed integrity: // the project is in sync. @@ -1095,24 +1113,24 @@ pub(super) async fn revert_pnpm_dialect( // pnpm-workspace.yaml override surface (pnpm >= 11): delete a file we // created, or splice our override back out of one we edited. - if let Some(rec) = entry.wiring.iter().find(|r| { + let ws_rec = entry.wiring.iter().find(|r| { dialect == PnpmDialect::V9 && r.file == PNPM_WORKSPACE && r.kind == KIND_WS_OVERRIDE - }) { - let (created_file, created_overrides, created_packages) = match &entry.pnpm { - Some(meta) => ( - meta.created_workspace_file, - meta.created_workspace_overrides, - meta.created_workspace_packages, - ), - None => (false, false, false), - }; + }); + let ws_approvals: Vec<&WiringRecord> = entry + .wiring + .iter() + .filter(|r| { + dialect == PnpmDialect::V9 + && r.file == PNPM_WORKSPACE + && build_approvals::is_ws_kind(&r.kind) + }) + .collect(); + if ws_rec.is_some() || !ws_approvals.is_empty() { if let Err(e) = revert_workspace( project_root, - rec, - created_file, - created_overrides, - created_packages, - &entry.uuid, + ws_rec, + &ws_approvals, + entry, &mut outcome.warnings, ) .await @@ -1137,16 +1155,24 @@ pub(super) async fn revert_pnpm_dialect( /// still holds only the vendoring scaffold), or splice our override key back /// out of a file we edited (restoring the taken-over value, and dropping an /// `overrides:` section we created once it empties). Drift ⇒ warning, left -/// alone. `Err` is a genuine write failure. +/// alone. Build approvals vendoring added (`approvals`) are taken out too. +/// `Err` is a genuine write failure. async fn revert_workspace( project_root: &Path, - rec: &WiringRecord, - created_file: bool, - created_overrides: bool, - created_packages: bool, - entry_uuid: &str, + rec: Option<&WiringRecord>, + approvals: &[&WiringRecord], + entry: &VendorEntry, warnings: &mut Vec, ) -> Result<(), String> { + let (created_file, created_overrides, created_packages) = match &entry.pnpm { + Some(meta) => ( + meta.created_workspace_file, + meta.created_workspace_overrides, + meta.created_workspace_packages, + ), + None => (false, false, false), + }; + let entry_uuid = entry.uuid.as_str(); let path = project_root.join(PNPM_WORKSPACE); let text = match read_regular_to_string(&path).await { Ok(t) => t, @@ -1154,8 +1180,9 @@ async fn revert_workspace( // ALREADY CONVERGED: for an Added override (no recorded // original) the reverted state is "no override" — a missing // file trivially satisfies it (an earlier partial revert may - // have deleted the scaffold we created). Not drift. - if rec.original.is_none() { + // have deleted the scaffold we created). Not drift. Build + // approvals we added are gone with the file. + if rec.is_none_or(|r| r.original.is_none()) { return Ok(()); } warnings.push(drifted(format!( @@ -1180,7 +1207,7 @@ async fn revert_workspace( // Fast path: a file we created that is still byte-identical to the // scaffold we wrote → delete it (byte-restore to "no file"). - if created_file { + if let Some(rec) = rec.filter(|_| created_file) { let scaffold = match (rec.key.as_deref(), rec.new.as_ref().and_then(Value::as_str)) { (Some(key), Some(spec)) => Some(ws_scaffold_text(key, spec)), _ => None, @@ -1195,7 +1222,9 @@ async fn revert_workspace( let mut lines = split_lines(&text); let mut dirty = false; - revert_ws_record(&mut lines, rec, entry_uuid, &mut dirty, warnings); + if let Some(rec) = rec { + revert_ws_record(&mut lines, rec, entry_uuid, &mut dirty, warnings); + } if dirty && created_overrides { remove_empty_ws_overrides_section(&mut lines); // The `packages` scaffold added to a file with no keys (#1096) @@ -1208,6 +1237,7 @@ async fn revert_workspace( } } } + dirty |= build_approvals::revert_ws(&mut lines, approvals); if dirty { atomic_write_bytes_preserving_mode(&path, lines.join("\n").as_bytes()) .await @@ -1971,6 +2001,19 @@ async fn pinned_pre_10_5(project_root: &Path, pkg_bytes: &[u8]) -> bool { .is_some() } +/// Whether the project's pnpm pins (package.json, the installed +/// `node_modules/.modules.yaml`) say its pnpm matches build decisions +/// against a vendored `file:` id (#1597); `None` when they don't tell. +async fn matches_file_id_build_keys(project_root: &Path, pkg_bytes: &[u8]) -> Option { + let modules = read_regular_to_string(&project_root.join("node_modules/.modules.yaml")) + .await + .ok(); + crate::formats::pnpm::matches_file_id_build_keys( + std::str::from_utf8(pkg_bytes).ok(), + modules.as_deref(), + ) +} + /// The bytes of a freshly created pnpm-workspace.yaml: a root-only /// `packages:` list (pnpm 9 refuses a workspace file with no `packages` /// field) plus the single `overrides:` entry. Kept in one place so the @@ -3137,6 +3180,10 @@ pub(super) fn revert_pkg_record( dirty: &mut bool, warnings: &mut Vec, ) { + if build_approvals::is_pkg_kind(&rec.kind) { + *dirty |= build_approvals::revert_pkg(doc, rec); + return; + } if rec.kind != KIND_PKG_OVERRIDE { warnings.push(VendorWarning::new( "vendor_lock_entry_drifted", @@ -11011,4 +11058,209 @@ snapshots: assert!(!same_package_key(&a, &b), "{a} !~ {b}"); } } + + // ── build approvals carried to the vendored key (#1597) ─────────────── + + /// Record the pnpm that installed `node_modules`, as pnpm does. + async fn installed_by(fx: &Fixture, pm: &str) { + tokio::fs::write( + fx.root().join("node_modules/.modules.yaml"), + format!("{{\"packageManager\":\"{pm}\"}}"), + ) + .await + .unwrap(); + } + + /// pnpm 10.20–10.34.1 refuse a `file:` build key and older pnpm still + /// matches the name: a project pinned there gets no key and no warning. + /// Without any pin nothing is written either, and the warning says + /// which key a pnpm 10.34.2+ / 11.5.3+ needs. + #[tokio::test] + async fn build_approval_key_is_written_only_for_a_pnpm_that_reads_it() { + let ws = "packages:\n - '.'\nallowBuilds:\n left-pad: true\n"; + for pm in [Some("pnpm@10.30.0"), None] { + let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; + if let Some(pm) = pm { + installed_by(&fx, pm).await; + } + write_ws(&fx, ws).await; + let (result, entry, warnings) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + let entry = entry.unwrap(); + assert!( + entry + .wiring + .iter() + .all(|r| !build_approvals::is_ws_kind(&r.kind)), + "{:?}", + entry.wiring + ); + let live = fx.read(PNPM_WORKSPACE).await; + assert!(!live.contains("left-pad@file:"), "{live}"); + assert_eq!(no_approval_warning(&warnings), pm.is_some(), "{warnings:?}"); + } + } + + fn no_approval_warning(warnings: &[VendorWarning]) -> bool { + warnings + .iter() + .all(|w| w.code != build_approvals::VENDOR_PNPM_BUILD_APPROVAL_LOST) + } + + /// pnpm 10.34.2+ / 11.5.3+ match `allowBuilds` / `onlyBuiltDependencies` + /// against the vendored `left-pad@file:` id, so a name-only or + /// pinned decision stops applying (pnpm 11/12 fail every install with + /// ERR_PNPM_IGNORED_BUILDS). Vendoring adds the same decision under the + /// vendored key, and revert takes it out again byte-exactly. P1's + /// `consumer` depends on left-pad too: the key covers that copy. + #[tokio::test] + async fn ws_build_approval_is_carried_to_the_vendored_key_and_reverted() { + for decision in ["true", "false"] { + let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; + installed_by(&fx, "pnpm@11.28.7").await; + let ws = format!( + "packages:\n - '.'\nallowBuilds:\n left-pad: {decision}\n \ + esbuild: true\nonlyBuiltDependencies:\n - left-pad@1.3.0\n" + ); + write_ws(&fx, &ws).await; + let (result, entry, warnings) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + assert!(no_approval_warning(&warnings), "{warnings:?}"); + let entry = entry.unwrap(); + let key = format!("left-pad@file:{}", fx.rel_tgz()); + let live = fx.read(PNPM_WORKSPACE).await; + assert!( + live.contains(&format!( + " left-pad: {decision}\n {key}: {decision}\n esbuild: true\n" + )), + "{live}" + ); + assert!( + live.contains(&format!(" - left-pad@1.3.0\n - '{key}'\n")), + "{live}" + ); + let added: Vec<_> = entry + .wiring + .iter() + .filter(|r| build_approvals::is_ws_kind(&r.kind)) + .collect(); + assert_eq!(added.len(), 2, "{:?}", entry.wiring); + assert!(added.iter().all(|r| r.key.as_deref() == Some(key.as_str()))); + + // An in-sync re-run adds nothing. + let (_, again, _) = expect_done(fx.vendor(false).await); + assert!(again.is_none(), "re-run is in sync"); + assert_eq!(fx.read(PNPM_WORKSPACE).await, live); + + let outcome = revert_pnpm(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings); + assert_eq!(fx.read(PNPM_WORKSPACE).await, ws, "workspace byte-restored"); + assert_eq!(fx.read(PNPM_LOCK).await, P1_BEFORE_LOCK); + assert!(!uuid_dir(&fx).exists()); + } + } + + /// pnpm 10 also reads `package.json#pnpm.onlyBuiltDependencies`: the + /// approval is carried there (next to the override) and reverted. + #[tokio::test] + async fn package_json_only_built_approval_is_carried_and_reverted() { + let pkg = P1_BEFORE_PKG.replace( + " \"private\": true,\n", + " \"private\": true,\n \"pnpm\": {\n \"onlyBuiltDependencies\": [\n \"left-pad\"\n ]\n },\n", + ); + let fx = fixture_with(&pkg, P1_BEFORE_LOCK).await; + installed_by(&fx, "pnpm@11.28.7").await; + let (result, entry, warnings) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + assert!(no_approval_warning(&warnings), "{warnings:?}"); + let entry = entry.unwrap(); + let live: Value = serde_json::from_str(&fx.read(PACKAGE_JSON).await).unwrap(); + let key = format!("left-pad@file:{}", fx.rel_tgz()); + assert_eq!( + live["pnpm"]["onlyBuiltDependencies"], + serde_json::json!(["left-pad", key]) + ); + assert!(entry + .wiring + .iter() + .any(|r| build_approvals::is_pkg_kind(&r.kind))); + + let outcome = revert_pnpm(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings); + assert_eq!( + fx.read(PACKAGE_JSON).await, + pkg, + "package.json byte-restored" + ); + assert_eq!(fx.read(PNPM_LOCK).await, P1_BEFORE_LOCK); + } + + /// A superseding patch (new uuid) replaces the older patch's key with + /// its own, so no dead key piles up, and revert after the supersede + /// restores the user's file. + #[tokio::test] + async fn superseding_patch_replaces_the_older_vendored_approval_key() { + let mut fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; + installed_by(&fx, "pnpm@11.28.7").await; + let ws = "packages:\n - '.'\nallowBuilds:\n left-pad: true\n"; + write_ws(&fx, ws).await; + let (_, prev, _) = expect_done(fx.vendor(false).await); + let prev = prev.unwrap(); + let old_key = format!("left-pad@file:{}", fx.rel_tgz()); + + fx.record.uuid = "0a1b2c3d-4e5f-6a7b-8c9d-0e1f2a3b4c5d".to_string(); + let (_, next, _) = expect_done(fx.vendor(false).await); + let mut merged = next.unwrap(); + let new_key = format!( + "left-pad@file:.socket/vendor/npm/{}/left-pad-1.3.0.tgz", + merged.uuid + ); + let live = fx.read(PNPM_WORKSPACE).await; + assert!(!live.contains(&old_key), "{live}"); + assert!( + live.contains(&format!( + "allowBuilds:\n left-pad: true\n {new_key}: true\n" + )), + "{live}" + ); + + super::super::state::carry_forward_wiring(&prev, &mut merged); + let outcome = revert_pnpm(&merged, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings); + assert_eq!(fx.read(PNPM_WORKSPACE).await, ws); + } + + /// A flow-style decision can't be extended by the line surgery: the + /// vendoring still succeeds, leaves the setting alone and says which + /// key to add. + #[tokio::test] + async fn inline_build_approval_warns_with_the_key_to_add() { + let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; + write_ws( + &fx, + "packages:\n - '.'\nonlyBuiltDependencies: [left-pad]\n", + ) + .await; + let (result, entry, warnings) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + assert!(entry.is_some()); + let warning = warnings + .iter() + .find(|w| w.code == build_approvals::VENDOR_PNPM_BUILD_APPROVAL_LOST) + .unwrap_or_else(|| panic!("{warnings:?}")); + assert!( + warning + .detail + .contains(&format!("left-pad@file:{}", fx.rel_tgz())), + "{}", + warning.detail + ); + assert!(fx + .read(PNPM_WORKSPACE) + .await + .contains("onlyBuiltDependencies: [left-pad]\n")); + } } diff --git a/docs/ecosystems.md b/docs/ecosystems.md index deda930d5..0461c5a4d 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -209,6 +209,23 @@ Each Legacy format has an upgrade path and an undo path. Both work in v5: upgrading to pnpm >=11. When no pin says which pnpm runs, the file is created, and pnpm 9.0–10.4 then need `pnpm add -w `. Vendored mode follows the same rule for its `overrides:` mirror. + **Vendored build approvals (#1597):** pnpm 10.34.2+ and 11.5.3+ match + `allowBuilds` / `onlyBuiltDependencies` against the id a package installs + as, and a vendored package installs as `name@file:.socket/vendor/npm/…`. + A decision keyed by the name (`sp-build`, `sp-build@1.0.0`) would stop + applying: pnpm 10 skips the approved script, pnpm 11 and 12 fail every + install with `ERR_PNPM_IGNORED_BUILDS`. Vendoring adds the same decision + under the vendored key next to the user's entry (`pnpm-workspace.yaml`, or + `package.json#pnpm` for pnpm 10), replaces a key an older patch left, and + revert removes it. The key is written only when the project's pnpm pins + (`packageManager`, `devEngines`, `engines.pnpm`, or the pnpm that last + installed `node_modules`) say 10.34.2+ (within 10.x), 11.5.3+ (within 11.x) or + 12+: pnpm 10.20–10.34.1 and 11.0–11.5.2 refuse such a key (`ERR_PNPM_INVALID_VERSION_UNION`) and still match the name, so + nothing is needed there. Without a usable pin, or for a decision the + rewriter can't extend (a flow-style value such as + `onlyBuiltDependencies: [pkg]`, a non-boolean `allowBuilds` value), the + setting is left alone with the `vendor_pnpm_build_approval_lost` warning + naming the key to add. Re-run the scan after upgrading pnpm past 10.34.1 or 11.5.2. **Reinstall after redirecting:** a successful warm-cache install can retain upstream bytes. Use a clean install tree and an empty store; `--force` is not a reliable substitute. Run `socket-patch vex` after installation to verify