Skip to content

Commit a80845e

Browse files
committed
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
1 parent c0aa2dc commit a80845e

2 files changed

Lines changed: 35 additions & 2 deletions

File tree

‎crates/gitlawb-node/src/graphql/query.rs‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,14 @@ fn parse_repo_cursor(cursor: &str) -> Result<(String, String)> {
3232
let bytes = URL_SAFE_NO_PAD
3333
.decode(cursor)
3434
.map_err(|_| async_graphql::Error::new("invalid repository cursor"))?;
35-
serde_json::from_slice(&bytes)
36-
.map_err(|_| async_graphql::Error::new("invalid repository cursor"))
35+
let position: (String, String) = serde_json::from_slice(&bytes)
36+
.map_err(|_| async_graphql::Error::new("invalid repository cursor"))?;
37+
// PostgreSQL text cannot contain NUL. Reject it as malformed input before
38+
// binding either cursor component, rather than reporting a database error.
39+
if position.0.contains('\0') || position.1.contains('\0') {
40+
return Err(async_graphql::Error::new("invalid repository cursor"));
41+
}
42+
Ok(position)
3743
}
3844

3945
pub struct QueryRoot;
@@ -643,6 +649,31 @@ mod tests {
643649
);
644650
}
645651

652+
#[tokio::test]
653+
async fn repos_page_rejects_nul_cursors_before_database_access() {
654+
let pool = sqlx::postgres::PgPoolOptions::new()
655+
.connect_lazy("postgres://localhost/unused")
656+
.unwrap();
657+
let schema = schema(Arc::new(Db::for_testing(pool)));
658+
for position in [("did:key:zOwner\0", "repo"), ("did:key:zOwner", "repo\0")] {
659+
let cursor = super::URL_SAFE_NO_PAD.encode(serde_json::to_vec(&position).unwrap());
660+
let query = format!("{{ reposPage(after: \"{cursor}\") {{ hasNextPage }} }}");
661+
let response =
662+
tokio::time::timeout(std::time::Duration::from_secs(1), anon(&schema, &query))
663+
.await
664+
.expect("invalid cursors must be rejected without accessing the database");
665+
assert_eq!(response.errors.len(), 1);
666+
assert_eq!(response.errors[0].message, "invalid repository cursor");
667+
assert_eq!(response.data, async_graphql::Value::Null);
668+
}
669+
let position = ("did:key:zOwner", "repo");
670+
let cursor = super::URL_SAFE_NO_PAD.encode(serde_json::to_vec(&position).unwrap());
671+
assert_eq!(
672+
super::parse_repo_cursor(&cursor).unwrap(),
673+
(position.0.to_owned(), position.1.to_owned())
674+
);
675+
}
676+
646677
#[tokio::test]
647678
async fn repos_page_rejects_invalid_inputs_before_database_access() {
648679
let pool = sqlx::postgres::PgPoolOptions::new()

‎docs/graphql-pagination.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ Start with `after: null`. When `hasNextPage` is true, pass the returned
2424
An empty page has a null `endCursor`. The default limit is 50; limits outside
2525
1–200 are rejected. The document complexity budget still applies, so requesting
2626
many fields may require a smaller page.
27+
Malformed cursors, including decoded owner or name strings containing NUL,
28+
return `invalid repository cursor` before database access.
2729

2830
Pages are ordered by normalized owner DID and repository name. Visibility and
2931
mirror deduplication are applied in the database before limiting the page;

0 commit comments

Comments
 (0)