diff --git a/app/src/androidTest/kotlin/io/privkey/keep/nip55/AppSignPolicyOverridesInstrumentedTest.kt b/app/src/androidTest/kotlin/io/privkey/keep/nip55/AppSignPolicyOverridesInstrumentedTest.kt index 958cde8c..b18bd2a1 100644 --- a/app/src/androidTest/kotlin/io/privkey/keep/nip55/AppSignPolicyOverridesInstrumentedTest.kt +++ b/app/src/androidTest/kotlin/io/privkey/keep/nip55/AppSignPolicyOverridesInstrumentedTest.kt @@ -13,6 +13,7 @@ import io.privkey.keep.uniffi.SignPolicyStore import kotlinx.coroutines.runBlocking import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue @@ -60,6 +61,7 @@ class AppSignPolicyOverridesInstrumentedTest { private fun clearPrefs() { context.deleteSharedPreferences(SELECTION_PREFS) context.deleteSharedPreferences(LEGACY_PREFS) + context.deleteSharedPreferences(AUTO_SIGNING_PREFS) // Only our own one-shot marker; the marker file is shared with other // migrations, so it must not be deleted wholesale. context.getSharedPreferences( @@ -215,7 +217,7 @@ class AppSignPolicyOverridesInstrumentedTest { ) core.setAppOverride(PKG, SignPolicySelection.BASIC) - store.cleanupExpired(core) + store.cleanupExpired(core, AutoSigningSafeguards(context)) assertNull(core.appOverride(PKG)) assertNull(newCore().appOverride(PKG)) @@ -257,13 +259,38 @@ class AppSignPolicyOverridesInstrumentedTest { ) ) - store.cleanupExpired() + store.cleanupExpired(null, AutoSigningSafeguards(context)) 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)) } + /** + * With no safeguards store this session, "not opted in" cannot be told apart from + * "opted in and unclearable", so the sweep keeps the row rather than dropping the one + * record that keeps the app expired. Fail-closed: the app stays refused until a sweep + * that can establish it runs, instead of silently regaining the global policy. + */ + @Test + fun expirySweepWithoutSafeguardsKeepsTheRow() = runBlocking { + val now = System.currentTimeMillis() + database.appSettingsDao().insertOrUpdate( + Nip55AppSettings( + callerPackage = PKG, + expiresAt = now - 1_000L, + signPolicyOverride = null, + createdAt = now - 2_000L, + createdAtElapsed = 0L, + durationMs = null + ) + ) + + store.cleanupExpired(core, null) + + assertNotNull(store.getAppSettings(PKG)) + } + @Test fun expirySweepLeavesAnUnexpiredOverrideAlone() = runBlocking { AppSignPolicyOverrides.setOverride(core, store, PKG, SignPolicySelection.MANUAL) @@ -312,6 +339,76 @@ class AppSignPolicyOverridesInstrumentedTest { ) } + /** + * An expired window must not take the user's standing refusal with it, and must not + * leave the app able to auto-sign under the global policy. + * + * A DENY carries its own expiry, so it is not part of the time-boxed grant: the sweep + * leaving it in place is what stops expiry from loosening. Clearing the opt-in is what + * makes the app re-approvable rather than silently back on the global. + */ + @Test + fun theExpirySweepKeepsAStandingDenyAndClearsTheOptIn() = runBlocking { + val safeguards = AutoSigningSafeguards(context) + safeguards.setOptedIn(PKG, true) + val now = System.currentTimeMillis() + // A refusal with no expiry of its own: the user denied this app, full stop. + database.permissionDao().insertPermission( + Nip55Permission( + callerPackage = PKG, + requestType = Nip55RequestType.SIGN_EVENT.name, + eventKind = 1, + decision = "deny", + expiresAt = null, + createdAt = now - 2_000L + ) + ) + database.appSettingsDao().insertOrUpdate( + Nip55AppSettings( + callerPackage = PKG, + expiresAt = now - 1_000L, + signPolicyOverride = null, + createdAt = now - 2_000L, + createdAtElapsed = 0L, + durationMs = null + ) + ) + + store.setPermissionToAsk(PKG, Nip55RequestType.SIGN_EVENT, 3) + // An ALLOW with no expiry of its own, which per-row expiry would never retire. + database.permissionDao().insertPermission( + Nip55Permission( + callerPackage = PKG, + requestType = Nip55RequestType.SIGN_EVENT.name, + eventKind = 2, + decision = "allow", + expiresAt = null, + createdAt = now - 2_000L + ) + ) + + store.cleanupExpired(core, safeguards) + + assertEquals( + PermissionDecision.DENY, + store.getPermissionDecision(PKG, Nip55RequestType.SIGN_EVENT, 1) + ) + // The grant goes with the window. Keeping it would auto-approve at the stored + // permission gate as soon as this sweep drops the settings row and the app stops + // counting as expired. + assertNull(store.getPermissionDecision(PKG, Nip55RequestType.SIGN_EVENT, 2)) + // An explicit "always ask" is a standing instruction too, and at keep v0.11.0 it + // blocks the policy auto-approve, so deleting it would hand that back. + assertEquals( + PermissionDecision.ASK, + store.getPermissionDecision(PKG, Nip55RequestType.SIGN_EVENT, 3) + ) + assertFalse(safeguards.isOptedIn(PKG)) + // The row only goes once the opt-in clear is durable, since the package cannot be + // enumerated as expired again afterwards. + assertNull(store.getAppSettings(PKG)) + } + /** * A storeless account switch cannot clear a tier, so it cannot drop the row that * indexes one. That row must not hand the next account the previous one's choice or @@ -651,6 +748,7 @@ class AppSignPolicyOverridesInstrumentedTest { const val PKG = "com.test.app" const val OTHER_PKG = "com.test.other" const val SELECTION_PREFS = "keep_sign_policy_selection" + const val AUTO_SIGNING_PREFS = "nip55_auto_signing" const val LEGACY_PREFS = "keep_sign_policy" } } diff --git a/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt b/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt index 5c948bb4..43b9f1ca 100644 --- a/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt +++ b/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt @@ -226,7 +226,7 @@ class KeepMobileApp : Application() { eventLogStore = eventLog initializeSigningAuditLog(db) applicationScope.launch { - store.cleanupExpired(signPolicyStore) + store.cleanupExpired(signPolicyStore, autoSigningSafeguards) // 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. // diff --git a/app/src/main/kotlin/io/privkey/keep/MainActivity.kt b/app/src/main/kotlin/io/privkey/keep/MainActivity.kt index 314ad18c..f7ea72ef 100644 --- a/app/src/main/kotlin/io/privkey/keep/MainActivity.kt +++ b/app/src/main/kotlin/io/privkey/keep/MainActivity.kt @@ -384,6 +384,8 @@ fun MainScreen( val appLiveState = (LocalContext.current.applicationContext as? KeepMobileApp)?.liveState val signingAuditLog = (LocalContext.current.applicationContext as? KeepMobileApp)?.getSigningAuditLog() val eventLogStore = (LocalContext.current.applicationContext as? KeepMobileApp)?.getEventLogStore() + val autoSigningSafeguards = + (LocalContext.current.applicationContext as? KeepMobileApp)?.getAutoSigningSafeguards() val peers = appLiveState?.peers ?: emptyList() val pendingCount = appLiveState?.pendingRequests?.size ?: 0 val connectionStatus = appLiveState?.connectionStatus @@ -1604,7 +1606,7 @@ fun MainScreen( onSigningHistoryClick = { showHistoryScreen = true }, onClearLogsAndActivity = { withContext(Dispatchers.IO) { - permissionStore.cleanupExpired(signPolicyStore) + permissionStore.cleanupExpired(signPolicyStore, autoSigningSafeguards) } } ) diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/AutoSigningSafeguards.kt b/app/src/main/kotlin/io/privkey/keep/nip55/AutoSigningSafeguards.kt index c9069bdb..915e7251 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/AutoSigningSafeguards.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/AutoSigningSafeguards.kt @@ -25,6 +25,16 @@ class AutoSigningSafeguards(context: Context) { prefs.edit().putBoolean(KEY_PREFIX_OPTED_IN + packageName, optedIn).apply() } + /** + * Removes the opt-in and reports whether that reached disk. + * + * Durable where [setOptedIn] is fire-and-forget, because the expiry sweep drops the + * app's settings row on the strength of this and the package cannot be enumerated as + * expired again afterwards, so a lost write could never be retried. + */ + fun clearOptIn(packageName: String): Boolean = + prefs.edit().remove(KEY_PREFIX_OPTED_IN + packageName).commit() + fun clearAll() { prefs.edit().clear().commit() } 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 30dab6b1..c95e2cd8 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt @@ -369,7 +369,12 @@ 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(currentApp.getSignPolicyStore()) } + if (outcome.reason == "deny_expired") runWithTimeout { + store.cleanupExpired( + currentApp.getSignPolicyStore(), + currentApp.getAutoSigningSafeguards() + ) + } runWithTimeout { store.logOperation(callerPackage, requestType, eventKind, outcome.reason, wasAutomatic = true) } rejectedCursor(null) } diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/PermissionDaos.kt b/app/src/main/kotlin/io/privkey/keep/nip55/PermissionDaos.kt index 24423a6e..1f354c58 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/PermissionDaos.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/PermissionDaos.kt @@ -30,6 +30,17 @@ interface Nip55PermissionDao { @Query("DELETE FROM nip55_permissions WHERE callerPackage = :callerPackage") suspend fun deleteForCaller(callerPackage: String) + // Everything the app's expiry window granted, but neither a refusal nor an explicit + // "always ask": both are standing instructions about the app rather than part of the + // time-boxed grant, both are written only by the user's own toggle with no expiry of + // their own, and at keep v0.11.0 an ASK row blocks the policy auto-approve and the + // whitelisted-relay auto-accept, so deleting it would hand back an auto-approve the + // user had switched off. LOWER() to match the core's to_ascii_lowercase rather than + // because the column holds mixed case; every writer stores lowercase, since + // PermissionDecision.toString() is overridden to name.lowercase(). + @Query("DELETE FROM nip55_permissions WHERE callerPackage = :callerPackage AND LOWER(decision) NOT IN ('deny', 'ask')") + suspend fun deleteGrantsForCaller(callerPackage: String) + @Query("DELETE FROM nip55_permissions WHERE callerPackage LIKE 'nip46:%'") suspend fun deleteNip46Permissions() 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 9794617e..5ead4217 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt @@ -35,6 +35,26 @@ class PermissionStore(private val database: Nip55Database) { val riskAssessor: RiskAssessor by lazy { RiskAssessor(auditDao, appSettingsDao) } /** + * Retires what an expired app-settings window granted, so the app has to be approved + * again rather than dropping back to the global policy. + * + * Expiry used to LOOSEN. The sweep deleted every permission row for the caller, which + * took the user's explicit DENY with it, left the per-package auto-signing opt-in + * intact, and removed the settings row, so `isAppExpired` then reported false and the + * app resolved to the global policy. Under a global of Auto or Basic that is silent + * auto-approval for an app whose window had just closed, and for one the user had + * explicitly refused. A time box must not end in broader access than it granted. + * + * So the per-caller delete now spares a DENY and an explicit ASK, both of which are + * standing instructions about the app written only by the user's own toggle. What the + * window granted still goes, + * because an ALLOW may be [PermissionDuration.FOREVER] and would otherwise outlive + * the window and auto-approve at the stored-permission gate the moment this sweep + * drops the settings row. A refusal is not part of what was granted, and it carries + * its own expiry, so it lives exactly as long as the user asked. [autoSigning] clears + * the opt-in for each expired package, which is what makes re-approval necessary + * instead of optional. + * * [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. @@ -50,7 +70,10 @@ class PermissionStore(private val database: Nip55Database) { * 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) { + suspend fun cleanupExpired( + signPolicyStore: SignPolicyStore? = null, + autoSigning: AutoSigningSafeguards? = null + ) { val now = System.currentTimeMillis() val nowElapsed = SystemClock.elapsedRealtime() var expiredPackages = emptyList() @@ -59,7 +82,12 @@ class PermissionStore(private val database: Nip55Database) { dao.deleteNip46Permissions() expiredPackages = appSettingsDao.getExpiredPackages(now, nowElapsed) expiredPackages.forEach { pkg -> - dao.deleteForCaller(pkg) + // Grants only. An ALLOW can be PermissionDuration.FOREVER, so per-row + // expiry would never retire it, and once this sweep drops the settings row + // the app stops being expired and that ALLOW auto-approves at the stored + // permission gate. A DENY is left alone: it is not part of what the window + // granted, and it has its own expiry. Nor is an explicit "always ask". + dao.deleteGrantsForCaller(pkg) } } for (pkg in expiredPackages) { @@ -73,10 +101,40 @@ class PermissionStore(private val database: Nip55Database) { } else { coreOverrideCleared(signPolicyStore, pkg) } - if (cleared) appSettingsDao.delete(pkg) + // Gated the same way as the core clear, and for the same reason: once the row + // is gone this package never comes back from getExpiredPackages, so neither + // can be retried. An opt-in left behind is what lets the policy gate + // auto-approve the app again, which is the whole point of the sweep. + if (!cleared || !optInCleared(autoSigning, pkg)) continue + // Re-read before deleting. Both clears above are blocking keystore and disk + // commits, so the user can refresh this app's window while they run, and the + // isExpired check at the top of the loop does not cover the delete. This + // narrows that window to a single read rather than closing it; a refresh + // landing between this read and the delete still loses the row. + val current = appSettingsDao.getSettings(pkg) ?: continue + if (current.isExpired()) appSettingsDao.delete(pkg) } } + /** + * Whether [callerPackage]'s auto-signing opt-in is durably gone. + * + * Re-approval, not reset-to-global: an app that keeps its opt-in is auto-approved by + * whatever the global policy happens to be on its next request. The clear has to be + * durable rather than fire-and-forget, because the caller drops the settings row on + * the strength of it and the package cannot be enumerated as expired again afterwards. + * No safeguards store this session means it cannot be established, which keeps the row + * for a sweep that can. + * + * Deliberately no `isOptedIn` short-circuit. The encrypted prefs return the default + * when a value cannot be decrypted, so a false read is indistinguishable from an + * opt-in that is still on disk, and skipping the write on that basis would drop the + * row while leaving the opt-in to come back when the read recovers. The cost is one + * removal per expired package. + */ + private fun optInCleared(autoSigning: AutoSigningSafeguards?, callerPackage: String): Boolean = + autoSigning?.clearOptIn(callerPackage) ?: false + /** * Clears [callerPackage]'s core-owned sign-policy override and reports whether that * clear durably persisted.