From 28e65b58819061d6eecac1bf6c0d1091cd7a6ec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=97=A0=E4=B8=BA?= <27188611+Haven2026@users.noreply.github.com> Date: Fri, 24 Jul 2026 16:05:47 +0800 Subject: [PATCH 1/3] fix(eval-runner): harden fixture isolation --- skills/bmad-eval-runner/SKILL.md | 4 +- skills/bmad-eval-runner/scripts/run_evals.py | 206 ++++++++++++++++-- .../scripts/tests/test_fixture_integrity.py | 147 +++++++++++++ 3 files changed, 335 insertions(+), 22 deletions(-) create mode 100644 skills/bmad-eval-runner/scripts/tests/test_fixture_integrity.py diff --git a/skills/bmad-eval-runner/SKILL.md b/skills/bmad-eval-runner/SKILL.md index 171b693..98453d3 100644 --- a/skills/bmad-eval-runner/SKILL.md +++ b/skills/bmad-eval-runner/SKILL.md @@ -78,7 +78,7 @@ python3 {skill-root}/scripts/run_triggers.py \ It stages a synthetic skill where the runtime discovers skills, sends each query through the adapter, and detects the skill-load tool call. Each query runs several times for stability. When the user wants to optimize the description rather than just measure it, follow `references/description-optimization.md`. -For quality mode, spawn the grader described in `references/grader.md` per case, passing the case's rubric, transcript path, artifacts dir (the case's `cwd/`), and a `grading_path` of `/grading.json`. The grader writes that file, gives no partial credit, and flags weak or non-discriminating assertions; relay that feedback. If a grader subagent errors, mark that case `grading_error` — never substitute a default verdict. +For quality mode, spawn the grader described in `references/grader.md` per case, passing the case's rubric, transcript path, artifacts dir (the case's `cwd/`), trusted fixture evidence at `/fixture-evidence/`, and a `grading_path` of `/grading.json`. Fixture-grounded claims must use the immutable snapshot and manifest, never the adapter-writable copies in `cwd/`. The grader writes that file, gives no partial credit, and flags weak or non-discriminating assertions; relay that feedback. If a grader subagent errors, mark that case `grading_error` — never substitute a default verdict. When `--runs` is greater than one, call `python3 {skill-root}/scripts/aggregate_benchmark.py --baseline / --variant /` to produce the mean, sample standard deviation, min, max, and the delta between configs (`--runs /` for a single config's spread). @@ -86,7 +86,7 @@ When a run fails or comes back weak and the user wants the skill improved from t ## Artifacts -Every run writes a dated run folder under the output dir, and those artifacts are permanent. Each case folder holds its prompt, transcript, the `cwd/` with any files the skill wrote, `timing.json`, and `grading.json` when quality mode ran. Never delete, overwrite, or rotate a run folder; disk usage is the user's call. The run's `.memlog.md` records the decisions and deltas so a resumed or audited run reads back cleanly. +Every run writes a dated run folder under the output dir, and those artifacts are permanent. Each case folder holds its prompt, transcript, the `cwd/` with any files the skill wrote, immutable `fixture-evidence/` written only after the adapter exits, `timing.json`, and `grading.json` when quality mode ran. Unsafe absolute or parent-traversal fixture paths, source/destination symlink escapes, pre-run hash changes, post-run mutation, and attempts to pre-create the evidence path fail closed. Never delete, overwrite, or rotate a run folder; disk usage is the user's call. The run's `.memlog.md` records the decisions and deltas so a resumed or audited run reads back cleanly. Tell the user where the run folder is when you finish. diff --git a/skills/bmad-eval-runner/scripts/run_evals.py b/skills/bmad-eval-runner/scripts/run_evals.py index c518c93..3cbde3b 100644 --- a/skills/bmad-eval-runner/scripts/run_evals.py +++ b/skills/bmad-eval-runner/scripts/run_evals.py @@ -34,8 +34,10 @@ every config would measure the bare model. Fixtures: each path in a case's `files` list is staged into the case cwd at -its own relative path. Sources resolve against --project-root, then the cases -file's directory, then as absolute paths. +its own relative path. Sources resolve only inside --project-root or the cases +file's directory. Absolute paths, parent traversal, and symlink escapes fail +closed. Trusted bytes and hashes stay in parent-process memory while the +adapter runs, then land in `fixture-evidence/` for grading. Isolation: the subprocess env is built from scratch, never inherited. It holds PATH, a fresh empty HOME at /.home, CLAUDE_CONFIG_DIR inside @@ -84,14 +86,17 @@ from __future__ import annotations import argparse +import hashlib import json import os import shutil +import stat import subprocess import sys import time from collections.abc import Mapping from concurrent.futures import ThreadPoolExecutor, as_completed +from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path @@ -201,35 +206,181 @@ def stage_skill(skill_path: Path, cwd: Path, skills_subdir: str) -> Path: return dest +@dataclass(frozen=True, slots=True) +class Fixture: + source: Path + dest_rel: Path + content: bytes + sha256: str + mode: int + + +@dataclass(frozen=True, slots=True) +class FixtureError(ValueError): + message: str + + def __str__(self) -> str: + return self.message + + +def sha256_bytes(data: bytes) -> str: + return hashlib.sha256(data).hexdigest() + + +def sha256_file(path: Path) -> str: + return sha256_bytes(path.read_bytes()) + + +def _fixture_relative_path(entry: str | Path) -> Path: + rel = Path(str(entry)) + if rel.is_absolute() or ".." in rel.parts: + raise FixtureError( + f"fixture path must be relative without '..': {entry}" + ) + if not rel.parts or rel == Path("."): + raise FixtureError(f"fixture path must name a file: {entry}") + return rel + + +def _contained(path: Path, root: Path, label: str) -> Path: + root = root.resolve() + resolved = path.resolve(strict=True) + try: + resolved.relative_to(root) + except ValueError as exc: + raise FixtureError(f"{label} escapes trusted root: {path}") from exc + return resolved + + def resolve_fixtures(files: list, project_root: Path, - cases_dir: Path) -> list[tuple[Path, str]]: + cases_dir: Path) -> list[Fixture]: """Map each `files` entry to (source, dest-relative-path). The entry's own relative path is preserved inside the cwd, so a bare filename lands at the workspace root and a nested path keeps its directory structure — matching the path the case input references. """ - out: list[tuple[Path, str]] = [] + roots = tuple(dict.fromkeys((project_root.resolve(), cases_dir.resolve()))) + out: list[Fixture] = [] + destinations: set[Path] = set() for entry in files or []: - entry = str(entry) - for candidate in ( - (project_root / entry).resolve(), - (cases_dir / entry).resolve(), - Path(entry).resolve(), - ): - if candidate.is_file(): - out.append((candidate, entry)) + rel = _fixture_relative_path(entry) + if rel in destinations: + raise FixtureError(f"duplicate fixture destination: {rel}") + for root in roots: + candidate = root / rel + if candidate.exists() or candidate.is_symlink(): + source = _contained(candidate, root, "fixture source") + if not source.is_file(): + raise FixtureError( + f"fixture source is not a file: {candidate}" + ) + content = source.read_bytes() + out.append(Fixture( + source=source, + dest_rel=rel, + content=content, + sha256=sha256_bytes(content), + mode=stat.S_IMODE(source.stat().st_mode), + )) + destinations.add(rel) break else: - print(f"Warning: fixture not found: {entry}", file=sys.stderr) + raise FixtureError(f"fixture not found in trusted roots: {entry}") return out -def stage_fixtures(fixtures: list[tuple[Path, str]], cwd: Path) -> None: - for src, dest_rel in fixtures: - dest = cwd / dest_rel +def _fixture_destination(cwd: Path, dest_rel: Path) -> Path: + cwd = cwd.resolve() + dest = cwd / dest_rel + try: + dest.resolve(strict=False).relative_to(cwd) + except ValueError as exc: + raise FixtureError( + f"fixture destination escapes case cwd: {dest_rel}" + ) from exc + current = cwd + for part in dest_rel.parts[:-1]: + current /= part + if current.is_symlink(): + raise FixtureError( + f"fixture destination traverses symlink: {dest_rel}" + ) + if dest.is_symlink(): + raise FixtureError(f"fixture destination is a symlink: {dest_rel}") + return dest + + +def stage_fixtures(fixtures: list[Fixture], cwd: Path) -> None: + for fixture in fixtures: + if sha256_file(fixture.source) != fixture.sha256: + raise FixtureError( + f"fixture source changed before staging: {fixture.dest_rel}" + ) + dest = _fixture_destination(cwd, fixture.dest_rel) + dest.parent.mkdir(parents=True, exist_ok=True) + dest.write_bytes(fixture.content) + dest.chmod(fixture.mode) + if sha256_file(dest) != fixture.sha256: + raise FixtureError( + f"staged fixture hash mismatch: {fixture.dest_rel}" + ) + + +def verify_staged_fixtures(fixtures: list[Fixture], cwd: Path) -> list[str]: + errors: list[str] = [] + for fixture in fixtures: + try: + dest = _fixture_destination(cwd, fixture.dest_rel) + actual = sha256_file(dest) + except (FileNotFoundError, OSError, RuntimeError, ValueError) as exc: + errors.append(f"{fixture.dest_rel}: {exc}") + continue + if actual != fixture.sha256: + errors.append( + f"{fixture.dest_rel}: expected {fixture.sha256}, got {actual}" + ) + return errors + + +def persist_fixture_evidence(fixtures: list[Fixture], case_dir: Path) -> Path: + """Persist the trusted snapshot only after the adapter has exited. + + Until this point the immutable bytes and hashes exist only in the parent + process, outside the adapter's cwd and argv. Pre-existing evidence paths + indicate an adapter write attempt and fail closed. + """ + evidence_dir = case_dir / "fixture-evidence" + if evidence_dir.exists() or evidence_dir.is_symlink(): + rejected = case_dir / f"fixture-evidence-rejected-{time.time_ns()}" + evidence_dir.rename(rejected) + raise FixtureError("fixture evidence path existed before persistence") + snapshot_dir = evidence_dir / "snapshot" + snapshot_dir.mkdir(parents=True) + manifest_fixtures = [] + for fixture in fixtures: + dest = _fixture_destination(snapshot_dir, fixture.dest_rel) dest.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(src, dest) + dest.write_bytes(fixture.content) + dest.chmod(0o400) + manifest_fixtures.append({ + "path": fixture.dest_rel.as_posix(), + "sha256": fixture.sha256, + "size": len(fixture.content), + }) + manifest_path = evidence_dir / "manifest.json" + write_json(manifest_path, { + "schema": "bmad-fixture-evidence/v1", + "captured_at": utc_now_iso(), + "fixtures": manifest_fixtures, + }) + manifest_path.chmod(0o400) + for directory in sorted( + (p for p in snapshot_dir.rglob("*") if p.is_dir()), reverse=True): + directory.chmod(0o500) + snapshot_dir.chmod(0o500) + evidence_dir.chmod(0o500) + return evidence_dir # --- case composition ------------------------------------------------------- @@ -323,7 +474,7 @@ def account_transcript(transcript_text: str) -> dict: def run_case(case: dict, case_dir: Path, run_dir: Path, adapter: dict | None, timeout: int, config: str, skill_path: Path | None, - fixtures: list[tuple[Path, str]]) -> dict: + fixtures: list[Fixture]) -> dict: case_id = str(case.get("id", "unnamed")) cwd = case_dir / "cwd" cwd.mkdir(parents=True, exist_ok=True) @@ -338,6 +489,7 @@ def run_case(case: dict, case_dir: Path, run_dir: Path, write_json(case_dir / "case.json", case) if adapter is None: + persist_fixture_evidence(fixtures, case_dir) result = { "case_id": case_id, "config": config, @@ -381,6 +533,7 @@ def run_case(case: dict, case_dir: Path, run_dir: Path, except FileNotFoundError as e: # Adapter invocation command is not on PATH: degrade, do not crash. elapsed = time.time() - start + persist_fixture_evidence(fixtures, case_dir) write_json(case_dir / "timing.json", { "case_id": case_id, "config": config, "status": "adapter-missing", "elapsed_s": round(elapsed, 3), "captured_at": utc_now_iso(), @@ -399,6 +552,15 @@ def run_case(case: dict, case_dir: Path, run_dir: Path, error_tail = f"TIMEOUT after {timeout}s" elapsed = time.time() - start + fixture_errors = verify_staged_fixtures(fixtures, cwd) + try: + persist_fixture_evidence(fixtures, case_dir) + except (OSError, RuntimeError, ValueError) as exc: + fixture_errors.append(str(exc)) + if fixture_errors: + status = "fixture-integrity-error" + error_tail = "fixture integrity failure: " + "; ".join(fixture_errors) + transcript_text, source = read_transcript( adapter.get("transcript", {}), captured, cwd ) @@ -602,13 +764,17 @@ def main(argv: list[str] | None = None) -> int: "skipped": sum(1 for r in results if r.get("status") == "skipped"), "failures": sum(1 for r in results if r.get("status") in ("error", "timeout", "exception", - "adapter-missing")), + "adapter-missing", + "fixture-integrity-error")), "run_dir": str(run_dir), "results": results, } write_json(run_dir / "execution-summary.json", summary) print(json.dumps(summary, indent=2)) - return 0 + return 1 if any( + result.get("status") == "fixture-integrity-error" + for result in results + ) else 0 if __name__ == "__main__": diff --git a/skills/bmad-eval-runner/scripts/tests/test_fixture_integrity.py b/skills/bmad-eval-runner/scripts/tests/test_fixture_integrity.py new file mode 100644 index 0000000..852071f --- /dev/null +++ b/skills/bmad-eval-runner/scripts/tests/test_fixture_integrity.py @@ -0,0 +1,147 @@ +#!/usr/bin/env python3 +"""Adversarial checks for fixture containment and immutable evidence.""" + +import json +import sys +from pathlib import Path + +import pytest + +SCRIPTS_DIR = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(SCRIPTS_DIR)) + +import run_evals # noqa: E402 + + +def fixture(root: Path, rel: str = "fixture.txt", content: bytes = b"trusted"): + path = root / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(content) + return run_evals.resolve_fixtures([rel], root, root) + + +@pytest.mark.parametrize("entry", ["/etc/passwd", "../outside.txt"]) +def test_rejects_absolute_and_parent_paths(tmp_path, entry): + with pytest.raises(ValueError, match="relative without"): + run_evals.resolve_fixtures([entry], tmp_path, tmp_path) + + +def test_rejects_source_symlink_escape(tmp_path): + trusted = tmp_path / "trusted" + trusted.mkdir() + outside = tmp_path / "outside.txt" + outside.write_text("secret", encoding="utf-8") + (trusted / "escape.txt").symlink_to(outside) + + with pytest.raises(ValueError, match="escapes trusted root"): + run_evals.resolve_fixtures(["escape.txt"], trusted, trusted) + + +def test_rejects_destination_symlink_escape(tmp_path): + source = tmp_path / "source" + fixtures = fixture(source, "nested/fixture.txt") + cwd = tmp_path / "cwd" + cwd.mkdir() + outside = tmp_path / "outside" + outside.mkdir() + (cwd / "nested").symlink_to(outside, target_is_directory=True) + + with pytest.raises(ValueError, match="destination (escapes|traverses)"): + run_evals.stage_fixtures(fixtures, cwd) + + +def test_detects_source_hash_change_before_staging(tmp_path): + source = tmp_path / "source" + fixtures = fixture(source) + (source / "fixture.txt").write_text("changed", encoding="utf-8") + + with pytest.raises(ValueError, match="source changed"): + run_evals.stage_fixtures(fixtures, tmp_path / "cwd") + + +def test_detects_post_run_mutation_and_preserves_snapshot(tmp_path): + source = tmp_path / "source" + fixtures = fixture(source) + run_dir = tmp_path / "run" + case_dir = run_dir / "skill" / "mutation" + adapter = { + "invocation": [ + sys.executable, + "-c", + "from pathlib import Path; Path('fixture.txt').write_text('changed')", + ] + } + + result = run_evals.run_case( + {"id": "mutation", "input": "test"}, case_dir, run_dir, + adapter, 10, "skill", None, fixtures, + ) + + assert result["status"] == "fixture-integrity-error" + evidence = case_dir / "fixture-evidence" + assert (evidence / "snapshot" / "fixture.txt").read_bytes() == b"trusted" + manifest = json.loads((evidence / "manifest.json").read_text()) + assert manifest["fixtures"][0]["sha256"] == fixtures[0].sha256 + + +def test_detects_manifest_write_attempt(tmp_path): + source = tmp_path / "source" + fixtures = fixture(source) + run_dir = tmp_path / "run" + case_dir = run_dir / "skill" / "manifest-write" + adapter = { + "invocation": [ + sys.executable, + "-c", + "from pathlib import Path; p=Path('../fixture-evidence'); " + "p.mkdir(); (p/'manifest.json').write_text('forged')", + ] + } + + result = run_evals.run_case( + {"id": "manifest-write", "input": "test"}, case_dir, run_dir, + adapter, 10, "skill", None, fixtures, + ) + + assert result["status"] == "fixture-integrity-error" + assert "existed before persistence" in result["error_tail"] + assert not (case_dir / "fixture-evidence").exists() + assert len(list(case_dir.glob("fixture-evidence-rejected-*"))) == 1 + + +def test_cli_fails_when_adapter_mutates_fixture(tmp_path): + project = tmp_path / "project" + project.mkdir() + (project / "fixture.txt").write_text("trusted", encoding="utf-8") + cases = project / "cases.json" + cases.write_text(json.dumps({"cases": [{ + "id": "mutation", + "input": "test", + "rubric": [], + "files": ["fixture.txt"], + }]}), encoding="utf-8") + skill = tmp_path / "skill" + skill.mkdir() + (skill / "SKILL.md").write_text("# test", encoding="utf-8") + adapter = tmp_path / "adapter.json" + adapter.write_text(json.dumps({"invocation": [ + sys.executable, + "-c", + "from pathlib import Path; Path('fixture.txt').write_text('changed')", + ]}), encoding="utf-8") + output = tmp_path / "output" + + exit_code = run_evals.main([ + "--cases", str(cases), + "--skill-path", str(skill), + "--project-root", str(project), + "--output-dir", str(output), + "--adapter", str(adapter), + "--workers", "1", + "--quiet", + ]) + + assert exit_code == 1 + summary_path = next(output.glob("*/execution-summary.json")) + summary = json.loads(summary_path.read_text(encoding="utf-8")) + assert summary["results"][0]["status"] == "fixture-integrity-error" From 24eb3cfaa7cc5fadc6879b0e760d5415baefc7eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=97=A0=E4=B8=BA?= <27188611+Haven2026@users.noreply.github.com> Date: Fri, 24 Jul 2026 16:12:37 +0800 Subject: [PATCH 2/3] fix(eval-runner): fail closed on staging errors --- .../references/eval-format.md | 2 +- skills/bmad-eval-runner/scripts/run_evals.py | 14 ++++--- .../scripts/tests/test_fixture_integrity.py | 40 +++++++++++++++++++ 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/skills/bmad-eval-runner/references/eval-format.md b/skills/bmad-eval-runner/references/eval-format.md index 63c9d40..eb7adf5 100644 --- a/skills/bmad-eval-runner/references/eval-format.md +++ b/skills/bmad-eval-runner/references/eval-format.md @@ -24,7 +24,7 @@ Field semantics: - `input`: the realistic, messy user request. Use real file paths, company names, typos, and casual speech, because a polished input tests a situation the skill rarely meets. The runner sends this verbatim to the invocation, after prepending any `state_prefix`. - `rubric`: a list of named expectations, each gradeable to `{text, passed, evidence}` by the grader. The strong-versus-weak taxonomy below decides whether each one is worth keeping. - `state_prefix`: optional bracketed prime that places the skill mid-workflow (see below). Null or absent means the skill starts cold. -- `files`: optional fixture paths staged into the case's clean working directory before the run. A bare filename lands at the workspace root; a nested path keeps its directory structure, so the input can reference it verbatim. Sources resolve against `--project-root`, then the cases file's directory, then as absolute paths. +- `files`: optional fixture paths staged into the case's clean working directory before the run. A bare filename lands at the workspace root; a nested path keeps its directory structure, so the input can reference it verbatim. Sources resolve only inside `--project-root` or the cases file's directory. Paths must be relative and cannot contain `..`; absolute paths and source/destination symlink escapes fail closed. For trigger cases the shape is lighter: a `query` and a `should_trigger` boolean, because there is no artifact to grade, only whether the skill fired. Those cases are covered in `platform-adapter.md` and `description-optimization.md`. diff --git a/skills/bmad-eval-runner/scripts/run_evals.py b/skills/bmad-eval-runner/scripts/run_evals.py index 3cbde3b..40cd7e7 100644 --- a/skills/bmad-eval-runner/scripts/run_evals.py +++ b/skills/bmad-eval-runner/scripts/run_evals.py @@ -206,8 +206,10 @@ def stage_skill(skill_path: Path, cwd: Path, skills_subdir: str) -> Path: return dest -@dataclass(frozen=True, slots=True) +@dataclass(frozen=True) class Fixture: + __slots__ = ("source", "dest_rel", "content", "sha256", "mode") + source: Path dest_rel: Path content: bytes @@ -215,12 +217,8 @@ class Fixture: mode: int -@dataclass(frozen=True, slots=True) class FixtureError(ValueError): - message: str - - def __str__(self) -> str: - return self.message + pass def sha256_bytes(data: bytes) -> str: @@ -746,6 +744,10 @@ def main(argv: list[str] | None = None) -> int: c = fut_to_case[fut] try: res = fut.result() + except FixtureError as e: + res = {"case_id": str(c.get("id")), + "status": "fixture-integrity-error", + "reason": str(e)} except Exception as e: res = {"case_id": str(c.get("id")), "status": "exception", "reason": str(e)} diff --git a/skills/bmad-eval-runner/scripts/tests/test_fixture_integrity.py b/skills/bmad-eval-runner/scripts/tests/test_fixture_integrity.py index 852071f..e11685d 100644 --- a/skills/bmad-eval-runner/scripts/tests/test_fixture_integrity.py +++ b/skills/bmad-eval-runner/scripts/tests/test_fixture_integrity.py @@ -145,3 +145,43 @@ def test_cli_fails_when_adapter_mutates_fixture(tmp_path): summary_path = next(output.glob("*/execution-summary.json")) summary = json.loads(summary_path.read_text(encoding="utf-8")) assert summary["results"][0]["status"] == "fixture-integrity-error" + + +def test_cli_fails_when_source_changes_before_staging(tmp_path, monkeypatch): + project = tmp_path / "project" + project.mkdir() + source = project / "fixture.txt" + source.write_text("trusted", encoding="utf-8") + cases = project / "cases.json" + cases.write_text(json.dumps({"cases": [{ + "id": "source-race", + "input": "test", + "rubric": [], + "files": ["fixture.txt"], + }]}), encoding="utf-8") + skill = tmp_path / "skill" + skill.mkdir() + (skill / "SKILL.md").write_text("# test", encoding="utf-8") + original_resolve = run_evals.resolve_fixtures + + def resolve_then_mutate(files, project_root, cases_dir): + fixtures = original_resolve(files, project_root, cases_dir) + source.write_text("changed", encoding="utf-8") + return fixtures + + monkeypatch.setattr(run_evals, "resolve_fixtures", resolve_then_mutate) + output = tmp_path / "output" + + exit_code = run_evals.main([ + "--cases", str(cases), + "--skill-path", str(skill), + "--project-root", str(project), + "--output-dir", str(output), + "--workers", "1", + "--quiet", + ]) + + assert exit_code == 1 + summary_path = next(output.glob("*/execution-summary.json")) + summary = json.loads(summary_path.read_text(encoding="utf-8")) + assert summary["results"][0]["status"] == "fixture-integrity-error" From 01aa076bfe8b911887a957345defa9e30b48801e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=97=A0=E4=B8=BA?= <27188611+Haven2026@users.noreply.github.com> Date: Fri, 24 Jul 2026 17:41:56 +0800 Subject: [PATCH 3/3] fix(eval-runner): trust parent-held fixture digests Split fixture containment into fixture_integrity, re-validate evidence with an adapter-unreachable digest, exclusive run dirs, and fail-closed CLI exits. chmod is no longer treated as the immutability boundary. --- .../scripts/fixture_integrity.py | 241 ++++++++++++++++++ skills/bmad-eval-runner/scripts/run_evals.py | 225 +++------------- .../scripts/tests/test_fixture_integrity.py | 69 +++++ 3 files changed, 350 insertions(+), 185 deletions(-) create mode 100644 skills/bmad-eval-runner/scripts/fixture_integrity.py diff --git a/skills/bmad-eval-runner/scripts/fixture_integrity.py b/skills/bmad-eval-runner/scripts/fixture_integrity.py new file mode 100644 index 0000000..e8a1779 --- /dev/null +++ b/skills/bmad-eval-runner/scripts/fixture_integrity.py @@ -0,0 +1,241 @@ +"""Contain, stage, and attest eval fixture evidence.""" + +from __future__ import annotations + +import hashlib +import json +import stat +import time +from dataclasses import dataclass +from datetime import datetime, timezone +from pathlib import Path + + +@dataclass(frozen=True) +class Fixture: + __slots__ = ("source", "dest_rel", "content", "sha256", "mode") + + source: Path + dest_rel: Path + content: bytes + sha256: str + mode: int + + +class FixtureError(ValueError): + pass + + +def sha256_bytes(data: bytes) -> str: + return hashlib.sha256(data).hexdigest() + + +def sha256_file(path: Path) -> str: + return sha256_bytes(path.read_bytes()) + + +def _fixture_relative_path(entry: str | Path) -> Path: + rel = Path(str(entry)) + if rel.is_absolute() or ".." in rel.parts: + raise FixtureError( + f"fixture path must be relative without '..': {entry}" + ) + if not rel.parts or rel == Path("."): + raise FixtureError(f"fixture path must name a file: {entry}") + return rel + + +def _contained(path: Path, root: Path, label: str) -> Path: + root = root.resolve() + resolved = path.resolve(strict=True) + try: + resolved.relative_to(root) + except ValueError as exc: + raise FixtureError(f"{label} escapes trusted root: {path}") from exc + return resolved + + +def resolve_fixtures(files: list, project_root: Path, + cases_dir: Path) -> list[Fixture]: + roots = tuple(dict.fromkeys((project_root.resolve(), cases_dir.resolve()))) + fixtures: list[Fixture] = [] + destinations: set[Path] = set() + for entry in files or []: + rel = _fixture_relative_path(entry) + if rel in destinations: + raise FixtureError(f"duplicate fixture destination: {rel}") + for root in roots: + candidate = root / rel + if candidate.exists() or candidate.is_symlink(): + source = _contained(candidate, root, "fixture source") + if not source.is_file(): + raise FixtureError( + f"fixture source is not a file: {candidate}" + ) + content = source.read_bytes() + fixtures.append(Fixture( + source=source, + dest_rel=rel, + content=content, + sha256=sha256_bytes(content), + mode=stat.S_IMODE(source.stat().st_mode), + )) + destinations.add(rel) + break + else: + raise FixtureError(f"fixture not found in trusted roots: {entry}") + return fixtures + + +def _fixture_destination(root: Path, dest_rel: Path) -> Path: + root = root.resolve() + dest = root / dest_rel + try: + dest.resolve(strict=False).relative_to(root) + except ValueError as exc: + raise FixtureError( + f"fixture destination escapes root: {dest_rel}" + ) from exc + current = root + for part in dest_rel.parts[:-1]: + current /= part + if current.is_symlink(): + raise FixtureError( + f"fixture destination traverses symlink: {dest_rel}" + ) + if dest.is_symlink(): + raise FixtureError(f"fixture destination is a symlink: {dest_rel}") + return dest + + +def stage_fixtures(fixtures: list[Fixture], cwd: Path) -> None: + for fixture in fixtures: + if sha256_file(fixture.source) != fixture.sha256: + raise FixtureError( + f"fixture source changed before staging: {fixture.dest_rel}" + ) + dest = _fixture_destination(cwd, fixture.dest_rel) + dest.parent.mkdir(parents=True, exist_ok=True) + dest.write_bytes(fixture.content) + dest.chmod(fixture.mode) + if sha256_file(dest) != fixture.sha256: + raise FixtureError( + f"staged fixture hash mismatch: {fixture.dest_rel}" + ) + + +def verify_staged_fixtures(fixtures: list[Fixture], cwd: Path) -> list[str]: + errors: list[str] = [] + for fixture in fixtures: + try: + dest = _fixture_destination(cwd, fixture.dest_rel) + actual = sha256_file(dest) + except (FileNotFoundError, OSError, RuntimeError, ValueError) as exc: + errors.append(f"{fixture.dest_rel}: {exc}") + continue + if actual != fixture.sha256: + errors.append( + f"{fixture.dest_rel}: expected {fixture.sha256}, got {actual}" + ) + return errors + + +def _records_digest(records: list[dict]) -> str: + canonical = json.dumps( + sorted(records, key=lambda item: item["path"]), + separators=(",", ":"), + sort_keys=True, + ).encode("utf-8") + return f"sha256:{sha256_bytes(canonical)}" + + +def fixture_evidence_digest(fixtures: list[Fixture]) -> str: + return _records_digest([{ + "path": fixture.dest_rel.as_posix(), + "sha256": fixture.sha256, + "size": len(fixture.content), + } for fixture in fixtures]) + + +def persist_fixture_evidence(fixtures: list[Fixture], + case_dir: Path) -> tuple[Path, str]: + """Persist evidence after adapter exit and return its stdout trust anchor.""" + evidence_dir = case_dir / "fixture-evidence" + if evidence_dir.exists() or evidence_dir.is_symlink(): + rejected = case_dir / f"fixture-evidence-rejected-{time.time_ns()}" + evidence_dir.rename(rejected) + raise FixtureError("fixture evidence path existed before persistence") + snapshot_dir = evidence_dir / "snapshot" + snapshot_dir.mkdir(parents=True) + records = [] + for fixture in fixtures: + dest = _fixture_destination(snapshot_dir, fixture.dest_rel) + dest.parent.mkdir(parents=True, exist_ok=True) + dest.write_bytes(fixture.content) + dest.chmod(0o400) + records.append({ + "path": fixture.dest_rel.as_posix(), + "sha256": fixture.sha256, + "size": len(fixture.content), + }) + digest = _records_digest(records) + manifest_path = evidence_dir / "manifest.json" + manifest_path.write_text(json.dumps({ + "schema": "bmad-fixture-evidence/v1", + "captured_at": datetime.now(timezone.utc).strftime( + "%Y-%m-%dT%H:%M:%SZ" + ), + "artifact_digest": digest, + "fixtures": records, + }, indent=2) + "\n", encoding="utf-8") + manifest_path.chmod(0o400) + for directory in sorted( + (p for p in snapshot_dir.rglob("*") if p.is_dir()), reverse=True): + directory.chmod(0o500) + snapshot_dir.chmod(0o500) + evidence_dir.chmod(0o500) + validate_fixture_evidence(evidence_dir, digest) + return evidence_dir, digest + + +def validate_fixture_evidence(evidence_dir: Path, + expected_digest: str) -> None: + """Fail unless snapshot, manifest, and caller-held digest agree exactly.""" + manifest = json.loads( + (evidence_dir / "manifest.json").read_text(encoding="utf-8") + ) + if not isinstance(manifest, dict) or ( + manifest.get("schema") != "bmad-fixture-evidence/v1"): + raise FixtureError("fixture evidence manifest schema is invalid") + raw_records = manifest.get("fixtures") + if not isinstance(raw_records, list): + raise FixtureError("fixture evidence records are invalid") + snapshot_dir = evidence_dir / "snapshot" + records = [] + paths: set[Path] = set() + for item in raw_records: + if not isinstance(item, dict): + raise FixtureError("fixture evidence record is invalid") + rel = _fixture_relative_path(item.get("path", "")) + if rel in paths: + raise FixtureError(f"duplicate fixture evidence path: {rel}") + content = _fixture_destination(snapshot_dir, rel).read_bytes() + record = { + "path": rel.as_posix(), + "sha256": sha256_bytes(content), + "size": len(content), + } + if record != item: + raise FixtureError(f"fixture evidence hash mismatch: {rel}") + records.append(record) + paths.add(rel) + actual_paths = { + path.relative_to(snapshot_dir) + for path in snapshot_dir.rglob("*") if path.is_file() + } + if actual_paths != paths: + raise FixtureError("fixture evidence snapshot file set is invalid") + actual_digest = _records_digest(records) + if (actual_digest != expected_digest or + manifest.get("artifact_digest") != expected_digest): + raise FixtureError("fixture evidence digest mismatch") diff --git a/skills/bmad-eval-runner/scripts/run_evals.py b/skills/bmad-eval-runner/scripts/run_evals.py index 40cd7e7..3a4480b 100644 --- a/skills/bmad-eval-runner/scripts/run_evals.py +++ b/skills/bmad-eval-runner/scripts/run_evals.py @@ -86,20 +86,29 @@ from __future__ import annotations import argparse -import hashlib import json import os +import secrets import shutil -import stat import subprocess import sys import time from collections.abc import Mapping from concurrent.futures import ThreadPoolExecutor, as_completed -from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path +from fixture_integrity import ( + Fixture, + FixtureError, + fixture_evidence_digest, + persist_fixture_evidence, + resolve_fixtures, + stage_fixtures, + validate_fixture_evidence, + verify_staged_fixtures, +) + # --- small self-contained helpers (no Docker/keychain imports) ------------- @@ -108,7 +117,8 @@ def utc_now_iso() -> str: def new_run_id(label: str) -> str: - return f"{datetime.now().strftime('%Y%m%d-%H%M%S')}-{label}" + timestamp = datetime.now().strftime("%Y%m%d-%H%M%S-%f") + return f"{timestamp}-{secrets.token_hex(4)}-{label}" def write_json(path: Path, data: object) -> None: @@ -206,180 +216,8 @@ def stage_skill(skill_path: Path, cwd: Path, skills_subdir: str) -> Path: return dest -@dataclass(frozen=True) -class Fixture: - __slots__ = ("source", "dest_rel", "content", "sha256", "mode") - - source: Path - dest_rel: Path - content: bytes - sha256: str - mode: int - - -class FixtureError(ValueError): - pass - - -def sha256_bytes(data: bytes) -> str: - return hashlib.sha256(data).hexdigest() - - -def sha256_file(path: Path) -> str: - return sha256_bytes(path.read_bytes()) - - -def _fixture_relative_path(entry: str | Path) -> Path: - rel = Path(str(entry)) - if rel.is_absolute() or ".." in rel.parts: - raise FixtureError( - f"fixture path must be relative without '..': {entry}" - ) - if not rel.parts or rel == Path("."): - raise FixtureError(f"fixture path must name a file: {entry}") - return rel - - -def _contained(path: Path, root: Path, label: str) -> Path: - root = root.resolve() - resolved = path.resolve(strict=True) - try: - resolved.relative_to(root) - except ValueError as exc: - raise FixtureError(f"{label} escapes trusted root: {path}") from exc - return resolved - - -def resolve_fixtures(files: list, project_root: Path, - cases_dir: Path) -> list[Fixture]: - """Map each `files` entry to (source, dest-relative-path). - - The entry's own relative path is preserved inside the cwd, so a bare - filename lands at the workspace root and a nested path keeps its - directory structure — matching the path the case input references. - """ - roots = tuple(dict.fromkeys((project_root.resolve(), cases_dir.resolve()))) - out: list[Fixture] = [] - destinations: set[Path] = set() - for entry in files or []: - rel = _fixture_relative_path(entry) - if rel in destinations: - raise FixtureError(f"duplicate fixture destination: {rel}") - for root in roots: - candidate = root / rel - if candidate.exists() or candidate.is_symlink(): - source = _contained(candidate, root, "fixture source") - if not source.is_file(): - raise FixtureError( - f"fixture source is not a file: {candidate}" - ) - content = source.read_bytes() - out.append(Fixture( - source=source, - dest_rel=rel, - content=content, - sha256=sha256_bytes(content), - mode=stat.S_IMODE(source.stat().st_mode), - )) - destinations.add(rel) - break - else: - raise FixtureError(f"fixture not found in trusted roots: {entry}") - return out - - -def _fixture_destination(cwd: Path, dest_rel: Path) -> Path: - cwd = cwd.resolve() - dest = cwd / dest_rel - try: - dest.resolve(strict=False).relative_to(cwd) - except ValueError as exc: - raise FixtureError( - f"fixture destination escapes case cwd: {dest_rel}" - ) from exc - current = cwd - for part in dest_rel.parts[:-1]: - current /= part - if current.is_symlink(): - raise FixtureError( - f"fixture destination traverses symlink: {dest_rel}" - ) - if dest.is_symlink(): - raise FixtureError(f"fixture destination is a symlink: {dest_rel}") - return dest - - -def stage_fixtures(fixtures: list[Fixture], cwd: Path) -> None: - for fixture in fixtures: - if sha256_file(fixture.source) != fixture.sha256: - raise FixtureError( - f"fixture source changed before staging: {fixture.dest_rel}" - ) - dest = _fixture_destination(cwd, fixture.dest_rel) - dest.parent.mkdir(parents=True, exist_ok=True) - dest.write_bytes(fixture.content) - dest.chmod(fixture.mode) - if sha256_file(dest) != fixture.sha256: - raise FixtureError( - f"staged fixture hash mismatch: {fixture.dest_rel}" - ) - - -def verify_staged_fixtures(fixtures: list[Fixture], cwd: Path) -> list[str]: - errors: list[str] = [] - for fixture in fixtures: - try: - dest = _fixture_destination(cwd, fixture.dest_rel) - actual = sha256_file(dest) - except (FileNotFoundError, OSError, RuntimeError, ValueError) as exc: - errors.append(f"{fixture.dest_rel}: {exc}") - continue - if actual != fixture.sha256: - errors.append( - f"{fixture.dest_rel}: expected {fixture.sha256}, got {actual}" - ) - return errors - - -def persist_fixture_evidence(fixtures: list[Fixture], case_dir: Path) -> Path: - """Persist the trusted snapshot only after the adapter has exited. - - Until this point the immutable bytes and hashes exist only in the parent - process, outside the adapter's cwd and argv. Pre-existing evidence paths - indicate an adapter write attempt and fail closed. - """ - evidence_dir = case_dir / "fixture-evidence" - if evidence_dir.exists() or evidence_dir.is_symlink(): - rejected = case_dir / f"fixture-evidence-rejected-{time.time_ns()}" - evidence_dir.rename(rejected) - raise FixtureError("fixture evidence path existed before persistence") - snapshot_dir = evidence_dir / "snapshot" - snapshot_dir.mkdir(parents=True) - manifest_fixtures = [] - for fixture in fixtures: - dest = _fixture_destination(snapshot_dir, fixture.dest_rel) - dest.parent.mkdir(parents=True, exist_ok=True) - dest.write_bytes(fixture.content) - dest.chmod(0o400) - manifest_fixtures.append({ - "path": fixture.dest_rel.as_posix(), - "sha256": fixture.sha256, - "size": len(fixture.content), - }) - manifest_path = evidence_dir / "manifest.json" - write_json(manifest_path, { - "schema": "bmad-fixture-evidence/v1", - "captured_at": utc_now_iso(), - "fixtures": manifest_fixtures, - }) - manifest_path.chmod(0o400) - for directory in sorted( - (p for p in snapshot_dir.rglob("*") if p.is_dir()), reverse=True): - directory.chmod(0o500) - snapshot_dir.chmod(0o500) - evidence_dir.chmod(0o500) - return evidence_dir +# Fixture containment/attestation lives in fixture_integrity.py # --- case composition ------------------------------------------------------- @@ -486,8 +324,11 @@ def run_case(case: dict, case_dir: Path, run_dir: Path, (case_dir / "prompt.txt").write_text(prompt, encoding="utf-8") write_json(case_dir / "case.json", case) + trusted_digest = fixture_evidence_digest(fixtures) + if adapter is None: - persist_fixture_evidence(fixtures, case_dir) + evidence_dir, digest = persist_fixture_evidence(fixtures, case_dir) + validate_fixture_evidence(evidence_dir, trusted_digest) result = { "case_id": case_id, "config": config, @@ -495,10 +336,12 @@ def run_case(case: dict, case_dir: Path, run_dir: Path, "reason": "no runtime adapter configured", "prompt_chars": len(prompt), "cwd": str(cwd.relative_to(run_dir)), + "fixture_evidence_digest": digest, } write_json(case_dir / "timing.json", { "case_id": case_id, "config": config, "status": "skipped", "captured_at": utc_now_iso(), + "fixture_evidence_digest": digest, }) return result @@ -531,10 +374,12 @@ def run_case(case: dict, case_dir: Path, run_dir: Path, except FileNotFoundError as e: # Adapter invocation command is not on PATH: degrade, do not crash. elapsed = time.time() - start - persist_fixture_evidence(fixtures, case_dir) + evidence_dir, digest = persist_fixture_evidence(fixtures, case_dir) + validate_fixture_evidence(evidence_dir, trusted_digest) write_json(case_dir / "timing.json", { "case_id": case_id, "config": config, "status": "adapter-missing", "elapsed_s": round(elapsed, 3), "captured_at": utc_now_iso(), + "fixture_evidence_digest": digest, }) return { "case_id": case_id, @@ -542,6 +387,7 @@ def run_case(case: dict, case_dir: Path, run_dir: Path, "status": "adapter-missing", "reason": f"invocation command not found: {e}", "cwd": str(cwd.relative_to(run_dir)), + "fixture_evidence_digest": digest, } except subprocess.TimeoutExpired as e: captured = e.stdout or b"" @@ -551,8 +397,11 @@ def run_case(case: dict, case_dir: Path, run_dir: Path, elapsed = time.time() - start fixture_errors = verify_staged_fixtures(fixtures, cwd) + evidence_digest = trusted_digest try: - persist_fixture_evidence(fixtures, case_dir) + evidence_dir, evidence_digest = persist_fixture_evidence(fixtures, case_dir) + # Trust anchor is the parent-held digest (not chmod bits on disk). + validate_fixture_evidence(evidence_dir, trusted_digest) except (OSError, RuntimeError, ValueError) as exc: fixture_errors.append(str(exc)) if fixture_errors: @@ -581,6 +430,7 @@ def run_case(case: dict, case_dir: Path, run_dir: Path, "total_steps": accounting["total_steps"], "total_tool_calls": accounting["total_tool_calls"], "captured_at": utc_now_iso(), + "fixture_evidence_digest": evidence_digest, } write_json(case_dir / "timing.json", timing) @@ -595,6 +445,7 @@ def run_case(case: dict, case_dir: Path, run_dir: Path, "tokens": accounting["total_tokens"], "tool_calls": accounting["tool_calls"], "error_tail": error_tail, + "fixture_evidence_digest": evidence_digest, } @@ -697,8 +548,10 @@ def main(argv: list[str] | None = None) -> int: adapter_note = f"invalid: {e}" run_id = new_run_id(args.label) + args.output_dir.mkdir(parents=True, exist_ok=True) run_dir = (args.output_dir / run_id).resolve() - run_dir.mkdir(parents=True, exist_ok=True) + # Exclusive create: concurrent workers must not share/clobber a run dir. + run_dir.mkdir(parents=False, exist_ok=False) write_json(run_dir / "run.json", { "run_id": run_id, @@ -773,10 +626,12 @@ def main(argv: list[str] | None = None) -> int: } write_json(run_dir / "execution-summary.json", summary) print(json.dumps(summary, indent=2)) - return 1 if any( - result.get("status") == "fixture-integrity-error" - for result in results - ) else 0 + # Fail closed on integrity errors and unexpected exceptions (e.g. staging). + fail_statuses = { + "fixture-integrity-error", + "exception", + } + return 1 if any(result.get("status") in fail_statuses for result in results) else 0 if __name__ == "__main__": diff --git a/skills/bmad-eval-runner/scripts/tests/test_fixture_integrity.py b/skills/bmad-eval-runner/scripts/tests/test_fixture_integrity.py index e11685d..dfafbbe 100644 --- a/skills/bmad-eval-runner/scripts/tests/test_fixture_integrity.py +++ b/skills/bmad-eval-runner/scripts/tests/test_fixture_integrity.py @@ -185,3 +185,72 @@ def resolve_then_mutate(files, project_root, cases_dir): summary_path = next(output.glob("*/execution-summary.json")) summary = json.loads(summary_path.read_text(encoding="utf-8")) assert summary["results"][0]["status"] == "fixture-integrity-error" + + +def test_parent_held_digest_detects_evidence_tamper(tmp_path): + """chmod 0400 is not the trust boundary; parent-held digest is.""" + import fixture_integrity as fi + + source = tmp_path / "source" + fixtures = fixture(source) + case_dir = tmp_path / "case" + case_dir.mkdir() + trusted = fi.fixture_evidence_digest(fixtures) + evidence_dir, digest = fi.persist_fixture_evidence(fixtures, case_dir) + assert digest == trusted + fi.validate_fixture_evidence(evidence_dir, trusted) + + # Same-UID process can still rewrite after chmod; digest must catch it. + snap = evidence_dir / "snapshot" / "fixture.txt" + snap.chmod(0o600) + snap.write_bytes(b"forged") + snap.chmod(0o400) + + with pytest.raises(ValueError, match="digest mismatch|hash mismatch"): + fi.validate_fixture_evidence(evidence_dir, trusted) + + +def test_run_dir_is_exclusive(tmp_path): + project = tmp_path / "project" + project.mkdir() + cases = project / "cases.json" + cases.write_text(json.dumps({"cases": [{ + "id": "empty", + "input": "test", + "rubric": [], + "files": [], + }]}), encoding="utf-8") + skill = tmp_path / "skill" + skill.mkdir() + (skill / "SKILL.md").write_text("# test", encoding="utf-8") + output = tmp_path / "output" + + # Force collision on run id + fixed = "fixed-run-id" + monkey_ids = iter([fixed, fixed]) + + def fixed_id(label): + return next(monkey_ids) + + original = run_evals.new_run_id + run_evals.new_run_id = fixed_id + try: + assert run_evals.main([ + "--cases", str(cases), + "--skill-path", str(skill), + "--project-root", str(project), + "--output-dir", str(output), + "--workers", "1", + "--quiet", + ]) == 0 + with pytest.raises(FileExistsError): + run_evals.main([ + "--cases", str(cases), + "--skill-path", str(skill), + "--project-root", str(project), + "--output-dir", str(output), + "--workers", "1", + "--quiet", + ]) + finally: + run_evals.new_run_id = original