Make an expired window require re-approval instead of resetting to the global policy

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"
     }
 }

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.
                 //

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)
                         }
                     }
                 )

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()
     }

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)
             }

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()
 

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<String>()
@@ -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.