From 8def1cae7bb28b5d0f7ee427b7ef95f21bf141ab Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:18:08 -0500 Subject: [PATCH 1/5] fix(gl): refuse signed requests to a plaintext remote --- crates/gl/src/http.rs | 197 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 197 insertions(+) diff --git a/crates/gl/src/http.rs b/crates/gl/src/http.rs index 7a28c6f7..a22f02ab 100644 --- a/crates/gl/src/http.rs +++ b/crates/gl/src/http.rs @@ -19,6 +19,74 @@ const MAX_ICAPTCHA_RETRIES: usize = 2; /// response body, so it bounds a slow download and not just a slow handshake. const TOTAL_REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); +/// Whether `url` would send a signed request 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 `Signature` header and +/// the body 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::() { + 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 to sign a request destined for a cleartext hop off this machine +/// unless the operator has opted in. `GITLAWB_ALLOW_INSECURE_HTTP` exists for a +/// private LAN where the operator has decided that is acceptable; its presence +/// alone opts in, matching git-remote-gitlawb's guard for the same hop. +fn ensure_signing_transport(node_base: &str, allow_insecure: bool) -> Result<()> { + if allow_insecure || !is_insecure_remote(node_base) { + return Ok(()); + } + anyhow::bail!( + "refusing to send a signed request to {node_base} over plaintext http.\n\ + The request is signed but not encrypted, so the Signature header and \ + the body 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)." + ) +} + +/// The operator opt-in for a plaintext remote, read at request time so a test +/// or shell can set it without rebuilding the client. +fn insecure_http_allowed() -> bool { + std::env::var_os("GITLAWB_ALLOW_INSECURE_HTTP").is_some() +} + /// Follow a redirect only when it stays on the origin that issued it AND re-issues the /// identical request-target, and only for as long as the chain bound allows. /// @@ -115,6 +183,7 @@ impl NodeClient { .keypair .as_ref() .context("get_signed requires an identity keypair")?; + ensure_signing_transport(&self.node_url, insecure_http_allowed())?; let signed = sign_request(kp, "GET", path, b""); let req = self .inner @@ -133,6 +202,7 @@ impl NodeClient { let url = format!("{}{}", self.node_url, path); let mut req = self.inner.get(&url); if let Some(kp) = &self.keypair { + ensure_signing_transport(&self.node_url, insecure_http_allowed())?; let signed = sign_request(kp, "GET", path, b""); req = req .header("Content-Digest", signed.content_digest) @@ -214,6 +284,7 @@ impl NodeClient { .body(body.to_vec()); if let Some(kp) = &self.keypair { + ensure_signing_transport(&self.node_url, insecure_http_allowed())?; let signed = sign_request(kp, method, path, body); req = req .header("Content-Digest", signed.content_digest) @@ -1268,4 +1339,130 @@ mod tests { let out = sanitize_node_msg("ok \u{0627}\u{200D}b"); assert_eq!(out, "ok \u{0627}\u{200D}b"); } + + // ── #413 plaintext-remote signing guard ────────────────────────────── + + /// Holds [`ICAPTCHA_ENV_LOCK`] while it points `GITLAWB_ALLOW_INSECURE_HTTP` + /// at `value` (or removes it), restoring the prior value on drop. + struct InsecureHttpEnv { + _lock: MutexGuard<'static, ()>, + prev: Option, + } + + impl InsecureHttpEnv { + fn set(value: Option<&str>) -> Self { + let lock = ICAPTCHA_ENV_LOCK.lock().unwrap(); + let prev = std::env::var_os("GITLAWB_ALLOW_INSECURE_HTTP"); + match value { + Some(v) => std::env::set_var("GITLAWB_ALLOW_INSECURE_HTTP", v), + None => std::env::remove_var("GITLAWB_ALLOW_INSECURE_HTTP"), + } + InsecureHttpEnv { _lock: lock, prev } + } + } + + impl Drop for InsecureHttpEnv { + fn drop(&mut self) { + match self.prev.take() { + Some(v) => std::env::set_var("GITLAWB_ALLOW_INSECURE_HTTP", v), + None => std::env::remove_var("GITLAWB_ALLOW_INSECURE_HTTP"), + } + } + } + + #[test] + fn insecure_remote_flags_only_plaintext_off_machine() { + // Loopback of every spelling is this machine and exempt. + for url in [ + "http://localhost", + "http://localhost:7545", + "http://127.0.0.1:7545", + "http://127.0.0.2", + "http://[::1]", + "http://[::ffff:127.0.0.1]", + // https to anywhere is encrypted; the guard is not its business. + "https://node.gitlawb.com", + // Not a URL: unparseable input fails later with its own error. + "not a url", + ] { + assert!(!is_insecure_remote(url), "{url} must not be flagged"); + } + // Plaintext http off this machine is the flagged shape. + for url in [ + "http://10.0.0.36:7777", + "http://node.example.com", + "http://[fd00::1]", + "http://127.0.0.1.evil.example", + ] { + assert!(is_insecure_remote(url), "{url} must be flagged"); + } + } + + #[test] + fn signing_transport_refuses_plaintext_remote() { + let err = ensure_signing_transport("http://10.0.0.36:7777", false).unwrap_err(); + assert!( + err.to_string().contains("plaintext http") + && err.to_string().contains("GITLAWB_ALLOW_INSECURE_HTTP"), + "the refusal names the risk and the opt-in: {err}" + ); + // Opted in, loopback, and https all pass. + assert!(ensure_signing_transport("http://10.0.0.36:7777", true).is_ok()); + assert!(ensure_signing_transport("http://localhost:7545", false).is_ok()); + assert!(ensure_signing_transport("https://node.gitlawb.com", false).is_ok()); + } + + /// A signed call against a plaintext remote errors out before any request + /// leaves; the refusal must beat the send. `.invalid` NXDOMAINs fast, so a + /// missed guard surfaces as a DNS error rather than a slow timeout. + #[tokio::test] + async fn get_signed_refuses_plaintext_remote_before_sending() { + let _env = InsecureHttpEnv::set(None); + let client = NodeClient::new("http://gl-nonexistent-node.invalid", Some(test_keypair())); + let err = client.get_signed("/api/v1/x").await.unwrap_err(); + assert!( + err.to_string().contains("plaintext http"), + "expected the transport refusal, got: {err}" + ); + } + + /// With the opt-in set, the same call passes the guard and fails later on + /// the unresolvable host, proving the env var unblocks the send rather than + /// merely being read. + #[tokio::test] + async fn get_signed_proceeds_when_insecure_http_opted_in() { + let _env = InsecureHttpEnv::set(Some("1")); + let client = NodeClient::new("http://gl-nonexistent-node.invalid", Some(test_keypair())); + let err = client.get_signed("/api/v1/x").await.unwrap_err(); + assert!( + !err.to_string().contains("plaintext http"), + "the opt-in should let the request attempt the hop, got: {err}" + ); + } + + /// Unsigned calls are not this guard's business: no keypair means no + /// signature to leak, and the request proceeds to its own failure. + #[tokio::test] + async fn unsigned_call_to_plaintext_remote_is_ungated() { + let _env = InsecureHttpEnv::set(None); + let client = NodeClient::new("http://gl-nonexistent-node.invalid", None); + let err = client.post("/api/v1/x", b"{}").await.unwrap_err(); + assert!( + !err.to_string().contains("plaintext http"), + "an unsigned request should not hit the signing guard, got: {err}" + ); + } + + /// The opt-in is presence-based, matching the remote helper's read. + #[test] + fn insecure_http_allowed_reads_the_env_var() { + { + let _env = InsecureHttpEnv::set(Some("1")); + assert!(insecure_http_allowed()); + } + { + let _env = InsecureHttpEnv::set(None); + assert!(!insecure_http_allowed()); + } + } } From 9ed392f91fc87a88dd4e169eba56e2293b6b5d81 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:53:28 -0500 Subject: [PATCH 2/5] fix(gl): refuse signed loopback http when a proxy carries it off-machine --- crates/gl/src/http.rs | 143 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 141 insertions(+), 2 deletions(-) diff --git a/crates/gl/src/http.rs b/crates/gl/src/http.rs index a22f02ab..edf60774 100644 --- a/crates/gl/src/http.rs +++ b/crates/gl/src/http.rs @@ -63,12 +63,75 @@ fn is_insecure_remote(url: &str) -> bool { true } +/// True when `no_proxy` covers `host`: a `*` entry, an exact match, or a +/// domain suffix match (`example.com` covers `a.example.com`). +fn no_proxy_covers(host: &str, no_proxy: &str) -> bool { + no_proxy + .split(',') + .map(str::trim) + .map(|e| e.trim_start_matches('.')) + .filter(|e| !e.is_empty()) + .any(|e| e == "*" || host == e || host.ends_with(&format!(".{e}"))) +} + +/// True when a plaintext request to a loopback `host` would still leave this +/// machine. reqwest reads HTTP_PROXY/ALL_PROXY (and lowercase) at client +/// build; a configured proxy without a NO_PROXY entry for the host carries the +/// signed request off-machine in cleartext, which is the leak this module +/// guards against. +fn loopback_goes_off_machine(host: &str) -> bool { + let proxied = ["HTTP_PROXY", "http_proxy", "ALL_PROXY", "all_proxy"] + .iter() + .any(|k| std::env::var_os(k).is_some_and(|v| !v.is_empty())); + if !proxied { + return false; + } + let no_proxy = std::env::var("NO_PROXY") + .or_else(|_| std::env::var("no_proxy")) + .unwrap_or_default(); + !no_proxy_covers(host, &no_proxy) +} + +/// The normalized host when `url` is an `http://` loopback address, else None. +/// Same loopback rules as [`is_insecure_remote`]. +fn loopback_http_host(url: &str) -> Option { + let parsed = reqwest::Url::parse(url.trim()).ok()?; + if parsed.scheme() != "http" { + return None; + } + let host = parsed + .host_str()? + .trim_end_matches('.') + .to_ascii_lowercase(); + if host == "localhost" { + return Some(host); + } + let bare = host + .trim_start_matches('[') + .trim_end_matches(']') + .to_string(); + let ip = bare.parse::().ok()?; + if ip.is_loopback() { + return Some(bare); + } + if let std::net::IpAddr::V6(v6) = ip { + if v6.to_ipv4_mapped().is_some_and(|v4| v4.is_loopback()) { + return Some(bare); + } + } + None +} + /// Refuse to sign a request destined for a cleartext hop off this machine /// unless the operator has opted in. `GITLAWB_ALLOW_INSECURE_HTTP` exists for a /// private LAN where the operator has decided that is acceptable; its presence -/// alone opts in, matching git-remote-gitlawb's guard for the same hop. +/// alone opts in, matching git-remote-gitlawb's guard for the same hop. The +/// loopback exemption does not apply when a proxy would carry the request +/// off-machine anyway. fn ensure_signing_transport(node_base: &str, allow_insecure: bool) -> Result<()> { - if allow_insecure || !is_insecure_remote(node_base) { + let insecure = is_insecure_remote(node_base) + || loopback_http_host(node_base).is_some_and(|h| loopback_goes_off_machine(&h)); + if allow_insecure || !insecure { return Ok(()); } anyhow::bail!( @@ -1465,4 +1528,80 @@ mod tests { assert!(!insecure_http_allowed()); } } + + #[test] + fn no_proxy_covers_matches_exact_suffix_and_star() { + assert!(no_proxy_covers("localhost", "localhost,127.0.0.1")); + assert!(no_proxy_covers("127.0.0.1", "localhost, 127.0.0.1")); + assert!(no_proxy_covers("a.example.com", ".example.com")); + assert!(no_proxy_covers("a.example.com", "example.com")); + assert!(no_proxy_covers("anything", "*")); + assert!(!no_proxy_covers("localhost", "127.0.0.1")); + assert!(!no_proxy_covers("127.0.0.1", "")); + assert!(!no_proxy_covers("notevil.com", "evil.com")); + } + + /// A proxied loopback plaintext hop must be refused. The env plumbing is + /// checked in a re-execed child so the process-global proxy vars never + /// touch sibling tests' reqwest clients. + #[test] + fn proxied_loopback_http_is_refused() { + if std::env::var_os("GL_PROXY_CHILD").is_none() { + let out = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "tests::http::tests::proxied_loopback_http_is_refused", + ]) + .env("GL_PROXY_CHILD", "1") + .env("HTTP_PROXY", "http://10.9.9.9:3128") + .env_remove("http_proxy") + .env_remove("ALL_PROXY") + .env_remove("all_proxy") + .env_remove("NO_PROXY") + .env_remove("no_proxy") + .env_remove("GITLAWB_ALLOW_INSECURE_HTTP") + .output() + .unwrap(); + assert!( + out.status.success(), + "child failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + return; + } + let err = + ensure_signing_transport("http://127.0.0.1:1", insecure_http_allowed()).unwrap_err(); + assert!( + err.to_string().contains("plaintext http"), + "a proxy without NO_PROXY carries the signed request off-machine: {err}" + ); + } + + /// And NO_PROXY covering the host restores the loopback exemption. + #[test] + fn proxied_loopback_with_no_proxy_is_allowed() { + if std::env::var_os("GL_PROXY_CHILD2").is_none() { + let out = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "tests::http::tests::proxied_loopback_with_no_proxy_is_allowed", + ]) + .env("GL_PROXY_CHILD2", "1") + .env("HTTP_PROXY", "http://10.9.9.9:3128") + .env("NO_PROXY", "localhost,127.0.0.1") + .env_remove("GITLAWB_ALLOW_INSECURE_HTTP") + .output() + .unwrap(); + assert!( + out.status.success(), + "child failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + return; + } + assert!( + ensure_signing_transport("http://127.0.0.1:1", insecure_http_allowed()).is_ok(), + "NO_PROXY covering the host keeps the request on this machine" + ); + } } From 07b8234b9b6ed5c7a6240199efda1f2b44dee512 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Fri, 11 Sep 2026 12:14:49 -0500 Subject: [PATCH 3/5] fix(gl): judge the client's build-time proxy state, and prove the env tests run --- crates/gl/src/http.rs | 164 ++++++++++++++++++++++++------------------ 1 file changed, 95 insertions(+), 69 deletions(-) diff --git a/crates/gl/src/http.rs b/crates/gl/src/http.rs index edf60774..b5ccbaeb 100644 --- a/crates/gl/src/http.rs +++ b/crates/gl/src/http.rs @@ -64,32 +64,23 @@ fn is_insecure_remote(url: &str) -> bool { } /// True when `no_proxy` covers `host`: a `*` entry, an exact match, or a -/// domain suffix match (`example.com` covers `a.example.com`). +/// domain suffix match (`example.com` covers `a.example.com`). Entries match +/// case-insensitively, as DNS names are. fn no_proxy_covers(host: &str, no_proxy: &str) -> bool { no_proxy .split(',') .map(str::trim) - .map(|e| e.trim_start_matches('.')) + .map(|e| e.trim_start_matches('.').to_ascii_lowercase()) .filter(|e| !e.is_empty()) .any(|e| e == "*" || host == e || host.ends_with(&format!(".{e}"))) } /// True when a plaintext request to a loopback `host` would still leave this -/// machine. reqwest reads HTTP_PROXY/ALL_PROXY (and lowercase) at client -/// build; a configured proxy without a NO_PROXY entry for the host carries the -/// signed request off-machine in cleartext, which is the leak this module -/// guards against. -fn loopback_goes_off_machine(host: &str) -> bool { - let proxied = ["HTTP_PROXY", "http_proxy", "ALL_PROXY", "all_proxy"] - .iter() - .any(|k| std::env::var_os(k).is_some_and(|v| !v.is_empty())); - if !proxied { - return false; - } - let no_proxy = std::env::var("NO_PROXY") - .or_else(|_| std::env::var("no_proxy")) - .unwrap_or_default(); - !no_proxy_covers(host, &no_proxy) +/// machine under the proxy env the client was built with. A configured proxy +/// without a NO_PROXY entry for the host carries the signed request +/// off-machine in cleartext, which is the leak this module guards against. +fn loopback_goes_off_machine(host: &str, proxy_env: &(bool, String)) -> bool { + proxy_env.0 && !no_proxy_covers(host, &proxy_env.1) } /// The normalized host when `url` is an `http://` loopback address, else None. @@ -127,10 +118,15 @@ fn loopback_http_host(url: &str) -> Option { /// private LAN where the operator has decided that is acceptable; its presence /// alone opts in, matching git-remote-gitlawb's guard for the same hop. The /// loopback exemption does not apply when a proxy would carry the request -/// off-machine anyway. -fn ensure_signing_transport(node_base: &str, allow_insecure: bool) -> Result<()> { +/// off-machine anyway; `proxy_env` is the proxy state the client was built +/// with, so the guard judges the same routing the client will use. +fn ensure_signing_transport( + node_base: &str, + allow_insecure: bool, + proxy_env: &(bool, String), +) -> Result<()> { let insecure = is_insecure_remote(node_base) - || loopback_http_host(node_base).is_some_and(|h| loopback_goes_off_machine(&h)); + || loopback_http_host(node_base).is_some_and(|h| loopback_goes_off_machine(&h, proxy_env)); if allow_insecure || !insecure { return Ok(()); } @@ -189,6 +185,11 @@ fn same_origin_redirect(attempt: reqwest::redirect::Attempt<'_>) -> reqwest::red pub struct NodeClient { inner: reqwest::Client, + /// Proxy env state captured at client build, so the transport guard judges + /// the same routing the client was built with. `(proxied, no_proxy)` where + /// `proxied` means HTTP_PROXY/ALL_PROXY was set and `no_proxy` is the raw + /// NO_PROXY value. + proxy_env: (bool, String), pub node_url: String, keypair: Option, } @@ -212,10 +213,17 @@ impl NodeClient { .user_agent(format!("gl/{} gitlawb-cli", env!("CARGO_PKG_VERSION"))) .build() .expect("failed to build HTTP client"); + let proxied = ["HTTP_PROXY", "http_proxy", "ALL_PROXY", "all_proxy"] + .iter() + .any(|k| std::env::var_os(k).is_some_and(|v| !v.is_empty())); + let no_proxy = std::env::var("NO_PROXY") + .or_else(|_| std::env::var("no_proxy")) + .unwrap_or_default(); Self { inner, node_url: node_url.into(), keypair, + proxy_env: (proxied, no_proxy), } } @@ -246,7 +254,7 @@ impl NodeClient { .keypair .as_ref() .context("get_signed requires an identity keypair")?; - ensure_signing_transport(&self.node_url, insecure_http_allowed())?; + ensure_signing_transport(&self.node_url, insecure_http_allowed(), &self.proxy_env)?; let signed = sign_request(kp, "GET", path, b""); let req = self .inner @@ -265,7 +273,7 @@ impl NodeClient { let url = format!("{}{}", self.node_url, path); let mut req = self.inner.get(&url); if let Some(kp) = &self.keypair { - ensure_signing_transport(&self.node_url, insecure_http_allowed())?; + ensure_signing_transport(&self.node_url, insecure_http_allowed(), &self.proxy_env)?; let signed = sign_request(kp, "GET", path, b""); req = req .header("Content-Digest", signed.content_digest) @@ -347,7 +355,7 @@ impl NodeClient { .body(body.to_vec()); if let Some(kp) = &self.keypair { - ensure_signing_transport(&self.node_url, insecure_http_allowed())?; + ensure_signing_transport(&self.node_url, insecure_http_allowed(), &self.proxy_env)?; let signed = sign_request(kp, method, path, body); req = req .header("Content-Digest", signed.content_digest) @@ -1463,16 +1471,18 @@ mod tests { #[test] fn signing_transport_refuses_plaintext_remote() { - let err = ensure_signing_transport("http://10.0.0.36:7777", false).unwrap_err(); + let no_proxy_env = (false, String::new()); + let err = + ensure_signing_transport("http://10.0.0.36:7777", false, &no_proxy_env).unwrap_err(); assert!( err.to_string().contains("plaintext http") && err.to_string().contains("GITLAWB_ALLOW_INSECURE_HTTP"), "the refusal names the risk and the opt-in: {err}" ); // Opted in, loopback, and https all pass. - assert!(ensure_signing_transport("http://10.0.0.36:7777", true).is_ok()); - assert!(ensure_signing_transport("http://localhost:7545", false).is_ok()); - assert!(ensure_signing_transport("https://node.gitlawb.com", false).is_ok()); + assert!(ensure_signing_transport("http://10.0.0.36:7777", true, &no_proxy_env).is_ok()); + assert!(ensure_signing_transport("http://localhost:7545", false, &no_proxy_env).is_ok()); + assert!(ensure_signing_transport("https://node.gitlawb.com", false, &no_proxy_env).is_ok()); } /// A signed call against a plaintext remote errors out before any request @@ -1541,36 +1551,58 @@ mod tests { assert!(!no_proxy_covers("notevil.com", "evil.com")); } + /// Re-exec this test binary running exactly `child_test` under the given + /// env, and prove the child actually ran one test rather than filter- + /// matching nothing (a wrong `--exact` path exits 0 vacuously). + fn run_env_child(child_test: &str, envs: &[(&str, &str)]) { + let mut cmd = std::process::Command::new(std::env::current_exe().unwrap()); + cmd.args(["--exact", child_test, "--nocapture"]); + // Clear any inherited proxy state first, then apply the case's env; + // env_remove after envs would silently undo a provided NO_PROXY. + for k in [ + "HTTP_PROXY", + "http_proxy", + "ALL_PROXY", + "all_proxy", + "NO_PROXY", + "no_proxy", + "GITLAWB_ALLOW_INSECURE_HTTP", + ] { + cmd.env_remove(k); + } + cmd.envs(envs.iter().copied()); + let out = cmd.output().unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains("1 passed"), + "the child must run the test, not exit green on a filter miss. \ + stdout:\n{stdout}\nstderr:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + out.status.success(), + "child failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + } + /// A proxied loopback plaintext hop must be refused. The env plumbing is /// checked in a re-execed child so the process-global proxy vars never /// touch sibling tests' reqwest clients. - #[test] - fn proxied_loopback_http_is_refused() { + #[tokio::test] + async fn proxied_loopback_http_is_refused() { if std::env::var_os("GL_PROXY_CHILD").is_none() { - let out = std::process::Command::new(std::env::current_exe().unwrap()) - .args([ - "--exact", - "tests::http::tests::proxied_loopback_http_is_refused", - ]) - .env("GL_PROXY_CHILD", "1") - .env("HTTP_PROXY", "http://10.9.9.9:3128") - .env_remove("http_proxy") - .env_remove("ALL_PROXY") - .env_remove("all_proxy") - .env_remove("NO_PROXY") - .env_remove("no_proxy") - .env_remove("GITLAWB_ALLOW_INSECURE_HTTP") - .output() - .unwrap(); - assert!( - out.status.success(), - "child failed: {}", - String::from_utf8_lossy(&out.stderr) + run_env_child( + "http::tests::proxied_loopback_http_is_refused", + &[ + ("GL_PROXY_CHILD", "1"), + ("HTTP_PROXY", "http://10.9.9.9:3128"), + ], ); return; } - let err = - ensure_signing_transport("http://127.0.0.1:1", insecure_http_allowed()).unwrap_err(); + let client = NodeClient::new("http://127.0.0.1:1", Some(test_keypair())); + let err = client.get_signed("/api/v1/x").await.unwrap_err(); assert!( err.to_string().contains("plaintext http"), "a proxy without NO_PROXY carries the signed request off-machine: {err}" @@ -1578,30 +1610,24 @@ mod tests { } /// And NO_PROXY covering the host restores the loopback exemption. - #[test] - fn proxied_loopback_with_no_proxy_is_allowed() { + #[tokio::test] + async fn proxied_loopback_with_no_proxy_is_allowed() { if std::env::var_os("GL_PROXY_CHILD2").is_none() { - let out = std::process::Command::new(std::env::current_exe().unwrap()) - .args([ - "--exact", - "tests::http::tests::proxied_loopback_with_no_proxy_is_allowed", - ]) - .env("GL_PROXY_CHILD2", "1") - .env("HTTP_PROXY", "http://10.9.9.9:3128") - .env("NO_PROXY", "localhost,127.0.0.1") - .env_remove("GITLAWB_ALLOW_INSECURE_HTTP") - .output() - .unwrap(); - assert!( - out.status.success(), - "child failed: {}", - String::from_utf8_lossy(&out.stderr) + run_env_child( + "http::tests::proxied_loopback_with_no_proxy_is_allowed", + &[ + ("GL_PROXY_CHILD2", "1"), + ("HTTP_PROXY", "http://10.9.9.9:3128"), + ("NO_PROXY", "localhost,127.0.0.1"), + ], ); return; } + let client = NodeClient::new("http://127.0.0.1:1", Some(test_keypair())); + let err = client.get_signed("/api/v1/x").await.unwrap_err(); assert!( - ensure_signing_transport("http://127.0.0.1:1", insecure_http_allowed()).is_ok(), - "NO_PROXY covering the host keeps the request on this machine" + !err.to_string().contains("plaintext http"), + "NO_PROXY covering the host keeps the request on this machine: {err}" ); } } From 935d98544d6bc2211951a615ae978220b234b6f1 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Fri, 11 Sep 2026 12:31:04 -0500 Subject: [PATCH 4/5] refactor(gl): run the transport guard before request construction --- crates/gl/src/http.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/gl/src/http.rs b/crates/gl/src/http.rs index b5ccbaeb..5d1fe765 100644 --- a/crates/gl/src/http.rs +++ b/crates/gl/src/http.rs @@ -271,9 +271,11 @@ impl NodeClient { /// to be authenticated. Mirrors the conditional signing of post/put/delete. pub async fn get_maybe_signed(&self, path: &str) -> Result { let url = format!("{}{}", self.node_url, path); + if self.keypair.is_some() { + ensure_signing_transport(&self.node_url, insecure_http_allowed(), &self.proxy_env)?; + } let mut req = self.inner.get(&url); if let Some(kp) = &self.keypair { - ensure_signing_transport(&self.node_url, insecure_http_allowed(), &self.proxy_env)?; let signed = sign_request(kp, "GET", path, b""); req = req .header("Content-Digest", signed.content_digest) @@ -348,6 +350,9 @@ impl NodeClient { proof: Option<&str>, ) -> Result { let url = format!("{}{}", self.node_url, path); + if self.keypair.is_some() { + ensure_signing_transport(&self.node_url, insecure_http_allowed(), &self.proxy_env)?; + } let mut req = self .inner .request(method.parse().expect("valid HTTP method"), &url) @@ -355,7 +360,6 @@ impl NodeClient { .body(body.to_vec()); if let Some(kp) = &self.keypair { - ensure_signing_transport(&self.node_url, insecure_http_allowed(), &self.proxy_env)?; let signed = sign_request(kp, method, path, body); req = req .header("Content-Digest", signed.content_digest) From d6ffeaf9fa8648a0f6d0fac9b039060665069967 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Fri, 11 Sep 2026 13:02:32 -0500 Subject: [PATCH 5/5] docs(gl): the guard is stricter than the helper's, not identical --- crates/gl/src/http.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/gl/src/http.rs b/crates/gl/src/http.rs index 5d1fe765..ca6e2602 100644 --- a/crates/gl/src/http.rs +++ b/crates/gl/src/http.rs @@ -116,9 +116,10 @@ fn loopback_http_host(url: &str) -> Option { /// Refuse to sign a request destined for a cleartext hop off this machine /// unless the operator has opted in. `GITLAWB_ALLOW_INSECURE_HTTP` exists for a /// private LAN where the operator has decided that is acceptable; its presence -/// alone opts in, matching git-remote-gitlawb's guard for the same hop. The +/// alone opts in, matching the variable git-remote-gitlawb's guard uses for +/// the same hop. This guard is stricter than the helper's in one respect: the /// loopback exemption does not apply when a proxy would carry the request -/// off-machine anyway; `proxy_env` is the proxy state the client was built +/// off-machine anyway. `proxy_env` is the proxy state the client was built /// with, so the guard judges the same routing the client will use. fn ensure_signing_transport( node_base: &str,