From 89bfcd54c83ddecb00d600fc5d0d0e081f9fad40 Mon Sep 17 00:00:00 2001 From: XYSK-lilong007 <267018309+XYSK-lilong007@users.noreply.github.com> Date: Thu, 12 Mar 2026 02:33:13 +0800 Subject: [PATCH] fix(exec): ignore URL segments in workspace guard --- pkg/tools/shell.go | 38 +++++++++++++++++++++++++++++++++-- pkg/tools/shell_test.go | 44 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 67e2ad257..576360184 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -12,6 +12,8 @@ import ( "runtime" "strings" "time" + "unicode" + "unicode/utf8" "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/constants" @@ -78,7 +80,8 @@ var ( regexp.MustCompile(`\bsource\s+.*\.sh\b`), } - // absolutePathPattern matches absolute file paths in commands (Unix and Windows). + // absolutePathPattern matches path-like substrings in commands (Unix and Windows). + // A separate boundary check is applied before treating a match as a filesystem path. absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`) // safePaths are kernel pseudo-devices that are always safe to reference in @@ -373,7 +376,7 @@ func (t *ExecTool) guardCommand(command, cwd string) string { return "" } - matches := absolutePathPattern.FindAllString(cmd, -1) + matches := findAbsolutePathMatches(cmd) for _, raw := range matches { p, err := filepath.Abs(raw) @@ -399,6 +402,37 @@ func (t *ExecTool) guardCommand(command, cwd string) string { return "" } +func findAbsolutePathMatches(command string) []string { + indexes := absolutePathPattern.FindAllStringIndex(command, -1) + if len(indexes) == 0 { + return nil + } + + matches := make([]string, 0, len(indexes)) + for _, idx := range indexes { + start := idx[0] + if !isPathBoundary(command, start) { + continue + } + matches = append(matches, command[start:idx[1]]) + } + + return matches +} + +func isPathBoundary(command string, start int) bool { + if start <= 0 { + return true + } + + r, _ := utf8.DecodeLastRuneInString(command[:start]) + if unicode.IsSpace(r) { + return true + } + + return strings.ContainsRune(`"'=<>|&;()[]{},`, r) +} + func (t *ExecTool) SetTimeout(timeout time.Duration) { t.timeout = timeout } diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index 90265e5bd..3463a2c4f 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -489,6 +489,50 @@ func TestShellTool_SafePathsInWorkspaceRestriction(t *testing.T) { } } +func TestShellTool_GuardCommand_IgnoresURLPathSegments(t *testing.T) { + tmpDir := t.TempDir() + tool, err := NewExecTool(tmpDir, true) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + commands := []string{ + `curl -s "wttr.in/Beijing?T"`, + `curl -s https://example.com/api/v1/weather`, + } + + for _, cmd := range commands { + if got := tool.guardCommand(cmd, tmpDir); got != "" { + t.Fatalf("guardCommand(%q) = %q, want empty", cmd, got) + } + } +} + +func TestShellTool_GuardCommand_BlocksAbsolutePathOutsideWorkspace(t *testing.T) { + root := t.TempDir() + workspace := filepath.Join(root, "workspace") + outsideDir := filepath.Join(root, "outside") + outsideFile := filepath.Join(outsideDir, "secret.txt") + if err := os.MkdirAll(workspace, 0o755); err != nil { + t.Fatalf("failed to create workspace: %v", err) + } + if err := os.MkdirAll(outsideDir, 0o755); err != nil { + t.Fatalf("failed to create outside dir: %v", err) + } + if err := os.WriteFile(outsideFile, []byte("secret"), 0o644); err != nil { + t.Fatalf("failed to create outside file: %v", err) + } + + tool, err := NewExecTool(workspace, true) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + if got := tool.guardCommand(`cat "`+outsideFile+`"`, workspace); !strings.Contains(got, "path outside working dir") { + t.Fatalf("guardCommand should block outside path, got %q", got) + } +} + // TestShellTool_CustomAllowPatterns verifies that custom allow patterns exempt // commands from deny pattern checks. func TestShellTool_CustomAllowPatterns(t *testing.T) {