From c3130c1491cb0e555368774e81e79a4482f6a8d1 Mon Sep 17 00:00:00 2001 From: Kriday Dave Date: Sun, 8 Nov 2026 14:53:20 +0530 Subject: [PATCH] fix: canonicalize policy fingerprints across court/store/visibility (X-12) Sort list fields and include every visibility-policy field in the canonical fingerprint inputs so semantically identical policies (e.g. reordered private_paths) produce the same key everywhere. Before, court sorted lists but store filter keys and visibility audit keys joined in file order and field coverage differed, so a no-op reorder wiped the export cache while the court/docket policy key stayed unchanged. Stored fingerprints change format: the export cache marker will mismatch once and trigger a safe full rebuild; persisted dockets/adjudication log entries keep their old keys as append-only history (no validation rejects them). --- src/court.rs | 32 +++++++++++++++++++------ src/store.rs | 19 ++++----------- src/visibility.rs | 59 +++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 79 insertions(+), 31 deletions(-) diff --git a/src/court.rs b/src/court.rs index 198c307..b7ea000 100644 --- a/src/court.rs +++ b/src/court.rs @@ -184,7 +184,9 @@ pub fn log_adjudication(store: &Store, persisted: &PersistedDocket) -> Result<() /// Stable fingerprint of the two policies an adjudication ran under — the /// export cache key's trick applied to governance: canonical serialization, /// then hash. Any new policy field MUST join the canonical string or stale -/// keys will look equal after a policy change. +/// keys will look equal after a policy change. List fields are sorted and +/// every visibility-policy field is included so semantically identical policies +/// produce the same key regardless of file order. pub fn policy_key(meaning: &MeaningPolicy, visibility: &VisibilityPolicy) -> String { let mut canon = String::new(); canon.push_str("block_on="); @@ -193,12 +195,8 @@ pub fn policy_key(meaning: &MeaningPolicy, visibility: &VisibilityPolicy) -> Str push_list(&mut canon, &meaning.review_on); canon.push_str(";rename_min="); canon.push_str(&meaning.rename_min_score.to_string()); - canon.push_str(";private_paths="); - push_list(&mut canon, &visibility.private_paths); - canon.push_str(";private_branches="); - push_list(&mut canon, &visibility.private_branches); - canon.push_str(";embargo_until="); - canon.push_str(visibility.embargo_until.as_deref().unwrap_or("")); + canon.push(';'); + canon.push_str(&visibility.canonical_fields()); fnv1a(canon.as_bytes()) } @@ -413,6 +411,26 @@ mod tests { assert_ne!(base, policy_key(&MeaningPolicy::default(), &extra_path)); } + // Regression (X-12): reordering semantically identical list entries must not + // change the court policy key (and must agree with visibility/export keys). + #[test] + fn test_policy_key_canonicalizes_list_order() { + let meaning = MeaningPolicy::default(); + let a = VisibilityPolicy { + private_paths: vec!["secrets/".into(), ".env".into()], + private_branches: vec!["internal".into(), "audit".into()], + embargo_recipients: vec!["bob@example.com".into(), "alice@example.com".into()], + ..Default::default() + }; + let b = VisibilityPolicy { + private_paths: vec![".env".into(), "secrets/".into()], + private_branches: vec!["audit".into(), "internal".into()], + embargo_recipients: vec!["alice@example.com".into(), "bob@example.com".into()], + ..Default::default() + }; + assert_eq!(policy_key(&meaning, &a), policy_key(&meaning, &b)); + } + #[test] fn test_adjudicate_root_change_against_empty_base() { let (_tmp, store) = temp_store("root"); diff --git a/src/store.rs b/src/store.rs index ef1d6d7..38434c5 100644 --- a/src/store.rs +++ b/src/store.rs @@ -1272,22 +1272,11 @@ impl Store { // Export mappings are only valid for the policy they were produced // under: a filtered export's shas mean nothing to an unfiltered one // and vice versa. A changed policy wipes the cache before anything - // can silently mix decisions from two regimes. + // can silently mix decisions from two regimes. The key is canonical + // (sorted lists, every visibility field) so reordering semantically + // identical entries does not invalidate the cache. let filter_key = match policy { - Some(p) if filtering => { - let resign = p - .resign_key_id - .as_deref() - .map(str::trim) - .unwrap_or_default(); - format!( - "{}\u{1f}{}\u{1f}{}\u{1f}{}", - p.private_paths.join(","), - p.private_branches.join(","), - p.embargo_until.as_deref().unwrap_or_default(), - resign - ) - } + Some(p) if filtering => p.canonical_fields(), _ => String::new(), }; self.reset_export_cache_if_policy_changed(&filter_key)?; diff --git a/src/visibility.rs b/src/visibility.rs index 5af7850..4ea7046 100644 --- a/src/visibility.rs +++ b/src/visibility.rs @@ -61,20 +61,39 @@ impl Default for VisibilityPolicy { } impl VisibilityPolicy { - /// A stable fingerprint of the fields that decide who may receive a - /// bundle, recorded with the seal event so a later edit to - /// `embargo_recipients` or `embargo_until` is detectable afterwards. - pub fn audit_key(&self) -> String { + /// Canonical form of every visibility-policy field: list fields are sorted + /// so semantically identical policies hash the same regardless of file + /// order, and every field is included so no policy change can hide behind + /// an unchanged fingerprint. Shared by `audit_key`, export filter-keying, + /// and court adjudication keys so they genuinely agree. + pub fn canonical_fields(&self) -> String { let mut canon = String::new(); canon.push_str("embargo_until="); canon.push_str(self.embargo_until.as_deref().unwrap_or("")); - canon.push_str(";recipients="); - canon.push_str(&self.embargo_recipients.join(",")); + canon.push_str(";resign_key_id="); + canon.push_str(self.resign_key_id.as_deref().unwrap_or("")); canon.push_str(";private_paths="); - canon.push_str(&self.private_paths.join(",")); + push_canonical_list(&mut canon, &self.private_paths); canon.push_str(";private_branches="); - canon.push_str(&self.private_branches.join(",")); - crate::court::fnv1a(canon.as_bytes()) + push_canonical_list(&mut canon, &self.private_branches); + canon.push_str(";embargo_recipients="); + push_canonical_list(&mut canon, &self.embargo_recipients); + canon + } + + /// A stable fingerprint of the visibility policy, recorded with seal events + /// and embargo bundles so later edits are detectable afterwards. + pub fn audit_key(&self) -> String { + crate::court::fnv1a(self.canonical_fields().as_bytes()) + } +} + +fn push_canonical_list(out: &mut String, items: &[String]) { + let mut sorted = items.to_vec(); + sorted.sort(); + for item in sorted { + out.push_str(&item); + out.push('\u{1f}'); } } @@ -477,4 +496,26 @@ mod tests { let _ = std::fs::remove_file(&path); let _ = std::fs::remove_dir(&dir); } + + // Regression (X-12): reordering list fields is semantically a no-op, so the + // visibility fingerprint must not change and invalidate caches/keys. + #[test] + fn test_visibility_fingerprint_canonicalizes_list_order() { + let a = VisibilityPolicy { + private_paths: vec!["secrets/".into(), ".env".into(), "vault/".into()], + private_branches: vec!["internal".into(), "audit".into()], + embargo_recipients: vec!["bob@example.com".into(), "alice@example.com".into()], + embargo_until: Some("2026-10-01".into()), + resign_key_id: Some("KEY1".into()), + }; + let b = VisibilityPolicy { + private_paths: vec![".env".into(), "vault/".into(), "secrets/".into()], + private_branches: vec!["audit".into(), "internal".into()], + embargo_recipients: vec!["alice@example.com".into(), "bob@example.com".into()], + embargo_until: Some("2026-10-01".into()), + resign_key_id: Some("KEY1".into()), + }; + assert_eq!(a.canonical_fields(), b.canonical_fields()); + assert_eq!(a.audit_key(), b.audit_key()); + } }