Skip to content

fix: authorize inbound envelopes, repair messenger lifecycle, wire read receipts - #5

Merged
ffaerber merged 4 commits into
mainfrom
claude/bug-hunt-rwj46j
Aug 7, 2026
Merged

fix: authorize inbound envelopes, repair messenger lifecycle, wire read receipts#5
ffaerber merged 4 commits into
mainfrom
claude/bug-hunt-rwj46j

Conversation

@ffaerber

@ffaerber ffaerber commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Inbound envelopes were authenticated (signature recovers to from) but never
authorized — nothing checked that the sender was entitled to the effect they
were asking for. Plus a set of lifecycle and correctness defects found in the
same pass.

Security

  • envelope: bind msgId to keccak(from, nonce, ts) at verify time. It was
    attacker-chosen, and it keys the dedup set, the outbox, and ack/read
    correlation, so a sender could collide with someone else's message.
  • reliability: only the addressee may ack/read a message. A forged receipt
    also cancelled the retry timer, suppressing retransmission of a message the
    real recipient never received.
  • calls: scope call-answer/ice/call-hangup to the peer on the other end of the
    call, and require a matching callId on hangup — a missing one was treated as
    a wildcard, letting any peer tear down the active call.
  • groups: gate inbound group/state on the group's current admin (not the
    payload's own admin field, which let a member promote themselves), require
    self to be a member, and validate payload shape. Group messages are now
    dropped unless the group is known and the sender is in it; messages that
    race ahead of their group/state are held in a bounded buffer and replayed,
    since the auto-ack means the sender will not retry.

Correctness

  • messenger: clear the cached feed identity when switching to a wallet that
    has none, instead of silently reusing the previous wallet's feed key.
  • messenger: drop handles on teardown so the UI cannot write to closed
    IndexedDB databases while the next messenger is being built.
  • messenger: a group send that reaches nobody now persists a uniquely-keyed
    row marked failed and throws, rather than writing a zero-hash row marked
    sent that the next such failure would overwrite.
  • media: stop caching rejected resolves, which made one transient bee error
    permanent for that reference (and surfaced as an unhandled rejection).
  • transport: refresh msgIds on re-mark so dedup eviction is LRU, not FIFO.
  • calls: report false from toggleAudio/toggleVideo when no track of that kind
    exists.
  • ContactRegistry: clamp getUsers against the remaining span so a large limit
    cannot revert with an arithmetic panic.

Features completed

  • Read receipts and unread counts were dead code: markRead was never called
    and unread was hardcoded to 0. Viewing a conversation now clears unread
    state and emits one receipt per message, and the sidebar shows unread
    badges for 1:1 and group chats.

Tests: 38 new cases (162 total). 33 of them fail against the pre-fix source;
the contract change was verified by executing both versions in an EVM.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01KnBiaAgJUWmo2WuAbKLYUi

claude added 4 commits August 5, 2026 12:49
…ad receipts

Inbound envelopes were authenticated (signature recovers to `from`) but never
authorized — nothing checked that the sender was entitled to the effect they
were asking for. Plus a set of lifecycle and correctness defects found in the
same pass.

Security
- envelope: bind `msgId` to keccak(from, nonce, ts) at verify time. It was
  attacker-chosen, and it keys the dedup set, the outbox, and ack/read
  correlation, so a sender could collide with someone else's message.
- reliability: only the addressee may ack/read a message. A forged receipt
  also cancelled the retry timer, suppressing retransmission of a message the
  real recipient never received.
- calls: scope call-answer/ice/call-hangup to the peer on the other end of the
  call, and require a matching callId on hangup — a missing one was treated as
  a wildcard, letting any peer tear down the active call.
- groups: gate inbound group/state on the group's current admin (not the
  payload's own admin field, which let a member promote themselves), require
  self to be a member, and validate payload shape. Group messages are now
  dropped unless the group is known and the sender is in it; messages that
  race ahead of their group/state are held in a bounded buffer and replayed,
  since the auto-ack means the sender will not retry.

Correctness
- messenger: clear the cached feed identity when switching to a wallet that
  has none, instead of silently reusing the previous wallet's feed key.
- messenger: drop handles on teardown so the UI cannot write to closed
  IndexedDB databases while the next messenger is being built.
- messenger: a group send that reaches nobody now persists a uniquely-keyed
  row marked failed and throws, rather than writing a zero-hash row marked
  sent that the next such failure would overwrite.
- media: stop caching rejected resolves, which made one transient bee error
  permanent for that reference (and surfaced as an unhandled rejection).
- transport: refresh msgIds on re-mark so dedup eviction is LRU, not FIFO.
- calls: report false from toggleAudio/toggleVideo when no track of that kind
  exists.
- ContactRegistry: clamp getUsers against the remaining span so a large limit
  cannot revert with an arithmetic panic.

Features completed
- Read receipts and unread counts were dead code: markRead was never called
  and unread was hardcoded to 0. Viewing a conversation now clears unread
  state and emits one receipt per message, and the sidebar shows unread
  badges for 1:1 and group chats.

Tests: 38 new cases (162 total). 33 of them fail against the pre-fix source;
the contract change was verified by executing both versions in an EVM.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KnBiaAgJUWmo2WuAbKLYUi
The repository had no workflows, so nothing verified either test suite. The
contract job is the one that matters most right now: the Foundry tests have
not been executed in this environment (forge is unavailable here), including
the new getUsers pagination regression test.

No test forks the chain, so `forge test` needs no RPC access.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KnBiaAgJUWmo2WuAbKLYUi
`eth_rpc_url` in [profile.default] makes `forge test` fork from that endpoint
on every run, so the entire contract suite executed against the public Gnosis
node. CI surfaced the consequence immediately: the unit suite passed, then the
fuzz and invariant suites failed with HTTP 429 from `vm.prank: failed to get
account` — 7 failures that have nothing to do with the contract.

The Makefile already treats forking as opt-in (`test-fork` passes --fork-url,
`test-unit` is documented "no fork", every deploy target passes --rpc-url), so
nothing depended on the implicit default. The rpc_endpoints aliases stay.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KnBiaAgJUWmo2WuAbKLYUi
Both v4 releases run on Node 20, which the runners now force onto Node 24 with
a deprecation warning. The v5 releases target Node 24 natively. No input
changes are needed for either action.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KnBiaAgJUWmo2WuAbKLYUi
@ffaerber
ffaerber merged commit de7dd46 into main Aug 7, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants