diff --git a/.env.example b/.env.example index bbd9a342..c0820201 100644 --- a/.env.example +++ b/.env.example @@ -13,9 +13,19 @@ GITLAWB_PUBLIC_URL=https://your-node.example.com # ── Server ──────────────────────────────────────────────────────────────── GITLAWB_HOST=0.0.0.0 GITLAWB_PORT=7545 +# Optional address to bind a Prometheus /metrics exposition endpoint on (e.g. +# 127.0.0.1:9091). Leave empty (default) to disable. Bind to localhost or a +# private interface — the metrics endpoint is unauthenticated. +GITLAWB_METRICS_ADDR= +# Max seconds to wait for in-flight requests to drain on shutdown before the +# server returns 503 to anything still in flight and exits. Default: 30. +GITLAWB_SHUTDOWN_GRACE_SECS=30 # ── Storage ─────────────────────────────────────────────────────────────── GITLAWB_REPOS_DIR=/data/repos +# Tigris (S3-compatible) bucket for repo storage. Leave empty (default) to +# disable Tigris and use local-only storage. +GITLAWB_TIGRIS_BUCKET= # PostgreSQL connection URL. Required. # When using the bundled docker-compose, this is wired automatically. @@ -39,6 +49,9 @@ GITLAWB_DB_RETRY_INITIAL_SECS=5 GITLAWB_DB_RETRY_MAX_SECS=60 # ── IPFS pinning (Pinata) ───────────────────────────────────────────────── +# URL of a local IPFS/Kubo node HTTP API (e.g. http://127.0.0.1:5001). Leave +# empty (default) to disable local IPFS. +GITLAWB_IPFS_API= # Get a JWT at https://app.pinata.cloud/developers/api-keys GITLAWB_PINATA_JWT= GITLAWB_PINATA_UPLOAD_URL=https://uploads.pinata.cloud/v3/files @@ -127,6 +140,20 @@ GITLAWB_PUSH_RATE_LIMIT=600 # the client IP. 0 disables. Default 120. GITLAWB_CREATE_RATE_LIMIT=120 +# ── Write rate limiting (non-creation authenticated writes) ─────────────── +# Max non-creation write requests per client IP per hour: issue/PR comments, +# labels, stars, merges, protect/unprotect, replicas, visibility, tasks, +# bounties, profile, and all GraphQL HTTP requests. The brake wraps the whole +# /graphql route, so queries and the playground GET consume this bucket too, not +# only mutations (GraphQL WebSocket subscriptions are excluded). Its own bucket, +# separate from the creation and push brakes. Uses GITLAWB_TRUSTED_PROXY to +# resolve the client IP. +# NOTE: this is a per-IP aggregate across ALL those write actions, so behind a +# shared NAT/egress IP (or with GITLAWB_TRUSTED_PROXY unset) many users collapse +# onto one bucket — raise this for automation-heavy or multi-user single-IP +# deployments. 0 disables. Default 600. +GITLAWB_WRITE_RATE_LIMIT=600 + # ── Peer-sync rate limiting (per client IP, uses GITLAWB_TRUSTED_PROXY below) ─ # /api/v1/peers/announce and /api/v1/sync/notify accept unsigned requests from # known peers and run at higher frequency, so a generous bucket. Separate from diff --git a/Cargo.lock b/Cargo.lock index d12daa43..47f2824f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3300,7 +3300,7 @@ dependencies = [ [[package]] name = "git-remote-gitlawb" -version = "0.5.0" +version = "0.5.1" dependencies = [ "anyhow", "gitlawb-core", @@ -3312,7 +3312,7 @@ dependencies = [ [[package]] name = "gitlawb-attest" -version = "0.5.0" +version = "0.5.1" dependencies = [ "base64", "ed25519-dalek", @@ -3329,7 +3329,7 @@ dependencies = [ [[package]] name = "gitlawb-core" -version = "0.5.0" +version = "0.5.1" dependencies = [ "anyhow", "base64", @@ -3356,13 +3356,14 @@ dependencies = [ [[package]] name = "gitlawb-node" -version = "0.5.0" +version = "0.5.1" dependencies = [ "alloy", "anyhow", "async-compression", "async-graphql", "async-graphql-axum", + "async-trait", "aws-config", "aws-sdk-s3", "axum", @@ -3412,7 +3413,7 @@ dependencies = [ [[package]] name = "gl" -version = "0.5.0" +version = "0.5.1" dependencies = [ "alloy", "anyhow", diff --git a/crates/gitlawb-node/Cargo.toml b/crates/gitlawb-node/Cargo.toml index 9e42c084..3e721181 100644 --- a/crates/gitlawb-node/Cargo.toml +++ b/crates/gitlawb-node/Cargo.toml @@ -12,6 +12,7 @@ path = "src/main.rs" [dependencies] gitlawb-core = { path = "../gitlawb-core" } +async-trait = "0.1" ed25519-dalek = { workspace = true } base64 = { workspace = true } tokio = { workspace = true } diff --git a/crates/gitlawb-node/src/admin.rs b/crates/gitlawb-node/src/admin.rs new file mode 100644 index 00000000..a5c2edf5 --- /dev/null +++ b/crates/gitlawb-node/src/admin.rs @@ -0,0 +1,1470 @@ +//! Admin subcommands invoked out-of-band, not part of the running node. +//! +//! `purge-spam` produces a reviewable dry-run list of empty spam-burst repos and, +//! only behind an explicit `--execute` flag, deletes them one at a time. The +//! selection logic is the load-bearing security part: a repo qualifies ONLY if it +//! is owned by the named burst DID AND is verified empty (zero git refs) PER REPO, +//! and a hard exclusion gate (evaluated BEFORE the empty check) keeps +//! content-bearing and intern/mirror-bot DIDs out no matter what. + +use std::path::{Path, PathBuf}; + +use anyhow::{Context, Result}; +use tracing::{info, warn}; + +use crate::db::{Db, RepoRecord}; +use crate::git::store; + +/// The did:key of the spam burst this tool targets. The purge is scoped to +/// exactly this owner; an empty repo owned by anyone else is never a candidate. +pub const SPAM_BURST_TARGET_DID: &str = "did:key:z6Mkopj6mhcMayipekXbTRFMZPM6Bsgy4FQZuN9fannXSLTC"; + +/// DIDs that must never be touched, even when they own an empty repo whose +/// signature otherwise matches the burst. The gate is evaluated BEFORE the empty +/// check so it wins unconditionally: +/// - `z6Mkk4L…` is a content-bearing live user. +/// - `z6MkqRz…` is the intern / mirror-bot DID. +pub const EXCLUDED_DIDS: &[&str] = &[ + "did:key:z6Mkk4LDvfA8VQmdehbJDvxp133sdtXUhR2UkUnMPguX7gnP", + "did:key:z6MkqRzACJ5iCDdkiymAPK3gq18z2iecZHeAuUyW6JnwRfoM", +]; + +/// Outcome tally of a `run_purge_spam` execute pass. Returned so the DB-delete +/// count is never conflated with full success: `disk_failed` records repos whose +/// row was deleted but whose on-disk dir could not be removed (or escaped the +/// repos_dir containment check), so an operator sees the DB/disk drift instead of +/// a clean "N deleted" summary. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub struct PurgeSummary { + /// Repo rows actually deleted from the DB. + pub deleted: u64, + /// Candidates skipped because they were no longer empty (pre-filter or the + /// authoritative recheck under the lock). + pub skipped_not_empty: usize, + /// Candidates skipped because a live writer held the per-repo lock. + pub skipped_locked: usize, + /// Candidates skipped because the object store could not be consulted for the + /// authoritative emptiness recheck (fail-closed: never delete on a store + /// error rather than risk deleting a repo with live remote refs). + pub skipped_store_error: usize, + /// Rows deleted whose on-disk dir removal FAILED (or was refused by the + /// containment guard) — DB/disk drift the operator must reconcile. + pub disk_failed: usize, + /// Rows+dirs deleted whose object-store archive removal FAILED — the archive + /// survives and could be re-downloaded into a later same-owner/name repo, so + /// this is tracked separately and never folded into a clean success. + pub archive_failed: usize, + /// Candidates with no local copy, admitted only because an object store is + /// configured (emptiness decided under the lock at execute time). Reported so + /// the dry-run can surface them distinctly from locally-verified candidates. + pub remote_unverified: usize, +} + +/// A repo selected for purge, with the evidence that qualified it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Candidate { + pub id: String, + pub owner_did: String, + pub name: String, + /// Number of git refs found on disk. 0 for a locally-verified empty candidate; + /// also 0 for a remote-unverified one whose emptiness is decided under the lock. + pub ref_count: usize, + /// True when the repo has no local copy and was admitted only because an object + /// store is configured. Its emptiness has NOT been verified — the execute path + /// must refresh from the archive and recheck UNDER the per-repo lock before any + /// delete; the dry-run lists it distinctly and never touches it. + pub remote_unverified: bool, +} + +/// Whether a DID is on the hard exclusion list. Compared under did:key +/// normalization (the same convention the repos table and every ownership check +/// use), so an excluded identity stored in either `did:key:z6…` or bare `z6…` +/// form is protected regardless of the form the exclusion constant is written in. +fn is_excluded(owner_did: &str) -> bool { + let owner_key = crate::db::normalize_owner_key(owner_did); + EXCLUDED_DIDS + .iter() + .any(|d| crate::db::normalize_owner_key(d) == owner_key) +} + +/// Pure candidate selector — the security core, isolated from disk and DB so the +/// exclusion + empty logic is directly testable. +/// +/// `repos` is the raw row set to consider (the caller supplies the target DID's +/// rows). `local_refs_of` returns `Some(n)` when a local bare repo exists (n +/// refs) and `None` when there is no local copy; the CLI wires the real on-disk +/// source, tests inject precomputed states. `store_configured` gates whether a +/// missing-local repo may be admitted. +/// +/// A repo qualifies ONLY if, PER REPO: +/// 1. its owner is NOT on the exclusion list (gate evaluated FIRST), AND +/// 2. its owner is exactly the target burst DID, AND +/// 3. EITHER it is locally verified empty (`Some(0)`), OR it has no local copy +/// (`None`) AND an object store is configured — in which case it is admitted +/// as remote-unverified and its emptiness is decided under the lock later. +/// +/// The exclusion gate is checked before everything so an empty repo owned by an +/// excluded DID is dropped regardless of its ref signature. A missing-local repo +/// with no object store fails closed (skipped). +pub fn select_spam_candidates( + repos: &[RepoRecord], + target_did: &str, + store_configured: bool, + mut local_refs_of: F, +) -> Vec +where + F: FnMut(&RepoRecord) -> Option, +{ + let mut out = Vec::new(); + for repo in repos { + // Hard exclusion gate FIRST — wins over the empty signature. + if is_excluded(&repo.owner_did) { + continue; + } + // Scope to the named burst only, under did:key normalization so a burst + // row stored in either did:key or bare form is matched consistently. + if crate::db::normalize_owner_key(&repo.owner_did) + != crate::db::normalize_owner_key(target_did) + { + continue; + } + // `local_refs_of` is `Some(n)` when a local bare repo exists and `None` + // when it does not. A local empty repo (Some(0)) is a verified candidate; + // a local non-empty repo is skipped; a missing local copy is a candidate + // ONLY when an object store is configured (its emptiness is then decided + // authoritatively under the lock after refresh), else it fails closed. + let remote_unverified = match local_refs_of(repo) { + Some(0) => false, + Some(_) => continue, + None => { + if store_configured { + true + } else { + continue; + } + } + }; + out.push(Candidate { + id: repo.id.clone(), + owner_did: repo.owner_did.clone(), + name: repo.name.clone(), + ref_count: 0, + remote_unverified, + }); + } + out +} + +/// Count the git refs of a repo on disk. Zero refs means the repo is empty. +/// +/// Resolves the repo's bare path from `repos_dir` + owner/name (the same layout +/// `git::store::repo_disk_path` writes) and shells to `git for-each-ref` via +/// `store::list_refs`. A repo whose on-disk path is missing or unreadable is +/// treated as having an unknown, non-empty ref count so it is NOT selected — the +/// tool fails closed and never deletes on a read error. +/// +/// Critically, a `0` count must come from THIS exact bare repo and never from +/// git's upward repository discovery. `git for-each-ref` runs with the repo path +/// as its cwd and no explicit `--git-dir`, so if the path exists but is not +/// itself a git dir, git walks parent directories for a `.git` — and `repos_dir` +/// may live inside the operator's own git checkout. That would read a DIFFERENT +/// repo's refs (possibly `0`) and delete a real repo. We defend by requiring the +/// bare-repo markers (`HEAD` file + `objects/` dir) before trusting any count; +/// anything else fails closed (treated non-empty, skipped). +/// Local ref state for selection: `Some(n)` when a local bare repo exists (n +/// refs), `None` when there is no local copy. `None` is what lets selection +/// distinguish a missing-local repo (a remote-unverified candidate when a store +/// is configured) from a one-ref repo — both of which `ref_count_on_disk` +/// collapses to a non-zero count. An unsafe name or an unreadable repo fails +/// closed to `Some(1)` so it is skipped, never admitted as remote-unverified. +fn local_refs_on_disk(repos_dir: &Path, owner_did: &str, name: &str) -> Option { + // Fail closed on an unsafe repo name BEFORE building any on-disk path (a + // peer-mirror row can carry a `../` name). Report it as non-empty so it is + // never a candidate — never as missing (which could admit it remote-unverified). + if let Err(e) = crate::git::repo_store::validate_repo_name(name) { + warn!(name = %name, err = %e, + "purge-spam: unsafe repo name — treating as non-empty (skipped)"); + return Some(1); + } + let path = store::repo_disk_path(repos_dir, owner_did, name); + if !path.join("HEAD").is_file() || !path.join("objects").is_dir() { + // No local bare repo at the expected path. + return None; + } + match store::list_refs(&path) { + Ok(refs) => Some(refs.len()), + Err(e) => { + warn!(path = %path.display(), err = %e, + "purge-spam: could not read refs — treating as non-empty (skipped)"); + Some(1) + } + } +} + +/// Ref count keyed on owner+name (returns 1 for a missing/unsafe/unreadable +/// repo — fail closed), used by the execute path to re-verify emptiness right +/// before deleting (using only a [`Candidate`]) and, after `refresh_from_archive` +/// downloads a remote-unverified candidate, to decide its emptiness under the lock. +fn ref_count_on_disk(repos_dir: &Path, owner_did: &str, name: &str) -> usize { + // Fail closed on an unsafe repo name BEFORE building any on-disk path. A + // peer-mirror row (which skips API name validation) can carry a `../` name; + // `repo_disk_path` would join it verbatim and resolve OUTSIDE `repos_dir`, + // pointing this "empty" check — and later the delete — at an unrelated repo. + // Reject it here so such a row is never a candidate (treated non-empty). + if let Err(e) = crate::git::repo_store::validate_repo_name(name) { + warn!(name = %name, err = %e, + "purge-spam: unsafe repo name — treating as non-empty (skipped)"); + return 1; + } + let path = store::repo_disk_path(repos_dir, owner_did, name); + if !path.join("HEAD").is_file() || !path.join("objects").is_dir() { + // Not a bare git repo at the exact expected path. Do NOT trust a ref + // count that git discovery could have read from a parent repository. + warn!(path = %path.display(), + "purge-spam: path is not a bare git repo — treating as non-empty (skipped)"); + return 1; + } + match store::list_refs(&path) { + Ok(refs) => refs.len(), + Err(e) => { + // Fail closed: an unreadable repo is not provably empty, so keep it + // out of the candidate set (report it as one ref so it's excluded). + warn!(path = %path.display(), err = %e, + "purge-spam: could not read refs — treating as non-empty (skipped)"); + 1 + } + } +} + +/// Whether `path` resolves canonically inside `root`. Both are canonicalized so +/// symlinks and `..` segments are fully resolved before the containment test; a +/// path that does not exist (or a root that cannot be canonicalized) fails closed +/// to `false`. Used as the last gate before a destructive `remove_dir_all`. +fn path_within(path: &Path, root: &Path) -> bool { + match (std::fs::canonicalize(path), std::fs::canonicalize(root)) { + (Ok(p), Ok(r)) => p.starts_with(&r), + _ => false, + } +} + +/// Split selected candidates into (delete, skip) by a fresh emptiness re-check, +/// so a repo that gained a ref between selection and deletion (a TOCTOU push) +/// is never deleted. Pure over the `recheck` closure so the skip branch is +/// directly testable; the CLI wires the real on-disk re-check. +fn partition_for_delete( + candidates: &[Candidate], + mut recheck: F, +) -> (Vec<&Candidate>, Vec<&Candidate>) +where + F: FnMut(&Candidate) -> usize, +{ + let mut to_delete = Vec::new(); + let mut to_skip = Vec::new(); + for c in candidates { + if recheck(c) == 0 { + to_delete.push(c); + } else { + to_skip.push(c); + } + } + (to_delete, to_skip) +} + +/// Run the `purge-spam` admin subcommand. +/// +/// Enumerates the target burst DID's repos, verifies each is empty on disk, +/// applies the exclusion gate, prints one dry-run row per candidate with owner + +/// ref-count evidence, and — only when `execute` is true — deletes the DB row of +/// each candidate one at a time. Dry-run (the default) deletes nothing. +pub async fn run_purge_spam( + db: &Db, + repo_store: &crate::git::repo_store::RepoStore, + repos_dir: &Path, + execute: bool, +) -> Result { + let repos_dir: PathBuf = repos_dir.to_path_buf(); + let rows = db + .list_repos_by_owner_did(SPAM_BURST_TARGET_DID) + .await + .context("listing repos for the spam-burst target DID")?; + + // A repo with no local copy is admitted as a remote-unverified candidate + // only when an object store is configured — its emptiness is then decided + // under the lock after refresh_from_archive. Without a store, missing-local + // fails closed (skipped), preserving the wrong-machine safety rule. + let store_configured = repo_store.has_object_store(); + let candidates = + select_spam_candidates(&rows, SPAM_BURST_TARGET_DID, store_configured, |repo| { + local_refs_on_disk(&repos_dir, &repo.owner_did, &repo.name) + }); + let remote_unverified_count = candidates.iter().filter(|c| c.remote_unverified).count(); + + info!( + target = SPAM_BURST_TARGET_DID, + scanned = rows.len(), + candidates = candidates.len(), + execute, + "purge-spam: candidate selection complete" + ); + + if candidates.is_empty() { + println!("purge-spam: no empty spam-burst repos found for {SPAM_BURST_TARGET_DID}"); + return Ok(PurgeSummary::default()); + } + + println!( + "purge-spam: {} candidate(s) for {} ({} mode)", + candidates.len(), + SPAM_BURST_TARGET_DID, + if execute { "EXECUTE" } else { "dry-run" } + ); + for c in &candidates { + let marker = if c.remote_unverified { + " [remote-only, emptiness verified under lock at execute]" + } else { + "" + }; + println!( + " {} owner={} name={} refs={}{marker}", + c.id, c.owner_did, c.name, c.ref_count + ); + } + + if !execute { + println!( + "purge-spam: dry-run — nothing deleted ({remote_unverified_count} remote-only, verified under lock only on --execute). Re-run with --execute to delete the {} candidate(s).", + candidates.len() + ); + return Ok(PurgeSummary { + remote_unverified: remote_unverified_count, + ..PurgeSummary::default() + }); + } + + // Re-verify emptiness immediately before deleting: a push may have landed + // between selection and now (TOCTOU). A remote-unverified candidate has no + // local copy yet, so `ref_count_on_disk` would report it non-empty and drop + // it here — pass it straight through instead; the authoritative emptiness + // check for it happens under the lock in the execute loop after refresh. + let (to_delete, to_skip) = partition_for_delete(&candidates, |c| { + if c.remote_unverified { + 0 + } else { + ref_count_on_disk(&repos_dir, &c.owner_did, &c.name) + } + }); + for c in &to_skip { + warn!(repo = %c.id, "purge-spam: repo no longer empty at delete time — skipped (TOCTOU)"); + } + + // Execute: delete per-repo, never a single blanket "delete all of owner X". + // A per-repo failure warns and continues rather than aborting the batch. + let mut summary = PurgeSummary { + remote_unverified: remote_unverified_count, + skipped_not_empty: to_skip.len(), + ..PurgeSummary::default() + }; + for c in &to_delete { + // Hold the per-repo advisory lock across the FINAL emptiness recheck and + // the delete, so a concurrent receive-pack cannot land a ref in the window + // between recheck and delete (M4). A repo currently locked by a live writer + // is skipped, never force-deleted out from under the push. + let guard = match repo_store.try_lock_repo(&c.owner_did, &c.name).await { + Ok(Some(g)) => g, + Ok(None) => { + warn!(repo = %c.id, "purge-spam: repo is locked by a live writer — skipped"); + summary.skipped_locked += 1; + continue; + } + Err(e) => { + warn!(repo = %c.id, err = %e, "purge-spam: could not acquire repo lock — skipped"); + summary.skipped_locked += 1; + continue; + } + }; + // Refresh the local copy from the authoritative object store (if any) + // before the recheck: on a Tigris deployment the admin node's local disk + // can be stale, so an emptiness check against local alone could delete a + // repo that has live remote refs. Fail closed on any store error — never + // delete on an unverified view. + if let Err(e) = repo_store.refresh_from_archive(&c.owner_did, &c.name).await { + warn!(repo = %c.id, err = %e, + "purge-spam: could not consult the object store — skipped (fail-closed)"); + summary.skipped_store_error += 1; + guard.release().await; + continue; + } + // Authoritative recheck UNDER the lock: a ref that landed before we locked + // (or that the object-store refresh surfaced) makes the repo non-empty, so + // it must not be deleted. + if ref_count_on_disk(&repos_dir, &c.owner_did, &c.name) != 0 { + warn!(repo = %c.id, "purge-spam: repo no longer empty under lock — skipped (TOCTOU)"); + summary.skipped_not_empty += 1; + guard.release().await; + continue; + } + match db.delete_repo_by_id(&c.id).await { + Ok(0) => { + warn!(repo = %c.id, "purge-spam: repo row already gone — nothing to delete"); + } + Ok(n) => { + summary.deleted += n; + // Remove the now-orphaned on-disk bare repo (empty, so cheap) so + // the DB row and disk stay consistent. Belt-and-suspenders: assert + // the resolved path is canonically INSIDE repos_dir before any + // remove_dir_all, so a symlinked slug dir or any residual traversal + // can never delete outside the repo root (the name is already + // validated at selection; this guards the destructive op itself). + // A disk-removal failure is counted separately, NOT folded into the + // deleted total, so the summary never reports a clean success while + // an on-disk dir survives (DB/disk drift the operator must fix). + let path = store::repo_disk_path(&repos_dir, &c.owner_did, &c.name); + if !path_within(&path, &repos_dir) { + warn!(repo = %c.id, path = %path.display(), + "purge-spam: on-disk path escapes repos_dir — refusing to remove"); + summary.disk_failed += 1; + } else if let Err(e) = std::fs::remove_dir_all(&path) { + warn!(repo = %c.id, path = %path.display(), err = %e, + "purge-spam: deleted DB row but could not remove on-disk repo dir"); + summary.disk_failed += 1; + } else { + info!(repo = %c.id, rows = n, "purge-spam: deleted repo row + on-disk dir"); + } + // Delete the object-store archive too (a no-op when no store is + // configured), else it survives and can be downloaded into a + // later repo created with the same owner/name. Counted separately + // so a surviving archive never reads as a clean success. + if let Err(e) = repo_store.delete_archive(&c.owner_did, &c.name).await { + warn!(repo = %c.id, err = %e, + "purge-spam: deleted row + dir but could not delete the object-store archive"); + summary.archive_failed += 1; + } + } + Err(e) => { + warn!(repo = %c.id, err = %e, "purge-spam: failed to delete repo row — continuing"); + } + } + guard.release().await; + } + println!( + "purge-spam: deleted {} repo row(s); skipped {} (no longer empty), {} (locked by a live writer), {} (object store unreachable); {} on-disk removal(s) failed, {} archive removal(s) failed.", + summary.deleted, + summary.skipped_not_empty, + summary.skipped_locked, + summary.skipped_store_error, + summary.disk_failed, + summary.archive_failed + ); + Ok(summary) +} + +#[cfg(test)] +mod tests { + use super::*; + use chrono::Utc; + + const TARGET: &str = SPAM_BURST_TARGET_DID; + const EXCLUDED_CONTENT: &str = EXCLUDED_DIDS[0]; // z6Mkk4L… content-bearing live user + const EXCLUDED_INTERN: &str = EXCLUDED_DIDS[1]; // z6MkqRz… intern/mirror-bot + const UNRELATED: &str = "did:key:z6MkUnrelatedStrangerDidThatIsNotTheBurst"; + + fn repo(id: &str, owner: &str, name: &str) -> RepoRecord { + RepoRecord { + id: id.to_string(), + name: name.to_string(), + owner_did: owner.to_string(), + description: None, + is_public: true, + default_branch: "main".to_string(), + created_at: Utc::now(), + updated_at: Utc::now(), + disk_path: format!("/srv/{id}.git"), + forked_from: None, + machine_id: None, + } + } + + /// Ref counts keyed by repo id; anything absent defaults to 0 (empty). + fn refs_by_id<'a>(map: &'a [(&'a str, usize)]) -> impl Fn(&RepoRecord) -> Option + 'a { + // A local bare repo exists for every test row (Some), with `n` refs. + move |r: &RepoRecord| { + Some( + map.iter() + .find(|(id, _)| *id == r.id) + .map(|(_, n)| *n) + .unwrap_or(0), + ) + } + } + + // Test 1: an empty repo owned by the target DID is a candidate. + #[test] + fn empty_target_repo_is_a_candidate() { + let repos = vec![repo("t-empty", TARGET, "spam1")]; + let got = select_spam_candidates(&repos, TARGET, false, refs_by_id(&[])); + assert_eq!(got.len(), 1, "empty target repo must be selected"); + assert_eq!(got[0].id, "t-empty"); + assert_eq!(got[0].owner_did, TARGET); + assert_eq!( + got[0].ref_count, 0, + "candidate must carry ref-count evidence" + ); + } + + // Test 2: a target-owned repo WITH refs is absent (per-repo empty check, not + // per-DID). + #[test] + fn target_repo_with_refs_is_absent() { + let repos = vec![ + repo("t-empty", TARGET, "spam1"), + repo("t-nonempty", TARGET, "real"), + ]; + let got = select_spam_candidates(&repos, TARGET, false, refs_by_id(&[("t-nonempty", 3)])); + let ids: Vec<&str> = got.iter().map(|c| c.id.as_str()).collect(); + assert!(ids.contains(&"t-empty"), "empty target repo still selected"); + assert!( + !ids.contains(&"t-nonempty"), + "a target repo WITH refs must NOT be selected (per-repo, not per-DID)" + ); + } + + // Test 3 (MUST ASSERT): an EMPTY repo owned by the excluded content DID is + // absent — the exclusion gate wins over the empty signature. + // + // Driven with the excluded DID passed AS the target so the exclusion gate is + // the ONLY barrier: with the gate removed this repo would match owner==target + // and be selected, so the test goes RED. This is what makes the gate + // load-bearing rather than shadowed by the target-scope check. + #[test] + fn empty_excluded_content_repo_is_absent() { + let repos = vec![repo("x-content", EXCLUDED_CONTENT, "anything")]; + let got = select_spam_candidates(&repos, EXCLUDED_CONTENT, false, refs_by_id(&[])); + assert!( + got.is_empty(), + "an empty repo owned by the excluded content DID must be excluded even \ + if that DID were the target, got {got:?}" + ); + // And of course it is also absent when the real burst DID is the target. + let got = select_spam_candidates(&repos, TARGET, false, refs_by_id(&[])); + assert!(got.is_empty()); + } + + // Test 4 (MUST ASSERT): an EMPTY repo owned by the intern DID is absent. + // Same construction as test 3: the intern DID is passed as the target so the + // exclusion gate is the sole reason it is dropped (RED without the gate). + #[test] + fn empty_intern_repo_is_absent() { + let repos = vec![repo("x-intern", EXCLUDED_INTERN, "mirror")]; + let got = select_spam_candidates(&repos, EXCLUDED_INTERN, false, refs_by_id(&[])); + assert!( + got.is_empty(), + "an empty repo owned by the intern/mirror-bot DID must be excluded even \ + if that DID were the target, got {got:?}" + ); + let got = select_spam_candidates(&repos, TARGET, false, refs_by_id(&[])); + assert!(got.is_empty()); + } + + // Test 5: an empty repo owned by an unrelated DID is absent — the tool targets + // the named burst only. + #[test] + fn empty_unrelated_repo_is_absent() { + let repos = vec![repo("u-empty", UNRELATED, "whatever")]; + let got = select_spam_candidates(&repos, TARGET, false, refs_by_id(&[])); + assert!( + got.is_empty(), + "an empty repo owned by a non-target DID must not be selected, got {got:?}" + ); + } + + // The full matrix in one selector pass: only the empty target repo survives. + #[test] + fn full_matrix_selects_only_empty_target() { + let repos = vec![ + repo("t-empty", TARGET, "spam1"), // selected + repo("t-nonempty", TARGET, "real"), // has refs → out + repo("x-content", EXCLUDED_CONTENT, "a"), // excluded, empty → out + repo("x-intern", EXCLUDED_INTERN, "b"), // excluded, empty → out + repo("u-empty", UNRELATED, "c"), // wrong owner → out + ]; + let got = select_spam_candidates(&repos, TARGET, false, refs_by_id(&[("t-nonempty", 2)])); + assert_eq!( + got.iter().map(|c| c.id.as_str()).collect::>(), + vec!["t-empty"], + "only the empty target repo may survive the full matrix" + ); + } + + // An excluded DID that is empty is still out even when that excluded DID is + // itself the target — pins that the exclusion gate runs BEFORE the owner/empty + // checks and is the sole barrier here (RED without the gate). + #[test] + fn exclusion_gate_precedes_empty_check() { + let repos = vec![repo("collision", EXCLUDED_CONTENT, "spam1")]; + let got = select_spam_candidates(&repos, EXCLUDED_CONTENT, false, refs_by_id(&[])); + assert!(got.is_empty(), "exclusion must win even on an empty repo"); + } + + // TOCTOU: a candidate that gained a ref between selection and the pre-delete + // re-check must be skipped, not deleted; the rest of the batch still deletes. + #[test] + fn partition_for_delete_skips_repos_no_longer_empty() { + let cand = |id: &str| Candidate { + id: id.into(), + owner_did: "o".into(), + name: id.into(), + ref_count: 0, + remote_unverified: false, + }; + let cands = vec![cand("still-empty"), cand("now-nonempty")]; + // Re-check reports the second repo as no longer empty. + let (to_delete, to_skip) = + partition_for_delete(&cands, |c| usize::from(c.id == "now-nonempty")); + assert_eq!( + to_delete.iter().map(|c| c.id.as_str()).collect::>(), + ["still-empty"] + ); + assert_eq!( + to_skip.iter().map(|c| c.id.as_str()).collect::>(), + ["now-nonempty"] + ); + } +} + +#[cfg(test)] +mod db_tests { + use super::*; + use crate::db::{Db, RepoRecord}; + use chrono::Utc; + use sqlx::PgPool; + + async fn db(pool: &PgPool) -> Db { + let db = Db::for_testing(pool.clone()); + db.run_migrations().await.unwrap(); + db + } + + /// Lock-only RepoStore over a test pool + repos_dir (no Tigris), for the purge + /// callers that now take the advisory lock (M4). + fn test_store(repos_dir: &Path, pool: &PgPool) -> crate::git::repo_store::RepoStore { + crate::git::repo_store::RepoStore::for_testing(repos_dir.to_path_buf(), pool.clone()) + } + + fn rec(id: &str, owner: &str, name: &str) -> RepoRecord { + RepoRecord { + id: id.to_string(), + name: name.to_string(), + owner_did: owner.to_string(), + description: None, + is_public: true, + default_branch: "main".to_string(), + created_at: Utc::now(), + updated_at: Utc::now(), + disk_path: format!("/srv/{id}.git"), + forked_from: None, + machine_id: None, + } + } + + async fn count_rows(db: &Db) -> i64 { + sqlx::query_scalar::<_, i64>("SELECT COUNT(*) FROM repos") + .fetch_one(db.pool()) + .await + .unwrap() + } + + // Test 6: a dry-run over the full matrix deletes nothing. The empty check is + // driven off a repos_dir with NO repos on disk, so every row reads as empty + // (list_refs on a missing repo returns Err → treated as non-empty and skipped), + // which is fine here: the assertion is that dry-run mutates no rows regardless. + #[sqlx::test] + async fn dry_run_deletes_nothing(pool: PgPool) { + let db = db(&pool).await; + for r in [ + rec("t-empty", SPAM_BURST_TARGET_DID, "spam1"), + rec("t-nonempty", SPAM_BURST_TARGET_DID, "real"), + rec("x-content", EXCLUDED_DIDS[0], "a"), + rec("x-intern", EXCLUDED_DIDS[1], "b"), + rec("u-empty", "did:key:z6MkUnrelated", "c"), + ] { + db.create_repo(&r).await.unwrap(); + } + let before = count_rows(&db).await; + assert_eq!(before, 5); + + // Materialize a REAL empty bare repo for the empty target so it is a genuine + // purge candidate. Without an on-disk candidate, run_purge_spam hits the + // no-candidate early return and the `if !execute { return }` guard is never + // exercised with candidates present — the L10 gap this test now closes. + let tmp = tempfile::TempDir::new().unwrap(); + let target_dir = store::repo_disk_path(tmp.path(), SPAM_BURST_TARGET_DID, "spam1"); + store::init_bare(&target_dir).unwrap(); + assert_eq!( + store::list_refs(&target_dir).unwrap().len(), + 0, + "precondition: the candidate is a real empty bare repo" + ); + + let store = test_store(tmp.path(), &pool); + let summary = run_purge_spam(&db, &store, tmp.path(), false) + .await + .unwrap(); + + // A candidate existed, but dry-run deletes nothing — DB row and on-disk dir + // both survive. RED if the `if !execute` guard is removed. + assert_eq!(summary.deleted, 0, "dry-run must delete no rows"); + let after = count_rows(&db).await; + assert_eq!(after, before, "dry-run must not delete any repo rows"); + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_some(), + "the candidate row must survive a dry-run" + ); + assert!( + target_dir.exists(), + "dry-run must not remove the on-disk repo dir" + ); + } + + // The DB accessor lists exactly the target DID's rows (exact owner match), and + // delete_repo_by_id removes exactly one row, so the execute path deletes per + // repo. This exercises the DB wiring end-to-end with a real empty repo on disk. + #[sqlx::test] + async fn execute_deletes_only_the_empty_target_repo_on_disk(pool: PgPool) { + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + + // One empty target repo (real bare repo, zero refs) and one target repo + // with a ref, plus an excluded-owner empty repo. + let empty = rec("t-empty", SPAM_BURST_TARGET_DID, "spam1"); + let nonempty = rec("t-refs", SPAM_BURST_TARGET_DID, "real"); + let excluded = rec("x-content", EXCLUDED_DIDS[0], "keep"); + for r in [&empty, &nonempty, &excluded] { + db.create_repo(r).await.unwrap(); + } + + // Materialize the two target repos on disk. + let empty_path = store::repo_disk_path(tmp.path(), &empty.owner_did, &empty.name); + store::init_bare(&empty_path).unwrap(); + let refs_path = store::repo_disk_path(tmp.path(), &nonempty.owner_did, &nonempty.name); + store::init_bare(&refs_path).unwrap(); + // Give the non-empty repo an actual ref. + seed_one_ref(&refs_path); + + // Sanity: our on-disk ref reader sees the expected counts. + assert_eq!(store::list_refs(&empty_path).unwrap().len(), 0); + assert!(!store::list_refs(&refs_path).unwrap().is_empty()); + + let repo_store = test_store(tmp.path(), &pool); + run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + // Only the empty target repo row is gone. + assert!(db + .get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_none()); + assert!(db + .get_repo(SPAM_BURST_TARGET_DID, "real") + .await + .unwrap() + .is_some()); + assert!(db + .get_repo(EXCLUDED_DIDS[0], "keep") + .await + .unwrap() + .is_some()); + } + + // Execute removes the on-disk bare repo dir too, so the DB row and disk stay + // consistent (no orphaned empty git dir left behind). + #[sqlx::test] + async fn execute_removes_the_on_disk_repo_dir(pool: PgPool) { + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + let empty = rec("t-empty", SPAM_BURST_TARGET_DID, "spam1"); + db.create_repo(&empty).await.unwrap(); + let path = store::repo_disk_path(tmp.path(), &empty.owner_did, &empty.name); + store::init_bare(&path).unwrap(); + assert!(path.exists(), "precondition: on-disk repo exists"); + + let repo_store = test_store(tmp.path(), &pool); + run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_none(), + "DB row deleted" + ); + assert!(!path.exists(), "on-disk bare repo dir must be removed too"); + } + + // U4 (M4): a repo whose per-repo advisory lock is held by a live writer must be + // SKIPPED by purge, not deleted out from under the push. Holds the lock via the + // same RepoStore (a separate pooled connection), runs execute, and asserts the + // row + on-disk dir survive; once the writer releases, purge deletes it. + #[sqlx::test] + async fn locked_repo_is_skipped_not_deleted(pool: PgPool) { + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + let empty = rec("t-empty", SPAM_BURST_TARGET_DID, "spam1"); + db.create_repo(&empty).await.unwrap(); + let path = store::repo_disk_path(tmp.path(), &empty.owner_did, &empty.name); + store::init_bare(&path).unwrap(); + + let repo_store = test_store(tmp.path(), &pool); + // A live writer holds the per-repo advisory lock. + let held = repo_store + .try_lock_repo(&empty.owner_did, &empty.name) + .await + .unwrap() + .expect("lock should be free initially"); + + run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + // Locked → skipped: both the row and the on-disk dir survive. + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_some(), + "a repo locked by a live writer must NOT be deleted" + ); + assert!(path.exists(), "on-disk dir must survive while locked"); + + // Once the writer releases, the empty repo is deleted (the lock was the + // only thing protecting it — baseline both ways). + held.release().await; + run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_none(), + "once unlocked, the empty repo is deleted" + ); + } + + // U5 (M6): a repo whose DB row is deleted but whose on-disk removal FAILS must + // be counted in `disk_failed`, never folded into a clean "deleted" success — + // else the summary reports success while the on-disk dir survives (DB/disk + // drift). Forces the failure by making the parent (slug) dir read-only. + #[sqlx::test] + async fn disk_removal_failure_is_counted_not_reported_as_success(pool: PgPool) { + use std::os::unix::fs::PermissionsExt; + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + let empty = rec("t-empty", SPAM_BURST_TARGET_DID, "spam1"); + db.create_repo(&empty).await.unwrap(); + let path = store::repo_disk_path(tmp.path(), &empty.owner_did, &empty.name); + store::init_bare(&path).unwrap(); + + // Read-only parent (slug) dir: remove_dir_all cannot unlink the repo dir. + let slug_dir = path.parent().unwrap().to_path_buf(); + let orig = std::fs::metadata(&slug_dir).unwrap().permissions(); + std::fs::set_permissions(&slug_dir, std::fs::Permissions::from_mode(0o555)).unwrap(); + + let repo_store = test_store(tmp.path(), &pool); + let summary = run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + // Restore perms so TempDir cleanup works regardless of assertion outcome. + std::fs::set_permissions(&slug_dir, orig).unwrap(); + + assert_eq!(summary.deleted, 1, "the DB row was deleted"); + assert_eq!( + summary.disk_failed, 1, + "a failed on-disk removal must be counted, not reported as clean success" + ); + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_none(), + "DB row is gone (delete succeeded)" + ); + assert!( + path.exists(), + "on-disk dir survived the failed removal (drift)" + ); + } + + // U1 (M3): a burst-owned row whose NAME traverses out of repos_dir must never + // cause a delete OUTSIDE repos_dir. Adversarial must-not: a real empty bare repo + // planted as a "victim" beside repos_dir is reachable from repos_dir// via + // a `../../victim` name; because the traversed path IS a real bare repo, the + // marker check passes and the candidate is selected — then remove_dir_all would + // delete the victim. The name validator must reject it so the victim survives. + #[sqlx::test] + async fn traversal_name_cannot_delete_a_repo_outside_repos_dir(pool: PgPool) { + let db = db(&pool).await; + let root = tempfile::TempDir::new().unwrap(); + let repos_dir = root.path().join("repos"); + std::fs::create_dir_all(&repos_dir).unwrap(); + + // The burst DID's own slug dir must exist for the OS to resolve the `..` + // segments (a burst that owns any normal repo already has this dir). + let slug = SPAM_BURST_TARGET_DID.replace([':', '/'], "_"); + std::fs::create_dir_all(repos_dir.join(&slug)).unwrap(); + + // Victim: a real empty bare repo OUTSIDE repos_dir (sibling under root). + let victim = root.path().join("victim.git"); + store::init_bare(&victim).unwrap(); + assert!(victim.join("HEAD").is_file(), "victim precondition"); + + // Sanity: the evil name resolves from repos_dir// onto the victim. + let evil = rec("evil", SPAM_BURST_TARGET_DID, "../../victim"); + let traversed = store::repo_disk_path(&repos_dir, &evil.owner_did, &evil.name); + assert_eq!( + std::fs::canonicalize(&traversed).unwrap(), + std::fs::canonicalize(&victim).unwrap(), + "test setup: the evil name must resolve onto the victim" + ); + + db.create_repo(&evil).await.unwrap(); + let repo_store = test_store(&repos_dir, &pool); + run_purge_spam(&db, &repo_store, &repos_dir, true) + .await + .unwrap(); + + assert!( + victim.join("HEAD").is_file(), + "a repo OUTSIDE repos_dir must never be deleted via a traversal name" + ); + } + + // The DB query normalizes did:key form (OWNER_KEY_CASE_SQL), so a burst repo + // stored in SHORT (bare) form is still found when querying by the full-form + // target DID — the SQL side of the normalization fix. + #[sqlx::test] + async fn list_repos_by_owner_did_matches_short_form(pool: PgPool) { + let db = db(&pool).await; + let short = crate::db::normalize_owner_key(SPAM_BURST_TARGET_DID); + assert_ne!(short, SPAM_BURST_TARGET_DID, "fixture must be short form"); + let repo = rec("short-owned", short, "spam"); + db.create_repo(&repo).await.unwrap(); + + let rows = db + .list_repos_by_owner_did(SPAM_BURST_TARGET_DID) + .await + .unwrap(); + assert!( + rows.iter().any(|r| r.id == "short-owned"), + "a short-form burst row must be found when querying by the full-form target" + ); + } + + /// Create a single commit + ref in a bare repo via a throwaway worktree, so the + /// repo reads as non-empty (≥1 ref). + fn seed_one_ref(bare: &Path) { + use std::process::Command; + let wt = bare.join("_seed"); + let run = |args: &[&str], dir: &Path| { + let ok = Command::new("git") + .args(args) + .current_dir(dir) + .status() + .unwrap() + .success(); + assert!(ok, "git {args:?} failed"); + }; + run( + &["worktree", "add", "--orphan", "-b", "main", "_seed"], + bare, + ); + std::fs::write(wt.join("f.txt"), b"x").unwrap(); + run(&["config", "user.email", "t@t"], &wt); + run(&["config", "user.name", "t"], &wt); + run(&["add", "."], &wt); + run(&["commit", "-qm", "seed"], &wt); + let _ = Command::new("git") + .args(["worktree", "remove", "--force", "_seed"]) + .current_dir(bare) + .status(); + } + + /// A `0` ref count must come only from a real bare repo at the exact path, + /// never from git discovery walking up to a parent `.git`. Load-bearing: the + /// tempdir root is itself a git repo (zero refs), so a naive `for-each-ref` + /// run from a child non-git dir would discover it and report 0 — the delete-a- + /// real-repo fail-open. The marker check must make that path fail closed. + #[test] + fn nongit_path_fails_closed_even_under_a_git_ancestor() { + let tmp = tempfile::TempDir::new().unwrap(); + // Make the tempdir root a git repo with zero refs (the discovery trap). + std::process::Command::new("git") + .args(["init", "-q"]) + .current_dir(tmp.path()) + .status() + .unwrap(); + + let repo = rec("t-x", SPAM_BURST_TARGET_DID, "spam1"); + let path = store::repo_disk_path(tmp.path(), &repo.owner_did, &repo.name); + + // (a) Path exists as a plain (non-git) directory under the git ancestor. + // Without the marker guard, git discovery would read the ancestor's 0 + // refs and this repo would be deleted. local_refs_on_disk must report no + // local repo (None) WITHOUT running list_refs — never the ancestor's 0. + // None is skipped unless a store is configured, and the under-lock + // recheck (ref_count_on_disk) fails closed on the same markers regardless. + std::fs::create_dir_all(&path).unwrap(); + assert_eq!( + local_refs_on_disk(tmp.path(), &repo.owner_did, &repo.name), + None, + "a non-git dir under a git ancestor must report no local repo, not read the ancestor's refs" + ); + assert_eq!( + ref_count_on_disk(tmp.path(), &repo.owner_did, &repo.name), + 1, + "the under-lock recheck must also fail closed on a non-git dir" + ); + + // (b) A real empty bare repo at the same path reads Some(0) — a candidate. + std::fs::remove_dir_all(&path).unwrap(); + store::init_bare(&path).unwrap(); + assert_eq!( + local_refs_on_disk(tmp.path(), &repo.owner_did, &repo.name), + Some(0) + ); + } + + /// The belt-and-suspenders containment gate (`path_within`) must reject a path + /// that resolves outside repos_dir even when the *name* itself is innocuous — + /// e.g. the owner slug dir is a symlink pointing elsewhere. Layer 1 (name + /// validation) can't see this; only the canonical-containment check catches it. + #[test] + fn path_within_rejects_symlink_escape() { + let root = tempfile::TempDir::new().unwrap(); + let repos_dir = root.path().join("repos"); + std::fs::create_dir_all(&repos_dir).unwrap(); + + // A real dir outside repos_dir, and a symlink INTO repos_dir that targets it. + let outside = root.path().join("outside.git"); + std::fs::create_dir_all(&outside).unwrap(); + let link = repos_dir.join("evil.git"); + std::os::unix::fs::symlink(&outside, &link).unwrap(); + + // The name is innocuous, but the path resolves outside repos_dir. + assert!( + !path_within(&link, &repos_dir), + "a symlink escaping repos_dir must fail the containment gate" + ); + // A genuine path inside repos_dir passes. + let inside = repos_dir.join("real.git"); + std::fs::create_dir_all(&inside).unwrap(); + assert!( + path_within(&inside, &repos_dir), + "an in-root path must pass" + ); + } + + /// The exclusion gate and the target scope must never overlap: if the burst + /// target were ever set to an excluded DID, the gate would fail to protect it. + #[test] + fn target_did_is_never_excluded() { + assert!( + !EXCLUDED_DIDS.contains(&SPAM_BURST_TARGET_DID), + "the purge target must never be an excluded (protected) DID" + ); + } + + /// Exclusion is normalization-consistent: an excluded identity stored in the + /// bare short form (as mirror upserts write it) is still excluded, even though + /// the exclusion constants are full did:key form — and an empty repo it owns + /// is never selected. + #[test] + fn short_form_excluded_did_is_still_protected() { + let short = crate::db::normalize_owner_key(EXCLUDED_DIDS[1]); // bare z6MkqRz… + assert_ne!( + short, EXCLUDED_DIDS[1], + "fixture must actually be short form" + ); + assert!( + is_excluded(short), + "short-form of an excluded DID must be excluded" + ); + + // An empty repo owned by the short-form excluded DID is spared even though + // its ref signature (0) otherwise matches the burst. + let empty_excluded_short = rec("x-short", short, "spam"); + let cands = select_spam_candidates( + &[empty_excluded_short], + SPAM_BURST_TARGET_DID, + false, + |_| Some(0), + ); + assert!( + cands.is_empty(), + "an empty repo owned by a short-form excluded DID must never be a candidate" + ); + } + + // ── Tigris-authoritative purge (P1b) ─────────────────────────────────── + + /// In-test object store. `download` materializes a bare repo with (or + /// without) a ref so the purge tool's authoritative recheck can be driven + /// without a live bucket; error flags exercise the fail-closed and + /// archive-delete-failure paths. + struct FakeStore { + has_archive: bool, + archive_has_refs: bool, + fail_recheck: bool, + fail_delete: bool, + deleted: std::sync::Arc, + } + + #[async_trait::async_trait] + impl crate::git::tigris::ObjectStore for FakeStore { + async fn exists(&self, _owner: &str, _repo: &str) -> Result { + if self.fail_recheck { + anyhow::bail!("fake object store unreachable"); + } + Ok(self.has_archive) + } + async fn upload(&self, _owner: &str, _repo: &str, _path: &Path) -> Result<()> { + Ok(()) + } + async fn download(&self, _owner: &str, _repo: &str, local_path: &Path) -> Result<()> { + if self.fail_recheck { + anyhow::bail!("fake object store unreachable"); + } + // Materialize the authoritative archive on local disk so + // ref_count_on_disk reflects remote state. + let _ = std::fs::remove_dir_all(local_path); + store::init_bare(local_path).unwrap(); + if self.archive_has_refs { + seed_one_ref(local_path); + } + Ok(()) + } + async fn delete(&self, _owner: &str, _repo: &str) -> Result<()> { + self.deleted + .store(true, std::sync::atomic::Ordering::SeqCst); + if self.fail_delete { + anyhow::bail!("fake object store delete failed"); + } + Ok(()) + } + } + + fn store_backed( + repos_dir: &Path, + pool: &PgPool, + fake: FakeStore, + ) -> crate::git::repo_store::RepoStore { + crate::git::repo_store::RepoStore::new( + repos_dir.to_path_buf(), + Some(std::sync::Arc::new(fake)), + pool.clone(), + ) + } + + fn fake( + has_archive: bool, + archive_has_refs: bool, + fail_recheck: bool, + fail_delete: bool, + ) -> (FakeStore, std::sync::Arc) { + let deleted = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + ( + FakeStore { + has_archive, + archive_has_refs, + fail_recheck, + fail_delete, + deleted: deleted.clone(), + }, + deleted, + ) + } + + // A locally-empty repo whose authoritative archive HAS refs (pushed via + // another machine) must NOT be purged on the stale-local view. Load-bearing: + // RED on the Tigris-blind purge (deletes it), GREEN once the recheck + // consults the object store. + #[sqlx::test] + async fn purge_skips_repo_with_remote_refs_when_local_empty(pool: PgPool) { + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + let target = rec("t-remote", SPAM_BURST_TARGET_DID, "spam1"); + db.create_repo(&target).await.unwrap(); + let path = store::repo_disk_path(tmp.path(), &target.owner_did, &target.name); + store::init_bare(&path).unwrap(); + assert_eq!( + store::list_refs(&path).unwrap().len(), + 0, + "local starts empty" + ); + + let (f, _deleted) = fake(true, true, false, false); + let repo_store = store_backed(tmp.path(), &pool, f); + let summary = run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_some(), + "a repo with live remote refs must not be purged on a stale-local view" + ); + assert_eq!(summary.deleted, 0); + assert_eq!(summary.skipped_not_empty, 1); + } + + // A genuinely-empty repo (empty archive) is deleted AND its archive removed, + // else the archive can be re-downloaded into a later same-name repo. + // Load-bearing: RED on the Tigris-blind purge (archive survives), GREEN once + // the delete wires the archive removal. + #[sqlx::test] + async fn purge_deletes_archive_on_successful_delete(pool: PgPool) { + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + let target = rec("t-empty", SPAM_BURST_TARGET_DID, "spam1"); + db.create_repo(&target).await.unwrap(); + let path = store::repo_disk_path(tmp.path(), &target.owner_did, &target.name); + store::init_bare(&path).unwrap(); + + let (f, deleted) = fake(true, false, false, false); + let repo_store = store_backed(tmp.path(), &pool, f); + let summary = run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_none(), + "a genuinely-empty repo is deleted" + ); + assert!( + deleted.load(std::sync::atomic::Ordering::SeqCst), + "the object-store archive must be deleted on a successful purge" + ); + assert_eq!(summary.deleted, 1); + assert_eq!(summary.archive_failed, 0); + } + + // An unreachable object store during the recheck must fail closed (skip), + // never delete on an unverified view. Load-bearing: RED on the Tigris-blind + // purge (deletes), GREEN once the recheck consults (and fails closed on) the + // store. + #[sqlx::test] + async fn purge_fails_closed_when_store_unreachable(pool: PgPool) { + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + let target = rec("t-empty", SPAM_BURST_TARGET_DID, "spam1"); + db.create_repo(&target).await.unwrap(); + let path = store::repo_disk_path(tmp.path(), &target.owner_did, &target.name); + store::init_bare(&path).unwrap(); + + let (f, _deleted) = fake(true, false, true, false); + let repo_store = store_backed(tmp.path(), &pool, f); + let summary = run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_some(), + "must not delete when the object store is unreachable (fail-closed)" + ); + assert_eq!(summary.deleted, 0); + assert_eq!(summary.skipped_store_error, 1); + } + + // An archive-delete failure after the row+dir are removed is counted + // separately, never folded into a clean success. + #[sqlx::test] + async fn purge_archive_delete_failure_counted_separately(pool: PgPool) { + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + let target = rec("t-empty", SPAM_BURST_TARGET_DID, "spam1"); + db.create_repo(&target).await.unwrap(); + let path = store::repo_disk_path(tmp.path(), &target.owner_did, &target.name); + store::init_bare(&path).unwrap(); + + let (f, deleted) = fake(true, false, false, true); + let repo_store = store_backed(tmp.path(), &pool, f); + let summary = run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_none(), + "the row+dir are still deleted" + ); + assert!(deleted.load(std::sync::atomic::Ordering::SeqCst)); + assert_eq!(summary.deleted, 1); + assert_eq!( + summary.archive_failed, 1, + "a surviving archive must be counted, not reported as clean success" + ); + } + + // AE3/R4: a repo that exists ONLY as an object-store archive (no local copy) + // with an EMPTY archive is reached, refreshed under the lock, and deleted + // (row + dir + archive). Pre-U4 a missing-local row was never a candidate + // (treated non-empty, skipped) -> RED; admitting it remote-unverified -> GREEN. + #[sqlx::test] + async fn purge_deletes_remote_only_empty_archive(pool: PgPool) { + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + let target = rec("t-remote-empty", SPAM_BURST_TARGET_DID, "spam1"); + db.create_repo(&target).await.unwrap(); + // NO local repo — it exists only as an archive. + let path = store::repo_disk_path(tmp.path(), &target.owner_did, &target.name); + assert!(!path.exists(), "no local copy — remote-only"); + + let (f, deleted) = fake(true, false, false, false); // archive exists, empty + let repo_store = store_backed(tmp.path(), &pool, f); + let summary = run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_none(), + "a remote-only empty archive must be reached and deleted" + ); + assert!( + deleted.load(std::sync::atomic::Ordering::SeqCst), + "the archive must be deleted too" + ); + assert_eq!(summary.deleted, 1); + assert_eq!( + summary.remote_unverified, 1, + "the candidate was admitted as remote-unverified" + ); + } + + // AE4/R4: a remote-only archive that turns out to HAVE refs is refreshed under + // the lock and then skipped — never deleted on the missing-local view. + #[sqlx::test] + async fn purge_skips_remote_only_archive_with_refs(pool: PgPool) { + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + let target = rec("t-remote-refs", SPAM_BURST_TARGET_DID, "spam1"); + db.create_repo(&target).await.unwrap(); + let path = store::repo_disk_path(tmp.path(), &target.owner_did, &target.name); + assert!(!path.exists()); + + let (f, _deleted) = fake(true, true, false, false); // archive exists, HAS refs + let repo_store = store_backed(tmp.path(), &pool, f); + let summary = run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_some(), + "a remote-only archive with refs must be refreshed and skipped, not deleted" + ); + assert_eq!(summary.deleted, 0); + assert_eq!(summary.skipped_not_empty, 1); + } + + // AE5/R5: no local copy AND no archive — the candidate is admitted (a store is + // configured) but the under-lock recheck finds nothing and fails closed. + #[sqlx::test] + async fn purge_skips_remote_unverified_with_no_archive(pool: PgPool) { + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + let target = rec("t-missing-both", SPAM_BURST_TARGET_DID, "spam1"); + db.create_repo(&target).await.unwrap(); + + let (f, _deleted) = fake(false, false, false, false); // NO archive + let repo_store = store_backed(tmp.path(), &pool, f); + let summary = run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_some(), + "missing local AND no archive must fail closed (not deleted)" + ); + assert_eq!(summary.deleted, 0); + assert_eq!( + summary.skipped_not_empty, 1, + "skipped by the authoritative under-lock recheck" + ); + } + + // R5: with NO object store, a repo with no local copy is not even a candidate + // (the missing-local admission is gated on a configured store). + #[sqlx::test] + async fn purge_storeless_missing_local_is_not_a_candidate(pool: PgPool) { + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + let target = rec("t-nolocal", SPAM_BURST_TARGET_DID, "spam1"); + db.create_repo(&target).await.unwrap(); + // Storeless RepoStore, no local repo on disk. + let repo_store = test_store(tmp.path(), &pool); + let summary = run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_some(), + "storeless + missing-local must fail closed — never a candidate" + ); + assert_eq!(summary.deleted, 0); + assert_eq!(summary.remote_unverified, 0); + } + + // R7: with no object store configured (single-machine), an empty repo is + // deleted exactly as before and nothing touches an archive. + #[sqlx::test] + async fn purge_tigris_disabled_deletes_empty_unchanged(pool: PgPool) { + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + let target = rec("t-empty", SPAM_BURST_TARGET_DID, "spam1"); + db.create_repo(&target).await.unwrap(); + let path = store::repo_disk_path(tmp.path(), &target.owner_did, &target.name); + store::init_bare(&path).unwrap(); + + let repo_store = test_store(tmp.path(), &pool); // Tigris = None + let summary = run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + assert!(db + .get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_none()); + assert_eq!(summary.deleted, 1); + assert_eq!(summary.skipped_store_error, 0); + assert_eq!(summary.archive_failed, 0); + } +} diff --git a/crates/gitlawb-node/src/api/repos.rs b/crates/gitlawb-node/src/api/repos.rs index b9cbc352..1656a83b 100644 --- a/crates/gitlawb-node/src/api/repos.rs +++ b/crates/gitlawb-node/src/api/repos.rs @@ -924,24 +924,46 @@ pub async fn git_receive_pack( } tracing::debug!(repo = %name, "acquiring write lock"); - let guard = state - .repo_store - .acquire_write(&record.owner_did, &record.name) - .await - .map_err(|e| { - tracing::error!(repo = %name, err = %e, "acquire_write failed"); - AppError::Git(e.to_string()) - })?; - let disk_path = guard.path().to_path_buf(); - tracing::debug!(repo = %name, path = %disk_path.display(), "running git receive-pack"); let body_len = body.len(); let git_timeout = std::time::Duration::from_secs(state.config.git_service_timeout_secs); - let receive_result = smart_http::receive_pack(&disk_path, body, git_timeout).await; - // Always release the advisory lock — even on error — to prevent stale locks - // from blocking subsequent pushes. Only upload to Tigris when the push - // succeeded; uploading a half-applied repo would propagate corruption. - guard.release(receive_result.is_ok()).await; + // Detach the acquire → receive-pack → release core from THIS handler future. + // The pack `body` is already fully buffered, so the spawned task is + // self-contained: a client disconnect drops the handler future but does NOT + // cancel the task, so a fully-received push still completes server-side — + // applies the pack, uploads, and releases the lock in order. What a + // disconnect forgoes is the cancellable post-push bookkeeping below, not the + // push itself. (KTD-3, R3.) + let repo_store = state.repo_store.clone(); + let owner_did = record.owner_did.clone(); + let repo_name = record.name.clone(); + let receive_result = tokio::spawn(async move { + let guard = repo_store + .acquire_write(&owner_did, &repo_name) + .await + .map_err(|e| { + tracing::error!(repo = %repo_name, err = %e, "acquire_write failed"); + AppError::Git(e.to_string()) + })?; + let disk_path = guard.path().to_path_buf(); + tracing::debug!(repo = %repo_name, path = %disk_path.display(), "running git receive-pack"); + let result = smart_http::receive_pack(&disk_path, body, git_timeout).await; + // Always release the advisory lock — even on error — to prevent stale + // locks from blocking subsequent pushes. Only upload to Tigris when the + // push succeeded; uploading a half-applied repo would propagate corruption. + guard.release(result.is_ok()).await; + Ok::<_, AppError>(result) + }) + .await + .map_err(|e| { + tracing::error!(repo = %name, err = %e, "receive-pack task panicked"); + AppError::Internal(anyhow::anyhow!("receive-pack task failed: {e}")) + })??; + + // Recompute the on-disk path for the post-push tail below — the guard that + // exposed it now lives inside the detached task. Identical to the path + // acquire_write resolved (repos_dir/owner_slug/name.git). + let disk_path = store::repo_disk_path(&state.config.repos_dir, &record.owner_did, &record.name); let result = receive_result.map_err(|e| { let app = git_service_app_error(&e); @@ -2579,6 +2601,165 @@ mod tests { ); } + // U2/R3/AE1: a fully-received push must complete server-side even when the + // client disconnects during the apply. The pack is buffered before the + // handler runs, so the acquire→receive→release core is detached from the + // handler future; dropping that future (the disconnect) must NOT cancel the + // push. A sleeping pre-receive hook creates a deterministic mid-apply window; + // dropping the handler during it kills the git group on the inline (pre-fix) + // code but not on the detached code, so the hook completes and the ref lands + // only after the fix. RED pre-fix (marker + ref absent), GREEN after. + #[sqlx::test] + async fn fully_received_push_completes_after_client_disconnect(pool: sqlx::PgPool) { + use axum::extract::{Path as AxPath, State}; + use axum::Extension; + use std::os::unix::fs::PermissionsExt; + use std::process::{Command, Stdio}; + use std::time::Duration; + + let mut state = crate::test_support::test_state(pool.clone()).await; + let repos_dir = tempfile::TempDir::new().unwrap(); + state.repo_store = crate::git::repo_store::RepoStore::for_testing( + repos_dir.path().to_path_buf(), + pool.clone(), + ); + + let owner = "z6u2pushowner"; + let name = "u2push"; + let owner_slug = owner.replace([':', '/'], "_"); + let bare = repos_dir + .path() + .join(&owner_slug) + .join(format!("{name}.git")); + + state + .db + .upsert_mirror_repo(owner, name, &bare.to_string_lossy(), None, false) + .await + .unwrap(); + + // Helper: run git, asserting success. + fn git(args: &[&str], dir: &std::path::Path) -> String { + let out = Command::new("git") + .args(args) + .current_dir(dir) + .output() + .unwrap(); + assert!( + out.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).trim().to_string() + } + + // Build a commit in a scratch working repo. + let work = tempfile::TempDir::new().unwrap(); + git(&["init", "-q", "-b", "main", "."], work.path()); + git(&["config", "user.email", "t@t"], work.path()); + git(&["config", "user.name", "t"], work.path()); + std::fs::write(work.path().join("f.txt"), "hi").unwrap(); + git(&["add", "f.txt"], work.path()); + git(&["commit", "-q", "-m", "c"], work.path()); + let oid = git(&["rev-parse", "HEAD"], work.path()); + + // Server bare repo: has the object (from the clone) but no refs/heads/main, + // so the push is a create satisfiable with an empty pack. + std::fs::create_dir_all(bare.parent().unwrap()).unwrap(); + let out = Command::new("git") + .args([ + "clone", + "--bare", + "-q", + &work.path().to_string_lossy(), + &bare.to_string_lossy(), + ]) + .output() + .unwrap(); + assert!( + out.status.success(), + "clone --bare failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + git( + &[ + "-C", + &bare.to_string_lossy(), + "update-ref", + "-d", + "refs/heads/main", + ], + work.path(), + ); + + // Sleeping pre-receive hook — the mid-apply window; writes a marker at the + // end so we can see the git child ran to completion. + let marker = repos_dir.path().join("hook_ran.marker"); + let hook = bare.join("hooks").join("pre-receive"); + std::fs::create_dir_all(hook.parent().unwrap()).unwrap(); + std::fs::write( + &hook, + format!( + "#!/bin/sh\ncat >/dev/null\nsleep 2\necho done > '{}'\nexit 0\n", + marker.display() + ), + ) + .unwrap(); + std::fs::set_permissions(&hook, std::fs::Permissions::from_mode(0o755)).unwrap(); + + // Receive-pack POST body: create refs/heads/main -> oid, plus an empty pack. + let zero = "0".repeat(40); + let cmd = format!("{zero} {oid} refs/heads/main\0report-status\n"); + let mut body = Vec::new(); + let len = cmd.len() + 4; + body.extend_from_slice(format!("{len:04x}").as_bytes()); + body.extend_from_slice(cmd.as_bytes()); + body.extend_from_slice(b"0000"); + let pack = Command::new("git") + .args([ + "-C", + &bare.to_string_lossy(), + "pack-objects", + "--stdout", + "-q", + ]) + .stdin(Stdio::null()) + .output() + .unwrap(); + assert!( + pack.status.success(), + "pack-objects failed: {}", + String::from_utf8_lossy(&pack.stderr) + ); + body.extend_from_slice(&pack.stdout); + let body = Bytes::from(body); + + // Drive the handler, then DROP it mid-hook (the "client disconnect"). + let handler = super::git_receive_pack( + State(state.clone()), + AxPath((owner.to_string(), format!("{name}.git"))), + Extension(crate::auth::AuthenticatedDid(owner.to_string())), + body, + ); + let _ = tokio::time::timeout(Duration::from_millis(800), handler).await; + + // Wait past the hook's sleep so a surviving (detached) push can finish. + tokio::time::sleep(Duration::from_millis(3000)).await; + + let ref_present = bare.join("refs/heads/main").exists() + || std::fs::read_to_string(bare.join("packed-refs")) + .unwrap_or_default() + .contains("refs/heads/main"); + assert!( + marker.exists(), + "pre-receive hook must run to completion despite the client disconnect (detached push)" + ); + assert!( + ref_present, + "refs/heads/main must be created after a fully-received push despite client disconnect" + ); + } + /// Repo creation must be throttled by the per-IP creation limiter BEFORE /// signature verification — otherwise a DID farm (one throwaway did:key per /// repo, each carrying a valid but machine-solved iCaptcha proof) walks past @@ -2625,4 +2806,359 @@ mod tests { "repo creation must be IP-throttled before signature verification" ); } + + // Shared request driver for the per-IP write-brake tests below: build a + // request with the given method/uri/headers/body, attach the socket peer as + // ConnectInfo (what the IP limiter keys on), send it through the router, and + // return the status. Mirrors post_from/post_with in rate_limit.rs. + async fn send_from( + router: &axum::Router, + method: axum::http::Method, + uri: &str, + headers: &[(&str, &str)], + body: axum::body::Body, + peer: std::net::SocketAddr, + ) -> axum::http::StatusCode { + use tower::ServiceExt; + let mut b = axum::http::Request::builder().method(method).uri(uri); + for (k, v) in headers { + b = b.header(*k, *v); + } + let mut req = b.body(body).unwrap(); + req.extensions_mut() + .insert(axum::extract::ConnectInfo(peer)); + router.clone().oneshot(req).await.unwrap().status() + } + + #[sqlx::test] + async fn write_route_is_rate_limited_by_ip(pool: sqlx::PgPool) { + use axum::body::Body; + use axum::http::{Method, StatusCode}; + use std::net::SocketAddr; + use std::time::Duration; + + let mut state = crate::test_support::test_state(pool).await; + // Tiny write bucket, keyed on the socket peer (no trusted proxy). + state.write_rate_limiter = crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60)); + state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; + + let peer: SocketAddr = "203.0.113.88:7000".parse().unwrap(); + // Exhaust this peer's single-request write budget up front. + assert!(state.write_rate_limiter.check(&peer.ip().to_string()).await); + + let router = crate::server::build_router(state); + // A write_routes sink (star). The IP brake is outermost, so the 429 + // fires before auth/handler — the path only needs to match. + let status = send_from( + &router, + Method::PUT, + "/api/v1/repos/someowner/somerepo/star", + &[], + Body::empty(), + peer, + ) + .await; + assert_eq!( + status, + StatusCode::TOO_MANY_REQUESTS, + "a write_routes sink must be IP-throttled before signature verification" + ); + } + + // KTD-1: the write bucket is separate from the creation bucket, so a write + // flood must not consume the creation budget (and vice versa). + #[sqlx::test] + async fn write_flood_does_not_drain_creation_budget(pool: sqlx::PgPool) { + use axum::body::Body; + use axum::http::{Method, StatusCode}; + use std::net::SocketAddr; + use std::time::Duration; + + let mut state = crate::test_support::test_state(pool).await; + // Exhaust the write bucket for this peer; leave the creation bucket ample. + state.write_rate_limiter = crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60)); + state.create_ip_rate_limiter = + crate::rate_limit::RateLimiter::new(1000, Duration::from_secs(60)); + state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; + + let peer: SocketAddr = "203.0.113.99:7000".parse().unwrap(); + assert!(state.write_rate_limiter.check(&peer.ip().to_string()).await); + + let router = crate::server::build_router(state); + + // Anchor the test: prove the write bucket is genuinely drained at the + // router (a write sink from this peer 429s) so the creation assertion + // below cannot pass vacuously on some unrelated non-429 status. + assert_eq!( + send_from( + &router, + Method::PUT, + "/api/v1/repos/someowner/somerepo/star", + &[], + Body::empty(), + peer, + ) + .await, + StatusCode::TOO_MANY_REQUESTS, + "write bucket must be drained for this peer (test precondition)" + ); + + // Creation from the same peer must NOT be 429 — its bucket is untouched. + // (It fails later on missing signature; the point is it is not throttled.) + let status = send_from( + &router, + Method::POST, + "/api/v1/repos", + &[("content-type", "application/json")], + Body::from(r#"{"name":"legit","is_public":true}"#), + peer, + ) + .await; + assert_ne!( + status, + StatusCode::TOO_MANY_REQUESTS, + "an exhausted write bucket must not throttle repo creation (separate buckets)" + ); + } + + // KTD-5: /graphql POST (the MutationRoot surface) draws from the write bucket. + #[sqlx::test] + async fn graphql_post_is_rate_limited_by_ip(pool: sqlx::PgPool) { + use axum::body::Body; + use axum::http::{Method, StatusCode}; + use std::net::SocketAddr; + use std::time::Duration; + + let mut state = crate::test_support::test_state(pool).await; + state.write_rate_limiter = crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60)); + state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; + + let peer: SocketAddr = "203.0.113.111:7000".parse().unwrap(); + assert!(state.write_rate_limiter.check(&peer.ip().to_string()).await); + + let router = crate::server::build_router(state); + let status = send_from( + &router, + Method::POST, + "/graphql", + &[("content-type", "application/json")], + Body::from(r#"{"query":"{ __typename }"}"#), + peer, + ) + .await; + assert_eq!( + status, + StatusCode::TOO_MANY_REQUESTS, + "/graphql must be IP-throttled by the write brake" + ); + } + + // Representative REST write group (issue comment) — same attachment as the + // task/bounty/profile groups. + #[sqlx::test] + async fn issue_comment_is_rate_limited_by_ip(pool: sqlx::PgPool) { + use axum::body::Body; + use axum::http::{Method, StatusCode}; + use std::net::SocketAddr; + use std::time::Duration; + + let mut state = crate::test_support::test_state(pool).await; + state.write_rate_limiter = crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60)); + state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; + + let peer: SocketAddr = "203.0.113.122:7000".parse().unwrap(); + assert!(state.write_rate_limiter.check(&peer.ip().to_string()).await); + + let router = crate::server::build_router(state); + let status = send_from( + &router, + Method::POST, + "/api/v1/repos/someowner/somerepo/issues/1/comments", + &[("content-type", "application/json")], + Body::from(r#"{"body":"flood"}"#), + peer, + ) + .await; + assert_eq!( + status, + StatusCode::TOO_MANY_REQUESTS, + "issue-write routes must be IP-throttled by the write brake" + ); + } + + // Adoption floor: an under-limit write must NOT be throttled. Guards against + // an off-by-one that braked the first request (invisible to the 429 tests, + // which all pre-exhaust the bucket). + #[sqlx::test] + async fn under_limit_write_is_not_throttled(pool: sqlx::PgPool) { + use axum::body::Body; + use axum::http::{Method, StatusCode}; + use std::net::SocketAddr; + use std::time::Duration; + + let mut state = crate::test_support::test_state(pool).await; + // Ample budget; bucket NOT exhausted. + state.write_rate_limiter = + crate::rate_limit::RateLimiter::new(100, Duration::from_secs(60)); + state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; + let peer: SocketAddr = "203.0.113.150:7000".parse().unwrap(); + + let router = crate::server::build_router(state); + assert_ne!( + send_from( + &router, + Method::PUT, + "/api/v1/repos/someowner/somerepo/star", + &[], + Body::empty(), + peer, + ) + .await, + StatusCode::TOO_MANY_REQUESTS, + "an under-limit write must pass the brake, not be 429'd" + ); + } + + // GITLAWB_WRITE_RATE_LIMIT=0 disables the brake end-to-end: no write is 429'd + // however many arrive from one IP. + #[sqlx::test] + async fn write_rate_limit_zero_disables_the_brake(pool: sqlx::PgPool) { + use axum::body::Body; + use axum::http::{Method, StatusCode}; + use std::net::SocketAddr; + use std::time::Duration; + + let mut state = crate::test_support::test_state(pool).await; + state.write_rate_limiter = crate::rate_limit::RateLimiter::new(0, Duration::from_secs(60)); + state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; + let peer: SocketAddr = "203.0.113.151:7000".parse().unwrap(); + + let router = crate::server::build_router(state); + for _ in 0..5 { + assert_ne!( + send_from( + &router, + Method::PUT, + "/api/v1/repos/someowner/somerepo/star", + &[], + Body::empty(), + peer, + ) + .await, + StatusCode::TOO_MANY_REQUESTS, + "a 0 write limit must disable the brake" + ); + } + } + + // The task/bounty/profile write groups share the write brake (same + // attachment as write_routes); prove each 429s at the route level. + #[sqlx::test] + async fn task_bounty_profile_writes_are_rate_limited(pool: sqlx::PgPool) { + use axum::body::Body; + use axum::http::{Method, StatusCode}; + use std::net::SocketAddr; + use std::time::Duration; + + let mut state = crate::test_support::test_state(pool).await; + state.write_rate_limiter = crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60)); + state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; + let peer: SocketAddr = "203.0.113.152:7000".parse().unwrap(); + assert!(state.write_rate_limiter.check(&peer.ip().to_string()).await); + + let router = crate::server::build_router(state); + for (method, uri) in [ + (Method::POST, "/api/v1/tasks"), + (Method::POST, "/api/v1/repos/o/r/bounties"), + (Method::PUT, "/api/v1/profile"), + ] { + assert_eq!( + send_from( + &router, + method, + uri, + &[("content-type", "application/json")], + Body::from("{}"), + peer, + ) + .await, + StatusCode::TOO_MANY_REQUESTS, + "write group {uri} must be IP-throttled by the write brake" + ); + } + } + + // /graphql/ws (subscriptions) is deliberately mounted AFTER the write brake + // layer, so it must stay unbraked even when the write bucket is exhausted. + #[sqlx::test] + async fn graphql_ws_is_not_braked(pool: sqlx::PgPool) { + use axum::body::Body; + use axum::http::{Method, StatusCode}; + use std::net::SocketAddr; + use std::time::Duration; + + let mut state = crate::test_support::test_state(pool).await; + state.write_rate_limiter = crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60)); + state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; + let peer: SocketAddr = "203.0.113.153:7000".parse().unwrap(); + assert!(state.write_rate_limiter.check(&peer.ip().to_string()).await); + + let router = crate::server::build_router(state); + // Not a real ws upgrade, so the subscription service rejects it with some + // non-429 status; the point is the write brake never sees it. + assert_ne!( + send_from( + &router, + Method::GET, + "/graphql/ws", + &[], + Body::empty(), + peer + ) + .await, + StatusCode::TOO_MANY_REQUESTS, + "/graphql/ws must not be behind the write brake" + ); + } + + // Adoption floor, per group: with an un-exhausted bucket, a write to EVERY + // braked group passes the brake (reaches auth/handler), not 429. Guards each + // group's grant path, not just the star representative. + #[sqlx::test] + async fn every_write_group_passes_under_limit(pool: sqlx::PgPool) { + use axum::body::Body; + use axum::http::{Method, StatusCode}; + use std::net::SocketAddr; + use std::time::Duration; + + let mut state = crate::test_support::test_state(pool).await; + state.write_rate_limiter = + crate::rate_limit::RateLimiter::new(100, Duration::from_secs(60)); + state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; + let peer: SocketAddr = "203.0.113.160:7000".parse().unwrap(); + + let router = crate::server::build_router(state); + for (method, uri) in [ + (Method::PUT, "/api/v1/repos/o/r/star"), + (Method::POST, "/graphql"), + (Method::POST, "/api/v1/repos/o/r/issues/1/comments"), + (Method::POST, "/api/v1/tasks"), + (Method::POST, "/api/v1/repos/o/r/bounties"), + (Method::PUT, "/api/v1/profile"), + ] { + assert_ne!( + send_from( + &router, + method, + uri, + &[("content-type", "application/json")], + Body::from(r#"{"query":"{ __typename }"}"#), + peer, + ) + .await, + StatusCode::TOO_MANY_REQUESTS, + "under-limit write to {uri} must pass the brake, not 429" + ); + } + } } diff --git a/crates/gitlawb-node/src/auth/mod.rs b/crates/gitlawb-node/src/auth/mod.rs index 720fb3ae..a9baa314 100644 --- a/crates/gitlawb-node/src/auth/mod.rs +++ b/crates/gitlawb-node/src/auth/mod.rs @@ -515,6 +515,7 @@ mod tests { repo_store: crate::git::repo_store::RepoStore::for_testing(PathBuf::from("/tmp"), pool), rate_limiter: RateLimiter::new(100, Duration::from_secs(60)), create_ip_rate_limiter: RateLimiter::new(1000, Duration::from_secs(3600)), + write_rate_limiter: RateLimiter::new(1000, Duration::from_secs(3600)), push_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)), push_limiter_trust: crate::rate_limit::TrustedProxy::None, sync_trigger_rate_limiter: RateLimiter::new(60, Duration::from_secs(3600)), diff --git a/crates/gitlawb-node/src/config.rs b/crates/gitlawb-node/src/config.rs index fc2247d9..06ff5081 100644 --- a/crates/gitlawb-node/src/config.rs +++ b/crates/gitlawb-node/src/config.rs @@ -1,18 +1,45 @@ -use clap::Parser; +use clap::{Parser, Subcommand}; use std::path::PathBuf; +/// Optional admin subcommands. When none is given, the binary runs the node +/// daemon as before — the default (no-subcommand) startup path is unchanged. +#[derive(Subcommand, Debug, Clone)] +pub enum Command { + /// Dry-run (default) or, with --execute, delete empty spam-burst repos owned + /// by the known burst DID. Never touches the hard-excluded DIDs, and verifies + /// each repo is empty (zero git refs) per repo before selecting it. + PurgeSpam { + /// Actually delete the candidates. Omit for a dry-run that prints the + /// candidate list and deletes nothing. + #[arg(long, default_value_t = false)] + execute: bool, + }, +} + #[derive(Parser, Debug, Clone)] #[command(name = "gitlawb-node", about = "gitlawb node daemon", version)] pub struct Config { - /// Directory where bare git repositories are stored - #[arg(long, env = "GITLAWB_REPOS_DIR", default_value = "./data/repos")] + /// Admin subcommand to run instead of the node daemon. Absent = run the node. + #[command(subcommand)] + pub command: Option, + + /// Directory where bare git repositories are stored. `global` so it can + /// follow a subcommand (e.g. `gitlawb-node purge-spam --repos-dir …`). + #[arg( + long, + env = "GITLAWB_REPOS_DIR", + default_value = "./data/repos", + global = true + )] pub repos_dir: PathBuf, - /// PostgreSQL connection URL (Supabase or any Postgres instance) + /// PostgreSQL connection URL (Supabase or any Postgres instance). `global` so + /// admin subcommands accept it in either position. #[arg( long, env = "DATABASE_URL", - default_value = "postgresql://localhost/gitlawb" + default_value = "postgresql://localhost/gitlawb", + global = true )] pub database_url: String, diff --git a/crates/gitlawb-node/src/db/mod.rs b/crates/gitlawb-node/src/db/mod.rs index 5324a4b9..a963f544 100644 --- a/crates/gitlawb-node/src/db/mod.rs +++ b/crates/gitlawb-node/src/db/mod.rs @@ -1259,6 +1259,106 @@ impl Db { .await?; Ok(()) } + + /// Every repo row whose owner resolves to `owner_did` under did:key + /// normalization, so `did:key:z6…` and bare `z6…` rows of the same identity + /// both match (mirroring how ownership is resolved everywhere else via + /// `OWNER_KEY_CASE_SQL` / the `idx_repos_owner_key_name` index). No dedup, so a + /// caller enumerating one DID's repos sees every physical row. Backs the + /// `purge-spam` admin tool, whose selection then applies its own per-repo empty + /// check and exclusion gate (both normalization-consistent). Ordered by `id`. + pub async fn list_repos_by_owner_did(&self, owner_did: &str) -> Result> { + let sql = format!( + "SELECT id, name, owner_did, description, is_public, default_branch, + created_at, updated_at, disk_path, forked_from, machine_id + FROM repos WHERE {key} = $1 ORDER BY id", + key = OWNER_KEY_CASE_SQL + ); + let rows = sqlx::query(&sql) + .bind(normalize_owner_key(owner_did)) + .fetch_all(&self.pool) + .await?; + Ok(rows.into_iter().map(row_to_repo).collect()) + } + + /// Delete a single repo row by its primary key `id`. Returns the number of + /// rows removed (0 if no such repo). Operates on one repo at a time by design: + /// the `purge-spam` tool deletes a vetted candidate list per-repo, never a + /// blanket "delete all repos of owner X". + pub async fn delete_repo_by_id(&self, id: &str) -> Result { + let mut tx = self.pool.begin().await?; + + // Resolve the row's identity so the `repo`-slug-keyed children (keyed on + // `normalize_owner_key(owner)/name`, not `repos.id`) can be matched. Absent + // row → nothing to delete; commit the empty tx and report 0. + let row: Option<(String, String)> = + sqlx::query_as("SELECT owner_did, name FROM repos WHERE id = $1") + .bind(id) + .fetch_optional(&mut *tx) + .await?; + let Some((owner_did, name)) = row else { + tx.commit().await?; + return Ok(0); + }; + let slug = format!("{}/{}", normalize_owner_key(&owner_did), name); + + // Grandchildren first: PR reviews/comments key on pr_id, so delete them + // before the parent PRs vanish. + for table in ["pr_reviews", "pr_comments"] { + sqlx::query(&format!( + "DELETE FROM {table} WHERE pr_id IN (SELECT id FROM pull_requests WHERE repo_id = $1)" + )) + .bind(id) + .execute(&mut *tx) + .await?; + } + + // Direct children keyed on repos.id. + for table in [ + "push_events", + "ref_certificates", + "pull_requests", + "webhooks", + "agent_tasks", + "protected_branches", + "repo_stars", + "repo_replicas", + "repo_labels", + "visibility_rules", + "encrypted_blobs", + "repo_icaptcha_proofs", + ] { + sqlx::query(&format!("DELETE FROM {table} WHERE repo_id = $1")) + .bind(id) + .execute(&mut *tx) + .await?; + } + + // Children keyed on the derived `owner_short/name` slug rather than the id. + for table in [ + "branch_cids", + "sync_queue", + "received_ref_updates", + "arweave_anchors", + ] { + sqlx::query(&format!("DELETE FROM {table} WHERE repo = $1")) + .bind(&slug) + .execute(&mut *tx) + .await?; + } + // NOTE: `bounties` (financial: amount/wallet/tx_hash) and `issue_comments` + // (no issues table to map issue_id → repo) are deliberately NOT cascaded + // here — dropping money records or unmappable rows on a repo delete would + // be wrong. They are left intact by design. + + let result = sqlx::query("DELETE FROM repos WHERE id = $1") + .bind(id) + .execute(&mut *tx) + .await?; + + tx.commit().await?; + Ok(result.rows_affected()) + } } // ── Agents / Trust ──────────────────────────────────────────────────────────── @@ -3537,6 +3637,92 @@ mod dedup_db_tests { } } + /// U2 (M7): deleting a repo must not orphan its child rows. Seeds one child in + /// a `repo_id`-keyed table (ref_certificates), one in a `repo`-slug-keyed table + /// (branch_cids), and a PR with a grandchild review (`pr_id`-keyed). Before the + /// transactional cascade these all survive `delete_repo_by_id` (RED); after, the + /// row and every child are gone (GREEN). + #[sqlx::test] + async fn delete_repo_by_id_removes_child_rows(pool: PgPool) { + let db = db(pool).await; + let owner = "did:key:z6MkChildOwnerFixtureForCascadeDelete"; + let repo = rec( + "rid-cascade", + owner, + "victim", + "d", + "2026-01-01T00:00:00Z", + "2026-01-01T00:00:00Z", + ); + db.create_repo(&repo).await.unwrap(); + let slug = format!("{}/victim", crate::db::normalize_owner_key(owner)); + + sqlx::query( + "INSERT INTO ref_certificates (id, repo_id, ref_name, old_sha, new_sha, pusher_did, node_did, signature, issued_at) + VALUES ('rc1', 'rid-cascade', 'refs/heads/main', '0', '1', 'p', 'n', 'sig', '2026-01-01T00:00:00Z')", + ).execute(db.pool()).await.unwrap(); + sqlx::query( + "INSERT INTO branch_cids (repo, ref_name, sha, cid, node_did, updated_at) + VALUES ($1, 'refs/heads/main', '1', 'cid', 'n', '2026-01-01T00:00:00Z')", + ) + .bind(&slug) + .execute(db.pool()) + .await + .unwrap(); + sqlx::query( + "INSERT INTO pull_requests (id, repo_id, number, title, author_did, source_branch, target_branch, created_at, updated_at) + VALUES ('pr1', 'rid-cascade', 1, 't', 'a', 'b', 'main', '2026-01-01T00:00:00Z', '2026-01-01T00:00:00Z')", + ).execute(db.pool()).await.unwrap(); + sqlx::query( + "INSERT INTO pr_reviews (id, pr_id, reviewer_did, status, created_at) + VALUES ('rev1', 'pr1', 'r', 'approved', '2026-01-01T00:00:00Z')", + ) + .execute(db.pool()) + .await + .unwrap(); + + let removed = db.delete_repo_by_id("rid-cascade").await.unwrap(); + assert_eq!(removed, 1, "parent repo row deleted"); + + async fn count(db: &Db, sql: &str, arg: &str) -> i64 { + sqlx::query_scalar::<_, i64>(sql) + .bind(arg) + .fetch_one(db.pool()) + .await + .unwrap() + } + assert_eq!( + count( + &db, + "SELECT COUNT(*) FROM ref_certificates WHERE repo_id=$1", + "rid-cascade" + ) + .await, + 0, + "repo_id-keyed child (ref_certificates) must be deleted" + ); + assert_eq!( + count(&db, "SELECT COUNT(*) FROM branch_cids WHERE repo=$1", &slug).await, + 0, + "slug-keyed child (branch_cids) must be deleted" + ); + assert_eq!( + count( + &db, + "SELECT COUNT(*) FROM pull_requests WHERE repo_id=$1", + "rid-cascade" + ) + .await, + 0, + "pull_requests must be deleted" + ); + assert_eq!( + count(&db, "SELECT COUNT(*) FROM pr_reviews WHERE pr_id=$1", "pr1").await, + 0, + "PR grandchild (pr_reviews) must be deleted with its parent PR" + ); + } + /// The canonical `did:key:` row and the short-owner mirror row of one logical /// repo collapse to a single deduped entry: the canonical row wins and inherits /// the group's most recent `updated_at`. diff --git a/crates/gitlawb-node/src/git/repo_store.rs b/crates/gitlawb-node/src/git/repo_store.rs index a5c367e9..f9a57c3a 100644 --- a/crates/gitlawb-node/src/git/repo_store.rs +++ b/crates/gitlawb-node/src/git/repo_store.rs @@ -18,17 +18,17 @@ use tokio::sync::Mutex; use tracing::{debug, info, warn}; use super::store; -use super::tigris::TigrisClient; +use super::tigris::ObjectStore; -/// Centralized repo storage: local disk cache + optional Tigris backend. +/// Centralized repo storage: local disk cache + optional object-store backend. #[derive(Clone)] pub struct RepoStore { repos_dir: PathBuf, - tigris: Option, + object_store: Option>, /// Shared Postgres pool for advisory locks. pool: PgPool, - /// Tracks repos already confirmed to exist in Tigris — avoids redundant - /// HEAD checks and background uploads for repos we've already migrated. + /// Tracks repos already confirmed to exist in the object store — avoids + /// redundant HEAD checks and background uploads for repos we've migrated. migrated: Arc>>, } @@ -37,16 +37,20 @@ impl RepoStore { pub fn for_testing(repos_dir: PathBuf, pool: PgPool) -> Self { Self { repos_dir, - tigris: None, + object_store: None, pool, migrated: Arc::new(tokio::sync::Mutex::new(std::collections::HashSet::new())), } } - pub fn new(repos_dir: PathBuf, tigris: Option, pool: PgPool) -> Self { + pub fn new( + repos_dir: PathBuf, + object_store: Option>, + pool: PgPool, + ) -> Self { Self { repos_dir, - tigris, + object_store, pool, migrated: Arc::new(Mutex::new(HashSet::new())), } @@ -63,14 +67,15 @@ impl RepoStore { if local_path.exists() { // Lazy migration: if Tigris is enabled and we haven't confirmed this // repo is in Tigris yet, check and upload in the background. - if let Some(ref tigris) = self.tigris { + if let Some(ref tigris) = self.object_store { let key = format!("{owner_slug}/{repo_name}"); let already_migrated = self.migrated.lock().await.contains(&key); if !already_migrated { + let store = self.clone(); let tigris = tigris.clone(); + let did = owner_did.to_string(); let slug = owner_slug.clone(); let name = repo_name.to_string(); - let path = local_path.clone(); let migrated = Arc::clone(&self.migrated); tokio::spawn(async move { // Check if already in Tigris before uploading @@ -80,8 +85,10 @@ impl RepoStore { } Ok(false) => { info!(repo = %name, "migrating local repo to tigris"); - if let Err(e) = tigris.upload(&slug, &name, &path).await { - warn!(repo = %name, err = %e, "lazy migration to tigris failed"); + // Upload under the per-repo lock so it can't race + // a purge. If it skipped (lock contended or dir + // gone), do NOT mark migrated — retry next acquire. + if !store.upload_locked(&did, &name, false).await { return; } info!(repo = %name, "lazy migration to tigris complete"); @@ -99,7 +106,7 @@ impl RepoStore { } // Try downloading from Tigris - if let Some(ref tigris) = self.tigris { + if let Some(ref tigris) = self.object_store { if tigris.exists(&owner_slug, repo_name).await.unwrap_or(false) { debug!(repo = %repo_name, "cache miss — downloading from tigris"); tigris @@ -127,7 +134,7 @@ impl RepoStore { pub async fn acquire_fresh(&self, owner_did: &str, repo_name: &str) -> Result { let (owner_slug, local_path) = self.local_path(owner_did, repo_name)?; - if let Some(ref tigris) = self.tigris { + if let Some(ref tigris) = self.object_store { if tigris.exists(&owner_slug, repo_name).await.unwrap_or(false) { debug!(repo = %repo_name, "acquire_fresh: downloading latest from tigris"); if let Err(e) = tigris.download(&owner_slug, repo_name, &local_path).await { @@ -157,93 +164,257 @@ impl RepoStore { let (owner_slug, local_path) = self.local_path(owner_did, repo_name)?; let lock_key = advisory_lock_key(&owner_slug, repo_name); - // Acquire Postgres advisory lock with retry using pg_try_advisory_lock - // to avoid blocking indefinitely on stale locks from crashed connections. - let mut acquired = false; - for attempt in 0..60 { - let row: (bool,) = sqlx::query_as("SELECT pg_try_advisory_lock($1)") - .bind(lock_key) - .fetch_one(&self.pool) - .await - .context("trying advisory lock")?; - if row.0 { - acquired = true; - break; + // Acquire the advisory lock on a DEDICATED connection, held for the + // guard's whole lifetime so the session-scoped lock lives on ONE + // connection and `release` unlocks it on that SAME connection, and so a + // concurrent `try_lock_repo`'s `pool.acquire()` cannot be handed the + // lock-owning connection and reentrantly re-grab it. Mirrors + // `RepoLockGuard`. Use pg_try_advisory_lock with retry to avoid blocking + // indefinitely on a stale lock from a crashed connection. + // + // Between FAILED attempts the connection is returned to the pool (the + // `drop` below) so a writer spinning on a contended repo does NOT hold a + // pool connection through the retry backoff — holding one per spinner + // would starve the shared pool under a same-repo write burst. Only the + // WINNING attempt keeps its connection (the lock lives on it). + let conn = { + let mut acquired = None; + for attempt in 0..60 { + let mut c = self + .pool + .acquire() + .await + .context("acquiring write-lock connection")?; + let row: (bool,) = sqlx::query_as("SELECT pg_try_advisory_lock($1)") + .bind(lock_key) + .fetch_one(&mut *c) + .await + .context("trying advisory lock")?; + if row.0 { + acquired = Some(c); + break; + } + drop(c); + if attempt < 59 { + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + } } - if attempt < 59 { - tokio::time::sleep(std::time::Duration::from_secs(1)).await; + match acquired { + Some(c) => c, + None => anyhow::bail!( + "could not acquire advisory lock after 60s — possible stale lock for {owner_slug}/{repo_name}" + ), } - } - if !acquired { - anyhow::bail!("could not acquire advisory lock after 60s — possible stale lock for {owner_slug}/{repo_name}"); - } + }; + + // Wrap the winning connection in the guard IMMEDIATELY, before any + // further await. The download below can be cancelled (the caller's + // future dropped mid-op) or fail; once the connection lives inside the + // guard, its `Drop` frees the advisory lock on every such exit. Before + // this ordering the lock orphaned on a bare connection returned to the + // pool holding it. (KTD-2.) + let guard = RepoWriteGuard { + owner_slug, + repo_name: repo_name.to_string(), + local_path, + lock_key, + conn: Some(conn), + object_store: self.object_store.clone(), + }; - // Always download the latest from Tigris before writing. + // Always download the latest from the object store before writing. // Local disk may be stale if another machine pushed since our last access. - if let Some(ref tigris) = self.tigris { - if tigris.exists(&owner_slug, repo_name).await.unwrap_or(false) { - debug!(repo = %repo_name, "write acquire: downloading latest from tigris"); - if let Err(e) = tigris.download(&owner_slug, repo_name, &local_path).await { + if let Some(ref store) = self.object_store { + if store + .exists(&guard.owner_slug, &guard.repo_name) + .await + .unwrap_or(false) + { + debug!(repo = %guard.repo_name, "write acquire: downloading latest from object store"); + if let Err(e) = store + .download(&guard.owner_slug, &guard.repo_name, &guard.local_path) + .await + { // Same self-healing fallback as acquire_fresh: a corrupt/unreadable - // Tigris archive must not block a write when a valid local copy + // archive must not block a write when a valid local copy // exists — release(success) will re-upload a good archive. - if local_path.exists() { - warn!(repo = %repo_name, err = %e, - "write acquire: tigris download failed — falling back to local copy"); + if guard.local_path.exists() { + warn!(repo = %guard.repo_name, err = %e, + "write acquire: object-store download failed — falling back to local copy"); } else { - return Err(e).context("downloading repo from tigris for write"); + // Dropping the guard frees the advisory lock (its `Drop` + // closes the connection). No Tigris upload on a bail, + // unlike release(true). + return Err(e).context("downloading repo from object store for write"); } } } } - Ok(RepoWriteGuard { - owner_slug, - repo_name: repo_name.to_string(), - local_path, + Ok(guard) + } + + /// Try to acquire ONLY the per-repo advisory lock, non-blocking, with no + /// Tigris I/O — the lightweight counterpart to [`acquire_write`](Self::acquire_write) + /// for out-of-band admin ops (purge-spam) that must mutually exclude a live + /// push during a destructive delete but never download/re-upload the repo. + /// + /// Returns `Some(guard)` if the lock was free, `None` if another writer holds + /// it (so the caller can skip rather than block). The guard holds a dedicated + /// pooled connection for its whole lifetime, so the lock lives on that one + /// connection and `release()` unlocks it on the SAME connection — a plain + /// pool query could unlock on a different connection and silently fail. + pub async fn try_lock_repo( + &self, + owner_did: &str, + repo_name: &str, + ) -> Result> { + let (owner_slug, _local) = self.local_path(owner_did, repo_name)?; + let lock_key = advisory_lock_key(&owner_slug, repo_name); + let mut conn = self + .pool + .acquire() + .await + .context("acquiring lock connection")?; + let row: (bool,) = sqlx::query_as("SELECT pg_try_advisory_lock($1)") + .bind(lock_key) + .fetch_one(&mut *conn) + .await + .context("try advisory lock")?; + if !row.0 { + return Ok(None); + } + Ok(Some(RepoLockGuard { + conn: Some(conn), lock_key, - pool: self.pool.clone(), - tigris: self.tigris.clone(), - }) + repo_name: repo_name.to_string(), + })) } /// Initialize a new bare repo on local disk and upload to Tigris. pub async fn init(&self, owner_did: &str, repo_name: &str) -> Result { - let (owner_slug, local_path) = self.local_path(owner_did, repo_name)?; + let (_owner_slug, local_path) = self.local_path(owner_did, repo_name)?; store::init_bare(&local_path).context("initializing bare repo")?; - // Upload to Tigris in background - if let Some(ref tigris) = self.tigris { - let tigris = tigris.clone(); - let owner_slug = owner_slug.clone(); - let repo_name = repo_name.to_string(); - let path = local_path.clone(); + // Upload to the object store in the background, UNDER the per-repo lock + // so the PUT can't race a purge. Skips on contention; a skipped init + // upload self-heals — the first write's release re-uploads the state. + if self.object_store.is_some() { + let store = self.clone(); + let did = owner_did.to_string(); + let name = repo_name.to_string(); tokio::spawn(async move { - if let Err(e) = tigris.upload(&owner_slug, &repo_name, &path).await { - warn!(repo = %repo_name, err = %e, "failed to upload new repo to tigris"); - } + store.upload_locked(&did, &name, false).await; }); } Ok(local_path) } - /// Upload a repo to Tigris after a write operation (push, merge, fork, etc.). - /// Call this after any operation that modifies the git repo on disk. + /// Upload a repo to Tigris after a write operation (fork, etc.). Call this + /// after any operation that modifies the git repo on disk. Uploads UNDER the + /// per-repo advisory lock so the PUT cannot race a concurrent purge (which + /// deletes the repo under the same lock) and resurrect a deleted archive. + /// Waits (bounded) for the lock — a skipped fork upload would have no later + /// retry; in practice the fork target's lock is uncontended (its DB row is + /// created after this upload). pub async fn release_after_write(&self, owner_did: &str, repo_name: &str) { - if let Some(ref tigris) = self.tigris { - let (owner_slug, local_path) = match self.local_path(owner_did, repo_name) { - Ok(p) => p, + self.upload_locked(owner_did, repo_name, true).await; + } + + /// Whether an object store is configured. Used by purge-spam to decide + /// whether a repo with no local copy can be a remote-unverified candidate + /// (its emptiness verified under the lock after a refresh) versus fail-closed. + pub fn has_object_store(&self) -> bool { + self.object_store.is_some() + } + + /// Upload the local repo to the object store while holding the per-repo + /// advisory lock, then release. The lock serializes the PUT against a + /// concurrent `purge-spam` that deletes the repo (row + dir + archive) under + /// the same lock, so an in-flight upload cannot resurrect a just-deleted + /// archive. Re-checks the local dir exists UNDER the lock — a purge removes + /// the dir under its lock, so a post-purge uploader finds it gone and skips. + /// Returns true iff a PUT was performed. A no-op when no store is configured. + /// + /// `wait`: fork's foreground upload waits (bounded) for the lock; the + /// background migration/init uploads pass `false` and skip on contention — + /// they self-heal (lazy migration retries on the next `acquire`, init's + /// state is re-uploaded by the first write's release). + async fn upload_locked(&self, owner_did: &str, repo_name: &str, wait: bool) -> bool { + let Some(ref store) = self.object_store else { + return false; + }; + let (owner_slug, local_path) = match self.local_path(owner_did, repo_name) { + Ok(p) => p, + Err(e) => { + warn!(repo = %repo_name, err = %e, "rejected unsafe path before object-store upload"); + return false; + } + }; + let guard = if wait { + match self.lock_repo_blocking(owner_did, repo_name).await { + Ok(Some(g)) => g, + Ok(None) => { + warn!(repo = %repo_name, "object-store upload skipped — repo lock still held after retry"); + return false; + } + Err(e) => { + warn!(repo = %repo_name, err = %e, "object-store upload skipped — could not acquire repo lock"); + return false; + } + } + } else { + match self.try_lock_repo(owner_did, repo_name).await { + Ok(Some(g)) => g, + Ok(None) => { + debug!(repo = %repo_name, "object-store upload skipped — repo locked by a live writer"); + return false; + } Err(e) => { - warn!(repo = %repo_name, err = %e, "rejected unsafe path in release_after_write"); - return; + warn!(repo = %repo_name, err = %e, "object-store upload skipped — could not acquire repo lock"); + return false; } - }; - if let Err(e) = tigris.upload(&owner_slug, repo_name, &local_path).await { - warn!(repo = %repo_name, err = %e, "failed to upload repo to tigris after write"); } + }; + // Re-check under the lock: a purge removes the on-disk dir under its lock, + // so a post-purge uploader must find it gone and NOT recreate the archive. + if !local_path.exists() { + warn!(repo = %repo_name, "object-store upload skipped — local repo dir gone under lock (purged?)"); + guard.release().await; + return false; } + let uploaded = match store.upload(&owner_slug, repo_name, &local_path).await { + Ok(()) => true, + Err(e) => { + warn!(repo = %repo_name, err = %e, "failed to upload repo to object store"); + false + } + }; + guard.release().await; + uploaded + } + + /// Bounded-wait lock-only acquire — the fork foreground upload's counterpart + /// to `acquire_write`'s spin, without any object-store I/O. Retries + /// `try_lock_repo` with short backoff. In practice the fork target's lock is + /// uncontended (its DB row is created after the upload), so this returns on + /// the first attempt. + async fn lock_repo_blocking( + &self, + owner_did: &str, + repo_name: &str, + ) -> Result> { + for attempt in 0..30 { + if let Some(g) = self.try_lock_repo(owner_did, repo_name).await? { + return Ok(Some(g)); + } + if attempt < 29 { + tokio::time::sleep(std::time::Duration::from_millis(200)).await; + } + } + Ok(None) } /// Compute the local disk path and owner slug for a repo. @@ -291,6 +462,34 @@ impl RepoStore { Ok((owner_slug, local_path)) } + + /// Refresh the local copy from the authoritative object-store archive so an + /// emptiness recheck reflects remote state (used by purge-spam on a Tigris + /// deployment, where the admin node's local disk can be stale). Downloads + /// the archive over the local path when it exists; a no-op when no object + /// store is configured (single-machine). Errors propagate so the caller can + /// fail closed rather than delete on a stale-local view. + pub async fn refresh_from_archive(&self, owner_did: &str, repo_name: &str) -> Result<()> { + let Some(ref store) = self.object_store else { + return Ok(()); + }; + let (owner_slug, local_path) = self.local_path(owner_did, repo_name)?; + if store.exists(&owner_slug, repo_name).await? { + store.download(&owner_slug, repo_name, &local_path).await?; + } + Ok(()) + } + + /// Delete the object-store archive for a repo. A no-op when no object store + /// is configured. Used by purge-spam so a deleted repo's archive cannot be + /// downloaded into a later repo created with the same owner/name. + pub async fn delete_archive(&self, owner_did: &str, repo_name: &str) -> Result<()> { + let Some(ref store) = self.object_store else { + return Ok(()); + }; + let (owner_slug, _local_path) = self.local_path(owner_did, repo_name)?; + store.delete(&owner_slug, repo_name).await + } } /// Strict allowlist validator for `owner_did` and `repo_name`. @@ -324,7 +523,7 @@ fn validate_owner_did(owner_did: &str) -> Result<()> { Ok(()) } -fn validate_repo_name(repo_name: &str) -> Result<()> { +pub(crate) fn validate_repo_name(repo_name: &str) -> Result<()> { if repo_name.is_empty() { anyhow::bail!("repo_name is empty"); } @@ -354,8 +553,13 @@ pub struct RepoWriteGuard { repo_name: String, pub local_path: PathBuf, lock_key: i64, - pool: PgPool, - tigris: Option, + /// Dedicated pooled connection the advisory lock lives on. Held for the + /// guard's lifetime so `release` unlocks on the SAME session that locked, + /// and so a concurrent `try_lock_repo` cannot be handed this connection and + /// reentrantly re-grab the lock. `Option` so `release` can take it (return + /// it to the pool) while `Drop` closes it when release was never reached. + conn: Option>, + object_store: Option>, } impl RepoWriteGuard { @@ -369,10 +573,10 @@ impl RepoWriteGuard { /// half-applied or otherwise inconsistent repo would propagate corruption to /// Tigris (and to every node that later downloads it). The lock is always /// released regardless, to avoid stale locks blocking future writes. - pub async fn release(self, success: bool) { + pub async fn release(mut self, success: bool) { // Upload to Tigris only on success. if success { - if let Some(ref tigris) = self.tigris { + if let Some(ref tigris) = self.object_store { if let Err(e) = tigris .upload(&self.owner_slug, &self.repo_name, &self.local_path) .await @@ -384,11 +588,67 @@ impl RepoWriteGuard { warn!(repo = %self.repo_name, "write failed — skipping tigris upload to avoid propagating an inconsistent repo"); } - // Release advisory lock - let _ = sqlx::query("SELECT pg_advisory_unlock($1)") - .bind(self.lock_key) - .execute(&self.pool) - .await; + // Unlock on the SAME connection that took the lock, then TAKE the + // connection so it returns to the pool on drop and the guard's `Drop` + // sees `None` (no close-on-drop). If this future is cancelled before the + // take completes, the connection stays in `self.conn` and `Drop` frees + // the lock by closing it instead. + if let Some(mut conn) = self.conn.take() { + let _ = sqlx::query("SELECT pg_advisory_unlock($1)") + .bind(self.lock_key) + .execute(&mut *conn) + .await; + } + } +} + +impl Drop for RepoWriteGuard { + fn drop(&mut self) { + // If `release()` was never reached (an early `?`, a panic, or the + // caller's future cancelled on client disconnect), the connection still + // holds the session advisory lock. Close it so Postgres frees the lock + // at session end, rather than returning a lock-holding connection to the + // pool where it would block every future write to this repo. (R1.) + if let Some(mut conn) = self.conn.take() { + warn!(repo = %self.repo_name, + "RepoWriteGuard dropped without release() — closing its connection to free the advisory lock"); + conn.close_on_drop(); + } + } +} + +/// Lock-only guard from [`RepoStore::try_lock_repo`]. Holds the Postgres advisory +/// lock (and the dedicated connection it lives on) until `release()`. No Tigris +/// I/O — unlike [`RepoWriteGuard`], releasing does NOT upload anything. +pub struct RepoLockGuard { + conn: Option>, + lock_key: i64, + repo_name: String, +} + +impl RepoLockGuard { + /// Release the advisory lock on the same connection that took it, then return + /// the connection to the pool. + pub async fn release(mut self) { + if let Some(mut conn) = self.conn.take() { + let _ = sqlx::query("SELECT pg_advisory_unlock($1)") + .bind(self.lock_key) + .execute(&mut *conn) + .await; + } + } +} + +impl Drop for RepoLockGuard { + fn drop(&mut self) { + // Same guarantee as RepoWriteGuard: a lock guard dropped without + // release() closes its connection so Postgres frees the advisory lock, + // rather than returning a lock-holding connection to the pool. (R1.) + if let Some(mut conn) = self.conn.take() { + warn!(repo = %self.repo_name, + "RepoLockGuard dropped without release() — closing its connection to free the advisory lock"); + conn.close_on_drop(); + } } } @@ -562,4 +822,531 @@ mod tests { ); } } + + // ── advisory-lock mutual exclusion (P1a) ──────────────────────────────── + + // A live `acquire_write` guard must exclude a concurrent `try_lock_repo` + // (the purge path) on the same repo. This is load-bearing for the purge + // tool's M4 mutual exclusion. + // + // The pool MUST be single-connection. `try_lock_repo`'s own return is the + // only observable that exposes the bug (it acquires through the pool), and + // it is only deterministic when the writer's connection is the ONLY one, so + // `try_lock_repo`'s `pool.acquire()` is forced onto it: + // * Pre-fix, `acquire_write` locks via `.fetch_one(&self.pool)` and returns + // the lock-owning connection to the pool. `try_lock_repo` re-acquires + // that same connection and `pg_try_advisory_lock` re-locks it + // reentrantly -> Ok(Some) (RED). (On a multi-connection pool the reentrant + // grab is non-deterministic: `acquire` may hand back a different idle + // connection, so the bug hides — verified by execution.) + // * Post-fix, `acquire_write` pins the connection, so `try_lock_repo`'s + // `pool.acquire()` cannot get one and times out -> Err. Either way the + // fix guarantees try_lock does NOT reentrantly grab the held lock. + #[sqlx::test] + async fn acquire_write_guard_blocks_try_lock( + pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: sqlx::postgres::PgConnectOptions, + ) { + let pool = pool_opts + .max_connections(1) + .acquire_timeout(std::time::Duration::from_secs(3)) + .connect_with(connect_opts) + .await + .unwrap(); + let tmp = tempfile::TempDir::new().unwrap(); + let store = RepoStore::for_testing(tmp.path().to_path_buf(), pool); + let owner = "did:key:z6MkWriterLockOwnerAAAAAAAAAAAAAAAAAAAAAAAA"; + let name = "locktest"; + + // A live writer holds the per-repo advisory lock. + let guard = store.acquire_write(owner, name).await.unwrap(); + + // The purge path must NOT reentrantly acquire the same lock while held. + // Pre-fix: Ok(Some) (reentrant grab). Post-fix: Err (connection pinned). + // The invariant is "not Ok(Some)". + let contended = store.try_lock_repo(owner, name).await; + assert!( + !matches!(contended, Ok(Some(_))), + "try_lock_repo must not reentrantly acquire a lock a live acquire_write \ + guard holds (got Ok(Some) — the reentrant-grab bug)" + ); + + // After the writer releases (on its pinned connection), the lock is free + // and the single connection is back in the pool. + guard.release(true).await; + let after = store.try_lock_repo(owner, name).await.unwrap(); + assert!( + after.is_some(), + "lock must be free once the writer releases it" + ); + after.unwrap().release().await; + } + + // The exclusion is real ACROSS sessions, not just the reentrant + // same-connection case: on a multi-connection pool the writer pins its + // connection, so `try_lock_repo`'s `pool.acquire()` gets a DIFFERENT + // connection whose `pg_try_advisory_lock` correctly observes the lock held + // -> Ok(None). This is the actual production topology (writer and purge on + // different sessions), and it also catches a pin-but-don't-actually-lock + // regression that the single-connection test (which passes via a + // pool-exhaustion Err) cannot distinguish from a real held lock. + #[sqlx::test] + async fn acquire_write_guard_excludes_try_lock_across_connections(pool: PgPool) { + let tmp = tempfile::TempDir::new().unwrap(); + let store = RepoStore::for_testing(tmp.path().to_path_buf(), pool); + let owner = "did:key:z6MkCrossConnExclusionAAAAAAAAAAAAAAAAAAAAA"; + let name = "xconn"; + + let guard = store.acquire_write(owner, name).await.unwrap(); + + // A second, distinct pooled connection must see the lock held. + let contended = store.try_lock_repo(owner, name).await.unwrap(); + assert!( + contended.is_none(), + "a live acquire_write guard must exclude try_lock_repo on a different \ + connection (Ok(None)); Ok(Some) would mean the lock is not held" + ); + + guard.release(true).await; + let after = store.try_lock_repo(owner, name).await.unwrap(); + assert!(after.is_some(), "lock is free after release"); + after.unwrap().release().await; + } + + // ── Drop-safety of the advisory-lock guards (U1: R1, R2; AE2) ─────────── + + // A minimal ObjectStore double: `exists()` is configurable, `download()` can + // park on a gate until notified (to hold `acquire_write` inside its + // post-lock await for the cancellation test), and `upload()` records its + // calls. Serves U1's reorder test and U3's serialization tests. + struct GatedStore { + // `exists` is dynamic so a delete() flips it false and an upload() flips + // it true — lets U3's resurrect test assert a deleted archive stays gone. + exists: std::sync::Arc, + download_gate: Option>, + uploads: std::sync::Arc>>, + deletes: std::sync::Arc>>, + } + + impl GatedStore { + fn new(exists: bool) -> Self { + Self { + exists: std::sync::Arc::new(std::sync::atomic::AtomicBool::new(exists)), + download_gate: None, + uploads: std::sync::Arc::new(std::sync::Mutex::new(Vec::new())), + deletes: std::sync::Arc::new(std::sync::Mutex::new(Vec::new())), + } + } + } + + #[async_trait::async_trait] + impl crate::git::tigris::ObjectStore for GatedStore { + async fn exists(&self, _o: &str, _r: &str) -> anyhow::Result { + Ok(self.exists.load(std::sync::atomic::Ordering::SeqCst)) + } + async fn upload(&self, o: &str, r: &str, _p: &std::path::Path) -> anyhow::Result<()> { + self.uploads + .lock() + .unwrap() + .push((o.to_string(), r.to_string())); + self.exists.store(true, std::sync::atomic::Ordering::SeqCst); + Ok(()) + } + async fn download(&self, _o: &str, _r: &str, _p: &std::path::Path) -> anyhow::Result<()> { + if let Some(gate) = &self.download_gate { + gate.notified().await; + } + Ok(()) + } + async fn delete(&self, o: &str, r: &str) -> anyhow::Result<()> { + self.deletes + .lock() + .unwrap() + .push((o.to_string(), r.to_string())); + self.exists + .store(false, std::sync::atomic::Ordering::SeqCst); + Ok(()) + } + } + + // Non-perturbing probe: true iff advisory lock `key` is free (grabs it and + // immediately releases it on the observer's own separate session). + async fn advisory_lock_is_free(conn: &mut sqlx::PgConnection, key: i64) -> bool { + let (got,): (bool,) = sqlx::query_as("SELECT pg_try_advisory_lock($1)") + .bind(key) + .fetch_one(&mut *conn) + .await + .unwrap(); + if got { + let _ = sqlx::query("SELECT pg_advisory_unlock($1)") + .bind(key) + .execute(&mut *conn) + .await; + } + got + } + + async fn poll_until_free(conn: &mut sqlx::PgConnection, key: i64) -> bool { + for _ in 0..50 { + if advisory_lock_is_free(conn, key).await { + return true; + } + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + } + false + } + + fn lock_key_for(owner: &str, name: &str) -> i64 { + advisory_lock_key(&owner.replace([':', '/'], "_"), name) + } + + // Build a pool whose idle/lifetime reaping is disabled, so a leaked + // (dropped-without-release) connection is NOT reclaimed by ambient sqlx + // maintenance during the poll window. Without this the default sqlx-test + // pool reaps the connection ~1.8s after drop, freeing the lock on its own + // and masking the leak — the test must observe ONLY the fix's own unlock. + async fn no_reap_pool( + pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: &sqlx::postgres::PgConnectOptions, + ) -> PgPool { + pool_opts + .max_connections(5) + .min_connections(0) + .idle_timeout(None) + .max_lifetime(None) + .test_before_acquire(false) + .connect_with(connect_opts.clone()) + .await + .unwrap() + } + + // R1/AE2: a RepoWriteGuard dropped WITHOUT release() must free its advisory + // lock — its connection is closed rather than returned to the pool still + // holding the session lock. Pre-fix (no Drop impl) the lock leaks -> RED. + #[sqlx::test] + async fn write_guard_dropped_without_release_frees_lock( + pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: sqlx::postgres::PgConnectOptions, + ) { + use sqlx::ConnectOptions; + let pool = no_reap_pool(pool_opts, &connect_opts).await; + let tmp = tempfile::TempDir::new().unwrap(); + let store = RepoStore::new(tmp.path().to_path_buf(), None, pool); + let owner = "did:key:z6MkDropFreesLockAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let name = "droptest"; + let key = lock_key_for(owner, name); + let mut observer = connect_opts.connect().await.unwrap(); + + let guard = store.acquire_write(owner, name).await.unwrap(); + assert!( + !advisory_lock_is_free(&mut observer, key).await, + "lock must be held while the guard is alive" + ); + + drop(guard); // NO release() + + assert!( + poll_until_free(&mut observer, key).await, + "advisory lock must be freed after a RepoWriteGuard is dropped without release()" + ); + } + + // R1: same guarantee for the purge lock-only guard. + #[sqlx::test] + async fn repo_lock_guard_dropped_without_release_frees_lock( + pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: sqlx::postgres::PgConnectOptions, + ) { + use sqlx::ConnectOptions; + let pool = no_reap_pool(pool_opts, &connect_opts).await; + let tmp = tempfile::TempDir::new().unwrap(); + let store = RepoStore::new(tmp.path().to_path_buf(), None, pool); + let owner = "did:key:z6MkLockGuardDropFreesAAAAAAAAAAAAAAAAAAAAAA"; + let name = "lockdrop"; + let key = lock_key_for(owner, name); + let mut observer = connect_opts.connect().await.unwrap(); + + let guard = store.try_lock_repo(owner, name).await.unwrap().unwrap(); + assert!( + !advisory_lock_is_free(&mut observer, key).await, + "lock must be held while the lock guard is alive" + ); + + drop(guard); // NO release() + + assert!( + poll_until_free(&mut observer, key).await, + "advisory lock must be freed after a RepoLockGuard is dropped without release()" + ); + } + + // R2/KTD-2: cancelling acquire_write DURING its post-lock freshness download + // must free the lock. Pre-reorder the lock is won onto a bare connection + // before the guard exists, so cancellation leaks it -> RED; post-reorder the + // guard wraps the connection first and its Drop frees the lock. + #[sqlx::test] + async fn acquire_write_cancelled_during_download_frees_lock( + pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: sqlx::postgres::PgConnectOptions, + ) { + use sqlx::ConnectOptions; + let pool = no_reap_pool(pool_opts, &connect_opts).await; + let tmp = tempfile::TempDir::new().unwrap(); + let mut ts = GatedStore::new(true); + ts.download_gate = Some(std::sync::Arc::new(tokio::sync::Notify::new())); + let store = RepoStore::new( + tmp.path().to_path_buf(), + Some(std::sync::Arc::new(ts)), + pool, + ); + let owner = "did:key:z6MkCancelDownloadFreesAAAAAAAAAAAAAAAAAAAA"; + let name = "canceldl"; + let key = lock_key_for(owner, name); + let mut observer = connect_opts.connect().await.unwrap(); + + { + let fut = store.acquire_write(owner, name); + tokio::pin!(fut); + tokio::select! { + _ = &mut fut => panic!("acquire_write should be parked in download, not complete"), + _ = tokio::time::sleep(std::time::Duration::from_millis(400)) => {} + } + // `fut` is dropped here — cancels acquire_write mid-download. + } + + assert!( + poll_until_free(&mut observer, key).await, + "advisory lock must be freed when acquire_write is cancelled mid-download" + ); + } + + // R2 negative: the normal release() path must NOT close the connection — it + // returns to the pool, so writes don't churn the pool. On a single-connection + // pool a second acquire_write can only succeed if the first returned its + // connection (unlocked, not closed). + #[sqlx::test] + async fn release_returns_connection_to_pool( + pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: sqlx::postgres::PgConnectOptions, + ) { + let pool = pool_opts + .max_connections(1) + .acquire_timeout(std::time::Duration::from_secs(3)) + .connect_with(connect_opts) + .await + .unwrap(); + let tmp = tempfile::TempDir::new().unwrap(); + let store = RepoStore::new(tmp.path().to_path_buf(), None, pool); + let owner = "did:key:z6MkReleasePoolsConnAAAAAAAAAAAAAAAAAAAAAAAA"; + let name = "releasepool"; + + for _ in 0..3 { + let guard = store.acquire_write(owner, name).await.unwrap(); + guard.release(true).await; + } + } + + // OQ1: settle whether a guard abandoned inside a detached task when the + // tokio runtime tears down panics in Drop. `PoolConnection::drop` spawns a + // task (both the close_on_drop path and the normal return path), and + // `rt::spawn` panics via `missing_rt` if no runtime handle is current. U2 + // makes receive-pack a detached task that graceful shutdown does not drain, + // so at process exit such a task can be dropped mid-flight holding a guard. + // This drops a real runtime with a parked guard-holding task and asserts the + // process survives (a panic-in-Drop during unwind would abort the binary). + #[test] + fn guard_drop_during_runtime_shutdown_does_not_panic() { + let url = match std::env::var("DATABASE_URL") { + Ok(u) => u, + Err(_) => return, // no DB configured — skip (mirrors sqlx::test gating) + }; + let rt = tokio::runtime::Builder::new_multi_thread() + .worker_threads(2) + .enable_all() + .build() + .unwrap(); + let ready = std::sync::Arc::new(tokio::sync::Notify::new()); + let ready2 = ready.clone(); + rt.block_on(async move { + let pool = sqlx::postgres::PgPoolOptions::new() + .max_connections(5) + .connect(&url) + .await + .unwrap(); + let tmp = tempfile::TempDir::new().unwrap(); + let store = RepoStore::new(tmp.path().to_path_buf(), None, pool); + tokio::spawn(async move { + let _guard = store + .acquire_write("did:key:z6MkOQ1ShutdownAAAAAAAAAAAAAAAAAAAAAAAAA", "oq1") + .await + .unwrap(); + ready2.notify_one(); + // Park forever holding the guard. + std::future::pending::<()>().await; + }); + ready.notified().await; + }); + // Drop the runtime while the detached task is parked holding the guard: + // the task is cancelled, dropping the guard during runtime teardown. Its + // Drop must not panic. Reaching the end of the test proves it didn't. + drop(rt); + } + + // ── U3: archive uploads serialize under the per-repo lock (R7, R8, R9) ─── + + fn repo_dir_of(repos_dir: &std::path::Path, owner: &str, name: &str) -> std::path::PathBuf { + repos_dir + .join(owner.replace([':', '/'], "_")) + .join(format!("{name}.git")) + } + + // R7/R8/AE6: init's background upload must SKIP while a live writer holds the + // repo lock. Pre-fix (unlocked upload) it PUTs regardless -> RED. + #[sqlx::test] + async fn init_upload_skips_while_repo_locked( + pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: sqlx::postgres::PgConnectOptions, + ) { + let pool = pool_opts + .max_connections(5) + .connect_with(connect_opts) + .await + .unwrap(); + let tmp = tempfile::TempDir::new().unwrap(); + let ts = GatedStore::new(false); + let uploads = ts.uploads.clone(); + let store = RepoStore::new( + tmp.path().to_path_buf(), + Some(std::sync::Arc::new(ts)), + pool, + ); + let owner = "did:key:z6MkInitSkipAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let name = "initskip"; + + let held = store.try_lock_repo(owner, name).await.unwrap().unwrap(); + store.init(owner, name).await.unwrap(); + // Let the spawned upload attempt-and-skip. + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + assert!( + uploads.lock().unwrap().is_empty(), + "init's background upload must skip while the repo is locked by a live writer" + ); + held.release().await; + } + + // R8/AE6: fork's foreground upload (release_after_write) WAITS for the lock + // rather than skipping, then PUTs once after it frees. Pre-fix it PUTs + // immediately while the lock is held -> RED on the "still empty" assert. + #[sqlx::test] + async fn release_after_write_waits_then_uploads_after_lock_frees( + pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: sqlx::postgres::PgConnectOptions, + ) { + let pool = pool_opts + .max_connections(5) + .connect_with(connect_opts) + .await + .unwrap(); + let tmp = tempfile::TempDir::new().unwrap(); + let ts = GatedStore::new(false); + let uploads = ts.uploads.clone(); + let store = RepoStore::new( + tmp.path().to_path_buf(), + Some(std::sync::Arc::new(ts)), + pool, + ); + let owner = "did:key:z6MkForkWaitAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let name = "forkwait"; + let slug = owner.replace([':', '/'], "_"); + std::fs::create_dir_all(repo_dir_of(tmp.path(), owner, name)).unwrap(); + + let held = store.try_lock_repo(owner, name).await.unwrap().unwrap(); + let store2 = store.clone(); + let owner2 = owner.to_string(); + let name2 = name.to_string(); + let h = tokio::spawn(async move { store2.release_after_write(&owner2, &name2).await }); + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + assert!( + uploads.lock().unwrap().is_empty(), + "fork upload must wait (not skip) while the repo lock is held" + ); + + held.release().await; + h.await.unwrap(); + let ups = uploads.lock().unwrap(); + assert_eq!( + ups.len(), + 1, + "fork upload must PUT exactly once after the lock frees" + ); + assert_eq!(ups[0], (slug, name.to_string())); + } + + // R9/AE6: after a purge deletes the archive AND removes the on-disk dir under + // the lock, a late upload (that lost the race) must NOT resurrect the archive + // — it finds the dir gone under the lock and skips. Pre-fix (no dir recheck) + // it re-PUTs and revives the archive -> RED. + #[sqlx::test] + async fn upload_does_not_resurrect_a_purged_archive(pool: sqlx::PgPool) { + use std::sync::atomic::Ordering::SeqCst; + let tmp = tempfile::TempDir::new().unwrap(); + let ts = GatedStore::new(false); + let uploads = ts.uploads.clone(); + let deletes = ts.deletes.clone(); + let exists = ts.exists.clone(); + let store = RepoStore::new( + tmp.path().to_path_buf(), + Some(std::sync::Arc::new(ts)), + pool, + ); + let owner = "did:key:z6MkResurrectAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let name = "resurrect"; + let dir = repo_dir_of(tmp.path(), owner, name); + std::fs::create_dir_all(&dir).unwrap(); + + store.release_after_write(owner, name).await; + assert!(exists.load(SeqCst), "archive exists after the first upload"); + assert_eq!(uploads.lock().unwrap().len(), 1); + + // Simulate a purge: delete the archive and remove the on-disk dir. + store.delete_archive(owner, name).await.unwrap(); + std::fs::remove_dir_all(&dir).unwrap(); + assert_eq!(deletes.lock().unwrap().len(), 1, "archive delete recorded"); + assert!(!exists.load(SeqCst), "archive is deleted by the purge"); + + // A late upload attempt must find the dir gone and skip, not resurrect. + store.release_after_write(owner, name).await; + assert!( + !exists.load(SeqCst), + "a purged archive must stay deleted — the upload found no dir and skipped" + ); + assert_eq!( + uploads.lock().unwrap().len(), + 1, + "no second PUT after the repo dir was purged" + ); + } + + // Negative: an uncontended fork upload PUTs exactly once (the lock is free). + #[sqlx::test] + async fn uncontended_release_after_write_uploads_once(pool: sqlx::PgPool) { + let tmp = tempfile::TempDir::new().unwrap(); + let ts = GatedStore::new(false); + let uploads = ts.uploads.clone(); + let store = RepoStore::new( + tmp.path().to_path_buf(), + Some(std::sync::Arc::new(ts)), + pool, + ); + let owner = "did:key:z6MkUncontendedAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let name = "uncontended"; + std::fs::create_dir_all(repo_dir_of(tmp.path(), owner, name)).unwrap(); + + store.release_after_write(owner, name).await; + assert_eq!( + uploads.lock().unwrap().len(), + 1, + "an uncontended fork upload must PUT exactly once" + ); + } } diff --git a/crates/gitlawb-node/src/git/tigris.rs b/crates/gitlawb-node/src/git/tigris.rs index ad26ddc5..9b447696 100644 --- a/crates/gitlawb-node/src/git/tigris.rs +++ b/crates/gitlawb-node/src/git/tigris.rs @@ -11,6 +11,21 @@ use anyhow::{Context, Result}; use aws_sdk_s3::Client as S3Client; use tracing::{debug, info}; +/// Object storage for git bare-repo archives. The seam that lets `RepoStore` +/// (and tests) depend on storage behavior without a live bucket. `TigrisClient` +/// is the production implementation. +#[async_trait::async_trait] +pub trait ObjectStore: Send + Sync { + /// Whether an archive exists for this repo. + async fn exists(&self, owner_slug: &str, repo_name: &str) -> Result; + /// Upload a local bare repo directory as an archive. + async fn upload(&self, owner_slug: &str, repo_name: &str, local_path: &Path) -> Result<()>; + /// Download and extract a repo archive to local disk. + async fn download(&self, owner_slug: &str, repo_name: &str, local_path: &Path) -> Result<()>; + /// Delete a repo archive. + async fn delete(&self, owner_slug: &str, repo_name: &str) -> Result<()>; +} + /// Wrapper around the S3 client with the configured bucket. #[derive(Clone)] pub struct TigrisClient { @@ -35,9 +50,12 @@ impl TigrisClient { fn repo_key(owner_slug: &str, repo_name: &str) -> String { format!("repos/v1/{owner_slug}/{repo_name}.tar.zst") } +} +#[async_trait::async_trait] +impl ObjectStore for TigrisClient { /// Check if a repo archive exists in Tigris. - pub async fn exists(&self, owner_slug: &str, repo_name: &str) -> Result { + async fn exists(&self, owner_slug: &str, repo_name: &str) -> Result { let key = Self::repo_key(owner_slug, repo_name); match self .s3 @@ -59,7 +77,7 @@ impl TigrisClient { } /// Upload a local bare repo directory to Tigris as a tar.zst archive. - pub async fn upload(&self, owner_slug: &str, repo_name: &str, local_path: &Path) -> Result<()> { + async fn upload(&self, owner_slug: &str, repo_name: &str, local_path: &Path) -> Result<()> { let key = Self::repo_key(owner_slug, repo_name); debug!(key = %key, path = %local_path.display(), "uploading repo to tigris"); @@ -89,12 +107,7 @@ impl TigrisClient { } /// Download a repo archive from Tigris and extract to local disk. - pub async fn download( - &self, - owner_slug: &str, - repo_name: &str, - local_path: &Path, - ) -> Result<()> { + async fn download(&self, owner_slug: &str, repo_name: &str, local_path: &Path) -> Result<()> { let key = Self::repo_key(owner_slug, repo_name); debug!(key = %key, path = %local_path.display(), "downloading repo from tigris"); @@ -128,8 +141,7 @@ impl TigrisClient { } /// Delete a repo archive from Tigris. - #[allow(dead_code)] - pub async fn delete(&self, owner_slug: &str, repo_name: &str) -> Result<()> { + async fn delete(&self, owner_slug: &str, repo_name: &str) -> Result<()> { let key = Self::repo_key(owner_slug, repo_name); self.s3 .delete_object() diff --git a/crates/gitlawb-node/src/main.rs b/crates/gitlawb-node/src/main.rs index aa0483db..83656be1 100644 --- a/crates/gitlawb-node/src/main.rs +++ b/crates/gitlawb-node/src/main.rs @@ -1,3 +1,4 @@ +mod admin; mod api; mod arweave; mod auth; @@ -70,6 +71,12 @@ async fn main() -> Result<()> { let mut config = Config::parse(); + // Admin subcommands run out-of-band and exit, never starting the node. The + // no-subcommand path below is the unchanged daemon startup. + if let Some(command) = config.command.clone() { + return run_admin_command(command, &config).await; + } + // Merge the embedded seed list of public network nodes into the runtime // bootstrap peers. Operators can opt out via GITLAWB_BOOTSTRAP_DISABLE_SEEDS. bootstrap::merge_seeds(&mut config); @@ -279,8 +286,10 @@ async fn main() -> Result<()> { None }; + let object_store = + tigris.map(|t| std::sync::Arc::new(t) as std::sync::Arc); let repo_store = - git::repo_store::RepoStore::new(config.repos_dir.clone(), tigris, db.pool().clone()); + git::repo_store::RepoStore::new(config.repos_dir.clone(), object_store, db.pool().clone()); // Per-DID limiter for the creation endpoints. Keyed on the authenticated // DID (attacker-varied), so bound its key set to cap memory. @@ -294,36 +303,24 @@ async fn main() -> Result<()> { // capped regardless of how many identities it mints. Sized well above any // legitimate per-IP creation rate; GITLAWB_CREATE_RATE_LIMIT overrides, 0 // disables. Bounded key set — the key is a client-influenced IP. - let create_limit = std::env::var("GITLAWB_CREATE_RATE_LIMIT") - .ok() - .and_then(|v| v.trim().parse::().ok()) - .unwrap_or(120); - let create_ip_rate_limiter = rate_limit::RateLimiter::new_bounded( - create_limit, - std::time::Duration::from_secs(3600), - 200_000, - ); - if create_limit == 0 { - tracing::warn!("GITLAWB_CREATE_RATE_LIMIT=0 — per-IP creation rate limiting disabled"); - } + let create_ip_rate_limiter = build_ip_limiter("GITLAWB_CREATE_RATE_LIMIT", 120, "creation"); + + // Per-IP brake for the authenticated non-creation write surface (issue/PR + // comments, labels, stars, merges, protect, replicas, visibility, tasks, + // bounties, profile, GraphQL mutations). Its own bucket, separate from the + // creation brake, so a write flood cannot drain the creation budget and vice + // versa (same rationale as the sync_trigger / peer_write split). Sized above + // any legitimate per-IP write rate — real agent automation makes many small + // writes per hour. GITLAWB_WRITE_RATE_LIMIT overrides; 0 disables. Bounded + // key set — the key is a client-influenced IP. + let write_rate_limiter = build_ip_limiter("GITLAWB_WRITE_RATE_LIMIT", 600, "write"); // Push-path flood brake: max git-receive-pack requests per client IP per // hour (counts both the info/refs advertisement and the push POST). Sized // for heavy agent automation while still stopping flood traffic (the June // 2026 attack pushed several times per second per IP). GITLAWB_PUSH_RATE_LIMIT // overrides; 0 disables. Bounded key set — the key is a client-influenced IP. - let push_limit = std::env::var("GITLAWB_PUSH_RATE_LIMIT") - .ok() - .and_then(|v| v.trim().parse::().ok()) - .unwrap_or(600); - let push_rate_limiter = rate_limit::RateLimiter::new_bounded( - push_limit, - std::time::Duration::from_secs(3600), - 200_000, - ); - if push_limit == 0 { - tracing::warn!("GITLAWB_PUSH_RATE_LIMIT=0 — per-IP push rate limiting disabled"); - } + let push_rate_limiter = build_ip_limiter("GITLAWB_PUSH_RATE_LIMIT", 600, "push"); // Which forwarded header the edge is trusted to set. Default None (trust // nothing, key on the socket peer). Fly nodes set GITLAWB_TRUSTED_PROXY=fly; @@ -331,7 +328,7 @@ async fn main() -> Result<()> { let push_limiter_trust = rate_limit::TrustedProxy::from_env_value( &std::env::var("GITLAWB_TRUSTED_PROXY").unwrap_or_default(), ); - tracing::info!(trust = ?push_limiter_trust, push_limit, "push rate limiter configured"); + tracing::info!(trust = ?push_limiter_trust, "push rate limiter trust configured"); // Peer-sync flood brakes, keyed on the resolved client IP (per-DID is useless // here — a did:key farm self-registers). Two buckets so an unsigned notify @@ -373,6 +370,7 @@ async fn main() -> Result<()> { repo_store, rate_limiter, create_ip_rate_limiter, + write_rate_limiter, push_rate_limiter, push_limiter_trust, sync_trigger_rate_limiter, @@ -408,22 +406,17 @@ async fn main() -> Result<()> { // Periodic cleanup of expired rate limit entries + consumed-proof ledger { - let rl = state.rate_limiter.clone(); - let create_ip_rl = state.create_ip_rate_limiter.clone(); - let push_rl = state.push_rate_limiter.clone(); - let sync_trigger_rl = state.sync_trigger_rate_limiter.clone(); - let peer_write_rl = state.peer_write_rate_limiter.clone(); + // Clone the whole state so the sweep uses AppState::cleanup_rate_limiters + // (the single source of truth that reaps EVERY limiter, including + // write_rate_limiter) rather than a hand-maintained list that can omit one. + let state = state.clone(); let db = state.db.clone(); let mut shutdown_rx = state.subscribe_shutdown(); tokio::spawn(async move { loop { tokio::select! { _ = tokio::time::sleep(std::time::Duration::from_secs(300)) => { - rl.cleanup().await; - create_ip_rl.cleanup().await; - push_rl.cleanup().await; - sync_trigger_rl.cleanup().await; - peer_write_rl.cleanup().await; + state.cleanup_rate_limiters().await; let now = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_secs() as i64) @@ -564,6 +557,102 @@ async fn main() -> Result<()> { Ok(()) } +/// Pure resolver for a usize rate-limit env value. Returns `(resolved, invalid)`: +/// an absent or empty value resolves to `default` and is NOT invalid; a non-empty +/// value that fails to parse resolves to `default` and IS invalid (so the caller +/// warns rather than silently disabling the intended, usually stricter, brake). +fn resolve_rate_limit(raw: Option<&str>, default: usize) -> (usize, bool) { + match raw.map(str::trim) { + None | Some("") => (default, false), + Some(t) => match t.parse::() { + Ok(n) => (n, false), + Err(_) => (default, true), + }, + } +} + +/// Read a rate-limit env var, warning on a present-but-unparseable value. A typo +/// like `GITLAWB_WRITE_RATE_LIMIT=5O` must not silently revert to the default and +/// leave the operator believing a stricter cap is in force. +fn rate_limit_from_env(var: &str, default: usize) -> usize { + let raw = std::env::var(var).ok(); + let (value, invalid) = resolve_rate_limit(raw.as_deref(), default); + if invalid { + tracing::warn!( + var, + value = raw.as_deref().unwrap_or(""), + default, + "invalid rate-limit value — using default; the intended brake is NOT applied" + ); + } + value +} + +/// Build a bounded per-client-IP rate limiter from an env var: resolve the limit +/// (honoring the present-but-unparseable warning in `rate_limit_from_env`), build +/// the limiter with the shared 1-hour window and 200k-key cap, and warn when the +/// limit is 0 (disabled). Collapses the identical create/write/push setup. +fn build_ip_limiter(var: &str, default: usize, label: &str) -> rate_limit::RateLimiter { + let limit = rate_limit_from_env(var, default); + let limiter = + rate_limit::RateLimiter::new_bounded(limit, std::time::Duration::from_secs(3600), 200_000); + if limit == 0 { + tracing::warn!("{var}=0 — per-IP {label} rate limiting disabled"); + } else { + tracing::info!(%var, limit, "per-IP {label} rate limiter configured"); + } + limiter +} + +/// Dispatch an admin subcommand. Connects the database directly (no server, no +/// p2p, no metrics) and runs the requested tool to completion, then exits. +async fn run_admin_command(command: config::Command, config: &Config) -> Result<()> { + match command { + config::Command::PurgeSpam { execute } => { + let acquire_timeout = std::time::Duration::from_secs(config.db_acquire_timeout_secs); + let db = Db::connect( + &config.database_url, + config.db_max_connections, + acquire_timeout, + ) + .await + .context("connecting to database for purge-spam")?; + // Build the object store so purge is authoritative against remote + // state: the emptiness recheck consults the archive (not just + // possibly-stale local disk) and a successful purge deletes the + // archive too. When no bucket is configured this stays None and purge + // is local-only, exactly as before. + let object_store: Option> = if config + .tigris_bucket + .is_empty() + { + None + } else { + match git::tigris::TigrisClient::new(&config.tigris_bucket).await { + Ok(client) => Some(std::sync::Arc::new(client)), + Err(e) => { + // Fail closed: without the configured store we cannot + // do the authoritative recheck, and a local-only purge + // on a Tigris deployment is the stale-view delete this + // guards against. Refuse to run rather than fall back. + anyhow::bail!( + "purge-spam: GITLAWB_TIGRIS_BUCKET is set but the object-store client failed to initialize ({e}); refusing to run a local-only purge on a Tigris deployment" + ); + } + } + }; + let repo_store = git::repo_store::RepoStore::new( + config.repos_dir.clone(), + object_store, + db.pool().clone(), + ); + admin::run_purge_spam(&db, &repo_store, &config.repos_dir, execute) + .await + .map(|_| ()) + } + } +} + fn spawn_shutdown_signal(tx: watch::Sender) { tokio::spawn(async move { #[cfg(unix)] @@ -1024,6 +1113,40 @@ fn load_or_create_keypair(config: &Config) -> Result { } } +#[cfg(test)] +mod rate_limit_env_tests { + use super::resolve_rate_limit; + + #[test] + fn absent_uses_default_without_warning() { + assert_eq!(resolve_rate_limit(None, 600), (600, false)); + } + + #[test] + fn empty_or_whitespace_uses_default_without_warning() { + assert_eq!(resolve_rate_limit(Some(""), 600), (600, false)); + assert_eq!(resolve_rate_limit(Some(" "), 600), (600, false)); + } + + #[test] + fn valid_value_parses() { + assert_eq!(resolve_rate_limit(Some("50"), 600), (50, false)); + assert_eq!(resolve_rate_limit(Some(" 50 "), 600), (50, false)); + // 0 is a VALID value (disables the brake); the ==0 warning is handled + // separately at the call site. It must NOT be flagged invalid here. + assert_eq!(resolve_rate_limit(Some("0"), 600), (0, false)); + } + + #[test] + fn present_but_unparseable_defaults_and_flags_invalid() { + // The L8 case: a fat-fingered stricter cap ("5O", letter O) must default + // AND be flagged so the caller warns, not silently disable the brake. + assert_eq!(resolve_rate_limit(Some("5O"), 600), (600, true)); + assert_eq!(resolve_rate_limit(Some("abc"), 600), (600, true)); + assert_eq!(resolve_rate_limit(Some("-1"), 600), (600, true)); + } +} + #[cfg(test)] mod gossip_ssrf_tests { use super::ping_peer_health; diff --git a/crates/gitlawb-node/src/rate_limit.rs b/crates/gitlawb-node/src/rate_limit.rs index d40e2691..1d38a0ae 100644 --- a/crates/gitlawb-node/src/rate_limit.rs +++ b/crates/gitlawb-node/src/rate_limit.rs @@ -130,6 +130,24 @@ impl RateLimiter { !w.timestamps.is_empty() }); } + + /// Test-only: insert a fully-expired entry (no live timestamps) that the next + /// `cleanup()` must reclaim. Models a key whose window has aged out, without + /// depending on wall-clock sleeps or a short window. Used cross-module by the + /// AppState cleanup guard test (H2). + #[cfg(test)] + pub(crate) async fn insert_reclaimable_for_test(&self, key: &str) { + self.state + .lock() + .await + .insert(key.to_string(), Window { timestamps: vec![] }); + } + + /// Test-only: number of distinct keys currently tracked. + #[cfg(test)] + pub(crate) async fn tracked_keys(&self) -> usize { + self.state.lock().await.len() + } } pub async fn rate_limit_by_did(request: Request, next: Next) -> Response { @@ -605,4 +623,104 @@ mod tests { "an exhausted IP must be braked with 429 before auth runs, not leak to 401" ); } + + // ── Adversarial TrustedProxy verification through the middleware (U5) ── + // These complement the client_key unit tests above by driving hostile + // headers through rate_limit_by_ip on a real router: the property under test + // is that the *bucket key* cannot be rotated by a spoofer. + + async fn post_with( + router: &axum::Router, + peer: SocketAddr, + hdrs: &[(&str, &str)], + ) -> StatusCode { + use tower::ServiceExt; + let mut b = axum::http::Request::builder() + .method(axum::http::Method::POST) + .uri("/api/v1/repos"); + for (k, v) in hdrs { + b = b.header(*k, *v); + } + let mut req = b.body(axum::body::Body::empty()).unwrap(); + req.extensions_mut().insert(ConnectInfo(peer)); + router.clone().oneshot(req).await.unwrap().status() + } + + #[tokio::test] + async fn xff_spoofed_leftmost_hop_cannot_rotate_bucket() { + // XForwardedFor mode, budget 1. A spoofer varies the client-controlled + // leftmost hop every request but the trusted proxy always appends the + // same rightmost hop. All requests must key to the rightmost hop, so the + // second is braked despite a fresh leftmost value. + let router = ip_limited_over_auth_router(IpRateLimiter { + limiter: RateLimiter::new(1, Duration::from_secs(60)), + trust: TrustedProxy::XForwardedFor, + }); + let p1: SocketAddr = "10.0.0.1:1".parse().unwrap(); + let p2: SocketAddr = "10.0.0.2:1".parse().unwrap(); + // First request passes the brake, reaches (failing) auth. + assert_eq!( + post_with(&router, p1, &[("x-forwarded-for", "9.9.9.1, 203.0.113.5")]).await, + StatusCode::UNAUTHORIZED + ); + // Different leftmost + different socket peer, SAME rightmost trusted hop: + // braked, because the key is the rightmost hop, not the spoofed value. + assert_eq!( + post_with(&router, p2, &[("x-forwarded-for", "9.9.9.2, 203.0.113.5")]).await, + StatusCode::TOO_MANY_REQUESTS, + "a spoofer varying the leftmost XFF hop must not rotate its bucket key" + ); + } + + #[tokio::test] + async fn xff_distinct_real_clients_get_distinct_buckets() { + // Distinct trusted (rightmost) hops are distinct clients: neither throttles + // the other, so a busy legitimate client cannot starve a different one. + let router = ip_limited_over_auth_router(IpRateLimiter { + limiter: RateLimiter::new(1, Duration::from_secs(60)), + trust: TrustedProxy::XForwardedFor, + }); + let peer: SocketAddr = "10.0.0.9:1".parse().unwrap(); + assert_eq!( + post_with( + &router, + peer, + &[("x-forwarded-for", "1.1.1.1, 203.0.113.5")] + ) + .await, + StatusCode::UNAUTHORIZED + ); + assert_eq!( + post_with( + &router, + peer, + &[("x-forwarded-for", "1.1.1.1, 203.0.113.6")] + ) + .await, + StatusCode::UNAUTHORIZED, + "a different trusted client hop must get its own bucket" + ); + } + + #[tokio::test] + async fn absent_trusted_header_collapses_to_socket_peer() { + // The documented fallback: in a trusted-proxy mode, a request with no + // forwarded header keys on the socket peer. Behind a real edge every + // request shares the proxy's socket, so they collapse onto one bucket — + // asserted here by name so any change to this behavior is deliberate. + let router = ip_limited_over_auth_router(IpRateLimiter { + limiter: RateLimiter::new(1, Duration::from_secs(60)), + trust: TrustedProxy::XForwardedFor, + }); + let proxy: SocketAddr = "172.16.0.1:1".parse().unwrap(); + assert_eq!( + post_with(&router, proxy, &[]).await, + StatusCode::UNAUTHORIZED + ); + assert_eq!( + post_with(&router, proxy, &[]).await, + StatusCode::TOO_MANY_REQUESTS, + "with no trusted header, requests fall back to the socket peer key (collapse)" + ); + } } diff --git a/crates/gitlawb-node/src/server.rs b/crates/gitlawb-node/src/server.rs index f4c0d3e3..1456b700 100644 --- a/crates/gitlawb-node/src/server.rs +++ b/crates/gitlawb-node/src/server.rs @@ -55,18 +55,50 @@ fn add_auth_layers(router: Router, state: AppState) -> Router Self; +} + +impl WriteBraked for Router { + fn write_braked(self, limiter: &rate_limit::IpRateLimiter) -> Self { + self.layer(middleware::from_fn(rate_limit::rate_limit_by_ip)) + .layer(axum::Extension(limiter.clone())) + } +} + pub fn build_router(state: AppState) -> Router { // ── GraphQL routes ───────────────────────────────────────────────────── let schema = state.graphql_schema.as_ref().clone(); + + // Per-IP write brake, shared across every authenticated non-creation write + // group (see `AppState::write_rate_limiter`). Built once here so each group + // clones the same bucket; attached outermost on each group so a flood is + // rejected before auth runs. Separate bucket from the creation brake. + let write_ip_limiter = rate_limit::IpRateLimiter { + limiter: state.write_rate_limiter.clone(), + trust: state.push_limiter_trust, + }; + let graphql_routes = Router::new() .route("/graphql", get(graphql_playground).post(graphql_handler)) // Attach the verified DID to /graphql when a signature is present. The // layer covers only routes added before it, so /graphql/ws (added after, - // read-only subscriptions) stays open. + // read-only subscriptions) stays open. The per-IP write brake covers the + // same /graphql (KTD-5: an HTTP-layer brake cannot tell a query POST from + // a mutation POST, so it counts every /graphql request — acceptable, both + // cost work); /graphql/ws subscriptions stay unbraked. .layer(middleware::from_fn(auth::optional_signature)) + .write_braked(&write_ip_limiter) .route_service("/graphql/ws", GraphQLSubscription::new(schema)); - // ── Task routes (write — require HTTP Signature) ─────────────────────── + // ── Task routes (write — require HTTP Signature + per-IP write brake) ── let task_write_routes = add_auth_layers( Router::new() .route("/api/v1/tasks", post(tasks::create_task)) @@ -74,7 +106,8 @@ pub fn build_router(state: AppState) -> Router { .route("/api/v1/tasks/{id}/complete", post(tasks::complete_task)) .route("/api/v1/tasks/{id}/fail", post(tasks::fail_task)), state.clone(), - ); + ) + .write_braked(&write_ip_limiter); // ── Task routes (read — open) ────────────────────────────────────────── let task_read_routes = Router::new() @@ -111,7 +144,13 @@ pub fn build_router(state: AppState) -> Router { .layer(middleware::from_fn(rate_limit::rate_limit_by_ip)) .layer(axum::Extension(create_ip_limiter)); - // ── Write routes — require HTTP Signature (no rate limit) ───────────── + // ── Write routes — require HTTP Signature + per-IP write brake ──────── + // Same per-IP flood defense as the creation routes, on its own `write` + // bucket (see `AppState::write_rate_limiter`). Per-DID is deliberately not + // paired (a DID farm never trips it; busy legitimate agents would false- + // positive). The bundled visibility GET is a read that also draws from the + // write bucket — harmless, since the bucket is sized far above any legit + // per-IP read rate. let write_routes = add_auth_layers( Router::new() .route( @@ -181,7 +220,8 @@ pub fn build_router(state: AppState) -> Router { axum::routing::delete(agents::deregister_agent), ), state.clone(), - ); + ) + .write_braked(&write_ip_limiter); // Body limit is raised to GITLAWB_MAX_PACK_BYTES (default 2 GB) for git // routes only — all other API routes keep axum's default 2 MB cap. @@ -247,7 +287,8 @@ pub fn build_router(state: AppState) -> Router { post(bounties::dispute_bounty), ), state.clone(), - ); + ) + .write_braked(&write_ip_limiter); // ── Bounty routes (read — open) ────────────────────────────────────── let bounty_read_routes = Router::new() @@ -268,9 +309,10 @@ pub fn build_router(state: AppState) -> Router { let profile_write_routes = add_auth_layers( Router::new().route("/api/v1/profile", axum::routing::put(profiles::set_profile)), state.clone(), - ); + ) + .write_braked(&write_ip_limiter); - // ── Issue routes (write — require HTTP Signature, no rate limit) ───── + // ── Issue routes (write — require HTTP Signature + per-IP write brake) ─ let issue_write_routes = add_auth_layers( Router::new() .route( @@ -282,7 +324,8 @@ pub fn build_router(state: AppState) -> Router { post(issues::create_issue_comment), ), state.clone(), - ); + ) + .write_braked(&write_ip_limiter); // ── Peer discovery routes ───────────────────────────────────────────── // Peer writes accept signatures when present and can require them after a diff --git a/crates/gitlawb-node/src/state.rs b/crates/gitlawb-node/src/state.rs index 6a84b3c2..6c033392 100644 --- a/crates/gitlawb-node/src/state.rs +++ b/crates/gitlawb-node/src/state.rs @@ -61,6 +61,16 @@ pub struct AppState { /// resolved client IP is what actually stops a single-source flood (same /// rationale as `push_rate_limiter`). Keyed by `push_limiter_trust`. pub create_ip_rate_limiter: RateLimiter, + /// Per-client-IP rate limiter for the authenticated write surface that is + /// not repo/agent creation: issue/PR comments, labels, stars, merges, + /// protect/unprotect, replicas, visibility, tasks, bounties, profile, and + /// the GraphQL `MutationRoot`. Separate bucket from `create_ip_rate_limiter` + /// (its own budget, per the `sync_trigger`/`peer_write` precedent) so a + /// write flood cannot drain the creation budget and vice versa. Per-DID is + /// deliberately NOT paired here: a DID farm never trips it, and busy + /// legitimate agents would false-positive. `GITLAWB_WRITE_RATE_LIMIT` + /// overrides the default, 0 disables. Keyed by `push_limiter_trust`. + pub write_rate_limiter: RateLimiter, /// Per-client-IP rate limiter for git-receive-pack. Per-DID limits cannot /// brake a push flood from a DID farm (one throwaway DID per repo), so the /// push path throttles on the resolved client IP instead. @@ -117,4 +127,48 @@ impl AppState { pub fn is_shutting_down(&self) -> bool { *self.shutdown_tx.borrow() } + + /// Reclaim expired entries from EVERY rate limiter. Single source of truth for + /// the periodic cleanup loop (main.rs), co-located with the limiter fields so a + /// newly-added limiter is cleaned here rather than silently omitted from a + /// hand-maintained list in main() (H2: `write_rate_limiter` was the omitted + /// one). Every limiter field above MUST be swept here; the guard test + /// `cleanup_rate_limiters_reaps_the_write_limiter` fails if it is not. + pub(crate) async fn cleanup_rate_limiters(&self) { + self.rate_limiter.cleanup().await; + self.create_ip_rate_limiter.cleanup().await; + self.write_rate_limiter.cleanup().await; + self.push_rate_limiter.cleanup().await; + self.sync_trigger_rate_limiter.cleanup().await; + self.peer_write_rate_limiter.cleanup().await; + } +} + +#[cfg(test)] +mod tests { + /// H2 guard: the write-surface limiter must be reaped by the shared cleanup + /// routine. Seeds a reclaimable entry into `write_rate_limiter` and asserts + /// `cleanup_rate_limiters` removes it. Goes RED if `write_rate_limiter` is + /// dropped from `cleanup_rate_limiters` — the exact H2 omission, now guarded. + #[tokio::test] + async fn cleanup_rate_limiters_reaps_the_write_limiter() { + let state = crate::test_support::test_state_lazy(); + state + .write_rate_limiter + .insert_reclaimable_for_test("some-ip") + .await; + assert_eq!( + state.write_rate_limiter.tracked_keys().await, + 1, + "precondition: the reclaimable entry is tracked" + ); + + state.cleanup_rate_limiters().await; + + assert_eq!( + state.write_rate_limiter.tracked_keys().await, + 0, + "write_rate_limiter must be reaped by cleanup_rate_limiters (H2)" + ); + } } diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 4fe52f64..687b5f90 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -77,6 +77,7 @@ fn build_state(db: Arc, pool: PgPool) -> AppState { repo_store: crate::git::repo_store::RepoStore::for_testing(PathBuf::from("/tmp"), pool), rate_limiter: RateLimiter::new(100, Duration::from_secs(60)), create_ip_rate_limiter: RateLimiter::new(1000, Duration::from_secs(3600)), + write_rate_limiter: RateLimiter::new(1000, Duration::from_secs(3600)), push_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)), push_limiter_trust: crate::rate_limit::TrustedProxy::None, sync_trigger_rate_limiter: RateLimiter::new(60, Duration::from_secs(3600)),