From da656688d553541de5aa58b92178a21fb5ae3f78 Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Fri, 5 Jun 2026 10:38:50 -0700 Subject: [PATCH 1/3] Mount: self-heal stale hook configurations Two related fixes that together let `gvfs mount` succeed against an enlistment whose pre-command hook is stale - typically because the GVFS install location baked into .git/hooks/pre-command.hooks at clone time has since moved (re-install, version-junction swap, system-to-user migration, etc.). Before this change, a stale .hooks text file causes every git invocation that fires the pre-command hook to fail with: fatal: pre-command hook aborted command which makes the mount path unrecoverable. Changes: HooksInstaller.TryUpdateHooks Also refresh the .hooks text files. Previously TryUpdateHooks only refreshed the .exe copies of GitHooksLoader; the .hooks text file (containing the absolute path of GVFS.Hooks.exe that the loader execs) was only written at clone time by InstallHooks. When the GVFS install moves, the .exe copies stay valid but the .hooks path goes stale - and gvfs.mount.exe's existing TryUpdateHooks call didn't repair it. The new TryInstallGitCommandHooks calls are idempotent: when the GVFS install path hasn't changed, the file is rewritten with the same content. GitProcess Pass usePreCommandHook:false on all git operations that run during the mount bootstrap path. These calls happen before gvfs.mount.exe reaches TryUpdateHooks, so without this flag they trip over the very stale-hook config we're trying to repair. Affected: SetInLocalConfig, AddInLocalConfig, DeleteFromLocalConfig, TryGetAllConfig, TryGetConfigUrlMatch, TryGetCredential, TryGetCertificatePassword, TryDeleteCredential, TryStoreCredential. GetFromConfig, GetFromLocalConfig and IsValidRepo also gain the flag (some via the existing GetOriginUrl pattern, some new). None of these operations mutate the working tree, so pre-command hook is semantically inappropriate anyway - skipping it is correct independent of the stale-hook scenario. The mechanism: usePreCommandHook:false sets the COMMAND_HOOK_LOCK environment variable, which Microsoft Git itself reads to suppress pre-command hook invocation. So the failure is bypassed at the git layer, not just inside GVFS.Hooks.exe. Testing: - 818/818 unit tests pass - Manually verified end-to-end with a real enlistment whose pre-command.hooks was corrupted to point at a non-existent path (C:\NonExistent\Path\GVFS.Hooks.exe). Before this change, `gvfs mount` failed with "pre-command hook aborted command". After this change, mount succeeds and the .hooks file is rewritten to point at the currently-running GVFS install. Assisted-by: Claude Opus 4.7 Signed-off-by: Tyrie Vella --- GVFS/GVFS.Common/FileSystem/HooksInstaller.cs | 22 +++++++ GVFS/GVFS.Common/Git/GitProcess.cs | 66 +++++++++++++------ 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/GVFS/GVFS.Common/FileSystem/HooksInstaller.cs b/GVFS/GVFS.Common/FileSystem/HooksInstaller.cs index bdf6a03cb..407918c67 100644 --- a/GVFS/GVFS.Common/FileSystem/HooksInstaller.cs +++ b/GVFS/GVFS.Common/FileSystem/HooksInstaller.cs @@ -124,6 +124,28 @@ public static bool TryUpdateHooks(GVFSContext context, out string errorMessage) return false; } + // Refresh the corresponding .hooks text files. These hold the + // absolute path of GVFS.Hooks.exe that the loader execs at hook + // time, and were originally written at clone time pointing at + // wherever GVFS was installed back then. If GVFS has moved + // (system-to-user migration, version-junction swap, hand-edited + // install), those paths go stale and the loader exits non-zero + // on every git invocation that fires a hook - making the + // enlistment unrecoverable through normal mount. Refreshing on + // every mount makes us self-healing against install-location + // drift, and is a no-op when paths are already current. + string precommandBasePath = Path.Combine(context.Enlistment.WorkingDirectoryBackingRoot, GVFSConstants.DotGit.Hooks.PreCommandPath); + if (!GVFSPlatform.Instance.TryInstallGitCommandHooks(context, ExecutingDirectory, GVFSConstants.DotGit.Hooks.PreCommandHookName, precommandBasePath, out errorMessage)) + { + return false; + } + + string postcommandBasePath = Path.Combine(context.Enlistment.WorkingDirectoryBackingRoot, GVFSConstants.DotGit.Hooks.PostCommandPath); + if (!GVFSPlatform.Instance.TryInstallGitCommandHooks(context, ExecutingDirectory, GVFSConstants.DotGit.Hooks.PostCommandHookName, postcommandBasePath, out errorMessage)) + { + return false; + } + return true; } diff --git a/GVFS/GVFS.Common/Git/GitProcess.cs b/GVFS/GVFS.Common/Git/GitProcess.cs index b818fd915..623a52f1f 100644 --- a/GVFS/GVFS.Common/Git/GitProcess.cs +++ b/GVFS/GVFS.Common/Git/GitProcess.cs @@ -191,7 +191,8 @@ public virtual bool TryDeleteCredential(ITracer tracer, string repoUrl, string u Result result = this.InvokeGitAgainstDotGitFolder( GenerateCredentialVerbCommand("reject"), stdin => stdin.Write(stdinConfig), - null); + null, + usePreCommandHook: false); if (result.ExitCodeIsFailure) { @@ -218,7 +219,8 @@ public virtual bool TryStoreCredential(ITracer tracer, string repoUrl, string us Result result = this.InvokeGitAgainstDotGitFolder( GenerateCredentialVerbCommand("approve"), stdin => stdin.Write(stdinConfig), - null); + null, + usePreCommandHook: false); if (result.ExitCodeIsFailure) { @@ -249,10 +251,13 @@ public virtual bool TryGetCertificatePassword( using (ITracer activity = tracer.StartActivity("TryGetCertificatePassword", EventLevel.Informational)) { + // See GetFromConfig for why pre-command hook is disabled + // for bootstrap-time git operations. Result gitCredentialOutput = this.InvokeGitAgainstDotGitFolder( "credential fill", stdin => stdin.Write("protocol=cert\npath=" + certificatePath + "\nusername=\n\n"), - parseStdOutLine: null); + parseStdOutLine: null, + usePreCommandHook: false); if (gitCredentialOutput.ExitCodeIsFailure) { @@ -300,10 +305,13 @@ public virtual bool TryGetCredential( using (ITracer activity = tracer.StartActivity(nameof(this.TryGetCredential), EventLevel.Informational)) { + // See GetFromConfig for why pre-command hook is disabled + // for bootstrap-time git operations. Result gitCredentialOutput = this.InvokeGitAgainstDotGitFolder( GenerateCredentialVerbCommand("fill"), stdin => stdin.Write($"url={repoUrl}\n\n"), - parseStdOutLine: null); + parseStdOutLine: null, + usePreCommandHook: false); if (gitCredentialOutput.ExitCodeIsFailure) { @@ -336,7 +344,10 @@ public virtual bool TryGetCredential( public bool IsValidRepo() { - Result result = this.InvokeGitAgainstDotGitFolder("rev-parse --show-toplevel"); + // Mount-time bootstrap check - skip pre-command hook so a broken + // hook config in the enlistment can be detected and repaired + // rather than blocking the mount that would fix it. + Result result = this.InvokeGitAgainstDotGitFolder("rev-parse --show-toplevel", usePreCommandHook: false); return result.ExitCodeIsSuccess; } @@ -352,24 +363,34 @@ public Result GetCurrentBranchName() public void DeleteFromLocalConfig(string settingName) { - this.InvokeGitAgainstDotGitFolder("config --local --unset-all " + settingName); + // git config operations never need the pre-command hook (no + // working-tree mutation). Skipping it also keeps mount bootstrap + // robust against a stale hook config that TryUpdateHooks will + // repair shortly. See GetFromConfig for the longer rationale. + this.InvokeGitAgainstDotGitFolder("config --local --unset-all " + settingName, usePreCommandHook: false); } public Result SetInLocalConfig(string settingName, string value, bool replaceAll = false) { - return this.InvokeGitAgainstDotGitFolder(string.Format( - "config --local {0} \"{1}\" \"{2}\"", - replaceAll ? "--replace-all " : string.Empty, - settingName, - value)); + // See DeleteFromLocalConfig for why pre-command hook is disabled. + return this.InvokeGitAgainstDotGitFolder( + string.Format( + "config --local {0} \"{1}\" \"{2}\"", + replaceAll ? "--replace-all " : string.Empty, + settingName, + value), + usePreCommandHook: false); } public Result AddInLocalConfig(string settingName, string value) { - return this.InvokeGitAgainstDotGitFolder(string.Format( - "config --local --add {0} {1}", - settingName, - value)); + // See DeleteFromLocalConfig for why pre-command hook is disabled. + return this.InvokeGitAgainstDotGitFolder( + string.Format( + "config --local --add {0} {1}", + settingName, + value), + usePreCommandHook: false); } public Result SetInFileConfig(string configFile, string settingName, string value, bool replaceAll = false) @@ -384,7 +405,8 @@ public Result SetInFileConfig(string configFile, string settingName, string valu public bool TryGetConfigUrlMatch(string section, string repositoryUrl, out Dictionary configSettings) { - Result result = this.InvokeGitAgainstDotGitFolder($"config --get-urlmatch {section} {repositoryUrl}"); + // See GetFromConfig for why pre-command hook is disabled. + Result result = this.InvokeGitAgainstDotGitFolder($"config --get-urlmatch {section} {repositoryUrl}", usePreCommandHook: false); if (result.ExitCodeIsFailure) { configSettings = null; @@ -399,7 +421,8 @@ public bool TryGetAllConfig(bool localOnly, out Dictionary From d2a393ace09ae1d90b2558a53315ee84c495186b Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Thu, 4 Jun 2026 11:48:42 -0700 Subject: [PATCH 2/3] Use git_odb_exists instead of git_revparse_single for ObjectExists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the heavyweight git_revparse_single call in LibGit2Repo.ObjectExists with git_odb_exists, a purpose-built existence check that skips revparse expression parsing and git_object handle allocation. Benchmarked on an os.2020 enlistment (59.7M objects, 14 packs): - Existing objects: ~800 ns/op (comparable) - Missing objects: 1.3ms vs 2.8ms (2.1x faster) The ODB handle is lazily acquired on first ObjectExists call via git_repository_odb (returns the repo's internal ODB, ref-counted) and freed in Dispose. Concurrent first-time calls are safe via Interlocked.CompareExchange — the loser frees its duplicate handle. Falls back to revparse if ODB acquisition fails. Add ObjectCanBeParsed method that retains the old revparse behavior for callers that need corruption detection (LooseObjectsStep) or that may receive refs/abbreviated SHAs rather than full 40-char hex (BlobPrefetcher.DownloadMissingCommit, which takes raw CLI input). Assisted-by: Claude Opus 4.6 Signed-off-by: Tyler Vella --- GVFS/GVFS.Common/Git/GitRepo.cs | 12 +++ GVFS/GVFS.Common/Git/LibGit2Repo.cs | 73 +++++++++++++++++++ .../Maintenance/LooseObjectsStep.cs | 2 +- GVFS/GVFS.Common/Prefetch/BlobPrefetcher.cs | 5 +- 4 files changed, 90 insertions(+), 2 deletions(-) diff --git a/GVFS/GVFS.Common/Git/GitRepo.cs b/GVFS/GVFS.Common/Git/GitRepo.cs index d88ebbd89..7b6dfddb8 100644 --- a/GVFS/GVFS.Common/Git/GitRepo.cs +++ b/GVFS/GVFS.Common/Git/GitRepo.cs @@ -114,6 +114,18 @@ public virtual bool ObjectExists(string blobSha) return output; } + /// + /// Checks whether the object can be fully parsed by libgit2 (not just that it exists). + /// Use this to detect corrupt objects. For simple existence checks, + /// prefer which is faster. + /// + public virtual bool ObjectCanBeParsed(string sha) + { + bool output = false; + this.libgit2RepoInvoker.TryInvoke(repo => repo.ObjectCanBeParsed(sha), out output); + return output; + } + /// /// Try to find the size of a given blob by SHA1 hash. /// diff --git a/GVFS/GVFS.Common/Git/LibGit2Repo.cs b/GVFS/GVFS.Common/Git/LibGit2Repo.cs index f0e7bc464..dafcc8d54 100644 --- a/GVFS/GVFS.Common/Git/LibGit2Repo.cs +++ b/GVFS/GVFS.Common/Git/LibGit2Repo.cs @@ -3,12 +3,14 @@ using System.Collections.Generic; using System.IO; using System.Runtime.InteropServices; +using System.Threading; namespace GVFS.Common.Git { public class LibGit2Repo : IDisposable { private bool disposedValue = false; + private IntPtr odbHandle = IntPtr.Zero; public delegate void MultiVarConfigCallback(string value); @@ -104,6 +106,55 @@ public virtual bool CommitAndRootTreeExists(string commitish, out string treeSha } public virtual bool ObjectExists(string sha) + { + IntPtr odb = this.odbHandle; + if (odb == IntPtr.Zero) + { + if (Native.Odb.GetOdb(out IntPtr newOdb, this.RepoHandle) != Native.ResultCode.Success) + { + return this.ObjectExistsFallback(sha); + } + + IntPtr existing = Interlocked.CompareExchange(ref this.odbHandle, newOdb, IntPtr.Zero); + if (existing != IntPtr.Zero) + { + // Another thread won the race — free our duplicate and use theirs + Native.Odb.Free(newOdb); + odb = existing; + } + else + { + odb = newOdb; + } + } + + GitOid oid; + if (Native.Odb.OidFromStr(out oid, sha) != Native.ResultCode.Success) + { + return false; + } + + return Native.Odb.Exists(odb, ref oid) == 1; + } + + private bool ObjectExistsFallback(string sha) + { + IntPtr objHandle; + if (Native.RevParseSingle(out objHandle, this.RepoHandle, sha) != Native.ResultCode.Success) + { + return false; + } + + Native.Object.Free(objHandle); + return true; + } + + /// + /// Checks whether the object can be fully parsed by libgit2 (not just that it exists). + /// Use this when you need to detect corrupt objects. For simple existence checks, + /// prefer which is faster. + /// + public virtual bool ObjectCanBeParsed(string sha) { IntPtr objHandle; if (Native.RevParseSingle(out objHandle, this.RepoHandle, sha) != Native.ResultCode.Success) @@ -360,6 +411,12 @@ protected virtual void Dispose(bool disposing) { if (!this.disposedValue) { + if (this.odbHandle != IntPtr.Zero) + { + Native.Odb.Free(this.odbHandle); + this.odbHandle = IntPtr.Zero; + } + Native.Repo.Free(this.RepoHandle); Native.Shutdown(); this.disposedValue = true; @@ -504,6 +561,22 @@ public static class Repo public static extern void Free(IntPtr repoHandle); } + public static class Odb + { + [DllImport(Git2NativeLibName, EntryPoint = "git_repository_odb")] + public static extern ResultCode GetOdb(out IntPtr odbHandle, IntPtr repoHandle); + + /// 1 if the object exists, 0 otherwise + [DllImport(Git2NativeLibName, EntryPoint = "git_odb_exists")] + public static extern int Exists(IntPtr odbHandle, ref GitOid id); + + [DllImport(Git2NativeLibName, EntryPoint = "git_odb_free")] + public static extern void Free(IntPtr odbHandle); + + [DllImport(Git2NativeLibName, EntryPoint = "git_oid_fromstr")] + public static extern ResultCode OidFromStr(out GitOid oid, string str); + } + public static class Config { [DllImport(Git2NativeLibName, EntryPoint = "git_repository_config")] diff --git a/GVFS/GVFS.Common/Maintenance/LooseObjectsStep.cs b/GVFS/GVFS.Common/Maintenance/LooseObjectsStep.cs index f71f0d6cc..f45049ec6 100644 --- a/GVFS/GVFS.Common/Maintenance/LooseObjectsStep.cs +++ b/GVFS/GVFS.Common/Maintenance/LooseObjectsStep.cs @@ -172,7 +172,7 @@ public void ClearCorruptLooseObjects(EventMetadata metadata) // may be more bad objects in the next batch after deleting the corrupt objects. foreach (string objectId in this.GetBatchOfLooseObjects(2 * this.MaxLooseObjectsInPack)) { - if (!this.Context.Repository.ObjectExists(objectId)) + if (!this.Context.Repository.ObjectCanBeParsed(objectId)) { string objectFile = this.GetLooseObjectFileName(objectId); diff --git a/GVFS/GVFS.Common/Prefetch/BlobPrefetcher.cs b/GVFS/GVFS.Common/Prefetch/BlobPrefetcher.cs index 29bc4cc67..7010ebb7e 100644 --- a/GVFS/GVFS.Common/Prefetch/BlobPrefetcher.cs +++ b/GVFS/GVFS.Common/Prefetch/BlobPrefetcher.cs @@ -476,7 +476,10 @@ protected void DownloadMissingCommit(string commitSha, GitObjects gitObjects) { using (LibGit2Repo repo = new LibGit2Repo(this.Tracer, this.Enlistment.WorkingDirectoryBackingRoot)) { - if (!repo.ObjectExists(commitSha)) + // Use ObjectCanBeParsed (revparse) rather than ObjectExists (odb_exists) + // because commitSha may be a ref name or abbreviated SHA from CLI input + // (e.g. FastFetch --commit), not necessarily a full 40-char hex SHA. + if (!repo.ObjectCanBeParsed(commitSha)) { if (!gitObjects.TryDownloadCommit(commitSha)) { From 009aaf94be010f6377e2d5ec33e697b39a7ee0f1 Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Thu, 4 Jun 2026 14:31:12 -0700 Subject: [PATCH 3/3] Fix CA2022 warnings: avoid inexact Stream.Read calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace Stream.Read with Stream.ReadExactly where the caller expects all requested bytes (EnlistmentHydrationSummary, ReusableMemoryStream). In GitRepo.ReadLooseObjectHeader, check the Read return value instead of switching to ReadExactly — ReadExactly would throw EndOfStreamException on a truncated header, routing to the IOException catch (LooseBlobState.Unknown) instead of the header-mismatch path (LooseBlobState.Corrupt) that quarantines the file. Suppress CA2022 in GitIndexParser.ReadNextPage where partial last-page reads are intentional and the parser stops after entryCount entries. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella --- GVFS/GVFS.Common/Git/GitRepo.cs | 9 +++++++-- .../HealthCalculator/EnlistmentHydrationSummary.cs | 2 +- GVFS/GVFS.UnitTests/Mock/ReusableMemoryStream.cs | 2 +- .../Projection/GitIndexProjection.GitIndexParser.cs | 4 ++++ 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/GVFS/GVFS.Common/Git/GitRepo.cs b/GVFS/GVFS.Common/Git/GitRepo.cs index d88ebbd89..d2d8b6eee 100644 --- a/GVFS/GVFS.Common/Git/GitRepo.cs +++ b/GVFS/GVFS.Common/Git/GitRepo.cs @@ -153,8 +153,13 @@ private static bool ReadLooseObjectHeader(Stream input, out long size) size = 0; byte[] buffer = new byte[5]; - input.Read(buffer, 0, buffer.Length); - if (!Enumerable.SequenceEqual(buffer, LooseBlobHeader)) + + // Verify bytesRead instead of using ReadExactly: a truncated header must + // return false (Corrupt) so the caller quarantines the file, rather than + // throwing EndOfStreamException which would be caught as IOException + // (Unknown) and skip quarantine. + int bytesRead = input.Read(buffer, 0, buffer.Length); + if (bytesRead < buffer.Length || !Enumerable.SequenceEqual(buffer, LooseBlobHeader)) { return false; } diff --git a/GVFS/GVFS.Common/HealthCalculator/EnlistmentHydrationSummary.cs b/GVFS/GVFS.Common/HealthCalculator/EnlistmentHydrationSummary.cs index a2f83afd4..5dbe7c335 100644 --- a/GVFS/GVFS.Common/HealthCalculator/EnlistmentHydrationSummary.cs +++ b/GVFS/GVFS.Common/HealthCalculator/EnlistmentHydrationSummary.cs @@ -196,7 +196,7 @@ internal static int GetIndexFileCount(GVFSEnlistment enlistment, PhysicalFileSys * the 4 bytes at offsets 8-11 of the index file. */ indexFile.Position = 8; var bytes = new byte[4]; - indexFile.Read( + indexFile.ReadExactly( bytes, // Destination buffer offset: 0, // Offset in destination buffer, not in indexFile count: 4); diff --git a/GVFS/GVFS.UnitTests/Mock/ReusableMemoryStream.cs b/GVFS/GVFS.UnitTests/Mock/ReusableMemoryStream.cs index 5afa60627..dfd70a943 100644 --- a/GVFS/GVFS.UnitTests/Mock/ReusableMemoryStream.cs +++ b/GVFS/GVFS.UnitTests/Mock/ReusableMemoryStream.cs @@ -67,7 +67,7 @@ public string ReadAt(long position, long length) this.Position = position; byte[] bytes = new byte[length]; - this.Read(bytes, 0, (int)length); + this.ReadExactly(bytes, 0, (int)length); this.Position = lastPosition; diff --git a/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.GitIndexParser.cs b/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.GitIndexParser.cs index 382a05945..4f83ff0fd 100644 --- a/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.GitIndexParser.cs +++ b/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.GitIndexParser.cs @@ -391,7 +391,11 @@ private FileSystemTaskResult ParseIndex( private void ReadNextPage() { + // Last page may be smaller than PageSize; partial fill is safe because + // the parser stops after entryCount entries and never reads stale bytes. +#pragma warning disable CA2022 // Avoid inexact read this.indexStream.Read(this.page, 0, PageSize); +#pragma warning restore CA2022 this.nextByteIndex = 0; }