From 45064806741001bf120a2e414adcdb9a5250f3a8 Mon Sep 17 00:00:00 2001 From: Gravirei Date: Tue, 30 Jun 2026 19:52:16 +0600 Subject: [PATCH 01/12] fix(node): gate GET /ipfs/{cid} on reachable allowed-set, not deny-set (#126) The IPFS visibility gate used withheld_blob_oids (a deny-set enumerating only reachable blobs), so a dangling/unreachable blob was absent from the set and served in cleartext to anonymous callers. Flip to an allowed-set (allowed_blob_set_for_caller) that enumerates reachable blobs the caller may read: a dangling blob has no path, is never in the set, and 404s. --- crates/gitlawb-node/src/api/ipfs.rs | 115 ++++++++++-------- .../gitlawb-node/src/git/visibility_pack.rs | 96 ++++++++++++++- crates/gitlawb-node/src/test_support.rs | 112 +++++++++++++++++ 3 files changed, 273 insertions(+), 50 deletions(-) diff --git a/crates/gitlawb-node/src/api/ipfs.rs b/crates/gitlawb-node/src/api/ipfs.rs index 6a43cb58..405eaedf 100644 --- a/crates/gitlawb-node/src/api/ipfs.rs +++ b/crates/gitlawb-node/src/api/ipfs.rs @@ -27,7 +27,7 @@ use std::str::FromStr; use crate::auth::AuthenticatedDid; use crate::error::{AppError, Result}; use crate::git::store; -use crate::git::visibility_pack::{has_path_scoped_rule, withheld_blob_oids}; +use crate::git::visibility_pack::{allowed_blob_set_for_caller, has_path_scoped_rule}; use crate::state::AppState; use crate::visibility::{visibility_check, Decision}; @@ -36,16 +36,22 @@ use crate::visibility::{visibility_check, Decision}; /// Search all repos on the node for a git object whose SHA-256 hash matches /// the given CIDv1, returning its raw content if the caller may read it. /// -/// Visibility (#110): the object is served only from a repo row the caller -/// passes. For each iterated row we gate against that row's OWN rules +/// Visibility (#110, #126): the object is served only from a repo row the +/// caller passes. For each iterated row we gate against that row's OWN rules /// (`visibility_check` at `"/"`), never re-resolving via `authorize_repo_read` /// — `get_repo`'s fuzzy match could otherwise authorize a different physical -/// row than the one read (KTD2a). When the row carries path-scoped rules, a -/// blob withheld from the caller (`withheld_blob_oids`) is skipped. Denial and -/// genuine not-found both fall through to an opaque 404. +/// row than the one read (KTD2a). When the row carries path-scoped rules +/// (KTD4) the served object must be either a non-blob (trees/commits are +/// structural; KTD3) OR a blob in the caller's *reachable* allowed-set +/// (`allowed_blob_set_for_caller`). The reachable allowed-set excludes +/// dangling blobs — a blob written via `git hash-object -w` and never +/// committed has no path to gate, so it is fail-closed 404'd under +/// path-scoped rules (#126). Denial and genuine not-found both fall through +/// to an opaque 404. /// -/// Scope: this closes the direct unauthenticated scan. A stale-public mirror -/// row still serves withheld content (tracked separately, #124). +/// Scope: this closes the direct unauthenticated scan, including the dangling +/// case. A stale-public mirror row still serves withheld content (tracked +/// separately, #124). pub async fn get_by_cid( Path(cid_str): Path, State(state): State, @@ -85,11 +91,16 @@ pub async fn get_by_cid( .await .map_err(AppError::Internal)?; - // Request-scoped memo of the per-repo withheld set (KTD1). The caller is - // constant for one request, so `repo.id` alone is a safe, sufficient key — - // never a coarse caller "class", which `visibility_check`'s exact full-DID - // reader match would make unsafe. - let mut withheld_memo: HashMap> = HashMap::new(); + // Request-scoped memo of the per-repo allowed-blob set (KTD1, #126). The + // caller is constant for one request, so `repo.id` alone is a safe, + // sufficient key — never a coarse caller "class", which + // `visibility_check`'s exact full-DID reader match would make unsafe. + // + // We flipped from a deny-set (`withheld_blob_oids`) to an allowed-set + // (`allowed_blob_set_for_caller`) so dangling blobs — never enumerated by + // the reachable walk — fail closed instead of slipping through an empty + // deny entry (#126). + let mut allowed_memo: HashMap> = HashMap::new(); for repo in &repos { // Repo-level read gate against THIS row's own rules (KTD2a). @@ -106,45 +117,51 @@ pub async fn get_by_cid( Err(_) => continue, }; - // Per-blob withholding only applies when a path-scoped rule exists (KTD4). - if has_path_scoped_rule(rules) { - if !withheld_memo.contains_key(&repo.id) { - let rp = repo_path.clone(); - let r = rules.to_vec(); - let is_public = repo.is_public; - let owner = repo.owner_did.clone(); - let caller_for_walk = caller_owned.clone(); - // Full-history walk shells out to git — keep it off the async runtime. - let walk = tokio::task::spawn_blocking(move || { - withheld_blob_oids(&rp, &r, is_public, &owner, caller_for_walk.as_deref()) - }) - .await; - // Fail closed on EITHER a task panic (JoinError) or a walk error: - // we cannot prove the caller may read here, so skip this repo and - // let a public copy (if any) serve. Never serve on an unproven gate. - let set = match walk { - Ok(Ok(set)) => set, - Ok(Err(e)) => { - tracing::warn!(repo = %repo.name, err = %e, "withheld walk failed; skipping repo"); - continue; - } - Err(e) => { - tracing::warn!(repo = %repo.name, err = %e, "withheld walk task panicked; skipping repo"); - continue; - } - }; - withheld_memo.insert(repo.id.clone(), set); - } - if withheld_memo - .get(&repo.id) - .is_some_and(|set| set.contains(&sha256_hex)) - { - continue; - } + // Per-blob gating only applies when a path-scoped rule exists (KTD4). + // Without any path-scoped rule, the "/" gate above is the whole story. + let path_scoped = has_path_scoped_rule(rules); + if path_scoped && !allowed_memo.contains_key(&repo.id) { + let rp = repo_path.clone(); + let r = rules.to_vec(); + let is_public = repo.is_public; + let owner = repo.owner_did.clone(); + let caller_for_walk = caller_owned.clone(); + // Full-history walk shells out to git — keep it off the async runtime. + let walk = tokio::task::spawn_blocking(move || { + allowed_blob_set_for_caller(&rp, &r, is_public, &owner, caller_for_walk.as_deref()) + }) + .await; + // Fail closed on EITHER a task panic (JoinError) or a walk error: + // we cannot prove the caller may read here, so skip this repo and + // let a public copy (if any) serve. Never serve on an unproven gate. + let set = match walk { + Ok(Ok(set)) => set, + Ok(Err(e)) => { + tracing::warn!(repo = %repo.name, err = %e, "allowed-blob walk failed; skipping repo"); + continue; + } + Err(e) => { + tracing::warn!(repo = %repo.name, err = %e, "allowed-blob walk task panicked; skipping repo"); + continue; + } + }; + allowed_memo.insert(repo.id.clone(), set); } match store::read_object(&repo_path, &sha256_hex) { - Ok(Some((_obj_type, content))) => { + Ok(Some((obj_type, content))) => { + // Path-scoped rules: serve trees/commits unconditionally + // (structural; KTD3); a blob must be in the reachable + // allowed-set, which excludes dangling blobs (#126). + if path_scoped && obj_type == "blob" { + let in_allowed = allowed_memo + .get(&repo.id) + .is_some_and(|set| set.contains(&sha256_hex)); + if !in_allowed { + continue; + } + } + // 3. Return the content with IPFS-style headers let mut headers = HeaderMap::new(); headers.insert( diff --git a/crates/gitlawb-node/src/git/visibility_pack.rs b/crates/gitlawb-node/src/git/visibility_pack.rs index 578ee40b..cb70e39c 100644 --- a/crates/gitlawb-node/src/git/visibility_pack.rs +++ b/crates/gitlawb-node/src/git/visibility_pack.rs @@ -309,11 +309,33 @@ pub fn replicable_blob_set( rules: &[VisibilityRule], is_public: bool, owner_did: &str, +) -> Result> { + allowed_blob_set_for_caller(repo_path, rules, is_public, owner_did, None) +} + +/// Reachable blob OIDs that visibility ALLOWS `caller` at some path. The +/// caller-aware generalization of `replicable_blob_set` (which is the anonymous +/// `caller = None` case). Used by `GET /ipfs/{cid}` to gate fail-closed against +/// dangling/unreachable blobs (#126): a blob written via `git hash-object -w` +/// but unreferenced is absent from the reachable walk, so it is never in this +/// set and the IPFS serve path drops it — even from the owner, who has no path +/// to authorize the blob at. +/// +/// A blob reachable at an allowed path is included even when also denied +/// elsewhere (its content is readable to this caller elsewhere). Trees and +/// commits are NOT included here; the caller decides per object type whether +/// the allow-set applies (it does not for trees/commits — KTD3). +pub fn allowed_blob_set_for_caller( + repo_path: &Path, + rules: &[VisibilityRule], + is_public: bool, + owner_did: &str, + caller: Option<&str>, ) -> Result> { let pairs = blob_paths(repo_path)?; let mut allowed = HashSet::new(); for (oid, path) in &pairs { - if visibility_check(rules, is_public, owner_did, None, path) == Decision::Allow { + if visibility_check(rules, is_public, owner_did, caller, path) == Decision::Allow { allowed.insert(oid.clone()); } } @@ -743,6 +765,78 @@ mod tests { ); } + #[test] + fn allowed_set_excludes_dangling_blob_for_every_caller() { + // #126: a blob written via `git hash-object -w` but never referenced has + // no path to gate on, so it is absent from the reachable allowed-set — + // for anonymous callers, listed readers, AND the owner. The IPFS serve + // path relies on this fail-closed property to drop dangling withheld + // blobs that the deny-set model leaked. + let td = TempDir::new().unwrap(); + let work = td.path().join("work"); + std::fs::create_dir_all(work.join("public")).unwrap(); + std::fs::write(work.join("public/a.txt"), b"public bytes\n").unwrap(); + let run = |args: &[&str]| { + assert!( + Command::new("git") + .args(args) + .current_dir(&work) + .status() + .unwrap() + .success(), + "git {args:?} failed" + ); + }; + run(&["init", "-q"]); + run(&["config", "user.email", "t@t"]); + run(&["config", "user.name", "t"]); + run(&["add", "."]); + run(&["commit", "-qm", "init"]); + let oid_of = |rev: &str| { + let out = Command::new("git") + .args(["rev-parse", rev]) + .current_dir(&work) + .output() + .unwrap(); + String::from_utf8_lossy(&out.stdout).trim().to_string() + }; + let public_oid = oid_of("HEAD:public/a.txt"); + + std::fs::write(work.join("orphan.bin"), b"DANGLING SECRET\n").unwrap(); + let dangling_oid = { + let out = Command::new("git") + .args(["hash-object", "-w", "orphan.bin"]) + .current_dir(&work) + .output() + .unwrap(); + String::from_utf8_lossy(&out.stdout).trim().to_string() + }; + assert!( + matches!(dangling_oid.len(), 40 | 64), + "precondition: hash-object stored the dangling blob" + ); + + // Path-scoped rule: /secret/** denied to anon, allowed to a listed reader. + let reader = "did:key:zReader"; + let rules = [rule("/secret/**", &[reader])]; + + // Every gate-relevant caller: anonymous, listed reader, owner. None of + // them can put the dangling blob in the allowed set — it has no path. + for caller in [None, Some(reader), Some(OWNER)] { + let allowed = allowed_blob_set_for_caller(&work, &rules, true, OWNER, caller).unwrap(); + assert!( + !allowed.contains(&dangling_oid), + "dangling blob must be absent from allowed-set (caller={caller:?})" + ); + // Sanity: the reachable public blob is still in the set for every + // caller (the rule does not deny /public/**). + assert!( + allowed.contains(&public_oid), + "reachable public blob must be in allowed-set (caller={caller:?})" + ); + } + } + #[test] fn recipients_are_owner_plus_allowed_readers_only() { let (_td, repo, secret_oid, public_oid) = fixture(); diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 98fccc56..6de9b0f0 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -1842,4 +1842,116 @@ mod tests { "walk error fails closed: repo skipped, even the public blob is not served" ); } + + /// #126: a dangling blob (written via `git hash-object -w`, never referenced + /// by any commit/tree) must 404 through `GET /ipfs/{cid}` under path-scoped + /// rules — for anon AND the owner. The pre-#126 deny-set was fail-open by + /// construction: dangling oids were absent from the reachable enumeration + /// and thus absent from the deny-set, so the handler served 200. The + /// allowed-set is fail-closed: dangling oids are absent from the reachable + /// allowed-set, so the handler 404s (per team memory: the owner shift to + /// 404 is the accepted fail-closed default — owners can still + /// `git cat-file` directly). + #[sqlx::test] + async fn ipfs_cid_dangling_blob_fails_closed_under_path_rules(pool: PgPool) { + use crate::db::VisibilityMode; + use gitlawb_core::identity::Keypair; + + let owner = Keypair::generate(); + let owner_did = owner.did().to_string(); + let slug = owner_did.replace([':', '/'], "_"); + let short = owner_did.split(':').next_back().unwrap().to_string(); + let state = test_state(pool).await; + + // Seed a normal repo with `secret/b.txt` reachable from HEAD, so the + // path-scoped rule has something to match — without this the rule has + // no anchor and we'd be testing nothing. + let _fx = seed_cid_repos(&slug, &short, &["dangling"]); + let bare = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("dangling.git"); + + // Write a dangling blob: `git hash-object -w --stdin` adds it to the + // object DB but nothing references it, so the reachable walk never + // enumerates it. + let mut cmd = std::process::Command::new("git"); + cmd.args(["hash-object", "-w", "--stdin"]) + .current_dir(&bare) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()); + let mut child = cmd.spawn().expect("spawn git hash-object"); + { + use std::io::Write; + let stdin = child.stdin.as_mut().expect("stdin"); + stdin.write_all(b"DANGLING SECRET\n").expect("write stdin"); + } + let out = child.wait_with_output().expect("hash-object output"); + assert!( + out.status.success(), + "git hash-object: {}", + String::from_utf8_lossy(&out.stderr) + ); + let dangling_oid = String::from_utf8_lossy(&out.stdout).trim().to_string(); + // Sanity: must be a 64-hex sha256 oid, since the repo is sha256-format. + assert_eq!( + dangling_oid.len(), + 64, + "expected sha256 oid: {dangling_oid}" + ); + let dangling_cid = cid_for_oid(&dangling_oid); + + state + .db + .create_repo(&seed_repo(&owner_did, "dangling")) + .await + .expect("seed repo"); + let rec = state + .db + .get_repo(&owner_did, "dangling") + .await + .unwrap() + .unwrap(); + // Path-scoped rule triggers the per-blob allowed-set gate (KTD4). + state + .db + .set_visibility_rule(&rec.id, "/secret/**", VisibilityMode::B, &[], &owner_did) + .await + .expect("deny rule"); + + // anon: the dangling blob is absent from the reachable allowed-set → + // 404, no leak. Pre-#126 (deny-set) would serve 200. + let (st, body) = cid_parts( + cid_router(&state) + .oneshot(cid_anon(&dangling_cid)) + .await + .unwrap(), + ) + .await; + assert_eq!( + st, + StatusCode::NOT_FOUND, + "dangling blob must 404 under path-scoped rules" + ); + assert!( + !body.contains("DANGLING SECRET"), + "404 body must not leak the dangling content" + ); + + // owner (signed): same 404. The dangling blob has no path, so it's + // never visibility-checked → never in the allowed set, even for the + // owner. This is the accepted fail-closed shift documented in the PR. + let (st, body) = cid_parts( + cid_router(&state) + .oneshot(cid_signed(&owner, &dangling_cid)) + .await + .unwrap(), + ) + .await; + assert_eq!( + st, + StatusCode::NOT_FOUND, + "owner also 404s on dangling blobs under path-scoped rules (fail-closed default)" + ); + assert!(!body.contains("DANGLING SECRET")); + } } From 3aa7bf06f8cd9acfc26f6b7cea99441760a4716b Mon Sep 17 00:00:00 2001 From: Gravirei Date: Tue, 30 Jun 2026 20:07:58 +0600 Subject: [PATCH 02/12] perf(ipfs): check object existence before allowed-blob walk Move store::read_object before the allowed_blob_set_for_caller spawn_blocking call so random-CID spray against repos with path-scoped rules cannot trigger full-history git walks on repos that don't carry the object. --- crates/gitlawb-node/src/api/ipfs.rs | 141 ++++++++++++++-------------- 1 file changed, 72 insertions(+), 69 deletions(-) diff --git a/crates/gitlawb-node/src/api/ipfs.rs b/crates/gitlawb-node/src/api/ipfs.rs index 405eaedf..2177350a 100644 --- a/crates/gitlawb-node/src/api/ipfs.rs +++ b/crates/gitlawb-node/src/api/ipfs.rs @@ -40,14 +40,16 @@ use crate::visibility::{visibility_check, Decision}; /// caller passes. For each iterated row we gate against that row's OWN rules /// (`visibility_check` at `"/"`), never re-resolving via `authorize_repo_read` /// — `get_repo`'s fuzzy match could otherwise authorize a different physical -/// row than the one read (KTD2a). When the row carries path-scoped rules -/// (KTD4) the served object must be either a non-blob (trees/commits are -/// structural; KTD3) OR a blob in the caller's *reachable* allowed-set -/// (`allowed_blob_set_for_caller`). The reachable allowed-set excludes -/// dangling blobs — a blob written via `git hash-object -w` and never -/// committed has no path to gate, so it is fail-closed 404'd under -/// path-scoped rules (#126). Denial and genuine not-found both fall through -/// to an opaque 404. +/// row than the one read (KTD2a). We check object existence via +/// `store::read_object` *before* the expensive reachability walk so random-CID +/// spray cannot trigger full-history git walks on repos that don't carry the +/// object. When the row carries path-scoped rules (KTD4) the served object +/// must be either a non-blob (trees/commits are structural; KTD3) OR a blob +/// in the caller's *reachable* allowed-set (`allowed_blob_set_for_caller`). +/// The reachable allowed-set excludes dangling blobs — a blob written via +/// `git hash-object -w` and never committed has no path to gate, so it is +/// fail-closed 404'd under path-scoped rules (#126). Denial and genuine +/// not-found both fall through to an opaque 404. /// /// Scope: this closes the direct unauthenticated scan, including the dangling /// case. A stale-public mirror row still serves withheld content (tracked @@ -117,76 +119,77 @@ pub async fn get_by_cid( Err(_) => continue, }; + // Check whether the object exists in this repo before any expensive + // reachability walk. This prevents random-CID spray from triggering + // full-history git walks on repos that don't carry the object. + let object = store::read_object(&repo_path, &sha256_hex); + let (obj_type, content) = match object { + Ok(Some(t)) => t, + Ok(None) => continue, + Err(e) => { + tracing::warn!(repo = %repo.name, err = %e, "error reading git object"); + continue; + } + }; + // Per-blob gating only applies when a path-scoped rule exists (KTD4). // Without any path-scoped rule, the "/" gate above is the whole story. + // Trees/commits are always served under path-scoped rules (KTD3). let path_scoped = has_path_scoped_rule(rules); - if path_scoped && !allowed_memo.contains_key(&repo.id) { - let rp = repo_path.clone(); - let r = rules.to_vec(); - let is_public = repo.is_public; - let owner = repo.owner_did.clone(); - let caller_for_walk = caller_owned.clone(); - // Full-history walk shells out to git — keep it off the async runtime. - let walk = tokio::task::spawn_blocking(move || { - allowed_blob_set_for_caller(&rp, &r, is_public, &owner, caller_for_walk.as_deref()) - }) - .await; - // Fail closed on EITHER a task panic (JoinError) or a walk error: - // we cannot prove the caller may read here, so skip this repo and - // let a public copy (if any) serve. Never serve on an unproven gate. - let set = match walk { - Ok(Ok(set)) => set, - Ok(Err(e)) => { - tracing::warn!(repo = %repo.name, err = %e, "allowed-blob walk failed; skipping repo"); - continue; - } - Err(e) => { - tracing::warn!(repo = %repo.name, err = %e, "allowed-blob walk task panicked; skipping repo"); - continue; - } - }; - allowed_memo.insert(repo.id.clone(), set); - } - - match store::read_object(&repo_path, &sha256_hex) { - Ok(Some((obj_type, content))) => { - // Path-scoped rules: serve trees/commits unconditionally - // (structural; KTD3); a blob must be in the reachable - // allowed-set, which excludes dangling blobs (#126). - if path_scoped && obj_type == "blob" { - let in_allowed = allowed_memo - .get(&repo.id) - .is_some_and(|set| set.contains(&sha256_hex)); - if !in_allowed { + if path_scoped && obj_type == "blob" { + if !allowed_memo.contains_key(&repo.id) { + let rp = repo_path.clone(); + let r = rules.to_vec(); + let is_public = repo.is_public; + let owner = repo.owner_did.clone(); + let caller_for_walk = caller_owned.clone(); + // Full-history walk shells out to git — keep it off the async runtime. + let walk = tokio::task::spawn_blocking(move || { + allowed_blob_set_for_caller(&rp, &r, is_public, &owner, caller_for_walk.as_deref()) + }) + .await; + // Fail closed on EITHER a task panic (JoinError) or a walk error: + // we cannot prove the caller may read here, so skip this repo and + // let a public copy (if any) serve. Never serve on an unproven gate. + let set = match walk { + Ok(Ok(set)) => set, + Ok(Err(e)) => { + tracing::warn!(repo = %repo.name, err = %e, "allowed-blob walk failed; skipping repo"); + continue; + } + Err(e) => { + tracing::warn!(repo = %repo.name, err = %e, "allowed-blob walk task panicked; skipping repo"); continue; } - } - - // 3. Return the content with IPFS-style headers - let mut headers = HeaderMap::new(); - headers.insert( - HeaderName::from_static("content-type"), - HeaderValue::from_static("application/octet-stream"), - ); - headers.insert( - HeaderName::from_static("x-content-cid"), - HeaderValue::from_str(&cid_str) - .unwrap_or_else(|_| HeaderValue::from_static("invalid")), - ); - headers.insert( - HeaderName::from_static("x-git-hash"), - HeaderValue::from_str(&sha256_hex) - .unwrap_or_else(|_| HeaderValue::from_static("invalid")), - ); - - return Ok((StatusCode::OK, headers, content).into_response()); + }; + allowed_memo.insert(repo.id.clone(), set); } - Ok(None) => continue, - Err(e) => { - tracing::warn!(repo = %repo.name, err = %e, "error reading git object"); + let in_allowed = allowed_memo + .get(&repo.id) + .is_some_and(|set| set.contains(&sha256_hex)); + if !in_allowed { continue; } } + + // 3. Return the content with IPFS-style headers + let mut headers = HeaderMap::new(); + headers.insert( + HeaderName::from_static("content-type"), + HeaderValue::from_static("application/octet-stream"), + ); + headers.insert( + HeaderName::from_static("x-content-cid"), + HeaderValue::from_str(&cid_str) + .unwrap_or_else(|_| HeaderValue::from_static("invalid")), + ); + headers.insert( + HeaderName::from_static("x-git-hash"), + HeaderValue::from_str(&sha256_hex) + .unwrap_or_else(|_| HeaderValue::from_static("invalid")), + ); + + return Ok((StatusCode::OK, headers, content).into_response()); } // Not found in any repo From 63580c730da890851324a45d872ede1c3c7d9a62 Mon Sep 17 00:00:00 2001 From: Gravirei Date: Tue, 30 Jun 2026 23:10:27 +0600 Subject: [PATCH 03/12] refactor(ipfs): improve formatting and readability in get_by_cid function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ✓ P3 blocker fixed: cargo fmt applied — the format gate will pass. ✓ P3 cleanup resolved: withheld_blob_oids is still used by replication code in repos.rs, so it stays. • P2 follow-up: Tree/commit disclosure tracked in #135 — out of scope here. --- crates/gitlawb-node/src/api/ipfs.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/gitlawb-node/src/api/ipfs.rs b/crates/gitlawb-node/src/api/ipfs.rs index 2177350a..d6405fa0 100644 --- a/crates/gitlawb-node/src/api/ipfs.rs +++ b/crates/gitlawb-node/src/api/ipfs.rs @@ -145,7 +145,13 @@ pub async fn get_by_cid( let caller_for_walk = caller_owned.clone(); // Full-history walk shells out to git — keep it off the async runtime. let walk = tokio::task::spawn_blocking(move || { - allowed_blob_set_for_caller(&rp, &r, is_public, &owner, caller_for_walk.as_deref()) + allowed_blob_set_for_caller( + &rp, + &r, + is_public, + &owner, + caller_for_walk.as_deref(), + ) }) .await; // Fail closed on EITHER a task panic (JoinError) or a walk error: @@ -180,8 +186,7 @@ pub async fn get_by_cid( ); headers.insert( HeaderName::from_static("x-content-cid"), - HeaderValue::from_str(&cid_str) - .unwrap_or_else(|_| HeaderValue::from_static("invalid")), + HeaderValue::from_str(&cid_str).unwrap_or_else(|_| HeaderValue::from_static("invalid")), ); headers.insert( HeaderName::from_static("x-git-hash"), From 002f35405874898dc538773f0dcda13bafb56f49 Mon Sep 17 00:00:00 2001 From: Gravirei Date: Wed, 1 Jul 2026 02:25:29 +0600 Subject: [PATCH 04/12] refactor(ipfs): streamline object retrieval by separating type and content reading --- crates/gitlawb-node/src/api/ipfs.rs | 14 +++++++++--- crates/gitlawb-node/src/git/store.rs | 34 ++++++++++++++++++++-------- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/crates/gitlawb-node/src/api/ipfs.rs b/crates/gitlawb-node/src/api/ipfs.rs index d6405fa0..46cbf739 100644 --- a/crates/gitlawb-node/src/api/ipfs.rs +++ b/crates/gitlawb-node/src/api/ipfs.rs @@ -122,12 +122,11 @@ pub async fn get_by_cid( // Check whether the object exists in this repo before any expensive // reachability walk. This prevents random-CID spray from triggering // full-history git walks on repos that don't carry the object. - let object = store::read_object(&repo_path, &sha256_hex); - let (obj_type, content) = match object { + let obj_type = match store::object_type(&repo_path, &sha256_hex) { Ok(Some(t)) => t, Ok(None) => continue, Err(e) => { - tracing::warn!(repo = %repo.name, err = %e, "error reading git object"); + tracing::warn!(repo = %repo.name, err = %e, "error checking git object type"); continue; } }; @@ -178,6 +177,15 @@ pub async fn get_by_cid( } } + // Now that we've passed the gate, read the content. + let content = match store::read_object_content(&repo_path, &sha256_hex, &obj_type) { + Ok(c) => c, + Err(e) => { + tracing::warn!(repo = %repo.name, err = %e, "error reading git object content"); + continue; + } + }; + // 3. Return the content with IPFS-style headers let mut headers = HeaderMap::new(); headers.insert( diff --git a/crates/gitlawb-node/src/git/store.rs b/crates/gitlawb-node/src/git/store.rs index b9759147..290da6cc 100644 --- a/crates/gitlawb-node/src/git/store.rs +++ b/crates/gitlawb-node/src/git/store.rs @@ -271,9 +271,8 @@ pub struct TreeEntry { /// `/ipfs/` is computed from these same content bytes via /// `gitlawb_core::cid::Cid::from_git_object_bytes`. /// -/// Returns `None` if the object does not exist in this repo. -pub fn read_object(repo_path: &Path, sha256_hex: &str) -> Result)>> { - // First check if the object exists and get its type +/// Get just the object type. Returns `None` if the object doesn't exist. +pub fn object_type(repo_path: &Path, sha256_hex: &str) -> Result> { let type_output = Command::new("git") .args(["cat-file", "-t", sha256_hex]) .current_dir(repo_path) @@ -284,13 +283,13 @@ pub fn read_object(repo_path: &Path, sha256_hex: &str) -> Result Result> { let content_output = Command::new("git") - .args(["cat-file", &obj_type, sha256_hex]) + .args(["cat-file", obj_type, sha256_hex]) .current_dir(repo_path) .output() .context("failed to run git cat-file ")?; @@ -300,7 +299,24 @@ pub fn read_object(repo_path: &Path, sha256_hex: &str) -> Result` is computed from these same content bytes via +/// `gitlawb_core::cid::Cid::from_git_object_bytes`. +/// +/// Returns `None` if the object does not exist in this repo. +pub fn read_object(repo_path: &Path, sha256_hex: &str) -> Result)>> { + let obj_type = match object_type(repo_path, sha256_hex)? { + Some(t) => t, + None => return Ok(None), + }; + let content = read_object_content(repo_path, sha256_hex, &obj_type)?; + Ok(Some((obj_type, content))) } /// Get the diff between two branches: changes on source_branch not in target_branch. From f2c91a868afb57b7e2a2b949536015490492cef7 Mon Sep 17 00:00:00 2001 From: Gravirei Date: Wed, 1 Jul 2026 07:22:24 +0600 Subject: [PATCH 05/12] docs(ipfs): update get_by_cid comment to reflect split object retrieval --- crates/gitlawb-node/src/api/ipfs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/gitlawb-node/src/api/ipfs.rs b/crates/gitlawb-node/src/api/ipfs.rs index 46cbf739..f3de7570 100644 --- a/crates/gitlawb-node/src/api/ipfs.rs +++ b/crates/gitlawb-node/src/api/ipfs.rs @@ -41,7 +41,7 @@ use crate::visibility::{visibility_check, Decision}; /// (`visibility_check` at `"/"`), never re-resolving via `authorize_repo_read` /// — `get_repo`'s fuzzy match could otherwise authorize a different physical /// row than the one read (KTD2a). We check object existence via -/// `store::read_object` *before* the expensive reachability walk so random-CID +/// `store::object_type` *before* the expensive reachability walk so random-CID /// spray cannot trigger full-history git walks on repos that don't carry the /// object. When the row carries path-scoped rules (KTD4) the served object /// must be either a non-blob (trees/commits are structural; KTD3) OR a blob From 03ba7149fc248798df15f9fb26bf897fd2901b4d Mon Sep 17 00:00:00 2001 From: Gravirei Date: Wed, 1 Jul 2026 23:40:07 +0600 Subject: [PATCH 06/12] Run cargo fmt on store.rs --- crates/gitlawb-node/src/git/store.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/gitlawb-node/src/git/store.rs b/crates/gitlawb-node/src/git/store.rs index 290da6cc..229ee695 100644 --- a/crates/gitlawb-node/src/git/store.rs +++ b/crates/gitlawb-node/src/git/store.rs @@ -283,7 +283,11 @@ pub fn object_type(repo_path: &Path, sha256_hex: &str) -> Result> return Ok(None); } - Ok(Some(String::from_utf8_lossy(&type_output.stdout).trim().to_string())) + Ok(Some( + String::from_utf8_lossy(&type_output.stdout) + .trim() + .to_string(), + )) } /// Read an object's content if its type is already known. From 3ec0acc7c08b0f28f2f2efc1e00551a929d1a5b2 Mon Sep 17 00:00:00 2001 From: Gravirei Date: Mon, 6 Jul 2026 16:21:59 +0600 Subject: [PATCH 07/12] fix(visibility): gate repo-scoped read surfaces on visibility (#120) --- Cargo.lock | 10 +- crates/gitlawb-node/src/api/bounties.rs | 4 + crates/gitlawb-node/src/api/certs.rs | 26 +- crates/gitlawb-node/src/api/issues.rs | 37 +- crates/gitlawb-node/src/api/labels.rs | 9 +- crates/gitlawb-node/src/api/mod.rs | 8 - crates/gitlawb-node/src/api/stars.rs | 13 +- crates/gitlawb-node/src/test_support.rs | 540 ++++++++++++++++++++++++ 8 files changed, 595 insertions(+), 52 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 30abfa8e..7e46b0b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3300,7 +3300,7 @@ dependencies = [ [[package]] name = "git-remote-gitlawb" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "gitlawb-core", @@ -3312,7 +3312,7 @@ dependencies = [ [[package]] name = "gitlawb-attest" -version = "0.4.0" +version = "0.5.0" dependencies = [ "base64", "ed25519-dalek", @@ -3329,7 +3329,7 @@ dependencies = [ [[package]] name = "gitlawb-core" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "base64", @@ -3356,7 +3356,7 @@ dependencies = [ [[package]] name = "gitlawb-node" -version = "0.4.0" +version = "0.5.0" dependencies = [ "alloy", "anyhow", @@ -3412,7 +3412,7 @@ dependencies = [ [[package]] name = "gl" -version = "0.4.0" +version = "0.5.0" dependencies = [ "alloy", "anyhow", diff --git a/crates/gitlawb-node/src/api/bounties.rs b/crates/gitlawb-node/src/api/bounties.rs index caee03c6..88cf911f 100644 --- a/crates/gitlawb-node/src/api/bounties.rs +++ b/crates/gitlawb-node/src/api/bounties.rs @@ -115,7 +115,11 @@ pub async fn list_repo_bounties( State(state): State, Path((owner, repo)): Path<(String, String)>, Query(q): Query, + auth: Option>, ) -> Result> { + let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?; + let bounties = state .db .list_bounties( diff --git a/crates/gitlawb-node/src/api/certs.rs b/crates/gitlawb-node/src/api/certs.rs index f17ebfb7..c818bcef 100644 --- a/crates/gitlawb-node/src/api/certs.rs +++ b/crates/gitlawb-node/src/api/certs.rs @@ -1,8 +1,9 @@ //! API handlers for ref certificates. -use axum::extract::{Path, State}; +use axum::extract::{Extension, Path, State}; use axum::Json; +use crate::auth::AuthenticatedDid; use crate::error::{AppError, Result}; use crate::state::AppState; @@ -10,12 +11,11 @@ use crate::state::AppState; pub async fn list_certs( State(state): State, Path((owner, name)): Path<(String, String)>, + auth: Option>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &name) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?; + let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &name, caller, "/").await?; let certs = state.db.list_ref_certificates(&record.id).await?; let certs_json: Vec = certs @@ -42,13 +42,11 @@ pub async fn list_certs( pub async fn get_cert( State(state): State, Path((owner, name, id)): Path<(String, String, String)>, + auth: Option>, ) -> Result> { - // Verify the repo exists - let _record = state - .db - .get_repo(&owner, &name) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?; + let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &name, caller, "/").await?; let cert = state .db @@ -56,6 +54,10 @@ pub async fn get_cert( .await? .ok_or_else(|| AppError::NotFound(format!("certificate {id}")))?; + if cert.repo_id != record.id { + return Err(AppError::NotFound(format!("certificate {id}"))); + } + Ok(Json(serde_json::json!({ "id": cert.id, "repo_id": cert.repo_id, diff --git a/crates/gitlawb-node/src/api/issues.rs b/crates/gitlawb-node/src/api/issues.rs index 59b647c8..703d110c 100644 --- a/crates/gitlawb-node/src/api/issues.rs +++ b/crates/gitlawb-node/src/api/issues.rs @@ -90,12 +90,11 @@ pub async fn create_issue( pub async fn list_issues( State(state): State, Path((owner, repo)): Path<(String, String)>, + auth: Option>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?; let disk_path = state .repo_store @@ -120,12 +119,11 @@ pub async fn list_issues( pub async fn get_issue( State(state): State, Path((owner, repo, issue_id)): Path<(String, String, String)>, + auth: Option>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?; let disk_path = state .repo_store @@ -191,12 +189,21 @@ pub async fn create_issue_comment( pub async fn list_issue_comments( State(state): State, Path((owner, repo, issue_id)): Path<(String, String, String)>, + auth: Option>, ) -> Result> { - let _record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?; + + let disk_path = state + .repo_store + .acquire(&record.owner_did, &record.name) + .await + .map_err(|e| AppError::Git(e.to_string()))?; + // Confirm the issue belongs to this repo before listing comments. + git_issues::get_issue(&disk_path, &issue_id) + .map_err(|e| AppError::Git(e.to_string()))? + .ok_or_else(|| AppError::RepoNotFound(format!("issue {issue_id} not found")))?; let comments = state.db.list_issue_comments(&issue_id).await?; Ok(Json(serde_json::json!({ "comments": comments }))) diff --git a/crates/gitlawb-node/src/api/labels.rs b/crates/gitlawb-node/src/api/labels.rs index 9b904277..7b42892f 100644 --- a/crates/gitlawb-node/src/api/labels.rs +++ b/crates/gitlawb-node/src/api/labels.rs @@ -74,12 +74,11 @@ pub async fn remove_label( pub async fn list_labels( State(state): State, Path((owner, name)): Path<(String, String)>, + auth: Option>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &name) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?; + let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &name, caller, "/").await?; let labels = state.db.list_labels(&record.id).await?; Ok(Json(serde_json::json!({ "labels": labels }))) diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index 939c8120..c2988fae 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -504,14 +504,6 @@ mod authz_guard { // Repo-scoped reads known to be ungated today, each tracked by an issue. // Remove an entry the moment its gate lands (the staleness assert enforces it). let known_ungated: &[(&str, &str)] = &[ - ("list_certs", "#120"), - ("get_cert", "#120"), - ("list_issues", "#120"), - ("get_issue", "#120"), - ("list_issue_comments", "#120"), - ("list_labels", "#120"), - ("list_repo_bounties", "#120"), - ("get_star_status", "#120"), ("list_webhooks", "#94 (PR #113)"), ("list_replicas", "PR #113"), ("list_protected_branches", "PR #113"), diff --git a/crates/gitlawb-node/src/api/stars.rs b/crates/gitlawb-node/src/api/stars.rs index 6e06b799..61e0630b 100644 --- a/crates/gitlawb-node/src/api/stars.rs +++ b/crates/gitlawb-node/src/api/stars.rs @@ -2,7 +2,7 @@ //! //! Any authenticated agent can star or unstar a repo. //! One agent = one star per repo (enforced by UNIQUE constraint in repo_stars). -//! Star count is public — no auth required on GET. +//! Star count is gated on repo read access. use axum::extract::{Extension, Path, State}; use axum::http::StatusCode; @@ -74,16 +74,15 @@ pub async fn unstar_repo( } /// GET /api/v1/repos/:owner/:repo/star -/// Returns star count — unauthenticated. +/// Returns star count for callers who can read the repo. pub async fn get_star_status( State(state): State, Path((owner, repo)): Path<(String, String)>, + auth: Option>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?; let count = state.db.count_stars(&record.id).await?; diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 02b443b4..856c8751 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -2386,4 +2386,544 @@ mod tests { ); assert!(!body.contains("DANGLING SECRET")); } + + // --------------------------------------------------------------------------- + // Issue #120 — repo-scoped read surfaces visibility gate + // --------------------------------------------------------------------------- + + #[sqlx::test] + async fn list_certs_gate_denies_anon_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zCERTSOWNER0000000000000000000000000000000"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/certs", + axum::routing::get(crate::api::certs::list_certs), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(anon_get( + "/api/v1/repos/zCERTSOWNER0000000000000000000000000000000/secret-repo/certs", + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn list_certs_gate_admits_owner_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zCERTSOWNER1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/certs", + axum::routing::get(crate::api::certs::list_certs), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + "/api/v1/repos/zCERTSOWNER1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/secret-repo/certs", + Body::empty(), + )) + .await + .unwrap(); + assert!(resp.status().is_success()); + } + + #[sqlx::test] + async fn get_cert_gate_denies_anon_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zCERTGETOWN00000000000000000000000000000000"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/certs/{id}", + axum::routing::get(crate::api::certs::get_cert), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(anon_get("/api/v1/repos/zCERTGETOWN00000000000000000000000000000000/secret-repo/certs/nonexistent")) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn get_cert_gate_admits_owner_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zCERTGETOWN1BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; + let repo = seed_private_repo(owner, "secret-repo"); + let repo_id = repo.id.clone(); + state.db.create_repo(&repo).await.unwrap(); + let cert = crate::db::RefCertificate { + id: "real-cert-120".into(), + repo_id: repo_id.clone(), + ref_name: "refs/heads/main".into(), + old_sha: "0".repeat(40), + new_sha: "b".repeat(40), + pusher_did: owner.into(), + node_did: "did:key:zNode".into(), + signature: "sig".into(), + issued_at: "2026-01-01T00:00:00Z".into(), + }; + state.db.insert_ref_certificate(&cert).await.unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/certs/{id}", + axum::routing::get(crate::api::certs::get_cert), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + "/api/v1/repos/zCERTGETOWN1BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB/secret-repo/certs/real-cert-120", + Body::empty(), + )) + .await + .unwrap(); + assert!(resp.status().is_success()); + } + + #[sqlx::test] + async fn list_issues_gate_denies_anon_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zISSOWNER0000000000000000000000000000000000"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/issues", + axum::routing::get(crate::api::issues::list_issues), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(anon_get( + "/api/v1/repos/zISSOWNER0000000000000000000000000000000000/secret-repo/issues", + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn list_issues_gate_admits_owner_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zISSOWNER1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let slug = owner.replace([':', '/'], "_"); + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + let repo_dir = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("secret-repo.git"); + let _ = std::fs::remove_dir_all(&repo_dir); + std::fs::create_dir_all(repo_dir.parent().unwrap()).unwrap(); + let _repo_guard = DirGuard(repo_dir.clone()); + crate::git::store::init_bare(&repo_dir).unwrap(); + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/issues", + axum::routing::get(crate::api::issues::list_issues), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + "/api/v1/repos/zISSOWNER1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/secret-repo/issues", + Body::empty(), + )) + .await + .unwrap(); + assert!(resp.status().is_success()); + } + + #[sqlx::test] + async fn get_issue_gate_denies_anon_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zISGETOWN0000000000000000000000000000000000"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/issues/{id}", + axum::routing::get(crate::api::issues::get_issue), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(anon_get("/api/v1/repos/zISGETOWN0000000000000000000000000000000000/secret-repo/issues/nonexistent")) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn get_issue_gate_admits_owner_on_private(pool: PgPool) { + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + let state = test_state(pool).await; + let owner = "did:key:zISGETOWN1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let slug = owner.replace([':', '/'], "_"); + let repo_dir = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("secret-repo.git"); + let _ = std::fs::remove_dir_all(&repo_dir); + std::fs::create_dir_all(repo_dir.parent().unwrap()).unwrap(); + let _repo_guard = DirGuard(repo_dir.clone()); + crate::git::store::init_bare(&repo_dir).unwrap(); + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let issue_id = "real-issue-120"; + let issue_json = serde_json::json!({ + "id": issue_id, + "title": "Test Issue", + "body": "test body", + "author": owner, + "created_at": "2026-01-01T00:00:00Z", + "status": "open", + }); + crate::git::issues::create_issue(&repo_dir, issue_id, &issue_json.to_string()).unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/issues/{id}", + axum::routing::get(crate::api::issues::get_issue), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + &format!("/api/v1/repos/{owner}/secret-repo/issues/{issue_id}"), + Body::empty(), + )) + .await + .unwrap(); + assert!(resp.status().is_success()); + } + + #[sqlx::test] + async fn list_issue_comments_gate_denies_anon_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zISCMTOWN0000000000000000000000000000000000"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/issues/{id}/comments", + axum::routing::get(crate::api::issues::list_issue_comments), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(anon_get("/api/v1/repos/zISCMTOWN0000000000000000000000000000000000/secret-repo/issues/nonexistent/comments")) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn list_issue_comments_gate_admits_owner_on_private(pool: PgPool) { + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + let state = test_state(pool).await; + let owner = "did:key:zISCMTOWN1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let short_key = "zISCMTOWN1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let slug = owner.replace([':', '/'], "_"); + let repo_dir = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("secret-repo.git"); + let _ = std::fs::remove_dir_all(&repo_dir); + std::fs::create_dir_all(repo_dir.parent().unwrap()).unwrap(); + let _repo_guard = DirGuard(repo_dir.clone()); + crate::git::store::init_bare(&repo_dir).unwrap(); + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let issue_id = "real-issue-comment-120"; + let issue_json = serde_json::json!({ + "id": issue_id, + "title": "Test Issue", + "body": "test body", + "author": owner, + "created_at": "2026-01-01T00:00:00Z", + "status": "open", + }); + crate::git::issues::create_issue(&repo_dir, issue_id, &issue_json.to_string()).unwrap(); + let comment = crate::db::IssueComment { + id: "real-comment-120".into(), + issue_id: issue_id.into(), + author_did: owner.into(), + body: "a comment".into(), + created_at: "2026-01-01T00:00:00Z".into(), + }; + state.db.create_issue_comment(&comment).await.unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/issues/{id}/comments", + axum::routing::get(crate::api::issues::list_issue_comments), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + &format!("/api/v1/repos/{short_key}/secret-repo/issues/{issue_id}/comments"), + Body::empty(), + )) + .await + .unwrap(); + assert!(resp.status().is_success()); + } + + #[sqlx::test] + async fn list_labels_gate_denies_anon_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zLABELOWN00000000000000000000000000000000000"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/labels", + axum::routing::get(crate::api::labels::list_labels), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(anon_get( + "/api/v1/repos/zLABELOWN00000000000000000000000000000000000/secret-repo/labels", + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn list_labels_gate_admits_owner_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zLABELOWN1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/labels", + axum::routing::get(crate::api::labels::list_labels), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + "/api/v1/repos/zLABELOWN1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/secret-repo/labels", + Body::empty(), + )) + .await + .unwrap(); + assert!(resp.status().is_success()); + } + + #[sqlx::test] + async fn list_repo_bounties_gate_denies_anon_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zBONOWNER00000000000000000000000000000000000"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/bounties", + axum::routing::get(crate::api::bounties::list_repo_bounties), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(anon_get( + "/api/v1/repos/zBONOWNER00000000000000000000000000000000000/secret-repo/bounties", + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn get_star_status_gate_denies_anon_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zSTAROWN000000000000000000000000000000000000"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/star", + axum::routing::get(crate::api::stars::get_star_status), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(anon_get( + "/api/v1/repos/zSTAROWN000000000000000000000000000000000000/secret-repo/star", + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn get_star_status_gate_admits_owner_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zSTAROWN1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/star", + axum::routing::get(crate::api::stars::get_star_status), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + "/api/v1/repos/zSTAROWN1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/secret-repo/star", + Body::empty(), + )) + .await + .unwrap(); + assert!(resp.status().is_success()); + } + + #[sqlx::test] + async fn list_repo_bounties_gate_admits_owner_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zBONOWNER1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/bounties", + axum::routing::get(crate::api::bounties::list_repo_bounties), + ) + .with_state(state.clone()) + }; + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + "/api/v1/repos/zBONOWNER1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/secret-repo/bounties", + Body::empty(), + )) + .await + .unwrap(); + assert!(resp.status().is_success()); + } } From f3b08c18f3fd49ff7e24e26e34512ec472d7d851 Mon Sep 17 00:00:00 2001 From: Gravirei Date: Mon, 6 Jul 2026 22:58:01 +0600 Subject: [PATCH 08/12] fix(visibility): enhance repo read authorization checks and improve test coverage --- crates/gitlawb-node/src/api/bounties.rs | 20 +- crates/gitlawb-node/src/server.rs | 3 +- crates/gitlawb-node/src/test_support.rs | 319 ++++++++++++++++++++++-- crates/gitlawb-node/src/visibility.rs | 3 +- 4 files changed, 320 insertions(+), 25 deletions(-) diff --git a/crates/gitlawb-node/src/api/bounties.rs b/crates/gitlawb-node/src/api/bounties.rs index 88cf911f..f37dc347 100644 --- a/crates/gitlawb-node/src/api/bounties.rs +++ b/crates/gitlawb-node/src/api/bounties.rs @@ -137,25 +137,43 @@ pub async fn list_repo_bounties( pub async fn list_all_bounties( State(state): State, Query(q): Query, + auth: Option>, ) -> Result> { let bounties = state .db .list_bounties(None, None, q.status.as_deref(), q.limit.unwrap_or(50)) .await?; - Ok(Json(serde_json::json!({ "bounties": bounties }))) + let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + let mut allowed = Vec::new(); + for b in bounties { + if crate::api::authorize_repo_read(&state, &b.repo_owner, &b.repo_name, caller, "/") + .await + .is_ok() + { + allowed.push(b); + } + } + + Ok(Json(serde_json::json!({ "bounties": allowed }))) } /// GET /api/v1/bounties/{id} pub async fn get_bounty( State(state): State, Path(id): Path, + auth: Option>, ) -> Result> { let bounty = state .db .get_bounty(&id) .await? .ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?; + + let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + crate::api::authorize_repo_read(&state, &bounty.repo_owner, &bounty.repo_name, caller, "/") + .await?; + Ok(Json(bounty)) } diff --git a/crates/gitlawb-node/src/server.rs b/crates/gitlawb-node/src/server.rs index 54939722..e808ddc8 100644 --- a/crates/gitlawb-node/src/server.rs +++ b/crates/gitlawb-node/src/server.rs @@ -248,7 +248,8 @@ pub fn build_router(state: AppState) -> Router { .route( "/api/v1/agents/{did}/bounties", get(bounties::agent_bounty_stats), - ); + ) + .layer(middleware::from_fn(auth::optional_signature)); // ── Profile routes (write — require HTTP Signature) ───────────────── let profile_write_routes = add_auth_layers( diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 856c8751..31552533 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -2452,11 +2452,22 @@ mod tests { async fn get_cert_gate_denies_anon_on_private(pool: PgPool) { let state = test_state(pool).await; let owner = "did:key:zCERTGETOWN00000000000000000000000000000000"; - state - .db - .create_repo(&seed_private_repo(owner, "secret-repo")) - .await - .unwrap(); + let repo = seed_private_repo(owner, "secret-repo"); + let repo_id = repo.id.clone(); + state.db.create_repo(&repo).await.unwrap(); + + let cert = crate::db::RefCertificate { + id: "real-cert-120".into(), + repo_id, + ref_name: "refs/heads/main".into(), + old_sha: "0".repeat(40), + new_sha: "b".repeat(40), + pusher_did: owner.into(), + node_did: "did:key:zNode".into(), + signature: "sig".into(), + issued_at: "2026-01-01T00:00:00Z".into(), + }; + state.db.insert_ref_certificate(&cert).await.unwrap(); let router = || { Router::new() @@ -2467,7 +2478,7 @@ mod tests { .with_state(state.clone()) }; let resp = router() - .oneshot(anon_get("/api/v1/repos/zCERTGETOWN00000000000000000000000000000000/secret-repo/certs/nonexistent")) + .oneshot(anon_get("/api/v1/repos/zCERTGETOWN00000000000000000000000000000000/secret-repo/certs/real-cert-120")) .await .unwrap(); assert_eq!(resp.status(), StatusCode::NOT_FOUND); @@ -2587,14 +2598,39 @@ mod tests { #[sqlx::test] async fn get_issue_gate_denies_anon_on_private(pool: PgPool) { + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } let state = test_state(pool).await; let owner = "did:key:zISGETOWN0000000000000000000000000000000000"; + let slug = owner.replace([':', '/'], "_"); + let repo_dir = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("secret-repo.git"); + let _ = std::fs::remove_dir_all(&repo_dir); + std::fs::create_dir_all(repo_dir.parent().unwrap()).unwrap(); + crate::git::store::init_bare(&repo_dir).unwrap(); + let _repo_guard = DirGuard(repo_dir.clone()); state .db .create_repo(&seed_private_repo(owner, "secret-repo")) .await .unwrap(); + let issue_id = "real-issue-120"; + let issue_json = serde_json::json!({ + "id": issue_id, + "title": "Test Issue", + "body": "test body", + "author": owner, + "created_at": "2026-01-01T00:00:00Z", + "status": "open", + }); + crate::git::issues::create_issue(&repo_dir, issue_id, &issue_json.to_string()).unwrap(); + let router = || { Router::new() .route( @@ -2604,7 +2640,7 @@ mod tests { .with_state(state.clone()) }; let resp = router() - .oneshot(anon_get("/api/v1/repos/zISGETOWN0000000000000000000000000000000000/secret-repo/issues/nonexistent")) + .oneshot(anon_get("/api/v1/repos/zISGETOWN0000000000000000000000000000000000/secret-repo/issues/real-issue-120")) .await .unwrap(); assert_eq!(resp.status(), StatusCode::NOT_FOUND); @@ -2668,14 +2704,47 @@ mod tests { #[sqlx::test] async fn list_issue_comments_gate_denies_anon_on_private(pool: PgPool) { + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } let state = test_state(pool).await; let owner = "did:key:zISCMTOWN0000000000000000000000000000000000"; + let slug = owner.replace([':', '/'], "_"); + let repo_dir = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("secret-repo.git"); + let _ = std::fs::remove_dir_all(&repo_dir); + std::fs::create_dir_all(repo_dir.parent().unwrap()).unwrap(); + crate::git::store::init_bare(&repo_dir).unwrap(); + let _repo_guard = DirGuard(repo_dir.clone()); state .db .create_repo(&seed_private_repo(owner, "secret-repo")) .await .unwrap(); + let issue_id = "real-issue-comment-120"; + let issue_json = serde_json::json!({ + "id": issue_id, + "title": "Test Issue", + "body": "test body", + "author": owner, + "created_at": "2026-01-01T00:00:00Z", + "status": "open", + }); + crate::git::issues::create_issue(&repo_dir, issue_id, &issue_json.to_string()).unwrap(); + let comment = crate::db::IssueComment { + id: "real-comment-120".into(), + issue_id: issue_id.into(), + author_did: owner.into(), + body: "a comment".into(), + created_at: "2026-01-01T00:00:00Z".into(), + }; + state.db.create_issue_comment(&comment).await.unwrap(); + let router = || { Router::new() .route( @@ -2685,7 +2754,7 @@ mod tests { .with_state(state.clone()) }; let resp = router() - .oneshot(anon_get("/api/v1/repos/zISCMTOWN0000000000000000000000000000000000/secret-repo/issues/nonexistent/comments")) + .oneshot(anon_get("/api/v1/repos/zISCMTOWN0000000000000000000000000000000000/secret-repo/issues/real-issue-comment-120/comments")) .await .unwrap(); assert_eq!(resp.status(), StatusCode::NOT_FOUND); @@ -2823,15 +2892,8 @@ mod tests { .await .unwrap(); - let router = || { - Router::new() - .route( - "/api/v1/repos/{owner}/{repo}/bounties", - axum::routing::get(crate::api::bounties::list_repo_bounties), - ) - .with_state(state.clone()) - }; - let resp = router() + let router = crate::server::build_router(state); + let resp = router .oneshot(anon_get( "/api/v1/repos/zBONOWNER00000000000000000000000000000000000/secret-repo/bounties", )) @@ -2900,26 +2962,239 @@ mod tests { #[sqlx::test] async fn list_repo_bounties_gate_admits_owner_on_private(pool: PgPool) { let state = test_state(pool).await; - let owner = "did:key:zBONOWNER1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let kp = gitlawb_core::identity::Keypair::generate(); + let owner = kp.did().to_string(); + let short = owner.split(':').next_back().unwrap(); state .db - .create_repo(&seed_private_repo(owner, "secret-repo")) + .create_repo(&seed_private_repo(&owner, "secret-repo")) .await .unwrap(); + let router = crate::server::build_router(state); + let uri = format!("/api/v1/repos/{short}/secret-repo/bounties"); + let sig = gitlawb_core::http_sig::sign_request(&kp, "GET", &uri, b""); + let req = Request::builder() + .method(Method::GET) + .uri(uri) + .header("content-type", "application/json") + .header("content-digest", sig.content_digest) + .header("signature-input", sig.signature_input) + .header("signature", sig.signature) + .body(Body::empty()) + .unwrap(); + + let resp = router.oneshot(req).await.unwrap(); + assert!(resp.status().is_success()); + } + + #[sqlx::test] + async fn get_cert_rejects_cross_repo_idor(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zCERTIDOROWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let short = owner.split(':').next_back().unwrap(); + let repo_a = seed_private_repo(owner, "repo-a"); + state.db.create_repo(&repo_a).await.unwrap(); + + let repo_b = seed_private_repo(owner, "repo-b"); + let repo_b_id = repo_b.id.clone(); + state.db.create_repo(&repo_b).await.unwrap(); + + let cert = crate::db::RefCertificate { + id: "cert-in-b".into(), + repo_id: repo_b_id, + ref_name: "refs/heads/main".into(), + old_sha: "0".repeat(40), + new_sha: "b".repeat(40), + pusher_did: owner.into(), + node_did: "did:key:zNode".into(), + signature: "sig".into(), + issued_at: "2026-01-01T00:00:00Z".into(), + }; + state.db.insert_ref_certificate(&cert).await.unwrap(); + let router = || { Router::new() .route( - "/api/v1/repos/{owner}/{repo}/bounties", - axum::routing::get(crate::api::bounties::list_repo_bounties), + "/api/v1/repos/{owner}/{repo}/certs/{id}", + axum::routing::get(crate::api::certs::get_cert), ) .with_state(state.clone()) }; + + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + &format!("/api/v1/repos/{short}/repo-a/certs/cert-in-b"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn list_issue_comments_rejects_cross_repo_idor(pool: PgPool) { + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + let state = test_state(pool).await; + let owner = "did:key:zISSCMTIDORAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let short = owner.split(':').next_back().unwrap(); + let slug = owner.replace([':', '/'], "_"); + + let repo_dir_a = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("repo-a.git"); + let _ = std::fs::remove_dir_all(&repo_dir_a); + std::fs::create_dir_all(repo_dir_a.parent().unwrap()).unwrap(); + crate::git::store::init_bare(&repo_dir_a).unwrap(); + let _guard_a = DirGuard(repo_dir_a.clone()); + state + .db + .create_repo(&seed_private_repo(owner, "repo-a")) + .await + .unwrap(); + + let repo_dir_b = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("repo-b.git"); + let _ = std::fs::remove_dir_all(&repo_dir_b); + std::fs::create_dir_all(repo_dir_b.parent().unwrap()).unwrap(); + crate::git::store::init_bare(&repo_dir_b).unwrap(); + let _guard_b = DirGuard(repo_dir_b.clone()); + state + .db + .create_repo(&seed_private_repo(owner, "repo-b")) + .await + .unwrap(); + + let issue_id = "idor-issue-120"; + let issue_json = serde_json::json!({ + "id": issue_id, + "title": "Test Issue", + "body": "test body", + "author": owner, + "created_at": "2026-01-01T00:00:00Z", + "status": "open", + }); + crate::git::issues::create_issue(&repo_dir_b, issue_id, &issue_json.to_string()).unwrap(); + let comment = crate::db::IssueComment { + id: "idor-comment-120".into(), + issue_id: issue_id.into(), + author_did: owner.into(), + body: "a comment".into(), + created_at: "2026-01-01T00:00:00Z".into(), + }; + state.db.create_issue_comment(&comment).await.unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/issues/{id}/comments", + axum::routing::get(crate::api::issues::list_issue_comments), + ) + .with_state(state.clone()) + }; + + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + &format!("/api/v1/repos/{short}/repo-a/issues/{issue_id}/comments"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn repo_gate_quarantined_repo_denied(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zQUARANTINEOWNERAAAAAAAAAAAAAAAAAAAAAAAAA"; + let short = owner.split(':').next_back().unwrap(); + let mut repo = seed_private_repo(owner, "quarantined-repo"); + repo.is_public = true; // Make it public to prove quarantine still denies it + let repo_id = repo.id.clone(); + state.db.create_repo(&repo).await.unwrap(); + + state.db.set_repo_quarantine(&repo_id, true).await.unwrap(); + + let router = crate::server::build_router(state); + let resp = router + .oneshot(anon_get(&format!( + "/api/v1/repos/{short}/quarantined-repo/issues" + ))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn repo_gate_public_repo_anon_read_admitted(pool: PgPool) { + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + let state = test_state(pool).await; + let owner = "did:key:zPUBLICREPOOWNERAAAAAAAAAAAAAAAAAAAAAAAAA"; + let short = owner.split(':').next_back().unwrap(); + + let slug = owner.replace([':', '/'], "_"); + let repo_dir = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("public-repo.git"); + let _ = std::fs::remove_dir_all(&repo_dir); + std::fs::create_dir_all(repo_dir.parent().unwrap()).unwrap(); + crate::git::store::init_bare(&repo_dir).unwrap(); + let _repo_guard = DirGuard(repo_dir.clone()); + + let mut repo = seed_private_repo(owner, "public-repo"); + repo.is_public = true; + state.db.create_repo(&repo).await.unwrap(); + + let router = crate::server::build_router(state); + let resp = router + .oneshot(anon_get(&format!( + "/api/v1/repos/{short}/public-repo/issues" + ))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + } + + #[sqlx::test] + async fn repo_gate_owner_bare_key_vs_full_did(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zBAREKEYFULLDIDOWNERAAAAAAAAAAAAAAAAAA"; + let short = owner.split(':').next_back().unwrap(); + + // Save repo with bare key as owner + let repo = seed_private_repo(short, "bare-repo"); + state.db.create_repo(&repo).await.unwrap(); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/certs", + axum::routing::get(crate::api::certs::list_certs), + ) + .with_state(state.clone()) + }; + + // Caller is full DID, should match bare key in DB let resp = router() .oneshot(signed_request_as( owner, Method::GET, - "/api/v1/repos/zBONOWNER1AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/secret-repo/bounties", + &format!("/api/v1/repos/{short}/bare-repo/certs"), Body::empty(), )) .await diff --git a/crates/gitlawb-node/src/visibility.rs b/crates/gitlawb-node/src/visibility.rs index 85040aa8..657bb2fe 100644 --- a/crates/gitlawb-node/src/visibility.rs +++ b/crates/gitlawb-node/src/visibility.rs @@ -33,7 +33,8 @@ fn nfc(s: &str) -> String { /// would let a non-key canonical row bypass the #124 visibility gate. fn is_owner(owner_did: &str, caller: &str) -> bool { let owner_short = crate::db::normalize_owner_key(owner_did); - caller == owner_did || caller == owner_short + let caller_short = crate::db::normalize_owner_key(caller); + owner_short == caller_short } /// The match prefix for a glob: "/" stays "/", "/secret/**" becomes "/secret". From 459e21e4039f6b8673e5416845de2a79e6ab2289 Mon Sep 17 00:00:00 2001 From: Gravirei Date: Tue, 7 Jul 2026 09:18:54 +0600 Subject: [PATCH 09/12] Fix review feedback: clamp limit, memoize authz, identical 404 body, resolved issue id in comments --- crates/gitlawb-node/src/api/bounties.rs | 46 ++++--- crates/gitlawb-node/src/api/issues.rs | 16 ++- crates/gitlawb-node/src/test_support.rs | 156 ++++++++++++++++++++++++ 3 files changed, 198 insertions(+), 20 deletions(-) diff --git a/crates/gitlawb-node/src/api/bounties.rs b/crates/gitlawb-node/src/api/bounties.rs index f37dc347..ad275fe0 100644 --- a/crates/gitlawb-node/src/api/bounties.rs +++ b/crates/gitlawb-node/src/api/bounties.rs @@ -110,6 +110,8 @@ pub async fn create_bounty( Ok((StatusCode::CREATED, Json(bounty))) } +const MAX_BOUNTY_LIMIT: i64 = 200; + /// GET /api/v1/repos/{owner}/{repo}/bounties pub async fn list_repo_bounties( State(state): State, @@ -120,14 +122,10 @@ pub async fn list_repo_bounties( let caller = auth.as_ref().map(|e| e.0 .0.as_str()); crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?; + let limit = q.limit.unwrap_or(50).clamp(1, MAX_BOUNTY_LIMIT); let bounties = state .db - .list_bounties( - Some(&owner), - Some(&repo), - q.status.as_deref(), - q.limit.unwrap_or(50), - ) + .list_bounties(Some(&owner), Some(&repo), q.status.as_deref(), limit) .await?; Ok(Json(serde_json::json!({ "bounties": bounties }))) @@ -139,21 +137,35 @@ pub async fn list_all_bounties( Query(q): Query, auth: Option>, ) -> Result> { + let raw_limit = q.limit.unwrap_or(50); + let limit = raw_limit.clamp(1, MAX_BOUNTY_LIMIT); + // Over-fetch to account for filtered-out private-repo bounties so the + // caller gets up to `limit` readable results even when newer rows are + // inaccessible. + let fetch = (limit * 5).min(1000); let bounties = state .db - .list_bounties(None, None, q.status.as_deref(), q.limit.unwrap_or(50)) + .list_bounties(None, None, q.status.as_deref(), fetch) .await?; let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + let mut seen = std::collections::HashSet::new(); let mut allowed = Vec::new(); - for b in bounties { + for b in &bounties { + let key = format!("{}/{}", b.repo_owner, b.repo_name); + if seen.contains(&key) { + allowed.push(b.clone()); + continue; + } + seen.insert(key.clone()); if crate::api::authorize_repo_read(&state, &b.repo_owner, &b.repo_name, caller, "/") .await .is_ok() { - allowed.push(b); + allowed.push(b.clone()); } } + allowed.truncate(limit as usize); Ok(Json(serde_json::json!({ "bounties": allowed }))) } @@ -164,15 +176,17 @@ pub async fn get_bounty( Path(id): Path, auth: Option>, ) -> Result> { - let bounty = state - .db - .get_bounty(&id) - .await? - .ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?; + let not_found = || AppError::NotFound(format!("bounty {id} not found")); + + let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?; let caller = auth.as_ref().map(|e| e.0 .0.as_str()); - crate::api::authorize_repo_read(&state, &bounty.repo_owner, &bounty.repo_name, caller, "/") - .await?; + if crate::api::authorize_repo_read(&state, &bounty.repo_owner, &bounty.repo_name, caller, "/") + .await + .is_err() + { + return Err(not_found()); + } Ok(Json(bounty)) } diff --git a/crates/gitlawb-node/src/api/issues.rs b/crates/gitlawb-node/src/api/issues.rs index 703d110c..17acf9af 100644 --- a/crates/gitlawb-node/src/api/issues.rs +++ b/crates/gitlawb-node/src/api/issues.rs @@ -200,12 +200,20 @@ pub async fn list_issue_comments( .acquire(&record.owner_did, &record.name) .await .map_err(|e| AppError::Git(e.to_string()))?; - // Confirm the issue belongs to this repo before listing comments. - git_issues::get_issue(&disk_path, &issue_id) + // Resolve the full issue ID (accepts 8-char prefix) so the DB fetch + // below uses the same canonical id as the git ref. + let full_id = match git_issues::resolve_issue_id(&disk_path, &issue_id) .map_err(|e| AppError::Git(e.to_string()))? - .ok_or_else(|| AppError::RepoNotFound(format!("issue {issue_id} not found")))?; + { + Some(id) => id, + None => { + return Err(AppError::RepoNotFound(format!( + "issue {issue_id} not found" + ))) + } + }; - let comments = state.db.list_issue_comments(&issue_id).await?; + let comments = state.db.list_issue_comments(&full_id).await?; Ok(Json(serde_json::json!({ "comments": comments }))) } diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 31552533..e1da6856 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -3170,6 +3170,162 @@ mod tests { assert_eq!(resp.status(), StatusCode::OK); } + #[sqlx::test] + async fn get_bounty_gate_denies_anon_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zGNB0UNTYANONPRIVOWNERAAAAAAAAAAAAAAAAAAA"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + let bounty = crate::db::BountyRecord { + id: "anon-private-bounty".into(), + repo_owner: owner.into(), + repo_name: "secret-repo".into(), + issue_id: None, + title: "Secret Bounty".into(), + amount: 100, + creator_did: owner.into(), + claimant_did: None, + claimant_wallet: None, + pr_id: None, + status: "open".into(), + created_at: "2026-01-01T00:00:00Z".into(), + claimed_at: None, + submitted_at: None, + completed_at: None, + deadline_secs: 86400, + tx_hash: None, + }; + state.db.create_bounty(&bounty).await.unwrap(); + + let router = crate::server::build_router(state); + let resp = router + .oneshot(anon_get("/api/v1/bounties/anon-private-bounty")) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn get_bounty_gate_admits_owner_on_private(pool: PgPool) { + let state = test_state(pool).await; + let kp = gitlawb_core::identity::Keypair::generate(); + let owner = kp.did().to_string(); + state + .db + .create_repo(&seed_private_repo(&owner, "secret-repo")) + .await + .unwrap(); + let bounty = crate::db::BountyRecord { + id: "owner-private-bounty".into(), + repo_owner: owner.clone(), + repo_name: "secret-repo".into(), + issue_id: None, + title: "Owner Bounty".into(), + amount: 200, + creator_did: owner.clone(), + claimant_did: None, + claimant_wallet: None, + pr_id: None, + status: "open".into(), + created_at: "2026-01-01T00:00:00Z".into(), + claimed_at: None, + submitted_at: None, + completed_at: None, + deadline_secs: 86400, + tx_hash: None, + }; + state.db.create_bounty(&bounty).await.unwrap(); + + let router = crate::server::build_router(state); + let uri = "/api/v1/bounties/owner-private-bounty"; + let sig = gitlawb_core::http_sig::sign_request(&kp, "GET", uri, b""); + let req = Request::builder() + .method(Method::GET) + .uri(uri) + .header("content-type", "application/json") + .header("content-digest", sig.content_digest) + .header("signature-input", sig.signature_input) + .header("signature", sig.signature) + .body(Body::empty()) + .unwrap(); + let resp = router.oneshot(req).await.unwrap(); + assert!(resp.status().is_success()); + } + + #[sqlx::test] + async fn list_all_bounties_filters_private_repos_for_anon(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zLSTALLBOUNTYOWNERAAAAAAAAAAAAAAAAAAAAAA"; + + // Private repo with a bounty (should be filtered out) + state + .db + .create_repo(&seed_private_repo(owner, "private-bounty-repo")) + .await + .unwrap(); + let private_bounty = crate::db::BountyRecord { + id: "private-bounty-1".into(), + repo_owner: owner.into(), + repo_name: "private-bounty-repo".into(), + issue_id: None, + title: "Private Bounty".into(), + amount: 100, + creator_did: owner.into(), + claimant_did: None, + claimant_wallet: None, + pr_id: None, + status: "open".into(), + created_at: "2026-01-01T00:00:00Z".into(), + claimed_at: None, + submitted_at: None, + completed_at: None, + deadline_secs: 86400, + tx_hash: None, + }; + state.db.create_bounty(&private_bounty).await.unwrap(); + + // Public repo with a bounty (should be visible to anon) + let mut public_repo = seed_private_repo(owner, "public-bounty-repo"); + public_repo.is_public = true; + state.db.create_repo(&public_repo).await.unwrap(); + let public_bounty = crate::db::BountyRecord { + id: "public-bounty-1".into(), + repo_owner: owner.into(), + repo_name: "public-bounty-repo".into(), + issue_id: None, + title: "Public Bounty".into(), + amount: 200, + creator_did: owner.into(), + claimant_did: None, + claimant_wallet: None, + pr_id: None, + status: "open".into(), + created_at: "2026-01-02T00:00:00Z".into(), + claimed_at: None, + submitted_at: None, + completed_at: None, + deadline_secs: 86400, + tx_hash: None, + }; + state.db.create_bounty(&public_bounty).await.unwrap(); + + let router = crate::server::build_router(state); + let resp = router.oneshot(anon_get("/api/v1/bounties")).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + let body: serde_json::Value = serde_json::from_slice( + &axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(), + ) + .unwrap(); + let bounties = body["bounties"].as_array().unwrap(); + assert_eq!(bounties.len(), 1, "anon should see only the public bounty"); + assert_eq!(bounties[0]["id"], "public-bounty-1"); + } + #[sqlx::test] async fn repo_gate_owner_bare_key_vs_full_did(pool: PgPool) { let state = test_state(pool).await; From 6fd29c92c7974583b5a1030c29b3612ebd463fe6 Mon Sep 17 00:00:00 2001 From: Gravirei Date: Tue, 7 Jul 2026 09:36:34 +0600 Subject: [PATCH 10/12] cargo update -p crossbeam-epoch: fix RUSTSEC-2026-0204 (invalid pointer dereference) --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7e46b0b9..d12daa43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2293,9 +2293,9 @@ dependencies = [ [[package]] name = "crossbeam-epoch" -version = "0.9.18" +version = "0.9.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b82ac4a3c2ca9c3460964f020e1402edd5753411d7737aa39c3714ad1b5420e" +checksum = "2d6914041f254d6e9176c01941b21115dcfb7089e55135a35411081bd106ef3f" dependencies = [ "crossbeam-utils", ] From 119bfb19e950e34116390b161c8d28325d6ee372 Mon Sep 17 00:00:00 2001 From: Gravirei Date: Tue, 7 Jul 2026 11:45:36 +0600 Subject: [PATCH 11/12] Fix review findings: correct authz memoization, page past hidden bounties, gate star/unstar --- crates/gitlawb-node/src/api/bounties.rs | 62 +++++--- crates/gitlawb-node/src/api/stars.rs | 16 +- crates/gitlawb-node/src/db/mod.rs | 7 +- crates/gitlawb-node/src/test_support.rs | 188 ++++++++++++++++++++++++ 4 files changed, 238 insertions(+), 35 deletions(-) diff --git a/crates/gitlawb-node/src/api/bounties.rs b/crates/gitlawb-node/src/api/bounties.rs index ad275fe0..97d11c63 100644 --- a/crates/gitlawb-node/src/api/bounties.rs +++ b/crates/gitlawb-node/src/api/bounties.rs @@ -125,7 +125,7 @@ pub async fn list_repo_bounties( let limit = q.limit.unwrap_or(50).clamp(1, MAX_BOUNTY_LIMIT); let bounties = state .db - .list_bounties(Some(&owner), Some(&repo), q.status.as_deref(), limit) + .list_bounties(Some(&owner), Some(&repo), q.status.as_deref(), limit, 0) .await?; Ok(Json(serde_json::json!({ "bounties": bounties }))) @@ -139,33 +139,49 @@ pub async fn list_all_bounties( ) -> Result> { let raw_limit = q.limit.unwrap_or(50); let limit = raw_limit.clamp(1, MAX_BOUNTY_LIMIT); - // Over-fetch to account for filtered-out private-repo bounties so the - // caller gets up to `limit` readable results even when newer rows are - // inaccessible. - let fetch = (limit * 5).min(1000); - let bounties = state - .db - .list_bounties(None, None, q.status.as_deref(), fetch) - .await?; let caller = auth.as_ref().map(|e| e.0 .0.as_str()); - let mut seen = std::collections::HashSet::new(); - let mut allowed = Vec::new(); - for b in &bounties { - let key = format!("{}/{}", b.repo_owner, b.repo_name); - if seen.contains(&key) { - allowed.push(b.clone()); - continue; + let mut memo = std::collections::HashMap::new(); + let mut allowed: Vec = Vec::new(); + let page_size = (limit * 5).clamp(1, 200); + let max_rows = 10_000i64; + let mut scanned: i64 = 0; + + while (allowed.len() as i64) < limit && scanned < max_rows { + let bounties = state + .db + .list_bounties(None, None, q.status.as_deref(), page_size, scanned) + .await?; + if bounties.is_empty() { + break; } - seen.insert(key.clone()); - if crate::api::authorize_repo_read(&state, &b.repo_owner, &b.repo_name, caller, "/") - .await - .is_ok() - { - allowed.push(b.clone()); + + for b in &bounties { + if (allowed.len() as i64) >= limit { + break; + } + let key = format!("{}/{}", b.repo_owner, b.repo_name); + let admitted = if let Some(&result) = memo.get(&key) { + result + } else { + let result = crate::api::authorize_repo_read( + &state, + &b.repo_owner, + &b.repo_name, + caller, + "/", + ) + .await + .is_ok(); + memo.insert(key, result); + result + }; + if admitted { + allowed.push(b.clone()); + } } + scanned += bounties.len() as i64; } - allowed.truncate(limit as usize); Ok(Json(serde_json::json!({ "bounties": allowed }))) } diff --git a/crates/gitlawb-node/src/api/stars.rs b/crates/gitlawb-node/src/api/stars.rs index 61e0630b..303b245c 100644 --- a/crates/gitlawb-node/src/api/stars.rs +++ b/crates/gitlawb-node/src/api/stars.rs @@ -9,7 +9,7 @@ use axum::http::StatusCode; use axum::Json; use crate::auth::AuthenticatedDid; -use crate::error::{AppError, Result}; +use crate::error::Result; use crate::state::AppState; /// PUT /api/v1/repos/:owner/:repo/star @@ -19,11 +19,8 @@ pub async fn star_repo( Extension(auth): Extension, Path((owner, repo)): Path<(String, String)>, ) -> Result<(StatusCode, Json)> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, Some(auth.0.as_str()), "/").await?; let caller = &auth.0; let inserted = state.db.star_repo(&record.id, caller).await?; @@ -54,11 +51,8 @@ pub async fn unstar_repo( Extension(auth): Extension, Path((owner, repo)): Path<(String, String)>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, Some(auth.0.as_str()), "/").await?; let caller = &auth.0; state.db.unstar_repo(&record.id, caller).await?; diff --git a/crates/gitlawb-node/src/db/mod.rs b/crates/gitlawb-node/src/db/mod.rs index 9d302b55..9b8730be 100644 --- a/crates/gitlawb-node/src/db/mod.rs +++ b/crates/gitlawb-node/src/db/mod.rs @@ -2935,6 +2935,7 @@ impl Db { repo_name: Option<&str>, status: Option<&str>, limit: i64, + offset: i64, ) -> Result> { let mut sql = String::from("SELECT * FROM bounties WHERE 1=1"); let mut binds: Vec = Vec::new(); @@ -2955,13 +2956,17 @@ impl Db { binds.push(s.to_string()); idx += 1; } - sql.push_str(&format!(" ORDER BY created_at DESC LIMIT ${idx}")); + sql.push_str(&format!( + " ORDER BY created_at DESC LIMIT ${idx} OFFSET ${}", + idx + 1 + )); let mut q = sqlx::query(&sql); for b in &binds { q = q.bind(b); } q = q.bind(limit); + q = q.bind(offset); let rows = q.fetch_all(&self.pool).await?; Ok(rows.iter().map(|r| self.bounty_from_row(r)).collect()) diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index e1da6856..c59338d0 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -3326,6 +3326,194 @@ mod tests { assert_eq!(bounties[0]["id"], "public-bounty-1"); } + #[sqlx::test] + async fn list_all_bounties_same_private_repo_two_bounties_anon_sees_none(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zP1SAME2PRIVBOUNTYOWNERAAAAAAAAAAAAAAAAA"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + for id in ["private-bounty-a", "private-bounty-b"] { + let b = crate::db::BountyRecord { + id: id.into(), + repo_owner: owner.into(), + repo_name: "secret-repo".into(), + issue_id: None, + title: "Private Bounty".into(), + amount: 100, + creator_did: owner.into(), + claimant_did: None, + claimant_wallet: None, + pr_id: None, + status: "open".into(), + created_at: "2026-01-01T00:00:00Z".into(), + claimed_at: None, + submitted_at: None, + completed_at: None, + deadline_secs: 86400, + tx_hash: None, + }; + state.db.create_bounty(&b).await.unwrap(); + } + + let router = crate::server::build_router(state); + let resp = router.oneshot(anon_get("/api/v1/bounties")).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + let body: serde_json::Value = serde_json::from_slice( + &axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(), + ) + .unwrap(); + let bounties = body["bounties"].as_array().unwrap(); + assert_eq!( + bounties.len(), + 0, + "anon should see 0 bounties from private repo even with 2 entries" + ); + } + + #[sqlx::test] + async fn list_all_bounties_past_private_window_finds_public(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zP2PASTPRIVWINDOWOWNERAAAAAAAAAAAAAAAAA"; + + // Seed a private repo with 6 bounties (more than one page of page_size=5) + state + .db + .create_repo(&seed_private_repo(owner, "private-repo")) + .await + .unwrap(); + for i in 0..6 { + let b = crate::db::BountyRecord { + id: format!("private-bounty-{i}"), + repo_owner: owner.into(), + repo_name: "private-repo".into(), + issue_id: None, + title: format!("Private Bounty {i}"), + amount: 100, + creator_did: owner.into(), + claimant_did: None, + claimant_wallet: None, + pr_id: None, + status: "open".into(), + created_at: format!("2026-01-{:02}T00:00:00Z", 6 - i), + claimed_at: None, + submitted_at: None, + completed_at: None, + deadline_secs: 86400, + tx_hash: None, + }; + state.db.create_bounty(&b).await.unwrap(); + } + + // Public repo with a bounty created after the private ones + let mut pub_repo = seed_private_repo(owner, "public-repo"); + pub_repo.is_public = true; + state.db.create_repo(&pub_repo).await.unwrap(); + let pub_bounty = crate::db::BountyRecord { + id: "public-bounty-past-window".into(), + repo_owner: owner.into(), + repo_name: "public-repo".into(), + issue_id: None, + title: "Public Bounty".into(), + amount: 200, + creator_did: owner.into(), + claimant_did: None, + claimant_wallet: None, + pr_id: None, + status: "open".into(), + // This is older (earlier date) so it appears after the private ones in DESC order + created_at: "2025-12-01T00:00:00Z".into(), + claimed_at: None, + submitted_at: None, + completed_at: None, + deadline_secs: 86400, + tx_hash: None, + }; + state.db.create_bounty(&pub_bounty).await.unwrap(); + + let router = crate::server::build_router(state); + let resp = router + .oneshot(anon_get("/api/v1/bounties?limit=1")) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + let body: serde_json::Value = serde_json::from_slice( + &axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(), + ) + .unwrap(); + let bounties = body["bounties"].as_array().unwrap(); + assert_eq!( + bounties.len(), + 1, + "anon should find the public bounty past the private window" + ); + assert_eq!(bounties[0]["id"], "public-bounty-past-window"); + } + + #[sqlx::test] + async fn star_repo_gate_denies_non_reader_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zSTARGATEDENYOWNERAAAAAAAAAAAAAAAAAAAAA"; + let short = owner.split(':').next_back().unwrap(); + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let non_owner_kp = gitlawb_core::identity::Keypair::generate(); + let uri = format!("/api/v1/repos/{short}/secret-repo/star"); + let sig = gitlawb_core::http_sig::sign_request(&non_owner_kp, "PUT", &uri, b""); + let req = Request::builder() + .method(Method::PUT) + .uri(&uri) + .header("content-type", "application/json") + .header("content-digest", sig.content_digest) + .header("signature-input", sig.signature_input) + .header("signature", sig.signature) + .body(Body::empty()) + .unwrap(); + + let router = crate::server::build_router(state); + let resp = router.oneshot(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn unstar_repo_gate_denies_non_reader_on_private(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zUNSTARGATEDENYOWNERAAAAAAAAAAAAAAAAAAA"; + let short = owner.split(':').next_back().unwrap(); + state + .db + .create_repo(&seed_private_repo(owner, "secret-repo")) + .await + .unwrap(); + + let non_owner_kp = gitlawb_core::identity::Keypair::generate(); + let uri = format!("/api/v1/repos/{short}/secret-repo/star"); + let sig = gitlawb_core::http_sig::sign_request(&non_owner_kp, "DELETE", &uri, b""); + let req = Request::builder() + .method(Method::DELETE) + .uri(&uri) + .header("content-digest", sig.content_digest) + .header("signature-input", sig.signature_input) + .header("signature", sig.signature) + .body(Body::empty()) + .unwrap(); + + let router = crate::server::build_router(state); + let resp = router.oneshot(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + #[sqlx::test] async fn repo_gate_owner_bare_key_vs_full_did(pool: PgPool) { let state = test_state(pool).await; From 0be5e42d505b976fe9ee320fa20ad20ab1e64f95 Mon Sep 17 00:00:00 2001 From: Gravirei Date: Tue, 7 Jul 2026 21:05:40 +0600 Subject: [PATCH 12/12] Fix findings: sign CLI/MCP reads, keyset pagination, gate claim_bounty --- crates/gitlawb-node/src/api/bounties.rs | 57 ++++++++++++++++++++----- crates/gitlawb-node/src/db/mod.rs | 17 +++++--- crates/gitlawb-node/src/error.rs | 6 +++ crates/gl/src/bounty.rs | 38 ++++++++++++----- crates/gl/src/cert.rs | 48 ++++++++++++++------- crates/gl/src/http.rs | 9 ++++ crates/gl/src/issue.rs | 16 ++++--- crates/gl/src/mcp.rs | 6 +-- crates/gl/src/repo.rs | 4 +- crates/gl/src/star.rs | 24 +++++++---- 10 files changed, 163 insertions(+), 62 deletions(-) diff --git a/crates/gitlawb-node/src/api/bounties.rs b/crates/gitlawb-node/src/api/bounties.rs index 97d11c63..68b98178 100644 --- a/crates/gitlawb-node/src/api/bounties.rs +++ b/crates/gitlawb-node/src/api/bounties.rs @@ -125,7 +125,14 @@ pub async fn list_repo_bounties( let limit = q.limit.unwrap_or(50).clamp(1, MAX_BOUNTY_LIMIT); let bounties = state .db - .list_bounties(Some(&owner), Some(&repo), q.status.as_deref(), limit, 0) + .list_bounties( + Some(&owner), + Some(&repo), + q.status.as_deref(), + limit, + None, + None, + ) .await?; Ok(Json(serde_json::json!({ "bounties": bounties }))) @@ -144,18 +151,35 @@ pub async fn list_all_bounties( let mut memo = std::collections::HashMap::new(); let mut allowed: Vec = Vec::new(); let page_size = (limit * 5).clamp(1, 200); - let max_rows = 10_000i64; - let mut scanned: i64 = 0; + let max_scanned = 10_000i64; + let mut total_scanned: i64 = 0; + let mut cursor: Option<(String, String)> = None; - while (allowed.len() as i64) < limit && scanned < max_rows { + while (allowed.len() as i64) < limit { + let after = cursor.as_ref().map(|(ts, id)| (ts.as_str(), id.as_str())); let bounties = state .db - .list_bounties(None, None, q.status.as_deref(), page_size, scanned) + .list_bounties( + None, + None, + q.status.as_deref(), + page_size, + after.map(|(ts, _)| ts), + after.map(|(_, id)| id), + ) .await?; if bounties.is_empty() { break; } + total_scanned += bounties.len() as i64; + if total_scanned > max_scanned { + return Err(AppError::Incomplete( + "too many rows scanned; refine your filters".into(), + )); + } + + let last = bounties.last().unwrap(); for b in &bounties { if (allowed.len() as i64) >= limit { break; @@ -180,7 +204,7 @@ pub async fn list_all_bounties( allowed.push(b.clone()); } } - scanned += bounties.len() as i64; + cursor = Some((last.created_at.clone(), last.id.clone())); } Ok(Json(serde_json::json!({ "bounties": allowed }))) @@ -214,11 +238,22 @@ pub async fn claim_bounty( Path(id): Path, Json(req): Json, ) -> Result> { - let bounty = state - .db - .get_bounty(&id) - .await? - .ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?; + let not_found = || AppError::NotFound(format!("bounty {id} not found")); + + let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?; + + if crate::api::authorize_repo_read( + &state, + &bounty.repo_owner, + &bounty.repo_name, + Some(auth.0.as_str()), + "/", + ) + .await + .is_err() + { + return Err(not_found()); + } if bounty.status != "open" { return Err(AppError::BadRequest(format!( diff --git a/crates/gitlawb-node/src/db/mod.rs b/crates/gitlawb-node/src/db/mod.rs index 9b8730be..25a85346 100644 --- a/crates/gitlawb-node/src/db/mod.rs +++ b/crates/gitlawb-node/src/db/mod.rs @@ -2935,7 +2935,8 @@ impl Db { repo_name: Option<&str>, status: Option<&str>, limit: i64, - offset: i64, + after_created_at: Option<&str>, + after_id: Option<&str>, ) -> Result> { let mut sql = String::from("SELECT * FROM bounties WHERE 1=1"); let mut binds: Vec = Vec::new(); @@ -2956,17 +2957,21 @@ impl Db { binds.push(s.to_string()); idx += 1; } - sql.push_str(&format!( - " ORDER BY created_at DESC LIMIT ${idx} OFFSET ${}", - idx + 1 - )); + if let Some(ts) = after_created_at { + let id = after_id.unwrap_or(""); + sql.push_str(&format!(" AND (created_at, id) < (${idx}, ${})", idx + 1)); + binds.push(ts.to_string()); + idx += 1; + binds.push(id.to_string()); + idx += 1; + } + sql.push_str(&format!(" ORDER BY created_at DESC, id DESC LIMIT ${idx}")); let mut q = sqlx::query(&sql); for b in &binds { q = q.bind(b); } q = q.bind(limit); - q = q.bind(offset); let rows = q.fetch_all(&self.pool).await?; Ok(rows.iter().map(|r| self.bounty_from_row(r)).collect()) diff --git a/crates/gitlawb-node/src/error.rs b/crates/gitlawb-node/src/error.rs index b9ffc2c3..aed680af 100644 --- a/crates/gitlawb-node/src/error.rs +++ b/crates/gitlawb-node/src/error.rs @@ -38,6 +38,9 @@ pub enum AppError { #[error("too many requests: {0}")] TooManyRequests(String), + #[error("incomplete: {0}")] + Incomplete(String), + #[error("git error: {0}")] Git(String), @@ -98,6 +101,9 @@ impl IntoResponse for AppError { AppError::TooManyRequests(msg) => { (StatusCode::TOO_MANY_REQUESTS, "rate_limited", msg.clone()) } + AppError::Incomplete(msg) => { + (StatusCode::UNPROCESSABLE_ENTITY, "incomplete", msg.clone()) + } AppError::Git(msg) => (StatusCode::INTERNAL_SERVER_ERROR, "git_error", msg.clone()), AppError::Db(e) => (StatusCode::INTERNAL_SERVER_ERROR, "db_error", e.to_string()), AppError::Internal(e) => ( diff --git a/crates/gl/src/bounty.rs b/crates/gl/src/bounty.rs index 7ee042eb..ccc658a1 100644 --- a/crates/gl/src/bounty.rs +++ b/crates/gl/src/bounty.rs @@ -8,6 +8,10 @@ use std::path::PathBuf; use crate::http::NodeClient; use crate::identity::load_keypair_from_dir; +fn signed_client(node: &str, dir: Option<&std::path::Path>) -> NodeClient { + NodeClient::new(node, load_keypair_from_dir(dir).ok()) +} + #[derive(Args)] pub struct BountyArgs { #[command(subcommand)] @@ -49,6 +53,8 @@ pub enum BountyCmd { status: Option, #[arg(long, default_value = "https://node.gitlawb.com", env = "GITLAWB_NODE")] node: String, + #[arg(long)] + dir: Option, }, /// Show details of a specific bounty Show { @@ -56,6 +62,8 @@ pub enum BountyCmd { id: String, #[arg(long, default_value = "https://node.gitlawb.com", env = "GITLAWB_NODE")] node: String, + #[arg(long)] + dir: Option, }, /// Claim an open bounty Claim { @@ -121,8 +129,13 @@ pub async fn run(args: BountyArgs) -> Result<()> { node, dir, } => cmd_create(repo, title, amount, issue, tx_hash, deadline, node, dir).await, - BountyCmd::List { repo, status, node } => cmd_list(repo, status, node).await, - BountyCmd::Show { id, node } => cmd_show(id, node).await, + BountyCmd::List { + repo, + status, + node, + dir, + } => cmd_list(repo, status, node, dir).await, + BountyCmd::Show { id, node, dir } => cmd_show(id, node, dir).await, BountyCmd::Claim { id, wallet, @@ -192,8 +205,13 @@ async fn cmd_create( Ok(()) } -async fn cmd_list(repo: Option, status: Option, node: String) -> Result<()> { - let client = NodeClient::new(&node, None); +async fn cmd_list( + repo: Option, + status: Option, + node: String, + dir: Option, +) -> Result<()> { + let client = signed_client(&node, dir.as_deref()); let url = if let Some(ref repo) = repo { let (owner, name) = repo @@ -214,7 +232,7 @@ async fn cmd_list(repo: Option, status: Option, node: String) -> }; let resp = client - .get(&url) + .get_authed(&url) .await .context("failed to connect to node")?; let body: Value = resp.json().await.unwrap_or_default(); @@ -237,9 +255,9 @@ async fn cmd_list(repo: Option, status: Option, node: String) -> Ok(()) } -async fn cmd_show(id: String, node: String) -> Result<()> { - let client = NodeClient::new(&node, None); - let resp = client.get(&format!("/api/v1/bounties/{id}")).await?; +async fn cmd_show(id: String, node: String, dir: Option) -> Result<()> { + let client = signed_client(&node, dir.as_deref()); + let resp = client.get_authed(&format!("/api/v1/bounties/{id}")).await?; let status = resp.status(); let body: Value = resp.json().await.unwrap_or_default(); @@ -462,7 +480,7 @@ mod tests { .create_async() .await; - cmd_list(None, None, server.url()).await.unwrap(); + cmd_list(None, None, server.url(), None).await.unwrap(); } #[tokio::test] @@ -506,7 +524,7 @@ mod tests { .create_async() .await; - let err = cmd_show("nonexistent".to_string(), server.url()) + let err = cmd_show("nonexistent".to_string(), server.url(), None) .await .unwrap_err(); assert!(err.to_string().contains("not found")); diff --git a/crates/gl/src/cert.rs b/crates/gl/src/cert.rs index 79402525..444d7316 100644 --- a/crates/gl/src/cert.rs +++ b/crates/gl/src/cert.rs @@ -5,10 +5,15 @@ use anyhow::{Context, Result}; use clap::{Args, Subcommand}; use serde_json::Value; +use std::path::PathBuf; use crate::http::NodeClient; use crate::identity::load_keypair_from_dir; +fn signed_client(node: &str, dir: Option<&std::path::Path>) -> NodeClient { + NodeClient::new(node, load_keypair_from_dir(dir).ok()) +} + #[derive(Args)] pub struct CertArgs { #[command(subcommand)] @@ -23,6 +28,8 @@ pub enum CertCmd { repo: String, #[arg(long, default_value = "https://node.gitlawb.com", env = "GITLAWB_NODE")] node: String, + #[arg(long)] + dir: Option, }, /// Show a specific ref certificate and verify its signature Show { @@ -32,28 +39,39 @@ pub enum CertCmd { id: String, #[arg(long, default_value = "https://node.gitlawb.com", env = "GITLAWB_NODE")] node: String, + #[arg(long)] + dir: Option, }, } pub async fn run(args: CertArgs) -> Result<()> { match args.cmd { - CertCmd::List { repo, node } => cmd_list(repo, node).await, - CertCmd::Show { repo, id, node } => cmd_show(repo, id, node).await, + CertCmd::List { repo, node, dir } => cmd_list(repo, node, dir).await, + CertCmd::Show { + repo, + id, + node, + dir, + } => cmd_show(repo, id, node, dir).await, } } /// Resolve "repo" into (owner, name) using the caller's DID when no slash is given. -async fn resolve_repo(repo: &str, node: &str) -> Result<(String, String)> { +async fn resolve_repo( + repo: &str, + node: &str, + dir: Option<&std::path::Path>, +) -> Result<(String, String)> { if let Some((owner, name)) = repo.split_once('/') { Ok((owner.to_string(), name.to_string())) } else { - let short = if let Ok(kp) = load_keypair_from_dir(None) { + let short = if let Ok(kp) = load_keypair_from_dir(dir) { let did = kp.did().to_string(); did.split(':').next_back().unwrap_or(&did).to_string() } else { - let client = NodeClient::new(node, None); + let client = signed_client(node, dir); let info: Value = client - .get("/") + .get_authed("/") .await? .json() .await @@ -65,13 +83,13 @@ async fn resolve_repo(repo: &str, node: &str) -> Result<(String, String)> { } } -async fn cmd_list(repo: String, node: String) -> Result<()> { - let (owner, name) = resolve_repo(&repo, &node).await?; +async fn cmd_list(repo: String, node: String, dir: Option) -> Result<()> { + let (owner, name) = resolve_repo(&repo, &node, dir.as_deref()).await?; - let client = NodeClient::new(&node, None); + let client = signed_client(&node, dir.as_deref()); let path = format!("/api/v1/repos/{owner}/{name}/certs"); let resp: Value = client - .get(&path) + .get_authed(&path) .await? .json() .await @@ -96,16 +114,16 @@ async fn cmd_list(repo: String, node: String) -> Result<()> { Ok(()) } -async fn cmd_show(repo: String, id: String, node: String) -> Result<()> { - let (owner, name) = resolve_repo(&repo, &node).await?; +async fn cmd_show(repo: String, id: String, node: String, dir: Option) -> Result<()> { + let (owner, name) = resolve_repo(&repo, &node, dir.as_deref()).await?; - let client = NodeClient::new(&node, None); + let client = signed_client(&node, dir.as_deref()); let id = resolve_cert_id(&client, &owner, &name, &id).await?; // Fetch the certificate let path = format!("/api/v1/repos/{owner}/{name}/certs/{id}"); let resp = client - .get(&path) + .get_authed(&path) .await? .error_for_status() .context("certificate not found")?; @@ -170,7 +188,7 @@ async fn resolve_cert_id(client: &NodeClient, owner: &str, name: &str, id: &str) let path = format!("/api/v1/repos/{owner}/{name}/certs"); let resp: Value = client - .get(&path) + .get_authed(&path) .await? .error_for_status() .context("failed to list certificates")? diff --git a/crates/gl/src/http.rs b/crates/gl/src/http.rs index 10372b2e..8cd7c97e 100644 --- a/crates/gl/src/http.rs +++ b/crates/gl/src/http.rs @@ -45,6 +45,15 @@ impl NodeClient { .with_context(|| format!("GET {url}")) } + /// GET that signs when a keypair is available; falls back to unsigned for public repos. + pub async fn get_authed(&self, path: &str) -> Result { + if self.keypair.is_some() { + self.get_signed(path).await + } else { + self.get(path).await + } + } + /// GET with RFC 9421 HTTP Signature auth, for owner-only read endpoints. /// Signs over the empty body (same shape the node verifies for signed reads). pub async fn get_signed(&self, path: &str) -> Result { diff --git a/crates/gl/src/issue.rs b/crates/gl/src/issue.rs index 9ce4e797..57bd6944 100644 --- a/crates/gl/src/issue.rs +++ b/crates/gl/src/issue.rs @@ -10,6 +10,10 @@ use uuid::Uuid; use crate::http::NodeClient; use crate::identity::load_keypair_from_dir; +fn signed_client(node: &str, dir: Option<&std::path::Path>) -> NodeClient { + NodeClient::new(node, load_keypair_from_dir(dir).ok()) +} + #[derive(Args)] pub struct IssueArgs { #[command(subcommand)] @@ -215,10 +219,10 @@ async fn cmd_create( async fn cmd_list(repo: String, node: String, dir: Option) -> Result<()> { let (owner, name) = resolve_repo(&repo, &node, dir.as_deref()).await?; - let client = NodeClient::new(&node, None); + let client = signed_client(&node, dir.as_deref()); let path = format!("/api/v1/repos/{owner}/{name}/issues"); let resp: Value = client - .get(&path) + .get_authed(&path) .await? .json() .await @@ -254,10 +258,10 @@ async fn cmd_list(repo: String, node: String, dir: Option) -> Result<() async fn cmd_show(repo: String, id: String, node: String, dir: Option) -> Result<()> { let (owner, name) = resolve_repo(&repo, &node, dir.as_deref()).await?; - let client = NodeClient::new(&node, None); + let client = signed_client(&node, dir.as_deref()); let path = format!("/api/v1/repos/{owner}/{name}/issues/{id}"); let resp = client - .get(&path) + .get_authed(&path) .await .context("failed to connect to node")?; let status = resp.status(); @@ -347,10 +351,10 @@ async fn cmd_issue_comments( dir: Option, ) -> Result<()> { let (owner, name) = resolve_repo(&repo, &node, dir.as_deref()).await?; - let client = NodeClient::new(&node, None); + let client = signed_client(&node, dir.as_deref()); let resp: Value = client - .get(&format!( + .get_authed(&format!( "/api/v1/repos/{owner}/{name}/issues/{id}/comments" )) .await? diff --git a/crates/gl/src/mcp.rs b/crates/gl/src/mcp.rs index 5a4b53ae..0668ac96 100644 --- a/crates/gl/src/mcp.rs +++ b/crates/gl/src/mcp.rs @@ -970,14 +970,14 @@ async fn call_tool( } u }; - let resp: Value = client.get(&url).await?.json().await?; + let resp: Value = client.get_authed(&url).await?.json().await?; Ok(serde_json::to_string_pretty(&resp)?) } "bounty_show" => { let id = args["id"].as_str().context("missing 'id'")?; let resp: Value = client - .get(&format!("/api/v1/bounties/{id}")) + .get_authed(&format!("/api/v1/bounties/{id}")) .await? .json() .await?; @@ -1165,7 +1165,7 @@ async fn call_tool( (owner, repo.to_string()) }; let resp: Value = client - .get(&format!("/api/v1/repos/{owner}/{name}/issues")) + .get_authed(&format!("/api/v1/repos/{owner}/{name}/issues")) .await? .json() .await?; diff --git a/crates/gl/src/repo.rs b/crates/gl/src/repo.rs index 585f75af..f6af7382 100644 --- a/crates/gl/src/repo.rs +++ b/crates/gl/src/repo.rs @@ -658,10 +658,10 @@ async fn cmd_label_remove( async fn cmd_label_list(repo: String, node: String, dir: Option) -> Result<()> { let (owner, name) = resolve_owner_repo_pair(&repo, &node, dir.as_deref()).await?; - let client = NodeClient::new(&node, None); + let client = NodeClient::new(&node, load_keypair_from_dir(dir.as_deref()).ok()); let resp: Value = client - .get(&format!("/api/v1/repos/{owner}/{name}/labels")) + .get_authed(&format!("/api/v1/repos/{owner}/{name}/labels")) .await? .json() .await diff --git a/crates/gl/src/star.rs b/crates/gl/src/star.rs index 4b7d8f91..120f824a 100644 --- a/crates/gl/src/star.rs +++ b/crates/gl/src/star.rs @@ -34,12 +34,14 @@ pub enum StarCmd { #[arg(long)] dir: Option, }, - /// Show star count for a repository (no auth required) + /// Show star count for a repository Count { /// Repository in owner/repo format repo: String, #[arg(long, default_value = "https://node.gitlawb.com", env = "GITLAWB_NODE")] node: String, + #[arg(long)] + dir: Option, }, } @@ -47,7 +49,7 @@ pub async fn run(args: StarArgs) -> Result<()> { match args.cmd { StarCmd::Add { repo, node, dir } => cmd_add(repo, node, dir).await, StarCmd::Remove { repo, node, dir } => cmd_remove(repo, node, dir).await, - StarCmd::Count { repo, node } => cmd_count(repo, node).await, + StarCmd::Count { repo, node, dir } => cmd_count(repo, node, dir).await, } } @@ -110,15 +112,15 @@ async fn cmd_remove(repo: String, node: String, dir: Option) -> Result< Ok(()) } -async fn cmd_count(repo: String, node: String) -> Result<()> { +async fn cmd_count(repo: String, node: String, dir: Option) -> Result<()> { let (owner, name) = repo .split_once('/') .map(|(o, n)| (o.to_string(), n.to_string())) .context("use owner/repo format for count (e.g. alice/myrepo)")?; - let client = NodeClient::new(&node, None); + let client = NodeClient::new(&node, load_keypair_from_dir(dir.as_deref()).ok()); let resp = client - .get(&format!("/api/v1/repos/{owner}/{name}/star")) + .get_authed(&format!("/api/v1/repos/{owner}/{name}/star")) .await .context("failed to connect to node")?; @@ -310,16 +312,20 @@ mod tests { .create_async() .await; - cmd_count("alice/myrepo".to_string(), server.url()) + cmd_count("alice/myrepo".to_string(), server.url(), None) .await .unwrap(); } #[tokio::test] async fn test_cmd_count_requires_slash() { - let err = cmd_count("noslash".to_string(), "http://127.0.0.1:1".to_string()) - .await - .unwrap_err(); + let err = cmd_count( + "noslash".to_string(), + "http://127.0.0.1:1".to_string(), + None, + ) + .await + .unwrap_err(); assert!(err.to_string().contains("owner/repo format")); }