auto-fix batch claude/friendly-maxwell-cCGXk 2026-05-04

.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 <scope> --all-targets -- -D warnings` — scope to touched crate(s) for speed; workspace-wide if changes ripple
@@ -113,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/<x>/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/<x>/` or anywhere those crates' tests read at runtime (HTML / CSS / JSON / TOML / YAML / sw.js), still run `cargo test -p <x>` (or at minimum `cargo test -p <x> --test <integration_test_name>` 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.

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"

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]

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;
 

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;

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 { .. })
+        ));
+    }
 }

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));
         }