From a4da137d11390eb61281d99e388a13ee4e82c097 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 13 Sep 2026 12:02:46 -0500 Subject: [PATCH 1/2] fix(node): gate id-keyed bounty mutations on repo read before status checks submit, approve, cancel, and dispute on /api/v1/bounties/{id}/* looked the bounty up and ran status and participant checks without gating on the bounty's repo read visibility. A signed stranger holding an id got a distinguishable 400/403 on a private-repo bounty where an absent id gets 404, so the routes were an existence and lifecycle oracle. Insert the same authorize_repo_read block get_bounty and claim_bounty already carry, right after the lookup, so a denied caller sees the not-found shape either way. Participant checks are unchanged; on a public repo a stranger still reaches them. Closes #341 --- crates/gitlawb-node/src/api/bounties.rs | 84 ++++++++--- crates/gitlawb-node/src/test_support.rs | 186 ++++++++++++++++++++++++ 2 files changed, 250 insertions(+), 20 deletions(-) diff --git a/crates/gitlawb-node/src/api/bounties.rs b/crates/gitlawb-node/src/api/bounties.rs index 68b98178..421bec23 100644 --- a/crates/gitlawb-node/src/api/bounties.rs +++ b/crates/gitlawb-node/src/api/bounties.rs @@ -285,11 +285,22 @@ pub async fn submit_bounty( Path(id): Path, Json(req): Json, ) -> Result> { - let bounty = state - .db - .get_bounty(&id) - .await? - .ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?; + let not_found = || AppError::NotFound(format!("bounty {id} not found")); + + let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?; + + if crate::api::authorize_repo_read( + &state, + &bounty.repo_owner, + &bounty.repo_name, + Some(auth.0.as_str()), + "/", + ) + .await + .is_err() + { + return Err(not_found()); + } if bounty.status != "claimed" { return Err(AppError::BadRequest(format!( @@ -325,11 +336,22 @@ pub async fn approve_bounty( Path(id): Path, Json(req): Json, ) -> Result> { - let bounty = state - .db - .get_bounty(&id) - .await? - .ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?; + let not_found = || AppError::NotFound(format!("bounty {id} not found")); + + let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?; + + if crate::api::authorize_repo_read( + &state, + &bounty.repo_owner, + &bounty.repo_name, + Some(auth.0.as_str()), + "/", + ) + .await + .is_err() + { + return Err(not_found()); + } if bounty.status != "submitted" { return Err(AppError::BadRequest(format!( @@ -372,11 +394,22 @@ pub async fn cancel_bounty( Extension(auth): Extension, Path(id): Path, ) -> Result> { - let bounty = state - .db - .get_bounty(&id) - .await? - .ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?; + let not_found = || AppError::NotFound(format!("bounty {id} not found")); + + let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?; + + if crate::api::authorize_repo_read( + &state, + &bounty.repo_owner, + &bounty.repo_name, + Some(auth.0.as_str()), + "/", + ) + .await + .is_err() + { + return Err(not_found()); + } if bounty.status != "open" { return Err(AppError::BadRequest(format!( @@ -407,11 +440,22 @@ pub async fn dispute_bounty( Extension(auth): Extension, Path(id): Path, ) -> Result> { - let bounty = state - .db - .get_bounty(&id) - .await? - .ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?; + let not_found = || AppError::NotFound(format!("bounty {id} not found")); + + let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?; + + if crate::api::authorize_repo_read( + &state, + &bounty.repo_owner, + &bounty.repo_name, + Some(auth.0.as_str()), + "/", + ) + .await + .is_err() + { + return Err(not_found()); + } if bounty.status != "claimed" && bounty.status != "submitted" { return Err(AppError::BadRequest(format!( diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 430c0600..5dcfa2fd 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -14732,6 +14732,192 @@ mod tests { assert!(resp.status().is_success()); } + // ── #341: the id-keyed bounty mutations must gate on repo read before the + // status and participant checks, so a caller holding an opaque id cannot + // distinguish an existing private-repo bounty from an absent one. The deny + // shape is 404 with a "not found" body, identical to the absent-id case. + + fn seed_bounty( + id: &str, + owner: &str, + repo: &str, + status: &str, + claimant: Option<&str>, + ) -> crate::db::BountyRecord { + crate::db::BountyRecord { + id: id.into(), + repo_owner: owner.into(), + repo_name: repo.into(), + issue_id: None, + title: "Bounty".into(), + amount: 100, + creator_did: owner.into(), + claimant_did: claimant.map(str::to_string), + claimant_wallet: None, + pr_id: None, + status: status.into(), + created_at: "2026-01-01T00:00:00Z".into(), + claimed_at: claimant.map(|_| "2026-01-02T00:00:00Z".into()), + submitted_at: None, + completed_at: None, + deadline_secs: 86400, + tx_hash: None, + } + } + + fn signed_bounty_post(kp: &Keypair, uri: &str, body: &[u8]) -> Request { + let signed = gitlawb_core::http_sig::sign_request(kp, "POST", uri, body); + Request::builder() + .method(Method::POST) + .uri(uri) + .header("content-type", "application/json") + .header("content-digest", signed.content_digest) + .header("signature-input", signed.signature_input) + .header("signature", signed.signature) + .body(Body::from(body.to_vec())) + .unwrap() + } + + /// Drives one bounty mutation route as a stranger against a private repo and + /// asserts the denial is indistinguishable from the absent-id case: same + /// 404 status, same "not found" body shape, no status or participant detail. + /// `bounty_status` is whichever lifecycle state reaches the route's + /// participant check today, so the assertion exercises the deepest leak. + async fn assert_bounty_mutation_denies_stranger_on_private( + pool: PgPool, + route: &str, + bounty_status: &str, + body: &[u8], + ) { + let state = test_state(pool).await; + let owner = "did:key:zB341PRIVOWNERAAAAAAAAAAAAAAAAAAAAAAAA"; + let claimant = "did:key:zB341PRIVCLAIMANTAAAAAAAAAAAAAAAAAAAAA"; + state + .db + .create_repo(&seed_private_repo(owner, "secret-bounty-repo")) + .await + .unwrap(); + state + .db + .create_bounty(&seed_bounty( + "b341-held-id", + owner, + "secret-bounty-repo", + bounty_status, + Some(claimant), + )) + .await + .unwrap(); + + let router = crate::server::build_router(state); + let stranger = Keypair::generate(); + + // Held id: the stranger knows the bounty id but cannot read the repo. + let held_uri = format!("/api/v1/bounties/b341-held-id/{route}"); + let resp = router + .clone() + .oneshot(signed_bounty_post(&stranger, &held_uri, body)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "{route} on a held private-repo bounty id must deny as 404, not a \ + distinguishable status/participant error" + ); + let msg = json_body(resp).await; + assert!( + msg["message"].as_str().unwrap_or("").contains("not found"), + "{route} denial body must be the not-found shape, got {msg}" + ); + + // Absent id: the control case, already 404 today. + let absent_uri = format!("/api/v1/bounties/b341-no-such-id/{route}"); + let resp = router + .oneshot(signed_bounty_post(&stranger, &absent_uri, body)) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[sqlx::test] + async fn submit_bounty_gate_denies_stranger_on_private(pool: PgPool) { + assert_bounty_mutation_denies_stranger_on_private( + pool, + "submit", + "claimed", + br#"{"pr_id":"owner/repo#1"}"#, + ) + .await; + } + + #[sqlx::test] + async fn approve_bounty_gate_denies_stranger_on_private(pool: PgPool) { + assert_bounty_mutation_denies_stranger_on_private(pool, "approve", "submitted", b"{}") + .await; + } + + #[sqlx::test] + async fn cancel_bounty_gate_denies_stranger_on_private(pool: PgPool) { + assert_bounty_mutation_denies_stranger_on_private(pool, "cancel", "open", b"").await; + } + + #[sqlx::test] + async fn dispute_bounty_gate_denies_stranger_on_private(pool: PgPool) { + assert_bounty_mutation_denies_stranger_on_private(pool, "dispute", "claimed", b"").await; + } + + /// The read gate must admit readable repos: on a PUBLIC repo a stranger + /// still reaches the participant checks, so the four routes answer 403 + /// rather than the gated 404. + #[sqlx::test] + async fn bounty_mutation_gates_still_reach_participant_check_on_public(pool: PgPool) { + let state = test_state(pool).await; + let owner = "did:key:zB341PUBOWNERAAAAAAAAAAAAAAAAAAAAAAAAA"; + let claimant = "did:key:zB341PUBCLAIMANTAAAAAAAAAAAAAAAAAAAAAA"; + let mut repo = seed_private_repo(owner, "public-bounty-repo"); + repo.is_public = true; + state.db.create_repo(&repo).await.unwrap(); + + let cases: [(&str, &str, &[u8]); 4] = [ + ("submit", "claimed", br#"{"pr_id":"owner/repo#1"}"#), + ("approve", "submitted", b"{}"), + ("cancel", "open", b""), + ("dispute", "claimed", b""), + ]; + for (i, (_, status, _)) in cases.iter().enumerate() { + let id = format!("b341-pub-{i}"); + state + .db + .create_bounty(&seed_bounty( + &id, + owner, + "public-bounty-repo", + status, + Some(claimant), + )) + .await + .unwrap(); + } + + let router = crate::server::build_router(state); + let stranger = Keypair::generate(); + for (i, (route, _, body)) in cases.iter().enumerate() { + let id = format!("b341-pub-{i}"); + let uri = format!("/api/v1/bounties/{id}/{route}"); + let resp = router + .clone() + .oneshot(signed_bounty_post(&stranger, &uri, body)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::FORBIDDEN, + "{route} on a public repo must still reach the participant check" + ); + } + } + #[sqlx::test] async fn list_all_bounties_filters_private_repos_for_anon(pool: PgPool) { let state = test_state(pool).await; From 9be99aa7a4a25999142d385ec58585fd09bd9124 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 13 Sep 2026 12:52:24 -0500 Subject: [PATCH 2/2] Address PR review feedback (#448) - assert the exact not_found body shape for both held and absent bounty ids in the mutation-gate helper, so a denial carrying lifecycle or participant detail fails the test --- crates/gitlawb-node/src/test_support.rs | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 5dcfa2fd..fea31708 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -14825,19 +14825,31 @@ mod tests { "{route} on a held private-repo bounty id must deny as 404, not a \ distinguishable status/participant error" ); - let msg = json_body(resp).await; - assert!( - msg["message"].as_str().unwrap_or("").contains("not found"), - "{route} denial body must be the not-found shape, got {msg}" - ); + let held_body = json_body(resp).await; - // Absent id: the control case, already 404 today. + // Absent id: the control case. The body must carry the same not_found + // shape so a denial discloses nothing beyond the caller-supplied id. let absent_uri = format!("/api/v1/bounties/b341-no-such-id/{route}"); let resp = router .oneshot(signed_bounty_post(&stranger, &absent_uri, body)) .await .unwrap(); assert_eq!(resp.status(), StatusCode::NOT_FOUND); + let absent_body = json_body(resp).await; + + for (id, got) in [ + ("b341-held-id", &held_body), + ("b341-no-such-id", &absent_body), + ] { + assert_eq!( + *got, + serde_json::json!({ + "error": "not_found", + "message": format!("bounty {id} not found"), + }), + "{route} denial body for {id} must be the bare not-found shape, got {got}" + ); + } } #[sqlx::test]