diff --git a/agent-schema.json b/agent-schema.json index 758f5a361..9b7edff03 100644 --- a/agent-schema.json +++ b/agent-schema.json @@ -1372,6 +1372,29 @@ "description": "Whether to ignore VCS files (.git directories and .gitignore patterns) in filesystem operations. Default: true", "default": true }, + "allow_list": { + "type": "array", + "description": "Allow-list of directories the filesystem tool is permitted to access (only valid for type 'filesystem'). Each entry may be '.' (the agent's working directory), '~' or '~/...' (the user's home directory), an absolute path, or a relative path (anchored at the working directory). Symlinks are followed before the containment check, so they cannot be used to escape the allow-list. An empty or omitted list preserves the default behaviour (any path the agent process can reach).", + "items": { + "type": "string" + }, + "examples": [ + ["."], + [".", "~/projects"], + ["/srv/data", "~/scratch"] + ] + }, + "deny_list": { + "type": "array", + "description": "Deny-list of directories the filesystem tool is forbidden to access (only valid for type 'filesystem'). Uses the same expansion rules as 'allow_list'. The deny-list takes precedence over the allow-list: a path that matches both is rejected.", + "items": { + "type": "string" + }, + "examples": [ + ["~/.ssh", "~/.aws"], + ["/etc", "/var/lib"] + ] + }, "defer": { "description": "Enable deferred loading for tools in this toolset. Set to true to defer all tools, or an array of tool names to defer only those tools. Deferred tools are not loaded into the agent's context immediately, but can be discovered and loaded on-demand using search_tool and add_tool.", "oneOf": [ diff --git a/docs/tools/filesystem/index.md b/docs/tools/filesystem/index.md index 6902a043a..e02461483 100644 --- a/docs/tools/filesystem/index.md +++ b/docs/tools/filesystem/index.md @@ -37,10 +37,50 @@ toolsets: | Property | Type | Default | Description | | --- | --- | --- | --- | -| `ignore_vcs` | boolean | `false` | When `true`, ignores `.gitignore` patterns and includes all files | +| `ignore_vcs` | boolean | `true` | When `true` (default), `.git` directories and `.gitignore` patterns are excluded from listings and searches. Set to `false` to include them. | | `post_edit` | array | `[]` | Commands to run after editing files matching a path pattern | | `post_edit[].path` | string | — | Glob pattern for files (e.g., `*.go`, `src/**/*.ts`) | | `post_edit[].cmd` | string | — | Command to run (use `${file}` for the edited file path) | +| `allow_list` | array | `[]` | Directories the tools may access. Empty = unrestricted (default). | +| `deny_list` | array | `[]` | Directories the tools must not access. Takes precedence over `allow_list`. | + +### Path access control + +By default the filesystem tools are unrestricted: relative paths resolve +from the working directory, but absolute paths and `..` traversals can +reach anywhere the agent process can. Configure `allow_list` and/or +`deny_list` to sandbox the toolset. + +Entries in either list are expanded as follows: + +- `"."` — the agent's working directory +- `"~"` or `"~/..."` — the user's home directory +- `"$VAR"` / `"${VAR}"` — environment variable expansion +- absolute paths — used as-is +- relative paths — anchored at the working directory + +Symlinks are resolved before the containment check, so a symlink inside an +allowed root cannot be used to escape it. When an `allow_list` is set, +each entry is opened as a Go [`*os.Root`](https://pkg.go.dev/os#Root) so +that the kernel's rooted-lookup semantics also reject `..` and symlink +escapes at I/O time, not just at resolve time. + +```yaml +toolsets: + - type: filesystem + # Restrict every operation to the working directory and the user's + # home folder, then carve credentials out of the home folder. + allow_list: + - "." + - "~" + deny_list: + - "~/.ssh" + - "~/.aws" +``` + +When the path supplied by the agent is rejected, the tool returns a +structured error rather than performing any filesystem I/O. This makes the +restriction visible to the model so it can adjust its plan. ### Post-Edit Hooks diff --git a/examples/filesystem_allow_deny.yaml b/examples/filesystem_allow_deny.yaml new file mode 100644 index 000000000..b44dd64ea --- /dev/null +++ b/examples/filesystem_allow_deny.yaml @@ -0,0 +1,75 @@ +#!/usr/bin/env docker agent run + +# Demonstrates allow_list / deny_list for the filesystem tool. +# +# By default the filesystem toolset is unrestricted: relative paths resolve +# from the working directory, but the tools will happily read or write +# absolute paths (or `..` traversals) anywhere the agent process can reach. +# That is fine for local development but a footgun when running an agent on +# someone else's behalf. +# +# Configure `allow_list` to limit operations to a fixed set of roots, and/or +# `deny_list` to carve specific subtrees out. Tokens are expanded as follows: +# +# "." -> the agent's working directory +# "~" -> the user's home directory +# "~/foo" -> $HOME/foo +# "$VAR" -> environment variable +# absolute -> used as-is +# relative -> joined with the working directory +# +# Symlinks are resolved before the containment check, so a symlink inside an +# allowed root cannot be used to escape it. +# +# Switch agents with `-a ` to try each variant. + +agents: + # Strict variant: only the working directory and the user's home are + # reachable, and the SSH/AWS credential directories are explicitly off-limits + # even though they live under the home directory. + root: + model: anthropic/claude-sonnet-4-5 + description: A filesystem agent restricted to its working directory and home folder. + instruction: | + You may read and write files under the current working directory and + under the user's home directory, except for the credentials folders + (~/.ssh, ~/.aws). If asked to touch a path outside that scope, refuse + and explain why. + toolsets: + - type: filesystem + allow_list: + - "." + - "~" + deny_list: + - "~/.ssh" + - "~/.aws" + - "~/.docker" + + # Workspace-only variant: the agent can only see its own working tree. + workspace_only: + model: anthropic/claude-sonnet-4-5 + description: A filesystem agent that cannot leave its working directory. + instruction: | + You may only read and write files inside the current working directory. + Refuse any request that would require touching files elsewhere on the + filesystem. + toolsets: + - type: filesystem + allow_list: + - "." + + # Deny-only variant: keeps the default permissive behaviour but explicitly + # forbids a few sensitive directories. + guarded: + model: anthropic/claude-sonnet-4-5 + description: A filesystem agent with a small set of off-limits directories. + instruction: | + You have broad filesystem access, but a few directories are off-limits. + If a request is rejected, do not retry and explain the situation to the + user. + toolsets: + - type: filesystem + deny_list: + - "~/.ssh" + - "~/.aws" + - "/etc" diff --git a/pkg/config/latest/types.go b/pkg/config/latest/types.go index 82a0323d4..545ac95e6 100644 --- a/pkg/config/latest/types.go +++ b/pkg/config/latest/types.go @@ -820,6 +820,23 @@ type Toolset struct { // For the `filesystem` tool - VCS integration IgnoreVCS *bool `json:"ignore_vcs,omitempty"` + // For the `filesystem` tool - allow-list of directories the tools are + // permitted to access. Each entry may be "." (the agent's working + // directory), "~" or "~/..." (the user's home directory), an absolute + // path, or a relative path (anchored at the working directory). When + // non-empty, every read/write operation is rejected unless its target + // resolves under one of the listed roots. Symlinks are followed before + // the containment check so they cannot be used to escape the allow-list. + // An empty or omitted list preserves the default behaviour (any path + // reachable by the process is allowed). + AllowList []string `json:"allow_list,omitempty" yaml:"allow_list,omitempty"` + + // For the `filesystem` tool - deny-list of directories the tools are + // forbidden to access. Same expansion and matching rules as `allow_list`. + // The deny-list takes precedence over `allow_list`: a path that matches + // both is rejected. An empty or omitted list disables the deny-list. + DenyList []string `json:"deny_list,omitempty" yaml:"deny_list,omitempty"` + // For the `lsp` tool FileTypes []string `json:"file_types,omitempty"` diff --git a/pkg/config/latest/validate.go b/pkg/config/latest/validate.go index 42787f87b..fe79b12e0 100644 --- a/pkg/config/latest/validate.go +++ b/pkg/config/latest/validate.go @@ -73,6 +73,18 @@ func (t *Toolset) validate() error { if t.IgnoreVCS != nil && t.Type != "filesystem" { return errors.New("ignore_vcs can only be used with type 'filesystem'") } + if len(t.AllowList) > 0 && t.Type != "filesystem" { + return errors.New("allow_list can only be used with type 'filesystem'") + } + if len(t.DenyList) > 0 && t.Type != "filesystem" { + return errors.New("deny_list can only be used with type 'filesystem'") + } + if err := validatePathRootEntries("allow_list", t.AllowList); err != nil { + return err + } + if err := validatePathRootEntries("deny_list", t.DenyList); err != nil { + return err + } if len(t.Env) > 0 && (t.Type != "shell" && t.Type != "script" && t.Type != "mcp" && t.Type != "lsp") { return errors.New("env can only be used with type 'shell', 'script', 'mcp' or 'lsp'") } @@ -215,6 +227,19 @@ func (t *Toolset) validate() error { return nil } +// validatePathRootEntries rejects empty / whitespace-only entries in a +// filesystem allow- or deny-list. An empty entry would be a foot-gun: it +// would resolve to the working directory and silently widen (or close) the +// matched set in surprising ways. +func validatePathRootEntries(field string, entries []string) error { + for i, e := range entries { + if strings.TrimSpace(e) == "" { + return fmt.Errorf("%s[%d] must not be empty", field, i) + } + } + return nil +} + // validateDomainPatterns rejects empty / whitespace-only entries in a fetch // allow- or block-list, since they silently match nothing and turn the list // into a foot-gun (e.g. allowed_domains: [""] would reject every URL). diff --git a/pkg/teamloader/registry.go b/pkg/teamloader/registry.go index c566b5f44..e589d6a3e 100644 --- a/pkg/teamloader/registry.go +++ b/pkg/teamloader/registry.go @@ -269,6 +269,15 @@ func createFilesystemTool(_ context.Context, toolset latest.Toolset, _ string, r } opts = append(opts, builtin.WithIgnoreVCS(ignoreVCS)) + // Handle allow/deny lists for filesystem operations. + // An empty / nil list preserves the default behaviour (no restriction). + if len(toolset.AllowList) > 0 { + opts = append(opts, builtin.WithAllowList(toolset.AllowList)) + } + if len(toolset.DenyList) > 0 { + opts = append(opts, builtin.WithDenyList(toolset.DenyList)) + } + // Handle post-edit commands if len(toolset.PostEdit) > 0 { postEditConfigs := make([]builtin.PostEditConfig, len(toolset.PostEdit)) diff --git a/pkg/tools/builtin/filesystem.go b/pkg/tools/builtin/filesystem.go index f3a3016ca..1f34f888f 100644 --- a/pkg/tools/builtin/filesystem.go +++ b/pkg/tools/builtin/filesystem.go @@ -45,6 +45,15 @@ type FilesystemTool struct { ignoreVCS bool repoMatcher *fsx.VCSMatcher repoMatcherOnce sync.Once + + // allowList, when non-nil, restricts every filesystem operation to paths + // that resolve under one of the listed roots. nil means "no allow-list"; + // the toolset accepts any path the OS will let it touch. + allowList *pathRootSet + // denyList, when non-nil, rejects every filesystem operation on paths + // that resolve under one of the listed roots, even when the path also + // matches the allow-list. nil means "no deny-list". + denyList *pathRootSet } // Verify interface compliance @@ -70,6 +79,47 @@ func WithIgnoreVCS(ignoreVCS bool) FileSystemOpt { } } +// WithAllowList restricts every filesystem operation to paths that resolve +// under one of the supplied roots. Each entry may be: +// - "." — the agent's working directory +// - "~" or "~/..." — the user's home directory +// - "$VAR" / "${VAR}" — an environment variable +// - any absolute or relative path (relative paths are anchored at the +// working directory) +// +// Symlinks are resolved before the containment check, so a symlink inside an +// allowed root cannot be used to escape it. An empty or nil slice disables +// the allow-list and preserves the default behaviour (any path is allowed). +// +// Invalid entries (e.g. an empty string) are logged and the allow-list is +// silently dropped, mirroring how WithIgnoreVCS handles construction errors. +func WithAllowList(roots []string) FileSystemOpt { + return func(t *FilesystemTool) { + set, err := newPathRootSet(t.workingDir, roots) + if err != nil { + slog.Warn("filesystem allow-list: invalid entry; ignoring allow-list", "error", err) + return + } + t.allowList = set + } +} + +// WithDenyList forbids every filesystem operation on paths that resolve under +// one of the supplied roots. Tokens follow the same expansion rules as +// [WithAllowList]. The deny-list takes precedence over the allow-list: a +// path that matches both is rejected. An empty or nil slice disables the +// deny-list. +func WithDenyList(roots []string) FileSystemOpt { + return func(t *FilesystemTool) { + set, err := newPathRootSet(t.workingDir, roots) + if err != nil { + slog.Warn("filesystem deny-list: invalid entry; ignoring deny-list", "error", err) + return + } + t.denyList = set + } +} + func NewFilesystemTool(workingDir string, opts ...FileSystemOpt) *FilesystemTool { t := &FilesystemTool{ workingDir: workingDir, @@ -83,12 +133,20 @@ func NewFilesystemTool(workingDir string, opts ...FileSystemOpt) *FilesystemTool } func (t *FilesystemTool) Instructions() string { - return `## Filesystem Tools + var b strings.Builder + b.WriteString(`## Filesystem Tools - Relative paths resolve from the working directory; absolute paths and ".." work as expected - Prefer read_multiple_files over sequential read_file calls - Use search_files_content to locate code or text across files -- Use exclude patterns in searches and max_depth in directory_tree to limit output` +- Use exclude patterns in searches and max_depth in directory_tree to limit output`) + if d := t.allowList.describe(); d != "" { + fmt.Fprintf(&b, "\n- These tools are restricted to paths under: %s. Any other path is rejected without touching the filesystem.", d) + } + if d := t.denyList.describe(); d != "" { + fmt.Fprintf(&b, "\n- These tools must not access paths under: %s. Such paths are rejected without touching the filesystem.", d) + } + return b.String() } type DirectoryTreeArgs struct { @@ -439,6 +497,10 @@ func (t *FilesystemTool) executePostEditCommands(ctx context.Context, filePath s // resolvePath resolves a path relative to the working directory. // Relative paths (including ".") are joined with the working directory. // Absolute paths and paths starting with ".." are used as-is. +// +// resolvePath does NOT enforce the allow- or deny-lists; callers should use +// [resolveAndCheckPath] when those checks are required (i.e. for any path +// that originates from a tool argument). func (t *FilesystemTool) resolvePath(path string) string { if filepath.IsAbs(path) { return filepath.Clean(path) @@ -447,6 +509,57 @@ func (t *FilesystemTool) resolvePath(path string) string { return filepath.Clean(filepath.Join(t.workingDir, path)) } +// resolveAndCheckPath is the canonical entry point used by every filesystem +// handler that operates on a user-supplied path. It resolves the path against +// the working directory and validates the result against the allow- and +// deny-lists. The returned string is the on-disk path the handler should pass +// to the os.* call. +// +// When neither list is configured the function is functionally equivalent to +// [resolvePath]. When a list is configured, symlinks are resolved before the +// containment check so that a symlink inside an allowed directory cannot leak +// access outside it. Paths that don't exist yet (e.g. for write_file or +// create_directory) are checked against their nearest existing ancestor so +// the caller can still create them. +// +// The deny-list takes precedence over the allow-list: a path that matches +// both is rejected. +// +// SECURITY NOTE: there is a small TOCTOU window between this check and the +// subsequent os.* call. A concurrent process running as the same user could +// in principle replace a directory with a symlink between the two and cause +// the I/O to escape the allowed area. This is acceptable because: +// +// - The intended threat model is the LLM itself — not another local +// process. The LLM exercises the toolset only through the exposed +// handlers (read/write/list/etc.) and has no symlink-creation primitive, +// so it cannot win this race from inside the agent. +// - The toolset still defends against the static cases that matter: pre- +// existing symlinks, ".." traversals, absolute paths outside the allow- +// list. Those are checked here AND verified through [*os.Root].Lstat +// when an [*os.Root] is available. +// +// Callers wanting full TOCTOU safety should perform their I/O through the +// [*os.Root] handles owned by [pathRootSet] rather than via os.* on the +// returned path. +func (t *FilesystemTool) resolveAndCheckPath(path string) (string, error) { + resolved := t.resolvePath(path) + if t.allowList == nil && t.denyList == nil { + return resolved, nil + } + realPath, err := resolveRealPath(resolved) + if err != nil { + return "", fmt.Errorf("resolving %q: %w", path, err) + } + if t.denyList != nil && t.denyList.contains(realPath) { + return "", fmt.Errorf("path %q is inside a denied directory (%s)", path, t.denyList.describe()) + } + if t.allowList != nil && !t.allowList.contains(realPath) { + return "", fmt.Errorf("path %q is outside the allowed directories (%s)", path, t.allowList.describe()) + } + return resolved, nil +} + // initGitignoreMatcher initializes the gitignore matcher for the working directory. // It is safe to call multiple times; initialization only happens once. func (t *FilesystemTool) initGitignoreMatcher() { @@ -492,7 +605,10 @@ func (t *FilesystemTool) shouldIgnorePath(path string) bool { // Handler implementations func (t *FilesystemTool) handleDirectoryTree(ctx context.Context, args DirectoryTreeArgs) (*tools.ToolCallResult, error) { - resolvedPath := t.resolvePath(args.Path) + resolvedPath, err := t.resolveAndCheckPath(args.Path) + if err != nil { + return tools.ResultError(err.Error()), nil + } tree, err := fsx.DirectoryTree(ctx, resolvedPath, allowAllPaths, t.shouldIgnorePath, maxFiles) if err != nil { @@ -554,7 +670,10 @@ func (t *FilesystemTool) editFileHandler() tools.ToolHandler { } func (t *FilesystemTool) handleEditFile(ctx context.Context, args EditFileArgs) (*tools.ToolCallResult, error) { - resolvedPath := t.resolvePath(args.Path) + resolvedPath, err := t.resolveAndCheckPath(args.Path) + if err != nil { + return tools.ResultError(err.Error()), nil + } content, err := os.ReadFile(resolvedPath) if err != nil { @@ -589,7 +708,10 @@ func (t *FilesystemTool) handleEditFile(ctx context.Context, args EditFileArgs) } func (t *FilesystemTool) handleListDirectory(_ context.Context, args ListDirectoryArgs) (*tools.ToolCallResult, error) { - resolvedPath := t.resolvePath(args.Path) + resolvedPath, err := t.resolveAndCheckPath(args.Path) + if err != nil { + return tools.ResultError(err.Error()), nil + } entries, err := os.ReadDir(resolvedPath) if err != nil { @@ -627,7 +749,14 @@ func (t *FilesystemTool) handleListDirectory(_ context.Context, args ListDirecto } func (t *FilesystemTool) handleReadFile(_ context.Context, args ReadFileArgs) (*tools.ToolCallResult, error) { - resolvedPath := t.resolvePath(args.Path) + resolvedPath, err := t.resolveAndCheckPath(args.Path) + if err != nil { + return &tools.ToolCallResult{ + Output: err.Error(), + IsError: true, + Meta: ReadFileMeta{Path: args.Path, Error: err.Error()}, + }, nil + } // Check if the file exists before any type detection. info, err := os.Stat(resolvedPath) @@ -741,7 +870,17 @@ func (t *FilesystemTool) handleReadMultipleFiles(ctx context.Context, args ReadM entry := ReadFileMeta{Path: path} - resolvedPath := t.resolvePath(path) + resolvedPath, err := t.resolveAndCheckPath(path) + if err != nil { + errMsg := err.Error() + contents = append(contents, PathContent{ + Path: path, + Content: errMsg, + }) + entry.Error = errMsg + meta.Files = append(meta.Files, entry) + continue + } content, err := os.ReadFile(resolvedPath) if err != nil { @@ -790,7 +929,10 @@ func (t *FilesystemTool) handleReadMultipleFiles(ctx context.Context, args ReadM } func (t *FilesystemTool) handleSearchFilesContent(_ context.Context, args SearchFilesContentArgs) (*tools.ToolCallResult, error) { - resolvedPath := t.resolvePath(args.Path) + resolvedPath, err := t.resolveAndCheckPath(args.Path) + if err != nil { + return tools.ResultError(err.Error()), nil + } var regex *regexp.Regexp if args.IsRegex { @@ -804,7 +946,7 @@ func (t *FilesystemTool) handleSearchFilesContent(_ context.Context, args Search var results []string filesWithMatches := make(map[string]struct{}) - err := filepath.WalkDir(resolvedPath, func(path string, d fs.DirEntry, err error) error { + err = filepath.WalkDir(resolvedPath, func(path string, d fs.DirEntry, err error) error { if err != nil { return nil } @@ -899,7 +1041,10 @@ func (t *FilesystemTool) handleSearchFilesContent(_ context.Context, args Search } func (t *FilesystemTool) handleWriteFile(ctx context.Context, args WriteFileArgs) (*tools.ToolCallResult, error) { - resolvedPath := t.resolvePath(args.Path) + resolvedPath, err := t.resolveAndCheckPath(args.Path) + if err != nil { + return tools.ResultError(err.Error()), nil + } // Create parent directory structure if it doesn't exist dir := filepath.Dir(resolvedPath) @@ -921,7 +1066,10 @@ func (t *FilesystemTool) handleWriteFile(ctx context.Context, args WriteFileArgs func (t *FilesystemTool) handleCreateDirectory(_ context.Context, args CreateDirectoryArgs) (*tools.ToolCallResult, error) { var results []string for _, path := range args.Paths { - resolvedPath := t.resolvePath(path) + resolvedPath, err := t.resolveAndCheckPath(path) + if err != nil { + return tools.ResultError(err.Error()), nil + } if err := os.MkdirAll(resolvedPath, 0o755); err != nil { return tools.ResultError(fmt.Sprintf("Error creating directory %s: %s", path, err)), nil } @@ -934,7 +1082,10 @@ func (t *FilesystemTool) handleCreateDirectory(_ context.Context, args CreateDir func (t *FilesystemTool) handleRemoveDirectory(_ context.Context, args RemoveDirectoryArgs) (*tools.ToolCallResult, error) { var results []string for _, path := range args.Paths { - resolvedPath := t.resolvePath(path) + resolvedPath, err := t.resolveAndCheckPath(path) + if err != nil { + return tools.ResultError(err.Error()), nil + } if err := rmdir(resolvedPath); err != nil { return tools.ResultError(fmt.Sprintf("Error removing directory %s: %s", path, err)), nil diff --git a/pkg/tools/builtin/filesystem_paths.go b/pkg/tools/builtin/filesystem_paths.go new file mode 100644 index 000000000..8d042ac30 --- /dev/null +++ b/pkg/tools/builtin/filesystem_paths.go @@ -0,0 +1,261 @@ +package builtin + +import ( + "errors" + "fmt" + "io/fs" + "log/slog" + "os" + "path/filepath" + "strings" +) + +// pathRootSet is a set of filesystem roots, used to back the filesystem +// toolset's allow- and deny-lists. Each entry is expanded once at construction +// time (token "." -> working directory, token "~" or leading "~/" -> user +// home directory, environment variables -> their values), then resolved to an +// absolute, symlink-free real path. +// +// An optional [*os.Root] is opened per entry. When available, it is used to +// confirm path containment via the kernel's rooted-lookup semantics, which +// reject symlink and ".." escapes regardless of the on-disk layout +// (TOCTOU-safe). Falls back to a lexical prefix check when [*os.Root] cannot +// be opened (root does not exist yet, restricted permissions, …). +type pathRootSet struct { + entries []pathRoot +} + +type pathRoot struct { + // raw is the original, un-expanded entry from the user configuration. + // Kept for error messages so that violations report what the user wrote. + raw string + // real is the absolute path with all symlinks resolved. May equal the + // expanded path when the entry does not yet exist; in that case root is + // nil and we fall back to a lexical prefix check. + real string + // root is an [*os.Root] handle for real, lazily set when [os.OpenRoot] + // succeeds. Used to make containment checks TOCTOU-safe. + root *os.Root +} + +// newPathRootSet expands the supplied tokens against workingDir and returns a +// pathRootSet. Returns nil for an empty input — callers should treat a nil +// set as "no constraint applies". +// +// Recognised tokens: +// - "." resolves to workingDir. +// - "~" or a leading "~/" resolves against the user's home directory. +// - "$VAR" / "${VAR}" expands environment variables. +// - Any other relative path is resolved against workingDir. +// - Absolute paths are kept as-is. +func newPathRootSet(workingDir string, tokens []string) (*pathRootSet, error) { + if len(tokens) == 0 { + return nil, nil + } + set := &pathRootSet{} + seen := make(map[string]struct{}, len(tokens)) + for _, token := range tokens { + entry, err := newPathRoot(workingDir, token) + if err != nil { + // Release any roots opened so far before returning. The toolset + // constructor swallows the error and falls back to "no list", + // so leaving open file descriptors here would leak silently. + set.close() + return nil, err + } + if _, dup := seen[entry.real]; dup { + // Duplicate of an earlier entry: release the *os.Root we just + // opened to avoid leaking the file descriptor. + if entry.root != nil { + _ = entry.root.Close() + } + continue + } + seen[entry.real] = struct{}{} + set.entries = append(set.entries, entry) + } + return set, nil +} + +// newPathRoot resolves a single token to a pathRoot. Errors when the token +// is empty (before or after env-var expansion) or its home/working directory +// expansion fails. Non-existent target directories are accepted (root is +// left nil and we fall back to the lexical check) so that the agent can +// still operate when, e.g., "~/projects" hasn't been created yet. +func newPathRoot(workingDir, token string) (pathRoot, error) { + expanded, err := expandPathToken(workingDir, token) + if err != nil { + return pathRoot{}, fmt.Errorf("%s: %w", token, err) + } + + abs, err := filepath.Abs(expanded) + if err != nil { + return pathRoot{}, fmt.Errorf("resolving %q: %w", token, err) + } + + realPath, err := resolveRealPath(abs) + if err != nil { + return pathRoot{}, fmt.Errorf("resolving %q: %w", token, err) + } + + entry := pathRoot{raw: token, real: realPath} + if root, err := os.OpenRoot(realPath); err == nil { + entry.root = root + } else if !errors.Is(err, fs.ErrNotExist) { + // Log unexpected failures but don't error out: lexical containment + // is still enforced below. + slog.Debug("filesystem allow/deny: os.OpenRoot failed; falling back to lexical check", + "path", realPath, "error", err) + } + return entry, nil +} + +// expandPathToken resolves "." / "~" / "$VAR" tokens and joins relative paths +// with workingDir. It does not resolve symlinks or canonicalise the result. +// +// Returns an error when the token is empty before OR after environment +// variable expansion. The post-expansion check matters because +// [os.ExpandEnv] silently substitutes undefined variables with the empty +// string — without rejection, an unset "$NOPE" entry would resolve to the +// working directory and silently widen an allow-list (or close a deny-list) +// in surprising ways. +func expandPathToken(workingDir, token string) (string, error) { + // Trim spaces but keep internal whitespace untouched (some macOS paths + // contain spaces, e.g. "~/Library/Application Support"). + original := strings.TrimSpace(token) + if original == "" { + return "", errors.New("path entry must not be empty") + } + token = os.ExpandEnv(original) + if strings.TrimSpace(token) == "" { + return "", fmt.Errorf("path entry %q expands to an empty string (undefined environment variable?)", original) + } + + switch { + case token == ".": + if workingDir == "" { + return os.Getwd() + } + return workingDir, nil + case token == "~": + return os.UserHomeDir() + case strings.HasPrefix(token, "~/") || strings.HasPrefix(token, "~"+string(filepath.Separator)): + home, err := os.UserHomeDir() + if err != nil { + return "", err + } + return filepath.Join(home, token[2:]), nil + case filepath.IsAbs(token): + return token, nil + default: + if workingDir == "" { + return token, nil + } + return filepath.Join(workingDir, token), nil + } +} + +// contains reports whether absPath is inside any of the roots in the set. +// absPath must be absolute and symlink-resolved (see resolveRealPath). +// +// For each root, we first do a lexical check (filepath.Rel) to short-circuit +// the obvious "outside" cases. When the path is lexically inside the root and +// an [*os.Root] is available, we additionally probe the path through the +// rooted handle: the kernel will reject any access that escapes the root via +// "..", an absolute symlink, or a relative symlink that climbs above the +// boundary, regardless of timing or on-disk changes. Non-existent paths are +// accepted (the caller may be about to create them) — a subsequent write +// goes through [resolveAndCheckPath] again before any I/O. +func (rs *pathRootSet) contains(absPath string) bool { + if rs == nil { + return false + } + for i := range rs.entries { + entry := &rs.entries[i] + rel, err := filepath.Rel(entry.real, absPath) + if err != nil { + continue + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + continue // lexically outside this root + } + // Lexically inside. If we have an [*os.Root] for this entry, confirm + // containment with a kernel-enforced rooted lookup. This catches the + // case where the lexical path is inside the root but a symlink along + // the way escapes it (e.g. workingDir/link -> /etc/passwd). + if entry.root == nil || rel == "." { + return true + } + if _, err := entry.root.Lstat(filepath.ToSlash(rel)); err == nil { + return true + } else if errors.Is(err, fs.ErrNotExist) { + // The path doesn't exist yet but every existing ancestor we + // looked at is inside the root, so creating it inside the + // rooted handle would also stay contained. Accept. + return true + } else { + // e.g. ELOOP from a symlink that escapes the root: treat as + // outside. + slog.Debug("filesystem allow/deny: rooted Lstat rejected path", + "root", entry.real, "rel", rel, "error", err) + continue + } + } + return false +} + +// describe returns a comma-separated, human-readable list of the entries. +// Used in error messages and tool instructions. +func (rs *pathRootSet) describe() string { + if rs == nil || len(rs.entries) == 0 { + return "" + } + parts := make([]string, len(rs.entries)) + for i, e := range rs.entries { + parts[i] = e.raw + } + return strings.Join(parts, ", ") +} + +// close releases any [*os.Root] handles owned by the set. Safe to call on a +// nil receiver. +func (rs *pathRootSet) close() { + if rs == nil { + return + } + for i := range rs.entries { + if rs.entries[i].root != nil { + _ = rs.entries[i].root.Close() + rs.entries[i].root = nil + } + } +} + +// resolveRealPath returns the absolute, symlink-resolved form of p. If p does +// not (yet) exist, it walks up to the nearest existing ancestor, resolves +// symlinks on that ancestor, and re-appends the missing tail. This lets the +// allow/deny check work for paths that are about to be created (write_file, +// create_directory, …) without falsely accepting a path that would, once +// created, escape the boundary. +func resolveRealPath(p string) (string, error) { + abs, err := filepath.Abs(p) + if err != nil { + return "", err + } + abs = filepath.Clean(abs) + if realPath, err := filepath.EvalSymlinks(abs); err == nil { + return realPath, nil + } else if !errors.Is(err, fs.ErrNotExist) { + return "", err + } + // Walk up to find the nearest existing ancestor. + parent := filepath.Dir(abs) + if parent == abs { + return abs, nil + } + realParent, err := resolveRealPath(parent) + if err != nil { + return "", err + } + return filepath.Join(realParent, filepath.Base(abs)), nil +} diff --git a/pkg/tools/builtin/filesystem_paths_test.go b/pkg/tools/builtin/filesystem_paths_test.go new file mode 100644 index 000000000..a43af82aa --- /dev/null +++ b/pkg/tools/builtin/filesystem_paths_test.go @@ -0,0 +1,523 @@ +package builtin + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// resetHomeDir overrides $HOME for the duration of the test (and also +// $USERPROFILE on Windows, which os.UserHomeDir falls back to). The original +// values are restored when the test ends. +func resetHomeDir(t *testing.T, dir string) { + t.Helper() + t.Setenv("HOME", dir) + t.Setenv("USERPROFILE", dir) +} + +func TestFilesystemTool_DefaultIsUnrestricted(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + tool := NewFilesystemTool(tmpDir) + + // No allow_list, no deny_list: everything resolvable goes through. + resolved, err := tool.resolveAndCheckPath("/etc/hosts") + require.NoError(t, err) + assert.Equal(t, "/etc/hosts", resolved) + + resolved, err = tool.resolveAndCheckPath("../../some/escape") + require.NoError(t, err) + // Equivalent to filepath.Clean of the joined relative escape. + want := filepath.Clean(filepath.Join(tmpDir, "..", "..", "some", "escape")) + assert.Equal(t, want, resolved) +} + +func TestFilesystemTool_AllowList_DotMeansWorkingDir(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + tool := NewFilesystemTool(tmpDir, WithAllowList([]string{"."})) + + // Inside working dir is fine. + _, err := tool.resolveAndCheckPath("file.txt") + require.NoError(t, err) + + _, err = tool.resolveAndCheckPath("subdir/nested/file.txt") + require.NoError(t, err) + + // Outside working dir is rejected. + _, err = tool.resolveAndCheckPath("/etc/hosts") + require.Error(t, err) + assert.Contains(t, err.Error(), "outside the allowed directories") + + // `..` traversals that escape the working dir are rejected. + _, err = tool.resolveAndCheckPath("../escape.txt") + require.Error(t, err) +} + +func TestFilesystemTool_AllowList_TildeMeansHome(t *testing.T) { + homeDir := t.TempDir() + resetHomeDir(t, homeDir) + wd := t.TempDir() + + tool := NewFilesystemTool(wd, WithAllowList([]string{"~"})) + + // A path under $HOME is allowed via ~/... + resolved, err := tool.resolveAndCheckPath(filepath.Join(homeDir, "doc.md")) + require.NoError(t, err) + assert.Equal(t, filepath.Join(homeDir, "doc.md"), resolved) + + // Working directory is NOT allowed (only ~ was listed). + _, err = tool.resolveAndCheckPath("file.txt") + require.Error(t, err) + assert.Contains(t, err.Error(), "outside the allowed directories") +} + +func TestFilesystemTool_AllowList_TildeSubdirectory(t *testing.T) { + homeDir := t.TempDir() + resetHomeDir(t, homeDir) + require.NoError(t, os.MkdirAll(filepath.Join(homeDir, "projects"), 0o755)) + wd := t.TempDir() + + tool := NewFilesystemTool(wd, WithAllowList([]string{"~/projects"})) + + // Inside the listed subdir. + _, err := tool.resolveAndCheckPath(filepath.Join(homeDir, "projects", "app", "main.go")) + require.NoError(t, err) + + // $HOME itself is NOT inside ~/projects. + _, err = tool.resolveAndCheckPath(filepath.Join(homeDir, "doc.md")) + require.Error(t, err) + + // Sibling directory is rejected. + _, err = tool.resolveAndCheckPath(filepath.Join(homeDir, "documents", "doc.md")) + require.Error(t, err) +} + +func TestFilesystemTool_AllowList_MultipleRoots(t *testing.T) { + t.Parallel() + wd := t.TempDir() + otherDir := t.TempDir() + + tool := NewFilesystemTool(wd, WithAllowList([]string{".", otherDir})) + + _, err := tool.resolveAndCheckPath("file.txt") + require.NoError(t, err) + + _, err = tool.resolveAndCheckPath(filepath.Join(otherDir, "file.txt")) + require.NoError(t, err) + + _, err = tool.resolveAndCheckPath("/etc/hosts") + require.Error(t, err) +} + +func TestFilesystemTool_AllowList_AbsolutePath(t *testing.T) { + t.Parallel() + wd := t.TempDir() + allowed := t.TempDir() + + tool := NewFilesystemTool(wd, WithAllowList([]string{allowed})) + + // Absolute path inside the allowed root is fine. + _, err := tool.resolveAndCheckPath(filepath.Join(allowed, "x", "y.txt")) + require.NoError(t, err) + + // Absolute path outside is rejected. + _, err = tool.resolveAndCheckPath("/etc/hosts") + require.Error(t, err) +} + +func TestFilesystemTool_DenyList_RejectsMatchingPaths(t *testing.T) { + t.Parallel() + wd := t.TempDir() + denied := filepath.Join(wd, "secret") + require.NoError(t, os.Mkdir(denied, 0o755)) + + tool := NewFilesystemTool(wd, WithDenyList([]string{"secret"})) + + // Anything under the denied subtree is rejected. + _, err := tool.resolveAndCheckPath("secret/key.pem") + require.Error(t, err) + assert.Contains(t, err.Error(), "denied directory") + + // Sibling files are still reachable. + _, err = tool.resolveAndCheckPath("public.md") + require.NoError(t, err) + + // And — because no allow-list is set — paths outside the working dir + // are still allowed (deny-only configurations preserve broad access). + _, err = tool.resolveAndCheckPath("/etc/hosts") + require.NoError(t, err) +} + +func TestFilesystemTool_DenyList_TakesPrecedenceOverAllowList(t *testing.T) { + t.Parallel() + wd := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(wd, "src"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(wd, "src", "vendor"), 0o755)) + + tool := NewFilesystemTool(wd, + WithAllowList([]string{"."}), + WithDenyList([]string{"src/vendor"})) + + // Allowed by allow-list, not denied. + _, err := tool.resolveAndCheckPath("src/main.go") + require.NoError(t, err) + + // Denied even though it's inside the allow-list. + _, err = tool.resolveAndCheckPath("src/vendor/lib.go") + require.Error(t, err) + assert.Contains(t, err.Error(), "denied directory") +} + +func TestFilesystemTool_AllowList_SymlinkEscapeRejected(t *testing.T) { + t.Parallel() + wd := t.TempDir() + target := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(target, "secret.txt"), []byte("nope"), 0o644)) + + // Plant a symlink inside the working dir that points outside. + link := filepath.Join(wd, "escape") + require.NoError(t, os.Symlink(target, link)) + + tool := NewFilesystemTool(wd, WithAllowList([]string{"."})) + + // Following the symlink escapes the allow-list and must be rejected. + _, err := tool.resolveAndCheckPath("escape/secret.txt") + require.Error(t, err) + assert.Contains(t, err.Error(), "outside the allowed directories") +} + +func TestFilesystemTool_DenyList_SymlinkIntoDeniedAreaRejected(t *testing.T) { + t.Parallel() + wd := t.TempDir() + denied := filepath.Join(wd, "secret") + require.NoError(t, os.Mkdir(denied, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(denied, "key.pem"), []byte("nope"), 0o644)) + + // Symlink that lives outside the denied directory but points into it. + link := filepath.Join(wd, "shortcut") + require.NoError(t, os.Symlink(denied, link)) + + tool := NewFilesystemTool(wd, WithDenyList([]string{"secret"})) + + // Reading via the symlink must still trigger the deny-list. + _, err := tool.resolveAndCheckPath("shortcut/key.pem") + require.Error(t, err) + assert.Contains(t, err.Error(), "denied directory") +} + +func TestFilesystemTool_AllowList_NewFilePath(t *testing.T) { + t.Parallel() + wd := t.TempDir() + tool := NewFilesystemTool(wd, WithAllowList([]string{"."})) + + // A path that doesn't exist yet (e.g. about to be created by write_file) + // must still be accepted when its lexical location is inside the allow-list. + _, err := tool.resolveAndCheckPath("new/dir/output.txt") + require.NoError(t, err) + + // But if the new path's parent escapes the allow-list (via ..) it's + // rejected. + _, err = tool.resolveAndCheckPath("../new.txt") + require.Error(t, err) +} + +func TestFilesystemTool_AllowList_EmptyDisablesCheck(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + + // nil and empty slice both leave the allow-list disabled. + for _, roots := range [][]string{nil, {}} { + tool := NewFilesystemTool(tmpDir, WithAllowList(roots)) + _, err := tool.resolveAndCheckPath("/etc/hosts") + require.NoError(t, err, "empty/nil allow-list must not constrain") + } +} + +func TestFilesystemTool_HandlersUseAllowList(t *testing.T) { + t.Parallel() + wd := t.TempDir() + other := t.TempDir() + + // Pre-populate a file outside the working dir. + outsideFile := filepath.Join(other, "outside.txt") + require.NoError(t, os.WriteFile(outsideFile, []byte("nope"), 0o644)) + + tool := NewFilesystemTool(wd, WithAllowList([]string{"."})) + + // read_file: must refuse the outside path. + res, err := tool.handleReadFile(t.Context(), ReadFileArgs{Path: outsideFile}) + require.NoError(t, err) + assert.True(t, res.IsError) + assert.Contains(t, res.Output, "outside the allowed directories") + + // write_file: must refuse to write outside, and must NOT create the file. + res, err = tool.handleWriteFile(t.Context(), WriteFileArgs{ + Path: filepath.Join(other, "should-not-exist.txt"), + Content: "x", + }) + require.NoError(t, err) + assert.True(t, res.IsError) + assert.NoFileExists(t, filepath.Join(other, "should-not-exist.txt")) + + // list_directory: must refuse the outside path. + res, err = tool.handleListDirectory(t.Context(), ListDirectoryArgs{Path: other}) + require.NoError(t, err) + assert.True(t, res.IsError) + + // search_files_content: must refuse the outside path. + res, err = tool.handleSearchFilesContent(t.Context(), SearchFilesContentArgs{ + Path: other, + Query: "nope", + }) + require.NoError(t, err) + assert.True(t, res.IsError) + + // directory_tree: must refuse the outside path. + res, err = tool.handleDirectoryTree(t.Context(), DirectoryTreeArgs{Path: other}) + require.NoError(t, err) + assert.True(t, res.IsError) + + // create_directory: must refuse, and must not create the directory. + res, err = tool.handleCreateDirectory(t.Context(), CreateDirectoryArgs{ + Paths: []string{filepath.Join(other, "newdir")}, + }) + require.NoError(t, err) + assert.True(t, res.IsError) + assert.NoDirExists(t, filepath.Join(other, "newdir")) + + // remove_directory: must refuse to operate on the outside path. + require.NoError(t, os.Mkdir(filepath.Join(other, "keep"), 0o755)) + res, err = tool.handleRemoveDirectory(t.Context(), RemoveDirectoryArgs{ + Paths: []string{filepath.Join(other, "keep")}, + }) + require.NoError(t, err) + assert.True(t, res.IsError) + assert.DirExists(t, filepath.Join(other, "keep")) + + // read_multiple_files: per-path errors don't fail the whole call but + // each rejected path is reported in the output. + require.NoError(t, os.WriteFile(filepath.Join(wd, "ok.txt"), []byte("ok"), 0o644)) + res, err = tool.handleReadMultipleFiles(t.Context(), ReadMultipleFilesArgs{ + Paths: []string{"ok.txt", outsideFile}, + }) + require.NoError(t, err) + assert.Contains(t, res.Output, "ok") // the legal one was read + assert.Contains(t, res.Output, "outside the allowed directories") +} + +func TestFilesystemTool_HandlersUseDenyList(t *testing.T) { + t.Parallel() + wd := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(wd, "secrets"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(wd, "secrets", "key.pem"), []byte("k"), 0o644)) + + tool := NewFilesystemTool(wd, WithDenyList([]string{"secrets"})) + + // edit_file: must refuse to read the file in a denied directory. + res, err := tool.handleEditFile(t.Context(), EditFileArgs{ + Path: "secrets/key.pem", + Edits: []Edit{{OldText: "k", NewText: "tampered"}}, + }) + require.NoError(t, err) + assert.True(t, res.IsError) + assert.Contains(t, res.Output, "denied directory") + // The file content must not have been modified. + got, err := os.ReadFile(filepath.Join(wd, "secrets", "key.pem")) + require.NoError(t, err) + assert.Equal(t, "k", string(got)) +} + +func TestFilesystemTool_Instructions_MentionsRestrictions(t *testing.T) { + t.Parallel() + wd := t.TempDir() + + // Default instructions: no restriction text. + plain := NewFilesystemTool(wd).Instructions() + assert.NotContains(t, plain, "restricted") + assert.NotContains(t, plain, "must not access") + + // With an allow-list: instructions mention the restriction. + allowed := NewFilesystemTool(wd, WithAllowList([]string{".", "~"})).Instructions() + assert.Contains(t, allowed, "restricted") + assert.Contains(t, allowed, ".") + assert.Contains(t, allowed, "~") + + // With a deny-list: instructions mention the deny entries. + denied := NewFilesystemTool(wd, WithDenyList([]string{"~/.ssh"})).Instructions() + assert.Contains(t, denied, "must not access") + assert.Contains(t, denied, "~/.ssh") +} + +func TestExpandPathToken(t *testing.T) { + homeDir := t.TempDir() + resetHomeDir(t, homeDir) + wd := t.TempDir() + t.Setenv("MY_VAR", "/var/data") + t.Setenv("EMPTY_VAR", "") + os.Unsetenv("DEFINITELY_NOT_SET") + + tests := []struct { + name string + token string + want string + wantErr string // substring match; empty = no error + }{ + {name: "dot", token: ".", want: wd}, + {name: "tilde", token: "~", want: homeDir}, + {name: "tilde-subdir", token: "~/projects", want: filepath.Join(homeDir, "projects")}, + {name: "absolute", token: "/srv/data", want: "/srv/data"}, + {name: "relative", token: "src", want: filepath.Join(wd, "src")}, + {name: "env-var", token: "$MY_VAR", want: "/var/data"}, + {name: "env-var-braces", token: "${MY_VAR}", want: "/var/data"}, + {name: "env-var-inside-tilde", token: "~/${MY_VAR}", want: filepath.Join(homeDir, "var", "data")}, + {name: "empty", token: "", wantErr: "empty"}, + {name: "whitespace", token: " ", wantErr: "empty"}, + // Regression: an undefined env var must NOT silently expand to the + // working directory — a typo in the var name would otherwise grant + // (or close) access to the entire working dir. + {name: "undefined-env-var", token: "$DEFINITELY_NOT_SET", wantErr: "empty string"}, + {name: "defined-but-empty-env-var", token: "$EMPTY_VAR", wantErr: "empty string"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := expandPathToken(wd, tc.token) + if tc.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestWithAllowList_RejectsUndefinedEnvVar(t *testing.T) { + // Regression test for the same bug at the toolset boundary: a typo in + // an env-var name in allow_list must NOT silently grant access to the + // working directory. WithAllowList logs+drops invalid entries (no + // error returned), which would let an attacker exploit the typo. We + // instead want the allow-list to remain disabled ("no constraint"), + // not silently expanded to the working dir. + os.Unsetenv("DEFINITELY_NOT_SET") + wd := t.TempDir() + tool := NewFilesystemTool(wd, WithAllowList([]string{"$DEFINITELY_NOT_SET"})) + + // The allow-list construction failed silently, so the toolset is + // unrestricted (default behaviour) instead of being silently scoped + // to the working dir. Verify by reaching outside the working dir. + _, err := tool.resolveAndCheckPath("/etc/hosts") + require.NoError(t, err, "undefined env var must not silently scope allow-list to working dir") +} + +func TestWithAllowList_AcceptsDefinedEnvVar(t *testing.T) { + wd := t.TempDir() + allowed := t.TempDir() + t.Setenv("ALLOWED_DIR", allowed) + + tool := NewFilesystemTool(wd, WithAllowList([]string{"$ALLOWED_DIR"})) + + // Inside the env-var-resolved root. + _, err := tool.resolveAndCheckPath(filepath.Join(allowed, "file.txt")) + require.NoError(t, err) + + // Outside is rejected — confirms the allow-list is actually active. + _, err = tool.resolveAndCheckPath("/etc/hosts") + require.Error(t, err) +} + +func TestDenyList_NonExistentPath(t *testing.T) { + // A common usage: deny ~/.ssh on a system that does not have a ~/.ssh + // yet. The deny-list must still apply when the directory is created + // after the toolset is constructed. + homeDir := t.TempDir() + resetHomeDir(t, homeDir) + wd := t.TempDir() + + tool := NewFilesystemTool(wd, WithDenyList([]string{"~/.ssh"})) + + // ~/.ssh does not exist yet — a write to a path inside it must be + // rejected before the directory is even created. + _, err := tool.resolveAndCheckPath(filepath.Join(homeDir, ".ssh", "id_rsa")) + require.Error(t, err) + assert.Contains(t, err.Error(), "denied directory") + + // Sibling files in $HOME are still reachable. + _, err = tool.resolveAndCheckPath(filepath.Join(homeDir, "doc.md")) + require.NoError(t, err) + + // Now create ~/.ssh and re-test — still denied. + require.NoError(t, os.Mkdir(filepath.Join(homeDir, ".ssh"), 0o700)) + _, err = tool.resolveAndCheckPath(filepath.Join(homeDir, ".ssh", "id_rsa")) + require.Error(t, err) +} + +func TestPathRootSet_DeduplicatesEntries(t *testing.T) { + t.Parallel() + wd := t.TempDir() + + set, err := newPathRootSet(wd, []string{".", ".", wd, "subdir/.."}) + require.NoError(t, err) + require.NotNil(t, set) + // All four resolve to wd. + assert.Len(t, set.entries, 1) + // Only one *os.Root must be retained — duplicates' handles must be + // closed during construction to avoid an fd leak. + require.NotNil(t, set.entries[0].root) + set.close() +} + +func TestPathRootSet_InvalidEntryClosesEarlierRoots(t *testing.T) { + t.Parallel() + wd := t.TempDir() + + // First two entries are valid (they open *os.Root handles); the third + // is empty and triggers an error. The constructor must release the + // handles it opened so far rather than leaking them. + set, err := newPathRootSet(wd, []string{".", wd, ""}) + require.Error(t, err) + assert.Nil(t, set) +} + +func TestPathRootSet_NilForEmptyInput(t *testing.T) { + t.Parallel() + wd := t.TempDir() + + set, err := newPathRootSet(wd, nil) + require.NoError(t, err) + assert.Nil(t, set) + + set, err = newPathRootSet(wd, []string{}) + require.NoError(t, err) + assert.Nil(t, set) +} + +func TestPathRootSet_RejectsEmptyEntry(t *testing.T) { + t.Parallel() + wd := t.TempDir() + + _, err := newPathRootSet(wd, []string{".", ""}) + require.Error(t, err) + assert.Contains(t, err.Error(), "empty") +} + +func TestPathRootSet_OpensOSRootForSandboxing(t *testing.T) { + t.Parallel() + wd := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(wd, "ok"), 0o755)) + + set, err := newPathRootSet(wd, []string{"."}) + require.NoError(t, err) + require.NotNil(t, set) + require.Len(t, set.entries, 1) + // Bonus: the entry should hold an *os.Root for the existing directory, + // which is what gives us TOCTOU-safe containment checks. + assert.NotNil(t, set.entries[0].root, "expected an *os.Root for an existing directory") + + set.close() +}