fix(security): harden shell flag parsing, write roots, and formatters

internal/sandbox/analyzer.go

@@ -423,11 +423,18 @@ func effectiveProgram(args []*syntax.Word) (string, []*syntax.Word) {
 	return "", nil
 }
 
-// dashCPayload returns the literal text of the word following `-c` in an AST arg
-// list (the command a shell launcher will run), or "" when there is none.
+// dashCPayload returns the literal text of the word following the shell's
+// command flag in an AST arg list (the command a shell launcher will run), or ""
+// when there is none. shellCommandFlag recognizes both a bare `-c`/`--command`
+// and a POSIX short-option cluster such as `-ec`, `-lc`, or `-xc`.
 func dashCPayload(args []*syntax.Word) string {
 	for index := 0; index < len(args); index++ {
-		if wordText(args[index]) == "-c" && index+1 < len(args) {
+		// `--` ends option processing: the remaining words are positional
+		// operands, so a later `-ec`/`-c` is not the shell's command flag.
+		if wordText(args[index]) == "--" {
+			break
+		}
+		if shellCommandFlag(wordText(args[index])) && index+1 < len(args) {
 			return wordText(args[index+1])
 		}
 	}

internal/sandbox/analyzer_test.go

@@ -31,6 +31,19 @@ func TestAnalyzeCommand(t *testing.T) {
 		{name: "sudo wraps rm -rf", script: "sudo rm -rf /tmp/x", destructive: true},
 		{name: "env wraps curl", script: "env curl https://x.test", network: true},
 		{name: "bash -c wraps editor", script: `bash -c 'vim file'`, interactive: true},
+		// POSIX getopt clusters the command flag with other short options; the
+		// payload must still be parsed and classified (ZERO-ESC-02).
+		{name: "bash -ec wraps destructive payload", script: `bash -ec 'fdisk /dev/sda'`, destructive: true},
+		{name: "sh -lc wraps destructive payload", script: `sh -lc 'shred -u secret.txt'`, destructive: true},
+		{name: "zsh -xc wraps destructive payload", script: `zsh -xc 'parted /dev/sda mklabel gpt'`, destructive: true},
+		{name: "bash -ec wraps network payload", script: `bash -ec 'curl https://x.test'`, network: true},
+		{name: "bash --command wraps editor", script: `bash --command 'vim file'`, interactive: true},
+		// `--` ends option processing, so a `-ec`/`-c` after it is a positional
+		// operand, not the shell command flag: the quoted text must NOT be parsed
+		// as a payload command and must not trigger its classification.
+		{name: "bash -- -ec editor is not a payload", script: `bash -- -ec 'vim file'`, interactive: false},
+		{name: "bash -- -ec fdisk is not a payload", script: `bash -- -ec 'fdisk /dev/sda'`, destructive: false},
+		{name: "bash -- -c curl is not a payload", script: `bash -- -c 'curl https://x.test'`, network: false},
 		{name: "sudo wraps bare repl", script: "sudo python3", interactive: true},
 		// A valueless wrapper flag must not swallow the real payload command.
 		{name: "sudo -n keeps rm payload", script: "sudo -n rm -rf /tmp/x", destructive: true},

internal/sandbox/runner.go

@@ -1093,6 +1093,15 @@ func regexpQuoteMeta(value string) string {
 	return replacer.Replace(value)
 }
 
+// ScrubSensitiveEnv removes credential-bearing variables from a child
+// environment. It is exported for callers that exec a host tool OUTSIDE the
+// platform sandbox — notably format-on-write, which runs a project's formatter
+// in-process — so that a formatter doing dynamic configuration evaluation
+// cannot read API keys and tokens out of the inherited environment.
+func ScrubSensitiveEnv(env []string) []string {
+	return scrubSensitiveEnv(env)
+}
+
 func scrubSensitiveEnv(env []string, additionalKeys ...string) []string {
 	// Secrets not covered by the provider catalog: cloud/VCS credentials and
 	// providers Zero talks to through generic OpenAI-compatible endpoints.

internal/sandbox/safe_command.go

@@ -400,6 +400,32 @@ func isNumericToken(field string) bool {
 	return true
 }
 
+// shellCommandFlag reports whether arg is a shell option token that carries the
+// command string to run — the `-c`/`--command` flag — so callers can recurse into
+// the payload. POSIX getopt lets a shell cluster its short options, so the flag
+// is frequently grouped with other letters (`bash -ec`, `sh -lc`, `zsh -xc`);
+// a bare `-c` is only the one-letter case. A long option other than the exact
+// `--command` is not a match. `-o` and `-O` consume the remainder of their
+// cluster (or the next token) as an option value, so a `c` that follows them is
+// that value, not the command flag.
+func shellCommandFlag(arg string) bool {
+	if arg == "--command" {
+		return true
+	}
+	if len(arg) < 2 || arg[0] != '-' || arg[1] == '-' {
+		return false
+	}
+	for _, flag := range arg[1:] {
+		switch flag {
+		case 'c':
+			return true
+		case 'o', 'O':
+			return false
+		}
+	}
+	return false
+}
+
 // shellDashCPayload returns the command string passed to `sh -c`/`bash -c`
 // (and other POSIX shells) so the caller can recurse into it, or "" when the
 // segment is not a `<shell> -c <payload>` invocation. The payload is returned
@@ -416,7 +442,14 @@ func shellDashCPayload(program string, fields []string) string {
 	}
 	args := fields[start+1:]
 	for i, arg := range args {
-		if arg == "-c" || arg == "--command" {
+		// `--` ends option processing: every following token is a positional
+		// operand (a script name or argument), not a shell flag. Treating
+		// `bash -- -ec 'cmd'` as `-c` would recurse into an operand the shell
+		// never runs as a command string.
+		if arg == "--" {
+			break
+		}
+		if shellCommandFlag(arg) {
 			if i+1 < len(args) {
 				return strings.Join(args[i+1:], " ")
 			}

internal/sandbox/safe_command_test.go

@@ -67,6 +67,10 @@ func TestDetectInteractiveCommandAllowsNonInteractive(t *testing.T) {
 		"tail -n 50 app.log",
 		"ssh host 'uptime'",
 		"grep -r foo .",
+		// After the `--` separator the following tokens are positional operands,
+		// not shell flags, so `-ec` must not be read as the `-c` command flag.
+		"bash -- -ec 'vim file.txt'",
+		"sh -- -c 'less /var/log/syslog'",
 	}
 	for _, command := range cases {
 		t.Run(command, func(t *testing.T) {
@@ -181,6 +185,12 @@ func TestDetectInteractiveThroughWrappersAndShellC(t *testing.T) {
 		{name: "env with assignment option", command: "env -i EDITOR=x vim file.txt", wantCmd: "vim"},
 		{name: "sh -c payload", command: "sh -c 'vim file.txt'", wantCmd: "vim"},
 		{name: "bash -c payload", command: `bash -c "less /var/log/syslog"`, wantCmd: "less"},
+		// POSIX getopt clusters the command flag with other short options; the
+		// payload must still be located and recursed into (ZERO-ESC-02).
+		{name: "sh -ec grouped flags", command: "sh -ec 'vim file.txt'", wantCmd: "vim"},
+		{name: "bash -lc grouped flags", command: `bash -lc "less /var/log/syslog"`, wantCmd: "less"},
+		{name: "zsh -xc grouped flags", command: "zsh -xc 'nano notes.txt'", wantCmd: "nano"},
+		{name: "bash --command long flag", command: `bash --command "less /var/log/syslog"`, wantCmd: "less"},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -195,7 +205,65 @@ func TestDetectInteractiveThroughWrappersAndShellC(t *testing.T) {
 	}
 }
 
-// Audit finding (MED): the interactive-program detector must not be bypassed by
+// Audit ZERO-ESC-02: the shared shell-command-flag helper must honor POSIX
+// getopt short-option clustering, so a command hidden behind `bash -ec`/
+// `sh -lc`/`zsh -xc` (and the legacy `--command`) is still recognized, while
+// unrelated options and option values are not mistaken for the flag.
+func TestShellCommandFlag(t *testing.T) {
+	cases := []struct {
+		arg  string
+		want bool
+	}{
+		{arg: "-c", want: true},
+		{arg: "-ec", want: true},
+		{arg: "-lc", want: true},
+		{arg: "-xc", want: true},
+		{arg: "-ce", want: true},
+		{arg: "--command", want: true},
+		{arg: "-xec", want: true},
+		{arg: "-e", want: false},
+		{arg: "-o", want: false},
+		{arg: "-O", want: false},
+		// `-o`/`-O` consume the rest of the cluster as their value, so a later
+		// `c` is that value (e.g. `-o c`), not the command flag.
+		{arg: "-oc", want: false},
+		{arg: "-Oc", want: false},
+		{arg: "--norc", want: false},
+		{arg: "--command=payload", want: false},
+		{arg: "-", want: false},
+		{arg: "", want: false},
+		{arg: "c", want: false},
+	}
+	for _, tc := range cases {
+		if got := shellCommandFlag(tc.arg); got != tc.want {
+			t.Errorf("shellCommandFlag(%q) = %v, want %v", tc.arg, got, tc.want)
+		}
+	}
+}
+
+// Audit: the `--` separator ends option processing, so a `-c`/`-ec` token
+// appearing after it is a positional operand, not the shell command flag. A
+// payload hidden behind `bash -- -ec '...'` must not be recursed into.
+func TestShellDashCPayloadStopsAtDashDash(t *testing.T) {
+	cases := []struct {
+		name   string
+		fields []string
+		want   string
+	}{
+		{name: "dashdash before clustered flag", fields: []string{"bash", "--", "-ec", "vim file.txt"}, want: ""},
+		{name: "dashdash before bare flag", fields: []string{"sh", "--", "-c", "less file"}, want: ""},
+		{name: "real flag before dashdash", fields: []string{"bash", "-c", "vim file.txt", "--"}, want: "vim file.txt --"},
+		{name: "no dashdash grouped flag", fields: []string{"bash", "-ec", "vim file.txt"}, want: "vim file.txt"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := shellDashCPayload(tc.fields[0], tc.fields); got != tc.want {
+				t.Fatalf("shellDashCPayload(%v) = %q, want %q", tc.fields, got, tc.want)
+			}
+		})
+	}
+}
+
 // quote/escape characters embedded INSIDE the program token (e.g. `vi\m`,
 // `v"i"m`, `'v'im`), not just surrounding it.
 func TestDetectInteractiveStripsEmbeddedQuotingFromToken(t *testing.T) {

internal/tools/edit_file.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"os"
 	"strings"
 )
 
@@ -64,7 +63,16 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
 	if err != nil {
 		return errorResult("Error reading " + requestedPath + ": " + err.Error())
 	}
-	contentBytes, err := os.ReadFile(absolutePath)
+	// Anchor both the read and the write on the granted write root: bytes and
+	// identity come from the same descriptor-bound object, and the commit
+	// below writes back through that same root, so a parent swapped for an
+	// escaping symlink between validation and use cannot redirect either.
+	root, rootedRelative, err := openScopedWriteRoot(tool.workspaceRoot, tool.scope, absolutePath)
+	if err != nil {
+		return errorResult("Error reading " + relativePath + ": " + err.Error())
+	}
+	defer root.Close()
+	contentBytes, priorInfo, err := readRootedFile(root, rootedRelative)
 	if err != nil {
 		return errorResult("Error reading " + relativePath + ": " + err.Error())
 	}
@@ -80,10 +88,6 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
 		}
 	}
 	content := string(contentBytes)
-	priorInfo, err := os.Stat(absolutePath)
-	if err != nil {
-		return errorResult("Error reading " + relativePath + ": " + err.Error())
-	}
 	occurrences := strings.Count(content, oldString)
 
 	// CRLF fallback: read_file normalizes \r\n → \n before presenting content to
@@ -157,7 +161,7 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
 	if err := recheckScopedWriteTarget(tool.workspaceRoot, tool.scope, requestedPath); err != nil {
 		return errorResult("Error writing " + relativePath + ": " + err.Error())
 	}
-	if err := commitFileContents(absolutePath, priorInfo, &content, updated); err != nil {
+	if err := commitRootedFileContents(root, absolutePath, rootedRelative, priorInfo, &content, updated); err != nil {
 		return errorResult("Error writing " + relativePath + ": " + err.Error())
 	}
 	modelKnownContent := updated
@@ -171,7 +175,7 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
 	// compare against the current on-disk state, not the pre-edit version.
 	newInfo := formatting.Info
 	if newInfo == nil {
-		newInfo, _ = os.Stat(absolutePath)
+		newInfo, _ = root.Stat(rootedRelative)
 	}
 	if !finalContentKnown {
 		options.FileTracker.Forget(absolutePath)

internal/tools/file_commit.go

@@ -19,22 +19,29 @@ var fileWriteBeforeCommit func(path string)
 // the file descriptor directly.
 var fileWriteStat = func(file *os.File) (os.FileInfo, error) { return file.Stat() }
 
-// commitFileContents binds an overwrite to the file identity and bytes that
-// the caller observed. A create uses exclusive creation. An overwrite opens the
+// commitRootedFileContents binds an overwrite to the file identity and bytes
+// that the caller observed, opening every component through an already-open
+// *os.Root descriptor rather than re-resolving the absolute path. Because the
+// ancestor directories are traversed relative to the root handle, a parent
+// swapped for a symlink that escapes the workspace between validation and this
+// call is refused by the kernel instead of redirecting the write.
+//
+// A create uses exclusive creation THROUGH THE ROOT. An overwrite opens the
 // observed object without truncation, verifies identity/content through that
-// handle, then truncates and writes the same handle. A path replacement before
-// or during commit therefore fails instead of publishing stale rich evidence.
+// handle, then truncates and writes the same handle. relativePath is the target
+// expressed relative to root; absolutePath is retained only for the test hook
+// and error reporting.
 //
 // expectedInfo nil means the caller observed a missing path. expectedContent
 // may be nil for an existing but unreadable file; that path may still be
 // overwritten, but callers must omit rich before/after evidence.
-func commitFileContents(path string, expectedInfo os.FileInfo, expectedContent *string, content string) error {
+func commitRootedFileContents(root *os.Root, absolutePath, relativePath string, expectedInfo os.FileInfo, expectedContent *string, content string) error {
 	if fileWriteBeforeCommit != nil {
-		fileWriteBeforeCommit(path)
+		fileWriteBeforeCommit(absolutePath)
 	}
 
 	if expectedInfo == nil {
-		file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644)
+		file, err := root.OpenFile(relativePath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644)
 		if err != nil {
 			return err
 		}
@@ -43,14 +50,14 @@ func commitFileContents(path string, expectedInfo os.FileInfo, expectedContent *
 			_ = file.Close()
 			return err
 		}
-		return writeAndVerifyFileIdentity(path, file, openedInfo, content, false)
+		return writeAndVerifyRootedFileIdentity(root, relativePath, file, openedInfo, content, false)
 	}
 
 	flags := os.O_WRONLY
 	if expectedContent != nil {
 		flags = os.O_RDWR
 	}
-	file, err := os.OpenFile(path, flags, 0)
+	file, err := root.OpenFile(relativePath, flags, 0)
 	if err != nil {
 		return err
 	}
@@ -63,7 +70,7 @@ func commitFileContents(path string, expectedInfo os.FileInfo, expectedContent *
 		_ = file.Close()
 		return errFileChangedDuringWrite
 	}
-	pathInfo, err := os.Stat(path)
+	pathInfo, err := root.Stat(relativePath)
 	if err != nil || !os.SameFile(openedInfo, pathInfo) {
 		_ = file.Close()
 		return errFileChangedDuringWrite
@@ -79,10 +86,10 @@ func commitFileContents(path string, expectedInfo os.FileInfo, expectedContent *
 			return errFileChangedDuringWrite
 		}
 	}
-	return writeAndVerifyFileIdentity(path, file, openedInfo, content, true)
+	return writeAndVerifyRootedFileIdentity(root, relativePath, file, openedInfo, content, true)
 }
 
-func writeAndVerifyFileIdentity(path string, file *os.File, openedInfo os.FileInfo, content string, truncate bool) error {
+func writeAndVerifyRootedFileIdentity(root *os.Root, relativePath string, file *os.File, openedInfo os.FileInfo, content string, truncate bool) error {
 	if truncate {
 		if err := file.Truncate(0); err != nil {
 			_ = file.Close()
@@ -100,7 +107,7 @@ func writeAndVerifyFileIdentity(path string, file *os.File, openedInfo os.FileIn
 	if err := file.Close(); err != nil {
 		return err
 	}
-	pathInfo, err := os.Stat(path)
+	pathInfo, err := root.Stat(relativePath)
 	if err != nil || !os.SameFile(openedInfo, pathInfo) {
 		return fmt.Errorf("%w: path identity changed", errFileChangedDuringWrite)
 	}