security: derive group verification tone from member state, not a hardcoded ok - #78
Open
konkomaji wants to merge 1 commit into
Open
security: derive group verification tone from member state, not a hardcoded ok#78konkomaji wants to merge 1 commit into
konkomaji wants to merge 1 commit into
Conversation
…dcoded ok (ni5arga#31) describeConversation() always returned tone: 'ok' for groups, so the chat notice, the eyebrow, and the home-screen tag all showed the same green "Private" treatment whether every member was verified or one of them was a MITM impersonator slipped in at introduction. Direct chats already gate this on ctx.verified; groups now gate on whether any member (including one with no contact record yet) is unverified, and name the count the same way the existing group-creation warning does. Closes ni5arga#31
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.
Why
describeConversation()hardcodestone: 'ok'for every group (src/lib/conversation.ts:63-73), so the mode notice, its eyebrow, and the home-screen tag (src/app/index.tsx:137) show the same green "Private" treatment whether every member is verified or one of them is a MITM impersonator who slipped in at introduction. A direct chat already carries a persistent "Not verified" caution until the safety number is compared (ctx.verified); a group — which is N independent chances of a bad key, the mode with the most exposure — got no persistent warning at all. This contradictsconversation.ts's own stated invariant that "the single most dangerous failure this app can have is someone misreading the mode."What changed
src/lib/conversation.ts:describeConversationnow takes an optionalcontactslist and derives group tone/warning from member verification — any unverified member, or a member with no matching contact record yet, turns tonecautionand switches the warning to a new plural string naming how many members are unverified. A contact-less member is treated as unverified (absence of proof is not proof of safety).src/components/mode-notice.tsx: the group eyebrow now reads "NOT ALL VERIFIED" (reusing the same word as the direct-chat case) when tone iscaution, instead of always "ENCRYPTED TO MEMBERS".src/app/index.tsx: the home-screen group tag switches from a static green "Private" to the same amber "N unverified member(s)" tag already used at group-creation time (group.unverifiedTitle), so the list view doesn't contradict the chat screen.src/app/chat/[id].tsx: passescontacts(already in scope) through todescribeConversation.conversation.groupWarningUnverified.{one,other},mode.notVerifiedMembers) added to all six locales — hi/bn/mr/te/ta translations are a best-effort pass and should get the same native-speaker review already flagged for the rest of the i18n work.docs/THREAT-MODEL.mdFixed section updated.Re-derives automatically on verification change since it's computed from
contacts+groupson every render, no separate invalidation needed.Test plan
src/lib/__tests__/conversation.test.ts: all-verified → ok/green; any unverified → caution/amber with correct count; member with no contact record → treated as unverified; count is exact across multiple unverified membersnpm test(235 tests; 1 pre-existing unrelated flaky perf-timing test inforward-secrecy.test.tson this machine, not touched by this change)npm run typechecknpm run lint(0 errors; pre-existing warnings unrelated to this change)npm run i18n:checkCloses #31