Skip to content

Commit 2434dde

Browse files
author
t
committed
fix(node,git): isolate authed pushes in a dedicated write pool (#62)
git-receive-pack now draws from a separate git_write_semaphore (max_concurrent_git_pushes) instead of the shared pool, so a flood of anonymous reads can no longer shed an authenticated push at admission (jatmn P1a). The shared field is renamed git_read_semaphore and continues to gate upload-pack and both info/refs advertisements. The write permit stays above acquire_write so it precedes the Tigris fresh-acquire (INV-10). Handler-layer tests: write-pool shed (503), and a cross-boundary proof that an exhausted read pool does NOT shed a push; both mutation-checked (routing receive-pack back to the read pool flips each RED). 497 tests pass. Part of #174.
1 parent db4de84 commit 2434dde

5 files changed

Lines changed: 65 additions & 16 deletions

File tree

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ pub async fn git_info_refs(
509509
) -> Result<Response> {
510510
// Shed with a 503 before spawning git when the concurrency cap is saturated;
511511
// held for the whole op (incl. the smart_http call), released on return.
512-
let _permit = git_permit(&state.git_semaphore)?;
512+
let _permit = git_permit(&state.git_read_semaphore)?;
513513
let name = smart_http_repo_name(&repo)?;
514514
tracing::info!(owner = %owner, repo = %name, "info/refs request");
515515
let record = state
@@ -637,7 +637,7 @@ pub async fn git_upload_pack(
637637
) -> Result<Response> {
638638
// Shed with a 503 before spawning git when the concurrency cap is saturated;
639639
// held for the whole op (incl. the smart_http call), released on return.
640-
let _permit = git_permit(&state.git_semaphore)?;
640+
let _permit = git_permit(&state.git_read_semaphore)?;
641641
let name = smart_http_repo_name(&repo)?;
642642
let record = state
643643
.db
@@ -876,9 +876,11 @@ pub async fn git_receive_pack(
876876
body: Bytes,
877877
) -> Result<Response> {
878878
// Shed with a 503 before spawning git when the concurrency cap is saturated.
879-
// Acquired at the very top so it wraps the write-guard below; held for the
880-
// whole op (incl. the smart_http call), released on return.
881-
let _permit = git_permit(&state.git_semaphore)?;
879+
// Pushes draw from the dedicated WRITE pool, separate from reads, so a flood of
880+
// anonymous reads cannot shed an authenticated push (#174). Acquired at the very
881+
// top so it wraps the write-guard below (and precedes the Tigris acquire_write,
882+
// bounding concurrent fresh acquires — INV-10); held for the whole op.
883+
let _permit = git_permit(&state.git_write_semaphore)?;
882884
let name = smart_http_repo_name(&repo)?;
883885
tracing::info!(owner = %owner, repo = %name, "receive-pack request");
884886
let record = state

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,8 @@ mod tests {
520520
sync_trigger_rate_limiter: RateLimiter::new(60, Duration::from_secs(3600)),
521521
peer_write_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
522522
shutdown_tx: tokio::sync::watch::channel(false).0,
523-
git_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
523+
git_read_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
524+
git_write_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
524525
}
525526
}
526527

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,10 @@ async fn main() -> Result<()> {
378378
sync_trigger_rate_limiter,
379379
peer_write_rate_limiter,
380380
shutdown_tx: shutdown_tx.clone(),
381-
git_semaphore: Arc::new(tokio::sync::Semaphore::new(config.max_concurrent_git_ops)),
381+
git_read_semaphore: Arc::new(tokio::sync::Semaphore::new(config.max_concurrent_git_ops)),
382+
git_write_semaphore: Arc::new(tokio::sync::Semaphore::new(
383+
config.max_concurrent_git_pushes,
384+
)),
382385
};
383386

384387
// Periodic peer-count poll for the metrics gauge. If p2p is disabled

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,16 @@ pub struct AppState {
8888
/// * the libp2p swarm task
8989
/// * the gossip, sync, operator heartbeat, and rate-limit cleanup loops
9090
pub shutdown_tx: tokio::sync::watch::Sender<bool>,
91-
/// Bounds concurrent served git operations. A handler acquires a permit
92-
/// before spawning git and holds it for the op; when none are free the
93-
/// request is shed with a 503 rather than exhausting the PID/thread table.
94-
pub git_semaphore: Arc<tokio::sync::Semaphore>,
91+
/// Bounds concurrent served git READ operations (upload-pack + both info/refs
92+
/// advertisements). A read handler acquires a permit before spawning git and
93+
/// holds it for the op; when none are free the request is shed with a 503.
94+
/// Writes draw from `git_write_semaphore` so a read flood cannot shed an
95+
/// authenticated push at admission (#174).
96+
pub git_read_semaphore: Arc<tokio::sync::Semaphore>,
97+
/// Bounds concurrent `git-receive-pack` (push) operations, a pool separate
98+
/// from `git_read_semaphore` so anonymous reads can never shed an authenticated
99+
/// push (#174). Sized by `max_concurrent_git_pushes`.
100+
pub git_write_semaphore: Arc<tokio::sync::Semaphore>,
95101
}
96102

97103
impl AppState {

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

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ fn build_state(db: Arc<crate::db::Db>, pool: PgPool) -> AppState {
8383
peer_write_rate_limiter: RateLimiter::new(600, Duration::from_secs(3600)),
8484
shutdown_tx: tokio::sync::watch::channel(false).0,
8585
// Generous — no test drives the handler-level shed (git_permit is unit-tested).
86-
git_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
86+
git_read_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
87+
git_write_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
8788
}
8889
}
8990

@@ -199,7 +200,7 @@ mod tests {
199200
#[tokio::test]
200201
async fn git_info_refs_sheds_with_503_when_semaphore_exhausted() {
201202
let mut state = test_state_lazy();
202-
state.git_semaphore = Arc::new(tokio::sync::Semaphore::new(0));
203+
state.git_read_semaphore = Arc::new(tokio::sync::Semaphore::new(0));
203204

204205
let router = Router::new()
205206
.route(
@@ -235,7 +236,7 @@ mod tests {
235236
#[tokio::test]
236237
async fn git_upload_pack_sheds_with_503_when_semaphore_exhausted() {
237238
let mut state = test_state_lazy();
238-
state.git_semaphore = Arc::new(tokio::sync::Semaphore::new(0));
239+
state.git_read_semaphore = Arc::new(tokio::sync::Semaphore::new(0));
239240

240241
let router = Router::new()
241242
.route(
@@ -273,7 +274,7 @@ mod tests {
273274
#[tokio::test]
274275
async fn git_receive_pack_sheds_with_503_when_semaphore_exhausted() {
275276
let mut state = test_state_lazy();
276-
state.git_semaphore = Arc::new(tokio::sync::Semaphore::new(0));
277+
state.git_write_semaphore = Arc::new(tokio::sync::Semaphore::new(0));
277278

278279
let router = Router::new()
279280
.route(
@@ -295,7 +296,7 @@ mod tests {
295296
assert_eq!(
296297
resp.status(),
297298
StatusCode::SERVICE_UNAVAILABLE,
298-
"an exhausted git semaphore must shed git-receive-pack with 503 before touching the DB"
299+
"an exhausted write pool must shed git-receive-pack with 503 before touching the DB"
299300
);
300301
assert_eq!(
301302
resp.headers()
@@ -306,6 +307,42 @@ mod tests {
306307
);
307308
}
308309

310+
/// #174 (SC1, load-bearing): a saturated READ pool must NOT shed an
311+
/// authenticated push — the write pool is a separate budget. Read pool at zero,
312+
/// write pool with capacity: the push proceeds PAST admission (it then errors on
313+
/// the placeholder DB, but crucially it is not a 503). Route git-receive-pack
314+
/// back to the read pool and this goes red — that is the isolation proof.
315+
#[tokio::test]
316+
async fn git_receive_pack_not_shed_by_exhausted_read_pool() {
317+
let mut state = test_state_lazy();
318+
// Read pool exhausted as if a flood of anonymous clones held every slot.
319+
state.git_read_semaphore = Arc::new(tokio::sync::Semaphore::new(0));
320+
// Write pool keeps its default capacity from test_state_lazy.
321+
322+
let router = Router::new()
323+
.route(
324+
"/{owner}/{repo}/git-receive-pack",
325+
axum::routing::post(crate::api::repos::git_receive_pack),
326+
)
327+
.with_state(state);
328+
let owner = "did:key:zRECVCROSSBOUNDARYAAAAAAAAAAAAAAAAAAAAA";
329+
let resp = router
330+
.oneshot(signed_request_as(
331+
owner,
332+
Method::POST,
333+
"/alice/repo.git/git-receive-pack",
334+
Body::from(&b"0000"[..]),
335+
))
336+
.await
337+
.unwrap();
338+
339+
assert_ne!(
340+
resp.status(),
341+
StatusCode::SERVICE_UNAVAILABLE,
342+
"an exhausted READ pool must not shed a push — the write pool is a separate budget (#174)"
343+
);
344+
}
345+
309346
/// N7: merge_pr is owner-only. A non-owner is rejected by require_repo_owner
310347
/// before any git work (so no on-disk repo is needed for the rejection).
311348
#[sqlx::test]

0 commit comments

Comments
 (0)