fix(relay): tombstone workflow definition events on NIP-09 deletion

crates/buzz-cli/src/commands/workflows.rs

@@ -34,6 +34,21 @@ pub async fn cmd_list_workflows(client: &BuzzClient, channel_id: &str) -> Result
     Ok(())
 }
 
+/// Report whether a live (non-tombstoned) kind:30620 definition exists.
+///
+/// The relay omits soft-deleted events from query results, so an empty result
+/// set means the workflow was never created or has since been deleted.
+async fn workflow_exists(client: &BuzzClient, workflow_id: &str) -> Result<bool, CliError> {
+    let filter = serde_json::json!({
+        "kinds": [30620],
+        "#d": [workflow_id],
+        "limit": 1
+    });
+    let resp = client.query(&filter).await?;
+    let events: Vec<serde_json::Value> = serde_json::from_str(&resp).unwrap_or_default();
+    Ok(!events.is_empty())
+}
+
 /// Get a single workflow definition.
 pub async fn cmd_get_workflow(client: &BuzzClient, workflow_id: &str) -> Result<(), CliError> {
     validate_uuid(workflow_id)?;
@@ -126,6 +141,20 @@ pub async fn cmd_update_workflow(
     let wf_uuid = parse_uuid(workflow_id)?;
     let yaml_definition = read_or_stdin(yaml)?;
 
+    // Refuse to update a workflow that no longer exists. kind:30620 is
+    // parameterized-replaceable, so publishing a definition for a deleted
+    // workflow is a valid NIP-33 republish at the protocol level — the relay
+    // accepts it and re-upserts the `workflows` row, silently resurrecting a
+    // deleted workflow with a freshly issued webhook secret. `update` means
+    // "modify an existing workflow", so the absence of a live definition is a
+    // not-found error here rather than an implicit create.
+    // See https://github.com/block/buzz/issues/4864.
+    if !workflow_exists(client, workflow_id).await? {
+        return Err(CliError::NotFound(format!(
+            "workflow not found: {workflow_id}"
+        )));
+    }
+
     let builder = buzz_sdk::build_workflow_update(channel_uuid, wf_uuid, &yaml_definition)
         .map_err(sdk_err)?;
     let event = client.sign_event(builder)?;

crates/buzz-relay/src/handlers/side_effects.rs

@@ -2102,8 +2102,10 @@ async fn handle_a_tag_deletion(
             tracing::debug!(d_tag, "NIP-09 deletion ignored for push lease");
         }
         buzz_core::kind::KIND_WORKFLOW_DEF => {
-            // Try UUID first (workflow_id); fall back to name-based lookup.
-            if let Ok(wf_id) = uuid::Uuid::parse_str(d_tag) {
+            // Resolve the workflow UUID. The a-tag carries either the UUID
+            // (what `build_workflow_def` writes as the d-tag) or a human-facing
+            // name that has to be resolved against the owner's workflows.
+            let resolved_id = if let Ok(wf_id) = uuid::Uuid::parse_str(d_tag) {
                 let channel_id = state
                     .db
                     .delete_workflow_for_owner(tenant.community(), wf_id, &actor_bytes)
@@ -2115,6 +2117,7 @@ async fn handle_a_tag_deletion(
                         .invalidate_channel_workflows(tenant.community(), channel_id);
                 }
                 tracing::info!(workflow_id = %wf_id, "Workflow deleted via NIP-09 a-tag (UUID)");
+                Some(wf_id)
             } else {
                 // Name-based lookup
                 match state
@@ -2136,25 +2139,69 @@ async fn handle_a_tag_deletion(
                                 .invalidate_channel_workflows(tenant.community(), channel_id);
                         }
                         tracing::info!(workflow_id = %wf.id, name = d_tag, "Workflow deleted via NIP-09 a-tag (name)");
+                        Some(wf.id)
                     }
                     Ok(None) => {
                         tracing::warn!(
                             "NIP-09 a-tag deletion: no workflow '{d_tag}' found for owner"
                         );
+                        None
                     }
                     Err(e) => {
                         tracing::warn!("NIP-09 a-tag deletion: DB lookup failed: {e}");
+                        None
                     }
                 }
+            };
+
+            // Tombstone the kind:30620 definition event as well. Dropping the
+            // `workflows` row stops execution, but every client reads
+            // definitions by querying kind:30620 events — `buzz workflows
+            // list`/`get` and the desktop's `get_channel_workflows` both do —
+            // so on its own the deleted workflow stays visible, and a later
+            // `workflows update` re-upserts the row from the still-live event,
+            // resurrecting the workflow with a freshly issued webhook secret.
+            //
+            // The coordinate is scoped to `actor_bytes`, the same owner check
+            // `delete_workflow_for_owner` applies, so a crafted a-tag naming
+            // another author's pubkey can never tombstone their definition.
+            // See https://github.com/block/buzz/issues/4864.
+            if let Some(wf_id) = resolved_id {
+                let deleted = state
+                    .db
+                    .soft_delete_by_coordinate(
+                        tenant.community(),
+                        buzz_core::kind::KIND_WORKFLOW_DEF as i32,
+                        &actor_bytes,
+                        &wf_id.to_string(),
+                        event.created_at.as_secs() as i64,
+                    )
+                    .await
+                    .map_err(|e| {
+                        anyhow::anyhow!(
+                            "failed to soft-delete workflow definition event {wf_id}: {e}"
+                        )
+                    })?;
+                if deleted {
+                    tracing::info!(
+                        workflow_id = %wf_id,
+                        "NIP-09 a-tag deletion: tombstoned workflow definition event"
+                    );
+                } else {
+                    tracing::debug!(
+                        workflow_id = %wf_id,
+                        "NIP-09 a-tag deletion: no live workflow definition event matched coordinate"
+                    );
+                }
             }
         }
         // Generic NIP-33 (parameterized-replaceable) soft-delete by coordinate.
         //
         // Listed after the workflow branch so workflow's bespoke deletion
-        // (which doesn't soft-delete the `events` row by design — that's a
-        // separate concern) takes precedence. For every other addressable
-        // kind, including kind:30023 (NIP-23 long-form), we soft-delete the
-        // live row matching `(kind, pubkey, d_tag)` so REQs stop returning it.
+        // (which additionally drops the `workflows` row that drives execution)
+        // takes precedence. For every other addressable kind, including
+        // kind:30023 (NIP-23 long-form), we soft-delete the live row matching
+        // `(kind, pubkey, d_tag)` so REQs stop returning it.
         // See https://github.com/block/sprout/issues/714.
         k if is_parameterized_replaceable(k) => {
             let pubkey_bytes = match hex::decode(pubkey_hex) {

crates/buzz-test-client/tests/e2e_workflow_delete.rs

@@ -0,0 +1,228 @@
+//! End-to-end tests for NIP-09 deletion of workflow definitions (kind:30620).
+//!
+//! These tests require a running relay instance. By default they are marked
+//! `#[ignore]` so that `cargo test` does not fail in CI when the relay is not
+//! available.
+//!
+//! # Running
+//!
+//! Start the relay, then run:
+//!
+//! ```text
+//! cargo test --test e2e_workflow_delete -- --ignored
+//! ```
+//!
+//! Override the relay URL with the `RELAY_URL` environment variable:
+//!
+//! ```text
+//! RELAY_URL=ws://relay.example.com cargo test --test e2e_workflow_delete -- --ignored
+//! ```
+
+use std::time::Duration;
+
+use buzz_test_client::BuzzTestClient;
+use nostr::{Alphabet, EventBuilder, Filter, Keys, Kind, SingleLetterTag, Tag};
+
+const KIND_WORKFLOW_DEF: u16 = 30620;
+
+fn relay_url() -> String {
+    std::env::var("RELAY_URL").unwrap_or_else(|_| "ws://localhost:3000".to_string())
+}
+
+fn sub_id(name: &str) -> String {
+    format!("e2e-{name}-{}", uuid::Uuid::new_v4())
+}
+
+fn workflow_yaml(name: &str) -> String {
+    format!(
+        "name: {name}\ntrigger:\n  on: webhook\nsteps:\n  - id: s1\n    action: send_message\n    text: hi\n"
+    )
+}
+
+/// Create an `open` stream channel owned by `keys` (kind:9007). The creator is
+/// bootstrapped as an owner-member, which the kind:30620 membership check
+/// requires.
+async fn create_channel(client: &mut BuzzTestClient, keys: &Keys) -> uuid::Uuid {
+    let channel_id = uuid::Uuid::new_v4();
+    let event = EventBuilder::new(Kind::Custom(9007), "")
+        .tags(vec![
+            Tag::parse(["h", &channel_id.to_string()]).unwrap(),
+            Tag::parse(["name", &format!("wf-del-{channel_id}")]).unwrap(),
+            Tag::parse(["channel_type", "stream"]).unwrap(),
+            Tag::parse(["visibility", "open"]).unwrap(),
+        ])
+        .sign_with_keys(keys)
+        .unwrap();
+    let ok = client.send_event(event).await.expect("send create-channel");
+    assert!(ok.accepted, "channel should be created: {}", ok.message);
+    channel_id
+}
+
+/// Query the live kind:30620 events for a workflow coordinate.
+async fn query_workflow(
+    client: &mut BuzzTestClient,
+    keys: &Keys,
+    workflow_id: &uuid::Uuid,
+    label: &str,
+) -> Vec<nostr::Event> {
+    let sid = sub_id(label);
+    let filter = Filter::new()
+        .kind(Kind::Custom(KIND_WORKFLOW_DEF))
+        .author(keys.public_key())
+        .custom_tag(
+            SingleLetterTag::lowercase(Alphabet::D),
+            workflow_id.to_string(),
+        );
+    client
+        .subscribe(&sid, vec![filter])
+        .await
+        .expect("subscribe");
+    client
+        .collect_until_eose(&sid, Duration::from_secs(5))
+        .await
+        .expect("collect")
+}
+
+/// NIP-09 a-tag deletion: a kind:5 deletion targeting the addressable
+/// coordinate `30620:<pubkey>:<workflow-id>` must soft-delete the live
+/// definition event, so subsequent REQs no longer return it.
+///
+/// Regression test for issue #4864 — before the fix, the workflow branch of
+/// `handle_a_tag_deletion` dropped the `workflows` row that drives execution
+/// but left the kind:30620 definition event live. Every client reads
+/// definitions by querying kind:30620 (`buzz workflows list`/`get`, the
+/// desktop's `get_channel_workflows`), so a deleted workflow stayed fully
+/// visible, and a later `workflows update` re-upserted the row from the
+/// still-live event — resurrecting the workflow with a fresh webhook secret.
+#[tokio::test]
+#[ignore]
+async fn test_workflow_a_tag_deletion_tombstones_definition_event() {
+    let url = relay_url();
+    let keys = Keys::generate();
+    let mut client = BuzzTestClient::connect(&url, &keys).await.expect("connect");
+
+    let channel_id = create_channel(&mut client, &keys).await;
+
+    // Publish a workflow definition keyed by a client-chosen d-tag — the same
+    // shape `buzz_sdk::build_workflow_def` writes.
+    let workflow_id = uuid::Uuid::new_v4();
+    let def = EventBuilder::new(
+        Kind::Custom(KIND_WORKFLOW_DEF),
+        workflow_yaml("e2e-delete-me"),
+    )
+    .tags(vec![
+        Tag::parse(["d", &workflow_id.to_string()]).unwrap(),
+        Tag::parse(["h", &channel_id.to_string()]).unwrap(),
+    ])
+    .sign_with_keys(&keys)
+    .unwrap();
+    let ok = client.send_event(def).await.expect("send workflow def");
+    assert!(
+        ok.accepted,
+        "workflow def should be accepted: {}",
+        ok.message
+    );
+
+    // Sanity check: queryable before deletion.
+    let pre = query_workflow(&mut client, &keys, &workflow_id, "wf-del-pre").await;
+    assert!(
+        !pre.is_empty(),
+        "workflow definition should be queryable before deletion"
+    );
+
+    // Delete via the addressable coordinate.
+    let a_coord = format!(
+        "{}:{}:{}",
+        KIND_WORKFLOW_DEF,
+        keys.public_key().to_hex(),
+        workflow_id
+    );
+    let del = EventBuilder::new(Kind::EventDeletion, "")
+        .tags(vec![Tag::parse(["a", &a_coord]).unwrap()])
+        .sign_with_keys(&keys)
+        .unwrap();
+    let ok_del = client.send_event(del).await.expect("send deletion");
+    assert!(
+        ok_del.accepted,
+        "a-tag deletion should be accepted: {}",
+        ok_del.message
+    );
+
+    // The definition event must no longer be returned.
+    let post = query_workflow(&mut client, &keys, &workflow_id, "wf-del-post").await;
+    assert!(
+        post.is_empty(),
+        "a-tag deletion should remove the workflow definition from REQ results (got {} events)",
+        post.len()
+    );
+
+    client.disconnect().await.expect("disconnect");
+}
+
+/// A deletion whose a-tag names a *different* author's pubkey must not
+/// tombstone that author's workflow definition. The coordinate delete is
+/// scoped to the deleting author, so a crafted a-tag is a no-op.
+#[tokio::test]
+#[ignore]
+async fn test_workflow_a_tag_deletion_cannot_delete_another_authors_workflow() {
+    let url = relay_url();
+    let owner = Keys::generate();
+    let attacker = Keys::generate();
+
+    let mut owner_client = BuzzTestClient::connect(&url, &owner)
+        .await
+        .expect("connect owner");
+    let channel_id = create_channel(&mut owner_client, &owner).await;
+
+    let workflow_id = uuid::Uuid::new_v4();
+    let def = EventBuilder::new(
+        Kind::Custom(KIND_WORKFLOW_DEF),
+        workflow_yaml("e2e-keep-me"),
+    )
+    .tags(vec![
+        Tag::parse(["d", &workflow_id.to_string()]).unwrap(),
+        Tag::parse(["h", &channel_id.to_string()]).unwrap(),
+    ])
+    .sign_with_keys(&owner)
+    .unwrap();
+    let ok = client_send(&mut owner_client, def).await;
+    assert!(ok, "owner workflow def should be accepted");
+
+    // The attacker signs a deletion naming the OWNER's coordinate.
+    let mut attacker_client = BuzzTestClient::connect(&url, &attacker)
+        .await
+        .expect("connect attacker");
+    let a_coord = format!(
+        "{}:{}:{}",
+        KIND_WORKFLOW_DEF,
+        owner.public_key().to_hex(),
+        workflow_id
+    );
+    let del = EventBuilder::new(Kind::EventDeletion, "")
+        .tags(vec![Tag::parse(["a", &a_coord]).unwrap()])
+        .sign_with_keys(&attacker)
+        .unwrap();
+    // Whether the relay accepts the envelope or not, the owner's definition
+    // must survive — that is the property under test.
+    let _ = attacker_client.send_event(del).await;
+
+    let survived = query_workflow(&mut owner_client, &owner, &workflow_id, "wf-del-attack").await;
+    assert!(
+        !survived.is_empty(),
+        "another author's a-tag deletion must not tombstone the owner's workflow definition"
+    );
+
+    owner_client.disconnect().await.expect("disconnect owner");
+    attacker_client
+        .disconnect()
+        .await
+        .expect("disconnect attacker");
+}
+
+async fn client_send(client: &mut BuzzTestClient, event: nostr::Event) -> bool {
+    client
+        .send_event(event)
+        .await
+        .map(|ok| ok.accepted)
+        .unwrap_or(false)
+}