Support external eval fixture files#13
Conversation
|
Local verification on the PR branch is clean:
That covers the external fixture loader slice, traversal rejection cases, and the repo's clippy warning gate. |
bennewell35
left a comment
There was a problem hiding this comment.
This is the right narrow first slice for external eval fixtures. I found one path-safety gap before merge:
copy_dir_all bounds the workspace directory, but it does not reject symlinked child entries. DirEntry::file_type() does not follow symlinks, so a symlinked file is not treated as a directory and falls into the fs::copy(entry.path(), target) branch. Rust fs::copy accepts a symlink to a regular file as a source, so a fixture workspace can contain workspace/basic/leak.txt -> /tmp/secret.txt; validation passes because both workspace and fileContains.path are syntactically relative, but the copied eval workspace now contains bytes from outside the fixture root.
Smallest fix: reject symlink entries while validating/copying fixture workspaces, or canonicalize every copied child/read target against the fixture root before allowing it. Please add a regression test with a symlinked file under the external workspace. That keeps this PR aligned with the README promise that external workspaces cannot escape the fixture root.
|
Addressed the symlink workspace-copy gap in What changed:
Local verification on Windows:
|
Summary
Adds the first narrow slice for external agent eval fixtures:
small-harness --eval <path-to-fixture.json>now loads a data-only fixture file.fileContains.pathrefs reject absolute paths and parent traversal.This is intentionally smaller than full fixture-pack provenance: no executable hooks, no pack manifest loader, no artifact indexing, and no receipt format changes.
Closes part of #12.
Verification
Passed locally on Windows:
Full
cargo testwas also run. It passed 424 tests and failed 3 tests that appear unrelated to this change and Windows/environment-specific: