diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 1c4f647a0..b105b5396 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -1201,7 +1201,7 @@ func (t *ExecTool) guardCommand(command, cwd string) string { } } - p, err := filepath.Abs(raw) + p, err := commandPathAbs(commandPathTextFromMatch(cmd, loc[0], loc[1]), cwdPath) if err != nil { continue } @@ -1243,6 +1243,66 @@ func (t *ExecTool) guardCommand(command, cwd string) string { return "" } +func commandPathAbs(pathText, cwdPath string) (string, error) { + if filepath.IsAbs(pathText) { + return filepath.Abs(pathText) + } + return filepath.Abs(filepath.Join(cwdPath, pathText)) +} + +func commandPathTextFromMatch(cmd string, start, end int) string { + raw := cmd[start:end] + if !strings.HasPrefix(raw, "/") || isUnixAbsolutePathMatchStart(cmd, start) { + return raw + } + + tokenStart, tokenEnd := shellTokenBounds(cmd, start) + prefix := cmd[tokenStart:start] + // For --flag=rel/path, validate the value. For ambiguous attached option + // forms like -isystem/path, keep the slash-starting path conservative. + if eq := strings.IndexByte(prefix, '='); eq >= 0 { + return cmd[tokenStart+eq+1 : tokenEnd] + } + if strings.HasPrefix(prefix, "-") { + return raw + } + return cmd[tokenStart:tokenEnd] +} + +func shellTokenBounds(cmd string, idx int) (int, int) { + start := idx + for start > 0 && !isShellTokenBoundary(cmd[start-1]) { + start-- + } + end := idx + for end < len(cmd) && !isShellTokenBoundary(cmd[end]) { + end++ + } + return start, end +} + +// isUnixAbsolutePathMatchStart returns true when a regex match beginning with +// "/" is actually an absolute path token, not the separator inside a relative +// path such as "skills/foo.py". +func isUnixAbsolutePathMatchStart(cmd string, idx int) bool { + if idx <= 0 { + return true + } + + prev := cmd[idx-1] + if isShellTokenBoundary(prev) || prev == '=' || prev == ',' || prev == '(' || prev == '[' || prev == '{' { + return true + } + + j := idx - 1 + for j >= 0 && !isShellTokenBoundary(cmd[j]) { + j-- + } + prefix := cmd[j+1 : idx] + + return strings.HasPrefix(prefix, "-") && !strings.Contains(prefix, "=") +} + // isShellTokenBoundary returns true when b is a byte that separates // tokens in a shell command (space, tab, colon, semicolon, pipe, etc.). func isShellTokenBoundary(b byte) bool { diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index b1460b9e3..cdc78b7fc 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -426,6 +426,90 @@ func TestShellTool_RestrictToWorkspace(t *testing.T) { } } +// TestShellTool_RelativePathWithSlashAllowed verifies that local relative paths +// under the workspace are not mistaken for absolute paths by the safety guard. +func TestShellTool_RelativePathWithSlashAllowed(t *testing.T) { + tmpDir := t.TempDir() + scriptDir := filepath.Join(tmpDir, "skills", "calendar-query", "scripts") + if err := os.MkdirAll(scriptDir, 0o755); err != nil { + t.Fatalf("failed to create script dir: %v", err) + } + scriptPath := filepath.Join(scriptDir, "query_calendar.py") + if err := os.WriteFile(scriptPath, []byte("calendar ok\n"), 0o644); err != nil { + t.Fatalf("failed to create script: %v", err) + } + + tool, err := NewExecTool(tmpDir, true) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + result := tool.Execute(context.Background(), map[string]any{ + "action": "run", + "command": "cat skills/calendar-query/scripts/query_calendar.py", + }) + + if result.IsError { + t.Fatalf("relative workspace script path should be allowed, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "calendar ok") { + t.Fatalf("expected script output, got: %s", result.ForLLM) + } +} + +func TestShellTool_AttachedAbsolutePathsStillBlocked(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{ + "cat --file=/etc/passwd", + "cc -I/etc main.c", + "echo -isystem/usr/include", + } + + for _, cmd := range commands { + result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd}) + if !result.IsError || !strings.Contains(result.ForLLM, "path outside working dir") { + t.Errorf("attached absolute path should be blocked: %q\n got: %s", cmd, result.ForLLM) + } + } +} + +func TestShellTool_OptionValueRelativeSymlinkEscapeBlocked(t *testing.T) { + root := t.TempDir() + workspace := filepath.Join(root, "workspace") + outsideDir := filepath.Join(root, "outside") + 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(filepath.Join(outsideDir, "passwd"), []byte("secret\n"), 0o644); err != nil { + t.Fatalf("failed to create outside file: %v", err) + } + if err := os.Symlink(outsideDir, filepath.Join(workspace, "link")); err != nil { + t.Skipf("symlinks not supported in this environment: %v", err) + } + + tool, err := NewExecTool(workspace, true) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + result := tool.Execute(context.Background(), map[string]any{ + "action": "run", + "command": "echo --config=link/passwd", + }) + + if !result.IsError || !strings.Contains(result.ForLLM, "path outside working dir") { + t.Fatalf("option value symlink escape should be blocked, got: %s", result.ForLLM) + } +} + // TestShellTool_DevNullAllowed verifies that /dev/null redirections are not blocked (issue #964). func TestShellTool_DevNullAllowed(t *testing.T) { tmpDir := t.TempDir()