Skip to content

Commit 2272857

Browse files
author
t
committed
fix(node): cap per-request ipfs walk fan-out and sweep the ipfs limiter (#173)
Two review findings on the /ipfs/{cid} retrieval path. Bound the full-history walk fan-out per request. The ipfs_rate_limiter check fires once per request, but within a single request the object can exist under path-scoped rules in many repos, and each distinct repo pays its own spawn_blocking allowed-set walk (the memo only dedups the same repo). One request could therefore fan out to O(repos) walks for a single rate-limiter token (INV-10). Add MAX_HISTORY_WALKS_PER_REQUEST (16): once that many walks have run, no further walk is spawned for the rest of the request. This also closes jatmn's open P1 ("bound every full-history walk, not just the first one"). The ceiling uses a plain break, not a whole-search break. The budget persists across the outer oid-candidate loop, so a later candidate servable WITHOUT a walk (a commit/tag, or a no-rule public copy) is still served, while any further walk it would need re-trips the guard and is skipped. Breaking the whole search would 404 that free candidate for no amplification benefit. Sweep the ipfs limiter in the periodic cleanup task. ipfs_rate_limiter was the one bounded limiter the 300s cleanup loop never called cleanup() on, so its map sat full of stale source-IP entries until an inline capacity sweep reclaimed them at the 200k cap. The six cleanup calls now live in AppState::sweep_rate_limiters, which the loop drives, so the set is testable and a new limiter has one place to be added. Tests (each RED->GREEN verified by execution): - ipfs_walk_fanout_capped_per_request: cap+1 deniers precede a readable copy; capped -> 404, neutralizing the break -> 200. - ipfs_walk_cap_still_serves_walk_free_candidate: a multi-oid CID whose blob candidate burns the budget still serves its walk-free commit candidate (200); a whole-search break -> 404. - sweep_rate_limiters_includes_ipfs_limiter: drives the sweep and asserts the ipfs limiter's expired entry is evicted; dropping its cleanup() -> entry survives.
1 parent 2fa6449 commit 2272857

5 files changed

Lines changed: 269 additions & 10 deletions

File tree

‎crates/gitlawb-node/src/api/ipfs.rs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,21 @@ use crate::git::visibility_pack::{
3636
use crate::state::AppState;
3737
use crate::visibility::{visibility_check, Decision};
3838

39+
/// Hard ceiling on the number of full-history reachability walks a single
40+
/// `GET /ipfs/{cid}` request may spawn. The per-request `ipfs_rate_limiter`
41+
/// check brakes *repeat* requests, but within one request the object can exist
42+
/// under path-scoped rules in many repos, and each distinct repo pays its own
43+
/// `spawn_blocking` walk (the memo only dedups the same repo). Without a ceiling
44+
/// a single request fans out to O(repos) walks for one rate-limiter token — an
45+
/// amplification sink (INV-10). Once this many walks have run, no further walk is
46+
/// spawned for the rest of the request: any remaining candidate that still needs
47+
/// a walk is skipped (and, with nothing else readable, the request falls through
48+
/// to the opaque 404). The bound is deliberately generous: a legitimate caller
49+
/// serves on the first repo that grants them, so reaching it requires being
50+
/// denied by this many path-scoped repos first, which real traffic effectively
51+
/// never does. Tunable if that assumption stops holding.
52+
pub(crate) const MAX_HISTORY_WALKS_PER_REQUEST: u32 = 16;
53+
3954
/// GET /ipfs/{cid}
4055
///
4156
/// Resolve the CIDv1 to its git oid via the `pinned_cids` table, then search all
@@ -158,6 +173,11 @@ pub async fn get_by_cid(
158173
// reused across candidates.
159174
let mut walk_rate_checked = false;
160175

176+
// Per-request full-history walk budget (INV-10). Declared outside BOTH loops
177+
// so it persists across oid candidates: once spent, the next candidate's walk
178+
// is skipped too, so a multi-oid CID cannot re-open the fan-out per candidate.
179+
let mut walks: u32 = 0;
180+
161181
// Outer loop over every oid the CID maps to (#173): the cid index is
162182
// non-unique, so try each candidate and serve the first readable one rather
163183
// than false-404 when an arbitrary pick is withheld/absent while another is
@@ -208,6 +228,21 @@ pub async fn get_by_cid(
208228
&mut allowed_tree_memo
209229
};
210230
if !memo.contains_key(&repo.id) {
231+
// Per-request fan-out ceiling (INV-10): once this request has
232+
// spawned MAX_HISTORY_WALKS_PER_REQUEST walks, stop paying more
233+
// O(history) walks for this oid candidate. `break` (not
234+
// `break 'outer`) is deliberate: the budget persists across the
235+
// outer oid loop, so a later candidate that can be served WITHOUT
236+
// a walk (a `commit`/`tag`, or a no-rule public copy) is still
237+
// served, while any further walk it would need re-trips this
238+
// guard and is skipped. Breaking the whole search instead would
239+
// 404 that free candidate for no amplification benefit. A repo
240+
// set with no walk-free candidate left simply falls through to
241+
// the opaque 404, exactly as if none were readable.
242+
if walks >= MAX_HISTORY_WALKS_PER_REQUEST {
243+
break;
244+
}
245+
walks += 1;
211246
// First actual walk of this request: brake it on the source IP
212247
// once. A memo hit below never reaches here, so a same-request
213248
// re-check is impossible. When no key can be resolved (e.g. a

‎crates/gitlawb-node/src/main.rs‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -427,22 +427,16 @@ async fn main() -> Result<()> {
427427

428428
// Periodic cleanup of expired rate limit entries + consumed-proof ledger
429429
{
430-
let rl = state.rate_limiter.clone();
431-
let create_ip_rl = state.create_ip_rate_limiter.clone();
432-
let push_rl = state.push_rate_limiter.clone();
433-
let sync_trigger_rl = state.sync_trigger_rate_limiter.clone();
434-
let peer_write_rl = state.peer_write_rate_limiter.clone();
430+
let cleanup_state = state.clone();
435431
let db = state.db.clone();
436432
let mut shutdown_rx = state.subscribe_shutdown();
437433
tokio::spawn(async move {
438434
loop {
439435
tokio::select! {
440436
_ = tokio::time::sleep(std::time::Duration::from_secs(300)) => {
441-
rl.cleanup().await;
442-
create_ip_rl.cleanup().await;
443-
push_rl.cleanup().await;
444-
sync_trigger_rl.cleanup().await;
445-
peer_write_rl.cleanup().await;
437+
// Sweep every per-IP/DID limiter (incl. the ipfs walk brake)
438+
// so bounded maps shed stale keys instead of sitting at cap.
439+
cleanup_state.sweep_rate_limiters().await;
446440
let now = std::time::SystemTime::now()
447441
.duration_since(std::time::UNIX_EPOCH)
448442
.map(|d| d.as_secs() as i64)

‎crates/gitlawb-node/src/rate_limit.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,13 @@ impl RateLimiter {
130130
!w.timestamps.is_empty()
131131
});
132132
}
133+
134+
/// Number of distinct keys currently tracked. Test-only introspection so a
135+
/// cross-module test can assert that a sweep actually evicted expired entries.
136+
#[cfg(test)]
137+
pub(crate) async fn tracked_keys(&self) -> usize {
138+
self.state.lock().await.len()
139+
}
133140
}
134141

135142
pub async fn rate_limit_by_did(request: Request, next: Next) -> Response {

‎crates/gitlawb-node/src/state.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,20 @@ impl AppState {
104104
self.shutdown_tx.subscribe()
105105
}
106106

107+
/// Sweep expired entries from every per-IP/DID rate limiter. Driven by the
108+
/// periodic cleanup task so a bounded limiter's key map sheds stale entries
109+
/// instead of sitting near its cap until an inline capacity sweep reclaims
110+
/// them. Every limiter on the state is swept here; adding a new limiter means
111+
/// adding it to this list.
112+
pub(crate) async fn sweep_rate_limiters(&self) {
113+
self.rate_limiter.cleanup().await;
114+
self.create_ip_rate_limiter.cleanup().await;
115+
self.push_rate_limiter.cleanup().await;
116+
self.ipfs_rate_limiter.cleanup().await;
117+
self.sync_trigger_rate_limiter.cleanup().await;
118+
self.peer_write_rate_limiter.cleanup().await;
119+
}
120+
107121
/// Trigger graceful shutdown. Idempotent — calling more than once
108122
/// has no effect. Returns `true` if this call was the one that
109123
/// flipped the signal.

‎crates/gitlawb-node/src/test_support.rs‎

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2985,6 +2985,215 @@ mod tests {
29852985
);
29862986
}
29872987

2988+
/// INV-10 amplification bound: a single `GET /ipfs/{cid}` must not fan out an
2989+
/// unbounded number of full-history walks. The per-request `ipfs_rate_limiter`
2990+
/// check only brakes REPEAT requests (it fires once per request); within one
2991+
/// request the same object can exist under path-scoped rules in many repos,
2992+
/// each paying its own walk. `MAX_HISTORY_WALKS_PER_REQUEST` caps that fan-out.
2993+
///
2994+
/// Load-bearing witness: a readable public copy (no path rule → served via the
2995+
/// no-walk path, exactly like `ipfs_cid_served_from_public_copy_when_withheld_elsewhere`)
2996+
/// is given the OLDEST `updated_at` so `list_all_repos` (ORDER BY updated_at
2997+
/// DESC) iterates it LAST. Ahead of it sit `cap + 1` path-scoped deniers, each
2998+
/// forcing an allowed-blob walk that denies anon. An UNCAPPED handler walks
2999+
/// every denier and then serves the copy (200); the cap stops spawning walks
3000+
/// after `cap` of them — before the copy is ever reached — and falls through to
3001+
/// the opaque 404. Neutralizing the `break` in `get_by_cid` (e.g. raising the
3002+
/// ceiling) turns this 404 back into a 200: the RED proof that the cap, not some
3003+
/// incidental deny, is what produces the 404.
3004+
#[sqlx::test]
3005+
async fn ipfs_walk_fanout_capped_per_request(pool: PgPool) {
3006+
use crate::db::VisibilityMode;
3007+
use chrono::Utc;
3008+
use gitlawb_core::identity::Keypair;
3009+
3010+
let owner = Keypair::generate();
3011+
let owner_did = owner.did().to_string();
3012+
let slug = owner_did.replace([':', '/'], "_");
3013+
let short = owner_did.split(':').next_back().unwrap().to_string();
3014+
let state = test_state(pool).await;
3015+
3016+
let cap = crate::api::ipfs::MAX_HISTORY_WALKS_PER_REQUEST as usize;
3017+
3018+
// `cap + 1` deniers guarantee the fan-out crosses the ceiling before the
3019+
// readable copy (iterated last) is reached. All bare clones share identical
3020+
// content, so the one secret-BLOB CID resolves to `secret_oid` in every repo.
3021+
let denier_names: Vec<String> = (0..=cap).map(|i| format!("denier{i}")).collect();
3022+
let mut names: Vec<&str> = vec!["readable"];
3023+
names.extend(denier_names.iter().map(|s| s.as_str()));
3024+
let fx = seed_cid_repos(&slug, &short, &names);
3025+
3026+
let readable_bare = std::path::PathBuf::from("/tmp")
3027+
.join(&slug)
3028+
.join("readable.git");
3029+
// The secret BLOB CID drives the path-scoped allowed-blob walk in every
3030+
// denier (the amplification surface) and is served cheaply from the
3031+
// no-rule public copy — the proven serve path.
3032+
let secret_cid = pin_cid_for(&readable_bare, &fx.secret_oid, &state.db).await;
3033+
3034+
// 1) Readable public copy — OLDEST updated_at → iterated LAST. Public with
3035+
// NO visibility rule, so the blob serves via the no-walk path. This is
3036+
// the copy an uncapped fan-out would eventually reach and serve.
3037+
let mut readable = seed_repo(&owner_did, "readable");
3038+
readable.updated_at = Utc::now() - chrono::Duration::seconds(60);
3039+
state
3040+
.db
3041+
.create_repo(&readable)
3042+
.await
3043+
.expect("seed readable copy");
3044+
3045+
// 2) cap+1 deniers with NEWER updated_at → iterated before the copy. Public
3046+
// at "/", but a `/secret/**` Mode B rule with an EMPTY reader list, so an
3047+
// anon blob fetch clears the "/" gate, runs the allowed-blob walk, and is
3048+
// denied (the secret blob is in no one's set) → continue. Each distinct
3049+
// repo.id is its own walk (the memo only dedups the same repo).
3050+
for name in &denier_names {
3051+
let mut denier = seed_repo(&owner_did, name);
3052+
denier.updated_at = Utc::now();
3053+
state.db.create_repo(&denier).await.expect("seed denier");
3054+
state
3055+
.db
3056+
.set_visibility_rule(&denier.id, "/secret/**", VisibilityMode::B, &[], &owner_did)
3057+
.await
3058+
.expect("path rule");
3059+
}
3060+
3061+
// Anon (no peer, no XFF → the IP brake is skipped, so the walk cap is the
3062+
// only thing in play). Capped → 404 before the readable copy is reached.
3063+
let (st, body) = cid_parts(
3064+
cid_router(&state)
3065+
.oneshot(cid_anon(&secret_cid))
3066+
.await
3067+
.unwrap(),
3068+
)
3069+
.await;
3070+
assert_eq!(
3071+
st,
3072+
StatusCode::NOT_FOUND,
3073+
"the per-request walk fan-out must be capped before serving a copy past the cap"
3074+
);
3075+
// Opaque 404 — the withheld blob content must not leak.
3076+
assert!(
3077+
!body.contains("TOP SECRET"),
3078+
"capped 404 must not leak withheld blob content"
3079+
);
3080+
}
3081+
3082+
/// Multi-oid companion to `ipfs_walk_fanout_capped_per_request`: exercises the
3083+
/// outer oid loop and proves the ceiling uses a plain `break`, not a
3084+
/// whole-search break. The walk budget persists across oid candidates (so a
3085+
/// second candidate can't re-open the fan-out), yet a candidate that needs NO
3086+
/// walk is still served after the budget is spent.
3087+
///
3088+
/// One CID is mapped to TWO oids (the non-unique cid index, #173): a withheld
3089+
/// `/secret` blob — walk-triggering, denied to anon in every denier — recorded
3090+
/// FIRST so a seq scan lists it before the root commit, which is walk-free (a
3091+
/// `commit` object clears the "/" gate and serves with no allowed-set walk).
3092+
/// The blob candidate burns the whole walk budget across the deniers; the
3093+
/// commit candidate is then served anyway (200, `x-git-hash` = the commit oid).
3094+
/// Reverting the plain `break` to a whole-search break turns this into a 404
3095+
/// (the RED proof that the ceiling bounds walks without discarding a free
3096+
/// candidate).
3097+
#[sqlx::test]
3098+
async fn ipfs_walk_cap_still_serves_walk_free_candidate(pool: PgPool) {
3099+
use crate::db::VisibilityMode;
3100+
use chrono::Utc;
3101+
use gitlawb_core::identity::Keypair;
3102+
3103+
let owner = Keypair::generate();
3104+
let owner_did = owner.did().to_string();
3105+
let slug = owner_did.replace([':', '/'], "_");
3106+
let short = owner_did.split(':').next_back().unwrap().to_string();
3107+
let state = test_state(pool).await;
3108+
3109+
let cap = crate::api::ipfs::MAX_HISTORY_WALKS_PER_REQUEST as usize;
3110+
3111+
// cap+1 path-scoped deniers, all carrying identical content (same oids).
3112+
let denier_names: Vec<String> = (0..=cap).map(|i| format!("m{i}")).collect();
3113+
let names: Vec<&str> = denier_names.iter().map(|s| s.as_str()).collect();
3114+
let fx = seed_cid_repos(&slug, &short, &names);
3115+
let bare = std::path::PathBuf::from("/tmp").join(&slug).join("m0.git");
3116+
3117+
// ONE cid → TWO oids. The withheld blob is recorded first (seq scan lists it
3118+
// first → tried first → burns the budget); the walk-free commit is second.
3119+
let multi_cid = pin_cid_for(&bare, &fx.secret_oid, &state.db).await;
3120+
state
3121+
.db
3122+
.record_pinned_cid(&fx.commit_oid, &multi_cid)
3123+
.await
3124+
.expect("co-locate the commit oid under the same cid");
3125+
3126+
for name in &denier_names {
3127+
let mut d = seed_repo(&owner_did, name);
3128+
d.updated_at = Utc::now();
3129+
state.db.create_repo(&d).await.expect("seed denier");
3130+
state
3131+
.db
3132+
.set_visibility_rule(&d.id, "/secret/**", VisibilityMode::B, &[], &owner_did)
3133+
.await
3134+
.expect("path rule");
3135+
}
3136+
3137+
// Anon: the blob candidate is denied in every denier (a walk each, capping
3138+
// out); the commit candidate is walk-free and must still be served.
3139+
let resp = cid_router(&state)
3140+
.oneshot(cid_anon(&multi_cid))
3141+
.await
3142+
.unwrap();
3143+
let served_hash = resp
3144+
.headers()
3145+
.get("x-git-hash")
3146+
.and_then(|v| v.to_str().ok())
3147+
.map(str::to_string);
3148+
let (st, body) = cid_parts(resp).await;
3149+
assert_eq!(
3150+
st,
3151+
StatusCode::OK,
3152+
"a walk-free candidate must still be served after the walk budget is spent"
3153+
);
3154+
assert_eq!(
3155+
served_hash.as_deref(),
3156+
Some(fx.commit_oid.as_str()),
3157+
"the served object must be the walk-free commit candidate, not the withheld blob"
3158+
);
3159+
assert!(
3160+
!body.contains("TOP SECRET"),
3161+
"the withheld blob must not leak"
3162+
);
3163+
}
3164+
3165+
/// The periodic cleanup task must sweep the ipfs walk limiter, not only its
3166+
/// five siblings. Drives `AppState::sweep_rate_limiters` — the exact method the
3167+
/// 300s loop calls — and asserts the ipfs limiter's expired entry is evicted.
3168+
/// Dropping `ipfs_rate_limiter.cleanup()` from that method leaves the entry in
3169+
/// place (`tracked_keys` stays 1): the RED proof that the sweep covers it.
3170+
#[sqlx::test]
3171+
async fn sweep_rate_limiters_includes_ipfs_limiter(pool: PgPool) {
3172+
let mut state = test_state(pool).await;
3173+
// Short window so a single recorded hit is already expired at sweep time.
3174+
state.ipfs_rate_limiter = crate::rate_limit::RateLimiter::new(5, Duration::from_millis(50));
3175+
3176+
assert!(
3177+
state.ipfs_rate_limiter.check("1.2.3.4").await,
3178+
"record a hit on the ipfs limiter"
3179+
);
3180+
assert_eq!(
3181+
state.ipfs_rate_limiter.tracked_keys().await,
3182+
1,
3183+
"the source-IP key is tracked before the sweep"
3184+
);
3185+
3186+
// Expire the entry (still mapped — cleanup hasn't run), then sweep.
3187+
tokio::time::sleep(Duration::from_millis(60)).await;
3188+
state.sweep_rate_limiters().await;
3189+
3190+
assert_eq!(
3191+
state.ipfs_rate_limiter.tracked_keys().await,
3192+
0,
3193+
"the periodic sweep must evict the ipfs limiter's expired entries"
3194+
);
3195+
}
3196+
29883197
// ---------------------------------------------------------------------------
29893198
// Issue #120 — repo-scoped read surfaces visibility gate
29903199
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)