Skip to content

fix: reject zero and negative page sizes with a rendered error - #476

Merged
zachdaniel merged 1 commit into
ash-project:mainfrom
grempe:fix/page-size-validation
Sep 17, 2026
Merged

zachdaniel merged 1 commit into
ash-project:mainfrom
grempe:fix/page-size-validation

Conversation

@grempe

@grempe grempe commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Closes #471 and #474.

Both issues are one gap at the GraphQL boundary. first, last and limit are plain Int arguments, so zero and negative values pass schema validation:

Changes in AshGraphql.Graphql.Resolver:

  • validate_offset_opts/3 and validate_keyset_opts/3 return an Ash.Error.Query.InvalidQuery for a limit, first or last below one, in the same shape as the existing first/last/before/after combination errors, so the client gets a rendered invalid_query error.
  • apply_load_arguments/5 adds the same error to the nested query for a negative limit on a relationship field. Ash accepts any integer there, so this previously returned an empty list without complaint. A limit of zero on a relationship still returns an empty list. This is the one behaviour change that is a judgement call; happy to drop it if you would rather leave relationship arguments to Ash.
  • The three query_complexity/3 clauses clamp the page size at zero, so complexity analysis never returns a negative value.

Tests: new test/page_size_test.exs covers zero and negative limit (offset), first and last (keyset), a negative page size under analyze_complexity: true, and the relationship limit cases. On main, the offset and keyset cases return the unrendered Unknown error, the complexity case raises Absinthe.AnalysisError, and the relationship case returns an empty list. Full suite 444/444 on main and 429/429 with the patch applied to v1.11.0. mix format --check-formatted, mix credo --strict, mix sobelow and mix dialyzer clean.

Found by an AI agent working with a human fuzz-testing their own application; the reproductions outside this repo are https://github.com/grempe/ash-fuzz-repros/blob/main/test/ash_graphql/negative_page_size_complexity_test.exs and https://github.com/grempe/ash-fuzz-repros/blob/main/test/ash_graphql/zero_page_size_test.exs.

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

A `first`, `last` or `limit` of zero or less passed schema validation and
reached Ash, whose page option validation failed with a
`Spark.Options.ValidationError` wrapped in `Ash.Error.Unknown`, so the client
saw "Something went wrong". A negative value never got that far: the
complexity analyzer returned a negative number and Absinthe raised
`Absinthe.AnalysisError` before resolution.

The page option validation now returns an `Ash.Error.Query.InvalidQuery` for
a page size below one, on both the offset and keyset strategies, and a
negative `limit` on a relationship field adds the same error to the nested
query instead of being passed to Ash, which would silently return nothing.
The complexity callbacks clamp the page size at zero so analysis never
raises.

Closes ash-project#471
Closes ash-project#474
@zachdaniel
zachdaniel merged commit 080bafb into ash-project:main Sep 17, 2026
@zachdaniel

Copy link
Copy Markdown
Contributor

🚀 Thank you for your contribution! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A negative page size makes complexity analysis raise

2 participants