diff --git a/skills/bmad-eval-runner/SKILL.md b/skills/bmad-eval-runner/SKILL.md index 171b693..cecaad8 100644 --- a/skills/bmad-eval-runner/SKILL.md +++ b/skills/bmad-eval-runner/SKILL.md @@ -47,7 +47,7 @@ These map directly onto the script CLIs below; anything not listed there (case s 4. Locate the skill and verify `/SKILL.md` exists. Halt with a clear error if it does not. -5. Resolve the adapter config per the discovery rules in `references/platform-adapter.md` (explicit `--adapter`, `BMAD_EVAL_ADAPTER`, `adapter.json` beside the cases file). When nothing is configured and the current runtime is Claude Code, use `{skill-root}/assets/adapter-claude-code.json`. +5. Resolve the adapter config per the trusted configuration rules in `references/platform-adapter.md` (explicit `--adapter` or `BMAD_EVAL_ADAPTER`). Never load an adapter from beside an untrusted cases or queries file. When nothing is configured and the current runtime is Claude Code, use `{skill-root}/assets/adapter-claude-code.json`. 6. Discover the cases file. Look at `--evals` first, then `/evals/`, then `/../../evals//`, then `/evals//`, then anywhere under `/evals/`. Take the first match. If nothing is found, halt and say so; the runner does not invent cases. diff --git a/skills/bmad-eval-runner/references/platform-adapter.md b/skills/bmad-eval-runner/references/platform-adapter.md index a17e739..10bcd61 100644 --- a/skills/bmad-eval-runner/references/platform-adapter.md +++ b/skills/bmad-eval-runner/references/platform-adapter.md @@ -34,9 +34,13 @@ An adapter is a JSON file the scripts read. A working Claude Code adapter ships 1. `--adapter ` on the command line. 2. `BMAD_EVAL_ADAPTER` env var pointing at a config file. -3. `adapter.json` or `.bmad-eval-adapter.json` beside the cases/queries file. -Nothing found means the run degrades to staging-only (cases prepared, results recorded as skipped). When the current runtime is Claude Code and no project adapter exists, pass `--adapter {skill-root}/assets/adapter-claude-code.json`. +The runner never searches beside the cases/queries file. Those directories may +come from an untrusted bundle, and the adapter controls executable argv and +optional host-environment forwarding. Nothing found means the run degrades to +staging-only (cases prepared, results recorded as skipped). When the current +runtime is Claude Code and no project adapter exists, pass +`--adapter {skill-root}/assets/adapter-claude-code.json`. ## Invocation and isolation diff --git a/skills/bmad-eval-runner/scripts/adapter.py b/skills/bmad-eval-runner/scripts/adapter.py new file mode 100644 index 0000000..10b8a8e --- /dev/null +++ b/skills/bmad-eval-runner/scripts/adapter.py @@ -0,0 +1,79 @@ +"""Trusted adapter configuration helpers for the eval runners. + +Adapter configuration is executable configuration: it chooses the subprocess +argv and which host environment variables are forwarded. It must therefore be +selected by an explicit command-line option or an operator-controlled +environment variable, never by a file shipped beside an untrusted eval bundle. +""" + +from __future__ import annotations + +import json +import os +from collections.abc import Mapping +from pathlib import Path + + +def find_adapter(explicit: Path | None, _data_file: Path) -> Path | None: + """Return only explicitly selected adapter configuration. + + ``data_file`` is accepted to keep the call contract clear at both runner + sites, but its parent directory is deliberately never searched. Cases and + query bundles are untrusted input and may contain an adjacent adapter that + would otherwise control command execution. + """ + if explicit is not None: + candidate = explicit.expanduser() + return candidate if candidate.is_file() else None + + env_path = os.environ.get("BMAD_EVAL_ADAPTER") + if env_path: + candidate = Path(env_path).expanduser() + if candidate.is_file(): + return candidate + return None + + +def load_adapter(path: Path) -> dict: + """Load and minimally validate a trusted adapter configuration.""" + cfg = json.loads(path.read_text(encoding="utf-8")) + if not isinstance(cfg, dict): + raise ValueError(f"adapter config must be a JSON object: {path}") + if ("invocation" not in cfg + or not isinstance(cfg["invocation"], list) + or not cfg["invocation"]): + raise ValueError("adapter config missing 'invocation' argv list") + return cfg + + +def build_argv(invocation: list, prompt: str, cwd: str) -> list[str]: + """Expand the supported prompt, query, and cwd argv placeholders.""" + argv: list[str] = [] + for tok in invocation: + tok = str(tok) + tok = (tok.replace("{prompt}", prompt) + .replace("{query}", prompt) + .replace("{cwd}", cwd)) + argv.append(tok) + return argv + + +def build_case_env(adapter: Mapping | None, home_dir: Path, + host_env: Mapping[str, str]) -> dict[str, str]: + """Build the minimal subprocess environment from trusted config.""" + adapter = adapter or {} + env = { + "PATH": host_env.get("PATH", ""), + "HOME": str(home_dir), + "CLAUDE_CONFIG_DIR": str(home_dir / ".claude"), + } + auth_env = adapter.get("auth_env") + if auth_env: + val = host_env.get(str(auth_env)) + if val: + env[str(auth_env)] = val + for key in adapter.get("env_passthrough") or []: + val = host_env.get(str(key)) + if val is not None: + env[str(key)] = val + return env diff --git a/skills/bmad-eval-runner/scripts/run_evals.py b/skills/bmad-eval-runner/scripts/run_evals.py index c518c93..884ce13 100644 --- a/skills/bmad-eval-runner/scripts/run_evals.py +++ b/skills/bmad-eval-runner/scripts/run_evals.py @@ -90,11 +90,13 @@ import subprocess import sys import time -from collections.abc import Mapping from concurrent.futures import ThreadPoolExecutor, as_completed from datetime import datetime, timezone from pathlib import Path +import adapter +from adapter import build_argv, build_case_env, load_adapter + # --- small self-contained helpers (no Docker/keychain imports) ------------- @@ -115,71 +117,9 @@ def read_json(path: Path) -> object: return json.loads(path.read_text(encoding="utf-8")) -# --- adapter ---------------------------------------------------------------- - def find_adapter(explicit: Path | None, cases_file: Path) -> Path | None: - """Locate the adapter config. Returns None when none is configured.""" - if explicit is not None: - return explicit if explicit.is_file() else None - env_path = os.environ.get("BMAD_EVAL_ADAPTER") - if env_path and Path(env_path).is_file(): - return Path(env_path) - for candidate in ( - cases_file.parent / "adapter.json", - cases_file.parent / ".bmad-eval-adapter.json", - ): - if candidate.is_file(): - return candidate - return None - - -def load_adapter(path: Path) -> dict: - cfg = read_json(path) - if not isinstance(cfg, dict): - raise ValueError(f"adapter config must be a JSON object: {path}") - if "invocation" not in cfg or not isinstance(cfg["invocation"], list): - raise ValueError("adapter config missing 'invocation' argv list") - return cfg - - -def build_argv(invocation: list, prompt: str, cwd: str) -> list[str]: - argv: list[str] = [] - for tok in invocation: - tok = str(tok) - tok = (tok.replace("{prompt}", prompt) - .replace("{query}", prompt) - .replace("{cwd}", cwd)) - argv.append(tok) - return argv - - -def build_case_env(adapter: Mapping | None, home_dir: Path, - host_env: Mapping[str, str]) -> dict[str, str]: - """Build the subprocess environment from scratch — never from os.environ. - - Inheriting the host env would leak shell config, tokens, and runtime - state into the clean room. The env holds exactly: PATH, a fresh HOME, - CLAUDE_CONFIG_DIR inside it, the adapter's auth var ONLY when set - non-empty in the host (an empty-string auth var breaks the runtime's own - credential fallback), and any adapter env_passthrough keys present in - the host env. - """ - adapter = adapter or {} - env = { - "PATH": host_env.get("PATH", ""), - "HOME": str(home_dir), - "CLAUDE_CONFIG_DIR": str(home_dir / ".claude"), - } - auth_env = adapter.get("auth_env") - if auth_env: - val = host_env.get(str(auth_env)) - if val: - env[str(auth_env)] = val - for key in adapter.get("env_passthrough") or []: - val = host_env.get(str(key)) - if val is not None: - env[str(key)] = val - return env + """Resolve adapter configuration through this runner's public seam.""" + return adapter.find_adapter(explicit, cases_file) # --- staging: skill under test + fixtures ------------------------------------ @@ -470,8 +410,8 @@ def main(argv: list[str] | None = None) -> int: help="base for resolving fixture paths; defaults to the " "cases file's directory") p.add_argument("--adapter", type=Path, default=None, - help="adapter config JSON; defaults to BMAD_EVAL_ADAPTER env " - "or adapter.json beside the cases file") + help="trusted adapter config JSON; defaults to the " + "BMAD_EVAL_ADAPTER environment variable") p.add_argument("--case-ids", default=None, help="comma-separated subset of case ids to run") p.add_argument("--runs", type=int, default=1, diff --git a/skills/bmad-eval-runner/scripts/run_triggers.py b/skills/bmad-eval-runner/scripts/run_triggers.py index a406b27..efb390a 100644 --- a/skills/bmad-eval-runner/scripts/run_triggers.py +++ b/skills/bmad-eval-runner/scripts/run_triggers.py @@ -65,6 +65,9 @@ from datetime import datetime, timezone from pathlib import Path +import adapter +from adapter import build_argv, build_case_env, load_adapter + # --- self-contained helpers ------------------------------------------------- @@ -85,6 +88,11 @@ def read_json(path: Path) -> object: return json.loads(path.read_text(encoding="utf-8")) +def find_adapter(explicit: Path | None, queries_file: Path) -> Path | None: + """Resolve adapter configuration through this runner's public seam.""" + return adapter.find_adapter(explicit, queries_file) + + def parse_skill_md(skill_path: Path) -> tuple[str, str]: """Return (name, description) from SKILL.md frontmatter.""" text = (skill_path / "SKILL.md").read_text(encoding="utf-8") @@ -115,69 +123,6 @@ def parse_skill_md(skill_path: Path) -> tuple[str, str]: return name, " ".join(desc_lines).strip() -# --- adapter ---------------------------------------------------------------- - -def find_adapter(explicit: Path | None, queries_file: Path) -> Path | None: - if explicit is not None: - return explicit if explicit.is_file() else None - env_path = os.environ.get("BMAD_EVAL_ADAPTER") - if env_path and Path(env_path).is_file(): - return Path(env_path) - for candidate in ( - queries_file.parent / "adapter.json", - queries_file.parent / ".bmad-eval-adapter.json", - ): - if candidate.is_file(): - return candidate - return None - - -def load_adapter(path: Path) -> dict: - cfg = read_json(path) - if not isinstance(cfg, dict) or "invocation" not in cfg: - raise ValueError(f"adapter config missing 'invocation': {path}") - return cfg - - -def build_argv(invocation: list, query: str, cwd: str) -> list[str]: - out: list[str] = [] - for tok in invocation: - tok = (str(tok).replace("{prompt}", query) - .replace("{query}", query) - .replace("{cwd}", cwd)) - out.append(tok) - return out - - -def build_case_env(adapter: dict | None, home_dir: Path, - host_env: dict) -> dict[str, str]: - """Build the subprocess environment from scratch — never from os.environ. - - Inheriting the host env would leak shell config, tokens, and runtime - state into the clean room. The env holds exactly: PATH, a fresh HOME, - CLAUDE_CONFIG_DIR inside it, the adapter's auth var ONLY when set - non-empty in the host (an empty-string auth var breaks the runtime's own - credential fallback), and any adapter env_passthrough keys present in - the host env. - """ - adapter = adapter or {} - env = { - "PATH": host_env.get("PATH", ""), - "HOME": str(home_dir), - "CLAUDE_CONFIG_DIR": str(home_dir / ".claude"), - } - auth_env = adapter.get("auth_env") - if auth_env: - val = host_env.get(str(auth_env)) - if val: - env[str(auth_env)] = val - for key in adapter.get("env_passthrough") or []: - val = host_env.get(str(key)) - if val is not None: - env[str(key)] = val - return env - - # --- synthetic skill staging ------------------------------------------------ def write_synthetic_skill(skills_dir: Path, skill_name: str, @@ -309,7 +254,9 @@ def main(argv: list[str] | None = None) -> int: p.add_argument("--skill-path", required=True, type=Path) p.add_argument("--queries", required=True, type=Path) p.add_argument("--output-dir", required=True, type=Path) - p.add_argument("--adapter", type=Path, default=None) + p.add_argument("--adapter", type=Path, default=None, + help="trusted adapter config JSON; defaults to the " + "BMAD_EVAL_ADAPTER environment variable") p.add_argument("--runs-per-query", type=int, default=3) p.add_argument("--threshold", type=float, default=0.5) p.add_argument("--timeout", type=int, default=60) diff --git a/skills/bmad-eval-runner/scripts/tests/test_adapter_resolution.py b/skills/bmad-eval-runner/scripts/tests/test_adapter_resolution.py new file mode 100644 index 0000000..d092442 --- /dev/null +++ b/skills/bmad-eval-runner/scripts/tests/test_adapter_resolution.py @@ -0,0 +1,93 @@ +#!/usr/bin/env python3 +"""Guard the adapter trust boundary in both eval runners.""" + +import json +import os +import sys +import tempfile +from pathlib import Path +from unittest.mock import patch + +SCRIPTS_DIR = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(SCRIPTS_DIR)) + +import run_evals # noqa: E402 +import run_triggers # noqa: E402 + +FINDERS = [ + ("run_evals", run_evals.find_adapter), + ("run_triggers", run_triggers.find_adapter), +] + + +def write_adapter(path: Path) -> None: + path.write_text(json.dumps({"invocation": ["trusted-runtime"]}), + encoding="utf-8") + + +def test_sibling_adapter_is_ignored(): + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + data_file = root / "cases.json" + data_file.write_text("[]", encoding="utf-8") + for filename in ("adapter.json", ".bmad-eval-adapter.json"): + write_adapter(root / filename) + with patch.dict(os.environ, {}, clear=True): + for runner, find in FINDERS: + assert find(None, data_file) is None, runner + + +def test_explicit_adapter_is_used_even_beside_bundle(): + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + data_file = root / "queries.json" + explicit = root / "trusted" / "adapter.json" + configured = root / "trusted" / "env-adapter.json" + data_file.write_text("[]", encoding="utf-8") + (root / "trusted").mkdir() + write_adapter(explicit) + write_adapter(configured) + for filename in ("adapter.json", ".bmad-eval-adapter.json"): + (root / filename).write_text( + json.dumps({"invocation": ["attacker-controlled"]}), + encoding="utf-8", + ) + with patch.dict(os.environ, + {"BMAD_EVAL_ADAPTER": str(configured)}, clear=True): + for runner, find in FINDERS: + assert find(explicit, data_file) == explicit, runner + + +def test_operator_environment_adapter_is_used(): + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + data_file = root / "cases.json" + configured = root / "trusted-adapter.json" + data_file.write_text("[]", encoding="utf-8") + write_adapter(configured) + with patch.dict(os.environ, + {"BMAD_EVAL_ADAPTER": str(configured)}, clear=True): + for runner, find in FINDERS: + assert find(None, data_file) == configured, runner + + +def test_empty_invocation_is_rejected(): + with tempfile.TemporaryDirectory() as temp: + config = Path(temp) / "empty.json" + config.write_text(json.dumps({"invocation": []}), encoding="utf-8") + for runner in (run_evals, run_triggers): + try: + runner.load_adapter(config) + except ValueError as exc: + assert "invocation" in str(exc), runner.__name__ + else: + raise AssertionError( + f"{runner.__name__} accepted an empty invocation") + + +if __name__ == "__main__": + test_sibling_adapter_is_ignored() + test_explicit_adapter_is_used_even_beside_bundle() + test_operator_environment_adapter_is_used() + test_empty_invocation_is_rejected() + print("ok: adapter selection requires explicit trusted configuration")