From 7eb2868c5629568904423a5d8fb0791e14f47bd4 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Fri, 11 Sep 2026 14:22:14 -0500 Subject: [PATCH 1/2] fix(node): gate replica register/unregister on repo read visibility list_replicas already applied authorize_repo_read, but the two mutations looked the repo up directly: a non-reader of a private repo could register or remove their own replica and receive the replica count in the response. Route both handlers through authorize_repo_read so a denied caller gets the same 404 as a missing repo and nothing is written. Self-registration and self-removal are unchanged for authorized readers. Fixes #435 --- crates/gitlawb-node/src/api/replicas.rs | 19 +- crates/gitlawb-node/src/test_support.rs | 256 ++++++++++++++++++++++++ 2 files changed, 265 insertions(+), 10 deletions(-) diff --git a/crates/gitlawb-node/src/api/replicas.rs b/crates/gitlawb-node/src/api/replicas.rs index ece82bff..affb82a1 100644 --- a/crates/gitlawb-node/src/api/replicas.rs +++ b/crates/gitlawb-node/src/api/replicas.rs @@ -39,11 +39,11 @@ pub async fn register_replica( ) -> Result<(StatusCode, Json)> { validate_replica_url(&req.url)?; - let record = state - .db - .get_repo(&owner, &repo) - .await? - .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + // The mutation response carries repo metadata (name, replica count), so + // registration applies the same read-visibility decision as the listing: + // a caller who may not read the repo gets the same 404 as a missing one. + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, Some(auth.0.as_str()), "/").await?; let replica_did = &auth.0; @@ -93,11 +93,10 @@ pub async fn unregister_replica( 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}")))?; + // Same gate as register_replica: removal mutates repo-scoped metadata and + // its response discloses the replica count. + let (record, _rules) = + crate::api::authorize_repo_read(&state, &owner, &repo, Some(auth.0.as_str()), "/").await?; let replica_did = &auth.0; state.db.unregister_replica(&record.id, replica_did).await?; diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 430c0600..a0511ddc 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -1427,6 +1427,262 @@ mod tests { assert_eq!(resp.status(), StatusCode::NOT_FOUND, "absent repo → 404"); } + /// #435: register_replica / unregister_replica apply the same read-visibility + /// gate as list_replicas. Both mutate caller-bound replica metadata and their + /// responses disclose the replica count, so a non-reader of a private repo + /// gets the same 404 as a missing repo and nothing is written. Authorized + /// readers keep self-registering and self-removing. + #[sqlx::test] + async fn replica_register_unregister_are_read_visibility_gated(pool: PgPool) { + use crate::db::VisibilityMode; + let owner = "did:key:zREPMUTOWNERRRRRRRRRRRRRRRRRRRRRRRRRR"; + let reader = "did:key:zREPMUTREADERRRRRRRRRRRRRRRRRRRRRRRRR"; + let stranger = "did:key:zREPMUTSTRGRRRRRRRRRRRRRRRRRRRRRRRRR"; + let state = test_state(pool).await; + + let mut priv_repo = seed_repo(owner, "repmut-priv"); + priv_repo.is_public = false; + state + .db + .create_repo(&priv_repo) + .await + .expect("seed private repo"); + let pub_repo = seed_repo(owner, "repmut-pub"); + state + .db + .create_repo(&pub_repo) + .await + .expect("seed public repo"); + // A listed reader of the private repo keeps mutation rights. + state + .db + .set_visibility_rule( + &priv_repo.id, + "/", + VisibilityMode::B, + &[reader.to_string()], + owner, + ) + .await + .expect("seed reader rule"); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/replicas", + axum::routing::put(crate::api::replicas::register_replica) + .delete(crate::api::replicas::unregister_replica), + ) + .with_state(state.clone()) + }; + let body = || Body::from(r#"{"url":"https://replica.example.com/me"}"#.to_string()); + + // Private repo, non-reader stranger: register → 404, nothing written. + let resp = router() + .oneshot(signed_request_as( + stranger, + Method::PUT, + &format!("/api/v1/repos/{owner}/repmut-priv/replicas"), + body(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "a non-reader must not register on a private repo" + ); + assert!( + state + .db + .list_replicas(&priv_repo.id) + .await + .unwrap() + .is_empty(), + "a denied register must not create a replica row" + ); + + // Private repo, non-reader stranger: unregister → 404. + let resp = router() + .oneshot(signed_request_as( + stranger, + Method::DELETE, + &format!("/api/v1/repos/{owner}/repmut-priv/replicas"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "a non-reader must not unregister on a private repo" + ); + + // Private repo, listed reader: register → 201, unregister → 200. + let resp = router() + .oneshot(signed_request_as( + reader, + Method::PUT, + &format!("/api/v1/repos/{owner}/repmut-priv/replicas"), + body(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::CREATED, + "a listed reader keeps self-registration on a private repo" + ); + let resp = router() + .oneshot(signed_request_as( + reader, + Method::DELETE, + &format!("/api/v1/repos/{owner}/repmut-priv/replicas"), + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "a listed reader keeps self-removal on a private repo" + ); + + // Public repo, authenticated stranger: register → 201. + let resp = router() + .oneshot(signed_request_as( + stranger, + Method::PUT, + &format!("/api/v1/repos/{owner}/repmut-pub/replicas"), + body(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::CREATED, + "public-repo registration stays open to any authenticated replica" + ); + + // Owner self-registration is still refused. + let resp = router() + .oneshot(signed_request_as( + owner, + Method::PUT, + &format!("/api/v1/repos/{owner}/repmut-pub/replicas"), + body(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::BAD_REQUEST, + "the owner still cannot register as their own replica" + ); + } + + /// #435 end-to-end: drive the replica mutations through the PRODUCTION + /// router (`app` → require_signature → handler) with real RFC-9421 + /// signatures, so the whole verify-then-authorize stack is exercised, not + /// just the handler with an injected DID. A stranger on a private repo + /// gets the same 404 as a missing repo with nothing written; a listed + /// reader registers and removes itself. + #[sqlx::test] + async fn replica_mutations_enforce_visibility_through_real_signature_e2e(pool: PgPool) { + use crate::db::VisibilityMode; + use gitlawb_core::http_sig::sign_request; + use gitlawb_core::identity::Keypair; + + let owner_kp = Keypair::generate(); + let owner_did = owner_kp.did().to_string(); + // Short owner form in the path keeps the signed @path byte-identical + // to what the node sees (no colons) while did_matches still resolves. + let short = owner_did.split(':').next_back().unwrap().to_string(); + let stranger_kp = Keypair::generate(); + let reader_kp = Keypair::generate(); + let reader_did = reader_kp.did().to_string(); + + let state = test_state(pool.clone()).await; + let mut repo = seed_repo(&owner_did, "sig-repl-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_did], &owner_did) + .await + .expect("seed reader rule"); + + let router = app(pool).await; + let path = format!("/api/v1/repos/{short}/sig-repl-priv/replicas"); + let reg_body: &[u8] = br#"{"url":"https://replica.example.com/e2e"}"#; + let signed_req = |kp: &Keypair, method: &str, body: &'static [u8]| { + let signed = sign_request(kp, method, &path, body); + Request::builder() + .method(method) + .uri(&path) + .header(axum::http::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)) + .unwrap() + }; + + // Stranger (verified signature, not a reader) → 404, no row written. + let resp = router + .clone() + .oneshot(signed_req(&stranger_kp, "PUT", reg_body)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "a verified non-reader must not register on a private repo" + ); + assert!( + state.db.list_replicas(&repo.id).await.unwrap().is_empty(), + "a denied register must not create a replica row" + ); + + // Stranger DELETE → 404. + let resp = router + .clone() + .oneshot(signed_req(&stranger_kp, "DELETE", b"")) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::NOT_FOUND, + "a verified non-reader must not unregister on a private repo" + ); + + // Listed reader: PUT → 201, DELETE → 200. + let resp = router + .clone() + .oneshot(signed_req(&reader_kp, "PUT", reg_body)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::CREATED, + "a listed reader registers through the real middleware" + ); + let resp = router + .clone() + .oneshot(signed_req(&reader_kp, "DELETE", b"")) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "a listed reader unregisters through the real middleware" + ); + } + /// #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 From ec3af1c30ce3168334a4fe0872c245fd94207f31 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Fri, 11 Sep 2026 15:06:18 -0500 Subject: [PATCH 2/2] test(node): assert denial bodies leak no replica or repo data CodeRabbit nitpick: the new replica-mutation denial tests only checked the 404 status. The path instructions for this file require non-leaking body assertions on denied responses, matching the sibling list_replicas_is_read_visibility_gated test. Assert the denied PUT and DELETE bodies contain neither the submitted replica URL nor the repo id, in both the handler-level test and the real-signature e2e. --- crates/gitlawb-node/src/test_support.rs | 40 ++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index a0511ddc..e1f9bb39 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -1475,7 +1475,12 @@ mod tests { ) .with_state(state.clone()) }; - let body = || Body::from(r#"{"url":"https://replica.example.com/me"}"#.to_string()); + let replica_url = "https://replica.example.com/me"; + let body = || Body::from(format!(r#"{{"url":"{replica_url}"}}"#)); + let leaks = |bytes: &[u8]| { + let text = String::from_utf8_lossy(bytes); + text.contains(replica_url) || text.contains(&priv_repo.id) + }; // Private repo, non-reader stranger: register → 404, nothing written. let resp = router() @@ -1492,6 +1497,13 @@ mod tests { StatusCode::NOT_FOUND, "a non-reader must not register on a private repo" ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + !leaks(&bytes), + "a denied register body must not leak the replica url or repo data" + ); assert!( state .db @@ -1517,6 +1529,13 @@ mod tests { StatusCode::NOT_FOUND, "a non-reader must not unregister on a private repo" ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + !leaks(&bytes), + "a denied unregister body must not leak the replica url or repo data" + ); // Private repo, listed reader: register → 201, unregister → 200. let resp = router() @@ -1618,7 +1637,12 @@ mod tests { let router = app(pool).await; let path = format!("/api/v1/repos/{short}/sig-repl-priv/replicas"); + let replica_url = "https://replica.example.com/e2e"; let reg_body: &[u8] = br#"{"url":"https://replica.example.com/e2e"}"#; + let leaks = |bytes: &[u8]| { + let text = String::from_utf8_lossy(bytes); + text.contains(replica_url) || text.contains(&repo.id) + }; let signed_req = |kp: &Keypair, method: &str, body: &'static [u8]| { let signed = sign_request(kp, method, &path, body); Request::builder() @@ -1643,6 +1667,13 @@ mod tests { StatusCode::NOT_FOUND, "a verified non-reader must not register on a private repo" ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + !leaks(&bytes), + "a denied register body must not leak the replica url or repo data" + ); assert!( state.db.list_replicas(&repo.id).await.unwrap().is_empty(), "a denied register must not create a replica row" @@ -1659,6 +1690,13 @@ mod tests { StatusCode::NOT_FOUND, "a verified non-reader must not unregister on a private repo" ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + !leaks(&bytes), + "a denied unregister body must not leak the replica url or repo data" + ); // Listed reader: PUT → 201, DELETE → 200. let resp = router