diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 72bffac7a..617a3f836 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -387,7 +387,7 @@ Recognition rules that hold for every ecosystem: |---|---|---| | Vendored: a lockfile/config wires a `.socket/vendor` artifact, or a live vendor ledger entry | The **committed artifact** is hashed against the record's `afterHash`. The ledger entry is used when it names the wired artifact (it carries the dir-artifact inventory); otherwise an entry is synthesized from the reference. A present installed tree with different bytes only warns `vendored_tree_out_of_sync`. | `(vendored)` | | Hosted: a discovered patch-host reference (or a live pre-v5 redirect-ledger record) | The installed copies the build **consumes** through the hosted wiring are hash-verified when any exist: the Go replacement module, never the pristine `M@v` in the module cache; the Socket-registry cargo source dir; maven's suffixed version. Installed evidence wins: `hash_mismatch` / `not_applied` are omitted. With **nothing installed**, a discovered reference whose lock pins the artifact (or whose format's rewriter never writes a pin) attests from that pin, which is the same evidence as in-run `scan --mode hosted --vex`. A pre-v5 ledger-only record, or a reference whose required pin is missing, stays `package_not_found`. So do purls that `--ecosystems` kept out of the crawl, because "not installed" has to mean the crawler looked. | `(redirected)` | -| Agent: a manifest record with no live hosted/vendored wiring | The installed tree, unchanged | none | +| Agent: a manifest record with no live hosted/vendored wiring | The installed tree, unchanged. **Every** installed copy the crawler finds for the purl (npm nests duplicates of one `name@version`) must hash to the patched bytes, as `apply` patches every copy. One unpatched copy omits the purl with that copy's tag (`not_applied` / `hash_mismatch`). | none | **Liveness gates.** These gates run before hashing, and `--no-verify` / `--vex-no-verify` skips only the hashing, never the gates: diff --git a/crates/socket-patch-cli/src/commands/vex.rs b/crates/socket-patch-cli/src/commands/vex.rs index 7eed23e6a..43eff8991 100644 --- a/crates/socket-patch-cli/src/commands/vex.rs +++ b/crates/socket-patch-cli/src/commands/vex.rs @@ -32,7 +32,7 @@ use crate::commands::vex_sources::{ self, Plan, Sources, RECORD_MISMATCH, RECORD_UNAVAILABLE, REDIRECT_UNWIRED, VENDOR_UNWIRED, WIRING_CONFLICT, }; -use crate::ecosystem_dispatch::{collapse_to_first, find_manifest_package_copies_reusing}; +use crate::ecosystem_dispatch::find_manifest_package_copies_reusing; use crate::json_envelope::{Command, Envelope, EnvelopeError, PatchAction, PatchEvent, RunWarning}; use crate::ui::plural; @@ -548,12 +548,14 @@ async fn generate_vex( // mirroring apply/rollback's `silent || json` gating. let quiet = common.silent || common.json || params.output.is_none(); let purls: Vec = manifest.patches.keys().cloned().collect(); - // ONE installed-tree lookup: the first copy of every purl for the - // record check, every copy of the hosted ones below. + // ONE installed-tree lookup: every copy of every purl, for the + // record check and the hosted ones below alike. `apply` patches + // every copy, so an agent record is attested only when EVERY copy + // verifies; the first copy alone would vouch for a later install's + // unpatched nested duplicate. let copies = find_manifest_package_copies_reusing(&purls, common, quiet, params.npm_prior.as_ref()) .await; - let package_paths = collapse_to_first(copies.clone()); let go_patches = synthesize_go_patches(common, manifest, &plan.vendor_entries).await; // Hosted-basis purls are judged by the copies their build CONSUMES // (the Go replacement module, the Socket registry's cargo src dir, @@ -573,12 +575,9 @@ async fn generate_vex( go_patches, hosted, }; - let mut outcome = socket_patch_core::vex::applied_patches_with_vendor( - manifest, - &package_paths, - Some(&vendor), - ) - .await; + let mut outcome = + socket_patch_core::vex::applied_patches_with_copies(manifest, &copies, Some(&vendor)) + .await; // Hosted lockfile basis: a DISCOVERED Socket-host reference whose // lock pins the artifact attests from that wiring when no installed // tree exists yet (a lockfile-only CI checkout) — the evidence the diff --git a/crates/socket-patch-cli/tests/e2e_vex.rs b/crates/socket-patch-cli/tests/e2e_vex.rs index 90c3adc38..ffe748bbf 100644 --- a/crates/socket-patch-cli/tests/e2e_vex.rs +++ b/crates/socket-patch-cli/tests/e2e_vex.rs @@ -885,6 +885,112 @@ fn verify_mode_includes_applied_omits_unapplied() { maybe_validate_with_vexctl(&stdout); } +/// Lay down `node_modules//node_modules/dup-pkg@1.0.0` for every +/// `(parent, index.js bytes)` pair: nested duplicates of ONE `name@version`, +/// the layout a later `npm install` of a new dependent produces. +fn lay_down_nested_dups(cwd: &Path, copies: &[(&str, &[u8])]) { + for (parent, content) in copies { + let parent_dir = cwd.join("node_modules").join(parent); + std::fs::create_dir_all(&parent_dir).unwrap(); + std::fs::write( + parent_dir.join("package.json"), + format!(r#"{{"name":"{parent}","version":"1.0.0"}}"#), + ) + .unwrap(); + let dup = parent_dir.join("node_modules").join("dup-pkg"); + std::fs::create_dir_all(&dup).unwrap(); + std::fs::write( + dup.join("package.json"), + r#"{"name":"dup-pkg","version":"1.0.0"}"#, + ) + .unwrap(); + std::fs::write(dup.join("index.js"), content).unwrap(); + } +} + +/// Regression (#516): an agent record is attested only when EVERY +/// installed copy of its `name@version` is patched. `apply` patches every +/// copy, but `vex` used to hash only the crawler's FIRST copy, so a fresh, +/// unpatched nested copy (added by a later install) was attested +/// `not_affected` whenever the patched copy happened to be crawled first. +/// Both crawl orders must omit the purl; all copies patched attests it. +#[test] +fn verify_mode_requires_every_installed_copy_patched() { + let patched: &[u8] = b"patched dup index"; + let pristine: &[u8] = b"pristine dup index"; + let after_hash = compute_git_sha256_from_bytes(patched); + let before_hash = compute_git_sha256_from_bytes(pristine); + + let run = |copies: &[(&str, &[u8])]| { + let tmp = tempfile::tempdir().unwrap(); + let cwd = tmp.path(); + lay_down_nested_dups(cwd, copies); + let mut manifest = PatchManifest::new(); + manifest.patches.insert( + "pkg:npm/dup-pkg@1.0.0".to_string(), + make_record( + "44444444-4444-4444-8444-444444444444", + "package/index.js", + before_hash.as_str(), + after_hash.as_str(), + "GHSA-dup", + &["CVE-DUP"], + ), + ); + write_manifest(cwd, &manifest); + let out = cli() + .args([ + "vex", + "--cwd", + cwd.to_str().unwrap(), + "--product", + "pkg:npm/test-app@1.0.0", + ]) + .output() + .expect("invoke vex"); + ( + out.status.success(), + String::from_utf8_lossy(&out.stdout).into_owned(), + String::from_utf8_lossy(&out.stderr).into_owned(), + ) + }; + + // One copy patched, the other pristine — in both orders, so the + // verdict cannot depend on which copy the crawler meets first. + for copies in [ + [("aaa-parent", patched), ("zzz-parent", pristine)], + [("aaa-parent", pristine), ("zzz-parent", patched)], + ] { + let (ok, stdout, stderr) = run(&copies); + assert!( + !ok, + "an unpatched installed copy must keep the purl out of the VEX \ + doc (nothing left to attest → non-zero exit). copies: {:?}\n\ + stdout:\n{stdout}\nstderr:\n{stderr}", + copies.map(|(p, _)| p) + ); + assert!( + !stdout.contains("GHSA-dup"), + "must not attest while a copy is unpatched:\n{stdout}" + ); + assert!( + stderr.contains("Warning: omitting pkg:npm/dup-pkg@1.0.0 from VEX") + && stderr.contains("(not_applied)"), + "the omission must name the unpatched copy's not_applied tag. \ + got: {stderr}" + ); + } + + // Control: every copy patched → attested. + let (ok, stdout, stderr) = run(&[("aaa-parent", patched), ("zzz-parent", patched)]); + assert!(ok, "all copies patched must attest. stderr:\n{stderr}"); + let doc: Value = serde_json::from_str(&stdout).unwrap(); + let stmts = doc["statements"].as_array().unwrap(); + assert_eq!(stmts.len(), 1, "doc:\n{stdout}"); + assert_eq!(stmts[0]["vulnerability"]["name"], "GHSA-dup"); + assert_eq!(stmts[0]["status"], "not_affected"); +} + #[test] fn verify_mode_all_failed_exits_non_zero() { let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/socket-patch-core/src/vex/mod.rs b/crates/socket-patch-core/src/vex/mod.rs index 561263f3d..0fbe828c3 100644 --- a/crates/socket-patch-core/src/vex/mod.rs +++ b/crates/socket-patch-core/src/vex/mod.rs @@ -36,8 +36,8 @@ pub use schema::{ OPENVEX_CONTEXT_V0_2_0, }; pub use verify::{ - applied_patches, applied_patches_with_vendor, FailedPatch, HostedCopies, VendorContext, - VerifyOutcome, + applied_patches, applied_patches_with_copies, applied_patches_with_vendor, FailedPatch, + HostedCopies, VendorContext, VerifyOutcome, }; #[cfg(test)] diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index d29c41a20..00093383c 100644 --- a/crates/socket-patch-core/src/vex/verify.rs +++ b/crates/socket-patch-core/src/vex/verify.rs @@ -139,10 +139,36 @@ pub async fn applied_patches( /// ([`HostedCopies`]) — no fallback to `package_paths` either, whose /// representative may be the pristine sibling the build never reads. /// 4. Otherwise the installed-tree behavior of [`applied_patches`], verbatim. +/// +/// `package_paths` carries ONE representative copy per PURL; see +/// [`applied_patches_with_copies`] for the every-copy form `vex` uses. pub async fn applied_patches_with_vendor( manifest: &PatchManifest, package_paths: &HashMap, vendor: Option<&VendorContext>, +) -> VerifyOutcome { + let copies: HashMap> = package_paths + .iter() + .map(|(purl, path)| (purl.clone(), vec![path.clone()])) + .collect(); + applied_patches_with_copies(manifest, &copies, vendor).await +} + +/// [`applied_patches_with_vendor`] over EVERY installed copy the crawler +/// found per PURL (crawl order), not one representative. +/// +/// `apply` patches every physical copy (npm nests genuine duplicates of one +/// `name@version`), and any copy may be the one some dependent loads — so an +/// installed-tree record is applied only when EVERY copy verifies; the +/// first failing copy's tag wins, exactly like [`HostedCopies`]. Judging the +/// first copy alone would attest `not_affected` while a later install's +/// fresh, unpatched nested copy runs. An empty copy list is +/// `package_not_found`. The vendored drift probe likewise flags the PURL +/// when ANY installed copy is out of sync. +pub async fn applied_patches_with_copies( + manifest: &PatchManifest, + package_copies: &HashMap>, + vendor: Option<&VendorContext>, ) -> VerifyOutcome { let mut out = VerifyOutcome::default(); @@ -155,8 +181,8 @@ pub async fn applied_patches_with_vendor( verify_patch_record(copy_dir, record).await } else if let Some(copies) = vendor.and_then(|ctx| ctx.hosted.get(purl)) { verify_hosted_copies(copies, record).await - } else if let Some(pkg_path) = package_paths.get(purl) { - verify_patch_record(pkg_path, record).await + } else if let Some(paths) = package_copies.get(purl).filter(|p| !p.is_empty()) { + verify_every_copy(paths, record).await } else { Err("package_not_found".to_string()) }; @@ -182,7 +208,12 @@ pub async fn applied_patches_with_vendor( // bypassing build, and "re-run your install to resync // it" is advice no `go` command can follow. let go_cache_copy = purl.starts_with("pkg:golang/"); - if let Some(pkg_path) = package_paths.get(purl).filter(|_| !go_cache_copy) { + let installed = package_copies + .get(purl) + .filter(|_| !go_cache_copy) + .map(Vec::as_slice) + .unwrap_or_default(); + for pkg_path in installed { let in_sync = if is_vlt_dir_entry(entry) { vlt_installed_copy_matches(&ctx.project_root, pkg_path, entry, record) .await @@ -191,6 +222,7 @@ pub async fn applied_patches_with_vendor( }; if !in_sync { out.vendored_out_of_sync.push(purl.clone()); + break; } } } @@ -297,6 +329,15 @@ fn judge_installed_files(pkg_path: &Path, files: &[(String, String)]) -> Install } } +/// Every copy must pass [`verify_patch_record`]; the first failure's tag +/// wins. The caller guarantees `paths` is non-empty. +async fn verify_every_copy(paths: &[PathBuf], record: &PatchRecord) -> Result<(), String> { + for path in paths { + verify_patch_record(path, record).await?; + } + Ok(()) +} + /// [`HostedCopies`] verdict: no consumed copy is `package_not_found`; every /// listed copy must pass [`verify_patch_record`] (under the maven file /// rename, when set), the first failure's tag wins. @@ -312,10 +353,7 @@ async fn verify_hosted_copies(copies: &HostedCopies, record: &PatchRecord) -> Re } None => record, }; - for path in &copies.paths { - verify_patch_record(path, record).await?; - } - Ok(()) + verify_every_copy(&copies.paths, record).await } /// `record` with every file key whose name starts with the whole component @@ -393,6 +431,67 @@ mod tests { assert!(out.failed.is_empty()); } + /// Two nested copies of one `name@version`: `(dir, bytes)` each. + async fn two_copies(a: &[u8], b: &[u8]) -> (tempfile::TempDir, Vec) { + let root = tempfile::tempdir().unwrap(); + let mut paths = Vec::new(); + for (i, bytes) in [a, b].into_iter().enumerate() { + let dir = root.path().join(format!("copy{i}")); + tokio::fs::create_dir_all(&dir).await.unwrap(); + tokio::fs::write(dir.join("index.js"), bytes).await.unwrap(); + paths.push(dir); + } + (root, paths) + } + + /// Regression (#516): an installed-tree record verifies only when EVERY + /// copy does, whichever copy the crawler met first; the unpatched + /// copy's tag is reported. + #[tokio::test] + async fn every_installed_copy_must_verify() { + let patched = b"patched-content"; + let pristine = b"pristine-content"; + let hash = compute_git_sha256_from_bytes(patched); + let mut record = record_with_one_file(&hash); + record.files.get_mut("index.js").unwrap().before_hash = + compute_git_sha256_from_bytes(pristine); + let mut manifest = PatchManifest::new(); + manifest + .patches + .insert("pkg:npm/x@1.0.0".to_string(), record); + + for (a, b) in [(&patched[..], &pristine[..]), (&pristine[..], &patched[..])] { + let (_root, paths) = two_copies(a, b).await; + let copies = HashMap::from([("pkg:npm/x@1.0.0".to_string(), paths)]); + let out = applied_patches_with_copies(&manifest, &copies, None).await; + assert!( + out.applied.is_empty(), + "a pristine copy must block attestation" + ); + assert_eq!(out.failed[0].reason, "not_applied"); + } + + let (_root, paths) = two_copies(patched, patched).await; + let copies = HashMap::from([("pkg:npm/x@1.0.0".to_string(), paths)]); + let out = applied_patches_with_copies(&manifest, &copies, None).await; + assert_eq!(out.applied, vec!["pkg:npm/x@1.0.0".to_string()]); + assert!(out.failed.is_empty()); + } + + /// An empty copy list is `package_not_found`, never a vacuous pass. + #[tokio::test] + async fn empty_copy_list_is_package_not_found() { + let mut manifest = PatchManifest::new(); + manifest.patches.insert( + "pkg:npm/x@1.0.0".to_string(), + record_with_one_file("deadbeef"), + ); + let copies = HashMap::from([("pkg:npm/x@1.0.0".to_string(), Vec::new())]); + let out = applied_patches_with_copies(&manifest, &copies, None).await; + assert!(out.applied.is_empty()); + assert_eq!(out.failed[0].reason, "package_not_found"); + } + #[tokio::test] async fn missing_path_falls_into_failed() { let mut manifest = PatchManifest::new(); @@ -1310,6 +1409,67 @@ mod tests { assert_eq!(out.vendored_out_of_sync, vec![purl.to_string()]); } + /// Drift disclosure covers EVERY installed copy: a first copy in sync + /// must not hide an out-of-sync second one (#516). + #[tokio::test] + async fn vendored_drift_probe_checks_every_installed_copy() { + let root = tempfile::tempdir().unwrap(); + let purl = "pkg:cargo/serde@1.0.0"; + let rel = format!(".socket/vendor/cargo/{VUUID}/serde-1.0.0"); + let original = b"original-unpatched"; + let patched = b"patched-content"; + let before = compute_git_sha256_from_bytes(original); + let after = compute_git_sha256_from_bytes(patched); + + let vdir = root.path().join(&rel); + tokio::fs::create_dir_all(&vdir).await.unwrap(); + tokio::fs::write(vdir.join("index.js"), patched) + .await + .unwrap(); + let mut installed = Vec::new(); + for (name, bytes) in [("in-sync", &patched[..]), ("drifted", &original[..])] { + let dir = root.path().join(name); + tokio::fs::create_dir_all(&dir).await.unwrap(); + tokio::fs::write(dir.join("index.js"), bytes).await.unwrap(); + installed.push(dir); + } + + let mut files = HashMap::new(); + files.insert( + "index.js".to_string(), + PatchFileInfo { + before_hash: before, + after_hash: after, + }, + ); + let rec = PatchRecord { + uuid: VUUID.to_string(), + exported_at: String::new(), + files, + vulnerabilities: HashMap::new(), + description: String::new(), + license: String::new(), + tier: String::new(), + }; + let mut manifest = PatchManifest::new(); + manifest.patches.insert(purl.to_string(), rec); + + let mut entries = HashMap::new(); + entries.insert(purl.to_string(), vendor_entry(purl, &rel)); + let ctx = VendorContext { + project_root: root.path().to_path_buf(), + entries, + go_patches: HashMap::new(), + hosted: HashMap::new(), + }; + let copies = HashMap::from([(purl.to_string(), installed)]); + + let out = applied_patches_with_copies(&manifest, &copies, Some(&ctx)).await; + assert_eq!(out.applied, vec![purl.to_string()]); + assert_eq!(out.vendored, vec![purl.to_string()]); + assert_eq!(out.vendored_out_of_sync, vec![purl.to_string()]); + } + /// Go vendored: the crawler's module-cache `M@v` is pristine by /// construction (immutable, go.sum-verified; the directory `replace` /// builds the committed copy), so it is never reported out of sync —