diff --git a/agents/runtime.go b/agents/runtime.go index a47c1de..b541331 100644 --- a/agents/runtime.go +++ b/agents/runtime.go @@ -1094,7 +1094,13 @@ func buildToolRegistry( if cfg.Tools.FileOps.Enabled { reg.Register(toolfile.NewReadFile(cfg.Tools.FileOps)) - logger.Debug("registered file tool") + reg.Register(toolfile.NewListDirectory(cfg.Tools.FileOps)) + if !cfg.Tools.FileOps.ReadOnly { + reg.Register(toolfile.NewWriteFile(cfg.Tools.FileOps)) + reg.Register(toolfile.NewAppendFile(cfg.Tools.FileOps)) + reg.Register(toolfile.NewDeleteFile(cfg.Tools.FileOps)) + } + logger.Debug("registered file tools") } // current_time is always available diff --git a/dev/issues/README.md b/dev/issues/README.md index 813836e..7b3cb3b 100644 --- a/dev/issues/README.md +++ b/dev/issues/README.md @@ -41,5 +41,5 @@ afectados y notas de implementacion. | 28 | Desacoplar launcher | [0028-decouple-launcher.md](completed/0028-decouple-launcher.md) | completado | | 29 | Tests para runtime y config | [0029-core-tests.md](0029-core-tests.md) | pendiente | | 30 | Separacion Robot vs Agente | [0030-robot-vs-agent.md](0030-robot-vs-agent.md) | pendiente | -| 31 | Expandir tools/file/ | [0031-expand-file-tools.md](0031-expand-file-tools.md) | pendiente | +| 31 | Expandir tools/file/ | [0031-expand-file-tools.md](completed/0031-expand-file-tools.md) | completado | | 32 | E2E: verificar skill /create-agent | [0032-e2e-create-agent-skill.md](0032-e2e-create-agent-skill.md) | pendiente | diff --git a/dev/issues/completed/0031-expand-file-tools.md b/dev/issues/completed/0031-expand-file-tools.md new file mode 100644 index 0000000..b6a0145 --- /dev/null +++ b/dev/issues/completed/0031-expand-file-tools.md @@ -0,0 +1,32 @@ +# 0031 — Expandir tools/file/ con write, list, append, delete + +## Objetivo + +Ampliar el paquete `tools/file/` con operaciones de escritura, listado, append y borrado. Mantener el patron deny-by-default, validacion de symlinks, y respetar el flag `read_only` del config. + +## Estado: completado + +Implementado en rama `issue/0031-expand-file-tools`. + +### Archivos creados/modificados + +- `tools/file/validate.go` — NEW: validatePath(), validateWritePath(), resolveReal() extraidos de file.go +- `tools/file/write.go` — NEW: write_file tool (crea/sobreescribe, MkdirAll, limite 1MB) +- `tools/file/list.go` — NEW: list_directory tool (plano/recursivo, limite 500 entries) +- `tools/file/append.go` — NEW: append_file tool (append o crear, limite 10MB total) +- `tools/file/delete.go` — NEW: delete_file tool (solo archivos, nunca directorios) +- `tools/file/file.go` — refactored: removidas funciones movidas a validate.go +- `tools/file/write_test.go` — NEW: 11 tests +- `tools/file/list_test.go` — NEW: 9 tests +- `tools/file/append_test.go` — NEW: 11 tests +- `tools/file/delete_test.go` — NEW: 9 tests +- `agents/runtime.go` — registro condicional de las 4 tools nuevas + +### Seguridad + +- Deny-by-default en todas las tools (AllowedPaths vacio = todo denegado) +- ReadOnly gate: write/append/delete solo se registran si ReadOnly == false +- Path traversal protegido via resolveReal() + prefix validation +- Symlink escape protegido via EvalSymlinks +- Solo archivos en delete (nunca directorios) +- Limites de tamano: 1MB write, 10MB append total, 64KB read output, 500 entries list diff --git a/tools/file/append.go b/tools/file/append.go new file mode 100644 index 0000000..065bfee --- /dev/null +++ b/tools/file/append.go @@ -0,0 +1,83 @@ +package file + +import ( + "context" + "fmt" + "os" + "path/filepath" + + "github.com/enmanuel/agents/internal/config" + "github.com/enmanuel/agents/tools" +) + +// maxAppendTotal is the maximum total file size after appending (10 MB). +const maxAppendTotal = 10 * 1024 * 1024 + +// NewAppendFile creates an append_file tool that appends content to a local file. +// Deny-by-default: if AllowedPaths is empty, all operations are rejected. +// Rejects if ReadOnly is true. Creates the file (and parent directories) if it does not exist. +func NewAppendFile(cfg config.FileOpsCfg) tools.Tool { + return tools.Tool{ + Def: tools.Def{ + Name: "append_file", + Description: "Append content to the end of a local file. Creates the file if it does not exist.", + Parameters: []tools.Param{ + {Name: "path", Type: "string", Description: "Absolute path to the file to append to", Required: true}, + {Name: "content", Type: "string", Description: "Content to append to the file", Required: true}, + }, + }, + Exec: func(ctx context.Context, args map[string]any) tools.Result { + path := tools.GetString(args, "path") + if path == "" { + return tools.Result{Err: fmt.Errorf("append_file: path is required")} + } + + content := tools.GetString(args, "content") + if content == "" { + return tools.Result{Err: fmt.Errorf("append_file: content is required")} + } + + absPath, err := filepath.Abs(path) + if err != nil { + return tools.Result{Err: fmt.Errorf("append_file: %w", err)} + } + + if err := validateWritePath(absPath, cfg.AllowedPaths, cfg.ReadOnly); err != nil { + return tools.Result{Err: err} + } + + // Check existing file size to enforce the total limit. + var existingSize int64 + info, err := os.Stat(absPath) + if err == nil { + existingSize = info.Size() + } + // err != nil means file doesn't exist, which is fine (will be created). + + newTotal := existingSize + int64(len(content)) + if newTotal > maxAppendTotal { + return tools.Result{Err: fmt.Errorf("append_file: resulting file size (%d bytes) exceeds 10 MB limit", newTotal)} + } + + // Create parent directories if they don't exist. + dir := filepath.Dir(absPath) + if err := os.MkdirAll(dir, 0755); err != nil { + return tools.Result{Err: fmt.Errorf("append_file: cannot create directories: %w", err)} + } + + f, err := os.OpenFile(absPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + return tools.Result{Err: fmt.Errorf("append_file: %w", err)} + } + defer f.Close() + + n, err := f.WriteString(content) + if err != nil { + return tools.Result{Err: fmt.Errorf("append_file: %w", err)} + } + + finalSize := existingSize + int64(n) + return tools.Result{Output: fmt.Sprintf("appended %d bytes to %s (total size: %d bytes)", n, absPath, finalSize)} + }, + } +} diff --git a/tools/file/append_test.go b/tools/file/append_test.go new file mode 100644 index 0000000..18d4490 --- /dev/null +++ b/tools/file/append_test.go @@ -0,0 +1,212 @@ +package file + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/enmanuel/agents/internal/config" +) + +func TestAppendFile_AppendsToExistingFile(t *testing.T) { + tmp := t.TempDir() + target := filepath.Join(tmp, "log.txt") + os.WriteFile(target, []byte("line1\n"), 0644) + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewAppendFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": target, + "content": "line2\n", + }) + if result.Err != nil { + t.Fatalf("expected success, got: %v", result.Err) + } + + data, _ := os.ReadFile(target) + if string(data) != "line1\nline2\n" { + t.Fatalf("expected 'line1\\nline2\\n', got %q", string(data)) + } + + if !strings.Contains(result.Output, "6 bytes") { + t.Fatalf("expected '6 bytes' in output, got: %q", result.Output) + } + if !strings.Contains(result.Output, "total size: 12 bytes") { + t.Fatalf("expected total size in output, got: %q", result.Output) + } +} + +func TestAppendFile_CreatesNewFileIfNotExists(t *testing.T) { + tmp := t.TempDir() + target := filepath.Join(tmp, "new.txt") + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewAppendFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": target, + "content": "first line", + }) + if result.Err != nil { + t.Fatalf("expected success, got: %v", result.Err) + } + + data, err := os.ReadFile(target) + if err != nil { + t.Fatalf("file not created: %v", err) + } + if string(data) != "first line" { + t.Fatalf("expected 'first line', got %q", string(data)) + } +} + +func TestAppendFile_RejectsReadOnly(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: true} + tool := NewAppendFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": filepath.Join(tmp, "test.txt"), + "content": "data", + }) + if result.Err == nil { + t.Fatal("expected error when ReadOnly is true") + } + if !strings.Contains(result.Err.Error(), "read_only") { + t.Fatalf("expected read_only error, got: %v", result.Err) + } +} + +func TestAppendFile_RejectsPathOutsideAllowed(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewAppendFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": "/etc/test.txt", + "content": "data", + }) + if result.Err == nil { + t.Fatal("expected error for path outside allowed") + } +} + +func TestAppendFile_RejectsTotalSizeOver10MB(t *testing.T) { + tmp := t.TempDir() + target := filepath.Join(tmp, "big.txt") + + // Create a file just under the limit + existing := strings.Repeat("x", maxAppendTotal-100) + os.WriteFile(target, []byte(existing), 0644) + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewAppendFile(cfg) + + // Try to append content that would exceed the limit + result := tool.Exec(context.Background(), map[string]any{ + "path": target, + "content": strings.Repeat("y", 200), + }) + if result.Err == nil { + t.Fatal("expected error when total size exceeds 10 MB") + } + if !strings.Contains(result.Err.Error(), "10 MB") { + t.Fatalf("expected 10 MB error, got: %v", result.Err) + } +} + +func TestAppendFile_PathTraversal(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewAppendFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": filepath.Join(tmp, "..", "..", "etc", "evil.txt"), + "content": "data", + }) + if result.Err == nil { + t.Fatal("expected error for path traversal") + } +} + +func TestAppendFile_SymlinkEscape(t *testing.T) { + tmp := t.TempDir() + link := filepath.Join(tmp, "escape") + os.Symlink("/tmp", link) + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewAppendFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": filepath.Join(link, "evil.txt"), + "content": "data", + }) + if result.Err == nil { + t.Fatal("expected error for symlink escape") + } +} + +func TestAppendFile_DenyByDefault(t *testing.T) { + cfg := config.FileOpsCfg{AllowedPaths: []string{}, ReadOnly: false} + tool := NewAppendFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": "/tmp/test.txt", + "content": "data", + }) + if result.Err == nil { + t.Fatal("expected error when AllowedPaths is empty") + } +} + +func TestAppendFile_EmptyPath(t *testing.T) { + cfg := config.FileOpsCfg{AllowedPaths: []string{"/tmp"}, ReadOnly: false} + tool := NewAppendFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": "", + "content": "data", + }) + if result.Err == nil { + t.Fatal("expected error for empty path") + } +} + +func TestAppendFile_EmptyContent(t *testing.T) { + cfg := config.FileOpsCfg{AllowedPaths: []string{"/tmp"}, ReadOnly: false} + tool := NewAppendFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": "/tmp/test.txt", + "content": "", + }) + if result.Err == nil { + t.Fatal("expected error for empty content") + } +} + +func TestAppendFile_CreatesParentDirectories(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewAppendFile(cfg) + + target := filepath.Join(tmp, "sub", "dir", "file.txt") + result := tool.Exec(context.Background(), map[string]any{ + "path": target, + "content": "nested content", + }) + if result.Err != nil { + t.Fatalf("expected success, got: %v", result.Err) + } + + data, err := os.ReadFile(target) + if err != nil { + t.Fatalf("file not created: %v", err) + } + if string(data) != "nested content" { + t.Fatalf("expected 'nested content', got %q", string(data)) + } +} diff --git a/tools/file/delete.go b/tools/file/delete.go new file mode 100644 index 0000000..b089756 --- /dev/null +++ b/tools/file/delete.go @@ -0,0 +1,57 @@ +package file + +import ( + "context" + "fmt" + "os" + "path/filepath" + + "github.com/enmanuel/agents/internal/config" + "github.com/enmanuel/agents/tools" +) + +// NewDeleteFile creates a delete_file tool that deletes a single file. +// Deny-by-default: if AllowedPaths is empty, all operations are rejected. +// Rejects if ReadOnly is true. Only deletes files, never directories. +// Resolves symlinks before deleting to prevent escaping allowed paths. +func NewDeleteFile(cfg config.FileOpsCfg) tools.Tool { + return tools.Tool{ + Def: tools.Def{ + Name: "delete_file", + Description: "Delete a single file. Cannot delete directories.", + Parameters: []tools.Param{ + {Name: "path", Type: "string", Description: "Absolute path to the file to delete", Required: true}, + }, + }, + Exec: func(ctx context.Context, args map[string]any) tools.Result { + path := tools.GetString(args, "path") + if path == "" { + return tools.Result{Err: fmt.Errorf("delete_file: path is required")} + } + + absPath, err := filepath.Abs(path) + if err != nil { + return tools.Result{Err: fmt.Errorf("delete_file: %w", err)} + } + + if err := validateWritePath(absPath, cfg.AllowedPaths, cfg.ReadOnly); err != nil { + return tools.Result{Err: err} + } + + // Stat the file to ensure it exists and is not a directory. + info, err := os.Stat(absPath) + if err != nil { + return tools.Result{Err: fmt.Errorf("delete_file: %w", err)} + } + if info.IsDir() { + return tools.Result{Err: fmt.Errorf("delete_file: %q is a directory, only files can be deleted", absPath)} + } + + if err := os.Remove(absPath); err != nil { + return tools.Result{Err: fmt.Errorf("delete_file: %w", err)} + } + + return tools.Result{Output: fmt.Sprintf("deleted %s", absPath)} + }, + } +} diff --git a/tools/file/delete_test.go b/tools/file/delete_test.go new file mode 100644 index 0000000..ac7ec3d --- /dev/null +++ b/tools/file/delete_test.go @@ -0,0 +1,160 @@ +package file + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/enmanuel/agents/internal/config" +) + +func TestDeleteFile_DeletesExistingFile(t *testing.T) { + tmp := t.TempDir() + target := filepath.Join(tmp, "doomed.txt") + os.WriteFile(target, []byte("bye"), 0644) + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewDeleteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": target}) + if result.Err != nil { + t.Fatalf("expected success, got: %v", result.Err) + } + + if _, err := os.Stat(target); !os.IsNotExist(err) { + t.Fatal("file should have been deleted") + } + + if !strings.Contains(result.Output, "deleted") { + t.Fatalf("expected 'deleted' in output, got: %q", result.Output) + } +} + +func TestDeleteFile_RejectsDirectories(t *testing.T) { + tmp := t.TempDir() + subdir := filepath.Join(tmp, "mydir") + os.Mkdir(subdir, 0755) + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewDeleteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": subdir}) + if result.Err == nil { + t.Fatal("expected error when trying to delete a directory") + } + if !strings.Contains(result.Err.Error(), "directory") { + t.Fatalf("expected directory error, got: %v", result.Err) + } + + // Verify directory still exists + if _, err := os.Stat(subdir); os.IsNotExist(err) { + t.Fatal("directory should not have been deleted") + } +} + +func TestDeleteFile_RejectsReadOnly(t *testing.T) { + tmp := t.TempDir() + target := filepath.Join(tmp, "protected.txt") + os.WriteFile(target, []byte("safe"), 0644) + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: true} + tool := NewDeleteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": target}) + if result.Err == nil { + t.Fatal("expected error when ReadOnly is true") + } + if !strings.Contains(result.Err.Error(), "read_only") { + t.Fatalf("expected read_only error, got: %v", result.Err) + } + + // Verify file still exists + if _, err := os.Stat(target); os.IsNotExist(err) { + t.Fatal("file should not have been deleted") + } +} + +func TestDeleteFile_RejectsPathOutsideAllowed(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewDeleteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": "/etc/hosts"}) + if result.Err == nil { + t.Fatal("expected error for path outside allowed") + } +} + +func TestDeleteFile_PathTraversal(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewDeleteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": filepath.Join(tmp, "..", "..", "etc", "hosts"), + }) + if result.Err == nil { + t.Fatal("expected error for path traversal") + } +} + +func TestDeleteFile_SymlinkEscape(t *testing.T) { + tmp := t.TempDir() + + // Create a file outside allowed paths + outside := t.TempDir() + outsideFile := filepath.Join(outside, "secret.txt") + os.WriteFile(outsideFile, []byte("secret"), 0644) + + // Create symlink inside allowed paths pointing to the outside file + link := filepath.Join(tmp, "link.txt") + os.Symlink(outsideFile, link) + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewDeleteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": link}) + if result.Err == nil { + t.Fatal("expected error for symlink escape") + } + + // Verify the outside file still exists + if _, err := os.Stat(outsideFile); os.IsNotExist(err) { + t.Fatal("outside file should not have been deleted") + } +} + +func TestDeleteFile_NonExistentFile(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewDeleteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": filepath.Join(tmp, "nonexistent.txt"), + }) + if result.Err == nil { + t.Fatal("expected error for non-existent file") + } +} + +func TestDeleteFile_EmptyPath(t *testing.T) { + cfg := config.FileOpsCfg{AllowedPaths: []string{"/tmp"}, ReadOnly: false} + tool := NewDeleteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": ""}) + if result.Err == nil { + t.Fatal("expected error for empty path") + } +} + +func TestDeleteFile_DenyByDefault(t *testing.T) { + cfg := config.FileOpsCfg{AllowedPaths: []string{}, ReadOnly: false} + tool := NewDeleteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": "/tmp/test.txt"}) + if result.Err == nil { + t.Fatal("expected error when AllowedPaths is empty") + } +} diff --git a/tools/file/file.go b/tools/file/file.go index 4c84e3d..95d0e57 100644 --- a/tools/file/file.go +++ b/tools/file/file.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "path/filepath" - "strings" "github.com/enmanuel/agents/internal/config" "github.com/enmanuel/agents/tools" @@ -53,55 +52,3 @@ func NewReadFile(cfg config.FileOpsCfg) tools.Tool { }, } } - -// validatePath checks that absPath is under one of the allowed paths. -// Deny-by-default: if allowedPaths is empty, no paths are allowed. -// Resolves symlinks to prevent traversal via ../ or symlink escapes. -func validatePath(absPath string, allowedPaths []string) error { - if len(allowedPaths) == 0 { - return fmt.Errorf("read_file: no allowed paths configured, all reads denied") - } - - // Resolve symlinks on the requested path to get the real path. - // If the file doesn't exist yet, resolve the parent directory. - realPath, err := resolveReal(absPath) - if err != nil { - return fmt.Errorf("read_file: cannot resolve path %q: %w", absPath, err) - } - - for _, allowed := range allowedPaths { - a, err := filepath.Abs(allowed) - if err != nil { - continue - } - // Resolve symlinks on the allowed path too. - realAllowed, err := resolveReal(a) - if err != nil { - continue - } - // Ensure the real path is strictly under the allowed directory. - // Add trailing separator to prevent /opt matching /opt1234. - if strings.HasPrefix(realPath, realAllowed+string(filepath.Separator)) || realPath == realAllowed { - return nil - } - } - return fmt.Errorf("path %q not under any allowed path", absPath) -} - -// resolveReal resolves symlinks for a path. -// If the exact path doesn't exist, it resolves the deepest existing ancestor -// and appends the remaining segments, preventing partial traversal. -func resolveReal(path string) (string, error) { - real, err := filepath.EvalSymlinks(path) - if err == nil { - return filepath.Clean(real), nil - } - // Path doesn't exist — resolve parent and append base. - parent := filepath.Dir(path) - base := filepath.Base(path) - realParent, err := filepath.EvalSymlinks(parent) - if err != nil { - return "", err - } - return filepath.Clean(filepath.Join(realParent, base)), nil -} diff --git a/tools/file/list.go b/tools/file/list.go new file mode 100644 index 0000000..98c8f34 --- /dev/null +++ b/tools/file/list.go @@ -0,0 +1,173 @@ +package file + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "github.com/enmanuel/agents/internal/config" + "github.com/enmanuel/agents/tools" +) + +// maxListEntries is the maximum number of entries returned by list_directory. +const maxListEntries = 500 + +// NewListDirectory creates a list_directory tool that lists files and directories. +// Deny-by-default: if AllowedPaths is empty, all listings are rejected. +// Does not follow symlinks that point outside of AllowedPaths. +func NewListDirectory(cfg config.FileOpsCfg) tools.Tool { + return tools.Tool{ + Def: tools.Def{ + Name: "list_directory", + Description: "List files and directories at the given path. Returns name, size, type (file/dir), and modification date for each entry.", + Parameters: []tools.Param{ + {Name: "path", Type: "string", Description: "Absolute path to the directory to list", Required: true}, + {Name: "recursive", Type: "boolean", Description: "List recursively (default: false)", Required: false}, + }, + }, + Exec: func(ctx context.Context, args map[string]any) tools.Result { + path := tools.GetString(args, "path") + if path == "" { + return tools.Result{Err: fmt.Errorf("list_directory: path is required")} + } + + recursive := getBool(args, "recursive") + + absPath, err := filepath.Abs(path) + if err != nil { + return tools.Result{Err: fmt.Errorf("list_directory: %w", err)} + } + + if err := validatePath(absPath, cfg.AllowedPaths); err != nil { + return tools.Result{Err: err} + } + + info, err := os.Stat(absPath) + if err != nil { + return tools.Result{Err: fmt.Errorf("list_directory: %w", err)} + } + if !info.IsDir() { + return tools.Result{Err: fmt.Errorf("list_directory: %q is not a directory", absPath)} + } + + var entries []string + if recursive { + entries, err = listRecursive(absPath, cfg.AllowedPaths) + } else { + entries, err = listFlat(absPath, cfg.AllowedPaths) + } + if err != nil { + return tools.Result{Err: fmt.Errorf("list_directory: %w", err)} + } + + if len(entries) > maxListEntries { + entries = entries[:maxListEntries] + entries = append(entries, fmt.Sprintf("... (truncated, showing %d of more entries)", maxListEntries)) + } + + return tools.Result{Output: strings.Join(entries, "\n")} + }, + } +} + +// listFlat lists immediate children of dir. +func listFlat(dir string, allowedPaths []string) ([]string, error) { + dirEntries, err := os.ReadDir(dir) + if err != nil { + return nil, err + } + + var results []string + for _, e := range dirEntries { + entryPath := filepath.Join(dir, e.Name()) + + // Skip symlinks that point outside allowed paths. + if e.Type()&os.ModeSymlink != 0 { + if err := validatePath(entryPath, allowedPaths); err != nil { + continue + } + } + + info, err := e.Info() + if err != nil { + continue + } + + results = append(results, formatEntry("", e.Name(), info)) + } + return results, nil +} + +// listRecursive lists all files under dir recursively. +func listRecursive(root string, allowedPaths []string) ([]string, error) { + var results []string + count := 0 + + err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + if err != nil { + return nil // skip entries with errors + } + if path == root { + return nil // skip the root directory itself + } + if count >= maxListEntries { + return filepath.SkipAll + } + + // Skip symlinks that point outside allowed paths. + if d.Type()&os.ModeSymlink != 0 { + if err := validatePath(path, allowedPaths); err != nil { + if d.IsDir() { + return filepath.SkipDir + } + return nil + } + } + + rel, err := filepath.Rel(root, path) + if err != nil { + return nil + } + + info, err := d.Info() + if err != nil { + return nil + } + + results = append(results, formatEntry("", rel, info)) + count++ + return nil + }) + + return results, err +} + +// formatEntry formats a single directory entry for output. +func formatEntry(prefix, name string, info os.FileInfo) string { + kind := "file" + if info.IsDir() { + kind = "dir" + } + mod := info.ModTime().Format(time.RFC3339) + display := name + if prefix != "" { + display = prefix + "/" + name + } + return fmt.Sprintf("%s\t%s\t%d\t%s", display, kind, info.Size(), mod) +} + +// getBool extracts a boolean argument by name, returning false if missing or wrong type. +func getBool(args map[string]any, key string) bool { + v, ok := args[key] + if !ok { + return false + } + b, ok := v.(bool) + if !ok { + return false + } + return b +} diff --git a/tools/file/list_test.go b/tools/file/list_test.go new file mode 100644 index 0000000..edfa86a --- /dev/null +++ b/tools/file/list_test.go @@ -0,0 +1,176 @@ +package file + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/enmanuel/agents/internal/config" +) + +func TestListDirectory_ListsFilesAndDirs(t *testing.T) { + tmp := t.TempDir() + os.WriteFile(filepath.Join(tmp, "file1.txt"), []byte("hello"), 0644) + os.WriteFile(filepath.Join(tmp, "file2.txt"), []byte("world"), 0644) + os.Mkdir(filepath.Join(tmp, "subdir"), 0755) + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}} + tool := NewListDirectory(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": tmp}) + if result.Err != nil { + t.Fatalf("expected success, got: %v", result.Err) + } + + if !strings.Contains(result.Output, "file1.txt") { + t.Fatalf("expected file1.txt in output, got: %s", result.Output) + } + if !strings.Contains(result.Output, "file2.txt") { + t.Fatalf("expected file2.txt in output, got: %s", result.Output) + } + if !strings.Contains(result.Output, "subdir") { + t.Fatalf("expected subdir in output, got: %s", result.Output) + } + if !strings.Contains(result.Output, "dir") { + t.Fatalf("expected 'dir' type in output, got: %s", result.Output) + } +} + +func TestListDirectory_Recursive(t *testing.T) { + tmp := t.TempDir() + sub := filepath.Join(tmp, "sub") + os.Mkdir(sub, 0755) + os.WriteFile(filepath.Join(tmp, "root.txt"), []byte("r"), 0644) + os.WriteFile(filepath.Join(sub, "nested.txt"), []byte("n"), 0644) + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}} + tool := NewListDirectory(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": tmp, + "recursive": true, + }) + if result.Err != nil { + t.Fatalf("expected success, got: %v", result.Err) + } + + if !strings.Contains(result.Output, "root.txt") { + t.Fatalf("expected root.txt in output, got: %s", result.Output) + } + if !strings.Contains(result.Output, filepath.Join("sub", "nested.txt")) { + t.Fatalf("expected sub/nested.txt in output, got: %s", result.Output) + } +} + +func TestListDirectory_RespectsMaxEntries(t *testing.T) { + tmp := t.TempDir() + // Create more than maxListEntries files with unique names + for i := 0; i < maxListEntries+10; i++ { + name := fmt.Sprintf("file_%04d.txt", i) + os.WriteFile(filepath.Join(tmp, name), []byte("x"), 0644) + } + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}} + tool := NewListDirectory(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": tmp}) + if result.Err != nil { + t.Fatalf("expected success, got: %v", result.Err) + } + + lines := strings.Split(result.Output, "\n") + // Should be maxListEntries + 1 (truncation message) + if len(lines) > maxListEntries+1 { + t.Fatalf("expected at most %d lines, got %d", maxListEntries+1, len(lines)) + } + if !strings.Contains(result.Output, "truncated") { + t.Fatalf("expected truncation message, got: %s", result.Output[len(result.Output)-200:]) + } +} + +func TestListDirectory_SymlinkOutsideAllowedSkipped(t *testing.T) { + tmp := t.TempDir() + // Create a symlink pointing outside AllowedPaths + link := filepath.Join(tmp, "escape") + os.Symlink("/etc", link) + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}} + tool := NewListDirectory(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": tmp}) + if result.Err != nil { + t.Fatalf("expected success, got: %v", result.Err) + } + + // The symlink should be skipped, not listed + if strings.Contains(result.Output, "escape") { + t.Fatalf("symlink pointing outside allowed paths should be skipped, got: %s", result.Output) + } +} + +func TestListDirectory_PathTraversal(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}} + tool := NewListDirectory(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": filepath.Join(tmp, "..", "..", "etc"), + }) + if result.Err == nil { + t.Fatal("expected error for path traversal") + } +} + +func TestListDirectory_DenyByDefault(t *testing.T) { + cfg := config.FileOpsCfg{AllowedPaths: []string{}} + tool := NewListDirectory(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": "/tmp"}) + if result.Err == nil { + t.Fatal("expected error when AllowedPaths is empty") + } +} + +func TestListDirectory_NotADirectory(t *testing.T) { + tmp := t.TempDir() + f := filepath.Join(tmp, "file.txt") + os.WriteFile(f, []byte("hello"), 0644) + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}} + tool := NewListDirectory(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": f}) + if result.Err == nil { + t.Fatal("expected error for non-directory path") + } + if !strings.Contains(result.Err.Error(), "not a directory") { + t.Fatalf("expected 'not a directory' error, got: %v", result.Err) + } +} + +func TestListDirectory_EmptyPath(t *testing.T) { + cfg := config.FileOpsCfg{AllowedPaths: []string{"/tmp"}} + tool := NewListDirectory(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": ""}) + if result.Err == nil { + t.Fatal("expected error for empty path") + } +} + +func TestListDirectory_EmptyDirectory(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}} + tool := NewListDirectory(cfg) + + result := tool.Exec(context.Background(), map[string]any{"path": tmp}) + if result.Err != nil { + t.Fatalf("expected success for empty dir, got: %v", result.Err) + } + if result.Output != "" { + t.Fatalf("expected empty output for empty dir, got: %q", result.Output) + } +} diff --git a/tools/file/validate.go b/tools/file/validate.go new file mode 100644 index 0000000..40336d7 --- /dev/null +++ b/tools/file/validate.go @@ -0,0 +1,83 @@ +package file + +import ( + "fmt" + "path/filepath" + "strings" +) + +// validatePath checks that absPath is under one of the allowed paths. +// Deny-by-default: if allowedPaths is empty, no paths are allowed. +// Resolves symlinks to prevent traversal via ../ or symlink escapes. +func validatePath(absPath string, allowedPaths []string) error { + if len(allowedPaths) == 0 { + return fmt.Errorf("file: no allowed paths configured, all operations denied") + } + + // Resolve symlinks on the requested path to get the real path. + // If the file doesn't exist yet, resolve the parent directory. + realPath, err := resolveReal(absPath) + if err != nil { + return fmt.Errorf("file: cannot resolve path %q: %w", absPath, err) + } + + for _, allowed := range allowedPaths { + a, err := filepath.Abs(allowed) + if err != nil { + continue + } + // Resolve symlinks on the allowed path too. + realAllowed, err := resolveReal(a) + if err != nil { + continue + } + // Ensure the real path is strictly under the allowed directory. + // Add trailing separator to prevent /opt matching /opt1234. + if strings.HasPrefix(realPath, realAllowed+string(filepath.Separator)) || realPath == realAllowed { + return nil + } + } + return fmt.Errorf("path %q not under any allowed path", absPath) +} + +// validateWritePath checks path validity AND that writing is allowed. +func validateWritePath(absPath string, allowedPaths []string, readOnly bool) error { + if readOnly { + return fmt.Errorf("file: write operations denied (read_only mode)") + } + return validatePath(absPath, allowedPaths) +} + +// resolveReal resolves symlinks for a path. +// If the exact path doesn't exist, it walks up the tree to find the deepest +// existing ancestor, resolves its symlinks, and appends the remaining segments. +// This prevents partial traversal attacks via symlinks in non-existent paths. +func resolveReal(path string) (string, error) { + real, err := filepath.EvalSymlinks(path) + if err == nil { + return filepath.Clean(real), nil + } + + // Walk up to find the deepest existing ancestor. + cleaned := filepath.Clean(path) + var tail []string + cur := cleaned + for { + parent := filepath.Dir(cur) + tail = append([]string{filepath.Base(cur)}, tail...) + realParent, err := filepath.EvalSymlinks(parent) + if err == nil { + // Found an existing ancestor — rebuild the path. + result := realParent + for _, seg := range tail { + result = filepath.Join(result, seg) + } + return filepath.Clean(result), nil + } + if parent == cur { + // Reached the root without finding an existing ancestor. + return "", fmt.Errorf("cannot resolve any ancestor of %q", path) + } + cur = parent + } +} diff --git a/tools/file/write.go b/tools/file/write.go new file mode 100644 index 0000000..7092818 --- /dev/null +++ b/tools/file/write.go @@ -0,0 +1,67 @@ +package file + +import ( + "context" + "fmt" + "os" + "path/filepath" + + "github.com/enmanuel/agents/internal/config" + "github.com/enmanuel/agents/tools" +) + +// maxWriteSize is the maximum content size for write_file (1 MB). +const maxWriteSize = 1 * 1024 * 1024 + +// NewWriteFile creates a write_file tool that writes content to a local file. +// Deny-by-default: if AllowedPaths is empty, all writes are rejected. +// Rejects if ReadOnly is true. Creates parent directories if needed. +func NewWriteFile(cfg config.FileOpsCfg) tools.Tool { + return tools.Tool{ + Def: tools.Def{ + Name: "write_file", + Description: "Write content to a local file. Creates the file if it does not exist. Creates parent directories if needed. Overwrites existing content.", + Parameters: []tools.Param{ + {Name: "path", Type: "string", Description: "Absolute path to the file to write", Required: true}, + {Name: "content", Type: "string", Description: "Content to write to the file", Required: true}, + }, + }, + Exec: func(ctx context.Context, args map[string]any) tools.Result { + path := tools.GetString(args, "path") + if path == "" { + return tools.Result{Err: fmt.Errorf("write_file: path is required")} + } + + content := tools.GetString(args, "content") + if content == "" { + return tools.Result{Err: fmt.Errorf("write_file: content is required")} + } + + if len(content) > maxWriteSize { + return tools.Result{Err: fmt.Errorf("write_file: content exceeds maximum size of 1 MB")} + } + + absPath, err := filepath.Abs(path) + if err != nil { + return tools.Result{Err: fmt.Errorf("write_file: %w", err)} + } + + if err := validateWritePath(absPath, cfg.AllowedPaths, cfg.ReadOnly); err != nil { + return tools.Result{Err: err} + } + + // Create parent directories if they don't exist. + dir := filepath.Dir(absPath) + if err := os.MkdirAll(dir, 0755); err != nil { + return tools.Result{Err: fmt.Errorf("write_file: cannot create directories: %w", err)} + } + + data := []byte(content) + if err := os.WriteFile(absPath, data, 0644); err != nil { + return tools.Result{Err: fmt.Errorf("write_file: %w", err)} + } + + return tools.Result{Output: fmt.Sprintf("wrote %d bytes to %s", len(data), absPath)} + }, + } +} diff --git a/tools/file/write_test.go b/tools/file/write_test.go new file mode 100644 index 0000000..af3ecf0 --- /dev/null +++ b/tools/file/write_test.go @@ -0,0 +1,202 @@ +package file + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/enmanuel/agents/internal/config" +) + +func TestWriteFile_CreatesNewFile(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewWriteFile(cfg) + + target := filepath.Join(tmp, "new.txt") + result := tool.Exec(context.Background(), map[string]any{ + "path": target, + "content": "hello world", + }) + if result.Err != nil { + t.Fatalf("expected success, got: %v", result.Err) + } + + data, err := os.ReadFile(target) + if err != nil { + t.Fatalf("file not created: %v", err) + } + if string(data) != "hello world" { + t.Fatalf("expected 'hello world', got %q", string(data)) + } + + if !strings.Contains(result.Output, "11 bytes") { + t.Fatalf("expected output mentioning bytes, got %q", result.Output) + } +} + +func TestWriteFile_RejectsReadOnly(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: true} + tool := NewWriteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": filepath.Join(tmp, "test.txt"), + "content": "data", + }) + if result.Err == nil { + t.Fatal("expected error when ReadOnly is true") + } + if !strings.Contains(result.Err.Error(), "read_only") { + t.Fatalf("expected read_only error, got: %v", result.Err) + } +} + +func TestWriteFile_RejectsPathOutsideAllowed(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewWriteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": "/etc/test.txt", + "content": "data", + }) + if result.Err == nil { + t.Fatal("expected error for path outside allowed") + } +} + +func TestWriteFile_RejectsContentOver1MB(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewWriteFile(cfg) + + bigContent := strings.Repeat("x", maxWriteSize+1) + result := tool.Exec(context.Background(), map[string]any{ + "path": filepath.Join(tmp, "big.txt"), + "content": bigContent, + }) + if result.Err == nil { + t.Fatal("expected error for content exceeding 1 MB") + } + if !strings.Contains(result.Err.Error(), "1 MB") { + t.Fatalf("expected 1 MB error, got: %v", result.Err) + } +} + +func TestWriteFile_CreatesParentDirectories(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewWriteFile(cfg) + + target := filepath.Join(tmp, "sub", "dir", "file.txt") + result := tool.Exec(context.Background(), map[string]any{ + "path": target, + "content": "nested", + }) + if result.Err != nil { + t.Fatalf("expected success, got: %v", result.Err) + } + + data, err := os.ReadFile(target) + if err != nil { + t.Fatalf("file not created: %v", err) + } + if string(data) != "nested" { + t.Fatalf("expected 'nested', got %q", string(data)) + } +} + +func TestWriteFile_OverwritesExistingFile(t *testing.T) { + tmp := t.TempDir() + target := filepath.Join(tmp, "exists.txt") + os.WriteFile(target, []byte("old"), 0644) + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewWriteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": target, + "content": "new content", + }) + if result.Err != nil { + t.Fatalf("expected success, got: %v", result.Err) + } + + data, _ := os.ReadFile(target) + if string(data) != "new content" { + t.Fatalf("expected 'new content', got %q", string(data)) + } +} + +func TestWriteFile_PathTraversal(t *testing.T) { + tmp := t.TempDir() + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewWriteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": filepath.Join(tmp, "..", "..", "etc", "evil.txt"), + "content": "data", + }) + if result.Err == nil { + t.Fatal("expected error for path traversal") + } +} + +func TestWriteFile_SymlinkEscape(t *testing.T) { + tmp := t.TempDir() + link := filepath.Join(tmp, "escape") + os.Symlink("/tmp", link) + + cfg := config.FileOpsCfg{AllowedPaths: []string{tmp}, ReadOnly: false} + tool := NewWriteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": filepath.Join(link, "evil.txt"), + "content": "data", + }) + if result.Err == nil { + t.Fatal("expected error for symlink escape") + } +} + +func TestWriteFile_EmptyPath(t *testing.T) { + cfg := config.FileOpsCfg{AllowedPaths: []string{"/tmp"}, ReadOnly: false} + tool := NewWriteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": "", + "content": "data", + }) + if result.Err == nil { + t.Fatal("expected error for empty path") + } +} + +func TestWriteFile_EmptyContent(t *testing.T) { + cfg := config.FileOpsCfg{AllowedPaths: []string{"/tmp"}, ReadOnly: false} + tool := NewWriteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": "/tmp/test.txt", + "content": "", + }) + if result.Err == nil { + t.Fatal("expected error for empty content") + } +} + +func TestWriteFile_DenyByDefault(t *testing.T) { + cfg := config.FileOpsCfg{AllowedPaths: []string{}, ReadOnly: false} + tool := NewWriteFile(cfg) + + result := tool.Exec(context.Background(), map[string]any{ + "path": "/tmp/test.txt", + "content": "data", + }) + if result.Err == nil { + t.Fatal("expected error when AllowedPaths is empty") + } +}