diff --git a/crates/buzz-cli/src/commands/repos.rs b/crates/buzz-cli/src/commands/repos.rs index 0f570df1aa..608d495055 100644 --- a/crates/buzz-cli/src/commands/repos.rs +++ b/crates/buzz-cli/src/commands/repos.rs @@ -83,27 +83,36 @@ fn build_protection_tag( Tag::parse(values).map_err(tag_error) } -enum ProtectionChange { - Set(Box), - Remove(String), +enum RepoChange { + SetProtection(Box), + RemoveProtection(String), + /// Bind (or rebind) the repo to a channel: replaces every existing + /// `buzz-channel` tag with exactly one carrying the validated UUID. + BindChannel(String), } fn build_updated_repo_announcement( existing: &Event, - change: ProtectionChange, + change: RepoChange, ) -> Result { let repo_id = repo_id_from_event(existing)?; - let (pattern, replacement) = match change { - ProtectionChange::Set(tag) => { + // What to strip beyond `auth` (always stripped), and what to append. + let (removed_pattern, removed_channel, replacement) = match change { + RepoChange::SetProtection(tag) => { let pattern = protection_pattern(&tag) .ok_or_else(|| CliError::Other("replacement is not a protection tag".into()))? .to_string(); - (pattern, Some(*tag)) + (Some(pattern), false, Some(*tag)) } - ProtectionChange::Remove(pattern) => { + RepoChange::RemoveProtection(pattern) => { RefPattern::parse(&pattern) .map_err(|error| CliError::Usage(format!("invalid ref pattern: {error}")))?; - (pattern, None) + (Some(pattern), false, None) + } + RepoChange::BindChannel(channel) => { + crate::validate::validate_uuid(&channel)?; + let tag = Tag::parse(["buzz-channel", channel.as_str()]).map_err(tag_error)?; + (None, true, Some(tag)) } }; @@ -111,7 +120,13 @@ fn build_updated_repo_announcement( .tags .iter() .filter(|tag| { - !has_tag_name(tag, "auth") && protection_pattern(tag) != Some(pattern.as_str()) + if has_tag_name(tag, "auth") { + return false; + } + if removed_channel && has_tag_name(tag, "buzz-channel") { + return false; + } + removed_pattern.is_none() || protection_pattern(tag) != removed_pattern.as_deref() }) .cloned() .collect(); @@ -199,21 +214,30 @@ async fn submit_repo_update(client: &BuzzClient, builder: EventBuilder) -> Resul Ok(()) } -pub async fn cmd_create_repo( - client: &BuzzClient, +/// Build the kind:30617 announcement for `repos create`, including the +/// `buzz-channel` binding when requested. +/// +/// Pure (no I/O) so the emitted tags are unit-testable. Exactly one +/// validated `buzz-channel` tag is appended — the tag is the git ACL +/// (issue #3527: without it the relay 404s every clone/fetch/push), so the +/// UUID is shape-validated here and its existence/membership is the relay's +/// authority at git-access time, same posture as `repos bind`. +#[allow(clippy::too_many_arguments)] +fn build_create_announcement( repo_id: &str, name: Option<&str>, description: Option<&str>, clone_urls: &[String], web_url: Option<&str>, relays: &[String], -) -> Result<(), CliError> { + channel: Option<&str>, +) -> Result { validate_repo_id(repo_id)?; let clone_refs: Vec<&str> = clone_urls.iter().map(|s| s.as_str()).collect(); let relay_refs: Vec<&str> = relays.iter().map(|s| s.as_str()).collect(); - let builder = buzz_sdk::build_repo_announcement( + let mut builder = buzz_sdk::build_repo_announcement( repo_id, name, description, @@ -223,6 +247,33 @@ pub async fn cmd_create_repo( ) .map_err(|e| CliError::Other(format!("build_repo_announcement failed: {e}")))?; + if let Some(channel) = channel { + crate::validate::validate_uuid(channel)?; + builder = builder.tag(Tag::parse(["buzz-channel", channel]).map_err(tag_error)?); + } + Ok(builder) +} + +#[allow(clippy::too_many_arguments)] +pub async fn cmd_create_repo( + client: &BuzzClient, + repo_id: &str, + name: Option<&str>, + description: Option<&str>, + clone_urls: &[String], + web_url: Option<&str>, + relays: &[String], + channel: Option<&str>, +) -> Result<(), CliError> { + let builder = build_create_announcement( + repo_id, + name, + description, + clone_urls, + web_url, + relays, + channel, + )?; let event = client.sign_event(builder)?; let resp = client.submit_event(event).await?; println!("{resp}"); @@ -320,7 +371,8 @@ async fn cmd_protect_set( require_patch, )?; let event = current_repo(client, repo_id).await?; - let builder = build_updated_repo_announcement(&event, ProtectionChange::Set(Box::new(tag)))?; + let builder = + build_updated_repo_announcement(&event, RepoChange::SetProtection(Box::new(tag)))?; submit_repo_update(client, builder).await } @@ -341,8 +393,27 @@ async fn cmd_protect_remove( "repository {repo_id:?} has no protection rule for {ref_pattern:?}" ))); } + let builder = build_updated_repo_announcement( + &event, + RepoChange::RemoveProtection(ref_pattern.to_string()), + )?; + submit_repo_update(client, builder).await +} + +/// Bind (or rebind) a repository to a channel — the fix path for issue +/// #3527's permanently-404 repos. Publishes a read-modify-write update of +/// the caller's own kind:30617 with exactly one `buzz-channel` tag; all +/// other metadata (protections, name, description, future tags) is +/// preserved by the same machinery `repos protect` uses. +/// +/// The UUID is validated for *shape* only — deliberately. Channel existence +/// and the caller's membership are the relay's authority at git-access +/// time; a CLI-side network pre-check would just be TOCTOU with extra +/// latency. +async fn cmd_bind_repo(client: &BuzzClient, repo_id: &str, channel: &str) -> Result<(), CliError> { + let event = current_repo(client, repo_id).await?; let builder = - build_updated_repo_announcement(&event, ProtectionChange::Remove(ref_pattern.to_string()))?; + build_updated_repo_announcement(&event, RepoChange::BindChannel(channel.to_string()))?; submit_repo_update(client, builder).await } @@ -356,6 +427,7 @@ pub async fn dispatch(cmd: crate::ReposCmd, client: &BuzzClient) -> Result<(), C clone_urls, web, relays, + channel, } => { cmd_create_repo( client, @@ -365,11 +437,13 @@ pub async fn dispatch(cmd: crate::ReposCmd, client: &BuzzClient) -> Result<(), C &clone_urls, web.as_deref(), &relays, + channel.as_deref(), ) .await } ReposCmd::Get { id, owner } => cmd_get_repo(client, &id, owner.as_deref()).await, ReposCmd::List { owner, limit } => cmd_list_repos(client, owner.as_deref(), limit).await, + ReposCmd::Bind { id, channel } => cmd_bind_repo(client, &id, &channel).await, ReposCmd::Protect(command) => match command { ReposProtectCmd::List { id } => cmd_protect_list(client, &id).await, ReposProtectCmd::Set { @@ -403,8 +477,8 @@ mod tests { use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; use super::{ - build_protection_tag, build_updated_repo_announcement, protection_rules_json, - validate_write_response, ProtectionChange, + build_create_announcement, build_protection_tag, build_updated_repo_announcement, + protection_rules_json, validate_write_response, RepoChange, }; fn signed_repo(tags: Vec, content: &str, created_at: u64) -> nostr::Event { @@ -439,7 +513,7 @@ mod tests { let updated = build_updated_repo_announcement( &existing, - ProtectionChange::Set(Box::new(replacement)), + RepoChange::SetProtection(Box::new(replacement)), ) .expect("build update") .sign_with_keys(&Keys::generate()) @@ -501,7 +575,7 @@ mod tests { let updated = build_updated_repo_announcement( &existing, - ProtectionChange::Remove("refs/heads/main".into()), + RepoChange::RemoveProtection("refs/heads/main".into()), ) .expect("build removal") .sign_with_keys(&Keys::generate()) @@ -538,7 +612,7 @@ mod tests { let error = build_updated_repo_announcement( &existing, - ProtectionChange::Set(Box::new(replacement)), + RepoChange::SetProtection(Box::new(replacement)), ) .expect_err("malformed existing rule must fail closed"); @@ -564,7 +638,7 @@ mod tests { let error = build_updated_repo_announcement( &existing, - ProtectionChange::Set(Box::new(replacement)), + RepoChange::SetProtection(Box::new(replacement)), ) .expect_err("the 51st rule must be rejected"); @@ -615,6 +689,145 @@ mod tests { .is_some_and(|error| error.contains("needs pattern + at least one rule"))); } + #[test] + fn bind_channel_replaces_duplicates_and_preserves_everything_else() { + let channel = uuid::Uuid::new_v4().to_string(); + let existing = signed_repo( + vec![ + tag(&["d", "demo"]), + tag(&["name", "Demo"]), + // Two stale bindings — e.g. from a buggy or vanilla client. + tag(&["buzz-channel", "old-and-broken"]), + tag(&["buzz-channel", &uuid::Uuid::new_v4().to_string()]), + tag(&["auth", &"a".repeat(64), "kind=30617", &"b".repeat(128)]), + tag(&["buzz-protect", "refs/heads/main", "push:admin"]), + tag(&["future-metadata", "preserve-me"]), + ], + "repository content", + 100, + ); + + let updated = + build_updated_repo_announcement(&existing, RepoChange::BindChannel(channel.clone())) + .expect("build bind update") + .sign_with_keys(&Keys::generate()) + .expect("sign bind update"); + + assert_eq!(updated.content, "repository content"); + assert_eq!(updated.created_at.as_secs(), 101); + // Exactly one binding remains, and it is the requested one. + let bindings: Vec<_> = updated + .tags + .iter() + .filter(|tag| tag.as_slice().first().map(String::as_str) == Some("buzz-channel")) + .collect(); + assert_eq!(bindings.len(), 1); + assert_eq!(bindings[0].as_slice(), ["buzz-channel", channel.as_str()]); + // Auth stripped (relay re-stamps); everything else preserved. + assert!(!updated + .tags + .iter() + .any(|tag| tag.as_slice().first().map(String::as_str) == Some("auth"))); + assert!(updated + .tags + .iter() + .any(|tag| tag.as_slice() == ["buzz-protect", "refs/heads/main", "push:admin"])); + assert!(updated + .tags + .iter() + .any(|tag| tag.as_slice() == ["future-metadata", "preserve-me"])); + assert!(updated + .tags + .iter() + .any(|tag| tag.as_slice() == ["name", "Demo"])); + } + + #[test] + fn bind_channel_adds_binding_to_unbound_repo() { + let channel = uuid::Uuid::new_v4().to_string(); + let existing = signed_repo(vec![tag(&["d", "demo"])], "", 10); + + let updated = + build_updated_repo_announcement(&existing, RepoChange::BindChannel(channel.clone())) + .expect("build bind update") + .sign_with_keys(&Keys::generate()) + .expect("sign bind update"); + + assert!(updated + .tags + .iter() + .any(|tag| tag.as_slice() == ["buzz-channel", channel.as_str()])); + } + + #[test] + fn bind_channel_rejects_malformed_uuid() { + let existing = signed_repo(vec![tag(&["d", "demo"])], "", 10); + + let error = + build_updated_repo_announcement(&existing, RepoChange::BindChannel("nope".into())) + .expect_err("malformed channel id must not build an update"); + + assert!(matches!(error, crate::error::CliError::Usage(_))); + } + + /// Issue #3527: `repos create --channel` must emit exactly one + /// `buzz-channel` tag so the primary create command stops producing + /// repos the relay 404s forever. + #[test] + fn create_with_channel_emits_exactly_one_binding_tag() { + let channel = uuid::Uuid::new_v4().to_string(); + let event = build_create_announcement( + "demo", + Some("Demo"), + None, + &["https://relay.example/git/owner/demo".to_string()], + None, + &[], + Some(&channel), + ) + .expect("build create announcement") + .sign_with_keys(&Keys::generate()) + .expect("sign create announcement"); + + assert_eq!(event.kind, Kind::Custom(30617)); + let bindings: Vec<_> = event + .tags + .iter() + .filter(|tag| tag.as_slice().first().map(String::as_str) == Some("buzz-channel")) + .collect(); + assert_eq!(bindings.len(), 1, "exactly one buzz-channel tag"); + assert_eq!(bindings[0].as_slice(), ["buzz-channel", channel.as_str()]); + // The standard metadata still rides along. + assert!(event.tags.iter().any(|tag| tag.as_slice() == ["d", "demo"])); + assert!(event + .tags + .iter() + .any(|tag| tag.as_slice() == ["name", "Demo"])); + } + + #[test] + fn create_without_channel_emits_no_binding_tag() { + let event = build_create_announcement("demo", None, None, &[], None, &[], None) + .expect("build create announcement") + .sign_with_keys(&Keys::generate()) + .expect("sign create announcement"); + + assert!( + !event + .tags + .iter() + .any(|tag| tag.as_slice().first().map(String::as_str) == Some("buzz-channel")), + "no --channel means no binding tag (vanilla NIP-34 stays possible)" + ); + } + + #[test] + fn create_rejects_malformed_channel_uuid() { + let error = build_create_announcement("demo", None, None, &[], None, &[], Some("nope")) + .expect_err("malformed channel id must not build an announcement"); + assert!(matches!(error, crate::error::CliError::Usage(_))); + } + #[test] fn duplicate_write_response_is_a_conflict() { let error = validate_write_response( diff --git a/crates/buzz-cli/src/lib.rs b/crates/buzz-cli/src/lib.rs index df02c65be9..02a58a618e 100644 --- a/crates/buzz-cli/src/lib.rs +++ b/crates/buzz-cli/src/lib.rs @@ -1129,6 +1129,11 @@ pub enum ReposCmd { /// Preferred Nostr relay(s) for repo discovery — can be specified multiple times #[arg(long = "nostr-relay")] relays: Vec, + /// Channel UUID to bind the repo to. The `buzz-channel` tag is the + /// git ACL: without it the relay 404s every clone/fetch/push until + /// the author runs `buzz repos bind` (issue #3527). + #[arg(long)] + channel: Option, }, /// Get a repository announcement Get { @@ -1148,6 +1153,20 @@ pub enum ReposCmd { #[arg(long)] limit: Option, }, + /// Bind (or rebind) one of your repositories to a channel. + /// + /// The `buzz-channel` tag on the announcement is the git ACL: the relay + /// authorizes clone/fetch/push by membership in the bound channel. A + /// repo announced without it (e.g. by a vanilla NIP-34 client) returns + /// 404 for everyone until its author binds it here. + Bind { + /// Repository identifier (d-tag). + #[arg(long)] + id: String, + /// Channel UUID to bind. Replaces any existing binding. + #[arg(long)] + channel: String, + }, /// Manage branch and tag protection rules on one of your repositories. #[command(subcommand)] Protect(ReposProtectCmd), @@ -1991,7 +2010,7 @@ mod tests { ); assert_eq!( names(&cmd, "repos"), - vec!["create", "get", "list", "protect"] + vec!["bind", "create", "get", "list", "protect"] ); let repos = cmd .get_subcommands() @@ -2054,7 +2073,7 @@ mod tests { ("patches", 4), ("pr", 5), ("reactions", 3), - ("repos", 4), + ("repos", 5), ("social", 7), ("upload", 1), ("users", 5), diff --git a/crates/buzz-core/src/git_perms.rs b/crates/buzz-core/src/git_perms.rs index 53acd704b9..391781163b 100644 --- a/crates/buzz-core/src/git_perms.rs +++ b/crates/buzz-core/src/git_perms.rs @@ -15,6 +15,30 @@ use crate::channel::MemberRole; use std::fmt; +/// Machine-readable token prefixing the push-policy denial for a kind:30617 +/// announcement with no `buzz-channel` binding. +/// +/// This is a **declared cross-component contract**, not a log string. Known +/// consumers switch on it: +/// - relay `api/git/policy.rs` — produces [`GIT_NO_CHANNEL_BINDING_BODY`] +/// - desktop `src-tauri/commands/project_git_workflow.rs` — merge-failure +/// classifier maps it to a structured `no_channel_binding` error code +/// - desktop `src/features/projects/lib/projectBranchErrors.ts` — dialog +/// copy matcher (TS re-types the literal; its test pins the value) +pub const GIT_NO_CHANNEL_BINDING_TOKEN: &str = "no_channel_binding"; + +/// Full push-policy denial body for an unbound repository. +/// +/// Format: `: `. The trailing prose deliberately +/// repeats the token's meaning because desktops already in the field match +/// the exact phrase `no channel binding` (spaces, not underscores — the +/// token alone would NOT satisfy that matcher). Do not "fix" the redundancy: +/// removing the phrase silently breaks every shipped desktop, and removing +/// the token breaks the structured consumers above. A relay-side test pins +/// both matchers. +pub const GIT_NO_CHANNEL_BINDING_BODY: &str = + "no_channel_binding: repository has no channel binding"; + /// Maximum number of `buzz-protect` tags per repo. pub const MAX_PROTECTION_RULES: usize = 50; /// Maximum character length of a ref pattern. diff --git a/crates/buzz-relay/src/api/git/binding.rs b/crates/buzz-relay/src/api/git/binding.rs new file mode 100644 index 0000000000..7ee0eccb23 --- /dev/null +++ b/crates/buzz-relay/src/api/git/binding.rs @@ -0,0 +1,128 @@ +//! Repo → channel binding resolution, shared by the read gate and push policy. +//! +//! The `buzz-channel` tag on a kind:30617 announcement IS the git ACL: the +//! read gate (SEC-005, `transport::authorize_git_read`) and the push policy +//! endpoint (`policy::hook_callback`) both authorize against membership in +//! the bound channel. Before this module they each parsed the tag with their +//! own code that agreed only by coincidence; the resolver makes the +//! agreement structural. +//! +//! # First-tag, fail-closed semantics +//! +//! Only the *first* `buzz-channel` tag is considered, and it must carry a +//! valid UUID. A malformed first binding resolves to [`RepoBinding::Broken`] +//! even if a later duplicate tag is valid — an ambiguous announcement must +//! fail closed, not silently resolve to whichever duplicate happens to +//! parse. If this ever became "find the first *parseable* tag", an author +//! who can append a second `buzz-channel` tag would pick the channel. +//! +//! # What this deliberately does NOT do +//! +//! No DB access. A well-formed UUID that names a nonexistent or deleted +//! channel still resolves to [`RepoBinding::Bound`]; each gate's own +//! membership lookup then denies (`get_member_role` joins +//! `channels … deleted_at IS NULL`, so a dead channel is indistinguishable +//! from a non-member — the info-leak-safe posture). Likewise each gate keeps +//! its own archived-channel policy: push denies on archived channels, read +//! does not, and this resolver must not unify that asymmetry as a side +//! effect. + +use uuid::Uuid; + +/// How a kind:30617 announcement binds (or fails to bind) a channel. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RepoBinding { + /// No `buzz-channel` tag at all. The announcement author (the only + /// identity that can rebind — 30617 is keyed by `(author, d)`) may be + /// offered remediation; everyone else gets the generic denial. + NotBound, + /// First `buzz-channel` tag carries a valid UUID. + Bound(Uuid), + /// First `buzz-channel` tag exists but its value is not a UUID. + /// Fail closed with the generic denial — never remediation, which + /// would leak that the repo exists. + Broken, +} + +/// Resolve the channel binding of a kind:30617 announcement from its tags. +pub fn resolve_repo_binding(event: &nostr::Event) -> RepoBinding { + let Some(first) = event + .tags + .iter() + .find(|t| t.as_slice().first().map(String::as_str) == Some("buzz-channel")) + else { + return RepoBinding::NotBound; + }; + match first.as_slice().get(1).map(|v| Uuid::parse_str(v)) { + Some(Ok(id)) => RepoBinding::Bound(id), + _ => RepoBinding::Broken, + } +} + +#[cfg(test)] +mod tests { + use nostr::{EventBuilder, Keys, Kind, Tag}; + + use super::{resolve_repo_binding, RepoBinding}; + + fn announcement(tags: Vec) -> nostr::Event { + EventBuilder::new(Kind::Custom(30617), "") + .tags(tags) + .sign_with_keys(&Keys::generate()) + .expect("sign 30617") + } + + #[test] + fn extracts_valid_uuid() { + let ch = uuid::Uuid::new_v4(); + let event = announcement(vec![ + Tag::parse(["d", "repo"]).unwrap(), + Tag::parse(["buzz-channel", &ch.to_string()]).unwrap(), + ]); + assert_eq!(resolve_repo_binding(&event), RepoBinding::Bound(ch)); + } + + #[test] + fn absent_tag_is_not_bound() { + let event = announcement(vec![Tag::parse(["d", "repo"]).unwrap()]); + assert_eq!(resolve_repo_binding(&event), RepoBinding::NotBound); + } + + #[test] + fn malformed_and_empty_values_are_broken_not_absent() { + let malformed = announcement(vec![ + Tag::parse(["d", "repo"]).unwrap(), + Tag::parse(["buzz-channel", "not-a-uuid"]).unwrap(), + ]); + assert_eq!(resolve_repo_binding(&malformed), RepoBinding::Broken); + + let empty = announcement(vec![ + Tag::parse(["d", "repo"]).unwrap(), + Tag::parse(["buzz-channel"]).unwrap(), + ]); + assert_eq!(resolve_repo_binding(&empty), RepoBinding::Broken); + } + + #[test] + fn fails_closed_on_ambiguous_duplicate_bindings() { + let ch = uuid::Uuid::new_v4(); + let other = uuid::Uuid::new_v4(); + + // Malformed first + valid second: the ambiguity denies; the valid + // duplicate must NOT win, or the duplicate picks the channel. + let malformed_first = announcement(vec![ + Tag::parse(["d", "repo"]).unwrap(), + Tag::parse(["buzz-channel", "not-a-uuid"]).unwrap(), + Tag::parse(["buzz-channel", &ch.to_string()]).unwrap(), + ]); + assert_eq!(resolve_repo_binding(&malformed_first), RepoBinding::Broken); + + // Valid first + different second: first wins deterministically. + let valid_first = announcement(vec![ + Tag::parse(["d", "repo"]).unwrap(), + Tag::parse(["buzz-channel", &ch.to_string()]).unwrap(), + Tag::parse(["buzz-channel", &other.to_string()]).unwrap(), + ]); + assert_eq!(resolve_repo_binding(&valid_first), RepoBinding::Bound(ch)); + } +} diff --git a/crates/buzz-relay/src/api/git/mod.rs b/crates/buzz-relay/src/api/git/mod.rs index ab0510fbeb..dd69d7dc36 100644 --- a/crates/buzz-relay/src/api/git/mod.rs +++ b/crates/buzz-relay/src/api/git/mod.rs @@ -22,6 +22,7 @@ use tower_http::limit::RequestBodyLimitLayer; use crate::state::AppState; +pub mod binding; pub mod cas_publish; pub mod hook; pub mod hydrate; diff --git a/crates/buzz-relay/src/api/git/policy.rs b/crates/buzz-relay/src/api/git/policy.rs index fd6c4fb688..32d63f4600 100644 --- a/crates/buzz-relay/src/api/git/policy.rs +++ b/crates/buzz-relay/src/api/git/policy.rs @@ -42,7 +42,10 @@ use tracing::{error, warn}; use uuid::Uuid; use buzz_core::channel::MemberRole; -use buzz_core::git_perms::{evaluate_push, parse_protection_tags, Denial, RefUpdate, UpdateKind}; +use buzz_core::git_perms::{ + evaluate_push, parse_protection_tags, Denial, RefUpdate, UpdateKind, + GIT_NO_CHANNEL_BINDING_BODY, +}; use buzz_db::EventQuery; use crate::state::AppState; @@ -297,12 +300,29 @@ pub async fn hook_policy_check( } }; - // 6. Resolve channel and check archived state (applies to ALL pushers including owner). - let channel_id = tags - .iter() - .find(|t| t.first().map(|s| s.as_str()) == Some("buzz-channel")) - .and_then(|t| t.get(1)) - .and_then(|id| Uuid::parse_str(id).ok()); + // 6. Resolve channel binding via the shared resolver (same first-tag, + // fail-closed semantics as the read gate) and check archived state + // (applies to ALL pushers including owner). + // + // `Broken` denies HERE, before owner resolution: a malformed or + // ambiguous first binding fails closed for *everyone*, exactly like the + // read gate. Letting it fall through as "unbound" would hand the owner + // short-circuit below a push path through a binding the read gate + // refuses to honor — the tri-state exists precisely so Broken and + // NotBound cannot collapse. Only genuinely-NotBound repos proceed, and + // only they may earn the remediation-token denial. + let channel_id = match crate::api::git::binding::resolve_repo_binding(&repo_event.event) { + crate::api::git::binding::RepoBinding::Bound(id) => Some(id), + crate::api::git::binding::RepoBinding::NotBound => None, + crate::api::git::binding::RepoBinding::Broken => { + warn!(repo = %req.repo_id, "hook callback: broken buzz-channel binding"); + // Deliberately NOT the no_channel_binding token body: the + // remediation contract is NotBound-only. A broken binding is + // ambiguity, and ambiguity gets a generic denial (matching the + // read gate's posture for the same announcement). + return (StatusCode::FORBIDDEN, "invalid channel binding").into_response(); + } + }; if let Some(ch_id) = channel_id { match state.db.get_channel(community, ch_id).await { @@ -350,7 +370,10 @@ pub async fn hook_policy_check( match channel_id { None => { warn!(repo = %req.repo_id, "hook callback: no buzz-channel binding"); - return (StatusCode::FORBIDDEN, "no channel binding").into_response(); + // Declared cross-component contract — see the const docs in + // buzz-core::git_perms for who consumes the token and why + // the body also repeats the legacy phrase. + return (StatusCode::FORBIDDEN, GIT_NO_CHANNEL_BINDING_BODY).into_response(); } Some(ch_id) => { match state @@ -482,6 +505,30 @@ mod tests { assert!(!verify_hmac(b"wrong-secret", &req)); } + /// Deploy-skew guard for the unbound-repo deny body. The token + /// (`no_channel_binding`, underscores) and the legacy phrase + /// (`no channel binding`, spaces) do NOT contain each other, so the body + /// must carry both: the token for structured consumers (Desktop's merge + /// classifier and dialog matcher), the phrase for desktops already in + /// the field that prose-match it. Relay ships continuously and Desktop + /// on release cadence — dropping the phrase strands every old desktop + /// on a new relay. Asserted against the shared consts, not re-typed + /// literals, so the const and this test cannot drift apart separately. + #[test] + fn no_channel_binding_body_satisfies_old_and_new_matchers() { + assert!( + GIT_NO_CHANNEL_BINDING_BODY.starts_with(&format!( + "{}: ", + buzz_core::git_perms::GIT_NO_CHANNEL_BINDING_TOKEN + )), + "new structured consumers match the token prefix" + ); + assert!( + GIT_NO_CHANNEL_BINDING_BODY.contains("no channel binding"), + "shipped desktops prose-match this exact phrase (spaces, not underscores)" + ); + } + #[test] fn hmac_tampered_repo_id_rejected() { let secret = b"test-secret"; @@ -772,4 +819,169 @@ printf '%s' "$HMAC_INPUT" | openssl dgst -sha256 -hmac "{secret}" -hex 2>/dev/nu "Single-ref HMAC mismatch!\n Rust: {rust_sig}\n Bash: {bash_sig}" ); } + + // ── hook_policy_check binding gate (requires Postgres) ────────────── + + const TEST_DB_URL: &str = "postgres://buzz:buzz_dev@localhost:5432/buzz"; // sadscan:disable np.postgres.1 + + async fn policy_test_state() -> Arc { + let mut config = crate::config::Config::from_env().expect("default config loads"); + config.require_relay_membership = false; + config.redis_url = "redis://127.0.0.1:1".to_string(); + config.database_url = std::env::var("BUZZ_TEST_DATABASE_URL") + .or_else(|_| std::env::var("DATABASE_URL")) + .unwrap_or_else(|_| TEST_DB_URL.to_string()); + let pool = sqlx::PgPool::connect(&config.database_url) + .await + .expect("connect test DB"); + let db = buzz_db::Db::from_pool(pool.clone()); + let redis_pool = deadpool_redis::Config::from_url(&config.redis_url) + .create_pool(Some(deadpool_redis::Runtime::Tokio1)) + .expect("redis pool"); + let pubsub = Arc::new( + buzz_pubsub::PubSubManager::new(&config.redis_url, redis_pool.clone()) + .await + .expect("pubsub manager"), + ); + let audit = buzz_audit::AuditService::new(pool.clone()); + let auth = buzz_auth::AuthService::new(config.auth.clone()); + let search = buzz_search::SearchService::new(pool.clone()); + let workflow_engine = Arc::new(buzz_workflow::WorkflowEngine::new( + db.clone(), + buzz_workflow::WorkflowConfig::default(), + )); + let media_storage = buzz_media::MediaStorage::new(&config.media).expect("media storage"); + let (state, _audit_shutdown) = AppState::new( + config, + db, + redis_pool, + audit, + pubsub, + auth, + search, + workflow_engine, + nostr::Keys::generate(), + media_storage, + ); + Arc::new(state) + } + + /// Announce `repo_id` with the given tags, then push to it as its own + /// announcement author and return the response. + async fn owner_push_response( + state: &Arc, + community: buzz_core::CommunityId, + keys: &nostr::Keys, + repo_id: &str, + binding_tags: Vec, + ) -> axum::response::Response { + use nostr::{EventBuilder, Kind, Tag}; + + let mut tags = vec![Tag::parse(["d", repo_id]).unwrap()]; + tags.extend(binding_tags); + let event = EventBuilder::new(Kind::Custom(30617), "") + .tags(tags) + .sign_with_keys(keys) + .expect("sign 30617"); + state + .db + .insert_event(community, &event, None) + .await + .expect("insert 30617"); + + let owner_hex = keys.public_key().to_hex(); + let mut req = HookCallbackRequest { + repo_id: repo_id.to_string(), + repo_owner: owner_hex.clone(), + community_id: community.as_uuid().to_string(), + pusher_pubkey: owner_hex, + ref_updates: vec![HookRefUpdate { + old_oid: "0".repeat(40), + new_oid: "2".repeat(40), + ref_name: "refs/heads/main".to_string(), + is_ancestor: false, + }], + timestamp: SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_secs(), + signature: String::new(), + }; + let secret = state.config.git_hook_hmac_secret.clone(); + sign_request(&mut req, secret.as_bytes()); + hook_policy_check(State(Arc::clone(state)), Json(req)).await + } + + async fn body_string(response: axum::response::Response) -> (StatusCode, String) { + let status = response.status(); + let bytes = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .expect("read body"); + (status, String::from_utf8(bytes.to_vec()).expect("utf-8")) + } + + /// The tri-state trap the resolver exists to prevent: a broken (malformed + /// or ambiguous-first) binding must fail closed for EVERYONE on push — + /// including the announcement author — *before* the owner short-circuit + /// grants `MemberRole::Owner`. Collapsing `Broken` into "unbound" hands + /// the owner a push path through a binding the read gate refuses to + /// honor. The remediation token stays reserved for genuinely NotBound. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn push_gate_denies_owner_through_broken_binding() { + use nostr::{Keys, Tag}; + + let state = policy_test_state().await; + let host = format!("policy-{}.example", uuid::Uuid::new_v4().simple()); + let community = state + .db + .ensure_configured_community(&host) + .await + .expect("community") + .id; + let keys = Keys::generate(); + + // Malformed first + valid-looking second: the ambiguity must deny, + // and the parseable duplicate must not rescue the push. + let response = owner_push_response( + &state, + community, + &keys, + &format!("repo-{}", uuid::Uuid::new_v4().simple()), + vec![ + Tag::parse(["buzz-channel", "not-a-uuid"]).unwrap(), + Tag::parse(["buzz-channel", &uuid::Uuid::new_v4().to_string()]).unwrap(), + ], + ) + .await; + let (status, body) = body_string(response).await; + assert_eq!(status, StatusCode::FORBIDDEN); + assert_eq!( + body, "invalid channel binding", + "owner pushing through a broken binding must be denied generically" + ); + assert!( + !body.contains(buzz_core::git_perms::GIT_NO_CHANNEL_BINDING_TOKEN), + "remediation token is NotBound-only; Broken must never earn it" + ); + + // Control: the same owner pushing a genuinely NEVER-BOUND repo is + // allowed (owner authority over an unbound announcement is the + // long-standing push semantics). This pins the denial above to + // Broken specifically, not to some broader regression. + let response = owner_push_response( + &state, + community, + &keys, + &format!("repo-{}", uuid::Uuid::new_v4().simple()), + vec![], + ) + .await; + let (status, body) = body_string(response).await; + assert_eq!( + status, + StatusCode::OK, + "owner push to a never-bound repo must remain allowed (got body: {body})" + ); + } } diff --git a/crates/buzz-relay/src/api/git/transport.rs b/crates/buzz-relay/src/api/git/transport.rs index df5bdd4c3e..11c4f6d35b 100644 --- a/crates/buzz-relay/src/api/git/transport.rs +++ b/crates/buzz-relay/src/api/git/transport.rs @@ -28,6 +28,7 @@ use tokio::process::Command; use tower_http::limit::RequestBodyLimitLayer; use tracing::{error, info, warn}; +use super::binding::{resolve_repo_binding, RepoBinding}; use super::cas_publish::{cas_publish, CasError, ParentState, PublishLimits}; use super::hook::install_hook; use super::hydrate::{ @@ -377,7 +378,15 @@ fn hydrate_error_to_response(owner: &str, repo: &str, err: HydrateError) -> Resp /// error all deny. There is deliberately **no repo-owner bypass**: an owner /// removed from the bound channel loses read access, which is the exact /// exploit shape this gate closes. Every denial is the same generic 404 as a -/// nonexistent repo so membership cannot be probed through the git endpoints. +/// nonexistent repo so membership cannot be probed through the git endpoints +/// — with exactly one carve-out: a **never-bound** repo read by its own +/// **announcement author** returns a 404 whose body tells the author how to +/// bind it (issue #3527: a vanilla NIP-34 client can announce without a +/// `buzz-channel` tag, and the repo then 404s forever with no explanation +/// for anyone). The author already knows the repo exists — they announced it +/// — so the remediation body leaks nothing, and only the author can rebind +/// (kind:30617 is keyed by `(author, d)`). A *broken* binding stays generic +/// even for the author: ambiguity fails closed. async fn authorize_git_read( db: &buzz_db::Db, community: buzz_core::CommunityId, @@ -415,9 +424,32 @@ async fn authorize_git_read( } }; - let Some(channel_id) = repo_bound_channel_id(&repo_event.event) else { - warn!(repo = %repo_name, "git read gate: missing/malformed buzz-channel binding (deny)"); - return Err(denied()); + let channel_id = match resolve_repo_binding(&repo_event.event) { + RepoBinding::Bound(id) => id, + RepoBinding::NotBound => { + // Remediation carve-out: author of a never-bound announcement. + // Status stays 404 — byte-identical to every other denial at the + // status level — so denial *class* is still unprobeable; only + // the body differs, and only for the one identity that already + // knows the repo exists. The body is a single verb-first line: + // Desktop error paths that keep one line keep the instruction. + if repo_event.event.pubkey == *caller { + warn!(repo = %repo_name, "git read gate: unbound repo read by its author (deny with remediation)"); + return Err(( + StatusCode::NOT_FOUND, + format!( + "run: buzz repos bind --id {repo_name} --channel — repository {repo_name:?} has no channel binding, so the relay cannot authorize access" + ), + ) + .into_response()); + } + warn!(repo = %repo_name, "git read gate: missing buzz-channel binding (deny)"); + return Err(denied()); + } + RepoBinding::Broken => { + warn!(repo = %repo_name, "git read gate: malformed buzz-channel binding (deny)"); + return Err(denied()); + } }; match db @@ -433,24 +465,6 @@ async fn authorize_git_read( } } -/// Extract the `buzz-channel` UUID from a kind:30617 announcement. -/// -/// First-tag semantics, matching the push policy endpoint: only the *first* -/// `buzz-channel` tag is considered, and it must carry a valid UUID. A -/// malformed first binding denies even if a later duplicate tag is valid — -/// an ambiguous announcement must fail closed, not silently resolve to -/// whichever duplicate happens to parse. -fn repo_bound_channel_id(event: &nostr::Event) -> Option { - let first = event - .tags - .iter() - .find(|t| t.as_slice().first().map(String::as_str) == Some("buzz-channel"))?; - first - .as_slice() - .get(1) - .and_then(|v| uuid::Uuid::parse_str(v).ok()) -} - /// Pure decision for [`authorize_git_read`]: a read requires a current /// active membership row whose role the relay recognizes. /// @@ -2454,75 +2468,30 @@ mod sec005_read_gate_tests { .expect("sign 30617") } - #[test] - fn repo_bound_channel_id_extracts_valid_uuid() { - let keys = Keys::generate(); - let ch = uuid::Uuid::new_v4(); - let event = announcement( - &keys, - vec![ - Tag::parse(["d", "r"]).unwrap(), - Tag::parse(["buzz-channel", &ch.to_string()]).unwrap(), - ], - ); - assert_eq!(repo_bound_channel_id(&event), Some(ch)); - } - - #[test] - fn repo_bound_channel_id_rejects_absent_and_malformed_bindings() { - let keys = Keys::generate(); - let absent = announcement(&keys, vec![Tag::parse(["d", "r"]).unwrap()]); - assert_eq!(repo_bound_channel_id(&absent), None); - - let malformed = announcement( - &keys, - vec![ - Tag::parse(["d", "r"]).unwrap(), - Tag::parse(["buzz-channel", "not-a-uuid"]).unwrap(), - ], - ); - assert_eq!(repo_bound_channel_id(&malformed), None); - - let empty = announcement( - &keys, - vec![ - Tag::parse(["d", "r"]).unwrap(), - Tag::parse(["buzz-channel"]).unwrap(), - ], - ); - assert_eq!(repo_bound_channel_id(&empty), None); + // Binding *parse* semantics (first-tag fails-closed, duplicate-tag + // ambiguity, malformed vs. absent) are unit-tested where the resolver + // lives: `super::super::binding`. The tests below prove the *gate* wires + // each resolver outcome to the right response — allow, generic denial + // body, or the author remediation body — which the resolver tests + // cannot see. + + /// Collapse an `authorize_git_read` denial to `(status, body)` so tests + /// can assert on the exact bytes a git client would see. A blind + /// `.is_err()` cannot distinguish the generic 404 from the remediation + /// 404 — and that distinction IS the security property. + async fn denial_parts(result: Result<(), Response>) -> (StatusCode, String) { + let response = result.expect_err("expected a denial"); + let status = response.status(); + let bytes = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .expect("read denial body"); + ( + status, + String::from_utf8(bytes.to_vec()).expect("utf-8 body"), + ) } - #[test] - fn repo_bound_channel_id_fails_closed_on_ambiguous_duplicate_bindings() { - // First-tag semantics: a malformed first binding must deny even when - // a later duplicate tag is valid. An ambiguous announcement must not - // silently resolve to whichever duplicate happens to parse. - let keys = Keys::generate(); - let ch = uuid::Uuid::new_v4(); - let malformed_first = announcement( - &keys, - vec![ - Tag::parse(["d", "r"]).unwrap(), - Tag::parse(["buzz-channel", "not-a-uuid"]).unwrap(), - Tag::parse(["buzz-channel", &ch.to_string()]).unwrap(), - ], - ); - assert_eq!(repo_bound_channel_id(&malformed_first), None); - - // And the mirror image: a valid first binding wins, matching the - // push policy endpoint's first-tag resolution. - let other = uuid::Uuid::new_v4(); - let valid_first = announcement( - &keys, - vec![ - Tag::parse(["d", "r"]).unwrap(), - Tag::parse(["buzz-channel", &ch.to_string()]).unwrap(), - Tag::parse(["buzz-channel", &other.to_string()]).unwrap(), - ], - ); - assert_eq!(repo_bound_channel_id(&valid_first), Some(ch)); - } + const GENERIC_DENIAL: &str = "repository not found"; // ── authorize_git_read matrix (requires Postgres) ──────────────────── @@ -2544,6 +2513,11 @@ mod sec005_read_gate_tests { Missing, /// `buzz-channel` tag whose value is not a UUID. Malformed, + /// `buzz-channel` tag carrying a well-formed UUID that names no + /// channel. The resolver reports `Bound`; the membership lookup + /// (whose SQL joins `channels … deleted_at IS NULL`) then returns + /// no role — the deliberate phase-1 posture for dead bindings. + UnknownChannel, } struct RepoFixture { @@ -2609,6 +2583,9 @@ mod sec005_read_gate_tests { Binding::Malformed => { tags.push(Tag::parse(["buzz-channel", "not-a-uuid"]).unwrap()); } + Binding::UnknownChannel => { + tags.push(Tag::parse(["buzz-channel", &uuid::Uuid::new_v4().to_string()]).unwrap()); + } } let event = announcement(&owner_keys, tags); db.insert_event(community, &event, None) @@ -2676,32 +2653,63 @@ mod sec005_read_gate_tests { #[tokio::test] #[ignore = "requires Postgres"] async fn read_gate_denies_missing_or_malformed_binding_and_absent_repo() { - // Missing buzz-channel tag → deny even for a channel member. + // Missing buzz-channel tag → deny even for a channel member, with + // the generic body: the remediation carve-out is author-only. let f = setup_repo(Binding::Missing).await; let member = f.member_keys.public_key(); - assert!( - authorize_git_read(&f.db, f.community, &member, &f.owner_hex, &f.repo) - .await - .is_err(), - "announcement without buzz-channel binding must deny" + let (status, body) = denial_parts( + authorize_git_read(&f.db, f.community, &member, &f.owner_hex, &f.repo).await, + ) + .await; + assert_eq!(status, StatusCode::NOT_FOUND); + assert_eq!( + body, GENERIC_DENIAL, + "unbound repo read by a NON-author must get the generic body — \ + remediation for anyone but the announcement author leaks repo existence" ); - // Malformed buzz-channel tag → deny. + // Malformed buzz-channel tag → deny with the generic body EVEN FOR + // THE AUTHOR. This is the assertion that pins the carve-out to + // NotBound: if it ever fires on Broken, this fails on bytes, not + // on Ok/Err (which cannot see the difference). let g = setup_repo(Binding::Malformed).await; - let member_g = g.member_keys.public_key(); - assert!( - authorize_git_read(&g.db, g.community, &member_g, &g.owner_hex, &g.repo) - .await - .is_err(), - "announcement with malformed buzz-channel binding must deny" + let g_owner = g.owner_keys.public_key(); + let (status, body) = denial_parts( + authorize_git_read(&g.db, g.community, &g_owner, &g.owner_hex, &g.repo).await, + ) + .await; + assert_eq!(status, StatusCode::NOT_FOUND); + assert_eq!( + body, GENERIC_DENIAL, + "broken binding must stay generic even for the author (ambiguity fails closed)" + ); + + // Well-formed UUID naming a nonexistent channel → resolver says + // Bound, membership lookup finds nothing → generic denial for + // everyone, author included. The dead-channel case must be + // indistinguishable from non-membership (phase-1 posture; ingest + // validation closes the front door in phase 2). + let u = setup_repo(Binding::UnknownChannel).await; + let u_owner = u.owner_keys.public_key(); + let (status, body) = denial_parts( + authorize_git_read(&u.db, u.community, &u_owner, &u.owner_hex, &u.repo).await, + ) + .await; + assert_eq!(status, StatusCode::NOT_FOUND); + assert_eq!( + body, GENERIC_DENIAL, + "binding to a nonexistent channel must deny generically, even for the author" ); // Nonexistent announcement → deny. - assert!( - authorize_git_read(&f.db, f.community, &member, &f.owner_hex, "no-such-repo") - .await - .is_err(), - "nonexistent repo must deny" + let (status, body) = denial_parts( + authorize_git_read(&f.db, f.community, &member, &f.owner_hex, "no-such-repo").await, + ) + .await; + assert_eq!(status, StatusCode::NOT_FOUND); + assert_eq!( + body, GENERIC_DENIAL, + "nonexistent repo must deny generically" ); // Owner-mismatch: URL owner differs from announcement author → deny. @@ -2722,6 +2730,53 @@ mod sec005_read_gate_tests { ); } + #[tokio::test] + #[ignore = "requires Postgres"] + async fn read_gate_gives_author_of_unbound_repo_remediation_body() { + // Issue #3527: the author of a never-bound announcement is the one + // identity that can fix it (30617 is keyed by (author, d)) and the + // one identity remediation cannot leak anything to. Status must stay + // 404 — identical to every other denial — with the bind command in + // the body. + let f = setup_repo(Binding::Missing).await; + let author = f.owner_keys.public_key(); + + let response = authorize_git_read(&f.db, f.community, &author, &f.owner_hex, &f.repo) + .await + .expect_err("unbound repo must still deny its author"); + assert_eq!(response.status(), StatusCode::NOT_FOUND); + // Guard against a future "tidy" into Json(...) or a custom + // IntoResponse: git prints `remote:` lines only for text/plain + // bodies — any other content-type makes the remediation silently + // invisible in the user's terminal with no failing assertion. + assert_eq!( + response + .headers() + .get(axum::http::header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()), + Some("text/plain; charset=utf-8"), + "remediation body must stay text/plain or git clients will swallow it" + ); + let bytes = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .expect("read remediation body"); + let body = String::from_utf8(bytes.to_vec()).expect("utf-8 body"); + assert!( + body.starts_with(&format!("run: buzz repos bind --id {}", f.repo)), + "remediation must lead with the actionable command (got {body:?})" + ); + assert_ne!(body, GENERIC_DENIAL); + + // Same repo, same state, different caller: a member of some channel + // who is not the author still gets the generic body. + let member = f.member_keys.public_key(); + let (_, body) = denial_parts( + authorize_git_read(&f.db, f.community, &member, &f.owner_hex, &f.repo).await, + ) + .await; + assert_eq!(body, GENERIC_DENIAL, "remediation is author-only"); + } + #[tokio::test] #[ignore = "requires Postgres"] async fn read_gate_follows_current_announcement_not_stale_registry() { diff --git a/crates/buzz-relay/src/handlers/ingest.rs b/crates/buzz-relay/src/handlers/ingest.rs index a30b0e714d..ee644d5a9b 100644 --- a/crates/buzz-relay/src/handlers/ingest.rs +++ b/crates/buzz-relay/src/handlers/ingest.rs @@ -2481,7 +2481,12 @@ async fn ingest_event_inner( crate::handlers::side_effects::handle_side_effects(tenant, kind_u32, &event, state) .await { - warn!(event_id = %event_id_hex, kind = kind_u32, "Side effect failed: {e}"); + // error!, not warn!: the event was accepted but its side effects + // (channel creation, git repo seeding, …) did not run — the relay + // is now in a state the client believes it isn't. Production runs + // RUST_LOG=error, so warn! made these failures invisible during + // the #3527 triage. + error!(event_id = %event_id_hex, kind = kind_u32, "Side effect failed: {e}"); } } diff --git a/crates/buzz-test-client/tests/e2e_git.rs b/crates/buzz-test-client/tests/e2e_git.rs index 63281fd18f..766ef9d9e5 100644 --- a/crates/buzz-test-client/tests/e2e_git.rs +++ b/crates/buzz-test-client/tests/e2e_git.rs @@ -61,6 +61,27 @@ async fn post_event(event: &nostr::Event) { ); } +/// Create a channel (kind:9007) owned by `keys` and return its UUID. +/// +/// The git read gate (SEC-005) authorizes against membership in the channel +/// named by the announcement's `buzz-channel` tag, so every repo these tests +/// announce must be bound to a channel its owner belongs to — creating the +/// channel makes the creator its owner-member. +async fn create_test_channel(keys: &Keys) -> String { + let channel_uuid = uuid::Uuid::new_v4().to_string(); + let event = EventBuilder::new(Kind::Custom(9007), "") + .tags(vec![ + Tag::parse(["h", &channel_uuid]).unwrap(), + Tag::parse(["name", &format!("git-e2e-{channel_uuid}")]).unwrap(), + Tag::parse(["channel_type", "stream"]).unwrap(), + Tag::parse(["visibility", "open"]).unwrap(), + ]) + .sign_with_keys(keys) + .unwrap(); + post_event(&event).await; + channel_uuid +} + /// Run `git` with the Buzz credential helper and isolated config. fn git_status(args: &[&str], cwd: &Path, owner_nsec: &str) -> std::process::Output { let helper = credential_helper(); @@ -204,10 +225,15 @@ async fn git_clone_push_fetch_force_roundtrip() { let s3 = GitS3Probe::from_env(); // Announce the repo (kind:30617) so the relay creates the bare repo + hook. + // The `buzz-channel` binding is the repo's ACL: without it the read gate + // 404s even for the owner (issue #3527), so bind to a channel the owner + // just created (and therefore belongs to). + let channel = create_test_channel(&owner).await; let announce = EventBuilder::new(Kind::from(30617), "") .tags(vec![ Tag::parse(["d", &repo]).unwrap(), Tag::parse(["name", "e2e git repo"]).unwrap(), + Tag::parse(["buzz-channel", &channel]).unwrap(), ]) .sign_with_keys(&owner) .unwrap(); @@ -341,10 +367,12 @@ async fn git_concurrent_push_one_wins_and_repo_recovers() { let repo = format!("e2e-git-concurrent-{}", std::process::id()); let s3 = GitS3Probe::from_env(); + let channel = create_test_channel(&owner).await; let announce = EventBuilder::new(Kind::from(30617), "") .tags(vec![ Tag::parse(["d", &repo]).unwrap(), Tag::parse(["name", "e2e concurrent git repo"]).unwrap(), + Tag::parse(["buzz-channel", &channel]).unwrap(), ]) .sign_with_keys(&owner) .unwrap(); diff --git a/desktop/src-tauri/src/commands/mod.rs b/desktop/src-tauri/src/commands/mod.rs index 1c89ee4f77..66ef7ef17b 100644 --- a/desktop/src-tauri/src/commands/mod.rs +++ b/desktop/src-tauri/src/commands/mod.rs @@ -44,6 +44,7 @@ mod project_git; mod project_git_branches; mod project_git_diff; mod project_git_exec; +mod project_git_merge_error; mod project_git_push; mod project_git_workflow; mod project_repo_paths; diff --git a/desktop/src-tauri/src/commands/project_git_merge_error.rs b/desktop/src-tauri/src/commands/project_git_merge_error.rs new file mode 100644 index 0000000000..460ce83fa3 --- /dev/null +++ b/desktop/src-tauri/src/commands/project_git_merge_error.rs @@ -0,0 +1,152 @@ +//! Structured pull-request merge failures returned across the Tauri boundary. + +use serde::Serialize; + +/// Machine-readable recovery metadata for a failed pull-request merge. +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct ProjectPullRequestMergeRecovery { + action: String, + target_branch: String, + source_branch: String, +} + +/// Structured pull-request merge failure returned across the Tauri boundary. +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct ProjectPullRequestMergeError { + code: String, + message: String, + recovery: Option, +} + +impl ProjectPullRequestMergeError { + pub(crate) fn new(code: &str, message: impl Into) -> Self { + Self { + code: code.to_string(), + message: message.into(), + recovery: None, + } + } + + fn conflict(target_branch: String, source_branch: String) -> Self { + Self { + code: "merge_conflict".to_string(), + message: "Pull request has merge conflicts.".to_string(), + recovery: Some(ProjectPullRequestMergeRecovery { + action: "open_terminal".to_string(), + target_branch, + source_branch, + }), + } + } +} + +impl From for ProjectPullRequestMergeError { + fn from(message: String) -> Self { + // Relay push-policy denial for a repo with no `buzz-channel` binding. + // The stable token is declared in `buzz-core::git_perms` + // (GIT_NO_CHANNEL_BINDING_TOKEN); the relay guarantees the denial body + // starts with it. Push failures reach this conversion as raw + // stderr/`remote:` text, so match the token anywhere in the message. + if message.contains(buzz_core_pkg::git_perms::GIT_NO_CHANNEL_BINDING_TOKEN) { + return Self::new( + buzz_core_pkg::git_perms::GIT_NO_CHANNEL_BINDING_TOKEN, + "This repository is not bound to a channel, so the relay cannot \ + authorize pushes. Bind it with: buzz repos bind --id \ + --channel ", + ); + } + Self::new("merge_failed", message) + } +} + +pub(crate) fn classify_merge_error( + message: String, + has_conflicts: bool, + target_branch: &str, + source_branch: &str, +) -> ProjectPullRequestMergeError { + if has_conflicts { + ProjectPullRequestMergeError::conflict(target_branch.to_string(), source_branch.to_string()) + } else { + ProjectPullRequestMergeError::new( + "merge_failed", + format!("Pull request merge failed: {message}"), + ) + } +} + +#[cfg(test)] +mod tests { + use super::{classify_merge_error, ProjectPullRequestMergeError}; + + #[test] + fn merge_conflict_error_has_stable_recovery_metadata() { + let error = + ProjectPullRequestMergeError::conflict("main".to_string(), "feature/demo".to_string()); + + assert_eq!(error.code, "merge_conflict"); + assert_eq!(error.message, "Pull request has merge conflicts."); + let recovery = error.recovery.expect("conflict recovery"); + assert_eq!(recovery.action, "open_terminal"); + assert_eq!(recovery.target_branch, "main"); + assert_eq!(recovery.source_branch, "feature/demo"); + } + + #[test] + fn merge_conflict_error_serializes_for_tauri_clients() { + let error = + ProjectPullRequestMergeError::conflict("main".to_string(), "feature/demo".to_string()); + let value = serde_json::to_value(error).expect("serialize merge conflict"); + + assert_eq!(value["code"], "merge_conflict"); + assert_eq!(value["recovery"]["targetBranch"], "main"); + assert_eq!(value["recovery"]["sourceBranch"], "feature/demo"); + } + + #[test] + fn merge_error_classification_only_recovers_conflicts() { + let conflict = classify_merge_error( + "CONFLICT (content): Merge conflict in src/main.rs".to_string(), + true, + "main", + "feature/demo", + ); + assert_eq!(conflict.code, "merge_conflict"); + assert!(conflict.recovery.is_some()); + + let other = classify_merge_error( + "fatal: refusing to merge unrelated histories".to_string(), + false, + "main", + "feature/demo", + ); + assert_eq!(other.code, "merge_failed"); + assert!(other.recovery.is_none()); + } + + #[test] + fn no_channel_binding_denial_converts_to_structured_code() { + // The relay's push-policy denial arrives as raw git stderr with + // `remote:` framing; the stable token must be recognized wherever it + // sits in the message. + let remote_stderr = format!( + "remote: {}\nerror: failed to push some refs", + buzz_core_pkg::git_perms::GIT_NO_CHANNEL_BINDING_BODY + ); + let error = ProjectPullRequestMergeError::from(remote_stderr); + + assert_eq!( + error.code, + buzz_core_pkg::git_perms::GIT_NO_CHANNEL_BINDING_TOKEN + ); + assert!(error.message.contains("buzz repos bind")); + assert!(error.recovery.is_none()); + + // Unrelated push failures keep the generic code and original text. + let generic = ProjectPullRequestMergeError::from("connection reset".to_string()); + assert_eq!(generic.code, "merge_failed"); + assert_eq!(generic.message, "connection reset"); + } +} diff --git a/desktop/src-tauri/src/commands/project_git_workflow.rs b/desktop/src-tauri/src/commands/project_git_workflow.rs index 39832feb10..624bbf4dfc 100644 --- a/desktop/src-tauri/src/commands/project_git_workflow.rs +++ b/desktop/src-tauri/src/commands/project_git_workflow.rs @@ -32,67 +32,10 @@ pub struct ProjectRepoMergeResult { pub status_publication_error: Option, } -/// Machine-readable recovery metadata for a failed pull-request merge. -#[derive(Debug, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct ProjectPullRequestMergeRecovery { - action: String, - target_branch: String, - source_branch: String, -} - -/// Structured pull-request merge failure returned across the Tauri boundary. -#[derive(Debug, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct ProjectPullRequestMergeError { - code: String, - message: String, - recovery: Option, -} - -impl ProjectPullRequestMergeError { - fn new(code: &str, message: impl Into) -> Self { - Self { - code: code.to_string(), - message: message.into(), - recovery: None, - } - } - - fn conflict(target_branch: String, source_branch: String) -> Self { - Self { - code: "merge_conflict".to_string(), - message: "Pull request has merge conflicts.".to_string(), - recovery: Some(ProjectPullRequestMergeRecovery { - action: "open_terminal".to_string(), - target_branch, - source_branch, - }), - } - } -} - -impl From for ProjectPullRequestMergeError { - fn from(message: String) -> Self { - Self::new("merge_failed", message) - } -} - -fn classify_merge_error( - message: String, - has_conflicts: bool, - target_branch: &str, - source_branch: &str, -) -> ProjectPullRequestMergeError { - if has_conflicts { - ProjectPullRequestMergeError::conflict(target_branch.to_string(), source_branch.to_string()) - } else { - ProjectPullRequestMergeError::new( - "merge_failed", - format!("Pull request merge failed: {message}"), - ) - } -} +/// Machine-readable pull-request merge failure types live in +/// [`super::project_git_merge_error`]; re-imported here for the merge +/// workflow below. +use super::project_git_merge_error::{classify_merge_error, ProjectPullRequestMergeError}; struct ProjectRepoMergeGitResult { message: String, @@ -739,8 +682,8 @@ pub async fn merge_project_pull_request( mod tests { use super::{ align_unborn_head_branch, build_merged_status_event, build_pull_request_status_event, - build_review_request_event, classify_merge_error, normalize_commit, same_repository, - validate_merge_status_metadata, ProjectPullRequestMergeError, + build_review_request_event, normalize_commit, same_repository, + validate_merge_status_metadata, }; use crate::commands::project_git_exec::{build_test_git_auth_config, run_git}; use nostr::{Event, JsonUtil, Keys, Timestamp}; @@ -785,51 +728,6 @@ mod tests { )); } - #[test] - fn merge_conflict_error_has_stable_recovery_metadata() { - let error = - ProjectPullRequestMergeError::conflict("main".to_string(), "feature/demo".to_string()); - - assert_eq!(error.code, "merge_conflict"); - assert_eq!(error.message, "Pull request has merge conflicts."); - let recovery = error.recovery.expect("conflict recovery"); - assert_eq!(recovery.action, "open_terminal"); - assert_eq!(recovery.target_branch, "main"); - assert_eq!(recovery.source_branch, "feature/demo"); - } - - #[test] - fn merge_conflict_error_serializes_for_tauri_clients() { - let error = - ProjectPullRequestMergeError::conflict("main".to_string(), "feature/demo".to_string()); - let value = serde_json::to_value(error).expect("serialize merge conflict"); - - assert_eq!(value["code"], "merge_conflict"); - assert_eq!(value["recovery"]["targetBranch"], "main"); - assert_eq!(value["recovery"]["sourceBranch"], "feature/demo"); - } - - #[test] - fn merge_error_classification_only_recovers_conflicts() { - let conflict = classify_merge_error( - "CONFLICT (content): Merge conflict in src/main.rs".to_string(), - true, - "main", - "feature/demo", - ); - assert_eq!(conflict.code, "merge_conflict"); - assert!(conflict.recovery.is_some()); - - let other = classify_merge_error( - "fatal: refusing to merge unrelated histories".to_string(), - false, - "main", - "feature/demo", - ); - assert_eq!(other.code, "merge_failed"); - assert!(other.recovery.is_none()); - } - #[test] fn merged_status_is_signed_by_repository_owner() { let keys = Keys::generate(); diff --git a/desktop/src/features/projects/lib/projectBranchErrors.test.mjs b/desktop/src/features/projects/lib/projectBranchErrors.test.mjs new file mode 100644 index 0000000000..c1c2a21a28 --- /dev/null +++ b/desktop/src/features/projects/lib/projectBranchErrors.test.mjs @@ -0,0 +1,43 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { + isNoChannelBindingError, + projectBranchErrorMessage, +} from "./projectBranchErrors.ts"; + +test("recognizes the relay's stable denial token", () => { + // Body produced by the relay push policy (buzz-core + // GIT_NO_CHANNEL_BINDING_BODY), as it arrives wrapped in git stderr. + assert.ok( + isNoChannelBindingError( + "remote: no_channel_binding: repository has no channel binding\nerror: failed to push some refs", + ), + ); +}); + +test("recognizes the legacy spaced phrase from older relays", () => { + assert.ok(isNoChannelBindingError("push denied: no channel binding")); +}); + +test("does not match unrelated errors", () => { + assert.ok(!isNoChannelBindingError("connection reset by peer")); + assert.ok(!isNoChannelBindingError("no channel")); +}); + +test("maps binding denials to remediation copy", () => { + const message = projectBranchErrorMessage( + new Error("remote: no_channel_binding: repository has no channel binding"), + "Failed to create branch.", + ); + assert.ok(message.includes("buzz repos bind")); +}); + +test("passes through other errors and falls back for non-errors", () => { + assert.equal( + projectBranchErrorMessage(new Error("boom"), "fallback"), + "boom", + ); + assert.equal(projectBranchErrorMessage("boom", "fallback"), "fallback"); + assert.equal(projectBranchErrorMessage(null, "fallback"), "fallback"); +}); diff --git a/desktop/src/features/projects/lib/projectBranchErrors.ts b/desktop/src/features/projects/lib/projectBranchErrors.ts new file mode 100644 index 0000000000..2cf990f1af --- /dev/null +++ b/desktop/src/features/projects/lib/projectBranchErrors.ts @@ -0,0 +1,35 @@ +/** + * Relay push-policy denial token for a repository with no `buzz-channel` + * binding. Declared in Rust as `buzz-core::git_perms:: + * GIT_NO_CHANNEL_BINDING_TOKEN`; the relay's denial body starts with it + * ("no_channel_binding: repository has no channel binding"). The legacy + * spaced phrase is kept as a second matcher so this build also recognizes + * denials from relays deployed before the token existed. + */ +const NO_CHANNEL_BINDING_TOKEN = "no_channel_binding"; +const NO_CHANNEL_BINDING_LEGACY_PHRASE = "no channel binding"; + +const NO_CHANNEL_BINDING_COPY = + "This repository is not linked to a project channel, so the relay cannot " + + "authorize access. The repository owner can link it with: " + + "buzz repos bind --id --channel "; + +/** True when a git/relay error text is the unbound-repository denial. */ +export function isNoChannelBindingError(message: string): boolean { + return ( + message.includes(NO_CHANNEL_BINDING_TOKEN) || + message.includes(NO_CHANNEL_BINDING_LEGACY_PHRASE) + ); +} + +/** Map a thrown branch-operation error to user-facing dialog copy. */ +export function projectBranchErrorMessage( + error: unknown, + fallback: string, +): string { + if (!(error instanceof Error)) return fallback; + if (isNoChannelBindingError(error.message)) { + return NO_CHANNEL_BINDING_COPY; + } + return error.message; +} diff --git a/desktop/src/features/projects/ui/ProjectBranchDialogs.tsx b/desktop/src/features/projects/ui/ProjectBranchDialogs.tsx index 20e8d14e1b..c2bde47355 100644 --- a/desktop/src/features/projects/ui/ProjectBranchDialogs.tsx +++ b/desktop/src/features/projects/ui/ProjectBranchDialogs.tsx @@ -5,6 +5,7 @@ import { normalizeProjectBranchName, projectBranchNameError, } from "@/features/projects/lib/projectBranches"; +import { projectBranchErrorMessage } from "@/features/projects/lib/projectBranchErrors"; import { AlertDialog, AlertDialogCancel, @@ -25,14 +26,6 @@ import { } from "@/shared/ui/dialog"; import { Input } from "@/shared/ui/input"; -function errorMessage(error: unknown, fallback: string) { - if (!(error instanceof Error)) return fallback; - if (error.message.includes("no channel binding")) { - return "This repository is owned by another identity and is not linked to a project channel."; - } - return error.message; -} - export function CreateProjectBranchDialog({ existingBranches, onCreate, @@ -69,7 +62,9 @@ export function CreateProjectBranchDialog({ await onCreate(branch); onOpenChange(false); } catch (error) { - setSubmitError(errorMessage(error, "Failed to create branch.")); + setSubmitError( + projectBranchErrorMessage(error, "Failed to create branch."), + ); } } @@ -165,7 +160,9 @@ export function DeleteProjectBranchDialog({ await onDelete(); onOpenChange(false); } catch (error) { - setSubmitError(errorMessage(error, "Failed to delete branch.")); + setSubmitError( + projectBranchErrorMessage(error, "Failed to delete branch."), + ); } }