Skip to content

Commit 3757e27

Browse files
Fix agent vex checking only one installed copy (#516) (#517)
* Start fix for #516 Assisted-by: Claude Code:claude-opus-5-5 * Check every installed copy before vex attests When a project holds several installed copies of the same package@version (npm nests duplicates, and a later install can add a fresh unpatched one), `vex` only hashed the first copy it found. It could then attest a patch as not_affected while another copy that a dependent loads was still unpatched. Agent-mode records are now attested only when every installed copy matches the patched bytes, the same rule `apply` follows when it patches and that hosted records already use. The vendored drift warning also checks every copy. Fixes #516 Assisted-by: Claude Code:claude-opus-5-5 * Document every-copy rule for agent vex records Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 35de754 commit 3757e27

5 files changed

Lines changed: 285 additions & 20 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ Recognition rules that hold for every ecosystem:
387387
|---|---|---|
388388
| 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)` |
389389
| 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)` |
390-
| Agent: a manifest record with no live hosted/vendored wiring | The installed tree, unchanged | none |
390+
| 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 |
391391

392392
**Liveness gates.** These gates run before hashing, and `--no-verify` / `--vex-no-verify` skips only the hashing, never the gates:
393393

‎crates/socket-patch-cli/src/commands/vex.rs‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use crate::commands::vex_sources::{
3232
self, Plan, Sources, RECORD_MISMATCH, RECORD_UNAVAILABLE, REDIRECT_UNWIRED, VENDOR_UNWIRED,
3333
WIRING_CONFLICT,
3434
};
35-
use crate::ecosystem_dispatch::{collapse_to_first, find_manifest_package_copies_reusing};
35+
use crate::ecosystem_dispatch::find_manifest_package_copies_reusing;
3636
use crate::json_envelope::{Command, Envelope, EnvelopeError, PatchAction, PatchEvent, RunWarning};
3737
use crate::ui::plural;
3838

@@ -548,12 +548,14 @@ async fn generate_vex(
548548
// mirroring apply/rollback's `silent || json` gating.
549549
let quiet = common.silent || common.json || params.output.is_none();
550550
let purls: Vec<String> = manifest.patches.keys().cloned().collect();
551-
// ONE installed-tree lookup: the first copy of every purl for the
552-
// record check, every copy of the hosted ones below.
551+
// ONE installed-tree lookup: every copy of every purl, for the
552+
// record check and the hosted ones below alike. `apply` patches
553+
// every copy, so an agent record is attested only when EVERY copy
554+
// verifies; the first copy alone would vouch for a later install's
555+
// unpatched nested duplicate.
553556
let copies =
554557
find_manifest_package_copies_reusing(&purls, common, quiet, params.npm_prior.as_ref())
555558
.await;
556-
let package_paths = collapse_to_first(copies.clone());
557559
let go_patches = synthesize_go_patches(common, manifest, &plan.vendor_entries).await;
558560
// Hosted-basis purls are judged by the copies their build CONSUMES
559561
// (the Go replacement module, the Socket registry's cargo src dir,
@@ -573,12 +575,9 @@ async fn generate_vex(
573575
go_patches,
574576
hosted,
575577
};
576-
let mut outcome = socket_patch_core::vex::applied_patches_with_vendor(
577-
manifest,
578-
&package_paths,
579-
Some(&vendor),
580-
)
581-
.await;
578+
let mut outcome =
579+
socket_patch_core::vex::applied_patches_with_copies(manifest, &copies, Some(&vendor))
580+
.await;
582581
// Hosted lockfile basis: a DISCOVERED Socket-host reference whose
583582
// lock pins the artifact attests from that wiring when no installed
584583
// tree exists yet (a lockfile-only CI checkout) — the evidence the

‎crates/socket-patch-cli/tests/e2e_vex.rs‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -885,6 +885,112 @@ fn verify_mode_includes_applied_omits_unapplied() {
885885
maybe_validate_with_vexctl(&stdout);
886886
}
887887

888+
/// Lay down `node_modules/<parent>/node_modules/dup-pkg@1.0.0` for every
889+
/// `(parent, index.js bytes)` pair: nested duplicates of ONE `name@version`,
890+
/// the layout a later `npm install` of a new dependent produces.
891+
fn lay_down_nested_dups(cwd: &Path, copies: &[(&str, &[u8])]) {
892+
for (parent, content) in copies {
893+
let parent_dir = cwd.join("node_modules").join(parent);
894+
std::fs::create_dir_all(&parent_dir).unwrap();
895+
std::fs::write(
896+
parent_dir.join("package.json"),
897+
format!(r#"{{"name":"{parent}","version":"1.0.0"}}"#),
898+
)
899+
.unwrap();
900+
let dup = parent_dir.join("node_modules").join("dup-pkg");
901+
std::fs::create_dir_all(&dup).unwrap();
902+
std::fs::write(
903+
dup.join("package.json"),
904+
r#"{"name":"dup-pkg","version":"1.0.0"}"#,
905+
)
906+
.unwrap();
907+
std::fs::write(dup.join("index.js"), content).unwrap();
908+
}
909+
}
910+
911+
/// Regression (#516): an agent record is attested only when EVERY
912+
/// installed copy of its `name@version` is patched. `apply` patches every
913+
/// copy, but `vex` used to hash only the crawler's FIRST copy, so a fresh,
914+
/// unpatched nested copy (added by a later install) was attested
915+
/// `not_affected` whenever the patched copy happened to be crawled first.
916+
/// Both crawl orders must omit the purl; all copies patched attests it.
917+
#[test]
918+
fn verify_mode_requires_every_installed_copy_patched() {
919+
let patched: &[u8] = b"patched dup index";
920+
let pristine: &[u8] = b"pristine dup index";
921+
let after_hash = compute_git_sha256_from_bytes(patched);
922+
let before_hash = compute_git_sha256_from_bytes(pristine);
923+
924+
let run = |copies: &[(&str, &[u8])]| {
925+
let tmp = tempfile::tempdir().unwrap();
926+
let cwd = tmp.path();
927+
lay_down_nested_dups(cwd, copies);
928+
let mut manifest = PatchManifest::new();
929+
manifest.patches.insert(
930+
"pkg:npm/dup-pkg@1.0.0".to_string(),
931+
make_record(
932+
"44444444-4444-4444-8444-444444444444",
933+
"package/index.js",
934+
before_hash.as_str(),
935+
after_hash.as_str(),
936+
"GHSA-dup",
937+
&["CVE-DUP"],
938+
),
939+
);
940+
write_manifest(cwd, &manifest);
941+
let out = cli()
942+
.args([
943+
"vex",
944+
"--cwd",
945+
cwd.to_str().unwrap(),
946+
"--product",
947+
"pkg:npm/test-app@1.0.0",
948+
])
949+
.output()
950+
.expect("invoke vex");
951+
(
952+
out.status.success(),
953+
String::from_utf8_lossy(&out.stdout).into_owned(),
954+
String::from_utf8_lossy(&out.stderr).into_owned(),
955+
)
956+
};
957+
958+
// One copy patched, the other pristine — in both orders, so the
959+
// verdict cannot depend on which copy the crawler meets first.
960+
for copies in [
961+
[("aaa-parent", patched), ("zzz-parent", pristine)],
962+
[("aaa-parent", pristine), ("zzz-parent", patched)],
963+
] {
964+
let (ok, stdout, stderr) = run(&copies);
965+
assert!(
966+
!ok,
967+
"an unpatched installed copy must keep the purl out of the VEX \
968+
doc (nothing left to attest → non-zero exit). copies: {:?}\n\
969+
stdout:\n{stdout}\nstderr:\n{stderr}",
970+
copies.map(|(p, _)| p)
971+
);
972+
assert!(
973+
!stdout.contains("GHSA-dup"),
974+
"must not attest while a copy is unpatched:\n{stdout}"
975+
);
976+
assert!(
977+
stderr.contains("Warning: omitting pkg:npm/dup-pkg@1.0.0 from VEX")
978+
&& stderr.contains("(not_applied)"),
979+
"the omission must name the unpatched copy's not_applied tag. \
980+
got: {stderr}"
981+
);
982+
}
983+
984+
// Control: every copy patched → attested.
985+
let (ok, stdout, stderr) = run(&[("aaa-parent", patched), ("zzz-parent", patched)]);
986+
assert!(ok, "all copies patched must attest. stderr:\n{stderr}");
987+
let doc: Value = serde_json::from_str(&stdout).unwrap();
988+
let stmts = doc["statements"].as_array().unwrap();
989+
assert_eq!(stmts.len(), 1, "doc:\n{stdout}");
990+
assert_eq!(stmts[0]["vulnerability"]["name"], "GHSA-dup");
991+
assert_eq!(stmts[0]["status"], "not_affected");
992+
}
993+
888994
#[test]
889995
fn verify_mode_all_failed_exits_non_zero() {
890996
let tmp = tempfile::tempdir().unwrap();

‎crates/socket-patch-core/src/vex/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ pub use schema::{
3636
OPENVEX_CONTEXT_V0_2_0,
3737
};
3838
pub use verify::{
39-
applied_patches, applied_patches_with_vendor, FailedPatch, HostedCopies, VendorContext,
40-
VerifyOutcome,
39+
applied_patches, applied_patches_with_copies, applied_patches_with_vendor, FailedPatch,
40+
HostedCopies, VendorContext, VerifyOutcome,
4141
};
4242

4343
#[cfg(test)]

0 commit comments

Comments
 (0)