Add .cab and ReconnectModal.razor.js to signing config#35026
Add .cab and ReconnectModal.razor.js to signing config#35026kubaflo merged 3 commits intodotnet:inflight/currentfrom
Conversation
The VS signing scan flags 75 unsigned cab1.cab.cab files inside MAUI workload pack MSIs (69 maui* + 6 aspnetcorewebviewmaui* payloads) and 3 unsigned ReconnectModal.razor.js files in mauitemplatesnet10 payloads. Add FileExtensionSignInfo for .cab and FileSignInfo for the Blazor reconnect script so the Arcade signing tool signs them during CI. Since UseDotNetCertificate is already true, Arcade will automatically use MicrosoftDotNet500 instead of Microsoft400.
|
🚀 Dogfood this PR with:
curl -fsSL https://github.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35026Or
iex "& { $(irm https://github.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35026" |
|
Hey there @@jesuszarate! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
There was a problem hiding this comment.
Pull request overview
Updates the repository’s signing configuration to address Visual Studio signing scan warnings for unsigned files included in MAUI workload pack MSIs.
Changes:
- Adds a signing rule for
ReconnectModal.razor.jsusing theMicrosoft400certificate. - Adds an extension-based signing rule for
.cabfiles (to cover embedded cab files inside workload MSIs). - Trims trailing whitespace at the end of
eng/Signing.props.
Address Copilot review: ReconnectModal.razor.js uses Microsoft400, not a 3rd-party cert, so it shouldn't be in the Third Party Assemblies group.
|
/review |
|
✅ Expert Code Review completed successfully! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Expert Code Review — PR #35026
Summary
Clean, additive signing configuration that addresses a real compliance gap. All 3 independent reviewers returned LGTM.
Findings
| # | Severity | File | Finding | Consensus |
|---|---|---|---|---|
| 1 | 🟢 MINOR | eng/Signing.props:32 |
Missing Label attribute on .cab ItemGroup — inconsistent with other labeled groups |
2/3 reviewers |
| 2 | 🟢 MINOR | eng/Signing.props:34 |
FileExtensionSignInfo for .cab is a broad catch-all — latent risk if third-party .cab files are ever introduced |
2/3 reviewers |
Discarded (single reviewer only): Document Microsoft400 → MicrosoftDotNet500 cert substitution — 1/3 reviewers flagged, lowest severity, discarded per consensus rules.
Assessment
- Certificate choices are correct:
Microsoft400withUseDotNetCertificate=truetriggers Arcade's automatic remap toMicrosoftDotNet500at sign time. - ReconnectModal.razor.js is correctly placed in its own "Microsoft files"
ItemGroup— it's first-party Blazor code, not a third-party dependency. - Trailing whitespace fix on
</Project>is a no-op for MSBuild. - No existing signing entries are modified — this is purely additive.
- CI status: Could not be queried due to integrity filtering; recommend verifying CI passes before merge.
- Test coverage: N/A — build/signing configuration changes; no tests expected.
Verdict: LGTM
Confidence: High
Methodology: 3 independent reviewers with adversarial consensus
Note
🔒 Integrity filter blocked 2 items
The following items were blocked because they don't meet the GitHub integrity level.
- #35026
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved". - #35026
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
To allow these resources, lower min-integrity in your GitHub frontmatter:
tools:
github:
min-integrity: approved # merged | approved | unapproved | noneGenerated by Expert Code Review for issue #35026 · ● 7.4M
| <ItemGroup> | ||
| <!-- Sign cab files embedded inside MSI workload packs --> | ||
| <FileExtensionSignInfo Include=".cab" CertificateName="Microsoft400" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
🟢 MINOR (2/3 reviewers) — Consider adding a Label attribute to this ItemGroup for consistency.
The "Third Party Assemblies" group (line 6) and the new "Microsoft files" group (line 28) both carry Label attributes, but this ItemGroup has none. A label like Label="MSI workload cab files" would improve scanability and maintain the file's existing convention.
Non-blocking — the XML comment on line 33 partially serves the same purpose.
|
|
||
| <ItemGroup> | ||
| <!-- Sign cab files embedded inside MSI workload packs --> | ||
| <FileExtensionSignInfo Include=".cab" CertificateName="Microsoft400" /> |
There was a problem hiding this comment.
🟢 MINOR (2/3 reviewers) — FileExtensionSignInfo matches all .cab files in the signing scope, not just the specific cab files in MAUI workload MSIs.
If a third-party .cab were ever introduced into the artifact tree, it would be silently signed with a Microsoft certificate. Currently MAUI ships no third-party .cab files, so this is not an active defect — just a latent risk worth being aware of.
Non-blocking — all reviewers agreed this is acceptable for MAUI's current build pipeline.
Address Expert Code Review feedback: add Label='MSI workload cab signing' to the FileExtensionSignInfo ItemGroup, matching the convention used by the other labeled groups in the file.
🤖 AI Summary
📊 Review Session —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35026 | Add FileSignInfo for ReconnectModal.razor.js + FileExtensionSignInfo for .cab with Microsoft400 cert |
eng/Signing.props |
Original PR — signing config only |
🔬 Code Review — Deep Analysis
Code Review — PR #35026
Independent Assessment
What this changes: Adds two new signing rules to eng/Signing.props:
- A
FileSignInfoentry forReconnectModal.razor.jsusing theMicrosoft400certificate. - A
FileExtensionSignInfoentry for all.cabfiles usingMicrosoft400.
Also removes trailing whitespace from the closing</Project>tag.
Inferred motivation: Files in the shipping artifacts are failing a signing scan — some .cab archives embedded inside workload MSIs and the Blazor reconnect modal JS file are reaching distribution unsigned. This is a compliance gap, not a functional bug.
Is the approach sound? Yes. The pattern is consistent with the rest of Signing.props. Microsoft400 is the conventional certificate name for Microsoft-first-party files; with UseDotNetCertificate=true in the <PropertyGroup>, Arcade automatically substitutes MicrosoftDotNet500 at actual sign time. Both groupings have descriptive Label attributes. The change is purely additive — no existing entries are modified.
Reconciliation with PR Narrative
Author claims: Visual Studio signing scan flags 75 unsigned .cab files (workload pack MSIs) and 3 unsigned ReconnectModal.razor.js files (mauitemplatesnet10 payloads). The PR adds the minimum config to bring these into compliance.
Agreement/disagreement: Full agreement. The code matches the description exactly. No discrepancy between narrative and implementation.
Note on prior review finding: The previous automated review flagged "Missing
Labelattribute on.cabItemGroup" — this is incorrect. The<ItemGroup Label="MSI workload cab signing">label is present in the final file at line 32. No action needed.
Findings
💡 Suggestion — .cab extension rule is a repo-wide catch-all
eng/Signing.props:34
FileExtensionSignInfo for .cab applies to every .cab file that enters the signing scope — now and in the future. Currently, only Microsoft-owned workload pack cabinet archives exist, so Microsoft400 is correct for all of them. However, if a third-party tool or dependency ever introduces a .cab file into the artifact tree, it would be inadvertently signed with a Microsoft certificate rather than the 3PartySHA2 cert used for other third-party binaries.
The inline comment (<!-- Sign cab files embedded inside MSI workload packs -->) documents intent, which is helpful. This is not a current bug, just a latent maintenance risk. If signing scope creep is a concern in this repo, a more targeted approach (e.g., explicit filename patterns like cab1.cab) would be more defensive.
Not a blocker. The current artifact inventory contains only Microsoft-owned .cab files, so the certificate assignment is correct today.
Devil's Advocate
Challenging the suggestion: Could .cab files from third parties already be in scope? Searching the repo for third-party .cab references yields nothing — this risk is theoretical. The Arcade signing infrastructure is also guarded by other processes (e.g., ItemsToSign inclusion lists) before the extension rule even fires. The suggestion is valid but low-priority.
Challenging approval: Is Microsoft400 → MicrosoftDotNet500 substitution guaranteed? Yes — it's an Arcade invariant when UseDotNetCertificate=true. Is ReconnectModal.razor.js actually first-party Microsoft content? Yes — it ships from Microsoft.AspNetCore.Components.Web (ASP.NET Core, owned by Microsoft). Is anything broken by the trailing-whitespace fix? No — MSBuild ignores it.
CI status: license/cla ✅ pass, add-dogfood-comment ✅ pass, Build Analysis pending (Arcade infra, not related to this change). No required checks are failing.
Verdict: LGTM
Confidence: High
Summary: Clean, additive signing configuration that addresses a real compliance gap. Certificate choices are correct, groupings are semantically appropriate, and no existing behavior is altered. The only finding is a minor latent-risk suggestion about .cab extension breadth, which is not a current problem. Ready for human approval.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | FileSignInfo with PackageName per workload package (scoped extension signing) | eng/Signing.props |
Uncertain: PackageName may not work for files in nested containers | |
| 2 | try-fix | Same signing rules as PR + rich safety comments + wixpack path fix + removed stale SignVerifyIgnore.txt entry | eng/Signing.props, eng/automation/SignVerifyIgnore.txt |
Best documentation; fixes pre-existing path bug | |
| 3 | try-fix | Unified two ItemGroups into one + wixpack path fix + removed stale ignore | eng/Signing.props, eng/automation/SignVerifyIgnore.txt |
Cleaner structure; functionally same as PR | |
| 4 | try-fix | Same PR structure + added ItemsToSignPostBuild for wixpack.zip + wixpack path fix + removed stale ignore |
eng/Signing.props, eng/automation/SignVerifyIgnore.txt |
Addresses PostBuildSign=true coverage gap; disputed (macios omits this) | |
| 5 | try-fix | FileSignInfo Include="cab1.cab.cab" (specific filename) instead of global extension rule + PostBuild for wixpack + path fix |
eng/Signing.props, eng/automation/SignVerifyIgnore.txt |
More specific but risks fragility if WiX naming changes | |
| PR | PR #35026 | FileSignInfo for ReconnectModal.razor.js + FileExtensionSignInfo for .cab (both Microsoft400) |
eng/Signing.props |
Original PR — code review verdict: LGTM (high confidence) |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | No | Confirmed no new ideas; wixpack path fix and PostBuild gap already covered |
| gpt-5.3-codex | 2 | Yes | FileSignInfo with specific filename cab1.cab.cab instead of extension rule → run as Attempt 5 |
| claude-sonnet-4.6 | 2 | Yes | Reference dotnet/macios as canonical pattern; macios intentionally omits ItemsToSignPostBuild for wixpack |
| gpt-5.5 | 2 | No | Confirmed solution space exhausted; PR approach matches proven macios pattern |
Exhausted: Yes
Selected Fix: PR #35026 — The PR's core signing rules are correct per code review (LGTM, high confidence) and match the proven dotnet/macios pattern. All try-fix alternatives were Blocked due to the structural limitation that signing config can only be validated by the official Arcade sign pipeline in CI. The main candidates offering incremental improvements (fix wixpack path bug, remove stale ignore entry) are pre-existing issues not introduced by this PR. The PR should be approved as-is; the pre-existing fixes can be filed as separate follow-up issues if desired.
📋 Report — Final Recommendation
✅ Final Recommendation: APPROVE
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | 1 implementation file, 0 test files; no linked issue |
| Code Review | LGTM (high) | 0 errors, 0 warnings, 1 suggestion (non-blocking) |
| Gate | No tests detected in this PR (signing config only) | |
| Try-Fix | ✅ COMPLETE | 5 attempts, 0 passing (all BLOCKED — no test env for signing config) |
| Report | ✅ COMPLETE |
Code Review Impact on Try-Fix
The code review's single suggestion — that FileExtensionSignInfo for .cab is a repo-wide catch-all — directly informed Try-Fix exploration. Attempt 1 (claude-opus-4.6) specifically tried FileSignInfo with PackageName scoping to address this; Attempt 5 tried a specific filename FileSignInfo Include="cab1.cab.cab". Cross-pollination confirmed that the extension-based rule is the proven pattern used by dotnet/macios, and the current artifact tree contains only Microsoft-owned .cab files.
Two pre-existing bugs (not introduced by this PR) were discovered during Try-Fix:
- Missing backslash in wixpack.zip glob:
$(ArtifactsShippingPackagesDir)**\*.wixpack.zip(line 39) - Stale
**\cab*.cab.cabentry ineng/automation/SignVerifyIgnore.txt(will become misleading once cabs are signed)
Summary
PR #35026 is a correct, minimal signing configuration change that addresses a real compliance gap. The code review returned LGTM with high confidence. All try-fix alternatives were structurally Blocked — signing configuration is only validated by the official Arcade signing pipeline in CI, which is not accessible locally. The PR's core approach (global FileExtensionSignInfo for .cab + FileSignInfo for ReconnectModal.razor.js) matches the proven dotnet/macios signing pattern. No better alternative was found; the PR fix is selected.
Root Cause
The MAUI workload pack MSIs contain embedded .cab cabinet archives that were never added to the signing configuration. Similarly, ReconnectModal.razor.js (from Microsoft.AspNetCore.Components.Web, packed into mauitemplatesnet10 workload MSIs) was absent from signing config. Both caused Visual Studio signing scan failures.
Fix Quality
The fix is correct and well-organized:
Microsoft400is the appropriate certificate (auto-converted toMicrosoftDotNet500viaUseDotNetCertificate=true)FileSignInfoforReconnectModal.razor.jsis correctly separated into its own "Microsoft files" ItemGroup (after a prior review flagged it being in "Third Party Assemblies")FileExtensionSignInfofor.cabhas a descriptive label and a clarifying comment- The single non-blocking suggestion (breadth of extension rule) is documented via the inline comment in
expert-pr-evalcandidate - Pre-existing issues noted for follow-up: wixpack.zip path separator bug (line 39) and stale SignVerifyIgnore.txt cab entry should be addressed in a follow-up PR
Selected Fix: PR (original PR fix as submitted — no alternative outperforms it)
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
|
|
||
| <ItemGroup Label="MSI workload cab signing"> | ||
| <!-- Sign cab files embedded inside MSI workload packs --> | ||
| <FileExtensionSignInfo Include=".cab" CertificateName="Microsoft400" /> |
There was a problem hiding this comment.
[minor] Build & MSBuild — FileExtensionSignInfo for .cab is a repo-wide catch-all: it will sign any .cab file that enters the signing scope in the future, including potential third-party or toolchain cabs that should use 3PartySHA2 instead of Microsoft400. The inline comment documents intent well, but consider whether a filename-specific rule (or per-pack explicit listing) would be safer long-term. Not a blocker — the current signing scope contains only Microsoft-owned workload pack cabs.
MauiBot
added
s/agent-approved
The Visual Studio signing scan flags unsigned files inside MAUI workload pack MSIs. This PR adds signing configuration for two categories of unsigned files:
1. Unsigned
.cabfiles (75 files across 75 payloads)Every MAUI workload pack MSI contains an embedded
cab1.cab.cabcabinet archive that is currently unsigned. This affects 69maui*payloads and 6aspnetcorewebviewmaui*payloads across net9.0/net10.0 × arm64/x64/x86.Fix: Add
FileExtensionSignInfofor.cabwithMicrosoft400(which Arcade auto-converts toMicrosoftDotNet500sinceUseDotNetCertificateistrue).Affected payloads (cab):
mauicontrols{10020100200,90120901200}{arm64,x64,x86}mauicontrolsbuildtasks{10020100200,90120901200}{arm64,x64,x86}mauicontrolscompatibility90120901200{arm64,x64,x86}mauicontrolscore{10020100200,90120901200}{arm64,x64,x86}mauicontrolsxaml{10020100200,90120901200}{arm64,x64,x86}mauicore{10020100200,90120901200}{arm64,x64,x86}mauiessentials{10020100200,90120901200}{arm64,x64,x86}mauigraphics{10020100200,90120901200}{arm64,x64,x86}mauigraphicswindows{10020100200,90120901200}{arm64,x64,x86}mauiresizetizer{10020100200,90120901200}{arm64,x64,x86}mauisdknet{10,9}{10020100200,90120901200}{arm64,x64,x86}mauitemplatesnet{10,9}{10020100200,90120901200}{arm64,x64,x86}aspnetcorewebviewmaui{10020100200,90120901200}{arm64,x64,x86}2. Unsigned
ReconnectModal.razor.js(3 files)The Blazor reconnect modal script from
Microsoft.AspNetCore.Components.Webis packed intomauitemplatesnet10workload MSIs without a signature.Fix: Add
FileSignInfoforReconnectModal.razor.jswithMicrosoft400.Affected payloads (js):
mauitemplatesnet1010020100200{arm64,x64,x86}