Merge pull request #3591 from vitorpamplona/claude/relay-auth-request-repeat-0frbqp

Fix AUTH grant persistence for long relay URLs

desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStore.kt

@@ -22,7 +22,9 @@ package com.vitorpamplona.amethyst.desktop.auth
 
 import com.vitorpamplona.amethyst.commons.relayClient.auth.AuthApprovalScope
 import com.vitorpamplona.amethyst.commons.relayClient.auth.AuthApprovalStore
+import com.vitorpamplona.quartz.nip01Core.core.toHexKey
 import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
+import com.vitorpamplona.quartz.utils.sha256.sha256
 import java.util.prefs.Preferences
 
 /**
@@ -46,6 +48,16 @@ import java.util.prefs.Preferences
  * `ONCE` scope is never persisted — that's the in-memory contract enforced
  * by the [AuthApprovalStore] interface. This implementation only writes
  * `ALWAYS` and `BLOCKED`.
+ *
+ * **Key length:** `java.util.prefs.Preferences` caps keys at
+ * [Preferences.MAX_KEY_LENGTH] (80 chars) and throws `IllegalArgumentException`
+ * from [Preferences.put] for anything longer. Relay URLs routinely exceed that
+ * — e.g. an outbox-proxy URL that embeds an npub and a query string
+ * (`wss://filter.nostr.wine/npub1…?broadcast=true`, 100+ chars). Storing such a
+ * URL raw made [setScope] throw; the caller (`RelayAuthenticator`) swallows the
+ * exception, so the `ALWAYS` / `BLOCKED` grant was silently never persisted and
+ * the AUTH banner re-appeared on every challenge. [keyFor] folds any over-long
+ * URL into a bounded 64-char SHA-256 hex key to stay under the cap.
  */
 class PreferencesAuthApprovalStore(
     private val accountPubKeyHex: String,
@@ -56,7 +68,7 @@ class PreferencesAuthApprovalStore(
         )
 
     override suspend fun getScope(relayUrl: NormalizedRelayUrl): AuthApprovalScope? {
-        val raw = node.get(relayUrl.url, null) ?: return null
+        val raw = node.get(authApprovalPreferenceKey(relayUrl), null) ?: return null
         return runCatching { AuthApprovalScope.valueOf(raw) }.getOrNull()
     }
 
@@ -70,7 +82,7 @@ class PreferencesAuthApprovalStore(
             // upgrade to "until next clear()".
             return
         }
-        node.put(relayUrl.url, scope.name)
+        node.put(authApprovalPreferenceKey(relayUrl), scope.name)
         node.flush()
     }
 
@@ -79,3 +91,25 @@ class PreferencesAuthApprovalStore(
         node.flush()
     }
 }
+
+/**
+ * The Preferences key for a relay URL, guaranteed to fit within
+ * [Preferences.MAX_KEY_LENGTH].
+ *
+ * Short URLs are stored verbatim (readable, and backward-compatible with grants
+ * written before this fix). URLs over the limit are hashed to a
+ * `sha256:`-prefixed 64-char hex digest (71 chars total, under the 80 cap). The
+ * prefix keeps the hashed keyspace disjoint from raw relay URLs, which always
+ * start with `ws://` / `wss://`, so the two can never collide.
+ *
+ * Pure and side-effect-free so it can be unit-tested without touching the OS
+ * Preferences backing store.
+ */
+internal fun authApprovalPreferenceKey(relayUrl: NormalizedRelayUrl): String {
+    val url = relayUrl.url
+    return if (url.length <= Preferences.MAX_KEY_LENGTH) {
+        url
+    } else {
+        "sha256:" + sha256(url.encodeToByteArray()).toHexKey()
+    }
+}

desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStoreTest.kt

@@ -0,0 +1,94 @@
+/*
+ * 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.amethyst.desktop.auth
+
+import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
+import java.util.prefs.Preferences
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNotEquals
+import kotlin.test.assertTrue
+
+/**
+ * Regression tests for the AUTH-banner-repeats bug.
+ *
+ * `PreferencesAuthApprovalStore` used the raw relay URL as the
+ * `java.util.prefs.Preferences` key. Preferences caps keys at
+ * [Preferences.MAX_KEY_LENGTH] (80 chars) and `put()` throws
+ * `IllegalArgumentException` for anything longer. Outbox-proxy relay URLs that
+ * embed an npub and a query string routinely exceed that, so the `ALWAYS` /
+ * `BLOCKED` grant silently failed to persist and the banner re-appeared on
+ * every AUTH challenge.
+ *
+ * These assert the key-derivation invariant directly (no OS Preferences I/O, so
+ * they run deterministically on every platform): every key stays within the
+ * cap, which is exactly the condition `Preferences.put` enforces.
+ */
+class PreferencesAuthApprovalStoreTest {
+    // 102 chars — the outbox-proxy URL from the bug report, over the 80 cap.
+    private val longUrl = "wss://filter.nostr.wine/npub1max2lm5977tkj4zc28djq25g2muzmjgh2jqf83mq7vy539hfs7eqgec4et?broadcast=true"
+
+    @Test
+    fun shortUrlStoredVerbatim() {
+        val url = "wss://relay.example/"
+        assertEquals(url, authApprovalPreferenceKey(NormalizedRelayUrl(url)))
+    }
+
+    @Test
+    fun overLongUrlIsHashedUnderTheCap() {
+        assertEquals(102, longUrl.length)
+        val key = authApprovalPreferenceKey(NormalizedRelayUrl(longUrl))
+
+        assertTrue(key.startsWith("sha256:"), "over-long URLs must fold to a hashed key")
+        assertTrue(
+            key.length <= Preferences.MAX_KEY_LENGTH,
+            "key length ${key.length} must stay within Preferences.MAX_KEY_LENGTH so put() never throws",
+        )
+    }
+
+    @Test
+    fun everyKeyStaysWithinThePreferencesCap() {
+        // Boundary + well over: whatever the URL length, the derived key must be
+        // storable (this is the invariant Preferences.put enforces).
+        listOf(
+            "wss://relay.example/",
+            "wss://" + "a".repeat(Preferences.MAX_KEY_LENGTH), // 86 chars
+            "wss://" + "a".repeat(500),
+            longUrl,
+        ).forEach { url ->
+            val key = authApprovalPreferenceKey(NormalizedRelayUrl(url))
+            assertTrue(
+                key.length <= Preferences.MAX_KEY_LENGTH,
+                "key for a ${url.length}-char URL was ${key.length} chars, over the ${Preferences.MAX_KEY_LENGTH} cap",
+            )
+        }
+    }
+
+    @Test
+    fun distinctOverLongUrlsProduceDistinctKeys() {
+        val a = "wss://filter.nostr.wine/npub1max2lm5977tkj4zc28djq25g2muzmjgh2jqf83mq7vy539hfs7eqgec4et?broadcast=true"
+        val b = "wss://filter.nostr.wine/npub1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq?broadcast=true"
+        assertNotEquals(
+            authApprovalPreferenceKey(NormalizedRelayUrl(a)),
+            authApprovalPreferenceKey(NormalizedRelayUrl(b)),
+        )
+    }
+}