From 32c9075b6cf67b5a4c12466ab571bd18cf38e6ec Mon Sep 17 00:00:00 2001 From: "eric.quintero@trailofbits.com" Date: Tue, 2 Jun 2026 16:26:23 +0000 Subject: [PATCH] Fail add_to_pydotorg on create errors --- add_to_pydotorg.py | 54 +++++++++++++++++++++++++++-------- tests/test_add_to_pydotorg.py | 45 +++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 12 deletions(-) diff --git a/add_to_pydotorg.py b/add_to_pydotorg.py index a10be889..0edf40ad 100755 --- a/add_to_pydotorg.py +++ b/add_to_pydotorg.py @@ -29,7 +29,7 @@ import re import subprocess import sys -from collections.abc import Generator +from collections.abc import Generator, Iterable from functools import cache from os import path from typing import Any, NoReturn @@ -308,19 +308,54 @@ def post_object(base_url: str, objtype: str, datadict: dict[str, Any]) -> int: headers=headers, ) if resp.status_code != 201: + messages = [f"Creating {objtype} failed: {resp.status_code}"] try: info = json.loads(resp.text) - print(info.get("error_message", "No error message.")) - print(info.get("traceback", "")) - except: # noqa: E722 + except json.JSONDecodeError: pass - print(f"Creating {objtype} failed: {resp.status_code}") - return -1 + else: + if isinstance(info, dict): + error_message = info.get("error_message") + traceback = info.get("traceback") + if error_message: + messages.append(str(error_message)) + if traceback: + messages.append(str(traceback)) + raise RuntimeError("\n".join(messages)) newloc = resp.headers["Location"] pk = int(newloc.strip("/").split("/")[-1]) return pk +def delete_object(base_url: str, objtype: str, pk: int) -> None: + """Delete an existing API object.""" + resp = requests.delete(base_url + f"downloads/{objtype}/{pk}/", headers=headers) + if resp.status_code != 204: + raise RuntimeError(f"Deleting {objtype} {pk} failed: {resp.status_code}") + + +def create_release_files(base_url: str, file_dicts: Iterable[dict[str, Any]]) -> int: + """Create ReleaseFile objects and clean up this run's rows on failure.""" + created_pks: list[int] = [] + try: + for file_dict in file_dicts: + file_pk = post_object(base_url, "release_file", file_dict) + created_pks.append(file_pk) + print("Created as id =", file_pk) + except Exception as create_error: + cleanup_errors = [] + for file_pk in reversed(created_pks): + try: + delete_object(base_url, "release_file", file_pk) + except Exception as cleanup_error: + cleanup_errors.append(f"{file_pk}: {cleanup_error}") + if cleanup_errors: + message = "Failed to clean up partially created release files:\n" + raise RuntimeError(message + "\n".join(cleanup_errors)) from create_error + raise + return len(created_pks) + + def sign_release_files_with_sigstore( ftp_root: str, release: str, release_files: list[tuple[str, str, str, bool, str]] ) -> None: @@ -453,7 +488,6 @@ def main() -> None: release_files = list(list_files(args.ftp_root, rel)) sign_release_files_with_sigstore(args.ftp_root, rel, release_files) - n = 0 file_dicts = {} for rfile, file_desc, os_slug, add_download, add_desc in release_files: if not os_slug: @@ -473,11 +507,7 @@ def main() -> None: ) if resp.status_code != 204: raise RuntimeError(f"deleting previous releases failed: {resp.status_code}") - for file_dict in file_dicts.values(): - file_pk = post_object(args.base_url, "release_file", file_dict) - if file_pk >= 0: - print("Created as id =", file_pk) - n += 1 + n = create_release_files(args.base_url, file_dicts.values()) print(f"Done - {n} files added") diff --git a/tests/test_add_to_pydotorg.py b/tests/test_add_to_pydotorg.py index 83d780fb..7970d990 100644 --- a/tests/test_add_to_pydotorg.py +++ b/tests/test_add_to_pydotorg.py @@ -1,7 +1,9 @@ import os from pathlib import Path +from typing import Any import pytest +import requests from pyfakefs.fake_filesystem import FakeFilesystem os.environ["AUTH_INFO"] = "test_username:test_api_key" @@ -77,6 +79,49 @@ def test_build_file_dict(tmp_path: Path) -> None: } +def test_post_object_raises_on_failure(monkeypatch: pytest.MonkeyPatch) -> None: + class Response: + status_code = 500 + text = '{"error_message": "validation failed", "traceback": "details"}' + + def fake_post(*args: Any, **kwargs: Any) -> Response: + return Response() + + monkeypatch.setattr(requests, "post", fake_post) + + with pytest.raises(RuntimeError, match="validation failed"): + add_to_pydotorg.post_object( + "https://example.invalid/api/v1/", "release_file", {"slug": "bad"} + ) + + +def test_create_release_files_cleans_up_created_rows( + monkeypatch: pytest.MonkeyPatch, +) -> None: + events: list[tuple[str, str | int]] = [] + + def fake_post_object(base_url: str, objtype: str, datadict: dict[str, Any]) -> int: + slug = str(datadict["slug"]) + events.append(("post", slug)) + if slug == "bad": + raise RuntimeError("create failed") + return 101 + + def fake_delete_object(base_url: str, objtype: str, pk: int) -> None: + events.append(("delete", pk)) + + monkeypatch.setattr(add_to_pydotorg, "post_object", fake_post_object) + monkeypatch.setattr(add_to_pydotorg, "delete_object", fake_delete_object) + + with pytest.raises(RuntimeError, match="create failed"): + add_to_pydotorg.create_release_files( + "https://example.invalid/api/v1/", + [{"slug": "ok"}, {"slug": "bad"}], + ) + + assert events == [("post", "ok"), ("post", "bad"), ("delete", 101)] + + @pytest.mark.parametrize( ["release", "expected"], [