From 4e23d28ab8104c448b20c980085d3e7e2b4e4971 Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Mon, 13 Jan 2025 14:22:30 -0800 Subject: [PATCH 1/2] Fix ownership when cloning elevated On Windows, if the current user is elevated then any directories created will be owned by the Administrators group. This can cause problems later on in non-elevated contexts due to git's "dubious ownership" check. Git for Windows does not currently consider a non-elevated admin to be the owner of a directory owned by the Administrators group, though a fix is in progress in the microsoft fork of git. Libgit2(sharp) also does not have this fix. Also, the GVFS service which automounts repositories runs under the SYSTEM account, which would still fail the check even if that fix were in place. This commit changes the ownership of the .git directory and the working directory to the current user when cloning. --- .../FileSystem/IPlatformFileSystem.cs | 1 + GVFS/GVFS.Common/GVFSEnlistment.cs | 2 +- .../WindowsFileSystem.cs | 28 +++++++++++++++++++ .../Mock/FileSystem/MockPlatformFileSystem.cs | 5 ++++ GVFS/GVFS/CommandLine/CloneVerb.cs | 11 ++++++++ 5 files changed, 46 insertions(+), 1 deletion(-) diff --git a/GVFS/GVFS.Common/FileSystem/IPlatformFileSystem.cs b/GVFS/GVFS.Common/FileSystem/IPlatformFileSystem.cs index ce480b512b..b9ac2c2e8d 100644 --- a/GVFS/GVFS.Common/FileSystem/IPlatformFileSystem.cs +++ b/GVFS/GVFS.Common/FileSystem/IPlatformFileSystem.cs @@ -18,5 +18,6 @@ public interface IPlatformFileSystem bool TryCreateDirectoryWithAdminAndUserModifyPermissions(string directoryPath, out string error); bool TryCreateOrUpdateDirectoryToAdminModifyPermissions(ITracer tracer, string directoryPath, out string error); bool IsFileSystemSupported(string path, out string error); + void EnsureDirectoryIsOwnedByCurrentUser(string workingDirectoryRoot); } } diff --git a/GVFS/GVFS.Common/GVFSEnlistment.cs b/GVFS/GVFS.Common/GVFSEnlistment.cs index e4eca57ad4..731f1b3559 100644 --- a/GVFS/GVFS.Common/GVFSEnlistment.cs +++ b/GVFS/GVFS.Common/GVFSEnlistment.cs @@ -217,7 +217,7 @@ public bool TryCreateEnlistmentSubFolders() { try { - Directory.CreateDirectory(this.WorkingDirectoryRoot); + GVFSPlatform.Instance.FileSystem.EnsureDirectoryIsOwnedByCurrentUser(this.WorkingDirectoryRoot); this.CreateHiddenDirectory(this.DotGVFSRoot); } catch (IOException) diff --git a/GVFS/GVFS.Platform.Windows/WindowsFileSystem.cs b/GVFS/GVFS.Platform.Windows/WindowsFileSystem.cs index 98990c4f5f..b79f1b3e5f 100644 --- a/GVFS/GVFS.Platform.Windows/WindowsFileSystem.cs +++ b/GVFS/GVFS.Platform.Windows/WindowsFileSystem.cs @@ -272,6 +272,34 @@ public bool IsFileSystemSupported(string path, out string error) return true; } + /// + /// On Windows, if the current user is elevated, the owner of the directory will be the Administrators group by default. + /// This runs afoul of the git "dubious ownership" check, which can fail if either the .git directory or the working directory + /// are not owned by the current user. + /// + /// At the moment git for windows does not consider a non-elevated admin to be the owner of a directory owned by the Administrators group, + /// though a fix is in progress in the microsoft fork of git. Libgit2(sharp) also does not have this fix. + /// + /// 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). + /// + public void EnsureDirectoryIsOwnedByCurrentUser(string directoryPath) + { + // Ensure directory exists, inheriting all other ACLS + Directory.CreateDirectory(directoryPath); + // If the user is currently elevated, the owner of the directory will be the Administrators group. + DirectorySecurity directorySecurity = Directory.GetAccessControl(directoryPath); + IdentityReference directoryOwner = directorySecurity.GetOwner(typeof(SecurityIdentifier)); + SecurityIdentifier administratorsSid = new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null); + if (directoryOwner == administratorsSid) + { + WindowsIdentity currentUser = WindowsIdentity.GetCurrent(); + directorySecurity.SetOwner(currentUser.User); + Directory.SetAccessControl(directoryPath, directorySecurity); + } + } + private class NativeFileReader { private const uint GenericRead = 0x80000000; diff --git a/GVFS/GVFS.UnitTests/Mock/FileSystem/MockPlatformFileSystem.cs b/GVFS/GVFS.UnitTests/Mock/FileSystem/MockPlatformFileSystem.cs index a75a669be9..9a01b7243b 100644 --- a/GVFS/GVFS.UnitTests/Mock/FileSystem/MockPlatformFileSystem.cs +++ b/GVFS/GVFS.UnitTests/Mock/FileSystem/MockPlatformFileSystem.cs @@ -70,5 +70,10 @@ public bool IsFileSystemSupported(string path, out string error) error = null; return true; } + + public void EnsureDirectoryIsOwnedByCurrentUser(string workingDirectoryRoot) + { + throw new NotSupportedException(); + } } } diff --git a/GVFS/GVFS/CommandLine/CloneVerb.cs b/GVFS/GVFS/CommandLine/CloneVerb.cs index 19cb05ba30..47d7d3f978 100644 --- a/GVFS/GVFS/CommandLine/CloneVerb.cs +++ b/GVFS/GVFS/CommandLine/CloneVerb.cs @@ -716,6 +716,17 @@ private Result TryInitRepo(ITracer tracer, GitRefs refs, Enlistment enlistmentTo return new Result(error); } + try + { + GVFSPlatform.Instance.FileSystem.EnsureDirectoryIsOwnedByCurrentUser(enlistmentToInit.DotGitRoot); + } + catch (IOException e) + { + string error = string.Format("Could not ensure .git directory is owned by current user: {0}", e.Message); + tracer.RelatedError(error); + return new Result(error); + } + GitProcess.Result remoteAddResult = new GitProcess(enlistmentToInit).RemoteAdd("origin", enlistmentToInit.RepoUrl); if (remoteAddResult.ExitCodeIsFailure) { From e2a00f56c9947889df1e17129ca2f2a4d00189e0 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Mon, 10 Feb 2025 14:00:24 +0000 Subject: [PATCH 2/2] deps: remove old unused upgrader methods Remove some of the unused methods left behind after the auto upgrader was shut down. Also remove the no longer needed dependencies. Signed-off-by: Matthew John Cheetham --- GVFS/GVFS.Common/GVFSPlatform.cs | 2 -- .../GVFS.Platform.Windows.csproj | 1 - GVFS/GVFS.Platform.Windows/WindowsPlatform.cs | 25 ------------------- .../Mock/Common/MockPlatform.cs | 5 ---- 4 files changed, 33 deletions(-) diff --git a/GVFS/GVFS.Common/GVFSPlatform.cs b/GVFS/GVFS.Common/GVFSPlatform.cs index 7c252d4934..0ec0bd6681 100644 --- a/GVFS/GVFS.Common/GVFSPlatform.cs +++ b/GVFS/GVFS.Common/GVFSPlatform.cs @@ -87,8 +87,6 @@ public static void Register(GVFSPlatform platform) public abstract bool TryGetGVFSHooksVersion(out string hooksVersion, out string error); public abstract bool TryInstallGitCommandHooks(GVFSContext context, string executingDirectory, string hookName, string commandHookPath, out string errorMessage); - public abstract bool TryVerifyAuthenticodeSignature(string path, out string subject, out string issuer, out string error); - public abstract Dictionary GetPhysicalDiskInfo(string path, bool sizeStatsOnly); public abstract bool IsConsoleOutputRedirectedToFile(); diff --git a/GVFS/GVFS.Platform.Windows/GVFS.Platform.Windows.csproj b/GVFS/GVFS.Platform.Windows/GVFS.Platform.Windows.csproj index 0528354ffc..5cbd7e1748 100644 --- a/GVFS/GVFS.Platform.Windows/GVFS.Platform.Windows.csproj +++ b/GVFS/GVFS.Platform.Windows/GVFS.Platform.Windows.csproj @@ -14,7 +14,6 @@ - diff --git a/GVFS/GVFS.Platform.Windows/WindowsPlatform.cs b/GVFS/GVFS.Platform.Windows/WindowsPlatform.cs index 1f22f32482..07043b727f 100644 --- a/GVFS/GVFS.Platform.Windows/WindowsPlatform.cs +++ b/GVFS/GVFS.Platform.Windows/WindowsPlatform.cs @@ -11,7 +11,6 @@ using System.IO; using System.IO.Pipes; using System.Linq; -using System.Management.Automation; using System.Security.AccessControl; using System.Security.Principal; using System.ServiceProcess; @@ -294,30 +293,6 @@ public override bool TryInstallGitCommandHooks(GVFSContext context, string execu return true; } - public override bool TryVerifyAuthenticodeSignature(string path, out string subject, out string issuer, out string error) - { - using (PowerShell powershell = PowerShell.Create()) - { - powershell.AddScript($"Get-AuthenticodeSignature -FilePath {path}"); - - Collection results = powershell.Invoke(); - if (powershell.HadErrors || results.Count <= 0) - { - subject = null; - issuer = null; - error = $"Powershell Get-AuthenticodeSignature failed, could not verify authenticode for {path}."; - return false; - } - - Signature signature = results[0].BaseObject as Signature; - bool isValid = signature.Status == SignatureStatus.Valid; - subject = signature.SignerCertificate.SubjectName.Name; - issuer = signature.SignerCertificate.IssuerName.Name; - error = isValid == false ? signature.StatusMessage : null; - return isValid; - } - } - public override string GetCurrentUser() { WindowsIdentity identity = WindowsIdentity.GetCurrent(); diff --git a/GVFS/GVFS.UnitTests/Mock/Common/MockPlatform.cs b/GVFS/GVFS.UnitTests/Mock/Common/MockPlatform.cs index 1d89320087..e7c5263c4e 100644 --- a/GVFS/GVFS.UnitTests/Mock/Common/MockPlatform.cs +++ b/GVFS/GVFS.UnitTests/Mock/Common/MockPlatform.cs @@ -59,11 +59,6 @@ public override bool TryInstallGitCommandHooks(GVFSContext context, string execu throw new NotSupportedException(); } - public override bool TryVerifyAuthenticodeSignature(string path, out string subject, out string issuer, out string error) - { - throw new NotImplementedException(); - } - public override string GetNamedPipeName(string enlistmentRoot) { return "GVFS_Mock_PipeName";