Skip to content

Beta to master #18395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 2, 2025
Merged

Beta to master #18395

merged 4 commits into from
Jul 2, 2025

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented Jul 2, 2025

No description provided.

actions-user and others added 4 commits July 2, 2025 01:43
Summary of the issue:

If a device doesn't trust NV Access's TLS certificate, fetches to the add-on store fails.
When performing an update check, we update windows root certificates if our certificate is invalid or out of date.
Description of user facing changes:

Users without a trusted NV Access TLS certificate installed already should be able to access the add-on store
Description of developer facing changes:

Code related to updating root certificates is deprecated as it is not intended for the public
Description of development approach:

Made the code to update certificates more generic
@Copilot Copilot AI review requested due to automatic review settings July 2, 2025 03:10
@seanbudd seanbudd requested a review from a team as a code owner July 2, 2025 03:10
@seanbudd seanbudd requested a review from SaschaCowley July 2, 2025 03:10
@seanbudd seanbudd merged commit 4ff02dd into master Jul 2, 2025
4 of 7 checks passed
@github-actions github-actions bot added this to the 2025.3 milestone Jul 2, 2025
Copy link

@Copilot 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 refactors update checking to use a centralized networking utility that handles Windows root certificate updates, introduces deprecation warnings for legacy constants, updates related documentation, and adjusts some driver and data manager calls to the new helper.

  • Add a utils/networking.py module with Windows cert-update helpers
  • Replace in-module update–fetch logic in updateCheck.py and addonStore/dataManager.py with _fetchUrlAndUpdateRootCertificates
  • Document deprecated CERT_USAGE_MATCH, CERT_CHAIN_PARA, and UPDATE_FETCH_TIMEOUT_S and update detection in a braille driver

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
user_docs/en/changes.md Add a “Deprecations” section for three soon-to-be-removed constants
source/utils/networking.py New helper for fetching URLs with Windows root cert updates
source/updateCheck.py Switch to _fetchUrlAndUpdateRootCertificates and clean up old cert logic
source/brailleDisplayDrivers/nlseReaderZoomax.py Update addUsbDevices call to use ProtocolType.SERIAL
source/addonStore/dataManager.py Use the new networking helper in _getCacheHash


data = res.read().decode("utf-8") # Ensure the response is decoded correctly
data = result.content.decode("utf-8") # Ensure the response is decoded correctly
Copy link
Preview

Copilot AI Jul 2, 2025

Choose a reason for hiding this comment

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

[nitpick] Using result.content.decode("utf-8") may ignore the server-declared encoding. Consider using result.text which respects response.encoding.

Suggested change
data = result.content.decode("utf-8") # Ensure the response is decoded correctly
data = result.text # Automatically decode using the server-declared encoding

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants