From a07d1a2a8a0be033371ac5d9c6d716d904d6ff80 Mon Sep 17 00:00:00 2001
From: beardthelion <56458543+beardthelion@users.noreply.github.com>
Date: Sun, 28 Jun 2026 23:47:29 -0500
Subject: [PATCH 1/7] fix(node,git-remote): gate receive-pack advertisement,
 sign client fetch/push

The info/refs ref advertisement was gated only for service=git-upload-pack
(the visibility check sat inside `if service == "git-upload-pack"`), so an
anonymous GET /{owner}/{repo}.git/info/refs?service=git-receive-pack returned a
private repo's branch names and commit tips, bypassing the read gate. Run the
visibility "/" check for both services.

Now that the advertisement is gated for both services, git-remote-gitlawb has to
authenticate its own requests, or a private repo's owner can neither fetch nor
push it. Sign the Phase-1 advertisement GET and the Phase-2 pack POST for both
services over path_and_query when an identity keypair is present; public repos
stay anonymous. This also repairs a pre-existing break where an owner could not
fetch their own private repo, because the upload-pack POST was sent unsigned.

Tests: node-side gate (anon denied, owner-signed cleared, both services); a
real-signature seam proving the client's own signature verifies under the node's
verification primitives; real served-content withholding against an on-disk repo
(the owner gets the ref bytes, a denied caller's 404 omits them); and mockito
coverage that the client attaches and omits signatures correctly.
---
 Cargo.lock                              |  11 +-
 crates/git-remote-gitlawb/Cargo.toml    |   3 +
 crates/git-remote-gitlawb/src/main.rs   | 336 +++++++++++++++++++--
 crates/gitlawb-node/src/api/repos.rs    |  16 +-
 crates/gitlawb-node/src/test_support.rs | 377 ++++++++++++++++++++++++
 5 files changed, 703 insertions(+), 40 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index d42c370..ad47d0b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3300,10 +3300,11 @@ dependencies = [
 
 [[package]]
 name = "git-remote-gitlawb"
-version = "0.3.9"
+version = "0.4.0"
 dependencies = [
  "anyhow",
  "gitlawb-core",
+ "mockito",
  "reqwest",
  "tracing",
  "tracing-subscriber",
@@ -3311,7 +3312,7 @@ dependencies = [
 
 [[package]]
 name = "gitlawb-attest"
-version = "0.3.9"
+version = "0.4.0"
 dependencies = [
  "base64",
  "ed25519-dalek",
@@ -3328,7 +3329,7 @@ dependencies = [
 
 [[package]]
 name = "gitlawb-core"
-version = "0.3.9"
+version = "0.4.0"
 dependencies = [
  "anyhow",
  "base64",
@@ -3355,7 +3356,7 @@ dependencies = [
 
 [[package]]
 name = "gitlawb-node"
-version = "0.3.9"
+version = "0.4.0"
 dependencies = [
  "alloy",
  "anyhow",
@@ -3411,7 +3412,7 @@ dependencies = [
 
 [[package]]
 name = "gl"
-version = "0.3.9"
+version = "0.4.0"
 dependencies = [
  "alloy",
  "anyhow",
diff --git a/crates/git-remote-gitlawb/Cargo.toml b/crates/git-remote-gitlawb/Cargo.toml
index c772a52..695bd3b 100644
--- a/crates/git-remote-gitlawb/Cargo.toml
+++ b/crates/git-remote-gitlawb/Cargo.toml
@@ -16,3 +16,6 @@ anyhow = { workspace = true }
 reqwest = { workspace = true }
 tracing = { workspace = true }
 tracing-subscriber = { workspace = true }
+
+[dev-dependencies]
+mockito = "1"
diff --git a/crates/git-remote-gitlawb/src/main.rs b/crates/git-remote-gitlawb/src/main.rs
index 63c3b79..ad8bf9e 100644
--- a/crates/git-remote-gitlawb/src/main.rs
+++ b/crates/git-remote-gitlawb/src/main.rs
@@ -14,7 +14,10 @@
 //! # Protocol flow (connect capability)
 //!   capabilities → "connect\n\n"
 //!   connect git-upload-pack  → GET /info/refs | POST /git-upload-pack
-//!   connect git-receive-pack → GET /info/refs | POST /git-receive-pack (+ auth header)
+//!   connect git-receive-pack → GET /info/refs | POST /git-receive-pack
+//! When an identity keypair is present, both the advertisement GET and the pack
+//! POST are RFC-9421 signed for BOTH services, so the node authorizes the owner on
+//! visibility-gated private repos (fetch and push); public repos work anonymously.
 
 use anyhow::{bail, Context, Result};
 use gitlawb_core::http_sig::sign_request;
@@ -63,7 +66,9 @@ fn main() -> Result<()> {
     let repo_base = format!("{}/{}/{}", node_base, short_owner, repo_name);
     tracing::debug!("repo_base: {repo_base}");
 
-    // Load keypair for signing push requests (optional — push still works unsigned in v0.1)
+    // Load keypair for signing requests (optional). When present, the helper signs
+    // both fetch and push so a private repo's owner is authorized; absent, public
+    // repos still work and push falls back to unsigned (v0.1 local alpha only).
     let keypair = load_keypair();
 
     run_helper(&repo_base, keypair.as_ref())
@@ -115,7 +120,7 @@ fn help_text() -> String {
          \n\
          ENVIRONMENT:\n\
          \x20   GITLAWB_NODE   Node base URL (default: http://127.0.0.1:7545)\n\
-         \x20   GITLAWB_KEY    Identity PEM path for signed pushes (default: ~/.gitlawb/identity.pem)\n\
+         \x20   GITLAWB_KEY    Identity PEM path for signed fetch/push (default: ~/.gitlawb/identity.pem)\n\
          \x20   GITLAWB_LOG    Log filter (default: warn)\n\
          \n\
          FLAGS:\n\
@@ -201,9 +206,7 @@ fn handle_connect(
     let refs_url = format!("{}/info/refs?service={}", repo_base, service);
     tracing::debug!("GET {refs_url}");
 
-    let refs_resp = client
-        .get(&refs_url)
-        .header("User-Agent", "git/2.0 git-remote-gitlawb/0.1.0")
+    let refs_resp = build_advertisement_request(&client, &refs_url, keypair)
         .send()
         .with_context(|| format!("GET {refs_url}"))?;
 
@@ -266,29 +269,7 @@ fn handle_connect(
     let post_url = format!("{}/{}", repo_base, service);
     tracing::debug!("POST {post_url} ({} bytes)", request_body.len());
 
-    // Extract the URL path for signing (e.g., "/z6Mk.../my-repo/git-receive-pack")
-    let path_for_sig = url_path(&post_url);
-
-    let mut req = client
-        .post(&post_url)
-        .header("Content-Type", format!("application/x-{}-request", service))
-        .header("User-Agent", "git/2.0 git-remote-gitlawb/0.1.0");
-
-    // Add RFC 9421 HTTP Signature auth on push operations
-    if service == "git-receive-pack" {
-        if let Some(kp) = keypair {
-            let signed = sign_request(kp, "POST", &path_for_sig, &request_body);
-            req = req
-                .header("Content-Digest", signed.content_digest)
-                .header("Signature-Input", signed.signature_input)
-                .header("Signature", signed.signature);
-            tracing::debug!("attached RFC 9421 HTTP Signature (DID: {})", kp.did());
-        } else {
-            tracing::warn!(
-                "no identity keypair found — push will be unsigned (v0.1 local alpha only)"
-            );
-        }
-    }
+    let req = build_pack_post_request(&client, &post_url, service, &request_body, keypair);
 
     // Attach the body after signing so the pack bytes are moved, not cloned —
     // packs can be large and the clone doubled peak memory on push.
@@ -310,6 +291,67 @@ fn handle_connect(
     Ok(())
 }
 
+// ── Smart-protocol request builders ───────────────────────────────────────────
+
+const USER_AGENT: &str = "git/2.0 git-remote-gitlawb/0.1.0";
+
+/// Build the Phase-1 ref-advertisement GET, signing it when an identity is
+/// present. The node gates the ref advertisement on read visibility for BOTH
+/// services, so a private repo's advertisement is denied (404) to an
+/// unauthenticated caller — without this signature the repo's own owner can
+/// neither fetch (upload-pack) nor push (receive-pack) it. Public repos still
+/// work anonymously (no keypair present, or the gate admits anonymous). Sign over
+/// the path *and* query (?service=...) because the node verifies the signature
+/// over its path_and_query.
+fn build_advertisement_request(
+    client: &reqwest::blocking::Client,
+    refs_url: &str,
+    keypair: Option<&Keypair>,
+) -> reqwest::blocking::RequestBuilder {
+    let mut req = client.get(refs_url).header("User-Agent", USER_AGENT);
+    if let Some(kp) = keypair {
+        let signed = sign_request(kp, "GET", &url_path(refs_url), b"");
+        req = req
+            .header("Content-Digest", signed.content_digest)
+            .header("Signature-Input", signed.signature_input)
+            .header("Signature", signed.signature);
+        tracing::debug!("signed info/refs advertisement (DID: {})", kp.did());
+    }
+    req
+}
+
+/// Build the Phase-2 pack POST, signing it when an identity is present, for BOTH
+/// services. The node read-gates the git-upload-pack POST as well as the
+/// git-receive-pack POST (each runs visibility_check at "/"), and the Phase-1
+/// advertisement signature does not carry to this separate request — so a private
+/// repo's owner must authenticate here too or their fetch/push is denied.
+/// Public-repo fetch still works anonymously when no keypair is present. The body
+/// is signed (content-digest) but NOT attached here, so the caller can move the
+/// (possibly large) pack bytes into `.body()` rather than clone them.
+fn build_pack_post_request(
+    client: &reqwest::blocking::Client,
+    post_url: &str,
+    service: &str,
+    body: &[u8],
+    keypair: Option<&Keypair>,
+) -> reqwest::blocking::RequestBuilder {
+    let mut req = client
+        .post(post_url)
+        .header("Content-Type", format!("application/x-{}-request", service))
+        .header("User-Agent", USER_AGENT);
+    if let Some(kp) = keypair {
+        let signed = sign_request(kp, "POST", &url_path(post_url), body);
+        req = req
+            .header("Content-Digest", signed.content_digest)
+            .header("Signature-Input", signed.signature_input)
+            .header("Signature", signed.signature);
+        tracing::debug!("signed {service} POST (DID: {})", kp.did());
+    } else if service == "git-receive-pack" {
+        tracing::warn!("no identity keypair found — push will be unsigned (v0.1 local alpha only)");
+    }
+    req
+}
+
 // ── URL helpers ───────────────────────────────────────────────────────────────
 
 /// Parse a `gitlawb://did:key:z6Mk.../repo-name` URL.
@@ -477,6 +519,227 @@ fn resolve_key_path() -> std::path::PathBuf {
 mod tests {
     use super::*;
 
+    /// The regression that round-1 missed: the Phase-2 `git-upload-pack` POST was
+    /// left unsigned, so an owner's fetch of a private repo cleared the (now signed)
+    /// advertisement and then 404'd on the pack POST. Drive BOTH request builders
+    /// against a mock node and assert the RFC-9421 headers actually go out on the
+    /// wire for BOTH services when a keypair is present (the mock only matches when
+    /// the headers exist, so `.assert()` fails if any are dropped).
+    #[test]
+    fn advertisement_and_pack_post_are_signed_for_both_services_with_keypair() {
+        let kp = Keypair::generate();
+        let client = reqwest::blocking::Client::new();
+        let body = b"0009done\n".to_vec();
+
+        for service in ["git-upload-pack", "git-receive-pack"] {
+            let mut server = mockito::Server::new();
+            let repo_base = format!("{}/zOwner/myrepo", server.url());
+
+            let get_mock = server
+                .mock("GET", mockito::Matcher::Regex(r"/info/refs".to_string()))
+                .match_header("signature", mockito::Matcher::Any)
+                .match_header("signature-input", mockito::Matcher::Any)
+                .match_header("content-digest", mockito::Matcher::Any)
+                .with_status(200)
+                .create();
+            let refs_url = format!("{repo_base}/info/refs?service={service}");
+            let resp = build_advertisement_request(&client, &refs_url, Some(&kp))
+                .send()
+                .unwrap();
+            assert!(resp.status().is_success());
+            get_mock.assert();
+
+            let post_mock = server
+                .mock("POST", mockito::Matcher::Regex(format!("/{service}$")))
+                .match_header("signature", mockito::Matcher::Any)
+                .match_header("signature-input", mockito::Matcher::Any)
+                .match_header("content-digest", mockito::Matcher::Any)
+                .with_status(200)
+                .create();
+            let post_url = format!("{repo_base}/{service}");
+            let resp = build_pack_post_request(&client, &post_url, service, &body, Some(&kp))
+                .body(body.clone())
+                .send()
+                .unwrap();
+            assert!(resp.status().is_success());
+            post_mock.assert();
+        }
+    }
+
+    /// Without an identity, the pack POST must go out UNSIGNED so public fetch (and
+    /// alpha unsigned push) still work. `Matcher::Missing` matches only when the
+    /// header is absent, so the mock is hit only if NO signature was attached — for
+    /// BOTH services (the receive-pack iteration also exercises the no-keypair warn
+    /// branch and confirms it does not block the request).
+    #[test]
+    fn pack_post_is_unsigned_without_keypair_for_both_services() {
+        let client = reqwest::blocking::Client::new();
+        let body = b"0009done\n".to_vec();
+
+        for service in ["git-upload-pack", "git-receive-pack"] {
+            let mut server = mockito::Server::new();
+            let post_url = format!("{}/zOwner/myrepo/{service}", server.url());
+
+            let post_mock = server
+                .mock("POST", mockito::Matcher::Regex(format!("/{service}$")))
+                .match_header("signature", mockito::Matcher::Missing)
+                .match_header("signature-input", mockito::Matcher::Missing)
+                .match_header("content-digest", mockito::Matcher::Missing)
+                .with_status(200)
+                .create();
+            let resp = build_pack_post_request(&client, &post_url, service, &body, None)
+                .body(body.clone())
+                .send()
+                .unwrap();
+            assert!(resp.status().is_success());
+            post_mock.assert();
+        }
+    }
+
+    /// Symmetric negative case for Phase 1: with no identity the advertisement GET
+    /// carries no signature, so public clones stay anonymous.
+    #[test]
+    fn advertisement_get_is_unsigned_without_keypair() {
+        let client = reqwest::blocking::Client::new();
+        let mut server = mockito::Server::new();
+        let refs_url = format!(
+            "{}/zOwner/myrepo/info/refs?service=git-upload-pack",
+            server.url()
+        );
+
+        let get_mock = server
+            .mock("GET", mockito::Matcher::Regex(r"/info/refs".to_string()))
+            .match_header("signature", mockito::Matcher::Missing)
+            .match_header("signature-input", mockito::Matcher::Missing)
+            .match_header("content-digest", mockito::Matcher::Missing)
+            .with_status(200)
+            .create();
+        let resp = build_advertisement_request(&client, &refs_url, None)
+            .send()
+            .unwrap();
+        assert!(resp.status().is_success());
+        get_mock.assert();
+    }
+
+    /// Cross-crate seam: the signature the CLIENT actually emits (via the real
+    /// request builders) verifies under the SAME `gitlawb_core::http_sig`
+    /// primitives the node's `require_signature` uses, over the `@path` the client
+    /// genuinely transmits (derived from the built reqwest request, exactly as the
+    /// node derives it from `uri.path_and_query()`). A tampered `@path` must fail —
+    /// this is the byte-match the whole A1 client fix depends on, now executed end
+    /// to end (sign here, verify with the node's verifier), not reasoned.
+    #[test]
+    fn client_signature_verifies_under_node_verification_for_both_services() {
+        use gitlawb_core::http_sig::{
+            build_signing_string, compute_content_digest, HttpSignature, COVERED_COMPONENTS,
+        };
+        use gitlawb_core::identity::verify;
+        use std::collections::HashMap;
+
+        let kp = Keypair::generate();
+        let client = reqwest::blocking::Client::new();
+        let body = b"0009done\n".to_vec();
+
+        // Re-implements the node's require_signature verification (auth/mod.rs):
+        // parse headers, recompute content-digest from the body, rebuild the signing
+        // string over @method/@path/content-digest, Ed25519-verify. Ok iff the node
+        // would accept it.
+        let node_verifies = |method: &str,
+                             path_and_query: &str,
+                             body: &[u8],
+                             sig_input: &str,
+                             sig_header: &str,
+                             content_digest: &str|
+         -> anyhow::Result<()> {
+            let sig = HttpSignature::parse(sig_input, sig_header)?;
+            sig.check_created()?;
+            assert!(
+                sig.missing_components().is_empty(),
+                "signature must cover all required components"
+            );
+            assert_eq!(sig.alg, "ed25519");
+            assert_eq!(
+                content_digest,
+                compute_content_digest(body),
+                "content-digest must match the body"
+            );
+            let vk = sig.key_id.to_verifying_key()?;
+            let mut values = HashMap::new();
+            values.insert("@method".to_string(), method.to_uppercase());
+            values.insert("@path".to_string(), path_and_query.to_string());
+            values.insert("content-digest".to_string(), content_digest.to_string());
+            let sig_params_value = sig_input.strip_prefix("sig1=").unwrap_or(sig_input);
+            let components: Vec<&str> = sig.components.iter().map(String::as_str).collect();
+            let signing_string = build_signing_string(&components, sig_params_value, &values)?;
+            let sig_array: [u8; 64] = sig.signature_bytes.as_slice().try_into()?;
+            verify(&vk, signing_string.as_bytes(), &sig_array)?;
+            let _ = COVERED_COMPONENTS;
+            Ok(())
+        };
+        // @path exactly as the node reconstructs it from the request it receives.
+        let path_and_query = |req: &reqwest::blocking::Request| match req.url().query() {
+            Some(q) => format!("{}?{}", req.url().path(), q),
+            None => req.url().path().to_string(),
+        };
+        let header = |req: &reqwest::blocking::Request, name: &str| {
+            req.headers()
+                .get(name)
+                .unwrap_or_else(|| panic!("missing {name}"))
+                .to_str()
+                .unwrap()
+                .to_string()
+        };
+
+        for service in ["git-upload-pack", "git-receive-pack"] {
+            // Phase-1 advertisement GET (empty body).
+            let refs_url = format!("http://node.example/zOwner/myrepo/info/refs?service={service}");
+            let req = build_advertisement_request(&client, &refs_url, Some(&kp))
+                .build()
+                .unwrap();
+            node_verifies(
+                "GET",
+                &path_and_query(&req),
+                b"",
+                &header(&req, "signature-input"),
+                &header(&req, "signature"),
+                &header(&req, "content-digest"),
+            )
+            .expect("client GET signature must verify under the node's verifier");
+
+            // Phase-2 pack POST (real body).
+            let post_url = format!("http://node.example/zOwner/myrepo/{service}");
+            let req = build_pack_post_request(&client, &post_url, service, &body, Some(&kp))
+                .body(body.clone())
+                .build()
+                .unwrap();
+            let pq = path_and_query(&req);
+            node_verifies(
+                "POST",
+                &pq,
+                &body,
+                &header(&req, "signature-input"),
+                &header(&req, "signature"),
+                &header(&req, "content-digest"),
+            )
+            .expect("client POST signature must verify under the node's verifier");
+
+            // Negative: a tampered @path (the A1 attack surface) must NOT verify.
+            let tampered = pq.replace(service, "git-evil");
+            assert!(
+                node_verifies(
+                    "POST",
+                    &tampered,
+                    &body,
+                    &header(&req, "signature-input"),
+                    &header(&req, "signature"),
+                    &header(&req, "content-digest"),
+                )
+                .is_err(),
+                "a tampered @path must fail verification"
+            );
+        }
+    }
+
     #[test]
     fn parse_standard_url() {
         let (did, owner, repo) = parse_gitlawb_url("gitlawb://did:key:z6MkFoo123/my-repo").unwrap();
@@ -524,6 +787,21 @@ mod tests {
         );
     }
 
+    #[test]
+    fn url_path_preserves_query_for_signed_advertisement() {
+        // The Phase-1 advertisement GET is signed over url_path(refs_url), and the
+        // node verifies the signature over its path_and_query — so the ?service=
+        // query MUST survive verbatim or the signatures will not byte-match.
+        assert_eq!(
+            url_path("http://127.0.0.1:7545/z6Mk/myrepo/info/refs?service=git-upload-pack"),
+            "/z6Mk/myrepo/info/refs?service=git-upload-pack"
+        );
+        assert_eq!(
+            url_path("http://127.0.0.1:7545/z6Mk/myrepo/info/refs?service=git-receive-pack"),
+            "/z6Mk/myrepo/info/refs?service=git-receive-pack"
+        );
+    }
+
     fn args(items: &[&str]) -> Vec<String> {
         items.iter().map(|s| s.to_string()).collect()
     }
diff --git a/crates/gitlawb-node/src/api/repos.rs b/crates/gitlawb-node/src/api/repos.rs
index b74f3f6..6674af5 100644
--- a/crates/gitlawb-node/src/api/repos.rs
+++ b/crates/gitlawb-node/src/api/repos.rs
@@ -482,7 +482,7 @@ pub async fn get_tree(
 
 // ── Git smart HTTP endpoints ──────────────────────────────────────────────
 
-/// GET /:owner/:repo.git/info/refs?service=git-upload-pack
+/// GET /:owner/:repo.git/info/refs?service=git-upload-pack|git-receive-pack
 pub async fn git_info_refs(
     State(state): State<AppState>,
     Path((owner, repo)): Path<(String, String)>,
@@ -508,10 +508,14 @@ pub async fn git_info_refs(
         .ok_or_else(|| AppError::BadRequest("missing ?service= parameter".into()))?;
     tracing::debug!(service = %service, repo = %name, "info/refs service");
 
-    // Enforce read (clone/fetch) visibility. The push advertisement
-    // (service=git-receive-pack) is authorized separately on the
-    // git-receive-pack POST, so leave it untouched here.
-    if service == "git-upload-pack" {
+    // Enforce read visibility on the ref advertisement, for BOTH services. The
+    // upload-pack (clone/fetch) and receive-pack (push) advertisements expose the
+    // same ref metadata — branch/tag names and commit tips — so a private repo's
+    // advertisement must be withheld from a non-reader regardless of which service
+    // is requested. The push itself stays separately owner-gated on the
+    // git-receive-pack POST; read access is a strict subset of push access, so a
+    // legitimate pusher (the owner) always clears this gate.
+    {
         let rules = state.db.list_visibility_rules(&record.id).await?;
         let caller = auth.as_ref().map(|e| e.0 .0.as_str());
         // Subtree (mode B) rules do not gate the advertisement: refs expose commit
@@ -519,7 +523,7 @@ pub async fn git_info_refs(
         if visibility_check(&rules, record.is_public, &record.owner_did, caller, "/")
             == Decision::Deny
         {
-            tracing::debug!(repo = %name, caller = ?caller, "info/refs read denied by visibility");
+            tracing::debug!(repo = %name, caller = ?caller, service = %service, "info/refs read denied by visibility");
             return Err(AppError::RepoNotFound(format!("{owner}/{name}")));
         }
     }
diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs
index f7e7f76..0da33e8 100644
--- a/crates/gitlawb-node/src/test_support.rs
+++ b/crates/gitlawb-node/src/test_support.rs
@@ -669,6 +669,383 @@ mod tests {
         );
     }
 
+    /// A1 read-gate bypass + its client remedy. `git_info_refs` serves BOTH the
+    /// `git-upload-pack` (clone/fetch) and `git-receive-pack` (push) ref
+    /// advertisement off one route, but the visibility gate was wrapped in
+    /// `if service == "git-upload-pack"`, so a private repo's ref advertisement
+    /// (branch/tag names + commit tips) leaked to any anonymous caller who asked
+    /// for `?service=git-receive-pack`. The fix gates the advertisement for both
+    /// services. Because the gate now denies an *unauthenticated* advertisement
+    /// of a private repo for both services, `git-remote-gitlawb` signs its
+    /// Phase-1 advertisement GET (over path_and_query) so the owner can still
+    /// fetch and push; this test exercises that exact request with a REAL
+    /// RFC-9421 signature through the production `optional_signature` middleware.
+    ///
+    /// Denied → 404 (`RepoNotFound`, existence-hiding) at the gate, before disk
+    /// access. Allowed → the handler clears the gate and falls through to
+    /// `acquire` + real `git ... --advertise-refs` against a repo absent from the
+    /// test disk, returning 500; that 500 (anything but 404) is the signal the
+    /// caller cleared the gate.
+    #[sqlx::test]
+    async fn git_info_refs_gates_advertisement_for_both_services(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();
+        // Short owner form in the URL so the signed @path and the node's
+        // path_and_query() match byte-for-byte; get_repo's owner LIKE + did_matches
+        // still authorize the full-DID signer as the owner.
+        let short = owner_did.split(':').next_back().unwrap().to_string();
+        let state = test_state(pool).await;
+
+        let mut priv_repo = seed_repo(&owner_did, "ir-priv");
+        priv_repo.is_public = false;
+        state
+            .db
+            .create_repo(&priv_repo)
+            .await
+            .expect("seed private repo");
+        // A public repo to guard against the unconditional gate accidentally
+        // denying public, anonymous clones.
+        state
+            .db
+            .create_repo(&seed_repo(&owner_did, "ir-pub"))
+            .await
+            .expect("seed public repo");
+
+        // Production-shaped router: the real optional_signature middleware, so a
+        // signed request is genuinely verified (not the injected-DID shortcut).
+        let router = || {
+            Router::new()
+                .route(
+                    "/{owner}/{repo}/info/refs",
+                    axum::routing::get(crate::api::repos::git_info_refs),
+                )
+                .layer(axum::middleware::from_fn(crate::auth::optional_signature))
+                .with_state(state.clone())
+        };
+        let path = |service: &str| format!("/{short}/ir-priv.git/info/refs?service={service}");
+        let anon = |service: &str| {
+            Request::builder()
+                .method(Method::GET)
+                .uri(path(service))
+                .body(Body::empty())
+                .unwrap()
+        };
+        // The advertisement GET exactly as git-remote-gitlawb now builds it: a
+        // real signature over the path_and_query, empty body.
+        let signed = |service: &str| {
+            let p = path(service);
+            let s = sign_request(&kp, "GET", &p, b"");
+            Request::builder()
+                .method(Method::GET)
+                .uri(&p)
+                .header("content-digest", s.content_digest)
+                .header("signature-input", s.signature_input)
+                .header("signature", s.signature)
+                .body(Body::empty())
+                .unwrap()
+        };
+
+        // Leak fix: anonymous advertisement of a private repo is denied (404) for
+        // BOTH services. Pre-fix the receive-pack case returned 500 (gate skipped).
+        for service in ["git-upload-pack", "git-receive-pack"] {
+            let resp = router().oneshot(anon(service)).await.unwrap();
+            assert_eq!(
+                resp.status(),
+                StatusCode::NOT_FOUND,
+                "anonymous {service} advertisement of a private repo must be denied"
+            );
+        }
+
+        // No-regression: a PUBLIC repo's advertisement stays anonymous for BOTH
+        // services. The gate admits the anonymous caller, so the handler clears it
+        // and 500s on the missing test-disk repo — anything but 404 (a gate denial)
+        // proves the unconditional gate did not accidentally lock out public reads.
+        for service in ["git-upload-pack", "git-receive-pack"] {
+            let req = Request::builder()
+                .method(Method::GET)
+                .uri(format!("/{short}/ir-pub.git/info/refs?service={service}"))
+                .body(Body::empty())
+                .unwrap();
+            let resp = router().oneshot(req).await.unwrap();
+            // 500 (not just non-404): the gate admits the public anonymous caller,
+            // so the handler reaches acquire + git advertise-refs on the missing
+            // test-disk repo. Pinning the exact 500 rules out a 401/403 regression
+            // masquerading as "not gated".
+            assert_eq!(
+                resp.status(),
+                StatusCode::INTERNAL_SERVER_ERROR,
+                "anonymous {service} advertisement of a PUBLIC repo must not be gated"
+            );
+        }
+
+        // Client remedy: the owner's SIGNED advertisement GET clears the gate for
+        // BOTH services (so fetch and push of a private repo keep working). It
+        // 500s on the missing test-disk repo — anything but 404 means cleared.
+        for service in ["git-upload-pack", "git-receive-pack"] {
+            let resp = router().oneshot(signed(service)).await.unwrap();
+            // INTERNAL_SERVER_ERROR specifically: the signature VERIFIED (passed
+            // require_signature, not 401/403) and the owner cleared the read gate
+            // (not 404), so the handler proceeded to acquire + git on a repo absent
+            // from the test disk. Asserting the exact 500 — rather than merely
+            // "not 404" — proves the request got PAST auth, not rejected by it.
+            assert_eq!(
+                resp.status(),
+                StatusCode::INTERNAL_SERVER_ERROR,
+                "the owner's signed {service} advertisement must verify and clear the gate"
+            );
+        }
+    }
+
+    /// A1 Phase-2 contract: the `git-upload-pack` POST (the actual fetch, after
+    /// the advertisement) is itself read-visibility gated. An ANONYMOUS upload-pack
+    /// POST against a private repo is denied (404) — so signing only the Phase-1
+    /// advertisement GET is NOT enough; `git-remote-gitlawb` must also sign this
+    /// POST, or an owner's fetch of their own private repo clears the advertisement
+    /// and then dies on the pack POST. A real owner signature clears the gate
+    /// (non-404; the missing test-disk repo then errors downstream).
+    #[sqlx::test]
+    async fn git_upload_pack_post_is_read_gated_on_private_repo(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 short = owner_did.split(':').next_back().unwrap().to_string();
+        let state = test_state(pool).await;
+
+        let mut priv_repo = seed_repo(&owner_did, "up-priv");
+        priv_repo.is_public = false;
+        state
+            .db
+            .create_repo(&priv_repo)
+            .await
+            .expect("seed private repo");
+
+        let router = || {
+            Router::new()
+                .route(
+                    "/{owner}/{repo}/git-upload-pack",
+                    axum::routing::post(crate::api::repos::git_upload_pack),
+                )
+                .layer(axum::middleware::from_fn(crate::auth::optional_signature))
+                .with_state(state.clone())
+        };
+        // A non-empty body (git-remote-gitlawb skips the POST when the body is empty).
+        let body = b"0032want 0000000000000000000000000000000000000000\n".to_vec();
+        let path = format!("/{short}/up-priv.git/git-upload-pack");
+
+        // Anonymous Phase-2 fetch of a private repo: denied at the gate (404). This
+        // is exactly the request git-remote-gitlawb sends today for upload-pack —
+        // the unsigned POST — which is why fetch breaks for the owner.
+        let anon = Request::builder()
+            .method(Method::POST)
+            .uri(&path)
+            .header("content-type", "application/x-git-upload-pack-request")
+            .body(Body::from(body.clone()))
+            .unwrap();
+        let resp = router().oneshot(anon).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::NOT_FOUND,
+            "an anonymous upload-pack POST against a private repo must be denied"
+        );
+
+        // The same POST signed by the owner clears the read gate (non-404). This is
+        // the request the client must send once it signs the upload-pack POST.
+        let signed = sign_request(&kp, "POST", &path, &body);
+        let signed_req = Request::builder()
+            .method(Method::POST)
+            .uri(&path)
+            .header("content-type", "application/x-git-upload-pack-request")
+            .header("content-digest", signed.content_digest)
+            .header("signature-input", signed.signature_input)
+            .header("signature", signed.signature)
+            .body(Body::from(body))
+            .unwrap();
+        let resp = router().oneshot(signed_req).await.unwrap();
+        // 500 (not merely non-404): the signature VERIFIED (passed require_signature,
+        // not 401/403) AND the owner cleared the read gate (not 404), so the handler
+        // reached git on the missing test-disk repo. Pinning 500 proves the request
+        // got past auth — a 401 regression would slip through a bare `!= 404`.
+        assert_eq!(
+            resp.status(),
+            StatusCode::INTERNAL_SERVER_ERROR,
+            "the owner's signed upload-pack POST must verify and clear the read gate"
+        );
+    }
+
+    /// Served-content seam: with a REAL on-disk bare repo (branch
+    /// `topsecret-branch`), the advertisement serves the actual ref names to
+    /// authorized callers and withholds them from denied ones — proving real
+    /// content egress + withholding, not just the gate decision (the other tests
+    /// land on a 500 from a missing-disk repo). Asserts the branch name appears for
+    /// allowed callers and never appears in a denied 404 body.
+    #[sqlx::test]
+    async fn advertisement_serves_real_refs_only_to_authorized_callers(pool: PgPool) {
+        use gitlawb_core::http_sig::sign_request;
+        use gitlawb_core::identity::Keypair;
+        use std::process::Command;
+
+        let kp = Keypair::generate();
+        let owner_did = kp.did().to_string();
+        let short = owner_did.split(':').next_back().unwrap().to_string();
+        // repo_store::for_testing uses /tmp; local_path = /tmp/<slug>/<name>.git
+        // with slug = owner_did with ':' and '/' replaced by '_'.
+        let slug = owner_did.replace([':', '/'], "_");
+        let state = test_state(pool).await;
+
+        let run = |args: &[&str], cwd: &std::path::Path| {
+            let out = Command::new("git")
+                .args(args)
+                .current_dir(cwd)
+                .output()
+                .expect("git runs");
+            assert!(
+                out.status.success(),
+                "git {args:?} failed: {}",
+                String::from_utf8_lossy(&out.stderr)
+            );
+        };
+
+        // Source repo with a recognizable branch + one commit.
+        let src = std::env::temp_dir().join(format!("gl-seam-src-{short}"));
+        let _ = std::fs::remove_dir_all(&src);
+        std::fs::create_dir_all(&src).unwrap();
+        run(&["init", "-q", "-b", "topsecret-branch"], &src);
+        run(&["config", "user.email", "t@t"], &src);
+        run(&["config", "user.name", "t"], &src);
+        std::fs::write(src.join("f.txt"), b"hi").unwrap();
+        run(&["add", "f.txt"], &src);
+        run(&["commit", "-q", "-m", "seed"], &src);
+
+        // Bare-clone into the exact path repo_store.acquire() will read.
+        let bare_for = |name: &str| {
+            let dir = std::path::PathBuf::from("/tmp")
+                .join(&slug)
+                .join(format!("{name}.git"));
+            let _ = std::fs::remove_dir_all(&dir);
+            std::fs::create_dir_all(dir.parent().unwrap()).unwrap();
+            let out = Command::new("git")
+                .args([
+                    "clone",
+                    "--bare",
+                    "-q",
+                    src.to_str().unwrap(),
+                    dir.to_str().unwrap(),
+                ])
+                .output()
+                .expect("git clone runs");
+            assert!(
+                out.status.success(),
+                "bare clone failed: {}",
+                String::from_utf8_lossy(&out.stderr)
+            );
+            dir
+        };
+        let pub_dir = bare_for("served-pub");
+        let priv_dir = bare_for("served-priv");
+
+        state
+            .db
+            .create_repo(&seed_repo(&owner_did, "served-pub"))
+            .await
+            .expect("seed public repo");
+        let mut priv_repo = seed_repo(&owner_did, "served-priv");
+        priv_repo.is_public = false;
+        state
+            .db
+            .create_repo(&priv_repo)
+            .await
+            .expect("seed private repo");
+
+        let router = || {
+            Router::new()
+                .route(
+                    "/{owner}/{repo}/info/refs",
+                    axum::routing::get(crate::api::repos::git_info_refs),
+                )
+                .layer(axum::middleware::from_fn(crate::auth::optional_signature))
+                .with_state(state.clone())
+        };
+        async fn body_of(resp: axum::response::Response) -> String {
+            let b = axum::body::to_bytes(resp.into_body(), usize::MAX)
+                .await
+                .unwrap();
+            String::from_utf8_lossy(&b).to_string()
+        }
+
+        // Public repo, anonymous → 200 and the real ref name is served.
+        let resp = router()
+            .oneshot(
+                Request::builder()
+                    .method(Method::GET)
+                    .uri(format!(
+                        "/{short}/served-pub.git/info/refs?service=git-upload-pack"
+                    ))
+                    .body(Body::empty())
+                    .unwrap(),
+            )
+            .await
+            .unwrap();
+        assert_eq!(resp.status(), StatusCode::OK);
+        assert!(
+            body_of(resp).await.contains("topsecret-branch"),
+            "public advertisement must serve the real ref name"
+        );
+
+        // Private repo, anonymous → 404 and the ref name is withheld.
+        let resp = router()
+            .oneshot(
+                Request::builder()
+                    .method(Method::GET)
+                    .uri(format!(
+                        "/{short}/served-priv.git/info/refs?service=git-upload-pack"
+                    ))
+                    .body(Body::empty())
+                    .unwrap(),
+            )
+            .await
+            .unwrap();
+        assert_eq!(resp.status(), StatusCode::NOT_FOUND);
+        assert!(
+            !body_of(resp).await.contains("topsecret-branch"),
+            "a denied 404 must not leak the real ref name"
+        );
+
+        // Private repo, owner's REAL signature → 200 and the real ref is served.
+        let path = format!("/{short}/served-priv.git/info/refs?service=git-upload-pack");
+        let s = sign_request(&kp, "GET", &path, b"");
+        let resp = router()
+            .oneshot(
+                Request::builder()
+                    .method(Method::GET)
+                    .uri(&path)
+                    .header("content-digest", s.content_digest)
+                    .header("signature-input", s.signature_input)
+                    .header("signature", s.signature)
+                    .body(Body::empty())
+                    .unwrap(),
+            )
+            .await
+            .unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::OK,
+            "the owner's signed request reads the private advertisement"
+        );
+        assert!(
+            body_of(resp).await.contains("topsecret-branch"),
+            "the verified owner gets the real ref name"
+        );
+
+        let _ = std::fs::remove_dir_all(&src);
+        let _ = std::fs::remove_dir_all(&pub_dir);
+        let _ = std::fs::remove_dir_all(&priv_dir);
+    }
+
     // ── #97: repo-listing surfaces are visibility-gated ──────────────────────
 
     fn seed_private_repo(owner_did: &str, name: &str) -> RepoRecord {

From 422e1c4a955fb392a128510120b0883b5dae0a7b Mon Sep 17 00:00:00 2001
From: beardthelion <56458543+beardthelion@users.noreply.github.com>
Date: Mon, 29 Jun 2026 12:14:28 -0500
Subject: [PATCH 2/7] docs(node,git-remote): correct auth-contract comments on
 receive-pack gating (#119)

receive-pack POST is signature-required and separately owner-gated, not
visibility_check-gated; push implies read, not read-subset-of-push.
---
 crates/git-remote-gitlawb/src/main.rs | 9 +++++----
 crates/gitlawb-node/src/api/repos.rs  | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/crates/git-remote-gitlawb/src/main.rs b/crates/git-remote-gitlawb/src/main.rs
index ad8bf9e..ece9917 100644
--- a/crates/git-remote-gitlawb/src/main.rs
+++ b/crates/git-remote-gitlawb/src/main.rs
@@ -321,10 +321,11 @@ fn build_advertisement_request(
 }
 
 /// Build the Phase-2 pack POST, signing it when an identity is present, for BOTH
-/// services. The node read-gates the git-upload-pack POST as well as the
-/// git-receive-pack POST (each runs visibility_check at "/"), and the Phase-1
-/// advertisement signature does not carry to this separate request — so a private
-/// repo's owner must authenticate here too or their fetch/push is denied.
+/// services. The node read-gates the git-upload-pack POST with visibility_check
+/// at "/", and separately owner-gates the git-receive-pack POST (signature
+/// required, enforced by middleware). The Phase-1 advertisement signature does
+/// not carry to this separate request, so a private repo's owner must
+/// authenticate here too or their fetch/push is denied.
 /// Public-repo fetch still works anonymously when no keypair is present. The body
 /// is signed (content-digest) but NOT attached here, so the caller can move the
 /// (possibly large) pack bytes into `.body()` rather than clone them.
diff --git a/crates/gitlawb-node/src/api/repos.rs b/crates/gitlawb-node/src/api/repos.rs
index 6674af5..5d3883f 100644
--- a/crates/gitlawb-node/src/api/repos.rs
+++ b/crates/gitlawb-node/src/api/repos.rs
@@ -513,7 +513,7 @@ pub async fn git_info_refs(
     // same ref metadata — branch/tag names and commit tips — so a private repo's
     // advertisement must be withheld from a non-reader regardless of which service
     // is requested. The push itself stays separately owner-gated on the
-    // git-receive-pack POST; read access is a strict subset of push access, so a
+    // git-receive-pack POST; push access implies read access here, so a
     // legitimate pusher (the owner) always clears this gate.
     {
         let rules = state.db.list_visibility_rules(&record.id).await?;

From 33cb8351cb6467af22bf729f89b692cda35178b7 Mon Sep 17 00:00:00 2001
From: beardthelion <56458543+beardthelion@users.noreply.github.com>
Date: Mon, 29 Jun 2026 12:40:27 -0500
Subject: [PATCH 3/7] test(node): assert unsigned receive-pack POST is rejected
 (#119)

No test covered the signature-required half of the receive-pack auth
contract. Wire the POST route behind require_signature as production does and
assert an unsigned POST returns 401 before reaching the handler; 401 (not the
handler's 404/500) proves require_signature rejected it pre-handler.
---
 crates/gitlawb-node/src/test_support.rs | 39 +++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs
index 0da33e8..e271170 100644
--- a/crates/gitlawb-node/src/test_support.rs
+++ b/crates/gitlawb-node/src/test_support.rs
@@ -799,6 +799,45 @@ mod tests {
         }
     }
 
+    /// Push is signature-gated, not merely owner-gated: an UNSIGNED
+    /// git-receive-pack POST is rejected by `require_signature` (401) before
+    /// reaching `git_receive_pack`. 401 (not the handler's 404/500) is the
+    /// discriminator that proves the request never reached the handler.
+    #[sqlx::test]
+    async fn unsigned_receive_pack_post_is_rejected(pool: PgPool) {
+        let state = test_state(pool).await;
+        let owner_did = Keypair::generate().did().to_string();
+        let short = owner_did.split(':').next_back().unwrap().to_string();
+        state
+            .db
+            .create_repo(&seed_repo(&owner_did, "rp-repo"))
+            .await
+            .expect("seed repo");
+
+        // Production wiring: the receive-pack POST sits behind require_signature
+        // (server.rs add_auth_layers); apply that same layer here.
+        let router = Router::new()
+            .route(
+                "/{owner}/{repo}/git-receive-pack",
+                axum::routing::post(crate::api::repos::git_receive_pack),
+            )
+            .layer(axum::middleware::from_fn(crate::auth::require_signature))
+            .with_state(state);
+
+        let req = Request::builder()
+            .method(Method::POST)
+            .uri(format!("/{short}/rp-repo.git/git-receive-pack"))
+            .body(Body::from(&b"0000"[..]))
+            .unwrap();
+        let resp = router.oneshot(req).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::UNAUTHORIZED,
+            "an unsigned receive-pack POST must be rejected by require_signature, \
+             not reach the handler"
+        );
+    }
+
     /// A1 Phase-2 contract: the `git-upload-pack` POST (the actual fetch, after
     /// the advertisement) is itself read-visibility gated. An ANONYMOUS upload-pack
     /// POST against a private repo is denied (404) — so signing only the Phase-1

From 88a737089671501919da335b811c00508f42f69f Mon Sep 17 00:00:00 2001
From: beardthelion <56458543+beardthelion@users.noreply.github.com>
Date: Mon, 29 Jun 2026 20:59:00 -0500
Subject: [PATCH 4/7] test(node): RAII-clean the served-refs test's temp dirs
 on panic (#119)

advertisement_serves_real_refs_only_to_authorized_callers created /tmp/<slug>/<name>.git and /tmp/gl-seam-src-<short> and only removed them after the final assertion, leaking those dirs on the runner when an earlier assertion failed. Wrap each known path in a DirGuard whose Drop calls remove_dir_all, bound right after creation, so cleanup runs on success or panic. tempfile::TempDir does not fit here because repo_store::for_testing fixes the path layout.
---
 crates/gitlawb-node/src/test_support.rs | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs
index e271170..332c781 100644
--- a/crates/gitlawb-node/src/test_support.rs
+++ b/crates/gitlawb-node/src/test_support.rs
@@ -928,6 +928,17 @@ mod tests {
         use gitlawb_core::identity::Keypair;
         use std::process::Command;
 
+        // repo_store::for_testing fixes the on-disk layout (/tmp/<slug>/<name>.git
+        // and /tmp/gl-seam-src-<short>), so tempfile::TempDir's random paths don't
+        // fit. Wrap each known path in a Drop guard so the dirs are removed even if
+        // an assertion below panics.
+        struct DirGuard(std::path::PathBuf);
+        impl Drop for DirGuard {
+            fn drop(&mut self) {
+                let _ = std::fs::remove_dir_all(&self.0);
+            }
+        }
+
         let kp = Keypair::generate();
         let owner_did = kp.did().to_string();
         let short = owner_did.split(':').next_back().unwrap().to_string();
@@ -953,6 +964,7 @@ mod tests {
         let src = std::env::temp_dir().join(format!("gl-seam-src-{short}"));
         let _ = std::fs::remove_dir_all(&src);
         std::fs::create_dir_all(&src).unwrap();
+        let _src_guard = DirGuard(src.clone());
         run(&["init", "-q", "-b", "topsecret-branch"], &src);
         run(&["config", "user.email", "t@t"], &src);
         run(&["config", "user.name", "t"], &src);
@@ -985,7 +997,9 @@ mod tests {
             dir
         };
         let pub_dir = bare_for("served-pub");
+        let _pub_guard = DirGuard(pub_dir.clone());
         let priv_dir = bare_for("served-priv");
+        let _priv_guard = DirGuard(priv_dir.clone());
 
         state
             .db
@@ -1080,9 +1094,7 @@ mod tests {
             "the verified owner gets the real ref name"
         );
 
-        let _ = std::fs::remove_dir_all(&src);
-        let _ = std::fs::remove_dir_all(&pub_dir);
-        let _ = std::fs::remove_dir_all(&priv_dir);
+        // Cleanup runs via the DirGuard Drop impls above, on success or panic.
     }
 
     // ── #97: repo-listing surfaces are visibility-gated ──────────────────────

From fbccf397cb732f41683b5ab5dfa27a7294a42f1a Mon Sep 17 00:00:00 2001
From: beardthelion <56458543+beardthelion@users.noreply.github.com>
Date: Tue, 30 Jun 2026 08:47:57 -0500
Subject: [PATCH 5/7] test(node): drop git_info_refs from KNOWN_UNGATED now
 that #119 gates it

The repo-scoped egress guard's allowlist (added in #122, merged to main
after this branch was opened) still listed git_info_refs as ungated. This
branch makes the info/refs visibility gate unconditional for both
git-upload-pack and git-receive-pack, so the handler is now gated and the
staleness assert requires removing it from the allowlist.
---
 crates/gitlawb-node/src/api/mod.rs | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs
index ea4591f..7ae5e5f 100644
--- a/crates/gitlawb-node/src/api/mod.rs
+++ b/crates/gitlawb-node/src/api/mod.rs
@@ -396,9 +396,6 @@ mod authz_guard {
         // Repo-scoped reads known to be ungated today, each tracked by an issue.
         // Remove an entry the moment its gate lands (the staleness assert enforces it).
         let known_ungated: &[(&str, &str)] = &[
-            // info/refs gates only git-upload-pack today; git-receive-pack
-            // advertisement is ungated until #119 makes the gate unconditional.
-            ("git_info_refs", "#119"),
             ("list_certs", "#120"),
             ("get_cert", "#120"),
             ("list_issues", "#120"),

From ae15f376c09737a0fb108bfda020077bab0f7de6 Mon Sep 17 00:00:00 2001
From: beardthelion <56458543+beardthelion@users.noreply.github.com>
Date: Tue, 30 Jun 2026 09:05:11 -0500
Subject: [PATCH 6/7] style(node,git-remote): tidy #119 review nits (em dashes,
 dead binding, test header)

Post-rebase ce-code-review pass on #119. No behavior change:
- replace em dashes in the added comments with commas/colons/parentheses
- drop the dead `let _ = COVERED_COMPONENTS;` binding (and its import); the
  `missing_components().is_empty()` assert already covers the intent
- add a section header above the new git-info-refs test block to match the
  module's sectioning convention
---
 crates/git-remote-gitlawb/src/main.rs   | 15 ++++++---------
 crates/gitlawb-node/src/api/repos.rs    |  2 +-
 crates/gitlawb-node/src/test_support.rs | 20 +++++++++++---------
 3 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/crates/git-remote-gitlawb/src/main.rs b/crates/git-remote-gitlawb/src/main.rs
index ece9917..e3a9c44 100644
--- a/crates/git-remote-gitlawb/src/main.rs
+++ b/crates/git-remote-gitlawb/src/main.rs
@@ -298,7 +298,7 @@ const USER_AGENT: &str = "git/2.0 git-remote-gitlawb/0.1.0";
 /// Build the Phase-1 ref-advertisement GET, signing it when an identity is
 /// present. The node gates the ref advertisement on read visibility for BOTH
 /// services, so a private repo's advertisement is denied (404) to an
-/// unauthenticated caller — without this signature the repo's own owner can
+/// unauthenticated caller; without this signature the repo's own owner can
 /// neither fetch (upload-pack) nor push (receive-pack) it. Public repos still
 /// work anonymously (no keypair present, or the gate admits anonymous). Sign over
 /// the path *and* query (?service=...) because the node verifies the signature
@@ -348,7 +348,7 @@ fn build_pack_post_request(
             .header("Signature", signed.signature);
         tracing::debug!("signed {service} POST (DID: {})", kp.did());
     } else if service == "git-receive-pack" {
-        tracing::warn!("no identity keypair found — push will be unsigned (v0.1 local alpha only)");
+        tracing::warn!("no identity keypair found, push will be unsigned (v0.1 local alpha only)");
     }
     req
 }
@@ -569,7 +569,7 @@ mod tests {
 
     /// Without an identity, the pack POST must go out UNSIGNED so public fetch (and
     /// alpha unsigned push) still work. `Matcher::Missing` matches only when the
-    /// header is absent, so the mock is hit only if NO signature was attached — for
+    /// header is absent, so the mock is hit only if NO signature was attached, for
     /// BOTH services (the receive-pack iteration also exercises the no-keypair warn
     /// branch and confirms it does not block the request).
     #[test]
@@ -626,14 +626,12 @@ mod tests {
     /// request builders) verifies under the SAME `gitlawb_core::http_sig`
     /// primitives the node's `require_signature` uses, over the `@path` the client
     /// genuinely transmits (derived from the built reqwest request, exactly as the
-    /// node derives it from `uri.path_and_query()`). A tampered `@path` must fail —
+    /// node derives it from `uri.path_and_query()`). A tampered `@path` must fail:
     /// this is the byte-match the whole A1 client fix depends on, now executed end
     /// to end (sign here, verify with the node's verifier), not reasoned.
     #[test]
     fn client_signature_verifies_under_node_verification_for_both_services() {
-        use gitlawb_core::http_sig::{
-            build_signing_string, compute_content_digest, HttpSignature, COVERED_COMPONENTS,
-        };
+        use gitlawb_core::http_sig::{build_signing_string, compute_content_digest, HttpSignature};
         use gitlawb_core::identity::verify;
         use std::collections::HashMap;
 
@@ -674,7 +672,6 @@ mod tests {
             let signing_string = build_signing_string(&components, sig_params_value, &values)?;
             let sig_array: [u8; 64] = sig.signature_bytes.as_slice().try_into()?;
             verify(&vk, signing_string.as_bytes(), &sig_array)?;
-            let _ = COVERED_COMPONENTS;
             Ok(())
         };
         // @path exactly as the node reconstructs it from the request it receives.
@@ -791,7 +788,7 @@ mod tests {
     #[test]
     fn url_path_preserves_query_for_signed_advertisement() {
         // The Phase-1 advertisement GET is signed over url_path(refs_url), and the
-        // node verifies the signature over its path_and_query — so the ?service=
+        // node verifies the signature over its path_and_query, so the ?service=
         // query MUST survive verbatim or the signatures will not byte-match.
         assert_eq!(
             url_path("http://127.0.0.1:7545/z6Mk/myrepo/info/refs?service=git-upload-pack"),
diff --git a/crates/gitlawb-node/src/api/repos.rs b/crates/gitlawb-node/src/api/repos.rs
index 5d3883f..9e1d94f 100644
--- a/crates/gitlawb-node/src/api/repos.rs
+++ b/crates/gitlawb-node/src/api/repos.rs
@@ -510,7 +510,7 @@ pub async fn git_info_refs(
 
     // Enforce read visibility on the ref advertisement, for BOTH services. The
     // upload-pack (clone/fetch) and receive-pack (push) advertisements expose the
-    // same ref metadata — branch/tag names and commit tips — so a private repo's
+    // same ref metadata (branch/tag names and commit tips), so a private repo's
     // advertisement must be withheld from a non-reader regardless of which service
     // is requested. The push itself stays separately owner-gated on the
     // git-receive-pack POST; push access implies read access here, so a
diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs
index 332c781..128c9c4 100644
--- a/crates/gitlawb-node/src/test_support.rs
+++ b/crates/gitlawb-node/src/test_support.rs
@@ -669,6 +669,8 @@ mod tests {
         );
     }
 
+    // ── #119: git-info-refs advertisement gate + client signing ──────────────
+
     /// A1 read-gate bypass + its client remedy. `git_info_refs` serves BOTH the
     /// `git-upload-pack` (clone/fetch) and `git-receive-pack` (push) ref
     /// advertisement off one route, but the visibility gate was wrapped in
@@ -761,7 +763,7 @@ mod tests {
 
         // No-regression: a PUBLIC repo's advertisement stays anonymous for BOTH
         // services. The gate admits the anonymous caller, so the handler clears it
-        // and 500s on the missing test-disk repo — anything but 404 (a gate denial)
+        // and 500s on the missing test-disk repo; anything but 404 (a gate denial)
         // proves the unconditional gate did not accidentally lock out public reads.
         for service in ["git-upload-pack", "git-receive-pack"] {
             let req = Request::builder()
@@ -783,14 +785,14 @@ mod tests {
 
         // Client remedy: the owner's SIGNED advertisement GET clears the gate for
         // BOTH services (so fetch and push of a private repo keep working). It
-        // 500s on the missing test-disk repo — anything but 404 means cleared.
+        // 500s on the missing test-disk repo; anything but 404 means cleared.
         for service in ["git-upload-pack", "git-receive-pack"] {
             let resp = router().oneshot(signed(service)).await.unwrap();
             // INTERNAL_SERVER_ERROR specifically: the signature VERIFIED (passed
             // require_signature, not 401/403) and the owner cleared the read gate
             // (not 404), so the handler proceeded to acquire + git on a repo absent
-            // from the test disk. Asserting the exact 500 — rather than merely
-            // "not 404" — proves the request got PAST auth, not rejected by it.
+            // from the test disk. Asserting the exact 500 (rather than merely
+            // "not 404") proves the request got PAST auth, not rejected by it.
             assert_eq!(
                 resp.status(),
                 StatusCode::INTERNAL_SERVER_ERROR,
@@ -840,7 +842,7 @@ mod tests {
 
     /// A1 Phase-2 contract: the `git-upload-pack` POST (the actual fetch, after
     /// the advertisement) is itself read-visibility gated. An ANONYMOUS upload-pack
-    /// POST against a private repo is denied (404) — so signing only the Phase-1
+    /// POST against a private repo is denied (404), so signing only the Phase-1
     /// advertisement GET is NOT enough; `git-remote-gitlawb` must also sign this
     /// POST, or an owner's fetch of their own private repo clears the advertisement
     /// and then dies on the pack POST. A real owner signature clears the gate
@@ -877,8 +879,8 @@ mod tests {
         let path = format!("/{short}/up-priv.git/git-upload-pack");
 
         // Anonymous Phase-2 fetch of a private repo: denied at the gate (404). This
-        // is exactly the request git-remote-gitlawb sends today for upload-pack —
-        // the unsigned POST — which is why fetch breaks for the owner.
+        // is exactly the request git-remote-gitlawb sends today for upload-pack
+        // (the unsigned POST), which is why fetch breaks for the owner.
         let anon = Request::builder()
             .method(Method::POST)
             .uri(&path)
@@ -908,7 +910,7 @@ mod tests {
         // 500 (not merely non-404): the signature VERIFIED (passed require_signature,
         // not 401/403) AND the owner cleared the read gate (not 404), so the handler
         // reached git on the missing test-disk repo. Pinning 500 proves the request
-        // got past auth — a 401 regression would slip through a bare `!= 404`.
+        // got past auth; a 401 regression would slip through a bare `!= 404`.
         assert_eq!(
             resp.status(),
             StatusCode::INTERNAL_SERVER_ERROR,
@@ -918,7 +920,7 @@ mod tests {
 
     /// Served-content seam: with a REAL on-disk bare repo (branch
     /// `topsecret-branch`), the advertisement serves the actual ref names to
-    /// authorized callers and withholds them from denied ones — proving real
+    /// authorized callers and withholds them from denied ones, proving real
     /// content egress + withholding, not just the gate decision (the other tests
     /// land on a 500 from a missing-disk repo). Asserts the branch name appears for
     /// allowed callers and never appears in a denied 404 body.

From ef61b57d11ef2505f12567b4ca1f916c7a1e2d8a Mon Sep 17 00:00:00 2001
From: beardthelion <56458543+beardthelion@users.noreply.github.com>
Date: Tue, 30 Jun 2026 18:11:12 -0500
Subject: [PATCH 7/7] test(node): recurse nested api modules in the egress-gate
 guard

The completeness scan in every_repo_scoped_handler_is_gated used a flat
read_dir over src/api, so a future api/<module>/mod.rs could add a
repo-scoped handler the scrape never inspects. Walk the api tree recursively
via a new collect_rs_files helper (covered by its own unit test), skipping
only the top-level mod.rs so nested module files are checked too.
---
 crates/gitlawb-node/src/api/mod.rs | 66 +++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 5 deletions(-)

diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs
index 7ae5e5f..b1b83ec 100644
--- a/crates/gitlawb-node/src/api/mod.rs
+++ b/crates/gitlawb-node/src/api/mod.rs
@@ -324,6 +324,56 @@ mod authz_guard {
         })
     }
 
+    /// Collect `.rs` source files under `dir`. Recursive so the completeness scan
+    /// covers nested API modules (`api/<module>/mod.rs` and deeper), not only the
+    /// immediate `api/*.rs` children.
+    fn collect_rs_files(dir: &std::path::Path) -> Vec<std::path::PathBuf> {
+        let mut out = Vec::new();
+        for entry in std::fs::read_dir(dir).expect("read api dir") {
+            let path = entry.expect("dir entry").path();
+            if path.is_dir() {
+                out.extend(collect_rs_files(&path));
+            } else if path.extension().is_some_and(|e| e == "rs") {
+                out.push(path);
+            }
+        }
+        out
+    }
+
+    #[test]
+    fn collect_rs_files_recurses_subdirs() {
+        let tmp = tempfile::TempDir::new().unwrap();
+        let root = tmp.path();
+        std::fs::write(root.join("a.rs"), "").unwrap();
+        std::fs::write(root.join("note.txt"), "").unwrap();
+        std::fs::create_dir_all(root.join("sub/deep")).unwrap();
+        std::fs::write(root.join("sub/mod.rs"), "").unwrap();
+        std::fs::write(root.join("sub/deep/c.rs"), "").unwrap();
+        let names: std::collections::HashSet<String> = collect_rs_files(root)
+            .iter()
+            .map(|p| {
+                p.strip_prefix(root)
+                    .unwrap()
+                    .to_string_lossy()
+                    .replace('\\', "/")
+            })
+            .collect();
+        assert!(names.contains("a.rs"));
+        assert!(
+            names.contains("sub/mod.rs"),
+            "nested module file must be collected"
+        );
+        assert!(
+            names.contains("sub/deep/c.rs"),
+            "deeply nested file must be collected"
+        );
+        assert!(
+            !names.iter().any(|n| n.ends_with(".txt")),
+            "non-rs files excluded"
+        );
+        assert_eq!(names.len(), 3);
+    }
+
     /// Egress gate guard: every repo-scoped handler (`Path<(String, String..)>`)
     /// must carry an authz marker — a read gate (`authorize_repo_read` /
     /// `visibility_check`), or a write gate (`require_repo_owner` / `require_owner`
@@ -376,20 +426,26 @@ mod authz_guard {
                 "read-guard `sources` lists {f} but the file does not exist"
             );
         }
-        for entry in std::fs::read_dir(api_dir).expect("read src/api") {
-            let path = entry.expect("dir entry").path();
+        let api_root = std::path::Path::new(api_dir);
+        for path in collect_rs_files(api_root) {
             let fname = path.file_name().unwrap().to_string_lossy().into_owned();
-            if !fname.ends_with(".rs") || fname == "mod.rs" || listed.contains(fname.as_str()) {
+            // Skip the guard file itself (the top-level mod.rs) and files already
+            // covered by the per-handler loop. A nested module file (including a
+            // nested mod.rs) IS scanned, so a new api/<module>/ cannot smuggle in
+            // an ungated repo-scoped handler the scrape never looks at.
+            if path == api_root.join("mod.rs") || listed.contains(fname.as_str()) {
                 continue;
             }
             let src = std::fs::read_to_string(&path).expect("read api file");
             let has_repo_handler = handler_names(&src)
                 .iter()
                 .any(|n| is_repo_scoped(&fn_body(&src, n)));
+            let rel = path.strip_prefix(api_root).unwrap_or(path.as_path());
             assert!(
                 !has_repo_handler,
-                "api/{fname} declares a repo-scoped handler but is not in the egress \
-                 guard `sources` list — add it so its handlers are gate-checked"
+                "api/{} declares a repo-scoped handler but is not in the egress \
+                 guard `sources` list — add it so its handlers are gate-checked",
+                rel.display()
             );
         }