diff --git a/crates/buzz-relay/src/api/bridge.rs b/crates/buzz-relay/src/api/bridge.rs index 36f0fa0119..8c41651710 100644 --- a/crates/buzz-relay/src/api/bridge.rs +++ b/crates/buzz-relay/src/api/bridge.rs @@ -2646,11 +2646,9 @@ pub struct WebhookQuery { /// Webhook trigger endpoint. No user auth — the webhook secret authenticates the caller. /// /// Prefers `X-Webhook-Secret` header over `?secret=` query param (headers aren't logged -/// by most proxies). Returns 202 Accepted; execution is async. A run is admitted -/// when its `workflow_runs` row is inserted; immediately before that insert, -/// admission checks the workflow owner's active ban in the host-bound community. -/// This point-in-time check does not cancel already-created runs, and a -/// concurrent ban committed after the lookup can race with row insertion. +/// by most proxies). Returns 202 Accepted; execution is async. Immediately +/// before the `workflow_runs` insert, the owner authority gate also rejects an +/// owner banned from the host-bound community. pub async fn workflow_webhook( State(state): State>, Path(id_str): Path, @@ -2744,9 +2742,10 @@ pub async fn workflow_webhook( // executes with the workflow **owner's** standing authority — so the // secret alone is insufficient. Immediately before run creation, reject // disabled/inactive workflows and recheck the owner's current channel - // membership (and role, for exfiltration-capable definitions). Fail - // closed with the same generic 404 as the lookups above so a - // revoked-owner workflow is indistinguishable from a nonexistent one. + // membership (and role, for exfiltration-capable definitions) and that + // the owner is not banned. Fail closed with the same generic 404 as the + // lookups above so a revoked-owner workflow is indistinguishable from a + // nonexistent one. if !workflow.enabled || workflow.status != buzz_db::workflow::WorkflowStatus::Active { return Err(not_found("workflow not found")); } @@ -2760,21 +2759,6 @@ pub async fn workflow_webhook( .await .map_err(|_| not_found("workflow not found"))?; - // The workflow_runs insert is the admission boundary. This is the last - // authorization gate before it. The restriction read is authoritative and - // tenant-scoped; lookup failures deny admission just like an active ban. It - // is intentionally a point-in-time check rather than a transaction with ban - // writes, so a ban racing after this read may follow the run insertion. Runs - // already inserted are unaffected. - let restrictions = state - .db - .moderation_restriction_state(community_id, &workflow.owner_pubkey) - .await - .map_err(|_| not_found("workflow not found"))?; - if restrictions.banned { - return Err(not_found("workflow not found")); - } - let run_id = state .db .create_workflow_run(community_id, id, None, trigger_ctx_json.as_ref()) diff --git a/crates/buzz-workflow/src/lib.rs b/crates/buzz-workflow/src/lib.rs index f33b57fdac..bfe4b117b1 100644 --- a/crates/buzz-workflow/src/lib.rs +++ b/crates/buzz-workflow/src/lib.rs @@ -142,10 +142,15 @@ impl WorkflowEngine { /// in a role that can write (a guest cannot); /// - if the definition contains an exfiltration-capable action /// (`call_webhook`), the owner must currently hold the `owner` or - /// `admin` role. + /// `admin` role; + /// - the owner, or the human owner of an agent owner, must not be banned + /// from the community. Bans leave channel membership in place, so the + /// role check alone would keep a banned owner's workflows running. /// /// Any lookup error denies (fail-closed): a removed owner must never keep /// exfiltration authority because a membership read happened to fail. + /// This is a point-in-time check: it does not cancel runs that already + /// exist, and a ban that commits after it can race with run creation. pub async fn check_owner_authority( &self, community_id: CommunityId, @@ -162,13 +167,26 @@ impl WorkflowEngine { "owner authority lookup failed (fail-closed): {e}" )) })?; - if owner_authority_allows(role.as_deref(), def.requires_elevated_authority()) { - Ok(()) - } else { - Err(WorkflowError::Unauthorized( + if !owner_authority_allows(role.as_deref(), def.requires_elevated_authority()) { + return Err(WorkflowError::Unauthorized( "workflow owner lacks current channel authority".into(), - )) + )); } + let restrictions = self + .db + .moderation_restriction_state(community_id, owner_pubkey) + .await + .map_err(|e| { + WorkflowError::Unauthorized(format!( + "owner restriction lookup failed (fail-closed): {e}" + )) + })?; + if restrictions.banned { + return Err(WorkflowError::Unauthorized( + "workflow owner is banned from the community".into(), + )); + } + Ok(()) } /// Set the action sink. Called once after `AppState` construction. @@ -2037,6 +2055,72 @@ steps: ); } + /// A ban leaves the owner's channel membership in place, so the event + /// path must still stop creating runs while the owner is banned, and + /// resume once the ban is lifted. Cron and webhook runs pass through the + /// same `check_owner_authority` gate. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn on_event_denies_run_while_owner_banned() { + let db = setup_db().await; + let creator = nostr::Keys::generate().public_key().to_bytes().to_vec(); + let member = nostr::Keys::generate().public_key().to_bytes().to_vec(); + let (community, channel_id) = setup_channel(&db, &creator, &member).await; + + let def_json = serde_json::json!({ + "name": "ban-event", + "trigger": {"on": "message_posted"}, + "steps": [{"id": "s1", "action": "send_message", "text": "hi"}], + "enabled": true, + }) + .to_string(); + let workflow_id = db + .create_workflow( + community, + Some(channel_id), + &member, + "ban-event", + &def_json, + &[0u8; 32], + ) + .await + .expect("create workflow"); + let engine = Arc::new(WorkflowEngine::new(db.clone(), WorkflowConfig::default())); + let run_count = || async { + db.list_workflow_runs(community, workflow_id, 10) + .await + .expect("list runs") + .len() + }; + + db.ban_community_member(community, &member, &creator, None, None) + .await + .expect("ban owner"); + engine + .on_event(community, &message_event(channel_id)) + .await + .expect("on_event while banned"); + assert_eq!( + run_count().await, + 0, + "a banned owner's workflow must not fire" + ); + + assert!(db + .unban_community_member(community, &member, &creator) + .await + .expect("unban owner")); + engine + .on_event(community, &message_event(channel_id)) + .await + .expect("on_event after unban"); + assert_eq!( + run_count().await, + 1, + "an unbanned owner's workflow fires again" + ); + } + /// Exfiltration-capable definitions (call_webhook) require the owner to /// currently hold an elevated role — a plain member's workflow must not /// fire even though the owner is still an active channel member.