fix(tools): platform-specific pager suggestions, quote/caret-safe cd detection

internal/sandbox/safe_command.go

@@ -26,6 +26,10 @@ type interactiveProgram struct {
 	suggestion string
 	// windowsOnly limits the match to GOOS == "windows" (e.g. notepad).
 	windowsOnly bool
+	// windowsSuggestion overrides suggestion on GOOS == "windows", for cases
+	// where the default suggestion names POSIX-only tools (cat/head/tail)
+	// that cmd.exe does not have.
+	windowsSuggestion string
 }
 
 // interactivePrograms maps a bare command name to its non-interactive guidance.
@@ -39,9 +43,21 @@ var interactivePrograms = map[string]interactiveProgram{
 	"emacs": {reason: "emacs opens an interactive session", suggestion: "Use `emacs --batch` for scripting, or the edit_file/write_file tools."},
 	"pico":  {reason: "pico is a full-screen editor that waits for keystrokes", suggestion: "Use the edit_file/write_file tools or `sed -i`."},
 	// Pagers.
-	"less": {reason: "less is a pager that waits for navigation keys", suggestion: "Use `cat`, `head`, or `tail -n N` to print file contents non-interactively."},
-	"more": {reason: "more is a pager that waits for navigation keys", suggestion: "Use `cat`, `head`, or `tail -n N` to print file contents non-interactively."},
-	"most": {reason: "most is a pager that waits for navigation keys", suggestion: "Use `cat`, `head`, or `tail -n N` to print file contents non-interactively."},
+	"less": {
+		reason:            "less is a pager that waits for navigation keys",
+		suggestion:        "Use `cat`, `head`, or `tail -n N` to print file contents non-interactively.",
+		windowsSuggestion: "Use `type` to print file contents non-interactively, or the read_file tool with offset/limit for a partial view.",
+	},
+	"more": {
+		reason:            "more is a pager that waits for navigation keys",
+		suggestion:        "Use `cat`, `head`, or `tail -n N` to print file contents non-interactively.",
+		windowsSuggestion: "Use `type` to print file contents non-interactively, or the read_file tool with offset/limit for a partial view.",
+	},
+	"most": {
+		reason:            "most is a pager that waits for navigation keys",
+		suggestion:        "Use `cat`, `head`, or `tail -n N` to print file contents non-interactively.",
+		windowsSuggestion: "Use `type` to print file contents non-interactively, or the read_file tool with offset/limit for a partial view.",
+	},
 	// Process/system monitors.
 	"top":   {reason: "top runs a live full-screen dashboard until you quit it", suggestion: "Use `ps aux` (optionally `| head`) for a one-shot snapshot."},
 	"htop":  {reason: "htop runs a live full-screen dashboard until you quit it", suggestion: "Use `ps aux` (optionally `| head`) for a one-shot snapshot."},
@@ -186,11 +202,15 @@ func DetectInteractiveCommand(command string, goos string) InteractiveCommandRes
 		if hasNonInteractiveFlag(first, fields) {
 			continue
 		}
+		suggestion := program.suggestion
+		if goos == "windows" && program.windowsSuggestion != "" {
+			suggestion = program.windowsSuggestion
+		}
 		return InteractiveCommandResult{
 			Interactive: true,
 			Command:     first,
 			Reason:      program.reason,
-			Suggestion:  program.suggestion,
+			Suggestion:  suggestion,
 		}
 	}
 

internal/sandbox/safe_command_test.go

@@ -88,6 +88,28 @@ func TestDetectInteractiveCommandHonorsWindows(t *testing.T) {
 	}
 }
 
+// TestDetectInteractiveCommandPagerSuggestionIsPlatformSpecific covers a fix
+// for suggesting POSIX-only tools (cat/head/tail) as the escape hatch for a
+// blocked pager on Windows, where cmd.exe has none of them.
+func TestDetectInteractiveCommandPagerSuggestionIsPlatformSpecific(t *testing.T) {
+	for _, pager := range []string{"less", "more", "most"} {
+		linux := DetectInteractiveCommand(pager+" notes.txt", "linux")
+		if !linux.Interactive || !strings.Contains(linux.Suggestion, "cat") {
+			t.Fatalf("%s on linux: expected cat/head/tail suggestion, got %+v", pager, linux)
+		}
+		windows := DetectInteractiveCommand(pager+" notes.txt", "windows")
+		if !windows.Interactive {
+			t.Fatalf("%s on windows: expected interactive block, got %+v", pager, windows)
+		}
+		if strings.Contains(windows.Suggestion, "cat") || strings.Contains(windows.Suggestion, "tail") {
+			t.Fatalf("%s on windows: suggestion should not name POSIX-only tools, got %q", pager, windows.Suggestion)
+		}
+		if !strings.Contains(windows.Suggestion, "type") {
+			t.Fatalf("%s on windows: expected type suggestion, got %q", pager, windows.Suggestion)
+		}
+	}
+}
+
 func TestDetectInteractiveCommandFindsAcrossSeparators(t *testing.T) {
 	// Interactive commands hidden after a shell operator should still be caught.
 	for _, command := range []string{

internal/tools/bash_tool_test.go

@@ -152,7 +152,7 @@ func TestDetectShellCommandIssueAllowsWindowsCDSwitch(t *testing.T) {
 	}
 }
 
-func TestDetectShellCommandIssueRequiresActualLSCommand(t *testing.T) {
+func TestDetectShellCommandIssueFlagsWindowsBadCommands(t *testing.T) {
 	for _, command := range []string{
 		`echo false ls -la`,
 		`echo list -items`,
@@ -172,6 +172,22 @@ func TestDetectShellCommandIssueRequiresActualLSCommand(t *testing.T) {
 			t.Fatalf("expected actual ls command to be flagged for %q", command)
 		}
 	}
+
+	// Common Unix filters used for truncation (e.g. gh output) must be caught.
+	for _, command := range []string{
+		`gh pr view 465 | head -200`,
+		`some-cmd | tail -n 10`,
+		`cat file | head -5`,
+		`ps aux | head`,
+		// Issue #467's exact reported case: `| cat` alone (no other POSIX
+		// utility in the pipeline) must also be flagged before execution.
+		`gh pr view 465 --json reviews,comments -q '.reviews | length' 2>&1 | cat`,
+		`some-cmd | cat`,
+	} {
+		if issue := detectShellCommandIssue(command, "windows"); issue == nil {
+			t.Fatalf("expected POSIX filter command to be flagged for %q", command)
+		}
+	}
 }
 
 func TestDetectShellCommandIssueFlagsPipedPosixUtilities(t *testing.T) {
@@ -191,6 +207,55 @@ func TestDetectShellCommandIssueFlagsPipedPosixUtilities(t *testing.T) {
 	}
 }
 
+func TestDetectShellCommandIssueIgnoresQuotedPipeText(t *testing.T) {
+	for _, command := range []string{
+		`gh pr comment --body "please do not use | cat here"`,
+		`git commit -m "grep for TODO later"`,
+		`git commit -m "fix | cat pipe"`,
+	} {
+		if issue := detectShellCommandIssue(command, "windows"); issue != nil {
+			t.Fatalf("expected POSIX-utility text inside a quoted argument to pass for %q, got %#v", command, issue)
+		}
+	}
+
+	// The same utility names, actually piped (unquoted), must still be flagged.
+	for _, command := range []string{
+		`echo test | cat`,
+		`echo test | grep TODO`,
+	} {
+		if issue := detectShellCommandIssue(command, "windows"); issue == nil {
+			t.Fatalf("expected unquoted POSIX utility pipeline to still be flagged for %q", command)
+		}
+	}
+}
+
+func TestDetectShellCommandIssueIgnoresCaretEscapedMetachars(t *testing.T) {
+	for _, command := range []string{
+		`foo^|cat`,
+		`echo a^&cat`,
+		`echo a^;cat`,
+	} {
+		if issue := detectShellCommandIssue(command, "windows"); issue != nil {
+			t.Fatalf("expected caret-escaped metachar to read as a literal for %q, got %#v", command, issue)
+		}
+	}
+
+	// The same shape, unescaped, must still be flagged.
+	if issue := detectShellCommandIssue(`foo|cat`, "windows"); issue == nil {
+		t.Fatal("expected unescaped pipe into cat to still be flagged")
+	}
+}
+
+func TestDetectShellCommandIssueSuggestionOmitsBlockedMore(t *testing.T) {
+	issue := detectShellCommandIssue(`some_command | wc -l`, "windows")
+	if issue == nil {
+		t.Fatal("expected POSIX utility pipeline to be flagged")
+	}
+	if strings.Contains(issue.Suggestion, "more") {
+		t.Fatalf("suggestion should not recommend more, which is itself blocked as an interactive pager on Windows: %q", issue.Suggestion)
+	}
+}
+
 func TestDetectShellCommandIssueAllowsUnrelatedCommands(t *testing.T) {
 	for _, command := range []string{
 		`git log --oneline`,
@@ -668,8 +733,15 @@ func TestBashToolBlocksInteractiveCommandThroughSandbox(t *testing.T) {
 	if result.Status != StatusError {
 		t.Fatalf("expected error status, got %s: %s", result.Status, result.Output)
 	}
-	if !strings.Contains(result.Output, "interactive") || !strings.Contains(result.Output, "cat") {
-		t.Fatalf("expected pager guard message with cat suggestion, got %q", result.Output)
+	// The suggested non-interactive alternative is platform-specific: cat
+	// doesn't exist on Windows cmd.exe, so the guard suggests `type` there
+	// instead (see safe_command.go's windowsSuggestion).
+	wantSuggestion := "cat"
+	if runtime.GOOS == "windows" {
+		wantSuggestion = "type"
+	}
+	if !strings.Contains(result.Output, "interactive") || !strings.Contains(result.Output, wantSuggestion) {
+		t.Fatalf("expected pager guard message with %q suggestion, got %q", wantSuggestion, result.Output)
 	}
 }
 

internal/tools/shell_runtime.go

@@ -20,7 +20,7 @@ type shellIssue struct {
 
 const windowsMsysSandboxKind = "windows_msys_sandbox"
 
-const windowsMsysSandboxSuggestion = "MSYS/Cygwin coreutils from Git for Windows cannot run under Zero's write-restricted Windows sandbox. Prefer Zero native tools (grep, read_file with offset/limit, list_directory, glob), cmd.exe findstr/more, or PowerShell Select-Object -First/-Last. If host-level execution is truly required, rerun with sandbox_permissions: \"require_escalated\" and a narrow justification."
+const windowsMsysSandboxSuggestion = "MSYS/Cygwin coreutils from Git for Windows cannot run under Zero's write-restricted Windows sandbox. Prefer Zero native tools (grep, read_file with offset/limit, list_directory, glob), cmd.exe findstr, or PowerShell Select-Object -First/-Last. If host-level execution is truly required, rerun with sandbox_permissions: \"require_escalated\" and a narrow justification."
 
 // windowsMsysProneNames is the single source of truth for POSIX coreutil names
 // that commonly resolve to a Git-for-Windows MSYS/Cygwin binary rather than a
@@ -168,7 +168,15 @@ func detectShellCommandIssue(command string, goos string) *shellIssue {
 		return nil
 	}
 	trimmed := strings.TrimSpace(command)
-	if windowsBashStyleCDPattern.MatchString(trimmed) {
+	// Blank out double-quoted spans before matching, so a `cd /foo`-shaped
+	// string that only appears inside a quoted argument value (e.g. a `gh`
+	// or `git commit` message) is not mistaken for an actual command cmd.exe
+	// would interpret. Then neutralize cmd.exe caret escapes of its own
+	// metacharacters (foo^|cat is the literal token foo|cat, not a pipe into
+	// cat), so an escaped metachar can't stand in as a fake segment boundary
+	// either.
+	unquoted := stripCmdCaretEscapes(stripDoubleQuotedSpans(trimmed))
+	if windowsBashStyleCDPattern.MatchString(unquoted) {
 		return &shellIssue{
 			Kind:       "windows_shell_syntax",
 			Message:    "Command looks like POSIX/Bash syntax, but Zero runs bash tool commands through Windows cmd.exe on this host.",
@@ -194,6 +202,44 @@ func detectShellCommandIssue(command string, goos string) *shellIssue {
 	return nil
 }
 
+// stripDoubleQuotedSpans replaces the contents of every double-quoted span
+// (quotes included) with spaces, preserving the string's length and the
+// position of unquoted text. cmd.exe treats a double-quoted span as a single
+// literal token, so operators/utility names inside one are not real command
+// syntax; blanking them out keeps the Windows command-issue regexes anchored
+// to text cmd.exe would actually interpret.
+func stripDoubleQuotedSpans(command string) string {
+	var b strings.Builder
+	b.Grow(len(command))
+	inQuotes := false
+	for _, c := range command {
+		if c == '"' {
+			inQuotes = !inQuotes
+			b.WriteByte(' ')
+			continue
+		}
+		if inQuotes {
+			b.WriteByte(' ')
+			continue
+		}
+		b.WriteRune(c)
+	}
+	return b.String()
+}
+
+// cmdEscapedMetacharPattern matches a cmd.exe caret escape of one of its own
+// metacharacters. cmd.exe treats the escaped character as a literal, not an
+// operator, so e.g. `foo^|cat` is the single literal token foo|cat, not a
+// pipe into cat.
+var cmdEscapedMetacharPattern = regexp.MustCompile(`\^[&|;^<>]`)
+
+// stripCmdCaretEscapes blanks out cmd.exe caret-escape sequences (both
+// characters), so an escaped metacharacter cannot be mistaken by the Windows
+// command-issue regexes for a real operator/segment boundary.
+func stripCmdCaretEscapes(command string) string {
+	return cmdEscapedMetacharPattern.ReplaceAllString(command, "  ")
+}
+
 // detectShellOutputIssue looks for MSYS runtime crash markers and cmd.exe
 // syntax-error text in output only, never in the command that was run. The
 // command line is attacker/user-controlled argument text (e.g. a `gh pr
@@ -214,7 +260,7 @@ func detectShellOutputIssue(output string, goos string) *shellIssue {
 		return &shellIssue{
 			Kind:       "windows_shell_syntax",
 			Message:    "Windows cmd.exe rejected the command syntax.",
-			Suggestion: "Translate the command to Windows cmd.exe syntax, set the bash tool cwd argument instead of running cd, or prefer native Zero tools for file inspection.",
+			Suggestion: "Use Windows cmd.exe syntax. Quote args with | using double quotes (e.g. --jq \".a | b\"). Avoid | head; use --jq or PowerShell Select-Object -First N instead. Prefer native tools.",
 		}
 	}
 	return nil