fix(relay): claim NIP-98 replay slot only after authorization, attribute cancels

Two defects from the kalvin-agent security review of the admin moderation API.

Replay-before-authorization: authorize_nip98 claimed the deployment-scoped
replay ID immediately after crypto verification, before resolve_admin_principal
ran the roster check. Any validly-signing but unrostered key (every
WARP-admitted laptop) could allocate replay slots at request rate. Split the
NIP-98 path into verify-only (authorize_nip98, returns pubkey + event id) and a
separate claim_nip98_replay called only after principal resolution succeeds, so
an unrostered signer never consumes a slot. Fail-closed Redis behavior and the
deployment-scoped key format are unchanged.

Cancel actor trail: cancel_report discarded the resolved principal and
cancel_action persisted nothing about who cancelled — the one mutation with no
actor attribution while BUZZ_AUDIT_ENABLED=false. Add a cancelled_by column to
relay_admin_actions (mirroring moderation_reports.resolved_by), stamped in the
cancel UPDATE and surfaced through AdminActionDto.cancelledBy. Migration 0033 is
branch-local and unshipped, so the column is added in place with matching
schema.sql; the pgschema parity test round-trips it through bin/pgschema.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

crates/buzz-db/src/admin_moderation.rs

@@ -88,6 +88,10 @@ pub struct AdminActionDto {
     pub action: String,
     /// State machine: `"pending"` | `"enforcing"` | `"succeeded"` | `"failed"` | `"cancelled"`.
     pub status: String,
+    /// Principal who cancelled the action (hex pubkey); null unless `status` is
+    /// `"cancelled"`. Attributes the one mutation that would otherwise carry no
+    /// actor trail while `BUZZ_AUDIT_ENABLED=false`.
+    pub cancelled_by: Option<String>,
     /// Operator reason, if provided.
     pub reason: Option<String>,
     /// Absolute timeout expiry for `timeout` actions; null otherwise. Absolute
@@ -114,6 +118,7 @@ impl AdminActionDto {
             actor_role: record.actor_role.clone(),
             action: record.action.clone(),
             status: record.state.clone(),
+            cancelled_by: record.cancelled_by.as_deref().map(hex::encode),
             reason: record.reason.clone(),
             expires_at: record.timeout_until,
             error_message: record.error_message.clone(),
@@ -235,6 +240,7 @@ pub async fn get_report(pool: &PgPool, report_id: Uuid) -> Result<Option<AdminRe
                act.actor_role AS action_actor_role,
                act.action AS action_name,
                act.state AS action_state,
+               act.cancelled_by AS action_cancelled_by,
                act.reason AS action_reason,
                act.timeout_until AS action_timeout_until,
                act.error_message AS action_error_message,
@@ -253,7 +259,7 @@ pub async fn get_report(pool: &PgPool, report_id: Uuid) -> Result<Option<AdminRe
         ) target ON TRUE
         LEFT JOIN LATERAL (
             SELECT a.id, a.request_id, a.actor_pubkey, a.actor_role, a.action,
-                   a.state, a.reason, a.timeout_until, a.error_message,
+                   a.state, a.cancelled_by, a.reason, a.timeout_until, a.error_message,
                    a.created_at, a.updated_at
             FROM relay_admin_actions a
             WHERE a.report_community_id = r.community_id
@@ -305,6 +311,9 @@ fn row_to_action_dto(row: &sqlx::postgres::PgRow) -> Result<Option<AdminActionDt
         actor_role: row.try_get("action_actor_role")?,
         action: row.try_get("action_name")?,
         status: row.try_get("action_state")?,
+        cancelled_by: row
+            .try_get::<Option<Vec<u8>>, _>("action_cancelled_by")?
+            .map(hex::encode),
         reason: row.try_get("action_reason")?,
         expires_at: row.try_get("action_timeout_until")?,
         error_message: row.try_get("action_error_message")?,
@@ -860,6 +869,7 @@ mod tests {
             actor_role: "operator".to_string(),
             action: "ban".to_string(),
             status: "succeeded".to_string(),
+            cancelled_by: None,
             reason: None,
             expires_at: None,
             error_message: None,
@@ -868,7 +878,7 @@ mod tests {
         };
         let value = serde_json::to_value(&dto).expect("serialize dto");
         let obj = value.as_object().expect("dto serializes to an object");
-        for key in ["reason", "expiresAt", "errorMessage"] {
+        for key in ["reason", "expiresAt", "errorMessage", "cancelledBy"] {
             assert_eq!(
                 obj.get(key),
                 Some(&serde_json::Value::Null),

crates/buzz-db/src/lib.rs

@@ -4735,14 +4735,23 @@ impl Db {
         relay_admin_actions::record_failure(&self.pool, action_id, error).await
     }
 
-    /// Cancel a pre-mutation failed action (returns report to 'open').
+    /// Cancel a pre-mutation failed action (returns report to 'open'),
+    /// attributing the cancel to `cancelled_by`.
     pub async fn cancel_admin_action(
         &self,
         action_id: uuid::Uuid,
         community_id: CommunityId,
         report_id: uuid::Uuid,
+        cancelled_by: &[u8],
     ) -> Result<bool> {
-        relay_admin_actions::cancel_action(&self.pool, action_id, community_id, report_id).await
+        relay_admin_actions::cancel_action(
+            &self.pool,
+            action_id,
+            community_id,
+            report_id,
+            cancelled_by,
+        )
+        .await
     }
 
     /// Reopen a terminal report (resolved|dismissed|escalated → open) with a

crates/buzz-db/src/relay_admin_actions.rs

@@ -42,6 +42,8 @@ pub struct AdminActionRecord {
     pub state: String,
     /// Durably committed step: `None` = not started, `"mutation_committed"`, `"artifacts_done"`.
     pub step_marker: Option<String>,
+    /// Principal who cancelled this action (32-byte pubkey); `None` until cancelled.
+    pub cancelled_by: Option<Vec<u8>>,
     /// Error from the last failure, if any.
     pub error_message: Option<String>,
     /// Row creation time.
@@ -237,7 +239,7 @@ pub async fn claim_report(
         let existing = sqlx::query(
             r#"
             SELECT id, report_id, report_community_id, request_id, actor_pubkey, actor_role,
-                   action, reason, timeout_until, state, step_marker, error_message,
+                   action, reason, timeout_until, state, step_marker, cancelled_by, error_message,
                    created_at, updated_at
             FROM relay_admin_actions
             WHERE report_community_id = $1 AND report_id = $2 AND request_id = $3
@@ -269,7 +271,7 @@ pub async fn claim_report(
             action, reason, timeout_until, state
         ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, 'pending')
         RETURNING id, report_id, report_community_id, request_id, actor_pubkey, actor_role,
-                  action, reason, timeout_until, state, step_marker, error_message,
+                  action, reason, timeout_until, state, step_marker, cancelled_by, error_message,
                   created_at, updated_at
         "#,
     )
@@ -981,17 +983,19 @@ pub async fn cancel_action(
     action_id: Uuid,
     community_id: CommunityId,
     report_id: Uuid,
+    cancelled_by: &[u8],
 ) -> Result<bool> {
     let mut tx = pool.begin().await?;
 
     // Cancel only a pre-mutation `failed` action that BELONGS to the path
     // report and community. Fencing on report_id + report_community_id is what
     // blocks cross-report cancellation: `/reports/A/cancel {actionId:B}` matches
-    // zero rows because B's report_id is not A.
+    // zero rows because B's report_id is not A. `cancelled_by` attributes the
+    // transition — the one mutation that would otherwise carry no actor trail.
     let updated = sqlx::query(
         r#"
         UPDATE relay_admin_actions
-        SET state = 'cancelled', updated_at = now()
+        SET state = 'cancelled', cancelled_by = $4, updated_at = now()
         WHERE id = $1
           AND report_id = $2
           AND report_community_id = $3
@@ -1002,6 +1006,7 @@ pub async fn cancel_action(
     .bind(action_id)
     .bind(report_id)
     .bind(community_id.as_uuid())
+    .bind(cancelled_by)
     .execute(&mut *tx)
     .await?;
 
@@ -1167,7 +1172,7 @@ pub async fn get_action(pool: &PgPool, action_id: Uuid) -> Result<Option<AdminAc
     let row = sqlx::query(
         r#"
         SELECT id, report_id, report_community_id, request_id, actor_pubkey, actor_role,
-               action, reason, timeout_until, state, step_marker, error_message,
+               action, reason, timeout_until, state, step_marker, cancelled_by, error_message,
                created_at, updated_at
         FROM relay_admin_actions WHERE id = $1
         "#,
@@ -1188,7 +1193,7 @@ pub async fn get_action_by_request(
     let row = sqlx::query(
         r#"
         SELECT id, report_id, report_community_id, request_id, actor_pubkey, actor_role,
-               action, reason, timeout_until, state, step_marker, error_message,
+               action, reason, timeout_until, state, step_marker, cancelled_by, error_message,
                created_at, updated_at
         FROM relay_admin_actions
         WHERE report_community_id = $1 AND report_id = $2 AND request_id = $3
@@ -1446,7 +1451,7 @@ pub async fn claim_stranded_action_batch(
               AND (action_lease_expires_at IS NULL OR action_lease_expires_at < now())
             RETURNING id, report_id, report_community_id, request_id, actor_pubkey,
                       actor_role, action, reason, timeout_until, state, step_marker,
-                      error_message, created_at, updated_at
+                      cancelled_by, error_message, created_at, updated_at
             "#,
         )
         .bind(id)
@@ -1542,6 +1547,7 @@ fn row_to_action(row: sqlx::postgres::PgRow) -> Result<AdminActionRecord> {
         timeout_until: row.try_get("timeout_until")?,
         state: row.try_get("state")?,
         step_marker: row.try_get("step_marker")?,
+        cancelled_by: row.try_get("cancelled_by")?,
         error_message: row.try_get("error_message")?,
         created_at: row.try_get("created_at")?,
         updated_at: row.try_get("updated_at")?,
@@ -1823,6 +1829,7 @@ mod tests {
             action_id,
             CommunityId::from_uuid(community_id),
             report_id,
+            &[0_u8; 32],
         )
         .await
         .expect("cancel_action");

crates/buzz-relay/src/api/admin/auth.rs

@@ -182,9 +182,14 @@ pub async fn authorize(
         AdminAuth::Disabled => None,
         AdminAuth::Nip98 => {
             let full_path = format!("{ADMIN_API_PREFIX}{path_and_query}");
-            let pubkey_bytes =
-                authorize_nip98(state, config, headers, &full_path, method, raw_body).await?;
+            let (pubkey_bytes, event_id) =
+                authorize_nip98(config, headers, &full_path, method, raw_body).await?;
+            // Resolve the roster grant BEFORE claiming the replay ID: an
+            // unrostered-but-validly-signing key (e.g. any WARP-admitted laptop)
+            // must not be able to consume replay slots at request rate. Only a
+            // request that clears authorization claims its event ID.
             let principal = resolve_admin_principal(state, pubkey_bytes).await?;
+            claim_nip98_replay(state, &event_id).await?;
             Some(principal)
         }
     };
@@ -327,21 +332,25 @@ fn authorize_bearer(
 }
 
 /// Require exactly one `Authorization: Nostr <base64 event>` header, verify
-/// the NIP-98 event (method, url, payload hash for body-bearing methods),
-/// check the replay guard, and return the authenticated pubkey bytes.
+/// the NIP-98 event (method, url, payload hash for body-bearing methods), and
+/// return the authenticated pubkey bytes and event id.
+///
+/// This performs signature/URL/method/payload verification only — it does NOT
+/// claim the replay ID. The caller resolves the principal (roster check) first
+/// and calls [`claim_nip98_replay`] only after authorization succeeds, so an
+/// unrostered signer can never consume a replay slot.
 ///
 /// For body-bearing methods (`POST`/`PUT`/`PATCH`/`DELETE`), the `payload`
 /// sha256 tag is required. The body bytes are verified against it.
 ///
 /// Uniform 401 on any auth failure — no oracle distinguishing the failure mode.
 async fn authorize_nip98(
-    state: &AppState,
     config: &AdminConfig,
     headers: &HeaderMap,
     path: &str,
     method: &str,
     raw_body: Option<&[u8]>,
-) -> Result<[u8; 32], ApiError> {
+) -> Result<([u8; 32], nostr::EventId), ApiError> {
     let unauth = || ApiError::unauthorized().with_www_authenticate("Nostr");
 
     // 1. Extract exactly one Authorization: Nostr header.
@@ -388,31 +397,38 @@ async fn authorize_nip98(
     let pubkey =
         buzz_auth::verify_nip98_event(&event_json, &url, method, raw_body).map_err(|_| unauth())?;
 
-    // 6. Replay guard — deployment-scoped, consumed only after crypto verification.
-    //    Redis failure fails closed.
-    let event_id = nostr::EventId::from_byte_array(event_id_bytes);
+    Ok((
+        pubkey.to_bytes(),
+        nostr::EventId::from_byte_array(event_id_bytes),
+    ))
+}
+
+/// Atomically claim a verified NIP-98 event ID against the deployment-scoped
+/// replay guard. Called only after [`authorize_nip98`] verified the event and
+/// [`resolve_admin_principal`] confirmed a roster grant, so an unrostered
+/// signer never consumes a slot. Redis failure fails closed.
+async fn claim_nip98_replay(state: &AppState, event_id: &nostr::EventId) -> Result<(), ApiError> {
+    let unauth = || ApiError::unauthorized().with_www_authenticate("Nostr");
     match state
         .nip98_replay
         .try_mark_in_scope(
             ADMIN_REPLAY_SCOPE,
-            &event_id,
+            event_id,
             buzz_auth::DEFAULT_REPLAY_TTL_SECS,
         )
         .await
     {
-        Ok(true) => {}
-        Ok(false) => return Err(unauth()),
+        Ok(true) => Ok(()),
+        Ok(false) => Err(unauth()),
         Err(err) => {
             tracing::warn!(
                 scope = ADMIN_REPLAY_SCOPE,
                 error = %err,
                 "admin NIP-98 replay guard failed; rejecting request fail-closed"
             );
-            return Err(unauth());
+            Err(unauth())
         }
     }
-
-    Ok(pubkey.to_bytes())
 }
 
 /// Extract the credential from an `Authorization: Nostr <base64>` value.

crates/buzz-relay/src/api/admin/mod.rs

@@ -733,7 +733,7 @@ async fn cancel_report(
     )
     .await?;
 
-    let _principal = require_mutation_principal(principal_opt)?;
+    let principal = require_mutation_principal(principal_opt)?;
 
     let body: CancelReportBody = serde_json::from_slice(&body_bytes)
         .map_err(|_| ApiError::bad_request("invalid_body", "invalid JSON body"))?;
@@ -752,7 +752,12 @@ async fn cancel_report(
 
     let cancelled = state
         .db
-        .cancel_admin_action(body.action_id, tenant.community(), report_id)
+        .cancel_admin_action(
+            body.action_id,
+            tenant.community(),
+            report_id,
+            &principal.pubkey,
+        )
         .await?;
 
     if !cancelled {
@@ -1685,6 +1690,12 @@ mod tests {
                 seen: std::sync::Mutex::new(std::collections::HashSet::new()),
             }
         }
+
+        /// Number of distinct event IDs the guard has been asked to claim.
+        /// Zero proves the replay guard was never consulted.
+        fn claim_count(&self) -> usize {
+            self.seen.lock().unwrap().len()
+        }
     }
 
     impl buzz_auth::Nip98ReplayGuard for TrackingReplayGuard {
@@ -1921,6 +1932,43 @@ mod tests {
         assert_eq!(second.status(), StatusCode::UNAUTHORIZED);
     }
 
+    #[tokio::test]
+    async fn nip98_mode_unrostered_signer_does_not_consume_a_replay_slot() {
+        // Regression: the replay ID must be claimed only AFTER principal
+        // resolution succeeds. A validly-signing but unrostered key (any
+        // WARP-admitted laptop) must not be able to allocate replay slots at
+        // request rate. Signer is not in the config roster, so resolution falls
+        // through to the DB lookup and fails (403 with Postgres, 500 without) —
+        // either way the request is rejected and the replay guard is never
+        // consulted, so no slot is consumed.
+        let operator = nostr::Keys::generate();
+        let unrostered = nostr::Keys::generate();
+        let tracking = Arc::new(TrackingReplayGuard::new());
+        let state =
+            nip98_state_with_replay(vec![operator.public_key().to_hex()], tracking.clone()).await;
+        let auth = make_nostr_auth(&unrostered, "/probe");
+        let response = status_for(
+            state,
+            Request::builder()
+                .uri("/probe")
+                .header(header::HOST, "admin.example")
+                .header(header::AUTHORIZATION, auth)
+                .body(Body::empty())
+                .expect("request"),
+        )
+        .await;
+        assert_ne!(
+            response.status(),
+            StatusCode::OK,
+            "unrostered signer must be rejected"
+        );
+        assert_eq!(
+            tracking.claim_count(),
+            0,
+            "replay guard must not be consulted for an unrostered signer"
+        );
+    }
+
     #[tokio::test]
     async fn nip98_mode_valid_credential_on_wrong_host_is_forbidden_not_unauthorized() {
         let keys = nostr::Keys::generate();
@@ -3124,10 +3172,15 @@ mod tests {
             .await
             .expect("commit_mutation_step");
 
-        let cancelled =
-            buzz_db::relay_admin_actions::cancel_action(&pool, action_id, cid, report_id)
-                .await
-                .expect("cancel_action");
+        let cancelled = buzz_db::relay_admin_actions::cancel_action(
+            &pool,
+            action_id,
+            cid,
+            report_id,
+            &[0_u8; 32],
+        )
+        .await
+        .expect("cancel_action");
         assert!(
             !cancelled,
             "cancel after mutation_committed must be rejected"
@@ -3354,24 +3407,36 @@ mod tests {
             .expect("body");
         let json: serde_json::Value = serde_json::from_slice(&bytes).expect("json");
         assert_eq!(json["status"], "open");
-        // The last-look DTO embeds the just-cancelled action with status cancelled.
+        // The last-look DTO embeds the just-cancelled action with status cancelled,
+        // attributed to the signing operator.
         assert_eq!(json["activeAction"]["id"], action_id.to_string());
         assert_eq!(json["activeAction"]["status"], "cancelled");
+        assert_eq!(
+            json["activeAction"]["cancelledBy"],
+            keys.public_key().to_hex(),
+            "cancel must be attributed to the signing principal"
+        );
 
-        // DB evidence: action is cancelled and the report is back to open.
-        let (state_col, report_status): (String, String) = sqlx::query_as(
-            r#"
-            SELECT a.state, r.status
+        // DB evidence: action is cancelled, attributed, and the report is back to open.
+        let (state_col, cancelled_by, report_status): (String, Option<Vec<u8>>, String) =
+            sqlx::query_as(
+                r#"
+            SELECT a.state, a.cancelled_by, r.status
             FROM relay_admin_actions a
             JOIN moderation_reports r ON r.id = a.report_id
             WHERE a.id = $1
             "#,
-        )
-        .bind(action_id)
-        .fetch_one(&pool)
-        .await
-        .expect("read action + report");
+            )
+            .bind(action_id)
+            .fetch_one(&pool)
+            .await
+            .expect("read action + report");
         assert_eq!(state_col, "cancelled");
+        assert_eq!(
+            cancelled_by.map(hex::encode),
+            Some(keys.public_key().to_hex()),
+            "cancelled_by must persist the acting principal"
+        );
         assert_eq!(report_status, "open");
 
         cleanup_admin_host_report(&pool, report_id).await;

migrations/0033_relay_admin_actions.sql

@@ -27,6 +27,10 @@ CREATE TABLE relay_admin_actions (
                     CHECK (state IN ('pending', 'enforcing', 'succeeded', 'failed', 'cancelled')),
     -- Step marker: the last durably committed mutation step (NULL = none yet).
     step_marker     TEXT CHECK (step_marker IN ('mutation_committed', 'artifacts_done')),
+    -- Principal who cancelled a pre-mutation failed action; NULL until cancelled.
+    -- Attributes the cancel transition on the action row itself, mirroring
+    -- moderation_reports.resolved_by for report resolution.
+    cancelled_by    BYTEA CHECK (cancelled_by IS NULL OR length(cancelled_by) = 32),
     -- Error from the last failure, if any.
     error_message   TEXT,
     created_at      TIMESTAMPTZ NOT NULL DEFAULT now(),

schema/schema.sql

@@ -1715,6 +1715,10 @@ CREATE TABLE relay_admin_actions (
     -- Step marker: the last durably committed mutation step (NULL = none yet).
     -- Values: 'mutation_committed' (core DB mutation done), 'artifacts_done' (tombstone/notice done).
     step_marker     TEXT CHECK (step_marker IN ('mutation_committed', 'artifacts_done')),
+    -- Principal who cancelled a pre-mutation failed action; NULL until cancelled.
+    -- Attributes the cancel transition on the action row itself, mirroring
+    -- moderation_reports.resolved_by for report resolution.
+    cancelled_by    BYTEA CHECK (cancelled_by IS NULL OR length(cancelled_by) = 32),
     -- Error from the last failure, if any.
     error_message   TEXT,
     -- Per-action exclusive lease (migration 0034): fences concurrent same-request