From c681b2671b63d2ce88f564112d772f29b885bf55 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 12:49:36 +0000 Subject: [PATCH 1/4] fix: authorize inbound envelopes, repair messenger lifecycle, wire read receipts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01KnBiaAgJUWmo2WuAbKLYUi --- frontend/src/components/Conversation.tsx | 7 + frontend/src/components/GroupConversation.tsx | 7 + frontend/src/components/Sidebar.tsx | 23 +- frontend/src/contexts/MessengerContext.tsx | 158 ++++++++-- frontend/src/hooks/useGroupConversations.ts | 24 ++ frontend/src/lib/calls.ts | 29 +- frontend/src/lib/envelope.ts | 13 +- frontend/src/lib/groups-store.ts | 49 +++ frontend/src/lib/idb-messages.ts | 58 +++- frontend/src/lib/media.ts | 6 + frontend/src/lib/messages-store.ts | 45 ++- frontend/src/lib/reliability.ts | 11 +- frontend/src/lib/transport.ts | 9 +- frontend/test/unit/authorization.test.ts | 289 ++++++++++++++++++ frontend/test/unit/media-resolver.test.ts | 63 ++++ frontend/test/unit/unread.test.ts | 123 ++++++++ src/ContactRegistry.sol | 5 +- test/ContactRegistry.t.sol | 22 ++ 18 files changed, 894 insertions(+), 47 deletions(-) create mode 100644 frontend/src/hooks/useGroupConversations.ts create mode 100644 frontend/test/unit/authorization.test.ts create mode 100644 frontend/test/unit/media-resolver.test.ts create mode 100644 frontend/test/unit/unread.test.ts diff --git a/frontend/src/components/Conversation.tsx b/frontend/src/components/Conversation.tsx index d73c86c..86d3fe1 100644 --- a/frontend/src/components/Conversation.tsx +++ b/frontend/src/components/Conversation.tsx @@ -116,6 +116,13 @@ export default function Conversation() { scrollRef.current?.scrollTo({ top: scrollRef.current.scrollHeight }) }, [messages.length]) + // Viewing the conversation is what marks it read. Idempotent: the store only + // returns rows that actually flipped, so re-renders don't re-send receipts. + useEffect(() => { + if (!handles || !peer) return + handles.markConversationRead(peer).catch(() => {}) + }, [handles, peer, messages.length]) + const onPaste = async (e: React.ClipboardEvent) => { const items = Array.from(e.clipboardData.files) if (items.length === 0) return diff --git a/frontend/src/components/GroupConversation.tsx b/frontend/src/components/GroupConversation.tsx index aeed831..56cd8e8 100644 --- a/frontend/src/components/GroupConversation.tsx +++ b/frontend/src/components/GroupConversation.tsx @@ -89,6 +89,13 @@ export default function GroupConversation() { scrollRef.current?.scrollTo({ top: scrollRef.current.scrollHeight }) }, [messages.length]) + // Viewing the group marks it read; the store only returns rows that actually + // flipped, so this stays idempotent across re-renders. + useEffect(() => { + if (!handles || !groupId) return + handles.markGroupRead(groupId).catch(() => {}) + }, [handles, groupId, messages.length]) + const onPaste = async (e: React.ClipboardEvent) => { const items = Array.from(e.clipboardData.files) if (items.length === 0) return diff --git a/frontend/src/components/Sidebar.tsx b/frontend/src/components/Sidebar.tsx index 46b6993..43c5a37 100644 --- a/frontend/src/components/Sidebar.tsx +++ b/frontend/src/components/Sidebar.tsx @@ -2,11 +2,22 @@ import { useState } from 'react' import { Link, NavLink, useLocation } from 'react-router' import { useConversations } from '../hooks/useConversations' import { useGroups } from '../hooks/useGroups' +import { useGroupConversations } from '../hooks/useGroupConversations' import { previewOf } from '../lib/messages-store' import { useMessenger } from '../contexts/MessengerContext' import CreateGroupModal from './CreateGroupModal' import EnsName from './EnsName' +/** Unread pill shown on chat-list rows. */ +function UnreadBadge({ count }: { count: number }) { + if (count <= 0) return null + return ( + + {count > 99 ? '99+' : count} + + ) +} + function formatRelative(ts: number): string { const diff = Date.now() - ts const m = Math.floor(diff / 60_000) @@ -20,6 +31,8 @@ function formatRelative(ts: number): string { export default function Sidebar() { const conversations = useConversations() const groups = useGroups() + const groupSummaries = useGroupConversations() + const unreadByGroup = new Map(groupSummaries.map(g => [g.groupId.toLowerCase(), g.unread])) const location = useLocation() const { ready } = useMessenger() const [showCreateGroup, setShowCreateGroup] = useState(false) @@ -70,6 +83,7 @@ export default function Sidebar() { {g.members.length} member{g.members.length === 1 ? '' : 's'} + ) @@ -105,9 +119,12 @@ export default function Sidebar() { {formatRelative(c.lastMessage.ts)} -
- {c.lastMessage.direction === 'out' ? 'You: ' : ''} - {previewOf(c.lastMessage)} +
+
+ {c.lastMessage.direction === 'out' ? 'You: ' : ''} + {previewOf(c.lastMessage)} +
+
diff --git a/frontend/src/contexts/MessengerContext.tsx b/frontend/src/contexts/MessengerContext.tsx index c2e5e6b..f772f0e 100644 --- a/frontend/src/contexts/MessengerContext.tsx +++ b/frontend/src/contexts/MessengerContext.tsx @@ -7,7 +7,13 @@ import { Reliability } from '../lib/reliability' import { Feeds } from '../lib/feeds' import { IndexedDBOutbox } from '../lib/idb-outbox' import { IndexedDBMessages } from '../lib/idb-messages' -import { IndexedDBGroups, makeGroupId, randomGroupNonce } from '../lib/groups-store' +import { + IndexedDBGroups, + isGroupMember, + makeGroupId, + mayApplyGroupState, + randomGroupNonce, +} from '../lib/groups-store' import { Blocklist } from '../lib/blocklist' import { deriveFeedKey } from '../lib/feed-key' import { uploadMedia, MediaResolver, classifyMime } from '../lib/media' @@ -35,6 +41,10 @@ interface MessengerHandles { sendToGroup: (groupId: Hex, text: string) => Promise /** Send a file to every member of a group (excluding self). */ sendFileToGroup: (groupId: Hex, file: File) => Promise + /** Clear unread state for a 1:1 chat and emit a read receipt per message. */ + markConversationRead: (peer: Hex) => Promise + /** Same, for a group chat. */ + markGroupRead: (groupId: Hex) => Promise } interface MessengerContextValue { @@ -51,6 +61,19 @@ const Ctx = createContext(null) const FEED_CACHE_KEY = (wallet: string) => `swarmchat:feed:${wallet.toLowerCase()}` +/** + * Local-only id for a row that never made it onto the wire. Must be unique: + * msgId is the IndexedDB key, so a shared placeholder would make each new + * failed message silently overwrite the last one. + */ +function localMsgId(): Hex { + const bytes = new Uint8Array(32) + crypto.getRandomValues(bytes) + let hex = '0x' + for (const b of bytes) hex += b.toString(16).padStart(2, '0') + return hex as Hex +} + /** Coerce inbound envelope payload to a ChatMessage row. Tolerates legacy { text } shape. */ function payloadToChatMessage(env: Envelope, peer: Hex, direction: 'in' | 'out'): ChatMessage | null { const p = env.payload as MsgPayload | { text?: string } | undefined @@ -61,6 +84,7 @@ function payloadToChatMessage(env: Envelope, peer: Hex, direction: 'in' | 'out') direction, ts: env.ts, ...(env.groupId ? { groupId: env.groupId } : {}), + ...(direction === 'in' ? { unread: true } : {}), } as const if ('kind' in p) { @@ -111,12 +135,17 @@ export function MessengerProvider({ children }: { children: ReactNode }) { const [handles, setHandles] = useState(null) const [status, setStatus] = useState('idle') - // Restore cached feed identity for this wallet. + // Restore cached feed identity for this wallet. Every path must assign, so + // that switching to a wallet with no cached key clears the previous wallet's + // identity instead of silently reusing it for the new account. useEffect(() => { if (!address) { setFeedIdentity(null); return } const cached = localStorage.getItem(FEED_CACHE_KEY(address)) - if (cached) { - try { setFeedIdentity(JSON.parse(cached)) } catch { /* corrupt cache */ } + if (!cached) { setFeedIdentity(null); return } + try { + setFeedIdentity(JSON.parse(cached)) + } catch { + setFeedIdentity(null) // corrupt cache — re-derive rather than carry over } }, [address]) @@ -202,6 +231,27 @@ export function MessengerProvider({ children }: { children: ReactNode }) { resolvePeer: w => resolvePeer(w as Hex), }) + /** + * Group messages that arrived before the group/state that authorizes them. + * PSS is unordered, so a group's first message can overtake its invite — + * and since Reliability already auto-acked, the sender will never retry. + * Bounded, because these envelopes are unauthenticated until a group/state + * vouches for them. + */ + const pendingGroupMsgs = new Map() + const PENDING_PER_GROUP = 20 + const PENDING_GROUPS = 50 + + const storeIncomingMsg = async (env: Envelope) => { + const msg = payloadToChatMessage(env, env.from as Hex, 'in') + if (!msg) return + const lower = (env.from.toLowerCase() as Hex) + if (!peerCacheRef.current.has(lower)) { + resolvePeer(env.from as Hex).catch(() => {}) + } + await messages.put(msg) + } + // Inbound: persist + reflect in UI; also dispatch call signaling. reliability.onIncomingMessage(async (env: Envelope) => { if (env.type === 'call-offer' || env.type === 'call-answer' @@ -212,16 +262,51 @@ export function MessengerProvider({ children }: { children: ReactNode }) { if (env.type !== 'msg') return // Group control messages: update the local groups store, don't show in chat. if (isGroupStatePayload(env.payload)) { - try { await groups.put(env.payload.group) } catch (err) { console.error('group/state', err) } + try { + const incoming = env.payload.group + const existing = await groups.get(incoming.id) + const args = { incoming, from: env.from as Hex, self: address as Hex, existing } + if (!mayApplyGroupState(args)) { + console.warn('group/state: rejected unauthorized update from', env.from) + return + } + await groups.put(args.incoming) + // Replay anything that was waiting on this group, re-checking + // membership against the state we just accepted. + const key = args.incoming.id.toLowerCase() + const queued = pendingGroupMsgs.get(key) ?? [] + pendingGroupMsgs.delete(key) + for (const q of queued) { + if (isGroupMember(args.incoming, q.from as Hex)) await storeIncomingMsg(q) + } + } catch (err) { console.error('group/state', err) } return } - const msg = payloadToChatMessage(env, env.from as Hex, 'in') - if (!msg) return - const lower = (env.from.toLowerCase() as Hex) - if (!peerCacheRef.current.has(lower)) { - resolvePeer(env.from as Hex).catch(() => {}) + // A group message is only ours to display if we know the group and the + // sender is in it — otherwise anyone who guesses a groupId could post + // into someone else's conversation. + if (env.groupId) { + const group = await groups.get(env.groupId) + if (!group) { + const key = env.groupId.toLowerCase() + const queue = pendingGroupMsgs.get(key) ?? [] + if (queue.length < PENDING_PER_GROUP) { + // Evict the oldest group (Map keeps insertion order) so a flood of + // invented groupIds can't grow this without bound. + if (!pendingGroupMsgs.has(key) && pendingGroupMsgs.size >= PENDING_GROUPS) { + const oldest = pendingGroupMsgs.keys().next().value + if (oldest !== undefined) pendingGroupMsgs.delete(oldest) + } + pendingGroupMsgs.set(key, [...queue, env]) + } + return + } + if (!isGroupMember(group, env.from as Hex)) { + console.warn('group msg: dropped from non-member', env.from) + return + } } - await messages.put(msg) + await storeIncomingMsg(env) }) // Outbound status changes (sent → delivered → read → failed). @@ -268,17 +353,39 @@ export function MessengerProvider({ children }: { children: ReactNode }) { const results = await fanOutToGroup(group, payload) // Use the first successful envelope's msgId/ts for the local row. const first = results.find(r => r !== null) - const ts = first?.envelope.ts ?? Date.now() - const msgId = (first?.envelope.msgId ?? ('0x' + '0'.repeat(64))) as Hex const persisted: ChatMessage = { ...clientRow, - msgId, - ts, + msgId: first?.envelope.msgId ?? localMsgId(), + ts: first?.envelope.ts ?? Date.now(), peer: address as Hex, // sender = self for outgoing groupId, - status: 'sent', + // Nothing reached anyone: keep the message visible, but don't claim + // it was sent, and give it a unique key so it can't clobber the + // previous failed row. + status: first ? 'sent' : 'failed', } await messages.put(persisted) + if (!first) throw new Error('group send failed: no member could be reached') + } + + /** + * Clear unread state and tell each sender we read their message. Receipts + * are best-effort — the local flag clears regardless, so the badge never + * gets stuck because a peer went offline. + */ + const emitReadReceipts = async (rows: ChatMessage[]) => { + const senders = new Map() + for (const row of rows) { + const k = row.peer.toLowerCase() + senders.set(k, [...(senders.get(k) ?? []), row.msgId]) + } + await Promise.all([...senders.entries()].map(async ([wallet, msgIds]) => { + const peer = await resolvePeer(wallet as Hex) + if (!peer) return + for (const msgId of msgIds) { + await reliability.markRead(msgId, peer).catch(() => {}) + } + })) } const createGroup = async (name: string, members: Hex[]): Promise => { @@ -313,7 +420,7 @@ export function MessengerProvider({ children }: { children: ReactNode }) { calls, send: async (to, text) => { const stub: ChatMessage = { - msgId: ('0x' + '0'.repeat(64)) as Hex, // overwritten with real msgId in sendPayload + msgId: localMsgId(), // overwritten with the real msgId in sendPayload peer: to, direction: 'out', ts: Date.now(), kind: 'text', text, status: 'sent', } await sendPayload(to, { kind: 'text', text }, stub) @@ -324,7 +431,7 @@ export function MessengerProvider({ children }: { children: ReactNode }) { const payload = uploadResultToPayload(upload) const kind = classifyMime(upload.mime) const stub: ChatMessage = { - msgId: ('0x' + '0'.repeat(64)) as Hex, + msgId: localMsgId(), peer: to, direction: 'out', ts: Date.now(), kind, ref: upload.ref as SwarmRef, @@ -341,7 +448,7 @@ export function MessengerProvider({ children }: { children: ReactNode }) { createGroup, sendToGroup: async (groupId, text) => { const stub: ChatMessage = { - msgId: ('0x' + '0'.repeat(64)) as Hex, // overwritten in sendGroupPayload + msgId: localMsgId(), // overwritten in sendGroupPayload peer: address as Hex, direction: 'out', ts: Date.now(), @@ -358,7 +465,7 @@ export function MessengerProvider({ children }: { children: ReactNode }) { const payload = uploadResultToPayload(upload) const kind = classifyMime(upload.mime) const stub: ChatMessage = { - msgId: ('0x' + '0'.repeat(64)) as Hex, + msgId: localMsgId(), peer: address as Hex, direction: 'out', ts: Date.now(), @@ -375,6 +482,12 @@ export function MessengerProvider({ children }: { children: ReactNode }) { } as ChatMessage await sendGroupPayload(groupId, payload, stub) }, + markConversationRead: async peer => { + await emitReadReceipts(await messages.markConversationRead(peer)) + }, + markGroupRead: async groupId => { + await emitReadReceipts(await messages.markGroupRead(groupId)) + }, }) setStatus('ready') }).catch(err => { @@ -384,6 +497,11 @@ export function MessengerProvider({ children }: { children: ReactNode }) { return () => { cancelled = true + // Drop the handles first: everything below closes the databases they + // wrap, so leaving them exposed lets the UI keep writing to a closed DB + // (and keeps `ready` true) until the next build finishes. + setHandles(null) + setStatus('idle') calls.hangup('cleanup').catch(() => {}) reliability.stop() resolveMedia.dispose() diff --git a/frontend/src/hooks/useGroupConversations.ts b/frontend/src/hooks/useGroupConversations.ts new file mode 100644 index 0000000..1ba8f70 --- /dev/null +++ b/frontend/src/hooks/useGroupConversations.ts @@ -0,0 +1,24 @@ +import { useEffect, useState } from 'react' +import { useMessenger } from '../contexts/MessengerContext' +import type { GroupConversationSummary } from '../lib/messages-store' + +/** Per-group message summaries (last message + unread count). */ +export function useGroupConversations() { + const { handles } = useMessenger() + const [list, setList] = useState([]) + + useEffect(() => { + if (!handles) { setList([]); return } + let cancelled = false + const refresh = () => { + handles.messages.listGroupConversations().then(l => { + if (!cancelled) setList(l) + }) + } + refresh() + const off = handles.messages.subscribe(refresh) + return () => { cancelled = true; off() } + }, [handles]) + + return list +} diff --git a/frontend/src/lib/calls.ts b/frontend/src/lib/calls.ts index a07ee4f..8766a55 100644 --- a/frontend/src/lib/calls.ts +++ b/frontend/src/lib/calls.ts @@ -209,10 +209,20 @@ export class CallManager { } } + /** + * Signalling for an in-progress call is only meaningful from the other end + * of that call. Everything after the offer must clear this, or any peer who + * can reach us over PSS could answer, inject ICE, or hang up our call. + */ + private isFromPeer(call: Call, env: Envelope): boolean { + return call.peer.wallet.toLowerCase() === env.from.toLowerCase() + } + private async onAnswer(env: Envelope): Promise { const call = this.current const p = env.payload as AnswerPayload | undefined if (!call || call.direction !== 'outgoing' || call.state !== 'calling') return + if (!this.isFromPeer(call, env)) return if (!p?.callId || p.callId !== call.callId || !p.sdp) return await call.pc.setRemoteDescription({ type: 'answer', sdp: p.sdp }) @@ -225,7 +235,8 @@ export class CallManager { private async onIce(env: Envelope): Promise { const call = this.current const p = env.payload as IcePayload | undefined - if (!call || !p?.callId || p.callId !== call.callId) return + if (!call || !this.isFromPeer(call, env)) return + if (!p?.callId || p.callId !== call.callId) return const list = Array.isArray(p.candidates) ? p.candidates : [] if (!this.remoteDescSet) { this.remoteIceQueue.push(...list) @@ -246,9 +257,11 @@ export class CallManager { private async onHangup(env: Envelope): Promise { const call = this.current const p = env.payload as HangupPayload | undefined - if (!call) return - if (p?.callId && p.callId !== call.callId) return - this.cleanup(call, p?.reason ?? 'remote-hangup') + if (!call || !this.isFromPeer(call, env)) return + // callId is required, not optional: treating a missing one as a wildcard + // let anyone tear down the active call without even knowing its id. + if (!p?.callId || p.callId !== call.callId) return + this.cleanup(call, p.reason ?? 'remote-hangup') } private attachPcHandlers(call: Call): void { @@ -334,8 +347,12 @@ export class CallManager { private toggleTracks(kind: 'audio' | 'video', enabled?: boolean): boolean { const stream = this.current?.localStream if (!stream) return false - let next = enabled ?? !stream.getTracks().some(t => t.kind === kind && t.enabled) - for (const t of stream.getTracks()) if (t.kind === kind) t.enabled = next + const tracks = stream.getTracks().filter(t => t.kind === kind) + // No track of this kind (e.g. video on an audio-only call) — report the + // truth rather than claiming we enabled something that doesn't exist. + if (tracks.length === 0) return false + const next = enabled ?? !tracks.some(t => t.enabled) + for (const t of tracks) t.enabled = next return next } diff --git a/frontend/src/lib/envelope.ts b/frontend/src/lib/envelope.ts index cb1121f..dc0827c 100644 --- a/frontend/src/lib/envelope.ts +++ b/frontend/src/lib/envelope.ts @@ -90,10 +90,21 @@ export async function signEnvelope(args: BuildEnvelopeArgs, signMessage: SignMes return { ...unsigned, sig } } -/** True iff the signature recovers to `env.from`. */ +/** + * True iff the signature recovers to `env.from` *and* `msgId` is the honest + * digest of (from, nonce, ts). + * + * The msgId check matters as much as the signature: msgId is the key every + * downstream layer uses — the dedup set, the outbox, ack/read correlation. A + * sender free to pick an arbitrary msgId for their own well-signed envelope + * could collide with someone else's message and poison those tables. + */ export async function verifyEnvelope(env: Envelope): Promise { const { sig, ...unsigned } = env try { + if (env.msgId?.toLowerCase() !== makeMsgId(env.from, env.nonce, env.ts).toLowerCase()) { + return false + } const recovered = await recoverMessageAddress({ message: canonicalize(unsigned), signature: sig, diff --git a/frontend/src/lib/groups-store.ts b/frontend/src/lib/groups-store.ts index 6839600..ad2210a 100644 --- a/frontend/src/lib/groups-store.ts +++ b/frontend/src/lib/groups-store.ts @@ -13,6 +13,55 @@ export function randomGroupNonce(): Hex { return toHex(out) } +/** Case-insensitive membership test. */ +export function isGroupMember(group: Group, wallet: Hex): boolean { + const w = wallet.toLowerCase() + return group.members.some(m => m.toLowerCase() === w) +} + +function isHexAddress(v: unknown): v is Hex { + return typeof v === 'string' && /^0x[0-9a-fA-F]{40}$/.test(v) +} + +/** Structural check on a `group/state` payload arriving off the wire. */ +export function isWellFormedGroup(value: unknown): value is Group { + if (!value || typeof value !== 'object') return false + const g = value as Partial + return typeof g.id === 'string' && /^0x[0-9a-fA-F]{64}$/.test(g.id) + && typeof g.name === 'string' + && Array.isArray(g.members) && g.members.length > 0 && g.members.every(isHexAddress) + && isHexAddress(g.admin) + && typeof g.createdAt === 'number' && Number.isFinite(g.createdAt) + && typeof g.version === 'number' && Number.isFinite(g.version) +} + +/** + * Decide whether an inbound `group/state` may be applied. + * + * Group membership is off-chain, so the only thing anchoring it is who sent + * the update. The rules: + * - the payload must be structurally sound; + * - we must be a member of the resulting group (otherwise it's an unsolicited + * group someone is pushing into our sidebar); + * - for a group we already know, only its *current* admin may update it — + * checking against the incoming payload's own `admin` field would let a + * member hand themselves the role; + * - for a group we've never seen, the sender must be the admin it declares. + */ +export function mayApplyGroupState(args: { + incoming: unknown + from: Hex + self: Hex + existing?: Group | null +}): args is { incoming: Group; from: Hex; self: Hex; existing?: Group | null } { + const { incoming, from, self, existing } = args + if (!isWellFormedGroup(incoming)) return false + if (existing && existing.id.toLowerCase() !== incoming.id.toLowerCase()) return false + if (!isGroupMember(incoming, self)) return false + const authority = existing ? existing.admin : incoming.admin + return authority.toLowerCase() === from.toLowerCase() +} + export interface GroupsStore { put(group: Group): Promise get(id: Hex): Promise diff --git a/frontend/src/lib/idb-messages.ts b/frontend/src/lib/idb-messages.ts index 15e08bf..395cb65 100644 --- a/frontend/src/lib/idb-messages.ts +++ b/frontend/src/lib/idb-messages.ts @@ -1,10 +1,11 @@ import type { Hex } from './types' -import type { - ChatMessage, - ConversationSummary, - GroupConversationSummary, - MessageStatus, - MessagesStore, +import { + unreadCount, + type ChatMessage, + type ConversationSummary, + type GroupConversationSummary, + type MessageStatus, + type MessagesStore, } from './messages-store' export interface IndexedDBMessagesOptions { @@ -146,7 +147,7 @@ export class IndexedDBMessages implements MessagesStore { const out: ConversationSummary[] = [] for (const [, list] of byPeer) { list.sort((a, b) => a.ts - b.ts) - out.push({ peer: list[0].peer, lastMessage: list[list.length - 1], unread: 0 }) + out.push({ peer: list[0].peer, lastMessage: list[list.length - 1], unread: unreadCount(list) }) } out.sort((a, b) => b.lastMessage.ts - a.lastMessage.ts) return out @@ -199,12 +200,53 @@ export class IndexedDBMessages implements MessagesStore { const out: GroupConversationSummary[] = [] for (const [, list] of byGroup) { list.sort((a, b) => a.ts - b.ts) - out.push({ groupId: list[0].groupId as Hex, lastMessage: list[list.length - 1], unread: 0 }) + out.push({ + groupId: list[0].groupId as Hex, + lastMessage: list[list.length - 1], + unread: unreadCount(list), + }) } out.sort((a, b) => b.lastMessage.ts - a.lastMessage.ts) return out } + async markConversationRead(peer: Hex): Promise { + return this.clearUnread('peerKey', peer.toLowerCase(), m => !m.groupId) + } + + async markGroupRead(groupId: Hex): Promise { + return this.clearUnread('groupKey', groupId.toLowerCase(), () => true) + } + + private async clearUnread( + indexName: 'peerKey' | 'groupKey', + key: string, + extra: (m: ChatMessage) => boolean, + ): Promise { + const db = await this.db() + const changed: ChatMessage[] = [] + await new Promise((resolve, reject) => { + const tx = db.transaction(this.storeName, 'readwrite') + const req = tx.objectStore(this.storeName).index(indexName).openCursor(IDBKeyRange.only(key)) + req.onsuccess = () => { + const cursor = req.result + if (!cursor) return + const v = cursor.value as ChatMessage + if (v.direction === 'in' && v.unread && extra(v)) { + const next = { ...v, unread: false } + cursor.update(next) + changed.push(next) + } + cursor.continue() + } + req.onerror = () => reject(req.error) + tx.oncomplete = () => resolve() + tx.onerror = () => reject(tx.error) + }) + if (changed.length > 0) this.emit() + return changed + } + subscribe(cb: () => void): () => void { this.listeners.add(cb) return () => this.listeners.delete(cb) diff --git a/frontend/src/lib/media.ts b/frontend/src/lib/media.ts index 90d333c..7abab21 100644 --- a/frontend/src/lib/media.ts +++ b/frontend/src/lib/media.ts @@ -96,6 +96,12 @@ export class MediaResolver { return URL.createObjectURL(blob) })() this.cache.set(key, promise) + // Only *successes* are worth memoizing. Leaving a rejected promise in the + // cache would make one transient bee error permanent for this reference, + // and would surface as an unhandled rejection if nothing else awaited it. + promise.catch(() => { + if (this.cache.get(key) === promise) this.cache.delete(key) + }) return promise } diff --git a/frontend/src/lib/messages-store.ts b/frontend/src/lib/messages-store.ts index c011bac..dcb1b7c 100644 --- a/frontend/src/lib/messages-store.ts +++ b/frontend/src/lib/messages-store.ts @@ -13,6 +13,8 @@ interface ChatMessageBase { status?: MessageStatus /** When set, this message belongs to a group conversation rather than a 1:1. */ groupId?: Hex + /** Inbound only — true until the user has actually looked at the conversation. */ + unread?: boolean } export interface TextMessage extends ChatMessageBase { @@ -55,6 +57,11 @@ export function previewOf(msg: ChatMessage): string { } } +/** Count of inbound rows still flagged unread. */ +export function unreadCount(list: ChatMessage[]): number { + return list.reduce((n, m) => n + (m.direction === 'in' && m.unread ? 1 : 0), 0) +} + export interface MessagesStore { put(msg: ChatMessage): Promise updateStatus(msgId: Hex, status: MessageStatus): Promise @@ -64,6 +71,14 @@ export interface MessagesStore { listGroupConversations(): Promise /** Delete all 1:1 messages with this peer. Group messages with the same peer are kept. */ clearForPeer(peer: Hex): Promise + /** + * Clear the unread flag on this conversation's inbound messages. Returns the + * rows that actually changed, so the caller can emit one read receipt each + * (and stays idempotent when the view re-renders). + */ + markConversationRead(peer: Hex): Promise + /** Same, for a group conversation. */ + markGroupRead(groupId: Hex): Promise /** A change-notification stream so React can refresh without polling. */ subscribe(cb: () => void): () => void } @@ -112,7 +127,7 @@ export class InMemoryMessages implements MessagesStore { const out: ConversationSummary[] = [] for (const [, list] of byPeer) { list.sort((a, b) => a.ts - b.ts) - out.push({ peer: list[0].peer, lastMessage: list[list.length - 1], unread: 0 }) + out.push({ peer: list[0].peer, lastMessage: list[list.length - 1], unread: unreadCount(list) }) } out.sort((a, b) => b.lastMessage.ts - a.lastMessage.ts) return out @@ -126,6 +141,28 @@ export class InMemoryMessages implements MessagesStore { this.emit() } + async markConversationRead(peer: Hex): Promise { + const target = peer.toLowerCase() + return this.clearUnread(m => !m.groupId && m.peer.toLowerCase() === target) + } + + async markGroupRead(groupId: Hex): Promise { + const target = groupId.toLowerCase() + return this.clearUnread(m => !!m.groupId && m.groupId.toLowerCase() === target) + } + + private clearUnread(match: (m: ChatMessage) => boolean): ChatMessage[] { + const changed: ChatMessage[] = [] + for (const [id, m] of this.byId) { + if (m.direction !== 'in' || !m.unread || !match(m)) continue + const next = { ...m, unread: false } + this.byId.set(id, next) + changed.push(next) + } + if (changed.length > 0) this.emit() + return changed + } + async listGroupConversations(): Promise { const byGroup = new Map() for (const m of this.byId.values()) { @@ -138,7 +175,11 @@ export class InMemoryMessages implements MessagesStore { const out: GroupConversationSummary[] = [] for (const [, list] of byGroup) { list.sort((a, b) => a.ts - b.ts) - out.push({ groupId: list[0].groupId as Hex, lastMessage: list[list.length - 1], unread: 0 }) + out.push({ + groupId: list[0].groupId as Hex, + lastMessage: list[list.length - 1], + unread: unreadCount(list), + }) } out.sort((a, b) => b.lastMessage.ts - a.lastMessage.ts) return out diff --git a/frontend/src/lib/reliability.ts b/frontend/src/lib/reliability.ts index 24c7532..20d7f16 100644 --- a/frontend/src/lib/reliability.ts +++ b/frontend/src/lib/reliability.ts @@ -212,10 +212,10 @@ export class Reliability { private async onIncoming(env: Envelope): Promise { if (env.type === 'ack') { const ackId = (env.payload as { ackMsgId?: Hex } | undefined)?.ackMsgId - if (ackId) await this.transitionTo(ackId, 'delivered') + if (ackId) await this.transitionTo(ackId, 'delivered', env.from) } else if (env.type === 'read') { const readId = (env.payload as { readMsgId?: Hex } | undefined)?.readMsgId - if (readId) await this.transitionTo(readId, 'read') + if (readId) await this.transitionTo(readId, 'read', env.from) } else if (env.type === 'msg') { // Auto-ack on receipt (spec §6). await this.autoAck(env) @@ -241,9 +241,14 @@ export class Reliability { } } - private async transitionTo(msgId: Hex, status: 'delivered' | 'read'): Promise { + private async transitionTo(msgId: Hex, status: 'delivered' | 'read', from: Hex): Promise { const entry = await this.opts.outbox.get(msgId) if (!entry) return + // Only the addressee of a message may report on its fate. Without this, + // any peer who learns a msgId can fake a receipt — and because the + // transition also cancels the retry timer below, that would suppress + // retransmission of a message the real recipient never got. + if (entry.to.wallet.toLowerCase() !== from.toLowerCase()) return // delivered -> read is allowed; read -> anything is terminal. if (entry.status === 'read') return if (entry.status === 'delivered' && status === 'delivered') return diff --git a/frontend/src/lib/transport.ts b/frontend/src/lib/transport.ts index 115ec31..7bd2734 100644 --- a/frontend/src/lib/transport.ts +++ b/frontend/src/lib/transport.ts @@ -127,9 +127,14 @@ export class Transport implements TransportLike { } private markSeen(msgId: string) { - if (this.dedup.size >= this.dedupCapacity) { + // Re-inserting moves the id to the end of the Set's iteration order, which + // is what makes the eviction below LRU rather than plain FIFO — otherwise a + // hot msgId (one being retransmitted) could be evicted while cold ones stay. + this.dedup.delete(msgId) + while (this.dedup.size >= this.dedupCapacity) { const oldest = this.dedup.values().next().value - if (oldest) this.dedup.delete(oldest) + if (oldest === undefined) break + this.dedup.delete(oldest) } this.dedup.add(msgId) } diff --git a/frontend/test/unit/authorization.test.ts b/frontend/test/unit/authorization.test.ts new file mode 100644 index 0000000..a888148 --- /dev/null +++ b/frontend/test/unit/authorization.test.ts @@ -0,0 +1,289 @@ +import { describe, it, expect, vi } from 'vitest' +import { recoverMessageAddress } from 'viem' +import { privateKeyToAccount, generatePrivateKey } from 'viem/accounts' +import { signEnvelope, verifyEnvelope, makeMsgId, canonicalize } from '../../src/lib/envelope' +import { Reliability } from '../../src/lib/reliability' +import { InMemoryOutbox } from '../../src/lib/outbox' +import { CallManager } from '../../src/lib/calls' +import { InMemoryGroups, mayApplyGroupState, isGroupMember } from '../../src/lib/groups-store' +import type { TransportLike, SendArgs, SubscribeArgs } from '../../src/lib/transport' +import type { Envelope, Group, Hex, PeerProfile } from '../../src/lib/types' + +const ALICE: Hex = ('0x' + 'aa'.repeat(20)) as Hex +const BOB: Hex = ('0x' + 'bb'.repeat(20)) as Hex +const MALLORY: Hex = ('0x' + 'cc'.repeat(20)) as Hex + +const prof = (w: Hex): PeerProfile => ({ + wallet: w, + pssPublicKey: ('0x02' + 'ff'.repeat(32)) as Hex, + swarmOverlay: ('0x' + 'ff'.repeat(32)) as Hex, +}) + +function inbound(from: Hex, type: Envelope['type'], payload: unknown): Envelope { + return { + v: 1, + type, + msgId: ('0x' + 'de'.repeat(32)) as Hex, + from, + to: ALICE, + feedOwner: ('0x' + 'cf'.repeat(20)) as Hex, + ts: 1, + nonce: ('0x' + '00'.repeat(16)) as Hex, + payload, + sig: ('0x' + '22'.repeat(65)) as Hex, + } +} + +class FakeTransport implements TransportLike { + sent: Envelope[] = [] + retransmits: Envelope[] = [] + private listener?: SubscribeArgs + private n = 0 + + async send(args: SendArgs): Promise { + this.n++ + const env: Envelope = { + v: 1, + type: args.type, + msgId: ('0x' + this.n.toString(16).padStart(64, '0')) as Hex, + from: ALICE, + to: args.to.wallet, + feedOwner: ('0x' + 'a1'.repeat(20)) as Hex, + ts: Date.now(), + nonce: ('0x' + 'ee'.repeat(16)) as Hex, + payload: args.payload, + sig: ('0x' + '11'.repeat(65)) as Hex, + } + this.sent.push(env) + return env + } + async retransmit(env: Envelope): Promise { this.retransmits.push(env) } + subscribe(args: SubscribeArgs) { + this.listener = args + return { cancel: () => { this.listener = undefined } } + } + async deliver(env: Envelope) { await this.listener?.onMessage(env) } +} + +describe('envelope: msgId is bound to (from, nonce, ts)', () => { + const alice = privateKeyToAccount(generatePrivateKey()) + const aliceSign = (m: string) => alice.signMessage({ message: m }) + const FEED = ('0x' + 'a1'.repeat(20)) as Hex + + it('rejects a perfectly-signed envelope carrying an attacker-chosen msgId', async () => { + // Build the body by hand so the signature is genuinely valid over it — + // the *only* defect is that msgId is not the digest of (from, nonce, ts). + // Without the binding check this envelope verifies, letting the sender + // collide with any msgId they like in the dedup set and the outbox. + const unsigned = { + v: 1 as const, + type: 'msg' as const, + msgId: ('0x' + 'ff'.repeat(32)) as Hex, + from: alice.address, + to: BOB, + feedOwner: FEED, + ts: 1737000000000, + nonce: ('0x' + 'ab'.repeat(16)) as Hex, + payload: { kind: 'text', text: 'hi' }, + } + const forged: Envelope = { ...unsigned, sig: await aliceSign(canonicalize(unsigned)) } + + // The signature really does recover to `from` … + expect(await recoverMessageAddress({ message: canonicalize(unsigned), signature: forged.sig })) + .toBe(alice.address) + // … and it is still rejected. + expect(await verifyEnvelope(forged)).toBe(false) + }) + + it('accepts an honest msgId', async () => { + const env = await signEnvelope( + { from: alice.address, to: BOB, feedOwner: FEED, type: 'msg', payload: {} }, + aliceSign, + ) + expect(env.msgId).toBe(makeMsgId(alice.address, env.nonce, env.ts)) + expect(await verifyEnvelope(env)).toBe(true) + }) +}) + +describe('reliability: only the addressee may report on a message', () => { + it('ignores an ack from someone who is not the recipient', async () => { + vi.useFakeTimers() + const transport = new FakeTransport() + const outbox = new InMemoryOutbox() + const rel = new Reliability({ transport, outbox, backoff: [1000, 2000] }) + await rel.start() + + const entry = await rel.send({ to: prof(BOB), type: 'msg', payload: {} }) + await transport.deliver(inbound(MALLORY, 'ack', { ackMsgId: entry.msgId })) + + const after = await outbox.get(entry.msgId) + expect(after?.status).toBe('sent') // not 'delivered' + expect(after?.nextRetryAt).not.toBeNull() + + // Crucially, the retry chain survives the forged receipt. + await vi.advanceTimersByTimeAsync(1500) + expect(transport.retransmits).toHaveLength(1) + vi.useRealTimers() + }) + + it('ignores a read receipt from a third party', async () => { + const transport = new FakeTransport() + const outbox = new InMemoryOutbox() + const rel = new Reliability({ transport, outbox, backoff: [1000] }) + await rel.start() + + const entry = await rel.send({ to: prof(BOB), type: 'msg', payload: {} }) + await transport.deliver(inbound(BOB, 'ack', { ackMsgId: entry.msgId })) + await transport.deliver(inbound(MALLORY, 'read', { readMsgId: entry.msgId })) + + expect((await outbox.get(entry.msgId))?.status).toBe('delivered') + }) + + it('still honours receipts from the real recipient, case-insensitively', async () => { + const transport = new FakeTransport() + const outbox = new InMemoryOutbox() + const rel = new Reliability({ transport, outbox, backoff: [1000] }) + await rel.start() + + const entry = await rel.send({ to: prof(BOB), type: 'msg', payload: {} }) + await transport.deliver(inbound(BOB.toUpperCase().replace('0X', '0x') as Hex, 'read', { + readMsgId: entry.msgId, + })) + expect((await outbox.get(entry.msgId))?.status).toBe('read') + }) +}) + +describe('calls: signalling is scoped to the other end of the call', () => { + class FakePc { + iceConnectionState: RTCIceConnectionState = 'new' + onicecandidate: unknown = null + ontrack: unknown = null + oniceconnectionstatechange: unknown = null + added: RTCIceCandidateInit[] = [] + closed = false + remote: RTCSessionDescriptionInit | null = null + addTrack() {} + async createOffer() { return { type: 'offer', sdp: 'v=0' } as RTCSessionDescriptionInit } + async createAnswer() { return { type: 'answer', sdp: 'v=0' } as RTCSessionDescriptionInit } + async setLocalDescription() {} + async setRemoteDescription(d: RTCSessionDescriptionInit) { this.remote = d } + async addIceCandidate(c: RTCIceCandidateInit) { this.added.push(c) } + close() { this.closed = true } + } + + function harness() { + const pcs: FakePc[] = [] + const manager = new CallManager({ + send: async () => {}, + resolvePeer: async w => prof(w), + rtcFactory: () => { const pc = new FakePc(); pcs.push(pc); return pc as unknown as RTCPeerConnection }, + getUserMedia: async () => ({ getTracks: () => [] } as unknown as MediaStream), + }) + return { manager, pcs } + } + + it('ignores a hangup from a stranger', async () => { + const { manager } = harness() + const call = await manager.startCall(prof(BOB), 'audio') + await manager.handleSignaling(inbound(MALLORY, 'call-hangup', { callId: call.callId })) + expect(call.state).toBe('calling') + expect(manager.currentCall).toBe(call) + }) + + it('ignores a hangup with no callId, even from the real peer', async () => { + const { manager } = harness() + const call = await manager.startCall(prof(BOB), 'audio') + // A missing callId used to act as a wildcard match. + await manager.handleSignaling(inbound(BOB, 'call-hangup', { reason: 'lol' })) + expect(call.state).toBe('calling') + }) + + it('honours a hangup from the peer with the right callId', async () => { + const { manager } = harness() + const call = await manager.startCall(prof(BOB), 'audio') + await manager.handleSignaling(inbound(BOB, 'call-hangup', { callId: call.callId, reason: 'bye' })) + expect(call.state).toBe('ended') + expect(call.hangupReason).toBe('bye') + }) + + it('ignores an answer and ICE from a third party', async () => { + const { manager, pcs } = harness() + const call = await manager.startCall(prof(BOB), 'audio') + await manager.handleSignaling(inbound(MALLORY, 'call-answer', { callId: call.callId, sdp: 'v=0' })) + expect(call.state).toBe('calling') + expect(pcs[0].remote).toBeNull() + + await manager.handleSignaling(inbound(MALLORY, 'ice', { + callId: call.callId, candidates: [{ candidate: 'x' }], + })) + expect(pcs[0].added).toHaveLength(0) + }) +}) + +describe('groups: who may change group state', () => { + const GID = ('0x' + '11'.repeat(32)) as Hex + const base: Group = { + id: GID, name: 'Team', members: [ALICE, BOB], admin: ALICE, + createdAt: 1000, version: 1, + } + + it('lets the admin update a group we already have', () => { + const incoming: Group = { ...base, name: 'Team v2', members: [ALICE, BOB, MALLORY], version: 2 } + expect(mayApplyGroupState({ incoming, from: ALICE, self: BOB, existing: base })).toBe(true) + }) + + it('rejects an update from a non-admin member', () => { + const incoming: Group = { ...base, name: 'hijacked', version: 2 } + expect(mayApplyGroupState({ incoming, from: BOB, self: BOB, existing: base })).toBe(false) + }) + + it('rejects an update from a total stranger', () => { + const incoming: Group = { ...base, name: 'pwned', members: [MALLORY], admin: MALLORY, version: 99 } + expect(mayApplyGroupState({ incoming, from: MALLORY, self: BOB, existing: base })).toBe(false) + }) + + it('rejects a member promoting themselves to admin', () => { + // The check must use the *existing* admin, not the incoming payload's. + const incoming: Group = { ...base, admin: BOB, version: 2 } + expect(mayApplyGroupState({ incoming, from: BOB, self: BOB, existing: base })).toBe(false) + }) + + it('accepts a first-contact invite from the declared admin', () => { + expect(mayApplyGroupState({ incoming: base, from: ALICE, self: BOB, existing: null })).toBe(true) + }) + + it('rejects a first-contact group that does not include us', () => { + const incoming: Group = { ...base, members: [ALICE, MALLORY] } + expect(mayApplyGroupState({ incoming, from: ALICE, self: BOB, existing: null })).toBe(false) + }) + + it('rejects a first-contact invite forged by a non-admin member', () => { + expect(mayApplyGroupState({ incoming: base, from: MALLORY, self: BOB, existing: null })).toBe(false) + }) + + it('rejects malformed payloads', () => { + for (const bad of [null, undefined, {}, 'nope', { ...base, members: [] }, { ...base, admin: 'x' }]) { + expect(mayApplyGroupState({ incoming: bad, from: ALICE, self: BOB, existing: null })).toBe(false) + } + }) + + it('rejects an update whose id does not match the group being replaced', () => { + const incoming: Group = { ...base, id: ('0x' + '22'.repeat(32)) as Hex, version: 2 } + expect(mayApplyGroupState({ incoming, from: ALICE, self: BOB, existing: base })).toBe(false) + }) + + it('isGroupMember is case-insensitive', () => { + expect(isGroupMember(base, BOB.toUpperCase().replace('0X', '0x') as Hex)).toBe(true) + expect(isGroupMember(base, MALLORY)).toBe(false) + }) + + it('the store still applies an authorized update end-to-end', async () => { + const groups = new InMemoryGroups() + await groups.put(base) + const incoming: Group = { ...base, name: 'Team v2', version: 2 } + const args = { incoming: incoming as unknown, from: ALICE, self: BOB, existing: base } + expect(mayApplyGroupState(args)).toBe(true) + if (mayApplyGroupState(args)) await groups.put(args.incoming) + expect((await groups.get(GID))?.name).toBe('Team v2') + }) +}) diff --git a/frontend/test/unit/media-resolver.test.ts b/frontend/test/unit/media-resolver.test.ts new file mode 100644 index 0000000..25d1c03 --- /dev/null +++ b/frontend/test/unit/media-resolver.test.ts @@ -0,0 +1,63 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { MediaResolver } from '../../src/lib/media' +import type { Bee } from '@ethersphere/bee-js' +import type { SwarmRef } from '../../src/lib/types' + +const REF = ('0x' + 'ab'.repeat(32)) as SwarmRef + +function stubUrls() { + let i = 0 + const revoked: string[] = [] + globalThis.URL.createObjectURL = vi.fn(() => `blob:${++i}`) + globalThis.URL.revokeObjectURL = vi.fn((u: string) => { revoked.push(u) }) + return revoked +} + +describe('MediaResolver caching', () => { + beforeEach(() => { stubUrls() }) + + it('retries after a failure instead of caching the rejection forever', async () => { + let calls = 0 + const bee = { + async downloadFile() { + calls++ + if (calls === 1) throw new Error('bee hiccup') + return { data: { toUint8Array: () => new Uint8Array([1, 2]) }, contentType: 'image/png' } + }, + } as unknown as Bee + + const r = new MediaResolver(bee) + await expect(r.resolve(REF)).rejects.toThrow('bee hiccup') + // The node has recovered; the second attempt must actually reach it. + await expect(r.resolve(REF)).resolves.toMatch(/^blob:/) + expect(calls).toBe(2) + }) + + it('still memoizes successes', async () => { + let calls = 0 + const bee = { + async downloadFile() { + calls++ + return { data: { toUint8Array: () => new Uint8Array([1]) }, contentType: 'image/png' } + }, + } as unknown as Bee + + const r = new MediaResolver(bee) + const [a, b] = await Promise.all([r.resolve(REF), r.resolve(REF)]) + expect(a).toBe(b) + expect(calls).toBe(1) + + await r.resolve(REF) + expect(calls).toBe(1) + }) + + it('a failed resolve leaves no entry to revoke on dispose', async () => { + const revoked = stubUrls() + const bee = { async downloadFile() { throw new Error('nope') } } as unknown as Bee + const r = new MediaResolver(bee) + await expect(r.resolve(REF)).rejects.toThrow('nope') + r.dispose() + await new Promise(res => setTimeout(res, 0)) + expect(revoked).toHaveLength(0) + }) +}) diff --git a/frontend/test/unit/unread.test.ts b/frontend/test/unit/unread.test.ts new file mode 100644 index 0000000..de04dfa --- /dev/null +++ b/frontend/test/unit/unread.test.ts @@ -0,0 +1,123 @@ +import { describe, it, expect, beforeEach } from 'vitest' +// IndexedDBMessages takes an injected factory but still reaches for the global +// IDBKeyRange (fine in a browser, absent in Node) — /auto installs both. +import 'fake-indexeddb/auto' +import { IDBFactory } from 'fake-indexeddb' +import { InMemoryMessages, unreadCount } from '../../src/lib/messages-store' +import type { ChatMessage, MessagesStore } from '../../src/lib/messages-store' +import { IndexedDBMessages } from '../../src/lib/idb-messages' +import type { Hex } from '../../src/lib/types' + +const ALICE: Hex = ('0x' + 'aa'.repeat(20)) as Hex +const BOB: Hex = ('0x' + 'bb'.repeat(20)) as Hex +const CAROL: Hex = ('0x' + 'cc'.repeat(20)) as Hex +const G1: Hex = ('0x' + '11'.repeat(32)) as Hex + +let n = 0 +function msg(over: Partial & { peer: Hex }): ChatMessage { + n++ + return { + msgId: ('0x' + n.toString(16).padStart(64, '0')) as Hex, + direction: 'in', + ts: n, + kind: 'text', + text: 'hi', + unread: true, + ...over, + } as ChatMessage +} + +describe('unreadCount', () => { + it('counts only inbound rows still flagged unread', () => { + expect(unreadCount([ + msg({ peer: BOB }), + msg({ peer: BOB, unread: false }), + msg({ peer: BOB, direction: 'out', unread: true }), // outbound never counts + msg({ peer: BOB }), + ])).toBe(2) + }) +}) + +/** Same behavioural contract for both MessagesStore implementations. */ +function sharedSpec(name: string, make: () => MessagesStore) { + describe(`${name} — unread + read receipts`, () => { + let store: MessagesStore + beforeEach(() => { store = make() }) + + it('reports unread counts per conversation', async () => { + await store.put(msg({ peer: BOB })) + await store.put(msg({ peer: BOB })) + await store.put(msg({ peer: BOB, direction: 'out', unread: false })) + await store.put(msg({ peer: CAROL })) + + const convos = await store.listConversations() + const byPeer = new Map(convos.map(c => [c.peer.toLowerCase(), c.unread])) + expect(byPeer.get(BOB.toLowerCase())).toBe(2) + expect(byPeer.get(CAROL.toLowerCase())).toBe(1) + }) + + it('markConversationRead clears the flag and returns what changed', async () => { + await store.put(msg({ peer: BOB })) + await store.put(msg({ peer: BOB })) + await store.put(msg({ peer: CAROL })) + + const changed = await store.markConversationRead(BOB) + expect(changed).toHaveLength(2) + expect(changed.every(m => m.unread === false)).toBe(true) + + const convos = await store.listConversations() + const byPeer = new Map(convos.map(c => [c.peer.toLowerCase(), c.unread])) + expect(byPeer.get(BOB.toLowerCase())).toBe(0) + expect(byPeer.get(CAROL.toLowerCase())).toBe(1) // untouched + }) + + it('is idempotent — a second call reports nothing to receipt', async () => { + await store.put(msg({ peer: BOB })) + expect(await store.markConversationRead(BOB)).toHaveLength(1) + expect(await store.markConversationRead(BOB)).toHaveLength(0) + }) + + it('does not touch group rows when marking a 1:1 read', async () => { + await store.put(msg({ peer: BOB })) + await store.put(msg({ peer: BOB, groupId: G1 })) + + expect(await store.markConversationRead(BOB)).toHaveLength(1) + const groups = await store.listGroupConversations() + expect(groups[0].unread).toBe(1) + }) + + it('markGroupRead clears the whole group regardless of sender', async () => { + await store.put(msg({ peer: BOB, groupId: G1 })) + await store.put(msg({ peer: CAROL, groupId: G1 })) + await store.put(msg({ peer: ALICE, groupId: G1, direction: 'out', unread: false })) + + const changed = await store.markGroupRead(G1) + expect(changed).toHaveLength(2) + const groups = await store.listGroupConversations() + expect(groups[0].unread).toBe(0) + }) + + it('markConversationRead is case-insensitive on the peer', async () => { + await store.put(msg({ peer: BOB })) + const upper = BOB.toUpperCase().replace('0X', '0x') as Hex + expect(await store.markConversationRead(upper)).toHaveLength(1) + }) + + it('notifies subscribers when rows flip', async () => { + await store.put(msg({ peer: BOB })) + let hits = 0 + const off = store.subscribe(() => { hits++ }) + await store.markConversationRead(BOB) + expect(hits).toBe(1) + await store.markConversationRead(BOB) // no-op, no notification + expect(hits).toBe(1) + off() + }) + }) +} + +sharedSpec('InMemoryMessages', () => new InMemoryMessages()) +sharedSpec('IndexedDBMessages', () => new IndexedDBMessages({ + dbName: `unread-${Math.random().toString(36).slice(2)}`, + idbFactory: new IDBFactory(), +})) diff --git a/src/ContactRegistry.sol b/src/ContactRegistry.sol index 84c3f5a..5f34247 100644 --- a/src/ContactRegistry.sol +++ b/src/ContactRegistry.sol @@ -72,8 +72,9 @@ contract ContactRegistry { function getUsers(uint256 offset, uint256 limit) external view returns (address[] memory page) { uint256 total = _users.length; if (offset >= total) return new address[](0); - uint256 end = offset + limit; - if (end > total) end = total; + // Compare against the remaining span rather than computing offset+limit, + // which would revert on overflow for a large `limit` instead of clamping. + uint256 end = limit > total - offset ? total : offset + limit; page = new address[](end - offset); for (uint256 i = offset; i < end; i++) { page[i - offset] = _users[i]; diff --git a/test/ContactRegistry.t.sol b/test/ContactRegistry.t.sol index 599beb4..9dc83e0 100644 --- a/test/ContactRegistry.t.sol +++ b/test/ContactRegistry.t.sol @@ -317,6 +317,28 @@ contract ContactRegistryTest is Test { assertEq(page[1], bob); } + /// A caller asking for "everything from here" must get a clamped page, not + /// an arithmetic panic from `offset + limit` wrapping. + function test_GetUsersMaxLimitClampsInsteadOfOverflowing() public { + vm.prank(alice); + registry.register("alice", alicePss, aliceOverlay); + vm.prank(bob); + registry.register("bob", bobPss, bobOverlay); + vm.prank(carol); + registry.register("carol", alicePss, aliceOverlay); + + address[] memory all = registry.getUsers(0, type(uint256).max); + assertEq(all.length, 3); + + // The overflow case: any non-zero offset with a max limit. + address[] memory tail = registry.getUsers(2, type(uint256).max); + assertEq(tail.length, 1); + assertEq(tail[0], carol); + + address[] memory past = registry.getUsers(3, type(uint256).max); + assertEq(past.length, 0); + } + function test_GetUsersDeactivatedStillListed() public { vm.prank(alice); registry.register("alice", alicePss, aliceOverlay); From 61b28fd48c22cfc5bd18fc22d61bfbd57df9bd7f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 12:53:09 +0000 Subject: [PATCH 2/4] ci: run contract and frontend suites on every push 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 Claude-Session: https://claude.ai/code/session_01KnBiaAgJUWmo2WuAbKLYUi --- .github/workflows/ci.yml | 61 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..74d1d95 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,61 @@ +name: CI + +on: + push: + branches: ['**'] + pull_request: + +# A newer push to the same branch makes the in-flight run obsolete. +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +jobs: + contracts: + name: Contracts (forge) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + submodules: recursive # lib/forge-std + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + + - run: forge --version + + - name: Build + run: forge build --sizes + + # No test forks the chain, so this needs no RPC access. + - name: Test + run: forge test -vvv + + frontend: + name: Frontend (vitest) + runs-on: ubuntu-latest + defaults: + run: + working-directory: frontend + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + cache-dependency-path: frontend/package-lock.json + + - name: Install + run: npm ci + + # `npm run build` also type-checks, but running tsc first gives a clearer + # failure when the break is types rather than bundling. + - name: Type-check + run: npx tsc -b --force + + - name: Test + run: npm test + + - name: Build + run: npm run build From 20a1ff2dd205df48323d54071d3b34dd1a204e88 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 12:55:07 +0000 Subject: [PATCH 3/4] fix(foundry): stop forcing every forge test run through a live RPC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_01KnBiaAgJUWmo2WuAbKLYUi --- .github/workflows/ci.yml | 3 ++- foundry.toml | 8 +++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 74d1d95..cfd054c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,7 +27,8 @@ jobs: - name: Build run: forge build --sizes - # No test forks the chain, so this needs no RPC access. + # Runs fully local: no test creates a fork, and foundry.toml deliberately + # leaves `eth_rpc_url` unset so the default profile doesn't force one. - name: Test run: forge test -vvv diff --git a/foundry.toml b/foundry.toml index b8f5ee3..43b90b8 100644 --- a/foundry.toml +++ b/foundry.toml @@ -3,7 +3,13 @@ src = "src" out = "out" libs = ["lib"] solc = "0.8.28" -eth_rpc_url = "gnosis" + +# NOTE: deliberately no `eth_rpc_url` here. Setting it makes `forge test` fork +# from that endpoint for every run, so the whole suite went over the network and +# the fuzz/invariant tests got 429-ed by the public Gnosis RPC — despite the +# Makefile documenting `test-unit` as "no fork". Targets that do need an +# endpoint pass `--rpc-url` / `--fork-url` explicitly (see `make test-fork`), +# and the aliases below still resolve for those. [rpc_endpoints] gnosis = "https://rpc.gnosischain.com" From 2de60b5bc68848b8fdda24cbb8fcc9e6ca13a4f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 06:08:17 +0000 Subject: [PATCH 4/4] ci: bump checkout and setup-node to v5 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 Claude-Session: https://claude.ai/code/session_01KnBiaAgJUWmo2WuAbKLYUi --- .github/workflows/ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cfd054c..6f2b99b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ jobs: name: Contracts (forge) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 with: submodules: recursive # lib/forge-std @@ -39,9 +39,9 @@ jobs: run: working-directory: frontend steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v5 with: node-version: 22 cache: npm