Source: whole-repo multi-agent review, 2026-07-09 (run wf_b41de68c-e85). The run was cut short by usage limits before its verification pass, so every finding below is an unverified finder candidate — validate each against the current code before implementing. Findings are listed medium first, then low. Critical/high candidates from the same run were verified separately and have their own issues.
Medium (5)
app/src/main/kotlin/org/libremail/data/security/DatabaseKeyCipher.kt:57 — DatabaseKeyCipher has no test at all - no JVM unit test exists and the only instrumented security test (KeystoreCryptoTest) covers the master key, despite the KDoc claiming on-device coverage - leaving its isInvalidated() exception classification (KeyPermanentlyInvalidated->true, UserNotAuthenticated->false, unknown->false) and the encrypt() invalidated-key self-heal path entirely unexercised, even though the classification is pure control flow testable via the same seams AesGcmKeystoreCipherTest already uses.
severity: medium · category: test-gap · angle: tests
A refactor accidentally lets UserNotAuthenticatedException fall into the KeyPermanentlyInvalidatedException/generic-true path (e.g. reordering catch clauses or collapsing them); no test fails. On devices with encryptCache+appLock on, every routine pre-auth foreground pass then reports keyInvalidated=true, KeyInvalidationPolicy resolves CLEAR_AND_REQUIRE_AUTH, and the app silently wipes the user's entire cached mail and restarts - repeatedly - with the regression only discoverable on a physical device.
app/src/main/kotlin/org/libremail/ui/lock/AppLockGateHost.kt:100 — The auto-prompt LaunchedEffect races the BiometricPrompt success callback on device-credential re-entry: an ON_START-driven onForeground pass can republish Locked (gate is still LOCKED) before onAuthenticationSucceeded lands, launching a second prompt whose cancellation then re-locks a user who just authenticated.
severity: medium · category: concurrency · angle: concurrency
API 29 device-credential fallback: tapping Unlock launches the system ConfirmDeviceCredential activity, which stops MainActivity -> onBackground() sets state to Checking. User enters the correct PIN; on return, ON_START fires onForeground() (async: settings.first() + withContext probe) while the prompt's success callback is delivered separately on the main executor. If the onForeground decision lands first, gate.state is still LOCKED so publish() emits Locked, the composition re-enters the Locked branch from Checking, and LaunchedEffect(Unit) auto-presents a SECOND BiometricPrompt. Moments later onAuthenticated() unlocks and composes the app, but the stale system prompt sheet is still showing (nothing dismisses it - the effect has no cancellation cleanup); the user dismisses it -> onAuthenticationError -> viewModel.onAuthError -> gate.lock() -> the just-unlocked app re-locks and demands auth again.
app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt:240 — The auth seal is never cleared after the on-disk DB has been decrypted back to plaintext, so unlockOrArm keeps gating on hasAuthSealedPassphrase() forever; a later biometric re-enrollment then classifies the stale seal as UNRECOVERABLE and clearCacheAndRestart wipes a plaintext, fully readable cache and force-restarts the app.
severity: medium · category: correctness · angle: invariants
State: appLock ON, encryptCache toggled OFF; at the next cold start DatabaseProvisioner.ensurePlaintext converts the DB to plaintext using the unwrapped seal — but SEALED_AUTH is left in the DataStore permanently (nothing removes it after conversion; only sealWithMaster on app-lock disable or resetSealedPassphrase on wipe touch it). Months later the user enrolls a new fingerprint, permanently invalidating the auth-bound key. Next unlock: decide() -> REQUIRE_AUTH (encryptCache off, so not CLEAR_AND_REQUIRE_AUTH), the prompt succeeds, then unlockOrArm sees hasAuthSealedPassphrase()==true -> unwrapSealedPassphrase -> KeyPermanentlyInvalidatedException -> UNRECOVERABLE -> clearCacheAndRestart(disableAppLock=false): the entire (plaintext, perfectly openable) mail cache is deleted and the process is killed mid-use, forcing a full re-sync — for a seal that no longer protects anything. Fix direction: drop SEALED_AUTH (or reseal-with-master) once the decrypt-to-plaintext conversion completes, or have unwrapSealedPassphrase treat invalidation as recoverable when the on-disk DB is not encrypted.
app/src/main/kotlin/org/libremail/ui/security/CacheEncryptionGate.kt:70 — The CacheEncryptionGate host composable's fail-closed wiring is untested: CacheEncryptionErrorScreenTest covers only the inner presentational screen, and no Robolectric host test (the analogue of AppLockGateHostJvmTest) verifies that content() is NOT composed during Checking or Unavailable, that Ready composes the app, or the error->prepareReport->EphemeralReportReviewScreen->dismissReport flow.
severity: medium · category: test-gap · angle: tests
A refactor of the when(state) branches composes content() behind the cover during Checking (mirroring AppLockGateHost's keep-composed pattern) or falls through to content() on Unavailable; no test fails, and DB-backed screens now compose before the encryption probe resolves - the exact fail-open condition issue #359's gate exists to prevent, shipping silently because the gate's only test never mounts the gate itself.
app/src/test/kotlin/org/libremail/auth/OutlookAuthManagerTest.kt:249 — The token-exchange tests stub performTokenRequest(any(), any()) without ever capturing or asserting the TokenRequest, so the scope and nonce parameters of the Microsoft code exchange - the exact two fields whose omission previously broke all Outlook sign-in (AADSTS70011 'must include a scope input parameter' and AppAuth id_token nonce validation) - are completely unpinned, as are the per-resource scopes of the two refresh paths.
severity: medium · category: test-gap · angle: tests
A refactor deletes .setScope(...) or .setNonce(response.request.nonce) from OutlookAuthManager.exchangeToken (or swaps GRAPH_SCOPE/OUTLOOK_SCOPE in refreshForScope - the 'graph-at'/'exchange-at' assertions only echo canned responses); every unit test still passes, the change merges, and every new Microsoft sign-in (or every Graph send / IMAP token refresh) fails in production against the real token endpoint - a regression of the historical Outlook-login-breaking bug with no test able to catch it.
Low (12)
app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt:137 — refreshForScope constructs a new AppAuth AuthorizationService per token refresh, whose constructor (AppAuth 0.11.1) runs a PackageManager browser query and binds the browser's CustomTabsService — needless IPC for a pure token-endpoint HTTP POST that never opens a browser.
An Outlook account is active: access tokens expire roughly hourly per resource, so freshOutlookToken/freshGraphToken each run BrowserSelector.select (PM query over all installed browsers) plus a bind/unbind of Chrome's CustomTabsService about twice per hour per account for the process lifetime, purely to make one HTTPS POST. A single long-lived AuthorizationService reused for token requests (or an AppAuthConfiguration whose browser matcher skips the custom-tab warmup) does the same work with zero browser-service churn; only createAuthIntent actually needs the custom-tab binding.
app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt:144 — The AppAuth token-request plumbing — construct AuthorizationService, bridge performTokenRequest through suspendCancellableCoroutine with the token/error-or-IllegalStateException mapping, dispose in finally — is copy-pasted between exchangeToken (lines 95-105, 120-122) and refreshForScope (lines 137-152, 159-161) instead of living in one private suspend helper.
Any fix to this plumbing (e.g. adding continuation.invokeOnCancellation to cancel/dispose the in-flight request when the caller's coroutine is cancelled, or improving the error mapping) must be applied twice and can drift; when the next AppAuth provider arrives (e.g. restoring the Gmail OAuth path alongside the app-password onboarding) the whole bridge gets a third copy, since the provider-generic mechanism currently lives inside the Outlook-specific class rather than a shared AppAuth executor.
app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt:172 — OutlookAuthManager contains no AppLog logging anywhere in the OAuth lifecycle, and emailFromIdToken silently swallows all parse failures via runCatching{...}.getOrNull(), violating CLAUDE.md's DoD rule that error/fallback paths and lifecycle transitions must be logged so behaviour is diagnosable from a debug report.
Microsoft returns an id_token whose payload fails Base64 decoding, JSON parsing, or has blank email/preferred_username claims (e.g. a tenant policy strips the email claim). exchangeToken then throws the generic 'Could not read the account email from the token' and the user's onboarding fails — but the debug report has zero breadcrumbs distinguishing decode failure vs missing claim vs blank claim, and no record that a token exchange or refresh was even attempted (token exchange/refresh success and failure at lines 97-105 and 144-152 are equally unlogged in this file).
app/src/main/kotlin/org/libremail/data/security/AesGcmKeystoreCipher.kt:113 — Every encrypt/decrypt re-loads the AndroidKeyStore and re-fetches the key entry under a shared lock instead of caching the resolved SecretKey handle, so the connect-per-op IMAP hot path pays a serialized Keystore binder roundtrip per mail operation.
A PASSWORD_IMAP account performs a sync or message open: MailRepositoryImpl/MailSyncer call imapParamsFor per operation (10+ call sites), each hitting CredentialStore.loadSecret -> crypto.decrypt -> decryptionKey() -> existingKey(), which does KeyStore.getInstance + load(null) + getEntry inside synchronized(keyLock) every single time. With dozens of operations per sync, and concurrent multi-account syncs serializing on the one keyLock, this adds repeated cross-process keystore-daemon latency that a cached per-alias SecretKey (invalidated in deleteKeyEntry, as Jetpack Security's MasterKey does) would eliminate.
app/src/main/kotlin/org/libremail/data/security/AppLockGate.kt:43 — onForeground's branch order makes the documented 'KEEP the marker for the genuine return to evaluate' intent unreachable: once the stale-pass branch (now < bgAt) sets state=LOCKED, the genuine return matches 'state == LOCKED -> LOCKED' before the grace comparison ever runs, so a within-grace return still demands re-authentication and the deliberately-kept backgroundedAt marker is consumed without effect.
Sequence on one main thread: app unlocked, user backgrounds (onBackground(t0)), returns (ON_START captures foregroundAt=t1 and launches the coroutine), then immediately backgrounds again (onBackground(t2), t2 > t1) before the coroutine's suspended settings read completes. The coroutine resumes and calls gate.onForeground(t1): now(t1) < bgAt(t2) -> LOCKED, marker kept per the comment 'for the genuine return to evaluate'. User returns 2 seconds later (well within the 30 s grace): onForeground(t3) hits 'state == LockState.LOCKED -> LOCKED' at line 43 before the grace branch, so the kept marker is discarded unevaluated and the user is forced through BiometricPrompt despite never exceeding the grace window. Fail-secure, but it contradicts the class's own documented grace semantics (spurious re-auth after any quick background/foreground flicker, e.g. a notification-shade peek during a slow settings read) and leaves the keep-the-marker logic as dead code.
app/src/main/kotlin/org/libremail/data/security/CredentialStore.kt:16 — saveSecret/loadSecret run blocking Android Keystore crypto (crypto.encrypt/decrypt - keymaster binder IPC, plus possible first-use key generation with a StrongBox attempt + fallback) on the caller's dispatcher, and callers reach it from viewModelScope on Main - unlike every other Keystore call site, which explicitly offloads (#308, AppLockViewModel's defaultDispatcher).
Adding the first account on a StrongBox device: AccountRepositoryImpl.addImapAccount/addOutlookAccount (invoked from a ViewModel on Dispatchers.Main) calls credentialStore.saveSecret, which synchronously mints the master key inside the StrongBox secure element (can take hundreds of ms, on top of the per-call keymaster binder round-trip) before the Room suspend call ever switches dispatchers - main-thread jank/frozen frames during onboarding, and the same binder hop on Main for every loadSecret from UI-driven paths.
app/src/main/kotlin/org/libremail/data/security/DatabaseKeyStore.kt:71 — resolvePassphrase's SealState.NONE branch is untested in both arms: DatabaseKeyStoreTest covers only MASTER+appLockEnabled=false and AUTH, so nothing pins that a first boot with app-lock ON waits for the session (session.current() ?: session.await()) instead of minting a master-sealed passphrase, nor that NONE+appLockEnabled=false generates via passphrase().
severity: low · category: test-gap · angle: tests
The NONE arm is 'simplified' to always call passphrase() (dropping the appLockEnabled fork); all existing tests stay green. A user enabling app-lock+encryptCache on first run then gets their cache encrypted under a master-sealed, no-auth-required passphrase - the auth-bound protection is silently defeated (or, once sealWithAuth runs, the check at line 92 throws and boot crashes) - with no unit test able to catch the flip.
app/src/main/kotlin/org/libremail/data/security/DatabaseKeyStore.kt:135 — DatabaseKeyStore performs every security-critical seal lifecycle transition (sealWithAuth, sealWithMaster, resetSealedPassphrase, generateAndSealMaster, setClearPending/clearClearPending) with zero AppLog logging, violating CLAUDE.md's Definition of done: 'No app source-code change is complete without appropriate logging added at its key points — lifecycle transitions, error/fallback paths, significant state changes — so behaviour is diagnosable from a user's debug report.'
User disables app-lock in Settings; sealWithMaster() throws (e.g. authCipher.decrypt fails because the 15-second auth-validity window lapsed). The only caller (SettingsViewModel.setAppLock line 127) swallows it via runCatching { databaseKeyStore.sealWithMaster() }.isSuccess with no log either, so the user's 'can't turn off app-lock' debug report contains not a single breadcrumb — no record of the attempted auth-to-master seal exchange, no exception, nothing. The same holds for a successful exchange: if the cache later turns out stranded under the wrong seal, the report cannot show which seal transitions ever ran.
app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt:217 — onAuthError — the BiometricPrompt failure/cancel path — re-locks the gate and publishes the error to the UI but never logs it through AppLog, an unlogged error path contrary to CLAUDE.md's DoD ('error/fallback paths ... so behaviour is diagnosable from a user's debug report'); the system-generated errString (non-PII) is shown transiently on screen and then lost.
A user is stuck unable to unlock the app (biometric lockout ERROR_LOCKOUT, ERROR_HW_UNAVAILABLE, or the null-FragmentActivity fallback in AppLockGateHost line 53 which calls onAuthError(null)). Their debug report shows the foreground decision breadcrumb but no trace of any of the repeated authentication failures or their error codes/messages, making the lockout loop undiagnosable — contrast onAuthenticated, whose every outcome is logged.
app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt:243 — unlockOrArm's runCatching around databaseKeyStore.sealWithAuth() swallows CancellationException and maps it to UnlockResult.RETRY, inconsistent with unwrapSealedPassphrase directly below which explicitly rethrows CancellationException.
The gate's host Activity finishes (viewModelScope cancelled) while the first-time arm is suspended inside sealWithAuth's DataStore edit: the CancellationException is caught by runCatching, logged as a spurious 'arming auth seal failed' (a scrubbed CancellationException polluting user debug reports), and returned as RETRY; only the enclosing withContext boundary rescues cooperative cancellation. Any future refactor that removes the withContext hop (e.g. calling unlockOrArm directly) would make this coroutine uncancellable-in-effect and publish a bogus lock-screen error during teardown.
app/src/main/kotlin/org/libremail/ui/security/CacheEncryptionGate.kt:171 — EphemeralReportReviewScreen copy-pastes three blocks from ReportReviewScreen.kt instead of sharing them: the CreateDocument("application/json") save-launcher (CacheEncryptionGate.kt:171-185 vs ReportReviewScreen.kt:72-88), the monospace SelectionContainer payload box (CacheEncryptionGate.kt:227-240 duplicates the existing private PayloadBox composable at ReportReviewScreen.kt:277-292), and the Copy/Save TextButton row (CacheEncryptionGate.kt:242-261 vs ReportReviewScreen.kt:190-209); PayloadBox and a shared save-launcher helper should be made internal in ui/reporting and reused.
severity: low · category: reuse · angle: reuse
Both save-launcher copies already share a latent quirk: runCatching swallows the openOutputStream/write failure and the R.string.report_saved snackbar is shown even when the file was never written. Whoever fixes that (or changes the export filename "libremail-report.json", MIME type, or payload styling) in ReportReviewScreen will almost certainly miss the second copy buried in the cache-encryption error gate, leaving the two report-export flows silently divergent.
app/src/main/kotlin/org/libremail/ui/security/CacheEncryptionGate.kt:182 — The diagnostic-report Save path swallows write failures (runCatching around openOutputStream, null stream ignored) and then unconditionally shows the 'Report saved' snackbar, so a failed export is reported as success.
On the fail-closed encryption error screen the user taps Save and picks a destination; contentResolver.openOutputStream throws (SAF provider gone, storage full, document revoked) or returns null. runCatching discards the error and no branch inspects the result, yet snackbarHostState.showSnackbar(savedMessage) still runs — the user is told the ephemeral report (which exists nowhere else, deliberately never persisted) was saved, dismisses the screen, and the diagnostic data is silently lost with no retry cue.
Source: whole-repo multi-agent review, 2026-07-09 (run `wf_b41de68c-e85`). The run was cut short by usage limits before its verification pass, so every finding below is an **unverified finder candidate** — validate each against the current code before implementing. Findings are listed medium first, then low. Critical/high candidates from the same run were verified separately and have their own issues.
## Medium (5)
### `app/src/main/kotlin/org/libremail/data/security/DatabaseKeyCipher.kt:57` — DatabaseKeyCipher has no test at all - no JVM unit test exists and the only instrumented security test (KeystoreCryptoTest) covers the master key, despite the KDoc claiming on-device coverage - leaving its isInvalidated() exception classification (KeyPermanentlyInvalidated->true, UserNotAuthenticated->false, unknown->false) and the encrypt() invalidated-key self-heal path entirely unexercised, even though the classification is pure control flow testable via the same seams AesGcmKeystoreCipherTest already uses.
- severity: **medium** · category: `test-gap` · angle: `tests`
> A refactor accidentally lets UserNotAuthenticatedException fall into the KeyPermanentlyInvalidatedException/generic-true path (e.g. reordering catch clauses or collapsing them); no test fails. On devices with encryptCache+appLock on, every routine pre-auth foreground pass then reports keyInvalidated=true, KeyInvalidationPolicy resolves CLEAR_AND_REQUIRE_AUTH, and the app silently wipes the user's entire cached mail and restarts - repeatedly - with the regression only discoverable on a physical device.
### `app/src/main/kotlin/org/libremail/ui/lock/AppLockGateHost.kt:100` — The auto-prompt LaunchedEffect races the BiometricPrompt success callback on device-credential re-entry: an ON_START-driven onForeground pass can republish Locked (gate is still LOCKED) before onAuthenticationSucceeded lands, launching a second prompt whose cancellation then re-locks a user who just authenticated.
- severity: **medium** · category: `concurrency` · angle: `concurrency`
> API 29 device-credential fallback: tapping Unlock launches the system ConfirmDeviceCredential activity, which stops MainActivity -> onBackground() sets state to Checking. User enters the correct PIN; on return, ON_START fires onForeground() (async: settings.first() + withContext probe) while the prompt's success callback is delivered separately on the main executor. If the onForeground decision lands first, gate.state is still LOCKED so publish() emits Locked, the composition re-enters the Locked branch from Checking, and LaunchedEffect(Unit) auto-presents a SECOND BiometricPrompt. Moments later onAuthenticated() unlocks and composes the app, but the stale system prompt sheet is still showing (nothing dismisses it - the effect has no cancellation cleanup); the user dismisses it -> onAuthenticationError -> viewModel.onAuthError -> gate.lock() -> the just-unlocked app re-locks and demands auth again.
### `app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt:240` — The auth seal is never cleared after the on-disk DB has been decrypted back to plaintext, so unlockOrArm keeps gating on hasAuthSealedPassphrase() forever; a later biometric re-enrollment then classifies the stale seal as UNRECOVERABLE and clearCacheAndRestart wipes a plaintext, fully readable cache and force-restarts the app.
- severity: **medium** · category: `correctness` · angle: `invariants`
> State: appLock ON, encryptCache toggled OFF; at the next cold start DatabaseProvisioner.ensurePlaintext converts the DB to plaintext using the unwrapped seal — but SEALED_AUTH is left in the DataStore permanently (nothing removes it after conversion; only sealWithMaster on app-lock disable or resetSealedPassphrase on wipe touch it). Months later the user enrolls a new fingerprint, permanently invalidating the auth-bound key. Next unlock: decide() -> REQUIRE_AUTH (encryptCache off, so not CLEAR_AND_REQUIRE_AUTH), the prompt succeeds, then unlockOrArm sees hasAuthSealedPassphrase()==true -> unwrapSealedPassphrase -> KeyPermanentlyInvalidatedException -> UNRECOVERABLE -> clearCacheAndRestart(disableAppLock=false): the entire (plaintext, perfectly openable) mail cache is deleted and the process is killed mid-use, forcing a full re-sync — for a seal that no longer protects anything. Fix direction: drop SEALED_AUTH (or reseal-with-master) once the decrypt-to-plaintext conversion completes, or have unwrapSealedPassphrase treat invalidation as recoverable when the on-disk DB is not encrypted.
### `app/src/main/kotlin/org/libremail/ui/security/CacheEncryptionGate.kt:70` — The CacheEncryptionGate host composable's fail-closed wiring is untested: CacheEncryptionErrorScreenTest covers only the inner presentational screen, and no Robolectric host test (the analogue of AppLockGateHostJvmTest) verifies that content() is NOT composed during Checking or Unavailable, that Ready composes the app, or the error->prepareReport->EphemeralReportReviewScreen->dismissReport flow.
- severity: **medium** · category: `test-gap` · angle: `tests`
> A refactor of the when(state) branches composes content() behind the cover during Checking (mirroring AppLockGateHost's keep-composed pattern) or falls through to content() on Unavailable; no test fails, and DB-backed screens now compose before the encryption probe resolves - the exact fail-open condition issue #359's gate exists to prevent, shipping silently because the gate's only test never mounts the gate itself.
### `app/src/test/kotlin/org/libremail/auth/OutlookAuthManagerTest.kt:249` — The token-exchange tests stub performTokenRequest(any(), any()) without ever capturing or asserting the TokenRequest, so the scope and nonce parameters of the Microsoft code exchange - the exact two fields whose omission previously broke all Outlook sign-in (AADSTS70011 'must include a scope input parameter' and AppAuth id_token nonce validation) - are completely unpinned, as are the per-resource scopes of the two refresh paths.
- severity: **medium** · category: `test-gap` · angle: `tests`
> A refactor deletes .setScope(...) or .setNonce(response.request.nonce) from OutlookAuthManager.exchangeToken (or swaps GRAPH_SCOPE/OUTLOOK_SCOPE in refreshForScope - the 'graph-at'/'exchange-at' assertions only echo canned responses); every unit test still passes, the change merges, and every new Microsoft sign-in (or every Graph send / IMAP token refresh) fails in production against the real token endpoint - a regression of the historical Outlook-login-breaking bug with no test able to catch it.
## Low (12)
### `app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt:137` — refreshForScope constructs a new AppAuth AuthorizationService per token refresh, whose constructor (AppAuth 0.11.1) runs a PackageManager browser query and binds the browser's CustomTabsService — needless IPC for a pure token-endpoint HTTP POST that never opens a browser.
- severity: **low** · category: `efficiency` · angle: `efficiency`
> An Outlook account is active: access tokens expire roughly hourly per resource, so freshOutlookToken/freshGraphToken each run BrowserSelector.select (PM query over all installed browsers) plus a bind/unbind of Chrome's CustomTabsService about twice per hour per account for the process lifetime, purely to make one HTTPS POST. A single long-lived AuthorizationService reused for token requests (or an AppAuthConfiguration whose browser matcher skips the custom-tab warmup) does the same work with zero browser-service churn; only createAuthIntent actually needs the custom-tab binding.
### `app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt:144` — The AppAuth token-request plumbing — construct AuthorizationService, bridge performTokenRequest through suspendCancellableCoroutine with the token/error-or-IllegalStateException mapping, dispose in finally — is copy-pasted between exchangeToken (lines 95-105, 120-122) and refreshForScope (lines 137-152, 159-161) instead of living in one private suspend helper.
- severity: **low** · category: `reuse` · angle: `reuse` · flagged by 2 finder(s)
> Any fix to this plumbing (e.g. adding continuation.invokeOnCancellation to cancel/dispose the in-flight request when the caller's coroutine is cancelled, or improving the error mapping) must be applied twice and can drift; when the next AppAuth provider arrives (e.g. restoring the Gmail OAuth path alongside the app-password onboarding) the whole bridge gets a third copy, since the provider-generic mechanism currently lives inside the Outlook-specific class rather than a shared AppAuth executor.
### `app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt:172` — OutlookAuthManager contains no AppLog logging anywhere in the OAuth lifecycle, and emailFromIdToken silently swallows all parse failures via runCatching{...}.getOrNull(), violating CLAUDE.md's DoD rule that error/fallback paths and lifecycle transitions must be logged so behaviour is diagnosable from a debug report.
- severity: **low** · category: `conventions` · angle: `conventions`
> Microsoft returns an id_token whose payload fails Base64 decoding, JSON parsing, or has blank email/preferred_username claims (e.g. a tenant policy strips the email claim). exchangeToken then throws the generic 'Could not read the account email from the token' and the user's onboarding fails — but the debug report has zero breadcrumbs distinguishing decode failure vs missing claim vs blank claim, and no record that a token exchange or refresh was even attempted (token exchange/refresh success and failure at lines 97-105 and 144-152 are equally unlogged in this file).
### `app/src/main/kotlin/org/libremail/data/security/AesGcmKeystoreCipher.kt:113` — Every encrypt/decrypt re-loads the AndroidKeyStore and re-fetches the key entry under a shared lock instead of caching the resolved SecretKey handle, so the connect-per-op IMAP hot path pays a serialized Keystore binder roundtrip per mail operation.
- severity: **low** · category: `efficiency` · angle: `efficiency`
> A PASSWORD_IMAP account performs a sync or message open: MailRepositoryImpl/MailSyncer call imapParamsFor per operation (10+ call sites), each hitting CredentialStore.loadSecret -> crypto.decrypt -> decryptionKey() -> existingKey(), which does KeyStore.getInstance + load(null) + getEntry inside synchronized(keyLock) every single time. With dozens of operations per sync, and concurrent multi-account syncs serializing on the one keyLock, this adds repeated cross-process keystore-daemon latency that a cached per-alias SecretKey (invalidated in deleteKeyEntry, as Jetpack Security's MasterKey does) would eliminate.
### `app/src/main/kotlin/org/libremail/data/security/AppLockGate.kt:43` — onForeground's branch order makes the documented 'KEEP the marker for the genuine return to evaluate' intent unreachable: once the stale-pass branch (now < bgAt) sets state=LOCKED, the genuine return matches 'state == LOCKED -> LOCKED' before the grace comparison ever runs, so a within-grace return still demands re-authentication and the deliberately-kept backgroundedAt marker is consumed without effect.
- severity: **low** · category: `correctness` · angle: `invariants`
> Sequence on one main thread: app unlocked, user backgrounds (onBackground(t0)), returns (ON_START captures foregroundAt=t1 and launches the coroutine), then immediately backgrounds again (onBackground(t2), t2 > t1) before the coroutine's suspended settings read completes. The coroutine resumes and calls gate.onForeground(t1): now(t1) < bgAt(t2) -> LOCKED, marker kept per the comment 'for the genuine return to evaluate'. User returns 2 seconds later (well within the 30 s grace): onForeground(t3) hits 'state == LockState.LOCKED -> LOCKED' at line 43 before the grace branch, so the kept marker is discarded unevaluated and the user is forced through BiometricPrompt despite never exceeding the grace window. Fail-secure, but it contradicts the class's own documented grace semantics (spurious re-auth after any quick background/foreground flicker, e.g. a notification-shade peek during a slow settings read) and leaves the keep-the-marker logic as dead code.
### `app/src/main/kotlin/org/libremail/data/security/CredentialStore.kt:16` — saveSecret/loadSecret run blocking Android Keystore crypto (crypto.encrypt/decrypt - keymaster binder IPC, plus possible first-use key generation with a StrongBox attempt + fallback) on the caller's dispatcher, and callers reach it from viewModelScope on Main - unlike every other Keystore call site, which explicitly offloads (#308, AppLockViewModel's defaultDispatcher).
- severity: **low** · category: `concurrency` · angle: `concurrency`
> Adding the first account on a StrongBox device: AccountRepositoryImpl.addImapAccount/addOutlookAccount (invoked from a ViewModel on Dispatchers.Main) calls credentialStore.saveSecret, which synchronously mints the master key inside the StrongBox secure element (can take hundreds of ms, on top of the per-call keymaster binder round-trip) before the Room suspend call ever switches dispatchers - main-thread jank/frozen frames during onboarding, and the same binder hop on Main for every loadSecret from UI-driven paths.
### `app/src/main/kotlin/org/libremail/data/security/DatabaseKeyStore.kt:71` — resolvePassphrase's SealState.NONE branch is untested in both arms: DatabaseKeyStoreTest covers only MASTER+appLockEnabled=false and AUTH, so nothing pins that a first boot with app-lock ON waits for the session (session.current() ?: session.await()) instead of minting a master-sealed passphrase, nor that NONE+appLockEnabled=false generates via passphrase().
- severity: **low** · category: `test-gap` · angle: `tests`
> The NONE arm is 'simplified' to always call passphrase() (dropping the appLockEnabled fork); all existing tests stay green. A user enabling app-lock+encryptCache on first run then gets their cache encrypted under a master-sealed, no-auth-required passphrase - the auth-bound protection is silently defeated (or, once sealWithAuth runs, the check at line 92 throws and boot crashes) - with no unit test able to catch the flip.
### `app/src/main/kotlin/org/libremail/data/security/DatabaseKeyStore.kt:135` — DatabaseKeyStore performs every security-critical seal lifecycle transition (sealWithAuth, sealWithMaster, resetSealedPassphrase, generateAndSealMaster, setClearPending/clearClearPending) with zero AppLog logging, violating CLAUDE.md's Definition of done: 'No app source-code change is complete without appropriate logging added at its key points — lifecycle transitions, error/fallback paths, significant state changes — so behaviour is diagnosable from a user's debug report.'
- severity: **low** · category: `conventions` · angle: `conventions`
> User disables app-lock in Settings; sealWithMaster() throws (e.g. authCipher.decrypt fails because the 15-second auth-validity window lapsed). The only caller (SettingsViewModel.setAppLock line 127) swallows it via `runCatching { databaseKeyStore.sealWithMaster() }.isSuccess` with no log either, so the user's 'can't turn off app-lock' debug report contains not a single breadcrumb — no record of the attempted auth-to-master seal exchange, no exception, nothing. The same holds for a successful exchange: if the cache later turns out stranded under the wrong seal, the report cannot show which seal transitions ever ran.
### `app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt:217` — onAuthError — the BiometricPrompt failure/cancel path — re-locks the gate and publishes the error to the UI but never logs it through AppLog, an unlogged error path contrary to CLAUDE.md's DoD ('error/fallback paths ... so behaviour is diagnosable from a user's debug report'); the system-generated errString (non-PII) is shown transiently on screen and then lost.
- severity: **low** · category: `conventions` · angle: `conventions`
> A user is stuck unable to unlock the app (biometric lockout ERROR_LOCKOUT, ERROR_HW_UNAVAILABLE, or the null-FragmentActivity fallback in AppLockGateHost line 53 which calls onAuthError(null)). Their debug report shows the foreground decision breadcrumb but no trace of any of the repeated authentication failures or their error codes/messages, making the lockout loop undiagnosable — contrast onAuthenticated, whose every outcome is logged.
### `app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt:243` — unlockOrArm's runCatching around databaseKeyStore.sealWithAuth() swallows CancellationException and maps it to UnlockResult.RETRY, inconsistent with unwrapSealedPassphrase directly below which explicitly rethrows CancellationException.
- severity: **low** · category: `concurrency` · angle: `concurrency`
> The gate's host Activity finishes (viewModelScope cancelled) while the first-time arm is suspended inside sealWithAuth's DataStore edit: the CancellationException is caught by runCatching, logged as a spurious 'arming auth seal failed' (a scrubbed CancellationException polluting user debug reports), and returned as RETRY; only the enclosing withContext boundary rescues cooperative cancellation. Any future refactor that removes the withContext hop (e.g. calling unlockOrArm directly) would make this coroutine uncancellable-in-effect and publish a bogus lock-screen error during teardown.
### `app/src/main/kotlin/org/libremail/ui/security/CacheEncryptionGate.kt:171` — EphemeralReportReviewScreen copy-pastes three blocks from ReportReviewScreen.kt instead of sharing them: the CreateDocument("application/json") save-launcher (CacheEncryptionGate.kt:171-185 vs ReportReviewScreen.kt:72-88), the monospace SelectionContainer payload box (CacheEncryptionGate.kt:227-240 duplicates the existing private PayloadBox composable at ReportReviewScreen.kt:277-292), and the Copy/Save TextButton row (CacheEncryptionGate.kt:242-261 vs ReportReviewScreen.kt:190-209); PayloadBox and a shared save-launcher helper should be made internal in ui/reporting and reused.
- severity: **low** · category: `reuse` · angle: `reuse`
> Both save-launcher copies already share a latent quirk: runCatching swallows the openOutputStream/write failure and the R.string.report_saved snackbar is shown even when the file was never written. Whoever fixes that (or changes the export filename "libremail-report.json", MIME type, or payload styling) in ReportReviewScreen will almost certainly miss the second copy buried in the cache-encryption error gate, leaving the two report-export flows silently divergent.
### `app/src/main/kotlin/org/libremail/ui/security/CacheEncryptionGate.kt:182` — The diagnostic-report Save path swallows write failures (runCatching around openOutputStream, null stream ignored) and then unconditionally shows the 'Report saved' snackbar, so a failed export is reported as success.
- severity: **low** · category: `swallowed-exception` · angle: `logic`
> On the fail-closed encryption error screen the user taps Save and picks a destination; contentResolver.openOutputStream throws (SAF provider gone, storage full, document revoked) or returns null. runCatching discards the error and no branch inspects the result, yet snackbarHostState.showSnackbar(savedMessage) still runs — the user is told the ephemeral report (which exists nowhere else, deliberately never persisted) was saved, dismisses the screen, and the diagnostic data is silently lost with no retry cue.
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.
Source: whole-repo multi-agent review, 2026-07-09 (run
wf_b41de68c-e85). The run was cut short by usage limits before its verification pass, so every finding below is an unverified finder candidate — validate each against the current code before implementing. Findings are listed medium first, then low. Critical/high candidates from the same run were verified separately and have their own issues.Medium (5)
app/src/main/kotlin/org/libremail/data/security/DatabaseKeyCipher.kt:57— DatabaseKeyCipher has no test at all - no JVM unit test exists and the only instrumented security test (KeystoreCryptoTest) covers the master key, despite the KDoc claiming on-device coverage - leaving its isInvalidated() exception classification (KeyPermanentlyInvalidated->true, UserNotAuthenticated->false, unknown->false) and the encrypt() invalidated-key self-heal path entirely unexercised, even though the classification is pure control flow testable via the same seams AesGcmKeystoreCipherTest already uses.test-gap· angle:testsapp/src/main/kotlin/org/libremail/ui/lock/AppLockGateHost.kt:100— The auto-prompt LaunchedEffect races the BiometricPrompt success callback on device-credential re-entry: an ON_START-driven onForeground pass can republish Locked (gate is still LOCKED) before onAuthenticationSucceeded lands, launching a second prompt whose cancellation then re-locks a user who just authenticated.concurrency· angle:concurrencyapp/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt:240— The auth seal is never cleared after the on-disk DB has been decrypted back to plaintext, so unlockOrArm keeps gating on hasAuthSealedPassphrase() forever; a later biometric re-enrollment then classifies the stale seal as UNRECOVERABLE and clearCacheAndRestart wipes a plaintext, fully readable cache and force-restarts the app.correctness· angle:invariantsapp/src/main/kotlin/org/libremail/ui/security/CacheEncryptionGate.kt:70— The CacheEncryptionGate host composable's fail-closed wiring is untested: CacheEncryptionErrorScreenTest covers only the inner presentational screen, and no Robolectric host test (the analogue of AppLockGateHostJvmTest) verifies that content() is NOT composed during Checking or Unavailable, that Ready composes the app, or the error->prepareReport->EphemeralReportReviewScreen->dismissReport flow.test-gap· angle:testsapp/src/test/kotlin/org/libremail/auth/OutlookAuthManagerTest.kt:249— The token-exchange tests stub performTokenRequest(any(), any()) without ever capturing or asserting the TokenRequest, so the scope and nonce parameters of the Microsoft code exchange - the exact two fields whose omission previously broke all Outlook sign-in (AADSTS70011 'must include a scope input parameter' and AppAuth id_token nonce validation) - are completely unpinned, as are the per-resource scopes of the two refresh paths.test-gap· angle:testsLow (12)
app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt:137— refreshForScope constructs a new AppAuth AuthorizationService per token refresh, whose constructor (AppAuth 0.11.1) runs a PackageManager browser query and binds the browser's CustomTabsService — needless IPC for a pure token-endpoint HTTP POST that never opens a browser.efficiency· angle:efficiencyapp/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt:144— The AppAuth token-request plumbing — construct AuthorizationService, bridge performTokenRequest through suspendCancellableCoroutine with the token/error-or-IllegalStateException mapping, dispose in finally — is copy-pasted between exchangeToken (lines 95-105, 120-122) and refreshForScope (lines 137-152, 159-161) instead of living in one private suspend helper.reuse· angle:reuse· flagged by 2 finder(s)app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt:172— OutlookAuthManager contains no AppLog logging anywhere in the OAuth lifecycle, and emailFromIdToken silently swallows all parse failures via runCatching{...}.getOrNull(), violating CLAUDE.md's DoD rule that error/fallback paths and lifecycle transitions must be logged so behaviour is diagnosable from a debug report.conventions· angle:conventionsapp/src/main/kotlin/org/libremail/data/security/AesGcmKeystoreCipher.kt:113— Every encrypt/decrypt re-loads the AndroidKeyStore and re-fetches the key entry under a shared lock instead of caching the resolved SecretKey handle, so the connect-per-op IMAP hot path pays a serialized Keystore binder roundtrip per mail operation.efficiency· angle:efficiencyapp/src/main/kotlin/org/libremail/data/security/AppLockGate.kt:43— onForeground's branch order makes the documented 'KEEP the marker for the genuine return to evaluate' intent unreachable: once the stale-pass branch (now < bgAt) sets state=LOCKED, the genuine return matches 'state == LOCKED -> LOCKED' before the grace comparison ever runs, so a within-grace return still demands re-authentication and the deliberately-kept backgroundedAt marker is consumed without effect.correctness· angle:invariantsapp/src/main/kotlin/org/libremail/data/security/CredentialStore.kt:16— saveSecret/loadSecret run blocking Android Keystore crypto (crypto.encrypt/decrypt - keymaster binder IPC, plus possible first-use key generation with a StrongBox attempt + fallback) on the caller's dispatcher, and callers reach it from viewModelScope on Main - unlike every other Keystore call site, which explicitly offloads (#308, AppLockViewModel's defaultDispatcher).concurrency· angle:concurrencyapp/src/main/kotlin/org/libremail/data/security/DatabaseKeyStore.kt:71— resolvePassphrase's SealState.NONE branch is untested in both arms: DatabaseKeyStoreTest covers only MASTER+appLockEnabled=false and AUTH, so nothing pins that a first boot with app-lock ON waits for the session (session.current() ?: session.await()) instead of minting a master-sealed passphrase, nor that NONE+appLockEnabled=false generates via passphrase().test-gap· angle:testsapp/src/main/kotlin/org/libremail/data/security/DatabaseKeyStore.kt:135— DatabaseKeyStore performs every security-critical seal lifecycle transition (sealWithAuth, sealWithMaster, resetSealedPassphrase, generateAndSealMaster, setClearPending/clearClearPending) with zero AppLog logging, violating CLAUDE.md's Definition of done: 'No app source-code change is complete without appropriate logging added at its key points — lifecycle transitions, error/fallback paths, significant state changes — so behaviour is diagnosable from a user's debug report.'conventions· angle:conventionsapp/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt:217— onAuthError — the BiometricPrompt failure/cancel path — re-locks the gate and publishes the error to the UI but never logs it through AppLog, an unlogged error path contrary to CLAUDE.md's DoD ('error/fallback paths ... so behaviour is diagnosable from a user's debug report'); the system-generated errString (non-PII) is shown transiently on screen and then lost.conventions· angle:conventionsapp/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt:243— unlockOrArm's runCatching around databaseKeyStore.sealWithAuth() swallows CancellationException and maps it to UnlockResult.RETRY, inconsistent with unwrapSealedPassphrase directly below which explicitly rethrows CancellationException.concurrency· angle:concurrencyapp/src/main/kotlin/org/libremail/ui/security/CacheEncryptionGate.kt:171— EphemeralReportReviewScreen copy-pastes three blocks from ReportReviewScreen.kt instead of sharing them: the CreateDocument("application/json") save-launcher (CacheEncryptionGate.kt:171-185 vs ReportReviewScreen.kt:72-88), the monospace SelectionContainer payload box (CacheEncryptionGate.kt:227-240 duplicates the existing private PayloadBox composable at ReportReviewScreen.kt:277-292), and the Copy/Save TextButton row (CacheEncryptionGate.kt:242-261 vs ReportReviewScreen.kt:190-209); PayloadBox and a shared save-launcher helper should be made internal in ui/reporting and reused.reuse· angle:reuseapp/src/main/kotlin/org/libremail/ui/security/CacheEncryptionGate.kt:182— The diagnostic-report Save path swallows write failures (runCatching around openOutputStream, null stream ignored) and then unconditionally shows the 'Report saved' snackbar, so a failed export is reported as success.swallowed-exception· angle:logic