Skip to content

Commit bb7cf43

Browse files
committed
fix(node): cap receive-pack ref-update count before per-ref work and intent
1 parent 18a3860 commit bb7cf43

1 file changed

Lines changed: 120 additions & 0 deletions

File tree

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

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2086,6 +2086,29 @@ pub async fn git_receive_pack(
20862086
"parsed ref updates from pack"
20872087
);
20882088

2089+
// ── Ref-count cap (request shape, before ANY per-ref work) ───────────
2090+
// Checked right after the parse and before the owner-push and
2091+
// branch-protection gates: those gates are themselves per-ref work (one
2092+
// `is_branch_protected` SELECT each), and past them the intent insert
2093+
// writes one durable child row per ref. Refusing here bounds all of it to
2094+
// one comparison, leaves no rows behind, and holds no lease or permit —
2095+
// the intent-before-lease ordering below is unchanged. It precedes the
2096+
// owner gate on purpose: this bounds the REQUEST, not who may send it, so
2097+
// an oversized push from anyone is one cheap answer rather than a pile of
2098+
// per-ref lookups first.
2099+
if ref_updates.len() > MAX_REF_UPDATES {
2100+
tracing::warn!(
2101+
repo = %name,
2102+
pusher = %auth.0,
2103+
ref_count = ref_updates.len(),
2104+
"refusing push: ref-update count exceeds MAX_REF_UPDATES"
2105+
);
2106+
return Err(AppError::BadRequest(format!(
2107+
"too many ref updates in one push: {} > {MAX_REF_UPDATES}",
2108+
ref_updates.len()
2109+
)));
2110+
}
2111+
20892112
// ── Owner-only push enforcement (on by default; GITLAWB_ENFORCE_OWNER_PUSH) ──
20902113
// Runs before branch protection on purpose: when enabled, a non-owner is
20912114
// rejected here regardless of whether the target branch is protected, so a
@@ -4205,6 +4228,21 @@ fn ref_is_internal_namespace(ref_name: &str) -> bool {
42054228
ref_name == "refs/gitlawb" || ref_name.starts_with("refs/gitlawb/")
42064229
}
42074230

4231+
/// Maximum ref commands accepted in one receive-pack body.
4232+
///
4233+
/// `parse_ref_updates` is unbounded, and every entry costs a per-ref
4234+
/// branch-protection SELECT plus a durable child row that rides the aggregate
4235+
/// into reconcile/quarantine scans until retention. One body (`max_pack_bytes`
4236+
/// allows gigabytes) could otherwise turn a single request into tens of
4237+
/// thousands of pre-lease DB operations and an unbounded durable row set for
4238+
/// one signer. Sized well above a legitimate mirror push (branches + tags of
4239+
/// any realistic repo) while keeping the worst-case pre-intent work at ~8k
4240+
/// indexed SELECTs and ~8k child rows — comfortably inside the 30s
4241+
/// intent-write guard. A repo needing to move more than this in one push
4242+
/// pushes in batches. Pinned by
4243+
/// `receive_pack_refuses_a_ref_update_count_over_the_cap`.
4244+
const MAX_REF_UPDATES: usize = 8192;
4245+
42084246
/// `Clone` so `git_receive_pack` can hand the parsed updates to the detached
42094247
/// replication tail at the durability boundary while the certificate and webhook
42104248
/// loops below still iterate their own copy (#174 U5).
@@ -8481,6 +8519,88 @@ mod tests {
84818519
axum::body::Bytes::from(format!("{:04x}{}0000", line.len() + 4, line))
84828520
}
84838521

8522+
/// Build a receive-pack body carrying `count` distinct ref-update commands,
8523+
/// framed like [`ref_update_body`] (one pkt-line each, then the flush).
8524+
fn ref_update_body_many(count: usize) -> axum::body::Bytes {
8525+
let mut s = String::new();
8526+
for i in 0..count {
8527+
let line = format!("{ZERO_SHA} {:040x} refs/heads/branch-{i}", i + 1);
8528+
s.push_str(&format!("{:04x}{}", line.len() + 4, line));
8529+
}
8530+
s.push_str("0000");
8531+
axum::body::Bytes::from(s)
8532+
}
8533+
8534+
// The ref-count cap refuses BEFORE the owner-push and branch-protection
8535+
// gates and BEFORE the durable intent insert: one oversized body cannot
8536+
// become thousands of per-ref `is_branch_protected` SELECTs or a durable
8537+
// row set that rides reconcile/quarantine scans until retention. The
8538+
// signer here is NOT the owner, so if the cap were removed — or moved
8539+
// below the owner gate — the answer would become Forbidden instead:
8540+
// pinning both the refusal and its placement. Goes RED without
8541+
// `MAX_REF_UPDATES`.
8542+
#[sqlx::test]
8543+
async fn receive_pack_refuses_a_ref_update_count_over_the_cap(pool: sqlx::PgPool) {
8544+
use axum::extract::{Path, State};
8545+
use axum::Extension;
8546+
use std::net::SocketAddr;
8547+
8548+
let owner = "z6refcapowner";
8549+
let name = "rc1";
8550+
let state =
8551+
crate::test_support::test_state_with(pool.clone(), |cfg| cfg.enforce_owner_push = true)
8552+
.await;
8553+
state
8554+
.db
8555+
.upsert_mirror_repo(owner, name, "/tmp/z6refcapowner-rc1", None, false)
8556+
.await
8557+
.unwrap();
8558+
let repo_id = state
8559+
.db
8560+
.get_repo(owner, name)
8561+
.await
8562+
.unwrap()
8563+
.expect("mirror row exists")
8564+
.id;
8565+
8566+
let pusher = "did:key:z6refcapother";
8567+
let peer: SocketAddr = "203.0.113.92:5000".parse().unwrap();
8568+
let result = git_receive_pack(
8569+
State(state.clone()),
8570+
Path((owner.to_string(), name.to_string())),
8571+
Extension(crate::auth::AuthenticatedDid(pusher.to_string())),
8572+
crate::rate_limit::PeerAddr(Some(peer)),
8573+
axum::http::HeaderMap::new(),
8574+
ref_update_body_many(MAX_REF_UPDATES + 1),
8575+
)
8576+
.await;
8577+
match result {
8578+
Err(AppError::BadRequest(msg)) => assert!(
8579+
msg.contains("too many ref updates"),
8580+
"the refusal names the cap; got: {msg}"
8581+
),
8582+
Err(other) => panic!("over-cap push must refuse with BadRequest/400; got {other:?}"),
8583+
Ok(_) => panic!("over-cap push must refuse, not serve"),
8584+
}
8585+
8586+
// No rows at all: the refusal precedes the durable intent insert, so
8587+
// nothing rides retention, reconcile, or quarantine scans.
8588+
let (req_count,): (i64,) =
8589+
sqlx::query_as("SELECT COUNT(*) FROM receive_pack_requests WHERE repo_id = $1")
8590+
.bind(&repo_id)
8591+
.fetch_one(state.db.pool())
8592+
.await
8593+
.unwrap();
8594+
assert_eq!(req_count, 0, "no intent aggregate for a refused push");
8595+
let (child_count,): (i64,) =
8596+
sqlx::query_as("SELECT COUNT(*) FROM pending_ref_transitions WHERE repo_id = $1")
8597+
.bind(&repo_id)
8598+
.fetch_one(state.db.pool())
8599+
.await
8600+
.unwrap();
8601+
assert_eq!(child_count, 0, "no child rows for a refused push");
8602+
}
8603+
84848604
/// Absent report-status with exit zero is indeterminate, not proof.
84858605
/// A capability-free client omits `report-status`; Git then emits zero
84868606
/// result bytes even for rejected commands. The handler must leave every

0 commit comments

Comments
 (0)