From b5f388d15758cbf3f6918e08b17d4229420f2fb2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Apr 2026 08:25:54 +0000 Subject: [PATCH 01/10] chore: open auto-fix batch claude/friendly-maxwell-Oggvw 2026-04-28 From 94af1cbead7706f998f65d991eb04f0966abe059 Mon Sep 17 00:00:00 2001 From: intendednull Date: Tue, 28 Apr 2026 01:47:13 -0700 Subject: [PATCH 02/10] fix(web): add Content-Security-Policy meta tag to index.html (#175) (#462) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lock down the document head with a CSP that matches the app's actual loads: * default-src 'self' / object-src 'none' / frame-ancestors 'none' / base-uri 'self' / form-action 'self' — kill the broad injection + clickjacking surfaces. * script-src 'self' 'wasm-unsafe-eval' 'unsafe-eval' — 'wasm-unsafe-eval' is required for the Leptos WASM module; 'unsafe-eval' is currently required because crates/web still calls js_sys::eval() (theme bootstrap, focus helpers, relay-URL probe). Removing it is tracked by issues #171 / #425. Documented inline so the next person who reads the head sees the rationale. * style-src 'self' 'unsafe-inline' + Google Fonts host — Leptos views emit inline style="…" attributes, and foundation.css @imports the Fraunces / IBM Plex Sans / JetBrains Mono stylesheet. font-src separately allow-lists fonts.gstatic.com for the actual font files. * connect-src 'self' ws: wss: https: — relay WebSocket transport plus the relay's /bootstrap-id HTTP probe. * img-src 'self' data: blob: + media-src 'self' blob: — avatar data URIs, runtime URL.createObjectURL attachments, the chime.webm. * worker-src 'self' — service worker registration in init.js. Add a static-asset test that asserts the meta tag is present and that each required directive substring is in the content attribute, so any future loosening / rewording trips the test before regressing the policy. Refs #175 Co-authored-by: Claude --- crates/web/index.html | 24 ++++++++++++++++ crates/web/tests/static_assets.rs | 47 +++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/crates/web/index.html b/crates/web/index.html index 11e3f8b2..11506818 100644 --- a/crates/web/index.html +++ b/crates/web/index.html @@ -2,6 +2,30 @@ + + diff --git a/crates/web/tests/static_assets.rs b/crates/web/tests/static_assets.rs index 0343c5e0..593d9af3 100644 --- a/crates/web/tests/static_assets.rs +++ b/crates/web/tests/static_assets.rs @@ -112,6 +112,53 @@ fn index_html_only_references_bundled_svg_icons() { assert_all_in_allow_list("index.html", &refs); } +/// Required directives for the CSP meta tag baked into `index.html`. +/// +/// Each entry is a substring search against the meta tag's `content` value; +/// they collectively encode the policy decisions described in the inline +/// HTML comment beside the tag (see GitHub issue #175). If you intentionally +/// loosen or tighten one of these directives, update both this list and the +/// inline comment so the rationale stays in sync. +const REQUIRED_CSP_DIRECTIVES: &[&str] = &[ + "default-src 'self'", + // WASM module + the still-extant js_sys::eval() sites (tracked by + // issues #171 / #425). Drop 'unsafe-eval' once those are gone. + "script-src 'self' 'wasm-unsafe-eval' 'unsafe-eval'", + // Inline style="…" attrs from Leptos views + Google Fonts CSS @import. + "style-src 'self' 'unsafe-inline' https://fonts.googleapis.com", + "font-src 'self' https://fonts.gstatic.com", + // ws/wss for relay transport, https for the relay HTTP bootstrap probe. + "connect-src 'self' ws: wss: https:", + // data: for avatar URIs, blob: for runtime createObjectURL attachments. + "img-src 'self' data: blob:", + "media-src 'self' blob:", + "worker-src 'self'", + "object-src 'none'", + "base-uri 'self'", + "form-action 'self'", + "frame-ancestors 'none'", +]; + +#[test] +fn index_html_declares_content_security_policy() { + let contents = read_asset("index.html"); + let needle = "http-equiv=\"Content-Security-Policy\""; + assert!( + contents.contains(needle), + "index.html is missing the Content-Security-Policy meta tag. \ + See GitHub issue #175 — the CSP guards against script injection \ + and clickjacking and must stay in the document head.", + ); + for directive in REQUIRED_CSP_DIRECTIVES { + assert!( + contents.contains(directive), + "index.html CSP is missing required directive `{directive}`. \ + Update both the meta tag in index.html and the rationale \ + comment beside it if you are intentionally changing this.", + ); + } +} + #[test] fn manifest_json_only_references_bundled_svg_icons() { let contents = read_asset("manifest.json"); From fbe256f79f337875f22d28d4198e70e2a5323e37 Mon Sep 17 00:00:00 2001 From: intendednull Date: Tue, 28 Apr 2026 02:02:49 -0700 Subject: [PATCH 03/10] test(identity): malformed-bytes rejection paths (#424) three new unit tests in crates/identity/src/lib.rs covering endpoint id hex parsing, garbage profile bytes, and truncated/short-pk envelope rejection paths. tests-only. Refs #424 --- crates/identity/src/lib.rs | 97 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/crates/identity/src/lib.rs b/crates/identity/src/lib.rs index 917d9a39..92b5cecc 100644 --- a/crates/identity/src/lib.rs +++ b/crates/identity/src/lib.rs @@ -847,6 +847,103 @@ mod tests { assert!(result.is_err(), "tampered profile must not verify"); } + /// Issue #424: an `EndpointId` parsed from a string that is not valid hex + /// (and not a valid z32-encoded key) must fail with a parse error rather + /// than silently producing a bogus key. `EndpointId` aliases iroh's + /// `PublicKey`, whose `FromStr` impl rejects garbage input. + #[test] + fn endpoint_id_rejects_malformed_hex() { + use std::str::FromStr; + + // Not hex, not z32, just a clearly-wrong string. + let result = EndpointId::from_str("not-a-valid-key!!!"); + assert!( + result.is_err(), + "malformed string must not parse to an EndpointId, got: {result:?}" + ); + + // 64 chars but contains non-hex characters — should still fail. + let bad_hex = "z".repeat(64); + let result = EndpointId::from_str(&bad_hex); + assert!( + result.is_err(), + "non-hex 64-char string must not parse, got: {result:?}" + ); + + // Wrong length hex (too short). + let result = EndpointId::from_str("deadbeef"); + assert!( + result.is_err(), + "short hex must not parse to an EndpointId, got: {result:?}" + ); + } + + /// Issue #424: feeding random / garbage bytes to `unpack_profile` + /// (which deserializes via `willow_transport::unpack`, i.e. bincode) + /// must surface as `IdentityError::Serde`, not a panic and not a + /// silent default-constructed profile. + #[test] + fn profile_decode_rejects_invalid_cbor() { + // Random-looking bytes that don't form a valid bincode-encoded + // `SignedMessage`. (The crate uses bincode under the hood, but + // the issue still names the test for historical reasons.) + let garbage: [u8; 16] = [ + 0xFF, 0x00, 0xDE, 0xAD, 0xBE, 0xEF, 0x12, 0x34, 0x56, 0x78, 0x9A, 0xBC, 0xDE, 0xF0, + 0x42, 0x42, + ]; + + let err = unpack_profile(&garbage).unwrap_err(); + match err { + IdentityError::Serde(_) => {} + other => panic!("expected IdentityError::Serde for garbage bytes, got {other:?}"), + } + + // Empty input must also be rejected as a serialization error + // (bincode can't parse an empty buffer into a `SignedMessage`). + let err = unpack_profile(&[]).unwrap_err(); + match err { + IdentityError::Serde(_) => {} + other => panic!("expected IdentityError::Serde for empty bytes, got {other:?}"), + } + } + + /// Issue #424: malformed serialized envelope bytes — truncated mid-record + /// or otherwise non-decodable — must be rejected by `unpack` with + /// `IdentityError::Serde`. Exercises the outer-envelope deserialization + /// path, distinct from a tampered-but-well-formed payload (which is + /// covered by `tampered_payload_fails_verification`). + #[test] + fn unpack_rejects_truncated_envelope_bytes() { + let alice = Identity::generate(); + let signed = pack(&String::from("hello"), &alice).unwrap(); + + // Truncate to roughly the first quarter of the envelope so that + // bincode can't reconstruct the `SignedMessage` framing + // (length prefixes / Vec contents will run off the end). + let truncated = &signed[..signed.len() / 4]; + let err = unpack::(truncated).unwrap_err(); + match err { + IdentityError::Serde(_) => {} + other => panic!("expected IdentityError::Serde for truncated bytes, got {other:?}"), + } + + // A `SignedMessage` whose `public_key` field is the wrong length + // must reach the verify path and fail with `PublicKeyDecode`, + // proving the signature path's malformed-bytes rejection works + // for well-framed but cryptographically-invalid inputs. + let bogus = SignedMessage { + public_key: vec![0u8; 7], // not 32 bytes + signature: vec![0u8; 64], + payload: willow_transport::pack(&String::from("anything")).unwrap(), + }; + let bytes = willow_transport::pack(&bogus).unwrap(); + let err = unpack::(&bytes).unwrap_err(); + match err { + IdentityError::PublicKeyDecode(_) => {} + other => panic!("expected IdentityError::PublicKeyDecode for short pk, got {other:?}"), + } + } + /// A key file with fewer than 32 bytes causes `load_or_generate` to return /// an error rather than panic. #[test] From f384f78da12789c8231f453bcac719ec0d207867 Mon Sep 17 00:00:00 2001 From: intendednull Date: Tue, 28 Apr 2026 02:21:37 -0700 Subject: [PATCH 04/10] test(client): happy-path tests for voice mutators (#414) (#464) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Voice mutation API in crates/client/src/mutations.rs had zero test coverage at any tier. Added crates/client/src/tests/voice.rs with one happy-path test per mutator using the test_client() harness — tier 2 per CLAUDE.md (lowest tier covering behaviour, no DOM needed). Coverage: - join_voice: active channel set, local peer in participants - leave_voice: active channel cleared, local peer removed - toggle_mute: returns new value, is_voice_muted reflects, round-trip - toggle_deafen: same shape as toggle_mute - voice_peer_joined: peer inserted into participants, ClientEvent::VoiceJoined fired - voice_peer_left: peer removed, ClientEvent::VoiceLeft fired Tradeoffs: - voice_peer_joined / voice_peer_left arrive in production via the gossip listener unpacking WireMessage::VoiceJoin / VoiceLeave. Driving them end-to-end through a fake wire message would re-test serialisation; the mutators themselves are best exercised directly with a synthetic peer id (mirrors what record_typing does in multi_peer_sync.rs). - join_voice / leave_voice's broadcast_on_topic call no-ops without a subscribed topic, so the test_client harness suffices — no MemHub needed. Wire delivery between peers is exercised in the listener tests already. Refs #414 Co-authored-by: Claude --- crates/client/src/lib.rs | 4 + crates/client/src/tests/voice.rs | 234 +++++++++++++++++++++++++++++++ 2 files changed, 238 insertions(+) create mode 100644 crates/client/src/tests/voice.rs diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index 3199884d..ee7c6299 100644 --- a/crates/client/src/lib.rs +++ b/crates/client/src/lib.rs @@ -68,6 +68,10 @@ mod tests_profile_view; #[path = "tests/ephemeral.rs"] mod tests_ephemeral; +#[cfg(test)] +#[path = "tests/voice.rs"] +mod tests_voice; + /// How long a typing indicator remains visible after the last typing event, in milliseconds. pub const TYPING_INDICATOR_TTL_MS: u64 = 5_000; diff --git a/crates/client/src/tests/voice.rs b/crates/client/src/tests/voice.rs new file mode 100644 index 00000000..c3a25d16 --- /dev/null +++ b/crates/client/src/tests/voice.rs @@ -0,0 +1,234 @@ +//! Happy-path tests for the voice mutation API. +//! +//! Covers the six voice mutators in [`crate::mutations::ClientMutations`]: +//! +//! * `join_voice` — local-state + outbound `WireMessage::VoiceJoin` +//! * `leave_voice` — local-state + outbound `WireMessage::VoiceLeave` +//! * `toggle_mute` — local-only flag toggle +//! * `toggle_deafen` — local-only flag toggle +//! * `voice_peer_joined` — listener-side mutation + `ClientEvent::VoiceJoined` +//! * `voice_peer_left` — listener-side mutation + `ClientEvent::VoiceLeft` +//! +//! These run against the in-memory `test_client` harness — no real +//! network. `join_voice` / `leave_voice` therefore drop their wire-side +//! broadcast (no topic subscribed) but still mutate the voice +//! `StateActor`, which is exactly what the UI binds to. Wire-message +//! delivery between peers is exercised in the gossip listener tests. +//! +//! `voice_peer_joined` / `voice_peer_left` model what happens *after* +//! the listener has unpacked an inbound `WireMessage::VoiceJoin` / +//! `VoiceLeave` — building a fake wire message and routing it through +//! the listener would re-test serialisation rather than the mutator +//! itself, so we call them directly with a synthetic peer id. + +use std::time::Duration; + +use willow_identity::Identity; + +use crate::event_receiver::EventReceiver; +use crate::events::ClientEvent; +use crate::test_client; +use crate::ClientHandle; + +async fn subscribe_rx( + client: &ClientHandle, + broker: &willow_actor::Addr>, +) -> EventReceiver { + EventReceiver::subscribe(broker, &client.system).await +} + +// ───── join_voice ─────────────────────────────────────────────────────── + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn join_voice_sets_active_channel_and_inserts_self() { + let (client, _broker) = test_client(); + let me = client.identity.endpoint_id(); + + client.join_voice("voice-room").await; + + assert_eq!( + client.active_voice_channel().await, + Some("voice-room".to_string()), + "join_voice must set the active channel" + ); + let participants = client.voice_participants("voice-room").await; + assert!( + participants.contains(&me), + "join_voice must insert the local peer into the channel's participant set, got {participants:?}" + ); +} + +// ───── leave_voice ────────────────────────────────────────────────────── + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn leave_voice_clears_active_channel_and_removes_self() { + let (client, _broker) = test_client(); + let me = client.identity.endpoint_id(); + + client.join_voice("voice-room").await; + assert_eq!( + client.active_voice_channel().await, + Some("voice-room".to_string()), + "precondition: join_voice landed" + ); + + client.leave_voice().await; + + assert_eq!( + client.active_voice_channel().await, + None, + "leave_voice must clear the active channel" + ); + let participants = client.voice_participants("voice-room").await; + assert!( + !participants.contains(&me), + "leave_voice must remove the local peer from the participant set, got {participants:?}" + ); +} + +// ───── toggle_mute ────────────────────────────────────────────────────── + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn toggle_mute_flips_state_and_returns_new_value() { + let (client, _broker) = test_client(); + + assert!( + !client.is_voice_muted().await, + "default voice state must be unmuted" + ); + + let after_first = client.toggle_mute().await; + assert!(after_first, "first toggle_mute must return true"); + assert!( + client.is_voice_muted().await, + "is_voice_muted must reflect the toggled state" + ); + + let after_second = client.toggle_mute().await; + assert!(!after_second, "second toggle_mute must return false"); + assert!( + !client.is_voice_muted().await, + "second toggle restores unmuted" + ); +} + +// ───── toggle_deafen ──────────────────────────────────────────────────── + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn toggle_deafen_flips_state_and_returns_new_value() { + let (client, _broker) = test_client(); + + assert!( + !client.is_voice_deafened().await, + "default voice state must be undeafened" + ); + + let after_first = client.toggle_deafen().await; + assert!(after_first, "first toggle_deafen must return true"); + assert!( + client.is_voice_deafened().await, + "is_voice_deafened must reflect the toggled state" + ); + + let after_second = client.toggle_deafen().await; + assert!(!after_second, "second toggle_deafen must return false"); + assert!( + !client.is_voice_deafened().await, + "second toggle restores undeafened" + ); +} + +// ───── voice_peer_joined ──────────────────────────────────────────────── + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn voice_peer_joined_inserts_peer_and_emits_event() { + let (client, broker) = test_client(); + let mut rx = subscribe_rx(&client, &broker).await; + + let other = Identity::generate().endpoint_id(); + client + .mutations() + .voice_peer_joined("voice-room".to_string(), other) + .await; + + let participants = client.voice_participants("voice-room").await; + assert!( + participants.contains(&other), + "voice_peer_joined must insert the remote peer into the participant set, got {participants:?}" + ); + + // Best-effort drain — surface the matching VoiceJoined event. + let mut saw_event = false; + for _ in 0..8 { + match tokio::time::timeout(Duration::from_millis(100), rx.recv()).await { + Ok(Some(ClientEvent::VoiceJoined { + channel_id, + peer_id, + })) if channel_id == "voice-room" && peer_id == other => { + saw_event = true; + break; + } + Ok(Some(_)) => continue, + Ok(None) | Err(_) => break, + } + } + assert!( + saw_event, + "voice_peer_joined must publish a ClientEvent::VoiceJoined for the inserted peer" + ); +} + +// ───── voice_peer_left ────────────────────────────────────────────────── + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn voice_peer_left_removes_peer_and_emits_event() { + let (client, broker) = test_client(); + let mut rx = subscribe_rx(&client, &broker).await; + + let other = Identity::generate().endpoint_id(); + + // Seed the participant set so there's something for `voice_peer_left` + // to remove. Drain the receiver of the resulting `VoiceJoined` so it + // doesn't shadow the event we actually care about asserting. + client + .mutations() + .voice_peer_joined("voice-room".to_string(), other) + .await; + for _ in 0..8 { + match tokio::time::timeout(Duration::from_millis(50), rx.recv()).await { + Ok(Some(ClientEvent::VoiceJoined { .. })) => break, + Ok(Some(_)) => continue, + Ok(None) | Err(_) => break, + } + } + + client + .mutations() + .voice_peer_left("voice-room".to_string(), other) + .await; + + let participants = client.voice_participants("voice-room").await; + assert!( + !participants.contains(&other), + "voice_peer_left must remove the peer from the participant set, got {participants:?}" + ); + + let mut saw_event = false; + for _ in 0..8 { + match tokio::time::timeout(Duration::from_millis(100), rx.recv()).await { + Ok(Some(ClientEvent::VoiceLeft { + channel_id, + peer_id, + })) if channel_id == "voice-room" && peer_id == other => { + saw_event = true; + break; + } + Ok(Some(_)) => continue, + Ok(None) | Err(_) => break, + } + } + assert!( + saw_event, + "voice_peer_left must publish a ClientEvent::VoiceLeft for the removed peer" + ); +} From 286da78e8a9cc6b88475c91b3f8ab675a06a7d78 Mon Sep 17 00:00:00 2001 From: intendednull Date: Tue, 28 Apr 2026 02:39:46 -0700 Subject: [PATCH 05/10] test(client): happy-path tests for governance mutators (#416) (#466) Adds five tests in crates/client/src/tests/governance.rs covering the mutators previously only Playwright-covered (or with zero coverage anywhere): * propose_grant_admin -> Propose { GrantAdmin } * propose_revoke_admin -> Propose { RevokeAdmin } * propose_kick_member -> Propose { KickMember } * propose_set_threshold -> Propose { SetVoteThreshold } * delete_role -> DeleteRole Each test drives the mutator against a `test_client()` fixture (genesis author = owner = automatic admin) and inspects the managed DAG to assert the expected EventKind variant and payload. Downstream materialisation (vote tally, role removal) is tier-1 state-machine territory and stays out of scope, mirroring the convention from voice.rs (#464). Refs #416 Co-authored-by: Claude --- crates/client/src/lib.rs | 4 + crates/client/src/tests/governance.rs | 263 ++++++++++++++++++++++++++ 2 files changed, 267 insertions(+) create mode 100644 crates/client/src/tests/governance.rs diff --git a/crates/client/src/lib.rs b/crates/client/src/lib.rs index ee7c6299..59e59db3 100644 --- a/crates/client/src/lib.rs +++ b/crates/client/src/lib.rs @@ -72,6 +72,10 @@ mod tests_ephemeral; #[path = "tests/voice.rs"] mod tests_voice; +#[cfg(test)] +#[path = "tests/governance.rs"] +mod tests_governance; + /// How long a typing indicator remains visible after the last typing event, in milliseconds. pub const TYPING_INDICATOR_TTL_MS: u64 = 5_000; diff --git a/crates/client/src/tests/governance.rs b/crates/client/src/tests/governance.rs new file mode 100644 index 00000000..9fd9b0cb --- /dev/null +++ b/crates/client/src/tests/governance.rs @@ -0,0 +1,263 @@ +//! Happy-path tests for the governance mutation API. +//! +//! Covers the five governance mutators in [`crate::mutations::ClientMutations`] +//! that previously only had Playwright coverage (or none at all): +//! +//! * `propose_grant_admin` — `EventKind::Propose { ProposedAction::GrantAdmin }` +//! * `propose_revoke_admin` — `EventKind::Propose { ProposedAction::RevokeAdmin }` +//! * `propose_kick_member` — `EventKind::Propose { ProposedAction::KickMember }` +//! * `propose_set_threshold` — `EventKind::Propose { ProposedAction::SetVoteThreshold }` +//! * `delete_role` — `EventKind::DeleteRole` +//! +//! These run against the in-memory `test_client` harness. The genesis +//! author (`test_client()`'s identity) is the server owner, which by +//! the authority model is automatically an admin and the root of +//! trust — so `Propose { … }` and `DeleteRole` events are accepted +//! without further permission setup. +//! +//! Scope: each test only asserts that the mutator emits the expected +//! `EventKind` (variant + payload) into the local DAG. We deliberately +//! do not assert *downstream* state-machine effects (vote tally, +//! actual admin promotion, role removal) — those live in the +//! tier-1 state-machine tests in `crates/state/src/materialize.rs`. +//! Mirroring the convention established by `voice.rs` (PR #464), the +//! focus here is "the mutator emitted the right event". +//! +//! Why poke the DAG instead of intercepting the wire broadcast? +//! `test_client()` does not subscribe to any topic, so +//! `broadcast_event` drops the bytes with a warning. Reading the DAG +//! captures the same signed event the broadcast would carry — the +//! mutator's `apply_event` and `broadcast_event` calls are fed the +//! identical `Event` value. See `mutations.rs` for the call sequence. + +use willow_identity::Identity; +use willow_state::{Event, EventKind, ProposedAction, VoteThreshold}; + +use crate::test_client; +use crate::ClientHandle; + +/// Snapshot every event currently in `client`'s managed DAG, in +/// topological order. The owner-authored mutation under test lands at +/// the tail of this vector once the mutator's `apply_event` returns. +async fn dag_events(client: &ClientHandle) -> Vec { + willow_actor::state::select(&client.dag_addr, |ds| { + ds.managed + .dag() + .topological_sort() + .into_iter() + .cloned() + .collect() + }) + .await +} + +/// Find the first event in the DAG matching `predicate`. Tests use +/// this to assert *exactly one* event of the expected variant landed, +/// without depending on the ordering of unrelated genesis / channel +/// events that `test_client()` seeds. +async fn find_event(client: &ClientHandle, predicate: F) -> Option +where + N: willow_network::Network, + F: Fn(&EventKind) -> bool, +{ + dag_events(client) + .await + .into_iter() + .find(|e| predicate(&e.kind)) +} + +// ───── propose_grant_admin ────────────────────────────────────────────── + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn propose_grant_admin_emits_propose_grant_admin_event() { + let (client, _broker) = test_client(); + let target = Identity::generate().endpoint_id(); + + client + .mutations() + .propose_grant_admin(target) + .await + .expect("owner can propose grant_admin"); + + let event = find_event(&client, |kind| { + matches!( + kind, + EventKind::Propose { + action: ProposedAction::GrantAdmin { .. } + } + ) + }) + .await + .expect("propose_grant_admin must emit a Propose { GrantAdmin } event into the DAG"); + + match event.kind { + EventKind::Propose { + action: ProposedAction::GrantAdmin { peer_id }, + } => { + assert_eq!( + peer_id, target, + "GrantAdmin proposal must target the requested peer_id" + ); + } + other => panic!("expected Propose {{ GrantAdmin }}, got {other:?}"), + } + assert_eq!( + event.author, + client.identity.endpoint_id(), + "propose event must be authored by the local (owner) identity" + ); +} + +// ───── propose_revoke_admin ───────────────────────────────────────────── + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn propose_revoke_admin_emits_propose_revoke_admin_event() { + let (client, _broker) = test_client(); + let target = Identity::generate().endpoint_id(); + + client + .mutations() + .propose_revoke_admin(target) + .await + .expect("owner can propose revoke_admin"); + + // `propose_revoke_admin` *also* fires a follow-up + // `RevokePermission(SendMessages)` (best-effort untrust). That side + // effect is documented on the mutator and is fine — we only assert + // that the Propose event is among the resulting DAG entries. + let event = find_event(&client, |kind| { + matches!( + kind, + EventKind::Propose { + action: ProposedAction::RevokeAdmin { .. } + } + ) + }) + .await + .expect("propose_revoke_admin must emit a Propose { RevokeAdmin } event into the DAG"); + + match event.kind { + EventKind::Propose { + action: ProposedAction::RevokeAdmin { peer_id }, + } => { + assert_eq!( + peer_id, target, + "RevokeAdmin proposal must target the requested peer_id" + ); + } + other => panic!("expected Propose {{ RevokeAdmin }}, got {other:?}"), + } +} + +// ───── propose_kick_member ────────────────────────────────────────────── + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn propose_kick_member_emits_propose_kick_member_event() { + let (client, _broker) = test_client(); + let target = Identity::generate().endpoint_id(); + + client + .mutations() + .propose_kick_member(target) + .await + .expect("owner can propose kick_member"); + + let event = find_event(&client, |kind| { + matches!( + kind, + EventKind::Propose { + action: ProposedAction::KickMember { .. } + } + ) + }) + .await + .expect("propose_kick_member must emit a Propose { KickMember } event into the DAG"); + + match event.kind { + EventKind::Propose { + action: ProposedAction::KickMember { peer_id }, + } => { + assert_eq!( + peer_id, target, + "KickMember proposal must target the requested peer_id" + ); + } + other => panic!("expected Propose {{ KickMember }}, got {other:?}"), + } +} + +// ───── propose_set_threshold ──────────────────────────────────────────── + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn propose_set_threshold_emits_propose_set_threshold_event() { + let (client, _broker) = test_client(); + + client + .mutations() + .propose_set_threshold(VoteThreshold::Unanimous) + .await + .expect("owner can propose set_threshold"); + + let event = find_event(&client, |kind| { + matches!( + kind, + EventKind::Propose { + action: ProposedAction::SetVoteThreshold { .. } + } + ) + }) + .await + .expect("propose_set_threshold must emit a Propose { SetVoteThreshold } event into the DAG"); + + match event.kind { + EventKind::Propose { + action: ProposedAction::SetVoteThreshold { threshold }, + } => { + assert_eq!( + threshold, + VoteThreshold::Unanimous, + "SetVoteThreshold proposal must carry the requested threshold variant" + ); + } + other => panic!("expected Propose {{ SetVoteThreshold }}, got {other:?}"), + } +} + +// ───── delete_role ────────────────────────────────────────────────────── + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn delete_role_emits_delete_role_event() { + let (client, _broker) = test_client(); + + // `delete_role` is a direct admin mutation. We don't have to seed + // a matching `CreateRole` first — the state machine accepts the + // event regardless of whether the role exists, and this tier of + // test is asserting the mutator emits the right event, not the + // downstream materialisation. Use a fixed role id so we can + // pinpoint the event in the DAG. + let role_id = "role-to-delete"; + client + .mutations() + .delete_role(role_id) + .await + .expect("owner can delete_role"); + + let event = find_event(&client, |kind| matches!(kind, EventKind::DeleteRole { .. })) + .await + .expect("delete_role must emit a DeleteRole event into the DAG"); + + match event.kind { + EventKind::DeleteRole { role_id: emitted } => { + assert_eq!( + emitted, role_id, + "DeleteRole event must carry the requested role_id" + ); + } + other => panic!("expected DeleteRole, got {other:?}"), + } + assert_eq!( + event.author, + client.identity.endpoint_id(), + "delete_role event must be authored by the local (owner) identity" + ); +} From 1ab83a4278236fa93368033971e012c811b8f1f2 Mon Sep 17 00:00:00 2001 From: intendednull Date: Tue, 28 Apr 2026 02:54:36 -0700 Subject: [PATCH 06/10] fix(crypto): add RatchetCache::clear() for explicit key eviction (#178) (#469) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RatchetCache stored derived ChannelKey values in BTreeMap + ratchet_states without any caller-driven way to wipe them. ChannelKey already implements ZeroizeOnDrop, but secrets only zeroize when the map drops the entry — so cached keys lingered until eviction pressure hit max_entries or the cache itself was dropped. Add `pub fn clear(&mut self)` that empties both maps. BTreeMap::clear and HashMap::clear drop each value in place, triggering the per-key ZeroizeOnDrop pass before the buckets are released, so secret material is wiped synchronously at the call point. Also add two unit tests: - ratchet_cache_clear_drops_all_state: populate three epochs, call clear(), assert both `cache` and `ratchet_states` are empty + the cache is still usable for fresh derivations. - ratchet_cache_clear_is_idempotent: clear-on-empty is a no-op. Brainstorm — wiring point. RatchetCache is currently API surface only: no client/web code calls `derive_or_cached`, so there is no live production cache to wire into leave_server / sign-out today. Speculatively integrating the cache into the client decrypt path was considered (runner-up) and rejected: that's a perf/correctness change distinct from this security cleanup, and the task explicitly cautions against scope creep. Filing a follow-up so the wire-up is not forgotten when the cache lands in the decrypt hot path. The HashMap that actually holds production keys (crates/client/src/state.rs, state_actors.rs) is already cleaned up correctly: leave_server removes the ServerEntry, dropping its keys map and zeroizing each ChannelKey via ZeroizeOnDrop. Verification: - cargo test -p willow-crypto: 64 passed - cargo clippy --workspace --all-targets -- -D warnings: clean - cargo check --target wasm32-unknown-unknown -p willow-crypto -p willow-client: clean - cargo fmt --check: clean Refs #178 Co-authored-by: Claude --- crates/crypto/src/lib.rs | 63 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/crates/crypto/src/lib.rs b/crates/crypto/src/lib.rs index eedf8372..e9d7023a 100644 --- a/crates/crypto/src/lib.rs +++ b/crates/crypto/src/lib.rs @@ -597,6 +597,19 @@ impl RatchetCache { self.ratchet_states.remove(&epoch); } + /// Drop every cached message key and saved ratchet state. + /// + /// Call this on sign-out, server-leave, or any other identity-bound + /// teardown so derived [`ChannelKey`] material does not linger in + /// process memory longer than necessary. Both [`ChannelKey`] and + /// [`KeyRatchet`] implement [`zeroize::ZeroizeOnDrop`], so removing + /// them from the underlying maps wipes their secret material before + /// the allocation is freed. + pub fn clear(&mut self) { + self.cache.clear(); + self.ratchet_states.clear(); + } + /// Number of entries currently in the cache. pub fn len(&self) -> usize { self.cache.len() @@ -1180,6 +1193,56 @@ mod tests { assert_eq!(fresh.as_bytes(), direct.as_bytes()); } + /// `clear()` must wipe both the message-key cache and the saved + /// per-epoch ratchet state. Issue #178: without explicit eviction, + /// derived `ChannelKey` material lingered in `RatchetCache` past the + /// point where the owning identity / server context was torn down. + #[test] + fn ratchet_cache_clear_drops_all_state() { + let key = generate_channel_key(); + let mut cache = RatchetCache::new(128); + + // Populate multiple epochs so both `cache` and `ratchet_states` + // pick up entries. + let _ = cache.derive_or_cached(&key, 0, 5); + let _ = cache.derive_or_cached(&key, 1, 7); + let _ = cache.derive_or_cached(&key, 2, 3); + + assert!(!cache.is_empty(), "cache should be populated before clear"); + assert!(!cache.ratchet_states.is_empty()); + + cache.clear(); + + assert!(cache.is_empty(), "clear() must empty the message-key cache"); + assert_eq!(cache.len(), 0); + assert!( + cache.ratchet_states.is_empty(), + "clear() must also drop saved ratchet states" + ); + + // The cache must remain functional after clear(): derivations + // still produce correct keys (matching `derive_message_key`), + // proving we did not corrupt the cache, only emptied it. + let post = cache.derive_or_cached(&key, 0, 5); + let direct = derive_message_key(&key, 0, 5); + assert_eq!( + post.as_bytes(), + direct.as_bytes(), + "cache must remain usable after clear()" + ); + } + + /// `clear()` on an already-empty cache is a no-op (idempotent). + #[test] + fn ratchet_cache_clear_is_idempotent() { + let mut cache = RatchetCache::new(64); + assert!(cache.is_empty()); + cache.clear(); + assert!(cache.is_empty()); + cache.clear(); + assert!(cache.is_empty()); + } + /// The cache must not grow beyond `max_entries`. #[test] fn ratchet_cache_respects_max_entries() { From 20f1ed1067c946b7c206c5ed16a698803bc34d21 Mon Sep 17 00:00:00 2001 From: intendednull Date: Tue, 28 Apr 2026 03:18:22 -0700 Subject: [PATCH 07/10] chore: prune #[allow(dead_code)] cluster (#327) (#470) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit audit #327 listed 13 sites. count drifted to 10 (storage.rs and 2 others removed in earlier work). per-site breakdown: deletes (genuinely unused): - crates/web/src/app.rs::toggle_theme — never called; on_toggle_theme Callback wires through palette_actions, not this fn. remove redundant annotation (symbol used): - crates/web/src/palette_recents.rs::clear — used by browser tests. - crates/web/tests/browser.rs::TestShell, mount_test_with_shell, ensure_components_css_loaded, mod test_support — all referenced throughout browser.rs flow tests. - crates/web/src/state.rs::set_pinned_messages — used in state.rs:952. - crates/web/src/state.rs::set_pin_labels — fed by struct init only; field access not warned because crate-wide #![allow(dead_code)] in willow-web/src/lib.rs already masks it. annotation was redundant. - crates/web/src/components/mobile_shell.rs::MobilePush — match arms on Thread/Call/Onboarding count as use; no warning even when one variant is constructed. keep with reason: - crates/actor/src/fsm.rs::DoorState::Open — test fixture for StrictDoor rejection path; never constructed because tests start in Closed and assert further transitions are rejected. Kept with #[allow(dead_code, reason = ...)] (rust 1.81+). verification: cargo check --workspace --all-targets clean; cargo clippy -D warnings clean; cargo test --workspace passes; wasm32 lib + tests build clean. scope-creep guard hit: removing willow-web's crate-wide #![allow(dead_code)] surfaces 9 unrelated dead symbols across 5 files. left as-is — separate follow-up. Refs #327 Co-authored-by: Claude --- crates/actor/src/fsm.rs | 5 ++++- crates/web/src/app.rs | 8 -------- crates/web/src/components/mobile_shell.rs | 1 - crates/web/src/palette_recents.rs | 1 - crates/web/src/state.rs | 2 -- crates/web/tests/browser.rs | 4 ---- 6 files changed, 4 insertions(+), 17 deletions(-) diff --git a/crates/actor/src/fsm.rs b/crates/actor/src/fsm.rs index 4e359059..303e9a0d 100644 --- a/crates/actor/src/fsm.rs +++ b/crates/actor/src/fsm.rs @@ -215,7 +215,10 @@ mod tests { struct StrictDoor; #[derive(Debug, Clone, PartialEq)] - #[allow(dead_code)] + #[allow( + dead_code, + reason = "Open variant exists for completeness; tests start in Closed and assert StrictDoor rejects further transitions." + )] enum DoorState { Open, Closed, diff --git a/crates/web/src/app.rs b/crates/web/src/app.rs index fe896bf9..2b31d1bf 100644 --- a/crates/web/src/app.rs +++ b/crates/web/src/app.rs @@ -45,14 +45,6 @@ fn detect_platform() -> &'static str { } } -#[allow(dead_code)] -pub fn toggle_theme() { - js_sys::eval( - r#"var h=document.documentElement;var c=h.getAttribute('data-theme')||'dark';var n=c==='dark'?'light':'dark';h.setAttribute('data-theme',n);localStorage.setItem('willow-theme',n);"#, - ) - .ok(); -} - /// How many milliseconds to wait before clearing the loading state automatically. const LOADING_TIMEOUT_MS: u32 = 5_000; diff --git a/crates/web/src/components/mobile_shell.rs b/crates/web/src/components/mobile_shell.rs index 2f87d7ed..1741479c 100644 --- a/crates/web/src/components/mobile_shell.rs +++ b/crates/web/src/components/mobile_shell.rs @@ -60,7 +60,6 @@ impl MobileTab { /// Full-screen pushes that can be stacked over a primary tab. Each /// push hides the tab bar and renders translated in from the right. #[derive(Clone, PartialEq, Eq, Debug)] -#[allow(dead_code)] pub enum MobilePush { /// Channel chat view (name = channel id). Channel(String), diff --git a/crates/web/src/palette_recents.rs b/crates/web/src/palette_recents.rs index 4040f948..f360e412 100644 --- a/crates/web/src/palette_recents.rs +++ b/crates/web/src/palette_recents.rs @@ -61,7 +61,6 @@ pub fn push(entry: Recent) { } /// Clear all recents. -#[allow(dead_code)] pub fn clear() { if let Some(s) = storage() { let _ = s.remove_item(KEY); diff --git a/crates/web/src/state.rs b/crates/web/src/state.rs index 5279c681..e1d55e85 100644 --- a/crates/web/src/state.rs +++ b/crates/web/src/state.rs @@ -358,9 +358,7 @@ pub struct ChatWriteSignals { pub set_channels: WriteSignal>, pub set_replying_to: WriteSignal>, pub set_editing: WriteSignal>, - #[allow(dead_code)] pub set_pinned_messages: WriteSignal>, - #[allow(dead_code)] pub set_pin_labels: WriteSignal>, pub set_channel_views: WriteSignal>, } diff --git a/crates/web/tests/browser.rs b/crates/web/tests/browser.rs index bc8e4a5d..4a8869b4 100644 --- a/crates/web/tests/browser.rs +++ b/crates/web/tests/browser.rs @@ -42,7 +42,6 @@ where /// [`mount_test_with_shell`] to pin the choice via a `data-shell` /// attribute on ``. #[derive(Clone, Copy, Debug, PartialEq, Eq)] -#[allow(dead_code)] pub enum TestShell { Desktop, Mobile, @@ -54,7 +53,6 @@ pub enum TestShell { /// `.shell-desktop` / `.shell-mobile` visibility on that attribute /// when present (falls back to the viewport media query when the /// attribute is absent). -#[allow(dead_code)] pub fn mount_test_with_shell(shell: TestShell, view: F) -> web_sys::HtmlElement where F: FnOnce() -> V + 'static, @@ -80,7 +78,6 @@ where /// does not pull in the app's CSS via `index.html`; without this, every /// element keeps its UA-default `display` and the shell override cannot /// be observed. -#[allow(dead_code)] fn ensure_components_css_loaded(doc: &web_sys::Document) { const STYLE_ID: &str = "willow-test-components-css"; if doc.get_element_by_id(STYLE_ID).is_some() { @@ -5771,7 +5768,6 @@ mod notifications { /// `mobile_actions`). Each group used to carry its own copies of /// `click_selector` / `fill_selector` / etc.; they are hoisted here so /// new modules can reuse them without duplication. -#[allow(dead_code)] mod test_support { use super::*; use willow_web::app::App; From 1cda5b85f9ee2ed6e1adf3b97e178b69c0f712e6 Mon Sep 17 00:00:00 2001 From: intendednull Date: Tue, 28 Apr 2026 03:34:42 -0700 Subject: [PATCH 08/10] refactor(web): rename ServerState/ChatState/VoiceState -> *Signals (#261) (#471) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit web crate had three types that name-clash with authoritative willow_state and willow_client types: - web::state::ServerState vs willow_state::ServerState - web::state::ChatState vs willow_client::state::ChatState - web::state::VoiceState vs willow_client::state_actors::VoiceState spec say willow_state::ServerState is THE single source of truth. web sidecar holding Leptos signals shouldnt steal the name. rename web sidecar types to *Signals suffix. signals is what they hold (ReadSignal<...> fields), so name now describe content not fight upstream. mechanical find-replace, no surrounding refactor. callsites: 10 rename touches in 2 files (state.rs + call_page.rs comment). browser tests had no references. authoritative ServerState comments in settings.rs/holder_pill.rs/channel_sidebar.rs left alone — they correctly point at willow_state::ServerState fields (mute_state, channel_keys). verification: - cargo check -p willow-web --target wasm32-unknown-unknown: green - cargo check -p willow-web --tests --target wasm32-unknown-unknown: green - cargo fmt --check: clean - cargo clippy --workspace --all-targets -- -D warnings: zero warnings - cargo test --workspace: all green runner-up: ServerViewSignals/etc — rejected, unnecessarily verbose when *Signals already disambiguates from authoritative *State. Refs #261 Co-authored-by: Claude --- crates/web/src/components/call_page.rs | 2 +- crates/web/src/state.rs | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/web/src/components/call_page.rs b/crates/web/src/components/call_page.rs index e0d882eb..cef0c658 100644 --- a/crates/web/src/components/call_page.rs +++ b/crates/web/src/components/call_page.rs @@ -167,7 +167,7 @@ pub fn CallPage( let handle = use_context::().unwrap(); let vm = use_context::().unwrap(); - // Local video stream — stored globally in VoiceState so it survives remounts. + // Local video stream — stored globally in VoiceSignals so it survives remounts. let local_video_stream = app_state.voice.local_video_stream; // Duration timer — increments every second. Clean up on unmount so diff --git a/crates/web/src/state.rs b/crates/web/src/state.rs index e1d55e85..e5066925 100644 --- a/crates/web/src/state.rs +++ b/crates/web/src/state.rs @@ -81,11 +81,11 @@ pub struct ParsedJoinToken { #[derive(Clone, Copy)] pub struct AppState { - pub chat: ChatState, + pub chat: ChatSignals, pub network: NetworkState, - pub server: ServerState, + pub server: ServerSignals, pub ui: UiState, - pub voice: VoiceState, + pub voice: VoiceSignals, pub trust: TrustState, pub presence: PresenceUiState, /// Local-search UI state (phase 2e — `local-search.md`). @@ -224,7 +224,7 @@ pub struct TrustState { } #[derive(Clone, Copy)] -pub struct ChatState { +pub struct ChatSignals { pub messages: ReadSignal>, pub current_channel: ReadSignal, pub channels: ReadSignal>, @@ -249,7 +249,7 @@ pub struct NetworkState { } #[derive(Clone, Copy)] -pub struct ServerState { +pub struct ServerSignals { pub servers: ReadSignal>, pub active_server_id: ReadSignal, pub active_server_name: ReadSignal, @@ -288,7 +288,7 @@ pub struct UiState { } #[derive(Clone, Copy)] -pub struct VoiceState { +pub struct VoiceSignals { pub voice_channel: ReadSignal>, pub voice_muted: ReadSignal, pub voice_deafened: ReadSignal, @@ -590,7 +590,7 @@ pub fn create_signals() -> InitialSignals { let (profile_open, set_profile_open) = signal(Option::::None); let app_state = AppState { - chat: ChatState { + chat: ChatSignals { messages, current_channel, channels, @@ -608,7 +608,7 @@ pub fn create_signals() -> InitialSignals { connection_state, loading, }, - server: ServerState { + server: ServerSignals { servers, active_server_id, active_server_name, @@ -635,7 +635,7 @@ pub fn create_signals() -> InitialSignals { join_token, join_status, }, - voice: VoiceState { + voice: VoiceSignals { voice_channel, voice_muted, voice_deafened, From 7fbe2c80993afeb4535312704748607553669de8 Mon Sep 17 00:00:00 2001 From: intendednull Date: Tue, 28 Apr 2026 03:53:07 -0700 Subject: [PATCH 09/10] fix(web): make STUN URL configurable, default empty (#179) (#472) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hardcoded `stun:stun.l.google.com:19302` leaked every voice-call participant's public IP to Google. Replace it with a configurable list that defaults to empty — voice ICE now relies on host candidates plus the iroh relay path, so no third-party server learns the user's IP unless the deployment opts in. Knob: `window.__WILLOW_STUN_URLS = ['stun:host:port', ...]`, mirroring the existing `window.__WILLOW_RELAY_URL` override pattern in `crates/web/init.js` / `app.rs::resolve_relay_url`. `init.js` ships the override commented out alongside an example for restoring Google STUN or pointing at a self-hosted server. Brainstorm: chose the JS window global (Option A) over a build-time env var (rebuild-required, less flexible) and over a query-string override (exposed in URLs/logs, doesn't match the existing pattern). Tests (`crates/web/tests/browser.rs::stun_config`): - default `resolve_stun_urls()` returns empty Vec - override populates the Vec - default `RTCConfiguration` has no `iceServers` entries - override surfaces the supplied URL in the config Scope: short-term fix only. Self-hosted STUN (medium-term) and iroh-relay-based ICE traversal (long-term) tracked under #179. Refs #179 Co-authored-by: Claude --- crates/web/init.js | 9 +++ crates/web/src/voice.rs | 108 ++++++++++++++++++++++++++++++++---- crates/web/tests/browser.rs | 106 +++++++++++++++++++++++++++++++++++ 3 files changed, 213 insertions(+), 10 deletions(-) diff --git a/crates/web/init.js b/crates/web/init.js index ec67ce38..cf3b71a4 100644 --- a/crates/web/init.js +++ b/crates/web/init.js @@ -6,4 +6,13 @@ if (!window.__WILLOW_RELAY_URL && (h === '127.0.0.1' || h === 'localhost')) { window.__WILLOW_RELAY_URL = 'http://' + h + ':3340'; } + // STUN server overrides for WebRTC voice calls. Privacy-first default: + // no STUN servers are configured, so voice ICE relies on host candidates + // plus the iroh relay path — no third-party server learns the user's IP. + // See issue #179. + // + // To opt back into Google's public STUN server (leaks your IP to Google): + // window.__WILLOW_STUN_URLS = ['stun:stun.l.google.com:19302']; + // Or point at a self-hosted STUN server: + // window.__WILLOW_STUN_URLS = ['stun:stun.example.com:3478']; })(); diff --git a/crates/web/src/voice.rs b/crates/web/src/voice.rs index ece50b08..e0933187 100644 --- a/crates/web/src/voice.rs +++ b/crates/web/src/voice.rs @@ -283,17 +283,24 @@ impl VoiceManager { self.local_stream = Some(stream); } - /// Build an `RTCConfiguration` with a public STUN server. + /// Build an `RTCConfiguration` honouring the configured STUN URL list. + /// + /// See [`resolve_stun_urls`] for the privacy-first default (empty list) + /// and the `window.__WILLOW_STUN_URLS` override knob. fn rtc_config() -> RtcConfiguration { - let config = RtcConfiguration::new(); - let ice_servers = js_sys::Array::new(); - let server = RtcIceServer::new(); - let urls = js_sys::Array::new(); - urls.push(&"stun:stun.l.google.com:19302".into()); - server.set_urls(&urls); - ice_servers.push(&server); - config.set_ice_servers(&ice_servers); - config + build_rtc_config(&resolve_stun_urls()) + } + + /// Test-only accessor for the resolved `RTCConfiguration`. + /// + /// Browser tests live in a separate compilation unit (the + /// `wasm-bindgen-test` integration harness in `crates/web/tests/`) and + /// cannot see the private `rtc_config`. A thin `pub` wrapper exposes + /// just enough surface for those tests without leaking ICE-config + /// construction into the public API. + #[doc(hidden)] + pub fn rtc_config_for_test() -> RtcConfiguration { + Self::rtc_config() } /// Add local audio tracks (and video track if sharing) to a peer connection. @@ -819,3 +826,84 @@ impl VoiceManager { self.local_stream = None; } } + +/// Resolve the list of STUN server URLs to use for WebRTC ICE gathering. +/// +/// # Privacy-first default +/// +/// Returns an **empty list** by default. WebRTC will then rely on host +/// candidates plus whatever relay path the iroh layer provides — no +/// third-party STUN server learns the user's IP address. +/// +/// Hardcoding `stun:stun.l.google.com:19302` (the previous behaviour) leaked +/// every voice-call participant's public IP to Google. See issue #179. +/// +/// # Override +/// +/// Deployments that need NAT traversal via STUN can set the +/// `window.__WILLOW_STUN_URLS` global to an array of STUN URLs *before* the +/// WASM module boots, e.g. in `init.js`: +/// +/// ```js +/// // Opt back into Google's public STUN server (leaks your IP to Google): +/// window.__WILLOW_STUN_URLS = ['stun:stun.l.google.com:19302']; +/// +/// // Or point at a self-hosted STUN server: +/// window.__WILLOW_STUN_URLS = ['stun:stun.example.com:3478']; +/// ``` +/// +/// Values that are not a JS array, or entries that are not strings, are +/// silently ignored. +/// +/// # Follow-up +/// +/// - Medium term: ship a self-hosted STUN server alongside the relay. +/// - Long term: use the iroh relay path for ICE traversal directly so STUN +/// becomes unnecessary. +pub fn resolve_stun_urls() -> Vec { + let Some(window) = web_sys::window() else { + return Vec::new(); + }; + let raw = match js_sys::Reflect::get( + &window, + &wasm_bindgen::JsValue::from_str("__WILLOW_STUN_URLS"), + ) { + Ok(v) => v, + Err(_) => return Vec::new(), + }; + if raw.is_undefined() || raw.is_null() { + return Vec::new(); + } + let Ok(array) = raw.dyn_into::() else { + return Vec::new(); + }; + let mut out = Vec::with_capacity(array.length() as usize); + for i in 0..array.length() { + if let Some(s) = array.get(i).as_string() { + if !s.is_empty() { + out.push(s); + } + } + } + out +} + +/// Build an `RTCConfiguration` from a resolved list of STUN URLs. +/// +/// An empty `urls` list yields a config with **no** `iceServers` entries — +/// the privacy-first default. See [`resolve_stun_urls`] for rationale. +fn build_rtc_config(urls: &[String]) -> RtcConfiguration { + let config = RtcConfiguration::new(); + let ice_servers = js_sys::Array::new(); + if !urls.is_empty() { + let server = RtcIceServer::new(); + let url_array = js_sys::Array::new(); + for u in urls { + url_array.push(&wasm_bindgen::JsValue::from_str(u)); + } + server.set_urls(&url_array); + ice_servers.push(&server); + } + config.set_ice_servers(&ice_servers); + config +} diff --git a/crates/web/tests/browser.rs b/crates/web/tests/browser.rs index 4a8869b4..f114ea1a 100644 --- a/crates/web/tests/browser.rs +++ b/crates/web/tests/browser.rs @@ -12380,3 +12380,109 @@ mod service_worker_bridge { drop(cb); } } + +// ── STUN configuration (issue #179: privacy-first ICE config) ─────────────── + +mod stun_config { + //! Tests that the WebRTC ICE configuration honours the + //! `window.__WILLOW_STUN_URLS` override and defaults to an empty + //! `iceServers` list (privacy-first — no third-party STUN by default). + //! + //! See `crates/web/src/voice.rs::resolve_stun_urls` and + //! `crates/web/src/voice.rs::VoiceManager::rtc_config`. + + use wasm_bindgen::JsCast; + use wasm_bindgen_test::*; + use willow_web::voice; + + /// Clear any prior override so each test starts from a clean slate. + fn clear_override() { + let window = web_sys::window().expect("window exists"); + let _ = js_sys::Reflect::delete_property( + &window, + &wasm_bindgen::JsValue::from_str("__WILLOW_STUN_URLS"), + ); + } + + /// Set the global override to the supplied list of URLs. + fn set_override(urls: &[&str]) { + let window = web_sys::window().expect("window exists"); + let arr = js_sys::Array::new(); + for u in urls { + arr.push(&wasm_bindgen::JsValue::from_str(u)); + } + js_sys::Reflect::set( + &window, + &wasm_bindgen::JsValue::from_str("__WILLOW_STUN_URLS"), + &arr, + ) + .expect("set window override"); + } + + #[wasm_bindgen_test] + fn default_resolves_to_empty_list() { + clear_override(); + let urls = voice::resolve_stun_urls(); + assert!( + urls.is_empty(), + "default STUN URL list must be empty for privacy (got {urls:?})" + ); + } + + #[wasm_bindgen_test] + fn override_resolves_to_supplied_urls() { + set_override(&["stun:foo:1234", "stun:bar:5678"]); + let urls = voice::resolve_stun_urls(); + clear_override(); + assert_eq!( + urls, + vec!["stun:foo:1234".to_string(), "stun:bar:5678".to_string()] + ); + } + + #[wasm_bindgen_test] + fn default_rtc_config_has_no_ice_servers() { + clear_override(); + let cfg = voice::VoiceManager::rtc_config_for_test(); + // The `iceServers` property should either be absent or an empty array. + let ice_servers = + js_sys::Reflect::get(&cfg, &wasm_bindgen::JsValue::from_str("iceServers")) + .expect("read iceServers"); + if !ice_servers.is_undefined() && !ice_servers.is_null() { + let arr: js_sys::Array = ice_servers + .dyn_into() + .expect("iceServers should be an array if present"); + assert_eq!( + arr.length(), + 0, + "default iceServers must be empty (privacy-first default)" + ); + } + } + + #[wasm_bindgen_test] + fn override_rtc_config_includes_supplied_url() { + set_override(&["stun:example.com:3478"]); + let cfg = voice::VoiceManager::rtc_config_for_test(); + clear_override(); + + let ice_servers = + js_sys::Reflect::get(&cfg, &wasm_bindgen::JsValue::from_str("iceServers")) + .expect("read iceServers"); + let arr: js_sys::Array = ice_servers + .dyn_into() + .expect("iceServers should be an array"); + assert_eq!(arr.length(), 1, "exactly one ice server entry expected"); + + let server = arr.get(0); + let urls = js_sys::Reflect::get(&server, &wasm_bindgen::JsValue::from_str("urls")) + .expect("read urls"); + + // The `urls` field on a single RTCIceServer can be a string or an + // array of strings; web-sys's setter wraps in an array. + let urls_arr: js_sys::Array = urls.dyn_into().expect("urls should be array"); + assert_eq!(urls_arr.length(), 1); + let first = urls_arr.get(0).as_string().expect("url is a string"); + assert_eq!(first, "stun:example.com:3478"); + } +} From 75be15a8ea863858b80e0e23849f5e90610c804a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Apr 2026 10:54:54 +0000 Subject: [PATCH 10/10] =?UTF-8?q?chore(skill):=20sandbox=20notes=20?= =?UTF-8?q?=E2=80=94=20raw=20cargo=20fallback,=20worktree=20residue,=20bro?= =?UTF-8?q?wser-test=20compile-only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .claude/skills/resolving-issues/SKILL.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.claude/skills/resolving-issues/SKILL.md b/.claude/skills/resolving-issues/SKILL.md index a2769ffc..47a3a9d9 100644 --- a/.claude/skills/resolving-issues/SKILL.md +++ b/.claude/skills/resolving-issues/SKILL.md @@ -42,6 +42,7 @@ All sub-fixes land on one master branch per session. Master PR is opened **only - **Sub-PR base ≠ main means GH Actions workflows scoped to `pull_request: branches: [main]` won't fire.** Implementer treats local `just check` green (fmt + clippy + test + wasm) as the merge gate; do not park sub-PR open waiting for CI that won't run. Master PR (base=main, opened end-of-run) runs full CI before human merge — the load-bearing gate. - Implementer watches CI on sub-PR ONLY when CI actually runs (rare; usually requires PR base = main). CI green → merge sub-PR into master branch. No CI run → local `just check` green is the gate, then merge. - CI red after one fix attempt → file follow-up issue (caveman body, link blocker), close sub-PR. Move on. Do NOT leave it as a draft for someone to resume — the next scheduled run picks up the follow-up issue automatically. +- **`just` may be absent in some sandboxes.** Fall back to raw `cargo` equivalents — `cargo fmt --check`, `cargo clippy --workspace --all-targets -- -D warnings`, `cargo test --workspace`, `cargo check --target wasm32-unknown-unknown -p ` for dual-target lib crates. Same gate, different binary. Report which path was used in the sub-PR body. ## Core Loop @@ -130,6 +131,7 @@ Never defer skill edits to a follow-up — they ship with the run that surfaced - Pre-worktree: `git stash` or `git restore` main dir; `.claude/worktrees/` in `.gitignore`. - Worktree per issue, branched off master branch. Tear down after sub-PR merges or parks as draft. +- **Worktree dir may be pre-populated** with residue from a prior session that didn't tear down cleanly. Inspect first: if it contains the same logical work the implementer would do, incorporate it (run gates, finish the workflow). Don't blindly `git worktree remove --force` — that destroys legitimate in-progress work. Reset only if the residue is from an unrelated branch. ## Quality @@ -139,3 +141,4 @@ Never defer skill edits to a follow-up — they ship with the run that surfaced - Master PR opened only at end of run, **non-draft**, with whatever sub-PRs landed. Master PR runs full CI when opened — the actual quality net for the run. - Anything blocked → follow-up issue, sub-PR closed, next scheduled run picks it up. Never open the master PR as draft. Only skip opening the PR entirely if literally zero sub-PRs landed. - No in-session continuation. The issue queue is the durable handoff. +- **Browser tests may be compile-only when wasm-pack / firefox / geckodriver are absent in the sandbox.** `cargo check --target wasm32-unknown-unknown -p willow-web --tests` is the fallback gate — it confirms the test compiles. The full headless run executes on real CI when the master PR opens. Implementer must flag the gap explicitly in the sub-PR body so the human knows what was actually run vs. compile-checked.