Skip to content

Commit 244eb0b

Browse files
authored
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
1 parent e883e14 commit 244eb0b

File tree

2 files changed

+69
-1
lines changed

2 files changed

+69
-1
lines changed

pkg/tools/shell.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,15 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
144144

145145
cwd := t.workingDir
146146
if wd, ok := args["working_dir"].(string); ok && wd != "" {
147-
cwd = wd
147+
if t.restrictToWorkspace && t.workingDir != "" {
148+
resolvedWD, err := validatePath(wd, t.workingDir, true)
149+
if err != nil {
150+
return ErrorResult("Command blocked by safety guard (" + err.Error() + ")")
151+
}
152+
cwd = resolvedWD
153+
} else {
154+
cwd = wd
155+
}
148156
}
149157

150158
if cwd == "" {

pkg/tools/shell_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,66 @@ func TestShellTool_OutputTruncation(t *testing.T) {
186186
}
187187
}
188188

189+
// TestShellTool_WorkingDir_OutsideWorkspace verifies that working_dir cannot escape the workspace directly
190+
func TestShellTool_WorkingDir_OutsideWorkspace(t *testing.T) {
191+
root := t.TempDir()
192+
workspace := filepath.Join(root, "workspace")
193+
outsideDir := filepath.Join(root, "outside")
194+
if err := os.MkdirAll(workspace, 0755); err != nil {
195+
t.Fatalf("failed to create workspace: %v", err)
196+
}
197+
if err := os.MkdirAll(outsideDir, 0755); err != nil {
198+
t.Fatalf("failed to create outside dir: %v", err)
199+
}
200+
201+
tool := NewExecTool(workspace, true)
202+
result := tool.Execute(context.Background(), map[string]interface{}{
203+
"command": "pwd",
204+
"working_dir": outsideDir,
205+
})
206+
207+
if !result.IsError {
208+
t.Fatalf("expected working_dir outside workspace to be blocked, got output: %s", result.ForLLM)
209+
}
210+
if !strings.Contains(result.ForLLM, "blocked") {
211+
t.Errorf("expected 'blocked' in error, got: %s", result.ForLLM)
212+
}
213+
}
214+
215+
// TestShellTool_WorkingDir_SymlinkEscape verifies that a symlink inside the workspace
216+
// pointing outside cannot be used as working_dir to escape the sandbox.
217+
func TestShellTool_WorkingDir_SymlinkEscape(t *testing.T) {
218+
root := t.TempDir()
219+
workspace := filepath.Join(root, "workspace")
220+
secretDir := filepath.Join(root, "secret")
221+
if err := os.MkdirAll(workspace, 0755); err != nil {
222+
t.Fatalf("failed to create workspace: %v", err)
223+
}
224+
if err := os.MkdirAll(secretDir, 0755); err != nil {
225+
t.Fatalf("failed to create secret dir: %v", err)
226+
}
227+
os.WriteFile(filepath.Join(secretDir, "secret.txt"), []byte("top secret"), 0644)
228+
229+
// symlink lives inside the workspace but resolves to secretDir outside it
230+
link := filepath.Join(workspace, "escape")
231+
if err := os.Symlink(secretDir, link); err != nil {
232+
t.Skipf("symlinks not supported in this environment: %v", err)
233+
}
234+
235+
tool := NewExecTool(workspace, true)
236+
result := tool.Execute(context.Background(), map[string]interface{}{
237+
"command": "cat secret.txt",
238+
"working_dir": link,
239+
})
240+
241+
if !result.IsError {
242+
t.Fatalf("expected symlink working_dir escape to be blocked, got output: %s", result.ForLLM)
243+
}
244+
if !strings.Contains(result.ForLLM, "blocked") {
245+
t.Errorf("expected 'blocked' in error, got: %s", result.ForLLM)
246+
}
247+
}
248+
189249
// TestShellTool_RestrictToWorkspace verifies workspace restriction
190250
func TestShellTool_RestrictToWorkspace(t *testing.T) {
191251
tmpDir := t.TempDir()

0 commit comments

Comments
 (0)