From 3303dc0c130de2baf4d60c9d61f267b38ac79a88 Mon Sep 17 00:00:00 2001 From: jaivial <114494844+jaivial@users.noreply.github.com> Date: Fri, 24 Jul 2026 15:30:19 +0000 Subject: [PATCH 01/11] fix(sandbox): resolve git metadata carveouts for worktrees .git is a file (gitdir: ...) in worktrees/submodules, not a dir. The hooks/config write-deny carveout assumed a directory and pointed at /.git/hooks, which has no mkdir-able parent under the .git file. bwrap then failed 'Can't mkdir parents ... Not a directory', blocking every sandboxed tool in worktree checkouts. Resolve the real (common) git dir via the gitdir pointer + commondir so worktrees deny the shared hooks/config like a plain clone. Plain checkouts (.git dir) and .git-absent roots keep the literal paths. --- internal/sandbox/profile.go | 66 +++++++++++++++++++- internal/sandbox/profile_worktree_test.go | 75 +++++++++++++++++++++++ 2 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 internal/sandbox/profile_worktree_test.go diff --git a/internal/sandbox/profile.go b/internal/sandbox/profile.go index 349e2b1c6..b83204eba 100644 --- a/internal/sandbox/profile.go +++ b/internal/sandbox/profile.go @@ -101,10 +101,72 @@ var sandboxFullyProtectedMetadataNames = []string{".zero", ".agents"} // subprocesses. Nonexistent paths are harmless no-ops in every backend's // enforcement (seatbelt regex, bwrap ro-bind, Windows ACL deny entry). func gitMetadataWriteCarveouts(root string) []string { + // Worktrees/submodules store .git as a *file* ("gitdir: "), so + // .git/hooks has no parent dir — a --tmpfs carveout there makes bwrap fail + // ("Can't mkdir parents ... Not a directory") and blocks every sandboxed + // tool. Resolve the real (common) git dir in that case; otherwise keep the + // plain-checkout paths (harmless no-ops when .git is absent). + gitDir := filepath.Join(root, ".git") + if info, err := os.Stat(gitDir); err == nil && !info.IsDir() { + if real := resolveGitDir(root); real != "" { + gitDir = resolveGitCommonDir(real) + } + } return []string{ - filepath.Join(root, ".git", "hooks"), - filepath.Join(root, ".git", "config"), + filepath.Join(gitDir, "hooks"), + filepath.Join(gitDir, "config"), + } +} + +// resolveGitDir returns the real git directory for a workspace root, handling +// both regular checkouts (.git is a directory) and worktrees/submodules (.git +// is a file containing "gitdir: "). Returns "" when .git is absent, so +// carveouts collapse to no-ops instead of pointing at a bogus path — a bogus +// path under a .git *file* makes bwrap fail ("Can't mkdir parents ... Not a +// directory") and blocks every sandboxed tool. +func resolveGitDir(root string) string { + gitPath := filepath.Join(root, ".git") + info, err := os.Stat(gitPath) + if err != nil { + return "" + } + if info.IsDir() { + return gitPath + } + data, err := os.ReadFile(gitPath) + if err != nil { + return "" + } + dir := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(string(data)), "gitdir:")) + if dir == "" { + return "" + } + // Worktree gitdir pointers are often relative to the worktree root. + if !filepath.IsAbs(dir) { + dir = filepath.Join(root, dir) + } + if _, err := os.Stat(dir); err != nil { + return "" + } + return dir +} + +// resolveGitCommonDir returns the shared git dir for a (possibly worktree) +// gitDir. Worktree gitdirs carry a "commondir" pointer (usually "../.."); plain +// checkouts have none and are their own common dir. +func resolveGitCommonDir(gitDir string) string { + data, err := os.ReadFile(filepath.Join(gitDir, "commondir")) + if err != nil { + return gitDir + } + common := strings.TrimSpace(string(data)) + if common == "" { + return gitDir + } + if !filepath.IsAbs(common) { + common = filepath.Join(gitDir, common) } + return filepath.Clean(common) } func PermissionProfileFromPolicy(workspaceRoot string, policy Policy, scope *Scope) PermissionProfile { diff --git a/internal/sandbox/profile_worktree_test.go b/internal/sandbox/profile_worktree_test.go new file mode 100644 index 000000000..f67fecf32 --- /dev/null +++ b/internal/sandbox/profile_worktree_test.go @@ -0,0 +1,75 @@ +package sandbox + +import ( + "os" + "path/filepath" + "testing" +) + +// Worktree/submodule checkouts store .git as a *file* ("gitdir: "). The +// hooks/config carveout must resolve the real (common) git dir instead of the +// bogus /.git/hooks under that file — a --tmpfs there makes bwrap fail +// ("Can't mkdir parents ... Not a directory") and blocks every sandboxed tool. +func TestGitMetadataWriteCarveoutsResolvesWorktree(t *testing.T) { + main := t.TempDir() + commonGit := filepath.Join(main, ".git") + worktreeGit := filepath.Join(commonGit, "worktrees", "wt") + if err := os.MkdirAll(worktreeGit, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(worktreeGit, "commondir"), []byte("../..\n"), 0o644); err != nil { + t.Fatal(err) + } + + worktree := t.TempDir() + if err := os.WriteFile(filepath.Join(worktree, ".git"), []byte("gitdir: "+worktreeGit+"\n"), 0o644); err != nil { + t.Fatal(err) + } + + got := gitMetadataWriteCarveouts(worktree) + want := []string{ + filepath.Join(commonGit, "hooks"), + filepath.Join(commonGit, "config"), + } + if len(got) != len(want) { + t.Fatalf("carveouts = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("carveout[%d] = %q, want %q", i, got[i], want[i]) + } + } + // Must never point under the .git *file* — that path has no mkdir-able parent. + bogus := filepath.Join(worktree, ".git", "hooks") + for _, c := range got { + if c == bogus { + t.Fatalf("carveout points under .git file: %q", c) + } + } +} + +// Plain checkouts (.git is a dir) and .git-absent roots keep the literal +// /.git/{hooks,config} carveouts. +func TestGitMetadataWriteCarveoutsPlainCheckout(t *testing.T) { + root := t.TempDir() + want := []string{ + filepath.Join(root, ".git", "hooks"), + filepath.Join(root, ".git", "config"), + } + got := gitMetadataWriteCarveouts(root) + for i := range want { + if got[i] != want[i] { + t.Fatalf("carveout[%d] = %q, want %q", i, got[i], want[i]) + } + } + + if err := os.MkdirAll(filepath.Join(root, ".git"), 0o755); err != nil { + t.Fatal(err) + } + got = gitMetadataWriteCarveouts(root) + for i := range want { + if got[i] != want[i] { + t.Fatalf("dir carveout[%d] = %q, want %q", i, got[i], want[i]) + } + } +} From 84446e1458f926fcbd808894cdd635203acdebe9 Mon Sep 17 00:00:00 2001 From: jatmn Date: Thu, 27 Aug 2026 15:34:28 -0700 Subject: [PATCH 02/11] fix(sandbox): bound worktree git metadata carveouts to the write root Refuse gitdir/commondir pointers that escape the workspace instead of carving outside confinement. Invalid, stale, or non-directory pointers return no carveouts so bwrap does not mkdir under a .git file. --- internal/sandbox/profile.go | 72 +++++++--- internal/sandbox/profile_worktree_test.go | 156 +++++++++++++++++----- 2 files changed, 181 insertions(+), 47 deletions(-) diff --git a/internal/sandbox/profile.go b/internal/sandbox/profile.go index b83204eba..9d55c315a 100644 --- a/internal/sandbox/profile.go +++ b/internal/sandbox/profile.go @@ -106,24 +106,38 @@ func gitMetadataWriteCarveouts(root string) []string { // ("Can't mkdir parents ... Not a directory") and blocks every sandboxed // tool. Resolve the real (common) git dir in that case; otherwise keep the // plain-checkout paths (harmless no-ops when .git is absent). - gitDir := filepath.Join(root, ".git") - if info, err := os.Stat(gitDir); err == nil && !info.IsDir() { - if real := resolveGitDir(root); real != "" { - gitDir = resolveGitCommonDir(real) + gitPath := filepath.Join(root, ".git") + info, err := os.Stat(gitPath) + if err != nil || info.IsDir() { + return []string{ + filepath.Join(gitPath, "hooks"), + filepath.Join(gitPath, "config"), } } + + // .git is a file. Following gitdir/commondir is only safe when the + // resolved directory stays inside this write root. An escaping pointer is + // untrusted input: do not carve outside the workspace, and do not fall + // back to /.git/{hooks,config} (that path has no mkdir-able parent + // under a .git file and would crash bwrap). Missing carveouts drop git + // metadata protection; unbounded carveouts drop confinement. + gitDir := resolveGitDir(root) + if gitDir == "" || !pathContainedInRoot(root, gitDir) { + return nil + } + common := resolveGitCommonDir(gitDir) + if common == "" || !pathContainedInRoot(root, common) { + return nil + } return []string{ - filepath.Join(gitDir, "hooks"), - filepath.Join(gitDir, "config"), + filepath.Join(common, "hooks"), + filepath.Join(common, "config"), } } -// resolveGitDir returns the real git directory for a workspace root, handling -// both regular checkouts (.git is a directory) and worktrees/submodules (.git -// is a file containing "gitdir: "). Returns "" when .git is absent, so -// carveouts collapse to no-ops instead of pointing at a bogus path — a bogus -// path under a .git *file* makes bwrap fail ("Can't mkdir parents ... Not a -// directory") and blocks every sandboxed tool. +// resolveGitDir returns the real git directory for a workspace root when .git +// is a gitdir: pointer file. Returns "" for absent, malformed, or unreadable +// pointers so callers can refuse carveouts instead of pointing at a bogus path. func resolveGitDir(root string) string { gitPath := filepath.Join(root, ".git") info, err := os.Stat(gitPath) @@ -137,7 +151,12 @@ func resolveGitDir(root string) string { if err != nil { return "" } - dir := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(string(data)), "gitdir:")) + line := strings.TrimSpace(string(data)) + const prefix = "gitdir:" + if !strings.HasPrefix(strings.ToLower(line), prefix) { + return "" + } + dir := strings.TrimSpace(line[len(prefix):]) if dir == "" { return "" } @@ -145,7 +164,9 @@ func resolveGitDir(root string) string { if !filepath.IsAbs(dir) { dir = filepath.Join(root, dir) } - if _, err := os.Stat(dir); err != nil { + dir = filepath.Clean(dir) + info, err = os.Stat(dir) + if err != nil || !info.IsDir() { return "" } return dir @@ -153,7 +174,8 @@ func resolveGitDir(root string) string { // resolveGitCommonDir returns the shared git dir for a (possibly worktree) // gitDir. Worktree gitdirs carry a "commondir" pointer (usually "../.."); plain -// checkouts have none and are their own common dir. +// checkouts have none and are their own common dir. An unreadable or empty +// commondir keeps gitDir. A present but non-directory commondir is refused. func resolveGitCommonDir(gitDir string) string { data, err := os.ReadFile(filepath.Join(gitDir, "commondir")) if err != nil { @@ -166,7 +188,25 @@ func resolveGitCommonDir(gitDir string) string { if !filepath.IsAbs(common) { common = filepath.Join(gitDir, common) } - return filepath.Clean(common) + common = filepath.Clean(common) + info, err := os.Stat(common) + if err != nil || !info.IsDir() { + return "" + } + return common +} + +func pathContainedInRoot(root, target string) bool { + root = filepath.Clean(root) + target = filepath.Clean(target) + rel, err := filepath.Rel(root, target) + if err != nil { + return false + } + if rel == "." { + return true + } + return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) } func PermissionProfileFromPolicy(workspaceRoot string, policy Policy, scope *Scope) PermissionProfile { diff --git a/internal/sandbox/profile_worktree_test.go b/internal/sandbox/profile_worktree_test.go index f67fecf32..e56b09ade 100644 --- a/internal/sandbox/profile_worktree_test.go +++ b/internal/sandbox/profile_worktree_test.go @@ -6,41 +6,49 @@ import ( "testing" ) +func writeGitFile(t *testing.T, path, contents string) { + t.Helper() + if err := os.WriteFile(path, []byte(contents), 0o644); err != nil { + t.Fatal(err) + } +} + +func mustEqualCarveouts(t *testing.T, got, want []string) { + t.Helper() + if len(got) != len(want) { + t.Fatalf("carveouts = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("carveout[%d] = %q, want %q", i, got[i], want[i]) + } + } +} + // Worktree/submodule checkouts store .git as a *file* ("gitdir: "). The // hooks/config carveout must resolve the real (common) git dir instead of the // bogus /.git/hooks under that file — a --tmpfs there makes bwrap fail // ("Can't mkdir parents ... Not a directory") and blocks every sandboxed tool. func TestGitMetadataWriteCarveoutsResolvesWorktree(t *testing.T) { - main := t.TempDir() - commonGit := filepath.Join(main, ".git") - worktreeGit := filepath.Join(commonGit, "worktrees", "wt") + root := t.TempDir() + commonGit := filepath.Join(root, ".git-common") + worktreeGit := filepath.Join(root, ".git-wt") if err := os.MkdirAll(worktreeGit, 0o755); err != nil { t.Fatal(err) } - if err := os.WriteFile(filepath.Join(worktreeGit, "commondir"), []byte("../..\n"), 0o644); err != nil { + if err := os.MkdirAll(commonGit, 0o755); err != nil { t.Fatal(err) } + writeGitFile(t, filepath.Join(worktreeGit, "commondir"), filepath.Join("..", ".git-common")+"\n") + writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+worktreeGit+"\n") - worktree := t.TempDir() - if err := os.WriteFile(filepath.Join(worktree, ".git"), []byte("gitdir: "+worktreeGit+"\n"), 0o644); err != nil { - t.Fatal(err) - } - - got := gitMetadataWriteCarveouts(worktree) + got := gitMetadataWriteCarveouts(root) want := []string{ filepath.Join(commonGit, "hooks"), filepath.Join(commonGit, "config"), } - if len(got) != len(want) { - t.Fatalf("carveouts = %v, want %v", got, want) - } - for i := range want { - if got[i] != want[i] { - t.Fatalf("carveout[%d] = %q, want %q", i, got[i], want[i]) - } - } - // Must never point under the .git *file* — that path has no mkdir-able parent. - bogus := filepath.Join(worktree, ".git", "hooks") + mustEqualCarveouts(t, got, want) + bogus := filepath.Join(root, ".git", "hooks") for _, c := range got { if c == bogus { t.Fatalf("carveout points under .git file: %q", c) @@ -48,8 +56,22 @@ func TestGitMetadataWriteCarveoutsResolvesWorktree(t *testing.T) { } } -// Plain checkouts (.git is a dir) and .git-absent roots keep the literal -// /.git/{hooks,config} carveouts. +func TestGitMetadataWriteCarveoutsOutsideWorktreeGitdirReturnsNone(t *testing.T) { + main := t.TempDir() + commonGit := filepath.Join(main, ".git") + worktreeGit := filepath.Join(commonGit, "worktrees", "wt") + if err := os.MkdirAll(worktreeGit, 0o755); err != nil { + t.Fatal(err) + } + writeGitFile(t, filepath.Join(worktreeGit, "commondir"), "../..\n") + worktree := t.TempDir() + writeGitFile(t, filepath.Join(worktree, ".git"), "gitdir: "+worktreeGit+"\n") + got := gitMetadataWriteCarveouts(worktree) + if len(got) != 0 { + t.Fatalf("outside-workspace gitdir carveouts = %v, want none", got) + } +} + func TestGitMetadataWriteCarveoutsPlainCheckout(t *testing.T) { root := t.TempDir() want := []string{ @@ -57,19 +79,91 @@ func TestGitMetadataWriteCarveoutsPlainCheckout(t *testing.T) { filepath.Join(root, ".git", "config"), } got := gitMetadataWriteCarveouts(root) - for i := range want { - if got[i] != want[i] { - t.Fatalf("carveout[%d] = %q, want %q", i, got[i], want[i]) - } - } + mustEqualCarveouts(t, got, want) if err := os.MkdirAll(filepath.Join(root, ".git"), 0o755); err != nil { t.Fatal(err) } got = gitMetadataWriteCarveouts(root) - for i := range want { - if got[i] != want[i] { - t.Fatalf("dir carveout[%d] = %q, want %q", i, got[i], want[i]) - } + mustEqualCarveouts(t, got, want) +} + +func TestGitMetadataWriteCarveoutsRejectsEscapingGitdir(t *testing.T) { + outside := t.TempDir() + if err := os.MkdirAll(filepath.Join(outside, "git"), 0o755); err != nil { + t.Fatal(err) + } + root := t.TempDir() + writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+filepath.Join(outside, "git")+"\n") + got := gitMetadataWriteCarveouts(root) + if len(got) != 0 { + t.Fatalf("escaping absolute gitdir carveouts = %v, want none", got) + } +} + +func TestGitMetadataWriteCarveoutsRejectsRelativeEscapingGitdir(t *testing.T) { + parent := t.TempDir() + outside := filepath.Join(parent, "elsewhere") + if err := os.MkdirAll(outside, 0o755); err != nil { + t.Fatal(err) + } + root := filepath.Join(parent, "ws") + if err := os.MkdirAll(root, 0o755); err != nil { + t.Fatal(err) + } + writeGitFile(t, filepath.Join(root, ".git"), "gitdir: ../elsewhere\n") + got := gitMetadataWriteCarveouts(root) + if len(got) != 0 { + t.Fatalf("escaping relative gitdir carveouts = %v, want none", got) + } +} + +func TestGitMetadataWriteCarveoutsRejectsEscapingCommondir(t *testing.T) { + root := t.TempDir() + gitDir := filepath.Join(root, ".git-real") + if err := os.MkdirAll(gitDir, 0o755); err != nil { + t.Fatal(err) + } + outside := t.TempDir() + writeGitFile(t, filepath.Join(gitDir, "commondir"), outside+"\n") + writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+gitDir+"\n") + got := gitMetadataWriteCarveouts(root) + if len(got) != 0 { + t.Fatalf("escaping commondir carveouts = %v, want none", got) + } +} + +func TestGitMetadataWriteCarveoutsCommondirReadFailureKeepsGitDir(t *testing.T) { + root := t.TempDir() + gitDir := filepath.Join(root, ".git-real") + if err := os.MkdirAll(gitDir, 0o755); err != nil { + t.Fatal(err) + } + writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+gitDir+"\n") + got := gitMetadataWriteCarveouts(root) + want := []string{ + filepath.Join(gitDir, "hooks"), + filepath.Join(gitDir, "config"), + } + mustEqualCarveouts(t, got, want) +} + +func TestGitMetadataWriteCarveoutsStaleGitdirReturnsNone(t *testing.T) { + root := t.TempDir() + writeGitFile(t, filepath.Join(root, ".git"), "gitdir: missing-target\n") + got := gitMetadataWriteCarveouts(root) + if len(got) != 0 { + t.Fatalf("stale gitdir carveouts = %v, want none", got) + } +} + +func TestGitMetadataWriteCarveoutsRejectsNonDirectoryTarget(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "not-a-dir") + writeGitFile(t, target, "nope\n") + writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+target+"\n") + got := gitMetadataWriteCarveouts(root) + if len(got) != 0 { + t.Fatalf("non-directory gitdir carveouts = %v, want none", got) } } From 1b72ceb78333e0006121e423ca1194d4b73aa874 Mon Sep 17 00:00:00 2001 From: jatmn Date: Thu, 27 Aug 2026 15:41:40 -0700 Subject: [PATCH 03/11] fix(sandbox): evaluate gitdir/commondir through symlinks Containment must use the real path so an in-root symlink cannot punch hooks/config carveouts outside the write root. --- internal/sandbox/profile.go | 6 ++++++ internal/sandbox/profile_worktree_test.go | 14 ++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/internal/sandbox/profile.go b/internal/sandbox/profile.go index 9d55c315a..35b0af489 100644 --- a/internal/sandbox/profile.go +++ b/internal/sandbox/profile.go @@ -169,6 +169,9 @@ func resolveGitDir(root string) string { if err != nil || !info.IsDir() { return "" } + if resolved, err := filepath.EvalSymlinks(dir); err == nil { + dir = resolved + } return dir } @@ -193,6 +196,9 @@ func resolveGitCommonDir(gitDir string) string { if err != nil || !info.IsDir() { return "" } + if resolved, err := filepath.EvalSymlinks(common); err == nil { + common = resolved + } return common } diff --git a/internal/sandbox/profile_worktree_test.go b/internal/sandbox/profile_worktree_test.go index e56b09ade..459051498 100644 --- a/internal/sandbox/profile_worktree_test.go +++ b/internal/sandbox/profile_worktree_test.go @@ -167,3 +167,17 @@ func TestGitMetadataWriteCarveoutsRejectsNonDirectoryTarget(t *testing.T) { t.Fatalf("non-directory gitdir carveouts = %v, want none", got) } } + +func TestGitMetadataWriteCarveoutsRejectsSymlinkGitdir(t *testing.T) { + outside := t.TempDir() + root := t.TempDir() + alias := filepath.Join(root, "alias") + if err := os.Symlink(outside, alias); err != nil { + t.Fatal(err) + } + writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+alias+"\n") + got := gitMetadataWriteCarveouts(root) + if len(got) != 0 { + t.Fatalf("symlink gitdir carveouts = %v, want none", got) + } +} From 7134ddd6e7e29e733c61e6bf8e167c47ccdfa6a9 Mon Sep 17 00:00:00 2001 From: jatmn Date: Thu, 27 Aug 2026 15:48:11 -0700 Subject: [PATCH 04/11] fix(sandbox): fail closed on gitdir eval and protect the pointer file Refuse carveouts when EvalSymlinks cannot produce a real in-root path, compare evaluated roots and targets, and write-deny the .git pointer so it cannot redirect hooks/config between commands. --- internal/sandbox/profile.go | 39 ++++++++++------ internal/sandbox/profile_worktree_test.go | 55 ++++++++++++++--------- 2 files changed, 61 insertions(+), 33 deletions(-) diff --git a/internal/sandbox/profile.go b/internal/sandbox/profile.go index 35b0af489..f658b5133 100644 --- a/internal/sandbox/profile.go +++ b/internal/sandbox/profile.go @@ -116,20 +116,25 @@ func gitMetadataWriteCarveouts(root string) []string { } // .git is a file. Following gitdir/commondir is only safe when the - // resolved directory stays inside this write root. An escaping pointer is - // untrusted input: do not carve outside the workspace, and do not fall - // back to /.git/{hooks,config} (that path has no mkdir-able parent - // under a .git file and would crash bwrap). Missing carveouts drop git - // metadata protection; unbounded carveouts drop confinement. + // real resolved directory stays inside this write root. An escaping + // pointer is untrusted input: do not carve outside the workspace, and + // do not fall back to /.git/{hooks,config} (that path has no + // mkdir-able parent under a .git file and would crash bwrap). Missing + // carveouts drop git metadata protection; unbounded carveouts drop + // confinement. + // + // Always write-deny the .git pointer file itself so a sandboxed command + // cannot rewrite gitdir: between PermissionProfileFromPolicy calls. gitDir := resolveGitDir(root) if gitDir == "" || !pathContainedInRoot(root, gitDir) { - return nil + return []string{gitPath} } common := resolveGitCommonDir(gitDir) if common == "" || !pathContainedInRoot(root, common) { - return nil + return []string{gitPath} } return []string{ + gitPath, filepath.Join(common, "hooks"), filepath.Join(common, "config"), } @@ -169,10 +174,11 @@ func resolveGitDir(root string) string { if err != nil || !info.IsDir() { return "" } - if resolved, err := filepath.EvalSymlinks(dir); err == nil { - dir = resolved + resolved, err := filepath.EvalSymlinks(dir) + if err != nil { + return "" } - return dir + return resolved } // resolveGitCommonDir returns the shared git dir for a (possibly worktree) @@ -196,15 +202,22 @@ func resolveGitCommonDir(gitDir string) string { if err != nil || !info.IsDir() { return "" } - if resolved, err := filepath.EvalSymlinks(common); err == nil { - common = resolved + resolved, err := filepath.EvalSymlinks(common) + if err != nil { + return "" } - return common + return resolved } func pathContainedInRoot(root, target string) bool { root = filepath.Clean(root) + if resolved, err := filepath.EvalSymlinks(root); err == nil { + root = resolved + } target = filepath.Clean(target) + if resolved, err := filepath.EvalSymlinks(target); err == nil { + target = resolved + } rel, err := filepath.Rel(root, target) if err != nil { return false diff --git a/internal/sandbox/profile_worktree_test.go b/internal/sandbox/profile_worktree_test.go index 459051498..60d42f65b 100644 --- a/internal/sandbox/profile_worktree_test.go +++ b/internal/sandbox/profile_worktree_test.go @@ -44,6 +44,7 @@ func TestGitMetadataWriteCarveoutsResolvesWorktree(t *testing.T) { got := gitMetadataWriteCarveouts(root) want := []string{ + filepath.Join(root, ".git"), filepath.Join(commonGit, "hooks"), filepath.Join(commonGit, "config"), } @@ -67,9 +68,8 @@ func TestGitMetadataWriteCarveoutsOutsideWorktreeGitdirReturnsNone(t *testing.T) worktree := t.TempDir() writeGitFile(t, filepath.Join(worktree, ".git"), "gitdir: "+worktreeGit+"\n") got := gitMetadataWriteCarveouts(worktree) - if len(got) != 0 { - t.Fatalf("outside-workspace gitdir carveouts = %v, want none", got) - } + want := []string{filepath.Join(worktree, ".git")} + mustEqualCarveouts(t, got, want) } func TestGitMetadataWriteCarveoutsPlainCheckout(t *testing.T) { @@ -96,9 +96,7 @@ func TestGitMetadataWriteCarveoutsRejectsEscapingGitdir(t *testing.T) { root := t.TempDir() writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+filepath.Join(outside, "git")+"\n") got := gitMetadataWriteCarveouts(root) - if len(got) != 0 { - t.Fatalf("escaping absolute gitdir carveouts = %v, want none", got) - } + mustEqualCarveouts(t, got, []string{filepath.Join(root, ".git")}) } func TestGitMetadataWriteCarveoutsRejectsRelativeEscapingGitdir(t *testing.T) { @@ -113,9 +111,7 @@ func TestGitMetadataWriteCarveoutsRejectsRelativeEscapingGitdir(t *testing.T) { } writeGitFile(t, filepath.Join(root, ".git"), "gitdir: ../elsewhere\n") got := gitMetadataWriteCarveouts(root) - if len(got) != 0 { - t.Fatalf("escaping relative gitdir carveouts = %v, want none", got) - } + mustEqualCarveouts(t, got, []string{filepath.Join(root, ".git")}) } func TestGitMetadataWriteCarveoutsRejectsEscapingCommondir(t *testing.T) { @@ -128,9 +124,7 @@ func TestGitMetadataWriteCarveoutsRejectsEscapingCommondir(t *testing.T) { writeGitFile(t, filepath.Join(gitDir, "commondir"), outside+"\n") writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+gitDir+"\n") got := gitMetadataWriteCarveouts(root) - if len(got) != 0 { - t.Fatalf("escaping commondir carveouts = %v, want none", got) - } + mustEqualCarveouts(t, got, []string{filepath.Join(root, ".git")}) } func TestGitMetadataWriteCarveoutsCommondirReadFailureKeepsGitDir(t *testing.T) { @@ -142,6 +136,7 @@ func TestGitMetadataWriteCarveoutsCommondirReadFailureKeepsGitDir(t *testing.T) writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+gitDir+"\n") got := gitMetadataWriteCarveouts(root) want := []string{ + filepath.Join(root, ".git"), filepath.Join(gitDir, "hooks"), filepath.Join(gitDir, "config"), } @@ -152,9 +147,7 @@ func TestGitMetadataWriteCarveoutsStaleGitdirReturnsNone(t *testing.T) { root := t.TempDir() writeGitFile(t, filepath.Join(root, ".git"), "gitdir: missing-target\n") got := gitMetadataWriteCarveouts(root) - if len(got) != 0 { - t.Fatalf("stale gitdir carveouts = %v, want none", got) - } + mustEqualCarveouts(t, got, []string{filepath.Join(root, ".git")}) } func TestGitMetadataWriteCarveoutsRejectsNonDirectoryTarget(t *testing.T) { @@ -163,9 +156,7 @@ func TestGitMetadataWriteCarveoutsRejectsNonDirectoryTarget(t *testing.T) { writeGitFile(t, target, "nope\n") writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+target+"\n") got := gitMetadataWriteCarveouts(root) - if len(got) != 0 { - t.Fatalf("non-directory gitdir carveouts = %v, want none", got) - } + mustEqualCarveouts(t, got, []string{filepath.Join(root, ".git")}) } func TestGitMetadataWriteCarveoutsRejectsSymlinkGitdir(t *testing.T) { @@ -177,7 +168,31 @@ func TestGitMetadataWriteCarveoutsRejectsSymlinkGitdir(t *testing.T) { } writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+alias+"\n") got := gitMetadataWriteCarveouts(root) - if len(got) != 0 { - t.Fatalf("symlink gitdir carveouts = %v, want none", got) + mustEqualCarveouts(t, got, []string{filepath.Join(root, ".git")}) +} + +func TestGitMetadataWriteCarveoutsResolvesWorktreeUnderSymlinkRoot(t *testing.T) { + realRoot := t.TempDir() + commonGit := filepath.Join(realRoot, ".git-common") + worktreeGit := filepath.Join(realRoot, ".git-wt") + if err := os.MkdirAll(worktreeGit, 0o755); err != nil { + t.Fatal(err) } + if err := os.MkdirAll(commonGit, 0o755); err != nil { + t.Fatal(err) + } + writeGitFile(t, filepath.Join(worktreeGit, "commondir"), filepath.Join("..", ".git-common")+"\n") + writeGitFile(t, filepath.Join(realRoot, ".git"), "gitdir: "+worktreeGit+"\n") + parent := t.TempDir() + alias := filepath.Join(parent, "ws") + if err := os.Symlink(realRoot, alias); err != nil { + t.Fatal(err) + } + got := gitMetadataWriteCarveouts(alias) + want := []string{ + filepath.Join(alias, ".git"), + filepath.Join(commonGit, "hooks"), + filepath.Join(commonGit, "config"), + } + mustEqualCarveouts(t, got, want) } From 5e717da181dfaa4289a6ebea7d4cb46695743227 Mon Sep 17 00:00:00 2001 From: jatmn Date: Thu, 27 Aug 2026 15:51:37 -0700 Subject: [PATCH 05/11] fix(sandbox): deny worktree-local gitdir metadata and parse first gitdir line Keep hooks/config write-denied on both the common dir and the in-root worktree gitdir, emit those paths under the lexical write root, and accept a gitdir: pointer that has trailing comment lines. --- internal/sandbox/profile.go | 48 ++++++++++++++++++----- internal/sandbox/profile_worktree_test.go | 24 +++++++++++- 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/internal/sandbox/profile.go b/internal/sandbox/profile.go index f658b5133..1f5b59dfc 100644 --- a/internal/sandbox/profile.go +++ b/internal/sandbox/profile.go @@ -133,11 +133,25 @@ func gitMetadataWriteCarveouts(root string) []string { if common == "" || !pathContainedInRoot(root, common) { return []string{gitPath} } - return []string{ + // Emit carveouts under the caller's lexical write root so a symlink + // workspace still matches the path the sandbox binds. + gitDir = pathUnderRoot(root, gitDir) + common = pathUnderRoot(root, common) + if gitDir == "" || common == "" { + return []string{gitPath} + } + carveouts := []string{ gitPath, filepath.Join(common, "hooks"), filepath.Join(common, "config"), } + if gitDir != common { + carveouts = append(carveouts, + filepath.Join(gitDir, "hooks"), + filepath.Join(gitDir, "config"), + ) + } + return carveouts } // resolveGitDir returns the real git directory for a workspace root when .git @@ -156,7 +170,14 @@ func resolveGitDir(root string) string { if err != nil { return "" } - line := strings.TrimSpace(string(data)) + line := "" + for _, candidate := range strings.Split(string(data), "\n") { + candidate = strings.TrimSpace(candidate) + if candidate != "" { + line = candidate + break + } + } const prefix = "gitdir:" if !strings.HasPrefix(strings.ToLower(line), prefix) { return "" @@ -210,22 +231,31 @@ func resolveGitCommonDir(gitDir string) string { } func pathContainedInRoot(root, target string) bool { + return pathUnderRoot(root, target) != "" +} + +func pathUnderRoot(root, target string) string { root = filepath.Clean(root) + target = filepath.Clean(target) + evalRoot := root if resolved, err := filepath.EvalSymlinks(root); err == nil { - root = resolved + evalRoot = resolved } - target = filepath.Clean(target) + evalTarget := target if resolved, err := filepath.EvalSymlinks(target); err == nil { - target = resolved + evalTarget = resolved } - rel, err := filepath.Rel(root, target) + rel, err := filepath.Rel(evalRoot, evalTarget) if err != nil { - return false + return "" + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "" } if rel == "." { - return true + return root } - return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) + return filepath.Join(root, rel) } func PermissionProfileFromPolicy(workspaceRoot string, policy Policy, scope *Scope) PermissionProfile { diff --git a/internal/sandbox/profile_worktree_test.go b/internal/sandbox/profile_worktree_test.go index 60d42f65b..f0721860b 100644 --- a/internal/sandbox/profile_worktree_test.go +++ b/internal/sandbox/profile_worktree_test.go @@ -47,6 +47,8 @@ func TestGitMetadataWriteCarveoutsResolvesWorktree(t *testing.T) { filepath.Join(root, ".git"), filepath.Join(commonGit, "hooks"), filepath.Join(commonGit, "config"), + filepath.Join(worktreeGit, "hooks"), + filepath.Join(worktreeGit, "config"), } mustEqualCarveouts(t, got, want) bogus := filepath.Join(root, ".git", "hooks") @@ -191,8 +193,26 @@ func TestGitMetadataWriteCarveoutsResolvesWorktreeUnderSymlinkRoot(t *testing.T) got := gitMetadataWriteCarveouts(alias) want := []string{ filepath.Join(alias, ".git"), - filepath.Join(commonGit, "hooks"), - filepath.Join(commonGit, "config"), + filepath.Join(alias, ".git-common", "hooks"), + filepath.Join(alias, ".git-common", "config"), + filepath.Join(alias, ".git-wt", "hooks"), + filepath.Join(alias, ".git-wt", "config"), + } + mustEqualCarveouts(t, got, want) +} + +func TestGitMetadataWriteCarveoutsParsesFirstGitdirLine(t *testing.T) { + root := t.TempDir() + gitDir := filepath.Join(root, ".git-real") + if err := os.MkdirAll(gitDir, 0o755); err != nil { + t.Fatal(err) + } + writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+gitDir+"\n# comment\n") + got := gitMetadataWriteCarveouts(root) + want := []string{ + filepath.Join(root, ".git"), + filepath.Join(gitDir, "hooks"), + filepath.Join(gitDir, "config"), } mustEqualCarveouts(t, got, want) } From a8af70c47fbd1578efb45c7a6d620483f42aed38 Mon Sep 17 00:00:00 2001 From: jatmn Date: Thu, 27 Aug 2026 16:02:00 -0700 Subject: [PATCH 06/11] fix(sandbox): skip special git metadata files and re-check carveouts at apply Read .git and commondir only when they are regular files so a FIFO cannot block profile construction. At sandbox apply, drop ReadOnlySubpaths that no longer sit inside the write root after a symlink swap. --- internal/sandbox/linux_helper.go | 2 +- internal/sandbox/profile.go | 40 ++++++++++++++-- internal/sandbox/profile_worktree_test.go | 58 +++++++++++++++++++++++ internal/sandbox/runner.go | 6 +-- internal/sandbox/windows_acl.go | 6 +-- 5 files changed, 98 insertions(+), 14 deletions(-) diff --git a/internal/sandbox/linux_helper.go b/internal/sandbox/linux_helper.go index f3ea5c457..27372c981 100644 --- a/internal/sandbox/linux_helper.go +++ b/internal/sandbox/linux_helper.go @@ -288,7 +288,7 @@ func buildLinuxBwrapFilesystemPlan(profile PermissionProfile) linuxBwrapFilesyst continue } args = append(args, "--bind", root.Root, root.Root) - for _, subpath := range root.ReadOnlySubpaths { + for _, subpath := range containedReadOnlySubpaths(root.Root, root.ReadOnlySubpaths) { args = appendReadOnlyLinuxPathArgs(args, subpath) } for _, name := range root.ProtectedMetadataNames { diff --git a/internal/sandbox/profile.go b/internal/sandbox/profile.go index 1f5b59dfc..84b05378c 100644 --- a/internal/sandbox/profile.go +++ b/internal/sandbox/profile.go @@ -107,13 +107,16 @@ func gitMetadataWriteCarveouts(root string) []string { // tool. Resolve the real (common) git dir in that case; otherwise keep the // plain-checkout paths (harmless no-ops when .git is absent). gitPath := filepath.Join(root, ".git") - info, err := os.Stat(gitPath) + info, err := os.Lstat(gitPath) if err != nil || info.IsDir() { return []string{ filepath.Join(gitPath, "hooks"), filepath.Join(gitPath, "config"), } } + if !info.Mode().IsRegular() { + return []string{gitPath} + } // .git is a file. Following gitdir/commondir is only safe when the // real resolved directory stays inside this write root. An escaping @@ -159,14 +162,17 @@ func gitMetadataWriteCarveouts(root string) []string { // pointers so callers can refuse carveouts instead of pointing at a bogus path. func resolveGitDir(root string) string { gitPath := filepath.Join(root, ".git") - info, err := os.Stat(gitPath) + info, err := os.Lstat(gitPath) if err != nil { return "" } if info.IsDir() { return gitPath } - data, err := os.ReadFile(gitPath) + if !info.Mode().IsRegular() { + return "" + } + data, err := readRegularFile(gitPath) if err != nil { return "" } @@ -207,7 +213,7 @@ func resolveGitDir(root string) string { // checkouts have none and are their own common dir. An unreadable or empty // commondir keeps gitDir. A present but non-directory commondir is refused. func resolveGitCommonDir(gitDir string) string { - data, err := os.ReadFile(filepath.Join(gitDir, "commondir")) + data, err := readRegularFile(filepath.Join(gitDir, "commondir")) if err != nil { return gitDir } @@ -230,6 +236,32 @@ func resolveGitCommonDir(gitDir string) string { return resolved } +func readRegularFile(path string) ([]byte, error) { + info, err := os.Lstat(path) + if err != nil { + return nil, err + } + if !info.Mode().IsRegular() { + return nil, os.ErrInvalid + } + return os.ReadFile(path) +} + +func containedReadOnlySubpaths(root string, subpaths []string) []string { + out := make([]string, 0, len(subpaths)) + for _, subpath := range subpaths { + subpath = strings.TrimSpace(subpath) + if subpath == "" { + continue + } + if !pathContainedInRoot(root, subpath) { + continue + } + out = append(out, subpath) + } + return out +} + func pathContainedInRoot(root, target string) bool { return pathUnderRoot(root, target) != "" } diff --git a/internal/sandbox/profile_worktree_test.go b/internal/sandbox/profile_worktree_test.go index f0721860b..539a80913 100644 --- a/internal/sandbox/profile_worktree_test.go +++ b/internal/sandbox/profile_worktree_test.go @@ -3,7 +3,9 @@ package sandbox import ( "os" "path/filepath" + "syscall" "testing" + "time" ) func writeGitFile(t *testing.T, path, contents string) { @@ -216,3 +218,59 @@ func TestGitMetadataWriteCarveoutsParsesFirstGitdirLine(t *testing.T) { } mustEqualCarveouts(t, got, want) } + +func TestGitMetadataWriteCarveoutsRejectsFIFOGitdir(t *testing.T) { + if syscall.Getuid() == -1 { + t.Skip("no posix fifo") + } + root := t.TempDir() + gitPath := filepath.Join(root, ".git") + if err := syscall.Mkfifo(gitPath, 0o644); err != nil { + t.Skip(err) + } + done := make(chan []string, 1) + go func() { + done <- gitMetadataWriteCarveouts(root) + }() + select { + case got := <-done: + mustEqualCarveouts(t, got, []string{gitPath}) + case <-time.After(2 * time.Second): + t.Fatal("blocked reading fifo .git") + } +} + +func TestGitMetadataWriteCarveoutsRejectsFIFOCommondir(t *testing.T) { + root := t.TempDir() + gitDir := filepath.Join(root, ".git-real") + if err := os.MkdirAll(gitDir, 0o755); err != nil { + t.Fatal(err) + } + if err := syscall.Mkfifo(filepath.Join(gitDir, "commondir"), 0o644); err != nil { + t.Skip(err) + } + writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+gitDir+"\n") + done := make(chan []string, 1) + go func() { + done <- gitMetadataWriteCarveouts(root) + }() + select { + case got := <-done: + want := []string{ + filepath.Join(root, ".git"), + filepath.Join(gitDir, "hooks"), + filepath.Join(gitDir, "config"), + } + mustEqualCarveouts(t, got, want) + case <-time.After(2 * time.Second): + t.Fatal("blocked reading fifo commondir") + } +} + +func TestContainedReadOnlySubpathsDropsEscapingCarveout(t *testing.T) { + root := t.TempDir() + inside := filepath.Join(root, "hooks") + outside := t.TempDir() + got := containedReadOnlySubpaths(root, []string{inside, filepath.Join(outside, "hooks"), ""}) + mustEqualCarveouts(t, got, []string{inside}) +} diff --git a/internal/sandbox/runner.go b/internal/sandbox/runner.go index 8528e7e82..0f0d0ce8b 100644 --- a/internal/sandbox/runner.go +++ b/internal/sandbox/runner.go @@ -873,11 +873,7 @@ func writeRootCarveoutDenyRules(fs FileSystemPolicy) []string { } var out []string for _, root := range fs.WriteRoots { - for _, subpath := range root.ReadOnlySubpaths { - subpath = strings.TrimSpace(subpath) - if subpath == "" { - continue - } + for _, subpath := range containedReadOnlySubpaths(root.Root, root.ReadOnlySubpaths) { escaped := sandboxProfileString(subpath) out = append(out, `(deny file-write* (literal "`+escaped+`"))`, diff --git a/internal/sandbox/windows_acl.go b/internal/sandbox/windows_acl.go index 55d37d347..0895663c2 100644 --- a/internal/sandbox/windows_acl.go +++ b/internal/sandbox/windows_acl.go @@ -97,10 +97,8 @@ func windowsWriteRootCapabilities(config WindowsSandboxCommandConfig) ([]windows return nil, err } protected := make([]string, 0, len(root.ProtectedMetadataNames)+len(root.ReadOnlySubpaths)) - for _, subpath := range root.ReadOnlySubpaths { - if trimmed := strings.TrimSpace(subpath); trimmed != "" { - protected = append(protected, trimmed) - } + for _, subpath := range containedReadOnlySubpaths(rootPath, root.ReadOnlySubpaths) { + protected = append(protected, subpath) } for _, name := range root.ProtectedMetadataNames { if trimmed := strings.TrimSpace(name); trimmed != "" { From e25d7e5bb33fb5115ea3b5a878705d6f917906c4 Mon Sep 17 00:00:00 2001 From: jatmn Date: Thu, 27 Aug 2026 16:05:40 -0700 Subject: [PATCH 07/11] fix(sandbox): follow directory .git symlinks and unix-tag FIFO tests A .git symlink to a directory is a normal checkout, so keep lexical hooks/config carveouts. FIFO coverage stays on unix so Windows still compiles. --- internal/sandbox/profile.go | 25 +++++++- internal/sandbox/profile_worktree_test.go | 61 +++---------------- .../sandbox/profile_worktree_unix_test.go | 56 +++++++++++++++++ 3 files changed, 90 insertions(+), 52 deletions(-) create mode 100644 internal/sandbox/profile_worktree_unix_test.go diff --git a/internal/sandbox/profile.go b/internal/sandbox/profile.go index 84b05378c..f9bb26d34 100644 --- a/internal/sandbox/profile.go +++ b/internal/sandbox/profile.go @@ -108,12 +108,28 @@ func gitMetadataWriteCarveouts(root string) []string { // plain-checkout paths (harmless no-ops when .git is absent). gitPath := filepath.Join(root, ".git") info, err := os.Lstat(gitPath) - if err != nil || info.IsDir() { + if err != nil { + return []string{ + filepath.Join(gitPath, "hooks"), + filepath.Join(gitPath, "config"), + } + } + if info.IsDir() { return []string{ filepath.Join(gitPath, "hooks"), filepath.Join(gitPath, "config"), } } + if info.Mode()&os.ModeSymlink != 0 { + target, statErr := os.Stat(gitPath) + if statErr == nil && target.IsDir() { + return []string{ + filepath.Join(gitPath, "hooks"), + filepath.Join(gitPath, "config"), + } + } + return []string{gitPath} + } if !info.Mode().IsRegular() { return []string{gitPath} } @@ -169,6 +185,13 @@ func resolveGitDir(root string) string { if info.IsDir() { return gitPath } + if info.Mode()&os.ModeSymlink != 0 { + target, statErr := os.Stat(gitPath) + if statErr == nil && target.IsDir() { + return gitPath + } + return "" + } if !info.Mode().IsRegular() { return "" } diff --git a/internal/sandbox/profile_worktree_test.go b/internal/sandbox/profile_worktree_test.go index 539a80913..1a6d4c7ec 100644 --- a/internal/sandbox/profile_worktree_test.go +++ b/internal/sandbox/profile_worktree_test.go @@ -3,9 +3,7 @@ package sandbox import ( "os" "path/filepath" - "syscall" "testing" - "time" ) func writeGitFile(t *testing.T, path, contents string) { @@ -219,58 +217,19 @@ func TestGitMetadataWriteCarveoutsParsesFirstGitdirLine(t *testing.T) { mustEqualCarveouts(t, got, want) } -func TestGitMetadataWriteCarveoutsRejectsFIFOGitdir(t *testing.T) { - if syscall.Getuid() == -1 { - t.Skip("no posix fifo") - } - root := t.TempDir() - gitPath := filepath.Join(root, ".git") - if err := syscall.Mkfifo(gitPath, 0o644); err != nil { - t.Skip(err) - } - done := make(chan []string, 1) - go func() { - done <- gitMetadataWriteCarveouts(root) - }() - select { - case got := <-done: - mustEqualCarveouts(t, got, []string{gitPath}) - case <-time.After(2 * time.Second): - t.Fatal("blocked reading fifo .git") - } -} - -func TestGitMetadataWriteCarveoutsRejectsFIFOCommondir(t *testing.T) { +func TestGitMetadataWriteCarveoutsFollowsSymlinkGitDir(t *testing.T) { root := t.TempDir() - gitDir := filepath.Join(root, ".git-real") - if err := os.MkdirAll(gitDir, 0o755); err != nil { + realGit := filepath.Join(root, "real.git") + if err := os.MkdirAll(realGit, 0o755); err != nil { t.Fatal(err) } - if err := syscall.Mkfifo(filepath.Join(gitDir, "commondir"), 0o644); err != nil { - t.Skip(err) + if err := os.Symlink(realGit, filepath.Join(root, ".git")); err != nil { + t.Fatal(err) } - writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+gitDir+"\n") - done := make(chan []string, 1) - go func() { - done <- gitMetadataWriteCarveouts(root) - }() - select { - case got := <-done: - want := []string{ - filepath.Join(root, ".git"), - filepath.Join(gitDir, "hooks"), - filepath.Join(gitDir, "config"), - } - mustEqualCarveouts(t, got, want) - case <-time.After(2 * time.Second): - t.Fatal("blocked reading fifo commondir") + got := gitMetadataWriteCarveouts(root) + want := []string{ + filepath.Join(root, ".git", "hooks"), + filepath.Join(root, ".git", "config"), } -} - -func TestContainedReadOnlySubpathsDropsEscapingCarveout(t *testing.T) { - root := t.TempDir() - inside := filepath.Join(root, "hooks") - outside := t.TempDir() - got := containedReadOnlySubpaths(root, []string{inside, filepath.Join(outside, "hooks"), ""}) - mustEqualCarveouts(t, got, []string{inside}) + mustEqualCarveouts(t, got, want) } diff --git a/internal/sandbox/profile_worktree_unix_test.go b/internal/sandbox/profile_worktree_unix_test.go new file mode 100644 index 000000000..6ffd79f28 --- /dev/null +++ b/internal/sandbox/profile_worktree_unix_test.go @@ -0,0 +1,56 @@ +//go:build unix + +package sandbox + +import ( + "os" + "path/filepath" + "syscall" + "testing" + "time" +) + +func TestGitMetadataWriteCarveoutsRejectsFIFOGitdir(t *testing.T) { + root := t.TempDir() + gitPath := filepath.Join(root, ".git") + if err := syscall.Mkfifo(gitPath, 0o644); err != nil { + t.Skip(err) + } + done := make(chan []string, 1) + go func() { + done <- gitMetadataWriteCarveouts(root) + }() + select { + case got := <-done: + mustEqualCarveouts(t, got, []string{gitPath}) + case <-time.After(2 * time.Second): + t.Fatal("blocked reading fifo .git") + } +} + +func TestGitMetadataWriteCarveoutsRejectsFIFOCommondir(t *testing.T) { + root := t.TempDir() + gitDir := filepath.Join(root, ".git-real") + if err := os.MkdirAll(gitDir, 0o755); err != nil { + t.Fatal(err) + } + if err := syscall.Mkfifo(filepath.Join(gitDir, "commondir"), 0o644); err != nil { + t.Skip(err) + } + writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+gitDir+"\n") + done := make(chan []string, 1) + go func() { + done <- gitMetadataWriteCarveouts(root) + }() + select { + case got := <-done: + want := []string{ + filepath.Join(root, ".git"), + filepath.Join(gitDir, "hooks"), + filepath.Join(gitDir, "config"), + } + mustEqualCarveouts(t, got, want) + case <-time.After(2 * time.Second): + t.Fatal("blocked reading fifo commondir") + } +} From 014c32eae11ff5b8b83976cb9cbc494aab80fef4 Mon Sep 17 00:00:00 2001 From: jatmn Date: Thu, 27 Aug 2026 17:14:25 -0700 Subject: [PATCH 08/11] fix(sandbox): protect worktree git pointer directly --- internal/sandbox/linux_helper.go | 2 +- internal/sandbox/profile.go | 202 +---------------- internal/sandbox/profile_worktree_test.go | 211 +++--------------- .../sandbox/profile_worktree_unix_test.go | 42 +--- internal/sandbox/runner.go | 6 +- internal/sandbox/windows_acl.go | 6 +- 6 files changed, 56 insertions(+), 413 deletions(-) diff --git a/internal/sandbox/linux_helper.go b/internal/sandbox/linux_helper.go index 27372c981..f3ea5c457 100644 --- a/internal/sandbox/linux_helper.go +++ b/internal/sandbox/linux_helper.go @@ -288,7 +288,7 @@ func buildLinuxBwrapFilesystemPlan(profile PermissionProfile) linuxBwrapFilesyst continue } args = append(args, "--bind", root.Root, root.Root) - for _, subpath := range containedReadOnlySubpaths(root.Root, root.ReadOnlySubpaths) { + for _, subpath := range root.ReadOnlySubpaths { args = appendReadOnlyLinuxPathArgs(args, subpath) } for _, name := range root.ProtectedMetadataNames { diff --git a/internal/sandbox/profile.go b/internal/sandbox/profile.go index f9bb26d34..1ef6980bd 100644 --- a/internal/sandbox/profile.go +++ b/internal/sandbox/profile.go @@ -96,19 +96,19 @@ var protectedMetadataNames = []string{".git", ".zero", ".agents"} // gitMetadataWriteCarveouts below. var sandboxFullyProtectedMetadataNames = []string{".zero", ".agents"} -// gitMetadataWriteCarveouts returns the .git subpaths that stay write-denied -// under the OS-level sandbox even though the rest of .git is writable to git -// subprocesses. Nonexistent paths are harmless no-ops in every backend's -// enforcement (seatbelt regex, bwrap ro-bind, Windows ACL deny entry). +// gitMetadataWriteCarveouts returns the .git paths that stay write-denied under +// the OS-level sandbox even though the rest of .git is writable to git +// subprocesses. Worktrees and submodules store .git as a pointer file, so they +// protect that file itself: constructing .git/hooks below a file makes bwrap +// fail before the command starts. The pointer is deliberately not parsed here; +// repository-controlled metadata must not redirect sandbox rules elsewhere. func gitMetadataWriteCarveouts(root string) []string { - // Worktrees/submodules store .git as a *file* ("gitdir: "), so - // .git/hooks has no parent dir — a --tmpfs carveout there makes bwrap fail - // ("Can't mkdir parents ... Not a directory") and blocks every sandboxed - // tool. Resolve the real (common) git dir in that case; otherwise keep the - // plain-checkout paths (harmless no-ops when .git is absent). gitPath := filepath.Join(root, ".git") info, err := os.Lstat(gitPath) if err != nil { + if !os.IsNotExist(err) { + return []string{gitPath} + } return []string{ filepath.Join(gitPath, "hooks"), filepath.Join(gitPath, "config"), @@ -123,194 +123,14 @@ func gitMetadataWriteCarveouts(root string) []string { if info.Mode()&os.ModeSymlink != 0 { target, statErr := os.Stat(gitPath) if statErr == nil && target.IsDir() { + // Preserve the pre-existing behavior for directory symlinks. return []string{ filepath.Join(gitPath, "hooks"), filepath.Join(gitPath, "config"), } } - return []string{gitPath} - } - if !info.Mode().IsRegular() { - return []string{gitPath} - } - - // .git is a file. Following gitdir/commondir is only safe when the - // real resolved directory stays inside this write root. An escaping - // pointer is untrusted input: do not carve outside the workspace, and - // do not fall back to /.git/{hooks,config} (that path has no - // mkdir-able parent under a .git file and would crash bwrap). Missing - // carveouts drop git metadata protection; unbounded carveouts drop - // confinement. - // - // Always write-deny the .git pointer file itself so a sandboxed command - // cannot rewrite gitdir: between PermissionProfileFromPolicy calls. - gitDir := resolveGitDir(root) - if gitDir == "" || !pathContainedInRoot(root, gitDir) { - return []string{gitPath} - } - common := resolveGitCommonDir(gitDir) - if common == "" || !pathContainedInRoot(root, common) { - return []string{gitPath} - } - // Emit carveouts under the caller's lexical write root so a symlink - // workspace still matches the path the sandbox binds. - gitDir = pathUnderRoot(root, gitDir) - common = pathUnderRoot(root, common) - if gitDir == "" || common == "" { - return []string{gitPath} - } - carveouts := []string{ - gitPath, - filepath.Join(common, "hooks"), - filepath.Join(common, "config"), - } - if gitDir != common { - carveouts = append(carveouts, - filepath.Join(gitDir, "hooks"), - filepath.Join(gitDir, "config"), - ) - } - return carveouts -} - -// resolveGitDir returns the real git directory for a workspace root when .git -// is a gitdir: pointer file. Returns "" for absent, malformed, or unreadable -// pointers so callers can refuse carveouts instead of pointing at a bogus path. -func resolveGitDir(root string) string { - gitPath := filepath.Join(root, ".git") - info, err := os.Lstat(gitPath) - if err != nil { - return "" - } - if info.IsDir() { - return gitPath - } - if info.Mode()&os.ModeSymlink != 0 { - target, statErr := os.Stat(gitPath) - if statErr == nil && target.IsDir() { - return gitPath - } - return "" - } - if !info.Mode().IsRegular() { - return "" - } - data, err := readRegularFile(gitPath) - if err != nil { - return "" - } - line := "" - for _, candidate := range strings.Split(string(data), "\n") { - candidate = strings.TrimSpace(candidate) - if candidate != "" { - line = candidate - break - } - } - const prefix = "gitdir:" - if !strings.HasPrefix(strings.ToLower(line), prefix) { - return "" - } - dir := strings.TrimSpace(line[len(prefix):]) - if dir == "" { - return "" - } - // Worktree gitdir pointers are often relative to the worktree root. - if !filepath.IsAbs(dir) { - dir = filepath.Join(root, dir) - } - dir = filepath.Clean(dir) - info, err = os.Stat(dir) - if err != nil || !info.IsDir() { - return "" - } - resolved, err := filepath.EvalSymlinks(dir) - if err != nil { - return "" - } - return resolved -} - -// resolveGitCommonDir returns the shared git dir for a (possibly worktree) -// gitDir. Worktree gitdirs carry a "commondir" pointer (usually "../.."); plain -// checkouts have none and are their own common dir. An unreadable or empty -// commondir keeps gitDir. A present but non-directory commondir is refused. -func resolveGitCommonDir(gitDir string) string { - data, err := readRegularFile(filepath.Join(gitDir, "commondir")) - if err != nil { - return gitDir - } - common := strings.TrimSpace(string(data)) - if common == "" { - return gitDir - } - if !filepath.IsAbs(common) { - common = filepath.Join(gitDir, common) - } - common = filepath.Clean(common) - info, err := os.Stat(common) - if err != nil || !info.IsDir() { - return "" - } - resolved, err := filepath.EvalSymlinks(common) - if err != nil { - return "" - } - return resolved -} - -func readRegularFile(path string) ([]byte, error) { - info, err := os.Lstat(path) - if err != nil { - return nil, err - } - if !info.Mode().IsRegular() { - return nil, os.ErrInvalid - } - return os.ReadFile(path) -} - -func containedReadOnlySubpaths(root string, subpaths []string) []string { - out := make([]string, 0, len(subpaths)) - for _, subpath := range subpaths { - subpath = strings.TrimSpace(subpath) - if subpath == "" { - continue - } - if !pathContainedInRoot(root, subpath) { - continue - } - out = append(out, subpath) - } - return out -} - -func pathContainedInRoot(root, target string) bool { - return pathUnderRoot(root, target) != "" -} - -func pathUnderRoot(root, target string) string { - root = filepath.Clean(root) - target = filepath.Clean(target) - evalRoot := root - if resolved, err := filepath.EvalSymlinks(root); err == nil { - evalRoot = resolved - } - evalTarget := target - if resolved, err := filepath.EvalSymlinks(target); err == nil { - evalTarget = resolved - } - rel, err := filepath.Rel(evalRoot, evalTarget) - if err != nil { - return "" - } - if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { - return "" - } - if rel == "." { - return root } - return filepath.Join(root, rel) + return []string{gitPath} } func PermissionProfileFromPolicy(workspaceRoot string, policy Policy, scope *Scope) PermissionProfile { diff --git a/internal/sandbox/profile_worktree_test.go b/internal/sandbox/profile_worktree_test.go index 1a6d4c7ec..5810946a3 100644 --- a/internal/sandbox/profile_worktree_test.go +++ b/internal/sandbox/profile_worktree_test.go @@ -6,13 +6,6 @@ import ( "testing" ) -func writeGitFile(t *testing.T, path, contents string) { - t.Helper() - if err := os.WriteFile(path, []byte(contents), 0o644); err != nil { - t.Fatal(err) - } -} - func mustEqualCarveouts(t *testing.T, got, want []string) { t.Helper() if len(got) != len(want) { @@ -25,53 +18,45 @@ func mustEqualCarveouts(t *testing.T, got, want []string) { } } -// Worktree/submodule checkouts store .git as a *file* ("gitdir: "). The -// hooks/config carveout must resolve the real (common) git dir instead of the -// bogus /.git/hooks under that file — a --tmpfs there makes bwrap fail -// ("Can't mkdir parents ... Not a directory") and blocks every sandboxed tool. -func TestGitMetadataWriteCarveoutsResolvesWorktree(t *testing.T) { +// Worktree/submodule checkouts store .git as a pointer file. Protect the file +// itself and never create a bwrap mount target beneath it. +func TestGitMetadataWriteCarveoutsProtectsWorktreePointer(t *testing.T) { root := t.TempDir() - commonGit := filepath.Join(root, ".git-common") - worktreeGit := filepath.Join(root, ".git-wt") - if err := os.MkdirAll(worktreeGit, 0o755); err != nil { + gitPath := filepath.Join(root, ".git") + if err := os.WriteFile(gitPath, []byte("gitdir: ../outside\n"), 0o644); err != nil { t.Fatal(err) } - if err := os.MkdirAll(commonGit, 0o755); err != nil { - t.Fatal(err) - } - writeGitFile(t, filepath.Join(worktreeGit, "commondir"), filepath.Join("..", ".git-common")+"\n") - writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+worktreeGit+"\n") got := gitMetadataWriteCarveouts(root) - want := []string{ - filepath.Join(root, ".git"), - filepath.Join(commonGit, "hooks"), - filepath.Join(commonGit, "config"), - filepath.Join(worktreeGit, "hooks"), - filepath.Join(worktreeGit, "config"), - } - mustEqualCarveouts(t, got, want) - bogus := filepath.Join(root, ".git", "hooks") - for _, c := range got { - if c == bogus { - t.Fatalf("carveout points under .git file: %q", c) + mustEqualCarveouts(t, got, []string{gitPath}) + for _, carveout := range got { + if carveout == filepath.Join(gitPath, "hooks") || carveout == filepath.Join(gitPath, "config") { + t.Fatalf("carveout points beneath .git file: %q", carveout) } } } -func TestGitMetadataWriteCarveoutsOutsideWorktreeGitdirReturnsNone(t *testing.T) { - main := t.TempDir() - commonGit := filepath.Join(main, ".git") - worktreeGit := filepath.Join(commonGit, "worktrees", "wt") - if err := os.MkdirAll(worktreeGit, 0o755); err != nil { - t.Fatal(err) +func TestLinuxBwrapPlanDoesNotMountBelowWorktreePointer(t *testing.T) { + root := t.TempDir() + gitPath := filepath.Join(root, ".git") + if err := os.WriteFile(gitPath, []byte("gitdir: ../outside\n"), 0o644); err != nil { + t.Fatal(err) + } + profile := PermissionProfile{FileSystem: FileSystemPolicy{ + Kind: FileSystemRestricted, + WriteRoots: []WritableRoot{{ + Root: root, + ReadOnlySubpaths: gitMetadataWriteCarveouts(root), + }}, + }} + args := linuxBwrapFilesystemArgs(profile) + assertArgsContainSequence(t, args, "--ro-bind", gitPath, gitPath) + for _, child := range []string{"hooks", "config"} { + bogus := filepath.Join(gitPath, child) + if argsContainSequence(args, "--tmpfs", bogus) || argsContainSequence(args, "--ro-bind", bogus, bogus) { + t.Fatalf("bwrap args contain mount below .git file %q: %v", bogus, args) + } } - writeGitFile(t, filepath.Join(worktreeGit, "commondir"), "../..\n") - worktree := t.TempDir() - writeGitFile(t, filepath.Join(worktree, ".git"), "gitdir: "+worktreeGit+"\n") - got := gitMetadataWriteCarveouts(worktree) - want := []string{filepath.Join(worktree, ".git")} - mustEqualCarveouts(t, got, want) } func TestGitMetadataWriteCarveoutsPlainCheckout(t *testing.T) { @@ -80,144 +65,15 @@ func TestGitMetadataWriteCarveoutsPlainCheckout(t *testing.T) { filepath.Join(root, ".git", "hooks"), filepath.Join(root, ".git", "config"), } - got := gitMetadataWriteCarveouts(root) - mustEqualCarveouts(t, got, want) + mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), want) if err := os.MkdirAll(filepath.Join(root, ".git"), 0o755); err != nil { t.Fatal(err) } - got = gitMetadataWriteCarveouts(root) - mustEqualCarveouts(t, got, want) + mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), want) } -func TestGitMetadataWriteCarveoutsRejectsEscapingGitdir(t *testing.T) { - outside := t.TempDir() - if err := os.MkdirAll(filepath.Join(outside, "git"), 0o755); err != nil { - t.Fatal(err) - } - root := t.TempDir() - writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+filepath.Join(outside, "git")+"\n") - got := gitMetadataWriteCarveouts(root) - mustEqualCarveouts(t, got, []string{filepath.Join(root, ".git")}) -} - -func TestGitMetadataWriteCarveoutsRejectsRelativeEscapingGitdir(t *testing.T) { - parent := t.TempDir() - outside := filepath.Join(parent, "elsewhere") - if err := os.MkdirAll(outside, 0o755); err != nil { - t.Fatal(err) - } - root := filepath.Join(parent, "ws") - if err := os.MkdirAll(root, 0o755); err != nil { - t.Fatal(err) - } - writeGitFile(t, filepath.Join(root, ".git"), "gitdir: ../elsewhere\n") - got := gitMetadataWriteCarveouts(root) - mustEqualCarveouts(t, got, []string{filepath.Join(root, ".git")}) -} - -func TestGitMetadataWriteCarveoutsRejectsEscapingCommondir(t *testing.T) { - root := t.TempDir() - gitDir := filepath.Join(root, ".git-real") - if err := os.MkdirAll(gitDir, 0o755); err != nil { - t.Fatal(err) - } - outside := t.TempDir() - writeGitFile(t, filepath.Join(gitDir, "commondir"), outside+"\n") - writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+gitDir+"\n") - got := gitMetadataWriteCarveouts(root) - mustEqualCarveouts(t, got, []string{filepath.Join(root, ".git")}) -} - -func TestGitMetadataWriteCarveoutsCommondirReadFailureKeepsGitDir(t *testing.T) { - root := t.TempDir() - gitDir := filepath.Join(root, ".git-real") - if err := os.MkdirAll(gitDir, 0o755); err != nil { - t.Fatal(err) - } - writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+gitDir+"\n") - got := gitMetadataWriteCarveouts(root) - want := []string{ - filepath.Join(root, ".git"), - filepath.Join(gitDir, "hooks"), - filepath.Join(gitDir, "config"), - } - mustEqualCarveouts(t, got, want) -} - -func TestGitMetadataWriteCarveoutsStaleGitdirReturnsNone(t *testing.T) { - root := t.TempDir() - writeGitFile(t, filepath.Join(root, ".git"), "gitdir: missing-target\n") - got := gitMetadataWriteCarveouts(root) - mustEqualCarveouts(t, got, []string{filepath.Join(root, ".git")}) -} - -func TestGitMetadataWriteCarveoutsRejectsNonDirectoryTarget(t *testing.T) { - root := t.TempDir() - target := filepath.Join(root, "not-a-dir") - writeGitFile(t, target, "nope\n") - writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+target+"\n") - got := gitMetadataWriteCarveouts(root) - mustEqualCarveouts(t, got, []string{filepath.Join(root, ".git")}) -} - -func TestGitMetadataWriteCarveoutsRejectsSymlinkGitdir(t *testing.T) { - outside := t.TempDir() - root := t.TempDir() - alias := filepath.Join(root, "alias") - if err := os.Symlink(outside, alias); err != nil { - t.Fatal(err) - } - writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+alias+"\n") - got := gitMetadataWriteCarveouts(root) - mustEqualCarveouts(t, got, []string{filepath.Join(root, ".git")}) -} - -func TestGitMetadataWriteCarveoutsResolvesWorktreeUnderSymlinkRoot(t *testing.T) { - realRoot := t.TempDir() - commonGit := filepath.Join(realRoot, ".git-common") - worktreeGit := filepath.Join(realRoot, ".git-wt") - if err := os.MkdirAll(worktreeGit, 0o755); err != nil { - t.Fatal(err) - } - if err := os.MkdirAll(commonGit, 0o755); err != nil { - t.Fatal(err) - } - writeGitFile(t, filepath.Join(worktreeGit, "commondir"), filepath.Join("..", ".git-common")+"\n") - writeGitFile(t, filepath.Join(realRoot, ".git"), "gitdir: "+worktreeGit+"\n") - parent := t.TempDir() - alias := filepath.Join(parent, "ws") - if err := os.Symlink(realRoot, alias); err != nil { - t.Fatal(err) - } - got := gitMetadataWriteCarveouts(alias) - want := []string{ - filepath.Join(alias, ".git"), - filepath.Join(alias, ".git-common", "hooks"), - filepath.Join(alias, ".git-common", "config"), - filepath.Join(alias, ".git-wt", "hooks"), - filepath.Join(alias, ".git-wt", "config"), - } - mustEqualCarveouts(t, got, want) -} - -func TestGitMetadataWriteCarveoutsParsesFirstGitdirLine(t *testing.T) { - root := t.TempDir() - gitDir := filepath.Join(root, ".git-real") - if err := os.MkdirAll(gitDir, 0o755); err != nil { - t.Fatal(err) - } - writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+gitDir+"\n# comment\n") - got := gitMetadataWriteCarveouts(root) - want := []string{ - filepath.Join(root, ".git"), - filepath.Join(gitDir, "hooks"), - filepath.Join(gitDir, "config"), - } - mustEqualCarveouts(t, got, want) -} - -func TestGitMetadataWriteCarveoutsFollowsSymlinkGitDir(t *testing.T) { +func TestGitMetadataWriteCarveoutsPreservesDirectorySymlinkBehavior(t *testing.T) { root := t.TempDir() realGit := filepath.Join(root, "real.git") if err := os.MkdirAll(realGit, 0o755); err != nil { @@ -226,10 +82,9 @@ func TestGitMetadataWriteCarveoutsFollowsSymlinkGitDir(t *testing.T) { if err := os.Symlink(realGit, filepath.Join(root, ".git")); err != nil { t.Fatal(err) } - got := gitMetadataWriteCarveouts(root) want := []string{ filepath.Join(root, ".git", "hooks"), filepath.Join(root, ".git", "config"), } - mustEqualCarveouts(t, got, want) + mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), want) } diff --git a/internal/sandbox/profile_worktree_unix_test.go b/internal/sandbox/profile_worktree_unix_test.go index 6ffd79f28..02606d2df 100644 --- a/internal/sandbox/profile_worktree_unix_test.go +++ b/internal/sandbox/profile_worktree_unix_test.go @@ -3,54 +3,16 @@ package sandbox import ( - "os" "path/filepath" "syscall" "testing" - "time" ) -func TestGitMetadataWriteCarveoutsRejectsFIFOGitdir(t *testing.T) { +func TestGitMetadataWriteCarveoutsProtectsSpecialGitEntry(t *testing.T) { root := t.TempDir() gitPath := filepath.Join(root, ".git") if err := syscall.Mkfifo(gitPath, 0o644); err != nil { t.Skip(err) } - done := make(chan []string, 1) - go func() { - done <- gitMetadataWriteCarveouts(root) - }() - select { - case got := <-done: - mustEqualCarveouts(t, got, []string{gitPath}) - case <-time.After(2 * time.Second): - t.Fatal("blocked reading fifo .git") - } -} - -func TestGitMetadataWriteCarveoutsRejectsFIFOCommondir(t *testing.T) { - root := t.TempDir() - gitDir := filepath.Join(root, ".git-real") - if err := os.MkdirAll(gitDir, 0o755); err != nil { - t.Fatal(err) - } - if err := syscall.Mkfifo(filepath.Join(gitDir, "commondir"), 0o644); err != nil { - t.Skip(err) - } - writeGitFile(t, filepath.Join(root, ".git"), "gitdir: "+gitDir+"\n") - done := make(chan []string, 1) - go func() { - done <- gitMetadataWriteCarveouts(root) - }() - select { - case got := <-done: - want := []string{ - filepath.Join(root, ".git"), - filepath.Join(gitDir, "hooks"), - filepath.Join(gitDir, "config"), - } - mustEqualCarveouts(t, got, want) - case <-time.After(2 * time.Second): - t.Fatal("blocked reading fifo commondir") - } + mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), []string{gitPath}) } diff --git a/internal/sandbox/runner.go b/internal/sandbox/runner.go index 0f0d0ce8b..8528e7e82 100644 --- a/internal/sandbox/runner.go +++ b/internal/sandbox/runner.go @@ -873,7 +873,11 @@ func writeRootCarveoutDenyRules(fs FileSystemPolicy) []string { } var out []string for _, root := range fs.WriteRoots { - for _, subpath := range containedReadOnlySubpaths(root.Root, root.ReadOnlySubpaths) { + for _, subpath := range root.ReadOnlySubpaths { + subpath = strings.TrimSpace(subpath) + if subpath == "" { + continue + } escaped := sandboxProfileString(subpath) out = append(out, `(deny file-write* (literal "`+escaped+`"))`, diff --git a/internal/sandbox/windows_acl.go b/internal/sandbox/windows_acl.go index 0895663c2..55d37d347 100644 --- a/internal/sandbox/windows_acl.go +++ b/internal/sandbox/windows_acl.go @@ -97,8 +97,10 @@ func windowsWriteRootCapabilities(config WindowsSandboxCommandConfig) ([]windows return nil, err } protected := make([]string, 0, len(root.ProtectedMetadataNames)+len(root.ReadOnlySubpaths)) - for _, subpath := range containedReadOnlySubpaths(rootPath, root.ReadOnlySubpaths) { - protected = append(protected, subpath) + for _, subpath := range root.ReadOnlySubpaths { + if trimmed := strings.TrimSpace(subpath); trimmed != "" { + protected = append(protected, trimmed) + } } for _, name := range root.ProtectedMetadataNames { if trimmed := strings.TrimSpace(name); trimmed != "" { From 02b3ebc152a5072f575d471fdb173a93704e56a5 Mon Sep 17 00:00:00 2001 From: jatmn Date: Thu, 27 Aug 2026 17:24:26 -0700 Subject: [PATCH 09/11] test(sandbox): skip unavailable symlink fixture --- internal/sandbox/profile_worktree_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/sandbox/profile_worktree_test.go b/internal/sandbox/profile_worktree_test.go index 5810946a3..93f93e0bc 100644 --- a/internal/sandbox/profile_worktree_test.go +++ b/internal/sandbox/profile_worktree_test.go @@ -80,7 +80,7 @@ func TestGitMetadataWriteCarveoutsPreservesDirectorySymlinkBehavior(t *testing.T t.Fatal(err) } if err := os.Symlink(realGit, filepath.Join(root, ".git")); err != nil { - t.Fatal(err) + t.Skipf("symlink unavailable: %v", err) } want := []string{ filepath.Join(root, ".git", "hooks"), From 79419efba095bb430b0535d0aee7576ea804c40f Mon Sep 17 00:00:00 2001 From: jatmn Date: Thu, 27 Aug 2026 18:03:20 -0700 Subject: [PATCH 10/11] fix(sandbox): preserve git symlink semantics --- internal/sandbox/profile.go | 12 +------- internal/sandbox/profile_worktree_test.go | 37 +++++++++++++++-------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/internal/sandbox/profile.go b/internal/sandbox/profile.go index 1ef6980bd..bbea3eff5 100644 --- a/internal/sandbox/profile.go +++ b/internal/sandbox/profile.go @@ -114,22 +114,12 @@ func gitMetadataWriteCarveouts(root string) []string { filepath.Join(gitPath, "config"), } } - if info.IsDir() { + if info.IsDir() || info.Mode()&os.ModeSymlink != 0 { return []string{ filepath.Join(gitPath, "hooks"), filepath.Join(gitPath, "config"), } } - if info.Mode()&os.ModeSymlink != 0 { - target, statErr := os.Stat(gitPath) - if statErr == nil && target.IsDir() { - // Preserve the pre-existing behavior for directory symlinks. - return []string{ - filepath.Join(gitPath, "hooks"), - filepath.Join(gitPath, "config"), - } - } - } return []string{gitPath} } diff --git a/internal/sandbox/profile_worktree_test.go b/internal/sandbox/profile_worktree_test.go index 93f93e0bc..9cbef5950 100644 --- a/internal/sandbox/profile_worktree_test.go +++ b/internal/sandbox/profile_worktree_test.go @@ -73,18 +73,29 @@ func TestGitMetadataWriteCarveoutsPlainCheckout(t *testing.T) { mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), want) } -func TestGitMetadataWriteCarveoutsPreservesDirectorySymlinkBehavior(t *testing.T) { - root := t.TempDir() - realGit := filepath.Join(root, "real.git") - if err := os.MkdirAll(realGit, 0o755); err != nil { - t.Fatal(err) - } - if err := os.Symlink(realGit, filepath.Join(root, ".git")); err != nil { - t.Skipf("symlink unavailable: %v", err) - } - want := []string{ - filepath.Join(root, ".git", "hooks"), - filepath.Join(root, ".git", "config"), +func TestGitMetadataWriteCarveoutsPreservesSymlinkBehavior(t *testing.T) { + for _, targetKind := range []string{"directory", "file", "dangling"} { + t.Run(targetKind, func(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "real.git") + switch targetKind { + case "directory": + if err := os.MkdirAll(target, 0o755); err != nil { + t.Fatal(err) + } + case "file": + if err := os.WriteFile(target, []byte("gitdir: ../outside\n"), 0o644); err != nil { + t.Fatal(err) + } + } + if err := os.Symlink(target, filepath.Join(root, ".git")); err != nil { + t.Skipf("symlink unavailable: %v", err) + } + want := []string{ + filepath.Join(root, ".git", "hooks"), + filepath.Join(root, ".git", "config"), + } + mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), want) + }) } - mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), want) } From 2fdff700376dd5087ed4828f282c422460aeef2a Mon Sep 17 00:00:00 2001 From: jatmn Date: Thu, 27 Aug 2026 18:20:00 -0700 Subject: [PATCH 11/11] fix(sandbox): limit git pointer carveout to regular files --- internal/sandbox/profile.go | 31 +++++++++---------- internal/sandbox/profile_worktree_test.go | 14 +++++++++ .../sandbox/profile_worktree_unix_test.go | 7 +++-- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/internal/sandbox/profile.go b/internal/sandbox/profile.go index bbea3eff5..8be099168 100644 --- a/internal/sandbox/profile.go +++ b/internal/sandbox/profile.go @@ -98,27 +98,24 @@ var sandboxFullyProtectedMetadataNames = []string{".zero", ".agents"} // gitMetadataWriteCarveouts returns the .git paths that stay write-denied under // the OS-level sandbox even though the rest of .git is writable to git -// subprocesses. Worktrees and submodules store .git as a pointer file, so they -// protect that file itself: constructing .git/hooks below a file makes bwrap -// fail before the command starts. The pointer is deliberately not parsed here; +// subprocesses. Worktrees and submodules store .git as a regular pointer file, +// so they protect that file itself: constructing .git/hooks below it makes +// bwrap fail before the command starts. All other entry and error states retain +// the legacy child paths. The pointer is deliberately not parsed here; // repository-controlled metadata must not redirect sandbox rules elsewhere. func gitMetadataWriteCarveouts(root string) []string { + return gitMetadataWriteCarveoutsWithLstat(root, os.Lstat) +} + +func gitMetadataWriteCarveoutsWithLstat(root string, lstat func(string) (os.FileInfo, error)) []string { gitPath := filepath.Join(root, ".git") - info, err := os.Lstat(gitPath) - if err != nil { - if !os.IsNotExist(err) { - return []string{gitPath} - } - return []string{ - filepath.Join(gitPath, "hooks"), - filepath.Join(gitPath, "config"), - } + children := []string{ + filepath.Join(gitPath, "hooks"), + filepath.Join(gitPath, "config"), } - if info.IsDir() || info.Mode()&os.ModeSymlink != 0 { - return []string{ - filepath.Join(gitPath, "hooks"), - filepath.Join(gitPath, "config"), - } + info, err := lstat(gitPath) + if err != nil || !info.Mode().IsRegular() { + return children } return []string{gitPath} } diff --git a/internal/sandbox/profile_worktree_test.go b/internal/sandbox/profile_worktree_test.go index 9cbef5950..e89d23953 100644 --- a/internal/sandbox/profile_worktree_test.go +++ b/internal/sandbox/profile_worktree_test.go @@ -1,6 +1,7 @@ package sandbox import ( + "errors" "os" "path/filepath" "testing" @@ -99,3 +100,16 @@ func TestGitMetadataWriteCarveoutsPreservesSymlinkBehavior(t *testing.T) { }) } } + +func TestGitMetadataWriteCarveoutsPreservesInspectionErrorBehavior(t *testing.T) { + root := t.TempDir() + gitPath := filepath.Join(root, ".git") + want := []string{ + filepath.Join(gitPath, "hooks"), + filepath.Join(gitPath, "config"), + } + got := gitMetadataWriteCarveoutsWithLstat(root, func(string) (os.FileInfo, error) { + return nil, errors.New("inspection failed") + }) + mustEqualCarveouts(t, got, want) +} diff --git a/internal/sandbox/profile_worktree_unix_test.go b/internal/sandbox/profile_worktree_unix_test.go index 02606d2df..e26ca2f0c 100644 --- a/internal/sandbox/profile_worktree_unix_test.go +++ b/internal/sandbox/profile_worktree_unix_test.go @@ -8,11 +8,14 @@ import ( "testing" ) -func TestGitMetadataWriteCarveoutsProtectsSpecialGitEntry(t *testing.T) { +func TestGitMetadataWriteCarveoutsPreservesSpecialGitEntryBehavior(t *testing.T) { root := t.TempDir() gitPath := filepath.Join(root, ".git") if err := syscall.Mkfifo(gitPath, 0o644); err != nil { t.Skip(err) } - mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), []string{gitPath}) + mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), []string{ + filepath.Join(gitPath, "hooks"), + filepath.Join(gitPath, "config"), + }) }