Skip to content

OsvClient: cap fetch_all_pages at MAX_PAGES to avoid unbounded loop#52

Open
SAY-5 wants to merge 1 commit intoHomebrew:mainfrom
SAY-5:fix-osv-pagination-loop-bound
Open

OsvClient: cap fetch_all_pages at MAX_PAGES to avoid unbounded loop#52
SAY-5 wants to merge 1 commit intoHomebrew:mainfrom
SAY-5:fix-osv-pagination-loop-bound

Conversation

@SAY-5
Copy link
Copy Markdown

@SAY-5 SAY-5 commented Apr 15, 2026

Closes #47.

OsvClient#fetch_all_pages (lib/brew/vulns/osv_client.rb:120) loops as long as the OSV response carries a next_page_token, with no upper bound:

while page_token
  payload = original_payload.merge(page_token: page_token)
  response = post("/query", payload)
  vulns.concat(response["vulns"] || [])
  page_token = response["next_page_token"]
end

If the upstream returns a buggy response that echoes the same next_page_token (or unconditionally returns one), the loop concatenates into vulns until the process OOM-kills.

Add a MAX_PAGES = 100 ceiling and raise ApiError with a clear message when the limit is hit, turning the infinite-pagination case into an actionable error. Added a regression test (test_pagination_aborts_after_max_pages) that stubs the server to return the same token forever and asserts the ApiError.

Local verification: Ruby -c syntax check passes for both files. I couldn't run bundle exec rake test locally because system Ruby is 2.6 and Gemfile.lock pins bundler 4.0.6 which requires Ruby >= 3.2 — happy to iterate on the test if CI catches anything.

fetch_all_pages keeps calling /v1/query as long as the OSV response has
a next_page_token. If OSV ever returns a buggy response that echoes the
same token (or returns a token unconditionally), the loop allocates
into 'vulns' until the process is OOM-killed.

Add a 100-page upper bound and raise ApiError with a clear message when
the limit is hit, so an infinite-pagination response surfaces as an
actionable error instead of an OOM. Add a regression test that stubs
the server to return the same next_page_token every call and asserts
the ApiError.
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.

Pagination loop has no upper bound

1 participant