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 <noreply@anthropic.com>
This commit is contained in:
parent
5fcc7c8f5a
commit
d90fa21e80
2 changed files with 73 additions and 5 deletions
|
|
@ -5,6 +5,7 @@ import (
|
||||||
"io"
|
"io"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"regexp"
|
||||||
"runtime"
|
"runtime"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
@ -42,8 +43,25 @@ var (
|
||||||
mu sync.RWMutex
|
mu sync.RWMutex
|
||||||
writers []io.Writer
|
writers []io.Writer
|
||||||
consoleWriter zerolog.ConsoleWriter
|
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() {
|
func init() {
|
||||||
once.Do(func() {
|
once.Do(func() {
|
||||||
zerolog.SetGlobalLevel(zerolog.InfoLevel)
|
zerolog.SetGlobalLevel(zerolog.InfoLevel)
|
||||||
|
|
@ -88,13 +106,17 @@ func formatFieldValue(i any) string {
|
||||||
case []byte:
|
case []byte:
|
||||||
s = string(val)
|
s = string(val)
|
||||||
default:
|
default:
|
||||||
return fmt.Sprintf("%v", i)
|
return escapeControlChars(fmt.Sprintf("%v", i))
|
||||||
}
|
}
|
||||||
|
|
||||||
if unquoted, err := strconv.Unquote(s); err == nil {
|
if unquoted, err := strconv.Unquote(s); err == nil {
|
||||||
s = unquoted
|
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") {
|
if strings.Contains(s, "\n") {
|
||||||
return fmt.Sprintf("\n%s", s)
|
return fmt.Sprintf("\n%s", s)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -28,6 +28,23 @@ var (
|
||||||
sessionManagerMu sync.RWMutex
|
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 {
|
func getSessionManager() *SessionManager {
|
||||||
sessionManagerMu.RLock()
|
sessionManagerMu.RLock()
|
||||||
defer sessionManagerMu.RUnlock()
|
defer sessionManagerMu.RUnlock()
|
||||||
|
|
@ -443,6 +460,10 @@ func (t *ExecTool) runSync(ctx context.Context, command, cwd string) *ToolResult
|
||||||
output = "(no output)"
|
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
|
maxLen := 10000
|
||||||
if len(output) > maxLen {
|
if len(output) > maxLen {
|
||||||
output = output[:maxLen] + fmt.Sprintf("\n... (truncated, %d more chars)", 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()
|
output := session.Read()
|
||||||
|
// Escape terminal control and format characters to prevent
|
||||||
|
// malicious output from altering terminal state.
|
||||||
|
output = escapeControlChars(output)
|
||||||
|
|
||||||
resp := ExecResponse{
|
resp := ExecResponse{
|
||||||
SessionID: sessionID,
|
SessionID: sessionID,
|
||||||
|
|
@ -1054,15 +1078,37 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
}
|
}
|
||||||
|
|
||||||
if t.restrictToWorkspace {
|
if t.restrictToWorkspace {
|
||||||
if strings.Contains(cmd, "..\\") || strings.Contains(cmd, "../") {
|
|
||||||
return "Command blocked by safety guard (path traversal detected)"
|
|
||||||
}
|
|
||||||
|
|
||||||
cwdPath, err := filepath.Abs(cwd)
|
cwdPath, err := filepath.Abs(cwd)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return ""
|
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
|
// Web URL schemes whose path components (starting with //) should be exempt
|
||||||
// from workspace sandbox checks. file: is intentionally excluded so that
|
// from workspace sandbox checks. file: is intentionally excluded so that
|
||||||
// file:// URIs are still validated against the workspace boundary.
|
// file:// URIs are still validated against the workspace boundary.
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue