From 4d775b198e3ada505656627652e35c9fe9c6e7c8 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 11:48:51 -0600 Subject: [PATCH 1/4] Never patch a pnpm, yarn or vlt dependency the user patched A package the project patches itself is applied by the package manager on top of whatever tarball the lock names, so a Socket pin of it either fails the install or installs the user's diff stacked on the Socket patch. The CLI's consent gate (patch::user_patches::UserPatchIndex, #1646) already kept `scan` / `get` / `vendor` / `apply` from wiring such pnpm, yarn berry and patch-package packages unless --allow-patch-conflicts, but two consumers did not read that evidence: - vex discovery attested a pnpm, yarn, vlt (or npm / Bun) pin of such a package: a pin written before the patch was declared, or one an --allow-patch-conflicts run wrote. - the in-memory hosted engine (Node addon, hosted-bundle) has no gate in front of it and rewired them. One detector, three consumers. UserPatchIndex's pnpm / yarn / patch-package parse rules are factored into pure helpers (pnpm_yaml, yarn_manifest, yarn_lock, patch_package_dirs, patch_file), which the consent gate keeps using, plus UserPatchIndex::npm_family_in, which reads the same declarations through a ProjectView (disk, snapshot or memory): package.json's pnpm.patchedDependencies, yarn patch: descriptors and patch-package dirs, pnpm-workspace.yaml's patchedDependencies, the patches of the given pnpm locks (top-level map, patch_hash suffixes) and yarn.lock's user patch: locators. It leaves out npm's and Bun's own top-level patchedDependencies, which no other package manager reads and which #711 / #367 / #2010 already judge per resolution. - Discovery: a new vex::discover::user_patched pass marks every npm ref the index protects UnattestedKind::UserPatchedDependency (the ref stays, so rollback / remove / list still find it). The bytes the install runs are Socket's patch alone only when no user patch is declared, so a declared one always withholds the statement, with or without consent. - Hosted: engine::withhold_user_patched drops such candidates from every rewriter and records them skipped `user_patch_conflict` (the consent gate's code), plus a warning in memory. The in-memory engine always applies it; the CLI disk flow applies it unless --allow-patch-conflicts (it adds the Rush lock the consent gate does not read). - Vendored: unchanged. Every vendored entry point already runs the consent gate, which refuses (`vendor`, vendored writers) or skips (`scan`, `apply`) these packages. Override: the existing --allow-patch-conflicts, invocation-only. It already covered pnpm / yarn / patch-package at the consent gate and now also lifts the engine gate; vex never attests while the patch is declared. npm / Bun native patches stay refused even with consent, as before. vlt has no patch mechanism (vlt@1.6.2 has no patchedDependencies, patch_hash or `patch` command; vlt.json modifiers rewrite specs), and neither has yarn classic, so patch-package is what is read for both. Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 13 +- .../src/commands/scan/hosted.rs | 14 + .../src/commands/vex_sources.rs | 15 +- .../tests/hosted_memory_engine.rs | 122 +++++ .../tests/in_process_redirect_pnpm.rs | 72 +++ crates/socket-patch-core/src/hosted/engine.rs | 99 ++++ .../src/hosted/memory/stages.rs | 6 + .../src/patch/user_patches.rs | 434 ++++++++++++++---- .../socket-patch-core/src/vex/discover/mod.rs | 17 +- .../src/vex/discover/user_patched.rs | 343 ++++++++++++++ docs/usage.md | 11 +- 11 files changed, 1039 insertions(+), 107 deletions(-) create mode 100644 crates/socket-patch-core/src/vex/discover/user_patched.rs diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 52165d509..298671ed9 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -175,6 +175,12 @@ A missing referenced diff does not remove that protection. `error.code: user_patch_conflict`, before their patch writes. `vendor` still reverts entries dropped from the manifest first. A hosted or vendored writer rechecks under its apply lock before rewiring. +- The hosted engine applies the same rules to npm-family packages on its own (pnpm `patchedDependencies`, a Yarn `patch:` + descriptor, a `patch-package` file, read at the project root and in its pnpm, Rush and yarn locks): a matching candidate is + withheld from every rewriter and reported as a `skipped` event / `skipped[]` row with reason `user_patch_conflict`. The + in-memory engine has no consent option, so it always skips them and adds a `user_patch_conflict` warning; + `--allow-patch-conflicts` lifts the disk flow's check. With consent the package is wired under the user's patch, which the + package manager still applies on top, so `vex` never attests it (`vex_npm_patched_dependency`). - `--allow-patch-conflicts` approves the attempt for this invocation and emits `user_patch_override` warnings. It is forwarded to nested apply. `--yes`, `--force`, a UUID, policy settings, and `SOCKET_ALLOW_PATCH_CONFLICTS` do not grant @@ -187,7 +193,8 @@ A missing referenced diff does not remove that protection. reversal (`rollback`, `remove`, `vendor --revert`) keep their existing behavior. `repair` may restore Socket-owned artifacts; any re-vendoring uses the same consent gate. Existing Socket pins are not automatically unwound when a user - adds a patch: inspect them and explicitly roll back if necessary. One + adds a patch: inspect them and explicitly roll back if necessary (`vex` stops + attesting an npm-family one meanwhile, `vex_npm_patched_dependency`). One exception: a Yarn Berry hosted pin or vendored tarball that a user `patch:` descriptor wraps (`yarn patch` run over the Socket pin) cannot be unwound under it, since the wrapper would name a descriptor no lock entry resolves. @@ -547,7 +554,7 @@ Recognition rules that hold for every ecosystem: * **Patch hosts.** A hosted reference counts only on `https://patch.socket.dev` or the `--patch-server-url` / `SOCKET_PATCH_SERVER_URL` origin, with no userinfo. The uuid is the URL's LAST canonical-uuid path segment, because grant tokens may themselves be uuid-shaped. The Go module prefix is fixed. `socket-patch-` registry / repository / source names count only through a pin. For a URL on any other host, see **Patch hosts** above. * **Pins, not definitions.** A registry, index or source *definition* alone (cargo `[registries]`, nuget ``, pom ``, uv index tables, `.npmrc`) never makes a reference, because it survives a reverted pin. Sections the package manager ignores are not read: npm's v2 `dependencies` mirror, a `.cargo/config.toml` shadowed by `.cargo/config`. A Socket pin inside a maven `` is diagnosed, never a reference. * **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. The root `requirements.txt` and its in-root `-r` includes count as one lock here: pip reads them as one requirement set, where a direct reference wins over a compatible `==` pin of the same version in another file of the set (#1086). A PyPI version matches under PEP 440 equality, so a requirements line pinned in any spelling that can only install the wired release (`==1.2.3.0`, `==v1.2.3`, `===1.2.3`) contests it like `==1.2.3` does; a range or wildcard (`~=1.2.3`, `==1.2.*`) does not (#1602). A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`, beneath a dependency that bundles it; the root project's own bundles install from the lock, #1618) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). pnpm unpacks bundled copies too, but its lock cannot tie one to a reference (see **Unattested references** below). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. yarn classic's `yarn.lock` records nothing about bundled copies either (#758), so for a lock with a Socket wiring they are found in the installed `node_modules`: each package a parent's `bundleDependencies` / `bundledDependencies` names inside the parent's own `node_modules`, the dependencies of those beside them, and everything nested under them. Such a copy contests the reference in `yarn.lock` (the diagnostic names the parent's lock entry and the copy's directory); a lock-only `vex` with no `node_modules` can't see it. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`, and a yarn classic hosted scan names one with `redirect_copy_unpatched`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. A reference withheld only because such an unreachable copy installs beside it (a bundled npm / Bun / vlt copy, an npm copy beneath a `hasShrinkwrap` package (#753) or one npm installs from a git / url / `file:` spec, or a yarn classic git or `file:` directory block of the same `name@version`) is still the rewriter's own wiring of exactly one package version: it is never attested, but `rollback`, `remove`, `list` and the hosted → vendored takeover treat it as an ordinary hosted pin and restore its upstream registry entry (#828), rather than refusing it with `hosted_wiring_contested`. Another entry of the **same** npm, Bun or yarn lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install` / `yarn install`; for yarn berry, a registry locator such as `left-pad@npm:1.3.0` beside the hosted `…::__archiveUrl=` one) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy. -* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Six cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **pnpm branch locks** (`vex_pnpm_git_branch_lockfile`): with `gitBranchLockfile` on, pnpm installs a git branch from its own `pnpm-lock..yaml` (the **pnpm per-branch locks** state above), which no wiring reaches. `vex` does not decide whether the setting is on: the presence of any branch lock (at the root, or in a workspace member that installs from its own lock under `sharedWorkspaceLockfile: false`, read from the project's files and never from the environment) omits every reference of the root pnpm lock (`pnpm-lock.yaml`, or `shrinkwrap.yaml` without it) and of each member's own lock, hosted and vendored alike, whatever `gitBranchLockfile` or pnpm's `lockfile` setting says (#556; deterministic and fail-closed). This is checked only when a root or member pnpm lock wires a patch. Hosted mode refuses every pnpm pin when the setting is on and vendored mode refuses the project, so this also covers wiring written before the branch lock appeared. The run warning names the branch lock in the marked lock's own directory. Rush locks are not affected. **Self-patched npm and Bun packages** (`vex_npm_patched_dependency`): a reference in `package-lock.json` / `npm-shrinkwrap.json` / `bun.lock` / `bun.lockb` to a package the project patches itself is omitted, by the hosted rewriters' own per-dependency rule (`redirect_npm_patched_dependency_skipped`, #711; `redirect_bun_patched_dependency_skipped`, #367): a root `package.json` `patchedDependencies` key for its `name@version`, its bare name, or `name@` of a tarball other than the one the reference pins (npm and Bun alike, as both hosted gates read it); for npm also a non-null `patched` record on any present npm lock's entry for that `name@version`; for `bun.lock` also the keys it mirrors (the run warning then names `bun.lock` as the key's source). npm applies the user's diff on top of whatever tarball the lock names and Bun applies it only to the resolution it is keyed on, so both modes leave such a package unwired; this covers pins an earlier release wrote, and an npm one fails `npm ci` with `EPATCHFAILED` until `socket-patch rollback ` restores the registry entry (the remedy says so). A key naming the pinned tarball itself is no conflict; a reference with neither a hosted url nor a vendored tarball is matched by its `name@version` and counts every tarball key. Only a reference that survives the lock contests carries this reason. +* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Six cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **pnpm branch locks** (`vex_pnpm_git_branch_lockfile`): with `gitBranchLockfile` on, pnpm installs a git branch from its own `pnpm-lock..yaml` (the **pnpm per-branch locks** state above), which no wiring reaches. `vex` does not decide whether the setting is on: the presence of any branch lock (at the root, or in a workspace member that installs from its own lock under `sharedWorkspaceLockfile: false`, read from the project's files and never from the environment) omits every reference of the root pnpm lock (`pnpm-lock.yaml`, or `shrinkwrap.yaml` without it) and of each member's own lock, hosted and vendored alike, whatever `gitBranchLockfile` or pnpm's `lockfile` setting says (#556; deterministic and fail-closed). This is checked only when a root or member pnpm lock wires a patch. Hosted mode refuses every pnpm pin when the setting is on and vendored mode refuses the project, so this also covers wiring written before the branch lock appeared. The run warning names the branch lock in the marked lock's own directory. Rush locks are not affected. **Self-patched npm and Bun packages** (`vex_npm_patched_dependency`): a reference in `package-lock.json` / `npm-shrinkwrap.json` / `bun.lock` / `bun.lockb` to a package the project patches itself is omitted, by the hosted rewriters' own per-dependency rule (`redirect_npm_patched_dependency_skipped`, #711; `redirect_bun_patched_dependency_skipped`, #367): a root `package.json` `patchedDependencies` key for its `name@version`, its bare name, or `name@` of a tarball other than the one the reference pins (npm and Bun alike, as both hosted gates read it); for npm also a non-null `patched` record on any present npm lock's entry for that `name@version`; for `bun.lock` also the keys it mirrors (the run warning then names `bun.lock` as the key's source). npm applies the user's diff on top of whatever tarball the lock names and Bun applies it only to the resolution it is keyed on, so both modes leave such a package unwired; this covers pins an earlier release wrote, and an npm one fails `npm ci` with `EPATCHFAILED` until `socket-patch rollback ` restores the registry entry (the remedy says so). A key naming the pinned tarball itself is no conflict; a reference with neither a hosted url nor a vendored tarball is matched by its `name@version` and counts every tarball key. Only a reference that survives the lock contests carries this reason. **Packages patched with pnpm, Yarn or patch-package** (`vex_npm_patched_dependency` too): a reference of any npm-family lock (pnpm, Rush, yarn classic and berry, vlt, npm, Bun; hosted or vendored) to a `name@version` the project patches itself is omitted, read from the project root: pnpm's `patchedDependencies` (`package.json`'s `pnpm.patchedDependencies`, `pnpm-workspace.yaml`'s `patchedDependencies`, and the `patchedDependencies` map or a `(patch_hash=…)` key suffix in the reference's own pnpm lock or the root one), a yarn berry `patch:` descriptor outside Yarn's builtin compatibility patches (`package.json`'s dependency maps and `resolutions`, or a `patch:` locator `yarn.lock` resolves, including one wrapping the hosted tarball), or a patch-package file (`patches/+.patch`, or under a `--patch-dir` a `package.json` script names). A key's version may be exact, a range or absent (every version), and metadata that cannot be read (an unparseable `package.json`, a `--patch-dir` the script computes) omits every npm reference. These are the parse rules of the `user_patch_conflict` consent gate (see [Existing project patches](#existing-project-patches)) without its workspace and ancestor walk, and the evidence the in-memory hosted engine withholds a package by. The package manager (or the postinstall) applies the user's diff on top of whatever tarball the lock names, so the install never runs the Socket patch alone: the reference is omitted even when an `--allow-patch-conflicts` run wired it, for as long as the patch is declared. Yarn classic and vlt have no patch mechanism of their own (vlt's `vlt.json` `modifiers` rewrite specs, not contents), so patch-package is what is read for them. npm's and Bun's own top-level `patchedDependencies` are judged by the rule above, not here. * **Shrinkwrap without a twin (#899).** npm >= 12 never reads `npm-shrinkwrap.json`: on a project whose only npm lock is the shrinkwrap it resolves the tree from the registry and writes a fresh `package-lock.json`, so a Socket reference there reaches npm <= 11 only. The reference is still discovered, so `list`, `rollback` and `remove` manage it, but `vex` omits the patch (`vex_npm_shrinkwrap_only`, as a run warning and as the `failed[].reason`). A `package-lock.json` beside it (wired the same way) makes it attestable again. Hosted and vendored runs still rewire the shrinkwrap and warn `redirect_npm_shrinkwrap_only` / `vendor_npm_shrinkwrap_only`. * **Lockless pins.** With no lock to name a version, a `Cargo.toml` pin (every declaration on `socket-patch-`, that registry defined on the patch host for the same uuid) or an exclusive nuget exact-id mapping is never a reference on its own, so v5.0 does not attest it (nor does `list` show it, or `rollback` / `remove` restore it — restore those files from version control). Only a pre-v5 redirect-ledger record naming a version the pin admits keeps it live. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged). @@ -2679,7 +2686,7 @@ match it, withholds the statement: | `vex_maven_reactor_root` | run warning and `failed[].reason` | A hosted Maven pin sits in a root `pom.xml` that declares `` / `` (a profile's included), where a module's own literal `` overrides it; only the root pom is read, so no statement (#261). `list`, `rollback` and `remove` still find the pin. | | `vex_maven_classifier_unpatched` | run warning and `failed[].reason` | A hosted Maven pin's GA also has a live executable `` copy (not `sources` / `javadoc`) at a version other than the pin, which the classifier-less pin does not retarget and the grant does not serve; no statement while it does. `list`, `rollback` and `remove` still find the pin. | | `vex_pnpm_git_branch_lockfile` | run warning and `failed[].reason` | An npm pin is wired in a pnpm lock, but a `pnpm-lock..yaml` exists, which pnpm installs the branch from instead when `gitBranchLockfile` is on (#556); the branch lock's presence withholds the statement whatever the setting says, until the branch locks are folded in and deleted. `list`, `rollback` and `remove` still find the pin, and `vendor --check` and `scan` still treat the wiring as live. | -| `vex_npm_patched_dependency` | run warning and `failed[].reason` | An npm or Bun pin is wired, but the project patches that package itself (`patchedDependencies`, or npm's lock-entry `patched` record; #711, #367), which hosted and vendored mode never wire; no statement while it does. `list`, `rollback` and `remove` still find the pin. | +| `vex_npm_patched_dependency` | run warning and `failed[].reason` | An npm-family pin is wired, but the project patches that package itself: npm's or Bun's `patchedDependencies` or npm's lock-entry `patched` record (#711, #367), which hosted and vendored mode never wire, or pnpm's `patchedDependencies`, a Yarn `patch:` descriptor or a patch-package file, which they wire only with `--allow-patch-conflicts`, under the user's patch; no statement while the patch is declared. `list`, `rollback` and `remove` still find the pin. | | `vex_gradle_derived_cache_unchecked` | run warning | The derived-cache walk was cut short (a very large transforms cache); the statement is still emitted, the copies the walk did reach were checked. | A vendored Gradle entry attests only while its wiring is live (apply line, index rows, diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index 37f1f4600..3d1216748 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -887,6 +887,20 @@ async fn run_redirect_selected_untracked( &vlt_preflight.withheld_everywhere, &mut skipped, ); + // The engine's own user-patch gate (pnpm, yarn, patch-package), which + // the in-memory engine applies too. The consent gate above already + // left such packages out unless `--allow-patch-conflicts`, which lifts + // this one as well; it still covers evidence that gate does not read + // (a Rush lock's patched entries). Each such package is a skip event, + // as the consent gate reports it. + let _ = engine::withhold_user_patched( + &view, + &std::collections::BTreeMap::new(), + &mut candidates, + &mut skipped, + common.allow_patch_conflicts, + ) + .await; // A NEW row the lockfiles already pin on the patch server these // references name (not a configured one, so the recorded view's // discovery could not see it) is ALREADY: decided here, before the lock diff --git a/crates/socket-patch-cli/src/commands/vex_sources.rs b/crates/socket-patch-cli/src/commands/vex_sources.rs index 130d6f0c0..b14a9d47d 100644 --- a/crates/socket-patch-cli/src/commands/vex_sources.rs +++ b/crates/socket-patch-cli/src/commands/vex_sources.rs @@ -236,8 +236,9 @@ const NOTE_MAVEN_CLASSIFIER_UNPATCHED: &str = "vex_maven_classifier_unpatched"; /// `pnpm-lock..yaml` exists, which pnpm installs from instead when /// `gitBranchLockfile` is on; its presence withholds (`vex::Unattested`). const NOTE_PNPM_GIT_BRANCH_LOCKFILE: &str = "vex_pnpm_git_branch_lockfile"; -/// Omission tag and note: an npm or Bun pin's package is one the project -/// patches itself (`patchedDependencies`, `vex::Unattested`). +/// Omission tag and note: an npm-family pin's package is one the project +/// patches itself (npm / Bun / pnpm `patchedDependencies`, a yarn `patch:` +/// descriptor, a patch-package patch; `vex::Unattested`). const NOTE_NPM_PATCHED_DEPENDENCY: &str = "vex_npm_patched_dependency"; /// The omission tag of an [`Unattested`](socket_patch_core::vex::Unattested) @@ -284,10 +285,12 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) { ), UnattestedKind::UserPatchedDependency => ( NOTE_NPM_PATCHED_DEPENDENCY, - "not attested while the project patches the package itself (fold the Socket fix \ - into your own patch, or remove the `patchedDependencies` entry and re-run); an \ - npm pin an earlier hosted run wrote fails `npm ci` with `EPATCHFAILED` until \ - `socket-patch rollback ` restores the registry entry", + "not attested while the project patches the package itself, even when it was \ + wired with `--allow-patch-conflicts` (fold the Socket fix into your own patch, or \ + remove your patch: the `patchedDependencies` entry, the yarn `patch:` descriptor \ + or the patch-package file, and re-run); an npm pin an earlier hosted run wrote \ + fails `npm ci` with `EPATCHFAILED` until `socket-patch rollback ` restores \ + the registry entry", ), } } diff --git a/crates/socket-patch-cli/tests/hosted_memory_engine.rs b/crates/socket-patch-cli/tests/hosted_memory_engine.rs index c9f771b64..4a944c13b 100644 --- a/crates/socket-patch-cli/tests/hosted_memory_engine.rs +++ b/crates/socket-patch-cli/tests/hosted_memory_engine.rs @@ -2202,3 +2202,125 @@ async fn issue_1630_berry_exec_and_git_copies_are_named() { } } } + +/// REGRESSION: the in-memory engine (no consent gate in front of it, unlike +/// the CLI) pinned a package the project patches itself with pnpm +/// (`patchedDependencies`, in `pnpm-workspace.yaml`, `package.json`'s +/// `pnpm` field or the lock), yarn (a `patch:` descriptor in +/// `package.json`, or the locator `yarn.lock` resolves) or patch-package, +/// so the install stacked the user's diff on the hosted +/// patch. Such a candidate is now withheld from every rewriter, skipped +/// `user_patch_conflict` with a warning, and its lock left untouched; +/// another package's patch withholds nothing. +#[tokio::test] +async fn user_patched_npm_packages_are_never_pinned() { + let berry_wrapper = + "\"left-pad\": \"patch:left-pad@npm%3A^1.3.0#./.yarn/patches/left-pad.patch\""; + let cases: Vec<(&str, &str, String, &str)> = vec![ + ( + "redirect/npm/pnpm/basic", + "pnpm-workspace.yaml", + "patchedDependencies:\n left-pad@1.3.0: patches/left-pad@1.3.0.patch\n".into(), + "pnpm-lock.yaml", + ), + ( + "redirect/npm/pnpm/basic", + "package.json", + r#"{"pnpm":{"patchedDependencies":{"left-pad":"patches/left-pad.patch"}}}"#.into(), + "pnpm-lock.yaml", + ), + ( + "redirect/npm/yarn-berry/basic", + "package.json", + String::from_utf8( + fixture_files(&fixtures_root().join("redirect/npm/yarn-berry/basic/input")) + ["package.json"] + .clone(), + ) + .unwrap() + .replacen("\"left-pad\": \"^1.3.0\"", berry_wrapper, 1), + "yarn.lock", + ), + ( + "redirect/npm/yarn-berry/basic", + "yarn.lock", + format!( + "{}\n\"left-pad@patch:left-pad@npm%3A1.3.0#./.yarn/patches/lp.patch\":\n \ + version: 1.3.0\n resolution: \"left-pad@patch:left-pad@npm%3A1.3.0#./.yarn/\ + patches/lp.patch::version=1.3.0&hash=abc123\"\n languageName: node\n \ + linkType: hard\n", + String::from_utf8( + fixture_files(&fixtures_root().join("redirect/npm/yarn-berry/basic/input")) + ["yarn.lock"] + .clone() + ) + .unwrap() + ), + "yarn.lock", + ), + ( + NPM_FIXTURE, + "patches/left-pad+1.3.0.patch", + "diff\n".into(), + "package-lock.json", + ), + ]; + for (fixture, file, text, lock) in cases { + let dir = fixtures_root().join(fixture); + let server = MockServer::start().await; + mount_api( + &server, + &patches_from_overrides(&dir.join("overrides.json"), None), + ) + .await; + let files = fixture_files(&dir.join("input")); + let control = run_engine(&server, build_input(&files, &[], options(false))).await; + assert_eq!( + control.projects[0].redirected.len(), + 1, + "{fixture}: control {:?}", + control.projects[0].skipped + ); + + let mut files = files; + files.insert(file.to_string(), text.clone().into_bytes()); + let output = run_engine(&server, build_input(&files, &[], options(false))).await; + let project = &output.projects[0]; + assert!(project.error.is_none(), "{fixture}: {:?}", project.error); + assert!( + project.redirected.is_empty(), + "{fixture} {file}: {:?}", + project.redirected + ); + let skip = project + .skipped + .iter() + .find(|s| s.reason == "user_patch_conflict") + .unwrap_or_else(|| panic!("{fixture} {file}: {:?}", project.skipped)); + assert_eq!(skip.purl, "pkg:npm/left-pad@1.3.0"); + assert!( + skip.detail.as_deref().is_some_and(|d| d.contains(file)), + "{fixture}: {:?}", + skip.detail + ); + let warnings = project.redirect["warnings"].as_array().unwrap(); + assert!( + warnings.iter().any(|w| w["code"] == "user_patch_conflict"), + "{fixture}: {warnings:?}" + ); + assert!( + output.changed_files.iter().all(|f| f.path != lock), + "{fixture}: {lock} must stay untouched" + ); + + // Another package's patch withholds nothing. + let mut files = fixture_files(&dir.join("input")); + files.insert( + "pnpm-workspace.yaml".into(), + b"patchedDependencies:\n is-number@7.0.0: patches/is-number.patch\n".to_vec(), + ); + files.insert("patches/is-number+7.0.0.patch".into(), b"diff\n".to_vec()); + let output = run_engine(&server, build_input(&files, &[], options(false))).await; + assert_eq!(output.projects[0].redirected.len(), 1, "{fixture}: other"); + } +} diff --git a/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs b/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs index fb7f1cbc7..4abbd02c3 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs @@ -3354,3 +3354,75 @@ async fn issue_1268_member_lock_still_hosted_keeps_trust_without_warning() { "the workspace file is left as it was" ); } + +/// A package the project patches itself with pnpm's `patchedDependencies` +/// is never pinned by default (the consent gate skips it), and only the +/// explicit `--allow-patch-conflicts` override pins it. pnpm then applies +/// the user's diff on top of the hosted tarball, so the install never runs +/// the Socket patch alone: lockfile discovery keeps the pin as a ref (so +/// `rollback` restores it) but marks it unattested, and `vex` omits it. +#[tokio::test] +#[serial] +async fn hosted_pins_a_pnpm_patched_dependency_only_with_explicit_consent() { + use socket_patch_core::vex::discover::{ + discover_patched_refs_with, DiscoverOptions, UnattestedKind, + }; + + let server = MockServer::start().await; + mock_discovery(&server).await; + mock_reference(&server).await; + + let tmp = tempfile::tempdir().unwrap(); + write_pnpm_project(tmp.path()); + let workspace = format!( + "packages:\n - '.'\npatchedDependencies:\n {NAME}@{VERSION}: patches/{NAME}.patch\n" + ); + std::fs::write(tmp.path().join("pnpm-workspace.yaml"), &workspace).unwrap(); + let pristine = std::fs::read_to_string(tmp.path().join("pnpm-lock.yaml")).unwrap(); + + // Default: skipped, nothing written. + let code = run(hosted_args(tmp.path(), server.uri())).await; + assert_eq!(code, 0, "a user-patch skip alone exits 0"); + assert_eq!( + std::fs::read_to_string(tmp.path().join("pnpm-lock.yaml")).unwrap(), + pristine, + "the user-patched package is left on its registry entry" + ); + + // Explicit consent: pinned. + let mut args = hosted_args(tmp.path(), server.uri()); + args.common.allow_patch_conflicts = true; + let code = run(args).await; + assert_eq!(code, 0, "the consented run succeeds"); + let lock = std::fs::read_to_string(tmp.path().join("pnpm-lock.yaml")).unwrap(); + assert!( + lock.contains(&format!("tarball: {HOSTED_URL}")), + "--allow-patch-conflicts pins the package; got:\n{lock}" + ); + + // ...but the pin is never attested while pnpm stacks the user's patch. + let out = discover_patched_refs_with( + tmp.path(), + &DiscoverOptions { + patch_server_origins: vec!["http://patch.test".to_string()], + }, + ) + .await; + assert!( + out.refs.iter().any(|r| r.purl == PURL && r.uuid == UUID), + "the pin stays a ref: {:?}", + out.refs + ); + let marked: Vec<_> = out + .unattested + .iter() + .filter(|u| u.kind == UnattestedKind::UserPatchedDependency) + .map(|u| (u.purl.as_str(), u.uuid.as_str())) + .collect(); + assert_eq!(marked, [(PURL, UUID)], "{:#?}", out.unattested); + assert!( + out.unattested[0].detail.contains("pnpm-workspace.yaml"), + "{}", + out.unattested[0].detail + ); +} diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 09901663b..75b8dfdf1 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -337,6 +337,105 @@ pub fn withhold_everywhere( candidates.retain(|c| !withheld_everywhere.contains_key(&c.dep.patch_uuid)); } +/// Skip reason of an npm candidate withheld because the project patches +/// that package itself ([`withhold_user_patched`]): the code the CLI's +/// consent gate reports for the same packages. +pub const USER_PATCH_CONFLICT: &str = "user_patch_conflict"; + +/// The pnpm and yarn locks [`withhold_user_patched`] reads when `files` +/// does not already hold them. +const USER_PATCH_LOCKS: [&str; 4] = [ + "pnpm-lock.yaml", + "shrinkwrap.yaml", + "yarn.lock", + crate::constants::npm_family::RUSH_COMMON_LOCK_REL, +]; + +/// Withhold from every rewriter each npm candidate whose package the +/// project patches itself with pnpm (`patchedDependencies`), yarn (a +/// `patch:` descriptor) or patch-package, recording each as skipped +/// ([`USER_PATCH_CONFLICT`]). The package manager applies the user's +/// diff on top of whatever tarball the lock names, so a hosted pin either +/// fails the install once the diff no longer applies or installs bytes +/// that are neither the user's package nor the Socket patch. The +/// evidence is [`UserPatchIndex::npm_family_in`], which `vex` discovery +/// reads too, over `files` (the candidate texts already read: a member's +/// own pnpm lock) and the root pnpm, yarn and Rush locks. npm's and Bun's +/// own `patchedDependencies` are not read here: their rewriters skip such +/// a package per resolution (#711, #367). `allow` is the caller's explicit +/// consent (the CLI's `--allow-patch-conflicts`): nothing is withheld. +/// Returns one [`USER_PATCH_CONFLICT`] warning per withheld candidate, +/// with the skip's detail. +/// +/// [`UserPatchIndex::npm_family_in`]: crate::patch::user_patches::UserPatchIndex::npm_family_in +pub async fn withhold_user_patched( + view: &ProjectView<'_>, + files: &BTreeMap, + candidates: &mut Vec, + skipped: &mut Vec, + allow: bool, +) -> Vec { + let mut warnings = Vec::new(); + if allow || !candidates.iter().any(|c| c.dep.ecosystem == "npm") { + return warnings; + } + let mut read: Vec<(String, String)> = Vec::new(); + for rel in USER_PATCH_LOCKS { + if files.contains_key(rel) { + continue; + } + if let Ok(text) = view.read_text(rel).await { + read.push((rel.to_string(), text)); + } + } + let locks: Vec<(&str, &str)> = files + .iter() + .chain(read.iter().map(|(rel, text)| (rel, text))) + .filter(|(rel, _)| { + matches!( + rel.rsplit('/').next(), + Some("pnpm-lock.yaml" | "shrinkwrap.yaml" | "yarn.lock") + ) + }) + .map(|(rel, text)| (rel.as_str(), text.as_str())) + .collect(); + let index = crate::patch::user_patches::UserPatchIndex::npm_family_in(view, &locks).await; + if index.is_empty() { + return warnings; + } + candidates.retain(|c| { + if c.dep.ecosystem != "npm" { + return true; + } + let sources = index.conflicts(&c.purl); + if sources.is_empty() { + return true; + } + let detail = user_patched_detail(&c.purl, &sources); + warnings.push(warning(USER_PATCH_CONFLICT, detail.clone())); + skipped.push(SkippedPatch { + detail: Some(detail), + ..SkippedPatch::new(&c.purl, &c.dep.patch_uuid, USER_PATCH_CONFLICT) + }); + false + }); + warnings +} + +/// [`withhold_user_patched`]'s skip detail. +fn user_patched_detail(purl: &str, sources: &[String]) -> String { + format!( + "{purl}: {} declares a patch the project applies to this package itself (pnpm \ + `patchedDependencies`, a yarn `patch:` descriptor, or a patch-package patch); the \ + package manager applies it on top of whatever tarball the lock names, so pinning the \ + hosted patch would stack the two (or fail every install once the user's diff no \ + longer applies). It is left unchanged and stays without the Socket patch: fold the \ + Socket fix into your own patch, or remove the user patch and re-run (the CLI's \ + explicit `--allow-patch-conflicts` wires it anyway)", + sources.join(", ") + ) +} + /// The project's candidate files as the rewriters read them. #[derive(Debug, Clone, Default)] pub struct CandidateFiles { diff --git a/crates/socket-patch-core/src/hosted/memory/stages.rs b/crates/socket-patch-core/src/hosted/memory/stages.rs index 63811b36b..8cce04a2b 100644 --- a/crates/socket-patch-core/src/hosted/memory/stages.rs +++ b/crates/socket-patch-core/src/hosted/memory/stages.rs @@ -207,6 +207,12 @@ pub(crate) async fn plan( } else { engine::read_candidate_files(&view, &unreadable, &candidates).await }; + // A package the project patches itself (pnpm, yarn, patch-package) is + // never wired: the in-memory engine has no consent knob. + pre_warnings.extend( + engine::withhold_user_patched(&view, &read.files, &mut candidates, &mut skipped, false) + .await, + ); let wheels = engine::wheel_targets(&candidates, &read.files) .into_iter() .map(|(dep, sha256)| (dep.artifact_url.clone(), sha256.to_string())) diff --git a/crates/socket-patch-core/src/patch/user_patches.rs b/crates/socket-patch-core/src/patch/user_patches.rs index 3bf200a29..78a2efd1f 100644 --- a/crates/socket-patch-core/src/patch/user_patches.rs +++ b/crates/socket-patch-core/src/patch/user_patches.rs @@ -17,6 +17,7 @@ use crate::utils::fs::read_regular_to_string; use crate::utils::npm_range::npm_range_admits; use crate::utils::purl::{normalize_purl, strip_purl_qualifiers}; use crate::utils::workspace_globs::{workspaces_include_as, WorkspacesDialect}; +use crate::vendor::lock_inventory::ProjectView; #[derive(Default, Debug)] pub struct UserPatchIndex { @@ -79,18 +80,7 @@ impl UserPatchIndex { let mut globs = Vec::new(); if let Some(value) = index.json(&dir.join("package.json"), "npm").await { index.native(&value, &dir.join("package.json")); - for field in [ - "dependencies", - "devDependencies", - "optionalDependencies", - "resolutions", - ] { - if let Some(map) = value[field].as_object() { - for spec in map.values().filter_map(Value::as_str) { - index.yarn(spec, &dir.join("package.json")); - } - } - } + index.yarn_manifest(&value, &dir.join("package.json")); // A malformed field still names its string items (#1900): // walking them can only find more patches. globs.extend(Workspaces::of(&value).into_listed().into_iter().flatten()); @@ -100,52 +90,20 @@ impl UserPatchIndex { } let yarn_lock = dir.join("yarn.lock"); if let Some(text) = index.read(&yarn_lock, "npm").await { - // Only Berry has patch: locators. Every Berry lock carries - // builtin compat patches, so scan the resolution lines rather - // than parse what can be a very large lockfile, and visit - // only the few lines that can hold one. - for line in lines_containing(&text, "@patch:") { - let Some(value) = line.trim_start().strip_prefix("resolution:") else { - continue; - }; - let value = value.trim().trim_matches('"'); - // A locator has an ident before @patch:. - if let Some((_, source)) = value.split_once("@patch:") { - index.yarn(&format!("patch:{source}"), &yarn_lock); - } - } + index.yarn_lock(&text, &yarn_lock); } for file in ["pnpm-workspace.yaml", "pnpm-lock.yaml"] { let path = dir.join(file); if let Some(text) = index.read(&path, "npm").await { - if file == "pnpm-lock.yaml" - && !text.contains("patchedDependencies") - && !text.contains("patch_hash") - && !text.contains("patched:") - { - continue; - } - match serde_saphyr::from_multiple_with_options::(&text, lock_options()) { - Ok(documents) => { - for value in documents { - index.native(&value, &path); - if file == "pnpm-workspace.yaml" { - if let Some(packages) = value["packages"].as_array() { - globs.extend( - packages - .iter() - .filter_map(Value::as_str) - .map(str::to_owned), - ); - } - } + let lock = file == "pnpm-lock.yaml"; + for value in index.pnpm_yaml(&text, &path, lock, true) { + if !lock { + if let Some(packages) = value["packages"].as_array() { + globs.extend( + packages.iter().filter_map(Value::as_str).map(str::to_owned), + ); } } - Err(e) => index.unknown( - "npm", - &path, - &format!("cannot inspect patch metadata: {e}"), - ), } } } @@ -208,6 +166,99 @@ impl UserPatchIndex { index } + /// The patches a project's pnpm, yarn or patch-package setup applies + /// to its npm dependencies, declared at the project root and read + /// through `view` as the hosted engine and `vex` discovery read the + /// project: `package.json`'s `pnpm.patchedDependencies`, its yarn + /// `patch:` descriptors and its patch-package directories (`patches/` + /// and every `--patch-dir` its scripts name), `pnpm-workspace.yaml`'s + /// `patchedDependencies`, and of `locks` (root-relative path and text) + /// each pnpm lock's patches and each `yarn.lock`'s user `patch:` + /// locators. These are the parse rules [`Self::discover`] (the CLI's + /// consent gate) applies, without its workspace and ancestor walk, and + /// without npm's and Bun's own top-level `patchedDependencies`, which + /// no other package manager reads and which their own rewriters and + /// discovery judge per resolution (#711, #367). vlt has no patch + /// mechanism of its own, so a vlt project patches a dependency only + /// through a postinstall tool such as patch-package. + pub(crate) async fn npm_family_in(view: &ProjectView<'_>, locks: &[(&str, &str)]) -> Self { + let mut index = Self::default(); + let manifest = Path::new("package.json"); + match index.read_view(view, "package.json").await { + Some(text) => { + if let Some(value) = index.parse_json(&text, manifest, "npm") { + index.patched_dependencies(&value["pnpm"]["patchedDependencies"], manifest); + index.yarn_manifest(&value, manifest); + index.list_patch_directory(view, "patches", false).await; + for dir in index.patch_package_dirs(manifest, &value) { + index.list_patch_directory(view, &dir, true).await; + } + } + } + None => index.list_patch_directory(view, "patches", false).await, + } + let workspace = "pnpm-workspace.yaml"; + if let Some(text) = index.read_view(view, workspace).await { + index.pnpm_yaml(&text, Path::new(workspace), false, false); + } + for (rel, text) in locks { + let path = Path::new(rel); + match rel.rsplit('/').next() { + Some("pnpm-lock.yaml" | "shrinkwrap.yaml") => { + index.pnpm_yaml(strip_bom(text), path, true, false); + } + Some("yarn.lock") => index.yarn_lock(text, path), + _ => {} + } + } + index + } + + /// Whether the index holds no evidence at all. + pub(crate) fn is_empty(&self) -> bool { + self.entries.is_empty() + } + + /// [`Self::read`] through a project view. + async fn read_view(&mut self, view: &ProjectView<'_>, rel: &str) -> Option { + match view.read_text(rel).await { + Ok(text) => Some(strip_bom(&text).to_owned()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + // An in-memory link has no content to read: the engine refuses + // a project with a linked candidate file on its own, so it is + // not taken for unknown metadata, which would withhold every + // package ahead of that refusal. + Err(_) if view.is_symlink(rel) => None, + Err(e) => { + self.unknown( + "npm", + Path::new(rel), + &format!("cannot inspect patch metadata: {e}"), + ); + None + } + } + } + + /// [`Self::patch_directory`] through a project view. + async fn list_patch_directory(&mut self, view: &ProjectView<'_>, rel: &str, required: bool) { + let rel = rel.trim_start_matches("./").trim_end_matches('/'); + match view.list_dir(rel).await { + Ok(entries) => { + for entry in entries { + let path = Path::new(rel).join(&entry.name); + self.patch_file(&entry.name, &path, required); + } + } + Err(e) if !required && e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => self.unknown( + "npm", + Path::new(rel), + &format!("cannot inspect patch-package directory: {e}"), + ), + } + } + // Use the existing npm/Yarn glob grammar, with a bounded, link-free walk. // Positive patterns are a conservative union: a patch in an excluded member // may still protect the same release elsewhere, never silently lose it. @@ -435,19 +486,28 @@ impl UserPatchIndex { } fn native(&mut self, value: &Value, path: &Path) { - for field in [ - &value["patchedDependencies"], - &value["pnpm"]["patchedDependencies"], - ] { - if let Some(map) = field.as_object() { - for key in map.keys() { - let (name, version) = split_selector(key); - self.add("npm", name, version, path); - } - } else if !field.is_null() { - self.unknown("npm", path, "invalid patchedDependencies"); + self.patched_dependencies(&value["patchedDependencies"], path); + self.patched_dependencies(&value["pnpm"]["patchedDependencies"], path); + self.pnpm_lock_patches(value, path); + } + + /// A `patchedDependencies` map's keys (`name`, `name@version`, + /// `name@range`, or Bun's `name@`), as npm, Bun and pnpm + /// all spell them. + fn patched_dependencies(&mut self, field: &Value, path: &Path) { + if let Some(map) = field.as_object() { + for key in map.keys() { + let (name, version) = split_selector(key); + self.add("npm", name, version, path); } + } else if !field.is_null() { + self.unknown("npm", path, "invalid patchedDependencies"); } + } + + /// The pnpm lock entries pnpm patches: a `(patch_hash=…)` key suffix, + /// or an older lock's `patched: true`. + fn pnpm_lock_patches(&mut self, value: &Value, path: &Path) { // Older pnpm locks can carry patch hashes without a mirrored map. for field in ["packages", "snapshots"] { if let Some(map) = value[field].as_object() { @@ -463,6 +523,74 @@ impl UserPatchIndex { } } + /// The pnpm declarations of `pnpm-workspace.yaml` (`lock` false) or of + /// a pnpm lock (`lock` true): the top-level `patchedDependencies` map + /// (pnpm 10+'s workspace setting, and the copy every lock records) and + /// the lock's patched entries. A lock that never spells a patch is not + /// parsed. `any_map` also reads a `pnpm.patchedDependencies` map, which + /// only `package.json` defines. Returns the parsed documents. + fn pnpm_yaml(&mut self, text: &str, path: &Path, lock: bool, any_map: bool) -> Vec { + if lock + && !text.contains("patchedDependencies") + && !text.contains("patch_hash") + && !text.contains("patched:") + { + return Vec::new(); + } + match serde_saphyr::from_multiple_with_options::(text, lock_options()) { + Ok(documents) => { + for value in &documents { + self.patched_dependencies(&value["patchedDependencies"], path); + if any_map { + self.patched_dependencies(&value["pnpm"]["patchedDependencies"], path); + } + self.pnpm_lock_patches(value, path); + } + documents + } + Err(e) => { + self.unknown("npm", path, &format!("cannot inspect patch metadata: {e}")); + Vec::new() + } + } + } + + /// The yarn `patch:` descriptors a `package.json` declares, in its + /// dependency maps and `resolutions`. + fn yarn_manifest(&mut self, value: &Value, path: &Path) { + for field in [ + "dependencies", + "devDependencies", + "optionalDependencies", + "resolutions", + ] { + if let Some(map) = value[field].as_object() { + for spec in map.values().filter_map(Value::as_str) { + self.yarn(spec, path); + } + } + } + } + + /// The user `patch:` locators a yarn berry lock resolves (a classic + /// lock has none). + fn yarn_lock(&mut self, text: &str, path: &Path) { + // Only Berry has patch: locators. Every Berry lock carries builtin + // compat patches, so scan the resolution lines rather than parse + // what can be a very large lockfile, and visit only the few lines + // that can hold one. + for line in lines_containing(text, "@patch:") { + let Some(value) = line.trim_start().strip_prefix("resolution:") else { + continue; + }; + let value = value.trim().trim_matches('"'); + // A locator has an ident before @patch:. + if let Some((_, source)) = value.split_once("@patch:") { + self.yarn(&format!("patch:{source}"), path); + } + } + } + fn yarn(&mut self, spec: &str, path: &Path) { if !spec.starts_with("patch:") { return; @@ -479,12 +607,23 @@ impl UserPatchIndex { async fn patch_package(&mut self, dir: &Path, value: &Value) { self.patch_directory(&dir.join("patches"), false).await; + for patch_dir in self.patch_package_dirs(&dir.join("package.json"), value) { + self.patch_directory(&dir.join(patch_dir), true).await; + } + } + + /// The `--patch-dir` directories the `patch-package` scripts of the + /// `package.json` at `manifest` name, relative to its directory. A + /// script whose directory can't be read without running it protects + /// every npm package instead. + fn patch_package_dirs(&mut self, manifest: &Path, value: &Value) -> Vec { // Compiled once, and only for a project whose scripts run patch-package. static FLAG: LazyLock = LazyLock::new(|| { Regex::new(r#"--patch-dir(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))"#).unwrap() }); // `cd` as a command, not the tail of a word like `abcd`. static CD: LazyLock = LazyLock::new(|| Regex::new(r"(?:^|[\s;&|(])cd\s").unwrap()); + let mut dirs = Vec::new(); if let Some(scripts) = value["scripts"].as_object() { for script in scripts .values() @@ -499,7 +638,7 @@ impl UserPatchIndex { if moved || (script.contains("--patch-dir") && !FLAG.is_match(script)) { self.unknown( "npm", - &dir.join("package.json"), + manifest, "cannot resolve patch-package directory without executing the script", ); } @@ -508,15 +647,16 @@ impl UserPatchIndex { if path.contains(['$', '`']) || moved { self.unknown( "npm", - &dir.join("package.json"), + manifest, "dynamic patch-package directory; inspect it before opting in", ); } else { - self.patch_directory(&dir.join(path), true).await; + dirs.push(path.to_string()); } } } } + dirs } async fn patch_directory(&mut self, dir: &Path, required: bool) { @@ -533,35 +673,40 @@ impl UserPatchIndex { } }; for entry in entries { - let entry = match entry { - Ok(e) => e, - Err(e) => { - self.unknown("npm", dir, &e.to_string()); - continue; - } - }; - let file = entry.file_name().to_string_lossy().into_owned(); - let Some(stem) = file.strip_suffix(".patch") else { - continue; - }; - let leaf = stem.rsplit("++").next().unwrap_or(stem); - let parts: Vec<_> = leaf.split('+').collect(); - let scoped = leaf.starts_with('@'); - let version_idx = if scoped { 2 } else { 1 }; - if let Some(version) = parts.get(version_idx) { - let version = version.trim_end_matches(".dev"); - let name = if scoped { - format!("{}/{}", parts[0], parts[1]) - } else { - parts[0].to_string() - }; - self.add("npm", &name, Some(version), &entry.path()); - } else if required { - self.unknown("npm", &entry.path(), "unrecognized patch-package filename"); + match entry { + Ok(entry) => self.patch_file( + &entry.file_name().to_string_lossy(), + &entry.path(), + required, + ), + Err(e) => self.unknown("npm", dir, &e.to_string()), } } } + /// One file of a patch-package directory, named + /// `[++]+[.dev].patch` (`@scope+name+…` scoped). + fn patch_file(&mut self, file: &str, path: &Path, required: bool) { + let Some(stem) = file.strip_suffix(".patch") else { + return; + }; + let leaf = stem.rsplit("++").next().unwrap_or(stem); + let parts: Vec<_> = leaf.split('+').collect(); + let scoped = leaf.starts_with('@'); + let version_idx = if scoped { 2 } else { 1 }; + if let Some(version) = parts.get(version_idx) { + let version = version.trim_end_matches(".dev"); + let name = if scoped { + format!("{}/{}", parts[0], parts[1]) + } else { + parts[0].to_string() + }; + self.add("npm", &name, Some(version), path); + } else if required { + self.unknown("npm", path, "unrecognized patch-package filename"); + } + } + async fn composer(&mut self, dir: &Path) { let manifest = dir.join("composer.json"); let Some(value) = self.json(&manifest, "composer").await else { @@ -1240,4 +1385,111 @@ mod tests { assert!(index.conflicts("pkg:pypi/anything@1.0.0").is_empty()); } } + + /// The view-based index the hosted engine and `vex` discovery read + /// ([`UserPatchIndex::npm_family_in`]) protects exactly what the CLI's + /// consent gate ([`UserPatchIndex::discover`]) protects for a root + /// project's pnpm, yarn and patch-package declarations, so hosted, + /// vendored and discovery never disagree about one, in memory and on + /// disk. npm's and Bun's own top-level `patchedDependencies` are the + /// gate's alone: their rewriters and extractors judge them per + /// resolution. + #[tokio::test] + async fn npm_family_view_index_agrees_with_the_consent_gate() { + use crate::vendor::lock_inventory::{MemoryEntry, MemoryProject}; + + const LOCK: &str = "pnpm-lock.yaml"; + let pnpm_lock = "lockfileVersion: '9.0'\n\npatchedDependencies:\n left-pad@1.3.0:\n hash: abc\n path: patches/left-pad.patch\n\npackages:\n left-pad@1.3.0:\n resolution: {integrity: sha512-x}\n\nsnapshots:\n left-pad@1.3.0(patch_hash=abc): {}\n"; + let berry_lock = "__metadata:\n version: 8\n\n\"left-pad@patch:left-pad@npm%3A1.3.0#./.yarn/patches/lp.patch\":\n version: 1.3.0\n resolution: \"left-pad@patch:left-pad@npm%3A1.3.0#./.yarn/patches/lp.patch::version=1.3.0&hash=abc\"\n"; + let cases: &[(&[(&str, &str)], bool)] = &[ + (&[(LOCK, pnpm_lock)], true), + (&[("yarn.lock", berry_lock)], true), + ( + &[( + "pnpm-workspace.yaml", + "patchedDependencies:\n left-pad: p.patch\n", + )], + true, + ), + ( + &[( + "package.json", + r#"{"pnpm":{"patchedDependencies":{"left-pad@^1":"p.patch"}}}"#, + )], + true, + ), + ( + &[( + "package.json", + r#"{"resolutions":{"left-pad":"patch:left-pad@npm%3A1.3.0#./p.patch"}}"#, + )], + true, + ), + (&[("patches/left-pad+1.3.0.patch", "diff")], true), + ( + &[ + ( + "package.json", + r#"{"scripts":{"postinstall":"patch-package --patch-dir fixes"}}"#, + ), + ("fixes/left-pad+1.3.0.patch", "diff"), + ], + true, + ), + (&[("patches/left-pad+1.2.0.patch", "diff")], false), + ( + &[( + "package.json", + r#"{"resolutions":{"x":"patch:x@npm%3A1.0.0#~/.yarn/patches/x.patch"}}"#, + )], + false, + ), + (&[("package.json", "{broken json")], true), + ]; + for (files, protected) in cases { + let tmp = tempfile::tempdir().unwrap(); + let mut memory = MemoryProject::new(); + for (path, text) in *files { + write(tmp.path(), path, text); + memory.insert(*path, MemoryEntry::Text((*text).into())); + } + let gate = UserPatchIndex::discover(tmp.path()).await; + let locks: Vec<(&str, &str)> = files + .iter() + .copied() + .filter(|(p, _)| *p == LOCK || *p == "yarn.lock") + .collect(); + for view in [ProjectView::Disk(tmp.path()), ProjectView::Memory(&memory)] { + let index = UserPatchIndex::npm_family_in(&view, &locks).await; + for purl in ["pkg:npm/left-pad@1.3.0", "pkg:npm/other@1.0.0"] { + let want = + *protected && (purl.contains("left-pad") || files[0].1 == "{broken json"); + assert_eq!( + !gate.conflicts(purl).is_empty(), + want, + "gate {files:?} {purl}" + ); + assert_eq!( + !index.conflicts(purl).is_empty(), + want, + "{files:?} {purl}: {index:?}" + ); + } + } + } + // npm's and Bun's own map: the gate's alone. + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "package.json", + r#"{"patchedDependencies":{"left-pad@1.3.0":"p.patch"}}"#, + ); + let purl = "pkg:npm/left-pad@1.3.0"; + assert!(!UserPatchIndex::discover(tmp.path()) + .await + .conflicts(purl) + .is_empty()); + let index = UserPatchIndex::npm_family_in(&ProjectView::Disk(tmp.path()), &[]).await; + assert!(index.is_empty(), "{index:?}"); + } } diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 0f3360d31..e6178efec 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -214,6 +214,7 @@ pub(crate) mod sbt; pub use sbt::{ DIAG_SBT_OWNED_FILE_MODIFIED, DIAG_SBT_RESOLUTION_UNVERIFIED, DIAG_VENDORED_TREE_MISSING, }; +pub(crate) mod user_patched; pub(crate) mod vlt; pub(crate) mod yarn; @@ -611,12 +612,15 @@ pub enum UnattestedKind { /// from its own lock, which no wiring reaches, so hosted mode withholds /// these locks and vendored mode refuses the project. PnpmGitBranchLock, - /// An npm or Bun pin of a package the project patches itself - /// (`patchedDependencies`, npm's lock-entry `patched` record; npm - /// #711, `bun patch` #367): the package manager applies the user's - /// diff on top of the locked tarball (npm) or keys it to another - /// resolution (Bun), so hosted and vendored mode leave the package - /// unwired and never confirm it. + /// An npm-family pin of a package the project patches itself: npm's + /// and Bun's `patchedDependencies` and npm's lock-entry `patched` + /// record (npm #711, `bun patch` #367), pnpm's `patchedDependencies`, + /// a yarn berry `patch:` descriptor, or a patch-package patch + /// ([`user_patched`]). The package manager applies the user's diff on + /// top of the locked tarball (npm, pnpm, yarn, patch-package) or keys + /// it to another resolution (Bun), so the install never runs the + /// Socket patch alone; hosted and vendored mode leave the package + /// unwired unless the user explicitly consents. UserPatchedDependency, } @@ -1537,6 +1541,7 @@ async fn discover_with_ctx(mut ctx: DiscoverCtx<'_>) -> Discovery { yarn::extract(&ctx, &mut out).await; bun::extract(&ctx, &mut out).await; vlt::extract(&ctx, &mut out).await; + user_patched::extract(&ctx, &mut out).await; ctx.ecosystem = "cargo"; cargo::extract(&ctx, &mut out).await; ctx.ecosystem = "golang"; diff --git a/crates/socket-patch-core/src/vex/discover/user_patched.rs b/crates/socket-patch-core/src/vex/discover/user_patched.rs new file mode 100644 index 000000000..8e1cace53 --- /dev/null +++ b/crates/socket-patch-core/src/vex/discover/user_patched.rs @@ -0,0 +1,343 @@ +//! npm-family packages the project patches itself with pnpm, yarn or +//! patch-package. +//! +//! pnpm's `patchedDependencies` (in `package.json`'s `pnpm` field, in +//! `pnpm-workspace.yaml`, and recorded in every pnpm lock's own +//! `patchedDependencies` map and `(patch_hash=…)` key suffixes), a yarn +//! berry `patch:` descriptor (in `package.json`, and resolved in +//! `yarn.lock`), and a patch-package patch (`patches/+.patch`, +//! or a `--patch-dir` a script names) are each applied by the package +//! manager, or its postinstall, on top of whatever tarball the lock names: +//! a Socket pin of such a package installs the Socket patch with the +//! user's diff stacked on it (or fails the install when the diff no longer +//! applies), never the Socket patch alone. Yarn classic has no patch +//! protocol, and neither has vlt (its `vlt.json` `modifiers` rewrite +//! dependency specs, never package contents), so a yarn classic or vlt +//! project patches a dependency only through patch-package, which is read +//! for every npm-family lock alike. +//! +//! So a ref of such a package is marked +//! [`UnattestedKind::UserPatchedDependency`]: it stays a ref (`rollback` / +//! `remove` / `list` still find a pin an earlier run, or an +//! `--allow-patch-conflicts` run, wrote), and only `vex` omits it, whatever +//! wiring the ref is and whether or not the run that wrote it had consent. +//! The evidence is [`UserPatchIndex::npm_family_in`], the parse rules of the +//! CLI's consent gate that keeps hosted and vendored mode from wiring these +//! packages, and the one the in-memory hosted engine withholds them by, so +//! the three never disagree. npm's and Bun's own `patchedDependencies` +//! are judged per resolution by their extractors instead (#711, #367). + +use super::{DiscoverCtx, Discovery, UnattestedKind}; +use crate::patch::user_patches::UserPatchIndex; + +pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { + if !out.refs.iter().any(|r| r.purl.starts_with("pkg:npm/")) { + return; + } + let mut lock_files: Vec = out + .refs + .iter() + .filter(|r| r.purl.starts_with("pkg:npm/")) + .map(|r| r.source_file.to_string_lossy().replace('\\', "/")) + .filter(|f| is_patch_lock(f)) + .collect(); + lock_files.extend(["pnpm-lock.yaml".to_string(), "yarn.lock".to_string()]); + lock_files.sort(); + lock_files.dedup(); + let mut locks: Vec<(String, String)> = Vec::new(); + for rel in lock_files { + if let Some(text) = ctx.read_advisory_text(&rel).await { + locks.push((rel, text)); + } + } + let locks: Vec<(&str, &str)> = locks + .iter() + .map(|(rel, text)| (rel.as_str(), text.as_str())) + .collect(); + let index = UserPatchIndex::npm_family_in(&ctx.view, &locks).await; + if index.is_empty() { + return; + } + let marks: Vec<(String, String, String, String)> = out + .refs + .iter() + .filter(|r| r.purl.starts_with("pkg:npm/")) + .filter(|r| { + !out.unattested.iter().any(|u| { + u.kind == UnattestedKind::UserPatchedDependency + && u.purl == r.purl + && u.uuid == r.uuid + }) + }) + .filter_map(|r| { + let sources = index.conflicts(&r.purl); + if sources.is_empty() { + return None; + } + let detail = format!( + "{} declares a patch the project applies to this package itself (pnpm \ + `patchedDependencies`, a yarn `patch:` descriptor, or a patch-package patch); \ + the package manager applies it on top of whatever tarball the lock names, so \ + the install never runs the Socket patch alone, and hosted and vendored mode \ + leave the package alone without `--allow-patch-conflicts`", + sources.join(", ") + ); + Some(( + r.purl.clone(), + r.uuid.clone(), + r.source_file.to_string_lossy().into_owned(), + detail, + )) + }) + .collect(); + for (purl, uuid, file, detail) in marks { + out.unattested( + &purl, + &uuid, + &file, + detail, + UnattestedKind::UserPatchedDependency, + ); + } +} + +/// A lock whose own records can declare a user patch: a pnpm lock (root, +/// Rush or a member's) or a yarn lock. +fn is_patch_lock(rel: &str) -> bool { + matches!( + rel.rsplit('/').next(), + Some("pnpm-lock.yaml" | "shrinkwrap.yaml" | "yarn.lock") + ) +} + +#[cfg(test)] +mod tests { + use super::super::testing::*; + use super::super::*; + + const LEFT_PAD: &str = "pkg:npm/left-pad@1.3.0"; + + /// The `(purl, uuid)` of every ref marked user-patched. + fn marked(out: &Discovery) -> Vec<(String, String)> { + out.unattested + .iter() + .filter(|u| u.kind == UnattestedKind::UserPatchedDependency) + .map(|u| (u.purl.clone(), u.uuid.clone())) + .collect() + } + + /// Every ref of `out` (at least one) is marked, and only once. + fn all_marked(out: &Discovery, what: &str) { + assert!(!out.refs.is_empty(), "{what}: no refs"); + let mut want: Vec<(String, String)> = out + .refs + .iter() + .map(|r| (r.purl.clone(), r.uuid.clone())) + .collect(); + want.sort(); + want.dedup(); + assert_eq!(marked(out), want, "{what}: {:#?}", out.unattested); + } + + fn none_marked(out: &Discovery, what: &str) { + assert!(!out.refs.is_empty(), "{what}: no refs"); + assert_eq!(marked(out), [], "{what}: {:#?}", out.unattested); + } + + /// REGRESSION: a pnpm pin of a package the project patches itself with + /// pnpm's `patchedDependencies` was attested, though pnpm applies the + /// user's diff on top of the pinned tarball. Every place pnpm reads or + /// records the patch counts: `pnpm-workspace.yaml` (pnpm 10+), + /// `package.json`'s `pnpm` field, the lock's own `patchedDependencies` + /// map, and a `(patch_hash=…)` snapshot key; another version's key and + /// npm's top-level `patchedDependencies` (which pnpm never reads) do + /// not. + #[tokio::test] + async fn pnpm_patched_dependencies_mark_the_pin() { + let fixture = "redirect/npm/pnpm/basic/expected"; + let p = Project::new(); + p.copy_fixture(fixture); + none_marked(&p.discover().await, "control"); + let lock = std::fs::read_to_string(p.root().join("pnpm-lock.yaml")).unwrap(); + let with_map = lock.replacen( + "\nimporters:", + "\npatchedDependencies:\n left-pad@1.3.0:\n hash: abc123\n path: patches/left-pad@1.3.0.patch\n\nimporters:", + 1, + ); + let with_hash = lock.replacen( + " left-pad@1.3.0: {}", + " left-pad@1.3.0(patch_hash=abc123): {}", + 1, + ); + assert!( + with_map != lock && with_hash != lock, + "the lock edits apply" + ); + + for (file, text, hit) in [ + ( + "pnpm-workspace.yaml", + "packages:\n - '.'\npatchedDependencies:\n left-pad@1.3.0: patches/left-pad@1.3.0.patch\n", + true, + ), + ( + "pnpm-workspace.yaml", + "patchedDependencies:\n left-pad: patches/left-pad.patch\n", + true, + ), + ( + "pnpm-workspace.yaml", + "patchedDependencies:\n left-pad@^1.0.0: patches/left-pad.patch\n", + true, + ), + ( + "pnpm-workspace.yaml", + "patchedDependencies:\n left-pad@1.2.0: patches/left-pad.patch\n", + false, + ), + ( + "package.json", + r#"{"pnpm":{"patchedDependencies":{"left-pad@1.3.0":"patches/left-pad.patch"}}}"#, + true, + ), + ( + "package.json", + r#"{"patchedDependencies":{"left-pad@1.3.0":"patches/left-pad.patch"}}"#, + false, + ), + ("pnpm-lock.yaml", with_map.as_str(), true), + ("pnpm-lock.yaml", with_hash.as_str(), true), + ] { + let p = Project::new(); + p.copy_fixture(fixture); + p.write(file, text); + let out = p.discover().await; + if hit { + all_marked(&out, text); + let detail = &out.unattested[0].detail; + assert!(detail.contains(file), "{detail}"); + } else { + none_marked(&out, text); + } + } + } + + /// REGRESSION: a yarn berry pin of a package the project patches with a + /// `patch:` descriptor (`yarn patch`) was attested. The descriptor in + /// `package.json` and the `patch:` locator `yarn.lock` resolves both + /// count, including one wrapping the hosted tarball itself (a `yarn + /// patch` made after the rewire, #962): yarn applies the user's diff on + /// top of it. Yarn's builtin compatibility patches do not. + #[tokio::test] + async fn yarn_berry_patch_descriptors_mark_the_pin() { + let fixture = "redirect/npm/yarn-berry/basic/expected"; + let p = Project::new(); + p.copy_fixture(fixture); + none_marked(&p.discover().await, "control"); + let manifest = std::fs::read_to_string(p.root().join("package.json")).unwrap(); + let lock = std::fs::read_to_string(p.root().join("yarn.lock")).unwrap(); + let url = "https://patch.socket.dev/patch/npm/11111111-1111-1111-1111-111111111111/77777777-7777-7777-7777-777777777777/left-pad-1.3.0.tgz"; + let patched_entry = |patch: &str| { + format!( + "\n\"left-pad@patch:left-pad@npm%3A1.3.0#{patch}\":\n version: 1.3.0\n resolution: \"left-pad@patch:left-pad@npm%3A1.3.0#{patch}::version=1.3.0&hash=abc123\"\n languageName: node\n linkType: hard\n" + ) + }; + let wrapping = manifest.replacen( + "\"left-pad\": \"^1.3.0\"", + &format!( + "\"left-pad\": \"patch:left-pad@{}#./.yarn/patches/left-pad.patch\"", + url.replace(':', "%3A") + ), + 1, + ); + assert_ne!(wrapping, manifest, "the manifest edit applies"); + for (file, text, hit) in [ + ("package.json", wrapping, true), + ( + "yarn.lock", + format!( + "{lock}{}", + patched_entry("./.yarn/patches/left-pad-npm-1.3.0-abc.patch") + ), + true, + ), + ( + "yarn.lock", + format!( + "{lock}{}", + patched_entry("optional!builtin") + ), + false, + ), + ] { + let p = Project::new(); + p.copy_fixture(fixture); + p.write(file, &text); + let out = p.discover().await; + if hit { + all_marked(&out, &text); + } else { + none_marked(&out, &text); + } + } + } + + /// REGRESSION: yarn classic and vlt have no patch mechanism of their + /// own, so a project patches a dependency there with patch-package, a + /// postinstall that applies `patches/+.patch` (or a + /// `--patch-dir` a script names) over the installed copy. A pin of such + /// a package (hosted or vendored) was attested; npm-lock pins likewise. + /// Another version's patch file does not count. + #[tokio::test] + async fn patch_package_patches_mark_yarn_classic_vlt_and_npm_pins() { + for fixture in [ + "redirect/npm/yarn-classic/basic/expected", + "redirect/npm/vlt/basic/expected", + "redirect/npm/vlt/vendored-entry/input", + "redirect/npm/package-lock-v3/basic/expected", + ] { + let p = Project::new(); + p.copy_fixture(fixture); + none_marked(&p.discover().await, fixture); + + p.write("patches/left-pad+1.2.0.patch", "diff\n"); + none_marked(&p.discover().await, fixture); + + p.write("patches/left-pad+1.3.0.patch", "diff\n"); + let out = p.discover().await; + all_marked(&out, fixture); + assert!( + out.unattested[0] + .detail + .contains("patches/left-pad+1.3.0.patch"), + "{}", + out.unattested[0].detail + ); + + // A custom directory a patch-package script names. + let p = Project::new(); + p.copy_fixture(fixture); + p.write( + "package.json", + r#"{"scripts":{"postinstall":"patch-package --patch-dir fixes"}}"#, + ); + p.write("fixes/left-pad+1.3.0.patch", "diff\n"); + all_marked(&p.discover().await, fixture); + } + } + + /// The pins of other packages stay attested. + #[tokio::test] + async fn other_packages_are_not_marked() { + let p = Project::new(); + p.copy_fixture("redirect/npm/pnpm/basic/expected"); + p.write( + "pnpm-workspace.yaml", + "patchedDependencies:\n is-number@7.0.0: patches/is-number.patch\n", + ); + p.write("patches/is-number+7.0.0.patch", "diff\n"); + let out = p.discover().await; + assert!(out.refs.iter().any(|r| r.purl == LEFT_PAD)); + none_marked(&out, "other package"); + } +} diff --git a/docs/usage.md b/docs/usage.md index 9a6a117ae..bf3753018 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -266,6 +266,15 @@ consent permits replacing a mismatched file with Socket's verified content; it does not merge your changes. Without consent, mismatched files fail by default, unless they hold an earlier Socket patch's content (a patch update). `--yes` and `--force` do not approve combining declared project patches. Some -package-manager combinations remain unsupported even with consent. See +package-manager combinations remain unsupported even with consent: a package +patched with npm's or Bun's own `patchedDependencies` stays unwired in hosted +mode and is refused in vendored mode. +With consent, hosted and vendored mode do wire a package patched with pnpm's +`patchedDependencies`, a Yarn `patch:` descriptor or `patch-package` (Yarn +classic and vlt have no patch mechanism of their own, so `patch-package` is how +those projects patch). The package manager then applies your patch on top of +Socket's, so `vex` never attests that package while your patch is declared. +The in-memory hosted engine has no consent option and always skips such a +package (`user_patch_conflict`). See [existing project patches](ecosystems.md#existing-project-patches) for the mechanisms checked and the limitations. From 3f076f67e0efaaf77635064d7530c2b905ea0900 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 12:04:25 -0600 Subject: [PATCH 2/4] Read member patches and stop attesting Bun patches on the pin 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. The shared npm / Bun helper (npm::user_patch_source, behind mark_user_patched) now reads the keys against no target (any key for the name@version counts) and flags the case where only a key for the ref's own tarball matches, which gets its own detail; npm and Bun refs alike. 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. Contest survivors only. Like #2010's npm / Bun marks, the pnpm / yarn / patch-package pass now records each mark as an UnwiredCopy for exactly that ref (CopyTarget::Ref), so it lands only on a ref that survives the lock contests and a ref an earlier extractor already marked keeps that first reason; a_contested_user_patched_ref_leaves_no_mark covers it. Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 14 +- .../src/patch/user_patches.rs | 654 ++++++++++++------ .../socket-patch-core/src/vex/discover/bun.rs | 35 +- .../socket-patch-core/src/vex/discover/npm.rs | 31 +- .../src/vex/discover/user_patched.rs | 193 +++++- 5 files changed, 659 insertions(+), 268 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 298671ed9..4800df280 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -176,7 +176,8 @@ A missing referenced diff does not remove that protection. reverts entries dropped from the manifest first. A hosted or vendored writer rechecks under its apply lock before rewiring. - The hosted engine applies the same rules to npm-family packages on its own (pnpm `patchedDependencies`, a Yarn `patch:` - descriptor, a `patch-package` file, read at the project root and in its pnpm, Rush and yarn locks): a matching candidate is + descriptor, a `patch-package` file, read by the consent gate's walk over the project and its workspace members, and in + its pnpm, Rush and yarn locks): a matching candidate is withheld from every rewriter and reported as a `skipped` event / `skipped[]` row with reason `user_patch_conflict`. The in-memory engine has no consent option, so it always skips them and adds a `user_patch_conflict` warning; `--allow-patch-conflicts` lifts the disk flow's check. With consent the package is wired under the user's patch, which the @@ -204,7 +205,14 @@ A missing referenced diff does not remove that protection. `patch:` descriptor to be replaced by the registry range first. Detection covers root and declared npm workspace manifests, ancestor workspace -settings, and the following evidence: +settings, and the following evidence. Nothing outside the git repository root +(the nearest ancestor of the project, inclusive, holding `.git`) is read: the +ancestor walk stops there, and a workspace member or literal `--patch-dir` +beyond it protects every npm package with a `workspace outside the git +repository` / `patch-package directory outside the git repository` diagnostic +instead of being read. Without a `.git`, the ancestor walk stops below the home +directory. The hosted engine and `vex` run this same walk (npm-family rows only, +without native npm / Bun); in memory the project is its own repository root. | Mechanism | Evidence | | --- | --- | @@ -554,7 +562,7 @@ Recognition rules that hold for every ecosystem: * **Patch hosts.** A hosted reference counts only on `https://patch.socket.dev` or the `--patch-server-url` / `SOCKET_PATCH_SERVER_URL` origin, with no userinfo. The uuid is the URL's LAST canonical-uuid path segment, because grant tokens may themselves be uuid-shaped. The Go module prefix is fixed. `socket-patch-` registry / repository / source names count only through a pin. For a URL on any other host, see **Patch hosts** above. * **Pins, not definitions.** A registry, index or source *definition* alone (cargo `[registries]`, nuget ``, pom ``, uv index tables, `.npmrc`) never makes a reference, because it survives a reverted pin. Sections the package manager ignores are not read: npm's v2 `dependencies` mirror, a `.cargo/config.toml` shadowed by `.cargo/config`. A Socket pin inside a maven `` is diagnosed, never a reference. * **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. The root `requirements.txt` and its in-root `-r` includes count as one lock here: pip reads them as one requirement set, where a direct reference wins over a compatible `==` pin of the same version in another file of the set (#1086). A PyPI version matches under PEP 440 equality, so a requirements line pinned in any spelling that can only install the wired release (`==1.2.3.0`, `==v1.2.3`, `===1.2.3`) contests it like `==1.2.3` does; a range or wildcard (`~=1.2.3`, `==1.2.*`) does not (#1602). A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`, beneath a dependency that bundles it; the root project's own bundles install from the lock, #1618) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). pnpm unpacks bundled copies too, but its lock cannot tie one to a reference (see **Unattested references** below). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. yarn classic's `yarn.lock` records nothing about bundled copies either (#758), so for a lock with a Socket wiring they are found in the installed `node_modules`: each package a parent's `bundleDependencies` / `bundledDependencies` names inside the parent's own `node_modules`, the dependencies of those beside them, and everything nested under them. Such a copy contests the reference in `yarn.lock` (the diagnostic names the parent's lock entry and the copy's directory); a lock-only `vex` with no `node_modules` can't see it. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`, and a yarn classic hosted scan names one with `redirect_copy_unpatched`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. A reference withheld only because such an unreachable copy installs beside it (a bundled npm / Bun / vlt copy, an npm copy beneath a `hasShrinkwrap` package (#753) or one npm installs from a git / url / `file:` spec, or a yarn classic git or `file:` directory block of the same `name@version`) is still the rewriter's own wiring of exactly one package version: it is never attested, but `rollback`, `remove`, `list` and the hosted → vendored takeover treat it as an ordinary hosted pin and restore its upstream registry entry (#828), rather than refusing it with `hosted_wiring_contested`. Another entry of the **same** npm, Bun or yarn lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install` / `yarn install`; for yarn berry, a registry locator such as `left-pad@npm:1.3.0` beside the hosted `…::__archiveUrl=` one) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy. -* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Six cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **pnpm branch locks** (`vex_pnpm_git_branch_lockfile`): with `gitBranchLockfile` on, pnpm installs a git branch from its own `pnpm-lock..yaml` (the **pnpm per-branch locks** state above), which no wiring reaches. `vex` does not decide whether the setting is on: the presence of any branch lock (at the root, or in a workspace member that installs from its own lock under `sharedWorkspaceLockfile: false`, read from the project's files and never from the environment) omits every reference of the root pnpm lock (`pnpm-lock.yaml`, or `shrinkwrap.yaml` without it) and of each member's own lock, hosted and vendored alike, whatever `gitBranchLockfile` or pnpm's `lockfile` setting says (#556; deterministic and fail-closed). This is checked only when a root or member pnpm lock wires a patch. Hosted mode refuses every pnpm pin when the setting is on and vendored mode refuses the project, so this also covers wiring written before the branch lock appeared. The run warning names the branch lock in the marked lock's own directory. Rush locks are not affected. **Self-patched npm and Bun packages** (`vex_npm_patched_dependency`): a reference in `package-lock.json` / `npm-shrinkwrap.json` / `bun.lock` / `bun.lockb` to a package the project patches itself is omitted, by the hosted rewriters' own per-dependency rule (`redirect_npm_patched_dependency_skipped`, #711; `redirect_bun_patched_dependency_skipped`, #367): a root `package.json` `patchedDependencies` key for its `name@version`, its bare name, or `name@` of a tarball other than the one the reference pins (npm and Bun alike, as both hosted gates read it); for npm also a non-null `patched` record on any present npm lock's entry for that `name@version`; for `bun.lock` also the keys it mirrors (the run warning then names `bun.lock` as the key's source). npm applies the user's diff on top of whatever tarball the lock names and Bun applies it only to the resolution it is keyed on, so both modes leave such a package unwired; this covers pins an earlier release wrote, and an npm one fails `npm ci` with `EPATCHFAILED` until `socket-patch rollback ` restores the registry entry (the remedy says so). A key naming the pinned tarball itself is no conflict; a reference with neither a hosted url nor a vendored tarball is matched by its `name@version` and counts every tarball key. Only a reference that survives the lock contests carries this reason. **Packages patched with pnpm, Yarn or patch-package** (`vex_npm_patched_dependency` too): a reference of any npm-family lock (pnpm, Rush, yarn classic and berry, vlt, npm, Bun; hosted or vendored) to a `name@version` the project patches itself is omitted, read from the project root: pnpm's `patchedDependencies` (`package.json`'s `pnpm.patchedDependencies`, `pnpm-workspace.yaml`'s `patchedDependencies`, and the `patchedDependencies` map or a `(patch_hash=…)` key suffix in the reference's own pnpm lock or the root one), a yarn berry `patch:` descriptor outside Yarn's builtin compatibility patches (`package.json`'s dependency maps and `resolutions`, or a `patch:` locator `yarn.lock` resolves, including one wrapping the hosted tarball), or a patch-package file (`patches/+.patch`, or under a `--patch-dir` a `package.json` script names). A key's version may be exact, a range or absent (every version), and metadata that cannot be read (an unparseable `package.json`, a `--patch-dir` the script computes) omits every npm reference. These are the parse rules of the `user_patch_conflict` consent gate (see [Existing project patches](#existing-project-patches)) without its workspace and ancestor walk, and the evidence the in-memory hosted engine withholds a package by. The package manager (or the postinstall) applies the user's diff on top of whatever tarball the lock names, so the install never runs the Socket patch alone: the reference is omitted even when an `--allow-patch-conflicts` run wired it, for as long as the patch is declared. Yarn classic and vlt have no patch mechanism of their own (vlt's `vlt.json` `modifiers` rewrite specs, not contents), so patch-package is what is read for them. npm's and Bun's own top-level `patchedDependencies` are judged by the rule above, not here. +* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Six cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **pnpm branch locks** (`vex_pnpm_git_branch_lockfile`): with `gitBranchLockfile` on, pnpm installs a git branch from its own `pnpm-lock..yaml` (the **pnpm per-branch locks** state above), which no wiring reaches. `vex` does not decide whether the setting is on: the presence of any branch lock (at the root, or in a workspace member that installs from its own lock under `sharedWorkspaceLockfile: false`, read from the project's files and never from the environment) omits every reference of the root pnpm lock (`pnpm-lock.yaml`, or `shrinkwrap.yaml` without it) and of each member's own lock, hosted and vendored alike, whatever `gitBranchLockfile` or pnpm's `lockfile` setting says (#556; deterministic and fail-closed). This is checked only when a root or member pnpm lock wires a patch. Hosted mode refuses every pnpm pin when the setting is on and vendored mode refuses the project, so this also covers wiring written before the branch lock appeared. The run warning names the branch lock in the marked lock's own directory. Rush locks are not affected. **Self-patched npm and Bun packages** (`vex_npm_patched_dependency`): a reference in `package-lock.json` / `npm-shrinkwrap.json` / `bun.lock` / `bun.lockb` to a package the project patches itself is omitted, by the hosted rewriters' own per-dependency rule (`redirect_npm_patched_dependency_skipped`, #711; `redirect_bun_patched_dependency_skipped`, #367): a root `package.json` `patchedDependencies` key for its `name@version`, its bare name, or `name@` of any tarball (npm and Bun alike); for npm also a non-null `patched` record on any present npm lock's entry for that `name@version`; for `bun.lock` also the keys it mirrors (the run warning then names `bun.lock` as the key's source). npm applies the user's diff on top of whatever tarball the lock names and Bun applies it only to the resolution it is keyed on, so both modes leave such a package unwired; this covers pins an earlier release wrote, and an npm one fails `npm ci` with `EPATCHFAILED` until `socket-patch rollback ` restores the registry entry (the remedy says so). A key naming the pinned tarball itself (a patch made on top of the pin) is no conflict for the hosted rewriters, which keep the pin, but the reference is omitted all the same: the user's diff runs over the Socket bytes, never the Socket patch alone; a reference with neither a hosted url nor a vendored tarball is matched by its `name@version` and counts every tarball key. Only a reference that survives the lock contests carries this reason. **Packages patched with pnpm, Yarn or patch-package** (`vex_npm_patched_dependency` too): a reference of any npm-family lock (pnpm, Rush, yarn classic and berry, vlt, npm, Bun; hosted or vendored) to a `name@version` the project patches itself is omitted, read from the project, its declared workspace members and (on disk) the ancestor workspace settings up to the git repository root, by the consent gate's own walk: pnpm's `patchedDependencies` (`package.json`'s `pnpm.patchedDependencies`, `pnpm-workspace.yaml`'s `patchedDependencies`, and the `patchedDependencies` map or a `(patch_hash=…)` key suffix in the reference's own pnpm lock or the root one), a yarn berry `patch:` descriptor outside Yarn's builtin compatibility patches (`package.json`'s dependency maps and `resolutions`, or a `patch:` locator `yarn.lock` resolves, including one wrapping the hosted tarball), or a patch-package file (`patches/+.patch`, or under a `--patch-dir` a `package.json` script names). A key's version may be exact, a range or absent (every version), and metadata that cannot be read (an unparseable `package.json`, a `--patch-dir` the script computes) omits every npm reference. This is the `user_patch_conflict` consent gate's evidence (see [Existing project patches](#existing-project-patches)) minus native npm / Bun and the other ecosystems, and the evidence the in-memory hosted engine withholds a package by. The package manager (or the postinstall) applies the user's diff on top of whatever tarball the lock names, so the install never runs the Socket patch alone: the reference is omitted even when an `--allow-patch-conflicts` run wired it, for as long as the patch is declared. Yarn classic and vlt have no patch mechanism of their own (vlt's `vlt.json` `modifiers` rewrite specs, not contents), so patch-package is what is read for them. npm's and Bun's own top-level `patchedDependencies` are judged by the rule above, not here. * **Shrinkwrap without a twin (#899).** npm >= 12 never reads `npm-shrinkwrap.json`: on a project whose only npm lock is the shrinkwrap it resolves the tree from the registry and writes a fresh `package-lock.json`, so a Socket reference there reaches npm <= 11 only. The reference is still discovered, so `list`, `rollback` and `remove` manage it, but `vex` omits the patch (`vex_npm_shrinkwrap_only`, as a run warning and as the `failed[].reason`). A `package-lock.json` beside it (wired the same way) makes it attestable again. Hosted and vendored runs still rewire the shrinkwrap and warn `redirect_npm_shrinkwrap_only` / `vendor_npm_shrinkwrap_only`. * **Lockless pins.** With no lock to name a version, a `Cargo.toml` pin (every declaration on `socket-patch-`, that registry defined on the patch host for the same uuid) or an exclusive nuget exact-id mapping is never a reference on its own, so v5.0 does not attest it (nor does `list` show it, or `rollback` / `remove` restore it — restore those files from version control). Only a pre-v5 redirect-ledger record naming a version the pin admits keeps it live. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged). diff --git a/crates/socket-patch-core/src/patch/user_patches.rs b/crates/socket-patch-core/src/patch/user_patches.rs index 78a2efd1f..d7f172fa5 100644 --- a/crates/socket-patch-core/src/patch/user_patches.rs +++ b/crates/socket-patch-core/src/patch/user_patches.rs @@ -37,66 +37,182 @@ impl UserPatchIndex { /// settings. Errors in a potentially governing file protect its ecosystem; /// unreadable metadata must not silently disable the consent check. pub async fn discover(root: &Path) -> Self { - let mut index = Self::default(); let root = std::path::absolute(root).unwrap_or_else(|_| root.to_path_buf()); - let mut pending = VecDeque::from([root.clone()]); - // A member invocation still inherits the workspace's patch policy. - // The walk stops at the checkout's `.git`, and never reaches the home - // directory (whose `~/.cargo/config.toml` is user-wide, not a - // workspace) or above it, as in a container build with no `.git`. - let home = crate::utils::fs::home_dir(); - for parent in root - .ancestors() - .skip(1) - .take_while(|_| !root.join(".git").exists()) - .take_while(|parent| home.as_deref().is_none_or(|home| !home.starts_with(parent))) - { - if parent.join("pnpm-workspace.yaml").exists() - || parent.join("go.work").exists() - || parent.join(".cargo/config.toml").exists() - || parent.join(".cargo/config").exists() - || read_regular_to_string(&parent.join("package.json")) - .await - .is_ok_and(|s| s.contains("\"workspaces\"")) - || read_regular_to_string(&parent.join("Cargo.toml")) - .await - .is_ok_and(|s| s.contains("[workspace]")) - { - pending.push_back(parent.to_path_buf()); + Self::walk(&ProjectView::Disk(&root), Some(&root), Scope::Gate).await + } + + /// The patches a project's pnpm, yarn or patch-package setup applies + /// to its npm dependencies, read through `view` as the hosted engine + /// and `vex` discovery read the project, by the very walk of + /// [`Self::discover`] (the CLI's consent gate): the project, its + /// declared workspace members and (on disk) the ancestor workspace + /// settings up to the git repository root. Each directory's + /// `package.json` (`pnpm.patchedDependencies`, yarn `patch:` + /// descriptors, patch-package directories: `patches/` and every + /// `--patch-dir` its scripts name), `pnpm-workspace.yaml` + /// (`patchedDependencies`), pnpm lock (its `patchedDependencies` map and + /// `(patch_hash=…)` keys) and `yarn.lock` (user `patch:` locators) are + /// read, plus `locks` (root-relative path and text: a Rush or member + /// lock outside the walk). Left out: npm's and Bun's own top-level + /// `patchedDependencies` and npm's lock-entry `patched` records, which + /// no other package manager reads and which their own rewriters and + /// discovery judge per resolution (#711, #367), and the other + /// ecosystems' declarations. vlt has no patch mechanism of its own, so + /// a vlt project patches a dependency only through a postinstall tool + /// such as patch-package. Sources are reported root-relative. + pub(crate) async fn npm_family_in(view: &ProjectView<'_>, locks: &[(&str, &str)]) -> Self { + let mut index = Self::walk(view, None, Scope::NpmFamily).await; + for (rel, text) in locks { + let path = Path::new(rel); + match rel.rsplit('/').next() { + Some("pnpm-lock.yaml" | "shrinkwrap.yaml") => { + index.pnpm_yaml(strip_bom(text), path, true, false); + } + Some("yarn.lock") => index.yarn_lock(text, path), + _ => {} } - if parent.join(".git").exists() { - break; + } + index + } + + /// Whether the index holds no evidence at all. + pub(crate) fn is_empty(&self) -> bool { + self.entries.is_empty() + } + + /// The walk [`Self::discover`] and [`Self::npm_family_in`] share. Every + /// project file is read through `view` at its root-relative path (`..` + /// leading to an ancestor). `shown_root` is where reported sources are + /// anchored: the absolute project root for the gate, `None` for + /// root-relative sources. + /// + /// Nothing outside the git repository root is read: the ancestor walk + /// stops at the directory holding `.git`, and a workspace member or + /// patch-package directory beyond it protects every npm package + /// instead of being read. Without a `.git` the gate's container rule + /// holds (the walk never reaches the home directory or above it). An + /// in-memory project is its own repository root. + async fn walk(view: &ProjectView<'_>, shown_root: Option<&Path>, scope: Scope) -> Self { + let mut index = Self::default(); + let shown_base = shown_root.unwrap_or(Path::new("")); + let at = |rel: &Path| Dir { + rel: rel.to_path_buf(), + shown: lexical_join(shown_base, rel), + }; + let mut pending = VecDeque::from([at(Path::new(""))]); + // How many levels above the project root the repository root is; + // `None`: no repository root is known. + let mut repo_depth: Option = None; + if let Some(root) = view.disk_root_reading(std::iter::empty::<&Path>()) { + // A member invocation still inherits the workspace's patch + // policy. The walk stops at the checkout's `.git`, and never + // reaches the home directory (whose `~/.cargo/config.toml` is + // user-wide, not a workspace) or above it, as in a container + // build with no `.git`. + let root = std::path::absolute(root).unwrap_or_else(|_| root.to_path_buf()); + let home = crate::utils::fs::home_dir(); + let mut probes: Vec = vec![root.join(".git")]; + if root.join(".git").exists() { + repo_depth = Some(0); + } + for (depth, parent) in root.ancestors().enumerate().skip(1) { + if repo_depth.is_some() || home.as_deref().is_some_and(|h| h.starts_with(parent)) { + break; + } + const MARKERS: [&str; 7] = [ + "pnpm-workspace.yaml", + "go.work", + ".cargo/config.toml", + ".cargo/config", + "package.json", + "Cargo.toml", + ".git", + ]; + probes.extend(MARKERS.iter().map(|m| parent.join(m))); + // Any workspace marker brings the directory in, whichever + // scope reads it, so both scopes walk the same directories. + if parent.join("pnpm-workspace.yaml").exists() + || parent.join("go.work").exists() + || parent.join(".cargo/config.toml").exists() + || parent.join(".cargo/config").exists() + || read_regular_to_string(&parent.join("package.json")) + .await + .is_ok_and(|s| s.contains("\"workspaces\"")) + || read_regular_to_string(&parent.join("Cargo.toml")) + .await + .is_ok_and(|s| s.contains("[workspace]")) + { + let rel: std::path::PathBuf = std::iter::repeat_n("..", depth).collect(); + pending.push_back(Dir { + shown: shown_root.map_or_else(|| rel.clone(), |_| parent.to_path_buf()), + rel, + }); + } + if parent.join(".git").exists() { + repo_depth = Some(depth); + } } + // Fingerprint what the probes read for a snapshot's reuse. + let _ = view.disk_root_reading(&probes); + } else { + repo_depth = Some(0); } let mut seen = BTreeSet::new(); while let Some(dir) = pending.pop_front() { - if !seen.insert(dir.clone()) { + if !seen.insert(dir.rel.clone()) { continue; } if seen.len() > 4096 { - index.unknown("npm", &dir, "workspace inspection limit exceeded"); + index.unknown("npm", &dir.shown, "workspace inspection limit exceeded"); break; } + if outside_repo(&dir.rel, repo_depth) { + index.unknown( + "npm", + &dir.shown, + "workspace outside the git repository is not inspected", + ); + continue; + } let mut globs = Vec::new(); - if let Some(value) = index.json(&dir.join("package.json"), "npm").await { - index.native(&value, &dir.join("package.json")); - index.yarn_manifest(&value, &dir.join("package.json")); + let manifest = dir.file("package.json"); + if let Some(value) = index.json_view(view, &manifest).await { + if scope == Scope::Gate { + index.native(&value, &manifest.shown); + } else { + index.patched_dependencies( + &value["pnpm"]["patchedDependencies"], + &manifest.shown, + ); + } + index.yarn_manifest(&value, &manifest.shown); // A malformed field still names its string items (#1900): // walking them can only find more patches. globs.extend(Workspaces::of(&value).into_listed().into_iter().flatten()); - index.patch_package(&dir, &value).await; + index + .list_patch_directory(view, &dir.file("patches"), false, None) + .await; + for patch_dir in index.patch_package_dirs(&manifest.shown, &value) { + let patch_dir = dir.file(&patch_dir); + index + .list_patch_directory(view, &patch_dir, true, repo_depth) + .await; + } } else { - index.patch_directory(&dir.join("patches"), false).await; + index + .list_patch_directory(view, &dir.file("patches"), false, None) + .await; } - let yarn_lock = dir.join("yarn.lock"); - if let Some(text) = index.read(&yarn_lock, "npm").await { - index.yarn_lock(&text, &yarn_lock); + let yarn_lock = dir.file("yarn.lock"); + if let Some(text) = index.read_view(view, &yarn_lock, "npm").await { + index.yarn_lock(&text, &yarn_lock.shown); } for file in ["pnpm-workspace.yaml", "pnpm-lock.yaml"] { - let path = dir.join(file); - if let Some(text) = index.read(&path, "npm").await { + let path = dir.file(file); + if let Some(text) = index.read_view(view, &path, "npm").await { let lock = file == "pnpm-lock.yaml"; - for value in index.pnpm_yaml(&text, &path, lock, true) { + let any_map = scope == Scope::Gate; + for value in index.pnpm_yaml(&text, &path.shown, lock, any_map) { if !lock { if let Some(packages) = value["packages"].as_array() { globs.extend( @@ -107,132 +223,113 @@ impl UserPatchIndex { } } } - for file in ["package-lock.json", "npm-shrinkwrap.json", "bun.lock"] { - let path = dir.join(file); - // Like the pnpm lock: a lock that never says `patched` holds - // no patch evidence, so an unparseable one can't hide any. - let Some(text) = index - .read(&path, "npm") - .await - .filter(|text| text.contains("patched")) - else { - continue; - }; - if let Some(value) = index.parse_json(&text, &path, "npm") { - index.native(&value, &path); - if let Some(packages) = value["packages"].as_object() { - for (key, entry) in packages { - if !entry["patched"].is_null() { - let name = entry["name"].as_str().or_else(|| { - key.rsplit_once("node_modules/").map(|(_, name)| name) - }); - if let Some(name) = name { - index.add("npm", name, entry["version"].as_str(), &path); - } else { - index.unknown("npm", &path, "unidentified patched lock entry"); - } - } - } - } - } - } - index.composer(&dir).await; - index.cargo(&dir).await; - for file in ["go.mod", "go.work"] { - let path = dir.join(file); - if let Some(text) = index.read(&path, "golang").await { - for entry in crate::vendor::go_mod_edit::parse_replace_entries(&text) { - if !entry.socket_owned() { - index.add("golang", &entry.module, entry.version.as_deref(), &path); - } - } - // Workspace members' go.mod replaces still apply under go.work. - if file == "go.work" { - pending.extend( - crate::vendor::go_mod_edit::parse_use_dirs(&text) - .into_iter() - .map(|member| dir.join(member)), - ); - } - } + if scope == Scope::Gate { + index.gate_only(view, &dir, &mut pending, shown_base).await; } for pattern in globs .iter() .filter(|g| !g.starts_with('!') || g.starts_with("!(")) { - pending.extend(index.workspace_members(&dir, pattern)); + for rel in index + .workspace_members(view, &dir, pattern, repo_depth) + .await + { + pending.push_back(at(&rel)); + } } } index } - /// The patches a project's pnpm, yarn or patch-package setup applies - /// to its npm dependencies, declared at the project root and read - /// through `view` as the hosted engine and `vex` discovery read the - /// project: `package.json`'s `pnpm.patchedDependencies`, its yarn - /// `patch:` descriptors and its patch-package directories (`patches/` - /// and every `--patch-dir` its scripts name), `pnpm-workspace.yaml`'s - /// `patchedDependencies`, and of `locks` (root-relative path and text) - /// each pnpm lock's patches and each `yarn.lock`'s user `patch:` - /// locators. These are the parse rules [`Self::discover`] (the CLI's - /// consent gate) applies, without its workspace and ancestor walk, and - /// without npm's and Bun's own top-level `patchedDependencies`, which - /// no other package manager reads and which their own rewriters and - /// discovery judge per resolution (#711, #367). vlt has no patch - /// mechanism of its own, so a vlt project patches a dependency only - /// through a postinstall tool such as patch-package. - pub(crate) async fn npm_family_in(view: &ProjectView<'_>, locks: &[(&str, &str)]) -> Self { - let mut index = Self::default(); - let manifest = Path::new("package.json"); - match index.read_view(view, "package.json").await { - Some(text) => { - if let Some(value) = index.parse_json(&text, manifest, "npm") { - index.patched_dependencies(&value["pnpm"]["patchedDependencies"], manifest); - index.yarn_manifest(&value, manifest); - index.list_patch_directory(view, "patches", false).await; - for dir in index.patch_package_dirs(manifest, &value) { - index.list_patch_directory(view, &dir, true).await; + /// The declarations only the consent gate reads in each directory: + /// npm's and Bun's lock-entry `patched` records and mirrored maps, and + /// the Composer, Cargo and Go overrides (a `go.work`'s members join + /// the walk). + async fn gate_only( + &mut self, + view: &ProjectView<'_>, + dir: &Dir, + pending: &mut VecDeque, + shown_base: &Path, + ) { + for file in ["package-lock.json", "npm-shrinkwrap.json", "bun.lock"] { + let path = dir.file(file); + // Like the pnpm lock: a lock that never says `patched` holds + // no patch evidence, so an unparseable one can't hide any. + let Some(text) = self + .read_view(view, &path, "npm") + .await + .filter(|text| text.contains("patched")) + else { + continue; + }; + if let Some(value) = self.parse_json(&text, &path.shown, "npm") { + self.native(&value, &path.shown); + if let Some(packages) = value["packages"].as_object() { + for (key, entry) in packages { + if !entry["patched"].is_null() { + let name = entry["name"] + .as_str() + .or_else(|| key.rsplit_once("node_modules/").map(|(_, name)| name)); + if let Some(name) = name { + self.add("npm", name, entry["version"].as_str(), &path.shown); + } else { + self.unknown("npm", &path.shown, "unidentified patched lock entry"); + } + } } } } - None => index.list_patch_directory(view, "patches", false).await, - } - let workspace = "pnpm-workspace.yaml"; - if let Some(text) = index.read_view(view, workspace).await { - index.pnpm_yaml(&text, Path::new(workspace), false, false); } - for (rel, text) in locks { - let path = Path::new(rel); - match rel.rsplit('/').next() { - Some("pnpm-lock.yaml" | "shrinkwrap.yaml") => { - index.pnpm_yaml(strip_bom(text), path, true, false); + self.composer(&dir.shown).await; + self.cargo(&dir.shown).await; + for file in ["go.mod", "go.work"] { + let path = dir.file(file); + if let Some(text) = self.read_view(view, &path, "golang").await { + for entry in crate::vendor::go_mod_edit::parse_replace_entries(&text) { + if !entry.socket_owned() { + self.add( + "golang", + &entry.module, + entry.version.as_deref(), + &path.shown, + ); + } + } + // Workspace members' go.mod replaces still apply under go.work. + if file == "go.work" { + for member in crate::vendor::go_mod_edit::parse_use_dirs(&text) { + let rel = lexical_join(&dir.rel, Path::new(&member)); + pending.push_back(Dir { + shown: lexical_join(shown_base, &rel), + rel, + }); + } } - Some("yarn.lock") => index.yarn_lock(text, path), - _ => {} } } - index - } - - /// Whether the index holds no evidence at all. - pub(crate) fn is_empty(&self) -> bool { - self.entries.is_empty() } /// [`Self::read`] through a project view. - async fn read_view(&mut self, view: &ProjectView<'_>, rel: &str) -> Option { - match view.read_text(rel).await { + async fn read_view( + &mut self, + view: &ProjectView<'_>, + file: &Dir, + ecosystem: &'static str, + ) -> Option { + let rel = file.view_rel(); + match view.read_text(&rel).await { Ok(text) => Some(strip_bom(&text).to_owned()), Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, // An in-memory link has no content to read: the engine refuses // a project with a linked candidate file on its own, so it is // not taken for unknown metadata, which would withhold every // package ahead of that refusal. - Err(_) if view.is_symlink(rel) => None, + Err(_) if matches!(view, ProjectView::Memory(_)) && view.is_symlink(&rel) => None, Err(e) => { self.unknown( - "npm", - Path::new(rel), + ecosystem, + &file.shown, &format!("cannot inspect patch metadata: {e}"), ); None @@ -240,20 +337,40 @@ impl UserPatchIndex { } } - /// [`Self::patch_directory`] through a project view. - async fn list_patch_directory(&mut self, view: &ProjectView<'_>, rel: &str, required: bool) { - let rel = rel.trim_start_matches("./").trim_end_matches('/'); - match view.list_dir(rel).await { + /// [`Self::json`] through a project view. + async fn json_view(&mut self, view: &ProjectView<'_>, file: &Dir) -> Option { + let text = self.read_view(view, file, "npm").await?; + self.parse_json(&text, &file.shown, "npm") + } + + /// The patch-package files of directory `dir`. `required`: a directory + /// a script names, which must exist; `repo_depth` bounds it to the git + /// repository ([`Self::walk`]). + async fn list_patch_directory( + &mut self, + view: &ProjectView<'_>, + dir: &Dir, + required: bool, + repo_depth: Option, + ) { + if outside_repo(&dir.rel, repo_depth) { + self.unknown( + "npm", + &dir.shown, + "patch-package directory outside the git repository is not inspected", + ); + return; + } + match view.list_dir(&dir.view_rel()).await { Ok(entries) => { for entry in entries { - let path = Path::new(rel).join(&entry.name); - self.patch_file(&entry.name, &path, required); + self.patch_file(&entry.name, &dir.shown.join(&entry.name), required); } } Err(e) if !required && e.kind() == std::io::ErrorKind::NotFound => {} Err(e) => self.unknown( "npm", - Path::new(rel), + &dir.shown, &format!("cannot inspect patch-package directory: {e}"), ), } @@ -262,44 +379,65 @@ impl UserPatchIndex { // Use the existing npm/Yarn glob grammar, with a bounded, link-free walk. // Positive patterns are a conservative union: a patch in an excluded member // may still protect the same release elsewhere, never silently lose it. - fn workspace_members(&mut self, root: &Path, pattern: &str) -> Vec { + // Returns the members' root-relative paths. + async fn workspace_members( + &mut self, + view: &ProjectView<'_>, + dir: &Dir, + pattern: &str, + repo_depth: Option, + ) -> Vec { if Path::new(pattern).is_absolute() { - self.unknown("npm", root, "workspace outside the inspection root"); + self.unknown("npm", &dir.shown, "workspace outside the inspection root"); return Vec::new(); } // Resolve `..` lexically: `packages/b/../a` is `packages/a`, and a // leading `../shared/*` re-roots the walk at the parent. - let mut root = root.to_path_buf(); + let mut root = dir.clone(); let mut segments: Vec<&str> = Vec::new(); for segment in pattern.split('/') { match segment { "" | "." => {} ".." => match segments.last() { Some(last) if last.contains(['*', '?', '{', '[', '(']) => { - self.unknown("npm", &root, "workspace glob traverses a wildcard"); + self.unknown("npm", &root.shown, "workspace glob traverses a wildcard"); return Vec::new(); } Some(_) => { segments.pop(); } None => { - if !root.pop() { - self.unknown("npm", &root, "workspace outside the filesystem root"); + if root.shown.is_absolute() && root.shown.parent().is_none() { + self.unknown( + "npm", + &root.shown, + "workspace outside the filesystem root", + ); return Vec::new(); } + root = root.file(".."); } }, _ => segments.push(segment), } } let pattern = segments.join("/"); - let (root, pattern) = (root.as_path(), pattern.as_str()); + let pattern = pattern.as_str(); let prefix: std::path::PathBuf = pattern .split('/') .take_while(|p| !p.contains(['*', '?', '{', '[', '('])) .collect(); - let base = root.join(&prefix); - if !base.exists() && !base.is_symlink() { + if outside_repo(&root.rel, repo_depth) { + self.unknown( + "npm", + &root.shown, + "workspace outside the git repository is not inspected", + ); + return Vec::new(); + } + let base = root.file(&prefix.to_string_lossy()); + let base_rel = base.view_rel(); + if !view.is_dir(&base_rel) && !view.is_file(&base_rel) && !view.is_symlink(&base_rel) { return Vec::new(); } let max_depth = if pattern.contains("**") { @@ -310,49 +448,51 @@ impl UserPatchIndex { .count() .saturating_sub(prefix.components().count()) }; - let walk = walkdir::WalkDir::new(&base) - .follow_links(false) - .max_depth(max_depth) - .into_iter() - .filter_entry(|entry| { - (entry.file_type().is_dir() || entry.path_is_symlink()) - && !matches!( - entry.file_name().to_str(), - Some("node_modules" | ".git" | ".socket" | "target" | ".venv") - ) - }); + let skipped = |name: &str| { + matches!( + name, + "node_modules" | ".git" | ".socket" | "target" | ".venv" + ) + }; let mut out = Vec::new(); let patterns = vec![pattern.to_string()]; - for (count, entry) in walk.enumerate() { - if count >= 16384 { - self.unknown("npm", root, "workspace inspection limit exceeded"); - break; - } - let entry = match entry { - Ok(entry) => entry, - Err(error) => { - self.unknown("npm", root, &format!("cannot inspect workspace: {error}")); - continue; - } - }; - let components: Vec<_> = entry - .path() - .strip_prefix(root) - .unwrap_or(entry.path()) + // Depth-first, as `walkdir` walked it: (directory, depth, its + // components below `root`). + let base_name = prefix.file_name().map(|n| n.to_string_lossy().into_owned()); + if base_name.as_deref().is_some_and(skipped) { + return Vec::new(); + } + let mut stack: Vec<(Dir, usize, Vec)> = vec![( + base.clone(), + 0, + prefix .components() .map(|c| c.as_os_str().to_string_lossy().into_owned()) - .collect(); - if entry.depth() == 32 && entry.file_type().is_dir() { - self.unknown("npm", entry.path(), "workspace inspection depth exceeded"); + .collect(), + )]; + let mut count = 0usize; + while let Some((entry, depth, components)) = stack.pop() { + count += 1; + if count > 16384 { + self.unknown("npm", &root.shown, "workspace inspection limit exceeded"); + break; + } + let rel = entry.view_rel(); + let is_link = view.is_symlink(&rel); + if !is_link && !view.is_dir(&rel) { + continue; + } + if depth == 32 && !is_link { + self.unknown("npm", &entry.shown, "workspace inspection depth exceeded"); } // Package managers only treat a directory with a package.json as // a workspace; anything else under a `**` glob is just source. - let member = entry.path().join("package.json").is_file(); - if entry.file_type().is_symlink() { + let member = view.is_file(&entry.file("package.json").view_rel()); + if is_link { // The walk doesn't follow links, so a link that is (or leads // to) a member can't be inspected. - if entry.depth() == 0 - || (entry.path().is_dir() + if depth == 0 + || (view.is_dir(&rel) && (member || workspaces_include_as( &patterns, @@ -362,7 +502,7 @@ impl UserPatchIndex { { self.unknown( "npm", - entry.path(), + &entry.shown, "linked workspace cannot be inspected safely", ); } @@ -371,7 +511,32 @@ impl UserPatchIndex { if member && workspaces_include_as(&patterns, &components, WorkspacesDialect::YARN_BERRY) { - out.push(entry.path().to_path_buf()); + out.push(entry.rel.clone()); + } + if depth >= max_depth { + continue; + } + match view.list_dir_strict(&rel).await { + Ok(children) => { + for child in children.into_iter().rev() { + let child_dir = entry.file(&child.name); + if skipped(&child.name) + || !child.is_dir && !view.is_symlink(&child_dir.view_rel()) + { + continue; + } + let mut below = components.clone(); + below.push(child.name.clone()); + stack.push((child_dir, depth + 1, below)); + } + } + Err(error) => { + self.unknown( + "npm", + &root.shown, + &format!("cannot inspect workspace: {error}"), + ); + } } } out @@ -605,13 +770,6 @@ impl UserPatchIndex { } } - async fn patch_package(&mut self, dir: &Path, value: &Value) { - self.patch_directory(&dir.join("patches"), false).await; - for patch_dir in self.patch_package_dirs(&dir.join("package.json"), value) { - self.patch_directory(&dir.join(patch_dir), true).await; - } - } - /// The `--patch-dir` directories the `patch-package` scripts of the /// `package.json` at `manifest` name, relative to its directory. A /// script whose directory can't be read without running it protects @@ -659,31 +817,6 @@ impl UserPatchIndex { dirs } - async fn patch_directory(&mut self, dir: &Path, required: bool) { - let entries = match std::fs::read_dir(dir) { - Ok(entries) => entries, - Err(e) if !required && e.kind() == std::io::ErrorKind::NotFound => return, - Err(e) => { - self.unknown( - "npm", - dir, - &format!("cannot inspect patch-package directory: {e}"), - ); - return; - } - }; - for entry in entries { - match entry { - Ok(entry) => self.patch_file( - &entry.file_name().to_string_lossy(), - &entry.path(), - required, - ), - Err(e) => self.unknown("npm", dir, &e.to_string()), - } - } - } - /// One file of a patch-package directory, named /// `[++]+[.dev].patch` (`@scope+name+…` scoped). fn patch_file(&mut self, file: &str, path: &Path, required: bool) { @@ -817,6 +950,83 @@ impl UserPatchIndex { } } +/// What a [`UserPatchIndex::walk`] reads. +#[derive(Clone, Copy, PartialEq, Eq)] +enum Scope { + /// Everything the CLI's consent gate protects. + Gate, + /// The npm-family declarations of [`UserPatchIndex::npm_family_in`]. + NpmFamily, +} + +/// A directory or file of a [`UserPatchIndex::walk`]: its path relative to +/// the project root (lexically normalized; leading `..` reach an +/// ancestor), and the path its evidence is reported under. +#[derive(Clone, Debug)] +struct Dir { + rel: std::path::PathBuf, + shown: std::path::PathBuf, +} + +impl Dir { + fn file(&self, name: &str) -> Dir { + Dir { + rel: lexical_join(&self.rel, Path::new(name)), + shown: lexical_join(&self.shown, Path::new(name)), + } + } + + /// The `/`-separated path a [`ProjectView`] reads it at (`""` for the + /// root; an absolute path as is). + fn view_rel(&self) -> String { + if self.rel.is_absolute() { + return self.rel.to_string_lossy().into_owned(); + } + self.rel + .components() + .map(|c| c.as_os_str().to_string_lossy()) + .collect::>() + .join("/") + } +} + +/// `base` joined with `rel`, resolving `.` and `..` lexically (an absolute +/// `rel` replaces `base`; `..` above a relative `base` stays a leading +/// `..`). +fn lexical_join(base: &Path, rel: &Path) -> std::path::PathBuf { + use std::path::Component; + let mut out = base.to_path_buf(); + for component in rel.components() { + match component { + Component::CurDir => {} + Component::ParentDir => match out.components().next_back() { + Some(Component::Normal(_)) => { + out.pop(); + } + Some(Component::RootDir | Component::Prefix(_)) => {} + _ => out.push(".."), + }, + other => out.push(other.as_os_str()), + } + } + out +} + +/// Whether root-relative `rel` lies outside the git repository whose root +/// is `repo_depth` levels above the project root (`None`: unknown, so +/// nothing is). +fn outside_repo(rel: &Path, repo_depth: Option) -> bool { + let Some(depth) = repo_depth else { + return false; + }; + rel.is_absolute() + || rel + .components() + .take_while(|c| matches!(c, std::path::Component::ParentDir)) + .count() + > depth +} + /// The package a Yarn locator installs. An alias (`alias@npm:real@1.0.0`) /// installs `real`, which is the name the crawler reports. fn locator_target(locator: &str) -> (&str, Option<&str>) { diff --git a/crates/socket-patch-core/src/vex/discover/bun.rs b/crates/socket-patch-core/src/vex/discover/bun.rs index f18aaaa04..0cc1b77d4 100644 --- a/crates/socket-patch-core/src/vex/discover/bun.rs +++ b/crates/socket-patch-core/src/vex/discover/bun.rs @@ -189,13 +189,17 @@ async fn extract_text(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { // ── `bun patch` ────────────────────────────────────────────────────────── /// Mark every ref of `file` whose package the project patches itself with -/// `bun patch` (#367) [`UnattestedKind::UserPatchedDependency`], by the -/// hosted rewriters' own per-dependency rule (the npm extractor's -/// [`mark_user_patched`], read against the ref's hosted URL or vendored -/// tarball): Bun applies that patch only to the resolution it is keyed on, -/// so hosted and vendored mode leave the package where it is and never -/// confirm the Socket patch. A ref a pre-#367 run wired stays a ref, so -/// `rollback` / `remove` / `list` still find it. +/// `bun patch` (#367) [`UnattestedKind::UserPatchedDependency`] (the npm +/// extractor's [`mark_user_patched`]): any `patchedDependencies` key for +/// its `name@version`, whatever resolution the key names. A key for +/// another resolution is one Bun applies only there, so hosted and +/// vendored mode leave the package where it is and never confirm the +/// Socket patch. A key for the ref's own tarball (a `bun patch` made on +/// top of the pin, #1019) is one Bun applies on top of the Socket bytes: +/// the hosted rewriters leave that pin wired (no conflict for them), but +/// the install never runs the Socket patch alone, so it is never attested +/// either. A ref stays a ref, so `rollback` / `remove` / `list` still find +/// it. fn unattest_user_patched(file: &str, user_patches: &NpmUserPatches, out: &mut Discovery) { let refs: Vec = out .refs @@ -1930,9 +1934,11 @@ mod tests { /// `bun patch` is one the hosted rewriters (text and binary) leave on /// its resolution and never confirm, and one vendored mode refuses. A /// pin a pre-#367 run left stays a ref, for rollback, but is never - /// attested — by the rewriters' own per-dependency rule, so a key for - /// the pin's own tarball (a `bun patch` made on top of it) is no - /// conflict, and the manifest's keys and `bun.lock`'s mirror both count. + /// attested. A key for the pin's own tarball (a `bun patch` made on top + /// of it) is no conflict for the rewriters, which keep the pin, but the + /// user's diff then runs over the Socket bytes, so that pin is never + /// attested either. The manifest's keys and `bun.lock`'s mirror both + /// count. #[tokio::test] async fn bun_user_patched_dependency_pin_is_not_attested() { let p = fixture("basic", "expected"); @@ -1953,12 +1959,17 @@ mod tests { assert!(detail.contains(&format!("`{key}`")), "{detail}"); } // A patch keyed to the pin's own tarball applies on top of it: the - // rewriters leave such a package wired, and so does discovery. + // rewriters leave such a package wired, but the install runs the + // user's diff over the Socket bytes, so it is never attested. p.write( "package.json", format!(r#"{{"patchedDependencies":{{"left-pad@{url}":"patches/left-pad.patch"}}}}"#), ); - assert_eq!(user_patched(&run(&p).await), []); + let out = run(&p).await; + assert_refs(&out, &[(LEFT_PAD, FIXTURE_UUID, WiringMode::Hosted)]); + assert_eq!(user_patched(&out), [(LEFT_PAD, FIXTURE_UUID)]); + let detail = &out.unattested[0].detail; + assert!(detail.contains("on top of this pin's tarball"), "{detail}"); // Vendored, keyed to the registry release; the key mirrored at the // top of `bun.lock` counts like the manifest's. diff --git a/crates/socket-patch-core/src/vex/discover/npm.rs b/crates/socket-patch-core/src/vex/discover/npm.rs index 5f1e4d772..2fcad5ffc 100644 --- a/crates/socket-patch-core/src/vex/discover/npm.rs +++ b/crates/socket-patch-core/src/vex/discover/npm.rs @@ -312,22 +312,29 @@ fn push_uncontested( /// Why the project counts as patching `r`'s package itself /// ([`NpmUserPatches::source`], the hosted rewriters' #711 / #367 gate), -/// read against what `r` resolves the package to: its hosted URL, or its -/// vendored tarball. A ref with neither is still matched by its -/// `name@version` (and every tarball key counts against it). -fn user_patch_source(user_patches: &NpmUserPatches, r: &PatchedRef) -> Option { +/// read against no target: unlike for that gate, a `patchedDependencies` +/// key naming `r`'s own tarball (its hosted URL or vendored tarball) counts +/// too, since the user's diff is then applied on top of the Socket bytes +/// and the install never runs the Socket patch alone. The flag is set when +/// only such a key matches (the gate, read against `r`'s target, finds +/// none). A ref with neither a url nor a vendored tarball is matched by its +/// `name@version`. +fn user_patch_source(user_patches: &NpmUserPatches, r: &PatchedRef) -> Option<(String, bool)> { let (name, version) = crate::utils::purl::purl_name_version(&r.purl)?; + let source = user_patches.source(name, version, "")?; let target = r .url .as_deref() .or(r.artifact_rel.as_deref()) .unwrap_or_default(); - user_patches.source(name, version, target) + let on_pin = !target.is_empty() && user_patches.source(name, version, target).is_none(); + Some((source, on_pin)) } /// Mark the npm or Bun ref `r` [`UnattestedKind::UserPatchedDependency`] /// when the project patches its package itself ([`user_patch_source`]). -/// `how` completes the detail after the key or record it names. The mark +/// `how` completes the detail after the key or record it names, unless the +/// key names `r`'s own tarball (a patch made on top of the pin). The mark /// is an [`UnwiredCopy`] for exactly `r`, so it lands only if `r` survives /// every contest: a ref another lock drops carries no stale reason. pub(super) fn mark_user_patched( @@ -336,9 +343,15 @@ pub(super) fn mark_user_patched( how: &str, out: &mut Discovery, ) { - let Some(source) = user_patch_source(user_patches, r) else { + let Some((source, on_pin)) = user_patch_source(user_patches, r) else { return; }; + let how = if on_pin { + "a patch the project applies on top of this pin's tarball, so the install runs the \ + user's diff over the Socket patch, never the Socket patch alone" + } else { + how + }; out.unwired_copy(UnwiredCopy { scope: Some(r.source_file.clone()), target: CopyTarget::Ref { @@ -3718,8 +3731,8 @@ mod tests { let source = super::user_patch_source(&patches, &r); assert!( source - .as_deref() - .is_some_and(|s| s.contains("`left-pad@1.3.0`")), + .as_ref() + .is_some_and(|(s, on_pin)| s.contains("`left-pad@1.3.0`") && !on_pin), "{source:?}" ); } diff --git a/crates/socket-patch-core/src/vex/discover/user_patched.rs b/crates/socket-patch-core/src/vex/discover/user_patched.rs index 8e1cace53..9b67b68df 100644 --- a/crates/socket-patch-core/src/vex/discover/user_patched.rs +++ b/crates/socket-patch-core/src/vex/discover/user_patched.rs @@ -27,7 +27,7 @@ //! the three never disagree. npm's and Bun's own `patchedDependencies` //! are judged per resolution by their extractors instead (#711, #367). -use super::{DiscoverCtx, Discovery, UnattestedKind}; +use super::{CopyTarget, DiscoverCtx, Discovery, UnattestedKind, UnwiredCopy}; use crate::patch::user_patches::UserPatchIndex; pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { @@ -58,17 +58,14 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { if index.is_empty() { return; } - let marks: Vec<(String, String, String, String)> = out + // Each mark is an `UnwiredCopy` for exactly that ref, as the npm and + // Bun extractors' (`npm::mark_user_patched`): it lands only on a ref + // that survives every contest, and a ref one of those already covers + // keeps that first reason. + let marks: Vec = out .refs .iter() .filter(|r| r.purl.starts_with("pkg:npm/")) - .filter(|r| { - !out.unattested.iter().any(|u| { - u.kind == UnattestedKind::UserPatchedDependency - && u.purl == r.purl - && u.uuid == r.uuid - }) - }) .filter_map(|r| { let sources = index.conflicts(&r.purl); if sources.is_empty() { @@ -82,22 +79,20 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { leave the package alone without `--allow-patch-conflicts`", sources.join(", ") ); - Some(( - r.purl.clone(), - r.uuid.clone(), - r.source_file.to_string_lossy().into_owned(), + Some(UnwiredCopy { + scope: Some(r.source_file.clone()), + target: CopyTarget::Ref { + purl: r.purl.clone(), + uuid: r.uuid.clone(), + }, + file: r.source_file.clone(), detail, - )) + kind: UnattestedKind::UserPatchedDependency, + }) }) .collect(); - for (purl, uuid, file, detail) in marks { - out.unattested( - &purl, - &uuid, - &file, - detail, - UnattestedKind::UserPatchedDependency, - ); + for mark in marks { + out.unwired_copy(mark); } } @@ -326,6 +321,160 @@ mod tests { } } + /// Every file under `root` but the repository marker, as an in-memory + /// project. + fn memory_copy(root: &std::path::Path) -> crate::vendor::lock_inventory::MemoryProject { + use crate::vendor::lock_inventory::{MemoryEntry, MemoryProject}; + let mut project = MemoryProject::new(); + for entry in walkdir::WalkDir::new(root).into_iter().flatten() { + let rel = entry.path().strip_prefix(root).unwrap(); + if !entry.file_type().is_file() || rel.starts_with(".git") { + continue; + } + let text = std::fs::read_to_string(entry.path()).unwrap(); + let rel = rel.to_string_lossy().replace('\\', "/"); + project.insert(rel, MemoryEntry::Text(text.into())); + } + project + } + + /// Discovery reads a workspace member's user patches as the CLI's + /// consent gate does (the very same walk): a member's + /// `pnpm.patchedDependencies`, its yarn `patch:` descriptors and its own + /// `patches/` folder, members named by `package.json` `workspaces` or + /// `pnpm-workspace.yaml` `packages`, on disk and in memory alike. A + /// member beyond the git repository root is never read: it protects + /// every npm package instead, for the gate and discovery alike. + #[tokio::test] + async fn workspace_member_patches_agree_with_the_consent_gate() { + use crate::patch::user_patches::UserPatchIndex; + use crate::vendor::lock_inventory::ProjectView; + + let berry = + r#"{"name":"a","resolutions":{"left-pad":"patch:left-pad@npm%3A1.3.0#./p.patch"}}"#; + let cases: &[(&str, &str, &[(&str, &str)])] = &[ + ( + "package.json workspaces, pnpm field", + r#"{"workspaces":["packages/*"]}"#, + &[( + "packages/a/package.json", + r#"{"name":"a","pnpm":{"patchedDependencies":{"left-pad@1.3.0":"x.patch"}}}"#, + )], + ), + ( + "package.json workspaces, member patches/", + r#"{"workspaces":{"packages":["packages/**"]}}"#, + &[ + ("packages/nested/a/package.json", r#"{"name":"a"}"#), + ("packages/nested/a/patches/left-pad+1.3.0.patch", "diff\n"), + ], + ), + ( + "pnpm-workspace.yaml packages, yarn patch:", + r#"{"name":"root"}"#, + &[ + ("pnpm-workspace.yaml", "packages:\n - 'tools/*'\n"), + ("tools/a/package.json", berry), + ], + ), + ]; + for (what, manifest, files) in cases { + let p = Project::new(); + p.copy_fixture("redirect/npm/pnpm/basic/expected"); + p.write(".git/HEAD", "ref: refs/heads/main\n"); + p.write("package.json", manifest); + let gate = UserPatchIndex::discover(p.root()).await; + assert!(gate.conflicts(LEFT_PAD).is_empty(), "{what}: control"); + for (path, text) in *files { + p.write(path, text); + } + let gate = UserPatchIndex::discover(p.root()).await; + assert!(!gate.conflicts(LEFT_PAD).is_empty(), "{what}: gate"); + assert!(gate.conflicts("pkg:npm/other@1.0.0").is_empty(), "{what}"); + + let out = p.discover().await; + all_marked(&out, what); + let member = files[files.len() - 1].0.rsplit_once('/').unwrap().0; + assert!( + out.unattested[0].detail.contains(member), + "{what}: {}", + out.unattested[0].detail + ); + + let memory = memory_copy(p.root()); + let out = discover_patched_refs_view( + ProjectView::Memory(&memory), + &DiscoverOptions::default(), + ) + .await; + all_marked(&out, what); + } + + // Beyond the repository root: never read, every package protected. + let outer = tempfile::tempdir().unwrap(); + let repo = outer.path().join("repo"); + std::fs::create_dir_all(repo.join(".git")).unwrap(); + std::fs::create_dir_all(outer.path().join("shared/a/patches")).unwrap(); + std::fs::write(outer.path().join("shared/a/package.json"), "{}").unwrap(); + std::fs::write( + outer.path().join("shared/a/patches/left-pad+1.3.0.patch"), + "diff\n", + ) + .unwrap(); + for entry in std::fs::read_dir(fixture_path("redirect/npm/pnpm/basic/expected")).unwrap() { + let entry = entry.unwrap(); + std::fs::copy(entry.path(), repo.join(entry.file_name())).unwrap(); + } + std::fs::write( + repo.join("package.json"), + r#"{"workspaces":["../shared/*"]}"#, + ) + .unwrap(); + let gate = UserPatchIndex::discover(&repo).await; + let sources = gate.conflicts("pkg:npm/other@1.0.0"); + assert!( + sources + .iter() + .any(|s| s.contains("outside the git repository")), + "{sources:?}" + ); + assert!(!sources.iter().any(|s| s.contains(".patch")), "{sources:?}"); + let out = discover_patched_refs_with(&repo, &DiscoverOptions::default()).await; + all_marked(&out, "outside the repository"); + assert!( + out.unattested[0] + .detail + .contains("outside the git repository"), + "{}", + out.unattested[0].detail + ); + } + + /// A user-patched pnpm ref another lock contests is dropped by the + /// cross-lock contest and leaves no mark behind: the mark targets that + /// ref alone ([`CopyTarget::Ref`]) and lands only on contest survivors. + #[tokio::test] + async fn a_contested_user_patched_ref_leaves_no_mark() { + let p = Project::new(); + p.copy_fixture("redirect/npm/pnpm/basic/expected"); + p.write( + "pnpm-workspace.yaml", + "patchedDependencies:\n left-pad@1.3.0: patches/left-pad.patch\n", + ); + all_marked(&p.discover().await, "control"); + p.write( + "package-lock.json", + r#"{"name":"app","lockfileVersion":3,"requires":true,"packages":{"":{"name":"app","dependencies":{"left-pad":"1.3.0"}},"node_modules/left-pad":{"version":"1.3.0","resolved":"https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz","integrity":"sha512-UP=="}}}"#, + ); + let out = p.discover().await; + assert!( + out.refs.iter().all(|r| r.purl != LEFT_PAD), + "the contest drops the ref: {:#?}", + out.refs + ); + assert!(out.unattested.is_empty(), "{:#?}", out.unattested); + } + /// The pins of other packages stay attested. #[tokio::test] async fn other_packages_are_not_marked() { From 23dd236de7eec938b91be6a2f7f1287f76b14718 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 14:39:26 -0600 Subject: [PATCH 3/4] Bound user-patch reads to the repository in workspace runs only Owner clarification: the repository-root bound is for workspaces; a global run should be able to read anywhere on the system. The shared user-patch walk now takes a ReadScope { Workspace, Global }, chosen once from the CLI mode (GlobalArgs::read_scope, from --global / -g / --global-prefix), and only a Workspace walk bounds its reads: a workspace member or --patch-dir beyond the git repository root protects every npm package instead of being read. A Global walk reads both wherever they are and protects exactly what they patch. The ancestor walk itself (stop at .git, never reach the home directory) is the same in both scopes. UserPatchIndex::discover stays the workspace walk; UserPatchIndex::discover_scoped takes the scope, which the CLI's inspect() now threads through. A global run still inspects no project (a local checkout does not govern the global tree, #1646), so the Global arm is reached through the core API today. vex discovery and the in-memory hosted engine only run on workspaces and keep the bound. ReadScope has the shape of #2025's utils::repo_root::ReadScope (same variants and of_global), so it is swapped for that type when the two branches meet. Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 6 +- crates/socket-patch-cli/src/args.rs | 8 + .../src/commands/user_patches.rs | 13 +- .../src/patch/user_patches.rs | 140 ++++++++++++++++-- 4 files changed, 151 insertions(+), 16 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 4800df280..65347edfb 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -211,7 +211,11 @@ ancestor walk stops there, and a workspace member or literal `--patch-dir` beyond it protects every npm package with a `workspace outside the git repository` / `patch-package directory outside the git repository` diagnostic instead of being read. Without a `.git`, the ancestor walk stops below the home -directory. The hosted engine and `vex` run this same walk (npm-family rows only, +directory. This repository bound is a workspace-run rule: the walk's global +read scope has none (members and patch directories are read wherever they +are, and protect only what they patch), though a global run (`--global`, +`--global-prefix`) inspects no project today, since a local checkout does not +govern the global tree. The hosted engine and `vex` run this same walk (npm-family rows only, without native npm / Bun); in memory the project is its own repository root. | Mechanism | Evidence | diff --git a/crates/socket-patch-cli/src/args.rs b/crates/socket-patch-cli/src/args.rs index 128e19687..e7fc3a506 100644 --- a/crates/socket-patch-cli/src/args.rs +++ b/crates/socket-patch-cli/src/args.rs @@ -447,6 +447,14 @@ impl GlobalArgs { self.global || self.global_prefix.is_some() } + /// The read scope this run's mode fixes: a global run (`--global` / + /// `--global-prefix`) reads anywhere on the system; a project run never + /// reads a project file outside the git repository root. #2025's + /// `GlobalArgs::read_scope` has the same contract. + pub(crate) fn read_scope(&self) -> socket_patch_core::patch::user_patches::ReadScope { + socket_patch_core::patch::user_patches::ReadScope::of_global(self.is_global()) + } + /// Whether `--ecosystems` selects `eco` (every ecosystem when unset or /// empty). The names are validated at parse time, so this is an exact /// match. diff --git a/crates/socket-patch-cli/src/commands/user_patches.rs b/crates/socket-patch-cli/src/commands/user_patches.rs index a273a3be0..9d395928f 100644 --- a/crates/socket-patch-cli/src/commands/user_patches.rs +++ b/crates/socket-patch-cli/src/commands/user_patches.rs @@ -1,7 +1,7 @@ //! The CLI consent gate. --yes, --force, UUID selection and socket.yml do //! not grant permission to combine a Socket patch with a project's own patch. -use socket_patch_core::patch::user_patches::UserPatchIndex; +use socket_patch_core::patch::user_patches::{ReadScope, UserPatchIndex}; use crate::args::GlobalArgs; use crate::json_envelope::{Command, Envelope, EnvelopeError, PatchAction, PatchEvent}; @@ -25,11 +25,12 @@ fn any_detectable<'a>(common: &GlobalArgs, mut purls: impl Iterator UserPatchIndex { - if common.is_global() { - // A local checkout does not govern a separately selected global tree. - UserPatchIndex::default() - } else { - UserPatchIndex::discover(&common.cwd).await + match common.read_scope() { + // A local checkout does not govern a separately selected global + // tree, so a global run inspects no project (it would read one + // with no repository bound, `ReadScope::Global`). + ReadScope::Global => UserPatchIndex::default(), + scope @ ReadScope::Workspace => UserPatchIndex::discover_scoped(&common.cwd, scope).await, } } diff --git a/crates/socket-patch-core/src/patch/user_patches.rs b/crates/socket-patch-core/src/patch/user_patches.rs index d7f172fa5..dbbe542a9 100644 --- a/crates/socket-patch-core/src/patch/user_patches.rs +++ b/crates/socket-patch-core/src/patch/user_patches.rs @@ -36,9 +36,22 @@ impl UserPatchIndex { /// Inspect the project, declared workspace members and ancestor workspace /// settings. Errors in a potentially governing file protect its ecosystem; /// unreadable metadata must not silently disable the consent check. + /// + /// A workspace run: nothing outside the git repository root is read + /// (see [`Self::discover_scoped`]). pub async fn discover(root: &Path) -> Self { + Self::discover_scoped(root, ReadScope::Workspace).await + } + + /// [`Self::discover`] in the read scope the run's mode fixes. A + /// [`ReadScope::Workspace`] run never reads a project file outside the + /// git repository root: a workspace member or `--patch-dir` beyond it + /// protects every npm package instead. A [`ReadScope::Global`] run reads + /// the evidence wherever it lives, with no repository bound and no + /// such fallback. + pub async fn discover_scoped(root: &Path, reads: ReadScope) -> Self { let root = std::path::absolute(root).unwrap_or_else(|_| root.to_path_buf()); - Self::walk(&ProjectView::Disk(&root), Some(&root), Scope::Gate).await + Self::walk(&ProjectView::Disk(&root), Some(&root), Scope::Gate, reads).await } /// The patches a project's pnpm, yarn or patch-package setup applies @@ -61,7 +74,8 @@ impl UserPatchIndex { /// a vlt project patches a dependency only through a postinstall tool /// such as patch-package. Sources are reported root-relative. pub(crate) async fn npm_family_in(view: &ProjectView<'_>, locks: &[(&str, &str)]) -> Self { - let mut index = Self::walk(view, None, Scope::NpmFamily).await; + // Discovery and the hosted engine only ever run on a workspace. + let mut index = Self::walk(view, None, Scope::NpmFamily, ReadScope::Workspace).await; for (rel, text) in locks { let path = Path::new(rel); match rel.rsplit('/').next() { @@ -86,13 +100,20 @@ impl UserPatchIndex { /// anchored: the absolute project root for the gate, `None` for /// root-relative sources. /// - /// Nothing outside the git repository root is read: the ancestor walk - /// stops at the directory holding `.git`, and a workspace member or - /// patch-package directory beyond it protects every npm package - /// instead of being read. Without a `.git` the gate's container rule - /// holds (the walk never reaches the home directory or above it). An - /// in-memory project is its own repository root. - async fn walk(view: &ProjectView<'_>, shown_root: Option<&Path>, scope: Scope) -> Self { + /// The ancestor walk stops at the directory holding `.git`, and without + /// a `.git` the gate's container rule holds (the walk never reaches the + /// home directory or above it). In a [`ReadScope::Workspace`] run + /// nothing outside that git repository root is read: a workspace member + /// or patch-package directory beyond it protects every npm package + /// instead of being read. An in-memory project is its own repository + /// root. A [`ReadScope::Global`] run reads members and patch-package + /// directories wherever they are. + async fn walk( + view: &ProjectView<'_>, + shown_root: Option<&Path>, + scope: Scope, + reads: ReadScope, + ) -> Self { let mut index = Self::default(); let shown_base = shown_root.unwrap_or(Path::new("")); let at = |rel: &Path| Dir { @@ -157,6 +178,12 @@ impl UserPatchIndex { } else { repo_depth = Some(0); } + // The read bound: the repository root in a workspace run, none in a + // global one. + let repo_depth = match reads { + ReadScope::Workspace => repo_depth, + ReadScope::Global => None, + }; let mut seen = BTreeSet::new(); while let Some(dir) = pending.pop_front() { if !seen.insert(dir.rel.clone()) { @@ -950,6 +977,31 @@ impl UserPatchIndex { } } +/// Which project files a run may read, chosen once from the CLI mode +/// (never inferred from the filesystem). The same shape as #2025's +/// `utils::repo_root::ReadScope`, which replaces it when the two meet. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ReadScope { + /// A project run (the default `--cwd` project): project files are read + /// only inside the git repository root. + Workspace, + /// A global run (`--global` / `--global-prefix`): files are read + /// wherever they live on the system. + Global, +} + +impl ReadScope { + /// The scope of a run whose global flag (`--global` or + /// `--global-prefix`) is `global`. + pub fn of_global(global: bool) -> Self { + if global { + ReadScope::Global + } else { + ReadScope::Workspace + } + } +} + /// What a [`UserPatchIndex::walk`] reads. #[derive(Clone, Copy, PartialEq, Eq)] enum Scope { @@ -1596,6 +1648,76 @@ mod tests { } } + /// The repository-root bound is a workspace-run rule only. A workspace + /// run reads no workspace member or `--patch-dir` outside the git + /// repository: each protects every npm package instead. A global run + /// reads both wherever they are and protects exactly the packages they + /// patch. + #[tokio::test] + async fn the_repository_bound_holds_for_workspace_runs_only() { + let outer = tempfile::tempdir().unwrap(); + let repo = outer.path().join("repo"); + write(&repo, ".git/HEAD", "ref: refs/heads/main\n"); + write( + &repo, + "package.json", + r#"{"workspaces":["../shared/*"],"scripts":{"postinstall":"patch-package --patch-dir ../fixes"}}"#, + ); + write(outer.path(), "fixes/left-pad+1.3.0.patch", "diff\n"); + write(outer.path(), "shared/a/package.json", "{}"); + write( + outer.path(), + "shared/a/patches/is-number+7.0.0.patch", + "diff\n", + ); + + let workspace = UserPatchIndex::discover_scoped(&repo, ReadScope::Workspace).await; + let sources = workspace.conflicts("pkg:npm/other@1.0.0"); + for what in [ + "workspace outside the git repository", + "patch-package directory outside", + ] { + assert!( + sources.iter().any(|s| s.contains(what)), + "{what}: {sources:?}" + ); + } + assert!( + !sources.iter().any(|s| s.ends_with(".patch")), + "{sources:?}" + ); + assert_eq!( + UserPatchIndex::discover(&repo) + .await + .conflicts("pkg:npm/other@1.0.0"), + sources, + "discover() is the workspace scope" + ); + + let global = UserPatchIndex::discover_scoped(&repo, ReadScope::Global).await; + assert!( + global.conflicts("pkg:npm/other@1.0.0").is_empty(), + "{global:?}" + ); + let left_pad = global.conflicts("pkg:npm/left-pad@1.3.0"); + assert!( + left_pad + .iter() + .any(|s| s.ends_with("fixes/left-pad+1.3.0.patch")), + "{left_pad:?}" + ); + let is_number = global.conflicts("pkg:npm/is-number@7.0.0"); + assert!( + is_number + .iter() + .any(|s| s.ends_with("shared/a/patches/is-number+7.0.0.patch")), + "{is_number:?}" + ); + assert!(global.conflicts("pkg:npm/left-pad@1.2.0").is_empty()); + assert_eq!(ReadScope::of_global(true), ReadScope::Global); + assert_eq!(ReadScope::of_global(false), ReadScope::Workspace); + } + /// The view-based index the hosted engine and `vex` discovery read /// ([`UserPatchIndex::npm_family_in`]) protects exactly what the CLI's /// consent gate ([`UserPatchIndex::discover`]) protects for a root From 93709da822d797aed0d02703126621f952488fa6 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 22:02:29 -0600 Subject: [PATCH 4/4] Fix /code-review findings on the user-patch walk and selection In-memory selection (the engine's view holds only what select_paths asks the host for): - A workspace member's package.json was selected presence-only (a pnpm member under named roots) or not at all (npm / yarn members), and patch-package files and a --patch-dir's contents never: the engine either withheld every npm package over a member it saw without content, or pinned a package a member or patches/ file patches. Selection now asks, outside node_modules / .git / .socket / target / .venv (WALK_SKIP, shared with the walk), for every package.json, pnpm-workspace.yaml, pnpm-lock.yaml and yarn.lock with content and every *.patch file by name. A file the walk reads that the host supplied without content still protects every npm package (never a silent miss); a required --patch-dir with no listing in memory holds no *.patch (selection supplies them all), so it no longer withholds everything. user_patches_are_selected_and_agree_with_the_disk_flow drives select_paths -> engine for each case, with detected and named roots, against the disk flow. The walk: - Ancestors come from utils::repo_root::search_dirs (git's lookup, honoring the home rule and GIT_CEILING_DIRECTORIES), checked and read at the same canonical absolute path, and the repository root from find_git_repo; the hand-rolled search and lexical_join are gone (utils::relpath::normalize_lexically_keeping_escapes). - The bound resolves absolute paths: an absolute --patch-dir or member inside the repository is read. - An out-of-repo directory is recorded under the ecosystem that named it: a go.work use member protects Go modules, not npm packages. - npm_family_in parses a passed lock only when the walk did not already read that directory's lock. Hosted disk flow: the engine gate no longer re-walks the project after the consent gate; it withholds by the Rush lock alone (UserPatchIndex::from_locks + engine::withhold_user_patched_by), and before the vlt artifact preflight, so a withheld artifact is never downloaded. Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 18 +- .../src/commands/scan/hosted.rs | 32 +- .../tests/hosted_memory_parity.rs | 173 ++++++++ crates/socket-patch-core/src/hosted/engine.rs | 18 +- .../src/hosted/memory/select.rs | 25 ++ .../src/patch/user_patches.rs | 372 ++++++++++++------ 6 files changed, 491 insertions(+), 147 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 65347edfb..6cbf9ef45 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -206,9 +206,13 @@ A missing referenced diff does not remove that protection. Detection covers root and declared npm workspace manifests, ancestor workspace settings, and the following evidence. Nothing outside the git repository root -(the nearest ancestor of the project, inclusive, holding `.git`) is read: the -ancestor walk stops there, and a workspace member or literal `--patch-dir` -beyond it protects every npm package with a `workspace outside the git +(found as git finds it: the nearest ancestor of the project's real path, +inclusive, holding `.git`, never entering the home directory or a +`GIT_CEILING_DIRECTORIES` entry) is read: the ancestor walk stops there, each +ancestor is read at the path its markers were checked at, and a workspace member +or `--patch-dir` beyond it (relative or absolute; an absolute one inside the +repository is read) protects every package of the ecosystem that named it (npm, +or Go for a `go.work` `use` member) with a `workspace outside the git repository` / `patch-package directory outside the git repository` diagnostic instead of being read. Without a `.git`, the ancestor walk stops below the home directory. This repository bound is a workspace-run rule: the walk's global @@ -216,7 +220,13 @@ read scope has none (members and patch directories are read wherever they are, and protect only what they patch), though a global run (`--global`, `--global-prefix`) inspects no project today, since a local checkout does not govern the global tree. The hosted engine and `vex` run this same walk (npm-family rows only, -without native npm / Bun); in memory the project is its own repository root. +without native npm / Bun); in memory the project is its own repository root, +and path selection asks the host for every file the walk can read (each +`package.json`, `pnpm-workspace.yaml`, `pnpm-lock.yaml` and `yarn.lock` with +content, each `*.patch` file by name, outside `node_modules`, `.git`, `.socket`, +`target` and `.venv`). A file the walk reads that the host supplied without +content (oversize, an LFS pointer) protects every npm package; a `--patch-dir` +with no `*.patch` file below it holds no patch. | Mechanism | Evidence | | --- | --- | diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index 3d1216748..a7a0e17e7 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -870,6 +870,24 @@ async fn run_redirect_selected_untracked( ); } + // The engine's user-patch gate (pnpm, yarn, patch-package), which the + // in-memory engine applies over its own walk. Here the consent gate + // already walked the project and left such packages out unless + // `--allow-patch-conflicts` (which lifts this one too), so only what + // its walk does not read is added: a Rush lock's patched entries. Each + // such package is a skip event, as the consent gate reports it. Before + // the vlt preflight, so a withheld artifact is never downloaded. + if !common.allow_patch_conflicts { + let rush = socket_patch_core::constants::npm_family::RUSH_COMMON_LOCK_REL; + if let Ok(text) = view.read_text(rush).await { + let index = socket_patch_core::patch::user_patches::UserPatchIndex::from_locks(&[( + rush, + text.as_str(), + )]); + let _ = engine::withhold_user_patched_by(&index, &mut candidates, &mut skipped); + } + } + // vlt artifact preflight: before any takeover or rewrite (dry runs // included), each in-scope artifact is fetched as vlt fetches it. A // failure while vlt drives withholds the dep from every rewriter; @@ -887,20 +905,6 @@ async fn run_redirect_selected_untracked( &vlt_preflight.withheld_everywhere, &mut skipped, ); - // The engine's own user-patch gate (pnpm, yarn, patch-package), which - // the in-memory engine applies too. The consent gate above already - // left such packages out unless `--allow-patch-conflicts`, which lifts - // this one as well; it still covers evidence that gate does not read - // (a Rush lock's patched entries). Each such package is a skip event, - // as the consent gate reports it. - let _ = engine::withhold_user_patched( - &view, - &std::collections::BTreeMap::new(), - &mut candidates, - &mut skipped, - common.allow_patch_conflicts, - ) - .await; // A NEW row the lockfiles already pin on the patch server these // references name (not a configured one, so the recorded view's // discovery could not see it) is ALREADY: decided here, before the lock diff --git a/crates/socket-patch-cli/tests/hosted_memory_parity.rs b/crates/socket-patch-cli/tests/hosted_memory_parity.rs index 1e168ff14..cc6dc03b0 100644 --- a/crates/socket-patch-cli/tests/hosted_memory_parity.rs +++ b/crates/socket-patch-cli/tests/hosted_memory_parity.rs @@ -1852,3 +1852,176 @@ async fn a_pnpm_workspace_file_listing_nothing_adds_no_root() { ); } } + +/// The in-memory engine through path selection, and the disk flow, over a +/// project whose `left-pad@1.3.0` (the pnpm golden fixture's patch) the +/// project may patch itself: the `redirected` count of each, and whether +/// the memory root project skipped a candidate `user_patch_conflict`. +/// The memory run is made twice through selection: with the roots it +/// detects, and with the caller naming the root (`projectRoots: [""]`), +/// where a pnpm member with a lock is no root of its own; both must agree. +async fn user_patch_runs(files: &BTreeMap>) -> (u64, usize, bool) { + use socket_patch_cli::hosted_memory::{select_paths, SelectOptions, TreeEntryInput}; + let dir = fixtures_root().join("redirect/npm/pnpm/basic"); + let server = MockServer::start().await; + let patches = patches_from_overrides(&dir.join("overrides.json"), Some(&server.uri())); + mount_api(&server, &patches).await; + let disk = run_disk_with(&server, files, false, &["--no-trust-lockfile-config"]); + let entries: Vec = files + .iter() + .map(|(p, bytes)| TreeEntryInput { + path: p.clone(), + mode: "100644".into(), + kind: "blob".into(), + size: Some(bytes.len() as u64), + }) + .collect(); + let mut runs = Vec::new(); + for roots in [None, Some(vec![String::new()])] { + let selection = select_paths( + &entries, + &SelectOptions { + project_roots: roots, + ..SelectOptions::default() + }, + ); + let fetched: BTreeMap> = selection + .fetch_text + .iter() + .chain(selection.fetch_binary.iter()) + .map(|p| (p.clone(), files[p].clone())) + .collect(); + let present: Vec<&str> = selection.present_only.iter().map(String::as_str).collect(); + let mut opts = options(false); + opts.project_roots = Some(selection.roots.clone()); + opts.trust_lockfile_config = Some(false); + let memory = run_engine(&server, build_input(&fetched, &present, opts)).await; + let root = memory + .projects + .iter() + .find(|p| p.root.is_empty()) + .expect("the root project"); + assert!(root.error.is_none(), "{:?}", root.error); + let skipped = root + .skipped + .iter() + .any(|s| s.reason == "user_patch_conflict"); + runs.push((root.redirected.len(), skipped)); + } + assert_eq!(runs[0], runs[1], "detected and named roots agree"); + ( + disk.redirect["redirected"].as_u64().unwrap_or(0), + runs[0].0, + runs[0].1, + ) +} + +/// Review (#2018): the in-memory engine reads user patches only from the +/// files path selection asks the host for. A pnpm workspace's member +/// manifests, an npm / yarn workspace member's manifest and `patches/` +/// folder, and the root's patch-package files and `--patch-dir` must all +/// be selected (with content where the walk reads it), or the engine +/// either pins a package the project patches itself or withholds every +/// npm package over a file it saw without content. Through selection, +/// memory agrees with the disk flow, whose consent gate reads the same +/// declarations: a patched `left-pad` is never pinned, and an unrelated +/// patch withholds nothing. +#[tokio::test] +async fn user_patches_are_selected_and_agree_with_the_disk_flow() { + let lock = + String::from_utf8(read_fixture("redirect/npm/pnpm/basic/input/pnpm-lock.yaml")).unwrap(); + let pnpm_ws = "packages:\n - 'packages/*'\nsharedWorkspaceLockfile: false\n"; + let member_patch = r#"{ "name": "a", "dependencies": { "left-pad": "1.3.0" }, "pnpm": { "patchedDependencies": { "left-pad@1.3.0": "x.patch" } } }"#; + let npm_ws = r#"{ "name": "root", "private": true, "workspaces": ["packages/*"] }"#; + let patch_dir = + r#"{ "name": "root", "scripts": { "postinstall": "patch-package --patch-dir fixes" } }"#; + let root_manifest = r#"{ "name": "root", "private": true }"#; + let with_root_lock = |extra: &[(&str, &str)]| -> Vec<(String, String)> { + let mut files = vec![("pnpm-lock.yaml".to_string(), lock.clone())]; + files.extend(extra.iter().map(|(p, t)| (p.to_string(), t.to_string()))); + files + }; + let pnpm_members = |a: &str| -> Vec<(String, String)> { + vec![ + ("pnpm-workspace.yaml".into(), pnpm_ws.into()), + ("package.json".into(), root_manifest.into()), + ("packages/a/package.json".into(), a.into()), + ("packages/a/pnpm-lock.yaml".into(), lock.clone()), + ( + "packages/b/package.json".into(), + r#"{ "name": "b" }"#.into(), + ), + ] + }; + let cases: Vec<(&str, Vec<(String, String)>, bool)> = vec![ + ("pnpm member manifest", pnpm_members(member_patch), true), + ( + "pnpm members without a patch", + pnpm_members(r#"{ "name": "a", "dependencies": { "left-pad": "1.3.0" } }"#), + false, + ), + ( + "npm workspace member's patches/", + with_root_lock(&[ + ("package.json", npm_ws), + ("packages/a/package.json", r#"{ "name": "a" }"#), + ("packages/a/patches/left-pad+1.3.0.patch", "diff\n"), + ]), + true, + ), + ( + "npm workspace member's patch: descriptor", + with_root_lock(&[ + ("package.json", npm_ws), + ( + "packages/a/package.json", + r#"{ "name": "a", "resolutions": { "left-pad": "patch:left-pad@npm%3A1.3.0#./p.patch" } }"#, + ), + ]), + true, + ), + ( + "npm workspace members without a patch", + with_root_lock(&[ + ("package.json", npm_ws), + ("packages/a/package.json", r#"{ "name": "a" }"#), + ]), + false, + ), + ( + "root patches/", + with_root_lock(&[("patches/left-pad+1.3.0.patch", "diff\n")]), + true, + ), + ( + "root --patch-dir", + with_root_lock(&[ + ("package.json", patch_dir), + ("fixes/left-pad+1.3.0.patch", "diff\n"), + ]), + true, + ), + ( + "root --patch-dir patching another package", + with_root_lock(&[ + ("package.json", patch_dir), + ("fixes/other+1.0.0.patch", "diff\n"), + ]), + false, + ), + ]; + for (what, files, patched) in cases { + let files: BTreeMap> = files + .into_iter() + .map(|(p, t)| (p, t.into_bytes())) + .collect(); + let (disk, memory, skipped) = user_patch_runs(&files).await; + if patched { + assert_eq!((disk, memory), (0, 0), "{what}: never pinned"); + assert!(skipped, "{what}: memory skips it user_patch_conflict"); + } else { + assert!(disk > 0 && memory > 0, "{what}: pinned ({disk}, {memory})"); + assert!(!skipped, "{what}"); + } + } +} diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 75b8dfdf1..665233044 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -375,9 +375,8 @@ pub async fn withhold_user_patched( skipped: &mut Vec, allow: bool, ) -> Vec { - let mut warnings = Vec::new(); if allow || !candidates.iter().any(|c| c.dep.ecosystem == "npm") { - return warnings; + return Vec::new(); } let mut read: Vec<(String, String)> = Vec::new(); for rel in USER_PATCH_LOCKS { @@ -400,6 +399,21 @@ pub async fn withhold_user_patched( .map(|(rel, text)| (rel.as_str(), text.as_str())) .collect(); let index = crate::patch::user_patches::UserPatchIndex::npm_family_in(view, &locks).await; + withhold_user_patched_by(&index, candidates, skipped) +} + +/// [`withhold_user_patched`] by evidence the caller already holds: the +/// disk flow passes what its consent gate's walk did not read (a Rush +/// lock), [`UserPatchIndex::from_locks`], instead of walking the project +/// a second time. +/// +/// [`UserPatchIndex::from_locks`]: crate::patch::user_patches::UserPatchIndex::from_locks +pub fn withhold_user_patched_by( + index: &crate::patch::user_patches::UserPatchIndex, + candidates: &mut Vec, + skipped: &mut Vec, +) -> Vec { + let mut warnings = Vec::new(); if index.is_empty() { return warnings; } diff --git a/crates/socket-patch-core/src/hosted/memory/select.rs b/crates/socket-patch-core/src/hosted/memory/select.rs index a61e1b555..82e5ca52a 100644 --- a/crates/socket-patch-core/src/hosted/memory/select.rs +++ b/crates/socket-patch-core/src/hosted/memory/select.rs @@ -175,6 +175,9 @@ fn classify(rel: &str, root_files: &BTreeSet<&str>) -> Option { } return None; } + if let Some(need) = user_patch_need(rel) { + return Some(need); + } let rush = root_files.contains("rush.json"); if rush && (rel == RUSH_COMMON_LOCK_REL || is_rush_subspace_file(rel, "pnpm-lock.yaml")) { return Some(Need::Text); @@ -200,6 +203,28 @@ fn classify(rel: &str, root_files: &BTreeSet<&str>) -> Option { None } +/// What the engine's user-patch walk +/// ([`crate::patch::user_patches::UserPatchIndex::npm_family_in`]) needs +/// of the nested file `rel`, outside the directories it never enters: +/// which directories are workspace members, and which a `--patch-dir` +/// names, is only known from content, so every file it could read is +/// selected by name. A `package.json`, `pnpm-workspace.yaml`, +/// `pnpm-lock.yaml` or `yarn.lock` is read (text); a patch-package file +/// counts by its name alone (present). With every such file selected, a +/// patch directory the engine finds no `.patch` file in holds none. +fn user_patch_need(rel: &str) -> Option { + use crate::patch::user_patches::WALK_SKIP; + let (dir, base) = split_path(rel); + if dir.split('/').any(|seg| WALK_SKIP.contains(&seg)) { + return None; + } + match base { + "package.json" | "pnpm-workspace.yaml" | "pnpm-lock.yaml" | "yarn.lock" => Some(Need::Text), + _ if base.ends_with(".patch") => Some(Need::Present), + _ => None, + } +} + /// What a pnpm workspace root (one with a `pnpm-workspace.yaml`) needs of /// its nested file `rel` to read its members' own locks (#492, see /// `utils::pnpm_workspace::member_locks`): in a directory holding a package diff --git a/crates/socket-patch-core/src/patch/user_patches.rs b/crates/socket-patch-core/src/patch/user_patches.rs index dbbe542a9..fc2382b31 100644 --- a/crates/socket-patch-core/src/patch/user_patches.rs +++ b/crates/socket-patch-core/src/patch/user_patches.rs @@ -16,6 +16,7 @@ use crate::formats::text::strip_bom; use crate::utils::fs::read_regular_to_string; use crate::utils::npm_range::npm_range_admits; use crate::utils::purl::{normalize_purl, strip_purl_qualifiers}; +use crate::utils::relpath::normalize_lexically_keeping_escapes; use crate::utils::workspace_globs::{workspaces_include_as, WorkspacesDialect}; use crate::vendor::lock_inventory::ProjectView; @@ -51,7 +52,9 @@ impl UserPatchIndex { /// such fallback. pub async fn discover_scoped(root: &Path, reads: ReadScope) -> Self { let root = std::path::absolute(root).unwrap_or_else(|_| root.to_path_buf()); - Self::walk(&ProjectView::Disk(&root), Some(&root), Scope::Gate, reads).await + Self::walk(&ProjectView::Disk(&root), Some(&root), Scope::Gate, reads) + .await + .0 } /// The patches a project's pnpm, yarn or patch-package setup applies @@ -75,71 +78,96 @@ impl UserPatchIndex { /// such as patch-package. Sources are reported root-relative. pub(crate) async fn npm_family_in(view: &ProjectView<'_>, locks: &[(&str, &str)]) -> Self { // Discovery and the hosted engine only ever run on a workspace. - let mut index = Self::walk(view, None, Scope::NpmFamily, ReadScope::Workspace).await; + let (mut index, walked) = + Self::walk(view, None, Scope::NpmFamily, ReadScope::Workspace).await; + // A directory the walk visited had its pnpm and yarn locks read + // already; only the others (a Rush lock, `shrinkwrap.yaml`, a lock + // in a directory the walk never reached) are parsed here. + let fresh: Vec<(&str, &str)> = locks + .iter() + .copied() + .filter(|(rel, _)| { + let path = Path::new(rel); + let dir = path.parent().unwrap_or(Path::new("")); + !(walked.contains(dir) + && matches!( + path.file_name().and_then(|n| n.to_str()), + Some("pnpm-lock.yaml" | "yarn.lock") + )) + }) + .collect(); + index.add_locks(&fresh); + index + } + + /// The patches the pnpm locks (`pnpm-lock.yaml`, `shrinkwrap.yaml`) and + /// yarn locks among `locks` (root-relative path and text) record, as + /// [`Self::npm_family_in`] reads them: for a lock no walk reaches, such + /// as a Rush lock. + pub fn from_locks(locks: &[(&str, &str)]) -> Self { + let mut index = Self::default(); + index.add_locks(locks); + index + } + + fn add_locks(&mut self, locks: &[(&str, &str)]) { for (rel, text) in locks { let path = Path::new(rel); match rel.rsplit('/').next() { Some("pnpm-lock.yaml" | "shrinkwrap.yaml") => { - index.pnpm_yaml(strip_bom(text), path, true, false); + self.pnpm_yaml(strip_bom(text), path, true, false); } - Some("yarn.lock") => index.yarn_lock(text, path), + Some("yarn.lock") => self.yarn_lock(text, path), _ => {} } } - index } /// Whether the index holds no evidence at all. - pub(crate) fn is_empty(&self) -> bool { + pub fn is_empty(&self) -> bool { self.entries.is_empty() } /// The walk [`Self::discover`] and [`Self::npm_family_in`] share. Every - /// project file is read through `view` at its root-relative path (`..` - /// leading to an ancestor). `shown_root` is where reported sources are - /// anchored: the absolute project root for the gate, `None` for - /// root-relative sources. + /// project file is read through `view` at its root-relative path, or, + /// for an ancestor directory, at its absolute path. `shown_root` is + /// where reported sources are anchored: the absolute project root for + /// the gate, `None` for root-relative sources. Also returns the + /// directories visited (as read: root-relative, or absolute). /// - /// The ancestor walk stops at the directory holding `.git`, and without - /// a `.git` the gate's container rule holds (the walk never reaches the - /// home directory or above it). In a [`ReadScope::Workspace`] run - /// nothing outside that git repository root is read: a workspace member - /// or patch-package directory beyond it protects every npm package - /// instead of being read. An in-memory project is its own repository - /// root. A [`ReadScope::Global`] run reads members and patch-package - /// directories wherever they are. + /// The ancestor walk is git's own repository lookup + /// ([`crate::utils::repo_root::search_dirs`]: through the directory + /// holding `.git`, never into the home directory or a + /// `GIT_CEILING_DIRECTORIES` entry), and each ancestor is checked and + /// read at the same (canonical) path. In a [`ReadScope::Workspace`] run + /// nothing outside that git repository root is read: a workspace + /// member or patch-package directory beyond it protects every package + /// of the ecosystem that named it instead of being read. An in-memory + /// project is its own repository root. A [`ReadScope::Global`] run + /// reads members and patch-package directories wherever they are. async fn walk( view: &ProjectView<'_>, shown_root: Option<&Path>, scope: Scope, reads: ReadScope, - ) -> Self { + ) -> (Self, BTreeSet) { let mut index = Self::default(); let shown_base = shown_root.unwrap_or(Path::new("")); let at = |rel: &Path| Dir { rel: rel.to_path_buf(), - shown: lexical_join(shown_base, rel), + shown: normalize_lexically_keeping_escapes(&shown_base.join(rel)), }; - let mut pending = VecDeque::from([at(Path::new(""))]); - // How many levels above the project root the repository root is; - // `None`: no repository root is known. - let mut repo_depth: Option = None; + // Each pending directory with the ecosystem whose declaration + // brought it in. + let mut pending = VecDeque::from([(at(Path::new("")), "npm")]); + let mut bound = Bound::Memory; if let Some(root) = view.disk_root_reading(std::iter::empty::<&Path>()) { // A member invocation still inherits the workspace's patch - // policy. The walk stops at the checkout's `.git`, and never - // reaches the home directory (whose `~/.cargo/config.toml` is - // user-wide, not a workspace) or above it, as in a container - // build with no `.git`. - let root = std::path::absolute(root).unwrap_or_else(|_| root.to_path_buf()); - let home = crate::utils::fs::home_dir(); - let mut probes: Vec = vec![root.join(".git")]; - if root.join(".git").exists() { - repo_depth = Some(0); - } - for (depth, parent) in root.ancestors().enumerate().skip(1) { - if repo_depth.is_some() || home.as_deref().is_some_and(|h| h.starts_with(parent)) { - break; - } + // policy. + let dirs = crate::utils::repo_root::search_dirs(root); + let root_canon = dirs.first().cloned().unwrap_or_else(|| root.to_path_buf()); + let mut probes: Vec = Vec::new(); + for (depth, parent) in dirs.iter().enumerate().skip(1) { const MARKERS: [&str; 7] = [ "pnpm-workspace.yaml", "go.work", @@ -163,29 +191,33 @@ impl UserPatchIndex { .await .is_ok_and(|s| s.contains("[workspace]")) { - let rel: std::path::PathBuf = std::iter::repeat_n("..", depth).collect(); - pending.push_back(Dir { - shown: shown_root.map_or_else(|| rel.clone(), |_| parent.to_path_buf()), - rel, - }); - } - if parent.join(".git").exists() { - repo_depth = Some(depth); + let up: std::path::PathBuf = std::iter::repeat_n("..", depth).collect(); + pending.push_back(( + Dir { + rel: parent.clone(), + shown: shown_root.map_or(up, |_| parent.clone()), + }, + "npm", + )); } } // Fingerprint what the probes read for a snapshot's reuse. let _ = view.disk_root_reading(&probes); - } else { - repo_depth = Some(0); + bound = match crate::utils::repo_root::find_git_repo(root) { + Some(repo) => Bound::Disk { + root: root_canon, + repo: repo.root, + }, + None => Bound::None, + }; } // The read bound: the repository root in a workspace run, none in a // global one. - let repo_depth = match reads { - ReadScope::Workspace => repo_depth, - ReadScope::Global => None, - }; + if reads == ReadScope::Global { + bound = Bound::None; + } let mut seen = BTreeSet::new(); - while let Some(dir) = pending.pop_front() { + while let Some((dir, ecosystem)) = pending.pop_front() { if !seen.insert(dir.rel.clone()) { continue; } @@ -193,9 +225,9 @@ impl UserPatchIndex { index.unknown("npm", &dir.shown, "workspace inspection limit exceeded"); break; } - if outside_repo(&dir.rel, repo_depth) { + if bound.outside(&dir.rel) { index.unknown( - "npm", + ecosystem, &dir.shown, "workspace outside the git repository is not inspected", ); @@ -217,17 +249,17 @@ impl UserPatchIndex { // walking them can only find more patches. globs.extend(Workspaces::of(&value).into_listed().into_iter().flatten()); index - .list_patch_directory(view, &dir.file("patches"), false, None) + .list_patch_directory(view, &dir.file("patches"), false, &Bound::None) .await; for patch_dir in index.patch_package_dirs(&manifest.shown, &value) { let patch_dir = dir.file(&patch_dir); index - .list_patch_directory(view, &patch_dir, true, repo_depth) + .list_patch_directory(view, &patch_dir, true, &bound) .await; } } else { index - .list_patch_directory(view, &dir.file("patches"), false, None) + .list_patch_directory(view, &dir.file("patches"), false, &Bound::None) .await; } let yarn_lock = dir.file("yarn.lock"); @@ -251,34 +283,28 @@ impl UserPatchIndex { } } if scope == Scope::Gate { - index.gate_only(view, &dir, &mut pending, shown_base).await; + for member in index.gate_only(view, &dir).await { + pending.push_back((at(&member), "golang")); + } } for pattern in globs .iter() .filter(|g| !g.starts_with('!') || g.starts_with("!(")) { - for rel in index - .workspace_members(view, &dir, pattern, repo_depth) - .await - { - pending.push_back(at(&rel)); + for rel in index.workspace_members(view, &dir, pattern, &bound).await { + pending.push_back((at(&rel), "npm")); } } } - index + (index, seen) } /// The declarations only the consent gate reads in each directory: /// npm's and Bun's lock-entry `patched` records and mirrored maps, and - /// the Composer, Cargo and Go overrides (a `go.work`'s members join - /// the walk). - async fn gate_only( - &mut self, - view: &ProjectView<'_>, - dir: &Dir, - pending: &mut VecDeque, - shown_base: &Path, - ) { + /// the Composer, Cargo and Go overrides. Returns a `go.work`'s `use` + /// members, which join the walk for the Go ecosystem. + async fn gate_only(&mut self, view: &ProjectView<'_>, dir: &Dir) -> Vec { + let mut members = Vec::new(); for file in ["package-lock.json", "npm-shrinkwrap.json", "bun.lock"] { let path = dir.file(file); // Like the pnpm lock: a lock that never says `patched` holds @@ -326,15 +352,12 @@ impl UserPatchIndex { // Workspace members' go.mod replaces still apply under go.work. if file == "go.work" { for member in crate::vendor::go_mod_edit::parse_use_dirs(&text) { - let rel = lexical_join(&dir.rel, Path::new(&member)); - pending.push_back(Dir { - shown: lexical_join(shown_base, &rel), - rel, - }); + members.push(normalize_lexically_keeping_escapes(&dir.rel.join(member))); } } } } + members } /// [`Self::read`] through a project view. @@ -371,16 +394,18 @@ impl UserPatchIndex { } /// The patch-package files of directory `dir`. `required`: a directory - /// a script names, which must exist; `repo_depth` bounds it to the git - /// repository ([`Self::walk`]). + /// a script names, which must exist (on disk: an in-memory project + /// holds a directory only through a file below it, and path selection + /// supplies every `*.patch` file, so one with no listing holds no + /// patch); `bound` keeps it inside the git repository ([`Self::walk`]). async fn list_patch_directory( &mut self, view: &ProjectView<'_>, dir: &Dir, required: bool, - repo_depth: Option, + bound: &Bound, ) { - if outside_repo(&dir.rel, repo_depth) { + if bound.outside(&dir.rel) { self.unknown( "npm", &dir.shown, @@ -388,13 +413,14 @@ impl UserPatchIndex { ); return; } + let in_memory = matches!(view, ProjectView::Memory(_)); match view.list_dir(&dir.view_rel()).await { Ok(entries) => { for entry in entries { self.patch_file(&entry.name, &dir.shown.join(&entry.name), required); } } - Err(e) if !required && e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) if (!required || in_memory) && e.kind() == std::io::ErrorKind::NotFound => {} Err(e) => self.unknown( "npm", &dir.shown, @@ -412,7 +438,7 @@ impl UserPatchIndex { view: &ProjectView<'_>, dir: &Dir, pattern: &str, - repo_depth: Option, + bound: &Bound, ) -> Vec { if Path::new(pattern).is_absolute() { self.unknown("npm", &dir.shown, "workspace outside the inspection root"); @@ -454,7 +480,7 @@ impl UserPatchIndex { .split('/') .take_while(|p| !p.contains(['*', '?', '{', '[', '('])) .collect(); - if outside_repo(&root.rel, repo_depth) { + if bound.outside(&root.rel) { self.unknown( "npm", &root.shown, @@ -475,12 +501,7 @@ impl UserPatchIndex { .count() .saturating_sub(prefix.components().count()) }; - let skipped = |name: &str| { - matches!( - name, - "node_modules" | ".git" | ".socket" | "target" | ".venv" - ) - }; + let skipped = |name: &str| WALK_SKIP.contains(&name); let mut out = Vec::new(); let patterns = vec![pattern.to_string()]; // Depth-first, as `walkdir` walked it: (directory, depth, its @@ -1023,8 +1044,8 @@ struct Dir { impl Dir { fn file(&self, name: &str) -> Dir { Dir { - rel: lexical_join(&self.rel, Path::new(name)), - shown: lexical_join(&self.shown, Path::new(name)), + rel: normalize_lexically_keeping_escapes(&self.rel.join(name)), + shown: normalize_lexically_keeping_escapes(&self.shown.join(name)), } } @@ -1042,41 +1063,44 @@ impl Dir { } } -/// `base` joined with `rel`, resolving `.` and `..` lexically (an absolute -/// `rel` replaces `base`; `..` above a relative `base` stays a leading -/// `..`). -fn lexical_join(base: &Path, rel: &Path) -> std::path::PathBuf { - use std::path::Component; - let mut out = base.to_path_buf(); - for component in rel.components() { - match component { - Component::CurDir => {} - Component::ParentDir => match out.components().next_back() { - Some(Component::Normal(_)) => { - out.pop(); - } - Some(Component::RootDir | Component::Prefix(_)) => {} - _ => out.push(".."), - }, - other => out.push(other.as_os_str()), - } - } - out +/// The directories the user-patch walk never enters, which path selection +/// skips too. +pub(crate) const WALK_SKIP: [&str; 5] = ["node_modules", ".git", ".socket", "target", ".venv"]; + +/// What bounds a [`UserPatchIndex::walk`]'s reads. +enum Bound { + /// Nothing: a global run, or a disk project with no git repository + /// above it (the ancestor walk's home rule still applies). + None, + /// A disk project at `root` inside the checkout at `repo` (both + /// canonical, as [`crate::utils::repo_root::search_dirs`] spells them). + Disk { + root: std::path::PathBuf, + repo: std::path::PathBuf, + }, + /// An in-memory project, its own repository root. + Memory, } -/// Whether root-relative `rel` lies outside the git repository whose root -/// is `repo_depth` levels above the project root (`None`: unknown, so -/// nothing is). -fn outside_repo(rel: &Path, repo_depth: Option) -> bool { - let Some(depth) = repo_depth else { - return false; - }; - rel.is_absolute() - || rel - .components() - .take_while(|c| matches!(c, std::path::Component::ParentDir)) - .count() - > depth +impl Bound { + /// Whether `rel` (root-relative, or absolute) lies outside the bound. + /// An absolute path is inside when it lexically lies under the + /// repository root. + fn outside(&self, rel: &Path) -> bool { + match self { + Bound::None => false, + Bound::Memory => { + rel.is_absolute() + || matches!( + rel.components().next(), + Some(std::path::Component::ParentDir) + ) + } + Bound::Disk { root, repo } => { + !normalize_lexically_keeping_escapes(&root.join(rel)).starts_with(repo) + } + } + } } /// The package a Yarn locator installs. An alias (`alias@npm:real@1.0.0`) @@ -1718,6 +1742,100 @@ mod tests { assert_eq!(ReadScope::of_global(false), ReadScope::Workspace); } + /// Review (#2018): a `go.work` `use` member outside the git repository + /// is not read in a workspace run, and protects every Go module (the + /// ecosystem whose declaration named it), not every npm package. + #[tokio::test] + async fn an_out_of_repo_go_member_protects_go_not_npm() { + let outer = tempfile::tempdir().unwrap(); + let repo = outer.path().join("repo"); + write(&repo, ".git/HEAD", "ref: refs/heads/main\n"); + write(&repo, "go.work", "go 1.22\n\nuse (\n\t.\n\t../shared\n)\n"); + write(&repo, "go.mod", "module example.com/app\n\ngo 1.22\n"); + write( + outer.path(), + "shared/go.mod", + "module example.com/shared\n\ngo 1.22\n\nreplace example.com/dep => ../dep\n", + ); + let index = UserPatchIndex::discover(&repo).await; + let go = index.conflicts("pkg:golang/example.com/other@v1.0.0"); + assert!( + go.iter().any(|s| s.contains("outside the git repository")), + "{go:?}" + ); + assert!( + index.conflicts("pkg:npm/left-pad@1.3.0").is_empty(), + "{index:?}" + ); + // A global run reads it: only the replaced module is protected. + let global = UserPatchIndex::discover_scoped(&repo, ReadScope::Global).await; + assert!(!global + .conflicts("pkg:golang/example.com/dep@v1.0.0") + .is_empty()); + assert!(global + .conflicts("pkg:golang/example.com/other@v1.0.0") + .is_empty()); + } + + /// Review (#2018): an absolute `--patch-dir` inside the repository is + /// read like a relative one (it is not "outside" for being absolute). + #[tokio::test] + async fn an_absolute_patch_dir_inside_the_repository_is_read() { + let tmp = tempfile::tempdir().unwrap(); + let repo = std::fs::canonicalize(tmp.path()).unwrap(); + write(&repo, ".git/HEAD", "ref: refs/heads/main\n"); + let fixes = repo.join("fixes"); + write( + &repo, + "package.json", + &format!( + r#"{{"scripts":{{"postinstall":"patch-package --patch-dir {}"}}}}"#, + fixes.display() + ), + ); + write(&repo, "fixes/left-pad+1.3.0.patch", "diff\n"); + let index = UserPatchIndex::discover(&repo).await; + assert!( + !index.conflicts("pkg:npm/left-pad@1.3.0").is_empty(), + "{index:?}" + ); + assert!( + index.conflicts("pkg:npm/other@1.0.0").is_empty(), + "{index:?}" + ); + } + + /// Review (#2018): through a symlinked project root, the ancestor + /// workspace settings are found and read where the directory really is + /// (where a `..` read lands), not at the link's lexical parent. + #[cfg(unix)] + #[tokio::test] + async fn a_symlinked_root_reads_its_real_ancestors() { + let outer = tempfile::tempdir().unwrap(); + let real = outer.path().join("real"); + write(&real, ".git/HEAD", "ref: refs/heads/main\n"); + write( + &real, + "pnpm-workspace.yaml", + "packages:\n - 'app'\npatchedDependencies:\n left-pad@1.3.0: p.patch\n", + ); + write(&real, "app/package.json", "{}"); + let link = outer.path().join("link"); + std::os::unix::fs::symlink(real.join("app"), &link).unwrap(); + for view_root in [&link, &real.join("app")] { + let index = UserPatchIndex::discover(view_root).await; + assert!( + !index.conflicts("pkg:npm/left-pad@1.3.0").is_empty(), + "{}: {index:?}", + view_root.display() + ); + let view = ProjectView::Disk(view_root); + let index = UserPatchIndex::npm_family_in(&view, &[]).await; + assert!(!index.conflicts("pkg:npm/left-pad@1.3.0").is_empty()); + assert!(index.conflicts("pkg:npm/other@1.0.0").is_empty()); + } + } + /// The view-based index the hosted engine and `vex` discovery read /// ([`UserPatchIndex::npm_family_in`]) protects exactly what the CLI's /// consent gate ([`UserPatchIndex::discover`]) protects for a root