From f113a970d3214c75d11e99967789a86e9d5ce1b6 Mon Sep 17 00:00:00 2001 From: Steven Enamakel <31011319+senamakel@users.noreply.github.com> Date: Sat, 4 Jul 2026 04:38:42 -0700 Subject: [PATCH] =?UTF-8?q?test:=20Phase=201=20P0=20=E2=80=94=20encryption?= =?UTF-8?q?=20core=20+=20event-bus=20panic=20isolation=20(re-verified=20ba?= =?UTF-8?q?cklog)=20(#4497)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- plan.md | 32 ++++++-- src/core/event_bus/bus.rs | 61 ++++++++++++++ src/openhuman/encryption/core.rs | 135 +++++++++++++++++++++++++++++++ 3 files changed, 222 insertions(+), 6 deletions(-) diff --git a/plan.md b/plan.md index 57baedb68..d2b89f5f1 100644 --- a/plan.md +++ b/plan.md @@ -363,12 +363,32 @@ Dimensions the suite (and this audit) currently have **zero** coverage of: - [x] Resolve the Release CI Gate Playwright `continue-on-error` bypass (§5bis item 1, #3615): `playwright-e2e` excluded from the gate's `needs`/results with an explanatory comment. -**Phase 1 — P0 coverage (1–2 weeks)** -- Security gate-matrix suite (command_checks/path_checks). -- Encryption core round-trip + tamper suite; RPC-integrated variant. -- `delete_source_rpc` cascade suite. -- Event-bus panic isolation; webhook flood behavior (or file the product gap). -- `stop_core_process` debug command + crash-recovery E2E; RPC auth-failure E2E. +**Phase 1 — P0 coverage** — ⏳ partial (PR: `test/phase1-p0-coverage`). Each item was +**re-verified against current source first** (the §4 backlog was never skeptic-verified); three +of the five "ZERO tests" claims turned out **already covered**, so only the genuine gaps were filled. +- [x] **Encryption core round-trip + tamper suite** — GENUINE GAP (verified 0 prior tests in + `encryption/core.rs`). Added 11 tests: bytes/string round-trip, KDF determinism (behavioural — + key_bytes is private), wrong-password / different-salt rejection, tampered-ciphertext and + tampered-nonce GCM auth failure, fresh-random-nonce-per-call, salt length+randomness, + malformed-JSON and empty-plaintext edge cases. +- [x] **Event-bus panic isolation** — GENUINE GAP (verified: `catch_unwind` in `bus.rs` had no + test; only `publish_without_subscribers`). Added a two-subscriber test: one handler panics on its + first event then recovers; asserts the panicked handler's own loop survives AND a peer keeps + receiving every event. +- [~] **Security gate-matrix (command_checks/path_checks)** — ALREADY COVERED. `policy_tests.rs` + comprehensively tests `classify_command` (unknown⇒Write, redirect-lifts, highest-segment-wins, + all classes), `gate_decision` (all tiers × classes + Install), `is_always_forbidden`, and + `is_workspace_internal_path`. Plan's "ZERO tests" was inaccurate; no suite added (would duplicate). +- [~] **`delete_source_rpc` cascade** — ALREADY COVERED. `memory_store/chunks/store_tests.rs` has + `delete_source_rpc_purges_document_source_fully`, `_unknown_id_is_idempotent`, + `_rejects_empty_source_id`, `_cleans_legacy_partial_delete`, plus the cascade (shared-tree + preserved while referenced, orphan cascade, escape-path/symlink rejection, per-owner scoping). +- [~] **Webhook flood** — PRODUCT GAP (verified: no rate-limit/backpressure/throttle anywhere in + `webhooks/router.rs`; it is registration/routing/logging only). Filed as a product gap, not a test. +- [ ] **core-process crash/recovery E2E** and **RPC bearer-auth-failure E2E** — deferred: both need + the Tauri crate (blocked locally by missing system `glib-2.0`) and/or the WDIO desktop harness. + `tests/json_rpc_e2e.rs` already covers RPC bearer auth/401s at the transport layer; the remaining + gap is the frontend `core_rpc_relay` E2E. To be done in the CI-capable environment. **Phase 2 — deletions & rewrites (parallel with Phase 1, low risk)** — ✅ landed (PR: `test/phase2-drops-rewrites`) - [x] §2.1 deletions applied (⚠️ items re-verified against source before deleting): diff --git a/src/core/event_bus/bus.rs b/src/core/event_bus/bus.rs index 0632a6f07..3faa9d63d 100644 --- a/src/core/event_bus/bus.rs +++ b/src/core/event_bus/bus.rs @@ -320,6 +320,67 @@ mod tests { assert_eq!(counter.load(Ordering::SeqCst), 2); } + #[tokio::test] + async fn panicking_subscriber_does_not_stop_the_bus_or_starve_peers() { + // The per-subscriber `catch_unwind` (bus.rs) exists precisely so one + // handler panicking cannot kill its own dispatch loop or starve other + // subscribers. Nothing exercised it before (plan.md §4 P0 #4). + let bus = EventBus::create(16); + + // Subscriber A panics on its FIRST event, then processes normally. + let a_calls = Arc::new(AtomicUsize::new(0)); + let a_success = Arc::new(AtomicUsize::new(0)); + let a_calls_c = Arc::clone(&a_calls); + let a_success_c = Arc::clone(&a_success); + let _ha = bus.on("panicky", move |_| { + let calls = Arc::clone(&a_calls_c); + let success = Arc::clone(&a_success_c); + Box::pin(async move { + if calls.fetch_add(1, Ordering::SeqCst) == 0 { + panic!("boom in handler"); + } + success.fetch_add(1, Ordering::SeqCst); + }) + }); + + // Subscriber B is a well-behaved peer that just counts. + let b_count = Arc::new(AtomicUsize::new(0)); + let b_c = Arc::clone(&b_count); + let _hb = bus.on("steady", move |_| { + let c = Arc::clone(&b_c); + Box::pin(async move { + c.fetch_add(1, Ordering::SeqCst); + }) + }); + + // Event 1: A panics (must be caught), B counts. + bus.publish(DomainEvent::SystemStartup { + component: "e1".into(), + }); + sleep(Duration::from_millis(50)).await; + // Event 2: A must recover and process; B counts again. + bus.publish(DomainEvent::SystemStartup { + component: "e2".into(), + }); + sleep(Duration::from_millis(50)).await; + + assert_eq!( + a_calls.load(Ordering::SeqCst), + 2, + "the panicked handler's own loop must survive and be invoked for the next event" + ); + assert_eq!( + a_success.load(Ordering::SeqCst), + 1, + "the handler must process the event that follows the one it panicked on" + ); + assert_eq!( + b_count.load(Ordering::SeqCst), + 2, + "a co-subscriber must keep receiving every event despite a peer panicking" + ); + } + #[tokio::test] async fn domain_filtering_works() { use super::super::subscriber::EventHandler; diff --git a/src/openhuman/encryption/core.rs b/src/openhuman/encryption/core.rs index bc6ed22e7..db561466e 100644 --- a/src/openhuman/encryption/core.rs +++ b/src/openhuman/encryption/core.rs @@ -180,3 +180,138 @@ pub async fn ai_decrypt(password: String, encrypted: String) -> Result EncryptionKey { + EncryptionKey::derive(password, salt).expect("derive") + } + + #[test] + fn encrypt_decrypt_bytes_round_trip() { + let k = key( + "correct horse battery staple", + &EncryptionKey::generate_salt(), + ); + let plaintext = b"the launch codes are 0000".to_vec(); + let payload = k.encrypt(&plaintext).expect("encrypt"); + assert_ne!( + payload.ciphertext, plaintext, + "ciphertext must not be plaintext" + ); + assert_eq!(k.decrypt(&payload).expect("decrypt"), plaintext); + } + + #[test] + fn encrypt_decrypt_string_round_trip() { + let k = key("pw", &EncryptionKey::generate_salt()); + let secret = "sk-live-🔐-multibyte"; + let json = k.encrypt_string(secret).expect("encrypt_string"); + assert_eq!(k.decrypt_string(&json).expect("decrypt_string"), secret); + } + + #[test] + fn kdf_is_deterministic_for_same_password_and_salt() { + // Two independent derivations from the same (password, salt) must yield + // the same key: key_a encrypts, key_b decrypts. + let salt = EncryptionKey::generate_salt(); + let key_a = key("hunter2", &salt); + let key_b = key("hunter2", &salt); + let payload = key_a.encrypt(b"cross-key").expect("encrypt"); + assert_eq!(key_b.decrypt(&payload).expect("decrypt"), b"cross-key"); + } + + #[test] + fn wrong_password_cannot_decrypt() { + let salt = EncryptionKey::generate_salt(); + let good = key("right-password", &salt); + let bad = key("wrong-password", &salt); + let payload = good.encrypt(b"top secret").expect("encrypt"); + assert!( + bad.decrypt(&payload).is_err(), + "a key from a different password must not decrypt" + ); + } + + #[test] + fn different_salt_derives_a_different_key() { + let a = key("same-password", &EncryptionKey::generate_salt()); + let b = key("same-password", &EncryptionKey::generate_salt()); + let payload = a.encrypt(b"salted").expect("encrypt"); + assert!( + b.decrypt(&payload).is_err(), + "same password + different salt must yield a non-interchangeable key" + ); + } + + #[test] + fn tampered_ciphertext_is_rejected_by_gcm_auth() { + let k = key("pw", &EncryptionKey::generate_salt()); + let mut payload = k.encrypt(b"authentic bytes").expect("encrypt"); + payload.ciphertext[0] ^= 0xFF; // flip a bit in the ciphertext/tag + assert!( + k.decrypt(&payload).is_err(), + "AES-GCM must reject a tampered ciphertext (auth failure)" + ); + } + + #[test] + fn tampered_nonce_is_rejected() { + let k = key("pw", &EncryptionKey::generate_salt()); + let mut payload = k.encrypt(b"authentic bytes").expect("encrypt"); + payload.nonce[0] ^= 0xFF; // wrong nonce → auth tag no longer verifies + assert!( + k.decrypt(&payload).is_err(), + "decrypting under a mutated nonce must fail" + ); + } + + #[test] + fn each_encryption_uses_a_fresh_random_nonce() { + // Nonce reuse under a fixed key is catastrophic for GCM. Encrypting the + // same plaintext twice must produce distinct nonces (and, therefore, + // distinct ciphertexts) — the nonce is drawn from the CSPRNG per call. + let k = key("pw", &EncryptionKey::generate_salt()); + let p1 = k.encrypt(b"identical plaintext").expect("encrypt"); + let p2 = k.encrypt(b"identical plaintext").expect("encrypt"); + assert_ne!( + p1.nonce, p2.nonce, + "each encryption must draw a fresh nonce" + ); + assert_ne!( + p1.ciphertext, p2.ciphertext, + "a fresh nonce must produce different ciphertext for the same plaintext" + ); + } + + #[test] + fn generate_salt_is_correct_length_and_random() { + let s1 = EncryptionKey::generate_salt(); + let s2 = EncryptionKey::generate_salt(); + assert_eq!(s1.len(), SALT_LENGTH, "salt must be {SALT_LENGTH} bytes"); + assert_ne!(s1, s2, "two generated salts must differ (CSPRNG)"); + } + + #[test] + fn decrypt_string_rejects_malformed_json() { + let k = key("pw", &EncryptionKey::generate_salt()); + assert!( + k.decrypt_string("not-json").is_err(), + "non-JSON payload must be a clean Err, not a panic" + ); + } + + #[test] + fn empty_plaintext_round_trips() { + let k = key("pw", &EncryptionKey::generate_salt()); + let payload = k.encrypt(b"").expect("encrypt empty"); + assert_eq!(k.decrypt(&payload).expect("decrypt empty"), b""); + } +}