diff --git a/README.md b/README.md index 86ca90c..952c462 100644 --- a/README.md +++ b/README.md @@ -12,11 +12,13 @@ agent-sandbox [claude args...] # e.g. agent-sandbox --continue agent-sandbox --exec bash # a shell in the sandbox instead of claude ``` -- `$PWD` is mounted read-write at the same path. Files you create there are +- `$PWD` is mounted read-write at the same path (its resolved target is what + gets mounted). Files you create there are 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), never from `$HOME` or a directory containing its config. + below), never from `$HOME`, a directory containing its config, or one that + is or contains a `$PATH` entry (such as `~/bin` or `~/go`). - 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 @@ -48,7 +50,8 @@ 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`). +`$XDG_RUNTIME_DIR` or podman's storage (`~/.local/share/containers`); anything +that is or contains a `$PATH` entry. ``` ~/.local/src @@ -85,6 +88,8 @@ pod on that network that holds the sandbox's netns, DNS and hosts file. `git.unkin.net` and `api.anthropic.com` can carry your data out. `$PWD` is writable, so anything there, including `.git/hooks`, runs later on the host if you trust it. +- Files written in the launch dir can still run on the host later (git hooks, + Makefiles, `.envrc`), so review changes before running them. - Router-side DNS resolves allowlisted names on the host's resolvers, so internal names like `vault.service.consul` work. - The home volume persists across sessions and is shared by all of them. diff --git a/main.go b/main.go index 1c25d8a..e7a31f3 100644 --- a/main.go +++ b/main.go @@ -84,13 +84,14 @@ func launch(ctx context.Context, args []string) int { warn("%v", err) return 1 } - if strings.Contains(pwd, ":") { - warn("cannot mount a directory containing ':'") + allowlist := allowlistPath() + src, err := checkLaunchDir(pwd, allowlist) + if err != nil { + warn("%v", err) return 1 } - allowlist := allowlistPath() - if err := checkLaunchDir(pwd, allowlist); err != nil { - warn("%v", err) + if strings.Contains(pwd+src, ":") { + warn("cannot mount a directory containing ':'") return 1 } if err := ensureAllowlist(allowlist); err != nil { @@ -134,7 +135,7 @@ func launch(ctx context.Context, args []string) int { opCtx, cancel := context.WithCancel(conn) defer cancel() defer context.AfterFunc(ctx, cancel)() - code := runSession(opCtx, eng, session, tmp, pwd, entries, cmd) + code := runSession(opCtx, eng, session, tmp, src, pwd, entries, cmd) if ctx.Err() != nil { return 130 } @@ -147,7 +148,8 @@ func fail(err error) int { return 125 } -func runSession(ctx context.Context, eng engine, session, tmp, pwd string, entries []entry, cmd []string) int { +// runSession mounts the checked src at pwd, the path the user launched from. +func runSession(ctx context.Context, eng engine, session, tmp, src, pwd string, entries []entry, cmd []string) int { image, routerImage := imageNames() homeVolume := envOr("AGENT_SANDBOX_HOME_VOLUME", "agent-sandbox-home") router := session + "-router" @@ -257,7 +259,7 @@ func runSession(ctx context.Context, eng engine, session, tmp, pwd string, entri c.Annotations = map[string]string{define.InspectAnnotationLabel: "disable"} c.Devices = []spec.LinuxDevice{{Path: "/dev/fuse"}, {Path: "/dev/net/tun"}} c.Volumes = []*specgen.NamedVolume{{Name: homeVolume, Dest: "/home/claude"}} - c.Mounts = []spec.Mount{{Type: "bind", Source: pwd, Destination: pwd}} + c.Mounts = []spec.Mount{{Type: "bind", Source: src, Destination: pwd}} c.WorkDir = pwd c.Env = map[string]string{"DISABLE_AUTOUPDATER": "1", "CLAUDE_CODE_TMPDIR": "/var/tmp"} c.Command = cmd @@ -371,6 +373,11 @@ func allowedRoots(home string) (roots []string, def bool, err error) { return nil, false, fmt.Errorf("%s: %q is under %s", allowedPathsFile(), l, bad) } } + for _, d := range pathDirs() { + if inside(d, r) { + return nil, false, fmt.Errorf("%s: %q is or contains $PATH entry %s", allowedPathsFile(), l, d) + } + } 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) @@ -381,35 +388,45 @@ func allowedRoots(home string) (roots []string, def bool, err error) { return roots, false, nil } -// checkLaunchDir allows only launch dirs strictly under an allowed root, never -// $HOME itself or anything holding the sandbox's config or the API socket. -// Under the default $HOME root, dot-dirs of $HOME are refused too. -func checkLaunchDir(pwd, allowlist string) error { +// pathDirs resolves $PATH; a sandbox-written file there would run on the host. +func pathDirs() []string { + var dirs []string + for _, d := range filepath.SplitList(os.Getenv("PATH")) { + dirs = append(dirs, realpath(d)) + } + return dirs +} + +// checkLaunchDir returns pwd resolved if it is strictly under an allowed root, +// never $HOME itself or anything holding the sandbox's config, the API socket +// or a $PATH entry. Under the default $HOME root, dot-dirs of $HOME are +// refused too. +func checkLaunchDir(pwd, allowlist string) (string, error) { pwd = realpath(pwd) home := realpath(os.Getenv("HOME")) roots, def, err := allowedRoots(home) if err != nil { - return err + return "", err } if inside(home, pwd) { - return fmt.Errorf("refusing to mount %s: it is or contains $HOME", pwd) + return "", fmt.Errorf("refusing to mount %s: it is or contains $HOME", pwd) } 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) + return "", fmt.Errorf("refusing to mount %s: it is under a dot-directory of $HOME", pwd) } paths := allowedPathsFile() - for _, dir := range []string{configDir(), filepath.Dir(allowlist), filepath.Dir(realpath(allowlist)), - filepath.Dir(paths), filepath.Dir(realpath(paths)), runtimeDir()} { + for _, dir := range append([]string{configDir(), filepath.Dir(allowlist), filepath.Dir(realpath(allowlist)), + filepath.Dir(paths), filepath.Dir(realpath(paths)), runtimeDir()}, pathDirs()...) { if dir = realpath(dir); inside(dir, pwd) { - return fmt.Errorf("refusing to mount %s: it contains %s", pwd, dir) + return "", fmt.Errorf("refusing to mount %s: it contains %s", pwd, dir) } } for _, r := range roots { if pwd != r && inside(pwd, r) { - return nil + return pwd, nil } } - return fmt.Errorf("refusing to mount %s: it is not under an allowed path (%s)", pwd, paths) + return "", fmt.Errorf("refusing to mount %s: it is not under an allowed path (%s)", pwd, paths) } // inside reports whether path p is dir or lies under it. diff --git a/main_test.go b/main_test.go index fb369ad..db257ca 100644 --- a/main_test.go +++ b/main_test.go @@ -211,6 +211,7 @@ func setup(t *testing.T) *env { t.Setenv("AGENT_SANDBOX_IMAGE", "") t.Setenv("AGENT_SANDBOX_ROUTER_IMAGE", "") t.Setenv("AGENT_SANDBOX_HOME_VOLUME", "") + t.Setenv("PATH", work+"/home/bin:"+work+"/home/go/bin:/usr/bin") // t.TempDir lives under /tmp, which real configs may not use as a root. origRoots := forbiddenRoots forbiddenRoots = []string{"/run", "/var/tmp", "/proc", "/sys", "/dev"} @@ -240,8 +241,6 @@ func (e *env) launchCtx(t *testing.T, ctx context.Context, dir string, args ...s return launch(ctx, args) } -var runUser = fmt.Sprintf("/run/user/%d", os.Getuid()) - func writePaths(t *testing.T, e *env, lines ...string) { t.Helper() f := allowedPathsFile() @@ -298,12 +297,26 @@ func TestLaunchRefusals(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", e.work+"/home/cfglink") t.Setenv("AGENT_SANDBOX_ALLOWLIST", "") }}, - {"run-user", runUser, nil}, - {"run", "/run", nil}, + {"runtime-under-home", "home/rt", func(t *testing.T, e *env) { + _ = os.MkdirAll(e.work+"/home/rt", 0o700) + t.Setenv("XDG_RUNTIME_DIR", e.work+"/home/rt") + }}, {"tmp", "/tmp", nil}, + {"path-entry", "home/bin", func(t *testing.T, e *env) { _ = os.MkdirAll(e.work+"/home/bin", 0o755) }}, + {"contains-path-entry", "home/go", func(t *testing.T, e *env) { _ = os.MkdirAll(e.work+"/home/go/bin", 0o755) }}, + {"symlink-to-path-entry", "home/binlink", func(t *testing.T, e *env) { + _ = os.MkdirAll(e.work+"/home/bin", 0o755) + _ = os.Symlink(e.work+"/home/bin", e.work+"/home/binlink") + }}, + {"relative-path-entry", "home/proj", func(t *testing.T, e *env) { t.Setenv("PATH", "/usr/bin:.") }}, + {"root-contains-path-entry", "home/src/x", func(t *testing.T, e *env) { + _ = os.MkdirAll(e.work+"/home/src/x", 0o755) + t.Setenv("PATH", e.work+"/home/src/x/bin:/usr/bin") + writePaths(t, e, "~/src") + }}, {"home-config", "home/.config/x", func(t *testing.T, e *env) { _ = os.MkdirAll(e.work+"/home/.config/x", 0o755) }}, {"home-ssh", "home/.ssh", func(t *testing.T, e *env) { _ = os.MkdirAll(e.work+"/home/.ssh", 0o700) }}, - {"symlink-to-run-user", "home/runlink", func(t *testing.T, e *env) { _ = os.Symlink(runUser, e.work+"/home/runlink") }}, + {"symlink-to-runtime-dir", "home/runlink", func(t *testing.T, e *env) { _ = os.Symlink(e.work+"/rt", 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/src/p", func(t *testing.T, e *env) { @@ -327,7 +340,7 @@ func TestLaunchRefusals(t *testing.T) { dir = filepath.Join(e.work, dir) } if _, err := os.Stat(dir); err != nil { - t.Skip(err) + t.Fatal(err) } check(t, "exit", 1, e.launch(t, dir, "--exec", "true")) check(t, "podman calls", []string(nil), e.fake.calls) @@ -339,12 +352,14 @@ func TestCheckLaunchDirRuntimeFallback(t *testing.T) { t.Setenv("HOME", "/nonexistent/home") t.Setenv("XDG_CONFIG_HOME", "/nonexistent/cfg") t.Setenv("XDG_RUNTIME_DIR", "") - err := checkLaunchDir("/tmp", "/nonexistent/hosts") + t.Setenv("PATH", "/usr/bin") + _, err := checkLaunchDir("/tmp", "/nonexistent/hosts") if err == nil || !strings.Contains(err.Error(), "contains "+realpath("/tmp")) { t.Fatalf("got %v", err) } t.Setenv("HOME", "/tmp/home") - check(t, "subdir", nil, checkLaunchDir("/tmp/home/proj", "/nonexistent/hosts")) + _, err = checkLaunchDir("/tmp/home/proj", "/nonexistent/hosts") + check(t, "subdir", nil, err) } func leftovers(t *testing.T, dir string) { @@ -562,18 +577,21 @@ func TestCheckLaunchDirRoots(t *testing.T) { t.Setenv("HOME", "/home/u") t.Setenv("XDG_CONFIG_HOME", cfg) t.Setenv("XDG_DATA_HOME", "") + t.Setenv("PATH", "/usr/bin:/srv/tools/bin") 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) } write("/srv/work\n~/src") for dir, ok := range map[string]bool{"/srv/work/x": true, "/home/u/src/x": true, "/srv/other": false, - "/srv/work": false, "/home/u/proj": false, "/run/user/1000": false, "/tmp/x": false} { - check(t, dir, ok, checkLaunchDir(dir, "/nowhere/hosts") == nil) + "/srv/work": false, "/home/u/proj": false, "/run/user/1000": false, "/run": false, "/tmp/x": false, + "/srv/work/b": true} { + _, err := checkLaunchDir(dir, "/nowhere/hosts") + check(t, dir, ok, err == nil) } 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"} { + "/sys", "/dev/shm", "/usr", "/srv/tools"} { write("/srv/work\n" + bad) _, _, err := allowedRoots("/home/u") check(t, "invalid root "+bad, true, strings.Contains(fmt.Sprint(err), "allowed-paths")) @@ -601,3 +619,28 @@ func TestLaunchPassesArgsVerbatim(t *testing.T) { } } } + +func TestLaunchPathEntries(t *testing.T) { + e := setup(t) + _ = os.MkdirAll(e.work+"/home/src/x", 0o755) + _ = os.MkdirAll(e.work+"/home/go/bin", 0o755) + check(t, "src allowed", 0, e.launch(t, e.work+"/home/src/x", "--exec", "true")) + e.reset() + check(t, "go refused", 1, e.launch(t, e.work+"/home/go", "--exec", "true")) + check(t, "podman calls", []string(nil), e.fake.calls) +} + +func TestLaunchMountsResolvedSource(t *testing.T) { + e := setup(t) + _ = os.MkdirAll(e.work+"/home/.local/src/x", 0o755) + _ = os.Symlink(e.work+"/home/.local/src", e.work+"/home/src") + writePaths(t, e, "~/src") + dir := e.work + "/home/src/x" + check(t, "exit", 0, e.launch(t, dir, "--exec", "true")) + for name, c := range e.fake.specs { + if strings.HasSuffix(name, "-claude") { + check(t, "mount", []spec.Mount{{Type: "bind", Source: e.work + "/home/.local/src/x", Destination: dir}}, c.Mounts) + check(t, "workdir", dir, c.WorkDir) + } + } +}