auto-fix batch 2026-04-28 (claude/friendly-maxwell-Oggvw)#473
Merged
Conversation
…462) 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 <noreply@anthropic.com>
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
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
… (#469) 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<String, ChannelKey> 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
) (#471) 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…rowser-test compile-only
intendednull
pushed a commit
that referenced
this pull request
Apr 28, 2026
Resolves two conflicts: 1. crates/client/src/lib.rs — both branches added test modules. Kept all three: tests_actions (this batch, #420), tests_voice + tests_governance (PR #473, #414/#416). `cargo check -p willow-client --tests` clean post-resolution. 2. .claude/skills/resolving-issues/SKILL.md — both branches edited the skill. Resolved by full rewrite per human review feedback on PR #484: drop worktrees and sub-PRs entirely, work sequentially on the master branch directly (or via local feature branches that merge back into master without going through GitHub PRs). Skill rewrite captures the new workflow shape: - One master branch per session, all fixes land sequentially. - No sub-PRs (they were causing merge confusion + silent CI skip when sub-PR base ≠ main). - No worktrees (sequential = no isolation needed; also dodged the sandbox signing-service incompatibility documented in the prior skill version). - Implementer commits directly to master branch (Pattern A) or via a local feature branch + `git merge --no-ff` back into master (Pattern B). Master PR opens at end of run with everything in it. Preserved from the prior skill: - Coordinator-never-codes rule (with two narrow exceptions: session-open commit + Lessons Learned skill edits). - Complexity gate (auto-brainstorm + plan). - Already-fixed-upstream + stale-audit-with-residual-gap paths. - Caveman GH comms. - Sequential between issues, fresh agent per issue, max 10 per run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
8 small-scope fixes swept from open issue queue. All sub-PRs merged via local cargo gate (sub-PR base ≠ main → CI no run). Master PR runs full CI on open — load-bearing gate.
Fixes
crates/web/index.html(sub-PR fix(web): add CSP meta tag to index.html (#175) #462).js_sys::evalsites still live, kept'unsafe-eval'until [security] XSS-prone js_sys::eval() in pinned message jump #171 / [WS-1] Web: js_sys::eval(format!()) for pinned-message scroll uses band-aid sanitization #425 close.crates/identity/src/lib.rs(sub-PR test(identity): malformed-bytes rejection paths (#424) #463).crates/client/src/tests/voice.rs(sub-PR test(client): happy-path tests for voice mutators (#414) #464).crates/client/src/tests/governance.rs(sub-PR test(client): happy-path tests for governance mutators (#416) #466).RatchetCache::clear()+ 2 unit tests, drops both maps forZeroizeOnDrop(sub-PR fix(crypto): add RatchetCache::clear() for explicit key eviction (#178) #469). Wiring deferred — see## Follow-ups.#[allow(dead_code)]cluster in web/state and web tests #327 —#[allow(dead_code)]cluster pruned, 13 → small (sub-PR chore: prune #[allow(dead_code)] cluster (#327) #470). Crate-wide#![allow(dead_code)]incrates/web/src/lib.rsleft as-is, hit scope guard.ServerStatename collision betweenwillow-stateandwillow-web#261 — web-sideServerState/ChatState/VoiceStaterenamed to*Signals, 10 callsites (sub-PR refactor(web): rename ServerState/ChatState/VoiceState -> *Signals (#261) #471).window.__WILLOW_STUN_URLS, default empty Vec (privacy-first), 4 browser tests (sub-PR fix(web): make STUN URL configurable, default empty (#179) #472).Skill Evolution
Skill commit on this branch:
75be15a— three sandbox notes folded into.claude/skills/resolving-issues/SKILL.md:justmay be absent in some sandboxes — implementer falls back to rawcargoequivalents (same gate, different binary). Note added to### Sub-PR rules.--forcereset. Note added to## Setup.cargo check --target wasm32-unknown-unknown -p willow-web --testsis the fallback gate, real headless run executes on master-PR CI. Note added to## Quality.Follow-ups
RatchetCache::clear()into leave-server / sign-out paths once decrypt path uses the cache. Filed by [security] RatchetCache retains old channel keys in memory without explicit eviction #178 implementer, scope-out for this run.Lessons Learned
git fetch + reset --hard origin/<master>) worked clean across 8 dispatches. Each implementer worktree had latest prior merges as base.willow-actor::send_dead_actor_returns_send_error(passes in isolation, fails undercargo test --workspace). Not blocking — unrelated to this batch.Test plan
just check(fmt + clippy + test + wasm + browser).just dev+ agent-browser navigation through web UI to confirm CSP didn't regress mount, voice config didn't regress join flow,*Signalsrename didn't regress reactivity.Generated by Claude Code