Skip to content

fix(tui): allow worktree git metadata writes in sandbox#3356

Closed
cyq1017 wants to merge 1 commit into
Hmbown:mainfrom
cyq1017:codex/codewhale-3355-worktree-sandbox
Closed

fix(tui): allow worktree git metadata writes in sandbox#3356
cyq1017 wants to merge 1 commit into
Hmbown:mainfrom
cyq1017:codex/codewhale-3355-worktree-sandbox

Conversation

@cyq1017

@cyq1017 cyq1017 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Fixes #3355.

Problem:
Git worktrees store mutable Git metadata outside the worktree root, so the macOS seatbelt workspace-write sandbox blocks commands like git add unless the user switches to YOLO/trust mode.

Change:
When workspace-write resolves writable roots, detect linked-worktree .git pointer files, require a commondir, and only add the derived worktree gitdir plus shared .git directory when the gitdir is under the shared .git/worktrees/ directory. Plan mode remains read-only and YOLO remains full access.

Verification:

  • cargo test -p codewhale-tui workspace_write_includes_git_worktree_metadata_roots
  • cargo test -p codewhale-tui sandbox::policy
  • cargo test -p codewhale-tui sandbox_policy_for_mode_returns_correct_policy_per_mode
  • cargo fmt --all -- --check
  • git diff --check

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for Git worktrees in the sandbox policy by allowing the gitdir and commondir derived from a workspace .git pointer to be included as writable roots. The feedback highlights a critical issue where resolve_gitdir_pointer fails if executed from a subdirectory of a worktree because it only checks for .git in the immediate root directory. To resolve this, it is recommended to traverse upwards using root.ancestors() to find the .git file, and to update the unit tests to cover subdirectory resolution.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +237 to +255
fn resolve_gitdir_pointer(root: &Path) -> Option<PathBuf> {
let contents = fs::read_to_string(root.join(".git")).ok()?;
let value = contents
.lines()
.find_map(|line| line.strip_prefix("gitdir:"))?
.trim();
if value.is_empty() {
return None;
}

let path = PathBuf::from(value);
let resolved = if path.is_absolute() {
path
} else {
root.join(path)
};

resolved.canonicalize().ok()
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If a command is executed from a subdirectory of a git worktree (which is extremely common), root.join(".git") will not exist because the .git file is only located at the root of the worktree. As a result, the sandbox will fail to detect the worktree metadata roots, and git commands will be blocked by the sandbox.

To fix this, we should traverse upwards using root.ancestors() to find the .git file (or directory, in which case we can stop early as it's a normal repository).

fn resolve_gitdir_pointer(root: &Path) -> Option<PathBuf> {
    for ancestor in root.ancestors() {
        let git_path = ancestor.join(".git");
        if git_path.is_file() {
            if let Ok(contents) = fs::read_to_string(&git_path) {
                if let Some(value) = contents
                    .lines()
                    .find_map(|line| line.trim().strip_prefix("gitdir:"))
                {
                    let value = value.trim();
                    if value.is_empty() {
                        return None;
                    }

                    let path = PathBuf::from(value);
                    let resolved = if path.is_absolute() {
                        path
                    } else {
                        ancestor.join(path)
                    };

                    return resolved.canonicalize().ok();
                }
            }
        } else if git_path.is_dir() {
            // Normal git repository, no external worktree metadata roots needed
            return None;
        }
    }
    None
}

Comment on lines +410 to +440
fn workspace_write_includes_git_worktree_metadata_roots() {
let tmp = tempfile::tempdir().expect("tempdir");
let common_git_dir = tmp.path().join("main-repo").join(".git");
let worktree_git_dir = common_git_dir.join("worktrees").join("feature");
let worktree = tmp.path().join("feature-worktree");
std::fs::create_dir_all(&worktree_git_dir).expect("mkdir gitdir");
std::fs::create_dir_all(&worktree).expect("mkdir worktree");
std::fs::write(
worktree.join(".git"),
format!("gitdir: {}\n", worktree_git_dir.display()),
)
.expect("write git pointer");
std::fs::write(worktree_git_dir.join("commondir"), "../..").expect("write commondir");

let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![worktree.clone()],
network_access: true,
exclude_tmpdir: true,
exclude_slash_tmp: true,
};

let root_paths: Vec<PathBuf> = policy
.get_writable_roots(&worktree)
.into_iter()
.map(|root| root.root)
.collect();

assert!(root_paths.contains(&worktree.canonicalize().expect("canonical worktree")));
assert!(root_paths.contains(&worktree_git_dir.canonicalize().expect("canonical gitdir")));
assert!(root_paths.contains(&common_git_dir.canonicalize().expect("canonical common git")));
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Let's update the test to also verify that worktree metadata roots are correctly resolved when executing from a subdirectory of the worktree.

    #[test]
    fn workspace_write_includes_git_worktree_metadata_roots() {
        let tmp = tempfile::tempdir().expect("tempdir");
        let common_git_dir = tmp.path().join("main-repo").join(".git");
        let worktree_git_dir = common_git_dir.join("worktrees").join("feature");
        let worktree = tmp.path().join("feature-worktree");
        std::fs::create_dir_all(&worktree_git_dir).expect("mkdir gitdir");
        std::fs::create_dir_all(&worktree).expect("mkdir worktree");
        std::fs::write(
            worktree.join(".git"),
            format!("gitdir: {}\n", worktree_git_dir.display()),
        )
        .expect("write git pointer");
        std::fs::write(worktree_git_dir.join("commondir"), "../..").expect("write commondir");

        let policy = SandboxPolicy::WorkspaceWrite {
            writable_roots: vec![worktree.clone()],
            network_access: true,
            exclude_tmpdir: true,
            exclude_slash_tmp: true,
        };

        // Test resolving from the worktree root
        let root_paths: Vec<PathBuf> = policy
            .get_writable_roots(&worktree)
            .into_iter()
            .map(|root| root.root)
            .collect();

        assert!(root_paths.contains(&worktree.canonicalize().expect("canonical worktree")));
        assert!(root_paths.contains(&worktree_git_dir.canonicalize().expect("canonical gitdir")));
        assert!(root_paths.contains(&common_git_dir.canonicalize().expect("canonical common git")));

        // Test resolving from a subdirectory of the worktree
        let subdirectory = worktree.join("src");
        std::fs::create_dir_all(&subdirectory).expect("mkdir subdirectory");

        let root_paths_sub: Vec<PathBuf> = policy
            .get_writable_roots(&subdirectory)
            .into_iter()
            .map(|root| root.root)
            .collect();

        assert!(root_paths_sub.contains(&worktree.canonicalize().expect("canonical worktree")));
        assert!(root_paths_sub.contains(&worktree_git_dir.canonicalize().expect("canonical gitdir")));
        assert!(root_paths_sub.contains(&common_git_dir.canonicalize().expect("canonical common git")));
    }

@Hmbown

Hmbown commented Jun 21, 2026

Copy link
Copy Markdown
Owner

Thanks @cyq1017 — this worktree sandbox fix is now carried locally in the v0.8.64 integration branch. I preserved your authorship with the canonical noreply identity from .github/AUTHOR_MAP, and the commit body credits PR #3356 plus @linletian for the worktree repro and diagnostics.

Local verification included the focused sandbox policy tests, and the release credit gate now passes over the integration branch. I am leaving this PR open until the integration branch is pushed/landed so the public repo state stays honest.

@Hmbown

Hmbown commented Jun 21, 2026

Copy link
Copy Markdown
Owner

Follow-up status for v0.8.64 integration:

The linked-worktree sandbox fix is still carried locally, and I also added the review follow-up for commands launched from subdirectories inside a linked worktree. The local patch now resolves the worktree .git pointer from ancestor directories while keeping the existing derived-metadata-root guard.

Verified locally with:

  • cargo test -p codewhale-tui --bin codewhale-tui --locked sandbox::policy
  • cargo fmt --all -- --check
  • cargo check -p codewhale-tui --bin codewhale-tui --locked
  • git diff --check

Thanks again @cyq1017 — this remains in the v0.8.64 train, and I am keeping the PR open until the integration branch is pushed/landed.

@github-actions

Copy link
Copy Markdown

Thanks @cyq1017 — your contribution landed in 529d4ad795a7 on main:

fix(tui): allow worktree git metadata writes in sandbox

Closing this PR now that the code is on main. Credit lives in the commit message and (where applicable) the CHANGELOG.md entry for the next release. Apologies for not closing this at the time of the merge — the auto-close workflow is new in v0.8.31.

If you want to land more work and would prefer your future PRs merge cleanly without a harvest step, the CONTRIBUTING.md doc has a short note on what makes a contribution mergeable as-is.

@github-actions github-actions Bot closed this Jun 22, 2026
pull Bot pushed a commit to soitun/CodeWhale that referenced this pull request Jun 22, 2026
Workspace-write sandbox policies now derive the linked worktree gitdir and shared commondir from a workspace .git pointer and add only those Git metadata roots to the writable set.

Fixes Hmbown#3355.

Harvested from PR Hmbown#3356 by @cyq1017; thanks @linletian for the worktree repro and diagnostics.
pull Bot pushed a commit to soitun/CodeWhale that referenced this pull request Jun 22, 2026
Follow-up to PR Hmbown#3356 by @cyq1017 after review noted that commands running below a linked worktree root could miss the worktree .git pointer. Resolve gitdir pointers from ancestor directories while preserving the existing commondir/worktrees trust check.

Verified with:

- cargo test -p codewhale-tui --bin codewhale-tui --locked sandbox::policy

- cargo fmt --all -- --check

- cargo check -p codewhale-tui --bin codewhale-tui --locked

- git diff --check
pull Bot pushed a commit to soitun/CodeWhale that referenced this pull request Jun 22, 2026
Require Git worktree metadata to point back to the workspace .git pointer before adding external gitdir or commondir paths to sandbox writable roots. This keeps the v0.8.64 worktree usability fix while avoiding a crafted pointer widening write access without reciprocal Git metadata.

Follow-up to the harvested Hmbown#3356 worktree sandbox fix.

Verification:

- cargo test -p codewhale-tui --bin codewhale-tui --locked workspace_write_

- cargo test -p codewhale-tui --bin codewhale-tui --locked sandbox::policy

- cargo fmt --all -- --check

- git diff --check
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 blocks Git write ops on worktree workspaces — allow worktree-linked paths without trust_mode

2 participants