Skip to content

Fix npm file: copy attested by in-run VEX (#1481) - #1482

Merged
Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
agent/fix-npm-file-copy-vex
Oct 13, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
agent/fix-npm-file-copy-vex

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

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

Fixes #1481

What users saw

Take an npm workspace where member a depends on registry
left-pad@1.3.0 and member b depends on a file: copy of the same
release, either a packed tarball or a source directory.
scan --mode hosted --vex pinned the registry entry and its in-run VEX
attested left-pad@1.3.0 not_affected. But npm installs b's copy
from its spec, so that copy stays unpatched. The tarball shape at least
warned. The directory shape warned about nothing. A standalone vex after
npm ci correctly refused both.

Root cause

The npm lock rewriter (patch/redirect/mod.rs) already handles copies it
can't redirect by recording the uuid in bundled_skipped_uuids, which
keeps the in-run VEX from assuming the patch applied. The bundled and
hasShrinkwrap branches do that. Two shapes did not:

  • file: tarball: the non_registry branch warned
    redirect_npm_non_registry_entry_skipped but never recorded the uuid.
    Before Fix open npm issues #1008 the engine's attribution pass vetoed the pin. Since Fix open npm issues #1008
    it lets a shadowed pin through, so nothing stopped the attestation.
  • file: directory: npm locks it as a versionless link entry
    plus the source directory's own entry, which carries the name and
    version. The rewriter matches on name@version and never saw the copy.

#1469 fixes the pnpm counterpart (#935) in the engine, but only for pnpm
locks. This PR fixes npm in the npm rewriter, so the two PRs don't
overlap.

Fix

  • The non_registry branch records the uuid, like its bundled and
    shrinkwrap siblings.
  • A versionless link entry that npm reached through a file: spec
    takes the identity of its source directory's entry. That entry's
    name, else its folder name with any @scope/ parent kept, as npm's
    nameFromFolder does. The existing link branch then warns
    redirect_npm_link_entry_skipped (its detail now says the copy stays
    UNPATCHED) and records the uuid.
  • A plain workspace-member link is first-party source, not a copy, so it
    is left alone. It still reports redirect_npm_entry_not_found as
    before.
  • The registry copy is still pinned. That keeps the existing npm and yarn
    classic policy: patch what can be patched, warn about the rest, and keep
    the in-run VEX out of it.

Tests (red → green)

Issue / shape Test Without fix With fix
#1481 tarball in_process_redirect::scan_redirect_npm_file_copy_is_not_attested_in_run FAILED: tarball: in-run VEX must not attest … ok
#1481 directory same test, directory leg (run with only the link hunks reverted) FAILED: directory: in-run VEX must not attest … ok
#1481 both (rewriter) redirect::tests::issue_1481_npm_file_copy_beside_a_pin_is_warned_and_recorded FAILED (tarball: uuid not recorded; directory: warns redirect_npm_link_entry_skipped: []) ok
scoped dir / workspace member redirect::tests::issue_1481_npm_link_identity_scoped_folder_and_workspace_member new ok

The lock shapes in the tests are copied from a real npm 10.9.4
install-strategy=nested workspace install of the issue's repro.

tests/equivalence/npm_lock_rewrite.golden is re-blessed. Only the
output digests change, for the seeded cases with link or non-registry
entries, which now carry the recorded uuid and the new detail text. The
inputs are unchanged.

Run locally:

  • cargo fmt --all -- --check: ok
  • cargo clippy --workspace --all-features -- -D warnings: ok
  • cargo test -p socket-patch-core --all-features --lib: 6420 passed.
    5 failed before the re-bless: the golden, plus 4 read-only-directory
    tests that can't fail when run as root in this container (copy_tree,
    vlt_heal, poetry/requirements write failure). They are unrelated to
    this diff.
  • cargo test -p socket-patch-cli --all-features --test in_process_redirect --test e2e_vex_redirect --test in_process_redirect_pnpm:
    all pass except the same class of 3 root-only write-failure tests.
  • A full cargo test --workspace ran out of this container's disk
    allowance, so CI is the record for the rest.
  • e2e_redirect_npm_build -- --ignored (real npm 10.9.4, npm required):
    6 passed.

Review notes

/code-review high findings and what happened to each:

  • The stale npm_link_entry_versioned_…_generic_not_found assertion and
    the workspace-member false positive: fixed by limiting link matching to
    links reached through a file: spec. The test still holds as written.
  • Golden digests: re-blessed (see above).
  • Scoped folder names: fixed, with a test.
  • Not fixed: requiring the in-run VEX file to exist. When no statement
    survives, no VEX document is written, so that assertion would fail on
    correct behavior. The test keeps the same tolerant read as the existing
    bundled-copy test, and the registry-pin assertion proves the run
    reached the rewrite.
  • Not fixed: bundled_skipped_uuids also exempts a uuid from the
    engine's contested-wiring veto.
    Since Fix open npm issues #1008 these shapes reach the
    engine as shadowed, not contested, so the pin was already written. This
    only adds the warning and the VEX demotion. Bundled and shrinkwrap
    copies already behave this way.
  • Follow-up, not fixed here: link identity in shared code. Lockfile
    discovery (vex/discover/npm.rs) and the vendored npm backend still
    skip a versionless file: directory link. A standalone vex doesn't
    attest the issue's directory shape (it reports not_applied), so this
    PR keeps the fix in the rewriter. Moving link identity into the shared
    npm_lock_entries walk would touch every consumer, so it should be its
    own change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RFYJv8oyp5Fz9ySBaorZcd


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A hosted npm scan with --vex attested a package not_affected when the
same lock also installed that release from a file: tarball or a file:
directory. npm installs those copies from their spec, so they stay
unpatched, yet the run's own VEX said the app was fixed.

The npm lock rewriter now records the patch for both shapes so the
in-run VEX verifies the install instead of assuming the patch applied.
A tarball copy already warned; a directory copy (a versionless link
entry) is now matched through its source directory's entry and warns
redirect_npm_link_entry_skipped. The registry copy is still pinned.

Fixes #1481

Assisted-by: Claude Code:claude-opus-5-5
Code review follow-ups. The link identity fill-in now applies only to a
link npm reached through a file: spec, so a workspace member that
happens to share a patched package's name and version is no longer
reported as an unpatched copy. A source folder with no name field keeps
its @scope/ parent, matching npm, so scoped file: directories are
caught too. The npm rewrite golden is re-blessed: only its output
digests change, for the seeded link and non-registry cases that now
record the patch for the in-run VEX.

Refs #1481

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 13, 2026 14:16
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 868fe6f. Configure here.

Merged via the queue into main with commit 0d4c930 Oct 13, 2026
86 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-npm-file-copy-vex branch October 13, 2026 14:59
Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Oct 13, 2026
Covers #1469, #1470, #1474, #1475, #1478, #1479, #1480 and #1482; #1484 is
CI-only. The notes now cover main through 285a5e8.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
github-merge-queue Bot pushed a commit that referenced this pull request Oct 13, 2026
* ci(release): publish npm from release.yml, not publish-npm.yml

Every socket-patch npm package's trusted publisher is registered against
release.yml, and npm allows only one publisher per package. The 5.0.0
release dispatched publish-npm.yml, whose OIDC token named that file, so
npm rejected it with E401 before staging anything.

Move the npm leg back inline as release.yml's npm-publish job (same steps
as publish-npm.yml) and delete publish-npm.yml, so the existing
registration keeps working. Add an `npm-retry-version` input that skips
the version gate, build, tag, GitHub release and crates.io jobs and
re-stages npm from an existing release's assets; this retries 5.0.0 now
and replaces the manual Publish npm dispatch. crates.io stays dispatched
through publish-cargo.yml, since crates.io allows several publishers per
crate.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore(release): fold 22 more PRs into the 5.0.0 notes

Cover the PRs merged to main after #1432's base (1779317..4cb3643):
Cargo source replacement and registry = "crates-io" (#338/#455, #386),
pnpm lockfile=false (#1074), berry removed descriptors (#1435), yarnrc
${VAR} reads (#1100), CRLF .npmrc append (#1196), Composer vendor-dir
cascade and global home (#439/#686, #586), Go +incompatible vex (#484),
gem moved BUNDLE_GEMFILE and commented Gemfile revert (#952, #988),
Poetry source = "pypi" (#1282) and Hatch path characters (#547).

No entries for #1411 (macOS fix to code #1393 added in this release;
the #1393 entry already describes the intended behavior), #1426 (perf
regression introduced and fixed within 5.0.0) or the CI/test/bench PRs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore(release): cover #1419, #1434, #1440, #1442, #1444 in the 5.0.0 notes

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore(release): 5.0.1

Cut [Unreleased] into [5.0.1] — 2026-10-13 and stamp 5.0.1 with
scripts/version-sync.sh. The 5.0.1 notes cover main through 501c6d9
(#1449), i.e. every PR after the v5.0.0 tag (6ad6add): the npm
publish fix (5.0.0 never reached npm or crates.io, 5.0.1 is the first
v5 there), #1465, #759, #1460, #1462, #1445, #869, #1447, #1450,
#1467, and #1449 under Maintenance. CI/test-only PRs (#1448, #1452,
#1454, #1457, #1459) have no entries.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore(release): 5.0.1 notes for PRs merged since the cut

Covers #1469, #1470, #1474, #1475, #1478, #1479, #1480 and #1482; #1484 is
CI-only. The notes now cover main through 285a5e8.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 14, 2026
Vendored `scan --vex`, a standalone `vex` and `vendor --check` read the
npm lock through discovery, which skipped a `file:` directory
dependency: npm records it as a link entry with no version, so
discovery never saw that it holds the patched release. VEX then said
`not_affected` while that workspace member loaded the unpatched
directory.

Discovery now resolves such a link to its source directory's entry,
as the hosted rewriter does since #1482, and withholds the reference
with `patched_ref_unattributable`. The lookup moves to
vendor::npm_origin so both readers share it.

Fixes #1511

Assisted-by: Claude Code:claude-opus-5-5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants