diff --git a/.env.example b/.env.example index 45fe3245..b9124d98 100644 --- a/.env.example +++ b/.env.example @@ -95,6 +95,16 @@ GITLAWB_MAX_PACK_BYTES=2147483648 # IP per hour. 0 disables. Default 600. GITLAWB_PUSH_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 +# the trigger bucket so an unsigned notify flood can't drain trigger's quota. +# 0 disables. Default 600. +GITLAWB_PEER_WRITE_RATE_LIMIT=600 +# /api/v1/sync/trigger requires a signature and fans out to every peer per call, +# so it gets a tight bucket. 0 disables. Default 60. +GITLAWB_SYNC_TRIGGER_RATE_LIMIT=60 + # Which forwarded header the edge is trusted to set, used to resolve the real # client IP for the push limiter. One of: # (unset) — no trusted proxy: key on the socket peer address, ignore headers. diff --git a/Cargo.lock b/Cargo.lock index 30abfa8e..d12daa43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2293,9 +2293,9 @@ dependencies = [ [[package]] name = "crossbeam-epoch" -version = "0.9.18" +version = "0.9.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b82ac4a3c2ca9c3460964f020e1402edd5753411d7737aa39c3714ad1b5420e" +checksum = "2d6914041f254d6e9176c01941b21115dcfb7089e55135a35411081bd106ef3f" dependencies = [ "crossbeam-utils", ] @@ -3300,7 +3300,7 @@ dependencies = [ [[package]] name = "git-remote-gitlawb" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "gitlawb-core", @@ -3312,7 +3312,7 @@ dependencies = [ [[package]] name = "gitlawb-attest" -version = "0.4.0" +version = "0.5.0" dependencies = [ "base64", "ed25519-dalek", @@ -3329,7 +3329,7 @@ dependencies = [ [[package]] name = "gitlawb-core" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "base64", @@ -3356,7 +3356,7 @@ dependencies = [ [[package]] name = "gitlawb-node" -version = "0.4.0" +version = "0.5.0" dependencies = [ "alloy", "anyhow", @@ -3412,7 +3412,7 @@ dependencies = [ [[package]] name = "gl" -version = "0.4.0" +version = "0.5.0" dependencies = [ "alloy", "anyhow", diff --git a/README.md b/README.md index 4bcdc03b..0be37d20 100644 --- a/README.md +++ b/README.md @@ -302,20 +302,21 @@ POST /api/v1/bounties/{id}/... POST /{owner}/{repo}/git-receive-pack ``` -Peer write routes support staged rollout: +These peer write routes support staged rollout: ```txt POST /api/v1/peers/announce POST /api/v1/sync/notify -POST /api/v1/sync/trigger ``` -When `GITLAWB_REQUIRE_SIGNED_PEER_WRITES=false`, unsigned legacy peers are accepted, but signed requests are verified when signature headers are present. Once all live peers upgrade, operators can set: +When `GITLAWB_REQUIRE_SIGNED_PEER_WRITES=false`, unsigned legacy peers are accepted on those two routes, but signed requests are verified when signature headers are present. Once all live peers upgrade, operators can set: ```bash GITLAWB_REQUIRE_SIGNED_PEER_WRITES=true ``` +`POST /api/v1/sync/trigger` is not part of the staged rollout: it always requires a signature in both config modes and returns 401 without one, because each call drives an O(peers) outbound fan-out. + --- ## Configuration diff --git a/crates/gitlawb-node/src/api/peers.rs b/crates/gitlawb-node/src/api/peers.rs index 7dc006ab..a125c313 100644 --- a/crates/gitlawb-node/src/api/peers.rs +++ b/crates/gitlawb-node/src/api/peers.rs @@ -537,4 +537,458 @@ mod tests { // pins the `o[0] == 0` check against an off-by-one into 1.x. assert!(is_public_http_url("http://1.0.0.0/")); } + + // ── #82: /sync/trigger signature gate + per-IP brake on the peer-sync routes ── + // + // These drive the FULL production router (crate::server::build_router) so the + // route wiring, layer order, and config-mode branching are all under test. + // The positive-path and DID-agnostic cases mount the handler directly because + // signed_request_as injects only an AuthenticatedDid extension (not a real + // RFC-9421 signature), which require_signature rejects on the full router. + use crate::rate_limit::{IpRateLimiter, RateLimiter, TrustedProxy}; + use crate::state::AppState; + use crate::test_support::{signed_request_as, test_state}; + use axum::body::Body; + use axum::extract::ConnectInfo; + use axum::http::{header, Method, Request, StatusCode}; + use axum::routing::post; + use axum::{middleware, Extension, Router}; + use gitlawb_core::http_sig::sign_request; + use gitlawb_core::identity::Keypair; + use sqlx::PgPool; + use std::net::SocketAddr; + use std::sync::Arc; + use std::time::Duration; + use tower::ServiceExt; + + fn unsigned_post(uri: &str, body: &str, peer: &str) -> Request { + let mut req = Request::builder() + .method(Method::POST) + .uri(uri) + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(body.to_string())) + .unwrap(); + req.extensions_mut() + .insert(ConnectInfo(peer.parse::().unwrap())); + req + } + + fn require_signed_peer_writes(state: &mut AppState) { + let mut cfg = (*state.config).clone(); + cfg.require_signed_peer_writes = true; + state.config = Arc::new(cfg); + } + + const NOTIFY_BODY: &str = r#"{"repo":"demo","ref_name":"refs/heads/main","new_sha":"0000000000000000000000000000000000000000","node_did":"PEER_DID"}"#; + + // ── trigger: mandatory signature ────────────────────────────────────────── + + #[sqlx::test] + async fn sync_trigger_rejects_unsigned_in_default_mode(pool: PgPool) { + // The hole: with require_signed_peer_writes=false (default), an anonymous + // caller reaches the fan-out. Must be 401 regardless of the flag. + let state = test_state(pool).await; + let router = crate::server::build_router(state); + let resp = router + .oneshot(unsigned_post( + "/api/v1/sync/trigger", + "{}", + "203.0.113.1:5000", + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + } + + #[sqlx::test] + async fn sync_trigger_rejects_unsigned_in_signed_writes_mode(pool: PgPool) { + // The signature requirement must not depend on the flag. + let mut state = test_state(pool).await; + require_signed_peer_writes(&mut state); + let router = crate::server::build_router(state); + let resp = router + .oneshot(unsigned_post( + "/api/v1/sync/trigger", + "{}", + "203.0.113.2:5000", + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + } + + #[sqlx::test] + async fn sync_trigger_admits_signed_caller(pool: PgPool) { + // must-not-over-throttle: a signed caller reaches the handler and (with no + // peers seeded) gets 200, not 429. Direct-mount because a real signature + // cannot be forged through the full router in tests. + let state = test_state(pool).await; + let did = Keypair::generate().did().to_string(); + let app = Router::new() + .route("/api/v1/sync/trigger", post(super::trigger_sync)) + .layer(middleware::from_fn(crate::rate_limit::rate_limit_by_ip)) + .layer(Extension(IpRateLimiter { + limiter: RateLimiter::new(60, Duration::from_secs(3600)), + trust: TrustedProxy::None, + })) + .with_state(state); + let mut req = + signed_request_as(&did, Method::POST, "/api/v1/sync/trigger", Body::from("{}")); + req.extensions_mut().insert(ConnectInfo( + "203.0.113.3:5000".parse::().unwrap(), + )); + let resp = app.oneshot(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + } + + // ── trigger: IP brake ───────────────────────────────────────────────────── + + #[sqlx::test] + async fn sync_trigger_ip_flood_is_throttled(pool: PgPool) { + // The brake is outermost, so an over-limit request 429s before it ever + // reaches require_signature — an unsigned request suffices to prove it. + let mut state = test_state(pool).await; + state.sync_trigger_rate_limiter = RateLimiter::new(1, Duration::from_secs(3600)); + state.push_limiter_trust = TrustedProxy::None; + let peer = "203.0.113.4:5000"; + // Exhaust the single-request budget up front. + assert!( + state + .sync_trigger_rate_limiter + .check(&peer.parse::().unwrap().ip().to_string()) + .await + ); + let router = crate::server::build_router(state); + let resp = router + .oneshot(unsigned_post("/api/v1/sync/trigger", "{}", peer)) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::TOO_MANY_REQUESTS); + } + + #[sqlx::test] + async fn sync_trigger_brake_is_did_agnostic(pool: PgPool) { + // Why per-IP, not per-DID: a DID farm (fresh did:key per request) does not + // bypass an IP-keyed brake. Two DISTINCT DIDs from one IP, brake limit 1 → + // the second is 429. A per-DID limiter would give each DID its own bucket. + let state = test_state(pool).await; + let app = Router::new() + .route("/api/v1/sync/trigger", post(super::trigger_sync)) + .layer(middleware::from_fn(crate::rate_limit::rate_limit_by_ip)) + .layer(Extension(IpRateLimiter { + limiter: RateLimiter::new(1, Duration::from_secs(3600)), + trust: TrustedProxy::None, + })) + .with_state(state); + let peer = "203.0.113.5:5000".parse::().unwrap(); + let mk = |did: &str| { + let mut r = + signed_request_as(did, Method::POST, "/api/v1/sync/trigger", Body::from("{}")); + r.extensions_mut().insert(ConnectInfo(peer)); + r + }; + let first = app + .clone() + .oneshot(mk(&Keypair::generate().did().to_string())) + .await + .unwrap(); + assert_eq!(first.status(), StatusCode::OK); + let second = app + .oneshot(mk(&Keypair::generate().did().to_string())) + .await + .unwrap(); + assert_eq!(second.status(), StatusCode::TOO_MANY_REQUESTS); + } + + #[sqlx::test] + async fn sync_trigger_forged_forwarded_header_cannot_bypass(pool: PgPool) { + // TrustedProxy::None keys on the socket peer, so rotating X-Forwarded-For + // from one socket peer does not refill the bucket. limit 1: first request + // consumes it (then 401 unsigned), second from the same socket → 429 + // regardless of a different XFF value. + let mut state = test_state(pool).await; + state.sync_trigger_rate_limiter = RateLimiter::new(1, Duration::from_secs(3600)); + state.push_limiter_trust = TrustedProxy::None; + let router = crate::server::build_router(state); + let mut a = unsigned_post("/api/v1/sync/trigger", "{}", "203.0.113.6:5000"); + a.headers_mut() + .insert("x-forwarded-for", "1.1.1.1".parse().unwrap()); + assert_eq!( + router.clone().oneshot(a).await.unwrap().status(), + StatusCode::UNAUTHORIZED + ); + let mut b = unsigned_post("/api/v1/sync/trigger", "{}", "203.0.113.6:5000"); + b.headers_mut() + .insert("x-forwarded-for", "2.2.2.2".parse().unwrap()); + assert_eq!( + router.oneshot(b).await.unwrap().status(), + StatusCode::TOO_MANY_REQUESTS + ); + } + + #[sqlx::test] + async fn sync_trigger_rate_limit_zero_disables_but_signature_holds(pool: PgPool) { + // 0 disables the brake (RateLimiter::check early-returns), proving the two + // halves are independent: no 429 even under a flood, but the signature gate + // still 401s every unsigned request. + let mut state = test_state(pool).await; + state.sync_trigger_rate_limiter = RateLimiter::new(0, Duration::from_secs(3600)); + state.push_limiter_trust = TrustedProxy::None; + let router = crate::server::build_router(state); + for _ in 0..3 { + let resp = router + .clone() + .oneshot(unsigned_post( + "/api/v1/sync/trigger", + "{}", + "203.0.113.7:5000", + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + } + } + + // ── notify: braked, but signature behavior unchanged ────────────────────── + + async fn seed_peer(state: &AppState) -> String { + let did = Keypair::generate().did().to_string(); + state + .db + .upsert_peer(&did, "https://peer.example.com") + .await + .unwrap(); + did + } + + #[sqlx::test] + async fn sync_notify_unsigned_flood_is_throttled(pool: PgPool) { + // notify still accepts an unsigned known-peer notification (rolling-upgrade + // compat), but a flood from one IP is now braked. brake limit 1: first + // enqueues (200), second from the same IP → 429. + let mut state = test_state(pool).await; + state.peer_write_rate_limiter = RateLimiter::new(1, Duration::from_secs(3600)); + state.push_limiter_trust = TrustedProxy::None; + let peer_did = seed_peer(&state).await; + let body = NOTIFY_BODY.replace("PEER_DID", &peer_did); + let router = crate::server::build_router(state); + let first = router + .clone() + .oneshot(unsigned_post( + "/api/v1/sync/notify", + &body, + "203.0.113.8:5000", + )) + .await + .unwrap(); + assert_eq!(first.status(), StatusCode::OK); + let second = router + .oneshot(unsigned_post( + "/api/v1/sync/notify", + &body, + "203.0.113.8:5000", + )) + .await + .unwrap(); + assert_eq!(second.status(), StatusCode::TOO_MANY_REQUESTS); + } + + #[sqlx::test] + async fn sync_notify_distinct_ips_are_not_collaterally_throttled(pool: PgPool) { + // must-not-over-throttle: per-IP keying means one peer's volume does not + // throttle another. brake limit 1, two DIFFERENT source IPs → both 200. + let mut state = test_state(pool).await; + state.peer_write_rate_limiter = RateLimiter::new(1, Duration::from_secs(3600)); + state.push_limiter_trust = TrustedProxy::None; + let peer_did = seed_peer(&state).await; + let body = NOTIFY_BODY.replace("PEER_DID", &peer_did); + let router = crate::server::build_router(state); + for ip in ["203.0.113.20:5000", "203.0.113.21:5000"] { + let resp = router + .clone() + .oneshot(unsigned_post("/api/v1/sync/notify", &body, ip)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "distinct IP {ip} must not be throttled" + ); + } + } + + #[sqlx::test] + async fn notify_flood_does_not_exhaust_trigger_bucket(pool: PgPool) { + // Separate buckets: draining peer_write via an unsigned notify flood must + // NOT throttle the signed trigger caller from the same IP. After the notify + // bucket is spent, an unsigned trigger from that IP hits its own (unspent) + // bucket and is rejected by the SIGNATURE gate (401), not the brake (429). + let mut state = test_state(pool).await; + state.peer_write_rate_limiter = RateLimiter::new(1, Duration::from_secs(3600)); + state.sync_trigger_rate_limiter = RateLimiter::new(60, Duration::from_secs(3600)); + state.push_limiter_trust = TrustedProxy::None; + let peer_did = seed_peer(&state).await; + let body = NOTIFY_BODY.replace("PEER_DID", &peer_did); + let router = crate::server::build_router(state); + let ip = "203.0.113.9:5000"; + assert_eq!( + router + .clone() + .oneshot(unsigned_post("/api/v1/sync/notify", &body, ip)) + .await + .unwrap() + .status(), + StatusCode::OK + ); + assert_eq!( + router + .clone() + .oneshot(unsigned_post("/api/v1/sync/notify", &body, ip)) + .await + .unwrap() + .status(), + StatusCode::TOO_MANY_REQUESTS, + "peer_write bucket should now be exhausted for this IP" + ); + let trigger = router + .oneshot(unsigned_post("/api/v1/sync/trigger", "{}", ip)) + .await + .unwrap(); + assert_eq!( + trigger.status(), + StatusCode::UNAUTHORIZED, + "trigger must hit its own bucket (401 from the sig gate), not the drained peer_write bucket (429)" + ); + } + + #[sqlx::test] + async fn announce_still_accepts_unsigned_in_default_mode(pool: PgPool) { + // must-not-over-reach: adding the brake must not tighten announce's + // rolling-upgrade behavior — an unsigned announce with a public URL still + // succeeds (it only gains the brake, which a single request is under). + let state = test_state(pool).await; + let did = Keypair::generate().did().to_string(); + let body = format!(r#"{{"did":"{did}","http_url":"https://peer.example.com"}}"#); + let router = crate::server::build_router(state); + let resp = router + .oneshot(unsigned_post( + "/api/v1/peers/announce", + &body, + "203.0.113.10:5000", + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + } + + // Build a request carrying a REAL RFC 9421 signature (not just an injected + // extension), so it passes require_signature on the full production router. + fn real_signed_trigger(peer: &str) -> Request { + let kp = Keypair::generate(); + let body = b"{}"; + let s = sign_request(&kp, "POST", "/api/v1/sync/trigger", body); + let mut req = Request::builder() + .method(Method::POST) + .uri("/api/v1/sync/trigger") + .header(header::CONTENT_TYPE, "application/json") + .header("content-digest", s.content_digest) + .header("signature-input", s.signature_input) + .header("signature", s.signature) + .body(Body::from(body.to_vec())) + .unwrap(); + req.extensions_mut() + .insert(ConnectInfo(peer.parse::().unwrap())); + req + } + + #[sqlx::test] + async fn sync_trigger_admits_real_signature_through_full_router(pool: PgPool) { + // The positive path through the REAL gate: a validly-signed trigger passes + // require_signature on the full router and reaches the handler (no peers + // seeded → 200), in BOTH config modes. + let state = test_state(pool.clone()).await; + let resp = crate::server::build_router(state) + .oneshot(real_signed_trigger("203.0.113.30:5000")) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "a real signature must reach the handler (default mode)" + ); + + let mut state = test_state(pool).await; + require_signed_peer_writes(&mut state); + let resp = crate::server::build_router(state) + .oneshot(real_signed_trigger("203.0.113.30:5001")) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "a real signature must reach the handler (signed-writes mode)" + ); + } + + #[sqlx::test] + async fn announce_flood_is_throttled(pool: PgPool) { + // The peer_write brake covers /peers/announce too (co-benefit): limit 1, + // first unsigned announce from an IP is 200, the second from that IP 429. + let mut state = test_state(pool).await; + state.peer_write_rate_limiter = RateLimiter::new(1, Duration::from_secs(3600)); + state.push_limiter_trust = TrustedProxy::None; + let did = Keypair::generate().did().to_string(); + let body = format!(r#"{{"did":"{did}","http_url":"https://peer.example.com"}}"#); + let router = crate::server::build_router(state); + let ip = "203.0.113.31:5000"; + assert_eq!( + router + .clone() + .oneshot(unsigned_post("/api/v1/peers/announce", &body, ip)) + .await + .unwrap() + .status(), + StatusCode::OK + ); + assert_eq!( + router + .oneshot(unsigned_post("/api/v1/peers/announce", &body, ip)) + .await + .unwrap() + .status(), + StatusCode::TOO_MANY_REQUESTS, + "announce must be braked by the peer_write limiter" + ); + } + + #[sqlx::test] + async fn sync_trigger_429_body_is_generic_not_push(pool: PgPool) { + // The shared 429 middleware now serves the sync routes, so its body must + // not claim "push" (RED before the message was genericized). + let mut state = test_state(pool).await; + state.sync_trigger_rate_limiter = RateLimiter::new(1, Duration::from_secs(3600)); + state.push_limiter_trust = TrustedProxy::None; + let peer = "203.0.113.32:5000"; + assert!( + state + .sync_trigger_rate_limiter + .check(&peer.parse::().unwrap().ip().to_string()) + .await + ); + let router = crate::server::build_router(state); + let resp = router + .oneshot(unsigned_post("/api/v1/sync/trigger", "{}", peer)) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::TOO_MANY_REQUESTS); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let s = String::from_utf8_lossy(&bytes); + assert!(s.contains("rate limit exceeded"), "429 body: {s:?}"); + assert!( + !s.contains("push"), + "a 429 on a sync route must not say 'push': {s:?}" + ); + } } diff --git a/crates/gitlawb-node/src/auth/mod.rs b/crates/gitlawb-node/src/auth/mod.rs index cb6f37a8..569db8fa 100644 --- a/crates/gitlawb-node/src/auth/mod.rs +++ b/crates/gitlawb-node/src/auth/mod.rs @@ -516,6 +516,8 @@ mod tests { rate_limiter: RateLimiter::new(100, Duration::from_secs(60)), push_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)), push_limiter_trust: crate::rate_limit::TrustedProxy::None, + sync_trigger_rate_limiter: RateLimiter::new(60, Duration::from_secs(3600)), + peer_write_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)), shutdown_tx: tokio::sync::watch::channel(false).0, } } diff --git a/crates/gitlawb-node/src/config.rs b/crates/gitlawb-node/src/config.rs index 3a9a507f..1f0a482e 100644 --- a/crates/gitlawb-node/src/config.rs +++ b/crates/gitlawb-node/src/config.rs @@ -140,6 +140,20 @@ pub struct Config { #[arg(long, env = "GITLAWB_MAX_PACK_BYTES", default_value_t = 2_147_483_648)] pub max_pack_bytes: usize, + /// Per-client-IP rate limit for `POST /api/v1/sync/trigger`, in requests per + /// hour. `/sync/trigger` requires a signature and drives an O(peers) outbound + /// fan-out per call, so it gets a tight bucket. `0` disables. Default: 60. + #[arg(long, env = "GITLAWB_SYNC_TRIGGER_RATE_LIMIT", default_value_t = 60)] + pub sync_trigger_rate_limit: usize, + + /// Per-client-IP rate limit for the peer-write routes (`/peers/announce`, + /// `/sync/notify`), in requests per hour. These accept unsigned requests from + /// known peers and run at higher frequency, so the bucket is generous. Keeping + /// it separate from the trigger bucket stops an unsigned notify flood from + /// draining the signed trigger caller's quota. `0` disables. Default: 600. + #[arg(long, env = "GITLAWB_PEER_WRITE_RATE_LIMIT", default_value_t = 600)] + pub peer_write_rate_limit: usize, + /// Optional address to bind a Prometheus `/metrics` exposition endpoint on. /// Example: `127.0.0.1:9091`. Leave empty (default) to disable. /// Bind to localhost or a private interface — the metrics endpoint is diff --git a/crates/gitlawb-node/src/main.rs b/crates/gitlawb-node/src/main.rs index 1d014859..5e8dffe7 100644 --- a/crates/gitlawb-node/src/main.rs +++ b/crates/gitlawb-node/src/main.rs @@ -213,6 +213,29 @@ async fn main() -> Result<()> { ); tracing::info!(trust = ?push_limiter_trust, push_limit, "push rate limiter 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 + // flood can't drain the signed trigger caller's quota (#82). Bounded key sets + // (the key is a client-influenced IP); 0 disables each. + let sync_trigger_rate_limiter = rate_limit::RateLimiter::new_bounded( + config.sync_trigger_rate_limit, + std::time::Duration::from_secs(3600), + 200_000, + ); + let peer_write_rate_limiter = rate_limit::RateLimiter::new_bounded( + config.peer_write_rate_limit, + std::time::Duration::from_secs(3600), + 200_000, + ); + if config.sync_trigger_rate_limit == 0 { + tracing::warn!( + "GITLAWB_SYNC_TRIGGER_RATE_LIMIT=0 — /sync/trigger IP rate limiting disabled" + ); + } + if config.peer_write_rate_limit == 0 { + tracing::warn!("GITLAWB_PEER_WRITE_RATE_LIMIT=0 — peer-write IP rate limiting disabled"); + } + // Initialize the iCaptcha proof gate (inert unless ICAPTCHA_MODE is set). icaptcha::init().await; @@ -231,6 +254,8 @@ async fn main() -> Result<()> { rate_limiter, push_rate_limiter, push_limiter_trust, + sync_trigger_rate_limiter, + peer_write_rate_limiter, shutdown_tx: shutdown_tx.clone(), }; @@ -292,6 +317,8 @@ async fn main() -> Result<()> { { let rl = state.rate_limiter.clone(); let push_rl = state.push_rate_limiter.clone(); + let sync_trigger_rl = state.sync_trigger_rate_limiter.clone(); + let peer_write_rl = state.peer_write_rate_limiter.clone(); let db = state.db.clone(); let mut shutdown_rx = state.subscribe_shutdown(); tokio::spawn(async move { @@ -300,6 +327,8 @@ async fn main() -> Result<()> { _ = tokio::time::sleep(std::time::Duration::from_secs(300)) => { rl.cleanup().await; push_rl.cleanup().await; + sync_trigger_rl.cleanup().await; + peer_write_rl.cleanup().await; let now = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_secs() as i64) diff --git a/crates/gitlawb-node/src/rate_limit.rs b/crates/gitlawb-node/src/rate_limit.rs index 01784508..40b4bdee 100644 --- a/crates/gitlawb-node/src/rate_limit.rs +++ b/crates/gitlawb-node/src/rate_limit.rs @@ -250,12 +250,14 @@ impl axum::extract::FromRequestParts for PeerAddr { } } -/// The shared 429 response for the push flood brake. +/// 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() -> Response { ( StatusCode::TOO_MANY_REQUESTS, [("retry-after", "60")], - "push rate limit exceeded — try again later", + "rate limit exceeded — try again later", ) .into_response() } @@ -273,7 +275,7 @@ 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 { - tracing::warn!(key = %key, path = %request.uri().path(), "push rate limit exceeded"); + tracing::warn!(key = %key, path = %request.uri().path(), "per-IP rate limit exceeded"); return too_many_requests(); } } diff --git a/crates/gitlawb-node/src/server.rs b/crates/gitlawb-node/src/server.rs index 54939722..c949fb9f 100644 --- a/crates/gitlawb-node/src/server.rs +++ b/crates/gitlawb-node/src/server.rs @@ -277,15 +277,41 @@ pub fn build_router(state: AppState) -> Router { .route("/api/v1/peers", get(peers::list_peers)) .route("/api/v1/peers/{did}/ping", get(peers::ping_peer)); + // /sync/trigger drives an O(peers) outbound fan-out + per-repo enqueue, so it + // ALWAYS requires a signature (both config modes) and carries a tight per-IP + // brake. A signature alone does not cap cost — a did:key farm self-registers + // (INV-10) — so the IP brake is a separate, load-bearing half. The brake is + // outermost (runs before signature verification burns CPU) and is keyed on the + // client IP before any DID is read, so DID rotation cannot bypass it. + let sync_trigger_routes = add_auth_layers( + Router::new().route("/api/v1/sync/trigger", post(peers::trigger_sync)), + state.clone(), + ) + .layer(middleware::from_fn(rate_limit::rate_limit_by_ip)) + .layer(axum::Extension(rate_limit::IpRateLimiter { + limiter: state.sync_trigger_rate_limiter.clone(), + trust: state.push_limiter_trust, + })); + + // announce + notify keep their rolling-upgrade signature behavior (unsigned + // accepted until all peers upgrade), but both reach peer-write side effects — + // notify hits the same enqueue_sync sink as trigger — so they carry a per-IP + // brake too, on a SEPARATE bucket from trigger's, so an unsigned notify flood + // cannot drain the signed trigger caller's quota. let mut peer_write_routes = Router::new() .route("/api/v1/peers/announce", post(peers::announce)) - .route("/api/v1/sync/trigger", post(peers::trigger_sync)) .route("/api/v1/sync/notify", post(peers::notify_sync)); peer_write_routes = if state.config.require_signed_peer_writes { add_auth_layers(peer_write_routes, state.clone()) } else { peer_write_routes.layer(middleware::from_fn(auth::optional_signature)) }; + let peer_write_routes = peer_write_routes + .layer(middleware::from_fn(rate_limit::rate_limit_by_ip)) + .layer(axum::Extension(rate_limit::IpRateLimiter { + limiter: state.peer_write_rate_limiter.clone(), + trust: state.push_limiter_trust, + })); // ── Read routes — open for public repos ─────────────────────────────── let read_routes = Router::new() @@ -434,6 +460,7 @@ pub fn build_router(state: AppState) -> Router { .merge(read_routes) .merge(peer_read_routes) .merge(peer_write_routes) + .merge(sync_trigger_routes) .merge(ipfs_routes) .merge(arweave_routes) .merge(meta_routes) diff --git a/crates/gitlawb-node/src/state.rs b/crates/gitlawb-node/src/state.rs index b9746903..5f235b04 100644 --- a/crates/gitlawb-node/src/state.rs +++ b/crates/gitlawb-node/src/state.rs @@ -57,7 +57,19 @@ pub struct AppState { pub push_rate_limiter: RateLimiter, /// Which forwarded header (if any) the edge is trusted to set, for /// resolving the push limiter's client-IP key. See `GITLAWB_TRUSTED_PROXY`. + /// Node-wide; also keys the two peer-sync limiters below. pub push_limiter_trust: crate::rate_limit::TrustedProxy, + /// Per-client-IP limiter for `POST /api/v1/sync/trigger` (tight). The route + /// requires a signature, but a signature does not cap cost (a did:key farm + /// self-registers), and its per-call cost is an O(peers) fan-out, so the IP + /// brake is a separate, load-bearing half. Its own bucket so an unsigned + /// `/sync/notify` flood cannot drain the signed trigger caller's quota. + pub sync_trigger_rate_limiter: RateLimiter, + /// Per-client-IP limiter for the peer-write routes (`/peers/announce`, + /// `/sync/notify`) (generous). `/sync/notify` reaches the same `enqueue_sync` + /// sink as trigger and accepts unsigned requests from known peers, so it is + /// braked too; each peer's distinct IP gets its own bucket. + pub peer_write_rate_limiter: RateLimiter, /// Process-wide graceful-shutdown signal. Sending `true` causes every /// task that holds a `watch::Receiver` to exit at its next await point. /// Used by: diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 02b443b4..0d389883 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -78,6 +78,8 @@ fn build_state(db: Arc, pool: PgPool) -> AppState { rate_limiter: RateLimiter::new(100, Duration::from_secs(60)), push_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)), push_limiter_trust: crate::rate_limit::TrustedProxy::None, + sync_trigger_rate_limiter: RateLimiter::new(60, Duration::from_secs(3600)), + peer_write_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)), shutdown_tx: tokio::sync::watch::channel(false).0, } } diff --git a/crates/gl/src/sync.rs b/crates/gl/src/sync.rs index f02ecd56..72fb2758 100644 --- a/crates/gl/src/sync.rs +++ b/crates/gl/src/sync.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{Context, Result}; use clap::{Args, Subcommand}; use std::path::PathBuf; @@ -30,15 +30,39 @@ pub enum SyncCmd { pub async fn run(args: SyncArgs) -> Result<()> { match args.cmd { SyncCmd::Trigger => { - let keypair = load_keypair_from_dir(args.dir.as_deref()).ok(); - let client = NodeClient::new(&args.node, keypair); - let resp: serde_json::Value = client - .post("/api/v1/sync/trigger", b"{}") - .await? - .json() - .await?; - let peers = resp["peers_reached"].as_u64().unwrap_or(0); - let enqueued = resp["repos_enqueued"].as_u64().unwrap_or(0); + // /api/v1/sync/trigger always requires a signature, so a missing or + // unreadable identity must fail here, locally, rather than sending an + // unsigned request that can only 401 remotely (matches the other + // signed CLI writes). + let keypair = load_keypair_from_dir(args.dir.as_deref()) + .context("identity not found — run `gl identity new` first")?; + let client = NodeClient::new(&args.node, Some(keypair)); + let resp = client.post("/api/v1/sync/trigger", b"{}").await?; + // The node now requires a signature on this route and rate-limits it, + // so a denial (401/429/…) is expected. Check the status BEFORE parsing: + // otherwise a JSON-ish error body deserializes into a zero-count struct + // and prints a fabricated "✓ sync triggered / 0 peers" success. + let status = resp.status(); + if !status.is_success() { + // Bound the read: a hostile or broken node must not force an + // unbounded allocation just to surface a denial (INV-6, read half). + let raw = read_body_capped(resp, 8 * 1024).await; + let msg = serde_json::from_str::(&raw) + .ok() + .and_then(|v| { + v.get("message") + .or_else(|| v.get("error")) + .and_then(|m| m.as_str()) + .map(str::to_string) + }) + .unwrap_or(raw); + anyhow::bail!( + "sync trigger failed ({status}): {}", + sanitize_node_msg(&msg) + ); + } + let resp: serde_json::Value = resp.json().await?; + let (peers, enqueued) = trigger_counts(&resp); println!("✓ sync triggered"); println!(" peers reached: {peers}"); println!(" repos enqueued: {enqueued}"); @@ -69,3 +93,254 @@ pub async fn run(args: SyncArgs) -> Result<()> { } Ok(()) } + +/// Extract `(peers_reached, repos_enqueued)` from a successful sync-trigger +/// response. Split out so the extraction is unit-testable (missing or malformed +/// fields default to 0 rather than panicking). +fn trigger_counts(resp: &serde_json::Value) -> (u64, u64) { + ( + resp["peers_reached"].as_u64().unwrap_or(0), + resp["repos_enqueued"].as_u64().unwrap_or(0), + ) +} + +/// Read at most `cap` bytes of a response body. Bounds the allocation from a +/// hostile or broken node returning a huge error body — the display is capped +/// separately, but the read itself must not be unbounded (INV-6, read half). +async fn read_body_capped(mut resp: reqwest::Response, cap: usize) -> String { + let mut buf: Vec = Vec::new(); + while buf.len() < cap { + match resp.chunk().await { + Ok(Some(chunk)) => { + let take = (cap - buf.len()).min(chunk.len()); + buf.extend_from_slice(&chunk[..take]); + if take < chunk.len() { + break; // hit the cap mid-chunk + } + } + _ => break, // end of body or read error — return what we have + } + } + String::from_utf8_lossy(&buf).into_owned() +} + +/// Strip terminal-dangerous characters from (and cap the length of) a +/// node-supplied error string before surfacing it. The node a caller talks to +/// could be hostile and embed escape sequences in its error body; those must not +/// reach the terminal verbatim (INV-6). We drop the C0/C1 control bytes (which +/// defangs ANSI/OSC escapes) AND the Unicode bidi/format controls (which +/// `char::is_control` does not cover — they can reorder the displayed line). +fn sanitize_node_msg(s: &str) -> String { + s.chars() + .filter(|c| !c.is_control() && !is_bidi_format(*c)) + .take(200) + .collect() +} + +/// Unicode bidirectional and directional-isolate format characters (category +/// `Cf`). These are not `char::is_control()` (that is category `Cc` only), but a +/// right-to-left override or isolate can visually reorder a terminal line to +/// spoof the error text, so they are stripped alongside the control bytes. +fn is_bidi_format(c: char) -> bool { + matches!(c, + '\u{200E}' | '\u{200F}' | '\u{061C}' // LRM, RLM, ALM + | '\u{202A}'..='\u{202E}' // LRE, RLE, PDF, LRO, RLO + | '\u{2066}'..='\u{2069}' // LRI, RLI, FSI, PDI + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn trigger_args(node: String) -> (SyncArgs, tempfile::TempDir) { + // Seed a real identity so `run` gets past the mandatory-keypair check and + // reaches the status-handling path. The mocks below return a fixed status + // regardless of the signature, so these tests exercise the client's + // status-check-before-parse, not signature verification (that is proved + // server-side). Return the TempDir so the caller keeps it alive. + let dir = tempfile::TempDir::new().unwrap(); + let kp = gitlawb_core::identity::Keypair::generate(); + std::fs::write( + dir.path().join("identity.pem"), + kp.to_pem().unwrap().as_bytes(), + ) + .unwrap(); + let args = SyncArgs { + cmd: SyncCmd::Trigger, + node, + dir: Some(dir.path().to_path_buf()), + }; + (args, dir) + } + + #[tokio::test] + async fn trigger_requires_identity_fails_before_request() { + // Empty identity dir → no keypair. `sync trigger` must fail locally with + // a clear identity error BEFORE issuing any request. The node URL points + // at an unreachable port, so a request attempt would surface a different + // (connection) error; getting the identity error proves we never dialed. + let dir = tempfile::TempDir::new().unwrap(); + let args = SyncArgs { + cmd: SyncCmd::Trigger, + node: "http://127.0.0.1:1".to_string(), + dir: Some(dir.path().to_path_buf()), + }; + let err = run(args).await.unwrap_err(); + assert!( + err.to_string().contains("identity not found"), + "expected a local identity error before any request, got: {err}" + ); + } + + #[tokio::test] + async fn trigger_surfaces_401_as_error_not_fake_success() { + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("POST", "/api/v1/sync/trigger") + .with_status(401) + .with_header("content-type", "application/json") + // Valid JSON: the parse-without-status-check bug deserializes this + // into a zero-count success struct and prints "✓ sync triggered". + .with_body(r#"{"message":"unauthorized"}"#) + .create_async() + .await; + let (args, _dir) = trigger_args(server.url()); + let err = run(args).await.unwrap_err(); + assert!( + err.to_string().contains("401"), + "expected 401 surfaced, got: {err}" + ); + } + + #[tokio::test] + async fn trigger_surfaces_429_as_error() { + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("POST", "/api/v1/sync/trigger") + .with_status(429) + .with_header("content-type", "application/json") + .with_body(r#"{"message":"slow down"}"#) + .create_async() + .await; + let (args, _dir) = trigger_args(server.url()); + let err = run(args).await.unwrap_err(); + assert!( + err.to_string().contains("429"), + "expected 429 surfaced, got: {err}" + ); + } + + #[tokio::test] + async fn trigger_sanitizes_control_chars_in_node_error() { + // A hostile node embeds an ANSI color escape (ESC) and a bell (BEL) in + // the JSON message field. The surfaced error must contain neither raw + // control byte, while keeping the printable text. + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("POST", "/api/v1/sync/trigger") + .with_status(401) + .with_header("content-type", "application/json") + // Valid JSON whose message carries JSON-escaped ESC (\u001b) and + // BEL (\u0007); serde decodes them to real control bytes a naive + // client would print. (The status-check bug fake-successes here.) + .with_body("{\"message\":\"pwned\\u001b[31m\\u0007bad\"}") + .create_async() + .await; + let (args, _dir) = trigger_args(server.url()); + let err = run(args).await.unwrap_err(); + let s = err.to_string(); + assert!(!s.contains('\u{1b}'), "ESC leaked to terminal: {s:?}"); + assert!(!s.contains('\u{07}'), "BEL leaked to terminal: {s:?}"); + assert!( + s.contains("pwned") && s.contains("bad"), + "message text dropped: {s:?}" + ); + } + + #[tokio::test] + async fn trigger_ok_prints_counts() { + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("POST", "/api/v1/sync/trigger") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"peers_reached":2,"repos_enqueued":5}"#) + .create_async() + .await; + let (args, _dir) = trigger_args(server.url()); + run(args).await.unwrap(); + } + + #[test] + fn sanitize_strips_controls_bidi_and_caps_length() { + // C0 (ESC/BEL) and the Cf bidi override (U+202E) are both removed; the + // printable text survives. (Note: a stripped ESC leaves any following + // "[31m" as inert literal text — that is the point, so the input here + // avoids that residue to keep the expectation unambiguous.) + let out = sanitize_node_msg("a\u{1b}\u{07}b\u{202e}c"); + assert!( + !out.chars().any(|c| c.is_control()), + "control char leaked: {out:?}" + ); + assert!( + !out.contains('\u{202e}'), + "RLO bidi override leaked: {out:?}" + ); + assert_eq!(out, "abc"); + // Length is capped at 200 chars regardless of input size. + let long = "x".repeat(250); + assert_eq!(sanitize_node_msg(&long).chars().count(), 200); + } + + #[tokio::test] + async fn trigger_handles_oversized_error_body_without_unbounded_output() { + // A hostile/broken node returns a 2 MB error body. The command must still + // surface the denial with a bounded message, not hang or dump the body. + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("POST", "/api/v1/sync/trigger") + .with_status(401) + .with_body("A".repeat(2_000_000)) + .create_async() + .await; + let (args, _dir) = trigger_args(server.url()); + let err = run(args).await.unwrap_err(); + let s = err.to_string(); + assert!(s.contains("401"), "denial not surfaced: {s:.80?}"); + assert!( + s.len() < 500, + "error message not bounded: {} chars", + s.len() + ); + } + + #[tokio::test] + async fn read_body_capped_bounds_the_read() { + // The read must stop at the cap — a 2 MB body yields at most `cap` bytes, + // not the whole thing (which resp.text() would return). + let mut server = mockito::Server::new_async().await; + let _m = server + .mock("GET", "/big") + .with_status(200) + .with_body("A".repeat(2_000_000)) + .create_async() + .await; + let resp = reqwest::get(format!("{}/big", server.url())).await.unwrap(); + let out = read_body_capped(resp, 8192).await; + assert!(out.len() <= 8192, "read not bounded: {} bytes", out.len()); + assert!(!out.is_empty(), "expected some body"); + } + + #[test] + fn trigger_counts_extracts_both_values() { + let v = serde_json::json!({"peers_reached": 2, "repos_enqueued": 5}); + assert_eq!(trigger_counts(&v), (2, 5)); + // Missing/malformed fields default to 0, never panic. + assert_eq!(trigger_counts(&serde_json::json!({})), (0, 0)); + assert_eq!( + trigger_counts(&serde_json::json!({"peers_reached": "x"})), + (0, 0) + ); + } +} diff --git a/docs/OSS-READINESS-AUDIT.md b/docs/OSS-READINESS-AUDIT.md index 02e2d5ac..0a0bc5f2 100644 --- a/docs/OSS-READINESS-AUDIT.md +++ b/docs/OSS-READINESS-AUDIT.md @@ -120,7 +120,7 @@ Positive: Risks: - Many commands default to the public node; `git-remote-gitlawb` defaults to localhost. This split should be called out in README examples. -- `gl sync trigger` signs the request when a local identity is available and falls back to the legacy unsigned request for live compatibility. +- `gl sync trigger` requires a local identity and always sends a signed request; the `/api/v1/sync/trigger` route rejects unsigned calls, so the command fails locally when no identity is configured. - Several CLI commands parse dynamic JSON responses permissively; good for compatibility, but error messages can hide response-shape regressions. ## CI and release readiness