fix(node): close two spam-vector root causes (trust upsert + ungated push) (#152)

* fix(node): close two spam-vector root causes (trust upsert + ungated push)

The June 2026 spam wave kept re-materializing after cleanup because two
paths let an unauthenticated-by-captcha actor keep operating:

1. `update_trust_score` was an upsert. Any authenticated push/issue/PR
   from a DID with no agent row silently INSERTed one with a fresh
   `registered_at`, bypassing the iCaptcha gate on /api/register — so
   deregistered spam DIDs came back the moment they pushed. Make it an
   UPDATE-only no-op for unregistered DIDs; registration is the sole way
   into the agents table. Regression test added.

2. `git-receive-pack` had no rate limit. The per-DID limiter on the
   creation routes can't brake a push flood from a DID farm (one
   throwaway identity per repo), so the node absorbed several pushes/sec
   per IP, each a Tigris round-trip + peer-notify fan-out. Add a per-
   client-IP limiter (Fly-Client-IP / X-Forwarded-For) on the push path,
   default 600/h, `GITLAWB_PUSH_RATE_LIMIT` override, 0 disables. Fails
   open without a proxy header so self-hosted nodes aren't broken.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(node): address PR review — advertisement throttle, trusted-proxy IP key, bounded limiter

Resolves the review findings on the push rate limiter:

P1 — advertisement path bypassed the limiter. A push hits
GET info/refs?service=git-receive-pack first, which calls acquire_fresh()
(a Tigris download); it sat in git_read_routes under only optional_signature,
so the flood was reachable via the cheap GET without ever sending the throttled
POST. Now git_info_refs applies the same per-IP brake before the fresh acquire.

P1 — client-supplied forwarded headers were trusted. The key came from
Fly-Client-IP / the first X-Forwarded-For hop, both client-settable, so a
flooder could rotate the header and never fill a bucket. Replaced with a
TrustedProxy policy (GITLAWB_TRUSTED_PROXY = fly | x-forwarded-for | unset):
trust only the operator's edge header, fall back to the real socket peer
(ConnectInfo) otherwise. Fly config trusts Fly-Client-IP; the AWS/Caddy
compose trusts the rightmost X-Forwarded-For hop.

P2 — unbounded limiter key set / insert-before-reject. Added a max_keys cap
(reject-before-insert, inline eviction when full) so a varied key cannot grow
the map, and a rejected request never allocates. Applied to both the per-IP
and per-DID limiters.

P2 — malformed forwarded header disabled the brake. Empty leading XFF hop
returned None and skipped the limiter; empty Fly-Client-IP returned Some("").
client_key now takes the rightmost XFF hop and always falls back to the peer,
so a malformed header can never turn the limiter off.

P2 — missing tests. Added: middleware 429 path, per-peer isolation, key-cap
eviction/rejection, trusted-proxy header resolution, and an integration test
asserting the receive-pack advertisement is throttled before the acquire.

Also serve with into_make_service_with_connect_info so the peer address is
available. New env: GITLAWB_PUSH_RATE_LIMIT, GITLAWB_TRUSTED_PROXY (documented
in .env.example).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Update infra/aws/compose.yaml.tftpl

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fix(node): amortize the rate limiter's capacity eviction scan

`RateLimiter::check` ran a full O(max_keys) `retain` sweep on every new-key
miss once the map was at capacity — which is exactly the distinct-key-flood
state the cap defends against, so the limiter serialized all traffic behind a
200k-entry scan per request under the mutex.

Gate the inline sweep to run at most once per `sweep_interval` (min of the
window and 1s), tracked by `last_sweep`. Between sweeps a new key is rejected
without scanning; the background `cleanup()` loop still reclaims independently.
The fast path for existing keys is unchanged, the key cap invariant holds
(map never exceeds max_keys, rejected keys never allocate), and post-flood
self-healing stays bounded to one interval. Added a test asserting a burst of
capacity misses does not trigger repeated eviction scans.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Author: 3 people

.env.example

@@ -90,6 +90,21 @@ GITLAWB_PUBLIC_READ=true
 # Maximum git smart-HTTP pack request size, in bytes.
 GITLAWB_MAX_PACK_BYTES=2147483648
 
+# ── Push rate limiting (git-receive-pack flood brake) ─────────────────────
+# Max receive-pack requests (info/refs advertisement + push POST) per client
+# IP per hour. 0 disables. Default 600.
+GITLAWB_PUSH_RATE_LIMIT=600
+
+# 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.
+#   fly     — behind Fly's edge (trust Fly-Client-IP).
+#   x-forwarded-for — behind a single reverse proxy like Caddy/NGINX (trust the
+#             rightmost X-Forwarded-For hop).
+# Only set this when a proxy you control actually fronts the node; trusting a
+# forwarded header on a directly-exposed node lets clients spoof the key.
+GITLAWB_TRUSTED_PROXY=
+
 # ── Sync ─────────────────────────────────────────────────────────────────
 # Enable automatic background sync from known peers
 GITLAWB_AUTO_SYNC=false

crates/gitlawb-node/src/api/repos.rs

@@ -492,6 +492,8 @@ pub async fn git_info_refs(
     State(state): State<AppState>,
     Path((owner, repo)): Path<(String, String)>,
     Query(query): Query<InfoRefsQuery>,
+    crate::rate_limit::PeerAddr(peer): crate::rate_limit::PeerAddr,
+    headers: axum::http::HeaderMap,
     auth: Option<Extension<AuthenticatedDid>>,
 ) -> Result<Response> {
     let name = repo.trim_end_matches(".git");
@@ -533,6 +535,23 @@ pub async fn git_info_refs(
         }
     }
 
+    // Push flood brake on the advertisement phase. A push always hits this
+    // GET first, and for receive-pack it forces a fresh Tigris download below;
+    // throttling only the receive-pack POST would leave the expensive
+    // fresh-acquire reachable unauthenticated and unlimited. Applied before the
+    // acquire so a rejected request does no Tigris work. Same per-IP limiter and
+    // trusted-proxy policy as the POST middleware (shared buckets).
+    if service == "git-receive-pack" {
+        if let Some(key) = crate::rate_limit::client_key(&headers, peer, state.push_limiter_trust) {
+            if !state.push_rate_limiter.check(&key).await {
+                tracing::warn!(repo = %name, key = %key, "receive-pack advertisement rate limited");
+                return Err(AppError::TooManyRequests(
+                    "push rate limit exceeded — try again later".into(),
+                ));
+            }
+        }
+    }
+
     // For receive-pack (push), download the latest from Tigris so the client
     // sees the same refs that acquire_write() will operate on.
     let disk_path = if service == "git-receive-pack" {
@@ -2430,4 +2449,49 @@ mod tests {
             response_non_key.clone_url
         );
     }
+
+    /// The receive-pack *advertisement* (`GET info/refs?service=git-receive-pack`)
+    /// must be throttled by the per-IP push limiter BEFORE it does the fresh
+    /// Tigris acquire — otherwise the flood brake on the POST is bypassable via
+    /// the cheaper unauthenticated GET (PR #152 review P1). Pre-filling the
+    /// bucket makes the assertion deterministic and keeps the test off the
+    /// acquire path entirely.
+    #[sqlx::test]
+    async fn receive_pack_advertisement_is_rate_limited(pool: sqlx::PgPool) {
+        use axum::body::Body;
+        use axum::extract::ConnectInfo;
+        use axum::http::{Method, Request, StatusCode};
+        use std::net::SocketAddr;
+        use std::time::Duration;
+        use tower::ServiceExt;
+
+        let mut state = crate::test_support::test_state(pool).await;
+        // Tiny limit, keyed on the socket peer (no trusted proxy).
+        state.push_rate_limiter = crate::rate_limit::RateLimiter::new(1, Duration::from_secs(60));
+        state.push_limiter_trust = crate::rate_limit::TrustedProxy::None;
+        state
+            .db
+            .upsert_mirror_repo("z6advowner", "adv", "/tmp/adv", None, false)
+            .await
+            .unwrap();
+
+        let peer: SocketAddr = "203.0.113.55:6000".parse().unwrap();
+        // Exhaust this peer's single-request budget up front.
+        assert!(state.push_rate_limiter.check(&peer.ip().to_string()).await);
+
+        let router = crate::server::build_router(state);
+        let mut req = Request::builder()
+            .method(Method::GET)
+            .uri("/z6advowner/adv/info/refs?service=git-receive-pack")
+            .body(Body::empty())
+            .unwrap();
+        req.extensions_mut().insert(ConnectInfo(peer));
+
+        let status = router.oneshot(req).await.unwrap().status();
+        assert_eq!(
+            status,
+            StatusCode::TOO_MANY_REQUESTS,
+            "receive-pack advertisement must be throttled before the Tigris acquire"
+        );
+    }
 }

crates/gitlawb-node/src/auth/mod.rs

@@ -514,6 +514,8 @@ mod tests {
             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)),
+            push_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
+            push_limiter_trust: crate::rate_limit::TrustedProxy::None,
             shutdown_tx: tokio::sync::watch::channel(false).0,
         }
     }

crates/gitlawb-node/src/db/mod.rs

@@ -1372,18 +1372,18 @@ impl Db {
         Ok(row.map(|r| r.get::<f64, _>("trust_score")).unwrap_or(0.0))
     }
 
+    /// Update an EXISTING agent's trust score; a no-op for unregistered DIDs.
+    /// Deliberately never inserts: the only path into the agents table is
+    /// `register_agent`, which sits behind the iCaptcha gate on /api/register.
+    /// This used to be an upsert, which let any authenticated push/issue/PR
+    /// re-create a deregistered DID's row with a fresh `registered_at`,
+    /// bypassing the registration gate entirely.
     pub async fn update_trust_score(&self, agent_did: &str, score: f64) -> Result<()> {
-        let now = Utc::now().to_rfc3339();
-        sqlx::query(
-            "INSERT INTO agents (did, trust_score, capabilities, registered_at)
-             VALUES ($1, $2, '[]', $3)
-             ON CONFLICT(did) DO UPDATE SET trust_score = $2",
-        )
-        .bind(agent_did)
-        .bind(score)
-        .bind(&now)
-        .execute(&self.pool)
-        .await?;
+        sqlx::query("UPDATE agents SET trust_score = $2 WHERE did = $1")
+            .bind(agent_did)
+            .bind(score)
+            .execute(&self.pool)
+            .await?;
         Ok(())
     }
 
@@ -4302,6 +4302,27 @@ mod icaptcha_quarantine_tests {
         let with_stars = db.list_all_repos_deduped_with_stars(None).await.unwrap();
         assert!(with_stars.iter().all(|(r, _)| r.name != "spam"));
     }
+
+    /// `update_trust_score` must never create an agent row. Registration (behind
+    /// the iCaptcha gate) is the only way in; otherwise a push/issue/PR from a
+    /// deregistered DID would silently re-register it and bypass the gate.
+    #[sqlx::test]
+    async fn update_trust_score_never_creates_agent(pool: PgPool) {
+        let db = db(pool).await;
+        let did = "did:key:zNeverRegistered";
+
+        // Unregistered DID: updating its score is a no-op, not an insert.
+        db.update_trust_score(did, 0.9).await.unwrap();
+        assert!(
+            db.get_agent(did).await.unwrap().is_none(),
+            "update_trust_score must not resurrect an unregistered DID"
+        );
+
+        // Once genuinely registered, the score updates in place.
+        db.register_agent(did, &[]).await.unwrap();
+        db.update_trust_score(did, 0.9).await.unwrap();
+        assert_eq!(db.get_trust_score(did).await.unwrap(), 0.9);
+    }
 }
 
 #[cfg(test)]

crates/gitlawb-node/src/error.rs

@@ -29,6 +29,9 @@ pub enum AppError {
     #[error("invalid request: {0}")]
     BadRequest(String),
 
+    #[error("too many requests: {0}")]
+    TooManyRequests(String),
+
     #[error("git error: {0}")]
     Git(String),
 
@@ -65,6 +68,9 @@ impl IntoResponse for AppError {
                 msg.clone(),
             ),
             AppError::BadRequest(msg) => (StatusCode::BAD_REQUEST, "bad_request", msg.clone()),
+            AppError::TooManyRequests(msg) => {
+                (StatusCode::TOO_MANY_REQUESTS, "rate_limited", msg.clone())
+            }
             AppError::Git(msg) => (StatusCode::INTERNAL_SERVER_ERROR, "git_error", msg.clone()),
             AppError::Db(e) => (StatusCode::INTERNAL_SERVER_ERROR, "db_error", e.to_string()),
             AppError::Internal(e) => (

crates/gitlawb-node/src/main.rs

@@ -182,7 +182,36 @@ async fn main() -> Result<()> {
     let repo_store =
         git::repo_store::RepoStore::new(config.repos_dir.clone(), tigris, db.pool().clone());
 
-    let rate_limiter = rate_limit::RateLimiter::new(10, std::time::Duration::from_secs(3600));
+    // Per-DID limiter for the creation endpoints. Keyed on the authenticated
+    // DID (attacker-varied), so bound its key set to cap memory.
+    let rate_limiter =
+        rate_limit::RateLimiter::new_bounded(10, std::time::Duration::from_secs(3600), 200_000);
+
+    // 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::<usize>().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");
+    }
+
+    // Which forwarded header the edge is trusted to set. Default None (trust
+    // nothing, key on the socket peer). Fly nodes set GITLAWB_TRUSTED_PROXY=fly;
+    // a node behind Caddy/NGINX sets it to x-forwarded-for.
+    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");
 
     // Initialize the iCaptcha proof gate (inert unless ICAPTCHA_MODE is set).
     icaptcha::init().await;
@@ -200,6 +229,8 @@ async fn main() -> Result<()> {
         machine_id,
         repo_store,
         rate_limiter,
+        push_rate_limiter,
+        push_limiter_trust,
         shutdown_tx: shutdown_tx.clone(),
     };
 
@@ -260,13 +291,15 @@ async fn main() -> Result<()> {
     // Periodic cleanup of expired rate limit entries + consumed-proof ledger
     {
         let rl = state.rate_limiter.clone();
+        let push_rl = state.push_rate_limiter.clone();
         let db = state.db.clone();
         let mut shutdown_rx = state.subscribe_shutdown();
         tokio::spawn(async move {
             loop {
                 tokio::select! {
                     _ = tokio::time::sleep(std::time::Duration::from_secs(300)) => {
                         rl.cleanup().await;
+                        push_rl.cleanup().await;
                         let now = std::time::SystemTime::now()
                             .duration_since(std::time::UNIX_EPOCH)
                             .map(|d| d.as_secs() as i64)
@@ -391,19 +424,25 @@ async fn main() -> Result<()> {
     let grace = std::time::Duration::from_secs(config.shutdown_grace_secs);
     info!(grace_secs = config.shutdown_grace_secs, "axum server ready");
 
-    let serve_result = axum::serve(listener, router)
-        .with_graceful_shutdown(async move {
-            let mut rx = shutdown_signal_for_axum;
-            // Wait until the watcher flips to true, then return so axum
-            // can begin draining.
-            while !*rx.borrow_and_update() {
-                if rx.changed().await.is_err() {
-                    // Sender dropped — treat as shutdown.
-                    break;
-                }
+    // `into_make_service_with_connect_info` exposes the socket peer address as
+    // `ConnectInfo<SocketAddr>` so the push limiter can key on the real client
+    // when no trusted proxy header applies (see `rate_limit::client_key`).
+    let serve_result = axum::serve(
+        listener,
+        router.into_make_service_with_connect_info::<std::net::SocketAddr>(),
+    )
+    .with_graceful_shutdown(async move {
+        let mut rx = shutdown_signal_for_axum;
+        // Wait until the watcher flips to true, then return so axum
+        // can begin draining.
+        while !*rx.borrow_and_update() {
+            if rx.changed().await.is_err() {
+                // Sender dropped — treat as shutdown.
+                break;
             }
-        })
-        .await;
+        }
+    })
+    .await;
 
     // Server has stopped accepting new connections and drained in-flight
     // requests. Tear the rest of the system down.