fix: authorize inbound envelopes, repair messenger lifecycle, wire read receipts - #5
Merged
Conversation
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Inbound envelopes were authenticated (signature recovers to
from) but neverauthorized — 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
msgIdto keccak(from, nonce, ts) at verify time. It wasattacker-chosen, and it keys the dedup set, the outbox, and ack/read
correlation, so a sender could collide with someone else's message.
also cancelled the retry timer, suppressing retransmission of a message the
real recipient never received.
call, and require a matching callId on hangup — a missing one was treated as
a wildcard, letting any peer tear down the active call.
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
has none, instead of silently reusing the previous wallet's feed key.
IndexedDB databases while the next messenger is being built.
row marked failed and throws, rather than writing a zero-hash row marked
sent that the next such failure would overwrite.
permanent for that reference (and surfaced as an unhandled rejection).
exists.
cannot revert with an arithmetic panic.
Features completed
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