diff --git a/scripts/checks/extract_scorecard_artifact.py b/scripts/checks/extract_scorecard_artifact.py index 4c8e9f61..cab739db 100644 --- a/scripts/checks/extract_scorecard_artifact.py +++ b/scripts/checks/extract_scorecard_artifact.py @@ -23,7 +23,9 @@ def resolve_artifact_zip(source: Path) -> Path: raise ValueError(f"artifact source does not exist: {source}") ensure_non_symlink_path(source, path_kind="artifact path") candidates: list[Path] = [] - for path in sorted(candidate for candidate in source.iterdir() if candidate.suffix == ".zip"): + for path in sorted( + candidate for candidate in source.iterdir() if candidate.suffix == ".zip" + ): ensure_non_symlink_path(path, path_kind="artifact path") candidates.append(path) if len(candidates) != 1: @@ -40,7 +42,7 @@ def validate_member(member: zipfile.ZipInfo) -> None: if ( member.filename != EXPECTED_MEMBER or member_path.is_absolute() - or ".." in member_path.parts + or ".." in member.filename.replace("\\", "/").split("/") or member.is_dir() or stat.S_ISLNK(unix_mode) ): @@ -63,7 +65,9 @@ def ensure_non_symlink_path(path: Path, *, path_kind: str = "output path") -> No raise ValueError(f"symlinked {path_kind} is not allowed: {component}") -def write_new_file_without_following_symlinks(target: Path, source_file: IO[bytes]) -> None: +def write_new_file_without_following_symlinks( + target: Path, source_file: IO[bytes] +) -> None: """Stream-write to a new file without following an existing symlink.""" flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY if hasattr(os, "O_NOFOLLOW"): diff --git a/scripts/release/extract_release_artifacts.py b/scripts/release/extract_release_artifacts.py index 8b5f5926..7b5355e3 100644 --- a/scripts/release/extract_release_artifacts.py +++ b/scripts/release/extract_release_artifacts.py @@ -45,7 +45,9 @@ def artifact_zip_paths(source: Path) -> list[Path]: raise ValueError(f"artifact source does not exist: {source}") ensure_non_symlink_path(source, path_kind="artifact path") candidates: list[Path] = [] - for path in sorted(candidate for candidate in source.iterdir() if candidate.suffix == ".zip"): + for path in sorted( + candidate for candidate in source.iterdir() if candidate.suffix == ".zip" + ): ensure_non_symlink_path(path, path_kind="artifact path") candidates.append(path) if not candidates: @@ -60,7 +62,7 @@ def validate_member(member: zipfile.ZipInfo) -> None: if ( RELEASE_MEMBER.fullmatch(member.filename) is None or member_path.is_absolute() - or ".." in member_path.parts + or ".." in member.filename.replace("\\", "/").split("/") or member.is_dir() or stat.S_ISLNK(unix_mode) ): @@ -69,7 +71,9 @@ def validate_member(member: zipfile.ZipInfo) -> None: raise ValueError(f"release artifact member too large: {member.filename}") -def write_new_file_without_following_symlinks(target: Path, source_file: IO[bytes]) -> None: +def write_new_file_without_following_symlinks( + target: Path, source_file: IO[bytes] +) -> None: """Stream-write to a new file without following an existing symlink.""" flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY if hasattr(os, "O_NOFOLLOW"): @@ -110,7 +114,9 @@ def extract_release_artifacts(source: Path, output_dir: Path) -> list[Path]: if total_bytes > MAX_TOTAL_RELEASE_ARTIFACT_BYTES: raise ValueError("release artifact bundle too large") if member.filename in seen: - raise ValueError(f"duplicate release artifact member: {member.filename}") + raise ValueError( + f"duplicate release artifact member: {member.filename}" + ) seen.add(member.filename) target = output_dir / member.filename with archive.open(member) as source_file: diff --git a/services/analysis-engine/tests/test_artifact_extractors.py b/services/analysis-engine/tests/test_artifact_extractors.py new file mode 100644 index 00000000..324ec6a1 --- /dev/null +++ b/services/analysis-engine/tests/test_artifact_extractors.py @@ -0,0 +1,132 @@ +"""Tests for repository-owned GitHub artifact ZIP extractors.""" + +from __future__ import annotations + +import zipfile +from pathlib import Path + +import pytest +from conftest import load_module, make_symlink_or_skip + + +def _write_zip(path: Path, members: dict[str, bytes]) -> None: + """Write a ZIP archive with explicit member names and byte payloads.""" + with zipfile.ZipFile(path, "w") as archive: + for member_name, payload in members.items(): + archive.writestr(member_name, payload) + + +def test_scorecard_artifact_extractor_writes_expected_sarif(tmp_path: Path) -> None: + """Extract the single expected Scorecard SARIF member.""" + extractor = load_module( + "scripts/checks/extract_scorecard_artifact.py", + "extract_scorecard_artifact_valid", + ) + archive_path = tmp_path / "scorecard.zip" + _write_zip(archive_path, {"results.sarif": b'{"version":"2.1.0"}'}) + + extracted = extractor.extract_scorecard_artifact(archive_path, tmp_path / "out") + + assert extracted.name == "results.sarif" + assert extracted.read_bytes() == b'{"version":"2.1.0"}' + + +def test_scorecard_artifact_extractor_rejects_backslash_traversal_member( + tmp_path: Path, +) -> None: + """Reject Windows-style traversal names before any artifact is written.""" + extractor = load_module( + "scripts/checks/extract_scorecard_artifact.py", + "extract_scorecard_artifact_backslash_traversal", + ) + archive_path = tmp_path / "scorecard.zip" + output_dir = tmp_path / "out" + _write_zip(archive_path, {"..\\results.sarif": b"{}"}) + + with pytest.raises(ValueError, match="unexpected artifact member"): + extractor.extract_scorecard_artifact(archive_path, output_dir) + + assert not output_dir.exists() + + +def test_scorecard_artifact_extractor_rejects_symlink_output_path( + tmp_path: Path, +) -> None: + """Reject extraction into an existing symlinked output directory.""" + extractor = load_module( + "scripts/checks/extract_scorecard_artifact.py", + "extract_scorecard_artifact_symlink_output", + ) + archive_path = tmp_path / "scorecard.zip" + real_output = tmp_path / "real-output" + symlink_output = tmp_path / "linked-output" + real_output.mkdir() + make_symlink_or_skip(symlink_output, real_output, target_is_directory=True) + _write_zip(archive_path, {"results.sarif": b"{}"}) + + with pytest.raises(ValueError, match="symlinked output path is not allowed"): + extractor.extract_scorecard_artifact(archive_path, symlink_output) + + +def test_release_artifact_extractor_writes_allowlisted_release_files( + tmp_path: Path, +) -> None: + """Extract allowlisted release artifacts with matching sidecar names.""" + extractor = load_module( + "scripts/release/extract_release_artifacts.py", + "extract_release_artifacts_valid", + ) + archive_path = tmp_path / "release.zip" + _write_zip( + archive_path, + { + "bandscope-windows-amd64-abc123def456.exe": b"exe", + "bandscope-windows-amd64-abc123def456.exe.sha256": b"sha", + "bandscope-macos-arm64-abc123def456.dmg.manifest.txt": b"manifest", + }, + ) + + extracted = extractor.extract_release_artifacts(archive_path, tmp_path / "out") + + assert [path.name for path in extracted] == [ + "bandscope-macos-arm64-abc123def456.dmg.manifest.txt", + "bandscope-windows-amd64-abc123def456.exe", + "bandscope-windows-amd64-abc123def456.exe.sha256", + ] + + +def test_release_artifact_extractor_rejects_backslash_traversal_member( + tmp_path: Path, +) -> None: + """Reject Windows-style traversal names in downloaded release artifacts.""" + extractor = load_module( + "scripts/release/extract_release_artifacts.py", + "extract_release_artifacts_backslash_traversal", + ) + archive_path = tmp_path / "release.zip" + output_dir = tmp_path / "out" + _write_zip(archive_path, {"..\\bandscope-windows-amd64-abc123def456.exe": b"exe"}) + + with pytest.raises(ValueError, match="unexpected release artifact member"): + extractor.extract_release_artifacts(archive_path, output_dir) + + assert list(output_dir.iterdir()) == [] + + +def test_release_artifact_extractor_rejects_symlink_output_path( + tmp_path: Path, +) -> None: + """Reject release extraction into an existing symlinked output directory.""" + extractor = load_module( + "scripts/release/extract_release_artifacts.py", + "extract_release_artifacts_symlink_output", + ) + archive_path = tmp_path / "release.zip" + real_output = tmp_path / "real-output" + symlink_output = tmp_path / "linked-output" + real_output.mkdir() + make_symlink_or_skip(symlink_output, real_output, target_is_directory=True) + _write_zip(archive_path, {"bandscope-windows-amd64-abc123def456.exe": b"exe"}) + + with pytest.raises(ValueError, match="symlinked output path is not allowed"): + extractor.extract_release_artifacts(archive_path, symlink_output)