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..f02ece6b 100644 --- a/.github/workflows/mutation.yml +++ b/.github/workflows/mutation.yml @@ -60,15 +60,24 @@ jobs: exit 0 fi - # Build --file flags for each changed file - file_args="" - for f in $changed_rs; do - file_args="$file_args --file $f" - done - echo "Running mutation testing for changed files:" echo "$changed_rs" - cargo mutants $file_args + + # These filenames come from a PR diff, so they're attacker-controlled. + # Built as a bash array (never joined into a string) and passed with + # "${file_args[@]}" so a crafted filename can only ever be a single + # --file value, never additional argv tokens — the intermediate + # string-then-make(ARGS)-then-shell round trip this used to take was + # an argument-injection vector into cargo-mutants/cargo/rustc's own + # flag surface. This deliberately bypasses `make mutation-test`, + # whose $(ARGS) is a plain string splice safe only for + # human-typed, trusted input (the baseline job below still uses it). + file_args=() + while IFS= read -r f; do + file_args+=(--file "$f") + done <<< "$changed_rs" + + CARGO_MUTANTS_JOBS=2 cargo mutants "${file_args[@]}" - name: Upload mutation report uses: actions/upload-artifact@v4 @@ -195,7 +204,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/.gitignore b/.gitignore index fd5316fb..450d5ee5 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ lnurl-test-server/target .idea .vscode .cursor +.claude # settings file settings.toml diff --git a/Makefile b/Makefile index 348bfa2f..2b417669 100644 --- a/Makefile +++ b/Makefile @@ -66,3 +66,13 @@ docker-build-startos: cd docker && \ docker compose build mostro-startos +# ARGS is spliced into the shell command as plain text — only pass +# hand-typed, trusted values (e.g. `make mutation-test ARGS="--file +# src/foo.rs"`). Never build ARGS from PR-diff filenames or other +# attacker-controlled input; that class of data must be turned into a +# bash array and passed to `cargo mutants` directly instead (see the +# PR job in .github/workflows/mutation.yml). +mutation-test: + @set -o pipefail; \ + CARGO_MUTANTS_JOBS=2 cargo mutants $(ARGS) + diff --git a/src/app/restore_session.rs b/src/app/restore_session.rs index 0c47350a..2e9fbd0c 100644 --- a/src/app/restore_session.rs +++ b/src/app/restore_session.rs @@ -1,8 +1,15 @@ use crate::app::context::AppContext; -use crate::{db::RestoreSessionManager, util::enqueue_restore_session_msg}; +use crate::{ + db::RestoreSessionManager, + util::{enqueue_restore_session_msg, is_valid_hex_pubkey}, +}; use mostro_core::prelude::*; use nostr_sdk::prelude::*; +/// Restore session results wait for this long before the requester is told +/// to retry instead of hanging forever. +const RESTORE_SESSION_TIMEOUT_SECS: u64 = 60 * 60; + /// Handle restore session action /// This function starts a background task to process the restore session /// and immediately returns, avoiding blocking the main application @@ -17,12 +24,12 @@ pub async fn restore_session_action( let trade_key = event.sender.to_string(); // Validate the master key format - if !master_key.chars().all(|c| c.is_ascii_hexdigit()) || master_key.len() != 64 { + if !is_valid_hex_pubkey(&master_key) { return Err(MostroCantDo(CantDoReason::InvalidPubkey)); } // Validate the trade key format - if !trade_key.chars().all(|c| c.is_ascii_hexdigit()) || trade_key.len() != 64 { + if !is_valid_hex_pubkey(&trade_key) { return Err(MostroCantDo(CantDoReason::InvalidPubkey)); } @@ -51,7 +58,7 @@ pub async fn restore_session_action( /// Handle restore session results in the background async fn handle_restore_session_results(mut manager: RestoreSessionManager, trade_key: String) { // Wait for the result with a timeout - let timeout = tokio::time::Duration::from_secs(60 * 60); // 1 hour timeout + let timeout = tokio::time::Duration::from_secs(RESTORE_SESSION_TIMEOUT_SECS); match tokio::time::timeout(timeout, manager.wait_for_result()).await { Ok(Some(result)) => { @@ -267,4 +274,9 @@ mod tests { Err(MostroCantDo(CantDoReason::InvalidPubkey)) )); } + + #[test] + fn restore_session_timeout_is_one_hour() { + assert_eq!(RESTORE_SESSION_TIMEOUT_SECS, 3600); + } } diff --git a/src/util.rs b/src/util.rs index cbe46a1e..cd03e596 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1251,6 +1251,12 @@ pub fn bytes_to_string(bytes: &[u8]) -> String { }) } +/// A Nostr pubkey, as carried on the wire, is a 64-char hex string +/// (case-insensitive). +pub fn is_valid_hex_pubkey(key: &str) -> bool { + key.len() == 64 && key.chars().all(|c| c.is_ascii_hexdigit()) +} + pub async fn enqueue_cant_do_msg( request_id: Option, order_id: Option, @@ -1712,6 +1718,43 @@ mod tests { assert_eq!(result, ""); } + const VALID_HEX_PUBKEY: &str = + "1111111111111111111111111111111111111111111111111111111111111111"; + + #[test] + fn valid_hex_pubkey_is_accepted() { + assert_eq!(VALID_HEX_PUBKEY.len(), 64); + assert!(is_valid_hex_pubkey(VALID_HEX_PUBKEY)); + } + + #[test] + fn valid_hex_pubkey_accepts_uppercase() { + assert!(is_valid_hex_pubkey(&VALID_HEX_PUBKEY.to_uppercase())); + } + + #[test] + fn valid_hex_pubkey_rejects_too_short_key() { + assert!(!is_valid_hex_pubkey(&VALID_HEX_PUBKEY[..63])); + } + + #[test] + fn valid_hex_pubkey_rejects_too_long_key() { + let too_long = format!("{VALID_HEX_PUBKEY}1"); + assert!(!is_valid_hex_pubkey(&too_long)); + } + + #[test] + fn valid_hex_pubkey_rejects_non_hex_characters() { + let mut key = VALID_HEX_PUBKEY[..63].to_string(); + key.push('g'); + assert!(!is_valid_hex_pubkey(&key)); + } + + #[test] + fn valid_hex_pubkey_rejects_empty_key() { + assert!(!is_valid_hex_pubkey("")); + } + #[tokio::test] async fn test_send_dm() { initialize();