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..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 @@ -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() + } +} 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..b17d737b5c --- /dev/null +++ b/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)), + ) + } +}