GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||
GeezerVison is showing github.com — Fix npm file: copy attested by in-run VEX (#1481) by mikolalysenko · Pull Request #1482 · SocketDev/socket-patch Conversation
|
| 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: okcargo clippy --workspace --all-features -- -D warnings: okcargo test -p socket-patch-core --all-features --lib: 6420 passed.cargo test -p socket-patch-cli --all-features --test in_process_redirect --test e2e_vex_redirect --test in_process_redirect_pnpm:cargo test --workspace ran out of this container's diske2e_redirect_npm_build -- --ignored (real npm 10.9.4, npm required):/code-review high findings and what happened to each:
npm_link_entry_versioned_…_generic_not_found assertion andfile: spec. The test still holds as written.bundled_skipped_uuids also exempts a uuid from thevex/discover/npm.rs) and the vendored npm backend stillfile: directory link. A standalone vex doesn'tnot_applied), so thisnpm_lock_entries walk would touch every consumer, so it should be its🤖 Generated with Claude Code
https://claude.ai/code/session_01RFYJv8oyp5Fz9ySBaorZcd
Generated by Claude Code
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
* 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) <[email protected]> * 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) <[email protected]> * chore(release): cover #1419, #1434, #1440, #1442, #1444 in the 5.0.0 notes Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * 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) <[email protected]> * 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) <[email protected]> --------- Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
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
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Hosted npmscan --vex attests not_affected when the lock also installs the same name@version from a file: tarball or directory, which npm installs unpatched (tarball shape regressed in #1008)
There was an error while loading. Please reload this page.