fix(tools): Block MSYS coreutils under Windows sandbox

internal/agent/command_prefix.go

@@ -2,6 +2,7 @@
 
 import (
 	"fmt"
+	"runtime"
 	"strings"
 
 	"github.com/Gitlawb/zero/internal/sandbox"
@@ -27,17 +28,41 @@
 		}
 		return nil
 	}
-	for _, tokens := range segments {
-		if !knownSafeCommandSegment(tokens) && sandbox.ValidCommandPrefix(tokens) {
-			return append([]string(nil), tokens...)
+	// Only propose approving a prefix of one segment when every other segment
+	// in the command is independently known-safe. Once a prefix is approved,
+	// shellExecutionArgsForApproval escalates the whole command (every
+	// segment, not just the approved one) to bypass the sandbox, so offering a
+	// prefix that leaves an MSYS-prone (or otherwise unsafe) segment uncovered
+	// would let that segment run unsandboxed without ever being reviewed.
+	for index, tokens := range segments {
+		if knownSafeCommandSegment(tokens) {
+			continue
+		}
+		if !sandbox.ValidCommandPrefix(tokens) || !otherSegmentsKnownSafe(segments, index) {
+			return nil
 		}
+		return append([]string(nil), tokens...)
 	}
 	if len(segments) == 0 || !sandbox.ValidCommandPrefix(segments[0]) {
 		return nil
 	}
 	return append([]string(nil), segments[0]...)
 }
 
+// otherSegmentsKnownSafe reports whether every segment other than the one at
+// skip is known-safe on its own.
+func otherSegmentsKnownSafe(segments [][]string, skip int) bool {
+	for index, tokens := range segments {
+		if index == skip {
+			continue
+		}
+		if !knownSafeCommandSegment(tokens) {
+			return false
+		}
+	}
+	return true
+}
+
 func matchCommandPrefix(toolName string, args map[string]any, options Options) (sandbox.CommandPrefixGrant, bool, bool) {
 	if !isShellCommandTool(toolName) || options.Sandbox == nil {
 		return sandbox.CommandPrefixGrant{}, false, false
@@ -168,7 +193,7 @@
 	return cleaned
 }
 
 func safeRequestedPrefix(prefix []string, command []string) bool {
 	return safeRequestedPrefixForSegments(prefix, [][]string{command})
 }
 
@@ -196,7 +221,7 @@
 	return matched
 }
 
 func safeShellCommandTokens(command string) ([]string, bool) {
 	segments, ok := safeShellCommandSegments(command)
 	if !ok || len(segments) != 1 {
 		return nil, false
@@ -275,7 +300,11 @@
 	if len(command) == 0 {
 		return false
 	}
-	switch commandName(command[0]) {
+	name := commandName(command[0])
+	if runtime.GOOS == "windows" && tools.MsysProneCommandName(name) {
+		return false
+	}
+	switch name {
 	case "cat", "cd", "cut", "echo", "expr", "false", "grep", "head", "id",
 		"ls", "nl", "paste", "pwd", "rev", "seq", "stat", "tail", "tr",
 		"true", "uname", "uniq", "wc", "which", "whoami":
@@ -485,7 +514,7 @@
 	return true
 }
 
 func equalStringSlices(left []string, right []string) bool {
 	if len(left) != len(right) {
 		return false
 	}

internal/agent/command_prefix_test.go

@@ -1,9 +1,11 @@
 package agent
 
 import (
+	"runtime"
 	"testing"
 
 	"github.com/Gitlawb/zero/internal/sandbox"
+	"github.com/Gitlawb/zero/internal/tools"
 )
 
 func TestProposedCommandPrefixUsesSafeSimpleCommands(t *testing.T) {
@@ -35,12 +37,37 @@ func TestProposedCommandPrefixHonorsValidatedRequestedPrefix(t *testing.T) {
 
 func TestProposedCommandPrefixSupportsSegmentedCommands(t *testing.T) {
 	got := proposedCommandPrefix("bash", map[string]any{"command": "ps aux | head -5"})
+	if runtime.GOOS == "windows" {
+		// head is MSYS-prone on Windows (#458), so proposedCommandPrefix must
+		// not offer "ps aux" as a reusable prefix here: approving it would
+		// escalate the whole command, including the uncovered head segment,
+		// to bypass the sandbox unreviewed. See
+		// TestProposedCommandPrefixRejectsPrefixLeavingUnsafeTailUncovered for
+		// the platform-independent regression coverage of this behavior.
+		if got != nil {
+			t.Fatalf("expected no prefix on Windows because head is MSYS-prone, got %#v", got)
+		}
+		return
+	}
 	want := []string{"ps", "aux"}
 	if !equalStringSlices(got, want) {
 		t.Fatalf("prefix = %#v, want %#v", got, want)
 	}
 }
 
+// TestProposedCommandPrefixRejectsPrefixLeavingUnsafeTailUncovered guards
+// against proposedCommandPrefix offering to approve one segment of a
+// multi-segment command (e.g. "ps aux") while a different segment (e.g. "npm
+// install") is not known-safe. shellExecutionArgsForApproval escalates the
+// entire command once any prefix is approved, so an uncovered unsafe segment
+// would bypass the sandbox unreviewed. Uses npm, which is never known-safe on
+// any platform, so the assertion does not depend on runtime.GOOS.
+func TestProposedCommandPrefixRejectsPrefixLeavingUnsafeTailUncovered(t *testing.T) {
+	if got := proposedCommandPrefix("bash", map[string]any{"command": "ps aux && npm install"}); got != nil {
+		t.Fatalf("expected no prefix because npm segment is not known-safe, got %#v", got)
+	}
+}
+
 func TestProposedCommandPrefixHonorsRequestedPrefixAcrossSegments(t *testing.T) {
 	got := proposedCommandPrefix("bash", map[string]any{
 		"command":     "git status --short && git status --branch",
@@ -120,13 +147,35 @@ func TestProposedCommandPrefixRejectsUnsafeLaunchers(t *testing.T) {
 func TestMatchCommandPrefixCoversSegmentedCommandWithSafeTail(t *testing.T) {
 	engine := sandbox.NewEngine(sandbox.EngineOptions{WorkspaceRoot: t.TempDir()})
 	engine.GrantCommandPrefixForSession("bash", []string{"ps", "aux"})
+	// head is MSYS-prone on Windows (#458) and must not count as a known-safe tail.
+	command := "ps aux | head -5"
+	if runtime.GOOS == "windows" {
+		command = "ps aux | echo ok"
+	}
 
-	grant, ok, session := matchCommandPrefix("bash", map[string]any{"command": "ps aux | head -5"}, Options{Sandbox: engine})
+	grant, ok, session := matchCommandPrefix("bash", map[string]any{"command": command}, Options{Sandbox: engine})
 	if !ok || !session || !equalStringSlices(grant.Prefix, []string{"ps", "aux"}) {
 		t.Fatalf("match = %#v ok=%v session=%v, want session ps aux prefix", grant, ok, session)
 	}
 }
 
+func TestKnownSafeCommandSegmentRejectsMsysProneOnWindows(t *testing.T) {
+	if runtime.GOOS != "windows" {
+		t.Skip("windows-only known-safe MSYS guard")
+	}
+	for _, command := range [][]string{{"head", "-5"}, {"cat", "file.txt"}, {"grep", "pat"}} {
+		if knownSafeCommandSegment(command) {
+			t.Fatalf("expected %q to be unsafe on Windows, got known-safe", command)
+		}
+	}
+	if !knownSafeCommandSegment([]string{"echo", "ok"}) {
+		t.Fatal("expected echo to remain known-safe on Windows")
+	}
+	if !tools.MsysProneCommandName("head") {
+		t.Fatal("expected head to be MSYS-prone")
+	}
+}
+
 func TestMatchCommandPrefixRejectsUncoveredSegment(t *testing.T) {
 	engine := sandbox.NewEngine(sandbox.EngineOptions{WorkspaceRoot: t.TempDir()})
 	engine.GrantCommandPrefixForSession("bash", []string{"ps", "aux"})

internal/agent/loop_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -1945,6 +1946,11 @@ func TestRunCommandPrefixApprovalBypassesSandboxForMatchingShellCalls(t *testing
 
 func TestRunCommandPrefixApprovalCoversSegmentedShellWithSafeTail(t *testing.T) {
 	root := t.TempDir()
+	segmentedCommand := `ps aux | head -5`
+	if runtime.GOOS == "windows" {
+		// head is MSYS-prone on Windows (#458) and no longer counts as a known-safe tail.
+		segmentedCommand = `ps aux | echo ok`
+	}
 	retryTool := &sandboxDeniedRetryTool{}
 	registry := tools.NewRegistry()
 	registry.Register(retryTool)
@@ -1958,7 +1964,7 @@ func TestRunCommandPrefixApprovalCoversSegmentedShellWithSafeTail(t *testing.T)
 			},
 			{
 				{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-2", ToolName: "bash"},
-				{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-2", ArgumentsFragment: `{"command":"ps aux | head -5"}`},
+				{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-2", ArgumentsFragment: fmt.Sprintf(`{"command":%q}`, segmentedCommand)},
 				{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-2"},
 				{Type: zeroruntime.StreamEventDone},
 			},
@@ -2907,8 +2913,10 @@ func TestBuildSystemPromptInjectsHostShellContext(t *testing.T) {
 		t.Fatalf("expected operating system in environment block, got %q", prompt)
 	}
 	if runtime.GOOS == "windows" {
-		if !strings.Contains(prompt, "Windows cmd.exe syntax") || !strings.Contains(prompt, "cwd argument") {
-			t.Fatalf("expected Windows cmd.exe shell guidance in prompt, got %q", prompt)
+		for _, want := range []string{"Windows cmd.exe syntax", "cwd argument", "MSYS binaries", "grep", "require_escalated"} {
+			if !strings.Contains(prompt, want) {
+				t.Fatalf("expected Windows shell guidance to mention %q, got %q", want, prompt)
+			}
 		}
 	} else if !strings.Contains(prompt, "/bin/sh syntax") {
 		t.Fatalf("expected POSIX shell guidance in prompt, got %q", prompt)

internal/agent/system_prompt.go

@@ -286,7 +286,7 @@ func workspaceContext(cwd string) string {
 	b.WriteString("Working directory: " + cwd + "\n")
 	b.WriteString("Operating system: " + runtime.GOOS + "\n")
 	if runtime.GOOS == "windows" {
-		b.WriteString("Shell syntax: Windows cmd.exe syntax for exec_command/bash tools; prefer the workdir/cwd argument instead of cd when changing directories.\n")
+		b.WriteString("Shell syntax: Windows cmd.exe syntax for exec_command/bash tools; prefer the workdir/cwd argument instead of cd when changing directories. Do not pipe to or invoke POSIX coreutils from Git for Windows (usr\\bin head/grep/tail/cat/...): they are MSYS binaries and fail under the write-restricted sandbox; use native Zero tools (grep, read_file, list_directory, glob) or cmd.exe findstr/more instead, or sandbox_permissions require_escalated only when host-level execution is truly required.\n")
 	} else {
 		b.WriteString("Shell syntax: /bin/sh syntax for exec_command/bash tools; prefer the workdir/cwd argument instead of cd when changing directories.\n")
 	}

internal/tools/bash.go

@@ -83,7 +83,12 @@ func (tool bashTool) run(ctx context.Context, args map[string]any, engine *zeroS
 	if err != nil {
 		return errorResult("Error: Invalid arguments for bash: " + err.Error())
 	}
-	if issue := detectShellCommandIssue(commandText, runtime.GOOS); issue != nil {
+	// Resolve the command engine before the MSYS preflight check so an
+	// approved require_escalated call (commandEngine == nil, truly
+	// unsandboxed) can actually bypass the MSYS guard instead of being
+	// hard-blocked by the same check it was meant to escalate past.
+	commandEngine := commandEngineForSandboxPermissions(engine, sandboxPermissions)
+	if issue := detectShellCommandIssue(commandText, runtime.GOOS); issue != nil && !msysGuardBypassed(issue, commandEngine) {
 		return shellIssueBlockResult(*issue)
 	}
 
@@ -106,7 +111,6 @@ func (tool bashTool) run(ctx context.Context, args map[string]any, engine *zeroS
 		"cwd":        relativeCwd,
 		"timeout_ms": strconv.Itoa(timeoutMS),
 	}
-	commandEngine := commandEngineForSandboxPermissions(engine, sandboxPermissions)
 	if commandEngine == nil && sandboxPermissions == SandboxPermissionsRequireEscalated {
 		meta["sandbox_permissions"] = string(SandboxPermissionsRequireEscalated)
 	}
@@ -167,7 +171,7 @@ func (tool bashTool) run(ctx context.Context, args map[string]any, engine *zeroS
 		outText, errText, truncated := budgetBashCapture(stdoutText, stdout.total, stderrText, stderrTotal, meta)
 		return Result{
 			Status:    StatusError,
-			Output:    formatBashOutputWithShellHint(commandText, outText, errText, exitCode, meta),
+			Output:    formatBashOutputWithShellHint(outText, errText, exitCode, meta),
 			Truncated: truncated,
 			Meta:      meta,
 		}
@@ -178,7 +182,7 @@ func (tool bashTool) run(ctx context.Context, args map[string]any, engine *zeroS
 	if meta[SandboxLikelyDeniedMeta] == "true" {
 		return Result{
 			Status:    StatusError,
-			Output:    formatBashOutputWithShellHint(commandText, outText, errText, exitCode, meta),
+			Output:    formatBashOutputWithShellHint(outText, errText, exitCode, meta),
 			Truncated: truncated,
 			Meta:      meta,
 		}
@@ -198,6 +202,18 @@ func commandEngineForSandboxPermissions(engine *zeroSandbox.Engine, sandboxPermi
 	return engine
 }
 
+// msysGuardBypassed reports whether a windows_msys_sandbox preflight issue no
+// longer applies because sandbox_permissions: require_escalated actually
+// resolved to unsandboxed, host-level execution (commandEngine is nil only
+// then, per commandEngineForSandboxPermissions). MSYS coreutils fail because
+// of the sandbox's restricted token/handles, so running outside it removes
+// the failure mode this guard exists for. Any other issue kind (e.g. a real
+// cmd.exe syntax problem) still blocks regardless of escalation, since
+// running outside the sandbox does not fix a syntax error.
+func msysGuardBypassed(issue *shellIssue, commandEngine *zeroSandbox.Engine) bool {
+	return issue.Kind == windowsMsysSandboxKind && commandEngine == nil
+}
+
 // appendSandboxBlocks appends a <sandbox_blocks> block listing the denials
 // the sandbox log monitor captured, so the model can see what was blocked. With no
 // blocks the stderr is returned unchanged.
@@ -520,14 +536,6 @@ func (b *boundedBuffer) retained() string {
 	return string(b.head) + string(b.tail)
 }
 
-// truncateHeadTail keeps the first and last halves of value when it exceeds
-// maxBytes, dropping the middle behind a marker. Returns the possibly-truncated
-// text, the raw byte length, and whether truncation happened.
-func truncateHeadTail(value string, maxBytes int) (string, int, bool) {
-	// value holds the whole stream, so its length is the true raw size.
-	return truncateHeadTailWithTotal(value, len(value), maxBytes)
-}
-
 // truncateHeadTailWithTotal head+tail-truncates value to maxBytes, using total —
 // the full original byte count — for the "N bytes omitted" marker and the raw
 // count. total may exceed len(value) when the middle was already discarded during
@@ -547,9 +555,9 @@ func truncateHeadTailWithTotal(value string, total, maxBytes int) (string, int,
 	return utf8Prefix(value, head) + marker + utf8Suffix(value, tail), total, true
 }
 
-func formatBashOutputWithShellHint(command string, stdout string, stderr string, exitCode int, meta map[string]string) string {
+func formatBashOutputWithShellHint(stdout string, stderr string, exitCode int, meta map[string]string) string {
 	output := formatBashOutput(stdout, stderr, exitCode)
-	if issue := detectShellOutputIssue(command, stdout+"\n"+stderr, runtime.GOOS); issue != nil {
+	if issue := detectShellOutputIssue(stdout+"\n"+stderr, runtime.GOOS); issue != nil {
 		meta["shell_issue"] = issue.Kind
 		output = appendShellIssueHint(output, *issue)
 	}