feat!: bound pagination limit and page through list endpoints - #27807
Closed
jscottmiller wants to merge 5 commits into
Closed
feat!: bound pagination limit and page through list endpoints#27807jscottmiller wants to merge 5 commits into
jscottmiller wants to merge 5 commits into
Conversation
jscottmiller
force-pushed
the
scott/plat-386-bound-pagination-limit
branch
2 times, most recently
from
August 4, 2026 17:44
75c8ea4 to
6b1951c
Compare
Docs previewCheck off each page once it's been reviewed. If a page changes in a later push, its checkbox clears automatically so it gets a fresh look. Pages not yet wired into the docs navigation aren't listed here. |
Resolve a limit of 0 or an omitted limit to MaxPaginationLimit (1000) and reject a larger explicit limit, so paginated endpoints can no longer issue an unbounded query. Refs PLAT-386.
ParsePagination now defaults an omitted limit to MaxPaginationLimit and rejects anything outside 1..MaxPaginationLimit, so several doc strings no longer describe the behavior: - codersdk.Pagination.Limit no longer means "no limit" at <= 0. - paginated-members no longer returns all members at limit=0. - limit is no longer required on /audit and /connectionlog. The ai-gateway session, model, and client list handlers each defaulted a zero limit and rejected limits above 1000. ParsePagination guarantees both conditions, so the blocks and their constants are removed. The effective ceiling on those routes drops from 1000 to MaxPaginationLimit. Refs PLAT-386.
The bound describes a value clients need in order to size their own page requests, so it lives next to the Pagination type that carries it rather than in coderd.
The server now caps a page at MaxPaginationLimit, so commands that relied on an omitted limit returning every row request successive pages instead.
An explicit limit of 0 is now rejected, so the callers that used it either request a single row when they only read the count, or page to exhaustion. Workspace and support page sizes derive from MaxPaginationLimit.
jscottmiller
force-pushed
the
scott/plat-386-bound-pagination-limit
branch
from
August 11, 2026 15:24
0bace98 to
546969f
Compare
jscottmiller
changed the base branch from
main
to
scott/plat-386-workspace-pagination
August 11, 2026 15:25
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacked on #28028. Marked experimental: the open items below are not done.
ParsePaginationleft an omittedlimitat 0, which every downstream querytreats as "no limit", so any client could ask a list endpoint for the entire
table in one request. It now resolves an omitted limit to
codersdk.MaxPaginationLimit(100) and rejects a limit outside1..100with avalidation error rather than clamping it.
MaxPaginationLimitlives incodersdk, next to thePaginationtype thatcarries it, so clients can size their own page requests.
exhaustion:
coder list,coder users list, template versions, templatepull, and SSH completion.
limit: 0either request a single row when theyonly read the count, or page to exhaustion (org members).
enterprise/coderd/aibridge.go.Breaking change
?limit=0and?limit=<n>forn > 100now return 400 on every endpoint thatuses
ParsePagination. Clients that relied onlimit=0meaning "all rows" mustpage. The Go SDK never put
limit=0on the wire, so Go integrators seetruncation rather than an error.
Testing
go test ./coderd/ -run TestPaginationgo test ./support/ -run TestRungo test ./codersdk/ -run TestAllWorkspacespnpm vitest run src/api/api.test.tsOutstanding review items
Blocking, each corroborated by more than one reviewer:
enterprise/cli/groupedit.go:64callsclient.Usersunpaginated, so--add-users/--rm-userssilently resolves against the first 100 users./templates/{template}/acl/availabletruncates at 100.GetGroupshas noOFFSETand noORDER BY, andcodersdk.ACLAvailablehas no count, sothe truncation is neither pageable nor detectable.
from
DetailtoValidations.codersdk/aibridge.gostill documents1000. Decide whether to keep 100 and declare it, or give those endpoints
their own cap.
The stated invariant does not hold yet:
?limit=0is still unbounded oncoderd/provisionerjobs.goandcoderd/provisionerdaemons.go.coderd/exp_chats.go(chatCostUsers) accepts up toMaxInt32;enterprise/coderd/agentfirewall.gohas a default but no maximum.offsetis uncapped and the sort runs beforeOFFSET, so this boundsresponse size, not database work. It should not be described as a DoS
mitigation.
Contract and documentation:
docs/reference/api/page states the bound, and the documented?limit=0curl examples inaudit.mdandenterprise.mdnow return 400.enterprise/coderd/templates.go,coderd/exp_chats.go(listChats), andthe aibridge models/clients endpoints reject out-of-range limits without
documenting a
limitoroffsetparameter.MaxPaginationLimit's comment claims it is the server-wide maximum;exp_chatsenforces 1-200 and 0-2000 elsewhere. Scope the wording toendpoints that use
ParsePagination.Tests and lower priority:
coderd/pagination_test.goasserts onlyapiError.Message, which is thesame constant for every failure. Assert
Validations[0].Field, the detailtext, and that exactly one validation error is returned.
at most 100.
?limit=(present but empty) resolves to 100 instead of erroring, and?limit=&limit=50silently discards the 50.cli/userlist.goandcli/templateversions.gopage by offset where thoseendpoints support
AfterID.coder templates pullwalks every version page to produce one warning.This pull request was created by Coder Agents on behalf of @jscottmiller.