diff --git a/internal/sandbox/config.go b/internal/sandbox/config.go index 67d27d1..6f3c3ff 100644 --- a/internal/sandbox/config.go +++ b/internal/sandbox/config.go @@ -132,19 +132,20 @@ func checkPath(cfg *Config, path string, write bool, cwd string, getenv EnvFunc) } var allowed []string - for _, p := range cfg.Filesystem.RW { - allowed = append(allowed, expandPath(p, cwd, getenv)) - } - for _, p := range cfg.Filesystem.RWX { - allowed = append(allowed, expandPath(p, cwd, getenv)) + expandAndSplit := func(entries []string) { + for _, p := range entries { + for _, ep := range strings.Split(expandPath(p, cwd, getenv), ":") { + if ep != "" { + allowed = append(allowed, ep) + } + } + } } + expandAndSplit(cfg.Filesystem.RW) + expandAndSplit(cfg.Filesystem.RWX) if !write { - for _, p := range cfg.Filesystem.RO { - allowed = append(allowed, expandPath(p, cwd, getenv)) - } - for _, p := range cfg.Filesystem.ROX { - allowed = append(allowed, expandPath(p, cwd, getenv)) - } + expandAndSplit(cfg.Filesystem.RO) + expandAndSplit(cfg.Filesystem.ROX) } for _, root := range allowed { diff --git a/internal/sandbox/sandbox_test.go b/internal/sandbox/sandbox_test.go index ba5e7d7..627c93f 100644 --- a/internal/sandbox/sandbox_test.go +++ b/internal/sandbox/sandbox_test.go @@ -675,6 +675,71 @@ func TestIsSuperpowerdRunning_XDGFallback(t *testing.T) { } } +func TestWrapCommand_ColonSeparatedPaths(t *testing.T) { + tmpDir := t.TempDir() + dir1 := filepath.Join(tmpDir, "skills1") + dir2 := filepath.Join(tmpDir, "skills2") + os.MkdirAll(dir1, 0755) + os.MkdirAll(dir2, 0755) + + // Simulate OLLIE_SKILLS_PATH="/path1:/path2" + t.Setenv("OLLIE_SKILLS_PATH", dir1+":"+dir2) + + cfg := &Config{ + Filesystem: FilesystemConfig{ + RWX: []string{"{OLLIE_SKILLS_PATH}"}, + }, + } + got := mustWrapCommand(t, cfg, []string{"sh"}, tmpDir) + assertFlagValue(t, got, "--rwx", dir1) + assertFlagValue(t, got, "--rwx", dir2) +} + +func TestCheckPath_ColonSeparatedPaths(t *testing.T) { + tmpDir := t.TempDir() + dir1 := filepath.Join(tmpDir, "skills1") + dir2 := filepath.Join(tmpDir, "skills2") + os.MkdirAll(dir1, 0755) + os.MkdirAll(dir2, 0755) + + file1 := filepath.Join(dir1, "test.md") + os.WriteFile(file1, []byte("hi"), 0644) + file2 := filepath.Join(dir2, "test.md") + os.WriteFile(file2, []byte("hi"), 0644) + + getenv := func(k string) string { + if k == "OLLIE_SKILLS_PATH" { + return dir1 + ":" + dir2 + } + if k == "HOME" { + return tmpDir + } + return "" + } + + cfg := &Config{ + Filesystem: FilesystemConfig{ + RWX: []string{"{OLLIE_SKILLS_PATH}"}, + }, + } + + // Both paths should be allowed for read + if err := checkPath(cfg, file1, false, tmpDir, getenv); err != nil { + t.Errorf("file in dir1 should be allowed: %v", err) + } + if err := checkPath(cfg, file2, false, tmpDir, getenv); err != nil { + t.Errorf("file in dir2 should be allowed: %v", err) + } + + // Path outside both should be denied + outside := filepath.Join(tmpDir, "outside", "file.txt") + os.MkdirAll(filepath.Dir(outside), 0755) + os.WriteFile(outside, []byte("x"), 0644) + if err := checkPath(cfg, outside, false, tmpDir, getenv); err == nil { + t.Error("path outside both dirs should be denied") + } +} + func TestIsSuperpowerdRunning_SocketExists(t *testing.T) { tmpDir := t.TempDir() sockPath := filepath.Join(tmpDir, "superpowerd.sock") diff --git a/internal/sandbox/wrapper.go b/internal/sandbox/wrapper.go index 2e37679..27ec488 100644 --- a/internal/sandbox/wrapper.go +++ b/internal/sandbox/wrapper.go @@ -7,6 +7,7 @@ import ( "os/exec" "path/filepath" "sort" + "strings" ) // pathEntry holds a path with its permission type @@ -49,30 +50,21 @@ func WrapCommand(cfg *Config, originalCmd []string, cwd string, getenv EnvFunc) } var entries []pathEntry - for _, path := range cfg.Filesystem.RO { - expanded := expandPath(path, cwd, getenv) - if pathExists(expanded) { - entries = append(entries, pathEntry{expanded, "--ro"}) - } - } - for _, path := range cfg.Filesystem.ROX { - expanded := expandPath(path, cwd, getenv) - if pathExists(expanded) { - entries = append(entries, pathEntry{expanded, "--rox"}) - } - } - for _, path := range cfg.Filesystem.RW { - expanded := expandPath(path, cwd, getenv) - if pathExists(expanded) { - entries = append(entries, pathEntry{expanded, "--rw"}) - } - } - for _, path := range cfg.Filesystem.RWX { - expanded := expandPath(path, cwd, getenv) - if pathExists(expanded) { - entries = append(entries, pathEntry{expanded, "--rwx"}) + addPaths := func(paths []string, flag string) { + for _, path := range paths { + expanded := expandPath(path, cwd, getenv) + // Handle colon-separated paths (e.g. OLLIE_SKILLS_PATH) + for _, p := range strings.Split(expanded, ":") { + if p != "" && pathExists(p) { + entries = append(entries, pathEntry{p, flag}) + } + } } } + addPaths(cfg.Filesystem.RO, "--ro") + addPaths(cfg.Filesystem.ROX, "--rox") + addPaths(cfg.Filesystem.RW, "--rw") + addPaths(cfg.Filesystem.RWX, "--rwx") // Sort so parents come before children sort.Slice(entries, func(i, j int) bool {