fix: identify bare repo via GIT_DIR for safe.bareRepository=explicit#82
Open
MattKotsenas wants to merge 1 commit into
Open
fix: identify bare repo via GIT_DIR for safe.bareRepository=explicit#82MattKotsenas wants to merge 1 commit into
MattKotsenas wants to merge 1 commit into
Conversation
… scoping
Grove had four direct Command::new("git") callsites with three different
scoping patterns: bare-repo (via current_dir), worktree (via current_dir),
and unbound (no scoping). The bare-repo pattern broke under
safe.bareRepository=explicit because it relied on git's CWD-based bare-repo
discovery, which that setting disables.
Bug 1: safe.bareRepository=explicit breaks every grove command that
touches the bare repo
The setting is injected on every spawned shell by GitHub Copilot CLI 1.0+
(via GIT_CONFIG_COUNT/_KEY_0/_VALUE_0) and by hardened corporate
environments. Verified the source by spawning a fresh copilot.exe -p with
a cleaned env and observing the markers in its child shell with a fresh
agent session id, ruling out env inheritance.
Affected: grove add, grove list, grove sync, grove prune,
grove remove, plus the post-clone config call in clone_bare_repository.
All fail with:
fatal: cannot use bare repository '...' (safe.bareRepository is 'explicit')
Bug 2 (independent): git-for-Windows rejects \\?\ extended-length paths
in GIT_DIR and --git-dir
Switching to GIT_DIR exposed a latent bug. discover_bare_clone uses
std::fs::canonicalize, which on Windows always returns paths prefixed
with \\?\. Git accepts those paths as a process working directory because
Win32 round-trips them transparently, but rejects them when supplied via
GIT_DIR or --git-dir because git's portable path parser does not
recognize the prefix:
fatal: not a git repository: '\\?\D:\repo\bare.git'
This is independent of path length and independent of core.longpaths.
Verified with an 87-char path and -c core.longpaths=true forced.
This is essentially a git-for-Windows wart: \\?\D:\foo is a valid Windows
path that every Win32 path API accepts, but git's path parser doesn't
recognize the prefix. The Rust-side discussion at
rust-lang/rust#42869 is downstream of this -
Rust returns the canonical Windows form from canonicalize, and tools
that don't accept it (cl.exe, git, etc.) force every Rust caller to
post-process. Stripping \\?\ before handing the path to git is the
ecosystem-standard workaround (see also the dunce crate).
Refactor: single funnel for all git invocations
Replace all four direct Command::new("git") callsites with a single
�uild_git_command(target, args) -> Command helper. The GitTarget enum
encodes the three legitimate scoping modes:
- BareRepo(&Path) - existing bare repo. Identified via GIT_DIR
(satisfies Bug 1) with the path stripped of \\?\ (satisfies Bug 2).
- WorkTree(&Path) - inside a worktree. Uses current_dir; git's own
discovery finds the repository from there.
- Unbound - no pre-existing repository (e.g. git clone,
git --version).
This gives uniform scoping policy, a single place to test the invariants
without spinning up real repos, and makes future regressions visible: a
new direct Command::new("git") in a future PR stands out against the
established pattern of going through the helper.
Tests
Six unit tests pin the invariants of each GitTarget mode:
- �uild_git_command_bare_repo_sets_git_dir_env_var (Bug 1)
- �uild_git_command_bare_repo_does_not_set_current_dir (Bug 1
regression guard)
- �uild_git_command_bare_repo_normalizes_windows_extended_prefix
(Bug 2)
- �uild_git_command_work_tree_sets_current_dir_and_no_git_dir
- �uild_git_command_unbound_sets_neither_current_dir_nor_git_dir
- �uild_git_command_forwards_args_in_order
Total: 138 passing (132 baseline + 6 new). cargo fmt --check and
cargo clippy -- -D warnings both clean.
End-to-end verified by running grove list, grove add, and grove remove
against a real bare clone with safe.bareRepository=explicit injected and
the bare repo at a path that canonicalizes to \\?\D:\.... All succeed
with the patch; all fail without it.
MattKotsenas
added a commit
to MattKotsenas/grove
that referenced
this pull request
Jun 13, 2026
After 'git clone --bare', 'grove init' now also runs 'git fetch origin' and 'git remote set-head origin --auto' so refs/remotes/origin/* and origin/HEAD are populated immediately. This makes 'grove sync', '--track', and 'get_default_branch' work without requiring users to run an extra manual fetch. 'grove add <branch>' now sets upstream tracking to origin/<branch> when a matching remote-tracking ref exists and the branch has no upstream yet. Mirrors the DWIM behavior of 'git checkout' in a non-bare clone. Skipped when the user passed an explicit --track (handled by existing code) or when no matching origin/<branch> exists. Implements both workflow steps described in https://blog.safia.rocks/2025/09/03/git-worktrees/ that grove was previously skipping. The two post-clone steps run against the freshly-cloned bare repository, so they go through build_git_command(BareRepo(..)) like the other bare-repo invocations rather than Command::new("git").current_dir(target_dir). The current_dir pattern silently breaks under safe.bareRepository=explicit (the setting agent shells like GitHub Copilot CLI 1.0+ apply via GIT_CONFIG_KEY_0/VALUE_0), because that setting disables git's CWD-based bare-repo discovery and requires GIT_DIR / --git-dir for identification. Each callsite is wrapped in a named build_*_command function so a unit test can assert the constructed Command sets GIT_DIR and leaves current_dir unset. The tests run without a real bare repo or a CI runner with safe.bareRepository=explicit pre-set. Note for review: this branch is stacked on fix/safe-bare-repository (PR captainsafia#82). Merge that PR first; this PR depends on build_git_command and GitTarget::BareRepo which it introduces.
MattKotsenas
added a commit
to MattKotsenas/grove
that referenced
this pull request
Jun 13, 2026
After 'git clone --bare', 'grove init' now also runs 'git fetch origin' and 'git remote set-head origin --auto' so refs/remotes/origin/* and origin/HEAD are populated immediately. This makes 'grove sync', '--track', and 'get_default_branch' work without requiring users to run an extra manual fetch. 'grove add <branch>' now sets upstream tracking to origin/<branch> when a matching remote-tracking ref exists and the branch has no upstream yet. Mirrors the DWIM behavior of 'git checkout' in a non-bare clone. Skipped when the user passed an explicit --track (handled by existing code) or when no matching origin/<branch> exists. Implements both workflow steps described in https://blog.safia.rocks/2025/09/03/git-worktrees/ that grove was previously skipping. The two post-clone steps run against the freshly-cloned bare repository, so they go through build_git_command(BareRepo(..)) like the other bare-repo invocations rather than Command::new("git").current_dir(target_dir). The current_dir pattern silently breaks under safe.bareRepository=explicit (the setting agent shells like GitHub Copilot CLI 1.0+ apply via GIT_CONFIG_KEY_0/VALUE_0), because that setting disables git's CWD-based bare-repo discovery and requires GIT_DIR / --git-dir for identification. Each callsite is wrapped in a named build_*_command function so a unit test can assert the constructed Command sets GIT_DIR and leaves current_dir unset. The tests run without a real bare repo or a CI runner with safe.bareRepository=explicit pre-set. Note for review: this branch is stacked on fix/safe-bare-repository (PR captainsafia#82). Merge that PR first; this PR depends on build_git_command and GitTarget::BareRepo which it introduces.
MattKotsenas
added a commit
to MattKotsenas/grove
that referenced
this pull request
Jun 13, 2026
After 'git clone --bare', 'grove init' now also runs 'git fetch origin' and 'git remote set-head origin --auto' so refs/remotes/origin/* and origin/HEAD are populated immediately. This makes 'grove sync', '--track', and 'get_default_branch' work without requiring users to run an extra manual fetch. 'grove add <branch>' now sets upstream tracking to origin/<branch> when a matching remote-tracking ref exists and the branch has no upstream yet. Mirrors the DWIM behavior of 'git checkout' in a non-bare clone. Skipped when the user passed an explicit --track (handled by existing code) or when no matching origin/<branch> exists. Implements both workflow steps described in https://blog.safia.rocks/2025/09/03/git-worktrees/ that grove was previously skipping. The two post-clone steps run against the freshly-cloned bare repository, so they go through build_git_command(BareRepo(..)) like the other bare-repo invocations rather than Command::new("git").current_dir(target_dir). The current_dir pattern silently breaks under safe.bareRepository=explicit (the setting agent shells like GitHub Copilot CLI 1.0+ apply via GIT_CONFIG_KEY_0/VALUE_0), because that setting disables git's CWD-based bare-repo discovery and requires GIT_DIR / --git-dir for identification. Each callsite is wrapped in a named build_*_command function so a unit test can assert the constructed Command sets GIT_DIR and leaves current_dir unset. The tests run without a real bare repo or a CI runner with safe.bareRepository=explicit pre-set. Note for review: this branch is stacked on fix/safe-bare-repository (PR captainsafia#82). Merge that PR first; this PR depends on build_git_command and GitTarget::BareRepo which it introduces.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR addresses two distinct bugs that happen to co-occur on Windows under agent shells.
Bug 1:
safe.bareRepository=explicitbreaks every grove command that touches the bare repoGrove invokes git by setting
current_dirto the bare repository path, relying on git's CWD-based repo discovery. That discovery is disabled whensafe.bareRepository=explicitis in effect (git-config(1)):This setting is injected on every spawned shell by GitHub Copilot CLI. It sets
GIT_CONFIG_COUNT=1+GIT_CONFIG_KEY_0=safe.bareRepository+GIT_CONFIG_VALUE_0=explicitso the agent can't be tricked into operating on a stray.gitdirectory it didn't intend to touch. Affected commands:grove add,grove list,grove sync,grove prune,grove remove(anything that routes throughgit_raw), plus the post-clone config call inclone_bare_repository.Reproduction
Bug 2: git-for-Windows rejects
\\?\extended-length paths inGIT_DIRand--git-dirSwitching to
GIT_DIRexposed a separate latent bug.discover_bare_clonereturns the bare repo path via Rust'sstd::fs::canonicalize, which on Windows always emits paths with the\\?\extended-length prefix (documented stdlib behavior). Git accepts\\?\paths as a process working directory because Win32 round-trips them transparently and git never parses the CWD as a string. But supplied viaGIT_DIRor--git-dir, git's portable path parser rejects the prefix as malformed input:This is independent of path length and independent of
core.longpaths. Verified with an 87-character path and-c core.longpaths=trueexplicitly forced.Root cause
This is fundamentally a wart in git-for-Windows:
\\?\D:\foois a valid Windows path that every Win32 path API accepts, but git's path parser doesn't recognize the prefix. The corresponding Rust-side discussion (rust-lang/rust#42869, open since 2017, 60+ comments) is downstream of this; Rust returns the canonical Windows form fromcanonicalize, and tools that don't accept it (cl.exe,git, etc.) force every Rust caller to post-process. The community workaround is theduncecrate, which strips\\?\when safe.Why
GIT_DIRand not-c safe.bareRepository=allThe override approach would mechanically work (later
-cwins overGIT_CONFIG_*-supplied config), but it actively subverts the hardening signal the agent set deliberately.GIT_DIRcooperates with the policy, which is whatexplicitis asking for. It also future-proofs against similar policies (safe.directory, etc.) without each requiring its own override.Fix
Reuse the existing
normalize_path_for_git(added in commit5fb0067for the same root issue on worktree-path args). Stripping\\?\loses extended-length-path support for >260-char paths, but that's the same tradeoff already accepted for worktree-path args, so this PR doesn't introduce a new regression.Additionally, rather than scatter
GIT_DIR-vs-current_dirdecisions across callsites, this PR routes everyCommand::new("git")through one helper:All four pre-existing callsites are migrated:
git_raw(the main bare-repo workhorse)BareRepoclone_bare_repositoryinitial cloneUnboundclone_bare_repositorypost-clone configBareRepocomplete_worktree_infostatus checkWorkTreeThis provides a uniform policy that prevents drift across callsites and simplifies test assertions about the command.