diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 863dbcbca..749f3e734 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -87,7 +87,7 @@ Beyond the globals above, each subcommand defines a small set of local arguments | Subcommand | Local arg | Env var | Purpose | |---|---|---|---| | `apply` | `--force` / `-f` | — | Bypass beforeHash check | -| `apply` | `--check` | — | Read-only audit that every in-scope (`--ecosystems`) manifest patch is in place, for CI / GitHub-App auditing (v5.0: previously Go-only, which passed on any unpatched non-Go tree). Local Go patches: the committed `.socket/go-patches/` copies and `go.mod` `replace` directives match the manifest (`go_redirect_drift`). Every other patch: each installed copy hashes to the record's `afterHash` — the `vex` verifier over the `vex` copy lookup; the copies are narrowed by `apply`'s own rules: a release variant (a qualified purl such as `?artifact_id=` / `?platform=`) is judged only on the copies holding its distribution, matched against every variant of its base as `apply` matches them, and an installed copy that holds none of them is drift of the base purl (`no_matching_variant`, the copy `apply` fails with "no matching variant found"; a Gradle / Ivy cache dir is exempt, as in `apply`); for gem, once a bundle-store copy exists the `gem env` fallback-home copies are not judged (`apply` treats them as best-effort). Drift is a `failed` event per patch with `errorCode` `not_applied` (still unpatched), `hash_mismatch` (neither the original nor the patched bytes), `file_not_found` or `no_matching_variant`, status `partialFailure`, exit 1, and the human `Error: Patches are OUT OF SYNC:` report (printed even under `--silent`). In sync is exit 0: `Patches are in sync (N checked).` and, under `--json`, a `skipped` event per verified patch (`errorCode: already_patched`). A patch with no installed copy is skipped as `apply` skips it (`package_not_installed`; the human line adds `M not installed, skipped`). Vendor-owned patches are excluded (`vendor --check` audits them). Lock-free, fetch-free, offline-safe; it never writes. An unreadable manifest is drift (`manifest_unreadable`, exit 1) | +| `apply` | `--check` | — | Read-only audit that every in-scope (`--ecosystems`) manifest patch is in place, for CI / GitHub-App auditing (v5.0: previously Go-only, which passed on any unpatched non-Go tree). Local Go patches: the committed `.socket/go-patches/` copies and `go.mod` `replace` directives match the manifest (`go_redirect_drift`). Every other patch: each installed copy hashes to the record's `afterHash` — the `vex` verifier over the `vex` copy lookup; the copies are narrowed by `apply`'s own rules: a release variant (a qualified purl such as `?artifact_id=` / `?platform=`) is judged only on the copies holding its distribution, matched against every variant of its base as `apply` matches them, and an installed copy that holds none of them is drift of the base purl (`no_matching_variant`, the copy `apply` fails with "no matching variant found"; a Gradle / Ivy cache dir is exempt, as in `apply`); for gem, once a bundle-store copy exists the `gem env` fallback-home copies are not judged (`apply` treats them as best-effort). Drift is a `failed` event per patch with `errorCode` `not_applied` (still unpatched), `hash_mismatch` (neither the original nor the patched bytes), `file_not_found` or `no_matching_variant`, status `partialFailure`, exit 1, and the human `Error: Patches are OUT OF SYNC:` report (printed even under `--silent`), which ends with the remedy ``Run `socket-patch apply ` to regenerate them.``: `` repeats the check's own `--cwd`, `--manifest-path`, `-g` / `--global-prefix` and `--ecosystems` (each shell-quoted when needed), so running it verbatim heals the tree that was checked (v5.0, #1219). In sync is exit 0: `Patches are in sync (N checked).` and, under `--json`, a `skipped` event per verified patch (`errorCode: already_patched`). A patch with no installed copy is skipped as `apply` skips it (`package_not_installed`; the human line adds `M not installed, skipped`). Vendor-owned patches are excluded (`vendor --check` audits them). Lock-free, fetch-free, offline-safe; it never writes. An unreadable manifest is drift (`manifest_unreadable`, exit 1) | | `vendor` | `--force` / `-f` | — | Tolerate missing patch-target files in the stage + bypass the variant probe. A beforeHash mismatch no longer needs it: vendor staging auto-overwrites with the verified patched content (`vendor_content_mismatch_overwritten` warning) | | `vendor` | `--revert` | `SOCKET_VENDOR_REVERT` | Undo vendoring: restore recorded original lockfile fragments + remove `.socket/vendor/` artifacts. Works without a manifest. A package vendored over a hosted pin returns to its upstream registry entry, never to hosted (see "Takeover reconciliation") | | `vendor` | `--check` | — | Offline, read-only artifact and wiring audit; exits 1 on drift. Conflicts with `--revert`. | diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index 22ed44888..572396283 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -676,7 +676,7 @@ async fn run_check(args: &ApplyArgs, manifest_path: &Path) -> i32 { for (_, _, detail) in &drifts { eprintln!(" {detail}"); } - eprintln!("Run `socket-patch apply` to regenerate them."); + eprintln!("{}", check_remedy(&args.common)); } 1 } @@ -844,6 +844,15 @@ fn record_check_skips(env: &mut Envelope, in_sync: &[String], not_installed: &[S } } +/// The last line of the `apply --check` drift report: the `apply` that heals +/// it, with the check's own scope (`--cwd`, `--manifest-path`, `-g` / +/// `--global-prefix`, `--ecosystems`). A bare `socket-patch apply` would +/// act on another tree, exit 0 and leave the drift in place (#1219). +fn check_remedy(common: &GlobalArgs) -> String { + let command = format!("socket-patch apply{}", crate::ui::scope_args(common)); + format!("Run `{command}` to regenerate them.") +} + /// The `apply --check` success line: how many patches were checked, and how /// many were skipped for having no installed copy, so a check over an /// uninstalled tree never reads as a vacuous "in sync". @@ -4477,6 +4486,54 @@ mod tests { ); } + /// #1219: the drift report's remedy keeps the check's scope, so running + /// it verbatim heals the tree that was checked. + #[test] + fn check_remedy_repeats_the_check_scope() { + let remedy = |common: GlobalArgs| check_remedy(&common); + assert_eq!( + remedy(GlobalArgs::default()), + "Run `socket-patch apply` to regenerate them." + ); + assert_eq!( + remedy(GlobalArgs { + cwd: PathBuf::from("app"), + ..GlobalArgs::default() + }), + "Run `socket-patch apply --cwd app` to regenerate them." + ); + assert_eq!( + remedy(GlobalArgs { + global: true, + ..GlobalArgs::default() + }), + "Run `socket-patch apply -g` to regenerate them." + ); + let quoted = if cfg!(windows) { + "\"/opt/my site\"" + } else { + "'/opt/my site'" + }; + assert_eq!( + remedy(GlobalArgs { + global: true, + global_prefix: Some(PathBuf::from("/opt/my site")), + ..GlobalArgs::default() + }), + format!("Run `socket-patch apply --global-prefix {quoted}` to regenerate them.") + ); + assert_eq!( + remedy(GlobalArgs { + cwd: PathBuf::from("svc"), + manifest_path: "../shared/manifest.json".to_string(), + ecosystems: Some(vec!["npm".to_string(), "pypi".to_string()]), + ..GlobalArgs::default() + }), + "Run `socket-patch apply --cwd svc --manifest-path ../shared/manifest.json \ + --ecosystems npm,pypi` to regenerate them." + ); + } + #[test] fn failure_and_none_installed_errors() { assert_eq!( diff --git a/crates/socket-patch-cli/src/commands/scan/render.rs b/crates/socket-patch-cli/src/commands/scan/render.rs index e3db8a4b4..5311e93cb 100644 --- a/crates/socket-patch-cli/src/commands/scan/render.rs +++ b/crates/socket-patch-cli/src/commands/scan/render.rs @@ -275,14 +275,7 @@ pub(super) fn dry_run_line(plan: Plan, refused: usize) -> String { /// 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 scope = ui::global_scope_arg(common); let (scan, get) = match &scope { Some(scope) => ( format!(" socket-patch scan --mode agent {scope}"), @@ -300,28 +293,6 @@ 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). 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() - || matches!(c, '/' | '.' | '_' | '-' | ':' | '+' | '=' | ',' | '@') - || (cfg!(windows) && c == '\\') - }; - if !word.is_empty() && word.chars().all(plain) { - word.to_string() - } else if cfg!(windows) { - let trailing = word.len() - word.trim_end_matches('\\').len(); - format!("\"{word}{}\"", "\\".repeat(trailing)) - } 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. diff --git a/crates/socket-patch-cli/src/ui/mod.rs b/crates/socket-patch-cli/src/ui/mod.rs index 53dce36cd..7ff7f4a75 100644 --- a/crates/socket-patch-cli/src/ui/mod.rs +++ b/crates/socket-patch-cli/src/ui/mod.rs @@ -25,9 +25,49 @@ pub(crate) use prompt::confirm; pub use prompt::{select_one, SelectError}; pub(crate) use status::StatusLine; pub(crate) use text::{ - manifest_error_message, next_steps, plural, sentence_case, short_uuid, sweep_failure, truncate, + manifest_error_message, next_steps, plural, sentence_case, shell_word, short_uuid, + sweep_failure, truncate, }; +/// A global run's scope as one pasteable argument: `--global-prefix ` +/// (shell-quoted when needed) or `-g`; `None` for a project run. +pub(crate) fn global_scope_arg(common: &GlobalArgs) -> Option { + 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, + } +} + +/// The flags that pick WHICH tree a run acts on, as a pasteable suffix +/// (leading space, empty for a default run): `--cwd`, `--manifest-path`, +/// `-g` / `--global-prefix` and `--ecosystems`, each only when it differs +/// from its default. A remedy a command prints repeats them, so running it +/// verbatim acts on the tree the command just reported on (#464, #1219). +pub(crate) fn scope_args(common: &GlobalArgs) -> String { + let mut out = String::new(); + if common.cwd != std::path::Path::new(".") { + out.push_str(" --cwd "); + out.push_str(&shell_word(&common.cwd.to_string_lossy())); + } + if common.manifest_path != socket_patch_core::constants::DEFAULT_PATCH_MANIFEST_PATH { + out.push_str(" --manifest-path "); + out.push_str(&shell_word(&common.manifest_path)); + } + if let Some(global) = global_scope_arg(common) { + out.push(' '); + out.push_str(&global); + } + if let Some(ecosystems) = common.ecosystems.as_deref().filter(|e| !e.is_empty()) { + out.push_str(" --ecosystems "); + out.push_str(&shell_word(&ecosystems.join(","))); + } + out +} + /// Where a physical copy lives, relative to `cwd` when it is inside it. /// Shared by `apply` and `rollback`. pub(crate) fn display_copy_path(package_path: &str, cwd: &std::path::Path) -> String { diff --git a/crates/socket-patch-cli/src/ui/text.rs b/crates/socket-patch-cli/src/ui/text.rs index 02a3021e6..a983a1464 100644 --- a/crates/socket-patch-cli/src/ui/text.rs +++ b/crates/socket-patch-cli/src/ui/text.rs @@ -175,6 +175,28 @@ pub fn truncate(s: &str, max: usize) -> String { format!("{}{ELLIPSIS}", head.trim_end()) } +/// `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). 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. +pub(crate) 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) { + let trailing = word.len() - word.trim_end_matches('\\').len(); + format!("\"{word}{}\"", "\\".repeat(trailing)) + } else { + format!("'{}'", word.replace('\'', r"'\''")) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/socket-patch-cli/tests/apply/check_verifies_installed_tree.rs b/crates/socket-patch-cli/tests/apply/check_verifies_installed_tree.rs index da78eeb33..5d44fc7b6 100644 --- a/crates/socket-patch-cli/tests/apply/check_verifies_installed_tree.rs +++ b/crates/socket-patch-cli/tests/apply/check_verifies_installed_tree.rs @@ -339,3 +339,119 @@ fn check_fails_on_an_unpatched_copy_in_a_refused_bundle_path() { // The refused root is only read: the loaded copy stays unpatched. assert_eq!(std::fs::read(&loaded).unwrap(), GEM_ORIGINAL); } + +/// The `Run \`socket-patch …\`` remedy that ends a human drift report, as +/// argv (without the leading `socket-patch`). Undoes the quoting +/// `ui::shell_word` applies (POSIX `'…'`, Windows `"…"`). +fn printed_remedy(stderr: &str) -> Vec { + let line = stderr + .lines() + .find(|l| l.starts_with("Run `socket-patch ")) + .unwrap_or_else(|| panic!("no remedy line in {stderr}")); + let command = line + .strip_prefix("Run `socket-patch ") + .and_then(|rest| rest.split_once('`')) + .map(|(command, _)| command) + .unwrap(); + let mut words = Vec::new(); + let mut word = String::new(); + let mut quote: Option = None; + let mut in_word = false; + for c in command.chars() { + match (quote, c) { + (Some(q), c) if c == q => quote = None, + (Some(_), c) => word.push(c), + (None, '\'' | '"') => { + quote = Some(c); + in_word = true; + } + (None, ' ') => { + if in_word { + words.push(std::mem::take(&mut word)); + in_word = false; + } + } + (None, c) => { + word.push(c); + in_word = true; + } + } + } + if in_word { + words.push(word); + } + words +} + +/// Run `remedy` (+ `--offline`) as printed, then re-check with `check`. +fn run_remedy_then_check(cwd: &Path, remedy: &[String], check: &[&str]) -> (i32, String) { + let mut argv: Vec<&str> = remedy.iter().map(String::as_str).collect(); + argv.push("--offline"); + let (code, stdout, stderr) = run_with_env(cwd, &argv, &[("SOCKET_TELEMETRY_DISABLED", "1")]); + assert_eq!( + code, 0, + "the printed remedy runs\nstdout={stdout}\nstderr={stderr}" + ); + let (code, _stdout, stderr) = run_check(cwd, check); + (code, stderr) +} + +/// Stores the patched bytes as the after-hash blob, so `apply --offline` +/// can heal the tree. +fn store_after_blob(project_root: &Path) { + let blobs = project_root.join(".socket/blobs"); + std::fs::create_dir_all(&blobs).unwrap(); + std::fs::write(blobs.join(git_sha256(PATCHED)), PATCHED).unwrap(); +} + +/// #1219: a `--cwd` check's remedy keeps `--cwd`. Before, it said plain +/// `socket-patch apply`, which from the parent dir found no manifest, +/// exited 0 and left the project unpatched. +#[test] +fn check_remedy_keeps_cwd_and_heals_the_checked_tree() { + let tmp = project(Some(ORIGINAL)); + store_after_blob(tmp.path()); + let parent = tmp.path().parent().unwrap(); + let dir = tmp.path().file_name().unwrap().to_str().unwrap(); + + let (code, stdout, stderr) = run_check(parent, &["--cwd", dir]); + assert_eq!(code, 1, "drift\nstdout={stdout}\nstderr={stderr}"); + let remedy = printed_remedy(&stderr); + assert_eq!(remedy, ["apply", "--cwd", dir], "{stderr}"); + + let (code, stderr) = run_remedy_then_check(parent, &remedy, &["--cwd", dir]); + assert_eq!(code, 0, "the remedy healed the checked tree: {stderr}"); + assert_eq!( + std::fs::read(tmp.path().join("node_modules/check-target/index.js")).unwrap(), + PATCHED + ); +} + +/// #1219: a `--global-prefix` check's remedy keeps the prefix (quoted when +/// it holds a space). Before, plain `socket-patch apply` patched the cwd +/// project, reported the patch as not found and exited 0. +#[test] +fn check_remedy_keeps_global_prefix_and_heals_the_checked_tree() { + let tmp = project(None); + store_after_blob(tmp.path()); + let prefix = tmp.path().join("global lib").join("node_modules"); + let index = prefix.join("check-target/index.js"); + std::fs::create_dir_all(index.parent().unwrap()).unwrap(); + std::fs::write( + prefix.join("check-target/package.json"), + r#"{ "name": "check-target", "version": "1.0.0" }"#, + ) + .unwrap(); + std::fs::write(&index, ORIGINAL).unwrap(); + let prefix_arg = prefix.to_str().unwrap(); + + let (code, stdout, stderr) = run_check(tmp.path(), &["--global-prefix", prefix_arg]); + assert_eq!(code, 1, "drift\nstdout={stdout}\nstderr={stderr}"); + let remedy = printed_remedy(&stderr); + assert_eq!(remedy, ["apply", "--global-prefix", prefix_arg], "{stderr}"); + + let (code, stderr) = + run_remedy_then_check(tmp.path(), &remedy, &["--global-prefix", prefix_arg]); + assert_eq!(code, 0, "the remedy healed the checked tree: {stderr}"); + assert_eq!(std::fs::read(&index).unwrap(), PATCHED); +}