jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-10-27 12:40:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(workflow): deny banned owners at every run-creation door

Browse Source 
The webhook handler checked the owner's ban itself, so cron and
event-triggered workflows of a banned owner still created runs. Bans
leave channel membership in place, so the shared SEC-006 owner authority
gate passed them.

Move the restriction read into WorkflowEngine::check_owner_authority,
which the webhook, event, cron and signed-trigger doors all call before
creating a run. A lookup failure still denies. The handler-local branch
goes away; the webhook keeps its generic 404.

Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Luke Tornquist <tornquist@squareup.com>
This commit is contained in:
Luke Tornquist
2026-10-26 14:05:51 -04:00
parent 44f469939b
commit 9eb5e07d1b
2 changed files with 97 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
crates/buzz-relay/src/api/bridge.rs
View File

@@ -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<Arc<AppState>>,
Path(id_str): Path<String>,
@@ -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())

96
crates/buzz-workflow/src/lib.rs
View File

@@ -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.