From 5c2087cedcd715efddaaf7b354a516ced857227c Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Mon, 27 Jul 2026 22:38:34 +0500 Subject: [PATCH] fix: use bounded read for workflow catalog HTTP responses The workflow catalog fetch used unbounded resp.read() to read HTTP responses into memory. A malicious or misconfigured catalog server could return an arbitrarily large response causing OOM. Replace with read_response_limited() capped at MAX_JSON_METADATA_BYTES (1 MiB) at both call sites, consistent with how other JSON fetch paths in the codebase enforce bounded reads. --- src/specify_cli/workflows/catalog.py | 10 +- tests/test_workflows.py | 284 +++++++++++++++++++++++++++ 2 files changed, 292 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/workflows/catalog.py b/src/specify_cli/workflows/catalog.py index 63ddc76395..5356940e3c 100644 --- a/src/specify_cli/workflows/catalog.py +++ b/src/specify_cli/workflows/catalog.py @@ -22,6 +22,8 @@ import yaml +from .._download_security import MAX_JSON_METADATA_BYTES, read_response_limited + # --------------------------------------------------------------------------- # Errors @@ -538,7 +540,9 @@ def _validate_redirect(_old_url: str, new_url: str) -> None: entry.url, timeout=30, redirect_validator=_validate_redirect ) as resp: _validate_catalog_url(resp.geturl()) - data = json.loads(resp.read().decode("utf-8")) + data = json.loads( + read_response_limited(resp, max_bytes=MAX_JSON_METADATA_BYTES).decode("utf-8") + ) except Exception as exc: # Fall back to cache if available if cache_file.exists(): @@ -1211,7 +1215,9 @@ def _validate_redirect(_old_url: str, new_url: str) -> None: entry.url, timeout=30, redirect_validator=_validate_redirect ) as resp: _validate_url(resp.geturl()) - data = json.loads(resp.read().decode("utf-8")) + data = json.loads( + read_response_limited(resp, max_bytes=MAX_JSON_METADATA_BYTES).decode("utf-8") + ) except Exception as exc: if cache_safe and cache_file.exists(): try: diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 969daf9c1d..097718cc30 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -6657,6 +6657,148 @@ def _raising_open(file, mode="r", *args, **kwargs): with pytest.raises(WorkflowValidationError, match="Failed to write catalog config"): catalog.remove_catalog(0) + def test_oversized_workflow_catalog_response_rejected(self, project_dir, monkeypatch): + """WorkflowCatalog._fetch_single_catalog rejects responses exceeding + MAX_JSON_METADATA_BYTES instead of reading unbounded into memory.""" + from specify_cli.workflows.catalog import ( + WorkflowCatalog, + WorkflowCatalogEntry, + WorkflowCatalogError, + ) + from specify_cli.authentication import http as auth_http + + monkeypatch.setattr( + "specify_cli.workflows.catalog.MAX_JSON_METADATA_BYTES", 512 + ) + + class _OversizedResponse: + def __init__(self): + self._data = b"x" * 1024 + self._pos = 0 + + def read(self, n=-1): + if n < 0: + chunk = self._data[self._pos:] + self._pos = len(self._data) + return chunk + chunk = self._data[self._pos : self._pos + n] + self._pos += len(chunk) + return chunk + + def geturl(self): + return "https://example.com/catalog.json" + + def __enter__(self): + return self + + def __exit__(self, *a): + pass + + monkeypatch.setattr( + auth_http, + "open_url", + lambda url, timeout=30, redirect_validator=None: _OversizedResponse(), + ) + + catalog = WorkflowCatalog(project_dir) + entry = WorkflowCatalogEntry( + url="https://example.com/catalog.json", + name="test", + priority=1, + install_allowed=True, + ) + with pytest.raises(WorkflowCatalogError, match="exceeds maximum size"): + catalog._fetch_single_catalog(entry, force_refresh=True) + + def test_oversized_workflow_catalog_does_not_block_healthy_one(self, project_dir, monkeypatch): + """A healthy catalog still works after an oversized one was rejected.""" + from specify_cli.workflows.catalog import ( + WorkflowCatalog, + WorkflowCatalogEntry, + WorkflowCatalogError, + ) + from specify_cli.authentication import http as auth_http + + monkeypatch.setattr( + "specify_cli.workflows.catalog.MAX_JSON_METADATA_BYTES", 512 + ) + + call_count = [0] + + class _OversizedResponse: + def __init__(self): + self._data = b"x" * 1024 + self._pos = 0 + + def read(self, n=-1): + if n < 0: + chunk = self._data[self._pos:] + self._pos = len(self._data) + return chunk + chunk = self._data[self._pos : self._pos + n] + self._pos += len(chunk) + return chunk + + def geturl(self): + return "https://bad.example.com/catalog.json" + + def __enter__(self): + return self + + def __exit__(self, *a): + pass + + class _HealthyResponse: + def __init__(self): + self._data = b'{"workflows": {}}' + self._pos = 0 + + def read(self, n=-1): + if n < 0: + chunk = self._data[self._pos:] + self._pos = len(self._data) + return chunk + chunk = self._data[self._pos : self._pos + n] + self._pos += len(chunk) + return chunk + + def geturl(self): + return "https://good.example.com/catalog.json" + + def __enter__(self): + return self + + def __exit__(self, *a): + pass + + def fake_open(url, timeout=30, redirect_validator=None): + call_count[0] += 1 + if call_count[0] == 1: + return _OversizedResponse() + return _HealthyResponse() + + monkeypatch.setattr(auth_http, "open_url", fake_open) + + catalog = WorkflowCatalog(project_dir) + + bad_entry = WorkflowCatalogEntry( + url="https://bad.example.com/catalog.json", + name="bad", + priority=1, + install_allowed=True, + ) + with pytest.raises(WorkflowCatalogError, match="exceeds maximum size"): + catalog._fetch_single_catalog(bad_entry, force_refresh=True) + + good_entry = WorkflowCatalogEntry( + url="https://good.example.com/catalog.json", + name="good", + priority=1, + install_allowed=True, + ) + result = catalog._fetch_single_catalog(good_entry, force_refresh=True) + assert isinstance(result, dict) + # ===== Integration Test ===== @@ -7342,6 +7484,148 @@ def test_get_step_info_returns_entry_or_none(self, project_dir, monkeypatch): missing = catalog.get_step_info("nonexistent") assert missing is None + def test_oversized_step_catalog_response_rejected(self, project_dir, monkeypatch): + """StepCatalog._fetch_single_catalog rejects responses exceeding + MAX_JSON_METADATA_BYTES instead of reading unbounded into memory.""" + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + StepCatalogError, + ) + from specify_cli.authentication import http as auth_http + + monkeypatch.setattr( + "specify_cli.workflows.catalog.MAX_JSON_METADATA_BYTES", 512 + ) + + class _OversizedResponse: + def __init__(self): + self._data = b"x" * 1024 + self._pos = 0 + + def read(self, n=-1): + if n < 0: + chunk = self._data[self._pos:] + self._pos = len(self._data) + return chunk + chunk = self._data[self._pos : self._pos + n] + self._pos += len(chunk) + return chunk + + def geturl(self): + return "https://example.com/steps.json" + + def __enter__(self): + return self + + def __exit__(self, *a): + pass + + monkeypatch.setattr( + auth_http, + "open_url", + lambda url, timeout=30, redirect_validator=None: _OversizedResponse(), + ) + + catalog = StepCatalog(project_dir) + entry = StepCatalogEntry( + url="https://example.com/steps.json", + name="test", + priority=1, + install_allowed=True, + ) + with pytest.raises(StepCatalogError, match="exceeds maximum size"): + catalog._fetch_single_catalog(entry, force_refresh=True) + + def test_oversized_step_catalog_does_not_block_healthy_one(self, project_dir, monkeypatch): + """A healthy step catalog still works after an oversized one was rejected.""" + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + StepCatalogError, + ) + from specify_cli.authentication import http as auth_http + + monkeypatch.setattr( + "specify_cli.workflows.catalog.MAX_JSON_METADATA_BYTES", 512 + ) + + call_count = [0] + + class _OversizedResponse: + def __init__(self): + self._data = b"x" * 1024 + self._pos = 0 + + def read(self, n=-1): + if n < 0: + chunk = self._data[self._pos:] + self._pos = len(self._data) + return chunk + chunk = self._data[self._pos : self._pos + n] + self._pos += len(chunk) + return chunk + + def geturl(self): + return "https://bad.example.com/steps.json" + + def __enter__(self): + return self + + def __exit__(self, *a): + pass + + class _HealthyResponse: + def __init__(self): + self._data = b'{"steps": {}}' + self._pos = 0 + + def read(self, n=-1): + if n < 0: + chunk = self._data[self._pos:] + self._pos = len(self._data) + return chunk + chunk = self._data[self._pos : self._pos + n] + self._pos += len(chunk) + return chunk + + def geturl(self): + return "https://good.example.com/steps.json" + + def __enter__(self): + return self + + def __exit__(self, *a): + pass + + def fake_open(url, timeout=30, redirect_validator=None): + call_count[0] += 1 + if call_count[0] == 1: + return _OversizedResponse() + return _HealthyResponse() + + monkeypatch.setattr(auth_http, "open_url", fake_open) + + catalog = StepCatalog(project_dir) + + bad_entry = StepCatalogEntry( + url="https://bad.example.com/steps.json", + name="bad", + priority=1, + install_allowed=True, + ) + with pytest.raises(StepCatalogError, match="exceeds maximum size"): + catalog._fetch_single_catalog(bad_entry, force_refresh=True) + + good_entry = StepCatalogEntry( + url="https://good.example.com/steps.json", + name="good", + priority=1, + install_allowed=True, + ) + result = catalog._fetch_single_catalog(good_entry, force_refresh=True) + assert isinstance(result, dict) + # ===== Load Custom Steps Tests =====