diff --git a/GVFS/FastFetch/FastFetchVerb.cs b/GVFS/FastFetch/FastFetchVerb.cs index 0b0c3147a7..6762e258dd 100644 --- a/GVFS/FastFetch/FastFetchVerb.cs +++ b/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)) { diff --git a/GVFS/GVFS.Common/GVFSEnlistment.cs b/GVFS/GVFS.Common/GVFSEnlistment.cs index 961d546242..7d040291b5 100644 --- a/GVFS/GVFS.Common/GVFSEnlistment.cs +++ b/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; } } diff --git a/GVFS/GVFS.Common/Git/ObjectFormat.cs b/GVFS/GVFS.Common/Git/ObjectFormat.cs new file mode 100644 index 0000000000..fb8e50c5f3 --- /dev/null +++ b/GVFS/GVFS.Common/Git/ObjectFormat.cs @@ -0,0 +1,92 @@ +using System; + +namespace GVFS.Common.Git +{ + /// + /// 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. + /// + public static class ObjectFormat + { + public const string ObjectFormatConfigName = "extensions.objectformat"; + public const string Sha256Value = "sha256"; + + /// + /// Length, in hex characters, of a SHA256 object id (32 bytes) - twice + /// GVFSConstants.ShaStringLength, the equivalent SHA1 length. Whatever writes + /// .git/packed-refs ("<objectid> <refname>") - 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. + /// + 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')."; + + /// + /// Returns true if the given value of extensions.objectformat identifies a SHA256 + /// repository. A missing/empty value means the repository defaults to SHA1. + /// + public static bool IsSha256(string objectFormatConfigValue) + { + return string.Equals(objectFormatConfigValue?.Trim(), Sha256Value, StringComparison.OrdinalIgnoreCase); + } + + /// + /// 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 if the caller wants to distinguish + /// and surface that failure instead of silently treating it as SHA1. + /// + public static bool IsSha256Repo(GitProcess git) + { + TryIsSha256Repo(git, out bool isSha256, out string _); + return isSha256; + } + + /// + /// Reads extensions.objectformat from the local config of the repo backing the given + /// GitProcess. Returns false and populates 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. + /// + 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; + } + } +} diff --git a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerTestCase/RepairTests.cs b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerTestCase/RepairTests.cs index f7d2516657..0cfd978e74 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerTestCase/RepairTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerTestCase/RepairTests.cs @@ -159,6 +159,46 @@ public void FixesCorruptGitConfig() 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 corruptionAction) { string tempIndexPath = indexPath + ".lock"; diff --git a/GVFS/GVFS.Mount/InProcessMount.cs b/GVFS/GVFS.Mount/InProcessMount.cs index a6b5d9e94b..cf9276b05f 100644 --- a/GVFS/GVFS.Mount/InProcessMount.cs +++ b/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); } + /// + /// 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. + /// + 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; diff --git a/GVFS/GVFS.UnitTests/Common/WaitUntilMountedProcessTrackingTests.cs b/GVFS/GVFS.UnitTests/Common/WaitUntilMountedProcessTrackingTests.cs index cf04194ebe..64ae80bd8a 100644 --- a/GVFS/GVFS.UnitTests/Common/WaitUntilMountedProcessTrackingTests.cs +++ b/GVFS/GVFS.UnitTests/Common/WaitUntilMountedProcessTrackingTests.cs @@ -1,9 +1,12 @@ using GVFS.Common; +using GVFS.Common.NamedPipes; using GVFS.Tests.Should; using GVFS.UnitTests.Mock.Common; using NUnit.Framework; using System; +using System.IO.Pipes; using System.Threading; +using System.Threading.Tasks; namespace GVFS.UnitTests.Common { @@ -84,5 +87,94 @@ public void DetectsLateProcessExitWhilePipeNeverAppears() // exit on the second poll, well within a few seconds. Assert.That(elapsed.TotalSeconds, Is.LessThan(10), "WaitUntilMounted should detect late process exit within a few connect retries"); } + + [TestCase] + public void BrokenPipeMidPollProducesShortMessageAndLogsFullExceptionDetail() + { + string pipeName = "GVFS_UnitTest_BrokenPipe_" + Guid.NewGuid().ToString("N"); + + using (NamedPipeServerStream serverStream = new NamedPipeServerStream( + pipeName, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.Asynchronous)) + { + Task serverTask = Task.Run(() => + { + serverStream.WaitForConnection(); + + // Read the client's first GetStatus request using the same framing + // NamedPipeStreamWriter/Reader use, then disappear without responding - + // simulating the mount process exiting mid-poll (e.g. FailMountAndExit + // calling Environment.Exit while the client already sent a request). + new NamedPipeStreamReader(serverStream).ReadMessage(); + serverStream.Dispose(); + }); + + MockTracer tracer = new MockTracer(); + string errorMessage; + bool result = GVFSEnlistment.WaitUntilMounted( + tracer, + pipeName, + "C:\\fake\\root", + false, + out errorMessage); + + serverTask.Wait(TimeSpan.FromSeconds(10)); + + result.ShouldBeFalse(); + errorMessage.ShouldNotBeNull(); + errorMessage.ShouldContain("Could not connect to GVFS.Mount"); + + // The console-facing message must stay short - no embedded stack trace + // (a stack trace is what BrokenPipeException.ToString() would produce). + errorMessage.ShouldNotContain(false, " at ", Environment.NewLine); + + // The full exception detail must still be captured for diagnostics. + tracer.RelatedErrorEvents.Count.ShouldEqual(1); + tracer.RelatedErrorEvents[0].ShouldContain("Exception"); + } + } + + [TestCase] + public void JsonExceptionMidPollProducesShortMessageAndLogsFullExceptionDetail() + { + string pipeName = "GVFS_UnitTest_JsonException_" + Guid.NewGuid().ToString("N"); + + using (NamedPipeServerStream serverStream = new NamedPipeServerStream( + pipeName, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.Asynchronous)) + { + Task serverTask = Task.Run(() => + { + serverStream.WaitForConnection(); + + // Read the client's GetStatus request, then respond with a payload that + // is not valid JSON - simulating a truncated or malformed mount response - + // so Response.FromJson throws a JsonException. + new NamedPipeStreamReader(serverStream).ReadMessage(); + new NamedPipeStreamWriter(serverStream).WriteMessage("{ not valid json"); + }); + + MockTracer tracer = new MockTracer(); + string errorMessage; + bool result = GVFSEnlistment.WaitUntilMounted( + tracer, + pipeName, + "C:\\fake\\root", + false, + out errorMessage); + + serverTask.Wait(TimeSpan.FromSeconds(10)); + + result.ShouldBeFalse(); + errorMessage.ShouldNotBeNull(); + errorMessage.ShouldContain("Failed to parse response from GVFS.Mount"); + + // The console-facing message must stay short - no embedded stack trace + // (a stack trace is what JsonException.ToString() would produce). + errorMessage.ShouldNotContain(false, " at ", Environment.NewLine); + + // The full exception detail must still be captured for diagnostics. + tracer.RelatedErrorEvents.Count.ShouldEqual(1); + tracer.RelatedErrorEvents[0].ShouldContain("Exception"); + } + } } } diff --git a/GVFS/GVFS.UnitTests/Git/ObjectFormatTests.cs b/GVFS/GVFS.UnitTests/Git/ObjectFormatTests.cs new file mode 100644 index 0000000000..0832380183 --- /dev/null +++ b/GVFS/GVFS.UnitTests/Git/ObjectFormatTests.cs @@ -0,0 +1,144 @@ +using GVFS.Common.Git; +using GVFS.Tests.Should; +using GVFS.UnitTests.Mock.Git; +using NUnit.Framework; + +namespace GVFS.UnitTests.Git +{ + [TestFixture] + public class ObjectFormatTests + { + private const string ConfigCommand = "config --local " + ObjectFormat.ObjectFormatConfigName; + + [TestCase] + public void IsSha256IsTrueForSha256Value() + { + ObjectFormat.IsSha256("sha256").ShouldEqual(true); + } + + [TestCase] + public void IsSha256IsTrueForMixedCaseSha256Value() + { + ObjectFormat.IsSha256("SHA256").ShouldEqual(true); + } + + [TestCase] + public void IsSha256IsTrueForSha256ValueWithSurroundingWhitespace() + { + ObjectFormat.IsSha256(" sha256 \n").ShouldEqual(true); + } + + [TestCase] + public void IsSha256IsFalseForSha1Value() + { + ObjectFormat.IsSha256("sha1").ShouldEqual(false); + } + + [TestCase] + public void IsSha256IsFalseForNullValue() + { + ObjectFormat.IsSha256(null).ShouldEqual(false); + } + + [TestCase] + public void IsSha256IsFalseForEmptyValue() + { + ObjectFormat.IsSha256(string.Empty).ShouldEqual(false); + } + + [TestCase] + public void IsSha256RepoIsTrueWhenConfigIsSha256() + { + MockGitProcess git = new MockGitProcess(); + git.SetExpectedCommandResult( + ConfigCommand, + () => new GitProcess.Result("sha256\n", string.Empty, GitProcess.Result.SuccessCode)); + + ObjectFormat.IsSha256Repo(git).ShouldEqual(true); + } + + [TestCase] + public void IsSha256RepoIsFalseWhenConfigIsSha1() + { + MockGitProcess git = new MockGitProcess(); + git.SetExpectedCommandResult( + ConfigCommand, + () => new GitProcess.Result("sha1\n", string.Empty, GitProcess.Result.SuccessCode)); + + ObjectFormat.IsSha256Repo(git).ShouldEqual(false); + } + + [TestCase] + public void IsSha256RepoIsFalseWhenConfigIsMissing() + { + MockGitProcess git = new MockGitProcess(); + + // A missing config key causes 'git config' to exit non-zero with no stderr output, + // which is the observed behavior for a SHA1 repo that predates extensions.objectformat. + git.SetExpectedCommandResult( + ConfigCommand, + () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.GenericFailureCode)); + + ObjectFormat.IsSha256Repo(git).ShouldEqual(false); + } + + [TestCase] + public void IsSha256RepoSwallowsGenuineReadFailureAndReturnsFalse() + { + MockGitProcess git = new MockGitProcess(); + + // The non-Try overload deliberately swallows a genuine config-read failure + // (non-zero exit with real stderr content) and reports "not SHA256", matching + // GitProcess.TryGetFromConfig's "failure == not set" convention. Callers that + // need to distinguish a read failure use TryIsSha256Repo instead. + git.SetExpectedCommandResult( + ConfigCommand, + () => new GitProcess.Result(string.Empty, "fatal: not a git repository", GitProcess.Result.GenericFailureCode)); + + ObjectFormat.IsSha256Repo(git).ShouldEqual(false); + } + + [TestCase] + public void TryIsSha256RepoSucceedsWithNoErrorWhenConfigIsSha256() + { + MockGitProcess git = new MockGitProcess(); + git.SetExpectedCommandResult( + ConfigCommand, + () => new GitProcess.Result("sha256\n", string.Empty, GitProcess.Result.SuccessCode)); + + ObjectFormat.TryIsSha256Repo(git, out bool isSha256, out string error).ShouldEqual(true); + isSha256.ShouldEqual(true); + error.ShouldEqual(string.Empty); + } + + [TestCase] + public void TryIsSha256RepoSucceedsWithNoErrorWhenConfigIsMissing() + { + MockGitProcess git = new MockGitProcess(); + git.SetExpectedCommandResult( + ConfigCommand, + () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.GenericFailureCode)); + + ObjectFormat.TryIsSha256Repo(git, out bool isSha256, out string error).ShouldEqual(true); + isSha256.ShouldEqual(false); + error.ShouldEqual(string.Empty); + } + + [TestCase] + public void TryIsSha256RepoFailsAndReportsErrorWhenConfigReadFails() + { + MockGitProcess git = new MockGitProcess(); + + // A genuine 'git config' failure (non-zero exit with real stderr content, as + // opposed to a missing key) should be surfaced distinctly rather than silently + // treated the same as "not SHA256". + git.SetExpectedCommandResult( + ConfigCommand, + () => new GitProcess.Result(string.Empty, "fatal: not a git repository", GitProcess.Result.GenericFailureCode)); + + ObjectFormat.TryIsSha256Repo(git, out bool isSha256, out string error).ShouldEqual(false); + isSha256.ShouldEqual(false); + error.ShouldNotBeNull(); + } + } +} diff --git a/GVFS/GVFS.UnitTests/RepairJobs/GitConfigRepairJobTests.cs b/GVFS/GVFS.UnitTests/RepairJobs/GitConfigRepairJobTests.cs new file mode 100644 index 0000000000..d22f207a91 --- /dev/null +++ b/GVFS/GVFS.UnitTests/RepairJobs/GitConfigRepairJobTests.cs @@ -0,0 +1,116 @@ +using GVFS.Common; +using GVFS.RepairJobs; +using GVFS.Tests.Should; +using GVFS.UnitTests.Mock.Common; +using NUnit.Framework; +using System; +using System.IO; + +namespace GVFS.UnitTests.RepairJobs +{ + [TestFixture] + public class GitConfigRepairJobTests + { + private string testEnlistmentRoot; + private string dotGitRoot; + + [SetUp] + public void CreateTestEnlistmentRoot() + { + this.testEnlistmentRoot = Path.Combine(Path.GetTempPath(), "GVFSUnitTests_GitConfigRepairJob_" + Guid.NewGuid().ToString("N")); + + // Construct a real (non-mock) GVFSEnlistment pointed at a scratch directory purely + // to get DotGitRoot's platform-specific layout right - PackedRefsLookSha256 only + // does real file I/O under that path, so there is no need to mock GitProcess or + // register GVFSPlatform for these tests specifically (though the [SetUpFixture] in + // GVFS.UnitTests.Setup must still have run, as it does for the whole assembly). + GVFSEnlistment enlistment = new GVFSEnlistment(this.testEnlistmentRoot, "mock://repoUrl", "mock:\\git.exe", authentication: null); + this.dotGitRoot = enlistment.DotGitRoot; + Directory.CreateDirectory(this.dotGitRoot); + } + + [TearDown] + public void DeleteTestEnlistmentRoot() + { + if (Directory.Exists(this.testEnlistmentRoot)) + { + Directory.Delete(this.testEnlistmentRoot, recursive: true); + } + } + + [TestCase] + public void PackedRefsLookSha256IsTrueForSha256WidthObjectId() + { + this.WritePackedRefs("bb3fd16685b64a36d8d433fd0684d192277bb3b64f2b8441c5832d8d3ba8da91 refs/heads/master"); + + this.CreateJob().PackedRefsLookSha256().ShouldEqual(true); + } + + [TestCase] + public void PackedRefsLookSha256IsFalseForSha1WidthObjectId() + { + this.WritePackedRefs("a3541ccec5694b0334d91298e348dc7401c07ff7 refs/heads/master"); + + this.CreateJob().PackedRefsLookSha256().ShouldEqual(false); + } + + [TestCase] + public void PackedRefsLookSha256SkipsHeaderCommentAndPeeledLines() + { + // Matches the real shape 'git pack-refs'/GitRefs.ToPackedRefs produce: a leading + // '#' comment, and '^'-prefixed peeled object ids for annotated tags that must not + // be mistaken for the preceding ref's own object id. + this.WritePackedRefs( + "# pack-refs with: peeled fully-peeled", + "bb3fd16685b64a36d8d433fd0684d192277bb3b64f2b8441c5832d8d3ba8da91 refs/tags/v1", + "^0000000000000000000000000000000000000000000000000000000000000000"); + + this.CreateJob().PackedRefsLookSha256().ShouldEqual(true); + } + + [TestCase] + public void PackedRefsLookSha256IsFalseWhenFileIsMissing() + { + this.CreateJob().PackedRefsLookSha256().ShouldEqual(false); + } + + [TestCase] + public void PackedRefsLookSha256SkipsNonHexGarbageAtSha1WidthAndFindsRealSignal() + { + // A 40-character garbled first token (not valid hex) must not be trusted as a + // definitive SHA1 answer - that would mask the genuine SHA256-width signal on + // the next line, which is the false-negative failure mode on exactly the + // corrupt enlistments this probe runs against. + this.WritePackedRefs( + "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz refs/heads/garbled", + "bb3fd16685b64a36d8d433fd0684d192277bb3b64f2b8441c5832d8d3ba8da91 refs/heads/master"); + + this.CreateJob().PackedRefsLookSha256().ShouldEqual(true); + } + + [TestCase] + public void PackedRefsLookSha256SkipsNonHexGarbageAtSha256WidthOnHealthySha1Repo() + { + // A 64-character garbled token (not valid hex) must not pin this to "SHA256" - + // that would permanently misclassify a healthy SHA1 repo, the false-positive + // failure mode. The genuine SHA1-width signal on the next line is the real + // answer. + this.WritePackedRefs( + "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz refs/heads/garbled", + "a3541ccec5694b0334d91298e348dc7401c07ff7 refs/heads/master"); + + this.CreateJob().PackedRefsLookSha256().ShouldEqual(false); + } + + private GitConfigRepairJob CreateJob() + { + GVFSEnlistment enlistment = new GVFSEnlistment(this.testEnlistmentRoot, "mock://repoUrl", "mock:\\git.exe", authentication: null); + return new GitConfigRepairJob(new MockTracer(), new StringWriter(), enlistment); + } + + private void WritePackedRefs(params string[] lines) + { + File.WriteAllLines(Path.Combine(this.dotGitRoot, GVFSConstants.DotGit.PackedRefsName), lines); + } + } +} diff --git a/GVFS/GVFS/CommandLine/CloneVerb.cs b/GVFS/GVFS/CommandLine/CloneVerb.cs index ce579d0459..00dca57d66 100644 --- a/GVFS/GVFS/CommandLine/CloneVerb.cs +++ b/GVFS/GVFS/CommandLine/CloneVerb.cs @@ -865,10 +865,10 @@ private Result TryInitRepo(ITracer tracer, GitRefs refs, Enlistment enlistmentTo } // 'git init' above pins --ref-format=files when the installed git supports it (git - // 2.45+), so a user's init.defaultRefFormat=reftable no longer produces a reftable - // repo on a supported git. This check remains as defense-in-depth: it covers the - // case where the git version could not be determined (the pin is then omitted) and - // any repo that reached v1 by another route. VFS for Git populates refs by writing + // 2.45+), so init.defaultRefFormat=reftable cannot produce a reftable repo on a + // supported git. This check remains as defense-in-depth: it covers the case where + // the git version could not be determined (the pin is then omitted) and any repo + // that reached v1 by another route. VFS for Git populates refs by writing // .git/packed-refs directly (below, and via GitRefs.ToPackedRefs) rather than // through git's ref backend, and a reftable repo ignores packed-refs - so such a // clone would silently produce an enlistment whose remote refs are invisible to @@ -876,7 +876,8 @@ private Result TryInitRepo(ITracer tracer, GitRefs refs, Enlistment enlistmentTo // fatal here: this is our own just-initialized repo, so a read error (as opposed to // the key being absent, which reports files-format) means its config is unreadable // and the clone cannot safely continue. - if (!RefStorage.TryIsReftableRepo(enlistmentToInit.CreateGitProcess(), out bool isReftableRepo, out string refStorageReadError)) + GitProcess gitForFormatChecks = enlistmentToInit.CreateGitProcess(); + if (!RefStorage.TryIsReftableRepo(gitForFormatChecks, out bool isReftableRepo, out string refStorageReadError)) { string readError = "Could not determine the new repository's ref storage format: " + refStorageReadError; tracer.RelatedError(readError); @@ -888,6 +889,25 @@ private Result TryInitRepo(ITracer tracer, GitRefs refs, Enlistment enlistmentTo return new Result(RefStorage.UnsupportedReftableErrorMessage); } + // 'git init' above pins --object-format=sha1, so init.defaultObjectFormat=sha256 + // cannot produce a SHA256 repo on a supported git. This check remains as + // defense-in-depth: it covers any pre-existing repo that reached v1 by another + // route (e.g. an older GVFS build, or hand-edited config). A genuine + // config-read failure is fatal here: this is our own just-initialized repo, so + // a read error (as opposed to the key being absent, which reports SHA1) means + // its config is unreadable and the clone cannot safely continue. + if (!ObjectFormat.TryIsSha256Repo(gitForFormatChecks, out bool isSha256Repo, out string objectFormatReadError)) + { + string readError = "Could not determine the new repository's object format: " + objectFormatReadError; + tracer.RelatedError(readError); + return new Result(readError); + } + else if (isSha256Repo) + { + tracer.RelatedError(ObjectFormat.UnsupportedSha256ErrorMessage); + return new Result(ObjectFormat.UnsupportedSha256ErrorMessage); + } + try { GVFSPlatform.Instance.FileSystem.EnsureDirectoryIsOwnedByCurrentUser(enlistmentToInit.DotGitRoot); diff --git a/GVFS/GVFS/RepairJobs/GitConfigRepairJob.cs b/GVFS/GVFS/RepairJobs/GitConfigRepairJob.cs index a7b33df884..d8c3646a5d 100644 --- a/GVFS/GVFS/RepairJobs/GitConfigRepairJob.cs +++ b/GVFS/GVFS/RepairJobs/GitConfigRepairJob.cs @@ -2,6 +2,7 @@ using GVFS.Common; using GVFS.Common.Git; using GVFS.Common.Tracing; +using System; using System.Collections.Generic; using System.IO; @@ -38,6 +39,20 @@ public override IssueType HasIssue(List messages) return IssueType.CantFix; } + // A SHA256 repository is not something 'gvfs repair' can make usable (see + // TryFixIssues), so report it as an unfixable issue rather than letting + // 'gvfs repair' declare the enlistment healthy. Detect it both from the config + // extension and from the physical .git/packed-refs object-id width: the latter + // still identifies a SHA256 repo when the config is too corrupt for 'git config' + // to read, which is exactly the state repair runs in. A config-read failure + // alone is otherwise swallowed as "not SHA256" so the normal config checks below + // still run - repair exists to diagnose exactly that kind of broken config. + if (ObjectFormat.IsSha256Repo(git) || this.PackedRefsLookSha256()) + { + messages.Add(ObjectFormat.UnsupportedSha256ErrorMessage); + return IssueType.CantFix; + } + GitProcess.ConfigResult originResult = git.GetOriginUrl(); string error; string originUrl; @@ -90,6 +105,8 @@ public override IssueType HasIssue(List messages) public override FixResult TryFixIssues(List messages) { + GitProcess git = new GitProcess(this.Enlistment); + // Check before touching the config file at all: TryFixIssues rebuilds the // entire config from scratch (wiping it to empty, then writing only the // required/optional settings this codebase knows about via @@ -109,7 +126,7 @@ public override FixResult TryFixIssues(List messages) // files-format config, so a repo whose config is merely unreadable is exactly what // repair must be allowed to fix. bool reftableDirectoryExists = this.ReftableBackendDirectoryExists(); - bool configReadable = RefStorage.TryIsReftableRepo(new GitProcess(this.Enlistment), out bool isReftableByConfig, out string refStorageReadError); + bool configReadable = RefStorage.TryIsReftableRepo(git, out bool isReftableByConfig, out string refStorageReadError); if (reftableDirectoryExists || (configReadable && isReftableByConfig)) { messages.Add(RefStorage.UnsupportedReftableErrorMessage); @@ -121,6 +138,44 @@ public override FixResult TryFixIssues(List messages) this.Tracer.RelatedWarning("Could not determine the repository's ref storage format; proceeding with repair: " + refStorageReadError); } + // Same rationale as above, for SHA256: TrySetRequiredGitConfigSettings also + // force-writes core.repositoryformatversion=0 and drops extensions.objectformat. + // Rebuilding a SHA256 repo's config that way would strip the objectformat + // extension while its objects/index remain SHA256-hashed, producing a + // differently-broken repo. There is no way for 'gvfs repair' to make a + // SHA256 repo usable by VFS for Git, so fail clearly instead of attempting + // the rebuild. + // + // Detect SHA256 from the config AND from the physical .git/packed-refs + // object-id width (PackedRefsLookSha256 - see ObjectFormat.Sha256HexIdLength for + // why that width is a reliable physical signal). This mirrors the reftable + // directory check above, and is what makes the fallback safe when the config is + // unreadable. A config-read failure with no SHA256-width packed-refs is NOT + // blocked, unlike at mount/clone/FastFetch: repair's whole purpose is to rebuild + // a corrupt config, so a repo whose config is merely unreadable is exactly what + // repair must be allowed to fix. + // + // In practice this whole block is unreachable through 'gvfs repair': HasIssue + // above runs this identical detection first and returns CantFix for the same + // condition, so RepairVerb.Execute never adds this job to its "fixable" set and + // never calls TryFixIssues for it (RepairTests.DoesNotRebuildSha256RepoConfig + // exercises exactly that HasIssue/CantFix path). The check stays here anyway, as + // defense-in-depth should TryFixIssues ever be called directly or RepairVerb's + // sequencing change, so a future caller does not silently rebuild a SHA256 + // repo's config as SHA1. + bool packedRefsLookSha256 = this.PackedRefsLookSha256(); + bool objectFormatReadable = ObjectFormat.TryIsSha256Repo(git, out bool isSha256ByConfig, out string objectFormatReadError); + if (packedRefsLookSha256 || (objectFormatReadable && isSha256ByConfig)) + { + messages.Add(ObjectFormat.UnsupportedSha256ErrorMessage); + return FixResult.Failure; + } + + if (!objectFormatReadable) + { + this.Tracer.RelatedWarning("Could not determine the repository's object format; proceeding with repair: " + objectFormatReadError); + } + string configPath = Path.Combine(this.Enlistment.WorkingDirectoryBackingRoot, GVFSConstants.DotGit.Config); string configBackupPath; if (!this.TryRenameToBackupFile(configPath, out configBackupPath, messages)) @@ -179,5 +234,85 @@ private bool ReftableBackendDirectoryExists() return Directory.Exists(reftableDirectory); } + + internal bool PackedRefsLookSha256() + { + // Same DotGitRoot assumption as ReftableBackendDirectoryExists() above - holds for + // the primary enlistment repair targets, not a linked worktree. + // + // VFS for Git writes .git/packed-refs directly during clone (GitRefs.ToPackedRefs, + // consumed by CloneVerb.TryInitRepo) rather than through git's ref backend, so it + // is reliably present. See ObjectFormat.Sha256HexIdLength for why its object-id + // width is a reliable physical signal of the repository's object format even if + // the file is later rewritten by git itself. + // + // This only ever runs against an enlistment 'gvfs repair' has already flagged as + // broken, so a malformed line is the expected case, not the exotic one: each + // candidate line's leading token must actually be hex and of a known object-id + // width before it is trusted, and a line that fails that check is skipped rather + // than treated as a definitive answer - a non-hex or wrong-width token picked up + // from the first space it happens to contain could otherwise mask a genuine + // SHA256-width line further down the file, or misclassify a healthy SHA1 repo. + string packedRefsPath = Path.Combine(this.Enlistment.DotGitRoot, GVFSConstants.DotGit.PackedRefsName); + + try + { + foreach (string line in File.ReadLines(packedRefsPath)) + { + if (line.Length == 0 || line[0] == '#' || line[0] == '^') + { + // '#' is the pack-refs header comment; '^' lines carry the peeled + // object id of an annotated tag, not a ref's own object id. + continue; + } + + int spaceIndex = line.IndexOf(' '); + if (spaceIndex <= 0) + { + continue; + } + + if (spaceIndex == ObjectFormat.Sha256HexIdLength && IsHex(line, 0, spaceIndex)) + { + return true; + } + + if (spaceIndex == GVFSConstants.ShaStringLength && IsHex(line, 0, spaceIndex)) + { + return false; + } + + // Neither a valid SHA256-width nor SHA1-width hex object id - this line + // is not a reliable signal either way, so keep scanning rather than + // committing to it. + } + } + catch (IOException) + { + // packed-refs missing or unreadable - not itself a SHA256 signal, and the + // config-based check this is paired with already handles a read failure. + } + catch (UnauthorizedAccessException) + { + // Same rationale as the IOException case above - File.ReadLines surfaces a + // permission/locking problem as UnauthorizedAccessException rather than + // IOException, and this probe must not propagate either kind of failure. + } + + return false; + } + + private static bool IsHex(string value, int start, int length) + { + for (int i = start; i < start + length; i++) + { + if (!Uri.IsHexDigit(value[i])) + { + return false; + } + } + + return true; + } } }