Compare commits

...
Author SHA1 Message Date
beardthelion fcd0ab256c chore: refresh PR head 2026-11-05 08:35:38 -06:00
beardthelion ec3af1c30c 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.
2026-09-11 15:06:18 -05:00
beardthelion 7eb2868c56 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
2026-09-11 14:40:09 -05:00
Kevin Codex bfc44f926d Merge pull request #381 from Gitlawb/fix/gl-whoami-capped-body-read
fix(gl): read whoami error bodies through CappedBody.text
2026-08-26 13:26:50 +08:00
beardthelion 65f5989a1d test(node): pin owner-push off in receive-pack lock tests
These lock-path proofs use non-owner proof DIDs. With enforce_owner_push
defaulting on, they fail before receive-pack runs.
2026-08-25 10:27:48 -05:00
beardthelion 4725146db2 fix(gl): read whoami error bodies through CappedBody.text
#173 changed read_body_capped to return CappedBody; peer.rs and sync.rs
were updated but whoami.rs was not, leaving main red after the #223 merge.
2026-08-25 10:04:20 -05:00
4 changed files with 313 additions and 11 deletions

No files matched your search

+9 -10
View File
@@ -39,11 +39,11 @@ pub async fn register_replica(
) -> Result<(StatusCode, Json<serde_json::Value>)> {
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<AuthenticatedDid>,
Path((owner, repo)): Path<(String, String)>,
) -> Result<Json<serde_json::Value>> {
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?;
+9
View File
@@ -6260,6 +6260,9 @@ mod tests {
// Long enough that the git-service timeout is never what ends this push; the
// disconnect is.
cfg.git_service_timeout_secs = 600;
// Lock-path test, not owner-push authorization: pin off so the default-on gate
// does not reject the proof DID before receive-pack runs.
cfg.enforce_owner_push = false;
state.config = std::sync::Arc::new(cfg);
state
.db
@@ -6430,6 +6433,9 @@ mod tests {
None,
crate::git::repo_store::build_lock_pool(&pool, 4, std::time::Duration::from_secs(5)),
);
let mut cfg = (*state.config).clone();
cfg.enforce_owner_push = false;
state.config = std::sync::Arc::new(cfg);
state
.db
.upsert_mirror_repo(owner, name, "/tmp/z6succ-sc1", None, false)
@@ -6516,6 +6522,9 @@ mod tests {
let owner = "z6lockpool";
let name = "lp1";
let mut state = crate::test_support::test_state(pool.clone()).await;
let mut cfg = (*state.config).clone();
cfg.enforce_owner_push = false;
state.config = std::sync::Arc::new(cfg);
// One lock-pool connection, short checkout timeout so the exhaustion surfaces
// promptly rather than at the handler's own acquire deadline.
state.repo_store = crate::git::repo_store::RepoStore::new(
+294
View File
@@ -1427,6 +1427,300 @@ 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 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()
.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"
);
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(&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"
);
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()
.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 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()
.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"
);
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"
);
// 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"
);
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
.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
+1 -1
View File
@@ -60,7 +60,7 @@ pub(crate) async fn run_to_writer(args: WhoamiArgs, w: &mut impl std::io::Write)
}
Ok(resp) => {
let status = resp.status();
let raw = read_body_capped(resp, 8 * 1024).await;
let raw = read_body_capped(resp, 8 * 1024).await.text;
let msg = serde_json::from_str::<Value>(&raw)
.ok()
.and_then(|v| {