fix: use bounded read for integration catalog HTTP responses (#3763)

* fix(skills): match closing frontmatter delimiter on its own line

SkillsIntegration.setup parsed each command template's frontmatter with
raw.split("---", 2). A bare substring split stops at the first `---`
*anywhere*, so a template whose description embeds `---` (e.g.
"Separate sections with --- markers") truncated the parsed frontmatter:
later keys were dropped, the description fell back to the generic default,
and the leftover frontmatter spilled into the skill body.

Scan for the closing `---` on its own line instead, for both the
description parse and the body strip. The frontmatter block is parsed
unstripped so trailing newlines in literal (|) block scalars still survive,
and the body slice keeps the newline after the marker so output stays
byte-for-byte identical to the old split for well-formed templates.

Adds regression tests covering the dashed-description truncation and the
frontmatter-spilled-into-body cases.

* fix: use bounded read for integration catalog HTTP responses

The integration 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), consistent with how other JSON fetch paths in the codebase
(_version.py, _github_http.py, authentication/azure_devops.py) already
enforce bounded reads.

Pass error_type=IntegrationCatalogError so oversized catalogs are
caught by the existing per-entry recovery path in
_get_merged_integrations() rather than aborting the entire merge.

Add regression test verifying oversized responses are rejected as
IntegrationCatalogError and that healthy catalogs remain usable.

Author: Quratulain-bilal

src/specify_cli/integrations/base.py

@@ -1652,13 +1652,27 @@ def setup(
             command_name = src_file.stem  # e.g. "plan"
             skill_name = f"speckit-{command_name.replace('.', '-')}"
 
-            # Parse frontmatter for description
+            # Parse frontmatter for description. Locate the closing ``---`` on
+            # its own line rather than with ``raw.split("---", 2)`` — a bare
+            # substring split stops at the first ``---`` *anywhere*, including
+            # one inside a value such as ``description: Separate sections
+            # with ---``, which truncates the frontmatter and drops later keys.
+            # The block between the delimiters is parsed unstripped so trailing
+            # newlines in literal (``|``) block scalars survive.
             frontmatter: dict[str, Any] = {}
             if raw.startswith("---"):
-                parts = raw.split("---", 2)
-                if len(parts) >= 3:
+                fm_lines = raw.splitlines(keepends=True)
+                fm_close = next(
+                    (
+                        i
+                        for i in range(1, len(fm_lines))
+                        if fm_lines[i].rstrip() == "---"
+                    ),
+                    None,
+                )
+                if fm_close is not None:
                     try:
-                        fm = yaml.safe_load(parts[1])
+                        fm = yaml.safe_load("".join(fm_lines[1:fm_close]))
                         if isinstance(fm, dict):
                             frontmatter = fm
                     except yaml.YAMLError:
@@ -1673,11 +1687,27 @@ def setup(
             # Strip the processed frontmatter — we rebuild it for skills.
             # Preserve leading whitespace in the body to match release ZIP
             # output byte-for-byte (the template body starts with \n after
-            # the closing ---).
+            # the closing ---). Scan for the closing ``---`` on its own line
+            # rather than ``split("---", 2)`` so a ``---`` embedded in a value
+            # does not truncate the frontmatter and spill it into the body.
             if processed_body.startswith("---"):
-                parts = processed_body.split("---", 2)
-                if len(parts) >= 3:
-                    processed_body = parts[2]
+                body_lines = processed_body.splitlines(keepends=True)
+                close_idx = next(
+                    (
+                        i
+                        for i in range(1, len(body_lines))
+                        if body_lines[i].rstrip() == "---"
+                    ),
+                    None,
+                )
+                if close_idx is not None:
+                    # Keep whatever trails the ``---`` marker on the closing
+                    # line (normally just the newline) so the body stays
+                    # byte-for-byte identical to ``split("---", 2)[2]``. The
+                    # line-anchored check guarantees ``---`` sits at index 0.
+                    processed_body = body_lines[close_idx][3:] + "".join(
+                        body_lines[close_idx + 1 :]
+                    )
 
             # Select description — use the original template description
             # to stay byte-for-byte identical with release ZIP output.

src/specify_cli/integrations/catalog.py

@@ -21,6 +21,7 @@
 import yaml
 from packaging import version as pkg_version
 
+from .._download_security import MAX_JSON_METADATA_BYTES, read_response_limited
 from ..catalogs import CatalogEntry, CatalogStackBase
 
 
@@ -200,7 +201,14 @@ def _fetch_single_catalog(
                 final_url = resp.geturl()
                 if final_url != entry.url:
                     self._validate_catalog_url(final_url)
-                catalog_data = json.loads(resp.read())
+                catalog_data = json.loads(
+                    read_response_limited(
+                        resp,
+                        max_bytes=MAX_JSON_METADATA_BYTES,
+                        error_type=IntegrationCatalogError,
+                        label=f"catalog from {entry.url}",
+                    )
+                )
 
             shape_error = _catalog_shape_error(catalog_data)
             if shape_error is not None:

tests/integrations/test_integration_catalog.py

@@ -220,6 +220,33 @@ 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."""
 
@@ -230,9 +257,16 @@ 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):
-                return self._data
+            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
@@ -395,6 +429,90 @@ 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)
@@ -592,8 +710,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
-            def read(self):
-                return self._data
+                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):

tests/integrations/test_skill_frontmatter_quoting.py

@@ -34,6 +34,19 @@
 Body of the command.
 """
 
+# A description whose value contains an embedded ``---``. A substring split
+# (``raw.split("---", 2)``) stops at this inner marker, truncating the parsed
+# frontmatter — the closing document separator on its own line is the real
+# boundary. See TestSkillFrontmatterEmbeddedDashes below.
+DASHED_DESCRIPTION = "Separate sections with --- markers"
+DASHED_TEMPLATE = """---
+description: Separate sections with --- markers
+name-marker: sentinel
+---
+
+Body of the command.
+"""
+
 
 def _parse_frontmatter(skill_file: Path) -> dict:
     content = skill_file.read_text(encoding="utf-8")
@@ -90,6 +103,62 @@ def test_control_character_description_parses(self, tmp_path, monkeypatch):
         assert fm["description"] == CONTROL
 
 
+def _parse_frontmatter_line_anchored(skill_file: Path) -> dict:
+    """Parse SKILL.md frontmatter using the closing ``---`` on its own line.
+
+    Unlike ``_parse_frontmatter`` (which uses ``split("---", 2)``), this is
+    robust to a ``---`` embedded in a value, so it can validate that the
+    generated frontmatter is itself well formed.
+    """
+    content = skill_file.read_text(encoding="utf-8")
+    assert content.startswith("---\n")
+    lines = content.splitlines(keepends=True)
+    end = next(i for i in range(1, len(lines)) if lines[i].rstrip() == "---")
+    return yaml.safe_load("".join(lines[1:end]))
+
+
+class TestSkillFrontmatterEmbeddedDashes:
+    """A ``---`` inside a description value must not truncate parsing (#3634).
+
+    The skills setup path parsed template frontmatter with
+    ``raw.split("---", 2)``, which stops at the first ``---`` *anywhere* —
+    including one inside a value such as ``description: ... --- ...``. That
+    dropped every frontmatter key after the marker (so the description fell
+    back to the generic default) and spilled the leftover frontmatter into
+    the skill body. The parser must match the closing ``---`` on its own line.
+    """
+
+    def _generate(self, tmp_path, monkeypatch, template: str) -> Path:
+        integration = get_integration("agy")
+        monkeypatch.setattr(
+            integration,
+            "shared_commands_dir",
+            lambda: _fake_templates(tmp_path, template),
+        )
+        manifest = IntegrationManifest("agy", tmp_path)
+        created = integration.setup(tmp_path, manifest)
+        skill_files = [f for f in created if f.name == "SKILL.md"]
+        assert len(skill_files) == 1
+        return skill_files[0]
+
+    def test_dashed_description_is_preserved(self, tmp_path, monkeypatch):
+        skill_file = self._generate(tmp_path, monkeypatch, DASHED_TEMPLATE)
+        fm = _parse_frontmatter_line_anchored(skill_file)
+        # Buggy split("---", 2) truncates the value to "Separate sections with"
+        # (or drops it entirely, falling back to "Spec Kit: plan workflow").
+        assert fm["description"] == DASHED_DESCRIPTION
+
+    def test_leftover_frontmatter_not_spilled_into_body(self, tmp_path, monkeypatch):
+        skill_file = self._generate(tmp_path, monkeypatch, DASHED_TEMPLATE)
+        content = skill_file.read_text(encoding="utf-8")
+        lines = content.splitlines(keepends=True)
+        end = next(i for i in range(1, len(lines)) if lines[i].rstrip() == "---")
+        body = "".join(lines[end + 1 :])
+        # The template's trailing frontmatter key must not leak into the body.
+        assert "name-marker: sentinel" not in body
+        assert "Body of the command." in body
+
+
 class TestHermesSkillFrontmatterQuoting:
     def test_multiline_description_survives(self, tmp_path, monkeypatch):
         home = tmp_path / "home"