Skip to content

Fix WindowsCertificateManager to correctly detect user cancellation of trust dialog#66604

Open
Copilot wants to merge 3 commits intomainfrom
copilot/fix-cancellation-reporting
Open

Fix WindowsCertificateManager to correctly detect user cancellation of trust dialog#66604
Copilot wants to merge 3 commits intomainfrom
copilot/fix-cancellation-reporting

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 6, 2026

WindowsCertificateManager.TrustCertificateCore was comparing exception.HResult against the raw Win32 error code (ERROR_CANCELLED = 1223 = 0x4C7) instead of its HRESULT-encoded form. Since HResult uses HRESULT_FROM_WIN32 encoding (0x80070000 | win32code), the check never matched, causing user cancellations to surface as unhandled exceptions rather than being caught and converted to UserCancelledTrustException.

Description

  • Changed UserCancelledErrorCode from 1223 to unchecked((int)0x800704C7) — the HRESULT encoding of ERROR_CANCELLED
// Before: never matched exception.HResult
private const int UserCancelledErrorCode = 1223;

// After: correctly matches HRESULT_FROM_WIN32(ERROR_CANCELLED)
private const int UserCancelledErrorCode = unchecked((int)0x800704C7);

Note: Exception.HResult always returns the HRESULT-encoded value (0x8007xxxx for Win32 errors), never the raw Win32 error code. The original value 1223 was therefore dead code — it could never match exception.HResult. Replacing it with unchecked((int)0x800704C7) is the correct fix; retaining 1223 alongside the new value would leave unreachable dead code.

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Copilot AI changed the title [WIP] Fix cancellation reporting in WindowsCertificateManager Fix WindowsCertificateManager to correctly detect user cancellation of trust dialog May 6, 2026
Copilot AI requested a review from Youssef1313 May 6, 2026 17:14
@github-actions github-actions Bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label May 6, 2026
Comment thread src/Shared/CertificateGeneration/WindowsCertificateManager.cs Outdated
@Youssef1313 Youssef1313 marked this pull request as ready for review May 7, 2026 08:21
Copilot AI review requested due to automatic review settings May 7, 2026 08:21
@Youssef1313 Youssef1313 requested a review from BrennanConroy May 7, 2026 08:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Windows certificate trust handling to correctly recognize when a user cancels the Windows trust prompt, so the cancellation can be translated into UserCancelledTrustException instead of surfacing as an unhandled CryptographicException.

Changes:

  • Updates the CryptographicException filter in WindowsCertificateManager.TrustCertificateCore to detect the cancellation code based on the exception HRESULT’s encoded structure.
  • Adds inline commentary explaining how the HRESULT is interpreted for this check.

Comment on lines +94 to +96
// https://msdn.microsoft.com/library/cc231198.aspx documents how HResult is formed.
// In short, the error code is only the 2 least significant bytes.
catch (CryptographicException exception) when ((exception.HResult & 0xFFFF) == UserCancelledErrorCode)
Comment on lines +94 to 99
// https://msdn.microsoft.com/library/cc231198.aspx documents how HResult is formed.
// In short, the error code is only the 2 least significant bytes.
catch (CryptographicException exception) when ((exception.HResult & 0xFFFF) == UserCancelledErrorCode)
{
Log.WindowsCertificateTrustCanceled();
throw new UserCancelledTrustException();
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Does WindowsCertificateManager correctly report cancellation?

4 participants