Skip to content

Commit a3f56f7

Browse files
Fix pnpm trustLockfile env override ignored (#1514) (#1533)
* Start fix for #1514 Assisted-by: Claude Code:claude-opus-5-5 * Respect a pnpm trustLockfile env override pnpm 11 and 12 read pnpm_config_trust_lockfile (or its uppercase spelling) ahead of pnpm-workspace.yaml. With it set to false, a hosted scan still wrote trustLockfile: true there and said installs need no extra flags, so every install in that environment failed with ERR_PNPM_TARBALL_URL_MISMATCH. The scan now treats a non-true value of that variable as an explicit user setting, like trustLockfile: false in the file: on a project whose pnpm is 11 or newer (or unknown) it writes nothing and the trust warning names the variable and the remedies (unset it or set it to true for installs, or use --mode vendored). The in-memory engine sees no environment and is unchanged. Fixes #1514 Assisted-by: Claude Code:claude-opus-5-5 * Document the pnpm trustLockfile env override Assisted-by: Claude Code:claude-opus-5-5 * Share pnpm major lookup; keep specific guidance Review follow-ups for the trustLockfile env override: - The trust check now reads the project's pnpm major through the same helper as the lockfile setting (#1460) instead of a second copy, and the env reader reuses the existing variable lookup. - A project pinned to pnpm 9.0-10.4, or with a keyless workspace file that lists members, keeps its more specific guidance; the override only replaces the generic write. - A workspace member scanned with the override set is no longer refused with advice to add trustLockfile: true to the root file, which the variable would override anyway; the scan pins the member lock and its warning names the variable. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b633045 commit a3f56f7

7 files changed

Lines changed: 355 additions & 31 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ Beyond the globals above, each subcommand defines a small set of local arguments
157157

158158
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.
159159

160-
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.
160+
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.
161161

162162
**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.
163163

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -949,11 +949,18 @@ async fn run_redirect_selected_untracked(
949949
// workspace / `lockfile-dir`, cargo workspace): the rewriters would
950950
// read only the member, so refuse before any takeover or write. The
951951
// same goes for a pnpm member whose `trustLockfile` setting lives in
952-
// the workspace root's pnpm-workspace.yaml.
952+
// the workspace root's pnpm-workspace.yaml, unless a
953+
// `pnpm_config_trust_lockfile` in this environment overrides that file
954+
// (#1514): adding the key there would not help, and the rewrite's
955+
// trust warning names the variable instead.
956+
let trust_env_off = socket_patch_core::utils::pnpm_workspace::trust_lockfile_env(None, |var| {
957+
std::env::var(var).ok()
958+
})
959+
.is_some_and(|found| found.as_bool() != Some(true));
953960
if let Some(refusal) = socket_patch_core::hosted::governing_root::refusal(
954961
&view,
955962
&candidates,
956-
!common.no_trust_lockfile_config,
963+
!common.no_trust_lockfile_config && !trust_env_off,
957964
)
958965
.await
959966
{
@@ -1297,6 +1304,9 @@ async fn run_redirect_selected_untracked(
12971304
// parent directory's and `$CARGO_HOME`'s): a directory source or local
12981305
// registry in any of them refuses the cargo rewrite like a project one.
12991306
let cargo_outer = || CargoCrawler::crates_io_source_for(&common.cwd);
1307+
// The scan runs in the install's environment: a pnpm setting there
1308+
// (`pnpm_config_trust_lockfile`) overrides the project's files.
1309+
let pnpm_env = |var: &str| std::env::var(var).ok();
13001310
let rewrite_options = || {
13011311
RewriteOptions {
13021312
dry_run: common.dry_run,
@@ -1311,6 +1321,7 @@ async fn run_redirect_selected_untracked(
13111321
npm_outer: &npm_outer,
13121322
yarn_classic_outer: &yarn_classic_outer,
13131323
cargo_outer: &cargo_outer,
1324+
pnpm_env: &pnpm_env,
13141325
blocking: true,
13151326
takeover_uuids: takeover_uuids.clone(),
13161327
patch_server_origins: patch_server_origins.clone(),

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

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1350,6 +1350,84 @@ async fn pnpm11_no_trust_config_opt_out_frozen_install_needs_manual_trust_lockfi
13501350
assert_fixture_manifestless_vex(&fx, &fresh, "pnpm11 --trust-lockfile");
13511351
}
13521352

1353+
/// #1514: pnpm 11+ read `pnpm_config_trust_lockfile` ahead of
1354+
/// pnpm-workspace.yaml, so a scan whose environment sets it to `false` must
1355+
/// not write a `trustLockfile: true` that installs there ignore, nor claim
1356+
/// they need no extra flags. Over a lock the real pnpm 11 scan pinned
1357+
/// without the trust config, a re-run in that environment (the heal path,
1358+
/// which would otherwise write the key) writes no workspace file and names
1359+
/// the variable in the trust warning.
1360+
#[tokio::test(flavor = "multi_thread")]
1361+
#[serial_test::serial]
1362+
#[ignore = "wall-bound real-pnpm install (~60s); runs on all 3 OSes as an e2e CI matrix leg"]
1363+
async fn pnpm11_trust_lockfile_env_override_is_respected() {
1364+
let Some(fx) = redirect_scanned_pnpm_project(
1365+
PNPM_TERTIARY,
1366+
"pnpm11-trust-env",
1367+
false,
1368+
true,
1369+
HostedDriver::Scan,
1370+
)
1371+
.await
1372+
else {
1373+
return;
1374+
};
1375+
let ws_path = fx.proj.join("pnpm-workspace.yaml");
1376+
assert!(
1377+
!ws_path.exists(),
1378+
"the opted-out fixture scan wrote no workspace file"
1379+
);
1380+
let lock_path = fx.proj.join(&fx.lock_name);
1381+
let lock_wired = std::fs::read_to_string(&lock_path).unwrap();
1382+
1383+
let api_url = fx._server.uri();
1384+
let args = [
1385+
"scan",
1386+
"--mode",
1387+
"hosted",
1388+
"--json",
1389+
"--yes",
1390+
"--cwd",
1391+
fx.proj.to_str().unwrap(),
1392+
"--api-url",
1393+
&api_url,
1394+
"--org",
1395+
ORG,
1396+
"--api-token",
1397+
"fake",
1398+
];
1399+
let env = [(
1400+
"pnpm_config_trust_lockfile".to_string(),
1401+
"false".to_string(),
1402+
)];
1403+
let (code, stdout, stderr) = run_socket_env(&fx.proj, &args, &env);
1404+
assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}");
1405+
let envelope = parse_envelope(&stdout);
1406+
assert!(
1407+
!ws_path.exists(),
1408+
"a trustLockfile key the environment overrides must not be written:\n{envelope}"
1409+
);
1410+
assert_eq!(std::fs::read_to_string(&lock_path).unwrap(), lock_wired);
1411+
let detail = envelope["warnings"]
1412+
.as_array()
1413+
.into_iter()
1414+
.flatten()
1415+
.find(|w| w["code"] == "redirect_pnpm_trust_lockfile")
1416+
.and_then(|w| w["detail"].as_str())
1417+
.unwrap_or_else(|| panic!("redirect_pnpm_trust_lockfile:\n{envelope}"))
1418+
.to_string();
1419+
assert!(
1420+
detail.contains("`pnpm_config_trust_lockfile=false`") && !detail.contains("no extra flags"),
1421+
"{detail}"
1422+
);
1423+
1424+
// Control: the same re-run without the variable heals the config.
1425+
let (code, stdout, stderr) = run_socket_env(&fx.proj, &args, &[]);
1426+
assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}");
1427+
let workspace = std::fs::read_to_string(&ws_path).unwrap_or_default();
1428+
assert!(workspace.contains("trustLockfile: true"), "{workspace}");
1429+
}
1430+
13531431
/// Legacy pnpm@7 leg: the fixture install emits a lockfileVersion 5.4 lock
13541432
/// (`/name/version:` path-style key), the scan splices its resolution like
13551433
/// any other grammar, and the fresh dead-registry frozen install proves

0 commit comments

Comments
 (0)