Merge remote-tracking branch 'origin/main' into fix/issue-26-split-3-cert-compat

A cert declaring an unsupported version printed INVALID, which reads as
a failed Ed25519 check even though verify_signature never ran. The
display now prints UNSUPPORTED VERSION for version refusals and keeps
INVALID for real signature failures. The Show help no longer cites
review rounds or commit history.

README.md

@@ -66,7 +66,7 @@ Good today:
 
 Known limitations:
 
-- Repository write authorization is not secure by default: `GITLAWB_ENFORCE_OWNER_PUSH` defaults to `false` for compatibility, so a valid HTTP Signature identifies a pusher but does not enforce owner-only pushes.
+- Repository write authorization is secure by default: `GITLAWB_ENFORCE_OWNER_PUSH` defaults to `true`, so a valid HTTP Signature identifies a pusher and the node additionally requires that pusher to be the repository owner. Set `GITLAWB_ENFORCE_OWNER_PUSH=false` only for the duration of a rolling upgrade whose pushers are not yet the repo owner. Authentication is not authorization on its own (anyone can mint a `did:key` and sign), so owner enforcement is the only thing that prevents every signed caller from pushing to every repository, private ones included.
 - UCAN proof chains are validated when supplied, but UCAN capabilities are not consulted by write authorization and the root issuer is not independently trust-anchored. UCANs therefore do not yet grant scoped collaborator access.
 - Agent lifecycle revocation is not enforced by HTTP Signature authorization; do not rely on removing or revoking an agent record to block a compromised signer.
 - Read visibility is not a blanket data-classification boundary: task, IPFS-pin, and Arweave-anchor listings are not repository-gated; withheld path names can be visible to a root reader; and later visibility changes cannot retract content already announced or externally anchored.
@@ -311,6 +311,7 @@ metadata        local disk / optional S3
 | DID | A user, agent, or node identity derived from an Ed25519 public key. |
 | HTTP Signature | RFC 9421 signature proving control of the DID key for write requests. |
 | Ref certificate | Signed record of a ref update. Useful for audit and replication. |
+| Ref certificate versioning | The wire format is versioned; new certificates are currently issued as v1. `gl cert show <repo> <id> --verify` requires `--expect-node <did>`, and the queried node's self-reported DID is **not** accepted as a trust anchor. v2 is reserved for a future payload shape that carries the version inside the signed bytes; this client refuses to verify v2+ rather than guessing. |
 | UCAN | Delegation token for future capability-based workflows. |
 | Peer announce | Node-to-node HTTP announcement of DID + public URL. |
 | Gossipsub | libp2p topic for ref-update events. |
@@ -400,7 +401,7 @@ Important node settings:
 | `GITLAWB_P2P_KEY` | Path to the node's persistent libp2p identity key file, which fixes the PeerId across restarts. Must name a key file inside a directory, such as `/data/keys/p2p.key` or `./keys/p2p.key`. Two kinds of problem are handled differently. A value that cannot name a securable key file at all (a bare filename, `..` traversal, a trailing `/`, the filesystem root, or a `~/` path that escapes home) is refused before the node binds, so the process exits. Anything wrong with the storage itself (a symlinked or non-directory parent, an unsafe ancestor or working directory, an unreadable or malformed key) leaves p2p off while HTTP keeps serving; the node logs it at error level as `p2p_identity_key_load_failed` and sets the `gitlawb_p2p_identity_key_load_failed` metric to 1. Note that `/health` still reports healthy in that state, so alert on the log event or the metric rather than on the health check. Correcting the storage and restarting restores the same PeerId. On Unix a new key is created `0600` inside a `0700` directory. A key directory that grants access beyond its owner is tightened to `0700` on start; one that is closed too far is refused with a `chmod 700` remedy rather than widened, since a directory locked down on purpose is an operator decision. Rotate the key only when the node logs that the directory was group- or world-writable (or the key file itself was group- or world-readable). An ordinary `0755` directory is tightened too, but others could not read a `0600` key or replace its directory entry. An existing key file is never modified: one with group or other bits set is rejected (`chmod 600`), and so is one this node cannot read. On other platforms no permissions are enforced. Default `~/.gitlawb/p2p.key`; point it at a persistent volume when running in a container. |
 | `GITLAWB_BOOTSTRAP_DISABLE_SEEDS` | Disable embedded seed peers for isolated dev/test networks. |
 | `GITLAWB_REQUIRE_SIGNED_PEER_WRITES` | Require peer writes to prove their DID on every transport: RFC 9421 signatures on the announce/sync routes, and an Ed25519 payload signature on gossip ref-update events. Defaults to `false` during the staged rollout below. Upgrade every gossip publisher before enabling, or their updates are dropped with no error on their side. |
-| `GITLAWB_ENFORCE_OWNER_PUSH` | Require the authenticated pusher to be the repo owner on `git-receive-pack`. **Defaults to `true`.** A `did:key` signature is authentication, not authorization — anyone can mint a key and sign — so with this off every signed caller may push to every repository, private ones included. Delegated and CI keys count as non-owners: a UCAN `git/push` capability is verified but not yet honored for authorization, so they cannot push while this is on. Set `false` only for a rolling upgrade; see [`docs/RUN-A-NODE.md`](docs/RUN-A-NODE.md). |
+| `GITLAWB_ENFORCE_OWNER_PUSH` | Require the authenticated pusher to be the repo owner on `git-receive-pack`. **Defaults to `true`.** A `did:key` signature is authentication, not authorization (anyone can mint a key and sign), so with this off every signed caller may push to every repository, private ones included. Delegated and CI keys count as non-owners: a UCAN `git/push` capability is verified but not yet honored for authorization, so they cannot push while this is on. Set `false` only for a rolling upgrade; see [`docs/RUN-A-NODE.md`](docs/RUN-A-NODE.md). |
 | `GITLAWB_AUTO_SYNC` | Enable automatic sync from known peers. |
 | `GITLAWB_MAX_PACK_BYTES` | Max git pack body size for smart-HTTP routes. |
 | `GITLAWB_GIT_SERVICE_TIMEOUT_SECS` | Max seconds a served git upload-pack, receive-pack, or `info/refs` advertisement may run before it is aborted (504). Default 600. Also bounds the withheld-blob classification walk (on both the upload-pack serve and receive-pack replication paths) and the push-side pin-candidate discovery (`rev-list` / `cat-file`), each reaped via process-group teardown at the deadline. On the path-scoped upload-pack path the classification walk and the pack serve share ONE deadline, so this value bounds their combined duration rather than granting each stage a full budget: a walk that consumes it leaves the serve nothing and the clone gets a 504. Serving large path-scoped repos may therefore need a higher value than they did when each stage was budgeted separately. Accepted range is 1 to 3153600000 (100 years), since the node derives deadlines from this value and a larger one cannot be represented. |
@@ -553,7 +554,7 @@ Short-term priorities:
 2. Add Docker and installer smoke tests.
 3. Improve operator docs and `gl doctor` checks.
 4. Harden peer writes and publish the signed-peer rollout plan.
-5. Close default-open write authorization and wire trusted UCAN delegation into repository permissions.
+5. Wire trusted UCAN delegation into repository write authorization.
 6. Add threaded, line-level pull-request discussions and enforce approval requirements on merges.
 7. Add metrics for pushes, fetches, pack sizes, peer sync, failed auth, and webhooks.
 

SECURITY.md

@@ -54,9 +54,9 @@ We will acknowledge receipt within 48 hours and aim to release a fix within 14 d
 These are documented limitations of the current live release. They should be prioritized without breaking existing nodes during rolling upgrades.
 
 ### Repository write authorization defaults
-- `git-receive-pack` verifies HTTP Signatures, but `GITLAWB_ENFORCE_OWNER_PUSH` defaults to `false` for compatibility during rollout.
-- **Impact:** With the default setting, a valid signature authenticates the pusher but does not require that DID to be the repository owner.
-- **Mitigation:** Set `GITLAWB_ENFORCE_OWNER_PUSH=true` on nodes where owner-only pushes are required. Confirm that every legitimate pusher uses the owner DID before enabling it.
+- `git-receive-pack` verifies HTTP Signatures and, with the default configuration, also requires the pusher DID to be the repository owner: `GITLAWB_ENFORCE_OWNER_PUSH` defaults to `true`.
+- **Impact:** With the default setting, a valid signature authenticates the pusher *and* the pusher must be the repository owner. Authentication is not authorization on its own — anyone can mint a `did:key` and sign — so owner enforcement is the only thing that prevents every signed caller from pushing to every repository, including private ones.
+- **Mitigation:** `GITLAWB_ENFORCE_OWNER_PUSH=false` is the explicit rolling-upgrade compatibility override. Disable it only for the duration of an upgrade whose pushers are not yet the repo owner, then turn it back on. Delegated and CI keys count as non-owners: a UCAN `git/push` capability is verified but not yet honored for authorization, so they cannot push while this is on.
 
 ### UCAN delegation and revocation
 - The middleware validates a supplied UCAN's complete proof chain, but a root token is accepted without an independently trusted issuer anchor. `Ucan::can` is not yet used by write handlers, so a UCAN does not grant scoped repository access.

crates/gitlawb-attest/src/attestation.rs

@@ -504,6 +504,23 @@ mod tests {
         assert!(matches!(err, Error::Did(_)));
     }
 
+    #[test]
+    fn verify_rejects_oversized_method_id_before_decoding() {
+        let sk = fresh();
+        let mut att = dummy_attestation(&sk, sample_cert_hash());
+        // A well-formed prefix on an absurdly long id must fail the length
+        // bound, not the decoder: base58 decode cost is quadratic in input
+        // length, so reaching it at all is the defect.
+        att.signer = format!("did:key:z{}", "1".repeat(256));
+        let err = att.verify_signature(sample_cert_hash()).unwrap_err();
+        // Match the bound's own message: any other Did error (a decode or
+        // length mismatch downstream) means the oversized input was decoded.
+        match err {
+            Error::Did(m) => assert!(m.contains("too long"), "unexpected: {m}"),
+            e => panic!("expected Error::Did, got {e:?}"),
+        }
+    }
+
     #[test]
     fn verify_rejects_bad_signature_base64() {
         let sk = fresh();

crates/gitlawb-attest/src/verifier.rs

@@ -150,23 +150,48 @@ impl Registry {
     }
 
     /// Verify a batch, then enforce `RequireAll`.
+    ///
+    /// Under [`Policy::RequireAll`] this never short-circuits: an attestation
+    /// that fails to verify — bad signature, cert-hash mismatch, or malformed
+    /// payload, the actual `Err` paths of [`Registry::verify`] — is dropped
+    /// from the result rather than aborting the batch. A valid attestation of
+    /// a type with NO registered verifier is not one of those: under
+    /// `RequireAll` it verifies to `Ok` with `fully_verified: false` and stays
+    /// in the result, preserving the distinction between *untrusted* and
+    /// *invalid* (see `require_all_is_lenient_on_unknown_types_in_the_batch`).
+    /// Any third party can attach an attestation, so a hard error here would
+    /// let an unrelated `attacker/spam/v1` deny-of-service the whole cert (the
+    /// DoS vector the module docstring warns about). The `required_types`
+    /// check below is the real gate, and it only counts `fully_verified`
+    /// entries — so a required type represented solely by a malformed
+    /// attestation still fails with [`Error::RequiredMissing`].
+    ///
+    /// Under `AcceptKnown` and `RejectUnknown` the batch stays strict: the
+    /// first verification error is surfaced to the caller.
     pub fn verify_all(
         &self,
         attestations: &[Attestation],
         expected_cert_hash: [u8; 32],
     ) -> Result<Vec<VerifiedAttestation>> {
-        let mut verified = Vec::with_capacity(attestations.len());
-        for a in attestations {
-            verified.push(self.verify(a, expected_cert_hash)?);
+        if self.policy != Policy::RequireAll {
+            let mut verified = Vec::with_capacity(attestations.len());
+            for a in attestations {
+                verified.push(self.verify(a, expected_cert_hash)?);
+            }
+            return Ok(verified);
         }
-        if self.policy == Policy::RequireAll {
-            for required in &self.required_types {
-                let present = verified
-                    .iter()
-                    .any(|v| &v.type_ == required && v.fully_verified);
-                if !present {
-                    return Err(Error::RequiredMissing(required.clone()));
-                }
+
+        // RequireAll: drop entries that fail to verify instead of aborting.
+        let verified: Vec<VerifiedAttestation> = attestations
+            .iter()
+            .filter_map(|a| self.verify(a, expected_cert_hash).ok())
+            .collect();
+        for required in &self.required_types {
+            let present = verified
+                .iter()
+                .any(|v| &v.type_ == required && v.fully_verified);
+            if !present {
+                return Err(Error::RequiredMissing(required.clone()));
             }
         }
         Ok(verified)
@@ -324,6 +349,38 @@ mod tests {
             .any(|v| v.type_ == "other/v1" && !v.fully_verified));
     }
 
+    /// The strict policies must surface a batch member's verification error
+    /// through `verify_all`, not launder it into a partial result. This binds
+    /// the POLICY BOUNDARY itself: only the `RequireAll` arm may drop failing
+    /// entries, and an apparently harmless cleanup that applies its
+    /// `filter_map(Result::ok)` shape to the strict arms would silently turn
+    /// the fail-closed contracts into fail-open — with every per-attestation
+    /// `verify` test still green, because none of them go through the batch
+    /// path. The failing entry is a wrong-cert-hash attestation (signed over a
+    /// different hash than the batch expects), a genuine `Err` under every
+    /// policy.
+    #[test]
+    fn strict_policies_propagate_a_batch_members_error() {
+        let sk = fresh();
+        let cert_hash = sample_hash();
+        let mut wrong_hash = sample_hash();
+        wrong_hash[0] ^= 0xff;
+
+        for policy in [Policy::AcceptKnown, Policy::RejectUnknown] {
+            let mut reg = Registry::new().with_policy(policy);
+            reg.register(DemoVerifier);
+            let good = signed_demo(&sk, cert_hash, "ok");
+            let bad = signed_demo(&sk, wrong_hash, "ok");
+            let err = reg
+                .verify_all(&[good, bad], cert_hash)
+                .expect_err("a strict policy must fail the whole batch");
+            assert!(
+                matches!(err, Error::Signature(_) | Error::CertHashMismatch),
+                "expected the underlying verification error, got {err:?}"
+            );
+        }
+    }
+
     /// A required type that is present but unverified (no verifier registered
     /// even though the type appears in `required_types`) must fail
     /// `verify_all` with `RequiredMissing`. Catches a misconfiguration where
@@ -341,6 +398,65 @@ mod tests {
         assert!(matches!(err, Error::RequiredMissing(t) if t == "demo/v1"));
     }
 
+    /// A cert-hash that does not match `sample_hash()`, so an attestation
+    /// signed against it fails `verify_signature` when the batch is checked
+    /// against `sample_hash()`.
+    fn other_hash() -> [u8; 32] {
+        let mut h = sample_hash();
+        h[0] ^= 0xff;
+        h
+    }
+
+    /// Under `RequireAll`, a malformed attestation (here: signed against the
+    /// wrong cert hash, so it fails signature verification) attached alongside a
+    /// valid required one must not abort the batch. The junk is dropped and the
+    /// cert still verifies. Before the fix, `verify_all` propagated the error
+    /// via `?` and the whole batch failed — the DoS vector the docstring warns
+    /// about.
+    #[test]
+    fn require_all_drops_a_malformed_extra_and_still_accepts() {
+        let sk = fresh();
+        let cert_hash = sample_hash();
+        let mut reg = Registry::new()
+            .with_policy(Policy::RequireAll)
+            .require_types(["demo/v1"]);
+        reg.register(DemoVerifier);
+
+        // Signed against `other_hash()`, so it will not verify under `cert_hash`.
+        let malformed = signed_demo(&sk, other_hash(), "boom");
+        let real = signed_demo(&sk, cert_hash, "ok");
+
+        let verified = reg
+            .verify_all(&[malformed, real], cert_hash)
+            .expect("a malformed extra must not abort the batch under RequireAll");
+        assert_eq!(verified.len(), 1, "the malformed entry should be dropped");
+        assert!(verified
+            .iter()
+            .any(|v| v.type_ == "demo/v1" && v.fully_verified));
+    }
+
+    /// Leniency must not open a hole: if the *only* attestation of a required
+    /// type is malformed, dropping it leaves the required type absent and
+    /// `verify_all` must still fail with `RequiredMissing`. Guards against a fix
+    /// that skips errors so eagerly it lets an unverified required type pass.
+    #[test]
+    fn require_all_rejects_when_only_a_malformed_required_type_is_present() {
+        let sk = fresh();
+        let cert_hash = sample_hash();
+        let mut reg = Registry::new()
+            .with_policy(Policy::RequireAll)
+            .require_types(["demo/v1"]);
+        reg.register(DemoVerifier);
+
+        // The sole demo/v1 is bound to the wrong cert hash → fails verification.
+        let malformed_required = signed_demo(&sk, other_hash(), "ok");
+
+        let err = reg
+            .verify_all(&[malformed_required], cert_hash)
+            .unwrap_err();
+        assert!(matches!(err, Error::RequiredMissing(t) if t == "demo/v1"));
+    }
+
     #[test]
     fn payload_check_failure_rejects() {
         let sk = fresh();

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/bounties.rs

@@ -285,11 +285,22 @@ pub async fn submit_bounty(
     Path(id): Path<String>,
     Json(req): Json<SubmitBountyRequest>,
 ) -> Result<Json<BountyRecord>> {
-    let bounty = state
-        .db
-        .get_bounty(&id)
-        .await?
-        .ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?;
+    let not_found = || AppError::NotFound(format!("bounty {id} not found"));
+
+    let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?;
+
+    if crate::api::authorize_repo_read(
+        &state,
+        &bounty.repo_owner,
+        &bounty.repo_name,
+        Some(auth.0.as_str()),
+        "/",
+    )
+    .await
+    .is_err()
+    {
+        return Err(not_found());
+    }
 
     if bounty.status != "claimed" {
         return Err(AppError::BadRequest(format!(
@@ -325,11 +336,22 @@ pub async fn approve_bounty(
     Path(id): Path<String>,
     Json(req): Json<ApproveBountyRequest>,
 ) -> Result<Json<BountyRecord>> {
-    let bounty = state
-        .db
-        .get_bounty(&id)
-        .await?
-        .ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?;
+    let not_found = || AppError::NotFound(format!("bounty {id} not found"));
+
+    let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?;
+
+    if crate::api::authorize_repo_read(
+        &state,
+        &bounty.repo_owner,
+        &bounty.repo_name,
+        Some(auth.0.as_str()),
+        "/",
+    )
+    .await
+    .is_err()
+    {
+        return Err(not_found());
+    }
 
     if bounty.status != "submitted" {
         return Err(AppError::BadRequest(format!(
@@ -372,11 +394,22 @@ pub async fn cancel_bounty(
     Extension(auth): Extension<AuthenticatedDid>,
     Path(id): Path<String>,
 ) -> Result<Json<BountyRecord>> {
-    let bounty = state
-        .db
-        .get_bounty(&id)
-        .await?
-        .ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?;
+    let not_found = || AppError::NotFound(format!("bounty {id} not found"));
+
+    let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?;
+
+    if crate::api::authorize_repo_read(
+        &state,
+        &bounty.repo_owner,
+        &bounty.repo_name,
+        Some(auth.0.as_str()),
+        "/",
+    )
+    .await
+    .is_err()
+    {
+        return Err(not_found());
+    }
 
     if bounty.status != "open" {
         return Err(AppError::BadRequest(format!(
@@ -407,11 +440,22 @@ pub async fn dispute_bounty(
     Extension(auth): Extension<AuthenticatedDid>,
     Path(id): Path<String>,
 ) -> Result<Json<BountyRecord>> {
-    let bounty = state
-        .db
-        .get_bounty(&id)
-        .await?
-        .ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?;
+    let not_found = || AppError::NotFound(format!("bounty {id} not found"));
+
+    let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?;
+
+    if crate::api::authorize_repo_read(
+        &state,
+        &bounty.repo_owner,
+        &bounty.repo_name,
+        Some(auth.0.as_str()),
+        "/",
+    )
+    .await
+    .is_err()
+    {
+        return Err(not_found());
+    }
 
     if bounty.status != "claimed" && bounty.status != "submitted" {
         return Err(AppError::BadRequest(format!(
@@ -456,21 +500,68 @@ pub async fn dispute_bounty(
     Ok(Json(updated))
 }
 
+/// Resolve the set of `(owner_did, name)` pairs for repos an anonymous caller
+/// can list.  Mirrors the `stats()` pattern in `server.rs` (#104): batch-load
+/// all deduped repos, batch-load their visibility rules, keep only those that
+/// pass `listable_at_root`.  Pure I/O after the two DB round-trips — no
+/// per-repo authorization queries.
+///
+/// Both bounty stats handlers mount behind `optional_signature` (see
+/// `bounty_read_routes` in server.rs), so the caller is always `None` for
+/// anonymous access.  Every caller, signed or not, currently receives the
+/// anonymous-scoped aggregates (matching the #104 pattern); per-caller
+/// visibility can be threaded through here when that work lands.
+///
+/// A database error is returned to the caller, never collapsed to an empty
+/// set: an empty set would render as HTTP 200 with zero counts, which a client
+/// reads as confirmed absence of activity.
+async fn visible_repo_pairs(state: &AppState) -> Result<Vec<(String, String)>> {
+    let rows = state.db.list_all_repos_deduped().await?;
+    let ids: Vec<String> = rows.iter().map(|r| r.id.clone()).collect();
+    let rules_by_repo = state.db.list_visibility_rules_for_repos(&ids).await?;
+    let pairs = rows
+        .iter()
+        .filter(|r| {
+            let rules = rules_by_repo.get(&r.id).map(Vec::as_slice).unwrap_or(&[]);
+            // mirror-rows-handled: a mirror with a canonical twin is deduped to the
+            // canonical row and its rules; a mirror-only repo counts as public, the
+            // same answer authorize_repo_read and /stats give. Pinned by
+            // bounty_stats_mirror_rows_follow_canonical_twin_or_count_when_alone.
+            crate::visibility::listable_at_root(rules, r.is_public, &r.owner_did, None)
+        })
+        .map(|r| {
+            (
+                crate::db::normalize_owner_key(&r.owner_did).to_string(),
+                r.name.clone(),
+            )
+        })
+        .collect();
+    Ok(pairs)
+}
+
 /// GET /api/v1/bounties/stats
+///
+/// Aggregates are restricted to anonymously-listable repos so private-repo
+/// bounty activity is not exposed (#477).  The visible-repo set is resolved
+/// once per request via `visible_repo_pairs` (two SQL round-trips), then
+/// passed into the filtered aggregate queries.
 pub async fn bounty_stats(State(state): State<AppState>) -> Result<Json<BountyStatsResponse>> {
-    let open = state.db.count_bounties_by_status("open").await.unwrap_or(0);
+    let visible = visible_repo_pairs(&state).await?;
+
+    let open = state
+        .db
+        .count_bounties_by_status_visible("open", &visible)
+        .await?;
     let claimed = state
         .db
-        .count_bounties_by_status("claimed")
-        .await
-        .unwrap_or(0);
+        .count_bounties_by_status_visible("claimed", &visible)
+        .await?;
     let completed = state
         .db
-        .count_bounties_by_status("completed")
-        .await
-        .unwrap_or(0);
+        .count_bounties_by_status_visible("completed", &visible)
+        .await?;
 
-    let leaders = state.db.bounty_leaderboard(10).await.unwrap_or_default();
+    let leaders = state.db.bounty_leaderboard_visible(10, &visible).await?;
     let leaderboard = leaders
         .into_iter()
         .map(|(did, cnt, total)| AgentBountyEntry {
@@ -489,11 +580,14 @@ pub async fn bounty_stats(State(state): State<AppState>) -> Result<Json<BountySt
 }
 
 /// GET /api/v1/agents/{did}/bounties
+///
+/// Per-agent earnings restricted to anonymously-listable repos (#477).
 pub async fn agent_bounty_stats(
     State(state): State<AppState>,
     Path(did): Path<String>,
 ) -> Result<Json<serde_json::Value>> {
-    let (count, total) = state.db.agent_bounty_stats(&did).await.unwrap_or((0, 0));
+    let visible = visible_repo_pairs(&state).await?;
+    let (count, total) = state.db.agent_bounty_stats_visible(&did, &visible).await?;
     Ok(Json(serde_json::json!({
         "did": did,
         "completed_bounties": count,

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

@@ -52,6 +52,7 @@ pub async fn list_certs(
                 "node_did":   c.node_did,
                 "signature":  c.signature,
                 "issued_at":  c.issued_at,
+                "version":    c.version,
             })
         })
         .collect();
@@ -92,5 +93,6 @@ pub async fn get_cert(
         "node_did":   cert.node_did,
         "signature":  cert.signature,
         "issued_at":  cert.issued_at,
+        "version":    cert.version,
     })))
 }

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

@@ -437,6 +437,7 @@ mod ref_updates_feed_tests {
             node_did: "did:key:z6MkNode".into(),
             signature: "sig".into(),
             issued_at: Utc::now().to_rfc3339(),
+            version: 1,
         }
     }
 

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/labels.rs

@@ -14,14 +14,13 @@ pub struct LabelRequest {
     pub label: String,
 }
 
-/// POST /api/v1/repos/:owner/:repo/labels
-pub async fn add_label(
-    State(state): State<AppState>,
-    Extension(auth): Extension<AuthenticatedDid>,
-    Path((owner, name)): Path<(String, String)>,
-    Json(req): Json<LabelRequest>,
-) -> Result<(StatusCode, Json<serde_json::Value>)> {
-    let label = req.label.trim().to_lowercase();
+/// Canonicalize and validate a label string. `add_label` and `remove_label`
+/// must agree on the form stored in the DB: `add_label` already trims and
+/// lowercases before storing, so `remove_label` has to do the same to find
+/// the row. Validation is shared so neither handler can drift on charset or
+/// length (#344).
+fn canonicalize_label(raw: &str) -> std::result::Result<String, AppError> {
+    let label = raw.trim().to_lowercase();
     if label.is_empty() || label.len() > 50 {
         return Err(AppError::BadRequest("label must be 1–50 characters".into()));
     }
@@ -33,6 +32,17 @@ pub async fn add_label(
             "label must contain only alphanumeric characters, hyphens, and colons".into(),
         ));
     }
+    Ok(label)
+}
+
+/// POST /api/v1/repos/:owner/:repo/labels
+pub async fn add_label(
+    State(state): State<AppState>,
+    Extension(auth): Extension<AuthenticatedDid>,
+    Path((owner, name)): Path<(String, String)>,
+    Json(req): Json<LabelRequest>,
+) -> Result<(StatusCode, Json<serde_json::Value>)> {
+    let label = canonicalize_label(&req.label)?;
 
     let record = state
         .db
@@ -59,6 +69,8 @@ pub async fn remove_label(
     Extension(auth): Extension<AuthenticatedDid>,
     Path((owner, name, label)): Path<(String, String, String)>,
 ) -> Result<Json<serde_json::Value>> {
+    let label = canonicalize_label(&label)?;
+
     let record = state
         .db
         .get_repo(&owner, &name)
@@ -66,7 +78,10 @@ pub async fn remove_label(
         .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?;
     crate::api::require_repo_owner(&record, &auth.0)?;
 
-    state.db.remove_label(&record.id, &label).await?;
+    let removed = state.db.remove_label(&record.id, &label).await?;
+    if !removed {
+        return Err(AppError::NotFound(format!("label '{label}'")));
+    }
     Ok(Json(serde_json::json!({ "label": label, "removed": true })))
 }
 

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/peers.rs

@@ -70,6 +70,8 @@ pub struct PeerResponse {
     pub reachable: bool,
 }
 
+pub(crate) const PUBLIC_HTTP_URL_REQUIREMENT: &str = "must be a public http(s) URL (no loopback, private, localhost, .localhost, .internal, or .local hosts)";
+
 /// Extract an IPv4 address embedded in an IPv6 literal across the transition
 /// formats that carry one: IPv4-mapped (`::ffff:a.b.c.d`), IPv4-compatible
 /// (`::a.b.c.d`), 6to4 (`2002:WWXX:YYZZ::/16`), and the NAT64 well-known prefix
@@ -104,8 +106,8 @@ fn embedded_ipv4(v6: std::net::Ipv6Addr) -> Option<std::net::Ipv4Addr> {
 
 /// Whether a peer `http_url` is a public http(s) endpoint safe to register.
 /// Rejects non-http(s) schemes, loopback/unspecified/private/link-local IPs,
-/// and `localhost` / `.local` / `.internal` hostnames. Used at announce time
-/// and by the boot-time prune of already-poisoned rows.
+/// and `localhost` / `.localhost` / `.local` / `.internal` hostnames. Used at
+/// announce time and by the boot-time prune of already-poisoned rows.
 pub fn is_public_http_url(raw: &str) -> bool {
     let url = match reqwest::Url::parse(raw) {
         Ok(u) => u,
@@ -125,6 +127,7 @@ pub fn is_public_http_url(raw: &str) -> bool {
     host.truncate(host.trim_end_matches('.').len());
     if host.is_empty()
         || host == "localhost"
+        || host.ends_with(".localhost")
         || host.ends_with(".local")
         || host.ends_with(".internal")
     {
@@ -260,9 +263,9 @@ pub async fn announce(
     // and turn our outbound sync-notify fan-out into an SSRF probe — and bury
     // the real peers under junk so node-origin repos stop replicating.
     if !is_public_http_url(&req.http_url) {
-        return Err(AppError::BadRequest(
-            "http_url must be a public http(s) URL (no loopback, private, or .internal/.local hosts)".into(),
-        ));
+        return Err(AppError::BadRequest(format!(
+            "http_url {PUBLIC_HTTP_URL_REQUIREMENT}"
+        )));
     }
 
     // Reject self-announcements: a peer row whose http_url is our own public
@@ -541,6 +544,7 @@ mod tests {
     fn rejects_loopback_private_and_internal() {
         for bad in [
             "http://localhost:7545",
+            "http://node.localhost:7545",
             "http://127.0.0.1:5432/",
             "http://localhost:22/",
             "http://0.0.0.0:7545",
@@ -620,11 +624,14 @@ mod tests {
     #[test]
     fn rejects_repeated_trailing_root_dots() {
         // Stripping a single root dot leaves `localhost.`, which matches
-        // neither the equality nor the suffix check. The subdomain form
-        // (`node.localhost..`) is not asserted here: `.localhost` is not yet a
-        // rejected suffix on this base, so that case belongs to whichever
-        // change adds it rather than to this one.
-        for bad in ["http://localhost../", "http://localhost...:7545"] {
+        // neither the equality nor the suffix check. The subdomain form now
+        // exercises the `.localhost` suffix rejection together with the extra
+        // trailing dot.
+        for bad in [
+            "http://localhost../",
+            "http://localhost...:7545",
+            "http://node.localhost../",
+        ] {
             assert!(!is_public_http_url(bad), "{bad:?} must be rejected");
         }
     }
@@ -1582,6 +1589,31 @@ mod tests {
         )
     }
 
+    #[sqlx::test]
+    async fn announce_rejects_localhost_subdomains_with_an_accurate_error(pool: PgPool) {
+        let state = test_state(pool).await;
+        let did = Keypair::generate().did().to_string();
+        let resp = announce_only(state.clone())
+            .oneshot(announce_as(
+                &did,
+                &announce_body(&did, "https://node.localhost:7545"),
+            ))
+            .await
+            .unwrap();
+
+        let (status, error, message) = status_and_error(resp).await;
+        assert_eq!(status, StatusCode::BAD_REQUEST);
+        assert_eq!(error, "bad_request");
+        assert_eq!(
+            message,
+            "http_url must be a public http(s) URL (no loopback, private, localhost, .localhost, .internal, or .local hosts)"
+        );
+        assert!(
+            snapshot(&state.db, &did).await.is_none(),
+            "a rejected .localhost announce must leave no row behind"
+        );
+    }
+
     /// U4 scenario 1, and the first test the keyid branch has ever had: a caller
     /// who proved control of one DID must not announce another's. Kills
     /// neutralizing the handler's keyid comparison, and kills demoting this

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

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

crates/gitlawb-node/src/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/api/webhooks.rs

@@ -46,9 +46,10 @@ pub async fn create_webhook(
     // endpoints (SSRF). Delivery runs on the shared no-redirect client
     // (main.rs), which closes the 3xx-to-internal bounce.
     if !crate::api::peers::is_public_http_url(&req.url) {
-        return Err(AppError::BadRequest(
-            "webhook URL must be a public http(s) URL (no loopback, private, or .internal/.local hosts)".into(),
-        ));
+        return Err(AppError::BadRequest(format!(
+            "webhook URL {}",
+            crate::api::peers::PUBLIC_HTTP_URL_REQUIREMENT
+        )));
     }
 
     let events = req.events.unwrap_or_else(|| vec!["*".into()]);
@@ -141,7 +142,19 @@ pub async fn delete_webhook(
 
 #[cfg(test)]
 mod tests {
-    use crate::api::peers::is_public_http_url;
+    use axum::extract::{Extension, Path, State};
+    use axum::Json;
+    use chrono::Utc;
+    use gitlawb_core::identity::Keypair;
+    use sqlx::PgPool;
+    use uuid::Uuid;
+
+    use super::{create_webhook, CreateWebhookRequest};
+    use crate::api::peers::{is_public_http_url, PUBLIC_HTTP_URL_REQUIREMENT};
+    use crate::auth::AuthenticatedDid;
+    use crate::db::RepoRecord;
+    use crate::error::AppError;
+    use crate::test_support::test_state;
 
     // create_webhook gates req.url through is_public_http_url. Pin the exact
     // SSRF targets from issue #81 so the webhook path can never regress to the
@@ -152,6 +165,7 @@ mod tests {
             "http://127.0.0.1:5432/",
             "http://169.254.169.254/latest/meta-data/",
             "http://localhost/",
+            "http://node.localhost/",
             "http://10.0.0.5/",
             "http://[::1]/",
             // IPv6 transition encodings smuggling loopback v4 (6to4 / NAT64).
@@ -169,4 +183,53 @@ mod tests {
         assert!(is_public_http_url("https://hooks.example.com/gitlawb"));
         assert!(is_public_http_url("http://203.0.113.10:7545/"));
     }
+
+    #[sqlx::test]
+    async fn webhook_localhost_rejection_uses_shared_public_url_contract(pool: PgPool) {
+        let state = test_state(pool).await;
+        let owner = Keypair::generate().did().to_string();
+        let repo_id = Uuid::new_v4().to_string();
+        state
+            .db
+            .create_repo(&RepoRecord {
+                id: repo_id.clone(),
+                name: "webhook-contract".to_string(),
+                owner_did: owner.clone(),
+                description: None,
+                is_public: true,
+                default_branch: "main".to_string(),
+                created_at: Utc::now(),
+                updated_at: Utc::now(),
+                disk_path: "/tmp/webhook-contract.git".to_string(),
+                forked_from: None,
+                machine_id: None,
+            })
+            .await
+            .unwrap();
+
+        let result = create_webhook(
+            State(state.clone()),
+            Extension(AuthenticatedDid(owner.clone())),
+            Path((owner, "webhook-contract".to_string())),
+            Json(CreateWebhookRequest {
+                url: "https://node.localhost/hook".to_string(),
+                secret: None,
+                events: None,
+            }),
+        )
+        .await;
+
+        match result {
+            Err(AppError::BadRequest(message)) => assert_eq!(
+                message,
+                format!("webhook URL {PUBLIC_HTTP_URL_REQUIREMENT}")
+            ),
+            Err(error) => panic!("unexpected webhook rejection: {error}"),
+            Ok(_) => panic!("a .localhost webhook must be rejected"),
+        }
+        assert!(
+            state.db.list_webhooks(&repo_id).await.unwrap().is_empty(),
+            "a rejected .localhost webhook must leave no row behind"
+        );
+    }
 }

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

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

crates/gitlawb-node/src/cert.rs

@@ -11,6 +11,74 @@ use uuid::Uuid;
 use crate::db::RefCertificate;
 use crate::state::AppState;
 
+/// Build the v1 signing payload — the EXACT bytes the node signs and
+/// gl's `verify_signature` reconstructs. This is the single source
+/// of truth shared by `issue_ref_certificate` and the frozen
+/// canonical-form tests below; drift here is what would break every
+/// existing cert.
+///
+/// Field order and key set are fixed: a default `serde_json::Value`
+/// serializes `Map<String, Value>` with sorted keys, so any change to
+/// the literal is observable as a different byte sequence on the
+/// wire.
+///
+/// v1 is the unversioned legacy shape: no `version` key in the signed
+/// JSON. From v2 onward the version belongs INSIDE the signed bytes
+/// (see `v2_signing_payload`): leaving it as an unsigned sibling
+/// would let a future v2 cert be downgraded to v1 and still verify
+/// cleanly, since the bytes would be identical.
+pub(crate) fn v1_signing_payload(
+    repo_id: &str,
+    ref_name: &str,
+    old_sha: &str,
+    new_sha: &str,
+    pusher_did: &str,
+    node_did: &str,
+    issued_at: &str,
+) -> serde_json::Value {
+    serde_json::json!({
+        "repo_id": repo_id,
+        "ref":     ref_name,
+        "old":     old_sha,
+        "new":     new_sha,
+        "pusher":  pusher_did,
+        "node":    node_did,
+        "ts":      issued_at,
+    })
+}
+
+/// Build the v2 signing payload — the future shape for versioned
+/// certs. Identical to v1 except the wire `version` is part of the
+/// signed bytes, so stripping or flipping the `version` column
+/// invalidates the signature instead of verifying cleanly under the
+/// other version's path.
+///
+/// NOT yet issued: `issue_ref_certificate` still stamps v1 because
+/// the shipped `gl` verifier only supports v1. This builder exists
+/// to pin the downgrade-resistant shape now, before any v2 cert is
+/// ever signed.
+#[allow(dead_code)]
+pub(crate) fn v2_signing_payload(
+    repo_id: &str,
+    ref_name: &str,
+    old_sha: &str,
+    new_sha: &str,
+    pusher_did: &str,
+    node_did: &str,
+    issued_at: &str,
+) -> serde_json::Value {
+    serde_json::json!({
+        "repo_id": repo_id,
+        "ref":     ref_name,
+        "old":     old_sha,
+        "new":     new_sha,
+        "pusher":  pusher_did,
+        "node":    node_did,
+        "ts":      issued_at,
+        "version": 2,
+    })
+}
+
 /// Issue a signed ref-update certificate for a successful push.
 ///
 /// Builds a canonical JSON payload, signs it with the node's Ed25519 key,
@@ -26,16 +94,12 @@ pub async fn issue_ref_certificate(
     let node_did = state.node_did.to_string();
     let issued_at = Utc::now().to_rfc3339();
 
-    // Build the canonical signing payload.
-    let payload = serde_json::json!({
-        "repo_id": repo_id,
-        "ref":     ref_name,
-        "old":     old_sha,
-        "new":     new_sha,
-        "pusher":  pusher_did,
-        "node":    node_did,
-        "ts":      issued_at,
-    });
+    // Build the canonical signing payload via the shared builder so
+    // the frozen-vector test below and the live signer cannot drift
+    // apart — a regression in either side fails both.
+    let payload = v1_signing_payload(
+        repo_id, ref_name, old_sha, new_sha, pusher_did, &node_did, &issued_at,
+    );
     let payload_bytes = serde_json::to_vec(&payload)?;
 
     let signature = state.node_keypair.sign_b64(&payload_bytes);
@@ -50,9 +114,351 @@ pub async fn issue_ref_certificate(
         node_did,
         signature,
         issued_at,
+        // #26 Split PR 3: the wire-format version. New certs stamp
+        // v1 — the pre-versioning 7-field payload with no `version`
+        // key in the signed JSON — because the shipped `gl`
+        // verifier only supports v1. Stamping v2 here would issue
+        // certificates the shipped client refuses (round-3
+        // finding: node pinned `version == 2` while gl pinned
+        // "2 is refused", with nothing exercising the
+        // composition). v2 is reserved for the future shape
+        // defined by `v2_signing_payload`, which carries the
+        // version INSIDE the signed bytes; land the v2 verify
+        // path before stamping v2.
+        version: 1,
     };
 
     // Persist and return the row as it exists in the database (on a
     // conflict the existing row survives when it is newer).
     state.db.insert_ref_certificate(&cert).await
 }
+
+#[cfg(test)]
+mod v1_payload_tests {
+    //! #26 Split PR 3 — cert payload version compat.
+    //!
+    //! The v1 cert payload is the frozen canonical byte form the
+    //! node signs. gl's `verify_signature` reconstructs the same
+    //! payload and verifies. Any drift in field order, key set, or
+    //! whitespace breaks every existing cert. The tests here pin
+    //! the v1 payload shape, the round-trip, the cert id
+    //! determinism per the v1 version, and the v2 read forward
+    //! compat (an unknown version refuses to verify rather than
+    //! guessing).
+    //!
+    //! Reviewer 1 finding: the live signer (`issue_ref_certificate`)
+    //! and the frozen-vector test (`v1_payload_matches_frozen_canonical_form`)
+    //! previously held two separate `serde_json::json!` literals,
+    //! so a regression in the signer could not fail this test.
+    //! Both now go through the shared `v1_signing_payload` builder.
+    use super::{v1_signing_payload, v2_signing_payload};
+    use crate::db::RefCertificate;
+    use gitlawb_core::identity::Keypair;
+
+    /// v1 payload is the same shape the legacy code signed, byte
+    /// for byte. Reverting this assertion to the v2 shape (which
+    /// includes a `version` key) is what the versioned format
+    /// forbids: an old client reading a v1 cert must not see a
+    /// `version` key in the signed JSON.
+    ///
+    /// The literal is built via the shared `v1_signing_payload`
+    /// helper that `issue_ref_certificate` also uses, so the live
+    /// signer and the frozen vector share one source of truth.
+    #[test]
+    fn v1_payload_matches_frozen_canonical_form() {
+        let payload = v1_signing_payload(
+            "repo-1",
+            "refs/heads/main",
+            &"0".repeat(40),
+            &"a".repeat(40),
+            "did:key:z6MkPusher",
+            "did:key:z6MkNode",
+            "2026-07-22T00:00:00+00:00",
+        );
+        let frozen = format!(
+            r#"{{"new":"{}","node":"did:key:z6MkNode","old":"{}","pusher":"did:key:z6MkPusher","ref":"refs/heads/main","repo_id":"repo-1","ts":"2026-07-22T00:00:00+00:00"}}"#,
+            "a".repeat(40),
+            "0".repeat(40),
+        );
+        assert_eq!(serde_json::to_string(&payload).unwrap(), frozen);
+    }
+
+    /// Round-trip: sign the v1 payload with a keypair, build a
+    /// RefCertificate with version: 1, and verify the structure.
+    /// This is the construction the live code does on every push.
+    ///
+    /// Uses the shared builder for both halves so a regression in
+    /// the live signer (adding/removing/renaming a field) cannot
+    /// pass this test by accident.
+    #[test]
+    fn v1_ref_certificate_structure_is_well_formed() {
+        let kp = Keypair::generate();
+        let node_did = kp.did().as_str().to_string();
+        let payload = v1_signing_payload(
+            "repo-1",
+            "refs/heads/main",
+            &"0".repeat(40),
+            &"a".repeat(40),
+            "did:key:z6MkPusher",
+            &node_did,
+            "2026-07-22T00:00:00+00:00",
+        );
+        let payload_bytes = serde_json::to_vec(&payload).unwrap();
+        let sig = kp.sign_b64(&payload_bytes);
+        let cert = RefCertificate {
+            id: "cert-id".into(),
+            repo_id: "repo-1".into(),
+            ref_name: "refs/heads/main".into(),
+            old_sha: "0".repeat(40),
+            new_sha: "a".repeat(40),
+            pusher_did: "did:key:z6MkPusher".into(),
+            node_did: node_did.clone(),
+            signature: sig.clone(),
+            issued_at: "2026-07-22T00:00:00+00:00".into(),
+            version: 1,
+        };
+        assert_eq!(cert.version, 1, "new certs carry version: 1");
+        // The signed payload reconstructs identically: gl's
+        // verify_signature would build the same JSON, hash the
+        // same bytes, and verify the same signature.
+        let reconstructed = v1_signing_payload(
+            &cert.repo_id,
+            &cert.ref_name,
+            &cert.old_sha,
+            &cert.new_sha,
+            &cert.pusher_did,
+            &cert.node_did,
+            &cert.issued_at,
+        );
+        let reconstructed_bytes = serde_json::to_vec(&reconstructed).unwrap();
+        assert_eq!(
+            reconstructed_bytes, payload_bytes,
+            "the round-trip serialization must be byte-identical; \
+             this is what makes gl's verify_signature succeed"
+        );
+        // Round 3 (P3 reviewer): byte-equality alone never calls
+        // `identity::verify`, so a bad signature helper would pass.
+        // Verify the computed signature against the reconstructed
+        // bytes — the same check gl performs.
+        let vk = node_did
+            .parse::<gitlawb_core::did::Did>()
+            .unwrap()
+            .to_verifying_key()
+            .unwrap();
+        use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine};
+        let sig_bytes: [u8; 64] = URL_SAFE_NO_PAD
+            .decode(&cert.signature)
+            .unwrap()
+            .try_into()
+            .unwrap();
+        gitlawb_core::identity::verify(&vk, &reconstructed_bytes, &sig_bytes)
+            .expect("well-formed v1 cert signature must verify");
+    }
+
+    /// Reviewer 1 finding: the live signer must share the
+    /// canonical-form literal so a regression in either side
+    /// fails both. This test pins the shared builder's output
+    /// against the literal a future v2 byte stream would have
+    /// to break — if someone adds a `version` key to
+    /// `v1_signing_payload`, every existing cert breaks verify,
+    /// and this test is one of the canaries that fires.
+    #[test]
+    fn v1_signing_payload_has_no_version_key() {
+        let payload = v1_signing_payload(
+            "repo-1",
+            "refs/heads/main",
+            "oldsha",
+            "newsha",
+            "did:key:z6MkPusher",
+            "did:key:z6MkNode",
+            "2026-07-22T00:00:00+00:00",
+        );
+        let obj = payload.as_object().expect("payload is a JSON object");
+        assert!(
+            !obj.contains_key("version"),
+            "v1 signing payload must not carry a `version` key — adding \
+             one changes the bytes and breaks every existing cert's \
+             signature. v2 certs build a different payload, not this one"
+        );
+        // Pin the exact set so a future field addition is caught.
+        let expected: std::collections::BTreeSet<&str> =
+            ["new", "node", "old", "pusher", "ref", "repo_id", "ts"]
+                .into_iter()
+                .collect();
+        let actual: std::collections::BTreeSet<&str> = obj.keys().map(String::as_str).collect();
+        assert_eq!(actual, expected, "v1 payload key set is frozen");
+    }
+
+    /// Round 3 (P2 reviewer): from v2 onward the version belongs
+    /// INSIDE the signed bytes. v1 stays the unversioned legacy
+    /// shape; v2 carries `"version": 2` in the payload so a
+    /// version flip without re-signing invalidates the signature
+    /// instead of verifying cleanly under the other path.
+    #[test]
+    fn v2_signing_payload_binds_version_and_resists_downgrade() {
+        let kp = Keypair::generate();
+        let node_did = kp.did().as_str().to_string();
+        let ts = "2026-07-22T00:00:00+00:00";
+        let v1 = v1_signing_payload(
+            "repo-1",
+            "refs/heads/main",
+            "oldsha",
+            "newsha",
+            "did:key:z6MkPusher",
+            &node_did,
+            ts,
+        );
+        let v2 = v2_signing_payload(
+            "repo-1",
+            "refs/heads/main",
+            "oldsha",
+            "newsha",
+            "did:key:z6MkPusher",
+            &node_did,
+            ts,
+        );
+        let v2_obj = v2.as_object().expect("v2 payload is a JSON object");
+        assert_eq!(
+            v2_obj.get("version"),
+            Some(&serde_json::json!(2)),
+            "v2 payload must carry the version inside the signed bytes"
+        );
+        let v1_bytes = serde_json::to_vec(&v1).unwrap();
+        let v2_bytes = serde_json::to_vec(&v2).unwrap();
+        assert_ne!(
+            v1_bytes, v2_bytes,
+            "v1 and v2 payloads must differ, otherwise a version flip is cryptographically invisible"
+        );
+
+        // A signature over v1 must NOT verify as v2 and vice versa.
+        let vk = node_did
+            .parse::<gitlawb_core::did::Did>()
+            .unwrap()
+            .to_verifying_key()
+            .unwrap();
+        use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine};
+        let sig_v1 = kp.sign_b64(&v1_bytes);
+        let sig_v1_bytes: [u8; 64] = URL_SAFE_NO_PAD.decode(&sig_v1).unwrap().try_into().unwrap();
+        assert!(
+            gitlawb_core::identity::verify(&vk, &v1_bytes, &sig_v1_bytes).is_ok(),
+            "v1 signature must verify under v1 bytes"
+        );
+        assert!(
+            gitlawb_core::identity::verify(&vk, &v2_bytes, &sig_v1_bytes).is_err(),
+            "a v1 signature presented as v2 (downgrade/upgrade flip) must not verify"
+        );
+    }
+
+    /// The live issuer stamps the version the shipped client can
+    /// verify. The gl client refuses to verify v2 certs explicitly;
+    /// this test pins that the version the ISSUING PATH claims
+    /// agrees with the payload shape it actually signs.
+    ///
+    /// Round 2: the prior form built a `RefCertificate` literal with
+    /// `version: 1` and asserted `version == 1` — true no matter what the
+    /// signer does, so flipping the issuer's version (or its payload shape)
+    /// left it green. This one drives the actual
+    /// `issue_ref_certificate` so a regression in the issuer flips the test.
+    /// The DB row is created against `test_support::test_state`, which
+    /// gives us a real `AppState` with a real node keypair.
+    ///
+    /// Round 3: the issuer stamps v1 because the shipped `gl`
+    /// verifier only supports v1 (stamping v2 shipped certs the
+    /// client refuses). The binding to the live issuer is kept —
+    /// flipping the stamp still turns this test red — without
+    /// changing what production issues.
+    #[sqlx::test]
+    #[allow(clippy::async_yields_async)]
+    async fn issuer_stamps_v1_over_v1_payload(pool: sqlx::PgPool) {
+        let state = crate::test_support::test_state(pool.clone()).await;
+        let repo_id = "repo-issuer-observe";
+        let ref_name = "refs/heads/main";
+        let old = "0".repeat(40);
+        let new = "a".repeat(40);
+        let pusher = "did:key:z6MkPusher";
+
+        // Drive the live issuer and read the persisted row back by
+        // id, so the test covers the stored `version` column and
+        // not just the in-process return value.
+        let cert =
+            crate::cert::issue_ref_certificate(&state, repo_id, ref_name, &old, &new, pusher)
+                .await
+                .expect("issue_ref_certificate must succeed");
+
+        // The cert returned by the issuer is the source of truth
+        // for both the version claim and the signed bytes.
+        assert_eq!(cert.repo_id, repo_id);
+        assert_eq!(cert.ref_name, ref_name);
+
+        // Cross-check 1: the issuer's claimed version must match
+        // what the live function stamps. This assertion is bound
+        // to the live function, not a hand-built literal:
+        // flipping the stamp in `issue_ref_certificate` breaks it
+        // through the actual call path.
+        assert_eq!(
+            cert.version, 1,
+            "the live issuer must stamp v1 until a v2 verifier ships; \
+             flipping the stamp breaks this assertion through the actual call path"
+        );
+
+        // Cross-check 1b: the persisted row carries the same stamp.
+        let stored = state
+            .db
+            .get_ref_certificate(&cert.id)
+            .await
+            .expect("persisted cert must be readable")
+            .expect("persisted cert must exist");
+        assert_eq!(
+            stored.version, cert.version,
+            "the stored version column must match the issued claim"
+        );
+
+        // Cross-check 2: the signature on the cert must verify
+        // against the v1 payload builder's bytes, not against some
+        // other shape the issuer might have introduced. This is
+        // what gl's `verify_signature` does for a v1 cert: rebuild
+        // the payload from the cert's own fields and verify the
+        // signature. If the issuer ever signs a different shape
+        // while still claiming v1, every shipped client breaks —
+        // and so does this.
+        let rebuilt = v1_signing_payload(
+            &cert.repo_id,
+            &cert.ref_name,
+            &cert.old_sha,
+            &cert.new_sha,
+            &cert.pusher_did,
+            &cert.node_did,
+            &cert.issued_at,
+        );
+        let vk = cert
+            .node_did
+            .parse::<gitlawb_core::did::Did>()
+            .unwrap()
+            .to_verifying_key()
+            .unwrap();
+        use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine};
+        let sig_bytes: [u8; 64] = URL_SAFE_NO_PAD
+            .decode(&cert.signature)
+            .unwrap()
+            .try_into()
+            .unwrap();
+        gitlawb_core::identity::verify(&vk, &serde_json::to_vec(&rebuilt).unwrap(), &sig_bytes)
+            .expect("a v1 cert's signature must verify against the v1 payload shape");
+
+        // Cross-check 3: the v1 signed payload must NOT contain a
+        // version key — v2 is the future shape that adds one inside
+        // the signed bytes (see `v2_signing_payload`). The v1
+        // payload (which the cert is signed over) is frozen at 7
+        // fields. A change that adds `version` to the v1 payload
+        // breaks every existing cert and this test.
+        let obj = rebuilt
+            .as_object()
+            .expect("rebuilt v1 payload is a JSON object");
+        assert!(
+            !obj.contains_key("version"),
+            "the v1 signing payload must not carry a version key — \
+             adding one changes the bytes and breaks every existing \
+             cert's signature"
+        );
+    }
+}

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/git/issues.rs

@@ -141,19 +141,10 @@ pub fn resolve_issue_id(repo_path: &Path, id_or_prefix: &str) -> Result<Option<S
         return Ok(None);
     }
 
-    // Try exact match first — fast path for callers passing the full UUID.
+    // List all refs that start with the given string; the glob also matches the exact
+    // ref. The id handed back is always git's own refname, never the request input.
     let exact_ref = format!("refs/gitlawb/issues/{id_or_prefix}");
-    let check = Command::new("git")
-        .args(["cat-file", "-e", &exact_ref])
-        .current_dir(repo_path)
-        .output()
-        .context("failed to run git cat-file -e")?;
-    if check.status.success() {
-        return Ok(Some(id_or_prefix.to_string()));
-    }
-
-    // Prefix search: list all refs that start with the given string.
-    let prefix_glob = format!("refs/gitlawb/issues/{id_or_prefix}*");
+    let prefix_glob = format!("{exact_ref}*");
     let list = Command::new("git")
         .args(["for-each-ref", "--format=%(refname)", &prefix_glob])
         .current_dir(repo_path)
@@ -165,25 +156,44 @@ pub fn resolve_issue_id(repo_path: &Path, id_or_prefix: &str) -> Result<Option<S
     }
 
     let output = String::from_utf8_lossy(&list.stdout);
-    let matches: Vec<&str> = output.lines().filter(|l| !l.trim().is_empty()).collect();
+    let matches: Vec<&str> = output
+        .lines()
+        .map(str::trim)
+        .filter(|l| !l.is_empty())
+        .collect();
 
-    match matches.len() {
-        0 => Ok(None),
-        1 => {
-            // Strip the "refs/gitlawb/issues/" prefix to get the bare ID.
-            let full_id = matches[0]
-                .trim()
-                .strip_prefix("refs/gitlawb/issues/")
-                .unwrap_or(matches[0].trim())
-                .to_string();
-            Ok(Some(full_id))
+    // An exact id wins over longer ids that share it as a prefix, but only if its
+    // object exists: for-each-ref also lists a ref whose blob is missing, so a
+    // dangling exact ref falls through to the prefix rules below.
+    let exact = match matches.iter().find(|m| **m == exact_ref) {
+        Some(m) => {
+            let check = Command::new("git")
+                .args(["cat-file", "-e", m])
+                .current_dir(repo_path)
+                .output()
+                .context("failed to run git cat-file -e")?;
+            check.status.success().then_some(*m)
         }
-        _ => anyhow::bail!(
-            "ambiguous issue prefix '{}': matches {} issues",
-            id_or_prefix,
-            matches.len()
-        ),
-    }
+        None => None,
+    };
+    let found = match exact {
+        Some(m) => m,
+        None => match matches.as_slice() {
+            [] => return Ok(None),
+            [m] => *m,
+            _ => anyhow::bail!(
+                "ambiguous issue prefix '{}': matches {} issues",
+                id_or_prefix,
+                matches.len()
+            ),
+        },
+    };
+    // Strip the "refs/gitlawb/issues/" prefix to get the bare ID.
+    let full_id = found
+        .strip_prefix("refs/gitlawb/issues/")
+        .unwrap_or(found)
+        .to_string();
+    Ok(Some(full_id))
 }
 
 /// Close an issue by updating its status to "closed" in the git ref.
@@ -267,6 +277,43 @@ mod tests {
         assert_eq!(resolved, Some(full_id.to_string()));
     }
 
+    #[test]
+    fn test_resolve_exact_id_wins_over_longer_id() {
+        let dir = TempDir::new().unwrap();
+        init_repo(&dir);
+        create_issue(dir.path(), "abc12345", r#"{"status":"open"}"#).unwrap();
+        create_issue(dir.path(), "abc12345-0000", r#"{"status":"open"}"#).unwrap();
+        let resolved = resolve_issue_id(dir.path(), "abc12345").unwrap();
+        assert_eq!(resolved, Some("abc12345".to_string()));
+    }
+
+    #[test]
+    fn test_resolve_dangling_exact_id_does_not_win_over_longer_id() {
+        let dir = TempDir::new().unwrap();
+        init_repo(&dir);
+        create_issue(dir.path(), "abc12345", r#"{"status":"open","n":1}"#).unwrap();
+        create_issue(dir.path(), "abc12345-0000", r#"{"status":"open","n":2}"#).unwrap();
+        // Dangle the exact ref: the ref stays listed by for-each-ref, but its blob is gone.
+        let out = Command::new("git")
+            .args(["rev-parse", "refs/gitlawb/issues/abc12345"])
+            .current_dir(dir.path())
+            .output()
+            .unwrap();
+        let hash = String::from_utf8(out.stdout).unwrap();
+        let hash = hash.trim();
+        std::fs::remove_file(
+            dir.path()
+                .join(".git/objects")
+                .join(&hash[..2])
+                .join(&hash[2..]),
+        )
+        .unwrap();
+
+        let result = resolve_issue_id(dir.path(), "abc12345");
+        assert!(result.unwrap_err().to_string().contains("ambiguous"));
+        assert!(close_issue(dir.path(), "abc12345").is_err());
+    }
+
     #[test]
     fn test_resolve_prefix_matches_unique() {
         let dir = TempDir::new().unwrap();

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/main.rs

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

crates/gitlawb-node/src/metrics.rs

@@ -314,7 +314,9 @@ pub fn record_sync_processed(status: &str) {
 }
 
 /// Record one webhook delivery attempt outcome. `result` ∈
-/// {ok, http_error, network_error, skipped}.
+/// {ok, http_error, network_error, skipped, invalid_url}.
+/// `invalid_url` is a policy skip: the stored hook URL failed the public-URL
+/// check at delivery time and the POST was never sent.
 pub fn record_webhook_delivery(result: &str) {
     if let Some(c) = WEBHOOK_DELIVERIES.get() {
         c.with_label_values(&[result]).inc();

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

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

crates/gitlawb-node/src/sync.rs

@@ -701,6 +701,7 @@ async fn clone_repo(remote_url: &str, local_path: &Path, mode: MirrorMode) -> an
     if mode == MirrorMode::Promisor {
         args.push("--filter=blob:limit=10g");
     }
+    args.push("--");
     args.push(remote_url);
     args.push(local_str);
 
@@ -729,7 +730,10 @@ async fn clone_repo(remote_url: &str, local_path: &Path, mode: MirrorMode) -> an
 async fn fetch_repo(local_path: &Path, remote_url: &str, mode: MirrorMode) -> anyhow::Result<()> {
     let local_str = local_path.to_str().unwrap_or(".");
 
-    git_run(&["-C", local_str, "remote", "set-url", "origin", remote_url]).await?;
+    git_run(&[
+        "-C", local_str, "remote", "set-url", "origin", "--", remote_url,
+    ])
+    .await?;
 
     match mode {
         MirrorMode::Promisor => {
@@ -1006,6 +1010,82 @@ mod tests {
         assert_eq!(probe, PromisorProbe::NotPromisor);
     }
 
+    /// #374: `clone_repo` interpolates the peer's origin URL into a `git
+    /// clone --mirror` argument list. Without a `--` delimiter git parses a
+    /// remote beginning with `-` as an option instead of a repository.
+    ///
+    /// The delimiter is what forces the same string to be a repository, and
+    /// git says so in its own words: it reports the whole argument as a
+    /// repository it cannot find. Undelimited, git consumes it as an option
+    /// and fails against the *destination* instead (`fatal: repository '<dest>' does not exist`),
+    /// never naming the injected string — which is why this assertion is RED
+    /// before the fix and GREEN after.
+    ///
+    /// The payload is deliberately free of `:` and spaces: a colon would make
+    /// git read the argument as an ssh-style `host:path` URL and report a
+    /// hostname rather than a repository, which is a different code path.
+    #[tokio::test]
+    async fn clone_never_parses_a_remote_as_a_git_option() {
+        let td = TempDir::new().unwrap();
+        let dest = td.path().join("mirror.git");
+        let injected = "--upload-pack=false";
+
+        let err = clone_repo(injected, &dest, MirrorMode::Plain)
+            .await
+            .expect_err("a remote that is not a repository must fail the clone")
+            .to_string();
+
+        assert!(
+            err.contains(injected),
+            "git must report the argument as a repository, not consume it as an option: {err}"
+        );
+    }
+
+    /// #374: `fetch_repo` interpolates the peer's origin URL into
+    /// `git remote set-url`, a second argv construction with the same contract as
+    /// the clone. It needs its own proof: the clone test cannot reach this path.
+    ///
+    /// The assertion is on stored state rather than on the error string, and that
+    /// is deliberate. `git_run` formats its failure as `git {args:?} failed:
+    /// {stderr}`, so the injected value is in the message via the debug-printed
+    /// argv whether or not the delimiter is present — asserting on containment
+    /// would pass with `--` deleted. Delimited, `set-url` succeeds and stores the
+    /// value verbatim; undelimited, git exits 129 with `unknown option` and leaves
+    /// the previous URL in place, so this goes red without the fix.
+    ///
+    /// The fetch that follows the rewrite fails, because the stored URL is not a
+    /// repository. That is expected and ignored: the property under test is what
+    /// `set-url` wrote, not whether the subsequent fetch could succeed.
+    #[tokio::test]
+    async fn fetch_never_parses_a_remote_as_a_git_option() {
+        let td = TempDir::new().unwrap();
+        let local = td.path().join("mirror.git");
+        let local_str = local.to_str().unwrap();
+        let placeholder = "http://example.invalid/x.git";
+
+        assert!(Command::new("git")
+            .args(["init", "-q", "--bare", local_str])
+            .status()
+            .unwrap()
+            .success());
+        assert!(Command::new("git")
+            .args(["-C", local_str, "remote", "add", "origin", placeholder])
+            .status()
+            .unwrap()
+            .success());
+
+        let injected = "--upload-pack=false";
+        let _ = fetch_repo(&local, injected, MirrorMode::Plain).await;
+
+        assert_eq!(
+            git_config_get(local_str, "remote.origin.url")
+                .await
+                .as_deref(),
+            Some(injected),
+            "set-url must store an option-shaped remote verbatim, not reject it as an option"
+        );
+    }
+
     #[tokio::test]
     async fn probe_reports_unknown_on_git_error() {
         // A path git cannot resolve as a repo at all (exit 128) is an indeterminate

crates/gitlawb-node/src/webhooks.rs

@@ -79,6 +79,19 @@ async fn fire_event_async(
         let delivery_id = uuid::Uuid::new_v4().to_string();
 
         tokio::spawn(async move {
+            // Policy skip, not a delivery failure: the stored URL predates the
+            // create-time gate. The "invalid_url" label is documented on
+            // record_webhook_delivery in metrics.rs.
+            if !crate::api::peers::is_public_http_url(&hook.url) {
+                crate::metrics::record_webhook_delivery("invalid_url");
+                tracing::warn!(
+                    url = %hook.url,
+                    event = %event_name,
+                    "webhook delivery skipped: stored URL is not a public http(s) target"
+                );
+                return;
+            }
+
             let signature = hook.secret.as_deref().map(|s| sign_payload(s, &bytes));
 
             let mut req = client

crates/gitlawb-node/tests/owner_push_default_docs.rs

@@ -0,0 +1,177 @@
+//! #26 Split PR 4 — `GITLAWB_ENFORCE_OWNER_PUSH` default is consistent across surfaces.
+//!
+//! Reviewer 2 (2026-08-28) closed PR #224 with a P2 finding: SECURITY.md and
+//! README.md both stated the default was `false`, while `Config::enforce_owner_push`
+//! declared `default_value_t = true` and the runtime, .env.example, and the env
+//! var table on the README all agreed with the code. Two operator-facing
+//! surfaces lied about the authorization policy. PR #330 flipped the code
+//! default and updated .env.example / README env table / docs/RUN-A-NODE.md,
+//! but missed these two prose mentions.
+//!
+//! This guard pins the documentation and example surfaces together. Each one
+//! is read off the working tree at test time, not a copy in the binary, so a
+//! revert of the doc edit turns it red:
+//!
+//! 1. `SECURITY.md` says the default is `true`.
+//! 2. `README.md` says the default is `true` in the known-limitations prose.
+//! 3. `README.md` says the default is `true` in the env-var table row.
+//! 4. `.env.example` sets `GITLAWB_ENFORCE_OWNER_PUSH=true`.
+//!
+//! The runtime declared default (`Config::enforce_owner_push`'s
+//! `default_value_t = true`) is protected by the
+//! `enforce_owner_push_is_declared_true_independent_of_the_environment`
+//! unit test in `config.rs`, which reads the parser directly and is
+//! env-independent. The two checks together cover the surfaces that
+//! produced the original doc-vs-code drift; this guard cannot reach into
+//! the binary crate's `Config` type (it is private to the bin target,
+//! which has no `lib.rs`) without a larger refactor, so the contract is
+//! stated as the four documentation/example surfaces above.
+//!
+//! The four are checked independently. A check that just verified "at least
+//! one surface says true" would pass while a stale SECURITY.md lied; a check
+//! that only verified the binary would not catch the doc drift that
+//! produced this finding in the first place.
+
+use std::path::Path;
+
+fn read(rel: &str) -> String {
+    let p = Path::new(env!("CARGO_MANIFEST_DIR"))
+        .join("..")
+        .join("..")
+        .join(rel);
+    std::fs::read_to_string(&p).unwrap_or_else(|e| panic!("read {}: {e}", p.display()))
+}
+
+/// Pin the prose mentions of `GITLAWB_ENFORCE_OWNER_PUSH` so a future doc
+/// edit cannot reintroduce the "defaults to false" wording the reviewer
+/// flagged. The wording in the test is what the docs MUST say about the
+/// default; absence of any prose mention is also a fail because operators
+/// rely on the docs to learn the default.
+#[test]
+fn security_md_states_owner_push_default_is_true() {
+    let security = read("SECURITY.md");
+    let section = section_after(&security, "### Repository write authorization defaults")
+        .unwrap_or_else(|| panic!("SECURITY.md is missing the owner-push section"));
+
+    assert!(
+        section.contains("defaults to `true`"),
+        "SECURITY.md owner-push section must state the default is true. \
+         Found:\n{section}"
+    );
+    assert!(
+        !section_claims_default_is_false(&section),
+        "SECURITY.md owner-push section must not claim the default is false. \
+         Found:\n{section}"
+    );
+}
+
+#[test]
+fn readme_known_limitations_states_owner_push_default_is_true() {
+    let readme = read("README.md");
+    let limitations = section_after(&readme, "Known limitations:")
+        .unwrap_or_else(|| panic!("README.md is missing the Known limitations: section"));
+
+    // The owner-push line is one bullet in the limitations list. Pull the
+    // single line that mentions the env var so the assertion is local to
+    // the claim, not a substring of unrelated text.
+    let line = limitations
+        .lines()
+        .find(|l| l.contains("GITLAWB_ENFORCE_OWNER_PUSH"))
+        .unwrap_or_else(|| {
+            panic!(
+                "README.md Known limitations must mention GITLAWB_ENFORCE_OWNER_PUSH.\n\
+                 Got:\n{limitations}"
+            )
+        });
+
+    assert!(
+        line.contains("defaults to `true`") || line.contains("defaults to true"),
+        "README Known-limitations owner-push bullet must state the default is true. \
+         Found:\n{line}"
+    );
+    assert!(
+        !section_claims_default_is_false(line),
+        "README Known-limitations owner-push bullet must not claim the default is false. \
+         Found:\n{line}"
+    );
+}
+
+#[test]
+fn readme_env_var_table_states_owner_push_default_is_true() {
+    let readme = read("README.md");
+    let line = readme
+        .lines()
+        .find(|l| l.contains("GITLAWB_ENFORCE_OWNER_PUSH") && l.contains("|"))
+        .unwrap_or_else(|| {
+            panic!("README.md is missing the env-var-table row for GITLAWB_ENFORCE_OWNER_PUSH")
+        });
+
+    assert!(
+        line.contains("Defaults to `true`") || line.contains("defaults to `true`"),
+        "README env-var-table row for GITLAWB_ENFORCE_OWNER_PUSH must state the default is true. \
+         Found:\n{line}"
+    );
+    assert!(
+        !section_claims_default_is_false(line),
+        "README env-var-table row for GITLAWB_ENFORCE_OWNER_PUSH must not claim the default is false. \
+         Found:\n{line}"
+    );
+}
+
+#[test]
+fn env_example_sets_owner_push_true() {
+    let env = read(".env.example");
+    let line = env
+        .lines()
+        .find(|l| l.trim_start().starts_with("GITLAWB_ENFORCE_OWNER_PUSH"))
+        .unwrap_or_else(|| panic!(".env.example is missing GITLAWB_ENFORCE_OWNER_PUSH"));
+
+    assert!(
+        line.contains("=true"),
+        ".env.example GITLAWB_ENFORCE_OWNER_PUSH must be set to true. Found:\n{line}"
+    );
+}
+
+/// A claim that the default is false can hide in many phrasings. Match the
+/// forms that have appeared in the wild on this branch: "defaults to `false`",
+/// "defaults to false", "default is false", "is `false` for compatibility",
+/// "is false for compatibility". The match is intentionally narrow — false
+/// positives would suppress a real doc edit.
+fn section_claims_default_is_false(section: &str) -> bool {
+    let lower = section.to_ascii_lowercase();
+    lower.contains("defaults to `false`")
+        || lower.contains("defaults to false")
+        || lower.contains("default is `false`")
+        || lower.contains("default is false")
+        || (lower.contains("`false`") && lower.contains("for compatibility"))
+}
+
+/// Return the prose after a marker up to the next heading boundary. The
+/// marker is normally a markdown heading (e.g. `### Section`); if it has
+/// no `#` prefix it is treated as a plain-text marker and the slice is
+/// bounded by the next heading of any level. This matters because a
+/// plain-text marker would otherwise produce `header_level = 0`, making
+/// the same-or-higher-level stop condition unsatisfiable and letting the
+/// slice run to EOF.
+fn section_after(doc: &str, heading: &str) -> Option<String> {
+    let start = doc.find(heading)?;
+    let after = &doc[start..];
+    let header_level = heading.chars().take_while(|c| *c == '#').count();
+
+    // Skip past the heading line itself.
+    let body_start = after.find('\n')? + 1;
+    let body = &after[body_start..];
+
+    // Stop at the next heading of the same or higher level, or at the next
+    // heading of any level when the marker had no `#` prefix.
+    let mut end = body.len();
+    for line in body.lines() {
+        let level = line.chars().take_while(|c| *c == '#').count();
+        if level > 0 && (header_level == 0 || level <= header_level) {
+            end = line.as_ptr() as usize - body.as_ptr() as usize;
+            break;
+        }
+    }
+
+    Some(body[..end].to_string())
+}

crates/gl/src/agent.rs

@@ -88,7 +88,7 @@ async fn cmd_list(node: String, capability: Option<String>) -> Result<()> {
         let short = did
             .split(':')
             .next_back()
-            .map(|s| &s[..s.len().min(16)])
+            .map(|s| crate::text::truncate(s, 16))
             .unwrap_or("?");
         let trust = agent["trust_score"].as_f64().unwrap_or(0.0);
         let caps = agent["capabilities"]

crates/gl/src/bounty.rs

@@ -275,7 +275,7 @@ async fn cmd_list(
                 let title = b["title"].as_str().unwrap_or("?");
                 let amount = b["amount"].as_i64().unwrap_or(0);
                 let st = b["status"].as_str().unwrap_or("?");
-                let short_id = &id[..8.min(id.len())];
+                let short_id = crate::text::truncate(id, 8);
                 println!("{short_id}  {st:<10}  {amount:>12} $GITLAWB  {title}");
             }
         }
@@ -403,7 +403,7 @@ async fn cmd_stats(node: String) -> Result<()> {
                 let did = entry["did"].as_str().unwrap_or("?");
                 let count = entry["completed"].as_i64().unwrap_or(0);
                 let earned = entry["total_earned"].as_i64().unwrap_or(0);
-                let short = &did[did.len().saturating_sub(8)..];
+                let short = crate::text::truncate_suffix(did, 8);
                 println!(
                     "  {}. ...{short}  {count} bounties  {earned} $GITLAWB",
                     i + 1

crates/gl/src/cert.rs

@@ -4,6 +4,7 @@
 
 use anyhow::{Context, Result};
 use clap::{Args, Subcommand};
+use serde_json::Value;
 use std::path::PathBuf;
 
 use crate::http::NodeClient;
@@ -34,7 +35,12 @@ pub enum CertCmd {
         #[arg(long)]
         dir: Option<PathBuf>,
     },
-    /// Show a specific ref certificate and verify its signature
+    /// Show a specific ref certificate and verify its signature.
+    ///
+    /// `--verify` requires `--expect-node <did>`: the queried node's
+    /// self-reported DID is not a trust anchor, because a node serving a
+    /// forged certificate also controls its own self-report. Callers
+    /// must name the issuer they trust.
     Show {
         /// Repository in <owner>/<repo> or <repo> format
         repo: String,
@@ -49,13 +55,17 @@ pub enum CertCmd {
         #[arg(long)]
         dir: Option<PathBuf>,
         /// Exit non-zero unless the Ed25519 signature verifies AND the
-        /// issuing node matches the queried node (or --expect-node)
+        /// issuing node matches --expect-node (which --verify requires:
+        /// without a caller-supplied anchor there is no trusted issuer to
+        /// verify against)
         #[arg(long)]
         verify: bool,
         /// Expected issuing node DID for --verify. A valid signature alone
         /// only proves the cert is internally consistent — signed by whatever
         /// key it names — so --verify also anchors the issuer to a DID you
-        /// trust: this value when given, else the queried node's DID.
+        /// trust. The queried node's self-reported DID is deliberately NOT
+        /// accepted as a fallback anchor: the node that served the cert can
+        /// serve a matching self-report.
         #[arg(long, requires = "verify")]
         expect_node: Option<String>,
     },
@@ -117,7 +127,10 @@ async fn cmd_list(repo: String, node: String, dir: Option<PathBuf>) -> Result<()
         let id = cert["id"].as_str().unwrap_or("?");
         let ref_name = cert["ref_name"].as_str().unwrap_or("?");
         let new_sha = cert["new_sha"].as_str().unwrap_or("?");
-        let issued_at = cert["issued_at"].as_str().map(|s| &s[..19]).unwrap_or("?");
+        let issued_at = cert["issued_at"]
+            .as_str()
+            .map(|s| crate::text::truncate(s, 19))
+            .unwrap_or("?");
         println!("  {id:.8}  {issued_at}  {ref_name}  {new_sha:.12}");
     }
     Ok(())
@@ -149,6 +162,24 @@ async fn cmd_show(
     let node_did = cert["node_did"].as_str().unwrap_or("?");
     let signature = cert["signature"].as_str().unwrap_or("?");
     let issued_at = cert["issued_at"].as_str().unwrap_or("?");
+    // #26 Split PR 3: read the wire-format version. A MISSING key
+    // selects legacy v1 (an old server that predates the field).
+    // A PRESENT value must be a JSON integer that fits in u32 and
+    // names a version this client supports — otherwise the response
+    // declares a format we cannot represent and we refuse to verify
+    // rather than guess the payload shape. Reviewer 2 finding:
+    // collapsing `null`, a string, a float, or an overflow integer
+    // onto v1 would let a well-signed v1 signature pass `--verify`
+    // against a server that explicitly said "version 2 (or 99, or
+    // 4294967297)" — that is the exact mismatch the field exists
+    // to prevent. parse_cert_version is the load-bearing parser
+    // that distinguishes missing-key (legacy v1) from invalid-value
+    // (unsupported).
+    let parsed_version = parse_cert_version(cert.get("version"));
+    let version_display: String = match &parsed_version {
+        Ok(v) => v.to_string(),
+        Err(reason) => format!("unsupported ({reason})"),
+    };
 
     println!("Ref Certificate: {cert_id}");
     println!("  Ref:       {ref_name}");
@@ -157,6 +188,7 @@ async fn cmd_show(
     println!("  Pusher:    {pusher}");
     println!("  Node DID:  {node_did}");
     println!("  Issued at: {issued_at}");
+    println!("  Version:   {version_display}");
     println!("  Signature: {signature}");
     println!();
 
@@ -166,9 +198,31 @@ async fn cmd_show(
     // This proves the cert is internally authentic — signed by the key it
     // names; the node-DID comparison below covers *which* node that is.
     let repo_id = cert["repo_id"].as_str().unwrap_or("");
-    let verdict = verify_signature(
-        repo_id, ref_name, old_sha, new_sha, pusher, node_did, issued_at, signature,
-    );
+    let verdict = match parsed_version {
+        // v1 is the only version this client verifies. The v1
+        // signed payload is the 7-field canonical form with no
+        // `version` key — see gitlawb-node/src/cert.rs. A future
+        // v2+ cert has a different signed payload shape (the
+        // version field becomes part of the signed JSON per
+        // `v2_signing_payload`), so a v2+ cert from a server this
+        // client does not know about must NOT be silently verified
+        // as v1.
+        // Reviewer 2: refuse rather than guess; the client and
+        // server must agree on the version.
+        // parse_cert_version admits exactly one Ok value: 1 (missing key and
+        // integer 1 both land there; 2, floats, strings, overflow and every
+        // other spelling come back Err with their own reason). A separate
+        // `Ok(v != 1)` arm existed here and was DEAD CODE — a test aimed at it
+        // could never catch drift in this match (review round 2) — so the
+        // match now mirrors the parser's real contract, which the
+        // `parse_cert_version_truth_table` test pins.
+        Ok(_) => verify_signature(
+            repo_id, ref_name, old_sha, new_sha, pusher, node_did, issued_at, signature,
+        ),
+        Err(ref reason) => Err(format!(
+            "cert declared a version this client cannot represent ({reason}); refusing to verify"
+        )),
+    };
 
     println!("Signature verification:");
     match &verdict {
@@ -178,7 +232,11 @@ async fn cmd_show(
             );
         }
         Err(reason) => {
-            println!("  INVALID — {reason}");
+            // A version refusal means no cryptographic check ran, so it must
+            // not print as an INVALID signature: operators and log scrapers
+            // would read that as a failed Ed25519 verification. INVALID is
+            // reserved for failures out of verify_signature itself.
+            println!("  {} — {reason}", signature_verdict_label(&parsed_version));
         }
     }
 
@@ -195,29 +253,39 @@ async fn cmd_show(
         },
         Err(e) => Err(e.to_string()),
     };
-    let current_node_did = current.as_ref().ok().cloned();
     for line in did_check_report(&current, node_did) {
         println!("{line}");
     }
 
     if require_valid {
         if let Err(reason) = verdict {
-            anyhow::bail!("certificate signature did not verify: {reason}");
+            // Round 3: do not claim the signature failed when it was
+            // never checked. A version refusal means the payload
+            // shape is unknown, so no signature verification ran.
+            anyhow::bail!("certificate could not be verified: {reason}");
         }
         // A valid signature proves internal consistency only: the payload was
         // signed by whatever key the certificate itself names. A hostile
         // source can mint a keypair, put its DID in node_did, and self-sign.
-        // --verify therefore also anchors the issuer to a trusted DID:
-        // --expect-node when given, else the DID of the node being queried.
-        let expected = expect_node.as_deref().or(current_node_did.as_deref());
-        match expected {
+        // --verify therefore also anchors the issuer — and the anchor must be
+        // a DID the CALLER trusts, which is exactly what --expect-node is.
+        // The DID the queried node reports at `/` is NOT an anchor: the same
+        // node that served a forged certificate can serve a matching
+        // self-report, and with that fallback a wholly forged cert printed
+        // VALID and exited 0 (review round 2, demonstrated by execution). So
+        // --verify without --expect-node refuses to claim a trusted issuer at
+        // all, rather than laundering the server's self-assertion into one.
+        match expect_node.as_deref() {
             Some(expected) if expected == node_did => {}
             Some(expected) => anyhow::bail!(
                 "certificate is signed by {node_did}, but the expected issuer is {expected} — \
                  a valid signature alone proves internal consistency, not a trusted issuer"
             ),
             None => anyhow::bail!(
-                "cannot anchor the issuer: node info is unreachable and no --expect-node was given"
+                "--verify needs --expect-node <did> to anchor the issuer: the signature is \
+                 valid, but it only proves the cert is self-consistent with the DID it \
+                 names ({node_did}); the queried node's self-reported DID is not a trust \
+                 anchor"
             ),
         }
     }
@@ -265,6 +333,18 @@ fn did_check_report(current: &std::result::Result<String, String>, node_did: &st
     }
 }
 
+/// The stdout label for a failed verification verdict. A `parsed_version`
+/// error means `verify_signature` never ran, so the refusal must print as
+/// UNSUPPORTED VERSION rather than INVALID, which operators and log
+/// scrapers would read as a failed Ed25519 check.
+fn signature_verdict_label(parsed_version: &std::result::Result<u32, String>) -> &'static str {
+    if parsed_version.is_err() {
+        "UNSUPPORTED VERSION"
+    } else {
+        "INVALID"
+    }
+}
+
 /// Rebuild the node's canonical signing payload (field order must match
 /// gitlawb-node/src/cert.rs::issue_ref_certificate exactly) and verify the
 /// certificate's Ed25519 signature against the key embedded in `node_did`.
@@ -311,6 +391,65 @@ fn verify_signature(
         .map_err(|_| "Ed25519 signature does not match the signed payload".to_string())
 }
 
+/// Parse the wire-format `version` field of a certificate response.
+///
+/// Semantics (Reviewer 2):
+///   - MISSING key (`None`) → `Ok(1)`. An old server predates the field, so
+///     the cert is by definition the v1 (pre-versioning) shape; the v1
+///     verify path is the only path that can possibly match the bytes.
+///   - PRESENT key must be a JSON integer that fits losslessly in `u32`
+///     and equals a version this client supports. `null`, a string,
+///     a float, an overflow integer, or any version other than 1 all
+///     yield `Err`. Collapsing any of these onto v1 would let a
+///     well-signed v1 signature pass `--verify` against a response
+///     that explicitly declared a format this client cannot represent
+///     — which is exactly the mismatch the version field exists to
+///     prevent.
+///
+/// The supported-version set is hard-coded to `{1}` because PR 3 ships
+/// only v1 verification; bump this set (and the verdict branch) when
+/// v2 verification lands.
+fn parse_cert_version(value: Option<&Value>) -> Result<u32, String> {
+    let v = match value {
+        None => return Ok(1),
+        Some(v) => v,
+    };
+
+    // serde_json::Value::as_u64 rejects strings, floats, booleans,
+    // nulls, arrays, and objects — but it silently truncates floats
+    // that are whole numbers (e.g. 2.0 → 2). We must reject floats
+    // explicitly so a server cannot smuggle a v2 cert through the
+    // float shape.
+    if v.is_f64() {
+        return Err(format!(
+            "version is a JSON number with a fractional part ({v}); expected an integer"
+        ));
+    }
+
+    let n = v.as_u64().ok_or_else(|| {
+        // null / string / bool / array / object — anything that is
+        // not a JSON integer.
+        format!("version is {v}; expected a JSON integer")
+    })?;
+
+    // Lossy narrowing: refuse anything that does not fit in u32.
+    // u32::MAX is 4_294_967_295; serde_json only goes up to u64.
+    if n > u32::MAX as u64 {
+        return Err(format!(
+            "version {n} does not fit in u32 (max {})",
+            u32::MAX
+        ));
+    }
+
+    let n = n as u32;
+    if n != 1 {
+        return Err(format!(
+            "this client supports cert version 1 only; server returned {n}"
+        ));
+    }
+    Ok(1)
+}
+
 async fn resolve_cert_id(client: &NodeClient, owner: &str, name: &str, id: &str) -> Result<String> {
     if id.len() >= 36 {
         return Ok(id.to_string());
@@ -808,7 +947,15 @@ mod tests {
         let mut server = mockito::Server::new_async().await;
         let (cert, _root) = cert_server(&mut server, &id, &body, Some(&info)).await;
 
-        let got = cmd_show("alice/r".to_string(), id, server.url(), None, true, None).await;
+        let got = cmd_show(
+            "alice/r".to_string(),
+            id,
+            server.url(),
+            None,
+            true,
+            Some(kp.did().as_str().to_string()),
+        )
+        .await;
         assert!(got.is_ok(), "valid sig + matching issuer must pass");
         cert.assert_async().await;
     }
@@ -824,10 +971,17 @@ mod tests {
         let mut server = mockito::Server::new_async().await;
         let (cert, _root) = cert_server(&mut server, &id, &body, Some(&info)).await;
 
-        let err = cmd_show("alice/r".to_string(), id, server.url(), None, true, None)
-            .await
-            .unwrap_err()
-            .to_string();
+        let err = cmd_show(
+            "alice/r".to_string(),
+            id,
+            server.url(),
+            None,
+            true,
+            Some(other.did().as_str().to_string()),
+        )
+        .await
+        .unwrap_err()
+        .to_string();
         assert!(err.contains("expected issuer"), "got: {err}");
         cert.assert_async().await;
     }
@@ -845,7 +999,7 @@ mod tests {
             .await
             .unwrap_err()
             .to_string();
-        assert!(err.contains("cannot anchor the issuer"), "got: {err}");
+        assert!(err.contains("--expect-node"), "got: {err}");
         cert.assert_async().await;
     }
 
@@ -886,7 +1040,7 @@ mod tests {
             .await
             .unwrap_err()
             .to_string();
-        assert!(err.contains("signature did not verify"), "got: {err}");
+        assert!(err.contains("does not match"), "got: {err}");
         cert.assert_async().await;
     }
 
@@ -970,4 +1124,373 @@ mod tests {
         assert!(report.contains("verify offline"), "got: {report}");
         assert!(!report.contains("WARNING"), "got: {report}");
     }
+
+    /// #26 Split PR 3: a missing `version` field on a cert defaults
+    /// to 1, so an old server's response is forward-compatible with
+    /// a new client. The v1 verify path is taken, and a
+    /// well-signed v1 cert round-trips through `verify_signature`.
+    ///
+    /// Reviewer 1 finding: the previous shape only parsed the field
+    /// and never called `verify_signature`, so the test could not
+    /// fail if the verdict branch was wired to the wrong path. This
+    /// version signs and verifies the full round-trip.
+    #[test]
+    fn missing_version_defaults_to_1_and_verifies() {
+        let kp = gitlawb_core::identity::Keypair::generate();
+        let node_did = kp.did().as_str().to_string();
+        let payload = serde_json::json!({
+            "repo_id": "repo-1",
+            "ref":     "refs/heads/main",
+            "old":     "0".repeat(40),
+            "new":     "a".repeat(40),
+            "pusher":  "did:key:z6MkPusher",
+            "node":    node_did,
+            "ts":      "2026-07-22T00:00:00+00:00",
+        });
+        let sig = kp.sign_b64(&serde_json::to_vec(&payload).unwrap());
+
+        // Simulate the JSON a v1 server returns: no `version` key.
+        let cert_json = serde_json::json!({
+            "id":         "test-id",
+            "repo_id":    "repo-1",
+            "ref_name":   "refs/heads/main",
+            "old_sha":    "0".repeat(40),
+            "new_sha":    "a".repeat(40),
+            "pusher_did": "did:key:z6MkPusher",
+            "node_did":   node_did,
+            "signature":  sig.clone(),
+            "issued_at":  "2026-07-22T00:00:00+00:00",
+            // no `version` field
+        });
+
+        // Forward-compat: the parse yields v1 (the only version this
+        // client verifies), and the verdict arm matching Ok(1) feeds
+        // the v1 payload through verify_signature end to end.
+        assert_eq!(parse_cert_version(cert_json.get("version")).unwrap(), 1);
+        let verdict = verify_signature(
+            "repo-1",
+            "refs/heads/main",
+            &"0".repeat(40),
+            &"a".repeat(40),
+            "did:key:z6MkPusher",
+            &node_did,
+            "2026-07-22T00:00:00+00:00",
+            &sig,
+        );
+        assert!(
+            verdict.is_ok(),
+            "missing version must take the v1 verify path: {verdict:?}"
+        );
+    }
+
+    /// #26 Split PR 3: an explicit `version: 1` on a cert is the v1
+    /// verify path. A v1 cert with an explicit version verifies the
+    /// same as a v1 cert without one — round-trip through
+    /// `verify_signature`.
+    #[test]
+    fn explicit_version_1_takes_v1_path() {
+        let kp = gitlawb_core::identity::Keypair::generate();
+        let node_did = kp.did().as_str().to_string();
+        let payload = serde_json::json!({
+            "repo_id": "repo-1",
+            "ref":     "refs/heads/main",
+            "old":     "0".repeat(40),
+            "new":     "a".repeat(40),
+            "pusher":  "did:key:z6MkPusher",
+            "node":    node_did,
+            "ts":      "2026-07-22T00:00:00+00:00",
+        });
+        let sig = kp.sign_b64(&serde_json::to_vec(&payload).unwrap());
+
+        // Simulate a v1 cert with an explicit `version: 1` field.
+        let cert_json = serde_json::json!({
+            "id":         "test-id",
+            "repo_id":    "repo-1",
+            "ref_name":   "refs/heads/main",
+            "old_sha":    "0".repeat(40),
+            "new_sha":    "a".repeat(40),
+            "pusher_did": "did:key:z6MkPusher",
+            "node_did":   node_did,
+            "signature":  sig.clone(),
+            "issued_at":  "2026-07-22T00:00:00+00:00",
+            "version":    1,
+        });
+
+        // The parse arms the Ok(1) verdict branch, which calls
+        // verify_signature on the v1 payload. A signature mismatch
+        // here would mean the verdict branch was wired to the
+        // wrong path or the payload drifted.
+        assert_eq!(parse_cert_version(cert_json.get("version")).unwrap(), 1);
+        let verdict = verify_signature(
+            "repo-1",
+            "refs/heads/main",
+            &"0".repeat(40),
+            &"a".repeat(40),
+            "did:key:z6MkPusher",
+            &node_did,
+            "2026-07-22T00:00:00+00:00",
+            &sig,
+        );
+        assert!(
+            verdict.is_ok(),
+            "explicit version: 1 must take the v1 verify path: {verdict:?}"
+        );
+    }
+
+    /// #26 Split PR 3 + Reviewer 2: parse_cert_version is the
+    /// load-bearing gate that distinguishes a missing-key (legacy
+    /// server) from a present-but-malformed value. The four cases
+    /// below pin every cell of that truth table; collapsing any
+    /// pair would let a server advertise "version 2" (or 99, or
+    /// 4294967297, or "two") while the client runs the v1
+    /// signature path on a different signed payload — a hostile
+    /// misconfiguration that `--verify` must not silently accept.
+    #[test]
+    fn parse_cert_version_truth_table() {
+        // MISSING key → legacy v1 (the v1 path is the only one that
+        // could match pre-versioning bytes).
+        assert_eq!(
+            parse_cert_version(None).unwrap(),
+            1,
+            "a missing version field is the legacy v1 path"
+        );
+
+        // Explicit 1 → v1.
+        assert_eq!(
+            parse_cert_version(Some(&serde_json::json!(1))).unwrap(),
+            1,
+            "explicit version 1 is the v1 path"
+        );
+
+        // Explicit 2 → Err, NOT Ok(1). Reviewer 2: a v1 signature
+        // must not verify against a server that said "version 2".
+        assert!(
+            parse_cert_version(Some(&serde_json::json!(2))).is_err(),
+            "explicit version 2 must not collapse to v1"
+        );
+
+        // u32 overflow (2^32 + 1) → Err, NOT Ok(1). The previous
+        // `.map(|v| v as u32)` silently truncated this to 1.
+        let overflow = serde_json::json!(u64::from(u32::MAX) + 1);
+        assert!(
+            parse_cert_version(Some(&overflow)).is_err(),
+            "a version that overflows u32 must not truncate to 1"
+        );
+
+        // Non-numeric: a string, a float, null, bool — every shape
+        // that is not a JSON integer is rejected. None of these
+        // may collapse to v1.
+        for bad in [
+            serde_json::json!("two"),
+            serde_json::json!(2.0), // serde_json::Value::is_f64 is true for this
+            serde_json::json!(2.5), // obviously fractional
+            serde_json::json!(true),
+            serde_json::json!(null),
+            serde_json::json!([2]),
+            serde_json::json!({"v": 2}),
+        ] {
+            assert!(
+                parse_cert_version(Some(&bad)).is_err(),
+                "non-integer version {bad} must not collapse to v1"
+            );
+        }
+    }
+
+    /// A version refusal must not print as INVALID — no cryptographic
+    /// check ran, so labelling it INVALID would tell operators an Ed25519
+    /// verification failed when the payload shape was the disqualifier.
+    /// Removing the label branch fails this test.
+    #[test]
+    fn signature_verdict_label_distinguishes_refusal_from_bad_signature() {
+        assert_eq!(
+            signature_verdict_label(&Err("version 2 not supported".to_string())),
+            "UNSUPPORTED VERSION"
+        );
+        assert_eq!(signature_verdict_label(&Ok(1)), "INVALID");
+    }
+
+    /// #26 Split PR 3, round 2: a `version: 2` certificate must fail
+    /// `--verify` THROUGH `cmd_show` itself, not through a copy of its
+    /// match. The prior form of this test duplicated the verdict match in
+    /// the test body and aimed at an `Ok(v != 1)` arm the real parser can
+    /// never produce, so gutting `cmd_show`'s rejection kept every test
+    /// green (demonstrated by the reviewer by execution). Driving the
+    /// command end to end is what makes this a guard: the mock serves a
+    /// cert whose SIGNATURE IS VALID for v1 bytes but which declares
+    /// version 2, and the command must exit Err with the version named —
+    /// the v1 verify path must not launder it through.
+    #[tokio::test]
+    async fn cmd_show_verify_rejects_v2_even_with_valid_v1_signature() {
+        let kp = gitlawb_core::identity::Keypair::generate();
+        let node_did = kp.did().as_str().to_string();
+
+        // Sign a v1-shaped payload so the ONLY disqualifier is the version.
+        let payload = serde_json::json!({
+            "repo_id": "repo-1",
+            "ref":     "refs/heads/main",
+            "old":     "0".repeat(40),
+            "new":     "a".repeat(40),
+            "pusher":  "did:key:z6MkPusher",
+            "node":    node_did,
+            "ts":      "2026-07-22T00:00:00+00:00",
+        });
+        let sig = kp.sign_b64(&serde_json::to_vec(&payload).unwrap());
+
+        let cert_json = serde_json::json!({
+            "id":         "0123456789abcdef0123456789abcdef0123",
+            "repo_id":    "repo-1",
+            "ref_name":   "refs/heads/main",
+            "old_sha":    "0".repeat(40),
+            "new_sha":    "a".repeat(40),
+            "pusher_did": "did:key:z6MkPusher",
+            "node_did":   node_did,
+            "signature":  sig,
+            "issued_at":  "2026-07-22T00:00:00+00:00",
+            "version":    2,
+        });
+
+        let mut server = mockito::Server::new_async().await;
+        // 36-char id short-circuits resolve_cert_id; "owner/repo" input
+        // short-circuits resolve_repo — the cert GET is the only required
+        // route. `/` may be probed for the contextual DID line; serving it
+        // keeps the test about the verdict, not about network noise.
+        let _cert = server
+            .mock(
+                "GET",
+                "/api/v1/repos/o/r/certs/0123456789abcdef0123456789abcdef0123",
+            )
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(cert_json.to_string())
+            .create_async()
+            .await;
+        let _root = server
+            .mock("GET", "/")
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(serde_json::json!({ "did": node_did }).to_string())
+            .create_async()
+            .await;
+
+        let err = cmd_show(
+            "o/r".to_string(),
+            "0123456789abcdef0123456789abcdef0123".to_string(),
+            server.url(),
+            None,
+            true,
+            Some(node_did.clone()),
+        )
+        .await
+        .expect_err("a version-2 cert must fail --verify even with a valid v1 signature");
+        let msg = format!("{err:#}");
+        assert!(
+            msg.contains("version"),
+            "the failure must name the version mismatch, not a generic invalid signature: {msg}"
+        );
+    }
+
+    /// Round 2's second demonstrated escape: with no --expect-node, the old
+    /// fallback anchored the issuer to the DID the QUERIED NODE reports at
+    /// `/` — which a forger controls along with the cert, so a wholly forged
+    /// self-signed certificate passed --verify with exit 0. `cmd_show` with
+    /// --verify and no --expect-node must now refuse to claim a trusted
+    /// issuer, even when the served cert's signature is valid and the node's
+    /// self-report matches it.
+    #[tokio::test]
+    async fn cmd_show_verify_without_expect_node_refuses_self_asserted_anchor() {
+        let kp = gitlawb_core::identity::Keypair::generate();
+        let node_did = kp.did().as_str().to_string();
+        let payload = serde_json::json!({
+            "repo_id": "repo-1",
+            "ref":     "refs/heads/main",
+            "old":     "0".repeat(40),
+            "new":     "a".repeat(40),
+            "pusher":  "did:key:z6MkPusher",
+            "node":    node_did,
+            "ts":      "2026-07-22T00:00:00+00:00",
+        });
+        let sig = kp.sign_b64(&serde_json::to_vec(&payload).unwrap());
+        let cert_json = serde_json::json!({
+            "id":         "0123456789abcdef0123456789abcdef0123",
+            "repo_id":    "repo-1",
+            "ref_name":   "refs/heads/main",
+            "old_sha":    "0".repeat(40),
+            "new_sha":    "a".repeat(40),
+            "pusher_did": "did:key:z6MkPusher",
+            "node_did":   node_did,
+            "signature":  sig,
+            "issued_at":  "2026-07-22T00:00:00+00:00",
+        });
+
+        let mut server = mockito::Server::new_async().await;
+        // P3 (reviewer round 2): both mocks are required to be
+        // hit for the test to mean anything. `mockito 1.7`'s
+        // `assert_on_drop` is off by default, so the previous
+        // `_cert` and `_root` bindings would silently let a
+        // regression that skipped the root fetch pass.
+        // `.expect(2)` pins each mock's call count (the cert mock
+        // is hit twice because the test runs `cmd_show` twice) and
+        // `.assert_async()` at the end fires if any are unmet.
+        let cert_mock = server
+            .mock(
+                "GET",
+                "/api/v1/repos/o/r/certs/0123456789abcdef0123456789abcdef0123",
+            )
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(cert_json.to_string())
+            .expect(2)
+            .create_async()
+            .await;
+        // The forger's matching self-report — the exact shape
+        // that used to convert into a trust anchor. The guard's
+        // whole point is to make the test FAIL if a future
+        // regression stops calling `/` here: `.expect(2)` +
+        // `.assert_async()` makes that dependency visible.
+        let root_mock = server
+            .mock("GET", "/")
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(serde_json::json!({ "did": node_did }).to_string())
+            .expect(2)
+            .create_async()
+            .await;
+
+        let err = cmd_show(
+            "o/r".to_string(),
+            "0123456789abcdef0123456789abcdef0123".to_string(),
+            server.url(),
+            None,
+            true,
+            None,
+        )
+        .await
+        .expect_err("--verify without --expect-node must not mint a trust anchor from the node's self-report");
+        let msg = format!("{err:#}");
+        assert!(
+            msg.contains("--expect-node"),
+            "the refusal must tell the caller how to anchor trust: {msg}"
+        );
+
+        // Positive control: the same cert with the DID explicitly trusted
+        // passes, so the guard is about the anchor, not the signature.
+        cmd_show(
+            "o/r".to_string(),
+            "0123456789abcdef0123456789abcdef0123".to_string(),
+            server.url(),
+            None,
+            true,
+            Some(node_did),
+        )
+        .await
+        .expect("explicitly anchored --verify of a valid v1 cert must pass");
+
+        // P3 (reviewer round 2): confirm the mocks were actually
+        // hit. Without this, a regression that skipped the cert or
+        // root fetches would still produce a passing test (the
+        // `expect_err` arm succeeds, the positive control succeeds)
+        // and the test would mean nothing.
+        cert_mock.assert_async().await;
+        root_mock.assert_async().await;
+    }
 }

crates/gl/src/changelog.rs

@@ -75,14 +75,14 @@ pub async fn run(args: ChangelogArgs) -> Result<()> {
 
     for event in &events {
         let ts = event["timestamp"].as_str().unwrap_or("?");
-        let date = &ts[..ts.len().min(10)];
+        let date = crate::text::truncate(ts, 10);
 
         match event["type"].as_str().unwrap_or("") {
             "commit" => {
                 let sha = event["sha"].as_str().unwrap_or("?");
                 let msg = event["message"].as_str().unwrap_or("?");
                 let first_line = msg.lines().next().unwrap_or(msg);
-                let short_sha = &sha[..sha.len().min(8)];
+                let short_sha = crate::text::truncate(sha, 8);
                 println!("  {date}  commit      {short_sha}  {first_line}");
             }
             "pr_merged" => {

crates/gl/src/clone.rs

@@ -163,8 +163,8 @@ pub fn setup_partial_clone(
 
     if withheld_globs.is_empty() {
         match branch {
-            Some(b) => git_global(&["clone", "-q", "--branch", b, remote_url, dest_str])?,
-            None => git_global(&["clone", "-q", remote_url, dest_str])?,
+            Some(b) => git_global(&["clone", "-q", "--branch", b, "--", remote_url, dest_str])?,
+            None => git_global(&["clone", "-q", "--", remote_url, dest_str])?,
         }
         return Ok(());
     }
@@ -174,6 +174,7 @@ pub fn setup_partial_clone(
         "-q",
         "--filter=blob:none",
         "--no-checkout",
+        "--",
         remote_url,
         dest_str,
     ])?;
@@ -388,7 +389,7 @@ async fn recover_encrypted_blobs(
         let plaintext = match open_blob(&envelope, keypair) {
             Ok(p) => p,
             Err(e) => {
-                eprintln!("warning: could not decrypt {oid}: {e}");
+                emit_warning(&format!("warning: could not decrypt {oid}: {e}"));
                 continue;
             }
         };
@@ -406,7 +407,9 @@ async fn recover_encrypted_blobs(
                 recovered.push(p.clone());
             }
         } else {
-            eprintln!("warning: recovered blob {oid} hashed to {written}; discarding");
+            emit_warning(&format!(
+                "warning: recovered blob {oid} hashed to {written}; discarding"
+            ));
         }
     }
     Ok(recovered)
@@ -804,7 +807,9 @@ async fn recover_from_arweave(
                 recovered.push(p.clone());
             }
         } else {
-            eprintln!("warning: recovered blob {oid} hashed to {written}; discarding");
+            emit_warning(&format!(
+                "warning: recovered blob {oid} hashed to {written}; discarding"
+            ));
         }
     }
     Ok(recovered)
@@ -1090,6 +1095,85 @@ mod tests {
         );
     }
 
+    #[test]
+    fn setup_partial_clone_plain_without_withheld_paths() {
+        let (td, url) = bare_remote(&[("file.txt", b"hello\n")]);
+        let dest = td.path().join("dest");
+        setup_partial_clone(&dest, &url, &[], &[], None).unwrap();
+        assert!(dest.join("file.txt").exists());
+    }
+
+    #[test]
+    fn setup_partial_clone_with_branch_without_withheld_paths() {
+        let (td, url) = bare_remote(&[("file.txt", b"hello\n")]);
+        let branch_out = Command::new("git")
+            .args([
+                "-C",
+                td.path().join("origin").to_str().unwrap(),
+                "branch",
+                "--show-current",
+            ])
+            .output()
+            .unwrap();
+        let branch = String::from_utf8(branch_out.stdout)
+            .unwrap()
+            .trim()
+            .to_string();
+        let dest = td.path().join("dest");
+        setup_partial_clone(&dest, &url, &[], &[], Some(&branch)).unwrap();
+        assert!(dest.join("file.txt").exists());
+    }
+
+    /// #374: every `setup_partial_clone` shape interpolates `remote_url` into a
+    /// `git clone` argv. The `--` delimiter is what forces an option-shaped
+    /// remote to be read as a repository rather than parsed as an option.
+    ///
+    /// The assertion is on git's own words — `repository '<value>'`, single-quoted
+    /// — and not merely on the value appearing somewhere in the error. `git_global`
+    /// formats its failure as `git {args:?} failed: {stderr}`, so the injected
+    /// string is already present in the debug-printed argv whether or not the
+    /// delimiter is there; asserting on containment alone would pass with `--`
+    /// deleted. Undelimited, git consumes the value as an option, leaving the
+    /// destination as the only positional, and names *that* instead
+    /// (`repository '<dest>' does not exist`) — so this goes red without the fix.
+    ///
+    /// The payload carries no `:` or space: a colon would make git read the
+    /// argument as an ssh-style `host:path` URL and report a hostname, which is a
+    /// different parse.
+    fn assert_option_shaped_remote_is_a_repository(withheld: &[String], branch: Option<&str>) {
+        let td = TempDir::new().unwrap();
+        let dest = td.path().join("dest");
+        let injected = "--upload-pack=false";
+
+        let err = setup_partial_clone(&dest, injected, withheld, &[], branch)
+            .expect_err("an option-shaped remote is not a repository and must fail")
+            .to_string();
+
+        assert!(
+            err.contains(&format!("repository '{injected}'")),
+            "git must name the argument as the repository, not consume it as an option: {err}"
+        );
+    }
+
+    #[test]
+    fn plain_clone_never_parses_a_remote_as_a_git_option() {
+        assert_option_shaped_remote_is_a_repository(&[], None);
+    }
+
+    #[test]
+    fn branch_clone_never_parses_a_remote_as_a_git_option() {
+        // Pins that `--branch <b>` stays ahead of the delimiter: if `--` were moved
+        // before `--branch`, git would read the branch name as a positional instead.
+        assert_option_shaped_remote_is_a_repository(&[], Some("main"));
+    }
+
+    #[test]
+    fn sparse_clone_never_parses_a_remote_as_a_git_option() {
+        // The withheld-globs arm is a separate argv construction
+        // (`--filter=blob:none --no-checkout`) and needs its own proof.
+        assert_option_shaped_remote_is_a_repository(&["/secret/**".to_string()], None);
+    }
+
     #[test]
     fn sparse_patterns_subtree_and_exact() {
         assert_eq!(sparse_patterns("/secret/**"), vec!["/secret/".to_string()]);

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/src/ipfs_cmd.rs

@@ -131,11 +131,7 @@ async fn cmd_list(node: String, dir: Option<PathBuf>) -> Result<()> {
         let sha = pin["sha256_hex"].as_str().unwrap_or("?");
         let pinned_at = pin["pinned_at"].as_str().unwrap_or("?");
         // Trim pinned_at to date+time without subseconds
-        let ts = if pinned_at.len() >= 19 {
-            &pinned_at[..19]
-        } else {
-            pinned_at
-        };
+        let ts = crate::text::truncate(pinned_at, 19);
         println!("  {cid}");
         println!("    sha256: {sha}");
         println!("    pinned: {ts}");

crates/gl/src/issue.rs

@@ -251,7 +251,7 @@ async fn cmd_list(repo: String, node: String, dir: Option<PathBuf>) -> Result<()
         let status = issue["status"].as_str().unwrap_or("?");
         let created = issue["created_at"]
             .as_str()
-            .map(|s| &s[..10])
+            .map(|s| crate::text::truncate(s, 10))
             .unwrap_or("?");
         let icon = match status {
             "open" => "○",
@@ -365,10 +365,13 @@ async fn cmd_issue_comments(
         let author_short = author
             .split(':')
             .next_back()
-            .map(|s| &s[..s.len().min(8)])
+            .map(|s| crate::text::truncate(s, 8))
             .unwrap_or("?");
         let cbody = c["body"].as_str().unwrap_or("");
-        let created = c["created_at"].as_str().map(|s| &s[..10]).unwrap_or("?");
+        let created = c["created_at"]
+            .as_str()
+            .map(|s| crate::text::truncate(s, 10))
+            .unwrap_or("?");
         println!("  · {author_short} ({created})");
         println!("    {cbody}");
         println!();
@@ -439,6 +442,34 @@ mod tests {
         .unwrap();
     }
 
+    #[tokio::test]
+    async fn test_cmd_list_survives_malformed_timestamps() {
+        let dir = TempDir::new().unwrap();
+        write_identity(&dir);
+
+        let mut server = mockito::Server::new_async().await;
+        // A short timestamp used to panic on `&s[..10]`; a timestamp whose
+        // byte 10 is mid-char panics even with a len() guard.
+        let _m = server
+            .mock(
+                "GET",
+                mockito::Matcher::Regex(r"^/api/v1/repos/[^/]+/myrepo/issues$".to_string()),
+            )
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(r#"{"issues":[{"id":"a","title":"short ts","status":"open","created_at":"2026"},{"id":"b","title":"mb ts","status":"open","created_at":"2026-08-1é5T00:00:00Z"},{"id":"c","title":"empty ts","status":"open","created_at":""}]}"#)
+            .create_async()
+            .await;
+
+        cmd_list(
+            "myrepo".to_string(),
+            server.url(),
+            Some(dir.path().to_path_buf()),
+        )
+        .await
+        .unwrap();
+    }
+
     #[tokio::test]
     async fn test_cmd_create_success() {
         let dir = TempDir::new().unwrap();

crates/gl/src/main.rs

@@ -30,6 +30,7 @@ mod star;
 mod status;
 mod sync;
 mod task;
+mod text;
 mod ucan_cmd;
 mod visibility;
 mod webhook;

crates/gl/src/mirror.rs

@@ -41,6 +41,17 @@ pub struct MirrorArgs {
     pub dir: Option<PathBuf>,
 }
 
+/// Argv for the mirror clone, with `--` immediately before the first positional
+/// so an option-shaped `source` cannot be parsed as a git option (#374).
+///
+/// This is a separate function purely so the delimiter's placement can be driven
+/// through real git by a test. The production call keeps `.status()` rather than
+/// `.output()`: a mirror clone of a large repo streams git's progress to the
+/// user's terminal, and capturing it to assert on stderr here would swallow that.
+fn mirror_clone_args(source: &str) -> [&str; 4] {
+    ["clone", "--mirror", "--", source]
+}
+
 pub async fn run(args: MirrorArgs) -> Result<()> {
     let source = args.source.trim_end_matches('/').to_string();
 
@@ -145,7 +156,7 @@ fn clone_mirror(source: &str, dest: &std::path::Path) -> Result<()> {
     // bounded-runner rule governs node request handlers. Same spawn as before,
     // only relocated into this helper.
     let status = Command::new("git")
-        .args(["clone", "--mirror", source])
+        .args(mirror_clone_args(source))
         .arg(dest)
         .status()
         .context("failed to run git clone — is git installed?")?;
@@ -186,6 +197,43 @@ pub fn extract_repo_name(url: &str) -> Option<String> {
 mod tests {
     use super::*;
 
+    /// #374: `gl mirror` interpolates the user-supplied source into a
+    /// `git clone --mirror` argv. `--` must sit immediately before the first
+    /// positional so an option-shaped source is read as a repository.
+    ///
+    /// This drives the production argv through real git rather than asserting on
+    /// the array's contents, so it fails on a delimiter that is missing *or*
+    /// misplaced. It cannot go through `run()`: that loads an identity keypair and
+    /// contacts a node before reaching the clone, and its bail message interpolates
+    /// `{source}` unconditionally, so an assertion there would pass with `--` gone.
+    ///
+    /// Asserting on git's single-quoted `repository '<value>'` is what makes this
+    /// red without the delimiter — undelimited, git consumes the value as an option
+    /// and names the destination as the repository instead.
+    #[test]
+    fn mirror_clone_never_parses_the_source_as_a_git_option() {
+        let td = tempfile::TempDir::new().unwrap();
+        let dest = td.path().join("mirror.git");
+        let injected = "--upload-pack=false";
+
+        let out = Command::new("git")
+            .env("LC_ALL", "C")
+            .args(mirror_clone_args(injected))
+            .arg(&dest)
+            .output()
+            .unwrap();
+        let stderr = String::from_utf8_lossy(&out.stderr);
+
+        assert!(
+            !out.status.success(),
+            "an option-shaped source is not a repository and must fail the clone"
+        );
+        assert!(
+            stderr.contains(&format!("repository '{injected}'")),
+            "git must name the source as the repository, not consume it as an option: {stderr}"
+        );
+    }
+
     #[test]
     fn test_extract_github_url() {
         assert_eq!(
@@ -330,7 +378,8 @@ mod tests {
             .create_async()
             .await;
 
-        // We can't easily test the git subprocess calls, but we can test the
+        // The git subprocess argv is pinned separately by
+        // `mirror_clone_never_parses_the_source_as_a_git_option`; this covers the
         // API error path by calling the create step directly via NodeClient.
         let kp2 = gitlawb_core::identity::Keypair::generate();
         let client = NodeClient::new(server.url(), Some(kp2));

crates/gl/src/node.rs

@@ -413,7 +413,7 @@ async fn cmd_status(node: String, dir: Option<PathBuf>) -> Result<()> {
                     let ref_name = ev["ref"].as_str().unwrap_or("?");
                     let ts = ev["timestamp"]
                         .as_str()
-                        .map(|s| &s[..10.min(s.len())])
+                        .map(|s| crate::text::truncate(s, 10))
                         .unwrap_or("?");
                     println!("  {ts}  {repo}  {ref_name}");
                 }

crates/gl/src/peer.rs

@@ -97,7 +97,7 @@ async fn cmd_list(node: String) -> Result<()> {
         let reachable = peer["reachable"].as_bool().unwrap_or(false);
         let last_seen = peer["last_seen"]
             .as_str()
-            .map(|s| &s[..10])
+            .map(|s| crate::text::truncate(s, 10))
             .unwrap_or("never");
         let status = if reachable { "✓" } else { "✗" };
         println!("  {status} {url}");

crates/gl/src/pr.rs

@@ -304,7 +304,7 @@ async fn cmd_list(repo: String, node: String, dir: Option<PathBuf>) -> Result<()
         let author_short = author
             .split(':')
             .next_back()
-            .map(|s| &s[..s.len().min(8)])
+            .map(|s| crate::text::truncate(s, 8))
             .unwrap_or("?");
         let status_icon = match status {
             "open" => "○",
@@ -374,7 +374,7 @@ async fn cmd_view(repo: String, number: u64, node: String, dir: Option<PathBuf>)
             let reviewer_short = reviewer
                 .split(':')
                 .next_back()
-                .map(|s| &s[..s.len().min(8)])
+                .map(|s| crate::text::truncate(s, 8))
                 .unwrap_or("?");
             let rstatus = r["status"].as_str().unwrap_or("?");
             let rbody = r["body"].as_str().unwrap_or("");
@@ -414,10 +414,13 @@ async fn cmd_view(repo: String, number: u64, node: String, dir: Option<PathBuf>)
             let author_short = author
                 .split(':')
                 .next_back()
-                .map(|s| &s[..s.len().min(8)])
+                .map(|s| crate::text::truncate(s, 8))
                 .unwrap_or("?");
             let cbody = c["body"].as_str().unwrap_or("");
-            let created = c["created_at"].as_str().map(|s| &s[..10]).unwrap_or("?");
+            let created = c["created_at"]
+                .as_str()
+                .map(|s| crate::text::truncate(s, 10))
+                .unwrap_or("?");
             println!("  · {author_short} ({created})");
             println!("    {cbody}");
         }
@@ -464,7 +467,7 @@ async fn cmd_merge(repo: String, number: u64, node: String, dir: Option<PathBuf>
 
     let sha = result["merge_sha"].as_str().unwrap_or("?");
     println!("✓ Merged PR #{number}");
-    println!("  Merge commit: {}", &sha[..sha.len().min(12)]);
+    println!("  Merge commit: {}", crate::text::truncate(sha, 12));
     Ok(())
 }
 
@@ -555,10 +558,13 @@ async fn cmd_comments(repo: String, number: u64, node: String, dir: Option<PathB
         let author_short = author
             .split(':')
             .next_back()
-            .map(|s| &s[..s.len().min(8)])
+            .map(|s| crate::text::truncate(s, 8))
             .unwrap_or("?");
         let cbody = c["body"].as_str().unwrap_or("");
-        let created = c["created_at"].as_str().map(|s| &s[..10]).unwrap_or("?");
+        let created = c["created_at"]
+            .as_str()
+            .map(|s| crate::text::truncate(s, 10))
+            .unwrap_or("?");
         println!("  · {author_short} ({created})");
         println!("    {cbody}");
         println!();

crates/gl/src/repo.rs

@@ -337,7 +337,10 @@ async fn cmd_list(node: String, dir: Option<PathBuf>) -> Result<()> {
         let name = r["name"].as_str().unwrap_or("?");
         let desc = r["description"].as_str().unwrap_or("");
         let public = r["is_public"].as_bool().unwrap_or(true);
-        let updated = r["updated_at"].as_str().map(|s| &s[..10]).unwrap_or("?");
+        let updated = r["updated_at"]
+            .as_str()
+            .map(|s| crate::text::truncate(s, 10))
+            .unwrap_or("?");
         let vis = if public { "public" } else { "private" };
         println!("  {name:<24}  {vis:<8}  {updated}  {desc}");
     }
@@ -514,7 +517,7 @@ async fn cmd_replicas(repo: String, node: String, dir: Option<PathBuf>) -> Resul
             let url = r["replica_url"].as_str().unwrap_or("?");
             let registered = r["registered_at"]
                 .as_str()
-                .map(|s| &s[..10.min(s.len())])
+                .map(|s| crate::text::truncate(s, 10))
                 .unwrap_or("?");
             println!("  {registered}  {did}  →  {url}");
         }
@@ -556,7 +559,7 @@ pub(crate) async fn cmd_commits(
             .or_else(|| c["sha"].as_str())
             .or_else(|| c["oid"].as_str())
             .unwrap_or("?");
-        let short_sha = &sha[..sha.len().min(10)];
+        let short_sha = crate::text::truncate(sha, 10);
         let msg = c["message"].as_str().unwrap_or("(no message)");
         let first_line = msg.lines().next().unwrap_or(msg);
         let author = c["author_name"]
@@ -566,7 +569,7 @@ pub(crate) async fn cmd_commits(
         let date = c["date"]
             .as_str()
             .or_else(|| c["committer_date"].as_str())
-            .map(|s| &s[..10.min(s.len())])
+            .map(|s| crate::text::truncate(s, 10))
             .unwrap_or("?");
         println!("  {short_sha}  {date}  {first_line}  ({author})");
     }

crates/gl/src/text.rs

@@ -0,0 +1,63 @@
+//! Display helpers for strings that come off the wire unchecked.
+
+/// Truncate `s` to at most `max` bytes, cutting at a char boundary.
+///
+/// `&s[..max]` panics when `s` is shorter than `max` bytes and when `max`
+/// lands inside a multi-byte char; `s.len().min(max)` guards only the first
+/// case. Node-supplied timestamps and ids hit both.
+pub(crate) fn truncate(s: &str, max: usize) -> &str {
+    let mut end = max.min(s.len());
+    while !s.is_char_boundary(end) {
+        end -= 1;
+    }
+    &s[..end]
+}
+
+/// Keep at most the last `max` bytes of `s`, cutting at a char boundary.
+///
+/// `&s[s.len().saturating_sub(max)..]` panics when the start index lands
+/// inside a multi-byte char; node-supplied ids hit that case.
+pub(crate) fn truncate_suffix(s: &str, max: usize) -> &str {
+    let mut start = s.len().saturating_sub(max);
+    while !s.is_char_boundary(start) {
+        start += 1;
+    }
+    &s[start..]
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{truncate, truncate_suffix};
+
+    #[test]
+    fn cuts_ascii_at_max() {
+        assert_eq!(truncate("2026-08-15T12:34:56Z", 10), "2026-08-15");
+    }
+
+    #[test]
+    fn short_and_empty_inputs_return_whole() {
+        assert_eq!(truncate("2026", 10), "2026");
+        assert_eq!(truncate("", 10), "");
+    }
+
+    #[test]
+    fn multibyte_cut_does_not_panic() {
+        // Byte index 10 lands inside 'é'; the cut must land on the boundary
+        // before it (9 bytes) rather than panic or exceed the byte limit.
+        assert_eq!(truncate("2026-08-1\u{e9}5T12:34:56Z", 10), "2026-08-1");
+        // A string that is entirely multi-byte and shorter than max.
+        assert_eq!(truncate("\u{e9}\u{e9}", 10), "\u{e9}\u{e9}");
+    }
+
+    #[test]
+    fn suffix_cut_does_not_panic() {
+        // 'é' is two bytes; a start index of len-8 lands inside it.
+        assert_eq!(truncate_suffix("did:key:\u{e9}1234567", 8), "1234567");
+    }
+
+    #[test]
+    fn suffix_short_input_returns_whole() {
+        assert_eq!(truncate_suffix("short", 8), "short");
+        assert_eq!(truncate_suffix("", 8), "");
+    }
+}

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

crates/icaptcha-client/src/lib.rs

@@ -88,20 +88,81 @@ fn sanitize_excerpt(s: &str) -> String {
     out
 }
 
-/// Whether `u` parses as a trusted URL.
+/// Fixed warning when `GITLAWB_ICAPTCHA_INSECURE` is present but not an
+/// explicit truthy value. Does not include the raw env value (arbitrary
+/// deployment input — secrets / terminal controls).
+fn warn_icaptcha_insecure_non_truthy() {
+    // One literal line on purpose: a continuation escape here keeps emitting
+    // correct prose (Rust strips the next line's leading whitespace), but the
+    // single-line form makes that property visible without knowing the escape
+    // semantics, which is what the review asked for.
+    tracing::warn!(
+        "GITLAWB_ICAPTCHA_INSECURE is set but not truthy (expected 1 or true); loopback HTTP trust relaxation remains disabled"
+    );
+}
+
+/// Parse a possibly-present `GITLAWB_ICAPTCHA_INSECURE` string.
 ///
-/// Production trusts only `https`.  When `GITLAWB_ICAPTCHA_INSECURE` is set (a
-/// runtime escape hatch for integration tests) the function also trusts
-/// `http://127.0.0.1` and `http://localhost` so the full iCaptcha retry path
-/// can be exercised against a local mockito server.
-fn is_https(u: &str) -> bool {
+/// Only an explicit truthy value (`1` / `true`, case-insensitive, after trim)
+/// enables the loopback-HTTP trust relaxation. Presence alone is not enough:
+/// `=0`, `=false`, or an empty value leave it off. A non-empty non-truthy
+/// value emits [`warn_icaptcha_insecure_non_truthy`].
+fn icaptcha_insecure_from_str(raw: Option<&str>) -> bool {
+    let Some(v) = raw else {
+        return false;
+    };
+    let t = v.trim();
+    if t.eq_ignore_ascii_case("1") || t.eq_ignore_ascii_case("true") {
+        true
+    } else {
+        if !t.is_empty() {
+            warn_icaptcha_insecure_non_truthy();
+        }
+        false
+    }
+}
+
+/// Interpret `std::env::var("GITLAWB_ICAPTCHA_INSECURE")` without reading the
+/// environment again — used by production and by unit tests.
+///
+/// `NotPresent` → disabled. `NotUnicode` → disabled + the same fixed warning
+/// as a present non-truthy value (fail closed; do not treat as unset).
+fn icaptcha_insecure_from_var(result: Result<String, std::env::VarError>) -> bool {
+    match result {
+        Ok(v) => icaptcha_insecure_from_str(Some(&v)),
+        Err(std::env::VarError::NotUnicode(_)) => {
+            warn_icaptcha_insecure_non_truthy();
+            false
+        }
+        Err(std::env::VarError::NotPresent) => false,
+    }
+}
+
+/// Whether `GITLAWB_ICAPTCHA_INSECURE` explicitly enables the loopback-HTTP
+/// trust relaxation used by integration tests.
+///
+/// Only an explicit truthy value (`1` / `true`, case-insensitive, after trim)
+/// enables it. Presence alone is not enough: `=0`, `=false`, or an empty
+/// value leave the relaxation off. A non-empty non-truthy value (including a
+/// present non-Unicode value) emits a `tracing::warn!` (visible when the
+/// `icaptcha_client` target is at WARN+, e.g. `RUST_LOG=icaptcha_client=warn`).
+fn icaptcha_insecure_enabled() -> bool {
+    icaptcha_insecure_from_var(std::env::var("GITLAWB_ICAPTCHA_INSECURE"))
+}
+
+/// Whether `u` parses as a trusted URL, given a pre-parsed insecure flag.
+///
+/// Production trusts only `https`. When `insecure` is true the function also
+/// trusts `http://127.0.0.1` and `http://localhost` so the full iCaptcha retry
+/// path can be exercised against a local mockito server.
+fn is_https_with(u: &str, insecure: bool) -> bool {
     let Ok(parsed) = reqwest::Url::parse(u) else {
         return false;
     };
     if parsed.scheme() == "https" {
         return true;
     }
-    if std::env::var_os("GITLAWB_ICAPTCHA_INSECURE").is_some()
+    if insecure
         && parsed.scheme() == "http"
         && matches!(parsed.host_str(), Some("127.0.0.1" | "localhost"))
     {
@@ -139,7 +200,15 @@ fn origin_key(u: &str) -> Option<String> {
 /// explicitly-configured host (never a node-discovered one), so a key is never
 /// exfiltrated to an origin the operator did not choose.
 fn resolve_solver_url(advertised: Option<&str>, operator: Option<&str>) -> (String, bool) {
-    let operator = operator.filter(|u| is_https(u));
+    resolve_solver_url_with(advertised, operator, icaptcha_insecure_enabled())
+}
+
+fn resolve_solver_url_with(
+    advertised: Option<&str>,
+    operator: Option<&str>,
+    insecure: bool,
+) -> (String, bool) {
+    let operator = operator.filter(|u| is_https_with(u, insecure));
     let operator_origin = operator.as_ref().and_then(|u| origin_key(u));
     let default_origin = origin_key(DEFAULT_URL);
 
@@ -157,7 +226,7 @@ fn resolve_solver_url(advertised: Option<&str>, operator: Option<&str>) -> (Stri
     };
 
     let chosen = match advertised {
-        Some(a) if is_https(a) && allowed(&origin_key(a)) => a.to_string(),
+        Some(a) if is_https_with(a, insecure) && allowed(&origin_key(a)) => a.to_string(),
         Some(a) => {
             tracing::warn!(
                 advertised = %a,
@@ -389,27 +458,36 @@ mod tests {
     use std::ffi::OsString;
     use std::sync::{Mutex, MutexGuard};
 
-    /// Serializes tests that touch the process-global
-    /// `GITLAWB_ICAPTCHA_INSECURE` env var so they never race.
+    /// Serializes the real-env reader test so it never races with other
+    /// process-global env mutations (and recovers from a poisoned lock).
     static ICAPTCHA_ENV_LOCK: Mutex<()> = Mutex::new(());
 
-    /// Set `GITLAWB_ICAPTCHA_INSECURE` for the test lifetime, restoring any
-    /// prior value on drop.
-    struct InsecureEnv {
+    struct EnvGuard {
         _lock: MutexGuard<'static, ()>,
         prev: Option<OsString>,
     }
 
-    impl InsecureEnv {
-        fn new() -> Self {
-            let lock = ICAPTCHA_ENV_LOCK.lock().unwrap();
+    impl EnvGuard {
+        fn lock() -> MutexGuard<'static, ()> {
+            ICAPTCHA_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner())
+        }
+
+        fn with_value(val: &str) -> Self {
+            let lock = Self::lock();
             let prev = std::env::var_os("GITLAWB_ICAPTCHA_INSECURE");
-            std::env::set_var("GITLAWB_ICAPTCHA_INSECURE", "1");
-            InsecureEnv { _lock: lock, prev }
+            std::env::set_var("GITLAWB_ICAPTCHA_INSECURE", val);
+            EnvGuard { _lock: lock, prev }
+        }
+
+        fn unset() -> Self {
+            let lock = Self::lock();
+            let prev = std::env::var_os("GITLAWB_ICAPTCHA_INSECURE");
+            std::env::remove_var("GITLAWB_ICAPTCHA_INSECURE");
+            EnvGuard { _lock: lock, prev }
         }
     }
 
-    impl Drop for InsecureEnv {
+    impl Drop for EnvGuard {
         fn drop(&mut self) {
             match self.prev.take() {
                 Some(v) => std::env::set_var("GITLAWB_ICAPTCHA_INSECURE", v),
@@ -418,6 +496,118 @@ mod tests {
         }
     }
 
+    /// #227 / #246: cover the production env read in `icaptcha_insecure_enabled`.
+    /// Pure-helper tests alone leave mutations of this function green.
+    #[test]
+    fn icaptcha_insecure_enabled_reads_real_env() {
+        {
+            let _g = EnvGuard::unset();
+            assert!(!icaptcha_insecure_enabled(), "unset must disable");
+        }
+        {
+            let _g = EnvGuard::with_value("0");
+            assert!(!icaptcha_insecure_enabled(), "=0 must disable");
+        }
+        {
+            let _g = EnvGuard::with_value("false");
+            assert!(!icaptcha_insecure_enabled(), "=false must disable");
+        }
+        {
+            let _g = EnvGuard::with_value("");
+            assert!(!icaptcha_insecure_enabled(), "empty must disable");
+        }
+        {
+            let _g = EnvGuard::with_value("1");
+            assert!(icaptcha_insecure_enabled(), "=1 must enable");
+        }
+        {
+            let _g = EnvGuard::with_value("true");
+            assert!(icaptcha_insecure_enabled(), "=true must enable");
+        }
+        {
+            let _g = EnvGuard::with_value("TRUE");
+            assert!(icaptcha_insecure_enabled(), "=TRUE must enable");
+        }
+        {
+            let _g = EnvGuard::with_value(" 1 ");
+            assert!(icaptcha_insecure_enabled(), "\" 1 \" (trimmed) must enable");
+        }
+    }
+
+    /// #227: `=0` / `=false` / empty / unset must NOT enable loopback HTTP;
+    /// only explicit truthy `1` / `true` may. Uses pure helpers so tests do
+    /// not mutate process-global environment (avoids races with parallel
+    /// resolver tests that also call `resolve_solver_url`).
+    #[test]
+    fn insecure_env_only_truthy_enables_loopback_http() {
+        assert!(
+            !is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(Some("0"))),
+            "=0 must leave loopback HTTP disabled"
+        );
+        assert!(!icaptcha_insecure_from_str(Some("0")));
+
+        assert!(
+            !is_https_with(
+                "http://127.0.0.1",
+                icaptcha_insecure_from_str(Some("false"))
+            ),
+            "=false must leave loopback HTTP disabled"
+        );
+        assert!(!icaptcha_insecure_from_str(Some("false")));
+
+        assert!(
+            !is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(Some(""))),
+            "empty value must leave loopback HTTP disabled"
+        );
+        assert!(!icaptcha_insecure_from_str(Some("")));
+
+        assert!(
+            !is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(None)),
+            "unset must leave loopback HTTP disabled"
+        );
+        assert!(!icaptcha_insecure_from_str(None));
+        assert!(!icaptcha_insecure_from_var(Err(
+            std::env::VarError::NotPresent
+        )));
+
+        assert!(
+            is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(Some("1"))),
+            "=1 must enable loopback HTTP"
+        );
+        assert!(is_https_with(
+            "http://localhost",
+            icaptcha_insecure_from_str(Some("1"))
+        ));
+        assert!(icaptcha_insecure_from_str(Some("1")));
+
+        assert!(
+            is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(Some("true"))),
+            "=true must enable loopback HTTP"
+        );
+        assert!(icaptcha_insecure_from_str(Some("true")));
+
+        assert!(
+            is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(Some("TRUE"))),
+            "=TRUE (case-insensitive) must enable loopback HTTP"
+        );
+
+        // Trim is load-bearing: whitespace around a truthy token must still enable.
+        assert!(
+            is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(Some(" 1 "))),
+            "\" 1 \" (trimmed) must enable loopback HTTP"
+        );
+        assert!(icaptcha_insecure_from_str(Some(" 1 ")));
+
+        // Present non-Unicode: fail closed + same fixed warning path as non-truthy.
+        assert!(!icaptcha_insecure_from_var(Err(
+            std::env::VarError::NotUnicode(OsString::from("x"))
+        )));
+
+        // https remains trusted regardless of the insecure flag.
+        assert!(is_https_with("https://icaptcha.gitlawb.com", false));
+        assert!(is_https_with("https://icaptcha.gitlawb.com", true));
+    }
+
     // ── P1: hostile error bodies must not reach the terminal raw ──────────
 
     #[test]
@@ -543,13 +733,12 @@ mod tests {
 
     #[test]
     fn rejects_insecure_advertised_url_on_different_port() {
-        // With GITLAWB_ICAPTCHA_INSECURE=1, two loopback URLs on different
+        // With insecure mode enabled, two loopback URLs on different
         // ports must be treated as distinct origins so a hostile node cannot
         // redirect the bearer key to a different listener on localhost.
-        let _env = InsecureEnv::new();
-
         let op = "http://localhost:3000";
-        let (url, key_trusted) = resolve_solver_url(Some("http://localhost:9000"), Some(op));
+        let (url, key_trusted) =
+            resolve_solver_url_with(Some("http://localhost:9000"), Some(op), true);
         assert_eq!(
             url, op,
             "must fall back to operator origin, not use different port"