Skip to content

Re-sync libgit2 overlay admin-owner patch with merged upstream (bump to 1.9.4)#2041

Merged
tyrielv merged 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/resync-libgit2-admin-owner-overlay
Jul 6, 2026
Merged

Re-sync libgit2 overlay admin-owner patch with merged upstream (bump to 1.9.4)#2041
tyrielv merged 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/resync-libgit2-admin-owner-overlay

Conversation

@tyrielv

@tyrielv tyrielv commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Re-syncs the VFSForGit libgit2 vcpkg overlay patch non-elevated-admin-owner.diff with the version that was actually merged to libgit2 main, replacing the buggy version that was copied verbatim from libgit2/libgit2#7200. Also bumps the pinned libgit2 version 1.9.31.9.4 and refreshes the overlay README provenance/release notes.

Why

The overlay carried the raw PR #7200 diff, which had four bugs that libgit2 maintainers fixed before/at merge:

  1. Handle leak — the linked_token HANDLE from GetTokenInformation(TokenLinkedToken) was never CloseHandle'd. Matters for the long-lived GVFS mount service running as SYSTEM.
  2. static HANDLE linked_token in git_fs_path_owner_is — not thread-safe and retained a stale handle across calls. Now a per-call local initialized to NULL by the caller.
  3. Failure-path pointer bug — on TokenLinkedToken failure the code did linked_token = NULL (nulling the local pointer parameter) instead of *linked_token = NULL (+ CloseHandle).
  4. current_user_sid() only called in the CURRENT_USER branch — but the admin-membership branch also reads linked_token, so a caller passing only USER_IS_ADMINISTRATOR would read an uninitialized handle. It is now called unconditionally before either branch.

What merged upstream

PR #7200 was manually merged to main on 2026-06-07. Relevant commits (all touch only src/util/fs_path.c):

  • cc477ee — "Support non-elevated admin user check on Windows"
  • f9f36a6 — "Extend Win32 safe.directory admin-owner check to also cover non-elevated processes" (the four bug fixes)
  • 44c05e5 — "fs: removed unused variable"
  • e805a16 — merge commit

The regenerated diff reproduces the net effect of cc477ee + f9f36a6 + 44c05e5 on top of the pinned libgit2 source.

Version bump: 1.9.3 → 1.9.4

  • v1.9.4 (2026-05-22) is the latest v1.9.x release; the official vcpkg libgit2 port already pins it.
  • src/util/fs_path.c is byte-identical between v1.9.3 and v1.9.4, so the patched region is unaffected and the diff applies cleanly to both.
  • Updated vcpkg.json version-semver and the portfile.cmake SHA512.

Release status

The patch is not yet in any libgit2 release (latest is v1.9.4, which predates the merge). It is expected to ship upstream in v2.0 (no ETA); v1.9.x is stated to be the last of the 1.x line. The overlay patch remains necessary until the pinned port moves to a libgit2 that includes it.

Verification

src\scripts\Build.bat Debug rebuilds libgit2 for both x64-windows-static-aot and x64-windows-dynamic triplets:

-- Applying patch dependencies.diff
-- Applying patch non-elevated-admin-owner.diff
...
All requested installations completed successfully

No hunk failures; libgit2 compiles and the full solution builds to an installer. The patched fs_path.c has HANDLE linked_token = NULL; (no static), a CloseHandle in the done: cleanup, CloseHandle(*linked_token); *linked_token = NULL; in the failure path, and current_user_sid() called unconditionally.

The `non-elevated-admin-owner.diff` overlay patch was copied verbatim from
libgit2 PR #7200. That PR version had four bugs that maintainers fixed
before merging to libgit2 `main`:

- `linked_token` HANDLE from TokenLinkedToken was never CloseHandle'd,
  leaking a handle in the long-lived GVFS mount service (SYSTEM).
- `current_user_sid` used `static HANDLE linked_token` — not thread-safe
  and retained a stale handle across calls; replaced with a per-call
  local initialized to NULL by the caller.
- The TokenLinkedToken failure path nulled the local pointer parameter
  (`linked_token = NULL`) instead of `*linked_token = NULL` (+CloseHandle).
- `current_user_sid()` was only invoked in the CURRENT_USER branch, but
  the admin-membership branch also reads `linked_token`; a caller passing
  only USER_IS_ADMINISTRATOR would read an uninitialized handle. It is now
  called unconditionally before either branch.

Regenerate the patch so that, applied to the pinned libgit2 source, it
produces the same corrected `src/util/fs_path.c` as upstream `main`
(commits cc477ee + f9f36a6 + 44c05e5, merge e805a16, merged 2026-06-07).
The patch is not yet in any libgit2 release, so the overlay remains
necessary.

Also bump the pinned libgit2 version 1.9.3 -> 1.9.4 (matching the official
vcpkg port). fs_path.c is byte-identical between 1.9.3 and 1.9.4, so the
patched region is unaffected and the diff applies cleanly to both.

Verified: Build.bat Debug rebuilds libgit2 for both x64-windows-static-aot
and x64-windows-dynamic triplets, applying non-elevated-admin-owner.diff
with no hunk failures, and the full solution builds to an installer.

Complements microsoft#2039 (the GVFS-side AP clone-ownership fix).
Work item: AB#62918022.

Assisted-by: Claude Opus 4.8
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
@tyrielv tyrielv marked this pull request as ready for review July 6, 2026 22:43
@tyrielv tyrielv enabled auto-merge July 6, 2026 22:57
@tyrielv tyrielv merged commit 2fa5b26 into microsoft:master Jul 6, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants