Repository navigation
fix: hand the peer the first epoch key before the first message, and answer a rekey only once stored - #361
Conversation
createRoomRekeyHandler answered "ok" before onRekey stored the unwrapped keys, so a sender could act on the answer while the recipient still held no key, and a store that failed after the answer left the sender believing the key was delivered. The handler now awaits onRekey before answering "ok", and answers store_failed and rejects when onRekey rejects. Refs #338
…message After joining a conversation, the participant that mints epoch 1 sent the room.rekey and its first message at the same time, so the message could reach the peer first. A peer that posted a notice as soon as it read that message held no content key yet and the post failed with "no content key held for room". The first message after a join now waits for the key delivery attempt, which the peer answers only once the key is stored; a refused delivery still lets the message through. Fixes #338
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
|
@codex security review |
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
The notices-offline e2e test failed intermittently because the console that posts the first notice sometimes held no content key yet. When the posting console (A) has the higher device id, the other console (B) mints epoch 1, and B did that in
deliver()as a fire-and-forgetroom.rekeysent alongside its first message after joining. The reply could reach A before the rekey, and even when the rekey arrived first, core'screateRoomRekeyHandleransweredokbeforeonRekeystored the key. A rendered the reply, the test posted,postEncryptedNoticefound no epoch, and the composer showedno content key held for room <path>: room.rekey this room before posting encrypted noticesin red instead of listing the notice.I reproduced it by running the spec repeatedly on a loaded machine with a copy that logged which side was lower and what the composer showed: every failure at this step had A as the higher participant, with that error under the composer. Whichever side is lower is decided by randomly generated device ids, which is the source of the nondeterminism.
Two changes close the window by construction:
createRoomRekeyHandlerawaitsonRekeybefore answeringok, sookmeans the recipient holds the key. A store that fails now answersstore_failedand rejects, where before the sender was toldok, marked the key delivered and never sent it again.The new unit tests drive the interleaving directly: a hub whose
room.rekeyanswer is held back shows that noroom.sendgoes out until it is answered, and a handler whoseonRekeyis still pending has not answered. Both fail on the previous code (I reverted each change and watched them fail) and pass with it.The same repeated runs also showed two failures at later steps of this spec, unrelated to this race: the reopened console's Connect button not appearing within the action timeout, and the reopened console not showing the offline notice within the step timeout. Neither is addressed here.
Fixes #338