From 9fa726bbe156120f72bdb6b3609022bf513303b4 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Thu, 17 Sep 2026 16:32:16 +0000 Subject: [PATCH] fix(audit): scope state to the selected repository's maintainers Fetching kind 30618 by identifier alone mixed independently owned repositories. A newer unrelated state could supply the wrong expected refs even though both repositories were valid on the relay. Fetch announcements alongside state, select each author's NIP-01-preferred announcement, and resolve current publishers from the selected coordinate. Support reciprocal legacy/indexed roles, transitive confirmation, departures and explicit lead forwarding; reject unresolved lead paths. Verify and scope events before selecting the newest authorized state. Keep the audit crate independent of ngit-grasp. Authority uses the probed relay's visible announcements. This does not change server authorization, add cross-relay discovery or provide an atomic Git/Nostr snapshot. Validation: 95 grasp-audit tests pass (5 ignored), including membership and ordering regressions. The local-relay integration target passes all 55 tests, with an end-to-end same-identifier collision and injected stale refs. Workspace all-target Clippy and formatting pass. Assisted-by: GPT-6 --- CHANGELOG.md | 6 +- grasp-audit/README.md | 6 + grasp-audit/src/probe.rs | 69 +++---- grasp-audit/src/probe/state.rs | 362 ++++++++++++++++++++++++++++++++- tests/audit_probe_state.rs | 115 +++++++++++ 5 files changed, 514 insertions(+), 44 deletions(-) create mode 100644 tests/audit_probe_state.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d8ba92..f8af0e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Report unexpected branches and tags in audit probes, including refs left - behind when the winning repository state is empty. +- Scope read-only audit state selection to the selected repository's confirmed + maintainers, excluding unrelated repositories with the same identifier. + Audit probes now also report unexpected branches and tags, including refs + left behind when the winning state is empty. - Select one complete repository state in the read-only audit probe, using the lower event ID for same-second ties. Older states no longer contribute diff --git a/grasp-audit/README.md b/grasp-audit/README.md index 6295d09..d07bb9d 100644 --- a/grasp-audit/README.md +++ b/grasp-audit/README.md @@ -69,6 +69,12 @@ bodies. They are deliberately read-only and cannot be combined with | `git_fetch_refs` | Git HTTP info/refs endpoint responds | | `git_refs_match_state` | Git refs match the newest kind:30618 state snapshot (lowest event ID breaks timestamp ties) | +The read-only ref check resolves the selected announcement's confirmed maintainer +component before choosing state. Same-identifier repositories owned by unrelated +authors cannot supply its expected refs. Legacy reciprocal listings and indexed +roles are supported; missing or conflicting lead paths fail the check. The probe +uses the announcements visible on the queried relay at the time of the check. + Both read-only and write probes compare the complete branch/tag set: missing, changed, and unexpected refs fail. HEAD, Nostr refs, and peeled annotated-tag entries are excluded from that set. diff --git a/grasp-audit/src/probe.rs b/grasp-audit/src/probe.rs index ff1bc8e..7e5c056 100644 --- a/grasp-audit/src/probe.rs +++ b/grasp-audit/src/probe.rs @@ -1472,53 +1472,42 @@ pub async fn run_probe_with_options( Some(body) => { let fetched_refs = parse_refs(&body); - // Fetch all state events for this repo_id from the relay. - // The relay only serves state events authorized by a selected - // coordinate's resolved reciprocal component. - let state_filter = Filter::new().kind(Kind::RepoState).custom_tag( - nostr_sdk::prelude::SingleLetterTag::LOWERCASE_D, - ann_id.clone(), - ); - let state_events = client + // Resolve authority and state from the same repository-scoped fetch. + let filter = Filter::new() + .kinds([Kind::GitRepoAnnouncement, Kind::RepoState]) + .custom_tag(SingleLetterTag::LOWERCASE_D, ann_id.clone()); + let expected = match client .client() - .fetch_events(state_filter) + .fetch_events(filter) .timeout( deadline .saturating_duration_since(Instant::now()) .min(Duration::from_secs(5)), ) .await - .unwrap_or_default(); - - if state_events.is_empty() { - checks.push(ProbeCheck { - name: ProbeCheckName::GitRefsMatchState.as_str(), - passed: false, - skipped: false, - duration_ms: 0, - detail: None, - error: Some( - "no kind:30618 state events found for this repo".to_string(), - ), - }); - } else { - let expected = expected_state_refs(state_events.iter()); - - let mismatches = state::ref_mismatches(&expected, &fetched_refs); - - checks.push(ProbeCheck { - name: ProbeCheckName::GitRefsMatchState.as_str(), - passed: mismatches.is_empty(), - skipped: false, - duration_ms: 0, - detail: None, - error: if mismatches.is_empty() { - None - } else { - Some(mismatches.join("; ")) - }, - }); - } + { + Ok(events) => { + state::scoped_state_refs(events.iter(), ev.pubkey, &ann_id) + } + Err(error) => Err(format!( + "failed to fetch repository authority and state: {error}" + )), + }; + let error = match expected { + Ok(expected) => { + let mismatches = state::ref_mismatches(&expected, &fetched_refs); + (!mismatches.is_empty()).then(|| mismatches.join("; ")) + } + Err(error) => Some(error), + }; + checks.push(ProbeCheck { + name: ProbeCheckName::GitRefsMatchState.as_str(), + passed: error.is_none(), + skipped: false, + duration_ms: 0, + detail: None, + error, + }); } } } diff --git a/grasp-audit/src/probe/state.rs b/grasp-audit/src/probe/state.rs index 9efb622..73f60d0 100644 --- a/grasp-audit/src/probe/state.rs +++ b/grasp-audit/src/probe/state.rs @@ -1,6 +1,159 @@ -//! Complete branch and tag comparison for audit probes. +//! Read-side authority and ref comparison for the selected probe coordinate. -use std::collections::{BTreeMap, HashMap}; +use nostr_sdk::prelude::*; +use std::collections::{BTreeMap, HashMap, HashSet}; + +fn preferred(left: &Event, right: &Event) -> std::cmp::Ordering { + left.created_at + .cmp(&right.created_at) + .then_with(|| right.id.cmp(&left.id)) +} + +struct Roles { + active_author: bool, + maintainers: HashSet, + leads: HashSet, +} + +fn roles(event: &Event) -> Roles { + let mut result = Roles { + active_author: true, + maintainers: HashSet::new(), + leads: HashSet::new(), + }; + let mut indexed = false; + let mut explicit_self = false; + for tag in event.tags.iter() { + let values = tag.as_slice(); + if !matches!(values.first().map(String::as_str), Some("M" | "m" | "o")) { + continue; + } + indexed = true; + let Some(key) = values.get(1).and_then(|key| PublicKey::from_hex(key).ok()) else { + continue; + }; + explicit_self |= key == event.pubkey; + let boundaries = &values[2..]; + // Only a final numeric start (or an untimed role) is active. + // Ended, deferred and malformed roles grant no current authority. + let active = (boundaries.is_empty() || boundaries.len() % 2 == 1) + && boundaries.iter().all(|value| value.parse::().is_ok()); + if values[0] != "o" && active { + result.maintainers.insert(key); + if values[0] == "M" { + result.leads.insert(key); + } + } + } + if indexed { + result.active_author = !explicit_self || result.maintainers.contains(&event.pubkey); + } else if let Some(tag) = event + .tags + .iter() + .find(|tag| tag.as_slice()[0] == "maintainers") + { + result.maintainers.extend( + tag.as_slice()[1..] + .iter() + .filter_map(|key| PublicKey::from_hex(key).ok()), + ); + } + result +} + +/// Resolve current state publishers from the selected coordinate, not every +/// repository that happens to share its identifier. +fn state_authors( + announcements: &HashMap, + selected: PublicKey, +) -> Result, String> { + let graph: HashMap<_, _> = announcements + .iter() + .map(|(key, event)| (*key, roles(event))) + .collect(); + let mut root = selected; + let mut followed_lead = false; + let mut visited = HashSet::new(); + loop { + if !visited.insert(root) { + return Err("repository authority has a lead cycle".into()); + } + let node = graph + .get(&root) + .ok_or("repository authority announcement is missing")?; + match node.leads.len() { + 0 if !followed_lead && node.active_author => break, + 0 => { + return Err( + "repository authority has an inactive author or incomplete lead path".into(), + ) + } + 1 => { + let lead = *node.leads.iter().next().expect("one lead"); + if lead == root { + break; + } + root = lead; + followed_lead = true; + } + _ => return Err("repository authority has ambiguous active leads".into()), + } + } + + let mut confirmed = HashSet::from([root]); + loop { + let mut additions = HashSet::new(); + for member in &confirmed { + for candidate in &graph[member].maintainers { + if confirmed.contains(candidate) { + continue; + } + if let Some(node) = graph.get(candidate) { + if node.active_author && !node.maintainers.is_disjoint(&confirmed) { + additions.insert(*candidate); + } + } + } + } + if additions.is_empty() { + return Ok(confirmed); + } + confirmed.extend(additions); + } +} + +pub(super) fn scoped_state_refs<'a>( + events: impl IntoIterator, + selected: PublicKey, + identifier: &str, +) -> Result, String> { + let events: Vec<_> = events + .into_iter() + .filter(|event| { + matches!(event.kind, Kind::GitRepoAnnouncement | Kind::RepoState) + && event.tags.identifier().as_deref() == Some(identifier) + && event.verify().is_ok() + }) + .collect(); + let mut announcements: HashMap = HashMap::new(); + for event in events + .iter() + .copied() + .filter(|event| event.kind == Kind::GitRepoAnnouncement) + { + let current = announcements.entry(event.pubkey).or_insert(event); + if preferred(event, current).is_gt() { + *current = event; + } + } + let authors = state_authors(&announcements, selected)?; + let winner = events + .into_iter() + .filter(|event| event.kind == Kind::RepoState && authors.contains(&event.pubkey)) + .max_by(|left, right| preferred(left, right)) + .ok_or("no authorized kind:30618 state events found for this repository")?; + Ok(super::expected_state_refs([winner])) +} /// Compare only branches and unpeeled tags. HEAD, protocol capabilities, /// peeled annotated tags and refs/nostr are not repository-state refs. @@ -40,6 +193,211 @@ pub(super) fn ref_mismatches( mod tests { use super::*; + fn announcement(keys: &Keys, tags: Vec, time: u64) -> Event { + EventBuilder::new(Kind::GitRepoAnnouncement, "") + .tags(std::iter::once(Tag::identifier("repo")).chain(tags)) + .custom_created_at(Timestamp::from_secs(time)) + .finalize(keys) + .unwrap() + } + + fn role(letter: &str, keys: &Keys, history: &[&str]) -> Tag { + Tag::custom( + letter, + std::iter::once(keys.public_key().to_hex()) + .chain(history.iter().map(|value| value.to_string())), + ) + } + + fn state(keys: &Keys, time: u64, hash: &str) -> Event { + EventBuilder::new(Kind::RepoState, "") + .tags([ + Tag::identifier("repo"), + Tag::custom("refs/heads/main", [hash]), + ]) + .custom_created_at(Timestamp::from_secs(time)) + .finalize(keys) + .unwrap() + } + + fn selected(events: &[Event], owner: &Keys) -> Result { + scoped_state_refs(events, owner.public_key(), "repo") + .map(|refs| refs["refs/heads/main"].clone()) + } + + #[test] + fn unrelated_and_unaccepted_authors_cannot_supply_state() { + let owner = Keys::generate(); + let unrelated = Keys::generate(); + let invited = Keys::generate(); + let events = vec![ + announcement(&owner, vec![role("m", &invited, &[])], 1), + announcement(&unrelated, vec![], 1), + announcement(&invited, vec![], 1), + state(&owner, 2, "owner"), + state(&unrelated, 3, "unrelated"), + state(&invited, 4, "unaccepted"), + ]; + assert_eq!(selected(&events, &owner).unwrap(), "owner"); + } + + #[test] + fn reciprocal_membership_reaches_transitive_maintainers() { + let owner = Keys::generate(); + let member = Keys::generate(); + let third = Keys::generate(); + for legacy in [false, true] { + let tags = |keys: &[&Keys]| { + if legacy { + vec![Tag::custom( + "maintainers", + keys.iter().map(|key| key.public_key().to_hex()), + )] + } else { + keys.iter().map(|key| role("m", key, &[])).collect() + } + }; + let events = vec![ + announcement(&owner, tags(&[&member]), 1), + announcement(&member, tags(&[&owner, &third]), 1), + announcement(&third, tags(&[&member]), 1), + state(&owner, 2, "owner"), + state(&third, 3, "third"), + ]; + assert_eq!(selected(&events, &owner).unwrap(), "third"); + } + } + + #[test] + fn indexed_roles_override_legacy_and_exclude_inactive_or_moderator_authors() { + let owner = Keys::generate(); + let member = Keys::generate(); + for self_role in [ + role("m", &member, &["1", "2"]), + role("m", &member, &["1", "defer"]), + role("m", &member, &["bad"]), + role("o", &member, &[]), + ] { + let events = vec![ + announcement(&owner, vec![role("m", &member, &[])], 1), + announcement(&member, vec![role("m", &owner, &[]), self_role], 1), + state(&owner, 2, "owner"), + state(&member, 3, "inactive"), + ]; + assert_eq!(selected(&events, &owner).unwrap(), "owner"); + } + let events = vec![ + announcement( + &owner, + vec![ + role("o", &member, &[]), + Tag::custom("maintainers", [member.public_key().to_hex()]), + ], + 1, + ), + announcement(&member, vec![role("m", &owner, &[])], 1), + state(&owner, 2, "owner"), + state(&member, 3, "moderator"), + ]; + assert_eq!(selected(&events, &owner).unwrap(), "owner"); + } + + #[test] + fn lead_redirect_does_not_restore_removed_author() { + let lead = Keys::generate(); + let former = Keys::generate(); + let events = vec![ + announcement(&lead, vec![role("M", &lead, &[])], 1), + announcement( + &former, + vec![role("M", &lead, &[]), role("m", &former, &["1", "2"])], + 1, + ), + state(&lead, 2, "lead"), + state(&former, 3, "removed"), + ]; + assert_eq!(selected(&events, &former).unwrap(), "lead"); + } + + #[test] + fn missing_ambiguous_and_cyclic_lead_paths_fail_closed() { + let owner = Keys::generate(); + let lead = Keys::generate(); + let mut events = vec![ + announcement(&owner, vec![role("M", &lead, &[])], 1), + state(&owner, 2, "owner"), + ]; + assert!(selected(&events, &owner).unwrap_err().contains("missing")); + events.push(announcement(&lead, vec![], 1)); + assert!(selected(&events, &owner) + .unwrap_err() + .contains("incomplete")); + events.pop(); + events.push(announcement(&lead, vec![role("M", &owner, &[])], 1)); + assert!(selected(&events, &owner).unwrap_err().contains("cycle")); + events[0] = announcement( + &owner, + vec![role("M", &owner, &[]), role("M", &lead, &[])], + 1, + ); + assert!(selected(&events, &owner).unwrap_err().contains("ambiguous")); + } + + #[test] + fn latest_announcement_controls_membership_including_same_second_ties() { + let owner = Keys::generate(); + let member = Keys::generate(); + let mut announcements = [ + announcement(&owner, vec![], 1), + announcement(&owner, vec![role("m", &member, &[])], 1), + ]; + announcements.sort_by_key(|event| event.id); + let grants = roles(&announcements[0]) + .maintainers + .contains(&member.public_key()); + for reverse in [false, true] { + let mut events = announcements.to_vec(); + if reverse { + events.reverse(); + } + events.extend([ + announcement(&member, vec![role("m", &owner, &[])], 1), + state(&owner, 2, "owner"), + state(&member, 3, "member"), + ]); + assert_eq!( + selected(&events, &owner).unwrap(), + if grants { "member" } else { "owner" } + ); + events.push(announcement(&owner, vec![], 4)); + assert_eq!(selected(&events, &owner).unwrap(), "owner"); + } + } + + #[test] + fn unrelated_identifiers_and_invalid_signatures_are_ignored() { + let owner = Keys::generate(); + let mut invalid = state(&owner, 100, "invalid"); + invalid.content = "tampered".into(); + let other = EventBuilder::new(Kind::RepoState, "") + .tags([ + Tag::identifier("other"), + Tag::custom("refs/heads/main", ["other"]), + ]) + .custom_created_at(Timestamp::from_secs(100)) + .finalize(&owner) + .unwrap(); + let events = vec![ + announcement(&owner, vec![], 1), + state(&owner, 2, "owner"), + invalid, + other, + ]; + assert_eq!(selected(&events, &owner).unwrap(), "owner"); + assert!(scoped_state_refs(&events[..1], owner.public_key(), "repo").is_err()); + assert!(scoped_state_refs(&events[1..], owner.public_key(), "repo").is_err()); + } + #[test] fn ref_comparison_requires_complete_branch_and_tag_set() { let expected: HashMap = HashMap::from([ diff --git a/tests/audit_probe_state.rs b/tests/audit_probe_state.rs new file mode 100644 index 0000000..49ec172 --- /dev/null +++ b/tests/audit_probe_state.rs @@ -0,0 +1,115 @@ +mod common; + +use common::{publish_served_audit_repo_with_state, wait_for_event_served, TestRelay}; +use grasp_audit::{probe::run_probe, AuditClient, AuditConfig, DETERMINISTIC_COMMIT_HASH}; +use nostr_sdk::prelude::*; +use std::time::Duration; + +#[tokio::test] +async fn probe_scopes_same_identifier_states_and_reports_extra_refs() { + let relay = TestRelay::start().await; + let client = AuditClient::new(relay.url(), AuditConfig::shared()) + .await + .unwrap(); + let (announcement, identifier, initial_state) = + publish_served_audit_repo_with_state(&client, "probe-scope").await; + let owner_path = relay.git_data_path().join( + ngit_grasp::nostr::events::RepositoryAnnouncement::from_event(announcement.clone()) + .unwrap() + .repo_path(), + ); + let unrelated = Keys::generate(); + let foreign_announcement = + common::create_repo_announcement(&unrelated, &[&relay.domain()], &identifier); + // Ensure the probe selects the owner's announcement, while the unrelated + // repository supplies the newest state for this identifier. + let foreign_announcement = EventBuilder::new(foreign_announcement.kind, "") + .tags(foreign_announcement.tags.iter().cloned()) + .custom_created_at(Timestamp::from_secs(announcement.created_at.as_secs() - 1)) + .finalize(&unrelated) + .unwrap(); + let foreign_path = relay.git_data_path().join( + ngit_grasp::nostr::events::RepositoryAnnouncement::from_event(foreign_announcement.clone()) + .unwrap() + .repo_path(), + ); + std::fs::create_dir_all(foreign_path.parent().unwrap()).unwrap(); + let cloned = grasp_audit::git_command() + .args(["clone", "--bare"]) + .arg(&owner_path) + .arg(&foreign_path) + .output() + .unwrap(); + assert!( + cloned.status.success(), + "{}", + String::from_utf8_lossy(&cloned.stderr) + ); + client + .send_event(foreign_announcement.clone()) + .await + .unwrap(); + let foreign_state = EventBuilder::new(Kind::RepoState, "") + .tags([ + Tag::identifier(&identifier), + Tag::custom("refs/heads/foreign", [DETERMINISTIC_COMMIT_HASH]), + Tag::custom("HEAD", ["ref: refs/heads/foreign"]), + ]) + .custom_created_at(Timestamp::from_secs(initial_state.created_at.as_secs() + 1)) + .finalize(&unrelated) + .unwrap(); + client.send_event(foreign_state.clone()).await.unwrap(); + for event in [&foreign_announcement, &foreign_state, &initial_state] { + wait_for_event_served(relay.url(), &event.id, Duration::from_secs(10)) + .await + .unwrap(); + } + + let report = run_probe(relay.url(), None, true, 10, 30).await; + let selection = report + .checks + .iter() + .find(|check| check.name == "serves_latest_announcement") + .unwrap(); + assert_eq!( + selection.detail.as_deref(), + Some( + format!( + "{}/{}", + client.public_key().to_bech32().unwrap(), + identifier + ) + .as_str() + ) + ); + let check = report + .checks + .iter() + .find(|check| check.name == "git_refs_match_state") + .unwrap(); + assert!(check.passed, "{check:?}"); + + for name in ["refs/heads/stale", "refs/tags/stale"] { + let result = grasp_audit::git_command() + .arg("--git-dir") + .arg(&owner_path) + .args(["update-ref", name, DETERMINISTIC_COMMIT_HASH]) + .output() + .unwrap(); + assert!(result.status.success()); + } + let report = run_probe(relay.url(), None, true, 10, 30).await; + let check = report + .checks + .iter() + .find(|check| check.name == "git_refs_match_state") + .unwrap(); + assert!(!check.passed); + let error = check.error.as_deref().unwrap(); + assert!( + error.contains("refs/heads/stale: unexpected ref"), + "{error}" + ); + assert!(error.contains("refs/tags/stale: unexpected ref"), "{error}"); + relay.stop().await; +}