Fail blob hydration cleanly on a malformed (NUL-byte) placeholder SHA - #2074
Open
tyrielv wants to merge 2 commits into
Open
Fail blob hydration cleanly on a malformed (NUL-byte) placeholder SHA#2074tyrielv wants to merge 2 commits into
tyrielv wants to merge 2 commits into
Conversation
tyrielv
force-pushed
the
tyrielv/fix-invalid-sha-hydration
branch
3 times, most recently
from
August 7, 2026 21:57
902a99b to
882b7d8
Compare
When a user process reads a virtualized placeholder whose stored
content-id is corrupt - specifically 40 NUL bytes instead of a hex SHA -
GVFS builds a loose-object path from it and Path.Combine throws
System.ArgumentException ("Illegal characters in path").
ArgumentException is not in RetryWrapper.IsHandlableException, so it
bypasses both the retry logic and the download fallback in
GVFSGitObjects.TryCopyBlobContentStream and propagates to the
virtualizer's outer catch, which returns FileNotAvailable to ProjFS. The
placeholder can never hydrate, so the failing read repeats forever - a
retry storm. This is the #1 blob-hydration failure cause on the LKG field
build 1.0.26014.1 (38 machines; ~61 machines / ~6.2K events across 30d;
one machine emitted ~2.49M error events).
This is a corrupt content-id, NOT GVFSConstants.AllZeroSha: AllZeroSha is
40 ASCII '0' characters, which yields directory "00" and does not throw.
Reject a malformed SHA before it is turned into a path:
- GitRepo.GetLooseBlobState returns LooseBlobState.Invalid (a clean,
non-retryable miss) for a SHA that is not 40 hex characters, so
Path.Combine can never throw here again.
- GitRepo.LooseObjectExists guards the same Path.Combine.
- GVFSGitObjects.TryCopyBlobContentStream short-circuits a malformed SHA
before the retry loop, so a bogus SHA never triggers a doomed 404
download or a retry storm.
- SHA1Util.IsValidShaFormat is now null-safe; SHA1Util.ToLoggableShaString
renders the bad value with non-hex characters escaped so telemetry stays
greppable and free of control characters.
- WindowsFileSystemVirtualizer routes the request's logged sha through
ToLoggableShaString, so a malformed content-id can no longer enter
telemetry with raw NUL/control bytes at the terminal hydration-failure
error either (a no-op for a valid hex SHA).
All three guard sites emit the same greppable Warning event
(*_MalformedBlobSha) at Warning level with no unhandled exception. Per an
existing decision this case stays telemetry category "Unexpected"; no new
BlobHydrationFailureCategory is added.
Stacked on microsoft#2071 (tyrielv/split-hydration-enum-telemetry): this branch is
rebased onto it, so microsoft#2071's out BlobHydrationFailureCategory parameter is
honored - the malformed-SHA short-circuit sets failureCategory =
Unexpected, so the virtualizer's terminal telemetry tags the case exactly
as before (it no longer reaches the outer catch because it no longer
throws). This PR must NOT merge before microsoft#2071; after microsoft#2071 lands, rebase
onto master.
Unit tests assert that a 40-NUL-byte SHA, a 40-char SHA with an embedded
path-illegal character, and other malformed SHAs return false from both
GitRepo.TryCopyBlobContentStream and GVFSGitObjects.TryCopyBlobContentStream
with no ArgumentException (Assert.DoesNotThrow), that no download/retry is
attempted, and that the out category is Unexpected.
Assisted-by: Claude Opus 4.8
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
force-pushed
the
tyrielv/fix-invalid-sha-hydration
branch
from
August 7, 2026 22:04
882b7d8 to
2d88803
Compare
tyrielv
marked this pull request as ready for review
August 7, 2026 22:05
tyrielv
added a commit
to tyrielv/VFSForGit
that referenced
this pull request
Aug 10, 2026
When a user process reads a virtualized placeholder whose stored content-id is corrupt - 40 NUL bytes instead of a hex blob SHA - GVFS cannot hydrate it from the corrupt content-id. microsoft#2074 makes that read fail cleanly (no crash, no retry storm). This change goes one step further and repairs the underlying data so the file works again. Telemetry shows this corruption is durable and localized: the same 1-4 files per machine fail repeatedly over multiple days until something rewrites the placeholder (~61 machines / ~6.2K events over 30 days). It is old and version-agnostic (spans >=4 GVFS builds), not a 2.0 regression. The triggering processes are readers (git.exe, copilot.exe, Code.exe); the placeholder was already corrupt on disk. The authoritative path->SHA still exists, because the path is still projected and the git index projection can return the correct SHA for it. Read-time self-heal (WindowsFileSystemVirtualizer.GetFileStreamHandlerAsyncHandler): - Plumb virtualPath into the handler and, when the placeholder's decoded SHA is not a valid hex SHA, recover the authoritative SHA for virtualPath from GitIndexProjection.GetProjectedFileInfo and hydrate the blob from that instead of the corrupt content-id. - The recovery passes a null BlobSizesConnection: repair needs only the SHA, not the blob size, so size resolution (which can throw SizesUnavailableException) is skipped and a size-lookup fault cannot deny a SHA-only self-heal. - A successful hydration writes the whole file, which converts the placeholder into a full file on disk. The corrupt content-id is superseded and future reads never call back, so the file is repaired for good. - If the path is no longer projected (deleted/renamed), the projection lookup throws, or the recovered SHA cannot be hydrated, fall back to the same clean, non-crashing FileNotAvailable failure as microsoft#2074. We deliberately do NOT rewrite the placeholder's content-id in place via UpdateFileIfNeeded. Confirmed empirically against real inbox ProjFS with a throwaway probe: (1) serving the full content converts the placeholder to a full file, so a second read issues no GetFileData callback (hydration alone is the repair); and (2) UpdateFileIfNeeded on the file mid-read returns 0x80070020 (ERROR_SHARING_VIOLATION) because the reader holds the file open. The same probe showed a corrupt placeholder is only injectable from the owning virtualization instance (WritePlaceholderInfo accepts an all-NUL content-id) and that an external FSCTL_SET_REPARSE_POINT rewrite is blocked (ERROR 1359), so this behavior is covered by unit tests rather than a functional test. Telemetry funnel (paired with microsoft#2074's *_MalformedBlobSha detection): - Repaired: *_MalformedBlobShaRepaired (Warning) with the recovered SHA, so we can watch the corrupt-placeholder population drain. - Repair miss: *_MalformedBlobShaRepairFailed (Warning) tagged with a MalformedShaRepairFailureReason (ProjectionMiss / ProjectionException / HydrateFailed / HydrateException). The failed event is emitted on every repair-failure exit - including hydration failures that throw after a SHA is recovered (size mismatch, local IO, ProjFS write failure) - so that repaired + repair-failed accounts for every repair attempt. Coordinates with microsoft#2071: a repair miss stays telemetry category Unexpected; a successful repair simply succeeds. No new BlobHydrationFailureCategory value. Two known, accepted behaviors are documented in code: the projection is read live, so a concurrent checkout can change the projected SHA between placeholder open and repair (serving the currently-projected SHA is the best answer for an already-corrupt file and matches the placeholder-creation path); and an unrepairable-but-projected file whose blob is unavailable pays the normal download + retry budget per read (the same cost any valid-but-unavailable placeholder pays), bounded and never re-crashing. Stacked on microsoft#2074 (tyrielv/fix-invalid-sha-hydration), which is stacked on microsoft#2071. Targets vnext: this is a new behavioral change on the read path for an old, rare, pre-existing corruption, so it does not belong on the 2.0 stabilization line. microsoft#2074 already removes the crash and retry storm on master. Unit tests (WindowsFileSystemVirtualizerTests) cover: repair success (asserting hydration uses the RECOVERED SHA, not the corrupt one) emits *_MalformedBlobShaRepaired and completes Ok; a non-projected path, a throwing projection lookup, an unhydratable recovered SHA, and a hydration that throws after recovery each emit *_MalformedBlobShaRepairFailed with the expected reason and fail cleanly; mid-repair cancellation emits neither repair event; and a valid content-id still hydrates with no repair telemetry. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
The malformed-SHA guard in TryCopyBlobContentStream only covers the blob hydration path. Two other callers reach the object download directly, so a corrupt (NUL-byte) placeholder SHA from them still reached the network: - the git.exe read-object hook (RequestSource.NamedPipeMessage, via InProcessMount), and - the gitattributes GVFSVerb (RequestSource.GVFSVerb). On .NET Framework the local Path.Combine threw ArgumentException on such a value, so the download was never reached. On modern .NET (which 2.0 runs) Path.Combine no longer validates path characters, so the malformed SHA silently misses the local object store and is sent to the cache server. The Application Gateway rejects the malformed URL with HTTP 400, and GVFS then treats the 400 as an auth failure and erases a valid credential, producing a credential-prompt storm (ICM 850075166). Reject a malformed object SHA at the download chokepoint (GVFSGitObjects.TryDownloadAndSaveObject, next to the existing AllZeroSha guard) for every request source, before any request is built, and emit the same greppable *_MalformedBlobSha Warning as the other guards. Also correct the GetLooseBlobState comment: the ArgumentException it described is .NET-Framework-only, so validating (not relying on the throw) is what makes the guard correct on modern .NET. Unit test asserts a malformed SHA returns Error from TryDownloadAndSaveObject across FileStreamCallback / NamedPipeMessage / GVFSVerb, never reaches the network (download call count stays 0), and is logged. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
added a commit
to tyrielv/VFSForGit
that referenced
this pull request
Aug 13, 2026
When a user process reads a virtualized placeholder whose stored content-id is corrupt - 40 NUL bytes instead of a hex blob SHA - GVFS cannot hydrate it from the corrupt content-id. microsoft#2074 makes that read fail cleanly (no crash, no retry storm). This change goes one step further and repairs the underlying data so the file works again. Telemetry shows this corruption is durable and localized: the same 1-4 files per machine fail repeatedly over multiple days until something rewrites the placeholder (~61 machines / ~6.2K events over 30 days). It is old and version-agnostic (spans >=4 GVFS builds), not a 2.0 regression. The triggering processes are readers (git.exe, copilot.exe, Code.exe); the placeholder was already corrupt on disk. The authoritative path->SHA still exists, because the path is still projected and the git index projection can return the correct SHA for it. Read-time self-heal (WindowsFileSystemVirtualizer.GetFileStreamHandlerAsyncHandler): - Plumb virtualPath into the handler and, when the placeholder's decoded SHA is not a valid hex SHA, recover the authoritative SHA for virtualPath from GitIndexProjection.GetProjectedFileInfo and hydrate the blob from that instead of the corrupt content-id. - The recovery passes a null BlobSizesConnection: repair needs only the SHA, not the blob size, so size resolution (which can throw SizesUnavailableException) is skipped and a size-lookup fault cannot deny a SHA-only self-heal. - A successful hydration writes the whole file, which converts the placeholder into a full file on disk. The corrupt content-id is superseded and future reads never call back, so the file is repaired for good. - If the path is no longer projected (deleted/renamed), the projection lookup throws, or the recovered SHA cannot be hydrated, fall back to the same clean, non-crashing FileNotAvailable failure as microsoft#2074. We deliberately do NOT rewrite the placeholder's content-id in place via UpdateFileIfNeeded. Confirmed empirically against real inbox ProjFS with a throwaway probe: (1) serving the full content converts the placeholder to a full file, so a second read issues no GetFileData callback (hydration alone is the repair); and (2) UpdateFileIfNeeded on the file mid-read returns 0x80070020 (ERROR_SHARING_VIOLATION) because the reader holds the file open. The same probe showed a corrupt placeholder is only injectable from the owning virtualization instance (WritePlaceholderInfo accepts an all-NUL content-id) and that an external FSCTL_SET_REPARSE_POINT rewrite is blocked (ERROR 1359), so this behavior is covered by unit tests rather than a functional test. Telemetry funnel (paired with microsoft#2074's *_MalformedBlobSha detection): - Repaired: *_MalformedBlobShaRepaired (Warning) with the recovered SHA, so we can watch the corrupt-placeholder population drain. - Repair miss: *_MalformedBlobShaRepairFailed (Warning) tagged with a MalformedShaRepairFailureReason (ProjectionMiss / ProjectionException / HydrateFailed / HydrateException). The failed event is emitted on every repair-failure exit - including hydration failures that throw after a SHA is recovered (size mismatch, local IO, ProjFS write failure) - so that repaired + repair-failed accounts for every repair attempt. Coordinates with microsoft#2071: a repair miss stays telemetry category Unexpected; a successful repair simply succeeds. No new BlobHydrationFailureCategory value. Two known, accepted behaviors are documented in code: the projection is read live, so a concurrent checkout can change the projected SHA between placeholder open and repair (serving the currently-projected SHA is the best answer for an already-corrupt file and matches the placeholder-creation path); and an unrepairable-but-projected file whose blob is unavailable pays the normal download + retry budget per read (the same cost any valid-but-unavailable placeholder pays), bounded and never re-crashing. Stacked on microsoft#2074 (tyrielv/fix-invalid-sha-hydration), which is stacked on microsoft#2071. Targets vnext: this is a new behavioral change on the read path for an old, rare, pre-existing corruption, so it does not belong on the 2.0 stabilization line. microsoft#2074 already removes the crash and retry storm on master. Unit tests (WindowsFileSystemVirtualizerTests) cover: repair success (asserting hydration uses the RECOVERED SHA, not the corrupt one) emits *_MalformedBlobShaRepaired and completes Ok; a non-projected path, a throwing projection lookup, an unhydratable recovered SHA, and a hydration that throws after recovery each emit *_MalformedBlobShaRepairFailed with the expected reason and fail cleanly; mid-repair cancellation emits neither repair event; and a valid content-id still hydrates with no repair telemetry. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
enabled auto-merge
August 13, 2026 18:35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a user process reads a virtualized placeholder file whose stored content-id (SHA) is corrupt — specifically 40 NUL bytes (
\u0000× 40) — GVFS builds a loose-object path from it withPath.Combine. What happens next depends on the .NET runtime, and the two shipped GVFS lines behave differently:On 1.0 (.NET Framework) — an unhandled
ArgumentExceptionand a retry stormPath.Combinevalidates path characters and throwsSystem.ArgumentException: Illegal characters in path:ArgumentExceptionis not inRetryWrapper.IsHandlableException(onlyHttpRequestException,IOException,RetryableException), so it bypasses both the retry logic and the download fallback inGVFSGitObjects.TryCopyBlobContentStreamand propagates to the virtualizer's outercatch (Exception), which returnsFileNotAvailableto ProjFS. The placeholder can never hydrate, so the failing read repeats indefinitely — a retry storm. This is the failure mode captured in field telemetry from the LKG build 1.0.26014.1 (see below).On 2.0 (.NET 10) — a bad request to the cache server, and an auth-popup storm
Modern .NET removed the path-character validation from
Path.Combine, so it no longer throws. The bogus path simply misses on disk (File.Existsreturns false), the read reports a clean local miss, and the request falls through to the object download — a URL built from the 40 NUL bytes is sent to the cache server. The Azure Application Gateway rejects the malformed URL with HTTP 400 before any cache server sees it (emptyCacheName).That 400 then collides with a separate GVFS bug:
HttpRequestortreats a 400 as an authentication failure and erases an approved credential. The nextgit credential getfinds no credential and GCM shows a WAM account prompt; under a VS Code launch swarm many fire at once — an auth-popup storm (observed in ICM 850075166 on GVFSv2.0.26222.1). The credential-erase half is fixed separately in #2088 ("HttpRequestor: do not reject credentials on HTTP 400"); this PR stops the malformed request that provokes it.Takeaway: the corrupt placeholder is the same on both lines; only the .NET runtime differs. The guard must therefore proactively validate the SHA (which works on both runtimes) rather than rely on
Path.Combinethrowing.Not
AllZeroShaThis is not
GVFSConstants.AllZeroSha.AllZeroShais 40 ASCII'0'characters, which yields directory"00"and is a valid hex string (it neither throws nor is rejected here). The failing SHA is literally 40 NUL bytes — a corrupt placeholder content-id.Where the malformed SHA comes from
A placeholder's content-id is the 40-char SHA encoded as UTF-16 (80 bytes) and stored in the ProjFS placeholder reparse point (
FileSystemVirtualizer.ConvertShaToContentId=Encoding.Unicode.GetBytes(sha)). On read,GetShaFromContentIddecodes those 80 bytes withEncoding.Unicode.GetString. If that on-disk content-id region is zeroed — a corrupt placeholder from an interrupted/partial placeholder write, a crash/power-loss during placeholder creation, or disk corruption — the decode yields 40\u0000characters. (What writes the corrupt content-id is a separate root-cause investigation; this PR hardens the read/download side.)Telemetry impact
Signature (1.0 retry-storm mode):
VFS.Error,ErrorMessagecontainsTryCopyBlobContentStream,ExceptioncontainsIllegal characters in pathandGetLooseBlobState.On 2.0 the same corruption surfaces instead as the HTTP-400 / auth-popup mode described above (ICM 850075166).
Fix
A minimal, surgical behavior fix (graceful-fail, defense-in-depth). Reject a malformed SHA — proactively, before it is turned into a filesystem path or a server request — at every relevant chokepoint:
GitRepo.GetLooseBlobStatereturnsLooseBlobState.Invalid(a clean, non-retryable miss) when the SHA is not 40 hex characters, soPath.Combinecan never throw (1.0) or silently mis-path (2.0) here again.GitRepo.LooseObjectExistsguards the samePath.Combine.GVFSGitObjects.TryCopyBlobContentStreamshort-circuits a malformed SHA before the retry loop, so a bogus SHA never triggers a doomed download or a retry storm on the hydration path.GVFSGitObjects.TryDownloadAndSaveObject(second commit) rejects a malformed object SHA at the download chokepoint — beside the existingAllZeroShaguard — for every request source, before any request URL is built. The hydration guard above only coversTryCopyBlobContentStream; two callers reach the download directly and bypass it: the git.exe read-object hook (RequestSource.NamedPipeMessage, viaInProcessMount) and the gitattributesGVFSVerb(RequestSource.GVFSVerb). This is what closes the 2.0 HTTP-400 path for all callers.SHA1Util.IsValidShaFormatis now null-safe;SHA1Util.ToLoggableShaStringrenders the bad value with non-hex characters escaped (\uXXXX) so telemetry stays greppable and free of control characters.WindowsFileSystemVirtualizer.GetFileDataCallbackroutes the request's loggedshathroughToLoggableShaString, so the terminal hydration-failureRelatedError(and every sink using that request metadata) can no longer record raw NUL/control bytes. No-op for a valid hex SHA.All guard sites emit the same greppable Warning event (
*_MalformedBlobSha, with the offending value under theshametadata key) at Warning level, with no unhandled exception (an expected, handled corrupt-placeholder read is a Warning, not an Error). TheGetLooseBlobStatecomment is corrected to document the two-runtime behavior (theArgumentExceptionis .NET-Framework-only), so a future reader understands why validating — not catching — is what makes the guard correct on modern .NET.Telemetry category stays "Unexpected"
Per an existing decision, a malformed SHA gets no dedicated telemetry category. This branch was developed on top of the (now-merged) blob-hydration telemetry work (#2071), which added
out BlobHydrationFailureCategorytoTryCopyBlobContentStream; the malformed-SHA short-circuit setsfailureCategory = Unexpected. The case previously reached the virtualizer's outercatchand was taggedUnexpectedthere; now that it no longer throws, it returns cleanly and the virtualizer tags its terminal telemetryUnexpectedvia the out-param — same bucket, no double-logging, no new category. The paired*_MalformedBlobShawarning lets an analyst separate this deterministic case from genuinely-unexpected exceptions.Relationship to other PRs
out BlobHydrationFailureCategory. This branch was developed on top of it and has since been rebased ontomaster, so the diff below is just this fix.HttpRequestor: do not reject credentials on HTTP 400): fixes the credential-erase half of the ICM 850075166 auth-popup storm. Independent of this PR — that change stops the credential erase; this change stops the malformed 400 request that triggers it. Both are wanted.Not in scope (follow-up)
detected → repaired / repair-failedtelemetry funnel — is a separate follow-up PR (Repair a corrupt (NUL-byte) placeholder SHA at read time #2081) targetingvnext(2.1): it is new, destabilizing behavior for an old, non-2.0-regressing problem, so it does not belong in the 2.0 stabilization line. This PR's graceful-fail guards remain the fallback for the repair-miss case.Tests
GVFSGitObjectsTests.TryCopyBlobContentStreamFailsCleanlyForCorruptAllNullByteSha/...ForOtherMalformedShas: a 40-NUL-byte SHA, a 40-char SHA with an embedded path-illegal character, and other malformed SHAs returnfalsefrom bothGitRepo.TryCopyBlobContentStreamandGVFSGitObjects.TryCopyBlobContentStreamwith noArgumentException(Assert.DoesNotThrow), no download/retry is attempted, the out category isUnexpected, and the*_MalformedBlobShadiagnostic events are actually emitted (verified via a smallMockTracerenhancement that recordsRelatedEventnames).GVFSGitObjectsTests.TryDownloadAndSaveObjectDoesNotSendMalformedShaToServer: a malformed SHA returnsErroracrossFileStreamCallback/NamedPipeMessage/GVFSVerb, never reaches the network (the mock's download call count stays 0), and is logged — the regression test for the 2.0 HTTP-400 mode.SHA1UtilTests: null-safety andToLoggableShaStringescaping (mixed valid/NUL, control chars, high code points).Review
Reviewed with a multi-lens self-review (correctness / security / design / tests / async / risk-rollout) over two iterations; feedback applied (consistent
*_MalformedBlobShaevent names andshametadata key across all guards; escaped telemetry at the single virtualizer chokepoint; explicit no-throw / no-download / emission assertions). No outstanding code findings; the only rollout note is to re-point any dashboard/alert keyed on the oldIllegal characters in pathexception signature at the new*_MalformedBlobShaevents / theUnexpectedcategory tag (the top-level hydration-failureRelatedErrorstill fires, so affected machines remain visible).