From 6a701288779a816f2cc9e75d98c67d0699d73c4b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 06:57:22 +0000 Subject: [PATCH 1/3] Start sweep of core utils notice..purl Assisted-by: Claude Code:claude-opus-5-5 From 7791a0ebf378aba3197dc1b12ee22a2966ec720b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 07:02:14 +0000 Subject: [PATCH 2/3] Sweep core utils notice, npm_range, pep440, pipenv, purl Inline notice's is_muted and pipenv's resolve_on_path, name npm's operator set once, and share pep440's trailing-zero trim. Drop the JsrPurlParts alias and the is_purl wrapper, and make parse_name_version private. Delete pipenv's three PATH-resolution tests (process.rs tests the shared resolver) and purl's builder, round-trip and is_purl tests, which restate a format! or repeat the parse tests. Fold purl's three test modules into one, and drop history and banner comments. Behavior is unchanged. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/utils/notice.rs | 14 +- .../socket-patch-core/src/utils/npm_range.rs | 11 +- crates/socket-patch-core/src/utils/pep440.rs | 22 +- crates/socket-patch-core/src/utils/pipenv.rs | 110 +----- crates/socket-patch-core/src/utils/purl.rs | 361 ++++-------------- crates/socket-patch-core/src/utils/target.rs | 6 +- 6 files changed, 94 insertions(+), 430 deletions(-) diff --git a/crates/socket-patch-core/src/utils/notice.rs b/crates/socket-patch-core/src/utils/notice.rs index 40f7efac8..69cfc364e 100644 --- a/crates/socket-patch-core/src/utils/notice.rs +++ b/crates/socket-patch-core/src/utils/notice.rs @@ -37,19 +37,15 @@ pub fn is_quiet() -> bool { SILENT.load(Ordering::Relaxed) || JSON.load(Ordering::Relaxed) } -/// Whether an advisory of `level` is suppressed in the current mode. -pub(crate) fn is_muted(level: Notice) -> bool { - match level { - Notice::Info => is_quiet(), - Notice::Warning => SILENT.load(Ordering::Relaxed), - } -} - /// Print `msg()` to stderr unless `level` is muted or `shown` already /// fired. The message is built lazily so a suppressed advisory costs /// nothing, and a muted call does not consume the once. pub(crate) fn notice_once(level: Notice, shown: &AtomicBool, msg: impl FnOnce() -> String) { - if is_muted(level) { + let muted = match level { + Notice::Info => is_quiet(), + Notice::Warning => SILENT.load(Ordering::Relaxed), + }; + if muted { return; } if !shown.swap(true, Ordering::Relaxed) { diff --git a/crates/socket-patch-core/src/utils/npm_range.rs b/crates/socket-patch-core/src/utils/npm_range.rs index 049bbe7e6..65536e305 100644 --- a/crates/socket-patch-core/src/utils/npm_range.rs +++ b/crates/socket-patch-core/src/utils/npm_range.rs @@ -30,16 +30,17 @@ pub(crate) fn npm_range_admits(range: &str, version: &str) -> Option { Some(admits) } +fn is_operator(c: char) -> bool { + matches!(c, '^' | '~' | '>' | '<' | '=') +} + /// The comparators of one npm range alternative, an operator npm allows /// to stand apart (`>= 1.2.0`) joined to its version. fn glue_operators(alternative: &str) -> Vec { let mut out: Vec = Vec::new(); let mut pending = String::new(); for token in alternative.split_whitespace() { - if token - .chars() - .all(|c| matches!(c, '^' | '~' | '>' | '<' | '=')) - { + if token.chars().all(is_operator) { pending.push_str(token); } else { out.push(format!("{}{token}", std::mem::take(&mut pending))); @@ -53,7 +54,7 @@ fn glue_operators(alternative: &str) -> Vec { /// crate reads both as caret ranges. fn npm_comparator(comparator: &str) -> Option { let op_len = comparator - .find(|c: char| !matches!(c, '^' | '~' | '>' | '<' | '=')) + .find(|c: char| !is_operator(c)) .unwrap_or(comparator.len()); let (op, version) = comparator.split_at(op_len); let version = version.strip_prefix('v').unwrap_or(version); diff --git a/crates/socket-patch-core/src/utils/pep440.rs b/crates/socket-patch-core/src/utils/pep440.rs index 100e38a4f..118aa00d1 100644 --- a/crates/socket-patch-core/src/utils/pep440.rs +++ b/crates/socket-patch-core/src/utils/pep440.rs @@ -35,6 +35,13 @@ fn number(digits: &str) -> String { if trimmed.is_empty() { "0" } else { trimmed }.to_owned() } +/// Drop trailing `0` release segments, keeping at least one. +fn trim_trailing_zeros(release: &mut Vec) { + while release.len() > 1 && release.last().is_some_and(|segment| segment == "0") { + release.pop(); + } +} + #[derive(Debug, PartialEq, Eq)] enum LocalPart { Number(String), @@ -107,13 +114,8 @@ pub(crate) fn versions_equal(a: &str, b: &str) -> bool { match (parse(a), parse(b)) { (Some(mut a), Some(mut b)) => { // Equality ignores trailing `.0` release segments. - for version in [&mut a, &mut b] { - while version.release.len() > 1 - && version.release.last().is_some_and(|segment| segment == "0") - { - version.release.pop(); - } - } + trim_trailing_zeros(&mut a.release); + trim_trailing_zeros(&mut b.release); a == b } _ => false, @@ -192,9 +194,7 @@ pub(crate) fn equivalent_release_spellings(version: &str) -> Vec { } let segments: Vec = version.split('.').map(number).collect(); let mut trimmed = segments.clone(); - while trimmed.len() > 1 && trimmed.last().is_some_and(|segment| segment == "0") { - trimmed.pop(); - } + trim_trailing_zeros(&mut trimmed); let mut out = Vec::new(); for len in trimmed.len()..=segments.len().max(3) { let mut release = trimmed.clone(); @@ -286,7 +286,7 @@ mod tests { } /// What `str(packaging.version.Version(v))` prints, which is what - /// uv.lock, wheel METADATA and PyPI record (#1589). + /// uv.lock, wheel METADATA and PyPI record. #[test] fn normalize_matches_packaging() { for (raw, normalized) in [ diff --git a/crates/socket-patch-core/src/utils/pipenv.rs b/crates/socket-patch-core/src/utils/pipenv.rs index 0e7836827..3d01b6458 100644 --- a/crates/socket-patch-core/src/utils/pipenv.rs +++ b/crates/socket-patch-core/src/utils/pipenv.rs @@ -3,8 +3,7 @@ //! the vendored backend refuses installers older than 2018, so both ask //! [`installed_major`] once per command. -use std::ffi::OsString; -use std::path::{Path, PathBuf}; +use std::path::Path; /// Pins the answer without spawning anything: CI images without pipenv on /// PATH, or a project installed with a different release than the machine's @@ -39,17 +38,6 @@ fn parse_major(output: &str) -> Option { major.parse().ok() } -/// The `pipenv` executable, searched on ABSOLUTE `PATH` entries only: the -/// CLI runs inside the project being scanned, so a relative entry (`.`, an -/// empty component) would execute a `pipenv` planted in that repository. On Windows every `PATHEXT` extension is tried, -/// so `pipenv.exe` and the `pipenv.bat` / `pipenv.cmd` shims (pyenv-win, -/// hand-written wrappers) are both found. The rule lives in -/// [`crate::utils::process::resolve_tool_with`], shared with every other -/// tool the CLI spawns inside a project (`git`, `vlt`, `node`). -fn resolve_on_path(var: &impl Fn(&str) -> Option) -> Option { - crate::utils::process::resolve_tool_with("pipenv", var) -} - /// The major of the `pipenv` on PATH (`11`, `2018`, `2026`, …), or `None` /// when none is found, it does not answer within 10 s, exits non-zero or /// prints something unrecognizable. [`MAJOR_OVERRIDE_ENV`] short-circuits @@ -61,10 +49,8 @@ pub async fn installed_major(root: &Path) -> Option { { return Some(forced); } - let program = resolve_on_path(&|name| std::env::var_os(name))?; - // The RESOLVED path is spawned; a Windows `.bat` / `.cmd` shim is run by - // `std` itself through cmd.exe with correct quoting (see the shared - // launcher's docs). + // Absolute PATH entries only, so a repo-planted `pipenv` never runs. + let program = crate::utils::process::resolve_tool("pipenv")?; let mut command = crate::utils::process::command_for(&program); // The version banner does not depend on a project, so the probe runs in a // NEUTRAL directory: with the scanned repository as cwd, Pipenv would read @@ -121,68 +107,6 @@ mod tests { ); } - #[test] - fn resolve_on_path_skips_relative_entries_and_finds_absolute_ones() { - let tmp = tempfile::tempdir().unwrap(); - let bin = tmp.path().join("bin"); - std::fs::create_dir_all(&bin).unwrap(); - let leaf = if cfg!(windows) { - "pipenv.exe" - } else { - "pipenv" - }; - std::fs::write(bin.join(leaf), b"").unwrap(); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(bin.join(leaf), std::fs::Permissions::from_mode(0o755)) - .unwrap(); - } - // A repo-planted `pipenv` under a RELATIVE entry must never win. - let planted = tmp.path().join("planted"); - std::fs::create_dir_all(&planted).unwrap(); - std::fs::write(planted.join(leaf), b"").unwrap(); - let joined = std::env::join_paths([ - std::path::PathBuf::from("."), - std::path::PathBuf::from(""), - std::path::PathBuf::from("planted"), - bin.clone(), - ]) - .unwrap(); - let var = |name: &str| (name == "PATH").then(|| joined.clone()); - assert_eq!(resolve_on_path(&var), Some(bin.join(leaf))); - - let only_relative = std::env::join_paths([ - std::path::PathBuf::from("."), - std::path::PathBuf::from("planted"), - ]) - .unwrap(); - let var = |name: &str| (name == "PATH").then(|| only_relative.clone()); - assert_eq!(resolve_on_path(&var), None); - let none = |_: &str| None::; - assert_eq!(resolve_on_path(&none), None); - } - - #[cfg(unix)] - #[test] - fn resolve_on_path_skips_non_executable_files() { - use std::os::unix::fs::PermissionsExt; - let tmp = tempfile::tempdir().unwrap(); - let data = tmp.path().join("data"); - let bin = tmp.path().join("bin"); - std::fs::create_dir_all(&data).unwrap(); - std::fs::create_dir_all(&bin).unwrap(); - std::fs::write(data.join("pipenv"), b"not a program").unwrap(); - std::fs::set_permissions(data.join("pipenv"), std::fs::Permissions::from_mode(0o644)) - .unwrap(); - std::fs::write(bin.join("pipenv"), b"").unwrap(); - std::fs::set_permissions(bin.join("pipenv"), std::fs::Permissions::from_mode(0o755)) - .unwrap(); - let joined = std::env::join_paths([data.clone(), bin.clone()]).unwrap(); - let var = |name: &str| (name == "PATH").then(|| joined.clone()); - assert_eq!(resolve_on_path(&var), Some(bin.join("pipenv"))); - } - #[test] fn override_accepts_a_major_or_a_full_release() { assert_eq!(parse_override("11"), Some(11)); @@ -193,32 +117,4 @@ mod tests { assert_eq!(parse_override(""), None); assert_eq!(parse_override("-11"), None); } - - #[cfg(windows)] - #[test] - fn resolve_on_path_honours_pathext_shims() { - let tmp = tempfile::tempdir().unwrap(); - let bin = tmp.path().join("bin"); - std::fs::create_dir_all(&bin).unwrap(); - std::fs::write(bin.join("pipenv.bat"), b"@echo pipenv, version 2024.4.1").unwrap(); - let joined = std::env::join_paths([bin.clone()]).unwrap(); - let var = |name: &str| match name { - "PATH" => Some(joined.clone()), - "PATHEXT" => Some(OsString::from(".COM;.EXE;.BAT;.CMD")), - _ => None, - }; - let found = resolve_on_path(&var).unwrap(); - assert_eq!(found, bin.join("pipenv.bat")); - // Spawned directly: std runs the .bat through cmd.exe itself, and the - // banner parses like a real pipenv's. - let out = crate::utils::process::command_for(&found) - .arg("--version") - .output() - .expect("std spawns a .bat shim"); - assert!(out.status.success(), "{out:?}"); - assert_eq!( - parse_major(&String::from_utf8_lossy(&out.stdout)), - Some(2024) - ); - } } diff --git a/crates/socket-patch-core/src/utils/purl.rs b/crates/socket-patch-core/src/utils/purl.rs index 08edd82c3..18f80ef2e 100644 --- a/crates/socket-patch-core/src/utils/purl.rs +++ b/crates/socket-patch-core/src/utils/purl.rs @@ -7,18 +7,8 @@ use crate::patch::path_safety::{ use crate::utils::purl_key::PurlKey; /// Strip the trailing `?qualifiers` and `#subpath` components from a PURL, -/// leaving the canonical `pkg:type/namespace/name@version` base. -/// -/// The PURL grammar is `pkg:type/ns/name@version?qualifiers#subpath`, so a -/// subpath can appear *with or without* a preceding qualifier. Cutting only -/// at `?` would let a bare `#subpath` (no qualifier) leak into the base — -/// corrupting the version when the result is later split on `@`, and -/// breaking the grouping/matching keys callers build from it (two PURLs for -/// the same `name@version` differing only by subpath must collapse to one -/// base). So we cut at whichever of `?`/`#` comes first. -/// -/// e.g. `"pkg:pypi/requests@2.28.0?artifact_id=abc"` -> `"pkg:pypi/requests@2.28.0"` -/// and `"pkg:golang/github.com/foo/bar@v1.0.0#cmd/tool"` -> `"pkg:golang/github.com/foo/bar@v1.0.0"` +/// leaving the `pkg:type/namespace/name@version` base. A subpath can appear +/// without a qualifier, so this cuts at whichever of `?`/`#` comes first. pub fn strip_purl_qualifiers(purl: &str) -> &str { match purl.find(['?', '#']) { Some(idx) => &purl[..idx], @@ -174,7 +164,7 @@ pub fn purl_parts(purl: &str) -> Option<(String, String, String)> { /// the version off at the LAST `@` — so the name/path keeps any internal /// slashes and `@`s. /// -/// Private: every caller goes through [`parse_name_version`] / +/// Every caller goes through [`parse_name_version`] / /// [`parse_namespaced`], which percent-decode the split-out components. The /// split has to happen on the ENCODED text so an escaped separator (`%2f`, /// `%40`) can never introduce a new segment or a second `@` at parse time. @@ -203,10 +193,7 @@ fn split_name_version<'a>(purl: &'a str, prefix: &str) -> Option<(&'a str, &'a s /// /// Feed this the purl as it arrived: an ALREADY-decoded spelling (e.g. /// [`canonical_purl`] output) would be decoded a second time. -pub(crate) fn parse_name_version<'a>( - purl: &'a str, - prefix: &str, -) -> Option<(Cow<'a, str>, Cow<'a, str>)> { +fn parse_name_version<'a>(purl: &'a str, prefix: &str) -> Option<(Cow<'a, str>, Cow<'a, str>)> { let (name, version) = split_name_version(purl, prefix)?; Some(( percent_decode_purl_component(name), @@ -297,29 +284,14 @@ pub(crate) fn build_composer_purl(namespace: &str, name: &str, version: &str) -> /// (jsr `@scope/name`, composer `vendor/name`). pub(crate) type NamespacedPurlParts<'a> = ((Cow<'a, str>, Cow<'a, str>), Cow<'a, str>); -/// [`NamespacedPurlParts`] under the JSR crawler's own name. -pub(crate) type JsrPurlParts<'a> = NamespacedPurlParts<'a>; - -/// Parse a JSR PURL to extract scope, name, and version. +/// Parse a JSR (Deno registry) PURL, `pkg:jsr/@/@`, +/// to extract scope, name, and version. The scope keeps its leading `@`, +/// which the API serves encoded (`%40std`). /// -/// JSR (https://jsr.io) is Deno's package registry. Packages are -/// always scoped (`@scope/name`). PURL form: -/// `pkg:jsr//@` — e.g. -/// `"pkg:jsr/@std/path@0.220.0"` -> `Some((("@std", "path"), "0.220.0"))`. -/// -/// `pkg:jsr/` isn't a standardized purl-type upstream as of writing, -/// but the convention is informally adopted by some Deno tooling. -/// We follow the same shape as `parse_composer_purl` since both -/// have a `/` namespace structure. The leading `@` on -/// the scope is preserved (matching npm's `@scope/name` convention). -pub(crate) fn parse_jsr_purl(purl: &str) -> Option> { - // `parse_namespaced` decodes AFTER splitting on `/`/`@` and BEFORE the - // shape check below (and the caller's `is_safe_jsr_component` gate) — - // see `percent_decode_purl_component`. The API serves `%40scope`. +/// e.g. `"pkg:jsr/@std/path@0.220.0"` -> `Some((("@std", "path"), "0.220.0"))` +pub(crate) fn parse_jsr_purl(purl: &str) -> Option> { let ((scope, name), version) = parse_namespaced(purl, "pkg:jsr/")?; - - // Scope must be `@`. The bare `@` (length 1) is - // invalid — there's no actual scope after the marker. + // A bare `@` is not a scope. if !scope.starts_with('@') || scope.len() < 2 { return None; } @@ -361,11 +333,6 @@ pub(crate) fn build_cargo_purl(name: &str, version: &str) -> String { format!("pkg:cargo/{name}@{version}") } -/// Check if a string looks like a PURL. -pub fn is_purl(s: &str) -> bool { - s.starts_with("pkg:") -} - /// Does a manifest PURL key match a user-supplied PURL identifier? /// /// Release-variant ecosystems key patches in the manifest by their @@ -395,13 +362,9 @@ pub fn purl_matches_identifier(manifest_key: &str, identifier: &str) -> bool { } } -// ── 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: -// each validates the coordinates fail-closed — lockfiles are committed, -// tamper-able input whose names and versions later feed filesystem paths -// and download urls — and returns `None` rather than a purl it would have -// to trust. +// The validating builders below take lock-recorded coordinates, which are +// committed, tamper-able input that later feeds filesystem paths and +// download urls, so each returns `None` rather than a purl it can't trust. /// `pkg:npm/@` for lock-recorded coordinates, or `None` when /// the name fails the npm backends' own shape rule (at most one `/`, only @@ -452,58 +415,6 @@ pub fn maven_purl(group: &str, artifact: &str, version: &str) -> Option .then(|| build_maven_purl(group, artifact, version)) } -#[cfg(test)] -mod builder_tests { - use super::*; - - #[test] - fn purl_builders_validate_coordinates() { - assert_eq!( - npm_purl("left-pad", "1.3.0").as_deref(), - Some("pkg:npm/left-pad@1.3.0") - ); - assert_eq!( - npm_purl("@s/x", "1.0.0").as_deref(), - Some("pkg:npm/@s/x@1.0.0") - ); - assert_eq!(npm_purl("a/b", "1.0.0"), None, "unscoped slash"); - assert_eq!(npm_purl("@s/x/y", "1.0.0"), None, "extra level"); - assert_eq!(npm_purl("x", "../1"), None); - assert_eq!(npm_purl("..", "1.0.0"), None); - assert_eq!( - pypi_purl("Foo.Bar", "1.0").as_deref(), - Some("pkg:pypi/foo-bar@1.0") - ); - assert_eq!(pypi_purl("a/b", "1.0"), None); - assert_eq!( - simple_purl("cargo", "serde", "1.0.0").as_deref(), - Some("pkg:cargo/serde@1.0.0") - ); - assert_eq!( - simple_purl("npm", "x", "1"), - None, - "not a single-segment ecosystem" - ); - assert_eq!(simple_purl("gem", "a:b", "1"), None); - assert_eq!( - golang_purl("github.com/foo/bar", "v1.2.3").as_deref(), - Some("pkg:golang/github.com/foo/bar@v1.2.3") - ); - assert_eq!(golang_purl("github.com/../x", "v1"), None); - assert_eq!( - composer_purl("Monolog/Monolog", "2.0.0").as_deref(), - Some("pkg:composer/monolog/monolog@2.0.0") - ); - assert_eq!(composer_purl("monolog", "2.0.0"), None); - assert_eq!( - maven_purl("org.slf4j", "slf4j-api", "1.7.36").as_deref(), - Some("pkg:maven/org.slf4j/slf4j-api@1.7.36") - ); - assert_eq!(maven_purl("org..x", "a", "1"), None); - assert_eq!(maven_purl("", "a", "1"), None, "empty group"); - } -} - #[cfg(test)] mod tests { use super::*; @@ -594,14 +505,6 @@ mod tests { )); } - #[test] - fn test_is_purl() { - assert!(is_purl("pkg:npm/lodash@4.17.21")); - assert!(is_purl("pkg:pypi/requests@2.28.0")); - assert!(!is_purl("lodash")); - assert!(!is_purl("CVE-2024-1234")); - } - #[test] fn test_parse_cargo_purl() { assert_eq!( @@ -617,10 +520,8 @@ mod tests { assert_eq!(parse_cargo_purl("pkg:cargo/serde@"), None); } - /// REGRESSION: the API serves canonical (percent-encoded) purls, so a - /// semver build-metadata version arrives as `0.11.0%2Bwasi-…`. Compared - /// undecoded against Cargo.lock / the registry checkout it never - /// matched, and cargo vendoring of every such crate failed closed. + /// The API serves canonical (percent-encoded) purls, so a semver + /// build-metadata version arrives as `0.11.0%2Bwasi-…`. #[test] fn parsers_percent_decode_the_version() { assert_eq!( @@ -690,38 +591,6 @@ mod tests { assert_eq!(version, "1.0.0%zz"); } - /// Round trip through the canonical (encoded) spelling the API serves. - #[test] - fn cargo_purl_round_trips_through_the_encoded_spelling() { - let (name, version) = ("wasi", "0.11.0+wasi-snapshot-preview1"); - let encoded = build_cargo_purl(name, &version.replace('+', "%2B")); - assert_eq!( - nv(parse_cargo_purl(&encoded)), - Some((name.into(), version.into())) - ); - // The decoded spelling parses to the same pair (both are served). - assert_eq!( - nv(parse_cargo_purl(&build_cargo_purl(name, version))), - Some((name.into(), version.into())) - ); - } - - #[test] - fn test_build_cargo_purl() { - assert_eq!( - build_cargo_purl("serde", "1.0.200"), - "pkg:cargo/serde@1.0.200" - ); - } - - #[test] - fn test_cargo_purl_round_trip() { - let purl = build_cargo_purl("tokio", "1.38.0"); - let (name, version) = parse_cargo_purl(&purl).unwrap(); - assert_eq!(name, "tokio"); - assert_eq!(version, "1.38.0"); - } - #[test] fn test_parse_gem_purl() { assert_eq!( @@ -737,19 +606,6 @@ mod tests { assert_eq!(parse_gem_purl("pkg:gem/rails@"), None); } - #[test] - fn test_build_gem_purl() { - assert_eq!(build_gem_purl("rails", "7.1.0"), "pkg:gem/rails@7.1.0"); - } - - #[test] - fn test_gem_purl_round_trip() { - let purl = build_gem_purl("nokogiri", "1.16.5"); - let (name, version) = parse_gem_purl(&purl).unwrap(); - assert_eq!(name, "nokogiri"); - assert_eq!(version, "1.16.5"); - } - #[test] fn test_parse_maven_purl() { assert_eq!( @@ -784,23 +640,6 @@ mod tests { ); } - #[test] - fn test_build_maven_purl() { - assert_eq!( - build_maven_purl("org.apache.commons", "commons-lang3", "3.12.0"), - "pkg:maven/org.apache.commons/commons-lang3@3.12.0" - ); - } - - #[test] - fn test_maven_purl_round_trip() { - let purl = build_maven_purl("com.google.guava", "guava", "32.1.3-jre"); - let (group_id, artifact_id, version) = parse_maven_purl(&purl).unwrap(); - assert_eq!(group_id, "com.google.guava"); - assert_eq!(artifact_id, "guava"); - assert_eq!(version, "32.1.3-jre"); - } - #[test] fn test_parse_golang_purl() { assert_eq!( @@ -816,39 +655,12 @@ mod tests { assert_eq!(parse_golang_purl("pkg:npm/lodash@4.17.21"), None); assert_eq!(parse_golang_purl("pkg:golang/@v1.0.0"), None); assert_eq!(parse_golang_purl("pkg:golang/github.com/foo/bar@"), None); - } - - /// The API's canonical purls percent-encode `+` (`%2B`); every consumer - /// of the parsed coordinates (crawler lookup, go.mod replace, vendor - /// copy dir) needs the decoded `+incompatible` version Go uses. - #[test] - fn test_parse_golang_purl_percent_decodes() { - let (module, version) = - parse_golang_purl("pkg:golang/github.com/foo/bar@v2.0.0%2Bincompatible").unwrap(); - assert_eq!(module, "github.com/foo/bar"); - assert_eq!(version, "v2.0.0+incompatible"); assert_eq!( - parse_golang_purl("pkg:golang/github.com/Azure/x@v1.0.0"), + nv(parse_golang_purl("pkg:golang/github.com/Azure/x@v1.0.0")), Some(("github.com/Azure/x".into(), "v1.0.0".into())) ); } - #[test] - fn test_build_golang_purl() { - assert_eq!( - build_golang_purl("github.com/gin-gonic/gin", "v1.9.1"), - "pkg:golang/github.com/gin-gonic/gin@v1.9.1" - ); - } - - #[test] - fn test_golang_purl_round_trip() { - let purl = build_golang_purl("golang.org/x/text", "v0.14.0"); - let (module_path, version) = parse_golang_purl(&purl).unwrap(); - assert_eq!(module_path, "golang.org/x/text"); - assert_eq!(version, "v0.14.0"); - } - #[test] fn test_parse_composer_purl() { assert_eq!( @@ -865,14 +677,6 @@ mod tests { assert_eq!(parse_composer_purl("pkg:composer/monolog/monolog@"), None); } - #[test] - fn test_build_composer_purl() { - assert_eq!( - build_composer_purl("monolog", "monolog", "3.5.0"), - "pkg:composer/monolog/monolog@3.5.0" - ); - } - fn jsr_parts(purl: &str) -> Option<(String, String, String)> { parse_jsr_purl(purl).map(|((s, n), v)| (s.into_owned(), n.into_owned(), v.into_owned())) } @@ -891,38 +695,13 @@ mod tests { assert_eq!(jsr_parts("pkg:jsr/std/path@0.220.0"), None); // Empty pieces. assert_eq!(jsr_parts("pkg:jsr/@/path@0.220.0"), None); + assert_eq!(jsr_parts("pkg:jsr/%40/path@0.220.0"), None); assert_eq!(jsr_parts("pkg:jsr/@std/@0.220.0"), None); assert_eq!(jsr_parts("pkg:jsr/@std/path@"), None); // Wrong scheme. assert_eq!(jsr_parts("pkg:npm/@std/path@0.220.0"), None); } - #[test] - fn test_build_jsr_purl() { - assert_eq!( - build_jsr_purl("@std", "path", "0.220.0"), - "pkg:jsr/@std/path@0.220.0" - ); - } - - #[test] - fn test_jsr_purl_round_trip() { - let purl = build_jsr_purl("@std", "path", "0.220.0"); - let ((scope, name), version) = parse_jsr_purl(&purl).unwrap(); - assert_eq!(scope, "@std"); - assert_eq!(name, "path"); - assert_eq!(version, "0.220.0"); - } - - #[test] - fn test_composer_purl_round_trip() { - let purl = build_composer_purl("symfony", "console", "6.4.1"); - let ((namespace, name), version) = parse_composer_purl(&purl).unwrap(); - assert_eq!(namespace, "symfony"); - assert_eq!(name, "console"); - assert_eq!(version, "6.4.1"); - } - #[test] fn test_parse_nuget_purl() { assert_eq!( @@ -938,30 +717,6 @@ mod tests { assert_eq!(parse_nuget_purl("pkg:nuget/Newtonsoft.Json@"), None); } - #[test] - fn test_build_nuget_purl() { - assert_eq!( - build_nuget_purl("Newtonsoft.Json", "13.0.3"), - "pkg:nuget/Newtonsoft.Json@13.0.3" - ); - } - - #[test] - fn test_nuget_purl_round_trip() { - let purl = build_nuget_purl("System.Text.Json", "8.0.0"); - let (name, version) = parse_nuget_purl(&purl).unwrap(); - assert_eq!(name, "System.Text.Json"); - assert_eq!(version, "8.0.0"); - } - - // --- Regression: qualifier handling ------------------------------------- - // - // Qualifiers are stripped *before* the version is split off with - // `rfind('@')`. This matters because a qualifier *value* can itself - // contain an `@` (e.g. a `git@github.com` source URL). If stripping - // ran after the `@` search, that trailing `@` would be mistaken for - // the version separator and corrupt both name and version. - #[test] fn test_strip_qualifiers_with_embedded_at() { assert_eq!( @@ -1077,8 +832,6 @@ mod tests { ); } - // --- Regression: purl_matches_identifier for non-PyPI keys -------------- - #[test] fn test_purl_matches_identifier_qualified_id_needs_exact_key() { // A qualified identifier must not match an unqualified manifest @@ -1099,13 +852,6 @@ mod tests { )); } - // --- Regression: PURL subpath (`#...`) handling ------------------------- - // - // The PURL grammar is `pkg:type/ns/name@version?qualifiers#subpath`. A - // subpath can appear *without* a preceding qualifier, so stripping only - // at `?` lets it leak into the base — which then corrupts the version - // (split on `@`) and breaks every grouping/matching key built from it. - #[test] fn test_strip_subpath_without_qualifier() { // No `?`, but a trailing `#subpath` must still be removed. @@ -1160,8 +906,6 @@ mod tests { )); } - // --- Percent-decoding: API purls carry %-encoded components -------------- - #[test] fn test_percent_decode_purl_component() { // The canonical case: an encoded npm scope marker. @@ -1244,7 +988,7 @@ mod tests { )); } - /// B73: remove/rollback identifiers fold what the ecosystem folds: a + /// Remove/rollback identifiers fold what the ecosystem folds: a /// PEP 503 spelling or a NuGet case variant names the recorded patch. #[test] fn test_purl_matches_identifier_folds_pep503_and_nuget_case() { @@ -1298,13 +1042,10 @@ mod tests { #[test] fn test_purl_matches_identifier_pypi_pep503_spellings() { - // #1024: patch keys are PEP 503 canonical, but users type the + // Patch keys are PEP 503 canonical, but users type the // name as the project declares it. Every spelling must select the // patch, for base and qualified identifiers alike. - let purl_eq = |a: &str, b: &str| { - crate::utils::purl_key::PurlKey::qualified(a) - == crate::utils::purl_key::PurlKey::qualified(b) - }; + let purl_eq = |a: &str, b: &str| PurlKey::qualified(a) == PurlKey::qualified(b); let key = "pkg:pypi/typing-extensions@4.7.1"; let qualified = "pkg:pypi/typing-extensions@4.7.1?artifact_id=abc"; for spelling in [ @@ -1349,18 +1090,6 @@ mod tests { )); } - #[test] - fn test_parse_jsr_purl_percent_encoded_scope() { - let ((scope, name), version) = parse_jsr_purl("pkg:jsr/%40std/path@0.220.0").unwrap(); - assert_eq!(scope, "@std"); - assert_eq!(name, "path"); - assert_eq!(version, "0.220.0"); - // The encoded bare `@` is still rejected post-decode. - assert_eq!(jsr_parts("pkg:jsr/%40/path@0.220.0"), None); - } - - // --- Regression: name must not absorb the version separator ------------- - #[test] fn test_parse_multiple_at_takes_last_as_version_separator() { // `rfind('@')` (not `find`) ensures the *last* `@` splits the @@ -1370,11 +1099,53 @@ mod tests { Some(("weird@name".into(), "1.0.0".into())) ); } -} -#[cfg(test)] -mod parts_tests { - use super::*; + #[test] + fn purl_builders_validate_coordinates() { + assert_eq!( + npm_purl("left-pad", "1.3.0").as_deref(), + Some("pkg:npm/left-pad@1.3.0") + ); + assert_eq!( + npm_purl("@s/x", "1.0.0").as_deref(), + Some("pkg:npm/@s/x@1.0.0") + ); + assert_eq!(npm_purl("a/b", "1.0.0"), None, "unscoped slash"); + assert_eq!(npm_purl("@s/x/y", "1.0.0"), None, "extra level"); + assert_eq!(npm_purl("x", "../1"), None); + assert_eq!(npm_purl("..", "1.0.0"), None); + assert_eq!( + pypi_purl("Foo.Bar", "1.0").as_deref(), + Some("pkg:pypi/foo-bar@1.0") + ); + assert_eq!(pypi_purl("a/b", "1.0"), None); + assert_eq!( + simple_purl("cargo", "serde", "1.0.0").as_deref(), + Some("pkg:cargo/serde@1.0.0") + ); + assert_eq!( + simple_purl("npm", "x", "1"), + None, + "not a single-segment ecosystem" + ); + assert_eq!(simple_purl("gem", "a:b", "1"), None); + assert_eq!( + golang_purl("github.com/foo/bar", "v1.2.3").as_deref(), + Some("pkg:golang/github.com/foo/bar@v1.2.3") + ); + assert_eq!(golang_purl("github.com/../x", "v1"), None); + assert_eq!( + composer_purl("Monolog/Monolog", "2.0.0").as_deref(), + Some("pkg:composer/monolog/monolog@2.0.0") + ); + assert_eq!(composer_purl("monolog", "2.0.0"), None); + assert_eq!( + maven_purl("org.slf4j", "slf4j-api", "1.7.36").as_deref(), + Some("pkg:maven/org.slf4j/slf4j-api@1.7.36") + ); + assert_eq!(maven_purl("org..x", "a", "1"), None); + assert_eq!(maven_purl("", "a", "1"), None, "empty group"); + } #[test] fn purl_parts_strip_qualifiers_and_decode() { diff --git a/crates/socket-patch-core/src/utils/target.rs b/crates/socket-patch-core/src/utils/target.rs index 5cfac39c3..03558104e 100644 --- a/crates/socket-patch-core/src/utils/target.rs +++ b/crates/socket-patch-core/src/utils/target.rs @@ -33,7 +33,7 @@ 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::{purl_matches_identifier, strip_purl_qualifiers}; use crate::utils::purl_key::{canonical_base_purl, PurlKey}; /// What a target token names. @@ -90,7 +90,7 @@ impl Target { TargetKind::Cve } else if is_ghsa_id(token) { TargetKind::Ghsa - } else if is_purl(token) { + } else if token.starts_with("pkg:") { TargetKind::Purl } else { TargetKind::Name @@ -367,7 +367,7 @@ pub fn is_ghsa_id(s: &str) -> bool { /// `@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) { + if token.starts_with("pkg:") || is_npm_scoped_name(token) { return false; } token.contains('/') From 9a0b930aaa3be4473d4ce5dca6ddbc71b1ea8d47 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 07:17:57 +0000 Subject: [PATCH 3/3] Keep one purl builder round-trip test Each builder now round-trips through its parser with a literal + version, replacing the 14 deleted builder and round-trip tests with one that fails when a builder and its parser disagree. The pipenv probe's unused root parameter is named _root. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/utils/pipenv.rs | 3 +- crates/socket-patch-core/src/utils/purl.rs | 40 ++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-core/src/utils/pipenv.rs b/crates/socket-patch-core/src/utils/pipenv.rs index 3d01b6458..bfc9a6e82 100644 --- a/crates/socket-patch-core/src/utils/pipenv.rs +++ b/crates/socket-patch-core/src/utils/pipenv.rs @@ -42,7 +42,7 @@ fn parse_major(output: &str) -> Option { /// when none is found, it does not answer within 10 s, exits non-zero or /// prints something unrecognizable. [`MAJOR_OVERRIDE_ENV`] short-circuits /// the probe. -pub async fn installed_major(root: &Path) -> Option { +pub async fn installed_major(_root: &Path) -> Option { if let Some(forced) = std::env::var(MAJOR_OVERRIDE_ENV) .ok() .and_then(|value| parse_override(&value)) @@ -56,7 +56,6 @@ pub async fn installed_major(root: &Path) -> Option { // NEUTRAL directory: with the scanned repository as cwd, Pipenv would read // its `.env`, `Pipfile` and `.venv` pointer — committed, attacker-shaped // inputs that must not influence (or slow down) a version check. - let _ = root; command .arg("--version") .current_dir(std::env::temp_dir()) diff --git a/crates/socket-patch-core/src/utils/purl.rs b/crates/socket-patch-core/src/utils/purl.rs index 18f80ef2e..777020de0 100644 --- a/crates/socket-patch-core/src/utils/purl.rs +++ b/crates/socket-patch-core/src/utils/purl.rs @@ -561,6 +561,46 @@ mod tests { ); } + /// Each builder's spelling parses back to its parts, including a + /// literal (already decoded) `+` version. + #[test] + fn builders_round_trip_through_their_parsers() { + let version = "1.0.0+meta"; + assert_eq!( + nv(parse_cargo_purl(&build_cargo_purl("wasi", version))), + Some(("wasi".into(), version.into())) + ); + assert_eq!( + nv(parse_gem_purl(&build_gem_purl("rails", version))), + Some(("rails".into(), version.into())) + ); + assert_eq!( + nv(parse_nuget_purl(&build_nuget_purl("Foo.Bar", version))), + Some(("Foo.Bar".into(), version.into())) + ); + assert_eq!( + nv(parse_golang_purl(&build_golang_purl( + "golang.org/x/text", + version + ))), + Some(("golang.org/x/text".into(), version.into())) + ); + assert_eq!( + nv3(parse_maven_purl(&build_maven_purl("org.x", "lib", version))), + Some(("org.x".into(), "lib".into(), version.into())) + ); + assert_eq!( + nsv(parse_composer_purl(&build_composer_purl( + "psr", "log", version + ))), + Some((("psr".into(), "log".into()), version.into())) + ); + assert_eq!( + jsr_parts(&build_jsr_purl("@std", "path", version)), + Some(("@std".into(), "path".into(), version.into())) + ); + } + /// The namespace and name halves are decoded too — and the split runs on /// the ENCODED text, so an escaped separator stays INSIDE the component /// it was written in instead of creating a new one (the path-safety