From 37f5a5a7ca54e4a36c19e2c47e509ed80bc3de9f Mon Sep 17 00:00:00 2001 From: Aleksy Siek Date: Mon, 14 Sep 2026 16:07:50 +0100 Subject: [PATCH 1/3] test: isolate integration suite XDG state Signed-off-by: Aleksy Siek --- .github/workflows/ci.yml | 5 +- Makefile | 46 +++++++++-- crates/nono-cli/src/approval_runtime.rs | 70 +++++++++++----- scripts/run_all_tests.sh | 103 ++++++++---------------- 4 files changed, 124 insertions(+), 100 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 89f94c1d0..488761574 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,6 +12,7 @@ permissions: env: CARGO_TERM_COLOR: always RUSTFLAGS: -Dwarnings + RUST_BACKTRACE: "1" jobs: actionlint: @@ -142,10 +143,10 @@ jobs: echo "OK: no libdbus dependency" - name: Run tests - run: cargo test --workspace --verbose + run: cargo test --workspace --verbose --no-fail-fast - name: Run doc tests - run: cargo test --doc --workspace --verbose + run: cargo test --doc --workspace --verbose --no-fail-fast fmt: name: Rustfmt diff --git a/Makefile b/Makefile index 8dbd3927c..f90311969 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,11 @@ # make check Run clippy and format check # make release Build release binaries -.PHONY: all build build-lib build-cli build-ffi build-arm64 test test-lib test-cli test-ffi test-spiffe check clippy fmt clean install audit help +.PHONY: all build build-lib build-cli build-ffi build-arm64 test test-workspace test-lib test-cli test-ffi test-doc test-one test-repeat test-spiffe check clippy fmt clean install audit help + +# Keep Rust panic locations available in both local and CI test output. A +# caller can still override this (for example, RUST_BACKTRACE=full make test). +RUST_BACKTRACE ?= 1 # Default target all: build @@ -40,19 +44,44 @@ build-arm64: @cross build --release --target aarch64-unknown-linux-gnu -p nono-cli # Test targets -test: test-lib test-cli test-ffi +# +# Match the regular Rust CI coverage. The former package-by-package aggregate +# omitted nono-proxy and any future workspace members. +test: test-workspace + +test-workspace: + RUST_BACKTRACE=$(RUST_BACKTRACE) cargo test --workspace --no-fail-fast test-lib: - cargo test -p nono + RUST_BACKTRACE=$(RUST_BACKTRACE) cargo test -p nono --no-fail-fast test-cli: - cargo test -p nono-cli + RUST_BACKTRACE=$(RUST_BACKTRACE) cargo test -p nono-cli --no-fail-fast test-ffi: - cargo test -p nono-ffi + RUST_BACKTRACE=$(RUST_BACKTRACE) cargo test -p nono-ffi --no-fail-fast test-doc: - cargo test --doc + RUST_BACKTRACE=$(RUST_BACKTRACE) cargo test --doc --workspace --no-fail-fast + +# Run one exact test with live process output. Usage: +# make test-one TEST=approval_runtime::tests::platform_poll_loop_gives_up_at_the_configured_timeout +test-one: + @test -n "$(TEST)" || { echo "Usage: make test-one TEST=" >&2; exit 2; } + RUST_BACKTRACE=$(RUST_BACKTRACE) cargo test --workspace --no-fail-fast "$(TEST)" -- --exact --nocapture --test-threads=1 + +# Repeat one exact test serially to reproduce intermittent failures. Usage: +# make test-repeat TEST= COUNT=100 +COUNT ?= 20 +test-repeat: + @test -n "$(TEST)" || { echo "Usage: make test-repeat TEST= [COUNT=]" >&2; exit 2; } + @set -eu; \ + i=1; \ + while [ "$$i" -le "$(COUNT)" ]; do \ + echo "==> $$i/$(COUNT): $(TEST)"; \ + RUST_BACKTRACE=$(RUST_BACKTRACE) cargo test --workspace --no-fail-fast "$(TEST)" -- --exact --nocapture --test-threads=1; \ + i=$$((i + 1)); \ + done test-spiffe: bash scripts/spire-test.sh @@ -115,7 +144,7 @@ lint-docs: bash scripts/lint-docs.sh # CI simulation (what CI would run) -ci: check test audit lint-docs +ci: check test test-doc audit lint-docs @echo "CI checks passed" # Help @@ -132,10 +161,13 @@ help: @echo "" @echo "Test:" @echo " make test Run all tests" + @echo " make test-workspace Run all workspace Rust tests" @echo " make test-lib Run library tests only" @echo " make test-cli Run CLI tests only" @echo " make test-ffi Run C FFI tests only" @echo " make test-doc Run doc tests only" + @echo " make test-one TEST= Run one exact Rust test with live output" + @echo " make test-repeat TEST= COUNT=N Repeat one exact Rust test serially" @echo " make test-spiffe Run SPIFFE/SPIRE integration tests (downloads SPIRE if needed)" @echo "" @echo "Check:" diff --git a/crates/nono-cli/src/approval_runtime.rs b/crates/nono-cli/src/approval_runtime.rs index 0e8dde63e..1102ee893 100644 --- a/crates/nono-cli/src/approval_runtime.rs +++ b/crates/nono-cli/src/approval_runtime.rs @@ -279,12 +279,12 @@ impl WebhookApproval { } } - fn http_status_denial(&self, status: u16, started: Instant) -> ApprovalDecision { + fn http_status_denial(&self, status: u16, elapsed: Duration) -> ApprovalDecision { ApprovalDecision::Denied { reason: format!( "approval webhook '{}' returned HTTP {status} after {}s", self.name, - started.elapsed().as_secs() + elapsed.as_secs() ), } } @@ -327,7 +327,7 @@ impl WebhookApproval { let status = response.status().as_u16(); let response_body = self.read_body(&mut response)?; if !(200..300).contains(&status) { - return Ok(self.http_status_denial(status, started)); + return Ok(self.http_status_denial(status, started.elapsed())); } self.parse_response(&response_body) } @@ -342,11 +342,24 @@ impl WebhookApproval { request_id: &str, body: &[u8], started: Instant, + send: impl FnMut(PlatformStep<'_>, Duration) -> Result, + ) -> Result { + self.drive_platform_approval_with_elapsed(request_id, body, || started.elapsed(), send) + } + + /// The poll-loop state machine with an injected monotonic elapsed-time + /// source. Keeping time separate from transport lets its deadline behavior + /// be tested without wall-clock sleeps or scheduler-dependent assertions. + fn drive_platform_approval_with_elapsed( + &self, + request_id: &str, + body: &[u8], + mut elapsed: impl FnMut() -> Duration, mut send: impl FnMut(PlatformStep<'_>, Duration) -> Result, ) -> Result { let mut submitted = false; loop { - let remaining = self.timeout.saturating_sub(started.elapsed()); + let remaining = self.timeout.saturating_sub(elapsed()); if remaining.is_zero() { return Ok(ApprovalDecision::Timeout); } @@ -359,11 +372,12 @@ impl WebhookApproval { // The deadline is checked again after the round trip: a decision // that arrives once this backend's timeout has elapsed is never // applied, however long the individual request took. - if started.elapsed() >= self.timeout { + let elapsed_after_reply = elapsed(); + if elapsed_after_reply >= self.timeout { return Ok(ApprovalDecision::Timeout); } if !(200..300).contains(&reply.status) { - return Ok(self.http_status_denial(reply.status, started)); + return Ok(self.http_status_denial(reply.status, elapsed_after_reply)); } let status: PlatformApprovalStatus = serde_json::from_str(&reply.body).map_err(|e| { @@ -825,32 +839,46 @@ mod tests { #[test] fn platform_poll_loop_gives_up_at_the_configured_timeout() { let backend = test_webhook(Duration::from_millis(40)); + let elapsed = std::rc::Rc::new(std::cell::Cell::new(Duration::ZERO)); + let clock = std::rc::Rc::clone(&elapsed); let mut polls = 0; let decision = backend - .drive_platform_approval("req-2", b"{}", Instant::now(), |_step, _remaining| { - polls += 1; - std::thread::sleep(Duration::from_millis(10)); - Ok(reply(202, r#"{"state":"pending"}"#)) - }) + .drive_platform_approval_with_elapsed( + "req-2", + b"{}", + || clock.get(), + |_step, _remaining| { + polls += 1; + elapsed.set(elapsed.get() + Duration::from_millis(10)); + Ok(reply(202, r#"{"state":"pending"}"#)) + }, + ) .unwrap(); assert!(matches!(decision, ApprovalDecision::Timeout)); - assert!( - polls >= 2, - "expected several polls before giving up, saw {polls}" + assert_eq!( + polls, 4, + "the fourth 10 ms reply reaches the 40 ms deadline" ); } #[test] fn platform_poll_loop_rejects_a_grant_that_arrives_after_the_deadline() { let backend = test_webhook(Duration::from_millis(40)); + let elapsed = std::rc::Rc::new(std::cell::Cell::new(Duration::ZERO)); + let clock = std::rc::Rc::clone(&elapsed); let decision = backend - .drive_platform_approval("req-late", b"{}", Instant::now(), |_, remaining| { - // The hop is handed only the remaining budget... - assert!(remaining <= Duration::from_millis(40)); - // ...but simulate a server that answers after the deadline. - std::thread::sleep(Duration::from_millis(60)); - Ok(reply(200, r#"{"state":"granted","decision":"granted"}"#)) - }) + .drive_platform_approval_with_elapsed( + "req-late", + b"{}", + || clock.get(), + |_, remaining| { + // The hop is handed only the remaining budget... + assert_eq!(remaining, Duration::from_millis(40)); + // ...but a server response can still arrive after it. + elapsed.set(Duration::from_millis(60)); + Ok(reply(200, r#"{"state":"granted","decision":"granted"}"#)) + }, + ) .unwrap(); assert!( matches!(decision, ApprovalDecision::Timeout), diff --git a/scripts/run_all_tests.sh b/scripts/run_all_tests.sh index cb96ff312..51fe49526 100755 --- a/scripts/run_all_tests.sh +++ b/scripts/run_all_tests.sh @@ -79,9 +79,6 @@ chmod +x "$TESTS_DIR"/lib/*.sh 2>/dev/null || true RESULTS_DIR=$(mktemp -d) TEST_ENV_DIR=$(mktemp -d) -mkdir -p "$TEST_ENV_DIR/trust-config" "$TEST_ENV_DIR/trust-keystore" -export NONO_TRUST_TEST_USER_POLICY_PATH="$TEST_ENV_DIR/trust-config/trust-policy.json" -export NONO_TRUST_TEST_KEYSTORE_DIR="$TEST_ENV_DIR/trust-keystore" export NONO_NO_UPDATE_CHECK=1 # Suppress the migration prompt (--profile when the pack # isn't installed) and the "save denied paths as user profile?" @@ -91,70 +88,9 @@ export NONO_NO_UPDATE_CHECK=1 export NONO_NO_MIGRATE=1 export NONO_NO_SAVE_PROMPT=1 -# Audit is on by default, so every test invocation that does not pass -# --no-audit writes a session under $XDG_STATE_HOME/nono/audit/ (and $XDG_STATE_HOME/nono/rollbacks/ -# for rollback tests), and appends to the audit ledger there. There is -# no env-var override for the audit root, so snapshot the pre-run state and -# restore it on exit. This removes only artefacts created during the run; -# pre-existing user sessions and ledger entries are preserved. -# Set NONO_TEST_KEEP_AUDIT=1 to skip cleanup for debugging. -NONO_AUDIT_ROOT="${XDG_STATE_HOME:-$HOME/.local/state}/nono/audit" -NONO_ROLLBACK_ROOT="${XDG_STATE_HOME:-$HOME/.local/state}/nono/rollbacks" -AUDIT_SNAPSHOT_DIR="$TEST_ENV_DIR/audit-snapshot" -mkdir -p "$AUDIT_SNAPSHOT_DIR" - -snapshot_dirs_in() { - local root="$1" - local out="$2" - if [[ -d "$root" ]]; then - find "$root" -maxdepth 1 -mindepth 1 -type d -print > "$out" 2>/dev/null || : - else - : > "$out" - fi -} - -snapshot_dirs_in "$NONO_AUDIT_ROOT" "$AUDIT_SNAPSHOT_DIR/audit.before" -snapshot_dirs_in "$NONO_ROLLBACK_ROOT" "$AUDIT_SNAPSHOT_DIR/rollback.before" - -NONO_LEDGER_FILE="$NONO_AUDIT_ROOT/ledger.ndjson" -NONO_LEDGER_LOCK="$NONO_AUDIT_ROOT/ledger.lock" -NONO_LEDGER_BACKUP="$AUDIT_SNAPSHOT_DIR/ledger.ndjson" -NONO_LEDGER_EXISTED=0 -NONO_LEDGER_LOCK_EXISTED=0 -if [[ -f "$NONO_LEDGER_FILE" ]]; then - cp "$NONO_LEDGER_FILE" "$NONO_LEDGER_BACKUP" - NONO_LEDGER_EXISTED=1 -fi -[[ -f "$NONO_LEDGER_LOCK" ]] && NONO_LEDGER_LOCK_EXISTED=1 - -cleanup_test_audit_artifacts() { - [[ "${NONO_TEST_KEEP_AUDIT:-0}" == "1" ]] && return 0 - - _remove_new_dirs() { - local root="$1" - local before="$2" - [[ -d "$root" ]] || return 0 - while IFS= read -r -d '' dir; do - if ! grep -Fxq "$dir" "$before" 2>/dev/null; then - rm -rf "$dir" - fi - done < <(find "$root" -maxdepth 1 -mindepth 1 -type d -print0) - } - - _remove_new_dirs "$NONO_AUDIT_ROOT" "$AUDIT_SNAPSHOT_DIR/audit.before" - _remove_new_dirs "$NONO_ROLLBACK_ROOT" "$AUDIT_SNAPSHOT_DIR/rollback.before" - - if [[ "$NONO_LEDGER_EXISTED" -eq 1 ]]; then - cp "$NONO_LEDGER_BACKUP" "$NONO_LEDGER_FILE" - elif [[ -f "$NONO_LEDGER_FILE" ]]; then - rm -f "$NONO_LEDGER_FILE" - fi - if [[ "$NONO_LEDGER_LOCK_EXISTED" -eq 0 && -f "$NONO_LEDGER_LOCK" ]]; then - rm -f "$NONO_LEDGER_LOCK" - fi -} - -trap 'cleanup_test_audit_artifacts; rm -rf "$RESULTS_DIR" "$TEST_ENV_DIR"' EXIT +# Each suite receives an isolated XDG_STATE_HOME below TEST_ENV_DIR, so audit +# sessions, rollback snapshots, and ledgers never touch the caller's state. +trap 'rm -rf "$RESULTS_DIR" "$TEST_ENV_DIR"' EXIT # All suites to run (script:name pairs) # Discover every shell suite so newly added suites cannot be omitted from CI. @@ -194,11 +130,31 @@ echo "" launch_suite() { local script="$1" local output_file="$2" - local exit_file="${output_file%.out}.exit" + local suite_id="${script%.sh}" + local suite_env_dir="$TEST_ENV_DIR/suites/$suite_id" + + # Suites run in parallel and must not share mutable XDG state. In + # particular, rollback and audit suites otherwise race on the default + # rollback root and audit ledger. Keep HOME intact: sensitive-path suites + # intentionally exercise the caller's real protected paths. + mkdir -p \ + "$suite_env_dir/config" \ + "$suite_env_dir/state" \ + "$suite_env_dir/cache" \ + "$suite_env_dir/data" \ + "$suite_env_dir/trust-config" \ + "$suite_env_dir/trust-keystore" if command -v timeout >/dev/null 2>&1; then - timeout "$SUITE_TIMEOUT" bash "$TESTS_DIR/integration/$script" > "$output_file" 2>&1 + env \ + XDG_CONFIG_HOME="$suite_env_dir/config" \ + XDG_STATE_HOME="$suite_env_dir/state" \ + XDG_CACHE_HOME="$suite_env_dir/cache" \ + XDG_DATA_HOME="$suite_env_dir/data" \ + NONO_TRUST_TEST_USER_POLICY_PATH="$suite_env_dir/trust-config/trust-policy.json" \ + NONO_TRUST_TEST_KEYSTORE_DIR="$suite_env_dir/trust-keystore" \ + timeout "$SUITE_TIMEOUT" bash "$TESTS_DIR/integration/$script" > "$output_file" 2>&1 rc=$? if [[ "$rc" -eq 124 ]]; then echo "" >> "$output_file" @@ -206,7 +162,14 @@ launch_suite() { fi echo "$rc" > "$exit_file" else - bash "$TESTS_DIR/integration/$script" > "$output_file" 2>&1; rc=$? + env \ + XDG_CONFIG_HOME="$suite_env_dir/config" \ + XDG_STATE_HOME="$suite_env_dir/state" \ + XDG_CACHE_HOME="$suite_env_dir/cache" \ + XDG_DATA_HOME="$suite_env_dir/data" \ + NONO_TRUST_TEST_USER_POLICY_PATH="$suite_env_dir/trust-config/trust-policy.json" \ + NONO_TRUST_TEST_KEYSTORE_DIR="$suite_env_dir/trust-keystore" \ + bash "$TESTS_DIR/integration/$script" > "$output_file" 2>&1; rc=$? echo "$rc" > "$exit_file" fi } From 692f4c60bdf1ad6c3fd275396d4409772cc9f68c Mon Sep 17 00:00:00 2001 From: Aleksy Siek Date: Mon, 14 Sep 2026 16:21:01 +0100 Subject: [PATCH 2/3] fix Signed-off-by: Aleksy Siek --- scripts/run_all_tests.sh | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/scripts/run_all_tests.sh b/scripts/run_all_tests.sh index 51fe49526..e215c1d75 100755 --- a/scripts/run_all_tests.sh +++ b/scripts/run_all_tests.sh @@ -78,6 +78,12 @@ chmod +x "$TESTS_DIR"/lib/*.sh 2>/dev/null || true # Temp directory for suite output files RESULTS_DIR=$(mktemp -d) TEST_ENV_DIR=$(mktemp -d) +# Keep XDG state outside /tmp. The Linux system_write policy deliberately +# grants /tmp, and nono must reject a capability that would cover its own +# state root. This directory is still per-run and removed by the exit trap. +TEST_XDG_PARENT="${XDG_CACHE_HOME:-$HOME/.cache}" +mkdir -p "$TEST_XDG_PARENT" +TEST_XDG_DIR=$(mktemp -d "$TEST_XDG_PARENT/nono-integration.XXXXXX") export NONO_NO_UPDATE_CHECK=1 # Suppress the migration prompt (--profile when the pack @@ -88,9 +94,9 @@ export NONO_NO_UPDATE_CHECK=1 export NONO_NO_MIGRATE=1 export NONO_NO_SAVE_PROMPT=1 -# Each suite receives an isolated XDG_STATE_HOME below TEST_ENV_DIR, so audit +# Each suite receives an isolated XDG_STATE_HOME outside /tmp, so audit # sessions, rollback snapshots, and ledgers never touch the caller's state. -trap 'rm -rf "$RESULTS_DIR" "$TEST_ENV_DIR"' EXIT +trap 'rm -rf "$RESULTS_DIR" "$TEST_ENV_DIR" "$TEST_XDG_DIR"' EXIT # All suites to run (script:name pairs) # Discover every shell suite so newly added suites cannot be omitted from CI. @@ -132,7 +138,7 @@ launch_suite() { local output_file="$2" local exit_file="${output_file%.out}.exit" local suite_id="${script%.sh}" - local suite_env_dir="$TEST_ENV_DIR/suites/$suite_id" + local suite_env_dir="$TEST_XDG_DIR/suites/$suite_id" # Suites run in parallel and must not share mutable XDG state. In # particular, rollback and audit suites otherwise race on the default From 4037cf1ad935cb2b9518d24a518e6c15c467921f Mon Sep 17 00:00:00 2001 From: Aleksy Siek Date: Mon, 14 Sep 2026 16:35:02 +0100 Subject: [PATCH 3/3] fix: use /var Signed-off-by: Aleksy Siek --- scripts/run_all_tests.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/scripts/run_all_tests.sh b/scripts/run_all_tests.sh index e215c1d75..1f4a3d16b 100755 --- a/scripts/run_all_tests.sh +++ b/scripts/run_all_tests.sh @@ -78,10 +78,11 @@ chmod +x "$TESTS_DIR"/lib/*.sh 2>/dev/null || true # Temp directory for suite output files RESULTS_DIR=$(mktemp -d) TEST_ENV_DIR=$(mktemp -d) -# Keep XDG state outside /tmp. The Linux system_write policy deliberately -# grants /tmp, and nono must reject a capability that would cover its own -# state root. This directory is still per-run and removed by the exit trap. -TEST_XDG_PARENT="${XDG_CACHE_HOME:-$HOME/.cache}" +# Keep XDG state outside /tmp and HOME. The Linux system_write policy grants +# /tmp, while the bypass-protection suite intentionally grants paths below +# HOME; either location would overlap nono's protected state root. /var/tmp is +# writable but is not part of either capability surface. Override when needed. +TEST_XDG_PARENT="${NONO_TEST_XDG_PARENT:-/var/tmp}" mkdir -p "$TEST_XDG_PARENT" TEST_XDG_DIR=$(mktemp -d "$TEST_XDG_PARENT/nono-integration.XXXXXX")