From cba4a2d990f35ada9f6a97ead2bfcf0c6efaeb73 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 02:15:01 +0000 Subject: [PATCH] fix(quartz): a NIP-77 reconcile is not a REQ page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NegSessionRegistry ran NEG-OPEN through the policy's REQ hook, so LimitsPolicy stamped the relay's `default_limit` (and clamped to `max_limit`) on the reconcile's filter. A reconcile then covered only the newest `default_limit` events of its filter — 500 on a typical relay — and reported that as the whole set: a mirror trusting it believed it was in sync with a fraction of the corpus. IRelayPolicy gains `accept(NegOpenCmd)`. Its default runs the REQ rules, so AUTH requirements and allow/deny lists written for REQ still govern a reconcile with no new code; a REQ rewrite into anything but one filter is refused. PolicyStack runs each member's own NEG-OPEN rule, and LimitsPolicy overrides it: the sub-id cap stays, the page limits do not. A reconcile's bound is NegentropySettings.maxSyncEvents (1,000,000, strfry parity), which refuses an oversized set with NEG-ERR instead of truncating it. A limit the client names still passes through. Tests: LimitsPolicy/PolicyStack unit cases in RelayLimitsTest, and an in-process relay with default_limit 5 reconciling 30 events (NegentropyIgnoresPageLimitsTest), which reconciled 5 before this change. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Rtkpq5k2zy2hGP44kXLtq9 --- .../relay/server/NegSessionRegistry.kt | 14 ++-- .../relay/server/policies/IRelayPolicy.kt | 29 +++++++ .../relay/server/policies/LimitsPolicy.kt | 14 ++++ .../relay/server/policies/PolicyStack.kt | 4 + .../nip01Core/relay/server/RelayLimitsTest.kt | 75 +++++++++++++++++++ .../relay/NegentropyIgnoresPageLimitsTest.kt | 68 +++++++++++++++++ 6 files changed, 198 insertions(+), 6 deletions(-) create mode 100644 quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/NegentropyIgnoresPageLimitsTest.kt diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NegSessionRegistry.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NegSessionRegistry.kt index 18b4a31925..cb4722ca00 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NegSessionRegistry.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NegSessionRegistry.kt @@ -22,7 +22,6 @@ package com.vitorpamplona.quartz.nip01Core.relay.server import com.vitorpamplona.quartz.nip01Core.relay.commands.toClient.Message import com.vitorpamplona.quartz.nip01Core.relay.commands.toClient.NoticeMessage -import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.ReqCmd import com.vitorpamplona.quartz.nip01Core.relay.server.backend.SessionBackend import com.vitorpamplona.quartz.nip01Core.relay.server.policies.IRelayPolicy import com.vitorpamplona.quartz.nip01Core.relay.server.policies.PolicyResult @@ -62,9 +61,12 @@ class NegSessionRegistry( * during the sync are not surfaced; clients re-open if they want * fresh state. * - * Access control reuses the REQ policy hook: a relay that requires - * AUTH or has kind/pubkey allow-deny lists applies the same rules - * to NEG-OPEN as it does to subscription REQs. + * Access control is the policy's NEG-OPEN hook, which by default runs + * the REQ rules: a relay that requires AUTH or has kind/pubkey + * allow-deny lists applies the same rules to NEG-OPEN as it does to + * subscription REQs. Page limits are the exception — see + * [com.vitorpamplona.quartz.nip01Core.relay.server.policies.LimitsPolicy] — since the + * snapshot cap below bounds a reconcile. * * Two strfry-parity protections fire here: * - **Per-connection session cap.** If an OPEN would push the @@ -80,12 +82,12 @@ class NegSessionRegistry( cmd: NegOpenCmd, policy: IRelayPolicy, ) { - val gate = policy.accept(ReqCmd(cmd.subId, listOf(cmd.filter))) + val gate = policy.accept(cmd) if (gate is PolicyResult.Rejected) { send(NegErrMessage(cmd.subId, gate.reason)) return } - val filters = (gate as PolicyResult.Accepted).cmd.filters + val filters = listOf((gate as PolicyResult.Accepted).cmd.filter) // Per-connection cap. Only fires when this is a NEW subId — // a same-subId re-open replaces the prior session 1-for-1. diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/IRelayPolicy.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/IRelayPolicy.kt index 2c3440053b..7649119f8a 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/IRelayPolicy.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/IRelayPolicy.kt @@ -30,6 +30,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.EventCmd import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.ReqCmd import com.vitorpamplona.quartz.nip01Core.relay.server.backend.RequestContext import com.vitorpamplona.quartz.nip42RelayAuth.RelayAuthEvent +import com.vitorpamplona.quartz.nip77Negentropy.NegOpenCmd /** * Defines custom behavior for this relay. @@ -71,6 +72,34 @@ interface IRelayPolicy { */ fun accept(cmd: CountCmd): PolicyResult + /** + * Evaluates a NIP-77 NEG-OPEN, optionally rewriting its filter. + * + * By default a reconcile answers to the REQ rules: it reads the same events a REQ with its + * filter would, so a relay that requires AUTH, or keeps kind/pubkey allow-deny lists, applies + * them here without writing a second hook. Override it where the two must differ — as + * [LimitsPolicy] does: a REQ's `default_limit` / `max_limit` are page sizes, and a reconcile + * is not a page. Its bound is [com.vitorpamplona.quartz.nip77Negentropy.NegentropySettings.maxSyncEvents], + * which refuses an oversized set with NEG-ERR instead of silently reconciling a truncated one. + * + * A REQ-rule rewrite into anything but ONE filter is refused: NIP-77 reconciles exactly one. + */ + fun accept(cmd: NegOpenCmd): PolicyResult = + when (val asReq = accept(ReqCmd(cmd.subId, listOf(cmd.filter)))) { + is PolicyResult.Rejected -> { + PolicyResult.Rejected(asReq.reason) + } + + is PolicyResult.Accepted -> { + val filter = asReq.cmd.filters.singleOrNull() + when { + filter == null -> PolicyResult.Rejected("error: a NEG-OPEN reconciles exactly one filter") + filter === cmd.filter -> PolicyResult.Accepted(cmd) + else -> PolicyResult.Accepted(NegOpenCmd(cmd.subId, filter, cmd.initialMessage)) + } + } + } + /** * Evaluates whether an incoming AUTH command should be accepted. * diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/LimitsPolicy.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/LimitsPolicy.kt index e543fa5a41..7b8b37b583 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/LimitsPolicy.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/LimitsPolicy.kt @@ -26,6 +26,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.CountCmd import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.EventCmd import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.ReqCmd import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter +import com.vitorpamplona.quartz.nip77Negentropy.NegOpenCmd /** * Enforces the per-command fields of a [RelayLimits]: rejects EVENTs whose @@ -82,6 +83,19 @@ class LimitsPolicy( return PolicyResult.Accepted(if (clamped === cmd.filters) cmd else CountCmd(cmd.queryId, clamped)) } + /** + * A NEG-OPEN keeps the sub-id cap and never receives [RelayLimits.defaultLimit] or + * [RelayLimits.maxLimit]: those size a REQ's PAGE, and a reconcile is not one. It walks the + * filter's whole set, bounded by `NegentropySettings.maxSyncEvents` (strfry's 1,000,000 by + * default), which answers an oversized set with NEG-ERR `blocked: too many query results`. + * + * Clamped like a REQ, a reconcile silently covered only the newest `default_limit` events of + * its filter — 500 on a typical relay — and reported that as the whole set, so a mirror that + * trusted it believed it was in sync with a fraction of the corpus. A limit the CLIENT names + * is its own business and passes through. + */ + override fun accept(cmd: NegOpenCmd): PolicyResult = subscriptionRejection(cmd.subId, listOf(cmd.filter))?.let { PolicyResult.Rejected(it) } ?: PolicyResult.Accepted(cmd) + /** The reason an EVENT violates a limit, or null when it's within bounds. */ private fun eventRejection(event: Event): String? { limits.maxContentLength?.let { if (event.content.length > it) return invalid("content too large (max $it)") } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/PolicyStack.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/PolicyStack.kt index 6adf69c413..69d8ec03c9 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/PolicyStack.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/PolicyStack.kt @@ -30,6 +30,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.EventCmd import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.ReqCmd import com.vitorpamplona.quartz.nip01Core.relay.server.backend.RequestContext import com.vitorpamplona.quartz.nip42RelayAuth.RelayAuthEvent +import com.vitorpamplona.quartz.nip77Negentropy.NegOpenCmd class PolicyStack( vararg policies: IRelayPolicy, @@ -49,6 +50,9 @@ class PolicyStack( override fun accept(cmd: CountCmd) = runPolicies(cmd) { p, c -> p.accept(c) } + /** Each member's OWN NEG-OPEN rule — not the stack's REQ rules, which would clamp a reconcile like a page. */ + override fun accept(cmd: NegOpenCmd) = runPolicies(cmd) { p, c -> p.accept(c) } + override fun accept(cmd: AuthCmd) = runPolicies(cmd) { p, c -> p.accept(c) } override suspend fun onAuthenticated(event: RelayAuthEvent): Boolean { diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/RelayLimitsTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/RelayLimitsTest.kt index 55f6aa7139..c25d851b8e 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/RelayLimitsTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/RelayLimitsTest.kt @@ -26,8 +26,11 @@ import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.EventCmd import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.ReqCmd import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter import com.vitorpamplona.quartz.nip01Core.relay.server.policies.LimitsPolicy +import com.vitorpamplona.quartz.nip01Core.relay.server.policies.PassThroughPolicy import com.vitorpamplona.quartz.nip01Core.relay.server.policies.PolicyResult +import com.vitorpamplona.quartz.nip01Core.relay.server.policies.PolicyStack import com.vitorpamplona.quartz.nip01Core.relay.server.policies.RelayLimits +import com.vitorpamplona.quartz.nip77Negentropy.NegOpenCmd import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNull @@ -191,6 +194,78 @@ class RelayLimitsTest { ) } + // -- LimitsPolicy: NEG-OPEN ------------------------------------------------ + + private fun negOpen(filter: Filter) = NegOpenCmd("n", filter, "6100") + + @Test + fun negOpenNeverGetsTheDefaultLimit() { + // default_limit sizes a REQ's page. Stamped on a reconcile it made the + // relay answer "in sync" over its newest 500 events only. + val policy = LimitsPolicy(RelayLimits(defaultLimit = 50, maxLimit = 100)) + val neg = negOpen(Filter(kinds = listOf(1))) + val result = policy.accept(neg) as PolicyResult.Accepted + assertTrue(result.cmd === neg) + assertNull(result.cmd.filter.limit) + } + + @Test + fun negOpenIsNotClampedToMaxLimit() { + // A reconcile's bound is NegentropySettings.maxSyncEvents, which refuses + // with NEG-ERR rather than truncating; max_limit is a page size. + val policy = LimitsPolicy(RelayLimits(defaultLimit = 50, maxLimit = 100)) + val result = policy.accept(negOpen(Filter(kinds = listOf(1), limit = 250_000))) as PolicyResult.Accepted + assertEquals(250_000, result.cmd.filter.limit) + } + + @Test + fun negOpenKeepsTheSubscriptionIdCap() { + val policy = LimitsPolicy(RelayLimits(maxSubidLength = 4)) + val result = policy.accept(NegOpenCmd("way-too-long", Filter(kinds = listOf(1)), "6100")) + assertTrue(result is PolicyResult.Rejected) + } + + @Test + fun aStackRunsEachMembersOwnNegOpenRule() { + // The stack must not route NEG-OPEN through ITS OWN accept(ReqCmd): that + // would run LimitsPolicy's REQ clamp and bring the truncation back. + val stack = PolicyStack(LimitsPolicy(RelayLimits(defaultLimit = 50, maxLimit = 100)), PassThroughPolicy()) + val result = stack.accept(negOpen(Filter(kinds = listOf(1)))) as PolicyResult.Accepted + assertNull(result.cmd.filter.limit) + } + + @Test + fun aPolicyWithOnlyReqRulesStillGovernsNegOpen() { + // Access rules written for REQ apply to a reconcile by default: a + // kind denied for reading is denied for reconciling too. + val noDms = + object : PassThroughPolicy() { + override fun accept(cmd: ReqCmd): PolicyResult = if (cmd.filters.any { it.kinds?.contains(4) == true }) PolicyResult.Rejected("restricted: no DMs") else PolicyResult.Accepted(cmd) + } + val stack = PolicyStack(LimitsPolicy(RelayLimits(defaultLimit = 50)), noDms) + assertTrue(stack.accept(negOpen(Filter(kinds = listOf(4)))) is PolicyResult.Rejected) + assertTrue(stack.accept(negOpen(Filter(kinds = listOf(1)))) is PolicyResult.Accepted) + } + + @Test + fun aReqRuleRewriteReachesTheNegOpenFilter() { + val narrowing = + object : PassThroughPolicy() { + override fun accept(cmd: ReqCmd): PolicyResult = PolicyResult.Accepted(ReqCmd(cmd.subId, cmd.filters.map { it.copy(kinds = listOf(1)) })) + } + val result = narrowing.accept(negOpen(Filter())) as PolicyResult.Accepted + assertEquals(listOf(1), result.cmd.filter.kinds) + } + + @Test + fun aReqRuleThatSplitsTheFilterIsRefusedForNegOpen() { + val splitting = + object : PassThroughPolicy() { + override fun accept(cmd: ReqCmd): PolicyResult = PolicyResult.Accepted(ReqCmd(cmd.subId, cmd.filters + cmd.filters)) + } + assertTrue(splitting.accept(negOpen(Filter())) is PolicyResult.Rejected) + } + @Test fun leavesAcceptableRequestUnchanged() { val policy = LimitsPolicy(RelayLimits(maxLimit = 100)) diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/NegentropyIgnoresPageLimitsTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/NegentropyIgnoresPageLimitsTest.kt new file mode 100644 index 0000000000..cad6a04867 --- /dev/null +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/NegentropyIgnoresPageLimitsTest.kt @@ -0,0 +1,68 @@ +/* + * 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 + +import com.vitorpamplona.geode.InProcessRelays +import com.vitorpamplona.geode.fixtures.SyntheticEvents +import com.vitorpamplona.geode.testing.preload +import com.vitorpamplona.quartz.nip01Core.relay.client.NostrClient +import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.negentropySync +import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter +import com.vitorpamplona.quartz.nip01Core.relay.server.policies.LimitsPolicy +import com.vitorpamplona.quartz.nip01Core.relay.server.policies.RelayLimits +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * A relay's REQ page limits must not bound a NIP-77 reconcile. With them applied, a relay with + * `default_limit` 5 reconciled the newest 5 of 30 events and called the set complete. + */ +class NegentropyIgnoresPageLimitsTest { + @Test + fun aReconcileCoversTheWholeSetPastDefaultAndMaxLimit() = + runBlocking { + val hub = InProcessRelays(defaultPolicy = { LimitsPolicy(RelayLimits(defaultLimit = 5, maxLimit = 10)) }) + val scope = CoroutineScope(Dispatchers.Default + SupervisorJob()) + val client = NostrClient(hub, scope) + try { + hub.getOrCreate(InProcessRelays.DEFAULT_URL).preload(SyntheticEvents.batch(30, kind = 1)) + + val result = + withTimeout(20_000) { + client.negentropySync(relay = InProcessRelays.DEFAULT_URL, filter = Filter(kinds = listOf(1))) { } + } + + // The reconcile itself — how many ids the relay said it holds. The download that + // follows is ordinary REQs, which default_limit rightly still pages. + assertEquals(30, result.needCount, "the reconcile saw the whole set, not a default_limit page") + } finally { + client.disconnect() + scope.cancel() + hub.close() + } + } +}