Skip to content

fix(nodeenv): redact mirror configuration in verbose output - #469

Open
iSanae233 wants to merge 1 commit into
ekalinin:masterfrom
iSanae233:fix/redact-verbose-mirror-config
Open

iSanae233 wants to merge 1 commit into
ekalinin:masterfrom
iSanae233:fix/redact-verbose-mirror-config

Conversation

@iSanae233

@iSanae233 iSanae233 commented Nov 4, 2026 •

Copy link
Copy Markdown

Config._load(..., verbose=True) prints the configured mirror value before main() separates URL credentials. When the mirror contains a password or token, captured verbose output can expose it to readers who cannot access the original configuration. #426 protects request and download-error handling, but this earlier configuration message still contains the original value.

This change displays <redacted> for the mirror without changing the value stored in Config.mirror. Request authentication and other configuration diagnostics keep their existing behavior. With synthetic credentials, the output changes from:

CONFIG nodeenvrc: mirror = https://user:example-password@mirror.example/nodejs

to:

CONFIG nodeenvrc: mirror = <redacted>

The whole mirror value is hidden rather than parsed again for logging. This also covers query-string tokens, scheme-less values and malformed URLs. The tradeoff is that the verbose configuration message no longer shows the mirror host or path.

The regression tests cover normal and verbose loading, including quotes, percent encoding, IPv6 and query tokens. A localhost HTTP test checks that the request still receives its Basic Authorization header while captured output contains neither the credential nor the encoded header. README.rst and the unreleased changelog document the output change.

Follow-up to #426. There is no separate issue to close.

Validation

Tested on macOS 27.2 / arm64 with CPython 3.14.7:

  • Before the fix, the 16 parameterized configuration cases produced 8 failures in verbose mode and 8 passes in normal mode.
  • Focused configuration and mirror-authentication tests: 22 passed after the fix.
  • tox -e py314 passed, including coverage and flake8: 473 passed, 71 skipped and 11 deselected, with 2 existing TLS deprecation warnings.
  • An sdist and wheel built successfully, and git diff --check passed.

The authentication test uses synthetic credentials and a server bound to 127.0.0.1. The skips are fish and Windows cases. The external-download integration tests and other Python/OS combinations were not run locally.

This PR addresses verbose mirror configuration output only; it does not change other diagnostic paths.

  • Tests cover the change
  • tox passes: the tests and flake8 (local py314 environment)
  • CHANGES has an entry under Version [unreleased], if nodeenv's
    behavior changes
  • README.rst is updated, if the change affects what it documents
  • The title has the form type(scope): subject

AI-assisted implementation and review with OpenAI Codex with GPT-6.1 Sol High.

Config._load logs the mirror value before main separates URL credentials.
Hide the value in diagnostics while preserving the parsed configuration
and Basic authentication behavior.

Add regression coverage for normal and verbose config loading and a
localhost mirror that still receives its Authorization header. Document
the output change in README.rst and the unreleased changelog.

Follow-up to ekalinin#426.

Co-Authored-By: GPT-6.1 Sol High

This branch has not been deployed

No deployments
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.

1 participant