fix: use bounded read for workflow catalog HTTP responses

[Quratulain-bilal]

Summary

The workflow catalog fetch (WorkflowCatalog._fetch_single_catalog and StepCatalog._fetch_single_catalog) used unbounded resp.read() to read HTTP responses into memory. A malicious or misconfigured catalog server could return an arbitrarily large response causing OOM.

Changes

• src/specify_cli/workflows/catalog.py (lines 541, 1214): Replaced resp.read() with read_response_limited(resp, max_bytes=MAX_JSON_METADATA_BYTES) capped at 1 MiB at both call sites, consistent with how other JSON fetch paths in the codebase enforce bounded reads.

Testing

All 176 tests in tests/workflows/ pass after the fix.

Security Impact

This is a Medium severity fix - it closes a potential memory exhaustion vector against the workflow catalog fetch endpoint.

[Copilot]

Pull request overview

Bounds workflow and step catalog HTTP responses to 1 MiB, mitigating memory-exhaustion risks.

Changes:

• Uses read_response_limited for both catalog fetch paths.
• Applies MAX_JSON_METADATA_BYTES as the response limit.

Show a summary per file

File	Description
src/specify_cli/workflows/catalog.py	Adds bounded reads for workflow and step catalogs.

Review details

Tip

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

Comments suppressed due to low confidence (1)

src/specify_cli/workflows/catalog.py:1219

• Please cover the step-catalog fetch path with an oversized streamed response and assert StepCatalogError. StepCatalog has separate fetch/cache handling, and its current fetch tests also exit before reading the body, so they do not verify that this call site actually enforces the new limit.

                data = json.loads(
                    read_response_limited(resp, max_bytes=MAX_JSON_METADATA_BYTES).decode("utf-8")

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

[mnriem]

Please address Copilot feedback

[Copilot]

Review details

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

[mnriem]

Please resolve conflicts

[Copilot]

Review details

Comments suppressed due to low confidence (6)

tests/test_workflows.py:6843

• This patch target does not exist: the workflow fetch passes MAX_JSON_CATALOG_BYTES to read_response_limited. As written, the test fails during setup rather than validating recovery after an oversized response.

            "specify_cli.workflows.catalog.MAX_JSON_METADATA_BYTES", 512

tests/test_workflows.py:7675

• workflows.catalog exposes MAX_JSON_CATALOG_BYTES, not MAX_JSON_METADATA_BYTES, so this monkeypatch fails with AttributeError before exercising StepCatalog._fetch_single_catalog. Patch the constant actually used by that fetch.

            "specify_cli.workflows.catalog.MAX_JSON_METADATA_BYTES", 512

tests/test_workflows.py:7727

• This test patches a nonexistent module attribute and therefore cannot verify that a healthy step catalog still works after rejection. The production bounded read uses MAX_JSON_CATALOG_BYTES.

            "specify_cli.workflows.catalog.MAX_JSON_METADATA_BYTES", 512

tests/test_workflows.py:7666

• The step-catalog fetch is bounded by MAX_JSON_CATALOG_BYTES, not the metadata limit. Update the docstring so it documents the actual regression contract.

        MAX_JSON_METADATA_BYTES instead of reading unbounded into memory."""

tests/test_workflows.py:6791

• workflows.catalog imports MAX_JSON_CATALOG_BYTES, not MAX_JSON_METADATA_BYTES, so monkeypatch.setattr raises AttributeError and this regression test never reaches the fetch. Patch the catalog limit used by the production call instead.

This issue also appears in the following locations of the same file:

• line 6843
• line 7675
• line 7727

            "specify_cli.workflows.catalog.MAX_JSON_METADATA_BYTES", 512

tests/test_workflows.py:6782

• The implementation now uses the catalog-specific 8 MiB ceiling, so this docstring names the wrong constant and incorrectly describes the behavior under test.

This issue also appears on line 7666 of the same file.

        MAX_JSON_METADATA_BYTES instead of reading unbounded into memory."""

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

[mnriem]

Please address test & lint errors

[github-actions]

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed in 30 days if no further activity occurs.

[Quratulain-bilal]

Addressed in c38a0ad6 (plus 3b825dd4, a merge of main).

Why this PR lost its original payload. The unbounded resp.read() fix landed upstream when src/specify_cli/workflows/catalog.py became the workflows/catalog/ package: both fetch sites now call read_response_limited() with MAX_JSON_CATALOG_BYTES (workflows/catalog/_domain.py:578, workflows/step/catalog/_domain.py:469), and the rejection path is already covered by tests/test_workflows.py::test_fetch_rejects_oversized_catalog_response (workflow and step variants).

What this branch now carries.

• Dropped the two rejection tests that duplicated that coverage. They also patched MAX_JSON_METADATA_BYTES, which no longer exists after the refactor, so they failed with AttributeError - that is the test failure you flagged.
• Kept the one scenario main does not cover: an oversized catalog is rejected without poisoning the next healthy fetch, for both the workflow and the step catalog.
• Patch target corrected to MAX_JSON_CATALOG_BYTES on the package each _max_json_catalog_bytes() reads at call time.
• Merged main (clean, no conflicts), so the earlier conflict blocker is gone. mergeable: MERGEABLE.

Verification. tests/test_workflows.py: 714 passed, 5 skipped. uvx ruff@0.15.0 check tests/test_workflows.py: all checks passed.

---

Disclosure. This change was prepared with AI coding assistance.

Tool	opencode (autonomous execution under direct human direction)
Model	mimo-v2.6-flash-free (model id: opencode/mimo-v2.6-flash-free)
Extent	Upstream diff analysis, test selection, test patch target correction, local verification
Human extent	Direction to revive this PR instead of closing it, scope of what to keep, review of findings, all release decisions

Assisted-by: opencode (model: mimo-v2.6-flash-free, autonomous)

[Copilot]

Copilot review overview

🟢 Approval recommended

The added tests cover rejection and recovery with no unresolved findings; before-and-after execution was not verified here.

Review effort: Balanced
Findings: None