diff --git a/docs/explanation/architecture.md b/docs/explanation/architecture.md index 348f652..09d804f 100644 --- a/docs/explanation/architecture.md +++ b/docs/explanation/architecture.md @@ -283,6 +283,22 @@ pub struct RepositoryState { ... } #### Maintainer Membership Model +Announcements carry maintainer listings in two formats: + +- **Indexed role tags** (`M` lead, `m` co-maintainer). A tag may record + history as alternating start/end timestamps; it is currently active when + it has fewer than four elements or an odd number of elements. Ended + entries are ignored entirely - the history is only used to conclude a + role has *ended*, never to grant authority for a past period. The lead / + co-maintainer distinction carries no meaning for this service. A pubkey + may appear in one `M` and one `m` tag to record a role transition and is + a maintainer while either entry is active; a second tag under the same + letter rejects the announcement. An author who appears in no role tag is + implicitly a maintainer for the repository's entire history. +- **Deprecated `maintainers` tag** (fallback). Ignored when `M`/`m` tags + are present. Without any listing tags the author is the sole maintainer. + A `u` (subordinate fork) tag has no effect on maintainership. + Authorization follows a reciprocal membership rule, computed per owner by [`compute_membership`](src/git/authorization.rs): @@ -293,6 +309,10 @@ Authorization follows a reciprocal membership rule, computed per owner by authoritative. - Confirmation is a fixpoint, so maintainers listed by other confirmed maintainers are reached recursively. +- The acknowledging announcement must assert an active role for its own + author: an ended `M`/`m` self-entry means the pubkey has left, which + takes precedence over assignments in other announcements. An author who + appears in no role tag implicitly asserts maintainership. - The owner is always a confirmed maintainer of their own repository: announcing a repository in their namespace is what creates it on this service. diff --git a/docs/explanation/decisions.md b/docs/explanation/decisions.md index d7a3d65..31aa965 100644 --- a/docs/explanation/decisions.md +++ b/docs/explanation/decisions.md @@ -235,3 +235,28 @@ NIP-34 maintainers model refined in nips commit `781590b`). (`reapply_stored`) so a newly confirmed maintainer's latest state re-points the owner's repository without another push. +### Indexed role tags (`M`/`m`) + +- `M` (lead) and `m` (co-maintainer) tags are the primary maintainer + listing (per the model clarified in nips commit `986edd1`); when present + the deprecated `maintainers` tag is ignored per NIP-34 (and parsed as + empty). The role distinction carries no meaning for this service: both + collapse into one maintainer set. +- Role tags may record history as alternating start/end timestamps; a tag + is currently active when it has fewer than four elements or an odd number + of elements. Ended entries are ignored entirely: role history is only + used to conclude that a pubkey is *no longer* a maintainer, never to + grant time-scoped retroactive authority over historic events. The + NIP's owner-first precedence for conflicting past-role records is + therefore unused. +- A pubkey may appear in one `M` and one `m` tag to record a role + transition and is a maintainer while either entry is active. A second + tag under the same letter is malformed and rejects the announcement. +- An announcement using role tags acknowledges its author via an active + self-entry, or implicitly: an author who appears in no role tag is a + maintainer for the repository's entire history. Only an ended self-entry + means the member left, which takes precedence over assignments in other + announcements. +- A `u` (subordinate fork) tag has no effect on maintainership: the author + of a role-less announcement asserts maintainership with or without it. + diff --git a/src/git/authorization.rs b/src/git/authorization.rs index 6897741..500fc68 100644 --- a/src/git/authorization.rs +++ b/src/git/authorization.rs @@ -331,7 +331,11 @@ pub struct RepoMembership { /// announcement for the same identifier lists back a pubkey that is already /// a confirmed maintainer (reciprocal acknowledgment). Confirmation is /// evaluated as a fixpoint, so maintainers listed by other confirmed -/// maintainers are reached recursively. +/// maintainers are reached recursively. The acknowledging announcement must +/// also assert an active role for its own author: an ended `M`/`m` +/// self-entry means the pubkey has left, which takes precedence over +/// assignments in other announcements. An author who appears in no role tag +/// implicitly asserts maintainership for the repository's entire history. /// /// The owner is always a confirmed maintainer of their own repository: /// announcing a repository in their namespace is what creates it on this @@ -369,6 +373,9 @@ pub fn compute_membership( let Some(candidate_announcement) = find(candidate) else { continue; // invited: no announcement of their own yet }; + if !candidate_announcement.author_role_active() { + continue; // left: an ended self-role takes precedence + } let lists_back = candidate_announcement .listed_maintainers() .iter() @@ -383,6 +390,9 @@ pub fn compute_membership( let invited = listed .into_iter() .filter(|pubkey| !confirmed.contains(pubkey)) + // A pubkey whose own announcement shows it left is not + // invited. + .filter(|pubkey| !find(pubkey).is_some_and(|a| a.author_has_left())) .collect(); return RepoMembership { state_maintainers: confirmed, @@ -1401,6 +1411,183 @@ mod tests { assert!(membership.invited.contains(&hex(&bob))); } + /// Build an announcement using NIP-34 indexed role tags. + /// Each entry is (tag name, values) where values start with the pubkey + /// followed by optional history timestamps. + fn create_role_announcement( + keys: &Keys, + identifier: &str, + roles: &[(&str, Vec)], + ) -> Event { + let mut tags = vec![Tag::custom("d", vec![identifier.to_string()])]; + for (name, values) in roles { + tags.push(Tag::custom(*name, values.clone())); + } + tags.push(Tag::custom( + "clone", + vec!["https://example.com/test.git".to_string()], + )); + tags.push(Tag::custom("relays", vec!["wss://example.com".to_string()])); + + EventBuilder::new(Kind::GitRepoAnnouncement, "Test repo") + .tags(tags) + .finalize(keys) + .unwrap() + } + + #[test] + fn test_role_tag_maintainer_confirmed_when_reciprocal() { + let alice = create_test_keys(); + let bob = create_test_keys(); + let identifier = "test-repo"; + + let announcements = vec![ + parse(create_role_announcement( + &alice, + identifier, + &[("M", vec![hex(&alice)]), ("m", vec![hex(&bob)])], + )), + parse(create_role_announcement( + &bob, + identifier, + &[("M", vec![hex(&alice)]), ("m", vec![hex(&bob)])], + )), + ]; + + let membership = compute_membership(&announcements, &hex(&alice), identifier); + assert!(membership.state_maintainers.contains(&hex(&bob))); + } + + #[test] + fn test_ended_role_is_no_longer_maintainer() { + let alice = create_test_keys(); + let bob = create_test_keys(); + let identifier = "test-repo"; + + // Alice's announcement records Bob's co-maintainer role as ended + let alice_announcement = create_role_announcement( + &alice, + identifier, + &[ + ("M", vec![hex(&alice)]), + ( + "m", + vec![hex(&bob), "0".to_string(), "1700000000".to_string()], + ), + ], + ); + // Bob still lists himself and Alice as active + let bob_announcement = create_role_announcement( + &bob, + identifier, + &[("M", vec![hex(&alice)]), ("m", vec![hex(&bob)])], + ); + + let announcements = vec![parse(alice_announcement), parse(bob_announcement)]; + let membership = compute_membership(&announcements, &hex(&alice), identifier); + assert!(!membership.state_maintainers.contains(&hex(&bob))); + assert!( + !membership.invited.contains(&hex(&bob)), + "an ended role must not be treated as an open invitation" + ); + } + + #[test] + fn test_returning_maintainer_history_is_active() { + let alice = create_test_keys(); + let bob = create_test_keys(); + let identifier = "test-repo"; + + // Odd element count: added, removed, then added again + let alice_announcement = create_role_announcement( + &alice, + identifier, + &[ + ("M", vec![hex(&alice)]), + ( + "m", + vec![ + hex(&bob), + "100".to_string(), + "200".to_string(), + "300".to_string(), + ], + ), + ], + ); + let bob_announcement = create_role_announcement( + &bob, + identifier, + &[("M", vec![hex(&alice)]), ("m", vec![hex(&bob)])], + ); + + let announcements = vec![parse(alice_announcement), parse(bob_announcement)]; + let membership = compute_membership(&announcements, &hex(&alice), identifier); + assert!(membership.state_maintainers.contains(&hex(&bob))); + } + + #[test] + fn test_self_leave_takes_precedence() { + let alice = create_test_keys(); + let bob = create_test_keys(); + let identifier = "test-repo"; + + // Alice still lists Bob as active + let alice_announcement = create_role_announcement( + &alice, + identifier, + &[("M", vec![hex(&alice)]), ("m", vec![hex(&bob)])], + ); + // Bob ended his own role: he has left + let bob_announcement = create_role_announcement( + &bob, + identifier, + &[ + ("M", vec![hex(&alice)]), + ( + "m", + vec![hex(&bob), "0".to_string(), "1700000000".to_string()], + ), + ], + ); + + let announcements = vec![parse(alice_announcement), parse(bob_announcement)]; + let membership = compute_membership(&announcements, &hex(&alice), identifier); + assert!(!membership.state_maintainers.contains(&hex(&bob))); + assert!(!membership.invited.contains(&hex(&bob))); + } + + #[test] + fn test_subordinate_fork_marker_does_not_block_acceptance() { + let alice = create_test_keys(); + let bob = create_test_keys(); + let identifier = "test-repo"; + + // Alice invites Bob; Bob's announcement carries a `u` (subordinate + // fork) tag. Per NIP-34 `u` has no effect on maintainership, so + // Bob's listing of Alice still acknowledges the invitation. + let alice_announcement = create_announcement_event(&alice, identifier, &[&bob]); + let mut tags = vec![ + Tag::custom("d", vec![identifier.to_string()]), + Tag::custom("maintainers", vec![hex(&alice)]), + Tag::custom("u", vec![format!("30617:{}:{}", hex(&alice), identifier)]), + ]; + tags.push(Tag::custom( + "clone", + vec!["https://example.com/test.git".to_string()], + )); + tags.push(Tag::custom("relays", vec!["wss://example.com".to_string()])); + let bob_announcement = EventBuilder::new(Kind::GitRepoAnnouncement, "Test repo") + .tags(tags) + .finalize(&bob) + .unwrap(); + + let announcements = vec![parse(alice_announcement), parse(bob_announcement)]; + let membership = compute_membership(&announcements, &hex(&alice), identifier); + assert!(membership.state_maintainers.contains(&hex(&bob))); + assert!(!membership.invited.contains(&hex(&bob))); + } + #[test] fn test_validate_push_refs_success() { let alice = create_test_keys(); diff --git a/src/nostr/events.rs b/src/nostr/events.rs index 9137216..6657e03 100644 --- a/src/nostr/events.rs +++ b/src/nostr/events.rs @@ -26,7 +26,22 @@ pub struct RepositoryAnnouncement { pub clone_urls: Vec, pub relays: Vec, pub web_urls: Vec, + /// Pubkeys from the deprecated `maintainers` tag. Empty when the + /// announcement uses `M`/`m` role tags, which take precedence per NIP-34. pub maintainers: Vec, + /// Currently-active maintainer pubkeys from NIP-34 `M`/`m` role tags; + /// `None` when the announcement contains no `M`/`m` tags. + /// + /// The lead (`M`) / co-maintainer (`m`) distinction carries no meaning + /// for this service, so both collapse into one maintainer set. Entries + /// whose role history shows the role has ended are excluded entirely: an + /// ended role means the pubkey is no longer a maintainer, and historic + /// listing never grants authority for a past period. + pub role_maintainers: Option>, + /// Whether any role tag names the author, active or ended. Per NIP-34 an + /// author without a self-entry is implicitly a maintainer for the + /// repository's entire history. + author_has_role_entry: bool, } impl RepositoryAnnouncement { @@ -107,19 +122,63 @@ impl RepositoryAnnouncement { }) .collect(); - // Extract maintainers from "maintainers" tag per NIP-34 + // Extract maintainer pubkeys from NIP-34 `M`/`m` role tags + // Format: ["M"|"m", "", "", "", ...] + // A role tag lists a pubkey followed by optional alternating start/end + // history timestamps; it is currently active when it has fewer than + // four elements or an odd number of elements. Ended entries are + // ignored entirely. A pubkey may appear in one `M` and one `m` tag to + // record a transition between the roles and remains a maintainer + // while either entry is active; a second tag under the same letter is + // malformed and rejects the announcement. + let author_hex = event.pubkey.to_hex(); + let mut role_tags_present = false; + let mut author_has_role_entry = false; + let mut role_active: Vec = Vec::new(); + let mut seen_role_entries: Vec<(String, String)> = Vec::new(); + for tag in event.tags.iter() { + let slice = tag.as_slice(); + let Some(kind) = slice.first() else { continue }; + if kind != "M" && kind != "m" { + continue; + } + role_tags_present = true; + let Some(pubkey) = slice.get(1).filter(|value| !value.is_empty()) else { + continue; + }; + let entry = (kind.clone(), pubkey.clone()); + if seen_role_entries.contains(&entry) { + return Err(anyhow!("duplicate `{kind}` role tag for pubkey {pubkey}")); + } + seen_role_entries.push(entry); + if *pubkey == author_hex { + author_has_role_entry = true; + } + let active = slice.len() < 4 || slice.len() % 2 == 1; + if active && !role_active.contains(pubkey) { + role_active.push(pubkey.clone()); + } + } + let role_maintainers = role_tags_present.then_some(role_active); + + // Extract maintainers from the deprecated "maintainers" tag per NIP-34 // Format: ["maintainers", "", "", ...] - let maintainers = event - .tags - .iter() - .find(|tag| tag.as_slice().first().map(|s| s.as_str()) == Some("maintainers")) - .map(|tag| { - tag.as_slice()[1..] // Skip the "maintainers" tag name - .iter() - .map(|s| s.to_string()) - .collect() - }) - .unwrap_or_default(); + // Ignored entirely when `M` or `m` role tags are present. + let maintainers = if role_tags_present { + Vec::new() + } else { + event + .tags + .iter() + .find(|tag| tag.as_slice().first().map(|s| s.as_str()) == Some("maintainers")) + .map(|tag| { + tag.as_slice()[1..] // Skip the "maintainers" tag name + .iter() + .map(|s| s.to_string()) + .collect() + }) + .unwrap_or_default() + }; Ok(RepositoryAnnouncement { event, @@ -130,6 +189,8 @@ impl RepositoryAnnouncement { relays, web_urls, maintainers, + role_maintainers, + author_has_role_entry, }) } @@ -142,8 +203,12 @@ impl RepositoryAnnouncement { /// listing alone makes none of their events authoritative. pub fn listed_maintainers(&self) -> Vec { let author = self.event.pubkey.to_hex(); + let source = self + .role_maintainers + .as_deref() + .unwrap_or(&self.maintainers); let mut listed: Vec = Vec::new(); - for pubkey in &self.maintainers { + for pubkey in source { if *pubkey != author && !listed.contains(pubkey) { listed.push(pubkey.clone()); } @@ -151,6 +216,34 @@ impl RepositoryAnnouncement { listed } + /// Whether this announcement asserts an active maintainer role for its + /// own author. + /// + /// With `M`/`m` role tags the author is active via an active self-entry, + /// or implicitly: per NIP-34 an author who appears in no role tag is a + /// maintainer for the repository's entire history. Only an ended + /// self-entry means the author has left, which takes precedence over + /// assignments in other announcements. In the deprecated format the + /// author always asserts maintainership; a `u` (subordinate fork) tag + /// has no effect on maintainership. + pub fn author_role_active(&self) -> bool { + match &self.role_maintainers { + Some(active) => { + !self.author_has_role_entry || active.contains(&self.event.pubkey.to_hex()) + } + None => true, + } + } + + /// Whether this announcement shows its author has left: a role tag names + /// the author but none of their entries is active. + /// + /// An author absent from all role tags has *not* left - they are + /// implicitly a maintainer for the repository's entire history. + pub fn author_has_left(&self) -> bool { + self.role_maintainers.is_some() && !self.author_role_active() + } + /// Check if this announcement lists the given domain in clone URLs /// /// Compares parsed host and port semantics, not substrings, so a URL such @@ -808,6 +901,200 @@ mod tests { ); } + fn role_announcement(keys: &Keys, tags: Vec<(&str, Vec)>) -> RepositoryAnnouncement { + use nostr_sdk::prelude::Tag; + + let mut event_tags = vec![Tag::custom("d", vec!["test-repo".to_string()])]; + for (name, values) in tags { + event_tags.push(Tag::custom(name, values)); + } + let event = EventBuilder::new(Kind::GitRepoAnnouncement, "Test repository") + .tags(event_tags) + .finalize(keys) + .unwrap(); + RepositoryAnnouncement::from_event(event).unwrap() + } + + #[test] + fn test_role_tag_activeness_by_element_count() { + let keys = create_test_keys(); + let author = keys.public_key().to_hex(); + let other = create_test_keys().public_key().to_hex(); + + // (history values after the pubkey, expected active) + // Tag element counts: 2 -> active, 3 -> active, 4 -> ended, + // 5 -> active, 6 -> ended. + let cases: Vec<(Vec<&str>, bool)> = vec![ + (vec![], true), + (vec!["100"], true), + (vec!["100", "200"], false), + (vec!["100", "200", "300"], true), + (vec!["100", "200", "300", "400"], false), + ]; + + for (history, expected_active) in cases { + let mut values = vec![other.clone()]; + values.extend(history.iter().map(|s| s.to_string())); + let announcement = + role_announcement(&keys, vec![("M", vec![author.clone()]), ("m", values)]); + assert_eq!( + announcement.listed_maintainers().contains(&other), + expected_active, + "history {history:?} expected active={expected_active}" + ); + } + } + + #[test] + fn test_lead_and_co_maintainer_roles_are_equivalent() { + let keys = create_test_keys(); + let author = keys.public_key().to_hex(); + let listed_as_lead = create_test_keys().public_key().to_hex(); + let listed_as_co = create_test_keys().public_key().to_hex(); + + let announcement = role_announcement( + &keys, + vec![ + ("M", vec![author.clone()]), + ("M", vec![listed_as_lead.clone()]), + ("m", vec![listed_as_co.clone()]), + ], + ); + + let listed = announcement.listed_maintainers(); + assert!(listed.contains(&listed_as_lead)); + assert!(listed.contains(&listed_as_co)); + assert!( + !listed.contains(&author), + "author is excluded from the listed set" + ); + assert!(announcement.author_role_active()); + } + + #[test] + fn test_maintainers_tag_ignored_when_role_tags_present() { + let keys = create_test_keys(); + let author = keys.public_key().to_hex(); + let legacy_listed = create_test_keys().public_key().to_hex(); + + let announcement = role_announcement( + &keys, + vec![ + ("M", vec![author]), + ("maintainers", vec![legacy_listed.clone()]), + ], + ); + + assert!(announcement.maintainers.is_empty()); + assert!(!announcement.listed_maintainers().contains(&legacy_listed)); + } + + #[test] + fn test_author_leave_via_ended_self_role() { + let keys = create_test_keys(); + let author = keys.public_key().to_hex(); + let lead = create_test_keys().public_key().to_hex(); + + let announcement = role_announcement( + &keys, + vec![ + ("M", vec![lead.clone()]), + ("m", vec![author, "0".to_string(), "1700000000".to_string()]), + ], + ); + + assert!(!announcement.author_role_active()); + assert!(announcement.listed_maintainers().contains(&lead)); + } + + #[test] + fn test_u_tag_has_no_effect_on_maintainership() { + let keys = create_test_keys(); + let upstream = create_test_keys().public_key().to_hex(); + + // Deprecated format: the author implicitly asserts maintainership, + // with or without a `u` (subordinate fork) tag. + let plain = role_announcement(&keys, vec![("maintainers", vec![upstream.clone()])]); + assert!(plain.author_role_active()); + + let fork = role_announcement( + &keys, + vec![ + ("maintainers", vec![upstream.clone()]), + ("u", vec![format!("30617:{upstream}:test-repo")]), + ], + ); + assert!(fork.author_role_active()); + assert!(!fork.author_has_left()); + } + + #[test] + fn test_author_without_self_role_entry_is_implicitly_active() { + let keys = create_test_keys(); + let other = create_test_keys().public_key().to_hex(); + + // Role tags that do not name the author: the author is implicitly a + // maintainer for the repository's entire history. + let announcement = role_announcement(&keys, vec![("m", vec![other])]); + assert!(announcement.author_role_active()); + assert!(!announcement.author_has_left()); + } + + #[test] + fn test_role_transition_across_letters_is_active_while_either_entry_active() { + let keys = create_test_keys(); + let author = keys.public_key().to_hex(); + let demoted = create_test_keys().public_key().to_hex(); + let gone = create_test_keys().public_key().to_hex(); + + // One `M` and one `m` tag for the same pubkey record a transition + // between the roles: the pubkey is a maintainer while either entry + // is active, and no longer one when both have ended. + let announcement = role_announcement( + &keys, + vec![ + ("M", vec![author.clone()]), + ( + "M", + vec![demoted.clone(), "0".to_string(), "100".to_string()], + ), + ("m", vec![demoted.clone(), "100".to_string()]), + ("M", vec![gone.clone(), "0".to_string(), "100".to_string()]), + ( + "m", + vec![gone.clone(), "100".to_string(), "200".to_string()], + ), + ], + ); + + let listed = announcement.listed_maintainers(); + assert!(listed.contains(&demoted)); + assert!(!listed.contains(&gone)); + } + + #[test] + fn test_duplicate_same_letter_role_tag_rejects_announcement() { + use nostr_sdk::prelude::Tag; + + let keys = create_test_keys(); + let author = keys.public_key().to_hex(); + let other = create_test_keys().public_key().to_hex(); + + let event = EventBuilder::new(Kind::GitRepoAnnouncement, "Test repository") + .tags(vec![ + Tag::custom("d", vec!["test-repo".to_string()]), + Tag::custom("M", vec![author]), + Tag::custom("m", vec![other.clone(), "0".to_string(), "100".to_string()]), + Tag::custom("m", vec![other]), + ]) + .finalize(&keys) + .unwrap(); + + let result = RepositoryAnnouncement::from_event(event); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("duplicate")); + } + #[test] fn test_state_with_tags() { use nostr_sdk::prelude::Tag; diff --git a/src/nostr/policy/announcement.rs b/src/nostr/policy/announcement.rs index 7e33148..fb24c4b 100644 --- a/src/nostr/policy/announcement.rs +++ b/src/nostr/policy/announcement.rs @@ -175,13 +175,10 @@ impl AnnouncementPolicy { )) } }; - let current_maintainers: HashSet<&str> = - current.maintainers.iter().map(String::as_str).collect(); - let incoming_maintainers: HashSet<&str> = announcement - .maintainers - .iter() - .map(String::as_str) - .collect(); + let current_maintainers: HashSet = + current.listed_maintainers().into_iter().collect(); + let incoming_maintainers: HashSet = + announcement.listed_maintainers().into_iter().collect(); if current_maintainers != incoming_maintainers { tracing::debug!( identifier = %announcement.identifier, @@ -423,14 +420,19 @@ impl AnnouncementPolicy { Ok(()) } - /// Check if a pubkey is listed as a maintainer in any announcement for this identifier + /// Check if a pubkey is currently listed as a maintainer in any announcement + /// for this identifier /// - /// A pubkey is considered a maintainer if: + /// A pubkey qualifies if: /// 1. They are the owner (pubkey) of an accepted announcement with this identifier, OR - /// 2. They are listed in the maintainers tag of ANY announcement with this identifier + /// 2. ANY announcement with this identifier currently lists them as a maintainer + /// (active `M`/`m` role tags or the deprecated `maintainers` tag) /// /// This enables accepting announcements from maintainers even when they don't list - /// this GRASP server, for maintainer chain discovery and GRASP-02 sync. + /// this GRASP server, for maintainer chain discovery and GRASP-02 sync. It + /// deliberately includes *invited* pubkeys - their announcement is exactly how we + /// learn they accepted the role - but excludes pubkeys whose role history shows + /// the role has ended. /// /// Checks both the database (promoted announcements) and purgatory (announcements /// waiting for git data). This is necessary because a maintainer's announcement @@ -479,9 +481,9 @@ impl AnnouncementPolicy { return Ok(true); } - // Check if author is listed in the maintainers tag + // Check if the announcement currently lists the author as a maintainer if let Ok(announcement) = RepositoryAnnouncement::from_event((*event).clone()) { - if announcement.maintainers.contains(&author_hex) { + if announcement.listed_maintainers().contains(&author_hex) { return Ok(true); } } diff --git a/src/purgatory/promotion_hooks.rs b/src/purgatory/promotion_hooks.rs index 291afb5..9437017 100644 --- a/src/purgatory/promotion_hooks.rs +++ b/src/purgatory/promotion_hooks.rs @@ -150,14 +150,17 @@ impl PurgatoryPromotionHooks for NostrPurgatoryPromotionHooks { return; }; + // Every currently-listed maintainer, including invited ones whose + // announcements we still need to notice acceptance. + let listed_maintainers = announcement.listed_maintainers(); debug!( identifier = %announcement.identifier, event_id = %event.id, - maintainer_count = announcement.maintainers.len(), + maintainer_count = listed_maintainers.len(), "Owner announcement promoted from purgatory, checking hot cache for rejected maintainer announcements" ); - for maintainer_hex in &announcement.maintainers { + for maintainer_hex in &listed_maintainers { match PublicKey::from_hex(maintainer_hex) { Ok(maintainer_pubkey) => { let (event_ids, hot_events) = rejected_events_index.dependency_candidates( diff --git a/src/sync/discovery.rs b/src/sync/discovery.rs index b4d9183..e872e23 100644 --- a/src/sync/discovery.rs +++ b/src/sync/discovery.rs @@ -160,13 +160,16 @@ pub fn accepted_repository_authors<'a>( continue; } authors.insert(event.pubkey); - for maintainer in event - .tags - .iter() - .filter(|tag| tag.as_slice().first().map(String::as_str) == Some("maintainers")) + // Include every currently-listed maintainer (active `M`/`m` role tags + // or the deprecated maintainers fallback). Invited pubkeys are + // included deliberately: their announcements must be fetched so we + // notice when they accept the role. Ended roles are excluded. + if let Ok(announcement) = + crate::nostr::events::RepositoryAnnouncement::from_event(event.clone()) { authors.extend( - maintainer.as_slice()[1..] + announcement + .listed_maintainers() .iter() .filter_map(|value| PublicKey::from_hex(value).ok()), ); diff --git a/src/sync/mod.rs b/src/sync/mod.rs index de8dea9..67e6e13 100644 --- a/src/sync/mod.rs +++ b/src/sync/mod.rs @@ -6635,8 +6635,10 @@ impl SyncManager { let relay_url = format!("ws://{}", self.service_domain); let mut dependency_refetch_ids = HashSet::new(); let mut hot_dependency_events = Vec::new(); + // Walk every currently-listed maintainer, including invited ones: + // their announcements must be recovered so acceptance is noticed. let mut maintainer_queue: VecDeque = - announcement.maintainers.iter().cloned().collect(); + announcement.listed_maintainers().into_iter().collect(); let mut visited_maintainers = HashSet::new(); let mut dependency_relay_urls: HashSet = announcement.relays.iter().cloned().collect(); @@ -6703,8 +6705,8 @@ impl SyncManager { if let Ok(accepted_announcement) = crate::nostr::events::RepositoryAnnouncement::from_event(accepted_event) { + maintainer_queue.extend(accepted_announcement.listed_maintainers()); dependency_relay_urls.extend(accepted_announcement.relays); - maintainer_queue.extend(accepted_announcement.maintainers); } } } @@ -7409,16 +7411,19 @@ impl SyncManager { match RepositoryAnnouncement::from_event(event.clone()) { Ok(announcement) => { // Re-process rejected maintainer announcements - if !announcement.maintainers.is_empty() { + // Re-process rejected announcements from every + // currently-listed maintainer (invited included) + let listed_maintainers = announcement.listed_maintainers(); + if !listed_maintainers.is_empty() { tracing::debug!( event_id = %event.id, identifier = %announcement.identifier, - maintainer_count = announcement.maintainers.len(), + maintainer_count = listed_maintainers.len(), "Owner announcement accepted, checking for rejected maintainer announcements" ); // For each maintainer, invalidate and get their events - for maintainer_hex in &announcement.maintainers { + for maintainer_hex in &listed_maintainers { // Parse maintainer public key match PublicKey::from_hex(maintainer_hex) { Ok(maintainer_pubkey) => { diff --git a/tests/state_authorization.rs b/tests/state_authorization.rs index 06ad556..f6abffd 100644 --- a/tests/state_authorization.rs +++ b/tests/state_authorization.rs @@ -330,3 +330,109 @@ async fn test_reject_state_from_invited_maintainer() { relay.stop().await; } + +#[tokio::test] +async fn test_accept_state_from_role_tag_maintainer() { + let relay = TestRelay::start().await; + + let owner_keys = Keys::generate(); + let maintainer_keys = Keys::generate(); + + // Owner uses NIP-34 indexed role tags: M for themselves, m for the + // co-maintainer. + let announcement = EventBuilder::new(Kind::GitRepoAnnouncement, "") + .tags([ + Tag::custom("d", ["test-repo"]), + Tag::custom("clone", [format!("https://{}/test.git", relay.domain())]), + Tag::custom("relays", [relay.url()]), + Tag::custom("M", [owner_keys.public_key().to_hex()]), + Tag::custom("m", [maintainer_keys.public_key().to_hex()]), + ]) + .finalize(&owner_keys) + .unwrap(); + + // Reciprocal announcement acknowledging the role and listing the lead. + let reciprocal_announcement = EventBuilder::new(Kind::GitRepoAnnouncement, "") + .tags([ + Tag::custom("d", ["test-repo"]), + Tag::custom("clone", [format!("https://{}/test.git", relay.domain())]), + Tag::custom("relays", [relay.url()]), + Tag::custom("M", [owner_keys.public_key().to_hex()]), + Tag::custom("m", [maintainer_keys.public_key().to_hex()]), + ]) + .finalize(&maintainer_keys) + .unwrap(); + + let client = Client::default(); + client.add_relay(relay.url()).await.unwrap(); + client.connect().await; + + client.send_event(&announcement).await.unwrap(); + client.send_event(&reciprocal_announcement).await.unwrap(); + tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; + + assert!( + !state_event_rejected_as_unauthorized(&client, &maintainer_keys).await, + "state event from a confirmed role-tag co-maintainer must not be rejected as unauthorized" + ); + + relay.stop().await; +} + +#[tokio::test] +async fn test_reject_state_from_removed_maintainer() { + let relay = TestRelay::start().await; + + let owner_keys = Keys::generate(); + let maintainer_keys = Keys::generate(); + + // The owner's announcement records the co-maintainer role as ended + // (four elements: pubkey plus start/end boundary timestamps). + let announcement = EventBuilder::new(Kind::GitRepoAnnouncement, "") + .tags([ + Tag::custom("d", ["test-repo"]), + Tag::custom("clone", [format!("https://{}/test.git", relay.domain())]), + Tag::custom("relays", [relay.url()]), + Tag::custom("M", [owner_keys.public_key().to_hex()]), + Tag::custom( + "m", + [ + maintainer_keys.public_key().to_hex(), + "0".to_string(), + "1700000000".to_string(), + ], + ), + ]) + .finalize(&owner_keys) + .unwrap(); + + // Even a reciprocal announcement cannot restore an ended role. It points + // at another host so it does not create the maintainer's own hosted repo + // here; with the role ended it also no longer qualifies for the + // maintainer exception, so the relay may reject it outright. + let reciprocal_announcement = EventBuilder::new(Kind::GitRepoAnnouncement, "") + .tags([ + Tag::custom("d", ["test-repo"]), + Tag::custom("clone", ["https://elsewhere.example/test.git".to_string()]), + Tag::custom("relays", ["wss://elsewhere.example".to_string()]), + Tag::custom("M", [owner_keys.public_key().to_hex()]), + Tag::custom("m", [maintainer_keys.public_key().to_hex()]), + ]) + .finalize(&maintainer_keys) + .unwrap(); + + let client = Client::default(); + client.add_relay(relay.url()).await.unwrap(); + client.connect().await; + + client.send_event(&announcement).await.unwrap(); + let _ = client.send_event(&reciprocal_announcement).await; + tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; + + assert!( + state_event_rejected_as_unauthorized(&client, &maintainer_keys).await, + "state event from a maintainer whose role has ended must be rejected as unauthorized" + ); + + relay.stop().await; +}