nik/bitchat
1
0
Fork 0
mirror of https://github.com/permissionlesstech/bitchat.git synced 2026-11-01 07:53:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

Drop an API nothing calls, and correct a comment that was wrong

Browse Source 
Three things found auditing the revision rather than the original, none of
them a behaviour change.

`isVerifiedAndNameBound` had no production caller. It was added at review
suggestion to collapse the double `queue.sync` at `showsVerifiedSeal` and
`isVerifiedSender` — but both of those now go through `sealAppliesToRow`,
which already answers in one lock, and the two sites that still take two
locks (`PeerListModel`, `VerificationModel`) need the two booleans
separately to decide between the verified and vouched seals, so the
conjunction cannot serve them. That leaves an unused requirement on a
protocol three test doubles have to satisfy. Removed; its cases are folded
into a test for what the baseline is actually kept for, which is letting a
sheet name the old nickname.

The comment on `demotedIfRenamed` still claimed nothing branches on
`.noiseVerified` beyond the glyph. That was shown to be 3-for-4 in review and
corrected in the thread, but not in the code, which is where it will be read.
No *behaviour* branches on it; four things the user perceives do, and all
four should move.

`MockIdentityManager` compared nicknames with `!=`, so the double called a
recase a rename while the real one does not. Which fold applies is the whole
subject of this change, so a double that gets it backwards is a trap for
whoever writes the next view test.

Verified: 201 suites / 1975 tests, every difference from `main` an addition
(8 -> 10, plus 7 and 23 new). The 17 XCTest-based suites now run too, under a
shim over swift-testing that is not in this branch, which puts
SecureIdentityStateManagerTests — the suite guarding the file this PR touches
most — under test here for the first time. Mutation-checked both ways: stub
the live mismatch and 11 binding cases fail; stub the demotion and the
coordinator case fails.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Shubham Bhandari 2026-09-14 14:03:28 +08:00
parent ff39744580
commit 729ca272b0
4 changed files with 25 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
bitchat/Identity/SecureIdentityStateManager.swift
View File

@ -137,9 +137,6 @@ protocol SecureIdentityStateManagerProtocol {
/// Whether this peer now announces a different nickname than the one its
/// trust was earned under.
func trustedNicknameMismatch(fingerprint: String) -> Bool
/// Verified AND still presenting the name it was verified under — what a
/// seal beside a LIVE name is actually asserting. One lock, not two.
func isVerifiedAndNameBound(fingerprint: String) -> Bool
/// Verified AND `renderedSender` is the name it was verified under — what
/// a seal beside a name frozen on a message row is asserting.
func sealAppliesToRow(fingerprint: String, renderedSender: String) -> Bool
@ -164,15 +161,11 @@ protocol SecureIdentityStateManagerProtocol {
}
extension SecureIdentityStateManagerProtocol {
// Default implementations so a conformance only has to provide the two
// primitives. `SecureIdentityStateManager` overrides both to answer in a
// single lock acquisition — these seal checks run per row, per render, and
// the format cache does not shield the one in ChatMessageFormatter because
// the answer is part of its cache key.
func isVerifiedAndNameBound(fingerprint: String) -> Bool {
isVerified(fingerprint: fingerprint) && !trustedNicknameMismatch(fingerprint: fingerprint)
}
// A default implementation so a conformance only has to provide the two
// primitives. `SecureIdentityStateManager` overrides it to answer in a
// single lock acquisition — this runs per row, per render, and the format
// cache does not shield the call in ChatMessageFormatter because the
// answer is part of its cache key.
func sealAppliesToRow(fingerprint: String, renderedSender: String) -> Bool {
guard isVerified(fingerprint: fingerprint) else { return false }
guard let pinned = trustedNickname(fingerprint: fingerprint), !pinned.isEmpty
@ -802,13 +795,6 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol {
queue.sync { cache.trustedNicknames?[fingerprint] }
}
func isVerifiedAndNameBound(fingerprint: String) -> Bool {
queue.sync {
guard cache.verifiedFingerprints.contains(fingerprint) else { return false }
return !liveNameMismatchLocked(fingerprint)
}
}
/// A message row shows the sender name frozen at receipt, so asking about
/// the peer's CURRENT name is the wrong question: rename away, post, rename
/// back, and the live name matches the baseline again while the archived row

13
bitchat/ViewModels/ChatPeerIdentityCoordinator.swift
View File

@ -477,9 +477,16 @@ final class ChatPeerIdentityCoordinator {
/// invalidated by verification and session changes, never by a rename, so a
/// demotion written into the cache would be both stale and sticky. The
/// session really is Noise-secured; only the claim about *who* is weakened,
/// which is exactly the difference between the two cases. Nothing branches
/// on `.noiseVerified` beyond the glyph — every behavioural site treats it
/// and `.noiseSecured` alike.
/// which is exactly the difference between the two cases.
///
/// No BEHAVIOUR branches on the distinction — `VerificationModel:33`,
/// `ContentSheetViews:659` and `:760` all treat `.noiseVerified` and
/// `.noiseSecured` alike. Four things the user perceives do, and all four
/// are meant to move: the icon (`checkmark.seal.fill` → `lock.fill`), the
/// `description` caption, the `accessibilityDescription` — so VoiceOver
/// stops saying "verified" on a demoted DM header — and the green tint on
/// the nickname at `FingerprintView:87`, which is the one site that does
/// NOT treat the two statuses identically.
@MainActor
private func demotedIfRenamed(_ status: EncryptionStatus, for peerID: PeerID) -> EncryptionStatus {
guard status == .noiseVerified,

6
bitchatTests/Mocks/MockIdentityManager.swift
View File

@ -107,7 +107,11 @@ final class MockIdentityManager: SecureIdentityStateManagerProtocol {
func trustedNicknameMismatch(fingerprint: String) -> Bool {
guard let pinned = trustedNicknames[fingerprint], !pinned.isEmpty,
let claimed = claimedNicknames[fingerprint], !claimed.isEmpty else { return false }
return pinned != claimed
// Folded like the real one. A mock that compared raw strings would
// call a recase a rename, so a view test could pass here and fail in
// production — and which fold applies is the whole subject of this
// change, so it is the last thing a double should get wrong.
return pinned.nicknameBindingKey != claimed.nicknameBindingKey
}
func isVerified(fingerprint: String) -> Bool {

11
bitchatTests/Services/SecureIdentityStateManagerNicknameBindingTests.swift
View File

@ -265,17 +265,17 @@ struct SecureIdentityStateManagerNicknameBindingTests {
}
@Test
func theCombinedLiveQueryAgreesWithItsParts() {
func theBaselineIsKeptSoASheetCanNameIt() {
// Suppressing the seal is the security fix; saying WHICH name was
// verified is what lets a reader recognise the peer instead of
// guessing. The baseline therefore survives the rename it detects.
let manager = makeManager()
announce(manager, vouchee, as: "ravi")
#expect(!manager.isVerifiedAndNameBound(fingerprint: vouchee), "not verified yet")
manager.setVerified(fingerprint: vouchee, verified: true)
#expect(manager.isVerifiedAndNameBound(fingerprint: vouchee))
#expect(manager.trustedNickname(fingerprint: vouchee) == "ravi")
announce(manager, vouchee, as: "medic")
#expect(!manager.isVerifiedAndNameBound(fingerprint: vouchee))
#expect(manager.trustedNicknameMismatch(fingerprint: vouchee))
#expect(manager.trustedNickname(fingerprint: vouchee) == "ravi",
"the baseline is what the sheet needs to name")
}
@ -292,7 +292,6 @@ struct SecureIdentityStateManagerNicknameBindingTests {
announce(manager, vouchee, as: "ravi")
#expect(!manager.trustedNicknameMismatch(fingerprint: vouchee))
#expect(manager.isVerifiedAndNameBound(fingerprint: vouchee))
#expect(manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "RAVI"))
}