From 98d2edd7c3ec50a4dbf964ca38c81fdeb00e3180 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Tue, 20 Oct 2026 16:47:37 -0400 Subject: [PATCH 1/9] fix(cli)!: require consent before patching user-patched packages Detect project patch ownership across pnpm, Yarn, patch-package, npm, Bun, Composer, Cargo and Go. Skip protected scan candidates and refuse explicit mutations unless --allow-user-patches is passed. Preserve native backend refusals and require explicit consent before overwriting unknown installed file contents. BREAKING CHANGE: declared project patches are protected by default and agent apply now rejects unknown file-content drift. Document the next-major-release requirement, detection limits and invocation-only consent in the CLI contract. --- crates/socket-patch-cli/CLI_CONTRACT.md | 84 +- crates/socket-patch-cli/src/args.rs | 18 +- .../src/commands/agent_download.rs | 2 + crates/socket-patch-cli/src/commands/apply.rs | 33 +- crates/socket-patch-cli/src/commands/get.rs | 20 + crates/socket-patch-cli/src/commands/mod.rs | 1 + .../src/commands/scan/hosted.rs | 14 + .../socket-patch-cli/src/commands/scan/mod.rs | 22 +- .../src/commands/user_patches.rs | 78 ++ .../socket-patch-cli/src/commands/vendor.rs | 44 + .../src/commands/vendored_backend/mod.rs | 11 + .../tests/apply/apply_network.rs | 26 +- .../apply/cli_gem_variant_mismatch_policy.rs | 16 +- .../socket-patch-cli/tests/cli_global_args.rs | 8 +- .../socket-patch-cli/tests/cli_parse_get.rs | 3 + .../tests/cli_parse_repair.rs | 3 + .../socket-patch-cli/tests/cli_parse_vex.rs | 3 + .../tests/get/get_failure_paths.rs | 68 +- .../socket-patch-cli/tests/get/get_fixture.rs | 1 + .../tests/in_process_cargo_apply.rs | 1 + .../tests/in_process_get_update_count.rs | 1 + .../tests/scan/hosted_failure_paths.rs | 2 +- .../tests/scan/scan_failure_paths.rs | 82 +- crates/socket-patch-cli/tests/user_patches.rs | 194 ++++ .../tests/vendor/in_process_vendor_bun.rs | 8 +- crates/socket-patch-cli/tests/vendor_eject.rs | 41 + crates/socket-patch-core/src/patch/apply.rs | 10 +- crates/socket-patch-core/src/patch/mod.rs | 1 + .../src/patch/user_patches.rs | 898 ++++++++++++++++++ docs/ecosystems.md | 37 + docs/usage.md | 22 + 31 files changed, 1704 insertions(+), 48 deletions(-) create mode 100644 crates/socket-patch-cli/src/commands/user_patches.rs create mode 100644 crates/socket-patch-cli/tests/user_patches.rs create mode 100644 crates/socket-patch-core/src/patch/user_patches.rs diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 3756e7abf..71f314617 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -102,7 +102,8 @@ Every subcommand accepts the same set of "global" flags via a single shared `Glo | `--vendor-url` | — | `SOCKET_VENDOR_URL` | (active API/proxy base) | string | Base host for the vendoring-service package-reference request | | `--patch-server-url` | — | `SOCKET_PATCH_SERVER_URL` | (server-returned) | string | Override the host of the prebuilt-archive download URL (local-dev / testing) | | `--offline` | — | `SOCKET_OFFLINE` | `false` | bool | **Strict airgap on every command** — never contact the network | -| `--strict` | — | `SOCKET_STRICT` | `false` | bool | Treat a beforeHash mismatch as a hard error in the in-place apply paths (see the mismatch-policy note below) | +| `--strict` | — | `SOCKET_STRICT` | `false` | bool | Keep rejecting file-content mismatches even with `--allow-user-patches`; mismatches already fail by default without consent. `apply --force` overrides this file check, but never grants consent for declared project patches. | +| `--allow-user-patches` | — | — | `false` | bool | Explicit, invocation-only consent to attempt Socket patches alongside project patches/replacements, or overwrite modified installed files. No environment or repository-policy binding. Backend safety refusals still apply. See [Existing project patches](#existing-project-patches). | | `--global` | `-g` | `SOCKET_GLOBAL` | `false` | bool | Operate on globally-installed packages | | `--global-prefix` | — | `SOCKET_GLOBAL_PREFIX` | (auto) | path | Override global packages root | | `--json` | `-j` | `SOCKET_JSON` | `false` | bool | Machine-readable output | @@ -121,7 +122,80 @@ Every subcommand accepts the same set of "global" flags via a single shared `Glo `--offline` means the same thing on every command (v3.0): never contact the network, fail loudly when a required local source is missing. On `repair`, `--offline` and `--download-only` are mutually exclusive (exit 2). `scan` and `get` need remote data for their core function (patch discovery / patch fetch), so `--offline` refuses them up front — exit 1 with an error naming the offline gate (JSON: `status: "error"`), before any crawl, client build, or network contact. This covers `scan --mode vendored` too: offline vendored staging is `vendor --offline`'s job. -The `--strict` mismatch policy applies to the in-place apply paths (apply/get/scan --mode agent/hook/go redirect). DEFAULT (v3.4): a file whose on-disk content matches neither the patch's beforeHash nor its afterHash is overwritten with the FULL verified patched content (blob writes are hash-gated to exactly afterHash; the missing blob is downloaded on demand) and surfaced as a `content_mismatch_overwritten` stderr warning + Skipped event (agent-mode `get` / `scan --json`: a `warnings[]` entry, see "Agent-mode mismatch overwrites"). A file the patch adds (empty beforeHash) that already exists with other content is the same case. `--strict` turns that case into a hard error. Rollback of an added file deletes it; the content it replaced is not kept. `--force` overrides `--strict` and additionally skips missing files. Vendoring is unaffected: it commits the patch server's verified artifact and never reads the installed bytes. +The in-place apply paths (`apply`, agent-mode `get` / `scan`, and their nested +apply calls) reject files matching neither `beforeHash` nor `afterHash` by +default. This also protects a pre-existing file where the Socket patch adds a +new file (empty `beforeHash`). `--allow-user-patches` permits the old +warn-and-overwrite behavior: only the full, hash-verified `afterHash` content +can be written, with `content_mismatch_overwritten` diagnostics. `--strict` +keeps rejecting file drift even with that consent. `apply --force` remains +explicit consent for file drift and additionally skips missing files, but a +**declared project patch still requires `--allow-user-patches`**. Overwriting +is not a merge: the replaced local content is not kept for rollback. +Vendoring commits the service artifact without merging installed changes. + +## Existing project patches + +**Release classification: MAJOR.** This intentionally tightens the default +file-mismatch policy and package eligibility. Publish it with the next major +release under the semver policy; version stamps and release notes belong to the +release train. + +Project-owned patches are protected at package level, even if they touch a +different file, are not installed yet, or all Socket `beforeHash` checks pass. +A missing referenced diff does not remove that protection. + +- `scan` (every mode, including previews) excludes matching packages before patch + selection and emits `warnings[].code: user_patch_conflict`, naming the purl + and evidence path. Other packages remain eligible; skips alone exit 0. A nested + agent apply also checks existing in-scope manifest entries and refuses if any + conflict remains, even if the newly selected records have already been saved. + The full installed inventory still protects skipped packages from prune-as-missing. +- Explicit `get` selection (including UUID, `--save-only`, and `--dry-run`), + mutating `apply`, and `vendor` refuse selected conflicts with exit 1, + `status: error`, `error.code: user_patch_conflict`, before their patch writes. + A hosted or vendored writer rechecks under its apply lock before rewiring. +- `--allow-user-patches` 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_USER_PATCHES` do not grant + this consent. Automatic scans never introduce a confirmation prompt. +- Consent does not turn an unsupported composition into a supported one. Native + Bun/npm refusals, Yarn layout restrictions, source ownership, integrity, + path-safety and other backend checks still apply. The CLI never edits, deletes, + rebases, or evaluates user patch files to make a Socket patch fit. +- Read-only `apply --check`, `vendor --check`, `list` and `vex`, and explicit + 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. + +Detection covers root and declared npm workspace manifests, ancestor workspace +settings, and the following evidence: + +| Mechanism | Evidence | +| --- | --- | +| pnpm | `pnpm.patchedDependencies` in `package.json`; `patchedDependencies` in `pnpm-workspace.yaml` and `pnpm-lock.yaml`; registry lock keys carrying `patch_hash` or `patched` markers, including legacy and multi-document locks | +| Yarn Berry | User `patch:` references in dependency/resolution fields or lock resolutions; builtin-only compatibility patches are exempt | +| `patch-package` (npm / Yarn Classic / other npm layouts) | Default `patches/` filenames, including scoped, nested (`++`), sequenced and `.dev.patch` files; literal `--patch-dir` paths in package scripts | +| Native npm / Bun | Root `patchedDependencies`; npm lock `patched` records; Bun text-lock declarations (binary Bun installs also have the root declaration) | +| Composer Patches | Root `extra.patches`, external `patches.json` / configured `patches-file`, `patches.lock.json`, and dependency `extra.patches` in `composer.lock` | +| Cargo | `[patch]` (including renamed `package` entries), `[replace]`, and `.cargo/config[.toml]` `paths` overrides; Socket-owned vendored paths are exempt | +| Go | User-owned `replace` directives in `go.mod` / `go.work`; Socket-owned replacements are exempt | + +Exact npm versions and supported npm ranges are matched against the selected +release; name-only and `*` declarations cover all releases, including prereleases. +Unrecognized ranges protect +the named package conservatively. Matching is conservative across copies and +workspace members: one declaration protects every selected copy of that release. +Unreadable/malformed metadata, dynamic patch directories, or a workspace glob the +bounded reader cannot inspect protect the affected ecosystem with an explanatory +conflict diagnostic. Filesystem inspection never executes hooks or fetches patch +URLs. Arbitrary scripts, external build pipelines, unrecorded edits to files the +Socket patch does not touch, and global-install patch declarations cannot be +identified reliably; rejecting unknown file drift remains the fallback. This is +not a proof that two patches compose, even after opt-in. See the +[research and source references](../../docs/ecosystems.md#existing-project-patches). + ## Per-subcommand arguments @@ -1517,7 +1591,7 @@ command's integration tests check its output. | `vendor_artifact_redownload_failed` | `failed` | repair: download unavailable, integrity mismatch, or downloaded bytes/inventory differ from the ledger. Existing files are preserved. | | `vendor_artifact_unrepairable` | `failed` | repair: the ledger identity or patch record cannot be trusted or recovered. | | `vendor_uuid_mismatch` | `skipped` | repair: the manifest's patch uuid moved past the vendored artifact — a re-vendor (`vendor` / `scan --mode vendored`) is pending; repair does not cross patch generations. | -| `content_mismatch_overwritten` | `skipped` (warning) | apply (default policy): a file matched NEITHER beforeHash nor afterHash and was overwritten with the full verified patched content. This includes a file the patch adds (empty beforeHash) that already exists with other content. `--strict` turns this case into a `failed` event instead. Agent-mode `get` / `scan --json` carry it as a `(content_mismatch_overwritten) …` `warnings[]` entry (#1004). | +| `content_mismatch_overwritten` | `skipped` (warning) | apply (explicit `--allow-user-patches` / `--force`): a file matched NEITHER beforeHash nor afterHash and was overwritten with the full verified patched content. This includes a file the patch adds (empty beforeHash) that already exists with other content. `--strict` turns this case into a `failed` event instead. Agent-mode `get` / `scan --json` carry it as a `(content_mismatch_overwritten) …` `warnings[]` entry (#1004). | | `vendor_lock_checksums_unsupported` / `vendor_stale_lock_checksum` | `failed` | vendor (gem): an ambiguous/platform CHECKSUMS entry, or a v1-wired lock whose stale token blocks the hot path (run `vendor --revert` + re-vendor). | | `redirect_unattributable` | `skipped` (hosted event) | scan/get `--mode hosted`: the rewriters would pin the patch, but lockfile discovery over the result reads that pin as contested (another lock or requirements file resolves the same version elsewhere, or the pin is not one the package manager consumes), so `vex`, `rollback`, `remove` and `vendor` would refuse it. The candidate is left out of the rewrite, so nothing is written for it; the detail carries discovery's findings. Exit code unchanged. | | `redirect_copy_unpatched` | `warnings[]` (warning) | scan/get `--mode hosted` (pnpm, yarn): the pin was written, but its own lock also installs an unpatched copy of the same `name@version`, which no rewrite reaches: a pnpm `file:` directory or tarball dependency (#935), or a yarn `file:` tarball / directory, URL tarball or berry `portal:` / `link:` dependency declared under another name (`lp2@file:…`, #939, #1236; not repeated for a copy a yarn classic skip warning already names). The detail names the lock entry. Neither the run's own `--vex` nor a later `vex` attests the package (`vex` warns `patched_ref_unattributable`), but discovery still manages the pin, so `rollback` / `remove` undo it. | @@ -1583,10 +1657,14 @@ command's integration tests check its output. | `vendor_prebuilt_stub_invalid` | `failed` | RubyGems: the server stub lacks required attributes or is otherwise invalid; no local stub fallback is permitted. | | `vendor_*` / `pypi_*` / `gemfile_*` / `lock_*` / `locked_version_mismatch` / `user_authored_*` / `native_extensions_unsupported` / `platform_gem_unsupported` | `failed`/`skipped` | vendor: per-ecosystem refusal + drift vocabulary; see the Vendor command contract section. New tags are additive (MINOR). | +`user_patch_conflict` is also a scan run warning for packages excluded by the +project-patch consent gate; `user_patch_override` records explicit consent. + ### Top-level `EnvelopeError` codes | Code | Subcommands | Meaning | |-----------------------|----------------------------------|---------| +| `user_patch_conflict` | get, apply, vendor; hosted/vendored write rechecks | A selected package has project-owned patch/replacement evidence and no `--allow-user-patches` consent. Evidence paths and a remedy are included; exit 1. | | `manifest_not_found` | remove, repair, rollback, vex, apply (only when the manifest vanishes between apply's existence probe and its locked read) (not `list` since v5.0: a missing manifest is an empty list) | `.socket/manifest.json` doesn't exist. For `vex` (and `scan --vex`) it fires only when, in addition, NOTHING else names a patch — no vendor-ledger entry, no lockfile reference (hosted or vendored) — and the message says so (exit 2 standalone; `apply`/`vendor --vex` treat it as their calm no-op). v3.5: `repair` proceeds anyway (vendored phase only) when a vendor ledger or vendor-path lockfile references exist, and exits 0 with a `redirect_only_project` skip (not this error) when the project's only patch state is hosted pins in its lockfiles (v5.0; or a pre-v5 `redirect-state.json`). `list` likewise no longer fires this on a hosted-only project: v5.0 lists every hosted pin the lockfiles wire (exit 0, labeled `details.mode: "hosted"` + `details.lockfiles: []` — no `details.ledger`, since hosted mode keeps none; when the manifest exists too, both are shown, purl-sorted with the manifest entry first on a tie). A pin carries its uuid and empty details unless a pre-v5 redirect ledger records the same purl and uuid (read for migration only: its record supplies the vulnerabilities / tier / description); a pre-v5 ledger record whose pin is in no lockfile is not listed. v5.0: `list` reads the vendor ledger the same way — a vendored-only project (every `scan`/`get --mode vendored` project) lists its ledger entries' embedded records labeled `Mode: vendored (recorded in .socket/vendor/state.json)` in human mode — the twin of the hosted `Mode: hosted (wired in )` line — (`details.mode: "vendored"` + `details.ledger: ".socket/vendor/state.json"` in JSON), exit 0. A standalone-`vendor` entry's fallback `record` lists the same way once no manifest entry covers it (by ledger key or base purl) — the copy manifest-less `vex` attests from, so `list` never reports `manifest_not_found` for a tree whose VEX document attests a patch; while the manifest covers it, only the manifest entry is listed. All sources always come from the SAME project (see the `--manifest-path` flag row), so `--manifest-path` into another project reads that project's state, never the local one. The error still fires when NONE of the three sources has a patch, and a present-but-broken manifest still reports `manifest_invalid`/`manifest_unreadable` regardless (corruption is never masked). A malformed pre-v5 redirect ledger degrades to "nothing to consult" with a stderr warning, muted by `--silent` (the pins still list); `list --json` carries it in the run-level `warnings[]` as `redirect_ledger_corrupt` instead of on stderr. v5.0: `rollback` likewise proceeds manifest-less when the vendor ledger or the lockfiles' hosted pins hold work (its error is the legacy `{status: "error", error: {code: "manifest_not_found", message: "Manifest not found"}, path}` shape); only the truly-empty project — no manifest, no vendor ledger, no hosted pin (a lone pre-v5 redirect ledger is deleted, exit 0) — keeps the exit-1 error, and a project whose lockfiles still reference `.socket/vendor/` artifacts with NO vendor ledger gets a distinct error naming `socket-patch repair`. `remove` (v5.0) proceeds manifest-less whenever a vendor ledger file exists or the lockfiles pin a hosted patch (an existence probe and the read-only hosted-pin discovery before the lock; the vendor ledger loads under it): ANY vendor-ledger entry matching the identifier — detached or not — is removed through the ledger path (`--preserve-state` and drift-keeps behave exactly as on the manifest path), a hosted-only match restores its upstream registry entry, and when that state exists but holds nothing for the identifier the error is `not_found` (exit 1), not this code — `manifest_not_found` fires from `remove` only when all three sources are empty. Manifest entries are removed in sorted purl order. | | `ambiguous_target` | remove, get, rollback | The identifier is a package name whose last-segment rule selects several packages across the manifest, the vendor ledger and the hosted pins (`core` → `@angular/core` and `@babel/core`). Nothing is changed (exit 1); the message names each package. Use the full name or a purl. See **Target grammar**. | | `manifest_invalid` | list, remove, apply (incl. `--check`), repair, vendor (incl. `--check`), vex, rollback, get, scan (agent mode) | Manifest exists but is unparseable (malformed JSON or a schema violation). **v5.0, MAJOR (#931)**: one mapping on every command — `apply` used to say `apply_failed`, `repair` `repair_failed`, `vendor` the undocumented `invalid_manifest`, and `apply --check` / `vendor --check` / `vex` / `get` / `rollback` `manifest_unreadable`. Exit codes unchanged (`vex`: 2, every other command 1). Agent-mode `scan` fails with it (exit 1); the other scan modes, which don't read the manifest, add a warning with the same code and go on. | diff --git a/crates/socket-patch-cli/src/args.rs b/crates/socket-patch-cli/src/args.rs index d78d0e582..b28a082dd 100644 --- a/crates/socket-patch-cli/src/args.rs +++ b/crates/socket-patch-cli/src/args.rs @@ -180,12 +180,10 @@ pub struct GlobalArgs { )] pub offline: bool, - /// Treat a beforeHash mismatch as a hard error. By DEFAULT a file whose - /// on-disk content matches neither the patch's beforeHash nor its - /// afterHash is overwritten with the full verified patched content and - /// surfaced as a stderr warning (`content_mismatch_overwritten`); this - /// flag restores the fail-closed behavior. On commands that have - /// `--force`, `--force` overrides it. + /// Reject modified file content even with --allow-user-patches. Without + /// explicit consent, mismatches already fail by default. On commands that + /// have --force, --force overrides this file-content check (but never the + /// separate check for declared project patches). #[arg( help_heading = GLOBAL_OPTIONS, long, @@ -195,6 +193,13 @@ pub struct GlobalArgs { )] pub strict: bool, + /// Explicitly permit attempting patches on packages with project patches or + /// local modifications. Review and test the combined result; backend safety + /// refusals still apply. --yes does not approve patch composition; --force + /// approves only file-content drift. + #[arg(help_heading = GLOBAL_OPTIONS, long)] + pub allow_user_patches: bool, + /// Operate on globally-installed packages. #[arg( help_heading = GLOBAL_OPTIONS, @@ -780,6 +785,7 @@ impl Default for GlobalArgs { patch_server_url: None, offline: false, strict: false, + allow_user_patches: false, global: false, global_prefix: None, json: false, diff --git a/crates/socket-patch-cli/src/commands/agent_download.rs b/crates/socket-patch-cli/src/commands/agent_download.rs index e23ef9658..9d5d044e4 100644 --- a/crates/socket-patch-cli/src/commands/agent_download.rs +++ b/crates/socket-patch-cli/src/commands/agent_download.rs @@ -367,6 +367,7 @@ pub struct DownloadParams { /// `--strict` forwarded to the nested apply (a beforeHash mismatch /// fails instead of warn-and-overwrite). pub strict: bool, + pub allow_user_patches: bool, /// `--ecosystems` forwarded to the nested apply, so it never touches /// other ecosystems' packages the user filtered out. pub ecosystems: Option>, @@ -1401,6 +1402,7 @@ pub(crate) fn nested_apply_args_from_params( global: params.global, global_prefix: params.global_prefix.clone(), strict: params.strict, + allow_user_patches: params.allow_user_patches, // Scope the nested apply like the caller was scoped: `None` would // apply the WHOLE manifest, mutating other ecosystems' packages the // user filtered out. diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index d7e8ed0aa..dc5ad9efd 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -120,12 +120,13 @@ fn mismatch_overwrite_warnings(results: &[ApplyResult], dry_run: bool) -> Vec MismatchPolicy { +/// Unknown local changes fail closed unless explicitly approved. --force is +/// existing explicit consent for file drift; declared patches have a separate +/// gate that requires --allow-user-patches even with --force. +fn mismatch_policy(force: bool, strict: bool, allow_user_patches: bool) -> MismatchPolicy { if force { MismatchPolicy::Force - } else if strict { + } else if strict || !allow_user_patches { MismatchPolicy::Strict } else { MismatchPolicy::Warn @@ -1039,7 +1040,7 @@ pub(crate) struct ApplyRunReport { /// when `code != 0`. pub applied: Vec, /// Non-fatal per-file warnings the caller's envelope must carry: one - /// `content_mismatch_overwritten` per file the default mismatch policy + /// `content_mismatch_overwritten` per file the explicitly approved mismatch policy /// overwrote (#1004). A nested apply never prints JSON and is silent /// for a JSON caller, so this is their only channel. Filled whenever /// the apply loop ran, whatever the exit code. @@ -1138,6 +1139,15 @@ pub(crate) async fn run_locked( } }; + let user_patch_warnings = + match super::user_patches::check(&args.common, manifest.patches.keys()).await { + Ok(warnings) => warnings, + Err(error) => { + let code = report_apply_failure(&args, &error, &error.message, &telemetry).await; + return ApplyRunReport::run_failure(code, &error.code, error.message); + } + }; + // Package-manager layout detection. yarn-berry PnP keeps packages // inside `.yarn/cache/*.zip` and resolves them via `.pnp.cjs` — // the npm crawler can't reach them and rewriting zips is a @@ -1230,6 +1240,11 @@ pub(crate) async fn run_locked( // rightly ignores it). It rides the run-warning channel so it // is never silent. let mut run_warnings = run_warnings; + run_warnings.extend( + user_patch_warnings + .into_iter() + .map(|(code, detail)| RunWarning { code, detail }), + ); run_warnings.extend(results.iter().filter_map(|r| { let note = r.error.as_deref().filter(|_| r.success)?; note.contains(socket_patch_core::patch::apply::OWNERSHIP_NOT_RESTORED_MARKER) @@ -2146,7 +2161,11 @@ async fn apply_patches_inner( // Apply patches let sources = staged.as_patch_sources(); - let policy = mismatch_policy(args.force, args.common.strict); + let policy = mismatch_policy( + args.force, + args.common.strict, + args.common.allow_user_patches, + ); let mut has_errors = false; // Group release-variant PURLs by base. PyPI (`?artifact_id=`), @@ -3502,7 +3521,7 @@ mod tests { /// `HashMap`-iteration-ordered pick let a variant describing a /// different, NOT-installed distribution randomly match and get /// attempted (nondeterministic spurious failures, or wrong-variant - /// content overwrites under the default mismatch policy). 64 rounds + /// content overwrites under the explicitly approved mismatch policy). 64 rounds /// with fresh maps so the randomized per-instance iteration order is /// actually exercised. #[tokio::test] diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index e84e63eed..b394f7e57 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -1300,6 +1300,17 @@ pub async fn run(args: GetArgs) -> i32 { ) .await; let selected = vec![search_result_from_response(&patch)]; + let user_patch_warnings = + match super::user_patches::check(&args.common, [&patch.purl]).await { + Ok(warnings) => warnings, + Err(error) => { + return super::user_patches::refuse( + &args.common, + JsonCommand::Get, + error, + ) + } + }; // Acting against the repo's socket.yml says so // (`policy_bypassed`). let mut uuid_warnings = @@ -1312,6 +1323,7 @@ pub async fn run(args: GetArgs) -> i32 { // Already on stderr from the client: JSON only, after the // print above. uuid_warnings.extend(org_warnings.iter().cloned()); + uuid_warnings.extend(user_patch_warnings); // Mode dispatch. All three reuse THIS fetched patch and // this possibly-proxy-fallback client rather than // re-fetching with a fresh one, which would re-hit the @@ -1728,6 +1740,10 @@ pub async fn run(args: GetArgs) -> i32 { } else { selected }; + match super::user_patches::check(&args.common, selected.iter().map(|p| &p.purl)).await { + Ok(warnings) => narrow_warnings.extend(warnings), + Err(error) => return super::user_patches::refuse(&args.common, JsonCommand::Get, error), + } if agent_preview { return agent_dry_run(&args, &selected, &narrow_skips, &narrow_warnings).await; } @@ -2165,6 +2181,7 @@ fn get_download_params(args: &GetArgs, save_only: bool, persist_blobs: bool) -> silent: args.common.silent, all_releases: args.all_releases, strict: args.common.strict, + allow_user_patches: args.common.allow_user_patches, ecosystems: args.common.ecosystems.clone(), persist_blobs, patch_server_url: args.common.patch_server_url.clone(), @@ -3845,6 +3862,7 @@ mod tests { silent: true, all_releases: false, strict: false, + allow_user_patches: false, ecosystems: None, persist_blobs: false, patch_server_url: None, @@ -4542,6 +4560,7 @@ mod tests { silent: true, all_releases: false, strict: false, + allow_user_patches: false, ecosystems: None, // The vendor-detached posture this fn exists for. persist_blobs: false, @@ -5619,6 +5638,7 @@ mod tests { let common = GlobalArgs { verbose: true, strict: true, + allow_user_patches: false, json: true, dry_run: true, ..GlobalArgs::default() diff --git a/crates/socket-patch-cli/src/commands/mod.rs b/crates/socket-patch-cli/src/commands/mod.rs index 1c0be1078..daf3d3454 100644 --- a/crates/socket-patch-cli/src/commands/mod.rs +++ b/crates/socket-patch-cli/src/commands/mod.rs @@ -635,3 +635,4 @@ mod tests { assert_eq!(mode, Ok(scan::ScanMode::Vendored)); } } +pub(crate) mod user_patches; diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index 5d6ee98d6..6acbc61f7 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -1041,6 +1041,20 @@ async fn run_redirect_selected_untracked( None }; + // Re-read project patch declarations under the write lock. A patch added + // during API selection must not be lost in a takeover or lock rewrite. + if !common.allow_user_patches { + if let Err(error) = + crate::commands::user_patches::check(common, candidates.iter().map(&sel_purl)).await + { + let command = scan_result + .as_ref() + .map(|env| env.command) + .unwrap_or(crate::json_envelope::Command::Scan); + return crate::commands::user_patches::refuse(common, command, error); + } + } + // v5 hosted mode keeps no ledger: the lockfiles are the only record of // a redirect (vex, list, vendor and rollback read the hosted pins from // them). This run's patch records (fetched below) feed only the diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index a5eadaa04..dfd5cd8bd 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -888,6 +888,7 @@ fn download_params(args: &ScanArgs, save_only: bool, json: bool, silent: bool) - silent, all_releases: args.all_releases, strict: args.common.strict, + allow_user_patches: args.common.allow_user_patches, ecosystems: args.common.ecosystems.clone(), persist_blobs: args.mode != Some(ScanMode::Vendored), patch_server_url: args.common.patch_server_url.clone(), @@ -2212,7 +2213,26 @@ async fn run_scan( .locate_nested_copies(&nm_roots, &filtered_crawled) .await; } - let filtered_crawled = policy.admit_crawled_copies(filtered_crawled, &supplement_purls); + let mut filtered_crawled = policy.admit_crawled_copies(filtered_crawled, &supplement_purls); + // Keep the full crawl above for GC: a protected package is still installed. + // Protect before querying/selecting, in every mode and in dry-run previews. + let user_patches = super::user_patches::inspect(&args.common).await; + filtered_crawled.retain(|package| { + let sources = user_patches.conflicts(&package.purl); + if sources.is_empty() { + return true; + } + layout_refusals.push(( + if args.common.allow_user_patches { + super::user_patches::ALLOWED + } else { + super::user_patches::CONFLICT + } + .into(), + super::user_patches::detail(&package.purl, &sources, args.common.allow_user_patches), + )); + args.common.allow_user_patches + }); // Gradle discovery notes (m2 gating, the user home) ride the run-level // warnings; the lock set only annotates `packages[]` below. diff --git a/crates/socket-patch-cli/src/commands/user_patches.rs b/crates/socket-patch-cli/src/commands/user_patches.rs new file mode 100644 index 000000000..01bf4c835 --- /dev/null +++ b/crates/socket-patch-cli/src/commands/user_patches.rs @@ -0,0 +1,78 @@ +//! 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 crate::args::GlobalArgs; +use crate::json_envelope::{Command, Envelope, EnvelopeError}; + +pub(crate) const CONFLICT: &str = "user_patch_conflict"; +pub(crate) const ALLOWED: &str = "user_patch_override"; + +pub(crate) async fn inspect(common: &GlobalArgs) -> UserPatchIndex { + if common.is_global() { + // A local checkout does not govern a separately selected global tree. + UserPatchIndex::default() + } else { + UserPatchIndex::discover(&common.cwd).await + } +} + +pub(crate) fn detail(purl: &str, sources: &[String], allowed: bool) -> String { + let sources = sources.join(", "); + if allowed { + format!("{purl}: --allow-user-patches permits attempting a Socket patch alongside project patches in {sources}; review and test the combined result. Backend safety checks still apply") + } else { + format!("{purl}: protected by project patch/replacement metadata in {sources}; leaving this package untouched. Merge the fixes manually, or explicitly accept the conflict risk with --allow-user-patches (ordinary --yes/--force do not approve this)") + } +} + +/// Explicit-target commands fail before mutation rather than silently succeeding +/// without the requested patch. The caller may attach approved warnings to its +/// own envelope. Nothing here downloads or applies the user's patch. +pub(crate) async fn check( + common: &GlobalArgs, + purls: impl IntoIterator>, +) -> Result, EnvelopeError> { + let index = inspect(common).await; + let mut warnings = Vec::new(); + let mut conflicts = Vec::new(); + for purl in purls { + let purl = purl.as_ref(); + if !common.purl_ecosystem_selected(purl) { + continue; + } + let sources = index.conflicts(purl); + if sources.is_empty() { + continue; + } + let message = detail(purl, &sources, common.allow_user_patches); + if common.allow_user_patches { + if !common.silent && !common.json { + eprintln!("Warning: {message}"); + } + warnings.push((ALLOWED.into(), message)); + } else { + conflicts.push(message); + } + } + if conflicts.is_empty() { + Ok(warnings) + } else { + conflicts.sort(); + conflicts.dedup(); + Err(EnvelopeError::new(CONFLICT, conflicts.join("\n"))) + } +} + +pub(crate) fn refuse(common: &GlobalArgs, command: Command, error: EnvelopeError) -> i32 { + if common.json { + let mut env = Envelope::new(command); + env.dry_run = common.dry_run; + env.mark_error(error); + crate::ui::print_json(&env.to_value()); + } else { + eprintln!("Error: {}", error.message); + } + 1 +} diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 3516e7c49..a8fdee231 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1728,6 +1728,11 @@ impl EjectSnapshot { /// the project to upstream, not to hosted). async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { let common = &args.common; + let user_patch_warnings = + match super::user_patches::check(common, pins.iter().map(|p| &p.purl)).await { + Ok(warnings) => warnings, + Err(error) => return super::user_patches::refuse(common, Command::Vendor, error), + }; let (client, use_public_proxy) = get_api_client_with_overrides(common.api_client_overrides()).await; let telemetry = TelemetryAuth::for_client(&client); @@ -1783,6 +1788,11 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { // before anything is touched, so every package stays hosted. if !fetch_failures.is_empty() { let mut env = Envelope::new(Command::Vendor); + env.warnings.extend( + user_patch_warnings + .into_iter() + .map(|(code, detail)| crate::json_envelope::RunWarning { code, detail }), + ); env.dry_run = common.dry_run; if common.json { env.warnings @@ -1844,6 +1854,11 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { } if !refused.is_empty() { let mut env = Envelope::new(Command::Vendor); + env.warnings.extend( + user_patch_warnings + .into_iter() + .map(|(code, detail)| RunWarning { code, detail }), + ); env.dry_run = common.dry_run; if common.json { env.warnings @@ -1873,6 +1888,11 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { // preview the vendor step would be a write. if common.dry_run { let mut env = Envelope::new(Command::Vendor); + env.warnings.extend( + user_patch_warnings + .into_iter() + .map(|(code, detail)| RunWarning { code, detail }), + ); env.dry_run = true; if common.json { env.warnings @@ -1926,6 +1946,11 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { return emit_eject_refusal(common, code, &message); } }; + if !common.allow_user_patches { + if let Err(error) = super::user_patches::check(common, pins.iter().map(|p| &p.purl)).await { + return super::user_patches::refuse(common, Command::Vendor, error); + } + } let touched: Vec = pins .iter() .flat_map(|p| p.files.iter().cloned()) @@ -1943,6 +1968,11 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { } }; let mut env = Envelope::new(Command::Vendor); + env.warnings.extend( + user_patch_warnings + .into_iter() + .map(|(code, detail)| RunWarning { code, detail }), + ); if common.json { env.warnings .extend(crate::commands::vex_sources::api_auth_fallback_warning( @@ -2197,6 +2227,20 @@ async fn run_vendor( // previous run whose patches were dropped from the manifest are reverted // even when zero in-scope patches remain. Its post-reconcile ledger // feeds the staging harvest below and then the engine. + match super::user_patches::check(common, manifest.patches.keys()).await { + Ok(warnings) => env.warnings.extend( + warnings + .into_iter() + .map(|(code, detail)| crate::json_envelope::RunWarning { code, detail }), + ), + Err(error) => { + if !common.json { + eprintln!("Error: {}", error.message); + } + env.mark_error(error); + return 1; + } + } let (mut has_errors, ledger) = reconcile_dropped(&manifest, common, env).await; let socket_dir = crate::args::socket_dir_of(manifest_path, &common.cwd); diff --git a/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs b/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs index 0830ec6b1..5132f8ea4 100644 --- a/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs +++ b/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs @@ -93,6 +93,17 @@ impl<'a> VendoredBackend<'a> { /// `scan_run_fits_windows_main_thread_stack`). pub(crate) async fn apply(&self, req: ApplyRequest<'_>, env: &mut Envelope) -> bool { let common: &GlobalArgs = &self.common; + if !common.allow_user_patches { + if let Err(error) = + super::user_patches::check(common, req.manifest.patches.keys()).await + { + if !common.json { + eprintln!("Error: {}", error.message); + } + env.mark_error(error); + return true; + } + } let blobs = req.socket_dir.join("blobs"); let sources = socket_patch_core::patch::apply::PatchSources { blobs_path: &blobs, diff --git a/crates/socket-patch-cli/tests/apply/apply_network.rs b/crates/socket-patch-cli/tests/apply/apply_network.rs index 379af2b57..1027b076f 100644 --- a/crates/socket-patch-cli/tests/apply/apply_network.rs +++ b/crates/socket-patch-cli/tests/apply/apply_network.rs @@ -436,7 +436,7 @@ async fn apply_with_force_overrides_hash_mismatch() { } #[tokio::test] -async fn apply_hash_mismatch_default_warns_and_applies_strict_fails() { +async fn apply_hash_mismatch_opt_in_warns_and_applies_strict_fails() { let after = b"after\n"; let after_hash = git_sha256(after); let expected_before = b"expected-before\n"; @@ -461,11 +461,11 @@ async fn apply_hash_mismatch_default_warns_and_applies_strict_fails() { tmp }; - // DEFAULT: the mismatch is overwritten with the full verified patched + // OPT-IN: the mismatch is overwritten with the full verified patched // content (the blob is hash-gated to afterHash) and surfaced as a warning event — exit 0. let tmp = fixture(); let out = hermetic::command(&binary()) - .args(["apply", "--json", "--offline"]) + .args(["apply", "--allow-user-patches", "--json", "--offline"]) .current_dir(tmp.path()) .output() .expect("run socket-patch"); @@ -474,7 +474,7 @@ async fn apply_hash_mismatch_default_warns_and_applies_strict_fails() { assert_eq!( out.status.code().unwrap_or(-1), 0, - "default mismatch is a warning, not an error: {v:#}" + "opted-in mismatch is a warning, not an error: {v:#}" ); assert_eq!(v["status"], "success", "{v:#}"); let events = v["events"].as_array().expect("events array"); @@ -497,7 +497,7 @@ async fn apply_hash_mismatch_default_warns_and_applies_strict_fails() { // The human run logs the warning to stderr. let tmp = fixture(); let out = hermetic::command(&binary()) - .args(["apply", "--offline", "--yes"]) + .args(["apply", "--allow-user-patches", "--offline", "--yes"]) .current_dir(tmp.path()) .output() .expect("run socket-patch"); @@ -511,7 +511,13 @@ async fn apply_hash_mismatch_default_warns_and_applies_strict_fails() { // --strict: fail closed — exit 1, failed event, file untouched. let tmp = fixture(); let out = hermetic::command(&binary()) - .args(["apply", "--json", "--offline", "--strict"]) + .args([ + "apply", + "--allow-user-patches", + "--json", + "--offline", + "--strict", + ]) .current_dir(tmp.path()) .output() .expect("run socket-patch"); @@ -686,7 +692,7 @@ async fn apply_uses_locally_cached_blob_without_fetching() { // Mismatch: the full blob is downloaded and applied. // --------------------------------------------------------------------------- -/// The default mismatch policy applies the FULL afterHash blob to a +/// The explicitly approved mismatch policy applies the FULL afterHash blob to a /// mismatched file. A leftover legacy package archive is not a source, so /// the stage step still downloads the blob. #[tokio::test] @@ -749,7 +755,7 @@ async fn apply_mismatch_redownloads_full_blob_and_applies() { .unwrap(); } - let (code, stdout, stderr) = run_apply(tmp.path(), &mock.uri(), &[]); + let (code, stdout, stderr) = run_apply(tmp.path(), &mock.uri(), &["--allow-user-patches"]); let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON"); assert_eq!(code, 0, "stdout={v:#}\nstderr={stderr}"); let events = v["events"].as_array().expect("events array"); @@ -944,7 +950,7 @@ async fn apply_online_ignores_legacy_package_archive_when_downloads_fail() { /// Two physical copies of one PURL (a root copy plus a nested duplicate): /// the root copy already carries the afterHash bytes, the nested one was /// locally modified (matches NEITHER hash). The stage step fetches the full -/// afterHash blob, and the default warn-and-overwrite policy must apply it +/// afterHash blob, and the explicitly approved warn-and-overwrite policy must apply it /// to EVERY copy, not just the first (clean, root) one. #[tokio::test] async fn mismatch_overwrites_every_copy_of_a_duplicated_package() { @@ -982,7 +988,7 @@ async fn mismatch_overwrites_every_copy_of_a_duplicated_package() { let socket = tmp.path().join(".socket"); write_manifest_with_patch(&socket, purl, uuid, &before_hash, &after_hash); - let (code, stdout, stderr) = run_apply(tmp.path(), &mock.uri(), &[]); + let (code, stdout, stderr) = run_apply(tmp.path(), &mock.uri(), &["--allow-user-patches"]); let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON"); assert_eq!( code, 0, diff --git a/crates/socket-patch-cli/tests/apply/cli_gem_variant_mismatch_policy.rs b/crates/socket-patch-cli/tests/apply/cli_gem_variant_mismatch_policy.rs index d47c732b3..c9e4b170c 100644 --- a/crates/socket-patch-cli/tests/apply/cli_gem_variant_mismatch_policy.rs +++ b/crates/socket-patch-cli/tests/apply/cli_gem_variant_mismatch_policy.rs @@ -1,17 +1,17 @@ //! Mismatch-policy contract for release-variant (gem) packages. //! -//! For npm, a locally-modified file is handled by the documented default +//! For npm, a locally-modified file is handled by the explicitly approved //! mismatch policy: warn + apply the full verified patched content, //! `--strict` refuses, `--force` also skips missing files (see -//! `apply_network.rs::apply_hash_mismatch_default_warns_and_applies_strict_fails`). +//! `apply_network.rs::apply_hash_mismatch_opt_in_warns_and_applies_strict_fails`). //! Release-variant ecosystems (gem/pypi/maven) route through the variant //! loop instead, whose installed-distribution gate must still let the -//! default policy reach an UNQUALIFIED SINGLETON base (one bare manifest +//! opt-in policy reach an UNQUALIFIED SINGLETON base (one bare manifest //! record for the `package@version`, the common case) with a //! locally-modified file: warn-overwrite, not "no matching variant found". //! //! Behaviors pinned (all offline, real binary, synthetic gem trees): -//! * unqualified singleton + locally-modified file: default apply warns +//! * unqualified singleton + locally-modified file: opted-in apply warns //! (`content_mismatch_overwritten`) AND applies the full afterHash //! bytes; `--strict` refuses (file untouched, exit 1); `--force` //! keeps applying as before. @@ -147,14 +147,14 @@ fn singleton_fixture(cwd: &Path) -> (PathBuf, Vec) { /// the full verified patched content and surfaced as a warning — the same /// npm contract, not "no matching variant found" with the file untouched. #[test] -fn singleton_mismatch_default_warns_and_applies() { +fn singleton_mismatch_opt_in_warns_and_applies() { let tmp = tempfile::tempdir().expect("tempdir"); let (file, patched) = singleton_fixture(tmp.path()); - let (code, _stdout, stderr) = run_apply(tmp.path(), &[]); + let (code, _stdout, stderr) = run_apply(tmp.path(), &["--allow-user-patches"]); assert_eq!( code, 0, - "default mismatch on a singleton variant is a warning, not an error; stderr={stderr}" + "opted-in mismatch on a singleton variant is a warning, not an error; stderr={stderr}" ); assert_eq!( std::fs::read(&file).expect("read patched file"), @@ -178,7 +178,7 @@ fn singleton_mismatch_default_warns_and_applies() { // warning event — the same shape apply_network pins for npm. let tmp = tempfile::tempdir().expect("tempdir"); let (file, patched) = singleton_fixture(tmp.path()); - let (code, stdout, _stderr) = run_apply(tmp.path(), &["--json"]); + let (code, stdout, _stderr) = run_apply(tmp.path(), &["--json", "--allow-user-patches"]); assert_eq!(code, 0, "json run must also succeed; stdout={stdout}"); assert_eq!(std::fs::read(&file).expect("read patched file"), patched); let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON envelope"); diff --git a/crates/socket-patch-cli/tests/cli_global_args.rs b/crates/socket-patch-cli/tests/cli_global_args.rs index c506e5f4e..a84cdbe07 100644 --- a/crates/socket-patch-cli/tests/cli_global_args.rs +++ b/crates/socket-patch-cli/tests/cli_global_args.rs @@ -101,6 +101,9 @@ fn global_flag_cases() -> Vec<(&'static str, Option<&'static str>, fn(&GlobalArg }), ("--offline", None, |c| assert!(c.offline)), ("--strict", None, |c| assert!(c.strict)), + ("--allow-user-patches", None, |c| { + assert!(c.allow_user_patches) + }), ("--global", None, |c| assert!(c.global)), ("--global-prefix", Some("/opt/global"), |c| { assert_eq!(c.global_prefix, Some(PathBuf::from("/opt/global"))) @@ -236,17 +239,18 @@ fn global_flag_cases_cover_every_global_field() { no_npm_allow_remote_config: _, no_vlt_install_cleanup: _, strict: _, + allow_user_patches: _, vendor_source: _, vendor_url: _, patch_server_url: _, maven_config: _, } = common; - // 26 fields ↔ 26 long-flag cases. Bump both this count and add a case when + // 27 fields ↔ 27 long-flag cases. Bump both this count and add a case when // the destructure above forces you to add a field. assert_eq!( global_flag_cases().len(), - 26, + 27, "every GlobalArgs field needs a long-flag case in global_flag_cases()", ); diff --git a/crates/socket-patch-cli/tests/cli_parse_get.rs b/crates/socket-patch-cli/tests/cli_parse_get.rs index f0c770ae2..0f52dfa21 100644 --- a/crates/socket-patch-cli/tests/cli_parse_get.rs +++ b/crates/socket-patch-cli/tests/cli_parse_get.rs @@ -136,6 +136,7 @@ struct Snap { patch_server_url: Option, offline: bool, strict: bool, + allow_user_patches: bool, global: bool, global_prefix: Option, json: bool, @@ -170,6 +171,7 @@ fn snapshot(a: &GetArgs) -> Snap { patch_server_url: a.common.patch_server_url.clone(), offline: a.common.offline, strict: a.common.strict, + allow_user_patches: a.common.allow_user_patches, global: a.common.global, global_prefix: a.common.global_prefix.clone(), json: a.common.json, @@ -212,6 +214,7 @@ fn expected_defaults(identifier: &str) -> Snap { patch_server_url: None, offline: false, strict: false, + allow_user_patches: false, global: false, global_prefix: None, json: false, diff --git a/crates/socket-patch-cli/tests/cli_parse_repair.rs b/crates/socket-patch-cli/tests/cli_parse_repair.rs index c08c72d87..454cedc33 100644 --- a/crates/socket-patch-cli/tests/cli_parse_repair.rs +++ b/crates/socket-patch-cli/tests/cli_parse_repair.rs @@ -132,6 +132,7 @@ struct Snap { patch_server_url: Option, offline: bool, strict: bool, + allow_user_patches: bool, global: bool, global_prefix: Option, json: bool, @@ -159,6 +160,7 @@ fn snapshot(a: &RepairArgs) -> Snap { patch_server_url: a.common.patch_server_url.clone(), offline: a.common.offline, strict: a.common.strict, + allow_user_patches: a.common.allow_user_patches, global: a.common.global, global_prefix: a.common.global_prefix.clone(), json: a.common.json, @@ -193,6 +195,7 @@ fn expected_defaults() -> Snap { patch_server_url: None, offline: false, strict: false, + allow_user_patches: false, global: false, global_prefix: None, json: false, diff --git a/crates/socket-patch-cli/tests/cli_parse_vex.rs b/crates/socket-patch-cli/tests/cli_parse_vex.rs index 23e5a15ab..7dc3c7451 100644 --- a/crates/socket-patch-cli/tests/cli_parse_vex.rs +++ b/crates/socket-patch-cli/tests/cli_parse_vex.rs @@ -205,6 +205,7 @@ struct Snap { patch_server_url: Option, offline: bool, strict: bool, + allow_user_patches: bool, global: bool, global_prefix: Option, json: bool, @@ -239,6 +240,7 @@ fn snapshot(a: &VexArgs) -> Snap { patch_server_url: a.common.patch_server_url.clone(), offline: a.common.offline, strict: a.common.strict, + allow_user_patches: a.common.allow_user_patches, global: a.common.global, global_prefix: a.common.global_prefix.clone(), json: a.common.json, @@ -280,6 +282,7 @@ fn expected_defaults() -> Snap { patch_server_url: None, offline: false, strict: false, + allow_user_patches: false, global: false, global_prefix: None, json: false, diff --git a/crates/socket-patch-cli/tests/get/get_failure_paths.rs b/crates/socket-patch-cli/tests/get/get_failure_paths.rs index 29b8e5b0d..5372dd655 100644 --- a/crates/socket-patch-cli/tests/get/get_failure_paths.rs +++ b/crates/socket-patch-cli/tests/get/get_failure_paths.rs @@ -1628,7 +1628,7 @@ async fn get_uuid_json_nested_apply_failure_names_the_patch() { } /// The agent engine (`scan --mode agent --json`'s path) over an installed -/// file a local edit changed: the default policy overwrites it, and the +/// file a local edit changed: the opted-in policy overwrites it, and the /// envelope must say so — the warning `apply --json` reports as a /// `content_mismatch_overwritten` event — while the patch still counts as /// applied. @@ -1644,6 +1644,7 @@ async fn engine_nested_apply_mismatch_overwrite_reaches_the_json_envelope() { std::fs::write(&installed, LOCAL_EDIT_BYTES).unwrap(); let mut params = engine_params(tmp.path()); params.save_only = false; + params.allow_user_patches = true; let selected = vec![search_result(UUID, PURL)]; let (code, json) = download_and_apply_patches(&selected, ¶ms, &server.uri()).await; @@ -1687,7 +1688,11 @@ async fn get_uuid_json_mismatch_overwrite_is_reported() { write_project(tmp.path()); let installed = tmp.path().join("node_modules").join(NAME).join("index.js"); std::fs::write(&installed, LOCAL_EDIT_BYTES).unwrap(); - let (code, stdout, stderr) = run_get_bin(tmp.path(), &server.uri(), &[UUID, "--json"]); + let (code, stdout, stderr) = run_get_bin( + tmp.path(), + &server.uri(), + &[UUID, "--json", "--allow-user-patches"], + ); assert_eq!(code, 0, "stdout={stdout}\nstderr={stderr}"); let json = parse_json_envelope(&stdout); assert_eq!(json["status"], "success", "json={json}"); @@ -1699,3 +1704,62 @@ async fn get_uuid_json_mismatch_overwrite_is_reported() { assert!(!stderr.contains("did not match"), "stderr={stderr}"); assert_eq!(std::fs::read(&installed).unwrap(), AFTER_BYTES); } + +#[tokio::test] +async fn get_uuid_requires_specific_consent_in_every_mode_before_persisting() { + let server = MockServer::start().await; + mount_real_view(&server, UUID, PURL).await; + for extra in [ + vec![UUID, "--json", "--mode", "agent"], + vec![UUID, "--json", "--mode", "hosted"], + vec![UUID, "--json", "--mode", "vendored"], + vec![UUID, "--json", "--save-only"], + vec![UUID, "--json", "--mode", "agent", "--dry-run"], + ] { + let tmp = tempfile::tempdir().unwrap(); + write_project(tmp.path()); + std::fs::create_dir(tmp.path().join("patches")).unwrap(); + std::fs::write( + tmp.path().join(format!("patches/{NAME}+1.0.0.patch")), + "user diff", + ) + .unwrap(); + let (code, stdout, stderr) = run_get_bin(tmp.path(), &server.uri(), &extra); + let json = parse_json_envelope(&stdout); + assert_eq!(code, 1, "{extra:?}: {stdout} {stderr}"); + assert_eq!(json["error"]["code"], "user_patch_conflict", "{json}"); + assert!(!tmp.path().join(".socket/manifest.json").exists()); + assert_eq!( + std::fs::read(tmp.path().join(format!("node_modules/{NAME}/index.js"))).unwrap(), + BEFORE_BYTES + ); + } +} + +#[tokio::test] +async fn get_uuid_forwards_explicit_consent_to_nested_apply() { + let server = MockServer::start().await; + mount_real_view(&server, UUID, PURL).await; + let tmp = tempfile::tempdir().unwrap(); + write_project(tmp.path()); + std::fs::create_dir(tmp.path().join("patches")).unwrap(); + let patch = tmp.path().join(format!("patches/{NAME}+1.0.0.patch")); + std::fs::write(&patch, "user diff").unwrap(); + let (code, stdout, stderr) = run_get_bin( + tmp.path(), + &server.uri(), + &[UUID, "--json", "--mode", "agent", "--allow-user-patches"], + ); + assert_eq!(code, 0, "{stdout} {stderr}"); + assert_eq!( + std::fs::read(tmp.path().join(format!("node_modules/{NAME}/index.js"))).unwrap(), + AFTER_BYTES + ); + assert_eq!(std::fs::read_to_string(patch).unwrap(), "user diff"); + let json = parse_json_envelope(&stdout); + assert!(json["warnings"] + .as_array() + .unwrap() + .iter() + .any(|w| w["code"] == "user_patch_override")); +} diff --git a/crates/socket-patch-cli/tests/get/get_fixture.rs b/crates/socket-patch-cli/tests/get/get_fixture.rs index b094249b4..43ec126bd 100644 --- a/crates/socket-patch-cli/tests/get/get_fixture.rs +++ b/crates/socket-patch-cli/tests/get/get_fixture.rs @@ -293,6 +293,7 @@ pub fn engine_params(root: &Path) -> DownloadParams { json: true, silent: true, strict: false, + allow_user_patches: false, ecosystems: None, persist_blobs: true, patch_server_url: None, diff --git a/crates/socket-patch-cli/tests/in_process_cargo_apply.rs b/crates/socket-patch-cli/tests/in_process_cargo_apply.rs index da00bb155..ff3a1defd 100644 --- a/crates/socket-patch-cli/tests/in_process_cargo_apply.rs +++ b/crates/socket-patch-cli/tests/in_process_cargo_apply.rs @@ -347,6 +347,7 @@ async fn cargo_apply_refuses_on_before_hash_mismatch() { // --force) deliberately downgrade a hash mismatch to "ready" // and the file WOULD be overwritten with verified content. strict: true, + allow_user_patches: false, ..socket_patch_cli::args::GlobalArgs::default() }, batch_size: Some(100), diff --git a/crates/socket-patch-cli/tests/in_process_get_update_count.rs b/crates/socket-patch-cli/tests/in_process_get_update_count.rs index 838720388..1e967e87a 100644 --- a/crates/socket-patch-cli/tests/in_process_get_update_count.rs +++ b/crates/socket-patch-cli/tests/in_process_get_update_count.rs @@ -91,6 +91,7 @@ fn params(root: &Path) -> DownloadParams { json: true, silent: true, strict: false, + allow_user_patches: false, ecosystems: None, persist_blobs: true, patch_server_url: None, diff --git a/crates/socket-patch-cli/tests/scan/hosted_failure_paths.rs b/crates/socket-patch-cli/tests/scan/hosted_failure_paths.rs index 12d5588b5..a4b9e779d 100644 --- a/crates/socket-patch-cli/tests/scan/hosted_failure_paths.rs +++ b/crates/socket-patch-cli/tests/scan/hosted_failure_paths.rs @@ -1208,7 +1208,7 @@ async fn unreadable_pnpm_workspace_gets_warning_only_guidance_in_a_live_run() { return; } - let (code, doc) = scan_hosted_json(tmp.path(), &server.uri(), &[], &[]); + let (code, doc) = scan_hosted_json(tmp.path(), &server.uri(), &["--allow-user-patches"], &[]); // Restore before asserting so the tempdir cleans up and the bytes can // be read back. std::fs::set_permissions(&ws, std::fs::Permissions::from_mode(0o644)).unwrap(); diff --git a/crates/socket-patch-cli/tests/scan/scan_failure_paths.rs b/crates/socket-patch-cli/tests/scan/scan_failure_paths.rs index f7e1c9a4a..758bf58dd 100644 --- a/crates/socket-patch-cli/tests/scan/scan_failure_paths.rs +++ b/crates/socket-patch-cli/tests/scan/scan_failure_paths.rs @@ -573,7 +573,11 @@ async fn scan_agent_json_mismatch_overwrite_reaches_the_apply_block() { b"before\n// local edit\n", ); - let (code, v) = run_json(tmp.path(), &mock.uri(), &["--mode", "agent"]); + let (code, v) = run_json( + tmp.path(), + &mock.uri(), + &["--mode", "agent", "--allow-user-patches"], + ); assert_eq!(code, 0, "{v:#}"); assert_eq!(v["summary"]["applied"], 1, "{v}"); let detail = hosted::warning_detail(&v, "content_mismatch_overwritten"); @@ -851,3 +855,79 @@ async fn scan_vendored_ignores_a_degraded_pre_v5_vlt_ledger_edit() { ); } } + +#[tokio::test] +async fn scan_skips_user_patches_in_every_mode_before_querying_or_writing() { + let mock = MockServer::start().await; + for mode in ["agent", "hosted", "vendored"] { + let tmp = minimist_project(b"before\n"); + let declaration = r#"{"dependencies":{"minimist":"1.2.8"},"pnpm":{"patchedDependencies":{"minimist":"missing.patch"}}}"#; + std::fs::write(tmp.path().join("package.json"), declaration).unwrap(); + let (code, json) = run_json(tmp.path(), &mock.uri(), &["--mode", mode, "--yes"]); + assert_eq!(code, 0, "{mode}: {json}"); + assert!( + json["warnings"] + .as_array() + .unwrap() + .iter() + .any(|w| w["code"] == "user_patch_conflict"), + "{json}" + ); + assert!(!tmp.path().join(".socket").exists()); + assert_eq!( + std::fs::read_to_string(tmp.path().join("package.json")).unwrap(), + declaration + ); + assert_eq!( + std::fs::read(tmp.path().join("node_modules/minimist/index.js")).unwrap(), + b"before\n" + ); + } + assert!(mock.received_requests().await.unwrap().is_empty()); +} + +#[tokio::test] +async fn scan_still_patches_unrelated_packages_and_forwards_explicit_consent() { + for allowed in [false, true] { + let mock = MockServer::start().await; + let purl = "pkg:npm/example@1.0.0"; + mount_one_patch_api(&mock, purl, b"before\n").await; + let tmp = tempfile::tempdir().unwrap(); + write_root_package_json(tmp.path()); + write_npm_package(tmp.path(), "example", "1.0.0", b"before\n"); + std::fs::create_dir(tmp.path().join("patches")).unwrap(); + if !allowed { + write_npm_package(tmp.path(), "unrelated", "1.0.0", b"before\n"); + } + let patched_name = if allowed { "example" } else { "unrelated" }; + std::fs::write( + tmp.path() + .join(format!("patches/{patched_name}+1.0.0.patch")), + "user diff", + ) + .unwrap(); + let mut args = vec!["--mode", "agent"]; + if allowed { + args.push("--allow-user-patches"); + } + let (code, json) = run_json(tmp.path(), &mock.uri(), &args); + assert_eq!(code, 0, "{json}"); + if !allowed { + assert_eq!( + std::fs::read(tmp.path().join("node_modules/unrelated/index.js")).unwrap(), + b"before\n" + ); + assert!(mock + .received_requests() + .await + .unwrap() + .iter() + .all(|request| !String::from_utf8_lossy(&request.body) + .contains("pkg:npm/unrelated@"))); + } + assert_eq!( + std::fs::read(tmp.path().join("node_modules/example/index.js")).unwrap(), + b"after\n" + ); + } +} diff --git a/crates/socket-patch-cli/tests/user_patches.rs b/crates/socket-patch-cli/tests/user_patches.rs new file mode 100644 index 000000000..c5a5b5fc9 --- /dev/null +++ b/crates/socket-patch-cli/tests/user_patches.rs @@ -0,0 +1,194 @@ +//! Consent is checked at package level, even when the Socket fix touches +//! a different file and beforeHash verification would succeed. +mod common; + +use common::{git_sha256, run_with_env, write_blob, write_minimal_manifest, PatchEntry}; +use std::path::Path; + +const PURL: &str = "pkg:npm/example@1.0.0"; +const UUID: &str = "11111111-1111-4111-8111-111111111111"; +const BEFORE: &[u8] = b"original\n"; +const AFTER: &[u8] = b"Socket fix\n"; + +fn write(root: &Path, path: &str, bytes: impl AsRef<[u8]>) { + let path = root.join(path); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, bytes).unwrap(); +} + +fn fixture() -> tempfile::TempDir { + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "package.json", "{}"); + write( + tmp.path(), + "node_modules/example/package.json", + r#"{"name":"example","version":"1.0.0"}"#, + ); + write(tmp.path(), "node_modules/example/index.js", BEFORE); + write(tmp.path(), "node_modules/example/user.js", b"user fix\n"); + let socket = tmp.path().join(".socket"); + write_minimal_manifest( + &socket, + PURL, + UUID, + &[PatchEntry { + file_name: "index.js", + before_hash: &git_sha256(BEFORE), + after_hash: &git_sha256(AFTER), + }], + ); + write_blob(&socket, &git_sha256(AFTER), AFTER); + tmp +} + +#[test] +fn apply_and_vendor_refuse_declared_patches_despite_matching_hashes_or_general_confirmation() { + for (file, contents) in [ + ( + "package.json", + r#"{"pnpm":{"patchedDependencies":{"example@1.0.0":"patches/fix.patch"}}}"#, + ), + ( + "pnpm-workspace.yaml", + "patchedDependencies:\n example@1.0.0: missing.patch\n", + ), + ( + "package.json", + r#"{"resolutions":{"example":"patch:example@npm%3A1.0.0#./fix.patch"}}"#, + ), + ("patches/example+1.0.0.patch", "diff for a different file"), + ( + "package.json", + r#"{"patchedDependencies":{"example@1.0.0":"patches/fix.patch"}}"#, + ), + ] { + for args in [ + vec!["apply", "--json", "--offline", "--yes"], + vec!["apply", "--json", "--offline", "--force"], + vec!["apply", "--json", "--offline", "--dry-run"], + vec!["vendor", "--json", "--offline", "--force"], + ] { + let tmp = fixture(); + write(tmp.path(), file, contents); + let original = std::fs::read(tmp.path().join(".socket/manifest.json")).unwrap(); + let (code, stdout, stderr) = run_with_env(tmp.path(), &args, &[]); + let value: serde_json::Value = serde_json::from_str(&stdout).expect(&stdout); + assert_eq!(code, 1, "{args:?} {file}: {stdout} {stderr}"); + assert_eq!(value["error"]["code"], "user_patch_conflict", "{value}"); + assert!(value["error"]["message"] + .as_str() + .unwrap() + .contains("--allow-user-patches")); + assert_eq!( + std::fs::read(tmp.path().join("node_modules/example/index.js")).unwrap(), + BEFORE + ); + assert_eq!( + std::fs::read(tmp.path().join(file)).unwrap(), + contents.as_bytes() + ); + assert_eq!( + std::fs::read(tmp.path().join(".socket/manifest.json")).unwrap(), + original + ); + assert!(!tmp.path().join(".socket/vendor/state.json").exists()); + assert!(!tmp.path().join(".socket/apply.lock").exists()); + } + } +} + +#[test] +fn consent_allows_agent_application_and_does_not_modify_the_user_patch() { + let tmp = fixture(); + write(tmp.path(), "patches/example+1.0.0.patch", b"user diff\n"); + let (code, stdout, stderr) = run_with_env( + tmp.path(), + &["apply", "--json", "--offline", "--allow-user-patches"], + &[], + ); + assert_eq!(code, 0, "{stdout} {stderr}"); + let value: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + assert!(value["warnings"] + .as_array() + .unwrap() + .iter() + .any(|w| w["code"] == "user_patch_override")); + assert_eq!( + std::fs::read(tmp.path().join("node_modules/example/index.js")).unwrap(), + AFTER + ); + assert_eq!( + std::fs::read(tmp.path().join("node_modules/example/user.js")).unwrap(), + b"user fix\n" + ); + assert_eq!( + std::fs::read(tmp.path().join("patches/example+1.0.0.patch")).unwrap(), + b"user diff\n" + ); +} + +#[test] +fn unknown_local_edits_fail_closed_by_default_including_yes_silent_and_dry_run() { + for extra in [ + vec![], + vec!["--yes"], + vec!["--silent"], + vec!["--dry-run"], + vec!["--allow-user-patches", "--strict"], + ] { + let tmp = fixture(); + write(tmp.path(), "node_modules/example/index.js", b"local edit\n"); + let mut args = vec!["apply", "--json", "--offline"]; + args.extend(extra); + let (code, stdout, stderr) = run_with_env(tmp.path(), &args, &[]); + assert_eq!(code, 1, "{args:?}: {stdout} {stderr}"); + assert_eq!( + std::fs::read(tmp.path().join("node_modules/example/index.js")).unwrap(), + b"local edit\n" + ); + } +} + +#[test] +fn unrelated_versions_and_ecosystems_are_not_blocked() { + let tmp = fixture(); + write( + tmp.path(), + "patches/example+2.0.0.patch", + b"different release\n", + ); + write( + tmp.path(), + "composer.json", + r#"{"extra":{"patches":{"a/b":{"fix":"fix.patch"}}}}"#, + ); + let (code, stdout, stderr) = run_with_env( + tmp.path(), + &["apply", "--json", "--offline", "--ecosystems", "npm"], + &[], + ); + assert_eq!(code, 0, "{stdout} {stderr}"); + assert_eq!( + std::fs::read(tmp.path().join("node_modules/example/index.js")).unwrap(), + AFTER + ); +} + +#[test] +fn environment_and_read_only_checks_do_not_grant_or_require_consent() { + let tmp = fixture(); + write(tmp.path(), "patches/example+1.0.0.patch", b"user diff\n"); + let (code, stdout, _) = run_with_env( + tmp.path(), + &["apply", "--json", "--offline"], + &[("SOCKET_ALLOW_USER_PATCHES", "1")], + ); + assert_eq!(code, 1, "{stdout}"); + assert!(stdout.contains("user_patch_conflict")); + let (_, stdout, _) = run_with_env( + tmp.path(), + &["apply", "--json", "--offline", "--check"], + &[], + ); + assert!(!stdout.contains("user_patch_conflict"), "{stdout}"); +} diff --git a/crates/socket-patch-cli/tests/vendor/in_process_vendor_bun.rs b/crates/socket-patch-cli/tests/vendor/in_process_vendor_bun.rs index 81f14e2d0..32d9b2934 100644 --- a/crates/socket-patch-cli/tests/vendor/in_process_vendor_bun.rs +++ b/crates/socket-patch-cli/tests/vendor/in_process_vendor_bun.rs @@ -493,7 +493,8 @@ fn assert_refusal_left_tree_alone(root: &Path, lock_before: &[u8]) { /// Drive `scan --mode vendored --json` on `shape` and pin the refusal /// contract for `code`: exit 1, `partialFailure`, the download-phase -/// record, zero downloads, zero view fetches, engine untouched. +/// record, zero downloads, zero view fetches, engine untouched. Explicit +/// user-patch consent still cannot bypass a malformed/unsupported lock. async fn assert_scan_refuses(shape: LockShape, code: &str) { let mock = MockServer::start().await; mount_patch_api(&mock).await; @@ -505,7 +506,8 @@ async fn assert_scan_refuses(shape: LockShape, code: &str) { Vec::new() }; - let (exit, stdout, stderr) = scan_vendored(tmp.path(), &mock.uri(), &["--json"]); + let (exit, stdout, stderr) = + scan_vendored(tmp.path(), &mock.uri(), &["--json", "--allow-user-patches"]); assert_eq!(exit, 1, "{shape:?}: stdout={stdout}\nstderr={stderr}"); let v = parse_single_json_doc(&stdout); assert_eq!(v["status"], "partialFailure", "{shape:?}: {v}"); @@ -1682,6 +1684,7 @@ async fn fifo_bun_lock_is_refused_without_blocking() { "--vendor-source", "service", "--json", + "--allow-user-patches", ], &uri, ); @@ -1705,6 +1708,7 @@ async fn fifo_bun_lock_is_refused_without_blocking() { "--vendor-source", "service", "--json", + "--allow-user-patches", ], &uri, ); diff --git a/crates/socket-patch-cli/tests/vendor_eject.rs b/crates/socket-patch-cli/tests/vendor_eject.rs index 8218d4ef4..2ee7df82c 100644 --- a/crates/socket-patch-cli/tests/vendor_eject.rs +++ b/crates/socket-patch-cli/tests/vendor_eject.rs @@ -271,6 +271,47 @@ async fn eject_vendors_hosted_pins_and_revert_returns_to_upstream() { assert!(!p.redirect_state().exists()); } +/// The hosted-to-vendored transition also needs explicit patch consent. +#[tokio::test] +async fn eject_requires_user_patch_consent_and_reports_it_in_previews_and_writes() { + let p = hosted_project(true).await; + let patch = p.root().join("patches/left-pad+1.3.0.patch"); + std::fs::create_dir_all(patch.parent().unwrap()).unwrap(); + std::fs::write(&patch, b"user diff\n").unwrap(); + let before = p.lock_bytes(); + + let (code, env) = p.run_json(&["vendor", "--yes"]); + assert_eq!(code, 1, "{env:#}"); + assert_eq!(env["error"]["code"], "user_patch_conflict"); + assert_eq!(p.lock_bytes(), before); + assert!(p.server.received_requests().await.unwrap().is_empty()); + + mock_registry(&p).await; + mock_view(&p).await; + for args in [ + vec!["vendor", "--allow-user-patches", "--dry-run"], + vec!["vendor", "--allow-user-patches"], + ] { + let (code, env) = p.run_json(&args); + assert_eq!(code, 0, "{args:?}: {env:#}"); + assert!( + env["warnings"] + .as_array() + .unwrap() + .iter() + .any(|w| w["code"] == "user_patch_override"), + "{env:#}" + ); + assert_eq!(std::fs::read(&patch).unwrap(), b"user diff\n"); + if args.contains(&"--dry-run") { + assert_eq!(p.lock_bytes(), before); + assert!(!p.artifact().exists()); + } else { + assert!(p.artifact().exists()); + } + } +} + /// Human eject: the first line announces the eject. #[tokio::test] async fn eject_human_output_announces_the_eject() { diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs index ac8c1eed7..a54ab1cd9 100644 --- a/crates/socket-patch-core/src/patch/apply.rs +++ b/crates/socket-patch-core/src/patch/apply.rs @@ -45,13 +45,13 @@ pub struct VerifyResult { /// `Strict` exists. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum MismatchPolicy { - /// DEFAULT: a beforeHash mismatch is overwritten with the verified + /// Explicitly permitted: a beforeHash mismatch is overwritten with the verified /// patched content and surfaced as a warning (the promoted /// [`VerifyResult`] keeps `expected_hash`/`current_hash`, which is /// how callers detect and report it). A MISSING pre-existing file is /// still a hard error. Warn, - /// A beforeHash mismatch is a hard error (`--strict`). + /// A beforeHash mismatch is a hard error (the CLI default, or `--strict`). Strict, /// [`MismatchPolicy::Warn`] PLUS missing pre-existing files are /// skipped instead of failing (`--force`). @@ -840,7 +840,7 @@ pub const NEW_FILE_COLLISION_MESSAGE: &str = /// (CLI_CONTRACT "mismatch policy"). [`verify_file_patch`] reports it as /// `Ready` (its callers only ask "can this be written?"); the apply engine /// turns it into a `HashMismatch` with an empty `expected_hash`, so -/// `--strict` refuses it and the default policy overwrites it with the +/// `Strict` refuses it and explicit mismatch tolerance overwrites it with the /// `content_mismatch_overwritten` warning instead of silently. fn mark_new_file_collision(verify: &mut VerifyResult, file_info: &PatchFileInfo) { if file_info.before_hash.is_empty() @@ -909,7 +909,7 @@ async fn apply_package_patch_at( && verify_result.status != VerifyStatus::AlreadyPatched { match (verify_result.status, policy) { - // Mismatch tolerated (default + force): promote to Ready. + // Mismatch explicitly tolerated (warn + force): promote to Ready. // The promoted result KEEPS `expected_hash`/`current_hash` // — the signature callers use to surface the warning. The // blob source is hash-gated to afterHash. @@ -3319,7 +3319,7 @@ mod tests { /// #626: agent apply must not write through a `node_modules` link to /// first-party source (a workspace member / `file:` dir, scoped or not, /// or an `npm link` target outside the project). The user's fork does - /// not match the patch's beforeHash, which the default policy would + /// not match the patch's beforeHash, which mismatch tolerance would /// otherwise overwrite. Refused for every policy, dry run included, and /// the fork keeps its bytes. #[cfg(unix)] diff --git a/crates/socket-patch-core/src/patch/mod.rs b/crates/socket-patch-core/src/patch/mod.rs index 48ca5ea9f..3f1f45b23 100644 --- a/crates/socket-patch-core/src/patch/mod.rs +++ b/crates/socket-patch-core/src/patch/mod.rs @@ -11,3 +11,4 @@ pub mod rollback; pub mod shared_store; pub mod sidecars; pub(crate) mod store_copies; +pub mod user_patches; diff --git a/crates/socket-patch-core/src/patch/user_patches.rs b/crates/socket-patch-core/src/patch/user_patches.rs new file mode 100644 index 000000000..62ba25d09 --- /dev/null +++ b/crates/socket-patch-core/src/patch/user_patches.rs @@ -0,0 +1,898 @@ +//! Read-only evidence of patches owned by the project, not Socket. +//! +//! A matching declaration is sufficient: the patch need not have been installed +//! yet, and need not touch the same file as the Socket fix. Never execute hooks +//! or fetch patch URLs to establish ownership. + +use std::collections::{BTreeSet, VecDeque}; +use std::path::Path; + +use serde_json::Value; + +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, percent_decode_purl_component, strip_purl_qualifiers}; +use crate::utils::workspace_globs::{workspaces_include_as, WorkspacesDialect}; + +#[derive(Default, Debug)] +pub struct UserPatchIndex { + entries: Vec, +} + +#[derive(Debug)] +struct Evidence { + ecosystem: &'static str, + name: String, + version: Option, + source: String, +} + +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. + 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. + for parent in root + .ancestors() + .skip(1) + .take_while(|_| !root.join(".git").exists()) + { + 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()); + } + if parent.join(".git").exists() { + break; + } + } + let mut seen = BTreeSet::new(); + while let Some(dir) = pending.pop_front() { + if !seen.insert(dir.clone()) { + continue; + } + if seen.len() > 4096 { + index.unknown("npm", &dir, "workspace inspection limit exceeded"); + break; + } + 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")); + } + } + } + let workspaces = value["workspaces"] + .as_array() + .or_else(|| value["workspaces"]["packages"].as_array()); + if let Some(workspaces) = workspaces { + globs.extend( + workspaces + .iter() + .filter_map(Value::as_str) + .map(str::to_owned), + ); + } + index.patch_package(&dir, &value).await; + } else { + index.patch_directory(&dir.join("patches"), false).await; + } + for file in ["pnpm-workspace.yaml", "pnpm-lock.yaml", "yarn.lock"] { + let path = dir.join(file); + if let Some(text) = index.read(&path, "npm").await { + // Yarn classic isn't YAML; only Berry has patch: locators. + if file == "yarn.lock" && !text.contains("patch:") { + continue; + } + if file == "pnpm-lock.yaml" + && !text.contains("patchedDependencies") + && !text.contains("patch_hash") + && !text.contains("patched:") + { + continue; + } + match serde_saphyr::from_multiple::(&text) { + 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), + ); + } + } + if file == "yarn.lock" { + if let Some(map) = value.as_object() { + for entry in map.values() { + if let Some(spec) = entry["resolution"].as_str() { + // A locator has an ident before @patch:. + if let Some((_, source)) = + spec.split_once("@patch:") + { + index.yarn(&format!("patch:{source}"), &path); + } + } + } + } + } + } + } + Err(e) => index.unknown( + "npm", + &path, + &format!("cannot inspect patch metadata: {e}"), + ), + } + } + } + for file in ["package-lock.json", "npm-shrinkwrap.json", "bun.lock"] { + let path = dir.join(file); + if let Some(value) = index.json(&path, "npm").await { + 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); + } + } + } + } + for pattern in globs + .iter() + .filter(|g| !g.starts_with('!') || g.starts_with("!(")) + { + pending.extend(index.workspace_members(&dir, pattern)); + } + } + index + } + + // 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 { + if Path::new(pattern).is_absolute() || pattern.split('/').any(|p| p == "..") { + self.unknown("npm", root, "workspace outside the inspection root"); + return Vec::new(); + } + let prefix: std::path::PathBuf = pattern + .split('/') + .take_while(|p| !p.contains(['*', '?', '{', '[', '('])) + .collect(); + let base = root.join(&prefix); + if !base.exists() && !base.is_symlink() { + return Vec::new(); + } + let max_depth = if pattern.contains("**") { + 32 + } else { + pattern + .split('/') + .count() + .saturating_sub(prefix.components().count()) + }; + let walk = walkdir::WalkDir::new(&base) + .follow_links(false) + .max_depth(max_depth) + .into_iter() + .filter_entry(|entry| { + !matches!( + entry.file_name().to_str(), + Some("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()) + .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"); + } + if entry.file_type().is_symlink() && (entry.depth() == 0 || entry.path().is_dir()) { + self.unknown( + "npm", + entry.path(), + "linked workspace cannot be inspected safely", + ); + continue; + } + if workspaces_include_as(&patterns, &components, WorkspacesDialect::YARN_BERRY) { + if entry.file_type().is_symlink() { + self.unknown( + "npm", + entry.path(), + "linked workspace cannot be inspected safely", + ); + } else if entry.file_type().is_dir() { + out.push(entry.path().to_path_buf()); + } + } + } + out + } + + /// All evidence for this release, sorted and deduplicated. Unknown ranges + /// protect every version of that name; known exact versions don't block an + /// unrelated release. Scoped npm identities remain case-sensitive. + pub fn conflicts(&self, purl: &str) -> Vec { + let normalized = normalize_purl(purl); + let base = strip_purl_qualifiers(&normalized); + let Some((ecosystem, coordinate)) = + base.strip_prefix("pkg:").and_then(|s| s.split_once('/')) + else { + return Vec::new(); + }; + let Some((name, version)) = coordinate.rsplit_once('@') else { + return Vec::new(); + }; + self.entries + .iter() + .filter(|e| { + e.ecosystem == ecosystem + && (e.name.is_empty() + || if ecosystem == "composer" { + e.name.eq_ignore_ascii_case(name) + } else { + e.name == name + }) + && e.version.as_deref().is_none_or(|selector| { + selector == version + || if ecosystem == "npm" { + // pnpm treats an explicit * as its all-versions + // bucket, including prereleases, rather than as + // an ordinary semver range. + selector.trim() == "*" + || npm_range_admits(selector, version).unwrap_or(true) + } else { + false + } + }) + }) + .map(|e| e.source.clone()) + .collect::>() + .into_iter() + .collect() + } + + fn add(&mut self, ecosystem: &'static str, name: &str, version: Option<&str>, path: &Path) { + self.entries.push(Evidence { + ecosystem, + name: name.into(), + version: version.map(str::to_owned), + source: path.display().to_string(), + }); + } + + fn unknown(&mut self, ecosystem: &'static str, path: &Path, reason: &str) { + self.entries.push(Evidence { + ecosystem, + name: String::new(), + version: None, + source: format!("{} ({reason})", path.display()), + }); + } + + async fn read(&mut self, path: &Path, ecosystem: &'static str) -> Option { + match read_regular_to_string(path).await { + Ok(text) => Some(strip_bom(&text).to_owned()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) => { + self.unknown( + ecosystem, + path, + &format!("cannot inspect patch metadata: {e}"), + ); + None + } + } + } + + async fn json(&mut self, path: &Path, ecosystem: &'static str) -> Option { + let text = self.read(path, ecosystem).await?; + match serde_json::from_str(&crate::vendor::bun_lock_text::strip_jsonc(&text)) { + Ok(value) => Some(value), + Err(e) => { + self.unknown( + ecosystem, + path, + &format!("cannot inspect patch metadata: {e}"), + ); + None + } + } + } + + 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"); + } + } + // Older pnpm locks can carry patch hashes without a mirrored map. + for field in ["packages", "snapshots"] { + if let Some(map) = value[field].as_object() { + for (key, entry) in map { + if key.contains("patch_hash=") || !entry["patched"].is_null() { + if let Some((name, version)) = crate::formats::pnpm::pnpm_registry_key(key) + { + self.add("npm", name, Some(version), path); + } + } + } + } + } + } + + fn yarn(&mut self, spec: &str, path: &Path) { + if !spec.starts_with("patch:") { + return; + } + let mut source = spec.to_owned(); + let mut user_patch = false; + // Inspect every layer: a compatibility patch can wrap a user patch, + // or vice versa. Both still belong to the original package. + for _ in 0..16 { + if let Some(inner) = source + .strip_prefix("patch:") + .or_else(|| source.split_once("@patch:").map(|(_, rest)| rest)) + { + let Some((original, patches)) = inner.split_once('#') else { + self.unknown("npm", path, "unrecognized patch: reference"); + return; + }; + // Only Yarn's compatibility namespace is exempt. Plugins can + // supply their own builtin patches through getBuiltinPatch. + let patches = percent_decode_purl_component(patches); + user_patch |= !patches + .split("::") + .next() + .unwrap_or(&patches) + .split('&') + .all(|p| { + let p = p.rsplit('!').next().unwrap_or(p).trim_start_matches('~'); + p.starts_with("builtin') + }); + source = percent_decode_purl_component(original).into_owned(); + continue; + } + if user_patch { + let (name, version) = split_selector(&source); + self.add( + "npm", + name, + version.map(|v| v.strip_prefix("npm:").unwrap_or(v)), + path, + ); + } + return; + } + self.unknown( + "npm", + path, + "patch locator nesting exceeds inspection limit", + ); + } + + async fn patch_package(&mut self, dir: &Path, value: &Value) { + self.patch_directory(&dir.join("patches"), false).await; + let flag = regex::Regex::new(r#"--patch-dir(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))"#) + .unwrap(); + if let Some(scripts) = value["scripts"].as_object() { + for script in scripts + .values() + .filter_map(Value::as_str) + .filter(|s| s.contains("patch-package")) + { + if script.contains("cd ") + || (script.contains("--patch-dir") && !flag.is_match(script)) + { + self.unknown( + "npm", + &dir.join("package.json"), + "cannot resolve patch-package directory without executing the script", + ); + } + for captures in flag.captures_iter(script) { + let path = captures.iter().skip(1).flatten().next().unwrap().as_str(); + if path.contains(['$', '`']) || script.contains("cd ") { + self.unknown( + "npm", + &dir.join("package.json"), + "dynamic patch-package directory; inspect it before opting in", + ); + } else { + self.patch_directory(&dir.join(path), true).await; + } + } + } + } + } + + 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 { + 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"); + } + } + } + + async fn composer(&mut self, dir: &Path) { + let manifest = dir.join("composer.json"); + let Some(value) = self.json(&manifest, "composer").await else { + return; + }; + self.composer_map(&value["extra"]["patches"], &manifest); + let file = value["extra"]["composer-patches"]["patches-file"] + .as_str() + .or_else(|| value["extra"]["patches-file"].as_str()) + .unwrap_or("patches.json"); + if file != "patches.json" && !dir.join(file).exists() { + self.unknown( + "composer", + &dir.join(file), + "configured patch definitions are missing", + ); + } + for path in [dir.join(file), dir.join("patches.lock.json")] { + if let Some(patches) = self.json(&path, "composer").await { + self.composer_map(&patches["patches"], &path); + } + } + let lock = dir.join("composer.lock"); + if let Some(value) = self.json(&lock, "composer").await { + for field in ["packages", "packages-dev"] { + if let Some(packages) = value[field].as_array() { + for package in packages { + self.composer_map(&package["extra"]["patches"], &lock); + } + } + } + } + } + + fn composer_map(&mut self, value: &Value, path: &Path) { + if let Some(map) = value.as_object() { + for name in map.keys() { + self.add("composer", name, None, path); + } + } else if let Some(patches) = value.as_array() { + for patch in patches { + if let Some(name) = patch["package"].as_str() { + self.add("composer", name, None, path); + } else { + self.unknown("composer", path, "unidentified Composer patch target"); + } + } + } else if !value.is_null() { + self.unknown("composer", path, "unrecognized Composer patch definitions"); + } + } + + async fn cargo(&mut self, dir: &Path) { + for file in ["Cargo.toml", ".cargo/config.toml", ".cargo/config"] { + let path = dir.join(file); + let Some(text) = self.read(&path, "cargo").await else { + continue; + }; + if !text.contains("patch") && !text.contains("replace") && !text.contains("paths") { + continue; + } + let doc = match text.parse::() { + Ok(doc) => doc, + Err(e) => { + self.unknown("cargo", &path, &e.to_string()); + continue; + } + }; + if let Some(tables) = doc.get("patch").and_then(toml_edit::Item::as_table_like) { + for (_, table) in tables.iter() { + if let Some(table) = table.as_table_like() { + for (key, entry) in table.iter() { + // Socket's managed paths/registries must remain eligible for updates. + let target = entry.get("path").and_then(toml_edit::Item::as_str); + if target.is_some_and(|p| { + p.trim_start_matches("./").starts_with(".socket/vendor/") + }) { + continue; + } + let name = entry + .get("package") + .and_then(toml_edit::Item::as_str) + .unwrap_or(key); + self.add("cargo", name, None, &path); + } + } + } + } + if let Some(table) = doc.get("replace").and_then(toml_edit::Item::as_table_like) { + for (key, _) in table.iter() { + let (name, version) = key + .split_once(':') + .map_or((key, None), |(n, v)| (n, Some(v))); + self.add("cargo", name, version, &path); + } + } + if doc + .get("paths") + .and_then(toml_edit::Item::as_array) + .is_some_and(|a| !a.is_empty()) + { + self.unknown( + "cargo", + &path, + "Cargo paths overrides may replace registry packages", + ); + } + } + } +} + +fn split_selector(key: &str) -> (&str, Option<&str>) { + match key.rsplit_once('@') { + Some((name, version)) if !name.is_empty() => (name, Some(version)), + _ => (key, None), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn write(root: &Path, path: &str, text: &str) { + let path = root.join(path); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, text).unwrap(); + } + + #[tokio::test] + async fn native_declarations_match_scopes_ranges_and_exact_versions_without_patch_files() { + for (file, text) in [ + ("package.json", r#"{"pnpm":{"patchedDependencies":{"@s/pkg@^1.0.0":"missing.patch","all":"all.patch","exact@2.0.0":"exact.patch"}}}"#), + ("pnpm-workspace.yaml", "patchedDependencies:\n '@s/pkg@^1.0.0': missing.patch\n all: all.patch\n exact@2.0.0: exact.patch\n"), + ("pnpm-lock.yaml", "lockfileVersion: '9.0'\npatchedDependencies:\n '@s/pkg@^1.0.0': {hash: abc, path: missing.patch}\n all: all.patch\n exact@2.0.0: exact.patch\n"), + ("bun.lock", r#"{ // root mirror + "patchedDependencies": {"@s/pkg@^1.0.0":"missing.patch","all":"all.patch","exact@2.0.0":"exact.patch"}, + }"#), + ] { + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), file, text); + let index = UserPatchIndex::discover(tmp.path()).await; + for purl in ["pkg:npm/%40s/pkg@1.2.0", "pkg:npm/all@4.0.0", "pkg:npm/exact@2.0.0"] { + assert!(!index.conflicts(purl).is_empty(), "{file}: {purl}"); + } + for purl in ["pkg:npm/@s/pkg@2.0.0", "pkg:npm/exact@2.0.1", "pkg:npm/other@1.0.0", "pkg:pypi/all@4.0.0"] { + assert!(index.conflicts(purl).is_empty(), "{file}: {purl}: {index:?}"); + } + } + } + + #[tokio::test] + async fn pnpm_lock_hashes_cover_legacy_and_multidocument_locks() { + for text in [ + "lockfileVersion: '6.0'\npackages:\n /@s/pkg@1.2.0(patch_hash=abc): {}\n", + "lockfileVersion: 5.4\npackages:\n /@s/pkg/1.2.0_patch_hash=abc:\n patched: true\n", + "lockfileVersion: '9.0'\n---\nlockfileVersion: '9.0'\nsnapshots:\n '@s/pkg@1.2.0(patch_hash=abc)': {}\n", + ] { + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "pnpm-lock.yaml", text); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!(!index.conflicts("pkg:npm/%40s/pkg@1.2.0").is_empty(), "{index:?}"); + assert!(index.conflicts("pkg:npm/@s/pkg@2.0.0").is_empty()); + } + } + + #[tokio::test] + async fn pnpm_all_versions_selector_protects_prereleases() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "pnpm-workspace.yaml", + "patchedDependencies:\n 'all@*': missing.patch\n 'range@^1.0.0': missing.patch\n", + ); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!(!index.conflicts("pkg:npm/all@2.0.0-beta.1").is_empty()); + assert!(index.conflicts("pkg:npm/range@2.0.0-beta.1").is_empty()); + } + + #[tokio::test] + async fn npm_lock_marker_is_sufficient_without_manifest_declaration() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "package-lock.json", + r#"{"packages":{"node_modules/parent/node_modules/@s/pkg":{"version":"1.2.0","patched":{"path":"patches/fix.patch"}}}}"#, + ); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!(!index.conflicts("pkg:npm/@s/pkg@1.2.0").is_empty()); + assert!(index.conflicts("pkg:npm/parent@1.2.0").is_empty()); + } + + #[tokio::test] + async fn yarn_protects_custom_patches_but_not_builtin_only_compatibility_patches() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "package.json", + r#"{"resolutions":{"alias":"patch:@s/pkg@npm%3A1.2.0#./fix.patch","typescript":"patch:typescript@npm%3A5.0.0#optional!builtin","plugin":"patch:plugin@npm%3A1.0.0#builtin","nested":"patch:nested@patch%3Anested@npm%253A1.0.0%23optional!builtin%3Ccompat/nested%3E#./outer.patch","wrapped":"patch:wrapped@patch%3Awrapped@npm%253A1.0.0%23./inner.patch#optional!builtin"}}"#, + ); + write(tmp.path(), "yarn.lock", "__metadata:\n version: 8\nentry:\n resolution: \"transitive@patch:transitive@npm%3A2.0.0#./fix.patch::version=2.0.0&hash=abc\"\n"); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!(!index.conflicts("pkg:npm/@s/pkg@1.2.0").is_empty()); + assert!(!index.conflicts("pkg:npm/transitive@2.0.0").is_empty()); + assert!(index.conflicts("pkg:npm/typescript@5.0.0").is_empty()); + assert!(!index.conflicts("pkg:npm/plugin@1.0.0").is_empty()); + assert!(!index.conflicts("pkg:npm/nested@1.0.0").is_empty()); + assert!(!index.conflicts("pkg:npm/wrapped@1.0.0").is_empty()); + assert!(index.conflicts("pkg:npm/alias@1.2.0").is_empty()); + assert!(index.conflicts("pkg:npm/@s/pkg@1.3.0").is_empty()); + } + + #[tokio::test] + async fn patch_package_supports_scoped_nested_sequenced_dev_and_custom_directories() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "package.json", + r#"{"scripts":{"postinstall":"patch-package --patch-dir='custom fixes'"}}"#, + ); + write( + tmp.path(), + "custom fixes/parent++@s+pkg+1.2.0+001+fix.dev.patch", + "user diff", + ); + write(tmp.path(), "patches/plain+3.0.0.dev.patch", "user diff"); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!(!index.conflicts("pkg:npm/@s/pkg@1.2.0").is_empty()); + assert!(!index.conflicts("pkg:npm/plain@3.0.0").is_empty()); + assert!(index.conflicts("pkg:npm/@s/pkg@1.3.0").is_empty()); + assert!(index.conflicts("pkg:npm/parent@1.2.0").is_empty()); + } + + #[tokio::test] + async fn workspace_members_and_parent_settings_are_both_consulted() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "pnpm-workspace.yaml", + "packages: ['packages/*']\npatchedDependencies:\n root@1.0.0: missing.patch\n", + ); + write(tmp.path(), "packages/a/package.json", "{}"); + write( + tmp.path(), + "packages/a/patches/child+2.0.0.patch", + "user diff", + ); + for root in [tmp.path().to_path_buf(), tmp.path().join("packages/a")] { + let index = UserPatchIndex::discover(&root).await; + assert!(!index.conflicts("pkg:npm/root@1.0.0").is_empty()); + assert!(!index.conflicts("pkg:npm/child@2.0.0").is_empty()); + assert!(index.conflicts("pkg:npm/other@1.0.0").is_empty()); + } + } + + #[tokio::test] + async fn workspace_globs_do_not_block_unrelated_packages() { + for pattern in [ + "packages/{a,b}", + "packages/[ab]", + "packages/@(a|b)", + "packages/**", + ] { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "package.json", + &serde_json::json!({"workspaces":[pattern]}).to_string(), + ); + write(tmp.path(), "packages/a/package.json", "{}"); + write( + tmp.path(), + "packages/a/patches/child+2.0.0.patch", + "user diff", + ); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!( + !index.conflicts("pkg:npm/child@2.0.0").is_empty(), + "{pattern}: {index:?}" + ); + assert!( + index.conflicts("pkg:npm/other@1.0.0").is_empty(), + "{pattern}: {index:?}" + ); + } + } + + #[cfg(unix)] + #[tokio::test] + async fn linked_workspace_is_not_silently_ignored() { + let tmp = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "package.json", + r#"{"workspaces":["packages/*"]}"#, + ); + std::os::unix::fs::symlink(outside.path(), tmp.path().join("packages")).unwrap(); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!(!index.conflicts("pkg:npm/child@1.0.0").is_empty()); + } + + #[tokio::test] + async fn composer_covers_external_locked_and_dependency_provided_patches() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "composer.json", + r#"{"extra":{"composer-patches":{"patches-file":"custom.json"},"patches":{"a/root":{"fix":"x.patch"}}}}"#, + ); + write( + tmp.path(), + "custom.json", + r#"{"patches":{"a/external":[{"url":"x.patch"}]}}"#, + ); + write( + tmp.path(), + "patches.lock.json", + r#"{"patches":[{"package":"a/locked","url":"x.patch"}]}"#, + ); + write( + tmp.path(), + "composer.lock", + r#"{"packages":[{"extra":{"patches":{"a/transitive":{"fix":"x.patch"}}}}]}"#, + ); + let index = UserPatchIndex::discover(tmp.path()).await; + for name in ["root", "external", "locked", "transitive"] { + assert!(!index + .conflicts(&format!("pkg:composer/a/{name}@1.0.0")) + .is_empty()); + } + assert!(index.conflicts("pkg:composer/a/other@1.0.0").is_empty()); + } + + #[tokio::test] + async fn cargo_and_go_preserve_user_overrides_and_recognize_socket_ownership() { + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "Cargo.toml", "[patch.crates-io]\nrenamed = { package = 'real', path = '../fork' }\nowned = { path = './.socket/vendor/cargo/uuid/owned' }\n[replace]\n'old:1.0.0' = { path = '../old' }\n"); + write(tmp.path(), "go.work", "go 1.23\nreplace (\n example.com/user v1.0.0 => ../fork\n example.com/owned v1.0.0 => ./.socket/go-patches/owned\n)\n"); + let index = UserPatchIndex::discover(tmp.path()).await; + for purl in [ + "pkg:cargo/real@1.0.0", + "pkg:cargo/old@1.0.0", + "pkg:golang/example.com/user@v1.0.0", + ] { + assert!(!index.conflicts(purl).is_empty(), "{purl}: {index:?}"); + } + for purl in [ + "pkg:cargo/renamed@1.0.0", + "pkg:cargo/owned@1.0.0", + "pkg:cargo/old@2.0.0", + "pkg:golang/example.com/owned@v1.0.0", + ] { + assert!(index.conflicts(purl).is_empty(), "{purl}: {index:?}"); + } + } + + #[tokio::test] + async fn unknown_metadata_does_not_silently_allow_patching() { + for text in [ + r#"{"patchedDependencies": "invalid"}"#, + r#"{"scripts":{"postinstall":"patch-package --patch-dir $PATCH_DIR"}}"#, + r#"{"scripts":{"postinstall":"patch-package --patch-dir missing"}}"#, + "{broken json", + ] { + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "package.json", text); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!( + !index.conflicts("pkg:npm/anything@1.0.0").is_empty(), + "{text}" + ); + assert!(index.conflicts("pkg:pypi/anything@1.0.0").is_empty()); + } + } +} diff --git a/docs/ecosystems.md b/docs/ecosystems.md index deda930d5..af9136194 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -1459,3 +1459,40 @@ Prebuilt binaries are published for: | Linux | x86_64, ARM64, 32-bit ARM hard-float (`arm-unknown-linux-gnueabihf` / `-musleabihf`), i686 | | Windows | x86_64, ARM64, i686 | | Android | ARM64 | + +## Existing project patches + +Socket Patch treats declared project patches as a separate source of ownership. +`scan` skips their packages and reports `user_patch_conflict`; explicit `get`, +`apply` and `vendor` refuse the conflict before patching. Use +`--allow-user-patches` only after reviewing how the fixes interact. This flag +permits an attempt; it does not merge diffs or bypass unsupported backend cases. +Installed file content that matches neither the original nor Socket-patched hash +also fails by default. See the [CLI contract](../crates/socket-patch-cli/CLI_CONTRACT.md#existing-project-patches) +for exact matching rules, diagnostics and detection limits. + +The protection applies before any comparison of changed filenames. Two fixes can +interact even when their diffs do not overlap, and a package manager may reapply +its own diff after Socket changes the dependency source. + +| Mechanism | Research and interaction | +| --- | --- | +| pnpm | [`pnpm patch`](https://pnpm.io/cli/patch) records `patchedDependencies`; selectors can be exact versions, ranges, or bare names. Configuration moved into [`pnpm-workspace.yaml`](https://pnpm.io/settings). pnpm's [selector grouping](https://github.com/pnpm/pnpm/blob/v10.20.0/patching/config/src/groupPatchedDependencies.ts) treats `*` as all versions, including prereleases. Both declarations and lock markers matter, including before installation. | +| Yarn | [`patch:`](https://yarnpkg.com/protocol/patch) changes package source at fetch time. [`yarn patch`](https://yarnpkg.com/features/patching) keeps the diff separately; replacing a locator is not a merge of that diff with Socket's artifact. Yarn builtin compatibility patches are distinguished from user patch paths. | +| `patch-package` | Its [documented workflow](https://github.com/ds300/patch-package) applies diffs after installation, supports workspace-local setups and `--patch-dir`. Its [filename parser](https://github.com/ds300/patch-package/blob/master/src/PackageDetails.ts) identifies scoped/nested targets, patch sequences and dev-only patches. Looking only for a dependency on `patch-package` is insufficient. | +| npm / Bun | [npm patch](https://docs.npmjs.com/cli/v12/commands/npm-patch/) and [Bun patch](https://bun.sh/docs/pm/cli/patch) persist project diffs. Socket's pre-existing native npm/Bun backend refusals remain in force after consent; a successful source rewrite alone does not establish that the user's diff was preserved. | +| Composer | [Composer Patches](https://docs.cweagans.net/composer-patches/usage/defining-patches/) accepts inline and external definitions. [Configuration](https://docs.cweagans.net/composer-patches/usage/configuration/) can relocate that file, and dependencies can provide patches. [`patches.lock.json`](https://docs.cweagans.net/composer-patches/api/patches-lock-json/) is additional read-only evidence. | +| Cargo / Go | Cargo's [`[patch]`, `[replace]`, and `paths`](https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html) and Go's [`replace`](https://go.dev/ref/mod#go-mod-file-replace) can select a user fork or directory. These source choices must not be treated as pristine registry copies. Socket-owned wiring is distinguished from user replacements. | + +Python local/VCS sources, Ruby path/git dependencies, JVM dependency substitutions, +NuGet source choices, and custom build hooks do not share a portable diff registry. +Their existing source/layout restrictions still apply; the consent detector does +not claim to interpret every possible plugin, script or external patch pipeline. +Unknown edits in a file Socket would replace are protected by the default hash +mismatch refusal. A separate, untouched file with an unrecorded local fix cannot +be detected by that hash check. + +Patch declarations and diffs remain the user's files. Consent is not persisted in +`socket.yml`, the manifest, or an environment variable; an automated hook must +specify it deliberately on each invocation if the project intends to combine +patches. Review the resulting installation and run the project's own tests. diff --git a/docs/usage.md b/docs/usage.md index 5f9841df8..304c19577 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -238,3 +238,25 @@ A package converted from hosted to vendored returns to upstream on `vendor --rev Vendored reversal preserves unrelated edits and refuses unsafe drift. Keep its ledger with the artifacts. Full details and per-ecosystem restoration limits are in the [rollback contract](../crates/socket-patch-cli/CLI_CONTRACT.md#rollback-command-contract-v50). + +## Packages you already patch + +By default, `scan` skips packages with declared project patches (pnpm, Yarn, +`patch-package`, native npm/Bun, Composer, Cargo or Go) and reports where the +patch was found. Explicit `get`, `apply` and `vendor` refuse those conflicts. +A user patch is protected even if it changes a different file than Socket's fix. + +After reviewing the interaction, you can explicitly approve an attempt: + +```sh +socket-patch scan --package minimist --mode agent --allow-user-patches +socket-patch apply --allow-user-patches --strict +``` + +The second form still refuses modified file content. Without `--strict`, explicit +consent permits replacing a mismatched file with Socket's verified content; it +does not merge your changes. Without consent, mismatched files fail by default. +`--yes` and `--force` do not approve combining declared project patches. Some +package-manager combinations remain unsupported even with consent. See +[existing project patches](ecosystems.md#existing-project-patches) for the +mechanisms checked and the limitations. From 29dd054d2bb0f3b0c851f4f7151177026de08c3c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 23:16:31 +0000 Subject: [PATCH 2/9] Test cargo vendor guards with user-patch consent The url-spelled crates-io [patch] table and ancestor .cargo/config [patch] fixtures are now caught earlier by the new UserPatchIndex, which refuses with user_patch_conflict before the cargo-specific guards run. Pass --allow-user-patches so these tests keep exercising the guards they were written for (cargo_manifest_patch_source_alias and user_authored_patch_entry); the default refusal is covered by tests/user_patches.rs. Co-Authored-By: Claude --- crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs index cc46f245a..33915132a 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs @@ -2653,6 +2653,7 @@ fn cargo_url_spelled_crates_io_patch_table_is_refused() { "vendor", "--json", "--offline", + "--allow-user-patches", "--cwd", proj.to_str().unwrap(), ], @@ -2734,6 +2735,7 @@ fn cargo_ancestor_config_patch_cannot_silently_shadow_the_vendored_copy() { "vendor", "--json", "--offline", + "--allow-user-patches", "--cwd", proj.to_str().unwrap(), ], From 16b0a0ca0ed1224c3932170c1552b9dce06d3e3b Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 21 Oct 2026 09:12:43 -0400 Subject: [PATCH 3/9] Address Bugbot review on user-patch consent - apply: release apply.lock before reporting a user_patch_conflict, like the manifest-load failures above it, so telemetry can't block siblings. - core: match Cargo [patch]/[replace] names ignoring case and -/_, since crates.io treats those as the same crate. - core: follow go.work `use` members so a member go.mod replace protects its module from the workspace root too. - scan: hide user_patch_conflict/override warnings under --silent, matching user_patches::check; the JSON envelope still carries them. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/src/commands/apply.rs | 1 + .../socket-patch-cli/src/commands/scan/mod.rs | 10 ++++- .../src/patch/user_patches.rs | 42 +++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index dc5ad9efd..bc696229e 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -1143,6 +1143,7 @@ pub(crate) async fn run_locked( match super::user_patches::check(&args.common, manifest.patches.keys()).await { Ok(warnings) => warnings, Err(error) => { + lock.release(); let code = report_apply_failure(&args, &error, &error.message, &telemetry).await; return ApplyRunReport::run_failure(code, &error.code, error.message); } diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index dfd5cd8bd..32e473f3a 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -1217,13 +1217,21 @@ fn is_info_note(code: &str) -> bool { } /// Print the run-level warnings to stderr: advisories as `Note:` (not -/// under `--silent`), everything else as `Warning:`. +/// under `--silent`), everything else as `Warning:`. User-patch skips are +/// silenced too, matching `user_patches::check`; the envelope keeps them. fn print_layout_refusals(refusals: &[(String, String)], silent: bool) { for (code, detail) in refusals { if is_info_note(code) { if !silent { eprintln!("Note: {detail}"); } + } else if matches!( + code.as_str(), + super::user_patches::CONFLICT | super::user_patches::ALLOWED + ) { + if !silent { + eprintln!("Warning: {detail}"); + } } else { eprintln!("Warning: {detail}"); } diff --git a/crates/socket-patch-core/src/patch/user_patches.rs b/crates/socket-patch-core/src/patch/user_patches.rs index 62ba25d09..11f29b682 100644 --- a/crates/socket-patch-core/src/patch/user_patches.rs +++ b/crates/socket-patch-core/src/patch/user_patches.rs @@ -180,6 +180,14 @@ impl UserPatchIndex { 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)), + ); + } } } for pattern in globs @@ -294,6 +302,19 @@ impl UserPatchIndex { && (e.name.is_empty() || if ecosystem == "composer" { e.name.eq_ignore_ascii_case(name) + } else if ecosystem == "cargo" { + // crates.io names are unique ignoring case and -/_. + e.name.len() == name.len() + && e.name.bytes().zip(name.bytes()).all(|(a, b)| { + let fold = |c: u8| { + if c == b'_' { + b'-' + } else { + c.to_ascii_lowercase() + } + }; + fold(a) == fold(b) + }) } else { e.name == name }) @@ -862,6 +883,7 @@ mod tests { let index = UserPatchIndex::discover(tmp.path()).await; for purl in [ "pkg:cargo/real@1.0.0", + "pkg:cargo/Real@1.0.0", "pkg:cargo/old@1.0.0", "pkg:golang/example.com/user@v1.0.0", ] { @@ -877,6 +899,26 @@ mod tests { } } + #[tokio::test] + async fn go_work_members_replaces_are_consulted() { + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "go.work", "go 1.23\nuse (\n ./app\n)\n"); + write( + tmp.path(), + "app/go.mod", + "module example.com/app\nreplace example.com/user => ../fork\n", + ); + for root in [tmp.path().to_path_buf(), tmp.path().join("app")] { + let index = UserPatchIndex::discover(&root).await; + assert!(!index + .conflicts("pkg:golang/example.com/user@v1.0.0") + .is_empty()); + assert!(index + .conflicts("pkg:golang/example.com/other@v1.0.0") + .is_empty()); + } + } + #[tokio::test] async fn unknown_metadata_does_not_silently_allow_patching() { for text in [ From 9a536158793dc559b614561dc07d450ed8e538fa Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 21 Oct 2026 09:18:02 -0400 Subject: [PATCH 4/9] Test cargo redirect guards with user-patch consent The #480 [patch] override shapes are now caught first by the user-patch gate (user_patch_conflict), so they no longer reach redirect_cargo_dep_overridden. Scan with --allow-user-patches so the shapes keep exercising the redirect guards, and ignore the resulting user_patch_override notice; tests/user_patches.rs covers the gate. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs b/crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs index ac1e6732c..966d2bfba 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs @@ -550,6 +550,9 @@ async fn run_shape(shape: Shape) -> Option<()> { "--json", "--yes", "--no-telemetry", + // Consent past the user-patch gate so the shapes reach the + // redirect guards; tests/user_patches.rs covers the gate itself. + "--allow-user-patches", "--cwd", &proj_s, "--api-url", @@ -574,6 +577,7 @@ async fn run_shape(shape: Shape) -> Option<()> { .into_iter() .flatten() .filter_map(|w| w["code"].as_str()) + .filter(|c| *c != "user_patch_override") .collect(); assert_eq!( codes, From 4b3d6208d83435412d091e647291ef8ca4fd8ee8 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 21 Oct 2026 09:29:28 -0400 Subject: [PATCH 5/9] Fix user-patch detector false positives and review findings From /code-review high on #1646: - A `cd` after patch-package (`patch-package && cd ios && pod install`) no longer marks the patch directory unresolvable and blocks all npm. - Yarn alias locators (`alias@npm:real@1.0.0`) protect `real`, the name the crawler reports, instead of a name that never matches. - Cargo [patch] paths use vendor's path_is_socket_owned, so a user path like `.socket/vendor/../../forks/x` is no longer treated as Socket's. - yarn.lock is scanned line by line for patch resolutions instead of parsed (every Berry lock has compat patches); pnpm locks parse with a budget large enough for big monorepos; package-lock/bun.lock are only parsed when they mention `patched`, so a stub or broken lock with no patch evidence no longer protects every npm package. - Workspace globs walk directories only, take only dirs with a package.json as members, flag a symlink only when it would be a member, and resolve `..` lexically (re-rooting `../shared/*`). - Approved user_patch_override warnings print once: check() no longer prints, callers do, and a nested apply leaves them to its caller. - The hosted re-check refuses through the hosted module's refuse(), so the scan envelope is kept and telemetry precedes the JSON error. - A scan whose every package is protected says so instead of "No packages found. Run your package manager's install first." - Go hosted/build tests for replace guards now pass --allow-user-patches so they keep testing the guards behind the consent gate. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/src/commands/apply.rs | 14 +- crates/socket-patch-cli/src/commands/get.rs | 10 +- .../src/commands/scan/hosted.rs | 16 +- .../socket-patch-cli/src/commands/scan/mod.rs | 13 +- .../src/commands/user_patches.rs | 18 +- .../socket-patch-cli/src/commands/vendor.rs | 18 +- .../tests/e2e_golang_build.rs | 27 +- .../tests/e2e_golang_hosted_state.rs | 9 +- .../src/patch/user_patches.rs | 295 +++++++++++++++--- 9 files changed, 340 insertions(+), 80 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index bc696229e..103ed03b3 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -1241,11 +1241,15 @@ pub(crate) async fn run_locked( // rightly ignores it). It rides the run-warning channel so it // is never silent. let mut run_warnings = run_warnings; - run_warnings.extend( - user_patch_warnings - .into_iter() - .map(|(code, detail)| RunWarning { code, detail }), - ); + // A nested apply's caller (get, scan) already reported the + // overrides it approved; repeating them would double each line. + if args.nested.is_none() { + run_warnings.extend( + user_patch_warnings + .into_iter() + .map(|(code, detail)| RunWarning { code, detail }), + ); + } run_warnings.extend(results.iter().filter_map(|r| { let note = r.error.as_deref().filter(|_| r.success)?; note.contains(socket_patch_core::patch::apply::OWNERSHIP_NOT_RESTORED_MARKER) diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index b394f7e57..808ec445d 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -1302,7 +1302,10 @@ pub async fn run(args: GetArgs) -> i32 { let selected = vec![search_result_from_response(&patch)]; let user_patch_warnings = match super::user_patches::check(&args.common, [&patch.purl]).await { - Ok(warnings) => warnings, + Ok(warnings) => { + super::user_patches::warn(&args.common, &warnings); + warnings + } Err(error) => { return super::user_patches::refuse( &args.common, @@ -1741,7 +1744,10 @@ pub async fn run(args: GetArgs) -> i32 { selected }; match super::user_patches::check(&args.common, selected.iter().map(|p| &p.purl)).await { - Ok(warnings) => narrow_warnings.extend(warnings), + Ok(warnings) => { + super::user_patches::warn(&args.common, &warnings); + narrow_warnings.extend(warnings); + } Err(error) => return super::user_patches::refuse(&args.common, JsonCommand::Get, error), } if agent_preview { diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index 6acbc61f7..a3e47ca5c 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -1047,11 +1047,17 @@ async fn run_redirect_selected_untracked( if let Err(error) = crate::commands::user_patches::check(common, candidates.iter().map(&sel_purl)).await { - let command = scan_result - .as_ref() - .map(|env| env.command) - .unwrap_or(crate::json_envelope::Command::Scan); - return crate::commands::user_patches::refuse(common, command, error); + // The shared refusal path keeps this run's envelope and lets + // telemetry go out before the JSON error. + return refuse( + common, + scan_result.take(), + &engine::Refusal { + code: error.code, + message: error.message, + }, + &mut telemetry.json_error, + ); } } diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index 32e473f3a..c844484fd 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -2225,11 +2225,15 @@ async fn run_scan( // Keep the full crawl above for GC: a protected package is still installed. // Protect before querying/selecting, in every mode and in dry-run previews. let user_patches = super::user_patches::inspect(&args.common).await; + let mut user_patch_withheld = 0usize; filtered_crawled.retain(|package| { let sources = user_patches.conflicts(&package.purl); if sources.is_empty() { return true; } + if !args.common.allow_user_patches { + user_patch_withheld += 1; + } layout_refusals.push(( if args.common.allow_user_patches { super::user_patches::ALLOWED @@ -2348,8 +2352,13 @@ async fn run_scan( emit_scan(&env); return code; } else if !args.common.silent { - // A project the policy skipped as a whole is not an empty one. - if !policy.root_excluded() { + // A project the policy skipped as a whole is not an empty one, + // and neither is one whose every package the consent gate held. + if user_patch_withheld > 0 { + println!( + "All {user_patch_withheld} installed package(s) are protected by project patches (see the warnings above); nothing was scanned. Pass --allow-user-patches to scan them anyway." + ); + } else if !policy.root_excluded() { println!( "{}", render::no_packages_message( diff --git a/crates/socket-patch-cli/src/commands/user_patches.rs b/crates/socket-patch-cli/src/commands/user_patches.rs index 01bf4c835..02f29699a 100644 --- a/crates/socket-patch-cli/src/commands/user_patches.rs +++ b/crates/socket-patch-cli/src/commands/user_patches.rs @@ -28,8 +28,9 @@ pub(crate) fn detail(purl: &str, sources: &[String], allowed: bool) -> String { } /// Explicit-target commands fail before mutation rather than silently succeeding -/// without the requested patch. The caller may attach approved warnings to its -/// own envelope. Nothing here downloads or applies the user's patch. +/// without the requested patch. Approved overrides come back as warnings for +/// the caller to print once (see [`warn`]) and attach to its own envelope. +/// Nothing here downloads or applies the user's patch. pub(crate) async fn check( common: &GlobalArgs, purls: impl IntoIterator>, @@ -48,9 +49,6 @@ pub(crate) async fn check( } let message = detail(purl, &sources, common.allow_user_patches); if common.allow_user_patches { - if !common.silent && !common.json { - eprintln!("Warning: {message}"); - } warnings.push((ALLOWED.into(), message)); } else { conflicts.push(message); @@ -65,6 +63,16 @@ pub(crate) async fn check( } } +/// The human copy of [`check`]'s approved overrides; JSON carries them in +/// the envelope and `--silent` mutes them. +pub(crate) fn warn(common: &GlobalArgs, warnings: &[(String, String)]) { + if !common.silent && !common.json { + for (_, detail) in warnings { + eprintln!("Warning: {detail}"); + } + } +} + pub(crate) fn refuse(common: &GlobalArgs, command: Command, error: EnvelopeError) -> i32 { if common.json { let mut env = Envelope::new(command); diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index a8fdee231..31f1a6891 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1730,7 +1730,10 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { let common = &args.common; let user_patch_warnings = match super::user_patches::check(common, pins.iter().map(|p| &p.purl)).await { - Ok(warnings) => warnings, + Ok(warnings) => { + super::user_patches::warn(common, &warnings); + warnings + } Err(error) => return super::user_patches::refuse(common, Command::Vendor, error), }; let (client, use_public_proxy) = @@ -2228,11 +2231,14 @@ async fn run_vendor( // even when zero in-scope patches remain. Its post-reconcile ledger // feeds the staging harvest below and then the engine. match super::user_patches::check(common, manifest.patches.keys()).await { - Ok(warnings) => env.warnings.extend( - warnings - .into_iter() - .map(|(code, detail)| crate::json_envelope::RunWarning { code, detail }), - ), + Ok(warnings) => { + super::user_patches::warn(common, &warnings); + env.warnings.extend( + warnings + .into_iter() + .map(|(code, detail)| crate::json_envelope::RunWarning { code, detail }), + ); + } Err(error) => { if !common.json { eprintln!("Error: {}", error.message); diff --git a/crates/socket-patch-cli/tests/e2e_golang_build.rs b/crates/socket-patch-cli/tests/e2e_golang_build.rs index 92a861bd8..4c6f2384b 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -720,6 +720,24 @@ fn apply_json(dir: &Path, modcache: &Path) -> (i32, String, String) { ) } +/// [`apply_json`] with `--allow-user-patches`, for guards behind that gate. +fn apply_json_consented(dir: &Path, modcache: &Path) -> (i32, String, String) { + run_socket( + dir, + &[ + "apply", + "--offline", + "--allow-user-patches", + "--ecosystems", + "golang", + "--json", + "--cwd", + dir.to_str().unwrap(), + ], + modcache, + ) +} + fn check(dir: &Path, modcache: &Path) -> (i32, String, String) { run_socket( dir, @@ -792,6 +810,8 @@ fn go_work_replace_overrides_the_socket_replace() { assert!(go_out(&consumer, &env).contains("OUT: FORK")); write_patch(&consumer); + // The go.work replace is also a user patch; consent past that gate so + // the run reaches the go.work guard this test is about. // The dry run predicts the refusal. let (code, so, se) = run_socket( &consumer, @@ -799,6 +819,7 @@ fn go_work_replace_overrides_the_socket_replace() { "apply", "--offline", "--dry-run", + "--allow-user-patches", "--ecosystems", "golang", "--json", @@ -812,7 +833,7 @@ fn go_work_replace_overrides_the_socket_replace() { "apply --dry-run must predict the refusal:\n{so}\n{se}" ); assert!(so.contains("go.work replaces"), "{so}"); - let (code, so, se) = apply_json(&consumer, &modcache); + let (code, so, se) = apply_json_consented(&consumer, &modcache); assert_ne!( code, 0, "apply must refuse an overridden replace:\n{so}\n{se}" @@ -827,12 +848,12 @@ fn go_work_replace_overrides_the_socket_replace() { // Applied before the go.work replace existed: the replace is inert. std::fs::remove_file(consumer.join("go.work")).unwrap(); - let (code, so, se) = apply_json(&consumer, &modcache); + let (code, so, se) = apply_json_consented(&consumer, &modcache); assert_eq!(code, 0, "apply without go.work:\n{so}\n{se}"); std::fs::write(consumer.join("go.work"), &go_work).unwrap(); assert!(go_out(&consumer, &env).contains("OUT: FORK")); // A re-run doesn't call the inert replace in sync. - let (code, so, se) = apply_json(&consumer, &modcache); + let (code, so, se) = apply_json_consented(&consumer, &modcache); assert_ne!(code, 0, "re-run apply must refuse:\n{so}\n{se}"); let (code, so, se) = check(&consumer, &modcache); assert_eq!( diff --git a/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs b/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs index d41f404e0..fd333e516 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs @@ -205,7 +205,14 @@ async fn refused_go_rewrite_is_not_confirmed_by_another_lockfile() { let server = MockServer::start().await; mount_hosted_grant(&server).await; - let env = get_hosted(&consumer, &server, &tmp.path().join("modcache")); + // The user's replace is also a user patch; consent past that gate so + // the run reaches the hosted rewriter's own replace refusal. + let env = get_hosted_with( + &consumer, + &server, + &tmp.path().join("modcache"), + &["--allow-user-patches"], + ); assert_eq!(hosted_pinned(&env), 0, "envelope: {env}"); assert!( env["warnings"] diff --git a/crates/socket-patch-core/src/patch/user_patches.rs b/crates/socket-patch-core/src/patch/user_patches.rs index 11f29b682..bce1c4542 100644 --- a/crates/socket-patch-core/src/patch/user_patches.rs +++ b/crates/socket-patch-core/src/patch/user_patches.rs @@ -98,13 +98,25 @@ impl UserPatchIndex { } else { index.patch_directory(&dir.join("patches"), false).await; } - for file in ["pnpm-workspace.yaml", "pnpm-lock.yaml", "yarn.lock"] { - let path = dir.join(file); - if let Some(text) = index.read(&path, "npm").await { - // Yarn classic isn't YAML; only Berry has patch: locators. - if file == "yarn.lock" && !text.contains("patch:") { + 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. + for line in text.lines() { + 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); } + } + } + 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") @@ -112,7 +124,7 @@ impl UserPatchIndex { { continue; } - match serde_saphyr::from_multiple::(&text) { + match serde_saphyr::from_multiple_with_options::(&text, lock_options()) { Ok(documents) => { for value in documents { index.native(&value, &path); @@ -126,20 +138,6 @@ impl UserPatchIndex { ); } } - if file == "yarn.lock" { - if let Some(map) = value.as_object() { - for entry in map.values() { - if let Some(spec) = entry["resolution"].as_str() { - // A locator has an ident before @patch:. - if let Some((_, source)) = - spec.split_once("@patch:") - { - index.yarn(&format!("patch:{source}"), &path); - } - } - } - } - } } } Err(e) => index.unknown( @@ -152,6 +150,15 @@ 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. + if !index + .read(&path, "npm") + .await + .is_some_and(|text| text.contains("patched")) + { + continue; + } if let Some(value) = index.json(&path, "npm").await { index.native(&value, &path); if let Some(packages) = value["packages"].as_object() { @@ -204,10 +211,37 @@ impl UserPatchIndex { // 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 { - if Path::new(pattern).is_absolute() || pattern.split('/').any(|p| p == "..") { + if Path::new(pattern).is_absolute() { self.unknown("npm", root, "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 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"); + return Vec::new(); + } + Some(_) => { + segments.pop(); + } + None => { + if !root.pop() { + self.unknown("npm", &root, "workspace outside the filesystem root"); + return Vec::new(); + } + } + }, + _ => segments.push(segment), + } + } + let pattern = segments.join("/"); + let (root, pattern) = (root.as_path(), pattern.as_str()); let prefix: std::path::PathBuf = pattern .split('/') .take_while(|p| !p.contains(['*', '?', '{', '[', '('])) @@ -229,10 +263,11 @@ impl UserPatchIndex { .max_depth(max_depth) .into_iter() .filter_entry(|entry| { - !matches!( - entry.file_name().to_str(), - Some("node_modules" | ".git" | ".socket" | "target" | ".venv") - ) + (entry.file_type().is_dir() || entry.path_is_symlink()) + && !matches!( + entry.file_name().to_str(), + Some("node_modules" | ".git" | ".socket" | "target" | ".venv") + ) }); let mut out = Vec::new(); let patterns = vec![pattern.to_string()]; @@ -258,24 +293,33 @@ impl UserPatchIndex { if entry.depth() == 32 && entry.file_type().is_dir() { self.unknown("npm", entry.path(), "workspace inspection depth exceeded"); } - if entry.file_type().is_symlink() && (entry.depth() == 0 || entry.path().is_dir()) { - self.unknown( - "npm", - entry.path(), - "linked workspace cannot be inspected safely", - ); - continue; - } - if workspaces_include_as(&patterns, &components, WorkspacesDialect::YARN_BERRY) { - if entry.file_type().is_symlink() { + // 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() { + // 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() + && (member + || workspaces_include_as( + &patterns, + &components, + WorkspacesDialect::YARN_BERRY, + ) && !pattern.contains("**"))) + { self.unknown( "npm", entry.path(), "linked workspace cannot be inspected safely", ); - } else if entry.file_type().is_dir() { - out.push(entry.path().to_path_buf()); } + continue; + } + if member + && workspaces_include_as(&patterns, &components, WorkspacesDialect::YARN_BERRY) + { + out.push(entry.path().to_path_buf()); } } out @@ -447,13 +491,8 @@ impl UserPatchIndex { continue; } if user_patch { - let (name, version) = split_selector(&source); - self.add( - "npm", - name, - version.map(|v| v.strip_prefix("npm:").unwrap_or(v)), - path, - ); + let (name, version) = locator_target(&source); + self.add("npm", name, version, path); } return; } @@ -474,9 +513,12 @@ impl UserPatchIndex { .filter_map(Value::as_str) .filter(|s| s.contains("patch-package")) { - if script.contains("cd ") - || (script.contains("--patch-dir") && !flag.is_match(script)) - { + // A `cd` after patch-package (`patch-package && cd ios && pod + // install`) can't move the directory it already read. + let moved = script + .find("cd ") + .is_some_and(|cd| cd < script.rfind("patch-package").unwrap_or(0)); + if moved || (script.contains("--patch-dir") && !flag.is_match(script)) { self.unknown( "npm", &dir.join("package.json"), @@ -485,7 +527,7 @@ impl UserPatchIndex { } for captures in flag.captures_iter(script) { let path = captures.iter().skip(1).flatten().next().unwrap().as_str(); - if path.contains(['$', '`']) || script.contains("cd ") { + if path.contains(['$', '`']) || moved { self.unknown( "npm", &dir.join("package.json"), @@ -616,9 +658,8 @@ impl UserPatchIndex { for (key, entry) in table.iter() { // Socket's managed paths/registries must remain eligible for updates. let target = entry.get("path").and_then(toml_edit::Item::as_str); - if target.is_some_and(|p| { - p.trim_start_matches("./").starts_with(".socket/vendor/") - }) { + if target.is_some_and(crate::vendor::cargo_config::path_is_socket_owned) + { continue; } let name = entry @@ -653,6 +694,37 @@ impl UserPatchIndex { } } +/// 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>) { + let at = locator + .char_indices() + .skip(1) + .find(|&(_, c)| c == '@') + .map(|(i, _)| i); + let Some(at) = at else { + return (locator, None); + }; + let (name, range) = (&locator[..at], &locator[at + 1..]); + match range.strip_prefix("npm:") { + Some(target) if target.get(1..).is_some_and(|t| t.contains('@')) => split_selector(target), + Some(version) => (name, Some(version)), + None => (name, Some(range)), + } +} + +fn lock_options() -> serde_saphyr::Options { + // Lockfiles of large monorepos exceed the default 250k-node budget, and a + // parse failure would protect every npm package. + let mut budget = serde_saphyr::Budget::default(); + budget.max_events = 64_000_000; + budget.max_nodes = 32_000_000; + budget.max_total_scalar_bytes = 1 << 30; + let mut options = serde_saphyr::Options::default(); + options.budget = Some(budget); + options +} + fn split_selector(key: &str) -> (&str, Option<&str>) { match key.rsplit_once('@') { Some((name, version)) if !name.is_empty() => (name, Some(version)), @@ -899,6 +971,127 @@ mod tests { } } + #[tokio::test] + async fn cd_after_patch_package_keeps_the_default_directory() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "package.json", + r#"{"scripts":{"postinstall":"patch-package && cd ios && pod install"}}"#, + ); + write(tmp.path(), "patches/plain+1.0.0.patch", "user diff"); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!(!index.conflicts("pkg:npm/plain@1.0.0").is_empty()); + assert!( + index.conflicts("pkg:npm/other@1.0.0").is_empty(), + "{index:?}" + ); + + write( + tmp.path(), + "package.json", + r#"{"scripts":{"postinstall":"cd app && patch-package"}}"#, + ); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!(!index.conflicts("pkg:npm/other@1.0.0").is_empty()); + } + + #[tokio::test] + async fn yarn_alias_patch_protects_the_real_package() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "package.json", + r#"{"dependencies":{"alias":"patch:alias@npm%3Areal%401.0.0#./fix.patch"}}"#, + ); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!( + !index.conflicts("pkg:npm/real@1.0.0").is_empty(), + "{index:?}" + ); + assert!( + index.conflicts("pkg:npm/real@2.0.0").is_empty(), + "{index:?}" + ); + } + + #[test] + fn locator_target_reads_plain_scoped_and_aliased_locators() { + assert_eq!(locator_target("real@npm:1.0.0"), ("real", Some("1.0.0"))); + assert_eq!(locator_target("@s/p@npm:1.0.0"), ("@s/p", Some("1.0.0"))); + assert_eq!(locator_target("a@npm:real@1.0.0"), ("real", Some("1.0.0"))); + assert_eq!(locator_target("a@npm:@s/p@1.0.0"), ("@s/p", Some("1.0.0"))); + assert_eq!(locator_target("a@npm:"), ("a", Some(""))); + assert_eq!(locator_target("bare"), ("bare", None)); + } + + #[tokio::test] + async fn cargo_path_merely_under_socket_vendor_is_still_a_user_override() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "Cargo.toml", + "[patch.crates-io]\nserde = { path = '.socket/vendor/../../forks/serde' }\n", + ); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!(!index.conflicts("pkg:cargo/serde@1.0.0").is_empty()); + } + + #[tokio::test] + async fn double_star_workspace_ignores_non_member_directories() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "pnpm-workspace.yaml", + "packages: ['packages/**']\n", + ); + write(tmp.path(), "packages/a/package.json", "{}"); + write( + tmp.path(), + "packages/a/src/fixtures/bad/package.json", + "{broken", + ); + write(tmp.path(), "packages/a/dist/x.js", ""); + let index = UserPatchIndex::discover(tmp.path()).await; + // pnpm itself reads that broken manifest as a project, so it still + // protects npm; directories without one are not inspected at all. + assert!(!index.conflicts("pkg:npm/x@1.0.0").is_empty()); + std::fs::remove_file(tmp.path().join("packages/a/src/fixtures/bad/package.json")).unwrap(); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!(index.conflicts("pkg:npm/x@1.0.0").is_empty(), "{index:?}"); + } + + #[tokio::test] + async fn dot_dot_workspace_globs_resolve_lexically() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "app/package.json", + r#"{"workspaces":["packages/b/../a", "../shared/*"]}"#, + ); + write(tmp.path(), "app/packages/a/package.json", "{}"); + write( + tmp.path(), + "app/packages/a/patches/one+1.0.0.patch", + "user diff", + ); + write(tmp.path(), "shared/s/package.json", "{}"); + write(tmp.path(), "shared/s/patches/two+2.0.0.patch", "user diff"); + let index = UserPatchIndex::discover(&tmp.path().join("app")).await; + assert!( + !index.conflicts("pkg:npm/one@1.0.0").is_empty(), + "{index:?}" + ); + assert!( + !index.conflicts("pkg:npm/two@2.0.0").is_empty(), + "{index:?}" + ); + assert!( + index.conflicts("pkg:npm/other@1.0.0").is_empty(), + "{index:?}" + ); + } + #[tokio::test] async fn go_work_members_replaces_are_consulted() { let tmp = tempfile::tempdir().unwrap(); From e48ded2097002e448eeac2813f04371b328c5d05 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 21 Oct 2026 13:34:44 -0400 Subject: [PATCH 6/9] Skip user-patched packages in apply; allow Socket-written drift Two review findings on #1646, resolved as decided with the maintainer: - apply no longer refuses a whole run over one protected manifest entry (a postinstall apply would fail every install). Protected records sit the run out with a user_patch_conflict warning, everything else applies, and skips alone exit 0. The manifest on disk is unchanged, and skipped Go entries are kept out of the stale-redirect reconcile. `apply --check` stays read-only and verifies every entry. Explicit `get` and `vendor` still refuse; vendor now reverts dropped entries before its check so a protected record can't block that cleanup. - The default mismatch policy is the new MismatchPolicy::SocketContent: Strict, except a file whose hash names a stored .socket/blobs blob (another Socket patch's output, as after an update to a newer patch for the same release) is overwritten with the usual content_mismatch_overwritten warning. --strict stays fully strict. Contract and usage docs updated to match. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/CLI_CONTRACT.md | 26 +++-- crates/socket-patch-cli/src/commands/apply.rs | 41 ++++--- .../src/commands/user_patches.rs | 39 +++++++ .../socket-patch-cli/src/commands/vendor.rs | 4 +- crates/socket-patch-cli/tests/user_patches.rs | 104 ++++++++++++++++-- crates/socket-patch-core/src/patch/apply.rs | 90 ++++++++++++++- docs/ecosystems.md | 4 +- docs/usage.md | 6 +- 8 files changed, 275 insertions(+), 39 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 71f314617..5bb196004 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -125,7 +125,11 @@ Every subcommand accepts the same set of "global" flags via a single shared `Glo The in-place apply paths (`apply`, agent-mode `get` / `scan`, and their nested apply calls) reject files matching neither `beforeHash` nor `afterHash` by default. This also protects a pre-existing file where the Socket patch adds a -new file (empty `beforeHash`). `--allow-user-patches` permits the old +new file (empty `beforeHash`). The exception is content a Socket patch wrote: +a file whose hash names a stored `.socket/blobs/` blob (as after an update to a +newer patch for the same release) is overwritten with the +`content_mismatch_overwritten` warning, except under `--strict`. +`--allow-user-patches` permits the old warn-and-overwrite behavior: only the full, hash-verified `afterHash` content can be written, with `content_mismatch_overwritten` diagnostics. `--strict` keeps rejecting file drift even with that consent. `apply --force` remains @@ -147,13 +151,17 @@ A missing referenced diff does not remove that protection. - `scan` (every mode, including previews) excludes matching packages before patch selection and emits `warnings[].code: user_patch_conflict`, naming the purl - and evidence path. Other packages remain eligible; skips alone exit 0. A nested - agent apply also checks existing in-scope manifest entries and refuses if any - conflict remains, even if the newly selected records have already been saved. + and evidence path. Other packages remain eligible; skips alone exit 0. The full installed inventory still protects skipped packages from prune-as-missing. -- Explicit `get` selection (including UUID, `--save-only`, and `--dry-run`), - mutating `apply`, and `vendor` refuse selected conflicts with exit 1, - `status: error`, `error.code: user_patch_conflict`, before their patch writes. +- Mutating `apply`, including the nested apply of agent-mode `get` / `scan`, + leaves each protected manifest entry out of the run with a + `user_patch_conflict` warning and applies every other entry; skips alone exit 0, + so a `postinstall` apply never fails an install over them. The manifest is not + changed, and a skipped entry is not reconciled away as dropped. +- Explicit `get` selection (including UUID, `--save-only`, and `--dry-run`) and + `vendor` refuse selected conflicts with exit 1, `status: error`, + `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. - `--allow-user-patches` approves the attempt for this invocation and emits `user_patch_override` warnings. It is forwarded to nested apply. `--yes`, @@ -1657,14 +1665,14 @@ command's integration tests check its output. | `vendor_prebuilt_stub_invalid` | `failed` | RubyGems: the server stub lacks required attributes or is otherwise invalid; no local stub fallback is permitted. | | `vendor_*` / `pypi_*` / `gemfile_*` / `lock_*` / `locked_version_mismatch` / `user_authored_*` / `native_extensions_unsupported` / `platform_gem_unsupported` | `failed`/`skipped` | vendor: per-ecosystem refusal + drift vocabulary; see the Vendor command contract section. New tags are additive (MINOR). | -`user_patch_conflict` is also a scan run warning for packages excluded by the +`user_patch_conflict` is also a scan and apply run warning for packages excluded by the project-patch consent gate; `user_patch_override` records explicit consent. ### Top-level `EnvelopeError` codes | Code | Subcommands | Meaning | |-----------------------|----------------------------------|---------| -| `user_patch_conflict` | get, apply, vendor; hosted/vendored write rechecks | A selected package has project-owned patch/replacement evidence and no `--allow-user-patches` consent. Evidence paths and a remedy are included; exit 1. | +| `user_patch_conflict` | get, vendor; hosted/vendored write rechecks | A selected package has project-owned patch/replacement evidence and no `--allow-user-patches` consent. Evidence paths and a remedy are included; exit 1. (`scan` and `apply` report it as a warning and skip the package.) | | `manifest_not_found` | remove, repair, rollback, vex, apply (only when the manifest vanishes between apply's existence probe and its locked read) (not `list` since v5.0: a missing manifest is an empty list) | `.socket/manifest.json` doesn't exist. For `vex` (and `scan --vex`) it fires only when, in addition, NOTHING else names a patch — no vendor-ledger entry, no lockfile reference (hosted or vendored) — and the message says so (exit 2 standalone; `apply`/`vendor --vex` treat it as their calm no-op). v3.5: `repair` proceeds anyway (vendored phase only) when a vendor ledger or vendor-path lockfile references exist, and exits 0 with a `redirect_only_project` skip (not this error) when the project's only patch state is hosted pins in its lockfiles (v5.0; or a pre-v5 `redirect-state.json`). `list` likewise no longer fires this on a hosted-only project: v5.0 lists every hosted pin the lockfiles wire (exit 0, labeled `details.mode: "hosted"` + `details.lockfiles: []` — no `details.ledger`, since hosted mode keeps none; when the manifest exists too, both are shown, purl-sorted with the manifest entry first on a tie). A pin carries its uuid and empty details unless a pre-v5 redirect ledger records the same purl and uuid (read for migration only: its record supplies the vulnerabilities / tier / description); a pre-v5 ledger record whose pin is in no lockfile is not listed. v5.0: `list` reads the vendor ledger the same way — a vendored-only project (every `scan`/`get --mode vendored` project) lists its ledger entries' embedded records labeled `Mode: vendored (recorded in .socket/vendor/state.json)` in human mode — the twin of the hosted `Mode: hosted (wired in )` line — (`details.mode: "vendored"` + `details.ledger: ".socket/vendor/state.json"` in JSON), exit 0. A standalone-`vendor` entry's fallback `record` lists the same way once no manifest entry covers it (by ledger key or base purl) — the copy manifest-less `vex` attests from, so `list` never reports `manifest_not_found` for a tree whose VEX document attests a patch; while the manifest covers it, only the manifest entry is listed. All sources always come from the SAME project (see the `--manifest-path` flag row), so `--manifest-path` into another project reads that project's state, never the local one. The error still fires when NONE of the three sources has a patch, and a present-but-broken manifest still reports `manifest_invalid`/`manifest_unreadable` regardless (corruption is never masked). A malformed pre-v5 redirect ledger degrades to "nothing to consult" with a stderr warning, muted by `--silent` (the pins still list); `list --json` carries it in the run-level `warnings[]` as `redirect_ledger_corrupt` instead of on stderr. v5.0: `rollback` likewise proceeds manifest-less when the vendor ledger or the lockfiles' hosted pins hold work (its error is the legacy `{status: "error", error: {code: "manifest_not_found", message: "Manifest not found"}, path}` shape); only the truly-empty project — no manifest, no vendor ledger, no hosted pin (a lone pre-v5 redirect ledger is deleted, exit 0) — keeps the exit-1 error, and a project whose lockfiles still reference `.socket/vendor/` artifacts with NO vendor ledger gets a distinct error naming `socket-patch repair`. `remove` (v5.0) proceeds manifest-less whenever a vendor ledger file exists or the lockfiles pin a hosted patch (an existence probe and the read-only hosted-pin discovery before the lock; the vendor ledger loads under it): ANY vendor-ledger entry matching the identifier — detached or not — is removed through the ledger path (`--preserve-state` and drift-keeps behave exactly as on the manifest path), a hosted-only match restores its upstream registry entry, and when that state exists but holds nothing for the identifier the error is `not_found` (exit 1), not this code — `manifest_not_found` fires from `remove` only when all three sources are empty. Manifest entries are removed in sorted purl order. | | `ambiguous_target` | remove, get, rollback | The identifier is a package name whose last-segment rule selects several packages across the manifest, the vendor ledger and the hosted pins (`core` → `@angular/core` and `@babel/core`). Nothing is changed (exit 1); the message names each package. Use the full name or a purl. See **Target grammar**. | | `manifest_invalid` | list, remove, apply (incl. `--check`), repair, vendor (incl. `--check`), vex, rollback, get, scan (agent mode) | Manifest exists but is unparseable (malformed JSON or a schema violation). **v5.0, MAJOR (#931)**: one mapping on every command — `apply` used to say `apply_failed`, `repair` `repair_failed`, `vendor` the undocumented `invalid_manifest`, and `apply --check` / `vendor --check` / `vex` / `get` / `rollback` `manifest_unreadable`. Exit codes unchanged (`vex`: 2, every other command 1). Agent-mode `scan` fails with it (exit 1); the other scan modes, which don't read the manifest, add a warning with the same code and go on. | diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index 103ed03b3..22ac31b3b 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -120,16 +120,19 @@ fn mismatch_overwrite_warnings(results: &[ApplyResult], dry_run: bool) -> Vec MismatchPolicy { if force { MismatchPolicy::Force - } else if strict || !allow_user_patches { + } else if strict { MismatchPolicy::Strict - } else { + } else if allow_user_patches { MismatchPolicy::Warn + } else { + MismatchPolicy::SocketContent } } @@ -1139,15 +1142,14 @@ pub(crate) async fn run_locked( } }; - let user_patch_warnings = - match super::user_patches::check(&args.common, manifest.patches.keys()).await { - Ok(warnings) => warnings, - Err(error) => { - lock.release(); - let code = report_apply_failure(&args, &error, &error.message, &telemetry).await; - return ApplyRunReport::run_failure(code, &error.code, error.message); - } - }; + // Packages the project patched itself sit this run out (warned); every + // other record still applies. The read-only `--check` verifies them all. + let mut manifest = manifest; + let (user_patch_warnings, user_patched) = if args.check { + Default::default() + } else { + super::user_patches::skip_protected(&args.common, &mut manifest.patches).await + }; // Package-manager layout detection. yarn-berry PnP keeps packages // inside `.yarn/cache/*.zip` and resolves them via `.pnp.cjs` — @@ -1219,7 +1221,7 @@ pub(crate) async fn run_locked( NpmPkgManager::Npm | NpmPkgManager::YarnClassic | NpmPkgManager::Unknown => {} } - match apply_patches_inner(&args, manifest, client).await { + match apply_patches_inner(&args, manifest, &user_patched, client).await { Ok(ApplyOutcome { success, results, @@ -1967,6 +1969,7 @@ impl FallbackHomeSkip { async fn apply_patches_inner( args: &ApplyArgs, mut manifest: PatchManifest, + user_patched: &HashSet, client: &ApiClient, ) -> Result { // Resolve patch sources (read `.socket/` directly, or stage an overlay @@ -2016,7 +2019,13 @@ async fn apply_patches_inner( // found" early returns — so orphans are reconciled even when the manifest // now lists zero in-scope go patches (the all-removed case). No-op unless // local go is in scope. - reconcile_local_go(&args.common, &target_manifest_purls).await; + // Records skipped for project patches are still recorded, not orphans. + let recorded: HashSet = target_manifest_purls + .iter() + .chain(user_patched) + .cloned() + .collect(); + reconcile_local_go(&args.common, &recorded).await; if partitioned.is_empty() { // Nothing in scope: the manifest lists no patches (or every patch was diff --git a/crates/socket-patch-cli/src/commands/user_patches.rs b/crates/socket-patch-cli/src/commands/user_patches.rs index 02f29699a..138e4d676 100644 --- a/crates/socket-patch-cli/src/commands/user_patches.rs +++ b/crates/socket-patch-cli/src/commands/user_patches.rs @@ -73,6 +73,45 @@ pub(crate) fn warn(common: &GlobalArgs, warnings: &[(String, String)]) { } } +/// The manifest-wide gate for `apply`: rather than refuse a whole run over +/// one protected entry (a postinstall apply would fail every install), drop +/// the protected records from this run's in-memory copy and report each as +/// a `user_patch_conflict` warning. Approved overrides stay in and come back +/// as `user_patch_override` warnings. Only the caller's copy changes; the +/// manifest on disk is never rewritten here. Also returns the skipped purls: +/// they are still recorded, so nothing may reconcile them away as dropped. +pub(crate) async fn skip_protected( + common: &GlobalArgs, + patches: &mut std::collections::HashMap< + String, + socket_patch_core::manifest::schema::PatchRecord, + >, +) -> (Vec<(String, String)>, std::collections::HashSet) { + let index = inspect(common).await; + let mut warnings = Vec::new(); + let mut skipped = std::collections::HashSet::new(); + patches.retain(|purl, _| { + if !common.purl_ecosystem_selected(purl) { + return true; + } + let sources = index.conflicts(purl); + if sources.is_empty() { + return true; + } + let message = detail(purl, &sources, common.allow_user_patches); + if common.allow_user_patches { + warnings.push((ALLOWED.into(), message)); + true + } else { + warnings.push((CONFLICT.into(), message)); + skipped.insert(purl.clone()); + false + } + }); + warnings.sort(); + (warnings, skipped) +} + pub(crate) fn refuse(common: &GlobalArgs, command: Command, error: EnvelopeError) -> i32 { if common.json { let mut env = Envelope::new(command); diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 31f1a6891..d02bbcd0c 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -2230,6 +2230,9 @@ async fn run_vendor( // previous run whose patches were dropped from the manifest are reverted // even when zero in-scope patches remain. Its post-reconcile ledger // feeds the staging harvest below and then the engine. + let (mut has_errors, ledger) = reconcile_dropped(&manifest, common, env).await; + // After the reconcile: reverting entries dropped from the manifest + // touches no project patch, so a protected record must not block it. match super::user_patches::check(common, manifest.patches.keys()).await { Ok(warnings) => { super::user_patches::warn(common, &warnings); @@ -2247,7 +2250,6 @@ async fn run_vendor( return 1; } } - let (mut has_errors, ledger) = reconcile_dropped(&manifest, common, env).await; let socket_dir = crate::args::socket_dir_of(manifest_path, &common.cwd); if manifest.patches.is_empty() && !common.json && !common.silent { diff --git a/crates/socket-patch-cli/tests/user_patches.rs b/crates/socket-patch-cli/tests/user_patches.rs index c5a5b5fc9..8e6b5ce69 100644 --- a/crates/socket-patch-cli/tests/user_patches.rs +++ b/crates/socket-patch-cli/tests/user_patches.rs @@ -42,7 +42,8 @@ fn fixture() -> tempfile::TempDir { } #[test] -fn apply_and_vendor_refuse_declared_patches_despite_matching_hashes_or_general_confirmation() { +fn apply_skips_and_vendor_refuses_declared_patches_despite_matching_hashes_or_general_confirmation() +{ for (file, contents) in [ ( "package.json", @@ -73,12 +74,24 @@ fn apply_and_vendor_refuse_declared_patches_despite_matching_hashes_or_general_c let original = std::fs::read(tmp.path().join(".socket/manifest.json")).unwrap(); let (code, stdout, stderr) = run_with_env(tmp.path(), &args, &[]); let value: serde_json::Value = serde_json::from_str(&stdout).expect(&stdout); - assert_eq!(code, 1, "{args:?} {file}: {stdout} {stderr}"); - assert_eq!(value["error"]["code"], "user_patch_conflict", "{value}"); - assert!(value["error"]["message"] - .as_str() - .unwrap() - .contains("--allow-user-patches")); + // apply leaves the package out with a warning and succeeds (a + // postinstall apply must not fail every install); vendor, an + // explicit vendoring of the whole manifest, refuses. + let message = if args[0] == "apply" { + assert_eq!(code, 0, "{args:?} {file}: {stdout} {stderr}"); + let warning = value["warnings"] + .as_array() + .unwrap() + .iter() + .find(|w| w["code"] == "user_patch_conflict") + .unwrap_or_else(|| panic!("{args:?} {file}: {value}")); + warning["detail"].as_str().unwrap().to_string() + } else { + assert_eq!(code, 1, "{args:?} {file}: {stdout} {stderr}"); + assert_eq!(value["error"]["code"], "user_patch_conflict", "{value}"); + value["error"]["message"].as_str().unwrap().to_string() + }; + assert!(message.contains("--allow-user-patches"), "{message}"); assert_eq!( std::fs::read(tmp.path().join("node_modules/example/index.js")).unwrap(), BEFORE @@ -183,8 +196,13 @@ fn environment_and_read_only_checks_do_not_grant_or_require_consent() { &["apply", "--json", "--offline"], &[("SOCKET_ALLOW_USER_PATCHES", "1")], ); - assert_eq!(code, 1, "{stdout}"); + assert_eq!(code, 0, "{stdout}"); assert!(stdout.contains("user_patch_conflict")); + assert_eq!( + std::fs::read(tmp.path().join("node_modules/example/index.js")).unwrap(), + BEFORE, + "the environment grants no consent" + ); let (_, stdout, _) = run_with_env( tmp.path(), &["apply", "--json", "--offline", "--check"], @@ -192,3 +210,73 @@ fn environment_and_read_only_checks_do_not_grant_or_require_consent() { ); assert!(!stdout.contains("user_patch_conflict"), "{stdout}"); } + +#[test] +fn apply_skips_only_the_protected_package() { + let tmp = fixture(); + write(tmp.path(), "patches/example+1.0.0.patch", b"user diff\n"); + write( + tmp.path(), + "node_modules/other/package.json", + r#"{"name":"other","version":"2.0.0"}"#, + ); + write(tmp.path(), "node_modules/other/index.js", BEFORE); + let manifest_path = tmp.path().join(".socket/manifest.json"); + let mut manifest: serde_json::Value = + serde_json::from_slice(&std::fs::read(&manifest_path).unwrap()).unwrap(); + let mut other = manifest["patches"][PURL].clone(); + other["uuid"] = "22222222-2222-4222-8222-222222222222".into(); + manifest["patches"]["pkg:npm/other@2.0.0"] = other; + std::fs::write(&manifest_path, manifest.to_string()).unwrap(); + let original = std::fs::read(&manifest_path).unwrap(); + + let (code, stdout, stderr) = run_with_env(tmp.path(), &["apply", "--json", "--offline"], &[]); + assert_eq!(code, 0, "{stdout} {stderr}"); + let value: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + let conflicts: Vec<_> = value["warnings"] + .as_array() + .unwrap() + .iter() + .filter(|w| w["code"] == "user_patch_conflict") + .collect(); + assert_eq!(conflicts.len(), 1, "{value}"); + assert!(conflicts[0]["detail"].as_str().unwrap().contains(PURL)); + assert_eq!( + std::fs::read(tmp.path().join("node_modules/example/index.js")).unwrap(), + BEFORE + ); + assert_eq!( + std::fs::read(tmp.path().join("node_modules/other/index.js")).unwrap(), + AFTER + ); + assert_eq!(std::fs::read(&manifest_path).unwrap(), original); +} + +#[test] +fn default_apply_replaces_an_earlier_socket_patch_but_not_a_local_edit() { + // After an update to a newer patch for the same release, the file holds + // the old patch's output, whose blob Socket stored: not a local change. + let previous: &[u8] = b"older Socket fix\n"; + let tmp = fixture(); + write_blob(&tmp.path().join(".socket"), &git_sha256(previous), previous); + write(tmp.path(), "node_modules/example/index.js", previous); + let (code, stdout, stderr) = run_with_env(tmp.path(), &["apply", "--json", "--offline"], &[]); + assert_eq!(code, 0, "{stdout} {stderr}"); + assert_eq!( + std::fs::read(tmp.path().join("node_modules/example/index.js")).unwrap(), + AFTER + ); + + // --strict still refuses it. + write(tmp.path(), "node_modules/example/index.js", previous); + let (code, stdout, stderr) = run_with_env( + tmp.path(), + &["apply", "--json", "--offline", "--strict"], + &[], + ); + assert_eq!(code, 1, "{stdout} {stderr}"); + assert_eq!( + std::fs::read(tmp.path().join("node_modules/example/index.js")).unwrap(), + previous + ); +} diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs index a54ab1cd9..bfb668c09 100644 --- a/crates/socket-patch-core/src/patch/apply.rs +++ b/crates/socket-patch-core/src/patch/apply.rs @@ -51,8 +51,14 @@ pub enum MismatchPolicy { /// how callers detect and report it). A MISSING pre-existing file is /// still a hard error. Warn, - /// A beforeHash mismatch is a hard error (the CLI default, or `--strict`). + /// A beforeHash mismatch is a hard error (`--strict`). Strict, + /// [`MismatchPolicy::Strict`], except content a Socket patch wrote + /// earlier is overwritten like [`MismatchPolicy::Warn`]: a file whose + /// hash names a stored blob (`.socket/blobs/`) holds another + /// Socket patch's output, as after an update from one patch to a newer + /// one for the same release, so no local change is lost (the CLI default). + SocketContent, /// [`MismatchPolicy::Warn`] PLUS missing pre-existing files are /// skipped instead of failing (`--force`). Force, @@ -908,6 +914,16 @@ async fn apply_package_patch_at( if verify_result.status != VerifyStatus::Ready && verify_result.status != VerifyStatus::AlreadyPatched { + let policy = match policy { + MismatchPolicy::SocketContent + if verify_result.status == VerifyStatus::HashMismatch + && is_socket_content(sources, verify_result.current_hash.as_deref()) + .await => + { + MismatchPolicy::Warn + } + policy => policy, + }; match (verify_result.status, policy) { // Mismatch explicitly tolerated (warn + force): promote to Ready. // The promoted result KEEPS `expected_hash`/`current_hash` @@ -1131,6 +1147,19 @@ async fn apply_package_patch_at( result } +/// Whether `hash` (a file's current content hash) names a blob Socket +/// stored: content some Socket patch wrote, not a local edit. +async fn is_socket_content(sources: &PatchSources<'_>, hash: Option<&str>) -> bool { + let Some(hash) = hash.filter(|h| !h.is_empty() && h.bytes().all(|b| b.is_ascii_hexdigit())) + else { + return false; + }; + sources.mem_blobs.is_some_and(|m| m.contains_key(hash)) + || tokio::fs::symlink_metadata(sources.blobs_path.join(hash)) + .await + .is_ok_and(|m| m.is_file()) +} + /// The on-disk blob source — read `blobs_path/` fail-closed. /// /// SECURITY: `hash` comes from a committed `.socket/manifest.json` that the @@ -1954,6 +1983,65 @@ mod tests { assert!(result.error.is_some()); } + /// The CLI default (`SocketContent`) overwrites content an earlier + /// Socket patch wrote (its hash names a stored blob), as on a patch + /// update, and refuses any other mismatch like `Strict`. + #[tokio::test] + async fn socket_content_policy_overwrites_only_earlier_socket_output() { + let pkg_dir = tempfile::tempdir().unwrap(); + let blobs_dir = tempfile::tempdir().unwrap(); + let patched = b"new socket patch"; + let after_hash = compute_git_sha256_from_bytes(patched); + let previous = b"old socket patch"; + tokio::fs::write(blobs_dir.path().join(&after_hash), patched) + .await + .unwrap(); + tokio::fs::write( + blobs_dir + .path() + .join(compute_git_sha256_from_bytes(previous)), + previous, + ) + .await + .unwrap(); + let mut files = HashMap::new(); + files.insert( + "index.js".to_string(), + PatchFileInfo { + before_hash: "aaaa".to_string(), + after_hash, + }, + ); + let target = pkg_dir.path().join("index.js"); + let sources = PatchSources::blobs_only(blobs_dir.path()); + let run = || { + apply_package_patch( + "pkg:npm/test@1.0.0", + pkg_dir.path(), + &files, + &sources, + false, + MismatchPolicy::SocketContent, + ) + }; + + tokio::fs::write(&target, previous).await.unwrap(); + let result = run().await; + assert!(result.success, "{:?}", result.error); + assert_eq!(tokio::fs::read(&target).await.unwrap(), patched); + let v = &result.files_verified[0]; + assert!( + v.expected_hash.is_some() && v.current_hash != v.expected_hash, + "the overwrite keeps its warning signature" + ); + + let local = b"a local edit"; + tokio::fs::write(&target, local).await.unwrap(); + let result = run().await; + assert!(!result.success); + assert_eq!(tokio::fs::read(&target).await.unwrap(), local); + } + /// beforeHash mismatch across the three policies: the DEFAULT (Warn) /// overwrites with the verified patched content and keeps the /// promoted warning signature (`Ready` + `expected_hash: Some` + diff --git a/docs/ecosystems.md b/docs/ecosystems.md index af9136194..236a4833a 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -1463,8 +1463,8 @@ Prebuilt binaries are published for: ## Existing project patches Socket Patch treats declared project patches as a separate source of ownership. -`scan` skips their packages and reports `user_patch_conflict`; explicit `get`, -`apply` and `vendor` refuse the conflict before patching. Use +`scan` and `apply` skip their packages and report `user_patch_conflict`; +explicit `get` and `vendor` refuse the conflict before patching. Use `--allow-user-patches` only after reviewing how the fixes interact. This flag permits an attempt; it does not merge diffs or bypass unsupported backend cases. Installed file content that matches neither the original nor Socket-patched hash diff --git a/docs/usage.md b/docs/usage.md index 304c19577..54a5dcd39 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -243,7 +243,8 @@ the [rollback contract](../crates/socket-patch-cli/CLI_CONTRACT.md#rollback-comm By default, `scan` skips packages with declared project patches (pnpm, Yarn, `patch-package`, native npm/Bun, Composer, Cargo or Go) and reports where the -patch was found. Explicit `get`, `apply` and `vendor` refuse those conflicts. +patch was found, and `apply` leaves them out while applying everything else. +Explicit `get` and `vendor` refuse those conflicts. A user patch is protected even if it changes a different file than Socket's fix. After reviewing the interaction, you can explicitly approve an attempt: @@ -255,7 +256,8 @@ socket-patch apply --allow-user-patches --strict The second form still refuses modified file content. Without `--strict`, explicit consent permits replacing a mismatched file with Socket's verified content; it -does not merge your changes. Without consent, mismatched files fail by default. +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 [existing project patches](ecosystems.md#existing-project-patches) for the From 6cb279310deae376b46d4f8bf8931dd5069f9e99 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 21 Oct 2026 13:46:19 -0400 Subject: [PATCH 7/9] Address second review pass on user-patch consent - A nested apply under `get` now reports the older manifest entries it skips for project patches (stderr and the JSON envelope via the run report); `get` checks only its own selection. `scan` already reports every protected installed package, so its nested apply stays quiet. - vendor: a conflict found after reconcile_dropped already wrote is recorded as per-package `failed` events instead of a top-level error, which the contract reserves for runs where nothing happened. - patch-package `cd` detection matches `cd` as a command, not the tail of a word such as `abcd`. - The ancestor workspace walk never reaches the home directory or above, so a checkout without `.git` doesn't read `~/.cargo/config.toml` or a stray `~/patches/` as project evidence. - Discovery is skipped when no in-scope purl is in an ecosystem the detector reads (an empty or pypi-only manifest on postinstall). - npm/Bun locks are read once instead of twice. - The scan message for protected packages no longer claims they were the only reason the scan came up empty. - Contract: a pruned or unshared blob means earlier Socket output reads as drift; `apply --force` is the explicit way through. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/CLI_CONTRACT.md | 5 +- .../src/commands/agent_download.rs | 11 +++- crates/socket-patch-cli/src/commands/apply.rs | 28 +++++++--- crates/socket-patch-cli/src/commands/get.rs | 4 ++ .../socket-patch-cli/src/commands/scan/mod.rs | 3 +- .../src/commands/user_patches.rs | 55 +++++++++++++++---- .../socket-patch-cli/src/commands/vendor.rs | 45 ++++++++++----- .../socket-patch-cli/tests/get/get_fixture.rs | 1 + .../tests/in_process_get_update_count.rs | 1 + .../src/patch/user_patches.rs | 44 ++++++++++++--- 10 files changed, 154 insertions(+), 43 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 5bb196004..3b948a19e 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -128,7 +128,10 @@ default. This also protects a pre-existing file where the Socket patch adds a new file (empty `beforeHash`). The exception is content a Socket patch wrote: a file whose hash names a stored `.socket/blobs/` blob (as after an update to a newer patch for the same release) is overwritten with the -`content_mismatch_overwritten` warning, except under `--strict`. +`content_mismatch_overwritten` warning, except under `--strict`. Only a stored +blob proves this: when it is gone (pruned by `gc` / `scan --prune`, or +`.socket/blobs/` is not shared with the checkout), the earlier output reads as +unknown drift and `apply --force` is the explicit way to replace it. `--allow-user-patches` permits the old warn-and-overwrite behavior: only the full, hash-verified `afterHash` content can be written, with `content_mismatch_overwritten` diagnostics. `--strict` diff --git a/crates/socket-patch-cli/src/commands/agent_download.rs b/crates/socket-patch-cli/src/commands/agent_download.rs index 9d5d044e4..e7ae38761 100644 --- a/crates/socket-patch-cli/src/commands/agent_download.rs +++ b/crates/socket-patch-cli/src/commands/agent_download.rs @@ -368,6 +368,10 @@ pub struct DownloadParams { /// fails instead of warn-and-overwrite). pub strict: bool, pub allow_user_patches: bool, + /// The caller already reported every installed package the user-patch + /// gate holds (`scan`), so the nested apply need not repeat the + /// manifest entries it skips for them. `get` checks only its selection. + pub user_patch_skips_reported: bool, /// `--ecosystems` forwarded to the nested apply, so it never touches /// other ecosystems' packages the user filtered out. pub ecosystems: Option>, @@ -1429,6 +1433,7 @@ pub(crate) fn nested_apply_args_from_params( pub(crate) async fn run_nested_apply( common: GlobalArgs, json: bool, + report_user_patch_skips: bool, client: &ApiClient, lock: LockGuard, ) -> ApplyRunReport { @@ -1438,7 +1443,10 @@ pub(crate) async fn run_nested_apply( force: false, check: false, vex: Default::default(), - nested: Some(super::apply::NestedApply { caller_json: json }), + nested: Some(super::apply::NestedApply { + caller_json: json, + report_user_patch_skips, + }), }; let report = super::apply::run_locked(apply_args, manifest_path, client, lock).await; // An error, so exempt from --silent ("errors only": a failing exit must @@ -1735,6 +1743,7 @@ pub async fn download_and_apply_patches_into( run_nested_apply( nested_apply_args_from_params(params, run, &manifest_path), params.json, + !params.user_patch_skips_reported, run.api_client, lock, ) diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index 22ac31b3b..78697c0fe 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -182,6 +182,10 @@ pub struct NestedApply { /// (one envelope per command), but the caller's stdout is, so the /// nested run's human error lines stay off stderr too. pub caller_json: bool, + /// Report the manifest entries the user-patch gate skips: the caller + /// (`get`) checked only its own selection. `scan` already reported + /// every protected installed package, so it passes `false`. + pub report_user_patch_skips: bool, } impl ApplyArgs { @@ -1245,13 +1249,22 @@ pub(crate) async fn run_locked( let mut run_warnings = run_warnings; // A nested apply's caller (get, scan) already reported the // overrides it approved; repeating them would double each line. - if args.nested.is_none() { - run_warnings.extend( - user_patch_warnings - .into_iter() - .map(|(code, detail)| RunWarning { code, detail }), - ); - } + // Skips are new to `get`, which checked only its selection; a + // JSON caller gets them through the report. + let user_patch_warnings: Vec = user_patch_warnings + .into_iter() + .filter(|(code, _)| match &args.nested { + None => true, + Some(n) => n.report_user_patch_skips && code == super::user_patches::CONFLICT, + }) + .map(|(code, detail)| RunWarning { code, detail }) + .collect(); + run_warnings.extend(user_patch_warnings.iter().cloned()); + let nested_skip_warnings = if args.nested.is_some() { + user_patch_warnings + } else { + Vec::new() + }; run_warnings.extend(results.iter().filter_map(|r| { let note = r.error.as_deref().filter(|_| r.success)?; note.contains(socket_patch_core::patch::apply::OWNERSHIP_NOT_RESTORED_MARKER) @@ -1519,6 +1532,7 @@ pub(crate) async fn run_locked( // JSON events above are the standalone apply's copy). let mut warnings = mismatch_overwrite_warnings(&results, args.common.dry_run); warnings.extend(cache_warnings); + warnings.extend(nested_skip_warnings); // A requested-but-failed VEX flips an otherwise-successful // apply to a non-zero exit (fail-the-command contract). if success && !vex_failed { diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index 808ec445d..24c1dd962 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -2107,6 +2107,7 @@ async fn save_and_apply_patch( run_nested_apply( nested_apply_args(&args.common, &manifest_path, quiet), args.common.json, + true, client, lock, ) @@ -2188,6 +2189,7 @@ fn get_download_params(args: &GetArgs, save_only: bool, persist_blobs: bool) -> all_releases: args.all_releases, strict: args.common.strict, allow_user_patches: args.common.allow_user_patches, + user_patch_skips_reported: false, ecosystems: args.common.ecosystems.clone(), persist_blobs, patch_server_url: args.common.patch_server_url.clone(), @@ -3869,6 +3871,7 @@ mod tests { all_releases: false, strict: false, allow_user_patches: false, + user_patch_skips_reported: false, ecosystems: None, persist_blobs: false, patch_server_url: None, @@ -4567,6 +4570,7 @@ mod tests { all_releases: false, strict: false, allow_user_patches: false, + user_patch_skips_reported: false, ecosystems: None, // The vendor-detached posture this fn exists for. persist_blobs: false, diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index c844484fd..16c173ee6 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -889,6 +889,7 @@ fn download_params(args: &ScanArgs, save_only: bool, json: bool, silent: bool) - all_releases: args.all_releases, strict: args.common.strict, allow_user_patches: args.common.allow_user_patches, + user_patch_skips_reported: true, ecosystems: args.common.ecosystems.clone(), persist_blobs: args.mode != Some(ScanMode::Vendored), patch_server_url: args.common.patch_server_url.clone(), @@ -2356,7 +2357,7 @@ async fn run_scan( // and neither is one whose every package the consent gate held. if user_patch_withheld > 0 { println!( - "All {user_patch_withheld} installed package(s) are protected by project patches (see the warnings above); nothing was scanned. Pass --allow-user-patches to scan them anyway." + "{user_patch_withheld} installed package(s) are protected by project patches (see the warnings above) and were not scanned. Pass --allow-user-patches to scan them anyway." ); } else if !policy.root_excluded() { println!( diff --git a/crates/socket-patch-cli/src/commands/user_patches.rs b/crates/socket-patch-cli/src/commands/user_patches.rs index 138e4d676..95baf918a 100644 --- a/crates/socket-patch-cli/src/commands/user_patches.rs +++ b/crates/socket-patch-cli/src/commands/user_patches.rs @@ -9,6 +9,21 @@ use crate::json_envelope::{Command, Envelope, EnvelopeError}; pub(crate) const CONFLICT: &str = "user_patch_conflict"; pub(crate) const ALLOWED: &str = "user_patch_override"; +/// Ecosystems the detector reads evidence for; any other purl can't conflict. +const DETECTED: &[&str] = &["pkg:npm/", "pkg:composer/", "pkg:cargo/", "pkg:golang/"]; + +/// Whether any of `purls` could match project evidence, so a run with no +/// such purl (an empty or pypi-only manifest) skips discovery entirely. +fn any_detectable<'a>(common: &GlobalArgs, mut purls: impl Iterator) -> bool { + purls.any(|purl| { + common.purl_ecosystem_selected(purl) + && DETECTED.iter().any(|prefix| { + purl.get(..prefix.len()) + .is_some_and(|p| p.eq_ignore_ascii_case(prefix)) + }) + }) +} + pub(crate) async fn inspect(common: &GlobalArgs) -> UserPatchIndex { if common.is_global() { // A local checkout does not govern a separately selected global tree. @@ -35,11 +50,32 @@ pub(crate) async fn check( common: &GlobalArgs, purls: impl IntoIterator>, ) -> Result, EnvelopeError> { + let (warnings, conflicts) = partition(common, purls).await; + if conflicts.is_empty() { + Ok(warnings) + } else { + let mut messages: Vec = conflicts.into_iter().map(|(_, m)| m).collect(); + messages.sort(); + messages.dedup(); + Err(EnvelopeError::new(CONFLICT, messages.join("\n"))) + } +} + +/// [`check`]'s two outcomes kept apart: approved-override warnings, and the +/// `(purl, message)` of each refused conflict. +pub(crate) async fn partition( + common: &GlobalArgs, + purls: impl IntoIterator>, +) -> (Vec<(String, String)>, Vec<(String, String)>) { + let purls: Vec = purls.into_iter().map(|p| p.as_ref().to_string()).collect(); + if !any_detectable(common, purls.iter().map(String::as_str)) { + return (Vec::new(), Vec::new()); + } let index = inspect(common).await; let mut warnings = Vec::new(); let mut conflicts = Vec::new(); - for purl in purls { - let purl = purl.as_ref(); + for purl in &purls { + let purl = purl.as_str(); if !common.purl_ecosystem_selected(purl) { continue; } @@ -51,16 +87,10 @@ pub(crate) async fn check( if common.allow_user_patches { warnings.push((ALLOWED.into(), message)); } else { - conflicts.push(message); + conflicts.push((purl.to_string(), message)); } } - if conflicts.is_empty() { - Ok(warnings) - } else { - conflicts.sort(); - conflicts.dedup(); - Err(EnvelopeError::new(CONFLICT, conflicts.join("\n"))) - } + (warnings, conflicts) } /// The human copy of [`check`]'s approved overrides; JSON carries them in @@ -87,9 +117,12 @@ pub(crate) async fn skip_protected( socket_patch_core::manifest::schema::PatchRecord, >, ) -> (Vec<(String, String)>, std::collections::HashSet) { - let index = inspect(common).await; let mut warnings = Vec::new(); let mut skipped = std::collections::HashSet::new(); + if !any_detectable(common, patches.keys().map(String::as_str)) { + return (warnings, skipped); + } + let index = inspect(common).await; patches.retain(|purl, _| { if !common.purl_ecosystem_selected(purl) { return true; diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index d02bbcd0c..b7ed604f6 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -2233,22 +2233,41 @@ async fn run_vendor( let (mut has_errors, ledger) = reconcile_dropped(&manifest, common, env).await; // After the reconcile: reverting entries dropped from the manifest // touches no project patch, so a protected record must not block it. - match super::user_patches::check(common, manifest.patches.keys()).await { - Ok(warnings) => { - super::user_patches::warn(common, &warnings); - env.warnings.extend( - warnings - .into_iter() - .map(|(code, detail)| crate::json_envelope::RunWarning { code, detail }), - ); - } - Err(error) => { + let (warnings, conflicts) = + super::user_patches::partition(common, manifest.patches.keys()).await; + super::user_patches::warn(common, &warnings); + env.warnings.extend( + warnings + .into_iter() + .map(|(code, detail)| crate::json_envelope::RunWarning { code, detail }), + ); + if !conflicts.is_empty() { + for (_, message) in &conflicts { if !common.json { - eprintln!("Error: {}", error.message); + eprintln!("Error: {message}"); } - env.mark_error(error); - return 1; } + if env.events.is_empty() { + let mut messages: Vec = conflicts.into_iter().map(|(_, m)| m).collect(); + messages.dedup(); + env.mark_error(crate::json_envelope::EnvelopeError::new( + super::user_patches::CONFLICT, + messages.join("\n"), + )); + } else { + // The reconcile already wrote: a top-level error would claim + // nothing happened, so the refusals are per-package failures. + for (purl, message) in conflicts { + env.record( + crate::json_envelope::PatchEvent::new( + crate::json_envelope::PatchAction::Failed, + purl.as_str(), + ) + .with_error(super::user_patches::CONFLICT, message), + ); + } + } + return 1; } let socket_dir = crate::args::socket_dir_of(manifest_path, &common.cwd); diff --git a/crates/socket-patch-cli/tests/get/get_fixture.rs b/crates/socket-patch-cli/tests/get/get_fixture.rs index 43ec126bd..d35118cea 100644 --- a/crates/socket-patch-cli/tests/get/get_fixture.rs +++ b/crates/socket-patch-cli/tests/get/get_fixture.rs @@ -294,6 +294,7 @@ pub fn engine_params(root: &Path) -> DownloadParams { silent: true, strict: false, allow_user_patches: false, + user_patch_skips_reported: false, ecosystems: None, persist_blobs: true, patch_server_url: None, diff --git a/crates/socket-patch-cli/tests/in_process_get_update_count.rs b/crates/socket-patch-cli/tests/in_process_get_update_count.rs index 1e967e87a..6c4fb8847 100644 --- a/crates/socket-patch-cli/tests/in_process_get_update_count.rs +++ b/crates/socket-patch-cli/tests/in_process_get_update_count.rs @@ -92,6 +92,7 @@ fn params(root: &Path) -> DownloadParams { silent: true, strict: false, allow_user_patches: false, + user_patch_skips_reported: false, ecosystems: None, persist_blobs: true, patch_server_url: None, diff --git a/crates/socket-patch-core/src/patch/user_patches.rs b/crates/socket-patch-core/src/patch/user_patches.rs index bce1c4542..fc0ac8c18 100644 --- a/crates/socket-patch-core/src/patch/user_patches.rs +++ b/crates/socket-patch-core/src/patch/user_patches.rs @@ -37,10 +37,15 @@ impl UserPatchIndex { 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() @@ -152,14 +157,14 @@ impl UserPatchIndex { 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. - if !index + let Some(text) = index .read(&path, "npm") .await - .is_some_and(|text| text.contains("patched")) - { + .filter(|text| text.contains("patched")) + else { continue; - } - if let Some(value) = index.json(&path, "npm").await { + }; + 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 { @@ -416,7 +421,11 @@ impl UserPatchIndex { async fn json(&mut self, path: &Path, ecosystem: &'static str) -> Option { let text = self.read(path, ecosystem).await?; - match serde_json::from_str(&crate::vendor::bun_lock_text::strip_jsonc(&text)) { + self.parse_json(&text, path, ecosystem) + } + + fn parse_json(&mut self, text: &str, path: &Path, ecosystem: &'static str) -> Option { + match serde_json::from_str(&crate::vendor::bun_lock_text::strip_jsonc(text)) { Ok(value) => Some(value), Err(e) => { self.unknown( @@ -507,6 +516,8 @@ impl UserPatchIndex { self.patch_directory(&dir.join("patches"), false).await; let flag = regex::Regex::new(r#"--patch-dir(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))"#) .unwrap(); + // `cd` as a command, not the tail of a word like `abcd`. + let cd = regex::Regex::new(r"(?:^|[\s;&|(])cd\s").unwrap(); if let Some(scripts) = value["scripts"].as_object() { for script in scripts .values() @@ -515,9 +526,9 @@ impl UserPatchIndex { { // A `cd` after patch-package (`patch-package && cd ios && pod // install`) can't move the directory it already read. - let moved = script - .find("cd ") - .is_some_and(|cd| cd < script.rfind("patch-package").unwrap_or(0)); + let moved = cd + .find(script) + .is_some_and(|cd| cd.start() < script.rfind("patch-package").unwrap_or(0)); if moved || (script.contains("--patch-dir") && !flag.is_match(script)) { self.unknown( "npm", @@ -996,6 +1007,21 @@ mod tests { assert!(!index.conflicts("pkg:npm/other@1.0.0").is_empty()); } + #[tokio::test] + async fn a_word_ending_in_cd_is_not_a_directory_change() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "package.json", + r#"{"scripts":{"postinstall":"tsc -p ./src/abcd && patch-package"}}"#, + ); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!( + index.conflicts("pkg:npm/other@1.0.0").is_empty(), + "{index:?}" + ); + } + #[tokio::test] async fn yarn_alias_patch_protects_the_real_package() { let tmp = tempfile::tempdir().unwrap(); From 1eef94046d917707f3d755211db13aa99556dbb9 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 22 Oct 2026 11:24:33 -0400 Subject: [PATCH 8/9] refactor(cli): rename consent flag to --allow-patch-conflicts Clarify that the flag accepts conflicts with existing project patches or local modifications. Rename the parser field and nested forwarding, diagnostics, contract, usage examples and existing tests consistently. --- crates/socket-patch-cli/CLI_CONTRACT.md | 16 ++++++++-------- crates/socket-patch-cli/src/args.rs | 11 ++++++----- .../src/commands/agent_download.rs | 4 ++-- crates/socket-patch-cli/src/commands/apply.rs | 8 ++++---- crates/socket-patch-cli/src/commands/get.rs | 8 ++++---- .../socket-patch-cli/src/commands/scan/hosted.rs | 2 +- crates/socket-patch-cli/src/commands/scan/mod.rs | 12 ++++++------ .../src/commands/user_patches.rs | 12 ++++++------ crates/socket-patch-cli/src/commands/vendor.rs | 2 +- .../src/commands/vendored_backend/mod.rs | 2 +- .../tests/apply/apply_network.rs | 10 +++++----- .../apply/cli_gem_variant_mismatch_policy.rs | 4 ++-- crates/socket-patch-cli/tests/cli_global_args.rs | 6 +++--- crates/socket-patch-cli/tests/cli_parse_get.rs | 6 +++--- .../socket-patch-cli/tests/cli_parse_repair.rs | 6 +++--- crates/socket-patch-cli/tests/cli_parse_vex.rs | 6 +++--- .../socket-patch-cli/tests/e2e_golang_build.rs | 6 +++--- .../tests/e2e_golang_hosted_state.rs | 2 +- .../tests/e2e_redirect_cargo_shapes.rs | 2 +- .../tests/e2e_vendor_cargo_build.rs | 4 ++-- .../tests/get/get_failure_paths.rs | 6 +++--- crates/socket-patch-cli/tests/get/get_fixture.rs | 2 +- .../tests/in_process_cargo_apply.rs | 2 +- .../tests/in_process_get_update_count.rs | 2 +- .../tests/scan/hosted_failure_paths.rs | 3 ++- .../tests/scan/scan_failure_paths.rs | 4 ++-- crates/socket-patch-cli/tests/user_patches.rs | 8 ++++---- .../tests/vendor/in_process_vendor_bun.rs | 11 +++++++---- crates/socket-patch-cli/tests/vendor_eject.rs | 4 ++-- docs/ecosystems.md | 2 +- docs/usage.md | 4 ++-- 31 files changed, 91 insertions(+), 86 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 3b948a19e..7485df1ad 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -102,8 +102,8 @@ Every subcommand accepts the same set of "global" flags via a single shared `Glo | `--vendor-url` | — | `SOCKET_VENDOR_URL` | (active API/proxy base) | string | Base host for the vendoring-service package-reference request | | `--patch-server-url` | — | `SOCKET_PATCH_SERVER_URL` | (server-returned) | string | Override the host of the prebuilt-archive download URL (local-dev / testing) | | `--offline` | — | `SOCKET_OFFLINE` | `false` | bool | **Strict airgap on every command** — never contact the network | -| `--strict` | — | `SOCKET_STRICT` | `false` | bool | Keep rejecting file-content mismatches even with `--allow-user-patches`; mismatches already fail by default without consent. `apply --force` overrides this file check, but never grants consent for declared project patches. | -| `--allow-user-patches` | — | — | `false` | bool | Explicit, invocation-only consent to attempt Socket patches alongside project patches/replacements, or overwrite modified installed files. No environment or repository-policy binding. Backend safety refusals still apply. See [Existing project patches](#existing-project-patches). | +| `--strict` | — | `SOCKET_STRICT` | `false` | bool | Keep rejecting file-content mismatches even with `--allow-patch-conflicts`; mismatches already fail by default without consent. `apply --force` overrides this file check, but never grants consent for declared project patches. | +| `--allow-patch-conflicts` | — | — | `false` | bool | Explicit, invocation-only consent to attempt Socket patches alongside project patches/replacements, or overwrite modified installed files. No environment or repository-policy binding. Backend safety refusals still apply. See [Existing project patches](#existing-project-patches). | | `--global` | `-g` | `SOCKET_GLOBAL` | `false` | bool | Operate on globally-installed packages | | `--global-prefix` | — | `SOCKET_GLOBAL_PREFIX` | (auto) | path | Override global packages root | | `--json` | `-j` | `SOCKET_JSON` | `false` | bool | Machine-readable output | @@ -132,12 +132,12 @@ newer patch for the same release) is overwritten with the blob proves this: when it is gone (pruned by `gc` / `scan --prune`, or `.socket/blobs/` is not shared with the checkout), the earlier output reads as unknown drift and `apply --force` is the explicit way to replace it. -`--allow-user-patches` permits the old +`--allow-patch-conflicts` permits the old warn-and-overwrite behavior: only the full, hash-verified `afterHash` content can be written, with `content_mismatch_overwritten` diagnostics. `--strict` keeps rejecting file drift even with that consent. `apply --force` remains explicit consent for file drift and additionally skips missing files, but a -**declared project patch still requires `--allow-user-patches`**. Overwriting +**declared project patch still requires `--allow-patch-conflicts`**. Overwriting is not a merge: the replaced local content is not kept for rollback. Vendoring commits the service artifact without merging installed changes. @@ -166,9 +166,9 @@ 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. -- `--allow-user-patches` approves the attempt for this invocation and emits +- `--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_USER_PATCHES` do not grant + `--force`, a UUID, policy settings, and `SOCKET_ALLOW_PATCH_CONFLICTS` do not grant this consent. Automatic scans never introduce a confirmation prompt. - Consent does not turn an unsupported composition into a supported one. Native Bun/npm refusals, Yarn layout restrictions, source ownership, integrity, @@ -1602,7 +1602,7 @@ command's integration tests check its output. | `vendor_artifact_redownload_failed` | `failed` | repair: download unavailable, integrity mismatch, or downloaded bytes/inventory differ from the ledger. Existing files are preserved. | | `vendor_artifact_unrepairable` | `failed` | repair: the ledger identity or patch record cannot be trusted or recovered. | | `vendor_uuid_mismatch` | `skipped` | repair: the manifest's patch uuid moved past the vendored artifact — a re-vendor (`vendor` / `scan --mode vendored`) is pending; repair does not cross patch generations. | -| `content_mismatch_overwritten` | `skipped` (warning) | apply (explicit `--allow-user-patches` / `--force`): a file matched NEITHER beforeHash nor afterHash and was overwritten with the full verified patched content. This includes a file the patch adds (empty beforeHash) that already exists with other content. `--strict` turns this case into a `failed` event instead. Agent-mode `get` / `scan --json` carry it as a `(content_mismatch_overwritten) …` `warnings[]` entry (#1004). | +| `content_mismatch_overwritten` | `skipped` (warning) | apply (explicit `--allow-patch-conflicts` / `--force`): a file matched NEITHER beforeHash nor afterHash and was overwritten with the full verified patched content. This includes a file the patch adds (empty beforeHash) that already exists with other content. `--strict` turns this case into a `failed` event instead. Agent-mode `get` / `scan --json` carry it as a `(content_mismatch_overwritten) …` `warnings[]` entry (#1004). | | `vendor_lock_checksums_unsupported` / `vendor_stale_lock_checksum` | `failed` | vendor (gem): an ambiguous/platform CHECKSUMS entry, or a v1-wired lock whose stale token blocks the hot path (run `vendor --revert` + re-vendor). | | `redirect_unattributable` | `skipped` (hosted event) | scan/get `--mode hosted`: the rewriters would pin the patch, but lockfile discovery over the result reads that pin as contested (another lock or requirements file resolves the same version elsewhere, or the pin is not one the package manager consumes), so `vex`, `rollback`, `remove` and `vendor` would refuse it. The candidate is left out of the rewrite, so nothing is written for it; the detail carries discovery's findings. Exit code unchanged. | | `redirect_copy_unpatched` | `warnings[]` (warning) | scan/get `--mode hosted` (pnpm, yarn): the pin was written, but its own lock also installs an unpatched copy of the same `name@version`, which no rewrite reaches: a pnpm `file:` directory or tarball dependency (#935), or a yarn `file:` tarball / directory, URL tarball or berry `portal:` / `link:` dependency declared under another name (`lp2@file:…`, #939, #1236; not repeated for a copy a yarn classic skip warning already names). The detail names the lock entry. Neither the run's own `--vex` nor a later `vex` attests the package (`vex` warns `patched_ref_unattributable`), but discovery still manages the pin, so `rollback` / `remove` undo it. | @@ -1675,7 +1675,7 @@ project-patch consent gate; `user_patch_override` records explicit consent. | Code | Subcommands | Meaning | |-----------------------|----------------------------------|---------| -| `user_patch_conflict` | get, vendor; hosted/vendored write rechecks | A selected package has project-owned patch/replacement evidence and no `--allow-user-patches` consent. Evidence paths and a remedy are included; exit 1. (`scan` and `apply` report it as a warning and skip the package.) | +| `user_patch_conflict` | get, vendor; hosted/vendored write rechecks | A selected package has project-owned patch/replacement evidence and no `--allow-patch-conflicts` consent. Evidence paths and a remedy are included; exit 1. (`scan` and `apply` report it as a warning and skip the package.) | | `manifest_not_found` | remove, repair, rollback, vex, apply (only when the manifest vanishes between apply's existence probe and its locked read) (not `list` since v5.0: a missing manifest is an empty list) | `.socket/manifest.json` doesn't exist. For `vex` (and `scan --vex`) it fires only when, in addition, NOTHING else names a patch — no vendor-ledger entry, no lockfile reference (hosted or vendored) — and the message says so (exit 2 standalone; `apply`/`vendor --vex` treat it as their calm no-op). v3.5: `repair` proceeds anyway (vendored phase only) when a vendor ledger or vendor-path lockfile references exist, and exits 0 with a `redirect_only_project` skip (not this error) when the project's only patch state is hosted pins in its lockfiles (v5.0; or a pre-v5 `redirect-state.json`). `list` likewise no longer fires this on a hosted-only project: v5.0 lists every hosted pin the lockfiles wire (exit 0, labeled `details.mode: "hosted"` + `details.lockfiles: []` — no `details.ledger`, since hosted mode keeps none; when the manifest exists too, both are shown, purl-sorted with the manifest entry first on a tie). A pin carries its uuid and empty details unless a pre-v5 redirect ledger records the same purl and uuid (read for migration only: its record supplies the vulnerabilities / tier / description); a pre-v5 ledger record whose pin is in no lockfile is not listed. v5.0: `list` reads the vendor ledger the same way — a vendored-only project (every `scan`/`get --mode vendored` project) lists its ledger entries' embedded records labeled `Mode: vendored (recorded in .socket/vendor/state.json)` in human mode — the twin of the hosted `Mode: hosted (wired in )` line — (`details.mode: "vendored"` + `details.ledger: ".socket/vendor/state.json"` in JSON), exit 0. A standalone-`vendor` entry's fallback `record` lists the same way once no manifest entry covers it (by ledger key or base purl) — the copy manifest-less `vex` attests from, so `list` never reports `manifest_not_found` for a tree whose VEX document attests a patch; while the manifest covers it, only the manifest entry is listed. All sources always come from the SAME project (see the `--manifest-path` flag row), so `--manifest-path` into another project reads that project's state, never the local one. The error still fires when NONE of the three sources has a patch, and a present-but-broken manifest still reports `manifest_invalid`/`manifest_unreadable` regardless (corruption is never masked). A malformed pre-v5 redirect ledger degrades to "nothing to consult" with a stderr warning, muted by `--silent` (the pins still list); `list --json` carries it in the run-level `warnings[]` as `redirect_ledger_corrupt` instead of on stderr. v5.0: `rollback` likewise proceeds manifest-less when the vendor ledger or the lockfiles' hosted pins hold work (its error is the legacy `{status: "error", error: {code: "manifest_not_found", message: "Manifest not found"}, path}` shape); only the truly-empty project — no manifest, no vendor ledger, no hosted pin (a lone pre-v5 redirect ledger is deleted, exit 0) — keeps the exit-1 error, and a project whose lockfiles still reference `.socket/vendor/` artifacts with NO vendor ledger gets a distinct error naming `socket-patch repair`. `remove` (v5.0) proceeds manifest-less whenever a vendor ledger file exists or the lockfiles pin a hosted patch (an existence probe and the read-only hosted-pin discovery before the lock; the vendor ledger loads under it): ANY vendor-ledger entry matching the identifier — detached or not — is removed through the ledger path (`--preserve-state` and drift-keeps behave exactly as on the manifest path), a hosted-only match restores its upstream registry entry, and when that state exists but holds nothing for the identifier the error is `not_found` (exit 1), not this code — `manifest_not_found` fires from `remove` only when all three sources are empty. Manifest entries are removed in sorted purl order. | | `ambiguous_target` | remove, get, rollback | The identifier is a package name whose last-segment rule selects several packages across the manifest, the vendor ledger and the hosted pins (`core` → `@angular/core` and `@babel/core`). Nothing is changed (exit 1); the message names each package. Use the full name or a purl. See **Target grammar**. | | `manifest_invalid` | list, remove, apply (incl. `--check`), repair, vendor (incl. `--check`), vex, rollback, get, scan (agent mode) | Manifest exists but is unparseable (malformed JSON or a schema violation). **v5.0, MAJOR (#931)**: one mapping on every command — `apply` used to say `apply_failed`, `repair` `repair_failed`, `vendor` the undocumented `invalid_manifest`, and `apply --check` / `vendor --check` / `vex` / `get` / `rollback` `manifest_unreadable`. Exit codes unchanged (`vex`: 2, every other command 1). Agent-mode `scan` fails with it (exit 1); the other scan modes, which don't read the manifest, add a warning with the same code and go on. | diff --git a/crates/socket-patch-cli/src/args.rs b/crates/socket-patch-cli/src/args.rs index b28a082dd..f3a1b5659 100644 --- a/crates/socket-patch-cli/src/args.rs +++ b/crates/socket-patch-cli/src/args.rs @@ -180,7 +180,7 @@ pub struct GlobalArgs { )] pub offline: bool, - /// Reject modified file content even with --allow-user-patches. Without + /// Reject modified file content even with --allow-patch-conflicts. Without /// explicit consent, mismatches already fail by default. On commands that /// have --force, --force overrides this file-content check (but never the /// separate check for declared project patches). @@ -193,12 +193,13 @@ pub struct GlobalArgs { )] pub strict: bool, - /// Explicitly permit attempting patches on packages with project patches or - /// local modifications. Review and test the combined result; backend safety + /// Accept the risk of conflicts with existing project patches or local + /// modifications when applying Socket patches. Modified files may be + /// overwritten; patches are not merged automatically. Backend safety /// refusals still apply. --yes does not approve patch composition; --force /// approves only file-content drift. #[arg(help_heading = GLOBAL_OPTIONS, long)] - pub allow_user_patches: bool, + pub allow_patch_conflicts: bool, /// Operate on globally-installed packages. #[arg( @@ -785,7 +786,7 @@ impl Default for GlobalArgs { patch_server_url: None, offline: false, strict: false, - allow_user_patches: false, + allow_patch_conflicts: false, global: false, global_prefix: None, json: false, diff --git a/crates/socket-patch-cli/src/commands/agent_download.rs b/crates/socket-patch-cli/src/commands/agent_download.rs index e7ae38761..eddace731 100644 --- a/crates/socket-patch-cli/src/commands/agent_download.rs +++ b/crates/socket-patch-cli/src/commands/agent_download.rs @@ -367,7 +367,7 @@ pub struct DownloadParams { /// `--strict` forwarded to the nested apply (a beforeHash mismatch /// fails instead of warn-and-overwrite). pub strict: bool, - pub allow_user_patches: bool, + pub allow_patch_conflicts: bool, /// The caller already reported every installed package the user-patch /// gate holds (`scan`), so the nested apply need not repeat the /// manifest entries it skips for them. `get` checks only its selection. @@ -1406,7 +1406,7 @@ pub(crate) fn nested_apply_args_from_params( global: params.global, global_prefix: params.global_prefix.clone(), strict: params.strict, - allow_user_patches: params.allow_user_patches, + allow_patch_conflicts: params.allow_patch_conflicts, // Scope the nested apply like the caller was scoped: `None` would // apply the WHOLE manifest, mutating other ecosystems' packages the // user filtered out. diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index 78697c0fe..9a97a4f09 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -123,13 +123,13 @@ fn mismatch_overwrite_warnings(results: &[ApplyResult], dry_run: bool) -> Vec MismatchPolicy { +/// separate gate that requires --allow-patch-conflicts even with --force. +fn mismatch_policy(force: bool, strict: bool, allow_patch_conflicts: bool) -> MismatchPolicy { if force { MismatchPolicy::Force } else if strict { MismatchPolicy::Strict - } else if allow_user_patches { + } else if allow_patch_conflicts { MismatchPolicy::Warn } else { MismatchPolicy::SocketContent @@ -2192,7 +2192,7 @@ async fn apply_patches_inner( let policy = mismatch_policy( args.force, args.common.strict, - args.common.allow_user_patches, + args.common.allow_patch_conflicts, ); let mut has_errors = false; diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index 24c1dd962..49a45cdc0 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -2188,7 +2188,7 @@ fn get_download_params(args: &GetArgs, save_only: bool, persist_blobs: bool) -> silent: args.common.silent, all_releases: args.all_releases, strict: args.common.strict, - allow_user_patches: args.common.allow_user_patches, + allow_patch_conflicts: args.common.allow_patch_conflicts, user_patch_skips_reported: false, ecosystems: args.common.ecosystems.clone(), persist_blobs, @@ -3870,7 +3870,7 @@ mod tests { silent: true, all_releases: false, strict: false, - allow_user_patches: false, + allow_patch_conflicts: false, user_patch_skips_reported: false, ecosystems: None, persist_blobs: false, @@ -4569,7 +4569,7 @@ mod tests { silent: true, all_releases: false, strict: false, - allow_user_patches: false, + allow_patch_conflicts: false, user_patch_skips_reported: false, ecosystems: None, // The vendor-detached posture this fn exists for. @@ -5648,7 +5648,7 @@ mod tests { let common = GlobalArgs { verbose: true, strict: true, - allow_user_patches: false, + allow_patch_conflicts: false, json: true, dry_run: true, ..GlobalArgs::default() diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index a3e47ca5c..e309b0252 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -1043,7 +1043,7 @@ async fn run_redirect_selected_untracked( // Re-read project patch declarations under the write lock. A patch added // during API selection must not be lost in a takeover or lock rewrite. - if !common.allow_user_patches { + if !common.allow_patch_conflicts { if let Err(error) = crate::commands::user_patches::check(common, candidates.iter().map(&sel_purl)).await { diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index 16c173ee6..62ed89116 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -888,7 +888,7 @@ fn download_params(args: &ScanArgs, save_only: bool, json: bool, silent: bool) - silent, all_releases: args.all_releases, strict: args.common.strict, - allow_user_patches: args.common.allow_user_patches, + allow_patch_conflicts: args.common.allow_patch_conflicts, user_patch_skips_reported: true, ecosystems: args.common.ecosystems.clone(), persist_blobs: args.mode != Some(ScanMode::Vendored), @@ -2232,19 +2232,19 @@ async fn run_scan( if sources.is_empty() { return true; } - if !args.common.allow_user_patches { + if !args.common.allow_patch_conflicts { user_patch_withheld += 1; } layout_refusals.push(( - if args.common.allow_user_patches { + if args.common.allow_patch_conflicts { super::user_patches::ALLOWED } else { super::user_patches::CONFLICT } .into(), - super::user_patches::detail(&package.purl, &sources, args.common.allow_user_patches), + super::user_patches::detail(&package.purl, &sources, args.common.allow_patch_conflicts), )); - args.common.allow_user_patches + args.common.allow_patch_conflicts }); // Gradle discovery notes (m2 gating, the user home) ride the run-level @@ -2357,7 +2357,7 @@ async fn run_scan( // and neither is one whose every package the consent gate held. if user_patch_withheld > 0 { println!( - "{user_patch_withheld} installed package(s) are protected by project patches (see the warnings above) and were not scanned. Pass --allow-user-patches to scan them anyway." + "{user_patch_withheld} installed package(s) are protected by project patches (see the warnings above) and were not scanned. Pass --allow-patch-conflicts to scan them anyway." ); } else if !policy.root_excluded() { println!( diff --git a/crates/socket-patch-cli/src/commands/user_patches.rs b/crates/socket-patch-cli/src/commands/user_patches.rs index 95baf918a..596d440dc 100644 --- a/crates/socket-patch-cli/src/commands/user_patches.rs +++ b/crates/socket-patch-cli/src/commands/user_patches.rs @@ -36,9 +36,9 @@ pub(crate) async fn inspect(common: &GlobalArgs) -> UserPatchIndex { pub(crate) fn detail(purl: &str, sources: &[String], allowed: bool) -> String { let sources = sources.join(", "); if allowed { - format!("{purl}: --allow-user-patches permits attempting a Socket patch alongside project patches in {sources}; review and test the combined result. Backend safety checks still apply") + format!("{purl}: --allow-patch-conflicts permits attempting a Socket patch alongside project patches in {sources}; review and test the combined result. Backend safety checks still apply") } else { - format!("{purl}: protected by project patch/replacement metadata in {sources}; leaving this package untouched. Merge the fixes manually, or explicitly accept the conflict risk with --allow-user-patches (ordinary --yes/--force do not approve this)") + format!("{purl}: protected by project patch/replacement metadata in {sources}; leaving this package untouched. Merge the fixes manually, or explicitly accept the conflict risk with --allow-patch-conflicts (ordinary --yes/--force do not approve this)") } } @@ -83,8 +83,8 @@ pub(crate) async fn partition( if sources.is_empty() { continue; } - let message = detail(purl, &sources, common.allow_user_patches); - if common.allow_user_patches { + let message = detail(purl, &sources, common.allow_patch_conflicts); + if common.allow_patch_conflicts { warnings.push((ALLOWED.into(), message)); } else { conflicts.push((purl.to_string(), message)); @@ -131,8 +131,8 @@ pub(crate) async fn skip_protected( if sources.is_empty() { return true; } - let message = detail(purl, &sources, common.allow_user_patches); - if common.allow_user_patches { + let message = detail(purl, &sources, common.allow_patch_conflicts); + if common.allow_patch_conflicts { warnings.push((ALLOWED.into(), message)); true } else { diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index b7ed604f6..6c78a086b 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1949,7 +1949,7 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { return emit_eject_refusal(common, code, &message); } }; - if !common.allow_user_patches { + if !common.allow_patch_conflicts { if let Err(error) = super::user_patches::check(common, pins.iter().map(|p| &p.purl)).await { return super::user_patches::refuse(common, Command::Vendor, error); } diff --git a/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs b/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs index 5132f8ea4..541db6540 100644 --- a/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs +++ b/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs @@ -93,7 +93,7 @@ impl<'a> VendoredBackend<'a> { /// `scan_run_fits_windows_main_thread_stack`). pub(crate) async fn apply(&self, req: ApplyRequest<'_>, env: &mut Envelope) -> bool { let common: &GlobalArgs = &self.common; - if !common.allow_user_patches { + if !common.allow_patch_conflicts { if let Err(error) = super::user_patches::check(common, req.manifest.patches.keys()).await { diff --git a/crates/socket-patch-cli/tests/apply/apply_network.rs b/crates/socket-patch-cli/tests/apply/apply_network.rs index 1027b076f..4878b1639 100644 --- a/crates/socket-patch-cli/tests/apply/apply_network.rs +++ b/crates/socket-patch-cli/tests/apply/apply_network.rs @@ -465,7 +465,7 @@ async fn apply_hash_mismatch_opt_in_warns_and_applies_strict_fails() { // content (the blob is hash-gated to afterHash) and surfaced as a warning event — exit 0. let tmp = fixture(); let out = hermetic::command(&binary()) - .args(["apply", "--allow-user-patches", "--json", "--offline"]) + .args(["apply", "--allow-patch-conflicts", "--json", "--offline"]) .current_dir(tmp.path()) .output() .expect("run socket-patch"); @@ -497,7 +497,7 @@ async fn apply_hash_mismatch_opt_in_warns_and_applies_strict_fails() { // The human run logs the warning to stderr. let tmp = fixture(); let out = hermetic::command(&binary()) - .args(["apply", "--allow-user-patches", "--offline", "--yes"]) + .args(["apply", "--allow-patch-conflicts", "--offline", "--yes"]) .current_dir(tmp.path()) .output() .expect("run socket-patch"); @@ -513,7 +513,7 @@ async fn apply_hash_mismatch_opt_in_warns_and_applies_strict_fails() { let out = hermetic::command(&binary()) .args([ "apply", - "--allow-user-patches", + "--allow-patch-conflicts", "--json", "--offline", "--strict", @@ -755,7 +755,7 @@ async fn apply_mismatch_redownloads_full_blob_and_applies() { .unwrap(); } - let (code, stdout, stderr) = run_apply(tmp.path(), &mock.uri(), &["--allow-user-patches"]); + let (code, stdout, stderr) = run_apply(tmp.path(), &mock.uri(), &["--allow-patch-conflicts"]); let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON"); assert_eq!(code, 0, "stdout={v:#}\nstderr={stderr}"); let events = v["events"].as_array().expect("events array"); @@ -988,7 +988,7 @@ async fn mismatch_overwrites_every_copy_of_a_duplicated_package() { let socket = tmp.path().join(".socket"); write_manifest_with_patch(&socket, purl, uuid, &before_hash, &after_hash); - let (code, stdout, stderr) = run_apply(tmp.path(), &mock.uri(), &["--allow-user-patches"]); + let (code, stdout, stderr) = run_apply(tmp.path(), &mock.uri(), &["--allow-patch-conflicts"]); let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON"); assert_eq!( code, 0, diff --git a/crates/socket-patch-cli/tests/apply/cli_gem_variant_mismatch_policy.rs b/crates/socket-patch-cli/tests/apply/cli_gem_variant_mismatch_policy.rs index c9e4b170c..f66877317 100644 --- a/crates/socket-patch-cli/tests/apply/cli_gem_variant_mismatch_policy.rs +++ b/crates/socket-patch-cli/tests/apply/cli_gem_variant_mismatch_policy.rs @@ -151,7 +151,7 @@ fn singleton_mismatch_opt_in_warns_and_applies() { let tmp = tempfile::tempdir().expect("tempdir"); let (file, patched) = singleton_fixture(tmp.path()); - let (code, _stdout, stderr) = run_apply(tmp.path(), &["--allow-user-patches"]); + let (code, _stdout, stderr) = run_apply(tmp.path(), &["--allow-patch-conflicts"]); assert_eq!( code, 0, "opted-in mismatch on a singleton variant is a warning, not an error; stderr={stderr}" @@ -178,7 +178,7 @@ fn singleton_mismatch_opt_in_warns_and_applies() { // warning event — the same shape apply_network pins for npm. let tmp = tempfile::tempdir().expect("tempdir"); let (file, patched) = singleton_fixture(tmp.path()); - let (code, stdout, _stderr) = run_apply(tmp.path(), &["--json", "--allow-user-patches"]); + let (code, stdout, _stderr) = run_apply(tmp.path(), &["--json", "--allow-patch-conflicts"]); assert_eq!(code, 0, "json run must also succeed; stdout={stdout}"); assert_eq!(std::fs::read(&file).expect("read patched file"), patched); let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON envelope"); diff --git a/crates/socket-patch-cli/tests/cli_global_args.rs b/crates/socket-patch-cli/tests/cli_global_args.rs index a84cdbe07..e2b69c8d9 100644 --- a/crates/socket-patch-cli/tests/cli_global_args.rs +++ b/crates/socket-patch-cli/tests/cli_global_args.rs @@ -101,8 +101,8 @@ fn global_flag_cases() -> Vec<(&'static str, Option<&'static str>, fn(&GlobalArg }), ("--offline", None, |c| assert!(c.offline)), ("--strict", None, |c| assert!(c.strict)), - ("--allow-user-patches", None, |c| { - assert!(c.allow_user_patches) + ("--allow-patch-conflicts", None, |c| { + assert!(c.allow_patch_conflicts) }), ("--global", None, |c| assert!(c.global)), ("--global-prefix", Some("/opt/global"), |c| { @@ -239,7 +239,7 @@ fn global_flag_cases_cover_every_global_field() { no_npm_allow_remote_config: _, no_vlt_install_cleanup: _, strict: _, - allow_user_patches: _, + allow_patch_conflicts: _, vendor_source: _, vendor_url: _, patch_server_url: _, diff --git a/crates/socket-patch-cli/tests/cli_parse_get.rs b/crates/socket-patch-cli/tests/cli_parse_get.rs index 0f52dfa21..f856580cc 100644 --- a/crates/socket-patch-cli/tests/cli_parse_get.rs +++ b/crates/socket-patch-cli/tests/cli_parse_get.rs @@ -136,7 +136,7 @@ struct Snap { patch_server_url: Option, offline: bool, strict: bool, - allow_user_patches: bool, + allow_patch_conflicts: bool, global: bool, global_prefix: Option, json: bool, @@ -171,7 +171,7 @@ fn snapshot(a: &GetArgs) -> Snap { patch_server_url: a.common.patch_server_url.clone(), offline: a.common.offline, strict: a.common.strict, - allow_user_patches: a.common.allow_user_patches, + allow_patch_conflicts: a.common.allow_patch_conflicts, global: a.common.global, global_prefix: a.common.global_prefix.clone(), json: a.common.json, @@ -214,7 +214,7 @@ fn expected_defaults(identifier: &str) -> Snap { patch_server_url: None, offline: false, strict: false, - allow_user_patches: false, + allow_patch_conflicts: false, global: false, global_prefix: None, json: false, diff --git a/crates/socket-patch-cli/tests/cli_parse_repair.rs b/crates/socket-patch-cli/tests/cli_parse_repair.rs index 454cedc33..554dd42bb 100644 --- a/crates/socket-patch-cli/tests/cli_parse_repair.rs +++ b/crates/socket-patch-cli/tests/cli_parse_repair.rs @@ -132,7 +132,7 @@ struct Snap { patch_server_url: Option, offline: bool, strict: bool, - allow_user_patches: bool, + allow_patch_conflicts: bool, global: bool, global_prefix: Option, json: bool, @@ -160,7 +160,7 @@ fn snapshot(a: &RepairArgs) -> Snap { patch_server_url: a.common.patch_server_url.clone(), offline: a.common.offline, strict: a.common.strict, - allow_user_patches: a.common.allow_user_patches, + allow_patch_conflicts: a.common.allow_patch_conflicts, global: a.common.global, global_prefix: a.common.global_prefix.clone(), json: a.common.json, @@ -195,7 +195,7 @@ fn expected_defaults() -> Snap { patch_server_url: None, offline: false, strict: false, - allow_user_patches: false, + allow_patch_conflicts: false, global: false, global_prefix: None, json: false, diff --git a/crates/socket-patch-cli/tests/cli_parse_vex.rs b/crates/socket-patch-cli/tests/cli_parse_vex.rs index 7dc3c7451..500314716 100644 --- a/crates/socket-patch-cli/tests/cli_parse_vex.rs +++ b/crates/socket-patch-cli/tests/cli_parse_vex.rs @@ -205,7 +205,7 @@ struct Snap { patch_server_url: Option, offline: bool, strict: bool, - allow_user_patches: bool, + allow_patch_conflicts: bool, global: bool, global_prefix: Option, json: bool, @@ -240,7 +240,7 @@ fn snapshot(a: &VexArgs) -> Snap { patch_server_url: a.common.patch_server_url.clone(), offline: a.common.offline, strict: a.common.strict, - allow_user_patches: a.common.allow_user_patches, + allow_patch_conflicts: a.common.allow_patch_conflicts, global: a.common.global, global_prefix: a.common.global_prefix.clone(), json: a.common.json, @@ -282,7 +282,7 @@ fn expected_defaults() -> Snap { patch_server_url: None, offline: false, strict: false, - allow_user_patches: false, + allow_patch_conflicts: false, global: false, global_prefix: None, json: false, diff --git a/crates/socket-patch-cli/tests/e2e_golang_build.rs b/crates/socket-patch-cli/tests/e2e_golang_build.rs index 4c6f2384b..ed0c08425 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -720,14 +720,14 @@ fn apply_json(dir: &Path, modcache: &Path) -> (i32, String, String) { ) } -/// [`apply_json`] with `--allow-user-patches`, for guards behind that gate. +/// [`apply_json`] with `--allow-patch-conflicts`, for guards behind that gate. fn apply_json_consented(dir: &Path, modcache: &Path) -> (i32, String, String) { run_socket( dir, &[ "apply", "--offline", - "--allow-user-patches", + "--allow-patch-conflicts", "--ecosystems", "golang", "--json", @@ -819,7 +819,7 @@ fn go_work_replace_overrides_the_socket_replace() { "apply", "--offline", "--dry-run", - "--allow-user-patches", + "--allow-patch-conflicts", "--ecosystems", "golang", "--json", diff --git a/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs b/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs index fd333e516..4336118f4 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs @@ -211,7 +211,7 @@ async fn refused_go_rewrite_is_not_confirmed_by_another_lockfile() { &consumer, &server, &tmp.path().join("modcache"), - &["--allow-user-patches"], + &["--allow-patch-conflicts"], ); assert_eq!(hosted_pinned(&env), 0, "envelope: {env}"); assert!( diff --git a/crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs b/crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs index 966d2bfba..63197e926 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs @@ -552,7 +552,7 @@ async fn run_shape(shape: Shape) -> Option<()> { "--no-telemetry", // Consent past the user-patch gate so the shapes reach the // redirect guards; tests/user_patches.rs covers the gate itself. - "--allow-user-patches", + "--allow-patch-conflicts", "--cwd", &proj_s, "--api-url", diff --git a/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs index 33915132a..7b19e468f 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs @@ -2653,7 +2653,7 @@ fn cargo_url_spelled_crates_io_patch_table_is_refused() { "vendor", "--json", "--offline", - "--allow-user-patches", + "--allow-patch-conflicts", "--cwd", proj.to_str().unwrap(), ], @@ -2735,7 +2735,7 @@ fn cargo_ancestor_config_patch_cannot_silently_shadow_the_vendored_copy() { "vendor", "--json", "--offline", - "--allow-user-patches", + "--allow-patch-conflicts", "--cwd", proj.to_str().unwrap(), ], diff --git a/crates/socket-patch-cli/tests/get/get_failure_paths.rs b/crates/socket-patch-cli/tests/get/get_failure_paths.rs index 5372dd655..7ca1190b3 100644 --- a/crates/socket-patch-cli/tests/get/get_failure_paths.rs +++ b/crates/socket-patch-cli/tests/get/get_failure_paths.rs @@ -1644,7 +1644,7 @@ async fn engine_nested_apply_mismatch_overwrite_reaches_the_json_envelope() { std::fs::write(&installed, LOCAL_EDIT_BYTES).unwrap(); let mut params = engine_params(tmp.path()); params.save_only = false; - params.allow_user_patches = true; + params.allow_patch_conflicts = true; let selected = vec![search_result(UUID, PURL)]; let (code, json) = download_and_apply_patches(&selected, ¶ms, &server.uri()).await; @@ -1691,7 +1691,7 @@ async fn get_uuid_json_mismatch_overwrite_is_reported() { let (code, stdout, stderr) = run_get_bin( tmp.path(), &server.uri(), - &[UUID, "--json", "--allow-user-patches"], + &[UUID, "--json", "--allow-patch-conflicts"], ); assert_eq!(code, 0, "stdout={stdout}\nstderr={stderr}"); let json = parse_json_envelope(&stdout); @@ -1748,7 +1748,7 @@ async fn get_uuid_forwards_explicit_consent_to_nested_apply() { let (code, stdout, stderr) = run_get_bin( tmp.path(), &server.uri(), - &[UUID, "--json", "--mode", "agent", "--allow-user-patches"], + &[UUID, "--json", "--mode", "agent", "--allow-patch-conflicts"], ); assert_eq!(code, 0, "{stdout} {stderr}"); assert_eq!( diff --git a/crates/socket-patch-cli/tests/get/get_fixture.rs b/crates/socket-patch-cli/tests/get/get_fixture.rs index d35118cea..6d4d57cfc 100644 --- a/crates/socket-patch-cli/tests/get/get_fixture.rs +++ b/crates/socket-patch-cli/tests/get/get_fixture.rs @@ -293,7 +293,7 @@ pub fn engine_params(root: &Path) -> DownloadParams { json: true, silent: true, strict: false, - allow_user_patches: false, + allow_patch_conflicts: false, user_patch_skips_reported: false, ecosystems: None, persist_blobs: true, diff --git a/crates/socket-patch-cli/tests/in_process_cargo_apply.rs b/crates/socket-patch-cli/tests/in_process_cargo_apply.rs index ff3a1defd..98463055b 100644 --- a/crates/socket-patch-cli/tests/in_process_cargo_apply.rs +++ b/crates/socket-patch-cli/tests/in_process_cargo_apply.rs @@ -347,7 +347,7 @@ async fn cargo_apply_refuses_on_before_hash_mismatch() { // --force) deliberately downgrade a hash mismatch to "ready" // and the file WOULD be overwritten with verified content. strict: true, - allow_user_patches: false, + allow_patch_conflicts: false, ..socket_patch_cli::args::GlobalArgs::default() }, batch_size: Some(100), diff --git a/crates/socket-patch-cli/tests/in_process_get_update_count.rs b/crates/socket-patch-cli/tests/in_process_get_update_count.rs index 6c4fb8847..82dd0c7dd 100644 --- a/crates/socket-patch-cli/tests/in_process_get_update_count.rs +++ b/crates/socket-patch-cli/tests/in_process_get_update_count.rs @@ -91,7 +91,7 @@ fn params(root: &Path) -> DownloadParams { json: true, silent: true, strict: false, - allow_user_patches: false, + allow_patch_conflicts: false, user_patch_skips_reported: false, ecosystems: None, persist_blobs: true, diff --git a/crates/socket-patch-cli/tests/scan/hosted_failure_paths.rs b/crates/socket-patch-cli/tests/scan/hosted_failure_paths.rs index a4b9e779d..9137d82e6 100644 --- a/crates/socket-patch-cli/tests/scan/hosted_failure_paths.rs +++ b/crates/socket-patch-cli/tests/scan/hosted_failure_paths.rs @@ -1208,7 +1208,8 @@ async fn unreadable_pnpm_workspace_gets_warning_only_guidance_in_a_live_run() { return; } - let (code, doc) = scan_hosted_json(tmp.path(), &server.uri(), &["--allow-user-patches"], &[]); + let (code, doc) = + scan_hosted_json(tmp.path(), &server.uri(), &["--allow-patch-conflicts"], &[]); // Restore before asserting so the tempdir cleans up and the bytes can // be read back. std::fs::set_permissions(&ws, std::fs::Permissions::from_mode(0o644)).unwrap(); diff --git a/crates/socket-patch-cli/tests/scan/scan_failure_paths.rs b/crates/socket-patch-cli/tests/scan/scan_failure_paths.rs index 758bf58dd..aafeb160f 100644 --- a/crates/socket-patch-cli/tests/scan/scan_failure_paths.rs +++ b/crates/socket-patch-cli/tests/scan/scan_failure_paths.rs @@ -576,7 +576,7 @@ async fn scan_agent_json_mismatch_overwrite_reaches_the_apply_block() { let (code, v) = run_json( tmp.path(), &mock.uri(), - &["--mode", "agent", "--allow-user-patches"], + &["--mode", "agent", "--allow-patch-conflicts"], ); assert_eq!(code, 0, "{v:#}"); assert_eq!(v["summary"]["applied"], 1, "{v}"); @@ -908,7 +908,7 @@ async fn scan_still_patches_unrelated_packages_and_forwards_explicit_consent() { .unwrap(); let mut args = vec!["--mode", "agent"]; if allowed { - args.push("--allow-user-patches"); + args.push("--allow-patch-conflicts"); } let (code, json) = run_json(tmp.path(), &mock.uri(), &args); assert_eq!(code, 0, "{json}"); diff --git a/crates/socket-patch-cli/tests/user_patches.rs b/crates/socket-patch-cli/tests/user_patches.rs index 8e6b5ce69..c79dcf57d 100644 --- a/crates/socket-patch-cli/tests/user_patches.rs +++ b/crates/socket-patch-cli/tests/user_patches.rs @@ -91,7 +91,7 @@ fn apply_skips_and_vendor_refuses_declared_patches_despite_matching_hashes_or_ge assert_eq!(value["error"]["code"], "user_patch_conflict", "{value}"); value["error"]["message"].as_str().unwrap().to_string() }; - assert!(message.contains("--allow-user-patches"), "{message}"); + assert!(message.contains("--allow-patch-conflicts"), "{message}"); assert_eq!( std::fs::read(tmp.path().join("node_modules/example/index.js")).unwrap(), BEFORE @@ -116,7 +116,7 @@ fn consent_allows_agent_application_and_does_not_modify_the_user_patch() { write(tmp.path(), "patches/example+1.0.0.patch", b"user diff\n"); let (code, stdout, stderr) = run_with_env( tmp.path(), - &["apply", "--json", "--offline", "--allow-user-patches"], + &["apply", "--json", "--offline", "--allow-patch-conflicts"], &[], ); assert_eq!(code, 0, "{stdout} {stderr}"); @@ -147,7 +147,7 @@ fn unknown_local_edits_fail_closed_by_default_including_yes_silent_and_dry_run() vec!["--yes"], vec!["--silent"], vec!["--dry-run"], - vec!["--allow-user-patches", "--strict"], + vec!["--allow-patch-conflicts", "--strict"], ] { let tmp = fixture(); write(tmp.path(), "node_modules/example/index.js", b"local edit\n"); @@ -194,7 +194,7 @@ fn environment_and_read_only_checks_do_not_grant_or_require_consent() { let (code, stdout, _) = run_with_env( tmp.path(), &["apply", "--json", "--offline"], - &[("SOCKET_ALLOW_USER_PATCHES", "1")], + &[("SOCKET_ALLOW_PATCH_CONFLICTS", "1")], ); assert_eq!(code, 0, "{stdout}"); assert!(stdout.contains("user_patch_conflict")); diff --git a/crates/socket-patch-cli/tests/vendor/in_process_vendor_bun.rs b/crates/socket-patch-cli/tests/vendor/in_process_vendor_bun.rs index 32d9b2934..38c592217 100644 --- a/crates/socket-patch-cli/tests/vendor/in_process_vendor_bun.rs +++ b/crates/socket-patch-cli/tests/vendor/in_process_vendor_bun.rs @@ -506,8 +506,11 @@ async fn assert_scan_refuses(shape: LockShape, code: &str) { Vec::new() }; - let (exit, stdout, stderr) = - scan_vendored(tmp.path(), &mock.uri(), &["--json", "--allow-user-patches"]); + let (exit, stdout, stderr) = scan_vendored( + tmp.path(), + &mock.uri(), + &["--json", "--allow-patch-conflicts"], + ); assert_eq!(exit, 1, "{shape:?}: stdout={stdout}\nstderr={stderr}"); let v = parse_single_json_doc(&stdout); assert_eq!(v["status"], "partialFailure", "{shape:?}: {v}"); @@ -1684,7 +1687,7 @@ async fn fifo_bun_lock_is_refused_without_blocking() { "--vendor-source", "service", "--json", - "--allow-user-patches", + "--allow-patch-conflicts", ], &uri, ); @@ -1708,7 +1711,7 @@ async fn fifo_bun_lock_is_refused_without_blocking() { "--vendor-source", "service", "--json", - "--allow-user-patches", + "--allow-patch-conflicts", ], &uri, ); diff --git a/crates/socket-patch-cli/tests/vendor_eject.rs b/crates/socket-patch-cli/tests/vendor_eject.rs index 2ee7df82c..ef4111aed 100644 --- a/crates/socket-patch-cli/tests/vendor_eject.rs +++ b/crates/socket-patch-cli/tests/vendor_eject.rs @@ -289,8 +289,8 @@ async fn eject_requires_user_patch_consent_and_reports_it_in_previews_and_writes mock_registry(&p).await; mock_view(&p).await; for args in [ - vec!["vendor", "--allow-user-patches", "--dry-run"], - vec!["vendor", "--allow-user-patches"], + vec!["vendor", "--allow-patch-conflicts", "--dry-run"], + vec!["vendor", "--allow-patch-conflicts"], ] { let (code, env) = p.run_json(&args); assert_eq!(code, 0, "{args:?}: {env:#}"); diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 236a4833a..981fe7149 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -1465,7 +1465,7 @@ Prebuilt binaries are published for: Socket Patch treats declared project patches as a separate source of ownership. `scan` and `apply` skip their packages and report `user_patch_conflict`; explicit `get` and `vendor` refuse the conflict before patching. Use -`--allow-user-patches` only after reviewing how the fixes interact. This flag +`--allow-patch-conflicts` only after reviewing how the fixes interact. This flag permits an attempt; it does not merge diffs or bypass unsupported backend cases. Installed file content that matches neither the original nor Socket-patched hash also fails by default. See the [CLI contract](../crates/socket-patch-cli/CLI_CONTRACT.md#existing-project-patches) diff --git a/docs/usage.md b/docs/usage.md index 54a5dcd39..cf471360a 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -250,8 +250,8 @@ A user patch is protected even if it changes a different file than Socket's fix. After reviewing the interaction, you can explicitly approve an attempt: ```sh -socket-patch scan --package minimist --mode agent --allow-user-patches -socket-patch apply --allow-user-patches --strict +socket-patch scan --package minimist --mode agent --allow-patch-conflicts +socket-patch apply --allow-patch-conflicts --strict ``` The second form still refuses modified file content. Without `--strict`, explicit From ce29b0016147c9cb20473fa0ed6043e237e4b581 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 22 Oct 2026 11:48:24 -0400 Subject: [PATCH 9/9] fix(cli): report skipped apply outcomes without claiming writes Preserve reason-bearing skip events across standalone and nested apply. Recheck conflicts under the apply lock and report late project patches in human and JSON output; count only actual applications, including mixed failures and multiple installed copies. Document the event contract and cover conflicts introduced during API fetches with deterministic regression tests. --- crates/socket-patch-cli/CLI_CONTRACT.md | 26 ++- .../src/commands/agent_download.rs | 103 ++++++++++-- crates/socket-patch-cli/src/commands/apply.rs | 155 +++++++++++++++--- crates/socket-patch-cli/src/commands/get.rs | 87 +++++++++- .../socket-patch-cli/src/commands/scan/mod.rs | 20 ++- .../src/commands/user_patches.rs | 22 ++- .../tests/get/get_failure_paths.rs | 77 +++++++++ .../tests/scan/scan_failure_paths.rs | 72 ++++++++ crates/socket-patch-cli/tests/user_patches.rs | 11 ++ docs/usage.md | 5 + 10 files changed, 528 insertions(+), 50 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 7485df1ad..2e4d25300 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -153,14 +153,23 @@ different file, are not installed yet, or all Socket `beforeHash` checks pass. A missing referenced diff does not remove that protection. - `scan` (every mode, including previews) excludes matching packages before patch - selection and emits `warnings[].code: user_patch_conflict`, naming the purl - and evidence path. Other packages remain eligible; skips alone exit 0. + selection and emits a `skipped` event plus `warnings[].code: user_patch_conflict`, + naming the purl and evidence path. Other packages remain eligible; skips alone exit 0. The full installed inventory still protects skipped packages from prune-as-missing. - Mutating `apply`, including the nested apply of agent-mode `get` / `scan`, leaves each protected manifest entry out of the run with a - `user_patch_conflict` warning and applies every other entry; skips alone exit 0, + `skipped` event and `user_patch_conflict` warning and applies every other entry; skips alone exit 0, so a `postinstall` apply never fails an install over them. The manifest is not changed, and a skipped entry is not reconciled away as dropped. +- Skipped events carry a stable `errorCode` and a human-readable `reason`. + `user_patch_conflict` identifies project patch/replacement metadata and names + its source; `vendored`, `already_patched`, and `package_not_installed` explain + other apply skips. A conflict introduced after selection is still `skipped` + by nested apply, with its selected purl and UUID, even on an otherwise + successful run. It never contributes to `summary.applied`. Known selection-time + skips are not emitted a second time by nested apply; newly discovered conflicts + are also shown in human output. Saving a manifest record remains a separate + `downloaded` / `updated` event and does not prove the package was patched. - Explicit `get` selection (including UUID, `--save-only`, and `--dry-run`) and `vendor` refuse selected conflicts with exit 1, `status: error`, `error.code: user_patch_conflict`, before their patch writes. `vendor` still @@ -1668,8 +1677,11 @@ command's integration tests check its output. | `vendor_prebuilt_stub_invalid` | `failed` | RubyGems: the server stub lacks required attributes or is otherwise invalid; no local stub fallback is permitted. | | `vendor_*` / `pypi_*` / `gemfile_*` / `lock_*` / `locked_version_mismatch` / `user_authored_*` / `native_extensions_unsupported` / `platform_gem_unsupported` | `failed`/`skipped` | vendor: per-ecosystem refusal + drift vocabulary; see the Vendor command contract section. New tags are additive (MINOR). | -`user_patch_conflict` is also a scan and apply run warning for packages excluded by the -project-patch consent gate; `user_patch_override` records explicit consent. +`user_patch_conflict` is a `skipped` event and a scan/apply run warning for packages +excluded by the project-patch consent gate; `reason` includes the evidence and the +opt-in remedy. `user_patch_override` records explicit consent. Agent get/scan +use `skipped` / `apply_not_attempted` if a successful nested apply has no matching +outcome for a selected record, rather than inferring a write from exit 0. ### Top-level `EnvelopeError` codes @@ -1737,7 +1749,7 @@ project-patch consent gate; `user_patch_override` records explicit consent. | `vex` | `Verified` (one per attested subcomponent) · `Skipped` (omissions) — only under `--json --output` | | `remove` | `Removed` (per purl; `Verified` on dry-run) · `RolledBack` (one per installed copy the in-place rollback restored; `summary.rolledBack`) · vendored/hosted-leg `Removed`/`Skipped` events with `details.mode` · artifact-level `Removed`/`Verified` GC carrier (`details: {count, checked}`, `bytes`; only when the sweep removed something — v5.0, MAJOR: replaces `details.blobsRemoved` / `archivesRemoved` / `rolledBack`) · top-level `gc` (absent under `--preserve-state`) · `cleanup_failed` warnings | | `rollback` | `RolledBack` (agent copies; vendored / hosted unwinds with `details.mode`) · `Verified` (dry-run previews, manifest removals included) · `Skipped` (already_original / no_files / package_not_installed) · `Failed` (per copy or leg; artifact-level `hosted_write_failed` / `hosted_wiring_contested`) · `Removed` (manifest entries, `details.manifest: true`) · top-level `gc` (absent under `--preserve-state`) | -| `get`, `scan` | Agent mode: `Downloaded` / `Updated` (+`oldUuid`) / `Skipped` (`already_in_manifest`, `vendored`, `package_not_installed`, …) / `Failed` per patch, then `Applied` / `Failed` for the nested apply · Hosted: `Applied` (pin written) / `Skipped` (`details.mode: "hosted"`) · Vendored: `Downloaded` / `Failed`, then the vendor engine's events (`details.mode: "vendored"`) · dry runs: `Verified` / `Skipped` · `scan --prune`: `Removed` (`details.manifest: true`, or a vendored revert) / `Skipped` + top-level `gc` · `get` search outcomes: `Skipped` (`paid_required`, `package_not_installed`, …) — see [`scan` and `get` JSON](#scan-and-get-json-v50-major) | +| `get`, `scan` | Agent mode: `Downloaded` / `Updated` (+`oldUuid`) / `Skipped` (`already_in_manifest`, `vendored`, `package_not_installed`, …) / `Failed` per patch, then `Applied` / `Skipped` / `Failed` for the nested apply · Hosted: `Applied` (pin written) / `Skipped` (`details.mode: "hosted"`) · Vendored: `Downloaded` / `Failed`, then the vendor engine's events (`details.mode: "vendored"`) · dry runs: `Verified` / `Skipped` · `scan --prune`: `Removed` (`details.manifest: true`, or a vendored revert) / `Skipped` + top-level `gc` · `get` search outcomes: `Skipped` (`paid_required`, `package_not_installed`, …) — see [`scan` and `get` JSON](#scan-and-get-json-v50-major) | | `--update` | `Downloaded` → `Updated` (success) · `Skipped` (already_latest) · `Verified` (dry-run check, reason update_check) — see the Self-update contract section for details fields and top-level error codes | ### Migration status (v3.0) @@ -1787,7 +1799,7 @@ One command is **intentionally not** plain-envelope and will stay that way (not } ``` -**Events per mode.** Agent mode (`get`, `scan --mode agent` / `--sync`): a fetched patch is `downloaded` (new manifest record) or `updated` (+`oldUuid`: it replaced another uuid), one already recorded at the same uuid is `skipped` / `already_in_manifest` (it is still re-applied), a patch that could not be fetched or recorded is `failed` (`download_failed`, `patch_no_applicable_files`, `blob_write_failed`); then the nested apply adds `applied` for each recorded patch it patched (or found patched) and `failed` (apply's `apply_failed` / `package_not_installed` with its `error`, #424) for each it failed — a failing manifest patch the run did not select (the nested apply covers the whole `--ecosystems`-scoped manifest) gets its own `failed` event, and a failure no single patch explains (an unreadable manifest, the yarn PnP refusal, unavailable patch sources) is a purl-less `failed` event, so a failed apply is always `partialFailure` (exit 1). The apply's mismatch overwrites (#1004) are `content_mismatch_overwritten` warnings; a record whose purl the vendor ledger wires at another uuid adds a `vendored_uuid_drift` warning. Vendor-owned and lockfile-only selections are `skipped` / `vendored` and `skipped` / `package_not_installed` before any download. Hosted and vendored events are described under "Scan modes" above; every one carries `details.mode`. `get`'s narrowing adds `skipped` events (`package_not_installed`, `yarn_pnp_unsupported`, `pnpm_pnp_unsupported`) and `paidRequired` runs one `skipped` / `paid_required` event per patch found. +**Events per mode.** Agent mode (`get`, `scan --mode agent` / `--sync`): a fetched patch is `downloaded` (new manifest record) or `updated` (+`oldUuid`: it replaced another uuid), one already recorded at the same uuid is `skipped` / `already_in_manifest` (it is still re-applied), a patch that could not be fetched or recorded is `failed` (`download_failed`, `patch_no_applicable_files`, `blob_write_failed`); then the nested apply adds `applied` only for each recorded patch it actually patched, `skipped` with `errorCode` / `reason` for each package it left untouched (including `already_patched`, `vendored`, `package_not_installed`, and `user_patch_conflict`), and `failed` (apply's `apply_failed` / `package_not_installed` with its `error`, #424) for each it failed — a failing manifest patch the run did not select (the nested apply covers the whole `--ecosystems`-scoped manifest) gets its own `failed` event, and a failure no single patch explains (an unreadable manifest, the yarn PnP refusal, unavailable patch sources) is a purl-less `failed` event, so a failed apply is always `partialFailure` (exit 1). The apply's mismatch overwrites (#1004) are `content_mismatch_overwritten` warnings; a record whose purl the vendor ledger wires at another uuid adds a `vendored_uuid_drift` warning. Vendor-owned and lockfile-only selections are `skipped` / `vendored` and `skipped` / `package_not_installed` before any download. Hosted and vendored events are described under "Scan modes" above; every one carries `details.mode`. `get`'s narrowing adds `skipped` events (`package_not_installed`, `yarn_pnp_unsupported`, `pnpm_pnp_unsupported`) and `paidRequired` runs one `skipped` / `paid_required` event per patch found. **Patch metadata.** `downloaded` / `updated` events (agent and vendored) carry the patch metadata in `details`, the same keys for every command: diff --git a/crates/socket-patch-cli/src/commands/agent_download.rs b/crates/socket-patch-cli/src/commands/agent_download.rs index eddace731..4d5830f72 100644 --- a/crates/socket-patch-cli/src/commands/agent_download.rs +++ b/crates/socket-patch-cli/src/commands/agent_download.rs @@ -1434,9 +1434,11 @@ pub(crate) async fn run_nested_apply( common: GlobalArgs, json: bool, report_user_patch_skips: bool, + recorded: &[(String, String)], client: &ApiClient, lock: LockGuard, ) -> ApplyRunReport { + let quiet = common.silent || json; let manifest_path = common.resolved_manifest_path(); let apply_args = super::apply::ApplyArgs { common, @@ -1448,7 +1450,28 @@ pub(crate) async fn run_nested_apply( report_user_patch_skips, }), }; - let report = super::apply::run_locked(apply_args, manifest_path, client, lock).await; + let mut report = super::apply::run_locked(apply_args, manifest_path, client, lock).await; + // Scan already printed its selection-time skips. A selected package that + // the locked recheck now skips is NEW evidence and must still be visible. + if !report_user_patch_skips { + for skip in &report.skipped { + if skip.error_code.as_deref() == Some(super::user_patches::CONFLICT) + && skip + .purl + .as_deref() + .is_some_and(|key| recorded.iter().any(|(purl, _)| apply_key_covers(key, purl))) + { + if let Some(reason) = &skip.reason { + report + .warnings + .push(RunWarning::new(super::user_patches::CONFLICT, reason)); + if !quiet { + eprintln!("Warning: {reason}"); + } + } + } + } + } // An error, so exempt from --silent ("errors only": a failing exit must // say why); JSON runs carry the failure in the envelope instead. if report.code != 0 && !json { @@ -1468,11 +1491,13 @@ pub(crate) fn apply_key_covers(key: &str, record: &str) -> bool { /// Record what the nested apply did to the run's recorded patches /// (`recorded`: `(purl, uuid)` of every selected patch now in the manifest) -/// into `env`, so `--json` says what the human run prints (#424). A clean -/// apply is one `applied` event per recorded patch. A failed one records a +/// into `env`, so `--json` says what the human run prints (#424). Only a +/// reported write counts as `applied`, even when apply exits 0. Skips retain +/// their `reason` and `errorCode`, including new conflicts found after selection. +/// A failed run records a /// `failed` event (apply's `errorCode` / `error`) for each recorded patch a -/// failure covers, `applied` for each one apply reports patched (or already -/// patched), a `failed` event for each other failing manifest patch (one +/// failure covers, `applied` for each one apply reports patched, a `failed` +/// event for each other failing manifest patch (one /// this run did not select — the nested apply covers the whole /// `--ecosystems`-scoped manifest; `uuid_of` looks up its uuid), and a /// purl-less `failed` event for a run-level reason (unreadable manifest, @@ -1492,14 +1517,6 @@ pub(crate) fn record_apply_outcome( return; }; env.warnings.extend(report.warnings.iter().cloned()); - if report.code == 0 { - for (purl, uuid) in recorded { - env.record( - PatchEvent::new(EventAction::Applied, purl.as_str()).with_uuid(uuid.as_str()), - ); - } - return; - } let mut failed_any = false; for (purl, uuid) in recorded { if let Some(failure) = report @@ -1513,12 +1530,51 @@ pub(crate) fn record_apply_outcome( .with_error(failure.code.as_str(), failure.error.as_str()), ); failed_any = true; + } else if let Some(skip) = report.skipped.iter().find(|skip| { + skip.purl + .as_deref() + .is_some_and(|key| apply_key_covers(key, purl)) + }) { + let mut event = skip.clone().with_uuid(uuid); + event.purl = Some(purl.clone()); + record_apply_skip(env, event); } else if report.applied.iter().any(|k| apply_key_covers(k, purl)) { env.record( PatchEvent::new(EventAction::Applied, purl.as_str()).with_uuid(uuid.as_str()), ); + } else if report.code == 0 { + record_apply_skip( + env, + PatchEvent::new(EventAction::Skipped, purl) + .with_uuid(uuid) + .with_reason( + "apply_not_attempted", + "The nested apply did not report applying this package", + ), + ); } } + for skip in &report.skipped { + let Some(key) = skip.purl.as_deref() else { + continue; + }; + if recorded.iter().any(|(purl, _)| apply_key_covers(key, purl)) + || report + .failures + .iter() + .any(|failure| apply_key_covers(&failure.purl, key)) + { + continue; + } + let mut event = skip.clone(); + if event.uuid.is_none() { + event.uuid = uuid_of(key); + } + record_apply_skip(env, event); + } + if report.code == 0 { + return; + } let mut appended: Vec<&str> = Vec::new(); for failure in &report.failures { let explained = recorded @@ -1553,6 +1609,26 @@ pub(crate) fn record_apply_outcome( } } +/// Scan can already carry the same selection-time skip. Keep one event, adding +/// the manifest UUID when the locked apply knows it, without double-counting. +fn record_apply_skip(env: &mut Envelope, event: PatchEvent) { + if let Some(existing) = env.events.iter_mut().find(|existing| { + existing.action == EventAction::Skipped + && existing.error_code == event.error_code + && existing + .purl + .as_deref() + .zip(event.purl.as_deref()) + .is_some_and(|(a, b)| PurlKey::qualified(a) == PurlKey::qualified(b)) + }) { + if existing.uuid.is_none() { + existing.uuid = event.uuid; + } + } else { + env.record(event); + } +} + /// [`download_and_apply_patches_into`] over a fresh `get` envelope: the /// public entry the in-process tests and embedders drive. Returns `(exit /// code, envelope)`; nothing is printed on stdout. @@ -1744,6 +1820,7 @@ pub async fn download_and_apply_patches_into( nested_apply_args_from_params(params, run, &manifest_path), params.json, !params.user_patch_skips_reported, + &recorded, run.api_client, lock, ) diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index 9a97a4f09..0a71cdcc5 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -838,9 +838,16 @@ fn manifest_targets_npm(manifest: &PatchManifest) -> bool { const YARN_PNP_UNSUPPORTED: &str = "yarn-berry Plug'n'Play layout is not supported by socket-patch (packages live inside .yarn/cache zips). Use `yarn patch ` instead."; fn refuse_yarn_pnp(args: &ApplyArgs) -> i32 { + refuse_yarn_pnp_with_skips(args, &[]) +} + +fn refuse_yarn_pnp_with_skips(args: &ApplyArgs, skipped: &[PatchEvent]) -> i32 { if args.common.json { let mut env = Envelope::new(Command::Apply); env.dry_run = args.common.dry_run; + for skip in skipped { + env.record(skip.clone()); + } env.mark_error(EnvelopeError::new( "yarn_pnp_unsupported", YARN_PNP_UNSUPPORTED, @@ -1042,10 +1049,13 @@ pub(crate) struct ApplyRunReport { /// `(errorCode, error)`. Set only when `code != 0` and `failures` /// alone would not explain it. pub run_error: Option<(String, String)>, - /// The package keys apply patched or found already patched, so a - /// failed run's caller can count exactly what applied. Filled only - /// when `code != 0`. + /// The package keys actually patched (or verified in a dry run). + /// Filled on successful and failed runs; exit 0 alone does not prove a write. pub applied: Vec, + /// Whole-package skips, including project patches, vendored ownership, + /// already-patched files and absent installations, with their reasons. + /// These are outcomes even when their warning was already printed by scan. + pub skipped: Vec, /// Non-fatal per-file warnings the caller's envelope must carry: one /// `content_mismatch_overwritten` per file the explicitly approved mismatch policy /// overwrote (#1004). A nested apply never prints JSON and is silent @@ -1101,6 +1111,55 @@ fn collect_apply_failures( failures } +/// Keep successful writes and whole-package skips separate for nested callers. +/// One already-patched copy must not hide a write to another copy of the same +/// package; per-copy advisories remain standalone apply events. +fn collect_apply_outcomes( + results: &[ApplyResult], + unmatched: &[String], + lockfile_only: &HashSet, + mut skipped: Vec, + dry_run: bool, +) -> (Vec, Vec) { + let mut applied = Vec::new(); + for result in results { + let event = result_to_event(result, dry_run); + match event.action { + PatchAction::Applied | PatchAction::Verified => { + applied.push(result.package_key.clone()); + } + PatchAction::Skipped => skipped.push(event), + _ => {} + } + } + for purl in unmatched { + let detail = if lockfile_only.contains(purl) { + LOCKFILE_ONLY_DETAIL + } else { + not_installed_detail(purl) + }; + skipped.push( + PatchEvent::new(PatchAction::Skipped, purl) + .with_reason("package_not_installed", detail), + ); + } + let applied_keys: HashSet<&str> = applied.iter().map(String::as_str).collect(); + let failed_keys: HashSet<&str> = results + .iter() + .filter(|r| !r.success) + .map(|r| r.package_key.as_str()) + .collect(); + skipped.retain(|event| { + event + .purl + .as_deref() + .is_none_or(|purl| !applied_keys.contains(purl) && !failed_keys.contains(purl)) + }); + skipped.sort_by(|a, b| (&a.purl, &a.error_code).cmp(&(&b.purl, &b.error_code))); + skipped.dedup_by(|a, b| a.purl == b.purl && a.error_code == b.error_code); + (applied, skipped) +} + /// The locked half of `apply`: everything from the manifest read on — the /// package-manager layout gate, the apply loop, embedded VEX, output and /// telemetry — over a `lock` the caller already holds and the caller's @@ -1133,7 +1192,7 @@ pub(crate) async fn run_locked( "manifest_not_found", format!("Manifest not found at {}", manifest_path.display()), ); - let code = report_apply_failure(&args, &err, &err.message, &telemetry).await; + let code = report_apply_failure(&args, &err, &err.message, &[], &telemetry).await; return ApplyRunReport::run_failure(code, &err.code, err.message); } Err(e) => { @@ -1141,7 +1200,7 @@ pub(crate) async fn run_locked( // One manifest-load mapping for every command (#931): // `manifest_invalid` / `manifest_unreadable`, never `apply_failed`. let err = crate::json_envelope::manifest_load_error(&manifest_path, &e); - let code = report_apply_failure(&args, &err, &e.to_string(), &telemetry).await; + let code = report_apply_failure(&args, &err, &e.to_string(), &[], &telemetry).await; return ApplyRunReport::run_failure(code, &err.code, err.message); } }; @@ -1149,11 +1208,15 @@ pub(crate) async fn run_locked( // Packages the project patched itself sit this run out (warned); every // other record still applies. The read-only `--check` verifies them all. let mut manifest = manifest; - let (user_patch_warnings, user_patched) = if args.check { + let (user_patch_warnings, user_patch_skips) = if args.check { Default::default() } else { super::user_patches::skip_protected(&args.common, &mut manifest.patches).await }; + let user_patched = user_patch_skips + .iter() + .filter_map(|e| e.purl.clone()) + .collect(); // Package-manager layout detection. yarn-berry PnP keeps packages // inside `.yarn/cache/*.zip` and resolves them via `.pnp.cjs` — @@ -1169,11 +1232,15 @@ pub(crate) async fn run_locked( match detect_npm_pkg_manager(&args.common.cwd) { NpmPkgManager::YarnBerryPnP => { if eco_in_local_scope(&args.common, Ecosystem::Npm) && manifest_targets_npm(&manifest) { - return ApplyRunReport::run_failure( - refuse_yarn_pnp(&args), - "yarn_pnp_unsupported", - YARN_PNP_UNSUPPORTED, - ); + let code = refuse_yarn_pnp_with_skips(&args, &user_patch_skips); + return ApplyRunReport { + skipped: user_patch_skips, + ..ApplyRunReport::run_failure( + code, + "yarn_pnp_unsupported", + YARN_PNP_UNSUPPORTED, + ) + }; } } NpmPkgManager::Pnpm => { @@ -1378,6 +1445,9 @@ pub(crate) async fn run_locked( if args.common.json { let mut env = Envelope::new(Command::Apply); env.dry_run = args.common.dry_run; + for skip in &user_patch_skips { + env.record(skip.clone()); + } for result in &results { env.record(result_to_event(result, args.common.dry_run)); // Mismatch overwrites ride as Skipped warning events @@ -1533,10 +1603,19 @@ pub(crate) async fn run_locked( let mut warnings = mismatch_overwrite_warnings(&results, args.common.dry_run); warnings.extend(cache_warnings); warnings.extend(nested_skip_warnings); + let (applied, skipped) = collect_apply_outcomes( + &results, + &unmatched, + &lockfile_only, + user_patch_skips, + args.common.dry_run, + ); // A requested-but-failed VEX flips an otherwise-successful // apply to a non-zero exit (fail-the-command contract). if success && !vex_failed { return ApplyRunReport { + applied, + skipped, warnings, ..ApplyRunReport::default() }; @@ -1566,25 +1645,23 @@ pub(crate) async fn run_locked( } else { None }; - // Vendor-owned results are skips, not applies. - let applied = results - .iter() - .filter(|r| r.success && r.package_path != VENDOR_OWNED_MARKER) - .map(|r| r.package_key.clone()) - .collect(); ApplyRunReport { code: 1, failures, run_error, applied, + skipped, warnings, } } Err(e) => { lock.release(); let err = EnvelopeError::new("apply_failed", e.clone()); - let code = report_apply_failure(&args, &err, &e, &telemetry).await; - ApplyRunReport::run_failure(code, "apply_failed", e) + let code = report_apply_failure(&args, &err, &e, &user_patch_skips, &telemetry).await; + ApplyRunReport { + skipped: user_patch_skips, + ..ApplyRunReport::run_failure(code, "apply_failed", e) + } } } } @@ -1600,12 +1677,16 @@ async fn report_apply_failure( args: &ApplyArgs, err: &EnvelopeError, error: &str, + skipped: &[PatchEvent], telemetry: &TelemetryAuth, ) -> i32 { track_patch_apply_failed(error, args.common.dry_run, telemetry).await; if args.common.json { let mut env = Envelope::new(Command::Apply); env.dry_run = args.common.dry_run; + for skip in skipped { + env.record(skip.clone()); + } env.mark_error(err.clone()); println!("{}", env.to_pretty_json()); } else { @@ -3367,6 +3448,42 @@ mod tests { } } + #[test] + fn nested_outcomes_keep_only_whole_package_skips() { + let mut unchanged_copy = sample_applied(VerifyStatus::AlreadyPatched); + let changed_copy = sample_applied(VerifyStatus::Ready); + let mut vendored = sample_applied(VerifyStatus::Ready); + vendored.package_key = "pkg:npm/vendor@1.0.0".into(); + vendored.package_path = VENDOR_OWNED_MARKER.into(); + let mut already_patched = unchanged_copy.clone(); + already_patched.package_key = "pkg:npm/already@1.0.0".into(); + unchanged_copy.package_path = "/another/node_modules/minimist".into(); + let missing = "pkg:npm/missing@1.0.0".to_string(); + let (applied, skipped) = collect_apply_outcomes( + &[unchanged_copy, changed_copy, vendored, already_patched], + std::slice::from_ref(&missing), + &HashSet::from([missing.clone()]), + Vec::new(), + false, + ); + assert_eq!(applied, vec!["pkg:npm/minimist@1.2.2"]); + let reasons: Vec<_> = skipped + .iter() + .map(|s| (s.purl.as_deref().unwrap(), s.error_code.as_deref().unwrap())) + .collect(); + assert_eq!( + reasons, + vec![ + ("pkg:npm/already@1.0.0", "already_patched"), + ("pkg:npm/missing@1.0.0", "package_not_installed"), + ("pkg:npm/vendor@1.0.0", "vendored"), + ] + ); + assert!(skipped + .iter() + .all(|s| s.reason.is_some() && s.files.is_empty())); + } + #[test] fn failed_result_maps_to_failed_action() { let mut result = sample_applied(VerifyStatus::Ready); diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index 49a45cdc0..32572721a 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -2108,6 +2108,7 @@ async fn save_and_apply_patch( nested_apply_args(&args.common, &manifest_path, quiet), args.common.json, true, + &recorded, client, lock, ) @@ -2144,7 +2145,10 @@ async fn save_and_apply_patch( env.warnings.extend(warnings); // A failed apply names what failed (#424); the manifest names the // uuid of any other failing record. - let manifest = if apply_report.as_ref().is_some_and(|r| r.code != 0) { + let manifest = if apply_report + .as_ref() + .is_some_and(|r| r.code != 0 || !r.skipped.is_empty()) + { Box::pin(read_manifest(&manifest_path)).await.ok().flatten() } else { None @@ -3207,6 +3211,7 @@ mod tests { failures, run_error: None, applied: applied.iter().map(|p| p.to_string()).collect(), + skipped: Vec::new(), warnings: Vec::new(), } } @@ -3244,7 +3249,10 @@ mod tests { #[test] fn record_apply_outcome_clean_apply_records_every_patch_applied() { let mut env = Envelope::new(JsonCommand::Get); - let report = ApplyRunReport::default(); + let report = ApplyRunReport { + applied: vec!["pkg:npm/a@1.0.0".into(), "pkg:npm/b@1.0.0".into()], + ..ApplyRunReport::default() + }; let recs = recorded(&[("pkg:npm/a@1.0.0", "ua"), ("pkg:npm/b@1.0.0", "ub")]); record_apply_outcome(&mut env, &recs, Some(&report), |_| None); assert_eq!( @@ -3262,6 +3270,80 @@ mod tests { assert!(env.events.is_empty()); } + #[test] + fn record_apply_outcome_preserves_skip_reasons_on_success_and_failure() { + for code in [0, 1] { + let mut env = Envelope::new(JsonCommand::Get); + let mut report = ApplyRunReport { + code, + applied: vec!["pkg:npm/clean@1.0.0".into()], + skipped: vec![ + PatchEvent::new(EventAction::Skipped, "pkg:npm/@scope/a@1.0.0") + .with_reason("user_patch_conflict", "project patch in patches/a.patch"), + PatchEvent::new(EventAction::Skipped, "pkg:npm/vendor@1.0.0") + .with_reason("vendored", "managed by socket-patch vendor"), + ], + ..ApplyRunReport::default() + }; + if code != 0 { + report + .failures + .push(failure("pkg:npm/failed@1.0.0", "apply_failed", "mismatch")); + } + let recs = recorded(&[ + ("pkg:npm/%40scope/a@1.0.0?artifact_id=x", "ua"), + ("pkg:npm/vendor@1.0.0", "uv"), + ("pkg:npm/clean@1.0.0", "uc"), + ]); + record_apply_outcome(&mut env, &recs, Some(&report), |_| None); + assert_eq!(env.summary.applied, 1); + assert_eq!(env.summary.skipped, 2); + assert_eq!(env.summary.failed, u32::from(code != 0)); + let json = env.to_value(); + assert_eq!(json["events"][0]["action"], "skipped"); + assert_eq!(json["events"][0]["uuid"], "ua"); + assert_eq!(json["events"][0]["errorCode"], "user_patch_conflict"); + assert_eq!( + json["events"][0]["reason"], + "project patch in patches/a.patch" + ); + assert!(json["events"][0].get("files").is_none()); + } + } + + #[test] + fn record_apply_outcome_deduplicates_a_previously_reported_manifest_skip() { + let mut env = Envelope::new(JsonCommand::Scan); + let skip = PatchEvent::new(EventAction::Skipped, "pkg:npm/@scope/a@1.0.0") + .with_reason("user_patch_conflict", "existing project patch"); + env.record(skip.clone()); + let report = ApplyRunReport { + skipped: vec![skip.clone(), skip], + ..ApplyRunReport::default() + }; + record_apply_outcome(&mut env, &[], Some(&report), |_| Some("ua".into())); + assert_eq!(env.summary.skipped, 1); + assert_eq!(env.summary.applied, 0); + assert_eq!(env.events[0].uuid.as_deref(), Some("ua")); + } + + #[test] + fn record_apply_outcome_never_infers_a_write_from_exit_zero() { + let mut env = Envelope::new(JsonCommand::Get); + record_apply_outcome( + &mut env, + &recorded(&[("pkg:npm/a@1.0.0", "ua")]), + Some(&ApplyRunReport::default()), + |_| None, + ); + assert_eq!( + outcomes(&env), + vec![o("skipped", "pkg:npm/a@1.0.0", "ua", "apply_not_attempted")] + ); + assert!(env.events[0].reason.is_some()); + assert_eq!(env.summary.applied, 0); + } + #[test] fn record_apply_outcome_marks_the_failed_patch_and_counts_the_applied_one() { let mut env = Envelope::new(JsonCommand::Get); @@ -3405,6 +3487,7 @@ mod tests { failures: Vec::new(), run_error: Some(("yarn_pnp_unsupported".to_string(), "pnp".to_string())), applied: Vec::new(), + skipped: Vec::new(), warnings: vec![RunWarning::new("content_mismatch_overwritten", "w")], }; let recs = recorded(&[("pkg:npm/a@1.0.0", "ua")]); diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index 62ed89116..3d80602e5 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -2227,13 +2227,25 @@ async fn run_scan( // Protect before querying/selecting, in every mode and in dry-run previews. let user_patches = super::user_patches::inspect(&args.common).await; let mut user_patch_withheld = 0usize; + let mut user_patch_skips = Vec::new(); filtered_crawled.retain(|package| { let sources = user_patches.conflicts(&package.purl); if sources.is_empty() { return true; } + let detail = + super::user_patches::detail(&package.purl, &sources, args.common.allow_patch_conflicts); if !args.common.allow_patch_conflicts { user_patch_withheld += 1; + if !user_patch_skips + .iter() + .any(|event: &PatchEvent| event.purl.as_deref() == Some(package.purl.as_str())) + { + user_patch_skips.push( + PatchEvent::new(PatchAction::Skipped, &package.purl) + .with_reason(super::user_patches::CONFLICT, &detail), + ); + } } layout_refusals.push(( if args.common.allow_patch_conflicts { @@ -2242,7 +2254,7 @@ async fn run_scan( super::user_patches::CONFLICT } .into(), - super::user_patches::detail(&package.purl, &sources, args.common.allow_patch_conflicts), + detail, )); args.common.allow_patch_conflicts }); @@ -2315,6 +2327,9 @@ async fn run_scan( // Layout refusals ride `warnings` so a consumer can tell an // unscannable project from an empty one. env.warnings.extend(layout_warnings(&layout_refusals)); + for skip in &user_patch_skips { + env.record(skip.clone()); + } if let Some(gc) = &unwired_gc { gc.record_into(&mut env, args.common.dry_run); } @@ -2722,6 +2737,9 @@ async fn run_scan( env.set_extra("updates", serde_json::Value::Array(updates_json(&updates))); env.set_extra("rollout", stage.json()); env.warnings.extend(layout_warnings(&layout_refusals)); + for skip in &user_patch_skips { + env.record(skip.clone()); + } // One warning per failed batch (status and exit unchanged). for (batch, err) in &failed_batches { let line = render::batch_failed_warning(*batch, total_batches, err); diff --git a/crates/socket-patch-cli/src/commands/user_patches.rs b/crates/socket-patch-cli/src/commands/user_patches.rs index 596d440dc..af9a6b506 100644 --- a/crates/socket-patch-cli/src/commands/user_patches.rs +++ b/crates/socket-patch-cli/src/commands/user_patches.rs @@ -4,7 +4,7 @@ use socket_patch_core::patch::user_patches::UserPatchIndex; use crate::args::GlobalArgs; -use crate::json_envelope::{Command, Envelope, EnvelopeError}; +use crate::json_envelope::{Command, Envelope, EnvelopeError, PatchAction, PatchEvent}; pub(crate) const CONFLICT: &str = "user_patch_conflict"; pub(crate) const ALLOWED: &str = "user_patch_override"; @@ -108,22 +108,23 @@ pub(crate) fn warn(common: &GlobalArgs, warnings: &[(String, String)]) { /// the protected records from this run's in-memory copy and report each as /// a `user_patch_conflict` warning. Approved overrides stay in and come back /// as `user_patch_override` warnings. Only the caller's copy changes; the -/// manifest on disk is never rewritten here. Also returns the skipped purls: -/// they are still recorded, so nothing may reconcile them away as dropped. +/// manifest on disk is never rewritten here. Also returns skipped events with +/// the evidence and UUID: callers must preserve these outcomes, and nothing may +/// reconcile their records away as dropped. pub(crate) async fn skip_protected( common: &GlobalArgs, patches: &mut std::collections::HashMap< String, socket_patch_core::manifest::schema::PatchRecord, >, -) -> (Vec<(String, String)>, std::collections::HashSet) { +) -> (Vec<(String, String)>, Vec) { let mut warnings = Vec::new(); - let mut skipped = std::collections::HashSet::new(); + let mut skipped = Vec::new(); if !any_detectable(common, patches.keys().map(String::as_str)) { return (warnings, skipped); } let index = inspect(common).await; - patches.retain(|purl, _| { + patches.retain(|purl, record| { if !common.purl_ecosystem_selected(purl) { return true; } @@ -136,12 +137,17 @@ pub(crate) async fn skip_protected( warnings.push((ALLOWED.into(), message)); true } else { - warnings.push((CONFLICT.into(), message)); - skipped.insert(purl.clone()); + warnings.push((CONFLICT.into(), message.clone())); + skipped.push( + PatchEvent::new(PatchAction::Skipped, purl) + .with_uuid(&record.uuid) + .with_reason(CONFLICT, message), + ); false } }); warnings.sort(); + skipped.sort_by(|a, b| a.purl.cmp(&b.purl)); (warnings, skipped) } diff --git a/crates/socket-patch-cli/tests/get/get_failure_paths.rs b/crates/socket-patch-cli/tests/get/get_failure_paths.rs index 7ca1190b3..ea1027609 100644 --- a/crates/socket-patch-cli/tests/get/get_failure_paths.rs +++ b/crates/socket-patch-cli/tests/get/get_failure_paths.rs @@ -1601,6 +1601,83 @@ async fn engine_nested_apply_success_keeps_added_and_counts_applied() { ); } +/// The selection was eligible, then a user patch appeared while its view was +/// being fetched. The locked apply must preserve the bytes and report a skip, +/// including when scan suppressed warnings already seen during selection or a +/// different selected package fails its apply. +#[tokio::test] +#[serial] +async fn engine_nested_apply_preserves_conflicts_introduced_during_fetch() { + const OTHER: &str = "pkg:npm/other@1.0.0"; + for sibling_fails in [false, true] { + for skips_reported in [false, true] { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + write_project(tmp.path()); + let patch = tmp.path().join(format!("patches/{NAME}+1.0.0.patch")); + let patch_at_fetch = patch.clone(); + let response = view_json( + UUID, + PURL, + serde_json::json!({ + "package/index.js": { + "beforeHash": git_hash(BEFORE_BYTES), + "afterHash": git_hash(AFTER_BYTES), + "blobContent": b64(AFTER_BYTES) + } + }), + ); + Mock::given(method("GET")) + .and(path(format!("/v0/orgs/{ORG}/patches/view/{UUID}"))) + .respond_with(move |_: &wiremock::Request| { + std::fs::create_dir_all(patch_at_fetch.parent().unwrap()).unwrap(); + std::fs::write(&patch_at_fetch, "user diff").unwrap(); + ResponseTemplate::new(200).set_body_json(&response) + }) + .mount(&server) + .await; + let mut selected = vec![search_result(UUID, PURL)]; + if sibling_fails { + let pkg = tmp.path().join("node_modules/other"); + std::fs::create_dir_all(&pkg).unwrap(); + std::fs::write( + pkg.join("package.json"), + r#"{"name":"other","version":"1.0.0"}"#, + ) + .unwrap(); + std::fs::write(pkg.join("index.js"), "local edit").unwrap(); + mount_real_view(&server, UUID_B, OTHER).await; + selected.push(search_result(UUID_B, OTHER)); + } + let mut params = engine_params(tmp.path()); + params.save_only = false; + params.user_patch_skips_reported = skips_reported; + let (code, json) = download_and_apply_patches(&selected, ¶ms, &server.uri()).await; + assert_eq!(code, i32::from(sibling_fails), "{json}"); + let skip = find_event(&json, "skipped", Some("user_patch_conflict")); + assert_eq!(skip["purl"], PURL); + assert_eq!(skip["uuid"], UUID); + assert!(skip["reason"] + .as_str() + .unwrap() + .contains(patch.to_str().unwrap())); + assert_eq!(json["summary"]["applied"], 0, "{json}"); + assert_eq!(json["summary"]["skipped"], 1, "{json}"); + assert_eq!( + json["summary"]["failed"], + usize::from(sibling_fails), + "{json}" + ); + assert_eq!( + std::fs::read(tmp.path().join(format!("node_modules/{NAME}/index.js"))).unwrap(), + BEFORE_BYTES + ); + assert_eq!(std::fs::read_to_string(patch).unwrap(), "user diff"); + assert_eq!(manifest_json(tmp.path())["patches"][PURL]["uuid"], UUID); + } + } +} + /// `get --json` (the single-patch path) with the nested apply ON /// over a first-party link: one JSON document whose patch record carries /// the apply failure, and `failed: 1`. diff --git a/crates/socket-patch-cli/tests/scan/scan_failure_paths.rs b/crates/socket-patch-cli/tests/scan/scan_failure_paths.rs index aafeb160f..1cebdcd1a 100644 --- a/crates/socket-patch-cli/tests/scan/scan_failure_paths.rs +++ b/crates/socket-patch-cli/tests/scan/scan_failure_paths.rs @@ -865,6 +865,11 @@ async fn scan_skips_user_patches_in_every_mode_before_querying_or_writing() { std::fs::write(tmp.path().join("package.json"), declaration).unwrap(); let (code, json) = run_json(tmp.path(), &mock.uri(), &["--mode", mode, "--yes"]); assert_eq!(code, 0, "{mode}: {json}"); + let skip = find_event(&json, "skipped", Some("user_patch_conflict")); + assert_eq!(skip["purl"], MINIMIST); + assert!(skip["reason"].as_str().unwrap().contains("package.json")); + assert_eq!(json["summary"]["applied"], 0); + assert_eq!(json["summary"]["skipped"], 1); assert!( json["warnings"] .as_array() @@ -886,6 +891,73 @@ async fn scan_skips_user_patches_in_every_mode_before_querying_or_writing() { assert!(mock.received_requests().await.unwrap().is_empty()); } +#[tokio::test] +async fn scan_reports_a_patch_added_after_selection_in_json_and_human_output() { + use wiremock::matchers::{method, path}; + use wiremock::{Mock, ResponseTemplate}; + + const PURL: &str = "pkg:npm/example@1.0.0"; + for json in [true, false] { + let mock = MockServer::start().await; + mount_one_patch_api(&mock, PURL, b"before\n").await; + let tmp = tempfile::tempdir().unwrap(); + write_root_package_json(tmp.path()); + write_npm_package(tmp.path(), "example", "1.0.0", b"before\n"); + let patch = tmp.path().join("patches/example+1.0.0.patch"); + let patch_at_fetch = patch.clone(); + let response = serde_json::json!({ + "uuid": UUID, "purl": PURL, "publishedAt": "2024-01-01T00:00:00Z", + "files": {"package/index.js": { + "beforeHash": crate::common::git_sha256(b"before\n"), + "afterHash": crate::common::git_sha256(b"after\n"), + "blobContent": "YWZ0ZXIK" + }}, + "vulnerabilities": {}, "description": "late conflict", "license": "MIT", "tier": "free" + }); + Mock::given(method("GET")) + .and(path(format!("/v0/orgs/{ORG_SLUG}/patches/view/{UUID}"))) + .respond_with(move |_: &wiremock::Request| { + std::fs::create_dir_all(patch_at_fetch.parent().unwrap()).unwrap(); + std::fs::write(&patch_at_fetch, "user diff").unwrap(); + ResponseTemplate::new(200).set_body_json(&response) + }) + .with_priority(1) + .mount(&mock) + .await; + let mut args = vec!["--mode", "agent", "--yes"]; + if json { + args.push("--json"); + } + let (code, stdout, stderr) = run_scan_api(tmp.path(), &mock.uri(), &args); + assert_eq!(code, 0, "{stdout}\n{stderr}"); + if json { + let value = parse_json_envelope(&stdout); + let skip = find_event(&value, "skipped", Some("user_patch_conflict")); + assert_eq!(skip["purl"], PURL); + assert_eq!(skip["uuid"], UUID); + assert!(skip["reason"] + .as_str() + .unwrap() + .contains("example+1.0.0.patch")); + assert_eq!(value["summary"]["applied"], 0); + assert_eq!(value["summary"]["skipped"], 1); + assert!(value["warnings"].as_array().unwrap().iter().any(|warning| { + warning["code"] == "user_patch_conflict" && warning["detail"] == skip["reason"] + })); + assert!(!stderr.contains("example+1.0.0.patch"), "{stderr}"); + } else { + assert!(stderr.contains("example+1.0.0.patch"), "{stdout}\n{stderr}"); + assert!(stderr.contains("--allow-patch-conflicts"), "{stderr}"); + } + assert_eq!( + std::fs::read(tmp.path().join("node_modules/example/index.js")).unwrap(), + b"before\n" + ); + assert_eq!(std::fs::read_to_string(patch).unwrap(), "user diff"); + assert_eq!(read_manifest(tmp.path())["patches"][PURL]["uuid"], UUID); + } +} + #[tokio::test] async fn scan_still_patches_unrelated_packages_and_forwards_explicit_consent() { for allowed in [false, true] { diff --git a/crates/socket-patch-cli/tests/user_patches.rs b/crates/socket-patch-cli/tests/user_patches.rs index c79dcf57d..01ba55d40 100644 --- a/crates/socket-patch-cli/tests/user_patches.rs +++ b/crates/socket-patch-cli/tests/user_patches.rs @@ -79,6 +79,17 @@ fn apply_skips_and_vendor_refuses_declared_patches_despite_matching_hashes_or_ge // explicit vendoring of the whole manifest, refuses. let message = if args[0] == "apply" { assert_eq!(code, 0, "{args:?} {file}: {stdout} {stderr}"); + let skip = value["events"] + .as_array() + .unwrap() + .iter() + .find(|event| event["action"] == "skipped" && event["purl"] == PURL) + .unwrap_or_else(|| panic!("missing skipped event: {value}")); + assert_eq!(skip["errorCode"], "user_patch_conflict"); + assert_eq!(skip["uuid"], UUID); + assert!(skip["reason"].as_str().unwrap().contains(file)); + assert_eq!(value["summary"]["applied"], 0); + assert_eq!(value["summary"]["skipped"], 1); let warning = value["warnings"] .as_array() .unwrap() diff --git a/docs/usage.md b/docs/usage.md index cf471360a..4edb5b158 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -247,6 +247,11 @@ patch was found, and `apply` leaves them out while applying everything else. Explicit `get` and `vendor` refuse those conflicts. A user patch is protected even if it changes a different file than Socket's fix. +With `--json`, a skipped package has an `action: "skipped"` event, a stable +`errorCode` such as `user_patch_conflict`, and a `reason` explaining the conflict +and where it was found. It is not counted in `summary.applied`, including when +the conflict appears after selection while a patch is downloading. + After reviewing the interaction, you can explicitly approve an attempt: ```sh