fix(shell): harden terminal output sanitization

Respond to PR feedback by tightening how terminal-facing text is sanitized across shell execution, logger formatting, and shared escape helpers.

Why:
- timeout and error responses could still surface raw terminal control sequences from captured command output
- logger component formatting needed to sanitize component text before applying color while preserving the already-sanitized value during named-part rendering
- shared escape handling did not yet cover all terminal-relevant control ranges, including C1 controls and private-use code points

What changed:
- move shell output escaping so captured stdout and stderr are sanitized before timeout and error result handling
- split logger field sanitization from final formatting, sanitize component values before colorization, and bypass reformatting for the component part once sanitized
- extend termutil.EscapeControlChars to escape C1 controls and private-use characters in both BMP and supplementary planes

Tests:
- add termutil coverage for C0, C1, bidi, zero-width, and private-use characters
- add logger coverage to verify sanitized component formatting is preserved after FormatPrepare
- add shell coverage to verify timeout output returns escaped ANSI and bidi sequences

pkg/logger/logger.go

@@ -57,7 +57,8 @@ func init() {
 			TimeFormat: "15:04:05", // TODO: make it configurable???
 
 			// Custom formatter to handle multiline strings and JSON objects
-			FormatFieldValue: formatFieldValue,
+			FormatFieldValue:      formatFieldValue,
+			FormatPartValueByName: formatPartValueByName,
 			PartsOrder: []string{
 				zerolog.TimestampFieldName,
 				zerolog.LevelFieldName,
@@ -67,8 +68,11 @@ func init() {
 			},
 			FieldsExclude: []string{Component},
 			FormatPrepare: func(fields map[string]any) error {
+				component := formatComponentValue(fields[Component])
 				if isTTY {
-					fields[Component] = fmt.Sprintf("\x1b[33m%v\x1b[0m", fields[Component])
+					fields[Component] = fmt.Sprintf("\x1b[33m%s\x1b[0m", component)
+				} else {
+					fields[Component] = component
 				}
 				return nil
 			},
@@ -82,6 +86,22 @@ func init() {
 }
 
 func formatFieldValue(i any) string {
+	s := sanitizeFieldString(i)
+	return formatSanitizedFieldValue(s)
+}
+
+func formatPartValueByName(i any, name string) string {
+	if name == Component {
+		return fmt.Sprintf("%v", i)
+	}
+	return formatFieldValue(i)
+}
+
+func formatComponentValue(i any) string {
+	return sanitizeFieldString(i)
+}
+
+func sanitizeFieldString(i any) string {
 	var s string
 
 	switch val := i.(type) {
@@ -97,12 +117,13 @@ func formatFieldValue(i any) string {
 		s = unquoted
 	}
 
-	s = termutil.EscapeControlChars(s)
+	return termutil.EscapeControlChars(s)
+}
 
+func formatSanitizedFieldValue(s string) string {
 	if strings.Contains(s, "\n") {
 		return fmt.Sprintf("\n%s", s)
 	}
-
 	if strings.Contains(s, " ") {
 		if (strings.HasPrefix(s, "{") && strings.HasSuffix(s, "}")) ||
 			(strings.HasPrefix(s, "[") && strings.HasSuffix(s, "]")) {

pkg/logger/logger_test.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -272,6 +273,29 @@ func TestFormatFieldValue(t *testing.T) {
 	}
 }
 
+func TestFormatPreparePreservesColoredComponent(t *testing.T) {
+	fields := map[string]any{
+		Component: "safe\u202ecomponent",
+	}
+
+	if err := consoleWriter.FormatPrepare(fields); err != nil {
+		t.Fatalf("FormatPrepare() error = %v", err)
+	}
+
+	component, ok := fields[Component].(string)
+	if !ok {
+		t.Fatalf("component field type = %T, want string", fields[Component])
+	}
+	if strings.ContainsRune(component, '\u202e') {
+		t.Fatalf("expected bidi control to be escaped in component, got %q", component)
+	}
+
+	formatted := formatPartValueByName(component, Component)
+	if formatted != component {
+		t.Fatalf("formatPartValueByName(component) = %q, want %q", formatted, component)
+	}
+}
+
 func TestDefaultLevelIsInfo(t *testing.T) {
 	// The package-level default (before any SetLevel call) should be INFO.
 	// Because earlier tests may have changed it, we just verify the constant is wired correctly.

pkg/termutil/escape.go

@@ -17,9 +17,9 @@ func EscapeControlChars(input string) string {
 		switch {
 		case r == '\n' || r == '\r' || r == '\t':
 			sb.WriteRune(r)
-		case r < 0x20 || r == 0x7f:
+		case r < 0x20 || r == 0x7f || (r >= 0x80 && r <= 0x9f):
 			sb.WriteString(fmt.Sprintf("\\x%02x", r))
-		case unicode.Is(unicode.Cf, r):
+		case unicode.Is(unicode.Cf, r) || unicode.Is(unicode.Co, r):
 			if r <= 0xffff {
 				sb.WriteString(fmt.Sprintf("\\u%04x", r))
 			} else {

pkg/termutil/escape_test.go

@@ -0,0 +1,70 @@
+package termutil
+
+import "testing"
+
+func TestEscapeControlChars(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "preserves printable text",
+			input:    "hello world",
+			expected: "hello world",
+		},
+		{
+			name:     "preserves whitespace controls",
+			input:    "a\tb\nc\rd",
+			expected: "a\tb\nc\rd",
+		},
+		{
+			name:     "escapes C0 controls",
+			input:    "\x1b[31mred",
+			expected: `\x1b[31mred`,
+		},
+		{
+			name:     "escapes DEL",
+			input:    "a\x7fb",
+			expected: `a\x7fb`,
+		},
+		{
+			name:     "escapes C1 controls",
+			input:    "a\u009bb",
+			expected: `a\x9bb`,
+		},
+		{
+			name:     "escapes bidi override",
+			input:    "safe\u202edanger",
+			expected: `safe\u202edanger`,
+		},
+		{
+			name:     "escapes zero width chars",
+			input:    "a\u200bb",
+			expected: `a\u200bb`,
+		},
+		{
+			name:     "escapes astral format chars",
+			input:    "a\U000e0001b",
+			expected: `a\U000e0001b`,
+		},
+		{
+			name:     "escapes BMP private use chars",
+			input:    "a\ue000b",
+			expected: `a\ue000b`,
+		},
+		{
+			name:     "escapes supplementary private use chars",
+			input:    "a\U000f0000b",
+			expected: `a\U000f0000b`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := EscapeControlChars(tt.input); got != tt.expected {
+				t.Fatalf("EscapeControlChars() = %q, want %q", got, tt.expected)
+			}
+		})
+	}
+}

pkg/tools/shell.go

@@ -436,6 +436,7 @@ func (t *ExecTool) runSync(ctx context.Context, command, cwd string) *ToolResult
 	if stderr.Len() > 0 {
 		output += "\nSTDERR:\n" + stderr.String()
 	}
+	output = termutil.EscapeControlChars(output)
 
 	if err != nil {
 		if errors.Is(cmdCtx.Err(), context.DeadlineExceeded) {
@@ -470,8 +471,6 @@ func (t *ExecTool) runSync(ctx context.Context, command, cwd string) *ToolResult
 		output = "(no output)"
 	}
 
-	output = termutil.EscapeControlChars(output)
-
 	maxLen := 10000
 	if len(output) > maxLen {
 		output = output[:maxLen] + fmt.Sprintf("\n... (truncated, %d more chars)", len(output)-maxLen)
@@ -1205,6 +1204,9 @@ func extractTraversalPathCandidates(command string) []string {
 				continue
 			}
 
+			// This is a lexical heuristic over the literal command text, not a full
+			// shell parser. Paths materialized only after shell expansion are a
+			// known limitation and are handled by the surrounding trust model.
 			if _, ok := seen[part]; ok {
 				continue
 			}

pkg/tools/shell_test.go

@@ -779,6 +779,36 @@ func TestShellTool_OutputEscapesTerminalControlChars(t *testing.T) {
 	}
 }
 
+func TestShellTool_TimeoutOutputEscapesTerminalControlChars(t *testing.T) {
+	tool, err := NewExecTool("", false)
+	if err != nil {
+		t.Fatalf("unable to configure exec tool: %s", err)
+	}
+
+	tool.SetTimeout(100 * time.Millisecond)
+
+	result := tool.Execute(context.Background(), map[string]any{
+		"action":  "run",
+		"command": "printf '\\033[31mred\\033[0m\\u202E'; sleep 10",
+	})
+
+	if !result.IsError {
+		t.Fatalf("expected timeout error, got success: %q", result.ForLLM)
+	}
+	if !strings.Contains(result.ForLLM, "timed out") {
+		t.Fatalf("expected timeout message, got: %q", result.ForLLM)
+	}
+	if strings.Contains(result.ForLLM, "\x1b") {
+		t.Fatalf("expected ANSI escape to be escaped in timeout output, got: %q", result.ForLLM)
+	}
+	if strings.ContainsRune(result.ForLLM, '\u202e') {
+		t.Fatalf("expected bidi control to be escaped in timeout output, got: %q", result.ForLLM)
+	}
+	if !strings.Contains(result.ForLLM, `\x1b[31mred\x1b[0m`) || !strings.Contains(strings.ToLower(result.ForLLM), `\u202e`) {
+		t.Fatalf("expected escaped control sequence in timeout output, got: %q", result.ForLLM)
+	}
+}
+
 func TestShellTool_Background_ReturnsImmediately(t *testing.T) {
 	tool, err := NewExecTool("", false)
 	require.NoError(t, err)