From b3fce19ff76605bb60cd583143950fb84ccfe07f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 01:25:34 +0000 Subject: [PATCH 1/2] fix: persist desktop AUTH approvals for long relay URLs The desktop AUTH banner (" requires authentication to deliver this message") kept re-appearing on every challenge even after the user clicked Always or Never. Root cause: PreferencesAuthApprovalStore used the raw relay URL as the java.util.prefs.Preferences key. Preferences caps keys at MAX_KEY_LENGTH (80 chars) and throws IllegalArgumentException from put() for anything longer. Outbox-proxy relay URLs that embed an npub and a query string routinely exceed that (e.g. wss://filter.nostr.wine/npub1...?broadcast=true is 102 chars), so setScope threw. RelayAuthenticator swallows the exception, so the ALWAYS/BLOCKED grant was silently never persisted and the relay re-prompted on the next AUTH challenge. Fold any relay URL over MAX_KEY_LENGTH into a bounded sha256:-prefixed 64-char hex key. Short URLs are still stored verbatim so existing grants keep working. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015aJPf1AsPTdzdMYR6pZ4e2 --- .../auth/PreferencesAuthApprovalStore.kt | 35 ++++++- .../auth/PreferencesAuthApprovalStoreTest.kt | 91 +++++++++++++++++++ 2 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStoreTest.kt diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStore.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStore.kt index ff47f1d38d..a61d888450 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStore.kt +++ b/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, @@ -55,8 +67,27 @@ class PreferencesAuthApprovalStore( "/com/vitorpamplona/amethyst/desktop/auth/$accountPubKeyHex", ) + /** + * 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 at or 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. + */ + private fun keyFor(relayUrl: NormalizedRelayUrl): String { + val url = relayUrl.url + return if (url.length <= Preferences.MAX_KEY_LENGTH) { + url + } else { + "sha256:" + sha256(url.encodeToByteArray()).toHexKey() + } + } + override suspend fun getScope(relayUrl: NormalizedRelayUrl): AuthApprovalScope? { - val raw = node.get(relayUrl.url, null) ?: return null + val raw = node.get(keyFor(relayUrl), null) ?: return null return runCatching { AuthApprovalScope.valueOf(raw) }.getOrNull() } @@ -70,7 +101,7 @@ class PreferencesAuthApprovalStore( // upgrade to "until next clear()". return } - node.put(relayUrl.url, scope.name) + node.put(keyFor(relayUrl), scope.name) node.flush() } diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStoreTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStoreTest.kt new file mode 100644 index 0000000000..6fa6506c48 --- /dev/null +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStoreTest.kt @@ -0,0 +1,91 @@ +/* + * 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.amethyst.commons.relayClient.auth.AuthApprovalScope +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import kotlinx.coroutines.test.runTest +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +class PreferencesAuthApprovalStoreTest { + // A distinct per-run account so the test never collides with a real user's + // stored grants under Preferences.userRoot(). + private val accountPubKeyHex = "test${System.nanoTime()}".padEnd(64, '0').take(64) + private val store = PreferencesAuthApprovalStore(accountPubKeyHex) + + @AfterTest + fun cleanup() = + runTest { + store.clear() + } + + @Test + fun shortRelayUrlRoundTrips() = + runTest { + val relay = NormalizedRelayUrl("wss://relay.example/") + store.setScope(relay, AuthApprovalScope.ALWAYS) + assertEquals(AuthApprovalScope.ALWAYS, store.getScope(relay)) + } + + @Test + fun overLongRelayUrlPersistsInsteadOfThrowing() = + runTest { + // 102 chars — over java.util.prefs.Preferences.MAX_KEY_LENGTH (80). + // Storing this raw threw "Key too long", the exception was swallowed + // upstream, and the grant silently never persisted -> the AUTH banner + // re-appeared on every challenge. Regression guard for that bug. + val relay = NormalizedRelayUrl("wss://filter.nostr.wine/npub1max2lm5977tkj4zc28djq25g2muzmjgh2jqf83mq7vy539hfs7eqgec4et?broadcast=true") + assertEquals(102, relay.url.length) + + store.setScope(relay, AuthApprovalScope.ALWAYS) + assertEquals(AuthApprovalScope.ALWAYS, store.getScope(relay)) + } + + @Test + fun overLongRelayUrlBlockedPersists() = + runTest { + val relay = NormalizedRelayUrl("wss://filter.nostr.wine/npub1max2lm5977tkj4zc28djq25g2muzmjgh2jqf83mq7vy539hfs7eqgec4et?broadcast=true") + store.setScope(relay, AuthApprovalScope.BLOCKED) + assertEquals(AuthApprovalScope.BLOCKED, store.getScope(relay)) + } + + @Test + fun distinctOverLongRelayUrlsDoNotCollide() = + runTest { + val a = NormalizedRelayUrl("wss://filter.nostr.wine/npub1max2lm5977tkj4zc28djq25g2muzmjgh2jqf83mq7vy539hfs7eqgec4et?broadcast=true") + val b = NormalizedRelayUrl("wss://filter.nostr.wine/npub1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq?broadcast=true") + store.setScope(a, AuthApprovalScope.ALWAYS) + store.setScope(b, AuthApprovalScope.BLOCKED) + assertEquals(AuthApprovalScope.ALWAYS, store.getScope(a)) + assertEquals(AuthApprovalScope.BLOCKED, store.getScope(b)) + } + + @Test + fun onceIsNeverPersisted() = + runTest { + val relay = NormalizedRelayUrl("wss://relay.example/") + store.setScope(relay, AuthApprovalScope.ONCE) + assertNull(store.getScope(relay)) + } +} From 72a70ab6352c21cd1e5792190b4a85fbb2e9d98e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 01:38:54 +0000 Subject: [PATCH 2/2] test: make AUTH approval key test OS-independent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous regression test drove PreferencesAuthApprovalStore against the real java.util.prefs.Preferences.userRoot() backing store, which threw IllegalArgumentException on the macOS CI runner (its preferences backend rejects operations the Linux one accepts). Extract the key derivation into a pure `authApprovalPreferenceKey` function and assert the invariant that actually matters — every derived key stays within Preferences.MAX_KEY_LENGTH, which is exactly the condition put() enforces. No OS I/O, so it runs deterministically on every platform. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015aJPf1AsPTdzdMYR6pZ4e2 --- .../auth/PreferencesAuthApprovalStore.kt | 45 ++++---- .../auth/PreferencesAuthApprovalStoreTest.kt | 109 +++++++++--------- 2 files changed, 80 insertions(+), 74 deletions(-) diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStore.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStore.kt index a61d888450..e059bc9dbc 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStore.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStore.kt @@ -67,27 +67,8 @@ class PreferencesAuthApprovalStore( "/com/vitorpamplona/amethyst/desktop/auth/$accountPubKeyHex", ) - /** - * 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 at or 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. - */ - private fun keyFor(relayUrl: NormalizedRelayUrl): String { - val url = relayUrl.url - return if (url.length <= Preferences.MAX_KEY_LENGTH) { - url - } else { - "sha256:" + sha256(url.encodeToByteArray()).toHexKey() - } - } - override suspend fun getScope(relayUrl: NormalizedRelayUrl): AuthApprovalScope? { - val raw = node.get(keyFor(relayUrl), null) ?: return null + val raw = node.get(authApprovalPreferenceKey(relayUrl), null) ?: return null return runCatching { AuthApprovalScope.valueOf(raw) }.getOrNull() } @@ -101,7 +82,7 @@ class PreferencesAuthApprovalStore( // upgrade to "until next clear()". return } - node.put(keyFor(relayUrl), scope.name) + node.put(authApprovalPreferenceKey(relayUrl), scope.name) node.flush() } @@ -110,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() + } +} diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStoreTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStoreTest.kt index 6fa6506c48..b17d737b5c 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStoreTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/auth/PreferencesAuthApprovalStoreTest.kt @@ -20,72 +20,75 @@ */ package com.vitorpamplona.amethyst.desktop.auth -import com.vitorpamplona.amethyst.commons.relayClient.auth.AuthApprovalScope import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl -import kotlinx.coroutines.test.runTest -import kotlin.test.AfterTest +import java.util.prefs.Preferences import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertNull +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 { - // A distinct per-run account so the test never collides with a real user's - // stored grants under Preferences.userRoot(). - private val accountPubKeyHex = "test${System.nanoTime()}".padEnd(64, '0').take(64) - private val store = PreferencesAuthApprovalStore(accountPubKeyHex) - - @AfterTest - fun cleanup() = - runTest { - store.clear() - } + // 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 shortRelayUrlRoundTrips() = - runTest { - val relay = NormalizedRelayUrl("wss://relay.example/") - store.setScope(relay, AuthApprovalScope.ALWAYS) - assertEquals(AuthApprovalScope.ALWAYS, store.getScope(relay)) - } + fun shortUrlStoredVerbatim() { + val url = "wss://relay.example/" + assertEquals(url, authApprovalPreferenceKey(NormalizedRelayUrl(url))) + } @Test - fun overLongRelayUrlPersistsInsteadOfThrowing() = - runTest { - // 102 chars — over java.util.prefs.Preferences.MAX_KEY_LENGTH (80). - // Storing this raw threw "Key too long", the exception was swallowed - // upstream, and the grant silently never persisted -> the AUTH banner - // re-appeared on every challenge. Regression guard for that bug. - val relay = NormalizedRelayUrl("wss://filter.nostr.wine/npub1max2lm5977tkj4zc28djq25g2muzmjgh2jqf83mq7vy539hfs7eqgec4et?broadcast=true") - assertEquals(102, relay.url.length) + fun overLongUrlIsHashedUnderTheCap() { + assertEquals(102, longUrl.length) + val key = authApprovalPreferenceKey(NormalizedRelayUrl(longUrl)) - store.setScope(relay, AuthApprovalScope.ALWAYS) - assertEquals(AuthApprovalScope.ALWAYS, store.getScope(relay)) - } + 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 overLongRelayUrlBlockedPersists() = - runTest { - val relay = NormalizedRelayUrl("wss://filter.nostr.wine/npub1max2lm5977tkj4zc28djq25g2muzmjgh2jqf83mq7vy539hfs7eqgec4et?broadcast=true") - store.setScope(relay, AuthApprovalScope.BLOCKED) - assertEquals(AuthApprovalScope.BLOCKED, store.getScope(relay)) + 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 distinctOverLongRelayUrlsDoNotCollide() = - runTest { - val a = NormalizedRelayUrl("wss://filter.nostr.wine/npub1max2lm5977tkj4zc28djq25g2muzmjgh2jqf83mq7vy539hfs7eqgec4et?broadcast=true") - val b = NormalizedRelayUrl("wss://filter.nostr.wine/npub1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq?broadcast=true") - store.setScope(a, AuthApprovalScope.ALWAYS) - store.setScope(b, AuthApprovalScope.BLOCKED) - assertEquals(AuthApprovalScope.ALWAYS, store.getScope(a)) - assertEquals(AuthApprovalScope.BLOCKED, store.getScope(b)) - } - - @Test - fun onceIsNeverPersisted() = - runTest { - val relay = NormalizedRelayUrl("wss://relay.example/") - store.setScope(relay, AuthApprovalScope.ONCE) - assertNull(store.getScope(relay)) - } + 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)), + ) + } }