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.

crates/gitlawb-core/src/cert.rs

@@ -5,6 +5,28 @@
 //!
 //! The schema is frozen at v1. All fields are mandatory for forward compatibility.
 //! Nodes that receive a certificate with an unknown version MUST reject it.
+//!
+//! ## Replay
+//!
+//! A certificate proves *who* asserted a ref update and *what* they asserted.
+//! It does not prove the assertion is fresh. [`RefUpdateBody::seq`] and
+//! [`RefUpdateBody::nonce`] are inside the signed bytes (see
+//! [`RefUpdateBody::to_signing_bytes`]), so a verifier can trust the values,
+//! but deciding whether a nonce is being reused or a `seq` is going backwards
+//! needs state.
+//!
+//! This crate is deliberately stateless — it is embedded in both nodes and
+//! clients — so replay is **not** enforced here and cannot be. Ownership of
+//! the check sits with the verifier that accepts and persists certificates,
+//! which must:
+//!
+//! - reject a `nonce` it has already accepted for the same `(repo, ref_name)`;
+//! - reject a `seq` that does not advance past the last accepted `seq` for
+//!   that `(repo, ref_name)`.
+//!
+//! Until such a verifier exists, replay of ref-update certificates is not
+//! enforced anywhere in this workspace. This contract exists so the first one
+//! inherits the requirement instead of re-deriving it.
 
 use std::collections::HashSet;
 
@@ -41,11 +63,19 @@ pub struct RefUpdateBody {
     pub from: String,
     /// The new commit hash (SHA-256 hex).
     pub to: String,
-    /// Monotonically increasing sequence number. Prevents replay.
+    /// Monotonically increasing sequence number, scoped to `(repo, ref_name)`.
+    ///
+    /// Signed, but not enforced by this crate: core holds no state. The
+    /// verifier that persists certificates must reject a `seq` that does not
+    /// advance for the same repo and ref. See the module docs on replay.
     pub seq: u64,
     /// RFC 3339 timestamp of this update.
     pub timestamp: DateTime<Utc>,
-    /// Random nonce for deduplication.
+    /// Random nonce, unique per certificate.
+    ///
+    /// Signed, but not enforced by this crate: core holds no state. The
+    /// verifier that persists certificates must reject a nonce it has already
+    /// accepted for the same repo and ref. See the module docs on replay.
     pub nonce: String,
 }
 
@@ -140,7 +170,16 @@ impl RefUpdateCert {
     ///
     /// Counts distinct signer DIDs, not signature entries: a repeated
     /// signature from the same maintainer counts once.
+    ///
+    /// `threshold` must be at least 1. Zero is rejected instead of being
+    /// treated as "no signatures required": a caller that derives the
+    /// threshold from configuration and gets 0 would otherwise accept any
+    /// certificate, whether its signatures verify or not.
     pub fn satisfies_threshold(&self, maintainers: &[Did], threshold: usize) -> Result<bool> {
+        if threshold == 0 {
+            return Err(Error::RefCert("threshold must be at least 1".to_string()));
+        }
+
         let valid = self.verify_all()?;
         let distinct_signers: HashSet<&Did> =
             valid.iter().filter(|d| maintainers.contains(d)).collect();
@@ -378,7 +417,7 @@ mod tests {
     }
 
     #[test]
-    fn threshold_zero_is_always_satisfied() {
+    fn threshold_zero_is_rejected() {
         let kp = Keypair::generate();
         let cert = RefUpdateCert::new(
             kp.did(),
@@ -389,6 +428,120 @@ mod tests {
             &kp,
         )
         .unwrap();
-        assert!(cert.satisfies_threshold(&[], 0).unwrap());
+
+        // Zero must never read as "no signatures required", even with an empty
+        // maintainer set.
+        let err = cert.satisfies_threshold(&[], 0).unwrap_err();
+        assert!(
+            err.to_string().contains("threshold must be at least 1"),
+            "zero must be refused as an invalid threshold"
+        );
+    }
+
+    /// The threshold is checked before any signature work, so a zero is
+    /// reported as a threshold error even when the certificate's signatures
+    /// are also invalid. Otherwise a broken certificate would let a caller
+    /// mistake the failure for a legitimate verification result.
+    #[test]
+    fn threshold_zero_is_rejected_before_signature_verification() {
+        let kp = Keypair::generate();
+        let other = Keypair::generate();
+        let mut cert = RefUpdateCert::new(
+            kp.did(),
+            "refs/heads/main".to_string(),
+            dummy_hash('0'),
+            dummy_hash('a'),
+            1,
+            &kp,
+        )
+        .unwrap();
+        cert.signatures[0].sig = other.sign_b64(b"unrelated bytes");
+
+        let err = cert.satisfies_threshold(&[kp.did()], 0).unwrap_err();
+        assert!(
+            err.to_string().contains("threshold must be at least 1"),
+            "the invalid threshold must win over the invalid signature"
+        );
+    }
+
+    /// `seq` and `nonce` are the verifier's replay inputs, so they have to be
+    /// inside the signed body; tapping either one must invalidate the
+    /// certificate rather than leave it verifying with different values.
+    #[test]
+    fn replay_fields_are_covered_by_the_signature() {
+        let kp = Keypair::generate();
+        let cert = RefUpdateCert::new(
+            kp.did(),
+            "refs/heads/main".to_string(),
+            dummy_hash('0'),
+            dummy_hash('a'),
+            7,
+            &kp,
+        )
+        .unwrap();
+        cert.verify_all().expect("baseline must verify");
+
+        let mut bumped = cert.clone();
+        bumped.body.seq = 8;
+        assert!(
+            matches!(bumped.verify_all(), Err(Error::SignatureInvalid)),
+            "changing seq must invalidate the signature"
+        );
+
+        let mut renamed = cert.clone();
+        renamed.body.nonce = "different-nonce".to_string();
+        assert!(
+            matches!(renamed.verify_all(), Err(Error::SignatureInvalid)),
+            "changing nonce must invalidate the signature"
+        );
+    }
+
+    /// A signature minted by a real key must not verify when the signer is
+    /// spelled in another multibase alphabet. Otherwise the same identity
+    /// would satisfy a maintainer allowlist under several DID strings.
+    ///
+    /// Base64url is deliberate: its 47-character method-id fits under the
+    /// pre-existing length bound, so this test reaches the canonical-prefix
+    /// rule and fails on the pre-fix code instead of being refused for length.
+    #[test]
+    fn verify_all_rejects_a_non_canonical_did_spelling() {
+        let kp = Keypair::generate();
+        let mut cert = RefUpdateCert::new(
+            kp.did(),
+            "refs/heads/main".to_string(),
+            dummy_hash('0'),
+            dummy_hash('a'),
+            1,
+            &kp,
+        )
+        .unwrap();
+
+        let mut prefixed = vec![0xed, 0x01];
+        prefixed.extend_from_slice(&kp.verifying_key().to_bytes());
+        cert.signatures[0].signer = format!(
+            "did:key:{}",
+            multibase::encode(multibase::Base::Base64Url, &prefixed)
+        )
+        .parse()
+        .expect("method is still `key`");
+
+        let err = cert
+            .verify_all()
+            .expect_err("a non-canonical did:key spelling must not resolve");
+        assert!(
+            err.to_string().contains("base58btc"),
+            "rejection must come from the canonical-prefix rule, not the length guard"
+        );
+        assert!(
+            !matches!(
+                cert.satisfies_threshold(&[cert.signatures[0].signer.clone()], 1),
+                Ok(true)
+            ),
+            "the alternate spelling must not satisfy a threshold"
+        );
+
+        // The canonical spelling of the same key is unaffected.
+        cert.signatures[0].signer = kp.did();
+        assert!(cert.satisfies_threshold(&[kp.did()], 1).unwrap());
     }
 }

crates/gitlawb-core/src/did.rs

@@ -20,6 +20,19 @@ use crate::{Error, Result};
 /// Value: 0xed01 encoded as a varint.
 const ED25519_MULTICODEC: &[u8] = &[0xed, 0x01];
 
+/// The only multibase prefix a `did:key` method-specific id may carry.
+///
+/// The DID:key spec encodes Ed25519 keys as base58btc, which is multibase
+/// `z`. `multibase::decode` happily accepts every other alphabet, and each
+/// spelling of the same key bytes would produce a distinct DID string while
+/// gates, allowlists, and database rows compare DIDs as strings — one
+/// identity would read as several. Rejecting the alternatives here keeps the
+/// mapping from `did:key` to key single-valued for every caller of
+/// [`Did::to_verifying_key`] (HTTP signatures, UCAN, ref certs, peer
+/// announces). `gitlawb-attest` enforces the same rule for attestation
+/// signers.
+const DID_KEY_MULTIBASE_PREFIX: char = 'z';
+
 /// A Decentralized Identifier.
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
 #[serde(transparent)]
@@ -71,8 +84,14 @@ impl Did {
 
     /// Resolve the Ed25519 verifying key from a `did:key`.
     ///
-    /// Returns `Err` if the DID is not a `did:key`, the key bytes are malformed,
-    /// or the key is a small-order (weak) point.
+    /// Only the canonical base58btc (`z`) spelling is accepted. Other
+    /// multibase encodings decode to the same key but spell a different DID
+    /// string, so accepting them would let one identity appear under several
+    /// identifiers to string-comparing gates and allowlists.
+    ///
+    /// Returns `Err` if the DID is not a `did:key`, uses a non-canonical
+    /// multibase prefix, the key bytes are invalid, or the key is a
+    /// small-order (weak) point.
     pub fn to_verifying_key(&self) -> Result<VerifyingKey> {
         if !self.is_did_key() {
             return Err(Error::InvalidDid(format!(
@@ -94,9 +113,27 @@ impl Did {
             ));
         }
 
-        let (_, bytes) =
+        // Cheap prefix gate before the decode, then the base check on what
+        // multibase actually parsed. The first keeps a clearly non-canonical id
+        // off the decoder. The second is belt-and-braces — multibase maps a
+        // leading `z` only to `Base58Btc` — but it states the requirement
+        // directly instead of relying on that mapping, and `gitlawb-attest`
+        // makes the same pair of checks.
+        if !self.method_id().starts_with(DID_KEY_MULTIBASE_PREFIX) {
+            return Err(Error::InvalidDid(
+                "did:key must use base58btc (z-prefix)".to_string(),
+            ));
+        }
+
+        let (base, bytes) =
             multibase::decode(self.method_id()).map_err(|e| Error::InvalidDid(e.to_string()))?;
 
+        if base != Base::Base58Btc {
+            return Err(Error::InvalidDid(
+                "did:key must use base58btc (z-prefix)".to_string(),
+            ));
+        }
+
         if !bytes.starts_with(ED25519_MULTICODEC) {
             return Err(Error::InvalidDid(
                 "not an ed25519 multicodec key".to_string(),
@@ -389,6 +426,124 @@ mod tests {
         did.to_verifying_key().expect("a real did:key must resolve");
     }
 
+    /// `multibase::decode` accepts every alphabet, so the same Ed25519 key can
+    /// be spelled as several `did:key` strings. Only base58btc is canonical;
+    /// the rest must be refused so one identity cannot appear as several to
+    /// gates and allowlists that compare DID strings.
+    #[test]
+    fn to_verifying_key_rejects_non_base58btc_multibase() {
+        use ed25519_dalek::SigningKey;
+        let signing = SigningKey::from_bytes(&[9u8; 32]);
+        let mut prefixed = ED25519_MULTICODEC.to_vec();
+        prefixed.extend_from_slice(&signing.verifying_key().to_bytes());
+
+        let canonical = Did::from_verifying_key(&signing.verifying_key());
+        assert_eq!(
+            canonical.to_verifying_key().unwrap().to_bytes(),
+            signing.verifying_key().to_bytes(),
+            "base58btc is the accepted spelling"
+        );
+
+        // These spellings fit inside the method-id bound, so they reach the
+        // canonical-prefix check and must be refused there.
+        for base in [Base::Base64Url, Base::Base32Lower] {
+            let alternative = format!("did:key:{}", multibase::encode(base, &prefixed));
+            let did: Did = alternative.parse().expect("method is still `key`");
+            assert_ne!(
+                did.as_str(),
+                canonical.as_str(),
+                "the same key must not compare equal under another spelling"
+            );
+
+            let err = did
+                .to_verifying_key()
+                .expect_err("non-canonical multibase must be refused");
+            assert!(
+                matches!(err, Error::InvalidDid(_)),
+                "must be an InvalidDid, got {err:?}"
+            );
+            assert!(
+                err.to_string().contains("base58btc"),
+                "the error must name the canonical encoding: {err}"
+            );
+        }
+
+        // A base16 spelling of the same key is longer than the method-id
+        // bound, so it is refused by the length guard before the prefix check.
+        // Either way it must not resolve.
+        for base in [Base::Base16Lower, Base::Base16Upper] {
+            let alternative = format!("did:key:{}", multibase::encode(base, &prefixed));
+            let did: Did = alternative.parse().expect("method is still `key`");
+            assert!(
+                matches!(did.to_verifying_key(), Err(Error::InvalidDid(_))),
+                "a base16 spelling of a real key must not resolve"
+            );
+        }
+    }
+
+    /// base58flickr (`Z`) is the non-bitcoin sibling alphabet. Its spelling of
+    /// a real key fits under the length bound, so it must be refused for the
+    /// prefix, not merely for its size.
+    #[test]
+    fn to_verifying_key_rejects_a_base58flickr_spelling() {
+        use ed25519_dalek::SigningKey;
+        let signing = SigningKey::from_bytes(&[11u8; 32]);
+        let mut prefixed = ED25519_MULTICODEC.to_vec();
+        prefixed.extend_from_slice(&signing.verifying_key().to_bytes());
+
+        let did: Did = format!(
+            "did:key:{}",
+            multibase::encode(Base::Base58Flickr, &prefixed)
+        )
+        .parse()
+        .expect("method is still `key`");
+
+        let err = did
+            .to_verifying_key()
+            .expect_err("base58flickr must be refused");
+        assert!(
+            err.to_string().contains("base58btc"),
+            "the error must name the canonical encoding: {err}"
+        );
+    }
+
+    /// An empty method-specific id must be refused by the prefix rule without
+    /// panicking on a missing first character.
+    #[test]
+    fn to_verifying_key_rejects_an_empty_method_id() {
+        let did: Did = "did:key:".parse().expect("method is still `key`");
+        let err = did
+            .to_verifying_key()
+            .expect_err("an empty method id must be refused");
+        assert!(
+            err.to_string().contains("base58btc"),
+            "the error must name the canonical encoding: {err}"
+        );
+    }
+
+    /// A canonical base58btc spelling is not enough on its own: the bytes must
+    /// also carry the ed25519-pub multicodec. This pins the check that stops a
+    /// key of another type from resolving as an Ed25519 key.
+    #[test]
+    fn to_verifying_key_rejects_base58btc_with_a_wrong_multicodec() {
+        use ed25519_dalek::SigningKey;
+        let signing = SigningKey::from_bytes(&[12u8; 32]);
+        let mut buf = vec![0x12, 0x00]; // sha2-256, not ed25519-pub
+        buf.extend_from_slice(&signing.verifying_key().to_bytes());
+
+        let did: Did = format!("did:key:{}", multibase::encode(Base::Base58Btc, &buf))
+            .parse()
+            .expect("method is still `key`");
+
+        let err = did
+            .to_verifying_key()
+            .expect_err("a non-ed25519 multicodec must be refused");
+        assert!(
+            err.to_string().contains("ed25519"),
+            "the error must name the expected multicodec: {err}"
+        );
+    }
+
     /// A `did:key` can encode a small-order (weak) Ed25519 point. Such a key
     /// satisfies the verification equation for any message and, converted to
     /// Montgomery form for X25519, yields the all-zero shared secret, so an

crates/gitlawb-core/src/http_sig.rs

@@ -566,6 +566,26 @@ mod tests {
         assert!(sig.check_created().is_ok());
     }
 
+    #[test]
+    fn extreme_created_timestamps_reject_without_wrapping() {
+        let kp = Keypair::generate();
+        let did = kp.did();
+        let parse = |created: i64| {
+            let sig_input =
+                format!(r#"sig1=("@method");keyid="{did}";alg="ed25519";created={created}"#);
+            HttpSignature::parse(&sig_input, "sig1=:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA:").unwrap()
+        };
+        // i64::MIN used to panic in debug (subtraction overflow) and wrap to a
+        // passing skew in release; now - 2^63 wrapped to i64::MIN, whose abs()
+        // wrapped back to itself and read as fresh. Both must reject.
+        for created in [i64::MIN, i64::MAX, Utc::now().timestamp() - i64::MAX] {
+            assert!(
+                parse(created).check_created().is_err(),
+                "created={created} must not pass the freshness gate"
+            );
+        }
+    }
+
     #[test]
     fn parse_error_missing_sig1_prefix() {
         let err = HttpSignature::parse(

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

@@ -1,12 +1,13 @@
 //! GET /api/v1/arweave/anchors — list Arweave ref-update anchors.
 
 use axum::{
-    extract::{Query, State},
+    extract::{Extension, Query, State},
     Json,
 };
 use serde::Deserialize;
 
-use crate::error::Result;
+use crate::db::normalize_owner_key;
+use crate::error::{AppError, Result};
 use crate::state::AppState;
 
 #[derive(Debug, Deserialize)]
@@ -21,17 +22,62 @@ fn default_limit() -> i64 {
 }
 
 /// GET /api/v1/arweave/anchors
+///
+/// `?repo=<owner>/<name>` returns anchors for one repository and binds the
+/// request to the canonical read-authorization path used by every other
+/// repo-scoped read: the same `authorize_repo_read` helper, with the caller
+/// identity attached. Missing repositories, quarantined mirrors, and signed
+/// non-readers all collapse to the standard `404` (no existence oracle). The
+/// unscoped listing is auth-only — the #121 contract — and does not admit
+/// visibility filtering, which is the #136 stale-index class and explicitly
+/// out of scope for this auth slice.
 pub async fn list_anchors(
     State(state): State<AppState>,
     Query(q): Query<ListAnchorsQuery>,
+    auth: Option<Extension<crate::auth::AuthenticatedDid>>,
 ) -> Result<Json<serde_json::Value>> {
-    let limit = q.limit.min(200);
-    // Bare `?` so connection-class sqlx failures downcast to `AppError::Db` and
-    // map to 503 `db_unavailable` (not 500 via `.map_err(AppError::Internal)`) (#251).
-    let anchors = state
-        .db
-        .list_arweave_anchors(q.repo.as_deref(), limit)
-        .await?;
+    // The route's `optional_signature` layer is permissive (it admits unsigned
+    // legacy callers) so this in-handler check is the actual gate. A signed
+    // non-reader still passes this check and proceeds to the authz step below.
+    let caller = auth.as_ref().map(|e| e.0 .0.as_str());
+    if caller.is_none() {
+        return Err(AppError::Unauthorized(
+            "authentication required for anchor listing".into(),
+        ));
+    }
+
+    // Clamp before the SQL query. A negative or non-numeric limit must not
+    // reach Postgres as `LIMIT -1` (which it rejects as a db error / 500) — the
+    // contract is the same default ceiling as the listing page itself.
+    let limit = q.limit.clamp(0, 200);
+
+    let anchors = if let Some(repo_slug) = q.repo.as_deref() {
+        // Parse the user-supplied "owner/name" through the same slug validator
+        // the sync path uses, so a malformed query is a 400, not a 500.
+        let (owner, name) = match crate::git::repo_store::validate_repo_slug(repo_slug) {
+            Ok(parts) => parts,
+            Err(e) => return Err(AppError::BadRequest(format!("invalid ?repo: {e}"))),
+        };
+        // Read gate. Returns `RepoNotFound` (→ 404) indistinguishably for
+        // missing repos, quarantined mirrors, and signed non-readers.
+        let (record, _rules) =
+            crate::api::authorize_repo_read(&state, owner, name, caller, "/").await?;
+        // Build the SQL filter from the canonical stored slug, NOT from the
+        // user-supplied `?repo=` string. Anchor rows are written as
+        // `{normalize_owner_key(owner_did)}/{name}` (the short form), so a
+        // request that passes authz with the full `did:key:…/name` form would
+        // match zero rows and return a false empty page. The gate already
+        // verified the repo is readable; `record.owner_did` is the canonical
+        // identity and `record.name` is the stored name.
+        let stored_slug = format!("{}/{}", normalize_owner_key(&record.owner_did), record.name);
+        state
+            .db
+            .list_arweave_anchors(Some(&stored_slug), limit)
+            .await?
+    } else {
+        // No scope → no repo-read decision. Auth-only.
+        state.db.list_arweave_anchors(None, limit).await?
+    };
 
     Ok(Json(serde_json::json!({
         "anchors": anchors,
@@ -60,6 +106,7 @@ mod closed_pool_tests {
             .oneshot(
                 Request::builder()
                     .uri("/api/v1/arweave/anchors")
+                    .extension(crate::auth::AuthenticatedDid("did:key:test".into()))
                     .body(axum::body::Body::empty())
                     .unwrap(),
             )

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

@@ -2133,7 +2133,15 @@ async fn gate_and_serve(
 /// Returns all CIDs that have been pinned to the local IPFS node from git
 /// objects received via push. Each entry includes the git SHA-256 hex, the
 /// CIDv1 string, and the timestamp when it was pinned.
-pub async fn list_pins(State(state): State<AppState>) -> Result<Json<serde_json::Value>> {
+pub async fn list_pins(
+    State(state): State<AppState>,
+    auth: Option<Extension<crate::auth::AuthenticatedDid>>,
+) -> Result<Json<serde_json::Value>> {
+    if auth.is_none() {
+        return Err(crate::error::AppError::Unauthorized(
+            "authentication required for pin listing".into(),
+        ));
+    }
     // Bare `?` so connection-class sqlx failures downcast to `AppError::Db` and
     // map to 503 `db_unavailable` (not 500 via `.map_err(AppError::Internal)`) (#251).
     let pins = state.db.list_pinned_cids().await?;
@@ -2426,6 +2434,7 @@ mod closed_pool_tests {
             .oneshot(
                 Request::builder()
                     .uri("/api/v1/ipfs/pins")
+                    .extension(crate::auth::AuthenticatedDid("did:key:test".into()))
                     .body(axum::body::Body::empty())
                     .unwrap(),
             )

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

@@ -80,15 +80,20 @@ pub(crate) fn did_matches(a: &str, b: &str) -> bool {
     !ka.contains(':') && !kb.contains(':') && ka == kb
 }
 
+/// Client-facing denial shared by every repo-owner gate copy. `require_repo_owner`
+/// renders it as `AppError::Forbidden`; a handler that emits its own
+/// `(StatusCode, Json)` envelope instead of `AppError` (e.g.
+/// `api::tasks::create_task`) must use this constant too, so two copies of one
+/// gate cannot drift in observable output.
+pub(crate) const REPO_OWNER_ONLY_MESSAGE: &str = "only the repo owner can perform this action";
+
 /// 403 unless `caller` is the repo owner. Uses [`did_matches`] so the owner check
 /// and the author check (close policy) share one normalization.
 pub(crate) fn require_repo_owner(record: &RepoRecord, caller: &str) -> Result<()> {
     if did_matches(caller, &record.owner_did) {
         Ok(())
     } else {
-        Err(AppError::Forbidden(
-            "only the repo owner can perform this action".into(),
-        ))
+        Err(AppError::Forbidden(REPO_OWNER_ONLY_MESSAGE.into()))
     }
 }
 
@@ -242,6 +247,11 @@ mod authz_guard {
             (events, "list_repo_events", "authorize_repo_read("),
             // Bucket C — signer-self: the acting DID is matched/bound to auth.0
             (tasks, "create_task", "did_matches("),
+            // #496: create_task is signer-self AND owner-when-repo-scoped — a
+            // repo_id naming a hosted repo admits tasks only from its owner.
+            // Both halves are pinned: the did_matches row guards the signer
+            // binding, this one the repo-ownership gate.
+            (tasks, "create_task", "require_repo_owner("),
             (tasks, "claim_task", "did_matches("),
             (tasks, "complete_task", "did_matches("),
             (tasks, "fail_task", "did_matches("),
@@ -341,7 +351,11 @@ mod authz_guard {
             (
                 "create_task",
                 "createTask",
-                &["require_signer(ctx)?", "did_matches("],
+                &[
+                    "require_signer(ctx)?",
+                    "did_matches(",
+                    "require_repo_owner(",
+                ],
             ),
             (
                 "claim_task",

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

@@ -30,6 +30,33 @@ fn forbidden(msg: &str) -> (StatusCode, Json<Value>) {
     )
 }
 
+/// This module's error shape with an opaque body: the real error is logged
+/// server-side and never serialized (#250), matching the envelope
+/// `AppError::Db` renders. Also mirrors its status split: pool-class failures
+/// are a retryable 503 `db_unavailable`, everything else the opaque 500
+/// `db_error`, so a transient outage is not reported as permanent.
+fn db_error(e: anyhow::Error) -> (StatusCode, Json<Value>) {
+    tracing::error!(error = %format!("{e:#}"), "task database error");
+    if let Some(sql) = e.chain().find_map(|c| c.downcast_ref::<sqlx::Error>()) {
+        if crate::error::db_unavailable(sql) {
+            return (
+                StatusCode::SERVICE_UNAVAILABLE,
+                Json(json!({
+                    "error": crate::error::DB_UNAVAILABLE_CODE,
+                    "message": crate::error::DB_UNAVAILABLE_MESSAGE,
+                })),
+            );
+        }
+    }
+    (
+        StatusCode::INTERNAL_SERVER_ERROR,
+        Json(json!({
+            "error": "db_error",
+            "message": crate::error::DB_ERROR_MESSAGE,
+        })),
+    )
+}
+
 // ── Request / response types ──────────────────────────────────────────────────
 
 #[derive(Deserialize)]
@@ -101,6 +128,37 @@ pub async fn create_task(
     if !crate::api::did_matches(&auth.0, &body.delegator_did) {
         return Err(forbidden("delegator_did must be the authenticated signer"));
     }
+    // #496: a caller-supplied repo_id must name a repo the caller owns. Without
+    // this, any signed caller can plant a task — payload and UCAN included —
+    // under a foreign repo id, where the named assignee can claim it. Resolve
+    // the id against hosted repos: a hosted, non-quarantined repo admits tasks
+    // only from its owner (403 otherwise). An id naming no hosted repo
+    // (unknown, or quarantined — which is hidden as if it did not exist, so
+    // 403ing it would confirm a real id) is stored verbatim as an opaque
+    // label: this is a write gate and does not bind an unresolved id to a
+    // repo. Task reads are caller-independent on this base (the list endpoints
+    // are not repo-scoped), before and after this change. The 403-vs-201 split
+    // between a hosted repo id and an unknown one is itself a one-bit oracle —
+    // a stranger holding a private repo's id learns it is hosted here — but
+    // denying foreign repos requires drawing exactly that line, and repo ids
+    // already surface via the task list on this base. Repo-less tasks are
+    // unaffected.
+    if let Some(repo_id) = body.repo_id.as_deref() {
+        let record = state.db.get_repo_by_id(repo_id).await.map_err(db_error)?;
+        if let Some(record) = record {
+            let quarantined = state
+                .db
+                .is_repo_quarantined(&record.id)
+                .await
+                .map_err(db_error)?;
+            if !quarantined && crate::api::require_repo_owner(&record, &auth.0).is_err() {
+                // Same denial text the GraphQL copy relays from
+                // `require_repo_owner`, so the two copies of this gate do not
+                // disagree in observable output.
+                return Err(forbidden(crate::api::REPO_OWNER_ONLY_MESSAGE));
+            }
+        }
+    }
     let now = Utc::now().to_rfc3339();
     let task = AgentTask {
         id: Uuid::new_v4().to_string(),
@@ -117,12 +175,7 @@ pub async fn create_task(
         updated_at: now,
         deadline: body.deadline,
     };
-    state.db.create_task(&task).await.map_err(|e| {
-        (
-            StatusCode::INTERNAL_SERVER_ERROR,
-            Json(json!({ "error": e.to_string() })),
-        )
-    })?;
+    state.db.create_task(&task).await.map_err(db_error)?;
     Ok((StatusCode::CREATED, Json(task_to_json(&task))))
 }
 
@@ -297,3 +350,36 @@ pub async fn fail_task(
     });
     Ok(Json(task_to_json(&task)))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// The gate's DB helper must mirror `AppError::Db`: a pool-level failure is
+    /// the retryable 503 `db_unavailable`, a query fault the opaque 500.
+    #[test]
+    fn db_error_matches_app_error_status_split() {
+        let unavailable = db_error(anyhow::Error::from(sqlx::Error::PoolTimedOut));
+        assert_eq!(unavailable.0, StatusCode::SERVICE_UNAVAILABLE);
+        assert_eq!(unavailable.1 .0["error"], crate::error::DB_UNAVAILABLE_CODE);
+        assert_eq!(
+            unavailable.1 .0["message"],
+            crate::error::DB_UNAVAILABLE_MESSAGE
+        );
+
+        let query = db_error(anyhow::Error::from(sqlx::Error::Protocol(
+            "column \"machine_id\" does not exist".into(),
+        )));
+        assert_eq!(query.0, StatusCode::INTERNAL_SERVER_ERROR);
+        assert_eq!(query.1 .0["error"], "db_error");
+        assert_eq!(query.1 .0["message"], crate::error::DB_ERROR_MESSAGE);
+        assert!(!query.1 .0.to_string().contains("machine_id"));
+    }
+
+    /// `.context()` layers must not hide the sqlx error from the status split.
+    #[test]
+    fn db_error_sees_sqlx_through_context() {
+        let wrapped = anyhow::Error::from(sqlx::Error::PoolClosed).context("repo lookup failed");
+        assert_eq!(db_error(wrapped).0, StatusCode::SERVICE_UNAVAILABLE);
+    }
+}

crates/gitlawb-node/src/error.rs

@@ -101,7 +101,7 @@ pub const DB_ERROR_MESSAGE: &str = "a database error occurred";
 
 /// Connection-level sqlx failures that mean the database is unreachable right
 /// now (retryable, 503), as opposed to server-reported query errors.
-fn db_unavailable(e: &sqlx::Error) -> bool {
+pub(crate) fn db_unavailable(e: &sqlx::Error) -> bool {
     matches!(
         e,
         sqlx::Error::PoolTimedOut

crates/gitlawb-node/src/graphql/mutation.rs

@@ -36,6 +36,33 @@ impl MutationRoot {
         }
         let delegator_did = caller.to_string();
         let db = ctx.data_unchecked::<Arc<Db>>();
+        // #496: same write gate as the REST handler — a repo_id naming a
+        // hosted, non-quarantined repo admits tasks only from its owner, so a
+        // stranger cannot file under a foreign repo id. An id naming no hosted
+        // repo (unknown, or quarantined — hidden as if it did not exist) is
+        // stored verbatim as an opaque label; the gate does not bind an
+        // unresolved id to a repo. Task reads are caller-independent on this
+        // base (the tasks resolver is not repo-scoped), before and after this
+        // change. As in tasks.rs, the success-vs-denial split between a hosted
+        // repo id and an unknown one is itself a one-bit oracle — a stranger
+        // holding a private repo's id learns it is hosted here — but denying
+        // foreign repos requires drawing exactly that line.
+        if let Some(repo_id) = input.repo_id.as_deref() {
+            let record = db
+                .get_repo_by_id(repo_id)
+                .await
+                .map_err(crate::graphql::graphql_db_err)?;
+            if let Some(record) = record {
+                let quarantined = db
+                    .is_repo_quarantined(&record.id)
+                    .await
+                    .map_err(crate::graphql::graphql_db_err)?;
+                if !quarantined {
+                    crate::api::require_repo_owner(&record, caller)
+                        .map_err(crate::graphql::graphql_app_err)?;
+                }
+            }
+        }
         let now = Utc::now().to_rfc3339();
         let task = AgentTask {
             id: Uuid::new_v4().to_string(),
@@ -186,6 +213,22 @@ mod tests {
             .join("; ")
     }
 
+    /// The `id` scalar returned by a successful `createTask` mutation.
+    fn created_id(resp: &Response) -> String {
+        assert!(resp.errors.is_empty(), "graphql errors: {:?}", resp.errors);
+        let async_graphql::Value::Object(obj) = &resp.data else {
+            panic!("data not an object: {:?}", resp.data);
+        };
+        let async_graphql::Value::Object(task) = obj.get("createTask").expect("createTask key")
+        else {
+            panic!("createTask not an object");
+        };
+        let async_graphql::Value::String(id) = task.get("id").expect("id key") else {
+            panic!("id not a string");
+        };
+        id.clone()
+    }
+
     /// N2: every GraphQL mutation requires a verified signer and binds the acting
     /// DID to it. Unsigned is rejected; a signer other than the claimed actor is
     /// rejected; the matching signer passes the auth gate.
@@ -353,4 +396,181 @@ mod tests {
             errors(&resp)
         );
     }
+
+    /// #496 (GraphQL): createTask applies the same repo-ownership gate as the
+    /// REST handler — a stranger naming a hosted repo is rejected without
+    /// persisting anything, while the owner files. The quarantined and
+    /// unknown-id arms are pinned too, including that each accepted write
+    /// stores the supplied id verbatim, so this copy of the gate cannot regress
+    /// silently.
+    #[sqlx::test]
+    async fn create_task_rejects_foreign_repo_id(pool: PgPool) {
+        let state = crate::test_support::test_state(pool).await;
+        let owner = "did:key:zGQLTASKOWNERAAAAAAAAAAAAAAAAAAAAAAAAAA";
+        let stranger = "did:key:zGQLTASKSTRANGERBBBBBBBBBBBBBBBBBBBBBB";
+        let now = chrono::Utc::now();
+        let repo = crate::db::RepoRecord {
+            id: uuid::Uuid::new_v4().to_string(),
+            name: "gql-task-gate-repo".to_string(),
+            owner_did: owner.to_string(),
+            description: None,
+            is_public: true,
+            default_branch: "main".to_string(),
+            created_at: now,
+            updated_at: now,
+            disk_path: "/tmp/gql-task-gate-repo".to_string(),
+            forked_from: None,
+            machine_id: None,
+        };
+        state.db.create_repo(&repo).await.expect("seed repo");
+        let schema = state.graphql_schema.as_ref();
+
+        let q = |actor: &str| {
+            format!(
+                r#"mutation {{ createTask(delegatorDid: "{actor}", input: {{ kind: "build", capability: "repo:write", repoId: "{}" }}) {{ id }} }}"#,
+                repo.id
+            )
+        };
+
+        // Stranger signs as themselves (so the signer binding passes) but
+        // names the victim's repo → rejected by the ownership gate, leaking
+        // nothing about the repo.
+        let resp = schema
+            .execute(Request::new(q(stranger)).data(AuthenticatedDid(stranger.into())))
+            .await;
+        let errs = errors(&resp);
+        assert!(
+            errs.contains(crate::api::REPO_OWNER_ONLY_MESSAGE),
+            "a non-owner must not file under a foreign repo id (shared denial text): {errs}"
+        );
+
+        // The denial must not persist: no task row may carry the victim's
+        // repo id afterwards.
+        let stored = state
+            .db
+            .list_tasks(None, None, 50)
+            .await
+            .expect("list tasks");
+        assert!(
+            stored
+                .iter()
+                .all(|t| t.repo_id.as_deref() != Some(repo.id.as_str())),
+            "a denied write must not leave a task under the foreign repo id"
+        );
+
+        // The owner files under their own repo id.
+        let resp = schema
+            .execute(Request::new(q(owner)).data(AuthenticatedDid(owner.into())))
+            .await;
+        assert!(
+            errors(&resp).is_empty(),
+            "the owner should file against their repo: {}",
+            errors(&resp)
+        );
+
+        // Unknown repo ids stay accepted as opaque labels.
+        let q_unknown = format!(
+            r#"mutation {{ createTask(delegatorDid: "{stranger}", input: {{ kind: "build", capability: "repo:write", repoId: "no-such-repo-id" }}) {{ id }} }}"#
+        );
+        let resp = schema
+            .execute(Request::new(q_unknown).data(AuthenticatedDid(stranger.into())))
+            .await;
+        assert!(
+            errors(&resp).is_empty(),
+            "an id naming no hosted repo stays an opaque label: {}",
+            errors(&resp)
+        );
+        // Read back the row: the unresolved id must be stored verbatim, not
+        // dropped or rewritten to a repo it never named.
+        let stored = state
+            .db
+            .get_task(&created_id(&resp))
+            .await
+            .expect("get task")
+            .expect("task must be stored");
+        assert_eq!(
+            stored.repo_id.as_deref(),
+            Some("no-such-repo-id"),
+            "an unresolved id must be stored verbatim as a label"
+        );
+
+        // A quarantined repo id is treated as unknown (accepted, never a 403
+        // that would confirm the id names a real repo). This pins the
+        // `!quarantined` arm on this copy of the gate.
+        state
+            .db
+            .set_repo_quarantine(&repo.id, true)
+            .await
+            .expect("quarantine");
+        let resp = schema
+            .execute(Request::new(q(stranger)).data(AuthenticatedDid(stranger.into())))
+            .await;
+        assert!(
+            errors(&resp).is_empty(),
+            "a quarantined repo id must not 403 (existence oracle): {}",
+            errors(&resp)
+        );
+        // Read back the row: the quarantined id is stored verbatim as a label
+        // too, so the accepted write is not silently dropped.
+        let stored = state
+            .db
+            .get_task(&created_id(&resp))
+            .await
+            .expect("get task")
+            .expect("task must be stored");
+        assert_eq!(
+            stored.repo_id.as_deref(),
+            Some(repo.id.as_str()),
+            "a quarantined id must be stored verbatim as a label"
+        );
+    }
+
+    /// #496 (GraphQL): a repo-lookup fault in createTask's gate must stay
+    /// opaque. Dropping a column `get_repo_by_id` selects faults that lookup;
+    /// the resolver must return the fixed DB message and leak no schema text
+    /// (#250), as `create_task_db_error_message_is_opaque` pins for the
+    /// agent_tasks fault.
+    #[sqlx::test]
+    async fn create_task_gate_repo_lookup_fault_is_opaque(pool: PgPool) {
+        let state = crate::test_support::test_state(pool.clone()).await;
+        let owner = "did:key:zGQLTASKFAULTOWNERAAAAAAAAAAAAAAAAAAAAAAAA";
+        let now = chrono::Utc::now();
+        let repo = crate::db::RepoRecord {
+            id: uuid::Uuid::new_v4().to_string(),
+            name: "gql-task-gate-fault-repo".to_string(),
+            owner_did: owner.to_string(),
+            description: None,
+            is_public: true,
+            default_branch: "main".to_string(),
+            created_at: now,
+            updated_at: now,
+            disk_path: "/tmp/gql-task-gate-fault-repo".to_string(),
+            forked_from: None,
+            machine_id: None,
+        };
+        state.db.create_repo(&repo).await.expect("seed repo");
+
+        sqlx::query("ALTER TABLE repos DROP COLUMN machine_id")
+            .execute(&pool)
+            .await
+            .expect("drop machine_id");
+
+        let q = format!(
+            r#"mutation {{ createTask(delegatorDid: "{owner}", input: {{ kind: "build", capability: "repo:write", repoId: "{}" }}) {{ id }} }}"#,
+            repo.id
+        );
+        let resp = state
+            .graphql_schema
+            .execute(Request::new(&q).data(AuthenticatedDid(owner.into())))
+            .await;
+        let errs = errors(&resp);
+        assert!(
+            errs.contains(crate::graphql::GRAPHQL_DB_ERROR_MESSAGE),
+            "gate sqlx fault must be opaque: {errs}"
+        );
+        assert!(
+            !errs.contains("machine_id") && !errs.contains("column"),
+            "schema text leaked through the gate: {errs}"
+        );
+    }
 }

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

@@ -7,11 +7,11 @@
 //! The node's PeerId comes from an Ed25519 keypair loaded from a persistent
 //! key file, so the PeerId is stable across restarts.
 
-use std::collections::{hash_map::DefaultHasher, HashMap};
+use std::collections::{hash_map::DefaultHasher, HashMap, HashSet, VecDeque};
 use std::hash::{Hash, Hasher};
 use std::path::{Component, Path, PathBuf};
 use std::sync::Arc;
-use std::time::Duration;
+use std::time::{Duration, Instant};
 
 use anyhow::{Context, Result};
 use chrono::{DateTime, Utc};
@@ -315,6 +315,17 @@ fn is_zero(v: &u32) -> bool {
     *v == 0
 }
 
+// Identify is an untrusted address source. Keep enough entries for normal
+// multi-homed peers while bounding both one peer and the whole routing table.
+const IDENTIFY_ADDRESS_LIMIT: usize = 8;
+// Bound canonicalization and sorting too, including rejected/duplicate entries.
+const IDENTIFY_REPORT_ADDRESS_LIMIT: usize = 64;
+const IDENTIFY_NEW_ADDRESS_LIMIT: usize = 8;
+const IDENTIFY_GLOBAL_ADDRESS_LIMIT: usize = 1024;
+const IDENTIFY_ADDRESS_WINDOW: Duration = Duration::from_secs(60);
+const IDENTIFY_ADDRESS_TTL: Duration = Duration::from_secs(30 * 60);
+const IDENTIFY_ADDRESS_CLEANUP_INTERVAL: Duration = Duration::from_secs(60);
+
 /// A ref-update event published to Gossipsub when a push lands.
 ///
 /// The signing bytes are this struct serialized with `sig` set to None (see
@@ -1631,6 +1642,200 @@ fn did_to_kad_key(did: &str) -> kad::RecordKey {
     kad::RecordKey::new(&format!("/gitlawb/did/{did}").as_bytes())
 }
 
+/// Returns true when `address` was explicitly configured for `peer` through
+/// AddKnownPeer. Explicit addresses are retained in Kademlia even when an
+/// overlapping Identify lease expires or is evicted.
+fn is_explicit_address(
+    explicit_addresses: &HashMap<PeerId, HashSet<Multiaddr>>,
+    peer: &PeerId,
+    address: &Multiaddr,
+) -> bool {
+    explicit_addresses
+        .get(peer)
+        .is_some_and(|addresses| addresses.contains(address))
+}
+
+#[derive(Debug)]
+struct IdentifyAddress {
+    address: Multiaddr,
+    expires_at: Instant,
+}
+
+#[derive(Debug)]
+struct IdentifyPeerAddresses {
+    addresses: VecDeque<IdentifyAddress>,
+    window_started: Instant,
+    new_addresses: usize,
+}
+
+#[derive(Debug, Default, PartialEq, Eq)]
+struct IdentifyAddressChanges {
+    added: Vec<Multiaddr>,
+    refreshed: Vec<Multiaddr>,
+    removed: Vec<(PeerId, Multiaddr)>,
+}
+
+#[derive(Debug, Default)]
+struct IdentifyAddressBook {
+    peers: HashMap<PeerId, IdentifyPeerAddresses>,
+    address_count: usize,
+}
+
+impl IdentifyAddressBook {
+    fn update(
+        &mut self,
+        peer_id: PeerId,
+        now: Instant,
+        addresses: &[Multiaddr],
+    ) -> IdentifyAddressChanges {
+        let mut changes = IdentifyAddressChanges::default();
+        if self.peers.contains_key(&peer_id) {
+            changes.removed.extend(self.expire_peer(peer_id, now));
+        }
+        let reported = addresses
+            .iter()
+            .take(IDENTIFY_REPORT_ADDRESS_LIMIT)
+            .filter_map(|address| address.clone().with_p2p(peer_id).ok())
+            .collect::<HashSet<_>>();
+        if reported.is_empty() && !self.peers.contains_key(&peer_id) {
+            return changes;
+        }
+        let state = self
+            .peers
+            .entry(peer_id)
+            .or_insert_with(|| IdentifyPeerAddresses {
+                addresses: VecDeque::new(),
+                window_started: now,
+                new_addresses: 0,
+            });
+
+        if now.saturating_duration_since(state.window_started) >= IDENTIFY_ADDRESS_WINDOW {
+            state.window_started = now;
+            state.new_addresses = 0;
+        }
+
+        // Refresh every retained address in the bounded report before admission.
+        // Refreshed addresses are reported so the Identify handler can re-admit
+        // them to Kademlia. Kademlia may have dropped a retained address
+        // after a failed dial or never kept it for a full k-bucket, and a
+        // TTL refresh alone would never re-add it.
+        for existing in &mut state.addresses {
+            if reported.contains(&existing.address) {
+                existing.expires_at = now + IDENTIFY_ADDRESS_TTL;
+                changes.refreshed.push(existing.address.clone());
+            }
+        }
+
+        let mut new_addresses: Vec<Multiaddr> = reported
+            .iter()
+            .filter(|address| {
+                !state
+                    .addresses
+                    .iter()
+                    .any(|existing| &existing.address == *address)
+            })
+            .cloned()
+            .collect();
+        // HashSet iteration is deliberately unordered. Sort new candidates so
+        // an oversized Identify report has deterministic admission behavior.
+        new_addresses.sort();
+
+        for address in new_addresses {
+            if state.new_addresses >= IDENTIFY_NEW_ADDRESS_LIMIT {
+                break;
+            }
+
+            let evicted = if state.addresses.len() < IDENTIFY_ADDRESS_LIMIT {
+                None
+            } else {
+                state
+                    .addresses
+                    .iter()
+                    .position(|existing| !reported.contains(&existing.address))
+                    .and_then(|index| state.addresses.remove(index))
+            };
+            if let Some(evicted) = evicted {
+                self.address_count -= 1;
+                changes.removed.push((peer_id, evicted.address));
+            } else if state.addresses.len() >= IDENTIFY_ADDRESS_LIMIT {
+                // Every retained address was refreshed in this report. Keep
+                // that stable subset and drop surplus new candidates.
+                break;
+            }
+
+            if self.address_count >= IDENTIFY_GLOBAL_ADDRESS_LIMIT {
+                // Reject the admission instead of evicting another peer's
+                // entry: cross-peer eviction lets a flood of new identities
+                // push honest peers' addresses out of the book and, when the
+                // evicted address is their last one, out of the Kademlia
+                // routing table. Per-peer eviction above still applies to the
+                // reporting peer's own addresses.
+                break;
+            }
+
+            state.new_addresses += 1;
+            state.addresses.push_back(IdentifyAddress {
+                address: address.clone(),
+                expires_at: now + IDENTIFY_ADDRESS_TTL,
+            });
+            self.address_count += 1;
+            changes.added.push(address);
+        }
+
+        // Do not keep a row when every candidate was rejected: a zero-address
+        // row is invisible to address_count and would otherwise linger until
+        // the expiry tick. This matches expire_peer, which drops empty rows.
+        if state.addresses.is_empty() {
+            self.peers.remove(&peer_id);
+        }
+
+        changes
+    }
+
+    fn expire(&mut self, now: Instant) -> Vec<(PeerId, Multiaddr)> {
+        let mut removed = Vec::new();
+        let peers = self.peers.keys().copied().collect::<Vec<_>>();
+        for peer_id in peers {
+            removed.extend(self.expire_peer(peer_id, now));
+        }
+        removed
+    }
+
+    fn expire_peer(&mut self, peer_id: PeerId, now: Instant) -> Vec<(PeerId, Multiaddr)> {
+        let expired = self
+            .peers
+            .get_mut(&peer_id)
+            .map(|state| Self::expire_state(state, now))
+            .unwrap_or_default();
+        self.address_count -= expired.len();
+        if self
+            .peers
+            .get(&peer_id)
+            .is_some_and(|state| state.addresses.is_empty())
+        {
+            self.peers.remove(&peer_id);
+        }
+        expired
+            .into_iter()
+            .map(|address| (peer_id, address))
+            .collect()
+    }
+
+    fn expire_state(state: &mut IdentifyPeerAddresses, now: Instant) -> Vec<Multiaddr> {
+        let mut expired = Vec::new();
+        let mut retained = VecDeque::with_capacity(state.addresses.len());
+        while let Some(address) = state.addresses.pop_front() {
+            if address.expires_at <= now {
+                expired.push(address.address);
+            } else {
+                retained.push_back(address);
+            }
+        }
+        state.addresses = retained;
+        expired
+    }
+}
+
 /// Combined libp2p behaviour.
 #[derive(NetworkBehaviour)]
 #[behaviour(prelude = "libp2p_swarm::derive_prelude")]
@@ -3960,6 +4165,14 @@ pub async fn start(
     // Track in-flight GetRecord queries → reply channels
     let mut pending_get_did: HashMap<kad::QueryId, oneshot::Sender<Option<DidRecord>>> =
         HashMap::new();
+    // Keep Identify-derived entries across disconnects so a bootstrap address
+    // learned through Identify remains usable for redial. The TTL cleanup below
+    // removes entries that are no longer refreshed without touching explicit
+    // AddKnownPeer addresses.
+    let mut identify_addresses = IdentifyAddressBook::default();
+    let mut explicit_addresses: HashMap<PeerId, HashSet<Multiaddr>> = HashMap::new();
+    let mut identify_cleanup = tokio::time::interval(IDENTIFY_ADDRESS_CLEANUP_INTERVAL);
+    identify_cleanup.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip);
 
     // Start the event loop as a background task
     tokio::spawn(async move {
@@ -3979,6 +4192,13 @@ pub async fn start(
                 _ = ingest_sweep.tick() => {
                     run_ingest_sweep(&ingest_limiters, &replay_guard, ingest_now()).await;
                 }
+                _ = identify_cleanup.tick() => {
+                    for (peer_id, address) in identify_addresses.expire(Instant::now()) {
+                        if !is_explicit_address(&explicit_addresses, &peer_id, &address) {
+                            swarm.behaviour_mut().kademlia.remove_address(&peer_id, &address);
+                        }
+                    }
+                }
                 // Graceful shutdown: exit the swarm loop when the
                 // process-wide signal flips. This drops the Swarm
                 // which closes all libp2p connections cleanly.
@@ -4113,7 +4333,21 @@ pub async fn start(
                             identify::Event::Received { peer_id, info, .. }
                         )) => {
                             debug!(peer = %peer_id, "identify received");
-                            for addr in info.listen_addrs {
+                            let changes = identify_addresses.update(
+                                peer_id,
+                                Instant::now(),
+                                &info.listen_addrs,
+                            );
+                            for (removed_peer, addr) in changes.removed {
+                                if !is_explicit_address(&explicit_addresses, &removed_peer, &addr)
+                                {
+                                    swarm
+                                        .behaviour_mut()
+                                        .kademlia
+                                        .remove_address(&removed_peer, &addr);
+                                }
+                            }
+                            for addr in changes.refreshed.into_iter().chain(changes.added) {
                                 swarm.behaviour_mut().kademlia.add_address(&peer_id, addr);
                             }
                         }
@@ -4150,6 +4384,14 @@ pub async fn start(
                             }
                         }
                         P2pCommand::AddKnownPeer { peer_id, addr } => {
+                            let Ok(addr) = addr.clone().with_p2p(peer_id) else {
+                                warn!(%peer_id, %addr, "dropping known-peer address with a foreign peer suffix");
+                                continue;
+                            };
+                            explicit_addresses
+                                .entry(peer_id)
+                                .or_default()
+                                .insert(addr.clone());
                             swarm.behaviour_mut().kademlia.add_address(&peer_id, addr);
                         }
                         P2pCommand::Dial(addr) => {
@@ -4201,6 +4443,453 @@ pub async fn start(
 mod tests {
     use super::*;
 
+    fn identify_address(peer_id: PeerId, octet: u8, port: u16) -> Multiaddr {
+        format!("/ip4/192.0.2.{octet}/udp/{port}/quic-v1")
+            .parse::<Multiaddr>()
+            .unwrap()
+            .with_p2p(peer_id)
+            .unwrap()
+    }
+
+    fn identify_address_without_peer(octet: u8, port: u16) -> Multiaddr {
+        format!("/ip4/192.0.2.{octet}/udp/{port}/quic-v1")
+            .parse::<Multiaddr>()
+            .unwrap()
+    }
+
+    #[test]
+    fn identify_empty_reports_do_not_retain_peer_entries() {
+        let now = Instant::now();
+        let mut book = IdentifyAddressBook::default();
+        let foreign = identify_address(PeerId::random(), 1, 10_000);
+        for _ in 0..IDENTIFY_GLOBAL_ADDRESS_LIMIT + 1 {
+            let peer = PeerId::random();
+            assert_eq!(
+                book.update(peer, now, &[]),
+                IdentifyAddressChanges::default()
+            );
+            assert_eq!(
+                book.update(peer, now, std::slice::from_ref(&foreign)),
+                IdentifyAddressChanges::default()
+            );
+        }
+        assert!(book.peers.is_empty());
+        assert_eq!(book.address_count, 0);
+    }
+
+    #[test]
+    fn identify_fully_rejected_peer_leaves_no_row_at_full_budget() {
+        let now = Instant::now();
+        let mut book = IdentifyAddressBook::default();
+        // Fill the global budget with one-address peers.
+        for index in 0..IDENTIFY_GLOBAL_ADDRESS_LIMIT {
+            let peer = PeerId::random();
+            let address = identify_address(peer, 1, 10_000 + index as u16);
+            book.update(peer, now, std::slice::from_ref(&address));
+        }
+        assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT);
+        // A fresh peer reporting at saturation has every candidate rejected
+        // by the global-budget check. The row created by or_insert_with must
+        // be dropped: a zero-address row is invisible to address_count and
+        // would otherwise linger until the expiry tick.
+        let fresh = PeerId::random();
+        let fresh_address = identify_address(fresh, 2, 30_000);
+        let changes = book.update(fresh, now, std::slice::from_ref(&fresh_address));
+        assert!(changes.added.is_empty());
+        assert!(!book.peers.contains_key(&fresh));
+        assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT);
+    }
+
+    #[test]
+    fn identify_update_reports_expired_addresses_as_removed() {
+        let peer = PeerId::random();
+        let now = Instant::now();
+        let address = identify_address(peer, 1, 10_000);
+        let mut book = IdentifyAddressBook::default();
+        book.update(peer, now, std::slice::from_ref(&address));
+        // An update past the TTL expires the stale address through
+        // expire_peer and reports it in changes.removed so the Identify
+        // handler can drop it from Kademlia.
+        let changes = book.update(
+            peer,
+            now + IDENTIFY_ADDRESS_TTL + Duration::from_secs(1),
+            std::slice::from_ref(&address),
+        );
+        assert_eq!(changes.removed, vec![(peer, address.clone())]);
+        assert_eq!(changes.added, vec![address]);
+    }
+
+    #[test]
+    fn identify_expire_drops_rows_emptied_by_expiry() {
+        let peer = PeerId::random();
+        let now = Instant::now();
+        let address = identify_address(peer, 1, 10_000);
+        let mut book = IdentifyAddressBook::default();
+        book.update(peer, now, std::slice::from_ref(&address));
+        assert!(book.peers.contains_key(&peer));
+        // Once every address in the row has expired, expire() must drop the
+        // row itself, matching the empty-row drop in update().
+        let removed = book.expire(now + IDENTIFY_ADDRESS_TTL + Duration::from_secs(1));
+        assert_eq!(removed, vec![(peer, address)]);
+        assert!(!book.peers.contains_key(&peer));
+        assert_eq!(book.address_count, 0);
+    }
+
+    #[test]
+    fn identify_report_processing_stops_at_the_input_limit() {
+        let peer = PeerId::random();
+        let now = Instant::now();
+        let foreign = identify_address(PeerId::random(), 1, 10_000);
+        let accepted = identify_address(peer, 2, 10_001);
+        let ignored = identify_address(peer, 3, 10_002);
+        let mut report = vec![foreign; IDENTIFY_REPORT_ADDRESS_LIMIT - 1];
+        report.push(accepted.clone());
+        report.push(ignored);
+        let mut book = IdentifyAddressBook::default();
+        let changes = book.update(peer, now, &report);
+        assert_eq!(changes.added, vec![accepted]);
+        assert!(changes.removed.is_empty());
+        assert_eq!(book.address_count, 1);
+    }
+
+    #[test]
+    fn identify_addresses_grow_without_eviction_below_the_peer_cap() {
+        let peer = PeerId::random();
+        let now = Instant::now();
+        let first = identify_address(peer, 1, 10_000);
+        let second = identify_address(peer, 2, 10_001);
+        let mut book = IdentifyAddressBook::default();
+        book.update(peer, now, std::slice::from_ref(&first));
+        let changes = book.update(
+            peer,
+            now + IDENTIFY_ADDRESS_WINDOW,
+            std::slice::from_ref(&second),
+        );
+        assert_eq!(changes.added, vec![second]);
+        assert!(changes.removed.is_empty());
+        assert_eq!(book.peers[&peer].addresses.len(), 2);
+        assert_eq!(book.address_count, 2);
+    }
+
+    #[test]
+    fn identify_global_budget_rejects_new_admissions() {
+        let now = Instant::now();
+        let mut book = IdentifyAddressBook::default();
+        let peers = (0..IDENTIFY_GLOBAL_ADDRESS_LIMIT)
+            .map(|index| {
+                let peer = PeerId::random();
+                let address = identify_address(peer, 1, 10_000 + index as u16);
+                book.update(peer, now, std::slice::from_ref(&address));
+                (peer, address)
+            })
+            .collect::<Vec<_>>();
+        let (peer, refreshed) = &peers[0];
+        let new_address = identify_address(*peer, 2, 30_000);
+        let changes = book.update(
+            *peer,
+            now + IDENTIFY_ADDRESS_WINDOW,
+            &[new_address.clone(), refreshed.clone()],
+        );
+        // The budget is full: the new address is rejected rather than
+        // evicting another peer's entry, and the refreshed address keeps its
+        // slot and is reported for re-admission to Kademlia.
+        assert!(changes.added.is_empty());
+        assert!(changes.removed.is_empty());
+        assert_eq!(changes.refreshed, vec![(*refreshed).clone()]);
+        assert_eq!(book.peers.len(), IDENTIFY_GLOBAL_ADDRESS_LIMIT);
+        assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT);
+        assert_eq!(
+            book.peers[peer]
+                .addresses
+                .iter()
+                .map(|a| &a.address)
+                .collect::<Vec<_>>(),
+            vec![refreshed]
+        );
+    }
+
+    #[test]
+    fn identify_per_peer_eviction_applies_at_full_global_budget() {
+        let now = Instant::now();
+        let mut book = IdentifyAddressBook::default();
+        // One filler peer goes in before the victim so the evicted address
+        // is not also the globally-oldest insertion. If saturation evicted
+        // the globally-oldest entry instead of the victim's own oldest,
+        // the sentinel's address would be the one removed.
+        let sentinel = PeerId::random();
+        let sentinel_address = identify_address(sentinel, 200, 19_999);
+        book.update(sentinel, now, std::slice::from_ref(&sentinel_address));
+        // The victim peer holds a full per-peer row while the rest of the
+        // global budget is filled by one-address peers.
+        let victim = PeerId::random();
+        let victim_addresses: Vec<Multiaddr> = (0..IDENTIFY_ADDRESS_LIMIT)
+            .map(|index| identify_address(victim, index as u8 + 1, 10_000 + index as u16))
+            .collect();
+        book.update(victim, now, &victim_addresses);
+        for index in 0..IDENTIFY_GLOBAL_ADDRESS_LIMIT - IDENTIFY_ADDRESS_LIMIT - 1 {
+            let peer = PeerId::random();
+            let address = identify_address(peer, 200, 20_000 + index as u16);
+            book.update(peer, now, &[address]);
+        }
+        assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT);
+
+        // The victim reports a ninth address after the rate window rolls over.
+        // Per-peer eviction must still apply at global saturation: the oldest
+        // non-reported address is evicted to make room, and the global count
+        // is unchanged. This ordering only holds because eviction runs before
+        // the global-budget check; hoisting that check above the eviction
+        // block makes this test fail.
+        let ninth = identify_address(victim, 100, 30_000);
+        let changes = book.update(
+            victim,
+            now + IDENTIFY_ADDRESS_WINDOW,
+            std::slice::from_ref(&ninth),
+        );
+        assert_eq!(changes.added, vec![ninth]);
+        assert_eq!(changes.removed, vec![(victim, victim_addresses[0].clone())]);
+        assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT);
+        assert_eq!(book.peers[&victim].addresses.len(), IDENTIFY_ADDRESS_LIMIT);
+        // The sentinel was inserted first; per-peer eviction must leave it
+        // alone. A global-oldest eviction policy would have removed it.
+        assert_eq!(
+            book.peers[&sentinel]
+                .addresses
+                .iter()
+                .map(|a| &a.address)
+                .collect::<Vec<_>>(),
+            vec![&sentinel_address]
+        );
+    }
+
+    #[test]
+    fn explicit_address_helper_detects_retained_addresses() {
+        let peer = PeerId::random();
+        let other = PeerId::random();
+        let address = identify_address(peer, 1, 10_000);
+        let mut explicit: HashMap<PeerId, HashSet<Multiaddr>> = HashMap::new();
+        // Unknown peer: nothing is explicit.
+        assert!(!is_explicit_address(&explicit, &peer, &address));
+        explicit.entry(peer).or_default().insert(address.clone());
+        assert!(is_explicit_address(&explicit, &peer, &address));
+        // The same address under a different peer is not retained.
+        assert!(!is_explicit_address(&explicit, &other, &address));
+        // A different address for the same peer is not retained.
+        assert!(!is_explicit_address(
+            &explicit,
+            &peer,
+            &identify_address(peer, 2, 10_001)
+        ));
+    }
+
+    #[test]
+    fn identify_addresses_are_capped_with_fifo_eviction() {
+        let peer_id = PeerId::random();
+        let now = Instant::now();
+        let initial = (0..IDENTIFY_ADDRESS_LIMIT)
+            .map(|index| identify_address(peer_id, index as u8 + 1, 10_000 + index as u16))
+            .collect::<Vec<_>>();
+        let replacement = identify_address(peer_id, 200, 20_000);
+        let mut book = IdentifyAddressBook::default();
+
+        let first = book.update(peer_id, now, &initial);
+        assert_eq!(first.added, initial);
+        assert!(first.removed.is_empty());
+
+        let second = book.update(
+            peer_id,
+            now + IDENTIFY_ADDRESS_WINDOW,
+            std::slice::from_ref(&replacement),
+        );
+        assert_eq!(second.added, vec![replacement]);
+        assert_eq!(second.removed, vec![(peer_id, initial[0].clone())]);
+    }
+
+    #[test]
+    fn identify_oversized_reports_keep_the_same_refreshed_subset() {
+        let peer_id = PeerId::random();
+        let now = Instant::now();
+        let mut report = (0..IDENTIFY_ADDRESS_LIMIT + 2)
+            .map(|index| identify_address(peer_id, index as u8 + 1, 10_000 + index as u16))
+            .collect::<Vec<_>>();
+        let mut book = IdentifyAddressBook::default();
+
+        let first = book.update(peer_id, now, &report);
+        assert_eq!(first.added.len(), IDENTIFY_ADDRESS_LIMIT);
+        assert_eq!(first.removed.len(), 0);
+        let retained = first.added.clone();
+
+        report.reverse();
+        let second = book.update(peer_id, now + IDENTIFY_ADDRESS_WINDOW, &report);
+        assert!(second.added.is_empty());
+        assert!(second.removed.is_empty());
+        let current = book
+            .peers
+            .get(&peer_id)
+            .unwrap()
+            .addresses
+            .iter()
+            .map(|entry| entry.address.clone())
+            .collect::<Vec<_>>();
+        assert_eq!(current, retained);
+    }
+
+    #[test]
+    fn identify_addresses_rate_limit_new_entries_across_updates() {
+        let peer_id = PeerId::random();
+        let now = Instant::now();
+        let initial = (0..IDENTIFY_NEW_ADDRESS_LIMIT)
+            .map(|index| identify_address(peer_id, index as u8 + 1, 10_000 + index as u16))
+            .collect::<Vec<_>>();
+        let replacement = identify_address(peer_id, 200, 20_000);
+        let mut book = IdentifyAddressBook::default();
+
+        assert_eq!(book.update(peer_id, now, &initial).added, initial);
+        assert!(book
+            .update(peer_id, now, std::slice::from_ref(&replacement))
+            .added
+            .is_empty());
+        assert_eq!(
+            book.update(
+                peer_id,
+                now + IDENTIFY_ADDRESS_WINDOW,
+                std::slice::from_ref(&replacement),
+            )
+            .added,
+            vec![replacement]
+        );
+    }
+
+    #[test]
+    fn identify_addresses_have_a_global_budget_across_peers() {
+        let now = Instant::now();
+        let mut book = IdentifyAddressBook::default();
+        let mut first = None;
+
+        for index in 0..IDENTIFY_GLOBAL_ADDRESS_LIMIT {
+            let peer_id = PeerId::random();
+            let address = identify_address(peer_id, (index % 254) as u8 + 1, 10_000 + index as u16);
+            if first.is_none() {
+                first = Some((peer_id, address.clone()));
+            }
+            assert!(book.update(peer_id, now, &[address]).removed.is_empty());
+        }
+
+        let (peer_id, first_address) = first.unwrap();
+        let address = identify_address(peer_id, 250, 30_000);
+        let changes = book.update(
+            peer_id,
+            now + IDENTIFY_ADDRESS_WINDOW,
+            std::slice::from_ref(&address),
+        );
+        // The global budget is full, so the admission is rejected and the
+        // peer's existing entry is left alone.
+        assert!(changes.added.is_empty());
+        assert!(changes.removed.is_empty());
+        assert_eq!(
+            book.peers[&peer_id]
+                .addresses
+                .iter()
+                .map(|a| &a.address)
+                .collect::<Vec<_>>(),
+            vec![&first_address]
+        );
+        assert_eq!(book.address_count, IDENTIFY_GLOBAL_ADDRESS_LIMIT);
+    }
+
+    #[test]
+    fn identify_addresses_refresh_and_expire() {
+        let peer_id = PeerId::random();
+        let now = Instant::now();
+        let address = identify_address(peer_id, 1, 10_000);
+        let mut book = IdentifyAddressBook::default();
+
+        assert_eq!(
+            book.update(peer_id, now, std::slice::from_ref(&address))
+                .added,
+            vec![address.clone()]
+        );
+        let refreshed = now + IDENTIFY_ADDRESS_TTL - Duration::from_secs(1);
+        let refresh = book.update(peer_id, refreshed, std::slice::from_ref(&address));
+        assert!(refresh.added.is_empty());
+        assert!(refresh.removed.is_empty());
+        // Refreshed addresses are reported so the Identify handler re-adds
+        // them to Kademlia even though they were not newly admitted.
+        assert_eq!(refresh.refreshed, vec![address.clone()]);
+        assert!(book
+            .expire(now + IDENTIFY_ADDRESS_TTL + Duration::from_secs(1))
+            .is_empty());
+        assert_eq!(
+            book.expire(refreshed + IDENTIFY_ADDRESS_TTL + Duration::from_secs(1)),
+            vec![(peer_id, address)]
+        );
+    }
+
+    #[test]
+    fn identify_refresh_reports_only_addresses_in_the_report() {
+        let peer_id = PeerId::random();
+        let now = Instant::now();
+        let reported = identify_address(peer_id, 1, 10_000);
+        let unreported = identify_address(peer_id, 2, 20_000);
+        let mut book = IdentifyAddressBook::default();
+
+        // Admit both addresses up front so the peer holds two.
+        assert_eq!(
+            book.update(peer_id, now, &[reported.clone(), unreported.clone()])
+                .added
+                .len(),
+            2
+        );
+
+        // A later report carrying only one of the two addresses must refresh
+        // just that one: pins the negative direction of the refresh
+        // predicate, so an address the peer stopped advertising keeps its
+        // original TTL instead of being refreshed back onto the Kademlia
+        // re-admit path on every report.
+        let report_at = now + IDENTIFY_ADDRESS_TTL - Duration::from_secs(1);
+        let changes = book.update(peer_id, report_at, std::slice::from_ref(&reported));
+        assert!(changes.added.is_empty());
+        assert!(changes.removed.is_empty());
+        assert_eq!(changes.refreshed, vec![reported.clone()]);
+
+        // The unreported address still expires at its original TTL, while
+        // the refreshed address survives until its refreshed TTL.
+        assert_eq!(
+            book.expire(now + IDENTIFY_ADDRESS_TTL + Duration::from_secs(1)),
+            vec![(peer_id, unreported)]
+        );
+        assert!(book.expire(report_at + Duration::from_secs(1)).is_empty());
+        assert_eq!(
+            book.expire(report_at + IDENTIFY_ADDRESS_TTL + Duration::from_secs(1)),
+            vec![(peer_id, reported)]
+        );
+    }
+
+    #[test]
+    fn identify_addresses_reject_foreign_peer_suffixes() {
+        let peer_id = PeerId::random();
+        let other_peer_id = PeerId::random();
+        let address = identify_address(other_peer_id, 1, 10_000);
+        let mut book = IdentifyAddressBook::default();
+
+        assert!(book
+            .update(peer_id, Instant::now(), &[address])
+            .added
+            .is_empty());
+    }
+
+    #[test]
+    fn identify_addresses_canonicalize_the_peer_suffix_once() {
+        let peer_id = PeerId::random();
+        let raw = identify_address_without_peer(1, 10_000);
+        let canonical = identify_address(peer_id, 1, 10_000);
+        let mut book = IdentifyAddressBook::default();
+
+        assert_eq!(
+            book.update(peer_id, Instant::now(), &[raw]).added,
+            vec![canonical]
+        );
+    }
+
     #[test]
     fn dir_mode_replace_authority_is_the_write_bits() {
         assert!(!dir_mode_allows_untrusted_replace(0o755));

crates/gitlawb-node/src/server.rs

@@ -237,35 +237,62 @@ pub fn build_router(state: AppState) -> Router {
     .layer(axum::Extension(push_limiter));
 
     // ── IPFS content-addressed retrieval and pin listing ──────────────────
-    // `/ipfs/{cid}` carries `optional_signature` so `get_by_cid` sees the caller
-    // identity and can apply per-repo visibility (#110); anonymous callers stay
-    // anonymous and still read genuinely public content. `/api/v1/ipfs/pins`
-    // stays unsigned — gating the pin index is tracked separately (#121).
-    // `/ipfs/{cid}` also carries a per-IP flood brake: it is anon-reachable and each
-    // request can drive a full-history git walk, so the per-IP rate limiter is the
-    // outermost layer (rejects a flood before the walk-admission work), mirroring the
-    // push/create routers. The extension MUST be attached or rate_limit_by_ip is a
-    // silent no-op. `/api/v1/ipfs/pins` (no walk) is merged in unbraked, as before.
+    // Two independent sub-routers, then merged. They share a URL prefix family
+    // but have separate rate-limit policies and must not share a bucket.
+    //
+    // `/ipfs/{cid}` (CID resolver): carries `optional_signature` so `get_by_cid`
+    // sees the caller identity and can apply per-repo visibility (#110); anon
+    // callers stay anonymous and still read genuinely public content. The
+    // per-IP flood brake is layered on because the resolver is anon-reachable
+    // and each request can drive a full-history git walk — the brake is the
+    // outermost layer (rejects a flood before the walk-admission work), mirroring
+    // the push/create routers. The `IpRateLimiter` extension MUST be attached
+    // or `rate_limit_by_ip` is a silent no-op.
+    //
+    // `/api/v1/ipfs/pins` (pin listing): now carries `optional_signature` only
+    // (#121). The handler rejects requests without a verified `AuthenticatedDid`
+    // with 401. It does NOT carry the CID flood brake — `list_pins` is a single
+    // `list_pinned_cids()` call, no walk, and routing pins through the resolver's
+    // bucket would let `/ipfs/{cid}` traffic exhaust the bucket and 429 the
+    // pins endpoint, or let signed pin polling exhaust the bucket for legitimate
+    // CID reads. The two surfaces have separate availability contracts.
+    //
+    // Both sub-routers are built first with their own layer sets, then merged.
+    // This is the structure the prior routing guidance called for and the
+    // earlier 429 test for `/ipfs/{cid}` (test_support.rs) assumes.
     let ipfs_limiter = rate_limit::IpRateLimiter {
         limiter: state.ipfs_rate_limiter.clone(),
         trust: state.push_limiter_trust,
     };
-    let ipfs_routes = Router::new()
+    let ipfs_cid_routes = Router::new()
         .route("/ipfs/{cid}", get(ipfs::get_by_cid))
         .layer(middleware::from_fn_with_state(
             auth::SignatureBodyLimit(SIGNED_BODY_LIMIT),
             auth::optional_signature,
         ))
         .layer(middleware::from_fn(rate_limit::rate_limit_by_ip))
-        .layer(axum::Extension(ipfs_limiter))
-        .merge(Router::new().route("/api/v1/ipfs/pins", get(ipfs::list_pins)));
+        .layer(axum::Extension(ipfs_limiter));
+    let ipfs_pins_routes = Router::new()
+        .route("/api/v1/ipfs/pins", get(ipfs::list_pins))
+        .layer(middleware::from_fn_with_state(
+            auth::SignatureBodyLimit(SIGNED_BODY_LIMIT),
+            auth::optional_signature,
+        ));
+    let ipfs_routes = ipfs_cid_routes.merge(ipfs_pins_routes);
 
     // ── Arweave permanent anchors ──────────────────────────────────────────
-    // List endpoint only (public; issue #134 tracks surfacing
-    // visibility rules on list). The verify endpoint and its probe
-    // are deferred to the vertical slice with the uploader and a
-    // real provider contract (reviewer 2, #26 split 2/4 round 5).
-    let arweave_routes = Router::new().route("/api/v1/arweave/anchors", get(arweave::list_anchors));
+    // `list_anchors` rejects callers without a verified `AuthenticatedDid`, so
+    // unsigned enumeration is denied. The same `optional_signature` layer used
+    // on the other read surfaces is applied here, closing the anonymous
+    // anchor-listing gap alongside `/api/v1/ipfs/pins`. The verify endpoint
+    // and its probe are deferred to the vertical slice with the uploader and
+    // a real provider contract.
+    let arweave_routes = Router::new()
+        .route("/api/v1/arweave/anchors", get(arweave::list_anchors))
+        .layer(middleware::from_fn_with_state(
+            auth::SignatureBodyLimit(SIGNED_BODY_LIMIT),
+            auth::optional_signature,
+        ));
 
     // ── Bounty routes (write — require HTTP Signature) ─────────────────
     let bounty_write_routes = add_auth_layers(
@@ -672,3 +699,667 @@ async fn p2p_info(State(state): State<AppState>) -> Json<serde_json::Value> {
         None => Json(json!({ "enabled": false })),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use axum::body::Body;
+    use axum::http::{Request, StatusCode};
+    use sqlx::PgPool;
+    use tower::ServiceExt;
+
+    use crate::db::RepoRecord;
+    use crate::test_support::test_state;
+
+    /// Regression: anonymous callers must not see the pin/anchor index (#121, #134).
+    #[sqlx::test]
+    async fn unsigned_get_pins_and_anchors_is_401_through_build_router(pool: PgPool) {
+        let state = test_state(pool).await;
+
+        // Seed identifiable rows so the denial oracle proves the 401 body omits
+        // them: a status-only check would stay green if a later change served
+        // the listing before the auth layer rejected.
+        state
+            .db
+            .record_pinned_cid("seeded-pin-oid", "bafyPIN-SECRET-DO-NOT-LEAK", None)
+            .await
+            .expect("seed pin");
+        let owner_did = "did:key:zSEEDOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+        let repo = seed_repo_inline(owner_did, "seeded-repo");
+        state.db.create_repo(&repo).await.expect("seed repo");
+        state
+            .db
+            .record_arweave_anchor(&crate::db::RecordAnchorInput {
+                repo: "zSEEDOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/seeded-repo",
+                owner_did,
+                ref_name: "refs/heads/secret",
+                old_sha: "0".repeat(64).as_str(),
+                new_sha: "1".repeat(64).as_str(),
+                cid: Some("bafytest"),
+                irys_tx_id: "irys-secret-tx-DO-NOT-LEAK",
+                arweave_url: "https://arweave.net/secret-tx",
+                node_did: "did:key:zNODE",
+            })
+            .await
+            .expect("seed anchor");
+
+        let router = build_router(state);
+
+        let pins = Request::builder()
+            .method("GET")
+            .uri("/api/v1/ipfs/pins?limit=50")
+            .body(Body::empty())
+            .unwrap();
+        let pins_resp = router.clone().oneshot(pins).await.unwrap();
+        assert_eq!(
+            pins_resp.status(),
+            StatusCode::UNAUTHORIZED,
+            "anonymous pin listing must be rejected"
+        );
+        let bytes = axum::body::to_bytes(pins_resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let body = String::from_utf8_lossy(&bytes);
+        assert!(
+            !body.contains("bafyPIN-SECRET-DO-NOT-LEAK"),
+            "the 401 body must not leak the seeded pin cid; body was: {body}"
+        );
+
+        let anchors = Request::builder()
+            .method("GET")
+            .uri("/api/v1/arweave/anchors?limit=50")
+            .body(Body::empty())
+            .unwrap();
+        let anchors_resp = router.oneshot(anchors).await.unwrap();
+        assert_eq!(
+            anchors_resp.status(),
+            StatusCode::UNAUTHORIZED,
+            "anonymous anchors listing must be rejected"
+        );
+        let bytes = axum::body::to_bytes(anchors_resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let body = String::from_utf8_lossy(&bytes);
+        assert!(
+            !body.contains("irys-secret-tx-DO-NOT-LEAK"),
+            "the 401 body must not leak the anchor's irys tx id; body was: {body}"
+        );
+    }
+
+    /// Regression: a real RFC-9421 signature produced exactly as `gl` does — built
+    /// with `gitlawb_core::http_sig::sign_request` over a GET, headers attached,
+    /// and sent through the actual `build_router` — is verified by the
+    /// `optional_signature` layer that wraps the pin/anchor routes, and the
+    /// handler returns 200. Pairs with the anonymous-denial test above; one
+    /// proves headers are required, the other proves a valid header is honored.
+    /// Without this test the unsigned-denial test would stay green even if the
+    /// `optional_signature` layer were never wired onto these routes, because
+    /// signed and unsigned requests would fail identically (#134 review).
+    #[sqlx::test]
+    async fn signed_get_pins_and_anchors_succeeds_through_build_router(pool: PgPool) {
+        use gitlawb_core::http_sig::sign_request;
+        use gitlawb_core::identity::Keypair;
+
+        let state = test_state(pool).await;
+        let router = build_router(state);
+
+        let kp = Keypair::generate();
+
+        let pins_path = "/api/v1/ipfs/pins";
+        let signed = sign_request(&kp, "GET", pins_path, b"");
+        let pins = Request::builder()
+            .method("GET")
+            .uri(pins_path)
+            .header("content-digest", signed.content_digest)
+            .header("signature-input", signed.signature_input)
+            .header("signature", signed.signature)
+            .body(Body::empty())
+            .unwrap();
+        let pins_resp = router.clone().oneshot(pins).await.unwrap();
+        assert_eq!(
+            pins_resp.status(),
+            StatusCode::OK,
+            "a valid signature on /api/v1/ipfs/pins must be honored through build_router"
+        );
+
+        let anchors_path = "/api/v1/arweave/anchors";
+        let signed = sign_request(&kp, "GET", anchors_path, b"");
+        let anchors = Request::builder()
+            .method("GET")
+            .uri(anchors_path)
+            .header("content-digest", signed.content_digest)
+            .header("signature-input", signed.signature_input)
+            .header("signature", signed.signature)
+            .body(Body::empty())
+            .unwrap();
+        let anchors_resp = router.oneshot(anchors).await.unwrap();
+        assert_eq!(
+            anchors_resp.status(),
+            StatusCode::OK,
+            "a valid signature on /api/v1/arweave/anchors must be honored through build_router"
+        );
+    }
+
+    /// Companion to the two regressions above: a request that *carries* signature
+    /// headers but whose signature does not verify (here, garbled) must be denied
+    /// with 401, not silently treated as anonymous and re-checked by the handler.
+    /// This pins the failure mode of the `optional_signature` layer: when the
+    /// caller claims to be signed, the layer must commit to verifying — there is
+    /// no fall-through path that lets a bad signature bypass auth.
+    #[sqlx::test]
+    async fn malformed_signature_on_pins_is_401_through_build_router(pool: PgPool) {
+        use gitlawb_core::http_sig::sign_request;
+        use gitlawb_core::identity::Keypair;
+
+        let state = test_state(pool).await;
+        let router = build_router(state);
+
+        let kp = Keypair::generate();
+        let path = "/api/v1/ipfs/pins";
+        let mut signed = sign_request(&kp, "GET", path, b"");
+        // Flip a character well inside the signature value (base64) so the header
+        // still parses but does not verify against the key.
+        let mut tampered = signed.signature.clone();
+        let mid = tampered.len() / 2;
+        let flipped = if tampered.as_bytes()[mid] == b'A' {
+            'B'
+        } else {
+            'A'
+        };
+        tampered.replace_range(mid..mid + 1, &flipped.to_string());
+        signed.signature = tampered;
+
+        let req = Request::builder()
+            .method("GET")
+            .uri(path)
+            .header("content-digest", signed.content_digest)
+            .header("signature-input", signed.signature_input)
+            .header("signature", signed.signature)
+            .body(Body::empty())
+            .unwrap();
+        let resp = router.oneshot(req).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::UNAUTHORIZED,
+            "a malformed signature must be rejected by the auth layer, not silently accepted"
+        );
+    }
+
+    /// The same malformed-signature rejection must hold on the anchors route,
+    /// which carries its own `optional_signature` layer beside pins: a later
+    /// edit that dropped the layer here (but not on pins) would otherwise stay
+    /// green.
+    #[sqlx::test]
+    async fn malformed_signature_on_anchors_is_401_through_build_router(pool: PgPool) {
+        use gitlawb_core::http_sig::sign_request;
+        use gitlawb_core::identity::Keypair;
+
+        let state = test_state(pool).await;
+        let router = build_router(state);
+
+        let kp = Keypair::generate();
+        let path = "/api/v1/arweave/anchors";
+        let mut signed = sign_request(&kp, "GET", path, b"");
+        let mut tampered = signed.signature.clone();
+        let mid = tampered.len() / 2;
+        let flipped = if tampered.as_bytes()[mid] == b'A' {
+            'B'
+        } else {
+            'A'
+        };
+        tampered.replace_range(mid..mid + 1, &flipped.to_string());
+        signed.signature = tampered;
+
+        let req = Request::builder()
+            .method("GET")
+            .uri(path)
+            .header("content-digest", signed.content_digest)
+            .header("signature-input", signed.signature_input)
+            .header("signature", signed.signature)
+            .body(Body::empty())
+            .unwrap();
+        let resp = router.oneshot(req).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::UNAUTHORIZED,
+            "a malformed signature must be rejected by the auth layer, not silently accepted"
+        );
+    }
+
+    // ── Scoped anchor contract (P1 follow-up: ?repo= must authorize_repo_read) ──
+    //
+    // The follow-up review (after the auth-layer wiring landed) found that a
+    // signed but unauthorized caller could still obtain scoped anchor metadata
+    // because the `?repo=` branch handed the user-supplied string straight to
+    // the SQL filter. These tests exercise the full production contract end to
+    // end: real RFC-9421 signature → `optional_signature` layer →
+    // `authorize_repo_read` → SQL. They fail closed if the authz call is moved
+    // after the query or removed entirely.
+
+    /// Inline repo seed for the scoped-anchor tests. Mirrors the shape in
+    /// `test_support::tests::seed_repo` without taking a cross-module private
+    /// helper as a dependency.
+    fn seed_repo_inline(owner_did: &str, name: &str) -> RepoRecord {
+        let now = chrono::Utc::now();
+        RepoRecord {
+            id: uuid::Uuid::new_v4().to_string(),
+            name: name.to_string(),
+            owner_did: owner_did.to_string(),
+            description: None,
+            is_public: true,
+            default_branch: "main".to_string(),
+            created_at: now,
+            updated_at: now,
+            disk_path: format!("/tmp/{name}"),
+            forked_from: None,
+            machine_id: None,
+        }
+    }
+
+    /// Unsigned scoped anchor request → 401, identical to the global anchor
+    /// 401 test. Proves the auth layer fires before any scope decision (no
+    /// existence oracle via the `?repo=` path either).
+    #[sqlx::test]
+    async fn unsigned_scoped_anchors_is_401_through_build_router(pool: PgPool) {
+        let state = test_state(pool).await;
+
+        // Seed an anchor under a repo the request claims to scope to, so the
+        // denial oracle proves the 401 body omits it rather than merely
+        // returning the right status over an empty table.
+        let owner_did = "did:key:zSCOPEDAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+        let repo = seed_repo_inline(owner_did, "priv");
+        state.db.create_repo(&repo).await.expect("seed repo");
+        state
+            .db
+            .record_arweave_anchor(&crate::db::RecordAnchorInput {
+                repo: "zSCOPEDAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/priv",
+                owner_did,
+                ref_name: "refs/heads/secret",
+                old_sha: "0".repeat(64).as_str(),
+                new_sha: "1".repeat(64).as_str(),
+                cid: Some("bafytest"),
+                irys_tx_id: "irys-scoped-secret-DO-NOT-LEAK",
+                arweave_url: "https://arweave.net/scoped-secret",
+                node_did: "did:key:zNODE",
+            })
+            .await
+            .expect("seed anchor");
+
+        let router = build_router(state);
+
+        let resp = router
+            .oneshot(
+                Request::builder()
+                    .method("GET")
+                    .uri("/api/v1/arweave/anchors?repo=did:key:zSCOPED%2Fpriv")
+                    .body(Body::empty())
+                    .unwrap(),
+            )
+            .await
+            .unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::UNAUTHORIZED,
+            "unsigned scoped anchor request must be 401, before any scope lookup"
+        );
+        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let body = String::from_utf8_lossy(&bytes);
+        assert!(
+            !body.contains("irys-scoped-secret-DO-NOT-LEAK"),
+            "the 401 body must not leak the anchor's irys tx id; body was: {body}"
+        );
+    }
+
+    /// Signed owner on a private repo → 200, anchor metadata returned. Mirrors
+    /// the existing `list_webhooks_accepts_a_real_gl_signature_e2e` shape: real
+    /// signature, real middleware, real `authorize_repo_read`.
+    #[sqlx::test]
+    async fn signed_scoped_anchors_owner_succeeds_through_build_router(pool: PgPool) {
+        use gitlawb_core::http_sig::sign_request;
+        use gitlawb_core::identity::Keypair;
+
+        let kp = Keypair::generate();
+        let owner_did = kp.did().to_string();
+        let state = test_state(pool).await;
+
+        let mut repo = seed_repo_inline(&owner_did, "scoped-priv");
+        repo.is_public = false;
+        state.db.create_repo(&repo).await.expect("seed repo");
+        let short = owner_did.split(':').next_back().unwrap().to_string();
+        state
+            .db
+            .record_arweave_anchor(&crate::db::RecordAnchorInput {
+                repo: &format!("{short}/scoped-priv"),
+                owner_did: &owner_did,
+                ref_name: "refs/heads/main",
+                old_sha: "0".repeat(64).as_str(),
+                new_sha: "1".repeat(64).as_str(),
+                cid: Some("bafytest"),
+                irys_tx_id: "irys-owner-tx",
+                arweave_url: "https://arweave.net/owner-tx",
+                node_did: "did:key:zNODE",
+            })
+            .await
+            .expect("seed anchor");
+
+        let path = format!("/api/v1/arweave/anchors?repo={short}/scoped-priv");
+        let signed = sign_request(&kp, "GET", &path, b"");
+        let req = Request::builder()
+            .method("GET")
+            .uri(&path)
+            .header("content-digest", signed.content_digest)
+            .header("signature-input", signed.signature_input)
+            .header("signature", signed.signature)
+            .body(Body::empty())
+            .unwrap();
+
+        let router = build_router(state);
+        let resp = router.oneshot(req).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::OK,
+            "the owner of a private repo must see their scoped anchors"
+        );
+        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let body = String::from_utf8_lossy(&bytes);
+        assert!(
+            body.contains("irys-owner-tx"),
+            "owner must see the anchor's irys tx id; body was: {body}"
+        );
+        // Owner must see the ref name + new SHA so this is not just a count oracle.
+        assert!(
+            body.contains("refs/heads/main"),
+            "owner must see the ref name; body was: {body}"
+        );
+
+        // Sanity: a second test would need a different repo name to avoid
+        // colliding on `did:key:zSCOPED/scoped-priv` in the anchors table.
+        // Distinct repo names per test keep the rows queryable in isolation.
+        let _ = repo;
+    }
+
+    /// #121 R3: the SQL filter must use the canonical stored slug, not the
+    /// user-supplied `?repo=` string. Anchor rows are stored as
+    /// `{normalize_owner_key(owner_did)}/{name}` (the short form). Authz
+    /// passes for the full-DID form, but a literal `WHERE repo=$1` against
+    /// the full DID would match zero rows and return a false empty page,
+    /// breaking callers that legitimately use the full `did:key:…/name`
+    /// form in `?repo=`. RED before the fix: handler built the filter from
+    /// the raw `?repo=` string, so this query returned `anchors: []`.
+    #[sqlx::test]
+    async fn signed_scoped_anchors_full_did_form_matches_stored_slug_through_build_router(
+        pool: PgPool,
+    ) {
+        use gitlawb_core::http_sig::sign_request;
+        use gitlawb_core::identity::Keypair;
+
+        let kp = Keypair::generate();
+        let owner_did = kp.did().to_string();
+        let state = test_state(pool).await;
+
+        let mut repo = seed_repo_inline(&owner_did, "scoped-did-form");
+        repo.is_public = false;
+        state.db.create_repo(&repo).await.expect("seed repo");
+        // Use the same slug-construction the writer uses
+        // (`db::normalize_owner_key`) so the test reader and the production
+        // writer can't drift if a future change moves the slug off the
+        // short-form. P3 (reviewer-3): the previous `split(':').next_back()`
+        // hand-rolled a third convention that agreed with the writer for
+        // `did:key` owners and diverged for every other method.
+        let short = crate::db::normalize_owner_key(&owner_did).to_string();
+        state
+            .db
+            .record_arweave_anchor(&crate::db::RecordAnchorInput {
+                repo: &format!("{short}/scoped-did-form"),
+                owner_did: &owner_did,
+                ref_name: "refs/heads/main",
+                old_sha: "0".repeat(64).as_str(),
+                new_sha: "1".repeat(64).as_str(),
+                cid: Some("bafytest"),
+                irys_tx_id: "irys-did-form-tx",
+                arweave_url: "https://arweave.net/did-form-tx",
+                node_did: "did:key:zNODE",
+            })
+            .await
+            .expect("seed anchor");
+
+        // P2 (reviewer-3): the previous suite only proved the authorized
+        // row is present. A correct filter must ALSO prove that anchors
+        // for OTHER repos do not leak into the response — the failure
+        // mode of the original bug was a cross-repo leak, not a
+        // fail-safe empty page. Seed a second repo (also under the
+        // same owner, so the authz test stays scoped) with its own
+        // irys_tx_id and assert it is absent from the body.
+        let other_owner = {
+            let kp2 = Keypair::generate();
+            kp2.did().to_string()
+        };
+        let mut other_repo = seed_repo_inline(&other_owner, "other-repo");
+        other_repo.is_public = true;
+        state
+            .db
+            .create_repo(&other_repo)
+            .await
+            .expect("seed other repo");
+        state
+            .db
+            .record_arweave_anchor(&crate::db::RecordAnchorInput {
+                repo: &format!(
+                    "{}/other-repo",
+                    crate::db::normalize_owner_key(&other_owner)
+                ),
+                owner_did: &other_owner,
+                ref_name: "refs/heads/main",
+                old_sha: "0".repeat(64).as_str(),
+                new_sha: "2".repeat(64).as_str(),
+                cid: Some("bafyother"),
+                irys_tx_id: "irys-other-repo-tx",
+                arweave_url: "https://arweave.net/other-tx",
+                node_did: "did:key:zNODE",
+            })
+            .await
+            .expect("seed other anchor");
+
+        // The caller passes the FULL DID form in ?repo=. validate_repo_slug
+        // accepts it, authorize_repo_read resolves it, and the handler MUST
+        // then translate to the canonical stored slug (short form) before
+        // running the SQL — otherwise this returns 200 with anchors: [].
+        let path = format!("/api/v1/arweave/anchors?repo={owner_did}/scoped-did-form");
+        let signed = sign_request(&kp, "GET", &path, b"");
+        let req = Request::builder()
+            .method("GET")
+            .uri(&path)
+            .header("content-digest", signed.content_digest)
+            .header("signature-input", signed.signature_input)
+            .header("signature", signed.signature)
+            .body(Body::empty())
+            .unwrap();
+
+        let router = build_router(state);
+        let resp = router.oneshot(req).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::OK,
+            "authz passed via the full DID form, so the response must be 200"
+        );
+        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let body = String::from_utf8_lossy(&bytes);
+        assert!(
+            body.contains("irys-did-form-tx"),
+            "the SQL filter must use the stored short slug, not the full DID — \
+             a false-empty page would break callers that pass ?repo=did:key:…/name. \
+             body was: {body}"
+        );
+        assert!(
+            !body.contains("irys-other-repo-tx"),
+            "the scoped anchor filter must exclude other repos' anchors — \
+             a cross-repo leak would expose metadata of repos the caller \
+             did not authorize. body was: {body}"
+        );
+
+        let _ = repo;
+    }
+
+    /// Signed non-reader on a private repo → 404, anchor metadata MUST NOT
+    /// leak. This is the test that catches the "auth-but-no-authz" bug if
+    /// the gate is moved after the SQL query or removed entirely. The 404
+    /// is the standard `repo_not_found` shape — indistinguishable from the
+    /// missing-repo case below.
+    #[sqlx::test]
+    async fn signed_scoped_anchors_non_reader_is_404_no_leak_through_build_router(pool: PgPool) {
+        use gitlawb_core::http_sig::sign_request;
+        use gitlawb_core::identity::Keypair;
+
+        let owner_kp = Keypair::generate();
+        let stranger_kp = Keypair::generate();
+        let owner_did = owner_kp.did().to_string();
+        let stranger_did = stranger_kp.did().to_string();
+        let state = test_state(pool).await;
+
+        let mut repo = seed_repo_inline(&owner_did, "scoped-priv-nr");
+        repo.is_public = false;
+        state.db.create_repo(&repo).await.expect("seed repo");
+        let short_owner = owner_did.split(':').next_back().unwrap().to_string();
+        state
+            .db
+            .record_arweave_anchor(&crate::db::RecordAnchorInput {
+                repo: &format!("{short_owner}/scoped-priv-nr"),
+                owner_did: &owner_did,
+                ref_name: "refs/heads/secret",
+                old_sha: "2".repeat(64).as_str(),
+                new_sha: "3".repeat(64).as_str(),
+                cid: Some("bafytest"),
+                irys_tx_id: "irys-secret-tx-DO-NOT-LEAK",
+                arweave_url: "https://arweave.net/secret-tx",
+                node_did: "did:key:zNODE",
+            })
+            .await
+            .expect("seed anchor");
+
+        let _ = stranger_did; // signature carries the DID; this is just documentation
+        let path = format!("/api/v1/arweave/anchors?repo={short_owner}/scoped-priv-nr");
+        let signed = sign_request(&stranger_kp, "GET", &path, b"");
+        let req = Request::builder()
+            .method("GET")
+            .uri(&path)
+            .header("content-digest", signed.content_digest)
+            .header("signature-input", signed.signature_input)
+            .header("signature", signed.signature)
+            .body(Body::empty())
+            .unwrap();
+
+        let router = build_router(state);
+        let resp = router.oneshot(req).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::NOT_FOUND,
+            "a signed non-reader must be 404 on a private repo's anchors"
+        );
+        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let body = String::from_utf8_lossy(&bytes);
+        assert!(
+            !body.contains("irys-secret-tx-DO-NOT-LEAK"),
+            "the 404 body must not leak the anchor's irys tx id; body was: {body}"
+        );
+        assert!(
+            !body.contains("refs/heads/secret"),
+            "the 404 body must not leak the ref name; body was: {body}"
+        );
+        let v: serde_json::Value = serde_json::from_slice(&bytes).expect("json body");
+        assert_eq!(
+            v["error"], "repo_not_found",
+            "non-reader denial must carry the same error code as a missing repo"
+        );
+    }
+
+    /// Signed caller on a missing repo → 404, same shape as the non-reader
+    /// case. This is the indistinguishability half: a probe cannot tell
+    /// "private" from "absent" from the response.
+    #[sqlx::test]
+    async fn signed_scoped_anchors_missing_repo_is_404_through_build_router(pool: PgPool) {
+        use gitlawb_core::http_sig::sign_request;
+        use gitlawb_core::identity::Keypair;
+
+        let kp = Keypair::generate();
+        let state = test_state(pool).await;
+
+        let short = kp
+            .did()
+            .to_string()
+            .split(':')
+            .next_back()
+            .unwrap()
+            .to_string();
+        let path = format!("/api/v1/arweave/anchors?repo={short}/does-not-exist");
+        let signed = sign_request(&kp, "GET", &path, b"");
+        let req = Request::builder()
+            .method("GET")
+            .uri(&path)
+            .header("content-digest", signed.content_digest)
+            .header("signature-input", signed.signature_input)
+            .header("signature", signed.signature)
+            .body(Body::empty())
+            .unwrap();
+
+        let router = build_router(state);
+        let resp = router.oneshot(req).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::NOT_FOUND,
+            "a missing repo must 404 indistinguishably from a non-readable private repo"
+        );
+        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let v: serde_json::Value = serde_json::from_slice(&bytes).expect("json body");
+        assert_eq!(
+            v["error"], "repo_not_found",
+            "the 404 body must carry the standard repo_not_found error code"
+        );
+    }
+
+    /// `?limit=-1` must not crash as `LIMIT -1` (Postgres 500). It clamps to
+    /// zero and returns 200 with an empty list — the same shape as a valid
+    /// listing that happens to have no rows in the configured range.
+    #[sqlx::test]
+    async fn signed_anchors_negative_limit_clamps_to_zero_through_build_router(pool: PgPool) {
+        use gitlawb_core::http_sig::sign_request;
+        use gitlawb_core::identity::Keypair;
+
+        let kp = Keypair::generate();
+        let state = test_state(pool).await;
+
+        let path = "/api/v1/arweave/anchors?limit=-1";
+        let signed = sign_request(&kp, "GET", path, b"");
+        let req = Request::builder()
+            .method("GET")
+            .uri(path)
+            .header("content-digest", signed.content_digest)
+            .header("signature-input", signed.signature_input)
+            .header("signature", signed.signature)
+            .body(Body::empty())
+            .unwrap();
+
+        let router = build_router(state);
+        let resp = router.oneshot(req).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::OK,
+            "?limit=-1 must clamp, not 500"
+        );
+        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let v: serde_json::Value = serde_json::from_slice(&bytes).expect("json body");
+        assert_eq!(v["count"], 0, "clamped limit yields an empty list");
+        assert_eq!(v["anchors"].as_array().map(|a| a.len()), Some(0));
+    }
+}

crates/gitlawb-node/src/test_support.rs

@@ -1036,6 +1036,345 @@ mod tests {
         );
     }
 
+    /// #496: create_task must reject a caller-supplied repo_id that names a
+    /// hosted repo the caller does not own. A stranger naming the victim's repo
+    /// gets 403, leaking nothing about the repo and persisting nothing; the
+    /// owner files (201). Repo-less tasks stay allowed; an id naming no hosted
+    /// repo (unknown, or quarantined, hidden as if it did not exist) is stored
+    /// verbatim as an opaque label, and the 403-vs-201 split between a hosted
+    /// id and an unknown one is itself a one-bit oracle. This is a write gate:
+    /// task reads are caller-independent and not repo-scoped on this base.
+    #[sqlx::test]
+    async fn create_task_rejects_foreign_repo_id(pool: PgPool) {
+        let owner = "did:key:zTASKREPOOWNERAAAAAAAAAAAAAAAAAAAAAAAAAA";
+        let stranger = "did:key:zTASKREPOSTRANGERBBBBBBBBBBBBBBBBBBBBBB";
+        let state = test_state(pool).await;
+        let repo = seed_repo(owner, "task-gate-repo");
+        state.db.create_repo(&repo).await.expect("seed repo");
+
+        let router = || {
+            Router::new()
+                .route(
+                    "/api/v1/tasks",
+                    axum::routing::post(crate::api::tasks::create_task),
+                )
+                .with_state(state.clone())
+        };
+        let post_as = |signer: &str, body: String| {
+            router().oneshot(signed_request_as(
+                signer,
+                Method::POST,
+                "/api/v1/tasks",
+                Body::from(body),
+            ))
+        };
+        let body_with = |signer: &str, repo_id: Option<&str>| {
+            let repo_field = repo_id
+                .map(|id| format!(r#","repo_id":"{id}""#))
+                .unwrap_or_default();
+            format!(
+                r#"{{"kind":"build","capability":"repo:write","delegator_did":"{signer}"{repo_field}}}"#
+            )
+        };
+        let created_id = |bytes: &[u8]| -> String {
+            let v: serde_json::Value = serde_json::from_slice(bytes).expect("task json");
+            v["id"].as_str().expect("task id").to_string()
+        };
+
+        // Stranger filing under the victim's repo id → exact 403.
+        let resp = post_as(stranger, body_with(stranger, Some(&repo.id)))
+            .await
+            .unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::FORBIDDEN,
+            "a non-owner must not file tasks against a foreign repo id"
+        );
+        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let text = String::from_utf8_lossy(&bytes);
+        assert!(
+            text.contains(r#""error":"forbidden""#),
+            "denial must keep the forbidden envelope: {text}"
+        );
+        assert!(
+            text.contains(crate::api::REPO_OWNER_ONLY_MESSAGE),
+            "denial must carry the shared owner-denial text (same as GraphQL): {text}"
+        );
+
+        // The denial must not persist: no task row may carry the victim's
+        // repo id afterwards — a 403 that still writes is the injection.
+        let stored = state
+            .db
+            .list_tasks(None, None, 50)
+            .await
+            .expect("list tasks");
+        assert!(
+            stored
+                .iter()
+                .all(|t| t.repo_id.as_deref() != Some(repo.id.as_str())),
+            "a denied write must not leave a task under the foreign repo id"
+        );
+
+        // Owner filing under their own repo id → 201.
+        let resp = post_as(owner, body_with(owner, Some(&repo.id)))
+            .await
+            .unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::CREATED,
+            "the owner must be able to file tasks against their repo"
+        );
+
+        // Repo-less task → 201 (unaffected).
+        let resp = post_as(stranger, body_with(stranger, None)).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::CREATED,
+            "repo-less task creation must keep working"
+        );
+
+        // Unknown repo id → 201 as an opaque label.
+        let resp = post_as(stranger, body_with(stranger, Some("no-such-repo-id")))
+            .await
+            .unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::CREATED,
+            "an id naming no hosted repo stays an opaque label"
+        );
+        // Read back the row: the unresolved id must be stored verbatim, not
+        // dropped or rewritten to a repo it never named.
+        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let task = state
+            .db
+            .get_task(&created_id(&bytes))
+            .await
+            .expect("get task")
+            .expect("task must be stored");
+        assert_eq!(
+            task.repo_id.as_deref(),
+            Some("no-such-repo-id"),
+            "an unresolved id must be stored verbatim as a label"
+        );
+
+        // Quarantined repo id → treated as unknown (201), never a 403 that
+        // would confirm the id names a real repo.
+        state
+            .db
+            .set_repo_quarantine(&repo.id, true)
+            .await
+            .expect("quarantine");
+        let resp = post_as(stranger, body_with(stranger, Some(&repo.id)))
+            .await
+            .unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::CREATED,
+            "a quarantined repo id must not 403 (existence oracle)"
+        );
+        // Read back the row: the quarantined id is stored verbatim as a label
+        // too, so the accepted write is not silently dropped.
+        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let task = state
+            .db
+            .get_task(&created_id(&bytes))
+            .await
+            .expect("get task")
+            .expect("task must be stored");
+        assert_eq!(
+            task.repo_id.as_deref(),
+            Some(repo.id.as_str()),
+            "a quarantined id must be stored verbatim as a label"
+        );
+    }
+
+    /// #496 (REST): a repo-lookup fault in the create_task gate must render the
+    /// module's opaque `db_error` envelope, not the raw sqlx text. Dropping a
+    /// column `get_repo_by_id` selects faults that first lookup; the 500 body
+    /// stays fixed and names no column (#250/#226).
+    #[sqlx::test]
+    async fn create_task_gate_repo_lookup_fault_is_opaque(pool: PgPool) {
+        let owner = "did:key:zTASKFAULTOWNERAAAAAAAAAAAAAAAAAAAAAAAAA";
+        let state = test_state(pool.clone()).await;
+        let repo = seed_repo(owner, "task-gate-fault-repo");
+        state.db.create_repo(&repo).await.expect("seed repo");
+
+        let router = || {
+            Router::new()
+                .route(
+                    "/api/v1/tasks",
+                    axum::routing::post(crate::api::tasks::create_task),
+                )
+                .with_state(state.clone())
+        };
+        let body = format!(
+            r#"{{"kind":"build","capability":"repo:write","delegator_did":"{owner}","repo_id":"{}"}}"#,
+            repo.id
+        );
+
+        // Drop a column the lookup's SELECT names so get_repo_by_id faults.
+        sqlx::query("ALTER TABLE repos DROP COLUMN machine_id")
+            .execute(&pool)
+            .await
+            .expect("drop machine_id");
+
+        let resp = router()
+            .oneshot(signed_request_as(
+                owner,
+                Method::POST,
+                "/api/v1/tasks",
+                Body::from(body),
+            ))
+            .await
+            .unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::INTERNAL_SERVER_ERROR,
+            "a faulted repo lookup must be a 500"
+        );
+        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let text = String::from_utf8_lossy(&bytes);
+        assert!(
+            text.contains(r#""error":"db_error""#),
+            "gate DB fault must keep the opaque envelope: {text}"
+        );
+        assert!(
+            text.contains(crate::error::DB_ERROR_MESSAGE),
+            "gate DB fault must carry the fixed message: {text}"
+        );
+        assert!(
+            !text.contains("machine_id") && !text.contains("column"),
+            "schema text leaked through the gate: {text}"
+        );
+    }
+
+    /// #496 (REST): the gate's second lookup (`is_repo_quarantined`) runs through
+    /// the same opaque helper. Dropping the column only it selects leaves
+    /// `get_repo_by_id` succeeding, so this pins that second call site too.
+    #[sqlx::test]
+    async fn create_task_gate_quarantine_lookup_fault_is_opaque(pool: PgPool) {
+        let owner = "did:key:zTASKFAULT2OWNERAAAAAAAAAAAAAAAAAAAAAAAA";
+        let state = test_state(pool.clone()).await;
+        let repo = seed_repo(owner, "task-gate-fault2-repo");
+        state.db.create_repo(&repo).await.expect("seed repo");
+
+        let router = || {
+            Router::new()
+                .route(
+                    "/api/v1/tasks",
+                    axum::routing::post(crate::api::tasks::create_task),
+                )
+                .with_state(state.clone())
+        };
+        let body = format!(
+            r#"{{"kind":"build","capability":"repo:write","delegator_did":"{owner}","repo_id":"{}"}}"#,
+            repo.id
+        );
+
+        // `quarantined` is not in get_repo_by_id's SELECT, so that lookup still
+        // succeeds and the fault lands on is_repo_quarantined.
+        sqlx::query("ALTER TABLE repos DROP COLUMN quarantined")
+            .execute(&pool)
+            .await
+            .expect("drop quarantined");
+
+        let resp = router()
+            .oneshot(signed_request_as(
+                owner,
+                Method::POST,
+                "/api/v1/tasks",
+                Body::from(body),
+            ))
+            .await
+            .unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::INTERNAL_SERVER_ERROR,
+            "a faulted quarantine lookup must be a 500"
+        );
+        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let text = String::from_utf8_lossy(&bytes);
+        assert!(
+            text.contains(r#""error":"db_error""#),
+            "quarantine DB fault must keep the opaque envelope: {text}"
+        );
+        assert!(
+            text.contains(crate::error::DB_ERROR_MESSAGE),
+            "quarantine DB fault must carry the fixed message: {text}"
+        );
+        assert!(
+            !text.contains("quarantined") && !text.contains("column"),
+            "schema text leaked through the gate: {text}"
+        );
+    }
+
+    /// #496 (REST): the create_task INSERT goes through the same opaque helper,
+    /// so a write fault must not serialize sqlx text either. A repo-less task
+    /// reaches the INSERT without a gate lookup; dropping a column the INSERT
+    /// names faults it.
+    #[sqlx::test]
+    async fn create_task_insert_fault_is_opaque(pool: PgPool) {
+        let signer = "did:key:zTASKINSERTFAULTAAAAAAAAAAAAAAAAAAAAAAAA";
+        let state = test_state(pool.clone()).await;
+
+        let router = || {
+            Router::new()
+                .route(
+                    "/api/v1/tasks",
+                    axum::routing::post(crate::api::tasks::create_task),
+                )
+                .with_state(state.clone())
+        };
+        let body =
+            format!(r#"{{"kind":"build","capability":"repo:write","delegator_did":"{signer}"}}"#);
+
+        sqlx::query("ALTER TABLE agent_tasks DROP COLUMN status")
+            .execute(&pool)
+            .await
+            .expect("drop status");
+
+        let resp = router()
+            .oneshot(signed_request_as(
+                signer,
+                Method::POST,
+                "/api/v1/tasks",
+                Body::from(body),
+            ))
+            .await
+            .unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::INTERNAL_SERVER_ERROR,
+            "a faulted INSERT must be a 500"
+        );
+        let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .unwrap();
+        let text = String::from_utf8_lossy(&bytes);
+        assert!(
+            text.contains(r#""error":"db_error""#),
+            "INSERT fault must keep the opaque envelope: {text}"
+        );
+        assert!(
+            text.contains(crate::error::DB_ERROR_MESSAGE),
+            "INSERT fault must carry the fixed message: {text}"
+        );
+        assert!(
+            !text.contains("status") && !text.contains("column"),
+            "schema text leaked through the INSERT: {text}"
+        );
+    }
+
     /// N3: get_tree gates on the REQUESTED subtree, not the repo root. A caller
     /// denied a withheld subtree is rejected there (404) but passes the gate on a
     /// non-withheld path (so the rejection is path-scoped, not repo-wide).
@@ -16560,6 +16899,181 @@ mod tests {
         );
     }
 
+    /// #134 R2 (split buckets): signed `/api/v1/ipfs/pins` polling must NOT debit
+    /// the CID resolver's per-IP flood brake. The pre-fix composition merged
+    /// pins under the same router as `/ipfs/{cid}`, so every signed pin
+    /// request consumed a token from `ipfs_rate_limiter` and a flood of pinned
+    /// traffic could 429 legitimate CID reads. RED before the split (pins
+    /// debited the bucket): a flood of pins followed by a CID request → 429
+    /// on the CID. GREEN after the split: the CID bucket is untouched.
+    #[sqlx::test]
+    async fn ipfs_pins_do_not_debit_the_cid_bucket(pool: PgPool) {
+        use gitlawb_core::http_sig::sign_request;
+        use gitlawb_core::identity::Keypair;
+        let kp = Keypair::generate();
+        let owner_did = kp.did().to_string();
+        let slug = owner_did.replace([':', '/'], "_");
+        let short = owner_did.split(':').next_back().unwrap().to_string();
+        let mut state = test_state(pool).await;
+        // Bucket size 2: any third /ipfs/{cid} request from the same IP would
+        // 429 if the bucket were touched by the preceding pin requests.
+        state.ipfs_rate_limiter = crate::rate_limit::RateLimiter::new(2, Duration::from_secs(3600));
+        state.ipfs_work_rate_limiter =
+            crate::rate_limit::RateLimiter::new(600, Duration::from_secs(3600));
+        state.push_limiter_trust = crate::rate_limit::TrustedProxy::XForwardedFor;
+
+        // Seed a public, walk-free CID so /ipfs/{cid} serves cheaply. This
+        // matters because the proof half of the test fires a real CID request
+        // and needs the route bucket to be the only thing that can 429 it.
+        let fx = seed_cid_repos(&slug, &short, &["pinsdebit"]);
+        let bare = std::path::PathBuf::from("/tmp")
+            .join(&slug)
+            .join("pinsdebit.git");
+        let repo = seed_repo(&owner_did, "pinsdebit");
+        state.db.create_repo(&repo).await.expect("seed repo");
+        let cid = pin_cid_for(&bare, &fx.public_oid, &state.db).await;
+
+        let router = crate::server::build_router(state);
+        let peer_ip = "203.0.113.50";
+
+        // Five signed pin requests — more than the bucket — all must 200 (the
+        // pins route has no rate limit; the bucket only charges /ipfs/{cid}).
+        let pins_path = "/api/v1/ipfs/pins";
+        let pins_signed = sign_request(&kp, "GET", pins_path, b"");
+        for i in 0..5 {
+            let req = Request::builder()
+                .method(Method::GET)
+                .uri(pins_path)
+                .header("content-digest", &pins_signed.content_digest)
+                .header("signature-input", &pins_signed.signature_input)
+                .header("signature", &pins_signed.signature)
+                .header("x-forwarded-for", peer_ip)
+                .body(Body::empty())
+                .unwrap();
+            let resp = router.clone().oneshot(req).await.unwrap();
+            assert_eq!(
+                resp.status(),
+                StatusCode::OK,
+                "pin request {i} must not be rate-limited even though the CID bucket is size 2"
+            );
+        }
+
+        // A subsequent /ipfs/{cid} request from the same IP must still serve
+        // 200 — the bucket was untouched by the pin traffic. (If pins had
+        // debited it, the bucket would be empty and the CID request would 429.)
+        let cid_path = format!("/ipfs/{cid}");
+        let cid_signed = sign_request(&kp, "GET", &cid_path, b"");
+        let req = Request::builder()
+            .method(Method::GET)
+            .uri(&cid_path)
+            .header("content-digest", cid_signed.content_digest)
+            .header("signature-input", cid_signed.signature_input)
+            .header("signature", cid_signed.signature)
+            .header("x-forwarded-for", peer_ip)
+            .body(Body::empty())
+            .unwrap();
+        let resp = router.oneshot(req).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::OK,
+            "the CID bucket must still have tokens — pin traffic did not debit it"
+        );
+    }
+
+    /// #134 R2 (split buckets, second half): an EXHAUSTED CID bucket must
+    /// NOT block a valid signed `/api/v1/ipfs/pins` listing. The pre-fix
+    /// composition routed pins through the same middleware stack, so a CID
+    /// flood that exhausted the bucket would 429 the pins endpoint too. RED
+    /// before the split: a CID request that 429s the bucket makes the next
+    /// pin request 429 too. GREEN after the split: pins never see this
+    /// bucket and remain servable.
+    #[sqlx::test]
+    async fn ipfs_pins_survive_an_exhausted_cid_bucket(pool: PgPool) {
+        use gitlawb_core::http_sig::sign_request;
+        use gitlawb_core::identity::Keypair;
+        let kp = Keypair::generate();
+        let owner_did = kp.did().to_string();
+        let slug = owner_did.replace([':', '/'], "_");
+        let short = owner_did.split(':').next_back().unwrap().to_string();
+        let mut state = test_state(pool).await;
+        // Bucket size 1: the first /ipfs/{cid} request from the IP spends the
+        // only token; the second is 429 (proves exhaustion).
+        state.ipfs_rate_limiter = crate::rate_limit::RateLimiter::new(1, Duration::from_secs(3600));
+        state.ipfs_work_rate_limiter =
+            crate::rate_limit::RateLimiter::new(600, Duration::from_secs(3600));
+        state.push_limiter_trust = crate::rate_limit::TrustedProxy::XForwardedFor;
+
+        let fx = seed_cid_repos(&slug, &short, &["pinsexh"]);
+        let bare = std::path::PathBuf::from("/tmp")
+            .join(&slug)
+            .join("pinsexh.git");
+        let repo = seed_repo(&owner_did, "pinsexh");
+        state.db.create_repo(&repo).await.expect("seed repo");
+        let cid = pin_cid_for(&bare, &fx.public_oid, &state.db).await;
+
+        let router = crate::server::build_router(state);
+        let peer_ip = "203.0.113.60";
+
+        // First /ipfs/{cid} request — debits the single CID token, serves 200.
+        let cid_path = format!("/ipfs/{cid}");
+        let cid_signed = sign_request(&kp, "GET", &cid_path, b"");
+        let req = Request::builder()
+            .method(Method::GET)
+            .uri(&cid_path)
+            .header("content-digest", cid_signed.content_digest)
+            .header("signature-input", cid_signed.signature_input)
+            .header("signature", cid_signed.signature)
+            .header("x-forwarded-for", peer_ip)
+            .body(Body::empty())
+            .unwrap();
+        let resp = router.clone().oneshot(req).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::OK,
+            "the first CID request spends the token and serves"
+        );
+
+        // Second /ipfs/{cid} request — bucket empty → 429 (proves the
+        // exhaustion is real and observable on this router). Re-sign because
+        // `SignedHeaders` holds owned `String`s.
+        let cid_signed_2 = sign_request(&kp, "GET", &cid_path, b"");
+        let req = Request::builder()
+            .method(Method::GET)
+            .uri(&cid_path)
+            .header("content-digest", cid_signed_2.content_digest)
+            .header("signature-input", cid_signed_2.signature_input)
+            .header("signature", cid_signed_2.signature)
+            .header("x-forwarded-for", peer_ip)
+            .body(Body::empty())
+            .unwrap();
+        let resp = router.clone().oneshot(req).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::TOO_MANY_REQUESTS,
+            "the second CID request from the same IP is 429 — bucket exhausted"
+        );
+
+        // A signed /api/v1/ipfs/pins request from the SAME IP must still
+        // serve 200 — pins are auth-only and never see this bucket.
+        let pins_path = "/api/v1/ipfs/pins";
+        let pins_signed = sign_request(&kp, "GET", pins_path, b"");
+        let req = Request::builder()
+            .method(Method::GET)
+            .uri(pins_path)
+            .header("content-digest", pins_signed.content_digest)
+            .header("signature-input", pins_signed.signature_input)
+            .header("signature", pins_signed.signature)
+            .header("x-forwarded-for", peer_ip)
+            .body(Body::empty())
+            .unwrap();
+        let resp = router.oneshot(req).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::OK,
+            "an exhausted CID bucket must not block a valid signed pin listing"
+        );
+    }
+
     /// U5 (R6): the two buckets are independent — the WORK budget can be exhausted
     /// (429) WITHOUT draining the ROUTE bucket. Through the production router, route
     /// generous (5) but work tight (1): one request drives two legacy probes, so the

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"
+    );
+}