diff --git a/src/clawbench/runner/run_support/task.py b/src/clawbench/runner/run_support/task.py index 96c4f6ee..24403b16 100644 --- a/src/clawbench/runner/run_support/task.py +++ b/src/clawbench/runner/run_support/task.py @@ -129,6 +129,30 @@ def add_item(item: Any, label: str) -> None: return entries, warnings +def validate_extra_info_path(task_dir: Path, rel_path: str) -> Path: + """Resolve an extra_info path and reject anything outside ``task_dir``. + + A task's ``extra_info[].path`` is untrusted input. Without a guard, an + absolute path or one containing ``..`` could make ``copy_extra_info`` read + arbitrary host files into the staged my-info dir (which may be shipped in a + shareable export). Reject absolute paths and any target whose resolved + location — symlinks followed — escapes ``task_dir``. Returns the unresolved + ``task_dir / rel_path`` so normal copy semantics are unchanged. + """ + if Path(rel_path).is_absolute(): + raise ValueError( + f"extra_info path must be relative, got absolute: {rel_path!r}" + ) + src = task_dir / rel_path + try: + src.resolve().relative_to(task_dir.resolve()) + except ValueError as e: + raise ValueError( + f"extra_info path escapes the task directory: {rel_path!r}" + ) from e + return src + + def copy_extra_info(task: dict, task_dir: Path, personal_info_dir: Path) -> list[str]: """Copy extra_info files from the test case into the my-info dir.""" entries, warnings = normalize_extra_info(task.get("extra_info")) @@ -138,7 +162,7 @@ def copy_extra_info(task: dict, task_dir: Path, personal_info_dir: Path) -> list rel_path = info.get("path") if not rel_path: continue - src = task_dir / rel_path + src = validate_extra_info_path(task_dir, rel_path) if not src.exists(): warning = f"extra_info path not found: {src}" warnings.append(warning) diff --git a/tests/test_host_resources.py b/tests/test_host_resources.py index bd291c90..cf41b962 100644 --- a/tests/test_host_resources.py +++ b/tests/test_host_resources.py @@ -6,11 +6,14 @@ import json from pathlib import Path +import pytest + from clawbench.runner.run_support.task import ( build_instruction, copy_extra_info, normalize_extra_info, prepare_personal_info, + validate_extra_info_path, ) from clawbench.utils.paths import ASSET_ROOT, SHARED_ROOT @@ -63,6 +66,39 @@ def test_extra_info_copy_and_instruction_are_host_side(tmp_path: Path) -> None: assert "2-day meal plan" in instruction +def test_validate_extra_info_path_accepts_valid_relative(tmp_path: Path) -> None: + (tmp_path / "info.json").write_text("{}") + src = validate_extra_info_path(tmp_path, "info.json") + assert src == tmp_path / "info.json" + + +def test_validate_extra_info_path_rejects_absolute(tmp_path: Path) -> None: + # Must be rejected. On POSIX "/etc/passwd" trips the is_absolute() guard + # ("absolute"); on Windows it isn't absolute (no drive), so it's rejected by + # the containment check ("escapes") instead — either way it raises. + with pytest.raises(ValueError, match="absolute|escapes"): + validate_extra_info_path(tmp_path, "/etc/passwd") + + +def test_validate_extra_info_path_rejects_dotdot_escape(tmp_path: Path) -> None: + with pytest.raises(ValueError, match="escapes"): + validate_extra_info_path(tmp_path, "../../etc/passwd") + + +def test_copy_extra_info_skips_traversal(tmp_path: Path) -> None: + """A malicious extra_info path must not copy files from outside the task dir.""" + secret = tmp_path / "secret.txt" + secret.write_text("top secret") + task_dir = tmp_path / "case" + task_dir.mkdir() + my_info = tmp_path / "my-info" + my_info.mkdir() + task = {"extra_info": [{"path": "../secret.txt"}]} + with pytest.raises(ValueError, match="escapes"): + copy_extra_info(task, task_dir, my_info) + assert not (my_info / "secret.txt").exists() + + def test_normalize_extra_info_accepts_legacy_and_schema_shapes() -> None: entries, warnings = normalize_extra_info( [