diff --git a/crates/buzz-db/src/admin_moderation.rs b/crates/buzz-db/src/admin_moderation.rs index c255a8eb4..3dc8bd94c 100644 --- a/crates/buzz-db/src/admin_moderation.rs +++ b/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, /// Operator reason, if provided. pub reason: Option, /// 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 Result Result>, _>("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), diff --git a/crates/buzz-db/src/lib.rs b/crates/buzz-db/src/lib.rs index 9dfe8d98f..00833b47c 100644 --- a/crates/buzz-db/src/lib.rs +++ b/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 { - 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 diff --git a/crates/buzz-db/src/relay_admin_actions.rs b/crates/buzz-db/src/relay_admin_actions.rs index 4bbd8c8fa..a455f2dc3 100644 --- a/crates/buzz-db/src/relay_admin_actions.rs +++ b/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, + /// Principal who cancelled this action (32-byte pubkey); `None` until cancelled. + pub cancelled_by: Option>, /// Error from the last failure, if any. pub error_message: Option, /// 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 { 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 Result { 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"); diff --git a/crates/buzz-relay/src/api/admin/auth.rs b/crates/buzz-relay/src/api/admin/auth.rs index 93fc55a8c..df793b900 100644 --- a/crates/buzz-relay/src/api/admin/auth.rs +++ b/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 ` 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 ` value. diff --git a/crates/buzz-relay/src/api/admin/mod.rs b/crates/buzz-relay/src/api/admin/mod.rs index 3ae4fbce9..cae35158e 100644 --- a/crates/buzz-relay/src/api/admin/mod.rs +++ b/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>, 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; diff --git a/migrations/0033_relay_admin_actions.sql b/migrations/0033_relay_admin_actions.sql index dc4c89d37..cede28318 100644 --- a/migrations/0033_relay_admin_actions.sql +++ b/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(), diff --git a/schema/schema.sql b/schema/schema.sql index 9108a518e..e53c0e850 100644 --- a/schema/schema.sql +++ b/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