Follow-up to #174 (part of #62). Not a regression: the served-git concurrency cap #174 added is a net improvement over unbounded PID/thread exhaustion. This is a residual fairness gap in that cap.
Gap
The cap is a single global tokio::sync::Semaphore (state.git_semaphore) shared by all three served-git handlers, with no per-caller or per-class sub-limit. git_info_refs and git_upload_pack are anonymous-reachable (auth: Option<Extension<AuthenticatedDid>>) and acquire a permit at the top of the handler, before any auth/authorization. git_receive_pack (authenticated push) draws from the same pool.
So sustained anonymous load (enough concurrent info/refs or upload-pack to hold all permits, default GITLAWB_MAX_CONCURRENT_GIT_OPS=128) sheds authenticated pushes with a 503. An unauthenticated caller can degrade or deny authenticated writes across the auth boundary.
Reproduction (executed)
Capacity forced to 1 to make the contention deterministic; one held permit represents an in-flight anon op (git_permit does sem.clone().try_acquire_owned(), and the anon handlers call it):
| Scenario |
Status |
Retry-After |
anon info/refs, slot held |
503 |
(anon draws from the same pool) |
authenticated git-receive-pack, anon holds the slot |
503 |
Some("1") (the Overloaded cap shed) |
authenticated git-receive-pack, slot freed |
500 |
None (reached the DB, past the cap gate) |
The Overloaded shed of the authenticated push happens only while the anon op holds the slot; freeing it lets the same push through the cap gate. Counterfactual confirmed, so the anon-held slot is the cause, not a coincident 503.
Impact
Permissionless, unauthenticated callers can deny service to authenticated writes by occupying the shared pool. It is graceful (a retryable 503, not a crash or a leak), but it crosses the auth boundary: low-privilege load starves high-privilege operations.
This compounds with the gap already documented on #174: info/refs and the withheld-blob (upload_pack_excluding) path are not duration-bounded and do not reap their git child on client disconnect, so a hung anonymous op holds its slot until it exits, making the slots cheap to occupy and hold.
Directions (not prescriptive)
- Reserve capacity for authenticated writes: a separate (smaller) semaphore for
receive-pack, or a reserved fraction of the pool the read paths can't consume.
- A per-caller / per-origin sub-limit so no single source can hold all permits.
- Class-based pools (anon read vs authenticated write) sized independently.
Any of these wants a handler-layer regression test in the shape of the reproduction above (anon-held pool must not shed an authenticated push).
Follow-up to #174 (part of #62). Not a regression: the served-git concurrency cap #174 added is a net improvement over unbounded PID/thread exhaustion. This is a residual fairness gap in that cap.
Gap
The cap is a single global
tokio::sync::Semaphore(state.git_semaphore) shared by all three served-git handlers, with no per-caller or per-class sub-limit.git_info_refsandgit_upload_packare anonymous-reachable (auth: Option<Extension<AuthenticatedDid>>) and acquire a permit at the top of the handler, before any auth/authorization.git_receive_pack(authenticated push) draws from the same pool.So sustained anonymous load (enough concurrent info/refs or upload-pack to hold all permits, default
GITLAWB_MAX_CONCURRENT_GIT_OPS=128) sheds authenticated pushes with a 503. An unauthenticated caller can degrade or deny authenticated writes across the auth boundary.Reproduction (executed)
Capacity forced to 1 to make the contention deterministic; one held permit represents an in-flight anon op (
git_permitdoessem.clone().try_acquire_owned(), and the anon handlers call it):info/refs, slot heldgit-receive-pack, anon holds the slotSome("1")(the Overloaded cap shed)git-receive-pack, slot freedNone(reached the DB, past the cap gate)The Overloaded shed of the authenticated push happens only while the anon op holds the slot; freeing it lets the same push through the cap gate. Counterfactual confirmed, so the anon-held slot is the cause, not a coincident 503.
Impact
Permissionless, unauthenticated callers can deny service to authenticated writes by occupying the shared pool. It is graceful (a retryable 503, not a crash or a leak), but it crosses the auth boundary: low-privilege load starves high-privilege operations.
This compounds with the gap already documented on #174:
info/refsand the withheld-blob (upload_pack_excluding) path are not duration-bounded and do not reap their git child on client disconnect, so a hung anonymous op holds its slot until it exits, making the slots cheap to occupy and hold.Directions (not prescriptive)
receive-pack, or a reserved fraction of the pool the read paths can't consume.Any of these wants a handler-layer regression test in the shape of the reproduction above (anon-held pool must not shed an authenticated push).