Skip to content

fix(sandbox): grant git-admin dirs for linked worktrees (#30)#31

Open
NoRiceToday wants to merge 5 commits into
TNG:mainfrom
NoRiceToday:sepia-visor
Open

fix(sandbox): grant git-admin dirs for linked worktrees (#30)#31
NoRiceToday wants to merge 5 commits into
TNG:mainfrom
NoRiceToday:sepia-visor

Conversation

@NoRiceToday

Copy link
Copy Markdown
Contributor

Fixes #30.

Problem

omac sandbox run grants the workdir read+write. For a plain clone that covers
.git (it lives inside the workdir), so git works. For a linked worktree,
git splits state: the workdir is <repo>/.worktrees/<name>/ but the git-admin
dir is <repo>/.git/worktrees/<name>/ — a sibling outside the workdir. So
git add/commit hit EPERM writing the object store, refs, reflogs and the
per-worktree index/HEAD, and an agent can't commit from any linked worktree.

Fix

Detect a linked worktree (.git is a file), resolve the shared common dir via
the admin dir's commondir, and grant the commit-relevant subdirs at the
workdir's access level:

  • rw: objects/, refs/, logs/, worktrees/<name>/
  • ro: config, info, packed-refs (blocks core.hooksPath / credential.helper mutation)
  • deny: hooks/ (a writable hooks dir would run un-sandboxed as the host user on the next host-side commit)

No-op for a plain clone or a submodule (whose admin dir has no commondir).

Security hardening

Grant paths are derived from in-workdir file content (.git, commondir) a
prior sandboxed session could tamper with, and both backends symlink-canonicalize
grants into kernel rules. A planted objects -> ~/.ssh symlink would otherwise
emit a write-allow on the target and — since SBPL write-allows come after the
protected denies — punch through the ~/.ssh deny (sandbox escape). Confined by
enforcing git's <common>/worktrees/<name> structural invariant and admitting
only entries whose symlink-resolved path physically lives inside the resolved
common dir.

Verification

  • Unit tests: full resolve->SBPL/bwrap pipeline against a real git worktree,
    plus regressions for the symlink-escape and spoofed-commondir vectors.
  • End-to-end under real Seatbelt: bug reproduced on base (commit lost); fixed
    binary commits cleanly (exit 0, fsck clean); pre-hardening binary allowed the
    symlink escape, hardened binary denies it (secret untouched).

Known limitation

git branch -d/-D and git tag -d fail inside a sandboxed worktree — git's
deletion transaction needs a lock in the ungranted common-dir root — but fail
cleanly (exit 1, ref preserved, no corruption). Commits emit a non-fatal
packed-refs.lock EPERM yet succeed; gc/pack-refs ref-packing no-ops. All
other operations (commit/amend/branch/checkout/rebase/stash) are unaffected.

Sajjad Ahmad added 3 commits July 1, 2026 15:46
`omac sandbox run` grants the workdir read+write. For a plain clone that
covers `.git` (it lives inside the workdir), but a linked worktree splits
repo state: the workdir is `<repo>/.worktrees/<name>/` while the git-admin
dir is `<repo>/.git/worktrees/<name>/` — a sibling outside the workdir. So
`git add`/`commit` hit EPERM writing the object store, refs, reflogs and
per-worktree index/HEAD, and the agent can't commit from any worktree.

Detect a linked worktree (`.git` is a file), resolve the shared common dir
via the admin dir's `commondir`, and restore plain-clone parity by granting
the commit-relevant subdirs at the workdir's access level:

  - allow (rw): objects, refs, logs, packed-refs, worktrees/<name>
  - read-only:  config, info  (blocks core.hooksPath / credential.helper)
  - deny:       hooks/         (a writable hooks dir would run un-sandboxed
                                as the host user on the next host-side commit)

No-op for a plain clone or a submodule (whose admin dir has no `commondir`).

Verified below the process boundary: a real-`git worktree` pipeline test
drives ResolveGrants -> SBPL and bwrap argv generation and asserts the
object store is bound read+write, config read-only, and hooks masked.

Signed-off-by: Sajjad Ahmad <sajjad.ahmad@tngtech.com>
The worktree grant paths are derived from in-workdir file content
(`.git`, `<admin>/commondir`) that a prior sandboxed session could have
tampered with, and both backends symlink-canonicalize every grant into a
kernel rule. A malicious agent with rw workdir could plant a crafted
`.git` + admin/`commondir` chain whose resolved common dir it controls,
with `objects` a symlink to a sensitive dir (e.g. ~/.ssh). Because SBPL
emits write-allows AFTER protected-path denies (last match wins), the
symlink-resolved write grant would punch through the ~/.ssh denial —
read+write escape on the next `omac sandbox run` in that workdir.

Confine the grant:
- Enforce git's structural invariant: admin must be <common>/worktrees/<name>,
  rejecting a commondir aimed at an unrelated location.
- Admit only subdirs whose symlink-resolved path physically lives inside
  the resolved common dir; drop any entry that escapes (pathWithinRoot).

Adds regression tests proving a symlinked `objects`/`refs` never widens a
grant to the target, and a spoofed commondir yields no worktree grants.

Signed-off-by: Sajjad Ahmad <sajjad.ahmad@tngtech.com>
End-to-end testing under real Seatbelt showed the earlier "make
packed-refs writable" change was both ineffective and unnecessary:

- git 2.x's ref transaction tries to create <common>/packed-refs.LOCK in
  the common-dir root on every ref update. That root is deliberately not
  granted (it holds config and hooks), so the lock EPERMs — but git falls
  back to the loose-ref path and the commit succeeds (verified: exit 0,
  fsck clean, refs correct). Granting the packed-refs *file* rw cannot
  silence this: the blocker is the lock sibling in the ungranted root.
- Granting the common root rw to allow the lock would, on macOS, let a
  config write slip through (SBPL emits write-allows last, after the
  protected denies), defeating the core.hooksPath guard — the exact vector
  TNG#30 closes. So the harmless lock noise is the accepted cost.

A normal add/commit only ever writes loose refs under refs/; packed-refs
is only rewritten by gc/pack-refs, which need that ungranted lock anyway.
So packed-refs is correctly read-only (git still reads it for status/log
and for committing onto a packed ref). Documented the limitation on
gitWorktreeGrants.

Signed-off-by: Sajjad Ahmad <sajjad.ahmad@tngtech.com>
@NoRiceToday NoRiceToday marked this pull request as draft July 1, 2026 14:39
@NoRiceToday

NoRiceToday commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Note

Resolved in afffc75 ("fix(sandbox): allow linked-worktree commit hooks to run (read-only)"). hooks/ is now read-only + executable (never writable), the contained() symlink check is kept on the read grant, and a real-Seatbelt e2e proves the hook runs during a sandboxed commit while the hooks dir stays unwritable. The section below is kept for context.


Post-merge finding from host e2e testing (linked-worktree commit)

Ran a real omac sandbox run … -- git commit inside a linked worktree on macOS (Seatbelt). Good news: the worktree grant works — the commit lands, refs are correct. But the e2e surfaced a workflow regression plus a design question worth settling before calling this production-ready.

Finding 1 — repo commit hooks are blocked, breaking hook-enforced workflows

gitWorktreeGrants currently hard-denies <common>/hooks (via ProtectedPaths, which emits both deny file-read* and deny file-write*). So for any repo whose hooks enforce policy — e.g. a prepare-commit-msg/commit-msg hook checking message format or an issue key — git commit inside the sandbox fails outright; the only way through is to bypass hooks (--no-verify / core.hooksPath=/dev/null). That silently defeats legitimate, host-authored governance for in-sandbox commits. A hook is not inherently "arbitrary code we must block" — it is often the exact quality gate the team relies on.

Finding 2 — packed-refs.lock EPERM noise

Expected and non-fatal (git falls back to loose refs). Already documented on the function. No change.

Threat models for .git/hooks

  • T1 — tampering for host-side execution (the vector Sandbox: git add/commit fail in linked worktrees (git-admin dir outside workdir) #30 closes): a sandboxed agent writes/modifies a hook that then runs un-sandboxed on the host at your next commit. Mitigation required: deny write.
  • T2 — the repo's own hooks are untrusted: if you sandbox because the repo is untrusted, you may not want to execute even its existing hooks (they'd run sandboxed, but still run). Mitigation: also deny read/execute.

Hard-deny covers both, but punishes the common case (your repo, your hooks — T1 only) by breaking hook execution entirely.

Proposed change: grant hooks/ read-only by default (done in afffc75)

Moved hooks/ out of denyAdds and into the read-only set (readable + executable, never writable):

  • Closes T1 — no write allow + (deny default) ⇒ the agent cannot create or modify a hook, so nothing new can persist to run on the host.
  • Restores hook execution — host-authored hooks run (sandboxed, contained), so governance works.
  • T2 stays explicit — for untrusted-repo runs a hard-deny/opt-out remains the right tool;

Guardrail the change must not skip (done)

Read-only hooks/ must go through the same contained() / pathWithinRoot symlink-canonicalization the other worktree grants use. Otherwise a planted hooks -> ~/.ssh symlink (the common dir is derived from in-workdir content a prior session could tamper with) would widen a read grant to the target — a brand-new escape the read-only change would introduce. An adversarial test for exactly that (hooks -> secret must grant nothing under secret) was added before touching the grant.

Flip the linked-worktree shared hooks dir from a hard deny to read-only:
readable+runnable so host-authored commit hooks (prepare-commit-msg,
commit-msg, pre-commit) execute inside the sandbox, but never writable so
the agent still cannot plant a hook that would run un-sandboxed on the
host's next commit (the TNG#30 persistence vector). The read grant stays inside
the contained() symlink-containment check, since unlike a deny it would
otherwise follow a planted hooks->secret symlink to the target.

Add a real-Seatbelt e2e (rooted under $HOME, not $TMPDIR, so the baseline
temp-write grant doesn't mask the read-only hooks grant) proving the hook
runs during a sandboxed commit while the hooks dir stays unwritable.

Signed-off-by: Sajjad Ahmad <sajjad.ahmad@tngtech.com>
@NoRiceToday NoRiceToday marked this pull request as ready for review July 5, 2026 07:20
@NoRiceToday NoRiceToday requested a review from nhuelstng July 6, 2026 06:20
Signed-off-by: Sajjad Ahmad <sajjad.ahmad@tngtech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sandbox: git add/commit fail in linked worktrees (git-admin dir outside workdir)

1 participant