Skip to content

C#: Support replaces-base via the DependabotProxy. - #22494

Draft
michaelnebel wants to merge 8 commits into
github:mainfrom
michaelnebel:csharp/replaces-base
Draft

C#: Support replaces-base via the DependabotProxy.#22494
michaelnebel wants to merge 8 commits into
github:mainfrom
michaelnebel:csharp/replaces-base

Conversation

@michaelnebel

@michaelnebelmichaelnebel commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

In this PR we add support using the replace-base flag for privateregistries. If any private registries are configured to replacebase, then we use these registries as NuGet feed sources instead of the default public nuget.org in fallback scenarios all scenarios - even if the public NuGet feed is mentioned in nuget.config files (unless it is explicitly configured as a fallback feed as well).

As an add on for this PR, we also prevent the fallback that doesn't provide feeds via the command line when restoring packages manually, if private registries are configured (to make the logic consistent with other similar paths).

DCA looks good.

CopilotAI 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

Feed-check opt-out behavior is regressed, and one fallback path can still retain nuget.org.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 Medium severity

New issues introduced by this change (1)
SeverityFinding
Medium severitycsharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​FeedManager.cs — This now probes the default feeds even when `CODEQL_EXTRACTOR_CSHARP_BUILDLESS_NUGET_FEEDS_CHECK=fal…
What changed in this PR

Adds replaces-base support for private NuGet registries used during buildless C# dependency restoration.

Changes:

  • Parses and exposes replacement-base registry URLs.
  • Uses replacement registries for default and fallback feeds.
  • Adds unit coverage and a change note.
FileDescription
csharp/​ql/​lib/​change-notes/​2026-09-03-replaces-base.mdDocuments the new behavior.
csharp/​extractor/​Semmle.Extraction.Tests/​FeedManager.csTests default and fallback feed selection.
csharp/​extractor/​Semmle.Extraction.Tests/​DependabotProxy.csTests replaces-base parsing.
csharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​PackagesConfigRestorer.csUses reachable default feeds during restoration.
csharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​IDependabotProxy.csExposes replacement-base registry URLs.
csharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​FeedManager.csSelects replacement registries as default feeds.
csharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​DependabotProxy.csParses and stores registry replacement metadata.
Suppressed comments (1)

csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FeedManager.cs:315

  • The replacement is only applied through GetReachableFallbackNugetFeeds, but NugetPackageRestorer.DownloadMissingPackages calls that path only when feed responsiveness checks are enabled; its disabled branch uses AllFeeds, which can still contain inherited nuget.org. Thus replaces-base: true does not actually replace the public base feed in fallback downloads when the supported responsiveness opt-out is used. Please separate construction of the configured fallback set from reachability filtering and use that set in both branches.
 fallbackFeeds.UnionWith(defaultFeeds);
logger.LogInfo($"No fallback NuGet feeds specified. Adding default feeds: {string.Join(", ", defaultFeeds.OrderBy(f => f))}");

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadcsharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FeedManager.cs Outdated

CopilotAI 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

The responsiveness-check opt-out path can still use public NuGet.org despite a configured replacement registry.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 2 Medium severity

New issues introduced by this change (1)
SeverityFinding
Medium severitycsharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​FeedManager.csDefaultFeeds is substituted only on the responsiveness-checking path. When…
Pre-existing issues (1)
SeverityFinding
Medium severitycsharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​FeedManager.cs — This now probes the default feeds even when `CODEQL_EXTRACTOR_CSHARP_BUILDLESS_NUGET_FEEDS_CHECK=fal… View comment

CopilotAI 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

Public NuGet endpoint variants such as the supported v2 URL are not replaced.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 2 Medium severity

New issues introduced by this change (1)
SeverityFinding
Medium severitycsharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​FeedManager.cs — This only recognizes the canonical v3 URL, so a configured public NuGet source such as…
Pre-existing issues (1)
SeverityFinding
Medium severitycsharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​FeedManager.csDefaultFeeds is substituted only on the responsiveness-checking path. When… View comment
Issues resolved since last review (1)
SeverityFinding
Medium severitycsharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​FeedManager.cs — This now probes the default feeds even when `CODEQL_EXTRACTOR_CSHARP_BUILDLESS_NUGET_FEEDS_CHECK=fal… View resolved comment

Comment threadcsharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FeedManager.cs Outdated

CopilotAI 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

The release note inaccurately describes the replacement behavior as fallback-only.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 Low severity

New issues introduced by this change (1)
SeverityFinding
Low severitycsharp/​ql/​lib/​change-notes/​2026-09-03-replaces-base.md — The release note describes the replacement as fallback-only, but GetFeeds now replaces NuGet.org…
Issues resolved since last review (2)
SeverityFinding
Medium severitycsharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​FeedManager.cs — This only recognizes the canonical v3 URL, so a configured public NuGet source such as… View resolved comment
Medium severitycsharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​FeedManager.csDefaultFeeds is substituted only on the responsiveness-checking path. When… View resolved comment

Comment threadcsharp/ql/lib/change-notes/2026-09-03-replaces-base.md Outdated

CopilotAI 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

🟢 Approval recommended

The implementation is coherent and the remaining documentation and regression-test gaps are minor.

Review tier: Balanced
Findings: 1 Medium severity · 1 Low severity

New issues introduced by this change (2)
SeverityFinding
Medium severitycsharp/​extractor/​Semmle.Extraction.Tests/​FeedManager.cs — The replacement test still supplies the canonical v3 endpoint in both inputs, so the newly…
Low severitycsharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​FeedManager.cs — This makes the fallback default conditional on replaces-base, but the public XML documentation…
Issues resolved since last review (1)
SeverityFinding
Low severitycsharp/​ql/​lib/​change-notes/​2026-09-03-replaces-base.md — The release note describes the replacement as fallback-only, but GetFeeds now replaces NuGet.org… View resolved comment

Comment threadcsharp/extractor/Semmle.Extraction.Tests/FeedManager.cs Outdated

CopilotAI 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

A trailing semicolon after the nested class declaration causes a compilation error.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 Low severity

Pre-existing issues (1)
SeverityFinding
Low severitycsharp/​extractor/​Semmle.Extraction.CSharp.DependencyFetching/​FeedManager.cs — This makes the fallback default conditional on replaces-base, but the public XML documentation… View comment
Issues resolved since last review (1)
SeverityFinding
Medium severitycsharp/​extractor/​Semmle.Extraction.Tests/​FeedManager.cs — The replacement test still supplies the canonical v3 endpoint in both inputs, so the newly… View resolved comment

Sign up for freeto 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.

2 participants

@michaelnebel