fix(node): gate /hooks and sibling read surfaces leaking private repo metadata (#94)

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,15 +52,39 @@ pub async fn list_repo_events(
     State(state): State<AppState>,
     Path((owner, repo_name)): Path<(String, String)>,
     Query(params): Query<HashMap<String, String>>,
+    auth: Option<Extension<AuthenticatedDid>>,
 ) -> Result<Json<serde_json::Value>> {
     let limit = params
         .get("limit")
         .and_then(|v| v.parse::<i64>().ok())
         .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"),

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");
@@ -173,6 +174,11 @@ 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. 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("),
             // Bucket A' — owner OR author (did_matches against the author)
@@ -186,6 +192,14 @@ 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("),
+            (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("),

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<AppState>,
     Path((owner, repo)): Path<(String, String)>,
+    auth: Option<Extension<AuthenticatedDid>>,
 ) -> Result<Json<serde_json::Value>> {
-    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?;
 

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<AppState>,
     Path((owner, repo)): Path<(String, String)>,
+    auth: Option<Extension<AuthenticatedDid>>,
 ) -> Result<Json<serde_json::Value>> {
-    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?;
 

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<AppState>,
     Path((owner, name)): Path<(String, String)>,
+    auth: Option<Extension<AuthenticatedDid>>,
 ) -> Result<Json<serde_json::Value>> {
-    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