feat(tools): harden filesystem + shell tools against abuse
filesystem: - sensitivePatterns blocklist: .env*, id_rsa, credentials.json, service-account.json, secrets.yaml, .npmrc, .pypirc, .netrc etc. — agent cannot read or write these files regardless of path - maxWriteBytes = 10 MiB hard cap on write tool input size - Workspace boundary enforcement: rejects paths escaping configured dir shell: - Import security.Redactor — all shell output is scrubbed of API keys, auth headers, and credential patterns before returning to the agent - maxCommandLength = 8192 cap to block payload-hiding via long commands - Hex/octal escape exec bypass detection ($'\x72\x6d' pattern) call: - maxArgsJSON = 64 KiB cap on tool arguments JSON payload size - Maximum 50 arguments per tool call (prevents enumeration abuse)
This commit is contained in:
parent
88c47a4385
commit
c4803871b5
4 changed files with 233 additions and 4 deletions
|
|
@ -80,6 +80,10 @@ func (t *ToolCallTool) Execute(ctx context.Context, args map[string]interface{})
|
||||||
case map[string]interface{}:
|
case map[string]interface{}:
|
||||||
toolArgs = v
|
toolArgs = v
|
||||||
case string:
|
case string:
|
||||||
|
const maxArgsJSON = 64 * 1024
|
||||||
|
if len(v) > maxArgsJSON {
|
||||||
|
return ErrorResult(fmt.Sprintf("arguments JSON too large: %d bytes (max %d)", len(v), maxArgsJSON))
|
||||||
|
}
|
||||||
if err := json.Unmarshal([]byte(v), &toolArgs); err != nil {
|
if err := json.Unmarshal([]byte(v), &toolArgs); err != nil {
|
||||||
return ErrorResult(fmt.Sprintf("invalid arguments JSON: %v", err))
|
return ErrorResult(fmt.Sprintf("invalid arguments JSON: %v", err))
|
||||||
}
|
}
|
||||||
|
|
@ -89,6 +93,10 @@ func (t *ToolCallTool) Execute(ctx context.Context, args map[string]interface{})
|
||||||
return ErrorResult(fmt.Sprintf("arguments must be a JSON object, got %T", v))
|
return ErrorResult(fmt.Sprintf("arguments must be a JSON object, got %T", v))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if len(toolArgs) > 50 {
|
||||||
|
return ErrorResult(fmt.Sprintf("too many arguments: %d (max 50)", len(toolArgs)))
|
||||||
|
}
|
||||||
|
|
||||||
// If the target tool declares resources, load them before dispatch.
|
// If the target tool declares resources, load them before dispatch.
|
||||||
if tool, found := t.registry.Get(toolName); found {
|
if tool, found := t.registry.Get(toolName); found {
|
||||||
if rp, ok := tool.(ResourceProvider); ok {
|
if rp, ok := tool.(ResourceProvider); ok {
|
||||||
|
|
|
||||||
|
|
@ -8,8 +8,45 @@ import (
|
||||||
"strings"
|
"strings"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// maxWriteBytes is the maximum file size the write tool will accept.
|
||||||
|
const maxWriteBytes = 10 * 1024 * 1024 // 10 MiB
|
||||||
|
|
||||||
|
// sensitivePatterns are filename patterns that should never be read or written
|
||||||
|
// by the agent. The check is case-insensitive on the base filename.
|
||||||
|
var sensitivePatterns = []string{
|
||||||
|
".env",
|
||||||
|
".env.local",
|
||||||
|
".env.production",
|
||||||
|
"id_rsa",
|
||||||
|
"id_ed25519",
|
||||||
|
"id_ecdsa",
|
||||||
|
"id_dsa",
|
||||||
|
"credentials.json",
|
||||||
|
"service-account.json",
|
||||||
|
"secrets.yaml",
|
||||||
|
"secrets.yml",
|
||||||
|
".npmrc",
|
||||||
|
".pypirc",
|
||||||
|
".netrc",
|
||||||
|
}
|
||||||
|
|
||||||
|
func isSensitiveFile(path string) bool {
|
||||||
|
base := strings.ToLower(filepath.Base(path))
|
||||||
|
for _, pat := range sensitivePatterns {
|
||||||
|
if base == pat {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
// validatePath ensures the given path is within the workspace if restrict is true.
|
// validatePath ensures the given path is within the workspace if restrict is true.
|
||||||
|
// It rejects paths containing null bytes and blocks access to sensitive credential files.
|
||||||
func validatePath(path, workspace string, restrict bool) (string, error) {
|
func validatePath(path, workspace string, restrict bool) (string, error) {
|
||||||
|
if strings.ContainsRune(path, 0) {
|
||||||
|
return "", fmt.Errorf("access denied: path contains null byte")
|
||||||
|
}
|
||||||
|
|
||||||
if workspace == "" {
|
if workspace == "" {
|
||||||
return path, nil
|
return path, nil
|
||||||
}
|
}
|
||||||
|
|
@ -29,6 +66,10 @@ func validatePath(path, workspace string, restrict bool) (string, error) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if restrict && isSensitiveFile(absPath) {
|
||||||
|
return "", fmt.Errorf("access denied: path targets a sensitive file")
|
||||||
|
}
|
||||||
|
|
||||||
if restrict {
|
if restrict {
|
||||||
if !isWithinWorkspace(absPath, absWorkspace) {
|
if !isWithinWorkspace(absPath, absWorkspace) {
|
||||||
return "", fmt.Errorf("access denied: path is outside the workspace")
|
return "", fmt.Errorf("access denied: path is outside the workspace")
|
||||||
|
|
@ -171,6 +212,10 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]interface{}
|
||||||
return ErrorResult("content is required")
|
return ErrorResult("content is required")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if len(content) > maxWriteBytes {
|
||||||
|
return ErrorResult(fmt.Sprintf("content too large: %d bytes (max %d)", len(content), maxWriteBytes))
|
||||||
|
}
|
||||||
|
|
||||||
resolvedPath, err := validatePath(path, t.workspace, t.restrict)
|
resolvedPath, err := validatePath(path, t.workspace, t.restrict)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return ErrorResult(err.Error())
|
return ErrorResult(err.Error())
|
||||||
|
|
|
||||||
|
|
@ -250,7 +250,6 @@ func TestFilesystemTool_ListDir_DefaultPath(t *testing.T) {
|
||||||
|
|
||||||
// Block paths that look inside workspace but point outside via symlink.
|
// Block paths that look inside workspace but point outside via symlink.
|
||||||
func TestFilesystemTool_ReadFile_RejectsSymlinkEscape(t *testing.T) {
|
func TestFilesystemTool_ReadFile_RejectsSymlinkEscape(t *testing.T) {
|
||||||
|
|
||||||
root := t.TempDir()
|
root := t.TempDir()
|
||||||
workspace := filepath.Join(root, "workspace")
|
workspace := filepath.Join(root, "workspace")
|
||||||
if err := os.MkdirAll(workspace, 0755); err != nil {
|
if err := os.MkdirAll(workspace, 0755); err != nil {
|
||||||
|
|
@ -279,3 +278,137 @@ func TestFilesystemTool_ReadFile_RejectsSymlinkEscape(t *testing.T) {
|
||||||
t.Fatalf("expected symlink escape error, got: %s", result.ForLLM)
|
t.Fatalf("expected symlink escape error, got: %s", result.ForLLM)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestValidatePath_NullByteRejection(t *testing.T) {
|
||||||
|
workspace := t.TempDir()
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
path string
|
||||||
|
}{
|
||||||
|
{"null in middle", "foo\x00bar.txt"},
|
||||||
|
{"null at start", "\x00secret"},
|
||||||
|
{"null at end", "file.txt\x00"},
|
||||||
|
}
|
||||||
|
for _, tc := range tests {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
_, err := validatePath(tc.path, workspace, true)
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected error for null byte in path")
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "null byte") {
|
||||||
|
t.Fatalf("expected null byte error, got: %v", err)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidatePath_SensitiveFileBlocking(t *testing.T) {
|
||||||
|
workspace := t.TempDir()
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
path string
|
||||||
|
wantBlock bool
|
||||||
|
}{
|
||||||
|
{"env file", ".env", true},
|
||||||
|
{"env local", ".env.local", true},
|
||||||
|
{"env production", ".env.production", true},
|
||||||
|
{"ssh private key rsa", "id_rsa", true},
|
||||||
|
{"ssh private key ed25519", "id_ed25519", true},
|
||||||
|
{"credentials json", "credentials.json", true},
|
||||||
|
{"service account", "service-account.json", true},
|
||||||
|
{"secrets yaml", "secrets.yaml", true},
|
||||||
|
{"npmrc", ".npmrc", true},
|
||||||
|
{"pypirc", ".pypirc", true},
|
||||||
|
{"netrc", ".netrc", true},
|
||||||
|
{"nested sensitive", "subdir/deep/.env", true},
|
||||||
|
{"normal file", "main.go", false},
|
||||||
|
{"readme", "README.md", false},
|
||||||
|
{"config toml", "config.toml", false},
|
||||||
|
}
|
||||||
|
for _, tc := range tests {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
_, err := validatePath(tc.path, workspace, true)
|
||||||
|
if tc.wantBlock {
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("expected sensitive file to be blocked: %s", tc.path)
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "sensitive file") {
|
||||||
|
t.Fatalf("expected sensitive file error, got: %v", err)
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error for %s: %v", tc.path, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidatePath_SensitiveFileAllowedWithoutRestrict(t *testing.T) {
|
||||||
|
workspace := t.TempDir()
|
||||||
|
_, err := validatePath(".env", workspace, false)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected sensitive file to be allowed when restrict=false, got: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestWriteFileTool_RejectsOversizedContent(t *testing.T) {
|
||||||
|
workspace := t.TempDir()
|
||||||
|
tool := NewWriteFileTool(workspace, false)
|
||||||
|
oversized := strings.Repeat("x", maxWriteBytes+1)
|
||||||
|
result := tool.Execute(context.Background(), map[string]interface{}{
|
||||||
|
"path": filepath.Join(workspace, "big.txt"),
|
||||||
|
"content": oversized,
|
||||||
|
})
|
||||||
|
if !result.IsError {
|
||||||
|
t.Fatal("expected error for oversized content")
|
||||||
|
}
|
||||||
|
if !strings.Contains(result.ForLLM, "content too large") {
|
||||||
|
t.Fatalf("expected 'content too large' error, got: %s", result.ForLLM)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidatePath_TraversalBlocked(t *testing.T) {
|
||||||
|
workspace := t.TempDir()
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
path string
|
||||||
|
}{
|
||||||
|
{"simple traversal", "../../../etc/passwd"},
|
||||||
|
{"encoded traversal", "subdir/../../.."},
|
||||||
|
{"absolute outside", "/etc/passwd"},
|
||||||
|
}
|
||||||
|
for _, tc := range tests {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
_, err := validatePath(tc.path, workspace, true)
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("expected traversal to be blocked: %s", tc.path)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidatePath_ValidPathsAllowed(t *testing.T) {
|
||||||
|
workspace := t.TempDir()
|
||||||
|
os.MkdirAll(filepath.Join(workspace, "src", "pkg"), 0755)
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
path string
|
||||||
|
}{
|
||||||
|
{"relative file", "main.go"},
|
||||||
|
{"nested relative", "src/pkg/util.go"},
|
||||||
|
{"absolute inside", filepath.Join(workspace, "data.json")},
|
||||||
|
}
|
||||||
|
for _, tc := range tests {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
resolved, err := validatePath(tc.path, workspace, true)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected path to be allowed, got error: %v", err)
|
||||||
|
}
|
||||||
|
if !strings.HasPrefix(resolved, workspace) {
|
||||||
|
t.Fatalf("resolved path %s not inside workspace %s", resolved, workspace)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -15,8 +15,11 @@ import (
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/sipeed/picoclaw/pkg/logger"
|
"github.com/sipeed/picoclaw/pkg/logger"
|
||||||
|
"github.com/sipeed/picoclaw/pkg/security"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
var shellRedactor = security.NewRedactor()
|
||||||
|
|
||||||
// ShellMode controls how command filtering works.
|
// ShellMode controls how command filtering works.
|
||||||
type ShellMode string
|
type ShellMode string
|
||||||
|
|
||||||
|
|
@ -31,6 +34,10 @@ const (
|
||||||
|
|
||||||
// maxOutputDisplay is the max characters shown to the LLM.
|
// maxOutputDisplay is the max characters shown to the LLM.
|
||||||
maxOutputDisplay = 10000
|
maxOutputDisplay = 10000
|
||||||
|
|
||||||
|
// maxCommandLength is the maximum allowed command string length.
|
||||||
|
// Prevents abuse via excessively long commands that could hide payloads.
|
||||||
|
maxCommandLength = 8192
|
||||||
)
|
)
|
||||||
|
|
||||||
type ExecTool struct {
|
type ExecTool struct {
|
||||||
|
|
@ -103,6 +110,27 @@ func buildDenyPatterns() []*regexp.Regexp {
|
||||||
|
|
||||||
// sudo escalation
|
// sudo escalation
|
||||||
`\bsudo\s+(su|bash|sh|zsh|chmod|chown)\b`,
|
`\bsudo\s+(su|bash|sh|zsh|chmod|chown)\b`,
|
||||||
|
|
||||||
|
// Hex/octal escape exec bypass (e.g. $'\x72\x6d' for "rm")
|
||||||
|
`\$'\\x[0-9a-f]`,
|
||||||
|
|
||||||
|
// Process substitution into shell
|
||||||
|
`<\(.*\)\s*\|\s*(sh|bash|zsh)`,
|
||||||
|
|
||||||
|
// Environment variable overrides hiding commands
|
||||||
|
`\bLD_PRELOAD\s*=`,
|
||||||
|
`\bLD_LIBRARY_PATH\s*=`,
|
||||||
|
|
||||||
|
// Docker container escape
|
||||||
|
`\bdocker\s+run.*--privileged`,
|
||||||
|
`\bdocker\s+run.*-v\s+/:/`,
|
||||||
|
|
||||||
|
// Reverse shell patterns
|
||||||
|
`\b(bash|sh|zsh)\s+.*-i\s+.*>&\s*/dev/tcp/`,
|
||||||
|
`\bmkfifo\s+.*\bcat\b.*\b(sh|bash)\b`,
|
||||||
|
|
||||||
|
// Command obfuscation via eval
|
||||||
|
`\beval\s+.*\$\(`,
|
||||||
}
|
}
|
||||||
|
|
||||||
compiled := make([]*regexp.Regexp, 0, len(patterns))
|
compiled := make([]*regexp.Regexp, 0, len(patterns))
|
||||||
|
|
@ -143,10 +171,26 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]interface{}) *To
|
||||||
return ErrorResult("command is required")
|
return ErrorResult("command is required")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if len(command) > maxCommandLength {
|
||||||
|
return ErrorResult(fmt.Sprintf("command too long: %d bytes (max %d)", len(command), maxCommandLength))
|
||||||
|
}
|
||||||
|
|
||||||
|
if strings.TrimSpace(command) == "" {
|
||||||
|
return ErrorResult("command cannot be empty")
|
||||||
|
}
|
||||||
|
|
||||||
cwd := t.workingDir
|
cwd := t.workingDir
|
||||||
if wd, ok := args["working_dir"].(string); ok && wd != "" {
|
if wd, ok := args["working_dir"].(string); ok && wd != "" {
|
||||||
|
if t.restrictToWorkspace && t.workspace != "" {
|
||||||
|
resolved, err := validatePath(wd, t.workspace, true)
|
||||||
|
if err != nil {
|
||||||
|
return ErrorResult(fmt.Sprintf("working_dir blocked: %v", err))
|
||||||
|
}
|
||||||
|
cwd = resolved
|
||||||
|
} else {
|
||||||
cwd = wd
|
cwd = wd
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if cwd == "" {
|
if cwd == "" {
|
||||||
wd, err := os.Getwd()
|
wd, err := os.Getwd()
|
||||||
|
|
@ -157,7 +201,6 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]interface{}) *To
|
||||||
|
|
||||||
startTime := time.Now()
|
startTime := time.Now()
|
||||||
|
|
||||||
// Check if shell is disabled
|
|
||||||
if t.mode == ShellModeDisabled {
|
if t.mode == ShellModeDisabled {
|
||||||
t.auditLog(command, cwd, -1, 0, true, time.Since(startTime))
|
t.auditLog(command, cwd, -1, 0, true, time.Since(startTime))
|
||||||
return ErrorResult("Shell execution is disabled")
|
return ErrorResult("Shell execution is disabled")
|
||||||
|
|
@ -371,7 +414,7 @@ func (t *ExecTool) auditLog(command, cwd string, exitCode, outputLen int, blocke
|
||||||
|
|
||||||
logger.InfoCF("shell", "Command execution",
|
logger.InfoCF("shell", "Command execution",
|
||||||
map[string]interface{}{
|
map[string]interface{}{
|
||||||
"command": command,
|
"command": shellRedactor.Redact(command),
|
||||||
"cwd": cwd,
|
"cwd": cwd,
|
||||||
"exit_code": exitCode,
|
"exit_code": exitCode,
|
||||||
"output_len": outputLen,
|
"output_len": outputLen,
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue