diff --git a/.env.example b/.env.example index cf032ac7..d6a5e4ae 100644 --- a/.env.example +++ b/.env.example @@ -365,6 +365,24 @@ GITLAWB_RATE_LIMIT_MAX=10 # limiter. Default 3600. GITLAWB_RATE_LIMIT_WINDOW_SECS=3600 +# ── Write rate limiting (non-creation authenticated writes) ─────────────── +# Max requests per client IP per hour on the shared write bucket. It is debited +# by: issue and PR comments, issue close, labels, stars, merges, protect and +# unprotect, replicas, webhook create/delete, visibility (including the GET on +# that group), tasks, bounties, profile, and every HTTP request to /graphql. +# The /graphql brake wraps the whole route, so queries and the playground GET +# consume this bucket too, not only mutations (GraphQL WebSocket subscriptions +# are excluded). Issue comment and issue close debit it inside the handler, +# after the read gate, rather than in router middleware. Issue close also draws +# on its own GITLAWB_CLOSE_ISSUE_RATE_LIMIT bucket, and a close refused by +# either cap spends nothing from the other. 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 d4a1fa08..77cbd12d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2208,7 +2208,7 @@ version = "3.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.2", ] [[package]] @@ -2701,7 +2701,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ab67060fc6b8ef687992d439ca0fa36e7ed17e9a0b16b25b601e8757df720de" dependencies = [ "data-encoding", - "syn 1.0.109", + "syn 2.0.117", ] [[package]] @@ -3540,6 +3540,7 @@ dependencies = [ "async-compression", "async-graphql", "async-graphql-axum", + "async-trait", "aws-config", "aws-sdk-s3", "axum", @@ -4983,11 +4984,10 @@ dependencies = [ [[package]] name = "multihash" -version = "0.19.3" +version = "0.19.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6b430e7953c29dd6a09afc29ff0bb69c6e306329ee6794700aee27b76a1aea8d" +checksum = "577c63b00ad74d57e8c9aa870b5fccebf2fd64a308a5aee9f1bb88e4aea19447" dependencies = [ - "core2", "unsigned-varint", ] @@ -7959,7 +7959,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/crates/gitlawb-node/Cargo.toml b/crates/gitlawb-node/Cargo.toml index c06780f6..8b775b14 100644 --- a/crates/gitlawb-node/Cargo.toml +++ b/crates/gitlawb-node/Cargo.toml @@ -30,6 +30,7 @@ required-features = ["test-harness"] [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..438ca042 --- /dev/null +++ b/crates/gitlawb-node/src/admin.rs @@ -0,0 +1,1922 @@ +//! 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, + /// Candidates skipped because receive-pack requests or pending ref + /// transitions still have in-flight work on the repo; deleting its + /// storage would strand that work mid-effect. + pub skipped_unsettled: usize, + /// Candidates skipped because the unsettled-work PROBE itself failed — + /// fail-closed like skipped_store_error, but bucketed separately so an + /// operator can tell "work in flight" apart from "could not even check". + pub skipped_unsettled_probe: usize, + /// Rows deleted whose destructive follow-through was refused because the + /// slug read as live afterwards — a fresh canonical or mirror row landed + /// in the post-commit window — or because the liveness probe itself + /// failed closed. Both disk and archive are left in place for these, so + /// the bucket never hides destruction skipped under a surviving row. + pub skipped_slug_live: 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` ONLY when the path is verifiably absent (a `NotFound` from +/// `symlink_metadata`). `None` is what lets selection distinguish a truly +/// 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. Everything else fails closed to `Some(1)` so it is skipped and +/// never admitted as remote-unverified: an unsafe name, an unreadable/unstat-able +/// path, a dangling symlink, or an existing directory that is not a bare git repo +/// (returning `None` for the latter would promote it, and the remote_unverified +/// refresh's `remove_dir_all` would overwrite that non-repository directory). +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 Ok(path) = crate::git::repo_store::validated_repo_disk_path(repos_dir, owner_did, name) + else { + warn!("purge-spam: could not validate repo path — treating as non-empty (skipped)"); + return Some(1); + }; + let path = path.into_path_buf(); + // Only a VERIFIED NotFound (the path truly does not exist) may return None, + // which promotes the repo to remote_unverified — and that refresh path does a + // remove_dir_all on the target. Use `symlink_metadata` (not `metadata`) so a + // dangling symlink counts as PRESENT, not NotFound. Any existing path, or any + // stat error other than NotFound, fails closed to Some(1) so the refresh never + // overwrites a non-repository directory sitting at the target. + match std::fs::symlink_metadata(&path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return None, + Err(e) => { + warn!(path = %path.display(), err = %e, + "purge-spam: could not stat path — treating as non-empty (skipped)"); + return Some(1); + } + // A symlink at the target is never a legitimate bare repo we may treat as + // empty-and-purgeable. The marker checks below FOLLOW the link, so a + // symlink to a real empty bare repo outside `repos_dir` would otherwise + // read 0 refs and be admitted — deleting the DB row while `path_within` + // spares the outside dir. Fail closed as present so it is never a candidate. + Ok(m) if m.file_type().is_symlink() => { + warn!(path = %path.display(), + "purge-spam: path is a symlink — treating as non-empty (skipped)"); + return Some(1); + } + Ok(_) => {} + } + if !path.join("HEAD").is_file() || !path.join("objects").is_dir() { + // The path exists but is not a bare repo — fail closed as present so it is + // never promoted to remote_unverified and overwritten by a refresh. + warn!(path = %path.display(), + "purge-spam: path exists but is not a bare git repo — treating as non-empty (skipped)"); + return Some(1); + } + 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 Ok(path) = crate::git::repo_store::validated_repo_disk_path(repos_dir, owner_did, name) + else { + warn!("purge-spam: could not validate repo path — treating as non-empty (skipped)"); + return 1; + }; + let path = path.into_path_buf(); + // Match selection's fail-closed rule: a symlink at the repo path can point + // at an empty bare repo outside repos_dir, and following it would report + // zero refs for a path that was never a candidate. Treat as non-empty. + if path.is_symlink() { + warn!(path = %path.display(), + "purge-spam: repo path is a symlink — treating as non-empty (skipped)"); + return 1; + } + 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 { + // Canonicalize once up front: the Config default `./repos` carries a + // CurDir component that validated_repo_disk_path rejects on every joined + // candidate, which would make local_refs_on_disk read every default-path + // repo as non-empty and silently select none. + let repos_dir: PathBuf = std::fs::canonicalize(repos_dir) + .with_context(|| format!("canonicalizing repos_dir {}", repos_dir.display()))?; + 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. + let bound = repo_store.transfer_bound(); + let refreshed = match tokio::time::timeout( + bound, + repo_store.refresh_from_archive(&c.owner_did, &c.name), + ) + .await + { + Ok(Ok(g)) => g, + Ok(Err(e)) => { + 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; + } + Err(_) => { + warn!(repo = %c.id, + "purge-spam: archive refresh exceeded the transfer bound — 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; + } + // Never delete storage under work still settling: receive-pack requests + // and pending ref transitions keyed by this repo would otherwise be + // stranded mid-effect. Fail closed on a check error. + let unsettled = match db.has_unsettled_repo_work(&c.id).await { + Ok(u) => u, + Err(e) => { + warn!(repo = %c.id, err = %e, + "purge-spam: could not check for unsettled work — skipped (fail-closed)"); + summary.skipped_unsettled_probe += 1; + guard.release().await; + continue; + } + }; + if unsettled { + warn!(repo = %c.id, + "purge-spam: repo has unsettled receive-pack or transition work — skipped"); + summary.skipped_unsettled += 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; + purge_deleted_repo_storage( + db, + repo_store, + &repos_dir, + c, + bound, + refreshed.as_deref(), + n, + &mut summary, + ) + .await; + } + 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), {} (unsettled work in flight), {} (unsettled probe failed), {} (slug still live after delete); {} on-disk removal(s) failed, {} archive removal(s) failed.", + summary.deleted, + summary.skipped_not_empty, + summary.skipped_locked, + summary.skipped_store_error, + summary.skipped_unsettled, + summary.skipped_unsettled_probe, + summary.skipped_slug_live, + summary.disk_failed, + summary.archive_failed + ); + Ok(summary) +} + +/// The destructive follow-through for one repo row `delete_repo_by_id` just +/// committed: the slug-liveness probe, the on-disk removal, and the +/// object-store archive delete. Factored out so the post-commit window — a +/// new row for the same slug landing between the delete commit and the +/// destructive ops — is drivable directly in tests. +#[allow(clippy::too_many_arguments)] +async fn purge_deleted_repo_storage( + db: &Db, + repo_store: &crate::git::repo_store::RepoStore, + repos_dir: &std::path::Path, + c: &Candidate, + bound: std::time::Duration, + refreshed: Option<&str>, + rows_deleted: u64, + summary: &mut PurgeSummary, +) { + // Post-commit window: delete_repo_by_id's slug lock ends at commit, so a + // concurrent mirror ingest (which takes only the transaction-scoped slug + // lock, not the session lock) can insert a NEW row for this owner/name + // with a mirror id before the destructive ops run. Probe the SLUG, not + // the deleted primary key: a live slug must refuse BOTH the disk delete + // and the archive delete — gating only the disk step would still strand + // the object-store bytes a live row needs. A probe error fails closed, + // the same as the lock-phase checks. + let slug_live = match db.get_repo(&c.owner_did, &c.name).await { + Ok(Some(_)) => { + warn!(repo = %c.id, + "purge-spam: a repo row for this slug reappeared after delete — refusing disk and archive removal under a live row"); + true + } + Ok(None) => false, + Err(e) => { + warn!(repo = %c.id, err = %e, + "purge-spam: could not re-check slug liveness — refusing disk and archive removal (fail-closed)"); + true + } + }; + if slug_live { + summary.skipped_slug_live += 1; + return; + } + + // 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). + // Disk removal is best-effort and NEVER gates the archive + // delete below: a failed or refused local removal must not + // strand the object-store copy of a row we just deleted. + let path = crate::git::repo_store::validated_repo_disk_path(repos_dir, &c.owner_did, &c.name) + .map(|p| p.into_path_buf()); + match &path { + Ok(p) if !path_within(p, repos_dir) => { + warn!(repo = %c.id, path = %p.display(), + "purge-spam: on-disk path escapes repos_dir — refusing to remove"); + summary.disk_failed += 1; + } + Ok(p) => { + if let Err(e) = std::fs::remove_dir_all(p) { + warn!(repo = %c.id, path = %p.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 = rows_deleted, "purge-spam: deleted repo row + on-disk dir"); + } + } + Err(e) => { + warn!(repo = %c.id, err = %e, + "purge-spam: could not validate repo path — refusing to remove"); + summary.disk_failed += 1; + } + } + + // 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. + // Fence on the generation refreshed above: a PUT landing after + // the emptiness check moves the archive generation, and a moved + // generation must read as skip, not delete-then-report. + match tokio::time::timeout( + bound, + repo_store.delete_archive(&c.owner_did, &c.name, refreshed), + ) + .await + { + Ok(Ok(true)) => {} + Ok(Ok(false)) => { + warn!(repo = %c.id, + "purge-spam: archive generation moved after the emptiness check — kept"); + summary.archive_failed += 1; + } + Ok(Err(e)) => { + warn!(repo = %c.id, err = %e, + "purge-spam: deleted row + dir but could not delete the object-store archive"); + summary.archive_failed += 1; + } + Err(_) => { + warn!(repo = %c.id, + "purge-spam: archive delete exceeded the transfer bound — counted as failed"); + summary.archive_failed += 1; + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use chrono::Utc; + + /// Post-commit window (P1): `delete_repo_by_id`'s slug lock ends at commit, + /// so a mirror ingest can land a NEW row for the same owner/name before the + /// destructive ops. The probe must read the slug, not the deleted primary + /// key — a mirror id (`owner/name`) is a different row id entirely — and a + /// live slug must refuse BOTH the disk delete and the archive delete. + #[sqlx::test] + async fn purge_post_delete_slug_liveness_refuses_disk_and_archive(pool: sqlx::PgPool) { + let db = crate::db::Db::for_testing(pool.clone()); + db.run_migrations().await.expect("migrations"); + let dir = tempfile::tempdir().unwrap(); + let repo_store = + crate::git::repo_store::RepoStore::for_testing(dir.path().to_path_buf(), pool.clone()); + + let owner = "did:key:z6MkPurgeSlugLiveOwner"; + let name = "victim"; + let mut rec = repo("rid-slug-live", owner, name); + rec.disk_path = dir + .path() + .join("z6MkPurgeSlugLiveOwner") + .join("victim.git") + .to_string_lossy() + .to_string(); + db.create_repo(&rec).await.unwrap(); + // The on-disk bare dir the purge would otherwise remove. + std::fs::create_dir_all(&rec.disk_path).unwrap(); + assert!(std::path::Path::new(&rec.disk_path).exists()); + + // The delete commits; the xact slug lock dies with it. + db.delete_repo_by_id(&rec.id).await.unwrap(); + // A concurrent ingest lands a fresh row for the SAME slug with a + // mirror id — `get_repo_by_id(&rec.id)` would read this window as + // empty, which is exactly the hole the probe closes. + let owner_short = crate::db::normalize_owner_key(owner); + db.upsert_mirror_repo(owner_short, name, &rec.disk_path, None, false) + .await + .unwrap(); + assert!( + db.get_repo(owner, name).await.unwrap().is_some(), + "the slug reads live through the normalized lookup" + ); + + let c = Candidate { + id: rec.id.clone(), + owner_did: owner.to_string(), + name: name.to_string(), + ref_count: 0, + remote_unverified: false, + }; + let mut summary = PurgeSummary::default(); + purge_deleted_repo_storage( + &db, + &repo_store, + dir.path(), + &c, + std::time::Duration::from_secs(30), + None, + 1, + &mut summary, + ) + .await; + + assert_eq!( + summary.skipped_slug_live, 1, + "a surviving slug row must refuse every destructive step" + ); + assert!( + std::path::Path::new(&rec.disk_path).exists(), + "the on-disk dir must survive under a live slug row" + ); + assert_eq!(summary.disk_failed, 0); + assert_eq!(summary.archive_failed, 0); + } + + 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()) + } + + /// A Postgres pool of a fixed size with a short acquire timeout and no ambient + /// reaping, so a purge exercised at GITLAWB_DB_MAX_CONNECTIONS=1 fails fast + /// (rather than stalling the whole test) if the single connection is pinned. + /// min/idle/lifetime pinned so a held connection is never reclaimed + /// mid-assertion. + async fn sized_no_reap_pool( + connect_opts: &sqlx::postgres::PgConnectOptions, + max_connections: u32, + ) -> PgPool { + sqlx::postgres::PgPoolOptions::new() + .max_connections(max_connections) + .acquire_timeout(std::time::Duration::from_secs(2)) + .min_connections(0) + .idle_timeout(None) + .max_lifetime(None) + .test_before_acquire(false) + .connect_with(connect_opts.clone()) + .await + .unwrap() + } + + 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"); + } + + // U3 (R3/KTD3): a purge at GITLAWB_DB_MAX_CONNECTIONS=1 (app pool size 1) must + // still delete. U2 split the pools: the advisory-lock guard pins a connection + // from the dedicated lock_pool while delete_repo_by_id runs on the app pool, so + // the single app connection is free for the delete's begin() even while the guard + // holds the repo lock. Load-bearing for the admin wiring: if the purge store were + // (mis)wired to share ONE pool for the guard and the delete, the held guard + // connection would leave begin() nothing to acquire, delete would time out + // (PoolTimedOut), and the row would survive — deleted stays 0. The split store + // built here is what keeps the purge correct at one connection. + #[sqlx::test] + async fn purge_at_one_app_connection_deletes_with_split_pools( + _pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: sqlx::postgres::PgConnectOptions, + ) { + // Migrate + seed on a normal multi-connection pool: migrate() pins one + // connection for its advisory lock while running migration queries on + // others, so it cannot run on a size-1 pool. + let setup_db = Db::for_testing(sized_no_reap_pool(&connect_opts, 5).await); + setup_db.run_migrations().await.unwrap(); + let empty = rec("t-empty", SPAM_BURST_TARGET_DID, "spam1"); + setup_db.create_repo(&empty).await.unwrap(); + + let tmp = tempfile::TempDir::new().unwrap(); + let path = store::repo_disk_path(tmp.path(), &empty.owner_did, &empty.name); + store::init_bare(&path).unwrap(); + assert_eq!( + store::list_refs(&path).unwrap().len(), + 0, + "precondition: the candidate is a real empty bare repo" + ); + + // The purge runs at ONE app connection (finding E's MAX_CONNECTIONS=1) with a + // SEPARATE lock pool. Split store: the guard draws from lock_pool, the delete + // runs on the app pool via `db` — the production wiring from main.rs's + // purge-spam path. + let app_pool = sized_no_reap_pool(&connect_opts, 1).await; + let lock_pool = sized_no_reap_pool(&connect_opts, 1).await; + let db = Db::for_testing(app_pool); + let store = crate::git::repo_store::RepoStore::new( + tmp.path().to_path_buf(), + None, + lock_pool, + std::time::Duration::from_secs(300), + ); + let summary = run_purge_spam(&db, &store, tmp.path(), true).await.unwrap(); + + assert_eq!( + summary.deleted, 1, + "a split-pool purge at one app connection must delete the candidate row" + ); + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_none(), + "the target row must be gone after a size-1 purge" + ); + } + + // 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); + + // (a0) A genuinely MISSING path is a verified NotFound — the only case that + // returns None (eligible for a remote-unverified archive refresh). + assert!(!path.exists()); + assert_eq!( + local_refs_on_disk(tmp.path(), &repo.owner_did, &repo.name), + None, + "a verifiably-absent path must return None (eligible for archive refresh)" + ); + + // (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. It must fail CLOSED to Some(1) — NOT + // None: returning None would promote it to remote_unverified, and the + // refresh's remove_dir_all would overwrite this existing directory. And it + // must never run list_refs to read the ancestor's 0. + std::fs::create_dir_all(&path).unwrap(); + assert_eq!( + local_refs_on_disk(tmp.path(), &repo.owner_did, &repo.name), + Some(1), + "an existing non-git dir must fail closed to Some(1), never None (which would overwrite it)" + ); + 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) + ); + } + + /// A symlink at a burst repo's expected on-disk path is never a legitimate + /// bare repo we may treat as empty-and-purgeable, even when its target is a + /// real EMPTY bare repo living OUTSIDE repos_dir. `symlink_metadata` only + /// distinguishes NotFound; the marker checks (`HEAD` file, `objects` dir) + /// FOLLOW the link, so the outside repo's 0 refs would be admitted — the DB + /// row is deleted (the `path_within` gate spares the outside dir, but only + /// AFTER delete_repo_by_id). A symlink at the target must fail CLOSED. + #[test] + fn symlink_to_outside_empty_bare_fails_closed() { + let tmp = tempfile::TempDir::new().unwrap(); + let repos_dir = tmp.path().join("repos"); + std::fs::create_dir_all(&repos_dir).unwrap(); + + // A real empty bare repo OUTSIDE repos_dir (HEAD + objects/ => looks bare). + let outside = tmp.path().join("outside.git"); + store::init_bare(&outside).unwrap(); + assert!(outside.join("HEAD").is_file() && outside.join("objects").is_dir()); + + let repo = rec("t-sym", SPAM_BURST_TARGET_DID, "spam1"); + let target = store::repo_disk_path(&repos_dir, &repo.owner_did, &repo.name); + std::fs::create_dir_all(target.parent().unwrap()).unwrap(); + std::os::unix::fs::symlink(&outside, &target).unwrap(); + + // Following the link the empty bare repo reads 0 refs (admitted as empty). + // A symlink at the target must fail closed to Some(1), never Some(0)/None. + assert_eq!( + local_refs_on_disk(&repos_dir, &repo.owner_did, &repo.name), + Some(1), + "a symlink at the target (even to a real empty bare repo outside repos_dir) must fail closed to Some(1)" + ); + } + + /// The DANGLING-symlink case (target does not exist) is why the stat uses + /// `symlink_metadata`, not `metadata`: `metadata` would surface NotFound and + /// wrongly return None (promoting the repo to remote-unverified, whose + /// refresh does a remove_dir_all). `symlink_metadata` succeeds, so the path + /// is present, and the symlink classification fails it closed to Some(1). + #[test] + fn dangling_symlink_fails_closed() { + let tmp = tempfile::TempDir::new().unwrap(); + let repos_dir = tmp.path().join("repos"); + std::fs::create_dir_all(&repos_dir).unwrap(); + + let repo = rec("t-dangle", SPAM_BURST_TARGET_DID, "spam1"); + let target = store::repo_disk_path(&repos_dir, &repo.owner_did, &repo.name); + std::fs::create_dir_all(target.parent().unwrap()).unwrap(); + std::os::unix::fs::symlink("/nonexistent/target", &target).unwrap(); + + assert_eq!( + local_refs_on_disk(&repos_dir, &repo.owner_did, &repo.name), + Some(1), + "a dangling symlink is present (not NotFound) and must fail closed to Some(1)" + ); + } + + /// 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) ─────────────────────────────────── + + use crate::git::repo_store::tests as rs_tests; + use crate::git::tigris::{TigrisClient, UploadPrecondition}; + + /// A RepoStore backed by the conditional-semantics S3 mock, so the purge + /// tool's authoritative recheck can be driven without a live bucket; the + /// mock's fault injection (failed HEAD, generation roll between HEAD and + /// DELETE) exercises the fail-closed and archive-delete-failure paths. + fn store_backed( + repos_dir: &Path, + pool: &PgPool, + endpoint: &str, + ) -> crate::git::repo_store::RepoStore { + crate::git::repo_store::RepoStore::for_testing_with_tigris( + repos_dir.to_path_buf(), + pool.clone(), + TigrisClient::for_testing_with_endpoint("test-bucket", endpoint), + ) + } + + /// Publish `owner_did/repo_name` into the mock so the purge's recheck sees + /// a real archive: an empty bare repo, or one carrying a single commit. + async fn seed_archive( + mock: &rs_tests::S3Mock, + owner_did: &str, + repo_name: &str, + with_ref: bool, + ) { + let dir = tempfile::TempDir::new().unwrap(); + let bare = dir.path().join("repo.git"); + store::init_bare(&bare).unwrap(); + if with_ref { + seed_one_ref(&bare); + } + rs_tests::mock_tigris(mock) + .upload( + &rs_tests::owner_slug_of(owner_did), + repo_name, + &bare, + UploadPrecondition::Unconditional, + ) + .await + .expect("seeding the archive"); + } + + // 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 mock = rs_tests::S3Mock::start().await; + seed_archive(&mock, &target.owner_did, &target.name, true).await; + let repo_store = store_backed(tmp.path(), &pool, mock.endpoint()); + 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 mock = rs_tests::S3Mock::start().await; + seed_archive(&mock, &target.owner_did, &target.name, false).await; + let repo_store = store_backed(tmp.path(), &pool, mock.endpoint()); + 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!( + mock.object_for(&rs_tests::owner_slug_of(&target.owner_did), &target.name) + .is_none(), + "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(); + + // A dead endpoint makes every store call fail, so the recheck + // errors and the purge must fail closed. + let repo_store = store_backed(tmp.path(), &pool, "http://127.0.0.1:1"); + 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 mock = rs_tests::S3Mock::start().await; + seed_archive(&mock, &target.owner_did, &target.name, false).await; + // Two HEADs precede the DELETE (the recheck's `exists`, then + // `delete_current`'s generation read); rolling the generation after + // the second makes the conditional DELETE refuse as a racing publish + // would. + mock.roll_generation_after_next_heads(2); + let repo_store = store_backed(tmp.path(), &pool, mock.endpoint()); + 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_eq!(mock.deletes(), 0, "the refused DELETE must not land"); + assert!( + mock.object_for(&rs_tests::owner_slug_of(&target.owner_did), &target.name) + .is_some(), + "a refused DELETE leaves the archive in place" + ); + 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 mock = rs_tests::S3Mock::start().await; + seed_archive(&mock, &target.owner_did, &target.name, false).await; + let repo_store = store_backed(tmp.path(), &pool, mock.endpoint()); + 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!( + mock.object_for(&rs_tests::owner_slug_of(&target.owner_did), &target.name) + .is_none(), + "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 mock = rs_tests::S3Mock::start().await; + seed_archive(&mock, &target.owner_did, &target.name, true).await; + let repo_store = store_backed(tmp.path(), &pool, mock.endpoint()); + 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); + } + + // U7/R7: an existing NON-repository directory (holding operator data) sits at + // the exact target path. It is not a bare git repo, so it must fail CLOSED — + // never be admitted remote-unverified, because that promotion drives + // refresh_from_archive, whose remove_dir_all + extract would OVERWRITE the + // directory. Load-bearing: on base, local_refs_on_disk returns None for a + // non-bare path, the candidate is promoted, refresh wipes the dir and the row + // is deleted (RED). After the fix, Some(1) keeps it out entirely (GREEN). + #[sqlx::test] + async fn purge_does_not_overwrite_non_repo_dir_at_target(pool: PgPool) { + let db = db(&pool).await; + let tmp = tempfile::TempDir::new().unwrap(); + let target = rec("t-nonrepo", SPAM_BURST_TARGET_DID, "spam1"); + db.create_repo(&target).await.unwrap(); + + // A plain directory with a file — deliberately NOT a bare git repo. + let path = store::repo_disk_path(tmp.path(), &target.owner_did, &target.name); + std::fs::create_dir_all(&path).unwrap(); + let sentinel = path.join("keep.txt"); + std::fs::write(&sentinel, b"operator data").unwrap(); + + let mock = rs_tests::S3Mock::start().await; + seed_archive(&mock, &target.owner_did, &target.name, false).await; + let repo_store = store_backed(tmp.path(), &pool, mock.endpoint()); + let summary = run_purge_spam(&db, &repo_store, tmp.path(), true) + .await + .unwrap(); + + assert!( + sentinel.exists(), + "an existing non-repo dir must not be overwritten by the purge refresh" + ); + assert_eq!( + std::fs::read(&sentinel).unwrap(), + b"operator data", + "the non-repo dir's contents must be intact" + ); + assert!( + db.get_repo(SPAM_BURST_TARGET_DID, "spam1") + .await + .unwrap() + .is_some(), + "a repo whose disk path is a non-bare dir must not be purged" + ); + assert_eq!(summary.deleted, 0); + assert_eq!( + summary.remote_unverified, 0, + "a non-bare existing dir must never be admitted remote-unverified" + ); + assert_eq!(mock.deletes(), 0, "the archive must not be deleted"); + } + + // 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 mock = rs_tests::S3Mock::start().await; // no archive seeded + let repo_store = store_backed(tmp.path(), &pool, mock.endpoint()); + 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/issues.rs b/crates/gitlawb-node/src/api/issues.rs index eefe168c..0039550d 100644 --- a/crates/gitlawb-node/src/api/issues.rs +++ b/crates/gitlawb-node/src/api/issues.rs @@ -164,6 +164,8 @@ pub async fn create_issue_comment( State(state): State, Extension(auth): Extension, Path((owner, repo, issue_id)): Path<(String, String, String)>, + headers: axum::http::HeaderMap, + crate::rate_limit::PeerAddr(peer): crate::rate_limit::PeerAddr, Json(req): Json, ) -> Result<(StatusCode, Json)> { if req.body.trim().is_empty() { @@ -176,6 +178,17 @@ pub async fn create_issue_comment( let (record, _rules) = crate::api::authorize_repo_read(&state, &owner, &repo, Some(auth.0.as_str()), "/").await?; + // Shared per-IP write brake, debited AFTER the read gate so a 429 cannot + // distinguish a hidden repo from a missing one (INV-12 — the same + // placement close_issue uses for its brake). The route group carries no + // outer brake for exactly this reason. + if let Some(key) = crate::rate_limit::client_key(&headers, peer, state.push_limiter_trust) { + if let Some(retry_after) = state.write_rate_limiter.check_retry(&key).await { + tracing::warn!(repo = %repo, key = %key, "create_issue_comment rate limited"); + return Err(AppError::RateLimited(retry_after)); + } + } + let disk_path = state .repo_store .acquire(&record.owner_did, &record.name) @@ -272,7 +285,22 @@ pub async fn close_issue( // close requests for arbitrary issue ids. Applied after the read gate so a // denied reader still sees 404, not 429. if let Some(key) = crate::rate_limit::client_key(&headers, peer, state.push_limiter_trust) { + // Both buckets debit here, after the read gate (INV-12): the shared + // per-IP write brake — the route group deliberately carries no outer + // brake — and this route's own close budget. + // + // A close refused by either cap spends nothing from the other bucket. + // The write bucket is debited first, and refunded when the close cap + // then refuses, so the guarantee holds even when a concurrent close + // takes the last close slot between the two checks. A write-bucket + // refusal advertises its window's Retry-After; the close-cap refusal + // keeps the plain 429 it has always returned. + if let Some(retry_after) = state.write_rate_limiter.check_retry(&key).await { + tracing::warn!(repo = %repo, key = %key, "close_issue rate limited"); + return Err(AppError::RateLimited(retry_after)); + } if !state.close_issue_rate_limiter.check(&key).await { + state.write_rate_limiter.refund(&key).await; tracing::warn!(repo = %repo, key = %key, "close_issue rate limited"); return Err(AppError::TooManyRequests( "rate limit exceeded — try again later".into(), @@ -434,11 +462,47 @@ pub async fn close_issue( } } + // Snapshot the issue's pre-close JSON so a failed durable upload can roll + // the close back on local disk, mirroring create_issue. Best-effort: if the + // snapshot read fails we proceed without a rollback closure (the close still + // applies and a failed upload still surfaces as a 5xx below). + // + // Resolve first so the compensating rewrite below lands on the SAME ref the + // close mutates. The path id may be an 8-char prefix, and rewriting + // refs/gitlawb/issues/{prefix} with the pre-close content would leave the + // canonical {full_id} ref closed while the client saw a 5xx. + let full_issue_id = git_issues::resolve_issue_id(&disk_path, &issue_id) + .ok() + .flatten(); + let prior_json = full_issue_id + .as_deref() + .and_then(|id| git_issues::get_issue(&disk_path, id).ok().flatten()); + let close_result = git_issues::close_issue(&disk_path, &issue_id); // Always release the advisory lock — even on error; upload to Tigris only on success. - // Same short-circuit as create_issue, and before the 200 body below. - guard.release(close_result.is_ok()).await.into_result()?; + // Same short-circuit as create_issue, and before the 200 body below. With a + // snapshot in hand, a failed upload rewrites the ref back to the pre-close + // content instead of leaving the local repo closed while the client saw 5xx. + let release_result = match (close_result.is_ok(), prior_json, full_issue_id) { + (true, Some(json), Some(resolved_id)) => { + let git_bin = state.git_bin.clone(); + let deadline = std::time::Instant::now() + + std::time::Duration::from_secs(state.config.git_service_timeout_secs); + guard + .release_compensating(true, move |path| { + // Same bounded-write strategy create_issue's compensator + // uses: the rollback rewrites the ref with the pre-close + // content through the deadline-aware runner, so a hung git + // cannot stall the guard past git_service_timeout. + git_issues::create_issue_bounded(&git_bin, path, &resolved_id, &json, deadline) + }) + .await + .into_result() + } + _ => guard.release(close_result.is_ok()).await.into_result(), + }; + release_result?; let updated = close_result .map_err(|e| AppError::Git(e.to_string()))? @@ -1233,6 +1297,198 @@ mod lock_pool_shed_tests { ); } + /// A close refused by its own cap must not spend the shared write budget: + /// the 429 completed no close, so stars/comments/GraphQL keep their slot. + #[sqlx::test] + async fn close_issue_refused_by_close_cap_does_not_debit_write_bucket(pool: PgPool) { + use std::net::SocketAddr; + use std::time::Duration; + + let owner = "did:key:zCLOSEDUALOWNERAAAAAAAAAAAAAAAAAAAAAAA"; + let mut state = crate::test_support::test_state(pool).await; + state.close_issue_rate_limiter = + crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60)); + state.write_rate_limiter = crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60)); + state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; + state + .db + .create_repo(&seed_repo(owner, "dual-close")) + .await + .expect("seed repo"); + + let peer: SocketAddr = "203.0.113.77:7000".parse().unwrap(); + let key = peer.ip().to_string(); + assert!( + state.close_issue_rate_limiter.check(&key).await, + "exhaust only the close bucket" + ); + + let res = close_issue( + State(state.clone()), + Extension(AuthenticatedDid(owner.to_string())), + Path((owner.to_string(), "dual-close".to_string(), "1".to_string())), + axum::http::HeaderMap::new(), + crate::rate_limit::PeerAddr(Some(peer)), + ) + .await; + assert!( + matches!(res, Err(AppError::TooManyRequests(_))), + "close cap exhausted must 429: {:?}", + res + ); + assert!( + state.write_rate_limiter.check(&key).await, + "a close refused by the close cap must leave the write bucket untouched" + ); + } + + /// A close refused by the shared write cap must not spend the close budget. + #[sqlx::test] + async fn close_issue_refused_by_write_cap_does_not_debit_close_bucket(pool: PgPool) { + use std::net::SocketAddr; + use std::time::Duration; + + let owner = "did:key:zCLOSEDUALOWNERBBBBBBBBBBBBBBBBBBBBBBB"; + let mut state = crate::test_support::test_state(pool).await; + state.close_issue_rate_limiter = + crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60)); + state.write_rate_limiter = crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60)); + state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; + state + .db + .create_repo(&seed_repo(owner, "dual-write")) + .await + .expect("seed repo"); + + let peer: SocketAddr = "203.0.113.78:7000".parse().unwrap(); + let key = peer.ip().to_string(); + assert!( + state.write_rate_limiter.check(&key).await, + "exhaust only the write bucket" + ); + + let res = close_issue( + State(state.clone()), + Extension(AuthenticatedDid(owner.to_string())), + Path((owner.to_string(), "dual-write".to_string(), "1".to_string())), + axum::http::HeaderMap::new(), + crate::rate_limit::PeerAddr(Some(peer)), + ) + .await; + let resp = res + .expect_err("write cap exhausted must refuse") + .into_response(); + assert_eq!( + resp.status(), + StatusCode::TOO_MANY_REQUESTS, + "write cap exhausted must 429" + ); + assert!( + state.close_issue_rate_limiter.check(&key).await, + "a close refused by the write cap must leave the close bucket untouched" + ); + } + + /// 429 from the shared write bucket must advertise a window-derived + /// Retry-After, as the outer-middleware 429s on the other write routes do. + fn assert_429_with_window_retry_after(err: AppError, what: &str) { + let resp = err.into_response(); + assert_eq!( + resp.status(), + StatusCode::TOO_MANY_REQUESTS, + "{what}: an exhausted write bucket must 429" + ); + let secs: u64 = resp + .headers() + .get("retry-after") + .unwrap_or_else(|| panic!("{what}: the write-bucket 429 must carry retry-after")) + .to_str() + .unwrap() + .parse() + .expect("retry-after must be integer seconds"); + assert!( + (1..=3600).contains(&secs), + "{what}: retry-after {secs}s must fall inside the 3600s window, not a constant" + ); + } + + #[sqlx::test] + async fn close_issue_write_cap_429_carries_retry_after(pool: PgPool) { + use std::net::SocketAddr; + use std::time::Duration; + + let owner = "did:key:zCLOSERETRYOWNERAAAAAAAAAAAAAAAAAAAAAA"; + let mut state = crate::test_support::test_state(pool).await; + state.write_rate_limiter = + crate::rate_limit::RateLimiter::new(1, Duration::from_secs(3600)); + state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; + state + .db + .create_repo(&seed_repo(owner, "retry-close")) + .await + .expect("seed repo"); + + let peer: SocketAddr = "203.0.113.61:7000".parse().unwrap(); + assert!(state.write_rate_limiter.check(&peer.ip().to_string()).await); + + let res = close_issue( + State(state), + Extension(AuthenticatedDid(owner.to_string())), + Path(( + owner.to_string(), + "retry-close".to_string(), + "1".to_string(), + )), + axum::http::HeaderMap::new(), + crate::rate_limit::PeerAddr(Some(peer)), + ) + .await; + assert_429_with_window_retry_after( + res.expect_err("write cap exhausted must refuse"), + "close_issue", + ); + } + + #[sqlx::test] + async fn create_issue_comment_write_cap_429_carries_retry_after(pool: PgPool) { + use std::net::SocketAddr; + use std::time::Duration; + + let owner = "did:key:zCOMMENTRETRYOWNERAAAAAAAAAAAAAAAAAAAA"; + let mut state = crate::test_support::test_state(pool).await; + state.write_rate_limiter = + crate::rate_limit::RateLimiter::new(1, Duration::from_secs(3600)); + state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; + state + .db + .create_repo(&seed_repo(owner, "retry-comment")) + .await + .expect("seed repo"); + + let peer: SocketAddr = "203.0.113.62:7000".parse().unwrap(); + assert!(state.write_rate_limiter.check(&peer.ip().to_string()).await); + + let res = create_issue_comment( + State(state), + Extension(AuthenticatedDid(owner.to_string())), + Path(( + owner.to_string(), + "retry-comment".to_string(), + "1".to_string(), + )), + axum::http::HeaderMap::new(), + crate::rate_limit::PeerAddr(Some(peer)), + Json(CreateIssueCommentRequest { + body: "x".to_string(), + }), + ) + .await; + assert_429_with_window_retry_after( + res.expect_err("write cap exhausted must refuse"), + "create_issue_comment", + ); + } + /// close_issue and receive-pack must not share one per-IP bucket. #[sqlx::test] async fn close_issue_rate_limit_does_not_drain_push_bucket(pool: PgPool) { @@ -1320,4 +1576,251 @@ mod lock_pool_shed_tests { server.abort(); } + + /// The compensating rewrite after a refused publish must land on the ref the + /// close actually mutated. A close addressed by an 8-char prefix resolves to + /// the full id and closes `refs/gitlawb/issues/{full_id}`; a compensator + /// that rewrites the path id verbatim mints `refs/gitlawb/issues/{prefix}` + /// with the pre-close content and leaves the canonical ref closed while the + /// client saw a 5xx. + /// + /// Reaching the compensator observably takes one contrivance: a definite + /// refusal invalidates (deletes) the live tree BEFORE the closure runs, so + /// the ref it writes exists only when that deletion cannot complete. The + /// fixture makes the tree read-only except the two directories the close + /// and its rollback write (`refs/gitlawb/issues/` and its reflog), and + /// pre-stores the closed blob so the mutation's `hash-object -w` needs no + /// write either. Without the contrivance the compensator runs against a + /// path that is already gone and there is nothing to assert on. + #[sqlx::test] + #[cfg(unix)] + async fn close_via_prefix_restores_the_full_ref_when_publish_refuses(pool: PgPool) { + use axum::response::IntoResponse; + use std::os::unix::fs::PermissionsExt; + + // Nothing is stored under the key (404 on HEAD: no under-lock download, + // a create-only publish fence) and the PUT is refused outright (a 4xx + // classifies as provably never dispatched), so release reaches the + // definite-refusal arm that invalidates the tree and runs the + // compensator. + let app = axum::Router::new().route( + "/{*key}", + axum::routing::any(|method: axum::http::Method| async move { + if method == axum::http::Method::HEAD { + axum::http::StatusCode::NOT_FOUND.into_response() + } else { + axum::http::StatusCode::FORBIDDEN.into_response() + } + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let endpoint = format!("http://127.0.0.1:{}", listener.local_addr().unwrap().port()); + let server = tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + + let owner = "did:key:zISSUECLOSEROLLBACKBBBBBBBBBBBBBBB"; + let repo_name = "rollback-close-prefix"; + let mut state = crate::test_support::test_state(pool.clone()).await; + let disk = tempfile::TempDir::new().unwrap(); + state.repo_store = crate::git::repo_store::RepoStore::for_testing_with_tigris( + disk.path().to_path_buf(), + crate::git::repo_store::build_lock_pool(&pool, 2, std::time::Duration::from_secs(1)), + crate::git::tigris::TigrisClient::for_testing_with_endpoint("test-bucket", &endpoint), + ); + state + .db + .create_repo(&seed_repo(owner, repo_name)) + .await + .expect("seed repo"); + + // Seed the live tree at the path the store resolves, with the issue + // under its full id so the route can be addressed by the 8-char prefix. + let live = state + .repo_store + .acquire(owner, repo_name) + .await + .expect("resolve live path"); + crate::git::store::init_bare(&live).expect("init bare repo"); + let full_id = "abc12345-0000-0000-0000-000000000000"; + let prefix = "abc12345"; + let open_json = serde_json::to_string(&IssueRecord { + id: full_id.to_string(), + title: "seeded".to_string(), + body: Some(String::new()), + author: Some(owner.to_string()), + created_at: chrono::Utc::now().to_rfc3339(), + status: "open".to_string(), + signed_payload: None, + }) + .expect("serialize seeded issue"); + git_issues::create_issue(&live, full_id, &open_json).expect("seed issue blob"); + + // Pre-store the blob `git_issues::close_issue` will write: the same + // parse/set-status/serialize sequence, so the bytes are identical and + // the mutation's `hash-object -w` is a no-op against read-only objects/. + let mut closed_value: serde_json::Value = + serde_json::from_str(&open_json).expect("parse seeded issue"); + closed_value["status"] = serde_json::Value::String("closed".into()); + let closed_json = serde_json::to_string(&closed_value).expect("closed issue json"); + { + use std::io::Write; + let mut child = std::process::Command::new("git") + .args(["hash-object", "--stdin", "-w"]) + .current_dir(&live) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::null()) + .spawn() + .expect("spawn hash-object"); + child + .stdin + .take() + .expect("stdin") + .write_all(closed_json.as_bytes()) + .expect("write blob"); + assert!( + child.wait().expect("hash-object").success(), + "pre-storing the closed blob must succeed" + ); + } + + // Stand the tree through the refusal's remove_dir_all: read-only + // everywhere, then write restored only where the close mutation and its + // rollback actually land (the ref and its reflog). + fn make_readonly(path: &std::path::Path) -> std::io::Result<()> { + let mut p = path.metadata()?.permissions(); + p.set_readonly(true); + std::fs::set_permissions(path, p)?; + if path.is_dir() { + for entry in std::fs::read_dir(path)? { + make_readonly(&entry?.path())?; + } + } + Ok(()) + } + make_readonly(&live).expect("make the live tree unremovable"); + for dir in [ + live.join("refs/gitlawb/issues"), + live.join("logs/refs/gitlawb/issues"), + ] { + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o755)) + .expect("restore write where the rollback lands"); + } + + let shed = close_issue( + State(state.clone()), + Extension(AuthenticatedDid(owner.to_string())), + Path((owner.to_string(), repo_name.to_string(), prefix.to_string())), + axum::http::HeaderMap::new(), + crate::rate_limit::PeerAddr(None), + ) + .await; + let err = shed.expect_err("a refused publish must not report a closed issue"); + assert!( + err.into_response().status().is_server_error(), + "a definite publish refusal must surface as a 5xx" + ); + + let restored = git_issues::get_issue(&live, full_id).expect("read the full ref"); + assert_eq!( + restored.as_deref(), + Some(open_json.as_str()), + "the compensator must restore the pre-close content on the FULL ref, \ + not on refs/gitlawb/issues/{prefix}" + ); + assert_eq!( + git_issues::resolve_issue_id(&live, prefix).expect("resolve the prefix"), + Some(full_id.to_string()), + "no stray refs/gitlawb/issues/{{prefix}} may be minted beside the \ + canonical ref: the prefix must still resolve to the full id" + ); + let issues = git_issues::list_issues(&live).expect("list issues"); + assert_eq!( + issues.len(), + 1, + "exactly one issue ref may survive the rollback: {issues:?}" + ); + + // Restore writability so the TempDir can clean itself up. + let _ = std::process::Command::new("chmod") + .args(["-R", "u+w"]) + .arg(&live) + .output(); + server.abort(); + } + /// INV-12 ordering: both issue write routes debit the shared write bucket + /// AFTER authorize_repo_read, so an exhausted bucket cannot answer 429 to a + /// caller the gate must 404 — that status difference would leak the repo's + /// existence. The route group carries no outer brake for this reason. + #[sqlx::test] + async fn an_exhausted_write_bucket_cannot_preempt_the_read_gate(pool: PgPool) { + let mut state = crate::test_support::test_state(pool.clone()).await; + state.write_rate_limiter = + crate::rate_limit::RateLimiter::new(1, std::time::Duration::from_secs(3600)); + state + .db + .create_repo(&crate::db::RepoRecord { + id: "rid-write-brake-order".to_string(), + name: "priv".to_string(), + owner_did: "did:key:z6MkWriteBrakeOwner".to_string(), + description: None, + is_public: false, + default_branch: "main".to_string(), + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + disk_path: "/tmp/write-brake-order".to_string(), + forked_from: None, + machine_id: None, + }) + .await + .unwrap(); + + let headers = axum::http::HeaderMap::new(); + let peer = || crate::rate_limit::PeerAddr(Some("203.0.113.77:5000".parse().unwrap())); + let key = crate::rate_limit::client_key(&headers, peer().0, state.push_limiter_trust) + .expect("a peer addr yields a brake key"); + assert!(state.write_rate_limiter.check(&key).await); + assert!( + !state.write_rate_limiter.check(&key).await, + "the shared write bucket is empty for this caller" + ); + + let stranger = || crate::auth::AuthenticatedDid("did:key:z6MkStranger".to_string()); + let path = || { + ( + "did:key:z6MkWriteBrakeOwner".to_string(), + "priv".to_string(), + "1".to_string(), + ) + }; + + let res = close_issue( + axum::extract::State(state.clone()), + axum::Extension(stranger()), + axum::extract::Path(path()), + headers.clone(), + peer(), + ) + .await; + assert!( + matches!(res, Err(AppError::RepoNotFound(_))), + "close on an unreadable repo must reach the read gate, not the empty write bucket: {res:?}" + ); + + let res = create_issue_comment( + axum::extract::State(state), + axum::Extension(stranger()), + axum::extract::Path(path()), + headers, + peer(), + axum::Json(CreateIssueCommentRequest { + body: "x".to_string(), + }), + ) + .await; + assert!( + matches!(res, Err(AppError::RepoNotFound(_))), + "comment on an unreadable repo must reach the read gate, not the empty write bucket: {res:?}" + ); + } } diff --git a/crates/gitlawb-node/src/api/pulls.rs b/crates/gitlawb-node/src/api/pulls.rs index ca3e7886..3cc525fa 100644 --- a/crates/gitlawb-node/src/api/pulls.rs +++ b/crates/gitlawb-node/src/api/pulls.rs @@ -225,6 +225,19 @@ pub async fn merge_pr( .map_err(|e| crate::api::repos::acquire_write_app_error(&e, &name))?; let disk_path = guard.path().to_path_buf(); let merger_did = auth.0; + + // Snapshot the target ref's pre-merge OID so a refused durable upload can + // rewind the local merge, mirroring the close_issue / create_issue + // compensating pattern. A read failure or an absent ref (a first-commit + // merge onto an unborn branch) yields no rollback closure: the merge still + // applies and a refused publish still surfaces as 5xx. + let target_ref = format!("refs/heads/{}", pr.target_branch); + let git_timeout = std::time::Duration::from_secs(state.config.git_service_timeout_secs); + let prior_tip = store::read_ref_bounded(&state.git_bin, &disk_path, &target_ref, git_timeout) + .await + .ok() + .flatten(); + let merge_result = store::merge_branch( &disk_path, &pr.target_branch, @@ -237,7 +250,27 @@ pub async fn merge_pr( // Short-circuit on a refused publish before the PR is marked merged and // before the webhook fires. Both are irreversible announcements of a merge // commit that only exists on this node's disk. - guard.release(merge_result.is_ok()).await.into_result()?; + let release_result = match (merge_result.is_ok(), prior_tip) { + (true, Some(tip)) => { + let git_bin = state.git_bin.clone(); + let deadline = std::time::Instant::now() + git_timeout; + guard + .release_compensating(true, move |path| { + crate::git::visibility_pack::run_bounded_git( + &git_bin, + &["update-ref", &target_ref, &tip], + path, + b"", + deadline, + ) + .map(|_| ()) + }) + .await + .into_result() + } + _ => guard.release(merge_result.is_ok()).await.into_result(), + }; + release_result?; let merge_sha = merge_result.map_err(|e| AppError::Git(e.to_string()))?; @@ -437,6 +470,136 @@ pub async fn list_comments( Ok(Json(serde_json::json!({ "comments": comments }))) } +#[cfg(test)] +mod merge_rollback_tests { + use super::*; + use axum::response::IntoResponse; + use sqlx::PgPool; + + fn git(dir: &std::path::Path, args: &[&str]) -> String { + let out = std::process::Command::new("git") + .args(["-C", &*dir.to_string_lossy()]) + .args([ + "-c", + "user.name=merge-rollback-test", + "-c", + "user.email=merge-rollback-test@example.com", + ]) + .args(args) + .output() + .unwrap(); + assert!( + out.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).trim().to_string() + } + + /// Build a bare repo whose `main` and `feature` refs diverge: one root + /// commit on main, one commit on feature, so merge_branch creates a real + /// merge commit that a refused publish must rewind. + fn seeded_merge_repo(dir: &std::path::Path) -> (String, String) { + crate::git::store::init_bare(dir).expect("bare repo"); + let empty_tree = git(dir, &["mktree"]); + let root = git(dir, &["commit-tree", &empty_tree, "-m", "root"]); + git(dir, &["update-ref", "refs/heads/main", &root]); + let tip2 = git( + dir, + &["commit-tree", &empty_tree, "-m", "feature", "-p", &root], + ); + git(dir, &["update-ref", "refs/heads/feature", &tip2]); + (root, tip2) + } + + /// On a definite publish refusal, the local merge commit must be rewound: + /// the client got a 5xx, so the repo cannot show a merge the mirror never + /// learned about (same contract create_issue's rollback enforces). + #[sqlx::test] + async fn merge_pr_rewinds_local_merge_when_publish_refuses(pool: PgPool) { + // Endpoint that answers HEAD with an etag but refuses every write. + let app = axum::Router::new().route( + "/{*key}", + axum::routing::any(|method: axum::http::Method| async move { + if method == axum::http::Method::HEAD { + let mut resp = axum::http::StatusCode::OK.into_response(); + resp.headers_mut() + .insert("etag", axum::http::HeaderValue::from_static("\"gen-1\"")); + resp + } else { + axum::http::StatusCode::INTERNAL_SERVER_ERROR.into_response() + } + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let endpoint = format!("http://127.0.0.1:{}", listener.local_addr().unwrap().port()); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + + let owner = "did:key:zMERGEROLLBACKAAAAAAAAAAAAAAAAAAAAAAAA"; + let repo_name = "rollback-merge"; + let mut state = crate::test_support::test_state(pool.clone()).await; + let disk = tempfile::TempDir::new().unwrap(); + let repo_path = disk.path().join("repo.git"); + let (main_tip, _feature_tip) = seeded_merge_repo(&repo_path); + + let mut seeded = crate::db::RepoRecord { + id: Uuid::new_v4().to_string(), + name: repo_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: String::new(), + forked_from: None, + machine_id: None, + }; + seeded.disk_path = repo_path.to_string_lossy().to_string(); + let repo_id = seeded.id.clone(); + state.db.create_repo(&seeded).await.expect("seed repo"); + let now = Utc::now().to_rfc3339(); + state + .db + .create_pr(&PullRequest { + id: Uuid::new_v4().to_string(), + repo_id, + number: 1, + title: "rb".to_string(), + body: None, + author_did: owner.to_string(), + source_branch: "feature".to_string(), + target_branch: "main".to_string(), + status: "open".to_string(), + merged_by_did: None, + merged_at: None, + created_at: now.clone(), + updated_at: now, + }) + .await + .expect("seed PR"); + state.repo_store = crate::git::repo_store::RepoStore::for_testing_with_tigris( + disk.path().to_path_buf(), + crate::git::repo_store::build_lock_pool(&pool, 2, std::time::Duration::from_secs(1)), + crate::git::tigris::TigrisClient::for_testing_with_endpoint("test-bucket", &endpoint), + ); + + let res = merge_pr( + State(state), + Extension(AuthenticatedDid(owner.to_string())), + Path((owner.to_string(), repo_name.to_string(), 1)), + ) + .await; + assert!(res.is_err(), "a refused publish must surface an error"); + + let tip_after = git(&repo_path, &["rev-parse", "refs/heads/main"]); + assert_eq!( + tip_after, main_tip, + "a refused publish must rewind the local merge to the pre-merge tip" + ); + } +} + /// #173 F1 follow-up: merge holds no admission permit, so an exhausted write-lock /// pool must shed 503 + Retry-After. #[cfg(test)] diff --git a/crates/gitlawb-node/src/api/repos.rs b/crates/gitlawb-node/src/api/repos.rs index 0e3d4930..644f5555 100644 --- a/crates/gitlawb-node/src/api/repos.rs +++ b/crates/gitlawb-node/src/api/repos.rs @@ -305,6 +305,24 @@ pub async fn create_repo( // Owner is the authenticated agent's DID let owner_did = auth.0; + // Serialize against a concurrent purge of the SAME owner/name. The purge + // holds this per-repo advisory lock across its delete-row + remove-dir, so + // without taking the same lock here a create could slip into the + // row-deleted-but-dir-not-yet-removed window and land a repos row pointing + // at a directory the purge then removes (a dangling row). Held across the + // existence-check -> init -> create_repo sequence; the guard's Drop frees + // the lock on every error path. Drawn from the dedicated lock pool, NOT the + // app pool, so a held guard cannot starve the db calls below (an app-pool + // lock at max_connections=1 would self-deadlock). A key held past the + // bounded wait means a live writer or purge owns it — a retryable 503, not + // a create failure. + let repo_lock = state + .repo_store + .lock_repo_blocking(&owner_did, &req.name) + .await + .map_err(|_| AppError::RepoBusy)? + .ok_or(AppError::RepoBusy)?; + // Check it doesn't already exist if state.db.get_repo(&owner_did, &req.name).await?.is_some() { return Err(AppError::RepoExists(req.name)); @@ -313,9 +331,9 @@ pub async fn create_repo( // Request is admissible — spend the proof now, immediately before the write. let verified_proof = proof.consume(&state.db).await?; - let disk_path = state + let (disk_path, create_upload) = state .repo_store - .init(&owner_did, &req.name) + .init_handle(&owner_did, &req.name) .await .map_err(|e| { // `{:#}` walks the anyhow chain to the leaf cause; the other git @@ -341,6 +359,51 @@ pub async fn create_repo( state.db.create_repo(&record).await?; + // Keep the initial archive upload inside the creation lock: a purge or + // retry that acquires the key after release must not find the row and dir + // while the create-only PUT is still in flight. Its OUTCOME also gates the + // 201: a failed or fenced upload leaves the row's storage inconsistent, so + // roll the create back (row + dir) and surface the error instead of + // reporting success. + if let Some(upload) = create_upload { + let landed = match upload.await { + Ok(inner) => inner, + Err(e) => Err(anyhow::anyhow!("empty-repo upload task failed: {e}")), + }; + if let Err(e) = landed { + tracing::warn!(repo = %req.name, err = %e, + "create: initial archive upload failed — rolling back row and dir"); + let row_deleted = match state.db.delete_repo_by_id(&record.id).await { + Ok(_) => true, + Err(dbe) => { + tracing::warn!(repo = %req.name, err = %dbe, + "create rollback: could not delete the repo row"); + false + } + }; + let dir_removed = std::fs::remove_dir_all(&disk_path).is_ok(); + repo_lock.release().await; + let residual = match (row_deleted, dir_removed) { + (true, true) => None, + (false, true) => Some("the repo row remains"), + (true, false) => Some("the on-disk directory remains"), + (false, false) => Some("the repo row and on-disk directory both remain"), + }; + return Err(match residual { + None => AppError::Git(format!( + "initial archive upload failed; the repo was rolled back: {e}" + )), + Some(what) => AppError::Git(format!( + "initial archive upload failed and rollback is incomplete: {what}; operator cleanup required: {e}" + )), + }); + } + } + + // Row + on-disk dir now both exist consistently; the race window is closed, + // so release the per-repo lock before the proof recording below. + repo_lock.release().await; + // Persist the proof so it can travel with the repo and a mirroring peer can // re-verify it (enforce-mode origins only; off/shadow yield no proof here). if let Some(p) = verified_proof { @@ -4753,50 +4816,89 @@ pub async fn fork_repo( ) .map_err(|e| AppError::BadRequest(e.to_string()))?; - // Check no name conflict under the forker's ownership let forker_short = crate::db::normalize_owner_key(&forker_did); - if state.db.get_repo(forker_short, &fork_name).await?.is_some() { - return Err(AppError::BadRequest(format!( - "you already have a repo named {fork_name}" - ))); - } - - // Request is admissible — spend the proof now, immediately before the write. - let verified_proof = proof.consume(&state.db).await?; // Ensure source repo is on local disk (downloads from Tigris on cache miss) + // BEFORE taking the target lock: a cache-miss acquire takes a nested + // lock-pool connection for the source's own namespace, and running it under + // the held target lock would pin two lock-pool connections at once. let source_path = state .repo_store .acquire(&source.owner_did, &source.name) .await .map_err(|e| AppError::Git(e.to_string()))?; - // Clone the source repo as a mirror - let output = std::process::Command::new("git") - .args(fork_clone_args( - source_path.to_str().unwrap_or(""), - disk_path.to_str().unwrap_or(""), - )) - .output() - .map_err(|e| AppError::Git(format!("git clone --mirror failed: {e}")))?; + // Serialize against a concurrent purge of the SAME forker/fork_name, the + // same advisory lock create_repo takes: held across the conflict check, + // clone, durable upload, and row insert so a fork cannot land inside the + // purge's delete-row -> remove-dir window. Contention is a retryable 503; + // the guard's Drop frees the lock on every error path below. + let repo_lock = state + .repo_store + .lock_repo_blocking(&forker_did, &fork_name) + .await + .map_err(|_| AppError::RepoBusy)? + .ok_or(AppError::RepoBusy)?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(AppError::Git(format!( - "git clone --mirror failed: {stderr}" + // Check no name conflict under the forker's ownership + if state.db.get_repo(forker_short, &fork_name).await?.is_some() { + return Err(AppError::BadRequest(format!( + "you already have a repo named {fork_name}" ))); } + // Request is admissible — spend the proof now, immediately before the write. + let verified_proof = proof.consume(&state.db).await?; + // ONE identity for the whole workflow: the DB row id this request will // insert is also the attempt id stamped into the object's metadata and onto // the disk clone. That is what lets confirmation ask "is my row there" rather // than "is a row there", and lets every cleanup ask "is this still mine" - // rather than "does this name resolve". + // rather than "does this name resolve". Stamped BEFORE the clone so the + // guard's Drop cleans a partial mirror even when the request future is + // cancelled mid-clone, and only ever removes the dir this attempt owns. let record_id = Uuid::new_v4().to_string(); let attempt = crate::git::publish::PublishAttemptId::from_owned(record_id.clone()); - let mut clone_guard = ForkCloneGuard::new(disk_path.clone(), attempt.clone()); + // Clone the source repo as a mirror, bounded so a wedged clone cannot pin + // the held target lock (and its lock-pool connection) indefinitely. Reuses + // the served-git ceiling; source_path is local so a normal clone never + // approaches it. `kill_on_drop` tears the child down when the timeout drops + // the future; the clone guard removes any partial dest on every failure, + // timeout, or cancellation path. + let clone_timeout = std::time::Duration::from_secs(state.config.git_service_timeout_secs); + let output = match tokio::time::timeout( + clone_timeout, + tokio::process::Command::new("git") + .args(fork_clone_args( + source_path.to_str().unwrap_or(""), + disk_path.to_str().unwrap_or(""), + )) + .kill_on_drop(true) + .output(), + ) + .await + { + Ok(Ok(output)) => output, + Ok(Err(e)) => { + return Err(AppError::Git(format!("git clone --mirror failed: {e}"))); + } + Err(_elapsed) => { + return Err(AppError::Git(format!( + "git clone --mirror timed out after {}s for {fork_name}", + clone_timeout.as_secs() + ))); + } + }; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(AppError::Git(format!( + "git clone --mirror failed: {stderr}" + ))); + } + // Upload fork to Tigris. Create-only: a refused precondition means an orphan // archive already sits under this key (a failed create_repo or another // writer), and proceeding would create a DB record whose archive is shadowed @@ -4931,6 +5033,7 @@ pub async fn fork_repo( match confirm_fork_repo_row(&state.db, &record_id, forker_short, &fork_name).await { Ok(ForkRowConfirmation::Ours(committed)) => { clone_guard.disarm(); + repo_lock.release().await; tracing::warn!( fork = %fork_name, forker = %forker_did, @@ -5009,6 +5112,8 @@ pub async fn fork_repo( // attempt's staging area, and nothing may ever compensate it away. crate::git::repo_store::release_fork_disk_claim(disk_path.as_path()); + repo_lock.release().await; + // Persist the proof so the fork carries it when it propagates to peers. if let Some(p) = verified_proof { if let Err(e) = p.record_for_repo(&state.db, &record.id).await { @@ -16082,4 +16187,1001 @@ exit 0 ); } } + + // ── Per-IP write-brake + purge-serialization coverage (#196) ────────── + + // 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" + ); + } + + // 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" + ); + } + } + + // ── U6/R6: create_repo serializes against a same-key purge ─────────────── + + // create_repo must take the SAME per-owner/name advisory lock the purge holds + // (try_lock_repo / RepoLockGuard), so a create cannot slip into the purge's + // delete-row -> [window] -> remove-dir gap and land a repos row pointing at a + // directory the purge then removes (a dangling row). Deterministic form of the + // race: hold the purge lock, then prove create BLOCKS on it rather than + // proceeding into the window — it must not insert its row while the lock is + // held, and must complete cleanly (row + on-disk dir both present) once the + // lock frees. RED on base (create takes no lock): the row lands while the lock + // is held. GREEN after (create serializes on the same key). + #[sqlx::test] + async fn create_repo_serializes_against_purge_lock( + pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: sqlx::postgres::PgConnectOptions, + ) { + use axum::extract::State; + use axum::Extension; + use std::time::Duration; + + // Multi-connection pool: while the purge guard pins one connection for the + // held lock, create's own try_lock_repo attempt must get a DIFFERENT + // connection and observe the advisory lock held (Ok(None) -> it retries), + // not merely block on pool exhaustion. + let pool = pool_opts + .max_connections(5) + .connect_with(connect_opts) + .await + .unwrap(); + 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 = "did:key:z6MkCreatePurgeRaceAAAAAAAAAAAAAAAAAAAAAAAA"; + let name = "racerepo"; + + // The purge path holds this exact per-repo advisory lock across its + // delete-row + remove-dir. Take it via the same helper the purge uses. + let purge_guard = state + .repo_store + .try_lock_repo(owner, name) + .await + .unwrap() + .expect("lock is free before the create"); + + // Kick off a create for the SAME owner/name while the purge holds the lock. + let state2 = state.clone(); + let owner2 = owner.to_string(); + let handle = tokio::spawn(async move { + super::create_repo( + State(state2), + Extension(crate::auth::AuthenticatedDid(owner2)), + axum::http::HeaderMap::new(), + Json(CreateRepoRequest { + name: name.to_string(), + description: None, + is_public: true, + default_branch: "main".to_string(), + }), + ) + .await + }); + + // While the purge lock is held, create must NOT complete its init+insert. + tokio::time::sleep(Duration::from_millis(600)).await; + assert!( + state.db.get_repo(owner, name).await.unwrap().is_none(), + "create_repo must not insert a repos row while a purge holds the same-key \ + lock (RED on base: create takes no lock and the row lands in the window)" + ); + assert!( + !handle.is_finished(), + "create must be blocked on the purge lock, not have completed" + ); + + // Purge releases; create can now proceed and create cleanly. + purge_guard.release().await; + + let created = tokio::time::timeout(Duration::from_secs(8), handle) + .await + .expect("create should finish once the lock frees") + .expect("create task join") + .expect("create_repo returns Ok once it wins the lock"); + assert_eq!(created.0, StatusCode::CREATED); + + // End state is consistent: the row AND its on-disk dir are both present — + // never a row pointing at a removed directory. + assert!( + state.db.get_repo(owner, name).await.unwrap().is_some(), + "repos row must be present after the create wins the lock" + ); + let dir = repos_dir + .path() + .join(owner.replace([':', '/'], "_")) + .join(format!("{name}.git")); + assert!( + dir.exists(), + "the created repo's on-disk dir must be present — no dangling row" + ); + } + + // U6 no-regression: with no concurrent purge, create_repo still succeeds and + // leaves a consistent row + on-disk dir. Guards against the lock acquisition + // wedging the uncontended hot path. + #[sqlx::test] + async fn create_repo_succeeds_without_lock_contention( + pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: sqlx::postgres::PgConnectOptions, + ) { + use axum::extract::State; + use axum::Extension; + + let pool = pool_opts + .max_connections(5) + .connect_with(connect_opts) + .await + .unwrap(); + 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 = "did:key:z6MkCreateNoContendBBBBBBBBBBBBBBBBBBBBBBBB"; + let name = "solo"; + + let created = super::create_repo( + State(state.clone()), + Extension(crate::auth::AuthenticatedDid(owner.to_string())), + axum::http::HeaderMap::new(), + Json(CreateRepoRequest { + name: name.to_string(), + description: None, + is_public: true, + default_branch: "main".to_string(), + }), + ) + .await + .expect("uncontended create_repo must succeed"); + assert_eq!(created.0, StatusCode::CREATED); + + assert!( + state.db.get_repo(owner, name).await.unwrap().is_some(), + "row present after an uncontended create" + ); + let dir = repos_dir + .path() + .join(owner.replace([':', '/'], "_")) + .join(format!("{name}.git")); + assert!( + dir.exists(), + "on-disk dir present after an uncontended create" + ); + } + + // ── U3/R2/R4: fork tail, guarded span, publish only after durability ───── + + const FORK_SRC_OWNER: &str = "z6forksrcowner"; + const FORK_SRC_NAME: &str = "forksrc"; + + /// Build a state whose repo_store AND config.repos_dir point at `repos_dir` + /// (fork_repo computes its clone target from config.repos_dir, so the two + /// must agree), seeded with a bare public source repo any caller may fork. + async fn fork_test_state( + repos_dir: &std::path::Path, + repo_store: crate::git::repo_store::RepoStore, + pool: sqlx::PgPool, + ) -> AppState { + let mut state = crate::test_support::test_state(pool).await; + state.repo_store = repo_store; + let mut cfg = (*state.config).clone(); + cfg.repos_dir = repos_dir.to_path_buf(); + state.config = std::sync::Arc::new(cfg); + + let bare = repos_dir + .join(FORK_SRC_OWNER) // slug == owner: no ':' or '/' to replace + .join(format!("{FORK_SRC_NAME}.git")); + std::fs::create_dir_all(bare.parent().unwrap()).unwrap(); + let out = std::process::Command::new("git") + .args(["init", "--bare", "-q", &bare.to_string_lossy()]) + .output() + .unwrap(); + assert!(out.status.success(), "git init --bare failed"); + state + .db + .upsert_mirror_repo( + FORK_SRC_OWNER, + FORK_SRC_NAME, + &bare.to_string_lossy(), + None, + false, + ) + .await + .unwrap(); + state + } + + async fn call_fork( + state: &AppState, + forker: &str, + fork_name: &str, + ) -> Result { + use axum::extract::{Path as AxPath, State}; + use axum::response::IntoResponse; + use axum::Extension; + super::fork_repo( + State(state.clone()), + Extension(crate::auth::AuthenticatedDid(forker.to_string())), + AxPath((FORK_SRC_OWNER.to_string(), FORK_SRC_NAME.to_string())), + axum::http::HeaderMap::new(), + Json(ForkRepoRequest { + name: Some(fork_name.to_string()), + }), + ) + .await + .map(|r| r.into_response()) + } + + // R2: fork must take the SAME per-owner/name advisory lock on its TARGET + // namespace that purge and create hold, so it cannot interleave with a + // purge's delete-row + remove-dir (or another creator) on that key. + // Deterministic form: hold the target lock, prove the fork BLOCKS (no clone + // dir, no repos row) rather than proceeding, then completes cleanly once the + // lock frees. RED on base: fork takes no lock and completes in the window. + #[sqlx::test] + async fn fork_serializes_against_target_namespace_lock( + pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: sqlx::postgres::PgConnectOptions, + ) { + use std::time::Duration; + + // Multi-connection pool: the held guard pins one connection, so the + // fork's own lock attempt must get a DIFFERENT one and observe the key + // held (Ok(None) -> retry), not merely block on pool exhaustion. + let pool = pool_opts + .max_connections(5) + .connect_with(connect_opts) + .await + .unwrap(); + let repos_dir = tempfile::TempDir::new().unwrap(); + let repo_store = crate::git::repo_store::RepoStore::for_testing( + repos_dir.path().to_path_buf(), + pool.clone(), + ); + let state = fork_test_state(repos_dir.path(), repo_store, pool).await; + + let forker = "did:key:z6MkForkSerializeAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let fork_name = "serialfork"; + + // A purge (or another creator) holds the fork target's advisory lock. + let held = state + .repo_store + .try_lock_repo(forker, fork_name) + .await + .unwrap() + .expect("target lock free before the fork"); + + let state2 = state.clone(); + let forker2 = forker.to_string(); + let fork_name2 = fork_name.to_string(); + let handle = tokio::spawn(async move { call_fork(&state2, &forker2, &fork_name2).await }); + + // While the target lock is held the fork must make no progress past it. + tokio::time::sleep(Duration::from_millis(500)).await; + let clone_dir = store::repo_disk_path(&state.config.repos_dir, forker, fork_name); + assert!( + state + .db + .get_repo(forker, fork_name) + .await + .unwrap() + .is_none(), + "fork must not insert a repos row while the target-namespace lock is \ + held (RED on base: fork takes no lock and the row lands in the window)" + ); + assert!( + !clone_dir.exists(), + "fork must not clone the mirror while the target-namespace lock is held" + ); + assert!( + !handle.is_finished(), + "fork must be blocked on the target lock, not have completed" + ); + + held.release().await; + let resp = tokio::time::timeout(Duration::from_secs(8), handle) + .await + .expect("fork should finish once the lock frees") + .expect("fork task join") + .expect("fork_repo returns Ok once it wins the lock"); + assert_eq!(resp.status(), StatusCode::CREATED); + assert!( + state + .db + .get_repo(forker, fork_name) + .await + .unwrap() + .is_some(), + "repos row present after the fork wins the lock" + ); + assert!(clone_dir.exists(), "cloned mirror present after the fork"); + } + + // KTD-2 trap case (must-not-break negative): a store-less node has nothing + // to upload, so fork succeeds exactly as today: "no object store" is + // nothing-to-do, never a failed upload. GREEN both before and after the fix. + #[sqlx::test] + async fn fork_without_object_store_succeeds(pool: sqlx::PgPool) { + let repos_dir = tempfile::TempDir::new().unwrap(); + let repo_store = crate::git::repo_store::RepoStore::for_testing( + repos_dir.path().to_path_buf(), + pool.clone(), + ); + let state = fork_test_state(repos_dir.path(), repo_store, pool).await; + + let forker = "did:key:z6MkForkNoStoreAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let fork_name = "nostorefork"; + + let resp = call_fork(&state, forker, fork_name) + .await + .expect("a store-less fork must succeed"); + assert_eq!(resp.status(), StatusCode::CREATED); + assert!( + state + .db + .get_repo(forker, fork_name) + .await + .unwrap() + .is_some(), + "repos row present after a store-less fork" + ); + let clone_dir = store::repo_disk_path(&state.config.repos_dir, forker, fork_name); + assert!(clone_dir.exists(), "cloned mirror present after the fork"); + } + + // R2: a target lock held past the fork's bounded wait must surface as the + // same retryable 503 create_repo returns, with no repos row and no clone + // residue. RED on base: fork ignores the lock and completes with a 201. + #[sqlx::test] + async fn fork_lock_contention_returns_503( + pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: sqlx::postgres::PgConnectOptions, + ) { + use axum::response::IntoResponse; + + let pool = pool_opts + .max_connections(5) + .connect_with(connect_opts) + .await + .unwrap(); + let repos_dir = tempfile::TempDir::new().unwrap(); + let repo_store = crate::git::repo_store::RepoStore::for_testing( + repos_dir.path().to_path_buf(), + pool.clone(), + ); + let state = fork_test_state(repos_dir.path(), repo_store, pool).await; + + let forker = "did:key:z6MkForkContendedAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let fork_name = "contendedfork"; + + // Held for the whole test: the fork's bounded wait must give up. + let held = state + .repo_store + .try_lock_repo(forker, fork_name) + .await + .unwrap() + .expect("target lock free before the fork"); + + let resp = call_fork(&state, forker, fork_name).await; + let status = match resp { + Ok(r) => r.status(), + Err(e) => e.into_response().status(), + }; + assert_eq!( + status, + StatusCode::SERVICE_UNAVAILABLE, + "a target lock held past the bounded wait must 503 like create_repo \ + (RED on base: fork ignores the lock and returns 201)" + ); + assert!( + state + .db + .get_repo(forker, fork_name) + .await + .unwrap() + .is_none(), + "no repos row after a 503'd fork" + ); + let clone_dir = store::repo_disk_path(&state.config.repos_dir, forker, fork_name); + assert!(!clone_dir.exists(), "no clone residue after a 503'd fork"); + held.release().await; + } + + // Finding 1: a `git clone --mirror` that exits non-zero after the dest dir + // exists must clear that dest and free the lock, so a retry of the same fork + // name is not permanently blocked by a leftover half-mirror. Driven by + // pre-seeding the dest so the clone fails "destination already exists" + // (exit 128, the `!output.status.success()` arm, not the timeout/spawn + // arms). RED on base: the arm returns without removing the dest, so the dest + // survives and the second fork's clone fails the same way (5xx forever). + #[sqlx::test] + async fn fork_nonzero_clone_exit_cleans_dest_and_allows_retry(pool: sqlx::PgPool) { + let repos_dir = tempfile::TempDir::new().unwrap(); + let repo_store = crate::git::repo_store::RepoStore::for_testing( + repos_dir.path().to_path_buf(), + pool.clone(), + ); + let state = fork_test_state(repos_dir.path(), repo_store, pool).await; + + let forker = "did:key:z6MkForkCloneNonzeroAAAAAAAAAAAAAAAAAAAAAA"; + let fork_name = "nonzerofork"; + + // Seed the dest as a non-empty dir so `git clone --mirror` refuses it and + // exits 128, standing in for a half-created mirror a prior failed clone + // (authz, corrupt source) would leave behind. + let clone_dir = store::repo_disk_path(&state.config.repos_dir, forker, fork_name); + std::fs::create_dir_all(&clone_dir).unwrap(); + std::fs::write(clone_dir.join("stale.txt"), b"half-created mirror").unwrap(); + + let resp = call_fork(&state, forker, fork_name).await; + let status = match resp { + Ok(r) => r.status(), + Err(e) => { + use axum::response::IntoResponse; + e.into_response().status() + } + }; + assert!( + status.is_server_error(), + "a non-zero clone exit must fail the fork (5xx); got {status}" + ); + assert!( + !clone_dir.exists(), + "the leftover dest must be removed after a non-zero clone exit \ + (RED on base: the !success arm leaves it, blocking every retry)" + ); + + // The dest is clean and the lock freed, so re-forking the same name now + // clones fresh and succeeds. + let resp2 = call_fork(&state, forker, fork_name) + .await + .expect("a retry after the dest is cleaned must succeed"); + assert_eq!(resp2.status(), StatusCode::CREATED); + assert!( + state + .db + .get_repo(forker, fork_name) + .await + .unwrap() + .is_some(), + "repos row present after the successful retry" + ); + assert!(clone_dir.exists(), "cloned mirror present after the retry"); + } + + // create_repo must NOT take its serialization lock from the APP pool. Doing so + // pins an app-pool connection in the lock guard while the create's own work + // (get_repo / proof.consume / db.create_repo, all on state.db = the app pool) + // still needs an app connection. At GITLAWB_DB_MAX_CONNECTIONS=1 (a supported + // config) the guard holds the only app connection and get_repo self-deadlocks + // (PoolTimedOut -> 500). Locking on the separate write lock_pool instead keeps + // the lock and the work on different pools, so a size-1 app pool never wedges. + // RED on the app-pool-lock code: create times out -> Err. GREEN after reverting + // create to lock on lock_pool. + #[sqlx::test] + async fn create_repo_does_not_self_deadlock_at_one_app_connection( + pool_opts: sqlx::postgres::PgPoolOptions, + connect_opts: sqlx::postgres::PgConnectOptions, + ) { + use axum::extract::State; + use axum::Extension; + use std::time::Duration; + + // Migrate the per-test DB on a normal pool: `migrate()` pins one connection for + // its cross-process advisory lock while applying statements on a second, so it + // needs >1 connection and cannot itself run on the size-1 pool under test. + let migrate_pool = pool_opts.connect_with(connect_opts.clone()).await.unwrap(); + let mut state = crate::test_support::test_state(migrate_pool).await; + + // Now point state.db at an APP pool of MAX size 1 on the SAME database, the + // minimal supported config (GITLAWB_DB_MAX_CONNECTIONS=1). The schema is + // already applied, so this pool only serves create's work (get_repo -> insert). + // A short acquire timeout makes the RED case (the lock guard pinning this one + // connection while get_repo waits for another) fail fast instead of hanging. + let app_pool = sqlx::postgres::PgPoolOptions::new() + .max_connections(1) + .acquire_timeout(Duration::from_secs(2)) + .connect_with(connect_opts.clone()) + .await + .unwrap(); + state.db = std::sync::Arc::new(crate::db::Db::for_testing(app_pool.clone())); + + // A SEPARATE write lock pool (where create's serialization lock belongs). Its + // being separate is the whole point: the lock guard must not draw from the app + // pool the create work runs on. + let lock_pool = sqlx::postgres::PgPoolOptions::new() + .max_connections(5) + .connect_with(connect_opts.clone()) + .await + .unwrap(); + let repos_dir = tempfile::TempDir::new().unwrap(); + state.repo_store = crate::git::repo_store::RepoStore::for_testing( + repos_dir.path().to_path_buf(), + lock_pool.clone(), + ); + + let owner = "did:key:z6MkOneAppConnNoDeadlockDDDDDDDDDDDDDDDDDDDD"; + let name = "solo"; + let created = super::create_repo( + State(state.clone()), + Extension(crate::auth::AuthenticatedDid(owner.to_string())), + axum::http::HeaderMap::new(), + Json(CreateRepoRequest { + name: name.to_string(), + description: None, + is_public: true, + default_branch: "main".to_string(), + }), + ) + .await + .expect("create must not self-deadlock at app-pool size 1 (lock on lock_pool)"); + assert_eq!(created.0, StatusCode::CREATED); + assert!( + state.db.get_repo(owner, name).await.unwrap().is_some(), + "row present after a create with a size-1 app pool" + ); + } + + // ── fork durable-write failure semantics on the fenced machinery ────── + + /// Seed `FORK_SRC_OWNER/FORK_SRC_NAME` into the mock so a warm local source + /// does not trigger the lazy-migration PUT (which would consume the fault a + /// test arms for the fork's own upload). + async fn seed_source_archive(mock: &crate::git::repo_store::tests::S3Mock) { + let dir = tempfile::TempDir::new().unwrap(); + let bare = dir.path().join("src.git"); + crate::git::store::init_bare(&bare).unwrap(); + crate::git::repo_store::tests::mock_tigris(mock) + .upload( + FORK_SRC_OWNER, + FORK_SRC_NAME, + &bare, + crate::git::tigris::UploadPrecondition::Unconditional, + ) + .await + .expect("seeding the source archive"); + } + + // When the fork's durable upload loses its response WITHOUT committing, the + // outcome is unknowable: the fork must fail retryably (5xx), insert no + // repos row, and KEEP the local clone — the only copy of a write that may + // yet have landed — rather than destroying it. + #[sqlx::test] + async fn fork_ambiguous_upload_failure_keeps_clone_and_inserts_no_row(pool: sqlx::PgPool) { + use crate::git::repo_store::tests as rs_tests; + + let mock = rs_tests::S3Mock::start().await; + let repos_dir = tempfile::TempDir::new().unwrap(); + let repo_store = crate::git::repo_store::RepoStore::for_testing_with_tigris( + repos_dir.path().to_path_buf(), + pool.clone(), + rs_tests::mock_tigris(&mock), + ); + let state = fork_test_state(repos_dir.path(), repo_store, pool).await; + seed_source_archive(&mock).await; + mock.fail_next_put_after_delivery(); + + let forker = "did:key:z6MkForkAmbigUploadAAAAAAAAAAAAAAAAAAAAAAA"; + let fork_name = "ambigfork"; + let resp = call_fork(&state, forker, fork_name).await; + let status = match resp { + Ok(r) => r.status(), + Err(e) => axum::response::IntoResponse::into_response(e).status(), + }; + assert!( + status.is_server_error(), + "an unresolvable durable upload must fail the fork (5xx); got {status}" + ); + assert!( + state + .db + .get_repo(forker, fork_name) + .await + .unwrap() + .is_none(), + "no repos row may exist for a fork whose durable upload did not land" + ); + let clone_dir = store::repo_disk_path(&state.config.repos_dir, forker, fork_name); + assert!( + clone_dir.exists(), + "an ambiguous upload keeps the clone for reconciliation — it is the only copy of a write that may have landed, and must not be destroyed" + ); + mock.shutdown(); + } + + // A repos-row insert that fails after the archive published must fail the + // fork (5xx), leave no row, and compensate the mirror and the archive — + // both carry this attempt's stamp, so cleanup removes ONLY this attempt's + // work. The decoy row holds the fork's exact disk_path under a different + // name, so the fork's own insert violates disk_path's UNIQUE constraint. + #[sqlx::test] + async fn fork_row_insert_failure_compensates_mirror_and_archive(pool: sqlx::PgPool) { + use crate::git::repo_store::tests as rs_tests; + + let mock = rs_tests::S3Mock::start().await; + let repos_dir = tempfile::TempDir::new().unwrap(); + let repo_store = crate::git::repo_store::RepoStore::for_testing_with_tigris( + repos_dir.path().to_path_buf(), + pool.clone(), + rs_tests::mock_tigris(&mock), + ); + let state = fork_test_state(repos_dir.path(), repo_store, pool).await; + seed_source_archive(&mock).await; + + let forker = "did:key:z6MkForkInsertFailAAAAAAAAAAAAAAAAAAAAAAAAA"; + let fork_name = "insertfailfork"; + let clone_dir = store::repo_disk_path(&state.config.repos_dir, forker, fork_name); + + // Decoy row occupying the fork's exact disk_path under a different + // name, so the fork's insert violates the UNIQUE constraint while + // get_repo(forker, fork_name) still returns None and the handler + // proceeds past its conflict guard. + let now = Utc::now(); + let decoy = crate::db::RepoRecord { + id: Uuid::new_v4().to_string(), + name: "decoyname".to_string(), + owner_did: "did:key:z6MkDecoyOwnerBBBBBBBBBBBBBBBBBBBBBBBBBBBB".to_string(), + description: None, + is_public: true, + default_branch: "main".to_string(), + created_at: now, + updated_at: now, + disk_path: clone_dir.to_string_lossy().to_string(), + forked_from: None, + machine_id: None, + }; + state.db.create_repo(&decoy).await.unwrap(); + + let resp = call_fork(&state, forker, fork_name).await; + let status = match resp { + Ok(r) => r.status(), + Err(e) => axum::response::IntoResponse::into_response(e).status(), + }; + assert!( + status.is_server_error(), + "a row-insert failure must fail the fork (5xx); got {status}" + ); + assert!( + state + .db + .get_repo(forker, fork_name) + .await + .unwrap() + .is_none(), + "no fork row may exist after the insert failure" + ); + assert!( + !clone_dir.exists(), + "the cloned mirror must be removed after the failed insert — the guard only ever compensates work stamped with this attempt's id" + ); + assert!( + mock + .object_for(&rs_tests::owner_slug_of(forker), fork_name) + .is_none(), + "the uploaded fork archive must be deleted after the failed insert — the attempt-fenced delete removes it because it is this attempt's own work" + ); + mock.shutdown(); + } } diff --git a/crates/gitlawb-node/src/auth/mod.rs b/crates/gitlawb-node/src/auth/mod.rs index b1f039ae..4ea3a132 100644 --- a/crates/gitlawb-node/src/auth/mod.rs +++ b/crates/gitlawb-node/src/auth/mod.rs @@ -738,6 +738,12 @@ mod tests { task_tx.clone(), task_cursor_key.clone(), )); + let ws_schema = Arc::new(graphql::build_ws_schema( + db.clone(), + ref_tx.clone(), + task_tx.clone(), + task_cursor_key.clone(), + )); crate::state::AppState { config: Arc::new(Config::parse_from(["gitlawb-node"])), db, @@ -748,11 +754,13 @@ mod tests { ref_update_tx: ref_tx, task_event_tx: task_tx, graphql_schema: schema, + graphql_ws_schema: ws_schema, task_cursor_key, machine_id: None, 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)), close_issue_rate_limiter: RateLimiter::new(120, Duration::from_secs(3600)), ipfs_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)), diff --git a/crates/gitlawb-node/src/config.rs b/crates/gitlawb-node/src/config.rs index 01cebf2b..c6b4c08d 100644 --- a/crates/gitlawb-node/src/config.rs +++ b/crates/gitlawb-node/src/config.rs @@ -1,4 +1,4 @@ -use clap::Parser; +use clap::{Parser, Subcommand}; use std::path::{Path, PathBuf}; /// Upper bound on `git_service_timeout_secs`, `ipfs_request_budget_secs`, and @@ -28,18 +28,45 @@ use std::path::{Path, PathBuf}; /// panicked are rejected. pub const GIT_SERVICE_TIMEOUT_SECS_MAX: u64 = 100 * 365 * 24 * 60 * 60; +/// 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 = "Twigpine 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, @@ -248,7 +275,7 @@ pub struct Config { /// Tigris (S3-compatible) bucket for repo storage. /// Leave empty to disable Tigris and use local-only storage. - #[arg(long, env = "GITLAWB_TIGRIS_BUCKET", default_value = "")] + #[arg(long, env = "GITLAWB_TIGRIS_BUCKET", default_value = "", global = true)] pub tigris_bucket: String, /// Maximum pack body size for git-receive-pack and git-upload-pack, in bytes. diff --git a/crates/gitlawb-node/src/db/mod.rs b/crates/gitlawb-node/src/db/mod.rs index d1fa22e2..5647db4f 100644 --- a/crates/gitlawb-node/src/db/mod.rs +++ b/crates/gitlawb-node/src/db/mod.rs @@ -933,7 +933,7 @@ impl Db { .await .context("acquiring migration advisory lock")?; - let result = self.run_pending_migrations().await; + let result = self.run_pending_migrations(&mut lock_conn).await; let _ = sqlx::query("SELECT pg_advisory_unlock($1)") .bind(MIGRATION_ADVISORY_LOCK) @@ -945,12 +945,37 @@ impl Db { /// Apply every migration whose version isn't yet recorded, in order. /// Must be called while holding the migration advisory lock. - async fn run_pending_migrations(&self) -> Result<()> { + /// Runs through the connection already holding the migration advisory + /// lock, so a pool of size 1 cannot deadlock against itself. + /// True while receive-pack requests or pending ref transitions still have + /// in-flight work on this repo. Purge checks this under the repo lock + /// before deleting storage: a row whose effects have not settled must not + /// be stranded against a repo that no longer exists. Settled receive-pack + /// states are 'complete' and 'rejected_at_git'; a 'quarantined' row stays + /// unsettled because the operator resolves it against the live repo. A + /// transition counts settled only once 'applied'. + pub(crate) async fn has_unsettled_repo_work(&self, repo_id: &str) -> Result { + let unsettled: bool = sqlx::query_scalar( + "SELECT EXISTS ( + SELECT 1 FROM receive_pack_requests + WHERE repo_id = $1 AND state NOT IN ('complete', 'rejected_at_git') + UNION ALL + SELECT 1 FROM pending_ref_transitions + WHERE repo_id = $1 AND state <> 'applied' + )", + ) + .bind(repo_id) + .fetch_one(&self.pool) + .await?; + Ok(unsettled) + } + + async fn run_pending_migrations(&self, conn: &mut sqlx::PgConnection) -> Result<()> { for m in MIGRATIONS { let existing: Option = sqlx::query("SELECT name FROM schema_migrations WHERE version = $1") .bind(m.version) - .fetch_optional(&self.pool) + .fetch_optional(&mut *conn) .await? .map(|r| r.get::("name")); @@ -981,7 +1006,7 @@ impl Db { // Run the migration body in a single transaction so a failure // mid-way leaves the database in its prior state rather than // partially mutated. - let mut tx = self.pool.begin().await?; + let mut tx = sqlx::Acquire::begin(&mut *conn).await?; for stmt in m.stmts { sqlx::query(stmt).execute(&mut *tx).await.with_context(|| { format!( @@ -2394,6 +2419,48 @@ pub(crate) fn normalize_owner_key(did: &str) -> &str { /// drift apart. If you change `normalize_owner_key`, update this const too. const OWNER_KEY_CASE_SQL: &str = "CASE WHEN owner_did LIKE 'did:key:%' AND position(':' in substr(owner_did, 9)) = 0 THEN substr(owner_did, 9) ELSE owner_did END"; +/// `OWNER_KEY_CASE_SQL` over an arbitrary owner column. The const above hardcodes +/// `owner_did` (the `repos` column the `idx_repos_owner_key_name` index is built +/// on); the bounty tombstone needs the same normalizer over `bounties.repo_owner`, +/// which is not index-backed. Byte-identical to `normalize_owner_key`; update this +/// alongside the const and the fn if the rule changes. +fn owner_key_case_sql(col: &str) -> String { + format!( + "CASE WHEN {col} LIKE 'did:key:%' AND position(':' in substr({col}, 9)) = 0 \ + THEN substr({col}, 9) ELSE {col} END" + ) +} + +/// Take a transaction-scoped Postgres advisory lock keyed on the normalized repo +/// slug (`normalize_owner_key(owner)/name`). `delete_repo_by_id`'s purge cascade and +/// `upsert_mirror_repo`'s ingest both take this lock on the identical slug string, so +/// a peer-sync mirror insert cannot interleave between the purge's point-in-time +/// sibling check and its slug-scoped deletes (which would half-wipe a live mirror's +/// child rows / bounty). `hashtextextended($1, 0)` folds the slug to the `bigint` the +/// lock API needs; both call sites hash the identical string, so their keys agree. +/// XACT-scoped, so it auto-releases on commit/rollback with no unlock bookkeeping +/// (mirrors the session-scoped `pg_advisory_lock` migration guard, but self-releasing). +async fn lock_repo_slug(conn: &mut sqlx::PgConnection, slug: &str) -> Result<()> { + sqlx::query("SELECT pg_advisory_xact_lock(hashtextextended($1, 0))") + .bind(slug) + .execute(conn) + .await?; + Ok(()) +} + +/// Normalize an `owner/name` slug to the exact string `delete_repo_by_id` builds its +/// advisory-lock key from: `normalize_owner_key(owner)/name`, splitting at the first +/// `/` (owners never contain `/`; DID owners use `:`). Writers that receive the slug +/// as one string (`branch_cids.repo`, `sync_queue.repo`, `received_ref_updates.repo`) +/// must lock this normalized form, not the verbatim input, or a full-DID caller would +/// hash a different key than the purge and slip past the serialization. A string with +/// no `/` is returned as-is; the purge never locks such a key, so it is inert. +fn normalize_repo_slug(repo: &str) -> String { + match repo.split_once('/') { + Some((owner, name)) => format!("{}/{}", normalize_owner_key(owner), name), + None => repo.to_string(), + } +} /// SQL CASE expression byte-identical to `normalize_owner_key`, but for columns /// named `did` (like in agent_profiles) instead of `owner_did`. const PROFILE_DID_CASE_SQL: &str = "CASE WHEN did LIKE 'did:key:%' AND position(':' in substr(did, 9)) = 0 THEN substr(did, 9) ELSE did END"; @@ -2677,6 +2744,14 @@ impl Db { } let now = Utc::now().to_rfc3339(); let id = format!("{owner_short}/{name}"); + // Slug the purge cascade keys on. Take the shared advisory lock on it inside a + // tx so a concurrent `delete_repo_by_id` purge of a colliding row cannot + // interleave its sibling-check-then-cascade around this insert and wipe the + // mirror we are ingesting. Normalize the owner the same way the cascade does so + // both call sites hash the identical slug string. + let slug = format!("{}/{}", normalize_owner_key(owner_short), name); + let mut tx = self.pool.begin().await?; + lock_repo_slug(&mut tx, &slug).await?; // `quarantined` is set only on first insert (the admission decision). // A re-sync (ON CONFLICT) preserves the existing flag — admission runs // once, and an operator's later release must not be silently reverted. @@ -2697,8 +2772,9 @@ impl Db { .bind(disk_path) .bind(machine_id) .bind(quarantined) - .execute(&self.pool) + .execute(&mut *tx) .await?; + tx.commit().await?; Ok(()) } @@ -3164,6 +3240,176 @@ 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); + + // Serialize this purge against a concurrent `upsert_mirror_repo` ingest that + // resolves to the same slug: both take this xact-scoped advisory lock on the + // identical slug string BEFORE the sibling check below, so a peer-sync insert + // cannot land between that point-in-time check and the slug-scoped cascade + // deletes (which would half-wipe the live mirror). Auto-released on commit. + lock_repo_slug(&mut tx, &slug).await?; + + // 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. + // INV-9 same-method slug collision: a canonical `did:key:zX` row and a bare + // `zX` mirror both normalize (via OWNER_KEY_CASE_SQL) to slug `zX/name`, so + // these tables cannot tell whose rows they are. Purging one row must not wipe + // a surviving sibling's records. Gate the slug-scoped deletes on the absence + // of any OTHER repos row resolving to the same slug, using the same + // OWNER_KEY_CASE_SQL the slug was built from so the check is byte-identical to + // the cascade key. The xact advisory lock taken at the top of this tx (and + // matched in `upsert_mirror_repo` on the identical slug) serializes this + // sibling-check-then-cascade against a concurrent peer-sync insert for the same + // slug, so this is no longer a bare point-in-time read a mirror upsert can slip + // past (closing the old KTD4 residual: the concurrent inserter is peer sync, + // not an operator, so the window was never operator-scoped). + let sibling_exists: bool = sqlx::query_scalar(&format!( + "SELECT EXISTS(SELECT 1 FROM repos WHERE id <> $1 AND {key} || '/' || name = $2)", + key = OWNER_KEY_CASE_SQL + )) + .bind(id) + .bind(&slug) + .fetch_one(&mut *tx) + .await?; + if !sibling_exists { + 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. + // + // But bounties key on (repo_owner, repo_name), not an immutable repo id, so + // an untouched OPEN bounty would silently re-attach to a same-name repo the + // owner recreates and become claimable again (finding F). Tombstone the + // repo's non-terminal bounties to a dead `purged` status instead of deleting + // them: this preserves the financial record while making them unclaimable + // (claim/submit/approve/cancel/dispute all gate on specific non-`purged` + // source statuses, so `purged` is inert and no transition reopens it). + // Scope by (repo_owner, repo_name) as the surviving purge does; the already + // terminal `completed`/`cancelled` bounties are left as-is. (Residual: a + // tombstoned bounty stays readable/listable under the same owner/name after + // recreation — a view, not a claim/payout re-attach; see the plan's KTD8.) + // + // Gate the tombstone on the SAME sibling check as the cascade: only when no + // other repos row resolves to this slug. If a mirror row survives, the logical + // repo still exists (and still serves via the mirror), so its bounty is still + // valid — tombstoning it would kill a live repo's bounty. Only when the last + // row for the slug is gone can a recreate re-attach an open bounty. + // + // When it does fire, match EVERY owner representation by normalizing BOTH sides. + // `create_bounty` stores `repo_owner` verbatim from the URL Path param, commonly + // the bare SHORT form (`z6Mk...`), but a full-DID URL that resolves to a bare + // mirror row yields the FULL `did:key:z6Mk...` form. The repo row here is likewise + // either form (`delete_repo_by_id` is reachable on bare mirror rows). Comparing the + // SQL-normalized stored `repo_owner` against the normalized target covers all four + // combinations (did:key-row + short-bounty AND bare-row + full-bounty); binding + // only one side left the mirror form `open`, so it re-attached to the recreated + // repo. It's a rare purge, so a seq scan on `bounties` (the normalizer is not + // index-backed) is fine. + if !sibling_exists { + sqlx::query(&format!( + "UPDATE bounties SET status = 'purged' + WHERE {key} = $1 AND repo_name = $2 + AND status IN ('open', 'claimed', 'submitted')", + key = owner_key_case_sql("repo_owner") + )) + .bind(normalize_owner_key(&owner_did)) + .bind(&name) + .execute(&mut *tx) + .await?; + } + + 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 ──────────────────────────────────────────────────────────── @@ -3482,6 +3728,15 @@ impl Db { cid: &str, node_did: &str, ) -> Result<()> { + // `branch_cids` is one of the slug-scoped tables `delete_repo_by_id` cascades + // over. Take the shared advisory lock (same normalized slug the purge hashes) + // inside a tx so this row cannot land between the purge's point-in-time + // sibling check and its cascade deletes (which would strand an orphan row or + // half-wipe a live mirror's state). Single lock, nothing else held, so the + // ordering matches every other slug-lock site and cannot deadlock. + let slug = normalize_repo_slug(repo); + let mut tx = self.pool.begin().await?; + lock_repo_slug(&mut tx, &slug).await?; sqlx::query( "INSERT INTO branch_cids (repo, ref_name, sha, cid, node_did, updated_at) VALUES ($1, $2, $3, $4, $5, $6) @@ -3495,8 +3750,9 @@ impl Db { .bind(cid) .bind(node_did) .bind(Utc::now().to_rfc3339()) - .execute(&self.pool) + .execute(&mut *tx) .await?; + tx.commit().await?; Ok(()) } @@ -3545,6 +3801,13 @@ impl Db { new_sha: &str, cid: Option<&str>, ) -> Result<()> { + // `sync_queue` is slug-scoped and cascaded by `delete_repo_by_id`; take the + // shared advisory lock on the same normalized slug so an enqueue cannot + // interleave with the purge's sibling-check-then-cascade (same shape and + // rationale as `upsert_branch_cid`; single lock, no deadlock exposure). + let slug = normalize_repo_slug(repo); + let mut tx = self.pool.begin().await?; + lock_repo_slug(&mut tx, &slug).await?; sqlx::query( "INSERT INTO sync_queue (id, repo, node_did, ref_name, new_sha, cid, status, enqueued_at) VALUES ($1, $2, $3, $4, $5, $6, 'pending', $7) @@ -3557,8 +3820,9 @@ impl Db { .bind(new_sha) .bind(cid) .bind(Utc::now().to_rfc3339()) - .execute(&self.pool) + .execute(&mut *tx) .await?; + tx.commit().await?; Ok(()) } @@ -8009,6 +8273,14 @@ impl Db { impl Db { pub async fn insert_ref_update(&self, update: &ReceivedRefUpdate) -> Result<()> { + // `received_ref_updates` is slug-scoped and cascaded by `delete_repo_by_id`; + // take the shared advisory lock on the same normalized slug so a peer-fed + // insert cannot land inside the purge's sibling-check-then-cascade window + // (same shape and rationale as `upsert_branch_cid`; single lock, no deadlock + // exposure). + let slug = normalize_repo_slug(&update.repo); + let mut tx = self.pool.begin().await?; + lock_repo_slug(&mut tx, &slug).await?; sqlx::query( "INSERT INTO received_ref_updates (id, node_did, pusher_did, repo, ref_name, old_sha, new_sha, timestamp, @@ -8028,8 +8300,9 @@ impl Db { .bind(&update.received_at) .bind(&update.from_peer) .bind(&update.owner_did) - .execute(&self.pool) + .execute(&mut *tx) .await?; + tx.commit().await?; Ok(()) } @@ -8446,6 +8719,13 @@ impl Db { pub async fn record_arweave_anchor(&self, input: &RecordAnchorInput<'_>) -> Result<()> { let id = Uuid::new_v4().to_string(); let now = Utc::now().to_rfc3339(); + // `arweave_anchors` is slug-scoped and cascaded by `delete_repo_by_id`; + // take the shared advisory lock on the same normalized slug so a post-push + // anchor cannot land inside the purge's sibling-check-then-cascade window + // (same shape as `upsert_branch_cid`; single lock, no deadlock exposure). + let slug = normalize_repo_slug(input.repo); + let mut tx = self.pool.begin().await?; + lock_repo_slug(&mut tx, &slug).await?; sqlx::query( "INSERT INTO arweave_anchors (id, repo, owner_did, ref_name, old_sha, new_sha, cid, irys_tx_id, arweave_url, node_did, anchored_at) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11)", @@ -8461,8 +8741,9 @@ impl Db { .bind(input.arweave_url) .bind(input.node_did) .bind(&now) - .execute(&self.pool) + .execute(&mut *tx) .await?; + tx.commit().await?; Ok(()) } @@ -11578,7 +11859,7 @@ mod agent_discovery_tests { #[cfg(test)] mod dedup_db_tests { - use super::{Db, RepoRecord}; + use super::{normalize_owner_key, BountyRecord, Db, RecordAnchorInput, RepoRecord}; use chrono::{DateTime, Utc}; use sqlx::PgPool; @@ -11617,6 +11898,709 @@ 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" + ); + } + + /// U4 gate-raise (INV-9 same-method slug collision): a canonical `did:key:zX` + /// row and a bare `zX` mirror of the same name both normalize to slug `zX/name`, + /// which is the key the slug-scoped child tables use. Purging the EMPTY canonical + /// row must NOT run the slug-scoped cascade, because that would wipe the surviving + /// non-empty mirror's branch_cids/arweave_anchors (keyed on the shared slug). The + /// physical `did:key:` row still goes; the sibling's slug-scoped records stay. + /// RED on base: the cascade fires and both survivor rows are deleted. + #[sqlx::test] + async fn delete_repo_keeps_sibling_slug_records_on_collision(pool: PgPool) { + let db = db(pool).await; + // Two rows collapsing to slug `z6MkSiblingCollisionFixture/victim`: an empty + // canonical `did:key:` row (deletion target) and a non-empty bare mirror. + let canonical = rec( + "rid-canonical-empty", + "did:key:z6MkSiblingCollisionFixture", + "victim", + "empty canonical", + "2026-01-01T00:00:00Z", + "2026-01-01T00:00:00Z", + ); + let mirror = rec( + "z6MkSiblingCollisionFixture/victim", + "z6MkSiblingCollisionFixture", + "victim", + "non-empty mirror", + "2026-02-01T00:00:00Z", + "2026-02-01T00:00:00Z", + ); + db.create_repo(&canonical).await.unwrap(); + db.create_repo(&mirror).await.unwrap(); + + // Both rows resolve to this slug; the child rows belong to the mirror. + let slug = format!( + "{}/victim", + crate::db::normalize_owner_key("did:key:z6MkSiblingCollisionFixture") + ); + sqlx::query( + "INSERT INTO branch_cids (repo, ref_name, sha, cid, node_did, updated_at) + VALUES ($1, 'refs/heads/main', '1', 'cid', 'n', '2026-02-01T00:00:00Z')", + ) + .bind(&slug) + .execute(db.pool()) + .await + .unwrap(); + sqlx::query( + "INSERT INTO arweave_anchors (id, repo, owner_did, ref_name, old_sha, new_sha, cid, irys_tx_id, arweave_url, node_did, anchored_at) + VALUES ('anch1', $1, 'z6MkSiblingCollisionFixture', 'refs/heads/main', '0', '1', 'cid', 'irys', 'https://ar/1', 'n', '2026-02-01T00:00:00Z')", + ) + .bind(&slug) + .execute(db.pool()) + .await + .unwrap(); + + // An OPEN bounty on the shared owner/name — it belongs to the logical repo the + // surviving mirror still serves, so purging the empty sibling must NOT tombstone it. + db.create_bounty(&BountyRecord { + id: "bnt-collision".to_string(), + repo_owner: "z6MkSiblingCollisionFixture".to_string(), + repo_name: "victim".to_string(), + issue_id: None, + title: "sibling bounty".to_string(), + amount: 5_000, + creator_did: "did:key:z6MkCreator".to_string(), + claimant_did: None, + claimant_wallet: None, + pr_id: None, + status: "open".to_string(), + created_at: "2026-02-01T00:00:00Z".to_string(), + claimed_at: None, + submitted_at: None, + completed_at: None, + deadline_secs: 604_800, + tx_hash: None, + }) + .await + .unwrap(); + + // Purge the empty canonical row. + let removed = db.delete_repo_by_id("rid-canonical-empty").await.unwrap(); + assert_eq!(removed, 1, "the empty canonical row is deleted"); + + async fn count(db: &Db, sql: &str, arg: &str) -> i64 { + sqlx::query_scalar::<_, i64>(sql) + .bind(arg) + .fetch_one(db.pool()) + .await + .unwrap() + } + // The surviving mirror keeps its slug-scoped records (the collision guard + // skips the slug-scoped cascade because a sibling row shares the slug). + assert!( + count(&db, "SELECT COUNT(*) FROM branch_cids WHERE repo=$1", &slug).await > 0, + "sibling's branch_cids must survive the collision purge" + ); + assert!( + count( + &db, + "SELECT COUNT(*) FROM arweave_anchors WHERE repo=$1", + &slug + ) + .await + > 0, + "sibling's arweave_anchors must survive the collision purge" + ); + assert_eq!( + count( + &db, + "SELECT COUNT(*) FROM repos WHERE id=$1", + "z6MkSiblingCollisionFixture/victim" + ) + .await, + 1, + "the surviving mirror repos row is untouched" + ); + // The bounty is NOT tombstoned: the mirror still serves the logical repo, so + // the tombstone is gated on no sibling surviving. + assert_eq!( + db.get_bounty("bnt-collision") + .await + .unwrap() + .unwrap() + .status, + "open", + "a bounty on a repo whose mirror survives the purge must stay open, not be tombstoned" + ); + } + + /// U1 (F1): the purge cascade must be serialized with mirror ingestion on the + /// shared slug. `delete_repo_by_id` runs a point-in-time sibling check and then, + /// only if no sibling was seen, wipes the slug-scoped child rows / tombstones the + /// bounty. A concurrent `upsert_mirror_repo` that inserts a colliding mirror row + /// AFTER that check but before/around the cascade would leave a half-wipe: the + /// mirror's `repos` row survives while its live child rows / bounty are gone. + /// + /// Deterministic race, with BOTH sides going through the real code: hold a session + /// advisory lock on the same `hashtextextended` slug key both sides hash, spawn the + /// REAL `upsert_mirror_repo` (parks on its own `lock_repo_slug`, first in the wait + /// queue), assert the mirror row has NOT landed while parked, then spawn the purge + /// (parks second) and release. Advisory-lock waiters are granted FIFO, so the + /// ingest commits the mirror row first and the purge's sibling check then SEES it + /// and skips the cascade: mirror row, child rows, and bounty all survive (GREEN). + /// Remove `lock_repo_slug` from `upsert_mirror_repo` and the ingest no longer + /// parks: the mirror row appears while the session lock is still held and the + /// mid-park absence assert fails (RED). Remove it from `delete_repo_by_id` and the + /// purge no longer blocks: it runs its sibling check before the parked ingest has + /// committed, cascades, and half-wipes the child rows / bounty (RED). + #[sqlx::test] + async fn purge_serializes_with_mirror_ingest_via_advisory_lock(pool: PgPool) { + let db = db(pool).await; + // Purge target: an empty canonical `did:key:` row. + let owner = "did:key:z6MkPurgeRaceMirrorFixtureForLock"; + let target = rec( + "rid-purge-race-target", + owner, + "victim", + "empty canonical", + "2026-01-01T00:00:00Z", + "2026-01-01T00:00:00Z", + ); + db.create_repo(&target).await.unwrap(); + + // The mirror's slug-scoped live state, keyed on the shared slug. The mirror's + // `repos` row is deliberately NOT present yet — ingestion lands it mid-race. + let slug = format!("{}/victim", crate::db::normalize_owner_key(owner)); + sqlx::query( + "INSERT INTO branch_cids (repo, ref_name, sha, cid, node_did, updated_at) + VALUES ($1, 'refs/heads/main', '1', 'cid', 'n', '2026-02-01T00:00:00Z')", + ) + .bind(&slug) + .execute(db.pool()) + .await + .unwrap(); + db.create_bounty(&BountyRecord { + id: "bnt-purge-race".to_string(), + repo_owner: "z6MkPurgeRaceMirrorFixtureForLock".to_string(), + repo_name: "victim".to_string(), + issue_id: None, + title: "mirror bounty".to_string(), + amount: 5_000, + creator_did: "did:key:z6MkCreator".to_string(), + claimant_did: None, + claimant_wallet: None, + pr_id: None, + status: "open".to_string(), + created_at: "2026-02-01T00:00:00Z".to_string(), + claimed_at: None, + submitted_at: None, + completed_at: None, + deadline_secs: 604_800, + tx_hash: None, + }) + .await + .unwrap(); + + // Hold a session-scoped advisory lock on the exact slug key the cascade takes. + // A session lock and the cascade's xact lock share Postgres' advisory lock + // space, so the purge's `pg_advisory_xact_lock` blocks on this until we unlock. + let mut lock_conn = db.pool().acquire().await.unwrap(); + sqlx::query("SELECT pg_advisory_lock(hashtextextended($1, 0))") + .bind(&slug) + .execute(&mut *lock_conn) + .await + .unwrap(); + + // Spawn the REAL mirror ingest; its own `lock_repo_slug` inside + // `upsert_mirror_repo` parks it on the held session lock, first in the wait + // queue. Bare owner short form, so it normalizes to the identical slug the + // purge locks. + let pool_for_ingest = db.pool().clone(); + let ingest = tokio::spawn(async move { + Db::for_testing(pool_for_ingest) + .upsert_mirror_repo( + "z6MkPurgeRaceMirrorFixtureForLock", + "victim", + "/srv/mirror", + None, + false, + ) + .await + }); + + // Give the ingest time to reach the lock wait, then assert it is actually + // parked: the mirror row must NOT exist while the session lock is held. This + // is the load-bearing check for the ingest side; without `lock_repo_slug` in + // `upsert_mirror_repo` the insert lands during this sleep (RED). + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + let landed_early: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM repos WHERE id=$1") + .bind(&slug) + .fetch_one(db.pool()) + .await + .unwrap(); + assert_eq!( + landed_early, 0, + "the real upsert_mirror_repo must block on the slug advisory lock, not insert past it" + ); + + // Spawn the purge; it parks on the same lock, second in the queue. Without + // `lock_repo_slug` in `delete_repo_by_id` it runs immediately instead: its + // sibling check sees no mirror row (the ingest is still parked), so it + // cascades the child rows and tombstones the bounty (RED below). + let pool_for_purge = db.pool().clone(); + let purge = tokio::spawn(async move { + Db::for_testing(pool_for_purge) + .delete_repo_by_id("rid-purge-race-target") + .await + }); + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + + // Release the lock. Advisory-lock waiters are woken FIFO: the ingest (queued + // first) commits the mirror row, then the purge acquires the lock and its + // sibling check observes the committed row. + sqlx::query("SELECT pg_advisory_unlock(hashtextextended($1, 0))") + .bind(&slug) + .execute(&mut *lock_conn) + .await + .unwrap(); + + ingest.await.unwrap().unwrap(); + let removed = purge.await.unwrap().unwrap(); + assert_eq!(removed, 1, "the empty canonical target row is purged"); + + async fn count(db: &Db, sql: &str, arg: &str) -> i64 { + sqlx::query_scalar::<_, i64>(sql) + .bind(arg) + .fetch_one(db.pool()) + .await + .unwrap() + } + // No half-wipe: the mirror row AND its slug-scoped state are all present. + assert_eq!( + count(&db, "SELECT COUNT(*) FROM repos WHERE id=$1", &slug).await, + 1, + "the mirror repos row survives the purge" + ); + assert!( + count(&db, "SELECT COUNT(*) FROM branch_cids WHERE repo=$1", &slug).await > 0, + "mirror branch_cids must survive: the purge serialized behind the ingest and saw the sibling" + ); + assert_eq!( + db.get_bounty("bnt-purge-race") + .await + .unwrap() + .unwrap() + .status, + "open", + "the mirror's bounty must stay open, not be tombstoned by the racing purge" + ); + } + + /// Sibling of the mirror-ingest race above, for the arweave-anchor writer. + /// `record_arweave_anchor` inserts a slug-scoped child row (`arweave_anchors`) + /// that `delete_repo_by_id`'s cascade wipes, so it must serialize on the same + /// `lock_repo_slug` key or a post-push anchor can land inside the purge's + /// sibling-check-then-cascade window. Deterministic choreography, same as the + /// mirror test: hold a session advisory lock on the exact `hashtextextended` + /// slug key, spawn the REAL `record_arweave_anchor` (its own `lock_repo_slug` + /// parks it), assert mid-park that the anchor row has NOT landed (this is the + /// load-bearing check: remove `lock_repo_slug` from `record_arweave_anchor` + /// and the insert lands during the sleep, RED), then release and assert the + /// row lands. The writer is handed the full-DID `owner/name` form so the test + /// also pins the `normalize_repo_slug` folding to the purge's key. + #[sqlx::test] + async fn purge_serializes_with_arweave_anchor_via_advisory_lock(pool: PgPool) { + let db = db(pool).await; + let owner = "did:key:z6MkAnchorRaceFixtureForSlugLock"; + let target = rec( + "rid-anchor-race-target", + owner, + "victim", + "canonical", + "2026-01-01T00:00:00Z", + "2026-01-01T00:00:00Z", + ); + db.create_repo(&target).await.unwrap(); + let slug = format!("{}/victim", crate::db::normalize_owner_key(owner)); + + // Hold a session-scoped advisory lock on the exact slug key the anchor + // writer's xact lock hashes; they share Postgres' advisory lock space. + let mut lock_conn = db.pool().acquire().await.unwrap(); + sqlx::query("SELECT pg_advisory_lock(hashtextextended($1, 0))") + .bind(&slug) + .execute(&mut *lock_conn) + .await + .unwrap(); + + // Spawn the REAL anchor writer with the full-DID slug form; its + // `normalize_repo_slug` folds it to the identical key the session lock + // holds, so `lock_repo_slug` parks it here. + let pool_for_anchor = db.pool().clone(); + let repo_arg = format!("{owner}/victim"); + let repo_arg_for_task = repo_arg.clone(); + let anchor = tokio::spawn(async move { + Db::for_testing(pool_for_anchor) + .record_arweave_anchor(&RecordAnchorInput { + repo: &repo_arg_for_task, + owner_did: "did:key:z6MkAnchorRaceFixtureForSlugLock", + ref_name: "refs/heads/main", + old_sha: "0", + new_sha: "1", + cid: Some("cid"), + irys_tx_id: "irys-tx-race", + arweave_url: "https://ar/anchor-race", + node_did: "did:key:z6MkNodeFixture", + }) + .await + }); + + // Give the writer time to reach the lock wait, then assert it is actually + // parked: no anchor row while the session lock is held. Without + // `lock_repo_slug` in `record_arweave_anchor` the insert commits during + // this sleep and the assert fails (RED). + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + let landed_early: i64 = + sqlx::query_scalar("SELECT COUNT(*) FROM arweave_anchors WHERE repo=$1") + .bind(&repo_arg) + .fetch_one(db.pool()) + .await + .unwrap(); + assert_eq!( + landed_early, 0, + "record_arweave_anchor must block on the slug advisory lock, not insert past it" + ); + + // Release; the parked writer acquires the lock and commits. + sqlx::query("SELECT pg_advisory_unlock(hashtextextended($1, 0))") + .bind(&slug) + .execute(&mut *lock_conn) + .await + .unwrap(); + anchor.await.unwrap().unwrap(); + + let landed: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM arweave_anchors WHERE repo=$1") + .bind(&repo_arg) + .fetch_one(db.pool()) + .await + .unwrap(); + assert_eq!(landed, 1, "the anchor lands once the lock is released"); + } + + /// U8 (F): bounties key on (repo_owner, repo_name), not an immutable repo id, + /// and `delete_repo_by_id` deliberately does not delete them (financial record). + /// Without tombstoning, a purged repo's OPEN bounty stays claimable and silently + /// re-attaches to a same-name repo recreated by the (spam) owner. The purge must + /// transition the affected non-terminal bounties to the terminal `purged` status + /// (preserving amount/wallet/tx_hash) so no claim/dispute path can re-expose them. + /// A bounty on an unrelated surviving repo must be left untouched. + /// RED on base: the bounty stays `open` and `claim_bounty` succeeds after recreate. + #[sqlx::test] + async fn delete_repo_by_id_tombstones_bounties_against_recreation(pool: PgPool) { + let db = db(pool).await; + let owner = "did:key:z6MkBountyPurgeFixtureForTombstone"; + let victim = rec( + "rid-bounty-victim", + owner, + "victim", + "spam repo", + "2026-01-01T00:00:00Z", + "2026-01-01T00:00:00Z", + ); + db.create_repo(&victim).await.unwrap(); + + // An unrelated surviving repo with its own open bounty. + let survivor_owner = "did:key:z6MkBountySurvivorFixtureUntouched"; + let survivor = rec( + "rid-bounty-survivor", + survivor_owner, + "keeper", + "legit repo", + "2026-01-01T00:00:00Z", + "2026-01-01T00:00:00Z", + ); + db.create_repo(&survivor).await.unwrap(); + + let bounty = |id: &str, r_owner: &str, r_name: &str, status: &str| BountyRecord { + id: id.to_string(), + repo_owner: r_owner.to_string(), + repo_name: r_name.to_string(), + issue_id: None, + title: "fix the bug".to_string(), + amount: 5_000, + creator_did: "did:key:z6MkCreator".to_string(), + claimant_did: None, + claimant_wallet: None, + pr_id: None, + status: status.to_string(), + created_at: "2026-01-01T00:00:00Z".to_string(), + claimed_at: None, + submitted_at: None, + completed_at: None, + deadline_secs: 604_800, + tx_hash: None, + }; + db.create_bounty(&bounty("bnt-victim", owner, "victim", "open")) + .await + .unwrap(); + // The common `create_bounty` path stores `repo_owner` verbatim from the URL + // Path param, which is the bare SHORT owner form (`z6Mk...`), not the full + // `did:key:z6Mk...`. The tombstone must match this form too or a short-form + // bounty stays `open` and re-attaches to the recreated repo. + let short_owner = normalize_owner_key(owner); + db.create_bounty(&bounty("bnt-victim-short", short_owner, "victim", "open")) + .await + .unwrap(); + // A mid-flight `claimed` bounty must also be tombstoned: otherwise a later + // dispute (which resets `claimed`/`submitted` back to `open`) would re-expose + // it on the recreated repo. + db.create_bounty(&bounty("bnt-victim-claimed", owner, "victim", "claimed")) + .await + .unwrap(); + // A mid-flight `submitted` bounty is in the tombstone set too. + db.create_bounty(&bounty( + "bnt-victim-submitted", + owner, + "victim", + "submitted", + )) + .await + .unwrap(); + // An already-terminal `completed` bounty is outside the tombstone set. + db.create_bounty(&bounty( + "bnt-victim-completed", + owner, + "victim", + "completed", + )) + .await + .unwrap(); + db.create_bounty(&bounty("bnt-survivor", survivor_owner, "keeper", "open")) + .await + .unwrap(); + + // Purge the spam repo, then let the owner recreate the same owner/name. + let removed = db.delete_repo_by_id("rid-bounty-victim").await.unwrap(); + assert_eq!(removed, 1, "the purged repo row is deleted"); + let recreated = rec( + "rid-bounty-recreated", + owner, + "victim", + "recreated repo", + "2026-03-01T00:00:00Z", + "2026-03-01T00:00:00Z", + ); + db.create_repo(&recreated).await.unwrap(); + + // The purged repo's bounty is tombstoned, so a claim is a no-op: the + // `UPDATE ... WHERE status='open'` matches nothing and it stays `purged`. + db.claim_bounty( + "bnt-victim", + "did:key:z6MkAttacker", + Some("0xattacker"), + "2026-03-02T00:00:00Z", + ) + .await + .unwrap(); + let victim_bounty = db.get_bounty("bnt-victim").await.unwrap().unwrap(); + assert_eq!( + victim_bounty.status, "purged", + "the purged repo's bounty must be tombstoned, not re-claimable" + ); + assert!( + victim_bounty.claimant_did.is_none(), + "the tombstoned bounty must not accept a new claimant" + ); + // The financial record is preserved (tombstoned, not deleted). + assert_eq!(victim_bounty.amount, 5_000, "amount preserved on tombstone"); + assert_eq!( + victim_bounty.creator_did, "did:key:z6MkCreator", + "creator_did preserved on tombstone" + ); + + // The short-owner-form bounty (the common create_bounty path) is tombstoned + // too, so it cannot re-attach to the recreated repo and become claimable. + db.claim_bounty( + "bnt-victim-short", + "did:key:z6MkAttacker", + Some("0xattacker"), + "2026-03-02T00:00:00Z", + ) + .await + .unwrap(); + let short_bounty = db.get_bounty("bnt-victim-short").await.unwrap().unwrap(); + assert_eq!( + short_bounty.status, "purged", + "a short-owner-form bounty on the purged repo must also be tombstoned" + ); + assert!( + short_bounty.claimant_did.is_none(), + "the tombstoned short-form bounty must not accept a new claimant" + ); + + // A mid-flight claimed bounty on the same repo is tombstoned too (so a + // dispute cannot reopen it against the recreated repo). + let claimed_bounty = db.get_bounty("bnt-victim-claimed").await.unwrap().unwrap(); + assert_eq!( + claimed_bounty.status, "purged", + "a claimed bounty on the purged repo must also be tombstoned" + ); + // Dispute gates on status IN ('claimed','submitted'); a tombstoned bounty is + // `purged`, so dispute is inert and cannot reopen it back to `open`. + db.dispute_bounty("bnt-victim-claimed").await.unwrap(); + let disputed_bounty = db.get_bounty("bnt-victim-claimed").await.unwrap().unwrap(); + assert_eq!( + disputed_bounty.status, "purged", + "dispute must not reopen a tombstoned bounty" + ); + + // A mid-flight submitted bounty on the same repo is tombstoned too. + let submitted_bounty = db + .get_bounty("bnt-victim-submitted") + .await + .unwrap() + .unwrap(); + assert_eq!( + submitted_bounty.status, "purged", + "a submitted bounty on the purged repo must also be tombstoned" + ); + + // An already-terminal completed bounty is left as-is (not in the tombstone set). + let completed_bounty = db + .get_bounty("bnt-victim-completed") + .await + .unwrap() + .unwrap(); + assert_eq!( + completed_bounty.status, "completed", + "a terminal completed bounty must be left untouched" + ); + + // The unrelated survivor's bounty is untouched and still claimable. + let survivor_bounty = db.get_bounty("bnt-survivor").await.unwrap().unwrap(); + assert_eq!( + survivor_bounty.status, "open", + "an unrelated repo's bounty must stay open" + ); + + // Symmetric owner-form case (the mirror of `bnt-victim-short`). Here the repo + // row is the BARE short-owner mirror (`owner_did = z6X`), while the bounty was + // stored in the FULL `did:key:z6X` form, the shape a full-DID URL that + // resolves to the bare mirror produces. The tombstone must normalize BOTH the + // stored `repo_owner` and the target, or this full-form bounty stays `open` + // when the bare row is purged and re-attaches to a recreated repo, the same + // re-attach hole in the opposite direction. `delete_repo_by_id` is reachable on + // bare mirror rows (`list_repos_by_owner_did` matches under normalization). + // RED before the symmetric fix (only the target side was normalized): stays + // `open`. GREEN after: `purged`. + let bare_owner = "z6MkBareOwnerFixtureForSymmetricTombstone"; + let bare = rec( + "rid-bounty-bare-victim", + bare_owner, + "victim", + "bare spam repo", + "2026-01-01T00:00:00Z", + "2026-01-01T00:00:00Z", + ); + db.create_repo(&bare).await.unwrap(); + db.create_bounty(&bounty( + "bnt-bare-full", + &format!("did:key:{bare_owner}"), + "victim", + "open", + )) + .await + .unwrap(); + let removed_bare = db + .delete_repo_by_id("rid-bounty-bare-victim") + .await + .unwrap(); + assert_eq!(removed_bare, 1, "the bare-owner repo row is deleted"); + let bare_full_bounty = db.get_bounty("bnt-bare-full").await.unwrap().unwrap(); + assert_eq!( + bare_full_bounty.status, "purged", + "a FULL-form bounty on a BARE-owner repo row must be tombstoned too \ + (symmetric normalization); RED before: only the target side was \ + normalized, so the full-form bounty stayed open" + ); + } + /// 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/error.rs b/crates/gitlawb-node/src/error.rs index c1af3f1b..7a382d88 100644 --- a/crates/gitlawb-node/src/error.rs +++ b/crates/gitlawb-node/src/error.rs @@ -50,6 +50,13 @@ pub enum AppError { #[error("too many requests: {0}")] TooManyRequests(String), + /// A 429 from a per-IP limiter that knows when its window frees a slot, so + /// the response advertises that delay as `Retry-After` the way the router + /// middleware's 429s do. `TooManyRequests` stays for limiters that have no + /// such delay to give. + #[error("too many requests: retry in {}s", .0.as_secs())] + RateLimited(std::time::Duration), + #[error("incomplete: {0}")] Incomplete(String), @@ -205,6 +212,11 @@ impl IntoResponse for AppError { AppError::TooManyRequests(msg) => { (StatusCode::TOO_MANY_REQUESTS, "rate_limited", msg.clone()) } + AppError::RateLimited(_) => ( + StatusCode::TOO_MANY_REQUESTS, + "rate_limited", + crate::rate_limit::RATE_LIMIT_MESSAGE.to_string(), + ), AppError::Incomplete(msg) => { (StatusCode::UNPROCESSABLE_ENTITY, "incomplete", msg.clone()) } @@ -221,6 +233,9 @@ impl IntoResponse for AppError { // 504, distinct from the 500 git_error and from the read-gate's 404 / // the auth 401, so the client can tell a deadline from a failure. AppError::Timeout(msg) => (StatusCode::GATEWAY_TIMEOUT, "git_timeout", msg.clone()), + // 503, retryable: a transient resource contention (e.g. the repo lock + // pool is pinned by a write burst or a purge holds the key), distinct + // from the 500 git_error so the client knows to back off and retry. // 503 with a FIXED body: the caller should retry, and must not be told // which repo is contended or for how long. AppError::RepoBusy => ( @@ -315,6 +330,12 @@ impl IntoResponse for AppError { axum::http::HeaderValue::from_static("1"), ); } + if let AppError::RateLimited(retry_after) = &self { + resp.headers_mut().insert( + axum::http::header::RETRY_AFTER, + axum::http::HeaderValue::from(retry_after.as_secs().max(1)), + ); + } resp } } diff --git a/crates/gitlawb-node/src/git/issues.rs b/crates/gitlawb-node/src/git/issues.rs index 7fdb8a47..240d4f80 100644 --- a/crates/gitlawb-node/src/git/issues.rs +++ b/crates/gitlawb-node/src/git/issues.rs @@ -61,6 +61,37 @@ pub fn create_issue(repo_path: &Path, issue_id: &str, json: &str) -> Result<()> Ok(()) } +/// Deadline-bounded variant of [`create_issue`] for compensation paths: the +/// same hash-object + update-ref write, but every git child runs through the +/// shared bounded runner so a hung git cannot stall the write guard past +/// `git_service_timeout`. +pub fn create_issue_bounded( + git_bin: &str, + repo_path: &Path, + issue_id: &str, + json: &str, + deadline: std::time::Instant, +) -> Result<()> { + let out = crate::git::visibility_pack::run_bounded_git( + git_bin, + &["hash-object", "--stdin", "-w"], + repo_path, + json.as_bytes(), + deadline, + )?; + let hash = String::from_utf8(out).context("git hash-object output is not valid UTF-8")?; + let hash = hash.trim(); + let ref_name = format!("refs/gitlawb/issues/{issue_id}"); + crate::git::visibility_pack::run_bounded_git( + git_bin, + &["update-ref", &ref_name, hash], + repo_path, + b"", + deadline, + )?; + Ok(()) +} + /// Remove a single issue ref while the write guard still holds the advisory lock, /// after a definite publish refusal. Best-effort: a failed delete leaves a /// local-only orphan, which is still better than telling the client to retry into a diff --git a/crates/gitlawb-node/src/git/repo_store.rs b/crates/gitlawb-node/src/git/repo_store.rs index c3cfe264..b1f67339 100644 --- a/crates/gitlawb-node/src/git/repo_store.rs +++ b/crates/gitlawb-node/src/git/repo_store.rs @@ -3,7 +3,8 @@ //! Every handler that needs access to a git repo on disk goes through `RepoStore`: //! //! - `acquire()` — ensures the repo is on local disk (downloads from Tigris on cache miss). -//! - `release_after_write()` — uploads the updated repo to Tigris after a write operation. +//! - `upload_under_guard()`: uploads the updated repo to Tigris after a write, under a +//! per-repo advisory lock the caller already holds (the fork tail's durability step). //! - `init()` — creates a new bare repo locally and uploads to Tigris. //! //! When Tigris is disabled (bucket empty), this is a simple passthrough to local disk. @@ -50,7 +51,7 @@ impl ConfirmedLiveTree { } } -/// 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, @@ -801,7 +802,22 @@ impl RepoStore { } /// Initialize a new bare repo on local disk and upload to Tigris. + /// Test-only convenience over `init_handle`; every + /// production caller keeps the upload inside its lock. + #[cfg(test)] pub async fn init(&self, owner_did: &str, repo_name: &str) -> Result { + let (path, upload) = self.init_handle(owner_did, repo_name).await?; + drop(upload); + Ok(path) + } + + /// [`init`](Self::init) plus the background archive upload's JoinHandle, so + /// callers holding a lock can keep the upload inside the lock's lifetime. + pub async fn init_handle( + &self, + owner_did: &str, + repo_name: &str, + ) -> Result<(PathBuf, Option>>)> { let (owner_slug, local_path) = self.local_path(owner_did, repo_name)?; store::init_bare(&local_path).context("initializing bare repo")?; @@ -812,12 +828,13 @@ impl RepoStore { clear_quarantine(&local_path, repo_name); // Upload to Tigris in background + let mut upload_handle = None; 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(); - tokio::spawn(async move { + upload_handle = Some(tokio::spawn(async move { // Create-only, and load-bearing: this uploads a freshly // initialized EMPTY repo, so a user who pushes immediately // after creating one would have their archive replaced by this @@ -828,20 +845,26 @@ impl RepoStore { .upload(&owner_slug, &repo_name, &path, UploadPrecondition::IfAbsent) .await { - Ok(_) => {} - // Distinct from the warn arm: the fence refusing is the - // design working, not a storage failure. + Ok(_) => Ok(()), + // Distinct from the error arm: the fence refusing means + // someone else already published this key, so the empty + // bytes dropped — but for a create caller the archive then + // belongs to another writer, which IS a creation failure. Err(UploadError::PreconditionLost { status }) => { info!(repo = %repo_name, status, "dropped the empty-repo upload: another writer already published this repo"); + Err(anyhow::anyhow!( + "empty-repo upload fenced: key already published (status {status})" + )) } Err(e) => { warn!(repo = %repo_name, err = %e, "failed to upload new repo to tigris"); + Err(anyhow::anyhow!("empty-repo upload failed: {e}")) } } - }); + })); } - Ok(local_path.into_path_buf()) + Ok((local_path.into_path_buf(), upload_handle)) } /// Upload a repo to Tigris after a write operation (push, merge, fork, etc.). @@ -893,6 +916,102 @@ impl RepoStore { Ok(()) } + /// 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.tigris.is_some() + } + + /// Try to take the per-repo advisory lock WITHOUT the write guard's + /// download/upload pipeline: a bare lock for callers (purge-spam, the + /// create/fork namespace serialization) that must serialize against the + /// write path on the same key but are not themselves publishing a tree. + /// + /// 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> { + self.try_lock_repo_on(&self.lock_pool, owner_did, repo_name) + .await + } + + /// [`try_lock_repo`](Self::try_lock_repo) against a caller-chosen pool. The + /// Postgres advisory lock is DATABASE-global, so the lock still mutually + /// excludes holders of the same key from any other pool — only the connection + /// the guard pins comes from `pool`. Backs the `lock_pool`-based delegators. + async fn try_lock_repo_on( + &self, + pool: &PgPool, + 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 conn = pool.acquire().await.context("acquiring lock connection")?; + // Route through LockProbe so a cancellation racing the query cannot + // return a connection holding an acquired session lock to the pool: + // the probe's Drop closes the session unless take_conn hands it out. + let mut probe = LockProbe::new(conn); + if !probe + .try_lock(lock_key) + .await + .context("try advisory lock")? + { + return Ok(None); + } + let conn = probe + .take_conn() + .context("lock probe lost its connection after a confirmed acquire")?; + Ok(Some(RepoLockGuard { + conn: Some(conn), + lock_key, + repo_name: repo_name.to_string(), + })) + } + + /// Bounded-wait lock-only acquire, without any object-store I/O. Retries + /// `try_lock_repo` with short backoff. Used by `create_repo` and `fork_repo` + /// to serialize their existence-check -> write -> row-insert spans against a + /// concurrent same-key purge (which holds this same lock across delete-row + + /// remove-dir). A key held past the bounded wait surfaces as `Ok(None)`, + /// which the create/fork handlers map to a retryable 503. + pub async fn lock_repo_blocking( + &self, + owner_did: &str, + repo_name: &str, + ) -> Result> { + self.lock_repo_blocking_on(&self.lock_pool, owner_did, repo_name) + .await + } + + /// [`lock_repo_blocking`](Self::lock_repo_blocking) against a caller-chosen + /// pool. The advisory lock is DB-global, so the guard mutually excludes the same + /// key across pools; only the pinned connection comes from `pool`. Backs the + /// `lock_pool`-based [`lock_repo_blocking`](Self::lock_repo_blocking) delegator. + pub async fn lock_repo_blocking_on( + &self, + pool: &PgPool, + owner_did: &str, + repo_name: &str, + ) -> Result> { + for attempt in 0..30 { + if let Some(g) = self.try_lock_repo_on(pool, 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. /// /// Three-layer defence against path traversal: @@ -1003,6 +1122,12 @@ pub(crate) fn claim_fork_disk_path(disk_path: &Path, attempt: &PublishAttemptId) let Some(path) = fork_attempt_path(disk_path) else { return; }; + // The stamp may be claimed BEFORE the mirror clone starts, when the owner + // directory does not exist yet; create it so the claim is not silently lost + // (a missing stamp makes every later ownership check refuse to clean). + if let Some(parent) = path.parent() { + let _ = std::fs::create_dir_all(parent); + } if let Err(e) = std::fs::write(&path, attempt.as_str()) { warn!( path = %disk_path.display(), @@ -1548,6 +1673,61 @@ fn invalidate_local_write_cache(local_path: &Path, repo_name: &str, reason: &str } } +impl RepoStore { + /// 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. + /// Returns the archive generation the refresh verified (its ETag), so the + /// caller can fence a later delete on exactly the view it checked. + pub async fn refresh_from_archive( + &self, + owner_did: &str, + repo_name: &str, + ) -> Result> { + let Some(ref store) = self.tigris else { + return Ok(None); + }; + let (owner_slug, local_path) = self.local_path(owner_did, repo_name)?; + let Some(generation) = store.head_generation(&owner_slug, repo_name).await? else { + return Ok(None); + }; + store + .download(&owner_slug, repo_name, &local_path, None) + .await?; + Ok(generation.etag) + } + + /// 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. The + /// delete is fenced on the generation the store sees, so a cross-node + /// publish racing the purge makes it fail rather than erasing the + /// successor's archive. + /// `expected` is the generation the caller verified empty (an ETag from + /// [`refresh_from_archive`]); a moved generation means a PUT landed after + /// the check and the archive must be kept, so `Ok(false)` is a skip. + pub async fn delete_archive( + &self, + owner_did: &str, + repo_name: &str, + expected: Option<&str>, + ) -> Result { + let Some(ref store) = self.tigris else { + return Ok(true); + }; + let (owner_slug, _local_path) = self.local_path(owner_did, repo_name)?; + store.delete_current(&owner_slug, repo_name, expected).await + } + + /// The bound applied to transfers that run while a repo lock is held. + pub(crate) fn transfer_bound(&self) -> std::time::Duration { + self.lock_held_transfer_timeout + } +} + /// Strict allowlist validator for `owner_did` and `repo_name`. /// /// Rejects any character that isn't explicitly safe, plus length and @@ -1704,7 +1884,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"); } @@ -2755,6 +2935,41 @@ pub fn build_lock_pool(source: &PgPool, max_connections: u32, acquire_timeout: D .connect_lazy_with((*source.connect_options()).clone()) } +/// 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. + 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(); + } + } +} + /// Compute a stable i64 hash for a Postgres advisory lock key. /// /// Uses SHA-256 (not `DefaultHasher`) so the same `(owner_slug, repo_name)` @@ -2780,7 +2995,7 @@ pub(crate) fn advisory_lock_key(owner_slug: &str, repo_name: &str) -> i64 { i64::from_le_bytes(digest[..8].try_into().expect("sha256 output is >= 8 bytes")) } #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::*; use crate::git::tigris::ATTEMPT_METADATA_KEY; use std::collections::HashMap; @@ -4917,7 +5132,7 @@ mod tests { } /// Seed the minimum bare-repo shape so the upload has something to archive. - fn seed_bare_repo(path: &Path) { + pub(crate) fn seed_bare_repo(path: &Path) { std::fs::create_dir_all(path.join("objects/info")).unwrap(); std::fs::create_dir_all(path.join("refs/heads")).unwrap(); std::fs::write(path.join("HEAD"), "ref: refs/heads/main\n").unwrap(); @@ -5080,7 +5295,7 @@ mod tests { /// models the real S3 arm we care about (body fully transmitted, commit /// decided later) with no timing in it. #[derive(Clone, Debug)] - struct CapturedPut { + pub(crate) struct CapturedPut { key: String, body: Vec, if_match: Option, @@ -5090,7 +5305,7 @@ mod tests { /// One stored object. #[derive(Clone, Debug)] - struct MockObject { + pub(crate) struct MockObject { body: Vec, etag: String, /// The `x-amz-meta-gitlawb-attempt` it was written with. This is what @@ -5103,14 +5318,14 @@ mod tests { /// `status` is `None` while a PUT is parked: it arrived and was logged, but /// no precondition has been evaluated for it yet. #[derive(Clone, Debug, PartialEq)] - struct PutAttempt { + pub(crate) struct PutAttempt { if_match: Option, if_none_match: Option, status: Option, } #[derive(Default)] - struct MockState { + pub(crate) struct MockState { /// Keyed by request path. A single-slot store was enough while every test /// drove one repo; fork creation touches the SOURCE key and the FORK key /// in one request, and collapsing them would make an assertion about one @@ -5145,7 +5360,7 @@ mod tests { /// /// The fence tests downstream are only worth anything if a precondition can /// actually fail here, so this helper carries its own semantics tests below. - struct S3Mock { + pub(crate) struct S3Mock { endpoint: String, state: Arc>, gate: Arc, @@ -5215,7 +5430,7 @@ mod tests { } impl S3Mock { - async fn start() -> Self { + pub(crate) async fn start() -> Self { use axum::response::IntoResponse; let state = Arc::new(std::sync::Mutex::new(MockState::default())); @@ -5424,7 +5639,7 @@ mod tests { } } - fn endpoint(&self) -> &str { + pub(crate) fn endpoint(&self) -> &str { &self.endpoint } @@ -5445,7 +5660,7 @@ mod tests { } /// The stored object under a specific repo key. - fn object_for(&self, owner_slug: &str, repo_name: &str) -> Option { + pub(crate) fn object_for(&self, owner_slug: &str, repo_name: &str) -> Option { let key = format!("test-bucket/repos/v1/{owner_slug}/{repo_name}.tar.zst"); self.state.lock().unwrap().objects.get(&key).cloned() } @@ -5459,7 +5674,7 @@ mod tests { /// to pick up a precondition and then PUTs on it is therefore fencing /// on a generation that is already gone, which is the only way to drive /// two consecutive lost preconditions deterministically. - fn roll_generation_after_next_heads(&self, n: u32) { + pub(crate) fn roll_generation_after_next_heads(&self, n: u32) { self.state.lock().unwrap().roll_after_heads = n; } @@ -5478,7 +5693,7 @@ mod tests { /// Deliver the next PUT in full and then fail it WITHOUT committing. The /// client's knowledge is identical to the arm above — that is the whole /// point — so only asking the store can tell the two apart. - fn fail_next_put_after_delivery(&self) { + pub(crate) fn fail_next_put_after_delivery(&self) { self.state.lock().unwrap().lose_next_response = Some(false); } @@ -5489,7 +5704,7 @@ mod tests { /// Fail the next HEAD for ONE repo key with a 500, so a caller's /// reconciliation cannot be answered either way. - fn fail_next_head_for(&self, owner_slug: &str, repo_name: &str) { + pub(crate) fn fail_next_head_for(&self, owner_slug: &str, repo_name: &str) { let key = format!("test-bucket/repos/v1/{owner_slug}/{repo_name}.tar.zst"); *self .state @@ -5501,7 +5716,7 @@ mod tests { } /// How many DELETEs the mock actually carried out. - fn deletes(&self) -> u32 { + pub(crate) fn deletes(&self) -> u32 { self.state.lock().unwrap().deletes } @@ -5531,7 +5746,7 @@ mod tests { .0 } - fn shutdown(&self) { + pub(crate) fn shutdown(&self) { self.server.abort(); } } @@ -6262,25 +6477,25 @@ mod tests { /// A client aimed at the same key the store under test publishes to, so a /// test can seed the archive or land an interfering publish of its own. - fn mock_tigris(mock: &S3Mock) -> TigrisClient { + pub(crate) fn mock_tigris(mock: &S3Mock) -> TigrisClient { TigrisClient::for_testing_with_endpoint("test-bucket", mock.endpoint()) } /// The slug `local_path` derives from a DID, needed because a test seeds /// and reads the archive key directly. - fn owner_slug_of(owner_did: &str) -> String { + pub(crate) fn owner_slug_of(owner_did: &str) -> String { owner_did.replace([':', '/'], "_") } /// A bare-repo-shaped directory carrying `marker`, so a test can tell whose /// tree is stored without comparing compressed bytes. - fn marked_repo(path: &Path, marker: &str) { + pub(crate) fn marked_repo(path: &Path, marker: &str) { seed_bare_repo(path); std::fs::write(path.join("MARKER"), marker).unwrap(); } /// The marker inside whatever archive is currently stored under the key. - async fn stored_marker(mock: &S3Mock, owner_slug: &str, repo_name: &str) -> String { + pub(crate) async fn stored_marker(mock: &S3Mock, owner_slug: &str, repo_name: &str) -> String { let out = TempDir::new().unwrap(); let validated = super::validated_repo_disk_path(out.path(), "did:key:stored", "stored") .expect("test repo path must validate"); diff --git a/crates/gitlawb-node/src/git/store.rs b/crates/gitlawb-node/src/git/store.rs index bb282e32..d5262bab 100644 --- a/crates/gitlawb-node/src/git/store.rs +++ b/crates/gitlawb-node/src/git/store.rs @@ -1817,6 +1817,10 @@ pub fn repo_disk_path(repos_dir: &Path, owner_did: &str, repo_name: &str) -> std #[cfg(test)] mod tests { + use super::{branch_diff_names, merge_branch}; + use std::path::Path; + use std::process::Command; + use super::{validate_git_ref, validate_git_ref_with_git, GitRefValidationError}; #[test] @@ -1883,10 +1887,6 @@ mod tests { ); } - use super::{branch_diff_names, merge_branch}; - use std::path::Path; - use std::process::Command; - /// #26 split 1/4: a bare repo this node creates must KEEP REFLOGS, because /// the durable-outbox reconcile has no other way to prove that a stranded /// transition actually landed. `core.logAllRefUpdates` defaults to false for diff --git a/crates/gitlawb-node/src/git/tigris.rs b/crates/gitlawb-node/src/git/tigris.rs index f3a82e90..82c53ac6 100644 --- a/crates/gitlawb-node/src/git/tigris.rs +++ b/crates/gitlawb-node/src/git/tigris.rs @@ -522,6 +522,49 @@ impl TigrisClient { } } + /// Delete whatever archive currently exists, fenced on the generation read + /// at call time: a concurrent publish between the HEAD and the DELETE makes + /// the store refuse rather than erasing the successor's object. Used by + /// purge-spam, which deletes a repo it did not publish (so + /// [`delete_if_attempt_matches`](Self::delete_if_attempt_matches) cannot + /// apply) while still needing the same moved-generation protection. + /// Returns `true` when an archive was deleted. + /// Delete only if the object's generation still matches `expected` — the + /// generation the caller checked for emptiness. A PUT landing between that + /// check and this delete makes the generation move; deleting the newer + /// (possibly non-empty) archive is exactly the outcome the fence exists to + /// prevent, so a moved generation refuses rather than deletes. + pub async fn delete_current( + &self, + owner_slug: &str, + repo_name: &str, + expected: Option<&str>, + ) -> Result { + let Some(generation) = self.head_generation(owner_slug, repo_name).await? else { + return Ok(false); + }; + if generation.etag.as_deref() != expected { + return Ok(false); + } + let key = Self::repo_key(owner_slug, repo_name); + let mut req = self.s3.delete_object().bucket(&self.bucket).key(&key); + if let Some(etag) = generation.etag.as_deref() { + req = req.if_match(etag); + } + match req.send().await { + Ok(_) => Ok(true), + Err(e) => { + if e.raw_response() + .map(|raw| raw.status().as_u16()) + .is_some_and(|s| s == 412 || s == 409) + { + anyhow::bail!("archive generation moved between head and delete; refusing"); + } + Err(anyhow::anyhow!("tigris DELETE {key}: {e}")) + } + } + } + /// Download a repo archive from Tigris and extract to local disk. pub async fn download( &self, diff --git a/crates/gitlawb-node/src/graphql/mod.rs b/crates/gitlawb-node/src/graphql/mod.rs index 70f8b92a..de6b2b4e 100644 --- a/crates/gitlawb-node/src/graphql/mod.rs +++ b/crates/gitlawb-node/src/graphql/mod.rs @@ -14,6 +14,11 @@ use subscription::SubscriptionRoot; pub type GitlawbSchema = Schema; +/// No mutations: the /graphql/ws transport keeps queries and subscriptions but +/// has no mutation root, so the socket can never be used to run a write and +/// sidestep the write-brake bucket the HTTP route pays. +pub type GitlawbWsSchema = Schema; + // Keep ordinary schema discovery and application queries usable while // bounding validation work and the number of resolver selections per request. const GRAPHQL_MAX_COMPLEXITY: usize = 400; @@ -171,6 +176,30 @@ pub fn build_schema( .finish() } +/// Build the subscriptions-only schema served over /graphql/ws. Carries the +/// same broadcast senders the subscription resolvers consume. Mutations are +/// disabled — the schema's mutation root is `EmptyMutation` — but queries still +/// run via `QueryRoot` (they are reads and already ride the TaskReadBrake), so +/// "subscriptions-only" refers to the write surface, not the operation set. +pub fn build_ws_schema( + db: Arc, + ref_update_tx: tokio::sync::broadcast::Sender, + task_event_tx: tokio::sync::broadcast::Sender, + task_cursor_key: crate::api::task_cursor::TaskCursorKey, +) -> GitlawbWsSchema { + apply_query_limits(Schema::build( + QueryRoot, + async_graphql::EmptyMutation, + SubscriptionRoot, + )) + .data(db) + .data(ref_update_tx) + .data(task_event_tx) + .data(task_cursor_key) + .extension(TaskReadBrakeExtension) + .finish() +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/gitlawb-node/src/lib.rs b/crates/gitlawb-node/src/lib.rs index f1c4c117..667372df 100644 --- a/crates/gitlawb-node/src/lib.rs +++ b/crates/gitlawb-node/src/lib.rs @@ -3,6 +3,7 @@ // migrations) so the out-of-crate deny-harness integration crate can spawn a // real node; `src/main.rs` is a thin shim over `run()`. Modules the harness // reaches are `pub`; the rest stay crate-private. +mod admin; mod ans104; pub mod api; mod arweave; @@ -84,6 +85,12 @@ pub async fn run() -> 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); @@ -327,6 +334,12 @@ pub async fn run() -> Result<()> { task_event_tx.clone(), task_cursor_key.clone(), )); + let graphql_ws_schema = Arc::new(graphql::build_ws_schema( + Arc::clone(&db), + ref_update_tx.clone(), + task_event_tx.clone(), + task_cursor_key.clone(), + )); let machine_id = std::env::var("FLY_MACHINE_ID").ok(); if let Some(ref mid) = machine_id { @@ -382,36 +395,26 @@ pub async fn run() -> 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, issue close, labels, stars, merges, protect, replicas, webhook + // create/delete, visibility, tasks, bounties, profile, every HTTP /graphql + // request). Issue comment/close debit it in the handler, after the read + // gate; the rest via the router-level brake. 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"); // close_issue drives a full archive download for non-owner author pre-checks. // Keep it on its own bucket so it cannot drain the receive-pack limit. @@ -436,7 +439,7 @@ pub async fn run() -> 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 @@ -477,10 +480,12 @@ pub async fn run() -> Result<()> { ref_update_tx, task_event_tx, graphql_schema, + graphql_ws_schema, machine_id, repo_store, rate_limiter, create_ip_rate_limiter, + write_rate_limiter, push_rate_limiter, close_issue_rate_limiter, ipfs_max_history_walks: crate::api::ipfs::MAX_HISTORY_WALKS_PER_REQUEST, @@ -630,14 +635,17 @@ pub async fn run() -> Result<()> { // Periodic cleanup of expired rate limit entries + consumed-proof ledger { - let sweep_state = state.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)) => { - sweep_state.sweep_rate_limiters().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) @@ -884,6 +892,110 @@ pub async fn run() -> 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(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" + ); + } + } + }; + // Give the purge-spam store a dedicated advisory-lock pool, separate + // from `db`'s app pool. The guard then holds a lock-pool connection + // while `delete_repo_by_id` runs on the app pool, so a purge at + // GITLAWB_DB_MAX_CONNECTIONS=1 does not self-deadlock (KTD3). + let lock_pool = Db::lock_pool( + &config.database_url, + config.db_lock_pool_max_connections, + acquire_timeout, + )?; + let repo_store = git::repo_store::RepoStore::new( + config.repos_dir.clone(), + object_store, + lock_pool, + std::time::Duration::from_secs(config.lock_held_transfer_timeout_secs), + ); + admin::run_purge_spam(&db, &repo_store, &config.repos_dir, execute) + .await + .map(|_| ()) + } + } +} + /// U4 (#173): spawn the periodic legacy provider-CID repair sweep. Releases before this /// version stored the PROVIDER CID (Kubo dag-pb / Pinata CIDv0) in `pinned_cids.cid`, /// and this version's `/ipfs/{cid}` resolver withholds any row whose stored key is not @@ -1485,6 +1597,7 @@ mod rate_limiter_sweep_tests { let mut state = crate::test_support::test_state_lazy(); state.rate_limiter = RateLimiter::new(10, window); state.create_ip_rate_limiter = RateLimiter::new(10, window); + state.write_rate_limiter = RateLimiter::new(10, window); state.push_rate_limiter = RateLimiter::new(10, window); state.close_issue_rate_limiter = RateLimiter::new(10, window); state.sync_trigger_rate_limiter = RateLimiter::new(10, window); @@ -1498,6 +1611,7 @@ mod rate_limiter_sweep_tests { [ s.rate_limiter.clone(), s.create_ip_rate_limiter.clone(), + s.write_rate_limiter.clone(), s.push_rate_limiter.clone(), s.close_issue_rate_limiter.clone(), s.sync_trigger_rate_limiter.clone(), @@ -1514,7 +1628,7 @@ mod rate_limiter_sweep_tests { } tokio::time::sleep(window * 3).await; - state.sweep_rate_limiters().await; + state.cleanup_rate_limiters().await; for (i, l) in limiters(&state).into_iter().enumerate() { assert_eq!(l.tracked_keys().await, 0, "limiter {i} was not swept"); @@ -1725,6 +1839,40 @@ async fn gossip_ping_round( } } +#[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::{ diff --git a/crates/gitlawb-node/src/rate_limit.rs b/crates/gitlawb-node/src/rate_limit.rs index d5ed801a..c35362d2 100644 --- a/crates/gitlawb-node/src/rate_limit.rs +++ b/crates/gitlawb-node/src/rate_limit.rs @@ -70,18 +70,21 @@ impl RateLimiter { self.window.min(Duration::from_secs(1)) } - /// The configured window, exposed so a 429 can carry an honest - /// `retry-after`: a hardcoded value lies when the operator retunes the - /// window, and a client that honors the header would retry into another - /// 429 until the real window elapses. - pub fn window(&self) -> Duration { - self.window + /// Returns `true` if the request is allowed. Thin wrapper over + /// [`check_retry`](Self::check_retry) for callers that only need the + /// allow/deny decision, not the rejection's Retry-After delay. + pub(crate) async fn check(&self, key: &str) -> bool { + self.check_retry(key).await.is_none() } - pub(crate) async fn check(&self, key: &str) -> bool { + /// Record a request against `key`. Returns `None` when it is allowed, or + /// `Some(retry_after)` when it is rejected — the delay a client should wait + /// before retrying, so a 429 can advertise a value consistent with the + /// actual window instead of a constant. + pub(crate) async fn check_retry(&self, key: &str) -> Option { // max_requests == 0 means the limiter is disabled, not "block all". if self.max_requests == 0 { - return true; + return None; } let now = Instant::now(); let mut state = self.state.lock().await; @@ -93,10 +96,25 @@ impl RateLimiter { .timestamps .retain(|t| now.duration_since(*t) < self.window); if window.timestamps.len() >= self.max_requests { - return false; + // Insertion order is preserved, so after the retain the first + // element is the oldest live request. It ages out (freeing a + // slot) one window after it landed, so that remaining time is + // the delay to advertise. Clamp to >= 1 so a 429 never tells a + // client to retry immediately. + let retry = window + .timestamps + .first() + .map(|oldest| { + self.window + .as_secs() + .saturating_sub(now.duration_since(*oldest).as_secs()) + }) + .unwrap_or(0) + .max(1); + return Some(Duration::from_secs(retry)); } window.timestamps.push(now); - return true; + return None; } // New key. Enforce the cap BEFORE inserting so a flood of distinct keys @@ -117,7 +135,11 @@ impl RateLimiter { } drop(last_sweep); if state.len() >= self.max_keys { - return false; + // The key is untracked (never inserted), so there is no oldest + // entry to derive a delay from. Advertise the full window as a + // conservative bound: capacity frees as other keys expire, at + // the latest one window from now. + return Some(self.window.max(Duration::from_secs(1))); } } state.insert( @@ -126,7 +148,23 @@ impl RateLimiter { timestamps: vec![now], }, ); - true + None + } + + /// Give back one slot a prior [`check`](Self::check) took for `key`, for a + /// caller that debits two buckets and must undo the first when the second + /// refuses. Drops the newest timestamp: the count is what the cap reads, so + /// that is equivalent to dropping the caller's own even if another request + /// landed in between. An unknown key is a no-op, and a key left with no + /// timestamps is removed so a refund never holds a map entry. + pub(crate) async fn refund(&self, key: &str) { + let mut state = self.state.lock().await; + if let Some(window) = state.get_mut(key) { + window.timestamps.pop(); + if window.timestamps.is_empty() { + state.remove(key); + } + } } /// Non-consuming check: is this key ALREADY at its limit for the current window? @@ -178,6 +216,19 @@ impl RateLimiter { }); } + /// 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. /// Number of distinct keys currently tracked. Test-only introspection so a /// cross-module test can assert that a sweep actually evicted expired entries /// and observe what it reclaimed. There is no production reader. @@ -185,6 +236,23 @@ impl RateLimiter { pub(crate) async fn tracked_keys(&self) -> usize { self.state.lock().await.len() } + + /// Test-only: seed a key's bucket with entries of the given ages (how long + /// ago each request landed), so a test can drive the Retry-After computation + /// off a controlled oldest-entry age without wall-clock sleeps. Ages that + /// would underflow the monotonic clock collapse to `now`. + #[cfg(test)] + pub(crate) async fn seed_aged_entry_for_test(&self, key: &str, ages: &[Duration]) { + let now = Instant::now(); + let timestamps = ages + .iter() + .map(|a| now.checked_sub(*a).unwrap_or(now)) + .collect(); + self.state + .lock() + .await + .insert(key.to_string(), Window { timestamps }); + } } /// Per-source concurrency cap derived from the write-pool size: one resolved client @@ -300,8 +368,8 @@ pub async fn rate_limit_by_did(request: Request, next: Next) -> Response { .map(|a| a.0.clone()); if let (Some(limiter), Some(did)) = (limiter, did) { - if !limiter.check(&did).await { - return too_many_requests(limiter.window().as_secs()); + if let Some(retry_after) = limiter.check_retry(&did).await { + return too_many_requests(retry_after); } } @@ -404,18 +472,21 @@ impl axum::extract::FromRequestParts for PeerAddr { } } +/// The shared 429 response for the rate-limit middlewares (per-DID and per-IP +/// alike). Route-agnostic: the message stays generic (the offending path, when +/// logged, is recorded at the call site). /// The message every per-IP brake answers with, shared so the GraphQL surface /// (which cannot return a 429 status inside a 200 GraphQL envelope) says the /// same thing as the REST routes. pub const RATE_LIMIT_MESSAGE: &str = "rate limit exceeded — try again later"; -/// The shared 429 response for the per-IP flood brakes. Route-agnostic: this -/// middleware now serves the push path AND the peer-sync routes, so the message -/// stays generic (the offending path is recorded in the warn log below). -pub fn too_many_requests(retry_after_secs: u64) -> Response { +/// The shared 429 response for the rate-limit middlewares (per-DID and per-IP +/// alike). Route-agnostic: the message stays generic (the offending path, when +/// logged, is recorded at the call site). +pub fn too_many_requests(retry_after: Duration) -> Response { ( StatusCode::TOO_MANY_REQUESTS, - [("retry-after", retry_after_secs.to_string())], + [("retry-after", retry_after.as_secs().to_string())], RATE_LIMIT_MESSAGE, ) .into_response() @@ -482,9 +553,9 @@ pub async fn rate_limit_by_ip(request: Request, next: Next) -> Response { if let Some(limiter) = limiter { if let Some(key) = client_key(request.headers(), peer, limiter.trust) { - if !limiter.limiter.check(&key).await { + if let Some(retry_after) = limiter.limiter.check_retry(&key).await { tracing::warn!(key = %key, path = %request.uri().path(), "per-IP rate limit exceeded"); - return too_many_requests(limiter.limiter.window().as_secs()); + return too_many_requests(retry_after); } } } @@ -870,4 +941,221 @@ 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)" + ); + } + + // ── Retry-After derived from the window, not a constant 60 (U5) ─────── + // A 429 must advertise the delay until the oldest live request ages out of + // the limiter's window, so the value tracks the real window instead of the + // hardcoded 60 that contradicted every limiter's 3600s window. + + async fn resp_from(router: &axum::Router, peer: SocketAddr) -> Response { + use tower::ServiceExt; + let mut req = axum::http::Request::builder() + .method(axum::http::Method::POST) + .uri("/o/r/git-receive-pack") + .body(axum::body::Body::empty()) + .unwrap(); + req.extensions_mut().insert(ConnectInfo(peer)); + router.clone().oneshot(req).await.unwrap() + } + + fn retry_after_secs(resp: &Response) -> u64 { + resp.headers() + .get("retry-after") + .expect("429 must carry a retry-after header") + .to_str() + .unwrap() + .parse() + .expect("retry-after must be an integer number of seconds") + } + + #[tokio::test] + async fn retry_after_reflects_full_window_for_freshly_filled_bucket() { + // A just-filled bucket's oldest entry is ~now, so the advertised delay + // must be close to the whole window (100s), NOT the old constant 60. + let router = ip_limited_router(IpRateLimiter { + limiter: RateLimiter::new(1, Duration::from_secs(100)), + trust: TrustedProxy::None, + }); + let peer: SocketAddr = "203.0.113.50:6000".parse().unwrap(); + assert_eq!(resp_from(&router, peer).await.status(), StatusCode::OK); + let resp = resp_from(&router, peer).await; + assert_eq!(resp.status(), StatusCode::TOO_MANY_REQUESTS); + let retry = retry_after_secs(&resp); + assert!( + (95..=100).contains(&retry), + "freshly-filled bucket must advertise ~window (95..=100s), got {retry} (constant-60 bug if 60)" + ); + } + + #[tokio::test] + async fn retry_after_shrinks_as_oldest_entry_ages() { + // Seed the sole slot with an entry already 95s into a 100s window: the + // next request is rejected and must advertise only the ~5s remaining, + // not 60. + let limiter = RateLimiter::new(1, Duration::from_secs(100)); + let peer: SocketAddr = "203.0.113.51:6000".parse().unwrap(); + limiter + .seed_aged_entry_for_test(&peer.ip().to_string(), &[Duration::from_secs(95)]) + .await; + let router = ip_limited_router(IpRateLimiter { + limiter, + trust: TrustedProxy::None, + }); + let resp = resp_from(&router, peer).await; + assert_eq!(resp.status(), StatusCode::TOO_MANY_REQUESTS); + let retry = retry_after_secs(&resp); + assert!( + (1..=10).contains(&retry), + "an aged bucket must advertise the small remaining delay (1..=10s), got {retry}" + ); + } + + #[tokio::test] + async fn retry_after_clamps_to_at_least_one_second() { + // Oldest entry is 99s into a 100s window: the raw remainder rounds toward + // 1s and must never be advertised as 0 (a 0/blank retry lets a rejected + // client hammer immediately). + let limiter = RateLimiter::new(1, Duration::from_secs(100)); + let peer: SocketAddr = "203.0.113.52:6000".parse().unwrap(); + limiter + .seed_aged_entry_for_test(&peer.ip().to_string(), &[Duration::from_secs(99)]) + .await; + let router = ip_limited_router(IpRateLimiter { + limiter, + trust: TrustedProxy::None, + }); + let resp = resp_from(&router, peer).await; + assert_eq!(resp.status(), StatusCode::TOO_MANY_REQUESTS); + let retry = retry_after_secs(&resp); + assert!(retry >= 1, "retry-after must clamp to >= 1, got {retry}"); + assert!( + retry <= 2, + "with 99s of a 100s window elapsed, the remaining delay is ~1s, got {retry}" + ); + } + + /// `refund` gives back exactly one slot a prior `check` took, so a caller + /// that debits two buckets can undo the first when the second refuses. + #[tokio::test] + async fn refund_returns_one_slot_and_is_a_no_op_for_an_unknown_key() { + let limiter = RateLimiter::new(2, Duration::from_secs(60)); + assert!(limiter.check("k").await); + assert!(limiter.check("k").await); + assert!(!limiter.check("k").await, "bucket is full"); + + limiter.refund("k").await; + assert!(limiter.check("k").await, "one slot came back"); + assert!(!limiter.check("k").await, "and only one"); + + // An unknown key must neither panic nor create an entry. + limiter.refund("never-seen").await; + assert!(limiter.state.lock().await.get("never-seen").is_none()); + + // Refunding the only slot drops the key, so a refunded key does not + // hold a map entry (the limiter's key cap bounds its own state). + let single = RateLimiter::new(1, Duration::from_secs(60)); + assert!(single.check("solo").await); + single.refund("solo").await; + assert!(single.state.lock().await.get("solo").is_none()); + } } diff --git a/crates/gitlawb-node/src/server.rs b/crates/gitlawb-node/src/server.rs index 848abf2b..66ca2760 100644 --- a/crates/gitlawb-node/src/server.rs +++ b/crates/gitlawb-node/src/server.rs @@ -67,7 +67,7 @@ async fn graphql_ws_handler( key: rate_limit::client_key(&headers, peer, state.push_limiter_trust), request_count: std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)), }); - let schema = state.graphql_schema.as_ref().clone(); + let schema = state.graphql_ws_schema.as_ref().clone(); upgrade .protocols(ALL_WEBSOCKET_PROTOCOLS) .on_upgrade(move |stream| { @@ -118,18 +118,57 @@ fn add_auth_layers( )) } +/// Attach the shared per-IP write brake to an authenticated write-route group, +/// outermost so a flood is rejected before signature verification. Every +/// authenticated non-creation write group MUST call this or it silently ships +/// unbraked with no compile error. Deliberately a separate call, NOT folded into +/// `add_auth_layers`: the creation routes use a different bucket +/// (`create_ip_rate_limiter`) and the graphql group is not built through +/// `add_auth_layers`, so a helper keyed on auth would attach the wrong limiter. +trait WriteBraked { + fn write_braked(self, limiter: &rate_limit::IpRateLimiter) -> 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 graphql_routes = Router::new() + // 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_http = 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 (mounted on + // its own router below, read-only subscriptions) gets no brake. The + // per-IP write brake covers /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 stays unbraked. + .layer(middleware::from_fn_with_state( + auth::SignatureBodyLimit(SIGNED_BODY_LIMIT), + auth::optional_signature, + )) + .write_braked(&write_ip_limiter); + let graphql_ws = Router::new() .route("/graphql/ws", get(graphql_ws_handler)) - // Attach the verified DID to /graphql and /graphql/ws when a signature is present. .layer(middleware::from_fn_with_state( auth::SignatureBodyLimit(SIGNED_BODY_LIMIT), auth::optional_signature, )); + let graphql_routes = graphql_http.merge(graphql_ws); - // ── 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)) @@ -138,7 +177,8 @@ pub fn build_router(state: AppState) -> Router { .route("/api/v1/tasks/{id}/fail", post(tasks::fail_task)), state.clone(), SIGNED_BODY_LIMIT, - ); + ) + .write_braked(&write_ip_limiter); // ── Task routes (read — open, but scoped) ────────────────────────────── // `optional_signature` attaches the verified DID when a signature is present @@ -197,7 +237,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( @@ -268,7 +314,8 @@ pub fn build_router(state: AppState) -> Router { ), state.clone(), SIGNED_BODY_LIMIT, - ); + ) + .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. @@ -381,7 +428,8 @@ pub fn build_router(state: AppState) -> Router { ), state.clone(), SIGNED_BODY_LIMIT, - ); + ) + .write_braked(&write_ip_limiter); // ── Bounty routes (read — open) ────────────────────────────────────── let bounty_read_routes = Router::new() @@ -406,9 +454,15 @@ pub fn build_router(state: AppState) -> Router { Router::new().route("/api/v1/profile", axum::routing::put(profiles::set_profile)), state.clone(), SIGNED_BODY_LIMIT, - ); - - // ── Issue routes (write — require HTTP Signature, no rate limit) ───── + ) + .write_braked(&write_ip_limiter); + + // ── Issue routes (write — require HTTP Signature; NO outer write brake) ── + // Both handlers are read-gated before they mutate, so an outer per-IP + // brake would answer 429 before authorize_repo_read could return the + // opaque 404 a hidden repo owes the caller — the brake's status would + // distinguish a real repo from a missing one (INV-12). The write bucket + // is debited inside each handler, right after the read gate. let issue_write_routes = add_auth_layers( Router::new() .route( diff --git a/crates/gitlawb-node/src/state.rs b/crates/gitlawb-node/src/state.rs index 0b88490d..5b60a030 100644 --- a/crates/gitlawb-node/src/state.rs +++ b/crates/gitlawb-node/src/state.rs @@ -72,6 +72,7 @@ pub struct AppState { pub task_event_tx: tokio::sync::broadcast::Sender, /// GraphQL schema (queries + mutations + subscriptions) pub graphql_schema: Arc, + pub graphql_ws_schema: Arc, /// MAC key for task-list continuation tokens, derived from the node /// keypair seed. Held here (and handed to the GraphQL schema) so REST and /// GraphQL mint and accept the same tokens. @@ -92,6 +93,21 @@ 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, webhook create/delete, visibility (its GET + /// shares the group), tasks, bounties, profile, and every HTTP `/graphql` + /// request (queries and the playground GET as well as mutations; the + /// WebSocket route is excluded). Issue comment and issue close debit it in + /// the handler after the read gate, not via router middleware; close also + /// draws on `close_issue_rate_limiter` and spends neither bucket when + /// either refuses. 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. @@ -372,24 +388,6 @@ impl AppState { self.shutdown_tx.subscribe() } - /// Sweep expired entries from every per-IP/DID rate limiter. Driven by the - /// periodic cleanup task so a bounded limiter's key map sheds stale entries - /// instead of sitting near its cap until an inline capacity sweep reclaims - /// them. Every limiter on the state is swept here; adding a new limiter means - /// adding it to this list. - pub(crate) async fn sweep_rate_limiters(&self) { - self.rate_limiter.cleanup().await; - self.create_ip_rate_limiter.cleanup().await; - self.push_rate_limiter.cleanup().await; - self.close_issue_rate_limiter.cleanup().await; - self.ipfs_rate_limiter.cleanup().await; - self.ipfs_work_rate_limiter.cleanup().await; - self.sync_trigger_rate_limiter.cleanup().await; - self.peer_write_rate_limiter.cleanup().await; - self.task_read_rate_limiter.cleanup().await; - self.federated_rate_limiter.cleanup().await; - } - /// Trigger graceful shutdown. Idempotent — calling more than once /// has no effect. Returns `true` if this call was the one that /// flipped the signal. @@ -411,6 +409,26 @@ impl AppState { *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.close_issue_rate_limiter.cleanup().await; + self.ipfs_rate_limiter.cleanup().await; + self.ipfs_work_rate_limiter.cleanup().await; + self.sync_trigger_rate_limiter.cleanup().await; + self.peer_write_rate_limiter.cleanup().await; + self.task_read_rate_limiter.cleanup().await; + self.federated_rate_limiter.cleanup().await; + } + /// Legacy-probe budget wired from the `GITLAWB_IPFS_MAX_LEGACY_PROBES` operator /// knob. The knob seeds `ipfs_max_legacy_probes` at construction so it controls the /// per-request legacy (NULL-provenance) probe fan-out it advertises. It deliberately @@ -1273,6 +1291,35 @@ mod repo_identity_key_tests { } } +#[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)" + ); + } +} + #[cfg(test)] mod repo_write_lease_tests { use super::RepoWriteLeases; diff --git a/crates/gitlawb-node/src/test_harness.rs b/crates/gitlawb-node/src/test_harness.rs index 64da604b..c1c1577c 100644 --- a/crates/gitlawb-node/src/test_harness.rs +++ b/crates/gitlawb-node/src/test_harness.rs @@ -170,6 +170,12 @@ fn build_state(db: Arc, pool: PgPool, repos_dir: PathBuf) -> AppS task_tx.clone(), task_cursor_key.clone(), )); + let ws_schema = Arc::new(crate::graphql::build_ws_schema( + db.clone(), + ref_tx.clone(), + task_tx.clone(), + task_cursor_key.clone(), + )); AppState { config: Arc::new(Config::parse_from(["gitlawb-node"])), db, @@ -180,11 +186,13 @@ fn build_state(db: Arc, pool: PgPool, repos_dir: PathBuf) -> AppS ref_update_tx: ref_tx, task_event_tx: task_tx, graphql_schema: schema, + graphql_ws_schema: ws_schema, task_cursor_key, machine_id: None, repo_store: crate::git::repo_store::RepoStore::for_testing(repos_dir, 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(600, Duration::from_secs(3600)), push_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)), close_issue_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)), federated_rate_limiter: RateLimiter::new(12, Duration::from_secs(60)), diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 66926209..2c4480ef 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -95,6 +95,13 @@ fn build_state(db: Arc, pool: PgPool) -> AppState { task_tx.clone(), task_cursor_key.clone(), )); + + let ws_schema = Arc::new(graphql::build_ws_schema( + db.clone(), + ref_tx.clone(), + task_tx.clone(), + task_cursor_key.clone(), + )); AppState { config: Arc::new(Config::parse_from(["gitlawb-node"])), db, @@ -105,11 +112,13 @@ fn build_state(db: Arc, pool: PgPool) -> AppState { ref_update_tx: ref_tx, task_event_tx: task_tx, graphql_schema: schema, + graphql_ws_schema: ws_schema, task_cursor_key, machine_id: None, 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)), close_issue_rate_limiter: RateLimiter::new(120, Duration::from_secs(3600)), ipfs_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)), @@ -3428,7 +3437,7 @@ mod tests { run(&["commit", "-q", "-m", "seed"], &src); // The tag that shadows the missing target branch: "release" resolves via // DWIM but refs/heads/release never exists. - run(&["tag", "release"], &src); + run(&["tag", "-m", "release", "release"], &src); run(&["checkout", "-q", "-b", "feature"], &src); std::fs::write(src.join("feature.txt"), b"feature-only").unwrap(); run(&["add", "feature.txt"], &src); @@ -3588,7 +3597,7 @@ mod tests { // moves past it. Revision DWIM prefers refs/tags/ over refs/heads/ for // an ambiguous short name, so this fixture is the strongest shadowing // shape: a naive resolution would merge onto the stale tag commit. - run(&["tag", "release"], &src); + run(&["tag", "-m", "release", "release"], &src); run(&["checkout", "-q", "-b", "release"], &src); std::fs::write(src.join("release.txt"), b"release-only").unwrap(); run(&["add", "release.txt"], &src); @@ -17954,12 +17963,12 @@ mod tests { } /// The periodic cleanup task must sweep the ipfs walk limiter, not only its - /// five siblings. Drives `AppState::sweep_rate_limiters` — the exact method the + /// five siblings. Drives `AppState::cleanup_rate_limiters` — the exact method the /// 300s loop calls — and asserts the ipfs limiter's expired entry is evicted. /// Dropping `ipfs_rate_limiter.cleanup()` from that method leaves the entry in /// place (`tracked_keys` stays 1): the RED proof that the sweep covers it. #[sqlx::test] - async fn sweep_rate_limiters_includes_ipfs_limiter(pool: PgPool) { + async fn cleanup_rate_limiters_includes_ipfs_limiter(pool: PgPool) { let mut state = test_state(pool).await; // Short window so a single recorded hit is already expired at sweep time. state.ipfs_rate_limiter = crate::rate_limit::RateLimiter::new(5, Duration::from_millis(50)); @@ -17976,7 +17985,7 @@ mod tests { // Expire the entry (still mapped — cleanup hasn't run), then sweep. tokio::time::sleep(Duration::from_millis(60)).await; - state.sweep_rate_limiters().await; + state.cleanup_rate_limiters().await; assert_eq!( state.ipfs_rate_limiter.tracked_keys().await, @@ -18279,12 +18288,12 @@ mod tests { /// U5 (R6): the periodic cleanup task sweeps the NEW work-budget limiter too, not /// only the route limiter and its siblings. Mirrors - /// `sweep_rate_limiters_includes_ipfs_limiter`: drive `sweep_rate_limiters` and + /// `cleanup_rate_limiters_includes_ipfs_limiter`: drive `cleanup_rate_limiters` and /// assert the work limiter's expired entry is evicted. Dropping the /// `ipfs_work_rate_limiter.cleanup()` call from that method leaves the entry in place /// (`tracked_keys` stays 1): the RED proof the sweep covers it. #[sqlx::test] - async fn sweep_rate_limiters_includes_ipfs_work_limiter(pool: PgPool) { + async fn cleanup_rate_limiters_includes_ipfs_work_limiter(pool: PgPool) { let mut state = test_state(pool).await; state.ipfs_work_rate_limiter = crate::rate_limit::RateLimiter::new(5, Duration::from_millis(50)); @@ -18300,7 +18309,7 @@ mod tests { ); tokio::time::sleep(Duration::from_millis(60)).await; - state.sweep_rate_limiters().await; + state.cleanup_rate_limiters().await; assert_eq!( state.ipfs_work_rate_limiter.tracked_keys().await,