diff --git a/README.md b/README.md index 011877c..86ca90c 100644 --- a/README.md +++ b/README.md @@ -16,8 +16,7 @@ agent-sandbox --exec bash # a shell in the sandbox instead of claude owned by your host user. Nothing else from the host is mounted, and SELinux labels on your files are never changed (`label=disable`). - It only launches from a directory strictly under an allowed path (see - below), and never from a dot-directory of `$HOME` or a directory containing - its config. + below), never from `$HOME` or a directory containing its config. - Claude's home is the `agent-sandbox-home` podman volume (login, config and nested container images live here). All sessions share it. - `podman` works inside the sandbox (nested rootless). The host podman socket is @@ -43,12 +42,16 @@ addresses inside the sandbox, and all other names return NXDOMAIN. ## Allowed paths `~/.config/agent-sandbox/allowed-paths` lists the roots you may launch from, -one absolute or `~/` path per line (`#` comments allowed). Without the file the -only root is `$HOME`. Roots that are `/`, a parent of `$HOME`, or under `/run`, -`/tmp`, `/var/tmp`, `/proc`, `/sys` or `/dev` abort the launch. +one absolute or `~/` path per line (`#` comments allowed), compared after +resolving symlinks. Without the file the only root is `$HOME`, minus its +dot-directories. Listed roots are trusted even under a dot-directory, but these +abort the launch: `$HOME` or its parents; a top-level dot-directory such as +`~/.local`; anything under `/run`, `/tmp`, `/var/tmp`, `/proc`, `/sys` or +`/dev`; anything that is, holds or sits in `~/.ssh`, `~/.gnupg`, `~/.config`, +`$XDG_RUNTIME_DIR` or podman's storage (`~/.local/share/containers`). ``` -~/src +~/.local/src /srv/work ``` diff --git a/main.go b/main.go index e09e4f4..7a8c906 100644 --- a/main.go +++ b/main.go @@ -338,16 +338,29 @@ func allowedPathsFile() string { return filepath.Join(configDir(), "allowed-paths") } -// allowedRoots reads the launch-dir roots, defaulting to $HOME. -func allowedRoots(home string) ([]string, error) { +// sensitiveDirs are host dirs an allowed-paths root may neither contain nor sit under. +func sensitiveDirs(home string) []string { + dirs := []string{home + "/.ssh", home + "/.gnupg", home + "/.config", filepath.Dir(configDir()), + home + "/.local/share/containers"} + if v := os.Getenv("XDG_RUNTIME_DIR"); v != "" { + dirs = append(dirs, v) + } + if v := os.Getenv("XDG_DATA_HOME"); v != "" { + dirs = append(dirs, v+"/containers") + } + return dirs +} + +// allowedRoots reads the launch-dir roots; without the file the only root is +// $HOME and def is true. +func allowedRoots(home string) (roots []string, def bool, err error) { b, err := os.ReadFile(allowedPathsFile()) if os.IsNotExist(err) { - return []string{home}, nil + return []string{home}, true, nil } if err != nil { - return nil, err + return nil, false, err } - var roots []string for _, l := range strings.Split(string(b), "\n") { l = strings.TrimSpace(l) if l == "" || strings.HasPrefix(l, "#") { @@ -358,35 +371,44 @@ func allowedRoots(home string) ([]string, error) { r = home + l[1:] } if !filepath.IsAbs(r) { - return nil, fmt.Errorf("%s: %q is not an absolute path", allowedPathsFile(), l) + return nil, false, fmt.Errorf("%s: %q is not an absolute path", allowedPathsFile(), l) } r = realpath(r) - if inside(home, r) && r != home { - return nil, fmt.Errorf("%s: %q contains $HOME", allowedPathsFile(), l) + if inside(home, r) { + return nil, false, fmt.Errorf("%s: %q is or contains $HOME", allowedPathsFile(), l) + } + if filepath.Dir(r) == home && strings.HasPrefix(filepath.Base(r), ".") { + return nil, false, fmt.Errorf("%s: %q is a dot-directory of $HOME", allowedPathsFile(), l) } for _, bad := range forbiddenRoots { if inside(r, bad) { - return nil, fmt.Errorf("%s: %q is under %s", allowedPathsFile(), l, bad) + return nil, false, fmt.Errorf("%s: %q is under %s", allowedPathsFile(), l, bad) + } + } + for _, s := range sensitiveDirs(home) { + if s = realpath(s); inside(r, s) || inside(s, r) { + return nil, false, fmt.Errorf("%s: %q overlaps %s", allowedPathsFile(), l, s) } } roots = append(roots, r) } - return roots, nil + return roots, false, nil } // checkLaunchDir allows only launch dirs strictly under an allowed root, never -// a dot-dir of $HOME, $HOME itself, or anything holding the sandbox's config. +// $HOME itself or anything holding the sandbox's config. Under the default +// $HOME root, dot-dirs of $HOME are refused too. func checkLaunchDir(pwd, allowlist string) error { pwd = realpath(pwd) home := realpath(os.Getenv("HOME")) - roots, err := allowedRoots(home) + roots, def, err := allowedRoots(home) if err != nil { return err } if inside(home, pwd) { return fmt.Errorf("refusing to mount %s: it is or contains $HOME", pwd) } - if rel, ok := strings.CutPrefix(pwd, home+"/"); ok && strings.HasPrefix(rel, ".") { + if rel, ok := strings.CutPrefix(pwd, home+"/"); def && ok && strings.HasPrefix(rel, ".") { return fmt.Errorf("refusing to mount %s: it is under a dot-directory of $HOME", pwd) } paths := allowedPathsFile() diff --git a/main_test.go b/main_test.go index c235cf1..8df3e72 100644 --- a/main_test.go +++ b/main_test.go @@ -199,12 +199,13 @@ type env struct { func setup(t *testing.T) *env { work, _ := filepath.EvalSymlinks(t.TempDir()) - for _, d := range []string{"home/proj", "other"} { + for _, d := range []string{"home/proj", "other", "rt"} { _ = os.MkdirAll(filepath.Join(work, d), 0o755) } e := &env{work: work, good: writeList(t, "api.anthropic.com")} t.Setenv("HOME", work+"/home") - t.Setenv("XDG_RUNTIME_DIR", work) + t.Setenv("XDG_RUNTIME_DIR", work+"/rt") + t.Setenv("XDG_DATA_HOME", "") t.Setenv("XDG_CONFIG_HOME", "") t.Setenv("AGENT_SANDBOX_ALLOWLIST", e.good) t.Setenv("AGENT_SANDBOX_IMAGE", "") @@ -291,11 +292,15 @@ func TestLaunchRefusals(t *testing.T) { {"symlink-to-run-user", "home/runlink", func(t *testing.T, e *env) { _ = os.Symlink(runUser, e.work+"/home/runlink") }}, {"outside-home", "other", nil}, {"custom-root-itself", "other", func(t *testing.T, e *env) { writePaths(t, e, e.work+"/other") }}, - {"allowed-paths-dir", "home/proj", func(t *testing.T, e *env) { - t.Setenv("XDG_CONFIG_HOME", e.work+"/home/proj/cfg") - writePaths(t, e, "~/") + {"allowed-paths-dir", "home/src/p", func(t *testing.T, e *env) { + _ = os.MkdirAll(e.work+"/home/src/p", 0o755) + t.Setenv("XDG_CONFIG_HOME", e.work+"/home/src/p/cfg") + writePaths(t, e, "~/src") + }}, + {"invalid-root", "home/proj", func(t *testing.T, e *env) { writePaths(t, e, "/run/x") }}, + {"default-root-dot-dir", "home/.local/src/x", func(t *testing.T, e *env) { + _ = os.MkdirAll(e.work+"/home/.local/src/x", 0o755) }}, - {"invalid-root", "home/proj", func(t *testing.T, e *env) { writePaths(t, e, "~/", "/run/x") }}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -369,7 +374,7 @@ func TestLaunch(t *testing.T) { yes := true r := f.specs[router] tmp := r.Mounts[0].Source - check(t, "tmp under runtime dir", true, strings.HasPrefix(tmp, e.work+"/"+session+".")) + check(t, "tmp under runtime dir", true, strings.HasPrefix(tmp, e.work+"/rt/"+session+".")) check(t, "api dir private", os.FileMode(0o700), apiMode) check(t, "api dir not mounted", false, strings.HasPrefix(f.apiDir, tmp)) for _, d := range []string{tmp, f.apiDir} { @@ -377,7 +382,7 @@ func TestLaunch(t *testing.T) { t.Errorf("%s not removed", d) } } - leftovers(t, e.work) + leftovers(t, e.work+"/rt") check(t, "router image", "localhost/agent-sandbox-router:dev", r.Image) check(t, "router rm", &yes, r.Remove) @@ -469,14 +474,14 @@ func TestLaunchSignalCleansUp(t *testing.T) { n := len(e.fake.calls) check(t, "cleanup", []string{"pod rm", "rm", "network rm", "service stop"}, []string{e.fake.calls[n-4][:6], e.fake.calls[n-3][:2], e.fake.calls[n-2][:10], e.fake.calls[n-1]}) - leftovers(t, e.work) + leftovers(t, e.work+"/rt") } func TestLaunchServiceFails(t *testing.T) { e := setup(t) startService = func(string) (context.Context, engine, func(), error) { return nil, nil, nil, errors.New("no podman") } check(t, "exit", 125, e.launch(t, e.work+"/home/proj", "--exec", "true")) - leftovers(t, e.work) + leftovers(t, e.work+"/rt") } func TestLaunchSeedsAllowlist(t *testing.T) { @@ -514,10 +519,23 @@ func TestLaunchCustomRoot(t *testing.T) { check(t, "podman calls", []string(nil), e.fake.calls) } +func TestLaunchDotDirRoot(t *testing.T) { + e := setup(t) + _ = os.MkdirAll(e.work+"/home/.local/src/x", 0o755) + writePaths(t, e, "~/.local/src") + check(t, "listed dot-dir root", 0, e.launch(t, e.work+"/home/.local/src/x", "--exec", "true")) + + e.reset() + _ = os.Symlink(e.work+"/home/.local/src", e.work+"/home/src") + writePaths(t, e, "~/src") + check(t, "symlinked root", 0, e.launch(t, e.work+"/home/src/x", "--exec", "true")) +} + func TestCheckLaunchDirRoots(t *testing.T) { cfg := t.TempDir() t.Setenv("HOME", "/home/u") t.Setenv("XDG_CONFIG_HOME", cfg) + t.Setenv("XDG_DATA_HOME", "") f := filepath.Join(cfg, "agent-sandbox", "allowed-paths") _ = os.MkdirAll(filepath.Dir(f), 0o755) write := func(l string) { _ = os.WriteFile(f, []byte(l+"\n"), 0o644) } @@ -527,10 +545,11 @@ func TestCheckLaunchDirRoots(t *testing.T) { "/srv/work": false, "/home/u/proj": false, "/run/user/1000": false, "/tmp/x": false} { check(t, dir, ok, checkLaunchDir(dir, "/nowhere/hosts") == nil) } - for _, bad := range []string{"/", "/home", "~", "relative/x", "/run/user/1000", "/tmp/x", "/var/tmp", "/proc/1", + for _, bad := range []string{"/", "/home", "~", "~/", "~/.local", "~/.local/share", "~/.ssh", "~/.ssh/x", + "~/.gnupg", "~/.config/foo", cfg, cfg + "/agent-sandbox/x", "~/.local/share/containers/s", "relative/x", "/run/user/1000", "/tmp/x", "/var/tmp", "/proc/1", "/sys", "/dev/shm"} { - write(bad) - check(t, "invalid root "+bad, true, strings.Contains(fmt.Sprint(checkLaunchDir("/srv/work/x", "/nowhere/hosts")), - "allowed-paths")) + write("/srv/work\n" + bad) + _, _, err := allowedRoots("/home/u") + check(t, "invalid root "+bad, true, strings.Contains(fmt.Sprint(err), "allowed-paths")) } }