From 2476f1667b1a0fee0388b68a008a725d0bba0b3e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 12:23:56 +0000 Subject: [PATCH 1/6] Start fix for #1557 Assisted-by: Claude Code:claude-opus-5-5 From 935d3be47fab8b5230400026f45ff0795ee01eb3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 12:42:31 +0000 Subject: [PATCH 2/6] Ignore an empty pnpm trust_lockfile env variable A hosted scan run with pnpm_config_trust_lockfile (or the uppercase spelling) set to an empty string, "null" or "undefined" treated it as an explicit false: it wrote no trustLockfile: true, warned that the key would not take effect, and the next frozen install failed. pnpm 11 and 12 ignore such a value and read pnpm-workspace.yaml, which is common in CI where an unset repository variable exports "". The scan now treats those values as unset, so it writes the key and a member scan still gets its governing-root refusal. Only a literal false is respected, as pnpm does. Fixes #1557 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_pnpm_build.rs | 96 ++++++++++++++++++- crates/socket-patch-core/src/hosted/engine.rs | 42 ++++++++ .../src/utils/pnpm_workspace.rs | 46 ++++++++- 3 files changed, 181 insertions(+), 3 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs index 2303108a0..e02ea05da 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs @@ -172,6 +172,14 @@ fn corepack(cwd: &Path, pm: &str, args: &[&str]) -> Output { /// registry.npmjs.org, where a clamp turned a single `read ECONNRESET` /// into a failed required leg. fn corepack_with_retries(cwd: &Path, pm: &str, args: &[&str], registry_retries: bool) -> Output { + corepack_command(cwd, pm, args, registry_retries) + .output() + .expect("failed to run corepack") +} + +/// The command [`corepack_with_retries`] runs, for a caller that adds +/// environment after the hermetic scrub. +fn corepack_command(cwd: &Path, pm: &str, args: &[&str], registry_retries: bool) -> Command { let mut cmd = pnpm_command(pm); let legacy = pm .strip_prefix("pnpm@") @@ -211,7 +219,7 @@ fn corepack_with_retries(cwd: &Path, pm: &str, args: &[&str], registry_retries: // would otherwise take the sandbox values back out again. cache_env::isolate(&mut cmd); cmd.env("COREPACK_ENABLE_DOWNLOAD_PROMPT", "0"); - cmd.output().expect("failed to run corepack") + cmd } fn run_socket(cwd: &Path, args: &[&str]) -> (i32, String, String) { @@ -818,6 +826,19 @@ fn fresh_checkout_install( label: &str, extra_args: &[&str], with_workspace_yaml: bool, +) -> (PathBuf, Output) { + fresh_checkout_install_env(fx, pm, label, extra_args, with_workspace_yaml, &[]) +} + +/// [`fresh_checkout_install`] with `env` set on the install, after the +/// hermetic scrub. +fn fresh_checkout_install_env( + fx: &PnpmRedirectFixture, + pm: &str, + label: &str, + extra_args: &[&str], + with_workspace_yaml: bool, + env: &[(&str, &str)], ) -> (PathBuf, Output) { let fresh = fx.tmp.path().join(format!("fresh-{label}")); std::fs::create_dir_all(&fresh).unwrap(); @@ -851,7 +872,9 @@ fn fresh_checkout_install( let store_flag = format!("--store-dir={}", fresh_store.display()); let mut args = vec!["install", "--frozen-lockfile", &store_flag]; args.extend_from_slice(extra_args); - let out = corepack(&fresh, pm, &args); + let mut cmd = corepack_command(&fresh, pm, &args, false); + cmd.envs(env.iter().copied()); + let out = cmd.output().expect("failed to run corepack"); (fresh, out) } @@ -1428,6 +1451,75 @@ async fn pnpm11_trust_lockfile_env_override_is_respected() { assert!(workspace.contains("trustLockfile: true"), "{workspace}"); } +/// #1557: pnpm 11+ ignore a `pnpm_config_trust_lockfile` that is empty +/// (what `${{ vars.X }}` exports when the repo variable is unset), `null` +/// or `undefined`, and read pnpm-workspace.yaml instead. A scan whose +/// environment sets it so must heal the trust config as if it were unset, +/// and a fresh frozen install in that same environment then lands the +/// patched bytes from the written key. +#[tokio::test(flavor = "multi_thread")] +#[serial_test::serial] +#[ignore = "wall-bound real-pnpm install (~60s); runs on all 3 OSes as an e2e CI matrix leg"] +async fn pnpm11_empty_trust_lockfile_env_is_ignored() { + let Some(fx) = redirect_scanned_pnpm_project( + PNPM_TERTIARY, + "pnpm11-trust-env-empty", + false, + true, + HostedDriver::Scan, + ) + .await + else { + return; + }; + let ws_path = fx.proj.join("pnpm-workspace.yaml"); + assert!( + !ws_path.exists(), + "the opted-out fixture scan wrote no workspace file" + ); + + let api_url = fx._server.uri(); + let args = [ + "scan", + "--mode", + "hosted", + "--json", + "--yes", + "--cwd", + fx.proj.to_str().unwrap(), + "--api-url", + &api_url, + "--org", + ORG, + "--api-token", + "fake", + ]; + let env = [("pnpm_config_trust_lockfile".to_string(), String::new())]; + let (code, stdout, stderr) = run_socket_env(&fx.proj, &args, &env); + assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}"); + let envelope = parse_envelope(&stdout); + let workspace = std::fs::read_to_string(&ws_path).unwrap_or_default(); + assert!( + workspace.contains("trustLockfile: true"), + "an empty variable pnpm ignores must not stop the write:\n{envelope}" + ); + assert!( + !stdout.contains("in the environment sets pnpm"), + "the warning must not claim the empty variable was respected:\n{envelope}" + ); + + // pnpm itself, run with the same empty variable, honours the key. + let (fresh, ci) = fresh_checkout_install_env( + &fx, + PNPM_TERTIARY, + "pnpm11-trust-env-empty", + &[], + true, + &[("pnpm_config_trust_lockfile", "")], + ); + assert_marker_landed(&fresh, &fx.patched, &ci, "pnpm11 empty trust env"); +} + /// Legacy pnpm@7 leg: the fixture install emits a lockfileVersion 5.4 lock /// (`/name/version:` path-style key), the scan splices its resolution like /// any other grammar, and the fresh dead-registry frozen install proves diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index f443bc187..a90d95641 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -4228,6 +4228,48 @@ snapshots: ); } + /// #1557: pnpm 11+ ignore a `pnpm_config_trust_lockfile` (or + /// `PNPM_CONFIG_TRUST_LOCKFILE`) that is empty, `null` or `undefined` + /// and read pnpm-workspace.yaml instead, so the scan writes + /// `trustLockfile: true` as if the variable were unset, and the warning + /// does not claim the variable was respected. + #[tokio::test] + async fn issue_1557_an_empty_trust_lockfile_variable_keeps_the_write() { + fn empty(var: &str) -> Option { + (var == "pnpm_config_trust_lockfile").then(String::new) + } + fn upper_empty(var: &str) -> Option { + (var == "PNPM_CONFIG_TRUST_LOCKFILE").then(String::new) + } + fn null(var: &str) -> Option { + (var == "pnpm_config_trust_lockfile").then(|| "null".to_string()) + } + fn undefined(var: &str) -> Option { + (var == "PNPM_CONFIG_TRUST_LOCKFILE").then(|| "undefined".to_string()) + } + let mut root = MemoryProject::new(); + root.insert_text("pnpm-lock.yaml", LEFT_PAD_V9_LOCK); + for env in [ + empty as fn(&str) -> Option, + upper_empty, + null, + undefined, + ] { + let (_, done) = + npm_rewrite_in(&ProjectView::Memory(&root), &BTreeSet::new(), &env).await; + assert!(done.rewrite.files.contains_key("pnpm-lock.yaml")); + let workspace = done + .rewrite + .files + .get(PNPM_WORKSPACE_REL) + .unwrap_or_else(|| panic!("{:?}", done.rewrite.files.keys())); + assert!(workspace.contains("trustLockfile: true"), "{workspace}"); + let detail = pnpm_trust_detail(&done); + assert!(detail.contains("no extra flags"), "{detail}"); + assert!(!detail.contains("in the environment"), "{detail}"); + } + } + /// REGRESSION (#367), binary lock: a `bun.lockb`-only project's root /// manifest is read for its `patchedDependencies`, and a package the /// project patches itself with `bun patch` keeps its registry record, diff --git a/crates/socket-patch-core/src/utils/pnpm_workspace.rs b/crates/socket-patch-core/src/utils/pnpm_workspace.rs index 651470dec..45494c3b7 100644 --- a/crates/socket-patch-core/src/utils/pnpm_workspace.rs +++ b/crates/socket-patch-core/src/utils/pnpm_workspace.rs @@ -162,6 +162,11 @@ fn env_setting( /// spelling, which pnpm 11 and newer read ahead of pnpm-workspace.yaml. /// `None` on pnpm 10 and older, which never verify lock URLs, and for /// `npm_config_trust_lockfile`, which no pnpm reads. +/// +/// pnpm ignores a variable that is empty, `null` or `undefined` and reads +/// pnpm-workspace.yaml instead (#1557), so such a value sets nothing here +/// either: only a literal `false` turns the setting off, and any other +/// string is read as true ([`PnpmSetting::as_bool`]). pub fn trust_lockfile_env( major: Option, env: impl Fn(&str) -> Option, @@ -169,8 +174,10 @@ pub fn trust_lockfile_env( if major.is_some_and(|major| major < 11) { return None; } + let env = + |var: &str| env(var).filter(|value| !matches!(value.trim(), "" | "null" | "undefined")); // pnpm 11's spelling, then the uppercase one pnpm 12 reads too. - env_setting("trust-lockfile", Some(major.unwrap_or(11)), &env).or_else(|| { + env_setting("trust-lockfile", Some(major.unwrap_or(11)), env).or_else(|| { let var = "PNPM_CONFIG_TRUST_LOCKFILE"; let value = env(var)?.trim().to_string(); Some(PnpmSetting { @@ -972,6 +979,43 @@ mod tests { ); } + /// #1557: pnpm 11 and newer ignore a `pnpm_config_trust_lockfile` (or + /// `PNPM_CONFIG_TRUST_LOCKFILE`) that is empty, `null` or `undefined` + /// and read `trustLockfile` from pnpm-workspace.yaml instead, so such a + /// value sets nothing. Only a literal `false` turns it off; any other + /// string (`0`, `no`, `FALSE`) is read as true. + #[test] + fn an_empty_or_null_trust_lockfile_variable_sets_nothing() { + let one = |var: &'static str, value: &'static str| { + move |name: &str| (name == var).then(|| value.to_string()) + }; + for var in ["pnpm_config_trust_lockfile", "PNPM_CONFIG_TRUST_LOCKFILE"] { + for value in ["", " ", "null", "undefined"] { + for major in [Some(11), Some(12), None] { + assert_eq!( + trust_lockfile_env(major, one(var, value)), + None, + "{var}={value:?} on pnpm {major:?}" + ); + } + } + for value in ["0", "no", "FALSE", "TRUE"] { + let found = trust_lockfile_env(Some(11), one(var, value)).unwrap(); + assert_eq!(found.as_bool(), Some(true), "{var}={value}"); + } + } + // An ignored lowercase spelling does not hide the uppercase one. + let found = trust_lockfile_env( + Some(12), + env(&[ + ("pnpm_config_trust_lockfile", ""), + ("PNPM_CONFIG_TRUST_LOCKFILE", "false"), + ]), + ) + .unwrap(); + assert_eq!(found.as_bool(), Some(false)); + } + /// #1460: pnpm 11 and newer read only `pnpm_config_*`, pnpm 10 and /// older only `npm_config_*`. #[test] From 6be02041498a19b5df0e3bf9bf0758bd0c0e8095 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 12:50:37 +0000 Subject: [PATCH 3/6] Document that an empty trust env var is ignored Assisted-by: Claude Code:claude-opus-5-5 --- docs/ecosystems.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 9d6762c2a..2319ac719 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -188,9 +188,11 @@ Each Legacy format has an upgrade path and an undo path. Both work in v5: For 9.0 root or member locks, the CLI configures `trustLockfile: true` in the root `pnpm-workspace.yaml` unless opted out with `--no-trust-lockfile-config` or explicitly disabled by the project or, for pnpm >=11, by a - `pnpm_config_trust_lockfile` environment variable the scan runs with (pnpm - reads it ahead of the file, so nothing is written and the warning names the - variable). pnpm >=11 needs this for hosted URLs. + `pnpm_config_trust_lockfile=false` environment variable the scan runs with + (pnpm reads it ahead of the file, so nothing is written and the warning names + the variable). An empty, `null` or `undefined` value disables nothing: pnpm + ignores it and reads the file, so the key is written as if it were unset. + pnpm >=11 needs this for hosted URLs. This skips registry re-verification for the whole lock; tarball integrity remains enforced. pnpm <=10 does not need the setting. An existing `pnpm-workspace.yaml` with no keys (empty or only comments) gets the From 9b470d470d3b70a0693327a6f834a1df9e241cc5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 13:30:15 +0000 Subject: [PATCH 4/6] Read pnpm 11 env booleans only as true or false pnpm 11 reads a pnpm_config_* boolean variable only when it is exactly "true" or "false" and otherwise falls back to pnpm-workspace.yaml. This holds for every key, not just trustLockfile: an empty pnpm_config_lockfile or pnpm_config_shared_workspace_lockfile was also read as false, so a scan could withhold pins or misread the workspace while pnpm itself ignored the variable. Move the rule from the trust-only wrapper into the shared env lookup, so every pnpm_config_* setting (and the uppercase trust spelling) follows it. The npm_config_* family that pnpm 10 and older read keeps its existing coercion. Refs #1557 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_pnpm_build.rs | 19 ++- crates/socket-patch-core/src/hosted/engine.rs | 55 ++++----- .../src/utils/pnpm_workspace.rs | 109 ++++++++++++------ docs/ecosystems.md | 5 +- 4 files changed, 112 insertions(+), 76 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs index e02ea05da..ebaf1109f 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs @@ -1451,9 +1451,10 @@ async fn pnpm11_trust_lockfile_env_override_is_respected() { assert!(workspace.contains("trustLockfile: true"), "{workspace}"); } -/// #1557: pnpm 11+ ignore a `pnpm_config_trust_lockfile` that is empty -/// (what `${{ vars.X }}` exports when the repo variable is unset), `null` -/// or `undefined`, and read pnpm-workspace.yaml instead. A scan whose +/// #1557: pnpm 11+ ignore a `pnpm_config_trust_lockfile` that is not +/// exactly `true` or `false`, such as the empty string `${{ vars.X }}` +/// exports when the repo variable is unset, and read pnpm-workspace.yaml +/// instead. A scan whose /// environment sets it so must heal the trust config as if it were unset, /// and a fresh frozen install in that same environment then lands the /// patched bytes from the written key. @@ -1503,9 +1504,17 @@ async fn pnpm11_empty_trust_lockfile_env_is_ignored() { workspace.contains("trustLockfile: true"), "an empty variable pnpm ignores must not stop the write:\n{envelope}" ); + let detail = envelope["warnings"] + .as_array() + .into_iter() + .flatten() + .find(|w| w["code"] == "redirect_pnpm_trust_lockfile") + .and_then(|w| w["detail"].as_str()) + .unwrap_or_else(|| panic!("redirect_pnpm_trust_lockfile:\n{envelope}")) + .to_string(); assert!( - !stdout.contains("in the environment sets pnpm"), - "the warning must not claim the empty variable was respected:\n{envelope}" + detail.contains("no extra flags") && !detail.contains("in the environment"), + "the warning must not claim the empty variable was respected:\n{detail}" ); // pnpm itself, run with the same empty variable, honours the key. diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index a90d95641..af2a6f0ad 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -4229,44 +4229,31 @@ snapshots: } /// #1557: pnpm 11+ ignore a `pnpm_config_trust_lockfile` (or - /// `PNPM_CONFIG_TRUST_LOCKFILE`) that is empty, `null` or `undefined` - /// and read pnpm-workspace.yaml instead, so the scan writes - /// `trustLockfile: true` as if the variable were unset, and the warning - /// does not claim the variable was respected. + /// `PNPM_CONFIG_TRUST_LOCKFILE`) that is not exactly `true` or `false` + /// (an unset CI variable's empty string, `null`, `undefined`) and read + /// pnpm-workspace.yaml instead, so the scan writes `trustLockfile: + /// true` as if the variable were unset, and the warning does not claim + /// the variable was respected. #[tokio::test] async fn issue_1557_an_empty_trust_lockfile_variable_keeps_the_write() { - fn empty(var: &str) -> Option { - (var == "pnpm_config_trust_lockfile").then(String::new) - } - fn upper_empty(var: &str) -> Option { - (var == "PNPM_CONFIG_TRUST_LOCKFILE").then(String::new) - } - fn null(var: &str) -> Option { - (var == "pnpm_config_trust_lockfile").then(|| "null".to_string()) - } - fn undefined(var: &str) -> Option { - (var == "PNPM_CONFIG_TRUST_LOCKFILE").then(|| "undefined".to_string()) - } let mut root = MemoryProject::new(); root.insert_text("pnpm-lock.yaml", LEFT_PAD_V9_LOCK); - for env in [ - empty as fn(&str) -> Option, - upper_empty, - null, - undefined, - ] { - let (_, done) = - npm_rewrite_in(&ProjectView::Memory(&root), &BTreeSet::new(), &env).await; - assert!(done.rewrite.files.contains_key("pnpm-lock.yaml")); - let workspace = done - .rewrite - .files - .get(PNPM_WORKSPACE_REL) - .unwrap_or_else(|| panic!("{:?}", done.rewrite.files.keys())); - assert!(workspace.contains("trustLockfile: true"), "{workspace}"); - let detail = pnpm_trust_detail(&done); - assert!(detail.contains("no extra flags"), "{detail}"); - assert!(!detail.contains("in the environment"), "{detail}"); + for var in ["pnpm_config_trust_lockfile", "PNPM_CONFIG_TRUST_LOCKFILE"] { + for value in ["", "null", "undefined"] { + let env = move |name: &str| (name == var).then(|| value.to_string()); + let (_, done) = + npm_rewrite_in(&ProjectView::Memory(&root), &BTreeSet::new(), &env).await; + assert!(done.rewrite.files.contains_key("pnpm-lock.yaml")); + let workspace = done + .rewrite + .files + .get(PNPM_WORKSPACE_REL) + .unwrap_or_else(|| panic!("{:?}", done.rewrite.files.keys())); + assert!(workspace.contains("trustLockfile: true"), "{workspace}"); + let detail = pnpm_trust_detail(&done); + assert!(detail.contains("no extra flags"), "{detail}"); + assert!(!detail.contains("in the environment"), "{detail}"); + } } } diff --git a/crates/socket-patch-core/src/utils/pnpm_workspace.rs b/crates/socket-patch-core/src/utils/pnpm_workspace.rs index 45494c3b7..0a1bd1056 100644 --- a/crates/socket-patch-core/src/utils/pnpm_workspace.rs +++ b/crates/socket-patch-core/src/utils/pnpm_workspace.rs @@ -143,10 +143,11 @@ fn env_setting( Some(_) => &["npm_config_"], None => &["pnpm_config_", "npm_config_"], }; - prefixes - .iter() - .map(|prefix| format!("{prefix}{key}")) - .find_map(|var| { + prefixes.iter().find_map(|prefix| { + let var = format!("{prefix}{key}"); + if *prefix == "pnpm_config_" { + pnpm_env_setting(&var, &env) + } else { let value = env(&var)?.trim().to_string(); let source = format!("`{var}={value}` in the environment"); Some(PnpmSetting { @@ -154,19 +155,30 @@ fn env_setting( source, yaml: false, }) - }) + } + }) +} + +/// A boolean setting from `var`, a variable of the `pnpm_config_*` family +/// (either case) that pnpm 11 and newer read. pnpm takes only an exact +/// `true` or `false` there and ignores any other value (empty, `null`, +/// `undefined`, `0`, `FALSE`, ` false`), reading the settings file instead +/// (#1557), so such a value sets nothing. +fn pnpm_env_setting(var: &str, env: impl Fn(&str) -> Option) -> Option { + let value = env(var).filter(|value| matches!(value.as_str(), "true" | "false"))?; + Some(PnpmSetting { + source: format!("`{var}={value}` in the environment"), + value, + yaml: false, + }) } /// pnpm's `trustLockfile` as the environment sets it, for a project on /// pnpm `major` (#1514): `pnpm_config_trust_lockfile`, or its uppercase -/// spelling, which pnpm 11 and newer read ahead of pnpm-workspace.yaml. -/// `None` on pnpm 10 and older, which never verify lock URLs, and for +/// spelling, which pnpm 11 and newer read ahead of pnpm-workspace.yaml +/// when it is `true` or `false` (see [`pnpm_env_setting`]). `None` on pnpm +/// 10 and older, which never verify lock URLs, and for /// `npm_config_trust_lockfile`, which no pnpm reads. -/// -/// pnpm ignores a variable that is empty, `null` or `undefined` and reads -/// pnpm-workspace.yaml instead (#1557), so such a value sets nothing here -/// either: only a literal `false` turns the setting off, and any other -/// string is read as true ([`PnpmSetting::as_bool`]). pub fn trust_lockfile_env( major: Option, env: impl Fn(&str) -> Option, @@ -174,18 +186,9 @@ pub fn trust_lockfile_env( if major.is_some_and(|major| major < 11) { return None; } - let env = - |var: &str| env(var).filter(|value| !matches!(value.trim(), "" | "null" | "undefined")); // pnpm 11's spelling, then the uppercase one pnpm 12 reads too. - env_setting("trust-lockfile", Some(major.unwrap_or(11)), env).or_else(|| { - let var = "PNPM_CONFIG_TRUST_LOCKFILE"; - let value = env(var)?.trim().to_string(); - Some(PnpmSetting { - source: format!("`{var}={value}` in the environment"), - value, - yaml: false, - }) - }) + env_setting("trust-lockfile", Some(major.unwrap_or(11)), &env) + .or_else(|| pnpm_env_setting("PNPM_CONFIG_TRUST_LOCKFILE", &env)) } /// [`pnpm_setting`], falling back to the environment ([`env_setting`], @@ -946,7 +949,7 @@ mod tests { None, env(&[ ("npm_config_git_branch_lockfile", "false"), - ("pnpm_config_git_branch_lockfile", "TRUE"), + ("pnpm_config_git_branch_lockfile", "true"), ]), ) .unwrap(); @@ -960,7 +963,7 @@ mod tests { #[test] fn the_environment_sets_trust_lockfile_only_for_pnpm_11_and_newer() { const LOWER: &[(&str, &str)] = &[("pnpm_config_trust_lockfile", "false")]; - const UPPER: &[(&str, &str)] = &[("PNPM_CONFIG_TRUST_LOCKFILE", " false ")]; + const UPPER: &[(&str, &str)] = &[("PNPM_CONFIG_TRUST_LOCKFILE", "false")]; const NPM: &[(&str, &str)] = &[("npm_config_trust_lockfile", "false")]; for major in [Some(11), Some(12), None] { let found = trust_lockfile_env(major, env(LOWER)).expect("lowercase"); @@ -979,18 +982,21 @@ mod tests { ); } - /// #1557: pnpm 11 and newer ignore a `pnpm_config_trust_lockfile` (or - /// `PNPM_CONFIG_TRUST_LOCKFILE`) that is empty, `null` or `undefined` - /// and read `trustLockfile` from pnpm-workspace.yaml instead, so such a - /// value sets nothing. Only a literal `false` turns it off; any other - /// string (`0`, `no`, `FALSE`) is read as true. + /// #1557: pnpm 11 and newer read a `pnpm_config_*` boolean only when it + /// is exactly `true` or `false` and otherwise ignore it for the settings + /// file (measured with `pnpm config get` on pnpm 11.28.5), so any other + /// value (an unset CI variable's empty string, `null`, `undefined`, + /// `0`, `FALSE`, ` false`) sets nothing, for `trustLockfile` and every + /// other key alike. npm's family keeps its own coercion (pnpm 10 and + /// older, see [`PnpmSetting::as_bool`]). #[test] - fn an_empty_or_null_trust_lockfile_variable_sets_nothing() { + fn a_pnpm_config_variable_counts_only_as_true_or_false() { let one = |var: &'static str, value: &'static str| { move |name: &str| (name == var).then(|| value.to_string()) }; + const IGNORED: &[&str] = &["", " ", "null", "undefined", "0", "FALSE", " false", "yes"]; for var in ["pnpm_config_trust_lockfile", "PNPM_CONFIG_TRUST_LOCKFILE"] { - for value in ["", " ", "null", "undefined"] { + for value in IGNORED { for major in [Some(11), Some(12), None] { assert_eq!( trust_lockfile_env(major, one(var, value)), @@ -999,12 +1005,31 @@ mod tests { ); } } - for value in ["0", "no", "FALSE", "TRUE"] { + for (value, on) in [("true", true), ("false", false)] { let found = trust_lockfile_env(Some(11), one(var, value)).unwrap(); - assert_eq!(found.as_bool(), Some(true), "{var}={value}"); + assert_eq!(found.as_bool(), Some(on), "{var}={value}"); } } - // An ignored lowercase spelling does not hide the uppercase one. + for key in [ + "lockfile", + "shared-workspace-lockfile", + "git-branch-lockfile", + ] { + let var: &'static str = + Box::leak(format!("pnpm_config_{}", key.replace('-', "_")).into_boxed_str()); + for value in IGNORED { + for major in [Some(11), None] { + assert_eq!( + env_setting(key, major, one(var, value)), + None, + "{var}={value:?}" + ); + } + } + let found = env_setting(key, Some(11), one(var, "false")).unwrap(); + assert_eq!(found.as_bool(), Some(false), "{var}"); + } + // An ignored pnpm spelling does not hide the next one pnpm reads. let found = trust_lockfile_env( Some(12), env(&[ @@ -1013,6 +1038,20 @@ mod tests { ]), ) .unwrap(); + assert!(found.source.contains("PNPM_CONFIG_TRUST_LOCKFILE=false")); + // With no major known, an ignored pnpm spelling leaves npm's. + let found = env_setting( + "lockfile", + None, + env(&[ + ("pnpm_config_lockfile", ""), + ("npm_config_lockfile", "false"), + ]), + ) + .unwrap(); + assert!(found.source.contains("`npm_config_lockfile=false`")); + // npm's family is unchanged: its empty value still reads as false. + let found = env_setting("lockfile", Some(10), env(&[("npm_config_lockfile", "")])).unwrap(); assert_eq!(found.as_bool(), Some(false)); } diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 2319ac719..14404666d 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -190,8 +190,9 @@ Each Legacy format has an upgrade path and an undo path. Both work in v5: explicitly disabled by the project or, for pnpm >=11, by a `pnpm_config_trust_lockfile=false` environment variable the scan runs with (pnpm reads it ahead of the file, so nothing is written and the warning names - the variable). An empty, `null` or `undefined` value disables nothing: pnpm - ignores it and reads the file, so the key is written as if it were unset. + the variable). pnpm >=11 reads a `pnpm_config_*` variable only when it is + exactly `true` or `false`; any other value (empty, `null`, `undefined`, `0`) + is ignored for the file, so the CLI ignores it too and writes the key. pnpm >=11 needs this for hosted URLs. This skips registry re-verification for the whole lock; tarball integrity remains enforced. pnpm <=10 does not need the setting. An existing From 9f1a373ffd151d6b75f7f130da6cbb0af06fbc00 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 14:23:28 +0000 Subject: [PATCH 5/6] Trim pnpm 12 env booleans before matching pnpm 12 trims a pnpm_config_* value before reading it as true or false, so " false" and "false " turn a setting off there; pnpm 11 does not trim and ignores them. The previous commit matched exactly for every major, so on pnpm 12 a padded false lockfile variable let hosted pins through that pnpm never installs, and a padded false trust variable got a trustLockfile key written that it overrides. Trim unless the project is on pnpm 11, including when the major is unknown, which matches the behaviour before this PR. Refs #1557 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/utils/pnpm_workspace.rs | 66 ++++++++++++++----- docs/ecosystems.md | 5 +- 2 files changed, 53 insertions(+), 18 deletions(-) diff --git a/crates/socket-patch-core/src/utils/pnpm_workspace.rs b/crates/socket-patch-core/src/utils/pnpm_workspace.rs index 0a1bd1056..4d4eb5687 100644 --- a/crates/socket-patch-core/src/utils/pnpm_workspace.rs +++ b/crates/socket-patch-core/src/utils/pnpm_workspace.rs @@ -146,7 +146,7 @@ fn env_setting( prefixes.iter().find_map(|prefix| { let var = format!("{prefix}{key}"); if *prefix == "pnpm_config_" { - pnpm_env_setting(&var, &env) + pnpm_env_setting(&var, major, &env) } else { let value = env(&var)?.trim().to_string(); let source = format!("`{var}={value}` in the environment"); @@ -160,12 +160,28 @@ fn env_setting( } /// A boolean setting from `var`, a variable of the `pnpm_config_*` family -/// (either case) that pnpm 11 and newer read. pnpm takes only an exact -/// `true` or `false` there and ignores any other value (empty, `null`, -/// `undefined`, `0`, `FALSE`, ` false`), reading the settings file instead -/// (#1557), so such a value sets nothing. -fn pnpm_env_setting(var: &str, env: impl Fn(&str) -> Option) -> Option { - let value = env(var).filter(|value| matches!(value.as_str(), "true" | "false"))?; +/// (either case) that pnpm 11 and newer read for a project on pnpm +/// `major`. pnpm takes only `true` or `false` there and ignores any other +/// value (empty, `null`, `undefined`, `0`, `FALSE`), reading the settings +/// file instead (#1557), so such a value sets nothing. pnpm 12 trims the +/// value first (` false` is false); pnpm 11 does not (` false` is +/// ignored). With the major unknown the value is trimmed, so a padded +/// `false` is respected: for the lock that withholds pins, the safe side. +fn pnpm_env_setting( + var: &str, + major: Option, + env: impl Fn(&str) -> Option, +) -> Option { + let raw = env(var)?; + let value = if major == Some(11) { + raw.as_str() + } else { + raw.trim() + }; + if !matches!(value, "true" | "false") { + return None; + } + let value = value.to_string(); Some(PnpmSetting { source: format!("`{var}={value}` in the environment"), value, @@ -187,8 +203,8 @@ pub fn trust_lockfile_env( return None; } // pnpm 11's spelling, then the uppercase one pnpm 12 reads too. - env_setting("trust-lockfile", Some(major.unwrap_or(11)), &env) - .or_else(|| pnpm_env_setting("PNPM_CONFIG_TRUST_LOCKFILE", &env)) + pnpm_env_setting("pnpm_config_trust_lockfile", major, &env) + .or_else(|| pnpm_env_setting("PNPM_CONFIG_TRUST_LOCKFILE", major, &env)) } /// [`pnpm_setting`], falling back to the environment ([`env_setting`], @@ -983,18 +999,21 @@ mod tests { } /// #1557: pnpm 11 and newer read a `pnpm_config_*` boolean only when it - /// is exactly `true` or `false` and otherwise ignore it for the settings - /// file (measured with `pnpm config get` on pnpm 11.28.5), so any other - /// value (an unset CI variable's empty string, `null`, `undefined`, - /// `0`, `FALSE`, ` false`) sets nothing, for `trustLockfile` and every - /// other key alike. npm's family keeps its own coercion (pnpm 10 and - /// older, see [`PnpmSetting::as_bool`]). + /// is `true` or `false` and otherwise ignore it for the settings file + /// (measured with `pnpm config get` on pnpm 11.28.5 and 12.11.2), so any + /// other value (an unset CI variable's empty string, `null`, + /// `undefined`, `0`, `FALSE`) sets nothing, for `trustLockfile` and + /// every other key alike. pnpm 12 trims the value first and pnpm 11 + /// does not, so a padded ` false` is ignored only on pnpm 11; with the + /// major unknown it is respected. npm's family keeps its own coercion + /// (pnpm 10 and older, see [`PnpmSetting::as_bool`]). #[test] fn a_pnpm_config_variable_counts_only_as_true_or_false() { let one = |var: &'static str, value: &'static str| { move |name: &str| (name == var).then(|| value.to_string()) }; - const IGNORED: &[&str] = &["", " ", "null", "undefined", "0", "FALSE", " false", "yes"]; + const IGNORED: &[&str] = &["", " ", "null", "undefined", "0", "FALSE", "yes"]; + const PADDED: &[&str] = &[" false", "false ", " false "]; for var in ["pnpm_config_trust_lockfile", "PNPM_CONFIG_TRUST_LOCKFILE"] { for value in IGNORED { for major in [Some(11), Some(12), None] { @@ -1009,6 +1028,14 @@ mod tests { let found = trust_lockfile_env(Some(11), one(var, value)).unwrap(); assert_eq!(found.as_bool(), Some(on), "{var}={value}"); } + for value in PADDED { + assert_eq!(trust_lockfile_env(Some(11), one(var, value)), None); + for major in [Some(12), None] { + let found = trust_lockfile_env(major, one(var, value)).unwrap(); + assert_eq!(found.as_bool(), Some(false), "{var}={value:?}"); + assert!(found.source.contains(&format!("`{var}=false`"))); + } + } } for key in [ "lockfile", @@ -1028,6 +1055,13 @@ mod tests { } let found = env_setting(key, Some(11), one(var, "false")).unwrap(); assert_eq!(found.as_bool(), Some(false), "{var}"); + for value in PADDED { + assert_eq!(env_setting(key, Some(11), one(var, value)), None); + for major in [Some(12), None] { + let found = env_setting(key, major, one(var, value)).unwrap(); + assert_eq!(found.as_bool(), Some(false), "{var}={value:?}"); + } + } } // An ignored pnpm spelling does not hide the next one pnpm reads. let found = trust_lockfile_env( diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 14404666d..b9959b28a 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -191,8 +191,9 @@ Each Legacy format has an upgrade path and an undo path. Both work in v5: `pnpm_config_trust_lockfile=false` environment variable the scan runs with (pnpm reads it ahead of the file, so nothing is written and the warning names the variable). pnpm >=11 reads a `pnpm_config_*` variable only when it is - exactly `true` or `false`; any other value (empty, `null`, `undefined`, `0`) - is ignored for the file, so the CLI ignores it too and writes the key. + `true` or `false` (pnpm 12 trims surrounding spaces first, pnpm 11 does + not); any other value (empty, `null`, `undefined`, `0`) is ignored for the + file, so the CLI ignores it too and writes the key. pnpm >=11 needs this for hosted URLs. This skips registry re-verification for the whole lock; tarball integrity remains enforced. pnpm <=10 does not need the setting. An existing From 45721dde308a0479f8c06b6470060840e67351b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 14:43:16 +0000 Subject: [PATCH 6/6] Read the trust env var at the member's pnpm major The hosted member preflight read pnpm_config_trust_lockfile with no pnpm major, so a padded " false" counted as an override on a pnpm 11 member and skipped the governing-root refusal, while the rewrite (on pnpm 11) ignored it and wrote no trust key, leaving pins that fail at install. The preflight now reads the variable at the project's own pnpm major, as the rewrite does. Refs #1557 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/commands/scan/hosted.rs | 13 ++++--- .../src/utils/pnpm_workspace.rs | 37 +++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index 3b5fba8b1..df1087ffb 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -952,11 +952,14 @@ async fn run_redirect_selected_untracked( // the workspace root's pnpm-workspace.yaml, unless a // `pnpm_config_trust_lockfile` in this environment overrides that file // (#1514): adding the key there would not help, and the rewrite's - // trust warning names the variable instead. - let trust_env_off = socket_patch_core::utils::pnpm_workspace::trust_lockfile_env(None, |var| { - std::env::var(var).ok() - }) - .is_some_and(|found| found.as_bool() != Some(true)); + // trust warning names the variable instead. The variable is read at + // the project's pnpm major, as the rewrite reads it (#1557). + let trust_env_off = + socket_patch_core::utils::pnpm_workspace::trust_lockfile_env_for(&view, |var| { + std::env::var(var).ok() + }) + .await + .is_some_and(|found| found.as_bool() != Some(true)); if let Some(refusal) = socket_patch_core::hosted::governing_root::refusal( &view, &candidates, diff --git a/crates/socket-patch-core/src/utils/pnpm_workspace.rs b/crates/socket-patch-core/src/utils/pnpm_workspace.rs index 4d4eb5687..b12928665 100644 --- a/crates/socket-patch-core/src/utils/pnpm_workspace.rs +++ b/crates/socket-patch-core/src/utils/pnpm_workspace.rs @@ -207,6 +207,18 @@ pub fn trust_lockfile_env( .or_else(|| pnpm_env_setting("PNPM_CONFIG_TRUST_LOCKFILE", major, &env)) } +/// [`trust_lockfile_env`] for the project `view` shows, at the pnpm major +/// that installs it: whether pnpm 11 ignores a padded value or pnpm 12 +/// trims it depends on the major. The project's files are read for it only +/// when the environment sets the variable at all. +pub async fn trust_lockfile_env_for( + view: &ProjectView<'_>, + env: impl Fn(&str) -> Option, +) -> Option { + trust_lockfile_env(None, &env)?; + trust_lockfile_env(pnpm_major(view).await, &env) +} + /// [`pnpm_setting`], falling back to the environment ([`env_setting`], /// for the project's pnpm major) on disk, where the CLI runs pnpm's /// environment. @@ -1089,6 +1101,31 @@ mod tests { assert_eq!(found.as_bool(), Some(false)); } + /// #1557: the trust variable is read at the project's own pnpm major, + /// so the hosted member preflight and the rewrite agree on a padded + /// value: ignored on pnpm 11, `false` on pnpm 12. + #[tokio::test] + async fn the_trust_variable_follows_the_project_pnpm_major() { + const PADDED: &[(&str, &str)] = &[("pnpm_config_trust_lockfile", " false")]; + for (pm, off) in [("pnpm@11.28.5", false), ("pnpm@12.11.2", true), ("", true)] { + let tmp = tempfile::tempdir().unwrap(); + let manifest = if pm.is_empty() { + r#"{"name":"app"}"#.to_string() + } else { + format!(r#"{{"name":"app","packageManager":"{pm}"}}"#) + }; + write(tmp.path(), "package.json", &manifest); + let view = ProjectView::Disk(tmp.path()); + let found = trust_lockfile_env_for(&view, env(PADDED)).await; + assert_eq!( + found.and_then(|found| found.as_bool()), + off.then_some(false), + "{pm}" + ); + assert_eq!(trust_lockfile_env_for(&view, env(&[])).await, None); + } + } + /// #1460: pnpm 11 and newer read only `pnpm_config_*`, pnpm 10 and /// older only `npm_config_*`. #[test]