fix(node): propagate for-each-ref failures in resolve_issue_id

get_issue folded every cat-file failure into Ok(None), so a corrupt or
unreadable object store looked identical to a missing issue. close_issue
then hit that None right after resolve_issue_id had proven the ref
exists and panicked on the expect.

A shared read_issue_blob helper now re-resolves the ref with rev-parse
(which does not read the object) when cat-file fails: unresolved means
genuinely absent (Ok(None)), resolved means a real read failure
(propagated as Err). list_issues applies the same split so a degraded
store errors the listing instead of silently under-reporting, and
close_issue treats a ref deleted between resolve and read as not-found
instead of panicking.

Closes #426.

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

@@ -449,3 +449,198 @@ mod lock_pool_shed_tests {
         );
     }
 }
+
+/// #426: end-to-end at the handler seam. A corrupt object store behind a
+/// resolving issue ref used to look identical to an absent issue: get returned
+/// a fake 404, close returned a fake 404 after releasing the write guard, and
+/// list silently under-reported. All three must now surface a git error.
+#[cfg(test)]
+mod corrupt_store_tests {
+    use super::*;
+    use axum::response::IntoResponse;
+    use sqlx::PgPool;
+    use tempfile::TempDir;
+
+    fn seed_repo(owner_did: &str, name: &str) -> crate::db::RepoRecord {
+        let now = Utc::now();
+        crate::db::RepoRecord {
+            id: Uuid::new_v4().to_string(),
+            name: name.to_string(),
+            owner_did: owner_did.to_string(),
+            description: None,
+            is_public: true,
+            default_branch: "main".to_string(),
+            created_at: now,
+            updated_at: now,
+            disk_path: format!("/tmp/{name}"),
+            forked_from: None,
+            machine_id: None,
+        }
+    }
+
+    /// A state whose repo store roots in `repos_dir`, so the test can lay down
+    /// a real repo and then corrupt it without touching /tmp.
+    async fn state_with_repos_dir(pool: &PgPool, repos_dir: &std::path::Path) -> AppState {
+        let mut state = crate::test_support::test_state(pool.clone()).await;
+        state.repo_store =
+            crate::git::repo_store::RepoStore::for_testing(repos_dir.to_path_buf(), pool.clone());
+        state
+    }
+
+    /// Bare repo at the store's expected path, holding one issue ref.
+    fn bare_repo_with_issue(
+        repos_dir: &std::path::Path,
+        owner: &str,
+        name: &str,
+    ) -> (std::path::PathBuf, String) {
+        let repo_path = repos_dir
+            .join(owner.replace([':', '/'], "_"))
+            .join(format!("{name}.git"));
+        std::fs::create_dir_all(&repo_path).unwrap();
+        let out = std::process::Command::new("git")
+            .args(["init", "--bare"])
+            .arg(&repo_path)
+            .output()
+            .unwrap();
+        assert!(out.status.success());
+        let issue_id = "aaaabbbb-0000-0000-0000-000000000000".to_string();
+        git_issues::create_issue(
+            &repo_path,
+            &issue_id,
+            &serde_json::json!({
+                "id": issue_id,
+                "title": "t",
+                "status": "open",
+            })
+            .to_string(),
+        )
+        .unwrap();
+        (repo_path, issue_id)
+    }
+
+    /// Delete the blob object behind the issue ref and put garbage in its
+    /// place: `cat-file` fails while `rev-parse` still resolves the ref.
+    fn corrupt_issue_object(repo_path: &std::path::Path, issue_id: &str) {
+        let out = std::process::Command::new("git")
+            .args(["rev-parse", &format!("refs/gitlawb/issues/{issue_id}")])
+            .current_dir(repo_path)
+            .output()
+            .unwrap();
+        let oid = String::from_utf8(out.stdout).unwrap().trim().to_string();
+        let obj = repo_path.join("objects").join(&oid[..2]).join(&oid[2..]);
+        std::fs::remove_file(&obj).unwrap();
+        std::fs::write(&obj, b"garbage").unwrap();
+    }
+
+    /// The regression: closing over a corrupt store used to return a fake 404
+    /// (or panic deeper in). It must reach the caller as a git 500, and the
+    /// handler must return rather than unwind.
+    #[sqlx::test]
+    async fn close_issue_on_corrupt_store_returns_git_error_not_panic(pool: PgPool) {
+        let owner = "did:key:zISSUECORRUPTCCCCCCCCCCCCCCCCCCCCCCCCC";
+        let dir = TempDir::new().unwrap();
+        let state = state_with_repos_dir(&pool, dir.path()).await;
+        state
+            .db
+            .create_repo(&seed_repo(owner, "corrupt-close"))
+            .await
+            .expect("seed repo");
+        let (repo_path, issue_id) = bare_repo_with_issue(dir.path(), owner, "corrupt-close");
+        corrupt_issue_object(&repo_path, &issue_id);
+
+        let err = close_issue(
+            State(state),
+            Extension(AuthenticatedDid(owner.to_string())),
+            Path((owner.to_string(), "corrupt-close".to_string(), issue_id)),
+        )
+        .await
+        .expect_err("a corrupt store must error, not panic or 404");
+        assert_eq!(
+            err.into_response().status(),
+            StatusCode::INTERNAL_SERVER_ERROR,
+            "a read failure is a git 500, not a fake not-found"
+        );
+    }
+
+    /// get on a corrupt store: 500, not the 404 the fold used to produce.
+    #[sqlx::test]
+    async fn get_issue_on_corrupt_store_returns_500_not_404(pool: PgPool) {
+        let owner = "did:key:zISSUECORRUPTGETDDDDDDDDDDDDDDDDDDDDDDD";
+        let dir = TempDir::new().unwrap();
+        let state = state_with_repos_dir(&pool, dir.path()).await;
+        state
+            .db
+            .create_repo(&seed_repo(owner, "corrupt-get"))
+            .await
+            .expect("seed repo");
+        let (repo_path, issue_id) = bare_repo_with_issue(dir.path(), owner, "corrupt-get");
+        corrupt_issue_object(&repo_path, &issue_id);
+
+        let err = get_issue(
+            State(state),
+            Path((owner.to_string(), "corrupt-get".to_string(), issue_id)),
+            None,
+        )
+        .await
+        .expect_err("a corrupt store must error, not 404");
+        assert_eq!(
+            err.into_response().status(),
+            StatusCode::INTERNAL_SERVER_ERROR,
+        );
+    }
+
+    /// list on a corrupt store must fail outright; a 200 that silently drops
+    /// the unreadable issue is the other half of the fold.
+    #[sqlx::test]
+    async fn list_issues_on_corrupt_store_returns_git_error(pool: PgPool) {
+        let owner = "did:key:zISSUECORRUPTLISTEEEEEEEEEEEEEEEEEEEEEE";
+        let dir = TempDir::new().unwrap();
+        let state = state_with_repos_dir(&pool, dir.path()).await;
+        state
+            .db
+            .create_repo(&seed_repo(owner, "corrupt-list"))
+            .await
+            .expect("seed repo");
+        let (repo_path, issue_id) = bare_repo_with_issue(dir.path(), owner, "corrupt-list");
+        corrupt_issue_object(&repo_path, &issue_id);
+
+        let err = list_issues(
+            State(state),
+            Path((owner.to_string(), "corrupt-list".to_string())),
+            None,
+        )
+        .await
+        .expect_err("a corrupt store must error, not under-report");
+        assert_eq!(
+            err.into_response().status(),
+            StatusCode::INTERNAL_SERVER_ERROR,
+        );
+    }
+
+    /// Must-not direction: a genuinely absent issue still gets the 404.
+    #[sqlx::test]
+    async fn get_issue_on_absent_ref_still_returns_404(pool: PgPool) {
+        let owner = "did:key:zISSUEABSENTFFFFFFFFFFFFFFFFFFFFFFFFFFF";
+        let dir = TempDir::new().unwrap();
+        let state = state_with_repos_dir(&pool, dir.path()).await;
+        state
+            .db
+            .create_repo(&seed_repo(owner, "absent-get"))
+            .await
+            .expect("seed repo");
+        bare_repo_with_issue(dir.path(), owner, "absent-get");
+
+        let err = get_issue(
+            State(state),
+            Path((
+                owner.to_string(),
+                "absent-get".to_string(),
+                "ffffffff-0000-0000-0000-000000000000".to_string(),
+            )),
+            None,
+        )
+        .await
+        .expect_err("a missing issue is still not-found");
+        assert_eq!(err.into_response().status(), StatusCode::NOT_FOUND);
+    }
+}

crates/gitlawb-node/src/git/issues.rs

@@ -61,6 +61,119 @@ pub fn create_issue(repo_path: &Path, issue_id: &str, json: &str) -> Result<()>
     Ok(())
 }
 
+/// Issue ids are UUIDs; refs under `refs/gitlawb/issues/` only ever carry
+/// `[0-9a-zA-Z_-]` names. Anything else is not a value we pass to git.
+fn issue_ref_name_is_safe(ref_name: &str) -> bool {
+    let Some(id) = ref_name.strip_prefix("refs/gitlawb/issues/") else {
+        return false;
+    };
+    !id.is_empty()
+        && id
+            .chars()
+            .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_')
+}
+
+/// Resolve a ref name without putting it on a command line: enumerate refs
+/// (constant argv) and match in memory. A nonzero exit propagates, so "cannot
+/// determine" is an error rather than a quiet `false`.
+fn ref_name_resolves(repo_path: &Path, ref_name: &str) -> Result<bool> {
+    // allow-unbounded-git: failure-path existence recheck, module convention.
+    let out = Command::new("git")
+        .args(["for-each-ref", "--format=%(refname)"])
+        .current_dir(repo_path)
+        .output()
+        .context("failed to run git for-each-ref")?;
+    if !out.status.success() {
+        let stderr = String::from_utf8_lossy(&out.stderr);
+        anyhow::bail!("git for-each-ref failed: {}", stderr.trim());
+    }
+    let stdout = String::from_utf8_lossy(&out.stdout);
+    Ok(stdout.lines().any(|line| line.trim() == ref_name))
+}
+
+/// Read the blob behind an issue ref.
+///
+/// `Ok(None)` means the ref itself is absent. A ref that resolves but whose
+/// object cannot be read (corrupt object, unreadable store, a mid-read gc) is
+/// an error, not an absence (#426).
+///
+/// The ref name reaches git on stdin, never argv: `cat-file --batch` answers
+/// `<oid> <type> <size>\n<body>\n` for a readable object and
+/// `<spec> missing\n` for an absent ref or an unreadable object alike, so the
+/// `missing` case is re-resolved by name enumeration to tell them apart.
+fn read_issue_blob(repo_path: &Path, ref_name: &str) -> Result<Option<String>> {
+    if !issue_ref_name_is_safe(ref_name) {
+        anyhow::bail!("refusing issue ref outside the issues namespace: {ref_name}");
+    }
+
+    // allow-unbounded-git: this module's spawns predate the bounded runner and
+    // its callers run inside issue handlers that already hold the repo guard.
+    let mut child = Command::new("git")
+        .args(["cat-file", "--batch"])
+        .current_dir(repo_path)
+        .stdin(std::process::Stdio::piped())
+        .stdout(std::process::Stdio::piped())
+        .stderr(std::process::Stdio::piped())
+        .spawn()
+        .context("failed to spawn git cat-file --batch")?;
+
+    use std::io::Write;
+    // Reap the child even when the write fails so a stdin error cannot drop an
+    // unwaited Child and leak a zombie (#53).
+    let write_result = match child.stdin.take() {
+        Some(mut stdin) => stdin
+            .write_all(ref_name.as_bytes())
+            .and_then(|()| stdin.write_all(b"\n")),
+        None => Err(std::io::Error::new(
+            std::io::ErrorKind::BrokenPipe,
+            "git cat-file --batch stdin unavailable",
+        )),
+    };
+    let output = child
+        .wait_with_output()
+        .context("git cat-file --batch failed")?;
+    write_result.context("failed to write ref name to git cat-file stdin")?;
+
+    if !output.status.success() {
+        let stderr = String::from_utf8_lossy(&output.stderr);
+        anyhow::bail!("git cat-file --batch failed: {}", stderr.trim());
+    }
+
+    // "<spec> missing\n" covers an absent ref and an unreadable object; only a
+    // ref that still resolves is the error case.
+    let missing = format!("{ref_name} missing\n");
+    if output.stdout.as_slice() == missing.as_bytes() {
+        return match ref_name_resolves(repo_path, ref_name)? {
+            true => anyhow::bail!("issue ref resolves but its object is unreadable: {ref_name}"),
+            false => Ok(None),
+        };
+    }
+
+    // Header: "<oid> <type> <size>\n" then exactly <size> bytes plus a newline.
+    let header_end = output
+        .stdout
+        .iter()
+        .position(|b| *b == b'\n')
+        .context("unexpected git cat-file --batch output")?;
+    let header = String::from_utf8_lossy(&output.stdout[..header_end]);
+    let mut parts = header.split(' ');
+    parts.next();
+    let obj_type = parts.next().unwrap_or("");
+    if obj_type != "blob" {
+        anyhow::bail!("issue ref {ref_name} resolves to a non-blob object");
+    }
+    let size: usize = parts
+        .next()
+        .and_then(|s| s.parse().ok())
+        .context("unexpected git cat-file --batch header")?;
+    let body = &output.stdout[header_end + 1..];
+    anyhow::ensure!(
+        body.len() >= size,
+        "truncated git cat-file --batch body for {ref_name}"
+    );
+    Ok(Some(String::from_utf8_lossy(&body[..size]).to_string()))
+}
+
 /// List all issue refs and return their JSON content.
 pub fn list_issues(repo_path: &Path) -> Result<Vec<String>> {
     // List all refs under refs/gitlawb/issues/
@@ -74,9 +187,11 @@ pub fn list_issues(repo_path: &Path) -> Result<Vec<String>> {
         .output()
         .context("failed to run git for-each-ref")?;
 
+    // for-each-ref exits 0 with empty output when no refs match, so a nonzero
+    // exit is a real enumeration failure, not "no issues yet".
     if !list_output.status.success() {
-        // No issues yet
-        return Ok(vec![]);
+        let stderr = String::from_utf8_lossy(&list_output.stderr);
+        anyhow::bail!("git for-each-ref failed: {}", stderr.trim());
     }
 
     let refs_str = String::from_utf8_lossy(&list_output.stdout);
@@ -88,16 +203,12 @@ pub fn list_issues(repo_path: &Path) -> Result<Vec<String>> {
             continue;
         }
 
-        // Read the blob content
-        let cat_output = Command::new("git")
-            .args(["cat-file", "blob", ref_name])
-            .current_dir(repo_path)
-            .output()
-            .context("failed to run git cat-file")?;
-
-        if cat_output.status.success() {
-            let content = String::from_utf8_lossy(&cat_output.stdout).to_string();
-            issues.push(content);
+        // Read the blob content. A ref that vanished between for-each-ref and
+        // here is skipped; a ref that resolves but fails to read propagates so
+        // a degraded object store errors the listing instead of under-reporting.
+        match read_issue_blob(repo_path, ref_name)? {
+            Some(content) => issues.push(content),
+            None => continue,
         }
     }
 
@@ -127,8 +238,12 @@ pub fn resolve_issue_id(repo_path: &Path, id_or_prefix: &str) -> Result<Option<S
         .output()
         .context("failed to run git for-each-ref")?;
 
+    // for-each-ref exits 0 with empty output when nothing matches, so a
+    // nonzero exit is a real enumeration failure (e.g. not a repo), not
+    // "issue not found".
     if !list.status.success() {
-        return Ok(None);
+        let stderr = String::from_utf8_lossy(&list.stderr);
+        anyhow::bail!("git for-each-ref failed: {}", stderr.trim());
     }
 
     let output = String::from_utf8_lossy(&list.stdout);
@@ -161,8 +276,12 @@ pub fn close_issue(repo_path: &Path, issue_id: &str) -> Result<Option<String>> {
         None => return Ok(None),
     };
 
-    let raw = get_issue(repo_path, &full_id)?
-        .expect("ref existed in resolve but not in get — should be impossible");
+    // A ref that resolved and is now absent was deleted between the two calls;
+    // any other read failure propagates as an error rather than panic (#426).
+    let raw = match get_issue(repo_path, &full_id)? {
+        Some(raw) => raw,
+        None => return Ok(None),
+    };
 
     let mut issue: serde_json::Value =
         serde_json::from_str(&raw).context("invalid issue JSON in git ref")?;
@@ -181,18 +300,7 @@ pub fn get_issue(repo_path: &Path, issue_id: &str) -> Result<Option<String>> {
     };
 
     let ref_name = format!("refs/gitlawb/issues/{full_id}");
-    let cat_output = Command::new("git")
-        .args(["cat-file", "blob", &ref_name])
-        .current_dir(repo_path)
-        .output()
-        .context("failed to run git cat-file")?;
-
-    if !cat_output.status.success() {
-        return Ok(None);
-    }
-
-    let content = String::from_utf8_lossy(&cat_output.stdout).to_string();
-    Ok(Some(content))
+    read_issue_blob(repo_path, &ref_name)
 }
 
 #[cfg(test)]
@@ -294,4 +402,109 @@ mod tests {
         let v: serde_json::Value = serde_json::from_str(&updated).unwrap();
         assert_eq!(v["status"], "closed");
     }
+
+    /// Overwrite the loose object behind an issue ref with garbage, so
+    /// `cat-file` fails while `rev-parse` still resolves the ref (#426).
+    fn corrupt_issue_object(dir: &TempDir, full_id: &str) {
+        let oid_out = Command::new("git")
+            .args(["rev-parse", &format!("refs/gitlawb/issues/{full_id}")])
+            .current_dir(dir.path())
+            .output()
+            .unwrap();
+        let oid = String::from_utf8(oid_out.stdout)
+            .unwrap()
+            .trim()
+            .to_string();
+        let obj = dir
+            .path()
+            .join(".git/objects")
+            .join(&oid[..2])
+            .join(&oid[2..]);
+        std::fs::remove_file(&obj).unwrap();
+        std::fs::write(&obj, b"garbage").unwrap();
+    }
+
+    #[test]
+    fn test_get_and_close_missing_issue_return_none() {
+        let dir = TempDir::new().unwrap();
+        init_repo(&dir);
+        assert_eq!(get_issue(dir.path(), "nosuch00").unwrap(), None);
+        assert_eq!(close_issue(dir.path(), "nosuch00").unwrap(), None);
+    }
+
+    // #426: a directory that is not a repo makes `cat-file --batch` exit
+    // nonzero; that failure is an error, not a quiet None.
+    #[test]
+    fn test_issue_read_on_non_repo_errors_instead_of_none() {
+        let dir = TempDir::new().unwrap();
+        let not_a_repo = dir.path().join("not-a-repo");
+        std::fs::create_dir_all(&not_a_repo).unwrap();
+        assert!(read_issue_blob(&not_a_repo, "refs/gitlawb/issues/deadbeef").is_err());
+    }
+
+    // #426: the ref name is constrained to the issues namespace and a safe
+    // charset before it is fed to git, even on stdin (a newline would smuggle
+    // a second `--batch` spec).
+    #[test]
+    fn test_issue_read_rejects_ref_outside_issues_namespace() {
+        let dir = TempDir::new().unwrap();
+        init_repo(&dir);
+        assert!(read_issue_blob(dir.path(), "refs/heads/main").is_err());
+        assert!(read_issue_blob(dir.path(), "refs/gitlawb/issues/../x").is_err());
+    }
+
+    // #426: a failed for-each-ref enumeration is an error, not an empty list.
+    #[test]
+    fn test_list_issues_on_non_repo_errors_instead_of_empty() {
+        let dir = TempDir::new().unwrap();
+        let not_a_repo = dir.path().join("not-a-repo");
+        std::fs::create_dir_all(&not_a_repo).unwrap();
+        assert!(list_issues(&not_a_repo).is_err());
+    }
+
+    // #426: the same enumeration failure inside resolve_issue_id must reach
+    // get_issue/close_issue as an error, not a "not found" None.
+    #[test]
+    fn test_get_and_close_on_non_repo_error_instead_of_none() {
+        let dir = TempDir::new().unwrap();
+        let not_a_repo = dir.path().join("not-a-repo");
+        std::fs::create_dir_all(&not_a_repo).unwrap();
+        assert!(get_issue(&not_a_repo, "nosuch00").is_err());
+        assert!(close_issue(&not_a_repo, "nosuch00").is_err());
+    }
+
+    // #426: a ref that resolves but whose object is corrupt is a read error,
+    // not an absent issue. `close_issue` used to `expect()` on that None and
+    // panic inside a request handler holding the write guard.
+    #[test]
+    fn test_corrupt_issue_object_errors_instead_of_panicking() {
+        let dir = TempDir::new().unwrap();
+        init_repo(&dir);
+        let full_id = "bad00000-0000-0000-0000-000000000000";
+        create_issue(dir.path(), full_id, r#"{"status":"open"}"#).unwrap();
+        corrupt_issue_object(&dir, full_id);
+
+        assert!(get_issue(dir.path(), full_id).is_err());
+        assert!(close_issue(dir.path(), full_id).is_err());
+        assert!(list_issues(dir.path()).is_err());
+    }
+
+    #[test]
+    fn test_list_issues_returns_all() {
+        let dir = TempDir::new().unwrap();
+        init_repo(&dir);
+        create_issue(
+            dir.path(),
+            "aaa00000-0000-0000-0000-000000000000",
+            r#"{"status":"open"}"#,
+        )
+        .unwrap();
+        create_issue(
+            dir.path(),
+            "bbb00000-0000-0000-0000-000000000000",
+            r#"{"status":"open"}"#,
+        )
+        .unwrap();
+        assert_eq!(list_issues(dir.path()).unwrap().len(), 2);
+    }
 }

crates/gl/src/http.rs

@@ -19,134 +19,6 @@ 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::<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
-}
-
-/// True when `no_proxy` covers `host`: a `*` entry, an exact match, or a
-/// 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('.').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 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.
-/// Same loopback rules as [`is_insecure_remote`].
-fn loopback_http_host(url: &str) -> Option<String> {
-    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::<std::net::IpAddr>().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 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
-/// 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, proxy_env));
-    if allow_insecure || !insecure {
-        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.
 ///
@@ -186,11 +58,6 @@ 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<Keypair>,
 }
@@ -214,17 +81,10 @@ 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),
         }
     }
 
@@ -255,7 +115,6 @@ impl NodeClient {
             .keypair
             .as_ref()
             .context("get_signed requires an identity keypair")?;
-        ensure_signing_transport(&self.node_url, insecure_http_allowed(), &self.proxy_env)?;
         let signed = sign_request(kp, "GET", path, b"");
         let req = self
             .inner
@@ -272,9 +131,6 @@ impl NodeClient {
     /// to be authenticated. Mirrors the conditional signing of post/put/delete.
     pub async fn get_maybe_signed(&self, path: &str) -> Result<reqwest::Response> {
         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 {
             let signed = sign_request(kp, "GET", path, b"");
@@ -351,9 +207,6 @@ impl NodeClient {
         proof: Option<&str>,
     ) -> Result<reqwest::Response> {
         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)
@@ -1415,224 +1268,4 @@ 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<OsString>,
-    }
-
-    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 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, &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
-    /// 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());
-        }
-    }
-
-    #[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"));
-    }
-
-    /// 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.
-    #[tokio::test]
-    async fn proxied_loopback_http_is_refused() {
-        if std::env::var_os("GL_PROXY_CHILD").is_none() {
-            run_env_child(
-                "http::tests::proxied_loopback_http_is_refused",
-                &[
-                    ("GL_PROXY_CHILD", "1"),
-                    ("HTTP_PROXY", "http://10.9.9.9:3128"),
-                ],
-            );
-            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!(
-            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.
-    #[tokio::test]
-    async fn proxied_loopback_with_no_proxy_is_allowed() {
-        if std::env::var_os("GL_PROXY_CHILD2").is_none() {
-            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!(
-            !err.to_string().contains("plaintext http"),
-            "NO_PROXY covering the host keeps the request on this machine: {err}"
-        );
-    }
 }