Fix pnpm trustLockfile env override ignored (#1514)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -157,7 +157,7 @@ Beyond the globals above, each subcommand defines a small set of local arguments
 
 Each matching package instance is spliced, including scoped, quoted and nested-peer keys, one `redirect_pnpm_resolution` edit per changed instance (`rollback` / `remove` restore each from the npm registry — see "Hosted unwind coverage"). LF/CRLF and unrelated lock bytes are preserved. Unsupported matching instances refuse that dependency across the lockfile set; an already-hosted URL elsewhere cannot confirm a partial rewrite.
 
-For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLockfile: true` (created with a root-only `packages:` scaffold, or appended while preserving user bytes). An existing file with no keys at all — empty, comments only, bare `---` / `...` / `%` directive lines — reads to pnpm like no file, but pnpm 8.x–10.4 refuse every command once it holds a key without `packages` ("packages field missing or empty", #1096), so it gets the same root-only `packages:` scaffold spliced in with the key, and follows the no-file rules below. pnpm 8–10.4 read a keyless file as every nested package (pnpm >= 10.5 as the root alone), so when `pnpm-lock.yaml` lists importers besides `.` the file is left untouched and the warning says to list the members under `packages:` after upgrading to pnpm >= 11 (or install with `--trust-lockfile`); the vendored mirror skips the file the same way. A keyless file holding several documents is not spliced (the shapes rule below). A project with no `pnpm-workspace.yaml` (or a keyless one) whose every pnpm pin — the installed `node_modules/.modules.yaml` `packageManager`, and package.json `packageManager`, `devEngines.packageManager` and `engines.pnpm` (at least one present) — names pnpm 9.0–10.4 gets no file (#734): those releases never read the key, and a created root-only workspace makes `pnpm add <pkg>` fail with ERR_PNPM_ADDING_TO_ROOT there; the warning names the pins and says to re-run after upgrading to pnpm >=11 (or install with `--trust-lockfile`). A pin naming a later pnpm, a range reaching past 10.4, an unparseable pin, or no pin keeps the scaffold, whose warning notes that pnpm 9.0–10.4 then need `pnpm add -w <pkg>` and that pinning that pnpm in package.json `packageManager` (then deleting the file and re-running) avoids it — provided every other pin names 9.0–10.4 too (the "every pin" rule above). **Known residual (#734)**: an unpinned project with no install record (no `node_modules/.modules.yaml`, no package.json pnpm pin — e.g. a fresh clone) still gets the root-only scaffold, since nothing shows its pnpm cannot read it; on pnpm 9.0–10.4 such a project needs `pnpm add -w <pkg>` to add dependencies, or a `packageManager` pin naming that pnpm so the scaffold is never created. pnpm >=11 requires this to accept hosted URLs; it disables registry re-verification for the whole lock, while sha512 tarball integrity remains enforced. The write (edit kind `redirect_pnpm_workspace_trust`) respects `--dry-run`, skips legacy locks and Rush repos, preserves explicit user settings (an existing top-level key in any YAML spelling: quoted, `trustLockfile :`, with a trailing comment), and is disabled by `--no-trust-lockfile-config`. The key goes inside the document (before a `...` end marker); a file a line append would corrupt (a flow-style root, an indented root, several documents) is left untouched and the warning gives the manual recoveries. The vendored `overrides:` mirror in `pnpm-workspace.yaml` reads keys the same way and refuses those shapes before writing. The `redirect_pnpm_trust_lockfile` warning explains manual configuration when required and clean reinstall guidance for all pnpm versions. When every lock the run spliced is a Rush lock, the same warning code carries the Rush remedy instead (rush runs pnpm in `common/temp`, so neither the repo-root key nor `--trust-lockfile` reaches it): `pnpm_config_trust_lockfile=true rush install`, plus `"usePnpmFrozenLockfileForRushInstall": true` in `common/config/rush/experiments.json` on pnpm 11 (whose default Rush install otherwise re-resolves the hosted entries to upstream with exit 0), and `rush purge` before reinstalling. A re-run over a 9.0 Rush lock an earlier run already redirected (nothing spliced) re-issues the same Rush remedy; a run that also spliced a non-Rush pnpm lock keeps the generic text plus a Rush note. Existing installs and warm stores can retain upstream files; use a clean install tree and empty store, then verify installed files with `socket-patch vex`. Neither a successful install nor a local VEX export guarantees hosted SBOM recognition or changes dashboard alert actions/counts.
+For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLockfile: true` (created with a root-only `packages:` scaffold, or appended while preserving user bytes). An existing file with no keys at all — empty, comments only, bare `---` / `...` / `%` directive lines — reads to pnpm like no file, but pnpm 8.x–10.4 refuse every command once it holds a key without `packages` ("packages field missing or empty", #1096), so it gets the same root-only `packages:` scaffold spliced in with the key, and follows the no-file rules below. pnpm 8–10.4 read a keyless file as every nested package (pnpm >= 10.5 as the root alone), so when `pnpm-lock.yaml` lists importers besides `.` the file is left untouched and the warning says to list the members under `packages:` after upgrading to pnpm >= 11 (or install with `--trust-lockfile`); the vendored mirror skips the file the same way. A keyless file holding several documents is not spliced (the shapes rule below). A project with no `pnpm-workspace.yaml` (or a keyless one) whose every pnpm pin — the installed `node_modules/.modules.yaml` `packageManager`, and package.json `packageManager`, `devEngines.packageManager` and `engines.pnpm` (at least one present) — names pnpm 9.0–10.4 gets no file (#734): those releases never read the key, and a created root-only workspace makes `pnpm add <pkg>` fail with ERR_PNPM_ADDING_TO_ROOT there; the warning names the pins and says to re-run after upgrading to pnpm >=11 (or install with `--trust-lockfile`). A pin naming a later pnpm, a range reaching past 10.4, an unparseable pin, or no pin keeps the scaffold, whose warning notes that pnpm 9.0–10.4 then need `pnpm add -w <pkg>` and that pinning that pnpm in package.json `packageManager` (then deleting the file and re-running) avoids it — provided every other pin names 9.0–10.4 too (the "every pin" rule above). **Known residual (#734)**: an unpinned project with no install record (no `node_modules/.modules.yaml`, no package.json pnpm pin — e.g. a fresh clone) still gets the root-only scaffold, since nothing shows its pnpm cannot read it; on pnpm 9.0–10.4 such a project needs `pnpm add -w <pkg>` to add dependencies, or a `packageManager` pin naming that pnpm so the scaffold is never created. pnpm >=11 requires this to accept hosted URLs; it disables registry re-verification for the whole lock, while sha512 tarball integrity remains enforced. The write (edit kind `redirect_pnpm_workspace_trust`) respects `--dry-run`, skips legacy locks and Rush repos, preserves explicit user settings (an existing top-level key in any YAML spelling: quoted, `trustLockfile :`, with a trailing comment; and a `pnpm_config_trust_lockfile` / `PNPM_CONFIG_TRUST_LOCKFILE` environment variable set to anything but `true` when the project's pnpm is >=11 or unknown, which pnpm reads ahead of the file, so nothing is written and the warning names the variable, #1514), and is disabled by `--no-trust-lockfile-config`. The key goes inside the document (before a `...` end marker); a file a line append would corrupt (a flow-style root, an indented root, several documents) is left untouched and the warning gives the manual recoveries. The vendored `overrides:` mirror in `pnpm-workspace.yaml` reads keys the same way and refuses those shapes before writing. The `redirect_pnpm_trust_lockfile` warning explains manual configuration when required and clean reinstall guidance for all pnpm versions. When every lock the run spliced is a Rush lock, the same warning code carries the Rush remedy instead (rush runs pnpm in `common/temp`, so neither the repo-root key nor `--trust-lockfile` reaches it): `pnpm_config_trust_lockfile=true rush install`, plus `"usePnpmFrozenLockfileForRushInstall": true` in `common/config/rush/experiments.json` on pnpm 11 (whose default Rush install otherwise re-resolves the hosted entries to upstream with exit 0), and `rush purge` before reinstalling. A re-run over a 9.0 Rush lock an earlier run already redirected (nothing spliced) re-issues the same Rush remedy; a run that also spliced a non-Rush pnpm lock keeps the generic text plus a Rush note. Existing installs and warm stores can retain upstream files; use a clean install tree and empty store, then verify installed files with `socket-patch vex`. Neither a successful install nor a local VEX export guarantees hosted SBOM recognition or changes dashboard alert actions/counts.
 
 **npm hosted-mode `allow-remote` contract**: npm >=12 defaults `allow-remote=none` and refuses (EALLOWREMOTE) every lockfile entry whose `resolved` tarball is not served by the configured registry — exactly what a hosted redirect writes into `package-lock.json` / `npm-shrinkwrap.json`. Whenever a run leaves a ROOT npm lock carrying a granted hosted artifact URL (spliced this run, or already redirected by an earlier one — a missed config heals on re-run), the CLI ensures `allow-remote=all` in the project-root `.npmrc`: the file is created holding exactly `allow-remote=all\n` when absent, otherwise one `allow-remote=all` line is spliced in after the last non-empty top-level line (before any ini `[section]` header), in the file's own line ending, with the BOM, CRLF and trailing-newline shape preserved. The write lands in `redirect.rewrittenFiles` (edit kind `redirect_npmrc_allow_remote`), respects `--dry-run` (nothing written; the warning says what would be — including for a vendored → hosted takeover the dry run only previews), and is disabled by `--no-npm-allow-remote-config` / `SOCKET_NO_NPM_ALLOW_REMOTE_CONFIG`. The `.npmrc` grammar is npm's own `ini` parser's (cross-checked against it): lines split on any run of `\r` / `\n` (a bare `\r` ends a line), only the exact key `allow-remote` counts after ini unquoting (npm ignores `allow_remote` / `ALLOW-REMOTE` in a `.npmrc`; such a line is left alone and the real key appended), comment lines are ignored, a `[section]` header is recognized only as npm does — on the UNTRIMMED line (an indented or BOM-prefixed `[sec]` is a plain top-level key) — and ends the top-level scope, quotes and inline comments are stripped, the LAST top-level assignment wins, and the value is case-sensitive. An explicit other value (`allow-remote=none` / `root` / anything but `all`) is RESPECTED and never rewritten — the pnpm `trustLockfile: false` precedent — in the project `.npmrc` AND in every other npm config layer npm would consult: an `npm_config_allow_remote` environment variable (any spelling npm normalizes; it beats every `.npmrc`, so a project write could not take effect), and — when the project file sets nothing — the user (`npm_config_userconfig` / `~/.npmrc`), global (`npm_config_globalconfig` / `<prefix>/etc/npmrc`, prefix from `npm_config_prefix`, the user/builtin config, `PREFIX` or the `node` binary's install root) and builtin (npm's own `npmrc` beside the `node` binary: `<dir>/lib/node_modules/npm/npmrc`, `<dir>\node_modules\npm\npmrc` on Windows) config files — path values `${VAR}`-expanded and `~`-expanded like npm, env names case-insensitive on Windows, where a committed project line would silently override a machine / org policy. A symlinked, non-regular or unreadable `.npmrc`, or one with bare-`\r` line endings (npm splits on them, the line splice does not), is left untouched. Every variant emits the `redirect_npm_allow_remote` warning (written / would write / already set / explicit value respected — naming the project file, the env var, or the user/global/builtin config path — / opted out / unreadable or unsupported), always with the tradeoff: `allow-remote=all` lets npm install ANY url-resolved dependency, not just Socket's patched ones, while the per-entry sha512 integrity pins stay enforced; the remedy for the non-writing variants is `allow-remote=all` in `.npmrc` or `npm ci --allow-remote=all`. npm <=11 is unaffected (11 defaults to `all`, <=10 has no such setting). **Unwind (v5.0: no ledger)**: once `rollback`, `remove` or a hosted → vendored takeover has restored the last hosted entry of the root `package-lock.json` / `npm-shrinkwrap.json` to its upstream registry entry (see "Hosted unwind coverage"), a project `.npmrc` holding exactly `allow-remote=all\n` (the file hosted mode creates) is deleted; any other `.npmrc` that still has a top-level `allow-remote=all` line is left untouched and the `npm_allow_remote_left` warning says the line may be removed if nothing else needs it (v5 keeps no record of whether hosted mode added it, so it is never removed behind the user's back). The rewrite's stage file is created with the `.npmrc`'s own permission bits (a 0600 token-bearing file is never staged world-readable). **Vendored mode is unaffected by `allow-remote`**: its `file:.socket/vendor/…` resolutions are npm `file` specs, which npm >= 11.14 gates by `allow-file` (default `all`), never `allow-remote` — verified by the real npm 12 vendored matrix. **npm vendored `allow-file` contract (#969)**: `allow-file=none` refuses every `file:` dependency, and any other non-`all` value (`root`) admits one only when the project root or a workspace declares it and resolves to that very lock node (arborist's `_isRoot`); otherwise every `npm ci` / `npm install` fails EALLOWFILE. The effective value is read from the same npm config layers as `allow-remote` (an `npm_config_allow_file` environment variable, then the project `.npmrc`, then the user / global / builtin config files). When it refuses a `package-lock.json` / `npm-shrinkwrap.json` instance of a vendored `name@version` (the set vendoring rewires), the vendored run (`vendor`, `scan` / `get --mode vendored`, dry runs and in-sync re-runs included) still vendors, leaves the setting untouched, and records the `vendor_npm_allow_file` advisory naming the setting's source, the refused lock entries and the remedy (`allow-file=all` in the project `.npmrc`, unsetting the env var, or `npm ci --allow-file=all`); `vendor --check` fails that entry `vendor_check_failed` with the same reason. Other npm-family flavors (yarn, pnpm, bun, vlt) do not read `allow-file` and are not checked.
 

crates/socket-patch-cli/src/commands/scan/hosted.rs

@@ -949,11 +949,18 @@ async fn run_redirect_selected_untracked(
     // workspace / `lockfile-dir`, cargo workspace): the rewriters would
     // read only the member, so refuse before any takeover or write. The
     // same goes for a pnpm member whose `trustLockfile` setting lives in
-    // the workspace root's pnpm-workspace.yaml.
+    // 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));
     if let Some(refusal) = socket_patch_core::hosted::governing_root::refusal(
         &view,
         &candidates,
-        !common.no_trust_lockfile_config,
+        !common.no_trust_lockfile_config && !trust_env_off,
     )
     .await
     {
@@ -1297,6 +1304,9 @@ async fn run_redirect_selected_untracked(
     // parent directory's and `$CARGO_HOME`'s): a directory source or local
     // registry in any of them refuses the cargo rewrite like a project one.
     let cargo_outer = || CargoCrawler::crates_io_source_for(&common.cwd);
+    // The scan runs in the install's environment: a pnpm setting there
+    // (`pnpm_config_trust_lockfile`) overrides the project's files.
+    let pnpm_env = |var: &str| std::env::var(var).ok();
     let rewrite_options = || {
         RewriteOptions {
         dry_run: common.dry_run,
@@ -1311,6 +1321,7 @@ async fn run_redirect_selected_untracked(
         npm_outer: &npm_outer,
         yarn_classic_outer: &yarn_classic_outer,
         cargo_outer: &cargo_outer,
+        pnpm_env: &pnpm_env,
         blocking: true,
         takeover_uuids: takeover_uuids.clone(),
         patch_server_origins: patch_server_origins.clone(),

crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs

@@ -1350,6 +1350,84 @@ async fn pnpm11_no_trust_config_opt_out_frozen_install_needs_manual_trust_lockfi
     assert_fixture_manifestless_vex(&fx, &fresh, "pnpm11 --trust-lockfile");
 }
 
+/// #1514: pnpm 11+ read `pnpm_config_trust_lockfile` ahead of
+/// pnpm-workspace.yaml, so a scan whose environment sets it to `false` must
+/// not write a `trustLockfile: true` that installs there ignore, nor claim
+/// they need no extra flags. Over a lock the real pnpm 11 scan pinned
+/// without the trust config, a re-run in that environment (the heal path,
+/// which would otherwise write the key) writes no workspace file and names
+/// the variable in the trust warning.
+#[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_trust_lockfile_env_override_is_respected() {
+    let Some(fx) = redirect_scanned_pnpm_project(
+        PNPM_TERTIARY,
+        "pnpm11-trust-env",
+        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 lock_path = fx.proj.join(&fx.lock_name);
+    let lock_wired = std::fs::read_to_string(&lock_path).unwrap();
+
+    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(),
+        "false".to_string(),
+    )];
+    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);
+    assert!(
+        !ws_path.exists(),
+        "a trustLockfile key the environment overrides must not be written:\n{envelope}"
+    );
+    assert_eq!(std::fs::read_to_string(&lock_path).unwrap(), lock_wired);
+    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("`pnpm_config_trust_lockfile=false`") && !detail.contains("no extra flags"),
+        "{detail}"
+    );
+
+    // Control: the same re-run without the variable heals the config.
+    let (code, stdout, stderr) = run_socket_env(&fx.proj, &args, &[]);
+    assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}");
+    let workspace = std::fs::read_to_string(&ws_path).unwrap_or_default();
+    assert!(workspace.contains("trustLockfile: true"), "{workspace}");
+}
+
 /// 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