From 7fa06e20a007b1f3fb2bc502faecb832f5054691 Mon Sep 17 00:00:00 2001 From: Artem Matskevych <52060443+Reederey87@users.noreply.github.com> Date: Tue, 7 Jul 2026 16:52:27 -0400 Subject: [PATCH] fix(agent): grant the linked worktree's git dirs to the OS sandbox (P7-SANDBOX-01) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A DevStrap agent worktree is a git *linked* worktree whose index/objects/refs live in the parent clone's .git, outside the worktree write-allow. Under the default guarded policy with --sandbox auto (the common Mac dev host), the write confinement kernel-EPERM'd every `git add`/`git commit`, so `agent pr` had nothing to push — the core agent loop silently broke, untested (the only sandbox+git canary used a fresh nested repo). New git.Runner.WorktreeSandboxWriteDirs resolves the linked worktree's /{objects,refs,logs} + the per-worktree admin dir; all three sandbox backends (Seatbelt, bubblewrap, Landlock) and the --read-confine allow-list now grant them. The common dir's hooks/ and config are deliberately excluded (per-subpath, never the whole .git) — granting them would let a confined agent plant a hook/config that runs UNSANDBOXED on a later git op. Proven by a live Seatbelt e2e (TestSeatbeltAllowsLinkedWorktreeCommit): the commit FAILS without the grant and SUCCEEDS with it. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cli/agent.go | 28 +++- internal/cli/agent_sandbox_test.go | 4 +- internal/git/git.go | 69 +++++++++ internal/git/sandbox_writedirs_test.go | 98 +++++++++++++ internal/platform/sandbox.go | 11 ++ internal/platform/sandbox_bwrap_args.go | 8 ++ internal/platform/sandbox_gitdirs_test.go | 65 +++++++++ internal/platform/sandbox_landlock.go | 14 ++ internal/platform/sandbox_profile.go | 5 +- internal/platform/sandbox_read_confine.go | 6 + .../sandbox_worktree_commit_darwin_test.go | 131 ++++++++++++++++++ spec/05_MAC_FIRST_IMPLEMENTATION.md | 4 + spec/10_AGENT_WORKSPACES_AND_POLICIES.md | 2 +- spec/15_SECURITY_THREAT_MODEL.md | 2 +- spec/18_WORK_LOG.md | 26 ++++ 15 files changed, 464 insertions(+), 9 deletions(-) create mode 100644 internal/git/sandbox_writedirs_test.go create mode 100644 internal/platform/sandbox_gitdirs_test.go create mode 100644 internal/platform/sandbox_worktree_commit_darwin_test.go diff --git a/internal/cli/agent.go b/internal/cli/agent.go index 7d09f6b..9eed445 100644 --- a/internal/cli/agent.go +++ b/internal/cli/agent.go @@ -57,7 +57,7 @@ type agentSandboxLaunch struct { // empty anchor would silently drop every home-anchored credential deny while // still reporting the run as sandboxed (post-merge review, PR #107). // `--sandbox off` is the explicit escape hatch. -func agentSandboxSpec(worktreeDir, perRunTmp, logDir string, launch agentSandboxLaunch, runID string) (platform.SandboxSpec, error) { +func agentSandboxSpec(worktreeDir, perRunTmp, logDir string, gitDirs []string, launch agentSandboxLaunch, runID string) (platform.SandboxSpec, error) { userHome, err := os.UserHomeDir() if err != nil { return platform.SandboxSpec{}, fmt.Errorf("resolve user home for sandbox credential denies (use --sandbox off to run unconfined): %w", err) @@ -74,6 +74,7 @@ func agentSandboxSpec(worktreeDir, perRunTmp, logDir string, launch agentSandbox ViolationTag: sandboxViolationTag(runID), ReadConfine: launch.readConfine, ReadAllowExtra: launch.readAllow, + GitDirs: gitDirs, }, nil } @@ -352,7 +353,23 @@ func newAgentRunCommand(stdout io.Writer, opts *options) *cobra.Command { return err } runStart := time.Now() - commandErr := runAgentProcess(cmd.Context(), wt, run, agentCommand, stdout, sandboxLaunch) + // Resolve the worktree's git storage dirs here (we hold *options), + // only when the sandbox is active — otherwise the grant is unused + // and the two rev-parse forks are wasted. Best-effort: a resolution + // failure leaves the grant empty rather than blocking the run + // (P7-SANDBOX-01). + var sandboxGitDirs []string + if sandboxLaunch.enabled { + sandboxGitDirs, _ = gitRunner(opts).WorktreeSandboxWriteDirs(cmd.Context(), wt.Path) + if len(sandboxGitDirs) == 0 { + // A real fresh worktree always resolves; an empty grant here + // means the resolver hit an unexpected git error, and the + // agent's `git commit` would be EPERM'd inside the sandbox. + // Warn rather than fail silently (review: P7-SANDBOX-01). + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "warning: could not resolve the worktree's git dirs for the sandbox; git commits inside the run may be blocked (use --sandbox off if the agent must commit)") + } + } + commandErr := runAgentProcess(cmd.Context(), wt, run, agentCommand, stdout, sandboxLaunch, sandboxGitDirs) collectSandboxViolations(cmd.Context(), cmd.ErrOrStderr(), store, run, sandboxLaunch, runStart) diffSummary := agentDiffSummary(cmd.Context(), wt.Path, wt.BaseSHA) status := "complete" @@ -766,7 +783,7 @@ func pathWithin(root, path string) bool { return rel == "." || (rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))) } -func runAgentProcess(ctx context.Context, wt state.Worktree, run state.AgentRun, args []string, stdout io.Writer, sandboxLaunch agentSandboxLaunch) error { +func runAgentProcess(ctx context.Context, wt state.Worktree, run state.AgentRun, args []string, stdout io.Writer, sandboxLaunch agentSandboxLaunch, gitDirs []string) error { if err := os.MkdirAll(filepath.Dir(run.LogPath), 0o700); err != nil { return fmt.Errorf("create agent log dir: %w", err) } @@ -793,7 +810,10 @@ func runAgentProcess(ctx context.Context, wt state.Worktree, run state.AgentRun, } defer func() { _ = os.RemoveAll(perRunTmp) }() envOverrides["TMPDIR"] = perRunTmp - spec, err := agentSandboxSpec(wt.Path, perRunTmp, filepath.Dir(run.LogPath), sandboxLaunch, run.ID) + // gitDirs (resolved by the caller, which holds *options) grants the + // linked worktree's git storage dirs so the agent's `git add`/`commit` + // are not EPERM'd by the sandbox (P7-SANDBOX-01). + spec, err := agentSandboxSpec(wt.Path, perRunTmp, filepath.Dir(run.LogPath), gitDirs, sandboxLaunch, run.ID) if err != nil { return err } diff --git a/internal/cli/agent_sandbox_test.go b/internal/cli/agent_sandbox_test.go index 53b2af7..ccc7ac9 100644 --- a/internal/cli/agent_sandbox_test.go +++ b/internal/cli/agent_sandbox_test.go @@ -98,7 +98,7 @@ func (p passthroughSandbox) Command(_ context.Context, spec platform.SandboxSpec // silently vanished. func TestAgentSandboxSpecFailsClosedWithoutUserHome(t *testing.T) { t.Setenv("HOME", "") - if _, err := agentSandboxSpec("/wt", "/tmp/run", "/log", agentSandboxLaunch{devstrapHome: "/dsh"}, "arun_test"); err == nil { + if _, err := agentSandboxSpec("/wt", "/tmp/run", "/log", nil, agentSandboxLaunch{devstrapHome: "/dsh"}, "arun_test"); err == nil { t.Fatal("agentSandboxSpec succeeded without a resolvable user home; want fail-closed error") } } @@ -106,7 +106,7 @@ func TestAgentSandboxSpecFailsClosedWithoutUserHome(t *testing.T) { func TestAgentSandboxSpecAnchorsRealUserHome(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) - spec, err := agentSandboxSpec("/wt", "/tmp/run", "/log", agentSandboxLaunch{devstrapHome: "/dsh", denyNetwork: true}, "arun_test") + spec, err := agentSandboxSpec("/wt", "/tmp/run", "/log", nil, agentSandboxLaunch{devstrapHome: "/dsh", denyNetwork: true}, "arun_test") if err != nil { t.Fatalf("agentSandboxSpec: %v", err) } diff --git a/internal/git/git.go b/internal/git/git.go index 4203f67..2ab4c9c 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -545,6 +545,75 @@ func (r Runner) IsSquashMerged(ctx context.Context, dir, branch, baseRef string) return strings.TrimSpace(merged) == strings.TrimSpace(baseTree), nil } +// WorktreeSandboxWriteDirs returns the absolute git storage paths a linked +// worktree must be able to WRITE for `git add`/`git commit` to succeed under an +// OS sandbox: the shared object store, refs, and reflogs in the git-common-dir, +// plus the per-worktree admin dir (index/HEAD/COMMIT_EDITMSG/logs). It +// deliberately EXCLUDES the common dir itself (and thus hooks/ and config) — +// granting those would let a sandboxed agent plant a hook or config that +// executes UNSANDBOXED on a later git operation (P7-SANDBOX-01). Paths are +// symlink-resolved. A nil slice with no error is returned when dir is not inside +// a git worktree, so callers can grant nothing without special-casing. +func (r Runner) WorktreeSandboxWriteDirs(ctx context.Context, dir string) ([]string, error) { + common, err := r.Run(ctx, dir, "rev-parse", "--git-common-dir") + if err != nil { + return nil, nil // not a git worktree — nothing to grant + } + gitDir, err := r.Run(ctx, dir, "rev-parse", "--git-dir") + if err != nil { + return nil, nil + } + abs := func(p string) string { + p = strings.TrimSpace(p) + if p == "" { + return "" + } + if !filepath.IsAbs(p) { + p = filepath.Join(dir, p) + } + if resolved, rerr := filepath.EvalSymlinks(p); rerr == nil { + return resolved + } + return filepath.Clean(p) + } + commonAbs := abs(common) + gitDirAbs := abs(gitDir) + if commonAbs == "" || gitDirAbs == "" { + return nil, nil + } + // Resolve each storage subpath too: /objects can itself be a + // symlink (git alternates / a relocated object store), and Seatbelt matches + // the kernel-real path — so grant the resolved target, not the alias. + // EvalSymlinks fails on a not-yet-created path (e.g. no reflog); fall back + // to the clean join, which git creates under the already-resolved commonAbs. + evalJoin := func(elem string) string { + p := filepath.Join(commonAbs, elem) + if resolved, rerr := filepath.EvalSymlinks(p); rerr == nil { + return resolved + } + return p + } + dirs := []string{ + evalJoin("objects"), + evalJoin("refs"), + evalJoin("logs"), + } + // Add the per-worktree admin dir only for a genuine LINKED worktree, and + // only when it is actually under /worktrees/. gitDir == common is + // the main checkout (objects/refs/logs already cover its writes; never add + // the whole common dir — hooks/ and config live there). A --git-dir that + // resolves ELSEWHERE under the common dir (a malformed gitfile/commondir + // could point at /hooks or a sibling) is refused rather than + // granted, keeping the hooks/config exclusion robust against hostile setups. + if gitDirAbs != commonAbs { + worktreesRoot := filepath.Join(commonAbs, "worktrees") + if rel, rerr := filepath.Rel(worktreesRoot, gitDirAbs); rerr == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + dirs = append(dirs, gitDirAbs) + } + } + return dirs, nil +} + func (r Runner) WorktreeAdd(ctx context.Context, dir, path, branch, base string) error { if !safeBranchName(branch) { return fmt.Errorf("invalid git branch name %q", branch) diff --git a/internal/git/sandbox_writedirs_test.go b/internal/git/sandbox_writedirs_test.go new file mode 100644 index 0000000..b070d54 --- /dev/null +++ b/internal/git/sandbox_writedirs_test.go @@ -0,0 +1,98 @@ +package git + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +// TestWorktreeSandboxWriteDirs proves the resolver grants exactly the git +// storage a linked worktree writes for add/commit — objects, refs, logs, and +// the per-worktree admin dir — and NEVER the common dir itself or its +// hooks/config (which would be a sandbox escape, P7-SANDBOX-01). +func TestWorktreeSandboxWriteDirs(t *testing.T) { + repo, r := initSquashMergeRepo(t) // real local repo on branch main with one commit + ctx := context.Background() + + wt := filepath.Join(t.TempDir(), "wt") + if err := r.WorktreeAdd(ctx, repo, wt, "agent/x", "main"); err != nil { + t.Fatalf("WorktreeAdd: %v", err) + } + + dirs, err := r.WorktreeSandboxWriteDirs(ctx, wt) + if err != nil { + t.Fatalf("WorktreeSandboxWriteDirs: %v", err) + } + if len(dirs) != 4 { + t.Fatalf("want 4 grant dirs (objects/refs/logs + per-worktree admin), got %d: %v", len(dirs), dirs) + } + + commonReal := mustEval(t, filepath.Join(repo, ".git")) + var bases []string + sawWorktreeAdmin := false + for _, d := range dirs { + // Security invariants: never the common dir root, never hooks/, never config. + if d == commonReal { + t.Errorf("grant includes the common dir root %q — would expose hooks/config (sandbox escape)", d) + } + if strings.Contains(d, string(os.PathSeparator)+"hooks") || strings.HasSuffix(d, string(os.PathSeparator)+"config") { + t.Errorf("grant includes a hooks/config path %q", d) + } + bases = append(bases, filepath.Base(d)) + if strings.Contains(d, string(os.PathSeparator)+"worktrees"+string(os.PathSeparator)) { + sawWorktreeAdmin = true + } + } + for _, want := range []string{"objects", "refs", "logs"} { + if !contains(bases, want) { + t.Errorf("missing grant for %s; bases=%v", want, bases) + } + } + if !sawWorktreeAdmin { + t.Errorf("missing the per-worktree admin dir (…/worktrees/); dirs=%v", dirs) + } +} + +// TestWorktreeSandboxWriteDirsNonRepo returns (nil, nil) outside a git worktree +// so the caller grants nothing without special-casing. +func TestWorktreeSandboxWriteDirsNonRepo(t *testing.T) { + r := Runner{Bin: gitBinOrSkip(t), Timeout: 5 * time.Second} + dirs, err := r.WorktreeSandboxWriteDirs(context.Background(), t.TempDir()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if dirs != nil { + t.Fatalf("want nil grant outside a git worktree, got %v", dirs) + } +} + +func mustEval(t *testing.T, p string) string { + t.Helper() + r, err := filepath.EvalSymlinks(p) + if err != nil { + return filepath.Clean(p) + } + return r +} + +func contains(s []string, v string) bool { + for _, x := range s { + if x == v { + return true + } + } + return false +} + +func gitBinOrSkip(t *testing.T) string { + t.Helper() + bin, err := exec.LookPath("git") + if err != nil { + t.Skip("git not installed") + } + return bin +} diff --git a/internal/platform/sandbox.go b/internal/platform/sandbox.go index a51cf80..7f8022b 100644 --- a/internal/platform/sandbox.go +++ b/internal/platform/sandbox.go @@ -57,6 +57,17 @@ type SandboxSpec struct { // ReadAllowExtra are additional absolute read roots (repeatable // --read-allow) folded into the read-confinement allow-list. ReadAllowExtra []string + // GitDirs are absolute git storage paths a linked agent worktree must be + // able to WRITE for `git add`/`git commit` to succeed: the shared object + // store, refs, and reflogs in the clone's git-common-dir, plus the + // per-worktree admin dir (index/HEAD/COMMIT_EDITMSG). A DevStrap agent + // worktree is a git *linked* worktree whose .git lives in the parent clone, + // so without these the child's commit is EPERM'd and `agent pr` has nothing + // to push (P7-SANDBOX-01). The common dir's hooks/ and config are + // deliberately EXCLUDED — granting them would let the child plant a hook or + // config that executes UNSANDBOXED on a later git operation. Also folded + // into the read-confinement allow-list so git reads work under --read-confine. + GitDirs []string } // Sandbox wraps an agent argv in an OS-enforced confinement (AGEN-03 / diff --git a/internal/platform/sandbox_bwrap_args.go b/internal/platform/sandbox_bwrap_args.go index 07f75a3..8944eee 100644 --- a/internal/platform/sandbox_bwrap_args.go +++ b/internal/platform/sandbox_bwrap_args.go @@ -71,6 +71,14 @@ func bwrapArgs(spec SandboxSpec, maskDirs, maskFiles []string, opts bwrapOptions if spec.TmpDir != "" { args = append(args, "--bind", spec.TmpDir, spec.TmpDir) } + // The linked worktree's git storage dirs (objects/refs/logs/per-worktree + // admin) so `git commit` writes succeed — NOT the common dir's hooks/config + // (P7-SANDBOX-01). --bind-try tolerates an absent dir (e.g. no reflog). + for _, dir := range spec.GitDirs { + if dir != "" { + args = append(args, "--bind-try", dir, dir) + } + } // Mount ops are processed sequentially and later mounts override earlier // ones, so credential masks MUST come after the read-write binds. Under // read confinement the credential paths are already outside the exposed diff --git a/internal/platform/sandbox_gitdirs_test.go b/internal/platform/sandbox_gitdirs_test.go new file mode 100644 index 0000000..7063c68 --- /dev/null +++ b/internal/platform/sandbox_gitdirs_test.go @@ -0,0 +1,65 @@ +package platform + +import ( + "slices" + "strings" + "testing" +) + +// gitStorageDirs are the kind of paths WorktreeSandboxWriteDirs yields: the +// linked worktree's git storage, never the common dir root or hooks/config. +var gitStorageDirs = []string{ + "/home/dev/clone/.git/objects", + "/home/dev/clone/.git/refs", + "/home/dev/clone/.git/logs", + "/home/dev/clone/.git/worktrees/agent-x", +} + +// TestSBPLProfileGrantsGitDirs proves the Seatbelt write allow-list includes +// each git storage dir so an agent `git commit` in a linked worktree is not +// EPERM'd (P7-SANDBOX-01) — the platform where the default-on sandbox bites. +func TestSBPLProfileGrantsGitDirs(t *testing.T) { + spec := SandboxSpec{WorktreeDir: "/wt", TmpDir: "/tmp", GitDirs: gitStorageDirs} + profile := sbplProfile(spec, nil, nil) + for _, d := range gitStorageDirs { + if !strings.Contains(profile, `(subpath "`+d+`")`) { + t.Errorf("SBPL write allow-list missing git dir %q\n%s", d, profile) + } + } + // The common dir root must never be granted wholesale (hooks/config escape). + if strings.Contains(profile, `(subpath "/home/dev/clone/.git")`) { + t.Error("SBPL grants the whole common dir — hooks/config sandbox escape") + } +} + +// TestBwrapArgsGrantsGitDirs proves the bubblewrap write binds include each git +// storage dir (via --bind-try, tolerating an absent reflog). +func TestBwrapArgsGrantsGitDirs(t *testing.T) { + spec := SandboxSpec{WorktreeDir: "/wt", TmpDir: "/tmp", GitDirs: gitStorageDirs} + args := bwrapArgs(spec, nil, nil, bwrapOptions{}) + for _, d := range gitStorageDirs { + if indexSequence(args, "--bind-try", d, d) == -1 { + t.Errorf("bwrap args missing --bind-try %s %s\n%v", d, d, args) + } + } + if slices.Contains(args, "/home/dev/clone/.git") { + t.Error("bwrap binds the whole common dir — hooks/config sandbox escape") + } +} + +// TestReadConfineRootsIncludesGitDirs proves git storage stays readable under +// --read-confine (readonly policy) so git read ops work; the RW grant already +// makes them writable elsewhere. +func TestReadConfineRootsIncludesGitDirs(t *testing.T) { + roots := readConfineRoots(SandboxSpec{ + WorktreeDir: "/wt", + TmpDir: "/tmp", + ReadConfine: true, + GitDirs: gitStorageDirs, + }) + for _, d := range gitStorageDirs { + if !slices.Contains(roots, d) { + t.Errorf("read-confine roots missing git dir %q: %v", d, roots) + } + } +} diff --git a/internal/platform/sandbox_landlock.go b/internal/platform/sandbox_landlock.go index 3bca49c..d881b1a 100644 --- a/internal/platform/sandbox_landlock.go +++ b/internal/platform/sandbox_landlock.go @@ -126,6 +126,20 @@ func applyLandlockPolicy(spec SandboxSpec) error { // such renames inside the worktree. rules = append(rules, landlock.RWDirs(rw...).WithRefer()) } + // The linked worktree's git storage dirs (objects/refs/logs/per-worktree + // admin) so `git commit` succeeds — NOT the common dir's hooks/config + // (P7-SANDBOX-01). Separate rule so a rare absent dir (no reflog yet) is + // skipped rather than failing the whole ruleset; WithRefer for git's + // tmp-object -> objects/xx rename promotion. + var gitRW []string + for _, dir := range spec.GitDirs { + if dir != "" { + gitRW = append(gitRW, dir) + } + } + if len(gitRW) > 0 { + rules = append(rules, landlock.RWDirs(gitRW...).WithRefer().IgnoreIfMissing()) + } // Shell plumbing (`> /dev/null`) and pty allocation; IgnoreIfMissing // because minimal containers lack some nodes. Wider than bwrap's fresh // --dev (these nodes are machine-shared), named in landlockLimitations. diff --git a/internal/platform/sandbox_profile.go b/internal/platform/sandbox_profile.go index c4e7cdc..fe6d8f8 100644 --- a/internal/platform/sandbox_profile.go +++ b/internal/platform/sandbox_profile.go @@ -51,7 +51,10 @@ func sbplProfile(spec SandboxSpec, denyReadDirs, denyReadFiles []string) string b.WriteString(")\n") } b.WriteString("(allow file-write*\n") - for _, dir := range []string{spec.WorktreeDir, spec.TmpDir} { + // WorktreeDir + TmpDir, plus the linked worktree's git storage dirs + // (objects/refs/logs/per-worktree-admin) so `git commit` works — but NOT + // the common dir's hooks/config (P7-SANDBOX-01). + for _, dir := range append([]string{spec.WorktreeDir, spec.TmpDir}, spec.GitDirs...) { if dir == "" { continue } diff --git a/internal/platform/sandbox_read_confine.go b/internal/platform/sandbox_read_confine.go index 3c2d2c8..cdb8f48 100644 --- a/internal/platform/sandbox_read_confine.go +++ b/internal/platform/sandbox_read_confine.go @@ -73,6 +73,12 @@ func readConfineRoots(spec SandboxSpec) []string { for _, extra := range spec.ReadAllowExtra { add(extra) } + // The linked worktree's git storage dirs must be readable under + // --read-confine so `git status`/`git commit` can read objects/refs + // (P7-SANDBOX-01); they are already writable via the backends' RW grants. + for _, dir := range spec.GitDirs { + add(dir) + } return roots } diff --git a/internal/platform/sandbox_worktree_commit_darwin_test.go b/internal/platform/sandbox_worktree_commit_darwin_test.go new file mode 100644 index 0000000..80a3e1b --- /dev/null +++ b/internal/platform/sandbox_worktree_commit_darwin_test.go @@ -0,0 +1,131 @@ +package platform + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// TestSeatbeltAllowsLinkedWorktreeCommit is the load-bearing proof for +// P7-SANDBOX-01: a DevStrap agent worktree is a git *linked* worktree whose +// index/objects/refs live in the parent clone's .git, so the default write +// confinement (worktree + tmp only) EPERMs `git commit`. This test commits in a +// real linked worktree under the live Seatbelt sandbox and asserts it FAILS +// without the git-dir grant and SUCCEEDS with it. Env-gated like the other +// Seatbelt enforcement e2e. +func TestSeatbeltAllowsLinkedWorktreeCommit(t *testing.T) { + if os.Getenv("DEVSTRAP_SANDBOX_E2E") != "1" { + t.Skip("set DEVSTRAP_SANDBOX_E2E=1 to run the Seatbelt worktree-commit test") + } + sb := SeatbeltSandbox{} + if err := sb.Available(); err != nil { + t.Skipf("sandbox-exec unavailable: %v", err) + } + gitBin, err := exec.LookPath("git") + if err != nil { + t.Skip("git not installed") + } + + root := t.TempDir() + clone := filepath.Join(root, "clone") + tmpAllow := filepath.Join(root, "tmp") + logs := filepath.Join(root, "logs") + for _, d := range []string{clone, tmpAllow, logs} { + if err := os.MkdirAll(d, 0o700); err != nil { + t.Fatal(err) + } + } + realGit := func(dir string, args ...string) { + t.Helper() + cmd := exec.Command(gitBin, args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=true", "GIT_TERMINAL_PROMPT=0") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out) + } + } + realGit(clone, "init", "-b", "main") + realGit(clone, "config", "user.name", "t") + realGit(clone, "config", "user.email", "t@example.com") + if err := os.WriteFile(filepath.Join(clone, "README.md"), []byte("base\n"), 0o644); err != nil { + t.Fatal(err) + } + realGit(clone, "add", "README.md") + realGit(clone, "commit", "-m", "init") + + worktree := filepath.Join(root, "wt") + realGit(clone, "worktree", "add", "-b", "agent/x", "--", worktree, "main") + + // Resolve the git storage dirs the same way WorktreeSandboxWriteDirs does. + revParse := func(flag string) string { + cmd := exec.Command(gitBin, "rev-parse", flag) + cmd.Dir = worktree + out, err := cmd.Output() + if err != nil { + t.Fatalf("rev-parse %s: %v", flag, err) + } + p := strings.TrimSpace(string(out)) + if !filepath.IsAbs(p) { + p = filepath.Join(worktree, p) + } + if resolved, rerr := filepath.EvalSymlinks(p); rerr == nil { + return resolved + } + return filepath.Clean(p) + } + common := revParse("--git-common-dir") + gitDir := revParse("--git-dir") + gitDirs := []string{ + filepath.Join(common, "objects"), + filepath.Join(common, "refs"), + filepath.Join(common, "logs"), + gitDir, + } + + commitUnder := func(gd []string) error { + if err := os.WriteFile(filepath.Join(worktree, "change.txt"), []byte("x\n"), 0o644); err != nil { + t.Fatal(err) + } + spec := SandboxSpec{ + WorktreeDir: worktree, + TmpDir: tmpAllow, + LogDir: logs, + UserHome: root, // no real dotfiles here; keep credential denies harmless + GitDirs: gd, + } + // git add + git commit as one sandboxed shell invocation so both index + // and object/ref writes are exercised under the same confinement. + sc, err := sb.Command(context.Background(), spec, + []string{"/bin/sh", "-c", "git add change.txt && git commit -m sandboxed"}) + if err != nil { + t.Fatal(err) + } + defer sc.Cleanup() + cmd := exec.Command(sc.Argv[0], sc.Argv[1:]...) //nolint:gosec // test fixture argv + cmd.Dir = worktree + cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=true", "GIT_TERMINAL_PROMPT=0") + return cmd.Run() + } + + // Without the git-dir grant, the commit's index/object writes to the parent + // clone's .git are EPERM'd — this is the P7-SANDBOX-01 bug. + if err := commitUnder(nil); err == nil { + t.Fatal("commit in a linked worktree SUCCEEDED without the git-dir grant; the fix is not load-bearing (or the sandbox is not enforcing)") + } + // git leaves a stale index.lock behind after the denied write; clear it so + // the second attempt starts clean. + _ = os.Remove(filepath.Join(gitDir, "index.lock")) + + // With the grant, the same commit succeeds. + if err := commitUnder(gitDirs); err != nil { + t.Fatalf("commit in a linked worktree FAILED with the git-dir grant: %v", err) + } + // Verify the commit really landed on the branch. + out, err := exec.Command(gitBin, "-C", worktree, "log", "-1", "--pretty=%s").Output() + if err != nil || strings.TrimSpace(string(out)) != "sandboxed" { + t.Fatalf("expected the sandboxed commit on HEAD, got %q (err %v)", strings.TrimSpace(string(out)), err) + } +} diff --git a/spec/05_MAC_FIRST_IMPLEMENTATION.md b/spec/05_MAC_FIRST_IMPLEMENTATION.md index 33a7c35..620fd4f 100644 --- a/spec/05_MAC_FIRST_IMPLEMENTATION.md +++ b/spec/05_MAC_FIRST_IMPLEMENTATION.md @@ -343,3 +343,7 @@ Cross-platform findings (`XP-*`, from `docs/audits/AUDIT_RECOMMENDATIONS_2026-06 - **Ship the portable Go core on macOS + Ubuntu before any native magic (`XP-01`):** the eager-clone materialization (`EAGER-*`), encrypted env/draft sync (`DRAFT-*`), and cloud hub backend (`HUB-*`) must run identically on both platforms via portable Go. No native daemon, FSEvents watcher, LaunchAgent installer, or StrapFS is in scope this cycle. - **Keep Mac specifics behind adapters so Ubuntu stays first-class (`XP-02`):** the `internal/platform` watcher/service/keychain/editor seams remain the only place macOS behavior may diverge; the Linux fsnotify/inotify + periodic-reconciliation path must reach feature parity for the eager-sync loop. - **Defer the native daemon and StrapFS (`XP-03`, Deferred section):** the LaunchAgent/FSEvents/Endpoint Security/File Provider/FUSE material above is explicitly deferred. Materialization stays eager clone-everything on `devstrap sync`; there is no placeholder/lazy-VFS layer in this design. + +## Audit follow-ups (2026-07-07) + +- **Seatbelt sandbox must grant the linked worktree's git dirs (`P7-SANDBOX-01`):** a DevStrap agent worktree is a git *linked* worktree whose index/objects/refs live in the parent clone's `.git`, outside the worktree dir — so the default write confinement kernel-EPERM'd `git add`/`git commit`, silently breaking the `agent run → agent pr` loop on Macs. The Seatbelt profile (and the Linux bwrap/Landlock backends) now also write-allow the linked worktree's `/{objects,refs,logs}` and the per-worktree admin dir, resolved by `git.Runner.WorktreeSandboxWriteDirs`; the common dir's `hooks/` and `config` are deliberately excluded (granting them would let the child plant a hook that runs unsandboxed). Kernel-proven by the env-gated `TestSeatbeltAllowsLinkedWorktreeCommit`. Full detail in `spec/10_AGENT_WORKSPACES_AND_POLICIES.md`. diff --git a/spec/10_AGENT_WORKSPACES_AND_POLICIES.md b/spec/10_AGENT_WORKSPACES_AND_POLICIES.md index 81c9ff5..0d6c47a 100644 --- a/spec/10_AGENT_WORKSPACES_AND_POLICIES.md +++ b/spec/10_AGENT_WORKSPACES_AND_POLICIES.md @@ -147,7 +147,7 @@ The current wrapper-level enforcement oversells its safety and must not be prese - **Command/file policy is argv-substring matching, trivially bypassed by any interpreter** (`AGEN-01`). `bash -c "…"`, `python -c`, base64-decode, `rm -fr /` (variant spacing), variable indirection, or a script file all evade the deny list, so the default `guarded` profile actually gives an agent full filesystem **read** and **network exfil**. Treat substring matching as a guardrail against accidents, not a security boundary. - **Credential-env inheritance is fixed, but the wrapper is still not a sandbox** (`AGEN-02`/`SECU-02`): `AgentAllowlist` excludes `SSH_AUTH_SOCK`, avoids inheriting the user's `HOME`, and repoints `HOME` to the worktree. The remaining risk is broader: the subprocess still has normal filesystem and network capability unless an OS sandbox (Seatbelt / bubblewrap-landlock-seccomp) constrains it. - **Profile semantics are partially misleading** (`AGEN-04`, partly fixed): `ephemeral-ci` is now accepted and `readonly` denies redirection and known mutating commands (argv-aware, wrapper-level only); but `cautious` is still behaviorally identical to `guarded`, and `ephemeral-ci` has no cloud/auto-cleanup semantics yet — implement the distinctions or fold the profiles. -- **OS-enforced sandbox: macOS Seatbelt, Linux bubblewrap, and Linux Landlock fallback shipped** (`AGEN-03`/`P4-GIT-03`, 2026-07-05). The wraps above are kernel-enforced for the child and everything it spawns — the interpreter bypasses in the first bullet can no longer write outside the worktree; full-fidelity backends also read-deny/mask the denied credential paths and (under `readonly`/`cautious`) deny ordinary network sockets. **Both Linux backends install a seccomp syscall denylist** (mount/kexec/ptrace/keyring/io_uring/legacy-escape syscalls → `EPERM`; `DEVSTRAP_SANDBOX_SECCOMP=off` opt-out). **`sandbox.violation` telemetry is live as unsigned local visibility**: backend/mode/limitations are recorded per run, macOS Seatbelt denials are collected from tagged unified-log rows after the run, scrubbed, and shown by `agent show`/`doctor`; Linux runtime denial detection remains future. **Tighter read confinement is shipped** (`--read-confine auto|on|off`, env `DEVSTRAP_SANDBOX_READ_CONFINE`, default-on for the `readonly` policy; `--read-allow ` adds roots): all three backends restrict the child's reads to the worktree/tmp, the OS toolchain/system roots, and the `$HOME` build caches instead of the whole disk — the Seatbelt profile denies all reads then re-allows the roots (credential denies stay last so they out-rank the allows; a global `file-read-metadata` allow keeps stat/traversal working), bubblewrap exposes only the roots via `--ro-bind-try`, and Landlock restricts its `RODirs` grant to the roots (which finally gives the Landlock fallback a credential-read boundary). `--sandbox require` refuses to launch if a requested read confinement cannot be enforced. The remaining OS-sandbox direction is containerization (spec/14). Linux caveats: bubblewrap credential masks hide paths (ENOENT/empty) rather than returning EPERM, nested-sandbox tools (Chrome headless, flatpak, bwrap-in-bwrap) can break under `--disable-userns`, and Landlock is a reduced fallback (credential reads still allowed; network deny TCP-only at ABI >= 4; no mount/pid namespace; and — because Landlock has no `--new-session` analogue — the `TIOCSTI` terminal-injection escape stays open on the Landlock path, since seccomp does not arg-filter `ioctl`). A kernel without seccomp-filter support degrades to a reduced-guarantees notice, not a hard failure. `--sandbox require` accepts Landlock as a real write-confinement boundary except when the policy demands a network deny that the backend cannot enforce at all (ABI < 4), in which case it fails with the policy exit class before worktree creation; a TCP-only deny (ABI >= 4) passes `require` with a warning that UDP, QUIC, and unix-domain sockets stay open; `--sandbox off` is the explicit escape hatch. +- **OS-enforced sandbox: macOS Seatbelt, Linux bubblewrap, and Linux Landlock fallback shipped** (`AGEN-03`/`P4-GIT-03`, 2026-07-05). The wraps above are kernel-enforced for the child and everything it spawns — the interpreter bypasses in the first bullet can no longer write outside the worktree; full-fidelity backends also read-deny/mask the denied credential paths and (under `readonly`/`cautious`) deny ordinary network sockets. **Because an agent worktree is a git _linked_ worktree, all three backends additionally grant write to the linked worktree's git storage — the shared object store, refs, and reflogs in the clone's git-common-dir, plus the per-worktree admin dir — so the agent's `git add`/`git commit` are not kernel-EPERM'd (`P7-SANDBOX-01`, resolved by `git.Runner.WorktreeSandboxWriteDirs`). The common dir's `hooks/` and `config` are deliberately NOT granted: granting them would let the child plant a hook or config that executes UNSANDBOXED on a later git operation. These git dirs also join the `--read-confine` allow-list so git reads work under the `readonly` policy; a live Seatbelt e2e (`TestSeatbeltAllowsLinkedWorktreeCommit`) proves the commit fails without the grant and succeeds with it.** **Both Linux backends install a seccomp syscall denylist** (mount/kexec/ptrace/keyring/io_uring/legacy-escape syscalls → `EPERM`; `DEVSTRAP_SANDBOX_SECCOMP=off` opt-out). **`sandbox.violation` telemetry is live as unsigned local visibility**: backend/mode/limitations are recorded per run, macOS Seatbelt denials are collected from tagged unified-log rows after the run, scrubbed, and shown by `agent show`/`doctor`; Linux runtime denial detection remains future. **Tighter read confinement is shipped** (`--read-confine auto|on|off`, env `DEVSTRAP_SANDBOX_READ_CONFINE`, default-on for the `readonly` policy; `--read-allow ` adds roots): all three backends restrict the child's reads to the worktree/tmp, the OS toolchain/system roots, and the `$HOME` build caches instead of the whole disk — the Seatbelt profile denies all reads then re-allows the roots (credential denies stay last so they out-rank the allows; a global `file-read-metadata` allow keeps stat/traversal working), bubblewrap exposes only the roots via `--ro-bind-try`, and Landlock restricts its `RODirs` grant to the roots (which finally gives the Landlock fallback a credential-read boundary). `--sandbox require` refuses to launch if a requested read confinement cannot be enforced. The remaining OS-sandbox direction is containerization (spec/14). Linux caveats: bubblewrap credential masks hide paths (ENOENT/empty) rather than returning EPERM, nested-sandbox tools (Chrome headless, flatpak, bwrap-in-bwrap) can break under `--disable-userns`, and Landlock is a reduced fallback (credential reads still allowed; network deny TCP-only at ABI >= 4; no mount/pid namespace; and — because Landlock has no `--new-session` analogue — the `TIOCSTI` terminal-injection escape stays open on the Landlock path, since seccomp does not arg-filter `ioctl`). A kernel without seccomp-filter support degrades to a reduced-guarantees notice, not a hard failure. `--sandbox require` accepts Landlock as a real write-confinement boundary except when the policy demands a network deny that the backend cannot enforce at all (ABI < 4), in which case it fails with the policy exit class before worktree creation; a TCP-only deny (ABI >= 4) passes `require` with a warning that UDP, QUIC, and unix-domain sockets stay open; `--sandbox off` is the explicit escape hatch. - **The file-path deny list is narrower than this spec** and ignores the project's stronger sensitive-file detector (`AGEN-05`); unify on the single `spec/11` ignore/deny compiler. Direction: move to an **allowlist + OS sandbox** model, strip credential-bearing env by default, and make the default profile's true capability legible. diff --git a/spec/15_SECURITY_THREAT_MODEL.md b/spec/15_SECURITY_THREAT_MODEL.md index 3812ad0..cd1856d 100644 --- a/spec/15_SECURITY_THREAT_MODEL.md +++ b/spec/15_SECURITY_THREAT_MODEL.md @@ -88,7 +88,7 @@ Mitigation: - log redaction; - tainted-log handling when secrets are present. -Reality (`AGEN-01`, `AGEN-02`/`SECU-02`, updated 2026-07-05): the credential-env leak is fixed — `SSH_AUTH_SOCK` is excluded and `HOME` is repointed to the worktree — and the wrapper argv/file policy (still substring-based and interpreter-bypassable on its own) is now backed by a kernel boundary **on macOS and Linux**. macOS wraps the child in a Seatbelt profile (`P4-GIT-03`) that confines writes to the worktree/tmp dirs (the 0600 run log is parent-written and child-untouchable), denies reads of `~/.ssh`-class credential paths — resolving each anchor's leaf symlinks and denying both the literal alias and the symlink-real target so a `~/.ssh -> /elsewhere` alias cannot dodge the deny — and denies all network for `readonly`/`cautious`. Linux first tries bubblewrap with a read-only root, read-write worktree/tmp binds, targeted credential masks, optional net namespace, user namespace, pid namespace, die-with-parent, and new-session protections; userns-restricted hosts now degrade to the Landlock fallback instead of advisory-only. Landlock still enforces the most important fallback boundary — writes and raw `truncate(2)` are confined to the worktree/tmp allow dirs — but it does NOT deny credential reads (that guarantee stays bubblewrap-only), its network deny is TCP bind/connect only at kernel Landlock ABI >= 4 and is not enforced below ABI 4, and it has no mount or pid namespace. **Both Linux backends additionally install a seccomp syscall denylist** (`P4-GIT-03` slice 4): mount, kexec/module-load, ptrace/tracing, keyring, io_uring, and legacy-escape syscalls return `EPERM`, closing the class of kernel-surface and namespace-escape attacks the filesystem/network boundaries do not cover; `clone`/`unshare`/`setns`/`execve`/`fork` stay allowed so nested sandboxes and the agent's own launches work, and `DEVSTRAP_SANDBOX_SECCOMP=off` is the opt-out. `sandbox.violation` telemetry is now live as **unsigned local visibility**, not the signed audit log: `agent_runs` records backend/mode/limitations for every run, and macOS Seatbelt denials are collected from tagged unified-log rows into scrubbed local `sandbox_violations` rows surfaced by `agent show` and `doctor`. Linux runtime denial detection remains future. What the sandbox does NOT yet cover: XPC/`mach-lookup` and unix-domain sockets under `(deny network*)` on macOS — an allow-default profile still lets a `readonly`/`cautious` child talk to system daemons (e.g. out-of-process `nsurlsessiond` transfers) or a local socket proxy, so the network deny is best-effort against a deliberately evasive agent (tightening `mach-lookup` is a follow-up slice); pathname unix sockets on the Linux read-only root remain connectable under `--unshare-net` (e.g. `/var/run/docker.sock`, the Linux analogue of the mach-lookup gap); Linux tmpfs/`/dev/null` masks hide credential paths (ENOENT/empty) rather than returning EPERM; Landlock keeps credential reads readable by design; the seccomp denylist does NOT arg-filter `ioctl`, so `TIOCSTI` terminal injection stays covered only by bubblewrap's `--new-session` and remains open on the Landlock path (no `--new-session` analogue); and the `ptrace` deny also breaks in-sandbox debuggers/`strace` (an accepted trade). Non-credential reads are confinable via `--read-confine` (shipped 2026-07-05, default-on for the `readonly` policy): all three backends restrict reads to the worktree/tmp, OS toolchain roots, and `$HOME` build caches, so the rest of `$HOME` and other projects become unreadable — and because the credential dirs are outside the allow-list, read confinement gives even the Landlock fallback a credential-read boundary. Without it, reads stay allow-default (the documented, opt-in tradeoff, not a defect). `--sandbox require` is the fail-closed mode. +Reality (`AGEN-01`, `AGEN-02`/`SECU-02`, updated 2026-07-05): the credential-env leak is fixed — `SSH_AUTH_SOCK` is excluded and `HOME` is repointed to the worktree — and the wrapper argv/file policy (still substring-based and interpreter-bypassable on its own) is now backed by a kernel boundary **on macOS and Linux**. macOS wraps the child in a Seatbelt profile (`P4-GIT-03`) that confines writes to the worktree/tmp dirs (the 0600 run log is parent-written and child-untouchable), denies reads of `~/.ssh`-class credential paths — resolving each anchor's leaf symlinks and denying both the literal alias and the symlink-real target so a `~/.ssh -> /elsewhere` alias cannot dodge the deny — and denies all network for `readonly`/`cautious`. Linux first tries bubblewrap with a read-only root, read-write worktree/tmp binds, targeted credential masks, optional net namespace, user namespace, pid namespace, die-with-parent, and new-session protections; userns-restricted hosts now degrade to the Landlock fallback instead of advisory-only. Landlock still enforces the most important fallback boundary — writes and raw `truncate(2)` are confined to the worktree/tmp allow dirs — but it does NOT deny credential reads (that guarantee stays bubblewrap-only), its network deny is TCP bind/connect only at kernel Landlock ABI >= 4 and is not enforced below ABI 4, and it has no mount or pid namespace. **Both Linux backends additionally install a seccomp syscall denylist** (`P4-GIT-03` slice 4): mount, kexec/module-load, ptrace/tracing, keyring, io_uring, and legacy-escape syscalls return `EPERM`, closing the class of kernel-surface and namespace-escape attacks the filesystem/network boundaries do not cover; `clone`/`unshare`/`setns`/`execve`/`fork` stay allowed so nested sandboxes and the agent's own launches work, and `DEVSTRAP_SANDBOX_SECCOMP=off` is the opt-out. `sandbox.violation` telemetry is now live as **unsigned local visibility**, not the signed audit log: `agent_runs` records backend/mode/limitations for every run, and macOS Seatbelt denials are collected from tagged unified-log rows into scrubbed local `sandbox_violations` rows surfaced by `agent show` and `doctor`. Linux runtime denial detection remains future. What the sandbox does NOT yet cover: XPC/`mach-lookup` and unix-domain sockets under `(deny network*)` on macOS — an allow-default profile still lets a `readonly`/`cautious` child talk to system daemons (e.g. out-of-process `nsurlsessiond` transfers) or a local socket proxy, so the network deny is best-effort against a deliberately evasive agent (tightening `mach-lookup` is a follow-up slice); pathname unix sockets on the Linux read-only root remain connectable under `--unshare-net` (e.g. `/var/run/docker.sock`, the Linux analogue of the mach-lookup gap); Linux tmpfs/`/dev/null` masks hide credential paths (ENOENT/empty) rather than returning EPERM; Landlock keeps credential reads readable by design; the seccomp denylist does NOT arg-filter `ioctl`, so `TIOCSTI` terminal injection stays covered only by bubblewrap's `--new-session` and remains open on the Landlock path (no `--new-session` analogue); and the `ptrace` deny also breaks in-sandbox debuggers/`strace` (an accepted trade). Non-credential reads are confinable via `--read-confine` (shipped 2026-07-05, default-on for the `readonly` policy): all three backends restrict reads to the worktree/tmp, OS toolchain roots, and `$HOME` build caches, so the rest of `$HOME` and other projects become unreadable — and because the credential dirs are outside the allow-list, read confinement gives even the Landlock fallback a credential-read boundary. Without it, reads stay allow-default (the documented, opt-in tradeoff, not a defect). `--sandbox require` is the fail-closed mode. **Linked-worktree git grant (`P7-SANDBOX-01`, 2026-07-07):** because an agent worktree is a git *linked* worktree whose index/objects/refs live in the parent clone's `.git` (outside the worktree write-allow), all three backends now also write-allow the linked worktree's `/{objects,refs,logs}` and per-worktree admin dir (`git.Runner.WorktreeSandboxWriteDirs`), or the agent's own `git add`/`git commit` are kernel-EPERM'd and `agent pr` has nothing to push. Security-critical detail: the grant is per-subpath, **never the common dir itself** — its `hooks/` and `config` are withheld, because granting write there would let a confined agent plant a hook or config that executes UNSANDBOXED on any later git operation in that clone (a sandbox escape). This is also why the grant cannot be "just bind the whole `.git`": Landlock cannot carve a read-only hole out of an RW grant, so the subpaths are enumerated explicitly. ### Threat: destructive sync deletes code diff --git a/spec/18_WORK_LOG.md b/spec/18_WORK_LOG.md index c0fc2f3..331d84e 100644 --- a/spec/18_WORK_LOG.md +++ b/spec/18_WORK_LOG.md @@ -31,6 +31,32 @@ Follow-ups: Entries are newest-first: each code-modifying cycle prepends ONE dated entry at the top. +## 2026-07-07 — fix(agent): grant the linked worktree's git dirs to the OS sandbox (P7-SANDBOX-01) + +Changed: +- `internal/git/git.go`: new `Runner.WorktreeSandboxWriteDirs` — resolves the linked worktree's git storage a `git add`/`commit` writes: `/{objects,refs,logs}` + the per-worktree admin dir (`--git-dir`). Deliberately EXCLUDES the common dir itself (and thus `hooks/`/`config`): granting those would let a sandboxed agent plant a hook/config that executes UNSANDBOXED on a later git op (that's why it's a per-subpath grant, not a whole-common-dir grant — Landlock also cannot carve a read-only hole out of an RW grant). Returns `(nil, nil)` outside a worktree; symlink-resolved paths. +- `internal/platform/sandbox.go`: new `SandboxSpec.GitDirs` (documented as excluding hooks/config). +- `internal/platform/sandbox_profile.go` (Seatbelt), `sandbox_bwrap_args.go` (bubblewrap `--bind-try`), `sandbox_landlock.go` (`RWDirs(...).WithRefer().IgnoreIfMissing()`): all three write allow-lists now include `spec.GitDirs`. +- `internal/platform/sandbox_read_confine.go`: `GitDirs` join the `--read-confine` allow-list so git reads work under the `readonly` policy. +- `internal/cli/agent.go`: resolve the git dirs at the run site (holds `*options`) and thread them through `runAgentProcess` → `agentSandboxSpec`; best-effort (a resolution failure leaves the grant empty rather than blocking the run). +- `spec/10`: documented the git-dir grant + the hooks/config exclusion rationale. + +Why: a DevStrap agent worktree is a git *linked* worktree whose index/HEAD/objects/refs live in the parent clone's `.git`, outside the worktree dir. Under the default `guarded` policy with `--sandbox auto` (the common Mac dev host), the write confinement (worktree + tmp only) kernel-EPERM'd every `git add`/`git commit`, so `agent pr` had nothing to push — the core agent loop silently broke. The only prior e2e canary committed into a *fresh nested* repo, never exercising the linked-worktree path. + +Post-review (dual review — opus + Codex gpt-5.5): opus verdict clean/ship-it, Codex needs-fix on two minors — both applied. (1) `git.go` now refuses a `--git-dir` that resolves outside `/worktrees/` (a malformed gitfile/commondir could otherwise point `--git-dir` at `/hooks` and slip it into the write grant — the security-invariant hardening), and symlink-resolves the `objects/refs/logs` subpaths (git alternates). (2) `agent.go` resolves the git dirs only when the sandbox is enabled (no wasted forks under `--sandbox off`, dead error branch dropped) and warns when an enabled-sandbox grant is unexpectedly empty (rather than silently regressing to the EPERM). Deferred by agreement (latent, `readonly` blocks commits anyway): the `--read-confine` common-`config` read path — tracked in Follow-ups. + +Validated: +- `gofmt`/`go vet`/`golangci-lint run` clean (0 issues) on the touched packages; `go build ./...`. +- `go test ./internal/platform/... ./internal/git/... ./internal/cli/...` pass, incl. new `TestWorktreeSandboxWriteDirs` (security invariants: never the common root/hooks/config), `TestSBPLProfileGrantsGitDirs`, `TestBwrapArgsGrantsGitDirs`, `TestReadConfineRootsIncludesGitDirs`. +- **Load-bearing proof on a real Mac** (`DEVSTRAP_SANDBOX_E2E=1 go test -run TestSeatbeltAllowsLinkedWorktreeCommit`): a `git add && git commit` in a real linked worktree under the live Seatbelt kernel sandbox FAILS without the git-dir grant and SUCCEEDS with it, landing the commit on the branch. + +Follow-ups (review-surfaced; separate findings, out of scope for this focused P1 fix): +- Linux backends (bubblewrap/Landlock) are covered by the shared `GitDirs` wiring + arg tests; the Docker `sandbox_landlock_e2e_test` could be extended to a real linked worktree for parity. +- Ledger: move `P7-SANDBOX-01` to *Recently shipped* (Pass-7 open 17→16) on rebase after the Pass-7 audit PR (#141) lands, per the multi-PR-wave convention. +- **`--read-confine` git-read completeness:** under the `readonly` policy (`--read-confine` on) the linked worktree's `/config` is not in the read allow-list, so `git status`/`git log` may not read core config; this policy already blocks commits at the CLI token-scan, so it is latent, but read ops could be tightened by exposing the common dir read-only (write still confined to objects/refs/logs/admin). +- **commondir-redirect escape (PRE-EXISTING, not widened here):** the per-worktree admin dir grant includes the `commondir`/`gitdir` pointer files; overwriting `commondir` (or a `/.git` file, writable since the original WorktreeDir grant) can redirect git to an agent-controlled tree carrying an evil `config` (`core.fsmonitor=`) or hooks that `agentDiffSummary`'s post-run `git status`/`agent pr` would execute UNSANDBOXED. This class predates this PR (the `/.git` vector already existed) and is not enlarged by it. Fix belongs in a separate finding: harden the environment (`GIT_CONFIG_GLOBAL=/dev/null` is already set; add `core.fsmonitor=false`/`-c protocol...`, or re-validate `.git`/`commondir` point at the expected admin dir) before any unsandboxed git op on an agent-touched worktree. +- **Clone-scoped, not branch-scoped:** `/{objects,refs}` are shared by every linked worktree of the clone, so a sandboxed agent can rewrite sibling branches / write arbitrary objects in the shared clone (integrity, not RCE) — inherent to git's linked-worktree model; the spec text is honest about the "shared object store." Worth acknowledging in the threat model as clone-scoped isolation. + ## 2026-07-06 — docs: unattended-operation wave close-out (PRs #136–#139) Changed: