From ce9a5774d013d87b366415be75c6db6c616a9eff Mon Sep 17 00:00:00 2001 From: Tori Date: Mon, 20 Jul 2026 17:04:47 -0500 Subject: [PATCH 1/4] ci(mutation): cap cargo-mutants concurrency to avoid OOM Uncapped parallel jobs + per-test thread fan-out exhausted RAM and crashed the machine during a local run. Cap via CARGO_MUTANTS_JOBS=2 (Makefile, verified with strace since .cargo/config.toml's [env] does not propagate to third-party subcommands) and --test-threads=4 (.cargo/mutants.toml). Both CI mutation jobs now go through the same `make mutation-test` target. --- .cargo/mutants.toml | 4 ++++ .github/workflows/mutation.yml | 4 ++-- Makefile | 4 ++++ 3 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 .cargo/mutants.toml diff --git a/.cargo/mutants.toml b/.cargo/mutants.toml new file mode 100644 index 00000000..f95d399a --- /dev/null +++ b/.cargo/mutants.toml @@ -0,0 +1,4 @@ +# Cap test-thread fan-out per mutant run. Without this, each cargo-mutants +# job spawns a test binary with --test-threads = num_cpus, which multiplies +# with CARGO_MUTANTS_JOBS (see `make mutation-test`) and can exhaust RAM. +additional_cargo_test_args = ["--test-threads=4"] diff --git a/.github/workflows/mutation.yml b/.github/workflows/mutation.yml index 818255f1..dea55947 100644 --- a/.github/workflows/mutation.yml +++ b/.github/workflows/mutation.yml @@ -68,7 +68,7 @@ jobs: echo "Running mutation testing for changed files:" echo "$changed_rs" - cargo mutants $file_args + make mutation-test ARGS="$file_args" - name: Upload mutation report uses: actions/upload-artifact@v4 @@ -195,7 +195,7 @@ jobs: tool: cargo-mutants - name: Run full mutation testing - run: cargo mutants + run: make mutation-test # Note: Do NOT fail on low score initially (report only mode) continue-on-error: true diff --git a/Makefile b/Makefile index 348bfa2f..094f1993 100644 --- a/Makefile +++ b/Makefile @@ -66,3 +66,7 @@ docker-build-startos: cd docker && \ docker compose build mostro-startos +mutation-test: + @set -o pipefail; \ + CARGO_MUTANTS_JOBS=2 cargo mutants $(ARGS) + From e8f50c5ffd6380620c131dc83ef7c186417a74af Mon Sep 17 00:00:00 2001 From: Tori Date: Thu, 30 Jul 2026 11:02:38 -0500 Subject: [PATCH 2/4] fix(mutation-test): make the local LN test port overridable cargo-mutants requires a fully green baseline before mutating anything, and the lightning-address test path was hardcoded to 127.0.0.1:8080 in both the test server and lnurl.rs's cfg!(test) URL builder. On a machine already using 8080 that baseline never passes. MOSTRO_TEST_LN_PORT (env, default 8080) lets `make mutation-test` point both sides at a free port without changing default `cargo test` behavior. Also drops --test-threads=4 from the mutation-test target: cargo-mutants 27.1.0 has no working way to forward libtest args (config key, CLI flag, and trailing -- args all insert before cargo test's own --), so the flag only broke the baseline. CARGO_MUTANTS_JOBS=2 remains the real OOM cap. --- .cargo/mutants.toml | 4 ---- Makefile | 2 +- src/lightning/invoice.rs | 7 ++++++- src/lnurl.rs | 22 +++++++++++++++++++--- 4 files changed, 26 insertions(+), 9 deletions(-) delete mode 100644 .cargo/mutants.toml diff --git a/.cargo/mutants.toml b/.cargo/mutants.toml deleted file mode 100644 index f95d399a..00000000 --- a/.cargo/mutants.toml +++ /dev/null @@ -1,4 +0,0 @@ -# Cap test-thread fan-out per mutant run. Without this, each cargo-mutants -# job spawns a test binary with --test-threads = num_cpus, which multiplies -# with CARGO_MUTANTS_JOBS (see `make mutation-test`) and can exhaust RAM. -additional_cargo_test_args = ["--test-threads=4"] diff --git a/Makefile b/Makefile index 094f1993..08ef928c 100644 --- a/Makefile +++ b/Makefile @@ -68,5 +68,5 @@ docker-build-startos: mutation-test: @set -o pipefail; \ - CARGO_MUTANTS_JOBS=2 cargo mutants $(ARGS) + CARGO_MUTANTS_JOBS=2 MOSTRO_TEST_LN_PORT=18080 cargo mutants $(ARGS) diff --git a/src/lightning/invoice.rs b/src/lightning/invoice.rs index abd4664e..f66ec1ae 100644 --- a/src/lightning/invoice.rs +++ b/src/lightning/invoice.rs @@ -243,7 +243,12 @@ mod tests { ) .layer(tower_http::cors::CorsLayer::permissive()); - let listener = TcpListener::bind("127.0.0.1:8080").await.unwrap(); + // Must match lnurl.rs's MOSTRO_TEST_LN_PORT override (default 8080). + let port = std::env::var("MOSTRO_TEST_LN_PORT") + .ok() + .and_then(|v| v.parse::().ok()) + .unwrap_or(8080); + let listener = TcpListener::bind(("127.0.0.1", port)).await.unwrap(); let addr = listener.local_addr().unwrap(); let port = addr.port(); diff --git a/src/lnurl.rs b/src/lnurl.rs index c1c2175e..c93d9f84 100644 --- a/src/lnurl.rs +++ b/src/lnurl.rs @@ -39,7 +39,14 @@ async fn extract_lnurl(address: &str) -> Result { None => return Err(MostroInternalErr(ServiceError::LnAddressParseError)), }; let base_url = if cfg!(test) { - format!("http://{domain}:8080") + // Overridable so test runs can dodge a port already taken on the + // host (e.g. cargo-mutants) without disturbing the 8080 default + // other tests assert on. + let port = std::env::var("MOSTRO_TEST_LN_PORT") + .ok() + .and_then(|v| v.parse::().ok()) + .unwrap_or(8080); + format!("http://{domain}:{port}") } else { format!("https://{domain}") }; @@ -260,11 +267,20 @@ mod tests { #[tokio::test] async fn extract_lnurl_builds_wellknown_url_for_lightning_address() { - // cfg!(test) pins lightning addresses to the local test host form. + // cfg!(test) pins lightning addresses to the local test host form, + // on MOSTRO_TEST_LN_PORT (default 8080) so this stays correct under + // a port override (e.g. cargo-mutants dodging a taken 8080). + let port = std::env::var("MOSTRO_TEST_LN_PORT") + .ok() + .and_then(|v| v.parse::().ok()) + .unwrap_or(8080); let extracted = extract_lnurl("alice@127.0.0.1") .await .expect("lightning address parses"); - assert_eq!(extracted, "http://127.0.0.1:8080/.well-known/lnurlp/alice"); + assert_eq!( + extracted, + format!("http://127.0.0.1:{port}/.well-known/lnurlp/alice") + ); } #[tokio::test] From 0d818b098e4a5dee8297d89eb6f167ec3ee8cdd3 Mon Sep 17 00:00:00 2001 From: Tori Date: Thu, 30 Jul 2026 11:02:52 -0500 Subject: [PATCH 3/4] test(app): cover accept_event's identity/timestamp/signature checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #636 (mutation testing for Nostr event handling) flagged accept_event()'s post-unwrap validation as uncovered: the replay-window timestamp check and the identity/signature check had zero mutants caught. Extracted both into pure functions (is_stale, missing_inner_signature) so cargo-mutants' boundary mutants can be hit directly, and added three accept_event-level tests built on a real wrap_message_with-signed GiftWrap event (happy path, wrong kind, wrong receiver) to cover the surrounding POW/kind/verify gates. Mutation run on src/app.rs + src/nip33.rs (partial, 90/108 mutants tested before this checkpoint): every accept_event mutant now caught except the 3 in the is_v2 spam-gate branch, deliberately deferred since exercising it needs the SpamGate global singleton initialized. Overall score and the rest of app.rs's dispatcher/warning_msg mutants still outstanding — see [[project_july_contribution_plan]] memory for the resume point. --- src/app.rs | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 173 insertions(+), 2 deletions(-) diff --git a/src/app.rs b/src/app.rs index 938641a6..711e982e 100644 --- a/src/app.rs +++ b/src/app.rs @@ -292,6 +292,24 @@ async fn handle_message_action( } } +/// True when a rumor's `created_at` falls outside the 10s replay window. +/// Pulled out of `accept_event` so the boundary can be hit directly instead +/// of only through a live wrapped event. +fn is_stale(created_at: Timestamp, since_time: u64) -> bool { + created_at.as_secs() < since_time +} + +/// True when the inner rumor is missing a required signature. Full-privacy +/// clients reuse the trade key as identity and send unsigned rumors; any +/// other identity/sender split must carry a valid inner signature. +fn missing_inner_signature( + identity: PublicKey, + sender: PublicKey, + signature: Option, +) -> bool { + identity != sender && signature.is_none() +} + /// Decode and fully validate one relay event into a dispatchable /// `(action, message, unwrapped)` triple, or `None` if it must be skipped /// (failed PoW, wrong kind, spam-gate drop, decrypt failure, stale, missing @@ -375,7 +393,7 @@ async fn accept_event( .checked_sub_signed(chrono::Duration::seconds(10)) .unwrap() .timestamp() as u64; - if unwrapped.created_at.as_secs() < since_time { + if is_stale(unwrapped.created_at, since_time) { return None; } let message = unwrapped.message.clone(); @@ -384,7 +402,7 @@ async fn accept_event( // unsigned rumors. Any other shape must carry a valid inner // signature — unwrap_message already verified it, so if identity // and sender differ here without a signature we bail out. - if unwrapped.identity != unwrapped.sender && unwrapped.signature.is_none() { + if missing_inner_signature(unwrapped.identity, unwrapped.sender, unwrapped.signature) { tracing::warn!( "Missing inner signature: identity {} differs from trade key {}", unwrapped.identity, @@ -610,6 +628,46 @@ mod tests { } } + #[test] + fn is_stale_rejects_events_older_than_the_cutoff() { + assert!(is_stale(Timestamp::from(99), 100)); + } + + #[test] + fn is_stale_accepts_events_exactly_at_the_cutoff() { + assert!(!is_stale(Timestamp::from(100), 100)); + } + + #[test] + fn is_stale_accepts_events_newer_than_the_cutoff() { + assert!(!is_stale(Timestamp::from(101), 100)); + } + + #[test] + fn missing_inner_signature_true_when_identity_and_sender_differ_unsigned() { + let identity = create_test_keys().public_key(); + let sender = create_test_keys().public_key(); + assert!(missing_inner_signature(identity, sender, None)); + } + + #[test] + fn missing_inner_signature_false_when_identity_equals_sender_unsigned() { + let same = create_test_keys().public_key(); + assert!(!missing_inner_signature(same, same, None)); + } + + #[test] + fn missing_inner_signature_false_when_signed_even_if_identity_differs() { + let identity = create_test_keys().public_key(); + let sender_keys = create_test_keys(); + let sig = sender_keys.sign_schnorr(&nostr_sdk::secp256k1::Message::from_digest([7u8; 32])); + assert!(!missing_inner_signature( + identity, + sender_keys.public_key(), + Some(sig) + )); + } + #[test] fn test_warning_msg_all_error_types() { let action = Action::NewOrder; @@ -866,6 +924,119 @@ mod tests { } } + mod accept_event_tests { + use super::*; + use crate::app::context::test_utils::{test_settings, TestContextBuilder}; + use mostro_core::prelude::*; + use sqlx::SqlitePool; + use std::sync::Arc; + + async fn create_test_ctx() -> AppContext { + let pool = Arc::new(SqlitePool::connect(":memory:").await.unwrap()); + TestContextBuilder::new() + .with_pool(pool) + .with_settings(test_settings()) + .build() + } + + #[allow(deprecated)] // Transport::GiftWrap: still the tested default; see #786. + async fn wrap_test_order( + identity_keys: &Keys, + trade_keys: &Keys, + receiver: PublicKey, + ) -> Event { + // RestoreSession is the one action whose payload is required to + // be None (mostro-core's `verify()`), which keeps this helper + // free of a hand-built `Payload::Order`. + wrap_message_with( + Transport::GiftWrap, + &create_test_message(Action::RestoreSession, None), + identity_keys, + trade_keys, + receiver, + WrapOptions::default(), + ) + .await + .expect("gift wrap succeeds") + } + + #[tokio::test] + async fn accepts_a_validly_wrapped_event_and_returns_the_action() { + let ctx = create_test_ctx().await; + let identity_keys = create_test_keys(); + let trade_keys = create_test_keys(); + let receiver_keys = create_test_keys(); + let event = + wrap_test_order(&identity_keys, &trade_keys, receiver_keys.public_key()).await; + + let result = accept_event( + &ctx, + &event, + &receiver_keys, + 0, + 0, + NostrKind::GiftWrap, + false, + ) + .await; + + let (action, _message, unwrapped) = result.expect("valid event should be accepted"); + assert_eq!(action, Action::RestoreSession); + assert_eq!(unwrapped.sender, trade_keys.public_key()); + assert_eq!(unwrapped.identity, identity_keys.public_key()); + } + + #[tokio::test] + async fn rejects_an_event_of_the_wrong_kind() { + let ctx = create_test_ctx().await; + let identity_keys = create_test_keys(); + let trade_keys = create_test_keys(); + let receiver_keys = create_test_keys(); + let event = + wrap_test_order(&identity_keys, &trade_keys, receiver_keys.public_key()).await; + + // The event is a real, validly-signed GiftWrap; accepted_kind is + // deliberately mismatched to exercise the kind-gate on its own. + let result = accept_event( + &ctx, + &event, + &receiver_keys, + 0, + 0, + NostrKind::TextNote, + false, + ) + .await; + + assert!(result.is_none()); + } + + #[tokio::test] + async fn rejects_an_event_not_addressed_to_the_receiver() { + let ctx = create_test_ctx().await; + let identity_keys = create_test_keys(); + let trade_keys = create_test_keys(); + let intended_receiver = create_test_keys(); + let eavesdropper = create_test_keys(); + let event = + wrap_test_order(&identity_keys, &trade_keys, intended_receiver.public_key()).await; + + // unwrap_incoming can't decrypt a rumor sealed for someone else. + let result = accept_event( + &ctx, + &event, + &eavesdropper, + 0, + 0, + NostrKind::GiftWrap, + false, + ) + .await; + + assert!(result.is_none()); + } + } + mod handle_message_action_tests { use super::*; use crate::app::context::test_utils::{test_settings, TestContextBuilder}; From 1c0e9a989235feb46ac5636e7e2bf508c0f92569 Mon Sep 17 00:00:00 2001 From: Tori Date: Thu, 30 Jul 2026 17:13:36 -0500 Subject: [PATCH 4/4] test(nostr): kill accept_event spam-gate and create_event expiration mutants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Targets the remaining surviving mutants on the Nostr event-handling path for issue #636: - accept_event's protocol-v2 spam-gate / PoW-first-contact branch (3 mutants around the `!gate.is_known(..) && !event.check_pow(..)` check) was never exercised by is_v2=true — add tests for the accepted and rejected first-contact cases. - nip33::create_event's NIP-40 expiration-tag dedup check (`||` between the canonical and custom expiration tag kinds) was only exercised via the "no existing tag" paths — add a test that pre-supplies a real expiration tag and asserts it isn't duplicated. spam_gate.rs: fix a latent test-order bug the new accept_event tests exposed — install_global_then_second_install_is_rejected assumed it would always be the first test in the binary to install the process-wide SpamGate OnceLock, which broke once another test raced it there. 10/10 targeted mutants confirmed killed via a `-F`-filtered cargo-mutants run scoped to accept_event/create_event. --- src/app.rs | 65 ++++++++++++++++++++++++++++++++++++++++++++++++ src/nip33.rs | 25 +++++++++++++++++++ src/spam_gate.rs | 15 ++++++----- 3 files changed, 99 insertions(+), 6 deletions(-) diff --git a/src/app.rs b/src/app.rs index 711e982e..ddc304e5 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1035,6 +1035,71 @@ mod tests { assert!(result.is_none()); } + + /// Ensures a spam gate is installed for this binary run without + /// clobbering one another test already installed — `SPAM_GATE` is a + /// process-wide `OnceLock`, so at most one instance ever wins, and + /// every test only relies on `is_known` being false for its own + /// freshly-generated (never-registered) pubkey. + fn ensure_spam_gate_installed() { + let _ = crate::spam_gate::SpamGate::new(crate::spam_gate::REPLAY_WINDOW_SECS) + .install_global(); + } + + #[tokio::test] + async fn unknown_first_contact_sender_clearing_the_pow_bar_is_accepted() { + ensure_spam_gate_installed(); + let ctx = create_test_ctx().await; + let identity_keys = create_test_keys(); + let trade_keys = create_test_keys(); + let receiver_keys = create_test_keys(); + let event = + wrap_test_order(&identity_keys, &trade_keys, receiver_keys.public_key()).await; + + // is_v2 = true routes through the spam gate; a never-seen sender + // pubkey is always unknown, and pow_first_contact = 0 is + // trivially cleared, so the first-contact lane must let it + // through. + let result = accept_event( + &ctx, + &event, + &receiver_keys, + 0, + 0, + NostrKind::GiftWrap, + true, + ) + .await; + + assert!(result.is_some()); + } + + #[tokio::test] + async fn unknown_first_contact_sender_below_the_pow_bar_is_dropped() { + ensure_spam_gate_installed(); + let ctx = create_test_ctx().await; + let identity_keys = create_test_keys(); + let trade_keys = create_test_keys(); + let receiver_keys = create_test_keys(); + let event = + wrap_test_order(&identity_keys, &trade_keys, receiver_keys.public_key()).await; + + // pow_first_contact = u8::MAX demands more leading-zero bits than + // any real event id will ever have, so this deterministically + // fails the first-contact PoW toll. + let result = accept_event( + &ctx, + &event, + &receiver_keys, + 0, + u8::MAX, + NostrKind::GiftWrap, + true, + ) + .await; + + assert!(result.is_none()); + } } mod handle_message_action_tests { diff --git a/src/nip33.rs b/src/nip33.rs index 2d0a26ad..cf55fa20 100644 --- a/src/nip33.rs +++ b/src/nip33.rs @@ -1117,6 +1117,31 @@ mod tests { ); } + #[test] + fn create_event_does_not_duplicate_a_caller_supplied_expiration_tag() { + // Order events (kind 38383) always get an auto expiration tag from + // config when one isn't already present. Pre-supplying a real NIP-40 + // `TagKind::Expiration` tag must suppress the auto-add, exercising + // the `||` in create_event's has_expiration_tag check on its + // `TagKind::Expiration` arm rather than only the `Custom("expiration")` arm. + init_test_settings(); + let keys = Keys::generate(); + let extra_tags = Tags::from_list(vec![Tag::expiration(Timestamp::from(123_456_u64))]); + + let order = super::new_order_event(&keys, "", "order-id".to_string(), extra_tags) + .expect("order event"); + + let expiration_tags = order + .tags + .iter() + .filter(|t| matches!(t.kind(), TagKind::Expiration)) + .count(); + assert_eq!( + expiration_tags, 1, + "caller-supplied expiration tag must not be duplicated" + ); + } + // ── create_rating_tag ──────────────────────────────────────────────── #[test] diff --git a/src/spam_gate.rs b/src/spam_gate.rs index 0de5bf46..936d00c4 100644 --- a/src/spam_gate.rs +++ b/src/spam_gate.rs @@ -216,20 +216,23 @@ mod tests { #[test] fn install_global_then_second_install_is_rejected() { - // The OnceLock is process-wide, so this single test owns both the - // first (successful) install and the AlreadyInstalled rejection. - let first = SpamGate::new(REPLAY_WINDOW_SECS).install_global(); - assert!(first.is_ok(), "first install must succeed"); + // The OnceLock is process-wide and shared with every other test in + // this binary (e.g. app.rs's accept_event tests also install a + // gate), so another test may have already won the race to install + // before this one runs. Only "some install eventually succeeds, and + // every attempt after that is rejected" is deterministic here — not + // which attempt is "first". + let _ = SpamGate::new(REPLAY_WINDOW_SECS).install_global(); assert!( SpamGate::global().is_some(), - "global() must expose the installed gate" + "global() must expose an installed gate" ); let second = SpamGate::new(REPLAY_WINDOW_SECS).install_global(); assert_eq!( second, Err(InstallError::AlreadyInstalled), - "a second install must be refused, not panic" + "an install attempted after one already succeeded must be refused, not panic" ); }