Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion src/clawbench/runner/run_support/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand All @@ -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)
Expand Down
36 changes: 36 additions & 0 deletions tests/test_host_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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(
[
Expand Down
Loading