From 218153f39350734fd5bdb8ac02b6a875d690f4b0 Mon Sep 17 00:00:00 2001 From: Dan Morosanu Date: Thu, 6 Aug 2026 13:42:43 +0300 Subject: [PATCH 1/4] feat(plugins): stage agent-visible plugin bundles with digest verification and agent env scrub --- src/coder_eval/agents/antigravity_agent.py | 30 +- src/coder_eval/agents/claude_code_agent.py | 9 +- src/coder_eval/agents/codex_agent.py | 9 +- src/coder_eval/orchestrator.py | 38 ++ src/coder_eval/plugin_bundle.py | 365 ++++++++++++++++++ src/coder_eval/utils.py | 26 ++ .../rules/ce034_plugin_bundle_choke_point.py | 75 ++++ tests/lint/runner.py | 2 + tests/test_antigravity_agent.py | 28 ++ tests/test_codex_agent.py | 48 ++- tests/test_custom_lint.py | 45 +++ tests/test_plugin_bundle.py | 357 +++++++++++++++++ 12 files changed, 1004 insertions(+), 28 deletions(-) create mode 100644 src/coder_eval/plugin_bundle.py create mode 100644 tests/lint/rules/ce034_plugin_bundle_choke_point.py create mode 100644 tests/test_plugin_bundle.py diff --git a/src/coder_eval/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 4e1475be..bafc1258 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/src/coder_eval/agents/antigravity_agent.py @@ -66,7 +66,7 @@ TurnEndStatus, TurnStartEvent, ) -from coder_eval.utils import expand_env_vars +from coder_eval.utils import AGENT_ENV_SCRUB_PREFIXES, AGENT_ENV_SCRUB_VARS, expand_env_vars logger = logging.getLogger(__name__) @@ -389,22 +389,32 @@ async def _harness_spawn_guard(self) -> AsyncIterator[None]: never affects the live harness. The lock is taken even when no prepend dirs were configured: a no-prepend spawn must still wait out any in-flight mutated- PATH window, or its harness would inherit another task's mock dirs. + + The same spawn window scrubs harness-internal vars (SKILLS_REPO_PATH — the + raw skills checkout with grading material — and CODER_EVAL_*) out of + ``os.environ`` so the harness subprocess never inherits them; they are + restored for the harness process itself in the same ``finally``. """ async with _harness_spawn_lock(): - if not self._env_path_prepend: - yield - return + scrubbed = { + k: os.environ.pop(k) + for k in list(os.environ) + if k in AGENT_ENV_SCRUB_VARS or k.startswith(AGENT_ENV_SCRUB_PREFIXES) + } path_key = next((k for k in os.environ if k.upper() == "PATH"), "PATH") original = os.environ.get(path_key) - os.environ[path_key] = os.pathsep.join([*self._env_path_prepend, original or ""]) - self._log.debug("PATH prepend for harness spawn: %s", os.pathsep.join(self._env_path_prepend)) + if self._env_path_prepend: + os.environ[path_key] = os.pathsep.join([*self._env_path_prepend, original or ""]) + self._log.debug("PATH prepend for harness spawn: %s", os.pathsep.join(self._env_path_prepend)) try: yield finally: - if original is None: - os.environ.pop(path_key, None) - else: - os.environ[path_key] = original + os.environ.update(scrubbed) + if self._env_path_prepend: + if original is None: + os.environ.pop(path_key, None) + else: + os.environ[path_key] = original async def communicate( self, diff --git a/src/coder_eval/agents/claude_code_agent.py b/src/coder_eval/agents/claude_code_agent.py index 71cb2267..f780c15b 100644 --- a/src/coder_eval/agents/claude_code_agent.py +++ b/src/coder_eval/agents/claude_code_agent.py @@ -70,7 +70,7 @@ TurnEndStatus, TurnStartEvent, ) -from coder_eval.utils import dump_dataclass, process_plugins +from coder_eval.utils import dump_dataclass, process_plugins, scrub_agent_env_overrides logger = logging.getLogger(__name__) @@ -765,7 +765,12 @@ def _build_sdk_env( Returns: Tuple of (env_vars_dict, model_override_or_None). """ - base_env: dict[str, str] = {} + # Defence-in-depth: the SDK spawns the CLI with {**os.environ, + # **options.env}, so harness-internal vars (SKILLS_REPO_PATH — the raw + # skills checkout with grading material — and CODER_EVAL_*) reach the + # agent unless explicitly overridden here. The grading env + # (Sandbox._build_run_command_env) keeps them. + base_env: dict[str, str] = scrub_agent_env_overrides() if path := os.environ.get("PATH"): base_env["PATH"] = path diff --git a/src/coder_eval/agents/codex_agent.py b/src/coder_eval/agents/codex_agent.py index c6e4b2d3..af0c30a5 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -52,7 +52,7 @@ TurnEndStatus, TurnStartEvent, ) -from coder_eval.utils import expand_env_vars +from coder_eval.utils import expand_env_vars, scrub_agent_env_overrides logger = logging.getLogger(__name__) @@ -1089,9 +1089,12 @@ def _build_codex_env(self) -> dict[str, str] | None: The SDK merges this dict over ``os.environ`` for the app-server process (and normalizes the PATH key case-insensitively), so a full PATH value - here safely replaces the inherited one. + here safely replaces the inherited one. That same merge is why the + harness-internal vars (SKILLS_REPO_PATH / CODER_EVAL_*) are masked with + explicit empty-string overrides — omitting them would leave the + inherited values visible to the agent. """ - env: dict[str, str] = {} + env: dict[str, str] = scrub_agent_env_overrides() api_key = os.getenv("CODEX_API_KEY") if api_key: env["CODEX_API_KEY"] = api_key diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index 8640dd7f..f755bbf5 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -2,6 +2,7 @@ import asyncio import logging +import os import re import time import uuid @@ -59,6 +60,7 @@ from .orchestration.early_stop import EarlyStopWatcher, early_stop_active, validate_early_stop from .orchestration.evaluation import load_reference from .path_utils import format_task_log_id, task_log_path +from .plugin_bundle import stage_agent_plugins from .sandbox import Sandbox from .simulation import DialogStopReason, SimulatorResult, UserSimulator, evaluate_stop from .streaming.callbacks import CompositeStreamCallback, StreamCallback, TaskScopedCallback, safe_emit @@ -1077,6 +1079,13 @@ async def _setup_sandbox() -> Any: logger.info("API routing: %s", _format_routing(self.route, self.task.agent.model if self.task.agent else None)) self.success_checker = SuccessChecker(self.sandbox, route=self.eval_route) + # Stage agent-visible plugin bundles BEFORE the agent is created: + # agent.plugins[].path is rewritten from the raw source checkout (which + # carries graders / reference answers) to a verified, file-allowlisted + # bundle. Grading keeps the raw path — run_command criteria resolve + # $SKILLS_REPO_PATH from the untouched sandbox/process env. + await self._stage_agent_plugin_bundles() + # Create and start the agent. For a no-op (type: none) task this dispatches # to NoOpAgent, whose start/communicate/stop are no-ops — the orchestrator # runs the normal lifecycle without any agentless branching, and the @@ -1254,6 +1263,35 @@ def _refresh_runtime_tool_versions(self) -> None: except Exception as exc: logger.debug("Failed to refresh runtime tool versions: %s", exc) + async def _stage_agent_plugin_bundles(self) -> None: + """Rewrite ``agent.plugins[].path`` to verified agent-visible bundles. + + The bundle is built once per resolved source path per run and + digest-verified on EVERY task before its agent starts (see + :mod:`coder_eval.plugin_bundle`); a drifted bundle raises + ``PluginBundleError``, which lands the task as ``FinalStatus.ERROR`` + with component ``orchestrator.setup`` — it never falls back to the raw + path. Each source's manifest digest is recorded on + ``environment_info["plugin_bundles"]`` for audit. + + Skipped inside docker task containers (``CODER_EVAL_IN_CONTAINER``): + the docker driver owns its own host-side plugin staging/mount surface + (isolation/docker_runner.py), and re-bundling in-container would add a + per-container recursive copy without changing what the host mounted. + """ + agent_cfg = self.task.agent + if agent_cfg is None or not agent_cfg.plugins: + return + if os.environ.get("CODER_EVAL_IN_CONTAINER"): + return + staged, digests = await asyncio.to_thread( + stage_agent_plugins, + agent_cfg.plugins, # type: ignore[arg-type] + ) + agent_cfg.plugins = staged # type: ignore[assignment] + if digests and self.result is not None: + self.result.environment_info["plugin_bundles"] = digests + async def _create_agent(self) -> Agent[Any]: """Create the appropriate agent based on task configuration. diff --git a/src/coder_eval/plugin_bundle.py b/src/coder_eval/plugin_bundle.py new file mode 100644 index 00000000..64540b82 --- /dev/null +++ b/src/coder_eval/plugin_bundle.py @@ -0,0 +1,365 @@ +"""Agent-visible plugin bundles: file-level allowlist staging + digest verification. + +Tasks hand agents a local plugin source (``agent.plugins: [{type: local, path: +"$SKILLS_REPO_PATH"}]``). Expanded raw, that path points at the entire skills +checkout — which carries its own answer key (``RESOLUTION.md`` reference +answers, ``check_*.py`` grader scripts, ``tests/`` fixtures and golden +outputs). This module stages a sanitized copy — the *bundle* — and the +orchestrator rewrites ``plugins[].path`` to it, so the agent never sees the +raw checkout. Grading is unaffected: ``run_command`` criteria and the sandbox +environment keep resolving ``$SKILLS_REPO_PATH`` to the raw path. + +Threat model: task authors are TRUSTED; the agent under evaluation is the sole +adversary. The skills repo legitimately stores hundreds of ``RESOLUTION.md`` / +``check_*.py`` files alongside the skill docs the agent must read, so the +checks below are (a) the projection that keeps that material out of the +agent's view and (b) loud authoring guardrails that catch mistakes (grading +material misfiled under ``skills/``, staging drift) — not defenses against a +hostile task author. + +Three layers, each failing CLOSED (a violation raises +:class:`PluginBundleError`; nothing ever falls back to the raw path): + +1. **Subtree allowlist** — only Claude Code's plugin-discovery subtrees + (``PLUGIN_AGENT_ALLOWED_SUBDIRS``) are considered. An allowlist, not a + denylist, so a new answer-bearing top-level directory is excluded by + default. Mirrors PR #85's docker-side projection; this module is the + canonical, driver-independent home so the two cannot diverge. +2. **File-level manifest** — every staged file is declared (bundle-relative + path -> sha256) at build time. A recognizable grading artifact *inside* an + allowed subtree (e.g. ``skills/x/RESOLUTION.md``) fails the build loudly + instead of shipping; a symlink whose target escapes the source root fails + the build; in-root symlinks are copied verbatim (never followed — + loop-proof against self-referential marketplace symlinks). +3. **Runtime digest verification** — before every agent start the staged + bundle is re-hashed against its recorded manifest digest. Any drift + (tampered, added, or missing file) aborts the run with a clear setup + error. + +Bundles are built ONCE per resolved source path per process and cached +(thread-safe): the suite runs hundreds of tasks against one skills checkout, +and a per-task recursive copy would be a serious regression. Verification +(hashing only) runs per task. +""" + +from __future__ import annotations + +import atexit +import hashlib +import json +import logging +import os +import shutil +import tempfile +import threading +from dataclasses import dataclass +from fnmatch import fnmatchcase +from pathlib import Path +from typing import Any + +from coder_eval.utils import process_plugins + + +logger = logging.getLogger(__name__) + +# Claude Code's plugin discovery surface. Only these top-level subdirs of a +# plugin root are staged for the agent; grader / reference / fixture trees are +# never in this set. Keep in sync with (and canonical over) the docker +# projection in PR #85's plugin_projection module. +PLUGIN_AGENT_ALLOWED_SUBDIRS = frozenset({"skills", "commands", "agents", ".claude-plugin", "hooks"}) + +# Grading material recognizable by NAME even inside an allowed subtree. +# Matched case-insensitively; a hit FAILS the build (loud, not a silent skip) +# so answer-key material can never ship by being filed under skills/. +HIDDEN_MATERIAL_FILE_PATTERNS: tuple[str, ...] = ("resolution.md", "check_*.py") +HIDDEN_MATERIAL_DIR_NAMES: frozenset[str] = frozenset({"tests"}) + +MANIFEST_SUFFIX = ".manifest.json" + + +class PluginBundleError(Exception): + """A plugin bundle could not be built or verified; the run must not start.""" + + +@dataclass(frozen=True) +class BundleManifest: + """The file-level allowlist for one staged bundle. + + ``files`` maps bundle-relative POSIX paths to sha256 hex digests of file + content; ``symlinks`` maps bundle-relative POSIX paths to their raw link + targets. ``digest`` is the sha256 of the canonical JSON of both maps — + the single value runtime verification compares against. + """ + + source: str + files: dict[str, str] + symlinks: dict[str, str] + digest: str + + +def _compute_digest(files: dict[str, str], symlinks: dict[str, str]) -> str: + canonical = json.dumps({"files": files, "symlinks": symlinks}, sort_keys=True, separators=(",", ":")) + return hashlib.sha256(canonical.encode("utf-8")).hexdigest() + + +def _hash_file(path: Path) -> str: + hasher = hashlib.sha256() + with path.open("rb") as fh: + for chunk in iter(lambda: fh.read(1 << 20), b""): + hasher.update(chunk) + return hasher.hexdigest() + + +def _is_hidden_material(name: str, *, is_dir: bool) -> bool: + lowered = name.lower() + if is_dir: + return lowered in HIDDEN_MATERIAL_DIR_NAMES + return any(fnmatchcase(lowered, pattern) for pattern in HIDDEN_MATERIAL_FILE_PATTERNS) + + +def _check_symlink_in_root(link: Path, source_root: Path) -> str: + """Return the link's raw target if it resolves inside ``source_root``, else raise. + + Resolution uses ``os.path.realpath`` (non-strict): a broken in-root link is + fine (copied verbatim, dangles harmlessly in the bundle) and a self- + referential loop resolves to an in-root path rather than recursing — the + walk never follows links, so loops cannot recurse either. + """ + target = os.readlink(link) + resolved = Path(os.path.realpath(link)) + root = Path(os.path.realpath(source_root)) + if not resolved.is_relative_to(root): + raise PluginBundleError( + f"Plugin bundle build failed: symlink {link} -> {target!r} escapes the source root {source_root}" + ) + return target + + +def build_manifest(source: Path) -> BundleManifest: + """Walk ``source``'s allowed subtrees and declare every agent-visible file. + + Raises :class:`PluginBundleError` on a symlink escaping the source root or + on hidden grading material (``HIDDEN_MATERIAL_FILE_PATTERNS`` / + ``HIDDEN_MATERIAL_DIR_NAMES``) inside an allowed subtree. + """ + files: dict[str, str] = {} + symlinks: dict[str, str] = {} + + def record(path: Path) -> None: + rel = path.relative_to(source).as_posix() + if path.is_symlink(): + symlinks[rel] = _check_symlink_in_root(path, source) + elif _is_hidden_material(path.name, is_dir=False): + raise PluginBundleError( + f"Plugin bundle build failed: hidden grading material inside an allowed subtree: {rel} " + + f"(patterns: {', '.join(HIDDEN_MATERIAL_FILE_PATTERNS)})" + ) + else: + files[rel] = _hash_file(path) + + for name in sorted(PLUGIN_AGENT_ALLOWED_SUBDIRS): + top = source / name + if top.is_symlink() or top.is_file(): + # .claude-plugin can be a file (plugin manifest) in some layouts; + # a symlinked top entry is recorded verbatim, never walked. + record(top) + continue + if not top.is_dir(): + continue + for root_str, dirnames, filenames in os.walk(top): # followlinks=False: never descend through links + root = Path(root_str) + for dirname in sorted(dirnames): + child = root / dirname + if _is_hidden_material(dirname, is_dir=True): + rel = child.relative_to(source).as_posix() + raise PluginBundleError( + f"Plugin bundle build failed: hidden grading directory inside an allowed subtree: {rel}/" + ) + if child.is_symlink(): + # Listed but never descended into by os.walk; record verbatim. + symlinks[child.relative_to(source).as_posix()] = _check_symlink_in_root(child, source) + for filename in sorted(filenames): + record(root / filename) + + return BundleManifest( + source=str(source), + files=files, + symlinks=symlinks, + digest=_compute_digest(files, symlinks), + ) + + +def manifest_path_for(bundle_dir: Path) -> Path: + """The manifest lives NEXT TO the bundle dir, never inside it — the bundle + contains only projected plugin content.""" + return bundle_dir.with_name(bundle_dir.name + MANIFEST_SUFFIX) + + +def stage_bundle(source: Path, bundle_dir: Path) -> BundleManifest: + """Build the manifest for ``source``, copy the declared files into + ``bundle_dir``, record the manifest + digest, and self-verify. + + Symlinks are recreated verbatim (not followed). The post-copy + :func:`verify_bundle` self-check guarantees the staged tree matches the + manifest exactly — an undeclared file in the bundle fails the build. + """ + manifest = build_manifest(source) + bundle_dir.mkdir(parents=True, exist_ok=True) + try: + for rel in manifest.files: + src = source / rel + dst = bundle_dir / rel + dst.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(src, dst) + for rel, target in manifest.symlinks.items(): + src = source / rel + dst = bundle_dir / rel + dst.parent.mkdir(parents=True, exist_ok=True) + os.symlink(target, dst, target_is_directory=src.is_dir()) + except OSError as exc: + raise PluginBundleError(f"Plugin bundle build failed copying {source} -> {bundle_dir}: {exc}") from exc + + manifest_path_for(bundle_dir).write_text( + json.dumps( + { + "source": manifest.source, + "files": manifest.files, + "symlinks": manifest.symlinks, + "digest": manifest.digest, + }, + indent=2, + sort_keys=True, + ), + encoding="utf-8", + ) + verify_bundle(bundle_dir) # post-build self-check: staged tree == manifest, digest intact + return manifest + + +def verify_bundle(bundle_dir: Path) -> BundleManifest: + """Verify the staged bundle matches its recorded manifest digest; fail closed. + + Re-hashes every file in the bundle and cross-checks BOTH directions + against the manifest: a tampered file, an undeclared (added) file, a + missing declared file, a changed symlink target, or a manifest whose own + digest does not match its maps all raise :class:`PluginBundleError`. + """ + mpath = manifest_path_for(bundle_dir) + try: + raw = json.loads(mpath.read_text(encoding="utf-8")) + manifest = BundleManifest( + source=raw["source"], files=raw["files"], symlinks=raw["symlinks"], digest=raw["digest"] + ) + except (OSError, KeyError, TypeError, json.JSONDecodeError) as exc: + raise PluginBundleError(f"Plugin bundle manifest unreadable at {mpath}: {exc}") from exc + + if _compute_digest(manifest.files, manifest.symlinks) != manifest.digest: + raise PluginBundleError(f"Plugin bundle manifest at {mpath} fails its own digest; refusing to run") + + actual_files: dict[str, str] = {} + actual_symlinks: dict[str, str] = {} + for root_str, dirnames, filenames in os.walk(bundle_dir): # followlinks=False + root = Path(root_str) + for dirname in list(dirnames): + child = root / dirname + if child.is_symlink(): + actual_symlinks[child.relative_to(bundle_dir).as_posix()] = os.readlink(child) + for filename in filenames: + child = root / filename + rel = child.relative_to(bundle_dir).as_posix() + if child.is_symlink(): + actual_symlinks[rel] = os.readlink(child) + else: + actual_files[rel] = _hash_file(child) + + if actual_files != manifest.files or actual_symlinks != manifest.symlinks: + missing = sorted(set(manifest.files) - set(actual_files))[:5] + undeclared = sorted(set(actual_files) - set(manifest.files))[:5] + changed = sorted(k for k in set(actual_files) & set(manifest.files) if actual_files[k] != manifest.files[k])[:5] + link_drift = actual_symlinks != manifest.symlinks + raise PluginBundleError( + f"Plugin bundle at {bundle_dir} does not match its manifest digest " + + f"(missing={missing}, undeclared={undeclared}, changed={changed}, symlink_drift={link_drift}); " + + "the bundle drifted since staging — aborting instead of falling back to the raw plugin path" + ) + return manifest + + +# --- once-per-run staging cache ------------------------------------------- +# +# One bundle per resolved source path per process. run_batch executes tasks +# concurrently (asyncio + to_thread), so the build is serialized under a +# process-wide lock; cache hits return the already-staged dir and re-verify. +_STAGE_LOCK = threading.Lock() +_BUNDLE_CACHE: dict[str, Path] = {} +_STAGING_ROOT: Path | None = None + + +def _staging_root() -> Path: + global _STAGING_ROOT + if _STAGING_ROOT is None: + root = Path(tempfile.mkdtemp(prefix="coder-eval-plugin-bundles-")) + atexit.register(shutil.rmtree, root, ignore_errors=True) + _STAGING_ROOT = root + return _STAGING_ROOT + + +def _get_or_build_bundle(source: Path, log: logging.Logger | logging.LoggerAdapter[Any]) -> Path: + key = str(source) + with _STAGE_LOCK: + cached = _BUNDLE_CACHE.get(key) + if cached is not None: + return cached + bundle_dir = _staging_root() / f"{source.name}-{hashlib.sha256(key.encode('utf-8')).hexdigest()[:8]}" + manifest = stage_bundle(source, bundle_dir) + if not manifest.files and not manifest.symlinks: + log.warning( + "Plugin source %s contains none of the allowed subtrees %s; the agent sees an EMPTY plugin " + + "bundle (nothing leaks, but no skills will be discovered — check the plugin path)", + source, + sorted(PLUGIN_AGENT_ALLOWED_SUBDIRS), + ) + else: + log.info( + "Staged agent plugin bundle %s -> %s (%d files, %d symlinks, digest %s)", + source, + bundle_dir, + len(manifest.files), + len(manifest.symlinks), + manifest.digest[:12], + ) + _BUNDLE_CACHE[key] = bundle_dir + return bundle_dir + + +def stage_agent_plugins( + plugins: list[dict[str, Any]], + *, + log: logging.Logger | logging.LoggerAdapter[Any] = logger, +) -> tuple[list[dict[str, Any]], dict[str, str]]: + """Rewrite local plugin entries to point at verified agent-visible bundles. + + Env vars in each ``path`` are expanded first (same semantics/warnings as + the agents' own :func:`coder_eval.utils.process_plugins` pass — which then + no-ops on the already-absolute bundle path). Entries whose path does not + resolve to an existing directory pass through unchanged; the agents + already warn loudly about those. Every returned bundle — cache hit or + fresh build — is digest-verified here, immediately before the agent + starts. + + Returns ``(staged_plugins, digests)`` where ``digests`` maps each raw + source path to its manifest digest (recorded on the run for audit). + """ + staged: list[dict[str, Any]] = [] + digests: dict[str, str] = {} + for plugin in process_plugins(plugins, log=log): + path = plugin.get("path") + if not path or not Path(path).is_dir(): + staged.append(plugin) + continue + source = Path(path) + bundle_dir = _get_or_build_bundle(source, log) + manifest = verify_bundle(bundle_dir) + digests[str(source)] = manifest.digest + staged.append({**plugin, "path": str(bundle_dir)}) + return staged, digests diff --git a/src/coder_eval/utils.py b/src/coder_eval/utils.py index 1624dae6..16c0b3b0 100644 --- a/src/coder_eval/utils.py +++ b/src/coder_eval/utils.py @@ -86,6 +86,32 @@ def process_plugins( return processed +# Harness-internal env vars that must never reach the agent-under-test's +# subprocess: SKILLS_REPO_PATH points at the raw skills checkout (which carries +# grading material), and CODER_EVAL_* are harness knobs the agent has no +# business reading. The grading side (Sandbox._build_run_command_env) and the +# harness process itself deliberately KEEP these — run_command criteria invoke +# graders via `$SKILLS_REPO_PATH/...`. +AGENT_ENV_SCRUB_VARS: tuple[str, ...] = ("SKILLS_REPO_PATH",) +AGENT_ENV_SCRUB_PREFIXES: tuple[str, ...] = ("CODER_EVAL_",) + + +def scrub_agent_env_overrides() -> dict[str, str]: + """Empty-string overrides masking harness-internal env vars from the agent. + + Agent SDKs spawn their CLI subprocess with ``{**os.environ, **overrides}`` + (verified against claude_agent_sdk's subprocess transport), so a variable + can only be removed from the agent's view by explicitly overriding it — + omitting it from the overrides dict leaves the inherited value intact. + ``AGENT_ENV_SCRUB_VARS`` are always masked (deterministic contract); + ``AGENT_ENV_SCRUB_PREFIXES`` mask whatever matching vars this process + currently carries. + """ + scrub = dict.fromkeys(AGENT_ENV_SCRUB_VARS, "") + scrub.update({k: "" for k in os.environ if k.startswith(AGENT_ENV_SCRUB_PREFIXES)}) + return scrub + + SKIP = object() # Sentinel marking values that serialize_value should drop from the result. diff --git a/tests/lint/rules/ce034_plugin_bundle_choke_point.py b/tests/lint/rules/ce034_plugin_bundle_choke_point.py new file mode 100644 index 00000000..44d3303e --- /dev/null +++ b/tests/lint/rules/ce034_plugin_bundle_choke_point.py @@ -0,0 +1,75 @@ +"""CE034: agents must only ever see staged plugin bundles, never raw plugin paths. + +The orchestrator's `_setup` is the single choke point where `agent.plugins[].path` +is rewritten from the raw source checkout (which carries graders, RESOLUTION.md +reference answers, and test fixtures) to a verified file-allowlisted bundle +(coder_eval.plugin_bundle.stage_agent_plugins). Every agent construction site in +the orchestrator sits downstream of that rewrite, so a future edit that removes +the staging call — or moves `_create_agent()` above it — silently hands the agent +a pointer to its own answer key again. + +This rule guards the choke point (mirroring CE033's container_perms guard on the +docker branch): in `coder_eval/orchestrator.py`, any method that calls +`self._create_agent()` must call `self._stage_agent_plugin_bundles()` first +(earlier in the same method body). It deliberately does NOT try to prove global +dataflow — new out-of-orchestrator agent-construction paths must route through +the same staging seam and extend this rule. +""" + +import ast +import re + +from tests.lint.rules.base import BaseRule + + +_ORCHESTRATOR_FILE = re.compile(r"[/\\]coder_eval[/\\]orchestrator\.py$") + +_STAGE_CALL = "_stage_agent_plugin_bundles" +_CREATE_CALL = "_create_agent" + + +def _self_method_calls(func: ast.AsyncFunctionDef | ast.FunctionDef, name: str) -> list[ast.Call]: + """All `self.(...)` calls inside ``func`` (including awaited ones).""" + calls: list[ast.Call] = [] + for node in ast.walk(func): + if ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Attribute) + and node.func.attr == name + and isinstance(node.func.value, ast.Name) + and node.func.value.id == "self" + ): + calls.append(node) + return calls + + +class PluginBundleChokePoint(BaseRule): + id = "CE034" + + def __init__(self, filepath: str) -> None: + super().__init__(filepath) + self._applies = bool(_ORCHESTRATOR_FILE.search(filepath)) + + def _check_method(self, node: ast.AsyncFunctionDef | ast.FunctionDef) -> None: + creates = _self_method_calls(node, _CREATE_CALL) + if not creates: + return + stages = _self_method_calls(node, _STAGE_CALL) + first_create = min(c.lineno for c in creates) + if not stages or min(s.lineno for s in stages) > first_create: + self.violation( + creates[0], + f"self.{_CREATE_CALL}() without a preceding self.{_STAGE_CALL}() in the same method: " + "the agent would receive raw plugin paths (graders / RESOLUTION.md answer key) instead of " + "the verified bundle. Stage plugins through coder_eval.plugin_bundle before creating the agent.", + ) + + def visit_FunctionDef(self, node: ast.FunctionDef) -> None: + if self._applies: + self._check_method(node) + self.generic_visit(node) + + def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: + if self._applies: + self._check_method(node) + self.generic_visit(node) diff --git a/tests/lint/runner.py b/tests/lint/runner.py index e360b8ec..2e777460 100644 --- a/tests/lint/runner.py +++ b/tests/lint/runner.py @@ -20,6 +20,7 @@ from tests.lint.rules.ce022_dialog_loop_statement_cap import SimulationDialogLoopStatementCap from tests.lint.rules.ce023_no_proxy_shim_import import NoProxyShimImports from tests.lint.rules.ce024_discriminated_unions import DiscriminatedUnions +from tests.lint.rules.ce034_plugin_bundle_choke_point import PluginBundleChokePoint from tests.lint.rules.no_agent_timing_access import NoAgentTimingAccess from tests.lint.rules.no_blocking_io_in_async import NoBlockingIoInAsync from tests.lint.rules.no_cli_imports_in_core import NoCliImportsInCore @@ -63,6 +64,7 @@ SimulationDialogLoopStatementCap, NoProxyShimImports, DiscriminatedUnions, + PluginBundleChokePoint, ] # Anti-shadow invariant (mirrors AgentRegistry / register_pricing): every CE rule diff --git a/tests/test_antigravity_agent.py b/tests/test_antigravity_agent.py index 99747950..4007f9bb 100644 --- a/tests/test_antigravity_agent.py +++ b/tests/test_antigravity_agent.py @@ -558,6 +558,34 @@ async def test_harness_spawn_guard_restores_absent_path(monkeypatch): assert "PATH" not in antigravity_agent.os.environ +async def test_harness_spawn_guard_scrubs_harness_vars_for_spawn(monkeypatch): + """SKILLS_REPO_PATH / CODER_EVAL_* are absent from os.environ during the spawn + window (the localharness inherits os.environ wholesale — no env seam) and + restored for the harness process afterwards.""" + monkeypatch.setenv("SKILLS_REPO_PATH", "/host/skills") + monkeypatch.setenv("CODER_EVAL_DEBUG", "1") + agent = AntigravityAgent(parse_agent_config(type="antigravity")) + + async with agent._harness_spawn_guard(): + assert "SKILLS_REPO_PATH" not in os.environ + assert "CODER_EVAL_DEBUG" not in os.environ + assert os.environ["SKILLS_REPO_PATH"] == "/host/skills" # restored for the harness itself + assert os.environ["CODER_EVAL_DEBUG"] == "1" + + +async def test_harness_spawn_guard_restores_scrubbed_vars_when_body_raises(monkeypatch): + """The scrub restore lives in ``finally`` — a failed harness boot must not + leave the harness process without its own SKILLS_REPO_PATH (grading needs it).""" + monkeypatch.setenv("SKILLS_REPO_PATH", "/host/skills") + agent = AntigravityAgent(parse_agent_config(type="antigravity")) + + with pytest.raises(RuntimeError, match="harness boot failed"): + async with agent._harness_spawn_guard(): + assert "SKILLS_REPO_PATH" not in os.environ + raise RuntimeError("harness boot failed") + assert os.environ["SKILLS_REPO_PATH"] == "/host/skills" + + async def test_harness_spawn_guard_restores_path_when_body_raises(monkeypatch): """PATH is restored even when the guarded spawn raises (the failed-boot path). diff --git a/tests/test_codex_agent.py b/tests/test_codex_agent.py index 95ee117f..962bc22a 100644 --- a/tests/test_codex_agent.py +++ b/tests/test_codex_agent.py @@ -125,16 +125,26 @@ def test_sandbox_is_full_access(self, monkeypatch, mode, in_container, os_name): assert agent._build_thread_options()["sandbox"] == Sandbox("full-access") +def _without_scrub(env: dict[str, str] | None) -> dict[str, str]: + """Drop the always-present harness-env scrub entries (SKILLS_REPO_PATH, + ambient CODER_EVAL_*) so the remaining assertions stay exact-equality.""" + assert env is not None + return {k: v for k, v in env.items() if k != "SKILLS_REPO_PATH" and not k.startswith("CODER_EVAL_")} + + class TestCodexEnvironmentConfiguration: - """Test _build_codex_env: only CODEX_API_KEY travels via env.""" + """Test _build_codex_env: only CODEX_API_KEY (plus the harness scrub) travels via env.""" - def test_build_codex_env_returns_none_without_key(self, monkeypatch): - """No CODEX_API_KEY -> None (base URL alone is not enough).""" + def test_build_codex_env_scrub_only_without_key(self, monkeypatch): + """No CODEX_API_KEY -> only the harness scrub travels (base URL alone adds nothing).""" monkeypatch.delenv("CODEX_API_KEY", raising=False) monkeypatch.setenv("CODEX_BASE_URL", "https://custom.api/v1") agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX)) - assert agent._build_codex_env() is None + env = agent._build_codex_env() + assert env is not None + assert env["SKILLS_REPO_PATH"] == "" + assert _without_scrub(env) == {} def test_build_codex_env_with_api_key(self, monkeypatch): """CODEX_API_KEY is delivered via env.""" @@ -142,7 +152,7 @@ def test_build_codex_env_with_api_key(self, monkeypatch): agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX)) env = agent._build_codex_env() - assert env == {"CODEX_API_KEY": "test-key-123"} + assert _without_scrub(env) == {"CODEX_API_KEY": "test-key-123"} def test_build_codex_env_omits_base_url(self, monkeypatch): """Base URL is applied via provider config, never via env.""" @@ -151,8 +161,7 @@ def test_build_codex_env_omits_base_url(self, monkeypatch): agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX)) env = agent._build_codex_env() - assert env == {"CODEX_API_KEY": "k"} - assert "CODEX_BASE_URL" not in env + assert _without_scrub(env) == {"CODEX_API_KEY": "k"} def test_build_codex_env_ignores_openai_and_azure_keys(self, monkeypatch): """Only CODEX_API_KEY is read; OPENAI_*/AZURE_* are not.""" @@ -161,7 +170,20 @@ def test_build_codex_env_ignores_openai_and_azure_keys(self, monkeypatch): monkeypatch.setenv("AZURE_OPENAI_API_KEY", "azure-key") agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX)) - assert agent._build_codex_env() is None + assert _without_scrub(agent._build_codex_env()) == {} + + def test_build_codex_env_masks_harness_vars(self, monkeypatch): + """SKILLS_REPO_PATH / CODER_EVAL_* are explicitly overridden to '' — the + Codex SDK merges this dict over os.environ, so omission would inherit.""" + monkeypatch.delenv("CODEX_API_KEY", raising=False) + monkeypatch.setenv("SKILLS_REPO_PATH", "/host/skills") + monkeypatch.setenv("CODER_EVAL_DEBUG", "1") + + agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX)) + env = agent._build_codex_env() + assert env is not None + assert env["SKILLS_REPO_PATH"] == "" + assert env["CODER_EVAL_DEBUG"] == "" def test_build_codex_env_prepends_path_when_env_path_prepend_set(self, monkeypatch): """env_path_prepend dirs land at the FRONT of PATH, in order, parent appended. @@ -183,7 +205,7 @@ def test_build_codex_env_prepends_path_when_env_path_prepend_set(self, monkeypat assert env["PATH"] == f"/sandbox/mocks{os.pathsep}/sandbox/bins{os.pathsep}/parent/bin" def test_build_codex_env_returns_path_only_when_no_api_key(self, monkeypatch): - """Prepend dirs set but no CODEX_API_KEY -> {"PATH": ...} alone, not None.""" + """Prepend dirs set but no CODEX_API_KEY -> {"PATH": ...} alone (plus scrub).""" import os monkeypatch.delenv("CODEX_API_KEY", raising=False) @@ -193,7 +215,7 @@ def test_build_codex_env_returns_path_only_when_no_api_key(self, monkeypatch): agent._env_path_prepend = ["/sandbox/mocks"] env = agent._build_codex_env() - assert env == {"PATH": f"/sandbox/mocks{os.pathsep}/parent/bin"} + assert _without_scrub(env) == {"PATH": f"/sandbox/mocks{os.pathsep}/parent/bin"} def test_build_codex_env_no_prepend_omits_path(self, monkeypatch): """Default (no env_path_prepend) never adds a PATH key — only the API key travels.""" @@ -202,8 +224,8 @@ def test_build_codex_env_no_prepend_omits_path(self, monkeypatch): agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX)) env = agent._build_codex_env() - assert env == {"CODEX_API_KEY": "k"} - assert "PATH" not in env + assert _without_scrub(env) == {"CODEX_API_KEY": "k"} + assert env is not None and "PATH" not in env def test_build_codex_env_resolves_path_key_case_insensitively(self, monkeypatch): """A non-uppercase PATH key (e.g. Windows 'Path') is reused, not duplicated.""" @@ -217,7 +239,7 @@ def test_build_codex_env_resolves_path_key_case_insensitively(self, monkeypatch) agent._env_path_prepend = ["/sandbox/mocks"] env = agent._build_codex_env() - assert env == {"Path": f"/sandbox/mocks{_os.pathsep}/parent/bin"} + assert _without_scrub(env) == {"Path": f"/sandbox/mocks{_os.pathsep}/parent/bin"} @pytest.mark.asyncio async def test_start_propagates_env_path_prepend(self, monkeypatch, tmp_path): diff --git a/tests/test_custom_lint.py b/tests/test_custom_lint.py index fe9ab6cc..f4071893 100644 --- a/tests/test_custom_lint.py +++ b/tests/test_custom_lint.py @@ -530,6 +530,51 @@ def test_flags_annassign_union(self): assert self._run(self._TAGGED_CLASSES + "X: object = A | B") +@pytest.mark.lint +class TestCE034PluginBundleChokePoint: + """CE034: in orchestrator.py, self._create_agent() requires a preceding + self._stage_agent_plugin_bundles() in the same method (agents must never + receive raw plugin paths).""" + + @staticmethod + def _run(src: str, *, path: str = "src/coder_eval/orchestrator.py"): + import ast + + from tests.lint.rules.ce034_plugin_bundle_choke_point import PluginBundleChokePoint + + return PluginBundleChokePoint(path).check(ast.parse(src)) + + def test_flags_create_agent_without_staging(self): + src = "class O:\n async def _setup(self):\n self.agent = await self._create_agent()" + assert self._run(src) + + def test_flags_create_agent_before_staging(self): + src = ( + "class O:\n" + " async def _setup(self):\n" + " self.agent = await self._create_agent()\n" + " await self._stage_agent_plugin_bundles()" + ) + assert self._run(src) + + def test_allows_staging_before_create_agent(self): + src = ( + "class O:\n" + " async def _setup(self):\n" + " await self._stage_agent_plugin_bundles()\n" + " self.agent = await self._create_agent()" + ) + assert not self._run(src) + + def test_ignores_methods_without_create_agent(self): + src = "class O:\n async def _setup(self):\n await self._run_pre_run_commands()" + assert not self._run(src) + + def test_ignores_files_other_than_orchestrator(self): + src = "class O:\n async def _setup(self):\n self.agent = await self._create_agent()" + assert not self._run(src, path="src/coder_eval/evaluation/sub_agent.py") + + @pytest.mark.lint class TestCE025LiveVerdictConsistency: """CE025: a criterion type's ``LiveSuccessCriterion`` subclassing (models/criteria.py) diff --git a/tests/test_plugin_bundle.py b/tests/test_plugin_bundle.py new file mode 100644 index 00000000..d3838c47 --- /dev/null +++ b/tests/test_plugin_bundle.py @@ -0,0 +1,357 @@ +"""Tests for the agent-visible plugin bundle (file-level allowlist + digest verification). + +Covers the tempdir-driver answer-key-leak fix: the agent's ``plugins[].path`` +must point at a verified bundle carrying only plugin-discovery content — never +the raw skills checkout with its ``RESOLUTION.md`` answers, ``check_*.py`` +graders, and ``tests/`` fixtures — while grading (``run_command`` criteria via +``$SKILLS_REPO_PATH``) keeps the raw path. +""" + +import os +from pathlib import Path + +import pytest + +from coder_eval import plugin_bundle +from coder_eval.plugin_bundle import ( + PluginBundleError, + build_manifest, + manifest_path_for, + stage_agent_plugins, + stage_bundle, + verify_bundle, +) + + +# Symlink creation on Windows requires either admin privileges or Developer +# Mode enabled; CI runners usually have neither. Mark the tests that rely on +# os.symlink so they skip cleanly there. +_SKIP_NO_SYMLINK = pytest.mark.skipif( + os.name == "nt", + reason="Symlink creation on Windows requires admin or Developer Mode; not asserted in CI.", +) + + +@pytest.fixture(autouse=True) +def _isolated_staging(tmp_path: Path, monkeypatch): + """Fresh per-test bundle cache + staging root (module-level state otherwise leaks).""" + monkeypatch.setattr(plugin_bundle, "_BUNDLE_CACHE", {}) + monkeypatch.setattr(plugin_bundle, "_STAGING_ROOT", tmp_path / "bundle-staging") + + +def _make_skills_repo(root: Path) -> Path: + """A miniature skills checkout: plugin content PLUS its own answer key.""" + repo = root / "skills-repo" + (repo / "skills" / "uipath-troubleshoot" / "references").mkdir(parents=True) + (repo / "skills" / "uipath-troubleshoot" / "SKILL.md").write_text("# troubleshoot", encoding="utf-8") + (repo / "skills" / "uipath-troubleshoot" / "references" / "guide.md").write_text("guide", encoding="utf-8") + (repo / "commands").mkdir() + (repo / "commands" / "triage.md").write_text("cmd", encoding="utf-8") + (repo / ".claude-plugin").mkdir() + (repo / ".claude-plugin" / "marketplace.json").write_text("{}", encoding="utf-8") + # Grading material that must NEVER reach the agent: + (repo / "tests" / "tasks" / "t1").mkdir(parents=True) + (repo / "tests" / "tasks" / "t1" / "check_result.py").write_text("assert True", encoding="utf-8") + (repo / "tests" / "tasks" / "t1" / "RESOLUTION.md").write_text("the answer", encoding="utf-8") + (repo / "reference_agents").mkdir() + (repo / "reference_agents" / "golden.py").write_text("golden", encoding="utf-8") + (repo / "README.md").write_text("readme", encoding="utf-8") + return repo + + +def _bundle_rel_paths(bundle_dir: Path) -> set[str]: + return {p.relative_to(bundle_dir).as_posix() for p in bundle_dir.rglob("*") if not p.is_dir()} + + +class TestAdversarialProjection: + """Tempdir flavor of PR #85's adversarial probe: no graded material in the bundle.""" + + def test_bundle_contains_only_allowed_subtrees(self, tmp_path: Path): + repo = _make_skills_repo(tmp_path) + bundle = tmp_path / "bundle" + stage_bundle(repo, bundle) + assert _bundle_rel_paths(bundle) == { + "skills/uipath-troubleshoot/SKILL.md", + "skills/uipath-troubleshoot/references/guide.md", + "commands/triage.md", + ".claude-plugin/marketplace.json", + } + + def test_bundle_has_no_answer_key_material(self, tmp_path: Path): + """The adversarial assertion: nothing an agent could grade itself with.""" + repo = _make_skills_repo(tmp_path) + bundle = tmp_path / "bundle" + stage_bundle(repo, bundle) + for path in bundle.rglob("*"): + rel = path.relative_to(bundle).as_posix().lower() + assert "tests/" not in rel and not rel.startswith("tests") + assert "resolution.md" not in rel + assert not (path.name.startswith("check_") and path.suffix == ".py") + assert "reference_agents" not in rel + + def test_empty_source_yields_empty_bundle_loudly(self, tmp_path: Path, caplog): + repo = tmp_path / "no-plugin-content" + (repo / "tests").mkdir(parents=True) + (repo / "README.md").write_text("x", encoding="utf-8") + with caplog.at_level("WARNING"): + staged, digests = stage_agent_plugins([{"type": "local", "path": str(repo)}]) + bundle_dir = Path(staged[0]["path"]) + assert bundle_dir != repo.resolve() + assert _bundle_rel_paths(bundle_dir) == set() + assert str(repo.resolve()) in digests + assert any("EMPTY plugin bundle" in r.message for r in caplog.records) + + +class TestHiddenMaterialInsideAllowedSubtrees: + """A file-level allowlist, not a subdir allowlist: grading material filed + under skills/ fails the BUILD loudly instead of shipping.""" + + def test_resolution_md_inside_skills_fails_build(self, tmp_path: Path): + repo = _make_skills_repo(tmp_path) + (repo / "skills" / "uipath-troubleshoot" / "RESOLUTION.md").write_text("leak", encoding="utf-8") + with pytest.raises(PluginBundleError, match="hidden grading material"): + build_manifest(repo) + + def test_check_script_inside_skills_fails_build(self, tmp_path: Path): + repo = _make_skills_repo(tmp_path) + (repo / "skills" / "uipath-troubleshoot" / "check_output.py").write_text("leak", encoding="utf-8") + with pytest.raises(PluginBundleError, match="hidden grading material"): + build_manifest(repo) + + def test_case_insensitive_match(self, tmp_path: Path): + repo = _make_skills_repo(tmp_path) + (repo / "skills" / "uipath-troubleshoot" / "Resolution.MD").write_text("leak", encoding="utf-8") + with pytest.raises(PluginBundleError, match="hidden grading material"): + build_manifest(repo) + + def test_tests_dir_inside_skills_fails_build(self, tmp_path: Path): + repo = _make_skills_repo(tmp_path) + (repo / "skills" / "uipath-troubleshoot" / "tests").mkdir() + with pytest.raises(PluginBundleError, match="hidden grading directory"): + build_manifest(repo) + + +class TestSymlinkSafety: + @_SKIP_NO_SYMLINK + def test_symlink_escaping_source_root_rejected(self, tmp_path: Path): + repo = _make_skills_repo(tmp_path) + outside = tmp_path / "outside.txt" + outside.write_text("secret", encoding="utf-8") + os.symlink(outside, repo / "skills" / "uipath-troubleshoot" / "leak.txt") + with pytest.raises(PluginBundleError, match="escapes the source root"): + build_manifest(repo) + + @_SKIP_NO_SYMLINK + def test_in_root_symlink_copied_verbatim(self, tmp_path: Path): + repo = _make_skills_repo(tmp_path) + os.symlink("SKILL.md", repo / "skills" / "uipath-troubleshoot" / "alias.md") + bundle = tmp_path / "bundle" + manifest = stage_bundle(repo, bundle) + link = bundle / "skills" / "uipath-troubleshoot" / "alias.md" + assert link.is_symlink() + assert os.readlink(link) == "SKILL.md" + assert manifest.symlinks == {"skills/uipath-troubleshoot/alias.md": "SKILL.md"} + + @_SKIP_NO_SYMLINK + def test_self_referential_symlink_is_loop_proof(self, tmp_path: Path): + """Marketplace-style `skills/loop -> ..` must neither recurse nor escape.""" + repo = _make_skills_repo(tmp_path) + os.symlink("..", repo / "skills" / "loop") + bundle = tmp_path / "bundle" + manifest = stage_bundle(repo, bundle) + assert manifest.symlinks == {"skills/loop": ".."} + assert (bundle / "skills" / "loop").is_symlink() + + +class TestDigestVerification: + def _staged(self, tmp_path: Path) -> Path: + repo = _make_skills_repo(tmp_path) + bundle = tmp_path / "bundle" + stage_bundle(repo, bundle) + return bundle + + def test_manifest_recorded_next_to_bundle(self, tmp_path: Path): + bundle = self._staged(tmp_path) + mpath = manifest_path_for(bundle) + assert mpath.exists() + assert mpath.parent == bundle.parent # never inside the agent-visible tree + manifest = verify_bundle(bundle) + assert manifest.digest and len(manifest.digest) == 64 + + def test_tampered_file_fails_closed(self, tmp_path: Path): + bundle = self._staged(tmp_path) + (bundle / "skills" / "uipath-troubleshoot" / "SKILL.md").write_text("tampered", encoding="utf-8") + with pytest.raises(PluginBundleError, match="does not match its manifest digest"): + verify_bundle(bundle) + + def test_undeclared_file_fails_closed(self, tmp_path: Path): + bundle = self._staged(tmp_path) + (bundle / "skills" / "smuggled.md").write_text("extra", encoding="utf-8") + with pytest.raises(PluginBundleError, match="undeclared"): + verify_bundle(bundle) + + def test_missing_declared_file_fails_closed(self, tmp_path: Path): + bundle = self._staged(tmp_path) + (bundle / "commands" / "triage.md").unlink() + with pytest.raises(PluginBundleError, match="missing"): + verify_bundle(bundle) + + def test_edited_manifest_fails_its_own_digest(self, tmp_path: Path): + bundle = self._staged(tmp_path) + mpath = manifest_path_for(bundle) + mpath.write_text(mpath.read_text(encoding="utf-8").replace("troubleshoot", "troublesh00t"), encoding="utf-8") + with pytest.raises(PluginBundleError, match="fails its own digest"): + verify_bundle(bundle) + + +class TestStageAgentPlugins: + def test_rewrites_path_expands_env_var_and_records_digest(self, tmp_path: Path, monkeypatch): + repo = _make_skills_repo(tmp_path) + monkeypatch.setenv("SKILLS_REPO_PATH", str(repo)) + staged, digests = stage_agent_plugins([{"type": "local", "path": "$SKILLS_REPO_PATH"}]) + assert staged[0]["type"] == "local" + bundle_dir = Path(staged[0]["path"]) + assert bundle_dir != repo.resolve() + assert (bundle_dir / "skills" / "uipath-troubleshoot" / "SKILL.md").exists() + assert not (bundle_dir / "tests").exists() + assert digests == {str(repo.resolve()): verify_bundle(bundle_dir).digest} + + def test_bundle_built_once_per_source(self, tmp_path: Path, monkeypatch): + repo = _make_skills_repo(tmp_path) + calls: list[Path] = [] + real_stage = plugin_bundle.stage_bundle + + def counting_stage(source: Path, bundle_dir: Path): + calls.append(source) + return real_stage(source, bundle_dir) + + monkeypatch.setattr(plugin_bundle, "stage_bundle", counting_stage) + plugins = [{"type": "local", "path": str(repo)}] + first, _ = stage_agent_plugins(plugins) + second, _ = stage_agent_plugins(plugins) + assert len(calls) == 1 # once per run, not once per task + assert first[0]["path"] == second[0]["path"] + + def test_drifted_bundle_aborts_instead_of_falling_back(self, tmp_path: Path): + repo = _make_skills_repo(tmp_path) + plugins = [{"type": "local", "path": str(repo)}] + staged, _ = stage_agent_plugins(plugins) + bundle_dir = Path(staged[0]["path"]) + (bundle_dir / "skills" / "planted.md").write_text("drift", encoding="utf-8") + with pytest.raises(PluginBundleError, match="does not match its manifest digest"): + stage_agent_plugins(plugins) + + def test_nonexistent_path_passes_through_unchanged(self, tmp_path: Path, monkeypatch): + monkeypatch.delenv("NOT_A_REAL_SKILLS_VAR", raising=False) + plugins = [{"type": "local", "path": "$NOT_A_REAL_SKILLS_VAR/skills"}] + staged, digests = stage_agent_plugins(plugins) + assert digests == {} + # The agents' own loud missing-path warnings stay authoritative. + assert "NOT_A_REAL_SKILLS_VAR" in staged[0]["path"] + + def test_entry_without_path_passes_through(self): + staged, digests = stage_agent_plugins([{"type": "local"}]) + assert staged == [{"type": "local"}] + assert digests == {} + + +class TestOrchestratorSeam: + async def test_setup_seam_rewrites_task_agent_plugins(self, tmp_path: Path, monkeypatch): + """The orchestrator rewrites agent.plugins in place before agent creation.""" + from datetime import datetime + + from coder_eval.models import EvaluationResult, FinalStatus, TaskDefinition + from coder_eval.orchestrator import Orchestrator + + repo = _make_skills_repo(tmp_path) + monkeypatch.delenv("CODER_EVAL_IN_CONTAINER", raising=False) + task = TaskDefinition.model_validate( + { + "task_id": "bundle-seam", + "description": "d", + "initial_prompt": "p", + "agent": {"type": "claude-code", "plugins": [{"type": "local", "path": str(repo)}]}, + "success_criteria": [{"type": "file_exists", "description": "out exists", "path": "out.txt"}], + } + ) + orch = Orchestrator(task, run_dir=tmp_path / "run", variant_id="default") + orch.result = EvaluationResult( + task_id="bundle-seam", + task_description="d", + variant_id="default", + agent_type="claude-code", + started_at=datetime.now(), + final_status=FinalStatus.FAILURE, + iteration_count=0, + ) + await orch._stage_agent_plugin_bundles() + assert task.agent is not None and task.agent.plugins is not None + bundle_dir = Path(task.agent.plugins[0]["path"]) + assert bundle_dir != repo.resolve() + assert (bundle_dir / "skills" / "uipath-troubleshoot" / "SKILL.md").exists() + assert not (bundle_dir / "tests").exists() + assert orch.result.environment_info["plugin_bundles"] == {str(repo.resolve()): verify_bundle(bundle_dir).digest} + + async def test_setup_seam_skipped_in_container(self, tmp_path: Path, monkeypatch): + """In-container runs keep the docker driver's own staging surface (PR #85).""" + from coder_eval.models import TaskDefinition + from coder_eval.orchestrator import Orchestrator + + repo = _make_skills_repo(tmp_path) + monkeypatch.setenv("CODER_EVAL_IN_CONTAINER", "1") + task = TaskDefinition.model_validate( + { + "task_id": "bundle-seam-docker", + "description": "d", + "initial_prompt": "p", + "agent": {"type": "claude-code", "plugins": [{"type": "local", "path": str(repo)}]}, + "success_criteria": [{"type": "file_exists", "description": "out exists", "path": "out.txt"}], + } + ) + orch = Orchestrator(task, run_dir=tmp_path / "run", variant_id="default") + await orch._stage_agent_plugin_bundles() + assert task.agent is not None and task.agent.plugins is not None + assert task.agent.plugins[0]["path"] == str(repo) + + +class TestAgentEnvScrubVsGradingEnv: + """SKILLS_REPO_PATH is masked from every agent subprocess but stays fully + resolvable in the grading env (run_command criteria invoke + `python3 $SKILLS_REPO_PATH/tests/.../check_*.py`).""" + + def test_grading_env_still_resolves_skills_repo_path(self, tmp_path: Path, monkeypatch): + from coder_eval.models import SandboxConfig + from coder_eval.sandbox import Sandbox + + monkeypatch.setenv("SKILLS_REPO_PATH", str(tmp_path / "skills")) + sandbox = Sandbox(SandboxConfig(driver="tempdir", python=None), task_id="grading-env") + try: + sandbox.setup() + env = sandbox._build_run_command_env() + assert env["SKILLS_REPO_PATH"] == str(tmp_path / "skills") + finally: + sandbox.cleanup() + + def test_claude_sdk_env_masks_harness_vars(self, monkeypatch): + from coder_eval.agents.claude_code_agent import ClaudeCodeAgent + from coder_eval.models import DirectRoute + + monkeypatch.setenv("SKILLS_REPO_PATH", "/host/skills") + monkeypatch.setenv("CODER_EVAL_RAW_SDK_LOG", "1") + env, _ = ClaudeCodeAgent._build_sdk_env(DirectRoute()) + assert env["SKILLS_REPO_PATH"] == "" + assert env["CODER_EVAL_RAW_SDK_LOG"] == "" + + def test_scrub_overrides_are_explicit_not_omitted(self, monkeypatch): + """The SDK merges {**os.environ, **options.env}: an omitted key would + inherit; only an explicit empty override masks it.""" + from coder_eval.utils import scrub_agent_env_overrides + + monkeypatch.setenv("SKILLS_REPO_PATH", "/host/skills") + monkeypatch.setenv("CODER_EVAL_DEBUG", "1") + scrub = scrub_agent_env_overrides() + assert scrub["SKILLS_REPO_PATH"] == "" + assert scrub["CODER_EVAL_DEBUG"] == "" + # SKILLS_REPO_PATH masked even when unset in the parent (deterministic contract). + monkeypatch.delenv("SKILLS_REPO_PATH") + assert scrub_agent_env_overrides()["SKILLS_REPO_PATH"] == "" From e899597e4930e169fbbca229b141a73a91a758ff Mon Sep 17 00:00:00 2001 From: Dan Morosanu Date: Thu, 6 Aug 2026 13:56:39 +0300 Subject: [PATCH 2/4] fix(plugins): allow nested tests dirs in skill assets, keep file-name answer-key guards --- src/coder_eval/plugin_bundle.py | 25 +++++++-------- src/coder_eval/utils.py | 10 ++++++ tests/test_plugin_bundle.py | 57 +++++++++++++++++++++++++++++---- 3 files changed, 72 insertions(+), 20 deletions(-) diff --git a/src/coder_eval/plugin_bundle.py b/src/coder_eval/plugin_bundle.py index 64540b82..55b842b4 100644 --- a/src/coder_eval/plugin_bundle.py +++ b/src/coder_eval/plugin_bundle.py @@ -68,11 +68,17 @@ # projection in PR #85's plugin_projection module. PLUGIN_AGENT_ALLOWED_SUBDIRS = frozenset({"skills", "commands", "agents", ".claude-plugin", "hooks"}) -# Grading material recognizable by NAME even inside an allowed subtree. +# Grading material recognizable by FILE NAME even inside an allowed subtree. # Matched case-insensitively; a hit FAILS the build (loud, not a silent skip) # so answer-key material can never ship by being filed under skills/. +# +# Deliberately NO directory-name check: the graders' repo-root ``tests/`` tree +# is already excluded by construction (``tests`` is not an allowed subtree and +# is never walked), while a nested ``tests`` directory inside a skill is +# legitimately shipped client content (e.g. a dashboard scaffold's test folder +# under ``skills/uipath-coded-apps/assets/``) — a deep path-component check +# rejected the real skills repo outright. HIDDEN_MATERIAL_FILE_PATTERNS: tuple[str, ...] = ("resolution.md", "check_*.py") -HIDDEN_MATERIAL_DIR_NAMES: frozenset[str] = frozenset({"tests"}) MANIFEST_SUFFIX = ".manifest.json" @@ -110,10 +116,8 @@ def _hash_file(path: Path) -> str: return hasher.hexdigest() -def _is_hidden_material(name: str, *, is_dir: bool) -> bool: +def _is_hidden_material(name: str) -> bool: lowered = name.lower() - if is_dir: - return lowered in HIDDEN_MATERIAL_DIR_NAMES return any(fnmatchcase(lowered, pattern) for pattern in HIDDEN_MATERIAL_FILE_PATTERNS) @@ -139,8 +143,8 @@ def build_manifest(source: Path) -> BundleManifest: """Walk ``source``'s allowed subtrees and declare every agent-visible file. Raises :class:`PluginBundleError` on a symlink escaping the source root or - on hidden grading material (``HIDDEN_MATERIAL_FILE_PATTERNS`` / - ``HIDDEN_MATERIAL_DIR_NAMES``) inside an allowed subtree. + on hidden grading material (``HIDDEN_MATERIAL_FILE_PATTERNS``) inside an + allowed subtree. """ files: dict[str, str] = {} symlinks: dict[str, str] = {} @@ -149,7 +153,7 @@ def record(path: Path) -> None: rel = path.relative_to(source).as_posix() if path.is_symlink(): symlinks[rel] = _check_symlink_in_root(path, source) - elif _is_hidden_material(path.name, is_dir=False): + elif _is_hidden_material(path.name): raise PluginBundleError( f"Plugin bundle build failed: hidden grading material inside an allowed subtree: {rel} " + f"(patterns: {', '.join(HIDDEN_MATERIAL_FILE_PATTERNS)})" @@ -170,11 +174,6 @@ def record(path: Path) -> None: root = Path(root_str) for dirname in sorted(dirnames): child = root / dirname - if _is_hidden_material(dirname, is_dir=True): - rel = child.relative_to(source).as_posix() - raise PluginBundleError( - f"Plugin bundle build failed: hidden grading directory inside an allowed subtree: {rel}/" - ) if child.is_symlink(): # Listed but never descended into by os.walk; record verbatim. symlinks[child.relative_to(source).as_posix()] = _check_symlink_in_root(child, source) diff --git a/src/coder_eval/utils.py b/src/coder_eval/utils.py index 16c0b3b0..2f6103c2 100644 --- a/src/coder_eval/utils.py +++ b/src/coder_eval/utils.py @@ -106,6 +106,16 @@ def scrub_agent_env_overrides() -> dict[str, str]: ``AGENT_ENV_SCRUB_VARS`` are always masked (deterministic contract); ``AGENT_ENV_SCRUB_PREFIXES`` mask whatever matching vars this process currently carries. + + Empty-string, not unset, is load-bearing for the Claude/Codex seams: the + merge offers no removal semantics, and unsetting via a transient + ``os.environ`` mutation would be process-global — concurrent tasks copy + ``os.environ`` for their grading subprocesses (``_build_run_command_env``) + and could observe the gap. The tradeoff is benign: a shell expanding + ``$SKILLS_REPO_PATH`` yields ``""`` for a present-but-empty var exactly as + it does for an unset one, and no agent-side code reads the variable. + (Antigravity, which has no env seam and already serializes its harness + spawn under a lock, does truly unset — see ``_harness_spawn_guard``.) """ scrub = dict.fromkeys(AGENT_ENV_SCRUB_VARS, "") scrub.update({k: "" for k in os.environ if k.startswith(AGENT_ENV_SCRUB_PREFIXES)}) diff --git a/tests/test_plugin_bundle.py b/tests/test_plugin_bundle.py index d3838c47..0d9e6c6b 100644 --- a/tests/test_plugin_bundle.py +++ b/tests/test_plugin_bundle.py @@ -45,6 +45,13 @@ def _make_skills_repo(root: Path) -> Path: (repo / "skills" / "uipath-troubleshoot" / "references").mkdir(parents=True) (repo / "skills" / "uipath-troubleshoot" / "SKILL.md").write_text("# troubleshoot", encoding="utf-8") (repo / "skills" / "uipath-troubleshoot" / "references" / "guide.md").write_text("guide", encoding="utf-8") + # A nested `tests` dir inside a skill is LEGITIMATE shipped client content + # (mirrors skills/uipath-coded-apps/assets/scripts/dashboards/tests/ in the + # real repo) — it must project into the bundle, unlike the repo-root tests/. + (repo / "skills" / "uipath-coded-apps" / "assets" / "scripts" / "dashboards" / "tests").mkdir(parents=True) + (repo / "skills" / "uipath-coded-apps" / "assets" / "scripts" / "dashboards" / "tests" / "dash.test.ts").write_text( + "test", encoding="utf-8" + ) (repo / "commands").mkdir() (repo / "commands" / "triage.md").write_text("cmd", encoding="utf-8") (repo / ".claude-plugin").mkdir() @@ -73,6 +80,7 @@ def test_bundle_contains_only_allowed_subtrees(self, tmp_path: Path): assert _bundle_rel_paths(bundle) == { "skills/uipath-troubleshoot/SKILL.md", "skills/uipath-troubleshoot/references/guide.md", + "skills/uipath-coded-apps/assets/scripts/dashboards/tests/dash.test.ts", "commands/triage.md", ".claude-plugin/marketplace.json", } @@ -84,9 +92,12 @@ def test_bundle_has_no_answer_key_material(self, tmp_path: Path): stage_bundle(repo, bundle) for path in bundle.rglob("*"): rel = path.relative_to(bundle).as_posix().lower() - assert "tests/" not in rel and not rel.startswith("tests") - assert "resolution.md" not in rel - assert not (path.name.startswith("check_") and path.suffix == ".py") + assert not rel.startswith("tests/") # the repo-root grader tree + assert "tests/tasks" not in rel + # Exact-name semantics, matching the builder: a doc like + # reference-resolution.md is legitimate; the answer file is not. + assert path.name.lower() != "resolution.md" + assert not (path.name.lower().startswith("check_") and path.suffix == ".py") assert "reference_agents" not in rel def test_empty_source_yields_empty_bundle_loudly(self, tmp_path: Path, caplog): @@ -124,11 +135,19 @@ def test_case_insensitive_match(self, tmp_path: Path): with pytest.raises(PluginBundleError, match="hidden grading material"): build_manifest(repo) - def test_tests_dir_inside_skills_fails_build(self, tmp_path: Path): + def test_nested_tests_dir_inside_skill_is_allowed(self, tmp_path: Path): + """Regression lock: a skill's own `tests` folder is shipped client + content, NOT grading material — the real skills repo carries + skills/uipath-coded-apps/assets/scripts/dashboards/tests/ and a deep + `tests` path-component check rejected the whole repo. Grading material + lives at the repo-root tests/ tree, which the subtree allowlist already + excludes by construction. Do not reintroduce a directory-name check.""" repo = _make_skills_repo(tmp_path) - (repo / "skills" / "uipath-troubleshoot" / "tests").mkdir() - with pytest.raises(PluginBundleError, match="hidden grading directory"): - build_manifest(repo) + bundle = tmp_path / "bundle" + manifest = stage_bundle(repo, bundle) + nested = "skills/uipath-coded-apps/assets/scripts/dashboards/tests/dash.test.ts" + assert nested in manifest.files + assert (bundle / nested).exists() class TestSymlinkSafety: @@ -232,6 +251,30 @@ def counting_stage(source: Path, bundle_dir: Path): assert len(calls) == 1 # once per run, not once per task assert first[0]["path"] == second[0]["path"] + async def test_concurrent_staging_copies_exactly_once(self, tmp_path: Path, monkeypatch): + """run_batch executes tasks concurrently (asyncio + to_thread); N + concurrent stagings of the same source must produce exactly ONE copy — + a lock that merely serializes N copies would still be a regression at + suite scale (~297 tasks x a 17 MB skills checkout).""" + import asyncio + + repo = _make_skills_repo(tmp_path) + copies: list[Path] = [] + real_stage = plugin_bundle.stage_bundle + + def counting_stage(source: Path, bundle_dir: Path): + copies.append(bundle_dir) + return real_stage(source, bundle_dir) + + monkeypatch.setattr(plugin_bundle, "stage_bundle", counting_stage) + plugins = [{"type": "local", "path": str(repo)}] + results = await asyncio.gather( + *(asyncio.to_thread(stage_agent_plugins, plugins) for _ in range(8)), + ) + assert len(copies) == 1 # one copy total, not one per concurrent task + staged_paths = {staged[0]["path"] for staged, _ in results} + assert staged_paths == {str(copies[0])} + def test_drifted_bundle_aborts_instead_of_falling_back(self, tmp_path: Path): repo = _make_skills_repo(tmp_path) plugins = [{"type": "local", "path": str(repo)}] From 1960e816a97f7e2e5c72355cf4dda57a94d143bf Mon Sep 17 00:00:00 2001 From: Dan Morosanu Date: Thu, 6 Aug 2026 18:45:39 +0300 Subject: [PATCH 3/4] test(plugins): assert staged plugin path lives under the bundle staging tempdir --- tests/test_plugin_bundle.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_plugin_bundle.py b/tests/test_plugin_bundle.py index 0d9e6c6b..5c1497d4 100644 --- a/tests/test_plugin_bundle.py +++ b/tests/test_plugin_bundle.py @@ -231,6 +231,11 @@ def test_rewrites_path_expands_env_var_and_records_digest(self, tmp_path: Path, assert staged[0]["type"] == "local" bundle_dir = Path(staged[0]["path"]) assert bundle_dir != repo.resolve() + # The path handed to the agent lives under the bundle staging tempdir, + # not anywhere inside the source checkout. + assert plugin_bundle._STAGING_ROOT is not None + assert bundle_dir.is_relative_to(plugin_bundle._STAGING_ROOT) + assert not bundle_dir.is_relative_to(repo.resolve()) assert (bundle_dir / "skills" / "uipath-troubleshoot" / "SKILL.md").exists() assert not (bundle_dir / "tests").exists() assert digests == {str(repo.resolve()): verify_bundle(bundle_dir).digest} From ac64e02ba1aa5eee7f274e1c8ff171aa2a27d5df Mon Sep 17 00:00:00 2001 From: Dan Morosanu Date: Thu, 6 Aug 2026 18:51:02 +0300 Subject: [PATCH 4/4] docs(plugins): use neutral wording for plugin bundle exposure in log message and tests --- src/coder_eval/plugin_bundle.py | 2 +- tests/test_plugin_bundle.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coder_eval/plugin_bundle.py b/src/coder_eval/plugin_bundle.py index 55b842b4..1199ec77 100644 --- a/src/coder_eval/plugin_bundle.py +++ b/src/coder_eval/plugin_bundle.py @@ -314,7 +314,7 @@ def _get_or_build_bundle(source: Path, log: logging.Logger | logging.LoggerAdapt if not manifest.files and not manifest.symlinks: log.warning( "Plugin source %s contains none of the allowed subtrees %s; the agent sees an EMPTY plugin " - + "bundle (nothing leaks, but no skills will be discovered — check the plugin path)", + + "bundle - no skills will be discovered from this source (check the plugin path)", source, sorted(PLUGIN_AGENT_ALLOWED_SUBDIRS), ) diff --git a/tests/test_plugin_bundle.py b/tests/test_plugin_bundle.py index 5c1497d4..b40a8391 100644 --- a/tests/test_plugin_bundle.py +++ b/tests/test_plugin_bundle.py @@ -1,6 +1,6 @@ """Tests for the agent-visible plugin bundle (file-level allowlist + digest verification). -Covers the tempdir-driver answer-key-leak fix: the agent's ``plugins[].path`` +Covers the tempdir-driver answer-key exposure fix: the agent's ``plugins[].path`` must point at a verified bundle carrying only plugin-discovery content — never the raw skills checkout with its ``RESOLUTION.md`` answers, ``check_*.py`` graders, and ``tests/`` fixtures — while grading (``run_command`` criteria via @@ -34,7 +34,7 @@ @pytest.fixture(autouse=True) def _isolated_staging(tmp_path: Path, monkeypatch): - """Fresh per-test bundle cache + staging root (module-level state otherwise leaks).""" + """Fresh per-test bundle cache + staging root (module-level state otherwise carries over).""" monkeypatch.setattr(plugin_bundle, "_BUNDLE_CACHE", {}) monkeypatch.setattr(plugin_bundle, "_STAGING_ROOT", tmp_path / "bundle-staging") @@ -119,19 +119,19 @@ class TestHiddenMaterialInsideAllowedSubtrees: def test_resolution_md_inside_skills_fails_build(self, tmp_path: Path): repo = _make_skills_repo(tmp_path) - (repo / "skills" / "uipath-troubleshoot" / "RESOLUTION.md").write_text("leak", encoding="utf-8") + (repo / "skills" / "uipath-troubleshoot" / "RESOLUTION.md").write_text("the answer", encoding="utf-8") with pytest.raises(PluginBundleError, match="hidden grading material"): build_manifest(repo) def test_check_script_inside_skills_fails_build(self, tmp_path: Path): repo = _make_skills_repo(tmp_path) - (repo / "skills" / "uipath-troubleshoot" / "check_output.py").write_text("leak", encoding="utf-8") + (repo / "skills" / "uipath-troubleshoot" / "check_output.py").write_text("the answer", encoding="utf-8") with pytest.raises(PluginBundleError, match="hidden grading material"): build_manifest(repo) def test_case_insensitive_match(self, tmp_path: Path): repo = _make_skills_repo(tmp_path) - (repo / "skills" / "uipath-troubleshoot" / "Resolution.MD").write_text("leak", encoding="utf-8") + (repo / "skills" / "uipath-troubleshoot" / "Resolution.MD").write_text("the answer", encoding="utf-8") with pytest.raises(PluginBundleError, match="hidden grading material"): build_manifest(repo) @@ -156,7 +156,7 @@ def test_symlink_escaping_source_root_rejected(self, tmp_path: Path): repo = _make_skills_repo(tmp_path) outside = tmp_path / "outside.txt" outside.write_text("secret", encoding="utf-8") - os.symlink(outside, repo / "skills" / "uipath-troubleshoot" / "leak.txt") + os.symlink(outside, repo / "skills" / "uipath-troubleshoot" / "escaped.txt") with pytest.raises(PluginBundleError, match="escapes the source root"): build_manifest(repo)