Skip to content

test(tools): add FileExecutor allowed_paths integration tests (#2117)#2127

Merged
bug-ops merged 2 commits intomainfrom
feat-issue-2117-file-executor-access-tests
Mar 22, 2026
Merged

test(tools): add FileExecutor allowed_paths integration tests (#2117)#2127
bug-ops merged 2 commits intomainfrom
feat-issue-2117-file-executor-access-tests

Conversation

@bug-ops
Copy link
Owner

@bug-ops bug-ops commented Mar 22, 2026

Summary

Adds 15 integration tests for FileExecutor access control (sandbox enforcement via allowed_paths). Closes #2117.

  • Read/write permitted inside sandbox, blocked outside
  • All 8 file tools (read, write, edit, list_directory, create_directory, delete_path, move_path, copy_path) blocked for outside-sandbox paths
  • Symlink escape (single hop + chain) blocked — #[cfg(unix)]
  • Path traversal (../..) blocked
  • Multiple allowed_paths enforced independently
  • allowed_paths = [] falls back to cwd
  • Tilde regression (#2115) — documents current buggy behavior
  • delete_sandbox_root_blocked — cannot delete the sandbox root itself
  • Cross-boundary move/copy blocked in all 4 directions
  • Nested write creates parent directories within sandbox
  • find_path glob results are post-filtered to exclude outside-sandbox paths
  • grep with default path (".") stays within sandbox

Docker omission rationale

The issue proposed Docker+testcontainers for a controlled FS hierarchy. After analysing the implementation, Docker is unnecessary: FileExecutor enforces access via a pure starts_with(allowed_path) check on the local filesystem — there are no Unix permission boundaries, chroot, or container namespaces involved. tempfile::TempDir provides identical, deterministic isolation with no external dependencies and sub-millisecond teardown. This avoids Docker daemon requirements in CI and local development.

Test execution

# Run all FileExecutor tests (no Docker, no flags needed)
cargo nextest run --config-file .github/nextest.toml -p zeph-tools --test file_access
# 15 tests run: 15 passed, 0 skipped

Post-merge action

Security issue #2125 filed: resolve_via_ancestors does not normalize .. in suffix for non-existent intermediate directories — real sandbox bypass. Tracked separately.

Test plan

  • cargo +nightly fmt --check passes
  • cargo clippy --workspace --features full -- -D warnings passes
  • cargo nextest run --config-file .github/nextest.toml --workspace --features full --lib --bins — 6366 passed
  • cargo nextest run -p zeph-tools --test file_access — 15 passed

bug-ops added 2 commits March 22, 2026 14:42
15 integration tests covering sandbox access controls: read/write inside
allowed paths, blocked access outside cwd, symlink escape (unix), path
traversal, tilde regression (#2115), multiple allowed paths, cross-boundary
move/copy, and glob/grep default-path confinement.
@github-actions github-actions bot added documentation Improvements or additions to documentation rust Rust code changes tests Test-related changes size/XL Extra large PR (500+ lines) labels Mar 22, 2026
@bug-ops bug-ops enabled auto-merge (squash) March 22, 2026 14:02
@bug-ops bug-ops merged commit bb48a9e into main Mar 22, 2026
25 checks passed
@bug-ops bug-ops deleted the feat-issue-2117-file-executor-access-tests branch March 22, 2026 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation rust Rust code changes size/XL Extra large PR (500+ lines) tests Test-related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: integration tests for FileExecutor allowed_paths using Docker

1 participant