diff --git a/scripts/validate_package_artifact.py b/scripts/validate_package_artifact.py index c17b668..f0bd9aa 100644 --- a/scripts/validate_package_artifact.py +++ b/scripts/validate_package_artifact.py @@ -5,11 +5,13 @@ import argparse import email +import hashlib import re +import stat import sys import tarfile import zipfile -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import NoReturn PACKAGE_NAME = "base-cli" @@ -18,6 +20,8 @@ REQUIRED_DEPENDENCIES = ("click<8.6,>=8.1",) YAML_DEPENDENCY_PREFIX = "PyYAML<7,>=6.0" DOCUMENTATION_URL = "Documentation, https://basefoundry.github.io/base-cli/" +STARTUP_HOOK_NAMES = frozenset({"sitecustomize.py", "usercustomize.py"}) +NATIVE_OR_EXECUTABLE_SUFFIXES = (".dll", ".dylib", ".exe", ".pyd", ".so") ALLOWED_WHEEL_DIST_INFO_FILES = frozenset({"METADATA", "RECORD", "WHEEL", "top_level.txt", "entry_points.txt"}) ALLOWED_SDIST_FILES = frozenset( { @@ -39,11 +43,11 @@ "docs/", "examples/", "compatibility/", - "lib/python/base_cli/", + f"lib/python/{IMPORT_NAME}/", "scripts/", "tests/", "base_cli.egg-info/", - "lib/python/base_cli.egg-info/", + f"lib/python/{IMPORT_NAME}.egg-info/", ) @@ -60,11 +64,34 @@ def read_expected_version() -> str: return lines[0].strip() -def expected_package_files() -> set[str]: +def expected_package_hashes() -> dict[str, str]: package_root = Path(__file__).resolve().parents[1] / "lib" / "python" / IMPORT_NAME if not package_root.is_dir(): fail(f"package source directory does not exist: {package_root}") - return {path.relative_to(package_root).as_posix() for path in package_root.rglob("*") if path.is_file()} + return { + path.relative_to(package_root).as_posix(): hashlib.sha256(path.read_bytes()).hexdigest() + for path in package_root.rglob("*") + if path.is_file() and "__pycache__" not in path.parts and path.suffix != ".pyc" + } + + +def validate_member_name(name: str, artifact_name: str) -> None: + normalized = name.replace("\\", "/") + path = PurePosixPath(normalized) + if not normalized or path.is_absolute() or ".." in path.parts: + fail(f"{artifact_name} contains an unsafe archive member path {name!r}") + lower_name = normalized.lower() + if lower_name.endswith(".pth") or path.name.lower() in STARTUP_HOOK_NAMES: + fail(f"{artifact_name} contains a Python startup hook {name!r}") + if lower_name.endswith(NATIVE_OR_EXECUTABLE_SUFFIXES): + fail(f"{artifact_name} contains an unexpected executable or native library {name!r}") + + +def validate_archive_names(names: list[str], artifact_name: str) -> None: + if len(names) != len(set(names)): + fail(f"{artifact_name} contains duplicate archive members") + for name in names: + validate_member_name(name, artifact_name) def read_expected_readme() -> str: @@ -91,9 +118,16 @@ def validate_metadata_description(metadata: email.message.Message, expected_read fail(f"{artifact_name} long description does not match the source README.md") -def validate_wheel(path: Path, expected_version: str, package_files: set[str], expected_readme: str) -> None: +def validate_wheel( + path: Path, + expected_version: str, + package_files: set[str], + package_hashes: dict[str, str], + expected_readme: str, +) -> None: with zipfile.ZipFile(path) as archive: names = archive.namelist() + validate_archive_names(names, path.name) metadata_names = [name for name in names if name.endswith(".dist-info/METADATA")] if len(metadata_names) != 1: fail(f"{path.name} must contain exactly one dist-info METADATA file") @@ -140,11 +174,22 @@ def validate_wheel(path: Path, expected_version: str, package_files: set[str], e fail(f"{path.name} contains repository test files") dist_info_prefix = metadata_names[0].rsplit("/", 1)[0] + "/" - for name in names: + for info in archive.infolist(): + name = info.filename + mode_bits = info.external_attr >> 16 + if stat.S_ISLNK(mode_bits): + fail(f"{path.name} contains a symbolic link archive member {name!r}") + if info.is_dir(): + continue + mode = mode_bits & 0o7777 + if mode & 0o111: + fail(f"{path.name} contains an executable archive member {name!r}") if name.startswith(f"{IMPORT_NAME}/"): relative = name[len(IMPORT_NAME) + 1 :] if relative not in package_files: fail(f"{path.name} contains non-allowlisted package file {name!r}") + if hashlib.sha256(archive.read(name)).hexdigest() != package_hashes[relative]: + fail(f"{path.name} package file {name!r} differs from the reviewed source") continue if name.startswith(dist_info_prefix): relative = name[len(dist_info_prefix) :] @@ -156,10 +201,11 @@ def validate_wheel(path: Path, expected_version: str, package_files: set[str], e fail(f"{path.name} contains non-allowlisted archive member {name!r}") -def validate_sdist(path: Path, expected_version: str, expected_readme: str) -> None: +def validate_sdist(path: Path, expected_version: str, package_hashes: dict[str, str], expected_readme: str) -> None: with tarfile.open(path, "r:gz") as archive: members = archive.getmembers() names = [member.name for member in members] + validate_archive_names(names, path.name) roots = {name.split("/", 1)[0] for name in names if name} if len(roots) != 1: fail(f"{path.name} must contain exactly one top-level directory") @@ -169,11 +215,23 @@ def validate_sdist(path: Path, expected_version: str, expected_readme: str) -> N name = member.name if name == root or member.isdir(): continue + if member.issym() or member.islnk(): + fail(f"{path.name} contains a symbolic or hard link: {name!r}") + if member.mode & 0o111 and not (name.startswith(f"{root_prefix}scripts/") and name.endswith(".py")): + fail(f"{path.name} contains an executable archive member {name!r}") if not name.startswith(root_prefix): fail(f"{path.name} contains member outside its top-level directory: {name!r}") relative = name[len(root_prefix) :] if relative not in ALLOWED_SDIST_FILES and not relative.startswith(ALLOWED_SDIST_PREFIXES): fail(f"{path.name} contains non-allowlisted archive member {relative!r}") + package_prefix = f"lib/python/{IMPORT_NAME}/" + if relative.startswith(package_prefix): + package_relative = relative[len(package_prefix) :] + payload = archive.extractfile(member) + if package_relative not in package_hashes or payload is None: + fail(f"{path.name} contains non-allowlisted package file {relative!r}") + if hashlib.sha256(payload.read()).hexdigest() != package_hashes[package_relative]: + fail(f"{path.name} package file {relative!r} differs from the reviewed source") required_suffixes = {"pyproject.toml", "README.md", "LICENSE", "VERSION"} present_suffixes = {name.rsplit("/", 1)[-1] for name in names} missing = required_suffixes - present_suffixes @@ -220,8 +278,9 @@ def main() -> None: fail(f"artifact filenames do not match version {expected_version}") expected_readme = read_expected_readme() - validate_wheel(wheels[0], expected_version, expected_package_files(), expected_readme) - validate_sdist(sdists[0], expected_version, expected_readme) + package_hashes = expected_package_hashes() + validate_wheel(wheels[0], expected_version, set(package_hashes), package_hashes, expected_readme) + validate_sdist(sdists[0], expected_version, package_hashes, expected_readme) print(f"Validated {PACKAGE_NAME} {expected_version}: wheel, sdist, metadata, package data, and test boundary.") diff --git a/tests/test_validate_package_artifact.py b/tests/test_validate_package_artifact.py index 004a5d2..f36b8d5 100644 --- a/tests/test_validate_package_artifact.py +++ b/tests/test_validate_package_artifact.py @@ -1,10 +1,65 @@ from __future__ import annotations import sys +import tarfile +import zipfile from pathlib import Path sys.path.insert(0, str(Path(__file__).resolve().parents[1])) -from scripts.validate_package_artifact import REQUIRED_DEPENDENCIES, normalized_text +import pytest + +from scripts.validate_package_artifact import ( + REQUIRED_DEPENDENCIES, + normalized_text, + validate_sdist, + validate_wheel, +) + + +def _write_valid_wheel(path: Path, *, include_directory: bool = False, include_symlink: bool = False) -> None: + package_init = b"from __future__ import annotations\n" + metadata = b"""Metadata-Version: 2.1 +Name: base-cli +Version: 0.5.1 +License: Apache-2.0 +Requires-Python: >=3.10 +Requires-Dist: click<8.6,>=8.1 +Provides-Extra: yaml +Requires-Dist: PyYAML<7,>=6.0; extra == \"yaml\" +Project-URL: Documentation, https://basefoundry.github.io/base-cli/ + +README +""" + with zipfile.ZipFile(path, "w") as archive: + if include_directory: + directory = zipfile.ZipInfo("base_cli/") + directory.create_system = 3 + directory.external_attr = 0o40755 << 16 + archive.writestr(directory, b"") + archive.writestr("base_cli/__init__.py", package_init) + archive.writestr("base_cli/py.typed", b"") + archive.writestr("base_cli-0.5.1.dist-info/METADATA", metadata) + archive.writestr("base_cli-0.5.1.dist-info/LICENSE", b"license") + archive.writestr("base_cli-0.5.1.dist-info/WHEEL", b"Wheel-Version: 1.0\n") + archive.writestr("base_cli-0.5.1.dist-info/RECORD", b"") + if include_symlink: + symlink = zipfile.ZipInfo("base_cli/linked.py") + symlink.create_system = 3 + symlink.external_attr = 0o120777 << 16 + archive.writestr(symlink, b"base_cli/__init__.py") + + +def _validate_test_wheel(path: Path) -> None: + validate_wheel( + path, + "0.5.1", + {"__init__.py", "py.typed"}, + { + "__init__.py": "5384bfdb2df380b6557cc7a71d16891415bccaa87699406e236f752c6415389f", + "py.typed": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + }, + "README", + ) def test_artifact_validator_matches_declared_click_window() -> None: @@ -13,3 +68,60 @@ def test_artifact_validator_matches_declared_click_window() -> None: def test_artifact_validator_normalizes_only_line_endings_and_outer_whitespace() -> None: assert normalized_text("\r\nREADME\r\n") == "README" + + +def test_wheel_validator_allows_executable_bits_on_directory_entries(tmp_path: Path) -> None: + wheel = tmp_path / "base_cli-0.5.1-py3-none-any.whl" + _write_valid_wheel(wheel, include_directory=True) + + _validate_test_wheel(wheel) + + +def test_wheel_validator_rejects_symbolic_links(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + wheel = tmp_path / "base_cli-0.5.1-py3-none-any.whl" + _write_valid_wheel(wheel, include_symlink=True) + + with pytest.raises(SystemExit): + _validate_test_wheel(wheel) + assert "symbolic link" in capsys.readouterr().err + + +def test_wheel_validator_rejects_duplicate_members_before_metadata_parsing( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + wheel = tmp_path / "base_cli-0.5.1-py3-none-any.whl" + with zipfile.ZipFile(wheel, "w") as archive: + archive.writestr("base_cli/__init__.py", b"first") + archive.writestr("base_cli/__init__.py", b"second") + + with pytest.raises(SystemExit): + validate_wheel(wheel, "0.5.1", {"__init__.py"}, {"__init__.py": ""}, "") + assert "duplicate archive members" in capsys.readouterr().err + + +def test_wheel_validator_rejects_startup_hooks_before_metadata_parsing( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + wheel = tmp_path / "base_cli-0.5.1-py3-none-any.whl" + with zipfile.ZipFile(wheel, "w") as archive: + archive.writestr("base_cli/sitecustomize.py", b"import os") + + with pytest.raises(SystemExit): + validate_wheel(wheel, "0.5.1", {"sitecustomize.py"}, {"sitecustomize.py": ""}, "") + assert "startup hook" in capsys.readouterr().err + + +def test_sdist_validator_rejects_traversal_and_startup_hooks( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + sdist = tmp_path / "base_cli-0.5.1.tar.gz" + with tarfile.open(sdist, "w:gz") as archive: + for name in ("base-cli-0.5.1/../escape.txt", "base-cli-0.5.1/usercustomize.py"): + payload = b"unsafe" + info = tarfile.TarInfo(name) + info.size = len(payload) + archive.addfile(info, __import__("io").BytesIO(payload)) + + with pytest.raises(SystemExit): + validate_sdist(sdist, "0.5.1", {}, "") + assert "unsafe archive member path" in capsys.readouterr().err