From ee2b4d064e2b3ff1737cfe39440f1ab2d6d11b8d Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 28 Jun 2026 15:08:37 -0500 Subject: [PATCH 01/11] fix(node): owner-gate list_webhooks to stop leaking webhook URLs (#94) list_webhooks sat on the optional_signature read group with no auth or ownership check, so any unauthenticated caller who knew a repo's owner/name could enumerate its webhooks (target URL, created_by_did, events; only the secret was redacted). Gate it read-visibility-then-owner, mirroring create_webhook/delete_webhook: headerless -> 401, non-reader of a private repo -> 404 (existence hidden, uniform with the sibling read surfaces), non-owner of a public repo -> 403. Add a list_webhooks row to the authz_guard drift table so the gate can't silently regress. --- crates/gitlawb-node/src/api/mod.rs | 3 + crates/gitlawb-node/src/api/webhooks.rs | 24 +++- crates/gitlawb-node/src/test_support.rs | 159 ++++++++++++++++++++++++ 3 files changed, 181 insertions(+), 5 deletions(-) diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index 85335fd..d93b3a9 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -173,6 +173,9 @@ mod authz_guard { (pulls, "merge_pr", "require_repo_owner("), (webhooks, "create_webhook", "require_repo_owner("), (webhooks, "delete_webhook", "require_repo_owner("), + // list_webhooks layers authorize_repo_read (404 hides a private repo) + // before the owner gate; the require_repo_owner marker guards the 403. + (webhooks, "list_webhooks", "require_repo_owner("), (labels, "add_label", "require_repo_owner("), (labels, "remove_label", "require_repo_owner("), // Bucket A' — owner OR author (did_matches against the author) diff --git a/crates/gitlawb-node/src/api/webhooks.rs b/crates/gitlawb-node/src/api/webhooks.rs index 52ef00b..8fa16c5 100644 --- a/crates/gitlawb-node/src/api/webhooks.rs +++ b/crates/gitlawb-node/src/api/webhooks.rs @@ -73,12 +73,26 @@ pub async fn create_webhook( pub async fn list_webhooks( 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}")))?; + // This route sits on `optional_signature`, so the DID is optional. Webhook + // callback URLs are owner-secret config and there is no anonymous form, so a + // headerless caller is rejected before any lookup (401, which fires for an + // existing-private and an absent repo alike, so it leaks no existence). + let Some(Extension(AuthenticatedDid(caller))) = auth else { + return Err(AppError::Unauthorized( + "listing webhooks requires authentication".into(), + )); + }; + + // Read-visibility first, then owner. authorize_repo_read returns 404 on a + // visibility deny, so a non-reader of a private repo cannot tell it exists + // (uniform with the sibling read surfaces); require_repo_owner then yields + // 403 only for a non-owner of a public/readable repo, where existence is not + // secret. + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &name, Some(&caller), "/").await?; + crate::api::require_repo_owner(&record, &caller)?; let hooks = state.db.list_webhooks(&record.id).await?; // Redact secrets in list response diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index e4cb7ba..b3205aa 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -621,6 +621,165 @@ mod tests { ); } + /// #94: list_webhooks is gated read-visibility THEN owner. Webhook callback + /// URLs are owner-secret, so the listing must hide a private repo's existence + /// (404, uniform with the read-visibility siblings) and 403 a non-owner of a + /// public repo, while a headerless caller gets 401 (no anonymous form). Mounts + /// the handler directly (it sits on `optional_signature`, so the handler does + /// its own check) and seeds a real webhook so a leak would surface in the body. + #[sqlx::test] + async fn list_webhooks_is_owner_gated(pool: PgPool) { + let owner = "did:key:zHOOKOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let stranger = "did:key:zHOOKSTRANGERBBBBBBBBBBBBBBBBBBBBBBBBBB"; + let state = test_state(pool).await; + + let pub_repo = seed_repo(owner, "hook-pub"); + state + .db + .create_repo(&pub_repo) + .await + .expect("seed public repo"); + let mut priv_repo = seed_repo(owner, "hook-priv"); + priv_repo.is_public = false; + state + .db + .create_repo(&priv_repo) + .await + .expect("seed private repo"); + + let secret_url = "https://hooks.example.com/sekret-endpoint"; + for repo in [&pub_repo, &priv_repo] { + state + .db + .create_webhook(&crate::db::Webhook { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + url: secret_url.to_string(), + secret: Some("topsecret".to_string()), + events: vec!["*".to_string()], + created_by_did: owner.to_string(), + created_at: Utc::now().to_rfc3339(), + active: true, + }) + .await + .expect("seed webhook"); + } + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/hooks", + axum::routing::get(crate::api::webhooks::list_webhooks), + ) + .with_state(state.clone()) + }; + let body_text = |resp_body: &[u8]| String::from_utf8_lossy(resp_body).to_string(); + + // Owner on the public repo → 200, hook listed, secret redacted, url present. + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + &format!("/api/v1/repos/{owner}/hook-pub/hooks"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "owner must read its own hooks" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let txt = body_text(&bytes); + assert!( + txt.contains(secret_url), + "owner response must include the url" + ); + assert!(txt.contains("***"), "secret must stay redacted"); + assert!( + !txt.contains("topsecret"), + "the real secret must never appear" + ); + + // Non-owner of a PUBLIC repo → 403 (repo is public, existence not secret). + let resp = router() + .oneshot(signed_request_as( + stranger, + Method::GET, + &format!("/api/v1/repos/{owner}/hook-pub/hooks"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::FORBIDDEN, + "a non-owner of a public repo must be forbidden, not served" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + !body_text(&bytes).contains(secret_url), + "403 must not leak the url" + ); + + // Non-owner of a PRIVATE repo → 404 (existence hidden, uniform with siblings). + let resp = router() + .oneshot(signed_request_as( + stranger, + Method::GET, + &format!("/api/v1/repos/{owner}/hook-priv/hooks"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "a non-reader of a private repo must get 404, not 403/200" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + !body_text(&bytes).contains(secret_url), + "404 must not leak the url" + ); + + // Headerless (no AuthenticatedDid) → 401: a webhook listing has no anon form. + let resp = router() + .oneshot( + Request::builder() + .method(Method::GET) + .uri(format!("/api/v1/repos/{owner}/hook-pub/hooks")) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::UNAUTHORIZED, + "a headerless caller must get 401" + ); + + // Absent repo → 404. + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + &format!("/api/v1/repos/{owner}/does-not-exist/hooks"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND, "absent repo → 404"); + } + /// Issue #6 / jatmn finding 2: `/api/v1/stats` counts logical repos, not raw /// rows. With a mirror+canonical pair and a standalone repo present, the /// `repos` count is 2. From 965cdfbf1ea3ba11de58e89003a5f7ab4666440d Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 28 Jun 2026 15:11:29 -0500 Subject: [PATCH 02/11] fix(node): read-visibility-gate list_replicas for private repos (#94) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit list_replicas sat on optional_signature with no visibility check, so a private repo's replica URLs were enumerable by anyone who knew owner/name. Replica lists are a documented public mirror-discovery surface, so gate on read visibility (authorize_repo_read) rather than owner-only: a public repo stays anonymously listable, a private repo returns 404 to anyone who cannot read it. register_replica registers non-owner DIDs, and a replica operator is not a visibility reader, so a non-owner replica operator of a private repo gets 404 — the intended contract, pinned by the new test. Add the authz_guard row. --- crates/gitlawb-node/src/api/mod.rs | 3 + crates/gitlawb-node/src/api/replicas.rs | 13 +-- crates/gitlawb-node/src/test_support.rs | 126 ++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 6 deletions(-) diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index d93b3a9..2a99315 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -189,6 +189,9 @@ mod authz_guard { (issues, "create_issue", "authorize_repo_read("), (bounties, "create_bounty", "authorize_repo_read("), (repos, "fork_repo", "authorize_repo_read("), + // #94 sibling read surfaces: gate private-repo metadata on read + // visibility (public repos stay anonymous; private repos 404). + (replicas, "list_replicas", "authorize_repo_read("), // Bucket C — signer-self: the acting DID is matched/bound to auth.0 (tasks, "create_task", "did_matches("), (tasks, "claim_task", "did_matches("), diff --git a/crates/gitlawb-node/src/api/replicas.rs b/crates/gitlawb-node/src/api/replicas.rs index e578b12..ece82bf 100644 --- a/crates/gitlawb-node/src/api/replicas.rs +++ b/crates/gitlawb-node/src/api/replicas.rs @@ -113,16 +113,17 @@ pub async fn unregister_replica( } /// GET /api/v1/repos/:owner/:repo/replicas -/// Public — returns the list of replicas (DID + URL + registration timestamp). +/// Read-visibility-gated: a PUBLIC repo's replica list stays anonymously +/// listable (mirror-discovery), but a PRIVATE repo's replica URLs are hidden +/// (404) from anyone who cannot read the repo at its root. pub async fn list_replicas( 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 replicas = state.db.list_replicas(&record.id).await?; diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index b3205aa..2429ef8 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -780,6 +780,132 @@ mod tests { assert_eq!(resp.status(), StatusCode::NOT_FOUND, "absent repo → 404"); } + /// #94 sibling: list_replicas is read-visibility-gated. Replica lists are a + /// documented public mirror-discovery surface, so a PUBLIC repo stays + /// anonymously listable, but a PRIVATE repo must not leak its replica URLs. + /// register_replica registers NON-owner DIDs (it rejects the owner), and a + /// replica operator is not a visibility reader, so a non-owner replica + /// operator of a private repo gets 404 — the intended contract, pinned here. + #[sqlx::test] + async fn list_replicas_is_read_visibility_gated(pool: PgPool) { + let owner = "did:key:zREPLOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let replica_op = "did:key:zREPLOPERATORBBBBBBBBBBBBBBBBBBBBBBBBBB"; + let state = test_state(pool).await; + + let pub_repo = seed_repo(owner, "repl-pub"); + state + .db + .create_repo(&pub_repo) + .await + .expect("seed public repo"); + let mut priv_repo = seed_repo(owner, "repl-priv"); + priv_repo.is_public = false; + state + .db + .create_repo(&priv_repo) + .await + .expect("seed private repo"); + + let replica_url = "https://replica.example.com/mirror-endpoint"; + for repo in [&pub_repo, &priv_repo] { + state + .db + .register_replica(&repo.id, replica_op, replica_url) + .await + .expect("seed replica"); + } + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/replicas", + axum::routing::get(crate::api::replicas::list_replicas), + ) + .with_state(state.clone()) + }; + let leaks = |bytes: &[u8]| String::from_utf8_lossy(bytes).contains(replica_url); + let anon = |uri: String| { + Request::builder() + .method(Method::GET) + .uri(uri) + .body(Body::empty()) + .unwrap() + }; + + // Public repo, anonymous → 200, replicas listed (mirror-discovery preserved). + let resp = router() + .oneshot(anon(format!("/api/v1/repos/{owner}/repl-pub/replicas"))) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "public replica list stays anonymous" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + leaks(&bytes), + "public response must include the replica url" + ); + + // Private repo, anonymous → 404, no replica URL leaked. + let resp = router() + .oneshot(anon(format!("/api/v1/repos/{owner}/repl-priv/replicas"))) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "private replica list is hidden from anon" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!(!leaks(&bytes), "404 must not leak the replica url"); + + // Private repo, owner → 200. + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + &format!("/api/v1/repos/{owner}/repl-priv/replicas"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "owner reads its private replica list" + ); + + // Private repo, the non-owner replica operator → 404 (intended contract: + // a replica operator is not a visibility reader). + let resp = router() + .oneshot(signed_request_as( + replica_op, + Method::GET, + &format!("/api/v1/repos/{owner}/repl-priv/replicas"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "a non-owner replica operator of a private repo is not a reader" + ); + + // Absent repo → 404. + let resp = router() + .oneshot(anon(format!("/api/v1/repos/{owner}/no-such-repo/replicas"))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND, "absent repo → 404"); + } + /// Issue #6 / jatmn finding 2: `/api/v1/stats` counts logical repos, not raw /// rows. With a mirror+canonical pair and a standalone repo present, the /// `repos` count is 2. From 0cf8cb76e3c47fba7135f34bf3be50e1b67c2242 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 28 Jun 2026 15:13:50 -0500 Subject: [PATCH 03/11] fix(node): read-visibility-gate list_protected_branches (#94) list_protected_branches sat on optional_signature with no visibility check, leaking a private repo's branch names to any unauthenticated caller. Gate on read visibility (authorize_repo_read at the root, INV-2 root-listing exception): public repos stay anonymously listable, private repos 404 to a non-reader. Add the authz_guard row. --- crates/gitlawb-node/src/api/mod.rs | 1 + crates/gitlawb-node/src/api/protect.rs | 12 +-- crates/gitlawb-node/src/test_support.rs | 112 ++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 5 deletions(-) diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index 2a99315..8dbf9fb 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -192,6 +192,7 @@ mod authz_guard { // #94 sibling read surfaces: gate private-repo metadata on read // visibility (public repos stay anonymous; private repos 404). (replicas, "list_replicas", "authorize_repo_read("), + (protect, "list_protected_branches", "authorize_repo_read("), // Bucket C — signer-self: the acting DID is matched/bound to auth.0 (tasks, "create_task", "did_matches("), (tasks, "claim_task", "did_matches("), diff --git a/crates/gitlawb-node/src/api/protect.rs b/crates/gitlawb-node/src/api/protect.rs index 7f51617..4dd08b0 100644 --- a/crates/gitlawb-node/src/api/protect.rs +++ b/crates/gitlawb-node/src/api/protect.rs @@ -79,12 +79,14 @@ pub async fn unprotect_branch( pub async fn list_protected_branches( 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}")))?; + // Read-visibility-gated (INV-2 root listing): a public repo's protected + // branches stay anonymously listable; a private repo's branch names are + // hidden (404) from anyone who cannot read it at the root. + 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 branches = state.db.list_protected_branches(&record.id).await?; diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 2429ef8..c7b378c 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -906,6 +906,118 @@ mod tests { assert_eq!(resp.status(), StatusCode::NOT_FOUND, "absent repo → 404"); } + /// #94 sibling: list_protected_branches is read-visibility-gated. A public + /// repo's protected-branch listing stays anonymous; a private repo must not + /// leak its branch names to a non-reader (404, uniform no-existence-oracle). + #[sqlx::test] + async fn list_protected_branches_is_read_visibility_gated(pool: PgPool) { + let owner = "did:key:zPROTOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let stranger = "did:key:zPROTSTRANGERBBBBBBBBBBBBBBBBBBBBBBBBBB"; + let state = test_state(pool).await; + + let pub_repo = seed_repo(owner, "prot-pub"); + state + .db + .create_repo(&pub_repo) + .await + .expect("seed public repo"); + let mut priv_repo = seed_repo(owner, "prot-priv"); + priv_repo.is_public = false; + state + .db + .create_repo(&priv_repo) + .await + .expect("seed private repo"); + + let secret_branch = "release-embargoed"; + for repo in [&pub_repo, &priv_repo] { + state + .db + .protect_branch(&repo.id, secret_branch, owner) + .await + .expect("seed protected branch"); + } + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/branches/protected", + axum::routing::get(crate::api::protect::list_protected_branches), + ) + .with_state(state.clone()) + }; + let leaks = |bytes: &[u8]| String::from_utf8_lossy(bytes).contains(secret_branch); + let anon = |uri: String| { + Request::builder() + .method(Method::GET) + .uri(uri) + .body(Body::empty()) + .unwrap() + }; + + // Public repo, anonymous → 200, branch listed. + let resp = router() + .oneshot(anon(format!( + "/api/v1/repos/{owner}/prot-pub/branches/protected" + ))) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "public protected-branch list stays anonymous" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + leaks(&bytes), + "public response must include the branch name" + ); + + // Private repo, anonymous → 404, no branch name leaked. + let resp = router() + .oneshot(anon(format!( + "/api/v1/repos/{owner}/prot-priv/branches/protected" + ))) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "private branch list hidden from anon" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!(!leaks(&bytes), "404 must not leak the branch name"); + + // Private repo, owner → 200, branch listed. + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + &format!("/api/v1/repos/{owner}/prot-priv/branches/protected"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "owner reads its private protected branches" + ); + + // Absent repo → 404. + let resp = router() + .oneshot(anon(format!( + "/api/v1/repos/{owner}/no-such-repo/branches/protected" + ))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND, "absent repo → 404"); + } + /// Issue #6 / jatmn finding 2: `/api/v1/stats` counts logical repos, not raw /// rows. With a mirror+canonical pair and a standalone repo present, the /// `repos` count is 2. From 1a722e18f318ca3671e934bc2448b16d7e916917 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 28 Jun 2026 15:20:12 -0500 Subject: [PATCH 04/11] fix(node): gate list_repo_events for local private repos, keep gossip-only (#94) list_repo_events served a private repo's ref certificates and gossip ref metadata (ref names, SHAs) to any unauthenticated caller. The handler also tolerates a repo it knows only via gossip (no local row) and legitimately serves its events, so a blanket get_repo->authorize_repo_read swap would have 404'd that federation path. Gate only the locally-hosted branch: when get_repo returns a record, run visibility_check on it at the root and 404 on deny before fetching either the cert or gossip events. A gossip-only repo (no local row) keeps its existing public federation-feed behavior; the global /api/v1/events/ref-updates feed remains a separately tracked deferral. Add the authz_guard row (visibility_check marker, since the gate is direct to avoid a second get_repo). --- crates/gitlawb-node/src/api/events.rs | 25 ++- crates/gitlawb-node/src/api/mod.rs | 5 + crates/gitlawb-node/src/test_support.rs | 203 +++++++++++++++++++++++- 3 files changed, 230 insertions(+), 3 deletions(-) diff --git a/crates/gitlawb-node/src/api/events.rs b/crates/gitlawb-node/src/api/events.rs index 45db8a0..4a1972a 100644 --- a/crates/gitlawb-node/src/api/events.rs +++ b/crates/gitlawb-node/src/api/events.rs @@ -2,11 +2,13 @@ use std::collections::HashMap; -use axum::extract::{Path, Query, State}; +use axum::extract::{Extension, Path, Query, State}; use axum::Json; -use crate::error::Result; +use crate::auth::AuthenticatedDid; +use crate::error::{AppError, Result}; use crate::state::AppState; +use crate::visibility::{visibility_check, Decision}; /// GET /api/v1/events/ref-updates?limit=50 pub async fn list_ref_updates( @@ -50,6 +52,7 @@ pub async fn list_repo_events( State(state): State, Path((owner, repo_name)): Path<(String, String)>, Query(params): Query>, + auth: Option>, ) -> Result> { let limit = params .get("limit") @@ -60,6 +63,24 @@ pub async fn list_repo_events( // Look up the repo record once so we can use the full owner DID let repo_record = state.db.get_repo(&owner, &repo_name).await.ok().flatten(); + // #94: if this node hosts the repo locally, gate on read visibility BEFORE + // serving any events (cert OR gossip). A non-reader of a local private repo + // gets 404, hiding both its existence and its ref metadata. A repo the node + // knows only via gossip (no local row) has no local visibility rules to + // consult and keeps its existing public federation-feed behavior — the + // None branch is intentionally left ungated (tracked with the global + // /api/v1/events/ref-updates deferral). visibility_check on the loaded record + // avoids a second get_repo (mirrors api/encrypted.rs). + if let Some(ref record) = repo_record { + let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + let rules = state.db.list_visibility_rules(&record.id).await?; + if visibility_check(&rules, record.is_public, &record.owner_did, caller, "/") + == Decision::Deny + { + return Err(AppError::RepoNotFound(format!("{owner}/{repo_name}"))); + } + } + // Build the repo identifier using the FULL DID key part (not the 8-char URL truncation). // Gossip events are stored as "{full_key_part}/{repo_name}" (e.g. "z6MksXZDfullkeyhere/myrepo"), // but the URL only carries the first 8 chars of the key. Without the full slug the diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index 8dbf9fb..e287547 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -159,6 +159,7 @@ mod authz_guard { let issues = include_str!("issues.rs"); let bounties = include_str!("bounties.rs"); let replicas = include_str!("replicas.rs"); + let events = include_str!("events.rs"); let tasks = include_str!("tasks.rs"); let stars = include_str!("stars.rs"); let protect = include_str!("protect.rs"); @@ -193,6 +194,10 @@ mod authz_guard { // visibility (public repos stay anonymous; private repos 404). (replicas, "list_replicas", "authorize_repo_read("), (protect, "list_protected_branches", "authorize_repo_read("), + // list_repo_events gates only the locally-hosted branch, so it calls + // visibility_check directly (no second get_repo) rather than + // authorize_repo_read; the gossip-only None path stays ungated. + (events, "list_repo_events", "visibility_check("), // Bucket C — signer-self: the acting DID is matched/bound to auth.0 (tasks, "create_task", "did_matches("), (tasks, "claim_task", "did_matches("), diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index c7b378c..e90a1a8 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -912,7 +912,6 @@ mod tests { #[sqlx::test] async fn list_protected_branches_is_read_visibility_gated(pool: PgPool) { let owner = "did:key:zPROTOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; - let stranger = "did:key:zPROTSTRANGERBBBBBBBBBBBBBBBBBBBBBBBBBB"; let state = test_state(pool).await; let pub_repo = seed_repo(owner, "prot-pub"); @@ -1018,6 +1017,208 @@ mod tests { assert_eq!(resp.status(), StatusCode::NOT_FOUND, "absent repo → 404"); } + /// #94 sibling: list_repo_events gates a LOCALLY-HOSTED private repo (both its + /// ref certificates and its gossip ref-updates → 404) without breaking a repo + /// the node knows only via gossip (no local row), which legitimately 200s. + #[sqlx::test] + async fn list_repo_events_gates_local_private_but_not_gossip_only(pool: PgPool) { + use crate::db::{ReceivedRefUpdate, RefCertificate}; + let owner = "did:key:zEVTOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let keypart = owner.split(':').next_back().unwrap(); + let state = test_state(pool).await; + + let pub_repo = seed_repo(owner, "evt-pub"); + state + .db + .create_repo(&pub_repo) + .await + .expect("seed public repo"); + let mut priv_repo = seed_repo(owner, "evt-priv"); + priv_repo.is_public = false; + state + .db + .create_repo(&priv_repo) + .await + .expect("seed private repo"); + + let seed_cert = |repo_id: &str, ref_name: &str, sha: &str| RefCertificate { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo_id.to_string(), + ref_name: ref_name.to_string(), + old_sha: "0".repeat(40), + new_sha: sha.to_string(), + pusher_did: owner.to_string(), + node_did: owner.to_string(), + signature: "sig".to_string(), + issued_at: Utc::now().to_rfc3339(), + }; + let seed_update = |slug: &str, ref_name: &str, sha: &str| ReceivedRefUpdate { + id: uuid::Uuid::new_v4().to_string(), + node_did: owner.to_string(), + pusher_did: owner.to_string(), + repo: slug.to_string(), + ref_name: ref_name.to_string(), + old_sha: "0".repeat(40), + new_sha: sha.to_string(), + timestamp: Utc::now().to_rfc3339(), + cert_id: None, + received_at: Utc::now().to_rfc3339(), + from_peer: "peer".to_string(), + }; + + // Public local repo: a visible cert. + state + .db + .insert_ref_certificate(&seed_cert( + &pub_repo.id, + "refs/heads/public-main", + "pubsha00", + )) + .await + .expect("seed public cert"); + // Private local repo: a secret cert AND a secret gossip update (slug uses + // the full owner-DID key part, as the handler computes for a local repo). + state + .db + .insert_ref_certificate(&seed_cert( + &priv_repo.id, + "refs/heads/embargo-cert", + "certSEKRET", + )) + .await + .expect("seed private cert"); + state + .db + .insert_ref_update(&seed_update( + &format!("{keypart}/evt-priv"), + "refs/heads/embargo-gossip", + "gossipSEKRET", + )) + .await + .expect("seed private gossip update"); + // Gossip-only repo: no local row; slug uses the URL owner segment. + state + .db + .insert_ref_update(&seed_update( + "zGHOSTOWNER/ghost-repo", + "refs/heads/ghost", + "ghostsha0", + )) + .await + .expect("seed gossip-only update"); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/events", + axum::routing::get(crate::api::events::list_repo_events), + ) + .with_state(state.clone()) + }; + let anon = |uri: String| { + Request::builder() + .method(Method::GET) + .uri(uri) + .body(Body::empty()) + .unwrap() + }; + let text = |bytes: &[u8]| String::from_utf8_lossy(bytes).to_string(); + + // Characterization 1: public local repo, anonymous → 200, cert present. + let resp = router() + .oneshot(anon(format!("/api/v1/repos/{owner}/evt-pub/events"))) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "public events stay anonymous" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + text(&bytes).contains("public-main"), + "public cert must be listed" + ); + + // Characterization 2: gossip-only repo (no local row), anonymous → 200, + // gossip event present. This is the None path the gate must not break. + let resp = router() + .oneshot(anon( + "/api/v1/repos/zGHOSTOWNER/ghost-repo/events".to_string(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "gossip-only repo still serves events" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + text(&bytes).contains("ghost"), + "gossip-only event must be served" + ); + + // The leak fix: locally-hosted PRIVATE repo, anonymous → 404, and neither + // the cert nor the gossip secret appears in the body. + let resp = router() + .oneshot(anon(format!("/api/v1/repos/{owner}/evt-priv/events"))) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "a non-reader of a local private repo must get 404" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let body = text(&bytes); + assert!( + !body.contains("embargo-cert"), + "404 must not leak the cert ref" + ); + assert!( + !body.contains("certSEKRET"), + "404 must not leak the cert sha" + ); + assert!( + !body.contains("embargo-gossip"), + "404 must not leak the gossip ref" + ); + assert!( + !body.contains("gossipSEKRET"), + "404 must not leak the gossip sha" + ); + + // Owner of the private repo → 200, events present. + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + &format!("/api/v1/repos/{owner}/evt-priv/events"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "owner reads its private events" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + text(&bytes).contains("embargo-cert"), + "owner sees the private cert" + ); + } + /// Issue #6 / jatmn finding 2: `/api/v1/stats` counts logical repos, not raw /// rows. With a mirror+canonical pair and a standalone repo present, the /// `repos` count is 2. From a6840ed3b25a617c6cea470ab7dd8cb996236075 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 28 Jun 2026 15:28:17 -0500 Subject: [PATCH 05/11] fix(gl): sign the webhook list request now that GET /hooks is owner-gated (#94) `gl webhook list` built its NodeClient with no keypair, so the GET went out anonymously. With the node now owner-gating GET /hooks, that request returns 401. Thread a --dir through the List subcommand and load the keypair, matching `gl webhook create`/`delete`, so the listing is signed as the owner. (Replica and protected-branch listings stay anonymous for public repos under read-visibility gating, so their gl clients need no change.) --- crates/gl/src/webhook.rs | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/crates/gl/src/webhook.rs b/crates/gl/src/webhook.rs index 45b80ec..a8b95da 100644 --- a/crates/gl/src/webhook.rs +++ b/crates/gl/src/webhook.rs @@ -42,6 +42,8 @@ pub enum WebhookCmd { repo: String, #[arg(long, default_value = "https://node.gitlawb.com", env = "GITLAWB_NODE")] node: String, + #[arg(long)] + dir: Option, }, /// Delete a webhook Delete { @@ -66,7 +68,7 @@ pub async fn run(args: WebhookArgs) -> Result<()> { node, dir, } => cmd_create(repo, url, events, secret, node, dir).await, - WebhookCmd::List { repo, node } => cmd_list(repo, node).await, + WebhookCmd::List { repo, node, dir } => cmd_list(repo, node, dir).await, WebhookCmd::Delete { repo, id, @@ -143,8 +145,11 @@ async fn cmd_create( Ok(()) } -async fn cmd_list(repo: String, node: String) -> Result<()> { - let client = NodeClient::new(&node, None); +async fn cmd_list(repo: String, node: String, dir: Option) -> Result<()> { + // The node owner-gates GET /hooks (callback URLs are owner-secret), so the + // list request must be signed — anonymous callers get 401. + let keypair = load_keypair_from_dir(dir.as_deref())?; + let client = NodeClient::new(&node, Some(keypair)); let owner = resolve_owner(&client).await?; let resp: Value = client @@ -337,6 +342,7 @@ mod tests { #[tokio::test] async fn test_list_webhooks_empty() { let mut server = mockito::Server::new_async().await; + let (dir, _kp) = tmp_identity(); let _root = mock_root(&mut server).await; let _m = server @@ -347,12 +353,19 @@ mod tests { .create_async() .await; - cmd_list("my-repo".to_string(), server.url()).await.unwrap(); + cmd_list( + "my-repo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap(); } #[tokio::test] async fn test_list_webhooks_with_items() { let mut server = mockito::Server::new_async().await; + let (dir, _kp) = tmp_identity(); let _root = mock_root(&mut server).await; let _m = server @@ -363,7 +376,13 @@ mod tests { .create_async() .await; - cmd_list("my-repo".to_string(), server.url()).await.unwrap(); + cmd_list( + "my-repo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap(); } // ── delete ─────────────────────────────────────────────────────── From 464fc088ce95f4a48330cba1c00844a20e7ba8d6 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 28 Jun 2026 15:42:43 -0500 Subject: [PATCH 06/11] fix(review): sign the webhook list GET (get_signed), not plain get (#94) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review caught that NodeClient::get() never attaches the RFC 9421 signature even when a keypair is loaded — only get_signed/post/put/delete sign. The earlier gl webhook-list fix loaded the keypair but still called get(), so `gl webhook list` (and the MCP webhook_list tool) 401'd against the now-owner-gated endpoint. Switch both to get_signed. Harden the two cmd_list tests to require a Signature/Signature-Input header on the mock (verified: they fail against plain get(), pass with get_signed) — the missing assertion that let the bug through. Add the untested owner-of-private- repo webhooks path (200 + redaction). Fix the stale module doc and the authz_guard bucket comment for list_webhooks. --- crates/gitlawb-node/src/api/mod.rs | 6 +++-- crates/gitlawb-node/src/api/webhooks.rs | 6 ++--- crates/gitlawb-node/src/test_support.rs | 30 +++++++++++++++++++++++++ crates/gl/src/mcp.rs | 3 ++- crates/gl/src/webhook.rs | 10 ++++++++- 5 files changed, 48 insertions(+), 7 deletions(-) diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index e287547..6a0a277 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -174,8 +174,10 @@ mod authz_guard { (pulls, "merge_pr", "require_repo_owner("), (webhooks, "create_webhook", "require_repo_owner("), (webhooks, "delete_webhook", "require_repo_owner("), - // list_webhooks layers authorize_repo_read (404 hides a private repo) - // before the owner gate; the require_repo_owner marker guards the 403. + // Bucket A/B hybrid — list_webhooks is read-visibility THEN owner: + // authorize_repo_read 404s a non-reader of a private repo, then + // require_repo_owner 403s a non-owner of a public one. The + // require_repo_owner marker guards the owner half. (webhooks, "list_webhooks", "require_repo_owner("), (labels, "add_label", "require_repo_owner("), (labels, "remove_label", "require_repo_owner("), diff --git a/crates/gitlawb-node/src/api/webhooks.rs b/crates/gitlawb-node/src/api/webhooks.rs index 8fa16c5..d6a6997 100644 --- a/crates/gitlawb-node/src/api/webhooks.rs +++ b/crates/gitlawb-node/src/api/webhooks.rs @@ -1,8 +1,8 @@ //! Webhook CRUD API. //! -//! POST /api/v1/repos/:owner/:repo/hooks — create (auth required) -//! GET /api/v1/repos/:owner/:repo/hooks — list -//! DELETE /api/v1/repos/:owner/:repo/hooks/:id — delete (auth required) +//! POST /api/v1/repos/:owner/:repo/hooks — create (owner only) +//! GET /api/v1/repos/:owner/:repo/hooks — list (owner only; auth required) +//! DELETE /api/v1/repos/:owner/:repo/hooks/:id — delete (owner only) use axum::extract::{Extension, Path, State}; use axum::http::StatusCode; diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index e90a1a8..1b467d0 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -750,6 +750,36 @@ mod tests { "404 must not leak the url" ); + // Owner of a PRIVATE repo → 200 (both guards pass: read-visibility admits + // the owner, then require_repo_owner admits the owner). Exercises the + // both-pass branch the public/owner case does not, and confirms redaction. + let resp = router() + .oneshot(signed_request_as( + owner, + Method::GET, + &format!("/api/v1/repos/{owner}/hook-priv/hooks"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "the owner must read its own private repo's hooks" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let txt = body_text(&bytes); + assert!( + txt.contains(secret_url), + "owner of private repo sees the url" + ); + assert!( + txt.contains("***"), + "secret stays redacted on the private repo" + ); + // Headerless (no AuthenticatedDid) → 401: a webhook listing has no anon form. let resp = router() .oneshot( diff --git a/crates/gl/src/mcp.rs b/crates/gl/src/mcp.rs index 5a4b53a..0916539 100644 --- a/crates/gl/src/mcp.rs +++ b/crates/gl/src/mcp.rs @@ -933,8 +933,9 @@ async fn call_tool( "webhook_list" => { let repo = args["repo"].as_str().context("missing 'repo'")?; let owner = resolve_owner(&args, &client).await?; + // Owner-gated route: must be signed (get_signed), not a plain get(). let resp: Value = client - .get(&format!("/api/v1/repos/{owner}/{repo}/hooks")) + .get_signed(&format!("/api/v1/repos/{owner}/{repo}/hooks")) .await? .json() .await?; diff --git a/crates/gl/src/webhook.rs b/crates/gl/src/webhook.rs index a8b95da..648f67b 100644 --- a/crates/gl/src/webhook.rs +++ b/crates/gl/src/webhook.rs @@ -152,8 +152,10 @@ async fn cmd_list(repo: String, node: String, dir: Option) -> Result<() let client = NodeClient::new(&node, Some(keypair)); let owner = resolve_owner(&client).await?; + // get_signed (not get) attaches the RFC 9421 signature — plain get() never + // signs, and the node owner-gates this route, so an unsigned GET 401s. let resp: Value = client - .get(&format!("/api/v1/repos/{owner}/{repo}/hooks")) + .get_signed(&format!("/api/v1/repos/{owner}/{repo}/hooks")) .await? .json() .await @@ -347,6 +349,10 @@ mod tests { let _m = server .mock("GET", mockito::Matcher::Regex(r"/hooks$".to_string())) + // The route is owner-gated, so the list request must be signed. + // Requiring the header here is what catches a regression to plain get(). + .match_header("signature", mockito::Matcher::Any) + .match_header("signature-input", mockito::Matcher::Any) .with_status(200) .with_header("content-type", "application/json") .with_body(r#"{"webhooks":[]}"#) @@ -370,6 +376,8 @@ mod tests { let _m = server .mock("GET", mockito::Matcher::Regex(r"/hooks$".to_string())) + .match_header("signature", mockito::Matcher::Any) + .match_header("signature-input", mockito::Matcher::Any) .with_status(200) .with_header("content-type", "application/json") .with_body(r#"{"webhooks":[{"id":"h1","url":"https://a.com","events":["push"],"active":true},{"id":"h2","url":"https://b.com","events":["*"],"active":false}]}"#) From 9f31daa6e343097b9d9fe9bbeff08f02cdb3ce72 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 28 Jun 2026 15:58:32 -0500 Subject: [PATCH 07/11] fix(gl): sign read-visibility GETs so private-repo owners can read their own repos (#94) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #94 server change read-visibility-gates GET /replicas and /branches/protected: public repos stay anonymous, private repos require the owner/reader to authenticate. Four gl read commands sent unsigned GETs and so broke (404) or silently degraded (empty output) for a private repo's owner: gl protect list, gl repo owner (silent empty protected list), gl repo info (silent missing replica count), gl repo replicas (and it had no --dir flag). Add NodeClient::get_maybe_signed — signs when an identity is present, stays anonymous otherwise (mirrors the conditional signing of post/put/delete) — and route the four call sites through it. Add --dir to `gl repo replicas`. Harden the protect-list tests to assert the request is signed (verified: they fail on plain get()). Public-repo anonymous reads are unchanged. --- crates/gl/src/http.rs | 17 +++++++++++++++++ crates/gl/src/protect.rs | 11 +++++++++-- crates/gl/src/repo.rs | 27 ++++++++++++++++++--------- 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/crates/gl/src/http.rs b/crates/gl/src/http.rs index 32e90a1..1eabd0a 100644 --- a/crates/gl/src/http.rs +++ b/crates/gl/src/http.rs @@ -52,6 +52,23 @@ impl NodeClient { req.send().await.with_context(|| format!("GET {url}")) } + /// GET that signs when an identity keypair is present and falls back to an + /// anonymous GET otherwise — for read-visibility endpoints, where a public + /// repo is readable anonymously but a private repo requires the owner/reader + /// to be authenticated. Mirrors the conditional signing of post/put/delete. + pub async fn get_maybe_signed(&self, path: &str) -> Result { + let url = format!("{}{}", self.node_url, path); + let mut req = self.inner.get(&url); + if let Some(kp) = &self.keypair { + let signed = sign_request(kp, "GET", path, b""); + req = req + .header("Content-Digest", signed.content_digest) + .header("Signature-Input", signed.signature_input) + .header("Signature", signed.signature); + } + req.send().await.with_context(|| format!("GET {url}")) + } + /// POST with JSON body + RFC 9421 HTTP Signature auth. pub async fn post(&self, path: &str, body: &[u8]) -> Result { let url = format!("{}{}", self.node_url, path); diff --git a/crates/gl/src/protect.rs b/crates/gl/src/protect.rs index cde0dd6..a45a5eb 100644 --- a/crates/gl/src/protect.rs +++ b/crates/gl/src/protect.rs @@ -154,10 +154,12 @@ async fn cmd_remove( async fn cmd_list(repo: String, node: String, dir: Option) -> Result<()> { let (owner, name) = resolve_owner_repo(&repo, &node, dir.as_deref()).await?; - let client = NodeClient::new(&node, None); + // Read-visibility-gated on the node: public repos list anonymously, private + // repos need the owner's signature. Sign when an identity is available. + let client = NodeClient::new(&node, load_keypair_from_dir(dir.as_deref()).ok()); let resp = client - .get(&format!("/api/v1/repos/{owner}/{name}/branches/protected")) + .get_maybe_signed(&format!("/api/v1/repos/{owner}/{name}/branches/protected")) .await .context("failed to connect to node")?; @@ -307,6 +309,9 @@ mod tests { "GET", mockito::Matcher::Regex(r"branches/protected".to_string()), ) + // An identity is supplied, so get_maybe_signed must sign the request. + .match_header("signature", mockito::Matcher::Any) + .match_header("signature-input", mockito::Matcher::Any) .with_status(200) .with_header("content-type", "application/json") .with_body(r#"{"protected_branches":[],"count":0}"#) @@ -338,6 +343,8 @@ mod tests { "GET", mockito::Matcher::Regex(r"branches/protected".to_string()), ) + .match_header("signature", mockito::Matcher::Any) + .match_header("signature-input", mockito::Matcher::Any) .with_status(200) .with_header("content-type", "application/json") .with_body(r#"{"protected_branches":["main","release"],"count":2}"#) diff --git a/crates/gl/src/repo.rs b/crates/gl/src/repo.rs index 455a55f..d27dc96 100644 --- a/crates/gl/src/repo.rs +++ b/crates/gl/src/repo.rs @@ -146,6 +146,8 @@ pub enum RepoCmd { repo: String, #[arg(long, default_value = "https://node.gitlawb.com", env = "GITLAWB_NODE")] node: String, + #[arg(long)] + dir: Option, }, } @@ -203,7 +205,7 @@ pub async fn run(args: RepoArgs) -> Result<()> { RepoCmd::ReplicaUnregister { repo, node, dir } => { cmd_replica_unregister(repo, node, dir).await } - RepoCmd::Replicas { repo, node } => cmd_replicas(repo, node).await, + RepoCmd::Replicas { repo, node, dir } => cmd_replicas(repo, node, dir).await, } } @@ -330,7 +332,10 @@ async fn cmd_clone(name: String, node: String, dir: Option) -> Result<( } async fn cmd_info(repo: String, node: String, dir: Option) -> Result<()> { - let client = NodeClient::new(&node, None); + // Sign when an identity is available so the read-visibility-gated replica + // sub-fetch below resolves for a private repo's owner (public repos still + // work anonymously). + let client = NodeClient::new(&node, load_keypair_from_dir(dir.as_deref()).ok()); let (owner, name) = if repo.contains('/') { let (o, n) = repo.split_once('/').unwrap(); @@ -368,7 +373,7 @@ async fn cmd_info(repo: String, node: String, dir: Option) -> Result<() // Replica count — failure to fetch is non-fatal (older nodes don't expose this). if let Ok(resp) = client - .get(&format!("/api/v1/repos/{owner}/{name}/replicas")) + .get_maybe_signed(&format!("/api/v1/repos/{owner}/{name}/replicas")) .await { if resp.status().is_success() { @@ -448,15 +453,17 @@ async fn cmd_replica_unregister(repo: String, node: String, dir: Option Ok(()) } -async fn cmd_replicas(repo: String, node: String) -> Result<()> { +async fn cmd_replicas(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")?; - let client = NodeClient::new(&node, None); + // Read-visibility-gated: public repos list anonymously, private repos need + // the owner's signature. Sign when an identity is available. + let client = NodeClient::new(&node, load_keypair_from_dir(dir.as_deref()).ok()); let resp = client - .get(&format!("/api/v1/repos/{owner}/{name}/replicas")) + .get_maybe_signed(&format!("/api/v1/repos/{owner}/{name}/replicas")) .await .context("failed to connect to node")?; let status = resp.status(); @@ -674,7 +681,9 @@ async fn cmd_owner(repo: String, node: String, dir: Option, json_out: b let my_short = my_did.split(':').next_back().unwrap_or(&my_did).to_string(); let (owner, name) = resolve_owner_repo_pair(&repo, &node, dir.as_deref()).await?; - let client = NodeClient::new(&node, None); + // Sign with the loaded identity so the read-visibility-gated protected-branch + // fetch below works on the owner's own private repos. + let client = NodeClient::new(&node, Some(keypair)); // Fetch repo info let resp = client @@ -693,9 +702,9 @@ async fn cmd_owner(repo: String, node: String, dir: Option, json_out: b .to_string(); let is_owner = my_did == owner_did || my_short == owner_short; - // Fetch protected branches + // Fetch protected branches (read-visibility-gated; signed via the client). let protected: Vec = match client - .get(&format!("/api/v1/repos/{owner}/{name}/branches/protected")) + .get_maybe_signed(&format!("/api/v1/repos/{owner}/{name}/branches/protected")) .await { Ok(resp) if resp.status().is_success() => { From e3db1808ecef8d03e8e38ab4fda841b0d0f2afbb Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 28 Jun 2026 16:11:30 -0500 Subject: [PATCH 08/11] fix(review): sign the primary repo fetch in gl repo info/owner too (#94) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review caught that cmd_info and cmd_owner loaded a keypair and signed their replica/protected-branch sub-fetch but left the PRIMARY GET /api/v1/repos/{owner}/{name} on plain get(). get_repo is read-visibility-gated, so for a private repo that primary fetch 404'd first and the command bailed before reaching the signed sub-fetch — the half-fix never actually let an owner inspect their own private repo. Route both primary fetches through get_maybe_signed. Add the missing signing guards: harden test_cmd_owner_is_owner to require a signature on both mocks, and add tests for cmd_replicas and cmd_info (both were untested) asserting the request is signed when an identity is present. Out of scope (pre-existing, gated before #94, untouched here): gl repo commits, gl pr list/view/diff, and the MCP repo_get/repo_commits/pr_* tools call other already-gated read endpoints unsigned and are broken for private repos independently of this change. --- crates/gl/src/repo.rs | 80 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 3 deletions(-) diff --git a/crates/gl/src/repo.rs b/crates/gl/src/repo.rs index d27dc96..aac95c5 100644 --- a/crates/gl/src/repo.rs +++ b/crates/gl/src/repo.rs @@ -346,7 +346,7 @@ async fn cmd_info(repo: String, node: String, dir: Option) -> Result<() }; let r: Value = client - .get(&format!("/api/v1/repos/{owner}/{name}")) + .get_maybe_signed(&format!("/api/v1/repos/{owner}/{name}")) .await? .json() .await @@ -685,9 +685,10 @@ async fn cmd_owner(repo: String, node: String, dir: Option, json_out: b // fetch below works on the owner's own private repos. let client = NodeClient::new(&node, Some(keypair)); - // Fetch repo info + // Fetch repo info (get_repo is read-visibility-gated; sign so the owner can + // inspect their own private repo). let resp = client - .get(&format!("/api/v1/repos/{owner}/{name}")) + .get_maybe_signed(&format!("/api/v1/repos/{owner}/{name}")) .await .context("failed to connect to node")?; if !resp.status().is_success() { @@ -1146,11 +1147,15 @@ mod tests { let short = did.split(':').next_back().unwrap().to_string(); let mut server = mockito::Server::new_async().await; + // Both fetches are read-visibility-gated; with an identity loaded they + // must be signed. Requiring the header guards a regression to get(). let _repo = server .mock( "GET", mockito::Matcher::Regex(format!(r"^/api/v1/repos/{short}/myrepo$")), ) + .match_header("signature", mockito::Matcher::Any) + .match_header("signature-input", mockito::Matcher::Any) .with_status(200) .with_header("content-type", "application/json") .with_body(format!( @@ -1165,6 +1170,8 @@ mod tests { r"^/api/v1/repos/{short}/myrepo/branches/protected$" )), ) + .match_header("signature", mockito::Matcher::Any) + .match_header("signature-input", mockito::Matcher::Any) .with_status(200) .with_header("content-type", "application/json") .with_body(r#"["main"]"#) @@ -1290,4 +1297,71 @@ mod tests { .unwrap_err(); assert!(err.to_string().contains("not found"), "got: {err}"); } + + #[tokio::test] + async fn test_cmd_replicas_signs_when_identity_present() { + let dir = TempDir::new().unwrap(); + write_identity(&dir); + + let mut server = mockito::Server::new_async().await; + // Read-visibility-gated: with an identity the request must be signed. + let _m = server + .mock("GET", mockito::Matcher::Regex(r"/replicas$".to_string())) + .match_header("signature", mockito::Matcher::Any) + .match_header("signature-input", mockito::Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"repo":"owner/myrepo","replica_count":0,"replicas":[]}"#) + .create_async() + .await; + + cmd_replicas( + "owner/myrepo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap(); + } + + #[tokio::test] + async fn test_cmd_info_signs_repo_and_replica_fetches() { + let dir = TempDir::new().unwrap(); + write_identity(&dir); + + let mut server = mockito::Server::new_async().await; + // Both the primary repo fetch and the replica sub-fetch are + // read-visibility-gated; with an identity loaded both must be signed. + let _repo = server + .mock( + "GET", + mockito::Matcher::Regex(r"^/api/v1/repos/owner/myrepo$".to_string()), + ) + .match_header("signature", mockito::Matcher::Any) + .match_header("signature-input", mockito::Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body( + r#"{"id":"r1","name":"myrepo","owner_did":"did:key:zOwner","is_public":false,"default_branch":"main"}"#, + ) + .create_async() + .await; + let _replicas = server + .mock("GET", mockito::Matcher::Regex(r"/replicas$".to_string())) + .match_header("signature", mockito::Matcher::Any) + .match_header("signature-input", mockito::Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"replica_count":0,"replicas":[]}"#) + .create_async() + .await; + + cmd_info( + "owner/myrepo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap(); + } } From 069b336e506caa040bcdd1a2830a0e73b9c57de5 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 28 Jun 2026 16:21:37 -0500 Subject: [PATCH 09/11] test(review): execute the previously-reasoned-only paths (#94) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close the vetting gaps from the review: - node: a listed reader (non-owner) passes the read gate but is still refused the webhook list (403), and headerless on an existing-private vs an absent repo both 401 — proving the 401 fires before any lookup, so it is not an existence oracle. - node: the read-visibility surfaces (replicas, protected branches) admit a listed reader who is not the owner (the allow-list branch of visibility_check), while a non-reader stranger still 404s. - gl: get_maybe_signed's anonymous fallback (no identity -> unsigned GET) is now exercised, RED-proven by flipping it to get_signed (which signs and fails the no-signature-header assertion). --- crates/gitlawb-node/src/test_support.rs | 213 ++++++++++++++++++++++++ crates/gl/src/repo.rs | 25 +++ 2 files changed, 238 insertions(+) diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 1b467d0..1042ae8 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -810,6 +810,219 @@ mod tests { assert_eq!(resp.status(), StatusCode::NOT_FOUND, "absent repo → 404"); } + /// #94: a visibility READER who is not the owner passes the read gate but is + /// still refused the webhook list (the require_repo_owner half), and the + /// headerless 401 fires before any lookup so it cannot be an existence oracle + /// (headerless on an existing private repo and on an absent repo both 401). + #[sqlx::test] + async fn list_webhooks_reader_403_and_no_existence_oracle(pool: PgPool) { + use crate::db::VisibilityMode; + let owner = "did:key:zHKRDROWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let reader = "did:key:zHKRDRREADERBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; + let state = test_state(pool).await; + + let mut repo = seed_repo(owner, "hook-reader"); + repo.is_public = false; + state.db.create_repo(&repo).await.expect("seed repo"); + // Root allow-list rule: `reader` may read the repo at "/", but is not the owner. + state + .db + .set_visibility_rule( + &repo.id, + "/", + VisibilityMode::B, + &[reader.to_string()], + owner, + ) + .await + .expect("seed reader rule"); + let secret_url = "https://hooks.example.com/reader-case"; + state + .db + .create_webhook(&crate::db::Webhook { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + url: secret_url.to_string(), + secret: None, + events: vec!["*".to_string()], + created_by_did: owner.to_string(), + created_at: Utc::now().to_rfc3339(), + active: true, + }) + .await + .expect("seed webhook"); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/hooks", + axum::routing::get(crate::api::webhooks::list_webhooks), + ) + .with_state(state.clone()) + }; + + // A listed reader passes authorize_repo_read but is not the owner → 403, + // and the webhook url does not leak. + let resp = router() + .oneshot(signed_request_as( + reader, + Method::GET, + &format!("/api/v1/repos/{owner}/hook-reader/hooks"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::FORBIDDEN, + "a non-owner reader passes the read gate but is refused the webhook list" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + !String::from_utf8_lossy(&bytes).contains(secret_url), + "403 must not leak the url to a reader" + ); + + // Existence-oracle check: headerless on the existing private repo → 401, + // headerless on an absent repo → 401. Indistinguishable ⇒ no oracle. + let headerless = |uri: String| { + Request::builder() + .method(Method::GET) + .uri(uri) + .body(Body::empty()) + .unwrap() + }; + let resp = router() + .oneshot(headerless(format!( + "/api/v1/repos/{owner}/hook-reader/hooks" + ))) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::UNAUTHORIZED, + "headerless on an existing private repo → 401 (before any lookup)" + ); + let resp = router() + .oneshot(headerless(format!( + "/api/v1/repos/{owner}/no-such-repo/hooks" + ))) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::UNAUTHORIZED, + "headerless on an absent repo → 401 too, so existence does not leak" + ); + } + + /// #94: the read-visibility surfaces admit a listed reader who is NOT the + /// owner (the allow-list branch of visibility_check). Pins that a private + /// repo's reader — not just its owner — can read replicas and protected + /// branches, while a non-reader stranger still 404s. + #[sqlx::test] + async fn read_visibility_admits_listed_reader(pool: PgPool) { + use crate::db::VisibilityMode; + let owner = "did:key:zRDRDOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let reader = "did:key:zRDRDREADERBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; + let stranger = "did:key:zRDRDSTRGRCCCCCCCCCCCCCCCCCCCCCCCCCCCC"; + let state = test_state(pool).await; + + let mut repo = seed_repo(owner, "rdr-repo"); + repo.is_public = false; + state.db.create_repo(&repo).await.expect("seed repo"); + state + .db + .set_visibility_rule( + &repo.id, + "/", + VisibilityMode::B, + &[reader.to_string()], + owner, + ) + .await + .expect("seed reader rule"); + state + .db + .register_replica(&repo.id, stranger, "https://replica.example.com/x") + .await + .expect("seed replica"); + state + .db + .protect_branch(&repo.id, "main", owner) + .await + .expect("seed protected branch"); + + let call = |handler_router: Router, did: Option<&str>, uri: String| { + let req = match did { + Some(d) => signed_request_as(d, Method::GET, &uri, Body::empty()), + None => Request::builder() + .method(Method::GET) + .uri(uri) + .body(Body::empty()) + .unwrap(), + }; + handler_router.oneshot(req) + }; + + let replicas_router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/replicas", + axum::routing::get(crate::api::replicas::list_replicas), + ) + .with_state(state.clone()) + }; + let protect_router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/branches/protected", + axum::routing::get(crate::api::protect::list_protected_branches), + ) + .with_state(state.clone()) + }; + + // Listed reader (non-owner) → 200 on both surfaces. + for (router, path) in [ + (replicas_router(), "replicas"), + (protect_router(), "branches/protected"), + ] { + let resp = call( + router, + Some(reader), + format!("/api/v1/repos/{owner}/rdr-repo/{path}"), + ) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "a listed reader must read {path}" + ); + } + + // A non-reader stranger → 404 on both (deny path). + for (router, path) in [ + (replicas_router(), "replicas"), + (protect_router(), "branches/protected"), + ] { + let resp = call( + router, + Some(stranger), + format!("/api/v1/repos/{owner}/rdr-repo/{path}"), + ) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "a non-reader stranger must be denied {path}" + ); + } + } + /// #94 sibling: list_replicas is read-visibility-gated. Replica lists are a /// documented public mirror-discovery surface, so a PUBLIC repo stays /// anonymously listable, but a PRIVATE repo must not leak its replica URLs. diff --git a/crates/gl/src/repo.rs b/crates/gl/src/repo.rs index aac95c5..d1299a4 100644 --- a/crates/gl/src/repo.rs +++ b/crates/gl/src/repo.rs @@ -1324,6 +1324,31 @@ mod tests { .unwrap(); } + #[tokio::test] + async fn test_cmd_replicas_anonymous_when_no_identity() { + // Empty dir → no identity → get_maybe_signed must fall back to an + // anonymous GET (the public-repo path). Assert NO signature header. + let dir = TempDir::new().unwrap(); + + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("GET", mockito::Matcher::Regex(r"/replicas$".to_string())) + .match_header("signature", mockito::Matcher::Missing) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"repo":"owner/pubrepo","replica_count":0,"replicas":[]}"#) + .create_async() + .await; + + cmd_replicas( + "owner/pubrepo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap(); + } + #[tokio::test] async fn test_cmd_info_signs_repo_and_replica_fetches() { let dir = TempDir::new().unwrap(); From e2ba8a877d4c867aad594b8a7e45e9363704317c Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 28 Jun 2026 16:47:27 -0500 Subject: [PATCH 10/11] test(review): execute the MCP dispatch and gl->node signature seams (#94) Close the last two reasoned-only paths by execution: - MCP webhook_list: a call_tool("webhook_list", ...) dispatch test against a mock node asserts the /hooks request carries a signature (get_signed). RED-proven: flipping the arm back to get() fails the header assertion. - gl->node end-to-end: a real RFC-9421 signature produced exactly as the gl client's get_signed does (gitlawb_core::http_sig::sign_request over GET + empty body) is run through the node's actual optional_signature middleware, which verifies it and authorizes the owner (200 + webhook body). RED-proven: signing over POST while sending GET makes the node reject it. This stitches the gl signing side and the node verifying side in one test rather than mocking one end. --- crates/gitlawb-node/src/test_support.rs | 73 +++++++++++++++++++++++++ crates/gl/src/mcp.rs | 35 ++++++++++++ 2 files changed, 108 insertions(+) diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 1042ae8..91b5890 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -1462,6 +1462,79 @@ mod tests { ); } + /// #94 end-to-end seam: a REAL RFC-9421 signature produced exactly as the gl + /// client's get_signed does (gitlawb_core::http_sig::sign_request over GET + + /// empty body) is accepted by the node's actual optional_signature middleware, + /// which verifies it and injects AuthenticatedDid, so the owner's signed + /// `gl webhook list` resolves to 200. This stitches the gl signing side and + /// the node verifying side in one test (not mockito on one end and a unit + /// verify on the other). + #[sqlx::test] + async fn list_webhooks_accepts_a_real_gl_signature_e2e(pool: PgPool) { + use gitlawb_core::http_sig::sign_request; + use gitlawb_core::identity::Keypair; + + let kp = Keypair::generate(); + let owner_did = kp.did().to_string(); + // Short owner form in the URL path: no colons (so the signed @path and the + // node's path_and_query() match byte-for-byte), and get_repo's owner LIKE + // match + did_matches still authorize the full-DID signer as the owner. + let short = owner_did.split(':').next_back().unwrap().to_string(); + let state = test_state(pool).await; + let repo = seed_repo(&owner_did, "real-sig-repo"); + state.db.create_repo(&repo).await.expect("seed repo"); + let url = "https://hooks.example.com/e2e"; + state + .db + .create_webhook(&crate::db::Webhook { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + url: url.to_string(), + secret: None, + events: vec!["*".to_string()], + created_by_did: owner_did.clone(), + created_at: Utc::now().to_rfc3339(), + active: true, + }) + .await + .expect("seed webhook"); + + let path = format!("/api/v1/repos/{short}/real-sig-repo/hooks"); + let signed = sign_request(&kp, "GET", &path, b""); + let req = Request::builder() + .method(Method::GET) + .uri(&path) + .header("content-digest", signed.content_digest) + .header("signature-input", signed.signature_input) + .header("signature", signed.signature) + .body(Body::empty()) + .unwrap(); + + // Mount the handler UNDER the production optional_signature middleware so + // the node actually verifies the signature (not the injected-DID shortcut). + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/hooks", + axum::routing::get(crate::api::webhooks::list_webhooks), + ) + .layer(axum::middleware::from_fn(crate::auth::optional_signature)) + .with_state(state); + + let resp = router.oneshot(req).await.unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "the node must verify a real gl-style signature and authorize the owner" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + String::from_utf8_lossy(&bytes).contains(url), + "the verified owner sees the webhook list" + ); + } + /// Issue #6 / jatmn finding 2: `/api/v1/stats` counts logical repos, not raw /// rows. With a mirror+canonical pair and a standalone repo present, the /// `repos` count is 2. diff --git a/crates/gl/src/mcp.rs b/crates/gl/src/mcp.rs index 0916539..cda9b60 100644 --- a/crates/gl/src/mcp.rs +++ b/crates/gl/src/mcp.rs @@ -1826,6 +1826,41 @@ mod tests { assert_eq!(parsed["body"], "looks good"); } + #[tokio::test] + async fn test_webhook_list_via_mcp_signs_the_request() { + let mut server = mockito::Server::new_async().await; + let dir = tempfile::TempDir::new().unwrap(); + let kp = gitlawb_core::identity::Keypair::generate(); + std::fs::write( + dir.path().join("identity.pem"), + kp.to_pem().unwrap().as_bytes(), + ) + .unwrap(); + + // /hooks is owner-gated, so the MCP webhook_list tool must send a SIGNED + // request (get_signed). Requiring the header catches a regression to get(). + // Passing `owner` in args makes resolve_owner skip the node-root lookup. + let _m = server + .mock("GET", mockito::Matcher::Regex(r"/hooks$".to_string())) + .match_header("signature", mockito::Matcher::Any) + .match_header("signature-input", mockito::Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"webhooks":[],"count":0}"#) + .create_async() + .await; + + let result = call_tool( + "webhook_list", + json!({"owner": "alice", "repo": "myrepo"}), + &server.url(), + Some(dir.path()), + ) + .await + .unwrap(); + assert!(result.contains("webhooks"), "got: {result}"); + } + #[test] fn test_tool_count_is_42() { let tools = tool_definitions(); From 845381441cd410083d97eea05917c363398aed6b Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 28 Jun 2026 20:34:52 -0500 Subject: [PATCH 11/11] fix(review): fail closed when the repo lookup errors in list_repo_events (#94) `get_repo(...).await.ok().flatten()` collapsed a lookup Err into None, so a transient DB failure skipped the read-visibility gate and the handler served a private repo's gossip ref-update events. Propagate with `?` instead: an error now maps to AppError::Internal (500) and fails closed, while a genuine Ok(None) (repo not hosted locally) stays the intentional ungated pass-through. Add a regression test that forces a get_repo error (drop the column its SELECT reads) and asserts 500 with no secret in the body. The injection is self-verifying: it asserts the lookup succeeds pre-drop and errors post-drop, so the test can't pass vacuously if get_repo's projection changes later. --- crates/gitlawb-node/src/api/events.rs | 9 ++- crates/gitlawb-node/src/test_support.rs | 100 ++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 2 deletions(-) diff --git a/crates/gitlawb-node/src/api/events.rs b/crates/gitlawb-node/src/api/events.rs index 4a1972a..57b7025 100644 --- a/crates/gitlawb-node/src/api/events.rs +++ b/crates/gitlawb-node/src/api/events.rs @@ -60,8 +60,13 @@ pub async fn list_repo_events( .unwrap_or(50) .min(200); - // Look up the repo record once so we can use the full owner DID - let repo_record = state.db.get_repo(&owner, &repo_name).await.ok().flatten(); + // Look up the repo record once so we can use the full owner DID. + // #113: propagate a lookup error (fail closed) instead of swallowing it with + // `.ok().flatten()`. Collapsing Err into None would skip the visibility gate + // below and serve a private repo's events. get_repo returns anyhow::Result, so + // `?` maps an error to AppError::Internal (500). Only a genuine Ok(None) (the + // repo is not hosted locally) is the intentional ungated pass-through. + let repo_record = state.db.get_repo(&owner, &repo_name).await?; // #94: if this node hosts the repo locally, gate on read visibility BEFORE // serving any events (cert OR gossip). A non-reader of a local private repo diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 91b5890..4208bc0 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -1462,6 +1462,106 @@ mod tests { ); } + /// #113 fail-closed: when the repo lookup ERRORS (not a clean Ok(None)), the + /// visibility gate must not be skipped. The buggy `.ok().flatten()` collapsed an + /// Err into None, so a transient DB failure during the lookup dropped the gate + /// and the handler served the private repo's gossip ref-updates via the + /// ungated None branch (slug taken from the URL owner segment). We force a + /// deterministic get_repo error by dropping the column its SELECT reads, then + /// require the handler to fail closed (500, no secret) instead of 200-with-secret. + #[sqlx::test] + async fn list_repo_events_fails_closed_when_repo_lookup_errors(pool: PgPool) { + use crate::db::ReceivedRefUpdate; + let owner = "did:key:zEVTERRAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + // Caller addresses the repo by the full key part (the slug gossip rows use), + // so the buggy None-branch fallback slug matches the seeded private update. + let keypart = owner.split(':').next_back().unwrap(); + let state = test_state(pool.clone()).await; + + let mut priv_repo = seed_repo(owner, "evt-priv"); + priv_repo.is_public = false; + state + .db + .create_repo(&priv_repo) + .await + .expect("seed private repo"); + + state + .db + .insert_ref_update(&ReceivedRefUpdate { + id: uuid::Uuid::new_v4().to_string(), + node_did: owner.to_string(), + pusher_did: owner.to_string(), + repo: format!("{keypart}/evt-priv"), + ref_name: "refs/heads/embargo-gossip".to_string(), + old_sha: "0".repeat(40), + new_sha: "gossipSEKRET".to_string(), + timestamp: Utc::now().to_rfc3339(), + cert_id: None, + received_at: Utc::now().to_rfc3339(), + from_peer: "peer".to_string(), + }) + .await + .expect("seed private gossip update"); + + // Force get_repo's SELECT (which reads machine_id, db/mod.rs) to error, + // simulating a transient DB failure during the visibility lookup. The repo + // row and the gossip update both remain present. + // Precondition: the lookup must succeed before we break it, otherwise the + // injection proves nothing. + state + .db + .get_repo(keypart, "evt-priv") + .await + .expect("pre-drop lookup must succeed") + .expect("private repo row must be present pre-drop"); + sqlx::query("ALTER TABLE repos DROP COLUMN machine_id") + .execute(&pool) + .await + .expect("drop column to force a get_repo error"); + // Guard the injection: if a future refactor drops machine_id from get_repo's + // SELECT, this assertion fails loudly instead of letting the test pass + // vacuously (get_repo would return Ok and the gate, not the error path, + // would drive the response). + assert!( + state.db.get_repo(keypart, "evt-priv").await.is_err(), + "dropping machine_id must make get_repo error, else this test no longer exercises the Err path" + ); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/events", + axum::routing::get(crate::api::events::list_repo_events), + ) + .with_state(state.clone()); + let resp = router + .oneshot( + Request::builder() + .method(Method::GET) + .uri(format!("/api/v1/repos/{keypart}/evt-priv/events")) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + // Fail closed: a lookup error must surface as 500, never a 200 that serves + // the private repo's ref metadata through the ungated branch. + assert_eq!( + resp.status(), + StatusCode::INTERNAL_SERVER_ERROR, + "a repo-lookup error must fail closed, not skip the gate" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let body = String::from_utf8_lossy(&bytes).to_string(); + assert!( + !body.contains("gossipSEKRET"), + "fail-closed response must not leak the gossip secret" + ); + } + /// #94 end-to-end seam: a REAL RFC-9421 signature produced exactly as the gl /// client's get_signed does (gitlawb_core::http_sig::sign_request over GET + /// empty body) is accepted by the node's actual optional_signature middleware,