Move per-app sign-policy overrides to the core-owned store - #456
Move per-app sign-policy overrides to the core-owned store#456kwsantiago wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
WalkthroughThe change centralizes per-app sign-policy overrides across Room and the core store, adds migration and lifecycle cleanup handling, updates policy consumers, and introduces instrumented coverage for precedence, expiry, account switching, and migration behavior. ChangesSign-policy override migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AppPermissionsScreen
participant AppSignPolicyOverrides
participant SignPolicyStore
participant PermissionStore
AppPermissionsScreen->>AppSignPolicyOverrides: read effective app policy
AppSignPolicyOverrides->>SignPolicyStore: check core override
AppSignPolicyOverrides->>PermissionStore: fall back to Room override
AppPermissionsScreen->>AppSignPolicyOverrides: persist selected policy
AppSignPolicyOverrides->>SignPolicyStore: write core override
AppSignPolicyOverrides->>PermissionStore: mirror Room override
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt (1)
33-56: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winBulk
deleteExpired()discards the retry safety net thatclearAllAppSettingsdeliberately implements.Inside the
forEach, a failedsignPolicyStore?.setAppOverride(pkg, null)is swallowed byrunCatchingwith the result discarded. Regardless of that outcome,appSettingsDao.deleteExpired(now, nowElapsed)runs unconditionally afterward and removes every expirednip55_app_settingsrow — including ones whose core override clear just failed.That row is the only index tracking that the package still holds a core override (per the docstring on
PermissionStore.clearAllAppSettings, which correctly keeps a row when its core clear fails so a later wipe can retry it). Deleting it here regardless strands the stale override in the core, permanently invisible to Kotlin and unrecoverable by any latercleanupExpired/clearAllAppSettingsrun — a security-relevant regression, since an expired app's (potentially looser, e.g.AUTO) override should be revoked but can now persist indefinitely after one transient core failure.🐛 Proposed fix — mirror `clearAllAppSettings`'s per-package retry semantics
suspend fun cleanupExpired(signPolicyStore: SignPolicyStore? = null) { val now = System.currentTimeMillis() val nowElapsed = SystemClock.elapsedRealtime() auditWriter.prune(now - 30 * DAY_MS) { dao.deleteExpired(now, nowElapsed) dao.deleteNip46Permissions() val expiredPackages = appSettingsDao.getExpiredPackages(now, nowElapsed) expiredPackages.forEach { pkg -> dao.deleteForCaller(pkg) - runCatching { signPolicyStore?.setAppOverride(pkg, null) } + if (runCatching { signPolicyStore?.setAppOverride(pkg, null) }.isSuccess) { + appSettingsDao.delete(pkg) + } } - appSettingsDao.deleteExpired(now, nowElapsed) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt` around lines 33 - 56, Update cleanupExpired around the expiredPackages loop and appSettingsDao.deleteExpired so each package’s settings row is removed only after signPolicyStore.setAppOverride(pkg, null) succeeds or no sign-policy store is provided. Preserve failed rows for a later retry, while continuing to delete unrelated expired records and successfully cleared package settings.
🧹 Nitpick comments (4)
app/src/androidTest/kotlin/io/privkey/keep/nip55/AppSignPolicyOverridesInstrumentedTest.kt (2)
165-207: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMissing coverage for the "cleanup failure retains the row for retry" guarantee.
Both expiry-sweep tests only exercise the happy path where the core clear succeeds. Per
PermissionStore.cleanupExpired(see snippet),appSettingsDao.deleteExpired(now, nowElapsed)runs unconditionally over all expired packages after the per-package loop, regardless of whetherrunCatching { signPolicyStore?.setAppOverride(pkg, null) }succeeded or threw for a given package. That appears to contradict the PR's stated invariant that "Database rows remain as the override index and are retained when cleanup fails so later retries remain possible" — nothing in the current implementation seems to skip deleting a package's row when its core clear failed.None of the tests here inject a core-clear failure to confirm the row actually survives for a retry. Given this is called out as a deliberate safety property in the PR description, it's worth adding a test with a failing/faulty
SignPolicyStoredouble to confirm the Room row is retained (and effectivePolicy remains correctly resolved via fallback) when the core clear throws.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/kotlin/io/privkey/keep/nip55/AppSignPolicyOverridesInstrumentedTest.kt` around lines 165 - 207, Add a failure-path test alongside expirySweepClearsTheCoreOverride using a faulty SignPolicyStore double that throws when clearing the app override. Verify cleanupExpired retains the expired Room row for retry, the core override remains or falls back as intended, and effectivePolicy resolves through the documented fallback; use the existing expirySweep and PermissionStore test symbols without changing the happy-path assertions.
57-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the existing prefs name constants in cleanup.
SignPolicySelectionPrefsdefinesPREFS_NAME,LEGACY_PREFS_NAME, andMARKER_PREFS_NAME;clearPrefs()should reuse those instead of hardcoding the shared-prefs names so the cleanup target stays locked to the production source of truth.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/androidTest/kotlin/io/privkey/keep/nip55/AppSignPolicyOverridesInstrumentedTest.kt` around lines 57 - 66, Update clearPrefs() to use SignPolicySelectionPrefs.PREFS_NAME and SignPolicySelectionPrefs.LEGACY_PREFS_NAME instead of SELECTION_PREFS and LEGACY_PREFS, while retaining SignPolicySelectionPrefs.MARKER_PREFS_NAME and MIGRATION_MARKER for the marker cleanup.app/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt (1)
438-452: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate KDoc block.
Two separate doc comments stack immediately before
clearAllAppSettings(438-443 and 444-452) describing the same behavior; only the second is actually attached as the function's KDoc. Looks like a merge leftover — worth consolidating into one.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt` around lines 438 - 452, Remove the duplicate KDoc immediately before clearAllAppSettings and consolidate any unique behavior details into a single attached documentation block. Preserve the explanation of clearing core sign-policy overrides before deleting rows and retaining rows when clearing fails.app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt (1)
101-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWhole-loop
runCatchingaborts remaining packages on the first failure.If
core.appOverride/core.setAppOverridethrows for one package mid-loop, therunCatchingaround the entireforloop stops migration for every package still pending in that iteration, not just the failing one. It's harmless (the Room fallback keeps working, per the docstring) but means those apps also miss the chance to be migrated this run for no reason related to their own state.♻️ Suggested per-package isolation
suspend fun migrateLegacyOverrides(core: SignPolicyStore, permissions: PermissionStore) { - runCatching { - for (settings in permissions.getAllAppSettings()) { - val ordinal = settings.signPolicyOverride ?: continue - 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) - } + runCatching { permissions.getAllAppSettings() } + .onFailure { if (BuildConfig.DEBUG) Log.w(TAG, "Sign-policy override migration failed", it) } + .getOrNull() + ?.forEach { settings -> + val ordinal = settings.signPolicyOverride ?: return@forEach + if (settings.isExpired()) return@forEach + runCatching { + if (core.appOverride(settings.callerPackage) == null) { + core.setAppOverride(settings.callerPackage, SignPolicy.fromOrdinal(ordinal).toSelection()) + } + }.onFailure { if (BuildConfig.DEBUG) Log.w(TAG, "Migration failed for ${settings.callerPackage}", it) } + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt` around lines 101 - 118, Update migrateLegacyOverrides so each package’s migration work is isolated in its own runCatching scope, allowing the loop to continue when core.appOverride or core.setAppOverride fails for one package. Preserve the existing expired-row and already-migrated checks, and log failures with the affected package context while retaining the DEBUG-only behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt`:
- Around line 27-53: Guard the core reads in AppSignPolicyOverrides.override and
effectivePolicy so unexpected failures from appOverride or globalPolicy are
caught rather than propagated through the background auto-sign path. On a failed
app override read, continue with legacyOverride; on a failed globalPolicy read,
fall back to SignPolicySelection.MANUAL, preserving the existing precedence and
fail-closed behavior.
---
Outside diff comments:
In `@app/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt`:
- Around line 33-56: Update cleanupExpired around the expiredPackages loop and
appSettingsDao.deleteExpired so each package’s settings row is removed only
after signPolicyStore.setAppOverride(pkg, null) succeeds or no sign-policy store
is provided. Preserve failed rows for a later retry, while continuing to delete
unrelated expired records and successfully cleared package settings.
---
Nitpick comments:
In
`@app/src/androidTest/kotlin/io/privkey/keep/nip55/AppSignPolicyOverridesInstrumentedTest.kt`:
- Around line 165-207: Add a failure-path test alongside
expirySweepClearsTheCoreOverride using a faulty SignPolicyStore double that
throws when clearing the app override. Verify cleanupExpired retains the expired
Room row for retry, the core override remains or falls back as intended, and
effectivePolicy resolves through the documented fallback; use the existing
expirySweep and PermissionStore test symbols without changing the happy-path
assertions.
- Around line 57-66: Update clearPrefs() to use
SignPolicySelectionPrefs.PREFS_NAME and
SignPolicySelectionPrefs.LEGACY_PREFS_NAME instead of SELECTION_PREFS and
LEGACY_PREFS, while retaining SignPolicySelectionPrefs.MARKER_PREFS_NAME and
MIGRATION_MARKER for the marker cleanup.
In `@app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt`:
- Around line 101-118: Update migrateLegacyOverrides so each package’s migration
work is isolated in its own runCatching scope, allowing the loop to continue
when core.appOverride or core.setAppOverride fails for one package. Preserve the
existing expired-row and already-migrated checks, and log failures with the
affected package context while retaining the DEBUG-only behavior.
In `@app/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt`:
- Around line 438-452: Remove the duplicate KDoc immediately before
clearAllAppSettings and consolidate any unique behavior details into a single
attached documentation block. Preserve the explanation of clearing core
sign-policy overrides before deleting rows and retaining rows when clearing
fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 74649ba1-5386-4c1d-b4da-d0f888bdfd07
📒 Files selected for processing (7)
app/src/androidTest/kotlin/io/privkey/keep/nip55/AppSignPolicyOverridesInstrumentedTest.ktapp/src/main/kotlin/io/privkey/keep/KeepMobileApp.ktapp/src/main/kotlin/io/privkey/keep/MainActivity.ktapp/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.ktapp/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.ktapp/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.ktapp/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt
| /** | ||
| * The override in force: the core store first, the legacy Room row as a fallback. | ||
| * Yields null only when neither store holds one, so an incomplete or failed | ||
| * migration can never drop an app onto the looser global policy. | ||
| */ | ||
| suspend fun override( | ||
| core: SignPolicyStore?, | ||
| permissions: PermissionStore, | ||
| callerPackage: String | ||
| ): SignPolicySelection? = | ||
| core?.appOverride(callerPackage) ?: legacyOverride(permissions, callerPackage) | ||
|
|
||
| /** | ||
| * 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 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'AppSignPolicyOverrides\.kt|Nip55ContentProvider|SignPolicyStore|PermissionStore|runWithTimeout|withTimeoutOrNull' || true
echo "== AppSignPolicyOverrides outline/contents =="
if [ -f app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt ]; then
ast-grep outline app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt || true
echo "--- lines 1-180 ---"
nl -ba app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt | sed -n '1,180p'
fi
echo "== content provider runWithTimeout section =="
rg -n "runWithTimeout|withTimeoutOrNull|decideBackgroundRequest|effectivePolicy\\(|override\\(" -S .Repository: privkeyio/keep-android
Length of output: 756
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Kotlin timeout behavior probe =="
cat > /tmp/timeout_probe.kt <<'KT'
import kotlinx.coroutines.*
fun main() = runBlocking {
println("non-cancellation exception propagation:")
try {
runWithTimeout(200) {
coroutineScope {
val deferred = async { error("boom") }
deferred.await()
}
}
println("no exception observed")
} catch (e: CancellationException) {
println("cancellation: ${e::class.simpleName}: ${e.message}")
} catch (e: Exception) {
println("other: ${e::class.simpleName}: ${e.message}")
}
}
KT
kotlinc /tmp/timeout_probe.kt -cp "$KOTLIN_HOME/jre/lib/rt.jar" 2>&1 | head -80 || true
echo "== Kotlin exception handling summary from stdlib docs not applicable offline; rely on repo code only =="Repository: privkeyio/keep-android
Length of output: 345
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file sizes =="
wc -l app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt \
app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt \
app/src/main/kotlin/io/privkey/keep/nip55/PermissionStore.kt \
app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt
echo "== AppSignPolicyOverrides.kt =="
cat -n app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt
echo "== Nip55ContentProvider.kt relevant sections =="
rg -n "runWithTimeout|withTimeoutOrNull|decideBackgroundRequest|effectivePolicy\\(|override\\(|query\\(|runCatching|try\\s*\\{" app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt
echo "--- lines 1-240 ---"
cat -n app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt | sed -n '1,240p'Repository: privkeyio/keep-android
Length of output: 22472
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Nip55ContentProvider lines 240-330 =="
cat -n app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt | sed -n '240,330p'
echo "== SignPolicyStore declarations/usages =="
rg -n "interface SignPolicyStore|class SignPolicyStore|fun appOverride\\(|fun globalPolicy\\(|suspend fun appOverride\\(|suspend fun globalPolicy\\(|appOverride\\(|globalPolicy\\(" -S app src || true
echo "== gradle UniFFI/uniffi deps if present =="
rg -n "uniffi|uniffi-bindgen|uniffi-rs|sign-policy|SignPolicyStore" -S . || trueRepository: privkeyio/keep-android
Length of output: 46156
🌐 Web query:
UniFFI Kotlin unexpected Rust panic InternalException RuntimeException non Result function
💡 Result:
In UniFFI-generated Kotlin bindings, when a Rust function panics, the generated scaffolding catches the panic using std::panic::catch_unwind and treats it as an unexpected internal error [1]. How this manifests in Kotlin depends on whether the function is defined as a fallible function (one that returns a Result and can throw errors) or a non-fallible function: 1. InternalException: Traditionally, UniFFI has used an InternalException to represent these unexpected failures [2][3]. In many generated versions, when a non-fallible function panics, the scaffolding explicitly throws an InternalException to signal this state [3]. 2. RuntimeException: For functions that are defined to return a Result (fallible functions), there has historically been a discrepancy where panics might manifest as a generic RuntimeException rather than the InternalException used by non-fallible functions [3]. These exceptions are generally considered indicators of an unrecoverable failure or a bug within the underlying Rust code (or the binding layer itself) [2][3]. They are not intended to be caught or handled as part of standard application logic [2][3]. If you encounter these, it is typically a sign of a Rust panic that has been intercepted by the UniFFI scaffolding. Since UniFFI attempts to prevent panics from crossing the FFI boundary—which would otherwise cause a crash—it converts them into these exceptions to be thrown in the foreign (Kotlin) code [1][2]. The InternalException is the designated type for these internal errors, and discrepancies where RuntimeException appears are generally recognized as bugs or inconsistencies in the binding generation [3]. To avoid these, ensure that all Rust code is panic-free and that any foreign trait methods implemented in Kotlin properly return expected error types, as failing to do so can cause the generated scaffolding to panic [4].
Citations:
- 1: https://mozilla.github.io/uniffi-rs/latest/internals/rust_calls.html
- 2: Document how unexpected errors are generated mozilla/uniffi-rs#982
- 3: Error returning functions will throw a different exception when panic'ing than non-error returning functions in kotlin mozilla/uniffi-rs#485
- 4: https://mozilla.github.io/uniffi-rs/latest/foreign_traits.html
Unguarded core FFI calls in the background auto-sign read path.
override() and effectivePolicy() call core.appOverride(...) and core.globalPolicy() without handling unexpected exceptions. Since effectivePolicy() is used by Nip55ContentProvider.decideBackgroundRequest, an unexpected core-FFI failure can escape runWithTimeout { ... } instead of being treated as “not a read result” and failing closed. Catch failed core reads here and fall back to the legacy/default policy path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/kotlin/io/privkey/keep/nip55/AppSignPolicyOverrides.kt` around
lines 27 - 53, Guard the core reads in AppSignPolicyOverrides.override and
effectivePolicy so unexpected failures from appOverride or globalPolicy are
caught rather than propagated through the background auto-sign path. On a failed
app override read, continue with legacyOverride; on a failed globalPolicy read,
fall back to SignPolicySelection.MANUAL, preserving the existing precedence and
fail-closed behavior.
… verify override clears
|
Converting to draft. Not mergeable as designed, and I am not going to patch it a fourth time. Three review rounds narrowed the two critical paths but did not close them, and each round surfaced a subtler variant rather than converging. The reason is structural rather than incidental: What survived review as genuinely correct and worth keeping:
What is still open, and why none of it is safe to ship:
The correct sequencing is to make the storage trait fallible in the core first, so a write or clear can report whether it persisted, and then redo the client migration on top of that. With a trustworthy signal the mirror row, the ordering rules and most of the enumeration machinery collapse into something much smaller. I have filed that as the prerequisite. Worth noting explicitly: the instrumented suite was green on the commit that carried both critical defects, and one test asserted the orphaned state as expected behaviour. Green CI was never going to catch this class. |
|
Unblocked. The prerequisite landed in the core (privkeyio/keep#919): the selection storage trait now reports whether a write durably persisted, and both setters propagate that result. This branch should be rebuilt on that rather than continued. With a trustworthy write signal, most of what made this version fragile disappears: the mirror row acting as an index, the different ordering rules for set versus clear, and the enumeration that had to reach packages whose row was already gone. The parts worth carrying over are the ones review accepted, namely resolving a disagreement between the two stores toward the stricter value, taking the core override down with the row in both the expiry sweep and the account-switch wipe, and keeping the blocking keystore commits out of the Room transaction. Two contract points to honour when reimplementing, both of which caused defects here:
Still absent on the core side: any way to enumerate per-app overrides, so a client index remains necessary to reconcile an orphan. |
1 participant
Summary
Completes the move of signing-policy selection into the core. The global selection moved previously; per-app overrides still lived only in the permissions database, so the core could not resolve a package's effective policy on its own and a second platform would have had to reimplement the override storage.
The two halves carry opposite risk, which shapes the design here. Losing the global value is harmless because it defaults to Manual, the strictest tier. Losing a per-app override is not: overrides are typically stricter than the global (an app pinned to Manual under a global Auto), so a lost override silently hands that app the looser global. Nothing in this change may leave an app on a looser effective policy than before.
Two lifecycle paths had to move with the data, or the change would have loosened policy:
Reads do not yet use the core's own effective-policy resolver, because it cannot see the database fallback. That switch belongs with the change that retires the fallback.
Closes privkeyio/keep#716 remaining client work.
Known residual
If clearing a single package's override throws during an account switch, that override survives into the next account. It requires an encrypted-storage write to fail mid-wipe, and it is the same failure class already accepted for the global selection. The row is retained so a later wipe retries it, but the window is real. Closing it properly needs an atomic clear-all on the core store, filed separately.
Test plan
./gradlew compileDebugKotlin compileDebugAndroidTestKotlin testDebugUnitTest lintDebug— BUILD SUCCESSFUL, 162 unit tests, 0 failures, warnings treated as errors.Summary by CodeRabbit
Bug Fixes
Tests