refactor: drop androidx.security from commons' key storage

Step 5a. SecureKeyStorage's Android actual was EncryptedSharedPreferences
from androidx.security.crypto — the same deprecated library, and the same
MasterKey.DEFAULT_MASTER_KEY_ALIAS, that EncryptedStorage uses. It now
runs on EncryptedDataStore sealed with SecretEncryption, which talks to
the AndroidKeyStore directly, so the key still never enters app memory
and the library leaves this module. Verified: androidx.security no longer
appears on commons' androidCompileClasspath.

Worth recording, because it corrects the plan this series was working to:
migrating the Android app's private keys *into* SecureKeyStorage would
have gained nothing. Both sides were the same deprecated implementation
under different filenames. The OS-keychain backing its KDoc describes is
the JVM actual, which desktop uses; Android never had it.

No migration, and none needed: SecureKeyStorage has 42 references in
desktopApp and none in amethyst, so `amethyst_secure_keys` has never been
written on an Android install. Were that to change, a migration would
have to land first — the class says so.

Also fixes a hazard this move would otherwise have introduced.
EncryptedDataStore.get() flattens a read failure into null, which is fine
for settings but wrong for getPrivateKeyOrThrow — the probe whose whole
purpose is telling "no key" apart from "backend failed", used before
creating a replacement key. Reading a merely unreadable store as absent
there overwrites a live key. getOrThrow() now propagates instead, and a
test truncates a store to prove the two reads diverge on it.

Not verified here: the Android actual itself. It needs an instrumented
test for the real AndroidKeyStore, and this environment has no device or
emulator (commons has no Robolectric either). The contract underneath it
— EncryptedDataStore over SecretEncryption — is covered by 14 jvmTest
cases against the JVM actual.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AXvKXakvup4inNFfAhhr4L

commons/build.gradle.kts

@@ -167,8 +167,8 @@ kotlin {
                 // Compose UI artifacts before the :commonsUI split.
                 implementation(libs.androidx.core.ktx)
 
-                // Secure key storage via Android Keystore
-                implementation(libs.androidx.security.crypto.ktx)
+                // Secure key storage talks to the AndroidKeyStore directly through
+                // SecretEncryption; androidx.security.crypto is gone from this module.
             }
         }
 

commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt

@@ -21,42 +21,48 @@
 package com.vitorpamplona.amethyst.commons.keystorage
 
 import android.content.Context
-import androidx.core.content.edit
-import androidx.security.crypto.EncryptedSharedPreferences
-import androidx.security.crypto.MasterKey
+import androidx.datastore.preferences.core.PreferenceDataStoreFactory
+import androidx.datastore.preferences.core.stringPreferencesKey
+import com.vitorpamplona.amethyst.commons.model.preferences.EncryptedDataStore
+import com.vitorpamplona.amethyst.commons.model.preferences.SecretEncryption
+import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.withContext
+import kotlinx.coroutines.SupervisorJob
+import okio.Path.Companion.toOkioPath
+import java.io.File
 
 /**
- * Android implementation of SecureKeyStorage using EncryptedSharedPreferences
- * backed by Android Keystore (AES-256-GCM, hardware-backed when available).
+ * Android implementation of [SecureKeyStorage]: an encrypted DataStore whose
+ * values are sealed with a key held in the AndroidKeyStore.
  *
- * ## Security Features
+ * ## Why not EncryptedSharedPreferences
  *
- * - **Hardware Security:** Uses Android Keystore (hardware-backed on supported devices with StrongBox)
- * - **Encryption:** AES-256-GCM for both keys and values
- * - **Key Derivation:** AES-256-SIV for preference keys, AES-256-GCM for values
- * - **Application Context:** Uses applicationContext to prevent memory leaks
- * - **Auto-backup Disabled:** EncryptedSharedPreferences automatically excluded from cloud backups
+ * This used to be `androidx.security.crypto`, which Google deprecated with no
+ * drop-in successor. [SecretEncryption] talks to the AndroidKeyStore directly —
+ * AES-256-GCM, StrongBox-backed where the device offers it — so the key still
+ * never enters app memory, and the library goes away.
  *
- * **Note:** While the encryption keys are protected by hardware security modules (when available),
- * the decrypted private keys returned by [getPrivateKey] are still subject to the String memory
- * limitation described in [SecureKeyStorage].
+ * Nothing is migrated from the old `amethyst_secure_keys` file because nothing
+ * ever wrote to it: this class is used by the desktop app, and the Android app
+ * has its own key storage in LocalPreferences. Were that to change, a migration
+ * would have to come first.
+ *
+ * ## Security note
+ *
+ * Only values are encrypted; the key names are not. That reveals which npubs
+ * this installation holds keys for, but not the keys themselves — the same
+ * trade-off the rest of the encrypted stores make.
+ *
+ * The String memory limitation described on [SecureKeyStorage] still applies:
+ * a decrypted private key cannot be zeroed from a JVM String.
  */
 actual class SecureKeyStorage private actual constructor() {
     actual companion object {
-        private const val PREFS_NAME = "amethyst_secure_keys"
+        private const val STORE_FILE = "datastore/secure_keys.preferences_pb"
         private const val KEY_PREFIX = "privkey_"
 
         private lateinit var appContext: Context
 
-        /**
-         * Creates a SecureKeyStorage instance for Android.
-         *
-         * @param context Android Context (will use applicationContext to avoid leaks)
-         * @return SecureKeyStorage instance
-         * @throws IllegalArgumentException if context is null or not a valid Context
-         */
         actual fun create(context: Any?): SecureKeyStorage {
             require(context is Context) { "Android requires a valid Context" }
             appContext = context.applicationContext
@@ -64,85 +70,61 @@ actual class SecureKeyStorage private actual constructor() {
         }
     }
 
-    // androidx.security.crypto is deprecated with no drop-in successor; migrating the
-    // on-disk key store is a separate, security-sensitive effort.
-    @Suppress("DEPRECATION")
-    private val masterKey: MasterKey by lazy {
-        MasterKey
-            .Builder(appContext, MasterKey.DEFAULT_MASTER_KEY_ALIAS)
-            .setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
-            .build()
-    }
+    private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob())
 
-    @Suppress("DEPRECATION")
-    private val encryptedPrefs by lazy {
-        EncryptedSharedPreferences.create(
-            appContext,
-            PREFS_NAME,
-            masterKey,
-            EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
-            EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM,
+    private val store by lazy {
+        EncryptedDataStore(
+            PreferenceDataStoreFactory.createWithPath(
+                scope = scope,
+                produceFile = { File(appContext.filesDir, STORE_FILE).toOkioPath() },
+            ),
+            SecretEncryption(),
+            scope = scope,
         )
     }
 
+    private fun keyFor(npub: String) = stringPreferencesKey(KEY_PREFIX + npub)
+
     actual suspend fun savePrivateKey(
         npub: String,
         privKeyHex: String,
     ) {
-        withContext(Dispatchers.IO) {
-            try {
-                encryptedPrefs.edit { putString(KEY_PREFIX + npub, privKeyHex) }
-            } catch (e: Exception) {
-                throw SecureStorageException("Failed to save private key", e)
-            }
+        try {
+            store.save(keyFor(npub), privKeyHex)
+        } catch (e: Exception) {
+            throw SecureStorageException("Failed to save private key", e)
         }
     }
 
     actual suspend fun getPrivateKey(npub: String): String? =
-        withContext(Dispatchers.IO) {
-            try {
-                encryptedPrefs.getString(KEY_PREFIX + npub, null)
-            } catch (e: Exception) {
-                throw SecureStorageException("Failed to retrieve private key", e)
-            }
+        try {
+            store.get(keyFor(npub))
+        } catch (e: Exception) {
+            throw SecureStorageException("Failed to retrieve private key", e)
         }
 
     /**
-     * Android backend: EncryptedSharedPreferences.contains + getString has no
-     * ambiguous-error state comparable to macOS Keychain user-cancel/deny, so
-     * "key not present" and "key present" are the only two null outcomes.
-     * Any thrown exception is a genuine failure and propagates.
+     * Unlike [getPrivateKey], this reads through [EncryptedDataStore.getOrThrow]
+     * so a store that cannot be read raises instead of reporting the key as
+     * absent. That distinction is the whole point of this method: callers use
+     * it to decide whether a key needs creating, and treating a transient read
+     * failure as "no key here" would overwrite a live one.
      */
     actual suspend fun getPrivateKeyOrThrow(npub: String): String? =
-        withContext(Dispatchers.IO) {
-            try {
-                val key = KEY_PREFIX + npub
-                if (!encryptedPrefs.contains(key)) {
-                    null
-                } else {
-                    encryptedPrefs.getString(key, null)
-                }
-            } catch (e: Exception) {
-                throw SecureStorageException("Failed to retrieve private key", e)
-            }
+        try {
+            store.getOrThrow(keyFor(npub))
+        } catch (e: Exception) {
+            throw SecureStorageException("Failed to retrieve private key", e)
         }
 
     actual suspend fun deletePrivateKey(npub: String): Boolean =
-        withContext(Dispatchers.IO) {
-            try {
-                val key = KEY_PREFIX + npub
-                val existed = encryptedPrefs.contains(key)
-                if (existed) {
-                    encryptedPrefs.edit { remove(key) }
-                }
-                existed
-            } catch (e: Exception) {
-                throw SecureStorageException("Failed to delete private key", e)
-            }
+        try {
+            val existed = store.get(keyFor(npub)) != null
+            if (existed) store.remove(keyFor(npub))
+            existed
+        } catch (e: Exception) {
+            throw SecureStorageException("Failed to delete private key", e)
         }
 
-    actual suspend fun hasPrivateKey(npub: String): Boolean =
-        withContext(Dispatchers.IO) {
-            encryptedPrefs.contains(KEY_PREFIX + npub)
-        }
+    actual suspend fun hasPrivateKey(npub: String): Boolean = getPrivateKey(npub) != null
 }

commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/EncryptedDataStore.kt

@@ -26,6 +26,7 @@ import androidx.datastore.preferences.core.edit
 import androidx.datastore.preferences.core.emptyPreferences
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.catch
+import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.firstOrNull
 import kotlinx.coroutines.flow.map
 import okio.IOException
@@ -58,6 +59,13 @@ class EncryptedDataStore(
         store.edit { prefs -> prefs[key] = encrypt(value) }
     }
 
+    /**
+     * The value, or null when the key is absent **or unreadable**.
+     *
+     * A read error is reported as absence, which is what most callers want.
+     * Anything that must not mistake a failure for an empty store — a probe
+     * deciding whether to create a replacement key, say — needs [getOrThrow].
+     */
     suspend fun get(key: Preferences.Key<String>): String? =
         store.data
             .catch { e ->
@@ -66,6 +74,16 @@ class EncryptedDataStore(
             ?.get(key)
             ?.let { decrypt(it) }
 
+    /**
+     * The value, or null only when the key is genuinely absent.
+     *
+     * Unlike [get], a failure to read propagates rather than being flattened
+     * into null. The difference matters wherever null means "nothing was ever
+     * stored" and the caller acts on that — overwriting a key that is present
+     * but temporarily unreadable is not recoverable.
+     */
+    suspend fun getOrThrow(key: Preferences.Key<String>): String? = store.data.first()[key]?.let { decrypt(it) }
+
     fun <T> getProperty(
         key: Preferences.Key<String>,
         parser: (String) -> T,

commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/EncryptedDataStoreTest.kt

@@ -25,10 +25,12 @@ import androidx.datastore.preferences.core.stringPreferencesKey
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.cancel
 import kotlinx.coroutines.test.runTest
 import okio.Path.Companion.toOkioPath
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
 import org.junit.Rule
 import org.junit.Test
 import org.junit.rules.TemporaryFolder
@@ -131,4 +133,47 @@ class EncryptedDataStoreTest {
 
             assertNull(subject.get(key))
         }
+
+    /**
+     * [EncryptedDataStore.get] flattens a read failure into null;
+     * [EncryptedDataStore.getOrThrow] does not.
+     *
+     * The difference guards a live key: a probe that decides whether to create
+     * one must not read "absent" from a store it merely failed to open, or it
+     * overwrites what is already there.
+     */
+    @Test
+    fun getSwallowsAReadFailureButGetOrThrowDoesNot() =
+        runTest {
+            val scope = CoroutineScope(Dispatchers.IO + SupervisorJob())
+            val n = seq++
+            val dataFile = File(folder.root, "corrupt_$n.preferences_pb")
+            val keyFile = File(folder.root, "corrupt_$n.key")
+            val subject =
+                EncryptedDataStore(
+                    PreferenceDataStoreFactory.createWithPath(scope = scope, produceFile = { dataFile.toOkioPath() }),
+                    SecretEncryption(keyFile),
+                    scope = scope,
+                )
+            subject.save(key, "a real value")
+            scope.cancel()
+
+            // Truncate the store so opening it fails rather than reading empty.
+            dataFile.writeBytes(byteArrayOf(0x01, 0x02, 0x03))
+
+            val readScope = CoroutineScope(Dispatchers.IO + SupervisorJob())
+            val reopened =
+                EncryptedDataStore(
+                    PreferenceDataStoreFactory.createWithPath(scope = readScope, produceFile = { dataFile.toOkioPath() }),
+                    SecretEncryption(keyFile),
+                    scope = readScope,
+                )
+
+            assertNull("get() reports the unreadable store as absent", reopened.get(key))
+            assertTrue(
+                "getOrThrow() must not call it absent",
+                runCatching { reopened.getOrThrow(key) }.isFailure,
+            )
+            readScope.cancel()
+        }
 }