Commit 8d69fc9
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` failed for `[gitdb]`
but passed for `[smmap]`, even though neither submodule's working
tree was in the workflow's `safe.directory` list before this commit.
The asymmetry comes down to which Owner SID NTFS records on each
path, and which paths git's ownership check requires to be owned.
There are six paths git's check might consider for the two
submodules: gitdb's `.git` gitfile, worktree directory
`git/ext/gitdb`, and gitdir `<repo>/.git/modules/gitdb`; smmap's
`.git` gitfile, worktree directory
`git/ext/gitdb/gitdb/ext/smmap`, and gitdir
`<repo>/.git/modules/gitdb/modules/smmap`. Of these, only one had
NTFS Owner = `BUILTIN\Administrators` (Cygwin uid 544): gitdb's
worktree directory. The other five had Owner = runneradmin (Cygwin
uid 197108). Cygwin's `geteuid()` was 197108.
The same Owner pattern held with and without `submodules: recursive`
in `actions/checkout`. The single Admin-owned path was gitdb's
worktree directory; all other paths -- including the ones that
Windows git's recursive submodule clone had just produced when
`submodules: recursive` was set -- were runneradmin-owned. The
differentiator is not which git binary clones the submodules, but
the fact that `git/ext/gitdb` is created by the outer `git clone`'s
checkout phase. When `git checkout` materializes a tree entry of
mode 160000, it calls `mkdir(path, 0777)` to create an empty
submodule directory (see git's `entry.c::checkout_entry`, case
`S_IFGITLINK` [1] [2]).
On the GHA `windows-latest` runner image, the system registry has
`ConsentPromptBehaviorAdmin` set to 0 ("Elevate without prompting")
and `EnableLUA` left at its default of 1, per
`actions/runner-images`'s `Configure-BaseImage.ps1`. UAC and Admin
Approval Mode remain enabled, but elevation prompts are suppressed.
The `actions/checkout` action runs under the `runneradmin` account,
a member of `Administrators`, and `CreateProcessW` propagates the
parent's primary token unchanged through the actions/checkout
process tree. Inside that tree, the outer `git clone`'s
`mkdir(path, 0777)` produces directories whose NTFS Owner is
`BUILTIN\Administrators` -- which is what we observed on the
workspace itself, on `.git`, on `git`, on `git/ext`, and on the
`git/ext/gitdb` placeholder.
Subsequent submodule-update operations -- both Windows git when
`actions/checkout` drives the recursive clone, and Cygwin git when
`init-tests-after-clone.sh` runs `git submodule update --init
--recursive` later -- produce paths that NTFS records as
runneradmin-owned. Why operations following the outer checkout
produce a different Owner from the outer checkout itself, even when
both are performed by the same git binary running under the same
runneradmin account, was not pinned down in source. The empirical
pattern is consistent across all observed scenarios (see
"Verification" below) and the fix does not depend on the precise
cause.
Cygwin's `lstat().st_uid` reports the actual NTFS Owner SID mapped
through Cygwin's SID-to-uid table. Git's
`is_path_owned_by_current_user` reduces to
`lstat(p).st_uid == geteuid()` on Cygwin (no special-casing). Git's
`ensure_valid_ownership` returns 1 (accepting the repository)
without consulting `safe.directory` when ALL of (gitfile, worktree,
gitdir) pass that owned-by-current-user check. Otherwise it falls
through to comparing the worktree's `real_pathdup` against each
configured `safe.directory` entry.
For gitdb the three Owners were runneradmin (gitfile),
**Administrators (worktree)**, and runneradmin (gitdir), so the
all-paths-owned check failed on the worktree. The workflow's
`safe.directory` before this commit contained only `$(pwd)` and
`$(pwd)/.git`, neither of which exact-matches `git/ext/gitdb`, so
the `safe.directory` comparison also failed, and
`ensure_valid_ownership` returned 0 -- git rejected the repository.
For smmap the three Owners were all runneradmin, so the
all-paths-owned check passed.
Cygwin's `chown` cannot rewrite the gitdb worktree's Owner SID from
Administrators to runneradmin: it returns `Permission denied`,
verified in the v3/v4 diag runs cited below. Adding both submodule
worktree paths to `safe.directory` is the correct fix and is
robust against shifts in which paths inherit which Owner.
Why `actions/checkout`'s own `safe.directory` does not 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.
The Cygwin-vs-Windows-git distinction is also why the bug surfaces
only on the Cygwin workflow rather than across every Windows
workflow. Git's `compat/mingw.c` defines
`is_path_owned_by_current_sid` [3], which accepts
`BUILTIN\Administrators`-owned paths when the current user is a
member of Administrators. Cygwin git compiles against the POSIX
path (`is_path_owned_by_current_uid` in `git-compat-util.h` [4])
without that leniency. So the same Admin-owned `git/ext/gitdb`
that Cygwin git rejects is silently accepted by Windows git, and
the `Python package` workflow's `windows-latest` jobs never trip
the trust check.
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 three diagnostic CI
runs:
- https://github.com/EliahKagan/GitPython/actions/runs/25542627150/job/74971640559
Ownership and `chown` experiment after `actions/checkout` with
default settings (no recursive submodules) and Cygwin's
`git submodule update --init --recursive`. `git/ext/gitdb` Owner
= `BUILTIN\Administrators`; every other relevant path Owner =
runneradmin; `chown` of the worktree returns `Permission denied`.
- https://github.com/EliahKagan/GitPython/actions/runs/25543128283/job/74973183684
Placeholder timing check: `git/ext/gitdb` exists as an empty
directory with Owner = `BUILTIN\Administrators` immediately
after `actions/checkout`, before any submodule update has run.
Confirms the Owner is set at the outer `git clone`'s checkout
phase, not by the submodule update.
- https://github.com/EliahKagan/GitPython/actions/runs/25557770540/job/75021052090
Recursive-clone replication: with `submodules: recursive` in
`actions/checkout` so Windows git does the recursive submodule
clone itself, the same pattern holds. `git/ext/gitdb` is
Admin-owned; every other submodule path -- including ones
Windows git has just created -- is runneradmin-owned.
[1]: https://github.com/git/git/blob/v2.51.0/entry.c#L397
[2]: https://github.com/git-for-windows/git/blob/v2.54.0.windows.1/entry.c#L397
[3]: https://github.com/git-for-windows/git/blob/v2.54.0.windows.1/compat/mingw.c#L3931
[4]: https://github.com/git/git/blob/v2.51.0/git-compat-util.h#L346
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>1 parent 882e0e0 commit 8d69fc9
1 file changed
Lines changed: 2 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
58 | 58 | | |
59 | 59 | | |
60 | 60 | | |
| 61 | + | |
| 62 | + | |
61 | 63 | | |
62 | 64 | | |
63 | 65 | | |
| |||
0 commit comments