From 8d044b0a64c8ecf31fcbf9fe60fa025fcb7d134a Mon Sep 17 00:00:00 2001 From: Daniel Elsner Date: Thu, 26 Mar 2026 17:35:27 +0100 Subject: [PATCH 1/2] Try with only basic auth support --- src/Runner.Common/HostContext.cs | 4 +++ src/Runner.Sdk/RunnerWebProxy.cs | 43 ++++++++++++++++++++++---------- src/Test/L0/RunnerWebProxyL0.cs | 41 +++++++++++++++--------------- 3 files changed, 54 insertions(+), 34 deletions(-) diff --git a/src/Runner.Common/HostContext.cs b/src/Runner.Common/HostContext.cs index ffb08684a53..a820afd602f 100644 --- a/src/Runner.Common/HostContext.cs +++ b/src/Runner.Common/HostContext.cs @@ -214,6 +214,10 @@ public HostContext(string hostType, string logFile = null) } } + // Wire up proxy credential tracing so GetCredential calls appear in runner logs. + // This lets us confirm which auth schemes .NET iterates through during proxy tunnel setup. + RunnerWebProxy.Tracing = (message) => _trace.Info(message); + if (StringUtil.ConvertToBoolean(Environment.GetEnvironmentVariable("GITHUB_ACTIONS_RUNNER_TLS_NO_VERIFY"))) { _trace.Warning($"Runner is running under insecure mode: HTTPS server certificate validation has been turned off by GITHUB_ACTIONS_RUNNER_TLS_NO_VERIFY environment variable."); diff --git a/src/Runner.Sdk/RunnerWebProxy.cs b/src/Runner.Sdk/RunnerWebProxy.cs index 947a484cfc6..35e4d2fc18b 100644 --- a/src/Runner.Sdk/RunnerWebProxy.cs +++ b/src/Runner.Sdk/RunnerWebProxy.cs @@ -95,13 +95,7 @@ public RunnerWebProxy() if (!string.IsNullOrEmpty(_httpProxyUsername) || !string.IsNullOrEmpty(_httpProxyPassword)) { - var credential = new NetworkCredential(_httpProxyUsername, _httpProxyPassword); - // Register under both the full URI (with userinfo) and stripped URI (without userinfo). - // .NET may or may not strip userinfo from the proxy URI before calling GetCredential, - // so we store both to ensure a match. Only "Basic" is registered so that .NET returns - // null for Negotiate/NTLM challenges and falls through to Basic, which works cross-platform. - AddBasicCredential(proxyHttpUri, credential); - AddBasicCredential(new UriBuilder(proxyHttpUri.Scheme, proxyHttpUri.Host, proxyHttpUri.Port).Uri, credential); + Credentials = new BasicProxyCredentials(_httpProxyUsername, _httpProxyPassword); } } @@ -131,9 +125,7 @@ public RunnerWebProxy() if (!string.IsNullOrEmpty(_httpsProxyUsername) || !string.IsNullOrEmpty(_httpsProxyPassword)) { - var credential = new NetworkCredential(_httpsProxyUsername, _httpsProxyPassword); - AddBasicCredential(proxyHttpsUri, credential); - AddBasicCredential(new UriBuilder(proxyHttpsUri.Scheme, proxyHttpsUri.Host, proxyHttpsUri.Port).Uri, credential); + Credentials = new BasicProxyCredentials(_httpsProxyUsername, _httpsProxyPassword); } } @@ -249,10 +241,35 @@ private bool IsUriInBypassList(Uri input) return false; } - private void AddBasicCredential(Uri proxyUri, NetworkCredential credential) + // Optional trace sink wired up by the runner host (e.g. HostContext) so that + // proxy credential lookups are visible in the runner log for diagnostics. + // Defaults to null (no-op). Never log the password itself. + public static Action Tracing { get; set; } + + // Returns credentials only for Basic auth, null for all other schemes. + // Returning null causes .NET to skip that scheme entirely, so Negotiate and NTLM + // are never attempted and Basic is used directly — avoiding failures when those + // schemes are advertised but not actually supported by the proxy. + // URI matching is intentionally ignored to avoid CredentialCache pitfalls. + private sealed class BasicProxyCredentials : ICredentials { - (Credentials as CredentialCache).Remove(proxyUri, "Basic"); - (Credentials as CredentialCache).Add(proxyUri, "Basic", credential); + private readonly NetworkCredential _credential; + + internal BasicProxyCredentials(string username, string password) + { + _credential = new NetworkCredential(username, password); + } + + public NetworkCredential GetCredential(Uri uri, string authType) + { + if (!string.Equals(authType, "Basic", StringComparison.OrdinalIgnoreCase)) + { + Tracing?.Invoke($"[RunnerWebProxy] GetCredential called: uri={uri}, authType={authType}, skipping (only Basic is supported)"); + return null; + } + Tracing?.Invoke($"[RunnerWebProxy] GetCredential called: uri={uri}, authType={authType}, returning credentials for user '{_credential.UserName}'"); + return _credential; + } } private string PrependHttpIfMissing(string proxyAddress) diff --git a/src/Test/L0/RunnerWebProxyL0.cs b/src/Test/L0/RunnerWebProxyL0.cs index 9a1b0a8c18a..273263b1580 100644 --- a/src/Test/L0/RunnerWebProxyL0.cs +++ b/src/Test/L0/RunnerWebProxyL0.cs @@ -584,12 +584,13 @@ public void WebProxyFromEnvironmentVariablesWithPort80() [Fact] [Trait("Level", "L0")] [Trait("Category", "Common")] - public void WebProxyCredentialsBasicAuthSchemeOnly() + public void WebProxyCredentialsBasicOnly() { - // Verifies the fix for corporate proxies that advertise Negotiate/NTLM/Basic: - // - GetCredential returns null for Negotiate and NTLM so .NET skips them - // - GetCredential returns credentials for Basic (what the proxy actually accepts) - // - Lookup works both with and without userinfo in the URI (handles .NET stripping userinfo) + // Verifies that proxy credentials are returned only for Basic auth. + // Returning null for Negotiate/NTLM causes .NET to skip those schemes entirely, + // going straight to Basic without attempting Negotiate or NTLM first. + // URI matching is intentionally ignored, so both userinfo and non-userinfo + // URI forms must work. try { Environment.SetEnvironmentVariable("https_proxy", "http://user:pass@127.0.0.1:8888"); @@ -598,22 +599,20 @@ public void WebProxyCredentialsBasicAuthSchemeOnly() var proxyUriWithUserInfo = new Uri("http://user:pass@127.0.0.1:8888"); var proxyUriWithoutUserInfo = new Uri("http://127.0.0.1:8888"); - // Negotiate and NTLM must return null so .NET falls through to Basic - Assert.Null(proxy.Credentials.GetCredential(proxyUriWithUserInfo, "Negotiate")); - Assert.Null(proxy.Credentials.GetCredential(proxyUriWithUserInfo, "NTLM")); - Assert.Null(proxy.Credentials.GetCredential(proxyUriWithoutUserInfo, "Negotiate")); - Assert.Null(proxy.Credentials.GetCredential(proxyUriWithoutUserInfo, "NTLM")); - - // Basic must return credentials regardless of whether the URI includes userinfo - var credWithUserInfo = proxy.Credentials.GetCredential(proxyUriWithUserInfo, "Basic"); - Assert.NotNull(credWithUserInfo); - Assert.Equal("user", credWithUserInfo.UserName); - Assert.Equal("pass", credWithUserInfo.Password); - - var credWithoutUserInfo = proxy.Credentials.GetCredential(proxyUriWithoutUserInfo, "Basic"); - Assert.NotNull(credWithoutUserInfo); - Assert.Equal("user", credWithoutUserInfo.UserName); - Assert.Equal("pass", credWithoutUserInfo.Password); + foreach (var uri in new[] { proxyUriWithUserInfo, proxyUriWithoutUserInfo }) + { + // Basic must return credentials + var cred = proxy.Credentials.GetCredential(uri, "Basic"); + Assert.NotNull(cred); + Assert.Equal("user", cred.UserName); + Assert.Equal("pass", cred.Password); + + // Non-Basic schemes must return null so .NET skips them + foreach (var scheme in new[] { "NTLM", "Negotiate", "Digest" }) + { + Assert.Null(proxy.Credentials.GetCredential(uri, scheme)); + } + } } finally { From 02565ced791e084910b1a7325d5b321472db97f5 Mon Sep 17 00:00:00 2001 From: Daniel Elsner Date: Thu, 26 Mar 2026 17:44:08 +0100 Subject: [PATCH 2/2] Fix --- src/Runner.Sdk/RunnerWebProxy.cs | 34 ++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/Runner.Sdk/RunnerWebProxy.cs b/src/Runner.Sdk/RunnerWebProxy.cs index 35e4d2fc18b..e9237967e4e 100644 --- a/src/Runner.Sdk/RunnerWebProxy.cs +++ b/src/Runner.Sdk/RunnerWebProxy.cs @@ -42,7 +42,8 @@ public class RunnerWebProxy : IWebProxy public RunnerWebProxy() { - Credentials = new CredentialCache(); + var proxyCredentials = new BasicProxyCredentials(); + Credentials = proxyCredentials; var httpProxyAddress = Environment.GetEnvironmentVariable("http_proxy"); if (string.IsNullOrEmpty(httpProxyAddress)) @@ -95,7 +96,7 @@ public RunnerWebProxy() if (!string.IsNullOrEmpty(_httpProxyUsername) || !string.IsNullOrEmpty(_httpProxyPassword)) { - Credentials = new BasicProxyCredentials(_httpProxyUsername, _httpProxyPassword); + proxyCredentials.Add(proxyHttpUri, _httpProxyUsername, _httpProxyPassword); } } @@ -125,7 +126,7 @@ public RunnerWebProxy() if (!string.IsNullOrEmpty(_httpsProxyUsername) || !string.IsNullOrEmpty(_httpsProxyPassword)) { - Credentials = new BasicProxyCredentials(_httpsProxyUsername, _httpsProxyPassword); + proxyCredentials.Add(proxyHttpsUri, _httpsProxyUsername, _httpsProxyPassword); } } @@ -246,18 +247,19 @@ private bool IsUriInBypassList(Uri input) // Defaults to null (no-op). Never log the password itself. public static Action Tracing { get; set; } - // Returns credentials only for Basic auth, null for all other schemes. - // Returning null causes .NET to skip that scheme entirely, so Negotiate and NTLM - // are never attempted and Basic is used directly — avoiding failures when those - // schemes are advertised but not actually supported by the proxy. - // URI matching is intentionally ignored to avoid CredentialCache pitfalls. + // Returns credentials only for Basic auth, keyed by proxy host:port. + // Returning null for non-Basic schemes causes .NET to skip Negotiate/NTLM entirely. + // Matching by host:port (ignoring userinfo) avoids CredentialCache URI-matching + // pitfalls when .NET strips userinfo from the proxy URI before calling GetCredential. private sealed class BasicProxyCredentials : ICredentials { - private readonly NetworkCredential _credential; + // Key: "host:port" (case-insensitive), Value: credential for that proxy + private readonly Dictionary _map = + new(StringComparer.OrdinalIgnoreCase); - internal BasicProxyCredentials(string username, string password) + internal void Add(Uri proxyUri, string username, string password) { - _credential = new NetworkCredential(username, password); + _map[$"{proxyUri.Host}:{proxyUri.Port}"] = new NetworkCredential(username, password); } public NetworkCredential GetCredential(Uri uri, string authType) @@ -267,8 +269,14 @@ public NetworkCredential GetCredential(Uri uri, string authType) Tracing?.Invoke($"[RunnerWebProxy] GetCredential called: uri={uri}, authType={authType}, skipping (only Basic is supported)"); return null; } - Tracing?.Invoke($"[RunnerWebProxy] GetCredential called: uri={uri}, authType={authType}, returning credentials for user '{_credential.UserName}'"); - return _credential; + var key = $"{uri.Host}:{uri.Port}"; + if (_map.TryGetValue(key, out var cred)) + { + Tracing?.Invoke($"[RunnerWebProxy] GetCredential called: uri={uri}, authType={authType}, returning credentials for user '{cred.UserName}'"); + return cred; + } + Tracing?.Invoke($"[RunnerWebProxy] GetCredential called: uri={uri}, authType={authType}, no credentials found for {key}"); + return null; } }