Skip to content

Commit 5c2087c

Browse files
Quratulain-bilalQuratulain-bilal
committed
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.
1 parent 42c7230 commit 5c2087c

2 files changed

Lines changed: 292 additions & 2 deletions

File tree

‎src/specify_cli/workflows/catalog.py‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222

2323
import yaml
2424

25+
from .._download_security import MAX_JSON_METADATA_BYTES, read_response_limited
26+
2527

2628
# ---------------------------------------------------------------------------
2729
# Errors
@@ -538,7 +540,9 @@ def _validate_redirect(_old_url: str, new_url: str) -> None:
538540
entry.url, timeout=30, redirect_validator=_validate_redirect
539541
) as resp:
540542
_validate_catalog_url(resp.geturl())
541-
data = json.loads(resp.read().decode("utf-8"))
543+
data = json.loads(
544+
read_response_limited(resp, max_bytes=MAX_JSON_METADATA_BYTES).decode("utf-8")
545+
)
542546
except Exception as exc:
543547
# Fall back to cache if available
544548
if cache_file.exists():
@@ -1211,7 +1215,9 @@ def _validate_redirect(_old_url: str, new_url: str) -> None:
12111215
entry.url, timeout=30, redirect_validator=_validate_redirect
12121216
) as resp:
12131217
_validate_url(resp.geturl())
1214-
data = json.loads(resp.read().decode("utf-8"))
1218+
data = json.loads(
1219+
read_response_limited(resp, max_bytes=MAX_JSON_METADATA_BYTES).decode("utf-8")
1220+
)
12151221
except Exception as exc:
12161222
if cache_safe and cache_file.exists():
12171223
try:

‎tests/test_workflows.py‎

Lines changed: 284 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6657,6 +6657,148 @@ def _raising_open(file, mode="r", *args, **kwargs):
66576657
with pytest.raises(WorkflowValidationError, match="Failed to write catalog config"):
66586658
catalog.remove_catalog(0)
66596659

6660+
def test_oversized_workflow_catalog_response_rejected(self, project_dir, monkeypatch):
6661+
"""WorkflowCatalog._fetch_single_catalog rejects responses exceeding
6662+
MAX_JSON_METADATA_BYTES instead of reading unbounded into memory."""
6663+
from specify_cli.workflows.catalog import (
6664+
WorkflowCatalog,
6665+
WorkflowCatalogEntry,
6666+
WorkflowCatalogError,
6667+
)
6668+
from specify_cli.authentication import http as auth_http
6669+
6670+
monkeypatch.setattr(
6671+
"specify_cli.workflows.catalog.MAX_JSON_METADATA_BYTES", 512
6672+
)
6673+
6674+
class _OversizedResponse:
6675+
def __init__(self):
6676+
self._data = b"x" * 1024
6677+
self._pos = 0
6678+
6679+
def read(self, n=-1):
6680+
if n < 0:
6681+
chunk = self._data[self._pos:]
6682+
self._pos = len(self._data)
6683+
return chunk
6684+
chunk = self._data[self._pos : self._pos + n]
6685+
self._pos += len(chunk)
6686+
return chunk
6687+
6688+
def geturl(self):
6689+
return "https://example.com/catalog.json"
6690+
6691+
def __enter__(self):
6692+
return self
6693+
6694+
def __exit__(self, *a):
6695+
pass
6696+
6697+
monkeypatch.setattr(
6698+
auth_http,
6699+
"open_url",
6700+
lambda url, timeout=30, redirect_validator=None: _OversizedResponse(),
6701+
)
6702+
6703+
catalog = WorkflowCatalog(project_dir)
6704+
entry = WorkflowCatalogEntry(
6705+
url="https://example.com/catalog.json",
6706+
name="test",
6707+
priority=1,
6708+
install_allowed=True,
6709+
)
6710+
with pytest.raises(WorkflowCatalogError, match="exceeds maximum size"):
6711+
catalog._fetch_single_catalog(entry, force_refresh=True)
6712+
6713+
def test_oversized_workflow_catalog_does_not_block_healthy_one(self, project_dir, monkeypatch):
6714+
"""A healthy catalog still works after an oversized one was rejected."""
6715+
from specify_cli.workflows.catalog import (
6716+
WorkflowCatalog,
6717+
WorkflowCatalogEntry,
6718+
WorkflowCatalogError,
6719+
)
6720+
from specify_cli.authentication import http as auth_http
6721+
6722+
monkeypatch.setattr(
6723+
"specify_cli.workflows.catalog.MAX_JSON_METADATA_BYTES", 512
6724+
)
6725+
6726+
call_count = [0]
6727+
6728+
class _OversizedResponse:
6729+
def __init__(self):
6730+
self._data = b"x" * 1024
6731+
self._pos = 0
6732+
6733+
def read(self, n=-1):
6734+
if n < 0:
6735+
chunk = self._data[self._pos:]
6736+
self._pos = len(self._data)
6737+
return chunk
6738+
chunk = self._data[self._pos : self._pos + n]
6739+
self._pos += len(chunk)
6740+
return chunk
6741+
6742+
def geturl(self):
6743+
return "https://bad.example.com/catalog.json"
6744+
6745+
def __enter__(self):
6746+
return self
6747+
6748+
def __exit__(self, *a):
6749+
pass
6750+
6751+
class _HealthyResponse:
6752+
def __init__(self):
6753+
self._data = b'{"workflows": {}}'
6754+
self._pos = 0
6755+
6756+
def read(self, n=-1):
6757+
if n < 0:
6758+
chunk = self._data[self._pos:]
6759+
self._pos = len(self._data)
6760+
return chunk
6761+
chunk = self._data[self._pos : self._pos + n]
6762+
self._pos += len(chunk)
6763+
return chunk
6764+
6765+
def geturl(self):
6766+
return "https://good.example.com/catalog.json"
6767+
6768+
def __enter__(self):
6769+
return self
6770+
6771+
def __exit__(self, *a):
6772+
pass
6773+
6774+
def fake_open(url, timeout=30, redirect_validator=None):
6775+
call_count[0] += 1
6776+
if call_count[0] == 1:
6777+
return _OversizedResponse()
6778+
return _HealthyResponse()
6779+
6780+
monkeypatch.setattr(auth_http, "open_url", fake_open)
6781+
6782+
catalog = WorkflowCatalog(project_dir)
6783+
6784+
bad_entry = WorkflowCatalogEntry(
6785+
url="https://bad.example.com/catalog.json",
6786+
name="bad",
6787+
priority=1,
6788+
install_allowed=True,
6789+
)
6790+
with pytest.raises(WorkflowCatalogError, match="exceeds maximum size"):
6791+
catalog._fetch_single_catalog(bad_entry, force_refresh=True)
6792+
6793+
good_entry = WorkflowCatalogEntry(
6794+
url="https://good.example.com/catalog.json",
6795+
name="good",
6796+
priority=1,
6797+
install_allowed=True,
6798+
)
6799+
result = catalog._fetch_single_catalog(good_entry, force_refresh=True)
6800+
assert isinstance(result, dict)
6801+
66606802

66616803
# ===== Integration Test =====
66626804

@@ -7342,6 +7484,148 @@ def test_get_step_info_returns_entry_or_none(self, project_dir, monkeypatch):
73427484
missing = catalog.get_step_info("nonexistent")
73437485
assert missing is None
73447486

7487+
def test_oversized_step_catalog_response_rejected(self, project_dir, monkeypatch):
7488+
"""StepCatalog._fetch_single_catalog rejects responses exceeding
7489+
MAX_JSON_METADATA_BYTES instead of reading unbounded into memory."""
7490+
from specify_cli.workflows.catalog import (
7491+
StepCatalog,
7492+
StepCatalogEntry,
7493+
StepCatalogError,
7494+
)
7495+
from specify_cli.authentication import http as auth_http
7496+
7497+
monkeypatch.setattr(
7498+
"specify_cli.workflows.catalog.MAX_JSON_METADATA_BYTES", 512
7499+
)
7500+
7501+
class _OversizedResponse:
7502+
def __init__(self):
7503+
self._data = b"x" * 1024
7504+
self._pos = 0
7505+
7506+
def read(self, n=-1):
7507+
if n < 0:
7508+
chunk = self._data[self._pos:]
7509+
self._pos = len(self._data)
7510+
return chunk
7511+
chunk = self._data[self._pos : self._pos + n]
7512+
self._pos += len(chunk)
7513+
return chunk
7514+
7515+
def geturl(self):
7516+
return "https://example.com/steps.json"
7517+
7518+
def __enter__(self):
7519+
return self
7520+
7521+
def __exit__(self, *a):
7522+
pass
7523+
7524+
monkeypatch.setattr(
7525+
auth_http,
7526+
"open_url",
7527+
lambda url, timeout=30, redirect_validator=None: _OversizedResponse(),
7528+
)
7529+
7530+
catalog = StepCatalog(project_dir)
7531+
entry = StepCatalogEntry(
7532+
url="https://example.com/steps.json",
7533+
name="test",
7534+
priority=1,
7535+
install_allowed=True,
7536+
)
7537+
with pytest.raises(StepCatalogError, match="exceeds maximum size"):
7538+
catalog._fetch_single_catalog(entry, force_refresh=True)
7539+
7540+
def test_oversized_step_catalog_does_not_block_healthy_one(self, project_dir, monkeypatch):
7541+
"""A healthy step catalog still works after an oversized one was rejected."""
7542+
from specify_cli.workflows.catalog import (
7543+
StepCatalog,
7544+
StepCatalogEntry,
7545+
StepCatalogError,
7546+
)
7547+
from specify_cli.authentication import http as auth_http
7548+
7549+
monkeypatch.setattr(
7550+
"specify_cli.workflows.catalog.MAX_JSON_METADATA_BYTES", 512
7551+
)
7552+
7553+
call_count = [0]
7554+
7555+
class _OversizedResponse:
7556+
def __init__(self):
7557+
self._data = b"x" * 1024
7558+
self._pos = 0
7559+
7560+
def read(self, n=-1):
7561+
if n < 0:
7562+
chunk = self._data[self._pos:]
7563+
self._pos = len(self._data)
7564+
return chunk
7565+
chunk = self._data[self._pos : self._pos + n]
7566+
self._pos += len(chunk)
7567+
return chunk
7568+
7569+
def geturl(self):
7570+
return "https://bad.example.com/steps.json"
7571+
7572+
def __enter__(self):
7573+
return self
7574+
7575+
def __exit__(self, *a):
7576+
pass
7577+
7578+
class _HealthyResponse:
7579+
def __init__(self):
7580+
self._data = b'{"steps": {}}'
7581+
self._pos = 0
7582+
7583+
def read(self, n=-1):
7584+
if n < 0:
7585+
chunk = self._data[self._pos:]
7586+
self._pos = len(self._data)
7587+
return chunk
7588+
chunk = self._data[self._pos : self._pos + n]
7589+
self._pos += len(chunk)
7590+
return chunk
7591+
7592+
def geturl(self):
7593+
return "https://good.example.com/steps.json"
7594+
7595+
def __enter__(self):
7596+
return self
7597+
7598+
def __exit__(self, *a):
7599+
pass
7600+
7601+
def fake_open(url, timeout=30, redirect_validator=None):
7602+
call_count[0] += 1
7603+
if call_count[0] == 1:
7604+
return _OversizedResponse()
7605+
return _HealthyResponse()
7606+
7607+
monkeypatch.setattr(auth_http, "open_url", fake_open)
7608+
7609+
catalog = StepCatalog(project_dir)
7610+
7611+
bad_entry = StepCatalogEntry(
7612+
url="https://bad.example.com/steps.json",
7613+
name="bad",
7614+
priority=1,
7615+
install_allowed=True,
7616+
)
7617+
with pytest.raises(StepCatalogError, match="exceeds maximum size"):
7618+
catalog._fetch_single_catalog(bad_entry, force_refresh=True)
7619+
7620+
good_entry = StepCatalogEntry(
7621+
url="https://good.example.com/steps.json",
7622+
name="good",
7623+
priority=1,
7624+
install_allowed=True,
7625+
)
7626+
result = catalog._fetch_single_catalog(good_entry, force_refresh=True)
7627+
assert isinstance(result, dict)
7628+
73457629

73467630
# ===== Load Custom Steps Tests =====
73477631

0 commit comments

Comments
 (0)