Commit f17cfde
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
(`ValueError`, `IndexError`, `AssertionError`) all trace back to
this rejection.
Why `[gitdb]` failed and `[smmap]` passed
-----------------------------------------
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 `git/ext/gitdb`, and
gitdir `<repo>/.git/modules/gitdb`; smmap's `.git` gitfile,
worktree `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. The other five had Owner = `runneradmin`, whose Cygwin uid
(197108) was the value `geteuid()` returned in the test process.
The same Owner pattern held both with and without
`submodules: recursive` in `actions/checkout`. The single
`Administrators`-owned path was gitdb's worktree. All other paths
were `runneradmin`-owned, including the ones that Git for Windows's
recursive submodule clone had just produced when
`submodules: recursive` was set. The differentiator is not which
git binary clones the submodules, but 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
`entry.c::write_entry`, case `S_IFGITLINK` [1] [2]).
On the GHA `windows-latest` runner, jobs run as `runneradmin`,
which is the built-in local Administrator account (its Cygwin uid
`197108 = 196608 + 500` matches Cygwin's mapping [3] for
machine-local accounts: `0x30000` plus the SID suffix `500`, the
well-known suffix of that account's SID). That
account is exempt from UAC token filtering by default
(`FilterAdministratorToken=0` [4]), so its processes hold the full
administrative access token. `CreateProcessW` propagates the
parent's primary token unchanged through `actions/checkout`'s process
tree. Inside that tree, the outer `git clone`'s
`mkdir(path, 0777)` produces directories whose NTFS Owner is
`BUILTIN\Administrators` -- as observed on every workspace
directory the outer clone materialized, including the
`git/ext/gitdb` placeholder.
Subsequent submodule-update operations -- both Git for Windows if
`actions/checkout` does a recursive clone, and Cygwin git if it
happens later due to `init-tests-after-clone.sh` -- produce paths
that NTFS records as `runneradmin`-owned. Both flows go through a
process whose primary token's `TokenOwner` field has been
rewritten from `BUILTIN\Administrators` to the user SID by a
Cygwin or Cygwin-derived runtime at DLL initialization. The
rewrite is inherited by every descendant via `CreateProcessW`'s
token duplication [5], so every `mkdir` issued after that point
produces a directory owned by the user.
Cygwin git triggers the rewrite directly: `cygheap_user::init`
in `cygwin1.dll` calls
`NtSetInformationToken(hProcToken, TokenOwner, &user_sid, ...)`
at process startup [6]. Git for Windows triggers it indirectly.
`git submodule` is not a builtin -- only `submodule--helper` is
[7] -- so it falls through to `execv_dashed_external` and runs
`git-submodule.sh`, a shell script whose shebang is rewritten at
install time to `sh.exe` shipped with Git for Windows. That
`sh.exe` is an MSYS2 binary linked against `msys-2.0.dll`, a
Cygwin fork that performs the same `TokenOwner` rewrite. From
there, every `git.exe` the script spawns inherits the user-SID
`TokenOwner` and produces user-owned directories.
Cygwin's `lstat().st_uid` reports the actual NTFS Owner SID mapped
through Cygwin's SID-to-uid table. `is_path_owned_by_current_user`
reduces to `lstat(p).st_uid == geteuid()` on Cygwin (no
special-casing). `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".
Adding both submodule worktree paths to `safe.directory` is the
correct fix and is robust against shifts in what paths inherit
which Owner.
Why `actions/checkout`'s own `safe.directory` does not help
-----------------------------------------------------------
`actions/checkout`'s `set-safe-directory: true` default writes
the main repository path to `safe.directory` in a temporary
config it points its own spawned git child at by overriding
`HOME` for that child process. That `HOME` override applies
only to git invocations the action itself spawns; subsequent
steps' processes inherit the runner user's real `HOME` (e.g.,
`C:\Users\runneradmin` on the Windows runner) and read its
actual `~/.gitconfig`, which never received the entry. So no
git in a later step -- Git for Windows or Cygwin git -- sees
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 distinction between Cygwin git and Git for Windows is also why
the bug surfaces only on the Cygwin workflow rather than across
every Windows workflow. `compat/mingw.c` defines
`is_path_owned_by_current_sid` [8], 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` [9])
without that leniency. So the same `Administrators`-owned
`git/ext/gitdb` that Cygwin git rejects is silently accepted by
Git for Windows, and the main CI 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 should
show those tests passing instead.
The Owner-SID claim above can be discerned 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 Git for Windows does the recursive
submodule clone itself, the same pattern holds. `git/ext/gitdb`
is `Administrators`-owned; every other submodule path -- including
ones Git for Windows 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://cygwin.com/cygwin-ug-net/ntsec.html
[4]: https://learn.microsoft.com/en-us/windows/security/application-security/application-control/user-account-control/settings-and-configuration
[5]: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw
[6]: https://sourceware.org/cgit/newlib-cygwin/tree/winsup/cygwin/uinfo.cc#n82
[7]: https://github.com/git-for-windows/git/blob/v2.54.0.windows.1/git.c#L661
[8]: https://github.com/git-for-windows/git/blob/v2.54.0.windows.1/compat/mingw.c#L3931
[9]: 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 969353e commit f17cfde
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