From 2bdc149511e73526cbaf28c463b17fcf216782b5 Mon Sep 17 00:00:00 2001 From: yingchaox <1020316234@qq.com> Date: Fri, 27 Feb 2026 00:06:00 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E4=BF=AE=E5=A4=8D=20read=20=E5=B7=A5?= =?UTF-8?q?=E5=85=B7=E8=B7=AF=E5=BE=84=E8=A7=A3=E6=9E=90=E9=97=AE=E9=A2=98?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 添加 home 目录路径 (~) 展开支持 - 添加外部路径权限检查和审批流程 - 改进路径解析逻辑，支持绝对路径和相对路径 Made-with: Cursor --- internal/bootstrap/build_helpers.go | 2 +- internal/config/config.go | 2 +- internal/permission/policy.go | 7 ++ internal/tools/read.go | 144 ++++++++++++++++++++++++++-- internal/tools/read_test.go | 140 ++++++++++++++++++++++++++- opencode | 1 + 6 files changed, 284 insertions(+), 12 deletions(-) create mode 160000 opencode diff --git a/internal/bootstrap/build_helpers.go b/internal/bootstrap/build_helpers.go index 9d56d1b..e1a904c 100644 --- a/internal/bootstrap/build_helpers.go +++ b/internal/bootstrap/build_helpers.go @@ -70,7 +70,7 @@ func buildToolRegistry( todoWriteTool := tools.NewTodoWriteTool(store, func() string { return *sessionIDRef }) toolList := []tools.Tool{ - tools.NewReadTool(ws), + tools.NewReadTool(ws, policy), tools.NewWriteTool(ws), tools.NewEditTool(ws), tools.NewListTool(ws), diff --git a/internal/config/config.go b/internal/config/config.go index 9777b8d..d35dd3f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -242,7 +242,7 @@ func Default() Config { "pnpm test*": "allow", "yarn test*": "allow", }, - ExternalDir: "deny", + ExternalDir: "ask", }, Workflow: WorkflowConfig{ RequireTodoForComplex: true, diff --git a/internal/permission/policy.go b/internal/permission/policy.go index 58e0e97..48d7c8d 100644 --- a/internal/permission/policy.go +++ b/internal/permission/policy.go @@ -254,6 +254,7 @@ func PresetConfig(name string) (config.PermissionConfig, bool) { Default: "ask", Read: "allow", Edit: "ask", Write: "ask", List: "allow", Glob: "allow", Grep: "allow", Patch: "ask", LSPDiagnostics: "allow", LSPDefinition: "allow", LSPHover: "allow", TodoRead: "allow", TodoWrite: "allow", Skill: "ask", Task: "ask", Fetch: "ask", + ExternalDir: "ask", 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"}, }, true case "plan": @@ -261,6 +262,7 @@ func PresetConfig(name string) (config.PermissionConfig, bool) { Default: "ask", Read: "allow", Edit: "deny", Write: "deny", List: "allow", Glob: "allow", Grep: "allow", Patch: "deny", LSPDiagnostics: "allow", LSPDefinition: "allow", LSPHover: "allow", TodoRead: "allow", TodoWrite: "allow", Skill: "allow", Task: "deny", Fetch: "allow", Question: "allow", + ExternalDir: "ask", Bash: map[string]string{ "*": "ask", "ls": "allow", @@ -296,3 +298,8 @@ func (p *Policy) ApplyPreset(name string) bool { p.cfg = cfg return true } + +// ExternalDirDecision 返回外部目录访问权限决策 +func (p *Policy) ExternalDirDecision() Decision { + return normalizeDecision(p.cfg.ExternalDir, DecisionAsk) +} diff --git a/internal/tools/read.go b/internal/tools/read.go index 0cf3d4b..190094c 100644 --- a/internal/tools/read.go +++ b/internal/tools/read.go @@ -6,24 +6,81 @@ import ( "encoding/json" "fmt" "os" + "path/filepath" "strings" "coder/internal/chat" + "coder/internal/permission" "coder/internal/security" ) type ReadTool struct { - ws *security.Workspace + ws *security.Workspace + policy *permission.Policy } -func NewReadTool(ws *security.Workspace) *ReadTool { - return &ReadTool{ws: ws} +func NewReadTool(ws *security.Workspace, policy *permission.Policy) *ReadTool { + return &ReadTool{ws: ws, policy: policy} } func (t *ReadTool) Name() string { return "read" } +// ApprovalRequest 实现 ApprovalAware 接口，检查是否需要审批外部路径 +// ApprovalRequest implements ApprovalAware interface to check if external path needs approval +func (t *ReadTool) ApprovalRequest(args json.RawMessage) (*ApprovalRequest, error) { + var in struct { + Path string `json:"path"` + } + if err := json.Unmarshal(args, &in); err != nil { + return nil, fmt.Errorf("parse args: %w", err) + } + + path := strings.TrimSpace(in.Path) + if path == "" { + return nil, nil + } + + // 解析路径（展开 ~） + if strings.HasPrefix(path, "~") { + expanded, err := t.expandHomePath(path) + if err != nil { + return nil, nil // 解析失败，让 Execute 处理错误 + } + path = expanded + } + + // 如果不是绝对路径，不需要审批（相对路径限制在 workspace 内） + if !filepath.IsAbs(path) { + return nil, nil + } + + // 检查是否在 workspace 内 + rel, err := filepath.Rel(t.ws.Root(), path) + if err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { + // 在 workspace 内，不需要审批 + return nil, nil + } + + // 在 workspace 外，检查权限策略 + decision := t.policy.ExternalDirDecision() + switch decision { + case permission.DecisionAllow: + // 允许，不需要审批 + return nil, nil + case permission.DecisionDeny: + // 拒绝，不需要审批（直接拒绝） + return nil, nil + default: + // ask，需要审批 + return &ApprovalRequest{ + Tool: t.Name(), + Reason: fmt.Sprintf("read external path: %s", in.Path), + }, nil + } +} + func (t *ReadTool) Definition() chat.ToolDef { return chat.ToolDef{ Type: "function", @@ -75,9 +132,9 @@ func (t *ReadTool) Execute(_ context.Context, args json.RawMessage) (string, err if in.Limit > maxLimit { in.Limit = maxLimit } - resolved, err := t.ws.Resolve(in.Path) - if err != nil { - return "", fmt.Errorf("resolve path: %w", err) + resolved, resolveErr := t.resolvePath(in.Path) + if resolveErr != nil { + return "", fmt.Errorf("resolve path: %w", resolveErr) } f, err := os.Open(resolved) if err != nil { @@ -153,3 +210,78 @@ func (t *ReadTool) Execute(_ context.Context, args json.RawMessage) (string, err "has_more": hasMore, }), nil } + +// resolvePath 统一处理路径解析，支持相对路径、绝对路径和 ~ 路径 +// resolvePath handles path resolution for relative, absolute and ~ paths +func (t *ReadTool) resolvePath(path string) (string, error) { + path = strings.TrimSpace(path) + if path == "" { + return "", fmt.Errorf("empty path") + } + + // 1. 处理 ~ 路径：展开为家目录绝对路径 + // Handle ~ path: expand to home directory + if strings.HasPrefix(path, "~") { + expanded, err := t.expandHomePath(path) + if err != nil { + return "", err + } + path = expanded + } + + // 2. 如果是绝对路径，检查外部权限 + // If absolute path, check external permission + if filepath.IsAbs(path) { + return t.checkExternalPath(path) + } + + // 3. 相对路径：限制在 workspace 内 + // Relative path: restrict to workspace + return t.ws.Resolve(path) +} + +// expandHomePath 将 ~ 展开为家目录绝对路径 +// expandHomePath expands ~ to home directory absolute path +func (t *ReadTool) expandHomePath(path string) (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("resolve home dir: %w", err) + } + + if path == "~" { + return home, nil + } + if strings.HasPrefix(path, "~/") { + return filepath.Join(home, strings.TrimPrefix(path, "~/")), nil + } + // ~username 格式暂不支持 / ~username format not supported + return "", fmt.Errorf("unsupported path format: %s", path) +} + +// checkExternalPath 检查外部路径权限 +// checkExternalPath checks external path permission +// 注意：当策略为 ask 时，如果 Execute 被调用，说明审批已通过 +// Note: when policy is ask, if Execute is called, approval has been granted +func (t *ReadTool) checkExternalPath(absPath string) (string, error) { + // 检查路径是否在 workspace 内 / Check if path is inside workspace + rel, err := filepath.Rel(t.ws.Root(), absPath) + if err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { + // 路径在 workspace 内，直接允许 / Path inside workspace, allow directly + return absPath, nil + } + + // 路径在 workspace 外，检查外部路径权限 / Path outside workspace, check permission + decision := t.policy.ExternalDirDecision() + switch decision { + case permission.DecisionAllow: + // 明确允许 / Explicitly allowed + return absPath, nil + case permission.DecisionDeny: + // 明确拒绝 / Explicitly denied + return "", fmt.Errorf("external path access denied by policy") + default: + // ask 策略：如果 Execute 被调用，说明审批已通过 + // ask policy: if Execute is called, approval has been granted + return absPath, nil + } +} diff --git a/internal/tools/read_test.go b/internal/tools/read_test.go index d08119a..d2bb4a8 100644 --- a/internal/tools/read_test.go +++ b/internal/tools/read_test.go @@ -9,6 +9,8 @@ import ( "strings" "testing" + "coder/internal/config" + "coder/internal/permission" "coder/internal/security" ) @@ -28,7 +30,9 @@ func TestReadToolSmallFileDefaultLimit(t *testing.T) { if err != nil { t.Fatal(err) } - tool := NewReadTool(ws) + cfg, _ := permission.PresetConfig("build") + policy := permission.New(cfg) + tool := NewReadTool(ws, policy) // 仅传 path，不带 offset/limit，期望读出全部 10 行 args, _ := json.Marshal(map[string]any{ @@ -77,7 +81,9 @@ func TestReadToolLargeFilePagination(t *testing.T) { if err != nil { t.Fatal(err) } - tool := NewReadTool(ws) + cfg, _ := permission.PresetConfig("build") + policy := permission.New(cfg) + tool := NewReadTool(ws, policy) // 默认只读前 50 行 args, _ := json.Marshal(map[string]any{ @@ -126,7 +132,9 @@ func TestReadToolOffsetAndLimit(t *testing.T) { if err != nil { t.Fatal(err) } - tool := NewReadTool(ws) + cfg, _ := permission.PresetConfig("build") + policy := permission.New(cfg) + tool := NewReadTool(ws, policy) // 从第 51 行开始读 50 行 args, _ := json.Marshal(map[string]any{ @@ -172,7 +180,9 @@ func TestReadToolOffsetBeyondEOFAndInvalidLimit(t *testing.T) { if err != nil { t.Fatal(err) } - tool := NewReadTool(ws) + cfg, _ := permission.PresetConfig("build") + policy := permission.New(cfg) + tool := NewReadTool(ws, policy) // offset 大于文件总行数，期望 content 为空、has_more=false argsBeyond, _ := json.Marshal(map[string]any{ @@ -204,3 +214,125 @@ func TestReadToolOffsetBeyondEOFAndInvalidLimit(t *testing.T) { t.Fatalf("execute read (invalid limit): %v", err) } } + +// TestReadToolExternalPathApproval 测试外部路径审批流程 +func TestReadToolExternalPathApproval(t *testing.T) { + // 创建一个临时目录作为 workspace + wsRoot := t.TempDir() + ws, err := security.NewWorkspace(wsRoot) + if err != nil { + t.Fatal(err) + } + + // 在工作区外创建一个文件 + externalDir := t.TempDir() + externalFile := filepath.Join(externalDir, "external.txt") + externalContent := "external file content" + if err := os.WriteFile(externalFile, []byte(externalContent), 0o644); err != nil { + t.Fatal(err) + } + + // 测试策略为 ask 时，ApprovalRequest 应该返回需要审批 + t.Run("ask_policy_returns_approval_request", func(t *testing.T) { + cfg := config.PermissionConfig{ExternalDir: "ask"} + policy := permission.New(cfg) + tool := NewReadTool(ws, policy) + + args, _ := json.Marshal(map[string]any{ + "path": externalFile, + }) + + req, err := tool.ApprovalRequest(args) + if err != nil { + t.Fatalf("ApprovalRequest error: %v", err) + } + if req == nil { + t.Fatal("expected ApprovalRequest for external path, got nil") + } + if req.Tool != "read" { + t.Fatalf("expected tool name 'read', got %q", req.Tool) + } + }) + + // 测试策略为 allow 时，ApprovalRequest 应该返回 nil + t.Run("allow_policy_no_approval_request", func(t *testing.T) { + cfg := config.PermissionConfig{ExternalDir: "allow"} + policy := permission.New(cfg) + tool := NewReadTool(ws, policy) + + args, _ := json.Marshal(map[string]any{ + "path": externalFile, + }) + + req, err := tool.ApprovalRequest(args) + if err != nil { + t.Fatalf("ApprovalRequest error: %v", err) + } + if req != nil { + t.Fatalf("expected no ApprovalRequest for allow policy, got %v", req) + } + }) + + // 测试策略为 deny 时，ApprovalRequest 应该返回 nil（直接拒绝） + t.Run("deny_policy_no_approval_request", func(t *testing.T) { + cfg := config.PermissionConfig{ExternalDir: "deny"} + policy := permission.New(cfg) + tool := NewReadTool(ws, policy) + + args, _ := json.Marshal(map[string]any{ + "path": externalFile, + }) + + req, err := tool.ApprovalRequest(args) + if err != nil { + t.Fatalf("ApprovalRequest error: %v", err) + } + if req != nil { + t.Fatalf("expected no ApprovalRequest for deny policy, got %v", req) + } + }) + + // 测试 ~ 路径展开和审批 + t.Run("home_path_expansion_and_approval", func(t *testing.T) { + cfg := config.PermissionConfig{ExternalDir: "ask"} + policy := permission.New(cfg) + tool := NewReadTool(ws, policy) + + args, _ := json.Marshal(map[string]any{ + "path": "~/test_file.txt", + }) + + req, err := tool.ApprovalRequest(args) + if err != nil { + t.Fatalf("ApprovalRequest error: %v", err) + } + if req == nil { + t.Fatal("expected ApprovalRequest for ~ path, got nil") + } + }) + + // 测试工作区内路径不需要审批 + t.Run("workspace_path_no_approval", func(t *testing.T) { + cfg := config.PermissionConfig{ExternalDir: "ask"} + policy := permission.New(cfg) + tool := NewReadTool(ws, policy) + + // 在工作区内创建文件 + internalFile := filepath.Join(wsRoot, "internal.txt") + if err := os.WriteFile(internalFile, []byte("internal content"), 0o644); err != nil { + t.Fatal(err) + } + + args, _ := json.Marshal(map[string]any{ + "path": "internal.txt", + }) + + req, err := tool.ApprovalRequest(args) + if err != nil { + t.Fatalf("ApprovalRequest error: %v", err) + } + if req != nil { + t.Fatalf("expected no ApprovalRequest for workspace path, got %v", req) + } + }) +} diff --git a/opencode b/opencode new file mode 160000 index 0000000..6b02165 --- /dev/null +++ b/opencode @@ -0,0 +1 @@ +Subproject commit 6b021658ad514255c7398983b088c1636caaa5e4