Skip to content

Fix in-run hosted VEX attesting npm bundled copies (#325) - #669

Open
Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
mainfrom
agent/fix-npm-hosted-bundled-assume-applied
Open

Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
mainfrom
agent/fix-npm-hosted-bundled-assume-applied

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Fixes #325

Summary

In hosted mode, scan --vex no longer attests an npm patch as not_affected when package-lock.json also contains a bundled copy of the same name@version. The bundled copy is inBundle: true, or bundled: true in a v1 lock. npm unpacks that copy from its parent's tarball, so hosted mode can't redirect it and it stays unpatched.

Root cause

scan --mode hosted --vex attests the patches this run redirected without checking hashes (assume_applied). It leaves out a patch whose uuid the rewriter put in RewriteResult::bundled_skipped_uuids, meaning some copy couldn't be redirected (#469).

The Bun rewriters record that uuid. The npm package-lock.json rewriter skips the bundled entry and warns redirect_npm_bundled_instance_skipped, but never records the uuid. So the hoisted copy gets redirected, and the in-run VEX attests the purl while the same run warns that the bundled copy stays unpatched.

Fix

  • crates/socket-patch-core/src/patch/redirect/mod.rs: record dep.patch_uuid in bundled_skipped_uuids for the tree npm actually installs from:

    • matching packages inBundle entries;
    • legacy dependencies bundled entries, but only when there is no object-valued packages map.

    A stale legacy mirror keeps its skip and warning but no longer blocks an in-run attestation. The existing assume_applied filter in scan/hosted.rs then sends the purl through normal verification. That matches standalone vex and the vendored in-run path.

  • tests/equivalence/npm_lock_rewrite.golden: re-blessed. Input digests are unchanged; only output digests for cases with a bundled match in the install tree moved.

  • CHANGELOG entry under Unreleased / Fixed.

Review follow-up

Codex found that the first version also recorded the uuid from a stale v2 dependencies mirror while an object-valued packages map existed. npm 7+ ignores that mirror, so a correctly redirected package lost its in-run attestation. afd320e fixed this: the legacy uuid is now recorded only when there's no object-valued packages map, the same precedence npm lock discovery uses. It also adds npm_stale_legacy_bundled_mirror_does_not_contest_packages, covering direct and nested mirrors with and without a real bundle, plus an in-process VEX regression. I reviewed the diff and it's correct.

Tests (red → green)

Test Without fix With fix
npm_inbundle_entry_is_skipped_with_loud_warning (asserts uuid recorded) FAILED ok
npm_inbundle_skip_leaves_sibling_rewrite_intact (redirected sibling + bundled copy) FAILED ok
npm_legacy_bundled_dependency_is_skipped (v1 bundled) FAILED ok
in_process_redirect::scan_redirect_npm_bundled_copy_is_not_attested_in_run (lock v3 inBundle and lock v1 bundled, real in-run --vex) FAILED: inBundle: in-run VEX must not attest a purl whose bundled copy stays unpatched ok
npm_stale_legacy_bundled_mirror_does_not_contest_packages (v2 stale mirror) fails on 4b18db6 ok on afd320e

Per-issue checklist

Local validation

  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo fmt: main itself fails cargo fmt --all -- --check, and CI doesn't run it. My hunks are rustfmt-clean.
  • cargo test --workspace --all-features --no-fail-fast: everything passes except tests that rely on chmod-based write failures. This sandbox runs as root, which ignores those modes, and the same tests fail identically on origin/main.
  • e2e_redirect_npm_build -- --ignored (npm 10.9.4): the only failures were at the rollback step, which needs registry.npmjs.org from the Rust client and is blocked by the sandbox proxy.

Additional focused validation (reviewer, on afd320e)

  • 146 repository tests: 37 core npm/bundled/golden tests and 109 hosted CLI/VEX tests.
  • 800-case golden replay: input digests unchanged.
  • Native controls with npm 11.19 / Node 24.21 and npm 8.11 / Node 18.3.

CI

CI and Bugbot on afd320eb9819d9f30758f307ef6e641370d4c98f are clear: 482 successful checks, 7 skipped, and 12 successful workflows (1 additional workflows skipped). Bugbot is clear on this commit; no unresolved review threads or new actionable findings.

No wrapper changes are needed. The npm, pypi and gem wrappers only dispatch to the binary.

🤖 Generated with Claude Code

Assisted-by: Claude Code:claude-opus-5-5
When a package-lock.json also lists a bundled copy of the patched
package (inBundle, or the legacy v1 "bundled" flag), hosted mode
redirects the regular entry but can't reach the bundled one: npm
unpacks it from the parent's tarball. The run already warned that
copy stays unpatched, yet `scan --vex` still attested the patch as
not_affected.

The npm rewriter now records the patch as having a skipped bundled
copy, the same way the Bun rewriter does, so the in-run VEX verifies
it instead of assuming it applied, and leaves it out.

Fixes #325

Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Claude Code:claude-opus-5-5
The seeded npm lock sweep generates bundled entries, and its recorded
rewrite result now carries the patch uuids those entries skipped.
Input digests are unchanged; only the cases with a bundled match
moved.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 3, 2026 09:12
@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.

Stale Bugbot comment from a previous run.

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

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Labeled Ready for review at head 4b18db6dd2 (4b18db6).

  • CI: 488/488 check runs green on the current head; mergeable, no conflicts.
  • Bugbot: reviewed 4b18db6, no new issues; no unresolved review threads.
  • Reviewer focus: the npm_lock_rewrite.golden re-bless (63 lines) reflects bundled copies now being recorded in bundled_skipped_uuids, so in-run --vex no longer attests them.

Generated by Claude Code

@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator Author

Codex follow-up review of afd320eb9819d9f30758f307ef6e641370d4c98f: the confirmed P2 regression is fixed; ready to merge as-is from this review.

The legacy bundled UUID is now recorded only when no object-valued packages map exists, matching npm lock discovery. The same decision is passed through nested legacy dependencies. Genuine inBundle copies and v1 bundles still prevent unsafe attestations, while an ignored v2 mirror cannot block a correct hosted attestation.

Validation on the exact committed source:

  • 146 repository tests passed: 37 core npm/bundled/golden tests and 109 hosted CLI/VEX tests. New permanent regressions cover direct/nested mirrors with and without a real bundle, plus valid VEX output for both npm lockfile names.
  • Two additional artifact tests passed: an 800-case golden oracle and a three-dialect mixed-target control that omits the bundled target while attesting the unaffected one. All 200 golden input digests are unchanged. The 54 changed output records versus main (10 versus the original PR head) differ only in the bundled-UUID metadata.
  • Native npm controls passed: real v1/v2/v3 bundles remain omitted, different-name/version controls attest, and the stale-mirror case now succeeds. With pinned npm 11.19 / Node 24.21, the original CLI failed that VEX step and the corrected CLI succeeds; both rewritten locks install patched bytes with npm ci, and standalone VEX succeeds. An additional npm 8.11 / Node 18.3 install control passes too.
  • Independent review matched all four committed file hashes. Production Clippy passed with -D warnings and only the pre-existing macOS unused_variables allowance. The patch and merge with main 045d7ec7 are clean.

482 successful checks, 7 skipped, and 12 successful workflows (1 additional workflows skipped). Bugbot is clear on this commit; no unresolved review threads or new actionable findings. Ready for review has been restored.

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

Copy link
Copy Markdown
Collaborator Author

Cursor (@cursor) review

@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 afd320e. Configure here.

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

npm VEX attests not_affected while a bundled (inBundle) copy of the same package@version stays unpatched

2 participants