From 18ba4fe0dfb1381a94deaa309ea75cd10040c19f Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Fri, 11 Sep 2026 20:41:14 -0400 Subject: [PATCH] fix(serve)!: close every route that names the accounts when listAgents closes An operator closing `bot.did.listAgents` got `DisclosureDisabled` over a roster still readable through `com.atproto.sync.listRepos`, `com.atproto.sync.subscribeRepos`, `/events` and `/firehose`. All five now answer to the same decision, which costs a closed deployment its relay crawl. Co-Authored-By: Claude Opus 5 (1M context) Change-Id: Ia4a16c17837d1fbeac07892a5417a869e4363cb0 --- crates/didbot-serve/src/auth.rs | 15 +++++- crates/didbot-serve/src/routes.rs | 26 ++++++++++ crates/didbot-serve/src/tests.rs | 82 +++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 1 deletion(-) diff --git a/crates/didbot-serve/src/auth.rs b/crates/didbot-serve/src/auth.rs index a9dca8a2..32a40d1a 100644 --- a/crates/didbot-serve/src/auth.rs +++ b/crates/didbot-serve/src/auth.rs @@ -238,7 +238,20 @@ pub enum Credential { /// different facts, and only one of them is suspicious. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Disclosure { - /// `bot.did.listAgents`. + /// Whether a stranger may read which accounts this deployment holds. + /// + /// Every route that names them alike, not `bot.did.listAgents` on its + /// own: `com.atproto.sync.listRepos` pages through the same accounts, + /// and `com.atproto.sync.subscribeRepos`, `/events` and `/firehose` + /// announce each one as it appears and as it writes. Closing one and + /// leaving the others would answer `DisclosureDisabled` over a roster + /// still readable four other ways, which is worse than not offering the + /// toggle: it says the accounts are hidden when they are not. + /// + /// Closing it costs federation. A relay crawls `subscribeRepos`, and a + /// deployment that refuses it is not indexed — the trade + /// `plan/spaces.md` names as "private means federation off", taken at + /// this grain. pub list_agents: bool, /// `bot.did.listAgentLedgers`. pub list_agent_ledgers: bool, diff --git a/crates/didbot-serve/src/routes.rs b/crates/didbot-serve/src/routes.rs index 1321f037..b19805ca 100644 --- a/crates/didbot-serve/src/routes.rs +++ b/crates/didbot-serve/src/routes.rs @@ -2054,6 +2054,11 @@ async fn events_stream( if let Err(err) = require_lifecycle(&state, |p| p.serves_reads, "/events") { return err.into_response(); } + // Announces each account as it appears, so the same decision closes it; + // see `auth::Disclosure`. + if let Err(err) = auth::require_disclosure(state.disclosure.list_agents, "/events") { + return err.into_response(); + } // Keep-alive comments stop an idle proxy, or a browser, from deciding a // stream with no provisioning traffic on it is dead. Sse::new(state.events.subscribe(query.cursor.as_deref())) @@ -2076,6 +2081,11 @@ async fn firehose_stream( if let Err(err) = require_lifecycle(&state, |p| p.serves_reads, "/firehose") { return err.into_response(); } + // Announces each account as it appears and as it writes, so the same + // decision closes it; see `auth::Disclosure`. + if let Err(err) = auth::require_disclosure(state.disclosure.list_agents, "/firehose") { + return err.into_response(); + } Sse::new(state.firehose.subscribe(query.cursor.as_deref())) .keep_alive(KeepAlive::default()) .into_response() @@ -3723,6 +3733,13 @@ async fn list_repos( State(state): State, query: Result, QueryRejection>, ) -> Response { + // This route names every account this deployment holds, so it is the + // same decision `bot.did.listAgents` is; see `auth::Disclosure`. + if let Err(err) = + auth::require_disclosure(state.disclosure.list_agents, "com.atproto.sync.listRepos") + { + return err.into_response(); + } let Query(query) = match query { Ok(query) => query, Err(rejection) => return ApiError::bad_request(rejection.body_text()).into_response(), @@ -3775,6 +3792,15 @@ async fn subscribe_repos( ) { return err.into_response(); } + // Refused pre-upgrade for the same reason, and under the same decision + // `bot.did.listAgents` is: this stream announces each account as it + // appears and as it writes. See `auth::Disclosure`. + if let Err(err) = auth::require_disclosure( + state.disclosure.list_agents, + "com.atproto.sync.subscribeRepos", + ) { + return err.into_response(); + } let cursor = params.cursor(); let exclude = params.exclusions(); upgrade.on_upgrade(move |socket| pump_repos(socket, state.repos, cursor, exclude)) diff --git a/crates/didbot-serve/src/tests.rs b/crates/didbot-serve/src/tests.rs index 31ca3a8f..74c86c9a 100644 --- a/crates/didbot-serve/src/tests.rs +++ b/crates/didbot-serve/src/tests.rs @@ -4336,6 +4336,88 @@ async fn a_development_run_answers_only_callers_on_this_host() { assert_eq!(status, StatusCode::OK, "{body}"); } +/// **What closing `listAgents` is for.** An operator closes it so a stranger +/// cannot enumerate the accounts this deployment holds. Four other public +/// routes name the same accounts: `com.atproto.sync.listRepos` pages through +/// them, and `subscribeRepos`, `/events` and `/firehose` announce each one. +/// Closing the first and leaving those open answered `DisclosureDisabled` +/// over a roster still readable four other ways. +#[tokio::test] +async fn closing_list_agents_closes_every_route_that_names_the_accounts() { + let enumerating = [ + "/xrpc/bot.did.listAgents", + "/xrpc/com.atproto.sync.listRepos", + "/events", + "/firehose", + ]; + + // A router per call: `/events` and `/firehose` hold their stream open + // for as long as something still holds the sink, and `call_on` reads the + // body to the end. + let router_with = |disclosure| { + app_with_auth( + Arc::new(FakeRegistry::seeded("kestrel")) as Arc, + BroadcastSink::default(), + Firehose::default(), + AuthState { + lifecycle: Arc::new(didbot_pds::ServerLifecycle::claimed()), + disclosure, + ..AuthState::default() + }, + crate::health::HealthState::new(), + ) + }; + + for uri in enumerating { + let (status, _, body) = call_on(router_with(Disclosure::default()), get(uri)).await; + assert_ne!( + status, + StatusCode::FORBIDDEN, + "{uri} is closed with disclosure open: {body}" + ); + } + + let quiet = Disclosure { + list_agents: false, + ..Disclosure::default() + }; + for uri in enumerating { + let (status, _, body) = call_on(router_with(quiet), get(uri)).await; + assert_eq!(status, StatusCode::FORBIDDEN, "{uri} still answers: {body}"); + assert_eq!(body["error"], json!("DisclosureDisabled"), "{uri}"); + } + + // The other disclosure routes are untouched: the toggle is per route, + // and this one covers the roster rather than the ledger. + let (status, _, _) = call_on(router_with(quiet), get("/xrpc/bot.did.stats")).await; + assert_eq!(status, StatusCode::OK); +} + +/// `subscribeRepos`'s half of the route above, read out of the source. +/// +/// `oneshot` cannot reach that handler: axum's upgrade extractor rejects a +/// synthesised upgrade request with `426` before anything runs, which is why +/// `a_booting_server_refuses_subscribe_repos_before_the_upgrade` checks its +/// own refusal for what it is *not*. The stream announces every account this +/// deployment holds, so it is under the same decision — before `on_upgrade`, +/// for the reason that function's own doc gives. +#[test] +fn subscribe_repos_asks_the_same_disclosure_question_before_it_upgrades() { + let source = include_str!("routes.rs"); + let handler = source + .split_once("async fn subscribe_repos(") + .expect("the handler is in this file") + .1 + .split_once("upgrade.on_upgrade(") + .expect("the handler upgrades at the end") + .0; + assert!( + handler.contains("auth::require_disclosure(") + && handler.contains("state.disclosure.list_agents"), + "subscribe_repos no longer asks the disclosure question before it upgrades" + ); +} + // --------------------------------------------------------------------------- // The health tick // --------------------------------------------------------------------------- -- 2.51.2