diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 7e564de16c..76fd485dc1 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -706,6 +706,18 @@ jobs: --base-sha "$PR_BASE_SHA" \ --head-sha "$PR_HEAD_SHA" \ --output-dir "$coverage_build_dir/base-javascript-packages" + # Vendors the base commit's Cargo dependency closure so `cargo llvm-cov` and any + # PyO3/maturin extension a Python test suite imports can build offline inside the + # `--network=none` sandbox below. Confirmed live on fast-mlsirm PRs #1868-#1892: with + # no vendored crates, `cargo llvm-cov` failed on `index.crates.io` DNS resolution and + # the generic Python coverage path failed at collection with `ImportError: cannot + # import name '_core'`, both surfacing as an indistinguishable "Coverage gate: failure" + # even when the pull request itself introduced no regression. + python3 -I "$GITHUB_WORKSPACE/scripts/ci/materialize_base_rust_dependencies.py" \ + --repo-root "$COVERAGE_SOURCE_WORKDIR" \ + --base-sha "$PR_BASE_SHA" \ + --output-dir "$coverage_build_dir/base-rust-dependencies" \ + --vendor-dir-for-config /opt/base-rust-dependencies/vendor cat >"$coverage_build_dir/Dockerfile" <<'DOCKERFILE' FROM docker.io/library/python:3.14-slim@sha256:b877e50bd90de10af8d82c57a022fc2e0dc731c5320d762a27986facfc3355c1 ENV DEBIAN_FRONTEND=noninteractive @@ -886,6 +898,7 @@ jobs: --requirements-root /tmp/base-python-requirements \ && rm -rf /tmp/base-python-requirements \ && rm -f /usr/local/libexec/install-base-python-locks.py + COPY base-rust-dependencies /opt/base-rust-dependencies DOCKERFILE if ! docker build --pull --no-cache --network=default \ --tag "$coverage_tool_image" \ @@ -970,6 +983,17 @@ jobs: fi mkdir -p "$RUNNER_TEMP" /work/.opencode-sandbox-home /work/.opencode-sandbox-cache chown "$OPENCODE_SANDBOX_UID:$OPENCODE_SANDBOX_GID" /work/.opencode-sandbox-home /work/.opencode-sandbox-cache + # `run_and_capture`/`run_and_capture_advisory` below pin CARGO_HOME to + # /work/.opencode-sandbox-home/.cargo, which lives on the mutable /work bind mount, not + # the read-only image -- so the baked offline vendor config from + # /opt/base-rust-dependencies (see materialize_base_rust_dependencies.py) has to be + # copied there explicitly rather than set as an image ENV default. + if [ -s /opt/base-rust-dependencies/cargo-config.toml ]; then + mkdir -p /work/.opencode-sandbox-home/.cargo + install -m 0444 /opt/base-rust-dependencies/cargo-config.toml \ + /work/.opencode-sandbox-home/.cargo/config.toml + chown -R "$OPENCODE_SANDBOX_UID:$OPENCODE_SANDBOX_GID" /work/.opencode-sandbox-home/.cargo + fi chmod 0700 "$RUNNER_TEMP" : >"$GITHUB_OUTPUT" chmod 0600 "$GITHUB_OUTPUT" diff --git a/scripts/ci/materialize_base_rust_dependencies.py b/scripts/ci/materialize_base_rust_dependencies.py new file mode 100644 index 0000000000..6feec9cedf --- /dev/null +++ b/scripts/ci/materialize_base_rust_dependencies.py @@ -0,0 +1,309 @@ +#!/usr/bin/env python3 +"""Materialize an offline Cargo vendor directory from a validated base commit. + +The sandboxed coverage-measurement container runs with ``--network=none`` (see +``opencode-review-dispatch.yml``'s "Measure test and docstring evidence" step). Python and +JavaScript dependencies already have an offline path through +``materialize_base_python_requirements.py`` and ``materialize_base_javascript_packages.py``, which +run here -- on the runner, before the network-isolated container exists -- and bake a base-pinned +dependency closure into the trusted image. Rust/Cargo had no equivalent: every coverage run against +a Rust crate (directly via ``cargo llvm-cov``, or indirectly through a PyO3/maturin extension a +Python test suite imports) needed ``index.crates.io``, which the offline container can never reach. +Confirmed live across ``fast-mlsirm`` PRs #1868-#1892 (dispatch runs 34884397167 and siblings): +``cargo llvm-cov`` failed with ``Could not resolve host: index.crates.io``, and the generic Python +pytest path failed at collection with ``ImportError: cannot import name '_core'`` because nothing in +the sandbox ever builds the compiled extension. Both surfaced as a generic "Coverage gate: failure", +indistinguishable from a real regression in the pull request. + +This mirrors the Python materializer's trust model: only the validated base commit's Cargo +manifests are read (never the pull request's), and vendoring itself uses Cargo's own built-in +per-package checksum verification (every ``[[package]]`` entry in a lock file carries a +``checksum``), so no separate hash-pin parser is needed the way ``requirements*.txt`` needed one. +""" + +from __future__ import annotations + +import argparse +import json +import pathlib +import re +import subprocess +import sys +import tempfile + +try: + import tomllib +except ModuleNotFoundError: # pragma: no cover - exercised by Python 3.10 CI. + import tomli as tomllib + + +SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") +CARGO_VENDOR_TIMEOUT_SECONDS = 600 + + +def _git(repo_root: pathlib.Path, *args: str) -> bytes: + """Run one read-only git command against the materialized repository.""" + completed = subprocess.run( + ["git", "-C", str(repo_root), *args], + check=False, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + if completed.returncode != 0: + stderr = completed.stderr.decode("utf-8", errors="replace").strip() + raise RuntimeError(f"git {args[0]} failed: {stderr}") + return completed.stdout + + +def _regular_cargo_blob_paths(repo_root: pathlib.Path, base_sha: str) -> list[str]: + """Return tracked, non-symlink ``Cargo.toml``/``Cargo.lock`` paths at ``base_sha``.""" + entries = _git(repo_root, "ls-tree", "-r", "-z", "--full-tree", base_sha) + paths: list[str] = [] + for raw_entry in entries.split(b"\0"): + if not raw_entry: + continue + metadata, separator, raw_path = raw_entry.partition(b"\t") + if not separator: + raise RuntimeError("git ls-tree returned a malformed entry") + fields = metadata.split() + if len(fields) != 3: + raise RuntimeError("git ls-tree returned malformed metadata") + mode, object_type, _object_id = ( + field.decode("ascii", errors="strict") for field in fields + ) + path = raw_path.decode("utf-8", errors="surrogateescape") + candidate = pathlib.PurePosixPath(path) + if ( + object_type != "blob" + or not mode.startswith("100") + or candidate.is_absolute() + or ".." in candidate.parts + ): + continue + if candidate.name in ("Cargo.toml", "Cargo.lock"): + paths.append(path) + return sorted(paths) + + +def _is_workspace_manifest(content: bytes) -> bool: + """Return whether one ``Cargo.toml`` blob declares a ``[workspace]`` table.""" + try: + parsed = tomllib.loads(content.decode("utf-8")) + except (UnicodeDecodeError, tomllib.TOMLDecodeError) as exc: + raise RuntimeError("could not parse a tracked base Cargo.toml") from exc + return "workspace" in parsed + + +def _select_vendor_root( + repo_root: pathlib.Path, base_sha: str, cargo_paths: list[str] +) -> str | None: + """Return the single directory ``cargo vendor`` should be invoked from, or ``None``. + + Only one topology is supported: a single Cargo workspace root, or a single standalone + crate with no workspace. Any other shape (independent multi-root layouts) fails closed + rather than guess which root's lock file is authoritative -- the same restraint + ``materialize_base_python_requirements.py`` takes with uv workspaces. + """ + manifests = [path for path in cargo_paths if path.endswith("Cargo.toml")] + locks = {path.rsplit("/", 1)[0] if "/" in path else "." for path in cargo_paths if path.endswith("Cargo.lock")} + workspace_dirs: list[str] = [] + for manifest_path in manifests: + content = _git(repo_root, "show", f"{base_sha}:{manifest_path}") + if _is_workspace_manifest(content): + manifest_dir = manifest_path.rsplit("/", 1)[0] if "/" in manifest_path else "." + workspace_dirs.append(manifest_dir) + + if len(workspace_dirs) == 1: + (root,) = workspace_dirs + if root in locks: + return root + raise RuntimeError( + f"base Cargo workspace root {root} has no sibling Cargo.lock" + ) + if len(workspace_dirs) > 1: + raise RuntimeError( + "base tree declares more than one Cargo workspace root; " + "Rust dependency vendoring needs exactly one" + ) + if len(locks) == 1: + (root,) = locks + manifest_path = "Cargo.toml" if root == "." else f"{root}/Cargo.toml" + if manifest_path in manifests: + return root + raise RuntimeError(f"base Cargo.lock at {root} has no sibling Cargo.toml") + if len(locks) > 1: + raise RuntimeError( + "base tree has more than one Cargo.lock with no single workspace root; " + "Rust dependency vendoring needs exactly one" + ) + return None + + +def _placeholder_target_paths(manifest_content: bytes) -> list[str]: + """Return package target source paths a manifest needs present to parse. + + ``cargo vendor`` never compiles anything -- it only resolves and downloads the locked + dependency graph -- but Cargo still refuses to *parse* a package manifest whose declared + targets do not exist on disk. Real source is never required for vendoring, so this returns + the conventional and any explicitly declared target paths; the caller writes empty + placeholder files at each one. + """ + try: + parsed = tomllib.loads(manifest_content.decode("utf-8")) + except (UnicodeDecodeError, tomllib.TOMLDecodeError): + return [] + if "package" not in parsed: + return [] + paths = {"src/lib.rs", "src/main.rs"} + lib_path = parsed.get("lib", {}).get("path") if isinstance(parsed.get("lib"), dict) else None + if isinstance(lib_path, str): + paths.add(lib_path) + for bin_target in parsed.get("bin", []) if isinstance(parsed.get("bin"), list) else []: + bin_path = bin_target.get("path") if isinstance(bin_target, dict) else None + if isinstance(bin_path, str): + paths.add(bin_path) + return sorted(paths) + + +def _reconstruct_base_tree( + repo_root: pathlib.Path, base_sha: str, cargo_paths: list[str], work_dir: pathlib.Path +) -> None: + """Write every tracked base Cargo manifest into ``work_dir`` at its repository path. + + Each package manifest's conventional/declared target paths also get an empty placeholder + file -- see :func:`_placeholder_target_paths` for why real source is never needed here. + """ + for path in cargo_paths: + content = _git(repo_root, "show", f"{base_sha}:{path}") + destination = work_dir / pathlib.Path(*pathlib.PurePosixPath(path).parts) + destination.parent.mkdir(parents=True, exist_ok=True) + destination.write_bytes(content) + if destination.name == "Cargo.toml": + for target_path in _placeholder_target_paths(content): + target_destination = destination.parent / pathlib.Path( + *pathlib.PurePosixPath(target_path).parts + ) + target_destination.parent.mkdir(parents=True, exist_ok=True) + if not target_destination.exists(): + target_destination.write_bytes(b"") + + +def _run_cargo_vendor( + manifest_path: pathlib.Path, vendor_dir: pathlib.Path +) -> subprocess.CompletedProcess[bytes]: + """Run ``cargo vendor`` for one reconstructed base manifest and return the result.""" + return subprocess.run( + [ + "cargo", + "vendor", + "--manifest-path", + str(manifest_path), + "--versioned-dirs", + str(vendor_dir), + ], + check=False, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=CARGO_VENDOR_TIMEOUT_SECONDS, + ) + + +def materialize( + repo_root: pathlib.Path, + base_sha: str, + output_dir: pathlib.Path, + *, + vendor_dir_for_config: str | None = None, +) -> list[str]: + """Vendor the base commit's Cargo dependency closure into ``output_dir``. + + Returns the list of source-tree-relative ``Cargo.lock`` paths that were vendored. An empty + list means no Rust project (or no lock file) exists at the base commit, which is not an + error -- most repositories reviewed by this pipeline have no Rust code at all. + + ``vendor_dir_for_config`` overrides the ``directory = `` path written into + ``cargo-config.toml``. Vendoring runs on the runner (this materializer's own working + directory), but the vendored files are later copied into the trusted coverage image at a + fixed path; the emitted config must name that final in-image path, not the runner's + temporary one. + """ + if not SHA_RE.fullmatch(base_sha): + raise ValueError("base SHA must be exactly 40 hexadecimal characters") + if output_dir.exists() and output_dir.is_symlink(): + raise ValueError("output directory must not be a symlink") + output_dir.mkdir(parents=True, exist_ok=True) + + resolved_repo = repo_root.resolve() + cargo_paths = _regular_cargo_blob_paths(resolved_repo, base_sha) + vendor_root = _select_vendor_root(resolved_repo, base_sha, cargo_paths) + manifest: list[str] = [] + if vendor_root is not None: + with tempfile.TemporaryDirectory() as work_dir: + work_path = pathlib.Path(work_dir) + _reconstruct_base_tree(resolved_repo, base_sha, cargo_paths, work_path) + manifest_path = work_path / ( + "Cargo.toml" if vendor_root == "." else f"{vendor_root}/Cargo.toml" + ) + lock_path = "Cargo.lock" if vendor_root == "." else f"{vendor_root}/Cargo.lock" + vendor_dir = output_dir / "vendor" + try: + completed = _run_cargo_vendor(manifest_path, vendor_dir) + except (OSError, subprocess.TimeoutExpired) as exc: + raise RuntimeError( + f"could not run trusted cargo vendor for base manifest {lock_path}: " + f"{type(exc).__name__}" + ) from exc + if completed.returncode != 0: + stderr = completed.stderr.decode("utf-8", errors="replace") + normalized_stderr = " ".join(stderr.split()) + detail = normalized_stderr[:500] if normalized_stderr else ( + f"exit status {completed.returncode}" + ) + raise RuntimeError(f"cargo vendor failed for base lock {lock_path}: {detail}") + config_text = completed.stdout + if vendor_dir_for_config is not None: + config_text = config_text.replace( + str(vendor_dir).encode("utf-8"), + vendor_dir_for_config.encode("utf-8"), + ) + (output_dir / "cargo-config.toml").write_bytes(config_text) + manifest = [lock_path] + + (output_dir / "manifest.json").write_text( + json.dumps(manifest, indent=2, sort_keys=True) + "\n", + encoding="utf-8", + ) + return manifest + + +def main(argv: list[str] | None = None) -> int: + """Materialize the base Cargo vendor directory and report what was selected.""" + parser = argparse.ArgumentParser() + parser.add_argument("--repo-root", required=True, type=pathlib.Path) + parser.add_argument("--base-sha", required=True) + parser.add_argument("--output-dir", required=True, type=pathlib.Path) + parser.add_argument("--vendor-dir-for-config", default=None) + args = parser.parse_args(argv) + + try: + manifest = materialize( + args.repo_root, + args.base_sha, + args.output_dir, + vendor_dir_for_config=args.vendor_dir_for_config, + ) + except (OSError, RuntimeError, ValueError) as exc: + print( + f"::error::Could not materialize base Rust dependencies: {exc}", file=sys.stderr + ) + return 1 + + if manifest: + print(f"Materialized trusted base Cargo vendor directory from {manifest[0]}.") + else: + print("No tracked Cargo.lock exists at the validated base SHA; Rust vendoring skipped.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_materialize_base_rust_dependencies.py b/tests/test_materialize_base_rust_dependencies.py new file mode 100644 index 0000000000..a44c1e7e06 --- /dev/null +++ b/tests/test_materialize_base_rust_dependencies.py @@ -0,0 +1,412 @@ +from __future__ import annotations + +import json +import runpy +import shutil +import subprocess +from pathlib import Path + +import pytest + +from scripts.ci import materialize_base_rust_dependencies as materializer + +pytestmark = pytest.mark.skipif( + shutil.which("cargo") is None, reason="cargo is required to vendor a real dependency graph" +) + + +def git(repo: Path, *args: str) -> str: + """Run git in a temporary fixture repository.""" + return subprocess.run( + ["git", "-C", str(repo), *args], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + +def _init_repo(repo: Path) -> None: + repo.mkdir(parents=True, exist_ok=True) + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + + +def _commit_all(repo: Path) -> str: + git(repo, "add", "-A") + git(repo, "commit", "-m", "materialize fixture") + return git(repo, "rev-parse", "HEAD") + + +def _write_single_crate_workspace(repo: Path) -> None: + (repo / "Cargo.toml").write_text( + '[workspace]\nmembers = ["crates/foo"]\nresolver = "2"\n', encoding="utf-8" + ) + crate_dir = repo / "crates" / "foo" + crate_dir.mkdir(parents=True) + (crate_dir / "Cargo.toml").write_text( + '[package]\nname = "foo"\nversion = "0.1.0"\nedition = "2021"\n\n' + '[dependencies]\nitoa = "1"\n', + encoding="utf-8", + ) + src_dir = crate_dir / "src" + src_dir.mkdir() + (src_dir / "lib.rs").write_text("pub fn x() {}\n", encoding="utf-8") + subprocess.run( + ["cargo", "generate-lockfile"], cwd=repo, check=True, capture_output=True + ) + + +def test_no_tracked_cargo_lock_skips_gracefully(tmp_path: Path) -> None: + """Repositories with no Rust code produce an empty manifest, not an error.""" + repo = tmp_path / "repo" + _init_repo(repo) + (repo / "README.md").write_text("hi\n", encoding="utf-8") + base_sha = _commit_all(repo) + + output_dir = tmp_path / "out" + manifest = materializer.materialize(repo, base_sha, output_dir) + + assert manifest == [] + assert json.loads((output_dir / "manifest.json").read_text()) == [] + assert not (output_dir / "vendor").exists() + + +def test_vendors_a_single_workspace_offline_afterward(tmp_path: Path) -> None: + """A workspace's locked dependency closure vendors, and cargo then builds offline from it.""" + repo = tmp_path / "repo" + _init_repo(repo) + _write_single_crate_workspace(repo) + base_sha = _commit_all(repo) + + output_dir = tmp_path / "out" + manifest = materializer.materialize( + repo, base_sha, output_dir, vendor_dir_for_config=str(output_dir / "vendor") + ) + + assert manifest == ["Cargo.lock"] + vendored_crates = {p.name.rsplit("-", 1)[0] for p in (output_dir / "vendor").iterdir()} + assert "itoa" in vendored_crates + config_text = (output_dir / "cargo-config.toml").read_text() + assert str(output_dir / "vendor") in config_text + + cargo_home = tmp_path / "cargo-home" + cargo_home.mkdir() + (cargo_home / "config.toml").write_text(config_text, encoding="utf-8") + build = subprocess.run( + ["cargo", "build", "--offline"], + cwd=repo, + env={**__import__("os").environ, "CARGO_HOME": str(cargo_home), "CARGO_NET_OFFLINE": "true"}, + capture_output=True, + text=True, + ) + assert build.returncode == 0, build.stderr + + +def test_pr_added_dependency_not_in_base_lock_is_not_materialized(tmp_path: Path) -> None: + """Vendoring reads only the validated base commit, never a later PR-controlled lock.""" + repo = tmp_path / "repo" + _init_repo(repo) + _write_single_crate_workspace(repo) + base_sha = _commit_all(repo) + + crate_toml = repo / "crates" / "foo" / "Cargo.toml" + crate_toml.write_text( + crate_toml.read_text().replace('itoa = "1"', 'itoa = "1"\nryu = "1"'), encoding="utf-8" + ) + subprocess.run(["cargo", "generate-lockfile"], cwd=repo, check=True, capture_output=True) + _commit_all(repo) + + output_dir = tmp_path / "out" + manifest = materializer.materialize(repo, base_sha, output_dir) + + assert manifest == ["Cargo.lock"] + vendored_crates = {p.name.rsplit("-", 1)[0] for p in (output_dir / "vendor").iterdir()} + assert "ryu" not in vendored_crates + + +def test_multiple_workspace_roots_fail_closed(tmp_path: Path) -> None: + """An ambiguous multi-root layout refuses to guess which lock is authoritative.""" + repo = tmp_path / "repo" + _init_repo(repo) + for name in ("a", "b"): + crate_dir = repo / name + (crate_dir).mkdir() + (crate_dir / "Cargo.toml").write_text( + f'[workspace]\nmembers = ["{name}-crate"]\n', encoding="utf-8" + ) + (crate_dir / "Cargo.lock").write_text("# empty lock\n", encoding="utf-8") + base_sha = _commit_all(repo) + + with pytest.raises(RuntimeError, match="more than one Cargo workspace root"): + materializer.materialize(repo, base_sha, tmp_path / "out") + + +def test_main_reports_error_and_exits_nonzero_on_failure( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """The CLI surfaces a materialization failure as ``::error::`` and exit code 1.""" + repo = tmp_path / "not-a-git-repo" + repo.mkdir() + + exit_code = materializer.main( + [ + "--repo-root", + str(repo), + "--base-sha", + "a" * 40, + "--output-dir", + str(tmp_path / "out"), + ] + ) + + assert exit_code == 1 + assert "::error::Could not materialize base Rust dependencies" in capsys.readouterr().err + + +def test_main_reports_success_with_no_rust_project( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """The CLI reports a clean skip for a repository with no Rust code.""" + repo = tmp_path / "repo" + _init_repo(repo) + (repo / "README.md").write_text("hi\n", encoding="utf-8") + base_sha = _commit_all(repo) + + exit_code = materializer.main( + [ + "--repo-root", + str(repo), + "--base-sha", + base_sha, + "--output-dir", + str(tmp_path / "out"), + ] + ) + + assert exit_code == 0 + assert "Rust vendoring skipped" in capsys.readouterr().out + + +def test_main_reports_success_with_a_vendored_workspace( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """The CLI names the vendored base lock file on a successful run.""" + repo = tmp_path / "repo" + _init_repo(repo) + _write_single_crate_workspace(repo) + base_sha = _commit_all(repo) + + exit_code = materializer.main( + [ + "--repo-root", + str(repo), + "--base-sha", + base_sha, + "--output-dir", + str(tmp_path / "out"), + ] + ) + + assert exit_code == 0 + assert "Materialized trusted base Cargo vendor directory from Cargo.lock." in ( + capsys.readouterr().out + ) + + +def test_module_entry_point_runs_main(monkeypatch: pytest.MonkeyPatch) -> None: + """``python -m`` execution reaches ``main`` and propagates its exit code.""" + monkeypatch.setattr("sys.argv", ["materialize_base_rust_dependencies.py"]) + with pytest.raises(SystemExit) as excinfo: + runpy.run_path( + str(Path(materializer.__file__)), run_name="__main__" + ) + assert excinfo.value.code == 2 # argparse: missing required arguments + + +def test_malformed_ls_tree_entry_without_tab_raises(monkeypatch: pytest.MonkeyPatch) -> None: + """A git ls-tree entry with no ```` separator is a git-format integrity failure.""" + monkeypatch.setattr(materializer, "_git", lambda *_a, **_k: b"bogus-entry-with-no-tab") + with pytest.raises(RuntimeError, match="malformed entry"): + materializer._regular_cargo_blob_paths(Path("/unused"), "a" * 40) + + +def test_malformed_ls_tree_metadata_raises(monkeypatch: pytest.MonkeyPatch) -> None: + """A git ls-tree entry with the wrong metadata field count is rejected.""" + monkeypatch.setattr(materializer, "_git", lambda *_a, **_k: b"100644 blob\tCargo.toml") + with pytest.raises(RuntimeError, match="malformed metadata"): + materializer._regular_cargo_blob_paths(Path("/unused"), "a" * 40) + + +def test_symlinked_cargo_toml_is_excluded(tmp_path: Path) -> None: + """A tracked symlink named ``Cargo.toml`` is never treated as a candidate manifest.""" + repo = tmp_path / "repo" + _init_repo(repo) + _write_single_crate_workspace(repo) + (repo / "linked-crate").symlink_to("crates/foo") + base_sha = _commit_all(repo) + + paths = materializer._regular_cargo_blob_paths(repo, base_sha) + + assert "linked-crate/Cargo.toml" not in paths + assert "Cargo.toml" in paths + + +def test_is_workspace_manifest_rejects_invalid_toml() -> None: + """An unparseable base ``Cargo.toml`` fails closed instead of being treated as non-workspace.""" + with pytest.raises(RuntimeError, match="could not parse"): + materializer._is_workspace_manifest(b"not = [valid toml") + + +def test_select_vendor_root_workspace_without_sibling_lock_raises( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A workspace root manifest with no ``Cargo.lock`` next to it fails closed.""" + monkeypatch.setattr( + materializer, "_git", lambda *_a, **_k: b'[workspace]\nmembers = ["crates/foo"]\n' + ) + with pytest.raises(RuntimeError, match="no sibling Cargo.lock"): + materializer._select_vendor_root(Path("/unused"), "a" * 40, ["Cargo.toml"]) + + +def test_select_vendor_root_returns_single_standalone_crate( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A single crate with no ``[workspace]`` table is its own vendor root.""" + monkeypatch.setattr(materializer, "_git", lambda *_a, **_k: b'[package]\nname = "foo"\n') + root = materializer._select_vendor_root( + Path("/unused"), "a" * 40, ["crate-a/Cargo.toml", "crate-a/Cargo.lock"] + ) + assert root == "crate-a" + + +def test_select_vendor_root_single_lock_without_manifest_raises( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A standalone ``Cargo.lock`` with no sibling ``Cargo.toml`` fails closed.""" + monkeypatch.setattr(materializer, "_git", lambda *_a, **_k: b"") + with pytest.raises(RuntimeError, match="no sibling Cargo.toml"): + materializer._select_vendor_root(Path("/unused"), "a" * 40, ["crate-a/Cargo.lock"]) + + +def test_select_vendor_root_multiple_locks_without_workspace_raises( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Two independent standalone crates with no shared workspace root fail closed.""" + monkeypatch.setattr(materializer, "_git", lambda *_a, **_k: b'[package]\nname = "x"\n') + with pytest.raises(RuntimeError, match="more than one Cargo.lock"): + materializer._select_vendor_root( + Path("/unused"), + "a" * 40, + ["crate-a/Cargo.toml", "crate-a/Cargo.lock", "crate-b/Cargo.toml", "crate-b/Cargo.lock"], + ) + + +def test_placeholder_target_paths_covers_explicit_lib_and_bin_entries() -> None: + """Explicitly declared ``[lib]``/``[[bin]]`` paths are added alongside the conventions.""" + manifest = ( + b'[package]\nname = "foo"\nversion = "0.1.0"\n\n' + b'[lib]\npath = "src/custom_lib.rs"\n\n' + b'[[bin]]\nname = "cli"\npath = "src/bin/cli.rs"\n' + b'[[bin]]\nname = "nameless"\n' + ) + paths = materializer._placeholder_target_paths(manifest) + assert paths == sorted( + {"src/lib.rs", "src/main.rs", "src/custom_lib.rs", "src/bin/cli.rs"} + ) + + +def test_placeholder_target_paths_returns_empty_for_invalid_or_workspace_only_toml() -> None: + """Invalid TOML and manifests with no ``[package]`` table need no placeholder targets.""" + assert materializer._placeholder_target_paths(b"not = [valid") == [] + assert materializer._placeholder_target_paths(b'[workspace]\nmembers = ["a"]\n') == [] + + +def test_reconstruct_base_tree_does_not_overwrite_an_existing_placeholder( + tmp_path: Path, +) -> None: + """Running placeholder synthesis twice for the same manifest is a no-op the second time.""" + repo = tmp_path / "repo" + _init_repo(repo) + _write_single_crate_workspace(repo) + base_sha = _commit_all(repo) + cargo_paths = materializer._regular_cargo_blob_paths(repo, base_sha) + + work_dir = tmp_path / "work" + materializer._reconstruct_base_tree(repo, base_sha, cargo_paths, work_dir) + marker = (work_dir / "crates" / "foo" / "src" / "lib.rs").read_text() + (work_dir / "crates" / "foo" / "src" / "lib.rs").write_text("not-overwritten") + materializer._reconstruct_base_tree(repo, base_sha, cargo_paths, work_dir) + + assert (work_dir / "crates" / "foo" / "src" / "lib.rs").read_text() == "not-overwritten" + assert marker == "" + + +def test_run_cargo_vendor_propagates_missing_binary(tmp_path: Path) -> None: + """A missing ``cargo`` executable surfaces as a materialize() ``RuntimeError``.""" + repo = tmp_path / "repo" + _init_repo(repo) + _write_single_crate_workspace(repo) + base_sha = _commit_all(repo) + + with pytest.MonkeyPatch.context() as monkeypatch: + monkeypatch.setattr( + materializer, + "_run_cargo_vendor", + lambda *_a, **_k: (_ for _ in ()).throw(FileNotFoundError("cargo")), + ) + with pytest.raises(RuntimeError, match="could not run trusted cargo vendor"): + materializer.materialize(repo, base_sha, tmp_path / "out") + + +def test_materialize_surfaces_cargo_vendor_failure_detail( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A non-zero ``cargo vendor`` exit is reported with its captured stderr detail.""" + repo = tmp_path / "repo" + _init_repo(repo) + _write_single_crate_workspace(repo) + base_sha = _commit_all(repo) + + monkeypatch.setattr( + materializer, + "_run_cargo_vendor", + lambda *_a, **_k: subprocess.CompletedProcess( + args=["cargo", "vendor"], returncode=101, stdout=b"", stderr=b"boom\n" + ), + ) + with pytest.raises(RuntimeError, match="cargo vendor failed for base lock Cargo.lock: boom"): + materializer.materialize(repo, base_sha, tmp_path / "out") + + +def test_materialize_surfaces_cargo_vendor_failure_with_no_stderr( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A non-zero ``cargo vendor`` exit with empty stderr still names the exit status.""" + repo = tmp_path / "repo" + _init_repo(repo) + _write_single_crate_workspace(repo) + base_sha = _commit_all(repo) + + monkeypatch.setattr( + materializer, + "_run_cargo_vendor", + lambda *_a, **_k: subprocess.CompletedProcess( + args=["cargo", "vendor"], returncode=101, stdout=b"", stderr=b"" + ), + ) + with pytest.raises(RuntimeError, match="exit status 101"): + materializer.materialize(repo, base_sha, tmp_path / "out") + + +def test_materialize_rejects_bad_sha_and_symlinked_output_dir(tmp_path: Path) -> None: + """Both input-validation guards fail closed before any git or cargo command runs.""" + with pytest.raises(ValueError, match="40 hexadecimal"): + materializer.materialize(Path("/unused"), "not-a-sha", tmp_path / "out") + + real_dir = tmp_path / "real" + real_dir.mkdir() + linked_output = tmp_path / "linked-out" + linked_output.symlink_to(real_dir) + with pytest.raises(ValueError, match="must not be a symlink"): + materializer.materialize(Path("/unused"), "a" * 40, linked_output) diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 232a903ccf..58dbc7728b 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -17,7 +17,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "7e564de16cf8dc9cd305581ee73be31e4c84c7a5" +REVIEW_DISPATCH_BLOB_SHA = "76fd485dc1095d002381bfd1b4aa746c8df7495c" def _workflow_text(path: Path) -> str: