diff --git a/internal/agent/loop_test.go b/internal/agent/loop_test.go index 86258f432..c2cd769d3 100644 --- a/internal/agent/loop_test.go +++ b/internal/agent/loop_test.go @@ -2404,7 +2404,11 @@ func TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant(t *testing.T) { root := t.TempDir() command := "PATH=.:$PATH curl https://example.com" if runtime.GOOS == "windows" { - command = "set PATH=.;%PATH% && curl https://example.com" + if windowsTestUsesPowerShell() { + command = `$env:PATH = '.;' + $env:PATH; curl.cmd https://example.com` + } else { + command = "set PATH=.;%PATH% && curl https://example.com" + } fakeCurl := filepath.Join(root, "curl.cmd") if err := os.WriteFile(fakeCurl, []byte("@echo fake curl %*\r\n"), 0o755); err != nil { t.Fatal(err) @@ -2525,13 +2529,17 @@ func TestRunDoesNotOfferPrefixApprovalForUnsafeBashCommand(t *testing.T) { func TestRunPromptsForDestructiveShellInsteadOfSandboxDeny(t *testing.T) { root := t.TempDir() + command := "echo rm -rf /" + if runtime.GOOS == "windows" && windowsTestUsesPowerShell() { + command = `Write-Output 'rm -rf /'` + } registry := tools.NewRegistry() registry.Register(tools.NewScopedBashTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { {Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "bash"}, - {Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"command":"echo rm -rf /"}`}, + {Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"command":` + quoteJSONString(command) + `}`}, {Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"}, {Type: zeroruntime.StreamEventDone}, }, @@ -2627,6 +2635,12 @@ func TestRunAlwaysAllowWithoutSandboxStillAllowsCall(t *testing.T) { } } +func windowsTestUsesPowerShell() bool { + executable, _ := tools.HostShellCommand("") + name := strings.ToLower(filepath.Base(executable)) + return strings.Contains(name, "powershell") || strings.HasPrefix(name, "pwsh") +} + func containsPermissionDecision(decisions []PermissionDecisionAction, want PermissionDecisionAction) bool { for _, decision := range decisions { if decision == want { @@ -3160,22 +3174,17 @@ func TestBuildSystemPromptInjectsHostShellContext(t *testing.T) { t.Fatalf("expected operating system in environment block, got %q", prompt) } if runtime.GOOS == "windows" { - for _, want := range []string{"Windows cmd.exe syntax", "cwd argument", "MSYS binaries", "grep", "require_escalated", "use double quotes around the value"} { - if !strings.Contains(prompt, want) { - t.Fatalf("expected Windows shell guidance to mention %q, got %q", want, prompt) - } + wants := []string{"workdir/cwd", "MSYS binaries"} + if windowsTestUsesPowerShell() { + wants = append(wants, "PowerShell", "Get-ChildItem", "Select-String", "$env:NAME", "require_escalated") + } else { + wants = append(wants, "cmd.exe", "double quotes") } - // The examples must themselves use the safe (double-quoted) form; a - // single-quoted example here would teach the model the exact syntax - // that fails under cmd.exe. - for _, want := range []string{`--jq ".a | b"`, `-run "A|B"`} { + for _, want := range wants { if !strings.Contains(prompt, want) { - t.Fatalf("expected double-quoted example %q in Windows shell guidance, got %q", want, prompt) + t.Fatalf("expected selected Windows shell guidance to mention %q, got %q", want, prompt) } } - if strings.Contains(prompt, `'.a | b'`) || strings.Contains(prompt, `'A|B'`) { - t.Fatalf("Windows shell guidance must not show the unsafe single-quoted form, got %q", prompt) - } } else if !strings.Contains(prompt, "/bin/sh syntax") { t.Fatalf("expected POSIX shell guidance in prompt, got %q", prompt) } diff --git a/internal/agent/system_prompt.go b/internal/agent/system_prompt.go index b84081996..d8f5f8341 100644 --- a/internal/agent/system_prompt.go +++ b/internal/agent/system_prompt.go @@ -11,6 +11,7 @@ import ( "github.com/Gitlawb/zero/internal/config" "github.com/Gitlawb/zero/internal/repomap" + "github.com/Gitlawb/zero/internal/tools" "github.com/Gitlawb/zero/internal/workspaceseed" ) @@ -310,11 +311,7 @@ func workspaceContext(cwd string) string { b.WriteString("\n") 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. To put | & > < etc inside an arg value, use double quotes around the value, not single quotes (single quotes do not protect metachars in cmd.exe): gh --jq \".a | b\", go test -run \"A|B\". 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. Prefer the workdir/cwd argument over cd when changing directories.\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") - } + b.WriteString(tools.HostShellEnvironmentGuidance() + "\n") if branch := gitBranchForPrompt(cwd); branch != "" { b.WriteString("Git branch: " + branch + "\n") } diff --git a/internal/sandbox/analyzer.go b/internal/sandbox/analyzer.go index c4481a53c..7973fa951 100644 --- a/internal/sandbox/analyzer.go +++ b/internal/sandbox/analyzer.go @@ -27,11 +27,22 @@ var destructivePrograms = map[string]bool{ "mkfs": true, "fdisk": true, "shred": true, "dd": true, "parted": true, } +var powerShellRemoveItemPrograms = map[string]bool{ + "remove-item": true, + "ri": true, + "rd": true, + "rmdir": true, + "del": true, + "erase": true, + "rm": true, +} + // networkPrograms are commands that perform network egress/ingress. var networkPrograms = map[string]bool{ "curl": true, "wget": true, "ssh": true, "scp": true, "sftp": true, "rsync": true, "nc": true, "ncat": true, "netcat": true, "telnet": true, - "ftp": true, + "ftp": true, "iwr": true, "irm": true, "invoke-webrequest": true, + "invoke-restmethod": true, } var localServerPrograms = map[string]bool{ @@ -172,7 +183,10 @@ func analyzeInto(script string, result *AnalysisResult, seen map[string]bool, de if commandUsesNetwork(prog, rest) { result.Network = true } - if destructivePrograms[prog] || (prog == "rm" && hasRecursiveForce(rest)) || (prog == "find" && hasFindDelete(rest)) { + if destructivePrograms[prog] || + (prog == "rm" && hasRecursiveForce(rest)) || + (powerShellRemoveItemPrograms[prog] && hasPowerShellRecursiveForce(rest)) || + (prog == "find" && hasFindDelete(rest)) { result.Destructive = true } return true @@ -479,6 +493,19 @@ func hasRecursiveForce(args []*syntax.Word) bool { return recursive && force } +func hasPowerShellRecursiveForce(args []*syntax.Word) bool { + recursive, force := false, false + for _, arg := range args { + switch strings.ToLower(wordText(arg)) { + case "-recurse", "-r": + recursive = true + case "-force": + force = true + } + } + return recursive && force +} + func hasFindDelete(args []*syntax.Word) bool { for _, arg := range args { if wordText(arg) == "-delete" { diff --git a/internal/sandbox/analyzer_test.go b/internal/sandbox/analyzer_test.go index 303de9999..9f44eca25 100644 --- a/internal/sandbox/analyzer_test.go +++ b/internal/sandbox/analyzer_test.go @@ -45,6 +45,27 @@ func TestAnalyzeCommand(t *testing.T) { {name: "sudo dynamic flag then rm -rf", script: `sudo "$maybe" rm -rf /tmp/x`, destructive: true}, {name: "curl", script: "curl https://example.com", network: true}, + {name: "PowerShell iwr alias", script: "iwr https://example.com", network: true}, + {name: "PowerShell irm alias", script: "irm https://example.com", network: true}, + {name: "PowerShell Invoke-WebRequest", script: "Invoke-WebRequest https://example.com", network: true}, + {name: "PowerShell Invoke-RestMethod", script: "Invoke-RestMethod https://example.com", network: true}, + {name: "PowerShell Remove-Item recursive force", script: `Remove-Item -Recurse -Force 'C:\temp\x'`, destructive: true}, + {name: "PowerShell rm recursive force", script: `rm -Recurse -Force 'C:\temp\x'`, destructive: true}, + {name: "PowerShell ri recursive force", script: `ri -Recurse -Force 'C:\temp\x'`, destructive: true}, + {name: "PowerShell rd recursive force", script: `rd -Recurse -Force 'C:\temp\x'`, destructive: true}, + {name: "PowerShell rmdir recursive force", script: `rmdir -Recurse -Force 'C:\temp\x'`, destructive: true}, + {name: "PowerShell del recursive force", script: `del -Recurse -Force 'C:\temp\x'`, destructive: true}, + {name: "PowerShell erase recursive force", script: `erase -Recurse -Force 'C:\temp\x'`, destructive: true}, + {name: "PowerShell ambiguous force abbreviation", script: `Remove-Item -Recurse -f 'C:\temp\x'`, destructive: false}, + {name: "PowerShell Remove-Item without recurse", script: `Remove-Item -Force 'C:\temp\x'`, destructive: false}, + {name: "Windows curl cmd", script: "curl.cmd https://example.com", network: true}, + {name: "Windows curl exe", script: "curl.exe https://example.com", network: true}, + {name: "Windows drive path curl exe", script: `'C:\tools\curl.exe' https://example.com`, network: true}, + {name: "Windows UNC path curl exe", script: `'\\server\share\curl.exe' https://example.com`, network: true}, + {name: "Windows dot relative path curl exe", script: `'.\curl.exe' https://example.com`, network: true}, + {name: "Windows relative path curl exe", script: `'tools\curl.exe' https://example.com`, network: true}, + {name: "Windows drive relative path curl exe", script: `'C:curl.exe' https://example.com`, network: true}, + {name: "Windows npm cmd", script: "npm.cmd install", network: true}, {name: "wget piped to shell", script: "wget -qO- https://x.test | sh", network: true}, {name: "python http server", script: "python3 -m http.server 8000", network: true}, {name: "python pip install", script: "python3 -m pip install requests", network: true}, diff --git a/internal/sandbox/engine_test.go b/internal/sandbox/engine_test.go index 42c8403ba..e91486e9d 100644 --- a/internal/sandbox/engine_test.go +++ b/internal/sandbox/engine_test.go @@ -74,6 +74,25 @@ func TestEngineBashAllowGrantDoesNotBypassNetworkPrompt(t *testing.T) { } } +func TestEngineClassifiesPowerShellCurlCommandAsNetwork(t *testing.T) { + engine := NewEngine(EngineOptions{ + WorkspaceRoot: t.TempDir(), + Policy: DefaultPolicy(), + }) + decision := engine.Evaluate(context.Background(), Request{ + ToolName: "bash", + SideEffect: SideEffectShell, + Permission: PermissionPrompt, + PermissionMode: PermissionModeAsk, + Args: map[string]any{ + "command": `$env:PATH = '.;' + $env:PATH; curl.cmd https://example.com`, + }, + }) + if decision.Action != ActionPrompt || decision.Reason != ReasonNetworkBlocked || !HasRiskCategory(decision.Risk, "network") { + t.Fatalf("PowerShell curl command decision = %#v, want network prompt", decision) + } +} + func TestUnsandboxedExecutionAllowedPreservesDeniedReads(t *testing.T) { root := t.TempDir() policy := DefaultPolicy() diff --git a/internal/sandbox/runner_windows_integration_test.go b/internal/sandbox/runner_windows_integration_test.go index 708f43e4c..d0f715744 100644 --- a/internal/sandbox/runner_windows_integration_test.go +++ b/internal/sandbox/runner_windows_integration_test.go @@ -266,6 +266,74 @@ func TestWindowsRestrictedTokenNestedPipeCapture(t *testing.T) { } } +// TestWindowsRestrictedTokenPowerShell exercises the default Windows shell +// shape end to end under the write-restricted token. It covers PowerShell +// initialization, a native pipeline (which creates IPC objects), UTF-8 output, +// and workspace write enforcement. +func TestWindowsRestrictedTokenPowerShell(t *testing.T) { + if os.Getenv("ZERO_SANDBOX_REAL_SMOKE") != "1" { + t.Skip("set ZERO_SANDBOX_REAL_SMOKE=1 to run real Windows sandbox smoke tests") + } + runnerExe := realSmokeExecutable(t, "ZERO_WINDOWS_COMMAND_RUNNER_EXE", WindowsSandboxCommandRunnerName) + powerShell := realSmokePowerShell(t) + + root := t.TempDir() + outside := t.TempDir() + sandboxHome := filepath.Join(root, ".zero-sandbox") + profile := PermissionProfile{ + FileSystem: FileSystemPolicy{ + Kind: FileSystemRestricted, + ReadRoots: []string{root}, + WriteRoots: []WritableRoot{{Root: root}}, + IncludePlatformRoots: true, + AllowTemp: true, + }, + Network: NetworkPolicy{Mode: NetworkDeny}, + } + config := WindowsSandboxCommandArgsOptions{ + SandboxHome: sandboxHome, + CommandCWD: root, + WorkspaceRoots: []string{root}, + PermissionProfile: profile, + SandboxLevel: WindowsSandboxLevelUnelevated, + } + + insideMarker := filepath.Join(root, "powershell-ok.txt") + script := "$ErrorActionPreference='Stop'; " + + "[Console]::OutputEncoding=[System.Text.Encoding]::UTF8; " + + "$values = 1,2,3 | ForEach-Object { $_ * 2 }; " + + "[IO.File]::WriteAllText(" + powershellSingleQuote(insideMarker) + ", (($values -join ',') + ' ✓'))" + runWindowsRealSmokeCommand(t, runnerExe, config, []string{ + powerShell, "-NoLogo", "-NoProfile", "-Command", script, + }, 0) + if bytes, err := os.ReadFile(insideMarker); err != nil || strings.TrimSpace(string(bytes)) != "2,4,6 ✓" { + t.Fatalf("PowerShell sandbox marker = %q, %v; want pipeline output", bytes, err) + } + + outsideMarker := filepath.Join(outside, "powershell-denied.txt") + deniedScript := "$ErrorActionPreference='Stop'; " + + "[IO.File]::WriteAllText(" + powershellSingleQuote(outsideMarker) + ", 'leaked')" + runWindowsRealSmokeCommand(t, runnerExe, config, []string{ + powerShell, "-NoLogo", "-NoProfile", "-Command", deniedScript, + }, 1) + if _, err := os.Stat(outsideMarker); err == nil { + t.Fatal("sandboxed PowerShell wrote outside every granted root") + } else if !os.IsNotExist(err) { + t.Fatalf("stat outside PowerShell marker: %v", err) + } +} + +func realSmokePowerShell(t *testing.T) string { + t.Helper() + for _, candidate := range []string{"pwsh.exe", "pwsh", "powershell.exe", "powershell"} { + if path, err := exec.LookPath(candidate); err == nil { + return path + } + } + t.Skip("PowerShell is unavailable") + return "" +} + func realSmokeExecutable(t *testing.T, envKey string, fallbackName string) string { t.Helper() if path := os.Getenv(envKey); path != "" { diff --git a/internal/sandbox/safe_command.go b/internal/sandbox/safe_command.go index 02f618824..807004710 100644 --- a/internal/sandbox/safe_command.go +++ b/internal/sandbox/safe_command.go @@ -430,25 +430,70 @@ func shellDashCPayload(program string, fields []string) string { // name: it strips shell quoting/escaping characters (", ', `, \) wherever they // appear in the token (including embedded ones like `vi\m` or `v"i"m`), strips // leading command-substitution markers, removes any directory prefix (so -// /usr/bin/vim and C:\tools\vim.exe match "vim"), and lowercases. This closes -// path/quote/substitution evasions of the detector. +// /usr/bin/vim and C:\tools\vim.exe match "vim"), removes Windows executable +// suffixes, and lowercases. This closes path/quote/substitution evasions of the +// detector and keeps curl.exe/npm.cmd equivalent to curl/npm for risk analysis. func normalizeProgramToken(field string) string { token := strings.TrimSpace(field) token = strings.TrimLeft(token, "$(") token = strings.TrimRight(token, ")") - // Strip shell quoting/escaping characters (", ', `, \) wherever they appear - // in the token — surrounding, embedded, or as a mid-word escape — so - // "vim", v"i"m, 'v'im, and vi\m all collapse to the program name. This is - // done BEFORE the directory-prefix trim so an escape can't masquerade as a - // path separator (e.g. vi\m must become vim, not m). - token = stripChars(token, "\"'`\\") - // Strip a directory prefix so /usr/bin/vim reduces to the basename. (A - // Windows-style backslash path separator is already removed above, so only - // the POSIX separator remains to split on.) - if i := strings.LastIndex(token, "/"); i >= 0 { - token = token[i+1:] - } - return strings.ToLower(token) + // PowerShell commonly invokes drive-letter, UNC, and relative paths. Extract + // their basename while backslashes still carry path-separator meaning; + // removing them first would turn C:\tools\curl.exe into c:toolscurl.exe and + // bypass the canonical curl risk entry. + pathToken := stripChars(token, "\"'`") + if windowsPathBasename, ok := windowsExecutablePathBasename(pathToken); ok { + token = windowsPathBasename + } else { + // Strip shell quoting/escaping characters (", ', `, \) wherever they + // appear in the token — surrounding, embedded, or as a mid-word escape + // — so "vim", v"i"m, 'v'im, and vi\m all collapse to the program name. + token = stripChars(token, "\"'`\\") + // Strip a directory prefix so /usr/bin/vim reduces to the basename. + if i := strings.LastIndex(token, "/"); i >= 0 { + token = token[i+1:] + } + } + token = strings.ToLower(token) + for _, suffix := range []string{".exe", ".cmd", ".bat", ".com"} { + if strings.HasSuffix(token, suffix) { + return strings.TrimSuffix(token, suffix) + } + } + return token +} + +func windowsExecutablePathBasename(token string) (string, bool) { + drivePath := len(token) >= 3 && + ((token[0] >= 'a' && token[0] <= 'z') || (token[0] >= 'A' && token[0] <= 'Z')) && + token[1] == ':' + uncPath := strings.HasPrefix(token, `\\`) + explicitRelativePath := strings.HasPrefix(token, `.\`) || strings.HasPrefix(token, `..\`) + backslashRelativePath := strings.ContainsRune(token, '\\') && hasWindowsExecutableSuffix(token) + if !drivePath && !uncPath && !explicitRelativePath && !backslashRelativePath { + return "", false + } + index := strings.LastIndexAny(token, `\/`) + if index >= 0 { + if index+1 >= len(token) { + return "", false + } + return token[index+1:], true + } + if drivePath && len(token) > 2 { + return token[2:], true + } + return "", false +} + +func hasWindowsExecutableSuffix(token string) bool { + token = strings.ToLower(token) + for _, suffix := range []string{".exe", ".cmd", ".bat", ".com"} { + if strings.HasSuffix(token, suffix) { + return true + } + } + return false } // stripChars returns s with every rune in cutset removed. diff --git a/internal/tools/bash.go b/internal/tools/bash.go index 2a40e2934..9e64b55e3 100644 --- a/internal/tools/bash.go +++ b/internal/tools/bash.go @@ -106,7 +106,7 @@ func (tool bashTool) run(ctx context.Context, args map[string]any, engine *zeroS // 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) { + if issue := detectShellCommandIssueForRuntime(commandText, detectShellRuntime(runtime.GOOS)); issue != nil && !msysGuardBypassed(issue, commandEngine) { return shellIssueBlockResult(*issue) } @@ -293,23 +293,19 @@ func shellIssueBlockResult(issue shellIssue) Result { } // buildBashCommand returns the exec.Cmd and the sandbox plan for running -// commandText. On Windows, when the command is not wrapped by the sandbox -// engine (plan.Wrapped == false), it also overrides the child's raw command -// line so commandText reaches cmd.exe unescaped; see -// zeroSandbox.WindowsShellCommandLine for why that matters. The wrapped case -// gets the same treatment inside the sandboxed runner process itself -// (internal/sandbox/windows_process_windows.go), since that command line is -// built there, not here. +// commandText. PowerShell is preferred on Windows, with cmd.exe retained as +// the fallback. Only cmd.exe needs the raw command-line override. func buildBashCommand(ctx context.Context, commandText string, absoluteCwd string, engine *zeroSandbox.Engine) (*exec.Cmd, zeroSandbox.CommandPlan, error) { + hostShell := detectShellRuntime(runtime.GOOS) spec := zeroSandbox.CommandSpec{ - Name: shellExecutable(), - Args: shellArguments(commandText), + Name: hostShell.Executable, + Args: hostShell.arguments(commandText), Dir: absoluteCwd, } if engine != nil { command, plan, err := engine.CommandContext(ctx, spec) if err == nil { - applyWindowsShellCommandLine(command, commandText, plan.Wrapped) + applyWindowsShellCommandLine(command, commandText, plan.Wrapped, hostShell.Kind == shellKindCmd) } return command, plan, err } @@ -333,7 +329,7 @@ func buildBashCommand(ctx context.Context, commandText string, absoluteCwd strin } command := exec.CommandContext(ctx, spec.Name, spec.Args...) command.Dir = spec.Dir - applyWindowsShellCommandLine(command, commandText, plan.Wrapped) + applyWindowsShellCommandLine(command, commandText, plan.Wrapped, hostShell.Kind == shellKindCmd) return command, plan, nil } @@ -385,20 +381,6 @@ func interactiveBlockResult(detection zeroSandbox.InteractiveCommandResult) Resu } } -func shellExecutable() string { - if runtime.GOOS == "windows" { - return "cmd.exe" - } - return "/bin/sh" -} - -func shellArguments(command string) []string { - if runtime.GOOS == "windows" { - return zeroSandbox.WindowsShellArgs(command) - } - return []string{"-c", command} -} - func commandExitCode(err error) int { if err == nil { return 0 @@ -621,7 +603,7 @@ func truncateHeadTailWithTotal(value string, total, maxBytes int) (string, int, func formatBashOutputWithShellHint(stdout string, stderr string, exitCode int, meta map[string]string) string { output := formatBashOutput(stdout, stderr, exitCode) - if issue := detectShellOutputIssue(stdout+"\n"+stderr, runtime.GOOS); issue != nil { + if issue := detectShellOutputIssueForRuntime(stdout+"\n"+stderr, detectShellRuntime(runtime.GOOS)); issue != nil { meta["shell_issue"] = issue.Kind output = appendShellIssueHint(output, *issue) } diff --git a/internal/tools/bash_proc_unix.go b/internal/tools/bash_proc_unix.go index 5c9b13733..cc896a90a 100644 --- a/internal/tools/bash_proc_unix.go +++ b/internal/tools/bash_proc_unix.go @@ -36,6 +36,6 @@ func hardenProcessLifetime(command *exec.Cmd) { } } -// applyWindowsShellCommandLine is a no-op outside Windows; there is no -// cmd.exe command-line-parsing quirk to work around here. -func applyWindowsShellCommandLine(command *exec.Cmd, commandText string, wrapped bool) {} +// applyWindowsShellCommandLine is a no-op outside Windows. +func applyWindowsShellCommandLine(command *exec.Cmd, commandText string, wrapped bool, cmdFallback bool) { +} diff --git a/internal/tools/bash_proc_windows.go b/internal/tools/bash_proc_windows.go index 589da5677..a4806c7a4 100644 --- a/internal/tools/bash_proc_windows.go +++ b/internal/tools/bash_proc_windows.go @@ -31,15 +31,12 @@ func hardenProcessLifetime(command *exec.Cmd) { } } -// applyWindowsShellCommandLine overrides command's raw child command line so -// commandText reaches cmd.exe unescaped instead of auto-quoted the way -// exec.Cmd would normally encode a single Args element. Skipped when wrapped -// is true: the sandbox engine then routes execution through a separate -// zero-windows-command-runner process, which builds its own child command -// line from scratch (internal/sandbox/windows_process_windows.go) rather than -// inheriting whatever this outer exec.Cmd is configured with. -func applyWindowsShellCommandLine(command *exec.Cmd, commandText string, wrapped bool) { - if wrapped { +// applyWindowsShellCommandLine overrides the cmd.exe fallback's raw child +// command line so commandText reaches cmd.exe unescaped. PowerShell consumes +// ordinary argv quoting and must not take this path. Wrapped commands are +// handled after unwrapping by the Windows sandbox runner. +func applyWindowsShellCommandLine(command *exec.Cmd, commandText string, wrapped bool, cmdFallback bool) { + if wrapped || !cmdFallback { return } command.SysProcAttr = &syscall.SysProcAttr{CmdLine: zeroSandbox.WindowsShellCommandLine(commandText)} diff --git a/internal/tools/bash_tool_test.go b/internal/tools/bash_tool_test.go index 204d9b194..f00f6e346 100644 --- a/internal/tools/bash_tool_test.go +++ b/internal/tools/bash_tool_test.go @@ -124,11 +124,15 @@ func TestBashToolDescribesHostShellSyntax(t *testing.T) { } if runtime.GOOS == "windows" { - if !strings.Contains(description, "cmd.exe") || !strings.Contains(description, "cwd") { - t.Fatalf("expected Windows cmd.exe and cwd guidance in bash description, got %q", description) - } - if !strings.Contains(description, "double quotes") || !strings.Contains(description, `--jq ".a | b"`) { - t.Fatalf("expected the double-quote metacharacter rule in bash description, got %q", description) + shell := detectShellRuntime(runtime.GOOS) + if shell.Kind == shellKindPowerShell { + for _, want := range []string{"powershell", "cwd", "get-childitem", "select-string", "$env:name"} { + if !strings.Contains(description, want) { + t.Fatalf("expected Windows PowerShell guidance %q in bash description, got %q", want, description) + } + } + } else if !strings.Contains(description, "cmd.exe") || !strings.Contains(description, "cwd") { + t.Fatalf("expected Windows cmd.exe fallback guidance in bash description, got %q", description) } return } @@ -455,13 +459,14 @@ func TestBashToolReturnsNonzeroExitAsError(t *testing.T) { if result.Status != StatusError { t.Fatalf("expected error status, got %s", result.Status) } - for _, want := range []string{"stdout:\nbefore failure", "stderr:\nfailure details", "exit_code: 7"} { + wantExitCode := strconv.Itoa(helperFailureExitCode()) + for _, want := range []string{"stdout:\nbefore failure", "stderr:\nfailure details", "exit_code: " + wantExitCode} { if !strings.Contains(result.Output, want) { t.Fatalf("expected output to contain %q, got %q", want, result.Output) } } - if result.Meta["exit_code"] != "7" { - t.Fatalf("expected exit_code metadata 7, got %q", result.Meta["exit_code"]) + if result.Meta["exit_code"] != wantExitCode { + t.Fatalf("expected exit_code metadata %s, got %q", wantExitCode, result.Meta["exit_code"]) } if result.ExecutionOutcome == nil || result.ExecutionOutcome.State != execution.StateFailed || result.ExecutionOutcome.Kind != execution.OutcomeApplicationFailure { t.Fatalf("execution outcome = %#v, want failed/application_failure", result.ExecutionOutcome) @@ -578,10 +583,16 @@ func TestBashToolRequireEscalatedMsysGuard(t *testing.T) { } registry := NewRegistry() registry.Register(NewScopedBashTool(root, nil)) + msysCommand := "cat somefile.txt" + if detectShellRuntime(runtime.GOOS).Kind == shellKindPowerShell { + // cat is a native Get-Content alias in PowerShell. Use a name that + // still resolves to an incompatible Git-for-Windows MSYS executable. + msysCommand = "grep pattern somefile.txt" + } t.Run("default sandboxing still blocks an MSYS-prone command", func(t *testing.T) { result := registry.RunWithOptions(context.Background(), "bash", map[string]any{ - "command": "cat somefile.txt", + "command": msysCommand, }, RunOptions{ PermissionGranted: true, Sandbox: newEngine(), @@ -594,7 +605,7 @@ func TestBashToolRequireEscalatedMsysGuard(t *testing.T) { t.Run("approved require_escalated bypasses the MSYS guard", func(t *testing.T) { result := registry.RunWithOptions(context.Background(), "bash", map[string]any{ - "command": "cat somefile.txt", + "command": msysCommand, "sandbox_permissions": string(SandboxPermissionsRequireEscalated), }, RunOptions{ PermissionGranted: true, @@ -648,8 +659,9 @@ func TestBashToolIgnoresMsysMarkersInCommandArgumentsAfterFailure(t *testing.T) "command": command, }) - if result.Status != StatusError || result.Meta["exit_code"] != "7" { - t.Fatalf("expected the helper's real exit 7 failure, got %s: %#v", result.Status, result) + wantExitCode := strconv.Itoa(helperFailureExitCode()) + if result.Status != StatusError || result.Meta["exit_code"] != wantExitCode { + t.Fatalf("expected the helper failure exit %s, got %s: %#v", wantExitCode, result.Status, result) } if result.Meta["shell_issue"] == "windows_msys_sandbox" { t.Fatalf("expected the MSYS marker in the command's own argument text to be ignored, got %#v", result) @@ -860,6 +872,9 @@ func TestBashToolRunsCommandLineForLoopSyntax(t *testing.T) { if runtime.GOOS != "windows" { t.Skip("cmd.exe command-line vs batch-file parsing is Windows-specific") } + if detectShellRuntime(runtime.GOOS).Kind != shellKindCmd { + t.Skip("cmd.exe fallback is not selected") + } root := t.TempDir() result := NewScopedBashTool(root, nil).Run(context.Background(), map[string]any{ @@ -881,6 +896,10 @@ func helperCommand(name string) string { return executable + " --zero-bash-helper " + name } +func helperFailureExitCode() int { + return 7 +} + func shellQuote(value string) string { if runtime.GOOS == "windows" { return value diff --git a/internal/tools/exec_command.go b/internal/tools/exec_command.go index b1b902b3d..4db1a566c 100644 --- a/internal/tools/exec_command.go +++ b/internal/tools/exec_command.go @@ -173,7 +173,7 @@ func (tool execCommandTool) run(ctx context.Context, args map[string]any, engine // 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, runtimeGOOS()); issue != nil && !msysGuardBypassed(issue, commandEngine) { + if issue := detectShellCommandIssueForRuntime(commandText, detectShellRuntime(runtimeGOOS())); issue != nil && !msysGuardBypassed(issue, commandEngine) { return shellIssueBlockResult(*issue) } if interactive := zeroSandbox.DetectInteractiveCommand(commandText, runtimeGOOS()); interactive.Interactive { @@ -459,7 +459,7 @@ func execToolResultWithBudget(input execToolResultInput, directBudget bool) Resu } body := formatExecCommandOutput(output, input.sessionID, input.exited, input.exitCode, input.interrupted) if status == StatusError && input.exited && !input.interrupted { - if issue := detectShellOutputIssue(output, runtimeGOOS()); issue != nil { + if issue := detectShellOutputIssueForRuntime(output, detectShellRuntime(runtimeGOOS())); issue != nil { meta["shell_issue"] = issue.Kind body = appendShellIssueHint(body, *issue) } diff --git a/internal/tools/exec_command_test.go b/internal/tools/exec_command_test.go index 8b4f11c8f..f21cda03a 100644 --- a/internal/tools/exec_command_test.go +++ b/internal/tools/exec_command_test.go @@ -76,11 +76,15 @@ func TestExecCommandToolDescribesHostShellSyntax(t *testing.T) { description := strings.ToLower(strings.Join(descriptionParts, " ")) if runtime.GOOS == "windows" { - if !strings.Contains(description, "cmd.exe") || !strings.Contains(description, "cwd") { - t.Fatalf("expected Windows cmd.exe and cwd guidance in exec_command description, got %q", description) - } - if !strings.Contains(description, "double quotes") || !strings.Contains(description, `--jq ".a | b"`) { - t.Fatalf("expected the double-quote metacharacter rule in exec_command description, got %q", description) + shell := detectShellRuntime(runtime.GOOS) + if shell.Kind == shellKindPowerShell { + for _, want := range []string{"powershell", "cwd", "get-childitem", "select-string", "$env:name"} { + if !strings.Contains(description, want) { + t.Fatalf("expected Windows PowerShell guidance %q in exec_command description, got %q", want, description) + } + } + } else if !strings.Contains(description, "cmd.exe") || !strings.Contains(description, "cwd") { + t.Fatalf("expected Windows cmd.exe fallback guidance in exec_command description, got %q", description) } } } @@ -192,9 +196,15 @@ func TestExecCommandRequireEscalatedBypassesMsysGuardAfterApproval(t *testing.T) Policy: sandbox.DefaultPolicy(), Backend: sandbox.Backend{Name: sandbox.BackendUnavailable, Message: "native sandbox unavailable"}, }) + msysCommand := "cat somefile.txt" + if detectShellRuntime(runtime.GOOS).Kind == shellKindPowerShell { + // cat is a native Get-Content alias in PowerShell, so use an executable + // name that still exercises the MSYS guard. + msysCommand = "grep pattern somefile.txt" + } result := registry.RunWithOptions(context.Background(), ExecCommandToolName, map[string]any{ - "cmd": "cat somefile.txt", + "cmd": msysCommand, "sandbox_permissions": string(SandboxPermissionsRequireEscalated), }, RunOptions{ PermissionGranted: true, @@ -204,8 +214,8 @@ func TestExecCommandRequireEscalatedBypassesMsysGuardAfterApproval(t *testing.T) // Assert on the preflight block sentinel (exit_code "-1", set only by // shellIssueBlockResult) rather than shell_issue: once the guard is - // bypassed, "cat somefile.txt" actually runs, and its real, - // PATH-dependent output could otherwise trip the unrelated + // bypassed, the command actually runs, and its real, PATH-dependent output + // could otherwise trip the unrelated // post-execution detectShellOutputIssue heuristic and make this // assertion flaky for reasons unrelated to the guard under test. if result.Meta["exit_code"] == "-1" { @@ -284,8 +294,9 @@ func TestExecCommandApplicationFailureHasTypedOutcome(t *testing.T) { if result.ExecutionOutcome == nil || result.ExecutionOutcome.State != execution.StateFailed || result.ExecutionOutcome.Kind != execution.OutcomeApplicationFailure { t.Fatalf("execution outcome = %#v, want failed/application_failure", result.ExecutionOutcome) } - if result.ExecutionOutcome.Exit == nil || result.ExecutionOutcome.Exit.Code != 7 { - t.Fatalf("execution exit = %#v, want code 7", result.ExecutionOutcome.Exit) + wantExitCode := helperFailureExitCode() + if result.ExecutionOutcome.Exit == nil || result.ExecutionOutcome.Exit.Code != wantExitCode { + t.Fatalf("execution exit = %#v, want code %d", result.ExecutionOutcome.Exit, wantExitCode) } } diff --git a/internal/tools/shell_runtime.go b/internal/tools/shell_runtime.go index dee7ca3da..276f1ef0b 100644 --- a/internal/tools/shell_runtime.go +++ b/internal/tools/shell_runtime.go @@ -1,15 +1,33 @@ package tools import ( + "context" + "os" + "os/exec" + "path/filepath" "regexp" + "runtime" "strings" + "sync" + "time" "unicode" + + zeroSandbox "github.com/Gitlawb/zero/internal/sandbox" +) + +type shellKind string + +const ( + shellKindPOSIX shellKind = "posix" + shellKindPowerShell shellKind = "powershell" + shellKindCmd shellKind = "cmd" ) type shellRuntime struct { GOOS string Executable string Syntax string + Kind shellKind } type shellIssue struct { @@ -20,7 +38,18 @@ type shellIssue struct { const windowsMsysSandboxKind = "windows_msys_sandbox" -const windowsMsysSandboxSuggestion = "MSYS/Cygwin coreutils and shells (bash, sh) from Git for Windows cannot run under Zero's write-restricted Windows sandbox, and the WSL bash launcher cannot reach the WSL service from it either. This also hits native commands that spawn Git Bash internally: git hooks and git/gh credential helpers can fail this way even though git and gh themselves run fine. 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." +const windowsMsysSandboxSuggestion = "MSYS/Cygwin executables and shells (bash, sh) from Git for Windows cannot run under Zero's write-restricted Windows sandbox, and the WSL bash launcher cannot reach the WSL service from it either. This also hits native commands that spawn Git Bash internally: git hooks and git/gh credential helpers can fail this way even though git and gh themselves run fine. Prefer native PowerShell cmdlets or Zero tools (grep, read_file with offset/limit, list_directory, glob). If host-level execution is truly required, rerun with sandbox_permissions: \"require_escalated\" and a narrow justification." + +const windowsPowerShellUTF8Prefix = "try { [Console]::OutputEncoding = [System.Text.Encoding]::UTF8 } catch {}\n" + +const windowsPowerShellFailurePrefix = "$ErrorActionPreference = 'Stop'\n" + +const windowsPowerShellExitSuffix = "\nif ($null -ne $LASTEXITCODE) { exit $LASTEXITCODE }\n" + +var ( + hostShellOnce sync.Once + hostShell shellRuntime +) // windowsMsysProneNames is the single source of truth for POSIX coreutil and // shell names that commonly resolve to a Git-for-Windows MSYS/Cygwin binary @@ -54,24 +83,139 @@ var ( ) func detectShellRuntime(goos string) shellRuntime { + if goos != runtime.GOOS { + return detectShellRuntimeWithLookup(goos, exec.LookPath, os.Getenv) + } + hostShellOnce.Do(func() { + hostShell = detectShellRuntimeWithProbe(goos, exec.LookPath, os.Getenv, powerShellExecutableUsable) + }) + return hostShell +} + +func detectShellRuntimeWithLookup(goos string, lookPath func(string) (string, error), getenv func(string) string) shellRuntime { + return detectShellRuntimeWithProbe(goos, lookPath, getenv, func(string) bool { return true }) +} + +func detectShellRuntimeWithProbe(goos string, lookPath func(string) (string, error), getenv func(string) string, usable func(string) bool) shellRuntime { if goos == "windows" { - return shellRuntime{GOOS: goos, Executable: "cmd.exe", Syntax: "Windows cmd.exe"} + for _, candidate := range windowsPowerShellCandidates(getenv) { + if path, err := lookPath(candidate); err == nil && strings.TrimSpace(path) != "" && usable(path) { + return shellRuntime{ + GOOS: goos, + Executable: path, + Syntax: "PowerShell", + Kind: shellKindPowerShell, + } + } + } + return shellRuntime{GOOS: goos, Executable: "cmd.exe", Syntax: "Windows cmd.exe", Kind: shellKindCmd} + } + return shellRuntime{GOOS: goos, Executable: "/bin/sh", Syntax: "/bin/sh", Kind: shellKindPOSIX} +} + +func powerShellExecutableUsable(path string) bool { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + command := exec.CommandContext(ctx, path, "-NoLogo", "-NoProfile", "-Command", "exit 0") + return command.Run() == nil +} + +func windowsPowerShellCandidates(getenv func(string) string) []string { + candidates := []string{"pwsh.exe", "pwsh"} + if programFiles := strings.TrimSpace(getenv("ProgramFiles")); programFiles != "" { + candidates = append(candidates, filepath.Join(programFiles, "PowerShell", "7", "pwsh.exe")) + } + candidates = append(candidates, "powershell.exe", "powershell") + if systemRoot := strings.TrimSpace(getenv("SystemRoot")); systemRoot != "" { + candidates = append(candidates, filepath.Join(systemRoot, "System32", "WindowsPowerShell", "v1.0", "powershell.exe")) + } + return candidates +} + +func (shell shellRuntime) arguments(command string) []string { + switch shell.Kind { + case shellKindPowerShell: + script := windowsPowerShellUTF8Prefix + windowsPowerShellFailurePrefix + command + windowsPowerShellExitSuffix + return []string{"-NoLogo", "-NoProfile", "-Command", script} + case shellKindCmd: + return zeroSandbox.WindowsShellArgs(command) + default: + return []string{"-c", command} } - return shellRuntime{GOOS: goos, Executable: "/bin/sh", Syntax: "/bin/sh"} +} + +// HostShellCommand returns the shell executable and argv used for a command on +// this host. Keeping this in one place makes agent tools and the user-typed TUI +// shell escape use the same Windows shell and fallback behavior. +func HostShellCommand(command string) (string, []string) { + shell := detectShellRuntime(runtime.GOOS) + return shell.Executable, shell.arguments(command) +} + +// NewHostShellCommandContext constructs a direct host-shell command. The +// cmd.exe fallback needs a Windows-specific raw command line, while PowerShell +// and POSIX shells use normal argv handling. +func NewHostShellCommandContext(ctx context.Context, commandText string) *exec.Cmd { + shell := detectShellRuntime(runtime.GOOS) + command := exec.CommandContext(ctx, shell.Executable, shell.arguments(commandText)...) + applyWindowsShellCommandLine(command, commandText, false, shell.Kind == shellKindCmd) + return command } func shellGuidanceForGOOS(goos string) string { - runtime := detectShellRuntime(goos) - if goos == "windows" { - return "Uses " + runtime.Syntax + " syntax on Windows; prefer cwd over cd when changing directories. To include | & > < or other metacharacters in an argument value, wrap the value in double quotes (e.g. --jq \".a | b\"); single quotes do not protect metacharacters in cmd.exe. MSYS/Cygwin coreutils on PATH (Git for Windows usr\\bin) are not sandbox-compatible; prefer native Zero file tools." + return shellGuidanceForRuntime(detectShellRuntime(goos)) +} + +func shellGuidanceForRuntime(shell shellRuntime) string { + if shell.GOOS == "windows" && shell.Kind == shellKindPowerShell { + guidance := "Uses PowerShell syntax on Windows; prefer cwd/workdir over Set-Location when changing directories. Examples: Get-ChildItem -Force; Get-ChildItem -Recurse -Filter *.go; Get-ChildItem -Recurse | Select-String -Pattern 'TODO'; Get-Process | Where-Object { $_.ProcessName -like '*node*' }; $env:NAME='value'; @'\nprint('hello')\n'@ | python -. Do not invoke Git-for-Windows MSYS/Cygwin executables (bash, sh, grep.exe, sed.exe, head.exe, and similar) inside the restricted sandbox; prefer PowerShell cmdlets or native Zero tools." + return guidance + legacyPowerShellChainGuidance(shell) } - guidance := "Uses " + runtime.Syntax + " syntax." - if goos == "darwin" { + if shell.GOOS == "windows" { + return "Uses " + shell.Syntax + " syntax on Windows because PowerShell was unavailable; prefer cwd/workdir over cd when changing directories. To include | & > < or other metacharacters in an argument value, wrap the value in double quotes (e.g. --jq \".a | b\"); single quotes do not protect metacharacters in cmd.exe. MSYS/Cygwin coreutils on PATH (Git for Windows usr\\bin) are not sandbox-compatible; prefer native Zero file tools." + } + guidance := "Uses " + shell.Syntax + " syntax." + if shell.GOOS == "darwin" { guidance += " To find or stop a process, use `lsof -i :PORT` (or `lsof -nP -iTCP -sTCP:LISTEN`) for the PID then `kill `; `ps` and `pgrep` do not work under the sandbox." } return guidance } +// HostShellEnvironmentGuidance returns the concise, model-facing shell rule +// for the current host. +func HostShellEnvironmentGuidance() string { + return hostShellEnvironmentGuidanceForRuntime(detectShellRuntime(runtime.GOOS)) +} + +func hostShellEnvironmentGuidanceForRuntime(shell shellRuntime) string { + if shell.GOOS == "windows" && shell.Kind == shellKindPowerShell { + guidance := "Shell syntax: PowerShell for exec_command/bash tools. Use PowerShell cmdlets and pipelines (Get-ChildItem, Get-Content, Select-String, Select-Object), use $env:NAME='value' for environment variables, and prefer the workdir/cwd argument over Set-Location. Do not invoke Git-for-Windows MSYS binaries (bash, sh, grep.exe, sed.exe, head.exe, and similar) inside the restricted sandbox; use native PowerShell cmdlets or Zero tools instead, or sandbox_permissions require_escalated only when host-level execution is truly required." + return guidance + legacyPowerShellChainGuidance(shell) + } + if shell.GOOS == "windows" { + return "Shell syntax: Windows cmd.exe syntax for exec_command/bash tools because PowerShell is unavailable. To put | & > < etc inside an arg value, use double quotes around the value, not single quotes. Do not invoke Git-for-Windows MSYS binaries inside the restricted sandbox; use native Zero tools instead. Prefer the workdir/cwd argument over cd." + } + return "Shell syntax: /bin/sh syntax for exec_command/bash tools; prefer the workdir/cwd argument instead of cd when changing directories." +} + +func legacyPowerShellChainGuidance(shell shellRuntime) string { + if !shell.isWindowsPowerShell() { + return "" + } + return " This host uses Windows PowerShell 5.1, which does not support && or ||. Use separate statements when execution is unconditional, or if ($?) { ... } and if (-not $?) { ... } for conditional chaining." +} + +func (shell shellRuntime) isWindowsPowerShell() bool { + if shell.Kind != shellKindPowerShell { + return false + } + executable := strings.TrimSpace(shell.Executable) + if index := strings.LastIndexAny(executable, `\/`); index >= 0 { + executable = executable[index+1:] + } + return strings.EqualFold(executable, "powershell") || strings.EqualFold(executable, "powershell.exe") +} + // MsysProneCommandName reports whether a bare command name commonly resolves to // a Git-for-Windows MSYS binary that fails under the Windows restricted sandbox. func MsysProneCommandName(name string) bool { @@ -172,7 +316,15 @@ func msysProneCommandWord(word string) bool { } func detectShellCommandIssue(command string, goos string) *shellIssue { - if goos != "windows" { + shell := shellRuntime{GOOS: goos, Executable: "/bin/sh", Syntax: "/bin/sh", Kind: shellKindPOSIX} + if goos == "windows" { + shell = shellRuntime{GOOS: goos, Executable: "cmd.exe", Syntax: "Windows cmd.exe", Kind: shellKindCmd} + } + return detectShellCommandIssueForRuntime(command, shell) +} + +func detectShellCommandIssueForRuntime(command string, shell shellRuntime) *shellIssue { + if shell.GOOS != "windows" { return nil } trimmed := strings.TrimSpace(command) @@ -184,7 +336,24 @@ func detectShellCommandIssue(command string, goos string) *shellIssue { // cat), so an escaped metachar can't stand in as a fake segment boundary // either. unquoted := stripCmdCaretEscapes(stripDoubleQuotedSpans(trimmed)) + if shell.Kind == shellKindPowerShell { + unquoted = stripPowerShellQuotedSpans(trimmed) + if shell.isWindowsPowerShell() && (strings.Contains(unquoted, "&&") || strings.Contains(unquoted, "||")) { + return &shellIssue{ + Kind: "windows_powershell_version", + Message: "Command uses && or ||, but this host runs Windows PowerShell 5.1, which does not support those operators.", + Suggestion: "Use separate statements when execution is unconditional, or if ($?) { ... } and if (-not $?) { ... } for conditional chaining.", + } + } + } if windowsBashStyleCDPattern.MatchString(unquoted) { + if shell.Kind == shellKindPowerShell { + return &shellIssue{ + Kind: "windows_shell_syntax", + Message: "Command looks like POSIX/Bash syntax, but Zero runs PowerShell commands on this Windows host.", + Suggestion: "Use the cwd/workdir argument instead of cd and use native PowerShell syntax or Zero tools such as list_directory, read_file, grep, and glob.", + } + } 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.", @@ -198,18 +367,128 @@ func detectShellCommandIssue(command string, goos string) *shellIssue { // Being segment/word anchored rather than a whole-string regex or scan, // neither check matches text that only appears inside a quoted argument // (e.g. a commit message or PR comment body discussing head.exe). - for _, segment := range windowsCommandSegments(trimmed) { + segments := windowsCommandSegments(trimmed) + if shell.Kind == shellKindPowerShell { + segments = powerShellCommandSegments(trimmed) + } + for _, segment := range segments { word := firstCommandWord(segment) if windowsMsysBinaryPathPattern.MatchString(word) { return windowsMsysSandboxIssue("Command invokes an MSYS/Cygwin binary path that cannot run under Zero's Windows sandbox.") } if msysProneCommandWord(word) { + if shell.Kind == shellKindPowerShell && powerShellAliasWord(word) { + continue + } return windowsMsysSandboxIssue("Command uses a POSIX coreutil (head/tail/grep/cat/...) that commonly resolves to Git-for-Windows MSYS binaries incompatible with the Windows sandbox.") } } return nil } +// powerShellCommandSegments splits the subset of PowerShell syntax needed for +// command-position checks. It respects single/double quoted strings and the +// backtick escape, then splits on pipeline/statement operators. Complex script +// interpretation remains PowerShell's job; this scanner only determines +// whether a known-incompatible MSYS executable is actually invoked. +func powerShellCommandSegments(command string) []string { + var segments []string + var current strings.Builder + var quote rune + escaped := false + for _, c := range command { + if escaped { + current.WriteRune(c) + escaped = false + continue + } + if c == '`' { + current.WriteRune(c) + escaped = true + continue + } + if quote != 0 { + current.WriteRune(c) + if c == quote { + quote = 0 + } + continue + } + if c == '\'' || c == '"' { + quote = c + current.WriteRune(c) + continue + } + if c == ';' || c == '|' || c == '&' { + if segment := strings.TrimSpace(current.String()); segment != "" { + segments = append(segments, segment) + } + current.Reset() + continue + } + current.WriteRune(c) + } + if segment := strings.TrimSpace(current.String()); segment != "" { + segments = append(segments, segment) + } + return segments +} + +func powerShellAliasWord(word string) bool { + trimmed := strings.Trim(word, `"'`) + if strings.ContainsAny(trimmed, `\/`) || strings.HasSuffix(strings.ToLower(trimmed), ".exe") { + return false + } + switch strings.ToLower(trimmed) { + case "cat", "ls": + return true + default: + return false + } +} + +func stripPowerShellQuotedSpans(command string) string { + var b strings.Builder + b.Grow(len(command)) + var quote rune + escaped := false + runes := []rune(command) + for index := 0; index < len(runes); index++ { + c := runes[index] + if escaped { + b.WriteRune(' ') + escaped = false + continue + } + if c == '`' { + b.WriteRune(' ') + escaped = true + continue + } + if quote != 0 { + b.WriteRune(' ') + if c == quote { + // PowerShell escapes a single quote inside a single-quoted + // string by doubling it. + if quote == '\'' && index+1 < len(runes) && runes[index+1] == '\'' { + index++ + b.WriteRune(' ') + continue + } + quote = 0 + } + continue + } + if c == '\'' || c == '"' { + quote = c + b.WriteRune(' ') + continue + } + b.WriteRune(c) + } + return b.String() +} + // 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 @@ -256,7 +535,15 @@ func stripCmdCaretEscapes(command string) string { // quoted-text false positives the preflight command-position check exists to // avoid, just after execution instead of before it. func detectShellOutputIssue(output string, goos string) *shellIssue { - if goos != "windows" { + shell := shellRuntime{GOOS: goos, Executable: "/bin/sh", Syntax: "/bin/sh", Kind: shellKindPOSIX} + if goos == "windows" { + shell = shellRuntime{GOOS: goos, Executable: "cmd.exe", Syntax: "Windows cmd.exe", Kind: shellKindCmd} + } + return detectShellOutputIssueForRuntime(output, shell) +} + +func detectShellOutputIssueForRuntime(output string, shell shellRuntime) *shellIssue { + if shell.GOOS != "windows" { return nil } lower := strings.ToLower(output) @@ -266,8 +553,8 @@ func detectShellOutputIssue(output string, goos string) *shellIssue { if wslServiceDeniedInOutput(lower) { return windowsMsysSandboxIssue("WSL bash could not connect to the WSL service under Zero's Windows sandbox (Bash/Service/CreateInstance/E_ACCESSDENIED).") } - if strings.Contains(lower, "the syntax of the command is incorrect") || - strings.Contains(lower, "is not recognized as an internal or external command") { + if shell.Kind == shellKindCmd && (strings.Contains(lower, "the syntax of the command is incorrect") || + strings.Contains(lower, "is not recognized as an internal or external command")) { return &shellIssue{ Kind: "windows_shell_syntax", Message: "Windows cmd.exe rejected the command syntax.", diff --git a/internal/tools/shell_runtime_test.go b/internal/tools/shell_runtime_test.go index 63bfef864..87dd5b5cb 100644 --- a/internal/tools/shell_runtime_test.go +++ b/internal/tools/shell_runtime_test.go @@ -1,10 +1,218 @@ package tools import ( + "errors" + "os/exec" + "runtime" "strings" "testing" ) +func TestDetectShellRuntimePrefersPowerShellSevenOnWindows(t *testing.T) { + lookPath := func(name string) (string, error) { + if name == "pwsh.exe" { + return `C:\Program Files\PowerShell\7\pwsh.exe`, nil + } + return "", errors.New("not found") + } + shell := detectShellRuntimeWithLookup("windows", lookPath, func(string) string { return "" }) + if shell.Kind != shellKindPowerShell || !strings.HasSuffix(strings.ToLower(shell.Executable), `\pwsh.exe`) { + t.Fatalf("shell = %#v, want PowerShell 7", shell) + } +} + +func TestDetectShellRuntimeFallsBackThroughWindowsPowerShellToCmd(t *testing.T) { + t.Run("windows powershell", func(t *testing.T) { + lookPath := func(name string) (string, error) { + if name == "powershell.exe" { + return `C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe`, nil + } + return "", errors.New("not found") + } + shell := detectShellRuntimeWithLookup("windows", lookPath, func(string) string { return "" }) + if shell.Kind != shellKindPowerShell || !strings.HasSuffix(strings.ToLower(shell.Executable), `\powershell.exe`) { + t.Fatalf("shell = %#v, want Windows PowerShell", shell) + } + }) + + t.Run("cmd", func(t *testing.T) { + shell := detectShellRuntimeWithLookup( + "windows", + func(string) (string, error) { return "", errors.New("not found") }, + func(string) string { return "" }, + ) + if shell.Kind != shellKindCmd || shell.Executable != "cmd.exe" { + t.Fatalf("shell = %#v, want cmd.exe fallback", shell) + } + }) +} + +func TestDetectShellRuntimeSkipsUnusablePowerShell(t *testing.T) { + lookPath := func(name string) (string, error) { + switch name { + case "pwsh.exe": + return `C:\broken\pwsh.exe`, nil + case "powershell.exe": + return `C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe`, nil + default: + return "", errors.New("not found") + } + } + shell := detectShellRuntimeWithProbe( + "windows", + lookPath, + func(string) string { return "" }, + func(path string) bool { return !strings.Contains(path, `\broken\`) }, + ) + if shell.Kind != shellKindPowerShell || strings.Contains(shell.Executable, `\broken\`) { + t.Fatalf("shell = %#v, want usable Windows PowerShell fallback", shell) + } +} + +func TestPowerShellArgumentsDisableProfilesAndRequestUTF8(t *testing.T) { + shell := shellRuntime{GOOS: "windows", Executable: "pwsh.exe", Syntax: "PowerShell", Kind: shellKindPowerShell} + args := shell.arguments("Write-Output 'hello'") + joined := strings.Join(args, "\n") + for _, want := range []string{ + "-NoLogo", + "-NoProfile", + "-Command", + windowsPowerShellUTF8Prefix, + "$ErrorActionPreference = 'Stop'", + "Write-Output 'hello'", + "exit $LASTEXITCODE", + } { + if !strings.Contains(joined, want) { + t.Fatalf("PowerShell args missing %q: %#v", want, args) + } + } + script := args[len(args)-1] + preferenceIndex := strings.Index(script, "$ErrorActionPreference") + commandIndex := strings.Index(script, "Write-Output 'hello'") + exitIndex := strings.Index(script, "exit $LASTEXITCODE") + if preferenceIndex < 0 || commandIndex <= preferenceIndex || exitIndex <= commandIndex { + t.Fatalf("PowerShell failure handling is not ordered around the command: %q", script) + } +} + +func TestWindowsPowerShellArgumentsPropagateFailures(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Windows PowerShell integration test") + } + executable, err := exec.LookPath("powershell.exe") + if err != nil { + t.Skipf("Windows PowerShell unavailable: %v", err) + } + shell := shellRuntime{ + GOOS: "windows", + Executable: executable, + Syntax: "PowerShell", + Kind: shellKindPowerShell, + } + + t.Run("native exit code after cmdlet", func(t *testing.T) { + command := exec.Command(executable, shell.arguments("cmd /c exit 5; Write-Output done")...) + output, err := command.CombinedOutput() + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) || exitErr.ExitCode() != 5 { + t.Fatalf("exit = %v, output = %q; want exact native exit code 5", err, output) + } + if !strings.Contains(string(output), "done") { + t.Fatalf("output = %q, want command output before propagated failure", output) + } + }) + + t.Run("cmdlet error stops script", func(t *testing.T) { + missing := strings.ReplaceAll(t.TempDir()+`\missing`, `'`, `''`) + commandText := "Get-Item -LiteralPath '" + missing + "'; Write-Output after" + command := exec.Command(executable, shell.arguments(commandText)...) + output, err := command.CombinedOutput() + if err == nil { + t.Fatalf("cmdlet failure reported success: %q", output) + } + if strings.Contains(string(output), "after") { + t.Fatalf("script continued after cmdlet failure: %q", output) + } + }) +} + +func TestWindowsPowerShellGuidanceAvoidsUnsupportedChainOperators(t *testing.T) { + shell := shellRuntime{ + GOOS: "windows", + Executable: `C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe`, + Syntax: "PowerShell", + Kind: shellKindPowerShell, + } + for _, guidance := range []string{ + shellGuidanceForRuntime(shell), + hostShellEnvironmentGuidanceForRuntime(shell), + } { + for _, want := range []string{"Windows PowerShell 5.1", "&&", "||", "if ($?)"} { + if !strings.Contains(guidance, want) { + t.Fatalf("legacy PowerShell guidance missing %q: %q", want, guidance) + } + } + } +} + +func TestWindowsPowerShellCommandIssueFlagsUnsupportedChainOperators(t *testing.T) { + legacy := shellRuntime{ + GOOS: "windows", + Executable: "powershell.exe", + Syntax: "PowerShell", + Kind: shellKindPowerShell, + } + for _, command := range []string{ + "Write-Output one && Write-Output two", + "Write-Output one || Write-Output two", + } { + issue := detectShellCommandIssueForRuntime(command, legacy) + if issue == nil || issue.Kind != "windows_powershell_version" { + t.Fatalf("legacy PowerShell command %q issue = %#v, want version issue", command, issue) + } + } + for _, command := range []string{ + `Write-Output "one && two"`, + "Write-Output 'one || two'", + "Write-Output one `&`& Write-Output two", + } { + if issue := detectShellCommandIssueForRuntime(command, legacy); issue != nil { + t.Fatalf("literal legacy PowerShell operators in %q produced issue %#v", command, issue) + } + } + + modern := shellRuntime{GOOS: "windows", Executable: "pwsh.exe", Syntax: "PowerShell", Kind: shellKindPowerShell} + if issue := detectShellCommandIssueForRuntime("Write-Output one && Write-Output two", modern); issue != nil { + t.Fatalf("PowerShell 7 chain operator produced issue %#v", issue) + } +} + +func TestPowerShellCommandIssueAllowsAliasesAndBlocksMsysExecutables(t *testing.T) { + shell := shellRuntime{GOOS: "windows", Executable: "pwsh.exe", Syntax: "PowerShell", Kind: shellKindPowerShell} + for _, command := range []string{ + `ls -Force`, + `cat README.md`, + `Get-ChildItem | Select-Object -First 10`, + `Write-Output 'grep README.md; head file.txt'`, + `Write-Output 'cd /tmp'`, + } { + if issue := detectShellCommandIssueForRuntime(command, shell); issue != nil { + t.Fatalf("PowerShell-native command %q was blocked: %#v", command, issue) + } + } + for _, command := range []string{ + `grep README.md`, + `Get-Content README.md | head -10`, + `cat.exe README.md`, + `bash -lc "make test"`, + `Write-Output ok; sed -n '1,5p' README.md`, + } { + if issue := detectShellCommandIssueForRuntime(command, shell); issue == nil || issue.Kind != windowsMsysSandboxKind { + t.Fatalf("MSYS-prone command %q was not blocked: %#v", command, issue) + } + } +} + func TestDetectShellCommandIssueFlagsMsysBinaryPaths(t *testing.T) { for _, command := range []string{ `for /F %i in ('whoami') do echo %i | "C:\Program Files\Git\usr\bin\head.exe" -1`, diff --git a/internal/tui/command_bash.go b/internal/tui/command_bash.go index 274e6cf9c..7f036d8bf 100644 --- a/internal/tui/command_bash.go +++ b/internal/tui/command_bash.go @@ -2,12 +2,11 @@ package tui import ( "context" - "os/exec" - "runtime" "strings" "time" tea "charm.land/bubbletea/v2" + "github.com/Gitlawb/zero/internal/tools" ) // bashEscapeTimeout bounds a "!cmd" shell escape so a hung command can't freeze @@ -30,8 +29,7 @@ func runBashEscape(cwd, command string) tea.Cmd { ctx, cancel := context.WithTimeout(context.Background(), bashEscapeTimeout) defer cancel() - name, shellArgs := escapeShell(command) - cmd := exec.CommandContext(ctx, name, shellArgs...) + cmd := tools.NewHostShellCommandContext(ctx, command) if strings.TrimSpace(cwd) != "" { cmd.Dir = cwd } @@ -51,15 +49,11 @@ func runBashEscape(cwd, command string) tea.Cmd { } } -// escapeShell returns the platform shell and arguments for a "!cmd" escape, -// matching the agent bash tool: cmd.exe on Windows, /bin/sh elsewhere. Hardcoding -// "bash" broke the escape on stock Windows (no bash on PATH) and anywhere bash is -// not installed at a predictable path. +// escapeShell returns the platform shell and arguments for tests and display. +// Execution itself uses tools.NewHostShellCommandContext so the cmd.exe +// fallback receives its required raw Windows command line. func escapeShell(command string) (string, []string) { - if runtime.GOOS == "windows" { - return "cmd.exe", []string{"/d", "/s", "/c", command} - } - return "/bin/sh", []string{"-c", command} + return tools.HostShellCommand(command) } func appendNote(text, note string) string { diff --git a/internal/tui/tui_fixes_test.go b/internal/tui/tui_fixes_test.go index 77e3f36ff..9458dffc7 100644 --- a/internal/tui/tui_fixes_test.go +++ b/internal/tui/tui_fixes_test.go @@ -42,19 +42,19 @@ func TestBashEscapeGatedByPermissionMode(t *testing.T) { } } -// The "!" escape must use the platform shell (cmd.exe on Windows, /bin/sh -// elsewhere), not a hardcoded "bash" that is absent on stock Windows. +// The "!" escape must use the same detected host shell as the agent tools. func TestEscapeShellWrapsCommandForPlatform(t *testing.T) { name, args := escapeShell("echo hi") if name == "" { t.Fatal("escapeShell returned an empty executable") } - if len(args) == 0 || args[len(args)-1] != "echo hi" { - t.Fatalf("escapeShell args = %v, want the command as the final arg", args) + if len(args) == 0 || !strings.Contains(args[len(args)-1], "echo hi") { + t.Fatalf("escapeShell args = %v, want the command in the final arg", args) } if runtime.GOOS == "windows" { - if name != "cmd.exe" || args[0] != "/d" { - t.Fatalf("windows shell = %q %v, want cmd.exe /d /s /c", name, args) + lowerName := strings.ToLower(filepath.Base(name)) + if lowerName != "pwsh.exe" && lowerName != "powershell.exe" && lowerName != "cmd.exe" { + t.Fatalf("windows shell = %q %v, want PowerShell or cmd.exe fallback", name, args) } } else if name != "/bin/sh" || args[0] != "-c" { t.Fatalf("unix shell = %q %v, want /bin/sh -c", name, args) diff --git a/scripts/sandbox-smoke.sh b/scripts/sandbox-smoke.sh index 23ec10564..d0741b170 100755 --- a/scripts/sandbox-smoke.sh +++ b/scripts/sandbox-smoke.sh @@ -52,7 +52,7 @@ case "$(go env GOOS)" in ZERO_SANDBOX_REAL_SMOKE=1 \ ZERO_WINDOWS_COMMAND_RUNNER_EXE="$tmpdir/windows-zero-windows-command-runner.exe" \ ZERO_WINDOWS_SANDBOX_SETUP_EXE="$tmpdir/windows-zero-windows-sandbox-setup.exe" \ - go test ./internal/sandbox -run TestWindowsRestrictedTokenRealSandboxSmoke -count=1 + go test ./internal/sandbox -run 'TestWindowsRestrictedTokenRealSandboxSmoke|TestWindowsRestrictedTokenPowerShell' -count=1 ;; *) echo "No real sandbox smoke is defined for this host platform."