From cc238277921ad11011d8385f42f07dc2df720092 Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 11 Aug 2026 10:19:28 +0200 Subject: [PATCH] Review fixes: guard stale auth callbacks; clear the setting on fail-open - Codex P1: backgrounding mid-authentication re-locked the app, but a Face ID success that resolved just after could still clear the new lock, bypassing the re-lock. Each lock bumps a generation and unlock callbacks from a superseded cycle are ignored. - Codex P2: the deliberate fail-open (device passcode removed) unlocked but left privacy.appLockEnabled true, so the toggle kept claiming the lock was on and re-adding a passcode silently reactivated it. The fail-open path now turns the setting off, matching the promised "turns itself off" behavior. Stale-callback path pinned by a new test. Co-Authored-By: Claude Fable 5 --- bitchat/App/AppLockModel.swift | 16 +++++++++++++++- bitchatTests/ChatViewModelTests.swift | 17 +++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/bitchat/App/AppLockModel.swift b/bitchat/App/AppLockModel.swift index 7a57f0dd..63956e55 100644 --- a/bitchat/App/AppLockModel.swift +++ b/bitchat/App/AppLockModel.swift @@ -45,6 +45,10 @@ final class AppLockModel: ObservableObject { private let isEnabledProvider: () -> Bool private let authenticate: (@escaping @MainActor (Bool) -> Void) -> Void + /// Bumped on every lock, so a success callback from a prior unlock + /// attempt (e.g. Face ID resolving just as the app backgrounded and + /// re-locked) cannot clear the new lock. + private var lockGeneration = 0 /// Providers are injectable so tests drive the lock without LAContext /// or the shared UserDefaults. @@ -67,14 +71,20 @@ final class AppLockModel: ObservableObject { func lockIfEnabled() { guard isEnabledProvider() else { return } + lockGeneration &+= 1 + isAuthenticating = false isLocked = true } func requestUnlock() { guard isLocked, !isAuthenticating else { return } isAuthenticating = true + let generation = lockGeneration authenticate { [weak self] success in guard let self else { return } + // Ignore a callback for a lock cycle that has already been + // superseded (backgrounded and re-locked mid-authentication). + guard generation == self.lockGeneration else { return } self.isAuthenticating = false if success { self.isLocked = false @@ -89,7 +99,11 @@ final class AppLockModel: ObservableObject { // Fail OPEN, deliberately: removing the device passcode already // requires knowing it, so this state means the owner disabled // it — locking them out of their own chats would punish exactly - // the wrong person. The settings copy states this rule. + // the wrong person. Turn the setting itself off too, so the + // toggle stops claiming the lock is on and re-adding a passcode + // later doesn't silently reactivate it (the copy promises the + // lock "turns itself off"). + AppLockSettings.setEnabled(false) Task { @MainActor in completion(true) } return } diff --git a/bitchatTests/ChatViewModelTests.swift b/bitchatTests/ChatViewModelTests.swift index 64c1e22a..73f11c0b 100644 --- a/bitchatTests/ChatViewModelTests.swift +++ b/bitchatTests/ChatViewModelTests.swift @@ -2420,6 +2420,23 @@ struct AppLockModelTests { #expect(model.isLocked) } + @Test @MainActor + func staleAuthCallbackCannotUnlockAfterRelock() { + var pending: ((Bool) -> Void)? + let model = AppLockModel( + isEnabledProvider: { true }, + authenticate: { completion in pending = completion } + ) + #expect(model.isLocked) + + // Auth begins, then the app backgrounds and re-locks before Face ID + // resolves; the late success must be ignored. + model.requestUnlock() + model.lockIfEnabled() + pending?(true) + #expect(model.isLocked) + } + @Test @MainActor func staysInertWhenDisabled() { let model = AppLockModel(