C#: Set proxy environment variables, if Dependabot proxy is detected

[mbg]

This PR is part of work to enable private package registries to be used in Default Setup.

The existing Default Setup workflow will initialise the Dependabot package proxy, if a private package registry configuration is set. The host, port, and certificate used by the proxy are then passed to CodeQL in the analyze step.

The changes in this PR modify the C# extractor to recognise when the corresponding environment variables are set. If so, we use the data from those environment variables to:

1. Construct the proxy address from the host and port and pass it to dotnet via the HTTP_PROXY and HTTPS_PROXY environment variables.
2. Store the certificate on disk, and pass the path to dotnet via SSL_CERT_FILE.
3. We also configure a suitable proxy for the feed reachability checks.

In testing so far, this works fine on Linux with fairly arbitrary versions of dotnet. It does not seem to work on macOS and likely also does not work on Windows.

[tamasvajk]

LGTM, added some comments/questions.

What's not working on macOS? Is it the dotnet calls, or the httpclient that's not working?

[tamasvajk]

Do we need this, because we need to support a certificate that's not signed by a well-known trusted root cert authority?

[mbg]

Correct. The Dependabot Proxy uses a self-signed certificate that is generated when we initialise it in the Default Setup workflow. That's then passed to us in an environment variable, and we should trust it.

[tamasvajk]

This could probably implement System.Net.IWebProxy.

[mbg]

Maybe, but I am not sure I see any immediate benefit to it. Although I guess it would simplify the implementation for the feed reachability checking? Happy to do that in a follow-up PR if it makes sense, but it would be nice to get this functionality shipped for now.

[mbg]

What's not working on macOS? Is it the dotnet calls

dotnet on macOS (and Windows) does not seem to respect HTTP(S)_PROXY and SSL_CERT_FILE (likely because it only uses OpenSSL under the hood on Linux?)

[tamasvajk]

dotnet on macOS (and Windows) does not seem to respect HTTP(S)_PROXY and SSL_CERT_FILE (likely because it only uses OpenSSL under the hood on Linux?)

If we don't expect this change to work on macos and windows, we should be explicit about it. Can we only create the dependabot proxy if we're on linux?

[mbg]

If we don't expect this change to work on macos and windows, we should be explicit about it. Can we only create the dependabot proxy if we're on linux?

@tamasvajk this is done as part of the Default Setup workflow with a dedicated action. Until recently, this only supported Linux. It does now support Windows and macOS as well, but the docs for the private registries feature will note the current limitations. In other words, it's not something we need to worry about in this C#-specific part.

[tamasvajk]

If we don't expect this change to work on macos and windows, we should be explicit about it. Can we only create the dependabot proxy if we're on linux?

@tamasvajk this is done as part of the Default Setup workflow with a dedicated action. Until recently, this only supported Linux. It does now support Windows and macOS as well, but the docs for the private registries feature will note the current limitations. In other words, it's not something we need to worry about in this C#-specific part.

Does this mean that on Windows and MacOS, the CODEQL_PROXY_* environment variables are not going to be set?

[mbg]

Does this mean that on Windows and MacOS, the CODEQL_PROXY_* environment variables are not going to be set?

@tamasvajk They do get set on Windows and macOS. The environment variables we set here just wouldn't affect dotnet on those platforms (though it would be desirable if they did).

However, the proxy we install for the feed reachability check would be affected. That would result in a weird situation on those platforms where we check whether the feeds that require the proxy (i.e. require authentication) are reachable, determine that they are, but then dotnet cannot connect them.

So I have now added a check to skip initialising the DependabotProxy instance on those platforms.

I have also addressed all of your other review comments.

[tamasvajk]

LGTM

[mbg]

@tamasvajk I tested this a bit more after the changes in response to your review, and ran into some flakiness with the new X509Certificate2 call failing since the certificate hadn't been flushed to disk correctly yet in some cases. I have added an explicit Close call on the writer, but also am just loading the certificate from the existing string value obtained from the environment variable, so we can skip reading the certificate from disk entirely.

[tamasvajk]

I'm somewhat surprised that this is needed. The writer is disposed at the end of this method, and we wouldn't access the file elsewhere before that. If you want to control the disposal a bit more fine grained, you could still use a using block, and you could get rid of the Close call.

using(var writer = certFile.CreateText()) 
{
  writer.Write(cert);
}