From c0e4ec0002dfe72f84b03ff72119abd18063dc3a Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 11:10:07 -0400 Subject: [PATCH 01/10] Route remove and rollback through one core target grammar Add socket_patch_core::utils::target: Target::parse classifies a token as a UUID, CVE, GHSA, purl or exact package name, and Target::matches_patch / matches_package match records and installed packages through it (names via policy::package_spec_matches, the matcher scan --package and socket.yml already use). Delete utils::purl::patch_matches and the api client's private is_valid_uuid copy; Ledgers::matching, VendorEntry::matches_target (renamed from matches_identifier), remove and rollback now take a parsed Target. remove and rollback accept a bare name and a versionless purl (every recorded version) instead of reporting 'No patch found', and rollback treats an npm @scope/name as a name rather than a path glob. Part of architecture audit theme 3.B (package target grammar). Co-Authored-By: Claude Opus 5.5 (1M context) --- .../socket-patch-cli/src/commands/remove.rs | 127 ++++-- .../socket-patch-cli/src/commands/rollback.rs | 106 +++-- crates/socket-patch-core/src/api/client.rs | 42 +- crates/socket-patch-core/src/ledgers.rs | 25 +- crates/socket-patch-core/src/utils/mod.rs | 1 + crates/socket-patch-core/src/utils/purl.rs | 41 -- crates/socket-patch-core/src/utils/target.rs | 388 ++++++++++++++++++ crates/socket-patch-core/src/vendor/state.rs | 39 +- 8 files changed, 608 insertions(+), 161 deletions(-) create mode 100644 crates/socket-patch-core/src/utils/target.rs diff --git a/crates/socket-patch-cli/src/commands/remove.rs b/crates/socket-patch-cli/src/commands/remove.rs index c5fbed562..22e26656d 100644 --- a/crates/socket-patch-cli/src/commands/remove.rs +++ b/crates/socket-patch-cli/src/commands/remove.rs @@ -5,7 +5,7 @@ use socket_patch_core::manifest::operations::{read_manifest, write_manifest}; use socket_patch_core::manifest::schema::PatchManifest; use socket_patch_core::patch::redirect::upstream::HostedPin; use socket_patch_core::telemetry::{track_patch_remove_failed, track_patch_removed}; -use socket_patch_core::utils::purl::patch_matches; +use socket_patch_core::utils::target::Target; use socket_patch_core::vendor::{ load_state, RevertOpts, VendorEntry, VendorState, VENDOR_STATE_REL, }; @@ -22,43 +22,43 @@ use crate::commands::vendored_backend::{RevertedEntry, VendorRevertStep, Vendore use crate::json_envelope::{Command, Envelope, EnvelopeError, PatchAction, PatchEvent, Status}; use crate::ui::plural; -/// Vendor-ledger entries matching a remove identifier +/// Vendor-ledger entries matching a remove target /// ([`socket_patch_core::ledgers::Ledgers::matching`]), sorted by key for /// deterministic event order. -fn vendor_entries_matching(state: &VendorState, identifier: &str) -> Vec<(String, VendorEntry)> { +fn vendor_entries_matching(state: &VendorState, target: &Target) -> Vec<(String, VendorEntry)> { socket_patch_core::ledgers::Ledgers { vendor: Some(state), ..Default::default() } - .matching(identifier) + .matching(target) .vendor } -/// The lockfiles' hosted pins matching a remove identifier (by purl or +/// The lockfiles' hosted pins matching a remove target (by purl, name or /// patch uuid), sorted by purl. -fn hosted_pins_matching(pins: &[HostedPin], identifier: &str) -> Vec { +fn hosted_pins_matching(pins: &[HostedPin], target: &Target) -> Vec { let mut matches: Vec = pins .iter() - .filter(|pin| patch_matches(&pin.purl, &pin.uuid, identifier)) + .filter(|pin| target.matches_patch(&pin.purl, &pin.uuid)) .cloned() .collect(); matches.sort_by(|a, b| a.purl.cmp(&b.purl)); matches } -/// Drop every manifest entry matching `identifier` except `exclusions` +/// Drop every manifest entry matching `target` except `exclusions` /// (drift-kept vendored purls, whose record must survive with their /// vendored state). Returns the removed purls, sorted. fn remove_matching( manifest: &mut PatchManifest, - identifier: &str, + target: &Target, exclusions: &HashSet, ) -> Vec { let mut removed: Vec = manifest .patches .iter() .filter(|(purl, patch)| { - patch_matches(purl, &patch.uuid, identifier) && !exclusions.contains(*purl) + target.matches_patch(purl, &patch.uuid) && !exclusions.contains(*purl) }) .map(|(purl, _)| purl.clone()) .collect(); @@ -271,7 +271,8 @@ fn format_blob_sweep( #[derive(Args)] pub struct RemoveArgs { - /// Package PURL or patch UUID. + /// Patch UUID, package PURL (`pkg:npm/lodash@4.17.21`, or versionless + /// for every version), or exact package name (`lodash`) pub identifier: String, #[command(flatten)] @@ -313,6 +314,9 @@ const DRY_RUN_FOOTER: &str = "Dry run: no changes made."; pub async fn run(args: RemoveArgs) -> i32 { apply_env_toggles(&args.common); + // The shared target grammar: a UUID, a purl (versioned or not) or an + // exact package name. Every store below matches through it. + let target = Target::parse(&args.identifier); // Self-enforced usage error (exit 2, like scan's mode conflicts): // `--skip-rollback` keeps the tree and drops the state, @@ -441,7 +445,7 @@ pub async fn run(args: RemoveArgs) -> i32 { let mut matching: Vec<_> = manifest .patches .iter() - .filter(|(purl, patch)| patch_matches(purl, &patch.uuid, &args.identifier)) + .filter(|(purl, patch)| target.matches_patch(purl, &patch.uuid)) .collect(); matching.sort_by(|a, b| a.0.cmp(b.0)); @@ -462,7 +466,7 @@ pub async fn run(args: RemoveArgs) -> i32 { // --revert`'s all-at-once). An unreadable ledger falls through to // `not_found`: nothing is mutated on that path. if let Ok(state) = vendor_state_result { - let ledger_matches = vendor_entries_matching(&state, &args.identifier); + let ledger_matches = vendor_entries_matching(&state, &target); if !ledger_matches.is_empty() { return remove_ledger_only( &args, @@ -478,7 +482,7 @@ pub async fn run(args: RemoveArgs) -> i32 { // Hosted-only patches likewise have no manifest entry — their // lockfile pins are their only persistence, and `remove` is their // per-purl exit path (restoring the upstream entry IS the removal). - let hosted_matches = hosted_pins_matching(&hosted_pins, &args.identifier); + let hosted_matches = hosted_pins_matching(&hosted_pins, &target); if !hosted_matches.is_empty() { return remove_hosted_only( &args, @@ -505,9 +509,10 @@ pub async fn run(args: RemoveArgs) -> i32 { // blast radius explicit so the user understands why a single // `remove pkg:pypi/foo@1.0` is removing several variants. if loud { - let variants = args.identifier.starts_with("pkg:") - && !args.identifier.contains('?') - && matching.len() > 1; + // Only an exact base purl expands to release variants; a name or + // versionless purl matching several entries is "several patches". + let variants = + target.is_versioned_purl() && !args.identifier.contains('?') && matching.len() > 1; eprintln!( "{}", format_remove_header( @@ -541,9 +546,9 @@ pub async fn run(args: RemoveArgs) -> i32 { } else { let vendored = vendor_state_result .as_ref() - .map(|st| vendor_entries_matching(st, &args.identifier).len()) + .map(|st| vendor_entries_matching(st, &target).len()) .unwrap_or(0); - let hosted = hosted_pins_matching(&hosted_pins, &args.identifier).len(); + let hosted = hosted_pins_matching(&hosted_pins, &target).len(); (vendored, hosted) }; let prompt = remove_prompt( @@ -602,7 +607,7 @@ pub async fn run(args: RemoveArgs) -> i32 { &socket_dir, &manifest, &vendored_keys, - InnerSelection::Identifier(Some(&args.identifier)), + InnerSelection::Identifier(Some(&target)), Some(&telemetry_client), ) .await @@ -722,7 +727,7 @@ pub async fn run(args: RemoveArgs) -> i32 { return 1; } }; - let vendored_matches = vendor_entries_matching(&vendor_state, &args.identifier); + let vendored_matches = vendor_entries_matching(&vendor_state, &target); let mut vendor_leg = RemoveVendorLeg::default(); if !vendored_matches.is_empty() { if args.skip_rollback { @@ -771,7 +776,7 @@ pub async fn run(args: RemoveArgs) -> i32 { // carried into the success envelope's `warnings[]`. let mut hosted_leg_warnings: Vec<(String, String)> = Vec::new(); if !args.skip_rollback { - let hosted_matches = hosted_pins_matching(&hosted_pins, &args.identifier); + let hosted_matches = hosted_pins_matching(&hosted_pins, &target); if !hosted_matches.is_empty() { let leg = match unwind_hosted(&args.common, &hosted_matches).await { Ok(leg) => { @@ -837,7 +842,7 @@ pub async fn run(args: RemoveArgs) -> i32 { let removed = if args.preserve_state { Vec::new() } else { - remove_matching(&mut updated_manifest, &args.identifier, &excluded_kept) + remove_matching(&mut updated_manifest, &target, &excluded_kept) }; if removed.is_empty() && !args.preserve_state { // Every matching entry was drift-kept (the identifier matched, so @@ -1629,7 +1634,11 @@ mod tests { fn remove_base_purl_removes_all_variants() { let mut manifest = multi_variant_manifest(); - let removed = remove_matching(&mut manifest, "pkg:pypi/six@1.16.0", &Default::default()); + let removed = remove_matching( + &mut manifest, + &Target::parse("pkg:pypi/six@1.16.0"), + &Default::default(), + ); // All three release variants removed (sorted); the npm package untouched. assert_eq!(removed.len(), 3); @@ -1648,7 +1657,7 @@ mod tests { let removed = remove_matching( &mut manifest, - "pkg:pypi/six@1.16.0?artifact_id=sdist", + &Target::parse("pkg:pypi/six@1.16.0?artifact_id=sdist"), &Default::default(), ); @@ -1664,7 +1673,11 @@ mod tests { fn remove_by_uuid_removes_single_variant() { let mut manifest = multi_variant_manifest(); - let removed = remove_matching(&mut manifest, "uuid-cp312", &Default::default()); + let removed = remove_matching( + &mut manifest, + &Target::parse("uuid-cp312"), + &Default::default(), + ); assert_eq!(removed, vec!["pkg:pypi/six@1.16.0?artifact_id=wheel-cp312"]); assert_eq!(manifest.patches.len(), 3); @@ -1684,7 +1697,11 @@ mod tests { setup: None, }; - let removed = remove_matching(&mut manifest, "pkg:npm/foo@1.0", &Default::default()); + let removed = remove_matching( + &mut manifest, + &Target::parse("pkg:npm/foo@1.0"), + &Default::default(), + ); assert_eq!(removed, vec!["pkg:npm/foo@1.0"]); assert_eq!(manifest.patches.len(), 1); @@ -1701,12 +1718,54 @@ mod tests { let mut manifest = multi_variant_manifest(); let before = manifest.clone(); - let removed = remove_matching(&mut manifest, "pkg:npm/not-here@9.9.9", &Default::default()); + let removed = remove_matching( + &mut manifest, + &Target::parse("pkg:npm/not-here@9.9.9"), + &Default::default(), + ); assert!(removed.is_empty(), "nothing should match"); assert_eq!(manifest, before, "manifest left intact"); } + /// The shared target grammar: a bare name and a versionless purl + /// select every recorded version (`remove six` used to be "No patch + /// found"), and a name stays exact (`six` is not `sixer`). + #[test] + fn remove_by_name_or_versionless_purl_removes_every_version() { + for token in ["six", "pkg:pypi/six"] { + let mut manifest = multi_variant_manifest(); + manifest + .patches + .insert("pkg:pypi/six@1.17.0".to_string(), make_record("uuid-17")); + manifest.patches.insert( + "pkg:pypi/sixer@1.0.0".to_string(), + make_record("uuid-sixer"), + ); + let removed = + remove_matching(&mut manifest, &Target::parse(token), &Default::default()); + assert!( + removed.contains(&"pkg:pypi/six@1.17.0".to_string()), + "{token}: {removed:?}" + ); + assert!( + removed.iter().all(|p| p.starts_with("pkg:pypi/six@")), + "{token}: {removed:?}" + ); + assert!( + manifest.patches.contains_key("pkg:pypi/sixer@1.0.0"), + "{token}" + ); + assert!( + !manifest + .patches + .keys() + .any(|k| k.starts_with("pkg:pypi/six@")), + "{token}" + ); + } + } + /// A base PURL must not bleed across versions: removing `six@1.16.0` /// leaves `six@1.17.0` (and its variants) in place. #[test] @@ -1725,7 +1784,11 @@ mod tests { setup: None, }; - let removed = remove_matching(&mut manifest, "pkg:pypi/six@1.16.0", &Default::default()); + let removed = remove_matching( + &mut manifest, + &Target::parse("pkg:pypi/six@1.16.0"), + &Default::default(), + ); assert_eq!(removed, vec!["pkg:pypi/six@1.16.0?artifact_id=sdist"]); assert_eq!(manifest.patches.len(), 1); @@ -1742,7 +1805,11 @@ mod tests { let exclusions: HashSet = ["pkg:pypi/six@1.16.0?artifact_id=sdist".to_string()].into(); - let removed = remove_matching(&mut manifest, "pkg:pypi/six@1.16.0", &exclusions); + let removed = remove_matching( + &mut manifest, + &Target::parse("pkg:pypi/six@1.16.0"), + &exclusions, + ); assert_eq!( removed.len(), diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index c3a7f8592..b0bf16905 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -14,7 +14,8 @@ use socket_patch_core::patch::rollback::{ VerifyRollbackResult, VerifyRollbackStatus, }; use socket_patch_core::telemetry::{track_patch_rollback_failed, track_patch_rolled_back}; -use socket_patch_core::utils::purl::{patch_matches, strip_purl_qualifiers}; +use socket_patch_core::utils::purl::strip_purl_qualifiers; +use socket_patch_core::utils::target::{is_path_shaped, Target, TargetKind}; use socket_patch_core::vendor::{purl_keys_cover, RevertOpts, VendorState}; use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; @@ -26,7 +27,6 @@ use crate::commands::lock_cli::acquire_or_emit; use crate::commands::vendored_backend::{RevertedEntry, VendorRevertStep, VendoredBackend}; use crate::ecosystem_dispatch::{find_all_packages_for_rollback, partition_purls, JvmScope}; use crate::json_envelope::Command as EnvelopeCommand; -use crate::looks_like_uuid; use crate::ui::{plural, StatusLine}; #[derive(Args)] @@ -374,31 +374,25 @@ fn format_reinstall_note(still_patched: usize, dry_run: bool) -> String { /// One classified rollback target token. #[derive(Debug, Clone, PartialEq)] pub(crate) enum RollbackTarget { - /// PURL or UUID — today's `patch_matches` semantics. - Identifier(String), + /// A UUID, purl or package name in the shared target grammar + /// ([`Target::matches_patch`]). + Identifier(Target), /// A path glob scoping the run to patches with an installed copy /// under a matching path. PathGlob(String), } /// Shape-classify a target token. Only path-SHAPED tokens become globs -/// (separator, glob metachar, `./` prefix, or absolute); `pkg:` and every -/// other bare word keep identifier semantics, so a truncated UUID or a -/// package name typed without its `pkg:` prefix stays a safe -/// "No patch found matching identifier" error instead of silently +/// ([`is_path_shaped`]: separator, glob metachar, `./` prefix, or absolute; +/// an npm `@scope/name` is a name); `pkg:` and every other token keep the +/// shared target grammar, so a truncated UUID or a mistyped name stays a +/// safe "No patch found matching identifier" error instead of silently /// selecting a directory subtree. pub(crate) fn classify_target(token: &str) -> RollbackTarget { - if token.starts_with("pkg:") { - return RollbackTarget::Identifier(token.to_string()); - } - let path_shaped = token.contains('/') - || token.contains('\\') - || token.contains(['*', '?', '[']) - || Path::new(token).is_absolute(); - if path_shaped { + if is_path_shaped(token) { RollbackTarget::PathGlob(token.to_string()) } else { - RollbackTarget::Identifier(token.to_string()) + RollbackTarget::Identifier(Target::parse(token)) } } @@ -450,7 +444,7 @@ pub(crate) enum InnerSelection<'a> { /// The legacy single-identifier filter (`remove`'s delegation): a /// no-match identifier is an error, a missing manifest is an error, /// and `None` selects the whole manifest. - Identifier(Option<&'a str>), + Identifier(Option<&'a Target>), /// A pre-resolved purl set from the CLI boundary's target resolver /// (identifiers ∪ path globs ∪ everything). No-match and /// missing-manifest handling already happened upstream, so an empty @@ -545,12 +539,12 @@ async fn try_rollback_local_go( fn find_patches_to_rollback( manifest: &PatchManifest, - identifier: Option<&str>, + target: Option<&Target>, ) -> Vec { manifest .patches .iter() - .filter(|(purl, patch)| identifier.is_none_or(|id| patch_matches(purl, &patch.uuid, id))) + .filter(|(purl, patch)| target.is_none_or(|t| t.matches_patch(purl, &patch.uuid))) .map(|(purl, patch)| PatchToRollback { purl: purl.clone(), patch: patch.clone(), @@ -996,7 +990,7 @@ pub async fn run(args: RollbackArgs) -> i32 { // Classify targets up front: the glob validation is a pre-network // usage check. - let mut identifiers: Vec = Vec::new(); + let mut identifiers: Vec = Vec::new(); let mut path_patterns: Vec = Vec::new(); for token in &args.targets { match classify_target(token) { @@ -1246,13 +1240,13 @@ pub async fn run(args: RollbackArgs) -> i32 { vendor_scope.extend(found.vendor.into_iter().map(|(k, _)| k)); // Hosted pins live in the lockfiles, not in a store. for (purl, uuid) in &redirect_records { - if patch_matches(purl, uuid, id) { + if id.matches_patch(purl, uuid) { hosted_scope.insert(purl.clone()); matched = true; } } if !matched { - let hint = if id.starts_with("pkg:") || looks_like_uuid(id) { + let hint = if matches!(id.kind(), TargetKind::Purl | TargetKind::Uuid) { String::new() } else { format!(" (to target a directory instead, use ./{id} or {id}/**)") @@ -2024,9 +2018,7 @@ pub(crate) async fn rollback_patches_inner( let mut blobs_path = socket_dir.join("blobs"); let patches_to_rollback = match &selection { - InnerSelection::Identifier(identifier) => { - find_patches_to_rollback(manifest, identifier.as_deref()) - } + InnerSelection::Identifier(identifier) => find_patches_to_rollback(manifest, *identifier), InnerSelection::Scope { purls, .. } => manifest .patches .iter() @@ -3077,12 +3069,13 @@ mod tests { dry_run, ..common.clone() }; + let target = identifier.map(Target::parse); let outcome = rollback_patches_inner( &delegated_common, &socket_dir, &manifest, &vendored_keys, - InnerSelection::Identifier(identifier), + InnerSelection::Identifier(target.as_ref()), None, ) .await?; @@ -3127,7 +3120,7 @@ mod tests { #[test] fn test_find_patches_to_rollback_purl_match() { let manifest = make_manifest(); - let result = find_patches_to_rollback(&manifest, Some("pkg:npm/foo@1.0")); + let result = find_patches_to_rollback(&manifest, Some(&Target::parse("pkg:npm/foo@1.0"))); assert_eq!(result.len(), 1); assert_eq!(result[0].purl, "pkg:npm/foo@1.0"); } @@ -3135,14 +3128,15 @@ mod tests { #[test] fn test_find_patches_to_rollback_purl_no_match() { let manifest = make_manifest(); - let result = find_patches_to_rollback(&manifest, Some("pkg:npm/nonexistent@1")); + let result = + find_patches_to_rollback(&manifest, Some(&Target::parse("pkg:npm/nonexistent@1"))); assert!(result.is_empty()); } #[test] fn test_find_patches_to_rollback_uuid_match() { let manifest = make_manifest(); - let result = find_patches_to_rollback(&manifest, Some("uuid-bar")); + let result = find_patches_to_rollback(&manifest, Some(&Target::parse("uuid-bar"))); assert_eq!(result.len(), 1); assert_eq!(result[0].patch.uuid, "uuid-bar"); assert_eq!(result[0].purl, "pkg:npm/bar@2.0"); @@ -3151,7 +3145,8 @@ mod tests { #[test] fn test_find_patches_to_rollback_uuid_no_match() { let manifest = make_manifest(); - let result = find_patches_to_rollback(&manifest, Some("uuid-does-not-exist")); + let result = + find_patches_to_rollback(&manifest, Some(&Target::parse("uuid-does-not-exist"))); assert!(result.is_empty()); } @@ -3181,7 +3176,8 @@ mod tests { #[test] fn test_find_patches_to_rollback_base_purl_matches_all_variants() { let manifest = make_multi_variant_manifest(); - let result = find_patches_to_rollback(&manifest, Some("pkg:pypi/six@1.16.0")); + let result = + find_patches_to_rollback(&manifest, Some(&Target::parse("pkg:pypi/six@1.16.0"))); // Base PURL (no qualifier) expands to every release variant. assert_eq!(result.len(), 3); for p in &result { @@ -3192,17 +3188,57 @@ mod tests { #[test] fn test_find_patches_to_rollback_qualified_purl_matches_one_variant() { let manifest = make_multi_variant_manifest(); - let result = - find_patches_to_rollback(&manifest, Some("pkg:pypi/six@1.16.0?artifact_id=sdist")); + let result = find_patches_to_rollback( + &manifest, + Some(&Target::parse("pkg:pypi/six@1.16.0?artifact_id=sdist")), + ); // A fully-qualified PURL targets exactly one variant. assert_eq!(result.len(), 1); assert_eq!(result[0].purl, "pkg:pypi/six@1.16.0?artifact_id=sdist"); } + /// Rollback shares the target grammar: a bare name (and an npm + /// `@scope/name`) is a package target, not a uuid-only identifier or a + /// path glob; only path-shaped tokens are globs. + #[test] + fn classify_target_uses_the_shared_grammar() { + assert!(matches!( + classify_target("lodash"), + RollbackTarget::Identifier(_) + )); + assert!(matches!( + classify_target("@babel/core"), + RollbackTarget::Identifier(_) + )); + assert!(matches!( + classify_target("pkg:npm/@s/x"), + RollbackTarget::Identifier(_) + )); + assert!(matches!( + classify_target("./node_modules"), + RollbackTarget::PathGlob(_) + )); + assert!(matches!( + classify_target("node_modules/**"), + RollbackTarget::PathGlob(_) + )); + let manifest = make_manifest(); + let result = find_patches_to_rollback(&manifest, Some(&Target::parse("foo"))); + assert_eq!(result.len(), 1, "a bare name selects its recorded patch"); + assert_eq!(result[0].purl, "pkg:npm/foo@1.0"); + let result = find_patches_to_rollback(&manifest, Some(&Target::parse("pkg:npm/foo"))); + assert_eq!( + result.len(), + 1, + "a versionless purl selects its recorded patch" + ); + } + #[test] fn test_find_patches_to_rollback_base_purl_does_not_leak_other_packages() { let manifest = make_multi_variant_manifest(); - let result = find_patches_to_rollback(&manifest, Some("pkg:pypi/six@1.16.0")); + let result = + find_patches_to_rollback(&manifest, Some(&Target::parse("pkg:pypi/six@1.16.0"))); assert!(result.iter().all(|p| p.purl.contains("six@1.16.0"))); } diff --git a/crates/socket-patch-core/src/api/client.rs b/crates/socket-patch-core/src/api/client.rs index 642213af4..4381826f2 100644 --- a/crates/socket-patch-core/src/api/client.rs +++ b/crates/socket-patch-core/src/api/client.rs @@ -24,6 +24,7 @@ use crate::utils::digest::is_hex; use crate::utils::env_compat::{is_debug_enabled, is_offline_env, proxy_url_from_env}; use crate::utils::notice::{notice_once, Notice}; use crate::utils::socket_cli_config; +use crate::utils::target::is_uuid_shaped; // Each client advisory prints at most once per process: commands build // several clients (telemetry, discovery, download) in one run. @@ -1072,7 +1073,7 @@ impl ApiClient { /// `/patch/diff/`; the authenticated API serves them under /// `/v0/orgs//patches/diff/`. pub async fn fetch_diff(&self, uuid: &str) -> Result, ApiError> { - if !is_valid_uuid(uuid) { + if !is_uuid_shaped(uuid) { return Err(ApiError::InvalidHash(format!( "Invalid patch UUID: {}", uuid @@ -1204,7 +1205,7 @@ impl ApiClient { vendor_url: Option<&str>, patch_server_url: Option<&str>, ) -> VendorServiceOutcome { - if !is_valid_uuid(uuid) { + if !is_uuid_shaped(uuid) { return VendorServiceOutcome::Failed(ApiError::InvalidHash(format!( "Invalid patch UUID: {uuid}" ))); @@ -1303,7 +1304,7 @@ impl ApiClient { ) -> VendorPrefetchGuard { let planned: Vec = downloads .into_iter() - .filter(|d| is_valid_uuid(&d.uuid)) + .filter(|d| is_uuid_shaped(&d.uuid)) .collect(); let plan = Arc::new(VendorPrefetch::new( planned, @@ -2542,19 +2543,6 @@ fn truncate_to_chars(s: &str, max_chars: usize) -> String { format!("{}...", truncated) } -/// Validate the standard 8-4-4-4-12 UUID hex grouping. -fn is_valid_uuid(s: &str) -> bool { - let parts: Vec<&str> = s.split('-').collect(); - if parts.len() != 5 { - return false; - } - let lengths = [8, 4, 4, 4, 12]; - parts - .iter() - .zip(lengths.iter()) - .all(|(part, &want)| part.len() == want && part.bytes().all(|b| b.is_ascii_hexdigit())) -} - /// Convert a `PatchSearchResult` into a `BatchPatchInfo`, extracting /// CVE/GHSA IDs and computing the highest severity. fn convert_search_result_to_batch_info(patch: PatchSearchResult) -> BatchPatchInfo { @@ -3662,25 +3650,25 @@ mod tests { // ── UUID validation tests ─────────────────────────────────────── #[test] - fn test_is_valid_uuid_accepts_standard_form() { - assert!(is_valid_uuid("80630680-4da6-45f9-bba8-b888e0ffd58c")); - assert!(is_valid_uuid("00000000-0000-0000-0000-000000000000")); + fn test_is_uuid_shaped_accepts_standard_form() { + assert!(is_uuid_shaped("80630680-4da6-45f9-bba8-b888e0ffd58c")); + assert!(is_uuid_shaped("00000000-0000-0000-0000-000000000000")); // Uppercase hex is acceptable. - assert!(is_valid_uuid("ABCDEF01-2345-6789-ABCD-EF0123456789")); + assert!(is_uuid_shaped("ABCDEF01-2345-6789-ABCD-EF0123456789")); } #[test] - fn test_is_valid_uuid_rejects_malformed() { - assert!(!is_valid_uuid("")); - assert!(!is_valid_uuid("not-a-uuid")); + fn test_is_uuid_shaped_rejects_malformed() { + assert!(!is_uuid_shaped("")); + assert!(!is_uuid_shaped("not-a-uuid")); // Wrong segment count. - assert!(!is_valid_uuid("80630680-4da6-45f9-bba8")); + assert!(!is_uuid_shaped("80630680-4da6-45f9-bba8")); // Wrong length on first segment. - assert!(!is_valid_uuid("8063068-4da6-45f9-bba8-b888e0ffd58c")); + assert!(!is_uuid_shaped("8063068-4da6-45f9-bba8-b888e0ffd58c")); // Non-hex character. - assert!(!is_valid_uuid("80630680-4da6-45f9-bba8-b888e0ffd58z")); + assert!(!is_uuid_shaped("80630680-4da6-45f9-bba8-b888e0ffd58z")); // No dashes. - assert!(!is_valid_uuid("80630680xxxxx")); + assert!(!is_uuid_shaped("80630680xxxxx")); } // ── fetch_diff validation tests ────────────────────────────────── diff --git a/crates/socket-patch-core/src/ledgers.rs b/crates/socket-patch-core/src/ledgers.rs index 9bcf56623..7227f7ecb 100644 --- a/crates/socket-patch-core/src/ledgers.rs +++ b/crates/socket-patch-core/src/ledgers.rs @@ -28,7 +28,8 @@ use std::path::Path; use crate::manifest::schema::{PatchManifest, PatchRecord}; use crate::patch::redirect::{CorruptRedirectState, RedirectState}; -use crate::utils::purl::{normalize_purl, patch_matches, strip_purl_qualifiers}; +use crate::utils::purl::{normalize_purl, strip_purl_qualifiers}; +use crate::utils::target::Target; use crate::vendor::{VendorEntry, VendorState}; /// A patch store, in owner-precedence order (a lower store wins a key). @@ -293,16 +294,15 @@ impl<'a> Ledgers<'a> { out } - /// Every entry a remove/rollback `identifier` (purl or uuid) matches: - /// manifest and hosted records by [`patch_matches`] on their key, - /// vendored entries by [`VendorEntry::matches_identifier`] (key or base - /// purl). - pub fn matching(&self, identifier: &str) -> Matches { + /// Every entry a remove/rollback `target` matches: manifest and hosted + /// records by [`Target::matches_patch`] on their key, vendored entries + /// by [`VendorEntry::matches_target`] (key or base purl). + pub fn matching(&self, target: &Target) -> Matches { let mut manifest: Vec = self .manifest .into_iter() .flat_map(|m| m.patches.iter()) - .filter(|(key, rec)| patch_matches(key, &rec.uuid, identifier)) + .filter(|(key, rec)| target.matches_patch(key, &rec.uuid)) .map(|(key, _)| key.clone()) .collect(); manifest.sort(); @@ -310,7 +310,7 @@ impl<'a> Ledgers<'a> { .vendor .into_iter() .flat_map(|s| s.entries.iter()) - .filter(|(key, entry)| entry.matches_identifier(key, identifier)) + .filter(|(key, entry)| entry.matches_target(key, target)) .map(|(k, e)| (k.clone(), e.clone())) .collect(); vendor.sort_by(|a, b| a.0.cmp(&b.0)); @@ -318,7 +318,7 @@ impl<'a> Ledgers<'a> { .redirect .into_iter() .flat_map(|r| r.records.iter()) - .filter(|(key, rec)| patch_matches(key, &rec.uuid, identifier)) + .filter(|(key, rec)| target.matches_patch(key, &rec.uuid)) .map(|(key, _)| key.clone()) .collect(); Matches { @@ -371,7 +371,6 @@ pub fn uuid_only_record(uuid: &str) -> PatchRecord { } } - /// Fold the hosted pins and the vendor ledger's patch records into the /// manifest view update detection consults. Hosted mode records purl→uuid /// ONLY in the lockfiles (`hosted_pins`, uuid only; v5 keeps no hosted @@ -569,11 +568,11 @@ mod tests { vendor: Some(&v), redirect: Some(&r), }; - let found = l.matching("pkg:npm/a@1"); + let found = l.matching(&Target::parse("pkg:npm/a@1")); assert_eq!(found.manifest, vec!["pkg:npm/a@1"]); assert_eq!(found.vendor.len(), 1); assert_eq!(found.hosted, vec!["pkg:npm/a@1"]); - assert!(l.matching("nope").is_empty()); - assert_eq!(l.matching("hb").hosted, vec!["pkg:npm/b@1"]); + assert!(l.matching(&Target::parse("nope")).is_empty()); + assert_eq!(l.matching(&Target::parse("hb")).hosted, vec!["pkg:npm/b@1"]); } } diff --git a/crates/socket-patch-core/src/utils/mod.rs b/crates/socket-patch-core/src/utils/mod.rs index efcb02f9c..e6ab6f92a 100644 --- a/crates/socket-patch-core/src/utils/mod.rs +++ b/crates/socket-patch-core/src/utils/mod.rs @@ -24,6 +24,7 @@ pub(crate) mod requirements; pub(crate) mod serde; pub mod socket_cli_config; pub mod socket_dir; +pub mod target; pub(crate) mod toml_edit_ext; pub mod uri; diff --git a/crates/socket-patch-core/src/utils/purl.rs b/crates/socket-patch-core/src/utils/purl.rs index 0962492a5..cc53d2efe 100644 --- a/crates/socket-patch-core/src/utils/purl.rs +++ b/crates/socket-patch-core/src/utils/purl.rs @@ -413,19 +413,6 @@ pub fn purl_matches_identifier(manifest_key: &str, identifier: &str) -> bool { } } -/// Does a patch (its manifest/ledger `purl` key and `uuid`) match a -/// user-supplied remove/rollback identifier? A `pkg:` identifier matches -/// by PURL with [`purl_matches_identifier`]'s variant rules (a base PURL -/// covers every release variant of that `package@version`; a qualified one -/// targets a single patch); anything else is compared to the patch uuid. -pub fn patch_matches(purl: &str, uuid: &str, identifier: &str) -> bool { - if is_purl(identifier) { - purl_matches_identifier(purl, identifier) - } else { - uuid == identifier - } -} - // ── validating builders ───────────────────────────────────────────────── // The purl builders lockfile discovery (`vex::discover`, which re-exports // them under the same names) and the lock inventory's registry views share: @@ -542,34 +529,6 @@ mod builder_tests { mod tests { use super::*; - /// `pkg:` identifiers match by PURL (base covers every variant, a - /// qualified one only its exact key); anything else is a uuid match. - #[test] - fn test_patch_matches_routes_purl_vs_uuid() { - const UUID: &str = "9f6b2c4e-1d3a-4f6b-8c2d-7e5a9b1c3d5f"; - let key = "pkg:pypi/requests@2.28.0?artifact_id=abc"; - assert!(patch_matches(key, UUID, "pkg:pypi/requests@2.28.0")); - assert!(patch_matches(key, UUID, key)); - assert!(!patch_matches( - key, - UUID, - "pkg:pypi/requests@2.28.0?artifact_id=zzz" - )); - assert!(!patch_matches(key, UUID, "pkg:pypi/requests@2.29.0")); - assert!(patch_matches(key, UUID, UUID)); - assert!(!patch_matches(key, UUID, "not-the-uuid")); - // A uuid identifier never matches by PURL text, and a PURL - // identifier is only ever compared against the purl field — a - // uuid field that happens to equal the identifier does not match. - assert!(!patch_matches( - "pkg:npm/other@1.0.0", - "pkg:pypi/requests@2.28.0", - "pkg:pypi/requests@2.28.0" - )); - assert!(!patch_matches(UUID, "other-uuid", UUID)); - assert!(!patch_matches("pkg:npm/a@1", UUID, "pkg:npm/b@1")); - } - #[test] fn test_strip_qualifiers() { assert_eq!( diff --git a/crates/socket-patch-core/src/utils/target.rs b/crates/socket-patch-core/src/utils/target.rs new file mode 100644 index 000000000..a427328b8 --- /dev/null +++ b/crates/socket-patch-core/src/utils/target.rs @@ -0,0 +1,388 @@ +//! The one package-target grammar. +//! +//! Every verb that takes a "which patch / which package" token — `get`'s +//! positional identifier, `remove`'s identifier, `rollback`'s targets, the +//! root `socket-patch ` shortcut — classifies it here, so the same +//! token means the same thing everywhere: +//! +//! * a UUID (`8-4-4-4-12` hex, any case) names one patch; +//! * `CVE-YYYY-N…` / `GHSA-xxxx-xxxx-xxxx` (any case) name an advisory; +//! * a `pkg:` token is a purl — with a version it names one release (a base +//! purl covers every release variant, a `?qualified` one exactly one +//! variant); without a version it names every installed version; +//! * anything else is a package name, matched EXACTLY (full name or last +//! segment, case-insensitive, PEP 503 for PyPI) by +//! [`crate::policy::package_spec_matches`] — the matcher `scan --package` +//! and `socket.yml` already use. There is no fuzzy matching here. +//! +//! Verbs reject the kinds they cannot act on (a CVE matches no manifest +//! entry, for example) rather than reinterpreting them. + +use std::fmt; + +use crate::policy::package_spec_matches; +use crate::utils::purl::{is_purl, purl_matches_identifier, strip_purl_qualifiers}; + +/// What a target token names. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TargetKind { + Uuid, + Cve, + Ghsa, + Purl, + /// A bare package name (`lodash`, `@scope/pkg`, `group:artifact`, + /// `github.com/org/mod`). + Name, +} + +impl fmt::Display for TargetKind { + /// User-facing vocabulary ("No patches found for CVE: …"). + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + TargetKind::Uuid => "UUID", + TargetKind::Cve => "CVE", + TargetKind::Ghsa => "GHSA", + TargetKind::Purl => "PURL", + TargetKind::Name => "package name", + }) + } +} + +/// One classified target token. The text is kept verbatim (it is what +/// error messages echo and what API searches send). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Target { + kind: TargetKind, + text: String, +} + +impl fmt::Display for Target { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.text) + } +} + +impl Target { + /// Classify `token` by shape: UUID, then CVE, then GHSA, then purl; + /// everything else is a package name. Never fails. + pub fn parse(token: &str) -> Self { + let kind = if is_uuid_shaped(token) { + TargetKind::Uuid + } else if is_cve_id(token) { + TargetKind::Cve + } else if is_ghsa_id(token) { + TargetKind::Ghsa + } else if is_purl(token) { + TargetKind::Purl + } else { + TargetKind::Name + }; + Self::with_kind(token, kind) + } + + /// A token whose kind the caller forced (`get --id/--cve/--ghsa/ + /// --package`). [`Self::shape_ok`] says whether the text fits it. + pub fn with_kind(token: &str, kind: TargetKind) -> Self { + Self { + kind, + text: token.to_string(), + } + } + + pub fn kind(&self) -> TargetKind { + self.kind + } + + pub fn as_str(&self) -> &str { + &self.text + } + + /// Does the text have its kind's shape? Always true for a parsed + /// target; a forced UUID/CVE/GHSA can fail it. Purls and names are + /// free-form. + pub fn shape_ok(&self) -> bool { + match self.kind { + TargetKind::Uuid => is_uuid_shaped(&self.text), + TargetKind::Cve => is_cve_id(&self.text), + TargetKind::Ghsa => is_ghsa_id(&self.text), + TargetKind::Purl | TargetKind::Name => true, + } + } + + /// A purl target that names an exact version (`pkg:type/name@ver`). + /// npm scope `@`s don't count: `pkg:npm/@scope/name` is versionless. + pub fn is_versioned_purl(&self) -> bool { + self.kind == TargetKind::Purl && purl_has_version(&self.text) + } + + /// Does this target select the installed package `purl`? Purls and + /// names only: a versioned purl selects that release (qualifiers + /// ignored), a versionless purl or a name selects every version. + pub fn matches_package(&self, purl: &str) -> bool { + match self.kind { + TargetKind::Purl | TargetKind::Name => package_spec_matches(&self.text, purl), + TargetKind::Uuid | TargetKind::Cve | TargetKind::Ghsa => false, + } + } + + /// Does this target select the recorded patch `(purl, uuid)` — a + /// manifest record, a vendor-ledger entry or a hosted pin? + /// + /// * UUID: the patch uuid (case-insensitive). + /// * Versioned or qualified purl: the record's purl, with release-variant + /// rules ([`purl_matches_identifier`]: a base purl covers every + /// variant, a qualified one exactly one). + /// * Versionless purl / name: every recorded version of the package. + /// A name is also compared to the uuid verbatim, so a non-canonical + /// recorded uuid stays addressable. + /// * CVE / GHSA: nothing (records carry no advisory index). + pub fn matches_patch(&self, purl: &str, uuid: &str) -> bool { + match self.kind { + TargetKind::Uuid => uuid.eq_ignore_ascii_case(&self.text), + TargetKind::Purl if self.text.contains('?') || purl_has_version(&self.text) => { + purl_matches_identifier(purl, &self.text) + } + TargetKind::Purl => package_spec_matches(&self.text, purl), + TargetKind::Name => uuid == self.text || package_spec_matches(&self.text, purl), + TargetKind::Cve | TargetKind::Ghsa => false, + } + } +} + +/// The standard `8-4-4-4-12` hex UUID grouping, any case. The one +/// user-input UUID shape (the stricter lowercase-only on-disk grammar is +/// `patch::path_safety::is_canonical_uuid`). +pub fn is_uuid_shaped(s: &str) -> bool { + let parts: Vec<&str> = s.split('-').collect(); + parts.len() == 5 + && parts + .iter() + .zip([8, 4, 4, 4, 12]) + .all(|(p, len)| p.len() == len && p.bytes().all(|b| b.is_ascii_hexdigit())) +} + +/// `CVE-YYYY-N…` (case-insensitive; the sequence number is 1+ digits). +pub fn is_cve_id(s: &str) -> bool { + let Some(prefix) = s.get(..4) else { + return false; + }; + if !prefix.eq_ignore_ascii_case("cve-") { + return false; + } + let mut parts = s[4..].splitn(2, '-'); + let year = parts.next().unwrap_or_default(); + let seq = parts.next().unwrap_or_default(); + year.len() == 4 + && year.bytes().all(|b| b.is_ascii_digit()) + && !seq.is_empty() + && seq.bytes().all(|b| b.is_ascii_digit()) +} + +/// `GHSA-xxxx-xxxx-xxxx` (case-insensitive alphanumeric groups of four). +pub fn is_ghsa_id(s: &str) -> bool { + let parts: Vec<&str> = s.split('-').collect(); + parts.len() == 4 + && parts[0].eq_ignore_ascii_case("ghsa") + && parts[1..] + .iter() + .all(|p| p.len() == 4 && p.bytes().all(|b| b.is_ascii_alphanumeric())) +} + +/// Is `token` shaped like a filesystem path or glob rather than a package +/// name? (A separator, a glob metacharacter, or absolute.) An npm +/// `@scope/name` is a name, not a path. Only `rollback` has path targets; +/// every other verb treats these tokens as names. +pub fn is_path_shaped(token: &str) -> bool { + if is_purl(token) || is_npm_scoped_name(token) { + return false; + } + token.contains('/') + || token.contains('\\') + || token.contains(['*', '?', '[']) + || std::path::Path::new(token).is_absolute() +} + +/// `@scope/name`: exactly one `/`, both halves non-empty, no glob +/// metacharacters or backslashes. +fn is_npm_scoped_name(token: &str) -> bool { + let Some(rest) = token.strip_prefix('@') else { + return false; + }; + let Some((scope, name)) = rest.split_once('/') else { + return false; + }; + !scope.is_empty() + && !name.is_empty() + && !name.contains('/') + && !token.contains(['*', '?', '[', '\\']) +} + +/// Does this purl carry an exact version (`pkg:type/name@version`)? The +/// candidate version after the last `@` must not contain a `/` (that `@` +/// is an npm scope). +fn purl_has_version(purl: &str) -> bool { + strip_purl_qualifiers(purl) + .strip_prefix("pkg:") + .and_then(|rest| rest.split_once('/')) + .and_then(|(_, coord)| coord.rsplit_once('@')) + .is_some_and(|(head, version)| { + !head.is_empty() && !version.is_empty() && !version.contains('/') + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + const UUID: &str = "a8b05a61-1e2f-4c5f-a65b-93e71deba1ae"; + + #[test] + fn parse_classifies_every_kind() { + assert_eq!(Target::parse(UUID).kind(), TargetKind::Uuid); + assert_eq!(Target::parse(&UUID.to_uppercase()).kind(), TargetKind::Uuid); + assert_eq!(Target::parse("CVE-2021-44906").kind(), TargetKind::Cve); + assert_eq!(Target::parse("cve-2021-44906").kind(), TargetKind::Cve); + assert_eq!( + Target::parse("GHSA-xvch-5gv4-984h").kind(), + TargetKind::Ghsa + ); + assert_eq!( + Target::parse("ghsa-XVCH-5gv4-984h").kind(), + TargetKind::Ghsa + ); + assert_eq!(Target::parse("pkg:npm/lodash").kind(), TargetKind::Purl); + for name in ["lodash", "@babel/core", "org.apache:commons-text", ""] { + assert_eq!(Target::parse(name).kind(), TargetKind::Name, "{name}"); + } + // Near misses stay names. + for name in ["CVE-21-1", "CVE-2021-", "GHSA-1", "a8b05a61-1e2f-4c5f-a65b"] { + assert_eq!(Target::parse(name).kind(), TargetKind::Name, "{name}"); + } + } + + #[test] + fn uuid_shape() { + assert!(is_uuid_shaped(UUID)); + assert!(is_uuid_shaped("00000000-0000-0000-0000-000000000000")); + assert!(!is_uuid_shaped(" a8b05a61-1e2f-4c5f-a65b-93e71deba1ae")); + assert!(!is_uuid_shaped("a8b05a61-1e2f-4c5f-a65b-93e71deba1a")); + assert!(!is_uuid_shaped("g8b05a61-1e2f-4c5f-a65b-93e71deba1ae")); + assert!(!is_uuid_shaped("----")); + assert!(!is_uuid_shaped("")); + } + + #[test] + fn forced_kind_shape_check() { + assert!(!Target::with_kind("lodash", TargetKind::Uuid).shape_ok()); + assert!(!Target::with_kind("lodash", TargetKind::Cve).shape_ok()); + assert!(!Target::with_kind("GHSA-1", TargetKind::Ghsa).shape_ok()); + assert!(Target::with_kind(UUID, TargetKind::Uuid).shape_ok()); + assert!(Target::with_kind("anything", TargetKind::Name).shape_ok()); + assert!(Target::with_kind("anything", TargetKind::Purl).shape_ok()); + } + + #[test] + fn versioned_purl() { + assert!(Target::parse("pkg:npm/lodash@4.17.21").is_versioned_purl()); + assert!(Target::parse("pkg:npm/@s/x@1").is_versioned_purl()); + assert!(!Target::parse("pkg:npm/@s/x").is_versioned_purl()); + assert!(!Target::parse("pkg:npm/lodash").is_versioned_purl()); + assert!(!Target::parse("lodash@1").is_versioned_purl()); + } + + /// The same token selects the same packages and records on every verb: + /// a name is exact (never a prefix or substring), a versionless purl + /// covers every version. + #[test] + fn name_and_versionless_purl_select_every_version_exactly() { + let nested = "pkg:npm/lodash@4.17.4"; + let top = "pkg:npm/lodash@4.17.21"; + for token in ["lodash", "LoDash", "pkg:npm/lodash"] { + let t = Target::parse(token); + assert!( + t.matches_package(nested) && t.matches_package(top), + "{token}" + ); + assert!( + t.matches_patch(nested, "u1") && t.matches_patch(top, "u2"), + "{token}" + ); + assert!(!t.matches_package("pkg:npm/lodash-es@4.17.21"), "{token}"); + assert!( + !t.matches_patch("pkg:npm/lodash-es@4.17.21", "u3"), + "{token}" + ); + } + let yaml = Target::parse("yaml"); + assert!(!yaml.matches_package("pkg:npm/yaml-ast-parser@0.0.43")); + assert!(yaml.matches_package("pkg:npm/yaml@2.0.0")); + let scoped = Target::parse("@babel/core"); + assert!(scoped.matches_patch("pkg:npm/%40babel/core@7.0.0", "u")); + } + + #[test] + fn matches_patch_keeps_variant_rules_and_uuid_identity() { + let key = "pkg:pypi/six@1.16.0?artifact_id=sdist"; + assert!(Target::parse("pkg:pypi/six@1.16.0").matches_patch(key, "u")); + assert!(Target::parse(key).matches_patch(key, "u")); + assert!(!Target::parse("pkg:pypi/six@1.16.0?artifact_id=whl").matches_patch(key, "u")); + assert!(!Target::parse("pkg:pypi/six@1.17.0").matches_patch(key, "u")); + assert!(Target::parse(UUID).matches_patch(key, UUID)); + assert!(Target::parse(&UUID.to_uppercase()).matches_patch(key, UUID)); + assert!(!Target::parse(UUID).matches_patch(key, "other")); + // A name is also compared to the recorded uuid verbatim. + assert!(Target::parse("uuid-bar").matches_patch("pkg:npm/foo@1", "uuid-bar")); + // Advisory ids match no record. + assert!(!Target::parse("CVE-2021-44906").matches_patch(key, "CVE-2021-44906")); + assert!(!Target::parse("CVE-2021-44906").matches_package(key)); + } + + /// The former `patch_matches` contract, carried over verbatim. + #[test] + fn purl_targets_match_only_the_purl_field() { + const UUID: &str = "9f6b2c4e-1d3a-4f6b-8c2d-7e5a9b1c3d5f"; + let m = |purl: &str, uuid: &str, id: &str| Target::parse(id).matches_patch(purl, uuid); + let key = "pkg:pypi/requests@2.28.0?artifact_id=abc"; + assert!(m(key, UUID, "pkg:pypi/requests@2.28.0")); + assert!(m(key, UUID, key)); + assert!(!m(key, UUID, "pkg:pypi/requests@2.28.0?artifact_id=zzz")); + assert!(!m(key, UUID, "pkg:pypi/requests@2.29.0")); + assert!(m(key, UUID, UUID)); + assert!(!m(key, UUID, "not-the-uuid")); + // A uuid identifier never matches by PURL text, and a PURL + // identifier is only ever compared against the purl field. + assert!(!m( + "pkg:npm/other@1.0.0", + "pkg:pypi/requests@2.28.0", + "pkg:pypi/requests@2.28.0" + )); + assert!(!m(UUID, "other-uuid", UUID)); + assert!(!m("pkg:npm/a@1", UUID, "pkg:npm/b@1")); + } + + #[test] + fn path_shape() { + for p in [ + "./x", + "node_modules/lodash", + "a\\b", + "*", + "x?", + "[ab]", + "github.com/x/y", + ] { + assert!(is_path_shaped(p), "{p}"); + } + for n in [ + "lodash", + "@babel/core", + "pkg:npm/@s/x", + "org.apache:x", + UUID, + ] { + assert!(!is_path_shaped(n), "{n}"); + } + assert!(is_path_shaped("@babel/*")); + assert!(is_path_shaped("@a/b/c")); + } +} diff --git a/crates/socket-patch-core/src/vendor/state.rs b/crates/socket-patch-core/src/vendor/state.rs index 39e43d955..c8de05e1f 100644 --- a/crates/socket-patch-core/src/vendor/state.rs +++ b/crates/socket-patch-core/src/vendor/state.rs @@ -33,9 +33,10 @@ use crate::constants::SOCKET_DIR; use crate::manifest::schema::PatchRecord; use crate::utils::composer_version::{composer_purl_identity, composer_purls_equivalent}; use crate::utils::fs::{atomic_write_artifact, read_regular_to_bytes}; -use crate::utils::purl::{patch_matches, strip_purl_qualifiers}; +use crate::utils::purl::strip_purl_qualifiers; use crate::utils::serde::serialize_sorted; use crate::utils::socket_dir::{prune_empty_dirs, remove_file_and_prune, write_json_ledger}; +use crate::utils::target::Target; use super::parse_memo::ParseMemo; use super::path::VENDOR_DIR; @@ -303,12 +304,11 @@ impl VendorEntry { } /// Does this entry, stored under ledger `key`, match a remove/rollback - /// identifier? By its ledger key or by its base purl (mirroring the - /// manifest matching of [`patch_matches`]; a golang key is case-encoded - /// while `base_purl` holds the decoded spelling users type), or by uuid. - pub fn matches_identifier(&self, key: &str, identifier: &str) -> bool { - patch_matches(key, &self.uuid, identifier) - || patch_matches(&self.base_purl, &self.uuid, identifier) + /// target? By its ledger key or by its base purl (the manifest rule, + /// [`Target::matches_patch`]; a golang key is case-encoded while + /// `base_purl` holds the decoded spelling users type), or by uuid. + pub fn matches_target(&self, key: &str, target: &Target) -> bool { + target.matches_patch(key, &self.uuid) || target.matches_patch(&self.base_purl, &self.uuid) } /// Does this entry, stored under ledger `key`, own the manifest purl @@ -1098,20 +1098,29 @@ mod tests { entry.ecosystem = "golang".into(); entry.base_purl = "pkg:golang/github.com/BurntSushi/toml@1.0.0".into(); let key = "pkg:golang/github.com/!burnt!sushi/toml@1.0.0"; - assert!(entry.matches_identifier(key, key)); - assert!(entry.matches_identifier(key, "pkg:golang/github.com/BurntSushi/toml@1.0.0")); - assert!(entry.matches_identifier(key, UUID)); - assert!(!entry.matches_identifier(key, "pkg:golang/github.com/BurntSushi/toml@2.0.0")); - assert!(!entry.matches_identifier(key, "00000000-0000-4000-8000-000000000000")); + assert!(entry.matches_target(key, &Target::parse(key))); + assert!(entry.matches_target( + key, + &Target::parse("pkg:golang/github.com/BurntSushi/toml@1.0.0") + )); + assert!(entry.matches_target(key, &Target::parse(UUID))); + assert!(!entry.matches_target( + key, + &Target::parse("pkg:golang/github.com/BurntSushi/toml@2.0.0") + )); + assert!(!entry.matches_target(key, &Target::parse("00000000-0000-4000-8000-000000000000"))); // A qualified pypi key: the base identifier covers it, another // variant's qualifier does not. let mut entry = sample_entry(); entry.base_purl = "pkg:pypi/requests@2.28.0".into(); let key = "pkg:pypi/requests@2.28.0?artifact_id=abc"; - assert!(entry.matches_identifier(key, "pkg:pypi/requests@2.28.0")); - assert!(entry.matches_identifier(key, key)); - assert!(!entry.matches_identifier(key, "pkg:pypi/requests@2.28.0?artifact_id=zzz")); + assert!(entry.matches_target(key, &Target::parse("pkg:pypi/requests@2.28.0"))); + assert!(entry.matches_target(key, &Target::parse(key))); + assert!(!entry.matches_target( + key, + &Target::parse("pkg:pypi/requests@2.28.0?artifact_id=zzz") + )); } /// `covers_purl`: the exact key, a qualifier-stripped twin of the key From 93d61af6e99b61d695a3d316b53b5f040b3e6ff6 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 11:10:07 -0400 Subject: [PATCH 02/10] Make get select exact packages within --ecosystems on every path get now classifies its identifier with the core Target parser and drops its own IdentifierType, CVE/GHSA regexes and purl_has_version copies. - B11: get no longer fuzzy-picks one installed purl. It searches every installed version of the EXACT name (get lodash also searches a nested lodash@4.17.4; get yaml no longer patches yaml-ast-parser). With no exact match it reports no_match and only suggests near names. - B56: --ecosystems scopes the package-name crawl and filters every search result, so get CVE-X -e npm no longer records or rewrites the advisory's PyPI patch. - B29 (#453) / B57: get applies the same rules as the search path: a patch outside --ecosystems is not acted on, and a socket.yml bypass emits policy_bypassed in agent, hosted and vendored modes. Delete the crawl_all_ecosystems wrapper (crawl_ecosystems(opts, None)). Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/src/args.rs | 2 +- crates/socket-patch-cli/src/commands/get.rs | 454 +++++++++------- .../src/ecosystem_dispatch.rs | 23 +- .../tests/covgap_commands_get.rs | 10 +- .../tests/e2e_socket_yml_policy.rs | 508 ++++++++++++++---- .../socket-patch-cli/tests/in_process_get.rs | 135 +++++ 6 files changed, 837 insertions(+), 295 deletions(-) diff --git a/crates/socket-patch-cli/src/args.rs b/crates/socket-patch-cli/src/args.rs index a2ecfe542..fc4511dad 100644 --- a/crates/socket-patch-cli/src/args.rs +++ b/crates/socket-patch-cli/src/args.rs @@ -28,7 +28,7 @@ use socket_patch_core::vendor::{VendorServiceConfig, VendorSource}; /// loudly instead of silently matching nothing. /// /// Without this, an unsupported name parsed fine and was then silently -/// dropped by `partition_purls`/`crawl_all_ecosystems`, so the user got a +/// dropped by `partition_purls`/`crawl_ecosystems`, so the user got a /// "0 patches" result with no hint that the ecosystem name was the cause. fn parse_supported_ecosystem(s: &str) -> Result { if Ecosystem::all().iter().any(|e| e.cli_name() == s) { diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index a41f95802..47ac23ec2 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -1,6 +1,5 @@ use clap::Args; use futures_util::StreamExt; -use regex::Regex; use socket_patch_core::api::client::{ build_proxy_fallback_client, get_api_client_with_overrides, hold_back_debug, is_fallback_candidate, ApiClient, ApiError, @@ -20,14 +19,11 @@ use socket_patch_core::patch::apply::{is_valid_blob_hash, select_installed_varia use socket_patch_core::patch::apply_lock::{LockError, LockGuard}; use socket_patch_core::telemetry::{track_patch_fetch_failed, track_patch_fetched}; use socket_patch_core::utils::concurrent::{api_concurrency_for, ordered_concurrent}; -use socket_patch_core::utils::purl::{ - canonical_purl, is_purl, normalize_purl, strip_purl_qualifiers, -}; +use socket_patch_core::utils::purl::{canonical_purl, normalize_purl, strip_purl_qualifiers}; +use socket_patch_core::utils::target::{Target, TargetKind}; use socket_patch_core::vendor::{load_state, lookup_entry, VendorEntry, VendorState}; use std::collections::HashMap; -use std::fmt; use std::path::{Path, PathBuf}; -use std::sync::LazyLock; use std::time::Duration; use crate::args::{apply_env_toggles, GlobalArgs}; @@ -40,8 +36,7 @@ use crate::commands::vlt_preflight::{ vlt_refusal_for, vlt_vendor_preflight_selected, VltVendorRefusal, }; use crate::ecosystem_dispatch::{ - crawl_all_ecosystems, find_all_packages_for_rollback, find_packages_for_rollback, - partition_purls, + crawl_ecosystems, find_all_packages_for_rollback, find_packages_for_rollback, partition_purls, }; use crate::ui::{print_json, select_one, SelectError}; @@ -462,51 +457,6 @@ pub struct GetArgs { pub mode: Option, } -#[derive(Debug, Clone, Copy, PartialEq)] -enum IdentifierType { - Uuid, - Cve, - Ghsa, - Purl, - Package, -} - -impl fmt::Display for IdentifierType { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - IdentifierType::Uuid => write!(f, "UUID"), - IdentifierType::Cve => write!(f, "CVE"), - IdentifierType::Ghsa => write!(f, "GHSA"), - IdentifierType::Purl => write!(f, "PURL"), - IdentifierType::Package => write!(f, "package name"), - } - } -} - -/// Case-insensitive advisory-id shapes, compiled once. The UUID shape is -/// [`crate::looks_like_uuid`] (the same 8-4-4-4-12 hex check the argv -/// rewrite uses), so the two detectors cannot drift. -static CVE_RE: LazyLock = - LazyLock::new(|| Regex::new(r"(?i)^CVE-\d{4}-\d+$").expect("hardcoded CVE regex must compile")); -static GHSA_RE: LazyLock = LazyLock::new(|| { - Regex::new(r"(?i)^GHSA-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{4}$") - .expect("hardcoded GHSA regex must compile") -}); - -fn detect_identifier_type(identifier: &str) -> Option { - if crate::looks_like_uuid(identifier) { - Some(IdentifierType::Uuid) - } else if CVE_RE.is_match(identifier) { - Some(IdentifierType::Cve) - } else if GHSA_RE.is_match(identifier) { - Some(IdentifierType::Ghsa) - } else if is_purl(identifier) { - Some(IdentifierType::Purl) - } else { - None - } -} - /// Advisory labels for a patch: every advisory's CVE ids, or the advisory /// id itself when it has no CVE assigned yet (a fresh GHSA). Sorted and /// deduplicated, so the text never depends on `HashMap` iteration order. @@ -693,18 +643,60 @@ fn format_search_results( out } -/// The stderr line naming the package a package-name search went with -/// (only the best fuzzy match is searched). -fn format_best_match(purl: &str, matches: usize) -> String { - if matches > 1 { - format!( - "Best match: {} (of {} matching packages)", - normalize_purl(purl), - matches - ) - } else { - format!("Best match: {}", normalize_purl(purl)) +/// The stderr line naming the installed packages a package-name search +/// matched (every one of them is searched). +fn format_matched_packages(purls: &[String]) -> String { + let names: Vec = purls + .iter() + .map(|p| normalize_purl(p).into_owned()) + .collect(); + match names.as_slice() { + [one] => format!("Matched: {one}"), + many => format!( + "Matched {} installed packages: {}", + many.len(), + many.join(", ") + ), + } +} + +/// Every installed purl a package-name `target` selects, deduplicated and +/// sorted (a monorepo can hold the same release in several places). +fn installed_target_matches( + target: &Target, + packages: &[socket_patch_core::crawlers::CrawledPackage], +) -> Vec { + let mut purls: Vec = packages + .iter() + .filter(|pkg| target.matches_package(&pkg.purl)) + .map(|pkg| pkg.purl.clone()) + .collect(); + purls.sort(); + purls.dedup(); + purls +} + +/// "Did you mean" for a name that matched nothing exactly: up to five +/// installed names the fuzzy ranker puts closest. A suggestion only — a +/// near name is never searched or patched. +fn format_did_you_mean( + query: &str, + packages: &[socket_patch_core::crawlers::CrawledPackage], +) -> Option { + let mut names: Vec = Vec::new(); + for pkg in fuzzy_match_packages(query, packages, usize::MAX) { + let name = match &pkg.namespace { + Some(ns) => format!("{ns}/{}", pkg.name), + None => pkg.name.clone(), + }; + if !names.contains(&name) { + names.push(name); + } + if names.len() == 5 { + break; + } } + (!names.is_empty()).then(|| format!("Did you mean: {}?", names.join(", "))) } /// The `--verbose` per-version detail behind [`format_skip_summary`]: one @@ -961,22 +953,19 @@ const APPLY_FAILED: &str = "Error: Some patches could not be applied."; /// `--ghsa`, so a typo fails fast with a readable message instead of a raw /// API 400 body. `None` when it is well-formed (or the type is not /// shape-checked). -fn forced_identifier_error(identifier: &str, id_type: IdentifierType) -> Option { - let (ok, what, form) = match id_type { - IdentifierType::Uuid => ( - crate::looks_like_uuid(identifier), - "patch UUID", - "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", - ), - IdentifierType::Cve => (CVE_RE.is_match(identifier), "CVE ID", "CVE-YYYY-NNNN"), - IdentifierType::Ghsa => ( - GHSA_RE.is_match(identifier), - "GHSA ID", - "GHSA-xxxx-xxxx-xxxx", - ), - IdentifierType::Purl | IdentifierType::Package => return None, +fn forced_identifier_error(target: &Target) -> Option { + if target.shape_ok() { + return None; + } + let (what, form) = match target.kind() { + TargetKind::Uuid => ("patch UUID", "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"), + TargetKind::Cve => ("CVE ID", "CVE-YYYY-NNNN"), + TargetKind::Ghsa => ("GHSA ID", "GHSA-xxxx-xxxx-xxxx"), + TargetKind::Purl | TargetKind::Name => return None, }; - (!ok).then(|| format!("\"{identifier}\" is not a valid {what} (expected {form})")) + Some(format!( + "\"{target}\" is not a valid {what} (expected {form})" + )) } /// Select one patch per PURL from available patches. @@ -1401,22 +1390,6 @@ fn sort_by_purl(patches: &mut [PatchSearchResult]) { patches.sort_by(|a, b| a.purl.cmp(&b.purl).then_with(|| a.uuid.cmp(&b.uuid))); } -/// Does this purl carry an exact version (`pkg:type/name@version`)? An -/// exact-versioned PURL identifier is exempt from the coarse installed- -/// version narrowing, like a UUID: the user named the version explicitly. -/// npm scope `@`s don't count (`pkg:npm/@scope/name` is versionless — the -/// candidate "version" after the last `@` still contains a `/`). -fn purl_has_version(purl: &str) -> bool { - let stripped = strip_purl_qualifiers(purl); - stripped - .strip_prefix("pkg:") - .and_then(|rest| rest.split_once('/')) - .and_then(|(_, coord)| coord.rsplit_once('@')) - .is_some_and(|(head, version)| { - !head.is_empty() && !version.is_empty() && !version.contains('/') - }) -} - /// Outcome of the coarse installed-VERSION narrowing over a CVE/GHSA/PURL /// search fan-out (see [`filter_to_installed_purls`]). struct InstalledNarrowing { @@ -2688,22 +2661,25 @@ pub async fn run(args: GetArgs) -> i32 { return 1; } - // Determine identifier type - let id_type = if args.id { - IdentifierType::Uuid - } else if args.cve { - IdentifierType::Cve - } else if args.ghsa { - IdentifierType::Ghsa - } else if args.package { - IdentifierType::Package - } else { - detect_identifier_type(&args.identifier).unwrap_or(IdentifierType::Package) + // Classify the identifier with the shared target grammar (the same + // one `remove` and `rollback` use), or take the forced kind. + let forced = [ + (args.id, TargetKind::Uuid), + (args.cve, TargetKind::Cve), + (args.ghsa, TargetKind::Ghsa), + (args.package, TargetKind::Name), + ] + .into_iter() + .find_map(|(set, kind)| set.then_some(kind)); + let target = match forced { + Some(kind) => Target::with_kind(&args.identifier, kind), + None => Target::parse(&args.identifier), }; + let id_type = target.kind(); // A forced type is shape-checked locally, before any network call, so // a typo reads as a plain message instead of a raw API 400 body. if args.id || args.cve || args.ghsa { - if let Some(err) = forced_identifier_error(&args.identifier, id_type) { + if let Some(err) = forced_identifier_error(&target) { report_error(args.common.json, err); return 2; } @@ -2713,7 +2689,7 @@ pub async fn run(args: GetArgs) -> i32 { // `--silent` is "errors only" (CLI_CONTRACT.md): every informational // print below is gated on this; errors and JSON envelopes are not. let quiet = args.common.json || args.common.silent; - if !quiet && id_type == IdentifierType::Package && !args.package { + if !quiet && id_type == TargetKind::Name && !args.package { eprintln!("Treating \"{}\" as a package name search", args.identifier); } let overrides = args.common.api_client_overrides(); @@ -2734,7 +2710,7 @@ pub async fn run(args: GetArgs) -> i32 { let mut status = crate::ui::StatusLine::stderr(args.common.json, args.common.silent); // Handle UUID: fetch and download directly - if id_type == IdentifierType::Uuid { + if id_type == TargetKind::Uuid { status.set(format!("Fetching patch {}...", args.identifier)); let mut fetch_result = api_client.fetch_patch(&args.identifier).await; // 401/403 from the auth endpoint → swap to the public proxy @@ -2804,6 +2780,30 @@ pub async fn run(args: GetArgs) -> i32 { telemetry_org.as_deref(), ) .await; + let selected = vec![search_result_from_response(&patch)]; + // The search path's selection rules hold here too: a patch + // outside `--ecosystems` is never acted on, and acting + // against the repo's socket.yml says so (`policy_bypassed`). + if !args.common.purl_ecosystem_selected(&patch.purl) { + if args.common.json { + print_json(&empty_result_json("not_found")); + } else if !args.common.silent { + println!( + "No patch found with UUID: {} in the selected ecosystems \ + (it patches {})", + args.identifier, + normalize_purl(&patch.purl) + ); + } + return 0; + } + let policy_warnings = + super::scan::policy::policy_bypass_warnings(&args.common, &selected); + if !args.common.silent { + for (_, detail) in &policy_warnings { + eprintln!("Warning: {detail}"); + } + } // 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 @@ -2812,14 +2812,12 @@ pub async fn run(args: GetArgs) -> i32 { return match mode { // Save to manifest and apply in place. super::scan::ScanMode::Agent => { - save_and_apply_patch(&args, &api_client, &patch).await + save_and_apply_patch(&args, &api_client, &patch, &policy_warnings).await } super::scan::ScanMode::Hosted => { - let selected = vec![search_result_from_response(&patch)]; - run_get_hosted(&args, &api_client, &selected, &[], &[]).await + run_get_hosted(&args, &api_client, &selected, &[], &policy_warnings).await } super::scan::ScanMode::Vendored => { - let selected = vec![search_result_from_response(&patch)]; run_get_vendored( &args, &api_client, @@ -2827,7 +2825,7 @@ pub async fn run(args: GetArgs) -> i32 { &selected, Some(&patch), &[], - &[], + &policy_warnings, telemetry_token.as_deref(), telemetry_org.as_deref(), ) @@ -2883,17 +2881,15 @@ pub async fn run(args: GetArgs) -> i32 { // CVE / GHSA / PURL share the same path: log the search, dispatch to // the matching endpoint, and surface errors via `report_fetch_failure`. let search_response: SearchResponse = match id_type { - IdentifierType::Cve | IdentifierType::Ghsa | IdentifierType::Purl => { + TargetKind::Cve | TargetKind::Ghsa | TargetKind::Purl => { status.set(format!( "Searching patches for {id_type} {}...", args.identifier )); let result = match id_type { - IdentifierType::Cve => api_client.search_patches_by_cve(&args.identifier).await, - IdentifierType::Ghsa => api_client.search_patches_by_ghsa(&args.identifier).await, - IdentifierType::Purl => { - api_client.search_patches_by_package(&args.identifier).await - } + TargetKind::Cve => api_client.search_patches_by_cve(&args.identifier).await, + TargetKind::Ghsa => api_client.search_patches_by_ghsa(&args.identifier).await, + TargetKind::Purl => api_client.search_patches_by_package(&args.identifier).await, _ => unreachable!(), }; status.finish(); @@ -2912,9 +2908,12 @@ pub async fn run(args: GetArgs) -> i32 { } } } - IdentifierType::Package => { + TargetKind::Name => { status.set("Enumerating packages..."); - let (all_packages, _, _) = crawl_all_ecosystems(&args.common.crawler_options()).await; + // `--ecosystems` scopes the crawl, so a name can only resolve + // inside the selected ecosystems. + let only = args.common.ecosystems.as_deref().filter(|l| !l.is_empty()); + let (all_packages, _, _) = crawl_ecosystems(&args.common.crawler_options(), only).await; if all_packages.is_empty() { status.finish(); @@ -2931,48 +2930,70 @@ pub async fn run(args: GetArgs) -> i32 { crate::ui::plural(all_packages.len(), "package", "packages") )); - let matches = fuzzy_match_packages(&args.identifier, &all_packages, 20); - - if matches.is_empty() { + // The shared target grammar: an EXACT name (full or last + // segment, case-insensitive, PEP 503 for PyPI), never a prefix + // or substring, and every installed version of it. + let matched = installed_target_matches(&target, &all_packages); + if matched.is_empty() { if args.common.json { print_json(&empty_result_json("no_match")); } else if !args.common.silent { println!("No packages matching \"{}\" found.", args.identifier); + // Near names are only ever suggested, never acted on. + if let Some(hint) = format_did_you_mean(&args.identifier, &all_packages) { + println!("{hint}"); + } } return 0; } - // Only the best match is searched: name it, so a fuzzy pick - // of the wrong package is visible. - let best_match = &matches[0]; if !quiet { - eprintln!("{}", format_best_match(&best_match.purl, matches.len())); + eprintln!("{}", format_matched_packages(&matched)); } - status.set(format!( - "Searching patches for {}...", - normalize_purl(&best_match.purl) - )); - let result = api_client.search_patches_by_package(&best_match.purl).await; - status.finish(); - match result { - Ok(r) => r, - Err(e) => { - return report_fetch_failure( - &args.identifier, - e, - fallback_to_proxy, - telemetry_token.as_deref(), - telemetry_org.as_deref(), - args.common.json, - ) - .await; + let mut merged = SearchResponse { + patches: Vec::new(), + can_access_paid_patches: false, + }; + for purl in &matched { + status.set(format!("Searching patches for {}...", normalize_purl(purl))); + let result = api_client.search_patches_by_package(purl).await; + status.finish(); + match result { + Ok(r) => { + merged.can_access_paid_patches |= r.can_access_paid_patches; + for patch in r.patches { + if !merged.patches.iter().any(|p| p.uuid == patch.uuid) { + merged.patches.push(patch); + } + } + } + Err(e) => { + return report_fetch_failure( + &args.identifier, + e, + fallback_to_proxy, + telemetry_token.as_deref(), + telemetry_org.as_deref(), + args.common.json, + ) + .await; + } } } + merged } _ => unreachable!(), }; drop(status); + // `--ecosystems` restricts what `get` acts on, whatever the identifier + // kind: an advisory or purl search can return patches for other + // ecosystems, and those are never selected. + let mut search_response = search_response; + search_response + .patches + .retain(|p| args.common.purl_ecosystem_selected(&p.purl)); + if search_response.patches.is_empty() { if args.common.json { print_json(&empty_result_json("not_found")); @@ -3007,8 +3028,8 @@ pub async fn run(args: GetArgs) -> i32 { })); } else if !args.common.silent { let all: Vec<&PatchSearchResult> = search_response.patches.iter().collect(); - if id_type == IdentifierType::Package && !quiet { - // Separate the stderr `Best match` line above on a terminal; + if id_type == TargetKind::Name && !quiet { + // Separate the stderr `Matched` line above on a terminal; // stdout itself starts with the result. eprintln!(); } @@ -3033,8 +3054,8 @@ pub async fn run(args: GetArgs) -> i32 { // never "not installed". let narrowing_exempt = args.all_releases || args.save_only - || id_type == IdentifierType::Package - || (id_type == IdentifierType::Purl && purl_has_version(&args.identifier)); + || id_type == TargetKind::Name + || target.is_versioned_purl(); // The narrowing runs over EVERY result (one crawl), paid no-access ones // included, so the listing can still show an installed package's paid // fix as `[PAID] (no access)`; selection, the skip records and the @@ -3075,7 +3096,7 @@ pub async fn run(args: GetArgs) -> i32 { } if accessible.is_empty() { // Every accessible patch was narrowed out. Additive status (never - // `no_match`, which is pinned to the fuzzy package-name path): + // `no_match`, which is pinned to the package-name path): // exit 0, the skips carry the detail via their errorCode. if args.common.json { let mut result = serde_json::json!({ @@ -3104,8 +3125,8 @@ pub async fn run(args: GetArgs) -> i32 { // per-version detail after the summary under --verbose. let listed: Vec<&PatchSearchResult> = listed.iter().collect(); if !quiet { - if id_type == IdentifierType::Package || !narrow_warnings.is_empty() { - // Separate the stderr lines above (`Best match`, warnings) on a + if id_type == TargetKind::Name || !narrow_warnings.is_empty() { + // Separate the stderr lines above (`Matched`, warnings) on a // terminal; stdout itself starts with the result. eprintln!(); } @@ -3496,7 +3517,15 @@ async fn save_patch_record( /// `--save-only`, apply it — under ONE apply lock, on the `client` the /// fetch used (a fresh client could re-hit the 401/403 its proxy fallback /// just recovered from). -async fn save_and_apply_patch(args: &GetArgs, client: &ApiClient, patch: &PatchResponse) -> i32 { +/// `policy_warnings` are the run's `(code, detail)` warnings already +/// printed to stderr (today: `policy_bypassed`); the JSON envelope carries +/// them like the search path's. +async fn save_and_apply_patch( + args: &GetArgs, + client: &ApiClient, + patch: &PatchResponse, + policy_warnings: &[(String, String)], +) -> i32 { // Same "errors only" gate as `run` — informational prints respect // `--silent`; errors and the JSON envelope do not. let quiet = args.common.json || args.common.silent; @@ -3506,7 +3535,13 @@ async fn save_and_apply_patch(args: &GetArgs, client: &ApiClient, patch: &PatchR // A dry run previews against the manifest and writes nothing — not // even the lock (which would create `.socket/`). if args.common.dry_run { - return agent_dry_run(args, &[search_result_from_response(patch)], &[], &[]).await; + return agent_dry_run( + args, + &[search_result_from_response(patch)], + &[], + policy_warnings, + ) + .await; } // See `download_and_apply_patches_with`: the RMW runs under the lock, // which also creates `.socket/` and prunes it again when nothing lands; @@ -3639,6 +3674,7 @@ async fn save_and_apply_patch(args: &GetArgs, client: &ApiClient, patch: &PatchR if !warnings.is_empty() { result_json["warnings"] = serde_json::json!(warnings); } + fold_narrowing_into_result(&mut result_json, &[], policy_warnings); print_json(&result_json); } @@ -3987,13 +4023,19 @@ mod tests { use socket_patch_core::api::types::{PatchFileResponse, VulnerabilityResponse}; use std::collections::HashMap; - // --- detect_identifier_type ------------------------------------------- + // --- identifier classification (the shared core target grammar) ------- + + /// `get`'s view of [`Target::parse`]: `None` for the bare-name fallback. + fn detect_identifier_type(identifier: &str) -> Option { + let kind = Target::parse(identifier).kind(); + (kind != TargetKind::Name).then_some(kind) + } #[test] fn detect_uuid_lowercase() { assert_eq!( detect_identifier_type("80630680-4da6-45f9-bba8-b888e0ffd58c"), - Some(IdentifierType::Uuid) + Some(TargetKind::Uuid) ); } @@ -4002,7 +4044,7 @@ mod tests { // Case-insensitive UUID regex per contract. assert_eq!( detect_identifier_type("80630680-4DA6-45F9-BBA8-B888E0FFD58C"), - Some(IdentifierType::Uuid) + Some(TargetKind::Uuid) ); } @@ -4010,7 +4052,7 @@ mod tests { fn detect_cve_uppercase() { assert_eq!( detect_identifier_type("CVE-2021-44906"), - Some(IdentifierType::Cve) + Some(TargetKind::Cve) ); } @@ -4019,7 +4061,7 @@ mod tests { // Load-bearing: CVE detection must be case-insensitive. assert_eq!( detect_identifier_type("cve-2021-44906"), - Some(IdentifierType::Cve) + Some(TargetKind::Cve) ); } @@ -4027,7 +4069,7 @@ mod tests { fn detect_ghsa_uppercase() { assert_eq!( detect_identifier_type("GHSA-abcd-1234-wxyz"), - Some(IdentifierType::Ghsa) + Some(TargetKind::Ghsa) ); } @@ -4036,7 +4078,7 @@ mod tests { // Load-bearing: GHSA detection must be case-insensitive. assert_eq!( detect_identifier_type("ghsa-abcd-1234-wxyz"), - Some(IdentifierType::Ghsa) + Some(TargetKind::Ghsa) ); } @@ -4044,7 +4086,7 @@ mod tests { fn detect_purl() { assert_eq!( detect_identifier_type("pkg:npm/foo@1.0"), - Some(IdentifierType::Purl) + Some(TargetKind::Purl) ); } @@ -5714,14 +5756,57 @@ mod tests { } #[test] - fn best_match_line_names_the_count_only_when_there_was_a_choice() { + fn matched_line_names_every_searched_package() { + assert_eq!( + format_matched_packages(&["pkg:npm/%40s/a@1".to_string()]), + "Matched: pkg:npm/@s/a@1" + ); + assert_eq!( + format_matched_packages(&["pkg:npm/a@1".to_string(), "pkg:npm/a@2".to_string()]), + "Matched 2 installed packages: pkg:npm/a@1, pkg:npm/a@2" + ); + } + + fn crawled( + purl: &str, + name: &str, + namespace: Option<&str>, + ) -> socket_patch_core::crawlers::CrawledPackage { + socket_patch_core::crawlers::CrawledPackage { + name: name.to_string(), + version: "1".to_string(), + namespace: namespace.map(str::to_string), + purl: purl.to_string(), + path: std::path::PathBuf::from("/fake"), + } + } + + /// B11: a package name selects EXACT matches only — every installed + /// version — and never a prefix/substring sibling (`yaml` is not + /// `yaml-ast-parser`); near names are only suggested. + #[test] + fn package_name_selects_every_exact_version_and_no_near_names() { + let pkgs = vec![ + crawled("pkg:npm/lodash@4.17.21", "lodash", None), + crawled("pkg:npm/lodash@4.17.4", "lodash", None), + crawled("pkg:npm/lodash@4.17.4", "lodash", None), + crawled("pkg:npm/lodash-es@4.17.21", "lodash-es", None), + crawled("pkg:npm/yaml-ast-parser@0.0.43", "yaml-ast-parser", None), + ]; + assert_eq!( + installed_target_matches(&Target::parse("lodash"), &pkgs), + vec!["pkg:npm/lodash@4.17.21", "pkg:npm/lodash@4.17.4"] + ); + assert!(installed_target_matches(&Target::parse("yaml"), &pkgs).is_empty()); assert_eq!( - format_best_match("pkg:npm/%40s/a@1", 1), - "Best match: pkg:npm/@s/a@1" + format_did_you_mean("yaml", &pkgs).as_deref(), + Some("Did you mean: yaml-ast-parser?") ); + assert_eq!(format_did_you_mean("zzqxjvwq", &pkgs), None); assert_eq!( - format_best_match("pkg:npm/a@1", 3), - "Best match: pkg:npm/a@1 (of 3 matching packages)" + installed_target_matches(&Target::parse("pkg:npm/lodash"), &pkgs).len(), + 2, + "a versionless purl selects every installed version" ); } @@ -5853,35 +5938,38 @@ mod tests { #[test] fn forced_identifier_shapes() { assert_eq!( - forced_identifier_error("lodash", IdentifierType::Uuid).as_deref(), + forced_identifier_error(&Target::with_kind("lodash", TargetKind::Uuid)).as_deref(), Some("\"lodash\" is not a valid patch UUID (expected xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx)") ); assert_eq!( - forced_identifier_error("lodash", IdentifierType::Cve).as_deref(), + forced_identifier_error(&Target::with_kind("lodash", TargetKind::Cve)).as_deref(), Some("\"lodash\" is not a valid CVE ID (expected CVE-YYYY-NNNN)") ); assert_eq!( - forced_identifier_error("GHSA-1", IdentifierType::Ghsa).as_deref(), + forced_identifier_error(&Target::with_kind("GHSA-1", TargetKind::Ghsa)).as_deref(), Some("\"GHSA-1\" is not a valid GHSA ID (expected GHSA-xxxx-xxxx-xxxx)") ); assert_eq!( - forced_identifier_error("a8b05a61-1e2f-4c5f-a65b-93e71deba1ae", IdentifierType::Uuid), + forced_identifier_error(&Target::with_kind( + "a8b05a61-1e2f-4c5f-a65b-93e71deba1ae", + TargetKind::Uuid + )), None ); assert_eq!( - forced_identifier_error("cve-2021-44906", IdentifierType::Cve), + forced_identifier_error(&Target::with_kind("cve-2021-44906", TargetKind::Cve)), None ); assert_eq!( - forced_identifier_error("GHSA-xvch-5gv4-984h", IdentifierType::Ghsa), + forced_identifier_error(&Target::with_kind("GHSA-xvch-5gv4-984h", TargetKind::Ghsa)), None ); assert_eq!( - forced_identifier_error("anything", IdentifierType::Package), + forced_identifier_error(&Target::with_kind("anything", TargetKind::Name)), None ); assert_eq!( - forced_identifier_error("anything", IdentifierType::Purl), + forced_identifier_error(&Target::with_kind("anything", TargetKind::Purl)), None ); } @@ -6151,15 +6239,15 @@ mod tests { assert_eq!(record, serde_json::json!({"purl": "pkg:npm/x@1.0.0"})); } - /// The `IdentifierType` Display labels are user-facing vocabulary (the + /// The `TargetKind` Display labels are user-facing vocabulary (the /// "No patches found for {type}: {id}" terminal) — pin all five. #[test] fn identifier_type_display_labels_are_stable() { - assert_eq!(IdentifierType::Uuid.to_string(), "UUID"); - assert_eq!(IdentifierType::Cve.to_string(), "CVE"); - assert_eq!(IdentifierType::Ghsa.to_string(), "GHSA"); - assert_eq!(IdentifierType::Purl.to_string(), "PURL"); - assert_eq!(IdentifierType::Package.to_string(), "package name"); + assert_eq!(TargetKind::Uuid.to_string(), "UUID"); + assert_eq!(TargetKind::Cve.to_string(), "CVE"); + assert_eq!(TargetKind::Ghsa.to_string(), "GHSA"); + assert_eq!(TargetKind::Purl.to_string(), "PURL"); + assert_eq!(TargetKind::Name.to_string(), "package name"); } /// JSON mode with multiple free patches for one purl: the diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index e2620eaa6..5f27f29df 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -16,7 +16,7 @@ use socket_patch_core::crawlers::GoCrawler; use socket_patch_core::crawlers::MavenCrawler; use socket_patch_core::crawlers::NuGetCrawler; -/// Whether [`crawl_all_ecosystems`] actually visits this PURL's ecosystem +/// Whether [`crawl_ecosystems`] actually visits this PURL's ecosystem /// in THIS process. An unrecognized `pkg:/` (a newer CLI's ecosystem /// in a committed manifest) has no crawler at all — for those, absence /// from the crawl carries no information about whether the package is @@ -767,22 +767,13 @@ where Box::pin(make()) } -/// Crawl all ecosystems and return all packages, per-ecosystem counts and +/// Crawl the ecosystems and return all packages, per-ecosystem counts and /// the gem crawl's refused config-sourced `BUNDLE_PATH` /// (`BundleStoreDiscovery::skipped_config_path`, local mode only) — /// recovered from the crawl that hit it, so callers surfacing the advisory /// never probe the Bundler roots a second time. -pub async fn crawl_all_ecosystems( - options: &CrawlerOptions, -) -> ( - Vec, - HashMap, - Option, -) { - crawl_ecosystems(options, None).await -} - -/// [`crawl_all_ecosystems`] over only the ecosystems `only` names +/// +/// Restricted to the ecosystems `only` names /// (`--ecosystems` spellings; `None` crawls every one). A crawler that is /// not selected never runs: it contributes no packages and no `counts` /// entry. Each crawler reports only its own ecosystem's purls, so the @@ -800,7 +791,7 @@ pub async fn crawl_ecosystems( (packages, counts, skipped_config_path) } -/// [`crawl_all_ecosystems`], also handing back the npm half of the crawl as +/// [`crawl_ecosystems`], also handing back the npm half of the crawl as /// an [`NpmCrawlSnapshot`] (its packages are the leading `counts[Npm]` /// entries of the package list). #[cfg(test)] @@ -1737,7 +1728,7 @@ mod tests { #[tokio::test] async fn crawl_all_includes_every_ecosystem_unconditionally() { let tmp = tempfile::tempdir().unwrap(); - let (_, counts, _) = crawl_all_ecosystems(&local_options(tmp.path().to_path_buf())).await; + let (_, counts, _) = crawl_ecosystems(&local_options(tmp.path().to_path_buf()), None).await; for eco in [ Ecosystem::Npm, Ecosystem::Pypi, @@ -2020,7 +2011,7 @@ mod tests { global_prefix: Some(root.to_path_buf()), }; - let (packages, counts, _) = crawl_all_ecosystems(&options).await; + let (packages, counts, _) = crawl_ecosystems(&options, None).await; let mut serial: Vec = Vec::new(); let mut serial_counts: HashMap = HashMap::new(); diff --git a/crates/socket-patch-cli/tests/covgap_commands_get.rs b/crates/socket-patch-cli/tests/covgap_commands_get.rs index c98f057a7..166e6c7c4 100644 --- a/crates/socket-patch-cli/tests/covgap_commands_get.rs +++ b/crates/socket-patch-cli/tests/covgap_commands_get.rs @@ -762,10 +762,10 @@ async fn human_global_package_search_empty_prefix_prints_no_global_packages() { ); } -/// Installed packages that fuzzy-match NOTHING: the `no_match` terminal — +/// Installed packages that match NOTHING (exactly or nearly): the `no_match` terminal — /// json envelope + human message — exits 0 with zero API calls. #[tokio::test] -async fn package_search_without_fuzzy_match_is_no_match_in_both_modes() { +async fn package_search_without_a_match_is_no_match_in_both_modes() { // json flavor. { let server = MockServer::start().await; @@ -811,7 +811,7 @@ async fn package_search_without_fuzzy_match_is_no_match_in_both_modes() { } } -/// The package path's search-API error arm: a fuzzy-matched package whose +/// The package path's search-API error arm: a matched package whose /// by-package search 500s must exit 1 via `report_fetch_failure`, after /// printing the "checking for available patches" progress line. #[tokio::test] @@ -837,8 +837,8 @@ async fn human_package_search_api_error_reports_fetch_failure() { "a 500 from the package search must exit 1; stdout={stdout}\nstderr={stderr}" ); assert!( - stderr.contains(&format!("Best match: pkg:npm/{NAME}@1.0.0\n")), - "the best-match line must print first; stderr={stderr}" + stderr.contains(&format!("Matched: pkg:npm/{NAME}@1.0.0\n")), + "the matched-packages line must print first; stderr={stderr}" ); assert!( stderr.contains("Error:"), diff --git a/crates/socket-patch-cli/tests/e2e_socket_yml_policy.rs b/crates/socket-patch-cli/tests/e2e_socket_yml_policy.rs index 50018d6a9..841dc7131 100644 --- a/crates/socket-patch-cli/tests/e2e_socket_yml_policy.rs +++ b/crates/socket-patch-cli/tests/e2e_socket_yml_policy.rs @@ -61,7 +61,11 @@ impl Patch { "low" => 3, _ => 4, }; - self.severities.iter().copied().min_by_key(|s| rank(s)).unwrap_or("unknown") + self.severities + .iter() + .copied() + .min_by_key(|s| rank(s)) + .unwrap_or("unknown") } } @@ -200,7 +204,9 @@ async fn mount_api(server: &MockServer, patches: Vec) { .await; let detail_map = by_purl.clone(); Mock::given(method("GET")) - .and(path_regex(format!("^/v0/orgs/{ORG}/patches/by-package/.+$"))) + .and(path_regex(format!( + "^/v0/orgs/{ORG}/patches/by-package/.+$" + ))) .respond_with(move |req: &Request| { let raw = req.url.path().rsplit('/').next().unwrap(); let purl = percent_decode(raw); @@ -216,11 +222,15 @@ async fn mount_api(server: &MockServer, patches: Vec) { }) }) .collect(); - ResponseTemplate::new(200).set_body_json(json!({"patches": list, "canAccessPaidPatches": false})) + ResponseTemplate::new(200) + .set_body_json(json!({"patches": list, "canAccessPaidPatches": false})) }) .mount(server) .await; - let by_uuid: BTreeMap = patches.iter().map(|p| (p.uuid.to_string(), p.clone())).collect(); + let by_uuid: BTreeMap = patches + .iter() + .map(|p| (p.uuid.to_string(), p.clone())) + .collect(); let refs = by_uuid.clone(); Mock::given(method("POST")) .and(path(format!("/v0/orgs/{ORG}/patches/package"))) @@ -277,8 +287,10 @@ fn write_npm_root(dir: &Path, deps: &[&str]) { let dep_map: BTreeMap<&str, &str> = deps.iter().map(|d| (*d, "1.0.0")).collect(); std::fs::write( dir.join("package.json"), - serde_json::to_string_pretty(&json!({"name": "consumer", "version": "0.0.0", "dependencies": dep_map})) - .unwrap(), + serde_json::to_string_pretty( + &json!({"name": "consumer", "version": "0.0.0", "dependencies": dep_map}), + ) + .unwrap(), ) .unwrap(); let mut packages = serde_json::Map::new(); @@ -289,7 +301,11 @@ fn write_npm_root(dir: &Path, deps: &[&str]) { for name in deps { let pkg = dir.join("node_modules").join(name); std::fs::create_dir_all(&pkg).unwrap(); - std::fs::write(pkg.join("package.json"), format!(r#"{{ "name": "{name}", "version": "1.0.0" }}"#)).unwrap(); + std::fs::write( + pkg.join("package.json"), + format!(r#"{{ "name": "{name}", "version": "1.0.0" }}"#), + ) + .unwrap(); std::fs::write(pkg.join("index.js"), orig_index(name)).unwrap(); packages.insert( format!("node_modules/{name}"), @@ -304,12 +320,20 @@ fn write_npm_root(dir: &Path, deps: &[&str]) { "name": "consumer", "version": "0.0.0", "lockfileVersion": 3, "requires": true, "packages": packages }); - std::fs::write(dir.join("package-lock.json"), serde_json::to_string_pretty(&lock).unwrap() + "\n").unwrap(); + std::fs::write( + dir.join("package-lock.json"), + serde_json::to_string_pretty(&lock).unwrap() + "\n", + ) + .unwrap(); } fn write_gem(dir: &Path, name: &str, version: &str) { - std::fs::create_dir_all(dir.join("vendor/bundle/ruby/3.0.0/gems").join(format!("{name}-{version}")).join("lib")) - .unwrap(); + std::fs::create_dir_all( + dir.join("vendor/bundle/ruby/3.0.0/gems") + .join(format!("{name}-{version}")) + .join("lib"), + ) + .unwrap(); } /// The monorepo: `services/web` (alpha, beta, left-pad + a gem), @@ -349,7 +373,11 @@ impl Repo { if entry.file_type().unwrap().is_dir() { walk(&path, root, out); } else { - let rel = path.strip_prefix(root).unwrap().to_string_lossy().into_owned(); + let rel = path + .strip_prefix(root) + .unwrap() + .to_string_lossy() + .into_owned(); out.insert(rel, std::fs::read(&path).unwrap()); } } @@ -369,7 +397,8 @@ fn run_cli(cwd: &Path, args: &[&str], env: &[(&str, &str)]) -> (i32, String, Str cmd.env_remove(key); } } - cmd.env_remove("GIT_CEILING_DIRECTORIES").env_remove("VIRTUAL_ENV"); + cmd.env_remove("GIT_CEILING_DIRECTORIES") + .env_remove("VIRTUAL_ENV"); cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); // The fixture's hosted pins name this origin; it makes them recorded. cmd.env("SOCKET_PATCH_SERVER_URL", "http://patch.test"); @@ -383,7 +412,8 @@ fn run_cli(cwd: &Path, args: &[&str], env: &[(&str, &str)]) -> (i32, String, Str ] { cmd.env(var, &absent); } - cmd.env("NPM_CONFIG_ALLOW_REMOTE", "").env("npm_config_allow_remote", ""); + cmd.env("NPM_CONFIG_ALLOW_REMOTE", "") + .env("npm_config_allow_remote", ""); for (k, v) in env { cmd.env(k, v); } @@ -418,8 +448,9 @@ fn scan_json(cwd: &Path, api: &str, extra: &[&str], env: &[(&str, &str)]) -> (i3 let mut args = vec!["--json"]; args.extend_from_slice(extra); let (code, stdout, stderr) = scan(cwd, api, &args, env); - let doc: Value = serde_json::from_str(&stdout) - .unwrap_or_else(|e| panic!("stdout must be JSON ({e})\nstdout=\n{stdout}\nstderr=\n{stderr}")); + let doc: Value = serde_json::from_str(&stdout).unwrap_or_else(|e| { + panic!("stdout must be JSON ({e})\nstdout=\n{stdout}\nstderr=\n{stderr}") + }); (code, doc) } @@ -428,7 +459,12 @@ fn filtered(doc: &Value) -> Vec<(Option, String)> { .as_array() .unwrap() .iter() - .map(|f| (f["purl"].as_str().map(str::to_string), f["reason"].as_str().unwrap().to_string())) + .map(|f| { + ( + f["purl"].as_str().map(str::to_string), + f["reason"].as_str().unwrap().to_string(), + ) + }) .collect() } @@ -444,7 +480,11 @@ fn filtered_reason<'a>(doc: &'a Value, purl: &str) -> &'a Value { fn warning_codes(doc: &Value) -> Vec { doc["warnings"] .as_array() - .map(|w| w.iter().filter_map(|e| e["code"].as_str().map(str::to_string)).collect()) + .map(|w| { + w.iter() + .filter_map(|e| e["code"].as_str().map(str::to_string)) + .collect() + }) .unwrap_or_default() } @@ -464,16 +504,28 @@ async fn hosted_filters_by_ecosystem_package_and_severity() { assert_eq!(code, 0, "{doc:#}"); let lock = repo.lock("services/web"); - assert!(lock.contains(&P_ALPHA.hosted_url()), "alpha is patched:\n{lock}"); - assert!(!lock.contains(P_BETA.uuid), "beta is below the floor:\n{lock}"); - assert!(!lock.contains(P_LEFTPAD.uuid), "left-pad is ignored:\n{lock}"); + assert!( + lock.contains(&P_ALPHA.hosted_url()), + "alpha is patched:\n{lock}" + ); + assert!( + !lock.contains(P_BETA.uuid), + "beta is below the floor:\n{lock}" + ); + assert!( + !lock.contains(P_LEFTPAD.uuid), + "left-pad is ignored:\n{lock}" + ); let policy = &doc["policy"]; assert_eq!(policy["source"], "file"); assert_eq!(policy["path"], "socket.yml"); assert_eq!(policy["sha256"].as_str().unwrap().len(), 64); assert_eq!(policy["enabled"], true); - assert_eq!(policy["minSeverity"], json!({"value": "high", "source": "file"})); + assert_eq!( + policy["minSeverity"], + json!({"value": "high", "source": "file"}) + ); let beta = filtered_reason(&doc, "pkg:npm/beta@1.0.0"); assert_eq!(beta["reason"], "policy_severity"); assert_eq!(beta["detail"], "low < high"); @@ -481,8 +533,15 @@ async fn hosted_filters_by_ecosystem_package_and_severity() { assert_eq!(beta["project"], "services/web"); let left_pad = filtered_reason(&doc, "pkg:npm/left-pad@1.0.0"); assert_eq!(left_pad["reason"], "policy_package_ignored"); - assert_eq!(left_pad["uuid"], Value::Null, "filtered before any patch lookup"); - assert_eq!(left_pad["detail"], "pkg:npm/left-pad (patches.ignorePackages)"); + assert_eq!( + left_pad["uuid"], + Value::Null, + "filtered before any patch lookup" + ); + assert_eq!( + left_pad["detail"], + "pkg:npm/left-pad (patches.ignorePackages)" + ); let rack = filtered_reason(&doc, "pkg:gem/rack@1.0.0"); assert_eq!(rack["reason"], "policy_ecosystem"); assert_eq!(policy["counts"]["filtered"], 3); @@ -492,7 +551,10 @@ async fn hosted_filters_by_ecosystem_package_and_severity() { for r in &reqs { if r.url.path().ends_with("/patches/batch") { let body = String::from_utf8_lossy(&r.body); - assert!(!body.contains("left-pad") && !body.contains("rack"), "{body}"); + assert!( + !body.contains("left-pad") && !body.contains("rack"), + "{body}" + ); } } assert_eq!(doc["redirect"]["redirected"], 1, "{:#}", doc["redirect"]); @@ -503,13 +565,27 @@ async fn hosted_filters_by_ecosystem_package_and_severity() { async fn hosted_dry_run_makes_the_same_decisions_and_writes_nothing() { let server = MockServer::start().await; mount_api(&server, catalog()).await; - let repo = Repo::new(Some("version: 2\npatches:\n minSeverity: high\n ecosystems: [npm]\n")); + let repo = Repo::new(Some( + "version: 2\npatches:\n minSeverity: high\n ecosystems: [npm]\n", + )); let before = repo.snapshot(); - let (code, doc) = scan_json(&repo.dir("services/web"), &server.uri(), &["--dry-run"], &[]); + let (code, doc) = scan_json( + &repo.dir("services/web"), + &server.uri(), + &["--dry-run"], + &[], + ); assert_eq!(code, 0, "{doc:#}"); assert_eq!(repo.snapshot(), before, "a dry run changes no bytes"); - assert_eq!(doc["redirect"]["redirected"], 2, "alpha and left-pad: {:#}", doc["redirect"]); - assert_eq!(filtered_reason(&doc, "pkg:npm/beta@1.0.0")["reason"], "policy_severity"); + assert_eq!( + doc["redirect"]["redirected"], 2, + "alpha and left-pad: {:#}", + doc["redirect"] + ); + assert_eq!( + filtered_reason(&doc, "pkg:npm/beta@1.0.0")["reason"], + "policy_severity" + ); } #[tokio::test] @@ -517,14 +593,24 @@ async fn hosted_dry_run_makes_the_same_decisions_and_writes_nothing() { async fn path_globs_apply_default_ignores_and_ignore_paths_human() { let server = MockServer::start().await; mount_api(&server, catalog()).await; - let repo = Repo::new(Some("version: 2\npatches:\n ignorePaths: [\"/services/legacy/\"]\n")); + let repo = Repo::new(Some( + "version: 2\npatches:\n ignorePaths: [\"/services/legacy/\"]\n", + )); let legacy = repo.lock("services/legacy"); let test_lock = repo.lock("services/test"); let (code, stdout, stderr) = scan(&repo.root, &server.uri(), &["services/*"], &[]); assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}"); assert!(repo.lock("services/web").contains(&P_ALPHA.hosted_url())); - assert_eq!(repo.lock("services/legacy"), legacy, "ignored by patches.ignorePaths"); - assert_eq!(repo.lock("services/test"), test_lock, "a discovered test/ root is a built-in ignore"); + assert_eq!( + repo.lock("services/legacy"), + legacy, + "ignored by patches.ignorePaths" + ); + assert_eq!( + repo.lock("services/test"), + test_lock, + "a discovered test/ root is a built-in ignore" + ); assert!(stdout.contains("Policy (socket.yml)"), "{stdout}"); // Named literally, the test/ root is explicit: defaults do not apply. @@ -538,7 +624,9 @@ async fn path_globs_apply_default_ignores_and_ignore_paths_human() { async fn include_paths_limit_roots() { let server = MockServer::start().await; mount_api(&server, catalog()).await; - let repo = Repo::new(Some("version: 2\npatches:\n includePaths: [\"/services/legacy/\"]\n")); + let repo = Repo::new(Some( + "version: 2\npatches:\n includePaths: [\"/services/legacy/\"]\n", + )); let web = repo.lock("services/web"); let (code, doc) = scan_json(&repo.dir("services/web"), &server.uri(), &[], &[]); assert_eq!(code, 0, "{doc:#}"); @@ -571,7 +659,10 @@ async fn invalid_file_fails_closed_before_any_request_or_write() { assert!(message.contains("--no-socket-yml"), "{message}"); assert!(doc.get("policy").is_none()); assert_eq!(repo.snapshot(), before); - assert!(server.received_requests().await.unwrap().is_empty(), "no request before the policy loads"); + assert!( + server.received_requests().await.unwrap().is_empty(), + "no request before the policy loads" + ); // Human output names the code on stderr, same exit code. let (code, _, stderr) = scan(&repo.dir("services/web"), &server.uri(), &[], &[]); @@ -579,10 +670,20 @@ async fn invalid_file_fails_closed_before_any_request_or_write() { assert!(stderr.contains("socket_yml_invalid"), "{stderr}"); // --no-socket-yml (and its env var) skips the file. - let (code, doc) = scan_json(&repo.dir("services/web"), &server.uri(), &["--no-socket-yml", "--dry-run"], &[]); + let (code, doc) = scan_json( + &repo.dir("services/web"), + &server.uri(), + &["--no-socket-yml", "--dry-run"], + &[], + ); assert_eq!(code, 0, "{doc:#}"); assert_eq!(doc["policy"]["source"], "bypassed"); - let (code, doc) = scan_json(&repo.dir("services/web"), &server.uri(), &["--dry-run"], &[("SOCKET_NO_SOCKET_YML", "1")]); + let (code, doc) = scan_json( + &repo.dir("services/web"), + &server.uri(), + &["--dry-run"], + &[("SOCKET_NO_SOCKET_YML", "1")], + ); assert_eq!(code, 0, "{doc:#}"); assert_eq!(doc["policy"]["source"], "bypassed"); } @@ -593,7 +694,11 @@ async fn both_files_disagreeing_is_ambiguous() { let server = MockServer::start().await; mount_api(&server, catalog()).await; let repo = Repo::new(Some("version: 2\npatches:\n maxNewPatches: 1\n")); - std::fs::write(repo.root.join("socket.yaml"), "version: 2\npatches:\n maxNewPatches: 2\n").unwrap(); + std::fs::write( + repo.root.join("socket.yaml"), + "version: 2\npatches:\n maxNewPatches: 2\n", + ) + .unwrap(); let (code, doc) = scan_json(&repo.dir("services/web"), &server.uri(), &[], &[]); assert_eq!(code, 1); assert_eq!(doc["errorCode"], "socket_yml_ambiguous"); @@ -606,26 +711,66 @@ async fn severity_flag_and_env_override_the_file() { mount_api(&server, catalog()).await; let repo = Repo::new(Some("version: 2\npatches:\n minSeverity: high\n")); let web = repo.dir("services/web"); - let (code, doc) = scan_json(&web, &server.uri(), &["--dry-run", "--min-severity", "none"], &[]); + let (code, doc) = scan_json( + &web, + &server.uri(), + &["--dry-run", "--min-severity", "none"], + &[], + ); assert_eq!(code, 0, "{doc:#}"); - assert_eq!(doc["policy"]["minSeverity"], json!({"value": null, "source": "flag"})); - assert_eq!(doc["redirect"]["redirected"], 3, "beta too once the floor is lifted"); + assert_eq!( + doc["policy"]["minSeverity"], + json!({"value": null, "source": "flag"}) + ); + assert_eq!( + doc["redirect"]["redirected"], 3, + "beta too once the floor is lifted" + ); - let (code, doc) = scan_json(&web, &server.uri(), &["--dry-run"], &[("SOCKET_MIN_SEVERITY", "critical")]); + let (code, doc) = scan_json( + &web, + &server.uri(), + &["--dry-run"], + &[("SOCKET_MIN_SEVERITY", "critical")], + ); assert_eq!(code, 0, "{doc:#}"); - assert_eq!(doc["policy"]["minSeverity"], json!({"value": "critical", "source": "env"})); + assert_eq!( + doc["policy"]["minSeverity"], + json!({"value": "critical", "source": "env"}) + ); assert_eq!(doc["redirect"]["redirected"], 1); // The flag beats the env; an empty env value is unset. - let (_, doc) = scan_json(&web, &server.uri(), &["--dry-run", "--min-severity", "moderate"], &[("SOCKET_MIN_SEVERITY", "critical")]); - assert_eq!(doc["policy"]["minSeverity"], json!({"value": "medium", "source": "flag"})); - let (_, doc) = scan_json(&web, &server.uri(), &["--dry-run"], &[("SOCKET_MIN_SEVERITY", "")]); - assert_eq!(doc["policy"]["minSeverity"], json!({"value": "high", "source": "file"})); + let (_, doc) = scan_json( + &web, + &server.uri(), + &["--dry-run", "--min-severity", "moderate"], + &[("SOCKET_MIN_SEVERITY", "critical")], + ); + assert_eq!( + doc["policy"]["minSeverity"], + json!({"value": "medium", "source": "flag"}) + ); + let (_, doc) = scan_json( + &web, + &server.uri(), + &["--dry-run"], + &[("SOCKET_MIN_SEVERITY", "")], + ); + assert_eq!( + doc["policy"]["minSeverity"], + json!({"value": "high", "source": "file"}) + ); // Malformed values are usage errors. let (code, _, stderr) = scan(&web, &server.uri(), &["--min-severity", "severe"], &[]); assert_eq!(code, 2, "{stderr}"); - let (code, _, stderr) = scan(&web, &server.uri(), &[], &[("SOCKET_MIN_SEVERITY", "severe")]); + let (code, _, stderr) = scan( + &web, + &server.uri(), + &[], + &[("SOCKET_MIN_SEVERITY", "severe")], + ); assert_eq!(code, 2, "{stderr}"); assert!(stderr.contains("SOCKET_MIN_SEVERITY"), "{stderr}"); } @@ -643,12 +788,20 @@ async fn narrowing_after_a_hosted_patch_leaves_the_pin_byte_identical() { assert!(pinned.contains(&P_ALPHA.hosted_url())); // A newer merged patch appears, and the repo now ignores alpha. - std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n ignorePackages: [alpha]\n").unwrap(); + std::fs::write( + repo.root.join("socket.yml"), + "version: 2\npatches:\n ignorePackages: [alpha]\n", + ) + .unwrap(); server.reset().await; mount_api(&server, vec![P_ALPHA, P_ALPHA_MERGED_NEW]).await; let (code, doc) = scan_json(&web, &server.uri(), &[], &[]); assert_eq!(code, 0, "{doc:#}"); - assert_eq!(repo.lock("services/web"), pinned, "retained: not upgraded, not removed"); + assert_eq!( + repo.lock("services/web"), + pinned, + "retained: not upgraded, not removed" + ); let retained = &doc["policy"]["retained"][0]; assert_eq!(retained["purl"], "pkg:npm/alpha@1.0.0"); assert_eq!(retained["recordedUuid"], P_ALPHA.uuid); @@ -666,7 +819,11 @@ async fn narrowing_after_a_hosted_patch_leaves_the_pin_byte_identical() { let (code, doc) = scan_json(&web, &server.uri(), &[], &[]); assert_eq!(code, 0, "{doc:#}"); assert_eq!(repo.lock("services/web"), pinned, "{yml}"); - assert_eq!(doc["policy"]["retained"][0]["purl"], "pkg:npm/alpha@1.0.0", "{yml}: {:#}", doc["policy"]); + assert_eq!( + doc["policy"]["retained"][0]["purl"], "pkg:npm/alpha@1.0.0", + "{yml}: {:#}", + doc["policy"] + ); } } @@ -681,9 +838,15 @@ async fn enabled_false_reports_and_writes_nothing() { assert_eq!(code, 0, "{doc:#}"); assert_eq!(repo.snapshot(), before); assert_eq!(doc["policy"]["enabled"], false); - assert!(warning_codes(&doc).contains(&"patches_disabled".to_string()), "{doc:#}"); + assert!( + warning_codes(&doc).contains(&"patches_disabled".to_string()), + "{doc:#}" + ); let reasons: Vec = filtered(&doc).into_iter().map(|(_, r)| r).collect(); - assert!(!reasons.is_empty() && reasons.iter().all(|r| r == "policy_disabled"), "{reasons:?}"); + assert!( + !reasons.is_empty() && reasons.iter().all(|r| r == "policy_disabled"), + "{reasons:?}" + ); assert_eq!(doc["redirect"]["redirected"], 0); } @@ -692,7 +855,9 @@ async fn enabled_false_reports_and_writes_nothing() { async fn report_only_json_fails_when_every_detail_query_fails() { let server = MockServer::start().await; Mock::given(method("GET")) - .and(path_regex(format!("^/v0/orgs/{ORG}/patches/by-package/.+$"))) + .and(path_regex(format!( + "^/v0/orgs/{ORG}/patches/by-package/.+$" + ))) .respond_with(ResponseTemplate::new(500)) .with_priority(1) .mount(&server) @@ -705,7 +870,10 @@ async fn report_only_json_fails_when_every_detail_query_fails() { assert_eq!(code, 1, "{doc:#}"); assert_eq!(doc["status"], "error", "{doc:#}"); assert!( - doc["error"].as_str().unwrap_or_default().contains("patch-detail queries failed"), + doc["error"] + .as_str() + .unwrap_or_default() + .contains("patch-detail queries failed"), "{doc:#}" ); assert_eq!(repo.snapshot(), before); @@ -726,7 +894,11 @@ async fn recorded_merge_below_the_floor_is_kept_until_a_more_severe_patch_is_ava "the only available patch is pinned:\n{pinned}" ); - std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n minSeverity: high\n").unwrap(); + std::fs::write( + repo.root.join("socket.yml"), + "version: 2\npatches:\n minSeverity: high\n", + ) + .unwrap(); let (code, doc) = scan_json(&web, &server.uri(), &[], &[]); assert_eq!(code, 0, "{doc:#}"); assert_eq!( @@ -778,11 +950,17 @@ async fn floor_with_nothing_admitted_reports_the_withheld_patch() { let (code, stdout, stderr) = scan(&web, &server.uri(), &[], &[]); assert_eq!(code, 0, "{stdout}\n{stderr}"); assert_eq!(repo.lock("services/web"), lock); - assert!(stdout.contains("Policy (socket.yml): 1 skipped by filters"), "{stdout}"); + assert!( + stdout.contains("Policy (socket.yml): 1 skipped by filters"), + "{stdout}" + ); // Only critical/high are named without --verbose. assert!(!stdout.contains("skipped beta"), "{stdout}"); let (_, stdout, _) = scan(&web, &server.uri(), &["--verbose"], &[]); - assert!(stdout.contains("skipped pkg:npm/beta@1.0.0 (low): low < critical"), "{stdout}"); + assert!( + stdout.contains("skipped pkg:npm/beta@1.0.0 (low): low < critical"), + "{stdout}" + ); } #[tokio::test] @@ -793,9 +971,17 @@ async fn path_outside_the_repo_is_a_usage_error() { let repo = Repo::new(None); let outside = repo.root.parent().unwrap().join("elsewhere"); write_npm_root(&outside, &["alpha"]); - let (code, _, stderr) = scan(&repo.dir("services"), &server.uri(), &["web", "../../elsewhere"], &[]); + let (code, _, stderr) = scan( + &repo.dir("services"), + &server.uri(), + &["web", "../../elsewhere"], + &[], + ); assert_eq!(code, 2, "{stderr}"); - assert!(stderr.contains("is outside") && stderr.contains("run one scan per repository"), "{stderr}"); + assert!( + stderr.contains("is outside") && stderr.contains("run one scan per repository"), + "{stderr}" + ); } #[tokio::test] @@ -803,7 +989,9 @@ async fn path_outside_the_repo_is_a_usage_error() { async fn project_ignore_paths_is_honored_without_a_patches_block() { let server = MockServer::start().await; mount_api(&server, catalog()).await; - let repo = Repo::new(Some("version: 2\nprojectIgnorePaths:\n - \"services/legacy/**\"\n")); + let repo = Repo::new(Some( + "version: 2\nprojectIgnorePaths:\n - \"services/legacy/**\"\n", + )); let legacy = repo.lock("services/legacy"); let (code, doc) = scan_json(&repo.dir("services/legacy"), &server.uri(), &[], &[]); assert_eq!(code, 0, "{doc:#}"); @@ -813,10 +1001,22 @@ async fn project_ignore_paths_is_honored_without_a_patches_block() { assert_eq!(entry["detail"], "services/legacy/** (projectIgnorePaths)"); // A malformed projectIgnorePaths without a patches block only warns. - std::fs::write(repo.root.join("socket.yml"), "version: 2\nprojectIgnorePaths: {a: 1}\n").unwrap(); - let (code, doc) = scan_json(&repo.dir("services/legacy"), &server.uri(), &["--dry-run"], &[]); + std::fs::write( + repo.root.join("socket.yml"), + "version: 2\nprojectIgnorePaths: {a: 1}\n", + ) + .unwrap(); + let (code, doc) = scan_json( + &repo.dir("services/legacy"), + &server.uri(), + &["--dry-run"], + &[], + ); assert_eq!(code, 0, "{doc:#}"); - assert!(warning_codes(&doc).contains(&"socket_yml_ignored_value".to_string()), "{doc:#}"); + assert!( + warning_codes(&doc).contains(&"socket_yml_ignored_value".to_string()), + "{doc:#}" + ); } // --------------------------------------------------------------------------- @@ -845,14 +1045,18 @@ async fn agent_mode_applies_only_admitted_patches() { let (code, doc) = scan_json(&web, &server.uri(), &["--mode", "agent"], &[]); assert_eq!(code, 0, "{doc:#}"); let manifest: Value = - serde_json::from_str(&std::fs::read_to_string(web.join(".socket/manifest.json")).unwrap()).unwrap(); + serde_json::from_str(&std::fs::read_to_string(web.join(".socket/manifest.json")).unwrap()) + .unwrap(); let keys: Vec<&String> = manifest["patches"].as_object().unwrap().keys().collect(); assert_eq!(keys, ["pkg:npm/alpha@1.0.0"]); assert_eq!( std::fs::read_to_string(web.join("node_modules/alpha/index.js")).unwrap(), patched_index("alpha") ); - assert_eq!(std::fs::read_to_string(web.join("node_modules/beta/index.js")).unwrap(), orig_index("beta")); + assert_eq!( + std::fs::read_to_string(web.join("node_modules/beta/index.js")).unwrap(), + orig_index("beta") + ); } #[tokio::test] @@ -867,13 +1071,23 @@ async fn agent_mode_retains_a_recorded_patch_the_policy_now_excludes() { let manifest_before = std::fs::read(web.join(".socket/manifest.json")).unwrap(); let installed_before = std::fs::read(web.join("node_modules/alpha/index.js")).unwrap(); - std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n ecosystems: [pypi]\n").unwrap(); + std::fs::write( + repo.root.join("socket.yml"), + "version: 2\npatches:\n ecosystems: [pypi]\n", + ) + .unwrap(); server.reset().await; mount_api(&server, vec![P_ALPHA, P_ALPHA_MERGED_NEW]).await; let (code, doc) = scan_json(&web, &server.uri(), &["--mode", "agent"], &[]); assert_eq!(code, 0, "{doc:#}"); - assert_eq!(std::fs::read(web.join(".socket/manifest.json")).unwrap(), manifest_before); - assert_eq!(std::fs::read(web.join("node_modules/alpha/index.js")).unwrap(), installed_before); + assert_eq!( + std::fs::read(web.join(".socket/manifest.json")).unwrap(), + manifest_before + ); + assert_eq!( + std::fs::read(web.join("node_modules/alpha/index.js")).unwrap(), + installed_before + ); assert_eq!(doc["policy"]["retained"][0]["reason"], "policy_ecosystem"); assert_eq!(doc["policy"]["retained"][0]["upgradeAvailable"], true); } @@ -889,7 +1103,12 @@ async fn vendored_dry_run_previews_only_admitted_patches() { mount_api(&server, catalog()).await; let repo = Repo::new(Some("version: 2\npatches:\n packages: [\"pkg:npm/beta\", \"pkg:npm/left-pad\"]\n minSeverity: medium\n")); let before = repo.snapshot(); - let (code, doc) = scan_json(&repo.dir("services/web"), &server.uri(), &["--mode", "vendored", "--dry-run"], &[]); + let (code, doc) = scan_json( + &repo.dir("services/web"), + &server.uri(), + &["--mode", "vendored", "--dry-run"], + &[], + ); assert_eq!(code, 0, "{doc:#}"); assert_eq!(repo.snapshot(), before); let previewed: Vec<&str> = doc["vendor"]["patches"] @@ -899,8 +1118,14 @@ async fn vendored_dry_run_previews_only_admitted_patches() { .filter_map(|p| p["purl"].as_str()) .collect(); assert_eq!(previewed, ["pkg:npm/left-pad@1.0.0"], "{doc:#}"); - assert_eq!(filtered_reason(&doc, "pkg:npm/alpha@1.0.0")["reason"], "policy_package_not_listed"); - assert_eq!(filtered_reason(&doc, "pkg:npm/beta@1.0.0")["reason"], "policy_severity"); + assert_eq!( + filtered_reason(&doc, "pkg:npm/alpha@1.0.0")["reason"], + "policy_package_not_listed" + ); + assert_eq!( + filtered_reason(&doc, "pkg:npm/beta@1.0.0")["reason"], + "policy_severity" + ); } // --------------------------------------------------------------------------- @@ -932,9 +1157,16 @@ async fn get_bypasses_the_policy_with_a_warning() { let (code, stdout, stderr) = run_cli(&web, &args, &[]); assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}"); let doc: Value = serde_json::from_str(&stdout).unwrap(); - let warnings: Vec<&str> = doc["warnings"].as_array().unwrap().iter().filter_map(Value::as_str).collect(); + let warnings: Vec<&str> = doc["warnings"] + .as_array() + .unwrap() + .iter() + .filter_map(Value::as_str) + .collect(); assert!( - warnings.iter().any(|w| w.starts_with("(policy_bypassed)") && w.contains("alpha")), + warnings + .iter() + .any(|w| w.starts_with("(policy_bypassed)") && w.contains("alpha")), "{doc:#}" ); @@ -945,6 +1177,43 @@ async fn get_bypasses_the_policy_with_a_warning() { assert!(!stdout.contains("policy_bypassed"), "{stdout}"); } +/// B29 (#453): `get ` dispatched before the policy check, so the +/// most direct form of `get` overrode socket.yml silently. Every mode now +/// warns like the purl/CVE forms. +#[tokio::test] +#[serial] +async fn get_by_uuid_bypasses_the_policy_with_a_warning_in_every_mode() { + let server = MockServer::start().await; + mount_api(&server, catalog()).await; + let repo = Repo::new(Some("version: 2\npatches:\n ignorePackages: [alpha]\n")); + let web = repo.dir("services/web"); + for mode in ["hosted", "vendored", "agent"] { + let args = [ + "get", + P_ALPHA.uuid, + "--mode", + mode, + "--json", + "--yes", + "--dry-run", + "--cwd", + web.to_str().unwrap(), + "--api-url", + &server.uri(), + "--org", + ORG, + "--api-token", + "fake", + ]; + let (code, stdout, stderr) = run_cli(&web, &args, &[]); + assert_eq!(code, 0, "{mode}: stdout:\n{stdout}\nstderr:\n{stderr}"); + assert!( + stdout.contains("(policy_bypassed)") && stdout.contains("alpha"), + "{mode}: the envelope must carry the policy_bypassed warning: {stdout}" + ); + } +} + #[tokio::test] #[serial] async fn agent_mode_honors_path_filters_and_keeps_the_prune_universe() { @@ -960,30 +1229,65 @@ async fn agent_mode_honors_path_filters_and_keeps_the_prune_universe() { // The root is excluded by path: nothing selected, and a --sync (agent // + prune) still judges the full crawl, so no entry is pruned. - std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n includePaths: [\"/services/legacy/\"]\n").unwrap(); + std::fs::write( + repo.root.join("socket.yml"), + "version: 2\npatches:\n includePaths: [\"/services/legacy/\"]\n", + ) + .unwrap(); let (code, doc) = scan_json(&web, &server.uri(), &["--sync"], &[]); assert_eq!(code, 0, "{doc:#}"); - assert_eq!(std::fs::read(web.join(".socket/manifest.json")).unwrap(), manifest_before); + assert_eq!( + std::fs::read(web.join(".socket/manifest.json")).unwrap(), + manifest_before + ); assert_eq!(doc["policy"]["filtered"][0]["purl"], Value::Null); - assert_eq!(doc["policy"]["filtered"][0]["reason"], "policy_path_not_included"); - assert_eq!(doc["policy"]["counts"]["retained"], 2, "{:#}", doc["policy"]); - assert_eq!(doc["gc"]["removed"].as_array().map_or(0, Vec::len), 0, "{:#}", doc["gc"]); + assert_eq!( + doc["policy"]["filtered"][0]["reason"], + "policy_path_not_included" + ); + assert_eq!( + doc["policy"]["counts"]["retained"], 2, + "{:#}", + doc["policy"] + ); + assert_eq!( + doc["gc"]["removed"].as_array().map_or(0, Vec::len), + 0, + "{:#}", + doc["gc"] + ); // A narrower ecosystem list under --sync prunes nothing either. - std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n ecosystems: [pypi]\n").unwrap(); + std::fs::write( + repo.root.join("socket.yml"), + "version: 2\npatches:\n ecosystems: [pypi]\n", + ) + .unwrap(); let (code, doc) = scan_json(&web, &server.uri(), &["--sync"], &[]); assert_eq!(code, 0, "{doc:#}"); - assert_eq!(std::fs::read(web.join(".socket/manifest.json")).unwrap(), manifest_before); + assert_eq!( + std::fs::read(web.join(".socket/manifest.json")).unwrap(), + manifest_before + ); // patches.enabled: false skips the GC entirely. std::fs::remove_dir_all(web.join("node_modules/beta")).unwrap(); - let pkg_lock = repo.lock("services/web").replace("\"node_modules/beta\"", "\"node_modules/gone\""); + let pkg_lock = repo + .lock("services/web") + .replace("\"node_modules/beta\"", "\"node_modules/gone\""); std::fs::write(web.join("package-lock.json"), pkg_lock).unwrap(); - std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n enabled: false\n").unwrap(); + std::fs::write( + repo.root.join("socket.yml"), + "version: 2\npatches:\n enabled: false\n", + ) + .unwrap(); let (code, doc) = scan_json(&web, &server.uri(), &["--sync"], &[]); assert_eq!(code, 0, "{doc:#}"); assert!(doc.get("gc").is_none(), "{doc:#}"); - assert_eq!(std::fs::read(web.join(".socket/manifest.json")).unwrap(), manifest_before); + assert_eq!( + std::fs::read(web.join(".socket/manifest.json")).unwrap(), + manifest_before + ); } #[tokio::test] @@ -997,29 +1301,53 @@ async fn narrowing_after_vendoring_leaves_the_vendored_package_byte_identical() let before = compute_git_sha256_from_bytes(orig_index("alpha").as_bytes()); let after = compute_git_sha256_from_bytes(patched_index("alpha").as_bytes()); std::fs::create_dir_all(web.join(".socket/blobs")).unwrap(); - std::fs::write(web.join(".socket/blobs").join(&after), patched_index("alpha")).unwrap(); + std::fs::write( + web.join(".socket/blobs").join(&after), + patched_index("alpha"), + ) + .unwrap(); let manifest = json!({"patches": {P_ALPHA.purl(): { "uuid": P_ALPHA.uuid, "exportedAt": "2026-01-01T00:00:00Z", "files": {"package/index.js": {"beforeHash": before, "afterHash": after}}, "vulnerabilities": {}, "description": "d", "license": "MIT", "tier": "free" }}}); - std::fs::write(web.join(".socket/manifest.json"), serde_json::to_vec_pretty(&manifest).unwrap()).unwrap(); + std::fs::write( + web.join(".socket/manifest.json"), + serde_json::to_vec_pretty(&manifest).unwrap(), + ) + .unwrap(); let fixture = prebuilt_common::Server::project(&web); let (code, stdout, stderr) = run_cli( &web, &["vendor", "--json", "--cwd", web.to_str().unwrap()], - &[("SOCKET_VENDOR_URL", &fixture.uri), ("SOCKET_PATCH_SERVER_URL", &fixture.uri)], + &[ + ("SOCKET_VENDOR_URL", &fixture.uri), + ("SOCKET_PATCH_SERVER_URL", &fixture.uri), + ], ); assert_eq!(code, 0, "vendor fixture: {stdout}\n{stderr}"); - assert!(repo.lock("services/web").contains(".socket/vendor/"), "vendored lock"); + assert!( + repo.lock("services/web").contains(".socket/vendor/"), + "vendored lock" + ); let snapshot = repo.snapshot(); - std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n ignorePackages: [\"pkg:npm/alpha\"]\n").unwrap(); + std::fs::write( + repo.root.join("socket.yml"), + "version: 2\npatches:\n ignorePackages: [\"pkg:npm/alpha\"]\n", + ) + .unwrap(); let (code, doc) = scan_json(&web, &server.uri(), &["--mode", "vendored"], &[]); assert_eq!(code, 0, "{doc:#}"); let mut after_scan = repo.snapshot(); after_scan.remove("socket.yml"); - assert_eq!(after_scan, snapshot, "the vendored package, its lock wiring and ledger stay byte-identical"); - assert_eq!(doc["policy"]["retained"][0]["purl"], "pkg:npm/alpha@1.0.0", "{:#}", doc["policy"]); + assert_eq!( + after_scan, snapshot, + "the vendored package, its lock wiring and ledger stay byte-identical" + ); + assert_eq!( + doc["policy"]["retained"][0]["purl"], "pkg:npm/alpha@1.0.0", + "{:#}", + doc["policy"] + ); } - diff --git a/crates/socket-patch-cli/tests/in_process_get.rs b/crates/socket-patch-cli/tests/in_process_get.rs index 5a6bd17fa..a659d1463 100644 --- a/crates/socket-patch-cli/tests/in_process_get.rs +++ b/crates/socket-patch-cli/tests/in_process_get.rs @@ -602,6 +602,141 @@ async fn get_package_name_matches_pypi_spellings_pip_accepts() { } } +// --------------------------------------------------------------------------- +// The shared target grammar (B11, B56, B57) +// --------------------------------------------------------------------------- + +fn requested_paths(requests: &[wiremock::Request]) -> Vec { + requests.iter().map(|r| r.url.path().to_string()).collect() +} + +#[tokio::test] +#[serial] +async fn get_package_name_searches_every_installed_version_exactly() { + // B11: `get ` used to fuzzy-pick ONE installed purl (the top-level + // copy), so a nested older (vulnerable) copy was never searched, and a + // prefix sibling could be picked instead. Now: every installed version + // of the EXACT name is searched; the sibling never is. + const UUID_OLD: &str = "22222222-2222-4222-8222-222222222222"; + const PURL_OLD: &str = "pkg:npm/in-process-test@0.9.0"; + let (server, url) = start_wiremock().await; + make_search_mock_one( + &server, + "by-package", + "pkg%3Anpm%2Fin-process-test%401.0.0", + UUID, + PURL, + "free", + ) + .await; + make_search_mock_one( + &server, + "by-package", + "pkg%3Anpm%2Fin-process-test%400.9.0", + UUID_OLD, + PURL_OLD, + "free", + ) + .await; + make_view_mock(&server, UUID, PURL, "free").await; + make_view_mock(&server, UUID_OLD, PURL_OLD, "free").await; + + let tmp = tempfile::tempdir().unwrap(); + install_npm_fixture(tmp.path(), "in-process-test", "1.0.0"); + install_npm_fixture(tmp.path(), "in-process-test-extra", "1.0.0"); + install_npm_fixture( + &tmp.path() + .join("node_modules") + .join("in-process-test-extra"), + "in-process-test", + "0.9.0", + ); + + let mut args = default_args("in-process-test", tmp.path()); + args.common.api_url = Some(url); + assert_eq!(run(args).await, 0); + assert_patch_saved(tmp.path(), PURL, UUID); + assert_patch_saved(tmp.path(), PURL_OLD, UUID_OLD); + let paths = requested_paths(&server.received_requests().await.unwrap()); + assert!( + !paths.iter().any(|p| p.contains("in-process-test-extra")), + "a prefix sibling must never be searched: {paths:?}" + ); +} + +#[tokio::test] +#[serial] +async fn get_package_name_never_falls_back_to_a_near_name() { + // B11: `get yaml` with only yaml-ast-parser installed used to search and + // patch yaml-ast-parser. A near name is only suggested: no_match, exit + // 0, no API call, nothing written. + let (server, url) = start_wiremock().await; + let tmp = tempfile::tempdir().unwrap(); + install_npm_fixture(tmp.path(), "in-process-test-extra", "1.0.0"); + let mut args = default_args("in-process-test", tmp.path()); + args.common.api_url = Some(url); + assert_eq!(run(args).await, 0); + assert_no_manifest(tmp.path()); + let paths = requested_paths(&server.received_requests().await.unwrap()); + assert!(paths.is_empty(), "no API call for a near name: {paths:?}"); +} + +#[tokio::test] +#[serial] +async fn get_cve_selects_only_the_requested_ecosystems() { + // B56: `--ecosystems` used to scope only the nested apply; an advisory + // spanning npm and PyPI recorded both. + const UUID_PY: &str = "33333333-3333-4333-8333-333333333333"; + const PURL_PY: &str = "pkg:pypi/in-process-test@1.0.0"; + let (server, url) = start_wiremock().await; + Mock::given(method("GET")) + .and(path(format!("/v0/orgs/{ORG}/patches/by-cve/CVE-2024-0001"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "patches": [ + {"uuid": UUID, "purl": PURL, "publishedAt": "2024-01-01T00:00:00Z", + "description": "x", "license": "MIT", "tier": "free", "vulnerabilities": {}}, + {"uuid": UUID_PY, "purl": PURL_PY, "publishedAt": "2024-01-01T00:00:00Z", + "description": "x", "license": "MIT", "tier": "free", "vulnerabilities": {}}, + ], + "canAccessPaidPatches": false, + }))) + .mount(&server) + .await; + make_view_mock(&server, UUID, PURL, "free").await; + make_view_mock(&server, UUID_PY, PURL_PY, "free").await; + + let tmp = tempfile::tempdir().unwrap(); + let mut args = default_args("CVE-2024-0001", tmp.path()); + args.common.api_url = Some(url); + args.common.ecosystems = Some(vec!["npm".to_string()]); + assert_eq!(run(args).await, 0); + assert_patch_saved(tmp.path(), PURL, UUID); + let body = std::fs::read_to_string(tmp.path().join(".socket/manifest.json")).unwrap(); + assert!( + !body.contains(PURL_PY), + "a PyPI patch outside -e npm was recorded: {body}" + ); + let paths = requested_paths(&server.received_requests().await.unwrap()); + assert!( + !paths.iter().any(|p| p.ends_with(UUID_PY)), + "the excluded patch must not even be fetched: {paths:?}" + ); +} + +#[tokio::test] +#[serial] +async fn get_uuid_outside_the_requested_ecosystems_is_not_acted_on() { + // B57: the UUID path follows the search path's `--ecosystems` rule. + let (server, url) = start_wiremock().await; + make_view_mock(&server, UUID, PURL, "free").await; + let tmp = tempfile::tempdir().unwrap(); + let mut args = default_args(UUID, tmp.path()); + args.common.api_url = Some(url); + args.common.ecosystems = Some(vec!["pypi".to_string()]); + assert_eq!(run(args).await, 0); + assert_no_manifest(tmp.path()); +} + // --------------------------------------------------------------------------- // Network failure // --------------------------------------------------------------------------- From e076d298c506e23c692dd8b79225439994b5080f Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 11:10:07 -0400 Subject: [PATCH 03/10] Rewrite the bare-UUID shortcut on the first UUID operand socket-patch --json failed with "unexpected argument '--json'" because the shortcut only looked at argv[1]. It now fires on the first UUID-shaped token before any subcommand name, using the core target grammar's is_uuid_shaped; the CLI's looks_like_uuid copy is deleted. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/src/lib.rs | 56 +++++++++++++++++++----------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/crates/socket-patch-cli/src/lib.rs b/crates/socket-patch-cli/src/lib.rs index 32e01e9eb..ace45a1f9 100644 --- a/crates/socket-patch-cli/src/lib.rs +++ b/crates/socket-patch-cli/src/lib.rs @@ -18,6 +18,7 @@ pub mod ui; pub mod update_notifier; use clap::{Parser, Subcommand}; +use socket_patch_core::utils::target::is_uuid_shaped; // CLI contract surface — subcommand names, visible_alias values, flag names, // defaults, JSON shapes, and exit codes are PUBLIC and SEMVER-SIGNIFICANT. @@ -238,24 +239,6 @@ pub fn try_parse_cli(argv: &[String]) -> Result { Cli::from_arg_matches_mut(&mut matches).map_err(|e| e.format(&mut cli_command())) } -/// Check whether `s` looks like a UUID (8-4-4-4-12 hex pattern). -/// -/// Used by [`parse_argv_with_shortcuts`] to detect the convenience form -/// `socket-patch ` and rewrite it to `socket-patch get `, and -/// by rollback's target resolver to decide whether a no-match identifier -/// error should hint at the path-glob spelling. -pub(crate) fn looks_like_uuid(s: &str) -> bool { - let parts: Vec<&str> = s.split('-').collect(); - if parts.len() != 5 { - return false; - } - let expected = [8, 4, 4, 4, 12]; - parts - .iter() - .zip(expected.iter()) - .all(|(p, &len)| p.len() == len && p.chars().all(|c| c.is_ascii_hexdigit())) -} - /// Parse a full argv vector with two convenience rewrites on failure: /// `--update [...]` becomes the hidden `self-update` subcommand, and a /// bare `` becomes `get `. Returns the original clap error if @@ -316,7 +299,22 @@ pub fn parse_argv_with_shortcuts(argv: Vec) -> Result Err(_) => Err(err), }; } - if argv.len() >= 2 && looks_like_uuid(&argv[1]) { + // The UUID shortcut keys on the first UUID-shaped token before + // any subcommand name, not just argv[1], so root-position flags + // are fine: `socket-patch --json ` is `get --json `. + // The shape is the shared target grammar's + // ([`is_uuid_shaped`]), the same one `get` classifies with. + let subcommands: Vec = cli_command() + .get_subcommands() + .flat_map(|c| std::iter::once(c.get_name()).chain(c.get_all_aliases())) + .map(str::to_string) + .collect(); + let uuid_operand = argv + .iter() + .skip(1) + .take_while(|a| !subcommands.contains(a)) + .any(|a| is_uuid_shaped(a)); + if uuid_operand { let mut new_args = vec![argv[0].clone(), "get".into()]; new_args.extend_from_slice(&argv[1..]); match try_parse_cli(&new_args) { @@ -344,8 +342,9 @@ mod tests { //! uses — both of which are part of the CLI contract (see //! `CLI_CONTRACT.md`). use super::*; + use socket_patch_core::utils::target::is_uuid_shaped as looks_like_uuid; - // ---------- looks_like_uuid ---------- + // ---------- looks_like_uuid (the shared core UUID shape) ---------- #[test] fn looks_like_uuid_accepts_canonical_lowercase() { @@ -466,6 +465,23 @@ mod tests { } } + /// The shortcut keys on the first UUID-shaped operand, not just + /// argv[1]: `socket-patch --json ` used to fail with + /// "unexpected argument '--json'". + #[test] + fn fallback_rewrites_a_uuid_after_leading_flags() { + let cli = parse_argv_with_shortcuts(argv(&["socket-patch", "--json", UUID])).unwrap(); + match cli.command { + Commands::Get(args) => { + assert_eq!(args.identifier, UUID); + assert!(args.common.json); + } + _ => panic!("expected the get subcommand"), + } + // A UUID after a real subcommand is that subcommand's operand. + assert!(parse_argv_with_shortcuts(argv(&["socket-patch", "list", UUID])).is_err()); + } + #[test] fn fallback_returns_original_error_when_first_arg_is_not_uuid() { // No rewrite should happen; the original clap error must surface. From 37753c2241140874102469d4278d2e6b20456b41 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 11:10:07 -0400 Subject: [PATCH 04/10] Document the shared package target grammar Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 3 ++- crates/socket-patch-cli/CLI_CONTRACT.md | 12 +++++++----- docs/usage.md | 3 ++- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index af6b393c0..ce33f0607 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,8 @@ socket-patch vendor # eject an existing hosted patch set socket-patch rollback # restore upstream dependencies ``` -`get` also accepts a GHSA, patch UUID, PURL, or package name. `scan` selects from +`get` also accepts a GHSA, patch UUID, PURL, or exact package name (every installed +version of that name is searched; near names are only suggested). `scan` selects from patches your account can download, preferring the highest severity, then the most advisories fixed, then the newest publication date. Existing patches are upgraded only by a better-ranked patch. diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 7eddb127c..f8b177122 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -34,7 +34,9 @@ Rows are in `--help` order (v5.0): the hosted/vendored workflow (`scan` → `vex **Lock lifecycle (v5.0).** `<.socket>/apply.lock` never outlives the command that took it: acquisition creates `.socket/` when it is missing, the guard's drop unlinks the file WHILE the lock is still held (so a waiter can never lock an orphaned inode), releases it, and then removes `.socket/` itself if that left the directory empty — a run that had nothing to persist leaves no `.socket/` behind, and there is nothing to `.gitignore`. A leftover file from a crashed (SIGKILLed) run is reclaimed in place and removed by the next lock-taking command. The lock is taken by `apply`, `rollback`, `remove`, `repair`, `vendor`, agent-mode `get` and `scan --apply`/`--sync` (download → manifest write → nested apply is ONE lock window — the nested apply never re-acquires), and `scan`/`get` in vendored **and hosted** mode — hosted acquires it around its first wet write (the takeover pre-reverts), never on `--dry-run` and never when the run would write nothing, so hosted previews and no-op runs create no `.socket/`. Dry runs of the other commands may still take the lock; it is residue-free either way. A live holder is `lock_held` (exit 1); a directory or special file squatting on `.socket/` or on the lock path is a lock I/O error — `lock_io` (exit 1, `failed to open lock file at : …`; a read-only project root surfaces the same code at the acquire, before any ledger or manifest write) — never `lock_held`. -**Bare-UUID fallback.** `socket-patch ` is rewritten to `socket-patch get `. The UUID shape checked is the standard 8-4-4-4-12 hex pattern (case-insensitive). See [`src/lib.rs::looks_like_uuid`](src/lib.rs). +**Bare-UUID fallback.** `socket-patch ` is rewritten to `socket-patch get `, also when root-position flags come first (`socket-patch --json `); a UUID after a subcommand name is that subcommand's operand. The UUID shape checked is the standard 8-4-4-4-12 hex pattern (case-insensitive), the target grammar's. + +**Target grammar (v5.0).** `get`, `remove`, `rollback` and the bare-UUID fallback classify their package/patch token with one parser (core `utils::target`): a UUID; `CVE-…` / `GHSA-…` (case-insensitive); a `pkg:` purl (with a version: that release, a base purl covering every release variant and a `?qualified` one exactly one; without a version: every version); otherwise an **exact** package name — full name or last segment, case-insensitive, PEP 503 for PyPI — the same matcher as `scan --package` and `socket.yml`. A name never matches by prefix or substring. `get ` searches every installed version of the matched name (within `--ecosystems`) and prints `Matched: …` on stderr; with no exact match it is `no_match` (exit 0, no API call) and, in human mode, suggests up to five near names (`Did you mean: …?`) without acting on them. `remove` / `rollback` accept the same names and versionless purls against manifest records, vendor-ledger entries and hosted pins; CVE/GHSA ids match no record there. **Root `--update` flag.** `socket-patch --update [VERSION]` updates the binary itself from GitHub Releases. It is a root flag, not a subcommand: argv is rewritten (the same mechanism as the bare-UUID fallback) onto an internal hidden subcommand whose name carries no stability guarantee — script the flag, never the internal name. Combining the flag with a subcommand (`socket-patch --update scan`) is a usage error (exit 2). Full contract: [Self-update contract](#self-update-contract-socket-patch---update). @@ -103,8 +105,8 @@ Beyond the globals above, each subcommand defines a small set of local arguments | `scan` | `--min-severity ` | `SOCKET_MIN_SEVERITY` | (v5.0) Severity floor for the patch a package may receive (worst advisory severity; unknown severity is skipped whenever a floor is set). Beats `patches.minSeverity`; the flag beats the env; `none` lifts the floor. A malformed value is exit 2. | | `get`, `scan` | `--all-releases` | `SOCKET_ALL_RELEASES` | Download patches for every release/distribution variant of a matched package — PyPI wheel/sdist (`artifact_id`), RubyGems (`platform`), Maven (`classifier`) — not just the one(s) matching the locally-installed distribution. On `scan` this makes the stored manifest portable across environments (e.g. cross-platform CI caches). On `get` (v3.6) it ALSO disables the coarse installed-**version** narrowing of CVE/GHSA fan-outs (see "get --mode and installed narrowing"): every found version's patch is fetched, installed or not | | `get` | positional `identifier`; `--id` / `--cve` / `--ghsa` / `--package` (`-p`); `--save-only` (alias `--no-apply`); `--mode ` | `SOCKET_SAVE_ONLY` | Patch lookup + consumption mode (v3.6). `--mode` reuses scan's value enum (same hidden value aliases `host`/`redirect`/`vendor`; deliberately no env binding, matching scan). Default (v5.0): `hosted`, like scan; `agent` (save + apply in place) when `--save-only` or `--global`/`--global-prefix` is given. An explicit `--mode hosted\|vendored` with `--global`/`--global-prefix` is a usage error (exit 2, scan's wording: global installs have no project lockfile). An explicit `--save-only` conflicts with `--mode hosted\|vendored` — rejected with **exit 1** via get's established self-enforced-conflict style (unlike scan's exit-2 mode conflicts; see the exit-code table) | -| `remove` | positional `identifier`; `--skip-rollback`; `--preserve-state` (v5.0) | `SOCKET_SKIP_ROLLBACK`, `SOCKET_PRESERVE_STATE` | Manifest entry removal. `--preserve-state` is the single-patch twin of `rollback --preserve-state`: restore the tree and unwind the identifier's vendored/hosted wiring, but keep the manifest entry, the vendored artifact + ledger entry, and skip all GC. Combining it with `--skip-rollback` is a self-enforced usage error (exit 2): one flag keeps the tree and drops the state, the other restores the tree and keeps the state — together they select the do-nothing quadrant ("the combination would be a no-op: nothing would change"). The conflict fires whether either flag is spelled on the command line or sourced from its env var | -| `rollback` | optional variadic positional `targets` (PURL \| UUID \| path glob); `--preserve-state` (v5.0) | `SOCKET_PRESERVE_STATE` | Rollback scope. Multiple targets union. A token becomes a path glob ONLY when it is path-SHAPED — contains a separator (`/` or `\`) or a glob metacharacter (`*?[`), or starts with `./`, or is absolute; a `pkg:` prefix is a PURL and every other bare word keeps identifier (PURL/UUID) semantics, so a mistyped identifier or truncated UUID stays a safe exit-1 "No patch found matching identifier: X" (with a hint suggesting `./X` or `X/**` for directory targeting) instead of silently becoming a path scope. An unparseable glob is a usage error (exit 2) | +| `remove` | positional `identifier` (target grammar: UUID, PURL with or without version, or exact package name); `--skip-rollback`; `--preserve-state` (v5.0) | `SOCKET_SKIP_ROLLBACK`, `SOCKET_PRESERVE_STATE` | Manifest entry removal. `--preserve-state` is the single-patch twin of `rollback --preserve-state`: restore the tree and unwind the identifier's vendored/hosted wiring, but keep the manifest entry, the vendored artifact + ledger entry, and skip all GC. Combining it with `--skip-rollback` is a self-enforced usage error (exit 2): one flag keeps the tree and drops the state, the other restores the tree and keeps the state — together they select the do-nothing quadrant ("the combination would be a no-op: nothing would change"). The conflict fires whether either flag is spelled on the command line or sourced from its env var | +| `rollback` | optional variadic positional `targets` (PURL \| UUID \| path glob); `--preserve-state` (v5.0) | `SOCKET_PRESERVE_STATE` | Rollback scope. Multiple targets union. A token becomes a path glob ONLY when it is path-SHAPED — contains a separator (`/` or `\`) or a glob metacharacter (`*?[`), or starts with `./`, or is absolute (an npm `@scope/name` is a name, not a path); a `pkg:` prefix is a PURL and every other token keeps the target grammar (UUID, PURL or exact package name), so a mistyped identifier or truncated UUID stays a safe exit-1 "No patch found matching identifier: X" (with a hint suggesting `./X` or `X/**` for directory targeting) instead of silently becoming a path scope. An unparseable glob is a usage error (exit 2) | | `vex` | `--output` / `-O`, `--product`, `--no-verify`, `--doc-id`, `--compact` | `SOCKET_VEX_OUTPUT`, `SOCKET_VEX_PRODUCT`, `SOCKET_VEX_NO_VERIFY`, `SOCKET_VEX_DOC_ID`, `SOCKET_VEX_COMPACT` | OpenVEX 0.2.0 document generation; see "vex output channels" below | | `repair` | `--download-only` | `SOCKET_DOWNLOAD_ONLY` | Repair-specific cleanup mode (mutually exclusive with `--offline`; combining them is a usage error, exit 2) | @@ -885,7 +887,7 @@ worse, lets a warm cache silently serve unpatched bytes): ### Targets -`rollback [TARGET]...` — zero or more targets, unioned. `pkg:` tokens are PURLs (base purl matches every release variant; qualified purl exact), other identifier-shaped tokens are UUIDs, and only **path-shaped** tokens (separator, glob metachar `*?[`, `./` prefix, or absolute) are path globs — see the per-subcommand args table for the safety rationale. Identifier matching runs across ALL THREE sources (a hosted pin matches by purl or by the patch uuid in its hosted URL); an identifier matching nothing anywhere is the familiar exit-1 error. Path globs use the same matcher as `scan [PATHS]` (ancestor rule, `require_literal_separator`, absolute-only outside `--cwd`, Windows case-insensitive): installed copies of every candidate purl are discovered and purls with ≥ 1 matching copy are selected. Scoping sentences (shared with scan): +`rollback [TARGET]...` — zero or more targets, unioned. `pkg:` tokens are PURLs (base purl matches every release variant; qualified purl exact; versionless purl every version), other tokens follow the target grammar (UUID or exact package name), and only **path-shaped** tokens (separator, glob metachar `*?[`, `./` prefix, or absolute) are path globs — see the per-subcommand args table for the safety rationale. Identifier matching runs across ALL THREE sources (a hosted pin matches by purl or by the patch uuid in its hosted URL); an identifier matching nothing anywhere is the familiar exit-1 error. Path globs use the same matcher as `scan [PATHS]` (ancestor rule, `require_literal_separator`, absolute-only outside `--cwd`, Windows case-insensitive): installed copies of every candidate purl are discovered and purls with ≥ 1 matching copy are selected. Scoping sentences (shared with scan): * **A target that selects nothing is an error on `rollback` (exit 1) and an empty scan on `scan` (exit 0).** Each rollback path pattern must select at least one patched package; the error names the pattern and the reachability rule. * **Path targets select installed copies; entries with no installed copy are reachable only by identifier or unscoped runs.** @@ -1720,7 +1722,7 @@ package those same binaries. See [the release runbook](../../docs/releasing.md). Every item in this document is locked in by at least one of: - **clap parser snapshots** in `crates/socket-patch-cli/tests/cli_parse_*.rs` — assert flag names, short forms, defaults, aliases, and CSV delimiters by calling `socket_patch_cli::Cli::try_parse_from(...)`. -- **Helper unit tests** in `crates/socket-patch-cli/src/**` (`#[cfg(test)] mod tests` blocks) — cover `looks_like_uuid`, `parse_argv_with_shortcuts`, `detect_identifier_type`, `select_patches`, `find_patches_to_rollback`, `partition_purls`, the JSON serializers, and the terminal UI in `src/ui/` (`StatusLine` redraw/clear/`println` byte streams, `confirm_with` answers and non-interactive notes, `select_one`'s JSON/empty guards, `plural`, `truncate`, the `color_enabled` truth table, `paint`/`severity`, and `pad`/`strip_ansi` alignment). +- **Helper unit tests** in `crates/socket-patch-cli/src/**` (`#[cfg(test)] mod tests` blocks) — cover `parse_argv_with_shortcuts`, `forced_identifier_error`, `installed_target_matches`, `select_patches`, `find_patches_to_rollback`, `partition_purls`, the JSON serializers, and the terminal UI in `src/ui/` (`StatusLine` redraw/clear/`println` byte streams, `confirm_with` answers and non-interactive notes, `select_one`'s JSON/empty guards, `plural`, `truncate`, the `color_enabled` truth table, `paint`/`severity`, and `pad`/`strip_ansi` alignment). - **Async `run()` integration tests** in `tests/cli_parse_list.rs`, `tests/cli_parse_remove.rs` — exercise the no-network error paths and assert JSON shape via `serde_json::from_str::` + per-key assertions. If you add a new flag/subcommand/JSON key, add a test here that locks the new surface in the same PR. diff --git a/docs/usage.md b/docs/usage.md index 338b63f17..b0f4d92db 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -33,7 +33,8 @@ socket-patch get pkg:npm/lodash@4.17.20 --mode agent ``` Replace the example identifiers with the advisory or package you need. `get` also -accepts a patch UUID or package name. It defaults to hosted mode; `--save-only` and +accepts a patch UUID or an exact package name, which covers every installed version +of that package. `remove` and `rollback` take the same names. `get` defaults to hosted mode; `--save-only` and global targeting default to agent mode instead. Hosted and vendored `get` do not prompt. Agent-mode searches can offer an interactive choice. From 645d005252472fd6e118ddc35d3c604245329518 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 12:27:06 -0400 Subject: [PATCH 05/10] Refuse package names that select several packages A bare name matches its full name or its last segment, so one short token could select several packages: `remove core` removed the patches for both @angular/core and @babel/core, and `get v2` reached every Go v2+ module. get, remove and rollback act on one package per name, so they now refuse a name whose matches cover more than one package identity (exit 1, naming each as a versionless purl; remove's JSON code is ambiguous_target). A Go major-version suffix is never a name. scan --package and socket.yml keep their semantics. Also: - rollback tries a slash-containing token (composer vendor/pkg, a go module path) as a target before treating it as a path glob, so it takes the same names as get and remove. - get checks --ecosystems before the paid gate, the "Found patch" line and the patch_fetched event. - get runs its per-version searches concurrently through ordered_concurrent; a failed search still fails the run. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/CLI_CONTRACT.md | 9 +- crates/socket-patch-cli/src/commands/get.rs | 66 ++-- .../socket-patch-cli/src/commands/remove.rs | 25 ++ .../socket-patch-cli/src/commands/rollback.rs | 73 ++++- .../tests/in_process_target_ambiguity.rs | 305 ++++++++++++++++++ crates/socket-patch-core/src/utils/target.rs | 154 ++++++++- docs/usage.md | 4 +- 7 files changed, 599 insertions(+), 37 deletions(-) create mode 100644 crates/socket-patch-cli/tests/in_process_target_ambiguity.rs diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index f8b177122..621e87df5 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -22,7 +22,7 @@ For task-oriented guidance, start with [usage](../../docs/usage.md), | `list` | — | Print patches in the local manifest, plus the vendor ledger's records (v5.0) and the hosted pins the lockfiles wire (labeled; see the action matrix; an empty project exits 0) | | `get` | `download` | Fetch a selected patch in hosted mode by default; `--mode agent` selects in-place application, also the default with `--save-only` or global targeting. Requires positional `identifier`. | | `apply` | — | Agent mode: apply patches from the local manifest | -| `rollback` | — | **Full-state rollback (v5.0, MAJOR)**: restore original files AND unwind vendored lockfile wiring / restore hosted pins to their upstream registry entries, remove the rolled-back entries from the manifest, and GC their blobs/archives; takes optional variadic positional `targets` (PURL \| UUID \| path glob). See [Rollback command contract](#rollback-command-contract-v50) | +| `rollback` | — | **Full-state rollback (v5.0, MAJOR)**: restore original files AND unwind vendored lockfile wiring / restore hosted pins to their upstream registry entries, remove the rolled-back entries from the manifest, and GC their blobs/archives; takes optional variadic positional `targets` (PURL \| UUID \| package name \| path glob). See [Rollback command contract](#rollback-command-contract-v50) | | `remove` | — | Restore and remove one patch across hosted, vendored, and agent state; requires positional `identifier`. | | `repair` | `gc` | Download missing agent blobs, redownload missing/corrupt vendored artifacts (never re-synthesizing a lost ledger), and clean up unused ones (refuses with `lock_held` when a live process holds the lock; see "Lock lifecycle" below) | @@ -36,7 +36,7 @@ Rows are in `--help` order (v5.0): the hosted/vendored workflow (`scan` → `vex **Bare-UUID fallback.** `socket-patch ` is rewritten to `socket-patch get `, also when root-position flags come first (`socket-patch --json `); a UUID after a subcommand name is that subcommand's operand. The UUID shape checked is the standard 8-4-4-4-12 hex pattern (case-insensitive), the target grammar's. -**Target grammar (v5.0).** `get`, `remove`, `rollback` and the bare-UUID fallback classify their package/patch token with one parser (core `utils::target`): a UUID; `CVE-…` / `GHSA-…` (case-insensitive); a `pkg:` purl (with a version: that release, a base purl covering every release variant and a `?qualified` one exactly one; without a version: every version); otherwise an **exact** package name — full name or last segment, case-insensitive, PEP 503 for PyPI — the same matcher as `scan --package` and `socket.yml`. A name never matches by prefix or substring. `get ` searches every installed version of the matched name (within `--ecosystems`) and prints `Matched: …` on stderr; with no exact match it is `no_match` (exit 0, no API call) and, in human mode, suggests up to five near names (`Did you mean: …?`) without acting on them. `remove` / `rollback` accept the same names and versionless purls against manifest records, vendor-ledger entries and hosted pins; CVE/GHSA ids match no record there. +**Target grammar (v5.0).** `get`, `remove`, `rollback` and the bare-UUID fallback classify their package/patch token with one parser (core `utils::target`): a UUID; `CVE-…` / `GHSA-…` (case-insensitive); a `pkg:` purl (with a version: that release, a base purl covering every release variant and a `?qualified` one exactly one; without a version: every version); otherwise an **exact** package name — full name or last segment, case-insensitive, PEP 503 for PyPI — the same matcher as `scan --package` and `socket.yml`. A name never matches by prefix or substring, and a Go major-version suffix (`v2`) is never a name. **Ambiguous names**: `get`, `remove` and `rollback` act on one package per name, so a name whose last-segment rule reaches several packages (`core` → `@angular/core` and `@babel/core`; the same name in two ecosystems) is refused with exit 1 before anything is searched or changed — `"core" is ambiguous: it names pkg:npm/@angular/core, pkg:npm/@babel/core; use the full name or a purl` (`remove --json`: `errorCode: "ambiguous_target"`; `get` / `rollback --json`: `{status: "error", error}`). Several versions of one package are not ambiguous. `scan --package` and `socket.yml` keep selecting every package the name reaches. `get ` searches every installed version of the matched name (within `--ecosystems`) and prints `Matched: …` on stderr; with no exact match it is `no_match` (exit 0, no API call) and, in human mode, suggests up to five near names (`Did you mean: …?`) without acting on them. `remove` / `rollback` accept the same names and versionless purls against manifest records, vendor-ledger entries and hosted pins; CVE/GHSA ids match no record there. A name containing `/` (composer `vendor/pkg`, a go module path) is path-shaped, so `rollback` first tries it as a name: when it selects a recorded or hosted patch it is a target, otherwise a path glob. **Root `--update` flag.** `socket-patch --update [VERSION]` updates the binary itself from GitHub Releases. It is a root flag, not a subcommand: argv is rewritten (the same mechanism as the bare-UUID fallback) onto an internal hidden subcommand whose name carries no stability guarantee — script the flag, never the internal name. Combining the flag with a subcommand (`socket-patch --update scan`) is a usage error (exit 2). Full contract: [Self-update contract](#self-update-contract-socket-patch---update). @@ -106,7 +106,7 @@ Beyond the globals above, each subcommand defines a small set of local arguments | `get`, `scan` | `--all-releases` | `SOCKET_ALL_RELEASES` | Download patches for every release/distribution variant of a matched package — PyPI wheel/sdist (`artifact_id`), RubyGems (`platform`), Maven (`classifier`) — not just the one(s) matching the locally-installed distribution. On `scan` this makes the stored manifest portable across environments (e.g. cross-platform CI caches). On `get` (v3.6) it ALSO disables the coarse installed-**version** narrowing of CVE/GHSA fan-outs (see "get --mode and installed narrowing"): every found version's patch is fetched, installed or not | | `get` | positional `identifier`; `--id` / `--cve` / `--ghsa` / `--package` (`-p`); `--save-only` (alias `--no-apply`); `--mode ` | `SOCKET_SAVE_ONLY` | Patch lookup + consumption mode (v3.6). `--mode` reuses scan's value enum (same hidden value aliases `host`/`redirect`/`vendor`; deliberately no env binding, matching scan). Default (v5.0): `hosted`, like scan; `agent` (save + apply in place) when `--save-only` or `--global`/`--global-prefix` is given. An explicit `--mode hosted\|vendored` with `--global`/`--global-prefix` is a usage error (exit 2, scan's wording: global installs have no project lockfile). An explicit `--save-only` conflicts with `--mode hosted\|vendored` — rejected with **exit 1** via get's established self-enforced-conflict style (unlike scan's exit-2 mode conflicts; see the exit-code table) | | `remove` | positional `identifier` (target grammar: UUID, PURL with or without version, or exact package name); `--skip-rollback`; `--preserve-state` (v5.0) | `SOCKET_SKIP_ROLLBACK`, `SOCKET_PRESERVE_STATE` | Manifest entry removal. `--preserve-state` is the single-patch twin of `rollback --preserve-state`: restore the tree and unwind the identifier's vendored/hosted wiring, but keep the manifest entry, the vendored artifact + ledger entry, and skip all GC. Combining it with `--skip-rollback` is a self-enforced usage error (exit 2): one flag keeps the tree and drops the state, the other restores the tree and keeps the state — together they select the do-nothing quadrant ("the combination would be a no-op: nothing would change"). The conflict fires whether either flag is spelled on the command line or sourced from its env var | -| `rollback` | optional variadic positional `targets` (PURL \| UUID \| path glob); `--preserve-state` (v5.0) | `SOCKET_PRESERVE_STATE` | Rollback scope. Multiple targets union. A token becomes a path glob ONLY when it is path-SHAPED — contains a separator (`/` or `\`) or a glob metacharacter (`*?[`), or starts with `./`, or is absolute (an npm `@scope/name` is a name, not a path); a `pkg:` prefix is a PURL and every other token keeps the target grammar (UUID, PURL or exact package name), so a mistyped identifier or truncated UUID stays a safe exit-1 "No patch found matching identifier: X" (with a hint suggesting `./X` or `X/**` for directory targeting) instead of silently becoming a path scope. An unparseable glob is a usage error (exit 2) | +| `rollback` | optional variadic positional `targets` (PURL \| UUID \| package name \| path glob); `--preserve-state` (v5.0) | `SOCKET_PRESERVE_STATE` | Rollback scope. Multiple targets union. A token becomes a path glob ONLY when it is path-SHAPED — contains a separator (`/` or `\`) or a glob metacharacter (`*?[`), or starts with `./`, or is absolute (an npm `@scope/name` is a name, not a path; a relative slash name without glob metacharacters or `.`/`..` segments — composer `vendor/pkg`, a go module path — is a name when it selects a recorded or hosted patch); a `pkg:` prefix is a PURL and every other token keeps the target grammar (UUID, PURL or exact package name), so a mistyped identifier or truncated UUID stays a safe exit-1 "No patch found matching identifier: X" (with a hint suggesting `./X` or `X/**` for directory targeting) instead of silently becoming a path scope. An unparseable glob is a usage error (exit 2) | | `vex` | `--output` / `-O`, `--product`, `--no-verify`, `--doc-id`, `--compact` | `SOCKET_VEX_OUTPUT`, `SOCKET_VEX_PRODUCT`, `SOCKET_VEX_NO_VERIFY`, `SOCKET_VEX_DOC_ID`, `SOCKET_VEX_COMPACT` | OpenVEX 0.2.0 document generation; see "vex output channels" below | | `repair` | `--download-only` | `SOCKET_DOWNLOAD_ONLY` | Repair-specific cleanup mode (mutually exclusive with `--offline`; combining them is a usage error, exit 2) | @@ -184,7 +184,7 @@ The rewriter reads a fixed set of candidate files from the project root: the npm * **Vendored** (`get GHSA-… --mode vendored`): the download phase is scan's vendored posture — **manifest-free (v5.0)**: the selected records are fetched into memory (`download_patch_records`; no blob staging; nothing under `.socket/` is written; the nested apply never runs), then scan's vendor step runs under the apply lock over exactly the selected records, like `scan --mode vendored` (no whole-manifest scope and no `[note]` about other records — that blast radius is retired with the manifest; a legacy manifest record for a vendored purl is migrated out of `.socket/manifest.json` the same way scan does it). JSON: get's envelope takes the detached download envelope's shape — `{status, found, downloaded, skipped, failed, detached: true, patches: [{purl, uuid, action: "downloaded" | "skipped" | "failed", …}], warnings?}` (`applied` is absent; `detached: true` is pinned; a `downloaded` record for a purl the vendor ledger holds at another uuid carries the additive `oldUuid`, derived from the ledger — the human `[fetch]` line reads ` (replacing )`) — and gains the nested `vendor` Envelope exactly like scan's `result["vendor"]`; a vendor-step error folds the partial envelope + `{status:"error", error:{code,message}}` in (a pre-failure takeover reconcile may have already mutated the ledger — its events must reach the consumer). Exit: download failures or vendor `has_errors` → `partial_failure`/1. Human prompt: `Download and vendor N patches?`; `--dry-run` prints `[dry-run] Would download and vendor N patches. No changes made.` on both identifier paths (uuid and search). Telemetry mirrors scan's vendored arms (`track_outcomes_for_vendor` / `track_patch_vendor_failed`). **Bun vendored preflight (additive)** — shared by `get --mode vendored` on both its paths and `scan --mode vendored`: before ANY patch download, and only when the selection holds a `pkg:npm/` purl, the download phase reads `bun.lock`/`bun.lockb` once (`preflight_vendor`) and, when the vendor backend would refuse the project — a malformed, unreadable or unsupported `bun.lockb` → `vendor_bun_lockb_invalid`; an unreadable `bun.lock` → `vendor_lockfile_missing`; a `lockfileVersion` other than 0/1/2 or a non-canonical `packages` grammar → `vendor_lockfile_version_unsupported`; `workspace:` packages in a lock below version 2 → `vendor_bun_workspace_unsupported` — every `pkg:npm/` result becomes `{action:"failed", errorCode:, error:}` with NO fetch (the patch view is never requested) and no patch record; other ecosystems' results are untouched. **Search path** (`get --mode vendored`) and `scan --mode vendored`: the records ride `patches[]` / `download.patches[]` with `downloaded: 0`, the download phase writes nothing under `.socket/` (v5.0 — a pre-existing `.socket/manifest.json`, including a record seeded for another purl, is left byte-untouched), the vendor step still runs over the remaining records (no event for the refused purl), exit `partial_failure`/1. **uuid path** (`get --mode vendored`): the uuid lookup is the only fetch; the run exits 1 BEFORE the vendor step with exactly `{status:"error", found:1, downloaded:0, skipped:0, failed:1, error:{code, message}, patches:[{purl, uuid, action:"failed", errorCode, error}]}` (the `error` OBJECT is the vendored-mode error shape of the vendor-step fold-in above) and writes nothing — no `.socket/` on a fresh project; human mode prints `Error (): ` on stderr. **Already-vendored exemption**: a purl is exempt from the workspace refusal only when every instance of its `name@version` in `bun.lock` is already a `.socket/vendor/npm/…` local tuple (any uuid; the digest-less 2-tuple counts) — the engine's own criterion — so in-sync re-runs, `repair`, and a superseding patch uuid on a project vendored before it grew a workspace member all flow to the engine (re-pinning an already-local tuple adds no workspace-relative exposure); a wiped ledger alone is not a refusal (the engine path decides). UUID equality in the ledger alone never exempts a purl: `rollback --preserve-state` retains its record after unwiring. Dry-run refusal takes priority over `already_vendored`. **Unreadable vendor ledger**: a `.socket/vendor/state.json` the preflight cannot read or parse is itself the refusal — `vendor_state_unreadable` with the io/parse detail, fail-closed (nothing is exempt) — on the uuid path, the search / `scan` path and the `--dry-run` preview alike; never a Bun lock code. **`--silent`** is "errors only" and never mutes the refusal: the code-tagged `[error] (): ` (per-patch paths) / `Error (): …` (uuid path) line stays on stderr with an empty stdout. **`--dry-run`** previews the refusal as the additive `would_refuse` action (see `--dry-run` below). Agent-mode `get --save-only` is NOT preflighted (record-only intent has no consumption precondition). Pinned by `tests/vendor/in_process_vendor_bun.rs` (exact uuid-path envelope, seeded-manifest survival, `--silent`, `--dry-run`) and `tests/scan_vendor_e2e.rs`. **Lock-text refusals before the download (v5.0)** — shared by `get --mode vendored` on both its paths and `scan --mode vendored`, after the Bun preflight above and the ledger's `already vendored` skip: a `pkg:npm/` result in a **pnpm, yarn classic or yarn berry** project, or a `pkg:cargo/` result, that its vendor backend refuses on the project's lock and manifest text alone is refused BEFORE its patch view is fetched — the pnpm / classic / berry gates the backend runs before it reads the package (coordinates, the lock and manifest reads and their line-ending / version / `cacheKey` / `.yarnrc.yml` gates, override and `resolutions` conflicts, the lock entry present and rewritable) and cargo's `locked_version_mismatch` (only when it is the crate's FIRST refusal; an in-tree `cargo vendor` copy still refuses in the loop as `already_vendored_in_tree`). **Scope:** only a package the vendor loop would hand to its backend is refused early — one installed on disk (the loop's own qualified-aware resolver plus the npm identity lookup), or one the lockfile inventory resolves to a verifiable registry source (a lock entry with an integrity, or the ledger-recovered pre-vendor resolution — exactly the entry the pristine fetch would use). A package absent from the lock and not installed never reached its backend and is untouched: its view is fetched, it downloads, and the vendor loop skips it `skipped` / `package_not_installed` as in v4.x (so cargo's `locked_version_mismatch` is refused early only for a crate installed at the unlocked version). The result becomes `{action:"failed", errorCode:, error:}` in `download.patches[]` / `patches[]` with the backend's exact code and detail, no view and no pristine fetch, no patch record, and therefore no vendor event: compared with v4.x, `download.downloaded` drops and `download.failed` rises by the number of such packages, `vendor.summary.failed` and `vendor.events` lose their `failed` events, and a lockfile-only package among them loses its `vendor_fetched_missing` event (it is never fetched). Exit code and top-level `status` are unchanged (`partial_failure`/1); the nested `vendor.status` becomes `success` when those refusals were the vendor step's only failures (observed on the depscan fixture: 3 refusals, `partialFailure` → `success`), and when every selected package is refused this way the human `scan --vendor` arm prints `Nothing was vendored: N patches failed (see above).`. **Precedence:** the lock-text refusal is decided before the view, so it wins over every view-derived outcome — a package that would also have been a paid-access 403 (`[PAID]`/no access), a failed view fetch, or a no-applicable-files skip reports the lock refusal instead (the Bun refusal and the ledger's `already vendored` skip still come first). The human `[error] (): ` line is printed during the download instead of the vendor step's failure line (the human (non-`--silent`) `scan --vendor` arm's baseline pre-check still fetches the views it verifies; only the download, the pristine fetch and the vendor step skip the package there). A purl the lockfiles pin hosted keeps the loop's refusal (its takeover restore rewrites the lock the gates read); other flavors (package-lock, pnpm-legacy, bun) and ecosystems are untouched, and `--dry-run` is unchanged. `vendor` (manifest-driven, no view fetch) keeps its per-package `failed` events but no longer fetches the pristine source of a lockfile-only package it refuses this way — the source is deferred to the backend, which refuses before reading it (no `vendor_fetched_missing` event and no registry request; a refused package whose registry is unreachable reports the gate's code instead of `vendor_fetch_failed`); only a package the lock resolves to a verifiable source is deferred, and one it does not resolve keeps its `package_not_installed` skip. Pinned by `tests/scan_vendor_e2e.rs` (`exact_download_plan`: scan and exact-purl get, pnpm and cargo scope), `tests/e2e_yarn_legacy_cachekey_refusal_build.rs` and `tests/vendor/vendor_rerun_no_network_e2e.rs`. -* **Installed-version narrowing** (all modes, `get`'s search path): a CVE/GHSA fan-out returns one patch record per patched VERSION; get keeps only versions present here and emits calm `skipped` records (`errorCode: "package_not_installed"`) for the rest — never an error exit. Presence = installed on disk (qualified-aware resolver) ∪ already tracked in the manifest (record maintenance keeps working on hosts without an installed copy); hosted/vendored modes additionally count lockfile-resolved deps and vendor-ledger purls (mirroring scan's discovery supplements, including their `--global` gate). **Exempt** (no narrowing): UUID identifiers, exact-versioned PURL identifiers (explicit intent), `--save-only` runs (record-only has no installation precondition — the fresh-clone record→vendor flow keeps working), `--all-releases`, and the package-name path (already installed-derived). When EVERY found patch is filtered out, get exits 0 with the additive status **`not_installed`** (`{status:"not_installed", found:N, downloaded:0, applied:0, patches:[], warnings?}`) — never `no_match`, which remains pinned to the fuzzy package-name path. PnP layouts are surfaced, not misreported: yarn-PnP npm results skip with `errorCode: "yarn_pnp_unsupported"` in every mode; pnpm-PnP skips carry `pnpm_pnp_unsupported` in agent/vendored modes; hosted mode — the refusal's own remedy — keeps ONLY the versions the raw `pnpm-lock.yaml` text actually resolves (boundary-anchored probe over the v5/v6/v9 key spellings, so a large fan-out never requests grants for every version ever patched), labels a JUDGED miss `package_not_installed` exactly like a non-PnP project (the layout blocked nothing — the lock was read and the version isn't resolved), and reserves the layout code for an unreadable lock (no judgment possible). When EVERY narrowed-out result is a PnP refusal, the human terminal names the layout instead of claiming "not installed" and never advises `--all-releases` (which cannot make PnP patchable); the JSON status stays `not_installed` — consumers dispatch on the per-record `errorCode`. Hosted mode also runs the per-release VARIANT filter (`filter_to_installed_releases`) on its search path before requesting grants — agent/vendored runs get it inside the download engines — with the same keep-all-plus-warning fallbacks (surfaced as `(release_narrowing)`-prefixed strings in `warnings[]`). An ecosystem this binary has no crawler for is likewise never judged: its results are KEPT (absence from a crawl that never looked carries no information — the same fail-safe as scan's prune GC). The human `Found N patches:` listing shows only the patches whose package version survived the narrowing (the narrowing is judged over every result, so an installed package's paid fix a free user cannot download still lists as `[PAID] (no access)`, while skip records and counts cover only accessible patches), sorted by PURL in natural version order (`4.17.2` before `4.17.10`); the narrowed-out ones are summarized on stderr in one line per reason (`Skipped N patches for M package versions not installed here (use --all-releases to include them).`), and `--verbose` adds one `[skip] ()` line per skipped version after that summary, in natural version order. When the candidates hold more patches than were selected and the pick was made without a menu (a paid user's auto-pick, `--yes`, a non-TTY run), a `Selected:` block names the patch (purl, tier, short uuid, advisories) that will be installed before the prompt. Machine output (the prompt count, the JSON envelope) uses the kept set, unchanged. The finer per-release variant narrowing (`filter_to_installed_releases`) is unchanged and still runs inside the download engines (and before an agent-mode `--dry-run` preview, so the preview names only the variants a wet run would fetch). +* **Installed-version narrowing** (all modes, `get`'s search path): a CVE/GHSA fan-out returns one patch record per patched VERSION; get keeps only versions present here and emits calm `skipped` records (`errorCode: "package_not_installed"`) for the rest — never an error exit. Presence = installed on disk (qualified-aware resolver) ∪ already tracked in the manifest (record maintenance keeps working on hosts without an installed copy); hosted/vendored modes additionally count lockfile-resolved deps and vendor-ledger purls (mirroring scan's discovery supplements, including their `--global` gate). **Exempt** (no narrowing): UUID identifiers, exact-versioned PURL identifiers (explicit intent), `--save-only` runs (record-only has no installation precondition — the fresh-clone record→vendor flow keeps working), `--all-releases`, and the package-name path (already installed-derived). When EVERY found patch is filtered out, get exits 0 with the additive status **`not_installed`** (`{status:"not_installed", found:N, downloaded:0, applied:0, patches:[], warnings?}`) — never `no_match`, which remains pinned to the package-name path (exact names; near names are only suggested). PnP layouts are surfaced, not misreported: yarn-PnP npm results skip with `errorCode: "yarn_pnp_unsupported"` in every mode; pnpm-PnP skips carry `pnpm_pnp_unsupported` in agent/vendored modes; hosted mode — the refusal's own remedy — keeps ONLY the versions the raw `pnpm-lock.yaml` text actually resolves (boundary-anchored probe over the v5/v6/v9 key spellings, so a large fan-out never requests grants for every version ever patched), labels a JUDGED miss `package_not_installed` exactly like a non-PnP project (the layout blocked nothing — the lock was read and the version isn't resolved), and reserves the layout code for an unreadable lock (no judgment possible). When EVERY narrowed-out result is a PnP refusal, the human terminal names the layout instead of claiming "not installed" and never advises `--all-releases` (which cannot make PnP patchable); the JSON status stays `not_installed` — consumers dispatch on the per-record `errorCode`. Hosted mode also runs the per-release VARIANT filter (`filter_to_installed_releases`) on its search path before requesting grants — agent/vendored runs get it inside the download engines — with the same keep-all-plus-warning fallbacks (surfaced as `(release_narrowing)`-prefixed strings in `warnings[]`). An ecosystem this binary has no crawler for is likewise never judged: its results are KEPT (absence from a crawl that never looked carries no information — the same fail-safe as scan's prune GC). The human `Found N patches:` listing shows only the patches whose package version survived the narrowing (the narrowing is judged over every result, so an installed package's paid fix a free user cannot download still lists as `[PAID] (no access)`, while skip records and counts cover only accessible patches), sorted by PURL in natural version order (`4.17.2` before `4.17.10`); the narrowed-out ones are summarized on stderr in one line per reason (`Skipped N patches for M package versions not installed here (use --all-releases to include them).`), and `--verbose` adds one `[skip] ()` line per skipped version after that summary, in natural version order. When the candidates hold more patches than were selected and the pick was made without a menu (a paid user's auto-pick, `--yes`, a non-TTY run), a `Selected:` block names the patch (purl, tier, short uuid, advisories) that will be installed before the prompt. Machine output (the prompt count, the JSON envelope) uses the kept set, unchanged. The finer per-release variant narrowing (`filter_to_installed_releases`) is unchanged and still runs inside the download engines (and before an agent-mode `--dry-run` preview, so the preview names only the variants a wet run would fetch). * **Deliberate divergences from scan** (documented, not drift): agent-mode get keeps its `selection_required` JSON posture for free multi-patch PURLs (scan and, v5.0, hosted/vendored get auto-pick); get has no `--vex` (an ambient `SOCKET_VEX` is ignored by get's modes), no `--prune`; get does not run scan's pre-vendor baseline annotation; and an all-narrowed-out run exits `not_installed` without entering the vendor step (heal-after-wipe re-vendoring stays `scan --mode vendored`'s job). Agent-mode `get` honors `--dry-run` too (v5.0): the search and uuid paths classify each selected patch against the manifest (read-only; an unreadable manifest fails closed like the wet run) and stop before the prompt, the download, any `.socket/` write and the apply — human `[would-add]` / `[would-update] … (replacing )` / `[skip] … (already in manifest)` lines then `[dry-run] Would download and apply N patches. No changes made.`; JSON `{status:"success", dryRun:true, found, downloaded:0, skipped, applied:0, patches:[{purl, uuid, action:"would_add"|"would_update"(+oldUuid)|"skipped"}, ], warnings?}`, exit 0. `--dry-run` previews what `apply` / `rollback` / `scan --apply` / `repair` / `remove` — and `get` in every mode (hosted/vendored since v3.6, agent since v5.0) — would do without mutating disk. `get --mode hosted --dry-run` flows through the hosted engine's dry-run contract (no lock, no `.socket/`, no lockfile writes, `redirect.dryRun: true`); `get --mode vendored --dry-run` emits the same ledger-classification preview as scan's (`would_vendor` / `already_vendored` / `would_revendor`+`oldUuid` under the nested `vendor` key — plus, additive, `would_refuse` + `errorCode` + `error` for npm purls the wet run's Bun preflight would refuse: an in-sync `already_vendored` entry is exempt, as is a `would_revendor` entry whose `bun.lock` instances are all already local tuples; a purl the lock still resolves from the registry is refused like a fresh one, and the preview stays exit 0 / `status: "success"` with nothing written) before any download, and both skip the confirm prompt (nothing to confirm). In JSON mode, the envelope is populated with would-be actions and counts (`remove --dry-run` skips the confirmation prompt — there is nothing to confirm — and flips its would-be `Removed` events to `Verified` previews, so `summary.removed` stays "entries actually deleted"). `rollback --dry-run` (v5.0) previews every leg — the in-place restore verification, the vendored unwire (`Would revert/unwire vendoring for …`), the hosted upstream restore (every pin is resolved exactly like a wet run — registry lookups included, so a pin the wet run would refuse is previewed as that refusal — and nothing is flushed to disk), the manifest removals (simulated in memory), and the blob/archive GC — with no writes and no prompt. @@ -1347,6 +1347,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | Code | Subcommands | Meaning | |-----------------------|----------------------------------|---------| | `manifest_not_found` | remove, repair, rollback, vex (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: the vendor ledger and the lockfiles are resolved against the root the RESOLVED manifest path implies (its `.socket` parent's parent in the standard layout, else the manifest file's directory — exactly `--cwd` for the default path), 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: "Manifest not found", path}` shape, not this envelope code); 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 | 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 | Manifest exists but is unparseable. | | `manifest_unreadable` | list, remove, vex | I/O error reading manifest (vex: also an unparseable manifest; exit 2). | | `no_patches` | vex | The manifest file exists but is empty AND no vendor-ledger record or lockfile reference names a patch (exit 1). | diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index 47ac23ec2..c5a980855 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -2741,6 +2741,23 @@ pub async fn run(args: GetArgs) -> i32 { status.finish(); match fetch_result { Ok(Some(patch)) => { + // The search path's selection rules hold here too: a patch + // outside `--ecosystems` is never acted on — checked before + // the paid gate, the "Found patch" line and the fetched + // event, since it is not this run's patch. + if !args.common.purl_ecosystem_selected(&patch.purl) { + if args.common.json { + print_json(&empty_result_json("not_found")); + } else if !args.common.silent { + println!( + "No patch found with UUID: {} in the selected ecosystems \ + (it patches {})", + args.identifier, + normalize_purl(&patch.purl) + ); + } + return 0; + } if patch.tier == "paid" && use_public_proxy { return report_paid_required_uuid( &args, @@ -2781,22 +2798,8 @@ pub async fn run(args: GetArgs) -> i32 { ) .await; let selected = vec![search_result_from_response(&patch)]; - // The search path's selection rules hold here too: a patch - // outside `--ecosystems` is never acted on, and acting - // against the repo's socket.yml says so (`policy_bypassed`). - if !args.common.purl_ecosystem_selected(&patch.purl) { - if args.common.json { - print_json(&empty_result_json("not_found")); - } else if !args.common.silent { - println!( - "No patch found with UUID: {} in the selected ecosystems \ - (it patches {})", - args.identifier, - normalize_purl(&patch.purl) - ); - } - return 0; - } + // Acting against the repo's socket.yml says so + // (`policy_bypassed`). let policy_warnings = super::scan::policy::policy_bypass_warnings(&args.common, &selected); if !args.common.silent { @@ -2947,6 +2950,13 @@ pub async fn run(args: GetArgs) -> i32 { return 0; } + // A name reaching several packages by last segment (`core` → + // `@angular/core` and `@babel/core`) is refused: `get` acts on + // one package per name. + if let Some(msg) = target.ambiguity(matched.iter().map(String::as_str)) { + report_error(args.common.json, msg); + return 1; + } if !quiet { eprintln!("{}", format_matched_packages(&matched)); } @@ -2954,11 +2964,23 @@ pub async fn run(args: GetArgs) -> i32 { patches: Vec::new(), can_access_paid_patches: false, }; - for purl in &matched { - status.set(format!("Searching patches for {}...", normalize_purl(purl))); - let result = api_client.search_patches_by_package(purl).await; - status.finish(); - match result { + // One search per installed version, concurrently, merged in + // `matched` order. Any failed search still fails the run (as + // the single search did): a partial result could silently + // miss the patch for the version that failed. + status.set(format!( + "Searching patches for {}...", + crate::ui::plural(matched.len(), "installed version", "installed versions") + )); + let window_len = matched.len(); + let api = &api_client; + let mut searches = std::pin::pin!(ordered_concurrent( + matched.iter(), + api_concurrency_for(api.uses_public_proxy(), window_len), + |purl| async move { hold_back_debug(api.search_patches_by_package(purl)).await }, + )); + while let Some(held) = searches.next().await { + match held.release() { Ok(r) => { merged.can_access_paid_patches |= r.can_access_paid_patches; for patch in r.patches { @@ -2968,6 +2990,7 @@ pub async fn run(args: GetArgs) -> i32 { } } Err(e) => { + status.finish(); return report_fetch_failure( &args.identifier, e, @@ -2980,6 +3003,7 @@ pub async fn run(args: GetArgs) -> i32 { } } } + status.finish(); merged } _ => unreachable!(), diff --git a/crates/socket-patch-cli/src/commands/remove.rs b/crates/socket-patch-cli/src/commands/remove.rs index 22e26656d..395b95233 100644 --- a/crates/socket-patch-cli/src/commands/remove.rs +++ b/crates/socket-patch-cli/src/commands/remove.rs @@ -460,6 +460,31 @@ pub async fn run(args: RemoveArgs) -> i32 { Ok(VendorState::default()) }; + // A name reaching several packages by last segment (`core` → + // `@angular/core` and `@babel/core`) is refused across every store: + // `remove` acts on one package per name. + { + let ledger_purls: Vec<&str> = vendor_state_result + .as_ref() + .map(|state| state.entries.keys().map(String::as_str).collect()) + .unwrap_or_default(); + let candidates = manifest + .patches + .keys() + .map(String::as_str) + .chain(ledger_purls) + .chain(hosted_pins.iter().map(|pin| pin.purl.as_str())); + if let Some(msg) = target.ambiguity(candidates) { + emit_error_envelope( + args.common.json, + args.common.dry_run, + "ambiguous_target", + msg, + ); + return 1; + } + } + if matching.is_empty() { // Ledger-only entries (vendored mode keeps no manifest record) — // `remove` is their per-purl exit path (alongside `vendor diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index b0bf16905..9a5b55aa3 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -388,6 +388,10 @@ pub(crate) enum RollbackTarget { /// shared target grammar, so a truncated UUID or a mistyped name stays a /// safe "No patch found matching identifier" error instead of silently /// selecting a directory subtree. +/// +/// A path-shaped token without glob metacharacters (composer +/// `vendor/pkg`, a go module path) is promoted back to a name once the +/// stores are loaded, when it selects a recorded or hosted patch. pub(crate) fn classify_target(token: &str) -> RollbackTarget { if is_path_shaped(token) { RollbackTarget::PathGlob(token.to_string()) @@ -396,6 +400,18 @@ pub(crate) fn classify_target(token: &str) -> RollbackTarget { } } +/// A path-shaped token that could also be a slash-containing package +/// name: relative, no glob metacharacter or backslash, no `.` / `..` +/// segment (`./x` and `x/..` are always paths). +fn is_name_shaped_path(token: &str) -> bool { + !token.contains(['*', '?', '[', '\\']) + && !std::path::Path::new(token).is_absolute() + && !token.starts_with('/') + && token + .split('/') + .all(|seg| !seg.is_empty() && seg != "." && seg != "..") +} + struct PatchToRollback { purl: String, patch: PatchRecord, @@ -1216,6 +1232,36 @@ pub async fn run(args: RollbackArgs) -> i32 { .map(|pin| (pin.purl.clone(), pin.uuid.clone())) .collect(); + let ledgers = socket_patch_core::ledgers::Ledgers { + manifest: Some(&manifest), + vendor: vendor_state_result.as_ref().ok(), + redirect: None, + }; + // A slash-containing package name (composer `vendor/pkg`, a go module + // path) is shaped like a path, but is a target first: one that selects + // a recorded or hosted patch is an identifier, as in `get` and + // `remove`; only otherwise is it a path glob. + let (identifiers, path_scope) = { + let mut identifiers = identifiers; + let mut globs: Vec = Vec::new(); + for raw in path_scope.raw() { + let named = is_name_shaped_path(raw).then(|| Target::parse(raw)).filter(|t| { + t.kind() == TargetKind::Name + && (!ledgers.matching(t).is_empty() + || redirect_records + .iter() + .any(|(purl, uuid)| t.matches_patch(purl, uuid))) + }); + match named { + Some(t) => identifiers.push(t), + None => globs.push(raw.clone()), + } + } + let path_scope = + crate::path_scope::PathScope::parse(&globs).expect("a subset of the parsed patterns"); + (identifiers, path_scope) + }; + let scoped = !identifiers.is_empty() || !path_scope.is_empty(); // Identifier matching runs across ALL THREE stores; an identifier @@ -1228,23 +1274,36 @@ pub async fn run(args: RollbackArgs) -> i32 { vendor_scope.extend(vendor_entries.iter().map(|(k, _)| k.clone())); hosted_scope.extend(redirect_records.iter().map(|(p, _)| p.clone())); } - let ledgers = socket_patch_core::ledgers::Ledgers { - manifest: Some(&manifest), - vendor: vendor_state_result.as_ref().ok(), - redirect: None, - }; for id in &identifiers { let found = ledgers.matching(id); let mut matched = !found.is_empty(); - manifest_scope.extend(found.manifest); - vendor_scope.extend(found.vendor.into_iter().map(|(k, _)| k)); // Hosted pins live in the lockfiles, not in a store. + let mut hosted_found: Vec<&str> = Vec::new(); for (purl, uuid) in &redirect_records { if id.matches_patch(purl, uuid) { hosted_scope.insert(purl.clone()); + hosted_found.push(purl); matched = true; } } + // A name reaching several packages by last segment (`core` → + // `@angular/core` and `@babel/core`) is refused across every + // store: `rollback` acts on one package per name. + let ambiguity = id.ambiguity( + found + .manifest + .iter() + .map(String::as_str) + .chain(found.vendor.iter().map(|(k, _)| k.as_str())) + .chain(hosted_found), + ); + manifest_scope.extend(found.manifest); + vendor_scope.extend(found.vendor.into_iter().map(|(k, _)| k)); + if let Some(msg) = ambiguity { + track_patch_rollback_failed(&msg, api_token.as_deref(), org_slug.as_deref()).await; + emit_rollback_error(args.common.json, &msg); + return 1; + } if !matched { let hint = if matches!(id.kind(), TargetKind::Purl | TargetKind::Uuid) { String::new() diff --git a/crates/socket-patch-cli/tests/in_process_target_ambiguity.rs b/crates/socket-patch-cli/tests/in_process_target_ambiguity.rs new file mode 100644 index 000000000..dc047d8b8 --- /dev/null +++ b/crates/socket-patch-cli/tests/in_process_target_ambiguity.rs @@ -0,0 +1,305 @@ +//! The shared target grammar on the verbs that act on one package per +//! name: `get`, `remove` and `rollback` refuse a bare name whose +//! last-segment rule reaches several packages (`core` → `@angular/core` +//! and `@babel/core`), a Go major-version suffix (`v2`) is never a name, +//! and `rollback` treats a slash-containing package name (composer +//! `vendor/pkg`) as a target before it treats it as a path glob. +//! +//! In-process, against manifest-only fixtures: nothing is installed, so +//! every assertion is about which records a token selects. + +use std::path::Path; + +use serial_test::serial; +use socket_patch_cli::commands::get::{run as get_run, GetArgs}; +use socket_patch_cli::commands::remove::{run as remove_run, RemoveArgs}; +use socket_patch_cli::commands::rollback::{run as rollback_run, RollbackArgs}; +use wiremock::matchers::{method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +const ORG: &str = "test-org"; + +fn record(uuid: &str) -> String { + format!( + r#"{{ + "uuid": "{uuid}", + "exportedAt": "2024-01-01T00:00:00Z", + "files": {{ "package/index.js": {{ + "beforeHash": "0000000000000000000000000000000000000000000000000000000000000000", + "afterHash": "1111111111111111111111111111111111111111111111111111111111111111" + }}}}, + "vulnerabilities": {{}}, "description": "x", + "license": "MIT", "tier": "free" + }}"# + ) +} + +/// Write `.socket/manifest.json` holding one record per `(purl, uuid)`; +/// returns the manifest text. +fn write_manifest(cwd: &Path, records: &[(&str, &str)]) -> String { + let body: Vec = records + .iter() + .map(|(purl, uuid)| format!("\"{purl}\": {}", record(uuid))) + .collect(); + let text = format!("{{ \"patches\": {{ {} }} }}", body.join(", ")); + let socket = cwd.join(".socket"); + std::fs::create_dir_all(&socket).unwrap(); + std::fs::write(socket.join("manifest.json"), &text).unwrap(); + std::fs::write( + cwd.join("package.json"), + r#"{"name":"r","version":"0.0.0"}"#, + ) + .unwrap(); + text +} + +fn read_manifest(cwd: &Path) -> String { + std::fs::read_to_string(cwd.join(".socket/manifest.json")).unwrap() +} + +fn common(cwd: &Path) -> socket_patch_cli::args::GlobalArgs { + socket_patch_cli::args::GlobalArgs { + cwd: cwd.to_path_buf(), + manifest_path: ".socket/manifest.json".to_string(), + yes: true, + json: true, + offline: true, + ..socket_patch_cli::args::GlobalArgs::default() + } +} + +fn remove_args(cwd: &Path, identifier: &str) -> RemoveArgs { + RemoveArgs { + common: common(cwd), + identifier: identifier.to_string(), + skip_rollback: true, + preserve_state: false, + } +} + +fn rollback_args(cwd: &Path, targets: &[&str]) -> RollbackArgs { + let mut common = common(cwd); + common.dry_run = true; + RollbackArgs { + targets: targets.iter().map(|t| t.to_string()).collect(), + common, + preserve_state: false, + } +} + +const ANGULAR: (&str, &str) = ( + "pkg:npm/%40angular/core@17.0.0", + "a1a1a1a1-0000-4000-8000-000000000001", +); +const BABEL: (&str, &str) = ( + "pkg:npm/@babel/core@7.0.0", + "b2b2b2b2-0000-4000-8000-000000000002", +); +const GO_V2: (&str, &str) = ( + "pkg:golang/github.com/x/y/v2@v2.0.0", + "c3c3c3c3-0000-4000-8000-000000000003", +); +const GO_Z_V2: (&str, &str) = ( + "pkg:golang/github.com/x/z/v2@v2.1.0", + "d4d4d4d4-0000-4000-8000-000000000004", +); +const MONOLOG: (&str, &str) = ( + "pkg:composer/monolog/monolog@2.9.0", + "e5e5e5e5-0000-4000-8000-000000000005", +); + +#[tokio::test] +#[serial] +async fn remove_refuses_a_name_that_reaches_two_packages() { + let tmp = tempfile::tempdir().unwrap(); + let before = write_manifest(tmp.path(), &[ANGULAR, BABEL]); + assert_eq!(remove_run(remove_args(tmp.path(), "core")).await, 1); + assert_eq!(read_manifest(tmp.path()), before, "nothing may be removed"); + + // The full name selects one package and removes only it. + assert_eq!(remove_run(remove_args(tmp.path(), "@babel/core")).await, 0); + let after = read_manifest(tmp.path()); + assert!(after.contains("angular"), "{after}"); + assert!(!after.contains("@babel/core"), "{after}"); +} + +#[tokio::test] +#[serial] +async fn remove_never_treats_a_go_major_suffix_as_a_name() { + let tmp = tempfile::tempdir().unwrap(); + let before = write_manifest(tmp.path(), &[GO_V2, GO_Z_V2]); + // `v2` names no package: not_found (exit 1), nothing removed. + assert_eq!(remove_run(remove_args(tmp.path(), "v2")).await, 1); + assert_eq!(read_manifest(tmp.path()), before); + // The module path still works. + assert_eq!( + remove_run(remove_args(tmp.path(), "github.com/x/y/v2")).await, + 0 + ); + let after = read_manifest(tmp.path()); + assert!(!after.contains("github.com/x/y/v2"), "{after}"); + assert!(after.contains("github.com/x/z/v2"), "{after}"); +} + +#[tokio::test] +#[serial] +async fn rollback_refuses_a_name_that_reaches_two_packages() { + let tmp = tempfile::tempdir().unwrap(); + write_manifest(tmp.path(), &[ANGULAR, BABEL]); + assert_eq!(rollback_run(rollback_args(tmp.path(), &["core"])).await, 1); + // One package by full name is fine (dry run: nothing installed). + assert_eq!( + rollback_run(rollback_args(tmp.path(), &["@angular/core"])).await, + 0 + ); +} + +#[tokio::test] +#[serial] +async fn rollback_never_treats_a_go_major_suffix_as_a_name() { + let tmp = tempfile::tempdir().unwrap(); + write_manifest(tmp.path(), &[GO_V2, GO_Z_V2]); + std::fs::write( + tmp.path().join("go.mod"), + "module example.com/app\n\ngo 1.21\n", + ) + .unwrap(); + assert_eq!(rollback_run(rollback_args(tmp.path(), &["v2"])).await, 1); + // The versionless module purl selects it (dry run: nothing installed). + assert_eq!( + rollback_run(rollback_args(tmp.path(), &["pkg:golang/github.com/x/y/v2"])).await, + 0 + ); +} + +#[tokio::test] +#[serial] +async fn rollback_takes_a_slash_package_name_as_a_target() { + // `remove monolog/monolog` selects the composer record; rollback used + // to read the same token as a path glob that matched no installed + // copy (exit 1). + let tmp = tempfile::tempdir().unwrap(); + write_manifest(tmp.path(), &[MONOLOG, BABEL]); + assert_eq!( + rollback_run(rollback_args(tmp.path(), &["monolog/monolog"])).await, + 0 + ); + assert_eq!( + rollback_run(rollback_args(tmp.path(), &["github.com/x/y/v2"])).await, + 1, + "a slash token selecting no record stays a path glob (matches nothing)" + ); +} + +fn install_npm(cwd: &Path, name: &str, version: &str) { + let dir = cwd.join("node_modules").join(name); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("package.json"), + serde_json::json!({ "name": name, "version": version }).to_string(), + ) + .unwrap(); +} + +fn get_args(identifier: &str, cwd: &Path, api_url: String) -> GetArgs { + GetArgs { + common: socket_patch_cli::args::GlobalArgs { + org: Some(ORG.to_string()), + cwd: cwd.to_path_buf(), + yes: true, + api_token: Some("fake-token-for-tests".to_string()), + api_url: Some(api_url), + json: true, + download_mode: "diff".to_string(), + ..socket_patch_cli::args::GlobalArgs::default() + }, + identifier: identifier.to_string(), + id: false, + cve: false, + ghsa: false, + package: false, + save_only: true, + all_releases: false, + mode: None, + } +} + +#[tokio::test] +#[serial] +async fn get_refuses_a_name_that_reaches_two_installed_packages() { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + install_npm(tmp.path(), "@angular/core", "17.0.0"); + install_npm(tmp.path(), "@babel/core", "7.0.0"); + assert_eq!(get_run(get_args("core", tmp.path(), server.uri())).await, 1); + assert!(!tmp.path().join(".socket").exists(), "nothing recorded"); + let searched: Vec = server + .received_requests() + .await + .unwrap() + .iter() + .map(|r| r.url.path().to_string()) + .filter(|p| p.contains("/patches/")) + .collect(); + assert!( + searched.is_empty(), + "no search for an ambiguous name: {searched:?}" + ); +} + +/// A UUID outside `--ecosystems` is refused before the "Found patch" line +/// and before the `patch_fetched` telemetry event: it is not this run's +/// patch. +#[tokio::test] +#[serial] +async fn get_uuid_outside_the_ecosystems_sends_no_fetched_event() { + const UUID: &str = "11111111-1111-4111-8111-111111111111"; + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path(format!("/v0/orgs/{ORG}/patches/view/{UUID}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "uuid": UUID, + "purl": "pkg:npm/in-process-test@1.0.0", + "publishedAt": "2024-01-01T00:00:00Z", + "files": {}, + "vulnerabilities": {}, + "description": "x", + "license": "MIT", + "tier": "free", + }))) + .mount(&server) + .await; + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + let tmp = tempfile::tempdir().unwrap(); + let mut args = get_args(UUID, tmp.path(), server.uri()); + args.common.ecosystems = Some(vec!["pypi".to_string()]); + + // Telemetry resolves its endpoint from the environment. + std::env::set_var("SOCKET_API_URL", server.uri()); + std::env::set_var("SOCKET_API_TOKEN", "fake"); + std::env::set_var("SOCKET_ORG_SLUG", ORG); + std::env::remove_var("SOCKET_TELEMETRY_DISABLED"); + std::env::remove_var("SOCKET_OFFLINE"); + std::env::remove_var("VITEST"); + let code = get_run(args).await; + std::env::remove_var("SOCKET_API_URL"); + std::env::remove_var("SOCKET_API_TOKEN"); + std::env::remove_var("SOCKET_ORG_SLUG"); + assert_eq!(code, 0); + assert!(!tmp.path().join(".socket").exists()); + let bodies: Vec = server + .received_requests() + .await + .unwrap() + .iter() + .filter(|r| r.url.path().ends_with("/telemetry")) + .map(|r| String::from_utf8_lossy(&r.body).into_owned()) + .collect(); + assert!( + !bodies.iter().any(|b| b.contains("patch_fetched")), + "a refused UUID must not record a fetch: {bodies:?}" + ); +} diff --git a/crates/socket-patch-core/src/utils/target.rs b/crates/socket-patch-core/src/utils/target.rs index a427328b8..8719aab4d 100644 --- a/crates/socket-patch-core/src/utils/target.rs +++ b/crates/socket-patch-core/src/utils/target.rs @@ -13,15 +13,23 @@ //! * anything else is a package name, matched EXACTLY (full name or last //! segment, case-insensitive, PEP 503 for PyPI) by //! [`crate::policy::package_spec_matches`] — the matcher `scan --package` -//! and `socket.yml` already use. There is no fuzzy matching here. +//! and `socket.yml` already use. There is no fuzzy matching here. A Go +//! major-version suffix (`v2` in `github.com/x/y/v2`) is never a name. +//! +//! A name's last-segment rule can select several packages (`core` → +//! `@angular/core` and `@babel/core`). `get`, `remove` and `rollback` act +//! on one package per name, so they refuse such a name +//! ([`Target::ambiguity`]) and ask for the full name or a purl; +//! `scan --package` and `socket.yml` keep selecting all of them. //! //! Verbs reject the kinds they cannot act on (a CVE matches no manifest //! entry, for example) rather than reinterpreting them. use std::fmt; +use crate::crawlers::python_crawler::canonicalize_pypi_name; use crate::policy::package_spec_matches; -use crate::utils::purl::{is_purl, purl_matches_identifier, strip_purl_qualifiers}; +use crate::utils::purl::{canonical_purl, is_purl, purl_matches_identifier, strip_purl_qualifiers}; /// What a target token names. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -120,11 +128,50 @@ impl Target { /// ignored), a versionless purl or a name selects every version. pub fn matches_package(&self, purl: &str) -> bool { match self.kind { - TargetKind::Purl | TargetKind::Name => package_spec_matches(&self.text, purl), + TargetKind::Purl => package_spec_matches(&self.text, purl), + TargetKind::Name => self.name_matches(purl), TargetKind::Uuid | TargetKind::Cve | TargetKind::Ghsa => false, } } + /// The name rule: [`package_spec_matches`], except that a Go + /// major-version suffix (`v2`) never selects a module by its last + /// segment (`github.com/x/y/v2` is `y`, major version 2). + fn name_matches(&self, purl: &str) -> bool { + if is_go_major_suffix(self.text.trim()) && is_golang_purl(purl) { + return false; + } + package_spec_matches(&self.text, purl) + } + + /// For a package name, the distinct packages it selects among `purls` + /// when there is more than one: `Some(message)` naming each (as a + /// versionless purl) and how to pick one. `None` for every other kind, + /// and for a name that selects one package (any number of its + /// versions) or none. + /// + /// `get`, `remove` and `rollback` refuse an ambiguous name instead of + /// acting on every package it reaches by last segment. + pub fn ambiguity<'a>(&self, purls: impl IntoIterator) -> Option { + if self.kind != TargetKind::Name { + return None; + } + let mut packages: Vec = purls + .into_iter() + .filter(|purl| self.name_matches(purl)) + .filter_map(package_identity) + .collect(); + packages.sort(); + packages.dedup(); + (packages.len() > 1).then(|| { + format!( + "\"{}\" is ambiguous: it names {}; use the full name or a purl", + self.text, + packages.join(", ") + ) + }) + } + /// Does this target select the recorded patch `(purl, uuid)` — a /// manifest record, a vendor-ledger entry or a hosted pin? /// @@ -143,12 +190,47 @@ impl Target { purl_matches_identifier(purl, &self.text) } TargetKind::Purl => package_spec_matches(&self.text, purl), - TargetKind::Name => uuid == self.text || package_spec_matches(&self.text, purl), + TargetKind::Name => uuid == self.text || self.name_matches(purl), TargetKind::Cve | TargetKind::Ghsa => false, } } } +/// A package's identity: its purl without version, qualifiers or subpath, +/// decoded and lowercased (`pkg:npm/@babel/core`), the PyPI name in its +/// PEP 503 form. Two purls with the same identity are versions (or +/// release variants) of one package. +pub fn package_identity(purl: &str) -> Option { + let canonical = canonical_purl(purl).to_lowercase(); + let rest = canonical.strip_prefix("pkg:")?; + let (ty, coord) = rest.split_once('/')?; + let name = match coord.rfind('@').filter(|&i| i > 0) { + Some(at) => &coord[..at], + None => coord, + }; + if name.is_empty() { + return None; + } + let name = if ty == "pypi" { + canonicalize_pypi_name(name) + } else { + name.to_string() + }; + Some(format!("pkg:{ty}/{name}")) +} + +/// `v2`, `v3`, …: a Go module path's major-version suffix. +fn is_go_major_suffix(token: &str) -> bool { + token + .strip_prefix(['v', 'V']) + .is_some_and(|n| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit())) +} + +fn is_golang_purl(purl: &str) -> bool { + purl.get(..11) + .is_some_and(|p| p.eq_ignore_ascii_case("pkg:golang/")) +} + /// The standard `8-4-4-4-12` hex UUID grouping, any case. The one /// user-input UUID shape (the stricter lowercase-only on-disk grammar is /// `patch::path_safety::is_canonical_uuid`). @@ -360,6 +442,70 @@ mod tests { assert!(!m("pkg:npm/a@1", UUID, "pkg:npm/b@1")); } + /// A name that reaches several packages by last segment is ambiguous; + /// several versions of one package are not. + #[test] + fn ambiguity_counts_distinct_packages() { + let core = Target::parse("core"); + let purls = [ + "pkg:npm/%40angular/core@17.0.0", + "pkg:npm/@babel/core@7.0.0", + "pkg:npm/@babel/core@7.1.0", + "pkg:npm/lodash@4.17.21", + ]; + let msg = core.ambiguity(purls).expect("ambiguous"); + assert!( + msg.contains("pkg:npm/@angular/core, pkg:npm/@babel/core"), + "{msg}" + ); + assert!(!msg.contains("lodash"), "{msg}"); + // One package, many versions (and variants): not ambiguous. + assert_eq!(core.ambiguity(purls[1..].iter().copied()), None); + assert_eq!( + Target::parse("six") + .ambiguity(["pkg:pypi/six@1.16.0?artifact_id=a", "pkg:pypi/Six@1.17.0",]), + None + ); + // The full name, a purl and a uuid are never ambiguous. + assert_eq!(Target::parse("@babel/core").ambiguity(purls), None); + assert_eq!(Target::parse("pkg:npm/core").ambiguity(purls), None); + assert_eq!(Target::parse(UUID).ambiguity(purls), None); + // Same name in two ecosystems: ambiguous. + assert!(Target::parse("six") + .ambiguity(["pkg:pypi/six@1", "pkg:npm/six@1"]) + .is_some()); + } + + #[test] + fn go_major_suffix_is_never_a_name() { + let v2 = "pkg:golang/github.com/x/y/v2@v2.0.0"; + for token in ["v2", "V2"] { + let t = Target::parse(token); + assert!(!t.matches_package(v2), "{token}"); + assert!(!t.matches_patch(v2, "u"), "{token}"); + } + assert!(Target::parse("github.com/x/y/v2").matches_package(v2)); + // Outside Go, `v2` is an ordinary name. + assert!(Target::parse("v2").matches_package("pkg:npm/v2@1.0.0")); + } + + #[test] + fn package_identity_drops_version_and_qualifiers() { + assert_eq!( + package_identity("pkg:npm/%40Babel/core@7.0.0").as_deref(), + Some("pkg:npm/@babel/core") + ); + assert_eq!( + package_identity("pkg:pypi/Typing_Extensions@4?artifact_id=x").as_deref(), + Some("pkg:pypi/typing-extensions") + ); + assert_eq!( + package_identity("pkg:golang/github.com/x/y/v2@v2.0.0").as_deref(), + Some("pkg:golang/github.com/x/y/v2") + ); + assert_eq!(package_identity("lodash"), None); + } + #[test] fn path_shape() { for p in [ diff --git a/docs/usage.md b/docs/usage.md index b0f4d92db..1949e2edb 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -34,7 +34,9 @@ socket-patch get pkg:npm/lodash@4.17.20 --mode agent Replace the example identifiers with the advisory or package you need. `get` also accepts a patch UUID or an exact package name, which covers every installed version -of that package. `remove` and `rollback` take the same names. `get` defaults to hosted mode; `--save-only` and +of that package. `remove` and `rollback` take the same names. A short name that +names several packages (`core` for `@angular/core` and `@babel/core`) is refused: +use the full name or a purl. `get` defaults to hosted mode; `--save-only` and global targeting default to agent mode instead. Hosted and vendored `get` do not prompt. Agent-mode searches can offer an interactive choice. From 367d8026369e25714bf39fe13c774d3297b88a0d Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 12:27:06 -0400 Subject: [PATCH 06/10] Drop formatting-only churn and stale names from tests Restore e2e_socket_yml_policy.rs to main's layout and keep only the new get-by-uuid policy test, and update test comments that still named the deleted crawl_all_ecosystems and IdentifierType::Package. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../tests/e2e_socket_yml_policy.rs | 471 ++++-------------- .../tests/get/get_edge_cases_e2e.rs | 4 +- .../socket-patch-cli/tests/in_process_get.rs | 4 +- 3 files changed, 94 insertions(+), 385 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_socket_yml_policy.rs b/crates/socket-patch-cli/tests/e2e_socket_yml_policy.rs index 841dc7131..b9a07f636 100644 --- a/crates/socket-patch-cli/tests/e2e_socket_yml_policy.rs +++ b/crates/socket-patch-cli/tests/e2e_socket_yml_policy.rs @@ -61,11 +61,7 @@ impl Patch { "low" => 3, _ => 4, }; - self.severities - .iter() - .copied() - .min_by_key(|s| rank(s)) - .unwrap_or("unknown") + self.severities.iter().copied().min_by_key(|s| rank(s)).unwrap_or("unknown") } } @@ -204,9 +200,7 @@ async fn mount_api(server: &MockServer, patches: Vec) { .await; let detail_map = by_purl.clone(); Mock::given(method("GET")) - .and(path_regex(format!( - "^/v0/orgs/{ORG}/patches/by-package/.+$" - ))) + .and(path_regex(format!("^/v0/orgs/{ORG}/patches/by-package/.+$"))) .respond_with(move |req: &Request| { let raw = req.url.path().rsplit('/').next().unwrap(); let purl = percent_decode(raw); @@ -222,15 +216,11 @@ async fn mount_api(server: &MockServer, patches: Vec) { }) }) .collect(); - ResponseTemplate::new(200) - .set_body_json(json!({"patches": list, "canAccessPaidPatches": false})) + ResponseTemplate::new(200).set_body_json(json!({"patches": list, "canAccessPaidPatches": false})) }) .mount(server) .await; - let by_uuid: BTreeMap = patches - .iter() - .map(|p| (p.uuid.to_string(), p.clone())) - .collect(); + let by_uuid: BTreeMap = patches.iter().map(|p| (p.uuid.to_string(), p.clone())).collect(); let refs = by_uuid.clone(); Mock::given(method("POST")) .and(path(format!("/v0/orgs/{ORG}/patches/package"))) @@ -287,10 +277,8 @@ fn write_npm_root(dir: &Path, deps: &[&str]) { let dep_map: BTreeMap<&str, &str> = deps.iter().map(|d| (*d, "1.0.0")).collect(); std::fs::write( dir.join("package.json"), - serde_json::to_string_pretty( - &json!({"name": "consumer", "version": "0.0.0", "dependencies": dep_map}), - ) - .unwrap(), + serde_json::to_string_pretty(&json!({"name": "consumer", "version": "0.0.0", "dependencies": dep_map})) + .unwrap(), ) .unwrap(); let mut packages = serde_json::Map::new(); @@ -301,11 +289,7 @@ fn write_npm_root(dir: &Path, deps: &[&str]) { for name in deps { let pkg = dir.join("node_modules").join(name); std::fs::create_dir_all(&pkg).unwrap(); - std::fs::write( - pkg.join("package.json"), - format!(r#"{{ "name": "{name}", "version": "1.0.0" }}"#), - ) - .unwrap(); + std::fs::write(pkg.join("package.json"), format!(r#"{{ "name": "{name}", "version": "1.0.0" }}"#)).unwrap(); std::fs::write(pkg.join("index.js"), orig_index(name)).unwrap(); packages.insert( format!("node_modules/{name}"), @@ -320,20 +304,12 @@ fn write_npm_root(dir: &Path, deps: &[&str]) { "name": "consumer", "version": "0.0.0", "lockfileVersion": 3, "requires": true, "packages": packages }); - std::fs::write( - dir.join("package-lock.json"), - serde_json::to_string_pretty(&lock).unwrap() + "\n", - ) - .unwrap(); + std::fs::write(dir.join("package-lock.json"), serde_json::to_string_pretty(&lock).unwrap() + "\n").unwrap(); } fn write_gem(dir: &Path, name: &str, version: &str) { - std::fs::create_dir_all( - dir.join("vendor/bundle/ruby/3.0.0/gems") - .join(format!("{name}-{version}")) - .join("lib"), - ) - .unwrap(); + std::fs::create_dir_all(dir.join("vendor/bundle/ruby/3.0.0/gems").join(format!("{name}-{version}")).join("lib")) + .unwrap(); } /// The monorepo: `services/web` (alpha, beta, left-pad + a gem), @@ -373,11 +349,7 @@ impl Repo { if entry.file_type().unwrap().is_dir() { walk(&path, root, out); } else { - let rel = path - .strip_prefix(root) - .unwrap() - .to_string_lossy() - .into_owned(); + let rel = path.strip_prefix(root).unwrap().to_string_lossy().into_owned(); out.insert(rel, std::fs::read(&path).unwrap()); } } @@ -397,8 +369,7 @@ fn run_cli(cwd: &Path, args: &[&str], env: &[(&str, &str)]) -> (i32, String, Str cmd.env_remove(key); } } - cmd.env_remove("GIT_CEILING_DIRECTORIES") - .env_remove("VIRTUAL_ENV"); + cmd.env_remove("GIT_CEILING_DIRECTORIES").env_remove("VIRTUAL_ENV"); cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); // The fixture's hosted pins name this origin; it makes them recorded. cmd.env("SOCKET_PATCH_SERVER_URL", "http://patch.test"); @@ -412,8 +383,7 @@ fn run_cli(cwd: &Path, args: &[&str], env: &[(&str, &str)]) -> (i32, String, Str ] { cmd.env(var, &absent); } - cmd.env("NPM_CONFIG_ALLOW_REMOTE", "") - .env("npm_config_allow_remote", ""); + cmd.env("NPM_CONFIG_ALLOW_REMOTE", "").env("npm_config_allow_remote", ""); for (k, v) in env { cmd.env(k, v); } @@ -448,9 +418,8 @@ fn scan_json(cwd: &Path, api: &str, extra: &[&str], env: &[(&str, &str)]) -> (i3 let mut args = vec!["--json"]; args.extend_from_slice(extra); let (code, stdout, stderr) = scan(cwd, api, &args, env); - let doc: Value = serde_json::from_str(&stdout).unwrap_or_else(|e| { - panic!("stdout must be JSON ({e})\nstdout=\n{stdout}\nstderr=\n{stderr}") - }); + let doc: Value = serde_json::from_str(&stdout) + .unwrap_or_else(|e| panic!("stdout must be JSON ({e})\nstdout=\n{stdout}\nstderr=\n{stderr}")); (code, doc) } @@ -459,12 +428,7 @@ fn filtered(doc: &Value) -> Vec<(Option, String)> { .as_array() .unwrap() .iter() - .map(|f| { - ( - f["purl"].as_str().map(str::to_string), - f["reason"].as_str().unwrap().to_string(), - ) - }) + .map(|f| (f["purl"].as_str().map(str::to_string), f["reason"].as_str().unwrap().to_string())) .collect() } @@ -480,11 +444,7 @@ fn filtered_reason<'a>(doc: &'a Value, purl: &str) -> &'a Value { fn warning_codes(doc: &Value) -> Vec { doc["warnings"] .as_array() - .map(|w| { - w.iter() - .filter_map(|e| e["code"].as_str().map(str::to_string)) - .collect() - }) + .map(|w| w.iter().filter_map(|e| e["code"].as_str().map(str::to_string)).collect()) .unwrap_or_default() } @@ -504,28 +464,16 @@ async fn hosted_filters_by_ecosystem_package_and_severity() { assert_eq!(code, 0, "{doc:#}"); let lock = repo.lock("services/web"); - assert!( - lock.contains(&P_ALPHA.hosted_url()), - "alpha is patched:\n{lock}" - ); - assert!( - !lock.contains(P_BETA.uuid), - "beta is below the floor:\n{lock}" - ); - assert!( - !lock.contains(P_LEFTPAD.uuid), - "left-pad is ignored:\n{lock}" - ); + assert!(lock.contains(&P_ALPHA.hosted_url()), "alpha is patched:\n{lock}"); + assert!(!lock.contains(P_BETA.uuid), "beta is below the floor:\n{lock}"); + assert!(!lock.contains(P_LEFTPAD.uuid), "left-pad is ignored:\n{lock}"); let policy = &doc["policy"]; assert_eq!(policy["source"], "file"); assert_eq!(policy["path"], "socket.yml"); assert_eq!(policy["sha256"].as_str().unwrap().len(), 64); assert_eq!(policy["enabled"], true); - assert_eq!( - policy["minSeverity"], - json!({"value": "high", "source": "file"}) - ); + assert_eq!(policy["minSeverity"], json!({"value": "high", "source": "file"})); let beta = filtered_reason(&doc, "pkg:npm/beta@1.0.0"); assert_eq!(beta["reason"], "policy_severity"); assert_eq!(beta["detail"], "low < high"); @@ -533,15 +481,8 @@ async fn hosted_filters_by_ecosystem_package_and_severity() { assert_eq!(beta["project"], "services/web"); let left_pad = filtered_reason(&doc, "pkg:npm/left-pad@1.0.0"); assert_eq!(left_pad["reason"], "policy_package_ignored"); - assert_eq!( - left_pad["uuid"], - Value::Null, - "filtered before any patch lookup" - ); - assert_eq!( - left_pad["detail"], - "pkg:npm/left-pad (patches.ignorePackages)" - ); + assert_eq!(left_pad["uuid"], Value::Null, "filtered before any patch lookup"); + assert_eq!(left_pad["detail"], "pkg:npm/left-pad (patches.ignorePackages)"); let rack = filtered_reason(&doc, "pkg:gem/rack@1.0.0"); assert_eq!(rack["reason"], "policy_ecosystem"); assert_eq!(policy["counts"]["filtered"], 3); @@ -551,10 +492,7 @@ async fn hosted_filters_by_ecosystem_package_and_severity() { for r in &reqs { if r.url.path().ends_with("/patches/batch") { let body = String::from_utf8_lossy(&r.body); - assert!( - !body.contains("left-pad") && !body.contains("rack"), - "{body}" - ); + assert!(!body.contains("left-pad") && !body.contains("rack"), "{body}"); } } assert_eq!(doc["redirect"]["redirected"], 1, "{:#}", doc["redirect"]); @@ -565,27 +503,13 @@ async fn hosted_filters_by_ecosystem_package_and_severity() { async fn hosted_dry_run_makes_the_same_decisions_and_writes_nothing() { let server = MockServer::start().await; mount_api(&server, catalog()).await; - let repo = Repo::new(Some( - "version: 2\npatches:\n minSeverity: high\n ecosystems: [npm]\n", - )); + let repo = Repo::new(Some("version: 2\npatches:\n minSeverity: high\n ecosystems: [npm]\n")); let before = repo.snapshot(); - let (code, doc) = scan_json( - &repo.dir("services/web"), - &server.uri(), - &["--dry-run"], - &[], - ); + let (code, doc) = scan_json(&repo.dir("services/web"), &server.uri(), &["--dry-run"], &[]); assert_eq!(code, 0, "{doc:#}"); assert_eq!(repo.snapshot(), before, "a dry run changes no bytes"); - assert_eq!( - doc["redirect"]["redirected"], 2, - "alpha and left-pad: {:#}", - doc["redirect"] - ); - assert_eq!( - filtered_reason(&doc, "pkg:npm/beta@1.0.0")["reason"], - "policy_severity" - ); + assert_eq!(doc["redirect"]["redirected"], 2, "alpha and left-pad: {:#}", doc["redirect"]); + assert_eq!(filtered_reason(&doc, "pkg:npm/beta@1.0.0")["reason"], "policy_severity"); } #[tokio::test] @@ -593,24 +517,14 @@ async fn hosted_dry_run_makes_the_same_decisions_and_writes_nothing() { async fn path_globs_apply_default_ignores_and_ignore_paths_human() { let server = MockServer::start().await; mount_api(&server, catalog()).await; - let repo = Repo::new(Some( - "version: 2\npatches:\n ignorePaths: [\"/services/legacy/\"]\n", - )); + let repo = Repo::new(Some("version: 2\npatches:\n ignorePaths: [\"/services/legacy/\"]\n")); let legacy = repo.lock("services/legacy"); let test_lock = repo.lock("services/test"); let (code, stdout, stderr) = scan(&repo.root, &server.uri(), &["services/*"], &[]); assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}"); assert!(repo.lock("services/web").contains(&P_ALPHA.hosted_url())); - assert_eq!( - repo.lock("services/legacy"), - legacy, - "ignored by patches.ignorePaths" - ); - assert_eq!( - repo.lock("services/test"), - test_lock, - "a discovered test/ root is a built-in ignore" - ); + assert_eq!(repo.lock("services/legacy"), legacy, "ignored by patches.ignorePaths"); + assert_eq!(repo.lock("services/test"), test_lock, "a discovered test/ root is a built-in ignore"); assert!(stdout.contains("Policy (socket.yml)"), "{stdout}"); // Named literally, the test/ root is explicit: defaults do not apply. @@ -624,9 +538,7 @@ async fn path_globs_apply_default_ignores_and_ignore_paths_human() { async fn include_paths_limit_roots() { let server = MockServer::start().await; mount_api(&server, catalog()).await; - let repo = Repo::new(Some( - "version: 2\npatches:\n includePaths: [\"/services/legacy/\"]\n", - )); + let repo = Repo::new(Some("version: 2\npatches:\n includePaths: [\"/services/legacy/\"]\n")); let web = repo.lock("services/web"); let (code, doc) = scan_json(&repo.dir("services/web"), &server.uri(), &[], &[]); assert_eq!(code, 0, "{doc:#}"); @@ -659,10 +571,7 @@ async fn invalid_file_fails_closed_before_any_request_or_write() { assert!(message.contains("--no-socket-yml"), "{message}"); assert!(doc.get("policy").is_none()); assert_eq!(repo.snapshot(), before); - assert!( - server.received_requests().await.unwrap().is_empty(), - "no request before the policy loads" - ); + assert!(server.received_requests().await.unwrap().is_empty(), "no request before the policy loads"); // Human output names the code on stderr, same exit code. let (code, _, stderr) = scan(&repo.dir("services/web"), &server.uri(), &[], &[]); @@ -670,20 +579,10 @@ async fn invalid_file_fails_closed_before_any_request_or_write() { assert!(stderr.contains("socket_yml_invalid"), "{stderr}"); // --no-socket-yml (and its env var) skips the file. - let (code, doc) = scan_json( - &repo.dir("services/web"), - &server.uri(), - &["--no-socket-yml", "--dry-run"], - &[], - ); + let (code, doc) = scan_json(&repo.dir("services/web"), &server.uri(), &["--no-socket-yml", "--dry-run"], &[]); assert_eq!(code, 0, "{doc:#}"); assert_eq!(doc["policy"]["source"], "bypassed"); - let (code, doc) = scan_json( - &repo.dir("services/web"), - &server.uri(), - &["--dry-run"], - &[("SOCKET_NO_SOCKET_YML", "1")], - ); + let (code, doc) = scan_json(&repo.dir("services/web"), &server.uri(), &["--dry-run"], &[("SOCKET_NO_SOCKET_YML", "1")]); assert_eq!(code, 0, "{doc:#}"); assert_eq!(doc["policy"]["source"], "bypassed"); } @@ -694,11 +593,7 @@ async fn both_files_disagreeing_is_ambiguous() { let server = MockServer::start().await; mount_api(&server, catalog()).await; let repo = Repo::new(Some("version: 2\npatches:\n maxNewPatches: 1\n")); - std::fs::write( - repo.root.join("socket.yaml"), - "version: 2\npatches:\n maxNewPatches: 2\n", - ) - .unwrap(); + std::fs::write(repo.root.join("socket.yaml"), "version: 2\npatches:\n maxNewPatches: 2\n").unwrap(); let (code, doc) = scan_json(&repo.dir("services/web"), &server.uri(), &[], &[]); assert_eq!(code, 1); assert_eq!(doc["errorCode"], "socket_yml_ambiguous"); @@ -711,66 +606,26 @@ async fn severity_flag_and_env_override_the_file() { mount_api(&server, catalog()).await; let repo = Repo::new(Some("version: 2\npatches:\n minSeverity: high\n")); let web = repo.dir("services/web"); - let (code, doc) = scan_json( - &web, - &server.uri(), - &["--dry-run", "--min-severity", "none"], - &[], - ); + let (code, doc) = scan_json(&web, &server.uri(), &["--dry-run", "--min-severity", "none"], &[]); assert_eq!(code, 0, "{doc:#}"); - assert_eq!( - doc["policy"]["minSeverity"], - json!({"value": null, "source": "flag"}) - ); - assert_eq!( - doc["redirect"]["redirected"], 3, - "beta too once the floor is lifted" - ); + assert_eq!(doc["policy"]["minSeverity"], json!({"value": null, "source": "flag"})); + assert_eq!(doc["redirect"]["redirected"], 3, "beta too once the floor is lifted"); - let (code, doc) = scan_json( - &web, - &server.uri(), - &["--dry-run"], - &[("SOCKET_MIN_SEVERITY", "critical")], - ); + let (code, doc) = scan_json(&web, &server.uri(), &["--dry-run"], &[("SOCKET_MIN_SEVERITY", "critical")]); assert_eq!(code, 0, "{doc:#}"); - assert_eq!( - doc["policy"]["minSeverity"], - json!({"value": "critical", "source": "env"}) - ); + assert_eq!(doc["policy"]["minSeverity"], json!({"value": "critical", "source": "env"})); assert_eq!(doc["redirect"]["redirected"], 1); // The flag beats the env; an empty env value is unset. - let (_, doc) = scan_json( - &web, - &server.uri(), - &["--dry-run", "--min-severity", "moderate"], - &[("SOCKET_MIN_SEVERITY", "critical")], - ); - assert_eq!( - doc["policy"]["minSeverity"], - json!({"value": "medium", "source": "flag"}) - ); - let (_, doc) = scan_json( - &web, - &server.uri(), - &["--dry-run"], - &[("SOCKET_MIN_SEVERITY", "")], - ); - assert_eq!( - doc["policy"]["minSeverity"], - json!({"value": "high", "source": "file"}) - ); + let (_, doc) = scan_json(&web, &server.uri(), &["--dry-run", "--min-severity", "moderate"], &[("SOCKET_MIN_SEVERITY", "critical")]); + assert_eq!(doc["policy"]["minSeverity"], json!({"value": "medium", "source": "flag"})); + let (_, doc) = scan_json(&web, &server.uri(), &["--dry-run"], &[("SOCKET_MIN_SEVERITY", "")]); + assert_eq!(doc["policy"]["minSeverity"], json!({"value": "high", "source": "file"})); // Malformed values are usage errors. let (code, _, stderr) = scan(&web, &server.uri(), &["--min-severity", "severe"], &[]); assert_eq!(code, 2, "{stderr}"); - let (code, _, stderr) = scan( - &web, - &server.uri(), - &[], - &[("SOCKET_MIN_SEVERITY", "severe")], - ); + let (code, _, stderr) = scan(&web, &server.uri(), &[], &[("SOCKET_MIN_SEVERITY", "severe")]); assert_eq!(code, 2, "{stderr}"); assert!(stderr.contains("SOCKET_MIN_SEVERITY"), "{stderr}"); } @@ -788,20 +643,12 @@ async fn narrowing_after_a_hosted_patch_leaves_the_pin_byte_identical() { assert!(pinned.contains(&P_ALPHA.hosted_url())); // A newer merged patch appears, and the repo now ignores alpha. - std::fs::write( - repo.root.join("socket.yml"), - "version: 2\npatches:\n ignorePackages: [alpha]\n", - ) - .unwrap(); + std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n ignorePackages: [alpha]\n").unwrap(); server.reset().await; mount_api(&server, vec![P_ALPHA, P_ALPHA_MERGED_NEW]).await; let (code, doc) = scan_json(&web, &server.uri(), &[], &[]); assert_eq!(code, 0, "{doc:#}"); - assert_eq!( - repo.lock("services/web"), - pinned, - "retained: not upgraded, not removed" - ); + assert_eq!(repo.lock("services/web"), pinned, "retained: not upgraded, not removed"); let retained = &doc["policy"]["retained"][0]; assert_eq!(retained["purl"], "pkg:npm/alpha@1.0.0"); assert_eq!(retained["recordedUuid"], P_ALPHA.uuid); @@ -819,11 +666,7 @@ async fn narrowing_after_a_hosted_patch_leaves_the_pin_byte_identical() { let (code, doc) = scan_json(&web, &server.uri(), &[], &[]); assert_eq!(code, 0, "{doc:#}"); assert_eq!(repo.lock("services/web"), pinned, "{yml}"); - assert_eq!( - doc["policy"]["retained"][0]["purl"], "pkg:npm/alpha@1.0.0", - "{yml}: {:#}", - doc["policy"] - ); + assert_eq!(doc["policy"]["retained"][0]["purl"], "pkg:npm/alpha@1.0.0", "{yml}: {:#}", doc["policy"]); } } @@ -838,15 +681,9 @@ async fn enabled_false_reports_and_writes_nothing() { assert_eq!(code, 0, "{doc:#}"); assert_eq!(repo.snapshot(), before); assert_eq!(doc["policy"]["enabled"], false); - assert!( - warning_codes(&doc).contains(&"patches_disabled".to_string()), - "{doc:#}" - ); + assert!(warning_codes(&doc).contains(&"patches_disabled".to_string()), "{doc:#}"); let reasons: Vec = filtered(&doc).into_iter().map(|(_, r)| r).collect(); - assert!( - !reasons.is_empty() && reasons.iter().all(|r| r == "policy_disabled"), - "{reasons:?}" - ); + assert!(!reasons.is_empty() && reasons.iter().all(|r| r == "policy_disabled"), "{reasons:?}"); assert_eq!(doc["redirect"]["redirected"], 0); } @@ -855,9 +692,7 @@ async fn enabled_false_reports_and_writes_nothing() { async fn report_only_json_fails_when_every_detail_query_fails() { let server = MockServer::start().await; Mock::given(method("GET")) - .and(path_regex(format!( - "^/v0/orgs/{ORG}/patches/by-package/.+$" - ))) + .and(path_regex(format!("^/v0/orgs/{ORG}/patches/by-package/.+$"))) .respond_with(ResponseTemplate::new(500)) .with_priority(1) .mount(&server) @@ -870,10 +705,7 @@ async fn report_only_json_fails_when_every_detail_query_fails() { assert_eq!(code, 1, "{doc:#}"); assert_eq!(doc["status"], "error", "{doc:#}"); assert!( - doc["error"] - .as_str() - .unwrap_or_default() - .contains("patch-detail queries failed"), + doc["error"].as_str().unwrap_or_default().contains("patch-detail queries failed"), "{doc:#}" ); assert_eq!(repo.snapshot(), before); @@ -894,11 +726,7 @@ async fn recorded_merge_below_the_floor_is_kept_until_a_more_severe_patch_is_ava "the only available patch is pinned:\n{pinned}" ); - std::fs::write( - repo.root.join("socket.yml"), - "version: 2\npatches:\n minSeverity: high\n", - ) - .unwrap(); + std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n minSeverity: high\n").unwrap(); let (code, doc) = scan_json(&web, &server.uri(), &[], &[]); assert_eq!(code, 0, "{doc:#}"); assert_eq!( @@ -950,17 +778,11 @@ async fn floor_with_nothing_admitted_reports_the_withheld_patch() { let (code, stdout, stderr) = scan(&web, &server.uri(), &[], &[]); assert_eq!(code, 0, "{stdout}\n{stderr}"); assert_eq!(repo.lock("services/web"), lock); - assert!( - stdout.contains("Policy (socket.yml): 1 skipped by filters"), - "{stdout}" - ); + assert!(stdout.contains("Policy (socket.yml): 1 skipped by filters"), "{stdout}"); // Only critical/high are named without --verbose. assert!(!stdout.contains("skipped beta"), "{stdout}"); let (_, stdout, _) = scan(&web, &server.uri(), &["--verbose"], &[]); - assert!( - stdout.contains("skipped pkg:npm/beta@1.0.0 (low): low < critical"), - "{stdout}" - ); + assert!(stdout.contains("skipped pkg:npm/beta@1.0.0 (low): low < critical"), "{stdout}"); } #[tokio::test] @@ -971,17 +793,9 @@ async fn path_outside_the_repo_is_a_usage_error() { let repo = Repo::new(None); let outside = repo.root.parent().unwrap().join("elsewhere"); write_npm_root(&outside, &["alpha"]); - let (code, _, stderr) = scan( - &repo.dir("services"), - &server.uri(), - &["web", "../../elsewhere"], - &[], - ); + let (code, _, stderr) = scan(&repo.dir("services"), &server.uri(), &["web", "../../elsewhere"], &[]); assert_eq!(code, 2, "{stderr}"); - assert!( - stderr.contains("is outside") && stderr.contains("run one scan per repository"), - "{stderr}" - ); + assert!(stderr.contains("is outside") && stderr.contains("run one scan per repository"), "{stderr}"); } #[tokio::test] @@ -989,9 +803,7 @@ async fn path_outside_the_repo_is_a_usage_error() { async fn project_ignore_paths_is_honored_without_a_patches_block() { let server = MockServer::start().await; mount_api(&server, catalog()).await; - let repo = Repo::new(Some( - "version: 2\nprojectIgnorePaths:\n - \"services/legacy/**\"\n", - )); + let repo = Repo::new(Some("version: 2\nprojectIgnorePaths:\n - \"services/legacy/**\"\n")); let legacy = repo.lock("services/legacy"); let (code, doc) = scan_json(&repo.dir("services/legacy"), &server.uri(), &[], &[]); assert_eq!(code, 0, "{doc:#}"); @@ -1001,22 +813,10 @@ async fn project_ignore_paths_is_honored_without_a_patches_block() { assert_eq!(entry["detail"], "services/legacy/** (projectIgnorePaths)"); // A malformed projectIgnorePaths without a patches block only warns. - std::fs::write( - repo.root.join("socket.yml"), - "version: 2\nprojectIgnorePaths: {a: 1}\n", - ) - .unwrap(); - let (code, doc) = scan_json( - &repo.dir("services/legacy"), - &server.uri(), - &["--dry-run"], - &[], - ); + std::fs::write(repo.root.join("socket.yml"), "version: 2\nprojectIgnorePaths: {a: 1}\n").unwrap(); + let (code, doc) = scan_json(&repo.dir("services/legacy"), &server.uri(), &["--dry-run"], &[]); assert_eq!(code, 0, "{doc:#}"); - assert!( - warning_codes(&doc).contains(&"socket_yml_ignored_value".to_string()), - "{doc:#}" - ); + assert!(warning_codes(&doc).contains(&"socket_yml_ignored_value".to_string()), "{doc:#}"); } // --------------------------------------------------------------------------- @@ -1045,18 +845,14 @@ async fn agent_mode_applies_only_admitted_patches() { let (code, doc) = scan_json(&web, &server.uri(), &["--mode", "agent"], &[]); assert_eq!(code, 0, "{doc:#}"); let manifest: Value = - serde_json::from_str(&std::fs::read_to_string(web.join(".socket/manifest.json")).unwrap()) - .unwrap(); + serde_json::from_str(&std::fs::read_to_string(web.join(".socket/manifest.json")).unwrap()).unwrap(); let keys: Vec<&String> = manifest["patches"].as_object().unwrap().keys().collect(); assert_eq!(keys, ["pkg:npm/alpha@1.0.0"]); assert_eq!( std::fs::read_to_string(web.join("node_modules/alpha/index.js")).unwrap(), patched_index("alpha") ); - assert_eq!( - std::fs::read_to_string(web.join("node_modules/beta/index.js")).unwrap(), - orig_index("beta") - ); + assert_eq!(std::fs::read_to_string(web.join("node_modules/beta/index.js")).unwrap(), orig_index("beta")); } #[tokio::test] @@ -1071,23 +867,13 @@ async fn agent_mode_retains_a_recorded_patch_the_policy_now_excludes() { let manifest_before = std::fs::read(web.join(".socket/manifest.json")).unwrap(); let installed_before = std::fs::read(web.join("node_modules/alpha/index.js")).unwrap(); - std::fs::write( - repo.root.join("socket.yml"), - "version: 2\npatches:\n ecosystems: [pypi]\n", - ) - .unwrap(); + std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n ecosystems: [pypi]\n").unwrap(); server.reset().await; mount_api(&server, vec![P_ALPHA, P_ALPHA_MERGED_NEW]).await; let (code, doc) = scan_json(&web, &server.uri(), &["--mode", "agent"], &[]); assert_eq!(code, 0, "{doc:#}"); - assert_eq!( - std::fs::read(web.join(".socket/manifest.json")).unwrap(), - manifest_before - ); - assert_eq!( - std::fs::read(web.join("node_modules/alpha/index.js")).unwrap(), - installed_before - ); + assert_eq!(std::fs::read(web.join(".socket/manifest.json")).unwrap(), manifest_before); + assert_eq!(std::fs::read(web.join("node_modules/alpha/index.js")).unwrap(), installed_before); assert_eq!(doc["policy"]["retained"][0]["reason"], "policy_ecosystem"); assert_eq!(doc["policy"]["retained"][0]["upgradeAvailable"], true); } @@ -1103,12 +889,7 @@ async fn vendored_dry_run_previews_only_admitted_patches() { mount_api(&server, catalog()).await; let repo = Repo::new(Some("version: 2\npatches:\n packages: [\"pkg:npm/beta\", \"pkg:npm/left-pad\"]\n minSeverity: medium\n")); let before = repo.snapshot(); - let (code, doc) = scan_json( - &repo.dir("services/web"), - &server.uri(), - &["--mode", "vendored", "--dry-run"], - &[], - ); + let (code, doc) = scan_json(&repo.dir("services/web"), &server.uri(), &["--mode", "vendored", "--dry-run"], &[]); assert_eq!(code, 0, "{doc:#}"); assert_eq!(repo.snapshot(), before); let previewed: Vec<&str> = doc["vendor"]["patches"] @@ -1118,14 +899,8 @@ async fn vendored_dry_run_previews_only_admitted_patches() { .filter_map(|p| p["purl"].as_str()) .collect(); assert_eq!(previewed, ["pkg:npm/left-pad@1.0.0"], "{doc:#}"); - assert_eq!( - filtered_reason(&doc, "pkg:npm/alpha@1.0.0")["reason"], - "policy_package_not_listed" - ); - assert_eq!( - filtered_reason(&doc, "pkg:npm/beta@1.0.0")["reason"], - "policy_severity" - ); + assert_eq!(filtered_reason(&doc, "pkg:npm/alpha@1.0.0")["reason"], "policy_package_not_listed"); + assert_eq!(filtered_reason(&doc, "pkg:npm/beta@1.0.0")["reason"], "policy_severity"); } // --------------------------------------------------------------------------- @@ -1157,16 +932,9 @@ async fn get_bypasses_the_policy_with_a_warning() { let (code, stdout, stderr) = run_cli(&web, &args, &[]); assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}"); let doc: Value = serde_json::from_str(&stdout).unwrap(); - let warnings: Vec<&str> = doc["warnings"] - .as_array() - .unwrap() - .iter() - .filter_map(Value::as_str) - .collect(); + let warnings: Vec<&str> = doc["warnings"].as_array().unwrap().iter().filter_map(Value::as_str).collect(); assert!( - warnings - .iter() - .any(|w| w.starts_with("(policy_bypassed)") && w.contains("alpha")), + warnings.iter().any(|w| w.starts_with("(policy_bypassed)") && w.contains("alpha")), "{doc:#}" ); @@ -1229,65 +997,30 @@ async fn agent_mode_honors_path_filters_and_keeps_the_prune_universe() { // The root is excluded by path: nothing selected, and a --sync (agent // + prune) still judges the full crawl, so no entry is pruned. - std::fs::write( - repo.root.join("socket.yml"), - "version: 2\npatches:\n includePaths: [\"/services/legacy/\"]\n", - ) - .unwrap(); + std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n includePaths: [\"/services/legacy/\"]\n").unwrap(); let (code, doc) = scan_json(&web, &server.uri(), &["--sync"], &[]); assert_eq!(code, 0, "{doc:#}"); - assert_eq!( - std::fs::read(web.join(".socket/manifest.json")).unwrap(), - manifest_before - ); + assert_eq!(std::fs::read(web.join(".socket/manifest.json")).unwrap(), manifest_before); assert_eq!(doc["policy"]["filtered"][0]["purl"], Value::Null); - assert_eq!( - doc["policy"]["filtered"][0]["reason"], - "policy_path_not_included" - ); - assert_eq!( - doc["policy"]["counts"]["retained"], 2, - "{:#}", - doc["policy"] - ); - assert_eq!( - doc["gc"]["removed"].as_array().map_or(0, Vec::len), - 0, - "{:#}", - doc["gc"] - ); + assert_eq!(doc["policy"]["filtered"][0]["reason"], "policy_path_not_included"); + assert_eq!(doc["policy"]["counts"]["retained"], 2, "{:#}", doc["policy"]); + assert_eq!(doc["gc"]["removed"].as_array().map_or(0, Vec::len), 0, "{:#}", doc["gc"]); // A narrower ecosystem list under --sync prunes nothing either. - std::fs::write( - repo.root.join("socket.yml"), - "version: 2\npatches:\n ecosystems: [pypi]\n", - ) - .unwrap(); + std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n ecosystems: [pypi]\n").unwrap(); let (code, doc) = scan_json(&web, &server.uri(), &["--sync"], &[]); assert_eq!(code, 0, "{doc:#}"); - assert_eq!( - std::fs::read(web.join(".socket/manifest.json")).unwrap(), - manifest_before - ); + assert_eq!(std::fs::read(web.join(".socket/manifest.json")).unwrap(), manifest_before); // patches.enabled: false skips the GC entirely. std::fs::remove_dir_all(web.join("node_modules/beta")).unwrap(); - let pkg_lock = repo - .lock("services/web") - .replace("\"node_modules/beta\"", "\"node_modules/gone\""); + let pkg_lock = repo.lock("services/web").replace("\"node_modules/beta\"", "\"node_modules/gone\""); std::fs::write(web.join("package-lock.json"), pkg_lock).unwrap(); - std::fs::write( - repo.root.join("socket.yml"), - "version: 2\npatches:\n enabled: false\n", - ) - .unwrap(); + std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n enabled: false\n").unwrap(); let (code, doc) = scan_json(&web, &server.uri(), &["--sync"], &[]); assert_eq!(code, 0, "{doc:#}"); assert!(doc.get("gc").is_none(), "{doc:#}"); - assert_eq!( - std::fs::read(web.join(".socket/manifest.json")).unwrap(), - manifest_before - ); + assert_eq!(std::fs::read(web.join(".socket/manifest.json")).unwrap(), manifest_before); } #[tokio::test] @@ -1301,53 +1034,29 @@ async fn narrowing_after_vendoring_leaves_the_vendored_package_byte_identical() let before = compute_git_sha256_from_bytes(orig_index("alpha").as_bytes()); let after = compute_git_sha256_from_bytes(patched_index("alpha").as_bytes()); std::fs::create_dir_all(web.join(".socket/blobs")).unwrap(); - std::fs::write( - web.join(".socket/blobs").join(&after), - patched_index("alpha"), - ) - .unwrap(); + std::fs::write(web.join(".socket/blobs").join(&after), patched_index("alpha")).unwrap(); let manifest = json!({"patches": {P_ALPHA.purl(): { "uuid": P_ALPHA.uuid, "exportedAt": "2026-01-01T00:00:00Z", "files": {"package/index.js": {"beforeHash": before, "afterHash": after}}, "vulnerabilities": {}, "description": "d", "license": "MIT", "tier": "free" }}}); - std::fs::write( - web.join(".socket/manifest.json"), - serde_json::to_vec_pretty(&manifest).unwrap(), - ) - .unwrap(); + std::fs::write(web.join(".socket/manifest.json"), serde_json::to_vec_pretty(&manifest).unwrap()).unwrap(); let fixture = prebuilt_common::Server::project(&web); let (code, stdout, stderr) = run_cli( &web, &["vendor", "--json", "--cwd", web.to_str().unwrap()], - &[ - ("SOCKET_VENDOR_URL", &fixture.uri), - ("SOCKET_PATCH_SERVER_URL", &fixture.uri), - ], + &[("SOCKET_VENDOR_URL", &fixture.uri), ("SOCKET_PATCH_SERVER_URL", &fixture.uri)], ); assert_eq!(code, 0, "vendor fixture: {stdout}\n{stderr}"); - assert!( - repo.lock("services/web").contains(".socket/vendor/"), - "vendored lock" - ); + assert!(repo.lock("services/web").contains(".socket/vendor/"), "vendored lock"); let snapshot = repo.snapshot(); - std::fs::write( - repo.root.join("socket.yml"), - "version: 2\npatches:\n ignorePackages: [\"pkg:npm/alpha\"]\n", - ) - .unwrap(); + std::fs::write(repo.root.join("socket.yml"), "version: 2\npatches:\n ignorePackages: [\"pkg:npm/alpha\"]\n").unwrap(); let (code, doc) = scan_json(&web, &server.uri(), &["--mode", "vendored"], &[]); assert_eq!(code, 0, "{doc:#}"); let mut after_scan = repo.snapshot(); after_scan.remove("socket.yml"); - assert_eq!( - after_scan, snapshot, - "the vendored package, its lock wiring and ledger stay byte-identical" - ); - assert_eq!( - doc["policy"]["retained"][0]["purl"], "pkg:npm/alpha@1.0.0", - "{:#}", - doc["policy"] - ); + assert_eq!(after_scan, snapshot, "the vendored package, its lock wiring and ledger stay byte-identical"); + assert_eq!(doc["policy"]["retained"][0]["purl"], "pkg:npm/alpha@1.0.0", "{:#}", doc["policy"]); } + diff --git a/crates/socket-patch-cli/tests/get/get_edge_cases_e2e.rs b/crates/socket-patch-cli/tests/get/get_edge_cases_e2e.rs index e32b4ea97..af833c01d 100644 --- a/crates/socket-patch-cli/tests/get/get_edge_cases_e2e.rs +++ b/crates/socket-patch-cli/tests/get/get_edge_cases_e2e.rs @@ -316,8 +316,8 @@ async fn get_by_package_with_single_paid_patch_emits_paid_required() { #[tokio::test] async fn get_with_invalid_search_purl_falls_through() { // A bare string that doesn't match UUID/CVE/GHSA/PURL is treated as a - // package-name search (IdentifierType::Package). That path first - // enumerates installed packages in the cwd; with an empty working dir + // package-name search (`TargetKind::Name`, the shared target grammar). + // That path first enumerates installed packages in the cwd; with an empty working dir // there are no packages to match, so the binary must short-circuit to // a `no_packages` envelope (exit 0) BEFORE it ever queries the API. // We mount the by-package mock to fail the test loudly if the binary diff --git a/crates/socket-patch-cli/tests/in_process_get.rs b/crates/socket-patch-cli/tests/in_process_get.rs index a659d1463..bbf12c029 100644 --- a/crates/socket-patch-cli/tests/in_process_get.rs +++ b/crates/socket-patch-cli/tests/in_process_get.rs @@ -441,7 +441,7 @@ async fn get_with_explicit_ghsa_flag() { } /// Write a minimal installed npm package under `/node_modules/` -/// so `crawl_all_ecosystems` discovers it as `pkg:npm/@`. +/// so `crawl_ecosystems` discovers it as `pkg:npm/@`. fn install_npm_fixture(cwd: &Path, name: &str, version: &str) { let pkg_dir = cwd.join("node_modules").join(name); std::fs::create_dir_all(&pkg_dir).unwrap(); @@ -455,7 +455,7 @@ fn install_npm_fixture(cwd: &Path, name: &str, version: &str) { #[tokio::test] #[serial] async fn get_with_explicit_package_no_install_short_circuits() { - // `--package` routes through `crawl_all_ecosystems` over the cwd. With + // `--package` routes through `crawl_ecosystems` over the cwd. With // NO installed packages the run short-circuits on `no_packages` and must // exit 0 WITHOUT ever contacting the API. We assert the full contract: // exit 0, no manifest, AND that the mounted mock saw zero requests — so a From 99033bdb67107c467026e635ab25cb03d8ccf9de Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 14:49:41 -0400 Subject: [PATCH 07/10] Fix two Bugbot findings in the target grammar - remove/rollback: the ambiguity refusal now counts a vendor-ledger entry under its decoded base purl when the target reaches it that way. A golang key is case-encoded (`!core`), so a last-segment `core` missed the key, skipped the refusal, and still selected the entry through base_purl. One purl per entry, so an encoded key and its base never count as two. - Bare-UUID shortcut: a UUID-shaped value of a value-taking flag (`--org `, `-o `, `--api-token `) is no longer taken as the shortcut operand. `socket-patch --org scan` used to parse as `get scan`; it now fails the same way a non-UUID org value does. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../socket-patch-cli/src/commands/remove.rs | 8 +- .../socket-patch-cli/src/commands/rollback.rs | 2 +- crates/socket-patch-cli/src/lib.rs | 105 +++++++++++++++++- crates/socket-patch-core/src/vendor/state.rs | 34 ++++++ 4 files changed, 142 insertions(+), 7 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/remove.rs b/crates/socket-patch-cli/src/commands/remove.rs index 8be3a6e18..20d6c62d0 100644 --- a/crates/socket-patch-cli/src/commands/remove.rs +++ b/crates/socket-patch-cli/src/commands/remove.rs @@ -466,7 +466,13 @@ pub async fn run(args: RemoveArgs) -> i32 { { let ledger_purls: Vec<&str> = vendor_state_result .as_ref() - .map(|state| state.entries.keys().map(String::as_str).collect()) + .map(|state| { + state + .entries + .iter() + .map(|(key, entry)| entry.ambiguity_purl(key, &target)) + .collect() + }) .unwrap_or_default(); let candidates = manifest .patches diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 65bae77a7..8f460ff91 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -1301,7 +1301,7 @@ pub async fn run(args: RollbackArgs) -> i32 { .manifest .iter() .map(String::as_str) - .chain(found.vendor.iter().map(|(k, _)| k.as_str())) + .chain(found.vendor.iter().map(|(k, e)| e.ambiguity_purl(k, id))) .chain(hosted_found), ); manifest_scope.extend(found.manifest); diff --git a/crates/socket-patch-cli/src/lib.rs b/crates/socket-patch-cli/src/lib.rs index ace45a1f9..6b6503c60 100644 --- a/crates/socket-patch-cli/src/lib.rs +++ b/crates/socket-patch-cli/src/lib.rs @@ -239,6 +239,66 @@ pub fn try_parse_cli(argv: &[String]) -> Result { Cli::from_arg_matches_mut(&mut matches).map_err(|e| e.format(&mut cli_command())) } +/// Is there a UUID-shaped operand among `args` (argv without argv[0]) +/// before the first subcommand name? A value-taking flag's value +/// (`--org `, `-o`) is not an operand, so a UUID-shaped org slug +/// or token never turns `socket-patch --org scan --help` into +/// `get`. After `--` every token is an operand. +fn first_operand_is_uuid(args: &[String], subcommands: &[String]) -> bool { + // The rewrite parses `args` as `get`'s, so `get`'s value-taking flags + // (the shared global options included) decide what is a flag value. + let cmd = cli_command(); + let get = cmd.find_subcommand("get").expect("get subcommand"); + let mut longs: Vec<&str> = Vec::new(); + let mut shorts: Vec = Vec::new(); + for arg in cmd.get_arguments().chain(get.get_arguments()) { + if !arg.get_action().takes_values() || arg.is_positional() { + continue; + } + longs.extend(arg.get_long()); + longs.extend(arg.get_all_aliases().unwrap_or_default()); + shorts.extend(arg.get_short()); + shorts.extend(arg.get_all_short_aliases().unwrap_or_default()); + } + let mut options_ended = false; + let mut i = 0; + while i < args.len() { + let a = args[i].as_str(); + i += 1; + if !options_ended { + if a == "--" { + options_ended = true; + continue; + } + if subcommands.iter().any(|s| s == a) { + return false; + } + if let Some(long) = a.strip_prefix("--") { + if !long.contains('=') && longs.contains(&long) { + i += 1; + } + continue; + } + if let Some(cluster) = a.strip_prefix('-').filter(|c| !c.is_empty()) { + // `-xo VALUE` or `-xoVALUE`: the first value-taking short + // consumes the rest of the cluster, or the next token + // when it ends the cluster. + let chars: Vec = cluster.chars().collect(); + if let Some(at) = chars.iter().position(|c| shorts.contains(c)) { + if at + 1 == chars.len() { + i += 1; + } + } + continue; + } + } + if is_uuid_shaped(a) { + return true; + } + } + false +} + /// Parse a full argv vector with two convenience rewrites on failure: /// `--update [...]` becomes the hidden `self-update` subcommand, and a /// bare `` becomes `get `. Returns the original clap error if @@ -309,11 +369,7 @@ pub fn parse_argv_with_shortcuts(argv: Vec) -> Result .flat_map(|c| std::iter::once(c.get_name()).chain(c.get_all_aliases())) .map(str::to_string) .collect(); - let uuid_operand = argv - .iter() - .skip(1) - .take_while(|a| !subcommands.contains(a)) - .any(|a| is_uuid_shaped(a)); + let uuid_operand = first_operand_is_uuid(&argv[1..], &subcommands); if uuid_operand { let mut new_args = vec![argv[0].clone(), "get".into()]; new_args.extend_from_slice(&argv[1..]); @@ -482,6 +538,45 @@ mod tests { assert!(parse_argv_with_shortcuts(argv(&["socket-patch", "list", UUID])).is_err()); } + #[test] + fn fallback_skips_a_uuid_shaped_flag_value() { + // A UUID-shaped flag value is the flag's value, not the shortcut + // operand: `--org scan` must fail exactly as `--org acme + // scan` does, never parse as `get scan` (which would run `get` + // with `scan` as its identifier). + for flags in [ + vec!["--org", UUID], + vec!["-o", UUID], + vec!["--api-token", UUID], + ] { + for tail in [vec!["scan"], vec!["scan", "--help"]] { + let mut args = vec!["socket-patch"]; + args.extend(flags.iter().copied()); + args.extend(tail.iter().copied()); + let err = match parse_argv_with_shortcuts(argv(&args)) { + Ok(cli) => panic!( + "{args:?} was rewritten to {:?}", + std::mem::discriminant(&cli.command) + ), + Err(e) => e, + }; + assert_eq!( + err.kind(), + clap::error::ErrorKind::UnknownArgument, + "{args:?}" + ); + } + } + assert!(!first_operand_is_uuid( + &argv(&["--org", UUID, "-o", UUID, "--org=x"]), + &[] + )); + assert!(first_operand_is_uuid(&argv(&["--org", "acme", UUID]), &[])); + assert!(first_operand_is_uuid(&argv(&["-oacme", UUID]), &[])); + assert!(first_operand_is_uuid(&argv(&["--org=acme", UUID]), &[])); + assert!(first_operand_is_uuid(&argv(&["--", UUID]), &[])); + } + #[test] fn fallback_returns_original_error_when_first_arg_is_not_uuid() { // No rewrite should happen; the original clap error must surface. diff --git a/crates/socket-patch-core/src/vendor/state.rs b/crates/socket-patch-core/src/vendor/state.rs index c8de05e1f..aa9372a99 100644 --- a/crates/socket-patch-core/src/vendor/state.rs +++ b/crates/socket-patch-core/src/vendor/state.rs @@ -311,6 +311,22 @@ impl VendorEntry { target.matches_patch(key, &self.uuid) || target.matches_patch(&self.base_purl, &self.uuid) } + /// The purl that names this entry's package for + /// [`Target::ambiguity`]: the decoded `base_purl` when `target` + /// reaches the entry through it, otherwise the ledger `key`. A golang + /// key is case-encoded (`!core`) and so can miss a last-segment name + /// that its `base_purl` (`Core`) matches; feeding the key alone would + /// skip the refusal while [`Self::matches_target`] still selects the + /// entry. One purl per entry, so an encoded key and its decoded base + /// never count as two packages. + pub fn ambiguity_purl<'a>(&'a self, key: &'a str, target: &Target) -> &'a str { + if target.matches_patch(&self.base_purl, &self.uuid) { + &self.base_purl + } else { + key + } + } + /// Does this entry, stored under ledger `key`, own the manifest purl /// `purl`? The ledger-key / qualifier-stripped-key / base-purl triple, /// plus composer release identity (`@3.0.2` owns `@3.0.2.0`) — the @@ -1110,6 +1126,24 @@ mod tests { )); assert!(!entry.matches_target(key, &Target::parse("00000000-0000-4000-8000-000000000000"))); + // A last-segment name reaching the entry only through its decoded + // base purl is counted under that purl, never under the encoded key. + let name = Target::parse("Toml"); + assert_eq!(entry.ambiguity_purl(key, &name), entry.base_purl); + let mut core = sample_entry(); + core.ecosystem = "golang".into(); + core.base_purl = "pkg:golang/github.com/x/Core@1.0.0".into(); + let core_key = "pkg:golang/github.com/x/!core@1.0.0"; + let other = "pkg:npm/core@1.0.0"; + let core_name = Target::parse("core"); + assert!(core.matches_target(core_key, &core_name)); + assert!(core_name + .ambiguity([core.ambiguity_purl(core_key, &core_name), other]) + .is_some()); + // One entry, encoded key plus decoded base: one package. + let sushi = Target::parse("toml"); + assert_eq!(sushi.ambiguity([entry.ambiguity_purl(key, &sushi)]), None); + // A qualified pypi key: the base identifier covers it, another // variant's qualifier does not. let mut entry = sample_entry(); From 8e62ca93eebe7d29940d7abf198df2cd459dc0de Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 02:24:29 +0000 Subject: [PATCH 08/10] Let a full-name match settle an ambiguous package name ambiguity() counted every package a name reaches, by full name or by last segment, so 'get lodash' in any project that also installs @types/lodash refused with 'use the full name or a purl' although the full name was typed. When some matches are by full name, only those are counted now; two last-segment-only matches (core -> @angular/core, @babel/core) and the same full name in two ecosystems stay ambiguous. Co-Authored-By: Claude --- crates/socket-patch-core/src/utils/target.rs | 39 ++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/crates/socket-patch-core/src/utils/target.rs b/crates/socket-patch-core/src/utils/target.rs index 8719aab4d..ae49fe276 100644 --- a/crates/socket-patch-core/src/utils/target.rs +++ b/crates/socket-patch-core/src/utils/target.rs @@ -163,6 +163,17 @@ impl Target { .collect(); packages.sort(); packages.dedup(); + // The full name typed is never ambiguous with packages it only + // reaches by last segment: `lodash` beside `@types/lodash` names + // `lodash`. + let full: Vec = packages + .iter() + .filter(|identity| self.is_full_name_of(identity)) + .cloned() + .collect(); + if !full.is_empty() { + packages = full; + } (packages.len() > 1).then(|| { format!( "\"{}\" is ambiguous: it names {}; use the full name or a purl", @@ -172,6 +183,22 @@ impl Target { }) } + /// Is this name the full name of the package `identity` (as + /// [`package_identity`] returns it), not just its last segment? + fn is_full_name_of(&self, identity: &str) -> bool { + let Some((ty, name)) = identity + .strip_prefix("pkg:") + .and_then(|rest| rest.split_once('/')) + else { + return false; + }; + let spec = self.text.trim().to_lowercase(); + if ty == "pypi" { + return name == canonicalize_pypi_name(&spec); + } + name == spec.replace(':', "/") + } + /// Does this target select the recorded patch `(purl, uuid)` — a /// manifest record, a vendor-ledger entry or a hosted pin? /// @@ -474,6 +501,18 @@ mod tests { assert!(Target::parse("six") .ambiguity(["pkg:pypi/six@1", "pkg:npm/six@1"]) .is_some()); + // A full-name match wins over last-segment ones (#1034 review): + // `lodash` beside `@types/lodash`, `core` beside `@x/core`. + let typed = ["pkg:npm/lodash@4.17.21", "pkg:npm/@types/lodash@4.17.0"]; + assert_eq!(Target::parse("lodash").ambiguity(typed), None); + assert_eq!( + core.ambiguity(["pkg:npm/core@1.0.0", "pkg:npm/@x/core@2.0.0"]), + None + ); + // ...but two last-segment-only matches stay ambiguous. + assert!(Target::parse("lodash") + .ambiguity(["pkg:npm/@types/lodash@4", "pkg:npm/@x/lodash@1"]) + .is_some()); } #[test] From 703694e889b923c0128f1b09f32cca8e4b1748ba Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 03:23:03 +0000 Subject: [PATCH 09/10] Keep the golang ambiguity test on last-segment-only matches 8e62ca9 lets a full-name match settle an ambiguous name, so `core` beside `pkg:npm/core` now names that package alone and the vendor-state test's ambiguity assertion failed (test windows-1, coverage). The test is about the golang entry being counted under its decoded base purl, so pair it with another last-segment-only match (`@x/core`) instead: the assertion still fails if the golang entry stops being counted. Co-Authored-By: Claude --- crates/socket-patch-core/src/vendor/state.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/vendor/state.rs b/crates/socket-patch-core/src/vendor/state.rs index a50f2948b..0a9b41c2d 100644 --- a/crates/socket-patch-core/src/vendor/state.rs +++ b/crates/socket-patch-core/src/vendor/state.rs @@ -1121,7 +1121,9 @@ mod tests { core.ecosystem = "golang".into(); core.base_purl = "pkg:golang/github.com/x/Core@1.0.0".into(); let core_key = "pkg:golang/github.com/x/!core@1.0.0"; - let other = "pkg:npm/core@1.0.0"; + // Another last-segment-only match: a full-name match such as + // `pkg:npm/core` would settle the name on its own. + let other = "pkg:npm/@x/core@1.0.0"; let core_name = Target::parse("core"); assert!(core.matches_target(core_key, &core_name)); assert!(core_name From ca475de47e4bdfd1e31aa9873691eae767d7b948 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 05:12:13 +0000 Subject: [PATCH 10/10] Act only on the package a name's ambiguity check settles on 8e62ca93 let a full-name match settle an ambiguous name (`lodash` beside `@types/lodash` names `lodash`), but only in the refusal. Selection still matched by last segment, so `remove lodash` and `rollback lodash` also removed and rolled back the `@types/lodash` patch, and `get lodash` searched and acted on every installed `@types/lodash` version. Target::settle now returns the target to act on: Err for an ambiguous name, otherwise the name narrowed to its full-name match when that match won. get, remove and rollback act on the settled target, so they never select more than the check allowed. ambiguity() stays as a wrapper. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/src/commands/get.rs | 18 +++++-- .../socket-patch-cli/src/commands/remove.rs | 46 ++++++++++-------- .../socket-patch-cli/src/commands/rollback.rs | 48 ++++++++++++------- crates/socket-patch-core/src/utils/target.rs | 45 +++++++++++++---- 4 files changed, 105 insertions(+), 52 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index 1a5408621..a1e3e3483 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -1363,11 +1363,19 @@ pub async fn run(args: GetArgs) -> i32 { // A name reaching several packages by last segment (`core` → // `@angular/core` and `@babel/core`) is refused: `get` acts on - // one package per name. - if let Some(msg) = target.ambiguity(matched.iter().map(String::as_str)) { - report_error(args.common.json, "ambiguous_target", &msg); - return 1; - } + // one package per name, and only on the one the check settled + // on (`lodash` beside `@types/lodash` is `lodash` alone). + let target = match target.settle(matched.iter().map(String::as_str)) { + Ok(settled) => settled, + Err(msg) => { + report_error(args.common.json, "ambiguous_target", &msg); + return 1; + } + }; + let matched: Vec = matched + .into_iter() + .filter(|purl| target.matches_package(purl)) + .collect(); if !quiet { eprintln!("{}", format_matched_packages(&matched)); } diff --git a/crates/socket-patch-cli/src/commands/remove.rs b/crates/socket-patch-cli/src/commands/remove.rs index 5587f6f46..68754af61 100644 --- a/crates/socket-patch-cli/src/commands/remove.rs +++ b/crates/socket-patch-cli/src/commands/remove.rs @@ -439,16 +439,6 @@ pub async fn run(args: RemoveArgs) -> i32 { } }; - // Find matching patches to show what will be removed (sorted: the - // manifest is a HashMap, and the listing must be deterministic). - let mut matching: Vec<_> = manifest - .patches - .iter() - .filter(|(purl, patch)| target.matches_patch(purl, &patch.uuid)) - .collect(); - matching.sort_by(|a, b| a.0.cmp(b.0)); - let matched_keys: Vec = matching.iter().map(|(purl, _)| (*purl).clone()).collect(); - // The vendor ledger, loaded ONCE under the lock: it scopes the nested // rollback (vendor-owned purls are not restored in place) and drives // the vendored leg. An unreadable ledger degrades to "nothing vendored" @@ -462,8 +452,9 @@ pub async fn run(args: RemoveArgs) -> i32 { // A name reaching several packages by last segment (`core` → // `@angular/core` and `@babel/core`) is refused across every store: - // `remove` acts on one package per name. - { + // `remove` acts on one package per name, and only on the one the + // check settled on (`lodash` beside `@types/lodash` is `lodash` alone). + let target = { let ledger_purls: Vec<&str> = vendor_state_result .as_ref() .map(|state| { @@ -480,16 +471,29 @@ pub async fn run(args: RemoveArgs) -> i32 { .map(String::as_str) .chain(ledger_purls) .chain(hosted_pins.iter().map(|pin| pin.purl.as_str())); - if let Some(msg) = target.ambiguity(candidates) { - emit_error_envelope( - args.common.json, - args.common.dry_run, - "ambiguous_target", - msg, - ); - return 1; + match target.settle(candidates) { + Ok(settled) => settled, + Err(msg) => { + emit_error_envelope( + args.common.json, + args.common.dry_run, + "ambiguous_target", + msg, + ); + return 1; + } } - } + }; + + // Find matching patches to show what will be removed (sorted: the + // manifest is a HashMap, and the listing must be deterministic). + let mut matching: Vec<_> = manifest + .patches + .iter() + .filter(|(purl, patch)| target.matches_patch(purl, &patch.uuid)) + .collect(); + matching.sort_by(|a, b| a.0.cmp(b.0)); + let matched_keys: Vec = matching.iter().map(|(purl, _)| (*purl).clone()).collect(); if matching.is_empty() { // Ledger-only entries (vendored mode keeps no manifest record) — diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index a7d5d0833..b96fdd274 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -1127,31 +1127,43 @@ pub async fn run(args: RollbackArgs) -> i32 { hosted_scope.extend(redirect_records.iter().map(|(p, _)| p.clone())); } for id in &identifiers { - let found = ledgers.matching(id); // Hosted pins live in the lockfiles, not in a store: matched by the // target, or as another generation of a matched manifest key. - let pins = - socket_patch_core::ledgers::hosted_pins_matching(&hosted_pins, id, &found.manifest); - let matched = !found.is_empty() || !pins.is_empty(); + let select = |id: &Target| { + let found = ledgers.matching(id); + let pins = + socket_patch_core::ledgers::hosted_pins_matching(&hosted_pins, id, &found.manifest); + (found, pins) + }; // A name reaching several packages by last segment (`core` → // `@angular/core` and `@babel/core`) is refused across every - // store: `rollback` acts on one package per name. - let ambiguity = id.ambiguity( - found - .manifest - .iter() - .map(String::as_str) - .chain(found.vendor.iter().map(|(k, e)| e.ambiguity_purl(k, id))) - .chain(pins.iter().map(|pin| pin.purl.as_str())), - ); + // store: `rollback` acts on one package per name, and only on the + // one the check settled on (`lodash` beside `@types/lodash` is + // `lodash` alone). + let settled = { + let (found, pins) = select(id); + id.settle( + found + .manifest + .iter() + .map(String::as_str) + .chain(found.vendor.iter().map(|(k, e)| e.ambiguity_purl(k, id))) + .chain(pins.iter().map(|pin| pin.purl.as_str())), + ) + }; + let id = match settled { + Ok(settled) => settled, + Err(msg) => { + track_patch_rollback_failed(&msg, api_token.as_deref(), org_slug.as_deref()).await; + emit_rollback_error(args.common.json, "ambiguous_target", &msg); + return 1; + } + }; + let (found, pins) = select(&id); + let matched = !found.is_empty() || !pins.is_empty(); manifest_scope.extend(found.manifest); vendor_scope.extend(found.vendor.into_iter().map(|(k, _)| k)); hosted_scope.extend(pins.into_iter().map(|pin| pin.purl)); - if let Some(msg) = ambiguity { - track_patch_rollback_failed(&msg, api_token.as_deref(), org_slug.as_deref()).await; - emit_rollback_error(args.common.json, "ambiguous_target", &msg); - return 1; - } if !matched { let hint = if matches!(id.kind(), TargetKind::Purl | TargetKind::Uuid) { String::new() diff --git a/crates/socket-patch-core/src/utils/target.rs b/crates/socket-patch-core/src/utils/target.rs index ae49fe276..8f017faf1 100644 --- a/crates/socket-patch-core/src/utils/target.rs +++ b/crates/socket-patch-core/src/utils/target.rs @@ -62,6 +62,9 @@ impl fmt::Display for TargetKind { pub struct Target { kind: TargetKind, text: String, + /// Set by [`Target::settle`] when a name's full-name match won over + /// last-segment ones: the name then selects only that full name. + full_name_only: bool, } impl fmt::Display for Target { @@ -94,6 +97,7 @@ impl Target { Self { kind, text: token.to_string(), + full_name_only: false, } } @@ -142,6 +146,8 @@ impl Target { return false; } package_spec_matches(&self.text, purl) + && (!self.full_name_only + || package_identity(purl).is_some_and(|identity| self.is_full_name_of(&identity))) } /// For a package name, the distinct packages it selects among `purls` @@ -149,12 +155,22 @@ impl Target { /// versionless purl) and how to pick one. `None` for every other kind, /// and for a name that selects one package (any number of its /// versions) or none. + pub fn ambiguity<'a>(&self, purls: impl IntoIterator) -> Option { + self.settle(purls).err() + } + + /// [`Self::ambiguity`] as the target to act on: `Err(message)` for an + /// ambiguous name, otherwise the target narrowed to what the check + /// settled on. A name whose full-name match won over last-segment + /// ones (`lodash` beside `@types/lodash`) selects only the full name + /// from then on; every other target comes back unchanged. /// /// `get`, `remove` and `rollback` refuse an ambiguous name instead of - /// acting on every package it reaches by last segment. - pub fn ambiguity<'a>(&self, purls: impl IntoIterator) -> Option { - if self.kind != TargetKind::Name { - return None; + /// acting on every package it reaches by last segment, and act on the + /// settled target so they never select more than the check allowed. + pub fn settle<'a>(&self, purls: impl IntoIterator) -> Result { + if self.kind != TargetKind::Name || self.full_name_only { + return Ok(self.clone()); } let mut packages: Vec = purls .into_iter() @@ -171,16 +187,19 @@ impl Target { .filter(|identity| self.is_full_name_of(identity)) .cloned() .collect(); + let mut settled = self.clone(); if !full.is_empty() { + settled.full_name_only = full.len() < packages.len(); packages = full; } - (packages.len() > 1).then(|| { - format!( + if packages.len() > 1 { + return Err(format!( "\"{}\" is ambiguous: it names {}; use the full name or a purl", self.text, packages.join(", ") - ) - }) + )); + } + Ok(settled) } /// Is this name the full name of the package `identity` (as @@ -505,6 +524,16 @@ mod tests { // `lodash` beside `@types/lodash`, `core` beside `@x/core`. let typed = ["pkg:npm/lodash@4.17.21", "pkg:npm/@types/lodash@4.17.0"]; assert_eq!(Target::parse("lodash").ambiguity(typed), None); + // ...and the settled target selects only the full name, so acting + // on it leaves `@types/lodash` alone. + let settled = Target::parse("lodash").settle(typed).unwrap(); + assert!(settled.matches_package(typed[0])); + assert!(settled.matches_patch(typed[0], "u1")); + assert!(!settled.matches_package(typed[1])); + assert!(!settled.matches_patch(typed[1], "u2")); + // Without a competing last-segment match nothing narrows. + let alone = Target::parse("lodash").settle([typed[1]]).unwrap(); + assert!(alone.matches_patch(typed[1], "u2")); assert_eq!( core.ambiguity(["pkg:npm/core@1.0.0", "pkg:npm/@x/core@2.0.0"]), None