Merge pull request #3292 from vitorpamplona/claude/elegant-goldberg-qzkt7c

Swallow signer exceptions in relay auth and bunker responses

quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/auth/RelayAuthenticator.kt

@@ -29,6 +29,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.commands.toClient.OkMessage
 import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.AuthCmd
 import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
 import com.vitorpamplona.quartz.nip01Core.signers.EventTemplate
+import com.vitorpamplona.quartz.nip01Core.signers.SignerExceptions
 import com.vitorpamplona.quartz.nip42RelayAuth.RelayAuthEvent
 import com.vitorpamplona.quartz.utils.Log
 import com.vitorpamplona.quartz.utils.cache.LargeCache
@@ -37,6 +38,7 @@ import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.IO
 import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.launch
+import kotlin.coroutines.cancellation.CancellationException
 
 interface IAuthStatus {
     fun hasFinishedAuthentication(relay: NormalizedRelayUrl): Boolean
@@ -83,12 +85,26 @@ class RelayAuthenticator(
         msg: AuthMessage,
     ) {
         scope.launch {
-            val ev = RelayAuthEvent.build(relay.url, msg.challenge)
-            signWithAllLoggedInUsers(ev).forEach { authEvent ->
-                // only send replies to new challenges to avoid infinite loop:
-                if (authStatus.get(relay.url)?.saveAuthSubmission(authEvent) == true) {
-                    relay.sendIfConnected(AuthCmd(authEvent))
+            // Relay auth is automatic and not user-initiated. Signing can fail in
+            // benign, expected ways — e.g. an external NIP-55 signer prompt that the
+            // user ignores surfaces as SignerExceptions.TimedOutException. Those must
+            // never escape this fire-and-forget launch: the host's scope may not carry
+            // a CoroutineExceptionHandler (viewModelScope, rememberCoroutineScope, …),
+            // so an uncaught throwable here crashes the whole app. Swallow + log them.
+            try {
+                val ev = RelayAuthEvent.build(relay.url, msg.challenge)
+                signWithAllLoggedInUsers(ev).forEach { authEvent ->
+                    // only send replies to new challenges to avoid infinite loop:
+                    if (authStatus.get(relay.url)?.saveAuthSubmission(authEvent) == true) {
+                        relay.sendIfConnected(AuthCmd(authEvent))
+                    }
                 }
+            } catch (e: CancellationException) {
+                throw e
+            } catch (e: SignerExceptions) {
+                Log.d("RelayAuthenticator") { "Could not sign auth for ${relay.url}: ${e.message}" }
+            } catch (e: Exception) {
+                Log.w("RelayAuthenticator", "Failed to authenticate with ${relay.url}", e)
             }
         }
     }

quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/NostrSignerRemote.kt

@@ -42,11 +42,13 @@ import com.vitorpamplona.quartz.nip46RemoteSigner.NostrConnectEvent
 import com.vitorpamplona.quartz.nip57Zaps.LnZapPrivateEvent
 import com.vitorpamplona.quartz.nip57Zaps.LnZapRequestEvent
 import com.vitorpamplona.quartz.utils.Hex
+import com.vitorpamplona.quartz.utils.Log
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.cancel
 import kotlinx.coroutines.launch
+import kotlin.coroutines.cancellation.CancellationException
 
 class NostrSignerRemote(
     val signer: NostrSignerInternal,
@@ -80,7 +82,19 @@ class NostrSignerRemote(
         ) { event ->
             if (event is NostrConnectEvent) {
                 scope.launch {
-                    manager.newResponse(event)
+                    // Incoming bunker responses come straight off the relay and are
+                    // decrypted here on a fire-and-forget launch whose scope carries no
+                    // CoroutineExceptionHandler. A malformed/hostile event (or a benign
+                    // SignerExceptions from decryption) must not escape and crash the app.
+                    try {
+                        manager.newResponse(event)
+                    } catch (e: CancellationException) {
+                        throw e
+                    } catch (e: SignerExceptions) {
+                        Log.d("NostrSignerRemote") { "Could not decrypt bunker response ${event.id}: ${e.message}" }
+                    } catch (e: Exception) {
+                        Log.w("NostrSignerRemote", "Failed to process bunker response ${event.id}", e)
+                    }
                 }
             }
         }

quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/auth/RelayAuthenticatorTimeoutTest.kt

@@ -0,0 +1,148 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.quartz.nip01Core.relay.client.auth
+
+import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair
+import com.vitorpamplona.quartz.nip01Core.relay.client.EmptyNostrClient
+import com.vitorpamplona.quartz.nip01Core.relay.client.INostrClient
+import com.vitorpamplona.quartz.nip01Core.relay.client.listeners.RelayConnectionListener
+import com.vitorpamplona.quartz.nip01Core.relay.client.single.IRelayClient
+import com.vitorpamplona.quartz.nip01Core.relay.commands.toClient.AuthMessage
+import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.AuthCmd
+import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.Command
+import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
+import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal
+import com.vitorpamplona.quartz.nip01Core.signers.SignerExceptions
+import com.vitorpamplona.quartz.nip42RelayAuth.RelayAuthEvent
+import kotlinx.coroutines.CoroutineExceptionHandler
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.runBlocking
+import kotlin.test.Test
+import kotlin.test.assertNull
+import kotlin.test.assertTrue
+
+/**
+ * Relay auth is automatic — the relay sends the AUTH challenge and
+ * [RelayAuthenticator] signs a reply in a fire-and-forget `scope.launch`. With an
+ * external NIP-55 Android signer, the user can simply ignore the approval prompt,
+ * which surfaces as [SignerExceptions.TimedOutException] from the signing lambda.
+ *
+ * The host scope passed in by Amethyst (`viewModelScope` + SupervisorJob) carries
+ * no [CoroutineExceptionHandler], so before the fix an uncaught throwable from that
+ * launch reached the platform default handler and crashed the whole app
+ * (TimedOutException: "Could not sign: User didn't accept or reject in time.").
+ *
+ * These tests pin the contract: signing failures during auth must be swallowed,
+ * and the happy path must still send the AUTH reply.
+ */
+class RelayAuthenticatorTimeoutTest {
+    private class CapturingClient(
+        private val delegate: INostrClient = EmptyNostrClient(),
+    ) : INostrClient by delegate {
+        @Volatile var captured: RelayConnectionListener? = null
+
+        override fun addConnectionListener(listener: RelayConnectionListener) {
+            captured = listener
+        }
+    }
+
+    private class FakeRelayClient(
+        override val url: NormalizedRelayUrl,
+    ) : IRelayClient {
+        val sent = mutableListOf<Command>()
+
+        override fun connect() = Unit
+
+        override fun needsToReconnect() = false
+
+        override fun connectAndSyncFiltersIfDisconnected(ignoreRetryDelays: Boolean) = Unit
+
+        override fun isConnected() = true
+
+        override fun sendOrConnectAndSync(cmd: Command) {
+            sent.add(cmd)
+        }
+
+        override fun sendIfConnected(cmd: Command) {
+            sent.add(cmd)
+        }
+
+        override fun disconnect() = Unit
+    }
+
+    @Test
+    fun signerTimeoutDuringAuthIsSwallowedAndDoesNotReachExceptionHandler() =
+        runBlocking {
+            var escaped: Throwable? = null
+            val handler = CoroutineExceptionHandler { _, t -> escaped = t }
+            // Unconfined runs the launched body eagerly so the assertion is deterministic.
+            val scope = CoroutineScope(Dispatchers.Unconfined + SupervisorJob() + handler)
+
+            val client = CapturingClient()
+            RelayAuthenticator(
+                client = client,
+                scope = scope,
+                signWithAllLoggedInUsers = {
+                    throw SignerExceptions.TimedOutException("User didn't accept or reject in time.")
+                },
+            )
+            val listener = client.captured ?: error("RelayAuthenticator did not register a listener")
+
+            val relay = FakeRelayClient(NormalizedRelayUrl("wss://relay.example/"))
+            listener.onConnecting(relay)
+            listener.onIncomingMessage(relay, "", AuthMessage("challenge-123"))
+
+            assertNull(escaped, "TimedOutException must not escape the auth launch")
+            assertTrue(relay.sent.isEmpty(), "No AUTH reply should be sent when signing fails")
+        }
+
+    @Test
+    fun successfulAuthStillSendsAuthCommand() =
+        runBlocking {
+            var escaped: Throwable? = null
+            val handler = CoroutineExceptionHandler { _, t -> escaped = t }
+            val scope = CoroutineScope(Dispatchers.Unconfined + SupervisorJob() + handler)
+
+            val signer = NostrSignerInternal(KeyPair())
+            val relay = FakeRelayClient(NormalizedRelayUrl("wss://relay.example/"))
+
+            val client = CapturingClient()
+            RelayAuthenticator(
+                client = client,
+                scope = scope,
+                signWithAllLoggedInUsers = { template ->
+                    listOf(RelayAuthEvent.create(relay.url, "challenge-123", signer))
+                },
+            )
+            val listener = client.captured ?: error("RelayAuthenticator did not register a listener")
+
+            listener.onConnecting(relay)
+            listener.onIncomingMessage(relay, "", AuthMessage("challenge-123"))
+
+            assertNull(escaped, "Happy path must not surface any exception")
+            assertTrue(
+                relay.sent.any { it is AuthCmd },
+                "A signed AUTH reply should be sent on the happy path",
+            )
+        }
+}