From 4d3474f85e6af51310a0bd204f311a6da46c50c1 Mon Sep 17 00:00:00 2001 From: Joshua Li Date: Sat, 31 Oct 2026 10:25:06 -0700 Subject: [PATCH 1/2] fix(releases): validate artifact manifest JSON shape The release archive manifest and artifact index are user-uploaded JSON. Validate that the decoded value is an object before treating it as one, so malformed input fails at the boundary instead of raising AttributeError deeper in the archive reader. --- src/sentry/models/releasefile.py | 25 +++++++++++++++++++------ tests/sentry/models/test_releasefile.py | 10 ++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/sentry/models/releasefile.py b/src/sentry/models/releasefile.py index fc0595d6eac2..7ea2a918db22 100644 --- a/src/sentry/models/releasefile.py +++ b/src/sentry/models/releasefile.py @@ -36,6 +36,20 @@ logger = logging.getLogger(__name__) +def _load_json_object(data: str | bytes) -> dict[str, object]: + raw = json.loads(data) + if not isinstance(raw, dict): + raise ValueError("expected a JSON object") + return raw + + +def _load_json_object_from_file(fp: IO[bytes]) -> dict[str, object]: + raw = json.load(fp) + if not isinstance(raw, dict): + raise ValueError("expected a JSON object") + return raw + + ARTIFACT_INDEX_FILENAME = "artifact-index.json" ARTIFACT_INDEX_TYPE = "release.artifact-index" @@ -177,9 +191,8 @@ def info(self, filename: str) -> zipfile.ZipInfo: def read(self, filename: str) -> bytes: return self._zip_file.read(filename) - def _read_manifest(self) -> dict: - manifest_bytes = self.read("manifest.json") - return json.loads(manifest_bytes.decode("utf-8")) + def _read_manifest(self) -> dict[str, object]: + return _load_json_object(self.read("manifest.json")) def get_file_by_url(self, url: str) -> tuple[IO[bytes], dict]: """Return file-like object and headers. @@ -244,7 +257,7 @@ def __init__(self, release: Release, dist: Distribution | None, **filter_args): self._ident = ReleaseFile.get_ident(ARTIFACT_INDEX_FILENAME, dist and dist.name) self._filter_args = filter_args # Extra constraints on artifact index release file - def readable_data(self) -> dict | None: + def readable_data(self) -> dict[str, object] | None: """Simple read, no synchronization necessary""" try: releasefile = self._releasefile_qs()[0] @@ -253,7 +266,7 @@ def readable_data(self) -> dict | None: else: fp = releasefile.file.getfile() with fp: - return json.load(fp) + return _load_json_object_from_file(fp) @contextmanager def writable_data(self, create: bool, initial_artifact_count=None): @@ -286,7 +299,7 @@ def writable_data(self, create: bool, initial_artifact_count=None): source_file = releasefile.file if source_file.type != ARTIFACT_INDEX_TYPE: raise RuntimeError("Unexpected file type for artifact index") - raw_data = json.load(source_file.getfile()) + raw_data = _load_json_object_from_file(source_file.getfile()) index_data = _ArtifactIndexData(raw_data) yield index_data # editable reference to index diff --git a/tests/sentry/models/test_releasefile.py b/tests/sentry/models/test_releasefile.py index 9f99d5207d13..6d4462b336a2 100644 --- a/tests/sentry/models/test_releasefile.py +++ b/tests/sentry/models/test_releasefile.py @@ -10,6 +10,7 @@ from sentry.models.files.file import File from sentry.models.releasefile import ( ARTIFACT_INDEX_FILENAME, + ReleaseArchive, ReleaseFile, _ArtifactIndexGuard, delete_from_artifact_index, @@ -72,6 +73,15 @@ def test_count_artifacts(self) -> None: class ReleaseArchiveTestCase(TestCase): + def test_manifest_must_be_a_json_object(self) -> None: + buffer = BytesIO() + with ZipFile(buffer, mode="w") as zf: + zf.writestr("manifest.json", "[]") + buffer.seek(0) + + with pytest.raises(ValueError, match="expected a JSON object"): + ReleaseArchive(buffer) + def create_archive(self, fields, files, dist=None): manifest = dict( fields, files={filename: {"url": f"fake://{filename}"} for filename in files} From c8353b2556b5cd9d5cf0ac534cfb9488a73250b8 Mon Sep 17 00:00:00 2001 From: Joshua Li Date: Sat, 31 Oct 2026 10:32:35 -0700 Subject: [PATCH 2/2] fix(releases): validate manifest files shape The manifest root is validated, but `files` still needs an object shape before it is counted and indexed. Reject non-object values at the archive boundary and cover both malformed shapes. --- src/sentry/models/releasefile.py | 10 +++++++--- tests/sentry/models/test_releasefile.py | 9 +++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/sentry/models/releasefile.py b/src/sentry/models/releasefile.py index 7ea2a918db22..f0e493491c67 100644 --- a/src/sentry/models/releasefile.py +++ b/src/sentry/models/releasefile.py @@ -6,7 +6,7 @@ from hashlib import sha1 from io import BytesIO from tempfile import TemporaryDirectory -from typing import IO, ClassVar, Self +from typing import IO, ClassVar, Self, cast from urllib.parse import urlunsplit from django.db import models, router @@ -171,9 +171,13 @@ def __init__(self, fileobj: IO): self._fileobj = fileobj self._zip_file = zipfile.ZipFile(self._fileobj) self.manifest = self._read_manifest() - self.artifact_count = len(self.manifest.get("files", {})) files = self.manifest.get("files", {}) - self._entries_by_url = {entry["url"]: (path, entry) for path, entry in files.items()} + if not isinstance(files, dict): + raise ValueError("expected manifest files to be a JSON object") + self.artifact_count = len(files) + self._entries_by_url = { + cast(str, entry["url"]): (path, entry) for path, entry in files.items() + } def __enter__(self): return self diff --git a/tests/sentry/models/test_releasefile.py b/tests/sentry/models/test_releasefile.py index 6d4462b336a2..84ab085f0165 100644 --- a/tests/sentry/models/test_releasefile.py +++ b/tests/sentry/models/test_releasefile.py @@ -82,6 +82,15 @@ def test_manifest_must_be_a_json_object(self) -> None: with pytest.raises(ValueError, match="expected a JSON object"): ReleaseArchive(buffer) + def test_manifest_files_must_be_a_json_object(self) -> None: + buffer = BytesIO() + with ZipFile(buffer, mode="w") as zf: + zf.writestr("manifest.json", json.dumps({"files": []})) + buffer.seek(0) + + with pytest.raises(ValueError, match="manifest files to be a JSON object"): + ReleaseArchive(buffer) + def create_archive(self, fields, files, dist=None): manifest = dict( fields, files={filename: {"url": f"fake://{filename}"} for filename in files}