diff --git a/manifests/context-packs/skill-triggers@1.json b/manifests/context-packs/skill-triggers@1.json index a563863c7..443ab5173 100644 --- a/manifests/context-packs/skill-triggers@1.json +++ b/manifests/context-packs/skill-triggers@1.json @@ -10,7 +10,7 @@ "id": "hand-seeded", "source": "manual-curation-pending-regeneration" }, - "registryDigest": "7249c1a4f4fc6673d3bf62d4800b995740a8a322b9d6cb62f1069c58e2ad5f26", + "registryDigest": "fc16661cec4110a3158718915eba7078799726f7905357db7b0395f63fc19692", "schemaVersion": 1, "triggers": { "skill:agent-runtime-gateway-smoke-test": [ diff --git a/skills/skill-comply/scripts/runner.py b/skills/skill-comply/scripts/runner.py index d638ad2e7..eeb8fea32 100644 --- a/skills/skill-comply/scripts/runner.py +++ b/skills/skill-comply/scripts/runner.py @@ -6,6 +6,7 @@ import json import re import shlex import shutil +import stat import subprocess from dataclasses import dataclass from pathlib import Path @@ -90,8 +91,26 @@ def run_scenario( ) +def _validate_sandbox_base() -> None: + """Reject links at the base itself; trusted parent aliases remain usable. + + This is a point-in-time preflight, not protection against concurrent + replacement of filesystem entries during setup. + """ + try: + metadata = SANDBOX_BASE.lstat() + except FileNotFoundError: + return + reparse_point = getattr(stat, "FILE_ATTRIBUTE_REPARSE_POINT", 0x400) + if stat.S_ISLNK(metadata.st_mode) or getattr(metadata, "st_file_attributes", 0) & reparse_point: + raise ValueError("Sandbox base must not be a symlink or directory reparse point") + if not stat.S_ISDIR(metadata.st_mode): + raise ValueError("Sandbox base must be a directory") + + def _safe_sandbox_dir(scenario_id: str) -> Path: """Sanitize scenario ID and ensure path stays within sandbox base.""" + _validate_sandbox_base() safe_id = re.sub(r"[^a-zA-Z0-9\-_]", "_", scenario_id) path = SANDBOX_BASE / safe_id resolved_path = path.resolve() @@ -105,6 +124,7 @@ def _safe_sandbox_dir(scenario_id: str) -> Path: def _setup_sandbox(sandbox_dir: Path, scenario: Scenario) -> None: """Create sandbox directory and run setup commands.""" + _validate_sandbox_base() if sandbox_dir.exists(): shutil.rmtree(sandbox_dir) sandbox_dir.mkdir(parents=True) diff --git a/skills/skill-comply/tests/test_runner.py b/skills/skill-comply/tests/test_runner.py index 94f6c4bdd..9c67a5468 100644 --- a/skills/skill-comply/tests/test_runner.py +++ b/skills/skill-comply/tests/test_runner.py @@ -3,9 +3,11 @@ from __future__ import annotations import json +import stat import subprocess from dataclasses import dataclass from pathlib import Path +from types import SimpleNamespace from unittest.mock import patch import pytest @@ -16,6 +18,7 @@ from scripts.runner import ( _setup_sandbox, run_scenario, ) +from scripts.scenario_generator import Scenario @dataclass(frozen=True) @@ -60,6 +63,136 @@ class TestSafeSandboxDirectory: _safe_sandbox_dir("escape") +class TestSandboxBaseBoundary: + def test_live_base_link_preserves_outside_files( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + outside = tmp_path / "outside" + target = outside / "probe" + target.mkdir(parents=True) + sentinel = target / "sentinel" + sentinel.write_bytes(b"outside must remain unchanged") + base = tmp_path / "base" + base.symlink_to(outside, target_is_directory=True) + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + with pytest.raises(ValueError, match="base"): + _safe_sandbox_dir("probe") + assert sentinel.read_bytes() == b"outside must remain unchanged" + assert not (target / ".git").exists() + + def test_setup_rechecks_base_before_cleanup( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + base = tmp_path / "base" + base.mkdir() + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + selected = _safe_sandbox_dir("probe") + outside = tmp_path / "outside" + target = outside / "probe" + target.mkdir(parents=True) + sentinel = target / "sentinel" + sentinel.write_bytes(b"outside must remain unchanged") + base.rmdir() + base.symlink_to(outside, target_is_directory=True) + with pytest.raises(ValueError, match="base"): + _setup_sandbox(selected, Scenario(id="probe", level=1, level_name="test", description="Disposable setup", prompt="No provider", setup_commands=())) + assert sentinel.read_bytes() == b"outside must remain unchanged" + assert not (target / ".git").exists() + + def test_public_scenario_rejects_base_link_before_subprocess( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + outside = tmp_path / "outside" + target = outside / "probe" + target.mkdir(parents=True) + sentinel = target / "sentinel" + sentinel.write_bytes(b"outside must remain unchanged") + base = tmp_path / "base" + base.symlink_to(outside, target_is_directory=True) + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + with patch("scripts.runner.subprocess.run") as execute: + with pytest.raises(ValueError, match="base"): + run_scenario(Scenario(id="probe", level=1, level_name="test", description="Disposable setup", prompt="No provider", setup_commands=())) + execute.assert_not_called() + assert sentinel.read_bytes() == b"outside must remain unchanged" + + def test_dangling_base_link_is_rejected( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + base = tmp_path / "base" + base.symlink_to(tmp_path / "missing", target_is_directory=True) + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + with pytest.raises(ValueError, match="base"): + _safe_sandbox_dir("probe") + + @pytest.mark.parametrize("entry", ["safe", "setup"]) + def test_directory_reparse_attributes_are_rejected( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, entry: str + ) -> None: + base = tmp_path / "base" + target = base / "probe" + target.mkdir(parents=True) + sentinel = target / "sentinel" + sentinel.write_bytes(b"preserve before reparse refusal") + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + original = Path.lstat + def controlled_lstat(path: Path) -> object: + if path == base: + return SimpleNamespace(st_mode=stat.S_IFDIR | 0o700, st_file_attributes=0x400) + return original(path) + monkeypatch.setattr(Path, "lstat", controlled_lstat) + with patch("scripts.runner.subprocess.run") as execute: + with pytest.raises(ValueError, match="base"): + if entry == "safe": + _safe_sandbox_dir("probe") + else: + _setup_sandbox(target, Scenario(id="probe", level=1, level_name="test", description="Disposable setup", prompt="No provider", setup_commands=())) + execute.assert_not_called() + assert sentinel.read_bytes() == b"preserve before reparse refusal" + + @pytest.mark.parametrize("existing", [False, True]) + def test_valid_setup_preserves_outside_sentinel( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, existing: bool + ) -> None: + base = tmp_path / "base" + target = base / "probe" + if existing: + target.mkdir(parents=True) + (target / "stale").write_bytes(b"prior scenario") + outside = tmp_path / "outside-sentinel" + outside.write_bytes(b"preserve outside") + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + _setup_sandbox(_safe_sandbox_dir("probe"), Scenario(id="probe", level=1, level_name="test", description="Disposable setup", prompt="No provider", setup_commands=())) + assert (target / ".git").is_dir() + assert not (target / "stale").exists() + assert outside.read_bytes() == b"preserve outside" + + def test_regular_file_base_is_rejected_before_cleanup( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + base = tmp_path / "base" + base.write_bytes(b"preserve base file") + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + with patch("scripts.runner.subprocess.run") as execute: + with pytest.raises(ValueError, match="base"): + _safe_sandbox_dir("probe") + execute.assert_not_called() + assert base.read_bytes() == b"preserve base file" + + def test_parent_alias_remains_usable( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + parent = tmp_path / "real-parent" + parent.mkdir() + alias = tmp_path / "parent-alias" + alias.symlink_to(parent, target_is_directory=True) + base = alias / "base" + monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base) + target = _safe_sandbox_dir("probe") + _setup_sandbox(target, Scenario(id="probe", level=1, level_name="test", description="Disposable setup", prompt="No provider", setup_commands=())) + assert (parent / "base/probe/.git").is_dir() + + class TestSetupSandboxSkipsShellBuiltins: """Setup commands containing shell builtins (cd/pushd/popd) must be skipped. diff --git a/tests/test_agent_self_evaluation.py b/tests/test_agent_self_evaluation.py index 23e22653b..21f85de82 100644 --- a/tests/test_agent_self_evaluation.py +++ b/tests/test_agent_self_evaluation.py @@ -67,8 +67,9 @@ def test_task_can_still_be_read_from_file(evaluator: ModuleType, tmp_path: Path, def test_missing_output_file_still_fails(evaluator: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: monkeypatch.setattr(sys, "stdin", io.StringIO("Task")) - with pytest.raises(SystemExit, match="1"): + with pytest.raises(SystemExit) as exc_info: evaluator._read_input(args(output=str(tmp_path / "missing.txt"))) + assert exc_info.value.code == 1 assert "not found" in capsys.readouterr().err