From 2ccd837f30b258373045fd3af14df60f52edd709 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Sun, 19 Jul 2026 19:38:52 -0400 Subject: [PATCH] fix(napplet): sign as the account a surface was launched as MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Requests resolved their signer through `sessionManager.loggedInAccount()` — whichever account is active *right now* — with no binding to the surface that asked. A full-screen host is a separate activity that an account switch does not tear down, so: - Open a site full-screen as A and log in via NIP-07: the page shows A. - Switch to B in the main app. - Return to that still-open surface and request a signature: the broker handed it **B's** key. Confirmed on device before the fix. Worse than a mismatched prompt: B's session was then written into **A's** WebView storage jar, so afterwards even the embedded tab — which rebuilds correctly and had been verified correct — displayed the wrong account. Per-account isolation held only until a full-screen surface wrote a foreign session into a jar. And it happened silently, because the ledger is per-account+origin and B had already granted "always allow" for that origin from an earlier session. `NappletLaunchRegistry.Session` now carries the account that minted the token, and the broker resolves *that* account out of the cache. This needs no new machinery to satisfy both halves of the rule: embedded surfaces are torn down and re-minted on a switch, so they follow the active account, while a full-screen surface keeps the account it was opened with. It also extends an argument the code already made — the sandbox can only act as the napplet it was launched as, because it holds only its own token; now the same is true of the account. Fails closed: if the launch account is no longer loaded, the request is refused rather than falling back to whoever is signed in now. The same live-account resolution existed on two adjacent paths, fixed here too: - Relay subscriptions took the account from a global supplier, so a full-screen surface's REQs would target the newly-active account's relays while its signatures came from the old one. The account is now passed per-open from the launch token. - `identity.changed` streamed the app's active account, so a page bound to A could be told it had become B while signatures still returned A — the same desync inverted. It is now bound to the surface's own account and reports only that account going away. Verified on device: with B active, a fresh identity read from a full-screen surface launched as A returns **A**; the embedded tab still follows B; both surfaces ran simultaneously under different accounts with no cross-writes between jars. Co-Authored-By: Claude Opus 4.8 --- .../amethyst/napplet/NappletBrokerService.kt | 61 +++++++++++++------ .../amethyst/napplet/NappletIdentityWatch.kt | 9 ++- .../amethyst/napplet/NappletLaunchRegistry.kt | 16 ++++- .../amethyst/napplet/NappletLauncher.kt | 11 +++- .../napplet/NappletLiveSubscriptions.kt | 13 ++-- 5 files changed, 82 insertions(+), 28 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletBrokerService.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletBrokerService.kt index 0367a3c9c2..7b37c290b4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletBrokerService.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletBrokerService.kt @@ -50,7 +50,7 @@ import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.napplet.gateways.AccountNappletGateways import com.vitorpamplona.amethyst.napplethost.NappletIpc import com.vitorpamplona.amethyst.ui.MainActivity -import com.vitorpamplona.amethyst.ui.screen.AccountState +import com.vitorpamplona.quartz.nip01Core.core.HexKey import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job @@ -93,17 +93,23 @@ class NappletBrokerService : Service() { // The broker for the current account, rebuilt only on account switch (see broker()). private var cachedBroker: Pair? = null - // Live relay subscriptions, keyed by the applet's subId; reads the current account live. - private val liveSubscriptions = NappletLiveSubscriptions { Amethyst.instance.sessionManager.loggedInAccount() } + // 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() // The app-wide inc pub/sub bus: routes inc.emit between live napplet sessions as inc.event pushes. private val incBus = NappletIncBus { replyTo, payload -> push(replyTo, payload) } - // Streams identity.changed pushes (account switch / connect / disconnect) to a watching applet. + // Streams identity.changed to a watching applet. Bound to the surface's LAUNCH account, not the + // app's active one: a surface acts as the account that opened it for its whole life, so switching + // accounts elsewhere is not an identity change *for it*. Announcing the newly-active pubkey here + // would tell a page it had become someone else while its signatures still came back as the + // original — the same desync the launch binding exists to prevent. What this does still report is + // that account going away (logout/removal), which emits "". private val identityWatch = - NappletIdentityWatch(scope) { - Amethyst.instance.sessionManager.accountContent - .map { (it as? AccountState.LoggedIn)?.account?.signer?.pubKey ?: "" } + NappletIdentityWatch(scope) { boundPubKey -> + Amethyst.instance.accountsCache.accounts + .map { loaded -> if (loaded.containsKey(boundPubKey)) boundPubKey else "" } } // Binding is restricted to our own UID by exported=false in the manifest, enforced by the OS. @@ -241,7 +247,10 @@ class NappletBrokerService : Service() { val replyTo = msg.replyTo ?: return true val origin = data.getString(NappletIpc.KEY_BROWSER_ORIGIN)?.takeIf { it.isNotBlank() } ?: return true val identity = NappletIdentity(authorPubKey = BROWSER_IDENTITY_AUTHOR, identifier = origin) - val token = NappletLaunchRegistry.register(identity, setOf(NappletCapability.IDENTITY, NappletCapability.RELAY)) + // Bind to the account active at mint time: a browser token minted for one account must + // never sign as another if the user switches while the page is still open. + val mintAccount = Amethyst.instance.sessionManager.loggedInAccount() ?: return true + val token = NappletLaunchRegistry.register(identity, setOf(NappletCapability.IDENTITY, NappletCapability.RELAY), mintAccount.pubKey) val response = Message.obtain(null, NappletIpc.MSG_BROWSER_TOKEN).apply { this.data = @@ -278,17 +287,20 @@ class NappletBrokerService : Service() { // The shared, host-agnostic router owns decode → broker → encode and the subscribe-vs-reply // decision (it stays wire-identical with the future desktop host). This service only supplies // the broker, the Messenger transport, and the live relay subscription each Outcome implies. - val broker = broker() + // The launch token decides whose key signs — not the active account. A surface opened by + // one account can never be handed another's signer, even while it stays open across a switch. + val broker = brokerFor(session.accountPubKey) if (broker == null) { - reply(replyTo, requestId, NappletProtocolJson.encodeResponse(requestType, NappletResponse.Failed("No account is signed in."))) + reply(replyTo, requestId, NappletProtocolJson.encodeResponse(requestType, NappletResponse.Failed("That account is no longer signed in."))) return@launch } when (val outcome = NappletRequestRouter.route(broker, identity, declared, payload)) { is NappletRequestRouter.Outcome.Ignore -> {} is NappletRequestRouter.Outcome.Reply -> reply(replyTo, requestId, outcome.payload) - is NappletRequestRouter.Outcome.OpenSubscription -> liveSubscriptions.open(outcome.subId, outcome.filters) { push(replyTo, it) } + is NappletRequestRouter.Outcome.OpenSubscription -> + liveSubscriptions.open(outcome.subId, outcome.filters, accountFor(session.accountPubKey)) { push(replyTo, it) } is NappletRequestRouter.Outcome.CloseSubscription -> liveSubscriptions.close(outcome.subId) - is NappletRequestRouter.Outcome.WatchIdentity -> identityWatch.start { push(replyTo, it) } + is NappletRequestRouter.Outcome.WatchIdentity -> identityWatch.start(session.accountPubKey) { push(replyTo, it) } is NappletRequestRouter.Outcome.UnwatchIdentity -> identityWatch.stop() is NappletRequestRouter.Outcome.Push -> outcome.payloads.forEach { push(replyTo, it) } is NappletRequestRouter.Outcome.SubscribeInc -> incBus.subscribe(replyTo, outcome.topic) @@ -327,14 +339,29 @@ class NappletBrokerService : Service() { } } + /** The launched-as account, or null once it is no longer loaded. */ + private fun accountFor(accountPubKey: HexKey): Account? = Amethyst.instance.accountsCache.accounts.value[accountPubKey] + /** - * The broker for the *currently* signed-in account, cached and rebuilt only when the account - * changes (reference identity). The gateways capture the account and read its flows live, so a - * cached broker stays correct across requests without per-request allocation. + * The broker for the account a surface was LAUNCHED as — [NappletLaunchRegistry.Session.accountPubKey], + * never whichever account is active right now. + * + * Resolving live was wrong in a way that defeated per-account isolation: a full-screen host is a + * separate activity that an account switch does not tear down, so its WebView kept account A's + * cookies while requests were signed by B. The page displayed one identity while another signed, + * and B's session was written into A's storage jar — after which even the embedded tab, which is + * rebuilt correctly, showed the wrong account. + * + * Binding to the launch account satisfies both halves of the rule with no extra machinery: + * embedded surfaces are torn down and re-minted on a switch, so they follow the active account, + * while a full-screen surface stays on the account it was opened with. + * + * 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 broker(): NappletBroker? { - val account = Amethyst.instance.sessionManager.loggedInAccount() ?: return null + private fun brokerFor(accountPubKey: HexKey): NappletBroker? { + val account = accountFor(accountPubKey) ?: return null cachedBroker?.let { (acc, broker) -> if (acc === account) return broker } val broker = AccountNappletGateways( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletIdentityWatch.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletIdentityWatch.kt index 8fc713bf34..f94f7bc2cf 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletIdentityWatch.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletIdentityWatch.kt @@ -39,15 +39,18 @@ import kotlinx.coroutines.launch */ class NappletIdentityWatch( private val scope: CoroutineScope, - private val pubKey: () -> Flow, + private val pubKey: (boundPubKey: String) -> Flow, ) { private var job: Job? = null - fun start(push: (String) -> Unit) { + fun start( + boundPubKey: String, + push: (String) -> Unit, + ) { stop() job = scope.launch { - pubKey() + pubKey(boundPubKey) .distinctUntilChanged() .drop(1) .collect { push(NappletProtocolJson.encodeIdentityChanged(it)) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletLaunchRegistry.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletLaunchRegistry.kt index 2b619b39ae..b91aabad36 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletLaunchRegistry.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletLaunchRegistry.kt @@ -22,6 +22,7 @@ package com.vitorpamplona.amethyst.napplet import com.vitorpamplona.amethyst.commons.napplet.NappletCapability import com.vitorpamplona.amethyst.commons.napplet.NappletIdentity +import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.core.toHexKey import java.security.SecureRandom @@ -45,6 +46,18 @@ object NappletLaunchRegistry { data class Session( val identity: NappletIdentity, val declared: Set, + /** + * The account this surface was launched as. Requests resolve their signer through THIS, not + * through whichever account happens to be active when they arrive. + * + * A full-screen host is a separate activity that an account switch does not tear down, so + * resolving live meant its WebView kept account A's cookies while the broker signed as B — + * a page showing one identity while another signed, and B's session written into A's + * storage jar. Binding here gives both halves of the rule for free: embedded surfaces are + * rebuilt on a switch, so they re-mint and follow the active account, while a full-screen + * surface keeps the account it was opened with. + */ + val accountPubKey: HexKey, ) // Access-ordered + capped so tokens from long-closed napplets can't accumulate without bound. The @@ -60,9 +73,10 @@ object NappletLaunchRegistry { fun register( identity: NappletIdentity, declared: Set, + accountPubKey: HexKey, ): String { val token = ByteArray(32).also(secureRandom::nextBytes).toHexKey() - sessions[token] = Session(identity, declared) + sessions[token] = Session(identity, declared, accountPubKey) return token } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletLauncher.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletLauncher.kt index c6da22a3ba..2c98ffb8fe 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletLauncher.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletLauncher.kt @@ -120,7 +120,16 @@ object NappletLauncher { // requests back to THIS identity + declared set, regardless of anything the sandbox sends. val identity = NappletIdentity(authorPubKey = authorPubKey, identifier = identifier, aggregateHash = aggregateHash) val declared = profile.declaredCapabilities(requires) - val launchToken = NappletLaunchRegistry.register(identity, declared) + // Bound to the account launching it, so the surface keeps signing as that account even if the + // user switches while it is open (an embedded surface is rebuilt on a switch and re-mints). + // An empty key can never match a loaded account, so a launch with nobody signed in fails + // closed at the broker rather than falling back to whoever signs in later. + val launchAccountPubKey = + Amethyst.instance.sessionManager + .loggedInAccount() + ?.pubKey + .orEmpty() + val launchToken = NappletLaunchRegistry.register(identity, declared, launchAccountPubKey) // Resolve the per-site network choice (Tor default; a site can be opted out to the open web). // Locked napplets always keep Tor for their blob fetches — only nSites expose the toggle. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletLiveSubscriptions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletLiveSubscriptions.kt index 1115bd026e..8429a0b721 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletLiveSubscriptions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletLiveSubscriptions.kt @@ -38,12 +38,13 @@ import java.util.concurrent.atomic.AtomicInteger * `relay.eose`. Encodes the `relay.event`/`relay.eose`/`relay.closed` pushes and hands them to the * caller-supplied sink — it never touches the transport itself. * - * [account] is read live (so it always targets the currently signed-in account); [open] is reached - * only after the broker authorized the subscription (RELAY consent). + * The account is supplied per [open] by the caller, which resolves it from the requesting surface's + * LAUNCH account — not from whoever is signed in at the time. A full-screen surface survives an + * account switch, and reading live would have pointed its REQs at the new account's relays while its + * signatures still came from the old one. [open] is reached only after the broker authorized the + * subscription (RELAY consent). */ -class NappletLiveSubscriptions( - private val account: () -> Account?, -) { +class NappletLiveSubscriptions { private val liveSubs = ConcurrentHashMap() private val liveSeq = AtomicInteger(0) @@ -62,9 +63,9 @@ class NappletLiveSubscriptions( fun open( nappletSubId: String, filters: List, + account: Account?, push: (String) -> Unit, ) { - val account = account() val relays = account?.homeRelays?.flow?.value ?: emptySet() if (account == null || filters.isEmpty() || relays.isEmpty()) { push(NappletProtocolJson.encodeRelayEose(nappletSubId))