From 3d0449650644b80cb8c14e1539a13fe52701583a Mon Sep 17 00:00:00 2001 From: Ashleigh Adams Date: Wed, 17 Mar 2021 23:47:16 +0000 Subject: [PATCH 1/3] Add support for disablePackageSources in NuGet.Config This prevents disabled package sources, such as those used for doing deploys to private package registries, from being used. #3295 --- .../nuget/update_checker/repository_finder.rb | 14 ++++ .../update_checker/repository_finder_spec.rb | 21 ++++++ .../fixtures/configs/disabled_sources.config | 72 +++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 nuget/spec/fixtures/configs/disabled_sources.config diff --git a/nuget/lib/dependabot/nuget/update_checker/repository_finder.rb b/nuget/lib/dependabot/nuget/update_checker/repository_finder.rb index 3691883230..1890176d07 100644 --- a/nuget/lib/dependabot/nuget/update_checker/repository_finder.rb +++ b/nuget/lib/dependabot/nuget/update_checker/repository_finder.rb @@ -166,6 +166,20 @@ def repos_from_config_file(config_file) } end + disabled_sources = + doc.css("configuration > disabledPackageSources > add").map do |node| + value = node.attribute("value")&.value&.strip&.downcase || + node.at_xpath("./value")&.content&.strip&.downcase + if value == "true" + node.attribute("key")&.value&.strip || + node.at_xpath("./key")&.content&.strip + end + end + + sources.reject! do |s| + disabled_sources.include?(s[:key]) + end + unless doc.css("configuration > packageSources > clear").any? sources << { url: DEFAULT_REPOSITORY_URL, key: nil } end diff --git a/nuget/spec/dependabot/nuget/update_checker/repository_finder_spec.rb b/nuget/spec/dependabot/nuget/update_checker/repository_finder_spec.rb index ac126e43b5..969c9b350a 100644 --- a/nuget/spec/dependabot/nuget/update_checker/repository_finder_spec.rb +++ b/nuget/spec/dependabot/nuget/update_checker/repository_finder_spec.rb @@ -253,6 +253,27 @@ ) end end + + context "that has disabled package sources" do + let(:config_file_fixture_name) { "disabled_sources.config" } + + it "only includes the enabled package sources" do + expect(dependency_urls).to match_array( + [{ + repository_url: "https://www.myget.org/F/exceptionless/api/v3/"\ + "index.json", + versions_url: "https://www.myget.org/F/exceptionless/api/v3/"\ + "flatcontainer/microsoft.extensions."\ + "dependencymodel/index.json", + search_url: "https://www.myget.org/F/exceptionless/api/v3/"\ + "query?q=microsoft.extensions.dependencymodel"\ + "&prerelease=true", + auth_header: { "Authorization" => "Basic bXk6cGFzc3cwcmQ=" }, + repository_type: "v3" + }] + ) + end + end end context "that has a numeric key" do diff --git a/nuget/spec/fixtures/configs/disabled_sources.config b/nuget/spec/fixtures/configs/disabled_sources.config new file mode 100644 index 0000000000..b4fe380eb7 --- /dev/null +++ b/nuget/spec/fixtures/configs/disabled_sources.config @@ -0,0 +1,72 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 31be7985b8e38f3b4bd3062794d982d95c5dfa2c Mon Sep 17 00:00:00 2001 From: Ashleigh Adams Date: Thu, 18 Mar 2021 00:46:24 +0000 Subject: [PATCH 2/3] Ran the linter and disabled Metrics/MethodLength --- .../dependabot/nuget/update_checker/repository_finder.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/nuget/lib/dependabot/nuget/update_checker/repository_finder.rb b/nuget/lib/dependabot/nuget/update_checker/repository_finder.rb index 1890176d07..1f9a93f14c 100644 --- a/nuget/lib/dependabot/nuget/update_checker/repository_finder.rb +++ b/nuget/lib/dependabot/nuget/update_checker/repository_finder.rb @@ -151,6 +151,7 @@ def config_file_repositories # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/PerceivedComplexity # rubocop:disable Metrics/AbcSize + # rubocop:disable Metrics/MethodLength def repos_from_config_file(config_file) doc = Nokogiri::XML(config_file.content) doc.remove_namespaces! @@ -169,12 +170,12 @@ def repos_from_config_file(config_file) disabled_sources = doc.css("configuration > disabledPackageSources > add").map do |node| value = node.attribute("value")&.value&.strip&.downcase || - node.at_xpath("./value")&.content&.strip&.downcase + node.at_xpath("./value")&.content&.strip&.downcase if value == "true" node.attribute("key")&.value&.strip || node.at_xpath("./key")&.content&.strip end - end + end sources.reject! do |s| disabled_sources.include?(s[:key]) @@ -196,6 +197,7 @@ def repos_from_config_file(config_file) sources end + # rubocop:enable Metrics/MethodLength # rubocop:enable Metrics/AbcSize # rubocop:enable Metrics/PerceivedComplexity # rubocop:enable Metrics/CyclomaticComplexity From 49b849af53d72f4e6641b0779372c6bda76df375 Mon Sep 17 00:00:00 2001 From: Ashleigh Adams Date: Thu, 18 Mar 2021 12:22:45 +0000 Subject: [PATCH 3/3] Pulled disabled_sources parsing into its own method - As suggested, `repos_from_config_file()` was getting too long, so pull the disabled sources parsing into its own method. - Reenable the MethodLength metric for `repos_from_config_file()`. - Includes a small optimization by @jurre. Co-Authored-By: Jurre --- .../nuget/update_checker/repository_finder.rb | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/nuget/lib/dependabot/nuget/update_checker/repository_finder.rb b/nuget/lib/dependabot/nuget/update_checker/repository_finder.rb index 1f9a93f14c..8638bd03b0 100644 --- a/nuget/lib/dependabot/nuget/update_checker/repository_finder.rb +++ b/nuget/lib/dependabot/nuget/update_checker/repository_finder.rb @@ -151,7 +151,6 @@ def config_file_repositories # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/PerceivedComplexity # rubocop:disable Metrics/AbcSize - # rubocop:disable Metrics/MethodLength def repos_from_config_file(config_file) doc = Nokogiri::XML(config_file.content) doc.remove_namespaces! @@ -167,16 +166,7 @@ def repos_from_config_file(config_file) } end - disabled_sources = - doc.css("configuration > disabledPackageSources > add").map do |node| - value = node.attribute("value")&.value&.strip&.downcase || - node.at_xpath("./value")&.content&.strip&.downcase - if value == "true" - node.attribute("key")&.value&.strip || - node.at_xpath("./key")&.content&.strip - end - end - + disabled_sources = disabled_sources(doc) sources.reject! do |s| disabled_sources.include?(s[:key]) end @@ -197,7 +187,6 @@ def repos_from_config_file(config_file) sources end - # rubocop:enable Metrics/MethodLength # rubocop:enable Metrics/AbcSize # rubocop:enable Metrics/PerceivedComplexity # rubocop:enable Metrics/CyclomaticComplexity @@ -214,6 +203,20 @@ def default_repository_details } end + # rubocop:disable Metrics/PerceivedComplexity + def disabled_sources(doc) + doc.css("configuration > disabledPackageSources > add").map do |node| + value = node.attribute("value")&.value || + node.at_xpath("./value")&.content + + if value&.strip&.downcase == "true" + node.attribute("key")&.value&.strip || + node.at_xpath("./key")&.content&.strip + end + end + end + # rubocop:enable Metrics/PerceivedComplexity + # rubocop:disable Metrics/PerceivedComplexity def add_config_file_credentials(sources:, doc:) sources.each do |source_details|