Use root certificates when using the add-on store and NVDA remote by seanbudd · Pull Request #18528 · nvaccess/nvda

seanbudd · 2025-07-22T04:44:14Z

Link to issue number:

Reimplements #18354
Reimplements #18490
Reimplements #18402
Fixes #15905

Summary of the issue:

When trying to download an add-on from the add-on store in certain environments such as corporates, the download fails due to the machine not trusting the root certificate of the add-on source.

This can be caused by two issues:

the requests library using certifi rather than the system root certificates
the system root certificates not trusting the add-on download source

When performing an update check, we update windows root certificates if our certificate is invalid or out of date.

Description of user facing changes:

NVDA should more reliably trust request endpoints using windows root certificates. e.g. accessing the add-on store in a corporate environment.

Description of developer facing changes:

requests and similar libraries now has truststore injected into it

Description of development approach:

This pull request improves how NVDA handles SSL/TLS certificate verification, particularly for HTTPS requests and add-on downloads. It ensures that the application uses the Windows root certificate store instead of the Python default (certifi), provides a fallback mechanism to update root certificates when encountering untrusted certificates, and enhances user interaction when certificate errors occur during add-on downloads. Additionally, shared networking logic has been refactored and centralized for maintainability.

Certificate Verification and Networking Improvements:

Added the truststore dependency and injected it at startup so that all requests HTTP calls use the Windows root certificate store instead of certifi, improving compatibility in corporate and managed environments. (pyproject.toml, source/core.py) [1] [2] [3]
Introduced a new utils/networking.py module that centralizes logic for fetching URLs, handling certificate verification errors, and updating Windows root certificates. This includes helper functions like _fetchUrlAndUpdateRootCertificates, _getCertificate, and _updateWindowsRootCertificates. (source/utils/networking.py)

Add-on Store and Update Check Enhancements:

Updated add-on store and update check logic to use the new _fetchUrlAndUpdateRootCertificates function, ensuring robust handling of certificate verification failures and automatic root certificate updates when necessary. (source/addonStore/dataManager.py, source/updateCheck.py) [1] [2] [3] [4] [5]
Improved the add-on download process to prompt the user when encountering an untrusted certificate, displaying the SHA256 fingerprint and allowing the user to trust and install the root certificate if appropriate. (source/addonStore/network.py) [1] [2]

Codebase Cleanup and Refactoring:

Removed duplicate and now-unnecessary certificate update logic from updateCheck.py and centralized all certificate handling in utils/networking.py. (source/updateCheck.py)
Updated variable naming and improved code clarity in the add-on download logic. (source/addonStore/network.py) [1] [2] [3] [4] [5]

Testing strategy:

Get user testing from NVDA unable to fetch add-ons in add-on store when a corporate HTTPS certificate is present #15905
Smoke test using add-on store
Smoke test update check

Known issues with pull request:

None

Code Review Checklist:

Documentation:
- Change log entry
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
Testing:
- Unit tests
- System (end to end) tests
- Manual testing
UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English
API is compatible with existing add-ons.
Security precautions taken.

@coderabbitai summary

CyrilleB79 · 2025-07-22T08:54:38Z

I do not know if it is still applicable or not. But if yes, please check that DEBUGWARNING's are not replaced in the log by VERBOSE's. See #18402 (comment) for more details.

seanbudd · 2025-10-03T07:06:41Z

@CyrilleB79 - I cannot reproduce that issue when running this PR from source.
@hwf1324 - could you re-test this PR and see if it still fixes #15905 ?

Copilot

Pull Request Overview

This PR improves NVDA's SSL/TLS certificate verification by switching from the default certifi certificate bundle to Windows root certificates, particularly addressing issues when accessing the add-on store in corporate environments.

Adds truststore dependency and injects it at startup to use Windows root certificates
Implements centralized certificate handling with automatic root certificate updates
Enhances add-on download process with user prompts for untrusted certificates

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pyproject.toml	Adds truststore dependency to override certifi
source/core.py	Injects truststore at startup to use Windows certificates
source/utils/networking.py	New module centralizing certificate handling and URL fetching
source/updateCheck.py	Refactors to use centralized networking utils
source/addonStore/dataManager.py	Updates to use centralized certificate handling
source/addonStore/network.py	Enhances download process with certificate verification prompts
source/_remoteClient/transport.py	Updates SSL context configuration
user_docs/en/changes.md	Documents changes for users and developers

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

hwf1324 · 2025-10-06T06:09:42Z

could you re-test this PR and see if it still fixes #15905 ?

Confirm that running from source successfully downloads add-ons.

SaschaCowley

Thanks, @seanbudd

Co-authored-by: Sascha Cowley <16543535+SaschaCowley@users.noreply.github.com>

…mote (#18528)" This reverts commit 2fa3658.

…mote (#18528)" (#19106) This reverts commit 2fa3658. ### Reverts PR Reverts #18528 ### Issues fixed Fixes #19076 ### Issues reopened Reopens #15905 ### Reason for revert - Add-on API solution doesn't work - extracting SSL from context is dangerous - Regression with NVDA remote ### Can this PR be reimplemented? If so, what is required for the next attempt - Instead use trust store directly rather than injecting into all SSL contexts

seanbudd added this to the 2026.1 milestone Jul 22, 2025

CyrilleB79 mentioned this pull request Jul 22, 2025

Create a confirmation dialog to update root certificates when downloading add-ons #18402

Merged

7 tasks

seanbudd added conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. api-breaking-change labels Jul 29, 2025

Reintroduce fixes for #15905

91b4511

seanbudd force-pushed the revert-18527-revertRoot branch from 8d1e41b to 91b4511 Compare October 3, 2025 06:45

seanbudd added 2 commits October 3, 2025 16:48

update API breaking changes

5076670

update uv.lock

a6c6b83

seanbudd marked this pull request as ready for review October 3, 2025 07:07

seanbudd requested a review from a team as a code owner October 3, 2025 07:07

seanbudd requested review from SaschaCowley and Copilot October 3, 2025 07:07

seanbudd added the blocked/needs-testing label Oct 3, 2025

Copilot AI reviewed Oct 3, 2025

View reviewed changes

Comment thread source/utils/networking.py

Comment thread source/addonStore/network.py

Comment thread source/_remoteClient/transport.py

seanbudd changed the title ~~Use root certificates when using the add-on store~~ Use root certificates when using the add-on store and NVDA remote Oct 3, 2025

fix deprecations

c1dfda9

seanbudd removed the blocked/needs-testing label Oct 6, 2025

SaschaCowley approved these changes Oct 9, 2025

View reviewed changes

Comment thread user_docs/en/changes.md Outdated

Update user_docs/en/changes.md

417eb85

Co-authored-by: Sascha Cowley <16543535+SaschaCowley@users.noreply.github.com>

SaschaCowley merged commit 2fa3658 into master Oct 10, 2025
53 of 55 checks passed

SaschaCowley deleted the revert-18527-revertRoot branch October 10, 2025 00:40

seanbudd mentioned this pull request Oct 10, 2025

NVDA alpha: Remote access cannot connect #19076

Closed

seanbudd added a commit that referenced this pull request Oct 15, 2025

Revert "Use root certificates when using the add-on store and NVDA re…

b503267

…mote (#18528)" This reverts commit 2fa3658.

seanbudd mentioned this pull request Oct 15, 2025

Revert "Use root certificates when using the add-on store and NVDA remote (#18528)" #19106

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use root certificates when using the add-on store and NVDA remote#18528

Use root certificates when using the add-on store and NVDA remote#18528
SaschaCowley merged 5 commits into
masterfrom
revert-18527-revertRoot

seanbudd commented Jul 22, 2025 •

edited

Loading

Uh oh!

CyrilleB79 commented Jul 22, 2025

Uh oh!

seanbudd commented Oct 3, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hwf1324 commented Oct 6, 2025

Uh oh!

SaschaCowley left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

seanbudd commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of user facing changes:

Description of developer facing changes:

Description of development approach:

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Uh oh!

CyrilleB79 commented Jul 22, 2025

Uh oh!

seanbudd commented Oct 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hwf1324 commented Oct 6, 2025

Uh oh!

SaschaCowley left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

seanbudd commented Jul 22, 2025 •

edited

Loading