From 26b7b50864eab20a55565f36d0215b5088731aa0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 2 Jul 2026 12:21:22 +0000 Subject: [PATCH 1/2] fix(validate): scope cross-repo UnknownPrefix to consumer-owned links (#649) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `rivet validate` walked EVERY artifact's outgoing links when computing `broken_cross_refs`, so any supplier's own external reference (e.g. `super:REQ-EXT` in an artifact synced under `sup:`) leaked into the consumer as `UnknownPrefix("super")` — 51 broken cross-refs in the reported ordeal case for prefixes the consumer never declared. That contradicts two of rivet's own documented positions: - DD-017 (`rivet docs cross-repo`): "declare direct deps only" - REQ-065 / AoU-X1 (`rivet validate --help`): "the consumer's PASS does not otherwise reflect the supplier's validation state" The cross-ref walk in `cmd_validate` now filters external (`prefix:ID`) artifacts out of both the `local_ids` set and the `all_refs` collection before calling `validate_refs`, mirroring the REQ-082 `is_external_artifact` convention already used across the per-artifact validation passes and by the lifecycle-completeness check on the same page. Consumer-owned broken prefixes still surface (the fix does not silently accept the consumer's own bogus refs); supplier internals stay in the supplier's validation gate, opt-in via `--with-externals-validate` (REQ-065 / AoU-X1). Regression: `validate_does_not_leak_transitive_external_prefixes` spins a supplier whose own artifact links to `super:REQ-EXT` (a prefix the consumer never declares) and a consumer whose own artifact links to a locally-broken `xyz:REQ-BOGUS`. After the fix: `broken_cross_refs` contains no `super:` findings but still contains the `xyz:` finding. Fixes: REQ-065 Refs: DD-017, #649 --- rivet-cli/src/main.rs | 19 ++++- rivet-cli/tests/cli_commands.rs | 146 ++++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 3 deletions(-) diff --git a/rivet-cli/src/main.rs b/rivet-cli/src/main.rs index 72960c2..45e7eed 100644 --- a/rivet-cli/src/main.rs +++ b/rivet-cli/src/main.rs @@ -5815,11 +5815,24 @@ fn cmd_validate( external_ids.insert(ext.prefix.clone(), ids); } - // Collect local IDs and all link targets - let local_ids: std::collections::HashSet = - store.iter().map(|a| a.id.clone()).collect(); + // Collect local IDs and all link targets — consumer-owned + // artifacts only. External (`prefix:ID`) artifacts are + // in the store solely so the consumer's `prefix:ID` + // cross-links resolve (REQ-082); walking their outgoing + // links here would fail the consumer on the supplier's + // OWN external refs (e.g. supplier A's link to `B:foo` + // where the consumer never declared B) — precisely the + // DD-017 / REQ-065 contradiction reported in #649. The + // `--with-externals-validate` opt-in below is the only + // gate that walks supplier internals. + let local_ids: std::collections::HashSet = store + .iter() + .filter(|a| !a.id.contains(':')) + .map(|a| a.id.clone()) + .collect(); let all_refs: Vec<&str> = store .iter() + .filter(|a| !a.id.contains(':')) .flat_map(|a| a.links.iter().map(|l| l.target.as_str())) .collect(); diff --git a/rivet-cli/tests/cli_commands.rs b/rivet-cli/tests/cli_commands.rs index 082bd22..1dedac3 100644 --- a/rivet-cli/tests/cli_commands.rs +++ b/rivet-cli/tests/cli_commands.rs @@ -1482,6 +1482,152 @@ fn validate_does_not_count_external_repo_violations() { ); } +/// #649 / DD-017 / REQ-065 (AoU-X1): the consumer's default `rivet +/// validate` must NOT fail on the SUPPLIER'S own outgoing external +/// references — a consumer that declares only supplier `sup` cannot be +/// expected to know or declare supplier's own externals (DD-017: +/// "declare direct deps only"). The consumer's cross-ref check now +/// walks only consumer-owned links; supplier internals stay in the +/// supplier's validation gate. +/// +/// The reverse case must still fire: a consumer artifact whose own link +/// targets an undeclared prefix continues to surface `UnknownPrefix`. +/// +/// rivet: verifies REQ-065 +#[test] +fn validate_does_not_leak_transitive_external_prefixes() { + let root = tempfile::tempdir().expect("temp dir"); + let supplier = root.path().join("supplier"); + let consumer = root.path().join("consumer"); + std::fs::create_dir_all(&supplier).unwrap(); + std::fs::create_dir_all(&consumer).unwrap(); + + // Supplier: a dev project whose own artifact carries an outgoing + // link to `super:REQ-EXT` — a prefix that would be one of the + // supplier's own externals. The consumer never declares `super:`. + let init_s = Command::new(rivet_bin()) + .args([ + "init", + "--preset", + "dev", + "--dir", + supplier.to_str().unwrap(), + ]) + .output() + .expect("init supplier"); + assert!( + init_s.status.success(), + "init supplier: {}", + String::from_utf8_lossy(&init_s.stderr) + ); + std::fs::write( + supplier.join("artifacts").join("with-transitive.yaml"), + "artifacts:\n\ + \x20 - id: REQ-SUPPLIER-A\n type: requirement\n\ + \x20 title: Supplier req linking to its own external\n\ + \x20 status: approved\n\ + \x20 fields: {priority: must, category: functional}\n\ + \x20 links:\n\ + \x20 - type: satisfies\n\ + \x20 target: super:REQ-EXT\n", + ) + .expect("write supplier with-transitive.yaml"); + + // Consumer: declares supplier ONLY; never mentions `super:`. It + // ALSO carries a legitimately broken outgoing ref of its own + // (`xyz:REQ-BOGUS`) so we can assert the counter-case: the fix + // must not silently accept the consumer's own broken external refs. + let init_c = Command::new(rivet_bin()) + .args([ + "init", + "--preset", + "dev", + "--dir", + consumer.to_str().unwrap(), + ]) + .output() + .expect("init consumer"); + assert!( + init_c.status.success(), + "init consumer: {}", + String::from_utf8_lossy(&init_c.stderr) + ); + std::fs::write( + consumer.join("artifacts").join("bogus.yaml"), + "artifacts:\n\ + \x20 - id: REQ-CONS-A\n type: requirement\n\ + \x20 title: Consumer req with a locally-broken external ref\n\ + \x20 status: approved\n\ + \x20 fields: {priority: must, category: functional}\n\ + \x20 links:\n\ + \x20 - type: satisfies\n\ + \x20 target: xyz:REQ-BOGUS\n", + ) + .expect("write consumer bogus.yaml"); + std::fs::write( + consumer.join("rivet.yaml"), + format!( + "project:\n name: consumer\n version: \"0.1.0\"\n schemas: [common, dev]\n\ + externals:\n sup:\n path: {}\n prefix: sup\n\ + sources:\n - path: artifacts\n format: generic-yaml\n", + supplier.display() + ), + ) + .expect("write consumer rivet.yaml"); + let sync = Command::new(rivet_bin()) + .args(["--project", consumer.to_str().unwrap(), "sync"]) + .output() + .expect("rivet sync"); + assert!( + sync.status.success(), + "sync: {}", + String::from_utf8_lossy(&sync.stderr) + ); + + let out = Command::new(rivet_bin()) + .args([ + "--project", + consumer.to_str().unwrap(), + "validate", + "--format", + "json", + ]) + .output() + .expect("validate"); + let parsed: serde_json::Value = + serde_json::from_slice(&out.stdout).expect("validate JSON"); + let broken = parsed + .get("broken_cross_refs") + .and_then(|v| v.as_array()) + .expect("broken_cross_refs array"); + let leaked: Vec<_> = broken + .iter() + .filter(|b| { + b.get("reference") + .and_then(|r| r.as_str()) + .is_some_and(|r| r.starts_with("super:")) + }) + .collect(); + assert!( + leaked.is_empty(), + "#649: supplier's own external refs must NOT leak into the \ + consumer's cross-ref check; got: {leaked:?}" + ); + let own_bad: Vec<_> = broken + .iter() + .filter(|b| { + b.get("reference") + .and_then(|r| r.as_str()) + .is_some_and(|r| r.starts_with("xyz:")) + }) + .collect(); + assert!( + !own_bad.is_empty(), + "the consumer's own broken external ref (`xyz:REQ-BOGUS`) must \ + still surface as an UnknownPrefix broken cross-ref; got only: {broken:?}" + ); +} + // ── rivet stats ───────────────────────────────────────────────────────── /// `rivet stats --format json` produces valid JSON with total count. From ca3cefe9763ec2d77c76d09bc7af54df16dc7d93 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Fri, 3 Jul 2026 07:53:30 +0200 Subject: [PATCH 2/2] style: cargo fmt (collapse one-line let in cli_commands test) --- rivet-cli/tests/cli_commands.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rivet-cli/tests/cli_commands.rs b/rivet-cli/tests/cli_commands.rs index 1dedac3..8dc0bc4 100644 --- a/rivet-cli/tests/cli_commands.rs +++ b/rivet-cli/tests/cli_commands.rs @@ -1594,8 +1594,7 @@ fn validate_does_not_leak_transitive_external_prefixes() { ]) .output() .expect("validate"); - let parsed: serde_json::Value = - serde_json::from_slice(&out.stdout).expect("validate JSON"); + let parsed: serde_json::Value = serde_json::from_slice(&out.stdout).expect("validate JSON"); let broken = parsed .get("broken_cross_refs") .and_then(|v| v.as_array())