From d910ea5996c3d322d0c4095cfcaa15d5f4822ffb Mon Sep 17 00:00:00 2001 From: euxaristia Date: Mon, 7 Sep 2026 02:39:49 -0400 Subject: [PATCH 01/20] fix(node): Bound GraphQL query cost. --- crates/gitlawb-node/src/graphql/mod.rs | 120 ++++++++++++++++++++++- crates/gitlawb-node/src/graphql/query.rs | 6 ++ 2 files changed, 124 insertions(+), 2 deletions(-) diff --git a/crates/gitlawb-node/src/graphql/mod.rs b/crates/gitlawb-node/src/graphql/mod.rs index 181bcc22..31d44494 100644 --- a/crates/gitlawb-node/src/graphql/mod.rs +++ b/crates/gitlawb-node/src/graphql/mod.rs @@ -3,7 +3,7 @@ pub mod query; pub mod subscription; pub mod types; -use async_graphql::Schema; +use async_graphql::{Schema, SchemaBuilder}; use std::sync::Arc; use crate::db::Db; @@ -14,6 +14,21 @@ use subscription::SubscriptionRoot; pub type GitlawbSchema = Schema; +// Keep ordinary schema discovery and application queries usable while +// bounding validation work and the number of resolver selections per request. +const GRAPHQL_MAX_COMPLEXITY: usize = 400; +// The current public schema is shallow; this leaves headroom for composed +// clients without allowing recursively nested documents to grow unchecked. +const GRAPHQL_MAX_DEPTH: usize = 12; + +fn apply_query_limits( + builder: SchemaBuilder, +) -> SchemaBuilder { + builder + .limit_complexity(GRAPHQL_MAX_COMPLEXITY) + .limit_depth(GRAPHQL_MAX_DEPTH) +} + /// Client-facing message for GraphQL resolver failures that wrap a real /// `sqlx::Error`. The real error is logged server-side; never put sqlx/Postgres /// detail in the GraphQL `errors` array (#250). @@ -81,7 +96,7 @@ pub fn build_schema( ref_update_tx: tokio::sync::broadcast::Sender, task_event_tx: tokio::sync::broadcast::Sender, ) -> GitlawbSchema { - Schema::build(QueryRoot, MutationRoot, SubscriptionRoot) + apply_query_limits(Schema::build(QueryRoot, MutationRoot, SubscriptionRoot)) .data(db) .data(ref_update_tx) .data(task_event_tx) @@ -91,6 +106,8 @@ pub fn build_schema( #[cfg(test)] mod tests { use super::*; + use async_graphql::{EmptyMutation, EmptySubscription, Object, Value}; + use std::sync::atomic::{AtomicUsize, Ordering}; #[test] fn graphql_db_err_opaques_sqlx_chain() { @@ -160,6 +177,105 @@ mod tests { assert!(!err.message.contains("failed to open")); } + #[tokio::test] + async fn expensive_root_aliases_are_rejected_before_database_access() { + // QueryRoot expects a Db in the schema data. Deliberately omit it: if + // validation ever lets this document reach a resolver, the test fails. + let schema = + apply_query_limits(Schema::build(QueryRoot, EmptyMutation, EmptySubscription)).finish(); + let fields = (0..8) + .map(|n| format!("r{n}: repos {{ name }}")) + .collect::>() + .join(" "); + + let response = schema.execute(format!("{{ {fields} }}")).await; + + assert_eq!(response.data, Value::Null); + assert_eq!(response.errors.len(), 1); + assert_eq!(response.errors[0].message, "Query is too complex."); + } + + #[tokio::test] + async fn ordinary_schema_introspection_remains_available() { + let schema = + apply_query_limits(Schema::build(QueryRoot, EmptyMutation, EmptySubscription)).finish(); + let response = schema + .execute( + r#" + query IntrospectionQuery { + __schema { + queryType { + name + fields { + name + type { + kind + name + ofType { kind name } + } + } + } + } + } + "#, + ) + .await; + + assert!( + response.errors.is_empty(), + "graphql errors: {:?}", + response.errors + ); + assert_ne!(response.data, Value::Null); + } + + #[derive(Clone, Copy)] + struct Nested; + + #[Object] + impl Nested { + async fn child(&self) -> Nested { + Nested + } + + async fn value(&self) -> i32 { + 1 + } + } + + struct CountingQuery(Arc); + + #[Object] + impl CountingQuery { + async fn nested(&self) -> Nested { + self.0.fetch_add(1, Ordering::Relaxed); + Nested + } + } + + #[tokio::test] + async fn deeply_nested_queries_are_rejected_before_resolver_execution() { + let calls = Arc::new(AtomicUsize::new(0)); + let schema = apply_query_limits(Schema::build( + CountingQuery(Arc::clone(&calls)), + EmptyMutation, + EmptySubscription, + )) + .finish(); + let selection = (0..GRAPHQL_MAX_DEPTH).fold("value".to_string(), |selection, _| { + format!("child {{ {selection} }}") + }); + + let response = schema + .execute(format!("{{ nested {{ {selection} }} }}")) + .await; + + assert_eq!(response.data, Value::Null); + assert_eq!(response.errors.len(), 1); + assert_eq!(response.errors[0].message, "Query is nested too deep."); + assert_eq!(calls.load(Ordering::Relaxed), 0); + } + /// Every `.map_err(` in the GraphQL query/mutation resolvers must route /// through the opaque helpers, or discard the error (`|_|`). Same source- /// scrape pattern as `api::authz_guard` (#255 review). diff --git a/crates/gitlawb-node/src/graphql/query.rs b/crates/gitlawb-node/src/graphql/query.rs index 84d7540b..eac2ec79 100644 --- a/crates/gitlawb-node/src/graphql/query.rs +++ b/crates/gitlawb-node/src/graphql/query.rs @@ -9,6 +9,9 @@ pub struct QueryRoot; #[Object] impl QueryRoot { + // DB-backed roots carry a base cost so aliases consume the request budget + // even when each alias selects only one inexpensive response field. + #[graphql(complexity = "50 + child_complexity")] async fn repos(&self, ctx: &Context<'_>) -> Result> { let db = ctx.data_unchecked::>(); let repos = db @@ -45,6 +48,7 @@ impl QueryRoot { .collect()) } + #[graphql(complexity = "50 + child_complexity")] async fn ref_updates( &self, ctx: &Context<'_>, @@ -105,6 +109,7 @@ impl QueryRoot { Ok(resolved) } + #[graphql(complexity = "50 + child_complexity")] async fn tasks( &self, ctx: &Context<'_>, @@ -128,6 +133,7 @@ impl QueryRoot { Ok(tasks.into_iter().map(AgentTaskType::from).collect()) } + #[graphql(complexity = "50 + child_complexity")] async fn task(&self, ctx: &Context<'_>, id: String) -> Result> { let db = ctx.data_unchecked::>(); let t = db From 1d1ec25a93cc19eb47bfb8d6576be9de5a8529a4 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Mon, 7 Sep 2026 02:50:18 -0400 Subject: [PATCH 02/20] test(node): Pin the GraphQL depth boundary. --- crates/gitlawb-node/src/graphql/mod.rs | 30 +++++++++++++++++--------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/crates/gitlawb-node/src/graphql/mod.rs b/crates/gitlawb-node/src/graphql/mod.rs index 31d44494..d96219a0 100644 --- a/crates/gitlawb-node/src/graphql/mod.rs +++ b/crates/gitlawb-node/src/graphql/mod.rs @@ -254,7 +254,7 @@ mod tests { } #[tokio::test] - async fn deeply_nested_queries_are_rejected_before_resolver_execution() { + async fn query_depth_limit_accepts_twelve_and_rejects_thirteen() { let calls = Arc::new(AtomicUsize::new(0)); let schema = apply_query_limits(Schema::build( CountingQuery(Arc::clone(&calls)), @@ -262,17 +262,27 @@ mod tests { EmptySubscription, )) .finish(); - let selection = (0..GRAPHQL_MAX_DEPTH).fold("value".to_string(), |selection, _| { - format!("child {{ {selection} }}") - }); + let query_at_depth = |depth: usize| { + let selection = (2..depth).fold("value".to_string(), |selection, _| { + format!("child {{ {selection} }}") + }); + format!("{{ nested {{ {selection} }} }}") + }; - let response = schema - .execute(format!("{{ nested {{ {selection} }} }}")) - .await; + let accepted = schema.execute(query_at_depth(GRAPHQL_MAX_DEPTH)).await; - assert_eq!(response.data, Value::Null); - assert_eq!(response.errors.len(), 1); - assert_eq!(response.errors[0].message, "Query is nested too deep."); + assert!( + accepted.errors.is_empty(), + "depth {GRAPHQL_MAX_DEPTH} should be accepted: {:?}", + accepted.errors + ); + assert_eq!(calls.swap(0, Ordering::Relaxed), 1); + + let rejected = schema.execute(query_at_depth(GRAPHQL_MAX_DEPTH + 1)).await; + + assert_eq!(rejected.data, Value::Null); + assert_eq!(rejected.errors.len(), 1); + assert_eq!(rejected.errors[0].message, "Query is nested too deep."); assert_eq!(calls.load(Ordering::Relaxed), 0); } From e77bfda7790699310dcc0385cf61927aad9199d3 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Tue, 8 Sep 2026 20:32:56 -0400 Subject: [PATCH 03/20] fix(node): Charge GraphQL mutations and list sizes against query limits. --- crates/gitlawb-node/src/graphql/mod.rs | 68 +++++++++++++++++++-- crates/gitlawb-node/src/graphql/mutation.rs | 4 ++ crates/gitlawb-node/src/graphql/query.rs | 4 +- 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/crates/gitlawb-node/src/graphql/mod.rs b/crates/gitlawb-node/src/graphql/mod.rs index d96219a0..09e73025 100644 --- a/crates/gitlawb-node/src/graphql/mod.rs +++ b/crates/gitlawb-node/src/graphql/mod.rs @@ -177,12 +177,72 @@ mod tests { assert!(!err.message.contains("failed to open")); } + fn production_test_schema() -> GitlawbSchema { + let pool = sqlx::postgres::PgPoolOptions::new() + .connect_lazy("postgres://localhost/unused") + .unwrap(); + build_schema( + Arc::new(Db::for_testing(pool)), + tokio::sync::broadcast::channel(1).0, + tokio::sync::broadcast::channel(1).0, + ) + } + + #[tokio::test] + async fn production_limits_reject_mutation_aliases_and_large_lists() { + let schema = production_test_schema(); + for field in [ + "claimTask(id: \"missing\", assigneeDid: \"did:key:test\") { id }", + "refUpdates(limit: 200) { repo }", + "tasks(limit: 200) { id }", + ] { + let count = if field.starts_with("claim") { 8 } else { 2 }; + let fields = (0..count) + .map(|n| format!("r{n}: {field}")) + .collect::>() + .join(" "); + let prefix = if field.starts_with("claim") { + "mutation" + } else { + "query" + }; + let response = schema.execute(format!("{prefix} {{ {fields} }}")).await; + assert_eq!(response.errors.len(), 1, "{:?}", response.errors); + assert_eq!(response.errors[0].message, "Query is too complex."); + } + } + + #[tokio::test] + async fn seven_root_aliases_are_accepted() { + struct Root(Arc); + #[Object] + impl Root { + #[graphql(complexity = "50 + child_complexity")] + async fn repos(&self) -> Vec { + self.0.fetch_add(1, Ordering::Relaxed); + vec![Nested] + } + } + let calls = Arc::new(AtomicUsize::new(0)); + let schema = apply_query_limits(Schema::build( + Root(calls.clone()), + EmptyMutation, + EmptySubscription, + )) + .finish(); + let fields = (0..7) + .map(|n| format!("r{n}: repos {{ value }}")) + .collect::>() + .join(" "); + let response = schema.execute(format!("{{ {fields} }}")).await; + assert!(response.errors.is_empty(), "{:?}", response.errors); + assert_eq!(calls.load(Ordering::Relaxed), 7); + } + #[tokio::test] async fn expensive_root_aliases_are_rejected_before_database_access() { - // QueryRoot expects a Db in the schema data. Deliberately omit it: if - // validation ever lets this document reach a resolver, the test fails. - let schema = - apply_query_limits(Schema::build(QueryRoot, EmptyMutation, EmptySubscription)).finish(); + // Use the production builder with a lazy pool: rejection must precede DB access. + let schema = production_test_schema(); let fields = (0..8) .map(|n| format!("r{n}: repos {{ name }}")) .collect::>() diff --git a/crates/gitlawb-node/src/graphql/mutation.rs b/crates/gitlawb-node/src/graphql/mutation.rs index 7fb7a1dc..c12e19d8 100644 --- a/crates/gitlawb-node/src/graphql/mutation.rs +++ b/crates/gitlawb-node/src/graphql/mutation.rs @@ -22,6 +22,7 @@ pub struct MutationRoot; #[Object] impl MutationRoot { + #[graphql(complexity = "50 + child_complexity")] async fn create_task( &self, ctx: &Context<'_>, @@ -58,6 +59,7 @@ impl MutationRoot { Ok(AgentTaskType::from(task)) } + #[graphql(complexity = "50 + child_complexity")] async fn claim_task( &self, ctx: &Context<'_>, @@ -87,6 +89,7 @@ impl MutationRoot { Ok(AgentTaskType::from(task)) } + #[graphql(complexity = "50 + child_complexity")] async fn complete_task( &self, ctx: &Context<'_>, @@ -129,6 +132,7 @@ impl MutationRoot { Ok(AgentTaskType::from(task)) } + #[graphql(complexity = "50 + child_complexity")] async fn fail_task( &self, ctx: &Context<'_>, diff --git a/crates/gitlawb-node/src/graphql/query.rs b/crates/gitlawb-node/src/graphql/query.rs index eac2ec79..9bd7e01c 100644 --- a/crates/gitlawb-node/src/graphql/query.rs +++ b/crates/gitlawb-node/src/graphql/query.rs @@ -48,7 +48,7 @@ impl QueryRoot { .collect()) } - #[graphql(complexity = "50 + child_complexity")] + #[graphql(complexity = "50 + (limit.clamp(0, 200) as usize) * child_complexity")] async fn ref_updates( &self, ctx: &Context<'_>, @@ -109,7 +109,7 @@ impl QueryRoot { Ok(resolved) } - #[graphql(complexity = "50 + child_complexity")] + #[graphql(complexity = "50 + (limit.clamp(0, 200) as usize) * child_complexity")] async fn tasks( &self, ctx: &Context<'_>, From c975747330e6b22a3840776102b0a5ace24ab64e Mon Sep 17 00:00:00 2001 From: euxaristia Date: Wed, 9 Sep 2026 01:37:49 -0400 Subject: [PATCH 04/20] fix(node): paginate visible GraphQL repository lists Apply root visibility and mirror deduplication before a bounded SQL page. Add reposPage continuation metadata and reject oversized legacy lists explicitly, preserving activity ordering for complete small lists. Cover visibility equivalence, cursor authority, page boundaries, overflow, and input validation. BREAKING CHANGE: repos returns an error above 200 visible repositories; clients must use reposPage to traverse larger lists. --- README.md | 1 + crates/gitlawb-node/src/db/mod.rs | 54 ++++ crates/gitlawb-node/src/graphql/mod.rs | 1 + crates/gitlawb-node/src/graphql/query.rs | 343 +++++++++++++++++++++-- crates/gitlawb-node/src/graphql/types.rs | 8 + docs/graphql-pagination.md | 38 +++ 6 files changed, 420 insertions(+), 25 deletions(-) create mode 100644 docs/graphql-pagination.md diff --git a/README.md b/README.md index 3a092bf2..fb20ef1d 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,7 @@ Good today: - Local or Docker node startup. - Postgres-backed repo metadata. +- Bounded GraphQL queries with [repository pagination](docs/graphql-pagination.md). - Bare git repository storage. - Git smart-HTTP clone/fetch/push. - RFC 9421-signed writes. diff --git a/crates/gitlawb-node/src/db/mod.rs b/crates/gitlawb-node/src/db/mod.rs index cc2cf0bd..d324157e 100644 --- a/crates/gitlawb-node/src/db/mod.rs +++ b/crates/gitlawb-node/src/db/mod.rs @@ -6,6 +6,9 @@ use std::time::Duration; use tracing::info; use uuid::Uuid; +/// Maximum visible repositories per page, plus one internal look-ahead row. +pub(crate) const MAX_VISIBLE_REPO_PAGE_SIZE: usize = 200; + // ── Public data types ───────────────────────────────────────────────────────── #[derive(Debug, Clone, Serialize, Deserialize)] @@ -1540,6 +1543,57 @@ impl Db { Ok(rows.into_iter().map(row_to_repo).collect()) } + /// A bounded, mirror-deduplicated page ordered by owner key and repository + /// name. Apply root visibility before LIMIT: private rows must not consume + /// page slots or influence continuation metadata. The root-rule predicate + /// mirrors `visibility::listable_at_root`; a differential test pins it. + /// Cursors only select a position and never confer read authority. + pub async fn list_visible_repos_page( + &self, + caller: Option<&str>, + after: Option<(&str, &str)>, + limit: usize, + ) -> Result> { + let sql = format!( + "{} + SELECT d.id, d.name, d.owner_did, d.description, d.is_public, + d.default_branch, d.created_at, d.updated_at, d.disk_path, + d.forked_from, d.machine_id + FROM deduped d + LEFT JOIN LATERAL ( + SELECT reader_dids FROM visibility_rules v + WHERE v.repo_id = d.id AND v.path_glob ~ '^/*([*][*])*$' + ORDER BY v.path_glob DESC LIMIT 1 + ) root_rule ON TRUE + WHERE ( + ($2::text IS NOT NULL AND ({key}) = $2) + OR CASE WHEN root_rule.reader_dids IS NULL THEN d.is_public + WHEN jsonb_typeof(root_rule.reader_dids::jsonb) = 'array' THEN + COALESCE(root_rule.reader_dids::jsonb ? $3::text, FALSE) + AND NOT EXISTS ( + SELECT 1 FROM jsonb_array_elements(root_rule.reader_dids::jsonb) reader + WHERE jsonb_typeof(reader) <> 'string' + ) + ELSE FALSE END + ) + AND ($4::text IS NULL OR (({key}), d.name) > ($4, $5::text)) + ORDER BY ({key}), d.name + LIMIT $6", + Self::dedup_cte(), + key = OWNER_KEY_CASE_SQL, + ); + let rows = sqlx::query(&sql) + .bind(None::<&str>) + .bind(caller.map(normalize_owner_key)) + .bind(caller) + .bind(after.map(|(owner, _)| normalize_owner_key(owner))) + .bind(after.map(|(_, name)| name)) + .bind(limit.min(MAX_VISIBLE_REPO_PAGE_SIZE + 1) as i64) + .fetch_all(&self.pool) + .await?; + Ok(rows.into_iter().map(row_to_repo).collect()) + } + /// Repos currently quarantined (admitted as mirrors but withheld from every /// listing surface). `list_all_repos_deduped` excludes these (its `DEDUP_CTE` /// filters `quarantined = FALSE`), so a gate that resolves a slug against the diff --git a/crates/gitlawb-node/src/graphql/mod.rs b/crates/gitlawb-node/src/graphql/mod.rs index 09e73025..22f46846 100644 --- a/crates/gitlawb-node/src/graphql/mod.rs +++ b/crates/gitlawb-node/src/graphql/mod.rs @@ -195,6 +195,7 @@ mod tests { "claimTask(id: \"missing\", assigneeDid: \"did:key:test\") { id }", "refUpdates(limit: 200) { repo }", "tasks(limit: 200) { id }", + "reposPage(limit: 200) { nodes { name } }", ] { let count = if field.starts_with("claim") { 8 } else { 2 }; let fields = (0..count) diff --git a/crates/gitlawb-node/src/graphql/query.rs b/crates/gitlawb-node/src/graphql/query.rs index 9bd7e01c..38359389 100644 --- a/crates/gitlawb-node/src/graphql/query.rs +++ b/crates/gitlawb-node/src/graphql/query.rs @@ -1,9 +1,40 @@ use async_graphql::{Context, Object, Result}; +use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine}; use std::sync::Arc; -use crate::db::Db; +use crate::db::{Db, RepoRecord, MAX_VISIBLE_REPO_PAGE_SIZE}; -use super::types::{AgentTaskType, RefUpdateType, RepoType}; +use super::types::{AgentTaskType, RefUpdateType, RepoPageType, RepoType}; + +fn repo_type(repo: RepoRecord) -> RepoType { + RepoType { + name: repo.name, + owner_did: repo.owner_did, + description: repo.description, + default_branch: repo.default_branch, + created_at: repo.created_at.to_rfc3339(), + } +} + +fn repo_cursor(repo: &RepoRecord) -> String { + URL_SAFE_NO_PAD.encode( + serde_json::to_vec(&(&repo.owner_did, &repo.name)) + .expect("a pair of strings is JSON serializable"), + ) +} + +fn parse_repo_cursor(cursor: &str) -> Result<(String, String)> { + // A position contains only public response fields, not a database id or + // authority. Every page re-evaluates visibility for the current caller. + if cursor.len() > 4096 { + return Err(async_graphql::Error::new("invalid repository cursor")); + } + let bytes = URL_SAFE_NO_PAD + .decode(cursor) + .map_err(|_| async_graphql::Error::new("invalid repository cursor"))?; + serde_json::from_slice(&bytes) + .map_err(|_| async_graphql::Error::new("invalid repository cursor")) +} pub struct QueryRoot; @@ -12,40 +43,68 @@ impl QueryRoot { // DB-backed roots carry a base cost so aliases consume the request budget // even when each alias selects only one inexpensive response field. #[graphql(complexity = "50 + child_complexity")] + /// Complete visible repository list, up to 200 entries. Larger lists must + /// use reposPage; this field returns an error instead of truncating silently. async fn repos(&self, ctx: &Context<'_>) -> Result> { let db = ctx.data_unchecked::>(); - let repos = db - .list_all_repos_deduped() + let caller = ctx + .data::() + .ok() + .map(|d| d.0.as_str()); + let mut repos = db + .list_visible_repos_page(caller, None, MAX_VISIBLE_REPO_PAGE_SIZE + 1) .await .map_err(crate::graphql::graphql_db_err)?; + if repos.len() > MAX_VISIBLE_REPO_PAGE_SIZE { + return Err(async_graphql::Error::new( + "repository list exceeds 200 entries; use reposPage with limit and after", + )); + } + // Preserve the legacy activity ordering for complete, small lists. + repos.sort_by_key(|repo| std::cmp::Reverse(repo.updated_at)); + Ok(repos.into_iter().map(repo_type).collect()) + } - // Apply the same "/" visibility gate the REST/per-repo endpoints use so - // this surface does not enumerate private repos (#97). The caller DID is - // threaded onto the context by optional_signature; absent = anonymous. + /// Bounded visible repositories ordered by owner and name. Continue with + /// endCursor while hasNextPage is true. Pages are not a database snapshot. + #[graphql(complexity = "50 + (limit.clamp(1, 200) as usize) * child_complexity")] + async fn repos_page( + &self, + ctx: &Context<'_>, + #[graphql( + default = 50, + desc = "Page size from 1 to 200; other values are rejected." + )] + limit: i64, + after: Option, + ) -> Result { + if !(1..=MAX_VISIBLE_REPO_PAGE_SIZE as i64).contains(&limit) { + return Err(async_graphql::Error::new("limit must be between 1 and 200")); + } + let after = after.as_deref().map(parse_repo_cursor).transpose()?; let caller = ctx .data::() .ok() .map(|d| d.0.as_str()); - let ids: Vec = repos.iter().map(|r| r.id.clone()).collect(); - let rules_by_repo = db - .list_visibility_rules_for_repos(&ids) + let db = ctx.data_unchecked::>(); + let mut repos = db + .list_visible_repos_page( + caller, + after + .as_ref() + .map(|(owner, name)| (owner.as_str(), name.as_str())), + limit as usize + 1, + ) .await .map_err(crate::graphql::graphql_db_err)?; - - Ok(repos - .into_iter() - .filter(|r| { - let rules = rules_by_repo.get(&r.id).map(Vec::as_slice).unwrap_or(&[]); - crate::visibility::listable_at_root(rules, r.is_public, &r.owner_did, caller) - }) - .map(|r| RepoType { - name: r.name, - owner_did: r.owner_did, - description: r.description, - default_branch: r.default_branch, - created_at: r.created_at.to_rfc3339(), - }) - .collect()) + let has_next_page = repos.len() > limit as usize; + repos.truncate(limit as usize); + let end_cursor = repos.last().map(repo_cursor); + Ok(RepoPageType { + nodes: repos.into_iter().map(repo_type).collect(), + has_next_page, + end_cursor, + }) } #[graphql(complexity = "50 + (limit.clamp(0, 200) as usize) * child_complexity")] @@ -153,6 +212,240 @@ mod tests { const OWNER: &str = "did:key:z6MkOwner"; + #[sqlx::test] + async fn repos_legacy_rejects_overflow_and_pages_reach_every_visible_repo(pool: PgPool) { + let db = db(pool).await; + let total = crate::db::MAX_VISIBLE_REPO_PAGE_SIZE + 5; + for index in 0..total { + let name = format!("repo-{index:03}"); + db.create_repo(&repo(&name, OWNER, &name, true)) + .await + .unwrap(); + } + // The SQL helper itself must bound materialization, even for a caller + // accidentally requesting an unlimited page. + assert_eq!( + db.list_visible_repos_page(None, None, usize::MAX) + .await + .unwrap() + .len(), + crate::db::MAX_VISIBLE_REPO_PAGE_SIZE + 1 + ); + let schema = schema(db); + let legacy = anon(&schema, "{ repos { name } }").await; + assert_eq!(legacy.errors.len(), 1); + assert!(legacy.errors[0].message.contains("use reposPage")); + assert_eq!(legacy.data, async_graphql::Value::Null); + + let mut cursor = None; + let mut names = Vec::new(); + loop { + let response = schema.execute( + async_graphql::Request::new( + "query($after: String) { reposPage(limit: 50, after: $after) { nodes { name } hasNextPage endCursor } }", + ).variables(async_graphql::Variables::from_json(serde_json::json!({"after": cursor}))), + ).await; + assert!(response.errors.is_empty(), "{:?}", response.errors); + let json = response.data.into_json().unwrap(); + let page = &json["reposPage"]; + let rows = page["nodes"].as_array().unwrap(); + assert!(rows.len() <= 50); + names.extend( + rows.iter() + .map(|row| row["name"].as_str().unwrap().to_owned()), + ); + if !page["hasNextPage"].as_bool().unwrap() { + break; + } + let next = page["endCursor"].as_str().unwrap().to_owned(); + assert_ne!(cursor.as_ref(), Some(&next)); + cursor = Some(next); + assert!(names.len() <= crate::db::MAX_VISIBLE_REPO_PAGE_SIZE); + } + assert_eq!( + names, + (0..total) + .map(|index| format!("repo-{index:03}")) + .collect::>() + ); + } + + #[sqlx::test] + async fn repos_legacy_accepts_exactly_the_visible_bound(pool: PgPool) { + let db = db(pool).await; + for index in 0..crate::db::MAX_VISIBLE_REPO_PAGE_SIZE { + let name = format!("repo-{index:03}"); + db.create_repo(&repo(&name, OWNER, &name, true)) + .await + .unwrap(); + } + db.create_repo(&repo("hidden", OWNER, "hidden", false)) + .await + .unwrap(); + let response = anon(&schema(db), "{ repos { name } }").await; + assert!(response.errors.is_empty(), "{:?}", response.errors); + assert_eq!( + response.data.into_json().unwrap()["repos"] + .as_array() + .unwrap() + .len(), + crate::db::MAX_VISIBLE_REPO_PAGE_SIZE + ); + } + + #[sqlx::test] + async fn repos_page_visibility_matches_the_shared_gate(pool: PgPool) { + use crate::db::VisibilityMode; + let db = db(pool).await; + let reader = "did:key:zReader"; + for (id, public) in [ + ("open", true), + ("private", false), + ("root-deny", true), + ("root-reader", false), + ("subtree", true), + ("root-tie", true), + ("odd-star", true), + ("quarantined", true), + ] { + db.create_repo(&repo(id, OWNER, id, public)).await.unwrap(); + } + // Canonical and mirror copies must still collapse before pagination. + db.create_repo(&repo("z6MkOwner/open", "z6MkOwner", "open", true)) + .await + .unwrap(); + db.create_repo(&repo( + "other-method", + "did:web:z6MkOwner", + "other-method", + false, + )) + .await + .unwrap(); + db.set_repo_quarantine("quarantined", true).await.unwrap(); + for (id, glob, readers) in [ + ("root-deny", "/", vec![]), + ("root-reader", "/**", vec![reader.to_owned()]), + ("subtree", "/secret/**", vec![]), + ("root-tie", "/", vec![reader.to_owned()]), + ("root-tie", "/**", vec![]), + ("odd-star", "/*", vec![]), + ] { + db.set_visibility_rule(id, glob, VisibilityMode::B, &readers, OWNER) + .await + .unwrap(); + } + let all = db.list_all_repos_deduped().await.unwrap(); + for caller in [ + None, + Some(OWNER), + Some("z6MkOwner"), + Some(reader), + Some("zReader"), + Some("did:web:z6MkOwner"), + ] { + let mut expected = Vec::new(); + for record in &all { + let rules = db.list_visibility_rules(&record.id).await.unwrap(); + if crate::visibility::listable_at_root( + &rules, + record.is_public, + &record.owner_did, + caller, + ) { + expected.push(record.id.clone()); + } + } + expected.sort(); + let mut actual = db + .list_visible_repos_page(caller, None, usize::MAX) + .await + .unwrap() + .into_iter() + .map(|record| record.id) + .collect::>(); + actual.sort(); + assert_eq!(actual, expected, "caller {caller:?}"); + } + let schema = schema(db); + let query = "{ reposPage(limit: 1) { nodes { name ownerDid } hasNextPage endCursor } }"; + let response = anon(&schema, query).await; + assert!(response.errors.is_empty(), "{:?}", response.errors); + let json = response.data.into_json().unwrap(); + let page = &json["reposPage"]; + assert_eq!( + page["nodes"], + serde_json::json!([{"name": "odd-star", "ownerDid": OWNER}]) + ); + assert_eq!(page["hasNextPage"], true); + let cursor = super::parse_repo_cursor(page["endCursor"].as_str().unwrap()).unwrap(); + assert_eq!(cursor, (OWNER.to_owned(), "odd-star".to_owned())); + assert!(!json.to_string().contains("private")); + assert!(!json.to_string().contains("quarantined")); + } + + #[sqlx::test] + async fn repos_page_rechecks_cursor_authority_and_exact_boundary(pool: PgPool) { + let db = db(pool).await; + let visible = repo("visible", OWNER, "a-visible", true); + db.create_repo(&visible).await.unwrap(); + db.create_repo(&repo("hidden", OWNER, "z-private", false)) + .await + .unwrap(); + let schema = schema(db); + let first = anon( + &schema, + "{ reposPage(limit: 1) { nodes { name } hasNextPage endCursor } }", + ) + .await; + assert!(first.errors.is_empty()); + let first = first.data.into_json().unwrap(); + assert_eq!(first["reposPage"]["hasNextPage"], false); + let query = format!( + "{{ reposPage(limit: 1, after: \"{}\") {{ nodes {{ name }} hasNextPage endCursor }} }}", + super::repo_cursor(&visible) + ); + let owner = authed(&schema, &query, OWNER).await; + assert!(owner.errors.is_empty()); + assert_eq!( + owner.data.into_json().unwrap()["reposPage"]["nodes"][0]["name"], + "z-private" + ); + let anon = anon(&schema, &query).await; + assert!(anon.errors.is_empty()); + assert_eq!( + anon.data.into_json().unwrap()["reposPage"], + serde_json::json!({ + "nodes": [], "hasNextPage": false, "endCursor": null + }) + ); + } + + #[tokio::test] + async fn repos_page_rejects_invalid_inputs_before_database_access() { + let pool = sqlx::postgres::PgPoolOptions::new() + .connect_lazy("postgres://localhost/unused") + .unwrap(); + let schema = schema(Arc::new(Db::for_testing(pool))); + for query in [ + "{ reposPage(limit: 0) { hasNextPage } }", + "{ reposPage(limit: -1) { hasNextPage } }", + "{ reposPage(limit: 201) { hasNextPage } }", + "{ reposPage(after: \"invalid!\") { hasNextPage } }", + ] { + let response = + tokio::time::timeout(std::time::Duration::from_secs(1), anon(&schema, query)) + .await + .unwrap(); + assert_eq!(response.errors.len(), 1); + assert!( + response.errors[0].message == "limit must be between 1 and 200" + || response.errors[0].message == "invalid repository cursor" + ); + } + assert!(super::parse_repo_cursor(&"a".repeat(4097)).is_err()); + } + async fn db(pool: PgPool) -> Arc { let db = Db::for_testing(pool); db.run_migrations().await.unwrap(); diff --git a/crates/gitlawb-node/src/graphql/types.rs b/crates/gitlawb-node/src/graphql/types.rs index 4264a581..bae1aa35 100644 --- a/crates/gitlawb-node/src/graphql/types.rs +++ b/crates/gitlawb-node/src/graphql/types.rs @@ -11,6 +11,14 @@ pub struct RepoType { pub created_at: String, } +#[derive(SimpleObject)] +pub struct RepoPageType { + pub nodes: Vec, + pub has_next_page: bool, + /// Position after the last returned visible repository; null for an empty page. + pub end_cursor: Option, +} + #[derive(SimpleObject, Clone)] pub struct AgentTaskType { pub id: String, diff --git a/docs/graphql-pagination.md b/docs/graphql-pagination.md new file mode 100644 index 00000000..065d2da5 --- /dev/null +++ b/docs/graphql-pagination.md @@ -0,0 +1,38 @@ +# GraphQL repository pagination + +`repos` returns the complete visible repository list when it contains at most +200 entries. Larger lists return a GraphQL error directing the caller to +`reposPage`; they are never silently truncated. + +Use `reposPage` for larger lists: + +```graphql +query Repositories($after: String) { + reposPage(limit: 50, after: $after) { + nodes { + name + ownerDid + } + hasNextPage + endCursor + } +} +``` + +Start with `after: null`. When `hasNextPage` is true, pass the returned +`endCursor` as `after` on the next request. Stop when `hasNextPage` is false. +An empty page has a null `endCursor`. The default limit is 50; limits outside +1–200 are rejected. The document complexity budget still applies, so requesting +many fields may require a smaller page. + +Pages are ordered by normalized owner DID and repository name. Visibility and +mirror deduplication are applied in the database before limiting the page; +hidden and quarantined repositories do not occupy page slots or create a +continuation signal. Cursors contain a position from the last returned visible +repository, not a permission grant. Each request checks the current caller's +visibility independently. Keep the same caller while traversing a list. + +Pagination is not a snapshot: concurrent renames, ownership changes, or visibility +changes can alter subsequent pages. Treat cursors as opaque and restart from the +first page when a fresh complete traversal is needed. Small legacy `repos` +queries retain their activity ordering. From 9d38e043a47af46d0070069ca1f980f5ae561007 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Wed, 9 Sep 2026 02:11:48 -0400 Subject: [PATCH 05/20] fix(node): charge subscription roots and test query boundaries Bound subscription receiver fan-out with the same root cost as queries and mutations. Exercise accepted and rejected production mutation aliases and list limits, and prove subscription validation rejects excess receivers before registration. --- crates/gitlawb-node/src/graphql/mod.rs | 190 +++++++++++++++++- .../gitlawb-node/src/graphql/subscription.rs | 2 + 2 files changed, 191 insertions(+), 1 deletion(-) diff --git a/crates/gitlawb-node/src/graphql/mod.rs b/crates/gitlawb-node/src/graphql/mod.rs index 22f46846..0d87f0c1 100644 --- a/crates/gitlawb-node/src/graphql/mod.rs +++ b/crates/gitlawb-node/src/graphql/mod.rs @@ -181,13 +181,201 @@ mod tests { let pool = sqlx::postgres::PgPoolOptions::new() .connect_lazy("postgres://localhost/unused") .unwrap(); + production_test_schema_with_db(Arc::new(Db::for_testing(pool))) + } + + fn production_test_schema_with_db(db: Arc) -> GitlawbSchema { build_schema( - Arc::new(Db::for_testing(pool)), + db, tokio::sync::broadcast::channel(1).0, tokio::sync::broadcast::channel(1).0, ) } + #[tokio::test] + async fn ref_update_subscription_alias_budget() { + assert_subscription_alias_budget("refUpdates { repo }").await; + } + + #[tokio::test] + async fn task_event_subscription_alias_budget() { + assert_subscription_alias_budget("taskEvents { taskId }").await; + } + + async fn assert_subscription_alias_budget(field: &str) { + use futures::StreamExt; + use std::time::Duration; + let pool = sqlx::postgres::PgPoolOptions::new() + .connect_lazy("postgres://localhost/unused") + .unwrap(); + let (ref_tx, _) = tokio::sync::broadcast::channel(16); + let (task_tx, _) = tokio::sync::broadcast::channel(16); + let schema = build_schema( + Arc::new(Db::for_testing(pool)), + ref_tx.clone(), + task_tx.clone(), + ); + for count in [7, 8] { + let fields = (0..count) + .map(|n| format!("r{n}: {field}")) + .collect::>() + .join(" "); + let query = format!("subscription {{ {fields} }}"); + let mut responses = Box::pin(schema.execute_stream(query)); + if count == 8 { + let response = tokio::time::timeout(Duration::from_secs(1), responses.next()) + .await + .expect("over-budget subscriptions must reject before waiting for events") + .expect("the validation error must be returned"); + assert_eq!(response.errors.len(), 1); + assert_eq!(response.errors[0].message, "Query is too complex."); + assert_eq!(ref_tx.receiver_count() + task_tx.receiver_count(), 0); + continue; + } + // Poll the accepted operation far enough to register its receivers. + assert!( + tokio::time::timeout(Duration::from_millis(30), responses.next()) + .await + .is_err() + ); + assert_eq!(ref_tx.receiver_count() + task_tx.receiver_count(), 7); + let _ = ref_tx.send(RefUpdateBroadcast { + repo: "owner/repo".into(), + ref_name: "refs/heads/main".into(), + old_sha: "0".repeat(40), + new_sha: "1".repeat(40), + pusher_did: "did:key:test".into(), + node_did: "did:key:node".into(), + timestamp: "2026-01-01T00:00:00Z".into(), + owner_did: "did:key:test".into(), + }); + let _ = task_tx.send(TaskEventBroadcast { + task_id: "task".into(), + old_status: "pending".into(), + new_status: "claimed".into(), + by_did: "did:key:test".into(), + at: "2026-01-01T00:00:00Z".into(), + }); + let response = tokio::time::timeout(Duration::from_secs(1), responses.next()) + .await + .unwrap() + .unwrap(); + assert!(response.errors.is_empty(), "{:?}", response.errors); + assert_ne!(response.data, Value::Null); + drop(responses); + assert_eq!(ref_tx.receiver_count() + task_tx.receiver_count(), 0); + } + } + + #[sqlx::test] + async fn production_mutation_budget_accepts_seven_and_rejects_eight(pool: sqlx::PgPool) { + let db = Arc::new(Db::for_testing(pool)); + db.run_migrations().await.unwrap(); + let caller = "did:key:reviewer"; + let now = chrono::Utc::now().to_rfc3339(); + for index in 0..8 { + db.create_task(&crate::db::AgentTask { + id: format!("task-{index}"), + repo_id: None, + kind: "test".into(), + status: "pending".into(), + delegator_did: caller.into(), + assignee_did: None, + capability: "test".into(), + ucan_token: None, + payload: None, + result: None, + created_at: now.clone(), + updated_at: now.clone(), + deadline: None, + }) + .await + .unwrap(); + } + let schema = production_test_schema_with_db(db.clone()); + for count in [8, 7] { + let fields = (0..count) + .map(|n| { + format!("r{n}: claimTask(id: \"task-{n}\", assigneeDid: \"{caller}\") {{ id }}") + }) + .collect::>() + .join(" "); + let response = schema + .execute( + async_graphql::Request::new(format!("mutation {{ {fields} }}")) + .data(crate::auth::AuthenticatedDid(caller.into())), + ) + .await; + if count == 8 { + assert_eq!(response.errors.len(), 1); + assert_eq!(response.errors[0].message, "Query is too complex."); + assert_eq!( + db.list_tasks(Some("pending"), None, 8).await.unwrap().len(), + 8 + ); + continue; + } + assert!(response.errors.is_empty(), "{:?}", response.errors); + assert_eq!( + response + .data + .into_json() + .unwrap() + .as_object() + .unwrap() + .len(), + 7 + ); + assert_eq!( + db.list_tasks(Some("claimed"), Some(caller), 8) + .await + .unwrap() + .len(), + 7 + ); + assert_eq!( + db.get_task("task-7").await.unwrap().unwrap().status, + "pending" + ); + } + } + + #[sqlx::test] + async fn production_list_cost_scales_with_the_same_alias_count(pool: sqlx::PgPool) { + let db = Arc::new(Db::for_testing(pool)); + db.run_migrations().await.unwrap(); + let schema = production_test_schema_with_db(db); + for (field, selection) in [ + ("refUpdates", "repo"), + ("tasks", "id"), + ("reposPage", "nodes { name }"), + ] { + for limit in [1, 200] { + let fields = (0..2) + .map(|n| format!("r{n}: {field}(limit: {limit}) {{ {selection} }}")) + .collect::>() + .join(" "); + let response = schema.execute(format!("{{ {fields} }}")).await; + if limit == 200 { + assert_eq!(response.errors.len(), 1); + assert_eq!(response.errors[0].message, "Query is too complex."); + continue; + } + assert!(response.errors.is_empty(), "{field}: {:?}", response.errors); + assert_eq!( + response + .data + .into_json() + .unwrap() + .as_object() + .unwrap() + .len(), + 2 + ); + } + } + } + #[tokio::test] async fn production_limits_reject_mutation_aliases_and_large_lists() { let schema = production_test_schema(); diff --git a/crates/gitlawb-node/src/graphql/subscription.rs b/crates/gitlawb-node/src/graphql/subscription.rs index 7248cbf4..73e431ca 100644 --- a/crates/gitlawb-node/src/graphql/subscription.rs +++ b/crates/gitlawb-node/src/graphql/subscription.rs @@ -20,6 +20,7 @@ impl SubscriptionRoot { /// `if announce` block, `api/repos.rs`). This is a single-point invariant — /// any new sender to `ref_update_tx` MUST be `announce`-gated, or private-repo /// ref metadata leaks here to unauthenticated subscribers (#112/#114 class). + #[graphql(complexity = "50 + child_complexity")] async fn ref_updates( &self, ctx: &Context<'_>, @@ -46,6 +47,7 @@ impl SubscriptionRoot { }) } + #[graphql(complexity = "50 + child_complexity")] async fn task_events( &self, ctx: &Context<'_>, From ebafeae909364359466a8438a7cc9acec9970a2c Mon Sep 17 00:00:00 2001 From: euxaristia Date: Thu, 10 Sep 2026 17:55:41 -0400 Subject: [PATCH 06/20] test(graphql): verify unauthorized caller, legacy bound, and production depth limit Address review feedback on PR #408: - Add route-level reposPage tests verifying unauthorized callers and readers exclude private, quarantined, and root-deny repositories without leaking data. - Assert that the hidden repository is excluded in the legacy repos boundary test. - Verify depth limit acceptance at 12 and rejection at 13 through the production build_schema path. Refs #408 --- crates/gitlawb-node/src/graphql/mod.rs | 27 +++++++++ crates/gitlawb-node/src/graphql/query.rs | 71 ++++++++++++++++++++++-- 2 files changed, 92 insertions(+), 6 deletions(-) diff --git a/crates/gitlawb-node/src/graphql/mod.rs b/crates/gitlawb-node/src/graphql/mod.rs index 0d87f0c1..57702a72 100644 --- a/crates/gitlawb-node/src/graphql/mod.rs +++ b/crates/gitlawb-node/src/graphql/mod.rs @@ -535,6 +535,33 @@ mod tests { assert_eq!(calls.load(Ordering::Relaxed), 0); } + #[tokio::test] + async fn production_schema_enforces_depth_limit() { + let schema = production_test_schema(); + // Depth 12 is accepted through the production build_schema path. + let of_types_12 = (0..7).fold("name".to_string(), |acc, _| format!("ofType {{ {acc} }}")); + let query_12 = + format!("{{ __schema {{ types {{ fields {{ type {{ {of_types_12} }} }} }} }} }}"); + let accepted = schema.execute(&query_12).await; + assert!(accepted.errors.is_empty(), "{:?}", accepted.errors); + + // Construct an introspection query of depth 13 using __schema. + // 1: __schema + // 2: types + // 3: fields + // 4: type + // 5..12: ofType (8 times) + // 13: name + // Total depth = 13 > GRAPHQL_MAX_DEPTH (12). + let of_types = (0..8).fold("name".to_string(), |acc, _| format!("ofType {{ {acc} }}")); + let query = format!("{{ __schema {{ types {{ fields {{ type {{ {of_types} }} }} }} }} }}"); + + let rejected = schema.execute(&query).await; + assert_eq!(rejected.data, async_graphql::Value::Null); + assert_eq!(rejected.errors.len(), 1); + assert_eq!(rejected.errors[0].message, "Query is nested too deep."); + } + /// Every `.map_err(` in the GraphQL query/mutation resolvers must route /// through the opaque helpers, or discard the error (`|_|`). Same source- /// scrape pattern as `api::authz_guard` (#255 review). diff --git a/crates/gitlawb-node/src/graphql/query.rs b/crates/gitlawb-node/src/graphql/query.rs index 38359389..91fe507a 100644 --- a/crates/gitlawb-node/src/graphql/query.rs +++ b/crates/gitlawb-node/src/graphql/query.rs @@ -284,12 +284,15 @@ mod tests { .unwrap(); let response = anon(&schema(db), "{ repos { name } }").await; assert!(response.errors.is_empty(), "{:?}", response.errors); - assert_eq!( - response.data.into_json().unwrap()["repos"] - .as_array() - .unwrap() - .len(), - crate::db::MAX_VISIBLE_REPO_PAGE_SIZE + let repos = response.data.into_json().unwrap()["repos"] + .as_array() + .unwrap() + .clone(); + assert_eq!(repos.len(), crate::db::MAX_VISIBLE_REPO_PAGE_SIZE); + let names: Vec<_> = repos.iter().filter_map(|r| r["name"].as_str()).collect(); + assert!( + !names.contains(&"hidden"), + "hidden repo must be excluded from legacy repos response" ); } @@ -382,6 +385,54 @@ mod tests { assert_eq!(cursor, (OWNER.to_owned(), "odd-star".to_owned())); assert!(!json.to_string().contains("private")); assert!(!json.to_string().contains("quarantined")); + + // Route-level check with unauthorized authenticated caller: + // Excludes private, quarantined, and root-deny repos, matching anonymous behavior. + let all_query = "{ reposPage(limit: 50) { nodes { name } hasNextPage } }"; + let unauth_response = authed(&schema, all_query, "did:key:zUnauthorized").await; + assert!( + unauth_response.errors.is_empty(), + "{:?}", + unauth_response.errors + ); + let unauth_json = unauth_response.data.into_json().unwrap(); + let unauth_names: Vec<&str> = unauth_json["reposPage"]["nodes"] + .as_array() + .unwrap() + .iter() + .filter_map(|n| n["name"].as_str()) + .collect(); + assert_eq!(unauth_names, vec!["odd-star", "open", "subtree"]); + let unauth_str = unauth_json.to_string(); + assert!(!unauth_str.contains("private")); + assert!(!unauth_str.contains("quarantined")); + assert!(!unauth_str.contains("root-deny")); + assert!(!unauth_str.contains("root-reader")); + assert!(!unauth_str.contains("root-tie")); + + // Reader caller gets root-reader and root-tie, but still excludes private, quarantined, root-deny. + let reader_response = authed(&schema, all_query, reader).await; + assert!( + reader_response.errors.is_empty(), + "{:?}", + reader_response.errors + ); + let reader_json = reader_response.data.into_json().unwrap(); + let reader_names: Vec<&str> = reader_json["reposPage"]["nodes"] + .as_array() + .unwrap() + .iter() + .filter_map(|n| n["name"].as_str()) + .collect(); + assert_eq!( + reader_names, + vec!["odd-star", "open", "root-reader", "subtree"] + ); + let reader_str = reader_json.to_string(); + assert!(!reader_str.contains("private")); + assert!(!reader_str.contains("quarantined")); + assert!(!reader_str.contains("root-deny")); + assert!(!reader_str.contains("root-tie")); } #[sqlx::test] @@ -419,6 +470,14 @@ mod tests { "nodes": [], "hasNextPage": false, "endCursor": null }) ); + let unauth = authed(&schema, &query, "did:key:zUnauthorized").await; + assert!(unauth.errors.is_empty()); + assert_eq!( + unauth.data.into_json().unwrap()["reposPage"], + serde_json::json!({ + "nodes": [], "hasNextPage": false, "endCursor": null + }) + ); } #[tokio::test] From 99426ca009a13a5e9813dca0260466ab10701a06 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Sat, 12 Sep 2026 02:27:19 -0400 Subject: [PATCH 07/20] fix(graphql): serve the advertised repository page range Charge rows once while retaining the base query cost, so a 200-row page fits the request budget. Fixup for ebafeae909364359466a8438a7cc9acec9970a2c. Refs #408 --- crates/gitlawb-node/src/graphql/mod.rs | 20 ++++++++++++++++++++ crates/gitlawb-node/src/graphql/query.rs | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/crates/gitlawb-node/src/graphql/mod.rs b/crates/gitlawb-node/src/graphql/mod.rs index 57702a72..356569b7 100644 --- a/crates/gitlawb-node/src/graphql/mod.rs +++ b/crates/gitlawb-node/src/graphql/mod.rs @@ -401,6 +401,26 @@ mod tests { } } + #[sqlx::test] + async fn production_repos_page_serves_the_documented_maximum(pool: sqlx::PgPool) { + let db = Arc::new(Db::for_testing(pool)); + db.run_migrations().await.unwrap(); + let schema = production_test_schema_with_db(db); + for selection in [ + "nodes { name }", + "nodes { name ownerDid } hasNextPage endCursor", + ] { + let response = schema + .execute(format!("{{ reposPage(limit: 200) {{ {selection} }} }}")) + .await; + assert!(response.errors.is_empty(), "{:?}", response.errors); + assert_eq!( + response.data.into_json().unwrap()["reposPage"]["nodes"], + serde_json::json!([]) + ); + } + } + #[tokio::test] async fn seven_root_aliases_are_accepted() { struct Root(Arc); diff --git a/crates/gitlawb-node/src/graphql/query.rs b/crates/gitlawb-node/src/graphql/query.rs index 91fe507a..fdad1148 100644 --- a/crates/gitlawb-node/src/graphql/query.rs +++ b/crates/gitlawb-node/src/graphql/query.rs @@ -67,7 +67,7 @@ impl QueryRoot { /// Bounded visible repositories ordered by owner and name. Continue with /// endCursor while hasNextPage is true. Pages are not a database snapshot. - #[graphql(complexity = "50 + (limit.clamp(1, 200) as usize) * child_complexity")] + #[graphql(complexity = "50 + (limit.clamp(1, 200) as usize) + child_complexity")] async fn repos_page( &self, ctx: &Context<'_>, From 0e5ea50c4fcd87602bfd35401367382195ac04b3 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Sat, 12 Sep 2026 02:27:19 -0400 Subject: [PATCH 08/20] test(graphql): cover every annotated root cost Reject alias amplification on every annotated task root and on small repository pages. Fixup for ebafeae909364359466a8438a7cc9acec9970a2c. Refs #408 --- crates/gitlawb-node/src/graphql/mod.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/crates/gitlawb-node/src/graphql/mod.rs b/crates/gitlawb-node/src/graphql/mod.rs index 356569b7..4490dc6b 100644 --- a/crates/gitlawb-node/src/graphql/mod.rs +++ b/crates/gitlawb-node/src/graphql/mod.rs @@ -379,22 +379,21 @@ mod tests { #[tokio::test] async fn production_limits_reject_mutation_aliases_and_large_lists() { let schema = production_test_schema(); - for field in [ - "claimTask(id: \"missing\", assigneeDid: \"did:key:test\") { id }", - "refUpdates(limit: 200) { repo }", - "tasks(limit: 200) { id }", - "reposPage(limit: 200) { nodes { name } }", + for (prefix, count, field) in [ + ("mutation", 8, "claimTask(id: \"missing\", assigneeDid: \"did:key:test\") { id }"), + ("mutation", 8, "createTask(delegatorDid: \"did:key:test\", input: { kind: \"test\", capability: \"test\" }) { id }"), + ("mutation", 8, "completeTask(id: \"missing\", byDid: \"did:key:test\", input: {}) { id }"), + ("mutation", 8, "failTask(id: \"missing\", byDid: \"did:key:test\", input: {}) { id }"), + ("query", 8, "task(id: \"missing\") { id }"), + ("query", 2, "refUpdates(limit: 200) { repo }"), + ("query", 2, "tasks(limit: 200) { id }"), + ("query", 2, "reposPage(limit: 200) { nodes { name } }"), + ("query", 8, "reposPage(limit: 1) { nodes { name } }"), ] { - let count = if field.starts_with("claim") { 8 } else { 2 }; let fields = (0..count) .map(|n| format!("r{n}: {field}")) .collect::>() .join(" "); - let prefix = if field.starts_with("claim") { - "mutation" - } else { - "query" - }; let response = schema.execute(format!("{prefix} {{ {fields} }}")).await; assert_eq!(response.errors.len(), 1, "{:?}", response.errors); assert_eq!(response.errors[0].message, "Query is too complex."); From 8d665c7c3550c6339b96fa9856309042eed3a2a8 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Sat, 12 Sep 2026 02:27:20 -0400 Subject: [PATCH 09/20] test(graphql): isolate the repository cursor length guard Use valid encoded cursor JSON so decoding errors cannot mask a missing length check. Fixup for ebafeae909364359466a8438a7cc9acec9970a2c. Refs #408 --- crates/gitlawb-node/src/graphql/query.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/gitlawb-node/src/graphql/query.rs b/crates/gitlawb-node/src/graphql/query.rs index fdad1148..c6fe9995 100644 --- a/crates/gitlawb-node/src/graphql/query.rs +++ b/crates/gitlawb-node/src/graphql/query.rs @@ -206,6 +206,7 @@ impl QueryRoot { #[cfg(test)] mod tests { use crate::db::{Db, ReceivedRefUpdate, RepoRecord}; + use base64::Engine; use chrono::Utc; use sqlx::PgPool; use std::sync::Arc; @@ -502,7 +503,15 @@ mod tests { || response.errors[0].message == "invalid repository cursor" ); } - assert!(super::parse_repo_cursor(&"a".repeat(4097)).is_err()); + // Valid base64 and valid cursor JSON: only the length guard rejects it. + let oversized = super::URL_SAFE_NO_PAD + .encode(serde_json::to_vec(&("did:key:reader", "a".repeat(3100))).unwrap()); + assert!(oversized.len() > 4096); + assert!(serde_json::from_slice::<(String, String)>( + &super::URL_SAFE_NO_PAD.decode(&oversized).unwrap() + ) + .is_ok()); + assert!(super::parse_repo_cursor(&oversized).is_err()); } async fn db(pool: PgPool) -> Arc { From 9d53e130d0786cf0c38157a517bcda103ec00a6f Mon Sep 17 00:00:00 2001 From: euxaristia Date: Sat, 12 Sep 2026 16:43:30 -0400 Subject: [PATCH 10/20] fix(node): pin GraphQL introspection depth and pagination contracts Raise maximum query depth to 14 to accept the canonical getIntrospectionQuery document, pin activity ordering for legacy repos queries, test keyset cursor traversal across owner boundaries, and pin the max-limit complexity contract for refUpdates and tasks. Refs #408, #446 --- crates/gitlawb-node/src/graphql/mod.rs | 207 +++++++++++++++++++---- crates/gitlawb-node/src/graphql/query.rs | 72 +++++++- 2 files changed, 242 insertions(+), 37 deletions(-) diff --git a/crates/gitlawb-node/src/graphql/mod.rs b/crates/gitlawb-node/src/graphql/mod.rs index 4490dc6b..0de17a64 100644 --- a/crates/gitlawb-node/src/graphql/mod.rs +++ b/crates/gitlawb-node/src/graphql/mod.rs @@ -18,8 +18,9 @@ pub type GitlawbSchema = Schema; // bounding validation work and the number of resolver selections per request. const GRAPHQL_MAX_COMPLEXITY: usize = 400; // The current public schema is shallow; this leaves headroom for composed -// clients without allowing recursively nested documents to grow unchecked. -const GRAPHQL_MAX_DEPTH: usize = 12; +// clients and the canonical getIntrospectionQuery without allowing +// recursively nested documents to grow unchecked. +const GRAPHQL_MAX_DEPTH: usize = 14; fn apply_query_limits( builder: SchemaBuilder, @@ -465,38 +466,176 @@ mod tests { #[tokio::test] async fn ordinary_schema_introspection_remains_available() { - let schema = - apply_query_limits(Schema::build(QueryRoot, EmptyMutation, EmptySubscription)).finish(); - let response = schema - .execute( - r#" - query IntrospectionQuery { - __schema { - queryType { + let schema = production_test_schema(); + let canonical_introspection = r#" + query IntrospectionQuery { + __schema { + queryType { name } + mutationType { name } + subscriptionType { name } + types { + ...FullType + } + directives { + name + description + locations + args { + ...InputValue + } + } + } + } + + fragment FullType on __Type { + kind + name + description + fields(includeDeprecated: true) { + name + description + args { + ...InputValue + } + type { + ...TypeRef + } + isDeprecated + deprecationReason + } + inputFields { + ...InputValue + } + interfaces { + ...TypeRef + } + enumValues(includeDeprecated: true) { + name + description + isDeprecated + deprecationReason + } + possibleTypes { + ...TypeRef + } + } + + fragment InputValue on __InputValue { + name + description + type { ...TypeRef } + defaultValue + } + + fragment TypeRef on __Type { + kind + name + ofType { + kind + name + ofType { + kind + name + ofType { + kind + name + ofType { + kind + name + ofType { + kind + name + ofType { + kind + name + ofType { + kind name - fields { - name - type { - kind - name - ofType { kind name } - } - } + } } + } } + } } - "#, - ) - .await; - + } + } + "#; + let response = schema.execute(canonical_introspection).await; assert!( response.errors.is_empty(), - "graphql errors: {:?}", + "canonical introspection query failed against production schema: {:?}", response.errors ); assert_ne!(response.data, Value::Null); } + #[tokio::test] + async fn ref_updates_and_tasks_single_alias_max_limit_complexity_contract() { + let pool = sqlx::postgres::PgPoolOptions::new() + .connect_lazy("postgres://localhost/unused") + .unwrap(); + pool.close().await; + let (ref_tx, _) = tokio::sync::broadcast::channel(16); + let (task_tx, _) = tokio::sync::broadcast::channel(16); + let schema = build_schema(Arc::new(Db::for_testing(pool)), ref_tx, task_tx); + + // refUpdates(limit: 200): + // Single-field selection has child_complexity 1 -> cost 50 + 200 * 1 = 250 <= 400. + // It passes validation and reaches the resolver (which yields db error on lazy pool). + let single_ref = schema.execute("{ refUpdates(limit: 200) { repo } }").await; + assert!( + !single_ref + .errors + .iter() + .any(|e| e.message == "Query is too complex."), + "single-field refUpdates at max limit 200 must pass complexity validation: {:?}", + single_ref.errors + ); + assert!( + single_ref + .errors + .iter() + .any(|e| e.message == GRAPHQL_DB_ERROR_MESSAGE), + "single-field refUpdates at max limit 200 must reach the resolver: {:?}", + single_ref.errors + ); + + // Two-field selection has child_complexity 2 -> cost 50 + 200 * 2 = 450 > 400 (rejected before resolver). + let multi_ref = schema + .execute("{ refUpdates(limit: 200) { repo refName } }") + .await; + assert_eq!(multi_ref.data, async_graphql::Value::Null); + assert_eq!(multi_ref.errors.len(), 1); + assert_eq!(multi_ref.errors[0].message, "Query is too complex."); + + // tasks(limit: 200): + // Single-field selection has child_complexity 1 -> cost 50 + 200 * 1 = 250 <= 400. + // It passes validation and reaches the resolver. + let single_task = schema.execute("{ tasks(limit: 200) { id } }").await; + assert!( + !single_task + .errors + .iter() + .any(|e| e.message == "Query is too complex."), + "single-field tasks at max limit 200 must pass complexity validation: {:?}", + single_task.errors + ); + assert!( + single_task + .errors + .iter() + .any(|e| e.message == GRAPHQL_DB_ERROR_MESSAGE), + "single-field tasks at max limit 200 must reach the resolver: {:?}", + single_task.errors + ); + + // Two-field selection has child_complexity 2 -> cost 50 + 200 * 2 = 450 > 400 (rejected before resolver). + let multi_task = schema.execute("{ tasks(limit: 200) { id status } }").await; + assert_eq!(multi_task.data, async_graphql::Value::Null); + assert_eq!(multi_task.errors.len(), 1); + assert_eq!(multi_task.errors[0].message, "Query is too complex."); + } + #[derive(Clone, Copy)] struct Nested; @@ -522,7 +661,7 @@ mod tests { } #[tokio::test] - async fn query_depth_limit_accepts_twelve_and_rejects_thirteen() { + async fn query_depth_limit_accepts_fourteen_and_rejects_fifteen() { let calls = Arc::new(AtomicUsize::new(0)); let schema = apply_query_limits(Schema::build( CountingQuery(Arc::clone(&calls)), @@ -557,22 +696,22 @@ mod tests { #[tokio::test] async fn production_schema_enforces_depth_limit() { let schema = production_test_schema(); - // Depth 12 is accepted through the production build_schema path. - let of_types_12 = (0..7).fold("name".to_string(), |acc, _| format!("ofType {{ {acc} }}")); - let query_12 = - format!("{{ __schema {{ types {{ fields {{ type {{ {of_types_12} }} }} }} }} }}"); - let accepted = schema.execute(&query_12).await; + // Depth 14 is accepted through the production build_schema path. + let of_types_14 = (0..9).fold("name".to_string(), |acc, _| format!("ofType {{ {acc} }}")); + let query_14 = + format!("{{ __schema {{ types {{ fields {{ type {{ {of_types_14} }} }} }} }} }}"); + let accepted = schema.execute(&query_14).await; assert!(accepted.errors.is_empty(), "{:?}", accepted.errors); - // Construct an introspection query of depth 13 using __schema. + // Construct an introspection query of depth 15 using __schema. // 1: __schema // 2: types // 3: fields // 4: type - // 5..12: ofType (8 times) - // 13: name - // Total depth = 13 > GRAPHQL_MAX_DEPTH (12). - let of_types = (0..8).fold("name".to_string(), |acc, _| format!("ofType {{ {acc} }}")); + // 5..14: ofType (10 times) + // 15: name + // Total depth = 15 > GRAPHQL_MAX_DEPTH (14). + let of_types = (0..10).fold("name".to_string(), |acc, _| format!("ofType {{ {acc} }}")); let query = format!("{{ __schema {{ types {{ fields {{ type {{ {of_types} }} }} }} }} }}"); let rejected = schema.execute(&query).await; diff --git a/crates/gitlawb-node/src/graphql/query.rs b/crates/gitlawb-node/src/graphql/query.rs index c6fe9995..90ffd872 100644 --- a/crates/gitlawb-node/src/graphql/query.rs +++ b/crates/gitlawb-node/src/graphql/query.rs @@ -274,11 +274,13 @@ mod tests { #[sqlx::test] async fn repos_legacy_accepts_exactly_the_visible_bound(pool: PgPool) { let db = db(pool).await; + let base_time = Utc::now(); for index in 0..crate::db::MAX_VISIBLE_REPO_PAGE_SIZE { let name = format!("repo-{index:03}"); - db.create_repo(&repo(&name, OWNER, &name, true)) - .await - .unwrap(); + let mut r = repo(&name, OWNER, &name, true); + // Distinct timestamps: repo-000 is oldest, repo-199 is newest. + r.updated_at = base_time + chrono::Duration::seconds(index as i64); + db.create_repo(&r).await.unwrap(); } db.create_repo(&repo("hidden", OWNER, "hidden", false)) .await @@ -295,6 +297,70 @@ mod tests { !names.contains(&"hidden"), "hidden repo must be excluded from legacy repos response" ); + let expected_names: Vec = (0..crate::db::MAX_VISIBLE_REPO_PAGE_SIZE) + .rev() + .map(|index| format!("repo-{index:03}")) + .collect(); + assert_eq!( + names, + expected_names + .iter() + .map(|s| s.as_str()) + .collect::>(), + "legacy repos must retain activity ordering (most recently updated first)" + ); + } + + #[sqlx::test] + async fn repos_page_pagination_crosses_owner_boundary(pool: PgPool) { + let db = db(pool).await; + let owner1 = "did:key:z6MkaOwner1"; + let owner2 = "did:key:z6MkbOwner2"; + db.create_repo(&repo("r1", owner1, "a-repo", true)) + .await + .unwrap(); + db.create_repo(&repo("r2", owner1, "b-repo", true)) + .await + .unwrap(); + db.create_repo(&repo("r3", owner2, "c-repo", true)) + .await + .unwrap(); + db.create_repo(&repo("r4", owner2, "d-repo", true)) + .await + .unwrap(); + + let schema = schema(db); + let page1_resp = anon( + &schema, + "{ reposPage(limit: 2) { nodes { name ownerDid } hasNextPage endCursor } }", + ) + .await; + assert!(page1_resp.errors.is_empty(), "{:?}", page1_resp.errors); + let p1 = page1_resp.data.into_json().unwrap()["reposPage"].clone(); + assert_eq!(p1["hasNextPage"], true); + assert_eq!( + p1["nodes"], + serde_json::json!([ + {"name": "a-repo", "ownerDid": owner1}, + {"name": "b-repo", "ownerDid": owner1}, + ]) + ); + let cursor = p1["endCursor"].as_str().unwrap(); + + let page2_query = format!( + "{{ reposPage(limit: 2, after: \"{cursor}\") {{ nodes {{ name ownerDid }} hasNextPage endCursor }} }}" + ); + let page2_resp = anon(&schema, &page2_query).await; + assert!(page2_resp.errors.is_empty(), "{:?}", page2_resp.errors); + let p2 = page2_resp.data.into_json().unwrap()["reposPage"].clone(); + assert_eq!(p2["hasNextPage"], false); + assert_eq!( + p2["nodes"], + serde_json::json!([ + {"name": "c-repo", "ownerDid": owner2}, + {"name": "d-repo", "ownerDid": owner2}, + ]) + ); } #[sqlx::test] From a06cb43e916c26ab3d40e4d48655b8df3a94d38c Mon Sep 17 00:00:00 2001 From: euxaristia Date: Sat, 12 Sep 2026 20:56:33 -0400 Subject: [PATCH 11/20] test(graphql): anti-correlate legacy repos sort and pin keyset tuple order Anti-correlate updated_at timestamps against name and insertion order so only ordering by updated_at DESC satisfies the legacy repos test. Seed owner2 repos with name sorting before owner1 cursor to ensure (owner_did, name) tuple ordering is required across page boundaries. Refs #408 --- crates/gitlawb-node/src/graphql/query.rs | 31 +++++++++++++++--------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/crates/gitlawb-node/src/graphql/query.rs b/crates/gitlawb-node/src/graphql/query.rs index 90ffd872..a98e95ef 100644 --- a/crates/gitlawb-node/src/graphql/query.rs +++ b/crates/gitlawb-node/src/graphql/query.rs @@ -275,11 +275,14 @@ mod tests { async fn repos_legacy_accepts_exactly_the_visible_bound(pool: PgPool) { let db = db(pool).await; let base_time = Utc::now(); - for index in 0..crate::db::MAX_VISIBLE_REPO_PAGE_SIZE { + let total = crate::db::MAX_VISIBLE_REPO_PAGE_SIZE; + for index in 0..total { let name = format!("repo-{index:03}"); let mut r = repo(&name, OWNER, &name, true); - // Distinct timestamps: repo-000 is oldest, repo-199 is newest. - r.updated_at = base_time + chrono::Duration::seconds(index as i64); + // Anti-correlate updated_at with name, creation order, and index + // so only ordering by updated_at DESC can satisfy the expectation. + let offset = (index * 37) % total; + r.updated_at = base_time + chrono::Duration::seconds(offset as i64); db.create_repo(&r).await.unwrap(); } db.create_repo(&repo("hidden", OWNER, "hidden", false)) @@ -291,14 +294,16 @@ mod tests { .as_array() .unwrap() .clone(); - assert_eq!(repos.len(), crate::db::MAX_VISIBLE_REPO_PAGE_SIZE); + assert_eq!(repos.len(), total); let names: Vec<_> = repos.iter().filter_map(|r| r["name"].as_str()).collect(); assert!( !names.contains(&"hidden"), "hidden repo must be excluded from legacy repos response" ); - let expected_names: Vec = (0..crate::db::MAX_VISIBLE_REPO_PAGE_SIZE) - .rev() + let mut expected_indices = (0..total).collect::>(); + expected_indices.sort_by_key(|&idx| std::cmp::Reverse((idx * 37) % total)); + let expected_names: Vec = expected_indices + .into_iter() .map(|index| format!("repo-{index:03}")) .collect(); assert_eq!( @@ -316,16 +321,18 @@ mod tests { let db = db(pool).await; let owner1 = "did:key:z6MkaOwner1"; let owner2 = "did:key:z6MkbOwner2"; - db.create_repo(&repo("r1", owner1, "a-repo", true)) + // Seed owner2 repos with a name ("a-repo") that sorts before owner1's cursor name ("z-repo") + // to ensure (owner_did, name) tuple ordering is required across page boundaries. + db.create_repo(&repo("r1", owner1, "b-repo", true)) .await .unwrap(); - db.create_repo(&repo("r2", owner1, "b-repo", true)) + db.create_repo(&repo("r2", owner1, "z-repo", true)) .await .unwrap(); - db.create_repo(&repo("r3", owner2, "c-repo", true)) + db.create_repo(&repo("r3", owner2, "a-repo", true)) .await .unwrap(); - db.create_repo(&repo("r4", owner2, "d-repo", true)) + db.create_repo(&repo("r4", owner2, "c-repo", true)) .await .unwrap(); @@ -341,8 +348,8 @@ mod tests { assert_eq!( p1["nodes"], serde_json::json!([ - {"name": "a-repo", "ownerDid": owner1}, {"name": "b-repo", "ownerDid": owner1}, + {"name": "z-repo", "ownerDid": owner1}, ]) ); let cursor = p1["endCursor"].as_str().unwrap(); @@ -357,8 +364,8 @@ mod tests { assert_eq!( p2["nodes"], serde_json::json!([ + {"name": "a-repo", "ownerDid": owner2}, {"name": "c-repo", "ownerDid": owner2}, - {"name": "d-repo", "ownerDid": owner2}, ]) ); } From e91b1ea76d7d0248b5459e22acf65132ba34787e Mon Sep 17 00:00:00 2001 From: euxaristia Date: Sun, 13 Sep 2026 23:38:40 -0400 Subject: [PATCH 12/20] test(graphql): pin normalized owner ordering across page cursors Seed an earlier normalized did:web owner whose repository name sorts past the cursor, then assert all three pages. Both an overbroad OR predicate and raw owner ordering now fail the pagination regression. Refs #408 --- crates/gitlawb-node/src/graphql/query.rs | 29 ++++++++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/crates/gitlawb-node/src/graphql/query.rs b/crates/gitlawb-node/src/graphql/query.rs index a98e95ef..666716eb 100644 --- a/crates/gitlawb-node/src/graphql/query.rs +++ b/crates/gitlawb-node/src/graphql/query.rs @@ -321,8 +321,15 @@ mod tests { let db = db(pool).await; let owner1 = "did:key:z6MkaOwner1"; let owner2 = "did:key:z6MkbOwner2"; - // Seed owner2 repos with a name ("a-repo") that sorts before owner1's cursor name ("z-repo") - // to ensure (owner_did, name) tuple ordering is required across page boundaries. + let earlier_owner = "did:web:example.com"; + // This owner sorts first only after did:key normalization. Its name + // sorts past the first cursor, so an unqualified OR name predicate + // would incorrectly serve it again on the next page. + db.create_repo(&repo("r0", earlier_owner, "zz-repo", true)) + .await + .unwrap(); + // Owner2's a-repo sorts before owner1's names, requiring the normalized + // (owner, name) tuple both within pages and across page boundaries. db.create_repo(&repo("r1", owner1, "b-repo", true)) .await .unwrap(); @@ -348,8 +355,8 @@ mod tests { assert_eq!( p1["nodes"], serde_json::json!([ + {"name": "zz-repo", "ownerDid": earlier_owner}, {"name": "b-repo", "ownerDid": owner1}, - {"name": "z-repo", "ownerDid": owner1}, ]) ); let cursor = p1["endCursor"].as_str().unwrap(); @@ -360,14 +367,26 @@ mod tests { let page2_resp = anon(&schema, &page2_query).await; assert!(page2_resp.errors.is_empty(), "{:?}", page2_resp.errors); let p2 = page2_resp.data.into_json().unwrap()["reposPage"].clone(); - assert_eq!(p2["hasNextPage"], false); + assert_eq!(p2["hasNextPage"], true); assert_eq!( p2["nodes"], serde_json::json!([ + {"name": "z-repo", "ownerDid": owner1}, {"name": "a-repo", "ownerDid": owner2}, - {"name": "c-repo", "ownerDid": owner2}, ]) ); + let cursor = p2["endCursor"].as_str().unwrap(); + let page3_query = format!( + "{{ reposPage(limit: 2, after: \"{cursor}\") {{ nodes {{ name ownerDid }} hasNextPage endCursor }} }}" + ); + let page3_resp = anon(&schema, &page3_query).await; + assert!(page3_resp.errors.is_empty(), "{:?}", page3_resp.errors); + let p3 = page3_resp.data.into_json().unwrap()["reposPage"].clone(); + assert_eq!(p3["hasNextPage"], false); + assert_eq!( + p3["nodes"], + serde_json::json!([{"name": "c-repo", "ownerDid": owner2}]) + ); } #[sqlx::test] From f564ec0013c1db0561d6cd8ef7b64b3335f1b2d2 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Sun, 13 Sep 2026 23:39:27 -0400 Subject: [PATCH 13/20] test(node): make Windows regression fixtures portable Gate Unix-only helpers with their callers, replace shell timing shims with native Rust fixtures, and normalize rooted fixture paths without a drive prefix. Distinguish admission rejection from a closed database by error code, since both correctly return HTTP 503. Refs #408 --- crates/gitlawb-node/src/api/ipfs.rs | 18 ++-- crates/gitlawb-node/src/api/repos.rs | 14 +++ crates/gitlawb-node/src/git/repo_store.rs | 4 +- crates/gitlawb-node/src/ipfs_pin.rs | 1 + crates/gitlawb-node/src/pinata.rs | 1 + crates/gitlawb-node/src/sync.rs | 24 ++++- crates/gitlawb-node/src/test_git_shim.rs | 75 ++++++++++++++++ crates/gitlawb-node/src/test_support.rs | 88 +++++-------------- .../gitlawb-node/tests/fixtures/git_shim.rs | 59 +++++++++++++ 9 files changed, 209 insertions(+), 75 deletions(-) create mode 100644 crates/gitlawb-node/src/test_git_shim.rs create mode 100644 crates/gitlawb-node/tests/fixtures/git_shim.rs diff --git a/crates/gitlawb-node/src/api/ipfs.rs b/crates/gitlawb-node/src/api/ipfs.rs index 92d12980..40139804 100644 --- a/crates/gitlawb-node/src/api/ipfs.rs +++ b/crates/gitlawb-node/src/api/ipfs.rs @@ -8338,6 +8338,7 @@ mod tests { #[tokio::test] async fn get_by_cid_per_source_cap_sheds_same_source_admits_other() { let mut state = crate::test_support::test_state_lazy(); + state.db.pool().close().await; // Global pool has room; the per-source cap is 1. state.git_ipfs_walk_semaphore = Arc::new(Semaphore::new(8)); state.git_ipfs_walk_per_caller = crate::rate_limit::PerCallerConcurrency::new(1, 100); @@ -8364,16 +8365,23 @@ mod tests { "a source at its per-source /ipfs walk cap must shed 503 with global capacity free" ); + let bytes = axum::body::to_bytes(resp.into_body(), 1024).await.unwrap(); + let body: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(body["error"], "overloaded"); + // A DIFFERENT source is NOT shed by the per-source cap: it clears admission and - // proceeds (then errors on the lazy DB, which is not a 503). + // proceeds to the closed DB, which has a distinct db_unavailable error code. let resp = ipfs_router(state) .oneshot(get_cid(&cid, Some(other))) .await .unwrap(); - assert_ne!( - resp.status(), - StatusCode::SERVICE_UNAVAILABLE, - "a different source must not be shed by the per-source cap" + assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE); + let bytes = axum::body::to_bytes(resp.into_body(), 1024).await.unwrap(); + let body: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!( + body["error"], + crate::error::DB_UNAVAILABLE_CODE, + "a different source must clear admission and reach the closed database" ); } diff --git a/crates/gitlawb-node/src/api/repos.rs b/crates/gitlawb-node/src/api/repos.rs index 4e327c42..847fa7e1 100644 --- a/crates/gitlawb-node/src/api/repos.rs +++ b/crates/gitlawb-node/src/api/repos.rs @@ -7124,6 +7124,7 @@ mod tests { /// A pkt-line receive-pack body carrying one branch-create ref update, so the /// handler's post-receive tail resolves a non-empty new-tip set (the delta /// scan's git stages run). + #[cfg(unix)] fn ref_update_body(new_sha: &str) -> axum::body::Bytes { let line = format!("{ZERO_SHA} {new_sha} refs/heads/main"); axum::body::Bytes::from(format!("{:04x}{}0000", line.len() + 4, line)) @@ -9581,6 +9582,7 @@ mod tests { ) } + #[cfg(unix)] fn f2a_log(log: &std::path::Path) -> String { std::fs::read_to_string(log).unwrap_or_default() } @@ -9588,6 +9590,7 @@ mod tests { /// Withheld-walk children run so far. `ls-tree` is the walk's signature child /// (`blob_paths` lists every reachable commit's tree); the delta scan and the /// full-scan fallback use `rev-list` / `cat-file` instead. + #[cfg(unix)] fn f2a_walks(log: &std::path::Path) -> usize { f2a_log(log) .lines() @@ -9599,6 +9602,7 @@ mod tests { /// the withheld walk actually runs rather than taking the no-rule shortcut). /// The repo's on-disk path is passed to the tail directly, so no repo_store or /// receive-pack plumbing is involved. + #[cfg(unix)] async fn f2a_state( pool: sqlx::PgPool, git_bin: &str, @@ -9630,6 +9634,7 @@ mod tests { (state, rec) } + #[cfg(unix)] fn f2a_update(ref_name: &str, new_sha: &str) -> Vec { vec![RefUpdate { old_sha: ZERO_SHA.to_string(), @@ -9638,6 +9643,7 @@ mod tests { }] } + #[cfg(unix)] const F2A_PUSHER: &str = "did:key:z6MkF2aPusherAAAAAAAAAAAAAAAAAAAAAAAAAA"; /// Scenario 1 (the finding). A second rapid push to the same repo coalesces @@ -9711,6 +9717,7 @@ mod tests { } /// Poll `cond` until it holds, with a bound so a regression fails the test /// rather than hanging the suite. + #[cfg(unix)] async fn f2a_wait_for(mut cond: impl FnMut() -> bool, what: &str) { let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20); while !cond() { @@ -9725,6 +9732,7 @@ mod tests { /// A `rev-list --objects` line names the tips a DELTA scan was asked to resolve, /// so it attributes that scan to one push's tips. The withheld walk's own /// `rev-list --all` / `ls-tree` lines never carry a tip as an argument this way. + #[cfg(unix)] fn f2a_delta_scanned(log: &std::path::Path, tip: &str) -> bool { f2a_log(log) .lines() @@ -9871,6 +9879,7 @@ mod tests { /// Mount a Pinata upload endpoint that assigns every object the same CID, and /// point the state at it. Returns the server (kept alive by the caller) and CID. + #[cfg(unix)] async fn f2a_pinata(state: &mut AppState) -> (mockito::ServerGuard, String) { let cid = "bafyf2acoalescedmapping".to_string(); let mut server = mockito::Server::new_async().await; @@ -9890,6 +9899,7 @@ mod tests { /// Poll the branch to CID table until the push's mapping lands (the Pinata /// worker is detached), bounded so a regression fails rather than hangs. + #[cfg(unix)] async fn f2a_wait_for_branch_cid( db: &crate::db::Db, slug: &str, @@ -9909,6 +9919,7 @@ mod tests { } } + #[cfg(unix)] fn f2a_slug(rec: &crate::db::RepoRecord) -> String { format!( "{}/{}", @@ -10042,6 +10053,7 @@ mod tests { // and `z6p2fail`), and owner-only push is on by default, so the identity has to // follow the repo each push targets rather than being fixed for both. + #[cfg(unix)] fn p2_push( state: &AppState, owner: &str, @@ -10060,6 +10072,7 @@ mod tests { ) } + #[cfg(unix)] fn p2_logged(log: &std::path::Path, prefix: &str) -> bool { f2a_log(log).lines().any(|l| l.starts_with(prefix)) } @@ -10359,6 +10372,7 @@ mod tests { /// runs exactly one `rev-list --all`, so this counts the walks that were attempted /// (the `ls-tree` counter above cannot: a walk whose enumeration fails never gets /// to `ls-tree`). + #[cfg(unix)] fn f2b_walk_attempts(log: &std::path::Path) -> usize { f2a_log(log) .lines() diff --git a/crates/gitlawb-node/src/git/repo_store.rs b/crates/gitlawb-node/src/git/repo_store.rs index 45820746..2a465c59 100644 --- a/crates/gitlawb-node/src/git/repo_store.rs +++ b/crates/gitlawb-node/src/git/repo_store.rs @@ -83,7 +83,7 @@ impl RepoStore { /// Test-only: every guard from this store parks in `release` right before the /// `pg_advisory_unlock` await, until `gate` is notified. Dropping the future /// while it is parked reproduces a client disconnect inside `release`. - #[cfg(test)] + #[cfg(all(test, unix))] pub fn with_pre_unlock_gate(mut self, gate: Arc) -> Self { self.pre_unlock_gate = Some(gate); self @@ -107,7 +107,7 @@ impl RepoStore { /// Test-only: how many write guards from this store have reached the Tigris upload /// site. See [`RepoStore::upload_site_reached`]. - #[cfg(test)] + #[cfg(all(test, unix))] pub fn tigris_upload_site_reached(&self) -> usize { self.upload_site_reached .load(std::sync::atomic::Ordering::SeqCst) diff --git a/crates/gitlawb-node/src/ipfs_pin.rs b/crates/gitlawb-node/src/ipfs_pin.rs index 5d4579a3..9b0e929d 100644 --- a/crates/gitlawb-node/src/ipfs_pin.rs +++ b/crates/gitlawb-node/src/ipfs_pin.rs @@ -2534,6 +2534,7 @@ mod tests { /// type probe carries its oid on stdin rather than in argv, so an oid appears in /// the log only once an object has already got past its probe, and a healthy /// object costs two invocations to a faulting one's one. + #[cfg(unix)] fn objects_attempted(log: &std::path::Path) -> usize { std::fs::read_to_string(log) .unwrap_or_default() diff --git a/crates/gitlawb-node/src/pinata.rs b/crates/gitlawb-node/src/pinata.rs index 14f1d582..d3e29217 100644 --- a/crates/gitlawb-node/src/pinata.rs +++ b/crates/gitlawb-node/src/pinata.rs @@ -911,6 +911,7 @@ mod tests { /// probe carries its oid on stdin rather than in argv, so an oid appears in the log only /// once an object has already got past its probe, and a healthy object costs two /// invocations to a faulting one's one. + #[cfg(unix)] fn objects_attempted(log: &std::path::Path) -> usize { std::fs::read_to_string(log) .unwrap_or_default() diff --git a/crates/gitlawb-node/src/sync.rs b/crates/gitlawb-node/src/sync.rs index 0ed4a9f9..62fa898a 100644 --- a/crates/gitlawb-node/src/sync.rs +++ b/crates/gitlawb-node/src/sync.rs @@ -1250,6 +1250,18 @@ mod tests { .await; } + /// Use a rooted path without a Windows drive prefix in the remote fixture. + /// The destination still resolves on the temp directory's current drive, + /// while the remote's relative path contains no illegal colon component. + fn absolute_slug_path(path: &Path) -> String { + assert!(path.is_absolute()); + path.components() + .filter(|part| !matches!(part, std::path::Component::Prefix(_))) + .collect::() + .to_string_lossy() + .replace('\\', "/") + } + #[sqlx::test] async fn process_batch_rejects_slug_escaping_repos_dir(pool: PgPool) { // The verified escape from #272: `PathBuf::join` discards everything @@ -1265,12 +1277,13 @@ mod tests { // fails on but leaves the parent it created, so assert on both. let outside = TempDir::new().unwrap(); let escape_dir = outside.path().join("nest"); - let slug = format!("a/{}/escape", escape_dir.display()); + let escape_path = absolute_slug_path(&escape_dir); + let slug = format!("a/{escape_path}/escape"); let escape_target = escape_dir.join("escape.git"); // Serve the composed URL for real, so a run without the guard genuinely // clones outside the root rather than merely failing at git. - let rel = format!("a{}/escape", escape_dir.display()); + let rel = format!("a{escape_path}/escape"); let (_remote, peer_url) = rooted_remote(&[&rel]); let did = "did:key:z6MkAttacker"; @@ -1390,6 +1403,7 @@ mod tests { // ── canonical containment before the git call (issue #272) ─────────────── /// Every ref in `repo`, as one string, for a before/after comparison. + #[cfg(unix)] fn refs_of(repo: &Path) -> String { let out = Command::new("git") .args(["-C", repo.to_str().unwrap(), "for-each-ref"]) @@ -1722,6 +1736,7 @@ mod tests { /// starvation tests is fixed rather than dependent on how fast the loop /// runs. Two rows enqueued in the same microsecond would otherwise order /// arbitrarily. + #[cfg(unix)] async fn enqueue_at(db: &Db, pool: &PgPool, repo: &str, did: &str, enqueued_at: &str) { enqueue(db, repo, did).await; sqlx::query("UPDATE sync_queue SET enqueued_at = $1 WHERE repo = $2") @@ -1915,7 +1930,8 @@ mod tests { let outside = TempDir::new().unwrap(); let escape_dir = outside.path().join("nest"); - let slug = format!("a/{}/gitlawb-probe", escape_dir.display()); + let escape_path = absolute_slug_path(&escape_dir); + let slug = format!("a/{escape_path}/gitlawb-probe"); let escape_target = escape_dir.join("gitlawb-probe.git"); // Two things this fixture must get right or the test is green for the @@ -1926,7 +1942,7 @@ mod tests { // run without the guard genuinely clones outside the root instead of // just failing at git. Db::upsert_peer cannot seed this row: it gates on // is_public_http_url, which rejects file://. - let rel = format!("a{}/gitlawb-probe", escape_dir.display()); + let rel = format!("a{escape_path}/gitlawb-probe"); let (_remote, peer_url) = rooted_remote(&[&rel]); let did = "did:key:z6MkAttacker"; seed_local_peer(&pool, did, &peer_url).await; diff --git a/crates/gitlawb-node/src/test_git_shim.rs b/crates/gitlawb-node/src/test_git_shim.rs new file mode 100644 index 00000000..a53b05d6 --- /dev/null +++ b/crates/gitlawb-node/src/test_git_shim.rs @@ -0,0 +1,75 @@ +//! Native Git fixtures shared by timing tests on Unix and Windows. + +use std::ops::Deref; +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; + +/// The fixture delays or hangs in its own process, so the production watchdog +/// can terminate it without depending on a shell or descendant process support. +pub(super) enum Behavior<'a> { + Delay(u64), + Hang, + HangOid(&'a str), + HangRepo(&'a str), + LogTypes(&'a Path), +} + +pub(super) struct GitShim { + _directory: tempfile::TempDir, + executable: PathBuf, +} + +impl Deref for GitShim { + type Target = Path; + + fn deref(&self) -> &Path { + &self.executable + } +} + +pub(super) fn create(name: &str, behavior: Behavior<'_>) -> GitShim { + static COMPILED: OnceLock = OnceLock::new(); + let compiled = COMPILED.get_or_init(|| { + let directory = tempfile::tempdir().expect("native Git fixture directory"); + let source = directory.path().join("git_shim.rs"); + std::fs::write(&source, include_str!("../tests/fixtures/git_shim.rs")) + .expect("write native Git fixture source"); + let executable = directory + .path() + .join(format!("git-shim{}", std::env::consts::EXE_SUFFIX)); + let output = + std::process::Command::new(std::env::var_os("RUSTC").unwrap_or_else(|| "rustc".into())) + .arg("--edition=2021") + .arg("-Dwarnings") + .arg(&source) + .arg("-o") + .arg(executable) + .output() + .expect("compile native Git fixture with the installed Rust toolchain"); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + directory + }); + let directory = tempfile::Builder::new() + .prefix(name) + .tempdir() + .expect("Git fixture instance"); + let file_name = format!("git-shim{}", std::env::consts::EXE_SUFFIX); + let executable = directory.path().join(&file_name); + std::fs::copy(compiled.path().join(file_name), &executable).expect("copy native Git fixture"); + let config = match behavior { + Behavior::Delay(milliseconds) => format!("delay\n{milliseconds}"), + Behavior::Hang => "hang\n".to_owned(), + Behavior::HangOid(oid) => format!("hang-oid\n{oid}"), + Behavior::HangRepo(repo) => format!("hang-repo\n{repo}"), + Behavior::LogTypes(path) => format!("log-types\n{}", path.display()), + }; + std::fs::write(directory.path().join("config"), config).expect("configure native Git fixture"); + GitShim { + _directory: directory, + executable, + } +} diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 430c0600..bd27f637 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -28,6 +28,9 @@ use gitlawb_core::identity::Keypair; use crate::auth::AuthenticatedDid; use crate::state::AppState; +#[path = "test_git_shim.rs"] +mod git_shim; + /// Build an [`AppState`] over a real, migrated Postgres pool (from `#[sqlx::test]`). /// Runs the schema migrations first, because the per-test database starts empty. /// @@ -427,11 +430,12 @@ mod tests { /// #174 (SC1, load-bearing): a saturated READ pool must NOT shed an /// authenticated push — the write pool is a separate budget. Read pool at zero, /// write pool with capacity: the push proceeds PAST admission (it then errors on - /// the placeholder DB, but crucially it is not a 503). Route git-receive-pack + /// the closed DB, with db_unavailable rather than overloaded). Route git-receive-pack /// back to the read pool and this goes red — that is the isolation proof. #[tokio::test] async fn git_receive_pack_not_shed_by_exhausted_read_pool() { let mut state = test_state_lazy(); + state.db.pool().close().await; // Read pool exhausted as if a flood of anonymous clones held every slot. state.git_read_semaphore = Arc::new(tokio::sync::Semaphore::new(0)); // Write pool keeps its default capacity from test_state_lazy. @@ -453,10 +457,13 @@ mod tests { .await .unwrap(); - assert_ne!( - resp.status(), - StatusCode::SERVICE_UNAVAILABLE, - "an exhausted READ pool must not shed a push — the write pool is a separate budget (#174)" + assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE); + let bytes = axum::body::to_bytes(resp.into_body(), 1024).await.unwrap(); + let body: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!( + body["error"], + crate::error::DB_UNAVAILABLE_CODE, + "the push must clear admission and reach the deliberately closed database" ); } @@ -7789,14 +7796,10 @@ mod tests { seed_legacy_pin(&pool, &bare, &fx.public_oid, Some(&repo.id)).await; // A git that takes 300ms per invocation (the read makes two: type, then content). - let slow_git = std::env::temp_dir().join(format!("gl-slow-git-{short}")); - std::fs::write(&slow_git, "#!/bin/sh\nsleep 0.3\nexec git \"$@\"\n").expect("write shim"); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&slow_git, std::fs::Permissions::from_mode(0o755)) - .expect("chmod shim"); - } + let slow_git = git_shim::create( + &format!("gl-slow-git-{short}"), + git_shim::Behavior::Delay(300), + ); let ticks = std::sync::Arc::new(AtomicUsize::new(0)); let ticker = { @@ -8572,19 +8575,6 @@ mod tests { ); } - /// Write an executable `git` stand-in and return its path. - fn write_git_shim(name: &str, script: &str) -> std::path::PathBuf { - let path = std::env::temp_dir().join(name); - std::fs::write(&path, script).expect("write the git shim"); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)) - .expect("chmod the git shim"); - } - path - } - /// F6 scenario 1 (#173 round 13): one hung candidate must not starve the rows behind /// it in the same pass. `DiscoveryCtx` is loaded once per pass, so before the per-row /// slice every source-less row in a pass shared ONE deadline: the first row's wedged @@ -8624,23 +8614,9 @@ mod tests { // The type stage feeds the oid on STDIN (`cat-file --batch-check`) and the // content stage puts it in argv, so the stand-in has to look in both places. - let git_bin = write_git_shim( + let git_bin = git_shim::create( &format!("gl-hung-git-{short}"), - &format!( - "#!/bin/sh\n\ - if [ \"$2\" = \"--batch-check\" ]; then\n\ - \x20 oid=$(cat)\n\ - \x20 case \"$oid\" in\n\ - \x20 {hung_oid}) sleep 30; exit 1 ;;\n\ - \x20 esac\n\ - \x20 printf '%s\\n' \"$oid\" | git \"$@\"\n\ - \x20 exit $?\n\ - fi\n\ - case \"$*\" in\n\ - \x20 *{hung_oid}*) sleep 30; exit 1 ;;\n\ - esac\n\ - exec git \"$@\"\n" - ), + git_shim::Behavior::HangOid(&hung_oid), ); let stats = tokio::time::timeout( @@ -8725,10 +8701,7 @@ mod tests { // Wedges on every invocation, so no row can ever be repaired and the only // question left is what each one COSTS. - let git_bin = write_git_shim( - &format!("gl-spent-git-{short}"), - "#!/bin/sh\nsleep 30\nexit 1\n", - ); + let git_bin = git_shim::create(&format!("gl-spent-git-{short}"), git_shim::Behavior::Hang); let stats = tokio::time::timeout( std::time::Duration::from_secs(60), @@ -9332,10 +9305,7 @@ mod tests { seed_legacy_pin(&pool, &src, oid, None).await; } - let git_bin = write_git_shim( - &format!("gl-starve-git-{short}"), - "#!/bin/sh\nsleep 30\nexit 1\n", - ); + let git_bin = git_shim::create(&format!("gl-starve-git-{short}"), git_shim::Behavior::Hang); tokio::time::timeout( std::time::Duration::from_secs(120), @@ -9403,9 +9373,9 @@ mod tests { let (raw_cid, provider_cid) = seed_legacy_pin(&pool, &src, &fx.public_oid, None).await; // Wedges only inside the position-nine repo, which the sweep enters by cwd. - let git_bin = write_git_shim( + let git_bin = git_shim::create( &format!("gl-mid-git-{short}"), - "#!/bin/sh\ncase \"$(pwd)\" in\n */midcand9.git) sleep 30; exit 1 ;;\nesac\nexec git \"$@\"\n", + git_shim::Behavior::HangRepo("midcand9.git"), ); let first = tokio::time::timeout( @@ -9567,19 +9537,9 @@ mod tests { let log = std::env::temp_dir().join(format!("gl-ali-log-{short}")); let _ = std::fs::remove_file(&log); - let git_bin = write_git_shim( + let git_bin = git_shim::create( &format!("gl-ali-git-{short}"), - &format!( - "#!/bin/sh\n\ - if [ \"$2\" = \"--batch-check\" ]; then\n\ - \x20 oid=$(cat)\n\ - \x20 printf '%s %s\\n' \"$oid\" \"$(basename $(pwd))\" >> {log}\n\ - \x20 printf '%s\\n' \"$oid\" | git \"$@\"\n\ - \x20 exit $?\n\ - fi\n\ - exec git \"$@\"\n", - log = log.display() - ), + git_shim::Behavior::LogTypes(&log), ); let mut traversal = crate::ipfs_pin::DiscoveryTraversalState::default(); diff --git a/crates/gitlawb-node/tests/fixtures/git_shim.rs b/crates/gitlawb-node/tests/fixtures/git_shim.rs new file mode 100644 index 00000000..162de46e --- /dev/null +++ b/crates/gitlawb-node/tests/fixtures/git_shim.rs @@ -0,0 +1,59 @@ +use std::io::{Read, Write}; +use std::process::{Command, Stdio}; +use std::time::Duration; + +fn main() -> std::io::Result<()> { + let executable = std::env::current_exe()?; + let config = std::fs::read_to_string(executable.parent().unwrap().join("config"))?; + let (mode, parameter) = config.split_once('\n').unwrap_or((&config, "")); + let args: Vec<_> = std::env::args_os().skip(1).collect(); + if mode == "delay" { + std::thread::sleep(Duration::from_millis(parameter.parse().unwrap())); + } + let batch = args.get(1).is_some_and(|arg| arg == "--batch-check"); + let input = if batch { + let mut bytes = Vec::new(); + std::io::stdin().read_to_end(&mut bytes)?; + Some(bytes) + } else { + None + }; + let cwd = std::env::current_dir()?; + let hang = mode == "hang" + || (mode == "hang-repo" && cwd.file_name().is_some_and(|name| name == parameter)) + || (mode == "hang-oid" + && (args.iter().any(|arg| arg == parameter) + || input + .as_ref() + .is_some_and(|bytes| String::from_utf8_lossy(bytes).trim() == parameter))); + if hang { + std::thread::sleep(Duration::from_secs(30)); + std::process::exit(1); + } + if mode == "log-types" && batch { + let mut log = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(parameter)?; + writeln!( + log, + "{} {}", + String::from_utf8_lossy(input.as_ref().unwrap()).trim(), + cwd.file_name().unwrap().to_string_lossy() + )?; + } + let mut child = Command::new("git") + .args(&args) + .stdin(if input.is_some() { + Stdio::piped() + } else { + Stdio::inherit() + }) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .spawn()?; + if let Some(bytes) = input { + child.stdin.take().unwrap().write_all(&bytes)?; + } + std::process::exit(child.wait()?.code().unwrap_or(1)); +} From 1de5eb9b94594cb19bef4dc01ed734f8f86d00e5 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Sun, 13 Sep 2026 23:39:27 -0400 Subject: [PATCH 14/20] fix(sync): use a supported Windows promisor blob filter Git for Windows rejects the 10 GiB filter. Use 4 GiB minus one byte on Windows for clone and fetch, retain 10 GiB on other targets, and document that larger Windows blobs remain available through on-demand fetching. Refs #95 Refs #408 --- README.md | 4 ++++ crates/gitlawb-node/src/sync.rs | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index fb20ef1d..0757834c 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,10 @@ Or build from source: cargo build --release -p gl -p git-remote-gitlawb -p gitlawb-node ``` +Node promisor mirrors use a 10 GiB blob filter on Unix. Git for Windows requires +a filter below 4 GiB, so Windows mirrors use 4 GiB minus one byte; larger blobs +remain available through on-demand fetching. + Put these binaries on your `PATH`: ```txt diff --git a/crates/gitlawb-node/src/sync.rs b/crates/gitlawb-node/src/sync.rs index 62fa898a..9111b84c 100644 --- a/crates/gitlawb-node/src/sync.rs +++ b/crates/gitlawb-node/src/sync.rs @@ -691,15 +691,22 @@ async fn existing_promisor_state(repo: &str) -> PromisorProbe { } } -/// Mirror-clone a repo from a remote URL into a local bare repo. -/// `Promisor` mode adds `--filter=blob:limit=10g`, which marks the repo a git -/// promisor (so a pack with origin-omitted withheld blobs is accepted) while -/// the huge size limit means every blob the origin *does* send is kept. +// Git for Windows parses blob limits as a 32-bit unsigned long. Larger +// Windows blobs remain available through promisor on-demand fetching. +#[cfg(windows)] +const PROMISOR_BLOB_FILTER: &str = "blob:limit=4294967295"; +#[cfg(not(windows))] +const PROMISOR_BLOB_FILTER: &str = "blob:limit=10g"; + +/// Mirror-clone a repo, marking filtered mirrors as promisors so packs that +/// omit withheld blobs are accepted. The filter threshold is 10 GiB on Unix +/// and 4 GiB minus one byte on Windows; larger blobs are fetched on demand. async fn clone_repo(remote_url: &str, local_path: &Path, mode: MirrorMode) -> anyhow::Result<()> { let local_str = local_path.to_str().unwrap_or("."); + let filter_arg = format!("--filter={PROMISOR_BLOB_FILTER}"); let mut args = vec!["clone", "--mirror"]; if mode == MirrorMode::Promisor { - args.push("--filter=blob:limit=10g"); + args.push(&filter_arg); } args.push(remote_url); args.push(local_str); @@ -739,7 +746,7 @@ async fn fetch_repo(local_path: &Path, remote_url: &str, mode: MirrorMode) -> an local_str, "config", "remote.origin.partialclonefilter", - "blob:limit=10g", + PROMISOR_BLOB_FILTER, ]) .await?; git_run(&["-C", local_str, "fetch", "--prune", "origin"]).await From e9998a74164299c6a5cd22f9f19e965ac839386a Mon Sep 17 00:00:00 2001 From: euxaristia Date: Mon, 14 Sep 2026 22:37:35 -0400 Subject: [PATCH 15/20] test(graphql): anti-correlate pagination ids and cover full terminal page Invert seeded repo ids and reverse insertion order relative to name ordering so neither d.id nor insertion-ordered scans can satisfy the asserted tuple order. Size the dataset to a full final page so hasNextPage distinguishes a terminal page from remaining rows. Refs #408 --- crates/gitlawb-node/src/graphql/query.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/crates/gitlawb-node/src/graphql/query.rs b/crates/gitlawb-node/src/graphql/query.rs index 666716eb..f5195003 100644 --- a/crates/gitlawb-node/src/graphql/query.rs +++ b/crates/gitlawb-node/src/graphql/query.rs @@ -325,21 +325,28 @@ mod tests { // This owner sorts first only after did:key normalization. Its name // sorts past the first cursor, so an unqualified OR name predicate // would incorrectly serve it again on the next page. - db.create_repo(&repo("r0", earlier_owner, "zz-repo", true)) + // + // Rows are inserted out of order and assigned IDs that invert name order + // within each owner, so neither insertion order / created_at nor d.id can + // satisfy the asserted (owner, name) ordering. Sized to 6 entries so the + // last page is full-sized (limit 2), proving hasNextPage distinguishes a + // full terminal page from a page with remaining rows. + db.create_repo(&repo("r1", owner2, "d-repo", true)) .await .unwrap(); - // Owner2's a-repo sorts before owner1's names, requiring the normalized - // (owner, name) tuple both within pages and across page boundaries. - db.create_repo(&repo("r1", owner1, "b-repo", true)) + db.create_repo(&repo("r6", owner2, "c-repo", true)) + .await + .unwrap(); + db.create_repo(&repo("r7", owner2, "a-repo", true)) .await .unwrap(); db.create_repo(&repo("r2", owner1, "z-repo", true)) .await .unwrap(); - db.create_repo(&repo("r3", owner2, "a-repo", true)) + db.create_repo(&repo("r8", owner1, "b-repo", true)) .await .unwrap(); - db.create_repo(&repo("r4", owner2, "c-repo", true)) + db.create_repo(&repo("r9", earlier_owner, "zz-repo", true)) .await .unwrap(); @@ -385,7 +392,10 @@ mod tests { assert_eq!(p3["hasNextPage"], false); assert_eq!( p3["nodes"], - serde_json::json!([{"name": "c-repo", "ownerDid": owner2}]) + serde_json::json!([ + {"name": "c-repo", "ownerDid": owner2}, + {"name": "d-repo", "ownerDid": owner2}, + ]) ); } From ac6c13431fe3734267d740329bd1257cea601971 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Mon, 14 Sep 2026 22:37:40 -0400 Subject: [PATCH 16/20] revert(sync): split Windows promisor filter out of GraphQL query cost PR Remove the clone and fetch promisor filter changes and README docs from this GraphQL-scoped PR. Refs #408 --- README.md | 4 ---- crates/gitlawb-node/src/sync.rs | 19 ++++++------------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 0757834c..fb20ef1d 100644 --- a/README.md +++ b/README.md @@ -145,10 +145,6 @@ Or build from source: cargo build --release -p gl -p git-remote-gitlawb -p gitlawb-node ``` -Node promisor mirrors use a 10 GiB blob filter on Unix. Git for Windows requires -a filter below 4 GiB, so Windows mirrors use 4 GiB minus one byte; larger blobs -remain available through on-demand fetching. - Put these binaries on your `PATH`: ```txt diff --git a/crates/gitlawb-node/src/sync.rs b/crates/gitlawb-node/src/sync.rs index 9111b84c..62fa898a 100644 --- a/crates/gitlawb-node/src/sync.rs +++ b/crates/gitlawb-node/src/sync.rs @@ -691,22 +691,15 @@ async fn existing_promisor_state(repo: &str) -> PromisorProbe { } } -// Git for Windows parses blob limits as a 32-bit unsigned long. Larger -// Windows blobs remain available through promisor on-demand fetching. -#[cfg(windows)] -const PROMISOR_BLOB_FILTER: &str = "blob:limit=4294967295"; -#[cfg(not(windows))] -const PROMISOR_BLOB_FILTER: &str = "blob:limit=10g"; - -/// Mirror-clone a repo, marking filtered mirrors as promisors so packs that -/// omit withheld blobs are accepted. The filter threshold is 10 GiB on Unix -/// and 4 GiB minus one byte on Windows; larger blobs are fetched on demand. +/// Mirror-clone a repo from a remote URL into a local bare repo. +/// `Promisor` mode adds `--filter=blob:limit=10g`, which marks the repo a git +/// promisor (so a pack with origin-omitted withheld blobs is accepted) while +/// the huge size limit means every blob the origin *does* send is kept. async fn clone_repo(remote_url: &str, local_path: &Path, mode: MirrorMode) -> anyhow::Result<()> { let local_str = local_path.to_str().unwrap_or("."); - let filter_arg = format!("--filter={PROMISOR_BLOB_FILTER}"); let mut args = vec!["clone", "--mirror"]; if mode == MirrorMode::Promisor { - args.push(&filter_arg); + args.push("--filter=blob:limit=10g"); } args.push(remote_url); args.push(local_str); @@ -746,7 +739,7 @@ async fn fetch_repo(local_path: &Path, remote_url: &str, mode: MirrorMode) -> an local_str, "config", "remote.origin.partialclonefilter", - PROMISOR_BLOB_FILTER, + "blob:limit=10g", ]) .await?; git_run(&["-C", local_str, "fetch", "--prune", "origin"]).await From 4e6488d885e177a46ca9b5c6033729c2466b27f3 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Thu, 17 Sep 2026 05:40:46 -0400 Subject: [PATCH 17/20] test(graphql): pin case ordering and test documented max page limit Permute insertion order and seed mixed-case repo to ensure folded and recency orderings cannot satisfy pagination tests, and verify limit:200 distinguishes a terminal page from row 201. Refs #408 --- crates/gitlawb-node/src/graphql/query.rs | 60 +++++++++++++++++++----- 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/crates/gitlawb-node/src/graphql/query.rs b/crates/gitlawb-node/src/graphql/query.rs index f5195003..7e7b7c98 100644 --- a/crates/gitlawb-node/src/graphql/query.rs +++ b/crates/gitlawb-node/src/graphql/query.rs @@ -326,27 +326,28 @@ mod tests { // sorts past the first cursor, so an unqualified OR name predicate // would incorrectly serve it again on the next page. // - // Rows are inserted out of order and assigned IDs that invert name order - // within each owner, so neither insertion order / created_at nor d.id can - // satisfy the asserted (owner, name) ordering. Sized to 6 entries so the - // last page is full-sized (limit 2), proving hasNextPage distinguishes a - // full terminal page from a page with remaining rows. - db.create_repo(&repo("r1", owner2, "d-repo", true)) + // Rows are inserted out of order and not in reverse asserted order. + // Assigned IDs do not track name order, and owner2 contains both + // "B-repo" and "a-repo" whose byte order (B < a) differs from their + // case-folded order (a < b), ensuring lower(d.name) cannot satisfy + // the keyset query. Sized to 6 entries so the last page is full-sized + // (limit 2), proving hasNextPage distinguishes a full terminal page. + db.create_repo(&repo("r5", owner2, "c-repo", true)) .await .unwrap(); - db.create_repo(&repo("r6", owner2, "c-repo", true)) + db.create_repo(&repo("r9", earlier_owner, "zz-repo", true)) .await .unwrap(); - db.create_repo(&repo("r7", owner2, "a-repo", true)) + db.create_repo(&repo("r3", owner2, "B-repo", true)) .await .unwrap(); - db.create_repo(&repo("r2", owner1, "z-repo", true)) + db.create_repo(&repo("r8", owner1, "z-repo", true)) .await .unwrap(); - db.create_repo(&repo("r8", owner1, "b-repo", true)) + db.create_repo(&repo("r2", owner1, "b-repo", true)) .await .unwrap(); - db.create_repo(&repo("r9", earlier_owner, "zz-repo", true)) + db.create_repo(&repo("r1", owner2, "a-repo", true)) .await .unwrap(); @@ -379,7 +380,7 @@ mod tests { p2["nodes"], serde_json::json!([ {"name": "z-repo", "ownerDid": owner1}, - {"name": "a-repo", "ownerDid": owner2}, + {"name": "B-repo", "ownerDid": owner2}, ]) ); let cursor = p2["endCursor"].as_str().unwrap(); @@ -393,12 +394,45 @@ mod tests { assert_eq!( p3["nodes"], serde_json::json!([ + {"name": "a-repo", "ownerDid": owner2}, {"name": "c-repo", "ownerDid": owner2}, - {"name": "d-repo", "ownerDid": owner2}, ]) ); } + #[sqlx::test] + async fn repos_page_at_documented_maximum_distinguishes_terminal_page(pool: PgPool) { + let db = db(pool).await; + let total = crate::db::MAX_VISIBLE_REPO_PAGE_SIZE; + for index in 0..total { + let name = format!("repo-{index:03}"); + db.create_repo(&repo(&name, OWNER, &name, true)) + .await + .unwrap(); + } + let schema = schema(db.clone()); + let query = + format!("{{ reposPage(limit: {total}) {{ nodes {{ name }} hasNextPage endCursor }} }}"); + let resp200 = anon(&schema, &query).await; + assert!(resp200.errors.is_empty(), "{:?}", resp200.errors); + let p200 = resp200.data.into_json().unwrap()["reposPage"].clone(); + assert_eq!(p200["hasNextPage"], false); + let nodes200 = p200["nodes"].as_array().unwrap().clone(); + assert_eq!(nodes200.len(), total); + + // Adding row 201 keeps page 1 content identical while flipping hasNextPage to true. + let name201 = format!("repo-{total:03}"); + db.create_repo(&repo(&name201, OWNER, &name201, true)) + .await + .unwrap(); + let resp201 = anon(&schema, &query).await; + assert!(resp201.errors.is_empty(), "{:?}", resp201.errors); + let p201 = resp201.data.into_json().unwrap()["reposPage"].clone(); + assert_eq!(p201["hasNextPage"], true); + assert_eq!(p201["nodes"], p200["nodes"]); + assert_eq!(p201["endCursor"], p200["endCursor"]); + } + #[sqlx::test] async fn repos_page_visibility_matches_the_shared_gate(pool: PgPool) { use crate::db::VisibilityMode; From f778793f0057dd6c959c8d24c7f1182adea80d28 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Thu, 17 Sep 2026 21:39:14 -0400 Subject: [PATCH 18/20] fix(db): pin COLLATE "C" for visible repos page ordering and keyset resume --- crates/gitlawb-node/src/db/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/gitlawb-node/src/db/mod.rs b/crates/gitlawb-node/src/db/mod.rs index d324157e..b41282d9 100644 --- a/crates/gitlawb-node/src/db/mod.rs +++ b/crates/gitlawb-node/src/db/mod.rs @@ -1576,8 +1576,8 @@ impl Db { ) ELSE FALSE END ) - AND ($4::text IS NULL OR (({key}), d.name) > ($4, $5::text)) - ORDER BY ({key}), d.name + AND ($4::text IS NULL OR (({key}) COLLATE \"C\", d.name COLLATE \"C\") > ($4, $5::text)) + ORDER BY ({key}) COLLATE \"C\", d.name COLLATE \"C\" LIMIT $6", Self::dedup_cte(), key = OWNER_KEY_CASE_SQL, From c0aa2dc51336e94223ba9ea3bad8c7fe08c897a7 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Sun, 20 Sep 2026 05:51:29 -0400 Subject: [PATCH 19/20] fix(db): deny malformed reader lists without failing repo pages Validate root-rule JSON before casting it so one malformed TEXT row cannot break every repository listing. Extend the shared-gate fixture and legacy listing checks while preserving owner access and other visible repositories. Refs #428 --- crates/gitlawb-node/src/db/mod.rs | 2 ++ crates/gitlawb-node/src/graphql/query.rs | 26 ++++++++++++++++++++++++ docs/graphql-pagination.md | 2 ++ 3 files changed, 30 insertions(+) diff --git a/crates/gitlawb-node/src/db/mod.rs b/crates/gitlawb-node/src/db/mod.rs index b41282d9..3d2fe8e3 100644 --- a/crates/gitlawb-node/src/db/mod.rs +++ b/crates/gitlawb-node/src/db/mod.rs @@ -1547,6 +1547,7 @@ impl Db { /// name. Apply root visibility before LIMIT: private rows must not consume /// page slots or influence continuation metadata. The root-rule predicate /// mirrors `visibility::listable_at_root`; a differential test pins it. + /// Malformed reader JSON denies that repo to non-owners without failing the page. /// Cursors only select a position and never confer read authority. pub async fn list_visible_repos_page( &self, @@ -1568,6 +1569,7 @@ impl Db { WHERE ( ($2::text IS NOT NULL AND ({key}) = $2) OR CASE WHEN root_rule.reader_dids IS NULL THEN d.is_public + WHEN NOT pg_input_is_valid(root_rule.reader_dids, 'jsonb') THEN FALSE WHEN jsonb_typeof(root_rule.reader_dids::jsonb) = 'array' THEN COALESCE(root_rule.reader_dids::jsonb ? $3::text, FALSE) AND NOT EXISTS ( diff --git a/crates/gitlawb-node/src/graphql/query.rs b/crates/gitlawb-node/src/graphql/query.rs index 7e7b7c98..251b0782 100644 --- a/crates/gitlawb-node/src/graphql/query.rs +++ b/crates/gitlawb-node/src/graphql/query.rs @@ -446,6 +446,7 @@ mod tests { ("subtree", true), ("root-tie", true), ("odd-star", true), + ("malformed-readers", true), ("quarantined", true), ] { db.create_repo(&repo(id, OWNER, id, public)).await.unwrap(); @@ -470,11 +471,20 @@ mod tests { ("root-tie", "/", vec![reader.to_owned()]), ("root-tie", "/**", vec![]), ("odd-star", "/*", vec![]), + ("malformed-readers", "/", vec![]), ] { db.set_visibility_rule(id, glob, VisibilityMode::B, &readers, OWNER) .await .unwrap(); } + // The typed setter cannot create malformed JSON, but existing TEXT rows + // can contain it. Such a rule must deny this repo, not abort the page. + sqlx::query("UPDATE visibility_rules SET reader_dids = $1 WHERE repo_id = $2") + .bind("not JSON") + .bind("malformed-readers") + .execute(db.pool()) + .await + .unwrap(); let all = db.list_all_repos_deduped().await.unwrap(); for caller in [ None, @@ -508,6 +518,22 @@ mod tests { assert_eq!(actual, expected, "caller {caller:?}"); } let schema = schema(db); + for caller in [None, Some("did:key:zUnauthorized")] { + let query = "{ repos { name } }"; + let response = match caller { + None => anon(&schema, query).await, + Some(caller) => authed(&schema, query, caller).await, + }; + assert!(response.errors.is_empty(), "{:?}", response.errors); + let mut names: Vec = response.data.into_json().unwrap()["repos"] + .as_array() + .unwrap() + .iter() + .map(|repo| repo["name"].as_str().unwrap().to_owned()) + .collect(); + names.sort(); + assert_eq!(names, ["odd-star", "open", "subtree"]); + } let query = "{ reposPage(limit: 1) { nodes { name ownerDid } hasNextPage endCursor } }"; let response = anon(&schema, query).await; assert!(response.errors.is_empty(), "{:?}", response.errors); diff --git a/docs/graphql-pagination.md b/docs/graphql-pagination.md index 065d2da5..0e111adb 100644 --- a/docs/graphql-pagination.md +++ b/docs/graphql-pagination.md @@ -31,6 +31,8 @@ hidden and quarantined repositories do not occupy page slots or create a continuation signal. Cursors contain a position from the last returned visible repository, not a permission grant. Each request checks the current caller's visibility independently. Keep the same caller while traversing a list. +Malformed reader lists in a repository's root visibility rule deny access to +non-owners for that repository while other visible repositories remain listable. Pagination is not a snapshot: concurrent renames, ownership changes, or visibility changes can alter subsequent pages. Treat cursors as opaque and restart from the From a80845e88536b678b7fdcb9a08a928acf162c7be Mon Sep 17 00:00:00 2001 From: euxaristia Date: Sun, 20 Sep 2026 05:51:29 -0400 Subject: [PATCH 20/20] fix(graphql): reject NUL bytes in repository cursors Reject decoded NULs before PostgreSQL text binding so malformed cursors use the existing input error instead of generating database errors. Cover both components before database access and document the rejection. Refs #428 --- crates/gitlawb-node/src/graphql/query.rs | 35 ++++++++++++++++++++++-- docs/graphql-pagination.md | 2 ++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/crates/gitlawb-node/src/graphql/query.rs b/crates/gitlawb-node/src/graphql/query.rs index 251b0782..ec51ce85 100644 --- a/crates/gitlawb-node/src/graphql/query.rs +++ b/crates/gitlawb-node/src/graphql/query.rs @@ -32,8 +32,14 @@ fn parse_repo_cursor(cursor: &str) -> Result<(String, String)> { let bytes = URL_SAFE_NO_PAD .decode(cursor) .map_err(|_| async_graphql::Error::new("invalid repository cursor"))?; - serde_json::from_slice(&bytes) - .map_err(|_| async_graphql::Error::new("invalid repository cursor")) + let position: (String, String) = serde_json::from_slice(&bytes) + .map_err(|_| async_graphql::Error::new("invalid repository cursor"))?; + // PostgreSQL text cannot contain NUL. Reject it as malformed input before + // binding either cursor component, rather than reporting a database error. + if position.0.contains('\0') || position.1.contains('\0') { + return Err(async_graphql::Error::new("invalid repository cursor")); + } + Ok(position) } pub struct QueryRoot; @@ -643,6 +649,31 @@ mod tests { ); } + #[tokio::test] + async fn repos_page_rejects_nul_cursors_before_database_access() { + let pool = sqlx::postgres::PgPoolOptions::new() + .connect_lazy("postgres://localhost/unused") + .unwrap(); + let schema = schema(Arc::new(Db::for_testing(pool))); + for position in [("did:key:zOwner\0", "repo"), ("did:key:zOwner", "repo\0")] { + let cursor = super::URL_SAFE_NO_PAD.encode(serde_json::to_vec(&position).unwrap()); + let query = format!("{{ reposPage(after: \"{cursor}\") {{ hasNextPage }} }}"); + let response = + tokio::time::timeout(std::time::Duration::from_secs(1), anon(&schema, &query)) + .await + .expect("invalid cursors must be rejected without accessing the database"); + assert_eq!(response.errors.len(), 1); + assert_eq!(response.errors[0].message, "invalid repository cursor"); + assert_eq!(response.data, async_graphql::Value::Null); + } + let position = ("did:key:zOwner", "repo"); + let cursor = super::URL_SAFE_NO_PAD.encode(serde_json::to_vec(&position).unwrap()); + assert_eq!( + super::parse_repo_cursor(&cursor).unwrap(), + (position.0.to_owned(), position.1.to_owned()) + ); + } + #[tokio::test] async fn repos_page_rejects_invalid_inputs_before_database_access() { let pool = sqlx::postgres::PgPoolOptions::new() diff --git a/docs/graphql-pagination.md b/docs/graphql-pagination.md index 0e111adb..02dc7942 100644 --- a/docs/graphql-pagination.md +++ b/docs/graphql-pagination.md @@ -24,6 +24,8 @@ Start with `after: null`. When `hasNextPage` is true, pass the returned An empty page has a null `endCursor`. The default limit is 50; limits outside 1–200 are rejected. The document complexity budget still applies, so requesting many fields may require a smaller page. +Malformed cursors, including decoded owner or name strings containing NUL, +return `invalid repository cursor` before database access. Pages are ordered by normalized owner DID and repository name. Visibility and mirror deduplication are applied in the database before limiting the page;