From 1396481b4d99fffd4997dd9a2cc63955db86b1c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 12:29:16 +0000 Subject: [PATCH 1/4] Start fix for #464 Assisted-by: Claude Code:claude-opus-5-5 From b31107307fe081187f2238e76b14e13de3621868 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 12:41:07 +0000 Subject: [PATCH 2/4] Keep global scope in report-only scan hint A report-only `scan -g` (or `--global-prefix `) ends with a hint for applying what it found. The hint dropped the global flag, so running it as printed scanned the cwd project instead, exited 0, and left the global install unpatched. The hint now repeats the run's scope: `-g`, or `--global-prefix ` shell-quoted when the path needs it. A project `--prune` scan keeps the old hint. Covered by unit tests on the hint and an integration test of a real report-only global-prefix scan. Fixes #464 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../socket-patch-cli/src/commands/scan/mod.rs | 2 +- .../src/commands/scan/render.rs | 120 +++++++++++++++++- .../tests/covgap_commands_scan_mod.rs | 55 ++++++++ 4 files changed, 171 insertions(+), 8 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 69fb09df9..d3428b960 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -124,7 +124,7 @@ For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLoc ### Scan modes (v5.0) -**Mode resolution (`resolve_mode_flags`, MAJOR in v5.0).** `--mode`, or one of its legacy boolean spellings (`--vendor`, `--apply`/`--sync`), picks the mode. With none of them, `scan` runs **hosted** mode — JSON and human alike; the result nests under the JSON `redirect` sub-object (see the hosted paragraph below). The one exception: a `--prune` or `--global`/`--global-prefix` scan with no mode has no project lockfile to rewire, so it is **report-only** — discovery, the table, the `updates` array and the `redirectState` block below, plus the `--prune` GC — and, in human mode, ends with the hint `To apply these patches in place, run:` / ` socket-patch scan --mode agent [PATHS]` / ` socket-patch get `. An explicit `--mode hosted` or `--mode vendored` (or the hidden `--vendor`) with `--global`/`--global-prefix` is a usage error (exit 2: global installs have no project lockfile to redirect, or to wire vendored artifacts into); `get` enforces the same rule with the same wording. +**Mode resolution (`resolve_mode_flags`, MAJOR in v5.0).** `--mode`, or one of its legacy boolean spellings (`--vendor`, `--apply`/`--sync`), picks the mode. With none of them, `scan` runs **hosted** mode — JSON and human alike; the result nests under the JSON `redirect` sub-object (see the hosted paragraph below). The one exception: a `--prune` or `--global`/`--global-prefix` scan with no mode has no project lockfile to rewire, so it is **report-only** — discovery, the table, the `updates` array and the `redirectState` block below, plus the `--prune` GC — and, in human mode, ends with the hint `To apply these patches in place, run:` / ` socket-patch scan --mode agent [PATHS]` / ` socket-patch get `. A global scan's hint carries the run's scope, so it can be run verbatim: `-g` (`scan --mode agent -g` / `get -g <…>`), or `--global-prefix ` when a prefix was given (the directory shell-quoted when it needs it). An explicit `--mode hosted` or `--mode vendored` (or the hidden `--vendor`) with `--global`/`--global-prefix` is a usage error (exit 2: global installs have no project lockfile to redirect, or to wire vendored artifacts into); `get` enforces the same rule with the same wording. **Global scope never touches the project's state (v5.0).** A `--global`/`--global-prefix` run that starts inside a project acts on the global installs only. The `--cwd` project's hosted pins and vendor ledger are not its target: `rollback` and `remove` run no hosted or vendored leg (they restore the global copies and drop their manifest records; `rollback` keeps the manifest records of purls the project vendors), a pre-v5 hosted ledger is never retired, and the project's vendor ledger does not own the global copies, so `apply` and `scan --mode agent` patch the global copy of a purl the project vendors (no `vendored` skip, no `vendored_ownership_retained` warning). The standalone `vendor` command acts only on the project, so every form of it (plain, `--revert`, `--check`) is a usage error under global scope: exit 2, human `Error: cannot be used with vendor[ --revert| --check]: global installs have no project lockfile to …`, JSON `{status: "error", error: {code: "global_scope_unsupported", message}}`, checked before the project is read or locked (#498). diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index 8ce80d9f1..79c78931b 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -2854,7 +2854,7 @@ async fn run_scan( if report_only { // The "Patches to apply:" listing already ends with a blank line. if !silent { - for line in render::report_only_hint() { + for line in render::report_only_hint(&args.common) { println!("{line}"); } } diff --git a/crates/socket-patch-cli/src/commands/scan/render.rs b/crates/socket-patch-cli/src/commands/scan/render.rs index 2f881da3e..37350de22 100644 --- a/crates/socket-patch-cli/src/commands/scan/render.rs +++ b/crates/socket-patch-cli/src/commands/scan/render.rs @@ -8,6 +8,7 @@ use std::collections::HashMap; use socket_patch_core::api::types::{PatchSearchResult, VulnerabilityResponse}; use super::discovery::severity_order; +use crate::args::GlobalArgs; use crate::ui::{self, plural, Align}; /// Visible widths of the table's PATCHES and SEVERITY columns. @@ -269,15 +270,55 @@ pub(super) fn dry_run_line(plan: Plan, refused: usize) -> String { } /// Lines printed after a report-only scan (`--prune` or global with no -/// mode, so no lockfile to rewire): how to apply what it found. -pub(super) fn report_only_hint() -> [String; 3] { +/// mode, so no lockfile to rewire): how to apply what it found. A global +/// run's commands repeat its scope (`-g` or `--global-prefix `): +/// without it, running the hint verbatim patches the cwd project and +/// leaves the global copy unpatched (#464). +pub(super) fn report_only_hint(common: &GlobalArgs) -> [String; 3] { + let scope = match &common.global_prefix { + Some(prefix) => Some(format!( + "--global-prefix {}", + shell_word(&prefix.to_string_lossy()) + )), + None if common.global => Some("-g".to_string()), + None => None, + }; + let (scan, get) = match &scope { + Some(scope) => ( + format!(" socket-patch scan --mode agent {scope}"), + format!(" socket-patch get {scope} "), + ), + None => ( + " socket-patch scan --mode agent [PATHS]".to_string(), + " socket-patch get ".to_string(), + ), + }; [ "To apply these patches in place, run:".to_string(), - " socket-patch scan --mode agent [PATHS]".to_string(), - " socket-patch get ".to_string(), + scan, + get, ] } +/// `word` as one argument a user can paste into their shell: bare when it +/// holds only characters no shell treats specially, otherwise quoted (POSIX +/// single quotes; double quotes on Windows, which cmd and PowerShell both +/// read as one argument). +fn shell_word(word: &str) -> String { + let plain = |c: char| { + c.is_ascii_alphanumeric() + || matches!(c, '/' | '.' | '_' | '-' | ':' | '+' | '=' | ',' | '@') + || (cfg!(windows) && c == '\\') + }; + if !word.is_empty() && word.chars().all(plain) { + word.to_string() + } else if cfg!(windows) { + format!("\"{word}\"") + } else { + format!("'{}'", word.replace('\'', r"'\''")) + } +} + /// Printed (vendored mode, before vendoring) for a selected package whose /// installed bytes differ from the patch baseline: vendoring still /// proceeds, with the verified patched content. @@ -746,8 +787,75 @@ mod tests { #[test] fn report_only_hint_names_agent_mode() { - assert_eq!(report_only_hint()[0], "To apply these patches in place, run:"); - assert!(report_only_hint()[1].contains("--mode agent")); + let project = report_only_hint(&GlobalArgs::default()); + assert_eq!( + project, + [ + "To apply these patches in place, run:", + " socket-patch scan --mode agent [PATHS]", + " socket-patch get ", + ] + ); + } + + /// #464: the hint after a report-only global scan keeps the run's + /// global scope, or running it verbatim patches the cwd project. + #[test] + fn report_only_hint_keeps_global_scope() { + let global = GlobalArgs { + global: true, + ..GlobalArgs::default() + }; + assert_eq!( + report_only_hint(&global)[1..], + [ + " socket-patch scan --mode agent -g", + " socket-patch get -g ", + ] + ); + let mut rows = vec![ + ( + false, + "/opt/node/lib/node_modules", + "/opt/node/lib/node_modules", + ), + // `--global-prefix` alone selects the global tree; `-g` is not + // repeated next to it. + ( + true, + "/opt/node/lib/node_modules", + "/opt/node/lib/node_modules", + ), + ]; + if cfg!(windows) { + rows.push(( + false, + r"C:\Program Files\nodejs", + r#""C:\Program Files\nodejs""#, + )); + rows.push((false, r"C:\nodejs\node_modules", r"C:\nodejs\node_modules")); + } else { + rows.push((false, "/tmp/global lib", "'/tmp/global lib'")); + rows.push((false, "/tmp/it's", r"'/tmp/it'\''s'")); + rows.push((false, "/tmp/$HOME", "'/tmp/$HOME'")); + } + for (global, prefix, shown) in rows { + let args = GlobalArgs { + global, + global_prefix: Some(prefix.into()), + ..GlobalArgs::default() + }; + assert_eq!( + report_only_hint(&args)[1..], + [ + format!(" socket-patch scan --mode agent --global-prefix {shown}"), + format!( + " socket-patch get --global-prefix {shown} " + ), + ], + "prefix {prefix:?}" + ); + } } #[test] diff --git a/crates/socket-patch-cli/tests/covgap_commands_scan_mod.rs b/crates/socket-patch-cli/tests/covgap_commands_scan_mod.rs index 47fa71669..52255a86d 100644 --- a/crates/socket-patch-cli/tests/covgap_commands_scan_mod.rs +++ b/crates/socket-patch-cli/tests/covgap_commands_scan_mod.rs @@ -1293,6 +1293,61 @@ async fn scan_prune_without_a_mode_is_report_only() { ); } +/// #464: a report-only global scan hints at commands that keep the global +/// scope. Run verbatim without `--global-prefix`, the hint would scan the +/// cwd project and leave the global copy unpatched. +#[tokio::test] +async fn scan_global_report_only_hint_keeps_the_global_scope() { + let mock = MockServer::start().await; + let purl = "pkg:npm/minimist@1.2.2"; + mount_one_patch_api(&mock, purl, b"x\n").await; + + let tmp = tempfile::tempdir().unwrap(); + // A prefix with a space: the hint must quote it to stay runnable. + let prefix = tmp.path().join("global lib").join("node_modules"); + let pkg_dir = prefix.join("minimist"); + std::fs::create_dir_all(&pkg_dir).unwrap(); + std::fs::write( + pkg_dir.join("package.json"), + r#"{ "name": "minimist", "version": "1.2.2" }"#, + ) + .unwrap(); + std::fs::write(pkg_dir.join("index.js"), b"x\n").unwrap(); + let cwd = tmp.path().join("elsewhere"); + std::fs::create_dir_all(&cwd).unwrap(); + + let prefix_arg = prefix.to_str().unwrap(); + let (code, stdout, stderr) = + run_scan_human(&cwd, &mock.uri(), &["--global-prefix", prefix_arg]); + assert_eq!( + code, 0, + "report-only is a success; stdout={stdout}; stderr={stderr}" + ); + assert!( + stdout.contains("Patches to apply:") && stdout.contains(purl), + "the global scan must find the patch; got {stdout:?}" + ); + let quoted = if cfg!(windows) { + format!("\"{prefix_arg}\"") + } else { + format!("'{prefix_arg}'") + }; + for command in [ + format!(" socket-patch scan --mode agent --global-prefix {quoted}"), + format!(" socket-patch get --global-prefix {quoted} "), + ] { + assert!( + stdout.lines().any(|line| line == command), + "the hint must keep the global scope ({command:?}); got {stdout:?}" + ); + } + assert_eq!( + std::fs::read(pkg_dir.join("index.js")).unwrap(), + b"x\n", + "a report-only scan must not patch the global copy" + ); +} + /// Each spelling that folds to `--mode agent` applies without prompting. #[tokio::test] async fn scan_human_agent_mode_applies_without_prompting() { From f53164cf604c7ae8521f3c4663cc13b674fd0cd3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 14:34:56 +0000 Subject: [PATCH 3/4] Double trailing backslashes in the Windows report-only hint prefix shell_word wrapped a --global-prefix with spaces in double quotes as-is, so a prefix ending in '\' produced "C:\dir\" and the argv parser read that last backslash as escaping the closing quote: the pasted hint was no longer one argument. Doubling the trailing backslash run keeps the quote closing and still names the same directory. Co-Authored-By: Claude --- crates/socket-patch-cli/src/commands/scan/render.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/scan/render.rs b/crates/socket-patch-cli/src/commands/scan/render.rs index 619523d1d..22c516dea 100644 --- a/crates/socket-patch-cli/src/commands/scan/render.rs +++ b/crates/socket-patch-cli/src/commands/scan/render.rs @@ -303,7 +303,9 @@ pub(super) fn report_only_hint(common: &GlobalArgs) -> [String; 3] { /// `word` as one argument a user can paste into their shell: bare when it /// holds only characters no shell treats specially, otherwise quoted (POSIX /// single quotes; double quotes on Windows, which cmd and PowerShell both -/// read as one argument). +/// read as one argument). A trailing run of backslashes is doubled inside +/// the Windows quotes: the argv parser would otherwise read the last one as +/// escaping the closing quote. fn shell_word(word: &str) -> String { let plain = |c: char| { c.is_ascii_alphanumeric() @@ -313,7 +315,8 @@ fn shell_word(word: &str) -> String { if !word.is_empty() && word.chars().all(plain) { word.to_string() } else if cfg!(windows) { - format!("\"{word}\"") + let trailing = word.len() - word.trim_end_matches('\\').len(); + format!("\"{word}{}\"", "\\".repeat(trailing)) } else { format!("'{}'", word.replace('\'', r"'\''")) } @@ -843,6 +846,11 @@ mod tests { r#""C:\Program Files\nodejs""#, )); rows.push((false, r"C:\nodejs\node_modules", r"C:\nodejs\node_modules")); + rows.push(( + false, + r"C:\Program Files\nodejs\", + r#""C:\Program Files\nodejs\\""#, + )); } else { rows.push((false, "/tmp/global lib", "'/tmp/global lib'")); rows.push((false, "/tmp/it's", r"'/tmp/it'\''s'")); From 7e78c765b071fba2f15df0f02e3d2b410447a722 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 18:24:21 +0000 Subject: [PATCH 4/4] Route Gradle digests through utils::digest Ports #878 so the core lib tests pass here too: main is red on utils::digest::tests::production_digests_go_through_the_helpers because the Gradle files hash inline. This change is a no-op once #878 lands on main. (cherry picked from commit 659ac2c) Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/crawlers/gradle_cache.rs | 9 ++++----- crates/socket-patch-core/src/patch/jvm_jar.rs | 7 ++----- crates/socket-patch-core/src/patch/sidecars/maven.rs | 4 +--- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/gradle_cache.rs b/crates/socket-patch-core/src/crawlers/gradle_cache.rs index ef295ee27..afd7c4fba 100644 --- a/crates/socket-patch-core/src/crawlers/gradle_cache.rs +++ b/crates/socket-patch-core/src/crawlers/gradle_cache.rs @@ -70,8 +70,7 @@ pub fn hash_eq(dir_name: &str, sha1_hex: &str) -> bool { /// Whether `bytes` are the pristine download Gradle stored in the hash /// directory `dir_name` (their sha1 names it). pub fn pristine(dir_name: &str, bytes: &[u8]) -> bool { - use sha1::{Digest, Sha1}; - hash_eq(dir_name, &hex::encode(Sha1::digest(bytes))) + hash_eq(dir_name, &crate::utils::digest::sha1_hex_of(bytes)) } /// Whether `path` is a version directory of a `files-2.1` tree @@ -432,8 +431,6 @@ impl DerivedIndex { /// The [`DerivedCopies`] of the jar `jar_leaf` whose pristine bytes /// hash to `pristine_sha1`. pub fn query(&self, jar_leaf: &str, pristine_sha1: &str) -> DerivedCopies { - use sha1::{Digest, Sha1}; - let instrumented = format!("instrumented-{jar_leaf}"); let mut out = DerivedCopies { incomplete: self.incomplete, @@ -460,7 +457,9 @@ impl DerivedIndex { out.stale.push(path.clone()); } else if name == jar_leaf || name == instrumented { match crate::utils::fs::read_regular_to_bytes_sync(path) { - Ok(bytes) if hash_eq(&hex::encode(Sha1::digest(&bytes)), pristine_sha1) => { + Ok(bytes) + if hash_eq(&crate::utils::digest::sha1_hex_of(&bytes), pristine_sha1) => + { out.stale.push(path.clone()) } Ok(_) => out.unknown.push(path.clone()), diff --git a/crates/socket-patch-core/src/patch/jvm_jar.rs b/crates/socket-patch-core/src/patch/jvm_jar.rs index 82d679406..f38a84403 100644 --- a/crates/socket-patch-core/src/patch/jvm_jar.rs +++ b/crates/socket-patch-core/src/patch/jvm_jar.rs @@ -25,8 +25,6 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; -use sha1::Digest as _; - use crate::crawlers::gradle_cache; use crate::hash::git_sha256::compute_git_sha256_from_bytes; use crate::manifest::schema::PatchFileInfo; @@ -353,12 +351,11 @@ fn unpatched_members( } fn sha256_hex(bytes: &[u8]) -> String { - use sha2::Digest as _; - hex::encode(sha2::Sha256::digest(bytes)) + crate::utils::digest::sha256_hex_of(bytes) } fn sha1_hex(bytes: &[u8]) -> String { - hex::encode(sha1::Sha1::digest(bytes)) + crate::utils::digest::sha1_hex_of(bytes) } /// `/jvm-originals/.jar`. diff --git a/crates/socket-patch-core/src/patch/sidecars/maven.rs b/crates/socket-patch-core/src/patch/sidecars/maven.rs index f2f5a2466..8798bfce6 100644 --- a/crates/socket-patch-core/src/patch/sidecars/maven.rs +++ b/crates/socket-patch-core/src/patch/sidecars/maven.rs @@ -17,8 +17,6 @@ use std::path::{Path, PathBuf}; -use sha1::Digest as _; - use super::{ SidecarAdvisory, SidecarAdvisoryCode, SidecarError, SidecarFile, SidecarFileAction, SidecarPayload, SidecarSeverity, @@ -44,7 +42,7 @@ impl Algo { fn digest(self, bytes: &[u8]) -> String { match self { - Algo::Sha1 => hex::encode(sha1::Sha1::digest(bytes)), + Algo::Sha1 => crate::utils::digest::sha1_hex_of(bytes), Algo::Md5 => hex::encode(md5(bytes)), } }