From ae3ff5b4b3270546fee4b49ac3fa3b8ff772269a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Apr 2026 00:31:14 +0000 Subject: [PATCH 1/6] chore: open auto-fix batch claude/friendly-maxwell-MVZZK From 22fc51bce872d38dcff552da359f4d5e2adae445 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Apr 2026 00:32:58 +0000 Subject: [PATCH 2/6] ci(lint): wire eslint into CI eslint.config.js banned waitForTimeout but no runner enforced it. Add `lint` script to package.json and dedicated `lint` job to ci.yml. placement: ci.yml not e2e.yml. - e2e.yml's npm install hides inside scripts/setup-e2e.sh after the 30-min just test-e2e-full step starts, no dep-install reuse to win. - ci.yml runs parallel jobs on every PR, lint surfaces in seconds. local verify: npm ci + npm run lint -> exit 0 (existing per-file eslint-disable headers tracking #458 keep tree green). Refs #491 --- .github/workflows/ci.yml | 12 ++++++++++++ package.json | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 01e0f6a2..22b3c2fd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,6 +23,18 @@ jobs: components: rustfmt - run: cargo fmt --check + lint: + name: ESLint (e2e/) + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: '20' + - run: npm ci + - run: npm run lint + clippy: name: Clippy runs-on: ubuntu-latest diff --git a/package.json b/package.json index 13868953..591e7c05 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,8 @@ "description": "", "main": "index.js", "scripts": { - "test": "echo \"Error: no test specified\" && exit 1" + "test": "echo \"Error: no test specified\" && exit 1", + "lint": "eslint e2e/" }, "repository": { "type": "git", From b34d82015f0b120648274c0241a4b3987e8b2ba6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Apr 2026 00:37:29 +0000 Subject: [PATCH 3/6] ci(guard): wire test-hooks symbol-leak check into CI + deploy scripts/check-no-test-hooks-in-prod.sh asserts prod trunk build does not leak WillowTestHooks (third-party JS could otherwise read DAG heads via window.__willow.snapshot). Previously only run by `just check-all` -> never enforced on PRs / deploys. ci.yml: dedicated `test-hooks-guard` job, parallel with clippy/test/ wasm/browser. fail-fast on PRs. deploy.yml: belt-and-braces re-run immediately before `Build web app`, catches force-pushed bypass past the CI gate. pattern A (dedicated job) over B (`just check-all` in matrix): - B drags `just test-browser` (Firefox + geckodriver + wasm-pack) into the same job and balloons wall-clock; browser tests already run via the existing `wasm-pack test` job. - matches the sibling 22fc51b eslint wiring (parallel job, fail-fast). action pins copied verbatim from existing ci.yml jobs (full SHAs, trunk@0.21.14 matching deploy.yml). no new action authors -> no dependabot surface bump. note: script uses `trunk build --release --offline`, so the job warms trunk's tool cache (wasm-bindgen-cli) with a non-`--offline` build first; deploy.yml's existing `Build web app` step is now preceded by the guard, so the guard's own trunk build doubles as the cache-warm there. local verify: trunk + just not in sandbox; script aborts at first trunk invocation (exit 127), confirming fail-loud rather than silent-pass on missing toolchain. YAML validated via `python3 -c "import yaml; yaml.safe_load(...)"`. cargo fmt / clippy untouched (workflow YAML only). Refs #490 --- .github/workflows/ci.yml | 28 ++++++++++++++++++++++++++++ .github/workflows/deploy.yml | 8 ++++++++ 2 files changed, 36 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 22b3c2fd..89a74321 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -158,6 +158,34 @@ jobs: echo '' } >> "$GITHUB_STEP_SUMMARY" + test-hooks-guard: + name: Test-hooks symbol-leak guard + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + with: + targets: wasm32-unknown-unknown + - uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + target + key: test-hooks-guard-${{ hashFiles('**/Cargo.lock') }} + restore-keys: test-hooks-guard- + - name: Install trunk + uses: taiki-e/install-action@cf525cb33f51aca27cd6fa02034117ab963ff9f1 # v2.9.4 trunk + with: + tool: trunk@0.21.14 + # Warm trunk's tool cache (wasm-bindgen-cli) so the guard's + # subsequent `--offline` builds succeed on a fresh CI runner. + - name: Warm trunk cache + run: cd crates/web && trunk build --release + - name: Run symbol-leak guard + run: ./scripts/check-no-test-hooks-in-prod.sh + audit: name: cargo-audit runs-on: ubuntu-latest diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 9ed9b83b..740c7b82 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -46,6 +46,14 @@ jobs: - name: Build relay server run: cargo build --release -p willow-relay + # Belt-and-braces: re-run the symbol-leak guard immediately + # before the prod build, in addition to the same guard in ci.yml. + # Catches a tampered/force-pushed branch reaching the deploy job. + # The guard performs its own trunk build, which also warms the + # tool cache for the subsequent `Build web app` step. + - name: Test-hooks symbol-leak guard + run: ./scripts/check-no-test-hooks-in-prod.sh + - name: Build web app run: cd crates/web && trunk build --release From f1c07a174fdcae7d3906051419145d3982792f06 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Apr 2026 00:48:58 +0000 Subject: [PATCH 4/6] fix(web): swap pinned-message scroll from js_sys::eval to safe DOM API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `on_pinned_jump` callback in `crates/web/src/app.rs` interpolated `msg_id` into a `js_sys::eval(format!(...))` string with only single- quote stripping as sanitization. Reachable risk was low (today the id is `EventHash::to_string()`, hex-only), but band-aid sanitization rots and the pattern invites copy-paste. Swap for `web_sys::window() → document() → get_element_by_id() → scroll_into_view_with_scroll_into_ view_options(...)` so adversarial ids become literal lookup keys with no JS context to escape. The other `js_sys::eval` sites in the web crate (theme bootstrap, focus helpers, relay-url probe) all use static JS strings with no interpolation and are out of scope for this fix; the `unsafe-eval` CSP directive comment in `crates/web/tests/static_assets.rs` already tracks them under #171 / #425. Test coverage at the lowest tier covering DOM behavior — wasm-pack browser tests in `crates/web/tests/browser.rs::pinned_jump_safe_scroll`: realistic hex id (positive), missing id (negative, was implicit via old `.ok()`), and adversarial ids with double-quotes / backslashes / newlines / brackets (the cases the old `replace('\'', "")` band-aid did not cover). web-sys 0.3.95 already exposed `ScrollIntoViewOptions` with the setter-style API the issue body proposed (`set_behavior`, `set_block`), and the required features (`Window`, `Document`, `Element`, `ScrollIntoViewOptions`, `ScrollBehavior`, `ScrollLogicalPosition`) were already enabled in `crates/web/Cargo.toml` — no API adaptation, no feature additions needed. Verified: `cargo fmt --check` clean, `cargo check --target wasm32-unknown-unknown -p willow-web [--tests]` clean, `cargo test -p willow-web --lib` passes (73 tests). Browser tests compile-checked via `cargo check --target wasm32-unknown-unknown -p willow-web --tests`; not run locally (sandbox lacks wasm-pack/Firefox/geckodriver) — CI will execute the headless run. Pre-existing clippy warnings in `day_separator.rs` are unrelated and on master HEAD; not touched. Refs #425 --- crates/web/src/app.rs | 13 +++-- crates/web/tests/browser.rs | 96 +++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 4 deletions(-) diff --git a/crates/web/src/app.rs b/crates/web/src/app.rs index c915ddcb..0fdddd48 100644 --- a/crates/web/src/app.rs +++ b/crates/web/src/app.rs @@ -1176,10 +1176,15 @@ pub fn App() -> impl IntoView { queue_open_rail.set(false); }); let on_pinned_jump = Callback::new(move |msg_id: String| { - js_sys::eval(&format!( - "document.getElementById('msg-{}')?.scrollIntoView({{behavior:'smooth',block:'center'}})", - msg_id.replace('\'', "") - )).ok(); + if let Some(elem) = web_sys::window() + .and_then(|w| w.document()) + .and_then(|d| d.get_element_by_id(&format!("msg-{msg_id}"))) + { + let opts = web_sys::ScrollIntoViewOptions::new(); + opts.set_behavior(web_sys::ScrollBehavior::Smooth); + opts.set_block(web_sys::ScrollLogicalPosition::Center); + elem.scroll_into_view_with_scroll_into_view_options(&opts); + } write.ui.set_show_pinned.set(false); }); view! { diff --git a/crates/web/tests/browser.rs b/crates/web/tests/browser.rs index 11c9d1d2..08763ec3 100644 --- a/crates/web/tests/browser.rs +++ b/crates/web/tests/browser.rs @@ -12507,3 +12507,99 @@ mod stun_config { assert_eq!(first, "stun:example.com:3478"); } } + +#[cfg(test)] +mod pinned_jump_safe_scroll { + //! Regression tests for the pinned-message jump callback in + //! `crates/web/src/app.rs` (`on_pinned_jump`). The callback used to + //! interpolate `msg_id` into a `js_sys::eval(format!(...))` string with + //! only single-quote stripping as sanitization; it now uses the safe + //! DOM API: `document.getElementById(&format!("msg-{msg_id}"))` plus + //! `Element::scroll_into_view_with_scroll_into_view_options`. + //! + //! These tests exercise the same DOM-lookup pattern the callback now + //! relies on. The callback body is inline (no extracted helper), so we + //! verify the underlying API contract directly: real-looking IDs hit, + //! missing IDs miss cleanly, and adversarial IDs (quotes, backslashes, + //! brackets, newlines) are treated as literal element IDs that simply + //! match nothing — no DOM injection is possible. + //! + //! Refs: https://github.com/intendednull/willow/issues/425 + //! + //! Note: this test does not assert that scrolling visually happened + //! (jsdom-style headless harnesses don't lay out content). The + //! original `eval` path also silently swallowed errors via `.ok()`, + //! so the property under test is "no panic + correct lookup result", + //! identical for both implementations. + use wasm_bindgen_test::*; + + /// Mimic the lookup path used by `on_pinned_jump` in + /// `crates/web/src/app.rs`: `window → document → get_element_by_id` + /// with the same `msg-{msg_id}` formatting. Returns the matched + /// element if any. + fn lookup_msg_element(msg_id: &str) -> Option { + web_sys::window() + .and_then(|w| w.document()) + .and_then(|d| d.get_element_by_id(&format!("msg-{msg_id}"))) + } + + /// Append a `
` to the body and return the id used. + /// Caller is responsible for unique ids per test to avoid cross-test + /// pollution in the shared document. + fn mount_msg_div(id: &str) { + let doc = web_sys::window().expect("window").document().expect("doc"); + let div = doc.create_element("div").expect("create_element"); + div.set_id(&format!("msg-{id}")); + doc.body() + .expect("body") + .append_child(&div) + .expect("append"); + } + + #[wasm_bindgen_test] + fn lookup_finds_existing_hex_id() { + // EventHash::to_string() is a hex digest; verify a realistic id + // round-trips through the safe lookup the callback now uses. + let id = "deadbeefcafef00d1234567890abcdef"; + mount_msg_div(id); + + let el = lookup_msg_element(id).expect("element with msg- id must be found"); + assert_eq!(el.id(), format!("msg-{id}")); + } + + #[wasm_bindgen_test] + fn lookup_returns_none_for_missing_id() { + // Property the old `eval(...).ok()` chain provided implicitly via + // optional chaining and silent error swallowing: a missing id is + // a no-op, never a panic. The safe API mirrors this with `None`. + let id = "nonexistent_id_does_not_match_any_element_in_the_dom"; + assert!( + lookup_msg_element(id).is_none(), + "missing id must return None, not panic" + ); + } + + #[wasm_bindgen_test] + fn lookup_treats_adversarial_id_as_literal_no_injection() { + // Cases the old `replace('\'', "")` band-aid did not cover: + // double-quote, backslash, newline, brackets, parens. With the + // safe DOM API, these become part of the literal id passed to + // `getElementById`, which simply finds no match — no JS context + // exists for them to break out of, so injection is impossible. + let adversarial_ids = [ + "abc\"';alert(1)//", + "id with spaces and 'quotes' and \"double\"", + "id\\with\\backslashes", + "id\nwith\nnewlines", + "id);scrollIntoView({behavior:'smooth'});//", + "", + ]; + + for bad in adversarial_ids { + assert!( + lookup_msg_element(bad).is_none(), + "adversarial id {bad:?} must not match any element", + ); + } + } +} From 6095f27d9ee054180ac1a30f5ba894717fd6bc19 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Apr 2026 00:52:37 +0000 Subject: [PATCH 5/6] docs(skill): wasm-clippy gate when dual-target lib crate touched native clippy + wasm `cargo check` together miss wasm-only lints. seen this batch in day_separator.rs (3x `as u32` on `js_sys::Date::get_month()` which already returns u32 on wasm). filed as #497. add explicit wasm-clippy step to step 7 gate so future implementers don't repeat. Refs auto-fix batch claude/friendly-maxwell-MVZZK --- .claude/skills/resolving-issues/SKILL.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.claude/skills/resolving-issues/SKILL.md b/.claude/skills/resolving-issues/SKILL.md index e2accc2c..a6d16629 100644 --- a/.claude/skills/resolving-issues/SKILL.md +++ b/.claude/skills/resolving-issues/SKILL.md @@ -88,6 +88,7 @@ Fresh agent per issue, scoped to one issue + master branch ref. Steps: - `cargo clippy --all-targets -- -D warnings` — scope to touched crate(s) for speed; workspace-wide if changes ripple - `cargo test ` — ditto on scope - `cargo check --target wasm32-unknown-unknown ` if dual-target lib crate touched + - `cargo clippy --target wasm32-unknown-unknown --all-targets -- -D warnings` if dual-target lib crate touched — wasm-only lints (e.g. unnecessary `as u32` on `js_sys::Date::get_month()` which already returns `u32` on wasm32, or `js_sys`-only call paths under `#[cfg(target_arch = "wasm32")]`) only fire on the wasm target. Native clippy misses them, and CI's `cargo check --target wasm32` catches compile errors but not lint warnings — the wasm-clippy gate is the only path that closes that gap locally - `just check` if available + scope is wide enough to warrant the full sweep Apply `superpowers:verification-before-completion` — confirm command output before claiming done. From c5452a4902d6acbee8bb5908f66cbbc31b91b2be Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Apr 2026 04:53:21 +0000 Subject: [PATCH 6/6] Revert "ci(guard): wire test-hooks symbol-leak check into CI + deploy" This reverts commit b34d82015f0b120648274c0241a4b3987e8b2ba6. Reverting because the new test-hooks-guard CI job hits a pre-existing wasm-streams duplicate-symbol link error (issue tracked by PR #387): rust-lld: error: duplicate symbol: __wbg_intounderlyingbytesource_free rust-lld: error: duplicate symbol: __wbindgen_describe_intounderlyingbytesource_* ... Two transitive crates pull conflicting versions: wasm-streams 0.4.2 <- server_fn 0.7.8 <- leptos 0.7.8 wasm-streams 0.5.0 <- reqwest 0.13.2 <- iroh-relay Trunk's prod build (cargo build --release --target wasm32-unknown-unknown on the bin target) fails to link. The existing 'browser' CI job hits the same bug but masks it via 'wasm-pack test ... | tee' under bash -e without pipefail (tee's exit 0 swallows wasm-pack's failure). My new step is a single command with no pipe -> bash -e propagates the failure -> CI red. Won't mask via tee; per CLAUDE.md 'No swallowing errors'. #490 has to wait for #387 (leptos 0.7 -> 0.8 upgrade collapses the wasm-streams duplication) to land. Filing follow-up issue noting the dependency chain. Leaving the eslint CI wiring (#491) and the js_sys::eval safe-DOM swap (#425) in place; both are unaffected by the wasm-streams bug. Refs #490, #387 --- .github/workflows/ci.yml | 28 ---------------------------- .github/workflows/deploy.yml | 8 -------- 2 files changed, 36 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 89a74321..22b3c2fd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -158,34 +158,6 @@ jobs: echo '' } >> "$GITHUB_STEP_SUMMARY" - test-hooks-guard: - name: Test-hooks symbol-leak guard - runs-on: ubuntu-latest - timeout-minutes: 15 - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable - with: - targets: wasm32-unknown-unknown - - uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: test-hooks-guard-${{ hashFiles('**/Cargo.lock') }} - restore-keys: test-hooks-guard- - - name: Install trunk - uses: taiki-e/install-action@cf525cb33f51aca27cd6fa02034117ab963ff9f1 # v2.9.4 trunk - with: - tool: trunk@0.21.14 - # Warm trunk's tool cache (wasm-bindgen-cli) so the guard's - # subsequent `--offline` builds succeed on a fresh CI runner. - - name: Warm trunk cache - run: cd crates/web && trunk build --release - - name: Run symbol-leak guard - run: ./scripts/check-no-test-hooks-in-prod.sh - audit: name: cargo-audit runs-on: ubuntu-latest diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 740c7b82..9ed9b83b 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -46,14 +46,6 @@ jobs: - name: Build relay server run: cargo build --release -p willow-relay - # Belt-and-braces: re-run the symbol-leak guard immediately - # before the prod build, in addition to the same guard in ci.yml. - # Catches a tampered/force-pushed branch reaching the deploy job. - # The guard performs its own trunk build, which also warms the - # tool cache for the subsequent `Build web app` step. - - name: Test-hooks symbol-leak guard - run: ./scripts/check-no-test-hooks-in-prod.sh - - name: Build web app run: cd crates/web && trunk build --release