feat: enhance sandbox security by blocking additional host paths and socket bind mounts, and reduce container stop timeout.
This commit is contained in:
parent
7ef1cacbfb
commit
e9ddf8cc8e
6 changed files with 150 additions and 18 deletions
|
|
@ -465,7 +465,7 @@ func (c *ContainerSandbox) sandboxStateDir() string {
|
|||
}
|
||||
|
||||
func (c *ContainerSandbox) stopAndRemoveContainer(ctx context.Context, containerName string) error {
|
||||
timeout := 10
|
||||
timeout := 5
|
||||
_ = c.cli.ContainerStop(ctx, containerName, container.StopOptions{Timeout: &timeout})
|
||||
if err := c.cli.ContainerRemove(ctx, containerName, container.RemoveOptions{Force: true}); err != nil {
|
||||
return err
|
||||
|
|
@ -484,7 +484,7 @@ func stopAndRemoveContainerByName(ctx context.Context, containerName string) err
|
|||
}
|
||||
defer cli.Close()
|
||||
|
||||
timeout := 10
|
||||
timeout := 5
|
||||
_ = cli.ContainerStop(ctx, name, container.StopOptions{Timeout: &timeout})
|
||||
if err := cli.ContainerRemove(ctx, name, container.RemoveOptions{Force: true}); err != nil {
|
||||
return err
|
||||
|
|
|
|||
|
|
@ -249,8 +249,8 @@ func TestContainerSandbox_Integration_MaybePruneRemovesOldContainer(t *testing.T
|
|||
}
|
||||
|
||||
// Explicitly call Prune (scoped manager would normally do this in loop)
|
||||
if err := sb.Prune(ctx); err != nil {
|
||||
t.Fatalf("Prune failed: %v", err)
|
||||
if pruneErr := sb.Prune(ctx); pruneErr != nil {
|
||||
t.Fatalf("Prune failed: %v", pruneErr)
|
||||
}
|
||||
|
||||
// Verify container is gone
|
||||
|
|
@ -313,10 +313,15 @@ func TestContainerSandbox_Integration_ExecTimeoutBreaksStdCopyBlock(t *testing.T
|
|||
}
|
||||
defer sb.Prune(ctx)
|
||||
|
||||
// Ensure container is started so the timeout only applies to command execution
|
||||
if _, err := sb.Exec(ctx, ExecRequest{Command: "true"}); err != nil {
|
||||
t.Fatalf("warmup failed: %v", err)
|
||||
}
|
||||
|
||||
// Simulate a command that hangs and might block output readers
|
||||
start := time.Now()
|
||||
_, err := sb.Exec(ctx, ExecRequest{
|
||||
Command: "cat", // Blocks waiting for stdin which is never provided
|
||||
Command: "sleep 10", // Guaranteed to hang
|
||||
TimeoutMs: 500,
|
||||
})
|
||||
elapsed := time.Since(start)
|
||||
|
|
|
|||
|
|
@ -10,25 +10,36 @@ import (
|
|||
)
|
||||
|
||||
var blockedHostPaths = []string{
|
||||
"/boot",
|
||||
"/dev",
|
||||
"/etc",
|
||||
"/private/etc",
|
||||
"/proc",
|
||||
"/sys",
|
||||
"/dev",
|
||||
"/root",
|
||||
"/boot",
|
||||
"/run",
|
||||
"/var/run",
|
||||
"/private/var/run",
|
||||
"/run/docker.sock",
|
||||
"/var/run/docker.sock",
|
||||
"/private/var/run/docker.sock",
|
||||
"/run/user",
|
||||
"/proc",
|
||||
"/root",
|
||||
"/run",
|
||||
"/run/containerd",
|
||||
"/run/crio",
|
||||
"/run/docker.sock",
|
||||
"/run/podman",
|
||||
"/run/user",
|
||||
"/sys",
|
||||
"/tmp/podman.sock",
|
||||
"/var/run",
|
||||
"/var/run/containerd",
|
||||
"/var/run/crio",
|
||||
"/var/run/docker.sock",
|
||||
"/xdg_runtime_dir",
|
||||
}
|
||||
|
||||
var blockedHostPathSuffixes = []string{
|
||||
"/.docker/run/docker.sock",
|
||||
"/.docker/desktop/docker.sock",
|
||||
"/.colima/default/docker.sock",
|
||||
"/.rd/docker.sock",
|
||||
}
|
||||
|
||||
var blockedEnvVarPatterns = []*regexp.Regexp{
|
||||
regexp.MustCompile(`(?i)^ANTHROPIC_API_KEY$`),
|
||||
regexp.MustCompile(`(?i)^OPENAI_API_KEY$`),
|
||||
|
|
@ -97,6 +108,18 @@ func validateBindSourcePath(bind, source string) error {
|
|||
return fmt.Errorf("sandbox security: bind mount %q targets blocked path %q", bind, blocked)
|
||||
}
|
||||
}
|
||||
for _, suffix := range blockedHostPathSuffixes {
|
||||
if source == suffix || strings.HasSuffix(source, suffix) {
|
||||
return fmt.Errorf("sandbox security: bind mount %q targets blocked path suffix %q", bind, suffix)
|
||||
}
|
||||
}
|
||||
isSocket, err := isUnixSocketPath(source)
|
||||
if err != nil {
|
||||
return fmt.Errorf("sandbox security: bind mount %q source %q cannot be validated: %w", bind, source, err)
|
||||
}
|
||||
if isSocket {
|
||||
return fmt.Errorf("sandbox security: bind mount %q targets unix socket %q", bind, source)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
|
|
@ -194,3 +217,18 @@ func isBlockedEnvVarKey(key string) bool {
|
|||
var filepathEvalSymlinks = func(path string) (string, error) {
|
||||
return filepath.EvalSymlinks(path)
|
||||
}
|
||||
|
||||
var osLstat = func(path string) (os.FileInfo, error) {
|
||||
return os.Lstat(path)
|
||||
}
|
||||
|
||||
func isUnixSocketPath(p string) (bool, error) {
|
||||
fi, err := osLstat(p)
|
||||
if err != nil {
|
||||
if os.IsNotExist(err) {
|
||||
return false, nil
|
||||
}
|
||||
return false, err
|
||||
}
|
||||
return fi.Mode()&os.ModeSocket != 0, nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package sandbox
|
||||
|
||||
import (
|
||||
"net"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
|
|
@ -17,6 +18,26 @@ func TestValidateBindMounts_BlocksDangerousPath(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestValidateBindMounts_BlocksAdditionalRuntimePaths(t *testing.T) {
|
||||
err := validateBindMounts([]string{"/run/containerd/containerd.sock:/mnt/runtime.sock:ro"})
|
||||
if err == nil {
|
||||
t.Fatal("expected blocked runtime bind path error")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "blocked path") {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateBindMounts_BlocksDangerousSocketSuffixes(t *testing.T) {
|
||||
err := validateBindMounts([]string{"/home/user/.docker/run/docker.sock:/mnt/docker.sock:ro"})
|
||||
if err == nil {
|
||||
t.Fatal("expected blocked dangerous socket suffix error")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "blocked path suffix") {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateBindMounts_BlocksNonAbsoluteSource(t *testing.T) {
|
||||
err := validateBindMounts([]string{"myvol:/mnt"})
|
||||
if err == nil {
|
||||
|
|
@ -33,6 +54,40 @@ func TestValidateBindMounts_AllowsProjectPath(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestValidateBindMounts_BlocksUnixSocketSource(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
socketPath := filepath.Join(tmpDir, "agent.sock")
|
||||
ln, err := net.Listen("unix", socketPath)
|
||||
if err != nil {
|
||||
t.Fatalf("listen unix: %v", err)
|
||||
}
|
||||
defer ln.Close()
|
||||
|
||||
err = validateBindMounts([]string{socketPath + ":/workspace/agent.sock:ro"})
|
||||
if err == nil {
|
||||
t.Fatal("expected unix socket source to be blocked")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "unix socket") {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateBindMounts_BlocksSymlinkToBlockedPath(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
link := filepath.Join(tmpDir, "etc-link")
|
||||
if err := os.Symlink("/etc", link); err != nil {
|
||||
t.Skipf("symlink not supported in this environment: %v", err)
|
||||
}
|
||||
|
||||
err := validateBindMounts([]string{link + ":/workspace/etc:ro"})
|
||||
if err == nil {
|
||||
t.Fatal("expected symlink-resolved blocked path error")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "blocked path") {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateNetworkMode_BlocksHost(t *testing.T) {
|
||||
if err := validateNetworkMode("HOST"); err == nil {
|
||||
t.Fatal("expected host network mode to be blocked")
|
||||
|
|
@ -138,7 +193,9 @@ func TestTryRealpathAbsolute_Branches(t *testing.T) {
|
|||
}
|
||||
|
||||
old := filepathEvalSymlinks
|
||||
oldLstat := osLstat
|
||||
t.Cleanup(func() { filepathEvalSymlinks = old })
|
||||
t.Cleanup(func() { osLstat = oldLstat })
|
||||
|
||||
filepathEvalSymlinks = old
|
||||
if got := tryRealpathAbsolute(link); got == link {
|
||||
|
|
@ -155,3 +212,36 @@ func TestTryRealpathAbsolute_Branches(t *testing.T) {
|
|||
t.Fatalf("tryRealpathAbsolute(nonexistent) got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsUnixSocketPath_Branches(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
socketPath := filepath.Join(tmpDir, "s.sock")
|
||||
ln, err := net.Listen("unix", socketPath)
|
||||
if err != nil {
|
||||
t.Fatalf("listen unix: %v", err)
|
||||
}
|
||||
defer ln.Close()
|
||||
|
||||
ok, err := isUnixSocketPath(socketPath)
|
||||
if err != nil {
|
||||
t.Fatalf("isUnixSocketPath(socket) error: %v", err)
|
||||
}
|
||||
if !ok {
|
||||
t.Fatal("expected socket path to be detected")
|
||||
}
|
||||
|
||||
ok, err = isUnixSocketPath(filepath.Join(tmpDir, "missing.sock"))
|
||||
if err != nil {
|
||||
t.Fatalf("isUnixSocketPath(missing) error: %v", err)
|
||||
}
|
||||
if ok {
|
||||
t.Fatal("missing path should not be detected as socket")
|
||||
}
|
||||
|
||||
old := osLstat
|
||||
t.Cleanup(func() { osLstat = old })
|
||||
osLstat = func(path string) (os.FileInfo, error) { return nil, os.ErrPermission }
|
||||
if _, err := isUnixSocketPath(socketPath); err == nil {
|
||||
t.Fatal("expected lstat permission error to be returned")
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -898,7 +898,7 @@ func (c *Config) ValidateModelList() error {
|
|||
}
|
||||
|
||||
// IntPtr is a convenience helper that returns a pointer to the provided int value.
|
||||
// It is used to initialise *int config fields with literal defaults.
|
||||
// It is used to initialize *int config fields with literal defaults.
|
||||
func IntPtr(v int) *int {
|
||||
return &v
|
||||
}
|
||||
|
|
|
|||
|
|
@ -309,9 +309,8 @@ func (t *CronTool) ExecuteJob(ctx context.Context, job *cron.CronJob) string {
|
|||
// Execute command if present
|
||||
if job.Payload.Command != "" {
|
||||
var output string
|
||||
cwd := ""
|
||||
if t.execGuard != nil {
|
||||
cwd = t.execGuard.workingDir
|
||||
cwd := t.execGuard.workingDir
|
||||
if guardError := t.execGuard.guardCommand(job.Payload.Command, cwd); guardError != "" {
|
||||
output = fmt.Sprintf("Error executing scheduled command: %s", guardError)
|
||||
pubCtx, pubCancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue