Repository navigation
Fixed process container proxy log redaction - #1696
Open
Vladexy88x wants to merge 3 commits into
Open
Vladexy88x wants to merge 3 commits into
Vladexy88x wants to merge 3 commits into
Conversation
Log networkProxy with redact_proxy_url instead of to_url(), so passwords no longer reach --debug or --log-file output. Add a wxc-exec log test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-proxy-log-redaction
Vladexy88x
changed the title
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new test can fail ordinary Windows CI on hosts without ProcessContainer support, while the scheduled supported-host suite does not run it, leaving a regression coverage gap.
What changed in this PR
This PR prevents ProcessContainer proxy passwords from appearing in diagnostic logs by using the existing redaction helper, without changing the proxy URL supplied to the sandbox.
Changes:
- Redacts credentials in the effective-proxy log message.
- Adds a Windows integration test that checks execution reaches that message and the log contains no proxy password.
| File | Description |
|---|---|
src/tools/wxc/tests/proxy_log_redaction.rs |
Adds an executor-level regression test for proxy password leakage. |
src/mxc-sdk/src/backends/process_container/common/base_container_runner.rs |
Applies the shared proxy URL redactor before logging. |
Review effort: Balanced
…suite The wxc integration test ran in the unfiltered Windows cargo test step, which fails on hosts without ProcessContainer support, and the scheduled ProcessContainer validation never ran it. Move the scenario into run_base_container_network_tests.ps1, which nightly validation already runs on native ProcessContainer hosts. It checks that the log file and --debug output show the redacted proxy URL and never the password. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
|
@microsoft-github-policy-service agree |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📖 Description
ProcessContainer printed the effective proxy with to_url(), so if runtimeConfig.networkProxy contained credentials (http://user:pass@host:port), the password showed up in --debug output and in the --log-file log.
This PR passes the URL through redact_proxy_url before logging it, which is what the other backends already do. Only the log line changes; the sandbox still gets the real proxy URL.
It also adds a
proxy-log-redactioncase totests/scripts/run_base_container_network_tests.ps1, which scheduled ProcessContainer validation already runs on native hosts. The case setsnetworkProxytohttp://alice:hunter2@127.0.0.1:<port>and runs with--debugand--log-file. It checks that the workload succeeds, that the log file and debug output both containeffective proxy: http://***@127.0.0.1:<port>, and that the password appears nowhere in the log, stdout, or stderr.🔗 References
No tracking issue.
🔍 Validation
run_base_container_network_tests.ps1on Windows 11 Pro 10.0.26300.9711 (native ProcessContainer), PowerShell 7.6.9:proxy-log-redactionpasses with the fix. With the fix reverted it fails, and the debug output showseffective proxy: http://alice:hunter2@127.0.0.1:<port>.default-deny-blocks-icmpcase fails on this host only because it matches English error text, and this Windows install reports "Access denied" in Russian; the ping is blocked as expected. This PR does not change that case.✅ Checklist
tests/scripts/README.mdlists the new coverage📋 Issue Type