Fix pnpm 11 env booleans other than true/false (#1557)

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,

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,84 @@ 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 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.
+#[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}"
+    );
+    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!(
+        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.
+    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

crates/socket-patch-core/src/hosted/engine.rs

@@ -4228,6 +4228,35 @@ snapshots:
         );
     }
 
+    /// #1557: pnpm 11+ ignore a `pnpm_config_trust_lockfile` (or
+    /// `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() {
+        let mut root = MemoryProject::new();
+        root.insert_text("pnpm-lock.yaml", LEFT_PAD_V9_LOCK);
+        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}");
+            }
+        }
+    }
+
     /// 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,