From 07d25c02db70a8aa0ffed1be1138857e4f72150e Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Mon, 22 Jun 2026 14:28:33 -0400 Subject: [PATCH] fix(napplet): serialize consent prompts so concurrent requests don't drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a napplet issued several consent-gated calls at once (the common case: it reads relays + storage + identity on load), each launched a NappletConsentActivity concurrently. The host can only show one, so the rest were delivered to the single-top activity and silently dropped — their broker calls hung forever (storage stuck pending; a subscription's consent lost, yielding 0 events). Gate the consent-prompt path behind a Mutex on the (per-account, reused) broker so prompts queue one at a time. After taking the lock, re-read the ledger so a sibling request for the same capability honors the just-recorded grant instead of prompting again. Only the prompt is serialized — execute() and already-granted paths stay parallel. Per-use capabilities (payments) still re-prompt every time. Adds a regression test asserting 5 concurrent same-capability requests yield exactly one prompt and never two dialogs at once. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../amethyst/commons/napplet/NappletBroker.kt | 41 ++++++++++++-- .../commons/napplet/NappletBrokerTest.kt | 55 +++++++++++++++++++ 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/napplet/NappletBroker.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/napplet/NappletBroker.kt index 616a676ec3..f73e789c0a 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/napplet/NappletBroker.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/napplet/NappletBroker.kt @@ -29,6 +29,8 @@ import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal import com.vitorpamplona.quartz.utils.TimeUtils +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlin.coroutines.cancellation.CancellationException /** @@ -68,6 +70,11 @@ class NappletBroker( private val upload: NappletUploadGateway? = null, private val identityReads: NappletIdentityGateway? = null, ) { + // Serializes the consent-prompt path so concurrent requests queue into one dialog at a time + // (see [authorizeWithConsent]). Only the prompt is held here; non-prompting paths and execute() + // run unserialized. + private val consentLock = Mutex() + /** * Authorizes and runs [request] on behalf of [identity]. [declared] is the capability set the * manifest's `requires` resolved to; a request outside it is refused before any prompt. @@ -105,11 +112,7 @@ class NappletBroker( signerSelfGates(request) -> true // A standing allow short-circuits, except for per-use capabilities (e.g. payments). ledger.decide(identity, capability) == PermissionDecision.ALLOW && !capability.requiresPerUseConsent -> true - else -> { - val grant = consentPrompt.request(identity, capability, request) - ledger.record(identity, capability, effectiveGrant(capability, grant)) - grant.allowsExecution - } + else -> authorizeWithConsent(identity, capability, request) } if (!authorized) return NappletResponse.Denied(capability, "The user declined.") @@ -123,6 +126,34 @@ class NappletBroker( } } + /** + * Prompts for consent under [consentLock] so several requests arriving at once — the common case, + * a napplet reading relays + storage + identity on load — queue into one dialog at a time instead + * of racing. Without this each would launch a consent prompt concurrently; the host can only show + * one, so the rest are dropped and their calls hang forever. + * + * After taking the lock we re-read the standing decision: a sibling request for the same capability + * may have recorded an Allow/Deny while we waited, so we honor that instead of prompting again. + * Per-use capabilities (payments) always re-prompt, so they skip the re-check. + */ + private suspend fun authorizeWithConsent( + identity: NappletIdentity, + capability: NappletCapability, + request: NappletRequest, + ): Boolean = + consentLock.withLock { + if (!capability.requiresPerUseConsent) { + when (ledger.decide(identity, capability)) { + PermissionDecision.DENY -> return@withLock false + PermissionDecision.ALLOW -> return@withLock true + PermissionDecision.ASK -> {} + } + } + val grant = consentPrompt.request(identity, capability, request) + ledger.record(identity, capability, effectiveGrant(capability, grant)) + grant.allowsExecution + } + /** Identity reads and sign-as-user ops are gated by us only when we hold the key; remote/external signers gate themselves. */ private fun signerSelfGates(request: NappletRequest): Boolean = (request.capability == NappletCapability.IDENTITY || request.signsAsUser) && signer !is NostrSignerInternal diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/napplet/NappletBrokerTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/napplet/NappletBrokerTest.kt index e0aee2702d..5503588eda 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/napplet/NappletBrokerTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/napplet/NappletBrokerTest.kt @@ -36,6 +36,10 @@ import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal import com.vitorpamplona.quartz.nip57Zaps.LnZapPrivateEvent import com.vitorpamplona.quartz.nip57Zaps.LnZapRequestEvent +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import kotlin.test.Test import kotlin.test.assertEquals @@ -226,6 +230,57 @@ class NappletBrokerTest { assertEquals(1, prompt.calls) // only the first request prompted } + /** A prompt that blocks inside [request] until [gate] is released, tracking peak concurrency. */ + private class GatedPrompt( + private val answer: GrantState, + ) : NappletConsentPrompt { + val gate = CompletableDeferred() + var calls = 0 + private set + var maxActive = 0 + private set + private var active = 0 + + override suspend fun request( + identity: NappletIdentity, + capability: NappletCapability, + request: NappletRequest, + ): GrantState { + calls++ + active++ + if (active > maxActive) maxActive = active + try { + gate.await() + return answer + } finally { + active-- + } + } + } + + @OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class) + @Test + fun concurrentRequestsForOneCapabilitySerializeIntoASinglePrompt() = + runTest { + // Five capability calls land at once (a napplet hitting relays/storage/identity on load). + // Without serialization each would launch its own consent prompt concurrently; the host + // can only show one, so the rest are dropped and hang. The broker must queue them through + // a single prompt, and siblings must honor the grant the first one records. + val prompt = GatedPrompt(GrantState.ALLOW_ALWAYS) + val broker = broker(prompt) + + val jobs = List(5) { async { broker.handle(applet, NappletRequest.GetPublicKey, allDeclared) } } + advanceUntilIdle() // everyone reaches the gate/lock; only the lock holder is inside request() + + assertEquals(1, prompt.maxActive) // never two prompts at once (the bug would show 5) + + prompt.gate.complete(Unit) + val responses = jobs.awaitAll() + + assertEquals(1, prompt.calls) // siblings honored the recorded ALLOW_ALWAYS instead of re-prompting + responses.forEach { assertEquals(NappletResponse.PublicKey(signer.pubKey), it) } + } + @Test fun publishSignsTheTemplateAsTheUserAndSends() = runTest {