Fail fast on SHA256 repositories in FastFetch, gvfs clone, gvfs mount, and gvfs repair

GVFS/FastFetch/FastFetchVerb.cs

@@ -205,13 +205,15 @@ private int ExecuteWithExitCode()
             }
 
             // Check this before any code that reads or writes refs assuming the "files"
-            // ref storage format runs, since FastFetch can be pointed at an arbitrary
-            // pre-existing git repository rather than only ones 'gvfs clone' created.
-            // A genuine config-read failure (as opposed to the key simply being absent,
-            // which reports the repository as files-format) is treated as fatal here: a
-            // missing key does not reach this branch, so a read error means the repo's
-            // config is anomalous and continuing risks operating on an unsupported repo.
-            if (!RefStorage.TryIsReftableRepo(enlistment.CreateGitProcess(), out bool isReftableRepo, out string refStorageReadError))
+            // ref storage format, or any code that assumes SHA1-shaped object ids, runs -
+            // since FastFetch can be pointed at an arbitrary pre-existing git repository
+            // rather than only ones 'gvfs clone' created. A genuine ref-storage config-read
+            // failure (as opposed to the key simply being absent, which reports the
+            // repository as files-format) is treated as fatal here: a missing key does not
+            // reach this branch, so a read error means the repo's config is anomalous and
+            // continuing risks operating on an unsupported repo.
+            GitProcess git = enlistment.CreateGitProcess();
+            if (!RefStorage.TryIsReftableRepo(git, out bool isReftableRepo, out string refStorageReadError))
             {
                 Console.WriteLine("Could not determine the repository's ref storage format: " + refStorageReadError);
                 return ExitFailure;
@@ -222,6 +224,22 @@ private int ExecuteWithExitCode()
                 return ExitFailure;
             }
 
+            // Same rationale as above, for SHA256: a genuine config-read failure (as
+            // opposed to the key simply being absent, which reports the repository as
+            // SHA1) is treated as fatal here: a missing key does not reach this branch,
+            // so a read error means the repo's config is anomalous and continuing risks
+            // operating on an unsupported repo.
+            if (!ObjectFormat.TryIsSha256Repo(git, out bool isSha256Repo, out string objectFormatReadError))
+            {
+                Console.WriteLine("Could not determine the repository's object format: " + objectFormatReadError);
+                return ExitFailure;
+            }
+            else if (isSha256Repo)
+            {
+                Console.WriteLine(ObjectFormat.UnsupportedSha256ErrorMessage);
+                return ExitFailure;
+            }
+
             string commitish = this.Commit ?? this.Branch;
             if (string.IsNullOrWhiteSpace(commitish))
             {

GVFS/GVFS.Common/GVFSEnlistment.cs

@@ -124,7 +124,10 @@ public override string GitHeadLogPath
         public string GitStatusCacheFolder { get; private set; }
         public string GitStatusCachePath { get; private set; }
 
-        // These version properties are only used in logging during clone and mount to track version numbers
+        // GitVersion is load-bearing, not just logging: CloneVerb.CreateClone re-parses it
+        // to decide whether to pin 'git init' to --ref-format=files (GitProcess.Init). The
+        // other two (GVFSVersion, GVFSHooksVersion) remain logging-only, to track version
+        // numbers during clone and mount.
         public string GitVersion
         {
             get { return this.gitVersion; }
@@ -311,14 +314,27 @@ public static bool WaitUntilMounted(
                     }
                     catch (BrokenPipeException e)
                     {
-                        errorMessage = string.Format("Could not connect to GVFS.Mount: {0}", e);
-                        tracer.RelatedError($"{nameof(WaitUntilMounted)}: {errorMessage}");
+                        // Keep the console-facing message short - the mount process may have
+                        // exited legitimately (e.g. it already logged its own clear, specific
+                        // failure reason and called Environment.Exit, which is enough to break
+                        // an in-flight pipe write/read here). The full exception detail still
+                        // goes to this trace, and 'gvfs log' points users at the mount
+                        // process's own log, where the real root cause is recorded.
+                        errorMessage = string.Format("Could not connect to GVFS.Mount: {0}", e.Message);
+                        tracer.RelatedError(
+                            new EventMetadata { { "Exception", e.ToString() } },
+                            $"{nameof(WaitUntilMounted)}: {errorMessage}");
                         return false;
                     }
                     catch (JsonException e)
                     {
-                        errorMessage = string.Format("Failed to parse response from GVFS.Mount.\n {0}", e);
-                        tracer.RelatedError($"{nameof(WaitUntilMounted)}: {errorMessage}");
+                        // Same rationale as the BrokenPipeException case above: keep the
+                        // console-facing message short, and send the full exception detail
+                        // to the trace log instead of dumping a raw stack trace to the user.
+                        errorMessage = string.Format("Failed to parse response from GVFS.Mount: {0}", e.Message);
+                        tracer.RelatedError(
+                            new EventMetadata { { "Exception", e.ToString() } },
+                            $"{nameof(WaitUntilMounted)}: {errorMessage}");
                         return false;
                     }
                 }

GVFS/GVFS.Common/Git/ObjectFormat.cs

@@ -0,0 +1,92 @@
+using System;
+
+namespace GVFS.Common.Git
+{
+    /// <summary>
+    /// Detects whether a git repository was initialized with the SHA256 object format
+    /// (git's newer, opt-in hash algorithm) rather than the SHA1 format VFS for Git
+    /// assumes throughout its object id handling.
+    ///
+    /// There is no supported way to convert a SHA256 repository to SHA1 in place: an
+    /// object's id is a hash of its content plus the algorithm used to compute it, so
+    /// "downgrading" would mean rewriting every object with a new identity - effectively
+    /// a fresh clone. Callers should use this detection to fail fast with a clear error
+    /// rather than let SHA1-only code (e.g. Sha1Id, SHA1Util, index parsing) hit
+    /// corruption or an unpredictable crash on 20-byte/40-hex-char assumptions.
+    /// </summary>
+    public static class ObjectFormat
+    {
+        public const string ObjectFormatConfigName = "extensions.objectformat";
+        public const string Sha256Value = "sha256";
+
+        /// <summary>
+        /// Length, in hex characters, of a SHA256 object id (32 bytes) - twice
+        /// GVFSConstants.ShaStringLength, the equivalent SHA1 length. Whatever writes
+        /// .git/packed-refs ("&lt;objectid&gt; &lt;refname&gt;") - VFS for Git's own clone
+        /// (GitRefs.ToPackedRefs, CloneVerb.TryInitRepo), or git itself via 'git pack-refs',
+        /// 'git gc', or a 'git maintenance' task - always serializes object ids at this full
+        /// hex width for the repository's actual object format. So a width check on
+        /// packed-refs is a reliable physical, on-disk signal of a repository's object
+        /// format that survives even when git config is unreadable, regardless of who most
+        /// recently wrote the file.
+        /// </summary>
+        public const int Sha256HexIdLength = 64;
+
+        public const string UnsupportedSha256ErrorMessage =
+            "VFS for Git only supports SHA1 repositories, but this repository was initialized " +
+            "with the SHA256 object format. There is no supported way to convert a SHA256 " +
+            "repository to SHA1 in place. Re-clone or re-initialize the repository as a SHA1 " +
+            "repository (for example, 'git init --object-format=sha1').";
+
+        /// <summary>
+        /// Returns true if the given value of extensions.objectformat identifies a SHA256
+        /// repository. A missing/empty value means the repository defaults to SHA1.
+        /// </summary>
+        public static bool IsSha256(string objectFormatConfigValue)
+        {
+            return string.Equals(objectFormatConfigValue?.Trim(), Sha256Value, StringComparison.OrdinalIgnoreCase);
+        }
+
+        /// <summary>
+        /// Reads extensions.objectformat from the local config of the repo backing the given
+        /// GitProcess, and returns true if it identifies a SHA256 repository.
+        ///
+        /// A config read failure (e.g. a broken git invocation, as opposed to the key simply
+        /// being absent) is treated the same as "not SHA256" here, matching the established
+        /// behavior of GitProcess.TryGetFromConfig elsewhere in this codebase for optional
+        /// config reads. Use <see cref="TryIsSha256Repo"/> if the caller wants to distinguish
+        /// and surface that failure instead of silently treating it as SHA1.
+        /// </summary>
+        public static bool IsSha256Repo(GitProcess git)
+        {
+            TryIsSha256Repo(git, out bool isSha256, out string _);
+            return isSha256;
+        }
+
+        /// <summary>
+        /// Reads extensions.objectformat from the local config of the repo backing the given
+        /// GitProcess. Returns false and populates <paramref name="error"/> if the config
+        /// could not be read at all (a missing key is not an error - it simply means the
+        /// repository defaults to SHA1, and isSha256 is set to false).
+        ///
+        /// Note the limits of this distinction: 'git config' reports a missing key and a
+        /// missing/unreadable .git/config file identically - exit code 1 with no stderr
+        /// content - so a repository whose entire config is gone is treated as "not SHA256"
+        /// here, not as a read failure. This matches the codebase-wide convention for
+        /// optional config reads (GitProcess.TryGetFromConfig has the same ambiguity) rather
+        /// than a gap specific to this check.
+        /// </summary>
+        public static bool TryIsSha256Repo(GitProcess git, out bool isSha256, out string error)
+        {
+            GitProcess.ConfigResult result = git.GetFromLocalConfig(ObjectFormatConfigName);
+            if (!result.TryParseAsString(out string value, out error))
+            {
+                isSha256 = false;
+                return false;
+            }
+
+            isSha256 = IsSha256(value);
+            return true;
+        }
+    }
+}

GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerTestCase/RepairTests.cs

@@ -131,7 +131,7 @@
                 (current, temp) =>
                 {
                     byte[] currentStartOfIndex = new byte[20];
                     current.Read(currentStartOfIndex, 0, currentStartOfIndex.Length);
                     temp.Write(currentStartOfIndex, 0, currentStartOfIndex.Length);
                 });
 
@@ -159,6 +159,46 @@
             this.Enlistment.MountGVFS();
         }
 
+        [TestCase]
+        public void DoesNotRebuildSha256RepoConfig()
+        {
+            this.Enlistment.UnmountGVFS();
+
+            // Make this otherwise-ordinary enlistment's config look like a SHA256 repo -
+            // core.repositoryformatversion=1 plus extensions.objectformat=sha256, the shape
+            // 'git init --object-format=sha256' produces - while leaving the rest of the
+            // config (including the origin remote) intact and parseable. The enlistment's
+            // actual objects/refs are still genuinely SHA1-hashed; this exercises the
+            // config-based detection path specifically, not the packed-refs physical
+            // fallback (which only fires when the config itself is unreadable).
+            string configPath = Path.Combine(this.Enlistment.RepoBackingRoot, ".git", "config");
+            string originalConfig = File.ReadAllText(configPath);
+            string sha256Config = originalConfig.Replace("repositoryformatversion = 0", "repositoryformatversion = 1")
+                + "[extensions]\n\tobjectformat = sha256\n";
+            File.WriteAllText(configPath, sha256Config);
+
+            this.Enlistment.TryMountGVFS().ShouldEqual(false, "GVFS shouldn't mount a SHA256 repo");
+
+            this.RepairWithoutConfirmShouldNotFix();
+
+            // This drives HasIssue's CantFix path (same detection as above), so repair
+            // reports the job as unfixable and never calls TryFixIssues for it at all -
+            // it does not exercise TryFixIssues's own, separate SHA256 guard, which
+            // RepairVerb never reaches this job for in practice. What this test does pin is
+            // the end-to-end, user-visible outcome: repair must leave this repo exactly as
+            // broken (still SHA256, still unmountable) as it found it, not silently rebuild
+            // it as a SHA1-shaped config over SHA256-hashed objects/refs.
+            this.Enlistment.Repair(confirm: true);
+            this.Enlistment.TryMountGVFS().ShouldEqual(false, "Repair must not make a SHA256 repo mountable");
+
+            // Assert the config is byte-for-byte what was written, not merely that it still
+            // mentions "sha256" - a weaker check would pass even on a half-rebuilt config
+            // where repositoryformatversion was reset to 0 but extensions.objectformat
+            // survived, which is exactly the broken state this guard exists to prevent.
+            string configAfterRepair = File.ReadAllText(configPath);
+            configAfterRepair.ShouldEqual(sha256Config, "Repair must not modify a SHA256 repo's config at all");
+        }
+
         private void CreateCorruptIndexAndRename(string indexPath, Action<FileStream, FileStream> corruptionAction)
         {
             string tempIndexPath = indexPath + ".lock";

GVFS/GVFS.Mount/InProcessMount.cs

@@ -291,8 +291,13 @@ private void MountWithLockAcquired(EventLevel verbosity, Keywords keywords)
                     // regardless. A genuine config-read failure is fatal here rather than a
                     // warning: a missing key reports the repo as files-format and does not reach
                     // this branch, so a read error means the config is anomalous and proceeding
-                    // into the force-write could brick an unsupported repo.
-                    if (!RefStorage.TryIsReftableRepo(git, out bool isReftableRepo, out string refStorageReadError))
+                    // into the force-write could brick an unsupported repo. Retry a few times
+                    // first (see RetryShortConfigRead) to ride out a purely transient failure -
+                    // e.g. AV momentarily holding .git/config - rather than failing mount on
+                    // something that would have succeeded moments later; a failure that
+                    // persists through the retries would fail mount later anyway once
+                    // TrySetRequiredGitConfigSettings itself tries to read/write that config.
+                    if (!RetryShortConfigRead(() => RefStorage.TryIsReftableRepo(git, out bool v, out string e) ? (true, v, e) : (false, v, e), out bool isReftableRepo, out string refStorageReadError))
                     {
                         // Pass the git error as a format argument, not concatenated into the
                         // format string: FailMountAndExit routes through ITracer.RelatedError(
@@ -306,6 +311,26 @@ private void MountWithLockAcquired(EventLevel verbosity, Keywords keywords)
                         this.FailMountAndExit(RefStorage.UnsupportedReftableErrorMessage);
                     }
 
+                    // Same rationale and retry policy as above, for SHA256: applying the
+                    // required-config force-write to a SHA256 repo (extensions.objectformat)
+                    // would brick it identically, so this check must also run before that
+                    // write. A genuine config-read failure is fatal here rather than a
+                    // warning: a missing key reports the repo as SHA1 and does not reach this
+                    // branch, so a read error means the config is anomalous and proceeding
+                    // into the force-write could brick an unsupported repo.
+                    if (!RetryShortConfigRead(() => ObjectFormat.TryIsSha256Repo(git, out bool v, out string e) ? (true, v, e) : (false, v, e), out bool isSha256Repo, out string objectFormatReadError))
+                    {
+                        // See the matching comment on the ref storage format check above:
+                        // FailMountAndExit requires the git error as a format argument rather
+                        // than concatenated into the format string, to avoid a FormatException
+                        // on a stray '{' in crafted/corrupt git stderr.
+                        this.FailMountAndExit("Could not determine the repository's object format: {0}", objectFormatReadError);
+                    }
+                    else if (isSha256Repo)
+                    {
+                        this.FailMountAndExit(ObjectFormat.UnsupportedSha256ErrorMessage);
+                    }
+
                     if (!GVFSPlatform.Instance.FileSystem.IsFileSystemSupported(this.enlistment.WorkingDirectoryRoot, out string fsError))
                     {
                         this.FailMountAndExit("FileSystem unsupported: " + fsError);
@@ -667,6 +692,39 @@ private void FailMountAndExit(string error, params object[] args)
             this.FailMountAndExit(ReturnCode.GenericError, error, args);
         }
 
+        /// <summary>
+        /// Retries a 'git config' read a few times, a short delay apart, before giving up -
+        /// so a purely transient failure (for example AV momentarily holding .git/config)
+        /// does not fail mount outright. A failure that persists through the retries is
+        /// still fatal to the caller: mount would reach the same unreadable config again
+        /// moments later via TrySetRequiredGitConfigSettings, so there is nothing to gain by
+        /// retrying longer than this.
+        /// </summary>
+        private static bool RetryShortConfigRead(Func<(bool success, bool value, string error)> tryRead, out bool value, out string error)
+        {
+            const int maxAttempts = 3;
+            const int delayMs = 100;
+
+            (bool success, bool value, string error) result = (false, false, null);
+            for (int attempt = 1; attempt <= maxAttempts; attempt++)
+            {
+                result = tryRead();
+                if (result.success)
+                {
+                    break;
+                }
+
+                if (attempt < maxAttempts)
+                {
+                    Thread.Sleep(delayMs);
+                }
+            }
+
+            value = result.value;
+            error = result.error;
+            return result.success;
+        }
+
         private void FailMountAndExit(ReturnCode returnCode, string error, params object[] args)
         {
             this.currentState = MountState.MountFailed;