fix(security): harden sandbox — shell escape bypass, symlink TOCTOU, SSRF, working_dir validation

.gitignore

@@ -41,6 +41,7 @@ tasks/
 # Editors
 .vscode/
 .idea/
+.claude/
 
 # Added by goreleaser init:
 dist/

README.md

@@ -525,6 +525,62 @@ Even with `restrict_to_workspace: false`, the `exec` tool blocks these dangerous
 * `shutdown`, `reboot`, `poweroff` — System shutdown
 * Fork bomb `:(){ :|:& };:`
 
+#### Shell Escape Sequence Protection
+
+When `restrict_to_workspace: true`, the `exec` tool also blocks shell escape sequences that can bypass metacharacter detection:
+
+| Pattern | Example | Risk |
+|---------|---------|------|
+| ANSI-C quoting `$'...'` | `$'\x24(id)'` | Embeds command substitution via hex escape |
+| Locale quoting `$"..."` | `$"$(cmd)"` | Alternative command substitution syntax |
+| Hex escapes `\xNN` | `\x24(id)` | Encodes `$` as `\x24` to bypass `$()` check |
+| Octal escapes `\NNN` | `\060` | Encodes characters via octal to bypass checks |
+| Escaped metacharacters | `` \` ``, `\$` | Bypasses backtick and dollar sign detection |
+
+#### Working Directory Validation
+
+When `restrict_to_workspace: true`, the `exec` tool validates the `working_dir` parameter to ensure it stays within the configured workspace. Passing `working_dir` pointing outside the workspace (e.g. `/etc`) is blocked:
+
+```
+Command blocked by safety guard (working directory outside workspace)
+```
+
+#### Symlink TOCTOU Protection
+
+All file tools (`read_file`, `write_file`, `edit_file`, `append_file`) re-verify symlink targets immediately before the actual I/O operation. This closes the time-of-check-to-time-of-use (TOCTOU) window where an attacker could swap a symlink between the initial `validatePath()` check and the subsequent file operation:
+
+1. **Check**: `validatePath()` resolves the symlink and verifies the target is inside the workspace
+2. **Re-check**: `safeReadFile` / `safeWriteFile` / `safeOpenFile` calls `Lstat` right before I/O — if the path is a symlink, it re-resolves and re-validates the target
+3. **Operate**: The file operation uses the resolved path
+
+If the symlink target has changed to a path outside the workspace between steps 1 and 2, the operation is denied.
+
+#### SSRF Protection (Web Fetch)
+
+The `web_fetch` tool blocks requests to internal and private network addresses to prevent Server-Side Request Forgery (SSRF) attacks. This protects against unauthorized access to cloud metadata endpoints (e.g. `169.254.169.254`), internal services, and local resources.
+
+| Blocked Range | Description |
+|---------------|-------------|
+| `127.0.0.0/8`, `::1` | Loopback addresses |
+| `0.0.0.0` | Unspecified address |
+| `169.254.0.0/16` | Link-local (cloud metadata) |
+| `10.0.0.0/8` | Private network (RFC 1918) |
+| `172.16.0.0/12` | Private network (RFC 1918) |
+| `192.168.0.0/16` | Private network (RFC 1918) |
+| `fc00::/7` | IPv6 unique local addresses |
+
+Hostnames are resolved before checking, so DNS rebinding to internal IPs is also blocked.
+
+#### TLS Warning for API Providers
+
+When an LLM provider is configured with a plain `http://` API base URL (not `localhost` or `127.0.0.1`), PicoClaw logs a warning:
+
+```
+[WARN] [provider] API base uses plain HTTP — API keys may be transmitted without encryption
+```
+
+This helps prevent accidental credential exposure over unencrypted connections.
+
 #### Error Examples
 
 ```

pkg/channels/base.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/sipeed/picoclaw/pkg/bus"
+	"github.com/sipeed/picoclaw/pkg/logger"
 )
 
 type Channel interface {
@@ -26,6 +27,11 @@ type BaseChannel struct {
 }
 
 func NewBaseChannel(name string, config interface{}, bus *bus.MessageBus, allowList []string) *BaseChannel {
+	if len(allowList) == 0 {
+		logger.WarnCF("channel", "Channel has empty allow_from: all messages will be rejected until configured", map[string]interface{}{
+			"channel": name,
+		})
+	}
 	return &BaseChannel{
 		config:    config,
 		bus:       bus,
@@ -45,7 +51,7 @@ func (c *BaseChannel) IsRunning() bool {
 
 func (c *BaseChannel) IsAllowed(senderID string) bool {
 	if len(c.allowList) == 0 {
-		return true
+		return false
 	}
 
 	// Extract parts from compound senderID like "123456|username"
@@ -84,6 +90,11 @@ func (c *BaseChannel) IsAllowed(senderID string) bool {
 
 func (c *BaseChannel) HandleMessage(senderID, chatID, content string, media []string, metadata map[string]string) {
 	if !c.IsAllowed(senderID) {
+		logger.WarnCF("channel", "Message rejected: sender not in allow_from list", map[string]interface{}{
+			"channel":   c.name,
+			"sender_id": senderID,
+			"chat_id":   chatID,
+		})
 		return
 	}
 

pkg/channels/base_test.go

@@ -10,10 +10,10 @@ func TestBaseChannelIsAllowed(t *testing.T) {
 		want      bool
 	}{
 		{
-			name:      "empty allowlist allows all",
+			name:      "empty allowlist denies all",
 			allowList: nil,
 			senderID:  "anyone",
-			want:      true,
+			want:      false,
 		},
 		{
 			name:      "compound sender matches numeric allowlist",

pkg/channels/slack_test.go

@@ -145,15 +145,15 @@ func TestNewSlackChannel(t *testing.T) {
 func TestSlackChannelIsAllowed(t *testing.T) {
 	msgBus := bus.NewMessageBus()
 
-	t.Run("empty allowlist allows all", func(t *testing.T) {
+	t.Run("empty allowlist denies all", func(t *testing.T) {
 		cfg := config.SlackConfig{
 			BotToken:  "xoxb-test",
 			AppToken:  "xapp-test",
 			AllowFrom: []string{},
 		}
 		ch, _ := NewSlackChannel(cfg, msgBus)
-		if !ch.IsAllowed("U_ANYONE") {
-			t.Error("empty allowlist should allow all users")
+		if ch.IsAllowed("U_ANYONE") {
+			t.Error("empty allowlist should deny all users by default")
 		}
 	})
 

pkg/providers/http_provider.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/sipeed/picoclaw/pkg/auth"
 	"github.com/sipeed/picoclaw/pkg/config"
+	"github.com/sipeed/picoclaw/pkg/logger"
 )
 
 type HTTPProvider struct {
@@ -41,6 +42,14 @@ func NewHTTPProvider(apiKey, apiBase, proxy string) *HTTPProvider {
 		}
 	}
 
+	if strings.HasPrefix(apiBase, "http://") &&
+		!strings.Contains(apiBase, "localhost") &&
+		!strings.Contains(apiBase, "127.0.0.1") {
+		logger.WarnCF("provider", "API base uses plain HTTP — API keys may be transmitted without encryption", map[string]interface{}{
+			"api_base": apiBase,
+		})
+	}
+
 	return &HTTPProvider{
 		apiKey:     apiKey,
 		apiBase:    strings.TrimRight(apiBase, "/"),

pkg/tools/edit.go

@@ -76,7 +76,7 @@ func (t *EditFileTool) Execute(ctx context.Context, args map[string]interface{})
 		return ErrorResult(fmt.Sprintf("file not found: %s", path))
 	}
 
-	content, err := os.ReadFile(resolvedPath)
+	content, err := safeReadFile(resolvedPath, t.allowedDir, t.restrict)
 	if err != nil {
 		return ErrorResult(fmt.Sprintf("failed to read file: %v", err))
 	}
@@ -94,7 +94,7 @@ func (t *EditFileTool) Execute(ctx context.Context, args map[string]interface{})
 
 	newContent := strings.Replace(contentStr, oldText, newText, 1)
 
-	if err := os.WriteFile(resolvedPath, []byte(newContent), 0644); err != nil {
+	if err := safeWriteFile(resolvedPath, []byte(newContent), 0644, t.allowedDir, t.restrict); err != nil {
 		return ErrorResult(fmt.Sprintf("failed to write file: %v", err))
 	}
 
@@ -151,7 +151,7 @@ func (t *AppendFileTool) Execute(ctx context.Context, args map[string]interface{
 		return ErrorResult(err.Error())
 	}
 
-	f, err := os.OpenFile(resolvedPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
+	f, err := safeOpenFile(resolvedPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644, t.workspace, t.restrict)
 	if err != nil {
 		return ErrorResult(fmt.Sprintf("failed to open file: %v", err))
 	}

pkg/tools/filesystem.go

@@ -77,6 +77,71 @@ func isWithinWorkspace(candidate, workspace string) bool {
 	return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator))
 }
 
+// recheckSymlink verifies that path does not resolve outside workspace via symlink.
+// This is called right before the actual I/O operation to close the TOCTOU window
+// between validatePath and the file operation.
+func recheckSymlink(path, workspace string, restrict bool) (string, error) {
+	if !restrict || workspace == "" {
+		return path, nil
+	}
+
+	info, err := os.Lstat(path)
+	if err != nil {
+		// File doesn't exist yet (e.g. new file write) — nothing to recheck
+		if os.IsNotExist(err) {
+			return path, nil
+		}
+		return "", fmt.Errorf("failed to stat path: %w", err)
+	}
+
+	if info.Mode()&os.ModeSymlink != 0 {
+		resolved, err := filepath.EvalSymlinks(path)
+		if err != nil {
+			return "", fmt.Errorf("failed to resolve symlink: %w", err)
+		}
+		absWorkspace, err := filepath.Abs(workspace)
+		if err != nil {
+			return "", fmt.Errorf("failed to resolve workspace: %w", err)
+		}
+		if wsResolved, err := filepath.EvalSymlinks(absWorkspace); err == nil {
+			absWorkspace = wsResolved
+		}
+		if !isWithinWorkspace(resolved, absWorkspace) {
+			return "", fmt.Errorf("access denied: symlink resolves outside workspace")
+		}
+		return resolved, nil
+	}
+
+	return path, nil
+}
+
+// safeReadFile re-checks symlinks right before reading to prevent TOCTOU attacks.
+func safeReadFile(path, workspace string, restrict bool) ([]byte, error) {
+	resolved, err := recheckSymlink(path, workspace, restrict)
+	if err != nil {
+		return nil, err
+	}
+	return os.ReadFile(resolved)
+}
+
+// safeWriteFile re-checks symlinks right before writing to prevent TOCTOU attacks.
+func safeWriteFile(path string, data []byte, perm os.FileMode, workspace string, restrict bool) error {
+	resolved, err := recheckSymlink(path, workspace, restrict)
+	if err != nil {
+		return err
+	}
+	return os.WriteFile(resolved, data, perm)
+}
+
+// safeOpenFile re-checks symlinks right before opening to prevent TOCTOU attacks.
+func safeOpenFile(path string, flag int, perm os.FileMode, workspace string, restrict bool) (*os.File, error) {
+	resolved, err := recheckSymlink(path, workspace, restrict)
+	if err != nil {
+		return nil, err
+	}
+	return os.OpenFile(resolved, flag, perm)
+}
+
 type ReadFileTool struct {
 	workspace string
 	restrict  bool
@@ -118,7 +183,7 @@ func (t *ReadFileTool) Execute(ctx context.Context, args map[string]interface{})
 		return ErrorResult(err.Error())
 	}
 
-	content, err := os.ReadFile(resolvedPath)
+	content, err := safeReadFile(resolvedPath, t.workspace, t.restrict)
 	if err != nil {
 		return ErrorResult(fmt.Sprintf("failed to read file: %v", err))
 	}
@@ -181,7 +246,7 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]interface{}
 		return ErrorResult(fmt.Sprintf("failed to create directory: %v", err))
 	}
 
-	if err := os.WriteFile(resolvedPath, []byte(content), 0644); err != nil {
+	if err := safeWriteFile(resolvedPath, []byte(content), 0644, t.workspace, t.restrict); err != nil {
 		return ErrorResult(fmt.Sprintf("failed to write file: %v", err))
 	}
 

pkg/tools/filesystem_test.go

@@ -279,3 +279,115 @@ func TestFilesystemTool_ReadFile_RejectsSymlinkEscape(t *testing.T) {
 		t.Fatalf("expected symlink escape error, got: %s", result.ForLLM)
 	}
 }
+
+// TestFilesystemTool_WriteFile_RejectsSymlinkEscape verifies that writing via a symlink
+// that points outside the workspace is blocked (TOCTOU protection).
+func TestFilesystemTool_WriteFile_RejectsSymlinkEscape(t *testing.T) {
+	root := t.TempDir()
+	workspace := filepath.Join(root, "workspace")
+	if err := os.MkdirAll(workspace, 0755); err != nil {
+		t.Fatalf("failed to create workspace: %v", err)
+	}
+
+	target := filepath.Join(root, "outside.txt")
+	if err := os.WriteFile(target, []byte("original"), 0644); err != nil {
+		t.Fatalf("failed to write target file: %v", err)
+	}
+
+	link := filepath.Join(workspace, "link.txt")
+	if err := os.Symlink(target, link); err != nil {
+		t.Skipf("symlink not supported in this environment: %v", err)
+	}
+
+	tool := NewWriteFileTool(workspace, true)
+	result := tool.Execute(context.Background(), map[string]interface{}{
+		"path":    link,
+		"content": "hacked",
+	})
+
+	if !result.IsError {
+		t.Fatalf("expected symlink escape to be blocked for write")
+	}
+	if !strings.Contains(result.ForLLM, "symlink resolves outside workspace") {
+		t.Fatalf("expected symlink escape error, got: %s", result.ForLLM)
+	}
+
+	// Verify original content was not overwritten
+	content, _ := os.ReadFile(target)
+	if string(content) != "original" {
+		t.Fatalf("expected original content to be preserved, got: %s", string(content))
+	}
+}
+
+// TestFilesystemTool_EditFile_RejectsSymlinkEscape verifies that editing via a symlink
+// that points outside the workspace is blocked (TOCTOU protection).
+func TestFilesystemTool_EditFile_RejectsSymlinkEscape(t *testing.T) {
+	root := t.TempDir()
+	workspace := filepath.Join(root, "workspace")
+	if err := os.MkdirAll(workspace, 0755); err != nil {
+		t.Fatalf("failed to create workspace: %v", err)
+	}
+
+	target := filepath.Join(root, "outside.txt")
+	if err := os.WriteFile(target, []byte("original content"), 0644); err != nil {
+		t.Fatalf("failed to write target file: %v", err)
+	}
+
+	link := filepath.Join(workspace, "link.txt")
+	if err := os.Symlink(target, link); err != nil {
+		t.Skipf("symlink not supported in this environment: %v", err)
+	}
+
+	tool := NewEditFileTool(workspace, true)
+	result := tool.Execute(context.Background(), map[string]interface{}{
+		"path":     link,
+		"old_text": "original",
+		"new_text": "hacked",
+	})
+
+	if !result.IsError {
+		t.Fatalf("expected symlink escape to be blocked for edit")
+	}
+
+	// Verify original content was not modified
+	content, _ := os.ReadFile(target)
+	if string(content) != "original content" {
+		t.Fatalf("expected original content to be preserved, got: %s", string(content))
+	}
+}
+
+// TestFilesystemTool_AppendFile_RejectsSymlinkEscape verifies that appending via a symlink
+// that points outside the workspace is blocked (TOCTOU protection).
+func TestFilesystemTool_AppendFile_RejectsSymlinkEscape(t *testing.T) {
+	root := t.TempDir()
+	workspace := filepath.Join(root, "workspace")
+	if err := os.MkdirAll(workspace, 0755); err != nil {
+		t.Fatalf("failed to create workspace: %v", err)
+	}
+
+	target := filepath.Join(root, "outside.txt")
+	if err := os.WriteFile(target, []byte("original"), 0644); err != nil {
+		t.Fatalf("failed to write target file: %v", err)
+	}
+
+	link := filepath.Join(workspace, "link.txt")
+	if err := os.Symlink(target, link); err != nil {
+		t.Skipf("symlink not supported in this environment: %v", err)
+	}
+
+	tool := NewAppendFileTool(workspace, true)
+	result := tool.Execute(context.Background(), map[string]interface{}{
+		"path":    link,
+		"content": "appended",
+	})
+
+	if !result.IsError {
+		t.Fatalf("expected symlink escape to be blocked for append")
+	}
+
+	// Verify original content was not modified
+	content, _ := os.ReadFile(target)
+	if string(content) != "original" {
+		t.Fatalf("expected original content to be preserved, got: %s", string(content))
+	}
+}