From 52bcb0a89f7908a92fe4f6c49b7554d0b056616e Mon Sep 17 00:00:00 2001 From: Test Writer Agent Date: Sat, 31 Oct 2026 09:09:26 +0000 Subject: [PATCH 1/5] test(security): regression tests for plan 03 Signed-off-by: Test Writer Agent --- .../derived_grants_security_test.go | 184 ++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 internal/sandboxrun/derived_grants_security_test.go diff --git a/internal/sandboxrun/derived_grants_security_test.go b/internal/sandboxrun/derived_grants_security_test.go new file mode 100644 index 00000000..6527036e --- /dev/null +++ b/internal/sandboxrun/derived_grants_security_test.go @@ -0,0 +1,184 @@ +package sandboxrun + +import ( + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/TNG/oh-my-agentic-coder/internal/sandboxprofile" +) + +// requireSandboxBackend skips on platforms without a usable built-in +// sandbox backend, mirroring the integration tests' bwrap guard without +// build tags so this file compiles everywhere. +func requireSandboxBackend(t *testing.T) { + t.Helper() + switch runtime.GOOS { + case "linux": + if _, err := exec.LookPath("bwrap"); err != nil { + t.Skip("bubblewrap (bwrap) not available on PATH") + } + case "darwin": + if _, err := exec.LookPath("sandbox-exec"); err != nil { + t.Skip("sandbox-exec not available on PATH") + } + default: + t.Skipf("the built-in sandbox is unsupported on %s", runtime.GOOS) + } +} + +// minimalDerivedGrants builds the minimal grant set a spawned child would +// receive: the workdir is the writable surface, everything else stays +// closed. ModeOpen plus env-only enforcement keeps the spawn argv free of +// kernel-gate machinery so the assertions observe the derived mounts only. +func minimalDerivedGrants(wd, home string) *Grants { + return &Grants{ + Workdir: wd, + HomeDir: home, + AllowPaths: []string{wd}, + NetworkMode: sandboxprofile.ModeOpen, + Enforcement: sandboxprofile.EnforceEnvOnly, + } +} + +// writeExecScript writes content and marks it executable so command +// resolution accepts it as the inner command or shebang interpreter. +func writeExecScript(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0o755); err != nil { + t.Fatal(err) + } +} + +// configRootDir returns a fresh directory below the (staged) user home's +// config root: the place profiles never grant without an explicit user +// request. +func configRootDir(t *testing.T, home string) string { + t.Helper() + base := filepath.Join(home, ".config") + if err := os.MkdirAll(base, 0o755); err != nil { + t.Fatal(err) + } + dir, err := os.MkdirTemp(base, "omac-security-") + if err != nil { + t.Fatal(err) + } + return dir +} + +// stableToolTree builds a small install tree below the package directory: +// outside the workdir, outside the sandbox's writable surface, and outside +// every temp root, the way a user-managed tool runtime lives on a real +// host. The confined process can read it but never modify it. +func stableToolTree(t *testing.T) (links, installs, runtimeDir string) { + t.Helper() + root, err := os.MkdirTemp(".", "omac-security-ctrl-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(root) }) + if root, err = filepath.Abs(root); err != nil { + t.Fatal(err) + } + links = filepath.Join(root, "links") + installs = filepath.Join(root, "installs", "v1") + runtimeDir = filepath.Join(root, "runtime") + for _, dir := range []string{links, installs, runtimeDir} { + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + } + return links, installs, runtimeDir +} + +// TestSecurityDerivedGrantsRefuseAgentWritableInnerCommand guards the +// derived-grant policy at the spawn boundary: an inner command the +// confined process can rewrite (it lives in the workdir) must not be able +// to extend the next sandbox's read surface via its shebang — here into +// the user's config root. The spawn must fail closed and name the +// directory it refused to derive. +func TestSecurityDerivedGrantsRefuseAgentWritableInnerCommand(t *testing.T) { + requireSandboxBackend(t) + + home := stagedHome(t) + wd := t.TempDir() + secretDir := configRootDir(t, home) + interp := filepath.Join(secretDir, "interp") + writeExecScript(t, interp, "#!/bin/sh\n") + + tool := filepath.Join(wd, "tool") + writeExecScript(t, tool, "#!"+interp+"\n") + + g := minimalDerivedGrants(wd, home) + _, err := BuildChildArgv(g, []string{tool}) + if err == nil { + t.Fatalf("spawn argv was built for an inner command planted in the workdir; expected a refusal that names %q", secretDir) + } + if !strings.Contains(err.Error(), secretDir) { + t.Errorf("refusal does not name the derived target directory %q: %v", secretDir, err) + } +} + +// TestSecuritySymlinkChainGrantBounded guards symlink-chain derivations: +// a chain that starts in the writable surface is refused even when it +// ends in trusted content; a chain entirely outside the writable surface +// keeps resolving the way version-manager shims do; and no derivation may +// cross into the home/config boundary without an explicit user grant. +func TestSecuritySymlinkChainGrantBounded(t *testing.T) { + requireSandboxBackend(t) + + home := stagedHome(t) + wd := t.TempDir() + links, installs, runtimeDir := stableToolTree(t) + g := minimalDerivedGrants(wd, home) + + t.Run("writable first hop refused despite trusted target", func(t *testing.T) { + target := filepath.Join(installs, "real-a") + writeExecScript(t, target, "#!/bin/sh\n") + hop := filepath.Join(wd, "hop") + if err := os.Symlink(target, hop); err != nil { + t.Fatal(err) + } + if _, err := BuildChildArgv(g, []string{hop}); err == nil { + t.Errorf("spawn argv was built for a chain whose first hop (%q) lies in the writable workdir; expected a refusal", hop) + } + }) + + t.Run("chain outside the writable surface still resolves", func(t *testing.T) { + interp := filepath.Join(runtimeDir, "interp") + writeExecScript(t, interp, "#!/bin/sh\n") + real := filepath.Join(installs, "real") + writeExecScript(t, real, "#!"+interp+"\n") + shim := filepath.Join(links, "tool") + if err := os.Symlink(real, shim); err != nil { + t.Fatal(err) + } + argv, err := BuildChildArgv(g, []string{shim}) + if err != nil { + t.Fatalf("a chain entirely outside the writable surface must keep resolving, got: %v", err) + } + joined := strings.Join(argv, " ") + if !strings.Contains(joined, runtimeDir) { + t.Errorf("the interpreter directory %q was not derived into the spawn argv — the fixture is broken, not the security property: %s", runtimeDir, joined) + } + if runtime.GOOS == "linux" && !strings.Contains(joined, "--ro-bind "+runtimeDir+" "+runtimeDir) { + t.Errorf("the interpreter directory %q is not a read-only mount in the spawn argv: %s", runtimeDir, joined) + } + }) + + t.Run("home boundary needs an explicit grant", func(t *testing.T) { + secretDir := configRootDir(t, home) + target := filepath.Join(secretDir, "tool") + writeExecScript(t, target, "#!/bin/sh\n") + hop := filepath.Join(links, "home-hop") + if err := os.Symlink(target, hop); err != nil { + t.Fatal(err) + } + if _, err := BuildChildArgv(g, []string{hop}); err == nil { + t.Errorf("spawn argv was built while deriving a grant below the user's config root %q; expected a refusal", secretDir) + } + }) +} From 33404f309b0812160831b43dce725c87bcbbc863 Mon Sep 17 00:00:00 2001 From: Test Writer Agent Date: Sat, 31 Oct 2026 09:21:12 +0000 Subject: [PATCH 2/5] test(security): regression tests for plan 03 Signed-off-by: Test Writer Agent --- .../derived_grants_security_test.go | 156 ++++++++++++------ 1 file changed, 102 insertions(+), 54 deletions(-) diff --git a/internal/sandboxrun/derived_grants_security_test.go b/internal/sandboxrun/derived_grants_security_test.go index 6527036e..41e17662 100644 --- a/internal/sandboxrun/derived_grants_security_test.go +++ b/internal/sandboxrun/derived_grants_security_test.go @@ -11,9 +11,8 @@ import ( "github.com/TNG/oh-my-agentic-coder/internal/sandboxprofile" ) -// requireSandboxBackend skips on platforms without a usable built-in -// sandbox backend, mirroring the integration tests' bwrap guard without -// build tags so this file compiles everywhere. +// requireSandboxBackend skips on platforms without the sandbox CLI of +// the built-in backend. No build tags, so this file compiles everywhere. func requireSandboxBackend(t *testing.T) { t.Helper() switch runtime.GOOS { @@ -30,10 +29,10 @@ func requireSandboxBackend(t *testing.T) { } } -// minimalDerivedGrants builds the minimal grant set a spawned child would -// receive: the workdir is the writable surface, everything else stays -// closed. ModeOpen plus env-only enforcement keeps the spawn argv free of -// kernel-gate machinery so the assertions observe the derived mounts only. +// minimalDerivedGrants is the smallest grant set a spawned child gets: +// the workdir is the writable surface, everything else stays closed. +// Open networking with env-only enforcement keeps the spawn argv free of +// kernel-gate machinery, so the assertions see the derived mounts only. func minimalDerivedGrants(wd, home string) *Grants { return &Grants{ Workdir: wd, @@ -44,8 +43,8 @@ func minimalDerivedGrants(wd, home string) *Grants { } } -// writeExecScript writes content and marks it executable so command -// resolution accepts it as the inner command or shebang interpreter. +// writeExecScript writes content and marks it executable, so command +// resolution accepts it as an inner command or shebang interpreter. func writeExecScript(t *testing.T, path, content string) { t.Helper() if err := os.WriteFile(path, []byte(content), 0o755); err != nil { @@ -53,9 +52,27 @@ func writeExecScript(t *testing.T, path, content string) { } } -// configRootDir returns a fresh directory below the (staged) user home's -// config root: the place profiles never grant without an explicit user -// request. +// stableHome stages HOME below the package directory: outside the +// writable surface, outside every temp root, and outside the real home. +// A home under TMPDIR (what t.TempDir gives) would itself count as +// agent-writable surface under the derived-grant taint policy, so a +// boundary refusal could then fire for the taint reason instead. +func stableHome(t *testing.T) string { + t.Helper() + home, err := os.MkdirTemp(".", "omac-security-home-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(home) }) + if home, err = filepath.Abs(home); err != nil { + t.Fatal(err) + } + t.Setenv("HOME", home) + return home +} + +// configRootDir returns a fresh directory below the (staged) home's +// config root: dotfile config trees must never be granted silently. func configRootDir(t *testing.T, home string) string { t.Helper() base := filepath.Join(home, ".config") @@ -69,10 +86,11 @@ func configRootDir(t *testing.T, home string) string { return dir } -// stableToolTree builds a small install tree below the package directory: -// outside the workdir, outside the sandbox's writable surface, and outside -// every temp root, the way a user-managed tool runtime lives on a real -// host. The confined process can read it but never modify it. +// stableToolTree builds an install tree below the package directory, +// the way a user-managed tool runtime lives on a real host: shim links, +// install dirs, an interpreter dir. It sits outside the writable +// surface, outside every temp root, and outside the staged home, so the +// confined process can read it but never modify it. func stableToolTree(t *testing.T) (links, installs, runtimeDir string) { t.Helper() root, err := os.MkdirTemp(".", "omac-security-ctrl-") @@ -94,59 +112,83 @@ func stableToolTree(t *testing.T) (links, installs, runtimeDir string) { return links, installs, runtimeDir } +// requireDerivedBind asserts dir was derived into the spawn argv as a +// read-only grant. On Linux that is a --ro-bind pair; on macOS the +// directory appears in the generated Seatbelt profile text. +func requireDerivedBind(t *testing.T, argv []string, dir string) { + t.Helper() + joined := strings.Join(argv, " ") + if !strings.Contains(joined, dir) { + t.Errorf("the interpreter directory %q was not derived into the spawn argv: %s", dir, joined) + } + if runtime.GOOS == "linux" && !strings.Contains(joined, "--ro-bind "+dir+" "+dir) { + t.Errorf("the interpreter directory %q is not a read-only mount in the spawn argv: %s", dir, joined) + } +} + // TestSecurityDerivedGrantsRefuseAgentWritableInnerCommand guards the // derived-grant policy at the spawn boundary: an inner command the -// confined process can rewrite (it lives in the workdir) must not be able -// to extend the next sandbox's read surface via its shebang — here into +// confined process can rewrite (it lives in the workdir) must not +// extend the next sandbox's read surface via its shebang, here into // the user's config root. The spawn must fail closed and name the -// directory it refused to derive. +// directory it refused to derive. The control case runs first: an +// identical spawn whose inner command lives in untainted content still +// derives its interpreter dir, so the refusal can only come from the +// agent-writability of the inner command, never from the environment +// or a blanket suppression of shebang derivation. func TestSecurityDerivedGrantsRefuseAgentWritableInnerCommand(t *testing.T) { requireSandboxBackend(t) - home := stagedHome(t) + home := stableHome(t) wd := t.TempDir() - secretDir := configRootDir(t, home) - interp := filepath.Join(secretDir, "interp") - writeExecScript(t, interp, "#!/bin/sh\n") + _, installs, runtimeDir := stableToolTree(t) + g := minimalDerivedGrants(wd, home) - tool := filepath.Join(wd, "tool") - writeExecScript(t, tool, "#!"+interp+"\n") + t.Run("untainted command still derives its interpreter dir", func(t *testing.T) { + interp := filepath.Join(runtimeDir, "interp") + writeExecScript(t, interp, "#!/bin/sh\n") + benign := filepath.Join(installs, "tool") + writeExecScript(t, benign, "#!"+interp+"\n") + argv, err := BuildChildArgv(g, []string{benign}) + if err != nil { + t.Fatalf("an inner command outside the writable surface must keep deriving, got: %v", err) + } + requireDerivedBind(t, argv, runtimeDir) + }) - g := minimalDerivedGrants(wd, home) - _, err := BuildChildArgv(g, []string{tool}) - if err == nil { - t.Fatalf("spawn argv was built for an inner command planted in the workdir; expected a refusal that names %q", secretDir) - } - if !strings.Contains(err.Error(), secretDir) { - t.Errorf("refusal does not name the derived target directory %q: %v", secretDir, err) - } + t.Run("workdir command with a shebang into the config root is refused", func(t *testing.T) { + secretDir := configRootDir(t, home) + interp := filepath.Join(secretDir, "interp") + writeExecScript(t, interp, "#!/bin/sh\n") + tool := filepath.Join(wd, "tool") + writeExecScript(t, tool, "#!"+interp+"\n") + argv, err := BuildChildArgv(g, []string{tool}) + if err == nil { + t.Fatalf("spawn argv was built for an inner command planted in the workdir; expected a refusal that names %q", secretDir) + } + if argv != nil { + t.Errorf("the spawn must fail closed, not return a partial grant set: %v", argv) + } + if !strings.Contains(err.Error(), secretDir) { + t.Errorf("refusal does not name the derived target directory %q: %v", secretDir, err) + } + }) } // TestSecuritySymlinkChainGrantBounded guards symlink-chain derivations: // a chain that starts in the writable surface is refused even when it -// ends in trusted content; a chain entirely outside the writable surface -// keeps resolving the way version-manager shims do; and no derivation may -// cross into the home/config boundary without an explicit user grant. +// ends in trusted content; a chain entirely outside the writable +// surface keeps resolving the way version-manager shims do; and no +// derivation may cross into the home/config boundary without an +// explicit user grant. func TestSecuritySymlinkChainGrantBounded(t *testing.T) { requireSandboxBackend(t) - home := stagedHome(t) + home := stableHome(t) wd := t.TempDir() links, installs, runtimeDir := stableToolTree(t) g := minimalDerivedGrants(wd, home) - t.Run("writable first hop refused despite trusted target", func(t *testing.T) { - target := filepath.Join(installs, "real-a") - writeExecScript(t, target, "#!/bin/sh\n") - hop := filepath.Join(wd, "hop") - if err := os.Symlink(target, hop); err != nil { - t.Fatal(err) - } - if _, err := BuildChildArgv(g, []string{hop}); err == nil { - t.Errorf("spawn argv was built for a chain whose first hop (%q) lies in the writable workdir; expected a refusal", hop) - } - }) - t.Run("chain outside the writable surface still resolves", func(t *testing.T) { interp := filepath.Join(runtimeDir, "interp") writeExecScript(t, interp, "#!/bin/sh\n") @@ -160,12 +202,18 @@ func TestSecuritySymlinkChainGrantBounded(t *testing.T) { if err != nil { t.Fatalf("a chain entirely outside the writable surface must keep resolving, got: %v", err) } - joined := strings.Join(argv, " ") - if !strings.Contains(joined, runtimeDir) { - t.Errorf("the interpreter directory %q was not derived into the spawn argv — the fixture is broken, not the security property: %s", runtimeDir, joined) + requireDerivedBind(t, argv, runtimeDir) + }) + + t.Run("writable first hop refused despite trusted target", func(t *testing.T) { + target := filepath.Join(installs, "real-a") + writeExecScript(t, target, "#!/bin/sh\n") + hop := filepath.Join(wd, "hop") + if err := os.Symlink(target, hop); err != nil { + t.Fatal(err) } - if runtime.GOOS == "linux" && !strings.Contains(joined, "--ro-bind "+runtimeDir+" "+runtimeDir) { - t.Errorf("the interpreter directory %q is not a read-only mount in the spawn argv: %s", runtimeDir, joined) + if _, err := BuildChildArgv(g, []string{hop}); err == nil { + t.Errorf("spawn argv was built for a chain whose first hop (%q) lies in the writable workdir; expected a refusal", hop) } }) From 305e2802ad39a411af1dc2d97d5df52ea0f24d38 Mon Sep 17 00:00:00 2001 From: Fix Writer Agent Date: Sat, 31 Oct 2026 09:45:09 +0000 Subject: [PATCH 3/5] fix(security): Filesystem grants derived from the inner command must not follow interpreter or symlink content that lives where the confined process can write, and each derived directory needs a bounded taint decision. Signed-off-by: Fix Writer Agent --- internal/sandboxrun/backend_darwin.go | 11 +- internal/sandboxrun/backend_darwin_test.go | 26 +- internal/sandboxrun/backend_linux.go | 12 +- internal/sandboxrun/backend_linux_test.go | 35 +- .../proxyinject_integration_linux_test.go | 9 +- internal/sandboxrun/resolve_binary.go | 313 ++++++++++++++++-- .../sandboxrun/resolve_binary_chain_test.go | 5 +- .../sandboxrun/resolve_binary_prefix_test.go | 11 +- 8 files changed, 366 insertions(+), 56 deletions(-) diff --git a/internal/sandboxrun/backend_darwin.go b/internal/sandboxrun/backend_darwin.go index b7d9910b..0fb128ee 100644 --- a/internal/sandboxrun/backend_darwin.go +++ b/internal/sandboxrun/backend_darwin.go @@ -55,7 +55,16 @@ func BuildChildArgv(g *Grants, innerArgv []string) ([]string, error) { return nil, fmt.Errorf("sandbox home %q is not a directory", home) } gz.HomeDir = home - gz.ReadPaths = append(append([]string{}, g.ReadPaths...), resolveInnerBinaryDirs(innerArgv)...) + // The derivation is bounded by the grants (derivedGrantPolicyForGrants): + // the inner command frequently lives where the agent can write (the + // workdir, a temp dir), and granting read access picked by whatever the + // agent last wrote there would let the agent choose what the sandbox can + // read. Crossing the boundary fails the launch instead. + derived, err := resolveInnerBinaryDirs(innerArgv, derivedGrantPolicyForGrants(&gz)) + if err != nil { + return nil, err + } + gz.ReadPaths = append(append([]string{}, g.ReadPaths...), derived...) profile := GenerateSBPL(&gz) argv := []string{sandboxExecPath, "-p", profile, "--"} argv = append(argv, absoluteInnerArgv(innerArgv)...) diff --git a/internal/sandboxrun/backend_darwin_test.go b/internal/sandboxrun/backend_darwin_test.go index 82f61470..e71ddca8 100644 --- a/internal/sandboxrun/backend_darwin_test.go +++ b/internal/sandboxrun/backend_darwin_test.go @@ -19,7 +19,18 @@ import ( // from a HOST-side lookup; handing sandbox-exec the path that lookup produced // is what keeps the two from disagreeing. func TestBuildChildArgvPassesTheInnerBinaryAsAnAbsolutePath(t *testing.T) { - dir := t.TempDir() + // Staged below the package directory, outside every shared temp root: + // BuildChildArgv bounds the derived grants by the writable surface, and + // a t.TempDir() fixture (under TMPDIR) would count as agent-writable + // and the derivation would be refused. + dir, err := os.MkdirTemp(".", "omac-darwin-argv-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(dir) }) + if dir, err = filepath.Abs(dir); err != nil { + t.Fatal(err) + } exe := filepath.Join(dir, "opencode") if err := os.WriteFile(exe, []byte("#!/bin/sh\n"), 0o755); err != nil { t.Fatal(err) @@ -132,7 +143,18 @@ func TestBuildChildArgvHomeOutsideWorkdir(t *testing.T) { // covers this layout — both the PATH-entry dir and the Cellar real dir must be // granted read AND map-executable, and the emitted argv must be absolute. func TestBuildChildArgvHomeRootedHomebrewPrefix(t *testing.T) { - home := t.TempDir() + // Staged below the package directory, outside every shared temp root: + // BuildChildArgv bounds the derived grants by the writable surface, and + // a t.TempDir() home (under TMPDIR) would count as agent-writable and + // the derivation would be refused. + home, err := os.MkdirTemp(".", "omac-darwin-home-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(home) }) + if home, err = filepath.Abs(home); err != nil { + t.Fatal(err) + } t.Setenv("HOME", home) cellarBin := filepath.Join(home, "homebrew", "Cellar", "opencode", "1.18.15", "bin") if err := os.MkdirAll(cellarBin, 0o755); err != nil { diff --git a/internal/sandboxrun/backend_linux.go b/internal/sandboxrun/backend_linux.go index 63a1c91d..c43fb5b7 100644 --- a/internal/sandboxrun/backend_linux.go +++ b/internal/sandboxrun/backend_linux.go @@ -260,7 +260,17 @@ func BuildChildArgv(g *Grants, innerArgv []string) ([]string, error) { // sibling files (shared libs, node runtime) are reachable too. Redundant // grants are harmless (bwrap re-binding under a parent bind is idempotent; // BuildBwrapArgv's dedupe drops only exact-duplicate paths, not overlaps). - gz.ReadPaths = append(gz.ReadPaths, resolveInnerBinaryDirs(innerArgv)...) + // + // The derivation is bounded by the grants (derivedGrantPolicyForGrants): + // the inner command frequently lives where the agent can write (the + // workdir, a temp dir), and granting read access picked by whatever the + // agent last wrote there would let the agent choose what the sandbox can + // read. Crossing the boundary fails the launch instead. + derived, err := resolveInnerBinaryDirs(innerArgv, derivedGrantPolicyForGrants(&gz)) + if err != nil { + return nil, err + } + gz.ReadPaths = append(gz.ReadPaths, derived...) // Rewrite argv[0] to the symlink-resolved real path so stage2 execs // the real binary directly instead of following a symlink chain inside diff --git a/internal/sandboxrun/backend_linux_test.go b/internal/sandboxrun/backend_linux_test.go index 27fb4afd..fe2f9a98 100644 --- a/internal/sandboxrun/backend_linux_test.go +++ b/internal/sandboxrun/backend_linux_test.go @@ -15,16 +15,24 @@ import ( ) func TestResolveInnerBinaryDirs(t *testing.T) { + // The zero policy keeps this a resolution-only test: with no grants to + // check against, nothing is refused and the output is unchanged. The + // backends own the policy enforcement. + resolve := func(argv []string) []string { + dirs, _ := resolveInnerBinaryDirs(argv, derivedGrantPolicy{}) + return dirs + } + // Empty / blank argv resolves to nothing. - if got := resolveInnerBinaryDirs(nil); got != nil { + if got := resolve(nil); got != nil { t.Errorf("nil argv = %v, want nil", got) } - if got := resolveInnerBinaryDirs([]string{""}); got != nil { + if got := resolve([]string{""}); got != nil { t.Errorf("blank argv[0] = %v, want nil", got) } // A name not on PATH resolves to nothing rather than guessing. - if got := resolveInnerBinaryDirs([]string{"definitely-not-a-real-binary-xyz"}); got != nil { + if got := resolve([]string{"definitely-not-a-real-binary-xyz"}); got != nil { t.Errorf("missing binary = %v, want nil", got) } @@ -60,7 +68,7 @@ func TestResolveInnerBinaryDirs(t *testing.T) { wantBoth := append([]string{shimDir, realDir}, interpDirs...) // Resolve via an absolute path to the shim (uses host PATH for sh). - if got := resolveInnerBinaryDirs([]string{shim}); !reflect.DeepEqual(got, wantBoth) { + if got := resolve([]string{shim}); !reflect.DeepEqual(got, wantBoth) { t.Errorf("symlinked binary dirs = %v, want %v", got, wantBoth) } @@ -69,7 +77,7 @@ func TestResolveInnerBinaryDirs(t *testing.T) { // path so the interpreter is still found via os.Stat, not PATH. t.Setenv("PATH", shimDir) wantPath := append([]string{shimDir, realDir}, interpDirs...) - if got := resolveInnerBinaryDirs([]string{"opencode"}); !reflect.DeepEqual(got, wantPath) { + if got := resolve([]string{"opencode"}); !reflect.DeepEqual(got, wantPath) { t.Errorf("PATH-resolved binary dirs = %v, want %v", got, wantPath) } @@ -83,7 +91,7 @@ func TestResolveInnerBinaryDirs(t *testing.T) { t.Fatal(err) } wantPlain := append([]string{plainDir}, interpDirs...) - if got := resolveInnerBinaryDirs([]string{plainBin}); !reflect.DeepEqual(got, wantPlain) { + if got := resolve([]string{plainBin}); !reflect.DeepEqual(got, wantPlain) { t.Errorf("plain binary dirs = %v, want %v", got, wantPlain) } @@ -92,7 +100,7 @@ func TestResolveInnerBinaryDirs(t *testing.T) { // command's dirs must still be granted, or a bun/npm-installed opencode // fails to exec inside the sandbox. Grant the shim via PATH lookup. t.Setenv("PATH", shimDir+string(os.PathListSeparator)+"/usr/bin:/bin") - got := resolveInnerBinaryDirs([]string{"env", "FOO=bar", "BAR=baz", "opencode"}) + got := resolve([]string{"env", "FOO=bar", "BAR=baz", "opencode"}) for _, want := range []string{shimDir, realDir} { if !slices.Contains(got, want) { t.Errorf("env-wrapped dirs = %v, missing wrapped-command dir %q", got, want) @@ -111,7 +119,18 @@ func TestBuildChildArgvGrantsEnvWrappedHarnessDir(t *testing.T) { // Shim in one dir, symlink target ("harness") in a different subtree, // mirroring bun (~/.bun/bin/opencode -> …/opencode-ai/bin/opencode.exe). - root := t.TempDir() + // Staged below the package directory, outside every shared temp root: + // BuildChildArgv bounds the derived grants by the writable surface, and + // a t.TempDir() fixture (under TMPDIR) would count as agent-writable + // and the derivation would be refused. + root, err := os.MkdirTemp(".", "omac-harness-dir-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(root) }) + if root, err = filepath.Abs(root); err != nil { + t.Fatal(err) + } realDir := filepath.Join(root, "pkg", "bin") if err := os.MkdirAll(realDir, 0o755); err != nil { t.Fatal(err) diff --git a/internal/sandboxrun/proxyinject_integration_linux_test.go b/internal/sandboxrun/proxyinject_integration_linux_test.go index 24ada7a0..9bd8ef58 100644 --- a/internal/sandboxrun/proxyinject_integration_linux_test.go +++ b/internal/sandboxrun/proxyinject_integration_linux_test.go @@ -188,7 +188,14 @@ func runToolThroughProxy(t *testing.T, omac string, proxy *netproxy.Server, inje // drift from it. g.ProxyPort = proxy.Port() g.ReadPaths = append(g.ReadPaths, filepath.Dir(omac)) - g.ReadPaths = append(g.ReadPaths, resolveInnerBinaryDirs([]string{tool})...) + // Zero policy: the tools are staged in temp workdirs (agent-writable by + // design), and this harness tests proxy env injection, not grant + // bounding — that is BuildChildArgv's job, exercised by its own tests. + derived, err := resolveInnerBinaryDirs([]string{tool}, derivedGrantPolicy{}) + if err != nil { + t.Fatal(err) + } + g.ReadPaths = append(g.ReadPaths, derived...) g.ReadPaths = append(g.ReadPaths, extraReads...) stage2 := append([]string{omac, "sandbox", "stage2"}, Stage2Args(g)...) diff --git a/internal/sandboxrun/resolve_binary.go b/internal/sandboxrun/resolve_binary.go index c0d8be96..d6af7c3a 100644 --- a/internal/sandboxrun/resolve_binary.go +++ b/internal/sandboxrun/resolve_binary.go @@ -1,6 +1,8 @@ package sandboxrun import ( + "errors" + "fmt" "os" "os/exec" "path/filepath" @@ -32,36 +34,64 @@ import ( // `env` itself — otherwise only `env`'s own directory would be granted and // the harness (e.g. a bun/npm-installed opencode) would fail to exec. // -// Returns nil when the command cannot be found or resolved. -func resolveInnerBinaryDirs(innerArgv []string) []string { - dirs := resolveCommandBinaryDirs(innerArgv) +// pol bounds what may be derived: every directory the resolution grants and +// every path it visits on the way is checked against it (see +// derivedGrantPolicy), so an inner command the confined process can rewrite +// cannot extend the next sandbox's read surface. The zero policy performs +// no checks — the backends pass one derived from the grants. +// +// Returns nil when the command cannot be found or resolved. An error means +// the resolution crossed the policy and the launch must fail closed. +func resolveInnerBinaryDirs(innerArgv []string, pol derivedGrantPolicy) ([]string, error) { + dirs, err := commandBinaryDirs(innerArgv, pol) + if err != nil { + return nil, err + } if cmd := UnwrapEnv(innerArgv); len(cmd) > 0 && cmd[0] != innerArgv[0] { - dirs = append(dirs, resolveCommandBinaryDirs(cmd)...) + wrapped, werr := commandBinaryDirs(cmd, pol) + if werr != nil { + return nil, werr + } + dirs = append(dirs, wrapped...) } + return dirs, nil +} + +// resolveCommandBinaryDirs is the unbounded form of commandBinaryDirs for +// callers that only inspect the resolution (diagnostics, tests): the +// policy is the zero value, so nothing is refused. Returns nil when argv +// is empty or unresolvable. +func resolveCommandBinaryDirs(argv []string) []string { + dirs, _ := commandBinaryDirs(argv, derivedGrantPolicy{}) return dirs } -// resolveCommandBinaryDirs is the core resolution for a single command argv +// commandBinaryDirs is the core resolution for a single command argv // (no env-wrapper handling): the directory of every hop of the symlink chain // from the PATH entry to the real file, plus shebang interpreter dirs. -// Returns nil when argv is empty or unresolvable. -func resolveCommandBinaryDirs(argv []string) []string { +// Returns nil when argv is empty or unresolvable. pol bounds the +// derivation; an error means it crossed the policy. +func commandBinaryDirs(argv []string, pol derivedGrantPolicy) ([]string, error) { if len(argv) == 0 { - return nil + return nil, nil } resolved := lookPathAbs(argv[0]) if resolved == "" { - return nil + return nil, nil } - dirs := symlinkChainDirs(resolved) + dirs, hops := walkSymlinkChain(resolved) if real, rerr := filepath.EvalSymlinks(resolved); rerr == nil { if interp := shebangInterpreter(real); interp != "" { - if idirs := resolveInterpreterDirs(interp); len(idirs) > 0 { - dirs = append(dirs, idirs...) - } + idirs, ihops := resolveInterpreterChain(interp) + dirs = append(dirs, idirs...) + hops = append(hops, ihops...) } } - return withPrefixSupportDirs(dirs) + dirs = withPrefixSupportDirs(dirs) + if err := refuseDerivedGrants(dirs, hops, pol); err != nil { + return nil, err + } + return dirs, nil } // maxSymlinkHops bounds the chain walk. Well above any real install layout @@ -69,9 +99,187 @@ func resolveCommandBinaryDirs(argv []string) []string { // cyclic or adversarial chain terminates here rather than spinning. const maxSymlinkHops = 32 -// symlinkChainDirs returns the directory of every hop of the symlink chain -// starting at path: the path itself, each intermediate link target, and the -// real file at the end. Duplicates are removed, order preserved. +// derivedGrantPolicy bounds what a per-launch derivation may grant. The +// inner command's resolution (commandBinaryDirs) hands every candidate +// directory and every path it visited to refuseDerivedGrants, which applies +// the two rules below. +// +// The zero policy performs no checks: legacy callers that only inspect a +// resolution (protectedInnerBinaryWarning, tests) have no grants to compare +// against. The backends pass one built from the grants, which always has +// at least the shared temp roots as writable. +type derivedGrantPolicy struct { + // writable holds the roots the confined process can change: write and + // allow grants plus the shared temp roots. A command, symlink hop, or + // shebang interpreter visited there is tainted: its content at launch + // time is whatever the agent last wrote, so it must not pick what the + // sandbox grants read access to. + writable []string + // granted holds the paths the profile already allows explicitly. + // Deriving a directory covered by them adds nothing the user has not + // already granted, so only the remainder (the extension) is subject + // to the rules. + granted []string + // homes holds the home directories a derivation must not silently + // extend into; see homeSensitiveDirs. + homes []string +} + +// derivedGrantPolicyForGrants builds the bounding policy from a grant set. +// The workdir is deliberately NOT treated as writable on its own: a +// read-only workdir cannot host a rewritten command, and ResolveGrants +// already maps writable workdirs into AllowPaths. +func derivedGrantPolicyForGrants(g *Grants) derivedGrantPolicy { + pol := derivedGrantPolicy{ + writable: append(append([]string{}, g.WritePaths...), g.AllowPaths...), + granted: append(append(append([]string{}, g.ReadPaths...), g.WritePaths...), g.AllowPaths...), + } + pol.writable = append(pol.writable, sharedTempRoots()...) + userHome, uerr := os.UserHomeDir() + if uerr != nil { + userHome = "" + } + for _, h := range []string{g.HomeDir, userHome} { + if h != "" && !slices.Contains(pol.homes, h) { + h = filepath.Clean(h) + if !slices.Contains(pol.homes, h) { + pol.homes = append(pol.homes, h) + } + } + } + return pol +} + +// sharedTempRoots returns the shared temp locations: the directory the +// process was configured with (TMPDIR and friends) plus the conventional +// defaults. Every process on the host can typically write there, so they +// count as agent-writable surface. +func sharedTempRoots() []string { + var roots []string + for _, r := range []string{os.TempDir(), "/tmp", "/var/tmp"} { + if r == "" { + continue + } + r = filepath.Clean(r) + if !slices.Contains(roots, r) { + roots = append(roots, r) + } + } + return roots +} + +// homeSensitiveDirs are the trees below a home directory that hold +// credentials or personal config: deriving read access into them off the +// back of a command resolution would expose secrets the profile never +// granted. Dot-cache is deliberately absent: version managers (mise and +// friends) keep their runtimes there, and those must keep resolving. +var homeSensitiveDirs = []string{ + ".config", ".ssh", ".gnupg", ".aws", ".azure", ".gcloud", ".kube", + ".docker", ".pki", ".terraform.d", ".credentials", ".secrets", ".keys", + ".password-store", ".1password", +} + +// refuseDerivedGrants applies the derivedGrantPolicy to a resolution: dirs +// is what the resolution wants to grant, hops is every path it visited on +// the way (the command file itself, each symlink target, each shebang +// interpreter). +// +// Two rules, in order: +// +// - Taint: a hop inside agent-writable surface means the command's +// content is attacker-chosen. Granting anything NEW on its say-so +// would let the agent pick what the sandbox reads, so the launch fails +// when the derivation extends past the explicit grants. It is the +// extension that is refused, not the derivation: a tainted command +// whose every dir is already granted stays launchable, because +// nothing new is exposed. +// - Home boundary: a NEW dir below a home's sensitive trees is refused +// even for untainted commands. A user's config root must be granted +// by the user, not derived. +// +// A nil policy (no writable surface, no homes) skips both rules. +func refuseDerivedGrants(dirs, hops []string, pol derivedGrantPolicy) error { + if len(pol.writable) == 0 && len(pol.homes) == 0 { + return nil + } + var ext []string + for _, d := range dirs { + if !pathCoveredByAny(d, pol.granted) { + ext = append(ext, d) + } + } + if len(ext) == 0 { + return nil + } + for _, hop := range hops { + if !pathCoveredByAny(hop, pol.writable) { + continue + } + var b strings.Builder + b.WriteString("omac can't start the sandbox: the inner command lives where the agent can write\n" + + "(the workdir or a temp dir), and launching it would have to grant read access to\n" + + "paths the profile does not already allow:\n") + for _, d := range ext { + fmt.Fprintf(&b, " %s\n", d) + } + b.WriteString("\n" + + "The command's content at launch time is whatever the agent last wrote there, so\n" + + "launching it must not choose what the sandbox can read. Move the command outside\n" + + "the agent-writable area, or grant the paths it needs explicitly (filesystem.read /\n" + + "--read).") + return errors.New(b.String()) + } + for _, d := range ext { + if home, sensitive := sensitiveUnderHome(d, pol.homes); sensitive { + //lint:ignore ST1005 multi-paragraph user-facing message, punctuation is intentional + return fmt.Errorf( + "omac can't start the sandbox: launching this command would derive read access\n"+ + "to %s\n"+ + "below the home directory %s, in a tree that holds config or credentials. omac\n"+ + "only derives access there when you grant it explicitly (filesystem.read / --read)\n"+ + "or move the tool outside the home tree.", d, home) + } + } + return nil +} + +// sensitiveUnderHome reports whether dir sits below one of a home +// directory's sensitive trees, returning that home when it does. +func sensitiveUnderHome(dir string, homes []string) (string, bool) { + for _, home := range homes { + rel, err := filepath.Rel(home, dir) + if err != nil || rel == ".." || rel == "." || + strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + continue + } + first := rel + if i := strings.IndexByte(rel, filepath.Separator); i >= 0 { + first = rel[:i] + } + if slices.Contains(homeSensitiveDirs, first) { + return home, true + } + } + return "", false +} + +// pathCoveredByAny reports whether path lies at or under any of roots, +// comparing every combination of literal and symlink-resolved forms. +func pathCoveredByAny(path string, roots []string) bool { + for _, r := range roots { + if pathCoveredBy(path, r) { + return true + } + } + return false +} + +// walkSymlinkChain walks the symlink chain starting at path and returns two +// lists: the directory of every hop (what the sandbox must be granted), and +// every path it visited on the way (the path itself, each intermediate link +// target, and the real file the chain ends at). The visited paths feed the +// derived-grant taint check: content the agent can rewrite at any hop makes +// the whole derivation untrustworthy. Duplicates removed, order preserved. // // Granting only the two ENDS of the chain — which is what taking Dir(path) // and Dir(EvalSymlinks(path)) amounts to — is not enough, because the kernel @@ -92,21 +300,28 @@ const maxSymlinkHops = 32 // chain instead of hardening argv[0] alone: passing an absolute path would // only have moved the failure from ENOENT to EPERM. // -// Note it is the DIRECTORIES that are returned, not the links: a subpath +// Note the first list holds the DIRECTORIES, not the links: a subpath // grant on the directory is what lets the kernel both read the link and stat // its siblings, matching how the rest of this file grants access. -func symlinkChainDirs(path string) []string { - var dirs []string - seen := map[string]bool{} - add := func(d string) { - if !seen[d] { - seen[d] = true +func walkSymlinkChain(path string) (dirs, hops []string) { + seenDirs := map[string]bool{} + seenHops := map[string]bool{} + addDir := func(d string) { + if !seenDirs[d] { + seenDirs[d] = true dirs = append(dirs, d) } } + addHop := func(p string) { + if !seenHops[p] { + seenHops[p] = true + hops = append(hops, p) + } + } cur := path for range maxSymlinkHops { - add(filepath.Dir(cur)) + addHop(cur) + addDir(filepath.Dir(cur)) fi, err := os.Lstat(cur) if err != nil || fi.Mode()&os.ModeSymlink == 0 { break @@ -125,8 +340,17 @@ func symlinkChainDirs(path string) []string { // never produces. EvalSymlinks on the end of the chain closes that gap; // it is a no-op for the common all-real-directories case. if real, rerr := filepath.EvalSymlinks(path); rerr == nil { - add(filepath.Dir(real)) + addHop(real) + addDir(filepath.Dir(real)) } + return dirs, hops +} + +// symlinkChainDirs is walkSymlinkChain's directory list alone, for callers +// that only inspect the grant surface (diagnostics, tests) and have no +// policy to check the visited paths against. +func symlinkChainDirs(path string) []string { + dirs, _ := walkSymlinkChain(path) return dirs } @@ -288,8 +512,12 @@ func unresolvedInnerCommandWarning(innerArgv []string) string { // quietly routing around it. Silently resolving the binary out of the denied // tree would launch it in defiance of the profile, which is not omac's call // to make in a security boundary. +// +// The zero policy keeps this a pure inspection: it has no grant set to check +// against, and the launch-time enforcement happens in the backends. func protectedInnerBinaryWarning(innerArgv []string, protected []string) string { - for _, dir := range resolveInnerBinaryDirs(innerArgv) { + dirs, _ := resolveInnerBinaryDirs(innerArgv, derivedGrantPolicy{}) + for _, dir := range dirs { for _, p := range protected { if pathCoveredBy(dir, p) { return "omac sandbox: warning: this profile denies " + p + @@ -390,22 +618,26 @@ func shebangInterpreter(path string) string { return interp } -// resolveInterpreterDirs resolves an interpreter (bare name like "node" or +// resolveInterpreterChain resolves an interpreter (bare name like "node" or // absolute path like "/usr/bin/node") on the host and returns the dirs to -// grant: the interpreter's dir and its symlink-resolved dir. -func resolveInterpreterDirs(interp string) []string { +// grant: the interpreter's dir and its symlink-resolved dir. The second list +// holds every path the resolution visited — the interpreter file, each +// symlink hop to it, the real file at the end — which feeds the derived-grant +// taint check (see derivedGrantPolicy). +func resolveInterpreterChain(interp string) (dirs, hops []string) { resolved, err := exec.LookPath(interp) if err != nil { if filepath.IsAbs(interp) { - if _, err := os.Stat(interp); err == nil { + if _, serr := os.Stat(interp); serr == nil { + hops = []string{interp} d := filepath.Dir(interp) if isBroadGrant(d) { - return nil + return nil, hops } - return withPrefixSupportDirs([]string{d}) + return withPrefixSupportDirs([]string{d}), hops } } - return nil + return nil, nil } if abs, aerr := filepath.Abs(resolved); aerr == nil { resolved = abs @@ -413,8 +645,17 @@ func resolveInterpreterDirs(interp string) []string { // Chain-walked for the same reason the harness itself is: an interpreter // reached through a version manager (nvm, mise, asdf) is a multi-hop // symlink, and a middle hop left ungranted fails the exec of every script - // that names it — see symlinkChainDirs. - return withPrefixSupportDirs(symlinkChainDirs(resolved)) + // that names it — see walkSymlinkChain. + idirs, ihops := walkSymlinkChain(resolved) + return withPrefixSupportDirs(idirs), ihops +} + +// resolveInterpreterDirs is resolveInterpreterChain's directory list alone, +// for callers that only inspect the grant surface (diagnostics, tests) and +// have no policy to check the visited paths against. +func resolveInterpreterDirs(interp string) []string { + dirs, _ := resolveInterpreterChain(interp) + return dirs } // prefixBinDirNames are the conventional executable subdirectories of a Unix diff --git a/internal/sandboxrun/resolve_binary_chain_test.go b/internal/sandboxrun/resolve_binary_chain_test.go index 5ca344b3..b88e7d0c 100644 --- a/internal/sandboxrun/resolve_binary_chain_test.go +++ b/internal/sandboxrun/resolve_binary_chain_test.go @@ -55,7 +55,8 @@ func TestResolveInnerBinaryDirsGrantsEverySymlinkHop(t *testing.T) { root := t.TempDir() head := writeHomebrewChain(t, root) - got := resolveInnerBinaryDirs([]string{head}) + // Zero policy: this tests the resolution itself, not grant bounding. + got, _ := resolveInnerBinaryDirs([]string{head}, derivedGrantPolicy{}) cellar := filepath.Join(root, "Cellar", "opencode", "1.18.15") for _, want := range []string{ @@ -76,7 +77,7 @@ func TestResolveInnerBinaryDirsGrantsEverySymlinkHopViaPATH(t *testing.T) { writeHomebrewChain(t, root) t.Setenv("PATH", filepath.Join(root, "bin")) - got := resolveInnerBinaryDirs([]string{"opencode"}) + got, _ := resolveInnerBinaryDirs([]string{"opencode"}, derivedGrantPolicy{}) want := filepath.Join(root, "Cellar", "opencode", "1.18.15", "bin") if !slices.Contains(got, want) { diff --git a/internal/sandboxrun/resolve_binary_prefix_test.go b/internal/sandboxrun/resolve_binary_prefix_test.go index 362bb4c7..0de551d9 100644 --- a/internal/sandboxrun/resolve_binary_prefix_test.go +++ b/internal/sandboxrun/resolve_binary_prefix_test.go @@ -40,7 +40,8 @@ func TestResolveInnerBinaryDirsGrantsInstallPrefixSupportDirs(t *testing.T) { root := t.TempDir() exe := writeInstallPrefix(t, root, "bin", "java", "lib", "conf") - got := resolveInnerBinaryDirs([]string{exe}) + // Zero policy: this tests the resolution itself, not grant bounding. + got, _ := resolveInnerBinaryDirs([]string{exe}, derivedGrantPolicy{}) for _, want := range []string{ filepath.Join(root, "bin"), @@ -59,7 +60,7 @@ func TestResolveInnerBinaryDirsSkipsAbsentSupportDirs(t *testing.T) { root := t.TempDir() exe := writeInstallPrefix(t, root, "bin", "tool", "lib") - got := resolveInnerBinaryDirs([]string{exe}) + got, _ := resolveInnerBinaryDirs([]string{exe}, derivedGrantPolicy{}) if !slices.Contains(got, filepath.Join(root, "lib")) { t.Errorf("existing lib missing from %v", got) @@ -79,7 +80,7 @@ func TestResolveInnerBinaryDirsNeverTreatsHomeAsInstallPrefix(t *testing.T) { t.Setenv("HOME", home) exe := writeInstallPrefix(t, home, "bin", "tool", "lib", "conf") - got := resolveInnerBinaryDirs([]string{exe}) + got, _ := resolveInnerBinaryDirs([]string{exe}, derivedGrantPolicy{}) for _, forbidden := range []string{home, filepath.Join(home, "lib"), filepath.Join(home, "conf")} { if slices.Contains(got, forbidden) { @@ -97,7 +98,7 @@ func TestResolveInnerBinaryDirsNeverGrantsPrefixRoot(t *testing.T) { root := t.TempDir() exe := writeInstallPrefix(t, root, "bin", "tool", "lib", "share") - got := resolveInnerBinaryDirs([]string{exe}) + got, _ := resolveInnerBinaryDirs([]string{exe}, derivedGrantPolicy{}) if slices.Contains(got, root) { t.Errorf("prefix root granted: %v", got) @@ -113,7 +114,7 @@ func TestResolveInnerBinaryDirsIgnoresNonBinParent(t *testing.T) { root := t.TempDir() exe := writeInstallPrefix(t, root, "opt", "tool", "lib") - got := resolveInnerBinaryDirs([]string{exe}) + got, _ := resolveInnerBinaryDirs([]string{exe}, derivedGrantPolicy{}) if p := filepath.Join(root, "lib"); slices.Contains(got, p) { t.Errorf("derived a prefix from a non-bin parent: %v", got) From d65a666ca56862231fd494a7add05d5167af95fa Mon Sep 17 00:00:00 2001 From: Fix Writer Agent Date: Sat, 31 Oct 2026 09:53:57 +0000 Subject: [PATCH 4/5] fix(security): Filesystem grants derived from the inner command must not follow interpreter or symlink content that lives where the confined process can write, and each derived directory needs a bounded taint decision. Signed-off-by: Fix Writer Agent --- .../derived_grants_security_test.go | 52 +++++++++++++++- internal/sandboxrun/resolve_binary.go | 60 +++++++++++++------ 2 files changed, 92 insertions(+), 20 deletions(-) diff --git a/internal/sandboxrun/derived_grants_security_test.go b/internal/sandboxrun/derived_grants_security_test.go index 41e17662..897368f1 100644 --- a/internal/sandboxrun/derived_grants_security_test.go +++ b/internal/sandboxrun/derived_grants_security_test.go @@ -180,7 +180,9 @@ func TestSecurityDerivedGrantsRefuseAgentWritableInnerCommand(t *testing.T) { // ends in trusted content; a chain entirely outside the writable // surface keeps resolving the way version-manager shims do; and no // derivation may cross into the home/config boundary without an -// explicit user grant. +// explicit user grant. The boundary covers the home root itself and the +// Library tree, while unlisted first-level dirs (~/homebrew, ~/.local) +// must keep resolving: real shims live there. func TestSecuritySymlinkChainGrantBounded(t *testing.T) { requireSandboxBackend(t) @@ -229,4 +231,52 @@ func TestSecuritySymlinkChainGrantBounded(t *testing.T) { t.Errorf("spawn argv was built while deriving a grant below the user's config root %q; expected a refusal", secretDir) } }) + + t.Run("command file in the home root is refused", func(t *testing.T) { + tool := filepath.Join(home, "tool") + writeExecScript(t, tool, "#!/bin/sh\n") + if _, err := BuildChildArgv(g, []string{tool}); err == nil { + t.Errorf("spawn argv was built while deriving a grant for the home root %q itself; expected a refusal", home) + } + }) + + t.Run("symlink hop in the home root is refused", func(t *testing.T) { + target := filepath.Join(installs, "real-home") + writeExecScript(t, target, "#!/bin/sh\n") + hop := filepath.Join(home, "home-hop") + if err := os.Symlink(target, hop); err != nil { + t.Fatal(err) + } + if _, err := BuildChildArgv(g, []string{hop}); err == nil { + t.Errorf("spawn argv was built while deriving a grant for the home root %q from a symlink hop in it; expected a refusal", home) + } + }) + + t.Run("the Library tree needs an explicit grant", func(t *testing.T) { + libDir := filepath.Join(home, "Library", "app-support") + if err := os.MkdirAll(libDir, 0o755); err != nil { + t.Fatal(err) + } + tool := filepath.Join(libDir, "tool") + writeExecScript(t, tool, "#!/bin/sh\n") + if _, err := BuildChildArgv(g, []string{tool}); err == nil { + t.Errorf("spawn argv was built while deriving a grant for the user's Library tree %q; expected a refusal", libDir) + } + }) + + t.Run("unlisted first-level home dirs still resolve", func(t *testing.T) { + for _, first := range []string{"homebrew", ".local"} { + binDir := filepath.Join(home, first, "bin") + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatal(err) + } + tool := filepath.Join(binDir, "tool") + writeExecScript(t, tool, "#!/bin/sh\n") + argv, err := BuildChildArgv(g, []string{tool}) + if err != nil { + t.Fatalf("a tool below the unlisted first-level home dir %q must keep resolving, got: %v", binDir, err) + } + requireDerivedBind(t, argv, binDir) + } + }) } diff --git a/internal/sandboxrun/resolve_binary.go b/internal/sandboxrun/resolve_binary.go index d6af7c3a..0c613424 100644 --- a/internal/sandboxrun/resolve_binary.go +++ b/internal/sandboxrun/resolve_binary.go @@ -173,10 +173,15 @@ func sharedTempRoots() []string { // back of a command resolution would expose secrets the profile never // granted. Dot-cache is deliberately absent: version managers (mise and // friends) keep their runtimes there, and those must keep resolving. +// Library is the macOS user tree: Application Support holds app tokens +// and keychain material. The home directory itself never sits in this +// list because sensitiveUnderHome treats it as sensitive directly. All +// other first-level dirs (~/bin, ~/homebrew, unlisted dotdirs) stay +// derivable: they are legitimate shim locations. var homeSensitiveDirs = []string{ ".config", ".ssh", ".gnupg", ".aws", ".azure", ".gcloud", ".kube", ".docker", ".pki", ".terraform.d", ".credentials", ".secrets", ".keys", - ".password-store", ".1password", + ".password-store", ".1password", "Library", } // refuseDerivedGrants applies the derivedGrantPolicy to a resolution: dirs @@ -193,9 +198,10 @@ var homeSensitiveDirs = []string{ // extension that is refused, not the derivation: a tainted command // whose every dir is already granted stays launchable, because // nothing new is exposed. -// - Home boundary: a NEW dir below a home's sensitive trees is refused -// even for untainted commands. A user's config root must be granted -// by the user, not derived. +// - Home boundary: a NEW dir that is itself a home or sits below one +// of its sensitive trees is refused even for untainted commands. A +// user's home and its config/credential roots must be granted by the +// user, not derived. // // A nil policy (no writable surface, no homes) skips both rules. func refuseDerivedGrants(dirs, hops []string, pol derivedGrantPolicy) error { @@ -235,34 +241,50 @@ func refuseDerivedGrants(dirs, hops []string, pol derivedGrantPolicy) error { return fmt.Errorf( "omac can't start the sandbox: launching this command would derive read access\n"+ "to %s\n"+ - "below the home directory %s, in a tree that holds config or credentials. omac\n"+ - "only derives access there when you grant it explicitly (filesystem.read / --read)\n"+ + "inside the home tree %s, which holds config or credentials. omac only\n"+ + "derives access there when you grant it explicitly (filesystem.read / --read)\n"+ "or move the tool outside the home tree.", d, home) } } return nil } -// sensitiveUnderHome reports whether dir sits below one of a home -// directory's sensitive trees, returning that home when it does. +// sensitiveUnderHome reports whether dir is one of the homes itself or +// sits below one of a home's sensitive trees, returning that home when it +// does. Both sides are compared in their literal and symlink-resolved +// spellings: a derived dir can arrive from EvalSymlinks, while a home can +// be spelled the way the user wrote it. func sensitiveUnderHome(dir string, homes []string) (string, bool) { for _, home := range homes { - rel, err := filepath.Rel(home, dir) - if err != nil || rel == ".." || rel == "." || - strings.HasPrefix(rel, ".."+string(filepath.Separator)) { - continue - } - first := rel - if i := strings.IndexByte(rel, filepath.Separator); i >= 0 { - first = rel[:i] - } - if slices.Contains(homeSensitiveDirs, first) { - return home, true + for _, hf := range pathForms(home) { + for _, df := range pathForms(dir) { + if sensitiveBelowHome(df, hf) { + return home, true + } + } } } return "", false } +// sensitiveBelowHome reports whether dir is home itself or sits below +// one of home's sensitive trees (homeSensitiveDirs). +func sensitiveBelowHome(dir, home string) bool { + rel, err := filepath.Rel(home, dir) + if err != nil || rel == ".." || + strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return false + } + if rel == "." { + return true + } + first := rel + if i := strings.IndexByte(rel, filepath.Separator); i >= 0 { + first = rel[:i] + } + return slices.Contains(homeSensitiveDirs, first) +} + // pathCoveredByAny reports whether path lies at or under any of roots, // comparing every combination of literal and symlink-resolved forms. func pathCoveredByAny(path string, roots []string) bool { From 39c3c6abf4f7e0291438e5c38fdd0d876b0a6fe6 Mon Sep 17 00:00:00 2001 From: Sajjad Ahmad Date: Mon, 2 Nov 2026 15:18:21 +0100 Subject: [PATCH 5/5] fix(sandboxrun): preserve checked command paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retain required execution and interpreter spellings in the sandbox namespace. Root read/write authorization alone does not materialize those mounts. Refs #321 🤖 Generated with OpenCode Signed-off-by: Sajjad Ahmad --- internal/sandboxrun/backend_darwin.go | 4 +- internal/sandboxrun/backend_linux.go | 16 +- internal/sandboxrun/binary_namespace.go | 95 ++++ internal/sandboxrun/bwrap.go | 76 ++- .../sandboxrun/derived_grants_alias_test.go | 506 ++++++++++++++++++ .../derived_grants_namespace_linux_test.go | 151 ++++++ .../derived_grants_namespace_test.go | 452 ++++++++++++++++ internal/sandboxrun/grants.go | 6 + internal/sandboxrun/resolve_binary.go | 340 +++++++++--- internal/sandboxrun/sbpl.go | 20 +- 10 files changed, 1566 insertions(+), 100 deletions(-) create mode 100644 internal/sandboxrun/binary_namespace.go create mode 100644 internal/sandboxrun/derived_grants_alias_test.go create mode 100644 internal/sandboxrun/derived_grants_namespace_linux_test.go create mode 100644 internal/sandboxrun/derived_grants_namespace_test.go diff --git a/internal/sandboxrun/backend_darwin.go b/internal/sandboxrun/backend_darwin.go index 0fb128ee..c0c2bf9d 100644 --- a/internal/sandboxrun/backend_darwin.go +++ b/internal/sandboxrun/backend_darwin.go @@ -31,7 +31,7 @@ func CheckPlatform() error { // harness binary (e.g. claude, codex) must have its directory granted // as readable — (deny default) blocks exec otherwise. We resolve the // binary on the host PATH and add its dir (and symlink-resolved dir) -// to ReadPaths before generating the SBPL, mirroring backend_linux.go. +// to the per-launch read rules before generating the SBPL, mirroring Linux. // // argv[0] is handed to sandbox-exec as an absolute path (see // absoluteInnerArgv). A bare name would make sandbox-exec's execvp redo @@ -64,7 +64,7 @@ func BuildChildArgv(g *Grants, innerArgv []string) ([]string, error) { if err != nil { return nil, err } - gz.ReadPaths = append(append([]string{}, g.ReadPaths...), derived...) + gz.derivedReadPaths = derived profile := GenerateSBPL(&gz) argv := []string{sandboxExecPath, "-p", profile, "--"} argv = append(argv, absoluteInnerArgv(innerArgv)...) diff --git a/internal/sandboxrun/backend_linux.go b/internal/sandboxrun/backend_linux.go index c43fb5b7..c7f0fc76 100644 --- a/internal/sandboxrun/backend_linux.go +++ b/internal/sandboxrun/backend_linux.go @@ -257,28 +257,24 @@ func BuildChildArgv(g *Grants, innerArgv []string) ([]string, error) { // host PATH (the same lookup `which opencode` performs) and grant both // the PATH-entry (shim) directory and the symlink-resolved real // directory read-only, so the shim is found on PATH and its target plus - // sibling files (shared libs, node runtime) are reachable too. Redundant - // grants are harmless (bwrap re-binding under a parent bind is idempotent; - // BuildBwrapArgv's dedupe drops only exact-duplicate paths, not overlaps). + // sibling files (shared libs, node runtime) are reachable too. // // The derivation is bounded by the grants (derivedGrantPolicyForGrants): // the inner command frequently lives where the agent can write (the // workdir, a temp dir), and granting read access picked by whatever the // agent last wrote there would let the agent choose what the sandbox can // read. Crossing the boundary fails the launch instead. - derived, err := resolveInnerBinaryDirs(innerArgv, derivedGrantPolicyForGrants(&gz)) + derived, names, err := resolveInnerBinaryPlan(innerArgv, derivedGrantPolicyForGrants(&gz)) if err != nil { return nil, err } - gz.ReadPaths = append(gz.ReadPaths, derived...) + gz.derivedReadPaths = derived + gz.derivedReadDirs = names // Rewrite argv[0] to the symlink-resolved real path so stage2 execs // the real binary directly instead of following a symlink chain inside - // the sandbox. This avoids granting read access to intermediate - // directories in the chain (e.g. ~/.codex/packages/standalone/current/ - // when ~/.local/bin/codex -> ~/.codex/.../current/bin/codex -> - // ~/.codex/.../releases/.../bin/codex). Only the binary's final - // directory is granted (via resolveInnerBinaryDirs above). + // the sandbox. The checked derivation above covers any additional + // directories required by the original path. resolvedArgv := make([]string, len(innerArgv)) copy(resolvedArgv, innerArgv) if p := resolveInnerBinaryPath(innerArgv); p != "" { diff --git a/internal/sandboxrun/binary_namespace.go b/internal/sandboxrun/binary_namespace.go new file mode 100644 index 00000000..f9a45920 --- /dev/null +++ b/internal/sandboxrun/binary_namespace.go @@ -0,0 +1,95 @@ +package sandboxrun + +import ( + "fmt" + "os" + "path/filepath" + "sort" +) + +// namespaceSymlink joins two sandbox pathnames. Neither name is a new host +// bind source: this cannot expose data outside the mounts already admitted. +type namespaceSymlink struct { + path string + target string +} + +// binaryNamespaceLinks restores execution spellings without rebinding mutable +// host aliases. In particular, a granted alias's host target need not itself +// be mounted, while an ungranted interpreter alias still needs its original +// spelling for the kernel's shebang lookup. +func binaryNamespaceLinks(dirs []derivedReadDir, mounts []*mount) ([]namespaceSymlink, error) { + dirs = append([]derivedReadDir(nil), dirs...) + sort.Slice(dirs, func(i, j int) bool { return dirs[i].path < dirs[j].path }) + var links []namespaceSymlink + for _, d := range dirs { + if namespaceHasDirectory(d.path, d.real, mounts, links) { + continue + } + // --dir/--symlink must only modify the fresh namespace, never a + // host-backed tree or a parent of an existing mount/link. + conflict := innermostMount(d.path, mounts) != nil || aboveAnyMount(d.path, mounts) + for _, link := range links { + conflict = conflict || isUnder(d.path, link.path) || isUnder(link.path, d.path) + } + if conflict { + return nil, fmt.Errorf("cannot restore command pathname %q inside an existing sandbox mount; grant the execution path explicitly", d.path) + } + target := "" + if namespaceHasDirectory(d.real, d.real, mounts, links) { + target = d.real + } else { + for _, m := range mounts { + root := canonicalPath(m.path) + if root == "" { + continue + } + if rel, ok := directoryRelativeTo(d.real, root); ok { + candidate := filepath.Join(m.path, rel) + if namespaceHasDirectory(candidate, d.real, mounts, links) { + target = candidate + break + } + } + } + } + if target == "" { + return nil, fmt.Errorf("checked command directory %q has no visible sandbox path; grant its execution path explicitly", d.real) + } + links = append(links, namespaceSymlink{path: d.path, target: target}) + } + return links, nil +} + +func namespaceHasDirectory(path, real string, mounts []*mount, links []namespaceSymlink) bool { + resolved, ok := sandboxResolveWithLinks(path, mounts, links) + if !ok || innermostMount(resolved, mounts) == nil { + return false + } + a, aerr := os.Stat(resolved) + b, berr := os.Stat(real) + return aerr == nil && berr == nil && a.IsDir() && os.SameFile(a, b) +} + +// directoryRelativeTo preserves case-sensitive identities, including when +// two spellings identify the same directory on a case-insensitive volume. +func directoryRelativeTo(path, root string) (string, bool) { + if isUnder(path, root) { + rel, err := filepath.Rel(root, path) + return rel, err == nil + } + info, err := os.Stat(root) + if err != nil { + return "", false + } + rel := "." + for p := path; ; p = filepath.Dir(p) { + if pi, err := os.Stat(p); err == nil && os.SameFile(pi, info) { + return rel, true + } + if p == filepath.Dir(p) { + return "", false + } + rel = filepath.Join(filepath.Base(p), rel) + } +} diff --git a/internal/sandboxrun/bwrap.go b/internal/sandboxrun/bwrap.go index b9d2dca4..a9bb0352 100644 --- a/internal/sandboxrun/bwrap.go +++ b/internal/sandboxrun/bwrap.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "sort" "strconv" "strings" @@ -75,6 +76,15 @@ func BuildBwrapArgv(g *Grants, stage2Argv []string) ([]string, error) { for _, p := range g.ReadPaths { add(p, false) } + for _, p := range g.derivedReadPaths { + // Omit a checked derived bind only when an explicit grant will actually + // be materialized in this namespace. Root read/write authorize host + // paths but are intentionally not bwrap binds, so they cannot cover an + // execution pathname here. + if !targetCoveredByAny(p, linuxMaterializedGrantRoots(g)) { + add(p, false) + } + } for _, p := range g.WritePaths { add(p, true) } @@ -154,6 +164,17 @@ func BuildBwrapArgv(g *Grants, stage2Argv []string) ([]string, error) { } argv = append(argv, flag, m.path, m.path) } + var links []namespaceSymlink + if !rootGranted { + var err error + links, err = binaryNamespaceLinks(g.derivedReadDirs, ordered) + if err != nil { + return nil, err + } + for _, link := range links { + argv = append(argv, "--dir", filepath.Dir(link.path), "--symlink", link.target, link.path) + } + } // Protected-path masking: when rootGranted every protected path is // covered; otherwise check against the ordered mount list. @@ -165,8 +186,13 @@ func BuildBwrapArgv(g *Grants, stage2Argv []string) ([]string, error) { // marker dir holding a single .omac-denied file. When no markers are // prepared, the historical /dev/null + empty-tmpfs behavior applies. for _, prot := range g.ProtectedPaths { + dest := prot if !rootGranted && !coveredByAny(prot, ordered) { - continue + var ok bool + dest, ok = sandboxResolveWithLinks(prot, ordered, links) + if !ok { + continue + } } fi, err := os.Lstat(prot) if err != nil { @@ -174,15 +200,15 @@ func BuildBwrapArgv(g *Grants, stage2Argv []string) ([]string, error) { } if fi.IsDir() { if g.markerDir != "" { - argv = append(argv, "--ro-bind", g.markerDir, prot) + argv = append(argv, "--ro-bind", g.markerDir, dest) } else { - argv = append(argv, "--tmpfs", prot) + argv = append(argv, "--tmpfs", dest) } } else { if g.markerFile != "" { - argv = append(argv, "--ro-bind", g.markerFile, prot) + argv = append(argv, "--ro-bind", g.markerFile, dest) } else { - argv = append(argv, "--ro-bind", "/dev/null", prot) + argv = append(argv, "--ro-bind", "/dev/null", dest) } } } @@ -201,6 +227,21 @@ func BuildBwrapArgv(g *Grants, stage2Argv []string) ([]string, error) { return argv, nil } +// linuxMaterializedGrantRoots returns the explicit grants that become bwrap +// mounts. Root allow is mounted to implement learn mode; root read/write are +// authorization-only and must not suppress a narrower derived execution bind. +func linuxMaterializedGrantRoots(g *Grants) []string { + roots := make([]string, 0, len(g.ReadPaths)+len(g.WritePaths)+len(g.AllowPaths)) + for _, paths := range [][]string{g.ReadPaths, g.WritePaths, g.AllowPaths} { + for _, p := range paths { + if filepath.Clean(p) != "/" || slices.Contains(g.AllowPaths, p) { + roots = append(roots, p) + } + } + } + return roots +} + // mount is one bwrap bind entry. type mount struct { path string @@ -285,6 +326,10 @@ func redundantLinkMount(m *mount, mounts []*mount) bool { // grant on /work alone fails here although the host resolves it. Returns // the resolved path, spelled as the sandbox sees it, and whether it exists. func sandboxResolve(p string, mounts []*mount) (string, bool) { + return sandboxResolveWithLinks(p, mounts, nil) +} + +func sandboxResolveWithLinks(p string, mounts []*mount, links []namespaceSymlink) (string, bool) { cur := "/" rest := strings.Split(filepath.Clean(p), "/") for hops := 0; len(rest) > 0; { @@ -298,9 +343,28 @@ func sandboxResolve(p string, mounts []*mount) (string, bool) { continue } next := filepath.Join(cur, name) + var linkTarget string + for _, link := range links { + if link.path == next { + linkTarget = link.target + break + } + } + if linkTarget != "" { + if hops++; hops > maxSymlinkHops { + return "", false + } + cur = "/" + rest = append(strings.Split(linkTarget, "/"), rest...) + continue + } in := innermostMount(next, mounts) if in == nil { - if !aboveAnyMount(next, mounts) { + aboveLink := false + for _, link := range links { + aboveLink = aboveLink || isUnder(link.path, next) + } + if !aboveAnyMount(next, mounts) && !aboveLink { return "", false } cur = next diff --git a/internal/sandboxrun/derived_grants_alias_test.go b/internal/sandboxrun/derived_grants_alias_test.go new file mode 100644 index 00000000..9139b691 --- /dev/null +++ b/internal/sandboxrun/derived_grants_alias_test.go @@ -0,0 +1,506 @@ +package sandboxrun + +import ( + "fmt" + "os" + "path/filepath" + "runtime" + "slices" + "strings" + "testing" + + "github.com/TNG/oh-my-agentic-coder/internal/sandboxprofile" +) + +// These fixtures live outside temp roots unless the test deliberately marks +// them writable, so refusals cannot accidentally pass for temp-directory taint. +func review446Root(t *testing.T) string { + t.Helper() + p, err := os.MkdirTemp(".", "review446-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(p) }) + p, err = filepath.Abs(p) + if err != nil { + t.Fatal(err) + } + return p +} + +func review446Script(t *testing.T, path string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatal(err) + } +} + +func TestDerivedGrantsWritableSupportAlias(t *testing.T) { + home, wd := stableHome(t), t.TempDir() + secret := filepath.Join(home, ".config", "private") + if err := os.MkdirAll(secret, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(secret, "token"), []byte("secret"), 0o600); err != nil { + t.Fatal(err) + } + tool := filepath.Join(wd, "prefix", "bin", "tool") + review446Script(t, tool) + lib := filepath.Join(wd, "prefix", "lib") + if err := os.Symlink(secret, lib); err != nil { + t.Fatal(err) + } + p := sandboxprofile.DefaultProfile() + p.Network.Mode = sandboxprofile.ModeOpen + g, err := ResolveGrants(p, wd, nil) + if err != nil { + t.Fatal(err) + } + dirs, err := resolveInnerBinaryDirs([]string{tool}, derivedGrantPolicyForGrants(g)) + if err == nil { + copy := *g + copy.ReadPaths = append(copy.ReadPaths, dirs...) + needle := "(allow file-read* (subpath " + sbplQuote(secret) + "))" + t.Logf("before exposes secret=%v; after exposes secret=%v; dirs=%v", strings.Contains(GenerateSBPL(g), needle), strings.Contains(GenerateSBPL(©), needle), dirs) + if runtime.GOOS == "darwin" { + argv, spawnErr := BuildChildArgv(g, []string{tool}) + t.Logf("actual BuildChildArgv err=%v; grants secret=%v", spawnErr, len(argv) > 2 && strings.Contains(argv[2], needle)) + } + t.Fatal("a workdir lib symlink is not an explicit grant of its sensitive target") + } +} + +func TestDerivedGrantsSupportAliasReplacedAfterCheck(t *testing.T) { + home, wd := stableHome(t), t.TempDir() + secret := filepath.Join(home, ".config", "private") + benign := filepath.Join(wd, "benign") + for _, dir := range []string{secret, benign} { + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + } + tool := filepath.Join(wd, "prefix", "bin", "tool") + review446Script(t, tool) + lib := filepath.Join(wd, "prefix", "lib") + if err := os.Symlink(benign, lib); err != nil { + t.Fatal(err) + } + p := sandboxprofile.DefaultProfile() + p.Network.Mode = sandboxprofile.ModeOpen + g, err := ResolveGrants(p, wd, nil) + if err != nil { + t.Fatal(err) + } + dirs, err := resolveInnerBinaryDirs([]string{tool}, derivedGrantPolicyForGrants(g)) + if err != nil { + t.Fatalf("initially fully covered command must be allowed: %v", err) + } + if err := os.Remove(lib); err != nil { + t.Fatal(err) + } + if err := os.Symlink(secret, lib); err != nil { + t.Fatal(err) + } + copy := *g + copy.ReadPaths = append(copy.ReadPaths, dirs...) + if strings.Contains(GenerateSBPL(©), "(allow file-read* (subpath "+sbplQuote(secret)+"))") { + t.Fatal("validated benign support alias was re-resolved to a sensitive target during backend lowering") + } +} + +func TestDerivedGrantsWritableAncestorSymlink(t *testing.T) { + for _, kind := range []string{"command", "interpreter", "support"} { + t.Run(kind, func(t *testing.T) { + root, wd := review446Root(t), t.TempDir() + home := filepath.Join(root, "home") + if err := os.Mkdir(home, 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("HOME", home) + outside := filepath.Join(root, "ungranted") + review446Script(t, filepath.Join(outside, "tool")) + if err := os.WriteFile(filepath.Join(outside, "private-data"), []byte("secret"), 0o600); err != nil { + t.Fatal(err) + } + redirect := filepath.Join(wd, "redirect") + if err := os.Symlink(outside, redirect); err != nil { + t.Fatal(err) + } + alias := filepath.Join(root, "alias") + if err := os.Symlink(redirect, alias); err != nil { + t.Fatal(err) + } + tool := filepath.Join(alias, "tool") + if kind == "interpreter" { + tool = filepath.Join(root, "trusted", "tool") + review446Script(t, tool) + if err := os.WriteFile(tool, []byte("#!"+filepath.Join(alias, "tool")+"\n"), 0o755); err != nil { + t.Fatal(err) + } + } + if kind == "support" { + tool = filepath.Join(root, "prefix", "bin", "tool") + review446Script(t, tool) + if err := os.Symlink(redirect, filepath.Join(root, "prefix", "lib")); err != nil { + t.Fatal(err) + } + } + g := minimalDerivedGrants(wd, home) + dirs, err := resolveInnerBinaryDirs([]string{tool}, derivedGrantPolicyForGrants(g)) + if err == nil { + copy := *g + copy.ReadPaths = append(copy.ReadPaths, dirs...) + profile := GenerateSBPL(©) + bwrap, berr := BuildBwrapArgv(©, []string{"/bin/sh"}) + t.Logf("derived=%v; SBPL exposes private-data parent=%v; bwrap err=%v argv=%v", dirs, strings.Contains(profile, "(allow file-read* (subpath "+sbplQuote(outside)+"))"), berr, bwrap) + if runtime.GOOS == "darwin" { + argv, spawnErr := BuildChildArgv(g, []string{tool}) + t.Logf("actual BuildChildArgv err=%v; grants outside=%v", spawnErr, len(argv) > 2 && strings.Contains(argv[2], "(allow file-read* (subpath "+sbplQuote(outside)+"))")) + } + t.Fatalf("must reject %s resolution through agent-replaceable ancestor %s", kind, redirect) + } + }) + } +} + +func TestDerivedGrantsSensitiveHomeAncestor(t *testing.T) { + root := review446Root(t) + home := filepath.Join(root, "home") + review446Script(t, filepath.Join(home, ".config", "private", "tool")) + t.Setenv("HOME", home) + tool := filepath.Join(root, "tool") + review446Script(t, tool) + g := minimalDerivedGrants(t.TempDir(), home) + dirs, err := resolveInnerBinaryDirs([]string{tool}, derivedGrantPolicyForGrants(g)) + if err == nil && slices.Contains(dirs, root) { + t.Fatalf("derived home ancestor %s, exposing %s recursively without an explicit grant", root, filepath.Join(home, ".config")) + } +} + +func TestDerivedGrantsExplicitRootGrant(t *testing.T) { + home := stableHome(t) + wd := t.TempDir() + interp := filepath.Join(home, "custom-runtime", "interp") + review446Script(t, interp) + p := sandboxprofile.DefaultProfile() + p.Network.Mode = sandboxprofile.ModeOpen + base, err := ResolveGrants(p, wd, nil) + if err != nil { + t.Fatal(err) + } + for _, tool := range []string{filepath.Join(home, ".config", "private", "tool"), filepath.Join(wd, "tool")} { + review446Script(t, tool) + if filepath.Dir(tool) == wd { + if err := os.WriteFile(tool, []byte("#!"+interp+"\n"), 0o755); err != nil { + t.Fatal(err) + } + } + g := base.withUnrestrictedFilesystem() + if _, err := resolveInnerBinaryDirs([]string{tool}, derivedGrantPolicyForGrants(g)); err != nil { + t.Errorf("explicit '/' (also learn mode) already grants every directory and must not refuse %s: %v", tool, err) + } + } +} + +func TestDerivedGrantsSensitiveCaseAlias(t *testing.T) { + home := stableHome(t) + tool := filepath.Join(home, ".config", "private", "tool") + review446Script(t, tool) + alias := filepath.Join(home, ".CONFIG", "private", "tool") + if _, err := os.Stat(alias); os.IsNotExist(err) { + t.Skip("filesystem is case-sensitive") + } else if err != nil { + t.Fatal(err) + } + canonical, err := filepath.EvalSymlinks(alias) + if err != nil { + t.Fatal(err) + } + t.Logf("case alias resolves as %s", canonical) + g := minimalDerivedGrants(t.TempDir(), home) + if _, err := resolveInnerBinaryDirs([]string{tool}, derivedGrantPolicyForGrants(g)); err == nil { + t.Fatal("control: ordinary lowercase .config must be refused") + } + if dirs, err := resolveInnerBinaryDirs([]string{alias}, derivedGrantPolicyForGrants(g)); err == nil { + t.Fatalf("case alias of sensitive .config accepted: %v", dirs) + } +} + +func TestDerivedGrantsCompatibilityControls(t *testing.T) { + home, wd := stableHome(t), t.TempDir() + node := filepath.Join(home, ".nvm", "versions", "node", "v24", "bin", "node") + review446Script(t, node) + t.Setenv("PATH", filepath.Dir(node)+":/usr/bin:/bin") + nvmTool := filepath.Join(home, "bin", "harness") + review446Script(t, nvmTool) + if err := os.WriteFile(nvmTool, []byte("#!/usr/bin/env node\n"), 0o755); err != nil { + t.Fatal(err) + } + pyenvTool := filepath.Join(home, ".pyenv", "versions", "3.14", "bin", "python") + review446Script(t, pyenvTool) + for _, d := range []string{"lib", "conf"} { + if err := os.MkdirAll(filepath.Join(filepath.Dir(filepath.Dir(pyenvTool)), d), 0o755); err != nil { + t.Fatal(err) + } + } + brew := writeHomebrewChain(t, filepath.Join(home, "homebrew")) + workTool := filepath.Join(wd, "tool") + review446Script(t, workTool) + sensitiveTool := filepath.Join(home, ".config", "private", "tool") + review446Script(t, sensitiveTool) + p := sandboxprofile.DefaultProfile() + p.Network.Mode = sandboxprofile.ModeOpen + g, err := ResolveGrants(p, wd, nil) + if err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + name string + argv []string + }{ + {"nvm shebang", []string{nvmTool}}, + {"env wrapper", []string{"env", "FOO=bar", nvmTool}}, + {"pyenv prefix", []string{pyenvTool}}, + {"homebrew chain", []string{brew}}, + {"writable workdir system shell", []string{workTool}}, + } { + t.Run(tc.name, func(t *testing.T) { + if _, err := resolveInnerBinaryDirs(tc.argv, derivedGrantPolicyForGrants(g)); err != nil { + t.Fatal(err) + } + if runtime.GOOS == "darwin" { + if _, err := BuildChildArgv(g, tc.argv); err != nil { + t.Fatal(err) + } + } + }) + } + t.Run("explicit sensitive dir grant", func(t *testing.T) { + g2 := *g + g2.ReadPaths = append(append([]string{}, g.ReadPaths...), filepath.Dir(sensitiveTool)) + if _, err := resolveInnerBinaryDirs([]string{sensitiveTool}, derivedGrantPolicyForGrants(&g2)); err != nil { + t.Fatal(err) + } + }) + t.Run("writable interpreter refused", func(t *testing.T) { + interp := filepath.Join(wd, "interp") + review446Script(t, interp) + if err := os.WriteFile(nvmTool, []byte("#!"+interp+"\n"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := resolveInnerBinaryDirs([]string{nvmTool}, derivedGrantPolicyForGrants(g)); err == nil { + t.Fatal("writable interpreter must not derive ungranted harness dir") + } + }) +} + +func TestDerivedGrantsRejectUnfinishedSupportResolution(t *testing.T) { + home, wd := stableHome(t), t.TempDir() + tool := filepath.Join(home, "runtime", "bin", "tool") + review446Script(t, tool) + prefix := filepath.Dir(filepath.Dir(tool)) + // A cycle used to be hidden by prefixSupportDirs' existence probe. + if err := os.Symlink("lib", filepath.Join(prefix, "lib")); err != nil { + t.Fatal(err) + } + g := minimalDerivedGrants(wd, home) + g.ReadPaths = []string{"/bin", "/usr"} + if _, err := resolveInnerBinaryDirs([]string{tool}, derivedGrantPolicyForGrants(g)); err == nil { + t.Fatal("must fail closed when a support directory cannot be resolved") + } +} + +func TestDerivedGrantsBoundAncestorSymlinkHops(t *testing.T) { + home := stableHome(t) + root := filepath.Join(home, "runtime") + review446Script(t, filepath.Join(root, "real", "tool")) + // Each component has a short chain, but the complete lookup exceeds + // the resolver's bound. An endpoint-only fallback must not admit it. + for i := 0; i < 34; i++ { + name := filepath.Join(root, fmt.Sprintf("link%d", i)) + target := "real" + if i < 33 { + target = fmt.Sprintf("link%d", i+1) + } + if err := os.Symlink(target, name); err != nil { + t.Fatal(err) + } + } + if err := os.MkdirAll(filepath.Join(root, "bin"), 0o755); err != nil { + t.Fatal(err) + } + tool := filepath.Join(root, "bin", "tool") + review446Script(t, tool) + if err := os.Symlink("link0", filepath.Join(root, "lib")); err != nil { + t.Fatal(err) + } + g := minimalDerivedGrants(t.TempDir(), home) + g.ReadPaths = []string{"/bin", "/usr"} + if _, err := resolveInnerBinaryDirs([]string{tool}, derivedGrantPolicyForGrants(g)); err == nil { + t.Fatal("must not derive grants after exceeding the support symlink hop bound") + } +} + +func TestDerivedGrantsExplicitRootReadAndWrite(t *testing.T) { + home := stableHome(t) + tool := filepath.Join(home, "runtime", "bin", "tool") + review446Script(t, tool) + g := minimalDerivedGrants(t.TempDir(), home) + g.ReadPaths = []string{"/"} + if _, err := resolveInnerBinaryDirs([]string{tool}, derivedGrantPolicyForGrants(g)); err != nil { + t.Fatalf("explicit root read already covers all targets: %v", err) + } + // Without readable coverage, root writability must taint even a + // command outside the workdir or temp roots. + pol := derivedGrantPolicy{writable: []string{"/"}} + if _, err := resolveInnerBinaryDirs([]string{tool}, pol); err == nil { + t.Fatal("root writability must taint the entire command resolution") + } +} + +func TestDerivedGrantsSensitiveHomeCaseAlias(t *testing.T) { + home := stableHome(t) + tool := filepath.Join(home, ".config", "private", "tool") + review446Script(t, tool) + aliasHome := filepath.Join(filepath.Dir(home), strings.ToUpper(filepath.Base(home))) + fi, err := os.Stat(aliasHome) + if os.IsNotExist(err) { + t.Skip("filesystem is case-sensitive") + } + if err != nil { + t.Fatal(err) + } + hi, err := os.Stat(home) + if err != nil { + t.Fatal(err) + } + if !os.SameFile(fi, hi) { + t.Fatal("fixture home alias does not identify the same directory") + } + g := minimalDerivedGrants(t.TempDir(), home) + if _, err := resolveInnerBinaryDirs([]string{filepath.Join(aliasHome, ".config", "private", "tool")}, derivedGrantPolicyForGrants(g)); err == nil { + t.Fatal("case alias of HOME must not hide its sensitive descendants") + } +} + +func TestDerivedGrantsPreserveCaseDistinctHomeDirs(t *testing.T) { + home := stableHome(t) + review446Script(t, filepath.Join(home, ".config", "private", "tool")) + alias := filepath.Join(home, ".CONFIG", "private", "tool") + if _, err := os.Stat(alias); err == nil { + t.Skip("filesystem is case-insensitive") + } else if !os.IsNotExist(err) { + t.Fatal(err) + } + review446Script(t, alias) + g := minimalDerivedGrants(t.TempDir(), home) + g.ReadPaths = []string{"/bin", "/usr"} + if _, err := resolveInnerBinaryDirs([]string{alias}, derivedGrantPolicyForGrants(g)); err != nil { + t.Fatalf("case-distinct nonsensitive directory must remain derivable: %v", err) + } +} + +func TestDerivedGrantsLoweringKeepsValidatedTargets(t *testing.T) { + home, wd := stableHome(t), t.TempDir() + tool := filepath.Join(home, "runtime", "tool") + review446Script(t, tool) + secret := filepath.Join(home, ".config", "private") + review446Script(t, filepath.Join(secret, "tool")) + g := minimalDerivedGrants(wd, home) + g.ReadPaths = []string{"/bin", "/usr"} + dirs, err := resolveInnerBinaryDirs([]string{tool}, derivedGrantPolicyForGrants(g)) + if err != nil { + t.Fatal(err) + } + g.derivedReadPaths = dirs + runtimeDir := filepath.Dir(tool) + needle := "(allow file-read* (subpath " + sbplQuote(runtimeDir) + "))" + if !strings.Contains(GenerateSBPL(g), needle) { + t.Fatal("control: admitted runtime must get a read rule") + } + // The agent cannot replace this untainted directory. This stronger + // phase check nevertheless ensures lowering emits the admitted target + // literally rather than interpreting a later host-side alias change. + if err := os.Rename(runtimeDir, runtimeDir+"-old"); err != nil { + t.Fatal(err) + } + if err := os.Symlink(secret, runtimeDir); err != nil { + t.Fatal(err) + } + profile := GenerateSBPL(g) + if !strings.Contains(profile, needle) { + t.Fatal("validated target disappeared during lowering") + } + if strings.Contains(profile, "(subpath "+sbplQuote(secret)+")") { + t.Fatal("lowering followed a validated target into an unchecked directory") + } + argv, err := BuildBwrapArgv(g, []string{"/bin/sh"}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(strings.Join(argv, " "), "--ro-bind "+runtimeDir+" "+runtimeDir) { + t.Fatal("Linux lowering must keep the admitted read-only target") + } + if slices.Contains(argv, secret) { + t.Fatal("Linux lowering expanded the admitted target") + } +} + +func TestDerivedGrantsSensitiveTreeTargetAncestor(t *testing.T) { + root := review446Root(t) + home := filepath.Join(root, "home") + if err := os.Mkdir(home, 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("HOME", home) + tool := filepath.Join(root, "runtime", "tool") + review446Script(t, tool) + secret := filepath.Join(root, "runtime", "private") + review446Script(t, filepath.Join(secret, "tool")) + if err := os.Symlink(secret, filepath.Join(home, ".config")); err != nil { + t.Fatal(err) + } + g := minimalDerivedGrants(t.TempDir(), home) + g.ReadPaths = []string{"/bin", "/usr"} + if _, err := resolveInnerBinaryDirs([]string{tool}, derivedGrantPolicyForGrants(g)); err == nil { + t.Fatal("a recursive grant must not contain a sensitive tree's external target") + } +} + +func TestDerivedGrantsExplicitWritePathKeepsCommandReadable(t *testing.T) { + home := stableHome(t) + tool := filepath.Join(home, "writable-runtime", "bin", "tool") + review446Script(t, tool) + g := minimalDerivedGrants(t.TempDir(), home) + g.ReadPaths = []string{"/bin", "/usr"} + g.WritePaths = []string{filepath.Dir(filepath.Dir(tool))} + dirs, err := resolveInnerBinaryDirs([]string{tool}, derivedGrantPolicyForGrants(g)) + if err != nil { + t.Fatal(err) + } + g.derivedReadPaths = dirs + if !strings.Contains(GenerateSBPL(g), "(allow file-read* (subpath "+sbplQuote(filepath.Dir(tool))+")") { + t.Fatal("a write-only grant bounds derivation but does not already grant macOS read access") + } + secret := filepath.Join(home, ".config", "private") + review446Script(t, filepath.Join(secret, "tool")) + if err := os.Rename(filepath.Dir(tool), filepath.Dir(tool)+"-old"); err != nil { + t.Fatal(err) + } + if err := os.Symlink(secret, filepath.Dir(tool)); err != nil { + t.Fatal(err) + } + if strings.Contains(GenerateSBPL(g), "(subpath "+sbplQuote(secret)+")") { + t.Fatal("macOS re-followed a mutable write-only path after admission") + } + argv, err := BuildBwrapArgv(g, []string{"/bin/sh"}) + if err != nil { + t.Fatal(err) + } + if slices.Contains(argv, filepath.Dir(tool)) || slices.Contains(argv, secret) { + t.Fatal("Linux added a redundant bind that can follow the replaced write-only descendant") + } +} diff --git a/internal/sandboxrun/derived_grants_namespace_linux_test.go b/internal/sandboxrun/derived_grants_namespace_linux_test.go new file mode 100644 index 00000000..0065f8b0 --- /dev/null +++ b/internal/sandboxrun/derived_grants_namespace_linux_test.go @@ -0,0 +1,151 @@ +//go:build linux + +package sandboxrun + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +// This is a real bwrap gate, not a namespace-model assertion. Keep it enabled +// on Linux CI; compilation on another OS is not evidence that these execs run. +func TestIntegrationDerivedGrantNamespaceExecution(t *testing.T) { + if err := CheckPlatform(); err != nil { + t.Skipf("live Linux namespace gate needs functional bwrap: %v", err) + } + for _, kind := range []string{"read", "write", "allow", "env", "shebang"} { + t.Run(kind, func(t *testing.T) { + home := stableHome(t) + realRoot := filepath.Join(home, "runtime") + realTool := filepath.Join(realRoot, "bin", "tool") + review446Script(t, realTool) + if err := os.WriteFile(realTool, []byte("#!/bin/sh\nprintf 'namespace-ok\\n'\n"), 0o755); err != nil { + t.Fatal(err) + } + alias := filepath.Join(home, "runtime-current") + if err := os.Symlink(realRoot, alias); err != nil { + t.Fatal(err) + } + tool := filepath.Join(alias, "bin", "tool") + g := minimalDerivedGrants(t.TempDir(), home) + for _, p := range []string{"/usr", "/bin", "/lib", "/lib64", "/etc"} { + if _, err := os.Stat(p); err == nil { + g.ReadPaths = append(g.ReadPaths, p) + } + } + inner := []string{tool} + required := tool + switch kind { + case "read": + g.ReadPaths = append(g.ReadPaths, alias) + case "write": + g.WritePaths = []string{alias} + case "allow": + g.AllowPaths = append(g.AllowPaths, alias) + case "env": + inner = []string{"/usr/bin/env", "FOO=bar", tool} + case "shebang": + outer := filepath.Join(home, "tools", "outer") + review446Script(t, outer) + if err := os.WriteFile(outer, []byte("#!"+tool+"\n"), 0o755); err != nil { + t.Fatal(err) + } + inner = []string{outer} + } + // Exercise production backend wiring as well as the shared lowerer. + backendArgv, err := BuildChildArgv(g, inner) + if err != nil { + t.Fatal(err) + } + mounts, links := namespaceFromBwrap(backendArgv) + if _, ok := sandboxResolveWithLinks(resolveInnerBinaryPath(inner), mounts, links); !ok { + t.Fatal("production backend's canonical executable is not visible") + } + if _, ok := sandboxResolveWithLinks(required, mounts, links); !ok { + t.Fatal("production backend omitted a required execution pathname") + } + planBinaryNamespace(t, g, inner) + stage := append([]string(nil), inner...) + stage[0] = resolveInnerBinaryPath(inner) + argv, err := BuildBwrapArgv(g, stage) + if err != nil { + t.Fatal(err) + } + out, err := exec.Command(argv[0], argv[1:]...).CombinedOutput() + if err != nil || string(out) != "namespace-ok\n" { + t.Fatalf("real %s exec: err=%v output=%q", kind, err, out) + } + }) + } +} + +// Keep these separate from the original five cases: each exercises a distinct +// checked-plan invariant that was previously lost during Linux lowering. +func TestIntegrationDerivedGrantNamespaceExecutionAdditional(t *testing.T) { + if err := CheckPlatform(); err != nil { + t.Skipf("live Linux namespace gate needs functional bwrap: %v", err) + } + for _, kind := range []string{"symlink-target-env", "symlink-target-shebang", "root-read", "root-write", "root-allow"} { + t.Run(kind, func(t *testing.T) { + home := stableHome(t) + realRoot := filepath.Join(home, "versions", "v1") + realTool := filepath.Join(realRoot, "bin", "tool") + review446Script(t, realTool) + if err := os.WriteFile(realTool, []byte("#!/bin/sh\nprintf 'namespace-ok\\n'\n"), 0o755); err != nil { + t.Fatal(err) + } + g := minimalDerivedGrants(t.TempDir(), home) + for _, p := range []string{"/usr", "/bin", "/lib", "/lib64", "/etc"} { + if _, err := os.Stat(p); err == nil { + g.ReadPaths = append(g.ReadPaths, p) + } + } + inner := []string{realTool} + switch kind { + case "symlink-target-env", "symlink-target-shebang": + current := filepath.Join(home, "current") + if err := os.Symlink("versions/v1", current); err != nil { + t.Fatal(err) + } + shim := filepath.Join(home, "shims", "tool") + if err := os.MkdirAll(filepath.Dir(shim), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink("../current/bin/tool", shim); err != nil { + t.Fatal(err) + } + inner = []string{"/usr/bin/env", "FOO=bar", shim} + if kind == "symlink-target-shebang" { + outer := filepath.Join(home, "tools", "outer") + review446Script(t, outer) + if err := os.WriteFile(outer, []byte("#!"+shim+"\n"), 0o755); err != nil { + t.Fatal(err) + } + inner = []string{outer} + } + case "root-read": + g.ReadPaths = append(g.ReadPaths, "/") + case "root-write": + g.WritePaths = []string{"/"} + case "root-allow": + g.AllowPaths = append(g.AllowPaths, "/") + } + if _, err := BuildChildArgv(g, inner); err != nil { + t.Fatalf("production backend %s plan: %v", kind, err) + } + planBinaryNamespace(t, g, inner) + stage := append([]string(nil), inner...) + stage[0] = resolveInnerBinaryPath(inner) + argv, err := BuildBwrapArgv(g, stage) + if err != nil { + t.Fatal(err) + } + out, err := exec.Command(argv[0], argv[1:]...).CombinedOutput() + if err != nil || string(out) != "namespace-ok\n" { + t.Fatalf("real %s exec: err=%v output=%q", kind, err, out) + } + }) + } +} diff --git a/internal/sandboxrun/derived_grants_namespace_test.go b/internal/sandboxrun/derived_grants_namespace_test.go new file mode 100644 index 00000000..317a91e2 --- /dev/null +++ b/internal/sandboxrun/derived_grants_namespace_test.go @@ -0,0 +1,452 @@ +package sandboxrun + +import ( + "os" + "path/filepath" + "slices" + "strings" + "testing" +) + +// planBinaryNamespace uses the same checked plan consumed by the Linux backend. +func planBinaryNamespace(t *testing.T, g *Grants, inner []string) []string { + t.Helper() + dirs, names, err := resolveInnerBinaryPlan(inner, derivedGrantPolicyForGrants(g)) + if err != nil { + t.Fatal(err) + } + g.derivedReadPaths, g.derivedReadDirs = dirs, names + return dirs +} + +func namespaceFromBwrap(argv []string) ([]*mount, []namespaceSymlink) { + var mounts []*mount + var links []namespaceSymlink + for i := 0; i < len(argv) && argv[i] != "--"; i++ { + switch argv[i] { + case "--bind", "--ro-bind": + mounts = append(mounts, &mount{path: argv[i+2], rw: argv[i] == "--bind"}) + i += 2 + case "--symlink": + links = append(links, namespaceSymlink{path: argv[i+2], target: argv[i+1]}) + i += 2 + } + } + return mounts, links +} + +func TestDerivedGrantsLinuxCanonicalGrantAlias(t *testing.T) { + home, wd := stableHome(t), t.TempDir() + realRoot := filepath.Join(home, "external-runtime") + realTool := filepath.Join(realRoot, "bin", "tool") + review446Script(t, realTool) + alias := filepath.Join(home, ".nvm") + if err := os.Symlink(realRoot, alias); err != nil { + t.Fatal(err) + } + tool := filepath.Join(alias, "bin", "tool") + for _, mode := range []string{"read", "write", "allow"} { + t.Run(mode, func(t *testing.T) { + g := minimalDerivedGrants(wd, home) + g.ReadPaths = []string{"/bin", "/usr"} + switch mode { + case "read": + g.ReadPaths = append(g.ReadPaths, alias) + case "write": + g.WritePaths = []string{alias} + case "allow": + g.AllowPaths = append(g.AllowPaths, alias) + } + dirs := planBinaryNamespace(t, g, []string{tool}) + argv, err := BuildBwrapArgv(g, []string{resolveInnerBinaryPath([]string{tool})}) + if err != nil { + t.Fatal(err) + } + mounts, links := namespaceFromBwrap(argv) + real, err := filepath.EvalSymlinks(realTool) + if err != nil { + t.Fatal(err) + } + _, reachable := sandboxResolveWithLinks(real, mounts, links) + t.Logf("derived=%v; stage2=%s; reachable=%v; argv=%s", dirs, real, reachable, strings.Join(argv, " ")) + if !reachable { + t.Fatal("canonical target already authorized via alias, but Linux stage2 executes an unmounted canonical pathname") + } + if !slices.Contains(links, namespaceSymlink{path: filepath.Dir(realTool), target: filepath.Join(alias, "bin")}) { + t.Fatal("canonical execution directory must link to the already-visible explicit grant") + } + for _, m := range mounts { + if m.path == filepath.Dir(realTool) { + t.Fatal("must not fix visibility by binding a mutable canonical descendant") + } + } + }) + } +} + +func TestDerivedGrantsRelativeSymlinkTraversal(t *testing.T) { + home, wd := stableHome(t), t.TempDir() + trusted := filepath.Join(home, "trusted") + review446Script(t, filepath.Join(trusted, "bin", "tool")) + if err := os.MkdirAll(filepath.Join(wd, "pivot"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(wd, "pivot"), filepath.Join(trusted, "alias")); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(wd, "bin"), 0o755); err != nil { + t.Fatal(err) + } + review446Script(t, filepath.Join(wd, "bin", "tool")) + link := filepath.Join(trusted, "link") + if err := os.Symlink("alias/../bin/tool", link); err != nil { + t.Fatal(err) + } + real, _, hops, err := walkSymlinkPath(link, false) + if err != nil { + t.Fatal(err) + } + want, err := filepath.EvalSymlinks(link) + if err != nil || real != want { + t.Fatalf("kernel-order traversal got %s, want %s, err %v", real, want, err) + } + if !pathCoveredByAny(filepath.Join(wd, "pivot"), hops) { + t.Fatalf("missing writable traversal history: %v", hops) + } + g := minimalDerivedGrants(wd, home) + g.ReadPaths = []string{"/bin", "/usr"} + if _, err := resolveInnerBinaryDirs([]string{link}, derivedGrantPolicyForGrants(g)); err == nil { + t.Fatal("relative link through writable component must taint derivation") + } +} + +func TestDerivedGrantsLinuxEnvAncestorAlias(t *testing.T) { + home, wd := stableHome(t), t.TempDir() + realRoot := filepath.Join(home, "runtime") + review446Script(t, filepath.Join(realRoot, "bin", "tool")) + alias := filepath.Join(home, "runtime-current") + if err := os.Symlink("runtime", alias); err != nil { + t.Fatal(err) + } + tool := filepath.Join(alias, "bin", "tool") + g := minimalDerivedGrants(wd, home) + g.ReadPaths = []string{"/bin", "/usr"} + inner := []string{"/usr/bin/env", "FOO=bar", tool} + dirs := planBinaryNamespace(t, g, inner) + argv, err := BuildBwrapArgv(g, inner) + if err != nil { + t.Fatal(err) + } + mounts, links := namespaceFromBwrap(argv) + _, reachable := sandboxResolveWithLinks(tool, mounts, links) + t.Logf("derived=%v; env-command=%s; reachable=%v; argv=%s", dirs, tool, reachable, strings.Join(argv, " ")) + if !reachable { + t.Fatal("Linux does not rewrite the wrapped command; canonical-only grants removed its ancestor alias from the namespace") + } + if !slices.Contains(links, namespaceSymlink{path: filepath.Dir(tool), target: filepath.Join(realRoot, "bin")}) { + t.Fatal("env's original command spelling must link to the checked runtime directory") + } +} + +func TestDerivedGrantsLinuxShebangAncestorAlias(t *testing.T) { + home, wd := stableHome(t), t.TempDir() + realRoot := filepath.Join(home, "runtime") + review446Script(t, filepath.Join(realRoot, "bin", "interp")) + alias := filepath.Join(home, "runtime-current") + if err := os.Symlink("runtime", alias); err != nil { + t.Fatal(err) + } + interp := filepath.Join(alias, "bin", "interp") + tool := filepath.Join(home, "tools", "tool") + review446Script(t, tool) + if err := os.WriteFile(tool, []byte("#!"+interp+"\n"), 0o755); err != nil { + t.Fatal(err) + } + g := minimalDerivedGrants(wd, home) + g.ReadPaths = []string{"/bin", "/usr"} + dirs := planBinaryNamespace(t, g, []string{tool}) + argv, err := BuildBwrapArgv(g, []string{resolveInnerBinaryPath([]string{tool})}) + if err != nil { + t.Fatal(err) + } + mounts, links := namespaceFromBwrap(argv) + _, reachable := sandboxResolveWithLinks(interp, mounts, links) + t.Logf("derived=%v; shebang-interpreter=%s; reachable=%v", dirs, interp, reachable) + if !reachable { + t.Fatal("kernel uses the script's original shebang; mounting only the canonical interpreter directory loses its ancestor alias") + } + if !slices.Contains(links, namespaceSymlink{path: filepath.Dir(interp), target: filepath.Join(realRoot, "bin")}) { + t.Fatal("kernel's original shebang spelling must link to the checked interpreter directory") + } +} + +func TestDerivedGrantsNamespaceAliasSwapDoesNotAddHostSource(t *testing.T) { + home := stableHome(t) + realDir := filepath.Join(home, "runtime", "bin") + review446Script(t, filepath.Join(realDir, "tool")) + alias := filepath.Join(home, "runtime-current") + if err := os.Symlink("runtime", alias); err != nil { + t.Fatal(err) + } + tool := filepath.Join(alias, "bin", "tool") + g := minimalDerivedGrants(t.TempDir(), home) + g.ReadPaths = []string{"/bin", "/usr"} + planBinaryNamespace(t, g, []string{"/usr/bin/env", "FOO=bar", tool}) + secret := filepath.Join(home, ".config", "private") + review446Script(t, filepath.Join(secret, "bin", "tool")) + if err := os.Remove(alias); err != nil { + t.Fatal(err) + } + if err := os.Symlink(secret, alias); err != nil { + t.Fatal(err) + } + argv, err := BuildBwrapArgv(g, []string{"/usr/bin/env", "FOO=bar", tool}) + if err != nil { + t.Fatal(err) + } + mounts, links := namespaceFromBwrap(argv) + if !slices.Contains(links, namespaceSymlink{path: filepath.Dir(tool), target: realDir}) { + t.Fatal("lowering must keep the checked target, not re-follow the changed host alias") + } + for _, m := range mounts { + if isUnder(m.path, alias) || isUnder(m.path, secret) { + t.Fatalf("unchecked alias/secret became a new host bind: %v", argv) + } + } +} + +func TestDerivedGrantsNamespaceMutableSupportSwapFailsClosed(t *testing.T) { + home, wd := stableHome(t), t.TempDir() + tool := filepath.Join(wd, "prefix", "bin", "tool") + review446Script(t, tool) + benign := filepath.Join(wd, "benign") + if err := os.Mkdir(benign, 0o755); err != nil { + t.Fatal(err) + } + lib := filepath.Join(wd, "prefix", "lib") + if err := os.Symlink(benign, lib); err != nil { + t.Fatal(err) + } + g := minimalDerivedGrants(wd, home) + g.ReadPaths = []string{"/bin", "/usr"} + planBinaryNamespace(t, g, []string{tool}) + if _, err := BuildBwrapArgv(g, []string{tool}); err != nil { + t.Fatalf("control: covered benign support alias must work: %v", err) + } + secret := filepath.Join(home, ".config", "private") + review446Script(t, filepath.Join(secret, "tool")) + if err := os.Remove(lib); err != nil { + t.Fatal(err) + } + if err := os.Symlink(secret, lib); err != nil { + t.Fatal(err) + } + if argv, err := BuildBwrapArgv(g, []string{tool}); err == nil || argv != nil { + t.Fatal("must not restore a namespace link inside the writable host-backed grant after the swap") + } +} + +func TestDerivedGrantsNamespaceProtectsCanonicalAliasPaths(t *testing.T) { + home := stableHome(t) + realRoot := filepath.Join(home, "runtime") + review446Script(t, filepath.Join(realRoot, "bin", "tool")) + secret := filepath.Join(realRoot, "bin", "token") + if err := os.WriteFile(secret, []byte("secret"), 0o600); err != nil { + t.Fatal(err) + } + alias := filepath.Join(home, "runtime-current") + if err := os.Symlink(realRoot, alias); err != nil { + t.Fatal(err) + } + g := minimalDerivedGrants(t.TempDir(), home) + g.ReadPaths = []string{"/bin", "/usr", alias} + g.ProtectedPaths = []string{secret} + tool := filepath.Join(alias, "bin", "tool") + planBinaryNamespace(t, g, []string{tool}) + argv, err := BuildBwrapArgv(g, []string{resolveInnerBinaryPath([]string{tool})}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(strings.Join(argv, " "), "--ro-bind /dev/null "+filepath.Join(alias, "bin", "token")) { + t.Fatalf("a protected canonical name must be masked at the backing namespace path: %v", argv) + } +} + +func TestDerivedGrantsNamespaceParentDirectoryIsNotReadableCoverage(t *testing.T) { + home := stableHome(t) + tool := filepath.Join(home, "runtime", "bin", "tool") + review446Script(t, tool) + child := filepath.Join(filepath.Dir(tool), "child") + if err := os.Mkdir(child, 0o755); err != nil { + t.Fatal(err) + } + mounts := []*mount{{path: child}} + if namespaceHasDirectory(filepath.Dir(tool), filepath.Dir(tool), mounts, nil) { + t.Fatal("bwrap-created ancestors of mounts do not expose their host directory contents") + } +} + +func TestDerivedGrantsNamespaceProtectsOriginalAliasPaths(t *testing.T) { + home := stableHome(t) + realRoot := filepath.Join(home, "runtime") + review446Script(t, filepath.Join(realRoot, "bin", "tool")) + secret := filepath.Join(realRoot, "bin", "token") + if err := os.WriteFile(secret, []byte("secret"), 0o600); err != nil { + t.Fatal(err) + } + alias := filepath.Join(home, "runtime-current") + if err := os.Symlink(realRoot, alias); err != nil { + t.Fatal(err) + } + g := minimalDerivedGrants(t.TempDir(), home) + g.ReadPaths = []string{"/bin", "/usr"} + g.ProtectedPaths = []string{filepath.Join(alias, "bin", "token")} + tool := filepath.Join(alias, "bin", "tool") + planBinaryNamespace(t, g, []string{"/usr/bin/env", tool}) + argv, err := BuildBwrapArgv(g, []string{"/usr/bin/env", tool}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(strings.Join(argv, " "), "--ro-bind /dev/null "+secret) { + t.Fatalf("protected alias spelling must be masked at its canonical backing path: %v", argv) + } +} + +// Fresh R3 counterexample: the file symlink's target, not argv itself, +// contains a directory alias. The original spelling is still kernel input. +func TestDerivedGrantsLinuxSymlinkTargetAncestorAlias(t *testing.T) { + for _, kind := range []string{"env", "shebang"} { + t.Run(kind, func(t *testing.T) { + home := stableHome(t) + realDir := filepath.Join(home, "versions", "v1", "bin") + review446Script(t, filepath.Join(realDir, "tool")) + current := filepath.Join(home, "current") + if err := os.Symlink("versions/v1", current); err != nil { + t.Fatal(err) + } + shim := filepath.Join(home, "shims", "tool") + if err := os.MkdirAll(filepath.Dir(shim), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink("../current/bin/tool", shim); err != nil { + t.Fatal(err) + } + inner := []string{"/usr/bin/env", "FOO=bar", shim} + if kind == "shebang" { + outer := filepath.Join(home, "outer", "tool") + review446Script(t, outer) + if err := os.WriteFile(outer, []byte("#!"+shim+"\n"), 0o755); err != nil { + t.Fatal(err) + } + inner = []string{outer} + } + g := minimalDerivedGrants(t.TempDir(), home) + g.ReadPaths = []string{"/bin", "/usr"} + planBinaryNamespace(t, g, inner) + argv, err := BuildBwrapArgv(g, inner) + if err != nil { + t.Fatal(err) + } + mounts, links := namespaceFromBwrap(argv) + t.Logf("names=%v; mounts=%v; links=%v; argv=%v", g.derivedReadDirs, g.derivedReadPaths, links, argv) + view := derivedGrantFixtureView(t, home, argv) + if _, err := os.Stat(filepath.Join(view, "shims", "tool")); err != nil { + t.Errorf("independent actual filesystem lookup of emitted fixture view: %v", err) + } + if _, ok := sandboxResolveWithLinks(shim, mounts, links); !ok { + t.Fatalf("%s still needs %s -> ../current/bin/tool, but current/bin has no namespace representation", kind, shim) + } + }) + } +} +func TestDerivedGrantsLinuxExplicitRootMountCoverage(t *testing.T) { + for _, mode := range []string{"read", "write", "allow"} { + t.Run(mode, func(t *testing.T) { + home := stableHome(t) + tool := filepath.Join(home, "custom-runtime", "bin", "tool") + review446Script(t, tool) + g := minimalDerivedGrants(t.TempDir(), home) + g.ReadPaths = []string{"/bin", "/usr"} + switch mode { + case "read": + g.ReadPaths = append(g.ReadPaths, "/") + case "write": + g.WritePaths = []string{"/"} + case "allow": + g.AllowPaths = append(g.AllowPaths, "/") + } + planBinaryNamespace(t, g, []string{tool}) + argv, err := BuildBwrapArgv(g, []string{tool}) + t.Logf("derived=%v; argv=%v; err=%v", g.derivedReadPaths, argv, err) + if err != nil { + t.Fatalf("explicit root %s must not make a previously derivable command unlaunchable: %v", mode, err) + } + }) + } +} + +// Independently reconstruct this fixture's emitted bind contents and links. +// This uses real filesystem lookup, not sandboxResolve's namespace model. +// It does not emulate or claim to execute bubblewrap. +func derivedGrantFixtureView(t *testing.T, home string, argv []string) string { + t.Helper() + view := t.TempDir() + inHome := func(p string) bool { return strings.HasPrefix(p, home+"/") } + mapPath := func(p string) string { + if inHome(p) { + return filepath.Join(view, strings.TrimPrefix(p, home+"/")) + } + return p + } + for i := 0; i < len(argv) && argv[i] != "--"; i++ { + switch argv[i] { + case "--bind", "--ro-bind": + src, dest := argv[i+1], argv[i+2] + i += 2 + if !inHome(dest) { + continue + } + if err := os.MkdirAll(mapPath(dest), 0o755); err != nil { + t.Fatal(err) + } + entries, err := os.ReadDir(src) + if err != nil { + t.Fatal(err) + } + for _, entry := range entries { + s, d := filepath.Join(src, entry.Name()), filepath.Join(mapPath(dest), entry.Name()) + if entry.Type()&os.ModeSymlink != 0 { + target, err := os.Readlink(s) + if err != nil { + t.Fatal(err) + } + err = os.Symlink(mapPath(target), d) + if err != nil { + t.Fatal(err) + } + } else { + data, err := os.ReadFile(s) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(d, data, 0o755); err != nil { + t.Fatal(err) + } + } + } + case "--symlink": + target, dest := argv[i+1], argv[i+2] + i += 2 + if !inHome(dest) { + continue + } + if err := os.MkdirAll(filepath.Dir(mapPath(dest)), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(mapPath(target), mapPath(dest)); err != nil { + t.Fatal(err) + } + } + } + return view +} diff --git a/internal/sandboxrun/grants.go b/internal/sandboxrun/grants.go index 50084800..a2b2ea8c 100644 --- a/internal/sandboxrun/grants.go +++ b/internal/sandboxrun/grants.go @@ -36,6 +36,12 @@ type Grants struct { ReadPaths []string // read-only WritePaths []string // write-only AllowPaths []string // read+write + // derivedReadPaths are new, resolved targets admitted by the command + // policy. Keep them separate so lowering never follows their aliases again. + derivedReadPaths []string + // derivedReadDirs also retains required execution spellings. Linux uses + // namespace-only links, not host alias binds, to make those names visible. + derivedReadDirs []derivedReadDir // ProtectedPaths are denied even under broader grants (expanded; // override_deny holes already punched). Not existence-filtered: diff --git a/internal/sandboxrun/resolve_binary.go b/internal/sandboxrun/resolve_binary.go index 0c613424..e120a264 100644 --- a/internal/sandboxrun/resolve_binary.go +++ b/internal/sandboxrun/resolve_binary.go @@ -38,23 +38,39 @@ import ( // every path it visits on the way is checked against it (see // derivedGrantPolicy), so an inner command the confined process can rewrite // cannot extend the next sandbox's read surface. The zero policy performs -// no checks — the backends pass one derived from the grants. +// no checks — the backends pass one derived from the grants. A bounded +// resolution returns only new, resolved targets; already-granted paths are +// omitted rather than re-emitting aliases that may have changed by lowering. // // Returns nil when the command cannot be found or resolved. An error means // the resolution crossed the policy and the launch must fail closed. func resolveInnerBinaryDirs(innerArgv []string, pol derivedGrantPolicy) ([]string, error) { - dirs, err := commandBinaryDirs(innerArgv, pol) + dirs, _, err := resolveInnerBinaryPlan(innerArgv, pol) + return dirs, err +} + +// derivedReadDir keeps authorization identity separate from a pathname still +// used by env, shebangs or runtime support lookups. Linux may need both names +// in its namespace; real is never replaced by a later lookup of path. +type derivedReadDir struct { + path string + real string +} + +func resolveInnerBinaryPlan(innerArgv []string, pol derivedGrantPolicy) ([]string, []derivedReadDir, error) { + dirs, names, err := commandBinaryPlan(innerArgv, pol) if err != nil { - return nil, err + return nil, nil, err } if cmd := UnwrapEnv(innerArgv); len(cmd) > 0 && cmd[0] != innerArgv[0] { - wrapped, werr := commandBinaryDirs(cmd, pol) + wrapped, wrappedNames, werr := commandBinaryPlan(cmd, pol) if werr != nil { - return nil, werr + return nil, nil, werr } dirs = append(dirs, wrapped...) + names = append(names, wrappedNames...) } - return dirs, nil + return dirs, names, nil } // resolveCommandBinaryDirs is the unbounded form of commandBinaryDirs for @@ -72,26 +88,67 @@ func resolveCommandBinaryDirs(argv []string) []string { // Returns nil when argv is empty or unresolvable. pol bounds the // derivation; an error means it crossed the policy. func commandBinaryDirs(argv []string, pol derivedGrantPolicy) ([]string, error) { + dirs, _, err := commandBinaryPlan(argv, pol) + return dirs, err +} + +func commandBinaryPlan(argv []string, pol derivedGrantPolicy) ([]string, []derivedReadDir, error) { if len(argv) == 0 { - return nil, nil + return nil, nil, nil } resolved := lookPathAbs(argv[0]) if resolved == "" { - return nil, nil + return nil, nil, nil } - dirs, hops := walkSymlinkChain(resolved) - if real, rerr := filepath.EvalSymlinks(resolved); rerr == nil { - if interp := shebangInterpreter(real); interp != "" { - idirs, ihops := resolveInterpreterChain(interp) - dirs = append(dirs, idirs...) - hops = append(hops, ihops...) + real, dirs, hops, err := walkSymlinkPath(resolved, false) + if err != nil { + return nil, nil, err + } + if interp := shebangInterpreter(real); interp != "" { + idirs, ihops, err := resolveInterpreterChain(interp) + if err != nil { + return nil, nil, err } + dirs = append(dirs, idirs...) + hops = append(hops, ihops...) + } + dirs, supportHops, err := resolvePrefixSupportDirs(dirs) + if err != nil { + return nil, nil, err + } + hops = append(hops, supportHops...) + var names []derivedReadDir + if len(pol.writable) != 0 || len(pol.homes) != 0 { + // Admission uses targets, never the literal aliases used for deny + // matching. Return only new, resolved grants: a covered mutable alias + // must not be followed again when a backend lowers the policy. + var targets []string + for _, d := range dirs { + real, _, dhops, err := walkSymlinkPath(d, true) + if err != nil { + return nil, nil, fmt.Errorf("resolve derived directory %q: %w", d, err) + } + hops = append(hops, dhops...) + names = append(names, derivedReadDir{path: d, real: real}) + if !slices.Contains(targets, real) { + targets = append(targets, real) + } + } + dirs = targets } - dirs = withPrefixSupportDirs(dirs) if err := refuseDerivedGrants(dirs, hops, pol); err != nil { - return nil, err + return nil, nil, err + } + if len(pol.writable) != 0 || len(pol.homes) != 0 { + var needed []string + for _, d := range dirs { + if !targetCoveredByAny(d, pol.readable) { + needed = append(needed, d) + } + } + dirs = needed } - return dirs, nil + return dirs, names, nil } // maxSymlinkHops bounds the chain walk. Well above any real install layout @@ -115,11 +172,13 @@ type derivedGrantPolicy struct { // time is whatever the agent last wrote, so it must not pick what the // sandbox grants read access to. writable []string - // granted holds the paths the profile already allows explicitly. - // Deriving a directory covered by them adds nothing the user has not - // already granted, so only the remainder (the extension) is subject - // to the rules. + // granted holds explicitly allowed path roots. Writable content may + // only derive read access within them; the remainder is an extension + // subject to the rules. granted []string + // readable excludes write-only grants: they bound derivation, but on + // macOS a command there still needs a new read rule to remain runnable. + readable []string // homes holds the home directories a derivation must not silently // extend into; see homeSensitiveDirs. homes []string @@ -133,6 +192,15 @@ func derivedGrantPolicyForGrants(g *Grants) derivedGrantPolicy { pol := derivedGrantPolicy{ writable: append(append([]string{}, g.WritePaths...), g.AllowPaths...), granted: append(append(append([]string{}, g.ReadPaths...), g.WritePaths...), g.AllowPaths...), + readable: append([]string{}, g.AllowPaths...), + } + // Linux deliberately does not bind root read grants. Keep a checked + // derived directory for that case so its lowerer can mount the directory + // that the command actually needs. Root allow is different: it is bound. + for _, p := range g.ReadPaths { + if filepath.Clean(p) != "/" { + pol.readable = append(pol.readable, p) + } } pol.writable = append(pol.writable, sharedTempRoots()...) userHome, uerr := os.UserHomeDir() @@ -198,7 +266,7 @@ var homeSensitiveDirs = []string{ // extension that is refused, not the derivation: a tainted command // whose every dir is already granted stays launchable, because // nothing new is exposed. -// - Home boundary: a NEW dir that is itself a home or sits below one +// - Home boundary: a NEW dir that contains a home or sits below one // of its sensitive trees is refused even for untainted commands. A // user's home and its config/credential roots must be granted by the // user, not derived. @@ -210,7 +278,7 @@ func refuseDerivedGrants(dirs, hops []string, pol derivedGrantPolicy) error { } var ext []string for _, d := range dirs { - if !pathCoveredByAny(d, pol.granted) { + if !targetCoveredByAny(d, pol.granted) { ext = append(ext, d) } } @@ -241,7 +309,7 @@ func refuseDerivedGrants(dirs, hops []string, pol derivedGrantPolicy) error { return fmt.Errorf( "omac can't start the sandbox: launching this command would derive read access\n"+ "to %s\n"+ - "inside the home tree %s, which holds config or credentials. omac only\n"+ + "overlapping the home tree %s, which holds config or credentials. omac only\n"+ "derives access there when you grant it explicitly (filesystem.read / --read)\n"+ "or move the tool outside the home tree.", d, home) } @@ -249,9 +317,9 @@ func refuseDerivedGrants(dirs, hops []string, pol derivedGrantPolicy) error { return nil } -// sensitiveUnderHome reports whether dir is one of the homes itself or -// sits below one of a home's sensitive trees, returning that home when it -// does. Both sides are compared in their literal and symlink-resolved +// sensitiveUnderHome reports whether dir contains a home or lies below one +// of its sensitive trees, returning that home when it does. +// Both sides are compared in their literal and symlink-resolved // spellings: a derived dir can arrive from EvalSymlinks, while a home can // be spelled the way the user wrote it. func sensitiveUnderHome(dir string, homes []string) (string, bool) { @@ -267,22 +335,42 @@ func sensitiveUnderHome(dir string, homes []string) (string, bool) { return "", false } -// sensitiveBelowHome reports whether dir is home itself or sits below -// one of home's sensitive trees (homeSensitiveDirs). +// sensitiveBelowHome checks recursive grant overlap. An ancestor of home +// exposes its sensitive descendants just as granting home itself does. func sensitiveBelowHome(dir, home string) bool { - rel, err := filepath.Rel(home, dir) - if err != nil || rel == ".." || - strings.HasPrefix(rel, ".."+string(filepath.Separator)) { - return false + if pathWithin(home, dir) { + return true } - if rel == "." { + for _, name := range homeSensitiveDirs { + for _, sensitive := range pathForms(filepath.Join(home, name)) { + if pathWithin(dir, sensitive) || pathWithin(sensitive, dir) { + return true + } + } + } + return false +} + +// pathWithin handles root without a double-separator prefix and recognizes +// case aliases by filesystem identity, not by folding strings. Walking the +// existing ancestors also handles aliases of HOME, while preserving distinct +// .config and .CONFIG directories on case-sensitive volumes. +func pathWithin(path, root string) bool { + if isUnder(filepath.Clean(path), filepath.Clean(root)) { return true } - first := rel - if i := strings.IndexByte(rel, filepath.Separator); i >= 0 { - first = rel[:i] + rootInfo, err := os.Stat(root) + if err != nil { + return false + } + for p := filepath.Clean(path); ; p = filepath.Dir(p) { + if info, err := os.Stat(p); err == nil && os.SameFile(info, rootInfo) { + return true + } + if p == filepath.Dir(p) { + return false + } } - return slices.Contains(homeSensitiveDirs, first) } // pathCoveredByAny reports whether path lies at or under any of roots, @@ -296,6 +384,18 @@ func pathCoveredByAny(path string, roots []string) bool { return false } +// targetCoveredByAny is grant containment, not conservative deny matching: +// an alias below a granted root does not grant its out-of-tree target. +func targetCoveredByAny(path string, roots []string) bool { + for _, root := range roots { + real, err := filepath.EvalSymlinks(root) + if err == nil && pathWithin(path, real) { + return true + } + } + return false +} + // walkSymlinkChain walks the symlink chain starting at path and returns two // lists: the directory of every hop (what the sandbox must be granted), and // every path it visited on the way (the path itself, each intermediate link @@ -326,6 +426,15 @@ func pathCoveredByAny(path string, roots []string) bool { // grant on the directory is what lets the kernel both read the link and stat // its siblings, matching how the rest of this file grants access. func walkSymlinkChain(path string) (dirs, hops []string) { + _, dirs, hops, _ = walkSymlinkPath(path, false) + return dirs, hops +} + +// walkSymlinkPath resolves every component, not just the final filename. +// An ancestor link can lead through writable surface before reaching an +// otherwise trusted endpoint. Never fall back to an endpoint-only lookup. +// directory selects whether path itself or its parent needs a read grant. +func walkSymlinkPath(path string, directory bool) (real string, dirs, hops []string, err error) { seenDirs := map[string]bool{} seenHops := map[string]bool{} addDir := func(d string) { @@ -340,32 +449,70 @@ func walkSymlinkChain(path string) (dirs, hops []string) { hops = append(hops, p) } } - cur := path - for range maxSymlinkHops { - addHop(cur) - addDir(filepath.Dir(cur)) - fi, err := os.Lstat(cur) - if err != nil || fi.Mode()&os.ModeSymlink == 0 { - break + addGrant := func(p string) { + if directory { + addDir(p) + } else { + addDir(filepath.Dir(p)) + } + } + addGrant(path) + addHop(path) + cur := string(filepath.Separator) + pending := strings.Split(path, string(filepath.Separator)) + nlinks := 0 + for len(pending) > 0 { + part := pending[0] + pending = pending[1:] + if part == "" || part == "." { + continue } - target, terr := os.Readlink(cur) - if terr != nil { - break + if part == ".." { + cur = filepath.Dir(cur) + continue } - if !filepath.IsAbs(target) { - target = filepath.Join(filepath.Dir(cur), target) + next := filepath.Join(cur, part) + addHop(next) + fi, lerr := os.Lstat(next) + if lerr != nil { + return "", dirs, hops, fmt.Errorf("resolve command dependency %q: %w", next, lerr) } - cur = filepath.Clean(target) - } - // A symlinked ancestor (e.g. a version dir that is itself a link) means - // the kernel matches the resolved spelling, which the per-hop walk above - // never produces. EvalSymlinks on the end of the chain closes that gap; - // it is a no-op for the common all-real-directories case. - if real, rerr := filepath.EvalSymlinks(path); rerr == nil { - addHop(real) - addDir(filepath.Dir(real)) + if fi.Mode()&os.ModeSymlink == 0 { + cur = next + continue + } + if len(pending) == 0 { + addGrant(next) + } + nlinks++ + if nlinks > maxSymlinkHops { + return "", dirs, hops, fmt.Errorf("resolve command dependency %q: more than %d symlink hops", path, maxSymlinkHops) + } + target, rerr := os.Readlink(next) + if rerr != nil { + return "", dirs, hops, fmt.Errorf("read command dependency %q: %w", next, rerr) + } + // The kernel continues from the link target, not just from the final + // resolved file. Keep that spelling too: a target such as + // ../current/bin/tool needs current/bin in the namespace even when + // current resolves to versions/v1 on the host. + spelling := target + if !filepath.IsAbs(spelling) { + spelling = filepath.Join(filepath.Dir(next), spelling) + } + if len(pending) > 0 { + spelling = filepath.Join(append([]string{spelling}, pending...)...) + } + addGrant(spelling) + if filepath.IsAbs(target) { + cur = string(filepath.Separator) + } + // Do not Clean target: a/../b must resolve a's symlink before .. + // is applied, just as the kernel does. + pending = append(strings.Split(target, string(filepath.Separator)), pending...) } - return dirs, hops + addGrant(cur) + return cur, dirs, hops, nil } // symlinkChainDirs is walkSymlinkChain's directory list alone, for callers @@ -572,7 +719,7 @@ func pathCoveredBy(path, root string) bool { } for _, d := range forms(path) { for _, r := range forms(root) { - if d == r || strings.HasPrefix(d, r+string(filepath.Separator)) { + if pathWithin(d, r) { return true } } @@ -646,20 +793,16 @@ func shebangInterpreter(path string) string { // holds every path the resolution visited — the interpreter file, each // symlink hop to it, the real file at the end — which feeds the derived-grant // taint check (see derivedGrantPolicy). -func resolveInterpreterChain(interp string) (dirs, hops []string) { +func resolveInterpreterChain(interp string) (dirs, hops []string, err error) { resolved, err := exec.LookPath(interp) + broadFallback := false if err != nil { if filepath.IsAbs(interp) { - if _, serr := os.Stat(interp); serr == nil { - hops = []string{interp} - d := filepath.Dir(interp) - if isBroadGrant(d) { - return nil, hops - } - return withPrefixSupportDirs([]string{d}), hops - } + resolved = interp + broadFallback = isBroadGrant(filepath.Dir(interp)) + } else { + return nil, nil, nil } - return nil, nil } if abs, aerr := filepath.Abs(resolved); aerr == nil { resolved = abs @@ -668,16 +811,19 @@ func resolveInterpreterChain(interp string) (dirs, hops []string) { // reached through a version manager (nvm, mise, asdf) is a multi-hop // symlink, and a middle hop left ungranted fails the exec of every script // that names it — see walkSymlinkChain. - idirs, ihops := walkSymlinkChain(resolved) - return withPrefixSupportDirs(idirs), ihops + _, dirs, hops, err = walkSymlinkPath(resolved, false) + if broadFallback { + dirs = nil + } + return dirs, hops, err } // resolveInterpreterDirs is resolveInterpreterChain's directory list alone, // for callers that only inspect the grant surface (diagnostics, tests) and // have no policy to check the visited paths against. func resolveInterpreterDirs(interp string) []string { - dirs, _ := resolveInterpreterChain(interp) - return dirs + dirs, _, _ := resolveInterpreterChain(interp) + return withPrefixSupportDirs(dirs) } // prefixBinDirNames are the conventional executable subdirectories of a Unix @@ -706,6 +852,16 @@ var prefixSupportDirNames = []string{"lib", "lib64", "libexec", "conf"} // tree, and deriving one would widen the grant to ~/lib off the back of any // binary the user drops there. func prefixSupportDirs(binDir string) []string { + var out []string + for _, p := range prefixSupportCandidates(binDir) { + if fi, err := os.Stat(p); err == nil && fi.IsDir() { + out = append(out, p) + } + } + return out +} + +func prefixSupportCandidates(binDir string) []string { if !slices.Contains(prefixBinDirNames, filepath.Base(binDir)) { return nil } @@ -716,13 +872,41 @@ func prefixSupportDirs(binDir string) []string { var out []string for _, name := range prefixSupportDirNames { p := filepath.Join(prefix, name) - if fi, err := os.Stat(p); err == nil && fi.IsDir() { - out = append(out, p) - } + out = append(out, p) } return out } +// resolvePrefixSupportDirs keeps the provenance of support-directory aliases. +// Lstat, unlike Stat, also exposes dangling or cyclic final links so an +// unfinished resolution cannot silently disappear from the policy check. +func resolvePrefixSupportDirs(dirs []string) ([]string, []string, error) { + out := append([]string(nil), dirs...) + var hops []string + for _, d := range dirs { + for _, p := range prefixSupportCandidates(d) { + if _, err := os.Lstat(p); os.IsNotExist(err) { + continue + } else if err != nil { + return nil, nil, fmt.Errorf("inspect support directory %q: %w", p, err) + } + real, _, visited, err := walkSymlinkPath(p, true) + if err != nil { + return nil, nil, err + } + hops = append(hops, visited...) + fi, err := os.Stat(real) + if err != nil { + return nil, nil, err + } + if fi.IsDir() && !slices.Contains(out, p) { + out = append(out, p) + } + } + } + return out, hops, nil +} + // isBroadGrant reports whether granting dir as a readable directory would // expose an unacceptably wide subtree. "/" and $HOME (and its ancestors) are // rejected; normal install-tree dirs (e.g. /usr/bin) are not. diff --git a/internal/sandboxrun/sbpl.go b/internal/sandboxrun/sbpl.go index 2ec0ac06..0962f2c0 100644 --- a/internal/sandboxrun/sbpl.go +++ b/internal/sandboxrun/sbpl.go @@ -69,8 +69,15 @@ func GenerateSBPL(g *Grants) string { // --- Read allows --- readable := append(append([]string{}, g.ReadPaths...), g.AllowPaths...) + readable = append(readable, g.derivedReadPaths...) + readForms := func(p string) []string { + if slices.Contains(g.derivedReadPaths, p) { + return []string{p} + } + return pathForms(p) + } for _, p := range readable { - for _, fp := range pathForms(p) { + for _, fp := range readForms(p) { fmt.Fprintf(&b, "(allow file-read* (subpath %s))\n", sbplQuote(fp)) } } @@ -80,7 +87,8 @@ func GenerateSBPL(g *Grants) string { // getcwd works inside the working directory, but not from HOME or // above it, where only the shell builtin `pwd` works. ancestorRoots := append(append(append([]string{g.Workdir}, g.ReadPaths...), g.WritePaths...), g.AllowPaths...) - resolvable := ancestorDirs(ancestorRoots) + ancestorRoots = append(ancestorRoots, g.derivedReadPaths...) + resolvable := ancestorDirsWithForms(ancestorRoots, readForms) if g.HomeDir != "" { for _, p := range pathForms(g.HomeDir) { if !slices.Contains(resolvable, p) { @@ -94,7 +102,7 @@ func GenerateSBPL(g *Grants) string { // DYLD-injection defense: only readable paths may be mapped // executable. (allow file-read* implies open; mapping is separate.) for _, p := range readable { - for _, fp := range pathForms(p) { + for _, fp := range readForms(p) { fmt.Fprintf(&b, "(allow file-map-executable (subpath %s))\n", sbplQuote(fp)) } } @@ -240,10 +248,14 @@ func canonicalPath(p string) string { // ancestorDirs returns every distinct ancestor directory of the given // paths, including "/". func ancestorDirs(paths []string) []string { + return ancestorDirsWithForms(paths, pathForms) +} + +func ancestorDirsWithForms(paths []string, forms func(string) []string) []string { seen := map[string]bool{} var out []string for _, p := range paths { - for _, form := range pathForms(p) { + for _, form := range forms(p) { dir := filepath.Dir(form) for dir != "/" && dir != "." { if seen[dir] {