Avoid races publishing immutable blobs

[missymessa]

Summary

• create non-overwritable Azure blobs atomically with If-None-Match: *
• when another publisher wins the create race, compare the existing blob and accept it only if the contents are identical
• preserve the existing error behavior for conflicting contents and overwrite-enabled feeds

Motivation

Roslyn Maestro promotions 3059731 and 3059734 published to .NET Core Tooling Dev and .NET 11.0.1xx SDK simultaneously. Both channels map blobs to dotnetbuilds/public, causing all 145 unique blob paths to be scheduled twice. The current ExistsAsync() followed by UploadAsync() is a time-of-check/time-of-use race: both publishers can observe a missing blob, then the losing upload receives BlobImmutableDueToLegalHold after the winning upload creates the legally held blob.

This also addresses the failure pattern previously tracked by #13176 without requiring repositories to avoid otherwise-valid channel combinations.

Testing

• AzureStorageAssetPublisherTests: passes for BlobAlreadyExists, BlobImmutableDueToLegalHold, and ConditionNotMet races, and rejects different existing content
• complete Release validation reaches an unrelated existing ARM64 failure in Microsoft.DotNet.XUnitV3Extensions.AlwaysFalseConditionalAssembly.Tests: missing generated apphost.exe

[Copilot]

Pull request overview

This PR updates AzureStorageAssetPublisher to make publishing “immutable” Azure blobs race-safe by using conditional creates (If-None-Match: *) and, when a concurrent publisher wins, validating that the existing blob’s content matches the local file (when configured to allow identical blobs).

Changes:

• Add BlobRequestConditions { IfNoneMatch = ETag.All } for non-overwrite uploads to make blob creation atomic.
• Treat specific “already exists / precondition failed / immutable” upload failures as a concurrent-create race and fall back to existing-blob handling.
• Add unit tests covering accepted/rejected concurrent-create scenarios based on content identity.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/Microsoft.DotNet.Build.Tasks.Feed/src/AzureStorageAssetPublisher.cs	Adds conditional upload + race handling for immutable blob publishing.
src/Microsoft.DotNet.Build.Tasks.Feed.Tests/AzureStorageAssetPublisherTests.cs	Adds tests validating identical-vs-different concurrent-create behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[missymessa]

@mmitche please take a look!

[Copilot]

🟡 Changes recommended

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/Microsoft.DotNet.Build.Tasks.Feed.Tests/AzureStorageAssetPublisherTests.cs:34

• This test computes the blob ContentHash using MD5.HashData, which can trigger weak-crypto/static-analysis findings (the production code already centralizes MD5 usage in AzureStorageExtensions.CalculateMD5 with an explicit suppression comment). Consider deriving the byte[] from AzureStorageExtensions.CalculateMD5 instead to avoid introducing a new direct MD5 usage in tests.

            {
                await File.WriteAllTextAsync(file, "asset contents");
                byte[] contentHash = MD5.HashData(await File.ReadAllBytesAsync(file));
                var blobClient = new Mock<BlobClient>();

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[mmitche]

@missymessa I think this might be a non-ideal fix. IIRC, the publisher is supposed to deduplicate the mappings so that if you publish to N channels, and both channels push assets to dotnetbuilds/public, then it would only try and push the asset once.

In other words, the ideal model is that the system finds the mapping of asset->output location based on in channels and their config info, then dedupes that so we only try to upload each unique combo once.

I would investigate and determine:

• Is deduplication supposed to happen?
• If so, why is it not working?

[missymessa]

@missymessa I think this might be a non-ideal fix. IIRC, the publisher is supposed to deduplicate the mappings so that if you publish to N channels, and both channels push assets to dotnetbuilds/public, then it would only try and push the asset once.

In other words, the ideal model is that the system finds the mapping of asset->output location based on in channels and their config info, then dedupes that so we only try to upload each unique combo once.

I would investigate and determine:

• Is deduplication supposed to happen?
• If so, why is it not working?

@JoeRobich FYI