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 8565ad53..19225e33 100644 --- a/src/app/restore_session.rs +++ b/src/app/restore_session.rs @@ -3,6 +3,10 @@ use crate::{db::RestoreSessionManager, util::enqueue_restore_session_msg}; 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 @@ -51,7 +55,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)) => { @@ -70,7 +74,9 @@ async fn handle_restore_session_results(mut manager: RestoreSessionManager, trad tracing::error!("Restore session result channel closed unexpectedly"); } Err(_) => { - tracing::error!("Restore session timed out after 1 hour"); + tracing::error!( + "Restore session timed out after {RESTORE_SESSION_TIMEOUT_SECS} seconds" + ); // Send timeout message to user if let Err(e) = send_restore_session_timeout(&trade_key).await { tracing::error!("Failed to send timeout message: {}", e); @@ -128,6 +134,11 @@ mod tests { use sqlx::SqlitePool; use std::sync::Arc; + #[test] + fn restore_session_timeout_is_one_hour() { + assert_eq!(RESTORE_SESSION_TIMEOUT_SECS, 3600); + } + async fn create_test_pool() -> SqlitePool { let pool = SqlitePool::connect("sqlite::memory:").await.unwrap(); sqlx::migrate!().run(&pool).await.unwrap();