Skip to content

Commit df77ce5

Browse files
committed
fix(node): bound GraphQL query cost
1 parent bfc44f9 commit df77ce5

21 files changed

Lines changed: 1773 additions & 122 deletions

‎CONTRIBUTING.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ time once updated.
8181

8282
**Requirements:**
8383
- Rust stable (≥ 1.91) — install via [rustup](https://rustup.rs)
84-
- PostgreSQL — required for the node. Use the bundled `docker-compose.yml` for local dev.
84+
- PostgreSQL 16+ — required for the node (it uses `pg_input_is_valid`, which exists only on PostgreSQL 16 and later). Use the bundled `docker-compose.yml` for local dev.
8585
- Docker (optional, for full-stack local testing)
8686

8787
**Environment variables:**

‎Cargo.lock‎

Lines changed: 12 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ Good today:
5353

5454
- Local or Docker node startup.
5555
- Postgres-backed repo metadata.
56+
- Bounded GraphQL queries with [repository pagination](docs/graphql-pagination.md).
5657
- Bare git repository storage.
5758
- Git smart-HTTP clone/fetch/push.
5859
- RFC 9421-signed writes.

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ use crate::state::AppState;
1111

1212
/// Hard ceiling on rows any ref-update feed returns in one request. Shared by the
1313
/// shared collector's clamp and the per-handler request caps so they can't drift.
14-
const MAX_VISIBLE_REF_UPDATES: i64 = 200;
14+
/// `pub(crate)` so the GraphQL complexity meter can price against the same bound.
15+
pub(crate) const MAX_VISIBLE_REF_UPDATES: i64 = 200;
1516

1617
/// Collect up to `limit` ref-update rows visible to `caller`, newest first,
1718
/// paging past rows the feed gate drops. Filtering after a plain SQL `LIMIT`

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8338,6 +8338,7 @@ mod tests {
83388338
#[tokio::test]
83398339
async fn get_by_cid_per_source_cap_sheds_same_source_admits_other() {
83408340
let mut state = crate::test_support::test_state_lazy();
8341+
state.db.pool().close().await;
83418342
// Global pool has room; the per-source cap is 1.
83428343
state.git_ipfs_walk_semaphore = Arc::new(Semaphore::new(8));
83438344
state.git_ipfs_walk_per_caller = crate::rate_limit::PerCallerConcurrency::new(1, 100);
@@ -8364,16 +8365,23 @@ mod tests {
83648365
"a source at its per-source /ipfs walk cap must shed 503 with global capacity free"
83658366
);
83668367

8368+
let bytes = axum::body::to_bytes(resp.into_body(), 1024).await.unwrap();
8369+
let body: serde_json::Value = serde_json::from_slice(&bytes).unwrap();
8370+
assert_eq!(body["error"], "overloaded");
8371+
83678372
// A DIFFERENT source is NOT shed by the per-source cap: it clears admission and
8368-
// proceeds (then errors on the lazy DB, which is not a 503).
8373+
// proceeds to the closed DB, which has a distinct db_unavailable error code.
83698374
let resp = ipfs_router(state)
83708375
.oneshot(get_cid(&cid, Some(other)))
83718376
.await
83728377
.unwrap();
8373-
assert_ne!(
8374-
resp.status(),
8375-
StatusCode::SERVICE_UNAVAILABLE,
8376-
"a different source must not be shed by the per-source cap"
8378+
assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE);
8379+
let bytes = axum::body::to_bytes(resp.into_body(), 1024).await.unwrap();
8380+
let body: serde_json::Value = serde_json::from_slice(&bytes).unwrap();
8381+
assert_eq!(
8382+
body["error"],
8383+
crate::error::DB_UNAVAILABLE_CODE,
8384+
"a different source must clear admission and reach the closed database"
83778385
);
83788386
}
83798387

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7124,6 +7124,7 @@ mod tests {
71247124
/// A pkt-line receive-pack body carrying one branch-create ref update, so the
71257125
/// handler's post-receive tail resolves a non-empty new-tip set (the delta
71267126
/// scan's git stages run).
7127+
#[cfg(unix)]
71277128
fn ref_update_body(new_sha: &str) -> axum::body::Bytes {
71287129
let line = format!("{ZERO_SHA} {new_sha} refs/heads/main");
71297130
axum::body::Bytes::from(format!("{:04x}{}0000", line.len() + 4, line))
@@ -9581,13 +9582,15 @@ mod tests {
95819582
)
95829583
}
95839584

9585+
#[cfg(unix)]
95849586
fn f2a_log(log: &std::path::Path) -> String {
95859587
std::fs::read_to_string(log).unwrap_or_default()
95869588
}
95879589

95889590
/// Withheld-walk children run so far. `ls-tree` is the walk's signature child
95899591
/// (`blob_paths` lists every reachable commit's tree); the delta scan and the
95909592
/// full-scan fallback use `rev-list` / `cat-file` instead.
9593+
#[cfg(unix)]
95919594
fn f2a_walks(log: &std::path::Path) -> usize {
95929595
f2a_log(log)
95939596
.lines()
@@ -9599,6 +9602,7 @@ mod tests {
95999602
/// the withheld walk actually runs rather than taking the no-rule shortcut).
96009603
/// The repo's on-disk path is passed to the tail directly, so no repo_store or
96019604
/// receive-pack plumbing is involved.
9605+
#[cfg(unix)]
96029606
async fn f2a_state(
96039607
pool: sqlx::PgPool,
96049608
git_bin: &str,
@@ -9630,6 +9634,7 @@ mod tests {
96309634
(state, rec)
96319635
}
96329636

9637+
#[cfg(unix)]
96339638
fn f2a_update(ref_name: &str, new_sha: &str) -> Vec<RefUpdate> {
96349639
vec![RefUpdate {
96359640
old_sha: ZERO_SHA.to_string(),
@@ -9638,6 +9643,7 @@ mod tests {
96389643
}]
96399644
}
96409645

9646+
#[cfg(unix)]
96419647
const F2A_PUSHER: &str = "did:key:z6MkF2aPusherAAAAAAAAAAAAAAAAAAAAAAAAAA";
96429648

96439649
/// Scenario 1 (the finding). A second rapid push to the same repo coalesces
@@ -9711,6 +9717,7 @@ mod tests {
97119717
}
97129718
/// Poll `cond` until it holds, with a bound so a regression fails the test
97139719
/// rather than hanging the suite.
9720+
#[cfg(unix)]
97149721
async fn f2a_wait_for(mut cond: impl FnMut() -> bool, what: &str) {
97159722
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20);
97169723
while !cond() {
@@ -9725,6 +9732,7 @@ mod tests {
97259732
/// A `rev-list --objects` line names the tips a DELTA scan was asked to resolve,
97269733
/// so it attributes that scan to one push's tips. The withheld walk's own
97279734
/// `rev-list --all` / `ls-tree` lines never carry a tip as an argument this way.
9735+
#[cfg(unix)]
97289736
fn f2a_delta_scanned(log: &std::path::Path, tip: &str) -> bool {
97299737
f2a_log(log)
97309738
.lines()
@@ -9871,6 +9879,7 @@ mod tests {
98719879

98729880
/// Mount a Pinata upload endpoint that assigns every object the same CID, and
98739881
/// point the state at it. Returns the server (kept alive by the caller) and CID.
9882+
#[cfg(unix)]
98749883
async fn f2a_pinata(state: &mut AppState) -> (mockito::ServerGuard, String) {
98759884
let cid = "bafyf2acoalescedmapping".to_string();
98769885
let mut server = mockito::Server::new_async().await;
@@ -9890,6 +9899,7 @@ mod tests {
98909899

98919900
/// Poll the branch to CID table until the push's mapping lands (the Pinata
98929901
/// worker is detached), bounded so a regression fails rather than hangs.
9902+
#[cfg(unix)]
98939903
async fn f2a_wait_for_branch_cid(
98949904
db: &crate::db::Db,
98959905
slug: &str,
@@ -9909,6 +9919,7 @@ mod tests {
99099919
}
99109920
}
99119921

9922+
#[cfg(unix)]
99129923
fn f2a_slug(rec: &crate::db::RepoRecord) -> String {
99139924
format!(
99149925
"{}/{}",
@@ -10042,6 +10053,7 @@ mod tests {
1004210053
// and `z6p2fail`), and owner-only push is on by default, so the identity has to
1004310054
// follow the repo each push targets rather than being fixed for both.
1004410055

10056+
#[cfg(unix)]
1004510057
fn p2_push(
1004610058
state: &AppState,
1004710059
owner: &str,
@@ -10060,6 +10072,7 @@ mod tests {
1006010072
)
1006110073
}
1006210074

10075+
#[cfg(unix)]
1006310076
fn p2_logged(log: &std::path::Path, prefix: &str) -> bool {
1006410077
f2a_log(log).lines().any(|l| l.starts_with(prefix))
1006510078
}
@@ -10359,6 +10372,7 @@ mod tests {
1035910372
/// runs exactly one `rev-list --all`, so this counts the walks that were attempted
1036010373
/// (the `ls-tree` counter above cannot: a walk whose enumeration fails never gets
1036110374
/// to `ls-tree`).
10375+
#[cfg(unix)]
1036210376
fn f2b_walk_attempts(log: &std::path::Path) -> usize {
1036310377
f2a_log(log)
1036410378
.lines()

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ use std::time::Duration;
66
use tracing::info;
77
use uuid::Uuid;
88

9+
/// Maximum visible repositories per page, plus one internal look-ahead row.
10+
pub(crate) const MAX_VISIBLE_REPO_PAGE_SIZE: usize = 200;
11+
12+
/// Maximum agent tasks returned per query. Shared by the GraphQL `tasks`
13+
/// complexity meter and the resolver clamp so they can't drift.
14+
pub(crate) const MAX_VISIBLE_TASK_PAGE_SIZE: i64 = 200;
15+
916
// ── Public data types ─────────────────────────────────────────────────────────
1017

1118
#[derive(Debug, Clone, Serialize, Deserialize)]
@@ -1540,6 +1547,73 @@ impl Db {
15401547
Ok(rows.into_iter().map(row_to_repo).collect())
15411548
}
15421549

1550+
/// A bounded, mirror-deduplicated page ordered by owner key and repository
1551+
/// name. Apply root visibility before LIMIT: private rows must not consume
1552+
/// page slots or influence continuation metadata. The root-rule predicate
1553+
/// mirrors `visibility::listable_at_root`; a differential test pins it.
1554+
/// Malformed reader JSON denies that repo to non-owners without failing the page.
1555+
/// Cursors only select a position and never confer read authority.
1556+
///
1557+
/// Performance note: the page orders and keyset-filters on
1558+
/// `(owner_key, name)` under `COLLATE "C"`, but the existing
1559+
/// `idx_repos_owner_key_name` index is built on the database default
1560+
/// collation and cannot serve that ordering, so each page sorts the full
1561+
/// deduped repo set before the keyset filter applies: O(total repos) work
1562+
/// per page. This is accepted (pages are capped at
1563+
/// `MAX_VISIBLE_REPO_PAGE_SIZE` rows over a narrow projection). A
1564+
/// C-collation expression index was deliberately not added in a migration:
1565+
/// versions 27-35 are claimed by other in-flight branches (Gitlawb/node#384
1566+
/// claims 27-35, #464 claims 27-28) and the runner keys applied migrations
1567+
/// on the version integer alone, so a colliding entry would be silently
1568+
/// skipped on whichever side merges second. Revisit once that range lands
1569+
/// or clears.
1570+
pub async fn list_visible_repos_page(
1571+
&self,
1572+
caller: Option<&str>,
1573+
after: Option<(&str, &str)>,
1574+
limit: usize,
1575+
) -> Result<Vec<RepoRecord>> {
1576+
let sql = format!(
1577+
"{}
1578+
SELECT d.id, d.name, d.owner_did, d.description, d.is_public,
1579+
d.default_branch, d.created_at, d.updated_at, d.disk_path,
1580+
d.forked_from, d.machine_id
1581+
FROM deduped d
1582+
LEFT JOIN LATERAL (
1583+
SELECT reader_dids FROM visibility_rules v
1584+
WHERE v.repo_id = d.id AND v.path_glob ~ '^/*([*][*])*$'
1585+
ORDER BY v.path_glob DESC LIMIT 1
1586+
) root_rule ON TRUE
1587+
WHERE (
1588+
($2::text IS NOT NULL AND ({key}) = $2)
1589+
OR CASE WHEN root_rule.reader_dids IS NULL THEN d.is_public
1590+
WHEN NOT pg_input_is_valid(root_rule.reader_dids, 'jsonb') THEN FALSE
1591+
WHEN jsonb_typeof(root_rule.reader_dids::jsonb) = 'array' THEN
1592+
COALESCE(root_rule.reader_dids::jsonb ? $3::text, FALSE)
1593+
AND NOT EXISTS (
1594+
SELECT 1 FROM jsonb_array_elements(root_rule.reader_dids::jsonb) reader
1595+
WHERE jsonb_typeof(reader) <> 'string'
1596+
)
1597+
ELSE FALSE END
1598+
)
1599+
AND ($4::text IS NULL OR (({key}) COLLATE \"C\", d.name COLLATE \"C\") > ($4, $5::text))
1600+
ORDER BY ({key}) COLLATE \"C\", d.name COLLATE \"C\"
1601+
LIMIT $6",
1602+
Self::dedup_cte(),
1603+
key = OWNER_KEY_CASE_SQL,
1604+
);
1605+
let rows = sqlx::query(&sql)
1606+
.bind(None::<&str>)
1607+
.bind(caller.map(normalize_owner_key))
1608+
.bind(caller)
1609+
.bind(after.map(|(owner, _)| normalize_owner_key(owner)))
1610+
.bind(after.map(|(_, name)| name))
1611+
.bind(limit.min(MAX_VISIBLE_REPO_PAGE_SIZE + 1) as i64)
1612+
.fetch_all(&self.pool)
1613+
.await?;
1614+
Ok(rows.into_iter().map(row_to_repo).collect())
1615+
}
1616+
15431617
/// Repos currently quarantined (admitted as mirrors but withheld from every
15441618
/// listing surface). `list_all_repos_deduped` excludes these (its `DEDUP_CTE`
15451619
/// filters `quarantined = FALSE`), so a gate that resolves a slug against the

‎crates/gitlawb-node/src/git/repo_store.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ impl RepoStore {
8383
/// Test-only: every guard from this store parks in `release` right before the
8484
/// `pg_advisory_unlock` await, until `gate` is notified. Dropping the future
8585
/// while it is parked reproduces a client disconnect inside `release`.
86-
#[cfg(test)]
86+
#[cfg(all(test, unix))]
8787
pub fn with_pre_unlock_gate(mut self, gate: Arc<tokio::sync::Notify>) -> Self {
8888
self.pre_unlock_gate = Some(gate);
8989
self
@@ -107,7 +107,7 @@ impl RepoStore {
107107

108108
/// Test-only: how many write guards from this store have reached the Tigris upload
109109
/// site. See [`RepoStore::upload_site_reached`].
110-
#[cfg(test)]
110+
#[cfg(all(test, unix))]
111111
pub fn tigris_upload_site_reached(&self) -> usize {
112112
self.upload_site_reached
113113
.load(std::sync::atomic::Ordering::SeqCst)

0 commit comments

Comments
 (0)