From d3bef1357a17d1d8f7720fd31d5b10bd94c696a8 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Tue, 24 Feb 2026 09:29:11 +0100 Subject: [PATCH 1/3] Fix path traversal vulnerability in ACP filesystem operations Add path validation to handleReadFile, handleWriteFile, and handleEditFile in the ACP filesystem toolset. User-supplied paths are now resolved and validated to ensure they do not escape the configured working directory. Previously, paths like '../../etc/passwd' could be used to access files outside the working directory. The new resolvePath method converts paths to absolute form and checks they remain within the working directory boundary. Fixes #1823 Assisted-By: cagent --- pkg/acp/filesystem.go | 44 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/pkg/acp/filesystem.go b/pkg/acp/filesystem.go index 0c6a80662..9a64faf1d 100644 --- a/pkg/acp/filesystem.go +++ b/pkg/acp/filesystem.go @@ -68,6 +68,25 @@ func (t *FilesystemToolset) Tools(ctx context.Context) ([]tools.Tool, error) { return baseTools, nil } +// resolvePath resolves a user-supplied path relative to the working directory +// and validates that the resulting path does not escape the working directory. +func (t *FilesystemToolset) resolvePath(userPath string) (string, error) { + resolved := filepath.Clean(filepath.Join(t.workingDir, userPath)) + absWorkingDir, err := filepath.Abs(t.workingDir) + if err != nil { + return "", fmt.Errorf("failed to resolve working directory: %w", err) + } + absResolved, err := filepath.Abs(resolved) + if err != nil { + return "", fmt.Errorf("failed to resolve path: %w", err) + } + // Ensure the resolved path is within the working directory + if !strings.HasPrefix(absResolved, absWorkingDir+string(filepath.Separator)) && absResolved != absWorkingDir { + return "", fmt.Errorf("path %q escapes the working directory", userPath) + } + return absResolved, nil +} + func (t *FilesystemToolset) handleReadFile(ctx context.Context, toolCall tools.ToolCall) (*tools.ToolCallResult, error) { var args builtin.ReadFileArgs if err := json.Unmarshal([]byte(toolCall.Function.Arguments), &args); err != nil { @@ -79,9 +98,14 @@ func (t *FilesystemToolset) handleReadFile(ctx context.Context, toolCall tools.T return tools.ResultError("Error: session ID not found in context"), nil } + resolvedPath, err := t.resolvePath(args.Path) + if err != nil { + return tools.ResultError(fmt.Sprintf("Error: %s", err)), nil + } + resp, err := t.agent.conn.ReadTextFile(ctx, acp.ReadTextFileRequest{ SessionId: acp.SessionId(sessionID), - Path: filepath.Join(t.workingDir, args.Path), + Path: resolvedPath, }) if err != nil { return tools.ResultError(fmt.Sprintf("Error reading file: %s", err)), nil @@ -101,9 +125,14 @@ func (t *FilesystemToolset) handleWriteFile(ctx context.Context, toolCall tools. return tools.ResultError("Error: session ID not found in context"), nil } - _, err := t.agent.conn.WriteTextFile(ctx, acp.WriteTextFileRequest{ + resolvedPath, err := t.resolvePath(args.Path) + if err != nil { + return tools.ResultError(fmt.Sprintf("Error: %s", err)), nil + } + + _, err = t.agent.conn.WriteTextFile(ctx, acp.WriteTextFileRequest{ SessionId: acp.SessionId(sessionID), - Path: filepath.Join(t.workingDir, args.Path), + Path: resolvedPath, Content: args.Content, }) if err != nil { @@ -124,9 +153,14 @@ func (t *FilesystemToolset) handleEditFile(ctx context.Context, toolCall tools.T return tools.ResultError("Error: session ID not found in context"), nil } + resolvedPath, err := t.resolvePath(args.Path) + if err != nil { + return tools.ResultError(fmt.Sprintf("Error: %s", err)), nil + } + resp, err := t.agent.conn.ReadTextFile(ctx, acp.ReadTextFileRequest{ SessionId: acp.SessionId(sessionID), - Path: filepath.Join(t.workingDir, args.Path), + Path: resolvedPath, }) if err != nil { return tools.ResultError(fmt.Sprintf("Error reading file: %s", err)), nil @@ -143,7 +177,7 @@ func (t *FilesystemToolset) handleEditFile(ctx context.Context, toolCall tools.T _, err = t.agent.conn.WriteTextFile(ctx, acp.WriteTextFileRequest{ SessionId: acp.SessionId(sessionID), - Path: filepath.Join(t.workingDir, args.Path), + Path: resolvedPath, Content: modifiedContent, }) if err != nil { From dae0551ffbcef3e809e6fa0e6f6f20a0a20e7348 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Tue, 24 Feb 2026 09:29:15 +0100 Subject: [PATCH 2/3] Log errors when persisting session title in TUI The persistSessionTitle function silently discarded errors from UpdateSessionTitle (other than ErrTitleGenerating). Add slog.Warn so failures are visible in debug logs. Fixes #1821 Assisted-By: cagent --- pkg/tui/page/chat/input_handlers.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/tui/page/chat/input_handlers.go b/pkg/tui/page/chat/input_handlers.go index 1ccee65ed..90f5e24ff 100644 --- a/pkg/tui/page/chat/input_handlers.go +++ b/pkg/tui/page/chat/input_handlers.go @@ -3,6 +3,7 @@ package chat import ( "context" "errors" + "log/slog" "charm.land/bubbles/v2/key" tea "charm.land/bubbletea/v2" @@ -81,7 +82,7 @@ func (p *chatPage) persistSessionTitle(newTitle string) tea.Cmd { if errors.Is(err, app.ErrTitleGenerating) { return notification.ShowMsg{Text: "Title is being generated, please wait", Type: notification.TypeWarning} } - // Log other errors but don't show them + slog.Warn("Failed to persist session title", "title", newTitle, "error", err) return nil } return nil From 181babfe0f702755a9a3d018f9a120b4aa7be95e Mon Sep 17 00:00:00 2001 From: David Gageot Date: Tue, 24 Feb 2026 16:00:51 +0100 Subject: [PATCH 3/3] Handle case-insensitive filesystems Signed-off-by: David Gageot --- pkg/acp/filesystem.go | 7 ++- pkg/acp/filesystem_test.go | 87 +++++++++++++++++++++++++++++ pkg/acp/pathcase_caseinsensitive.go | 12 ++++ pkg/acp/pathcase_unix.go | 8 +++ 4 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 pkg/acp/filesystem_test.go create mode 100644 pkg/acp/pathcase_caseinsensitive.go create mode 100644 pkg/acp/pathcase_unix.go diff --git a/pkg/acp/filesystem.go b/pkg/acp/filesystem.go index 9a64faf1d..9fcdf502d 100644 --- a/pkg/acp/filesystem.go +++ b/pkg/acp/filesystem.go @@ -80,8 +80,11 @@ func (t *FilesystemToolset) resolvePath(userPath string) (string, error) { if err != nil { return "", fmt.Errorf("failed to resolve path: %w", err) } - // Ensure the resolved path is within the working directory - if !strings.HasPrefix(absResolved, absWorkingDir+string(filepath.Separator)) && absResolved != absWorkingDir { + // Normalize paths for comparison to prevent bypasses on case-insensitive + // filesystems (macOS, Windows) where differing case could defeat the check. + normResolved := normalizePathForComparison(absResolved) + normWorkingDir := normalizePathForComparison(absWorkingDir) + if !strings.HasPrefix(normResolved, normWorkingDir+string(filepath.Separator)) && normResolved != normWorkingDir { return "", fmt.Errorf("path %q escapes the working directory", userPath) } return absResolved, nil diff --git a/pkg/acp/filesystem_test.go b/pkg/acp/filesystem_test.go new file mode 100644 index 000000000..77b2db011 --- /dev/null +++ b/pkg/acp/filesystem_test.go @@ -0,0 +1,87 @@ +package acp + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestResolvePath(t *testing.T) { + t.Parallel() + + workingDir := t.TempDir() + + ts := &FilesystemToolset{ + workingDir: workingDir, + } + + absWorkingDir, err := filepath.Abs(workingDir) + require.NoError(t, err) + + tests := []struct { + name string + userPath string + wantPath string + wantError bool + }{ + { + name: "simple relative path", + userPath: "file.txt", + wantPath: filepath.Join(absWorkingDir, "file.txt"), + }, + { + name: "nested relative path", + userPath: "subdir/file.txt", + wantPath: filepath.Join(absWorkingDir, "subdir", "file.txt"), + }, + { + name: "dot path resolves to working directory", + userPath: ".", + wantPath: absWorkingDir, + }, + { + name: "parent directory escape blocked", + userPath: "../escape.txt", + wantError: true, + }, + { + name: "deep parent directory escape blocked", + userPath: "subdir/../../escape.txt", + wantError: true, + }, + { + name: "dot-dot within working dir is fine", + userPath: "subdir/../file.txt", + wantPath: filepath.Join(absWorkingDir, "file.txt"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + resolved, err := ts.resolvePath(tt.userPath) + if tt.wantError { + require.Error(t, err) + assert.Contains(t, err.Error(), "escapes the working directory") + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantPath, resolved) + }) + } +} + +func TestNormalizePathForComparison(t *testing.T) { + t.Parallel() + + // On macOS/Windows (case-insensitive), normalization should lowercase. + // On Linux (case-sensitive), it should be identity. + result := normalizePathForComparison("/Some/Path") + + // This test validates the function exists and returns a string. + // The exact behavior depends on the platform. + assert.NotEmpty(t, result) +} diff --git a/pkg/acp/pathcase_caseinsensitive.go b/pkg/acp/pathcase_caseinsensitive.go new file mode 100644 index 000000000..2297ab48b --- /dev/null +++ b/pkg/acp/pathcase_caseinsensitive.go @@ -0,0 +1,12 @@ +//go:build windows || darwin + +package acp + +import "strings" + +// normalizePathForComparison lowercases the path on case-insensitive filesystems +// (macOS and Windows) to ensure path traversal checks cannot be bypassed by +// varying the case of path components. +func normalizePathForComparison(path string) string { + return strings.ToLower(path) +} diff --git a/pkg/acp/pathcase_unix.go b/pkg/acp/pathcase_unix.go new file mode 100644 index 000000000..151f19a6b --- /dev/null +++ b/pkg/acp/pathcase_unix.go @@ -0,0 +1,8 @@ +//go:build !windows && !darwin + +package acp + +// normalizePathForComparison returns the path unchanged on case-sensitive filesystems (Linux). +func normalizePathForComparison(path string) string { + return path +}