Skip to content

Commit 5501d11

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 each path, and which paths are required to pass git's ownership check. Across runner snapshots taken empirically (see "Verification" below), exactly one path among (gitfile, worktree, gitdir) for either submodule has NTFS Owner = `BUILTIN\Administrators` (Cygwin uid 544): gitdb's worktree directory `git/ext/gitdb`. Every other relevant path -- gitdb's `.git` gitlink file, gitdb's gitdir at `.git/modules/gitdb`, smmap's worktree at `git/ext/gitdb/gitdb/ext/smmap`, smmap's `.git` gitlink, smmap's gitdir at `.git/modules/gitdb/modules/smmap` -- has Owner = `runneradmin` (Cygwin uid 197108). Cygwin's `geteuid()` is 197108. This pattern reproduces whether `actions/checkout` is configured with `submodules: recursive` (so Windows git fetches the submodules itself) or not (so `init-tests-after-clone.sh` later runs Cygwin's `git submodule update --init --recursive`). The single Admin-owned path is the gitdb worktree directory; every other relevant path ends up runneradmin-owned regardless of which git binary populated it. The differentiator is not which binary clones the submodules, but the fact that `git/ext/gitdb` is created by the main repo's tree checkout: when `git checkout` materializes a tree entry of mode 160000 (a gitlink), it calls `mkdir(path, 0777)` to create the empty submodule directory (see git's `entry.c::checkout_entry`, case `S_IFGITLINK`). On the GHA windows-latest runner with UAC effectively disabled, the runneradmin process holds the full admin token, and the `mkdir` performed by the main `git clone`'s checkout phase produces a directory whose NTFS Owner is `BUILTIN\Administrators`. The same is true of every other directory the main `git clone` materializes (`git`, `git/ext`, `.git`, etc.), but those don't matter for the trust check on the submodules. Subsequent submodule-update operations -- regardless of which git binary runs them -- produce paths that NTFS records as runneradmin-owned, leaving the gitdb worktree as the only path inside the submodule trees with the foreign Owner SID. 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 shifts 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. The Owner-SID claim above is verifiable from two complementary CI snapshots taken on temporary diagnostic branches (now deleted but the runs are preserved): - Without `submodules: recursive` on `actions/checkout` (the chain's actual configuration): `git/ext/gitdb` exists as an empty directory immediately after `actions/checkout`, with Owner = `BUILTIN\Administrators`. Every other submodule path is created later by Cygwin's `git submodule update --init --recursive` and has Owner = runneradmin. - With `submodules: recursive` on `actions/checkout` (the pre-drop scenario, where Windows git does the recursive submodule clone itself): the same pattern holds. `git/ext/gitdb` is Admin-owned; every other submodule path -- including those Windows git's recursive clone created -- is runneradmin-owned. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 882e0e0 commit 5501d11

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)