Skip to content

fix: bound response read in integration catalog fetch - #3812

Open
Quratulain-bilal wants to merge 2 commits into
github:mainfrom
Quratulain-bilal:fix/unbounded-integrations-catalog-read
Open

fix: bound response read in integration catalog fetch#3812
Quratulain-bilal wants to merge 2 commits into
github:mainfrom
Quratulain-bilal:fix/unbounded-integrations-catalog-read

Conversation

@Quratulain-bilal

Copy link
Copy Markdown
Contributor

Replace unbounded
esp.read() with
ead_response_limited() in integrations/catalog.py to prevent DoS via oversized catalog responses.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Bounds integration catalog HTTP responses to mitigate memory-based denial-of-service risks.

Changes:

  • Uses the shared bounded-response reader.
  • Applies the 1 MiB JSON metadata limit.
Show a summary per file
File Description
src/specify_cli/integrations/catalog.py Limits catalog response reads before JSON parsing.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Medium

Comment on lines +204 to +205
catalog_data = json.loads(
read_response_limited(resp, max_bytes=MAX_JSON_METADATA_BYTES, error_type=IntegrationCatalogError)
Comment on lines +204 to +205
catalog_data = json.loads(
read_response_limited(resp, max_bytes=MAX_JSON_METADATA_BYTES, error_type=IntegrationCatalogError)
…egression test

- Update FakeResponse.read() to accept size parameter for bounded reads
- Add test_fetch_rejects_oversized_catalog_response regression test
- Verifies _fetch_single_catalog uses MAX_JSON_METADATA_BYTES

Fixes github#3812
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.

3 participants