Make GenerateMsixCert work without admin on subsequent runs

[akoeplinger]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

The Windows packaged test cert task in eng/devices/windows.cake previously opened LocalMachine\TrustedPeople with ReadWrite unconditionally, forcing every cake invocation to run elevated even when the cert already existed. It also stored the private key with MachineKeySet, which leaves the key in C:\ProgramData\Microsoft\Crypto\... where non-admin signtool can't read it — signtool then fails with No certificates were found that met all the given criteria even though the cert is visible in the store.

Changes

• Probe LocalMachine\TrustedPeople with ReadOnly first so detecting an existing cert needs no elevation.
• When creating a new cert, use UserKeySet | PersistKeySet so the user-owned signtool can use the private key for signing.
• Open LocalMachine\TrustedPeople ReadWrite only on the creation path, and translate CryptographicException into a clear first run needs admin message instead of the raw Access is denied.
• When the cert already exists in LocalMachine\TrustedPeople but not in CurrentUser\My (signing store), emit a warning explaining how to recover instead of silently failing to sign later.

Result

After this change a developer needs admin once (to create and install the cert), then can run the packaged-test cake task from a non-elevated shell.

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35663

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35663"

[akoeplinger]

I ran into this while working on #29905 and it allowed me to run the Windows packaged tests in a non-elevated shell.

[akoeplinger]

/azp run maui-pr-devicetests,maui-pr-uitests

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[kubaflo]

/review -b feature/enhanced-reviewer -p windows

[MauiBot]

Expert Review — 1 findings

See inline comments for details.

[kubaflo]

Could you please check the ai's suggestions?

[kubaflo]

/review rerun

[kubaflo]

Do you think maui-bot suggestions are helpful?

[akoeplinger]

@kubaflo yeah it raised some good points. Fixed.

[kubaflo]

/review -b feature/enhanced-reviewer -p windows

[kubaflo]

/review -b feature/enhanced-reviewer -p windows

[MauiBot]

AI Review Summary

@akoeplinger — new AI review results are available based on this last commit: ec58af6. To request a fresh review after new comments or commits, comment /review rerun.

Review Sessions — click to expand

Gate — Test Before & After Fix

Gate Result: ⚠️ SKIPPED

No tests were detected in this PR.

Recommendation: Add tests to verify the fix using the write-tests-agent.

---

Pre-Flight — Context & Validation

Issue: N/A - No linked issue detected
PR: #35663 - Make GenerateMsixCert work without admin on subsequent runs
Platforms Affected: Windows packaged test infrastructure
Files Changed: 1 implementation, 0 test

Key Findings

• eng\devices\windows.cake changes GenerateMsixCert to avoid opening LocalMachine\TrustedPeople read-write when an existing cert can be reused.
• The PR also changes new cert private-key persistence from machine-scoped to user-scoped so non-elevated signing can work after first setup.
• Gate was pre-run by the caller and skipped because no tests were detected in this PR.
• GitHub CLI is unauthenticated in this environment, so required CI status could not be verified through gh pr checks.

Code Review Summary

Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 0

Key code review findings:

• No concrete code correctness findings on added/modified lines.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #35663	Probe LocalMachine\TrustedPeople read-only, validate CurrentUser\My can sign, remove/recreate stale certs, and create new keys with `UserKeySet	PersistKeySet`.	⚠️ SKIPPED (Gate: no tests detected)	eng\devices\windows.cake

---

Code Review — Deep Analysis

Code Review — PR #35663

Independent Assessment

What this changes: Updates eng\devices\windows.cake packaged Windows device-test certificate handling to first probe LocalMachine\TrustedPeople read-only, reuse an existing cert only if the matching CurrentUser\My private key can actually sign, remove/recreate stale certs, and create new certs with UserKeySet | PersistKeySet.

Inferred motivation: Make subsequent packaged Windows test builds work from a non-elevated shell after the cert has been installed once, and avoid stale machine-key certs that are visible but unusable by non-admin signtool.

Reconciliation with PR Narrative

Author claims: The PR says the task avoids unconditional admin access, stores new private keys user-scoped, gives clearer first-run admin guidance, and recovers from stale/broken signing cert state.

Agreement/disagreement: The code matches the functional intent. One narrative detail appears stale: the description says a missing CurrentUser\My cert emits a warning, but current code attempts removal/recreation and throws a recovery exception if removal needs elevation. That is not a code correctness issue.

Prior Review Reconciliation

Prior ❌ Error Finding	Source	Status	Evidence
Existing machine-key certs are not migrated/repaired.	MauiBot review + inline major comment	✅ Fixed	Current code verifies signing with IsCurrentUserSigningCertUsable and removes/recreates stale entries at eng\devices\windows.cake:91-110.
ExportParameters(false) only reads public key and cannot detect inaccessible private keys.	MauiBot inline major comment	✅ Fixed	Current code exercises key.SignData(...) at eng\devices\windows.cake:202-210.
Recovery text says rerun elevated, but elevated rerun still reuses old cert.	MauiBot inline major comment	✅ Fixed	Current code removes stale certs before falling through to creation; if removal fails it asks for explicit manual removal.
Subject matching used substring matching.	MauiBot inline moderate comment	✅ Fixed	Current code uses exact c.Subject == expectedSubject at eng\devices\windows.cake:79-82.

Blast Radius Assessment

• Runs for all instances: No — only packaged Windows device-test builds via GenerateMsixCert.
• Startup impact: No app startup impact; build/test infrastructure only.
• Static/shared state: Limited script-level certificateThumbprint, consumed by buildOnly for packaged signing.

CI Status

• Required-check result: Could not run gh pr checks --required because GitHub CLI authentication is unavailable.
• Public fallback: GitHub public metadata was available, but required-check status remains undetermined without authentication.
• Classification: Undetermined.
• Action taken: Capped confidence to low and did not return LGTM per skill rules.

Findings

No concrete code correctness findings on added/modified lines.

Failure-Mode Probing

• Existing cert in LocalMachine\TrustedPeople but inaccessible machine-scoped private key: Detected by SignData; stale cert is removed/recreated or a clear manual-removal exception is thrown.
• Existing cert in LocalMachine\TrustedPeople but missing from CurrentUser\My: Treated unusable and repaired via the same removal/recreation path.
• Subsequent non-elevated run after successful same-user elevated setup: Reuses the existing thumbprint without opening LocalMachine read-write.
• First run without elevation: Fails early with a clearer message when adding to LocalMachine\TrustedPeople requires admin.

Verdict: NEEDS_DISCUSSION

Confidence: low
Summary: Code-only review found no actionable issues, and prior major findings appear addressed. Overall verdict is not LGTM only because required CI status could not be verified with unauthenticated gh.

---

Fix — Analysis & Comparison

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix	Key-file signing with public-only machine trust	❌ Fail	eng/devices/windows.cake	Avoided store-based signing but still opened LocalMachine\TrustedPeople ReadWrite unconditionally, so subsequent non-admin runs would still fail.
2	try-fix	CurrentUser-only signing and trust	⚠️ Blocked	eng/devices/windows.cake	Parsed and passed static checks, but depends on CurrentUser\TrustedPeople being sufficient for MSIX install trust; packaged runtime validation unavailable.
3	try-fix	User-owned signing cert with machine public trust	⚠️ Blocked	eng/devices/windows.cake	Best alternative: CurrentUser signing source plus conditional LocalMachine public trust. Parsed and passed static checks, but packaged runtime validation unavailable.
PR	PR #35663	Read-only LocalMachine probe, validate matching CurrentUser signing key, remove/recreate stale certs, create with UserKeySet.	⚠️ SKIPPED (Gate: no tests detected)	eng/devices/windows.cake	Original PR; code review found no concrete correctness issues.

Cross-Pollination

Model	Round	New Ideas?	Details
gpt-5.5 / maui-expert-reviewer	1	Yes	Suggested file-based PFX signing with public-only machine trust; failed because machine write remained unconditional.
gpt-5.5 / maui-expert-reviewer	2	Yes	Suggested CurrentUser-only trust and signing; blocked on whether per-user trust satisfies packaged install.
gpt-5.5 / maui-expert-reviewer	3	Yes	Suggested split signing/trust model: CurrentUser signing cert plus LocalMachine public trust; strongest alternative but still needs packaged validation.
gpt-5.5 / maui-expert-reviewer	4	No	Remaining meaningful variants collapse into either PR-style stale repair, CurrentUser-only trust, PFX key-file signing, or split signing/public trust.

Exhausted: Yes
Selected Fix: PR's fix — It is the only candidate with no identified code-review correctness issue and does not depend on unverified CurrentUser-only trust. Candidate #3 is a plausible simpler alternative but cannot be called demonstrably better without packaged Windows validation.

---

Report — Final Recommendation

Comparative Candidate Report - PR #35663

Candidates compared

Candidate	Result	Regression status	Assessment
pr	Winner	Gate skipped; no tests detected	Best available fix. It avoids repeated LocalMachine\TrustedPeople read-write opens, validates the CurrentUser\My signing key, repairs stale unusable cert state, preserves machine trust for MSIX install, and does not depend on unverified per-user trust behavior.
pr-plus-reviewer	Equivalent to pr	Gate skipped; no tests detected	Expert reviewer produced no actionable feedback, so this candidate has no code delta over pr.
try-fix-3	Blocked	Static checks passed; packaged runtime validation unavailable	Strongest alternative. It signs from CurrentUser\My and conditionally adds only the public cert to LocalMachine\TrustedPeople, but it does not remove stale same-subject machine-trust entries and remains unproven without elevated-first-run plus non-elevated-rerun packaged validation.
try-fix-2	Blocked	Static checks passed; packaged runtime validation unavailable	Simpler current-user-only approach, but it relies on CurrentUser\TrustedPeople being sufficient for MSIX install trust. That platform assumption was not validated.
try-fix-1	Failed	Failed static regression check	Ranked lower than all non-failing candidates because it still opens LocalMachine\TrustedPeople read-write unconditionally, preserving the repeated elevation/admin requirement.

Ranking

1. pr
2. pr-plus-reviewer
3. try-fix-3
4. try-fix-2
5. try-fix-1

No candidate has a full passing regression result because the gate was skipped and packaged Windows certificate-store validation was unavailable. Per the required rule, the failed try-fix-1 candidate is ranked below candidates that were skipped or blocked rather than demonstrably regressed.

Winning candidate

Winner: pr

The raw PR fix wins because it addresses the known regression without relying on the unverified CurrentUser\TrustedPeople install-trust assumption, and the expert reviewer found no actionable improvements to apply. try-fix-3 is a plausible simpler alternative, but it remains blocked on packaged runtime validation and has a residual stale-cert coexistence risk; try-fix-1 failed the core regression check.

Test recommendation

The PR should add or document validation for the intended Windows packaged flow: an elevated first run that installs trust, followed by a non-elevated rerun that reuses the cert and signs successfully.

---

Future Action — review latest findings

No alternative fix was selected for this run. Review the session findings and CI results before merging.