Summary

PR #2958 added named workspaces ([workspaces] in agent.toml / HAND.toml agent sections) and wired them into write-side denial — tool_runner.rs:419–434 checks kernel.readonly_workspace_prefixes() to reject writes into read-only named workspaces. But the corresponding read-side allowance was never wired: file_read, file_list, and apply_patch go through workspace_sandbox::resolve_sandbox_path which only checks the canonical primary workspace root.

Result: an agent that legitimately declares foo = { path = "shared/foo", mode = "rw" } and learns the absolute path from the auto-generated TOOLS.md line @foo → /abs/path/shared/foo (read-write) can shell_exec cat <path> against it (no sandbox check on shell), but file_read({"path": "<same path>"}) returns:

Access denied: path '/.../shared/foo/...' resolves outside workspace.

This PR closes the asymmetry.

Why no test caught it

crates/librefang-runtime/src/workspace_sandbox.rs tests all use a single workspace root. There's no positive case asserting that read tools succeed against a declared named workspace path. PR #2958's review feedback caught the write-side denial gap (the "address code review issues" commit added file_write enforcement), but no one noticed reads were also broken because no test asserted positive read-allowed behavior on named workspaces.

Design

1. KernelHandle::named_workspace_prefixes(agent_id) -> Vec<(PathBuf, WorkspaceMode)> — new method returning all canonical absolute paths with their modes. The kernel impl canonicalizes once. readonly_workspace_prefixes becomes a thin filter on top so canonical resolution isn't duplicated.

2. workspace_sandbox::resolve_sandbox_path_ext(user_path, workspace_root, additional_roots: &[&Path]) — the existing two-arg resolve_sandbox_path is preserved as a wrapper. The new function widens acceptance to inside_primary || inside_any_additional. .. rejection and the symlink-escape predicate are unchanged. Relative paths still join with workspace_root only — additional roots are absolute-only entry points.

3. Tool dispatch wires the prefixes:

   • file_read / file_list / apply_patch → all named-workspace prefixes (rw and r both allowed for reads).
   • file_write → rw-filtered prefixes only. The existing read-only denial check (readonly_workspace_prefixes) still runs before path resolution, preserving the spec'd behavior of a clear "Write denied: read-only named workspace" error message rather than a generic sandbox rejection.

The leaf functions (tool_file_read/write/list, apply_patch) gained an additional_roots: &[&Path] parameter; two file-private helpers in tool_runner.rs (named_ws_prefixes, named_ws_prefixes_writable) build the slice from the kernel handle.

Files changed

Tests

• workspace_sandbox.rs — went from 7 → 12 unit tests:
  • test_relative_path_inside_primary_workspace_with_additional — relative still joins primary
  • test_absolute_path_inside_additional_root_allowed — absolute under an additional root succeeds
  • test_absolute_path_outside_all_roots_blocked — neither primary nor additional → denied
  • test_dotdot_still_blocked_with_additional_roots — .. still rejected
  • test_symlink_escape_still_blocked_via_additional_root — symlink escape from additional root denied
• tool_runner.rs — 5 new integration tests using a small NamedWsKernel mock that drives named_workspace_prefixes directly. Covers: file_read against rw, file_read against r, file_list, file_write against rw, file_write against r returns the existing read-only denial.

Verification

cargo build --workspace --lib                                # clean
cargo clippy --workspace --all-targets -- -D warnings        # clean
cargo test --lib -p librefang-runtime                        # 1314 pass, 0 fail

cargo test --workspace --lib shows 9 pre-existing failures (config::tests::test_load_config_defaults, 8 cron_delivery tests rejecting loopback URLs / absolute paths). Verified independently on upstream/main — same 9 failures. Unrelated to this change.

Note on a small extension to the design

The non-existent-parent rebase branch in resolve_sandbox_path_ext (the path that supports writing new files whose parent dir doesn't exist yet) needed to be extended: when the candidate is absolute and lives under an additional root, the file-doesn't-yet-exist path rebases onto the additional root rather than the primary canon root. Without that, file_write of a new file inside a named workspace would starts_with-fail. This preserves the symlink-handling rationale of the original code and is exercised by the integration tests.

Risk / blast radius

Low. Read-side tools only widen acceptance; no path that was previously allowed becomes denied. Write-side denial preserved. The KernelHandle gets one new default-impl method, so any third-party KernelHandle implementor compiles unchanged.

Repro before this PR

# agent.toml
[workspaces]
foo = { path = "shared/foo", mode = "rw" }

$ # absolute path picked up from TOOLS.md auto-generated entry
$ # @foo → /home/.../shared/foo (read-write)
$ # via file_read tool:
file_read({"path": "/home/.../shared/foo/x.json"})
→ Access denied: path '...' resolves outside workspace.
$ # via shell_exec — works (no sandbox check):
shell_exec({"command": "cat /home/.../shared/foo/x.json"})
→ <file contents>

After this PR: both succeed. file_write against an r-mode named workspace continues to return the existing "Write denied" error.

Closes the read-side half of the contract introduced by #2958.