[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Fixes #1871

Summary

A hosted npm scan now warns redirect_npm_omit_lockfile_registry_resolved
when npm's effective omit-lockfile-registry-resolved is true. Before
this change the scan said nothing and exited 0. The first later
npm install (even a no-op) then dropped the hosted resolved URL but
kept the patched integrity, so cold-cache installs failed EINTEGRITY
and vex stopped finding the patch.

Root cause

The hosted run already reads replace-registry-host from every npm
config layer (#812) to warn when npm will undo its pins, but it never
read omit-lockfile-registry-resolved, which undoes them on the next
lock save in the same way.

Changes

• patch/redirect/npmrc.rs: OuterAllowRemote also resolves
  omit-lockfile-registry-resolved from the env / user / global /
  builtin layers. The env > project > outer lookup now goes through one
  private effective_setting. effective_replace_registry_host keeps
  its signature and uses it too.
• npm_config_bool: reads the value the way npm does
  (@npmcli/config parseField, then nopt's Boolean check; measured on
  npm 10.9.4 through c.flat). A bare key, a blank value, yes and
  False are true. false / null / undefined as written are false,
  and so is a numeric zero after ${VAR} expansion.
• hosted/engine.rs + hosted/guidance.rs: the new warning names
  the layer that sets the key (env var, project .npmrc, or a config
  path) and the remedies: set it to false in the project .npmrc,
  unset the env var, or use vendored mode. It also says a hosted re-run
  re-pins a stripped lock. Same trigger as the replace-registry-host
  warning, so an "already hosted" re-run warns too. Exit status is
  unchanged, and nothing is written to the user's config.
• Docs: CLI_CONTRACT.md (contract paragraph + warning-code row),
  docs/ecosystems.md, docs/testing/npm-compatibility.md.
• Wrappers (npm/, pypi/, gem/) only dispatch to the binary and
  name no warning codes, so they need no change.

Code review (/code-review high)

Fixed in 161b4e4:

• A blank value was read as false and undefined as true. npm 10.9.4
  reads them the other way round. Checked with @npmcli/config.
• The new test had split the The #812 replace-registry-host warning misses a ${VAR} value in .npmrc: npm expands it to always, so every npm ci fails E404, while the hosted scan reports success with no warning #1233 test's doc comment.
• The env / project / file remedy text was duplicated; both warnings now
  use one npm_setting_remedy helper (replace-registry-host wording
  unchanged).
• The full JS Number() parser was more than needed; it is now a
  numeric-zero check.

Not fixed, with reasons:

• Several env spellings of the key set to different values, where a
  0 spelling hides a true one. This only happens if both
  NPM_CONFIG_… and npm_config_… are set to different values, which
  is far-fetched. The tie-break is shared with every other key
  npm_config reads. Changing it to a predicate would touch the
  allow-remote / allow-file paths outside this fix.
• OuterAllowRemote keeps collecting one field per npm key; make it a
  general key table. That is a design refactor across the
  allow-remote, replace-registry-host and allow-file paths, so it is
  out of scope for this bug fix. Worth an arch-audit issue if wanted.

Test evidence

Issue	Test	Red → green
#1871	vendor suite: redirect_npm_allow_remote::omit_lockfile_registry_resolved_is_warned (project file on first run + re-run, human stderr, bare key / 1 / blank / false / null / undefined / 0 / section-scoped, user config, project false over user true, env over project)	Fails with the engine hook disabled (run 0: no redirect_npm_omit_lockfile_registry_resolved), passes with it
#1871	core unit: npmrc::tests::omit_lockfile_registry_resolved_layers, npmrc::tests::npm_config_bool_matches_npm	New API; passes

Local runs:

• cargo fmt --all -- --check: ok
• cargo clippy --workspace --all-features -- -D warnings: ok
• cargo test -p socket-patch-cli --all-features --test vendor -- redirect_npm_allow_remote: 12 passed
• cargo test --workspace --all-features --no-fail-fast: 15,334 passed, 11 failed. The 11 fail only because of this sandbox and none touch npm config: 9 tests use chmod to force write failures, and they can't fail as root (in_process_redirect x3, repair x2, copy_tree / vlt_heal / pypi_poetry / pypi_requirements unit tests). The other 2 are e2e_bun_lockb tests that need https://patches-api.socket.dev, which the sandbox proxy blocks, plus the locally installed bun. CI is the check for these.

https://claude.ai/code/session_011f9aeNu3D6QeUEGR4pgeCN

---

Note

Medium Risk
Adds npm hosted-mode advisory logic and Boolean config parsing; behavior is warn-only (no config writes or exit-code changes) but affects operator guidance for lock integrity.

Overview
Hosted npm scans now emit redirect_npm_omit_lockfile_registry_resolved when npm’s effective omit-lockfile-registry-resolved is true—the same trigger as the existing replace-registry-host advisory, including idempotent “already hosted” re-runs. Previously the run stayed silent while the next npm install could drop hosted resolved URLs and leave patched integrity, causing EINTEGRITY and breaking vex.

Resolution follows npm’s config layers (env → project .npmrc → user/global/builtin) via shared effective_setting, with npm_config_bool matching npm’s Boolean parsing. The CLI never rewrites that setting; warnings name the source layer and point to omit-lockfile-registry-resolved=false, unsetting the env var, or vendored mode. Warning copy for replace-registry-host is refactored through shared npm_setting_spelling / npm_setting_remedy helpers. Docs and integration/unit tests cover the contract and layer precedence.

^{Reviewed by Cursor Bugbot for commit 161b4e4. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 161b4e4. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at 161b4e4 (burn-down agent).

• CI: 50/50 checks green on the head (31 success, 19 skipped, no failures). No main-wide failures.
• Bugbot: Cursor Bugbot check passed on this head; no open review threads.
• Mergeable with no conflicts; no CHANGELOG.md change.

Slack announcement not sent this run (the Slack send tool was unavailable); a later run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief: 161b4e4

What it does. A hosted npm scan now warns redirect_npm_omit_lockfile_registry_resolved when npm will treat omit-lockfile-registry-resolved as true. That setting makes npm strip the hosted pin from the lockfile (#1871). The scan reads the setting from the same layers the existing replace-registry-host warning uses (env, then project, user, global and builtin config), and the warning names the layer that sets it and the remedies. It only adds a warning: it writes no config and doesn't change the exit code.

Risk: low. The change is warning-only and fires on the same trigger as the #812 warning. The replace-registry-host code was moved onto a shared effective_setting and shared remedy helpers. I compared its old and new text, and the #812/#1233 tests pass.

Look here

• npmrc.rs:679 and :723: effective_setting (env > project > file) and the omit-lockfile lookup
• npmrc.rs:743: npm_config_bool, which copies npm's parseField and nopt Boolean reading
• engine.rs:3184: where the warning is emitted
• guidance.rs:598: npm_setting_remedy and the detail text
• redirect_npm_allow_remote.rs:1032:`` the integration test

Verified

• npmrc::tests: 17/17 passed.
• vendor suite, filter redirect_npm_allow_remote: 12/12 passed. Against a binary built without the change, the new test fails.
• I checked the value reading against real npm 10.9.4 (@npmcli/config flat.omitLockfileRegistryResolved) and every case matched:
  • project undefined, 0 or null over user true → false
  • a blank value, False or env FALSE → true
  • env 0 over project true → false
  • a [section]-scoped key → false
  • key[]=false → true
• CRLF and the bare key are covered by the tests.
• CI: ci-ok and clippy are green. Cursor Bugbot is clean, there are no open threads, and CHANGELOG.md is untouched.

Open questions (non-blocking nits)

• npm_config_bool is pub but is only used inside its module, so it could be pub(crate).
• If two env spellings are set to different values, the one that isn't the default wins. The PR documents why it doesn't fix this.

Auto-merge is armed, so approving sends this PR straight to the merge queue.

---

Generated by Claude Code

[Wenxin Jiang (Wenxin-Jiang)]

[agent] Reviewed 161b4e41655baf4eb09298f036c1a0cc0eef6389. Checked effective npm configuration precedence and boolean coercion, warning emission and remedy selection for omit-lockfile-registry-resolved. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.