CI / check (push) Successful in 4m17s
Closes the final phase of docs/chat-hardening-plan.md. Two problems: a
locally echoed message always looked sent even when the core had no active
session or the gossip broadcast failed; and a fast burst could broadcast
'successfully' yet be silently dropped by every receiver's per-author rate
bucket (8 burst, then 1/s) with no sender feedback.
Send status: CoreCommand::SendChat/SendChatFile carry a local-only id (never
on the wire); the core replies with UiEvent::ChatSendResult after the gossip
broadcast succeeds or fails, and a no-active-session is now an explicit
failure rather than a silent no-op. gossip send_chat, which previously
returned Ok on a missing sender/topic or an encode failure, now returns Err.
ChatEntry gains local_send: Option<LocalSend>; failed sends render a red
'Not sent — {reason} [Retry]' line, Broadcast/Pending render nothing
(there are no delivery receipts, so silence is the honest success state).
Sender-side pacing (new src/app/sendqueue.rs): sends past the burst queue
locally as 'queued…' and trickle out at the receivers' sustained rate, so
nothing is lost and typing is never blocked (user chose queue-and-trickle
over input throttling). The pacer reuses the gossip gate's own TokenBucket +
per-author constants (now pub(crate)) so the two sides of the policy can't
drift. A 250ms drain subscription runs only while the queue is non-empty.
Retry re-dispatches the retained payload; re-serving the same attachment id
replaces the ServeStore entry rather than double-counting bytes. The pacer
and monotonic send-id counter survive a room reset (receivers' buckets
persist; ids never alias a late result); queue and retry payloads are cleared.
582 lib tests (+11: 4 pacer/queue seam, 7 app-level transition/retry/reset);
all-targets green, clippy -D warnings clean, fmt clean, smoke launch OK. No
wire change (GOSSIP_PROTO stays 5). Tests-green-only — the two owed
two-machine field-test items are logged in the plan.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
585 lines
35 KiB
Markdown
585 lines
35 KiB
Markdown
# Chat hardening — ephemeral implementation plan
|
||
|
||
**Status (2026-07-18):** Phases 1–5 COMPLETE (all plan phases done). Phase 1 =
|
||
shared text policy in `src/sanitize.rs`, ceilings enforced at UI input, sign
|
||
point, and gossip ingress. Phase 2 = roster-bound authorship
|
||
(`src/core/chatroster.rs`), replay dedup + rate limits (`ChatIngressGate` in
|
||
`src/network/gossip.rs`). Phase 3 = attachment cache/serve-store budgets,
|
||
downscaled previews, auto-fetch byte/request budgets (`src/core/fetchbudget.rs`),
|
||
exact transfers, bounded local reads. Phase 4 = parsed-URL link policy
|
||
(`is_safe_web_url`/`link_ranges` in `src/sanitize.rs`, `url` crate), 8-link cap,
|
||
cached link ranges in `ChatEntry`, 512 KiB history text budget, chat-body
|
||
bidi-override strip (closes S14). Phase 5 = honest local send status
|
||
(`CoreCommand::SendChat`/`SendChatFile` carry a local id, `UiEvent::ChatSendResult`,
|
||
`SendStatus` on own echoes) PLUS sender-side pacing (`src/app/sendqueue.rs`
|
||
mirrors the receivers' per-author budget so fast bursts trickle instead of being
|
||
silently dropped downstream). All gates green each phase. This is a temporary
|
||
scope contract for hardening the existing room chat; with every phase complete
|
||
and the two-machine field test done, delete this file (see the completion note
|
||
at the end). The two-machine field-test section below is still owed before that
|
||
deletion. Do not add link previews as part of this effort.
|
||
|
||
## Goal
|
||
|
||
Strengthen the current encrypted, signed, session-only room chat without changing
|
||
its product model: plain selectable text, clickable web links, and peer-to-peer
|
||
attachments over the existing gossip and files planes. The work should make chat
|
||
resistant to identity spoofing, replay, spam, oversized input, expensive rendering,
|
||
and attachment-driven memory/bandwidth pressure while preserving normal Unicode
|
||
conversation and the existing full-mesh architecture.
|
||
|
||
## Existing foundation to preserve
|
||
|
||
- Gossip payloads are signed by the claimed `EndpointId`, bound to the raw room
|
||
topic and protocol domain, and checked before dispatch.
|
||
- The signed envelope timestamp is admitted only within the two-minute gossip
|
||
freshness window.
|
||
- Inbound gossip frames are capped at 128 KiB before JSON deserialization. This
|
||
larger plane-wide cap must remain because `Announce` may contain a custom avatar.
|
||
- Chat history is session-only and capped at 300 entries.
|
||
- Only `http://` and `https://` links are opened, as a single process argument
|
||
without a shell.
|
||
- Attachment descriptors are signed with the chat payload; attachment bytes use
|
||
the encrypted files plane, have a 25 MiB per-file cap, and are keyed by both
|
||
author and attachment id.
|
||
- Image bytes are decoded defensively and automatic image fetches already have a
|
||
four-task concurrency limit.
|
||
|
||
## Working design decisions
|
||
|
||
These are the implementation defaults unless code inspection or tests reveal a
|
||
concrete reason to adjust them. Record any adjustment in the decision log.
|
||
|
||
1. **No wire change.** Keep `GossipMessage::Chat` unchanged and do not bump
|
||
`GOSSIP_PROTO`. The redundant wire `name` and inner `Chat.ts` remain serialized
|
||
for compatibility but are not trusted. Remove them only during a future planned
|
||
gossip-version bump.
|
||
2. **Roster identity is authoritative.** A chat line is admitted only for an
|
||
authenticated identity already known to the current room (including the
|
||
reconnect grace state). Its displayed name comes from the sanitized roster
|
||
state, never from `GossipMessage::Chat.name`.
|
||
3. **Body Unicode remains expressive.** Do not apply the short-label sanitizer to
|
||
the message body; it strips format characters used by some languages and emoji.
|
||
Continue neutralizing controls and whitespace, while treating author labels,
|
||
filenames, and URLs more strictly because those are spoof-sensitive surfaces.
|
||
4. **Bounds apply at every trust boundary.** UI input is bounded while editing,
|
||
outgoing text is normalized before signing, and incoming text is byte-checked
|
||
and normalized before it leaves the gossip layer. UI-only truncation is not an
|
||
adequate ingress defense.
|
||
5. **Automatic network work is stricter than manual work.** Keep the 25 MiB manual
|
||
attachment ceiling, but auto-fetch only small images. Larger images remain
|
||
available behind an explicit Load/Download action.
|
||
6. **Caches are bounded by cost, not only entry count.** Count encoded bytes and
|
||
estimated decoded image bytes. A count cap remains as a secondary bound.
|
||
7. **Rate limiting degrades quietly.** Drop excess/replayed peer messages with a
|
||
rate-limited log entry. Do not let a spammer produce a second UI-notification
|
||
flood.
|
||
|
||
## Proposed policy constants
|
||
|
||
Keep these together near the code that enforces them and cover them with boundary
|
||
tests. Values are starting points, not a compatibility contract.
|
||
|
||
| Policy | Initial value | Reason |
|
||
| --- | ---: | --- |
|
||
| Chat body characters | 2,000 | Preserves current UI behavior |
|
||
| Chat body UTF-8 bytes | 8 KiB | Covers 2,000 four-byte scalars with small headroom |
|
||
| Live input characters/bytes | Same as body | Prevent oversized paste/edit state |
|
||
| Clickable links per message | 8 | Bounds spans and opener targets |
|
||
| Retained chat text | 512 KiB plus 300 entries | Bounds redraw and selection work |
|
||
| Per-author chat limiter | Burst 8, refill 1/second | Allows normal bursts, stops sustained spam |
|
||
| Room-wide chat limiter | Burst 32, refill 8/second | Protects shared event/UI queues |
|
||
| Exact-chat replay cache | 1,024 digests, 2-minute TTL | Covers freshness window with a hard bound |
|
||
| Auto-fetch image encoded size | 4 MiB | Limits unsolicited bandwidth and allocations |
|
||
| Attachment cache encoded budget | 128 MiB | Allows several ordinary files without GiB growth |
|
||
| Attachment cache decoded-preview budget | 64 MiB | Bounds renderer-side image pressure |
|
||
| Served attachment budget | 256 MiB plus a count cap | Bounds sender memory for a long session |
|
||
| Inline preview longest side | 1,600 px | Chat renders near 260 px; full 4K decode is wasteful |
|
||
| Decoded source image pixels | 16 megapixels maximum | Adds a total-pixel bound to per-side bounds |
|
||
|
||
## Phase 1 — Shared text policy and live-input bounds
|
||
|
||
**Target:** downstream layers never receive or retain an unexpectedly large or
|
||
unsafe chat string.
|
||
|
||
- [x] Move chat constants and `sanitize_chat` from `src/app/mod.rs` into
|
||
`src/sanitize.rs` (or a narrowly scoped shared chat-policy module if that keeps
|
||
the API clearer).
|
||
- [x] Implement a single-pass sanitizer that:
|
||
- maps control characters to spaces;
|
||
- collapses whitespace and trims ends;
|
||
- enforces both the character and UTF-8 byte ceilings without splitting a scalar;
|
||
- returns empty for content with no visible text.
|
||
- [x] Add `cap_chat_input` for live editing. It must preserve the user's current
|
||
whitespace while enforcing character and byte ceilings; normalization remains a
|
||
submit/ingress operation so typing does not visibly jump.
|
||
- [x] Apply `cap_chat_input` in `AppMessage::ChatInputChanged`, covering keyboard,
|
||
clipboard, primary-selection, and context-menu paste paths through the controlled
|
||
input widget.
|
||
- [x] Sanitize outgoing text immediately before local echo and `CoreCommand` send.
|
||
- [x] Sanitize again before `GossipMessage::Chat` is signed, so a future non-UI
|
||
caller cannot bypass policy.
|
||
- [x] At gossip ingress, reject raw chat text over the byte ceiling before doing
|
||
downstream sanitization; sanitize accepted text before creating `RoomEvent`.
|
||
- [x] Keep attachment-only messages when the sanitized caption is empty; drop a
|
||
chat with neither visible text nor a valid attachment.
|
||
- [x] Stop sanitizing an incoming chat `name` with the body sanitizer. Phase 2
|
||
replaces it with the roster-bound name.
|
||
|
||
### Phase 1 tests
|
||
|
||
- [x] ASCII, multibyte Unicode, emoji, whitespace, NUL/CR/LF/TAB/ESC, empty input.
|
||
- [x] Exact character and byte boundaries, including a four-byte scalar at the
|
||
cutoff.
|
||
- [x] Oversized paste never makes `state.chat_input` exceed either ceiling.
|
||
- [x] Outgoing, incoming, and direct core/network paths converge on the same
|
||
normalized result.
|
||
- [x] Empty captions are retained only when a valid attachment remains.
|
||
|
||
## Phase 2 — Admission, identity binding, replay, and spam control
|
||
|
||
**Target:** only current authenticated room members can create chat UI work, and a
|
||
member cannot impersonate another participant or monopolize the control/UI queues.
|
||
|
||
- [x] Change the core event task's chat roster from a bare `HashSet<EndpointId>` to
|
||
a bounded map containing each member's latest sanitized display name (or retain a
|
||
parallel name map if less invasive).
|
||
- [x] Insert/update the map on `PeerJoined`/`PeerUpdated`, retain it during transient
|
||
reconnect grace, and remove it on graceful or terminal eviction.
|
||
- [x] Before attachment handling or UI forwarding, reject `RoomEvent::ChatMessage`
|
||
whose author is not present in that authoritative roster.
|
||
- [x] Replace the embedded wire name with the roster map's name before constructing
|
||
`UiEvent::ChatMessage`. The UI may keep storing a name snapshot so old chat lines
|
||
remain labeled after a peer leaves.
|
||
- [x] Add a lightweight early known-author gate in the gossip loop using its live
|
||
and disconnected-peer sets. Keep the core roster gate as defense in depth and as
|
||
the final authority.
|
||
- [x] Validate that the inner `Chat.ts` equals the signed envelope timestamp, or
|
||
ignore it entirely. Do not use the inner timestamp for replay or ordering.
|
||
- [x] Add exact-chat replay suppression after signature verification and before
|
||
event-channel send:
|
||
- hash the canonical signed bytes, not raw JSON formatting;
|
||
- use BLAKE3 (make it a direct dependency if needed; it is already in the iroh
|
||
dependency graph) or an equally collision-resistant existing primitive;
|
||
- store a `HashSet` plus FIFO/TTL order for bounded lookup and eviction;
|
||
- prune by both the gossip freshness window and the hard entry cap.
|
||
- [x] Add a bounded token bucket per admitted author and a room-wide bucket before
|
||
awaiting `event_tx.send`. Limiter state must be removed with roster eviction and
|
||
remain bounded by the roster cap.
|
||
- [x] Ensure duplicate messages are dropped before consuming rate-limit tokens, so
|
||
a replay cannot starve a legitimate new message from that author.
|
||
- [x] Rate-limit rejection logging per author/reason.
|
||
- [ ] Consider applying the same local submit policy to accidental rapid Enter or
|
||
button activation, without routing chat through the coalescing command path.
|
||
|
||
### Phase 2 tests
|
||
|
||
- [x] Valid roster author is admitted; never-announced, post-leave, forged, and
|
||
stale authors are rejected.
|
||
- [x] A peer sending `name = "Victim"` renders under its own roster name.
|
||
- [x] A name update affects future messages without rewriting history.
|
||
- [x] Reconnect grace continues accepting the known author; terminal eviction does
|
||
not.
|
||
- [x] The same signed chat is displayed once; distinct chats created in the same
|
||
millisecond are both admitted.
|
||
- [x] Replay-cache TTL/cap pruning cannot grow without bound.
|
||
- [x] Per-author burst/refill and room-wide burst/refill boundaries.
|
||
- [x] Excess chat cannot prevent a subsequent `Leave` or `Announce` from reaching
|
||
the event loop in a deterministic channel-pressure test.
|
||
|
||
## Phase 3 — Attachment transfer and memory hardening
|
||
|
||
**Target:** neither peers nor long local sessions can turn chat attachments into
|
||
unbounded memory, bandwidth, decoder, or task pressure.
|
||
|
||
### 3A. Cache and image cost
|
||
|
||
- [x] Extend `AttachmentCache` with encoded-byte and decoded-preview-byte counters.
|
||
Preserve the count cap, but evict oldest entries until all three budgets fit.
|
||
- [x] Give every entry an explicit weight. Replacement must subtract the old
|
||
weight before checking/inserting the new one.
|
||
- [x] Decide behavior for a single entry larger than the cache budget: service an
|
||
immediate pending Save/Play request without retaining it, then expose it as
|
||
evicted/unavailable rather than exceeding the budget.
|
||
- [x] Add a total-pixel limit to `validate_image_bytes` in addition to the existing
|
||
width/height limit.
|
||
- [x] Build a downscaled inline preview handle with a maximum 1,600 px side. Keep
|
||
original bytes only for Save; do not hand a full-resolution 4K image to the
|
||
renderer merely to display it at chat width.
|
||
- [x] Count estimated RGBA preview cost (`width * height * 4`) against the decoded
|
||
budget even if iced internally copies or uploads it.
|
||
- [x] Strip the same bidi/zero-width spoofing characters used for display labels
|
||
from attachment filenames, while preserving ordinary Unicode filenames.
|
||
|
||
### 3B. Automatic download policy and state
|
||
|
||
- [x] Auto-fetch only roster-authored images whose declared size is at or below
|
||
`MAX_AUTO_IMAGE_BYTES`; keep the existing `(author,id)` dedup and four-permit
|
||
concurrency bound.
|
||
- [x] Add per-author and session byte/request budgets for automatic fetches so a
|
||
peer cannot drain bandwidth sequentially after each permit is released.
|
||
- [x] Represent `NotFetched`, `Loading`, `Ready`, `Failed`, and `Evicted` distinctly
|
||
enough for the UI to avoid an indefinite “loading…” label when auto-fetch was
|
||
skipped or the cache evicted an item.
|
||
- [x] Render a Load image button for large/skipped images. A manual click may use
|
||
the 25 MiB file cap but still observes cache/decoder budgets.
|
||
- [x] Ensure a repeated click cannot create duplicate unguarded fetch tasks.
|
||
- [x] Keep non-image attachments manual-only.
|
||
|
||
### 3C. Exact transfers, local reads, and served files
|
||
|
||
- [x] In `IrohTransport::fetch_blob`, require `bytes.len() as u64 == declared_size`.
|
||
Reject empty, short, and overlong transfers with a concise local error.
|
||
- [x] Replace the file picker's unbounded `FileHandle::read()` with a helper that
|
||
reads at most `MAX_ATTACHMENT_BYTES + 1`. Check metadata first where available,
|
||
but retain the bounded read because metadata can race or be unavailable through
|
||
a portal.
|
||
- [x] Avoid duplicating a full attachment across UI, command queue, and serve store.
|
||
Prefer `Arc<Vec<u8>>`/`Arc<[u8]>` through `AttachmentState`, `CoreCommand`, and
|
||
`serve_attachment`, subject to iced handle API constraints.
|
||
- [x] Replace the unbounded session `served_files` map with a count- and byte-
|
||
budgeted FIFO store. Evicted ids should produce the existing “sender no longer
|
||
has the file” response rather than stale or aliased data.
|
||
- [x] Keep attachment ids keyed by author on receipt and preserve all existing
|
||
request-length, timeout, filename, and decoder checks.
|
||
|
||
### Phase 3 tests
|
||
|
||
- [x] Byte-budget eviction, count eviction, replacement accounting, clear/reset,
|
||
and an individually overweight entry.
|
||
- [x] Decoded-preview budget and downscale dimensions for wide, tall, square, and
|
||
boundary images.
|
||
- [x] Image with valid per-side dimensions but excessive total pixels is rejected.
|
||
- [x] A declared 4 MiB image auto-fetches; the first byte over the limit requires a
|
||
click.
|
||
- [x] Per-author/session auto-fetch budgets recover according to their policy and
|
||
never exceed task concurrency.
|
||
- [x] Short, exact, and overlong file responses.
|
||
- [x] Local file reader stops at cap + 1 instead of allocating the full source.
|
||
- [x] Served-file FIFO/byte eviction and replacement accounting.
|
||
- [x] Same attachment id from two authors remains isolated throughout fetch, cache,
|
||
save, and display.
|
||
|
||
## Phase 4 — URL and rendering resilience
|
||
|
||
**Target:** keep clickable links without making malformed/deceptive input or many
|
||
small spans an unnecessary UI/launcher surface.
|
||
|
||
- [x] Make `url` a direct dependency (already present transitively) and validate
|
||
link candidates with `url::Url`.
|
||
- [x] A clickable URL must have an `http` or `https` scheme and a valid host.
|
||
- [x] Treat URLs containing username/password syntax as plain text, or require an
|
||
explicit confirmation that shows the parsed destination host. Prefer plain text
|
||
for the first implementation.
|
||
- [x] Preserve the existing defense-in-depth validation in `AppMessage::OpenUrl`;
|
||
replace prefix checks with the shared parsed-URL policy.
|
||
- [x] Cap clickable candidates at eight per message. Remaining content stays
|
||
selectable plain text and must still round-trip exactly.
|
||
- [x] Refactor linkification to return borrowed ranges/offsets or cache link ranges
|
||
in `ChatEntry`, avoiding allocation and rescanning on every redraw.
|
||
- [x] Bound retained history by total sanitized text bytes as well as 300 entries.
|
||
Eviction must keep attachment bookkeeping coherent and should not invalidate an
|
||
open Save/Play operation.
|
||
- [x] Do not add metadata fetching, remote images, Markdown, or link previews.
|
||
- [x] (Folded in from S14, per the security handoff) Strip bidi
|
||
overrides/isolates from the chat BODY in `sanitize_chat`, keeping the other
|
||
expressive format characters (ZWJ/ZWNJ/LRM/RLM).
|
||
|
||
### Phase 4 tests
|
||
|
||
- [x] Valid HTTP/HTTPS, malformed host, empty host, mixed case, Unicode path/query,
|
||
punctuation, credentials/userinfo, and non-web schemes.
|
||
- [x] Eight-link boundary and many-link adversarial input.
|
||
- [x] Segment/range reconstruction exactly reproduces the sanitized message.
|
||
- [x] Entry-count and total-text-budget history eviction.
|
||
- [x] Opener policy cannot launch a non-web scheme even if called directly.
|
||
|
||
## Phase 5 — Honest local send status
|
||
|
||
**Target:** never present a locally echoed message as successfully broadcast when
|
||
the core rejected it or gossip broadcast failed.
|
||
|
||
- [x] Add a local-only message id and `Pending`/`Broadcast`/`Failed` state to local
|
||
chat entries. Do not put this id or state on the wire. (`ChatEntry.local_send:
|
||
Option<LocalSend>`; `SendStatus` also has `Queued` for the paced-but-not-yet-sent
|
||
state — see the pacing decision-log entry.)
|
||
- [x] Carry the local id through `CoreCommand::SendChat`/`SendChatFile` and return a
|
||
`UiEvent` result after the local gossip broadcast call succeeds or fails.
|
||
(`SendChat`/`SendChatFile` gained `local_id`; new `UiEvent::ChatSendResult { local_id,
|
||
error }`.)
|
||
- [x] If the core is not in an active session, return failure instead of silently
|
||
doing nothing. (`send_chat` now `Err`s on missing sender/topic and on encode
|
||
failure; the core arm maps no-session to a `ChatSendResult` error.)
|
||
- [x] Show failure compactly with a retry action. A successful local broadcast must
|
||
not be labeled “delivered” or “read”; PeerSpeak has no peer acknowledgements.
|
||
(Failed → red "⚠ Not sent — {reason} [Retry]" line; Broadcast/Pending render
|
||
nothing — silence is the honest success state.)
|
||
- [x] Retry creates one new signed broadcast while retaining replay correctness and
|
||
attachment serving state. (`RetryChatSend(id)` re-dispatches the retained
|
||
`PendingSend`; re-serving the same attachment id REPLACES the `ServeStore`
|
||
entry, never double-counts — see `serve_store_replacement_accounting_and_remove_clear`.)
|
||
|
||
### Phase 5 tests
|
||
|
||
- [x] Local echo starts pending, becomes broadcast on success, and becomes failed
|
||
on no-session/channel/gossip error. (`send_status_pending_then_broadcast_on_success`,
|
||
`send_status_failed_keeps_payload_for_retry`.)
|
||
- [x] Results update only the matching local entry, including after history
|
||
eviction or room reset. (`send_result_updates_only_the_matching_entry`,
|
||
`send_result_after_eviction_drops_orphan_payload`, `send_result_after_room_reset_is_a_noop`.)
|
||
- [x] Retry does not duplicate served bytes or mutate an unrelated entry.
|
||
(`retry_redispatches_only_the_targeted_send`; served-byte dedup =
|
||
`serve_store_replacement_accounting_and_remove_clear` in `files.rs`.)
|
||
|
||
## Compatibility and versioning
|
||
|
||
- The planned implementation changes validation, local data structures, and
|
||
internal `CoreCommand`/`UiEvent` shapes only. Keep the serialized
|
||
`GossipMessage::Chat` and file request/response formats unchanged.
|
||
- Therefore do **not** bump `GOSSIP_PROTO`, `FILES_PROTO`, or the pre-1.0 MINOR
|
||
solely for this plan. The eventual release is a compatible PATCH unless scope
|
||
expands into a wire change.
|
||
- If implementation requires removing/adding serialized fields, changing
|
||
attachment request framing, or introducing acknowledgements on the wire, stop
|
||
and revise this section before coding that part. Follow `VERSIONING.md` and use
|
||
the appropriate protocol plus release MINOR bump.
|
||
|
||
## Verification gates
|
||
|
||
Run after each phase, with focused tests first and the full gates before handoff:
|
||
|
||
```text
|
||
cargo fmt --check
|
||
cargo test --lib
|
||
cargo test --all-targets
|
||
cargo clippy --all-targets -- -D warnings
|
||
```
|
||
|
||
Also retain the existing ignored/loopback coverage where the environment supports
|
||
it; do not make ordinary unit tests depend on external network access.
|
||
|
||
### Two-machine field test
|
||
|
||
- [ ] Ordinary ASCII/Unicode conversation, rapid short burst, long boundary text,
|
||
and oversized paste.
|
||
- [ ] Rename during a room: new lines use the new roster name; old lines retain
|
||
their snapshot.
|
||
- [ ] Disconnect/reconnect grace and post-leave chat admission behavior.
|
||
- [ ] Multiple normal images, one image above the auto threshold, a malformed
|
||
“image”, and a maximum-size manual file.
|
||
- [ ] Download/save after cache eviction; clear failure state and no runaway
|
||
memory across repeated attachments.
|
||
- [ ] Observe process RSS and UI responsiveness during a bounded spam/attachment
|
||
stress run; verify leave/reconnect controls remain responsive.
|
||
- [ ] Linux and Windows URL opening for valid links; malformed/userinfo links remain
|
||
selectable but do not launch.
|
||
- [ ] A message with more than eight URLs renders eight clickable links and the
|
||
rest as selectable plain text, with nothing dropped.
|
||
- [ ] A message attempting bidi-override display spoofing renders in send order
|
||
(the override characters are stripped, emoji/joining-script text intact).
|
||
- [ ] Send a fast burst (>8 messages in a second): all arrive at the peer in
|
||
order, none silently lost; the sender sees "queued…" on the overflow that
|
||
then clears as each goes out.
|
||
- [ ] Send with no active session (or a failing broadcast): the message shows
|
||
"⚠ Not sent" with a Retry, and Retry resends it once when connectivity is back.
|
||
|
||
## Completion criteria
|
||
|
||
The plan is complete when:
|
||
|
||
1. Only active/grace-rostered authenticated authors reach chat UI state.
|
||
2. Chat identity is roster-bound and cannot be overridden by the embedded wire
|
||
name.
|
||
3. Exact replay and sustained spam are bounded before shared event queues.
|
||
4. Live input, inbound/outbound body size, history text, attachment caches,
|
||
automatic transfers, served files, and decoded previews all have tested hard
|
||
bounds.
|
||
5. File transfer length and image decoding/display costs are validated.
|
||
6. Clickable links pass a shared parsed-URL policy and rendering work is bounded.
|
||
7. Local broadcast failure is visible without claiming peer delivery.
|
||
8. Unit/all-target/clippy gates and the two-machine field test pass.
|
||
9. Relevant durable docs (`README.md`, `docs/FEATURES.md`, `CHANGELOG.md`, security
|
||
notes, and comments) describe the final behavior.
|
||
10. This ephemeral plan is deleted after its useful status/history is transferred
|
||
to durable documentation.
|
||
|
||
## Out of scope
|
||
|
||
- Link previews, metadata fetches, or remote thumbnail requests.
|
||
- Persistent/offline chat history or server-side message storage.
|
||
- Markdown, rich embeds, reactions, editing, deletion, threads, or search.
|
||
- Read receipts or peer delivery acknowledgements.
|
||
- Moderation UI, kicking, blocking, or trust-list redesign.
|
||
- Antivirus/malware scanning of user-requested downloaded files.
|
||
- A new application-layer group-encryption protocol or a broader cryptographic
|
||
redesign. If PeerSpeak makes a formal end-to-end-encryption product claim, audit
|
||
and document the exact iroh/gossip/relay threat model as a separate project.
|
||
|
||
## Decision log
|
||
|
||
- **2026-07-15:** Chose hardening over automatic link previews because receiving a
|
||
message should not trigger third-party web requests or weaken PeerSpeak's
|
||
privacy-oriented design.
|
||
- **2026-07-15:** Initial scope keeps all wire formats stable; hardening is local
|
||
admission, validation, resource accounting, and honest UI state.
|
||
- **2026-07-17 (Phase 1):** The 8 KiB byte ceiling deliberately cannot bind on
|
||
*sanitized* output (2,000 scalars × 4 bytes = 8,000 ≤ 8,192), so inside
|
||
`sanitize_chat`/`cap_chat_input` it is defense in depth; its operative role is
|
||
the raw-ingress reject in `admit_chat_text`.
|
||
- **2026-07-17 (Phase 1):** Interim until Phase 2's roster binding: the incoming
|
||
chat `name` now goes through the strict `sanitize_name` label sanitizer at the
|
||
UI edge (was the body sanitizer), so author labels already get bidi/zero-width
|
||
stripping and the 48-char label cap.
|
||
- **2026-07-17 (Phase 1):** `send_chat` at the gossip sign point silently no-ops
|
||
(Ok) on an empty-after-sanitize body with no attachment rather than erroring;
|
||
the UI already prevents this case, and Phase 5's send-status work is where
|
||
send-path feedback gets designed.
|
||
- **2026-07-17 (Phase 2):** Replay dedup is keyed on the payload's own Ed25519
|
||
**signature bytes** instead of a BLAKE3 digest (the plan allowed "an equally
|
||
collision-resistant existing primitive"): ed25519 signing is deterministic
|
||
(RFC 8032), so the 64-byte signature is already a collision-resistant
|
||
fingerprint of the exact signed bytes — same dedup power, zero new direct
|
||
dependencies. Cache entries are stamped with the signed envelope `ts` and
|
||
pruned once it exits the freshness window, because `verify_gossip` already
|
||
rejects such a frame before the cache is consulted.
|
||
- **2026-07-17 (Phase 2):** A room-bucket reject refunds the just-consumed
|
||
author token, so a room-wide squeeze caused by other members does not also
|
||
drain an innocent author's personal budget.
|
||
- **2026-07-17 (Phase 2):** Rate-limited frames are NOT entered into the replay
|
||
cache: only fully admitted chats are. A legitimate message the room was too
|
||
busy for, redelivered later by the swarm, is then displayed once instead of
|
||
being misread as a replay of something never shown.
|
||
- **2026-07-17 (Phase 2):** The "wire name never renders" guarantee is
|
||
structural: the core event task binds the wire field as `name: _` and builds
|
||
`UiEvent::ChatMessage` exclusively from `ChatRoster::name_of`, so there is no
|
||
code path from wire name to UI. The roster map behavior is unit-tested; the
|
||
end-to-end impersonation scenario stays on the (still-open) two-machine
|
||
field-test list.
|
||
- **2026-07-17 (Phase 2):** The channel-pressure requirement is met at the seam
|
||
level: chat admission is bounded (32-burst / 8-per-s room-wide) BEFORE any
|
||
`event_tx.send`, and `Announce`/`Leave` admission is independent of the chat
|
||
gate — verified by unit tests. A full gossip-loop pressure harness was not
|
||
built; the seam bound is what protects the channel.
|
||
- **2026-07-17 (Phase 2):** An empty-after-sanitize roster name falls back to
|
||
the short node id, so a member who announces an all-control-character name
|
||
still gets a stable, non-blank chat label.
|
||
- **2026-07-17 (Phase 2):** The "Consider applying the same local submit policy
|
||
to accidental rapid Enter" item is DEFERRED: the receiving side is the
|
||
security boundary (every peer independently enforces the buckets), and a
|
||
local silent drop would be a UX regression better designed alongside Phase
|
||
5's honest send status.
|
||
- **2026-07-18 (Phase 3):** Constants that deviate from the proposed table, all
|
||
bound-tested: total decoded pixels **14 MP** (not 16 MP) so the bound clears
|
||
12 MP phone photos (4032×3024) yet actually binds inside the 4096²≈16.8 MP
|
||
per-side envelope; cache encoded budget **96 MiB** (not 128) — still several
|
||
full-size files, tighter worst case; serve store **128 MiB + 16 entries**
|
||
(not 256 MiB) — a sender's own session should not pin a quarter GiB.
|
||
- **2026-07-18 (Phase 3):** `validate_image_bytes`/`decode_preview` precheck
|
||
dimensions from the container HEADER (`into_dimensions`) before any pixel
|
||
decode, so an over-limit decode bomb is rejected without paying its decode
|
||
cost; the decode-time `image::Limits` remain as defense in depth, and the
|
||
decoded dimensions must equal the prechecked header dimensions.
|
||
- **2026-07-18 (Phase 3):** Budget-pressure evictions leave NO cache entry
|
||
(absence = NotFetched → the same Load/Download affordance), while the
|
||
explicit `Evicted` state marks only an *individually over-budget* fetch whose
|
||
bytes were used once (pending Save/Play serviced from hand) and dropped. Both
|
||
render load-on-demand; only the bookkeeping differs.
|
||
- **2026-07-18 (Phase 3):** The core still runs `validate_image_bytes` before
|
||
emitting `AttachmentReady`, and the UI decodes once more to build the ≤1600px
|
||
preview. Two bounded decodes per image were accepted over shipping decoded
|
||
RGBA across the channel (which would defeat the encoded-only Arc sharing).
|
||
- **2026-07-18 (Phase 3):** The image lightbox now enlarges the ≤1600px preview
|
||
handle, not the original bitmap — originals are retained encoded-only for
|
||
Save. At the lightbox's window-sized draw area the visual difference is nil
|
||
for the chat use case; full fidelity remains one Save away.
|
||
- **2026-07-18 (Phase 3):** `AutoFetchBudget` checks all four buckets
|
||
(author/session × requests/bytes) and only then consumes atomically, so a
|
||
rejection burns nothing (no refund path like Phase 2's room bucket needed).
|
||
Tokens ARE consumed if the four-permit semaphore then rejects the spawn —
|
||
that only happens mid-flood, when charging the author is the intent.
|
||
- **2026-07-18 (Phase 3):** The auto-fetch budget's author map prunes
|
||
least-recently-active past 64 entries instead of wiring roster eviction into
|
||
the event task: authors are roster-gated upstream (≤32 live members), so
|
||
strangers cannot churn the map, and a pruned author returning with full
|
||
buckets is within policy.
|
||
- **2026-07-18 (Phase 3):** Music-track serving shares the bounded serve store
|
||
with chat attachments. A user who sends enough large attachments during a
|
||
broadcast can evict their own current track; listeners then get the standard
|
||
"sender no longer has the file" failure. Accepted: budget honesty over a
|
||
second store, and the store comfortably fits current+next track plus a
|
||
normal chat working set.
|
||
- **2026-07-18 (Phase 3):** The clip player's command channel still takes one
|
||
owned byte copy at the moment of a Play click (small, human-initiated). The
|
||
Arc de-duplication targeted the send path (UI cache / command queue / serve
|
||
store), which now shares a single allocation.
|
||
- **2026-07-18 (Phase 3):** Overlong transfers are rejected by the transport
|
||
read itself (`read_to_end(size)` errors past the bound) rather than an
|
||
explicit length compare; short transfers get the explicit
|
||
`len == declared_size` check. Music fetches ride `fetch_blob`, so they
|
||
inherit exactness for free.
|
||
- **2026-07-18 (Phase 4):** The S14 chat-body half (bidi strip) landed here per
|
||
the security handoff: `sanitize_chat` strips ONLY bidi overrides/isolates
|
||
(U+202A–202E, U+2066–2069) — the characters that can visually reorder a
|
||
rendered line — while ZWJ/ZWNJ (emoji sequences, joining scripts) and the
|
||
LRM/RLM direction *marks* (which cannot reorder) are kept. Labels/filenames
|
||
keep the stricter full-format-strip.
|
||
- **2026-07-18 (Phase 4):** A link's href is the exact displayed slice of the
|
||
message — validation is parse-only, no normalization on open — so what the
|
||
user sees IS the argv the opener receives. Consequence: WHATWG slash
|
||
collapsing means `http:///path` parses to host `path` (as in browsers) and is
|
||
accepted; the empty-host rejects are `http://` and friends that fail parsing.
|
||
- **2026-07-18 (Phase 4):** URLs with userinfo syntax went the plan-preferred
|
||
plain-text route (no confirmation dialog). A candidate that fails the policy
|
||
leaves its WHOLE whitespace-delimited run as plain text without re-scanning
|
||
the interior — `http://a@http://b.com` yields zero links, by design.
|
||
- **2026-07-18 (Phase 4):** Scheme detection became ASCII-case-insensitive
|
||
(`Http://…` from sentence auto-capitalization now linkifies); the policy
|
||
check is unaffected since `url` normalizes scheme/host case during parsing.
|
||
- **2026-07-18 (Phase 4):** Cached ranges in `ChatEntry.links`, filled inside
|
||
`push_chat` (the single history choke point), were chosen over
|
||
borrowed-return-per-redraw: redraws now slice cached char-boundary ranges,
|
||
and only link spans allocate (their href String).
|
||
- **2026-07-18 (Phase 4):** History byte-budget eviction (512 KiB, alongside
|
||
the 300-entry cap) deliberately does NOT touch the attachment byte cache:
|
||
that cache is bounded by its own Phase 3 budgets, and leaving it alone means
|
||
an open Save/Play on an evicted line keeps its bytes-in-hand (the save
|
||
dialog falls back to the generic "download" name). The just-pushed entry is
|
||
never evicted; a single message's 8 KiB ceiling cannot exceed the budget.
|
||
- **2026-07-18 (Phase 5):** Sender-side PACING was added to Phase 5's scope
|
||
(originally receiver-status only). The Phase 2 decision log deferred the
|
||
"apply the same local submit policy to accidental rapid Enter" item to pair
|
||
with Phase 5, and honest status alone would still let a fast burst broadcast
|
||
successfully yet be silently dropped by every receiver's per-author bucket
|
||
(8 burst, then 1/s) with no sender feedback. The user chose "queue and
|
||
trickle" over "throttle input": sends past the burst queue locally as
|
||
`SendStatus::Queued` ("queued…") and release at the receivers' sustained
|
||
rate, so nothing is lost and typing is never blocked.
|
||
- **2026-07-18 (Phase 5):** The pacer (`src/app/sendqueue.rs`) reuses the
|
||
gossip gate's OWN `TokenBucket` + `CHAT_AUTHOR_BURST`/`CHAT_AUTHOR_REFILL_PER_MS`
|
||
(made `pub(crate)`), so the two sides of the rate policy are one definition
|
||
and cannot drift. It mirrors only the PER-AUTHOR budget, not the room-wide
|
||
one — we cannot know other members' send rates, and the per-author bucket is
|
||
the one guaranteed to apply to us at every receiver.
|
||
- **2026-07-18 (Phase 5):** Send status renders as a line UNDER the message
|
||
(user pick over an inline suffix glyph); `Broadcast` and the transient
|
||
`Pending` show nothing because PeerSpeak has no delivery/read receipts, so an
|
||
unadorned message IS the honest "handed to the swarm" state. Only `Queued`
|
||
and `Failed` (with Retry) are surfaced.
|
||
- **2026-07-18 (Phase 5):** The pacer and the monotonic send-id counter
|
||
deliberately SURVIVE a room reset while the queue and retry payloads are
|
||
cleared: receivers' per-author buckets persist across our rejoin (so the
|
||
pacer should not refill to full), and never-reused ids keep a late
|
||
`ChatSendResult` from a pre-reset send from aliasing a new entry — verified by
|
||
`send_result_after_room_reset_is_a_noop`.
|
||
- **2026-07-18 (Phase 5):** The pacer clock is `Instant`-based
|
||
(`AppState.send_clock`), not wall-clock, so a system time jump can neither
|
||
rewind nor fast-forward the send budget.
|
||
|
||
## Completion
|
||
|
||
All five phases are implemented and every gate is green. Per the scope-contract
|
||
note at the top, this file should be DELETED once the owed two-machine field
|
||
test (the checklist below) has been run — that deletion is a separate,
|
||
user-gated step, not part of the Phase 5 commit. Until then the plan stays as
|
||
the record of what shipped and what remains to verify on real hardware.
|