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..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,13 +96,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); + proxyCredentials.Add(proxyHttpUri, _httpProxyUsername, _httpProxyPassword); } } @@ -131,9 +126,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); + proxyCredentials.Add(proxyHttpsUri, _httpsProxyUsername, _httpsProxyPassword); } } @@ -249,10 +242,42 @@ 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, 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 { - (Credentials as CredentialCache).Remove(proxyUri, "Basic"); - (Credentials as CredentialCache).Add(proxyUri, "Basic", credential); + // Key: "host:port" (case-insensitive), Value: credential for that proxy + private readonly Dictionary _map = + new(StringComparer.OrdinalIgnoreCase); + + internal void Add(Uri proxyUri, string username, string password) + { + _map[$"{proxyUri.Host}:{proxyUri.Port}"] = 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; + } + 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; + } } 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 {