diff --git a/.env.example b/.env.example index b8765d7..cd5b5aa 100644 --- a/.env.example +++ b/.env.example @@ -72,6 +72,12 @@ GITLAWB_BOOTSTRAP_DISABLE_SEEDS=false # rolling upgrades so existing live nodes can still communicate. GITLAWB_REQUIRE_SIGNED_PEER_WRITES=false +# Require the authenticated pusher to be the repo owner on git-receive-pack. +# A valid did:key signature is authentication, not authorization: anyone can +# sign as their own DID. When true, pushes from a non-owner DID are rejected. +# Keep false until the repo owner is ready for owner-only writes. +GITLAWB_ENFORCE_OWNER_PUSH=false + # Comma-separated libp2p multiaddrs. # Example: /ip4/1.2.3.4/udp/7546/quic-v1/p2p/12D3KooW... GITLAWB_P2P_BOOTSTRAP= diff --git a/crates/gitlawb-node/src/api/protect.rs b/crates/gitlawb-node/src/api/protect.rs index f0844a0..435dc89 100644 --- a/crates/gitlawb-node/src/api/protect.rs +++ b/crates/gitlawb-node/src/api/protect.rs @@ -7,7 +7,7 @@ use axum::extract::{Extension, Path, State}; use axum::http::StatusCode; use axum::Json; -use crate::auth::AuthenticatedDid; +use crate::auth::{is_repo_owner, AuthenticatedDid}; use crate::error::{AppError, Result}; use crate::state::AppState; @@ -25,12 +25,7 @@ pub async fn protect_branch( // Only the repo owner can protect branches let caller = &auth.0; - let owner_short = record - .owner_did - .split(':') - .next_back() - .unwrap_or(&record.owner_did); - if caller != &record.owner_did && caller != owner_short { + if !is_repo_owner(&record, caller) { return Err(AppError::BadRequest( "only the repo owner can protect branches".into(), )); @@ -63,12 +58,7 @@ pub async fn unprotect_branch( .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; let caller = &auth.0; - let owner_short = record - .owner_did - .split(':') - .next_back() - .unwrap_or(&record.owner_did); - if caller != &record.owner_did && caller != owner_short { + if !is_repo_owner(&record, caller) { return Err(AppError::BadRequest( "only the repo owner can unprotect branches".into(), )); diff --git a/crates/gitlawb-node/src/api/repos.rs b/crates/gitlawb-node/src/api/repos.rs index 2886926..7adcf53 100644 --- a/crates/gitlawb-node/src/api/repos.rs +++ b/crates/gitlawb-node/src/api/repos.rs @@ -5,7 +5,7 @@ use axum::Json; use bytes::Bytes; use std::sync::Arc; -use crate::auth::AuthenticatedDid; +use crate::auth::{caller_authorized_to_push, AuthenticatedDid}; use chrono::Utc; use serde::{Deserialize, Serialize}; use uuid::Uuid; @@ -461,10 +461,38 @@ pub async fn git_upload_pack( Ok(resp) } +/// Decide whether the owner-push gate rejects a `git-receive-pack` request. +/// +/// Returns `Some(error)` when the push must be rejected, `None` when it may +/// proceed. Pure function so the policy is unit-testable without a database or a +/// live git backend. +/// +/// Fails closed: when `enforce` is on, an absent identity (`None`) or a caller +/// that is not authorized to push is rejected. When `enforce` is off it always +/// allows, preserving the legacy (authentication-only) behavior. +fn owner_push_rejection( + enforce: bool, + record: &crate::db::RepoRecord, + caller: Option<&str>, +) -> Option { + if !enforce { + return None; + } + match caller { + Some(did) if caller_authorized_to_push(record, did) => None, + _ => Some(AppError::BadRequest( + "push rejected — only the repo owner may push to this repository \ + (GITLAWB_ENFORCE_OWNER_PUSH is enabled)" + .into(), + )), + } +} + /// POST /:owner/:repo.git/git-receive-pack (AUTH REQUIRED — enforced by middleware) pub async fn git_receive_pack( State(state): State, Path((owner, repo)): Path<(String, String)>, + Extension(auth): Extension, headers: HeaderMap, body: Bytes, ) -> Result { @@ -483,6 +511,26 @@ pub async fn git_receive_pack( "parsed ref updates from pack" ); + // ── Owner-only push enforcement (opt-in: GITLAWB_ENFORCE_OWNER_PUSH) ── + // Runs before branch protection on purpose: when enabled, a non-owner is + // rejected here regardless of whether the target branch is protected, so a + // single rejection never yields two different error bodies. The identity is + // the canonical DID injected by `require_signature`, not a re-parse of the + // request headers. Fails closed (see `owner_push_rejection`). + if let Some(err) = owner_push_rejection( + state.config.enforce_owner_push, + &record, + Some(auth.0.as_str()), + ) { + tracing::warn!( + repo = %name, + pusher = %auth.0, + owner_did = %record.owner_did, + "owner-push enforcement: rejecting push from non-owner" + ); + return Err(err); + } + // ── Branch protection check ────────────────────────────────────────── let pusher_did_for_check = extract_did_from_auth(&headers); tracing::debug!(pusher_did = ?pusher_did_for_check, "extracted pusher DID from auth headers"); @@ -1221,3 +1269,64 @@ fn to_response(record: &crate::db::RepoRecord, state: &AppState, star_count: i64 forked_from: record.forked_from.clone(), } } + +#[cfg(test)] +mod tests { + use super::owner_push_rejection; + + const OWNER_DID: &str = "did:key:z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH"; + const OWNER_SHORT: &str = "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH"; + const STRANGER_DID: &str = "did:key:z6Mkffonly5tranger0000000000000000000000000000000"; + + fn repo_owned_by(owner_did: &str) -> crate::db::RepoRecord { + let now = chrono::Utc::now(); + crate::db::RepoRecord { + id: "repo-id".into(), + name: "demo".into(), + owner_did: owner_did.into(), + description: None, + is_public: true, + default_branch: "main".into(), + created_at: now, + updated_at: now, + disk_path: "/tmp/demo".into(), + forked_from: None, + machine_id: None, + } + } + + #[test] + fn enforced_allows_owner_full_did() { + let repo = repo_owned_by(OWNER_DID); + assert!(owner_push_rejection(true, &repo, Some(OWNER_DID)).is_none()); + } + + #[test] + fn enforced_allows_owner_short_did() { + // Owners are accepted in bare-multibase form, matching the rest of the + // codebase's owner comparisons. + let repo = repo_owned_by(OWNER_DID); + assert!(owner_push_rejection(true, &repo, Some(OWNER_SHORT)).is_none()); + } + + #[test] + fn enforced_rejects_non_owner() { + let repo = repo_owned_by(OWNER_DID); + assert!(owner_push_rejection(true, &repo, Some(STRANGER_DID)).is_some()); + } + + #[test] + fn enforced_rejects_missing_did() { + // Fail closed: an absent authenticated identity is rejected, not allowed. + let repo = repo_owned_by(OWNER_DID); + assert!(owner_push_rejection(true, &repo, None).is_some()); + } + + #[test] + fn disabled_allows_non_owner_and_missing_did() { + // Flag off → legacy behavior: authentication-only, no owner gate. + let repo = repo_owned_by(OWNER_DID); + assert!(owner_push_rejection(false, &repo, Some(STRANGER_DID)).is_none()); + assert!(owner_push_rejection(false, &repo, None).is_none()); + } +} diff --git a/crates/gitlawb-node/src/api/visibility.rs b/crates/gitlawb-node/src/api/visibility.rs index 6665b9e..09f4e91 100644 --- a/crates/gitlawb-node/src/api/visibility.rs +++ b/crates/gitlawb-node/src/api/visibility.rs @@ -5,7 +5,7 @@ use axum::http::StatusCode; use axum::Json; use serde::Deserialize; -use crate::auth::AuthenticatedDid; +use crate::auth::{is_repo_owner, AuthenticatedDid}; use crate::db::VisibilityMode; use crate::error::{AppError, Result}; use crate::state::AppState; @@ -26,12 +26,7 @@ pub struct RemoveVisibilityRequest { } fn require_owner(record: &crate::db::RepoRecord, caller: &str) -> Result<()> { - let owner_short = record - .owner_did - .split(':') - .next_back() - .unwrap_or(&record.owner_did); - if caller != record.owner_did && caller != owner_short { + if !is_repo_owner(record, caller) { return Err(AppError::BadRequest( "only the repo owner can manage visibility".into(), )); diff --git a/crates/gitlawb-node/src/auth/mod.rs b/crates/gitlawb-node/src/auth/mod.rs index 8cbe9f9..7838fdf 100644 --- a/crates/gitlawb-node/src/auth/mod.rs +++ b/crates/gitlawb-node/src/auth/mod.rs @@ -17,6 +17,31 @@ use crate::state::AppState; #[derive(Clone, Debug)] pub struct AuthenticatedDid(pub String); +/// Whether `caller` is the owner of `record`. +/// +/// Owners are compared in both forms the rest of the codebase accepts: the full +/// `did:key:z6Mk…` string and its bare multibase suffix (`z6Mk…`). This is the +/// single source of truth for the owner-match logic that previously lived inline +/// in the receive-pack, branch-protection, and visibility paths. +pub fn is_repo_owner(record: &crate::db::RepoRecord, caller: &str) -> bool { + let owner_short = record + .owner_did + .split(':') + .next_back() + .unwrap_or(&record.owner_did); + caller == record.owner_did || caller == owner_short +} + +/// Whether `caller` is authorized to push to `record`. +/// +/// Phase 1 (`GITLAWB_ENFORCE_OWNER_PUSH`): owner-only. This is intentionally a +/// distinct, intent-named gate rather than a bare owner check so that Phase 2 can +/// extend it to honor a verified UCAN `git/push` capability as a pure addition +/// (`is_repo_owner(..) || ucan_grants_push(..)`) without rewriting call sites. +pub fn caller_authorized_to_push(record: &crate::db::RepoRecord, caller: &str) -> bool { + is_repo_owner(record, caller) +} + use gitlawb_core::http_sig::{ build_signing_string, compute_content_digest, HttpSignature, COVERED_COMPONENTS, }; diff --git a/crates/gitlawb-node/src/config.rs b/crates/gitlawb-node/src/config.rs index 929471a..3a9a507 100644 --- a/crates/gitlawb-node/src/config.rs +++ b/crates/gitlawb-node/src/config.rs @@ -49,6 +49,14 @@ pub struct Config { )] pub require_signed_peer_writes: bool, + /// Require the authenticated pusher to be the repo owner on `git-receive-pack`. + /// Authentication (a valid did:key signature) is not authorization on its own: + /// any party can sign as their own DID. When true, pushes whose authenticated + /// DID is not the repo owner are rejected. Keep false during rolling upgrades; + /// flip it on once owners are ready for owner-only writes. + #[arg(long, env = "GITLAWB_ENFORCE_OWNER_PUSH", default_value_t = false)] + pub enforce_owner_push: bool, + /// URL of local IPFS/Kubo node HTTP API (e.g. http://127.0.0.1:5001) #[arg(long, env = "GITLAWB_IPFS_API", default_value = "")] pub ipfs_api: String, diff --git a/docs/RUN-A-NODE.md b/docs/RUN-A-NODE.md index 987919f..b730d5f 100644 --- a/docs/RUN-A-NODE.md +++ b/docs/RUN-A-NODE.md @@ -141,6 +141,30 @@ During the cooldown your node still earns rewards if it keeps heartbeating. --- +## Hardening: owner-only push + +By default the node authenticates every `git-receive-pack` push (a valid RFC 9421 +`did:key` signature) but does **not** check that the pusher owns the repo, except +on branches that are explicitly protected. Because `did:key` is self-certifying, +any party can generate a key, derive its DID, sign, and push to an unprotected +branch — authentication is not authorization. + +To require the authenticated pusher to be the repo owner on **every** branch, set: + +```bash +GITLAWB_ENFORCE_OWNER_PUSH=true +``` + +- **Default `false`** — preserves current behavior so live nodes are unaffected by + an upgrade. Turn it on once you're ready for owner-only writes. +- **When `true`** — a push whose authenticated DID is not the repo owner is + rejected before any ref update is applied. The owner is matched in both the full + `did:key:z6Mk…` form and its bare `z6Mk…` suffix. +- Collaborator and UCAN-delegated push rights are a separate, planned follow-up; + today this is strictly owner-only. + +--- + ## Operational checklist | Concern | Recommendation |