Fail blob hydration cleanly on a malformed (NUL-byte) placeholder SHA

GVFS/GVFS.Common/Git/GVFSGitObjects.cs

@@ -67,6 +67,26 @@ public virtual bool TryCopyBlobContentStream(
             Action<Stream, long> writeAction,
             out BlobHydrationFailureCategory failureCategory)
         {
+            // Short-circuit a malformed SHA (for example a corrupt placeholder's all-NUL
+            // content-id) before the retry loop. GitRepo already rejects it as a clean miss,
+            // but a bogus SHA can never be downloaded either (the server returns 404), so
+            // attempting it would only produce doomed download retries. Because the read is
+            // never satisfied, the caller re-requests it endlessly, which turns one corrupt
+            // placeholder into an unbounded error/retry storm. Fail fast and cheap instead.
+            // The cause stays categorized as Unexpected (no dedicated category); the caller
+            // tags its terminal telemetry from failureCategory below.
+            if (!SHA1Util.IsValidShaFormat(sha))
+            {
+                EventMetadata metadata = new EventMetadata();
+                metadata.Add("sha", SHA1Util.ToLoggableShaString(sha));
+                metadata.Add("RequestSource", requestSource.ToString());
+                metadata.Add(TracingConstants.MessageKey.WarningMessage, "TryCopyBlobContentStream: Refusing to hydrate blob with malformed SHA");
+                this.Tracer.RelatedEvent(EventLevel.Warning, nameof(this.TryCopyBlobContentStream) + "_MalformedBlobSha", metadata, Keywords.Telemetry);
+
+                failureCategory = BlobHydrationFailureCategory.Unexpected;
+                return false;
+            }
+
             // Track the outcome of the most recent attempt so that the terminal failure
             // telemetry can attribute the failure to a cause (network vs. object-missing vs.
             // local copy) that is otherwise collapsed into the bool return value below. The
@@ -188,6 +208,27 @@ private DownloadAndSaveObjectResult TryDownloadAndSaveObject(
             RequestSource requestSource,
             bool retryOnFailure)
         {
+            // Defense in depth for a malformed object id (for example a corrupt placeholder's
+            // all-NUL content-id). On .NET Framework Path.Combine threw ArgumentException on
+            // such a value; on modern .NET it does not, so a malformed SHA silently misses the
+            // local object store and would otherwise be sent to the cache server, which rejects
+            // the URL with HTTP 400 - and GVFS then erases a valid credential (HttpRequestor
+            // treats 400 as an auth failure), producing a credential-prompt storm. Callers other
+            // than blob hydration reach this method WITHOUT going through the
+            // TryCopyBlobContentStream guard - the git.exe read-object hook (NamedPipeMessage,
+            // via InProcessMount) and the gitattributes GVFSVerb - so reject a malformed SHA here
+            // for every caller before any request is built.
+            if (!SHA1Util.IsValidShaFormat(objectId))
+            {
+                EventMetadata metadata = new EventMetadata();
+                metadata.Add("sha", SHA1Util.ToLoggableShaString(objectId));
+                metadata.Add("RequestSource", requestSource.ToString());
+                metadata.Add(TracingConstants.MessageKey.WarningMessage, nameof(this.TryDownloadAndSaveObject) + ": Refusing to download object with malformed SHA");
+                this.Tracer.RelatedEvent(EventLevel.Warning, nameof(this.TryDownloadAndSaveObject) + "_MalformedBlobSha", metadata, Keywords.Telemetry);
+
+                return DownloadAndSaveObjectResult.Error;
+            }
+
             if (objectId == GVFSConstants.AllZeroSha)
             {
                 return DownloadAndSaveObjectResult.Error;

GVFS/GVFS.Common/Git/GitRepo.cs

@@ -113,6 +113,19 @@ public virtual bool CommitAndRootTreeExists(string commitSha, out string rootTre
         /// </summary>
         public virtual bool LooseObjectExists(string sha)
         {
+            // Guard against a malformed SHA (for example a corrupt placeholder's all-NUL
+            // content-id) so Path.Combine cannot throw ArgumentException below. Emit the same
+            // greppable Warning as the other malformed-SHA guards so a silent "does not exist"
+            // answer is still diagnosable in telemetry.
+            if (!SHA1Util.IsValidShaFormat(sha))
+            {
+                EventMetadata metadata = new EventMetadata();
+                metadata.Add("sha", SHA1Util.ToLoggableShaString(sha));
+                metadata.Add(TracingConstants.MessageKey.WarningMessage, nameof(this.LooseObjectExists) + ": Malformed SHA cannot exist as a loose object");
+                this.tracer.RelatedEvent(EventLevel.Warning, nameof(this.LooseObjectExists) + "_MalformedBlobSha", metadata, Keywords.Telemetry);
+                return false;
+            }
+
             if (GVFSPlatform.Instance.Constants.CaseSensitiveFileSystem)
             {
                 sha = sha.ToLower();
@@ -343,6 +356,33 @@ private LooseBlobState GetLooseBlobStateAtPath(string blobPath, Action<Stream, l
 
         private LooseBlobState GetLooseBlobState(string blobSha, Action<Stream, long> writeAction, out long size)
         {
+            // A corrupt placeholder can carry a malformed content-id (for example 40 NUL
+            // bytes instead of a hex SHA). Reject it up front and report an invalid loose
+            // object, which the callers treat as a clean, non-retryable miss.
+            //
+            // The behavior of Path.Combine below is runtime-dependent, so validating here
+            // (rather than relying on an exception) is required on modern .NET:
+            //   - On .NET Framework, Path.Combine throws ArgumentException ("Illegal
+            //     characters in path") on the NUL bytes. ArgumentException is not handled by
+            //     RetryWrapper, so it bypasses both the retry logic and the download fallback
+            //     and fails the hydration permanently (a retry storm - the original symptom).
+            //   - On modern .NET (.NET Core/5+), Path.Combine no longer validates path
+            //     characters, so it does NOT throw; the bogus path simply misses on disk and
+            //     the request would fall through to a server download that the gateway rejects
+            //     with HTTP 400 (ICM 850075166). The download path is guarded separately in
+            //     GVFSGitObjects.TryDownloadAndSaveObject.
+            if (!SHA1Util.IsValidShaFormat(blobSha))
+            {
+                size = -1;
+
+                EventMetadata metadata = new EventMetadata();
+                metadata.Add("sha", SHA1Util.ToLoggableShaString(blobSha));
+                metadata.Add(TracingConstants.MessageKey.WarningMessage, nameof(this.GetLooseBlobState) + ": Refusing to build loose object path from malformed blob SHA");
+                this.tracer.RelatedEvent(EventLevel.Warning, nameof(this.GetLooseBlobState) + "_MalformedBlobSha", metadata, Keywords.Telemetry);
+
+                return LooseBlobState.Invalid;
+            }
+
             // Ensure SHA path is lowercase for case-sensitive filesystems
             if (GVFSPlatform.Instance.Constants.CaseSensitiveFileSystem)
             {

GVFS/GVFS.Common/SHA1Util.cs

@@ -9,7 +9,37 @@ public static class SHA1Util
     {
         public static bool IsValidShaFormat(string sha)
         {
-            return sha.Length == 40 && sha.All(c => Uri.IsHexDigit(c));
+            return sha != null && sha.Length == 40 && sha.All(c => Uri.IsHexDigit(c));
+        }
+
+        /// <summary>
+        /// Returns a log-safe rendering of a value that was expected to be a
+        /// 40-character hex SHA but is not. Non-hex characters (for example the
+        /// NUL bytes of a corrupt placeholder content-id) are escaped as \uXXXX
+        /// so the value stays greppable in telemetry and carries no control
+        /// characters.
+        /// </summary>
+        public static string ToLoggableShaString(string sha)
+        {
+            if (sha == null)
+            {
+                return "(null)";
+            }
+
+            StringBuilder builder = new StringBuilder(sha.Length);
+            foreach (char c in sha)
+            {
+                if (Uri.IsHexDigit(c))
+                {
+                    builder.Append(c);
+                }
+                else
+                {
+                    builder.AppendFormat("\\u{0:x4}", (int)c);
+                }
+            }
+
+            return builder.ToString();
         }
 
         public static string SHA1HashStringForUTF8String(string s)

GVFS/GVFS.Platform.Windows/WindowsFileSystemVirtualizer.cs

@@ -733,7 +733,7 @@ public HResult GetFileDataCallback(
                 metadata.Add("streamGuid", streamGuid);
                 metadata.Add("triggeringProcessId", triggeringProcessId);
                 metadata.Add("triggeringProcessImageFileName", triggeringProcessImageFileName);
-                metadata.Add("sha", sha);
+                metadata.Add("sha", SHA1Util.ToLoggableShaString(sha));
                 metadata.Add("placeholderVersion", placeholderVersion);
                 metadata.Add("commandId", commandId);
 

GVFS/GVFS.UnitTests/Common/SHA1UtilTests.cs

@@ -1,6 +1,7 @@
 using GVFS.Common;
 using GVFS.Tests.Should;
 using NUnit.Framework;
+using System.Linq;
 using System.Text;
 
 namespace GVFS.UnitTests.Common
@@ -31,6 +32,30 @@ public void IsValidFullSHAIsFalseForEmptyString()
             SHA1Util.IsValidShaFormat(string.Empty).ShouldEqual(false);
         }
 
+        [TestCase]
+        public void IsValidShaFormatIsFalseForNull()
+        {
+            SHA1Util.IsValidShaFormat(null).ShouldEqual(false);
+        }
+
+        [TestCase]
+        public void ToLoggableShaStringEscapesNonHexCharacters()
+        {
+            SHA1Util.ToLoggableShaString(null).ShouldEqual("(null)");
+            SHA1Util.ToLoggableShaString(new string('\0', 3)).ShouldEqual("\\u0000\\u0000\\u0000");
+            SHA1Util.ToLoggableShaString("abc\0").ShouldEqual("abc\\u0000");
+            SHA1Util.ToLoggableShaString("abcDEF123").ShouldEqual("abcDEF123");
+
+            // Control characters and non-ASCII / high code points must be escaped and padded to 4 hex digits.
+            SHA1Util.ToLoggableShaString("a\tb\n").ShouldEqual("a\\u0009b\\u000a");
+            SHA1Util.ToLoggableShaString("\u00e9\u1234").ShouldEqual("\\u00e9\\u1234");
+
+            // The realistic corrupt-content-id shape: a full 40-char value that is partly valid
+            // hex and partly NUL, rendered with the hex kept and the NULs escaped.
+            SHA1Util.ToLoggableShaString(new string('a', 20) + new string('\0', 20))
+                .ShouldEqual(new string('a', 20) + string.Concat(Enumerable.Repeat("\\u0000", 20)));
+        }
+
         [TestCase]
         public void IsValidFullSHAIsFalseForHexStringsNot40Chars()
         {