Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/Runner.Common/HostContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down
53 changes: 39 additions & 14 deletions src/Runner.Sdk/RunnerWebProxy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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<string> 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<string, NetworkCredential> _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)
Expand Down
41 changes: 20 additions & 21 deletions src/Test/L0/RunnerWebProxyL0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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
{
Expand Down