chore: refresh PR head

README.md

@@ -188,12 +188,7 @@ export GITLAWB_NODE=http://localhost:7545
 git clone gitlawb://did:key:z6Mk.../my-repo
 ```
 
-For public-network use, make sure `GITLAWB_NODE` points to the node you want, over
-`https://`. The helper defaults to localhost for local development, and plaintext
-`http://` is allowed only to this machine: requests are signed but not encrypted, so
-a cleartext hop to a remote node exposes the pack contents and the `Signature`
-header. A remote `http://` node is refused, and `GITLAWB_ALLOW_INSECURE_HTTP=1`
-overrides that for a trusted private network.
+For public-network use, make sure `GITLAWB_NODE` points to the node you want. The helper defaults to localhost for local development.
 
 ### Full lifecycle against an iCaptcha-enforcing node
 
@@ -398,7 +393,7 @@ Important node settings:
 | `GITLAWB_P2P_BOOTSTRAP` | Comma-separated libp2p multiaddrs. |
 | `GITLAWB_BOOTSTRAP_DISABLE_SEEDS` | Disable embedded seed peers for isolated dev/test networks. |
 | `GITLAWB_REQUIRE_SIGNED_PEER_WRITES` | Require signed peer announce/sync writes. Defaults to `false` during the staged rollout below. |
-| `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. |

crates/git-remote-gitlawb/src/main.rs

@@ -66,10 +66,6 @@ fn main() -> Result<()> {
     // v0.1: default to localhost. Override with GITLAWB_NODE env var.
     let node_base =
         std::env::var("GITLAWB_NODE").unwrap_or_else(|_| "http://127.0.0.1:7545".to_string());
-    check_transport_security(
-        &node_base,
-        std::env::var_os("GITLAWB_ALLOW_INSECURE_HTTP").is_some(),
-    )?;
     let repo_base = format!("{}/{}/{}", node_base, short_owner, repo_name);
     tracing::debug!("repo_base: {repo_base}");
 
@@ -82,76 +78,6 @@ fn main() -> Result<()> {
     run_helper(&repo_base, keypair.as_ref())
 }
 
-/// Whether `url` is plaintext http, regardless of where it points.
-fn is_http(url: &str) -> bool {
-    reqwest::Url::parse(url.trim())
-        .map(|u| u.scheme() == "http")
-        .unwrap_or(false)
-}
-
-/// Whether `url` would send git data off this machine in cleartext.
-///
-/// True only for `http://` to a non-loopback host. RFC 9421 signs the request
-/// but does not encrypt it, so on a plaintext hop the pack contents and the
-/// `Signature` header are both readable, and a captured signature is replayable
-/// for its freshness window against any host.
-///
-/// Loopback is decided from the parsed address rather than a string match, so
-/// `127.0.0.2`, `[::1]` and an IPv4-mapped `[::ffff:127.0.0.1]` are all
-/// recognised as this machine. A value that does not parse, or that is not
-/// http(s), is not this guard's business and returns false: it fails later with
-/// its own error, and naming it a TLS problem would misdirect the reader.
-fn is_insecure_remote(url: &str) -> bool {
-    let Ok(parsed) = reqwest::Url::parse(url.trim()) else {
-        return false;
-    };
-    if parsed.scheme() != "http" {
-        return false;
-    }
-    let Some(host) = parsed.host_str() else {
-        return false;
-    };
-    let host = host.trim_end_matches('.').to_ascii_lowercase();
-    if host == "localhost" {
-        return false;
-    }
-    // host_str() keeps the brackets on an IPv6 literal.
-    let bare = host.trim_start_matches('[').trim_end_matches(']');
-    if let Ok(ip) = bare.parse::<std::net::IpAddr>() {
-        if ip.is_loopback() {
-            return false;
-        }
-        // An IPv4-mapped IPv6 literal hides a v4 loopback from is_loopback().
-        if let std::net::IpAddr::V6(v6) = ip {
-            if let Some(v4) = v6.to_ipv4_mapped() {
-                if v4.is_loopback() {
-                    return false;
-                }
-            }
-        }
-    }
-    true
-}
-
-/// Refuse a cleartext hop off this machine unless the operator has opted in.
-///
-/// Fail closed: the alternative is signing a request and then handing it, and
-/// the pack it carries, to anyone on the path. `GITLAWB_ALLOW_INSECURE_HTTP`
-/// exists for a private LAN where the operator has decided that is acceptable.
-fn check_transport_security(node_base: &str, allow_insecure: bool) -> Result<()> {
-    if allow_insecure || !is_insecure_remote(node_base) {
-        return Ok(());
-    }
-    bail!(
-        "refusing to send git data to {node_base} over plaintext http.\n\
-         Requests are signed but not encrypted, so the pack contents and the \
-         Signature header are readable by anyone on the path, and a captured \
-         signature can be replayed.\n\
-         Use https://, or set GITLAWB_ALLOW_INSECURE_HTTP=1 to accept the risk \
-         (for a trusted private network only)."
-    )
-}
-
 // ── CLI argument handling ──────────────────────────────────────────────────────
 
 /// How the binary was invoked, derived from its CLI arguments.
@@ -199,7 +125,6 @@ fn help_text() -> String {
          ENVIRONMENT:\n\
          \x20   GITLAWB_NODE   Node base URL (default: http://127.0.0.1:7545)\n\
          \x20   GITLAWB_KEY    Identity PEM path for signed fetch/push (default: ~/.gitlawb/identity.pem)\n\
-         \x20   GITLAWB_ALLOW_INSECURE_HTTP  Permit plaintext http:// to a non-loopback node (unset by default)\n\
          \x20   GITLAWB_LOG    Log filter (default: warn)\n\
          \n\
          FLAGS:\n\
@@ -274,8 +199,7 @@ fn handle_connect<R: Read>(
         other => bail!("unsupported git service: {other}"),
     }
 
-    // A loopback node must never be proxied; see build_http_client.
-    let client = build_http_client(!is_insecure_remote(repo_base) && is_http(repo_base))?;
+    let client = build_http_client()?;
 
     // ── Phase 1: ref advertisement (GET /info/refs?service=<service>) ─────────
     //
@@ -363,8 +287,10 @@ fn handle_connect<R: Read>(
     //    this into one POST deadlocks a multi-round fetch (#117).
     //
     //  git-receive-pack (push):
-    //    Git sends ref-update commands + the complete PACK blob, then closes its
-    //    write pipe. read_to_end is safe and correct here, a single POST.
+    //    Git sends the ref-update command list (pkt-lines ending in a flush),
+    //    then the complete PACK blob only when a command has a non-zero new-oid.
+    //    A delete-only push sends no pack, and git holds its write pipe open for
+    //    the report-status response, so EOF cannot delimit the request (#369).
 
     let post_url = format!("{}/{}", repo_base, service);
 
@@ -372,10 +298,7 @@ fn handle_connect<R: Read>(
         return negotiate_upload_pack(&client, &post_url, service, signing_key, stdin, &mut stdout);
     }
 
-    let mut request_body = Vec::new();
-    stdin
-        .read_to_end(&mut request_body)
-        .context("reading receive-pack request")?;
+    let request_body = read_receive_pack_request(stdin)?;
     tracing::debug!("pack request: {} bytes from git", request_body.len());
 
     if request_body.is_empty() {
@@ -411,22 +334,8 @@ const HTTP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(300);
 /// node named, and on a 307/308 the pack body went with them. Scope the follow to the
 /// origin that issued the redirect AND to an identical request-target, which is the
 /// same predicate `gl` uses.
-/// `bypass_proxy` must be true whenever the node is on this machine. reqwest's
-/// default client honours `HTTP_PROXY` / `ALL_PROXY` for a loopback URL too, so a
-/// proxy variable pointing off-machine turns an allowed local request into a
-/// cleartext hop carrying the `Signature` header, straight past
-/// `check_transport_security`. Verified: with `HTTP_PROXY` set, reqwest logged
-/// `proxy(...) intercepts 'http://127.0.0.1:7545/'` and dialled the proxy. A
-/// local node is never legitimately reached through a proxy, so this refuses one
-/// rather than trying to reimplement `NO_PROXY` parsing.
-fn build_http_client(bypass_proxy: bool) -> Result<reqwest::blocking::Client> {
-    let builder = reqwest::blocking::Client::builder();
-    let builder = if bypass_proxy {
-        builder.no_proxy()
-    } else {
-        builder
-    };
-    Ok(builder
+fn build_http_client() -> Result<reqwest::blocking::Client> {
+    Ok(reqwest::blocking::Client::builder()
         .timeout(HTTP_TIMEOUT)
         .redirect(reqwest::redirect::Policy::custom(same_origin_redirect))
         .build()?)
@@ -625,6 +534,108 @@ fn read_upload_pack_round<R: Read>(stdin: &mut R) -> Result<(Vec<u8>, RoundEnd)>
     }
 }
 
+/// Read one git-receive-pack request: optional `shallow` lines, then EITHER a
+/// command list (pkt-lines terminated by a flush) or a push-cert block
+/// (pkt-lines terminated by a "push-cert-end" line), then the negotiated
+/// push-options block, then the pack ONLY when a command carries a non-zero
+/// new-oid.
+///
+/// A delete-only push sends no pack, and git holds its write pipe open waiting
+/// for the report-status response, so a plain read_to_end deadlocks the two
+/// (#369). A signed push is the other trap: when the client sends a push-cert
+/// it sends NO command list at all (the commands live inside the cert), so the
+/// request starts with a "push-cert" line and no flush ever arrives. The
+/// negotiated capability list is on the push-cert line, or on the first
+/// command line after its NUL when there is no cert. EOF at any point also
+/// ends the read, covering git versions that close the pipe early.
+fn read_receive_pack_request<R: Read>(stdin: &mut R) -> Result<Vec<u8>> {
+    /// Read one pkt-line into `body` (length prefix + data), returning its data
+    /// or `None` on flush or EOF. Malformed lengths bail rather than collapse
+    /// into a synthesized flush (#192 F5).
+    fn read_pkt<R: Read>(stdin: &mut R, body: &mut Vec<u8>) -> Result<Option<Vec<u8>>> {
+        let mut len_bytes = [0u8; 4];
+        match stdin.read_exact(&mut len_bytes) {
+            Ok(()) => {}
+            Err(e) if e.kind() == io::ErrorKind::UnexpectedEof => return Ok(None),
+            Err(e) => return Err(e.into()),
+        }
+        let Ok(len_hex) = std::str::from_utf8(&len_bytes) else {
+            bail!("malformed pkt-line length prefix: non-UTF-8 bytes {len_bytes:02x?}");
+        };
+        let Ok(pkt_len) = usize::from_str_radix(len_hex, 16) else {
+            bail!("malformed pkt-line length prefix: non-hex {len_hex:?}");
+        };
+        body.extend_from_slice(&len_bytes);
+        if pkt_len == 0 {
+            return Ok(None);
+        }
+        if pkt_len < 4 {
+            bail!("invalid pkt-line length: {pkt_len}");
+        }
+        let mut data = vec![0u8; pkt_len - 4];
+        stdin
+            .read_exact(&mut data)
+            .context("reading pkt-line data")?;
+        body.extend_from_slice(&data);
+        Ok(Some(data))
+    }
+
+    /// "<40-hex old> <40-hex new> <refname>" — the command shape, whether it
+    /// appears in a command list or inside a push-cert.
+    fn is_command(data: &[u8]) -> bool {
+        data.len() >= 82
+            && data[..40].iter().all(|b| b.is_ascii_hexdigit())
+            && data[40] == b' '
+            && data[41..81].iter().all(|b| b.is_ascii_hexdigit())
+            && data[81] == b' '
+    }
+
+    let mut body = Vec::new();
+    let mut caps = String::new();
+    let mut pack_expected = false;
+    let mut in_cert = false;
+    loop {
+        let Some(data) = read_pkt(stdin, &mut body)? else {
+            // Flush ends a command list; EOF ends anything. A cert has no
+            // legal flush inside it, so the same break covers malformed
+            // input without hanging.
+            break;
+        };
+        if !in_cert && data.starts_with(b"push-cert") {
+            in_cert = true;
+            if let Some(nul) = data.iter().position(|&b| b == 0) {
+                caps = String::from_utf8_lossy(&data[nul + 1..]).into_owned();
+            }
+            continue;
+        }
+        if in_cert && data == b"push-cert-end\n" {
+            break;
+        }
+        if is_command(&data) {
+            if caps.is_empty() {
+                if let Some(nul) = data.iter().position(|&b| b == 0) {
+                    caps = String::from_utf8_lossy(&data[nul + 1..]).into_owned();
+                }
+            }
+            if data[41..81].iter().any(|&b| b != b'0') {
+                pack_expected = true;
+            }
+        }
+    }
+    // When both are negotiated the push-options block follows the cert; when
+    // only options are negotiated it follows the command flush. Either way it
+    // is pkt-lines to a flush.
+    if caps.split(' ').any(|c| c.starts_with("push-options")) {
+        while read_pkt(stdin, &mut body)?.is_some() {}
+    }
+    if pack_expected {
+        stdin
+            .read_to_end(&mut body)
+            .context("reading receive-pack pack")?;
+    }
+    Ok(body)
+}
+
 /// POST one self-contained stateless-RPC request body and stream the response to
 /// `stdout`. Signs the request when `signing_key` is present, so every round of a
 /// private-repo fetch carries the caller's signature (the node visibility-gates
@@ -919,148 +930,6 @@ fn resolve_key_path() -> std::path::PathBuf {
 
 // ── Tests ─────────────────────────────────────────────────────────────────────
 
-#[cfg(test)]
-mod insecure_transport_tests {
-    use super::*;
-
-    /// Loopback in every form the transport can legitimately be pointed at.
-    /// Plaintext to this machine is the documented local-alpha default and must
-    /// keep working, so a false positive here breaks every stock install.
-    #[test]
-    fn loopback_http_is_allowed() {
-        for url in [
-            "http://127.0.0.1:7545",
-            "http://localhost:7545",
-            "http://[::1]:7545",
-            "http://127.0.0.2:7545",
-            "http://[::ffff:127.0.0.1]:7545",
-            "http://LocalHost:7545",
-        ] {
-            assert!(
-                !is_insecure_remote(url),
-                "{url} is this machine; plaintext to it must stay allowed"
-            );
-        }
-    }
-
-    /// The case the guard exists for: cleartext git data leaving the machine.
-    #[test]
-    fn remote_http_is_refused() {
-        for url in [
-            "http://node.example.com:7545",
-            "http://10.0.0.36:7777",
-            "http://192.168.1.10",
-            "http://[2001:db8::1]:7545",
-            "http://8.8.8.8",
-        ] {
-            assert!(
-                is_insecure_remote(url),
-                "{url} sends git data off-machine in cleartext and must be refused"
-            );
-        }
-    }
-
-    /// TLS is always fine, loopback or not, so the guard keys on the scheme and
-    /// not merely on the host being remote.
-    #[test]
-    fn https_is_always_allowed() {
-        for url in [
-            "https://node.gitlawb.com",
-            "https://10.0.0.36:7777",
-            "https://127.0.0.1:7545",
-        ] {
-            assert!(!is_insecure_remote(url), "{url} is TLS and must be allowed");
-        }
-    }
-
-    /// Spellings that defeat a naive loopback check. `Url` normalizes the scheme
-    /// and the host at parse time (lowercasing, and the WHATWG numeric forms), so
-    /// these must all still read as this machine.
-    #[test]
-    fn loopback_spellings_are_normalized_not_string_matched() {
-        for url in [
-            "HTTP://127.0.0.1:7545",
-            "Http://LOCALHOST:7545",
-            "http://localhost.:7545",
-            "http://user:[email protected]:7545",
-            "http://2130706433:7545",
-            "http://0x7f000001:7545",
-            "http://[::ffff:7f00:1]:7545",
-        ] {
-            assert!(
-                !is_insecure_remote(url),
-                "{url} resolves to this machine; plaintext to it must stay allowed"
-            );
-        }
-    }
-
-    /// The mirror of the above: an uppercase scheme must not become an escape
-    /// from the guard, and a remote host in any spelling is still remote.
-    #[test]
-    fn uppercase_scheme_does_not_escape_the_guard() {
-        for url in [
-            "HTTP://node.example.com:7545",
-            "Http://8.8.8.8",
-            "http://NODE.EXAMPLE.COM",
-            "http://user:[email protected]",
-        ] {
-            assert!(
-                is_insecure_remote(url),
-                "{url} is a remote cleartext hop and must be refused"
-            );
-        }
-    }
-
-    /// The gate itself, both directions, including the opt-in escape hatch.
-    #[test]
-    fn gate_refuses_remote_plaintext_unless_opted_in() {
-        let err = check_transport_security("http://node.example.com:7545", false)
-            .expect_err("remote plaintext must be refused by default");
-        let msg = err.to_string();
-        assert!(
-            msg.contains("plaintext http"),
-            "the refusal must say why: {msg}"
-        );
-        assert!(
-            msg.contains("GITLAWB_ALLOW_INSECURE_HTTP"),
-            "the refusal must name the escape hatch: {msg}"
-        );
-
-        check_transport_security("http://node.example.com:7545", true)
-            .expect("the opt-in must permit the same URL");
-        check_transport_security("http://127.0.0.1:7545", false)
-            .expect("the local default must keep working with no opt-in");
-        check_transport_security("https://node.gitlawb.com", false)
-            .expect("TLS must need no opt-in");
-    }
-
-    /// The documented knob must appear in --help. A gate the operator cannot
-    /// discover reads as a broken transport rather than a deliberate refusal.
-    #[test]
-    fn help_documents_the_opt_in() {
-        assert!(help_text().contains("GITLAWB_ALLOW_INSECURE_HTTP"));
-    }
-
-    /// An unparseable or non-http value is not classified as insecure here: it
-    /// fails later with its own error, and reporting it as a TLS problem would
-    /// send the reader after the wrong thing.
-    #[test]
-    fn unparseable_or_other_scheme_is_not_this_guards_problem() {
-        for url in [
-            "",
-            "   ",
-            "not a url",
-            "ftp://node.example.com",
-            "file:///tmp/x",
-        ] {
-            assert!(
-                !is_insecure_remote(url),
-                "{url:?} is not a cleartext-http-to-remote case"
-            );
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -1161,7 +1030,7 @@ mod tests {
     #[test]
     fn signed_requests_do_not_follow_a_redirect_off_the_node_origin() {
         let kp = Keypair::generate();
-        let client = build_http_client(false).unwrap();
+        let client = build_http_client().unwrap();
 
         let mut elsewhere = mockito::Server::new();
         let never = elsewhere
@@ -1263,7 +1132,7 @@ mod tests {
     #[test]
     fn a_same_origin_path_changing_redirect_is_refused() {
         let kp = Keypair::generate();
-        let client = build_http_client(false).unwrap();
+        let client = build_http_client().unwrap();
 
         let mut node = mockito::Server::new();
         let bounce = node
@@ -1314,7 +1183,7 @@ mod tests {
     #[test]
     fn an_identical_target_redirect_is_still_followed_up_to_the_chain_bound() {
         let kp = Keypair::generate();
-        let client = build_http_client(false).unwrap();
+        let client = build_http_client().unwrap();
 
         let mut node = mockito::Server::new();
         let loop_route = node
@@ -1475,7 +1344,7 @@ mod tests {
     fn a_rewritten_target_never_receives_the_signature() {
         let kp = Keypair::generate();
         let expected_did = kp.did().to_string();
-        let client = build_http_client(false).unwrap();
+        let client = build_http_client().unwrap();
         let slot = std::sync::Arc::new(std::sync::Mutex::new(None::<Verdict>));
 
         let mut node = mockito::Server::new();
@@ -1537,7 +1406,7 @@ mod tests {
     fn a_direct_signed_advertisement_verifies_under_the_node_verifier() {
         let kp = Keypair::generate();
         let expected_did = kp.did().to_string();
-        let client = build_http_client(false).unwrap();
+        let client = build_http_client().unwrap();
         let slot = std::sync::Arc::new(std::sync::Mutex::new(None::<Verdict>));
 
         let mut node = mockito::Server::new();
@@ -2249,6 +2118,184 @@ mod tests {
         assert!(help.ends_with('\n'));
     }
 
+    // ── #369 delete-only push deadlock ───────────────────────────────────────
+
+    /// A delete-only receive-pack request ends at the command flush; git then
+    /// holds the write pipe open for the report and sends no pack. A reader that
+    /// blocks after the command list reproduces the hang that a Cursor (EOF)
+    /// hides. The read must return at the flush, inside the timeout.
+    #[test]
+    fn delete_only_receive_pack_request_returns_at_command_flush() {
+        let old = "a".repeat(40);
+        let zero = "0".repeat(40);
+        let mut seed = Vec::new();
+        seed.extend_from_slice(&pkt(&format!("{old} {zero} refs/heads/gone\n")));
+        seed.extend_from_slice(b"0000"); // command-list flush; git then blocks
+
+        let (tx, rx) = std::sync::mpsc::channel::<()>();
+        let mut reader = BlockAfterSeed {
+            seed: io::Cursor::new(seed),
+            gate: rx,
+        };
+
+        let (done_tx, done_rx) = std::sync::mpsc::channel::<Vec<u8>>();
+        let handle = std::thread::spawn(move || {
+            let _ = done_tx.send(read_receive_pack_request(&mut reader).unwrap());
+        });
+
+        let got = done_rx.recv_timeout(std::time::Duration::from_secs(2));
+        let _ = tx.send(()); // unblock the reader so the thread can exit
+        let _ = handle.join();
+
+        let body = got.expect(
+            "read_receive_pack_request blocked past the command flush waiting for \
+             a pack a delete-only push never sends (#369)",
+        );
+        assert!(
+            body.ends_with(b"0000"),
+            "the request body keeps the command list and its flush terminator"
+        );
+    }
+
+    /// A push carrying objects still reads the pack through to EOF, and the
+    /// command flush sits between the commands and the pack in the body.
+    #[test]
+    fn receive_pack_request_with_pack_reads_through_eof() {
+        let old = "a".repeat(40);
+        let new = "b".repeat(40);
+        let mut stream = Vec::new();
+        stream.extend_from_slice(&pkt(&format!(
+            "{old} {new} refs/heads/main\0 report-status-v2\n"
+        )));
+        stream.extend_from_slice(b"0000");
+        stream.extend_from_slice(b"PACK\x00\x00\x00\x02fakepack");
+
+        let mut stdin = io::Cursor::new(stream);
+        let body = read_receive_pack_request(&mut stdin).unwrap();
+        let pos = body.windows(4).position(|w| w == b"0000").unwrap();
+        assert_eq!(
+            &body[pos + 4..],
+            b"PACK\x00\x00\x00\x02fakepack",
+            "a non-zero new-oid means the pack follows the flush"
+        );
+    }
+
+    /// A signed delete-only push sends NO command list: the request starts
+    /// with the "push-cert" line, the commands live inside the cert, and the
+    /// cert ends with a "push-cert-end" pkt-line (not a flush). The reader
+    /// must return after push-cert-end without waiting on the pipe.
+    #[test]
+    fn signed_delete_only_push_returns_after_push_cert_end() {
+        let old = "a".repeat(40);
+        let zero = "0".repeat(40);
+        let mut seed = Vec::new();
+        seed.extend_from_slice(&pkt("push-cert\0 report-status-v2 push-cert\n"));
+        seed.extend_from_slice(&pkt("certificate version 0.1\n"));
+        seed.extend_from_slice(&pkt("pusher did:key:z6Mk\n"));
+        seed.extend_from_slice(&pkt("\n")); // empty line ends the headers
+        seed.extend_from_slice(&pkt(&format!("{old} {zero} refs/heads/gone\n")));
+        seed.extend_from_slice(&pkt("sig-line\n"));
+        seed.extend_from_slice(&pkt("push-cert-end\n"));
+        // git now blocks for the report; the pipe stays open.
+
+        let (tx, rx) = std::sync::mpsc::channel::<()>();
+        let mut reader = BlockAfterSeed {
+            seed: io::Cursor::new(seed),
+            gate: rx,
+        };
+
+        let (done_tx, done_rx) = std::sync::mpsc::channel::<Vec<u8>>();
+        let handle = std::thread::spawn(move || {
+            let _ = done_tx.send(read_receive_pack_request(&mut reader).unwrap());
+        });
+        let got = done_rx.recv_timeout(std::time::Duration::from_secs(2));
+        let _ = tx.send(());
+        let _ = handle.join();
+
+        let body = got.expect(
+            "read_receive_pack_request blocked past push-cert-end on a signed \
+             delete-only push (#369)",
+        );
+        assert!(
+            body.windows(6).any(|w| w == b"pusher"),
+            "the cert block is part of the request body"
+        );
+    }
+
+    /// A signed pack-bearing push: cert with the commands inside, then the
+    /// pack. Everything must land in the body.
+    #[test]
+    fn signed_pack_push_keeps_cert_and_pack() {
+        let old = "a".repeat(40);
+        let new = "b".repeat(40);
+        let mut stream = Vec::new();
+        stream.extend_from_slice(&pkt("push-cert\0 report-status-v2 push-cert\n"));
+        stream.extend_from_slice(&pkt("certificate version 0.1\n"));
+        stream.extend_from_slice(&pkt("\n"));
+        stream.extend_from_slice(&pkt(&format!("{old} {new} refs/heads/main\n")));
+        stream.extend_from_slice(&pkt("push-cert-end\n"));
+        stream.extend_from_slice(b"PACK\x00\x00\x00\x02fakepack");
+
+        let mut stdin = io::Cursor::new(stream);
+        let body = read_receive_pack_request(&mut stdin).unwrap();
+        assert!(
+            body.windows(5).any(|w| w == b"PACK\x00"),
+            "the pack follows the cert into the body"
+        );
+    }
+
+    /// A negotiated push-options block is pkt-lines ending in its own flush.
+    /// With no cert it follows the command flush; with a cert it follows
+    /// push-cert-end.
+    #[test]
+    fn receive_pack_request_consumes_a_push_options_block() {
+        let old = "a".repeat(40);
+        let new = "b".repeat(40);
+        let mut stream = Vec::new();
+        stream.extend_from_slice(&pkt(&format!(
+            "{old} {new} refs/heads/main\0 report-status-v2 push-options\n"
+        )));
+        stream.extend_from_slice(b"0000"); // command flush
+        stream.extend_from_slice(&pkt("ci.skip\n"));
+        stream.extend_from_slice(b"0000"); // options flush
+        stream.extend_from_slice(b"PACK\x00\x00\x00\x02fakepack");
+
+        let mut stdin = io::Cursor::new(stream);
+        let body = read_receive_pack_request(&mut stdin).unwrap();
+        assert!(body.windows(7).any(|w| w == b"ci.skip"), "options survived");
+        assert!(
+            body.ends_with(b"PACK\x00\x00\x00\x02fakepack"),
+            "the pack lands after the options block"
+        );
+    }
+
+    /// Cert and options together: the cert comes first and the options block
+    /// sits between push-cert-end and the pack.
+    #[test]
+    fn signed_push_with_options_reads_cert_then_options_then_pack() {
+        let old = "a".repeat(40);
+        let new = "b".repeat(40);
+        let mut stream = Vec::new();
+        stream.extend_from_slice(&pkt(
+            "push-cert\0 report-status-v2 push-cert push-options\n",
+        ));
+        stream.extend_from_slice(&pkt("certificate version 0.1\n"));
+        stream.extend_from_slice(&pkt("\n"));
+        stream.extend_from_slice(&pkt(&format!("{old} {new} refs/heads/main\n")));
+        stream.extend_from_slice(&pkt("push-cert-end\n"));
+        stream.extend_from_slice(&pkt("ci.skip\n"));
+        stream.extend_from_slice(b"0000"); // options flush
+        stream.extend_from_slice(b"PACK\x00\x00\x00\x02fakepack");
+
+        let mut stdin = io::Cursor::new(stream);
+        let body = read_receive_pack_request(&mut stdin).unwrap();
+        assert!(body.windows(7).any(|w| w == b"ci.skip"), "options survived");
+        assert!(
+            body.ends_with(b"PACK\x00\x00\x00\x02fakepack"),
+            "the pack lands after the options block"
+        );
+    }
+
     // ── #117 multi-round fetch negotiation ───────────────────────────────────
 
     /// Encode a git pkt-line: 4-byte hex length (incl. the 4 bytes) + data.

crates/git-remote-gitlawb/tests/real_git_fetch.rs

@@ -95,6 +95,36 @@ fn upload_pack(repo: &Path, advertise: bool, input: &[u8]) -> Vec<u8> {
     out.stdout
 }
 
+/// Run `git receive-pack --stateless-rpc [--advertise-refs] <repo>`, optionally
+/// feeding `input` on stdin. Returns raw stdout (the wire bytes). The receive
+/// half of `upload_pack`, used so the shim can serve a real push.
+fn receive_pack(repo: &Path, advertise: bool, input: &[u8]) -> Vec<u8> {
+    let mut cmd = Command::new("git");
+    cmd.arg("receive-pack").arg("--stateless-rpc");
+    if advertise {
+        cmd.arg("--advertise-refs");
+    }
+    cmd.arg(repo)
+        .stdin(Stdio::piped())
+        .stdout(Stdio::piped())
+        .stderr(Stdio::piped());
+    let mut child = cmd.spawn().expect("spawn git receive-pack");
+    let mut stdin = child.stdin.take().unwrap();
+    let input = input.to_vec();
+    let writer = std::thread::spawn(move || {
+        let _ = stdin.write_all(&input);
+        // drop closes stdin
+    });
+    let out = child.wait_with_output().expect("wait receive-pack");
+    writer.join().ok();
+    assert!(
+        out.status.success(),
+        "git receive-pack failed: {}",
+        String::from_utf8_lossy(&out.stderr)
+    );
+    out.stdout
+}
+
 #[derive(Clone, Copy)]
 enum ShimMode {
     /// Faithful v0 serving: pipe each POST to `git upload-pack --stateless-rpc`.
@@ -218,7 +248,21 @@ fn handle_conn(stream: TcpStream, repo: &Path, mode: ShimMode, posts: &AtomicUsi
         reader.read_exact(&mut body).ok();
     }
 
-    let (content_type, payload) = if method == "GET" && target.contains("/info/refs") {
+    let (content_type, payload) = if method == "GET" && target.contains("service=git-receive-pack")
+    {
+        // Receive-pack advertisement, same wrapper shape as upload-pack.
+        let adv = receive_pack(repo, true, b"");
+        let mut wrapped = pkt(b"# service=git-receive-pack\n");
+        wrapped.extend_from_slice(b"0000");
+        wrapped.extend_from_slice(&adv);
+        ("application/x-git-receive-pack-advertisement", wrapped)
+    } else if method == "POST" && target.ends_with("/git-receive-pack") {
+        posts.fetch_add(1, Ordering::SeqCst);
+        (
+            "application/x-git-receive-pack-result",
+            receive_pack(repo, false, &body),
+        )
+    } else if method == "GET" && target.contains("/info/refs") {
         // v0 advertisement, wrapped exactly as the node's info_refs does.
         let adv = upload_pack(repo, true, b"");
         let mut wrapped = pkt(b"# service=git-upload-pack\n");
@@ -267,15 +311,6 @@ fn write_response(mut stream: TcpStream, status: &str, content_type: &str, body:
 /// Run `git fetch` in `clone` through the helper, with a hard timeout so a
 /// regression to the deadlock fails fast instead of hanging the suite.
 fn fetch_with_helper(clone: &Path, node_url: &str) -> (bool, std::process::Output) {
-    fetch_with_helper_env(clone, node_url, &[])
-}
-
-/// [`fetch_with_helper`] with extra environment for the helper process.
-fn fetch_with_helper_env(
-    clone: &Path,
-    node_url: &str,
-    extra_env: &[(&str, &str)],
-) -> (bool, std::process::Output) {
     let helper_bin = PathBuf::from(env!("CARGO_BIN_EXE_git-remote-gitlawb"));
     let helper_dir = helper_bin.parent().unwrap().to_path_buf();
     let path_env = match std::env::var_os("PATH") {
@@ -298,14 +333,35 @@ fn fetch_with_helper_env(
         // machine/CI with git-l10n installed and LANG set to a translated locale.
         .env("LC_ALL", "C")
         .env("GITLAWB_NODE", node_url)
-        .env("GITLAWB_KEY", "/nonexistent-key-for-anon-fetch")
-        // The helper inherits this process's environment, so an operator or CI
-        // that exports the insecure-HTTP override would decide the transport
-        // policy for every test below. Clear it and let each test opt in.
-        .env_remove("GITLAWB_ALLOW_INSECURE_HTTP");
-    for (k, v) in extra_env {
-        cmd.env(k, v);
-    }
+        .env("GITLAWB_KEY", "/nonexistent-key-for-anon-fetch");
+    run_bounded(cmd, Duration::from_secs(30))
+}
+
+/// Run `git push` in `local` through the helper, same wiring and hard timeout as
+/// [`fetch_with_helper`]. `args` carries the push spec, e.g.
+/// `["--delete", "doomed"]`.
+fn push_with_helper(local: &Path, node_url: &str, args: &[&str]) -> (bool, std::process::Output) {
+    let helper_bin = PathBuf::from(env!("CARGO_BIN_EXE_git-remote-gitlawb"));
+    let helper_dir = helper_bin.parent().unwrap().to_path_buf();
+    let path_env = match std::env::var_os("PATH") {
+        Some(p) => {
+            let mut dirs = vec![helper_dir.clone()];
+            dirs.extend(std::env::split_paths(&p));
+            std::env::join_paths(dirs).unwrap()
+        }
+        None => helper_dir.clone().into_os_string(),
+    };
+
+    let mut cmd = Command::new("git");
+    cmd.arg("-C")
+        .arg(local)
+        .arg("push")
+        .arg("origin")
+        .args(args)
+        .env("PATH", path_env)
+        .env("LC_ALL", "C")
+        .env("GITLAWB_NODE", node_url)
+        .env("GITLAWB_KEY", "/nonexistent-key-for-anon-push");
     run_bounded(cmd, Duration::from_secs(30))
 }
 
@@ -1259,70 +1315,76 @@ fn run_bounded_bounds_join_when_leader_exits_leaving_a_pipe_holder() {
     );
 }
 
-/// The plaintext-transport gate, proven at its CALL SITE rather than in
-/// isolation. `main.rs`'s unit tests cover `is_insecure_remote` and
-/// `check_transport_security` as functions; neither notices if the call in
-/// `main()` is deleted, which is the regression that would silently restore the
-/// cleartext hop. Only driving the built binary binds the wiring.
-///
-/// `192.0.2.1` is TEST-NET-1 (RFC 5737): non-loopback, reserved for
-/// documentation, and never routable, so the gate is what stops this and no
-/// connection is attempted even if it regressed.
+/// #369 end-to-end: a real `git push --delete` through the helper completes.
+/// The receive-pack request for a delete-only push is commands + flush with NO
+/// pack, and git holds its write pipe open for the report — so a helper that
+/// reads stdin to EOF deadlocks against it. The shim pipes the POST to a real
+/// `git receive-pack --stateless-rpc`, so the branch actually has to be deleted
+/// server-side for the assertions to pass.
 #[test]
-fn real_git_fetch_refuses_a_remote_plaintext_node() {
-    // A shallow fixture is enough: the gate fires before any negotiation.
-    let repos = build_divergent_repos(1);
-    let (ok, out) = fetch_with_helper(&repos.clone, "http://192.0.2.1:7545");
-    let stderr = String::from_utf8_lossy(&out.stderr);
+fn real_git_push_delete_completes() {
+    let dir = tempfile::tempdir().unwrap();
+    let server = dir.path().join("server.git");
+    let local = dir.path().join("local");
 
-    assert!(
-        !ok || !out.status.success(),
-        "the fetch must not succeed: {stderr}"
-    );
-    assert!(
-        stderr.contains("plaintext http"),
-        "the helper must refuse the cleartext hop and say so; stderr was: {stderr}"
-    );
-    assert!(
-        stderr.contains("GITLAWB_ALLOW_INSECURE_HTTP"),
-        "the refusal must name the escape hatch; stderr was: {stderr}"
-    );
-}
-
-/// A proxy variable must not divert a LOOPBACK node off this machine.
-///
-/// reqwest's default client honours `HTTP_PROXY` for a loopback URL, so before
-/// the fix a proxy pointing off-machine carried the signed plaintext request to
-/// it, past the transport guard, which only ever inspected the URL. Observed
-/// directly: `proxy(http://...:9999/) intercepts 'http://127.0.0.1:7545/'`.
-///
-/// The proxy here is a black hole on TEST-NET-1, so if the bypass regresses this
-/// fetch cannot succeed: it either hangs to the harness timeout or fails to
-/// connect. Success is only possible when the request went straight to the shim.
-#[test]
-fn real_git_fetch_ignores_a_proxy_for_a_loopback_node() {
-    let repos = build_divergent_repos(1);
-    let (server, clone) = (repos.server.clone(), repos.clone.clone());
-    let shim = start_shim(server.clone(), ShimMode::Normal);
-
-    let (completed, out) = fetch_with_helper_env(
-        &clone,
-        &shim.base_url,
+    // A bare repo holding the doomed branch, plus a live one.
+    git(
+        dir.path(),
         &[
-            ("HTTP_PROXY", "http://192.0.2.1:9999"),
-            ("http_proxy", "http://192.0.2.1:9999"),
-            ("ALL_PROXY", "http://192.0.2.1:9999"),
+            "init",
+            "--bare",
+            server.to_str().unwrap(),
+            "--initial-branch=main",
         ],
     );
+    let work = dir.path().join("work");
+    git(
+        dir.path(),
+        &["init", work.to_str().unwrap(), "--initial-branch=main"],
+    );
+    git(&work, &["config", "user.email", "t@t"]);
+    git(&work, &["config", "user.name", "t"]);
+    std::fs::write(work.join("f"), "x").unwrap();
+    git(&work, &["add", "f"]);
+    git(&work, &["commit", "-m", "c"]);
+    git(&work, &["branch", "doomed"]);
+    git(&work, &["push", server.to_str().unwrap(), "main", "doomed"]);
+
+    // The pusher: a local repo whose origin is the gitlawb remote pointing at
+    // the shim. Nothing needs to be fetched first; git takes the old-oid from
+    // the receive-pack advertisement.
+    git(
+        dir.path(),
+        &["init", local.to_str().unwrap(), "--initial-branch=main"],
+    );
+    git(
+        &local,
+        &["remote", "add", "origin", "gitlawb://did:key:z6MkTest/repo"],
+    );
+
+    let shim = start_shim(server.clone(), ShimMode::Normal);
+    let (completed, out) = push_with_helper(&local, &shim.base_url, &["--delete", "doomed"]);
+    let posts = shim.posts.load(Ordering::SeqCst);
 
     assert!(
         completed,
-        "the fetch did not finish; a proxied loopback request would stall here. stderr:\n{}",
+        "git push --delete did not complete within the timeout (deadlock signature). stderr:\n{}",
         String::from_utf8_lossy(&out.stderr)
     );
     assert!(
         out.status.success(),
-        "a loopback fetch must ignore the proxy variables. stderr:\n{}",
+        "git push --delete failed. stderr:\n{}",
         String::from_utf8_lossy(&out.stderr)
     );
+    assert_eq!(posts, 1, "a delete-only push is exactly one POST");
+    let gone = Command::new("git")
+        .arg("-C")
+        .arg(&server)
+        .args(["rev-parse", "--verify", "--quiet", "refs/heads/doomed"])
+        .output()
+        .unwrap();
+    assert!(
+        !gone.status.success(),
+        "refs/heads/doomed must be deleted in the server repo"
+    );
 }