From d90fa21e80906b0056f327680c3ee70604e46781 Mon Sep 17 00:00:00 2001 From: merlinmiao <820962493@qq.com> Date: Thu, 9 Apr 2026 16:53:15 +0800 Subject: [PATCH] fix(exec,logger): escape terminal control chars and refine path guard Prevents command output and logs from altering terminal state via ANSI escape sequences or Unicode bidi format characters. Changes: - Add escapeControlChars helper to replace control characters with \x## hex escapes in pkg/tools/shell.go and pkg/logger/logger.go - Apply escaping to exec tool output (runSync and executeRead) - Apply escaping to logger field formatting (formatFieldValue) - Refine guardCommand path traversal check: instead of blanket- blocking ../, resolve the path first and verify it stays within the working directory. Allows subdir/../file.txt but blocks ../secret.txt that escapes the workspace. Fixes #2377 Co-Authored-By: Claude Opus 4.6 --- pkg/logger/logger.go | 24 +++++++++++++++++++- pkg/tools/shell.go | 54 ++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 6d2e31791..be7bbbd6d 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -5,6 +5,7 @@ import ( "io" "os" "path/filepath" + "regexp" "runtime" "strconv" "strings" @@ -42,8 +43,25 @@ var ( mu sync.RWMutex writers []io.Writer consoleWriter zerolog.ConsoleWriter + + // controlCharPattern matches ANSI escape sequences and Unicode format + // characters that can alter terminal rendering or mislead operators. + controlCharPattern = regexp.MustCompile(`(\x1b\[[0-9;]*[a-zA-Z]|\x1b[()][AB012]|\x1b[>?[0-9]+[a-z]|\x1b\][^\x07]*\x07?|\x1b[^a-zA-Z]*[a-zA-Z]|\u202[aeo]|\u200f|\u200e|\u2066|\u2067|\u2068|\u2069)`) ) +// escapeControlChars replaces terminal control characters and Unicode format +// characters with their safe escaped representation. This prevents log output +// from altering terminal state or misleading operators. +func escapeControlChars(s string) string { + return controlCharPattern.ReplaceAllStringFunc(s, func(match string) string { + var buf strings.Builder + for _, r := range match { + fmt.Fprintf(&buf, "\\x%02x", r) + } + return buf.String() + }) +} + func init() { once.Do(func() { zerolog.SetGlobalLevel(zerolog.InfoLevel) @@ -88,13 +106,17 @@ func formatFieldValue(i any) string { case []byte: s = string(val) default: - return fmt.Sprintf("%v", i) + return escapeControlChars(fmt.Sprintf("%v", i)) } if unquoted, err := strconv.Unquote(s); err == nil { s = unquoted } + // Escape terminal control and format characters to prevent + // log output from altering terminal state or misleading operators. + s = escapeControlChars(s) + if strings.Contains(s, "\n") { return fmt.Sprintf("\n%s", s) } diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index a570ac9ec..290bc0fc4 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -28,6 +28,23 @@ var ( sessionManagerMu sync.RWMutex ) +// controlCharPattern matches ANSI escape sequences and Unicode format characters +// that can alter terminal rendering or be used for malicious purposes. +var controlCharPattern = regexp.MustCompile(`(\x1b\[[0-9;]*[a-zA-Z]|\x1b[()][AB012]|\x1b[>?[0-9]+[a-z-z]|\x1b\][^\x07]*\x07?|\x1b[^a-zA-Z]*[a-zA-Z]|\u202[aeo]|\u200f|\u200e|\u2066|\u2067|\u2068|\u2069)`) + +// escapeControlChars replaces terminal control characters and Unicode format +// characters with their safe escaped representation. This prevents commands +// from altering terminal state or misleading operators. +func escapeControlChars(s string) string { + return controlCharPattern.ReplaceAllStringFunc(s, func(match string) string { + var buf strings.Builder + for _, r := range match { + fmt.Fprintf(&buf, "\\x%02x", r) + } + return buf.String() + }) +} + func getSessionManager() *SessionManager { sessionManagerMu.RLock() defer sessionManagerMu.RUnlock() @@ -443,6 +460,10 @@ func (t *ExecTool) runSync(ctx context.Context, command, cwd string) *ToolResult output = "(no output)" } + // Escape terminal control and format characters to prevent + // command output from altering terminal state or misleading operators. + output = escapeControlChars(output) + maxLen := 10000 if len(output) > maxLen { output = output[:maxLen] + fmt.Sprintf("\n... (truncated, %d more chars)", len(output)-maxLen) @@ -706,6 +727,9 @@ func (t *ExecTool) executeRead(args map[string]any) *ToolResult { } output := session.Read() + // Escape terminal control and format characters to prevent + // malicious output from altering terminal state. + output = escapeControlChars(output) resp := ExecResponse{ SessionID: sessionID, @@ -1054,15 +1078,37 @@ func (t *ExecTool) guardCommand(command, cwd string) string { } if t.restrictToWorkspace { - if strings.Contains(cmd, "..\\") || strings.Contains(cmd, "../") { - return "Command blocked by safety guard (path traversal detected)" - } - cwdPath, err := filepath.Abs(cwd) if err != nil { return "" } + // Check for path traversal patterns that need resolution. + // Instead of blanket-blocking ../, we resolve the path and verify + // it stays within the working directory. + if strings.Contains(cmd, "..\\") || strings.Contains(cmd, "../") { + // Extract path segments containing .. + // Match path-like segments that include .. + pathTraversalPattern := regexp.MustCompile(`(?:[.\w]+/)++\.\.(?:/[.\w]+)*|[.\w]+/+\.\.(?:/[.\w]+)*`) + + traversalIndices := pathTraversalPattern.FindAllStringIndex(cmd, -1) + for _, loc := range traversalIndices { + traversalPath := cmd[loc[0]:loc[1]] + + // Resolve the traversal path relative to cwd + resolved, err := filepath.Abs(filepath.Join(cwdPath, traversalPath)) + if err != nil { + return "Command blocked by safety guard (path traversal detected)" + } + + // Check if resolved path is still within cwd + rel, err := filepath.Rel(cwdPath, resolved) + if err != nil || strings.HasPrefix(rel, "..") { + return "Command blocked by safety guard (path traversal detected)" + } + } + } + // Web URL schemes whose path components (starting with //) should be exempt // from workspace sandbox checks. file: is intentionally excluded so that // file:// URIs are still validated against the workspace boundary.