diff --git a/hud/integrations/harbor/adapt.py b/hud/integrations/harbor/adapt.py index 81b86bdf2..7a5896faa 100644 --- a/hud/integrations/harbor/adapt.py +++ b/hud/integrations/harbor/adapt.py @@ -49,6 +49,8 @@ class Artifact(BaseModel): model_config = ConfigDict(extra="forbid") source: str = Field(pattern=r"^/") + destination: str | None = None + exclude: list[str] = Field(default_factory=list) service: str = Field(default="main", min_length=1) @model_validator(mode="before") @@ -64,6 +66,21 @@ def normalize_source(cls, value: str) -> str: raise ValueError("artifact source must name a path beneath /") return str(path) + @field_validator("destination") + @classmethod + def normalize_destination(cls, value: str | None) -> str | None: + """Harbor host placement: a relative path beneath the trial's artifacts dir.""" + if not value: + return None + if "\\" in value: + raise ValueError("artifact destination must use forward slashes") + path = PurePosixPath(value) + if path.is_absolute() or ".." in path.parts: + raise ValueError("artifact destination must be a relative path without '..'") + if value.rstrip("/") == "manifest.json": + raise ValueError("artifact destination 'manifest.json' is reserved by Harbor") + return value + class Collect(BaseModel): model_config = ConfigDict(extra="forbid") diff --git a/hud/integrations/harbor/env.py b/hud/integrations/harbor/env.py index f16b5207a..2c3d436c0 100644 --- a/hud/integrations/harbor/env.py +++ b/hud/integrations/harbor/env.py @@ -4,6 +4,7 @@ import asyncio import contextlib +import fnmatch import grp import json import math @@ -342,16 +343,40 @@ def copy_artifact(source: Path, target: Path) -> None: raise RuntimeError(f"artifact {source} has a symbolic link in its path") target.parent.mkdir(parents=True, exist_ok=True) if source.is_dir(): - for root, directories, files in os.walk(source, followlinks=False): - for name in (*directories, *files): - entry = Path(root, name) - if entry.is_symlink(): - raise RuntimeError(f"artifact {source} contains symbolic link {entry}") - shutil.copytree(source, target) + shutil.copytree(source, target, symlinks=True) elif source.exists() or source.is_symlink(): shutil.copy2(source, target, follow_symlinks=False) +def prune_excluded(target: Path, patterns: list[str]) -> None: + """Drop staged entries matching Harbor exclude patterns (GNU tar semantics). + + A pattern excludes an entry when it matches any run of trailing path + components; matching a directory prunes its whole subtree. + """ + + def excluded(entry: Path) -> bool: + parts = entry.relative_to(target).as_posix().split("/") + return any( + fnmatch.fnmatch("/".join(parts[start:]), pattern) + for pattern in patterns + for start in range(len(parts)) + ) + + for root, directories, files in os.walk(target, topdown=True): + for name in list(directories): + entry = Path(root, name) + if excluded(entry): + if entry.is_symlink(): + entry.unlink() + else: + shutil.rmtree(entry) + directories.remove(name) + for name in files: + if excluded(Path(root, name)): + Path(root, name).unlink() + + async def collect(task: dict[str, Any]) -> None: clear(ARTIFACTS) services: dict[str, str] = {} @@ -440,7 +465,11 @@ async def container(service: str) -> str: continue else: copy_artifact(Path(source), target) - if target.is_symlink() or any(path.is_symlink() for path in target.rglob("*")): + if target.is_symlink(): + raise RuntimeError(f"artifact {source} contains a symbolic link") + if artifact["exclude"] and target.is_dir(): + prune_excluded(target, artifact["exclude"]) + if any(path.is_symlink() for path in target.rglob("*")): raise RuntimeError(f"artifact {source} contains a symbolic link") diff --git a/hud/integrations/harbor/tests/test_contract.py b/hud/integrations/harbor/tests/test_contract.py index 84fb6a14f..154b5b763 100644 --- a/hud/integrations/harbor/tests/test_contract.py +++ b/hud/integrations/harbor/tests/test_contract.py @@ -595,7 +595,7 @@ def test_adapt_groups_identical_images_and_keeps_row_metadata( assert taskset["build-pmars"].agent_config == {"timeout_seconds": 45.0} assert taskset["build-pmars"].args["task"]["verifier_timeout"] == 30.0 assert taskset["build-pmars"].args["task"]["artifacts"] == [ - {"service": "main", "source": "/tmp/result"} + {"service": "main", "source": "/tmp/result", "destination": None, "exclude": []} ] @@ -883,7 +883,9 @@ def test_adapt_builds_a_separate_verifier_and_reuses_the_runtime( } assert "tasks" not in manifest assert row.args["task"] == { - "artifacts": [{"service": "main", "source": "/tmp/agent.patch"}], + "artifacts": [ + {"service": "main", "source": "/tmp/agent.patch", "destination": None, "exclude": []} + ], "collect": [{"command": "redis-cli save", "service": "redis", "timeout_sec": 10.0}], "description": "", "id": "separate", @@ -995,6 +997,52 @@ def test_artifacts_must_name_normalized_paths_beneath_root( harbor.adapt(tmp_path) +def test_artifacts_keep_harbor_destination_and_exclude(tmp_path: Path) -> None: + task = make_harbor_task(tmp_path, "task-a") + (task / "task.toml").write_text( + """\ +[[artifacts]] +source = "/app/outputs" +destination = "results/outputs" +exclude = ["*.tmp", "cache"] +""", + encoding="utf-8", + ) + + taskset = harbor.adapt(tmp_path) + + assert taskset["task-a"].args["task"]["artifacts"] == [ + { + "service": "main", + "source": "/app/outputs", + "destination": "results/outputs", + "exclude": ["*.tmp", "cache"], + } + ] + + +@pytest.mark.parametrize( + "destination", + ["/absolute/path", "results/../../escape", "results\\\\windows", "manifest.json"], +) +def test_artifact_destinations_harbor_rejects_fail_adaptation( + tmp_path: Path, + destination: str, +) -> None: + task = make_harbor_task(tmp_path, "task-a") + (task / "task.toml").write_text( + f"""\ +[[artifacts]] +source = "/app/out.json" +destination = "{destination}" +""", + encoding="utf-8", + ) + + with pytest.raises(ValueError, match="not a valid Harbor task"): + harbor.adapt(tmp_path) + + def test_agent_timeout_becomes_per_task_agent_policy( tmp_path: Path, ) -> None: diff --git a/hud/integrations/harbor/tests/test_integration.py b/hud/integrations/harbor/tests/test_integration.py index 862ce0312..dfb786d68 100644 --- a/hud/integrations/harbor/tests/test_integration.py +++ b/hud/integrations/harbor/tests/test_integration.py @@ -241,6 +241,98 @@ def test_separate_verifier_rejects_artifact_symlinks( assert "artifact /app/main.html is a symbolic link" in (run.trace.error or "") +def test_separate_verifier_rejects_sidecar_symlink_artifact_roots( + tmp_path_factory: pytest.TempPathFactory, wheel: Path +) -> None: + dataset = tmp_path_factory.mktemp("harbor-sidecar-symlink") / "harbor-harness" + task = dataset / "sidecar-reachability" + shutil.copytree(TASKS / "sidecar-reachability", task) + (task / "task.toml").write_text( + """\ +artifacts = [{ source = "/link", service = "web", exclude = ["main.html"] }] + +[task] +name = "sidecar-reachability" + +[verifier] +environment_mode = "separate" +timeout_sec = 30 + +[[verifier.collect]] +service = "web" +command = "ln -sfn /app /link" +timeout_sec = 10 +""", + encoding="utf-8", + ) + (task / "solution" / "solve.sh").write_text("true\n", encoding="utf-8") + + run = asyncio.run(_grade_every_task(dataset, wheel))["sidecar-reachability"] + + assert run.reward == 0.0 + assert "artifact /link contains a symbolic link" in (run.trace.error or "") + + +def test_separate_verifier_sees_directory_artifacts_without_excluded_entries( + tmp_path_factory: pytest.TempPathFactory, wheel: Path +) -> None: + dataset = tmp_path_factory.mktemp("harbor-artifact-exclude") / "harbor-harness" + task = dataset / "sidecar-reachability" + shutil.copytree(TASKS / "sidecar-reachability", task) + (task / "task.toml").write_text( + """\ +artifacts = [ + { source = "/app/outputs", destination = "results", exclude = ["*.tmp", "cache"] }, +] + +[task] +name = "sidecar-reachability" + +[verifier] +environment_mode = "separate" +timeout_sec = 30 +""", + encoding="utf-8", + ) + (task / "solution" / "solve.sh").write_text( + """\ +#!/bin/sh +set -eu +mkdir -p /app/outputs/cache/nested /app/outputs/logs +echo keep > /app/outputs/keep.txt +echo junk > /app/outputs/junk.tmp +echo junk > /app/outputs/logs/nested.tmp +echo junk > /app/outputs/cache/nested/blob +ln -s /etc/passwd /app/outputs/cache/link +ln -s missing /app/outputs/logs/dangling.tmp +""", + encoding="utf-8", + ) + (task / "tests" / "test.sh").write_text( + """\ +#!/bin/sh +set -u +mkdir -p /logs/verifier +if [ "$(cat /app/outputs/keep.txt 2>/dev/null)" = "keep" ] \\ + && [ -d /app/outputs/logs ] \\ + && [ ! -e /app/outputs/junk.tmp ] \\ + && [ ! -e /app/outputs/logs/nested.tmp ] \\ + && [ ! -L /app/outputs/logs/dangling.tmp ] \\ + && [ ! -e /app/outputs/cache ]; then + echo 1 > /logs/verifier/reward.txt +else + echo "excluded artifact entries leaked into the verifier" >&2 + echo 0 > /logs/verifier/reward.txt +fi +""", + encoding="utf-8", + ) + + run = asyncio.run(_grade_every_task(dataset, wheel))["sidecar-reachability"] + + assert run.reward == 1.0, run.trace.error + + def test_separate_verifier_rejects_artifacts_beneath_symlinks( tmp_path_factory: pytest.TempPathFactory, wheel: Path ) -> None: