fix (security): ExecTool working_dir sandbox escape (#478)
* fix (security) Shell working_dir bypass * Feedback from @mengzhuo & Discord - reuse internal security package to validate path - add tests for workspace escape
This commit is contained in:
parent
e883e14b81
commit
244eb0b47d
2 changed files with 69 additions and 1 deletions
|
|
@ -144,8 +144,16 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
|
||||||
|
|
||||||
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.workingDir != "" {
|
||||||
|
resolvedWD, err := validatePath(wd, t.workingDir, true)
|
||||||
|
if err != nil {
|
||||||
|
return ErrorResult("Command blocked by safety guard (" + err.Error() + ")")
|
||||||
|
}
|
||||||
|
cwd = resolvedWD
|
||||||
|
} else {
|
||||||
cwd = wd
|
cwd = wd
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if cwd == "" {
|
if cwd == "" {
|
||||||
wd, err := os.Getwd()
|
wd, err := os.Getwd()
|
||||||
|
|
|
||||||
|
|
@ -186,6 +186,66 @@ func TestShellTool_OutputTruncation(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestShellTool_WorkingDir_OutsideWorkspace verifies that working_dir cannot escape the workspace directly
|
||||||
|
func TestShellTool_WorkingDir_OutsideWorkspace(t *testing.T) {
|
||||||
|
root := t.TempDir()
|
||||||
|
workspace := filepath.Join(root, "workspace")
|
||||||
|
outsideDir := filepath.Join(root, "outside")
|
||||||
|
if err := os.MkdirAll(workspace, 0755); err != nil {
|
||||||
|
t.Fatalf("failed to create workspace: %v", err)
|
||||||
|
}
|
||||||
|
if err := os.MkdirAll(outsideDir, 0755); err != nil {
|
||||||
|
t.Fatalf("failed to create outside dir: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
tool := NewExecTool(workspace, true)
|
||||||
|
result := tool.Execute(context.Background(), map[string]interface{}{
|
||||||
|
"command": "pwd",
|
||||||
|
"working_dir": outsideDir,
|
||||||
|
})
|
||||||
|
|
||||||
|
if !result.IsError {
|
||||||
|
t.Fatalf("expected working_dir outside workspace to be blocked, got output: %s", result.ForLLM)
|
||||||
|
}
|
||||||
|
if !strings.Contains(result.ForLLM, "blocked") {
|
||||||
|
t.Errorf("expected 'blocked' in error, got: %s", result.ForLLM)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestShellTool_WorkingDir_SymlinkEscape verifies that a symlink inside the workspace
|
||||||
|
// pointing outside cannot be used as working_dir to escape the sandbox.
|
||||||
|
func TestShellTool_WorkingDir_SymlinkEscape(t *testing.T) {
|
||||||
|
root := t.TempDir()
|
||||||
|
workspace := filepath.Join(root, "workspace")
|
||||||
|
secretDir := filepath.Join(root, "secret")
|
||||||
|
if err := os.MkdirAll(workspace, 0755); err != nil {
|
||||||
|
t.Fatalf("failed to create workspace: %v", err)
|
||||||
|
}
|
||||||
|
if err := os.MkdirAll(secretDir, 0755); err != nil {
|
||||||
|
t.Fatalf("failed to create secret dir: %v", err)
|
||||||
|
}
|
||||||
|
os.WriteFile(filepath.Join(secretDir, "secret.txt"), []byte("top secret"), 0644)
|
||||||
|
|
||||||
|
// symlink lives inside the workspace but resolves to secretDir outside it
|
||||||
|
link := filepath.Join(workspace, "escape")
|
||||||
|
if err := os.Symlink(secretDir, link); err != nil {
|
||||||
|
t.Skipf("symlinks not supported in this environment: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
tool := NewExecTool(workspace, true)
|
||||||
|
result := tool.Execute(context.Background(), map[string]interface{}{
|
||||||
|
"command": "cat secret.txt",
|
||||||
|
"working_dir": link,
|
||||||
|
})
|
||||||
|
|
||||||
|
if !result.IsError {
|
||||||
|
t.Fatalf("expected symlink working_dir 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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestShellTool_RestrictToWorkspace verifies workspace restriction
|
// TestShellTool_RestrictToWorkspace verifies workspace restriction
|
||||||
func TestShellTool_RestrictToWorkspace(t *testing.T) {
|
func TestShellTool_RestrictToWorkspace(t *testing.T) {
|
||||||
tmpDir := t.TempDir()
|
tmpDir := t.TempDir()
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue