diff --git a/app/src/androidTest/kotlin/io/privkey/keep/nip55/AppSignPolicyOverridesInstrumentedTest.kt b/app/src/androidTest/kotlin/io/privkey/keep/nip55/AppSignPolicyOverridesInstrumentedTest.kt new file mode 100644 index 00000000..04b31fe2 --- /dev/null +++ b/app/src/androidTest/kotlin/io/privkey/keep/nip55/AppSignPolicyOverridesInstrumentedTest.kt @@ -0,0 +1,532 @@ +package io.privkey.keep.nip55 + +import android.content.Context +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.privkey.keep.storage.SignPolicy +import io.privkey.keep.storage.SignPolicySelectionPrefs +import io.privkey.keep.uniffi.Nip55RequestType +import io.privkey.keep.uniffi.SignPolicySelection +import io.privkey.keep.uniffi.SignPolicySelectionStorage +import io.privkey.keep.uniffi.SignPolicyStore +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Per-app sign-policy overrides during the move from Room into the core-owned store. + * + * An override is normally STRICTER than the global policy, so the invariant under + * test is one-directional: an app must never come out of any of these paths on a + * looser policy than it went in on. Losing the global is survivable (it defaults to + * Manual); losing an override is not. + * + * These need the real core store, so they are instrumented: the uniffi types are + * never stubbed. + */ +@RunWith(AndroidJUnit4::class) +class AppSignPolicyOverridesInstrumentedTest { + + private val context: Context = ApplicationProvider.getApplicationContext() + + private lateinit var database: Nip55Database + private lateinit var store: PermissionStore + private lateinit var core: SignPolicyStore + + @Before + fun setup() { + clearPrefs() + database = Room.inMemoryDatabaseBuilder( + context, + Nip55Database::class.java + ).allowMainThreadQueries().build() + store = PermissionStore(database) + core = newCore() + } + + @After + fun teardown() { + database.close() + clearPrefs() + } + + private fun clearPrefs() { + context.deleteSharedPreferences(SELECTION_PREFS) + context.deleteSharedPreferences(LEGACY_PREFS) + // Only our own one-shot marker; the marker file is shared with other + // migrations, so it must not be deleted wholesale. + context.getSharedPreferences( + SignPolicySelectionPrefs.MARKER_PREFS_NAME, + Context.MODE_PRIVATE + ).edit().remove(SignPolicySelectionPrefs.MIGRATION_MARKER).commit() + } + + private fun newCore() = SignPolicyStore(SignPolicySelectionPrefs(context)) + + @Test + fun stricterWinsWhenTheCoreIsLooserThanRoom() = runBlocking { + core.setAppOverride(PKG, SignPolicySelection.AUTO) + store.setAppSignPolicyOverride(PKG, SignPolicy.MANUAL.ordinal) + + assertEquals(SignPolicySelection.MANUAL, AppSignPolicyOverrides.override(core, store, PKG)) + } + + @Test + fun stricterWinsWhenRoomIsLooserThanTheCore() = runBlocking { + core.setAppOverride(PKG, SignPolicySelection.MANUAL) + store.setAppSignPolicyOverride(PKG, SignPolicy.AUTO.ordinal) + + assertEquals(SignPolicySelection.MANUAL, AppSignPolicyOverrides.override(core, store, PKG)) + } + + @Test + fun stricterWinsAcrossTheMiddleTier() = runBlocking { + core.setAppOverride(PKG, SignPolicySelection.AUTO) + store.setAppSignPolicyOverride(PKG, SignPolicy.BASIC.ordinal) + + assertEquals(SignPolicySelection.BASIC, AppSignPolicyOverrides.override(core, store, PKG)) + } + + @Test + fun agreeingStoresReturnThatValue() = runBlocking { + core.setAppOverride(PKG, SignPolicySelection.AUTO) + store.setAppSignPolicyOverride(PKG, SignPolicy.AUTO.ordinal) + + assertEquals(SignPolicySelection.AUTO, AppSignPolicyOverrides.override(core, store, PKG)) + } + + @Test + fun roomOverrideIsUsedWhenTheCoreHasNone() = runBlocking { + store.setAppSignPolicyOverride(PKG, SignPolicy.BASIC.ordinal) + + assertNull(core.appOverride(PKG)) + assertEquals(SignPolicySelection.BASIC, AppSignPolicyOverrides.override(core, store, PKG)) + } + + @Test + fun noOverrideOnlyWhenBothStoresAreEmpty() = runBlocking { + assertNull(AppSignPolicyOverrides.override(core, store, PKG)) + } + + @Test + fun outOfRangeRoomOrdinalResolvesToManual() = runBlocking { + store.setAppSignPolicyOverride(PKG, 99) + + assertEquals(SignPolicySelection.MANUAL, AppSignPolicyOverrides.override(core, store, PKG)) + } + + @Test + fun effectivePolicyFallsBackToGlobalThenManual() = runBlocking { + assertEquals( + SignPolicySelection.MANUAL, + AppSignPolicyOverrides.effectivePolicy(core, store, PKG) + ) + + core.setGlobalPolicy(SignPolicySelection.AUTO) + assertEquals( + SignPolicySelection.AUTO, + AppSignPolicyOverrides.effectivePolicy(core, store, PKG) + ) + + store.setAppSignPolicyOverride(PKG, SignPolicy.MANUAL.ordinal) + assertEquals( + SignPolicySelection.MANUAL, + AppSignPolicyOverrides.effectivePolicy(core, store, PKG) + ) + } + + @Test + fun writeMirrorsTheSameValueIntoBothStores() = runBlocking { + store.setAppSignPolicyOverride(PKG, SignPolicy.MANUAL.ordinal) + + AppSignPolicyOverrides.setOverride(core, store, PKG, SignPolicySelection.BASIC) + + assertEquals(SignPolicySelection.BASIC, core.appOverride(PKG)) + assertEquals(SignPolicy.BASIC.ordinal, store.getAppSignPolicyOverride(PKG)) + assertEquals(SignPolicySelection.BASIC, AppSignPolicyOverrides.override(core, store, PKG)) + } + + /** + * The resurrection case: a clear has to null BOTH stores, or the dual read would + * hand the stale Room mirror straight back after the user cleared it. + */ + @Test + fun clearedOverrideDoesNotResurrectFromRoom() = runBlocking { + core.setGlobalPolicy(SignPolicySelection.AUTO) + store.setAppSignPolicyOverride(PKG, SignPolicy.MANUAL.ordinal) + AppSignPolicyOverrides.migrateLegacyOverrides(core, store) + + AppSignPolicyOverrides.setOverride(core, store, PKG, null) + + assertNull(core.appOverride(PKG)) + assertNull(store.getAppSignPolicyOverride(PKG)) + assertNull(AppSignPolicyOverrides.override(core, store, PKG)) + assertEquals( + SignPolicySelection.AUTO, + AppSignPolicyOverrides.effectivePolicy(core, store, PKG) + ) + // Also across a fresh core instance, which re-reads from disk. + assertNull(AppSignPolicyOverrides.override(newCore(), store, PKG)) + } + + @Test + fun writeKeepsTheAppExpiryOnTheRow() = runBlocking { + store.setAppExpiry(PKG, AppExpiryDuration.ONE_HOUR) + store.setAppSignPolicyOverride(PKG, SignPolicy.MANUAL.ordinal) + + AppSignPolicyOverrides.setOverride(core, store, PKG, SignPolicySelection.BASIC) + + val settings = store.getAppSettings(PKG) + assertNotNull(settings) + assertNotNull(settings!!.expiresAt) + assertEquals(SignPolicy.BASIC.ordinal, settings.signPolicyOverride) + } + + /** + * The Room mirror is what makes the override visible to the expiry sweep. Losing + * that link is how an override outlives its window, so the row and the core value + * must go together. + */ + @Test + fun expirySweepClearsTheCoreOverride() = runBlocking { + core.setGlobalPolicy(SignPolicySelection.AUTO) + val now = System.currentTimeMillis() + database.appSettingsDao().insertOrUpdate( + Nip55AppSettings( + callerPackage = PKG, + expiresAt = now - 1_000L, + signPolicyOverride = SignPolicy.BASIC.ordinal, + createdAt = now - 2_000L, + createdAtElapsed = 0L, + durationMs = null + ) + ) + core.setAppOverride(PKG, SignPolicySelection.BASIC) + + store.cleanupExpired(core) + + assertNull(core.appOverride(PKG)) + assertNull(newCore().appOverride(PKG)) + assertNull(store.getAppSignPolicyOverride(PKG)) + // Back to the global, exactly where an expired row left the app before the + // override moved into the core. + assertEquals( + SignPolicySelection.AUTO, + AppSignPolicyOverrides.effectivePolicy(core, store, PKG) + ) + } + + /** + * With no core store there is nothing to clear and nothing to confirm, so a row + * carrying an override is deferred to a sweep that can confirm it rather than + * deleted into an override nothing can reach. + */ + @Test + fun expirySweepWithoutACoreStoreDefersARowCarryingAnOverride() = runBlocking { + val now = System.currentTimeMillis() + database.appSettingsDao().insertOrUpdate( + Nip55AppSettings( + callerPackage = PKG, + expiresAt = now - 1_000L, + signPolicyOverride = SignPolicy.MANUAL.ordinal, + createdAt = now - 2_000L, + createdAtElapsed = 0L, + durationMs = null + ) + ) + database.appSettingsDao().insertOrUpdate( + Nip55AppSettings( + callerPackage = OTHER_PKG, + expiresAt = now - 1_000L, + signPolicyOverride = null, + createdAt = now - 2_000L, + createdAtElapsed = 0L, + durationMs = null + ) + ) + + store.cleanupExpired() + + assertNotNull(store.getAppSettings(PKG)) + // A row with no override has no core counterpart, so it expires as it always did. + assertNull(store.getAppSettings(OTHER_PKG)) + } + + @Test + fun expirySweepLeavesAnUnexpiredOverrideAlone() = runBlocking { + AppSignPolicyOverrides.setOverride(core, store, PKG, SignPolicySelection.MANUAL) + + store.cleanupExpired(core) + + assertEquals(SignPolicySelection.MANUAL, core.appOverride(PKG)) + assertEquals(SignPolicy.MANUAL.ordinal, store.getAppSignPolicyOverride(PKG)) + } + + @Test + fun accountSwitchLeavesNoCoreOverrideBehind() = runBlocking { + core.setGlobalPolicy(SignPolicySelection.AUTO) + AppSignPolicyOverrides.setOverride(core, store, PKG, SignPolicySelection.MANUAL) + AppSignPolicyOverrides.setOverride(core, store, OTHER_PKG, SignPolicySelection.BASIC) + + store.clearAllAppSettings(core) + + assertNull(core.appOverride(PKG)) + assertNull(core.appOverride(OTHER_PKG)) + assertNull(newCore().appOverride(PKG)) + assertNull(AppSignPolicyOverrides.override(core, store, PKG)) + assertEquals( + SignPolicySelection.AUTO, + AppSignPolicyOverrides.effectivePolicy(core, store, OTHER_PKG) + ) + } + + /** + * A session whose core store failed to construct writes to Room alone. The next + * session has a live core holding the STALE, looser value, and the migration skips + * the package because the core already knows it. Only stricter-wins keeps the + * tightening the user actually made. + */ + @Test + fun aTighteningMadeWithoutACoreStoreOutranksTheStaleCoreValue() = runBlocking { + core.setAppOverride(PKG, SignPolicySelection.AUTO) + + AppSignPolicyOverrides.setOverride(null, store, PKG, SignPolicySelection.MANUAL) + assertEquals(SignPolicy.MANUAL.ordinal, store.getAppSignPolicyOverride(PKG)) + + val liveCore = newCore() + AppSignPolicyOverrides.migrateLegacyOverrides(liveCore, store) + assertEquals(SignPolicySelection.AUTO, liveCore.appOverride(PKG)) + + assertEquals( + SignPolicySelection.MANUAL, + AppSignPolicyOverrides.override(liveCore, store, PKG) + ) + assertEquals( + SignPolicySelection.MANUAL, + AppSignPolicyOverrides.effectivePolicy(liveCore, store, PKG) + ) + } + + /** + * An override whose mirror row is gone is invisible to the app-settings table, so + * the wipe has to reach it through another record of the package. + */ + @Test + fun accountSwitchClearsACoreOverrideWithNoRoomRow() = runBlocking { + store.grantPermission( + callerPackage = PKG, + requestType = Nip55RequestType.SIGN_EVENT, + eventKind = 1, + duration = PermissionDuration.FOREVER + ) + core.setAppOverride(PKG, SignPolicySelection.MANUAL) + assertNull(store.getAppSettings(PKG)) + + store.clearAllAppSettings(core) + + assertNull(core.appOverride(PKG)) + assertNull(newCore().appOverride(PKG)) + } + + /** + * The storage backend swallows failures, so a clear that does not stick returns + * normally. The read-back has to catch it and the row has to stay, or the override + * is stranded where nothing can find it again. + */ + @Test + fun accountSwitchKeepsRowsWhoseClearDoesNotVerify() = runBlocking { + val flaky = SignPolicyStore(UnremovableStorage(PKG)) + AppSignPolicyOverrides.setOverride(flaky, store, PKG, SignPolicySelection.MANUAL) + AppSignPolicyOverrides.setOverride(flaky, store, OTHER_PKG, SignPolicySelection.BASIC) + + store.clearAllAppSettings(flaky) + + // Unprocessed package: override intact and still indexed by its row. + assertEquals(SignPolicySelection.MANUAL, flaky.appOverride(PKG)) + assertEquals(SignPolicy.MANUAL.ordinal, store.getAppSignPolicyOverride(PKG)) + assertEquals( + SignPolicySelection.MANUAL, + AppSignPolicyOverrides.override(flaky, store, PKG) + ) + // The verified package is gone from both stores. + assertNull(flaky.appOverride(OTHER_PKG)) + assertNull(store.getAppSettings(OTHER_PKG)) + } + + @Test + fun expirySweepKeepsARowWhoseClearDoesNotVerify() = runBlocking { + val flaky = SignPolicyStore(UnremovableStorage(PKG)) + val now = System.currentTimeMillis() + database.appSettingsDao().insertOrUpdate( + Nip55AppSettings( + callerPackage = PKG, + expiresAt = now - 1_000L, + signPolicyOverride = SignPolicy.MANUAL.ordinal, + createdAt = now - 2_000L, + createdAtElapsed = 0L, + durationMs = null + ) + ) + flaky.setAppOverride(PKG, SignPolicySelection.MANUAL) + + store.cleanupExpired(flaky) + + assertNotNull(store.getAppSettings(PKG)) + assertEquals(SignPolicy.MANUAL.ordinal, store.getAppSignPolicyOverride(PKG)) + assertEquals(SignPolicySelection.MANUAL, flaky.appOverride(PKG)) + } + + /** + * A clear writes the mirror first, so a mirror failure aborts before the core is + * touched: both stores still hold the override and there is nothing to resurrect. + * The reverse order would leave the stale mirror as the only copy, and the next + * migration would copy it back into the core for good. + */ + @Test + fun aClearWhoseMirrorWriteThrowsDoesNotResurrect() = runBlocking { + AppSignPolicyOverrides.setOverride(core, store, PKG, SignPolicySelection.MANUAL) + database.close() + + runCatching { AppSignPolicyOverrides.setOverride(core, store, PKG, null) } + + assertEquals(SignPolicySelection.MANUAL, core.appOverride(PKG)) + assertEquals(SignPolicySelection.MANUAL, newCore().appOverride(PKG)) + } + + @Test + fun migrationCopiesRoomOverridesIntoTheCore() = runBlocking { + store.setAppSignPolicyOverride(PKG, SignPolicy.MANUAL.ordinal) + store.setAppSignPolicyOverride(OTHER_PKG, SignPolicy.BASIC.ordinal) + + AppSignPolicyOverrides.migrateLegacyOverrides(core, store) + + assertEquals(SignPolicySelection.MANUAL, core.appOverride(PKG)) + assertEquals(SignPolicySelection.BASIC, core.appOverride(OTHER_PKG)) + // A fresh instance proves the copy was persisted, not just cached. + assertEquals(SignPolicySelection.MANUAL, newCore().appOverride(PKG)) + } + + @Test + fun migrationLeavesTheRoomValuesInPlaceForTheFallback() = runBlocking { + store.setAppSignPolicyOverride(PKG, SignPolicy.MANUAL.ordinal) + + AppSignPolicyOverrides.migrateLegacyOverrides(core, store) + + assertEquals(SignPolicy.MANUAL.ordinal, store.getAppSignPolicyOverride(PKG)) + } + + @Test + fun migrationIsIdempotentAndNeverOverwritesTheCore() = runBlocking { + // Room still holds the old, looser value the user has since tightened. + store.setAppSignPolicyOverride(PKG, SignPolicy.AUTO.ordinal) + core.setAppOverride(PKG, SignPolicySelection.MANUAL) + + AppSignPolicyOverrides.migrateLegacyOverrides(core, store) + AppSignPolicyOverrides.migrateLegacyOverrides(core, store) + AppSignPolicyOverrides.migrateLegacyOverrides(newCore(), store) + + assertEquals(SignPolicySelection.MANUAL, newCore().appOverride(PKG)) + } + + @Test + fun migrationSkipsAnExpiredRow() = runBlocking { + val now = System.currentTimeMillis() + database.appSettingsDao().insertOrUpdate( + Nip55AppSettings( + callerPackage = PKG, + expiresAt = now - 1_000L, + signPolicyOverride = SignPolicy.AUTO.ordinal, + createdAt = now - 2_000L, + createdAtElapsed = 0L, + durationMs = null + ) + ) + + AppSignPolicyOverrides.migrateLegacyOverrides(core, store) + + // Copying it would freeze a time-boxed override into the core, which has no + // expiry. The Room fallback still serves it until the expiry sweep runs. + assertNull(core.appOverride(PKG)) + assertEquals(SignPolicySelection.AUTO, AppSignPolicyOverrides.override(core, store, PKG)) + } + + /** + * The safety invariant end to end: an app pinned stricter than a loose global + * stays pinned across the migration, and stays pinned if the Room mirror is lost + * on its own, read back through a fresh core instance. + */ + @Test + fun strictOverrideSurvivesTheMigrationEndToEnd() = runBlocking { + core.setGlobalPolicy(SignPolicySelection.AUTO) + store.setAppSignPolicyOverride(PKG, SignPolicy.MANUAL.ordinal) + + assertEquals( + SignPolicySelection.MANUAL, + AppSignPolicyOverrides.effectivePolicy(core, store, PKG) + ) + + AppSignPolicyOverrides.migrateLegacyOverrides(core, store) + assertEquals( + SignPolicySelection.MANUAL, + AppSignPolicyOverrides.effectivePolicy(core, store, PKG) + ) + + store.clearAppSettings(PKG) + assertEquals( + SignPolicySelection.MANUAL, + AppSignPolicyOverrides.effectivePolicy(newCore(), store, PKG) + ) + } + + /** + * A migration that never ran (or failed outright) must not loosen anything: the + * Room fallback still pins the app. + */ + @Test + fun strictOverrideHoldsWhenTheMigrationNeverRan() = runBlocking { + core.setGlobalPolicy(SignPolicySelection.AUTO) + store.setAppSignPolicyOverride(PKG, SignPolicy.MANUAL.ordinal) + + assertNull(core.appOverride(PKG)) + assertEquals( + SignPolicySelection.MANUAL, + AppSignPolicyOverrides.effectivePolicy(core, store, PKG) + ) + } + + private companion object { + const val PKG = "com.test.app" + const val OTHER_PKG = "com.test.other" + const val SELECTION_PREFS = "keep_sign_policy_selection" + const val LEGACY_PREFS = "keep_sign_policy" + } +} + +/** + * A real backend for the real core store, not a stubbed uniffi type: it implements the + * same [SignPolicySelectionStorage] trait the production encrypted-prefs class does, + * and reproduces the failure mode that motivates the read-back checks. Removals for + * [unremovablePackage] are dropped and reported as success, exactly as the production + * backend does when it swallows an exception or `commit()` returns false. + */ +private class UnremovableStorage(private val unremovablePackage: String) : SignPolicySelectionStorage { + + private val values = HashMap() + + override fun load(key: String): String? = values[key] + + override fun save(key: String, value: String) { + values[key] = value + } + + override fun remove(key: String) { + if (key.endsWith(unremovablePackage)) return + values.remove(key) + } +} diff --git a/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt b/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt index a108816f..7d98f5cc 100644 --- a/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt +++ b/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt @@ -12,6 +12,7 @@ import io.privkey.keep.nip46.BunkerConfigStore import io.privkey.keep.nip46.BunkerService import io.privkey.keep.nip55.AUDIT_OP_ACCOUNT_SWITCH import io.privkey.keep.nip55.AndroidSigningRateLimiterStorage +import io.privkey.keep.nip55.AppSignPolicyOverrides import io.privkey.keep.nip55.AutoSigningSafeguards import io.privkey.keep.nip55.CallerVerificationStore import io.privkey.keep.nip55.EventLogCategory @@ -224,7 +225,10 @@ class KeepMobileApp : Application() { eventLogStore = eventLog initializeSigningAuditLog(db) applicationScope.launch { - store.cleanupExpired() + store.cleanupExpired(signPolicyStore) + // After the expiry sweep, so a row that just aged out is not copied + // into the core (which has no expiry) as a permanent override. + signPolicyStore?.let { AppSignPolicyOverrides.migrateLegacyOverrides(it, store) } callerVerificationStore?.cleanupExpiredNonces() runCatching { eventLog.cleanupOld(System.currentTimeMillis() - EVENT_LOG_MAX_AGE_MS) @@ -465,7 +469,7 @@ class KeepMobileApp : Application() { DescriptorSessionManager.clearAll() withContext(Dispatchers.IO) { runAccountSwitchCleanup("revoke permissions") { permissionStore?.revokeAllPermissions() } - runAccountSwitchCleanup("clear app settings") { permissionStore?.clearAllAppSettings() } + runAccountSwitchCleanup("clear app settings") { permissionStore?.clearAllAppSettings(signPolicyStore) } runAccountSwitchCleanup("clear velocity") { permissionStore?.clearAllVelocity() } runAccountSwitchCleanup("clear caller trust") { callerVerificationStore?.clearAllTrust() } runAccountSwitchCleanup("clear auto-signing state") { autoSigningSafeguards?.clearAll() } diff --git a/app/src/main/kotlin/io/privkey/keep/MainActivity.kt b/app/src/main/kotlin/io/privkey/keep/MainActivity.kt index e8f66ef3..8c76a226 100644 --- a/app/src/main/kotlin/io/privkey/keep/MainActivity.kt +++ b/app/src/main/kotlin/io/privkey/keep/MainActivity.kt @@ -1327,7 +1327,7 @@ fun MainScreen( onSigningHistoryClick = { showHistoryScreen = true }, onClearLogsAndActivity = { withContext(Dispatchers.IO) { - permissionStore.cleanupExpired() + permissionStore.cleanupExpired(signPolicyStore) } } ) diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt b/app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt index 4fb701ea..bcd87a59 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt @@ -29,6 +29,8 @@ import io.privkey.keep.KeepMobileApp import io.privkey.keep.nip46.BunkerConfigStore import io.privkey.keep.nip46.BunkerService import io.privkey.keep.nip46.Nip46ClientStore +import io.privkey.keep.storage.SignPolicy +import io.privkey.keep.storage.toSelection import io.privkey.keep.storage.toSignPolicy import io.privkey.keep.uniffi.BunkerConfigInfo import io.privkey.keep.uniffi.SignPolicyStore @@ -37,6 +39,15 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import kotlinx.coroutines.withContext +// The selector speaks SignPolicy ordinals; the stores speak SignPolicySelection. +private suspend fun readOverrideOrdinal( + signPolicyStore: SignPolicyStore?, + permissionStore: PermissionStore, + packageName: String +): Int? = AppSignPolicyOverrides.override(signPolicyStore, permissionStore, packageName) + ?.toSignPolicy() + ?.ordinal + private data class AppState( val label: String? = null, val icon: Drawable? = null, @@ -75,7 +86,7 @@ fun AppPermissionsScreen( .getOrDefault(emptyList()) val loadedSettings = permissionStore.getAppSettings(packageName) - val signPolicyOverride = runCatching { permissionStore.getAppSignPolicyOverride(packageName) } + val signPolicyOverride = runCatching { readOverrideOrdinal(signPolicyStore, permissionStore, packageName) } .getOrNull() Pair(AppState(label, null, true, true, permissions, signPolicyOverride, isLoading = false), loadedSettings) @@ -95,7 +106,7 @@ fun AppPermissionsScreen( .getOrDefault(emptyList()) val loadedSettings = permissionStore.getAppSettings(packageName) - val signPolicyOverride = runCatching { permissionStore.getAppSignPolicyOverride(packageName) } + val signPolicyOverride = runCatching { readOverrideOrdinal(signPolicyStore, permissionStore, packageName) } .onFailure { if (BuildConfig.DEBUG) Log.e("AppPermissions", "Failed to load sign policy [hash:$pkgHash]", it) } .getOrNull() @@ -269,10 +280,21 @@ private fun AppPermissionsListContent( onOverrideChange = { newOverride -> coroutineScope.launch { try { - withContext(Dispatchers.IO) { - permissionStore.setAppSignPolicyOverride(packageName, newOverride) + // Show what actually persisted. The core's storage + // trait cannot report a write failure, so a failed + // write leaves the previous override in force; the + // screen must not claim a tightening that did not + // take effect. + val persisted = withContext(Dispatchers.IO) { + AppSignPolicyOverrides.setOverride( + signPolicyStore, + permissionStore, + packageName, + newOverride?.let { SignPolicy.fromOrdinal(it).toSelection() } + ) + readOverrideOrdinal(signPolicyStore, permissionStore, packageName) } - onAppStateChange(appState.copy(signPolicyOverride = newOverride)) + onAppStateChange(appState.copy(signPolicyOverride = persisted)) } catch (e: Exception) { if (BuildConfig.DEBUG) Log.e("AppPermissions", "Failed to update sign policy", e) Toast.makeText(context, toastSignPolicyError, Toast.LENGTH_SHORT).show() diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt b/app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt new file mode 100644 index 00000000..a89fba42 --- /dev/null +++ b/app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt @@ -0,0 +1,166 @@ +package io.privkey.keep.nip55 + +import android.util.Log +import io.privkey.keep.BuildConfig +import io.privkey.keep.storage.SignPolicy +import io.privkey.keep.storage.toSelection +import io.privkey.keep.storage.toSignPolicy +import io.privkey.keep.uniffi.SignPolicySelection +import io.privkey.keep.uniffi.SignPolicyStore + +private const val TAG = "AppSignPolicyOverrides" + +/** + * Per-app sign-policy overrides, mid-move from the legacy Room `nip55_app_settings` + * row into the core-owned store. + * + * A per-app override is normally STRICTER than the global policy (an app pinned to + * Manual while the global is Auto), so losing one silently drops that app onto the + * looser global and auto-approves signing it should not get. Every path here is built + * around that: reads consult both stores and resolve a disagreement to the stricter + * side, a write only touches the second store once the first has confirmed, and the + * migration never overwrites the core. + * + * The content provider and the UI both go through here so the two cannot drift. + */ +object AppSignPolicyOverrides { + + /** + * The override in force, read from both stores. Yields null only when neither + * holds one, so an incomplete or failed migration can never drop an app onto the + * looser global policy. + * + * When the two disagree the STRICTER value wins (Manual < Basic < Auto), not the + * core. The stores can only disagree because a write landed in one and not the + * other, and there is no way to tell which side is the newer intent: the core's + * prefs backend swallows write failures and discards `commit()`'s result, so a + * value can be current in memory and absent on disk, and a session where the core + * store failed to construct writes to Room alone (the migration then skips that + * package forever, because the core already "knows" it). Core-first would let a + * stale looser value win in every one of those cases. Picking the stricter side + * costs the user a re-pick at worst; picking the looser one silently auto-approves + * signing the app was pinned away from. + */ + suspend fun override( + core: SignPolicyStore?, + permissions: PermissionStore, + callerPackage: String + ): SignPolicySelection? = + stricter(core?.appOverride(callerPackage), legacyOverride(permissions, callerPackage)) + + private fun stricter( + first: SignPolicySelection?, + second: SignPolicySelection? + ): SignPolicySelection? { + if (first == null) return second + if (second == null) return first + // Via SignPolicy so the ordering goes through the checked mapping rather than + // assuming the FFI enum's declaration order. + return if (first.toSignPolicy().ordinal <= second.toSignPolicy().ordinal) first else second + } + + /** + * Override -> global -> Manual, the precedence the signing path has always used. + * + * Deliberately not the core's `effectivePolicy`, which cannot see the Room + * fallback and would report the global for any app whose override has not been + * migrated yet. Switch to it once the fallback below is retired. + */ + suspend fun effectivePolicy( + core: SignPolicyStore?, + permissions: PermissionStore, + callerPackage: String + ): SignPolicySelection = + override(core, permissions, callerPackage) + ?: core?.globalPolicy() + ?: SignPolicySelection.MANUAL + + /** + * Writes [selection] to the core (null clears the override) and mirrors the same + * value into the Room row. + * + * Room is a mirror, not a stale leftover: a clear nulls both stores, so the dual + * read above cannot resurrect an override the user has cleared or loosened. The + * mirror is what keeps the Room row usable as the lifecycle index for overrides, + * which is how the expiry sweep and the account-switch wipe still find the + * packages whose core override has to go (see [PermissionStore.cleanupExpired] + * and [PermissionStore.clearAllAppSettings]). Clearing Room here instead would + * make core overrides invisible to Kotlin and immortal. + * + * On a SET the core goes first and Room is only touched once the core reports the + * new value back, because the core's storage trait cannot signal a failed write. + * + * On a CLEAR the order is reversed: the mirror goes first, and the core is left + * alone if that throws. Clearing the core first and then failing on the mirror + * would leave the stale override as the only copy, which [override] hands straight + * back and [migrateLegacyOverrides] then copies into the core permanently, since + * the core no longer holds anything to skip on. Mirror-first turns that into "the + * clear did not happen": both stores still agree on the old value, the caller sees + * the throw, and a re-read shows the override still in force. + */ + suspend fun setOverride( + core: SignPolicyStore?, + permissions: PermissionStore, + callerPackage: String, + selection: SignPolicySelection? + ) { + val ordinal = selection?.toSignPolicy()?.ordinal + if (core == null) { + // No core store this session (init failed). Keep Room authoritative + // rather than dropping the override on the floor. The stricter-wins rule + // in [override] stops a stale core value from outranking this later. + permissions.setAppSignPolicyOverride(callerPackage, ordinal) + return + } + if (selection == null) { + permissions.setAppSignPolicyOverride(callerPackage, null) + core.setAppOverride(callerPackage, null) + return + } + core.setAppOverride(callerPackage, selection) + if (core.appOverride(callerPackage) != selection) return + // A failed mirror write must not propagate: the core already holds the new + // value, so the write did take effect. The stores diverge until the next + // write, and stricter-wins bounds that to "no looser than either side". + runCatching { permissions.setAppSignPolicyOverride(callerPackage, ordinal) } + .onFailure { if (BuildConfig.DEBUG) Log.w(TAG, "Sign-policy mirror write failed", it) } + } + + /** + * Best-effort copy of the legacy Room overrides into the core, run at startup. + * Idempotent: a package the core already holds an override for is left alone, so + * a re-run can never clobber a newer choice with the stale Room value. Skipping + * those packages is safe precisely because [override] resolves a disagreement to + * the stricter side rather than to whatever the core happens to hold. + * + * The Room values stay on disk: [override] still falls back to them, and they are + * the index the lifecycle sweeps use. A failure here is therefore harmless, it + * just leaves the fallback doing the work. + */ + suspend fun migrateLegacyOverrides(core: SignPolicyStore, permissions: PermissionStore) { + runCatching { + for (settings in permissions.getAllAppSettings()) { + val ordinal = settings.signPolicyOverride ?: continue + // An expired row is on its way out via the expiry sweep; copying it + // would turn a time-boxed override into a permanent one. The Room + // fallback still covers it until the sweep removes it. + if (settings.isExpired()) continue + if (core.appOverride(settings.callerPackage) != null) continue + core.setAppOverride( + settings.callerPackage, + SignPolicy.fromOrdinal(ordinal).toSelection() + ) + } + }.onFailure { + if (BuildConfig.DEBUG) Log.w(TAG, "Sign-policy override migration failed", it) + } + } + + // An out-of-range stored ordinal resolves to Manual, the strictest tier. + private suspend fun legacyOverride( + permissions: PermissionStore, + callerPackage: String + ): SignPolicySelection? = + permissions.getAppSignPolicyOverride(callerPackage) + ?.let { SignPolicy.fromOrdinal(it).toSelection() } +} diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt b/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt index 8be875fc..97ee31ad 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt @@ -20,8 +20,6 @@ import androidx.core.content.ContextCompat import io.privkey.keep.BuildConfig import io.privkey.keep.KeepMobileApp import io.privkey.keep.R -import io.privkey.keep.storage.SignPolicy -import io.privkey.keep.storage.toSelection import io.privkey.keep.uniffi.AutoSignDecision import io.privkey.keep.uniffi.Nip55DecisionInputs import io.privkey.keep.uniffi.Nip55Handler @@ -272,13 +270,14 @@ class Nip55ContentProvider : ContentProvider() { } } - // Sign-policy precedence: per-app override (Room) -> core-owned global -> MANUAL - // default. The selection passes through as-is; collapsing BASIC onto AUTO here - // would discard the stricter Basic auto-approval band the core now enforces. + // Sign-policy precedence: per-app override -> core-owned global -> MANUAL + // default, resolved in AppSignPolicyOverrides so this and the settings UI + // cannot drift. The override read consults the core first and falls back to + // the legacy Room row, so an unmigrated app keeps its (usually stricter) + // override. The selection passes through as-is; collapsing BASIC onto AUTO + // here would discard the stricter Basic auto-approval band the core enforces. val policySelection = runWithTimeout { - store.getAppSignPolicyOverride(callerPackage)?.let { SignPolicy.fromOrdinal(it).toSelection() } - ?: currentApp.getSignPolicyStore()?.globalPolicy() - ?: SignPolicySelection.MANUAL + AppSignPolicyOverrides.effectivePolicy(currentApp.getSignPolicyStore(), store, callerPackage) } ?: SignPolicySelection.MANUAL val isOptedIn = currentApp.getAutoSigningSafeguards()?.isOptedIn(callerPackage) == true @@ -360,7 +359,7 @@ class Nip55ContentProvider : ContentProvider() { Nip55Outcome.AutoApprove -> executeBackgroundRequest(h, store, currentApp, callerPackage, requestType, rawContent, rawPubkey, null, eventKind, currentUser, v3Kind, v3Scope) is Nip55Outcome.Reject -> { - if (outcome.reason == "deny_expired") runWithTimeout { store.cleanupExpired() } + if (outcome.reason == "deny_expired") runWithTimeout { store.cleanupExpired(currentApp.getSignPolicyStore()) } runWithTimeout { store.logOperation(callerPackage, requestType, eventKind, outcome.reason, wasAutomatic = true) } rejectedCursor(null) } diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt b/app/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt index b34072d4..2dcced61 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt @@ -9,6 +9,7 @@ import io.privkey.keep.uniffi.Nip55PermissionDuration import io.privkey.keep.uniffi.Nip55RequestType import io.privkey.keep.uniffi.Nip55StoredPermission import io.privkey.keep.uniffi.Nip55VelocityResult +import io.privkey.keep.uniffi.SignPolicyStore import io.privkey.keep.uniffi.nip55AuditEntryHash import io.privkey.keep.uniffi.nip55CheckVelocity import io.privkey.keep.uniffi.nip55EffectiveGrantDuration @@ -29,20 +30,67 @@ class PermissionStore(private val database: Nip55Database) { val riskAssessor: RiskAssessor by lazy { RiskAssessor(auditDao, appSettingsDao) } - suspend fun cleanupExpired() { + /** + * [signPolicyStore] lets the sweep take the core-owned sign-policy override down + * with the expiring row. Without it a per-app override would outlive its expiry + * window, since the core store has no expiry of its own. + * + * The app-settings rows are deleted one package at a time after the transaction + * commits, replacing the bulk `deleteExpired`, because a row must not be dropped + * unless that package's core override is confirmed gone: the row is the only + * index Kotlin has for it, so deleting it first would orphan the override + * permanently and invisibly. Each delete re-checks `isExpired` first, which is the + * predicate the bulk statement applied at delete time, so a row refreshed between + * the enumeration and the delete is still spared. + * + * The clears run outside the transaction: they are blocking keystore and disk + * commits, and the transaction holds the process-wide audit mutex. + */ + suspend fun cleanupExpired(signPolicyStore: SignPolicyStore? = null) { val now = System.currentTimeMillis() val nowElapsed = SystemClock.elapsedRealtime() + var expiredPackages = emptyList() auditWriter.prune(now - 30 * DAY_MS) { dao.deleteExpired(now, nowElapsed) dao.deleteNip46Permissions() - val expiredPackages = appSettingsDao.getExpiredPackages(now, nowElapsed) + expiredPackages = appSettingsDao.getExpiredPackages(now, nowElapsed) expiredPackages.forEach { pkg -> dao.deleteForCaller(pkg) } - appSettingsDao.deleteExpired(now, nowElapsed) + } + for (pkg in expiredPackages) { + val settings = appSettingsDao.getSettings(pkg) ?: continue + if (!settings.isExpired()) continue + val cleared = if (signPolicyStore == null) { + // Nothing can be cleared or confirmed this session. A row carrying no + // override has no core counterpart to orphan, so it expires as it + // always did; one that does is left for a sweep that can confirm it. + settings.signPolicyOverride == null + } else { + coreOverrideCleared(signPolicyStore, pkg) + } + if (cleared) appSettingsDao.delete(pkg) } } + /** + * Clears [callerPackage]'s core-owned sign-policy override and confirms it by + * reading it back. + * + * The read-back is the whole point: the encrypted-prefs backend behind the core + * store swallows write failures and discards `commit()`'s boolean, so the call + * returning normally proves nothing. Anything short of an observed null leaves the + * override in place and the caller keeps the row that indexes it. + */ + private fun coreOverrideCleared(signPolicyStore: SignPolicyStore, callerPackage: String): Boolean = + runCatching { + // Nothing to clear is already the desired state, and skipping the write + // keeps the account-switch wipe from committing once per historical caller. + if (signPolicyStore.appOverride(callerPackage) == null) return@runCatching true + signPolicyStore.setAppOverride(callerPackage, null) + signPolicyStore.appOverride(callerPackage) == null + }.getOrDefault(false) + // Decision resolution (incl. the rule that sensitive kinds never fall back // to a generic grant) lives in Rust; Android fetches the candidate rows and // supplies the clock readings. @@ -423,7 +471,44 @@ class PermissionStore(private val database: Nip55Database) { suspend fun revokeAllPermissions() = dao.deleteAll() - suspend fun clearAllAppSettings() = appSettingsDao.deleteAll() + /** + * Wipe every app settings row, clearing each package's core sign-policy override + * first so no override survives an account switch. + * + * Every clear is confirmed by read-back, and the single `deleteAll` is issued only + * if all of them verify. Otherwise the rows for the packages that did verify are + * deleted individually and the rest are kept: the row is the only record that the + * package still holds a core override, so a later wipe can resume it. Deleting it + * regardless would strand an override Kotlin can no longer see and could never + * clear again. + * + * The candidates are the mirror rows plus the permission and audit callers. The + * core store cannot be enumerated, so an override whose mirror row is already gone + * is only reachable through some other record of that package. + */ + suspend fun clearAllAppSettings(signPolicyStore: SignPolicyStore? = null) { + if (signPolicyStore == null) { + // Nothing can be cleared or confirmed this session, and keeping the rows + // would carry the previous account's settings into the new one. + appSettingsDao.deleteAll() + return + } + val packages = LinkedHashSet() + runCatching { appSettingsDao.getAll().forEach { packages.add(it.callerPackage) } } + runCatching { packages.addAll(dao.getDistinctCallers()) } + runCatching { packages.addAll(auditDao.getDistinctCallers()) } + + val unclearable = packages.filterNot { coreOverrideCleared(signPolicyStore, it) } + if (unclearable.isEmpty()) { + // Also sweeps rows that appeared after the snapshot above. + appSettingsDao.deleteAll() + } else { + appSettingsDao.getAll() + .map { it.callerPackage } + .filterNot { it in unclearable } + .forEach { appSettingsDao.delete(it) } + } + } suspend fun clearAllVelocity() = velocityDao.deleteAll() @@ -453,6 +538,8 @@ class PermissionStore(private val database: Nip55Database) { eventKind: Int? ): Long? = auditDao.getLastUsedTimeForPermission(callerPackage, requestType, eventKind ?: EVENT_KIND_GENERIC) + suspend fun getAllAppSettings(): List = appSettingsDao.getAll() + suspend fun getAppSignPolicyOverride(callerPackage: String): Int? = appSettingsDao.getSettings(callerPackage)?.signPolicyOverride