Skip to content

Redact secrets when logging config and warn user when changing categories#19966

Merged
seanbudd merged 30 commits intomasterfrom
detectSecrets
Apr 22, 2026
Merged

Redact secrets when logging config and warn user when changing categories#19966
seanbudd merged 30 commits intomasterfrom
detectSecrets

Conversation

@seanbudd
Copy link
Copy Markdown
Member

@seanbudd seanbudd commented Apr 17, 2026

Link to issue number:

Closes #19465

Summary of the issue:

Secrets stored in NVDA config are often unintentionally logged in debug mode by NVDA

Description of user facing changes:

logging will attempt to redact secrets when the developer decides to sanitise risky log messages.
Added a new log level: secrets, to disable redactions for required debug logging.
Added a warning whenever selecting a log level below info.

Description of developer facing changes:

A new redactSecrets parameter for logging, which searches for and replaces secrets in the log message.

Description of development approach:

Use https://github.com/Yelp/detect-secrets

This pull request introduces secret redaction support in logging, ensuring that sensitive information is masked in log outputs when requested.
A new log level is added so you can view unredacted logs if needed.

Secret Redaction in Logging

  • Added a redactSecrets parameter to the Logger._log method in logHandler.py that, when enabled, uses the detect-secrets library to scan and mask detected secrets in log messages.
  • Updated logging calls in source/config/__init__.py to use redactSecrets=True when logging potentially sensitive configuration data.
  • Updated the developer documentation to describe the new redactSecrets parameter and recommend its use for sensitive data.

Dependency and Packaging Support

  • Added detect-secrets as a dependency in pyproject.toml and ensured all relevant submodules are included in frozen builds for dynamic plugin loading
  • Include multiprocessing in bundle - needed for import, seems to functionally work?

Testing strategy:

  • Manual testing by putting a secret in my NVDA config.
    • installed copy from build
    • source copy
  • Added unit tests for the new secret redaction logic in tests/unit/test_logHandler.py, covering both normal and edge cases.

Known issues with pull request:

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.

@seanbudd seanbudd changed the title Redact secrets when logging Redact secrets when logging config Apr 17, 2026
Comment thread source/logHandler.py Fixed
Comment thread source/logHandler.py Outdated
seanbudd and others added 5 commits April 17, 2026 14:16
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@seanbudd seanbudd marked this pull request as ready for review April 17, 2026 07:15
@seanbudd seanbudd requested a review from a team as a code owner April 17, 2026 07:15
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 introduces an opt-in mechanism to redact detected secrets from log output (notably for config dumps), backed by the detect-secrets dependency, and wires it into config logging plus developer-facing documentation.

Changes:

  • Add redactSecrets kwarg support to NVDA’s custom Logger._log implementation and perform secret detection/redaction on the formatted message.
  • Use redactSecrets=True when logging config state during load/upgrade to reduce risk of leaking secrets into logs.
  • Add detect-secrets==1.5.0 to dependencies/lockfile and document the new logging parameter in the changelog.

Reviewed changes

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

Show a summary per file
File Description
source/logHandler.py Adds redactSecrets kwarg to _log and integrates detect-secrets scanning/redaction into the logging path.
source/config/__init__.py Enables redaction for config dump log messages during load/upgrade.
pyproject.toml Adds detect-secrets as a runtime dependency.
uv.lock Locks detect-secrets and updates dependency lists accordingly.
source/setup.py Ensures detect_secrets is included in build packaging (function-scoped import).
user_docs/en/changes.md Documents the new redactSecrets logging parameter and its intended usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread source/logHandler.py Outdated
Comment thread source/logHandler.py Outdated
Comment thread source/logHandler.py Outdated
Comment thread source/logHandler.py Outdated
Comment thread source/logHandler.py Outdated
@seanbudd seanbudd marked this pull request as draft April 17, 2026 07:27
@seanbudd seanbudd mentioned this pull request Apr 17, 2026
5 tasks
@seanbudd seanbudd marked this pull request as ready for review April 17, 2026 07:46
Comment thread tests/unit/test_logHandler.py Fixed
@seanbudd
Copy link
Copy Markdown
Member Author

huh - seems like the module still isn't being included correctly

@seanbudd seanbudd marked this pull request as draft April 20, 2026 04:53
Comment thread tests/unit/test_logHandler.py Dismissed
@seanbudd seanbudd changed the title Redact secrets when logging config Redact secrets when logging config and warn user when changing categories Apr 21, 2026
Comment thread source/gui/settingsDialogs.py Fixed
Comment thread source/gui/settingsDialogs.py Outdated
Comment thread source/gui/settingsDialogs.py Outdated
Comment thread user_docs/en/changes.md
Comment thread source/logHandler.py Outdated
Comment thread source/logHandler.py Outdated
seanbudd and others added 2 commits April 21, 2026 16:32
Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>
Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>
@seanbudd seanbudd marked this pull request as draft April 21, 2026 07:31
@seanbudd
Copy link
Copy Markdown
Member Author

unit tests are failing

@seanbudd seanbudd marked this pull request as ready for review April 22, 2026 00:28
Copy link
Copy Markdown
Member

@Qchristensen Qchristensen left a comment

Choose a reason for hiding this comment

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

Changes read well

@seanbudd seanbudd enabled auto-merge (squash) April 22, 2026 05:37
@seanbudd seanbudd disabled auto-merge April 22, 2026 05:49
@seanbudd seanbudd merged commit 42f949f into master Apr 22, 2026
40 of 41 checks passed
@seanbudd seanbudd deleted the detectSecrets branch April 22, 2026 05:50
@github-actions github-actions Bot added this to the 2026.2 milestone Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add user warning that logging can store PII on the user's machine

5 participants