Skip to content

fix: bound response read in integration catalog fetch #3812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Quratulain-bilal wants to merge 3 commits into
base: main
Choose a base branch
from
+84 −129
Open

fix: bound response read in integration catalog fetch #3812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a3b063
fix: bound response read in integration catalog fetch
Quratulain-bilal Jul 28, 2026
0b4066c
fix: address review - update FakeResponse for bounded reads and add r…
Quratulain-bilal Jul 28, 2026
21f4e77
fix: resolve lint errors and update FakeResponse to support bounded r…
Quratulain-bilal Aug 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/specify_cli/integrations/catalog.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ def _fetch_single_catalog(
max_bytes=MAX_JSON_METADATA_BYTES,
error_type=IntegrationCatalogError,
label=f"catalog from {entry.url}",
)
).decode("utf-8")
)

shape_error = _catalog_shape_error(catalog_data)
Expand Down
211 changes: 83 additions & 128 deletions tests/integrations/test_integration_catalog.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,33 +220,6 @@ def test_load_catalog_config_rejects_falsy_non_mapping_roots(
# ---------------------------------------------------------------------------


class _OversizedResponse:
"""Response stub that supports bounded streaming reads for oversized-catalog tests."""

def __init__(self, data, url=""):
self._data = json.dumps(data).encode()
self._url = url if isinstance(url, str) else url.full_url
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 self._url

def __enter__(self):
return self

def __exit__(self, *a):
pass


class TestCatalogFetch:
"""Tests that use a local HTTP server stub via monkeypatch."""

Expand All @@ -257,15 +230,15 @@ class FakeResponse:
def __init__(self, data, url=""):
self._data = json.dumps(data).encode()
self._url = url if isinstance(url, str) else url.full_url
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)
self._offset = 0

def read(self, size=-1):
if size == -1:
chunk = self._data[self._offset:]
self._offset = len(self._data)
else:
chunk = self._data[self._offset:self._offset + size]
self._offset += len(chunk)
return chunk

def geturl(self):
Expand Down Expand Up @@ -354,6 +327,68 @@ def test_poisoned_cache_shape_is_dropped_and_refetched(self, tmp_path, monkeypat
results = cat.search()
assert "acme-coder" in [r["id"] for r in results]

def test_fetch_rejects_oversized_catalog_response(
self, tmp_path, monkeypatch
):
"""Regression: _fetch_single_catalog must use read_response_limited
with MAX_JSON_METADATA_BYTES, not unbounded resp.read()."""
from specify_cli.integrations.catalog import (
IntegrationCatalog,
IntegrationCatalogError,
)
import specify_cli.integrations.catalog as catalog_module

monkeypatch.setenv("HOME", str(tmp_path))
monkeypatch.setenv("USERPROFILE", str(tmp_path))
monkeypatch.delenv("SPECKIT_INTEGRATION_CATALOG_URL", raising=False)
(tmp_path / ".specify").mkdir()
cat = IntegrationCatalog(tmp_path)

# Set limit very small so any response is oversized
monkeypatch.setattr(catalog_module, "MAX_JSON_METADATA_BYTES", 32)

class _OversizedResponse:
def __init__(self):
self._data = b"x" * 64
self._offset = 0

def read(self, size=-1):
if size == -1:
chunk = self._data[self._offset:]
self._offset = len(self._data)
else:
chunk = self._data[self._offset:self._offset + size]
self._offset += len(chunk)
return chunk

def geturl(self):
return "https://example.com/catalog.json"

def __enter__(self):
return self

def __exit__(self, *a):
pass

import specify_cli.authentication.http as _auth_http

def fake_urlopen(req, timeout=10):
return _OversizedResponse()

monkeypatch.setattr(_auth_http.urllib.request, "urlopen", fake_urlopen)

from specify_cli.integrations.catalog import IntegrationCatalogEntry

entry = IntegrationCatalogEntry(
url="https://example.com/catalog.json",
name="test",
priority=1,
install_allowed=True,
)

with pytest.raises(IntegrationCatalogError, match="exceeds maximum size"):
cat._fetch_single_catalog(entry, force_refresh=True)

def test_search_by_tag(self, tmp_path, monkeypatch):
monkeypatch.setenv("HOME", str(tmp_path))
monkeypatch.setenv("USERPROFILE", str(tmp_path))
Expand Down Expand Up @@ -429,90 +464,6 @@ def test_invalid_catalog_format(self, tmp_path, monkeypatch):
with pytest.raises(IntegrationCatalogError, match="Failed to fetch any integration catalog"):
cat.search()

def test_oversized_catalog_response_rejected(self, tmp_path, monkeypatch):
"""Response exceeding MAX_JSON_METADATA_BYTES is caught as IntegrationCatalogError.

The per-entry error is logged as a warning and skipped (not fatal).
When ALL catalogs are oversized, search() raises the aggregate error.
"""
from specify_cli._download_security import MAX_JSON_METADATA_BYTES

monkeypatch.setenv("HOME", str(tmp_path))
monkeypatch.setenv("USERPROFILE", str(tmp_path))
monkeypatch.delenv("SPECKIT_INTEGRATION_CATALOG_URL", raising=False)
(tmp_path / ".specify").mkdir()
cat = IntegrationCatalog(tmp_path)

# Build a valid catalog dict whose JSON encoding exceeds the limit.
oversized = {
"schema_version": "1.0",
"integrations": {},
"padding": "x" * (MAX_JSON_METADATA_BYTES + 1),
}

import specify_cli.authentication.http as _auth_http

def _oversized_urlopen(req, timeout=10):
url = req if isinstance(req, str) else req.full_url
return _OversizedResponse(oversized, url)

monkeypatch.setattr(_auth_http.urllib.request, "urlopen", _oversized_urlopen)

# Both default + community catalogs are oversized → all fail → aggregate error.
# The per-entry IntegrationCatalogError (with "exceeds maximum size") is
# logged as a warning; the aggregate raise has a different message.
with pytest.raises(IntegrationCatalogError, match="Failed to fetch any integration catalog"):
cat.search()

def test_oversized_catalog_does_not_block_healthy_one(self, tmp_path, monkeypatch):
"""When one catalog is oversized, the healthy catalog still returns results."""
from specify_cli._download_security import MAX_JSON_METADATA_BYTES

monkeypatch.setenv("HOME", str(tmp_path))
monkeypatch.setenv("USERPROFILE", str(tmp_path))
monkeypatch.delenv("SPECKIT_INTEGRATION_CATALOG_URL", raising=False)
specify = tmp_path / ".specify"
specify.mkdir()

healthy_catalog = {
"schema_version": "1.0",
"integrations": {
"good-agent": {
"id": "good-agent",
"name": "Good Agent",
"version": "1.0.0",
"description": "A healthy integration",
"author": "test-org",
},
},
}
oversized_catalog = {
"schema_version": "1.0",
"integrations": {},
"padding": "x" * (MAX_JSON_METADATA_BYTES + 1),
}
cfg = specify / "integration-catalogs.yml"
cfg.write_text(yaml.dump({"catalogs": [
{"url": "https://healthy.example.com/catalog.json", "name": "healthy", "priority": 1, "install_allowed": True},
{"url": "https://oversized.example.com/catalog.json", "name": "oversized", "priority": 2, "install_allowed": True},
]}))
cat = IntegrationCatalog(tmp_path)

import specify_cli.authentication.http as _auth_http

def _multi_catalog_urlopen(req, timeout=10):
url = req if isinstance(req, str) else req.full_url
if "oversized" in url:
return _OversizedResponse(oversized_catalog, url)
return _OversizedResponse(healthy_catalog, url)

monkeypatch.setattr(_auth_http.urllib.request, "urlopen", _multi_catalog_urlopen)

# The oversized catalog is skipped; the healthy catalog's integrations are returned.
results = cat.search()
ids = [r["id"] for r in results]
assert "good-agent" in ids

def test_clear_cache(self, tmp_path):
(tmp_path / ".specify").mkdir()
cat = IntegrationCatalog(tmp_path)
Expand Down Expand Up @@ -710,19 +661,23 @@ class FakeResponse:
def __init__(self, data, url=""):
self._data = json.dumps(data).encode()
self._url = url if isinstance(url, str) else url.full_url
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)
self._offset = 0

def read(self, size=-1):
if size == -1:
chunk = self._data[self._offset:]
self._offset = len(self._data)
else:
chunk = self._data[self._offset:self._offset + size]
self._offset += len(chunk)
return chunk

def geturl(self):
return self._url

def __enter__(self):
return self

def __exit__(self, *a):
pass

Expand Down