diff --git a/src/api/erasure.rs b/src/api/erasure.rs index 6dad050..42b0174 100644 --- a/src/api/erasure.rs +++ b/src/api/erasure.rs @@ -91,12 +91,31 @@ pub struct ProofRequest { pub struct ErasureRequestResponse { pub erasure_id: String, pub status: String, - pub scheduled_for: String, } #[derive(Debug, Serialize)] pub struct StatusResponse { pub status: String, + /// When execution is scheduled (set on confirm, when grace starts). + #[serde(skip_serializing_if = "Option::is_none")] + pub scheduled_for: Option, +} + +/// Deterministic, unguessable erasure id for the absent-user path — STABLE across +/// repeated calls (keyed by the integrity secret), so it is indistinguishable +/// from a real get-or-create-stable id and cannot be probed as an existence +/// oracle. +fn synthetic_erasure_id(pubkey: &str, secret: &str) -> String { + let mut h = Sha256::new(); + h.update(secret.as_bytes()); + h.update([0x1f]); + h.update(b"erasure-synthetic"); + h.update([0x1f]); + h.update(pubkey.as_bytes()); + let digest = h.finalize(); + let mut bytes = [0u8; 16]; + bytes.copy_from_slice(&digest[..16]); + Uuid::from_bytes(bytes).to_string() } /// Verify a proof for `purpose` over `action_path`, consume the nonce, and @@ -175,7 +194,12 @@ pub async fn request_erasure( let grace = state.config.retention.grace_period_hours as i64; let (pubkey, user) = prove(&state, &body, PURPOSE_REQUEST, "/account/erasure", None).await?; - match user { + // Constant-shape across present/absent AND across repeated calls: a STABLE + // erasure_id (real for a user, deterministic-synthetic otherwise) and a fixed + // "pending" status. No scheduled_for here (it is meaningful only at confirm, + // when grace starts) — echoing a stable-vs-advancing timestamp would leak + // account existence. The real scheduled_for is returned by confirm. + let erasure_id = match user { Some(u) => { let req = state .db @@ -189,21 +213,14 @@ pub async fn request_erasure( ) .await; info!(erasure_id = %req.id, "erasure requested"); - Ok(Json(ErasureRequestResponse { - erasure_id: req.id.to_string(), - status: req.status, - scheduled_for: req.scheduled_for.to_rfc3339(), - })) + req.id.to_string() } - None => { - // Constant-shape: a synthetic id, no row, no audit. - Ok(Json(ErasureRequestResponse { - erasure_id: Uuid::new_v4().to_string(), - status: "pending".into(), - scheduled_for: (Utc::now() + chrono::Duration::hours(grace)).to_rfc3339(), - })) - } - } + None => synthetic_erasure_id(&pubkey, integrity_secret(&state)), + }; + Ok(Json(ErasureRequestResponse { + erasure_id, + status: "pending".into(), + })) } /// Phase 2: confirm (a second fresh proof bound to this `erasure_id`). Revokes @@ -243,7 +260,10 @@ pub async fn confirm_erasure( ) .await; info!(erasure_id = %req.id, "erasure confirmed"); - Ok(Json(StatusResponse { status: req.status })) + Ok(Json(StatusResponse { + status: req.status, + scheduled_for: Some(req.scheduled_for.to_rfc3339()), + })) } /// Cancel a live erasure during grace (a fresh `erasure_cancel` proof). @@ -278,7 +298,10 @@ pub async fn cancel_erasure( integrity_secret(&state), ) .await; - Ok(Json(StatusResponse { status: req.status })) + Ok(Json(StatusResponse { + status: req.status, + scheduled_for: None, + })) } #[derive(Debug, Serialize)] @@ -369,13 +392,10 @@ pub async fn run_erasure_sweep(state: &Arc, batch: i64) -> Result done += 1, + match state.db.execute_erasure(req.id, purge, secret).await { + Ok(true) => done += 1, + // false = another node/sweep already claimed it, or it was cancelled. + Ok(false) => {} Err(e) => warn!(erasure_id = %req.id, error = %e, "erasure execution failed"), } } diff --git a/src/config.rs b/src/config.rs index da1eae3..a261008 100644 --- a/src/config.rs +++ b/src/config.rs @@ -582,6 +582,13 @@ impl Config { "PUBLIC_API_URL is required when ERASURE_ENABLED is on (it binds the signed-action proof)", )); } + // Bound the grace window: a huge value panics Duration::hours and an + // overflow on the i64 cast could wrap negative (defeating grace). + if self.retention.grace_period_hours > 24 * 365 * 10 { + return Err(cfg_err( + "ERASURE_GRACE_PERIOD_HOURS must be <= 87600 (10 years)", + )); + } } // Per-enabled-chain infra config: hard error in production, warn in dev. diff --git a/src/infra/db/erasure.rs b/src/infra/db/erasure.rs index af05575..30a5524 100644 --- a/src/infra/db/erasure.rs +++ b/src/infra/db/erasure.rs @@ -137,16 +137,37 @@ impl Database { /// Execute a confirmed erasure in ONE fail-closed transaction: per-table /// policy + cascade delete + tombstone + hash-chained audit. `purge_login` /// chooses purge (default) vs anonymize for `login_events`. + /// + /// The FIRST statement atomically CLAIMS the row (`status='confirmed' -> + /// 'completed'`), re-asserting the precondition the sweeper's now-released + /// `SKIP LOCKED` lock can no longer guarantee. So a request cancelled after + /// the sweep, or a row another node already claimed, is skipped (returns + /// `false`) — no double-execute, no duplicate audit row, no executing a + /// cancelled request. On any later failure the whole tx rolls back (the claim + /// included), so it is retried next sweep. Returns whether it executed. #[instrument(skip(self, secret))] pub async fn execute_erasure( &self, id: Uuid, - user_id: Uuid, purge_login: bool, secret: &str, - ) -> Result<(), AppError> { + ) -> Result { let mut tx = self.pool().begin().await?; + // Atomic claim + tombstone: only the first executor of a still-confirmed + // request proceeds. `user_id` is still set here (delete happens below). + let claimed: Option<(Option,)> = sqlx::query_as( + "UPDATE erasure_requests SET status = 'completed', completed_at = NOW() \ + WHERE id = $1 AND status = 'confirmed' RETURNING user_id", + ) + .bind(id) + .fetch_optional(&mut *tx) + .await?; + let Some((Some(user_id),)) = claimed else { + tx.rollback().await?; + return Ok(false); + }; + // login_events: the (origin, asset, time) quasi-identifier — the cascade // only nulls user_id, so purge (default) or anonymize all three here. if purge_login { @@ -174,20 +195,13 @@ impl Database { .await?; // Structural delete: cascades wallets/sessions/user_keys/owned slatepacks; - // SET NULL on counterparty/audit/login links (incl. this tombstone row). + // SET NULL on counterparty/audit/login links (incl. this row's user_id, + // leaving it a completed tombstone with the link scrubbed). sqlx::query("DELETE FROM users WHERE id = $1") .bind(user_id) .execute(&mut *tx) .await?; - // Tombstone (user_id was just nulled by the cascade). - sqlx::query( - "UPDATE erasure_requests SET status = 'completed', completed_at = NOW() WHERE id = $1", - ) - .bind(id) - .execute(&mut *tx) - .await?; - self.append_admin_audit( &mut tx, &NewAdminAudit { @@ -203,6 +217,6 @@ impl Database { .await?; tx.commit().await?; - Ok(()) + Ok(true) } } diff --git a/tests/erasure.rs b/tests/erasure.rs index 3221bd0..f28606b 100644 --- a/tests/erasure.rs +++ b/tests/erasure.rs @@ -26,6 +26,21 @@ fn random_signer() -> SigningKey { } } +async fn mk_user(db: &smirk_backend_core::infra::db::Database, nostr_pubkey: &str) -> uuid::Uuid { + db.create_user(NewUser { + username: None, + pubkey_hash: Some(format!("pk-{}", uuid::Uuid::new_v4())), + nostr_pubkey: Some(nostr_pubkey.to_string()), + wallet_birthday: None, + seed_fingerprint: None, + xmr_start_height: None, + wow_start_height: None, + }) + .await + .unwrap() + .id +} + /// Build a `Nostr ` signed-action token for an erasure purpose. fn sign( sk: &SigningKey, @@ -83,19 +98,7 @@ async fn erasure_request_confirm_execute_flow() { // A user with a linked nostr identity that we control the key for. let sk = random_signer(); let pk = hex::encode(sk.verifying_key().to_bytes()); - app.state - .db - .create_user(NewUser { - username: None, - pubkey_hash: Some(format!("pk-{}", uuid::Uuid::new_v4())), - nostr_pubkey: Some(pk.clone()), - wallet_birthday: None, - seed_fingerprint: None, - xmr_start_height: None, - wow_start_height: None, - }) - .await - .unwrap(); + mk_user(&app.state.db, &pk).await; let challenge = |purpose: &'static str| { let app = &app; @@ -218,6 +221,65 @@ async fn erasure_request_confirm_execute_flow() { assert!(body["erasure_id"].as_str().is_some()); assert_eq!(body["status"], "pending"); + let secret = app.state.config.admin.key_integrity_secret.clone(); + + // [HIGH regression] a request CANCELLED after confirmation is NOT executed: + // execute claims `WHERE status='confirmed'`, so a cancelled row is skipped. + let pk_c = hex::encode(random_signer().verifying_key().to_bytes()); + let uc = mk_user(&app.state.db, &pk_c).await; + let rc = app.state.db.request_erasure(uc, "h", 0).await.unwrap(); + app.state + .db + .confirm_erasure_request(rc.id, uc, 0) + .await + .unwrap(); + app.state + .db + .cancel_erasure_request(rc.id, uc) + .await + .unwrap(); + assert!( + !app.state + .db + .execute_erasure(rc.id, true, &secret) + .await + .unwrap(), + "cancelled request is not executed" + ); + assert!( + app.state + .db + .find_user_by_nostr_pubkey(&pk_c) + .await + .unwrap() + .is_some(), + "cancelled user survives" + ); + + // Double-execution is a no-op (idempotent claim): the second call returns false. + let pk_d = hex::encode(random_signer().verifying_key().to_bytes()); + let ud = mk_user(&app.state.db, &pk_d).await; + let rd = app.state.db.request_erasure(ud, "h", 0).await.unwrap(); + app.state + .db + .confirm_erasure_request(rd.id, ud, 0) + .await + .unwrap(); + assert!(app + .state + .db + .execute_erasure(rd.id, true, &secret) + .await + .unwrap()); + assert!( + !app.state + .db + .execute_erasure(rd.id, true, &secret) + .await + .unwrap(), + "second execute is a no-op" + ); + for k in [ "ERASURE_ENABLED", "ADMIN_KEY_INTEGRITY_SECRET",