From 51769865bbcb9b573c5f0c60ef36eeae9a574d4e Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Fri, 29 May 2026 01:21:40 -0500 Subject: [PATCH 1/2] feat(node): enforce is_public on the git read path (whole-repo private-read) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `repos.is_public` is stored on every repo but never checked when serving clone/fetch, so private repos are world-readable over git smart-HTTP. This wires read enforcement for the whole-repo (scope=`/`) case — the literal "Implement private-read enforcement" short-term roadmap item. node: - api/repos.rs: add `authorize_read()`. Public repos stay open; private repos require the caller's authenticated DID to match the owner. Returns 404 (not 403) on denial so a private repo's existence does not leak. Mirrors the owner-match idiom in api/protect.rs. Gates `git_upload_pack` and the `git-upload-pack` branch of `git_info_refs`; the receive-pack (push) handshake is left untouched (authorized separately on the POST). - server.rs: move `info/refs` into `git_read_routes` and layer `optional_signature`, so an `AuthenticatedDid` is available on reads when a signature is present, without breaking anonymous clone of public repos. client (git-remote-gitlawb): - main.rs: sign the GET ref-advertisement, and broaden POST signing from push-only to fetch too, so a `git clone` of a private repo can authenticate. Out of scope (deliberately): path/package-scoped visibility, the gossip/sync and GraphQL/IPFS read surfaces, and the never-replicate-vs-fetch-denied question. See proposal discussion. Verification: `cargo check --workspace` clean (0 errors, 0 warnings); git-remote-gitlawb unit tests 6/6 pass. --- crates/git-remote-gitlawb/src/main.rs | 44 ++++++++++++++----------- crates/gitlawb-node/src/api/repos.rs | 46 +++++++++++++++++++++++++++ crates/gitlawb-node/src/server.rs | 7 ++-- 3 files changed, 76 insertions(+), 21 deletions(-) diff --git a/crates/git-remote-gitlawb/src/main.rs b/crates/git-remote-gitlawb/src/main.rs index 60ff1c38..077d8af5 100644 --- a/crates/git-remote-gitlawb/src/main.rs +++ b/crates/git-remote-gitlawb/src/main.rs @@ -127,11 +127,22 @@ fn handle_connect( let refs_url = format!("{}/info/refs?service={}", repo_base, service); tracing::debug!("GET {refs_url}"); - let refs_resp = client + let mut refs_req = client .get(&refs_url) - .header("User-Agent", "git/2.0 git-remote-gitlawb/0.1.0") - .send() - .with_context(|| format!("GET {refs_url}"))?; + .header("User-Agent", "git/2.0 git-remote-gitlawb/0.1.0"); + + // Sign the read so the node can authorize private-repo fetches (RFC 9421). + // GET has no body, so content-digest covers the empty body. Harmless for + // public repos — the node verifies only when the headers are present. + if let Some(kp) = keypair { + let signed = sign_request(kp, "GET", &url_path(&refs_url), &[]); + refs_req = refs_req + .header("Content-Digest", signed.content_digest) + .header("Signature-Input", signed.signature_input) + .header("Signature", signed.signature); + } + + let refs_resp = refs_req.send().with_context(|| format!("GET {refs_url}"))?; if !refs_resp.status().is_success() { bail!( @@ -201,20 +212,17 @@ fn handle_connect( .header("User-Agent", "git/2.0 git-remote-gitlawb/0.1.0") .body(request_body.clone()); - // Add RFC 9421 HTTP Signature auth on push operations - if service == "git-receive-pack" { - if let Some(kp) = keypair { - let signed = sign_request(kp, "POST", &path_for_sig, &request_body); - req = req - .header("Content-Digest", signed.content_digest) - .header("Signature-Input", signed.signature_input) - .header("Signature", signed.signature); - tracing::debug!("attached RFC 9421 HTTP Signature (DID: {})", kp.did()); - } else { - tracing::warn!( - "no identity keypair found — push will be unsigned (v0.1 local alpha only)" - ); - } + // Sign both push (receive-pack) and fetch (upload-pack). Fetch signing lets + // the node authorize private-repo clones; ignored by the node for public repos. + if let Some(kp) = keypair { + let signed = sign_request(kp, "POST", &path_for_sig, &request_body); + req = req + .header("Content-Digest", signed.content_digest) + .header("Signature-Input", signed.signature_input) + .header("Signature", signed.signature); + tracing::debug!("attached RFC 9421 HTTP Signature (DID: {})", kp.did()); + } else if service == "git-receive-pack" { + tracing::warn!("no identity keypair found — push will be unsigned (v0.1 local alpha only)"); } let pack_resp = req.send().with_context(|| format!("POST {post_url}"))?; diff --git a/crates/gitlawb-node/src/api/repos.rs b/crates/gitlawb-node/src/api/repos.rs index b6f919e3..83b44ddf 100644 --- a/crates/gitlawb-node/src/api/repos.rs +++ b/crates/gitlawb-node/src/api/repos.rs @@ -303,11 +303,47 @@ pub async fn get_tree( // ── Git smart HTTP endpoints ────────────────────────────────────────────── +/// Authorize a read (clone/fetch) against a repo's visibility. +/// +/// Public repos are world-readable. Private repos require the caller's +/// *authenticated* DID (verified upstream by `optional_signature`) to match the +/// owner. Returns `RepoNotFound` (404, NOT 403) when unauthorized so the node +/// does not leak the existence of a private repo. +/// +/// Whole-repo (`is_public`) only. Path/package-scoped visibility is a follow-up +/// that needs a `visibility_rules` table and a way to filter subtrees out of the +/// pack — see issue discussion. +fn authorize_read( + record: &crate::db::RepoRecord, + auth: Option<&AuthenticatedDid>, + owner: &str, + name: &str, +) -> Result<()> { + if record.is_public { + return Ok(()); + } + let caller = match auth { + Some(AuthenticatedDid(did)) => did.as_str(), + None => return Err(AppError::RepoNotFound(format!("{owner}/{name}"))), + }; + // Mirror the owner-match idiom from api/protect.rs (full did:key vs short form). + let owner_short = record + .owner_did + .split(':') + .next_back() + .unwrap_or(record.owner_did.as_str()); + if caller != record.owner_did.as_str() && caller != owner_short { + return Err(AppError::RepoNotFound(format!("{owner}/{name}"))); + } + Ok(()) +} + /// GET /:owner/:repo.git/info/refs?service=git-upload-pack pub async fn git_info_refs( State(state): State, Path((owner, repo)): Path<(String, String)>, Query(query): Query, + auth: Option>, ) -> Result { let name = repo.trim_end_matches(".git"); tracing::info!(owner = %owner, repo = %name, "info/refs request"); @@ -322,6 +358,13 @@ pub async fn git_info_refs( .ok_or_else(|| AppError::BadRequest("missing ?service= parameter".into()))?; tracing::debug!(service = %service, repo = %name, "info/refs service"); + // Enforce read (clone/fetch) visibility. The push advertisement + // (service=git-receive-pack) is authorized separately on the + // git-receive-pack POST, so leave it untouched here. + if service == "git-upload-pack" { + authorize_read(&record, auth.as_ref().map(|e| &e.0), &owner, name)?; + } + // For receive-pack (push), download the latest from Tigris so the client // sees the same refs that acquire_write() will operate on. let disk_path = if service == "git-receive-pack" { @@ -352,6 +395,7 @@ pub async fn git_info_refs( pub async fn git_upload_pack( State(state): State, Path((owner, repo)): Path<(String, String)>, + auth: Option>, body: Bytes, ) -> Result { let name = repo.trim_end_matches(".git"); @@ -361,6 +405,8 @@ pub async fn git_upload_pack( .await? .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?; + authorize_read(&record, auth.as_ref().map(|e| &e.0), &owner, name)?; + let disk_path = state .repo_store .acquire(&record.owner_did, &record.name) diff --git a/crates/gitlawb-node/src/server.rs b/crates/gitlawb-node/src/server.rs index 59bc69d0..da9f5a5a 100644 --- a/crates/gitlawb-node/src/server.rs +++ b/crates/gitlawb-node/src/server.rs @@ -304,18 +304,19 @@ pub fn build_router(state: AppState) -> Router { .route( "/api/v1/repos/{owner}/{repo}/replicas", get(replicas::list_replicas), - ) - .route("/{owner}/{repo}/info/refs", get(repos::git_info_refs)); + ); // git-upload-pack (clone/fetch) — same raised body limit as receive-pack so // large pack responses from the server don't get truncated on the client side. let git_read_routes = Router::new() + .route("/{owner}/{repo}/info/refs", get(repos::git_info_refs)) .route( "/{owner}/{repo}/git-upload-pack", post(repos::git_upload_pack), ) .layer(DefaultBodyLimit::disable()) - .layer(RequestBodyLimitLayer::new(pack_limit)); + .layer(RequestBodyLimitLayer::new(pack_limit)) + .layer(middleware::from_fn(auth::optional_signature)); // ── Meta ────────────────────────────────────────────────────────────── let meta_routes = Router::new() From 176009a3cf97102348470fb592b252b62cd02075 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Fri, 29 May 2026 01:53:17 -0500 Subject: [PATCH 2/2] test(node): truth-table for authorize_read private-read enforcement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six unit tests over the full visibility matrix for `authorize_read`: public → allow (anonymous and any DID); private → allow owner; private → deny anonymous; private → deny non-owner. Plus a no-leak contract test asserting the denial payload is byte-identical to a missing repo (`RepoNotFound("owner/secret")`), so a private repo's existence cannot be inferred from the response. Red-green verified: disabling the enforcement check fails exactly the three denial tests and leaves the three allow tests green. --- crates/gitlawb-node/src/api/repos.rs | 75 ++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/crates/gitlawb-node/src/api/repos.rs b/crates/gitlawb-node/src/api/repos.rs index 83b44ddf..9a2cd4a3 100644 --- a/crates/gitlawb-node/src/api/repos.rs +++ b/crates/gitlawb-node/src/api/repos.rs @@ -1112,3 +1112,78 @@ fn to_response(record: &crate::db::RepoRecord, state: &AppState, star_count: i64 forked_from: record.forked_from.clone(), } } + +#[cfg(test)] +mod tests { + use super::*; + + /// A repo owned by `did:key:z6MkOWNER` with the given visibility. + fn repo(is_public: bool) -> crate::db::RepoRecord { + crate::db::RepoRecord { + id: "r1".into(), + name: "secret".into(), + owner_did: "did:key:z6MkOWNER".into(), + description: None, + is_public, + default_branch: "main".into(), + created_at: Utc::now(), + updated_at: Utc::now(), + disk_path: "/tmp/x".into(), + forked_from: None, + machine_id: None, + } + } + + fn did(s: &str) -> AuthenticatedDid { + AuthenticatedDid(s.to_string()) + } + + #[test] + fn public_repo_allows_anonymous() { + authorize_read(&repo(true), None, "owner", "secret").unwrap(); + } + + #[test] + fn public_repo_allows_any_authenticated_did() { + let other = did("did:key:z6MkOTHER"); + authorize_read(&repo(true), Some(&other), "owner", "secret").unwrap(); + } + + #[test] + fn private_repo_allows_owner() { + let owner = did("did:key:z6MkOWNER"); + authorize_read(&repo(false), Some(&owner), "owner", "secret").unwrap(); + } + + #[test] + fn private_repo_denies_anonymous() { + let err = authorize_read(&repo(false), None, "owner", "secret").unwrap_err(); + assert!( + matches!(err, AppError::RepoNotFound(_)), + "anonymous read of a private repo must 404 (no-leak), got {err:?}" + ); + } + + #[test] + fn private_repo_denies_non_owner() { + let other = did("did:key:z6MkOTHER"); + let err = authorize_read(&repo(false), Some(&other), "owner", "secret").unwrap_err(); + assert!( + matches!(err, AppError::RepoNotFound(_)), + "non-owner read of a private repo must 404, not 403, got {err:?}" + ); + } + + /// The denial must be byte-identical to a genuinely-missing repo so a private + /// repo's existence cannot be inferred. `get_repo()` returns + /// `RepoNotFound("{owner}/{name}")` when a repo is absent; `authorize_read` + /// must produce the same payload for an unauthorized private read. + #[test] + fn private_denial_is_indistinguishable_from_missing() { + let err = authorize_read(&repo(false), None, "owner", "secret").unwrap_err(); + match err { + AppError::RepoNotFound(msg) => assert_eq!(msg, "owner/secret"), + other => panic!("expected RepoNotFound, got {other:?}"), + } + } +}