Skip to content

Commit 9194ba7

Browse files
fix(secureenv): trim whitespace before proxy credential redaction (MCP-2776) (#705)
Follow-up to PR #704 (MCP-2769) reviewer note: redactProxyCredentials did not trim surrounding whitespace, so a value like " http://user:pass@proxy " made url.Parse error (leading space) and fall through, forwarding the credentialed value verbatim and bypassing the redaction guarantee. Trim whitespace at function entry before parsing; whitespace is never meaningful in a proxy URL. Adds table cases for whitespace-wrapped scheme'd and schemeless credentialed values. Co-authored-by: Paperclip <noreply@paperclip.ing>
1 parent 85b2ef7 commit 9194ba7

2 files changed

Lines changed: 9 additions & 0 deletions

File tree

internal/secureenv/manager.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,10 @@ func anyProxyKeyPresent(present map[string]struct{}, group []string) bool {
342342
// proxy host/port so the proxy remains functional. Non-URL values (e.g. the
343343
// host list in NO_PROXY) and unparseable values are returned unchanged.
344344
func redactProxyCredentials(value string) string {
345+
// Surrounding whitespace would make url.Parse error (leading space) or
346+
// otherwise fall through, forwarding a credentialed value verbatim. Trim it
347+
// first; whitespace is never meaningful in a proxy URL.
348+
value = strings.TrimSpace(value)
345349
if value == "" {
346350
return value
347351
}

internal/secureenv/proxy_forward_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ func TestRedactProxyCredentials(t *testing.T) {
3636
{"schemeless user only", "user@proxy.example.com:3128", "proxy.example.com:3128"},
3737
{"schemeless no userinfo", "proxy.example.com:8080", "proxy.example.com:8080"},
3838
{"schemeless at-in-path not stripped", "proxy.example.com:8080/path@x", "proxy.example.com:8080/path@x"},
39+
// Whitespace-wrapped credentialed URLs would otherwise make url.Parse
40+
// error and fall through, forwarding creds verbatim (PR #704 non-blocking
41+
// review note). Surrounding whitespace is trimmed before redaction.
42+
{"whitespace-wrapped scheme creds", " http://user:pass@proxy.example.com:8080 ", "http://proxy.example.com:8080"},
43+
{"whitespace-wrapped schemeless creds", "\tuser:pass@proxy.example.com:8080\n", "proxy.example.com:8080"},
3944
}
4045
for _, tc := range cases {
4146
t.Run(tc.name, func(t *testing.T) {

0 commit comments

Comments
 (0)