From 7559e1f8ffd78067bfe8ae506852224472a0bfa7 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 4 May 2026 00:10:22 +0000 Subject: [PATCH 01/10] chore: open auto-fix batch claude/friendly-maxwell-cCGXk From 6a765eae0fe4a237b4f5469f6676c5cc2c448d40 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 4 May 2026 00:14:50 +0000 Subject: [PATCH 02/10] docs(identity): pin verify_strict invariant + test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit (issue #575) flagged `verify` for using ed25519-dalek's non-strict API. Investigation: `iroh_base::PublicKey::verify`, which our standalone `verify` delegates to, already calls `verify_strict` internally (iroh-base 0.98.0 src/key.rs:134-136). The vulnerability does not exist in the deployed call chain, but the invariant is load-bearing and silently breakable by future refactors. Defense in depth: document the strict-mode dependency at the call site so anyone changing this function knows the contract, and add a regression test that constructs a non-canonical S component (top bit of byte 63 set, pushing S above the curve order ℓ) and asserts `verify` rejects it. The test pins the malleability vector closed regardless of whether the underlying iroh-base wrapper changes. Refs #575 --- crates/identity/src/lib.rs | 39 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/crates/identity/src/lib.rs b/crates/identity/src/lib.rs index 92b5cecc..227a6121 100644 --- a/crates/identity/src/lib.rs +++ b/crates/identity/src/lib.rs @@ -333,6 +333,14 @@ impl std::fmt::Debug for Identity { // ───── Standalone verification ────────────────────────────────────────────── /// Verify a signature against a public key without needing an [`Identity`]. +/// +/// This delegates to [`PublicKey::verify`] from `iroh-base`, which internally +/// uses `ed25519_dalek::VerifyingKey::verify_strict` (RFC 8032 strict mode). +/// Strict verification rejects non-canonical signature encodings — in particular, +/// signatures whose `S` component is not reduced modulo the curve order — which +/// closes off Ed25519 signature malleability vectors. Defense in depth: any +/// future change that bypasses `iroh-base`'s wrapper must continue to use the +/// strict primitive. pub fn verify(key: &PublicKey, data: &[u8], sig: &Signature) -> bool { key.verify(data, sig).is_ok() } @@ -745,6 +753,37 @@ mod tests { assert!(!verify(&id.public_key(), b"wrong data", &sig)); } + /// Regression: `verify` must use strict Ed25519 verification (RFC 8032), + /// rejecting signatures whose `S` component is non-canonical (S ≥ ℓ). + /// + /// Setting the top bit of byte 63 of `S` makes `S` larger than the curve + /// order ℓ ≈ 2^252 + …, so non-strict verification might accept it while + /// strict verification must reject it. This pins the call-site invariant: + /// the underlying primitive is `verify_strict`, providing defense in depth + /// against signature malleability. + #[test] + fn verify_rejects_non_canonical_s_component() { + let id = Identity::generate(); + let data = b"test data"; + let sig = id.sign(data); + + // Original signature is valid. + assert!(verify(&id.public_key(), data, &sig)); + + // Mutate the S scalar's high bit. S occupies bytes 32..64 of the + // signature; its most-significant byte is at index 63. Flipping the top + // bit pushes S above the curve order, producing a non-canonical encoding + // that strict verification rejects. + let mut bytes = sig.to_bytes(); + bytes[63] ^= 0x80; + let mutated = Signature::from_bytes(&bytes); + + assert!( + !verify(&id.public_key(), data, &mutated), + "verify must reject non-canonical S (signature malleability vector)" + ); + } + #[test] fn endpoint_id_hash_map_key() { use std::collections::HashMap; From a3cc10c6bfddd834080b4000d42ac8bdcde24b64 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 4 May 2026 00:20:48 +0000 Subject: [PATCH 03/10] fix(state): check deps cap before sig verify EventDag::insert previously called event.verify() (Ed25519 + bincode + blake3, ~50us) BEFORE the cheap structural caps (MAX_EVENT_DEPS, MAX_ENCRYPTED_KEY_BYTES). An attacker holding any SendMessages permission could broadcast events with pathologically large deps; every receiver paid full sig-verify cost on each rejected event. Reorder so cheap O(1)/O(n) syntactic checks fire first and the crypto verify path only runs once an event passes the structural caps. The intent comment at the top of insert() already described this design ("Reject at the inbound DAG boundary so over-cap events never even reach applied_events"); this commit makes the code match that intent. Adds a regression test that constructs an event with deps.len() > MAX_EVENT_DEPS and a clobbered (all-zero) signature; the test asserts InsertError::DepsTooLong (not InvalidSignature), which proves the structural cap short-circuits before sig-verify. Pre-existing willow-state::tests_materialize::non_admin_set_profile_is_accepted failure (regression from PR #505, tracked at #565) is unrelated to this change and was already failing on HEAD. Refs #519 --- crates/state/src/dag.rs | 62 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/crates/state/src/dag.rs b/crates/state/src/dag.rs index 8fceefae..8d275897 100644 --- a/crates/state/src/dag.rs +++ b/crates/state/src/dag.rs @@ -127,18 +127,21 @@ impl EventDag { /// The first event must be `EventKind::CreateServer` with seq=1 and /// prev=ZERO. Unknown deps are silently accepted (soft-accept). pub fn insert(&mut self, event: Event) -> Result<(), InsertError> { - // 1. Verify signature. - if !event.verify() { - return Err(InsertError::InvalidSignature); - } - - // 2. Anti-DoS vector caps (SEC-V-07). + // 1. Anti-DoS vector caps (SEC-V-07). // // A peer holding any permission could otherwise broadcast events // with pathologically large `deps` or `encrypted_keys` blobs that // every other peer would clone into a `BTreeMap` during apply. // Reject at the inbound DAG boundary so over-cap events never // even reach `applied_events` / `materialize`. + // + // ORDERING: cheap structural caps first; signature verify last to + // short-circuit DoS. Ed25519 + bincode + blake3 verify costs ~50µs + // per event — an attacker with `SendMessages` could otherwise force + // every receiver to pay that cost on each over-cap event before + // rejection. The syntactic checks here are O(1) / O(n) over already- + // deserialized fields, so running them first lets us drop pathological + // payloads without ever invoking the expensive crypto path. if event.deps.len() > MAX_EVENT_DEPS { return Err(InsertError::DepsTooLong { got: event.deps.len(), @@ -156,6 +159,11 @@ impl EventDag { } } + // 2. Verify signature. + if !event.verify() { + return Err(InsertError::InvalidSignature); + } + // 3. Check duplicate. if self.events.contains_key(&event.hash) { return Err(InsertError::Duplicate); @@ -681,6 +689,48 @@ mod tests { assert!(matches!(err, InsertError::InvalidSignature)); } + /// SEC-V-07 regression: structural caps must short-circuit BEFORE the + /// expensive Ed25519 verify path, so an attacker with `SendMessages` + /// permission cannot force receivers to pay sig-verify cost on + /// pathologically over-cap events. Construct an event whose signature + /// is obviously bogus AND whose deps exceed `MAX_EVENT_DEPS`; we expect + /// `DepsTooLong` (not `InvalidSignature`), which proves the cheap cap + /// was checked first. + #[test] + fn insert_deps_cap_rejects_before_signature_verify() { + use willow_identity::Signature; + + let id = Identity::generate(); + let mut dag = test_dag(&id); + + // Build an event with deps.len() > MAX_EVENT_DEPS, then clobber the + // signature with all-zero bytes so that any attempt to verify would + // fail. If verify ran first we'd see `InvalidSignature`; if the + // structural cap runs first we get `DepsTooLong`. + let oversize_deps = vec![EventHash::ZERO; MAX_EVENT_DEPS + 1]; + let mut event = dag.create_event( + &id, + EventKind::SetProfile { + display_name: "dos".into(), + }, + oversize_deps, + 0, + ); + event.sig = Signature::from_bytes(&[0u8; 64]); + + let err = dag.insert(event).unwrap_err(); + assert!( + matches!( + err, + InsertError::DepsTooLong { + got, + max: MAX_EVENT_DEPS, + } if got == MAX_EVENT_DEPS + 1 + ), + "expected DepsTooLong (cheap cap first), got {err:?}", + ); + } + #[test] fn insert_rejects_seq_gap() { let id = Identity::generate(); From 9a700dd7f3f552237d40d629be8b58ad5212f885 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 4 May 2026 00:30:00 +0000 Subject: [PATCH 04/10] fix(messaging): cap Content::File filename + mime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content::File carried a self-declared u64 size_bytes plus unbounded filename / mime_type strings, all peer-supplied. Until now nothing rejected a 256 KB filename or MIME, and the size field had no warning that it was attacker-declared. Add MAX_FILENAME_BYTES = 255 (POSIX NAME_MAX) and MAX_MIME_BYTES = 255 (POSIX-aligned, comfortably above RFC 6838's 127+127 type/subtype limit) and expose Content::validate / Message::validate that reject oversized values with MessageValidationError. Wire validation into InMemoryStore::insert so peer-supplied messages cannot be persisted without first clearing the structural bounds. Document size_bytes as advisory-only — UIs may display it but must not use it for any preallocation or trust decision. The natural earlier ingress point sits in client/listeners.rs, but that file is locked under PR #566; an inline NOTE in store.rs flags the follow-up so the client side can also gate validation once #566 lands. No size_bytes preallocation hazards were found in the tree. Refs #583 --- crates/messaging/src/lib.rs | 211 +++++++++++++++++++++++++++++++++- crates/messaging/src/store.rs | 20 +++- 2 files changed, 227 insertions(+), 4 deletions(-) diff --git a/crates/messaging/src/lib.rs b/crates/messaging/src/lib.rs index 021e1cd0..d56aefc4 100644 --- a/crates/messaging/src/lib.rs +++ b/crates/messaging/src/lib.rs @@ -92,6 +92,51 @@ impl std::fmt::Display for ChannelId { // ───── Content types ───────────────────────────────────────────────────────── +/// Maximum allowed length, in bytes, for a [`Content::File`] filename. +/// +/// Matches POSIX `NAME_MAX` (255). Peer-supplied filenames longer than +/// this are rejected by [`Content::validate`]. +pub const MAX_FILENAME_BYTES: usize = 255; + +/// Maximum allowed length, in bytes, for a [`Content::File`] MIME type. +/// +/// RFC 6838 §4.2 caps the registered `type/subtype` form at 127+127 +/// characters; we use 255 as a conservative POSIX-aligned bound that +/// covers all realistic media types plus parameters. Peer-supplied +/// MIME types longer than this are rejected by [`Content::validate`]. +pub const MAX_MIME_BYTES: usize = 255; + +/// Errors returned by [`Content::validate`] / [`Message::validate`] when a +/// peer-supplied payload exceeds the structural bounds enforced by this +/// crate. +/// +/// These checks guard against unbounded `String` fields in `Content::File` +/// being abused to inflate gossip payloads or memory footprint after a +/// message has already cleared signature verification. The `Content` enum +/// is decoded by `bincode` before any structural check runs, so callers +/// receiving peer-supplied `Content` MUST invoke `validate()` on the +/// decoded value before trusting any of its fields. +#[derive(Debug, thiserror::Error, PartialEq, Eq)] +pub enum MessageValidationError { + /// `Content::File::filename` exceeded [`MAX_FILENAME_BYTES`]. + #[error("filename too long: {actual} bytes (max {max})")] + FilenameTooLong { + /// Observed length in bytes. + actual: usize, + /// Configured maximum. + max: usize, + }, + + /// `Content::File::mime_type` exceeded [`MAX_MIME_BYTES`]. + #[error("mime_type too long: {actual} bytes (max {max})")] + MimeTypeTooLong { + /// Observed length in bytes. + actual: usize, + /// Configured maximum. + max: usize, + }, +} + /// The payload inside a [`Message`]. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub enum Content { @@ -105,11 +150,21 @@ pub enum Content { File { /// Content-addressed hash of the file data. hash: String, - /// Original filename. + /// Original filename. Bounded to [`MAX_FILENAME_BYTES`] by + /// [`Content::validate`]; values exceeding the cap are rejected + /// at the inbound boundary. filename: String, - /// MIME type (e.g. `image/png`). + /// MIME type (e.g. `image/png`). Bounded to [`MAX_MIME_BYTES`] + /// by [`Content::validate`]; values exceeding the cap are + /// rejected at the inbound boundary. mime_type: String, - /// File size in bytes. + /// File size in bytes, as declared by the sender. + /// + /// **Attacker-declared.** UI MAY display this for the user but + /// MUST NOT use it for preallocation (`Vec::with_capacity`, + /// `reserve`, etc.), allocation hints, or trust decisions. The + /// authoritative size is whatever the content-addressed `hash` + /// resolves to once the file bytes are actually fetched. size_bytes: u64, }, @@ -155,6 +210,45 @@ pub enum Content { Encrypted(SealedContent), } +impl Content { + /// Validate structural bounds on peer-supplied content. + /// + /// `Content` is a wire enum decoded with `bincode` before any + /// structural check runs, so callers handling peer-supplied values + /// MUST invoke this method after decoding and before trusting the + /// fields. Currently the only bounded fields are + /// [`Content::File::filename`] (≤ [`MAX_FILENAME_BYTES`]) and + /// [`Content::File::mime_type`] (≤ [`MAX_MIME_BYTES`]); other + /// variants always validate successfully. + /// + /// `Content::Encrypted` is intentionally not recursed into here — + /// the inner `Content` is opaque ciphertext until the channel key + /// is applied; callers should re-invoke `validate()` on the + /// decrypted `Content` before using its fields. + pub fn validate(&self) -> Result<(), MessageValidationError> { + if let Content::File { + filename, + mime_type, + .. + } = self + { + if filename.len() > MAX_FILENAME_BYTES { + return Err(MessageValidationError::FilenameTooLong { + actual: filename.len(), + max: MAX_FILENAME_BYTES, + }); + } + if mime_type.len() > MAX_MIME_BYTES { + return Err(MessageValidationError::MimeTypeTooLong { + actual: mime_type.len(), + max: MAX_MIME_BYTES, + }); + } + } + Ok(()) + } +} + /// The encrypted form of a [`Content`] value. /// /// Produced by encrypting a serialized `Content` with ChaCha20-Poly1305. @@ -258,6 +352,16 @@ impl Message { } } + /// Validate structural bounds on this message's payload. + /// + /// Convenience wrapper around [`Content::validate`]; see that + /// method's docs for the contract. Callers receiving a peer-supplied + /// [`Message`] over the wire MUST invoke this before trusting any + /// fields of the inner [`Content`]. + pub fn validate(&self) -> Result<(), MessageValidationError> { + self.content.validate() + } + /// Create a file-sharing message. pub fn file( channel_id: ChannelId, @@ -535,4 +639,105 @@ mod tests { }; assert_eq!(sealed.ratchet_counter, 0); } + + // ── validate() bounds ─────────────────────────────────────────────────── + + fn file_content(filename: &str, mime_type: &str) -> Content { + Content::File { + hash: "deadbeef".into(), + filename: filename.into(), + mime_type: mime_type.into(), + size_bytes: 0, + } + } + + #[test] + fn validate_rejects_oversized_filename() { + let too_long = "a".repeat(MAX_FILENAME_BYTES + 1); + let content = file_content(&too_long, "image/png"); + assert_eq!( + content.validate(), + Err(MessageValidationError::FilenameTooLong { + actual: MAX_FILENAME_BYTES + 1, + max: MAX_FILENAME_BYTES, + }) + ); + } + + #[test] + fn validate_rejects_oversized_mime_type() { + let too_long = "a".repeat(MAX_MIME_BYTES + 1); + let content = file_content("ok.txt", &too_long); + assert_eq!( + content.validate(), + Err(MessageValidationError::MimeTypeTooLong { + actual: MAX_MIME_BYTES + 1, + max: MAX_MIME_BYTES, + }) + ); + } + + #[test] + fn validate_accepts_filename_at_boundary() { + let at_cap = "a".repeat(MAX_FILENAME_BYTES); + let mime_at_cap = "a".repeat(MAX_MIME_BYTES); + let content = file_content(&at_cap, &mime_at_cap); + assert!(content.validate().is_ok()); + } + + #[test] + fn validate_accepts_empty_filename_and_mime() { + let content = file_content("", ""); + assert!(content.validate().is_ok()); + } + + #[test] + fn validate_is_noop_for_non_file_variants() { + let cases = [ + Content::Text { + body: "x".repeat(1024), + }, + Content::Reaction { + target: MessageId::new(), + emoji: "👍".into(), + }, + Content::Reply { + parent: MessageId::new(), + body: "ok".into(), + }, + Content::Edit { + target: MessageId::new(), + new_body: "edited".into(), + }, + Content::Delete { + target: MessageId::new(), + }, + Content::System { + description: "joined".into(), + }, + Content::Encrypted(SealedContent { + ciphertext: vec![1, 2, 3], + nonce: [0u8; 12], + key_epoch: 0, + ratchet_counter: 0, + }), + ]; + for content in &cases { + assert!(content.validate().is_ok(), "expected ok for {content:?}"); + } + } + + #[test] + fn message_validate_delegates_to_content() { + let mut hlc = hlc::HLC::new(); + let peer = Identity::generate().endpoint_id(); + let channel = ChannelId::new(); + let too_long = "a".repeat(MAX_FILENAME_BYTES + 1); + + let msg = Message::file(channel, peer, "hash", too_long, "image/png", 42, &mut hlc); + assert!(matches!( + msg.validate(), + Err(MessageValidationError::FilenameTooLong { .. }) + )); + } } diff --git a/crates/messaging/src/store.rs b/crates/messaging/src/store.rs index 5ae7d8e0..edbf34ee 100644 --- a/crates/messaging/src/store.rs +++ b/crates/messaging/src/store.rs @@ -10,7 +10,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use willow_identity::EndpointId; -use crate::{hlc::HlcTimestamp, ChannelId, Message, MessageId}; +use crate::{hlc::HlcTimestamp, ChannelId, Message, MessageId, MessageValidationError}; /// Delivery state for a single message. /// @@ -48,6 +48,12 @@ pub enum StoreError { /// The requested message was not found. #[error("message not found: {0}")] NotFound(MessageId), + + /// Attempted to insert a message that failed structural validation + /// (e.g. peer-supplied `Content::File` with an oversized filename + /// or MIME type). See [`MessageValidationError`] for the variants. + #[error("invalid message: {0}")] + Invalid(#[from] MessageValidationError), } /// Trait for message storage backends. @@ -184,6 +190,18 @@ impl InMemoryStore { impl MessageStore for InMemoryStore { fn insert(&mut self, message: Message) -> Result<(), StoreError> { + // Structural validation guards against unbounded peer-supplied + // `Content::File` strings (filename, mime_type) being persisted + // without ever passing through a length check. Tracked in #583. + // + // NOTE: Ideally validation also gates earlier, at the network + // ingress in `crates/client/src/listeners.rs`, before a `Message` + // ever reaches a store. Those files are in flight under PR #566 + // (coordinator decision); revisit after merge so peers can't + // pin oversized payloads in client memory even when the store + // is bypassed. + message.validate()?; + if self.messages.contains_key(&message.id) { return Err(StoreError::DuplicateId(message.id)); } From 39a9f1d9e0fbee9e10bf8196113ccd5df9e99a5b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 4 May 2026 00:31:51 +0000 Subject: [PATCH 05/10] fix(web): allow https: img-src for auto-embed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #243 added referrerpolicy="no-referrer" to external tags in the chat auto-embed code path (crates/web/src/components/message.rs), signalling clear intent to render external images while protecting referrer privacy. CSP img-src 'self' data: blob: blocked them in the browser, so embeds silently failed. Add https: (HTTPS only — http: would risk a mixed-content downgrade on the page) to img-src, restoring the design intent. is_image_url() also accepts http:// schemes; the resulting CSP/filter mismatch is out-of-scope for this fix and will be tracked separately if it proves to be a real path peers exercise. Browser-side CSP enforcement was not testable in the dispatch sandbox (no Chrome/Firefox); change is verified syntactically and via cargo fmt. Existing Playwright e2e CI gate on the master branch will catch any HTML / CSP regressions on merge. Refs #584 --- crates/web/index.html | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/web/index.html b/crates/web/index.html index 11506818..db43d79c 100644 --- a/crates/web/index.html +++ b/crates/web/index.html @@ -21,11 +21,15 @@ relay's /bootstrap-id HTTP endpoint. * img-src and media-src include data:/blob: so that avatar data URIs and runtime URL.createObjectURL blobs (file attachments, - inline media) render. frame-ancestors 'none' blocks clickjacking; + inline media) render. img-src additionally allows https: so the + chat auto-embed code path (see issue #584 / PR #243, which added + referrerpolicy="no-referrer" to external tags) can fetch + remote images over HTTPS without a mixed-content downgrade. + frame-ancestors 'none' blocks clickjacking; base-uri / form-action / object-src lock down the remaining injection surfaces. --> - + From 6846feea868bc950fca7e73436331d0eb98b42ee Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 4 May 2026 00:37:43 +0000 Subject: [PATCH 06/10] fix(client): use web_time::Instant in WorkerCache WorkerCache called std::time::Instant::now() in a willow-client lib crate. willow-client must build for wasm32-unknown-unknown per the dual-target rule, and std::time::Instant compiles on WASM but its monotonic source is not wired through Performance.now() without explicit gating. Documented trip-hazard. Replace std::time::Instant with web_time::Instant. API-compatible drop-in: native uses std::time::Instant, WASM uses Performance.now() automatically. No behaviour change on native; correct semantics on WASM. TTL eviction continues to work on both targets (gating it out on WASM would have left dead workers in the cache, defeating TTL). Picked over cfg-gate-eviction (wrong semantics on web) and HLC-derived clock (wider scope, no benefit). Both rejected in favour of the minimal-change portable replacement. Cargo.lock churn: web-time was already a transitive dep; the only addition is willow-client's direct entry. Strictly additive. Test gap: wasm-bindgen-test is not wired into willow-client, so the wasm-target test from the audit suggestion was skipped. Coverage is provided by `cargo check --target wasm32-unknown-unknown -p willow-client` and the matching clippy gate, both of which pass and catch the original compile-time linkage concern. Refs #545 --- Cargo.lock | 1 + Cargo.toml | 4 ++++ crates/client/Cargo.toml | 1 + crates/client/src/worker_cache.rs | 3 ++- 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index d447c70a..591b7280 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6146,6 +6146,7 @@ dependencies = [ "wasm-bindgen", "wasm-bindgen-futures", "web-sys", + "web-time", "willow-actor", "willow-common", "willow-crypto", diff --git a/Cargo.toml b/Cargo.toml index 6d055b08..e45a87c9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,10 @@ blake3 = "1" # Time & IDs chrono = { version = "0.4", features = ["serde"] } uuid = { version = "1", features = ["v4", "serde", "js"] } +# Portable monotonic clock. Native uses `std::time::Instant`; WASM uses +# `Performance.now()`. Drop-in replacement for `std::time::Instant` so +# lib crates can build for both targets without `#[cfg]` gates. +web-time = "1" # Logging tracing = "0.1" diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index a6b6c4c3..da60ec8c 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -33,6 +33,7 @@ uuid = { workspace = true } tracing = { workspace = true } parking_lot = { workspace = true } futures = { workspace = true } +web-time = { workspace = true } regex = "1" [target.'cfg(not(target_arch = "wasm32"))'.dependencies] diff --git a/crates/client/src/worker_cache.rs b/crates/client/src/worker_cache.rs index 613ded6b..dadbb898 100644 --- a/crates/client/src/worker_cache.rs +++ b/crates/client/src/worker_cache.rs @@ -5,8 +5,9 @@ //! (default 30s) without a heartbeat. use std::collections::HashMap; -use std::time::{Duration, Instant}; +use std::time::Duration; +use web_time::Instant; use willow_common::{WorkerAnnouncement, WorkerRoleInfo}; use willow_identity::EndpointId; From 450a4e857acd0bdb14967721ce8ce76d517d3da4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 4 May 2026 00:39:10 +0000 Subject: [PATCH 07/10] docs(skill): cargo.lock conflicts are additive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skill say "skip files in flight" for in-flight PR overlap. Trip hazard: Cargo.lock in PR X's file list, your fix need new dep, implementer think file locked, abort. Wrong — Cargo.lock add strictly additive (your row append, theirs stay). Merge trivial. Refusing on Cargo.lock alone create infinite "wait for PR X" wait loop, block dep upgrade + wasm-fix forever. Note churn in commit body, ship the dep, let merge handle. Same logic apply to workspace Cargo.toml [workspace.dependencies] table — additive only. Coordinator brief should pre-authorise. Surface in PR #545 dispatch (web_time::Instant for WorkerCache wasm fix); coordinator brief explicit authorise the Cargo.lock churn so implementer no abort. --- .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 88aa0df2..82b48e08 100644 --- a/.claude/skills/resolving-issues/SKILL.md +++ b/.claude/skills/resolving-issues/SKILL.md @@ -99,6 +99,9 @@ Fresh agent per issue, scoped to one issue + master branch ref. Steps: 6. **Scope-creep guard:** if root-cause fix touches > 5 files OR > 200 LOC AND brainstorm in step 3 didn't already approve that scope, return to coordinator with a brainstorm note before pushing. Coordinator decides: split, defer, or proceed. Don't unilaterally balloon a small-scope ticket. **Mechanical call-site migration is part of the fix, not scope creep.** If the fix changes a small API (e.g. swapping `map.insert(k, v)` for `lru.insert(k, v)` to make a new cap take effect), every call-site rewrite is load-bearing — without them the cap is dead code. Count them in the LOC delta but don't abort just because they push past 200. Justify the count in the brainstorm + commit body so the human can see why the fan-out was unavoidable. Real scope creep = unrelated cleanup, drive-by refactors, "while I'm in here" tweaks — those still abort. + + **Cargo.lock conflicts with in-flight PRs are usually additive — don't refuse the dep.** When a fix needs a new workspace dep and `Cargo.lock` is in another open PR's file list, don't abort: dep additions are strictly additive in `Cargo.lock` (your row gets appended; the other PR's rows stay untouched). Merge resolution post-PR-#X is mechanical. Note the `Cargo.lock` churn in the commit body so the human reviewer expects a small textual conflict on the master PR's merge. Refusing on this basis creates infinite "wait for PR X to merge" deadlocks that block dependency upgrades and dual-target fixes indefinitely. Same logic applies to additive-only edits in any "shared by every change" file (e.g. workspace `Cargo.toml`'s `[workspace.dependencies]` table) — coordinator-narrowed briefs should explicitly authorise additive churn. + 7. **Local merge gate.** Run, in order: - `cargo fmt --all -- --check` (or `just fmt-check` if available) - `cargo clippy --all-targets -- -D warnings` — scope to touched crate(s) for speed; workspace-wide if changes ripple From 89f55a21a9c39141a9e7fe423ebb18a7f95dabaf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 4 May 2026 01:00:54 +0000 Subject: [PATCH 08/10] test(web): track CSP img-src https: change Commit 39a9f1d updated index.html CSP img-src to include https: (for auto-embed support per #584), but the static_assets integration test asserts the exact pre-change substring `img-src 'self' data: blob:` is present. The substring no longer appears verbatim, so CI's Test job fails. Update REQUIRED_CSP_DIRECTIVES to match the production CSP. Refs #584 --- crates/web/tests/static_assets.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/web/tests/static_assets.rs b/crates/web/tests/static_assets.rs index 593d9af3..cd44c083 100644 --- a/crates/web/tests/static_assets.rs +++ b/crates/web/tests/static_assets.rs @@ -130,7 +130,7 @@ const REQUIRED_CSP_DIRECTIVES: &[&str] = &[ // 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:", + "img-src 'self' https: data: blob:", "media-src 'self' blob:", "worker-src 'self'", "object-src 'none'", From 57fddc65d69e321ad4506344492f047fe4a20810 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 4 May 2026 01:01:52 +0000 Subject: [PATCH 09/10] docs(skill): static-asset tests need cargo test #584 brief tell implementer skip cargo test, "no Rust code change, HTML only." Wrong. crates/web/tests/static_assets.rs assert exact CSP directive substring in index.html. Change CSP, substring move, test red on master-PR CI (#598 Test job failed). Cost ~10 min CI rescue dispatch + extra commit on master branch. Avoidable: even non-Rust change need cargo test on crates whose integration tests read static asset at runtime (HTML/CSS/JSON/TOML /YAML/sw.js). Skill now mandate cargo test -p for any file in crates// regardless of language. Few seconds local, save CI red + rescue dispatch + commit noise. --- .claude/skills/resolving-issues/SKILL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/skills/resolving-issues/SKILL.md b/.claude/skills/resolving-issues/SKILL.md index 82b48e08..0554cfc7 100644 --- a/.claude/skills/resolving-issues/SKILL.md +++ b/.claude/skills/resolving-issues/SKILL.md @@ -116,6 +116,8 @@ Fresh agent per issue, scoped to one issue + master branch ref. Steps: **Browser tests (`wasm-pack` + Firefox + geckodriver) may be unavailable.** `cargo check --target wasm32-unknown-unknown -p willow-web --tests` is the fallback gate — confirms the test compiles. Real headless run executes on master-PR CI. Flag the gap in the commit body. + **Non-Rust file changes still need `cargo test` on related crates.** Coordinator briefs sometimes argue "CSS / HTML / JSON / YAML / `.well-known` change — no Rust touched, skip cargo test." This is a trap: integration tests (`crates//tests/*.rs`) commonly assert on the contents of static assets — e.g. `crates/web/tests/static_assets.rs` greps `index.html` for required CSP directives, manifest.json for icon refs, etc. A non-Rust change that flips an asserted substring is a CI-red landmine. **Default rule:** if you touched any file in `crates//` or anywhere those crates' tests read at runtime (HTML / CSS / JSON / TOML / YAML / sw.js), still run `cargo test -p ` (or at minimum `cargo test -p --test ` if you can identify the relevant integration suite). Cost is a few seconds; saves a master-PR CI-rescue dispatch (~5-10 min) and a noisy `wip`-shape commit on the master branch. + 8. **Commit + push.** Use `caveman:caveman-commit` for the message. Conventional Commits format. `Refs #N` (NOT `Fixes` — that lives only on the master PR). Push directly to origin master branch. **Never push `wip:` / `chore: checkpoint` / "work-in-progress" commits to the master branch.** Make all your edits, run the local gate, then commit ONCE with the proper Conventional Commits message. Coordinator's master PR body assembles `Fixes #N` rows from per-commit messages — `wip:` rows look like junk to a human reviewer and force a finalize-implementer detour to squash + force-push (real cost: ~10–15 min of cargo lock contention while the rescue agent re-runs gates, plus a `--force-with-lease` against a branch that may have already accumulated more commits from later dispatches if the coordinator misjudges sequencing). If you genuinely need intermediate checkpoints (long sessions, sandbox interference per next bullet), make them in a Pattern B local feature branch and squash via `git merge --no-ff` when done — never push intermediate commits to master. From 667615619d660e3675d03f2e4e247c7d71810f31 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 4 May 2026 07:35:36 +0000 Subject: [PATCH 10/10] test(state): bind grant dep on profile event The non_admin_set_profile_is_accepted test was flaky: admin's GrantPermission and alice's SetProfile were causally independent in the DAG (each on its own per-author prev chain, empty deps), so materialize()'s topo-sort between them depended on HashMap iter order. When SetProfile applied before GrantPermission, the membership gate (PR #505) rejected it and the assertion failed. Wire SetProfile's deps to the GrantPermission event hash. This forces topo-sort to apply the grant first, deterministically. Production behavior (membership gate) unchanged; this is the test-side fix from issue #565's two valid options. Refs #565 --- crates/state/src/tests/materialize.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/state/src/tests/materialize.rs b/crates/state/src/tests/materialize.rs index 134f884e..610f61a0 100644 --- a/crates/state/src/tests/materialize.rs +++ b/crates/state/src/tests/materialize.rs @@ -46,7 +46,7 @@ fn non_admin_set_profile_is_accepted() { let alice = Identity::generate(); // Grant SendMessages so alice is a member. - do_emit( + let grant = do_emit( &mut dag, &admin, EventKind::GrantPermission { @@ -55,13 +55,18 @@ fn non_admin_set_profile_is_accepted() { }, ); // Alice sets her own profile (no admin needed). - do_emit( - &mut dag, + // Causal dep on grant ensures topo-sort applies GrantPermission first + // (PR #505's membership gate); without this dep the test was flaky + // depending on HashMap iter order. See issue #565. + let set_profile = dag.create_event( &alice, EventKind::SetProfile { display_name: "Alice".to_string(), }, + vec![grant.hash], + 0, ); + dag.insert(set_profile).unwrap(); let state = materialize(&dag); assert_eq!(