mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
fix(quartz): a NIP-77 reconcile is not a REQ page
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rtkpq5k2zy2hGP44kXLtq9
This commit is contained in:
+8
-6
@@ -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.
|
||||
|
||||
+29
@@ -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<CountCmd>
|
||||
|
||||
/**
|
||||
* 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<NegOpenCmd> =
|
||||
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.
|
||||
*
|
||||
|
||||
+14
@@ -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<NegOpenCmd> = 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)") }
|
||||
|
||||
+4
@@ -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 {
|
||||
|
||||
+75
@@ -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<ReqCmd> = 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<ReqCmd> = 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<ReqCmd> = 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))
|
||||
|
||||
+68
@@ -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()
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user