feat: add FileSystemTool with path whitelist#2206
feat: add FileSystemTool with path whitelist#2206baobaodawang-creater wants to merge 1 commit intohuggingface:mainfrom
Conversation
VANDRANKI
left a comment
There was a problem hiding this comment.
The whitelist design is solid. Using resolve() before relative_to() correctly neutralises .. traversal and symlinks that escape the allowed tree - both are handled at the OS level before the check runs. Tests cover the main cases cleanly.
Two things worth considering before merge:
No file size limit on read_file
An agent could call read_file on a large binary or log file and load its entire contents into memory. A simple size cap (configurable, e.g. max_file_size_bytes=10*1024*1024) would prevent accidental or adversarial memory exhaustion.
No test for symlinks that escape the allowed tree
The traversal test covers ../ in the path string. Worth adding a case where a symlink inside the allowed dir points outside it - just to confirm the resolve() defence covers that path too.
Summary