From 3e24e8b6ad9c92f3cb3b78ee300901ea31f2878d Mon Sep 17 00:00:00 2001 From: Jin Pan Date: Thu, 23 Jul 2026 22:51:52 +0000 Subject: [PATCH 1/2] fix(mi355): harden exact-sha evidence worker Fail closed on fetch races and missing pinned images while preserving immutable, content-addressed evidence. Co-authored-by: Cursor --- ops/mi355/README.md | 20 ++-- ops/mi355/run-command-sandbox.sh | 21 ++++- ops/mi355/run-sandbox.sh | 21 ++++- scripts/evolve/mi355_worker.py | 157 ++++++++++++++++++++++--------- tests/test_evolution.py | 88 +++++++++++++++++ 5 files changed, 255 insertions(+), 52 deletions(-) diff --git a/ops/mi355/README.md b/ops/mi355/README.md index 287a6707..f69db335 100644 --- a/ops/mi355/README.md +++ b/ops/mi355/README.md @@ -13,7 +13,8 @@ Public pull-request events therefore cannot execute code on the GPU host. ``` 3. The worker verifies the current head SHA, the commenter's live repository - permission, and that the PR does not modify its control plane. + permission, the fetched commit SHA, and that the PR does not modify its + control plane. It rechecks the approval after validation before publishing. 4. The trusted `main` copy of `validation/run.py` executes the candidate's `validation/manifest.json` and `validation/probes/` in the sandbox. @@ -22,14 +23,17 @@ A new push invalidates the approval because the SHA no longer matches. ## Isolation `run-sandbox.sh` requires a digest-pinned ROCm image and starts it with no -network, a read-only root filesystem, dropped capabilities, no credentials, -read-only candidate/controller mounts, a bounded PID/tmpfs budget, and one -logical GPU selected through ROCr/HIP visibility. +runtime image pulls, no network, a read-only root filesystem, dropped +capabilities, no credentials, read-only candidate/controller mounts, a bounded +PID/tmpfs budget, and one logical GPU selected through ROCr/HIP visibility. The +container runs as the calling service UID/GID and receives only the +supplementary device groups needed for `/dev/kfd` and the selected render node. The compact evidence bundle contains `manifest.json`, `verdicts.json`, `summary.txt`, health snapshots, hashes, controller SHA, candidate SHA, and -approval identity. Large logs/traces belong in content-addressed storage; the -GitHub check records its URI and SHA-256. +approval identity. Its path includes the full content/provenance SHA-256 and an +existing bundle is never overwritten. Large logs/traces belong in +content-addressed storage; the GitHub check records its URI and SHA-256. ## Install @@ -37,6 +41,10 @@ GitHub check records its URI and SHA-256. container-runtime access it needs. 2. Copy `mi355.env.example` to `/etc/rocm-kernel-wiki/mi355.env`, restrict it to root, and fill in a short-lived GitHub App token plus a digest-pinned image. + The App needs repository `Contents: read`, `Pull requests: read`, and + `Checks: write` permissions; mandatory metadata read access is used to + verify collaborator permission. Installation tokens expire after one hour, + so rotate the environment file atomically before expiry. 3. Copy the service/timer files to `/etc/systemd/system/`. 4. Enable the timer: diff --git a/ops/mi355/run-command-sandbox.sh b/ops/mi355/run-command-sandbox.sh index ab1c3b69..92038104 100755 --- a/ops/mi355/run-command-sandbox.sh +++ b/ops/mi355/run-command-sandbox.sh @@ -27,23 +27,42 @@ else fi mkdir -p "${output}" +caller_uid="$(id -u)" +caller_gid="$(id -g)" device_args=(--device=/dev/kfd) +device_paths=(/dev/kfd) if [[ -n "${ROCM_WIKI_DRI_DEVICE:-}" ]]; then device_args+=("--device=${ROCM_WIKI_DRI_DEVICE}") + device_paths+=("${ROCM_WIKI_DRI_DEVICE}") else device_args+=(--device=/dev/dri) + device_paths+=(/dev/dri) fi +group_args=() +declare -A added_groups=() +for device in "${device_paths[@]}"; do + device_gid="$(stat -c '%g' "${device}")" + if [[ -z "${added_groups[${device_gid}]+x}" ]]; then + group_args+=("--group-add=${device_gid}") + added_groups["${device_gid}"]=1 + fi +done + exec "${runtime}" run --rm \ + --pull=never \ --network=none \ --read-only \ --cap-drop=ALL \ --security-opt=no-new-privileges \ + --user="${caller_uid}:${caller_gid}" \ --pids-limit=1024 \ --ipc=private \ --shm-size=1g \ - --tmpfs=/tmp:rw,nosuid,nodev,size=4g \ + --tmpfs=/tmp:rw,exec,nosuid,nodev,size=4g \ "${device_args[@]}" \ + "${group_args[@]}" \ + -e "HOME=/tmp" \ -e "ROCR_VISIBLE_DEVICES=${gpu}" \ -e "HIP_VISIBLE_DEVICES=${gpu}" \ -e "CUDA_VISIBLE_DEVICES=${gpu}" \ diff --git a/ops/mi355/run-sandbox.sh b/ops/mi355/run-sandbox.sh index da89a07c..873e5ea4 100755 --- a/ops/mi355/run-sandbox.sh +++ b/ops/mi355/run-sandbox.sh @@ -31,23 +31,42 @@ fi mkdir -p "${output}" +caller_uid="$(id -u)" +caller_gid="$(id -g)" device_args=(--device=/dev/kfd) +device_paths=(/dev/kfd) if [[ -n "${ROCM_WIKI_DRI_DEVICE:-}" ]]; then device_args+=("--device=${ROCM_WIKI_DRI_DEVICE}") + device_paths+=("${ROCM_WIKI_DRI_DEVICE}") else device_args+=(--device=/dev/dri) + device_paths+=(/dev/dri) fi +group_args=() +declare -A added_groups=() +for device in "${device_paths[@]}"; do + device_gid="$(stat -c '%g' "${device}")" + if [[ -z "${added_groups[${device_gid}]+x}" ]]; then + group_args+=("--group-add=${device_gid}") + added_groups["${device_gid}"]=1 + fi +done + exec "${runtime}" run --rm \ + --pull=never \ --network=none \ --read-only \ --cap-drop=ALL \ --security-opt=no-new-privileges \ + --user="${caller_uid}:${caller_gid}" \ --pids-limit=1024 \ --ipc=private \ --shm-size=1g \ - --tmpfs=/tmp:rw,nosuid,nodev,size=4g \ + --tmpfs=/tmp:rw,exec,nosuid,nodev,size=4g \ "${device_args[@]}" \ + "${group_args[@]}" \ + -e "HOME=/tmp" \ -e "ROCR_VISIBLE_DEVICES=${gpu}" \ -e "HIP_VISIBLE_DEVICES=${gpu}" \ -e "CUDA_VISIBLE_DEVICES=${gpu}" \ diff --git a/scripts/evolve/mi355_worker.py b/scripts/evolve/mi355_worker.py index d0eeff7b..5e43b1fd 100644 --- a/scripts/evolve/mi355_worker.py +++ b/scripts/evolve/mi355_worker.py @@ -89,6 +89,13 @@ def validate_changed_paths(paths: list[str]) -> None: ) +def require_exact_sha(observed: str, approved: str, *, source: str) -> None: + if observed != approved: + raise ValueError( + f"{source} SHA {observed!r} does not match approved SHA {approved!r}" + ) + + def _run( command: list[str], *, @@ -164,6 +171,32 @@ def _pull(repo: str, pr: int) -> dict[str, Any]: ) +def _authorize_pull( + repo: str, + pr: int, + *, + required_label: str, + expected_head_sha: str | None = None, +) -> tuple[dict[str, Any], str, dict[str, str]]: + pull = _pull(repo, pr) + labels = { + str(label.get("name")) + for label in (pull.get("labels") or []) + if isinstance(label, dict) + } + if required_label not in labels: + raise ValueError(f"PR lacks required label {required_label!r}") + head_sha = str(pull["headRefOid"]) + if expected_head_sha is not None: + require_exact_sha(head_sha, expected_head_sha, source="current PR head") + approval = find_approval( + _comments(repo, pr), + head_sha=head_sha, + permission_lookup=lambda login: _permission(repo, login), + ) + return pull, head_sha, approval + + def _changed_paths(repo: str, pr: int) -> list[str]: output = _run( ["gh", "pr", "diff", str(pr), "--repo", repo, "--name-only"], @@ -193,7 +226,9 @@ def _health_snapshot(command: str | None, gpu: int, destination: Path) -> None: raise RuntimeError("MI355 health check failed") -def _bundle_digest(bundle: Path) -> tuple[str, dict[str, str]]: +def _bundle_digest( + bundle: Path, provenance: dict[str, Any] +) -> tuple[str, dict[str, str]]: hashes = {} aggregate = hashlib.sha256() for path in sorted(bundle.rglob("*")): @@ -203,6 +238,15 @@ def _bundle_digest(bundle: Path) -> tuple[str, dict[str, str]]: digest = hashlib.sha256(path.read_bytes()).hexdigest() hashes[relative] = digest aggregate.update(f"{relative}:{digest}\n".encode("utf-8")) + aggregate.update( + ( + "provenance:" + + json.dumps( + provenance, sort_keys=True, separators=(",", ":"), ensure_ascii=True + ) + + "\n" + ).encode("utf-8") + ) return aggregate.hexdigest(), hashes @@ -216,35 +260,56 @@ def _copy_compact_bundle( approval: dict[str, str], artifact_uri: str | None, ) -> tuple[Path, dict[str, Any]]: - destination = evidence_root / f"pr-{pr}-{head_sha[:12]}" - if destination.exists(): - shutil.rmtree(destination) - destination.mkdir(parents=True) - for name in ("manifest.json", "verdicts.json", "summary.txt"): - source = run_output / name - if not source.is_file(): - raise RuntimeError(f"validation output is missing {name}") - shutil.copy2(source, destination / name) - for name in ("health-before.txt", "health-after.txt"): - source = run_output / name - if source.is_file(): - shutil.copy2(source, destination / name) - digest, hashes = _bundle_digest(destination) - evidence = { + provenance = { "schema_version": 1, - "created_at": utc_now(), "pr": pr, "head_sha": head_sha, "controller_sha": controller_sha, "approval": approval, - "artifact_uri": artifact_uri, - "bundle_sha256": digest, - "files": hashes, } - (destination / "EVIDENCE.yaml").write_text( - yaml.safe_dump(evidence, sort_keys=False, allow_unicode=True), - encoding="utf-8", - ) + with tempfile.TemporaryDirectory( + prefix=".mi355-bundle-", dir=evidence_root + ) as directory: + staging = Path(directory) + for name in ("manifest.json", "verdicts.json", "summary.txt"): + source = run_output / name + if not source.is_file(): + raise RuntimeError(f"validation output is missing {name}") + shutil.copy2(source, staging / name) + for name in ("health-before.txt", "health-after.txt"): + source = run_output / name + if source.is_file(): + shutil.copy2(source, staging / name) + digest, hashes = _bundle_digest(staging, provenance) + destination = ( + evidence_root + / f"pr-{pr}-{head_sha[:12]}-sha256-{digest}" + ) + if destination.exists(): + raise ValueError( + f"immutable evidence bundle already exists: {destination}" + ) + resolved_artifact_uri = ( + artifact_uri.rstrip("/") + "/" + destination.name + if artifact_uri + else None + ) + evidence = { + "schema_version": 1, + "created_at": utc_now(), + "pr": pr, + "head_sha": head_sha, + "controller_sha": controller_sha, + "approval": approval, + "artifact_uri": resolved_artifact_uri, + "bundle_sha256": digest, + "files": hashes, + } + (staging / "EVIDENCE.yaml").write_text( + yaml.safe_dump(evidence, sort_keys=False, allow_unicode=True), + encoding="utf-8", + ) + staging.rename(destination) return destination, evidence @@ -299,19 +364,10 @@ def _publish_check( def process_pr(args: argparse.Namespace, repo: str, pr_number: int) -> dict[str, Any]: - pull = _pull(repo, pr_number) - labels = { - str(label.get("name")) - for label in (pull.get("labels") or []) - if isinstance(label, dict) - } - if args.required_label not in labels: - raise ValueError(f"PR lacks required label {args.required_label!r}") - head_sha = str(pull["headRefOid"]) - approval = find_approval( - _comments(repo, pr_number), - head_sha=head_sha, - permission_lookup=lambda login: _permission(repo, login), + _, head_sha, approval = _authorize_pull( + repo, + pr_number, + required_label=args.required_label, ) changed_paths = _changed_paths(repo, pr_number) validate_changed_paths(changed_paths) @@ -337,12 +393,23 @@ def process_pr(args: argparse.Namespace, repo: str, pr_number: int) -> dict[str, "git", "fetch", "origin", - f"pull/{pr_number}/head:{fetch_ref}", + f"+pull/{pr_number}/head:{fetch_ref}", ], cwd=WIKI_ROOT, ) + fetched_sha = _run( + ["git", "rev-parse", f"{fetch_ref}^{{commit}}"], + cwd=WIKI_ROOT, + ) + require_exact_sha(fetched_sha, head_sha, source="fetched PR head") + _, _, approval = _authorize_pull( + repo, + pr_number, + required_label=args.required_label, + expected_head_sha=head_sha, + ) _run( - ["git", "worktree", "add", "--detach", str(candidate), fetch_ref], + ["git", "worktree", "add", "--detach", str(candidate), head_sha], cwd=WIKI_ROOT, ) try: @@ -376,6 +443,12 @@ def process_pr(args: argparse.Namespace, repo: str, pr_number: int) -> dict[str, args.gpu, run_output / "health-after.txt", ) + _, _, approval = _authorize_pull( + repo, + pr_number, + required_label=args.required_label, + expected_head_sha=head_sha, + ) finally: _run( ["git", "worktree", "remove", "--force", str(candidate)], @@ -394,11 +467,7 @@ def process_pr(args: argparse.Namespace, repo: str, pr_number: int) -> dict[str, head_sha=head_sha, controller_sha=controller_sha, approval=approval, - artifact_uri=( - args.artifact_uri.rstrip("/") + "/" + f"pr-{pr_number}-{head_sha[:12]}" - if args.artifact_uri - else None - ), + artifact_uri=args.artifact_uri, ) check = _publish_check( repo, diff --git a/tests/test_evolution.py b/tests/test_evolution.py index 0aae1d41..f4acd0d3 100644 --- a/tests/test_evolution.py +++ b/tests/test_evolution.py @@ -758,6 +758,94 @@ def test_mi355_worker_rejects_candidate_control_plane_changes(): raise AssertionError(f"MI355 control-plane change accepted: {path}") +def test_mi355_worker_rejects_fetched_sha_race(): + from evolve.mi355_worker import require_exact_sha + + approved = "a" * 40 + require_exact_sha(approved, approved, source="test") + try: + require_exact_sha("b" * 40, approved, source="fetched PR head") + except ValueError as error: + assert "does not match approved SHA" in str(error) + else: + raise AssertionError("worker accepted a fetched SHA that was not approved") + + +def test_mi355_evidence_bundle_is_content_addressed_and_immutable(): + from evolve.mi355_worker import _copy_compact_bundle + + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + run_output = root / "run-output" + evidence_root = root / "evidence" + run_output.mkdir() + evidence_root.mkdir() + for name, content in ( + ("manifest.json", "{}\n"), + ("verdicts.json", '{"overall_status":"pass"}\n'), + ("summary.txt", "overall_status=pass\n"), + ("health-before.txt", "healthy\n"), + ("health-after.txt", "healthy\n"), + ): + (run_output / name).write_text(content, encoding="utf-8") + approval = { + "login": "maintainer", + "permission": "write", + "sha": "a" * 40, + "approved_at": "2026-07-23T00:00:00Z", + } + + bundle, evidence = _copy_compact_bundle( + run_output, + evidence_root, + pr=12, + head_sha="a" * 40, + controller_sha="c" * 40, + approval=approval, + artifact_uri="s3://example/mi355", + ) + assert bundle.name.endswith(f"sha256-{evidence['bundle_sha256']}") + assert evidence["artifact_uri"] == f"s3://example/mi355/{bundle.name}" + original = (bundle / "EVIDENCE.yaml").read_bytes() + + try: + _copy_compact_bundle( + run_output, + evidence_root, + pr=12, + head_sha="a" * 40, + controller_sha="c" * 40, + approval=approval, + artifact_uri="s3://example/mi355", + ) + except ValueError as error: + assert "immutable evidence bundle already exists" in str(error) + else: + raise AssertionError("worker overwrote an immutable evidence bundle") + assert (bundle / "EVIDENCE.yaml").read_bytes() == original + + +def test_mi355_sandboxes_preserve_isolation_and_use_caller_identity(): + for relative in ( + "ops/mi355/run-sandbox.sh", + "ops/mi355/run-command-sandbox.sh", + ): + text = (ROOT / relative).read_text(encoding="utf-8") + for required in ( + "--pull=never", + "--network=none", + "--read-only", + "--cap-drop=ALL", + "--security-opt=no-new-privileges", + "--tmpfs=/tmp:rw,exec,nosuid,nodev,size=4g", + '--user="${caller_uid}:${caller_gid}"', + '"--group-add=${device_gid}"', + ): + assert required in text, f"{relative} lacks {required}" + for forbidden in ("GH_TOKEN", "GITHUB_TOKEN", "SSH_AUTH_SOCK", "docker.sock"): + assert forbidden not in text, f"{relative} exposes {forbidden}" + + def test_scored_retrieval_eval_meets_committed_thresholds(): from evaluate_skill import evaluate_retrieval, load_gold_cases From 53ac33c2d500ef91c771b45d3083d5437037b6a7 Mon Sep 17 00:00:00 2001 From: Jin Pan Date: Thu, 23 Jul 2026 23:24:49 +0000 Subject: [PATCH 2/2] test(mi355): pin exact-sha error diagnostics Co-authored-by: Cursor --- tests/test_evolution.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_evolution.py b/tests/test_evolution.py index f4acd0d3..c6583ecd 100644 --- a/tests/test_evolution.py +++ b/tests/test_evolution.py @@ -766,7 +766,10 @@ def test_mi355_worker_rejects_fetched_sha_race(): try: require_exact_sha("b" * 40, approved, source="fetched PR head") except ValueError as error: - assert "does not match approved SHA" in str(error) + assert str(error) == ( + f"fetched PR head SHA {'b' * 40!r} " + f"does not match approved SHA {approved!r}" + ) else: raise AssertionError("worker accepted a fetched SHA that was not approved")