From 30e68564e1ac899c03c03dac42a67ebb0067cd16 Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Mon, 10 Aug 2026 17:14:47 +0530 Subject: [PATCH 1/5] fix(node): close quarantine bypass on encrypted blobs and visibility routes Rebuilt per review: this branch now carries ONLY the quarantine fix (dropped the unrelated task-claim and Windows fetch commits, which live in #275/#312, and the author-local .cursor symlink). The ipfs.rs and db-layer halves of the original commit were superseded by main's paged scan, which already carries per-row quarantine flags; what remains is routing encrypted-blob and visibility handlers through authorize_repo_read so a quarantined repo is an opaque 404 there too, plus the three regression tests for exactly that surface. --- crates/gitlawb-node/src/api/encrypted.rs | 41 +-- crates/gitlawb-node/src/api/mod.rs | 7 +- crates/gitlawb-node/src/api/visibility.rs | 48 ++-- crates/gitlawb-node/src/test_support.rs | 304 ++++++++++++++++++++++ 4 files changed, 335 insertions(+), 65 deletions(-) diff --git a/crates/gitlawb-node/src/api/encrypted.rs b/crates/gitlawb-node/src/api/encrypted.rs index d9fa52a45..5bc2da6f2 100644 --- a/crates/gitlawb-node/src/api/encrypted.rs +++ b/crates/gitlawb-node/src/api/encrypted.rs @@ -6,29 +6,24 @@ use axum::Json; use crate::auth::AuthenticatedDid; use crate::error::{AppError, Result}; use crate::state::AppState; -use crate::visibility::{visibility_check, Decision}; /// GET /api/v1/repos/{owner}/{repo}/encrypted-blobs /// Returns [{oid, cid}] for every encrypted blob in the repo, to any caller who /// can read the repo. Not recipient-scoped: recipient identities are not stored, /// so access control here is repo readability and decryption is gated by the /// envelope crypto (only a real recipient can open an envelope). +/// +/// Quarantined repos are opaque 404 via [`crate::api::authorize_repo_read`] — +/// same as issues/changelogs — so a public-but-quarantined mirror cannot leak +/// its encrypted blob index. pub async fn list_encrypted_blobs( State(state): State, auth: Option>, Path((owner, repo)): Path<(String, String)>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; let 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}"))); - } + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?; let rows = state.db.list_all_encrypted_blobs(&record.id).await?; let blobs: Vec<_> = rows .into_iter() @@ -45,17 +40,9 @@ pub async fn get_encrypted_blob( auth: Option>, Path((owner, repo, oid)): Path<(String, String, String)>, ) -> 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 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}/{oid}"))); - } + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?; let cid = state .db .encrypted_blob_cid(&record.id, &oid) @@ -81,17 +68,9 @@ pub async fn replicate_encrypted_blobs( auth: Option>, Path((owner, repo)): Path<(String, String)>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; let 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}"))); - } + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?; let rows = state.db.list_all_encrypted_blobs(&record.id).await?; let blobs: Vec<_> = rows .into_iter() diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index df10175a9..0b0338fb1 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -242,13 +242,16 @@ mod authz_guard { // PRE-GATED — already owner-gated, in-scope group; guard the gate itself (protect, "protect_branch", "did_matches("), (protect, "unprotect_branch", "did_matches("), + (visibility, "set_visibility", "authorize_repo_read("), (visibility, "set_visibility", "require_owner("), + (visibility, "remove_visibility", "authorize_repo_read("), (visibility, "remove_visibility", "require_owner("), + (visibility, "list_visibility", "authorize_repo_read("), (visibility, "list_visibility", "require_owner("), ]; - // The visibility rows prove require_owner is CALLED; this proves the helper - // itself does DID-safe matching, not a raw/trailing-segment compare. + // Rows above prove each visibility handler calls authorize_repo_read and + // require_owner; this proves the helper itself does DID-safe matching. assert!( fn_body(visibility, "require_owner").contains("did_matches("), "visibility::require_owner must use did_matches for DID-safe owner matching" diff --git a/crates/gitlawb-node/src/api/visibility.rs b/crates/gitlawb-node/src/api/visibility.rs index b00d769ba..b5a7cdda7 100644 --- a/crates/gitlawb-node/src/api/visibility.rs +++ b/crates/gitlawb-node/src/api/visibility.rs @@ -85,11 +85,10 @@ pub async fn set_visibility( Path((owner, repo)): Path<(String, String)>, Json(req): Json, ) -> Result<(StatusCode, Json)> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + // Quarantine first (via authorize_repo_read), then owner — same posture as + // list_visibility so a quarantined repo cannot be mutated while reads 404. + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, Some(&auth.0), "/").await?; require_owner(&record, &auth.0)?; validate_path_glob(&req.path_glob)?; @@ -141,11 +140,8 @@ pub async fn remove_visibility( Path((owner, repo)): Path<(String, String)>, Json(req): Json, ) -> Result> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, Some(&auth.0), "/").await?; require_owner(&record, &auth.0)?; state @@ -171,14 +167,12 @@ pub async fn list_visibility( Extension(auth): Extension, Path((owner, repo)): Path<(String, String)>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + // Quarantine first (via authorize_repo_read), then owner — a quarantined + // mirror must be opaque even to a caller matching owner_did. + let (record, rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, Some(&auth.0), "/").await?; require_owner(&record, &auth.0)?; - let rules = state.db.list_visibility_rules(&record.id).await?; let rules_json: Vec<_> = rules .into_iter() .map(|r| { @@ -205,28 +199,18 @@ pub async fn list_visibility( /// denied one (`reinclude`), so a clean-clone client can sparse-exclude the /// denied subtrees while re-including the allowed nested paths. Unlike /// `list_visibility` this is not owner-gated and never exposes reader_dids. +/// +/// Quarantined repos are opaque 404 via [`crate::api::authorize_repo_read`] — +/// same posture as encrypted-blob discovery — so admission and private-subtree +/// layout are not disclosed to anon, owner, or peers. pub async fn withheld_paths( State(state): State, auth: Option>, Path((owner, repo)): Path<(String, String)>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; - - let rules = state.db.list_visibility_rules(&record.id).await?; let caller = auth.as_ref().map(|e| e.0 .0.as_str()); - - // Whole-repo read gate: a caller who cannot read "/" gets repo-not-found, - // matching the git read endpoints, so this never discloses a private repo's - // existence or its path layout to an unauthorized caller. - if crate::visibility::visibility_check(&rules, record.is_public, &record.owner_did, caller, "/") - == crate::visibility::Decision::Deny - { - return Err(AppError::RepoNotFound(format!("{owner}/{repo}"))); - } + let (record, rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?; let withheld = crate::visibility::withheld_globs(&rules, record.is_public, &record.owner_did, caller); diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 430c06002..741d3e575 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -14612,6 +14612,310 @@ mod tests { assert_eq!(resp.status(), StatusCode::NOT_FOUND); } + /// Public-but-quarantined repos must not expose encrypted blob indexes + /// (discovery or replicate). Previously these handlers used + /// `visibility_check` alone and skipped the quarantine short-circuit in + /// `authorize_repo_read`. + #[sqlx::test] + async fn encrypted_blobs_quarantined_repo_opaque_404(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zENCQUAROWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let short = owner.split(':').next_back().unwrap(); + let mut repo = seed_private_repo(owner, "enc-quar"); + repo.is_public = true; + let repo_id = repo.id.clone(); + state.db.create_repo(&repo).await.unwrap(); + state + .db + .record_encrypted_blob(&repo_id, "deadbeef", "bafybeiquarantinedcid", "") + .await + .unwrap(); + state.db.set_repo_quarantine(&repo_id, true).await.unwrap(); + + let router = crate::server::build_router(state.clone()); + for suffix in [ + "encrypted-blobs", + "encrypted-blobs/replicate", + "encrypted-blob/deadbeef", + ] { + let path = format!("/api/v1/repos/{short}/enc-quar/{suffix}"); + let resp = router.clone().oneshot(anon_get(&path)).await.unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "quarantined public repo must 404 on {path}" + ); + let body = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let text = String::from_utf8_lossy(&body); + assert!( + !text.contains("bafybeiquarantinedcid") && !text.contains("deadbeef"), + "blob index must not leak on quarantine 404 for {path}: {text}" + ); + } + + // Owner (full did:key and bare key) must also 404 — quarantine is not a + // visibility deny that the owner short-circuit can bypass. + for caller in [owner, short] { + for suffix in [ + "encrypted-blobs", + "encrypted-blobs/replicate", + "encrypted-blob/deadbeef", + ] { + let path = format!("/api/v1/repos/{short}/enc-quar/{suffix}"); + let resp = router + .clone() + .oneshot(signed_request_as(caller, Method::GET, &path, Body::empty())) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "owner form {caller} must not read quarantined {path}" + ); + } + } + + // Control: clear quarantine → all three discovery surfaces admit again. + // `encrypted-blob/{oid}` may 500 without a live IPFS node after the gate + // opens; assert it is not the quarantine 404. + state.db.set_repo_quarantine(&repo_id, false).await.unwrap(); + for suffix in ["encrypted-blobs", "encrypted-blobs/replicate"] { + let path = format!("/api/v1/repos/{short}/enc-quar/{suffix}"); + let resp = router.clone().oneshot(anon_get(&path)).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK, "released must admit {path}"); + let body = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let text = String::from_utf8_lossy(&body); + assert!( + text.contains("bafybeiquarantinedcid"), + "released repo must expose blob index on {path}: {text}" + ); + } + let get_path = format!("/api/v1/repos/{short}/enc-quar/encrypted-blob/deadbeef"); + let resp = router.oneshot(anon_get(&get_path)).await.unwrap(); + assert_ne!( + resp.status(), + StatusCode::NOT_FOUND, + "released get must clear the quarantine 404 (may 5xx without IPFS)" + ); + } + + /// Mirror-admission path (`upsert_mirror_repo` + slash-form id) must also + /// opaque-404 encrypted discovery while quarantined. + #[sqlx::test] + async fn encrypted_blobs_quarantined_mirror_admission_opaque_404(pool: PgPool) { + let state = test_state(pool).await; + let short = "z6MkEncMirrorAdmitAAAAAAAAAAAAAAAAAAAA"; + state + .db + .upsert_mirror_repo(short, "enc-mirror", "/tmp/enc-mirror", None, true) + .await + .unwrap(); + let repo_id = format!("{short}/enc-mirror"); + state + .db + .record_encrypted_blob(&repo_id, "aabbccdd", "bafybeimirrorcid", "") + .await + .unwrap(); + + let router = crate::server::build_router(state.clone()); + for suffix in [ + "encrypted-blobs", + "encrypted-blobs/replicate", + "encrypted-blob/aabbccdd", + ] { + let path = format!("/api/v1/repos/{short}/enc-mirror/{suffix}"); + let resp = router.clone().oneshot(anon_get(&path)).await.unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "anon must 404 quarantined mirror {path}" + ); + let body = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let text = String::from_utf8_lossy(&body); + assert!( + !text.contains("bafybeimirrorcid") && !text.contains("aabbccdd"), + "blob index must not leak on quarantine 404 for {path}: {text}" + ); + + let resp = router + .clone() + .oneshot(signed_request_as(short, Method::GET, &path, Body::empty())) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "bare-key mirror owner must not read quarantined {path}" + ); + } + + state.db.set_repo_quarantine(&repo_id, false).await.unwrap(); + for suffix in ["encrypted-blobs", "encrypted-blobs/replicate"] { + let path = format!("/api/v1/repos/{short}/enc-mirror/{suffix}"); + let resp = router.clone().oneshot(anon_get(&path)).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK, "released must admit {path}"); + } + let get_path = format!("/api/v1/repos/{short}/enc-mirror/encrypted-blob/aabbccdd"); + let resp = router.oneshot(anon_get(&get_path)).await.unwrap(); + assert_ne!( + resp.status(), + StatusCode::NOT_FOUND, + "released get must clear the quarantine 404 (may 5xx without IPFS)" + ); + } + + /// `withheld-paths` and `list_visibility` must share the quarantine gate. + #[sqlx::test] + async fn withheld_paths_and_list_visibility_quarantine_opaque(pool: PgPool) { + use crate::db::VisibilityMode; + + let state = test_state(pool).await; + let owner = "did:key:zVISQUAROWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let short = owner.split(':').next_back().unwrap(); + let mut repo = seed_private_repo(owner, "vis-quar"); + repo.is_public = true; + let repo_id = repo.id.clone(); + state.db.create_repo(&repo).await.unwrap(); + state + .db + .set_visibility_rule(&repo_id, "/secret/**", VisibilityMode::B, &[], owner) + .await + .unwrap(); + state.db.set_repo_quarantine(&repo_id, true).await.unwrap(); + + let router = crate::server::build_router(state.clone()); + let withheld = format!("/api/v1/repos/{short}/vis-quar/withheld-paths"); + let resp = router.clone().oneshot(anon_get(&withheld)).await.unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "anon withheld-paths must 404 while quarantined" + ); + let body = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let text = String::from_utf8_lossy(&body); + assert!( + !text.contains("/secret"), + "withheld layout must not leak: {text}" + ); + + for caller in [owner, short] { + let resp = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/withheld-paths", + axum::routing::get(crate::api::visibility::withheld_paths), + ) + .layer(axum::middleware::from_fn(crate::auth::optional_signature)) + .with_state(state.clone()) + .oneshot(signed_request_as( + caller, + Method::GET, + &withheld, + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "owner form {caller} withheld-paths must 404 while quarantined" + ); + + let vis = format!("/api/v1/repos/{short}/vis-quar/visibility"); + let resp = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/visibility", + axum::routing::get(crate::api::visibility::list_visibility), + ) + .with_state(state.clone()) + .oneshot(signed_request_as(caller, Method::GET, &vis, Body::empty())) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "owner form {caller} list_visibility must 404 while quarantined" + ); + + // PUT/DELETE must also opaque-404 while quarantined (no rule mutation). + let put_body = Body::from(r#"{"path_glob":"/extra/**","reader_dids":[]}"#); + let resp = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/visibility", + axum::routing::put(crate::api::visibility::set_visibility), + ) + .with_state(state.clone()) + .oneshot(signed_request_as(caller, Method::PUT, &vis, put_body)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "owner form {caller} set_visibility must 404 while quarantined" + ); + + let del_body = Body::from(r#"{"path_glob":"/secret/**"}"#); + let resp = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/visibility", + axum::routing::delete(crate::api::visibility::remove_visibility), + ) + .with_state(state.clone()) + .oneshot(signed_request_as(caller, Method::DELETE, &vis, del_body)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "owner form {caller} remove_visibility must 404 while quarantined" + ); + } + + state.db.set_repo_quarantine(&repo_id, false).await.unwrap(); + let resp = router.clone().oneshot(anon_get(&withheld)).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + let body = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let text = String::from_utf8_lossy(&body); + assert!( + text.contains("/secret"), + "released withheld-paths must return globs: {text}" + ); + + // Same release control for list_visibility (owner-gated after gate opens). + let vis = format!("/api/v1/repos/{short}/vis-quar/visibility"); + let resp = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/visibility", + axum::routing::get(crate::api::visibility::list_visibility), + ) + .with_state(state.clone()) + .oneshot(signed_request_as(owner, Method::GET, &vis, Body::empty())) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "released list_visibility must admit owner" + ); + let body = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let text = String::from_utf8_lossy(&body); + assert!( + text.contains("/secret"), + "released list_visibility must return rules: {text}" + ); + } + #[sqlx::test] async fn repo_gate_public_repo_anon_read_admitted(pool: PgPool) { struct DirGuard(std::path::PathBuf); From 55242a892dbe6524b637b787d231baf5b0230082 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Mon, 9 Nov 2026 18:48:02 -0600 Subject: [PATCH 2/5] fix(node): gate sibling mutation endpoints on quarantine, reconcile 404 contract in gl clone (#276) --- crates/gitlawb-node/src/api/issues.rs | 63 +++++++++---------------- crates/gitlawb-node/src/api/labels.rs | 14 ++---- crates/gitlawb-node/src/api/mod.rs | 44 +++++++++++++++-- crates/gitlawb-node/src/api/protect.rs | 14 ++---- crates/gitlawb-node/src/api/pulls.rs | 14 ++---- crates/gitlawb-node/src/api/webhooks.rs | 14 ++---- crates/gitlawb-node/src/sync.rs | 13 ++++- crates/gitlawb-node/src/test_support.rs | 58 ++++++++++++++++++++++- crates/gl/src/clone.rs | 26 +++++++++- 9 files changed, 171 insertions(+), 89 deletions(-) diff --git a/crates/gitlawb-node/src/api/issues.rs b/crates/gitlawb-node/src/api/issues.rs index f214d96ab..7d1359c8c 100644 --- a/crates/gitlawb-node/src/api/issues.rs +++ b/crates/gitlawb-node/src/api/issues.rs @@ -238,49 +238,32 @@ pub async fn close_issue( headers: axum::http::HeaderMap, crate::rate_limit::PeerAddr(peer): crate::rate_limit::PeerAddr, ) -> Result> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; - - // READ-GATE before any snapshot work or rate charging. The author fallback below - // needs the issue blob, which needs the repo tree, so authorship cannot be established - // without a download; but a caller who cannot even READ the repo must be - // stopped here, cheaply, before any Tigris transfer or extraction happens. - // Without this, any signed non-owner could issue parallel close requests for - // arbitrary issue ids and drive unbounded downloads and blocking extraction - // (a disposable-identity DoS), because the route has no other pre-authorization. + // READ-GATE before any snapshot work or rate charging, through the shared + // authorize_repo_read gate so quarantined repos are opaque here too. The + // author fallback below needs the issue blob, which needs the repo tree, so + // authorship cannot be established without a download; but a caller who + // cannot even READ the repo must be stopped here, cheaply, before any + // Tigris transfer or extraction happens. Without this, any signed non-owner + // could issue parallel close requests for arbitrary issue ids and drive + // unbounded downloads and blocking extraction (a disposable-identity DoS), + // because the route has no other pre-authorization. // - // Rate limiting runs AFTER this gate so a 429 cannot distinguish a hidden repo - // from a missing one (INV-12 read-denial status contract). + // Rate limiting runs AFTER this gate so a 429 cannot distinguish a hidden + // repo from a missing one (INV-12 read-denial status contract). // // mirror-rows-handled: a repo row synced from a peer is stored public and - // carries none of the owner's visibility rules, so for such a row this check - // can only return allow. That is deliberate rather than overlooked, and it is - // the same verdict every other read gate in this API reaches for one. Refusing - // instead would deny the repo's real owner and the issue's real author on any - // node whose only copy of the repo is a synced one, which is an ordinary state - // here, and it would not buy the protection it appears to, because a synced - // row's recorded owner comes from the peer that sent it. The expensive work - // this check guards is bounded ahead of it by the per-IP limiter and the read - // pool slot taken below, and the authoritative owner-or-author decision still - // runs below and again under the write lock. - { - // mirror-rows-handled: synced mirror rows carry no owner rules; read gate only. - let rules = state.db.list_visibility_rules(&record.id).await?; - let caller = auth.0.as_str(); - if crate::visibility::visibility_check( - &rules, - record.is_public, - &record.owner_did, - Some(caller), - "/", - ) == crate::visibility::Decision::Deny - { - return Err(AppError::RepoNotFound(format!("{owner}/{repo}"))); - } - } + // carries none of the owner's visibility rules, so for such a row this gate + // can only return allow. That is deliberate rather than overlooked, and it + // is the same verdict every other read gate in this API reaches for one. + // Refusing instead would deny the repo's real owner and the issue's real + // author on any node whose only copy of the repo is a synced one, which is + // an ordinary state here, and it would not buy the protection it appears + // to, because a synced row's recorded owner comes from the peer that sent + // it. The expensive work this check guards is bounded ahead of it by the + // per-IP limiter and the read pool slot taken below, and the authoritative + // owner-or-author decision still runs below and again under the write lock. + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, Some(auth.0.as_str()), "/").await?; // Per-IP flood brake, layered on the same shared limiter and trusted-proxy // policy as the push advertisement. The pre-lock snapshot downloads the diff --git a/crates/gitlawb-node/src/api/labels.rs b/crates/gitlawb-node/src/api/labels.rs index 85e07bc06..888a94525 100644 --- a/crates/gitlawb-node/src/api/labels.rs +++ b/crates/gitlawb-node/src/api/labels.rs @@ -44,11 +44,8 @@ pub async fn add_label( ) -> Result<(StatusCode, Json)> { let label = canonicalize_label(&req.label)?; - let record = state - .db - .get_repo(&owner, &name) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?; + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &name, Some(auth.0.as_str()), "/").await?; crate::api::require_repo_owner(&record, &auth.0)?; let added = state.db.add_label(&record.id, &label).await?; @@ -71,11 +68,8 @@ pub async fn remove_label( ) -> Result> { let label = canonicalize_label(&label)?; - let record = state - .db - .get_repo(&owner, &name) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?; + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &name, Some(auth.0.as_str()), "/").await?; crate::api::require_repo_owner(&record, &auth.0)?; let removed = state.db.remove_label(&record.id, &label).await?; diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index 71f8bc0ca..c65f80401 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -177,13 +177,22 @@ mod authz_guard { "\nasync fn ", "\npub fn ", "\nfn ", + "#[cfg(test)]", ] .iter() .filter_map(|p| rest[1..].find(p).map(|i| i + 1)) .min() .unwrap_or(rest.len()); - rest[..end] - .lines() + // Strip block comments too: a marker surviving only inside /* */ would + // otherwise satisfy a row with the real gate deleted. + let mut body = rest[..end].to_string(); + while let Some(open) = body.find("/*") { + match body[open..].find("*/").map(|i| open + i + 2) { + Some(close) => body.replace_range(open..close, ""), + None => break, + } + } + body.lines() .filter(|l| !l.trim_start().starts_with("//")) .collect::>() .join("\n") @@ -209,9 +218,14 @@ mod authz_guard { // (source, handler, expected gate marker) let rows: &[(&str, &str, &str)] = &[ - // Bucket A — owner-gate (require_repo_owner -> 403) + // Bucket A — owner-gate (require_repo_owner -> 403), now behind + // the shared read gate first so a hidden repo 404s for every + // caller class before the owner check can answer 403. + (pulls, "merge_pr", "authorize_repo_read("), (pulls, "merge_pr", "require_repo_owner("), + (webhooks, "create_webhook", "authorize_repo_read("), (webhooks, "create_webhook", "require_repo_owner("), + (webhooks, "delete_webhook", "authorize_repo_read("), (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 @@ -220,10 +234,15 @@ mod authz_guard { // (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", "authorize_repo_read("), (labels, "add_label", "require_repo_owner("), + (labels, "remove_label", "authorize_repo_read("), (labels, "remove_label", "require_repo_owner("), - // Bucket A' — owner OR author (did_matches against the author) + // Bucket A' — owner OR author (did_matches against the author), + // also behind the shared read gate first. + (pulls, "close_pr", "authorize_repo_read("), (pulls, "close_pr", "did_matches("), + (issues, "close_issue", "authorize_repo_read("), (issues, "close_issue", "did_matches("), // Bucket B — read-gate (authorize_repo_read) (pulls, "create_review", "authorize_repo_read("), @@ -269,8 +288,10 @@ mod authz_guard { (bounties, "dispute_bounty", "did_matches("), (replicas, "register_replica", "did_matches("), (replicas, "unregister_replica", "replica_did = &auth.0"), - // PRE-GATED — already owner-gated, in-scope group; guard the gate itself + // PRE-GATED — owner-gated behind the shared read gate; guard both + (protect, "protect_branch", "authorize_repo_read("), (protect, "protect_branch", "did_matches("), + (protect, "unprotect_branch", "authorize_repo_read("), (protect, "unprotect_branch", "did_matches("), (visibility, "set_visibility", "authorize_repo_read("), (visibility, "set_visibility", "require_owner("), @@ -287,6 +308,19 @@ mod authz_guard { "visibility::require_owner must use did_matches for DID-safe owner matching" ); + // Order matters as much as presence: hoisting require_owner above the + // read gate would 403 a non-owner on a hidden repo, an existence + // oracle. Pin quarantine-then-owner on every handler that takes both. + for func in ["set_visibility", "remove_visibility", "list_visibility"] { + let body = fn_body(visibility, func); + let gate = body.find("authorize_repo_read(").expect("gate marker"); + let owner = body.find("require_owner(").expect("owner marker"); + assert!( + gate < owner, + "visibility::{func} must run authorize_repo_read before require_owner" + ); + } + // The CID read surface (#173) enforces its gate inside the shared // `gate_and_serve`, which BOTH the provenance path and the legacy scan call, so // the markers must live there (the get_by_cid row above only proves delegation). diff --git a/crates/gitlawb-node/src/api/protect.rs b/crates/gitlawb-node/src/api/protect.rs index 4dd08b004..2092584f0 100644 --- a/crates/gitlawb-node/src/api/protect.rs +++ b/crates/gitlawb-node/src/api/protect.rs @@ -17,11 +17,8 @@ pub async fn protect_branch( Extension(auth): Extension, Path((owner, repo, branch)): Path<(String, String, String)>, ) -> Result<(StatusCode, Json)> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, Some(auth.0.as_str()), "/").await?; // Only the repo owner can protect branches (DID-safe match, shared idiom). let caller = &auth.0; @@ -51,11 +48,8 @@ pub async fn unprotect_branch( Extension(auth): Extension, Path((owner, repo, branch)): Path<(String, String, String)>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, Some(auth.0.as_str()), "/").await?; let caller = &auth.0; if !crate::api::did_matches(caller, &record.owner_did) { diff --git a/crates/gitlawb-node/src/api/pulls.rs b/crates/gitlawb-node/src/api/pulls.rs index 1adf3f3b5..d9efd13d1 100644 --- a/crates/gitlawb-node/src/api/pulls.rs +++ b/crates/gitlawb-node/src/api/pulls.rs @@ -199,11 +199,8 @@ pub async fn merge_pr( Extension(auth): Extension, Path((owner, name, number)): Path<(String, String, i64)>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &name) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?; + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &name, Some(auth.0.as_str()), "/").await?; // Owner-only merge (N7). Merging writes the served tree, so this is the same // trust boundary as owner-only push; it subsumes branch protection (a @@ -274,11 +271,8 @@ pub async fn close_pr( Extension(auth): Extension, Path((owner, name, number)): Path<(String, String, i64)>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &name) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?; + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &name, Some(auth.0.as_str()), "/").await?; let pr = state .db diff --git a/crates/gitlawb-node/src/api/webhooks.rs b/crates/gitlawb-node/src/api/webhooks.rs index 7cecd2021..88529ca5c 100644 --- a/crates/gitlawb-node/src/api/webhooks.rs +++ b/crates/gitlawb-node/src/api/webhooks.rs @@ -33,11 +33,8 @@ pub async fn create_webhook( Path((owner, name)): Path<(String, String)>, Json(req): Json, ) -> Result<(StatusCode, Json)> { - let record = state - .db - .get_repo(&owner, &name) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?; + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &name, Some(auth.0.as_str()), "/").await?; crate::api::require_repo_owner(&record, &auth.0)?; // Gate the target through the same hardened public-host validator the peer @@ -118,11 +115,8 @@ pub async fn delete_webhook( Extension(auth): Extension, Path((owner, name, id)): Path<(String, String, String)>, ) -> Result> { - let record = state - .db - .get_repo(&owner, &name) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?; + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &name, Some(auth.0.as_str()), "/").await?; crate::api::require_repo_owner(&record, &auth.0)?; // Verify the webhook belongs to this repo diff --git a/crates/gitlawb-node/src/sync.rs b/crates/gitlawb-node/src/sync.rs index 041336655..20a8f0b59 100644 --- a/crates/gitlawb-node/src/sync.rs +++ b/crates/gitlawb-node/src/sync.rs @@ -578,7 +578,18 @@ async fn replicate_encrypted_blobs( let url = format!("{origin_url}/api/v1/repos/{owner}/{repo}/encrypted-blobs/replicate"); let resp = match client.get(&url).send().await { Ok(r) if r.status().is_success() => r, - _ => return, + // A peer that predates the replicate route answers 404: expected and + // quiet. Anything else is the route existing and refusing or failing, + // which should be observable rather than read as route-absent. + Ok(r) if r.status().as_u16() == 404 => return, + Ok(r) => { + tracing::warn!(repo = %repo, status = %r.status(), "encrypted-blobs replicate refused; skipping"); + return; + } + Err(e) => { + tracing::warn!(repo = %repo, error = %e, "encrypted-blobs replicate request failed; skipping"); + return; + } }; let parsed: ReplicateResponse = match resp.json().await { Ok(p) => p, diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index f6e38c59f..efe7c2e14 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -18923,7 +18923,10 @@ mod tests { "/api/v1/repos/{owner}/{repo}/withheld-paths", axum::routing::get(crate::api::visibility::withheld_paths), ) - .layer(axum::middleware::from_fn(crate::auth::optional_signature)) + .layer(axum::middleware::from_fn_with_state( + crate::auth::SignatureBodyLimit(crate::server::SIGNED_BODY_LIMIT), + crate::auth::optional_signature, + )) .with_state(state.clone()) .oneshot(signed_request_as( caller, @@ -18989,6 +18992,59 @@ mod tests { ); } + // A signed non-owner must get 404 too, never 403: this leg is what pins + // the quarantine-before-owner ordering. If require_owner were hoisted + // above authorize_repo_read the same caller would read Forbidden, an + // existence oracle on a hidden repo. + let stranger = "did:key:zSTRANGERQUARAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let vis = format!("/api/v1/repos/{short}/vis-quar/visibility"); + for (method, path, body) in [ + (Method::GET, withheld.as_str(), Body::empty()), + (Method::GET, vis.as_str(), Body::empty()), + ( + Method::PUT, + vis.as_str(), + Body::from(r#"{"path_glob":"/extra/**","reader_dids":[]}"#), + ), + ( + Method::DELETE, + vis.as_str(), + Body::from(r#"{"path_glob":"/secret/**"}"#), + ), + ] { + let handler: axum::routing::MethodRouter = match method.clone() + { + Method::GET => axum::routing::get(crate::api::visibility::list_visibility), + Method::PUT => axum::routing::put(crate::api::visibility::set_visibility), + _ => axum::routing::delete(crate::api::visibility::remove_visibility), + }; + let router = if path == withheld.as_str() { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/withheld-paths", + axum::routing::get(crate::api::visibility::withheld_paths), + ) + .layer(axum::middleware::from_fn_with_state( + crate::auth::SignatureBodyLimit(crate::server::SIGNED_BODY_LIMIT), + crate::auth::optional_signature, + )) + .with_state(state.clone()) + } else { + Router::new() + .route("/api/v1/repos/{owner}/{repo}/visibility", handler) + .with_state(state.clone()) + }; + let resp = router + .oneshot(signed_request_as(stranger, method.clone(), path, body)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "non-owner {method} {path} must 404 (not 403) while quarantined" + ); + } + state.db.set_repo_quarantine(&repo_id, false).await.unwrap(); let resp = router.clone().oneshot(anon_get(&withheld)).await.unwrap(); assert_eq!(resp.status(), StatusCode::OK); diff --git a/crates/gl/src/clone.rs b/crates/gl/src/clone.rs index 22713fcd6..79cf36a08 100644 --- a/crates/gl/src/clone.rs +++ b/crates/gl/src/clone.rs @@ -267,7 +267,24 @@ async fn fetch_withheld(node: &str, owner: &str, name: &str) -> Result<(Vec r, - Ok(r) if matches!(r.status().as_u16(), 404 | 501) => return Ok((Vec::new(), Vec::new())), + Ok(r) if matches!(r.status().as_u16(), 404 | 501) => { + // Two 404s share this status and must be told apart before the + // legacy empty-glob fallback is taken: a route miss on a node that + // predates withheld-paths has an empty body, while a current node + // returns the JSON AppError body for absent/denied/quarantined + // repos. The JSON body proves the route exists, so the repo is + // unavailable to this caller and cloning cannot proceed. + if r.status().as_u16() == 404 { + let status = r.status(); + let body = r.text().await.unwrap_or_default(); + if !body.trim().is_empty() { + bail!( + "repository {owner}/{name} is not available on this node ({status}): {body}" + ); + } + } + return Ok((Vec::new(), Vec::new())); + } Ok(r) => bail!("withheld-paths lookup failed: {}", r.status()), Err(err) => return Err(err).context("fetching withheld paths"), }; @@ -327,7 +344,12 @@ async fn recover_encrypted_blobs( .await { Ok(r) if r.status().is_success() => r, - _ => return Ok(vec![]), + // Discovery failure is not an empty inventory: a denied, quarantined, + // erroring, or unreachable node must surface so the caller can warn + // and still run the Arweave fallback rather than report a clean clone + // with authorized blobs silently missing. + Ok(r) => bail!("encrypted-blobs inventory unavailable: {}", r.status()), + Err(err) => return Err(err).context("fetching encrypted-blobs inventory"), }; let body: EncryptedBlobsResponse = resp.json().await.context("parsing encrypted-blobs")?; if body.blobs.is_empty() { From 92f75c9c54a5127546ed73da6ef27ac9efc2336b Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Mon, 9 Nov 2026 21:31:17 -0600 Subject: [PATCH 3/5] fix(node): release quarantined mirror fixtures, bound 404 body read in gl clone (#566) --- crates/gitlawb-node/src/api/issues.rs | 51 +++++++++++++++++++++++++++ crates/gl/src/clone.rs | 13 +++++-- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/crates/gitlawb-node/src/api/issues.rs b/crates/gitlawb-node/src/api/issues.rs index 7d1359c8c..7e551898f 100644 --- a/crates/gitlawb-node/src/api/issues.rs +++ b/crates/gitlawb-node/src/api/issues.rs @@ -485,6 +485,15 @@ mod tests { .await .expect("get_repo") .expect("repo exists"); + // upsert_mirror_repo seeds the row quarantined, and the read gate now + // hides quarantined rows as opaque 404 before the owner-or-author check + // this test names. Release it so the stranger is refused by the 403, not + // by the quarantine short-circuit. + state + .db + .set_repo_quarantine(&record.id, false) + .await + .expect("release seed quarantine"); // An independent session holds the repo's write lock for the whole call. let key = crate::git::repo_store::advisory_lock_key_for_test( @@ -686,6 +695,15 @@ mod tests { .await .expect("get_repo") .expect("seeded repo exists"); + // upsert_mirror_repo seeds the admission flag as quarantined, but the + // gate now hides quarantined rows from every caller class. Release it + // so these tests exercise what they name: the owner-or-author check + // after the read gate, not the quarantine short-circuit. + state + .db + .set_repo_quarantine(&record.id, false) + .await + .expect("release seed quarantine"); let _ = owner_did; let path = state .repo_store @@ -711,6 +729,39 @@ mod tests { path } + /// Quarantine precedes every caller class: even the owner gets the opaque + /// 404 on a quarantined repo, and the seeded row's quarantined flag is left + /// set deliberately. The released-mirror variants above exercise the + /// owner-or-author path. + #[sqlx::test] + async fn quarantined_repo_close_is_opaque_404(pool: PgPool) { + let state = crate::test_support::test_state(pool.clone()).await; + let owner_did = "did:key:z6MkT1Owner"; + state + .db + .upsert_mirror_repo("z6MkT1Owner", "tquar", "/unused", None, true) + .await + .expect("seed repo row"); + + let res = close_issue( + axum::extract::State(state.clone()), + axum::Extension(crate::auth::AuthenticatedDid(owner_did.to_string())), + axum::extract::Path(( + "z6MkT1Owner".to_string(), + "tquar".to_string(), + "1".to_string(), + )), + axum::http::HeaderMap::new(), + crate::rate_limit::PeerAddr(Some("203.0.113.70:5000".parse().unwrap())), + ) + .await; + assert!( + matches!(res, Err(crate::error::AppError::RepoNotFound(_))), + "a quarantined repo must 404 close_issue for every caller: {:?}", + res.ok().map(|_| "unexpected success") + ); + } + /// INV-21(c) positive twin 1: the OWNER can still close. The reorder moved the /// owner check above the lock, so this is the arm most likely to have broken, /// and the deny test alone could not see it. diff --git a/crates/gl/src/clone.rs b/crates/gl/src/clone.rs index 79cf36a08..71e8e606a 100644 --- a/crates/gl/src/clone.rs +++ b/crates/gl/src/clone.rs @@ -276,10 +276,17 @@ async fn fetch_withheld(node: &str, owner: &str, name: &str) -> Result<(Vec Date: Tue, 10 Nov 2026 08:19:40 -0600 Subject: [PATCH 4/5] test(node): pin read-before-owner order on every dual-gated handler (#566) --- crates/gitlawb-node/src/api/issues.rs | 7 ++++--- crates/gitlawb-node/src/api/mod.rs | 30 ++++++++++++++++++++------- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/crates/gitlawb-node/src/api/issues.rs b/crates/gitlawb-node/src/api/issues.rs index 7e551898f..eefe168c8 100644 --- a/crates/gitlawb-node/src/api/issues.rs +++ b/crates/gitlawb-node/src/api/issues.rs @@ -729,9 +729,10 @@ mod tests { path } - /// Quarantine precedes every caller class: even the owner gets the opaque - /// 404 on a quarantined repo, and the seeded row's quarantined flag is left - /// set deliberately. The released-mirror variants above exercise the + /// The read gate blocks the owner (and any caller) before the + /// owner-or-author checks: even the owner gets the opaque 404 on a + /// quarantined repo, and the seeded row's quarantined flag is left set + /// deliberately. The released-mirror variants above exercise the /// owner-or-author path. #[sqlx::test] async fn quarantined_repo_close_is_opaque_404(pool: PgPool) { diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index c65f80401..7ab7e13e0 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -308,16 +308,32 @@ mod authz_guard { "visibility::require_owner must use did_matches for DID-safe owner matching" ); - // Order matters as much as presence: hoisting require_owner above the - // read gate would 403 a non-owner on a hidden repo, an existence - // oracle. Pin quarantine-then-owner on every handler that takes both. - for func in ["set_visibility", "remove_visibility", "list_visibility"] { - let body = fn_body(visibility, func); + // Order matters as much as presence: hoisting the owner/author check + // above the read gate would 403 a non-owner on a hidden repo, an + // existence oracle. Pin read-gate-then-owner on every handler that + // takes both; the owner marker differs per bucket (require_repo_owner + // for owner-only, require_owner for visibility, did_matches for the + // owner-or-author and protect paths). + for (src, func, owner_marker) in [ + (visibility, "set_visibility", "require_owner("), + (visibility, "remove_visibility", "require_owner("), + (visibility, "list_visibility", "require_owner("), + (pulls, "merge_pr", "require_repo_owner("), + (webhooks, "create_webhook", "require_repo_owner("), + (webhooks, "delete_webhook", "require_repo_owner("), + (labels, "add_label", "require_repo_owner("), + (labels, "remove_label", "require_repo_owner("), + (pulls, "close_pr", "did_matches("), + (issues, "close_issue", "did_matches("), + (protect, "protect_branch", "did_matches("), + (protect, "unprotect_branch", "did_matches("), + ] { + let body = fn_body(src, func); let gate = body.find("authorize_repo_read(").expect("gate marker"); - let owner = body.find("require_owner(").expect("owner marker"); + let owner = body.find(owner_marker).expect("owner marker"); assert!( gate < owner, - "visibility::{func} must run authorize_repo_read before require_owner" + "{func} must run authorize_repo_read before {owner_marker}" ); } From 344b194ec087fb6924a82d7cff2b099dbdab25b3 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Tue, 10 Nov 2026 09:03:29 -0600 Subject: [PATCH 5/5] test(node): pin list_webhooks in the order loop, close-owner second assert, tighter release legs (#566) --- crates/gitlawb-node/src/api/mod.rs | 11 +++++++++++ crates/gitlawb-node/src/test_support.rs | 16 ++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index 7ab7e13e0..a7c388905 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -319,6 +319,7 @@ mod authz_guard { (visibility, "remove_visibility", "require_owner("), (visibility, "list_visibility", "require_owner("), (pulls, "merge_pr", "require_repo_owner("), + (webhooks, "list_webhooks", "require_repo_owner("), (webhooks, "create_webhook", "require_repo_owner("), (webhooks, "delete_webhook", "require_repo_owner("), (labels, "add_label", "require_repo_owner("), @@ -335,6 +336,16 @@ mod authz_guard { gate < owner, "{func} must run authorize_repo_read before {owner_marker}" ); + // The owner-or-author and protect handlers also carry + // require_repo_owner: a hoist of that call above the read gate is + // the same 403-vs-404 oracle, so pin it too where it exists. + if func == "close_pr" || func == "close_issue" { + let owner_gate = body.find("require_repo_owner(").expect("owner marker"); + assert!( + gate < owner_gate, + "{func} must run authorize_repo_read before require_repo_owner(" + ); + } } // The CID read surface (#173) enforces its gate inside the shared diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index efe7c2e14..6747124f2 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -18807,10 +18807,10 @@ mod tests { } let get_path = format!("/api/v1/repos/{short}/enc-quar/encrypted-blob/deadbeef"); let resp = router.oneshot(anon_get(&get_path)).await.unwrap(); - assert_ne!( - resp.status(), - StatusCode::NOT_FOUND, - "released get must clear the quarantine 404 (may 5xx without IPFS)" + assert!( + resp.status() == StatusCode::OK || resp.status().is_server_error(), + "released get must admit (OK) or reach the backend (5xx); a 4xx means the denial path still fires: {}", + resp.status() ); } @@ -18874,10 +18874,10 @@ mod tests { } let get_path = format!("/api/v1/repos/{short}/enc-mirror/encrypted-blob/aabbccdd"); let resp = router.oneshot(anon_get(&get_path)).await.unwrap(); - assert_ne!( - resp.status(), - StatusCode::NOT_FOUND, - "released get must clear the quarantine 404 (may 5xx without IPFS)" + assert!( + resp.status() == StatusCode::OK || resp.status().is_server_error(), + "released get must admit (OK) or reach the backend (5xx); a 4xx means the denial path still fires: {}", + resp.status() ); }