mirror of
https://github.com/affaan-m/ECC.git
synced 2026-10-26 18:35:05 +00:00
fix: reject linked sandbox bases before scenario cleanup
This commit is contained in:
4 files changed
+156
-2
No files matched your search
@@ -10,7 +10,7 @@
|
|||||||
"id": "hand-seeded",
|
"id": "hand-seeded",
|
||||||
"source": "manual-curation-pending-regeneration"
|
"source": "manual-curation-pending-regeneration"
|
||||||
},
|
},
|
||||||
"registryDigest": "7249c1a4f4fc6673d3bf62d4800b995740a8a322b9d6cb62f1069c58e2ad5f26",
|
"registryDigest": "fc16661cec4110a3158718915eba7078799726f7905357db7b0395f63fc19692",
|
||||||
"schemaVersion": 1,
|
"schemaVersion": 1,
|
||||||
"triggers": {
|
"triggers": {
|
||||||
"skill:agent-runtime-gateway-smoke-test": [
|
"skill:agent-runtime-gateway-smoke-test": [
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import json
|
|||||||
import re
|
import re
|
||||||
import shlex
|
import shlex
|
||||||
import shutil
|
import shutil
|
||||||
|
import stat
|
||||||
import subprocess
|
import subprocess
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from pathlib import Path
|
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:
|
def _safe_sandbox_dir(scenario_id: str) -> Path:
|
||||||
"""Sanitize scenario ID and ensure path stays within sandbox base."""
|
"""Sanitize scenario ID and ensure path stays within sandbox base."""
|
||||||
|
_validate_sandbox_base()
|
||||||
safe_id = re.sub(r"[^a-zA-Z0-9\-_]", "_", scenario_id)
|
safe_id = re.sub(r"[^a-zA-Z0-9\-_]", "_", scenario_id)
|
||||||
path = SANDBOX_BASE / safe_id
|
path = SANDBOX_BASE / safe_id
|
||||||
resolved_path = path.resolve()
|
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:
|
def _setup_sandbox(sandbox_dir: Path, scenario: Scenario) -> None:
|
||||||
"""Create sandbox directory and run setup commands."""
|
"""Create sandbox directory and run setup commands."""
|
||||||
|
_validate_sandbox_base()
|
||||||
if sandbox_dir.exists():
|
if sandbox_dir.exists():
|
||||||
shutil.rmtree(sandbox_dir)
|
shutil.rmtree(sandbox_dir)
|
||||||
sandbox_dir.mkdir(parents=True)
|
sandbox_dir.mkdir(parents=True)
|
||||||
|
|||||||
@@ -3,9 +3,11 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import json
|
import json
|
||||||
|
import stat
|
||||||
import subprocess
|
import subprocess
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
from types import SimpleNamespace
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
@@ -16,6 +18,7 @@ from scripts.runner import (
|
|||||||
_setup_sandbox,
|
_setup_sandbox,
|
||||||
run_scenario,
|
run_scenario,
|
||||||
)
|
)
|
||||||
|
from scripts.scenario_generator import Scenario
|
||||||
|
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
@@ -60,6 +63,136 @@ class TestSafeSandboxDirectory:
|
|||||||
_safe_sandbox_dir("escape")
|
_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:
|
class TestSetupSandboxSkipsShellBuiltins:
|
||||||
"""Setup commands containing shell builtins (cd/pushd/popd) must be skipped.
|
"""Setup commands containing shell builtins (cd/pushd/popd) must be skipped.
|
||||||
|
|
||||||
|
|||||||
@@ -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:
|
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"))
|
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")))
|
evaluator._read_input(args(output=str(tmp_path / "missing.txt")))
|
||||||
|
assert exc_info.value.code == 1
|
||||||
assert "not found" in capsys.readouterr().err
|
assert "not found" in capsys.readouterr().err
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user