From 05719beabeb5c7f6fcb609f287c401a1709a220a Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:23:35 -0400 Subject: [PATCH] fix(sandbox): stop blocking git fetch/commit/add by unblocking .git writes The sandbox denied every write under .git for shell-executed commands, so any git operation that touches its own metadata (fetch, commit, add, pull, merge, stash) failed under the default sandbox. Narrow the block to .git/hooks (auto-executing scripts) and .git/config (remote URLs, credential.helper, core.hooksPath), the two subpaths that are actually dangerous to write; the rest of .git stays writable to git subprocesses since those writes go through git's own invariants. The app-level auto-allow gate for direct file-tool writes (write_file, edit_file, apply_patch) is unaffected and still treats all of .git as protected, since hand-editing git's internals bypasses those invariants. --- ...box_policy_windows_unavailable.golden.json | 5 +- internal/sandbox/manager_test.go | 9 ++- internal/sandbox/profile.go | 29 ++++++++- internal/sandbox/runner_test.go | 29 +++++++++ .../seatbelt_integration_darwin_test.go | 64 +++++++++++++++++++ 5 files changed, 133 insertions(+), 3 deletions(-) diff --git a/internal/cli/testdata/sandbox_policy_windows_unavailable.golden.json b/internal/cli/testdata/sandbox_policy_windows_unavailable.golden.json index 1fd749bfb..a081d124a 100644 --- a/internal/cli/testdata/sandbox_policy_windows_unavailable.golden.json +++ b/internal/cli/testdata/sandbox_policy_windows_unavailable.golden.json @@ -40,8 +40,11 @@ "writeRoots": [ { "root": "$WORKSPACE", + "readOnlySubpaths": [ + "$WORKSPACE/.git/hooks", + "$WORKSPACE/.git/config" + ], "protectedMetadataNames": [ - ".git", ".zero", ".agents" ] diff --git a/internal/sandbox/manager_test.go b/internal/sandbox/manager_test.go index 5dc754acf..d535d0d9e 100644 --- a/internal/sandbox/manager_test.go +++ b/internal/sandbox/manager_test.go @@ -41,9 +41,16 @@ func TestPermissionProfileFromPolicyBuildsWorkspaceWriteProfile(t *testing.T) { if !stringSliceContains(profile.FileSystem.ReadRoots, profileRootPath()) { t.Fatalf("read roots = %#v, want full read root %q", profile.FileSystem.ReadRoots, profileRootPath()) } - if !stringSliceContains(profile.FileSystem.WriteRoots[0].ProtectedMetadataNames, ".git") || !stringSliceContains(profile.FileSystem.WriteRoots[0].ProtectedMetadataNames, ".zero") { + if !stringSliceContains(profile.FileSystem.WriteRoots[0].ProtectedMetadataNames, ".zero") || !stringSliceContains(profile.FileSystem.WriteRoots[0].ProtectedMetadataNames, ".agents") { t.Fatalf("protected metadata names = %#v, want workspace metadata protected", profile.FileSystem.WriteRoots[0].ProtectedMetadataNames) } + resolvedRoot := profile.FileSystem.WriteRoots[0].Root + wantGitCarveouts := []string{filepath.Join(resolvedRoot, ".git", "hooks"), filepath.Join(resolvedRoot, ".git", "config")} + for _, want := range wantGitCarveouts { + if !stringSliceContains(profile.FileSystem.WriteRoots[0].ReadOnlySubpaths, want) { + t.Fatalf("read-only subpaths = %#v, want git metadata carveout %q", profile.FileSystem.WriteRoots[0].ReadOnlySubpaths, want) + } + } if len(profile.FileSystem.DenyRead) != 1 || len(profile.FileSystem.DenyWrite) != 1 { t.Fatalf("deny paths = %#v / %#v, want one each", profile.FileSystem.DenyRead, profile.FileSystem.DenyWrite) } diff --git a/internal/sandbox/profile.go b/internal/sandbox/profile.go index 525bf0258..a565edba2 100644 --- a/internal/sandbox/profile.go +++ b/internal/sandbox/profile.go @@ -39,8 +39,34 @@ type NetworkPolicy struct { Mode NetworkMode `json:"mode"` } +// protectedMetadataNames marks control-plane directories where the app-level +// auto-allow gate (see relativePathTouchesProtectedMetadata in engine.go) +// always requires a prompt for direct file-tool writes (write_file, edit_file, +// apply_patch): hand-editing git's objects/refs/index or Zero's own state +// bypasses git's and Zero's own consistency checks, regardless of subpath. var protectedMetadataNames = []string{".git", ".zero", ".agents"} +// sandboxFullyProtectedMetadataNames are the metadata directories the OS-level +// sandbox write-denies in full for shell-executed commands. .git is +// deliberately excluded here: git subprocesses (fetch, commit, add, merge, +// pull, stash, ...) need to write objects, refs, the index, and FETCH_HEAD, +// and those writes go through git's own invariants, unlike a raw file-tool +// write. Only .git/hooks (auto-executing scripts) and .git/config (remote +// URLs, credential.helper, core.hooksPath) stay write-denied, via +// 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). +func gitMetadataWriteCarveouts(root string) []string { + return []string{ + filepath.Join(root, ".git", "hooks"), + filepath.Join(root, ".git", "config"), + } +} + func DefaultPermissionProfile(workspaceRoot string) PermissionProfile { return PermissionProfileFromPolicy(workspaceRoot, DefaultPolicy(), nil) } @@ -65,7 +91,8 @@ func PermissionProfileFromPolicy(workspaceRoot string, policy Policy, scope *Sco for _, root := range roots { writeRoots = append(writeRoots, WritableRoot{ Root: root, - ProtectedMetadataNames: append([]string{}, protectedMetadataNames...), + ReadOnlySubpaths: gitMetadataWriteCarveouts(root), + ProtectedMetadataNames: append([]string{}, sandboxFullyProtectedMetadataNames...), }) } return PermissionProfile{ diff --git a/internal/sandbox/runner_test.go b/internal/sandbox/runner_test.go index 75047d77b..94c1709b6 100644 --- a/internal/sandbox/runner_test.go +++ b/internal/sandbox/runner_test.go @@ -461,6 +461,35 @@ func TestSeatbeltProfileProtectsMetadataAndDenyOrdering(t *testing.T) { } } +// TestSeatbeltProfileAllowsGitWritesExceptHooksAndConfig locks in the fix for +// git subprocesses (fetch, commit, add, ...) failing under the sandbox: the +// default profile must stop write-denying the whole .git tree and only carve +// out .git/hooks and .git/config, which stay dangerous (auto-executing +// scripts, remote/credential-helper rewrites) regardless of what wrote them. +func TestSeatbeltProfileAllowsGitWritesExceptHooksAndConfig(t *testing.T) { + workspace := t.TempDir() + profile := DefaultPermissionProfile(workspace) + sbpl := seatbeltProfileFromPermissionProfile(profile, Policy{Mode: ModeEnforce}, "") + + resolvedWorkspace := normalizeProfilePath(workspace) + gitRegex := `(deny file-write* (regex #"^` + regexpQuoteMeta(resolvedWorkspace) + `/\.git(/.*)?$"))` + if strings.Contains(sbpl, gitRegex) { + t.Fatalf("seatbelt profile must not blanket-deny the whole .git tree:\n%s", sbpl) + } + hooksPath := sandboxProfileString(filepath.Join(resolvedWorkspace, ".git", "hooks")) + configPath := sandboxProfileString(filepath.Join(resolvedWorkspace, ".git", "config")) + for _, want := range []string{ + `(deny file-write* (literal "` + hooksPath + `"))`, + `(deny file-write* (subpath "` + hooksPath + `"))`, + `(deny file-write* (literal "` + configPath + `"))`, + `(deny file-write* (subpath "` + configPath + `"))`, + } { + if !strings.Contains(sbpl, want) { + t.Fatalf("seatbelt profile missing %q:\n%s", want, sbpl) + } + } +} + func TestSandboxExecProfileTagsDenialsWhenMonitoring(t *testing.T) { off := sandboxExecProfile([]string{"/ws"}, Policy{Mode: ModeEnforce, EnforceWorkspace: true}, "") if strings.Contains(off, "with message") { diff --git a/internal/sandbox/seatbelt_integration_darwin_test.go b/internal/sandbox/seatbelt_integration_darwin_test.go index d07ad1050..ef310ee60 100644 --- a/internal/sandbox/seatbelt_integration_darwin_test.go +++ b/internal/sandbox/seatbelt_integration_darwin_test.go @@ -101,6 +101,70 @@ func TestSeatbeltEnforcesExtraWriteRoots(t *testing.T) { }) } +// TestSeatbeltAllowsGitMetadataWritesExceptHooksAndConfig proves under a real +// sandbox-exec run that git subprocesses can write their own metadata (the +// fix for git fetch/commit/add failing under the sandbox), while .git/hooks +// and .git/config - the two subpaths that can turn a write into code +// execution or a credential/remote hijack - stay kernel-denied. +func TestSeatbeltAllowsGitMetadataWritesExceptHooksAndConfig(t *testing.T) { + if _, err := exec.LookPath("sandbox-exec"); err != nil { + t.Skipf("sandbox-exec unavailable: %v", err) + } + backend := SelectBackend(BackendOptions{}) + if !backend.Available || backend.Name != BackendMacOSSeatbelt { + t.Skipf("host sandbox backend is not sandbox-exec: %s", backend.Message) + } + + workspace := t.TempDir() + if err := os.MkdirAll(filepath.Join(workspace, ".git", "hooks"), 0o755); err != nil { + t.Fatalf("MkdirAll .git/hooks: %v", err) + } + if err := os.WriteFile(filepath.Join(workspace, ".git", "config"), []byte("[core]\n"), 0o644); err != nil { + t.Fatalf("WriteFile .git/config: %v", err) + } + engine := NewEngine(EngineOptions{ + WorkspaceRoot: workspace, + Policy: DefaultPolicy(), + Backend: backend, + }) + resolvedWorkspace := resolvedTestPath(t, workspace) + + t.Run("WriteToGitMetadataSucceeds", func(t *testing.T) { + target := filepath.Join(resolvedWorkspace, ".git", "FETCH_HEAD") + output, runErr := runSeatbeltShellWrite(t, engine, target, "fetch-head-ok") + if runErr != nil { + t.Fatalf("git-managed write under .git failed: %v\noutput: %s", runErr, output) + } + assertSeatbeltFileContent(t, target, "fetch-head-ok") + }) + + t.Run("WriteToGitHooksIsDenied", func(t *testing.T) { + target := filepath.Join(resolvedWorkspace, ".git", "hooks", "pre-commit") + output, runErr := runSeatbeltShellWrite(t, engine, target, "backdoor") + if runErr == nil { + t.Fatalf("write to .git/hooks succeeded, want seatbelt denial\noutput: %s", output) + } + if _, statErr := os.Lstat(target); !os.IsNotExist(statErr) { + t.Fatalf("Lstat(%s) = %v, want not-exist", target, statErr) + } + }) + + t.Run("WriteToGitConfigIsDenied", func(t *testing.T) { + target := filepath.Join(resolvedWorkspace, ".git", "config") + output, runErr := runSeatbeltShellWrite(t, engine, target, "[credential]\n\thelper = evil\n") + if runErr == nil { + t.Fatalf("write to .git/config succeeded, want seatbelt denial\noutput: %s", output) + } + content, err := os.ReadFile(target) + if err != nil { + t.Fatalf("read back .git/config: %v", err) + } + if string(content) != "[core]\n" { + t.Fatalf(".git/config content = %q, want unchanged", content) + } + }) +} + // runSeatbeltShellWrite launches /bin/sh through the engine's sandbox-exec // wrapping and asks it to write content to target. It returns the combined // output and the run error so callers can assert success or kernel denial.