From 1f0eed25e9fd7b68670b2f8828c3f07c487f03a3 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Mon, 20 Apr 2026 09:47:13 -0300 Subject: [PATCH 1/5] chore: surface cause of keychain load failures in logs --- .../bitkit/data/keychain/AndroidKeyStore.kt | 2 + .../java/to/bitkit/data/keychain/Keychain.kt | 82 ++++++++++++---- .../bitkit/data/keychain/KeychainErrorTest.kt | 96 +++++++++++++++++++ .../to/bitkit/repositories/WalletRepoTest.kt | 11 +++ 4 files changed, 175 insertions(+), 16 deletions(-) create mode 100644 app/src/test/java/to/bitkit/data/keychain/KeychainErrorTest.kt diff --git a/app/src/main/java/to/bitkit/data/keychain/AndroidKeyStore.kt b/app/src/main/java/to/bitkit/data/keychain/AndroidKeyStore.kt index 3391df16e2..f7cfff3803 100644 --- a/app/src/main/java/to/bitkit/data/keychain/AndroidKeyStore.kt +++ b/app/src/main/java/to/bitkit/data/keychain/AndroidKeyStore.kt @@ -87,4 +87,6 @@ class AndroidKeyStore( } generateKey() } + + fun containsAlias(): Boolean = keyStore.containsAlias(alias) } diff --git a/app/src/main/java/to/bitkit/data/keychain/Keychain.kt b/app/src/main/java/to/bitkit/data/keychain/Keychain.kt index 9b777174a3..6685da2652 100644 --- a/app/src/main/java/to/bitkit/data/keychain/Keychain.kt +++ b/app/src/main/java/to/bitkit/data/keychain/Keychain.kt @@ -19,8 +19,11 @@ import to.bitkit.ext.fromBase64 import to.bitkit.ext.toBase64 import to.bitkit.utils.AppError import to.bitkit.utils.Logger +import java.util.Collections +import java.util.concurrent.ConcurrentHashMap import javax.inject.Inject import javax.inject.Singleton +import kotlin.coroutines.cancellation.CancellationException private val Context.keychainDataStore: DataStore by preferencesDataStore( name = "keychain" @@ -32,59 +35,76 @@ class Keychain @Inject constructor( @ApplicationContext private val context: Context, @IoDispatcher private val dispatcher: CoroutineDispatcher, ) : BaseCoroutineScope(dispatcher) { + companion object { + private const val TAG = "Keychain" + private const val CAUSE_CHAIN_DEPTH = 4 + } + private val keyStore by lazy { AndroidKeyStore(alias = "keychain") } @Suppress("MemberNameEqualsClassName") private val keychain = context.keychainDataStore + private val loadDiagnosticsEmitted: MutableSet = + Collections.newSetFromMap(ConcurrentHashMap()) + val snapshot get() = runBlocking(this.coroutineContext) { keychain.data.first() } fun loadString(key: String): String? = load(key)?.decodeToString() - @Suppress("TooGenericExceptionCaught", "SwallowedException") + @Suppress("TooGenericExceptionCaught") fun load(key: String): ByteArray? { try { return snapshot[key.indexed]?.fromBase64()?.let { keyStore.decrypt(it) } - } catch (_: Exception) { - throw KeychainError.FailedToLoad(key) + } catch (c: CancellationException) { + throw c + } catch (t: Throwable) { + emitLoadDiagnosticsOnce(key, t) + throw KeychainError.FailedToLoad(key, cause = t) } } suspend fun saveString(key: String, value: String) = save(key, value.toByteArray()) - @Suppress("TooGenericExceptionCaught", "SwallowedException") + @Suppress("TooGenericExceptionCaught", "ThrowsCount") suspend fun save(key: String, value: ByteArray) { if (exists(key)) throw KeychainError.FailedToSaveAlreadyExists(key) try { val encryptedValue = keyStore.encrypt(value) keychain.edit { it[key.indexed] = encryptedValue.toBase64() } - } catch (_: Exception) { - throw KeychainError.FailedToSave(key) + } catch (c: CancellationException) { + throw c + } catch (t: Throwable) { + throw KeychainError.FailedToSave(key, cause = t) } Logger.info("Saved to keychain: $key") } /** Inserts or replaces a string value associated with a given key in the keychain. */ - @Suppress("TooGenericExceptionCaught", "SwallowedException") + @Suppress("TooGenericExceptionCaught") suspend fun upsertString(key: String, value: String) { try { val encryptedValue = keyStore.encrypt(value.toByteArray()) keychain.edit { it[key.indexed] = encryptedValue.toBase64() } - } catch (_: Exception) { - throw KeychainError.FailedToSave(key) + } catch (c: CancellationException) { + throw c + } catch (t: Throwable) { + throw KeychainError.FailedToSave(key, cause = t) } Logger.info("Upsert in keychain: $key") } - @Suppress("TooGenericExceptionCaught", "SwallowedException") + @Suppress("TooGenericExceptionCaught") suspend fun delete(key: String) { try { keychain.edit { it.remove(key.indexed) } - } catch (_: Exception) { - throw KeychainError.FailedToDelete(key) + } catch (c: CancellationException) { + throw c + } catch (t: Throwable) { + throw KeychainError.FailedToDelete(key, cause = t) } Logger.debug("Deleted from keychain: $key") } @@ -120,6 +140,27 @@ class Keychain @Inject constructor( .map { string -> string?.toIntOrNull() } } + private fun emitLoadDiagnosticsOnce(key: String, cause: Throwable) { + if (!loadDiagnosticsEmitted.add(key)) return + + val aliasPresent = runCatching { keyStore.containsAlias() }.getOrDefault(false) + val entryPresent = runCatching { snapshot.contains(key.indexed) }.getOrDefault(false) + val walletIndex = runCatching { + runBlocking { db.configDao().getAll().first() }.firstOrNull()?.walletIndex ?: 0L + }.getOrDefault(-1L) + val causeChain = generateSequence(cause) { it.cause } + .take(CAUSE_CHAIN_DEPTH) + .map { it.javaClass.simpleName } + .joinToString(separator = " <- ") + + Logger.warn( + "Decrypt failed for key='$key' walletIndex='$walletIndex' " + + "aliasPresent='$aliasPresent' entryPresent='$entryPresent' " + + "causeChain='$causeChain'", + context = TAG, + ) + } + enum class Key { PUSH_NOTIFICATION_TOKEN, PUSH_NOTIFICATION_PRIVATE_KEY, @@ -130,10 +171,19 @@ class Keychain @Inject constructor( } } -sealed class KeychainError(message: String) : AppError(message) { - class FailedToDelete(key: String) : KeychainError("Failed to delete $key from keychain.") - class FailedToLoad(key: String) : KeychainError("Failed to load $key from keychain.") - class FailedToSave(key: String) : KeychainError("Failed to save to $key keychain.") +sealed class KeychainError(message: String, cause: Throwable? = null) : AppError(message, cause) { + class FailedToDelete(key: String, cause: Throwable? = null) : + KeychainError("Failed to delete $key from keychain${cause.causeSuffix()}", cause) + + class FailedToLoad(val key: String, cause: Throwable? = null) : + KeychainError("Failed to load $key from keychain${cause.causeSuffix()}", cause) + + class FailedToSave(key: String, cause: Throwable? = null) : + KeychainError("Failed to save to $key keychain${cause.causeSuffix()}", cause) + class FailedToSaveAlreadyExists(key: String) : KeychainError("Key $key already exists in keychain. Explicitly delete key before attempting to update value.") } + +private fun Throwable?.causeSuffix(): String = + this?.let { " (cause='${it.javaClass.simpleName}')" } ?: "." diff --git a/app/src/test/java/to/bitkit/data/keychain/KeychainErrorTest.kt b/app/src/test/java/to/bitkit/data/keychain/KeychainErrorTest.kt new file mode 100644 index 0000000000..f365aef06f --- /dev/null +++ b/app/src/test/java/to/bitkit/data/keychain/KeychainErrorTest.kt @@ -0,0 +1,96 @@ +package to.bitkit.data.keychain + +import org.junit.Test +import java.io.IOException +import javax.crypto.AEADBadTagException +import javax.crypto.BadPaddingException +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertSame + +class KeychainErrorTest { + + @Test + fun `FailedToLoad without cause renders key and terminating period`() { + val error = KeychainError.FailedToLoad(key = "BIP39_MNEMONIC") + + assertEquals("Failed to load BIP39_MNEMONIC from keychain.", error.message) + assertNull(error.cause) + } + + @Test + fun `FailedToLoad preserves cause and embeds its simple class name`() { + val cause = AEADBadTagException("leak-probe-9f3a") + val error = KeychainError.FailedToLoad(key = "BIP39_MNEMONIC", cause = cause) + + assertSame(cause, error.cause) + assertEquals( + "Failed to load BIP39_MNEMONIC from keychain (cause='AEADBadTagException')", + error.message, + ) + } + + @Test + fun `FailedToLoad does not leak underlying cause message`() { + val probe = "leak-probe-9f3a" + val causes = listOf( + AEADBadTagException(probe), + BadPaddingException(probe), + ClassCastException(probe), + IllegalArgumentException(probe), + IOException(probe), + ) + + causes.forEach { cause -> + val error = KeychainError.FailedToLoad(key = "BIP39_MNEMONIC", cause = cause) + assertFalse( + actual = error.message.orEmpty().contains(probe), + message = "message leaked cause.message for ${cause.javaClass.simpleName}", + ) + } + } + + @Test + fun `FailedToLoad exposes the failing key`() { + val error = KeychainError.FailedToLoad(key = "BIP39_PASSPHRASE", cause = IOException()) + + assertEquals("BIP39_PASSPHRASE", error.key) + } + + @Test + fun `FailedToSave preserves cause and renders simple class name`() { + val cause = IOException("probe") + val error = KeychainError.FailedToSave(key = "BIP39_MNEMONIC", cause = cause) + + assertSame(cause, error.cause) + assertEquals( + "Failed to save to BIP39_MNEMONIC keychain (cause='IOException')", + error.message, + ) + } + + @Test + fun `FailedToDelete preserves cause and renders simple class name`() { + val cause = IOException("probe") + val error = KeychainError.FailedToDelete(key = "BIP39_MNEMONIC", cause = cause) + + assertSame(cause, error.cause) + assertEquals( + "Failed to delete BIP39_MNEMONIC from keychain (cause='IOException')", + error.message, + ) + } + + @Test + fun `FailedToSaveAlreadyExists has no cause and static message`() { + val error = KeychainError.FailedToSaveAlreadyExists(key = "BIP39_MNEMONIC") + + assertNull(error.cause) + assertEquals( + "Key BIP39_MNEMONIC already exists in keychain. " + + "Explicitly delete key before attempting to update value.", + error.message, + ) + } +} diff --git a/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt b/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt index 63625d275d..4140a80b63 100644 --- a/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt @@ -155,6 +155,17 @@ class WalletRepoTest : BaseUnitTest() { assertFalse(result) } + @Test + fun `walletExists relies on exists and does not touch load paths`() = test { + whenever(keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(true) + + val result = sut.walletExists() + + assertTrue(result) + verify(keychain, never()).loadString(Keychain.Key.BIP39_MNEMONIC.name) + verify(keychain, never()).load(Keychain.Key.BIP39_MNEMONIC.name) + } + @Test fun `setWalletExistsState should update walletState with current existence status`() = test { whenever(keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(true) From 89aea3a247a3901a45bf66a1721dac5ca5ce7059 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Mon, 20 Apr 2026 09:58:59 -0300 Subject: [PATCH 2/5] chore: changelog entry --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e0ae19833..619f3bf93b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- Surface underlying cause class of keychain load failures in error logs #898 + ## [2.2.0] - 2026-04-07 ### Fixed From 7b87d7bd42de56f67ea8242e7e17e1f5b2b8df3d Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Mon, 20 Apr 2026 10:02:54 -0300 Subject: [PATCH 3/5] refactor: simplify collection call chain --- app/src/main/java/to/bitkit/data/keychain/Keychain.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/to/bitkit/data/keychain/Keychain.kt b/app/src/main/java/to/bitkit/data/keychain/Keychain.kt index 6685da2652..fd3fa87205 100644 --- a/app/src/main/java/to/bitkit/data/keychain/Keychain.kt +++ b/app/src/main/java/to/bitkit/data/keychain/Keychain.kt @@ -148,10 +148,9 @@ class Keychain @Inject constructor( val walletIndex = runCatching { runBlocking { db.configDao().getAll().first() }.firstOrNull()?.walletIndex ?: 0L }.getOrDefault(-1L) - val causeChain = generateSequence(cause) { it.cause } + val causeChain = generateSequence(cause) { it.cause } .take(CAUSE_CHAIN_DEPTH) - .map { it.javaClass.simpleName } - .joinToString(separator = " <- ") + .joinToString(separator = " <- ") { it.javaClass.simpleName } Logger.warn( "Decrypt failed for key='$key' walletIndex='$walletIndex' " + From 32a910151e111086150b9c460ec222beca5da71c Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Mon, 20 Apr 2026 10:25:29 -0300 Subject: [PATCH 4/5] fix: distinguish probe failure from false in keychain diagnostic Probes for aliasPresent, entryPresent and walletIndex previously collapsed any exception inside runCatching to the default value, making a keystore subsystem / DataStore / Room error indistinguishable from a legitimate 'alias missing'. Render tri-state instead: 'true' / 'false' / 'error:ClassName'. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../main/java/to/bitkit/data/keychain/Keychain.kt | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/to/bitkit/data/keychain/Keychain.kt b/app/src/main/java/to/bitkit/data/keychain/Keychain.kt index fd3fa87205..486039e8f3 100644 --- a/app/src/main/java/to/bitkit/data/keychain/Keychain.kt +++ b/app/src/main/java/to/bitkit/data/keychain/Keychain.kt @@ -143,11 +143,11 @@ class Keychain @Inject constructor( private fun emitLoadDiagnosticsOnce(key: String, cause: Throwable) { if (!loadDiagnosticsEmitted.add(key)) return - val aliasPresent = runCatching { keyStore.containsAlias() }.getOrDefault(false) - val entryPresent = runCatching { snapshot.contains(key.indexed) }.getOrDefault(false) - val walletIndex = runCatching { + val aliasPresent = probe { keyStore.containsAlias() } + val entryPresent = probe { snapshot.contains(key.indexed) } + val walletIndex = probe { runBlocking { db.configDao().getAll().first() }.firstOrNull()?.walletIndex ?: 0L - }.getOrDefault(-1L) + } val causeChain = generateSequence(cause) { it.cause } .take(CAUSE_CHAIN_DEPTH) .joinToString(separator = " <- ") { it.javaClass.simpleName } @@ -160,6 +160,12 @@ class Keychain @Inject constructor( ) } + private inline fun probe(block: () -> T): String = + runCatching(block).fold( + onSuccess = { it.toString() }, + onFailure = { "error:${it.javaClass.simpleName}" }, + ) + enum class Key { PUSH_NOTIFICATION_TOKEN, PUSH_NOTIFICATION_PRIVATE_KEY, From 0a113fc19fa8b0d09a8e6728f640781b665f1a33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Victor=20Sena?= Date: Mon, 20 Apr 2026 10:52:54 -0300 Subject: [PATCH 5/5] Update CHANGELOG.md Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> --- CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e97afe831..8c40bce0f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,9 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Changed -- Surface underlying cause class of keychain load failures in error logs #898 - ## [2.2.0] - 2026-04-07 ### Fixed