Fix hosted npm ignoring omit-lockfile-registry-resolved (#1871) by mikolalysenko · Pull Request #1872 · SocketDev/socket-patch

Mikola Lysenko (mikolalysenko) · 2026-10-30T01:23:27Z

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

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

Assisted-by: Claude Code:claude-opus-5-5

The hosted npm run already reads replace-registry-host from every npm config layer (env, project, user, global, builtin). Read omit-lockfile-registry-resolved the same way, through one shared effective-setting lookup instead of a second copy, and parse its value as npm's ini + nopt Boolean reading does (a bare key, `yes` or `False` are true; `false`, `null` and numeric zero are not). Assisted-by: Claude Code:claude-opus-5-5

With omit-lockfile-registry-resolved=true, the next npm install (even a no-op) drops the hosted `resolved` URL but keeps the patched integrity, so every cold-cache install fails EINTEGRITY and vex loses the patch. The hosted scan said nothing and exited 0. It now warns redirect_npm_omit_lockfile_registry_resolved, naming the config layer that sets it and the remedies (set it to false in the project .npmrc, unset the env var, or use vendored mode). Documented in CLI_CONTRACT.md, docs/ecosystems.md and the npm test matrix. Fixes #1871 Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-30T02:14:50Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

Code review, checked against npm 10.9.4: npm reads a blank omit-lockfile-registry-resolved= as true and a literal `undefined` as false, because it checks those keywords on the written value before expanding ${VAR}. The scan read them the other way round, so a blank value got no warning and `undefined` got a false one. Also share the where-is-it-set remedy text between the replace-registry-host and omit-lockfile-registry-resolved warnings, replace the full JS number parser with a numeric-zero check, and restore the #1233 test comment the new test had split. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-30T02:30:51Z

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) · 2026-10-30T04:10:09Z

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) · 2026-10-30T05:29:37Z

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

Start fix for #1871

a3790b8

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 30, 2026

Hosted npm scan doesn't warn about omit-lockfile-registry-resolved=true: the next plain npm install strips the hosted resolved URL but keeps the patched integrity, so every fresh npm ci then fails EINTEGRITY #1871

Open

Claude (claude) added 2 commits October 30, 2026 01:40

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 30, 2026 02:14

cursor Bot reviewed Oct 30, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 30, 2026

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 30, 2026

Swarm health manager log #1382

Open

Mikola Lysenko (mikolalysenko) enabled auto-merge October 30, 2026 05:29

Navigation Menu

Repository navigation

Fix hosted npm ignoring omit-lockfile-registry-resolved (#1871) - #1872

Fix hosted npm ignoring omit-lockfile-registry-resolved (#1871)#1872
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
mainfrom
agent/fix-npm-omit-lockfile-resolved

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026 •

edited

Loading

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

📋 Server Request & Proxy Logs

⚙️ Pengaturan Proxy Target

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Summary

Root cause

Changes

Code review (/code-review high)

Test evidence

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026

Final review brief: 161b4e4

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Mikola Lysenko (mikolalysenko) commented Oct 30, 2026 •

edited

Loading

Code review (`/code-review high`)

cursor Bot left a comment •

edited

Loading

Final review brief: `161b4e4`