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.

.github/codeql/codeql-config.yml

@@ -1,4 +0,0 @@
-name: "Gitlawb CodeQL config"
-paths-ignore:
-  - "**/tests/**"
-  - "**/test_support/**"

.github/workflows/pr-checks.yml

@@ -116,16 +116,6 @@ jobs:
           DATABASE_URL: postgres://postgres:postgres@localhost:5432/gitlawb_test
         run: cargo test --locked --workspace
 
-      # The real-node deny harness is behind the `test-harness` feature (so its
-      # spawn surface never compiles into the production binary), which means
-      # `--workspace` above skips it. Run it explicitly so the trust-boundary
-      # regression cases (INV-1/INV-2/INV-8) execute on every PR. Uses the same
-      # Postgres service; `#[sqlx::test]` provisions an isolated DB per test.
-      - name: cargo test (real-node deny harness)
-        env:
-          DATABASE_URL: postgres://postgres:postgres@localhost:5432/gitlawb_test
-        run: cargo test --locked -p gitlawb-node --features test-harness --lib --test deny_harness
-
   build-release:
     name: build --release
     runs-on: ubuntu-latest

Dockerfile

@@ -26,15 +26,11 @@ COPY crates/icaptcha-client/Cargo.toml crates/icaptcha-client/
 # is what let it sit dead from the moment gl gained a path dependency on
 # icaptcha-client. A failure here should stop the build, not silently turn every
 # image build into a cold one.
-# gitlawb-node declares BOTH a [[bin]] and a [lib], so it needs both stubs:
-# cargo resolves every declared target before it fetches anything, and a
-# missing src/lib.rs aborts the layer with a target resolution error.
 RUN mkdir -p crates/gitlawb-core/src crates/gitlawb-node/src crates/gl/src \
         crates/git-remote-gitlawb/src crates/gitlawb-attest/src crates/icaptcha-client/src && \
     echo 'fn main() {}' > crates/gitlawb-node/src/main.rs && \
     echo 'fn main() {}' > crates/gl/src/main.rs && \
     echo 'fn main() {}' > crates/git-remote-gitlawb/src/main.rs && \
-    echo '' > crates/gitlawb-node/src/lib.rs && \
     echo '' > crates/gitlawb-core/src/lib.rs && \
     echo '' > crates/gitlawb-attest/src/lib.rs && \
     echo '' > crates/icaptcha-client/src/lib.rs && \

crates/gitlawb-node/Cargo.toml

@@ -6,28 +6,10 @@ edition.workspace = true
 rust-version.workspace = true
 license.workspace = true
 
-[lib]
-name = "gitlawb_node"
-path = "src/lib.rs"
-
 [[bin]]
 name = "gitlawb-node"
 path = "src/main.rs"
 
-# Exposes the `test_harness` spawn surface (src/test_harness.rs) to the
-# real-node deny-harness integration crate. Off by default so the production
-# binary never compiles test-only boot code. Enable with
-# `cargo test -p gitlawb-node --features test-harness`.
-[features]
-test-harness = []
-
-# Only builds when `test-harness` is enabled; otherwise skipped (not an error),
-# so `cargo test --workspace` without the feature stays green.
-[[test]]
-name = "deny_harness"
-path = "tests/deny_harness.rs"
-required-features = ["test-harness"]
-
 [dependencies]
 gitlawb-core = { path = "../gitlawb-core" }
 ed25519-dalek = { workspace = true }
@@ -95,12 +77,6 @@ libp2p-dns = { version = "0.44.0", features = ["tokio"] }
 [dev-dependencies]
 mockito = "1"
 tempfile = "3"
-# test-util enables tokio::test(start_paused): the delayed claim re-sweep
-# test drives the real CLAIM_SWEEP_MIN_AGE-plus-margin sleep on paused time.
+# `test-util` for `#[tokio::test(start_paused = true)]`: lets the deadline tests assert
+# on which timer fired instead of on wall clock.
 tokio = { workspace = true, features = ["test-util"] }
-# Used by the deny-harness integration crate (tests/deny_harness.rs): the
-# #[sqlx::test] macro for an ephemeral per-test pool, and reqwest as the real
-# HTTP client that drives deny paths over the socket.
-sqlx = { version = "0.8", features = ["postgres", "runtime-tokio-rustls", "macros", "migrate"] }
-reqwest = { workspace = true }
-hex = "0.4"

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

@@ -205,10 +205,8 @@ pub struct Config {
     pub metrics_addr: String,
 
     /// Maximum time to wait for in-flight requests to drain on shutdown, in
-    /// seconds. On the shutdown signal the server stops accepting new
-    /// connections and gives in-flight requests this long to complete; at
-    /// expiry it abandons whatever remains (clients observe a closed
-    /// connection, not an error response) and exits. Default: 30s.
+    /// seconds. After this elapses, the server returns 503 to anything still
+    /// in flight and exits. Default: 30s.
     #[arg(long, env = "GITLAWB_SHUTDOWN_GRACE_SECS", default_value_t = 30)]
     pub shutdown_grace_secs: u64,
 

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

@@ -270,7 +270,7 @@ impl Db {
         &self.pool
     }
 
-    #[cfg(any(test, feature = "test-harness"))]
+    #[cfg(test)]
     pub fn for_testing(pool: PgPool) -> Self {
         Self { pool }
     }
@@ -279,7 +279,7 @@ impl Db {
     /// provisions an empty per-test database, so DB-backed tests must run this
     /// before seeding. Reuses the production `migrate()` path (the advisory lock
     /// is harmless on an isolated test DB and migrations are idempotent).
-    #[cfg(any(test, feature = "test-harness"))]
+    #[cfg(test)]
     pub(crate) async fn run_migrations(&self) -> Result<()> {
         self.migrate().await
     }
@@ -8278,8 +8278,8 @@ mod peer_authority_tests {
 /// | `prune_non_public_peers` (db/mod.rs) | a delete keyed on a computed bad-DID array; cannot repoint; boot-only caller in main.rs |
 /// | `seed_local_peer` (sync.rs) | excluded by test-module location: a deliberate `upsert_peer` bypass for `file://` fixtures, which the public-URL gate rejects |
 /// | `a_legacy_row_can_still_refresh_its_liveness` (db/mod.rs) | test-only. Seeds a PRE-GATE row by raw SQL on purpose: `upsert_peer` cannot create one, since the gate it is testing refuses exactly that DID. The fixture models what a deployed table already holds |
+/// | `gossip_ping_round_requires_two_failures_before_persisting_unreachable` (main.rs) | test-only fixture seed. Raw SQL because the test drives the readiness HYSTERESIS, which needs a row already at `last_ping_ok = TRUE` before the round runs; it never exercises the announce gate |
 /// | `manual_ping_uses_readiness_without_mutating_federation_gate` (api/peers.rs) | test-only fixture seed, same shape and same reason: the row under test must pre-exist so the assertion is about what the ping does NOT rewrite |
-/// | `gossip_ping_round_requires_two_failures_before_persisting_unreachable` (lib.rs) | test-only fixture seed for the gossip hysteresis round; the row must pre-exist so the assertion is about when `mark_peer_ping` fires |
 ///
 /// And the `upsert_peer` CALL-SITE authority table, which the ledger above
 /// structurally cannot hold, because the bootstrap site issues no SQL of its own
@@ -8366,11 +8366,11 @@ mod peers_table_writer_guard {
     const LEDGER: &[(&str, usize)] = &[
         ("a_legacy_row_can_still_refresh_its_liveness", 1),
         (
-            "manual_ping_uses_readiness_without_mutating_federation_gate",
+            "gossip_ping_round_requires_two_failures_before_persisting_unreachable",
             1,
         ),
         (
-            "gossip_ping_round_requires_two_failures_before_persisting_unreachable",
+            "manual_ping_uses_readiness_without_mutating_federation_gate",
             1,
         ),
         ("mark_peer_ping", 1),

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/gitlawb-node/src/git/repo_store.rs

@@ -71,7 +71,7 @@ impl RepoStore {
     /// Derives its own lock pool from `pool`, so callers that only have the main
     /// pool (tests, `for_testing` sites in other modules) still get the
     /// `after_release` semantics `acquire_write` depends on.
-    #[cfg(any(test, feature = "test-harness"))]
+    #[cfg(test)]
     pub fn for_testing(repos_dir: PathBuf, pool: PgPool) -> Self {
         Self::new(
             repos_dir,

crates/gitlawb-node/src/test_harness.rs

@@ -1,586 +0,0 @@
-//! Test-only spawn surface for the real-node deny harness (see
-//! `tests/deny_harness.rs`). Feature-gated behind `test-harness` so it never
-//! compiles into the production binary. It exposes a single boot constructor so
-//! the out-of-crate integration test can bring up a real node over a bound TCP
-//! socket without the integration crate needing access to `graphql`,
-//! `rate_limit`, `repo_store`, or the other internals `AppState` is built from.
-//!
-//! The node is built the same way `test_support::build_state` builds it (p2p
-//! disabled, real migrated pool), but bound on `127.0.0.1:0` and served through
-//! the real `axum::serve` stack with connect-info, so the middleware order,
-//! body limits, and per-IP rate limiters all run exactly as in production.
-
-use std::net::SocketAddr;
-use std::path::PathBuf;
-use std::sync::atomic::{AtomicU64, Ordering};
-use std::sync::Arc;
-use std::time::Duration;
-
-use chrono::Utc;
-use clap::Parser;
-use sqlx::PgPool;
-use tokio::net::TcpListener;
-use tokio::sync::watch;
-use uuid::Uuid;
-
-use gitlawb_core::identity::Keypair;
-
-use crate::config::Config;
-use crate::db::{Db, PullRequest, RepoRecord, VisibilityMode};
-use crate::rate_limit::{RateLimiter, TrustedProxy};
-use crate::state::AppState;
-
-/// A running node bound to an ephemeral port. End every test with
-/// [`TestNode::shutdown`]: it joins the serve task and closes the pool, so
-/// `#[sqlx::test]`'s `DROP DATABASE` cleanup (which runs the moment the test
-/// future returns) never races the server's still-open connections. Dropping
-/// without it signals shutdown, aborts any remaining serve task, and removes
-/// the temp repository directory, a fallback for tests that return or panic
-/// before reaching teardown.
-pub struct TestNode {
-    /// Base URL of the running node, e.g. `http://127.0.0.1:54321`.
-    pub base_url: String,
-    /// The node's own DID (for building requests that reference it).
-    pub node_did: String,
-    shutdown_tx: watch::Sender<bool>,
-    repos_dir: PathBuf,
-    /// Seeding handle over the same pool the node serves from. Kept private so
-    /// the integration crate seeds through the methods below rather than
-    /// naming `Db`/`RepoRecord` directly.
-    db: Arc<Db>,
-    /// The detached `axum::serve` task, joined by [`Self::shutdown`]. `None`
-    /// once taken, so the `Drop` fallback never overlaps a completed teardown.
-    /// Carries `serve`'s own `io::Result<()>` so `shutdown()` can observe it
-    /// rather than discard it (see the coverage note there).
-    server: Option<tokio::task::JoinHandle<std::io::Result<()>>>,
-    /// A clone of the served pool, kept so [`Self::shutdown`] can close it
-    /// (sqlx pool clones share one inner, so closing this closes the clones
-    /// inside `AppState`/`Db` too).
-    pool: PgPool,
-    /// Test-only: when set, `Drop` skips the graceful watch signal so ONLY
-    /// `handle.abort()` tears the serve task down. Defaults false, so every
-    /// other test keeps both redundant teardown mechanisms. Set via
-    /// [`Self::force_abort_only_teardown`] to fence that the abort alone
-    /// releases the database when the graceful chain is broken.
-    suppress_graceful_shutdown_on_drop: bool,
-}
-
-impl Drop for TestNode {
-    fn drop(&mut self) {
-        // Fallback for tests that return or panic before reaching
-        // [`TestNode::shutdown`]: flip the shared shutdown signal so the serve
-        // task exits gracefully, abort whatever is left of it, and remove the
-        // temp repos dir. Drop cannot await, so it cannot join the server or
-        // close the pool; a test that already failed should not panic again in
-        // teardown. All three actions are idempotent or no-ops after
-        // `shutdown()` (which takes the handle), so running afterwards is
-        // harmless.
-        //
-        // The abort matters when the graceful chain does not run to completion
-        // before sqlx's `DROP DATABASE` cleanup: `JoinHandle::abort()` is a
-        // synchronous cancellation request callable from non-async Drop, and
-        // the aborted task's future (with the `PgPool` clones inside its
-        // router/state) is dropped on a subsequent scheduler tick, which sqlx
-        // cleanup's own awaits provide before the drop statement runs. The
-        // race is closed by those scheduling points, not by the abort alone.
-        //
-        // `suppress_graceful_shutdown_on_drop` (test-only) forces the abort to
-        // carry teardown by itself, so a test can fence that the abort line is
-        // load-bearing; it is false everywhere else.
-        if !self.suppress_graceful_shutdown_on_drop {
-            let _ = self.shutdown_tx.send(true);
-        }
-        if let Some(handle) = self.server.take() {
-            handle.abort();
-        }
-        let _ = std::fs::remove_dir_all(&self.repos_dir);
-    }
-}
-
-/// Outcome of joining the serve task during teardown, split so the wedged-task
-/// path is observable to a test (which injects a short timeout) without waiting
-/// the production teardown window.
-enum ServeTeardown {
-    /// The task returned (carrying `serve`'s `io::Result<()>`) — either within
-    /// the timeout, or in the race between the timeout firing and the abort
-    /// landing. A task that actually finished is never a wedge.
-    Graceful(std::io::Result<()>),
-    /// The task did not finish within the timeout and was aborted (cancelled)
-    /// and reaped.
-    TimedOutAborted,
-}
-
-/// Join `handle` within `timeout`; on expiry `abort()` it and await the aborted
-/// handle so the serve task (and the `PgPool` clones inside its router/state) is
-/// reaped BEFORE the caller returns and `#[sqlx::test]` runs `DROP DATABASE`.
-/// Takes `&mut` (a `JoinHandle` is `Unpin`) so ownership is retained across the
-/// timeout and the handle can be aborted on expiry — the previous code moved the
-/// handle into `timeout` and, on elapse, dropped it, which DETACHES the still
-/// running task rather than aborting it, leaking the test database (#194 F2).
-async fn join_or_abort(
-    handle: &mut tokio::task::JoinHandle<std::io::Result<()>>,
-    timeout: Duration,
-) -> ServeTeardown {
-    match tokio::time::timeout(timeout, &mut *handle).await {
-        Ok(join_result) => ServeTeardown::Graceful(join_result.expect("serve task must not panic")),
-        Err(_elapsed) => {
-            handle.abort();
-            match handle.await {
-                // The task finished in the race between the timeout firing and
-                // the abort landing — honor its result, not a false wedge.
-                Ok(result) => ServeTeardown::Graceful(result),
-                Err(e) if e.is_cancelled() => ServeTeardown::TimedOutAborted,
-                // A genuine panic in the serve task surfaces here, mirroring the
-                // in-time arm's `.expect("serve task must not panic")`.
-                Err(e) => std::panic::resume_unwind(e.into_panic()),
-            }
-        }
-    }
-}
-
-/// Allocate a process-unique temp directory for a spawned node's repositories
-/// without pulling in the `tempfile` dev-dependency (which is unavailable to
-/// the library crate under `--features test-harness`).
-fn unique_repos_dir() -> PathBuf {
-    static COUNTER: AtomicU64 = AtomicU64::new(0);
-    let n = COUNTER.fetch_add(1, Ordering::Relaxed);
-    std::env::temp_dir().join(format!("gitlawb-deny-harness-{}-{}", std::process::id(), n))
-}
-
-/// Build an [`AppState`] over the given migrated pool, mirroring
-/// `test_support::build_state` but with a real on-disk repository directory so
-/// git smart-HTTP routes can serve. P2P is always disabled.
-fn build_state(db: Arc<crate::db::Db>, pool: PgPool, repos_dir: PathBuf) -> AppState {
-    let keypair = Keypair::generate();
-    let scan_token_key = crate::state::AppState::derive_scan_token_key(&keypair);
-    let node_did = keypair.did();
-    let (ref_tx, _) = tokio::sync::broadcast::channel(1);
-    let (task_tx, _) = tokio::sync::broadcast::channel(1);
-    let schema = Arc::new(crate::graphql::build_schema(
-        db.clone(),
-        ref_tx.clone(),
-        task_tx.clone(),
-    ));
-    AppState {
-        config: Arc::new(Config::parse_from(["gitlawb-node"])),
-        db,
-        node_did,
-        node_keypair: Arc::new(keypair),
-        p2p: None,
-        http_client: Arc::new(reqwest::Client::new()),
-        ref_update_tx: ref_tx,
-        task_event_tx: task_tx,
-        graphql_schema: schema,
-        machine_id: None,
-        repo_store: crate::git::repo_store::RepoStore::for_testing(repos_dir, pool),
-        rate_limiter: RateLimiter::new(100, Duration::from_secs(60)),
-        create_ip_rate_limiter: RateLimiter::new(1000, Duration::from_secs(3600)),
-        push_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
-        ipfs_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
-        ipfs_work_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
-        ipfs_max_history_walks: crate::api::ipfs::MAX_HISTORY_WALKS_PER_REQUEST,
-        ipfs_max_legacy_probes: crate::api::ipfs::MAX_LEGACY_PROBES_PER_REQUEST,
-        ipfs_legacy_scan_page_rows: crate::api::ipfs::LEGACY_SCAN_PAGE_ROWS,
-        ipfs_max_legacy_scan_rows: crate::api::ipfs::MAX_LEGACY_SCAN_ROWS_PER_REQUEST,
-        ipfs_max_legacy_scan_rule_bytes: crate::api::ipfs::MAX_LEGACY_SCAN_RULE_BYTES_PER_REQUEST,
-        ipfs_scan_token_key: Arc::new(scan_token_key),
-        ipfs_max_served_object_bytes: crate::api::ipfs::MAX_SERVED_OBJECT_BYTES,
-        push_limiter_trust: TrustedProxy::None,
-        sync_trigger_rate_limiter: RateLimiter::new(60, Duration::from_secs(3600)),
-        peer_write_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
-        shutdown_tx: watch::channel(false).0,
-        git_read_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
-        git_write_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
-        git_push_advert_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
-        git_encrypt_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
-        pin_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
-        encrypt_inflight: crate::state::EncryptInflight::new(),
-        repo_write_leases: crate::state::RepoWriteLeases::new(8),
-        git_read_per_caller: crate::rate_limit::PerCallerConcurrency::with_default_max_keys(16),
-        git_push_advert_per_caller: crate::rate_limit::PerCallerConcurrency::with_default_max_keys(
-            8,
-        ),
-        git_write_per_caller: crate::rate_limit::PerCallerConcurrency::with_default_max_keys(8),
-        git_ipfs_walk_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
-        git_ipfs_walk_per_caller: crate::rate_limit::PerCallerConcurrency::with_default_max_keys(
-            16,
-        ),
-        git_bin: "git".to_string(),
-    }
-}
-
-/// Spawn a real node bound to `127.0.0.1:0` over the given (already-created,
-/// empty) test pool. Runs the schema migrations, builds the router, and serves
-/// it on a background task through the production `axum::serve` stack with
-/// connect-info so per-IP layers key on the real peer. Returns once the socket
-/// is bound and accepting connections.
-pub async fn spawn_node(pool: PgPool) -> TestNode {
-    let db = Arc::new(crate::db::Db::for_testing(pool.clone()));
-    db.run_migrations()
-        .await
-        .expect("test schema migrations should apply");
-
-    let repos_dir = unique_repos_dir();
-    std::fs::create_dir_all(&repos_dir).expect("create temp repos dir");
-
-    let state = build_state(db.clone(), pool.clone(), repos_dir.clone());
-    let node_did = state.node_did.to_string();
-    let shutdown_tx = state.shutdown_tx.clone();
-    let mut shutdown_rx = shutdown_tx.subscribe();
-
-    let router = crate::server::build_router(state);
-
-    let listener = TcpListener::bind(("127.0.0.1", 0))
-        .await
-        .expect("bind ephemeral port");
-    let addr = listener.local_addr().expect("read bound addr");
-
-    let server = tokio::spawn(async move {
-        axum::serve(
-            listener,
-            router.into_make_service_with_connect_info::<SocketAddr>(),
-        )
-        .with_graceful_shutdown(async move {
-            let _ = shutdown_rx.changed().await;
-        })
-        .await
-    });
-
-    TestNode {
-        base_url: format!("http://{addr}"),
-        node_did,
-        shutdown_tx,
-        repos_dir,
-        db,
-        server: Some(server),
-        pool,
-        suppress_graceful_shutdown_on_drop: false,
-    }
-}
-
-impl TestNode {
-    /// Test-only: make `Drop` tear the serve task down through `handle.abort()`
-    /// alone by suppressing the graceful watch signal. Used to prove the abort
-    /// is load-bearing (that the database is released even when the graceful
-    /// chain is broken); no production or normal-teardown path calls this.
-    pub fn force_abort_only_teardown(&mut self) {
-        self.suppress_graceful_shutdown_on_drop = true;
-    }
-
-    /// Graceful async teardown: signal shutdown, join the serve task, close
-    /// the pool, remove the temp repos dir. Call this at the end of every
-    /// test. `#[sqlx::test]` issues `DROP DATABASE` as soon as the test
-    /// future returns, so the server (which owns pool clones inside
-    /// `AppState`/`Db`) must be gone and the pool closed before then, or the
-    /// cleanup fails and leaks per-test databases. A serve task that does not
-    /// exit within 10s panics loudly: a wedged graceful shutdown must fail
-    /// the test, not hang CI.
-    ///
-    /// When `self` drops on return, `Drop` re-sends the signal and re-removes
-    /// the dir; both are idempotent, and the serve handle was already taken,
-    /// so the overlap is harmless.
-    pub async fn shutdown(self) {
-        self.shutdown_with_timeout(Duration::from_secs(10)).await;
-    }
-
-    /// Body of [`Self::shutdown`] with an injectable teardown timeout so a test
-    /// can drive the wedged-shutdown path without waiting the production 10s.
-    /// Production always calls this with 10s.
-    async fn shutdown_with_timeout(mut self, timeout: Duration) {
-        let _ = self.shutdown_tx.send(true);
-        let outcome = match self.server.take() {
-            Some(mut handle) => Some(join_or_abort(&mut handle, timeout).await),
-            None => None,
-        };
-        // Release the pool and temp dir on EVERY path before returning OR
-        // panicking. A wedged graceful shutdown is exactly when in-flight
-        // per-connection tasks still hold `PgPool` clones; aborting the serve
-        // future alone does not reliably drop them, so the explicit
-        // `pool.close()` (which closes the shared inner) is what frees them
-        // before `#[sqlx::test]` runs `DROP DATABASE`. Panicking before this
-        // (the previous behavior) skipped the close on the one path that needs
-        // it most (#194 F2).
-        self.pool.close().await;
-        let _ = std::fs::remove_dir_all(&self.repos_dir);
-        match outcome {
-            // Honest coverage contract: on axum 0.8.8,
-            // `WithGracefulShutdown::into_future` always resolves `Ok(())` (the
-            // shutdown signal is treated as success and `accept()` errors are
-            // retried forever rather than returned), so this `Err` arm is
-            // unreachable today - every existing harness test exercises only the
-            // `Ok` path here. Forward-compat insurance for a future axum where
-            // `serve` can fail: that should surface as a clear panic, not be
-            // silently discarded.
-            Some(ServeTeardown::Graceful(result)) => {
-                result.expect("axum::serve exited with an error");
-            }
-            Some(ServeTeardown::TimedOutAborted) => {
-                panic!(
-                    "serve task did not exit within {timeout:?} of the shutdown \
-                     signal (wedged graceful shutdown)"
-                );
-            }
-            None => {}
-        }
-    }
-
-    /// Insert a repo owned by `owner_did` and return its repo id. Mirrors
-    /// `test_support::seed_repo` (the `disk_path` field is unused by the
-    /// integration path — `RepoStore` computes the on-disk path from
-    /// `repos_dir`/owner/name, see [`Self::seed_bare_repo`]).
-    pub async fn seed_repo(&self, owner_did: &str, name: &str, is_public: bool) -> String {
-        let now = Utc::now();
-        let record = RepoRecord {
-            id: Uuid::new_v4().to_string(),
-            name: name.to_string(),
-            owner_did: owner_did.to_string(),
-            description: None,
-            is_public,
-            default_branch: "main".to_string(),
-            created_at: now,
-            updated_at: now,
-            disk_path: format!("/tmp/{name}"),
-            forked_from: None,
-            machine_id: None,
-        };
-        self.db.create_repo(&record).await.expect("seed repo");
-        record.id
-    }
-
-    /// Mark `branch` as protected on `repo_id`, so a non-owner push to it is
-    /// rejected by the branch-protection gate (used by the deny-prober's
-    /// protected-push probe — #195, F3).
-    pub async fn seed_protected_branch(&self, repo_id: &str, branch: &str, created_by: &str) {
-        self.db
-            .protect_branch(repo_id, branch, created_by)
-            .await
-            .expect("seed protected branch");
-    }
-
-    /// Seed an open PR `number` in `repo_id`, authored by `author_did`. The
-    /// author-or-owner close gate (`close_pr`) loads the PR *before* it runs, so a
-    /// deny-prober row for `.../pulls/{number}/close` needs the PR to exist or a
-    /// stranger gets a 404 (absent entity) instead of the 403 the gate emits.
-    pub async fn seed_pr(&self, repo_id: &str, number: i64, author_did: &str) {
-        let now = Utc::now().to_rfc3339();
-        let pr = PullRequest {
-            id: Uuid::new_v4().to_string(),
-            repo_id: repo_id.to_string(),
-            number,
-            title: "seed pr".to_string(),
-            body: None,
-            author_did: author_did.to_string(),
-            source_branch: "feature".to_string(),
-            target_branch: "main".to_string(),
-            status: "open".to_string(),
-            merged_by_did: None,
-            merged_at: None,
-            created_at: now.clone(),
-            updated_at: now,
-        };
-        self.db.create_pr(&pr).await.expect("seed pr");
-    }
-
-    /// Seed issue `issue_id` (a git-ref JSON blob authored by `author_did`) in the
-    /// repo's bare on-disk store. Like [`Self::seed_pr`], the `close_issue`
-    /// owner-or-author gate loads the issue before it runs, so the deny-prober row
-    /// for `.../issues/{id}/close` needs the issue present to reach the 403.
-    pub fn seed_issue(&self, owner_did: &str, name: &str, issue_id: &str, author_did: &str) {
-        let slug = owner_did.replace([':', '/'], "_");
-        let bare = self.repos_dir.join(&slug).join(format!("{name}.git"));
-        let json = format!(r#"{{"author":"{author_did}"}}"#);
-        crate::git::issues::create_issue(&bare, issue_id, &json).expect("seed issue");
-    }
-
-    /// Add a path-scoped visibility rule restricting `path_glob` to
-    /// `reader_dids` (empty = only the owner). Mode B keeps object SHAs intact
-    /// and withholds blob content, which is what the read/replication gates
-    /// enforce.
-    pub async fn withhold_path(
-        &self,
-        repo_id: &str,
-        path_glob: &str,
-        reader_dids: &[String],
-        created_by: &str,
-    ) {
-        self.db
-            .set_visibility_rule(
-                repo_id,
-                path_glob,
-                VisibilityMode::B,
-                reader_dids,
-                created_by,
-            )
-            .await
-            .expect("set visibility rule");
-    }
-
-    /// Record `pinned_cids` rows for every blob OID returned by
-    /// [`Self::seed_bare_repo`] so `/ipfs/{cid}` can resolve via provenance on
-    /// current main (the table is the primary lookup path). CIDs are computed
-    /// from raw object bytes exactly as the production pin path does
-    /// (`Cid::from_git_object_bytes`), not from the git oid hash.
-    /// Returns each seeded path's canonical CID for use in `GET /ipfs/{cid}`.
-    pub async fn seed_pinned_cids_for_repo(
-        &self,
-        owner_did: &str,
-        repo_name: &str,
-        repo_id: &str,
-        oids: &std::collections::HashMap<String, String>,
-    ) -> std::collections::HashMap<String, String> {
-        let slug = owner_did.replace([':', '/'], "_");
-        let bare = self.repos_dir.join(&slug).join(format!("{repo_name}.git"));
-        let mut cids = std::collections::HashMap::new();
-        for (path, oid_hex) in oids {
-            if path == "HEAD" {
-                continue;
-            }
-            let (_ty, raw) = crate::git::store::read_object(&bare, oid_hex)
-                .expect("read seeded object")
-                .expect("seeded object exists in bare repo");
-            let preliminary = gitlawb_core::cid::Cid::from_git_object_bytes(&raw).to_string();
-            let cid = preliminary
-                .parse::<cid::CidGeneric<64>>()
-                .expect("parse production pin cid")
-                .to_string();
-            self.db
-                .record_pinned_cid_with_source(oid_hex, &cid, repo_id)
-                .await
-                .expect("seed pinned_cids row");
-            let roundtrip = self
-                .db
-                .oids_for_cid(&cid)
-                .await
-                .expect("oids_for_cid after seed");
-            assert!(
-                !roundtrip.is_empty(),
-                "pinned_cids seed for {path} did not round-trip cid {cid}"
-            );
-            cids.insert(path.clone(), cid);
-        }
-        cids
-    }
-
-    /// Create a real bare git repository on disk at the exact path
-    /// `RepoStore::acquire` reads (`<repos_dir>/<owner-slug>/<name>.git`), with
-    /// one commit on `main` containing `files` (`(path, contents)`). Returns the
-    /// blob OID of each path so callers can assert a withheld OID never appears
-    /// in served bytes (U8). Shells out to `git`, mirroring
-    /// `test_support`'s served-content seam.
-    /// `object_format` is `"sha1"` for the git smart-HTTP surfaces and
-    /// `"sha256"` for the content-addressed `/ipfs/{cid}` surface (whose CIDs
-    /// are the sha2-256 object ids).
-    pub fn seed_bare_repo(
-        &self,
-        owner_did: &str,
-        name: &str,
-        files: &[(&str, &str)],
-        object_format: &str,
-    ) -> std::collections::HashMap<String, String> {
-        let run = |args: &[&str], cwd: &std::path::Path| {
-            // Test-harness-only fixture seeding, feature-gated out of the production
-            // binary; runs local git against a tempdir inside a test's own lifetime.
-            // allow-unbounded-git: test-only seeding, never on a request path holding a permit
-            let out = std::process::Command::new("git")
-                .args(args)
-                .current_dir(cwd)
-                .output() // allow-unbounded-git: test-harness fixture seeding only
-                .expect("git runs");
-            assert!(
-                out.status.success(),
-                "git {args:?} failed: {}",
-                String::from_utf8_lossy(&out.stderr)
-            );
-            String::from_utf8_lossy(&out.stdout).trim().to_string()
-        };
-
-        // Build a source work tree, then bare-clone it into the served path.
-        let src = self.repos_dir.join(format!("src-{name}"));
-        std::fs::create_dir_all(&src).expect("create src dir");
-        let fmt_arg = format!("--object-format={object_format}");
-        run(&["init", "-q", "-b", "main", &fmt_arg], &src);
-        run(&["config", "user.email", "t@t"], &src);
-        run(&["config", "user.name", "t"], &src);
-        for (path, contents) in files {
-            let full = src.join(path);
-            if let Some(parent) = full.parent() {
-                std::fs::create_dir_all(parent).expect("create file parent dir");
-            }
-            std::fs::write(&full, contents).expect("write seed file");
-            run(&["add", path], &src);
-        }
-        run(&["commit", "-q", "-m", "seed"], &src);
-
-        let mut oids = std::collections::HashMap::new();
-        for (path, _) in files {
-            let oid = run(&["rev-parse", &format!("HEAD:{path}")], &src);
-            oids.insert((*path).to_string(), oid);
-        }
-        // The HEAD commit oid, used as the `want` when driving upload-pack.
-        oids.insert("HEAD".to_string(), run(&["rev-parse", "HEAD"], &src));
-
-        let slug = owner_did.replace([':', '/'], "_");
-        let bare = self.repos_dir.join(&slug).join(format!("{name}.git"));
-        std::fs::create_dir_all(bare.parent().unwrap()).expect("create bare parent");
-        run(
-            &[
-                "clone",
-                "--bare",
-                "-q",
-                src.to_str().unwrap(),
-                bare.to_str().unwrap(),
-            ],
-            &self.repos_dir,
-        );
-
-        oids
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::{join_or_abort, ServeTeardown};
-    use std::time::Duration;
-
-    /// A wedged serve task (never returns within the window) must be ABORTED and
-    /// reaped on timeout, not detached. The previous teardown moved the
-    /// `JoinHandle` into `timeout` and dropped it on elapse, which detaches the
-    /// still-running task and leaks the test DB across `DROP DATABASE` (#194 F2).
-    /// Load-bearing: neutralize the abort in `join_or_abort` (return without
-    /// abort+await) and this flips RED — `handle.is_finished()` stays false
-    /// because the task was left running.
-    #[tokio::test]
-    async fn join_or_abort_aborts_a_wedged_task_on_timeout() {
-        let mut handle: tokio::task::JoinHandle<std::io::Result<()>> = tokio::spawn(async {
-            loop {
-                tokio::time::sleep(Duration::from_secs(3600)).await;
-            }
-        });
-        assert!(
-            matches!(
-                join_or_abort(&mut handle, Duration::from_millis(50)).await,
-                ServeTeardown::TimedOutAborted
-            ),
-            "a task that never returns must be reported timed-out, not graceful"
-        );
-        assert!(
-            handle.is_finished(),
-            "the wedged serve task must be aborted and reaped, not detached"
-        );
-    }
-
-    /// A task that returns within the window is reported graceful, carrying its
-    /// `io::Result` — the normal teardown path.
-    #[tokio::test]
-    async fn join_or_abort_reports_graceful_when_task_exits_in_time() {
-        let mut handle: tokio::task::JoinHandle<std::io::Result<()>> =
-            tokio::spawn(async { Ok(()) });
-        assert!(
-            matches!(
-                join_or_abort(&mut handle, Duration::from_secs(5)).await,
-                ServeTeardown::Graceful(Ok(()))
-            ),
-            "a prompt clean exit must be reported graceful"
-        );
-    }
-}

crates/gitlawb-node/tests/support/assert.rs

@@ -1,237 +0,0 @@
-//! INV-8 deny assertion. A denial must be an actual refusal (4xx) whose body
-//! and headers carry none of the data it is withholding, and must never be a 2xx
-//! (an empty-200 rendered as success is the denial-as-success bug this guards).
-//!
-//! The check is split into a pure `check_denied` (unit-tested below with the
-//! self-check scenarios) and an async `assert_denied` wrapper that reads a real
-//! `reqwest::Response`.
-
-/// Pure deny check. Returns `Err(reason)` when the response is not a clean
-/// denial. `withheld` are exact tokens (OIDs, paths, private slugs) that must
-/// not appear anywhere in the body; register short-OID prefixes and encoded
-/// path forms as separate tokens at the call site (a raw substring scan cannot
-/// normalize them).
-pub fn check_denied(
-    status: u16,
-    body: &str,
-    expected: u16,
-    withheld: &[&str],
-) -> Result<(), String> {
-    // The harness only ever expects denials; a non-4xx expectation is a
-    // programming error in the test, not a node behavior under test.
-    if !(400..500).contains(&expected) {
-        return Err(format!(
-            "test bug: assert_denied expected status {expected} is not a 4xx"
-        ));
-    }
-    // INV-8: a denial rendered as success is the headline bug. Catch it
-    // explicitly so the failure message names it, rather than only failing the
-    // status-equality check below.
-    if (200..300).contains(&status) {
-        return Err(format!(
-            "denial rendered as success: got {status}, expected denial {expected}. \
-             body={body:?}"
-        ));
-    }
-    if status != expected {
-        return Err(format!(
-            "expected denial status {expected}, got {status}. body={body:?}"
-        ));
-    }
-    for token in withheld {
-        if token.is_empty() {
-            continue;
-        }
-        if body.contains(token) {
-            return Err(format!(
-                "withheld token {token:?} leaked in denial body: {body:?}"
-            ));
-        }
-    }
-    Ok(())
-}
-
-/// Like [`check_denied`] but also scans serialized response headers for withheld
-/// tokens. Call sites that only have a body should keep using `check_denied`.
-pub fn check_denied_with_headers(
-    status: u16,
-    body: &str,
-    header_text: &str,
-    expected: u16,
-    withheld: &[&str],
-) -> Result<(), String> {
-    if let Err(e) = check_denied(status, body, expected, withheld) {
-        return Err(e);
-    }
-    for token in withheld {
-        if token.is_empty() {
-            continue;
-        }
-        if header_text.contains(token) {
-            return Err(format!(
-                "withheld token {token:?} leaked in denial headers: {header_text:?}"
-            ));
-        }
-    }
-    Ok(())
-}
-
-/// Replace each withheld token in `s` with `[REDACTED]` so a test-failure
-/// witness does not itself leak the secret it caught.
-fn redact_withheld(s: &str, withheld: &[&str]) -> String {
-    let mut result = s.to_string();
-    for token in withheld {
-        if !token.is_empty() {
-            result = result.replace(token, "[REDACTED]");
-        }
-    }
-    result
-}
-
-/// Drive a real response through the deny check and panic on failure. Reads the
-/// full body once.
-pub async fn assert_denied(resp: reqwest::Response, expected: u16, withheld: &[&str]) {
-    let status = resp.status().as_u16();
-    let header_text = resp
-        .headers()
-        .iter()
-        .filter_map(|(name, value)| {
-            value
-                .to_str()
-                .ok()
-                .map(|v| format!("{}: {}", name.as_str(), v))
-        })
-        .collect::<Vec<_>>()
-        .join("\n");
-    // A body-read failure must NOT fold to "" — an empty body passes the withheld
-    // scan vacuously, so an unreadable denial (a mid-body reset/truncation) would be
-    // certified leak-free without ever inspecting the bytes. Fail loud instead. A
-    // legitimately empty body is `Ok("")`, not `Err`, so the honest empty-401 denials
-    // still pass.
-    let body = resp
-        .text()
-        .await
-        .expect("read denial body: an unreadable denial cannot be certified leak-free");
-    if let Err(reason) = check_denied_with_headers(status, &body, &header_text, expected, withheld)
-    {
-        // Panic with a fixed string: CodeQL's taint analysis traces withheld
-        // tokens from check_denied through redact_withheld into the panic sink,
-        // even though redact_withheld strips them at runtime. A fixed message
-        // keeps no tainted data on the sink path. The test name identifies
-        // which probe failed; check_denied's unit tests cover the error detail.
-        let _ = redact_withheld(&reason, withheld);
-        panic!("deny assertion failed: status/body/header leak check");
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::{check_denied, check_denied_with_headers, redact_withheld};
-
-    #[test]
-    fn clean_403_with_no_leak_passes() {
-        let r = check_denied(
-            403,
-            r#"{"error":"only the repo owner can do this"}"#,
-            403,
-            &["a1b2c3"],
-        );
-        assert!(r.is_ok(), "{r:?}");
-    }
-
-    #[test]
-    fn empty_200_is_flagged_as_denial_rendered_as_success() {
-        let r = check_denied(200, "", 403, &[]);
-        assert!(r.is_err());
-        assert!(r.unwrap_err().contains("rendered as success"));
-    }
-
-    #[test]
-    fn leaking_403_fails_and_names_the_token() {
-        let withheld = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
-        let body = format!("not found near object {withheld}");
-        let r = check_denied(403, &body, 403, &[withheld]);
-        assert!(r.is_err());
-        assert!(r.unwrap_err().contains(withheld));
-    }
-
-    #[test]
-    fn wrong_status_fails() {
-        // A 404 when the owner gate's 403 was expected must not pass: it means
-        // an earlier layer denied, so the gate under test was never exercised.
-        let r = check_denied(404, "not found", 403, &[]);
-        assert!(r.is_err());
-        assert!(r.unwrap_err().contains("expected denial status 403"));
-    }
-
-    #[test]
-    fn non_4xx_expected_is_a_test_bug() {
-        // Guards the harness itself: expecting a non-denial status is a test
-        // authoring error, not a node behavior, and must be rejected.
-        let r = check_denied(403, "denied", 200, &[]);
-        assert!(r.is_err());
-        assert!(r.unwrap_err().contains("not a 4xx"));
-    }
-
-    #[test]
-    fn empty_withheld_token_never_false_positives() {
-        // An empty token must be skipped, never treated as "contained in every
-        // body" — otherwise every denial would spuriously fail as a leak.
-        let r = check_denied(403, "any body", 403, &[""]);
-        assert!(r.is_ok(), "{r:?}");
-    }
-
-    #[test]
-    fn leaking_header_fails_and_names_the_token() {
-        let withheld = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
-        let headers = format!("X-Debug: object {withheld}");
-        let r = check_denied_with_headers(
-            404,
-            r#"{"error":"repository not found"}"#,
-            &headers,
-            404,
-            &[withheld],
-        );
-        assert!(r.is_err());
-        let err = r.unwrap_err();
-        assert!(err.contains("leaked in denial headers"));
-        assert!(err.contains(withheld));
-    }
-
-    #[test]
-    fn clean_headers_with_clean_body_passes() {
-        let withheld = "a1b2c3";
-        let r = check_denied_with_headers(
-            404,
-            r#"{"error":"repository not found"}"#,
-            "Content-Type: application/json",
-            404,
-            &[withheld],
-        );
-        assert!(r.is_ok(), "{r:?}");
-    }
-
-    #[test]
-    fn redact_withheld_replaces_secrets() {
-        let withheld = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
-        let msg = format!("withheld token {withheld:?} leaked in body: near {withheld}");
-        let redacted = redact_withheld(&msg, &[withheld]);
-        assert!(
-            !redacted.contains(withheld),
-            "token must be redacted: {redacted}"
-        );
-        assert!(
-            redacted.contains("[REDACTED]"),
-            "redaction marker must appear: {redacted}"
-        );
-    }
-
-    #[test]
-    fn redact_withheld_leaves_clean_strings() {
-        let result = redact_withheld(
-            "no secrets here",
-            &["deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"],
-        );
-        assert_eq!(result, "no secrets here");
-    }
-}

crates/gitlawb-node/tests/support/mod.rs

@@ -1,33 +0,0 @@
-//! Shared support for the real-node deny harness: the deny assertion (U4), the
-//! RFC-9421 signing client (U2), and (via `gitlawb_node::test_harness`) the
-//! node spawner (U3).
-
-pub mod assert;
-// U2 fixture state matrix + per-gate-class probe generators; driven by U3.
-#[allow(dead_code)]
-pub mod probe;
-// U1 deny-bearing route registry; fields consumed by U2/U3 as they land.
-#[allow(dead_code)]
-pub mod routes;
-pub mod signing;
-
-/// A reqwest client with a bounded request timeout, so a wedged git subprocess or
-/// route fails the suite rather than hanging it until CI kills the job (#195, F1).
-/// The two real-node probes that drive git subprocesses use this; the inline
-/// twins at the upload-pack and registry-sweep sites use the same pattern.
-#[allow(dead_code)]
-pub fn bounded_client() -> reqwest::Client {
-    reqwest::Client::builder()
-        .timeout(std::time::Duration::from_secs(30))
-        .build()
-        .unwrap()
-}
-
-/// Build an anonymous (unsigned) GET request for `path` under `base_url`.
-/// Extracted from inline call sites so URL construction goes through a
-/// function boundary, matching the `signed_request` pattern in `support::signing`.
-#[allow(dead_code)]
-pub fn anon_get(client: &reqwest::Client, base_url: &str, path: &str) -> reqwest::RequestBuilder {
-    let url = format!("{base_url}{path}");
-    client.get(url)
-}

crates/gitlawb-node/tests/support/signing.rs

@@ -1,32 +0,0 @@
-//! Real RFC-9421 request signing for the deny harness (U2). Wraps
-//! `gitlawb_core::http_sig::sign_request` (the same entry point the
-//! git-remote-gitlawb helper uses in production) so a test can present a valid
-//! signature for any identity, reaching the authenticated-but-wrong-owner path
-//! that an injected-DID shortcut cannot.
-
-use gitlawb_core::http_sig::sign_request;
-use gitlawb_core::identity::Keypair;
-
-/// Build a reqwest request carrying a valid RFC-9421 signature for `keypair`.
-///
-/// `path` is both appended to `base_url` to form the request URL and signed as
-/// the `@path` component, so the value the server reconstructs and the value
-/// that was signed are guaranteed identical. `body` is signed via its
-/// content-digest and sent as the request body.
-pub fn signed_request(
-    client: &reqwest::Client,
-    method: reqwest::Method,
-    base_url: &str,
-    path: &str,
-    body: Vec<u8>,
-    keypair: &Keypair,
-) -> reqwest::RequestBuilder {
-    let sig = sign_request(keypair, method.as_str(), path, &body);
-    let url = format!("{base_url}{path}");
-    client
-        .request(method, url)
-        .header("content-digest", sig.content_digest)
-        .header("signature-input", sig.signature_input)
-        .header("signature", sig.signature)
-        .body(body)
-}