diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index d9895a2c..8356800f 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 183a88e8..b7125cb2 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -2863,7 +2863,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 c5873221..619523d1 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. @@ -755,11 +796,75 @@ mod tests { #[test] fn report_only_hint_names_agent_mode() { + 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()[0], - "To apply these patches in place, run:" + report_only_hint(&global)[1..], + [ + " socket-patch scan --mode agent -g", + " socket-patch get -g ", + ] ); - assert!(report_only_hint()[1].contains("--mode agent")); + 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 7d940fe3..2d2cbb61 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() {