Skip to content

Commit 982447d

Browse files
EliahKaganclaude
andcommitted
Cover submodule working trees in safe.directory on Cygwin
Add the gitdb and smmap submodule working tree paths to the `safe.directory` config in the Cygwin CI workflow. Without this, when GitPython opens a submodule as a `Repo` and runs `git cat-file --batch-check` against it, git rejects the repository for dubious ownership. The user-visible failure modes documented in commit 1 (`ValueError`, `IndexError`, `AssertionError`) all trace back to this rejection. Why `[gitdb]` fails and `[smmap]` incidentally passes ----------------------------------------------------- The trust check in `test/test_fixture_health.py` fails for `[gitdb]` but passes for `[smmap]` even though neither submodule's working tree is in the pre-fix `safe.directory` list. The asymmetry comes down to which Owner SID NTFS records on the gitdb working tree directory. `actions/checkout`'s `git clone` materializes the main repo's tree using the Windows git binary. The tree contains a submodule entry at `git/ext/gitdb` (mode 160000), which `git checkout` realizes as an empty directory at that path. On the GHA windows-latest runner with UAC effectively disabled, the runneradmin process holds the full admin token, and Windows stamps the new directory's NTFS Owner as `BUILTIN\Administrators` (Cygwin uid 544) rather than the runneradmin individual user (Cygwin uid 197108). The same is true of the surrounding workspace directories (`git`, `git/ext`, `.git`, etc.) that `actions/checkout` also creates. When `init-tests-after-clone.sh` later runs Cygwin's `git submodule update --init --recursive`: - Cygwin git clones gitdb's contents into the existing `git/ext/gitdb` placeholder. The directory's top-level Owner stays `BUILTIN\Administrators`. Files Cygwin creates inside it (the `.git` gitlink, gitdb's contents) are owned by runneradmin. - Cygwin git creates `.git/modules/gitdb` (gitdb's gitdir) from scratch; runneradmin-owned. - For smmap, Cygwin git creates the entire chain `git/ext/gitdb/gitdb/ext/smmap` (worktree) and `.git/modules/gitdb/modules/smmap` (gitdir) from scratch, since none of those paths existed pre-update; all runneradmin-owned. Cygwin's `lstat().st_uid` reports the actual NTFS Owner SID mapped to a POSIX uid. Git's `is_path_owned_by_current_user` reduces to `lstat(p).st_uid == geteuid()` on Cygwin (no special-casing), and `ensure_valid_ownership` fast-paths the repository when ALL of (gitfile, worktree, gitdir) pass that check; otherwise it falls through to matching `safe.directory` against the worktree's `real_pathdup`. For gitdb: gitfile=runneradmin, **worktree=Administrators**, gitdir=runneradmin. The fast-path AND fails on the worktree. `safe.directory` contains only `$(pwd)` and `$(pwd)/.git` pre-fix, neither of which exact-matches `git/ext/gitdb`. Repository rejected. For smmap: all three paths runneradmin-owned. Fast-path accepts. Cygwin's `chown` cannot rewrite the gitdb worktree's Owner SID from Administrators to runneradmin (it returns "Permission denied" even with admin token). Adding both submodule worktree paths to `safe.directory` is the correct fix, and is robust against future changes in which paths happen to inherit which Owner. Why `actions/checkout`'s own `safe.directory` doesn't help --------------------------------------------------------- `actions/checkout`'s `set-safe-directory: true` default adds the main repository path to `safe.directory`, but on Cygwin that addition lands in the Windows-side global git config (under runneradmin's Windows HOME), not the Cygwin-side one (under the Cygwin user's HOME). Cygwin git reads only its own global config, so `actions/checkout`'s entry is invisible to it. The cygwin-test workflow therefore explicitly populates Cygwin git's `safe.directory` itself; this commit extends that to cover the gitdb and smmap working trees. Verification ------------ The `reproduce-safe-dir` matrix on the previous commit produces failures for all three affected tests; this commit's CI run is expected to show those tests passing instead. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d0dc297 commit 982447d

1 file changed

Lines changed: 2 additions & 0 deletions

File tree

.github/workflows/cygwin-test.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ jobs:
5858
run: |
5959
git config --global --add safe.directory "$(pwd)"
6060
git config --global --add safe.directory "$(pwd)/.git"
61+
git config --global --add safe.directory "$(pwd)/git/ext/gitdb"
62+
git config --global --add safe.directory "$(pwd)/git/ext/gitdb/gitdb/ext/smmap"
6163
git config --global core.autocrlf false
6264
6365
- &prepare-repo

0 commit comments

Comments
 (0)