Merge pull request #3087 from jp39/fix/exec-relative-workspace-paths

fix(tools): allow workspace relative exec paths

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 {

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()