Allow null device in jailed bash commands
- commit
- c0a71542b7587f4e7174aa3530039f896d76b854
- author
- patriceckhart · · 2026-07-23 12:27 UTC
- parent
- 734bf87
- signature
- unsigned
2 files · +29 -0
M packages/agent/tools/sandbox.go +6 -0
| @@ -249,6 +249,12 @@ func looksLikePath(s string) bool { | |||
| 249 | 249 | } | |
| 250 | 250 | ||
| 251 | 251 | func (s *Sandbox) checkCommandPath(path string) error { | |
| 252 | + | // The platform null device is safe for shell input/output redirection even | |
| 253 | + | // though it lives outside the sandbox root. Keep this allowlist exact so | |
| 254 | + | // other device paths and lookalike descendants still fail closed. | |
| 255 | + | if path == os.DevNull { | |
| 256 | + | return nil | |
| 257 | + | } | |
| 252 | 258 | rootAbs, err := canonical(s.Root) | |
| 253 | 259 | if err != nil { | |
| 254 | 260 | return fmt.Errorf("sandbox root: %w", err) | |
| @@ -249,6 +249,12 @@ func looksLikePath(s string) bool { | |||||
| 249 | } | 249 | } | ||
| 250 | 250 | ||||
| 251 | func (s *Sandbox) checkCommandPath(path string) error { | 251 | func (s *Sandbox) checkCommandPath(path string) error { | ||
| 252 | + | // The platform null device is safe for shell input/output redirection even | |||
| 253 | + | // though it lives outside the sandbox root. Keep this allowlist exact so | |||
| 254 | + | // other device paths and lookalike descendants still fail closed. | |||
| 255 | + | if path == os.DevNull { | |||
| 256 | + | return nil | |||
| 257 | + | } | |||
| 252 | rootAbs, err := canonical(s.Root) | 258 | rootAbs, err := canonical(s.Root) | ||
| 253 | if err != nil { | 259 | if err != nil { | ||
| 254 | return fmt.Errorf("sandbox root: %w", err) | 260 | return fmt.Errorf("sandbox root: %w", err) | ||
M packages/agent/tools/sandbox_test.go +23 -0
| @@ -75,6 +75,29 @@ func TestBashToolRejectsOutsidePathWhenLocked(t *testing.T) { | |||
| 75 | 75 | } | |
| 76 | 76 | } | |
| 77 | 77 | ||
| 78 | + | func TestSandboxAllowsNullDeviceRedirectionWhenLocked(t *testing.T) { | |
| 79 | + | sb := NewSandbox(t.TempDir()) | |
| 80 | + | sb.Lock() | |
| 81 | + | ||
| 82 | + | allowed := []string{ | |
| 83 | + | "command -v go >" + os.DevNull + " 2>&1", | |
| 84 | + | "cat <" + os.DevNull, | |
| 85 | + | "cat \"" + os.DevNull + "\"", | |
| 86 | + | } | |
| 87 | + | for _, command := range allowed { | |
| 88 | + | if err := sb.CheckCommand(command); err != nil { | |
| 89 | + | t.Errorf("expected %q to be allowed: %v", command, err) | |
| 90 | + | } | |
| 91 | + | } | |
| 92 | + | ||
| 93 | + | blocked := []string{"cat /dev/zero", "cat /dev/null/child"} | |
| 94 | + | for _, command := range blocked { | |
| 95 | + | if err := sb.CheckCommand(command); err == nil { | |
| 96 | + | t.Errorf("expected null-device lookalike %q to be blocked", command) | |
| 97 | + | } | |
| 98 | + | } | |
| 99 | + | } | |
| 100 | + | ||
| 78 | 101 | // TestSandboxAllowsCDIntoSubdir is the regression for issue #39: a `cd` | |
| 79 | 102 | // into a subdirectory of the sandbox root, spelled as an absolute path, | |
| 80 | 103 | // must be allowed. The old guard rejected any `cd /...` outright, which | |
| @@ -75,6 +75,29 @@ func TestBashToolRejectsOutsidePathWhenLocked(t *testing.T) { | |||||
| 75 | } | 75 | } | ||
| 76 | } | 76 | } | ||
| 77 | 77 | ||||
| 78 | + | func TestSandboxAllowsNullDeviceRedirectionWhenLocked(t *testing.T) { | |||
| 79 | + | sb := NewSandbox(t.TempDir()) | |||
| 80 | + | sb.Lock() | |||
| 81 | + | ||||
| 82 | + | allowed := []string{ | |||
| 83 | + | "command -v go >" + os.DevNull + " 2>&1", | |||
| 84 | + | "cat <" + os.DevNull, | |||
| 85 | + | "cat \"" + os.DevNull + "\"", | |||
| 86 | + | } | |||
| 87 | + | for _, command := range allowed { | |||
| 88 | + | if err := sb.CheckCommand(command); err != nil { | |||
| 89 | + | t.Errorf("expected %q to be allowed: %v", command, err) | |||
| 90 | + | } | |||
| 91 | + | } | |||
| 92 | + | ||||
| 93 | + | blocked := []string{"cat /dev/zero", "cat /dev/null/child"} | |||
| 94 | + | for _, command := range blocked { | |||
| 95 | + | if err := sb.CheckCommand(command); err == nil { | |||
| 96 | + | t.Errorf("expected null-device lookalike %q to be blocked", command) | |||
| 97 | + | } | |||
| 98 | + | } | |||
| 99 | + | } | |||
| 100 | + | ||||
| 78 | // TestSandboxAllowsCDIntoSubdir is the regression for issue #39: a `cd` | 101 | // TestSandboxAllowsCDIntoSubdir is the regression for issue #39: a `cd` | ||
| 79 | // into a subdirectory of the sandbox root, spelled as an absolute path, | 102 | // into a subdirectory of the sandbox root, spelled as an absolute path, | ||
| 80 | // must be allowed. The old guard rejected any `cd /...` outright, which | 103 | // must be allowed. The old guard rejected any `cd /...` outright, which | ||