From 18d059be5353ae742be4b71fdb40a80f081811e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 00:28:49 +0000 Subject: [PATCH 01/11] chore: open auto-fix batch claude/friendly-maxwell-UlJEd From 8089a6227169bb1aca84a7dfd1187ec3caa356c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 00:31:25 +0000 Subject: [PATCH 02/11] fix(scripts): use npm ci in setup-e2e MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `npm install` can mutate package-lock.json; `npm ci` installs exactly the lockfile + refuses to mutate it. Deterministic E2E setup demands the strict variant. No Rust changes — only fmt run as smoke check (clippy/test skipped). Refs #530 --- scripts/setup-e2e.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/setup-e2e.sh b/scripts/setup-e2e.sh index e1963a70..e325250a 100755 --- a/scripts/setup-e2e.sh +++ b/scripts/setup-e2e.sh @@ -55,7 +55,7 @@ fi # npm dependencies if [ ! -d "$ROOT/node_modules" ]; then step "Installing npm dependencies..." - (cd "$ROOT" && npm install) + (cd "$ROOT" && npm ci) fi # Playwright browsers. `--dry-run` prints the install location whether From a5e2ad0810abeca32ef5b01478cb299c737ddba0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 00:32:44 +0000 Subject: [PATCH 03/11] build(setup-e2e): pin trunk + just installs with --locked Bare `cargo install` ignores each tool's Cargo.lock, so any compromised transitive dep on crates.io silently lands in the E2E env. `--locked --version X.Y.Z` deterministic. - trunk 0.21.14 matches deploy.yml workflow pin - just 1.50.0 latest stable (no existing pin in workflows) Refs #529 --- scripts/setup-e2e.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/setup-e2e.sh b/scripts/setup-e2e.sh index e325250a..1b083dfc 100755 --- a/scripts/setup-e2e.sh +++ b/scripts/setup-e2e.sh @@ -43,13 +43,13 @@ fi # trunk if ! command -v trunk &>/dev/null; then step "Installing trunk (WASM bundler)..." - cargo install trunk 2>&1 | tail -1 + cargo install --locked --version 0.21.14 trunk 2>&1 | tail -1 fi # just if ! command -v just &>/dev/null; then step "Installing just (task runner)..." - cargo install just 2>&1 | tail -1 + cargo install --locked --version 1.50.0 just 2>&1 | tail -1 fi # npm dependencies From c65ffabb6e89b1ee16c8c6902d14bbb5187df335 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 00:40:31 +0000 Subject: [PATCH 04/11] fix(replay): cap peer HeadsSummary in sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror the storage cap added by PR #507 / b075140 (MAX_AUTHORS_PER_SYNC = 256) on the replay path. Without the guard, ReplayRole::handle_request(WorkerRequest::Sync) iterates a peer-supplied HeadsSummary into a BTreeMap and walks the in-memory DAG once per author — same DoS shape as storage's sync_since/history before #507. Approach A (centralize the const in willow-common alongside SYNC_BATCH_LIMIT) chosen over B (define a local const in replay): the cap is a wire-protocol invariant that BOTH workers must agree on. A single source of truth in willow-common — already a dep of both crates — guarantees they cannot drift. Cost is one extra crate dep edge for willow-replay (already had willow-state, willow-identity, willow-worker, willow-network). Storage's local pub const is removed; it now imports from willow-common. No behavioural change to storage — the value and the bail! sites are byte-identical. Replay uses WorkerResponse::Denied { reason } (sync handler, not anyhow::Result) mirroring the existing "unknown server" branch and the storage error message text. Tests: - sync_request_rejects_oversize_heads (MAX+1 → Denied) - sync_request_accepts_exact_cap_heads (MAX → not Denied) Refs #514 https://claude.ai/code/session_019HhgeDZ5HCbEUygRRLCjde --- Cargo.lock | 1 + crates/common/src/lib.rs | 18 ++++++++ crates/replay/Cargo.toml | 1 + crates/replay/src/role.rs | 90 +++++++++++++++++++++++++++++++++++++ crates/storage/src/store.rs | 18 +------- 5 files changed, 111 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ccf03abd..47ba1bea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6224,6 +6224,7 @@ dependencies = [ "tracing", "tracing-subscriber", "uuid", + "willow-common", "willow-identity", "willow-network", "willow-state", diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index e9ab8c3a..eab85f82 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -24,3 +24,21 @@ pub use worker_types::*; /// `willow-common` (already a dep of both crates) guarantees they stay /// aligned at compile time. pub const SYNC_BATCH_LIMIT: usize = 10_000; + +/// Maximum number of `(author, head)` entries accepted from a peer-supplied +/// [`willow_state::HeadsSummary`] in a single sync / history call. +/// +/// Single source of truth shared by: +/// - `willow-storage` — `sync_since` / `history` build SQL by concatenating +/// one `(author = ? AND seq ?)` fragment per entry; an oversize +/// summary would either exceed rusqlite's bind-parameter limit +/// (default 32766) or waste CPU compiling a giant prepared statement. +/// - `willow-replay` — `handle_request(Sync)` iterates per-author into a +/// `BTreeMap` then walks the in-memory DAG; an oversize summary forces +/// O(N) work and is the same DoS shape as the storage path. +/// +/// 256 is well above any plausible honest server's distinct-author count +/// while keeping bind-parameter and BTreeMap-construction costs bounded. +/// Both production sites MUST agree on this value, so the canonical +/// definition lives here alongside [`SYNC_BATCH_LIMIT`]. +pub const MAX_AUTHORS_PER_SYNC: usize = 256; diff --git a/crates/replay/Cargo.toml b/crates/replay/Cargo.toml index 0e648e25..2f489bd8 100644 --- a/crates/replay/Cargo.toml +++ b/crates/replay/Cargo.toml @@ -12,6 +12,7 @@ willow-worker = { path = "../worker" } willow-state = { path = "../state" } willow-identity = { path = "../identity" } willow-network = { path = "../network" } +willow-common = { path = "../common" } clap = { version = "4", features = ["derive"] } dirs = "6" anyhow = { workspace = true } diff --git a/crates/replay/src/role.rs b/crates/replay/src/role.rs index d51b56a7..1fe2da6d 100644 --- a/crates/replay/src/role.rs +++ b/crates/replay/src/role.rs @@ -7,6 +7,7 @@ use std::collections::{BTreeMap, HashMap}; use tracing::warn; +use willow_common::MAX_AUTHORS_PER_SYNC; use willow_state::{ apply_incremental, Event, EventDag, EventHash, EventKind, HeadsSummary, InsertError, PendingBuffer, ServerState, Snapshot, DEFAULT_PENDING_MAX_AGE_MS, DEFAULT_PENDING_MAX_ENTRIES, @@ -273,6 +274,21 @@ impl WorkerRole for ReplayRole { fn handle_request(&mut self, req: WorkerRequest) -> WorkerResponse { match req { WorkerRequest::Sync { server_id, heads } => { + // Reject peer-supplied summaries that would force O(N) + // BTreeMap construction and DAG walks (see + // `MAX_AUTHORS_PER_SYNC`). Mirrors the storage cap added in + // PR #507 / b075140; gated before any allocation so a hostile + // request fails fast. + if heads.heads.len() > MAX_AUTHORS_PER_SYNC { + return WorkerResponse::Denied { + reason: format!( + "too many heads in sync request: {} > {}", + heads.heads.len(), + MAX_AUTHORS_PER_SYNC + ), + }; + } + let data = match self.servers.get(&server_id) { Some(d) => d, None => { @@ -1458,6 +1474,80 @@ mod tests { } } + // ── Issue #514: oversize HeadsSummary rejection ────────────────────── + // + // Mirrors the storage cap added by PR #507 / b075140. Without a guard + // here, a malicious peer could send a multi-thousand-entry HeadsSummary + // and force replay to do per-author BTreeMap inserts and DAG walks for + // every entry — same DoS shape as the storage path the sibling PR fixed. + + /// Build a `HeadsSummary` with `n` distinct random authors. Mirrors the + /// helper in `crates/storage/src/store.rs` (sibling cap test infra). + fn heads_summary_with_authors(n: usize) -> HeadsSummary { + use willow_state::AuthorHead; + let mut heads = BTreeMap::new(); + for _ in 0..n { + let id = Identity::generate(); + heads.insert( + id.endpoint_id(), + AuthorHead { + seq: 1, + hash: EventHash::ZERO, + }, + ); + } + HeadsSummary { heads } + } + + /// A peer-supplied `HeadsSummary` with more than `MAX_AUTHORS_PER_SYNC` + /// entries must be rejected by `handle_request(Sync)` before any + /// per-author BTreeMap construction or DAG walk occurs. + #[test] + fn sync_request_rejects_oversize_heads() { + use willow_common::MAX_AUTHORS_PER_SYNC; + let mut role = ReplayRole::new(ReplayConfig::default()); + let (_, _) = setup_server(&mut role, "srv-1"); + + let oversize = heads_summary_with_authors(MAX_AUTHORS_PER_SYNC + 1); + let resp = role.handle_request(WorkerRequest::Sync { + server_id: "srv-1".to_string(), + heads: oversize, + }); + + match resp { + WorkerResponse::Denied { reason } => { + assert!( + reason.contains("too many heads"), + "denial reason should mention the cap; got: {reason}" + ); + } + other => panic!("expected Denied for oversize heads, got: {other:?}"), + } + } + + /// `handle_request(Sync)` must accept exactly `MAX_AUTHORS_PER_SYNC` + /// entries — the cap is inclusive on the legal side. + #[test] + fn sync_request_accepts_exact_cap_heads() { + use willow_common::MAX_AUTHORS_PER_SYNC; + let mut role = ReplayRole::new(ReplayConfig::default()); + let (_, _) = setup_server(&mut role, "srv-1"); + + let at_cap = heads_summary_with_authors(MAX_AUTHORS_PER_SYNC); + let resp = role.handle_request(WorkerRequest::Sync { + server_id: "srv-1".to_string(), + heads: at_cap, + }); + + // The peer's heads mention authors we don't know, so events_since + // returns the genesis event; our store has 1 author the peer doesn't, + // so they_are_behind is true. Either branch is acceptable — the only + // forbidden outcome is Denied for at-cap input. + if let WorkerResponse::Denied { reason } = resp { + panic!("at-cap heads must not be denied; got reason: {reason}"); + } + } + /// When the configured `pending_max_entries` is exceeded, the oldest /// entries are evicted and `pending_count()` reflects the cap. #[test] diff --git a/crates/storage/src/store.rs b/crates/storage/src/store.rs index 03028aae..69026a12 100644 --- a/crates/storage/src/store.rs +++ b/crates/storage/src/store.rs @@ -20,25 +20,9 @@ //! never reordered or rewritten so existing databases stay consistent. use rusqlite::{params, Connection}; -use willow_common::SYNC_BATCH_LIMIT; +use willow_common::{MAX_AUTHORS_PER_SYNC, SYNC_BATCH_LIMIT}; use willow_state::{Event, EventKind, HeadsSummary}; -/// Maximum number of `(author, head)` entries accepted from a peer-supplied -/// [`HeadsSummary`] in a single `sync_since` / `history` call. -/// -/// `sync_since` and `history` build their SQL clauses by concatenating one -/// `(author = ? AND seq ?)` fragment per entry. A peer-supplied summary -/// with thousands of entries (still well within the transport envelope cap) -/// would either exceed rusqlite's bind-parameter limit (default 32766) or -/// waste CPU compiling a giant prepared statement. -/// -/// 256 is well above any plausible honest server's distinct-author count -/// while keeping the bind-parameter count (~512 per call) and the SQL -/// expression-tree depth safely below SQLite's defaults -/// (32766 binds, depth 1000). Requests over the cap are rejected at the store layer; -/// the caller in `role.rs` maps the error to a `WorkerResponse::Denied`. -pub const MAX_AUTHORS_PER_SYNC: usize = 256; - /// Ordered list of schema migrations. Each entry is run inside its own /// transaction the first time the database is opened. Once a migration is /// shipped, never edit or reorder it — only append new entries. From d23f3f88b99687ae29b17a03ac651f267a2a5ebe Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 00:47:34 +0000 Subject: [PATCH 05/11] fix(web): treat sub-1ms transition as zero-duration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit is_zero_duration only matched "", "0s", "0ms" — but the global prefers-reduced-motion rule in style.css forces transition-duration to 0.01ms !important on every element, which engines serialise as either "0.01ms" or "0.0001s". Both slipped past the strict matcher, so mobile_shell, confirm_dialog, bottom_sheet, grove_drawer, and message reactions all sat waiting for a transitionend that never fires under reduced-motion — UI hang. Replace the string-equality check with parse_duration_seconds (parses both s and ms suffixes) and accept anything ≤ 1ms (epsilon) as zero. Unparseable input stays conservative (not zero). Sibling-of-closed audit follow-up to #496 (8d89f18). Approach A (parse-and-compare) chosen over B (hardcode the two known strings) because reduced-motion is the authoritative contract — any sub-millisecond duration is indistinguishable from "no transition" for transitionend purposes, so a numeric threshold is the durable fix. Tests added (native, no DOM): parse_duration_seconds_handles_units, parse_duration_seconds_rejects_malformed, is_zero_duration_str_recognises_explicit_zero, is_zero_duration_str_recognises_reduced_motion_override, is_zero_duration_str_treats_sub_millisecond_as_zero, is_zero_duration_str_rejects_real_durations, is_zero_duration_str_multi_value_all_zero, is_zero_duration_str_multi_value_mixed_is_not_zero, is_zero_duration_str_unparseable_is_not_zero. Gates: fmt clean, clippy native + wasm32 clean (-D warnings), 86 willow-web lib tests pass, wasm32 --tests check clean (wasm-pack / geckodriver not available in env — used cargo check --target wasm32-unknown-unknown --tests as fallback gate per CLAUDE.md). Refs #515 https://claude.ai/code/session_019HhgeDZ5HCbEUygRRLCjde --- crates/web/src/components/lifecycle.rs | 137 +++++++++++++++++++++++-- 1 file changed, 127 insertions(+), 10 deletions(-) diff --git a/crates/web/src/components/lifecycle.rs b/crates/web/src/components/lifecycle.rs index aaea2921..64e6e470 100644 --- a/crates/web/src/components/lifecycle.rs +++ b/crates/web/src/components/lifecycle.rs @@ -52,15 +52,68 @@ pub const fn advance(state: LifecycleState) -> LifecycleState { } } +/// Maximum transition duration (in seconds) that we treat as effectively +/// zero — i.e. "skip the wait, snap synchronously". 1ms is the threshold +/// because the global reduced-motion override at `style.css` writes +/// `transition-duration: 0.01ms !important` for every element, and we must +/// honour that as the authoritative reduced-motion contract. Any user CSS +/// using ≤1ms transitions is also indistinguishable from "no transition" +/// for the purpose of `transitionend` firing reliably across engines. +const ZERO_DURATION_EPSILON_SECONDS: f64 = 0.001; + +/// Parse a single CSS `