From 3cbc75f978159d7c4320c5e85709424368a51491 Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Thu, 2 Jul 2026 20:29:26 -0700 Subject: [PATCH] Fix clone ownership under Windows Administrator Protection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under Windows "Administrator protection" (AP), an elevated process runs as a hidden, profile-separated shadow admin account (MACHINE\admin_) whose SID is a local account (S-1-5-21-...) that differs from the real (non-elevated) user's SID. When `gvfs clone` runs elevated, the enlistment's `src` and `.git` directories are created owned by the Administrators group. EnsureDirectoryIsOwnedByCurrentUser then reassigned ownership to the current user to satisfy git's dubious-ownership check. Under AP the "current user" is the shadow admin, so ownership was set to admin_ — which the real user does not match. The real (non-elevated) user then hit `fatal: detected dubious ownership`, because git's Administrators-group membership grace does not cover another specific user's SID, and the shadow admin cannot SetOwner to the real user's SID either. Detect the AP shadow admin from the running process's own token — its effective elevation identity — and, when present, leave the directory owner as the Administrators group instead of reassigning it to the shadow admin. An Administrators-owned directory is accepted by modern git and the libgit2 non-elevated-admin-owner overlay for any Administrators member — the real user, the shadow admin, and SYSTEM (for automount) alike. Detection inspects the token rather than the TypeOfAdminApprovalMode policy value so it stays correct across an AP policy change that has not yet been rebooted (where the policy value is pending but elevation still yields a shadow admin). A shadow admin is identified as a local account SID (S-1-5-21-...) whose ProfileList profile directory is ADMIN_-prefixed; the real interactive user (a domain or Entra ID account) is excluded. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella --- .../WindowsFileSystem.cs | 23 ++++++++++ GVFS/GVFS.Platform.Windows/WindowsPlatform.cs | 44 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/GVFS/GVFS.Platform.Windows/WindowsFileSystem.cs b/GVFS/GVFS.Platform.Windows/WindowsFileSystem.cs index 7ba732a13..0fe4d781f 100644 --- a/GVFS/GVFS.Platform.Windows/WindowsFileSystem.cs +++ b/GVFS/GVFS.Platform.Windows/WindowsFileSystem.cs @@ -327,6 +327,10 @@ public bool IsFileSystemSupported(string path, out string error) /// Also, even if the fix were in place, automount would still fail because it runs under SYSTEM account. /// /// This method ensures that the directory is owned by the current user (which is verified to work for SYSTEM account for automount). + /// + /// Exception: under Windows "Administrator protection" (AP), an elevated process runs as a profile-separated shadow admin + /// account (MACHINE\admin_<user>) whose SID differs from the real (non-elevated) user's. In that case the directory owner is + /// left as the Administrators group rather than being reassigned to the shadow admin (see the AP note inline). /// public void EnsureDirectoryIsOwnedByCurrentUser(string directoryPath) { @@ -339,6 +343,25 @@ public void EnsureDirectoryIsOwnedByCurrentUser(string directoryPath) SecurityIdentifier administratorsSid = new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null); if (directoryOwner == administratorsSid) { + // Under Windows "Administrator protection" (AP), an elevated process runs as a profile-separated shadow + // admin account (MACHINE\admin_) whose SID differs from the real (non-elevated) user's. Reassigning + // ownership to the current user here would set the owner to that shadow admin, causing the real user to hit + // "fatal: detected dubious ownership" — git's Administrators-membership grace does not cover another specific + // user's SID. The shadow admin also cannot SetOwner to the real user's SID. Leaving the owner as the + // Administrators group is accepted by modern git and the libgit2 non-elevated-admin-owner overlay for any + // Administrators member (real user, shadow admin, and SYSTEM for automount alike), so skip the reassignment. + // + // Note: a libgit2 consumer that lacks the non-elevated-admin-owner patch (i.e. stock libgit2 without the + // Administrators-membership fix) will not be able to open an Administrators-owned repo. Addressing that would + // require setting git's safe.directory for the real user, but safe.directory is only honored from global/system + // config (never repo-local), and an elevated clone runs as the shadow admin — so covering the real user would + // mean mutating the real user's global gitconfig or machine-wide system config from the shadow admin. After + // consideration, GVFS is not the right place to do that; unpatched third-party libgit2 consumers are out of scope. + if (WindowsPlatform.IsCurrentUserAdminProtectionShadowAccount()) + { + return; + } + WindowsIdentity currentUser = WindowsIdentity.GetCurrent(); directorySecurity.SetOwner(currentUser.User); directoryInfo.SetAccessControl(directorySecurity); diff --git a/GVFS/GVFS.Platform.Windows/WindowsPlatform.cs b/GVFS/GVFS.Platform.Windows/WindowsPlatform.cs index 37949e6d6..9d26a71ce 100644 --- a/GVFS/GVFS.Platform.Windows/WindowsPlatform.cs +++ b/GVFS/GVFS.Platform.Windows/WindowsPlatform.cs @@ -24,6 +24,10 @@ public partial class WindowsPlatform : GVFSPlatform private const string BuildLabRegistryValue = "BuildLab"; private const string BuildLabExRegistryValue = "BuildLabEx"; + private const string ProfileListRegistryKey = "SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\ProfileList"; + private const string ProfileImagePathRegistryValue = "ProfileImagePath"; + private const string AdminProtectionProfilePrefix = "ADMIN_"; + public WindowsPlatform() : base(underConstruction: new UnderConstructionFlags()) { } @@ -76,6 +80,46 @@ public static object GetValueFromRegistry(RegistryHive registryHive, string key, return value; } + /// + /// Detects whether the current process token belongs to a Windows "Administrator protection" (AP) + /// shadow admin account. + /// + /// Under AP, elevating produces a token whose user is a hidden, system-managed local account + /// (MACHINE\admin_<user>, SID S-1-5-21-...) with its own user profile named ADMIN_<user>. That + /// account's SID differs from the real (interactive) user's SID, so treating it as "the current user" + /// for file-ownership purposes is wrong. + /// + /// This inspects the running process's own token (its effective elevation identity) rather than the + /// TypeOfAdminApprovalMode policy value, so it is correct even across an AP policy change that has not + /// yet been rebooted (where the policy value is pending but elevation still yields a shadow admin). + /// + public static bool IsCurrentUserAdminProtectionShadowAccount() + { + using (WindowsIdentity currentUser = WindowsIdentity.GetCurrent()) + { + SecurityIdentifier userSid = currentUser.User; + + // AP shadow accounts are local machine accounts (S-1-5-21-...); the real interactive user is a + // domain or Entra ID account (e.g. S-1-12-1-...), which is not an "account SID" in this sense. + if (userSid == null || !userSid.IsAccountSid()) + { + return false; + } + + string profilePath = GetStringFromRegistry( + $"{ProfileListRegistryKey}\\{userSid.Value}", + ProfileImagePathRegistryValue); + if (string.IsNullOrEmpty(profilePath)) + { + return false; + } + + string profileFolderName = Path.GetFileName( + profilePath.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)); + return profileFolderName.StartsWith(AdminProtectionProfilePrefix, StringComparison.OrdinalIgnoreCase); + } + } + public static bool TrySetDWordInRegistry(RegistryHive registryHive, string key, string valueName, uint value) { RegistryKey localKey = RegistryKey.OpenBaseKey(registryHive, RegistryView.Registry64);