Skip to content

Commit 2bdc149

Browse files
committed
fix: 修复 read 工具路径解析问题
- 添加 home 目录路径 (~) 展开支持 - 添加外部路径权限检查和审批流程 - 改进路径解析逻辑,支持绝对路径和相对路径 Made-with: Cursor
1 parent c35daa4 commit 2bdc149

6 files changed

Lines changed: 284 additions & 12 deletions

File tree

internal/bootstrap/build_helpers.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func buildToolRegistry(
7070
todoWriteTool := tools.NewTodoWriteTool(store, func() string { return *sessionIDRef })
7171

7272
toolList := []tools.Tool{
73-
tools.NewReadTool(ws),
73+
tools.NewReadTool(ws, policy),
7474
tools.NewWriteTool(ws),
7575
tools.NewEditTool(ws),
7676
tools.NewListTool(ws),

internal/config/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ func Default() Config {
242242
"pnpm test*": "allow",
243243
"yarn test*": "allow",
244244
},
245-
ExternalDir: "deny",
245+
ExternalDir: "ask",
246246
},
247247
Workflow: WorkflowConfig{
248248
RequireTodoForComplex: true,

internal/permission/policy.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,13 +254,15 @@ func PresetConfig(name string) (config.PermissionConfig, bool) {
254254
Default: "ask", Read: "allow", Edit: "ask", Write: "ask", List: "allow", Glob: "allow", Grep: "allow", Patch: "ask",
255255
LSPDiagnostics: "allow", LSPDefinition: "allow", LSPHover: "allow",
256256
TodoRead: "allow", TodoWrite: "allow", Skill: "ask", Task: "ask", Fetch: "ask",
257+
ExternalDir: "ask",
257258
Bash: map[string]string{"*": "ask", "ls *": "allow", "cat *": "allow", "grep *": "allow", "go test *": "allow", "pytest*": "allow", "npm test*": "allow", "pnpm test*": "allow", "yarn test*": "allow"},
258259
}, true
259260
case "plan":
260261
return config.PermissionConfig{
261262
Default: "ask", Read: "allow", Edit: "deny", Write: "deny", List: "allow", Glob: "allow", Grep: "allow", Patch: "deny",
262263
LSPDiagnostics: "allow", LSPDefinition: "allow", LSPHover: "allow",
263264
TodoRead: "allow", TodoWrite: "allow", Skill: "allow", Task: "deny", Fetch: "allow", Question: "allow",
265+
ExternalDir: "ask",
264266
Bash: map[string]string{
265267
"*": "ask",
266268
"ls": "allow",
@@ -296,3 +298,8 @@ func (p *Policy) ApplyPreset(name string) bool {
296298
p.cfg = cfg
297299
return true
298300
}
301+
302+
// ExternalDirDecision 返回外部目录访问权限决策
303+
func (p *Policy) ExternalDirDecision() Decision {
304+
return normalizeDecision(p.cfg.ExternalDir, DecisionAsk)
305+
}

internal/tools/read.go

Lines changed: 138 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,24 +6,81 @@ import (
66
"encoding/json"
77
"fmt"
88
"os"
9+
"path/filepath"
910
"strings"
1011

1112
"coder/internal/chat"
13+
"coder/internal/permission"
1214
"coder/internal/security"
1315
)
1416

1517
type ReadTool struct {
16-
ws *security.Workspace
18+
ws *security.Workspace
19+
policy *permission.Policy
1720
}
1821

19-
func NewReadTool(ws *security.Workspace) *ReadTool {
20-
return &ReadTool{ws: ws}
22+
func NewReadTool(ws *security.Workspace, policy *permission.Policy) *ReadTool {
23+
return &ReadTool{ws: ws, policy: policy}
2124
}
2225

2326
func (t *ReadTool) Name() string {
2427
return "read"
2528
}
2629

30+
// ApprovalRequest 实现 ApprovalAware 接口,检查是否需要审批外部路径
31+
// ApprovalRequest implements ApprovalAware interface to check if external path needs approval
32+
func (t *ReadTool) ApprovalRequest(args json.RawMessage) (*ApprovalRequest, error) {
33+
var in struct {
34+
Path string `json:"path"`
35+
}
36+
if err := json.Unmarshal(args, &in); err != nil {
37+
return nil, fmt.Errorf("parse args: %w", err)
38+
}
39+
40+
path := strings.TrimSpace(in.Path)
41+
if path == "" {
42+
return nil, nil
43+
}
44+
45+
// 解析路径(展开 ~)
46+
if strings.HasPrefix(path, "~") {
47+
expanded, err := t.expandHomePath(path)
48+
if err != nil {
49+
return nil, nil // 解析失败,让 Execute 处理错误
50+
}
51+
path = expanded
52+
}
53+
54+
// 如果不是绝对路径,不需要审批(相对路径限制在 workspace 内)
55+
if !filepath.IsAbs(path) {
56+
return nil, nil
57+
}
58+
59+
// 检查是否在 workspace 内
60+
rel, err := filepath.Rel(t.ws.Root(), path)
61+
if err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
62+
// 在 workspace 内,不需要审批
63+
return nil, nil
64+
}
65+
66+
// 在 workspace 外,检查权限策略
67+
decision := t.policy.ExternalDirDecision()
68+
switch decision {
69+
case permission.DecisionAllow:
70+
// 允许,不需要审批
71+
return nil, nil
72+
case permission.DecisionDeny:
73+
// 拒绝,不需要审批(直接拒绝)
74+
return nil, nil
75+
default:
76+
// ask,需要审批
77+
return &ApprovalRequest{
78+
Tool: t.Name(),
79+
Reason: fmt.Sprintf("read external path: %s", in.Path),
80+
}, nil
81+
}
82+
}
83+
2784
func (t *ReadTool) Definition() chat.ToolDef {
2885
return chat.ToolDef{
2986
Type: "function",
@@ -75,9 +132,9 @@ func (t *ReadTool) Execute(_ context.Context, args json.RawMessage) (string, err
75132
if in.Limit > maxLimit {
76133
in.Limit = maxLimit
77134
}
78-
resolved, err := t.ws.Resolve(in.Path)
79-
if err != nil {
80-
return "", fmt.Errorf("resolve path: %w", err)
135+
resolved, resolveErr := t.resolvePath(in.Path)
136+
if resolveErr != nil {
137+
return "", fmt.Errorf("resolve path: %w", resolveErr)
81138
}
82139
f, err := os.Open(resolved)
83140
if err != nil {
@@ -153,3 +210,78 @@ func (t *ReadTool) Execute(_ context.Context, args json.RawMessage) (string, err
153210
"has_more": hasMore,
154211
}), nil
155212
}
213+
214+
// resolvePath 统一处理路径解析,支持相对路径、绝对路径和 ~ 路径
215+
// resolvePath handles path resolution for relative, absolute and ~ paths
216+
func (t *ReadTool) resolvePath(path string) (string, error) {
217+
path = strings.TrimSpace(path)
218+
if path == "" {
219+
return "", fmt.Errorf("empty path")
220+
}
221+
222+
// 1. 处理 ~ 路径:展开为家目录绝对路径
223+
// Handle ~ path: expand to home directory
224+
if strings.HasPrefix(path, "~") {
225+
expanded, err := t.expandHomePath(path)
226+
if err != nil {
227+
return "", err
228+
}
229+
path = expanded
230+
}
231+
232+
// 2. 如果是绝对路径,检查外部权限
233+
// If absolute path, check external permission
234+
if filepath.IsAbs(path) {
235+
return t.checkExternalPath(path)
236+
}
237+
238+
// 3. 相对路径:限制在 workspace 内
239+
// Relative path: restrict to workspace
240+
return t.ws.Resolve(path)
241+
}
242+
243+
// expandHomePath 将 ~ 展开为家目录绝对路径
244+
// expandHomePath expands ~ to home directory absolute path
245+
func (t *ReadTool) expandHomePath(path string) (string, error) {
246+
home, err := os.UserHomeDir()
247+
if err != nil {
248+
return "", fmt.Errorf("resolve home dir: %w", err)
249+
}
250+
251+
if path == "~" {
252+
return home, nil
253+
}
254+
if strings.HasPrefix(path, "~/") {
255+
return filepath.Join(home, strings.TrimPrefix(path, "~/")), nil
256+
}
257+
// ~username 格式暂不支持 / ~username format not supported
258+
return "", fmt.Errorf("unsupported path format: %s", path)
259+
}
260+
261+
// checkExternalPath 检查外部路径权限
262+
// checkExternalPath checks external path permission
263+
// 注意:当策略为 ask 时,如果 Execute 被调用,说明审批已通过
264+
// Note: when policy is ask, if Execute is called, approval has been granted
265+
func (t *ReadTool) checkExternalPath(absPath string) (string, error) {
266+
// 检查路径是否在 workspace 内 / Check if path is inside workspace
267+
rel, err := filepath.Rel(t.ws.Root(), absPath)
268+
if err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
269+
// 路径在 workspace 内,直接允许 / Path inside workspace, allow directly
270+
return absPath, nil
271+
}
272+
273+
// 路径在 workspace 外,检查外部路径权限 / Path outside workspace, check permission
274+
decision := t.policy.ExternalDirDecision()
275+
switch decision {
276+
case permission.DecisionAllow:
277+
// 明确允许 / Explicitly allowed
278+
return absPath, nil
279+
case permission.DecisionDeny:
280+
// 明确拒绝 / Explicitly denied
281+
return "", fmt.Errorf("external path access denied by policy")
282+
default:
283+
// ask 策略:如果 Execute 被调用,说明审批已通过
284+
// ask policy: if Execute is called, approval has been granted
285+
return absPath, nil
286+
}
287+
}

internal/tools/read_test.go

Lines changed: 136 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"strings"
1010
"testing"
1111

12+
"coder/internal/config"
13+
"coder/internal/permission"
1214
"coder/internal/security"
1315
)
1416

@@ -28,7 +30,9 @@ func TestReadToolSmallFileDefaultLimit(t *testing.T) {
2830
if err != nil {
2931
t.Fatal(err)
3032
}
31-
tool := NewReadTool(ws)
33+
cfg, _ := permission.PresetConfig("build")
34+
policy := permission.New(cfg)
35+
tool := NewReadTool(ws, policy)
3236

3337
// 仅传 path,不带 offset/limit,期望读出全部 10 行
3438
args, _ := json.Marshal(map[string]any{
@@ -77,7 +81,9 @@ func TestReadToolLargeFilePagination(t *testing.T) {
7781
if err != nil {
7882
t.Fatal(err)
7983
}
80-
tool := NewReadTool(ws)
84+
cfg, _ := permission.PresetConfig("build")
85+
policy := permission.New(cfg)
86+
tool := NewReadTool(ws, policy)
8187

8288
// 默认只读前 50 行
8389
args, _ := json.Marshal(map[string]any{
@@ -126,7 +132,9 @@ func TestReadToolOffsetAndLimit(t *testing.T) {
126132
if err != nil {
127133
t.Fatal(err)
128134
}
129-
tool := NewReadTool(ws)
135+
cfg, _ := permission.PresetConfig("build")
136+
policy := permission.New(cfg)
137+
tool := NewReadTool(ws, policy)
130138

131139
// 从第 51 行开始读 50 行
132140
args, _ := json.Marshal(map[string]any{
@@ -172,7 +180,9 @@ func TestReadToolOffsetBeyondEOFAndInvalidLimit(t *testing.T) {
172180
if err != nil {
173181
t.Fatal(err)
174182
}
175-
tool := NewReadTool(ws)
183+
cfg, _ := permission.PresetConfig("build")
184+
policy := permission.New(cfg)
185+
tool := NewReadTool(ws, policy)
176186

177187
// offset 大于文件总行数,期望 content 为空、has_more=false
178188
argsBeyond, _ := json.Marshal(map[string]any{
@@ -204,3 +214,125 @@ func TestReadToolOffsetBeyondEOFAndInvalidLimit(t *testing.T) {
204214
t.Fatalf("execute read (invalid limit): %v", err)
205215
}
206216
}
217+
218+
// TestReadToolExternalPathApproval 测试外部路径审批流程
219+
func TestReadToolExternalPathApproval(t *testing.T) {
220+
// 创建一个临时目录作为 workspace
221+
wsRoot := t.TempDir()
222+
ws, err := security.NewWorkspace(wsRoot)
223+
if err != nil {
224+
t.Fatal(err)
225+
}
226+
227+
// 在工作区外创建一个文件
228+
externalDir := t.TempDir()
229+
externalFile := filepath.Join(externalDir, "external.txt")
230+
externalContent := "external file content"
231+
if err := os.WriteFile(externalFile, []byte(externalContent), 0o644); err != nil {
232+
t.Fatal(err)
233+
}
234+
235+
// 测试策略为 ask 时,ApprovalRequest 应该返回需要审批
236+
t.Run("ask_policy_returns_approval_request", func(t *testing.T) {
237+
cfg := config.PermissionConfig{ExternalDir: "ask"}
238+
policy := permission.New(cfg)
239+
tool := NewReadTool(ws, policy)
240+
241+
args, _ := json.Marshal(map[string]any{
242+
"path": externalFile,
243+
})
244+
245+
req, err := tool.ApprovalRequest(args)
246+
if err != nil {
247+
t.Fatalf("ApprovalRequest error: %v", err)
248+
}
249+
if req == nil {
250+
t.Fatal("expected ApprovalRequest for external path, got nil")
251+
}
252+
if req.Tool != "read" {
253+
t.Fatalf("expected tool name 'read', got %q", req.Tool)
254+
}
255+
})
256+
257+
// 测试策略为 allow 时,ApprovalRequest 应该返回 nil
258+
t.Run("allow_policy_no_approval_request", func(t *testing.T) {
259+
cfg := config.PermissionConfig{ExternalDir: "allow"}
260+
policy := permission.New(cfg)
261+
tool := NewReadTool(ws, policy)
262+
263+
args, _ := json.Marshal(map[string]any{
264+
"path": externalFile,
265+
})
266+
267+
req, err := tool.ApprovalRequest(args)
268+
if err != nil {
269+
t.Fatalf("ApprovalRequest error: %v", err)
270+
}
271+
if req != nil {
272+
t.Fatalf("expected no ApprovalRequest for allow policy, got %v", req)
273+
}
274+
})
275+
276+
// 测试策略为 deny 时,ApprovalRequest 应该返回 nil(直接拒绝)
277+
t.Run("deny_policy_no_approval_request", func(t *testing.T) {
278+
cfg := config.PermissionConfig{ExternalDir: "deny"}
279+
policy := permission.New(cfg)
280+
tool := NewReadTool(ws, policy)
281+
282+
args, _ := json.Marshal(map[string]any{
283+
"path": externalFile,
284+
})
285+
286+
req, err := tool.ApprovalRequest(args)
287+
if err != nil {
288+
t.Fatalf("ApprovalRequest error: %v", err)
289+
}
290+
if req != nil {
291+
t.Fatalf("expected no ApprovalRequest for deny policy, got %v", req)
292+
}
293+
})
294+
295+
// 测试 ~ 路径展开和审批
296+
t.Run("home_path_expansion_and_approval", func(t *testing.T) {
297+
cfg := config.PermissionConfig{ExternalDir: "ask"}
298+
policy := permission.New(cfg)
299+
tool := NewReadTool(ws, policy)
300+
301+
args, _ := json.Marshal(map[string]any{
302+
"path": "~/test_file.txt",
303+
})
304+
305+
req, err := tool.ApprovalRequest(args)
306+
if err != nil {
307+
t.Fatalf("ApprovalRequest error: %v", err)
308+
}
309+
if req == nil {
310+
t.Fatal("expected ApprovalRequest for ~ path, got nil")
311+
}
312+
})
313+
314+
// 测试工作区内路径不需要审批
315+
t.Run("workspace_path_no_approval", func(t *testing.T) {
316+
cfg := config.PermissionConfig{ExternalDir: "ask"}
317+
policy := permission.New(cfg)
318+
tool := NewReadTool(ws, policy)
319+
320+
// 在工作区内创建文件
321+
internalFile := filepath.Join(wsRoot, "internal.txt")
322+
if err := os.WriteFile(internalFile, []byte("internal content"), 0o644); err != nil {
323+
t.Fatal(err)
324+
}
325+
326+
args, _ := json.Marshal(map[string]any{
327+
"path": "internal.txt",
328+
})
329+
330+
req, err := tool.ApprovalRequest(args)
331+
if err != nil {
332+
t.Fatalf("ApprovalRequest error: %v", err)
333+
}
334+
if req != nil {
335+
t.Fatalf("expected no ApprovalRequest for workspace path, got %v", req)
336+
}
337+
})
338+
}

0 commit comments

Comments
 (0)