Skip to content

C#: NuGet CLI proxy environment. - #22752

Open
michaelnebel wants to merge 6 commits into
github:mainfrom
michaelnebel:csharp/nugetexeproxyconfig
Open

michaelnebel wants to merge 6 commits into
github:mainfrom
michaelnebel:csharp/nugetexeproxyconfig

Conversation

@michaelnebel

@michaelnebel michaelnebel commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

In this PR, we set the proxy and certificate environment variables when invoking the NuGet CLI for restoring packages found in packages.config files.
Furthermore, we also log the executed NuGet command, similar to what we do when invoking the dotnet CLI.

@michaelnebel
michaelnebel force-pushed the csharp/nugetexeproxyconfig branch 2 times, most recently from fb447b2 to a3a0565 Compare October 6, 2026 07:54
@michaelnebel
michaelnebel requested a balanced review from Copilot October 6, 2026 09:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

NuGet on Linux may not recognize the uppercase-only proxy variables.

Review effort: Balanced
Findings: 1 Medium severity

Open (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.config restores.
  • 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.

@michaelnebel
michaelnebel force-pushed the csharp/nugetexeproxyconfig branch from a3a0565 to 16ebfc8 Compare October 6, 2026 09:31
@michaelnebel
michaelnebel requested a review from mbg October 6, 2026 10:50
@michaelnebel
michaelnebel marked this pull request as ready for review October 6, 2026 10:50
@michaelnebel
michaelnebel requested a review from a team as a code owner October 6, 2026 10:50
mbg
mbg previously approved these changes Oct 6, 2026

@mbg mbg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +196 to +199
else
{
logger.LogDebug("No SSL certificate is configured for the registry proxy.");
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Suggested change
* 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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants