fix(sandbox): resolve git metadata carveouts for worktrees#805
Conversation
.git is a file (gitdir: ...) in worktrees/submodules, not a dir. The hooks/config write-deny carveout assumed a directory and pointed at <root>/.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.
Walkthrough
ChangesGit metadata carveout resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/sandbox/profile.go`:
- Around line 72-75: Update the gitDir resolution flow around resolveGitDir so
an unresolved non-directory .git file does not retain the literal root/.git
fallback; return no carveouts for stale, malformed, or unreadable gitdir
pointers. Preserve the literal fallback only when .git is absent, and add a
regression test covering a stale gitdir: pointer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a4a360b2-9b5c-48d9-b5ae-4d6f0af98017
📒 Files selected for processing (2)
internal/sandbox/profile.gointernal/sandbox/profile_worktree_test.go
| if info, err := os.Stat(gitDir); err == nil && !info.IsDir() { | ||
| if real := resolveGitDir(root); real != "" { | ||
| gitDir = resolveGitCommonDir(real) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Return no carveouts when a .git file cannot be resolved.
If resolveGitDir returns "" for a stale, malformed, or unreadable .git file, this leaves gitDir as <root>/.git and returns descendants of that file. That recreates the documented bwrap “Not a directory” failure and blocks sandboxed tools. Keep the literal fallback only when .git is absent; return no carveouts for an unresolved non-directory .git, with a regression test for a stale gitdir: pointer.
Proposed fix
if info, err := os.Stat(gitDir); err == nil && !info.IsDir() {
- if real := resolveGitDir(root); real != "" {
- gitDir = resolveGitCommonDir(real)
+ real := resolveGitDir(root)
+ if real == "" {
+ return nil
}
+ gitDir = resolveGitCommonDir(real)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if info, err := os.Stat(gitDir); err == nil && !info.IsDir() { | |
| if real := resolveGitDir(root); real != "" { | |
| gitDir = resolveGitCommonDir(real) | |
| } | |
| if info, err := os.Stat(gitDir); err == nil && !info.IsDir() { | |
| real := resolveGitDir(root) | |
| if real == "" { | |
| return nil | |
| } | |
| gitDir = resolveGitCommonDir(real) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/sandbox/profile.go` around lines 72 - 75, Update the gitDir
resolution flow around resolveGitDir so an unresolved non-directory .git file
does not retain the literal root/.git fallback; return no carveouts for stale,
malformed, or unreadable gitdir pointers. Preserve the literal fallback only
when .git is absent, and add a regression test covering a stale gitdir: pointer.
1 participant
Problem
Worktree/submodule checkouts store
.gitas a file (gitdir: <path>), not a directory. The sandbox's git metadata write-deny carveout (gitMetadataWriteCarveoutsininternal/sandbox/profile.go) hardcoded<root>/.git/hooks+<root>/.git/config, assuming.gitis a directory.In a worktree that path has no mkdir-able parent (
.gitis a file), so the Linux bwrap backend's--tmpfs <root>/.git/hooksfails:→ every sandboxed tool is blocked in a worktree checkout (bash, file tools, etc.).
Fix
.gitis a file → resolve the real gitdir from thegitdir:pointer, followcommondir, and deny the sharedhooks/config(a worktree is protected like a plain clone)..gitis a dir or absent → unchanged (<root>/.git/{hooks,config}).resolveGitDir,resolveGitCommonDir.Tests
internal/sandbox/profile_worktree_test.go: worktree resolves the common dir + never points under the.gitfile; plain checkout keeps literal paths.TestSeatbeltProfileAllowsGitWritesExceptHooksAndConfigstill passes.Verification
<worktree>/.git/hooksbwrap: Can't mkdir parents ... Not a directory<common>/.git/hooksgit rev-parseruns fineSummary by CodeRabbit
.gitis a file.