fix(relay): descriptive SEC-006 denial + structured skip log for workflow triggers

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

@@ -857,7 +857,7 @@ async fn handle_workflow_trigger(
     // member could otherwise invoke another user's webhook or message actions.
     if workflow.owner_pubkey != self_bytes {
         return Err(IngestError::Rejected(
-            "forbidden: not authorized to trigger this workflow".into(),
+            "forbidden: only the workflow owner may trigger this workflow".into(),
         ));
     }
 
@@ -866,25 +866,42 @@ async fn handle_workflow_trigger(
     // Without this, a disabled workflow — including one disabled because its
     // owner was removed from the channel — could still be fired by the owner.
     if !workflow.enabled || workflow.status != buzz_db::workflow::WorkflowStatus::Active {
-        return Err(IngestError::Rejected(
-            "forbidden: workflow is disabled or inactive".into(),
-        ));
+        return Err(IngestError::Rejected(format!(
+            "forbidden: workflow is {} (enabled={}); \
+             active runs require enabled=true and status=Active",
+            workflow.status, workflow.enabled
+        )));
     }
     let def: buzz_workflow::WorkflowDef = serde_json::from_value(workflow.definition.clone())
         .map_err(|e| IngestError::Internal(format!("error: corrupt workflow definition: {e}")))?;
     let Some(wf_channel_id) = workflow.channel_id else {
         // No channel scope means no channel authority to verify — fail closed.
         return Err(IngestError::Rejected(
-            "forbidden: workflow has no channel scope".into(),
+            "forbidden: workflow has no channel scope; cannot verify owner authority".into(),
         ));
     };
-    state
+    if let Err(e) = state
         .workflow_engine
         .check_owner_authority(community_id, wf_channel_id, &workflow.owner_pubkey, &def)
         .await
-        .map_err(|_| {
-            IngestError::Rejected("forbidden: not authorized to trigger this workflow".into())
-        })?;
+    {
+        // Give the caller a stable cause it can branch on (issue #5122) while
+        // keeping the underlying error server-side: `check_owner_authority`
+        // wraps store failures verbatim, so returning it would disclose
+        // internal database state on an authorization path.
+        // `owner_pubkey` is raw bytes with no Display impl; hex::encode is the
+        // convention used elsewhere in this file.
+        tracing::warn!(
+            %workflow_id,
+            owner_pubkey = %hex::encode(&workflow.owner_pubkey),
+            requires_elevated_authority = def.requires_elevated_authority(),
+            error = %e,
+            "workflow owner authority check failed"
+        );
+        return Err(IngestError::Rejected(workflow_owner_authority_denial(
+            def.requires_elevated_authority(),
+        )));
+    }
 
     // Persist the command event under the workflow channel even though the
     // trigger event itself only carries the workflow UUID. Storing channel
@@ -1368,3 +1385,84 @@ async fn resume_workflow_after_approval(
         .finalize_run(community_id, run_id, result, existing_trace)
         .await;
 }
+
+/// Build the user-facing rejection text when a manual workflow trigger fails
+/// its SEC-006 owner-authority check.
+///
+/// Exfiltration-capable definitions (those containing a `call_webhook` step)
+/// require the owner to currently hold an elevated role (`owner` or `admin`) —
+/// plain membership is insufficient. Ordinary definitions only require channel
+/// membership.
+///
+/// Previously both branches returned the same generic "not authorized" text,
+/// which left the owner of a `call_webhook` workflow unable to tell SEC-006
+/// (role missing) apart from a disabled workflow or a membership lapse (issue
+/// #5122: "workflow appears to succeed, no run record"). Naming the cause in
+/// the rejection gives the caller an actionable next step.
+/// Caller-facing denial for a failed SEC-006 owner-authority check.
+///
+/// Carries a stable public cause and never the underlying error.
+/// `check_owner_authority` wraps store failures verbatim (`owner authority
+/// lookup failed (fail-closed): {e}`), so interpolating it here turned an
+/// authorization denial into an internal-error disclosure whenever the
+/// membership store was unavailable. The detail is logged server-side.
+fn workflow_owner_authority_denial(requires_elevated_authority: bool) -> String {
+    if requires_elevated_authority {
+        "forbidden: SEC-006 role_required — workflow contains \
+         exfiltration-capable actions (call_webhook) that require the owner \
+         to hold the 'owner' or 'admin' role in this channel"
+            .to_string()
+    } else {
+        "forbidden: SEC-006 authority_lookup_failed — workflow owner's \
+         channel authority check failed"
+            .to_string()
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::workflow_owner_authority_denial;
+
+    #[test]
+    fn denial_for_elevated_definition_names_sec006_role_required() {
+        let msg = workflow_owner_authority_denial(true);
+        assert!(msg.starts_with("forbidden:"));
+        assert!(msg.contains("SEC-006 role_required"));
+        assert!(msg.contains("call_webhook"));
+        assert!(msg.contains("owner' or 'admin' role"));
+    }
+
+    #[test]
+    fn denial_for_ordinary_definition_names_authority_lookup_failed() {
+        let msg = workflow_owner_authority_denial(false);
+        assert!(msg.starts_with("forbidden:"));
+        assert!(msg.contains("SEC-006 authority_lookup_failed"));
+        assert!(!msg.contains("call_webhook"));
+    }
+
+    #[test]
+    fn denial_never_discloses_the_underlying_store_error() {
+        // Regression for the review on #5135: `check_owner_authority` wraps
+        // store failures verbatim, so interpolating the error into the
+        // caller-facing denial leaks internal database state on an
+        // authorization path.
+        const SENTINEL: &str = "connection refused to members.db at 10.0.0.7:5432";
+        for requires_elevated in [true, false] {
+            let msg = workflow_owner_authority_denial(requires_elevated);
+            assert!(!msg.contains(SENTINEL), "denial disclosed the store error: {msg}");
+            assert!(!msg.contains("members.db"));
+            assert!(!msg.contains("10.0.0.7"));
+            assert!(!msg.contains("lookup failed (fail-closed)"));
+        }
+    }
+
+    #[test]
+    fn denial_causes_are_stable_and_distinguishable() {
+        let elevated = workflow_owner_authority_denial(true);
+        let ordinary = workflow_owner_authority_denial(false);
+        assert_ne!(elevated, ordinary);
+        assert!(elevated.contains("role_required"));
+        assert!(ordinary.contains("authority_lookup_failed"));
+        assert!(!ordinary.contains("role_required"));
+    }
+}

crates/buzz-workflow/src/lib.rs

@@ -388,9 +388,16 @@ impl WorkflowEngine {
                 .check_owner_authority(community_id, channel_id, &workflow.owner_pubkey, &def)
                 .await
             {
+                // SEC-006 (exfiltration-capable definitions like call_webhook)
+                // denies runs without creating a run row or emitting an event —
+                // the only trace is this log. Include enough fields to diagnose
+                // "workflow appears to succeed, no run record" reports (issue
+                // #5122) from relay logs alone.
                 tracing::warn!(
                     workflow_id = %workflow.id,
-                    "Skipping workflow — owner authority check failed: {e}"
+                    owner_pubkey = %hex::encode(&workflow.owner_pubkey),
+                    requires_elevated_authority = def.requires_elevated_authority(),
+                    "Skipping workflow — SEC-006 owner authority check failed: {e}"
                 );
                 continue;
             }