Skip to content

Fix SSL certificate trust nudge on non-English Windows - #2932

Merged
bcotrim merged 5 commits into
trunkfrom
fix-windows-cert-trust
Apr 7, 2026
Merged

Fix SSL certificate trust nudge on non-English Windows#2932
bcotrim merged 5 commits into
trunkfrom
fix-windows-cert-trust

Conversation

@bcotrim

@bcotrim bcotrim commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

Related issues

How AI was used in this PR

I used AI to investigate the issue, confirm the cause (translated strings), lookup the value and implement the fix

Proposed Changes

  • Replace the localized "Server Authentication" string check with the RFC 5280 OID 1.3.6.1.5.5.7.3.1 (id-kp-serverAuth) in both desktop and CLI certificate managers
  • Extract the OID as a shared constant SERVER_AUTH_OID in tools/common/constants.ts

Root cause

isRootCATrusted() on Windows parses certutil -verify output looking for the string "Server Authentication". Windows localizes this label (e.g. "Autenticação de servidor" in Portuguese, likely "Uwierzytelnianie serwera" in Polish), so the check always fails on non-English systems. The OID 1.3.6.1.5.5.7.3.1 is a standardized identifier from RFC 5280 that remains constant regardless of locale.

Testing Instructions

  • Set Windows display language to a non-English language (e.g. Portuguese, Polish)
  • Create a site with a custom domain and HTTPS enabled
  • Observe the "Trust Certificate" nudge in site settings
  • Click "Trust Certificate" and approve the UAC prompt
  • Verify the nudge disappears after trusting

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
@bcotrim bcotrim self-assigned this Mar 27, 2026
@bcotrim
bcotrim requested a review from a team March 27, 2026 10:54
@wpmobilebot

wpmobilebot commented Mar 27, 2026

Copy link
Copy Markdown
Collaborator

📊 Performance Test Results

Comparing ac81349 vs trunk

app-size

Metric trunk ac81349 Diff Change
App Size (Mac) 1249.53 MB 1249.54 MB +0.00 MB ⚪ 0.0%

site-editor

Metric trunk ac81349 Diff Change
load 1571 ms 1912 ms +341 ms 🔴 21.7%

site-startup

Metric trunk ac81349 Diff Change
siteCreation 8201 ms 8182 ms 19 ms ⚪ 0.0%
siteStartup 4835 ms 4937 ms +102 ms 🔴 2.1%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

@ivan-ottinger ivan-ottinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice fix! The proposed changes look good to me and I can confirm they fix the issue.

Before:

Image

After:

Image

@epeicher epeicher left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @bcotrim! I have tested on Windows by changing the language to Spanish and I still see the warning. I have posted some suggestion about a potential fix

Comment thread apps/cli/lib/certificate-manager.ts Outdated
Comment on lines +165 to +166
const hasValidPolicies =
stdout.includes( 'Verified Application Policies:' ) &&
stdout.includes( 'Server Authentication' );
stdout.includes( 'Verified Application Policies:' ) && stdout.includes( SERVER_AUTH_OID );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verified Application Policies: is also localized by Windows (e.g. Directivas de aplicación comprobadas: in Spanish), so this check always fails on non-English systems. Since certutil -verify throws on failure, checking for the OID alone is sufficient.

Suggested change
const hasValidPolicies =
stdout.includes( 'Verified Application Policies:' ) &&
stdout.includes( 'Server Authentication' );
stdout.includes( 'Verified Application Policies:' ) && stdout.includes( SERVER_AUTH_OID );
const hasServerAuthOID = stdout.includes( SERVER_AUTH_OID );
const hasValidPolicies =
stdout.includes( 'Verified Application Policies:' ) &&
stdout.includes( 'Server Authentication' );
stdout.includes( 'Verified Application Policies:' ) && stdout.includes( SERVER_AUTH_OID );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar to the above

@bcotrim

bcotrim commented Apr 6, 2026

Copy link
Copy Markdown
Contributor Author

nice catch @epeicher
On my tests, with Windows in Portuguese Verified Application Policies: wasn't translated.
Using just the OID would not work since it would still show up in an untrusted certificate.
I’ve updated the condition to check for CERT_TRUST_IS_UNTRUSTED_ROOT.
Could you give it another review, please?

@bcotrim
bcotrim requested a review from epeicher April 6, 2026 08:11

@epeicher epeicher left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @bcotrim for addressing the comments! I have tested it on Windows on Spanish, and it works as expected. The nudge disappears after accepting the UAC dialog. LGTM!

Before clicking on Trust After clicking on Trust
Image Image
@bcotrim
bcotrim merged commit 59a6bf0 into trunk Apr 7, 2026
10 checks passed
@bcotrim
bcotrim deleted the fix-windows-cert-trust branch April 7, 2026 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

4 participants