diff --git a/.beads/interactions.jsonl b/.beads/interactions.jsonl --- a/.beads/interactions.jsonl +++ b/.beads/interactions.jsonl @@ -2,3 +2,4 @@ {"id":"int-6eedbf38","kind":"field_change","created_at":"2026-06-27T20:34:46.528783549Z","actor":"dawn","issue_id":"hydrant-ct8","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"not an issue"}} {"id":"int-55de70f4","kind":"field_change","created_at":"2026-06-27T20:34:46.630520455Z","actor":"dawn","issue_id":"hydrant-r6q","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"not an issue"}} {"id":"int-ea64011f","kind":"field_change","created_at":"2026-06-27T20:39:01.288049505Z","actor":"dawn","issue_id":"hydrant-28z","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"resolved (ban.rs and BanTracker completely removed from codebase back in March)"}} +{"id":"int-11e4d483","kind":"field_change","created_at":"2026-06-27T20:53:29.6549615Z","actor":"dawn","issue_id":"hydrant-8b8","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"resolved (added HYDRANT_DB_RECORDS_BLOOM_FILTERS configuration option to toggle bloom filters, defaulting to true to protect against public getRecord miss attacks)"}} diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -6,7 +6,7 @@ {"_type":"issue","id":"hydrant-28z","title":"Unbounded crawler PDS ban map enables memory DoS","description":"Finding: Unbounded crawler PDS ban map enables memory DoS\nSeverity: MEDIUM\nOriginal Finding ID: c2eefaff8ac08191afe7275e94231542\nIntroducing Commit: 106ab40683271705bf2096914ebe83075a01040f\nRelevant Paths: src/crawler/ban.rs | src/crawler/mod.rs | src/resolver.rs\n\nDescription:\nThis commit introduces BanTracker, a process-lifetime HashMap keyed by the full resolved PDS Url. check_repo_signals resolves each unknown DID to a PDS endpoint, then unconditionally calls tracker.get_handle(\u0026pds_url), which inserts a new State if that URL has not been seen before. Entries are never removed when a request succeeds, when a ban expires, or when the related DID is no longer relevant. Because crawler listRepos/DID documents are untrusted inputs in the threat model, an attacker controlling many DIDs or a malicious configured relay can provide many distinct PDS serviceEndpoint URLs and force unbounded in-memory state growth. Keying on the whole Url also means variants of the same origin, if accepted by DID document parsing, can create separate ban states while actual XRPC requests are joined to a fixed /xrpc path.","status":"closed","priority":2,"issue_type":"bug","owner":"90008@klbr.net","created_at":"2026-06-27T20:32:03Z","created_by":"dawn","updated_at":"2026-06-27T20:39:01Z","closed_at":"2026-06-27T20:39:01Z","close_reason":"resolved (ban.rs and BanTracker completely removed from codebase back in March)","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"hydrant-7o8","title":"Unbounded collection enumeration in public describeRepo","description":"Finding: Unbounded collection enumeration in public describeRepo\nSeverity: MEDIUM\nOriginal Finding ID: b4a352096900819184744d6128f0583c\nIntroducing Commit: 6624ccf71b0e47458bbb555539d78c61f866eb02\nRelevant Paths: src/api/mod.rs | src/api/xrpc/mod.rs | src/api/xrpc/describe_repo.rs | src/control/repos.rs\n\nDescription:\nThe commit adds /xrpc/systems.gaze.hydrant.describeRepo to the public XRPC router. Handling a request resolves the identifier, obtains a repo handle, then concurrently calls repo.collections(). The collections() helper scans the entire counts keyspace prefix for that DID and inserts every matching collection into a HashMap. The handler then converts that into a HashSet for the JSON response, discarding the count values. There is no cap on scanned entries, response size, execution time, or memory use. Because collection-count entries are derived from indexed repository data, a malicious indexed account can create many distinct collection NSIDs and then any unauthenticated client can repeatedly call this public endpoint to force large DB prefix scans, allocations, and JSON serialization. This is a resource-exhaustion issue introduced by the new public endpoint.","status":"open","priority":2,"issue_type":"bug","owner":"90008@klbr.net","created_at":"2026-06-27T20:32:03Z","created_by":"dawn","updated_at":"2026-06-27T20:32:03Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"hydrant-0hs","title":"Stream event format change can trigger replay DoS","description":"Finding: Stream event format change can trigger replay DoS\nSeverity: MEDIUM\nOriginal Finding ID: 809b9f6d9b2c8191ad81c8da842aee68\nIntroducing Commit: a022ce698b1f3ddad6f88f35a4042f16063c3a33\nRelevant Paths: src/types.rs | src/api/stream.rs | src/api/mod.rs | src/ops.rs | src/backfill/mod.rs\n\nDescription:\n`StoredEvent` is stored in the database with `rmp_serde::to_vec` and replayed from `/stream` with `rmp_serde::from_slice`. `rmp-serde`'s default struct encoding is positional, so adding `live` at the start changes the on-disk field order. Older rows were encoded as `[did, rev, collection, rkey, action, cid]`; the new decoder expects `[live, did, rev, collection, rkey, action, cid]`. The `#[serde(default)]` on `live` does not protect this case because the old first element is present and is decoded as a boolean, which fails. In the stream handler, the cursor is advanced before deserialization, and deserialization failures are logged and skipped. Because `/stream` is publicly exposed, an attacker can connect with an old cursor such as `cursor=0` on an upgraded instance with many old-format events and repeatedly make the server scan the events keyspace and emit one error per legacy event, bypassing normal send-side backpressure for those skipped events. This also breaks historical replay integrity for legitimate clients.","status":"open","priority":2,"issue_type":"bug","owner":"90008@klbr.net","created_at":"2026-06-27T20:32:02Z","created_by":"dawn","updated_at":"2026-06-27T20:32:02Z","dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"hydrant-8b8","title":"Public getRecord misses can cause disk-heavy DB reads","description":"Finding: Public getRecord misses can cause disk-heavy DB reads\nSeverity: MEDIUM\nOriginal Finding ID: 5a0782626f448191a4d162e1f6ca8438\nIntroducing Commit: a0a9359a895131783a843a7dd5327f2b8b7da0f5\nRelevant Paths: src/db/mod.rs | src/api/xrpc.rs\n\nDescription:\nThe records keyspace was changed from default Fjall options to `expect_point_read_hits(true)`. The added comment explicitly treats this as turning off Bloom filters because getRecord is assumed to be uncommon. However, `com.atproto.repo.getRecord` is publicly routed, accepts attacker-controlled repo/collection/rkey inputs, and directly performs a point lookup in `db.records` without first checking whether the repo or record exists. An unauthenticated client can therefore generate large numbers of guaranteed-missing record keys. On a large LSM keyspace, disabling miss filters makes those misses significantly more expensive because the database must do point-lookup work across on-disk state instead of cheaply rejecting absent keys. This can amplify public HTTP traffic into disk I/O and blocking-thread exhaustion.","status":"open","priority":2,"issue_type":"bug","owner":"90008@klbr.net","created_at":"2026-06-27T20:32:01Z","created_by":"dawn","updated_at":"2026-06-27T20:32:01Z","dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"hydrant-8b8","title":"Public getRecord misses can cause disk-heavy DB reads","description":"Finding: Public getRecord misses can cause disk-heavy DB reads\nSeverity: MEDIUM\nOriginal Finding ID: 5a0782626f448191a4d162e1f6ca8438\nIntroducing Commit: a0a9359a895131783a843a7dd5327f2b8b7da0f5\nRelevant Paths: src/db/mod.rs | src/api/xrpc.rs\n\nDescription:\nThe records keyspace was changed from default Fjall options to `expect_point_read_hits(true)`. The added comment explicitly treats this as turning off Bloom filters because getRecord is assumed to be uncommon. However, `com.atproto.repo.getRecord` is publicly routed, accepts attacker-controlled repo/collection/rkey inputs, and directly performs a point lookup in `db.records` without first checking whether the repo or record exists. An unauthenticated client can therefore generate large numbers of guaranteed-missing record keys. On a large LSM keyspace, disabling miss filters makes those misses significantly more expensive because the database must do point-lookup work across on-disk state instead of cheaply rejecting absent keys. This can amplify public HTTP traffic into disk I/O and blocking-thread exhaustion.","status":"closed","priority":2,"issue_type":"bug","owner":"90008@klbr.net","created_at":"2026-06-27T20:32:01Z","created_by":"dawn","updated_at":"2026-06-27T20:53:30Z","closed_at":"2026-06-27T20:53:30Z","close_reason":"resolved (added HYDRANT_DB_RECORDS_BLOOM_FILTERS configuration option to toggle bloom filters, defaulting to true to protect against public getRecord miss attacks)","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"hydrant-5ci","title":"Legacy DID excludes ignored after key-format change","description":"Finding: Legacy DID excludes ignored after key-format change\nSeverity: MEDIUM\nOriginal Finding ID: 8a58834a31e88191a776fe5ff8ae9c72\nIntroducing Commit: 53ac9dddc4e289bb6eaf5c9fabbf105306e328d1\nRelevant Paths: src/db/filter.rs | src/ingest/firehose.rs | src/crawler/mod.rs\n\nDescription:\nBefore this commit, exclude entries were stored as `x|{raw_did}`. The new `exclude_key` stores excludes as `x|{TrimmedDid}`. Existing databases are not migrated, and the firehose and crawler now only check the new key format. As a result, any DID that was excluded before the upgrade will no longer be skipped, allowing that repo's firehose events to be processed and crawler-discovered repos to be queued despite the operator's denylist. Patch removal also computes only the new key, so legacy entries are not removed unless the operator replaces the entire exclude set.","status":"open","priority":2,"issue_type":"bug","owner":"90008@klbr.net","created_at":"2026-06-27T20:32:00Z","created_by":"dawn","updated_at":"2026-06-27T20:32:00Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"hydrant-m6v","title":"Crawler retry wake can busy-loop on jittered retries","description":"Finding: Crawler retry wake can busy-loop on jittered retries\nSeverity: MEDIUM\nOriginal Finding ID: a84c29ca490881918a976bc3b0231ae7\nIntroducing Commit: e558361eb571e7e019a4a7967bb4ae7e666f3f25\nRelevant Paths: src/crawler/mod.rs\n\nDescription:\nprocess_retry_queue() adds a randomized backoff to each retry entry and correctly treats the entry as not ready when state.after + backoff is still in the future. However, the newly introduced Duration-based wake calculation uses only state.after - now. If now is after state.after but before state.after + backoff, to_std() fails and unwrap_or(Duration::ZERO) is used. The retry thread then calls sleep(Duration::ZERO), immediately re-enters process_retry_queue(), scans the retry keyspace again, and repeats until at least the minimum jitter window has elapsed. A malicious PDS or relay-listed DID that causes retry entries, especially with large Retry-After/throttle durations, can make the crawler retry thread consume CPU and repeatedly hit the DB for a predictable window. The wake duration should be calculated from the actual readiness threshold, e.g. state.after + backoff - now.","status":"open","priority":2,"issue_type":"bug","owner":"90008@klbr.net","created_at":"2026-06-27T20:32:00Z","created_by":"dawn","updated_at":"2026-06-27T20:32:00Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"hydrant-6w8","title":"Crawler pause does not stop retry queue processing","description":"Finding: Crawler pause does not stop retry queue processing\nSeverity: MEDIUM\nOriginal Finding ID: 1db4d5a08c8c81918e69516f00833326\nIntroducing Commit: ec1b1027784eb565a4310e955e2688b100408247\nRelevant Paths: src/main.rs | src/crawler/mod.rs\n\nDescription:\nThis commit changes startup so a Crawler is spawned whenever relay hosts exist, even if crawler_enabled is false. Inside Crawler::run, the retry thread is started unconditionally and repeatedly calls process_retry_queue without checking the watch flag. Only the per-relay crawl loop calls wait_enabled(). As a result, setting HYDRANT_ENABLE_CRAWLER=false, starting in filter mode where the crawler defaults to disabled, or PATCHing /ingestion to pause the crawler does not fully stop crawler activity if retry entries already exist in the database. Those entries can trigger batches of up to 1000 signal checks, which perform identity resolution and outbound describeRepo HTTP requests to PDS URLs controlled by DID documents. This defeats the intended runtime/config safety control and can keep causing outbound-network and resource-consumption activity after an operator believes the crawler is paused.","status":"open","priority":2,"issue_type":"bug","owner":"90008@klbr.net","created_at":"2026-06-27T20:31:59Z","created_by":"dawn","updated_at":"2026-06-27T20:31:59Z","dependency_count":0,"dependent_count":0,"comment_count":0} diff --git a/src/config.rs b/src/config.rs --- a/src/config.rs +++ b/src/config.rs @@ -204,6 +204,11 @@ /// in-memory write buffer (memtable) size for the records keyspace in MB. /// set via `HYDRANT_DB_RECORDS_MEMTABLE_SIZE_MB`. pub db_records_memtable_size_mb: u64, + /// db internals, tune only if you know what you're doing. + /// + /// whether bloom filters are enabled for the records keyspace. + /// set via `HYDRANT_DB_RECORDS_BLOOM_FILTERS`. + pub db_records_bloom_filters: bool, /// replay batch size. /// @@ -307,6 +312,7 @@ db_repos_memtable_size_mb: BASE_MEMTABLE_MB / 2, db_events_memtable_size_mb: BASE_MEMTABLE_MB, db_records_memtable_size_mb: BASE_MEMTABLE_MB / 3 * 2, + db_records_bloom_filters: false, stream_replay_chunk_size: 0, stream_replay_chunk_pause: Duration::ZERO, stream_pending_event_limit: 4096, @@ -415,6 +421,11 @@ f, "db records memtable", format_args!("{} mb", self.db_records_memtable_size_mb) + )?; + config_line!( + f, + "db records bloom filters", + self.db_records_bloom_filters )?; let replay_chunk = if self.stream_replay_chunk_size == 0 { "auto".to_owned() diff --git a/src/config/env.rs b/src/config/env.rs --- a/src/config/env.rs +++ b/src/config/env.rs @@ -162,6 +162,10 @@ "DB_RECORDS_MEMTABLE_SIZE_MB", defaults.db_records_memtable_size_mb ); + let db_records_bloom_filters: bool = cfg!( + "DB_RECORDS_BLOOM_FILTERS", + defaults.db_records_bloom_filters + ); let db_repos_memtable_size_mb = cfg!( "DB_REPOS_MEMTABLE_SIZE_MB", defaults.db_repos_memtable_size_mb @@ -363,6 +367,7 @@ db_repos_memtable_size_mb, db_events_memtable_size_mb, db_records_memtable_size_mb, + db_records_bloom_filters, stream_replay_chunk_size, stream_replay_chunk_pause, stream_pending_event_limit, diff --git a/src/db/open.rs b/src/db/open.rs --- a/src/db/open.rs +++ b/src/db/open.rs @@ -192,9 +192,10 @@ "records", opts() // point reads might miss when using getRecord - // but we assume thats not going to happen often... (todo: should be a config option maybe?) - // and since this keyspace is big, turning off bloom filters will help a lot - .expect_point_read_hits(true) + // but we assume thats not going to happen often... + // since this keyspace is big, turning off bloom filters will help a lot with memory/disk space, + // but leaves point reads vulnerable to disk I/O misses under public query load + .expect_point_read_hits(!cfg.db_records_bloom_filters) .max_memtable_size(mb(cfg.db_records_memtable_size_mb)) // its just did|col|rkey -> cid, very small (84 bytes for bsky post) .data_block_size_policy(BlockSizePolicy::new([kb(8), kb(16)]))