Add fileaccess harness context provider with shared-folder file tools rooted at a directory - #643
Conversation
64a5a47 to
86fa0f1
Compare
This comment has been minimized.
This comment has been minimized.
86fa0f1 to
596246b
Compare
Introduce a self-contained agent/harness/fileaccess package that mirrors the .NET FileAccessProvider. It registers a context provider that injects file tools scoped to a caller-granted root directory: file_access_read_file, file_access_save_file, file_access_list_files, file_access_list_subdirectories, file_access_search_files, and file_access_delete_file. The root comes from Options.RootDir (not session state). A local-filesystem store constrains every operation to the root, rejecting absolute paths and ".." traversal via filepath.Clean plus a prefix check. Options.ReadOnly omits the save and delete tools to match the .NET read-only shipping mode.
596246b to
f6be276
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Adds a new Go harness context provider (agent/harness/fileaccess) that injects shared-folder file tools (read/save/list/search/delete) into agent invocations, intended to mirror the .NET FileAccessProvider and support a read-only mode.
Changes:
- Introduces
fileaccess.Providerwith tool injection + default/read-only instructions and a local filesystem-backed store rooted atOptions.RootDir. - Implements six file tools (
file_access_*) with path resolution intended to constrain access to the configured root directory, plus read-only mode by omitting mutating tools. - Adds black-box tests validating tool exposure, read-only behavior, round-trip save/read, list/search semantics, delete, and basic
../absolute-path escape rejection.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| agent/harness/fileaccess/fileaccess.go | New file-access provider and local filesystem store for shared-folder tool operations. |
| agent/harness/fileaccess/fileaccess_test.go | New black-box tests covering tool presence/behavior, basic path escape rejection, and core operations. |
Suppressed comments (1)
agent/harness/fileaccess/fileaccess.go:283
- Path containment checks do not account for symlinks inside the root. For example, if the shared folder contains a symlink directory like "link" -> "/tmp", calling save_file with path "link/outside.txt" will pass resolve() (it stays under root textually) but os.MkdirAll/os.WriteFile will follow the symlink and write outside the root. The same issue applies to read/delete/search for symlink files.
func (s *store) SaveFile(rel, content string) error {
full, err := s.resolve(rel)
if err != nil {
return err
}
| // All operations are constrained to the configured root directory. Paths are | ||
| // resolved relative to the root and any attempt to escape it (via "..", an | ||
| // absolute path, or symlink-style traversal in the supplied name) is rejected. | ||
| // |
| full := filepath.Clean(filepath.Join(s.root, rel)) | ||
| if full != s.root && !strings.HasPrefix(full, s.root+string(os.PathSeparator)) { | ||
| return "", fmt.Errorf("path %q escapes the shared folder root", rel) | ||
| } | ||
| return full, nil |
Cross-SDK Parity Review — PR #643This PR adds 🔴 Tool-name set is out of sync with .NET and PythonBoth upstream SDKs expose seven tools, not six:
✦ = name differs from upstream convention Key divergences:
Upstream references:
🟡
|
What
Adds a new self-contained
agent/harness/fileaccesspackage, wired like the siblingagent/harness/todoprovider.New(*Options)returns aProviderbacked byagent.NewContextProvider, and itsProvidehook injects file tools plus instructions on each invocation.The provider exposes the same six tools as the .NET
FileAccessProvider:file_access_read_filefile_access_save_filefile_access_list_filesfile_access_list_subdirectoriesfile_access_search_filesfile_access_delete_fileAll operations go through a local-filesystem store rooted at a caller-granted
Options.RootDir(not session state).Options.ReadOnlyomits the save/delete tools, matching the .NET read-only shipping mode.Why
The Go harness tree (
agent/harness/) hadagentmode,loop,todo,toolapproval, andtoolautocall, but no file-access counterpart, while the .NET SDK shipsFileAccessProviderwith exactly this tool set (file_access_read_file/save_file/list_files/list_subdirectories/search_files/delete_file) and a read-only mode. This closes that cross-SDK parity gap so Go agents can be granted a scoped shared folder.Safety
Every path is interpreted relative to the root. The store resolves paths with
filepath.Clean(filepath.Join(root, rel))and rejects anything that is absolute or escapes the root via a prefix check, so../outside.txtand absolute paths are refused before touching the filesystem.Tests
fileaccess_test.gois black-box (package fileaccess_test) and reuses the same harness style astodo_test.go(agenttest.CreateSession, driving tools through the exportedInvokingAPI). It covers: default tool set + instructions present;ReadOnlyomitting save/delete; save->read round trip;list_filesdirect-children-only vslist_subdirectories;search_filesregex across nested files; delete; and path-escape rejection (../outside.txt, absolute path, escaping write not creating a file).go build ./...,go vet ./agent/harness/fileaccess/..., andgo test ./agent/harness/fileaccess/...all pass.Open design questions
search_filesreturning slash-separated relative paths matched against file contents) are chosen for parity; happy to align field names/semantics exactly with the .NET tool schemas if they differ.toolapprovalcould be a follow-up.