diff --git a/src/sentry/models/releasefile.py b/src/sentry/models/releasefile.py index fc0595d6eac2..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 @@ -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" @@ -157,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 @@ -177,9 +195,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 +261,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 +270,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 +303,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..84ab085f0165 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,24 @@ 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 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}