C#: NuGet CLI proxy environment. - #22752
michaelnebel wants to merge 6 commits into
Conversation
… the RegistryProxy class and add a unit test.
fb447b2 to
a3a0565
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
NuGet on Linux may not recognize the uppercase-only proxy variables.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds proxy and certificate environment configuration to NuGet CLI package restoration and logs executed commands.
Changes:
- Centralizes registry proxy environment setup.
- Applies proxy settings to
packages.configrestores. - Adds tests and a change note.
| File | Description |
|---|---|
csharp/ql/lib/change-notes/2026-10-05-nugetcli-proxy-config.md |
Documents proxy support. |
csharp/extractor/Semmle.Extraction.Tests/RegistryProxy.cs |
Tests process environment configuration. |
csharp/extractor/Semmle.Extraction.Tests/FeedManager.cs |
Updates proxy test stubs. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/RegistryProxy.cs |
Centralizes proxy environment setup. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/PackagesConfigRestorer.cs |
Configures and logs NuGet processes. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs |
Passes the proxy to package restoration. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IRegistryProxy.cs |
Exposes process configuration API. |
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNetCliInvoker.cs |
Reuses centralized configuration. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
a3a0565 to
16ebfc8
Compare
mbg
left a comment
mbg
left a comment
There was a problem hiding this comment.
These changes look good to me, thank you for taking this on! :) Just two very minor comments -- up to you if you want to address those and I am happy to approve as-is.
| else | ||
| { | ||
| logger.LogDebug("No SSL certificate is configured for the registry proxy."); | ||
| } |
There was a problem hiding this comment.
Minor: This makes sense to log when we are setting the value for CertificatePath after retrieving the certificate from the CODEQL_PROXY_ var, but maybe not here.
There was a problem hiding this comment.
Perhaps, we should just log when it is set instead - just to avoid that it happens silently (then we always know whether it happens or not).
| --- | ||
| category: minorAnalysis | ||
| --- | ||
| * Proxy and certificate environment variables are now set for the subprocess that invokes the NuGet CLI, enabling it to access private registries during this part of the workflow. |
There was a problem hiding this comment.
Minor: Change "Proxy" to "Private registry proxy" to disambiguate from ordinary proxies that may be configured on a system. Alternatively, since "private registries" are already mentioned in the last part of the note, restructure the sentence to emphasise that part. For example:
| * Proxy and certificate environment variables are now set for the subprocess that invokes the NuGet CLI, enabling it to access private registries during this part of the workflow. | |
| * The subprocess for the NuGet CLI is now provided with the proxy and certificate environment variables needed to access private registries if any are configured. |
In this PR, we set the proxy and certificate environment variables when invoking the NuGet CLI for restoring packages found in
packages.configfiles.Furthermore, we also log the executed NuGet command, similar to what we do when invoking the
dotnetCLI.