diff --git a/src/nostr/lifecycle/deletion/policy.rs b/src/nostr/lifecycle/deletion/policy.rs index ddc1a23..1237ebd 100644 --- a/src/nostr/lifecycle/deletion/policy.rs +++ b/src/nostr/lifecycle/deletion/policy.rs @@ -38,9 +38,10 @@ use nostr_relay_builder::prelude::{ WritePolicyResult, }; +use super::service::{request_is_served_with_config, UNSERVED_REPLAY_MESSAGE}; use super::DeletionContext; use crate::nostr::lifecycle::RequestClassification; -use crate::nostr::policy::{reject_error, reject_invalid}; +use crate::nostr::policy::{duplicate, reject_error, reject_invalid}; const MAX_DELETION_TARGET_TAGS: usize = 2048; /// Policy for handling NIP-09 event deletion requests @@ -92,12 +93,37 @@ impl DeletionPolicy { } else { RequestClassification::LocallyActionable }; - if let Err(e) = self - .ctx - .tombstones - .record_request(event, Timestamp::now(), classification) - .await - { + // Hold the lifecycle lock through the admission decision so cleanup + // cannot race an exact replay back into Main after its served deadline. + let tombstones = &self.ctx.tombstones; + let record_result = { + let _lifecycle_guard = tombstones.lock_lifecycle().await; + let existing = match tombstones.lifecycle_for_request_result(&event.id).await { + Ok(record) => record, + Err(error) => { + tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to read NIP-09 deletion lifecycle"); + return reject_error(format!( + "internal error reading deletion lifecycle: {error}" + )); + } + }; + if let Some(record) = existing { + match request_is_served_with_config(&self.ctx, &record, Timestamp::now()) { + Ok(false) => return duplicate(UNSERVED_REPLAY_MESSAGE), + Ok(true) => {} + Err(error) => { + tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to evaluate NIP-09 deletion lifecycle retention"); + return reject_error(format!( + "internal error evaluating deletion lifecycle retention: {error}" + )); + } + } + } + tombstones + .record_request_locked(event, Timestamp::now(), classification) + .await + }; + if let Err(e) = record_result { tracing::error!(event_id = %event.id.to_hex(), error = %e, "Failed to record NIP-09 deletion lifecycle"); return reject_error(format!("internal error recording deletion lifecycle: {e}")); } diff --git a/src/nostr/lifecycle/deletion/service.rs b/src/nostr/lifecycle/deletion/service.rs index 77bb8ca..f2e13c2 100644 --- a/src/nostr/lifecycle/deletion/service.rs +++ b/src/nostr/lifecycle/deletion/service.rs @@ -6,10 +6,36 @@ use nostr_relay_builder::prelude::{ use crate::nostr::events::RepositoryAnnouncement; use crate::nostr::lifecycle::{RequestClassification, RequestLifecycleRecord}; -use crate::nostr::policy::{reject_error, reject_invalid, AnnouncementResult}; +use crate::nostr::policy::{duplicate, reject_error, reject_invalid, AnnouncementResult}; use super::{DeletionContext, DeletionOutcome, DeletionPolicy}; +pub(super) const UNSERVED_REPLAY_MESSAGE: &str = + "deletion request is no longer served; it will be re-served when used to reject an in-scope event"; + +pub(super) fn request_is_served_with_config( + ctx: &DeletionContext, + record: &RequestLifecycleRecord, + now: Timestamp, +) -> Result { + let (anchor, duration) = match record.last_used_at { + Some(used) => ( + used, + ctx.config + .deletion_request_retention_used_served_after_last_used(), + ), + None => ( + record.first_seen_at, + ctx.config.deletion_request_retention_unused_served(), + ), + }; + let deadline = anchor + .as_secs() + .checked_add(duration.as_secs()) + .ok_or_else(|| anyhow::anyhow!("deletion-request served retention deadline overflow"))?; + Ok(now.as_secs() < deadline) +} + #[derive(Clone)] pub struct DeletionService { pub(super) ctx: DeletionContext, @@ -267,15 +293,39 @@ impl DeletionService { RequestClassification::LocallyActionable }; - // Persist every accepted NIP-62 request before accepting it or doing - // destructive work. `record_request` preserves receipt time and - // classification on an exact replay. - if let Err(error) = self - .ctx - .tombstones() - .record_request(event, Timestamp::now(), classification) - .await - { + // Keep the retention decision and lifecycle write atomic with cleanup. + // An exact replay during the unserved-but-gating interval must receive + // an OK duplicate response, not Accept: relay-builder persists Accept + // results back into Main and would make the request queryable again. + let tombstones = self.ctx.tombstones(); + let record_result = { + let _lifecycle_guard = tombstones.lock_lifecycle().await; + let existing = match tombstones.lifecycle_for_request_result(&event.id).await { + Ok(record) => record, + Err(error) => { + tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to read NIP-62 vanish lifecycle"); + return reject_error(format!( + "internal error reading vanish lifecycle: {error}" + )); + } + }; + if let Some(record) = existing { + match self.request_is_served(&record, Timestamp::now()) { + Ok(false) => return duplicate(UNSERVED_REPLAY_MESSAGE), + Ok(true) => {} + Err(error) => { + tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to evaluate NIP-62 vanish lifecycle retention"); + return reject_error(format!( + "internal error evaluating vanish lifecycle retention: {error}" + )); + } + } + } + tombstones + .record_request_locked(event, Timestamp::now(), classification) + .await + }; + if let Err(error) = record_result { tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to record NIP-62 vanish lifecycle"); return reject_error(format!( "internal error recording vanish lifecycle: {error}" diff --git a/tests/lifecycle/deletion_request_retention.rs b/tests/lifecycle/deletion_request_retention.rs index e0bc3ee..3e094bc 100644 --- a/tests/lifecycle/deletion_request_retention.rs +++ b/tests/lifecycle/deletion_request_retention.rs @@ -429,26 +429,57 @@ async fn nip62_targeting_gates_while_non_targeting_request_never_does() { } #[tokio::test] -async fn normal_mode_nip62_replay_from_unserved_gate_does_not_make_a_noop_request_used() { +async fn nip09_replay_from_unserved_gate_does_not_restore_its_served_copy() { let relay = TestRelay::start_with_deletion_lifecycle(lifecycle_options()).await; let client = AuditClient::new(relay.url(), AuditConfig::isolated()) .await .expect("create client"); - let request = build_vanish(&client, None); + let request = build_deletion(&client, &[], &[]); client .send_event(request.clone()) .await - .expect("accept initial no-op vanish request"); + .expect("accept initial deletion request"); + wait_until("deletion request moves to its unserved gate", || async { + !is_served(&client, request.id).await + }) + .await; + client + .send_event(request.clone()) + .await + .expect("exact replay receives the duplicate acknowledgement"); + assert!( + !is_served(&client, request.id).await, + "duplicate replay must not restore an unserved deletion request to Main" + ); + relay.stop().await; +} + +#[tokio::test] +async fn non_targeting_nip62_replay_from_unserved_gate_does_not_restore_its_served_copy() { + let relay = TestRelay::start_with_deletion_lifecycle(lifecycle_options()).await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create client"); + let request = build_vanish(&client, Some("wss://other.example")); + + client + .send_event(request.clone()) + .await + .expect("accept initial non-targeting vanish request"); wait_until( - "no-op vanish request moves to its unserved gate", + "non-targeting vanish request moves to its unserved gate", || async { !is_served(&client, request.id).await }, ) .await; client .send_event(request.clone()) .await - .expect("accept exact no-op vanish replay from its unserved gate"); + .expect("exact replay receives the duplicate acknowledgement"); + assert!( + !is_served(&client, request.id).await, + "duplicate replay must not restore an unserved non-targeting vanish request to Main" + ); let record = open_tombstones(relay.relay_data_path().clone()) .await @@ -458,7 +489,7 @@ async fn normal_mode_nip62_replay_from_unserved_gate_does_not_make_a_noop_reques .expect("retain request lifecycle"); assert_eq!( record.last_used_at, None, - "replaying a no-op vanish must not let it delete itself or count as use" + "replaying a non-targeting vanish must not count as use" ); relay.stop().await; }