Origin: code review of PR #45 (screen-lock app gate). Test-adequacy finding — the root reason several live bugs shipped with green CI.
Problem
The PR's security-critical branching shipped largely untested. The 20 new JVM tests cover only the three pure classes (AppLockGate, KeyInvalidationPolicy, PassphraseSession); the logic that actually decides when to wipe user data or drop the lock is unpinned:
AppLockViewModel.onForegroundLockAction dispatch (that DISABLE_APP_LOCK persists the setting, that CLEAR_* set the pending flag before restarting, that CLEAR_AND_REQUIRE_AUTH keeps app-lock on).
DatabaseKeyStore dual-seal exchange (sealWithAuth removing SEALED_MASTER, sealWithMaster, resetSealedPassphrase, consumeClearPending) — the "never both seals at once" / "not recoverable without auth" invariants.
SettingsViewModel.setAppLock reject/reseal/disable branches (fully JVM-testable under the repo's existing MockK pattern today).
KeyInvalidationPolicyTest pins only 6 of 8 app-lock-on rows; the common (appLock on, encryptCache off, secure, valid)→REQUIRE_AUTH row is unpinned — a mutation to PROCEED (silent lock bypass) passes all current tests.
Suggested fix
Add MockK ViewModel tests (mirroring MailboxViewModelTest) for AppLockViewModel and SettingsViewModel.setAppLock; make the exhaustive 16-row KeyInvalidationPolicy table a test; add instrumentation (androidTest) coverage for the genuinely device-only DatabaseKeyStore/DatabaseKeyCipher seal exchange, or introduce a DataStore/crypto seam so it is unit-testable. (Some of this is being added alongside the headline fixes; this ticket tracks bringing coverage to the whole surface.)
Origin: code review of PR #45 (screen-lock app gate). Test-adequacy finding — the root reason several live bugs shipped with green CI.
## Problem
The PR's security-critical branching shipped largely untested. The 20 new JVM tests cover only the three pure classes (`AppLockGate`, `KeyInvalidationPolicy`, `PassphraseSession`); the logic that actually decides when to **wipe user data** or drop the lock is unpinned:
- `AppLockViewModel.unlockOrArm` / `unwrapSealedPassphrase` OK/UNRECOVERABLE/RETRY classification.
- `AppLockViewModel.onForeground` `LockAction` dispatch (that DISABLE_APP_LOCK persists the setting, that CLEAR_* set the pending flag before restarting, that CLEAR_AND_REQUIRE_AUTH keeps app-lock on).
- `DatabaseKeyStore` dual-seal exchange (`sealWithAuth` removing `SEALED_MASTER`, `sealWithMaster`, `resetSealedPassphrase`, `consumeClearPending`) — the "never both seals at once" / "not recoverable without auth" invariants.
- `SettingsViewModel.setAppLock` reject/reseal/disable branches (fully JVM-testable under the repo's existing MockK pattern today).
- `KeyInvalidationPolicyTest` pins only 6 of 8 app-lock-on rows; the common (appLock on, encryptCache off, secure, valid)→REQUIRE_AUTH row is unpinned — a mutation to PROCEED (silent lock bypass) passes all current tests.
## Suggested fix
Add MockK ViewModel tests (mirroring `MailboxViewModelTest`) for `AppLockViewModel` and `SettingsViewModel.setAppLock`; make the exhaustive 16-row `KeyInvalidationPolicy` table a test; add instrumentation (androidTest) coverage for the genuinely device-only `DatabaseKeyStore`/`DatabaseKeyCipher` seal exchange, or introduce a DataStore/crypto seam so it is unit-testable. (Some of this is being added alongside the headline fixes; this ticket tracks bringing coverage to the whole surface.)
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Origin: code review of PR #45 (screen-lock app gate). Test-adequacy finding — the root reason several live bugs shipped with green CI.
Problem
The PR's security-critical branching shipped largely untested. The 20 new JVM tests cover only the three pure classes (
AppLockGate,KeyInvalidationPolicy,PassphraseSession); the logic that actually decides when to wipe user data or drop the lock is unpinned:AppLockViewModel.unlockOrArm/unwrapSealedPassphraseOK/UNRECOVERABLE/RETRY classification.AppLockViewModel.onForegroundLockActiondispatch (that DISABLE_APP_LOCK persists the setting, that CLEAR_* set the pending flag before restarting, that CLEAR_AND_REQUIRE_AUTH keeps app-lock on).DatabaseKeyStoredual-seal exchange (sealWithAuthremovingSEALED_MASTER,sealWithMaster,resetSealedPassphrase,consumeClearPending) — the "never both seals at once" / "not recoverable without auth" invariants.SettingsViewModel.setAppLockreject/reseal/disable branches (fully JVM-testable under the repo's existing MockK pattern today).KeyInvalidationPolicyTestpins only 6 of 8 app-lock-on rows; the common (appLock on, encryptCache off, secure, valid)→REQUIRE_AUTH row is unpinned — a mutation to PROCEED (silent lock bypass) passes all current tests.Suggested fix
Add MockK ViewModel tests (mirroring
MailboxViewModelTest) forAppLockViewModelandSettingsViewModel.setAppLock; make the exhaustive 16-rowKeyInvalidationPolicytable a test; add instrumentation (androidTest) coverage for the genuinely device-onlyDatabaseKeyStore/DatabaseKeyCipherseal exchange, or introduce a DataStore/crypto seam so it is unit-testable. (Some of this is being added alongside the headline fixes; this ticket tracks bringing coverage to the whole surface.)