From bae2031cf34e974c4bd245eee7dab9a58e3c932c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Jul 2026 02:32:35 +0000 Subject: [PATCH] fix(geode): validate config knobs and mirror filter at boot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fail-loud on config that would silently degrade the running relay: - readers = 0 makes every query hang forever on an empty reader pool; readers < 0 crashes with an unrelated message. optimize_interval_seconds <= 0 busy-loops PRAGMA optimize under the writer mutex. StaticConfig .validate() (called at boot) rejects both. - A typo in [[mirror]].filter (e.g. `kindss`) parsed to an empty match-everything filter through the tolerant deserializer — silently widening a trusted upstream's skip-verify scope to the whole firehose. MirrorFilterValidator strict-checks the filter JSON at boot: unknown keys and non-array list fields fail startup. - The self-mirror guard now compares scheme-insensitively (ws:// vs wss:// for the same host is still us) via displayUrl(). - The maintenance loop rethrows CancellationException and no longer swallows Errors. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01TtDNpayEYvJH7QuPswND3A --- geode/config.example.toml | 4 +- .../kotlin/com/vitorpamplona/geode/Main.kt | 32 +++++++- .../geode/config/MirrorFilterValidator.kt | 78 +++++++++++++++++++ .../geode/config/StaticConfig.kt | 24 +++++- .../geode/config/StaticConfigTest.kt | 53 +++++++++++++ 5 files changed, 187 insertions(+), 4 deletions(-) create mode 100644 geode/src/main/kotlin/com/vitorpamplona/geode/config/MirrorFilterValidator.kt diff --git a/geode/config.example.toml b/geode/config.example.toml index 1e5d9df79e..8ae62edc10 100644 --- a/geode/config.example.toml +++ b/geode/config.example.toml @@ -127,7 +127,9 @@ require_auth = false # inject events inside the declared scope. `since`/`limit` inside it # are ignored (backfill_seconds owns the time window). Omit to mirror # everything; for several disjoint scopes, repeat [[mirror]] with the -# same url. +# same url. The keys are validated at boot (a typo like `kindss` or a +# scalar where an array belongs fails startup) precisely because this +# filter is the trust boundary for `trusted = true`. # # `dir` (strfry-router parity) sets the flow direction: "down" pulls # from the upstream (default), "up" pushes this relay's matching diff --git a/geode/src/main/kotlin/com/vitorpamplona/geode/Main.kt b/geode/src/main/kotlin/com/vitorpamplona/geode/Main.kt index 8bb7895249..2ff1e4c866 100644 --- a/geode/src/main/kotlin/com/vitorpamplona/geode/Main.kt +++ b/geode/src/main/kotlin/com/vitorpamplona/geode/Main.kt @@ -21,6 +21,7 @@ package com.vitorpamplona.geode import com.vitorpamplona.geode.config.BannedEntry +import com.vitorpamplona.geode.config.MirrorFilterValidator import com.vitorpamplona.geode.config.RuntimeConfig import com.vitorpamplona.geode.config.RuntimeConfigData import com.vitorpamplona.geode.config.StaticConfig @@ -30,6 +31,7 @@ import com.vitorpamplona.geode.mirror.MirrorWorker import com.vitorpamplona.quartz.nip01Core.core.OptimizedJsonMapper import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.displayUrl import com.vitorpamplona.quartz.nip01Core.relay.normalizer.normalizeRelayUrl import com.vitorpamplona.quartz.nip01Core.relay.server.policies.EmptyPolicy import com.vitorpamplona.quartz.nip01Core.relay.server.policies.FullAuthPolicy @@ -40,6 +42,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.server.policies.VerifyAuthOnlyPo import com.vitorpamplona.quartz.nip01Core.relay.server.policies.VerifyPolicy import com.vitorpamplona.quartz.nip01Core.store.sqlite.EventStore import com.vitorpamplona.quartz.nip77Negentropy.NegentropySettings +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -95,6 +98,7 @@ fun main(args: Array) { .opt("--config") ?.let { StaticConfig.fromFile(File(it)) } ?: StaticConfig() + config.validate() val host = a.opt("--host") ?: config.network.host val port = a.opt("--port")?.toInt() ?: config.network.port @@ -210,6 +214,14 @@ fun main(args: Array) { // (backfill_seconds) and never bounds the subscription. val scope = m.filter?.let { json -> + // Strict-validate FIRST: the deserializer is tolerant + // (unknown keys skipped, wrong-typed entries dropped), + // so a typo like `{"kindss":[4]}` would silently parse + // to an empty filter — widening a trusted upstream's + // scope to the whole firehose. This filter is the + // containment boundary for `trusted = true`, so a typo + // must fail the boot, not the boundary. + MirrorFilterValidator.validate(m.url, json) val parsed = try { OptimizedJsonMapper.fromJsonTo(json) @@ -234,7 +246,14 @@ fun main(args: Array) { direction = direction, ) } - require(upstreams.none { it.url == advertisedUrl }) { + // Never mirror ourselves — a self-URL echoes every local publish + // back forever. Compare scheme-insensitively (ws:// vs wss:// for the + // same host is still us) and ignoring the trailing slash. This can't + // catch a public URL that resolves to this bind behind a proxy, nor a + // `--port 0` autobind, so it's a guardrail against the obvious typo, + // not a proof of non-self-reference. + val advertisedIdentity = advertisedUrl.displayUrl() + require(upstreams.none { it.url.displayUrl() == advertisedIdentity }) { "[[mirror]] must not list this relay's own URL ($advertisedUrl)" } val mirror = @@ -252,7 +271,16 @@ fun main(args: Array) { maintenanceScope.launch { while (true) { delay(secs * 1000) - runCatching { store.optimize() } + try { + store.optimize() + } catch (e: CancellationException) { + throw e // shutdown cancelled us; don't swallow it + } catch (e: Exception) { + // A missed refresh only means slightly staler planner + // stats until the next tick — log and keep the loop. + // Errors (OOM, etc.) are NOT swallowed. + println("PRAGMA optimize failed: ${e.message}") + } } } } diff --git a/geode/src/main/kotlin/com/vitorpamplona/geode/config/MirrorFilterValidator.kt b/geode/src/main/kotlin/com/vitorpamplona/geode/config/MirrorFilterValidator.kt new file mode 100644 index 0000000000..ab474cf596 --- /dev/null +++ b/geode/src/main/kotlin/com/vitorpamplona/geode/config/MirrorFilterValidator.kt @@ -0,0 +1,78 @@ +/* + * 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.geode.config + +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.node.ObjectNode + +/** + * Strict boot-time check for a `[[mirror]].filter` JSON string. The + * NIP-01 [com.vitorpamplona.quartz.nip01Core.relay.filters.Filter] + * deserializer is tolerant by design — it skips unknown keys and drops + * wrong-typed entries — which is right for untrusted wire input but + * wrong for operator config: a typo like `{"kindss":[4]}` would silently + * parse to an empty (match-everything) filter. Because this filter is + * the containment boundary for a `trusted = true` upstream (it caps what + * a skip-verify upstream may inject), a silent mis-parse would widen + * that trust to the whole firehose. So we reject the obvious mistakes at + * boot instead of degrading the boundary. + */ +object MirrorFilterValidator { + /** NIP-01 filter keys the deserializer recognizes; anything else is a typo. */ + private val KNOWN_KEYS = setOf("ids", "authors", "kinds", "since", "until", "limit", "search") + + /** Keys whose value must be a JSON array. */ + private val ARRAY_KEYS = setOf("ids", "authors", "kinds") + + private val mapper = ObjectMapper() + + /** + * Throws [IllegalArgumentException] when [json] is not a JSON object, + * carries an unrecognized top-level key, or gives a list-typed field + * (`ids`/`authors`/`kinds`, or a `#tag`/`&tag`) a non-array value. + */ + fun validate( + url: String, + json: String, + ) { + val node = + try { + mapper.readTree(json) + } catch (e: Exception) { + throw IllegalArgumentException("[[mirror]] filter for $url is not valid JSON: $json", e) + } + require(node is ObjectNode) { + "[[mirror]] filter for $url must be a JSON object (a NIP-01 filter), got: $json" + } + node.fieldNames().forEach { field -> + val isTagKey = field.length > 1 && (field[0] == '#' || field[0] == '&') + require(field in KNOWN_KEYS || isTagKey) { + "[[mirror]] filter for $url has unknown key \"$field\" — a NIP-01 filter uses " + + "ids/authors/kinds/since/until/limit/search or #tag/&tag" + } + if (field in ARRAY_KEYS || isTagKey) { + require(node.get(field).isArray) { + "[[mirror]] filter for $url: \"$field\" must be a JSON array" + } + } + } + } +} diff --git a/geode/src/main/kotlin/com/vitorpamplona/geode/config/StaticConfig.kt b/geode/src/main/kotlin/com/vitorpamplona/geode/config/StaticConfig.kt index c1559f6726..1145a36a1c 100644 --- a/geode/src/main/kotlin/com/vitorpamplona/geode/config/StaticConfig.kt +++ b/geode/src/main/kotlin/com/vitorpamplona/geode/config/StaticConfig.kt @@ -180,7 +180,10 @@ data class StaticConfig( /** * Keep an always-current in-memory `(created_at, id)` set so * full-corpus NEG-OPENs skip the table scan + seal (strfry - * parity). ~40 B per stored event of heap; on by default. + * parity). ~140 B per stored event of heap; on by default. Only + * built once the first full-corpus NEG-OPEN arrives, and only + * when the corpus fits `max_sync_events` (an over-cap corpus + * answers NEG-ERR from a capped scan instead). */ val live_index: Boolean = true, ) @@ -250,6 +253,25 @@ data class StaticConfig( val state_file: String? = null, ) + /** + * Boot-time sanity check for values the TOML types can't constrain. + * Throws [IllegalArgumentException] (fail-loud at startup) rather + * than letting a nonsensical knob degrade the running relay — a zero + * reader pool hangs every query, a non-positive optimize interval + * busy-loops the writer. Call once after parsing, before building + * the store. + */ + fun validate() { + database.readers?.let { + require(it >= 1) { "[database].readers must be >= 1 (got $it); a 0/negative pool can never answer a query" } + } + database.optimize_interval_seconds?.let { + require(it > 0) { + "[database].optimize_interval_seconds must be > 0 (got $it); a non-positive interval busy-loops PRAGMA optimize under the writer mutex" + } + } + } + companion object { private val mapper = tomlMapper { } diff --git a/geode/src/test/kotlin/com/vitorpamplona/geode/config/StaticConfigTest.kt b/geode/src/test/kotlin/com/vitorpamplona/geode/config/StaticConfigTest.kt index 5974fb9294..d7698300d7 100644 --- a/geode/src/test/kotlin/com/vitorpamplona/geode/config/StaticConfigTest.kt +++ b/geode/src/test/kotlin/com/vitorpamplona/geode/config/StaticConfigTest.kt @@ -25,6 +25,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter import java.io.File import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertNotNull import kotlin.test.assertTrue @@ -81,6 +82,58 @@ class StaticConfigTest { assertTrue(StaticConfig.fromToml("").mirror.isEmpty()) } + @Test + fun validateRejectsNonPositiveReaders() { + assertFailsWith { + StaticConfig.fromToml("[database]\nreaders = 0").validate() + } + assertFailsWith { + StaticConfig.fromToml("[database]\nreaders = -1").validate() + } + // A sane pool passes. + StaticConfig.fromToml("[database]\nreaders = 1").validate() + // Unset passes (quartz default applies). + StaticConfig.fromToml("").validate() + } + + @Test + fun validateRejectsNonPositiveOptimizeInterval() { + assertFailsWith { + StaticConfig.fromToml("[database]\noptimize_interval_seconds = 0").validate() + } + assertFailsWith { + StaticConfig.fromToml("[database]\noptimize_interval_seconds = -5").validate() + } + StaticConfig.fromToml("[database]\noptimize_interval_seconds = 3600").validate() + } + + @Test + fun mirrorFilterValidatorRejectsTyposAndScalars() { + val url = "wss://up.example/" + + // Unknown key (a typo) — must fail, not silently widen scope. + assertFailsWith { + MirrorFilterValidator.validate(url, """{"kindss":[4]}""") + } + // List field given a scalar. + assertFailsWith { + MirrorFilterValidator.validate(url, """{"authors":"abc"}""") + } + // Not an object. + assertFailsWith { + MirrorFilterValidator.validate(url, """["kinds",1]""") + } + // Malformed JSON. + assertFailsWith { + MirrorFilterValidator.validate(url, """{"kinds":[1,}""") + } + + // Valid shapes pass: recognized scalar + array + tag keys. + MirrorFilterValidator.validate(url, """{"kinds":[0,1,3],"#t":["nostr"],"since":123,"limit":5,"search":"x"}""") + MirrorFilterValidator.validate(url, """{"&p":["abc"]}""") + MirrorFilterValidator.validate(url, "{}") + } + @Test fun mirrorFilterJsonParsesToANip01Filter() { // The exact parse Main.kt runs on [[mirror]].filter at boot.