Fix SRI integrity failures in Blazor WASM incremental builds#52847
Fix SRI integrity failures in Blazor WASM incremental builds#52847lewing wants to merge 1 commit intodotnet:mainfrom
Conversation
During incremental builds, the compression task could read stale content from the destination file (wwwroot\_framework\dotnet.js) instead of the updated source file (obj\...\dotnet.js) because the file copy hadn't happened yet. This caused SRI integrity check failures when: 1. _GenerateBuildWasmBootJson creates obj\dotnet.js with NEW fingerprints 2. GenerateBuildCompressedStaticWebAssets compresses wwwroot\dotnet.js (OLD) 3. _BuildStaticWebAssetsPreserveNewest copies to wwwroot\dotnet.js (too late) Fix: When both RelatedAsset and RelatedAssetOriginalItemSpec exist and point to different files, compare timestamps and prefer the newer file. This ensures the compression task uses the freshly-generated source file during incremental builds, while still preferring RelatedAsset in the normal case after copying. Fixes: dotnet/aspnetcore#65271
|
Thanks for your PR, @@lewing. |
There was a problem hiding this comment.
Pull request overview
This PR fixes SRI (Subresource Integrity) integrity failures in Blazor WebAssembly incremental builds by ensuring the compression task uses the most recently updated asset file. The issue occurred when the compression task read stale content from the destination file because the file copy operation hadn't completed yet during incremental builds.
Changes:
- Added timestamp comparison logic to
AssetToCompress.TryFindInputFilePaththat prefers the newer file when bothRelatedAssetandRelatedAssetOriginalItemSpecexist and point to different files - Added comprehensive unit tests covering the bug scenario, normal post-copy scenario, same-file scenario, and existing edge cases
- Updated existing tests to explicitly set timestamps to ensure deterministic behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/StaticWebAssetsSdk/Tasks/Utils/AssetToCompress.cs | Added timestamp comparison logic to prefer the newer file when both source and destination exist, fixing the incremental build race condition |
| test/Microsoft.NET.Sdk.StaticWebAssets.Tests/StaticWebAssets/AssetToCompressTest.cs | Added three new tests for timestamp-based file selection and updated two existing tests to explicitly set file timestamps for deterministic behavior |
| var relatedAsset = assetToCompress.GetMetadata("RelatedAsset"); | ||
| var relatedAssetOriginalItemSpec = assetToCompress.GetMetadata("RelatedAssetOriginalItemSpec"); | ||
|
|
||
| var relatedAssetExists = File.Exists(relatedAsset); | ||
| var originalItemSpecExists = File.Exists(relatedAssetOriginalItemSpec); | ||
|
|
||
| // When both paths exist and point to different files, prefer the newer one. | ||
| // This handles incremental builds where the source file (OriginalItemSpec) may be | ||
| // newer than the destination (RelatedAsset), which hasn't been copied yet. | ||
| if (relatedAssetExists && originalItemSpecExists && | ||
| !string.Equals(relatedAsset, relatedAssetOriginalItemSpec, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| var relatedAssetTime = File.GetLastWriteTimeUtc(relatedAsset); | ||
| var originalItemSpecTime = File.GetLastWriteTimeUtc(relatedAssetOriginalItemSpec); | ||
|
|
||
| if (originalItemSpecTime > relatedAssetTime) | ||
| { | ||
| log.LogMessage(MessageImportance.Low, "Asset '{0}' using original item spec '{1}' because it is newer than '{2}'.", | ||
| assetToCompress.ItemSpec, | ||
| relatedAssetOriginalItemSpec, | ||
| relatedAsset); | ||
| fullPath = relatedAssetOriginalItemSpec; | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Ugh, no. This task shouldn't worry about incrementalism.
This is an artifact of the magic that we do on webassembly where we define the asset pointing to a non-existing place. We should do that as opposed to checking for newer files here.
this is ~4 file accesses per compression check that will get very costly soon. DefineStaticWebAssets is the only task that is "allowed" to read the file from disk to ensure we only do it once per file. Other than when we need to actually process the file.
This now points out to a bigger problem (surprised that we haven't seen it before) where the outputs from the build are considered inputs to the current one.
We need to do this on the wasm sdk, otherwise doing it here will destroy perf. One way to do this would be to delete the file at the beginning of the build when it changes, but better yet would be to stop defining these assets with an item spec in the wwwroot folder and just define them in their original location on disk.
There was a problem hiding this comment.
yeah, I was certain this would annoy you, I think in the common cases it only adds ~one more file exists call (both calls were already in the code the second was just short circuited) but I confess I'm not that familiar with exactly what amounts to common for these cases. The regression is unfortunate so whatever we do lets do it quickly.
|
closed in favor of dotnet/runtime#124125 |
## Summary Fix a **.NET 11 regression** causing SRI integrity failures during incremental Blazor WASM builds. Changes in `Microsoft.NET.Sdk.WebAssembly.Browser.targets`: 1. **Boot config ContentRoot**: Change the boot config's `DefineStaticWebAssets` `ContentRoot` from `$(OutDir)wwwroot` to `$(IntermediateOutputPath)` 2. **Preload matching**: Replace fragile `%(FileName)%(Extension)`-based scanning with direct references to the boot config output items 3. **WebCil ContentRoot (build-time)**: Use per-item `ContentRoot="%(RootDir)%(Directory)"` on `_WebCilAssetsCandidates` so each asset's Identity resolves to its actual file path on disk 4. **WebCil ContentRoot (publish-time)**: Add per-item `ContentRoot="%(RootDir)%(Directory)"` on both `_NewWebCilPublishStaticWebAssetsCandidatesNoMetadata` and `_PromotedWasmPublishStaticWebAssets` — the same fix applied to publish candidates ## Regression This is a regression in .NET 11 (works in 10.0). It was introduced by [dotnet/sdk#52283](dotnet/sdk#52283), which fixed an esproj compression bug by flipping the order in `AssetToCompress.TryFindInputFilePath` to prefer `RelatedAsset` (Identity) over `RelatedAssetOriginalItemSpec`. That fix was correct for esproj, but exposed a latent issue in the WASM SDK targets: the boot config and webcil assets' Identity pointed to a `wwwroot` copy rather than the actual source files. Before sdk#52283, `OriginalItemSpec` happened to point to the real file and was checked first, masking the wrong `ContentRoot`. After the flip, `RelatedAsset` (Identity) is checked first, and its stale `wwwroot` path is used — producing incorrect SRI hashes on incremental builds. Reported in [aspnetcore#65271](dotnet/aspnetcore#65271). ## Problem The WASM boot config file (e.g. `dotnet.boot.js`) is generated at `$(IntermediateOutputPath)` (the `obj/` folder), but its static web asset was defined with `ContentRoot="$(OutDir)wwwroot"`. This caused `DefineStaticWebAssets` to compute an Identity pointing to the `wwwroot` copy rather than the actual file in `obj/`. The same issue applied to WebCil asset candidates — files from `obj/webcil/`, the runtime pack, and other directories were all defined with `ContentRoot="$(OutputPath)wwwroot"`, producing synthetic Identities under `wwwroot/` that could become stale during incremental builds. ## Fix ### 1. Boot config ContentRoot Change `ContentRoot` to `$(IntermediateOutputPath)` so the asset Identity matches the real file location on disk. The `CopyToOutputDirectory="PreserveNewest"` attribute still ensures the file is copied to `wwwroot` for serving. This follows Javier's suggestion in dotnet/sdk#52847 to "stop defining these assets with an item spec in the wwwroot folder and just define them in their original location on disk". ### 2. Preload matching simplification The `_AddWasmPreloadBuildProperties` and `_AddWasmPreloadPublishProperties` targets previously scanned all `@(StaticWebAsset)` items by `%(FileName)%(Extension)` to find the boot config asset. This relied on the Identity path containing the fingerprint in the filename, which is an implementation detail of how `DefineStaticWebAssets` computes Identity based on `ContentRoot`. The fix replaces the scanning with direct references to `@(_WasmBuildBootConfigStaticWebAsset)` and `@(_WasmPublishBootConfigStaticWebAsset)` — the output items already produced by `DefineStaticWebAssets`. This is both correct and simpler. ### 3. WebCil per-item ContentRoot (build-time) Instead of using a single task-level `ContentRoot` parameter (which forces all candidates through the same ContentRoot path, causing synthesized Identity for files outside that directory), set per-item `ContentRoot="%(RootDir)%(Directory)"` on each `_WebCilAssetsCandidates` item. This means: - **WebCil-converted files** in `obj/webcil/` → ContentRoot = their parent dir → `FullPath.StartsWith(ContentRoot)` = true → Identity = real FullPath ✅ - **Runtime pack files** (e.g. `dotnet.native.js`, ICU `.dat`) → ContentRoot = their parent dir in the runtime pack → `FullPath.StartsWith(ContentRoot)` = true → Identity = real FullPath ✅ No synthesized paths, no CopyCandidate entries — every asset's Identity is its actual file on disk. ### 4. WebCil per-item ContentRoot (publish-time) The publish `DefineStaticWebAssets` call in `ProcessPublishFilesForWasm` previously had **no ContentRoot at all** — neither task-level nor per-item. This caused publish candidates (especially promoted build assets with fingerprint placeholders in their RelativePath) to have their fingerprinted filename baked into the item spec as Identity, producing paths like `dotnet.native.7z98fd2ohl.wasm` that don't exist on disk → MSB3030 "Could not copy file". The fix adds per-item `ContentRoot="%(RootDir)%(Directory)"` on both: - `_NewWebCilPublishStaticWebAssetsCandidatesNoMetadata` (freshly WebCil-converted publish files) - `_PromotedWasmPublishStaticWebAssets` (build assets promoted to publish) **Why this works**: Promoted assets carry `AssetKind=Build` from the build-time `DefineStaticWebAssets`. In `DefineStaticWebAssets.cs` line 252: `IsPublish("Build") = false`, so contentRoot is NOT nulled for publish. The per-item ContentRoot = each file's parent directory → `candidateFullPath.StartsWith(contentRoot)` = true → `computed=false` → Identity = real FullPath on disk. ## What's not changed - **Publish boot config ContentRoot** (`ContentRoot="$(PublishDir)wwwroot"`): Publish builds are clean and don't have the incremental staleness problem. ## Build Progression | # | Approach | Build Result | Why | |---|----------|-------------|-----| | 1-3 | Boot config + preload only | ✅ Pass | No WebCil changes | | 4-5 | Per-item ContentRoot (build only) | ❌ Fail | Build works, but publish has no ContentRoot → MSB3030 | | 6 | Task-level IntermediateOutputPath | ❌ Fail | Runtime pack files outside IntermediateOutputPath → synthesized Identity → MSB3030 | | 7 | **Per-item ContentRoot (build + publish)** | 🔄 Pending | This commit — applies the fix to both build and publish | Fixes dotnet/aspnetcore#65271 --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Marek Fišera <[email protected]> Co-authored-by: Copilot <[email protected]>
Summary
Fixes dotnet/aspnetcore#65271
During incremental builds, the compression task could read stale content from the destination file (\wwwroot_framework\dotnet.js) instead of the updated source file (\obj...\dotnet.js) because the file copy hadn't happened yet.
This caused SRI integrity check failures when:
Regression
This was caused by #52283 which changed the order to prefer \RelatedAsset\ over \RelatedAssetOriginalItemSpec. While that fixed the esproj scenario, it broke incremental builds where the source file is newer than the stale destination.
Fix
When both \RelatedAsset\ and \RelatedAssetOriginalItemSpec\ exist and point to different files, compare timestamps and prefer the newer file. This ensures the compression task uses the freshly-generated source file during incremental builds, while still preferring \RelatedAsset\ in the normal case after copying.
Testing