fix(tools): block symlink operand escapes in exec
This commit is contained in:
parent
0f700a6bf0
commit
b7532c99fe
2 changed files with 184 additions and 0 deletions
|
|
@ -12,6 +12,7 @@ import (
|
||||||
"runtime"
|
"runtime"
|
||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
"unicode"
|
||||||
|
|
||||||
"github.com/sipeed/picoclaw/pkg/config"
|
"github.com/sipeed/picoclaw/pkg/config"
|
||||||
"github.com/sipeed/picoclaw/pkg/constants"
|
"github.com/sipeed/picoclaw/pkg/constants"
|
||||||
|
|
@ -80,6 +81,7 @@ var (
|
||||||
|
|
||||||
// absolutePathPattern matches absolute file paths in commands (Unix and Windows).
|
// absolutePathPattern matches absolute file paths in commands (Unix and Windows).
|
||||||
absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`)
|
absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`)
|
||||||
|
leadingRedirection = regexp.MustCompile(`^[0-9]*[<>]+`)
|
||||||
|
|
||||||
// safePaths are kernel pseudo-devices that are always safe to reference in
|
// safePaths are kernel pseudo-devices that are always safe to reference in
|
||||||
// commands, regardless of workspace restriction. They contain no user data
|
// commands, regardless of workspace restriction. They contain no user data
|
||||||
|
|
@ -383,6 +385,10 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
for _, loc := range matchIndices {
|
for _, loc := range matchIndices {
|
||||||
raw := cmd[loc[0]:loc[1]]
|
raw := cmd[loc[0]:loc[1]]
|
||||||
|
|
||||||
|
if !isPathTokenStart(cmd, loc[0]) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
// Skip URL path components that look like they're from web URLs.
|
// Skip URL path components that look like they're from web URLs.
|
||||||
// When a URL like "https://github.com" is parsed, the regex captures
|
// When a URL like "https://github.com" is parsed, the regex captures
|
||||||
// "//github.com" as a match (the path portion after "https:").
|
// "//github.com" as a match (the path portion after "https:").
|
||||||
|
|
@ -422,11 +428,126 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
return "Command blocked by safety guard (path outside working dir)"
|
return "Command blocked by safety guard (path outside working dir)"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
for _, token := range shellWords(cmd) {
|
||||||
|
candidate := leadingRedirection.ReplaceAllString(token, "")
|
||||||
|
if candidate == "" || filepath.IsAbs(candidate) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if strings.Contains(candidate, "://") {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if !looksLikeRelativePathOperand(candidate) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if err := validateRelativeOperandPath(candidate, cwdPath, t.workingDir); err != nil {
|
||||||
|
return "Command blocked by safety guard (" + err.Error() + ")"
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func shellWords(command string) []string {
|
||||||
|
var (
|
||||||
|
words []string
|
||||||
|
current strings.Builder
|
||||||
|
inSingle bool
|
||||||
|
inDouble bool
|
||||||
|
escaped bool
|
||||||
|
)
|
||||||
|
|
||||||
|
flush := func() {
|
||||||
|
if current.Len() == 0 {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
words = append(words, current.String())
|
||||||
|
current.Reset()
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, r := range command {
|
||||||
|
switch {
|
||||||
|
case escaped:
|
||||||
|
current.WriteRune(r)
|
||||||
|
escaped = false
|
||||||
|
case r == '\\' && !inSingle:
|
||||||
|
escaped = true
|
||||||
|
case r == '\'' && !inDouble:
|
||||||
|
inSingle = !inSingle
|
||||||
|
case r == '"' && !inSingle:
|
||||||
|
inDouble = !inDouble
|
||||||
|
case unicode.IsSpace(r) && !inSingle && !inDouble:
|
||||||
|
flush()
|
||||||
|
default:
|
||||||
|
current.WriteRune(r)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
flush()
|
||||||
|
return words
|
||||||
|
}
|
||||||
|
|
||||||
|
func isPathTokenStart(command string, start int) bool {
|
||||||
|
if start <= 0 {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
switch command[start-1] {
|
||||||
|
case ' ', '\t', '\n', '\r', '"', '\'', '=', ':', '(', ')', '[', ']', '{', '}', '|', '&', ';', '<', '>':
|
||||||
|
return true
|
||||||
|
default:
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func looksLikeRelativePathOperand(token string) bool {
|
||||||
|
if token == "" || token == "." || token == ".." {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
return strings.Contains(token, "/") || strings.Contains(token, "\\") ||
|
||||||
|
strings.HasPrefix(token, ".")
|
||||||
|
}
|
||||||
|
|
||||||
|
func validateRelativeOperandPath(pathToken, cwd, workspace string) error {
|
||||||
|
workspacePath, err := filepath.Abs(workspace)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to resolve workspace path: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
absPath, err := filepath.Abs(filepath.Join(cwd, pathToken))
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to resolve file path: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if !isWithinWorkspace(absPath, workspacePath) {
|
||||||
|
return fmt.Errorf("path outside working dir")
|
||||||
|
}
|
||||||
|
|
||||||
|
workspaceReal := workspacePath
|
||||||
|
if resolved, err := filepath.EvalSymlinks(workspacePath); err == nil {
|
||||||
|
workspaceReal = resolved
|
||||||
|
}
|
||||||
|
|
||||||
|
if resolved, err := filepath.EvalSymlinks(absPath); err == nil {
|
||||||
|
if !isWithinWorkspace(resolved, workspaceReal) {
|
||||||
|
return fmt.Errorf("symlink resolves outside workspace")
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
} else if os.IsNotExist(err) {
|
||||||
|
parentResolved, parentErr := resolveExistingAncestor(filepath.Dir(absPath))
|
||||||
|
if parentErr == nil && !isWithinWorkspace(parentResolved, workspaceReal) {
|
||||||
|
return fmt.Errorf("symlink resolves outside workspace")
|
||||||
|
}
|
||||||
|
if parentErr != nil && !os.IsNotExist(parentErr) {
|
||||||
|
return fmt.Errorf("failed to resolve path: %w", parentErr)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
} else {
|
||||||
|
return fmt.Errorf("failed to resolve path: %w", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func (t *ExecTool) SetTimeout(timeout time.Duration) {
|
func (t *ExecTool) SetTimeout(timeout time.Duration) {
|
||||||
t.timeout = timeout
|
t.timeout = timeout
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -301,6 +301,69 @@ func TestShellTool_WorkingDir_SymlinkEscape(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestShellTool_RelativeOperand_SymlinkEscape(t *testing.T) {
|
||||||
|
root := t.TempDir()
|
||||||
|
workspace := filepath.Join(root, "workspace")
|
||||||
|
outside := filepath.Join(root, "outside")
|
||||||
|
if err := os.MkdirAll(workspace, 0o755); err != nil {
|
||||||
|
t.Fatalf("failed to create workspace: %v", err)
|
||||||
|
}
|
||||||
|
if err := os.MkdirAll(outside, 0o755); err != nil {
|
||||||
|
t.Fatalf("failed to create outside dir: %v", err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("top secret"), 0o600); err != nil {
|
||||||
|
t.Fatalf("failed to create outside secret: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
link := filepath.Join(workspace, "leak")
|
||||||
|
if err := os.Symlink(outside, link); err != nil {
|
||||||
|
t.Skipf("symlinks not supported in this environment: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
tool, err := NewExecTool(workspace, true)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unable to configure exec tool: %s", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
|
"command": "cat leak/secret.txt",
|
||||||
|
})
|
||||||
|
|
||||||
|
if !result.IsError {
|
||||||
|
t.Fatalf("expected symlink operand escape to be blocked, got output: %s", result.ForLLM)
|
||||||
|
}
|
||||||
|
if !strings.Contains(result.ForLLM, "blocked") {
|
||||||
|
t.Errorf("expected 'blocked' in error, got: %s", result.ForLLM)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestShellTool_RelativeOperand_InsideWorkspaceAllowed(t *testing.T) {
|
||||||
|
workspace := t.TempDir()
|
||||||
|
dir := filepath.Join(workspace, "docs")
|
||||||
|
if err := os.MkdirAll(dir, 0o755); err != nil {
|
||||||
|
t.Fatalf("failed to create docs dir: %v", err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(filepath.Join(dir, "note.txt"), []byte("hello"), 0o600); err != nil {
|
||||||
|
t.Fatalf("failed to create workspace file: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
tool, err := NewExecTool(workspace, true)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unable to configure exec tool: %s", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
|
"command": "cat docs/note.txt",
|
||||||
|
})
|
||||||
|
|
||||||
|
if result.IsError {
|
||||||
|
t.Fatalf("expected in-workspace relative operand to be allowed, got: %s", result.ForLLM)
|
||||||
|
}
|
||||||
|
if !strings.Contains(result.ForLLM, "hello") {
|
||||||
|
t.Errorf("expected file contents in output, got: %s", result.ForLLM)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestShellTool_RemoteChannelBlockedByDefault verifies exec is blocked for remote channels
|
// TestShellTool_RemoteChannelBlockedByDefault verifies exec is blocked for remote channels
|
||||||
func TestShellTool_RemoteChannelBlockedByDefault(t *testing.T) {
|
func TestShellTool_RemoteChannelBlockedByDefault(t *testing.T) {
|
||||||
cfg := &config.Config{}
|
cfg := &config.Config{}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue