diff --git a/README.md b/README.md index fe27847..ffc03b3 100644 --- a/README.md +++ b/README.md @@ -276,7 +276,7 @@ ngit-grasp implements multiple layers of defense against abuse, spam, and denial **Relay Sync Protection (GRASP-02):** - **Exponential backoff** - Failed connections: 5s → 10s → 20s → ... → 1 hour max -- **Naughty list** - Track relays with infrastructure issues separately (12h expiry) +- **Naughty list** - Suppress relays with infrastructure issues until expiry (12h default) - **Rate limit detection** - Auto 65s cooldown when remote relays rate limit us - **Domain throttling** - Max 5 concurrent, 30/min per domain for git data fetching diff --git a/docs/explanation/defensive-measures.md b/docs/explanation/defensive-measures.md index 34be641..666c653 100644 --- a/docs/explanation/defensive-measures.md +++ b/docs/explanation/defensive-measures.md @@ -89,7 +89,7 @@ These limits prevent individual connections from overwhelming the relay. - Tracks relays with persistent infrastructure issues (DNS, TLS, protocol errors) - Separate from normal connection failures - 12-hour expiration (configurable) -- Reduces retry frequency for broken relays +- Suppresses connection attempts until the entry expires **Rate Limit Detection:** - Detects when remote relay rate limits us diff --git a/src/sync/health.rs b/src/sync/health.rs index f82738b..8f47ac6 100644 --- a/src/sync/health.rs +++ b/src/sync/health.rs @@ -481,6 +481,14 @@ impl RelayHealthTracker { /// - The relay is healthy /// - The backoff period has elapsed pub fn should_attempt_connection(&self, relay_url: &str) -> bool { + if self + .naughty_list + .as_ref() + .is_some_and(|naughty_list| naughty_list.is_naughty(relay_url)) + { + return false; + } + let entry = self.health.get(relay_url); match entry { diff --git a/src/sync/mod.rs b/src/sync/mod.rs index 7b39487..f604180 100644 --- a/src/sync/mod.rs +++ b/src/sync/mod.rs @@ -3299,6 +3299,17 @@ impl SyncManager { return; } }; + if self + .health_tracker + .naughty_list() + .is_some_and(|naughty_list| naughty_list.is_naughty(&relay_url)) + { + tracing::debug!( + relay = %relay_url, + "Suppressing connection attempt for naughty relay" + ); + return; + } let Some(result_tx) = self.connect_attempt_result_tx.clone() else { tracing::error!(relay = %relay_url, "Connection scheduler is not running"); return; diff --git a/tests/sync.rs b/tests/sync.rs index b761032..6ddabf8 100644 --- a/tests/sync.rs +++ b/tests/sync.rs @@ -38,6 +38,7 @@ mod sync { pub mod live_sync; pub mod maintainer_reprocessing; pub mod metrics; + pub mod naughty_list_scheduling; pub mod neg_concurrency; pub mod purgatory_fetch; pub mod req_concurrency; diff --git a/tests/sync/mod.rs b/tests/sync/mod.rs index af0a8cf..4fbc8f4 100644 --- a/tests/sync/mod.rs +++ b/tests/sync/mod.rs @@ -138,4 +138,4 @@ pub mod metrics; pub mod neg_concurrency; pub mod purgatory_fetch; pub mod req_concurrency; -pub mod tag_variations; \ No newline at end of file +pub mod tag_variations; diff --git a/tests/sync/naughty_list_scheduling.rs b/tests/sync/naughty_list_scheduling.rs new file mode 100644 index 0000000..c5e9637 --- /dev/null +++ b/tests/sync/naughty_list_scheduling.rs @@ -0,0 +1,113 @@ +//! Scenario regression coverage for relay naughty-list scheduling. + +use std::path::Path; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::Arc; +use std::time::Duration; + +use crate::common::{TestClient, TestRelay}; +use nostr_sdk::prelude::*; +use tokio::io::AsyncWriteExt; + +const OBSERVATION_DEADLINE: Duration = Duration::from_secs(30); +const RECONNECT_OBSERVATION: Duration = Duration::from_secs(4); + +async fn wait_for_log_line(log_path: &Path, predicate: F) -> bool +where + F: Fn(&str) -> bool, +{ + let deadline = tokio::time::Instant::now() + OBSERVATION_DEADLINE; + loop { + if let Ok(content) = tokio::fs::read_to_string(log_path).await { + if content.lines().any(&predicate) { + return true; + } + } + if tokio::time::Instant::now() >= deadline { + return false; + } + tokio::time::sleep(Duration::from_millis(100)).await; + } +} + +/// A reachable TCP endpoint that deliberately violates the WebSocket +/// handshake. This produces a persistent protocol failure without relying on +/// external DNS or network state. +async fn start_broken_websocket() -> (String, Arc) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind broken websocket endpoint"); + let address = listener.local_addr().expect("broken endpoint address"); + let accepted = Arc::new(AtomicUsize::new(0)); + let task_accepted = Arc::clone(&accepted); + + tokio::spawn(async move { + while let Ok((mut stream, _)) = listener.accept().await { + task_accepted.fetch_add(1, Ordering::SeqCst); + let _ = stream.write_all(b"not a websocket handshake\r\n").await; + let _ = stream.shutdown().await; + } + }); + + (format!("ws://{address}"), accepted) +} + +#[tokio::test] +async fn naughty_relay_is_not_scheduled_for_reconnection() { + let relay = TestRelay::start_with_sync(None).await; + let (broken_relay, accepted) = start_broken_websocket().await; + let keys = Keys::generate(); + let identifier = "naughty-relay-scheduling"; + let npub = keys.public_key().to_bech32().expect("npub"); + let own_clone = format!("http://{}/{npub}/{identifier}.git", relay.domain()); + let own_relay = format!("ws://{}", relay.domain()); + let announcement = EventBuilder::new(Kind::GitRepoAnnouncement, "") + .tags(vec![ + Tag::identifier(identifier), + Tag::custom("clone", vec![own_clone]), + Tag::custom("relays", vec![own_relay, broken_relay.clone()]), + ]) + .finalize(&keys) + .expect("sign announcement"); + + let client = TestClient::new(relay.url(), keys) + .await + .expect("connect client"); + client + .send_event(&announcement) + .await + .expect("publish announcement"); + + assert!( + wait_for_log_line(&relay.log_path(), |line| { + line.contains("added to naughty list") && line.contains(&broken_relay) + }) + .await, + "broken endpoint must be classified as naughty" + ); + assert_eq!(accepted.load(Ordering::SeqCst), 1, "first dial is required"); + + let reconnect_deadline = tokio::time::Instant::now() + RECONNECT_OBSERVATION; + while tokio::time::Instant::now() < reconnect_deadline + && accepted.load(Ordering::SeqCst) == 1 + { + tokio::time::sleep(Duration::from_millis(100)).await; + } + assert_eq!( + accepted.load(Ordering::SeqCst), + 1, + "a naughty relay must not receive another dial across reconnect ticks" + ); + let log = tokio::fs::read_to_string(relay.log_path()) + .await + .expect("read relay log"); + assert!( + !log.lines().any(|line| { + line.contains("Attempting reconnection relay=") && line.contains(&broken_relay) + }), + "a naughty relay must be excluded before reconnect intent is logged" + ); + + client.disconnect().await; + relay.stop().await; +}