Repository navigation
Stop attesting npm-family pins pnpm or a user patch overrides - #2010
Draft
Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
Draft
Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
Conversation
VEX discovery attested two kinds of npm-family pins that the hosted and vendored writers refuse, so the statement could cover bytes the install never uses. N1, pnpm gitBranchLockfile (#556): with the setting on and a pnpm-lock.<branch>.yaml present, pnpm installs the branch from that lock. Hosted withholds the root and member locks and refuses every pnpm pin; vendored refuses the project. Discovery now runs the same detection (utils::pnpm_workspace::git_branch_locks, via a new git_branch_locks_in that skips the environment, as discovery already does for the lockfile setting) and, only while the lockfile setting is on and outside Rush, marks every ref of the root and member pnpm locks UnattestedKind::PnpmGitBranchLock. N2, user-patched npm / Bun packages (#711, #367): hosted leaves a package the project patches itself unwired and never confirms it; vendored refuses. Hosted's npm gate is factored into patch::redirect::NpmUserPatches (package.json patchedDependencies keys plus non-null lock-entry `patched` records in any present npm lock), which the hosted rewriter and npm discovery now share. Bun discovery applies the rewriters' per-dependency rule (bun_lock_text::patched_dependency_key over the manifest keys, plus bun.lock's mirrored keys for the text lock) against each ref's hosted URL or vendored tarball. Matching refs are marked UnattestedKind::UserPatchedDependency. Both keep the ref (rollback / remove / list still find a pre-fix pin and the ledgers keep the wiring live, since no re-run can clear the evidence) and only vex omits it, as vex_pnpm_git_branch_lockfile / vex_npm_patched_dependency. CLI_CONTRACT.md documents both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
added a commit
that referenced
this pull request
Nov 4, 2026
Two follow-ups under the rule "a user patch applied on top means never attest". Bun (and npm) keys naming the pinned tarball itself. #2010 kept attesting a pin whose `patchedDependencies` key names the pin's own tarball (a `bun patch` made on top of the Socket pin, #1019), because discovery reused the hosted rewriter's per-dependency rule, where such a key is no conflict. Bun applies that patch over the Socket bytes, so the install never runs the Socket patch alone. Discovery now reads the keys against no target (any key for the name@version counts) and marks such a ref UserPatchedDependency with its own detail; npm discovery reads its keys the same way. bun_user_patched_dependency_pin_is_not_attested pinned the old attested behavior and now asserts the mark. The hosted rewriter is unchanged: it keeps such a pin (and confirms it on an in-sync re-run), while vex's plan omits it through the unattested mark. Workspace members. The consent gate walked workspace members and ancestor workspace settings; discovery and the in-memory engine read the project root only. UserPatchIndex's walk now runs over a ProjectView and is shared: discover() (the gate) is the walk over the disk with every ecosystem, npm_family_in() the same walk over any view with the npm-family declarations (pnpm, yarn, patch-package), so members' package.json pnpm.patchedDependencies, yarn patch: descriptors, patches/ folders and --patch-dir, and their pnpm/yarn files are read alike. The member glob walk is rewritten over the view (same depth, skip list, limits and link handling as the walkdir one), and an ancestor with any workspace marker is walked in both scopes. The walk never reads outside the git repository root (the nearest ancestor holding .git, inclusive; in memory the project itself): the ancestor walk stops there, as before, and a workspace member or literal --patch-dir beyond it now protects every npm package with an "outside the git repository" diagnostic instead of being read, in the gate too. Without a .git the gate's home-directory rule still bounds the ancestor walk. workspace_member_patches_agree_with_the_consent_gate checks discovery and the gate agree on member cases, on disk and in memory, and on a member beyond the repository root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
VEX discovery (
vex/discover/) attested two kinds of npm-family pins that hosted and vendored mode both refuse. In both cases the install may not use the attested bytes.gitBranchLockfile(Hosted scan with pnpmgitBranchLockfilepins the stale pnpm-lock.yaml and reports success, while pnpm installs unpatched bytes from pnpm-lock.<branch>.yaml #556). When the setting is on and apnpm-lock.<branch>.yamlexists, pnpm installs the branch from that lock. Hosted leaves out the root locks and member locks and refuses every pnpm pin (hosted/engine.rs,refuse_git_branch_locks). Vendored refuses the project (vendor/pnpm_lock.rs,git_branch_lock_refusal). Discovery'sextract_pnpmnever checked the setting.patchedDependencies/ lockpatchedrecord Hosted npm scan pins a package that npm 12's nativepatchedDependenciesalso patches, so every laternpm ci/npm installfails EPATCHFAILED (and vendored refuses the lockfileVersion 4 lock with wrong advice) #711,bun patchHosted and vendored Bun rewiring silently discards the project's ownbun patch(patchedDependencies): fresh frozen installs drop the user's patch with exit 0 #367). Hosted skips these packages and records them as refused, and never confirms them. Vendored refuses them (npm v4 lock; Bun refuses the one dependency). Discovery's npm and Bun extractors never read these fields, so a pin written before Hosted npm scan pins a package that npm 12's nativepatchedDependenciesalso patches, so every laternpm ci/npm installfails EPATCHFAILED (and vendored refuses the lockfileVersion 4 lock with wrong advice) #711/Hosted and vendored Bun rewiring silently discards the project's ownbun patch(patchedDependencies): fresh frozen installs drop the user's patch with exit 0 #367 was attested.Fix
extract_pnpmruns hosted's own detection,utils::pnpm_workspace::git_branch_locks, through a newgit_branch_locks_in(view, env). It follows the same rules as hosted: only outside Rush, and only while pnpm'slockfilesetting is on (hosted useselse ifthere too). When branch locks are in use, every ref of the root pnpm lock (orshrinkwrap.yaml) and of each member's own lock is markedUnattestedKind::PnpmGitBranchLock. This goes throughUnwiredCopy { scope: <lock>, target: Any }, so only refs that survive the contests get marked. Hosted and vendored refs are both marked, because vendored mode also refuses this state.patchedDependenciesalso patches, so every laternpm ci/npm installfails EPATCHFAILED (and vendored refuses the lockfileVersion 4 lock with wrong advice) #711 gate is moved out ofrewrite_npm_lockintopatch::redirect::NpmUserPatches. It holds thepackage.jsonpatchedDependencieskeys plus the non-nullpatchedrecords from any present npm lock. The hosted rewriter and npm discovery both use it now (no behaviour change for hosted). Bun discovery uses the rewriters' per-dependency rule,bun_lock_text::patched_dependency_key: forbun.lockit reads the manifest keys and the keys mirrored in the lock, the same asrewrite_bun_lock; forbun.lockbit reads the manifest keys only, the same as the hosted engine andvendor::bun_binary. Each ref is checked against its hosted URL or vendored tarball, so a key naming the pinned tarball itself is still not a conflict. Matching refs are markedUnattestedKind::UserPatchedDependency.vex_sources.rsgets note armsvex_pnpm_git_branch_lockfile/vex_npm_patched_dependency.CLI_CONTRACT.mddocuments both: the Unattested references bullet, the code table, and a cross-link from the Hosted scan with pnpmgitBranchLockfilepins the stale pnpm-lock.yaml and reports success, while pnpm installs unpatched bytes from pnpm-lock.<branch>.yaml #556 paragraph.Why keep the ref and mark it unattested, instead of a diagnostic and no ref
In both cases the writers treat the wiring as present but unchangeable, not as dead:
socket-patch rollback <uuid>for a pin written before Hosted npm scan pins a package that npm 12's nativepatchedDependenciesalso patches, so every laternpm ci/npm installfails EPATCHFAILED (and vendored refuses the lockfileVersion 4 lock with wrong advice) #711. That rollback needs the ref.scan/vendorre-run can't clear the evidence in either case.If the ref were dropped, the ledger liveness checks (
hosted_claim/vendored_claim) would report the wiring as reverted. That's the same reasoning as #2005 / #2007,BundledCopyandNpmShrinkwrapOnly.Choices worth a look
gitBranchLockfile. It reads the setting frompnpm-workspace.yaml,.npmrcand the governing ancestor's files only. Hosted and vendored also readnpm_config_git_branch_lockfile/pnpm_config_git_branch_lockfilefrom the environment. I followed discovery's existing precedent for thelockfilesetting (lockfile_disabled(view, false): "the hosted engine runs in the install's environment,vexneed not"), sovexand the ledger checks don't depend on the scanner's shell.bun patchover an earlier hosted pin, Bun silently drops the user's patch, so the installed bytes may in fact be the pure Socket bytes. I still don't attest them: hosted refuses and never confirms, and the owner rule makes the modes the ground truth.Tests
Each new test fails on the code before this change (checked by switching the three new marking paths off: all three panic at their unattested assertions) and passes after:
vex::discover::npm::tests::pnpm_git_branch_lockfile_marks_the_lock_pins_unattested: root lock viapnpm-workspace.yamland via.npmrc, plus asharedWorkspaceLockfile: falsemember with its own branch lock. Controls: no branch lock, and the setting off.vex::discover::npm::tests::npm_user_patched_dependency_pin_is_not_attested: a manifest key, a v4 lock-entrypatchedrecord, a record in the sibling shrinkwrap only, and a key for another package or version as the control.vex::discover::bun::tests::bun_user_patched_dependency_pin_is_not_attested: hostedbun.lockwith versioned and bare keys; a key naming the pinned tarball (no conflict); a vendored ref keyed throughbun.lock's mirrored keys; hostedbun.lockb.commands::vex_sources::tests::npm_family_unattested_kinds_have_their_own_tags.Runs:
cargo test -j4 -p socket-patch-core --lib -- vex::discover:: patch::redirect:: vendor::npm vendor::bun vendor::pnpm hosted:: utils::pnpm_workspace: 2173 passed.vex::discover::testing::golden::tests::committed_fixture_corpus_matches_goldenpasses; no golden changes.cargo test -j4 -p socket-patch-cli --lib vex_sources: 15 passed.cargo test -p socket-patch-cli --test in_process_redirect_pnpm: 41 passed.cargo clippy --locked --workspace --all-features -- -D warningsandcargo fmt --all -- --checkare clean.Not done
patchedDependencies(pnpm.patchedDependencies/pnpm-workspace.yaml), yarn berrypatch:and vlt are not covered. They're out of scope for N2./code-review highhas not been run yet; this PR stays a draft until it is.🤖 Generated with Claude Code