fix(napplet): make revoking an app actually drop its live grants

Three defects, each of which made revocation look like it worked.

**`revokeSessionGrants` had no callers.** It was added with the KDoc "so
revoking an app takes effect immediately instead of lingering until this
broker instance dies" and then never wired, so revoking an app in
Connected Apps left its in-memory session grants active. The user
revokes; the app keeps signing.

**And it was broken as written.** `sessionAllows` keys are the
account-namespaced `napplet:<signer>:<coordinate>|<op>`, but every revoke
call site holds the BARE coordinate, so the prefix match found nothing.
Wiring it naively would have looked correct and silently done nothing. It
now namespaces before matching, and also clears the post-Cancel re-prompt
cooldown so a revoked app prompts on next use instead of being quietly
dropped.

**Worse: there were three ledgers.** `NappletBrokerService`,
`ConnectedAppsScreen` and `ConnectedAppDetailScreen` each constructed
their own `NappletPermissionLedger`, while ALLOW_SESSION grants are
per-instance in-memory state. So "Forget" cleared the screen's own
always-empty session map while the grants the broker actually consults
lived on. The KDoc described a process-wide singleton; it wasn't one.
Promoted to a real singleton in AppModules alongside the existing
permission store, and shared by all three.

The screens are plain composables with no binder to the broker service,
so rather than invent an IPC path the cached broker moved to the
service's companion under a lock — matching the sibling main-process
registries in that package. Both revoke paths call it: the Forget button
and the per-op revoke.

Also gives `NappletPermissionLedger.endSession()` its first caller, which
promoting the ledger made necessary: it used to die with the service, so
session grants had a natural bound. Now that it outlives the service,
`onDestroy` restores exactly the lifetime ALLOW_SESSION already implied.
The boundary is safe — the service is bind-only and is destroyed only
once every applet and browser surface has unbound, so switching between
two open applets never drops grants mid-use. Deliberately NOT wired to
account switch (already handled by account-keying) or to backgrounding
(would re-prompt mid-use).

Test notes, kept honest: the revoke test was verified to fail before the
namespacing fix. The `endSession` test PASSES without the change —
`endSession` itself was always correct, the bug was that nobody called
it — so it is characterization for the new lifetime contract, not a
regression test. The `onDestroy` wiring and the composable click handlers
have no automated coverage; `amethyst` has no Robolectric and no harness
was invented for them.

Known gap, left alone deliberately: changing an app's trust level to
PARANOID does not drop its live session grants, because `sessionAllows`
is consulted before the signer ledger. That is a revoke-shaped action and
belongs in the same fix, but it is a behaviour change and was out of
scope tonight.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Vitor Pamplona
2026-07-20 09:13:37 -04:00
co-authored by Claude Opus 4.8
parent 62440748a7
commit b0668baeec
7 changed files with 216 additions and 26 deletions
@@ -27,6 +27,7 @@ import androidx.security.crypto.EncryptedSharedPreferences
import coil3.disk.DiskCache
import coil3.memory.MemoryCache
import com.vitorpamplona.amethyst.commons.model.NoteState
import com.vitorpamplona.amethyst.commons.napplet.permissions.NappletPermissionLedger
import com.vitorpamplona.amethyst.commons.relayClient.BlockedRelayFilteringClient
import com.vitorpamplona.amethyst.commons.richtext.CachedRichTextParser
import com.vitorpamplona.amethyst.commons.robohash.CachedRobohash
@@ -704,6 +705,18 @@ class AppModules(
// Singleton stores for napplet permissions — DataStore v1 enforces one instance per file.
val nappletPermissionStore by lazy { DataStoreNappletPermissionStore(appContext, nappletAccountScope) }
/**
* The one napplet permission ledger for the main process. Its persistent half is just the store
* above, but it also holds the in-memory ALLOW_SESSION grants — and *those* only work if every
* caller shares this instance. The broker service and the Connected Apps screens used to build
* a ledger each, so a "Forget"/revoke tapped in the UI cleared the screen's own (always empty)
* session map while the grants the broker was actually consulting lived on untouched.
*
* Session lifetime is bounded by [com.vitorpamplona.amethyst.napplet.NappletBrokerService]'s
* onDestroy (all applet/browser surfaces gone), which calls `endSession()`.
*/
val nappletPermissionLedger by lazy { NappletPermissionLedger(nappletPermissionStore, nappletAccountScope) }
// NOT account-scoped here on purpose: this store is shared with NIP-46, whose coordinates already
// carry their owning account (`nip46:<signer>:<client>`) and whose sessions run for a specific
// account rather than the active one. The napplet path namespaces its own coordinate the same way
@@ -40,7 +40,6 @@ import com.vitorpamplona.amethyst.commons.napplet.NappletBroker
import com.vitorpamplona.amethyst.commons.napplet.NappletCapability
import com.vitorpamplona.amethyst.commons.napplet.NappletIdentity
import com.vitorpamplona.amethyst.commons.napplet.NappletRequestRouter
import com.vitorpamplona.amethyst.commons.napplet.permissions.NappletPermissionLedger
import com.vitorpamplona.amethyst.commons.napplet.protocol.NappletProtocolJson
import com.vitorpamplona.amethyst.commons.napplet.protocol.NappletResponse
import com.vitorpamplona.amethyst.favorites.BrowserHistoryRegistry
@@ -78,8 +77,10 @@ import kotlinx.coroutines.launch
class NappletBrokerService : Service() {
private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
// One ledger for the whole service lifetime: persistent grants on disk, session grants in RAM.
private val ledger by lazy { NappletPermissionLedger(Amethyst.instance.nappletPermissionStore, Amethyst.instance.nappletAccountScope) }
// Persistent grants on disk, session grants in RAM. Shared app-wide (see AppModules) so the
// Connected Apps screens revoke the very grants this broker consults; session grants are dropped
// in onDestroy, which is the "all applet surfaces closed" boundary.
private val ledger get() = Amethyst.instance.nappletPermissionLedger
// Per-app internal-signer permission ledger (policy + per-op overrides). Lazy so it's only
// instantiated in the main process where the signer lives; never touched from :napplet.
@@ -90,9 +91,6 @@ class NappletBrokerService : Service() {
private val incoming by lazy { Messenger(Handler(Looper.getMainLooper(), ::handleMessage)) }
// The broker for the current account, rebuilt only on account switch (see broker()).
private var cachedBroker: Pair<Account, NappletBroker>? = null
// Live relay subscriptions, keyed by the applet's subId. The account comes per-open from the
// requesting surface's launch token, so a surface's REQs always target the account it acts as.
private val liveSubscriptions = NappletLiveSubscriptions()
@@ -120,6 +118,13 @@ class NappletBrokerService : Service() {
override fun onDestroy() {
liveSubscriptions.closeAll()
identityWatch.stop()
// Every applet/browser surface has unbound, so the "session" the user granted for is over.
// The ledger and the broker cache are now app-wide singletons that outlive this service, so
// their in-memory session grants have to be dropped explicitly here — that keeps the lifetime
// the consent dialog promises ("allow for this session") instead of letting it become
// "allow until the app process dies".
dropCachedBroker()
Amethyst.instance.nappletPermissionLedger.endSession()
// Drop any foreground holds this broker still owns so they don't leak past the service.
synchronized(foregroundLeases) {
repeat(foregroundLeases.size) { SandboxForegroundHold.release() }
@@ -359,23 +364,24 @@ class NappletBrokerService : Service() {
* Returns null when that account is no longer loaded (logged out), so requests fail closed
* rather than silently falling back to someone else's key.
*/
@Synchronized
private fun brokerFor(accountPubKey: HexKey): NappletBroker? {
val account = accountFor(accountPubKey) ?: return null
cachedBroker?.let { (acc, broker) -> if (acc === account) return broker }
val broker =
AccountNappletGateways(
account = account,
context = applicationContext,
ledger = ledger,
storage = storage,
// Per-applet Tor decision (see NappletResourceFetcher): the shared manager routes
// through Tor when asked + active, and falls back to clearnet otherwise.
httpClient = { useProxy -> Amethyst.instance.okHttpClients.getHttpClient(useProxy) },
signerLedger = signerLedger,
).broker()
cachedBroker = account to broker
return broker
synchronized(brokerLock) {
cachedBroker?.let { (acc, broker) -> if (acc === account) return broker }
val broker =
AccountNappletGateways(
account = account,
context = applicationContext,
ledger = ledger,
storage = storage,
// Per-applet Tor decision (see NappletResourceFetcher): the shared manager routes
// through Tor when asked + active, and falls back to clearnet otherwise.
httpClient = { useProxy -> Amethyst.instance.okHttpClients.getHttpClient(useProxy) },
signerLedger = signerLedger,
).broker()
cachedBroker = account to broker
return broker
}
}
/**
@@ -430,6 +436,38 @@ class NappletBrokerService : Service() {
}
companion object {
/**
* Guards [cachedBroker]. Both live on the companion rather than the service instance so the
* Connected Apps UI can reach the running broker to revoke its live session grants — the
* screens are plain composables with no binder to this service, and the broker is the only
* holder of the in-memory "allow for this session" signer grants.
*
* Main-process only, like the sibling `Napplet*Registry` objects: the `:napplet` process gets
* its own (unused, empty) copy of these statics and must never touch them.
*/
private val brokerLock = Any()
// The broker for the current account, rebuilt only on account switch (see brokerFor()).
private var cachedBroker: Pair<Account, NappletBroker>? = null
/**
* Drops the live "allow for this session" signer grants the running broker holds for
* [coordinate] (the bare app coordinate). Called when the user revokes or forgets an app in
* Connected Apps: without it the persisted grants are cleared but the in-memory session ones
* keep authorizing signatures until the broker dies, so a revoked app goes on signing.
*
* No-op when no broker has been built yet (no applet has run this process).
*/
suspend fun revokeSessionGrants(coordinate: String) {
val broker = synchronized(brokerLock) { cachedBroker?.second } ?: return
broker.revokeSessionGrants(coordinate)
}
/** Forgets the cached broker, dropping every session grant it holds. */
private fun dropCachedBroker() {
synchronized(brokerLock) { cachedBroker = null }
}
/**
* Sentinel "author" for a browser-mode per-origin identity. The real key is the visited origin,
* carried in the identity's identifier (which the consent dialog shows); this constant only fills
@@ -82,6 +82,7 @@ import com.vitorpamplona.amethyst.commons.napplet.permissions.NappletPermissionL
import com.vitorpamplona.amethyst.favorites.BrowserIconRegistry
import com.vitorpamplona.amethyst.favorites.rememberManifestIconModel
import com.vitorpamplona.amethyst.favorites.rememberWebAppIconModel
import com.vitorpamplona.amethyst.napplet.NappletBrokerService
import com.vitorpamplona.amethyst.napplet.counterpartyLabel
import com.vitorpamplona.amethyst.napplet.descriptionRes
import com.vitorpamplona.amethyst.napplet.labelRes
@@ -118,7 +119,7 @@ fun ConnectedAppDetailScreen(
accountViewModel: AccountViewModel,
nav: INav,
) {
val capabilityLedger = remember { NappletPermissionLedger(Amethyst.instance.nappletPermissionStore, Amethyst.instance.nappletAccountScope) }
val capabilityLedger = Amethyst.instance.nappletPermissionLedger
val signerLedger = remember { NostrSignerPermissionLedger(Amethyst.instance.signerPermissionStore) }
val untitled = stringResource(CommonsR.string.napplet_untitled)
@@ -239,7 +240,14 @@ fun ConnectedAppDetailScreen(
OpOverrideRow(
opKey = opKey,
decision = decision,
onRevoke = { mutate { signerLedger.revokeOpDecision(coordinate, NostrSignerOp.fromKey(opKey) ?: return@mutate) } },
onRevoke = {
mutate {
signerLedger.revokeOpDecision(coordinate, NostrSignerOp.fromKey(opKey) ?: return@mutate)
// The persisted override is gone, but a live "allow for this session"
// grant would keep authorizing this app until the broker dies.
NappletBrokerService.revokeSessionGrants(coordinate)
}
},
)
}
}
@@ -295,6 +303,11 @@ fun ConnectedAppDetailScreen(
signerLedger.revokeAll(coordinate)
}
capabilityLedger.revokeAll(identity)
// Forgetting an app has to stop it signing *now*. The two ledgers above only
// drop persisted + capability grants; the broker separately holds the signer's
// in-memory "allow for this session" grants, which would otherwise keep the
// app authorized for as long as any applet surface stays open.
NappletBrokerService.revokeSessionGrants(coordinate)
}
nav.popBack()
},
@@ -96,7 +96,7 @@ fun ConnectedAppsScreen(
accountViewModel: AccountViewModel,
nav: INav,
) {
val capabilityLedger = remember { NappletPermissionLedger(Amethyst.instance.nappletPermissionStore, Amethyst.instance.nappletAccountScope) }
val capabilityLedger = Amethyst.instance.nappletPermissionLedger
val signerLedger = remember { NostrSignerPermissionLedger(Amethyst.instance.signerPermissionStore) }
var items by remember { mutableStateOf<List<ConnectedAppEntry>?>(null) }
@@ -470,7 +470,10 @@ class NappletBroker(
* an "always allow" granted by one npub can never authorize signing under another. The bare
* [NappletIdentity.coordinate] stays the app's display/UI identity and is unchanged.
*/
private fun signerCoordinateFor(identity: NappletIdentity): String = "napplet:${signer.pubKey}:${identity.coordinate}"
private fun signerCoordinateFor(identity: NappletIdentity): String = signerCoordinateFor(identity.coordinate)
/** [signerCoordinateFor] for a bare app coordinate — what the Connected Apps UI holds. */
private fun signerCoordinateFor(coordinate: String): String = "napplet:${signer.pubKey}:$coordinate"
/**
* Namespaces an in-memory session grant to the applet that was actually prompted for. Mirrors
@@ -485,10 +488,20 @@ class NappletBroker(
/**
* Drops every live session grant held for [coordinate], so revoking an app takes effect
* immediately instead of lingering until this broker instance dies.
*
* [coordinate] is the **bare** app coordinate (`<author>:<identifier>`, or `browser:<origin>`)
* the identity the Connected Apps UI holds. Session keys are stored under the
* account-namespaced [signerCoordinateFor] form, so this namespaces before matching; comparing
* the bare coordinate against those keys would silently match nothing and revoke nothing.
*
* Also clears any post-Cancel re-prompt suppression for the app: after an explicit revoke the
* user's next interaction should prompt, not be dropped by a cooldown from before.
*/
suspend fun revokeSessionGrants(coordinate: String) {
signerConsentLock.withLock {
sessionAllows.removeAll { it.startsWith("$coordinate|") }
val prefix = "${signerCoordinateFor(coordinate)}|"
sessionAllows.removeAll { it.startsWith(prefix) }
cancelledUntil.remove(coordinate)
}
}
@@ -24,7 +24,10 @@ import com.vitorpamplona.amethyst.commons.connectedApps.signers.AppConnectResult
import com.vitorpamplona.amethyst.commons.connectedApps.signers.AppSignerPolicy
import com.vitorpamplona.amethyst.commons.connectedApps.signers.InMemoryNostrSignerPermissionStore
import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrConnectPrompt
import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerConsentPrompt
import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerOp
import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerPermissionLedger
import com.vitorpamplona.amethyst.commons.connectedApps.signers.SignerOpGrant
import com.vitorpamplona.amethyst.commons.napplet.permissions.GrantState
import com.vitorpamplona.amethyst.commons.napplet.permissions.InMemoryNappletPermissionStore
import com.vitorpamplona.amethyst.commons.napplet.permissions.NappletPermissionLedger
@@ -308,6 +311,96 @@ class NappletBrokerTest {
}
}
/** A per-op signer consent prompt that always answers [answer] and counts its calls. */
private class ScriptedSignerPrompt(
private val answer: SignerOpGrant,
) : NostrSignerConsentPrompt {
var calls = 0
private set
override suspend fun request(
identity: NappletIdentity,
op: NostrSignerOp,
request: NappletRequest,
): SignerOpGrant {
calls++
return answer
}
}
@Test
fun revokingAnAppDropsItsLiveSessionSignerGrants() =
runTest {
// Regression: "allow for this session" grants live in the broker, keyed by the
// account-namespaced coordinate (`napplet:<signer>:<coordinate>|<op>`), while the
// Connected Apps UI only ever holds the BARE coordinate. Revoking used to clear the
// persisted ledgers and leave the session grants matching nothing — so a revoked app
// kept signing for as long as any applet surface stayed open.
val signerLedger = NostrSignerPermissionLedger(InMemoryNostrSignerPermissionStore())
// PARANOID asks for every op, so each publish either prompts or rides a session grant.
signerLedger.setPolicy("napplet:${signer.pubKey}:${applet.coordinate}", AppSignerPolicy.PARANOID)
val opPrompt = ScriptedSignerPrompt(SignerOpGrant.AllowForSession(NostrSignerOp.SignKind(1)))
val broker =
NappletBroker(
signer = signer,
ledger = NappletPermissionLedger(InMemoryNappletPermissionStore()),
consentPrompt = ScriptedPrompt(GrantState.ALLOW_ALWAYS),
relay = RecordingRelay(),
signerLedger = signerLedger,
signerConsentPrompt = opPrompt,
)
val publish = NappletRequest.Publish(kind = 1, tags = arrayOf(arrayOf("t", "napplet")), content = "gm")
// 1. First publish prompts, and the user allows for the session.
assertIs<NappletResponse.Published>(broker.handle(applet, publish, allDeclared))
assertEquals(1, opPrompt.calls)
// 2. The session grant carries the next publish with no prompt — that's the point of it.
assertIs<NappletResponse.Published>(broker.handle(applet, publish, allDeclared))
assertEquals(1, opPrompt.calls)
// 3. The user revokes the app in Connected Apps, which passes the bare coordinate.
broker.revokeSessionGrants(applet.coordinate)
// 4. ...so the app has to ask again rather than riding the dead grant.
assertIs<NappletResponse.Published>(broker.handle(applet, publish, allDeclared))
assertEquals(2, opPrompt.calls)
}
@Test
fun revokingOneAppLeavesAnotherAppsSessionGrantsAlone() =
runTest {
// The prefix match must not be so loose that revoking one app disarms every other one
// the user is still using.
val other = NappletIdentity(authorPubKey = "bb".repeat(32), identifier = "other")
val signerLedger = NostrSignerPermissionLedger(InMemoryNostrSignerPermissionStore())
signerLedger.setPolicy("napplet:${signer.pubKey}:${applet.coordinate}", AppSignerPolicy.PARANOID)
signerLedger.setPolicy("napplet:${signer.pubKey}:${other.coordinate}", AppSignerPolicy.PARANOID)
val opPrompt = ScriptedSignerPrompt(SignerOpGrant.AllowForSession(NostrSignerOp.SignKind(1)))
val broker =
NappletBroker(
signer = signer,
ledger = NappletPermissionLedger(InMemoryNappletPermissionStore()),
consentPrompt = ScriptedPrompt(GrantState.ALLOW_ALWAYS),
relay = RecordingRelay(),
signerLedger = signerLedger,
signerConsentPrompt = opPrompt,
)
val publish = NappletRequest.Publish(kind = 1, tags = arrayOf(arrayOf("t", "napplet")), content = "gm")
broker.handle(applet, publish, allDeclared) // applet prompts once
broker.handle(other, publish, allDeclared) // other prompts once
assertEquals(2, opPrompt.calls)
broker.revokeSessionGrants(applet.coordinate)
// The other app is untouched and still rides its own session grant.
assertIs<NappletResponse.Published>(broker.handle(other, publish, allDeclared))
assertEquals(2, opPrompt.calls)
}
@Test
fun aGrantFromOneAccountNeverAuthorizesTheSameAppUnderAnother() =
runTest {
@@ -52,6 +52,26 @@ class NappletPermissionLedgerTest {
assertEquals(PermissionDecision.ALLOW, ledger.decide(applet, NappletCapability.RELAY))
}
@Test
fun endSessionDropsSessionGrantsAcrossAppsButKeepsPersistedOnes() =
runTest {
// What the broker service calls in onDestroy, once every applet/browser surface has
// unbound. The ledger is now an app-wide singleton that outlives the service, so this is
// what keeps ALLOW_SESSION meaning "this session" rather than "until the process dies".
val store = InMemoryNappletPermissionStore()
val ledger = ledger(store)
ledger.record(applet, NappletCapability.RELAY, GrantState.ALLOW_SESSION)
ledger.record(other, NappletCapability.IDENTITY, GrantState.ALLOW_SESSION)
ledger.record(applet, NappletCapability.STORAGE, GrantState.ALLOW_ALWAYS)
ledger.endSession()
assertEquals(PermissionDecision.ASK, ledger.decide(applet, NappletCapability.RELAY))
assertEquals(PermissionDecision.ASK, ledger.decide(other, NappletCapability.IDENTITY))
// The user's persisted decision is untouched — ending a session is not a revoke.
assertEquals(PermissionDecision.ALLOW, ledger.decide(applet, NappletCapability.STORAGE))
}
@Test
fun unknownGrantDefaultsToAsk() =
runTest {