fix(gl): make doctor exit non-zero on Fail-class checks (#357) (#547)

* fix(gl): make doctor exit non-zero on Fail-class checks (#357)

gl doctor printed per-row status and 'Some checks failed' prose, but its
exit code was always 0. A diagnostic whose exit status cannot express
failure is a trap waiting for the first 'gl doctor && ...' to land
somewhere it should not (#357).

Three changes:

1. Re-tier the 'GITLAWB_NODE unset' check from Fail to Warn. The CLI's
   --node flag defaults to https://node.gitlawb.com (PUBLIC_NODE) and the
   CLI works fine without the env var, so an unset env is advisory, not
   a failure. Without this re-tier the obvious 'return non-zero on any
   failure' change would have flipped gl doctor to exit 1 on a stock
   working install, which is exactly the regression the issue warns
   against.

2. Route the exit-code decision through a new has_failures(&[Check])
   helper: exit 1 if and only if at least one row is Fail-class.
   Warn-class rows (iCaptcha offline, version drift, shell-alias
   shadowing, GITLAWB_NODE unset) keep the process at exit 0 because
   those conditions are still printed to the user and the obvious
   '&& pipeline' pattern stays valid. Use std::process::exit(1) rather
   than returning Err so anyhow's error frame does not duplicate the
   user-facing summary that already prints.

3. Three unit tests pin the new predicate against the three regimes:
   all-Ok, warn-only, and a single Fail row tripping the exit. The
   warn-only case is the regression guard for the re-tier: if anyone
   flips GITLAWB_NODE back to Fail, exit_predicate_is_false_for_warn_only
   fails too, so the two halves of the fix cannot drift apart.

Verified end-to-end against the public gitlawb node:
  * unset GITLAWB_NODE, missing identity/registration  -> exit 1
  * unset GITLAWB_NODE, full healthy install           -> exit 0 (warn)
  * missing git-remote-gitlawb                         -> exit 1

* fix(gl): pin doctor exit wiring and GITLAWB_NODE Warn tier

- Extract GITLAWB_NODE env classification into gitlawb_node_env_check()
  so unit tests can pin the Warn tier directly; the previous
  has_failures-only tests built their own Check::warn rows and could not
  see a re-tier back to Fail.

- Add three unit tests for the extracted helper (unset/empty is Warn,
  set is Pass, loopback is Pass).

- Add integration probe crates/gl/tests/doctor_exit.rs that drives the
  real binary with a temp --dir and --node http://127.0.0.1:1 and asserts
  exit 1. Deleting the std::process::exit(1) gate keeps the unit suite
  green but breaks this probe, closing the gap reviewer noted.

Co-authored-by: review fix for #391

* fix(gl): align GITLAWB_NODE operator copy with PUBLIC_NODE

* fix(gl): dial the public node when GITLAWB_NODE is empty

Check 3 groups an empty env with unset (Warn), but clap still binds
args.node to "" and the connectivity probe dialed it verbatim, so a
GITLAWB_NODE="" install showed a Fail row and exited 1. effective_node_url
normalizes empty or whitespace-only input to PUBLIC_NODE before the
probe so empty matches unset on check 4.

---------

Co-authored-by: Ayush7614 <[email protected]>

crates/gitlawb-node/src/api/repos.rs

@@ -3,6 +3,8 @@ use axum::http::StatusCode;
 use axum::response::Response;
 use axum::Json;
 use bytes::Bytes;
+use futures::{stream, StreamExt};
+use std::future::Future;
 use std::path::PathBuf;
 use std::sync::Arc;
 
@@ -22,6 +24,18 @@ use crate::webhooks;
 /// The git all-zeros object id — the create/delete sentinel in a ref update.
 const ZERO_SHA: &str = "0000000000000000000000000000000000000000";
 
+const FEDERATED_PEER_CONCURRENCY: usize = 4;
+const FEDERATED_PEER_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5);
+const FEDERATED_AGGREGATE_DEADLINE: std::time::Duration = std::time::Duration::from_secs(10);
+pub(crate) const MAX_FEDERATED_PEERS: usize = 200;
+const MAX_FEDERATED_PEER_BYTES: usize = 512 * 1024;
+const MAX_FEDERATED_PEER_REPOS: usize = 200;
+const MAX_FEDERATED_REPOS: usize = 1_000;
+const MAX_FEDERATED_REPO_JSON_BYTES: usize = 2 * 1024 * 1024;
+// The aggregate byte budget counts serialized repo values and separators. Keep
+// a fixed reserve for the response object's keys, counters, and closing bytes.
+const FEDERATED_RESPONSE_OVERHEAD_BYTES: usize = 256;
+
 /// The set of blob and withheld-subtree tree OIDs withheld from **anonymous**
 /// replication for a repo, or `None` when the repo must not replicate at all
 /// (private / mode A / undetermined, fail closed). This is the anonymous
@@ -3208,11 +3222,206 @@ pub async fn list_refs(
     ))
 }
 
+struct FederatedPeerRepos {
+    node_url: String,
+    node_did: String,
+    repos: Vec<serde_json::Value>,
+    truncated: bool,
+}
+
+struct FederatedPeerPage {
+    repos: Vec<serde_json::Value>,
+    truncated: bool,
+}
+
+struct BoundedFederatedPeerRows(Vec<serde_json::Value>, bool);
+
+impl<'de> serde::Deserialize<'de> for BoundedFederatedPeerRows {
+    fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        struct RowsVisitor;
+
+        impl<'de> serde::de::Visitor<'de> for RowsVisitor {
+            type Value = BoundedFederatedPeerRows;
+
+            fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+                formatter.write_str("a bounded array of repository objects")
+            }
+
+            fn visit_seq<A>(self, mut rows: A) -> std::result::Result<Self::Value, A::Error>
+            where
+                A: serde::de::SeqAccess<'de>,
+            {
+                let mut repos = Vec::with_capacity(MAX_FEDERATED_PEER_REPOS);
+                while repos.len() < MAX_FEDERATED_PEER_REPOS {
+                    let Some(repo) = rows.next_element::<serde_json::Value>()? else {
+                        return Ok(BoundedFederatedPeerRows(repos, false));
+                    };
+                    if !repo.is_object() {
+                        return Err(<A::Error as serde::de::Error>::custom(
+                            "peer repository row is not an object",
+                        ));
+                    }
+                    repos.push(repo);
+                }
+                let mut truncated = false;
+                while rows.next_element::<serde::de::IgnoredAny>()?.is_some() {
+                    truncated = true;
+                }
+                Ok(BoundedFederatedPeerRows(repos, truncated))
+            }
+        }
+
+        deserializer.deserialize_seq(RowsVisitor)
+    }
+}
+
+struct FederatedRepoBudget {
+    repos: Vec<serde_json::Value>,
+    json_bytes: usize,
+    max_repos: usize,
+    max_json_bytes: usize,
+    truncated: bool,
+}
+
+impl FederatedRepoBudget {
+    fn new(capacity: usize, max_repos: usize, max_json_bytes: usize) -> Self {
+        Self {
+            repos: Vec::with_capacity(capacity.min(max_repos)),
+            json_bytes: FEDERATED_RESPONSE_OVERHEAD_BYTES,
+            max_repos,
+            max_json_bytes,
+            truncated: false,
+        }
+    }
+
+    fn push(&mut self, repo: serde_json::Value) -> bool {
+        if self.is_saturated() {
+            self.truncated = true;
+            return false;
+        }
+        let Ok(repo_bytes) = serde_json::to_vec(&repo) else {
+            return true;
+        };
+        let charged = repo_bytes.len().saturating_add(1);
+        if self.json_bytes.saturating_add(charged) > self.max_json_bytes {
+            self.truncated = true;
+            return false;
+        }
+        self.json_bytes += charged;
+        self.repos.push(repo);
+        true
+    }
+
+    fn is_saturated(&self) -> bool {
+        self.repos.len() >= self.max_repos || self.json_bytes >= self.max_json_bytes
+    }
+}
+
+async fn fetch_federated_peer_repos(
+    client: &reqwest::Client,
+    url: &str,
+) -> Option<FederatedPeerPage> {
+    let mut response = client.get(url).send().await.ok()?;
+    if !response.status().is_success()
+        || response
+            .content_length()
+            .is_some_and(|len| len > MAX_FEDERATED_PEER_BYTES as u64)
+    {
+        return None;
+    }
+    let total = response
+        .headers()
+        .get("x-total-count")
+        .and_then(|value| value.to_str().ok())
+        .and_then(|value| value.trim().parse::<u64>().ok());
+
+    let capacity = response
+        .content_length()
+        .and_then(|len| usize::try_from(len).ok())
+        .unwrap_or(0)
+        .min(MAX_FEDERATED_PEER_BYTES);
+    let mut body = Vec::with_capacity(capacity);
+    while let Some(chunk) = response.chunk().await.ok()? {
+        if body.len().saturating_add(chunk.len()) > MAX_FEDERATED_PEER_BYTES {
+            return None;
+        }
+        body.extend_from_slice(&chunk);
+    }
+
+    let BoundedFederatedPeerRows(repos, overflow) = serde_json::from_slice(&body).ok()?;
+    let truncated = overflow || total.is_some_and(|total| total > repos.len() as u64);
+    Some(FederatedPeerPage { repos, truncated })
+}
+
+fn enrich_federated_repo(
+    mut repo: serde_json::Value,
+    node_url: &str,
+    node_did: &str,
+) -> Option<serde_json::Value> {
+    let object = repo.as_object_mut()?;
+    object.insert(
+        "node_url".to_string(),
+        serde_json::Value::String(node_url.to_string()),
+    );
+    object.insert(
+        "node_did".to_string(),
+        serde_json::Value::String(node_did.to_string()),
+    );
+    object.insert("local".to_string(), serde_json::Value::Bool(false));
+    Some(repo)
+}
+
+async fn collect_federated_fetches<I, F>(fetches: I, aggregate: &mut FederatedRepoBudget) -> usize
+where
+    I: IntoIterator<Item = F>,
+    F: Future<Output = Option<FederatedPeerRepos>>,
+{
+    let mut responses = stream::iter(fetches).buffer_unordered(FEDERATED_PEER_CONCURRENCY);
+    let mut nodes_queried = 0;
+
+    let deadline = tokio::time::Instant::now() + FEDERATED_AGGREGATE_DEADLINE;
+    loop {
+        let response = match tokio::time::timeout_at(deadline, responses.next()).await {
+            Ok(Some(response)) => response,
+            Ok(None) => break,
+            Err(_) => {
+                aggregate.truncated = true;
+                tracing::warn!("federated aggregation deadline exceeded");
+                break;
+            }
+        };
+        let Some(response) = response else {
+            aggregate.truncated = true;
+            tracing::warn!("federated peer fetch failed or exceeded its response limits");
+            continue;
+        };
+        nodes_queried += 1;
+        aggregate.truncated |= response.truncated;
+        for repo in response.repos {
+            let Some(repo) = enrich_federated_repo(repo, &response.node_url, &response.node_did)
+            else {
+                continue;
+            };
+            if !aggregate.push(repo) || aggregate.is_saturated() {
+                aggregate.truncated = true;
+                return nodes_queried;
+            }
+        }
+    }
+
+    nodes_queried
+}
+
 /// GET /api/v1/repos/federated
 ///
 /// Query all known peers for their public repos and return a merged view of
 /// the network. Each repo includes a `node_url` and `node_did` indicating
-/// which node hosts it. Results from unreachable peers are silently omitted.
+/// which node hosts it. Peer work is bounded by `FEDERATED_AGGREGATE_DEADLINE`
+/// and `MAX_FEDERATED_PEERS`; omitted
+/// results set `truncated`. The route admits 12 requests per minute per client IP.
 pub async fn list_federated_repos(
     State(state): State<AppState>,
     auth: Option<Extension<AuthenticatedDid>>,
@@ -3240,66 +3449,76 @@ pub async fn list_federated_repos(
         .unwrap_or_else(|| "http://127.0.0.1:7545".to_string());
     let local_node_did = state.node_did.to_string();
 
-    let mut all_repos: Vec<serde_json::Value> = Vec::with_capacity(local_repos.len());
+    let mut aggregate = FederatedRepoBudget::new(
+        local_repos.len(),
+        MAX_FEDERATED_REPOS,
+        MAX_FEDERATED_REPO_JSON_BYTES,
+    );
     for (r, count) in &local_repos {
-        let mut v = serde_json::to_value(to_response(r, &state, *count)).unwrap_or_default();
-        v["node_url"] = serde_json::Value::String(local_node_url.clone());
-        v["node_did"] = serde_json::Value::String(local_node_did.clone());
-        v["local"] = serde_json::Value::Bool(true);
-        all_repos.push(v);
-    }
-
-    // Query peers in parallel
-    let peers = state.db.list_peers().await.unwrap_or_default();
-    let client = &state.http_client;
-
-    let fetch_tasks: Vec<_> = peers
-        .into_iter()
-        .filter(|p| p.last_ping_ok && !p.http_url.is_empty())
-        .map(|peer| {
-            let client = Arc::clone(client);
-            let url = format!("{}/api/v1/repos", peer.http_url.trim_end_matches('/'));
-            let peer_did = peer.did.clone();
-            let peer_url = peer.http_url.clone();
-            tokio::spawn(async move {
-                let result = tokio::time::timeout(
-                    std::time::Duration::from_secs(5),
-                    client.get(&url).send(),
-                )
-                .await;
-                match result {
-                    Ok(Ok(resp)) if resp.status().is_success() => {
-                        if let Ok(repos) = resp.json::<Vec<serde_json::Value>>().await {
-                            let enriched: Vec<serde_json::Value> = repos
-                                .into_iter()
-                                .map(|mut r| {
-                                    r["node_url"] = serde_json::Value::String(peer_url.clone());
-                                    r["node_did"] = serde_json::Value::String(peer_did.clone());
-                                    r["local"] = serde_json::Value::Bool(false);
-                                    r
-                                })
-                                .collect();
-                            return enriched;
-                        }
-                    }
-                    _ => {}
-                }
-                vec![]
-            })
-        })
-        .collect();
-
-    for task in fetch_tasks {
-        if let Ok(repos) = task.await {
-            all_repos.extend(repos);
+        let Ok(mut repo) = serde_json::to_value(to_response(r, &state, *count)) else {
+            continue;
+        };
+        repo["node_url"] = serde_json::Value::String(local_node_url.clone());
+        repo["node_did"] = serde_json::Value::String(local_node_did.clone());
+        repo["local"] = serde_json::Value::Bool(true);
+        if !aggregate.push(repo) || aggregate.is_saturated() {
+            aggregate.truncated = true;
+            break;
         }
     }
 
-    let count = all_repos.len();
+    // Keep peer work bounded even when the table contains many reachable rows.
+    // Dropping the buffered stream at the aggregate ceiling cancels its in-flight
+    // request futures and leaves the remaining iterator entries unpolled.
+    // One extra row detects omitted peers without materializing the full table.
+    let mut peers = state
+        .db
+        .list_federation_peers(MAX_FEDERATED_PEERS as i64 + 1)
+        .await?;
+    aggregate.truncated |= peers.len() > MAX_FEDERATED_PEERS;
+    peers.truncate(MAX_FEDERATED_PEERS);
+    let fetches: Vec<_> = peers
+        .into_iter()
+        .filter(|p| p.last_ping_ok && !p.http_url.is_empty())
+        .map(|peer| {
+            let client = Arc::clone(&state.http_client);
+            let url = format!(
+                "{}/api/v1/repos?limit={MAX_FEDERATED_PEER_REPOS}&offset=0",
+                peer.http_url.trim_end_matches('/')
+            );
+            let peer_did = peer.did.clone();
+            let peer_url = peer.http_url.clone();
+            async move {
+                tokio::time::timeout(
+                    FEDERATED_PEER_TIMEOUT,
+                    fetch_federated_peer_repos(&client, &url),
+                )
+                .await
+                .ok()
+                .flatten()
+                .map(|page| FederatedPeerRepos {
+                    node_url: peer_url,
+                    node_did: peer_did,
+                    repos: page.repos,
+                    truncated: page.truncated,
+                })
+            }
+        })
+        .collect();
+
+    let peer_nodes_queried = if aggregate.is_saturated() {
+        0
+    } else {
+        collect_federated_fetches(fetches, &mut aggregate).await
+    };
+
+    let count = aggregate.repos.len();
+    let truncated = aggregate.truncated;
     Ok(Json(serde_json::json!({
-        "repos": all_repos,
+        "repos": aggregate.repos,
         "count": count,
-        "nodes_queried": 1, // local + peers that responded
+        "nodes_queried": 1 + peer_nodes_queried,
+        "truncated": truncated,
     })))
 }
 
@@ -4285,6 +4504,46 @@ mod tests {
         }
     }
 
+    #[sqlx::test]
+    async fn federated_peer_query_is_bounded(pool: sqlx::PgPool) {
+        let state = crate::test_support::test_state(pool.clone()).await;
+        sqlx::query("INSERT INTO peers (did, http_url, last_ping_ok, announced_at) SELECT 'peer-' || n, 'https://example.com', TRUE, '2026-01-01T00:00:00Z' FROM generate_series(1, $1) n")
+            .bind(MAX_FEDERATED_PEERS as i64 + 5)
+            .execute(&pool).await.unwrap();
+        let peers = state.db.list_federation_peers(i64::MAX).await.unwrap();
+        assert_eq!(peers.len(), MAX_FEDERATED_PEERS + 1);
+        assert_eq!(state.db.list_federation_peers(2).await.unwrap().len(), 2);
+        assert!(state.db.list_federation_peers(0).await.unwrap().is_empty());
+    }
+
+    #[sqlx::test]
+    async fn federated_route_brakes_repeated_anonymous_requests(pool: sqlx::PgPool) {
+        use tower::ServiceExt;
+        let mut state = crate::test_support::test_state(pool).await;
+        state.push_limiter_trust = crate::rate_limit::TrustedProxy::None;
+        state.federated_rate_limiter =
+            crate::rate_limit::RateLimiter::new_bounded(1, std::time::Duration::from_secs(60), 10);
+        let router = crate::server::build_router(state);
+        for n in 0..2 {
+            let mut request = axum::http::Request::builder()
+                .uri("/api/v1/repos/federated")
+                .body(axum::body::Body::empty())
+                .unwrap();
+            request.extensions_mut().insert(axum::extract::ConnectInfo(
+                "203.0.113.42:5000".parse::<std::net::SocketAddr>().unwrap(),
+            ));
+            let response = router.clone().oneshot(request).await.unwrap();
+            assert_eq!(
+                response.status(),
+                if n == 0 {
+                    StatusCode::OK
+                } else {
+                    StatusCode::TOO_MANY_REQUESTS
+                }
+            );
+        }
+    }
+
     #[test]
     fn fork_clone_guard_removes_its_own_mirror_on_drop() {
         let root = tempfile::TempDir::new().unwrap();
@@ -4332,6 +4591,464 @@ mod tests {
         );
     }
 
+    #[tokio::test]
+    async fn federated_route_uses_the_state_owned_bucket_before_database_access() {
+        use tower::ServiceExt;
+        let mut state = crate::test_support::test_state_lazy();
+        state.push_limiter_trust = crate::rate_limit::TrustedProxy::None;
+        state.federated_rate_limiter =
+            crate::rate_limit::RateLimiter::new_bounded(1, std::time::Duration::from_secs(60), 10);
+        assert!(state.federated_rate_limiter.check("203.0.113.42").await);
+        // Both routers must use the same exhausted bucket from AppState.
+        for router in [
+            crate::server::build_router(state.clone()),
+            crate::server::build_router(state),
+        ] {
+            let mut request = axum::http::Request::builder()
+                .uri("/api/v1/repos/federated")
+                .body(axum::body::Body::empty())
+                .unwrap();
+            request.extensions_mut().insert(axum::extract::ConnectInfo(
+                "203.0.113.42:5000".parse::<std::net::SocketAddr>().unwrap(),
+            ));
+            let response =
+                tokio::time::timeout(std::time::Duration::from_secs(1), router.oneshot(request))
+                    .await
+                    .expect("the exhausted bucket must reject before accessing the lazy database")
+                    .unwrap();
+            assert_eq!(response.status(), StatusCode::TOO_MANY_REQUESTS);
+        }
+    }
+
+    #[tokio::test]
+    async fn federated_aggregate_deadline_returns_partial_results() {
+        let mut budget = FederatedRepoBudget::new(0, 1000, MAX_FEDERATED_REPO_JSON_BYTES);
+        let fetches =
+            (0..8).map(|_| async { std::future::pending::<Option<FederatedPeerRepos>>().await });
+        let nodes = tokio::time::timeout(
+            FEDERATED_AGGREGATE_DEADLINE + std::time::Duration::from_secs(2),
+            collect_federated_fetches(fetches, &mut budget),
+        )
+        .await
+        .expect("the aggregate must stop before the caller deadline");
+        assert_eq!(nodes, 0);
+        assert!(budget.truncated);
+    }
+
+    #[tokio::test]
+    async fn federated_peer_response_enforces_byte_and_row_ceilings() {
+        let mut row_server = mockito::Server::new_async().await;
+        let row_body =
+            serde_json::to_string(&vec![serde_json::json!({}); MAX_FEDERATED_PEER_REPOS + 1])
+                .unwrap();
+        let row_mock = row_server
+            .mock("GET", "/api/v1/repos")
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(row_body)
+            .expect(1)
+            .create_async()
+            .await;
+
+        let client = reqwest::Client::new();
+        let page =
+            fetch_federated_peer_repos(&client, &format!("{}/api/v1/repos", row_server.url()))
+                .await
+                .unwrap();
+        assert_eq!(page.repos.len(), MAX_FEDERATED_PEER_REPOS);
+        assert!(page.truncated);
+        row_mock.assert_async().await;
+
+        let byte_body = serde_json::to_string(&vec![serde_json::json!({
+            "description": "x".repeat(MAX_FEDERATED_PEER_BYTES)
+        })])
+        .unwrap();
+        let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
+        let address = listener.local_addr().unwrap();
+        let byte_server = tokio::spawn(async move {
+            use tokio::io::{AsyncReadExt, AsyncWriteExt};
+
+            let (mut socket, _) = listener.accept().await.unwrap();
+            let mut request = [0; 1_024];
+            let _ = socket.read(&mut request).await.unwrap();
+            socket
+                .write_all(
+                    format!(
+                        "HTTP/1.1 200 OK\r\ncontent-type: application/json\r\n\
+                         transfer-encoding: chunked\r\nconnection: close\r\n\r\n{:x}\r\n",
+                        byte_body.len()
+                    )
+                    .as_bytes(),
+                )
+                .await
+                .unwrap();
+            let _ = socket.write_all(byte_body.as_bytes()).await;
+            let _ = socket.write_all(b"\r\n0\r\n\r\n").await;
+        });
+
+        assert!(
+            fetch_federated_peer_repos(&client, &format!("http://{address}/api/v1/repos"))
+                .await
+                .is_none(),
+            "a chunked peer body above the byte ceiling must be omitted without Content-Length"
+        );
+        byte_server.await.unwrap();
+    }
+
+    #[tokio::test]
+    async fn federated_peer_total_count_prevents_false_complete_page() {
+        let mut server = mockito::Server::new_async().await;
+        let body = serde_json::to_string(
+            &(0..MAX_FEDERATED_PEER_REPOS)
+                .map(|id| serde_json::json!({ "id": id }))
+                .collect::<Vec<_>>(),
+        )
+        .unwrap();
+        let page_mock = server
+            .mock("GET", "/api/v1/repos")
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_header("x-total-count", &(MAX_FEDERATED_PEER_REPOS + 1).to_string())
+            .with_body(body)
+            .expect(1)
+            .create_async()
+            .await;
+
+        let page = fetch_federated_peer_repos(
+            &reqwest::Client::new(),
+            &format!("{}/api/v1/repos", server.url()),
+        )
+        .await
+        .expect("the capped peer page is otherwise valid");
+        page_mock.assert_async().await;
+        assert_eq!(page.repos.len(), MAX_FEDERATED_PEER_REPOS);
+        assert!(page.truncated, "the peer total proves another row exists");
+
+        let mut aggregate = FederatedRepoBudget::new(300, 300, MAX_FEDERATED_REPO_JSON_BYTES);
+        let nodes = collect_federated_fetches(
+            [std::future::ready(Some(FederatedPeerRepos {
+                node_url: server.url(),
+                node_did: "did:key:peer".to_string(),
+                repos: page.repos,
+                truncated: page.truncated,
+            }))],
+            &mut aggregate,
+        )
+        .await;
+        assert_eq!(nodes, 1);
+        assert_eq!(aggregate.repos.len(), MAX_FEDERATED_PEER_REPOS);
+        assert!(
+            aggregate.truncated,
+            "the aggregate must not report the paged peer as complete"
+        );
+
+        let mut malformed_server = mockito::Server::new_async().await;
+        let malformed_mock = malformed_server
+            .mock("GET", "/api/v1/repos")
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_header("x-total-count", "not-a-count")
+            .with_body("[{}]")
+            .expect(1)
+            .create_async()
+            .await;
+        let malformed = fetch_federated_peer_repos(
+            &reqwest::Client::new(),
+            &format!("{}/api/v1/repos", malformed_server.url()),
+        )
+        .await
+        .expect("a malformed optional total must not discard a bounded page");
+        malformed_mock.assert_async().await;
+        assert!(!malformed.truncated);
+        assert_eq!(malformed.repos.len(), 1);
+    }
+
+    #[test]
+    fn federated_aggregate_stays_inside_row_and_serialized_byte_budgets() {
+        let mut rows = FederatedRepoBudget::new(3, 2, 4_096);
+        assert!(rows.push(serde_json::json!({ "name": "one" })));
+        assert!(rows.push(serde_json::json!({ "name": "two" })));
+        assert!(rows.is_saturated());
+        assert!(!rows.push(serde_json::json!({ "name": "three" })));
+        assert_eq!(rows.repos.len(), 2);
+        assert!(rows.truncated);
+
+        let max_bytes = 600;
+        let mut bytes = FederatedRepoBudget::new(8, 8, max_bytes);
+        while bytes.push(serde_json::json!({ "description": "x".repeat(100) })) {}
+        let count = bytes.repos.len();
+        let truncated = bytes.truncated;
+        let response = serde_json::json!({
+            "repos": bytes.repos,
+            "count": count,
+            "nodes_queried": 1,
+            "truncated": truncated,
+        });
+        assert!(serde_json::to_vec(&response).unwrap().len() <= max_bytes);
+        assert!(response["truncated"].as_bool().unwrap());
+    }
+
+    struct ActiveFetch {
+        active: std::sync::Arc<std::sync::atomic::AtomicUsize>,
+    }
+
+    impl Drop for ActiveFetch {
+        fn drop(&mut self) {
+            self.active
+                .fetch_sub(1, std::sync::atomic::Ordering::SeqCst);
+        }
+    }
+
+    #[tokio::test]
+    async fn federated_collection_bounds_concurrency_and_cancels_at_aggregate_limit() {
+        use std::sync::atomic::{AtomicUsize, Ordering};
+
+        let active = Arc::new(AtomicUsize::new(0));
+        let max_active = Arc::new(AtomicUsize::new(0));
+        let started = Arc::new(AtomicUsize::new(0));
+        let mut senders = Vec::new();
+        let mut fetches = Vec::new();
+
+        for i in 0..10 {
+            let (tx, rx) = tokio::sync::oneshot::channel();
+            senders.push(Some(tx));
+            let active = Arc::clone(&active);
+            let max_active = Arc::clone(&max_active);
+            let started = Arc::clone(&started);
+            fetches.push(async move {
+                started.fetch_add(1, Ordering::SeqCst);
+                let current = active.fetch_add(1, Ordering::SeqCst) + 1;
+                max_active.fetch_max(current, Ordering::SeqCst);
+                let _active = ActiveFetch { active };
+                rx.await.ok()?;
+                Some(FederatedPeerRepos {
+                    node_url: format!("https://peer-{i}.example"),
+                    node_did: format!("did:key:peer-{i}"),
+                    repos: vec![serde_json::json!({ "id": i })],
+                    truncated: false,
+                })
+            });
+        }
+
+        let task = tokio::spawn(async move {
+            let mut aggregate = FederatedRepoBudget::new(1, 1, usize::MAX);
+            let nodes = collect_federated_fetches(fetches, &mut aggregate).await;
+            (aggregate, nodes)
+        });
+        tokio::time::timeout(std::time::Duration::from_secs(1), async {
+            while started.load(Ordering::SeqCst) < FEDERATED_PEER_CONCURRENCY {
+                tokio::task::yield_now().await;
+            }
+        })
+        .await
+        .expect("the bounded fetch window should fill");
+
+        assert_eq!(started.load(Ordering::SeqCst), FEDERATED_PEER_CONCURRENCY);
+        assert_eq!(active.load(Ordering::SeqCst), FEDERATED_PEER_CONCURRENCY);
+        assert_eq!(
+            max_active.load(Ordering::SeqCst),
+            FEDERATED_PEER_CONCURRENCY
+        );
+
+        senders[0].take().unwrap().send(()).unwrap();
+        let (aggregate, nodes) = tokio::time::timeout(std::time::Duration::from_secs(1), task)
+            .await
+            .expect("the aggregate ceiling should stop collection")
+            .unwrap();
+
+        assert_eq!(nodes, 1);
+        assert_eq!(aggregate.repos.len(), 1);
+        assert!(aggregate.truncated);
+        assert_eq!(started.load(Ordering::SeqCst), FEDERATED_PEER_CONCURRENCY);
+        assert_eq!(
+            active.load(Ordering::SeqCst),
+            0,
+            "in-flight fetches must be dropped"
+        );
+    }
+
+    async fn json_body(resp: axum::response::Response) -> serde_json::Value {
+        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .expect("body bytes");
+        serde_json::from_slice(&bytes).expect("json body")
+    }
+
+    fn federated_request() -> axum::http::Request<axum::body::Body> {
+        let mut request = axum::http::Request::builder()
+            .uri("/api/v1/repos/federated")
+            .body(axum::body::Body::empty())
+            .unwrap();
+        request.extensions_mut().insert(axum::extract::ConnectInfo(
+            "203.0.113.42:5000".parse::<std::net::SocketAddr>().unwrap(),
+        ));
+        request
+    }
+
+    /// Pins the federated response contract end to end: the envelope fields
+    /// (`count`, `nodes_queried`, `truncated`) and the per-repo annotations
+    /// (`node_url`, `node_did`, `local`) are asserted on an actual HTTP 200
+    /// body driven through `build_router`, not on a self-constructed value.
+    /// The seeded repo is deliberately owned by a DID other than the node:
+    /// the envelope must carry the node's DID, so a regression that reports
+    /// the owner DID instead would fail the `node_did` assertion.
+    #[sqlx::test]
+    async fn federated_route_envelope_annotates_repos_on_real_200_body(pool: sqlx::PgPool) {
+        use tower::ServiceExt;
+
+        let state = crate::test_support::test_state(pool).await;
+        let node_did = state.node_did.to_string();
+        state
+            .db
+            .create_repo(&repo_owned_by("did:key:z6MkEnvelopeOwnerNotTheNode"))
+            .await
+            .expect("seed local repo");
+
+        let router = crate::server::build_router(state);
+        let response = router.oneshot(federated_request()).await.unwrap();
+        assert_eq!(response.status(), StatusCode::OK);
+        let body = json_body(response).await;
+
+        assert_eq!(body["count"].as_u64(), Some(1));
+        assert_eq!(
+            body["nodes_queried"].as_u64(),
+            Some(1),
+            "no peers seeded, only the local node was queried"
+        );
+        assert_eq!(body["truncated"].as_bool(), Some(false));
+        let repo = &body["repos"][0];
+        assert_eq!(repo["name"].as_str(), Some("demo"));
+        assert_eq!(repo["local"].as_bool(), Some(true));
+        assert_eq!(repo["node_did"].as_str(), Some(node_did.as_str()));
+        assert!(
+            repo["node_url"].as_str().is_some_and(|u| !u.is_empty()),
+            "local repos must carry the node's URL, got {repo}"
+        );
+    }
+
+    /// `truncated` must be observable on the wire when the peer table holds
+    /// more rows than `MAX_FEDERATED_PEERS`. Every seeded peer points at the
+    /// same reachable stub returning `[]`, so no fetch can fail and no peer
+    /// page can report truncation: the count bound is the only reason the
+    /// body says `truncated`. The `nodes_queried` assertion pins the fetch
+    /// side: every peer inside the bound is queried, and dropping the count
+    /// bound would report `truncated: false`.
+    #[sqlx::test]
+    async fn federated_route_reports_truncated_when_peer_table_overflows(pool: sqlx::PgPool) {
+        use tower::ServiceExt;
+
+        let mut peer_server = mockito::Server::new_async().await;
+        let peer_mock = peer_server
+            .mock("GET", "/api/v1/repos")
+            .match_query(mockito::Matcher::Exact(format!(
+                "limit={MAX_FEDERATED_PEER_REPOS}&offset=0"
+            )))
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body("[]")
+            .expect(MAX_FEDERATED_PEERS)
+            .create_async()
+            .await;
+
+        let state = crate::test_support::test_state(pool.clone()).await;
+        sqlx::query("INSERT INTO peers (did, http_url, last_ping_ok, announced_at) SELECT 'peer-' || n, $1, TRUE, '2026-01-01T00:00:00Z' FROM generate_series(1, $2) n")
+            .bind(peer_server.url())
+            .bind(MAX_FEDERATED_PEERS as i64 + 2)
+            .execute(&pool)
+            .await
+            .unwrap();
+
+        let router = crate::server::build_router(state);
+        let response = router.oneshot(federated_request()).await.unwrap();
+        assert_eq!(response.status(), StatusCode::OK);
+        let body = json_body(response).await;
+        assert_eq!(
+            body["truncated"].as_bool(),
+            Some(true),
+            "more peers than the bound must surface as truncated"
+        );
+        assert_eq!(
+            body["nodes_queried"].as_u64(),
+            Some(1 + MAX_FEDERATED_PEERS as u64),
+            "the stub is reachable, so every peer inside the bound is queried"
+        );
+        peer_mock.assert_async().await;
+    }
+
+    /// A single unreachable peer must still flip `truncated` on the wire via
+    /// the failed-fetch path, even though the peer table stays under the
+    /// bound. Guards the `aggregate.truncated = true` on fetch failure.
+    #[sqlx::test]
+    async fn federated_route_reports_truncated_when_peer_fetch_fails(pool: sqlx::PgPool) {
+        use tower::ServiceExt;
+
+        let state = crate::test_support::test_state(pool.clone()).await;
+        sqlx::query("INSERT INTO peers (did, http_url, last_ping_ok, announced_at) VALUES ('did:key:deadpeer', 'http://127.0.0.1:1/', TRUE, '2026-01-01T00:00:00Z')")
+            .execute(&pool)
+            .await
+            .unwrap();
+
+        let router = crate::server::build_router(state);
+        let response = router.oneshot(federated_request()).await.unwrap();
+        assert_eq!(response.status(), StatusCode::OK);
+        let body = json_body(response).await;
+        assert_eq!(
+            body["truncated"].as_bool(),
+            Some(true),
+            "a failed peer fetch must surface as truncated"
+        );
+        assert_eq!(body["nodes_queried"].as_u64(), Some(1));
+    }
+
+    /// Peer repos must carry `node_url`, `node_did` and `local: false` on the
+    /// real response body. Guards `enrich_federated_repo`: dropping an
+    /// annotation there must turn this red.
+    #[sqlx::test]
+    async fn federated_route_annotates_peer_repos_on_real_200_body(pool: sqlx::PgPool) {
+        use tower::ServiceExt;
+
+        let mut peer_server = mockito::Server::new_async().await;
+        let peer_mock = peer_server
+            .mock("GET", "/api/v1/repos")
+            // Pin the outbound peer query parameters: production must request
+            // the bounded page. Dropping the parameters stops matching this
+            // mock, so the peer fetch fails and the body reports
+            // `truncated: true` instead of `false` below.
+            .match_query(mockito::Matcher::Exact(format!(
+                "limit={MAX_FEDERATED_PEER_REPOS}&offset=0"
+            )))
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(r#"[{"id": "peer-repo-1", "name": "from-peer"}]"#)
+            .expect(1)
+            .create_async()
+            .await;
+
+        let state = crate::test_support::test_state(pool.clone()).await;
+        sqlx::query("INSERT INTO peers (did, http_url, last_ping_ok, announced_at) VALUES ('did:key:peer1', $1, TRUE, '2026-01-01T00:00:00Z')")
+            .bind(peer_server.url())
+            .execute(&pool)
+            .await
+            .unwrap();
+
+        let router = crate::server::build_router(state);
+        let response = router.oneshot(federated_request()).await.unwrap();
+        assert_eq!(response.status(), StatusCode::OK);
+        let body = json_body(response).await;
+        assert_eq!(body["truncated"].as_bool(), Some(false));
+        assert_eq!(body["nodes_queried"].as_u64(), Some(2));
+        let repo = body["repos"]
+            .as_array()
+            .unwrap()
+            .iter()
+            .find(|r| r["id"].as_str() == Some("peer-repo-1"))
+            .expect("peer repo present in federated body");
+        assert_eq!(repo["node_url"].as_str(), Some(peer_server.url().as_str()));
+        assert_eq!(repo["node_did"].as_str(), Some("did:key:peer1"));
+        assert_eq!(repo["local"].as_bool(), Some(false));
+        peer_mock.assert_async().await;
+    }
+
     #[test]
     fn upload_pack_request_finalizes_only_with_done_pktline() {
         let want = "0032want 1111111111111111111111111111111111111111\n";

crates/gitlawb-node/src/auth/mod.rs

@@ -668,6 +668,7 @@ mod tests {
             push_limiter_trust: crate::rate_limit::TrustedProxy::None,
             sync_trigger_rate_limiter: RateLimiter::new(60, Duration::from_secs(3600)),
             peer_write_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
+            federated_rate_limiter: RateLimiter::new_bounded(12, Duration::from_secs(60), 10_000),
             shutdown_tx: tokio::sync::watch::channel(false).0,
             git_read_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
             git_write_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),

crates/gitlawb-node/src/db/mod.rs

@@ -3027,6 +3027,26 @@ impl Db {
         )
     }
 
+    pub async fn list_federation_peers(&self, limit: i64) -> Result<Vec<PeerRecord>> {
+        let rows = sqlx::query(
+            "SELECT did, http_url, last_seen, last_ping_ok, announced_at
+             FROM peers WHERE last_ping_ok = TRUE AND http_url <> '' ORDER BY last_seen DESC NULLS LAST, did LIMIT $1",
+        )
+        .bind(limit.clamp(0, crate::api::repos::MAX_FEDERATED_PEERS as i64 + 1))
+        .fetch_all(&self.pool)
+        .await?;
+        Ok(rows
+            .into_iter()
+            .map(|r| PeerRecord {
+                did: r.get("did"),
+                http_url: r.get("http_url"),
+                last_seen: r.get("last_seen"),
+                last_ping_ok: r.get::<bool, _>("last_ping_ok"),
+                announced_at: r.get("announced_at"),
+            })
+            .collect())
+    }
+
     pub async fn list_peers(&self) -> Result<Vec<PeerRecord>> {
         let rows = sqlx::query(
             "SELECT did, http_url, last_seen, last_ping_ok, announced_at
@@ -9283,6 +9303,10 @@ mod peer_authority_tests {
 /// | `gossip_ping_round_requires_two_failures_before_persisting_unreachable` (main.rs) | test-only fixture seed. Raw SQL because the test drives the readiness HYSTERESIS, which needs a row already at `last_ping_ok = TRUE` before the round runs; it never exercises the announce gate |
 /// | `manual_ping_uses_readiness_without_mutating_federation_gate` (api/peers.rs) | test-only fixture seed, same shape and same reason: the row under test must pre-exist so the assertion is about what the ping does NOT rewrite |
 /// | `seed_peer` (p2p/mod.rs) | test-only fixture seed. Raw SQL so the gossip ingest tests can drive the known-peer gate directly, including the did:web case `upsert_peer` would refuse |
+/// | `federated_peer_query_is_bounded` (api/repos.rs) | test-only fixture seed for the bounded federation query; inserts reachable rows without exercising peer admission |
+/// | `federated_route_annotates_peer_repos_on_real_200_body` (api/repos.rs) | test-only fixture seed; inserts the mock peer row the bounded federation query reads, so the response-envelope assertions run against a real HTTP 200 body |
+/// | `federated_route_reports_truncated_when_peer_fetch_fails` (api/repos.rs) | test-only fixture seed; seeds one unreachable peer so the fetch-failure path is the witness under test |
+/// | `federated_route_reports_truncated_when_peer_table_overflows` (api/repos.rs) | test-only fixture seed; overflows the peer table past `MAX_FEDERATED_PEERS` so the count bound is the only witness for `truncated` |
 ///
 /// And the `upsert_peer` CALL-SITE authority table, which the ledger above
 /// structurally cannot hold, because the bootstrap site issues no SQL of its own
@@ -9368,6 +9392,13 @@ mod peers_table_writer_guard {
     /// listed function that no longer has one.
     const LEDGER: &[(&str, usize)] = &[
         ("a_legacy_row_can_still_refresh_its_liveness", 1),
+        ("federated_peer_query_is_bounded", 1),
+        ("federated_route_annotates_peer_repos_on_real_200_body", 1),
+        ("federated_route_reports_truncated_when_peer_fetch_fails", 1),
+        (
+            "federated_route_reports_truncated_when_peer_table_overflows",
+            1,
+        ),
         (
             "gossip_ping_round_requires_two_failures_before_persisting_unreachable",
             1,

crates/gitlawb-node/src/main.rs

@@ -429,6 +429,8 @@ async fn main() -> Result<()> {
         std::time::Duration::from_secs(3600),
         200_000,
     );
+    let federated_rate_limiter =
+        rate_limit::RateLimiter::new_bounded(12, std::time::Duration::from_secs(60), 10_000);
     if config.sync_trigger_rate_limit == 0 {
         tracing::warn!(
             "GITLAWB_SYNC_TRIGGER_RATE_LIMIT=0 — /sync/trigger IP rate limiting disabled"
@@ -467,6 +469,7 @@ async fn main() -> Result<()> {
         push_limiter_trust,
         sync_trigger_rate_limiter,
         peer_write_rate_limiter,
+        federated_rate_limiter,
         shutdown_tx: shutdown_tx.clone(),
         git_read_semaphore: Arc::new(tokio::sync::Semaphore::new(config.max_concurrent_git_ops)),
         git_write_semaphore: Arc::new(tokio::sync::Semaphore::new(
@@ -1171,6 +1174,7 @@ mod rate_limiter_sweep_tests {
         state.close_issue_rate_limiter = RateLimiter::new(10, window);
         state.sync_trigger_rate_limiter = RateLimiter::new(10, window);
         state.peer_write_rate_limiter = RateLimiter::new(10, window);
+        state.federated_rate_limiter = RateLimiter::new(10, window);
         state.ipfs_rate_limiter = RateLimiter::new(10, window);
 
         let limiters = |s: &crate::state::AppState| {
@@ -1181,6 +1185,7 @@ mod rate_limiter_sweep_tests {
                 s.close_issue_rate_limiter.clone(),
                 s.sync_trigger_rate_limiter.clone(),
                 s.peer_write_rate_limiter.clone(),
+                s.federated_rate_limiter.clone(),
                 s.ipfs_rate_limiter.clone(),
             ]
         };

crates/gitlawb-node/src/server.rs

@@ -412,7 +412,15 @@ pub fn build_router(state: AppState) -> Router {
     // ── Read routes — open for public repos ───────────────────────────────
     let read_routes = Router::new()
         .route("/api/v1/repos", get(repos::list_repos))
-        .route("/api/v1/repos/federated", get(repos::list_federated_repos))
+        .route(
+            "/api/v1/repos/federated",
+            get(repos::list_federated_repos)
+                .route_layer(middleware::from_fn(rate_limit::rate_limit_by_ip))
+                .route_layer(axum::Extension(rate_limit::IpRateLimiter {
+                    limiter: state.federated_rate_limiter.clone(),
+                    trust: state.push_limiter_trust,
+                })),
+        )
         .route("/api/v1/repos/{owner}/{repo}", get(repos::get_repo))
         .route(
             "/api/v1/repos/{owner}/{repo}/commits",

crates/gitlawb-node/src/state.rs

@@ -190,6 +190,9 @@ pub struct AppState {
     /// sink as trigger and accepts unsigned requests from known peers, so it is
     /// braked too; each peer's distinct IP gets its own bucket.
     pub peer_write_rate_limiter: RateLimiter,
+    /// Per-client-IP federation budget: 12 requests per minute, with at most
+    /// 10,000 tracked source keys. Shared by router instances and swept below.
+    pub federated_rate_limiter: RateLimiter,
     /// Process-wide graceful-shutdown signal. Sending `true` causes every
     /// task that holds a `watch::Receiver` to exit at its next await point.
     /// Used by:
@@ -351,6 +354,7 @@ impl AppState {
         self.ipfs_work_rate_limiter.cleanup().await;
         self.sync_trigger_rate_limiter.cleanup().await;
         self.peer_write_rate_limiter.cleanup().await;
+        self.federated_rate_limiter.cleanup().await;
     }
 
     /// Trigger graceful shutdown. Idempotent — calling more than once

crates/gitlawb-node/src/test_support.rs

@@ -116,6 +116,7 @@ fn build_state(db: Arc<crate::db::Db>, pool: PgPool) -> AppState {
         push_limiter_trust: crate::rate_limit::TrustedProxy::None,
         sync_trigger_rate_limiter: RateLimiter::new(60, Duration::from_secs(3600)),
         peer_write_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
+        federated_rate_limiter: RateLimiter::new_bounded(12, Duration::from_secs(60), 10_000),
         shutdown_tx: tokio::sync::watch::channel(false).0,
         // Generous — no test drives the handler-level shed (git_permit is unit-tested).
         git_read_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),

crates/gl/src/doctor.rs

@@ -3,7 +3,7 @@
 //! Checks:
 //!   1. Identity        — ~/.gitlawb/identity.pem exists, DID parseable
 //!   2. Registration    — ~/.gitlawb/ucan.json exists (registered with a node)
-//!   3. GITLAWB_NODE    — env var is set to a non-localhost URL
+//!   3. GITLAWB_NODE    — env var points at a public node (unset is advisory)
 //!   4. Node            — GITLAWB_NODE is reachable (default: https://node.twigpine.com)
 //!   5. git helper      — git-remote-gitlawb is in PATH
 //!   6. git             — git is installed
@@ -80,7 +80,6 @@ pub async fn run(args: DoctorArgs) -> Result<()> {
     });
 
     let mut checks = Vec::new();
-    let mut all_ok = true;
 
     // ── 1. Identity ───────────────────────────────────────────────────────
     let pem_path = dir.join("identity.pem");
@@ -141,32 +140,16 @@ pub async fn run(args: DoctorArgs) -> Result<()> {
     }
 
     // ── 3. GITLAWB_NODE env var ───────────────────────────────────────────
-    match std::env::var("GITLAWB_NODE") {
-        // A loopback host is a legitimate setup (self-hosted node, dev
-        // harness) — the connectivity check below fails loudly if it is not
-        // actually reachable, so don't red-flag the configuration itself.
-        Ok(v) if is_loopback_url(&v) => {
-            checks.push(Check::pass(
-                "GITLAWB_NODE",
-                format!(
-                    "{v} (local node — intentional for self-hosting/dev; unset to target the public network)"
-                ),
-            ));
-        }
-        Ok(v) if !v.is_empty() => {
-            checks.push(Check::pass("GITLAWB_NODE", v.to_string()));
-        }
-        _ => {
-            checks.push(Check::fail(
-                "GITLAWB_NODE",
-                "not set — git-remote-gitlawb will fall back to http://127.0.0.1:7545",
-                "export GITLAWB_NODE=https://node.twigpine.com",
-            ));
-        }
-    }
+    checks.push(gitlawb_node_env_check(
+        std::env::var("GITLAWB_NODE").ok().as_deref(),
+    ));
 
     // ── 4. Node connectivity ──────────────────────────────────────────────
-    let client = NodeClient::new(&args.node, None);
+    // GITLAWB_NODE="" binds args.node to "" through clap, and check 3 groups
+    // empty with unset. The probe must apply the same normalization or it
+    // would dial "" and Fail while the env row only Warns.
+    let node_url = effective_node_url(&args.node);
+    let client = NodeClient::new(node_url, None);
     match client.get("/").await {
         Ok(resp) if resp.status().is_success() => {
             let info = resp.json::<serde_json::Value>().await.unwrap_or_default();
@@ -175,7 +158,7 @@ pub async fn run(args: DoctorArgs) -> Result<()> {
             let short_did = did.chars().take(30).collect::<String>();
             checks.push(Check::pass(
                 "node",
-                format!("{} — v{version} ({short_did}…)", args.node),
+                format!("{} — v{version} ({short_did}…)", node_url),
             ));
             // Capability drift: a node newer than this CLI may require features
             // (RFC 9421 signing, iCaptcha) the CLI doesn't speak.
@@ -194,14 +177,14 @@ pub async fn run(args: DoctorArgs) -> Result<()> {
         Ok(resp) => {
             checks.push(Check::fail(
                 "node",
-                format!("{} returned HTTP {}", args.node, resp.status()),
+                format!("{} returned HTTP {}", node_url, resp.status()),
                 "check GITLAWB_NODE env var or try gl register --node <url>",
             ));
         }
         Err(e) => {
             checks.push(Check::fail(
                 "node",
-                format!("{} unreachable: {e}", args.node),
+                format!("{} unreachable: {e}", node_url),
                 "check your internet connection or set GITLAWB_NODE",
             ));
         }
@@ -285,23 +268,21 @@ pub async fn run(args: DoctorArgs) -> Result<()> {
             CheckState::Fail => "✗",
         };
         println!("  {icon}  {:<24}  {}", check.label, check.detail);
-        if matches!(check.state, CheckState::Fail) {
-            all_ok = false;
-        }
     }
 
     println!();
 
+    let has_failures_now = has_failures(&checks);
     let has_issues = checks
         .iter()
         .any(|c| matches!(c.state, CheckState::Fail | CheckState::Warn));
     if !has_issues {
         println!("Everything looks good. Run `gl quickstart` to create your first repo.");
     } else {
-        if all_ok {
-            println!("Setup looks good with some warnings:");
-        } else {
+        if has_failures_now {
             println!("Some checks failed. Suggested fixes:");
+        } else {
+            println!("Setup looks good with some warnings:");
         }
         for check in &checks {
             if matches!(check.state, CheckState::Fail | CheckState::Warn) {
@@ -311,14 +292,75 @@ pub async fn run(args: DoctorArgs) -> Result<()> {
             }
         }
         println!();
-        if !all_ok {
+        if has_failures_now {
             println!("Run `gl quickstart` for a guided setup.");
         }
     }
 
+    // #357: a non-zero exit is the only signal scripting and CI have. Warn-only
+    // rows stay exit 0 (iCaptcha offline, version drift, shell-alias shadowing
+    // are all non-fatal); Fail-class rows (identity, registration, node
+    // reachable, git-remote-gitlawb present, git present) flip to exit 1.
+    // Direct `process::exit` rather than returning Err: the prose above is the
+    // user-facing summary, and an anyhow Error frame would just duplicate it.
+    if has_failures(&checks) {
+        std::process::exit(1);
+    }
+
     Ok(())
 }
 
+/// Classify the `GITLAWB_NODE` env var for `gl doctor`. Extracted so unit
+/// tests can pin the Warn tier directly (#357): an unset env is advisory, not
+/// a failure, because `--node` defaults to `https://node.twigpine.com` and the
+/// CLI works without it. Flipping this back to `Check::fail` must break a
+/// test, not just change prose.
+fn gitlawb_node_env_check(env_val: Option<&str>) -> Check {
+    match env_val {
+        // A loopback host is a legitimate setup (self-hosted node, dev
+        // harness) — the connectivity check below fails loudly if it is not
+        // actually reachable, so don't red-flag the configuration itself.
+        Some(v) if is_loopback_url(v) => Check::pass(
+            "GITLAWB_NODE",
+            format!(
+                "{v} (local node — intentional for self-hosting/dev; unset to target the public network)"
+            ),
+        ),
+        Some(v) if !v.is_empty() => Check::pass("GITLAWB_NODE", v.to_string()),
+        // `--node` defaults to `https://node.twigpine.com` and the CLI works
+        // fine without the variable, so an unset env is advisory, not a
+        // failure. Warn-class keeps the exit code 0 for a stock working
+        // install; this used to be Fail, which the obvious "return non-zero
+        // on any failure" change would have flipped to exit 1 on every
+        // default-config install (#357).
+        _ => Check::warn(
+            "GITLAWB_NODE",
+            "not set — git-remote-gitlawb will fall back to http://127.0.0.1:7545",
+            format!("export GITLAWB_NODE={PUBLIC_NODE}"),
+        ),
+    }
+}
+
+/// The URL the connectivity probe should dial. `GITLAWB_NODE` set but empty
+/// (or whitespace-only) binds `args.node` to `""` through clap rather than
+/// to the `default_value`, and check 3 already groups empty with unset, so
+/// the probe applies the same normalization: an empty effective URL means
+/// the public node, same as unset.
+fn effective_node_url(node: &str) -> &str {
+    if node.trim().is_empty() {
+        PUBLIC_NODE
+    } else {
+        node
+    }
+}
+
+/// True when at least one check is Fail-class. Warn-only runs (iCaptcha offline,
+/// version drift, shell-alias shadowing, GITLAWB_NODE unset) keep exit 0;
+/// this is the gating predicate for #357's exit-status fix.
+fn has_failures(checks: &[Check]) -> bool {
+    checks.iter().any(|c| matches!(c.state, CheckState::Fail))
+}
+
 /// Check if a binary name exists anywhere on PATH.
 /// True when the rc file contains a real `unalias` command naming `gl` —
 /// not a comment, and not a longer word like `unalias global`. Ordering
@@ -644,4 +686,103 @@ mod tests {
         assert!(matches!(check.state, CheckState::Ok));
         assert!(check.detail.contains("GitHub API returned HTTP 403"));
     }
+
+    /// #357: an all-Ok run does not flip the exit code to non-zero — a healthy
+    /// install must keep returning 0 so scripts like `gl doctor && gl push`
+    /// keep working.
+    #[test]
+    fn exit_predicate_is_false_for_all_ok() {
+        let checks = vec![
+            Check::pass("identity", "ok"),
+            Check::pass("registration", "ok"),
+            Check::pass("git", "ok"),
+        ];
+        assert!(!has_failures(&checks));
+    }
+
+    /// #357: Warn-class rows (iCaptcha offline, version drift, shell-alias
+    /// shadowing, GITLAWB_NODE unset on a default-config install) are NOT
+    /// exit-status failures — they are still printed to the user but the
+    /// process exits 0. This is the rule that the previous "any
+    /// Fail-or-Warn row → exit 1" change would have broken for stock
+    /// installs that simply have not set the env var.
+    #[test]
+    fn exit_predicate_is_false_for_warn_only() {
+        let checks = vec![
+            Check::pass("identity", "ok"),
+            Check::warn("GITLAWB_NODE", "unset", "export GITLAWB_NODE=..."),
+            Check::warn("iCaptcha", "offline", "check connectivity"),
+            Check::warn("version", "drift", "upgrade"),
+            Check::warn("shell alias", "omz git plugin", "unalias gl"),
+        ];
+        assert!(!has_failures(&checks));
+    }
+
+    /// #357: a single Fail-class row trips the exit code. The set of Fail
+    /// rows is whatever `run` writes through `Check::fail`: identity missing
+    /// or unparseable, registration missing/malformed, node unreachable,
+    /// git-remote-gitlawb absent, git absent.
+    #[test]
+    fn exit_predicate_is_true_when_a_fail_row_is_present() {
+        let checks = vec![
+            Check::pass("registration", "ok"),
+            Check::fail("identity", "missing", "gl identity new"),
+            Check::pass("git", "ok"),
+        ];
+        assert!(has_failures(&checks));
+
+        let mixed_warns = vec![
+            Check::pass("identity", "ok"),
+            Check::warn("GITLAWB_NODE", "unset", "export"),
+            Check::fail("node", "unreachable", "check network"),
+            Check::warn("iCaptcha", "offline", "check"),
+        ];
+        assert!(has_failures(&mixed_warns));
+    }
+
+    /// #357: `GITLAWB_NODE` unset must be Warn, not Fail. This pins the
+    /// re-tier so flipping it back to `Check::fail` breaks the suite — the
+    /// previous `has_failures`-only tests built their own `Check::warn` rows
+    /// and could not see the re-tier at all.
+    #[test]
+    fn gitlawb_node_env_unset_is_warn() {
+        let check = gitlawb_node_env_check(None);
+        assert!(matches!(check.state, CheckState::Warn));
+        assert!(check.detail.contains("not set"));
+        let empty = gitlawb_node_env_check(Some(""));
+        assert!(matches!(empty.state, CheckState::Warn));
+    }
+
+    /// `GITLAWB_NODE=""` binds `args.node` to `""` through clap, and check 3
+    /// groups empty with unset. The probe must dial the same URL unset
+    /// would — removing the normalization makes check 4 probe `""` and Fail
+    /// while the env row only Warns, so an empty env install exits 1.
+    #[test]
+    fn effective_node_url_treats_empty_and_blank_as_unset() {
+        assert_eq!(effective_node_url(""), PUBLIC_NODE);
+        assert_eq!(effective_node_url("   "), PUBLIC_NODE);
+        assert_eq!(effective_node_url("\t\n"), PUBLIC_NODE);
+        assert_eq!(
+            effective_node_url("http://127.0.0.1:7545"),
+            "http://127.0.0.1:7545"
+        );
+    }
+
+    #[test]
+    fn gitlawb_node_env_set_is_pass() {
+        let check = gitlawb_node_env_check(Some("https://node.twigpine.com"));
+        assert!(matches!(check.state, CheckState::Ok));
+        assert!(check.detail.contains("https://node.twigpine.com"));
+    }
+
+    #[test]
+    fn gitlawb_node_env_loopback_is_pass() {
+        for url in ["http://127.0.0.1:7545", "http://localhost:7545"] {
+            let check = gitlawb_node_env_check(Some(url));
+            assert!(
+                matches!(check.state, CheckState::Ok),
+                "loopback {url} must be Ok, not Warn/Fail"
+            );
+        }
+    }
 }

crates/gl/tests/doctor_exit.rs

@@ -0,0 +1,48 @@
+//! #357: pin the `gl doctor` exit-code wiring, not just the predicate.
+//!
+//! The unit tests for `has_failures` never touch `std::process::exit(1)` or
+//! the `GITLAWB_NODE` Warn tier. Deleting the exit gate outright kept
+//! `cargo test -p gl doctor::` green, so this binary probe is the only thing
+//! that breaks when the wiring is gutted.
+
+use std::process::Command;
+
+#[test]
+fn doctor_exits_nonzero_when_fail_rows_present() {
+    let dir = tempfile::tempdir().expect("temp dir");
+    // Use an unroutable node so the "node unreachable" check is Fail-class.
+    // The temp dir has no identity.pem / ucan.json, so identity + registration
+    // are also Fail — at least one Fail must flip the process to exit 1.
+    let bin = env!("CARGO_BIN_EXE_gl");
+    let output = Command::new(bin)
+        .args([
+            "doctor",
+            "--dir",
+            dir.path().to_str().unwrap(),
+            "--node",
+            "http://127.0.0.1:1",
+        ])
+        // Inherit PATH but neutralize GITLAWB_NODE so the test is deterministic
+        // across machines that have it set (otherwise it would be Pass).
+        .env_remove("GITLAWB_NODE")
+        // iCaptcha probes a real URL by default; keep it unreachable too so
+        // it stays Warn and does not affect the Fail gating.
+        .env("GITLAWB_ICAPTCHA_URL", "http://127.0.0.1:1")
+        .output()
+        .expect("run gl doctor");
+
+    // The command should have exited 1 because at least one Fail row exists.
+    // If the `std::process::exit(1)` gate is removed, this becomes 0 and the
+    // test fails — that is the regression it pins.
+    assert!(
+        !output.status.success(),
+        "gl doctor must exit non-zero when Fail rows are present; stdout: {}\nstderr: {}",
+        String::from_utf8_lossy(&output.stdout),
+        String::from_utf8_lossy(&output.stderr)
+    );
+    assert_eq!(
+        output.status.code(),
+        Some(1),
+        "exit code must be 1, not some other non-zero"
+    );
+}