fix: reject unserved deletion request replays

This commit is contained in:
DanConwayDev
2026-07-17 13:25:08 +01:00
parent 782f3f1334
commit 6cd54eaf15
3 changed files with 130 additions and 23 deletions
+32 -6
View File
@@ -38,9 +38,10 @@ use nostr_relay_builder::prelude::{
WritePolicyResult, WritePolicyResult,
}; };
use super::service::{request_is_served_with_config, UNSERVED_REPLAY_MESSAGE};
use super::DeletionContext; use super::DeletionContext;
use crate::nostr::lifecycle::RequestClassification; 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; const MAX_DELETION_TARGET_TAGS: usize = 2048;
/// Policy for handling NIP-09 event deletion requests /// Policy for handling NIP-09 event deletion requests
@@ -92,12 +93,37 @@ impl DeletionPolicy {
} else { } else {
RequestClassification::LocallyActionable RequestClassification::LocallyActionable
}; };
if let Err(e) = self // Hold the lifecycle lock through the admission decision so cleanup
.ctx // cannot race an exact replay back into Main after its served deadline.
.tombstones let tombstones = &self.ctx.tombstones;
.record_request(event, Timestamp::now(), classification) 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 .await
{ };
if let Err(e) = record_result {
tracing::error!(event_id = %event.id.to_hex(), error = %e, "Failed to record NIP-09 deletion lifecycle"); 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}")); return reject_error(format!("internal error recording deletion lifecycle: {e}"));
} }
+59 -9
View File
@@ -6,10 +6,36 @@ use nostr_relay_builder::prelude::{
use crate::nostr::events::RepositoryAnnouncement; use crate::nostr::events::RepositoryAnnouncement;
use crate::nostr::lifecycle::{RequestClassification, RequestLifecycleRecord}; 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}; 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<bool> {
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)] #[derive(Clone)]
pub struct DeletionService { pub struct DeletionService {
pub(super) ctx: DeletionContext, pub(super) ctx: DeletionContext,
@@ -267,15 +293,39 @@ impl DeletionService {
RequestClassification::LocallyActionable RequestClassification::LocallyActionable
}; };
// Persist every accepted NIP-62 request before accepting it or doing // Keep the retention decision and lifecycle write atomic with cleanup.
// destructive work. `record_request` preserves receipt time and // An exact replay during the unserved-but-gating interval must receive
// classification on an exact replay. // an OK duplicate response, not Accept: relay-builder persists Accept
if let Err(error) = self // results back into Main and would make the request queryable again.
.ctx let tombstones = self.ctx.tombstones();
.tombstones() let record_result = {
.record_request(event, Timestamp::now(), classification) 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 .await
{ };
if let Err(error) = record_result {
tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to record NIP-62 vanish lifecycle"); tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to record NIP-62 vanish lifecycle");
return reject_error(format!( return reject_error(format!(
"internal error recording vanish lifecycle: {error}" "internal error recording vanish lifecycle: {error}"
+37 -6
View File
@@ -429,26 +429,57 @@ async fn nip62_targeting_gates_while_non_targeting_request_never_does() {
} }
#[tokio::test] #[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 relay = TestRelay::start_with_deletion_lifecycle(lifecycle_options()).await;
let client = AuditClient::new(relay.url(), AuditConfig::isolated()) let client = AuditClient::new(relay.url(), AuditConfig::isolated())
.await .await
.expect("create client"); .expect("create client");
let request = build_vanish(&client, None); let request = build_deletion(&client, &[], &[]);
client client
.send_event(request.clone()) .send_event(request.clone())
.await .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( 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 }, || async { !is_served(&client, request.id).await },
) )
.await; .await;
client client
.send_event(request.clone()) .send_event(request.clone())
.await .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()) let record = open_tombstones(relay.relay_data_path().clone())
.await .await
@@ -458,7 +489,7 @@ async fn normal_mode_nip62_replay_from_unserved_gate_does_not_make_a_noop_reques
.expect("retain request lifecycle"); .expect("retain request lifecycle");
assert_eq!( assert_eq!(
record.last_used_at, None, 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; relay.stop().await;
} }