diff --git a/Cargo.lock b/Cargo.lock index d42c3709..dc6540e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3300,7 +3300,7 @@ dependencies = [ [[package]] name = "git-remote-gitlawb" -version = "0.3.9" +version = "0.4.0" dependencies = [ "anyhow", "gitlawb-core", @@ -3311,7 +3311,7 @@ dependencies = [ [[package]] name = "gitlawb-attest" -version = "0.3.9" +version = "0.4.0" dependencies = [ "base64", "ed25519-dalek", @@ -3328,7 +3328,7 @@ dependencies = [ [[package]] name = "gitlawb-core" -version = "0.3.9" +version = "0.4.0" dependencies = [ "anyhow", "base64", @@ -3355,7 +3355,7 @@ dependencies = [ [[package]] name = "gitlawb-node" -version = "0.3.9" +version = "0.4.0" dependencies = [ "alloy", "anyhow", @@ -3411,7 +3411,7 @@ dependencies = [ [[package]] name = "gl" -version = "0.3.9" +version = "0.4.0" dependencies = [ "alloy", "anyhow", diff --git a/crates/gitlawb-node/src/api/events.rs b/crates/gitlawb-node/src/api/events.rs index 45db8a0d..57b70252 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") @@ -57,8 +60,31 @@ 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 + // 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"), diff --git a/crates/gitlawb-node/src/api/labels.rs b/crates/gitlawb-node/src/api/labels.rs index 9b904277..649d9984 100644 --- a/crates/gitlawb-node/src/api/labels.rs +++ b/crates/gitlawb-node/src/api/labels.rs @@ -71,15 +71,18 @@ pub async fn remove_label( } /// GET /api/v1/repos/:owner/:repo/labels +/// +/// Read-visibility-gated (INV-2 root listing): a public repo's labels stay +/// anonymously listable; a private repo's label names are hidden (404) from +/// anyone who cannot read it at the root. 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 ea4591fb..73958eb2 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -166,6 +166,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"); @@ -181,6 +182,13 @@ mod authz_guard { (pulls, "merge_pr", "require_repo_owner("), (webhooks, "create_webhook", "require_repo_owner("), (webhooks, "delete_webhook", "require_repo_owner("), + // 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. Both halves + // are pinned: the authorize_repo_read marker guards the read half + // (existence hiding) and require_repo_owner guards the owner half. + (webhooks, "list_webhooks", "authorize_repo_read("), + (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) @@ -197,6 +205,15 @@ mod authz_guard { // get_by_cid gates each iterated repo row directly via visibility_check // (KTD2a: it must NOT route through authorize_repo_read's fuzzy re-resolve). (ipfs, "get_by_cid", "visibility_check("), + // #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("), + (labels, "list_labels", "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("), @@ -404,13 +421,8 @@ mod authz_guard { ("list_issues", "#120"), ("get_issue", "#120"), ("list_issue_comments", "#120"), - ("list_labels", "#120"), ("list_repo_bounties", "#120"), ("get_star_status", "#120"), - ("list_repo_events", "#94 (PR #113)"), - ("list_webhooks", "#94 (PR #113)"), - ("list_replicas", "PR #113"), - ("list_protected_branches", "PR #113"), ]; let is_known = |n: &str| known_ungated.iter().any(|(k, _)| *k == n); // Any one of these = the handler binds the caller to an authz decision: the diff --git a/crates/gitlawb-node/src/api/protect.rs b/crates/gitlawb-node/src/api/protect.rs index 7f51617d..4dd08b00 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/api/replicas.rs b/crates/gitlawb-node/src/api/replicas.rs index e578b121..ece82bff 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/api/webhooks.rs b/crates/gitlawb-node/src/api/webhooks.rs index 52ef00b6..d6a6997c 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; @@ -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 f7e7f764..6ee571fb 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -621,6 +621,1294 @@ 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" + ); + + // 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( + 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"); + } + + /// #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. + /// 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"); + } + + /// #94 sibling: list_labels is read-visibility-gated. A public repo's labels + /// stay anonymously listable; a private repo's label names must not leak to a + /// non-reader (404). A listed reader of the private repo reads the label; the + /// owner reads it; a non-reader stranger 404s. + #[sqlx::test] + async fn list_labels_is_read_visibility_gated(pool: PgPool) { + use crate::db::VisibilityMode; + let owner = "did:key:zLBLOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let reader = "did:key:zLBLREADERBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; + let stranger = "did:key:zLBLSTRGRCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC"; + let state = test_state(pool).await; + + let mut repo = seed_repo(owner, "lbl-priv"); + repo.is_public = false; + state + .db + .create_repo(&repo) + .await + .expect("seed private repo"); + state + .db + .set_visibility_rule( + &repo.id, + "/", + VisibilityMode::B, + &[reader.to_string()], + owner, + ) + .await + .expect("seed reader rule"); + state + .db + .add_label(&repo.id, "bug") + .await + .expect("seed label"); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/labels", + axum::routing::get(crate::api::labels::list_labels), + ) + .with_state(state.clone()) + }; + let leaks = |bytes: &[u8]| String::from_utf8_lossy(bytes).contains("bug"); + let uri = format!("/api/v1/repos/{owner}/lbl-priv/labels"); + + // Owner (signed) → 200, sees the label. + let resp = router() + .oneshot(signed_request_as(owner, Method::GET, &uri, Body::empty())) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "owner reads its private labels" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!(leaks(&bytes), "owner response must include the label"); + + // Listed reader (signed, non-owner) → 200, sees the label. + let resp = router() + .oneshot(signed_request_as(reader, Method::GET, &uri, Body::empty())) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "a listed reader reads the labels" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + leaks(&bytes), + "listed reader response must include the label" + ); + + // Non-reader stranger (signed) → 404, no label leaked. + let resp = router() + .oneshot(signed_request_as( + stranger, + Method::GET, + &uri, + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "a non-reader stranger is denied the private labels" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!(!leaks(&bytes), "404 must not leak the label name"); + + // Anonymous on the private repo → 404. + let resp = router() + .oneshot( + Request::builder() + .method(Method::GET) + .uri(&uri) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "anon is denied the private labels" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!(!leaks(&bytes), "anon 404 must not leak the label name"); + + // Public repo, anonymous → 200, label visible. The gate must not break + // the existing anonymous read path for public repos. + let pub_repo = seed_repo(owner, "lbl-pub"); + state + .db + .create_repo(&pub_repo) + .await + .expect("seed public repo"); + state + .db + .add_label(&pub_repo.id, "pubtag") + .await + .expect("seed public label"); + let resp = router() + .oneshot( + Request::builder() + .method(Method::GET) + .uri(format!("/api/v1/repos/{owner}/lbl-pub/labels")) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "a public repo's labels stay anonymously listable" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + String::from_utf8_lossy(&bytes).contains("pubtag"), + "public anon response must include the label" + ); + + // Absent repo → 404 (uniform with the non-reader denial; no 500). + let resp = router() + .oneshot( + Request::builder() + .method(Method::GET) + .uri(format!("/api/v1/repos/{owner}/no-such-repo/labels")) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + 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 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"); + } + + /// #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" + ); + } + + /// #113: the events read-gate admits a listed reader (the allow-list branch + /// of visibility_check), not just the owner — parity with the replica and + /// protected-branch surfaces covered by `read_visibility_admits_listed_reader`. + /// A non-reader stranger still 404s with no leak. + #[sqlx::test] + async fn list_repo_events_admits_listed_reader(pool: PgPool) { + use crate::db::{RefCertificate, VisibilityMode}; + let owner = "did:key:zEVTRDROWNERAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let reader = "did:key:zEVTRDRREADERBBBBBBBBBBBBBBBBBBBBBBBB"; + let stranger = "did:key:zEVTRDRSTRGRCCCCCCCCCCCCCCCCCCCCCCCC"; + let state = test_state(pool).await; + + let mut repo = seed_repo(owner, "evt-rdr"); + repo.is_public = false; + state + .db + .create_repo(&repo) + .await + .expect("seed private repo"); + state + .db + .set_visibility_rule( + &repo.id, + "/", + VisibilityMode::B, + &[reader.to_string()], + owner, + ) + .await + .expect("seed reader rule"); + state + .db + .insert_ref_certificate(&RefCertificate { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + ref_name: "refs/heads/embargo-rdr".to_string(), + old_sha: "0".repeat(40), + new_sha: "rdrsha00".to_string(), + pusher_did: owner.to_string(), + node_did: owner.to_string(), + signature: "sig".to_string(), + issued_at: Utc::now().to_rfc3339(), + }) + .await + .expect("seed private cert"); + + 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 uri = format!("/api/v1/repos/{owner}/evt-rdr/events"); + let text = |bytes: &[u8]| String::from_utf8_lossy(bytes).to_string(); + + // Listed reader (non-owner) → 200, the private cert is served. + let resp = router() + .oneshot(signed_request_as(reader, Method::GET, &uri, Body::empty())) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "a listed reader (non-owner) must read events" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + text(&bytes).contains("embargo-rdr"), + "listed reader sees the private cert" + ); + + // A non-reader stranger → 404, and the cert ref does not leak. + let resp = router() + .oneshot(signed_request_as( + stranger, + Method::GET, + &uri, + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "a non-reader stranger must 404" + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + !text(&bytes).contains("embargo-rdr"), + "404 must not leak the cert ref" + ); + assert!( + !text(&bytes).contains("rdrsha00"), + "404 must not leak the cert sha" + ); + } + + /// #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, + /// 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/gitlawb-node/src/visibility.rs b/crates/gitlawb-node/src/visibility.rs index 335d2618..a7f1ce45 100644 --- a/crates/gitlawb-node/src/visibility.rs +++ b/crates/gitlawb-node/src/visibility.rs @@ -24,11 +24,11 @@ fn nfc(s: &str) -> String { s.nfc().collect() } -/// True if `caller` is the repo owner (matches full did:key or its short form), -/// mirroring the owner-match idiom in `api/protect.rs`. +/// True if `caller` is the repo owner. Delegates to [`crate::api::did_matches`] +/// so the read gate and `require_repo_owner` share one normalization: bare and +/// full forms collapse only within `did:key:`, never across DID methods. fn is_owner(owner_did: &str, caller: &str) -> bool { - let owner_short = owner_did.split(':').next_back().unwrap_or(owner_did); - caller == owner_did || caller == owner_short + crate::api::did_matches(owner_did, caller) } /// The match prefix for a glob: "/" stays "/", "/secret/**" becomes "/secret". @@ -383,6 +383,41 @@ mod tests { ); } + #[test] + fn owner_match_normalizes_did_key_both_directions() { + // Bare owner stored (mirror rows), full did:key caller signs in. + assert_eq!( + visibility_check(&[], false, "zABC", Some("did:key:zABC"), "/"), + Decision::Allow + ); + // Full owner stored, bare caller. + assert_eq!( + visibility_check(&[], false, "did:key:zABC", Some("zABC"), "/"), + Decision::Allow + ); + } + + #[test] + fn owner_match_rejects_cross_method_collision() { + // A did:gitlawb caller sharing the base58 id must not be admitted as the + // did:key owner. + assert_eq!( + visibility_check(&[], false, "did:key:zABC", Some("did:gitlawb:zABC"), "/"), + Decision::Deny + ); + // The over-match the old trailing-segment split allowed: an owner stored + // under a non-key method must not be matched by the bare base58 id. The + // old `owner_short` compare would wrongly Allow this; did_matches denies. + assert_eq!( + visibility_check(&[], false, "did:gitlawb:zABC", Some("zABC"), "/"), + Decision::Deny + ); + assert_eq!( + visibility_check(&[], false, "did:gitlawb:zABC", Some("did:key:zABC"), "/"), + Decision::Deny + ); + } + // Mirrors the gossip-announce gate in git_receive_pack: announce iff an // anonymous caller can read "/". #[test] diff --git a/crates/gl/src/http.rs b/crates/gl/src/http.rs index 32e90a1e..1eabd0ae 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/mcp.rs b/crates/gl/src/mcp.rs index 5a4b53ae..18acd2da 100644 --- a/crates/gl/src/mcp.rs +++ b/crates/gl/src/mcp.rs @@ -932,13 +932,33 @@ async fn call_tool( "webhook_list" => { let repo = args["repo"].as_str().context("missing 'repo'")?; - let owner = resolve_owner(&args, &client).await?; - let resp: Value = client - .get(&format!("/api/v1/repos/{owner}/{repo}/hooks")) - .await? - .json() + // Owner-gated route: an explicit `owner` arg wins, otherwise default to + // the signing keypair's short DID (NOT the node DID). The request is + // signed by this keypair, so the path owner must match it or the node + // returns 403/404. + let owner = if let Some(o) = args.get("owner").and_then(|v| v.as_str()) { + o.to_string() + } else { + let kp = keypair + .as_ref() + .context("no identity found — run `gl identity new` first")?; + let did = kp.did().to_string(); + did.split(':').next_back().unwrap_or(&did).to_string() + }; + // Owner-gated route: must be signed (get_signed), not a plain get(). + let resp = client + .get_signed(&format!("/api/v1/repos/{owner}/{repo}/hooks")) .await?; - Ok(serde_json::to_string_pretty(&resp)?) + // Check the HTTP status before deserializing: a 401/403/404 JSON error + // body (missing identity, wrong owner, private/deleted repo) must fail + // the tool call, not be returned as a successful result. + let status = resp.status(); + let body: Value = resp.json().await?; + if !status.is_success() { + let msg = body["message"].as_str().unwrap_or("unknown error"); + anyhow::bail!("webhook_list failed ({status}): {msg}"); + } + Ok(serde_json::to_string_pretty(&body)?) } "webhook_delete" => { @@ -1825,6 +1845,187 @@ 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}"); + } + + #[tokio::test] + async fn test_webhook_list_default_owner_is_keypair_not_node_did() { + // When `owner` is omitted, the owner segment must come from the signing + // keypair's short DID, NOT the node root DID. The "/" mock returns a + // different DID; the /hooks mock only matches the keypair's short DID, + // so a regression to the node DID would leave the request unmatched. + 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(); + + let _root = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"did":"did:key:z6MkNodeRootDid"}"#) + .create_async() + .await; + + let did = kp.did().to_string(); + let short = did.split(':').next_back().unwrap_or(&did).to_string(); + + let _m = server + .mock( + "GET", + mockito::Matcher::Regex(format!(r"/api/v1/repos/{short}/myrepo/hooks$")), + ) + .match_header("signature", mockito::Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"webhooks":[],"count":0}"#) + .expect(1) + .create_async() + .await; + + let result = call_tool( + "webhook_list", + json!({"repo": "myrepo"}), + &server.url(), + Some(dir.path()), + ) + .await + .unwrap(); + assert!(result.contains("webhooks"), "got: {result}"); + _m.assert_async().await; + } + + #[tokio::test] + async fn test_webhook_list_explicit_owner_overrides_keypair() { + // An explicit `owner` arg must win over the keypair default and the node + // DID. The path must use the supplied owner; "/" must not even be hit. + 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(); + + let _m = server + .mock( + "GET", + mockito::Matcher::Regex(r"/api/v1/repos/someoneelse/myrepo/hooks$".to_string()), + ) + .match_header("signature", mockito::Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"webhooks":[],"count":0}"#) + .expect(1) + .create_async() + .await; + + let result = call_tool( + "webhook_list", + json!({"repo": "myrepo", "owner": "someoneelse"}), + &server.url(), + Some(dir.path()), + ) + .await + .unwrap(); + assert!(result.contains("webhooks"), "got: {result}"); + _m.assert_async().await; + } + + #[tokio::test] + async fn test_webhook_list_non_success_status_errors() { + // A 403 (or any non-2xx) JSON error body for a missing identity, wrong + // owner, or private/deleted repo must fail the tool call, NOT be returned + // as a successful result. Mirrors the `gl webhook list` status check. + 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(); + + let _m = server + .mock("GET", mockito::Matcher::Regex(r"/hooks$".to_string())) + .match_header("signature", mockito::Matcher::Any) + .with_status(403) + .with_header("content-type", "application/json") + .with_body(r#"{"message":"only the repo owner can perform this action"}"#) + .create_async() + .await; + + let err = call_tool( + "webhook_list", + json!({"owner": "alice", "repo": "myrepo"}), + &server.url(), + Some(dir.path()), + ) + .await + .expect_err("non-2xx must error, not return the error body as success"); + let msg = err.to_string(); + assert!(msg.contains("webhook_list failed (403"), "got: {msg}"); + assert!( + msg.contains("only the repo owner"), + "must surface the node message, got: {msg}" + ); + } + + #[tokio::test] + async fn test_webhook_list_without_keypair_errors() { + // No identity loaded and no explicit owner → the tool must error rather + // than fall back to the node DID and issue an unsigned, mis-scoped GET. + let server = mockito::Server::new_async().await; + let dir = tempfile::TempDir::new().unwrap(); // empty: no identity.pem + + let err = call_tool( + "webhook_list", + json!({"repo": "myrepo"}), + &server.url(), + Some(dir.path()), + ) + .await + .expect_err("must error without an identity"); + assert!(err.to_string().contains("no identity found"), "got: {err}"); + } + #[test] fn test_tool_count_is_42() { let tools = tool_definitions(); diff --git a/crates/gl/src/protect.rs b/crates/gl/src/protect.rs index cde0dd6c..a45a5ebf 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 455a55f4..49db0d46 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,12 +205,12 @@ 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, } } /// Derive the short DID key segment from a keypair, or fall back to the node's DID. -async fn resolve_owner_did(node: &str, dir: Option<&std::path::Path>) -> Result { +pub(crate) async fn resolve_owner_did(node: &str, dir: Option<&std::path::Path>) -> Result { if let Ok(kp) = load_keypair_from_dir(dir) { let did = kp.did().to_string(); return Ok(did.split(':').next_back().unwrap_or(&did).to_string()); @@ -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(); @@ -340,12 +345,16 @@ async fn cmd_info(repo: String, node: String, dir: Option) -> Result<() (short, repo) }; - let r: Value = client - .get(&format!("/api/v1/repos/{owner}/{name}")) - .await? - .json() + let resp = client + .get_maybe_signed(&format!("/api/v1/repos/{owner}/{name}")) .await - .context("repo not found")?; + .context("failed to connect to node")?; + let status = resp.status(); + let r: Value = resp.json().await.unwrap_or_default(); + if !status.is_success() { + let msg = r["message"].as_str().unwrap_or("unknown error"); + anyhow::bail!("repo info failed ({status}): {msg}"); + } let owner_did = r["owner_did"].as_str().unwrap_or(&owner); let gitlawb_url = format!("gitlawb://{owner_did}/{name}"); @@ -368,7 +377,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 +457,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(); @@ -574,7 +585,7 @@ async fn cmd_fork( } /// Returns (owner, repo_name) — if repo contains '/', splits on it; otherwise uses caller's DID. -async fn resolve_owner_repo_pair( +pub(crate) async fn resolve_owner_repo_pair( repo: &str, node: &str, dir: Option<&std::path::Path>, @@ -647,16 +658,23 @@ 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); + // Read-visibility-gated like the sibling read surfaces (replicas, protected + // branches): sign when an identity is available so a private-repo owner can + // read their own labels, while public repos stay anonymously listable. + 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")) - .await? - .json() + let resp = client + .get_maybe_signed(&format!("/api/v1/repos/{owner}/{name}/labels")) .await - .context("invalid JSON")?; + .context("failed to connect to node")?; + let status = resp.status(); + let body: Value = resp.json().await.unwrap_or_default(); + if !status.is_success() { + let msg = body["message"].as_str().unwrap_or("unknown error"); + anyhow::bail!("list labels failed ({status}): {msg}"); + } - let labels = resp["labels"].as_array().cloned().unwrap_or_default(); + let labels = body["labels"].as_array().cloned().unwrap_or_default(); if labels.is_empty() { println!("No labels on {owner}/{name}"); } else { @@ -668,17 +686,39 @@ async fn cmd_label_list(repo: String, node: String, dir: Option) -> Res Ok(()) } +/// Parse the protected-branch list from the node's `GET /branches/protected` +/// response. The endpoint returns `{"protected_branches": [...], "count": N}` +/// (see `crates/gitlawb-node/src/api/protect.rs`); each entry may be a plain +/// branch-name string or an object carrying a `name` field. +fn parse_protected_branches(val: &Value) -> Vec { + val["protected_branches"] + .as_array() + .map(|arr| { + arr.iter() + .filter_map(|b| { + b.as_str() + .map(String::from) + .or_else(|| b["name"].as_str().map(String::from)) + }) + .collect() + }) + .unwrap_or_default() +} + async fn cmd_owner(repo: String, node: String, dir: Option, json_out: bool) -> Result<()> { let keypair = load_keypair_from_dir(dir.as_deref())?; let my_did = keypair.did().to_string(); 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 + // 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() { @@ -693,25 +733,14 @@ 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() => { let val: Value = resp.json().await.unwrap_or_default(); - val.as_array() - .or_else(|| val["branches"].as_array()) - .map(|arr| { - arr.iter() - .filter_map(|b| { - b.as_str() - .map(String::from) - .or_else(|| b["name"].as_str().map(String::from)) - }) - .collect() - }) - .unwrap_or_default() + parse_protected_branches(&val) } _ => Vec::new(), }; @@ -749,6 +778,20 @@ mod tests { use super::*; use tempfile::TempDir; + #[test] + fn parse_protected_branches_reads_node_shape() { + use serde_json::json; + assert_eq!( + parse_protected_branches(&json!({"protected_branches":["main","release"],"count":2})), + vec!["main".to_string(), "release".to_string()] + ); + assert!(parse_protected_branches(&json!({"count":0})).is_empty()); + assert_eq!( + parse_protected_branches(&json!({"protected_branches":[{"name":"main"}]})), + vec!["main".to_string()] + ); + } + fn write_identity(dir: &TempDir) { let kp = gitlawb_core::identity::Keypair::generate(); let pem = kp.to_pem().unwrap(); @@ -1086,6 +1129,10 @@ mod tests { "GET", mockito::Matcher::Regex(r"^/api/v1/repos/[^/]+/myrepo/labels$".to_string()), ) + // Identity is loaded, so get_maybe_signed must sign — requiring the + // header guards against a regression back to bare 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#"{"labels":[]}"#) @@ -1112,6 +1159,10 @@ mod tests { "GET", mockito::Matcher::Regex(r"^/api/v1/repos/[^/]+/myrepo/labels$".to_string()), ) + // Identity is loaded, so get_maybe_signed must sign — requiring the + // header guards against a regression back to bare 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#"{"labels":["language:rust","topic:defi"]}"#) @@ -1127,6 +1178,97 @@ mod tests { .unwrap(); } + #[tokio::test] + async fn test_cmd_label_list_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"/labels$".to_string())) + .match_header("signature", mockito::Matcher::Missing) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"labels":[]}"#) + .create_async() + .await; + + cmd_label_list( + "owner/pubrepo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap(); + } + + #[tokio::test] + async fn test_cmd_label_list_404_errors() { + // A non-reader of a private repo gets 404 from the now-gated endpoint. + // The client must surface that as an error, not print "No labels" and + // exit 0 (the error body has no `labels` key). + let dir = TempDir::new().unwrap(); + write_identity(&dir); + + let mut server = mockito::Server::new_async().await; + let _m = server + .mock( + "GET", + mockito::Matcher::Regex(r"^/api/v1/repos/[^/]+/myrepo/labels$".to_string()), + ) + .with_status(404) + .with_header("content-type", "application/json") + .with_body(r#"{"message":"repo not found"}"#) + .create_async() + .await; + + let err = cmd_label_list( + "myrepo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap_err(); + assert!( + err.to_string().contains("list labels failed (404") + && err.to_string().contains("repo not found"), + "got: {err}" + ); + } + + #[tokio::test] + async fn test_cmd_label_list_non_json_error_surfaces_status() { + // A non-2xx response with a non-JSON body (e.g. a proxy 502) must still + // surface the HTTP status, not collapse to a bare JSON-parse error. + let dir = TempDir::new().unwrap(); + write_identity(&dir); + + let mut server = mockito::Server::new_async().await; + let _m = server + .mock( + "GET", + mockito::Matcher::Regex(r"^/api/v1/repos/[^/]+/myrepo/labels$".to_string()), + ) + .with_status(502) + .with_header("content-type", "text/html") + .with_body("502 Bad Gateway") + .create_async() + .await; + + let err = cmd_label_list( + "myrepo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap_err(); + assert!( + err.to_string().contains("list labels failed (502"), + "got: {err}" + ); + } + #[tokio::test] async fn test_cmd_owner_is_owner() { let dir = TempDir::new().unwrap(); @@ -1137,11 +1279,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!( @@ -1156,9 +1302,11 @@ 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"]"#) + .with_body(r#"{"protected_branches":["main"],"count":1}"#) .create_async() .await; @@ -1199,7 +1347,7 @@ mod tests { ) .with_status(200) .with_header("content-type", "application/json") - .with_body(r#"["main","release"]"#) + .with_body(r#"{"protected_branches":["main","release"],"count":2}"#) .create_async() .await; @@ -1240,7 +1388,7 @@ mod tests { ) .with_status(200) .with_header("content-type", "application/json") - .with_body(r#"["main"]"#) + .with_body(r#"{"protected_branches":["main"],"count":1}"#) .create_async() .await; @@ -1281,4 +1429,163 @@ 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_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(); + 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(); + } + + #[tokio::test] + async fn test_cmd_info_404_surfaces_error_not_fake_repo() { + // A non-reader of a private repo gets a JSON 404 from the now-gated + // GET /api/v1/repos/{owner}/{name}. The command must surface that as an + // error, not parse the error body and print a fabricated repo summary + // (owner / "?" / public=true / main) before exiting 0. + let dir = TempDir::new().unwrap(); + write_identity(&dir); + + let mut server = mockito::Server::new_async().await; + let _repo = server + .mock( + "GET", + mockito::Matcher::Regex(r"^/api/v1/repos/owner/myrepo$".to_string()), + ) + .with_status(404) + .with_header("content-type", "application/json") + .with_body(r#"{"message":"repo not found"}"#) + .create_async() + .await; + + let err = cmd_info( + "owner/myrepo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap_err(); + assert!( + err.to_string().contains("repo info failed (404") + && err.to_string().contains("repo not found"), + "got: {err}" + ); + } + + #[tokio::test] + async fn test_cmd_info_non_json_error_surfaces_status() { + // A non-2xx response with a non-JSON body (e.g. a proxy 502) must still + // surface the HTTP status, not collapse to a bare JSON-parse error. + let dir = TempDir::new().unwrap(); + write_identity(&dir); + + let mut server = mockito::Server::new_async().await; + let _repo = server + .mock( + "GET", + mockito::Matcher::Regex(r"^/api/v1/repos/owner/myrepo$".to_string()), + ) + .with_status(502) + .with_header("content-type", "text/html") + .with_body("502 Bad Gateway") + .create_async() + .await; + + let err = cmd_info( + "owner/myrepo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap_err(); + assert!( + err.to_string().contains("repo info failed (502"), + "got: {err}" + ); + } } diff --git a/crates/gl/src/webhook.rs b/crates/gl/src/webhook.rs index 45b80ecd..a7311a84 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, @@ -76,15 +78,6 @@ pub async fn run(args: WebhookArgs) -> Result<()> { } } -async fn resolve_owner(client: &NodeClient) -> Result { - let info: Value = client.get("/").await?.json().await?; - let did = info["did"] - .as_str() - .context("node missing DID")? - .to_string(); - Ok(did.split(':').next_back().unwrap_or(&did).to_string()) -} - async fn cmd_create( repo: String, url: String, @@ -94,8 +87,8 @@ async fn cmd_create( dir: Option, ) -> Result<()> { let keypair = load_keypair_from_dir(dir.as_deref())?; + let (owner, name) = crate::repo::resolve_owner_repo_pair(&repo, &node, dir.as_deref()).await?; let client = NodeClient::new(&node, Some(keypair)); - let owner = resolve_owner(&client).await?; let event_list: Vec<&str> = events.split(',').map(str::trim).collect(); @@ -106,7 +99,7 @@ async fn cmd_create( }))?; let resp = client - .post(&format!("/api/v1/repos/{owner}/{repo}/hooks"), &payload) + .post(&format!("/api/v1/repos/{owner}/{name}/hooks"), &payload) .await .context("failed to connect to node")?; let status = resp.status(); @@ -143,18 +136,31 @@ async fn cmd_create( Ok(()) } -async fn cmd_list(repo: String, node: String) -> Result<()> { - let client = NodeClient::new(&node, None); - let owner = resolve_owner(&client).await?; +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 (owner, name) = crate::repo::resolve_owner_repo_pair(&repo, &node, dir.as_deref()).await?; + let client = NodeClient::new(&node, Some(keypair)); + + // 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 = client + .get_signed(&format!("/api/v1/repos/{owner}/{name}/hooks")) + .await?; + let status = resp.status(); + let body: Value = resp.json().await.context("invalid JSON")?; - let resp: Value = client - .get(&format!("/api/v1/repos/{owner}/{repo}/hooks")) - .await? - .json() - .await - .context("invalid JSON")?; + if !status.is_success() { + let msg = body["message"].as_str().unwrap_or("unknown error"); + anyhow::bail!("list webhooks failed ({status}): {msg}"); + } - let hooks = resp["webhooks"].as_array().cloned().unwrap_or_default(); + let hooks = body + .get("webhooks") + .and_then(Value::as_array) + .cloned() + .context("node response missing webhooks array")?; if hooks.is_empty() { println!("No webhooks for {repo}"); return Ok(()); @@ -185,13 +191,13 @@ async fn cmd_list(repo: String, node: String) -> Result<()> { async fn cmd_delete(repo: String, id: String, node: String, dir: Option) -> Result<()> { let keypair = load_keypair_from_dir(dir.as_deref())?; + let (owner, name) = crate::repo::resolve_owner_repo_pair(&repo, &node, dir.as_deref()).await?; let client = NodeClient::new(&node, Some(keypair)); - let owner = resolve_owner(&client).await?; let payload = serde_json::to_vec(&serde_json::json!({}))?; let resp = client .delete( - &format!("/api/v1/repos/{owner}/{repo}/hooks/{id}"), + &format!("/api/v1/repos/{owner}/{name}/hooks/{id}"), &payload, ) .await @@ -337,33 +343,170 @@ 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 .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":[]}"#) .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 .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}]}"#) .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_owner_comes_from_keypair_not_node_did() { + // The owner segment must be derived from the caller's keypair (loaded + // from --dir), NOT the node root DID. mock_root returns a different DID; + // if the code regressed to using it, this path mock would not match. + let mut server = mockito::Server::new_async().await; + let (dir, kp) = tmp_identity(); + let _root = mock_root(&mut server).await; + + let did = kp.did().to_string(); + let short = did.split(':').next_back().unwrap_or(&did).to_string(); + + let _m = server + .mock( + "GET", + mockito::Matcher::Regex(format!(r"/api/v1/repos/{short}/my-repo/hooks$")), + ) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"webhooks":[]}"#) + .expect(1) + .create_async() + .await; + + cmd_list( + "my-repo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap(); + _m.assert_async().await; + } + + #[tokio::test] + async fn test_list_honors_owner_repo_arg() { + // An explicit `owner/repo` argument is split and used verbatim. + let mut server = mockito::Server::new_async().await; + let (dir, _kp) = tmp_identity(); + let _root = mock_root(&mut server).await; + + let _m = server + .mock( + "GET", + mockito::Matcher::Regex(r"/api/v1/repos/someoneelse/their-repo/hooks$".to_string()), + ) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"webhooks":[]}"#) + .expect(1) + .create_async() + .await; + + cmd_list( + "someoneelse/their-repo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap(); + _m.assert_async().await; + } + + #[tokio::test] + async fn test_list_webhooks_non_success_status_errors() { + // A 403 (or any non-2xx) JSON body has no `webhooks` field; cmd_list must + // surface the failure, not print "No webhooks" and exit 0. + let mut server = mockito::Server::new_async().await; + let (dir, _kp) = tmp_identity(); + let _root = mock_root(&mut server).await; + + let _m = server + .mock("GET", mockito::Matcher::Regex(r"/hooks$".to_string())) + .with_status(403) + .with_header("content-type", "application/json") + .with_body(r#"{"message":"not the owner"}"#) + .create_async() + .await; + + let err = cmd_list( + "my-repo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap_err(); + assert!(err.to_string().contains("not the owner")); + } + + #[tokio::test] + async fn test_list_webhooks_2xx_missing_array_errors() { + // A 2xx body without a `webhooks` array signals a node/API contract + // regression. cmd_list must error rather than print "No webhooks". + let mut server = mockito::Server::new_async().await; + let (dir, _kp) = tmp_identity(); + let _root = mock_root(&mut server).await; + + 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#"{"ok":true}"#) + .create_async() + .await; + + let err = cmd_list( + "my-repo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap_err(); + assert!(err.to_string().contains("missing webhooks array")); } // ── delete ───────────────────────────────────────────────────────