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
17 changes: 17 additions & 0 deletions hud/integrations/harbor/adapt.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand Down
43 changes: 36 additions & 7 deletions hud/integrations/harbor/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import asyncio
import contextlib
import fnmatch
import grp
import json
import math
Expand Down Expand Up @@ -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] = {}
Expand Down Expand Up @@ -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"])
Comment thread
cursor[bot] marked this conversation as resolved.
if any(path.is_symlink() for path in target.rglob("*")):
raise RuntimeError(f"artifact {source} contains a symbolic link")
Comment thread
cursor[bot] marked this conversation as resolved.


Expand Down
52 changes: 50 additions & 2 deletions hud/integrations/harbor/tests/test_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": []}
]


Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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:
Expand Down
92 changes: 92 additions & 0 deletions hud/integrations/harbor/tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down