git.iko.soy/crush
on main help sign in
fix(tools/touch): gate outside-workingDir paths via permission prompt

Mirrors the view.go sanitizer pattern (filepath.Abs + filepath.Rel + ".." check) so CodeQL no longer flags os.Stat/MkdirAll/OpenFile in the touch tool as uncontrolled path expressions.

commit0da13d76995011ec0ba85bf19d444710f04fdf88 authorvorticalbox <vorticalbox@protonmail.com> dateWed 29 Apr 2026, 18:14 (6 months ago) parent010ca2f
2 files changed+129−15
compare with main
unifiedsplit
internal/agent/tools/touch.go+49−15 internal/agent/tools/touch_test.go+80
internal/agent/tools/touch.go+49−15
@@ -6,6 +6,7 @@ import (
66 "fmt"
77 "os"
88 "path/filepath"
9+ "strings"
910
1011 "charm.land/fantasy"
1112 "github.com/charmbracelet/crush/internal/filepathext"
@@ -57,17 +58,50 @@ func NewTouchTool(
5758
5859 filePath := filepathext.SmartJoin(workingDir, params.FilePath)
5960
60− fileInfo, err := os.Stat(filePath)
61+ absWorkingDir, err := filepath.Abs(workingDir)
62+ if err != nil {
63+ return fantasy.ToolResponse{}, fmt.Errorf("error resolving working directory: %w", err)
64+ }
65+ absFilePath, err := filepath.Abs(filePath)
66+ if err != nil {
67+ return fantasy.ToolResponse{}, fmt.Errorf("error resolving file path: %w", err)
68+ }
69+ relPath, relErr := filepath.Rel(absWorkingDir, absFilePath)
70+ isOutsideWorkDir := relErr != nil || strings.HasPrefix(relPath, "..")
71+
72+ if isOutsideWorkDir {
73+ granted, permReqErr := permissions.Request(ctx,
74+ permission.CreatePermissionRequest{
75+ SessionID: sessionID,
76+ Path: absFilePath,
77+ ToolCallID: call.ID,
78+ ToolName: TouchToolName,
79+ Action: "write",
80+ Description: fmt.Sprintf("Create empty file outside working directory: %s", absFilePath),
81+ Params: TouchPermissionsParams{
82+ FilePath: absFilePath,
83+ },
84+ },
85+ )
86+ if permReqErr != nil {
87+ return fantasy.ToolResponse{}, permReqErr
88+ }
89+ if !granted {
90+ return NewPermissionDeniedResponse(), nil
91+ }
92+ }
93+
94+ fileInfo, err := os.Stat(absFilePath)
6195 if err == nil {
6296 if fileInfo.IsDir() {
63− return fantasy.NewTextErrorResponse(fmt.Sprintf("Path is a directory, not a file: %s", filePath)), nil
97+ return fantasy.NewTextErrorResponse(fmt.Sprintf("Path is a directory, not a file: %s", absFilePath)), nil
6498 }
65− return fantasy.NewTextErrorResponse(fmt.Sprintf("File already exists: %s", filePath)), nil
99+ return fantasy.NewTextErrorResponse(fmt.Sprintf("File already exists: %s", absFilePath)), nil
66100 } else if !os.IsNotExist(err) {
67101 return fantasy.ToolResponse{}, fmt.Errorf("error checking file: %w", err)
68102 }
69103
70− dir := filepath.Dir(filePath)
104+ dir := filepath.Dir(absFilePath)
71105 if err = os.MkdirAll(dir, 0o755); err != nil {
72106 return fantasy.ToolResponse{}, fmt.Errorf("error creating directory: %w", err)
73107 }
@@ -75,13 +109,13 @@ func NewTouchTool(
75109 p, err := permissions.Request(ctx,
76110 permission.CreatePermissionRequest{
77111 SessionID: sessionID,
78− Path: fsext.PathOrPrefix(filePath, workingDir),
112+ Path: fsext.PathOrPrefix(absFilePath, absWorkingDir),
79113 ToolCallID: call.ID,
80114 ToolName: TouchToolName,
81115 Action: "write",
82− Description: fmt.Sprintf("Create empty file %s", filePath),
116+ Description: fmt.Sprintf("Create empty file %s", absFilePath),
83117 Params: TouchPermissionsParams{
84− FilePath: filePath,
118+ FilePath: absFilePath,
85119 OldContent: "",
86120 NewContent: "",
87121 },
@@ -94,10 +128,10 @@ func NewTouchTool(
94128 return NewPermissionDeniedResponse(), nil
95129 }
96130
97− file, err := os.OpenFile(filePath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o644)
131+ file, err := os.OpenFile(absFilePath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o644)
98132 if err != nil {
99133 if os.IsExist(err) {
100− return fantasy.NewTextErrorResponse(fmt.Sprintf("File already exists: %s", filePath)), nil
134+ return fantasy.NewTextErrorResponse(fmt.Sprintf("File already exists: %s", absFilePath)), nil
101135 }
102136 return fantasy.ToolResponse{}, fmt.Errorf("error creating file: %w", err)
103137 }
@@ -105,21 +139,21 @@ func NewTouchTool(
105139 return fantasy.ToolResponse{}, fmt.Errorf("error closing file: %w", err)
106140 }
107141
108− _, err = files.Create(ctx, sessionID, filePath, "")
142+ _, err = files.Create(ctx, sessionID, absFilePath, "")
109143 if err != nil {
110144 return fantasy.ToolResponse{}, fmt.Errorf("error creating file history: %w", err)
111145 }
112146
113− filetracker.RecordRead(ctx, sessionID, filePath)
147+ filetracker.RecordRead(ctx, sessionID, absFilePath)
114148
115− notifyLSPs(ctx, lspManager, filePath)
149+ notifyLSPs(ctx, lspManager, absFilePath)
116150
117− result := fmt.Sprintf("Empty file successfully created: %s", filePath)
151+ result := fmt.Sprintf("Empty file successfully created: %s", absFilePath)
118152 result = fmt.Sprintf("<result>\n%s\n</result>", result)
119− result += getDiagnostics(filePath, lspManager)
153+ result += getDiagnostics(absFilePath, lspManager)
120154 return fantasy.WithResponseMetadata(fantasy.NewTextResponse(result),
121155 TouchResponseMetadata{
122− FilePath: filePath,
156+ FilePath: absFilePath,
123157 },
124158 ), nil
125159 })
internal/agent/tools/touch_test.go+80
@@ -5,13 +5,39 @@ import (
55 "encoding/json"
66 "os"
77 "path/filepath"
8+ "strings"
89 "testing"
910 "time"
1011
1112 "charm.land/fantasy"
13+ "github.com/charmbracelet/crush/internal/permission"
14+ "github.com/charmbracelet/crush/internal/pubsub"
1215 "github.com/stretchr/testify/require"
1316)
1417
18+// recordingPermissionService captures permission requests and answers them
19+// according to a configurable response.
20+type recordingPermissionService struct {
21+ *pubsub.Broker[permission.PermissionRequest]
22+ requests []permission.CreatePermissionRequest
23+ grant bool
24+}
25+
26+func (m *recordingPermissionService) Request(ctx context.Context, req permission.CreatePermissionRequest) (bool, error) {
27+ m.requests = append(m.requests, req)
28+ return m.grant, nil
29+}
30+
31+func (m *recordingPermissionService) Grant(req permission.PermissionRequest) {}
32+func (m *recordingPermissionService) Deny(req permission.PermissionRequest) {}
33+func (m *recordingPermissionService) GrantPersistent(req permission.PermissionRequest) {}
34+func (m *recordingPermissionService) AutoApproveSession(sessionID string) {}
35+func (m *recordingPermissionService) SetSkipRequests(skip bool) {}
36+func (m *recordingPermissionService) SkipRequests() bool { return false }
37+func (m *recordingPermissionService) SubscribeNotifications(ctx context.Context) <-chan pubsub.Event[permission.PermissionNotification] {
38+ return make(<-chan pubsub.Event[permission.PermissionNotification])
39+}
40+
1541type mockFileTrackerService struct{}
1642
1743func (m mockFileTrackerService) RecordRead(ctx context.Context, sessionID, path string) {}
@@ -60,6 +86,60 @@ func TestTouchToolRefusesExistingFile(t *testing.T) {
6086 require.Equal(t, "content", string(content))
6187}
6288
89+func TestTouchToolStaysInsideWorkingDir(t *testing.T) {
90+ t.Parallel()
91+
92+ workingDir := t.TempDir()
93+ perms := &recordingPermissionService{grant: true}
94+ tool := NewTouchTool(nil, perms, &mockHistoryService{}, mockFileTrackerService{}, workingDir)
95+ ctx := context.WithValue(context.Background(), SessionIDContextKey, "test-session")
96+
97+ resp := runTouchTool(t, tool, ctx, TouchParams{FilePath: "inside.txt"})
98+ require.False(t, resp.IsError)
99+
100+ for _, req := range perms.requests {
101+ require.NotContains(t, req.Description, "outside working directory",
102+ "inside-workingDir touch should not trigger an outside-workingDir permission prompt")
103+ }
104+
105+ _, err := os.Stat(filepath.Join(workingDir, "inside.txt"))
106+ require.NoError(t, err)
107+}
108+
109+func TestTouchToolOutsideWorkingDirRequiresPermission(t *testing.T) {
110+ t.Parallel()
111+
112+ parent := t.TempDir()
113+ workingDir := filepath.Join(parent, "wd")
114+ require.NoError(t, os.MkdirAll(workingDir, 0o755))
115+
116+ // Denied: file outside workingDir must not be created.
117+ deny := &recordingPermissionService{grant: false}
118+ tool := NewTouchTool(nil, deny, &mockHistoryService{}, mockFileTrackerService{}, workingDir)
119+ ctx := context.WithValue(context.Background(), SessionIDContextKey, "test-session")
120+
121+ resp := runTouchTool(t, tool, ctx, TouchParams{FilePath: "../escape.txt"})
122+ require.True(t, resp.IsError)
123+
124+ require.Len(t, deny.requests, 1)
125+ require.True(t, strings.Contains(deny.requests[0].Description, "outside working directory"),
126+ "expected outside-working-directory permission prompt, got %q", deny.requests[0].Description)
127+
128+ _, err := os.Stat(filepath.Join(parent, "escape.txt"))
129+ require.True(t, os.IsNotExist(err), "denied permission should not create the file")
130+
131+ // Granted: same path now succeeds.
132+ grant := &recordingPermissionService{grant: true}
133+ tool = NewTouchTool(nil, grant, &mockHistoryService{}, mockFileTrackerService{}, workingDir)
134+ resp = runTouchTool(t, tool, ctx, TouchParams{FilePath: "../escape.txt"})
135+ require.False(t, resp.IsError)
136+ require.GreaterOrEqual(t, len(grant.requests), 1)
137+ require.Contains(t, grant.requests[0].Description, "outside working directory")
138+
139+ _, err = os.Stat(filepath.Join(parent, "escape.txt"))
140+ require.NoError(t, err)
141+}
142+
63143func TestWriteToolEmptyContentPointsToTouch(t *testing.T) {
64144 t.Parallel()
65145