From cfed359498293ead68feea6492ff33b331e88f72 Mon Sep 17 00:00:00 2001 From: Peter Wagner <1559510+thepwagner@users.noreply.github.com> Date: Mon, 8 Feb 2021 10:21:27 -0500 Subject: [PATCH 1/2] Dependabot::UnexpectedExternalCode This introduces the above new error, to be raised when Dependabot determines it may need to evaluate external code to update a dependency. The included sample and test cases centre around the Bundler ecosystem, where Git sources which may result in a evaluating the dependency's `.gemspec`, e.g. https://github.com/dependabot/dependabot-core/blob/main/common/dependabot-common.gemspec Note: this guard and the associated exception are an opt-in feature, to be used in conjunction with features that extend the resources accessible to Dependabot (e.g. private packages and sources). --- bundler/lib/dependabot/bundler/file_parser.rb | 15 +++++++ .../dependabot/bundler/file_parser_spec.rb | 39 ++++++++++++++++++- common/lib/dependabot/errors.rb | 3 ++ common/lib/dependabot/file_parsers/base.rb | 3 +- 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/bundler/lib/dependabot/bundler/file_parser.rb b/bundler/lib/dependabot/bundler/file_parser.rb index acd390ef270..a045f328579 100644 --- a/bundler/lib/dependabot/bundler/file_parser.rb +++ b/bundler/lib/dependabot/bundler/file_parser.rb @@ -21,11 +21,26 @@ def parse dependency_set += gemfile_dependencies dependency_set += gemspec_dependencies dependency_set += lockfile_dependencies + check_external_code(dependency_set.dependencies) dependency_set.dependencies end private + def check_external_code(dependencies) + return unless @reject_external_code + return unless git_source?(dependencies) + + # A git source dependency might contain a .gemspec that is evaluated + raise ::Dependabot::UnexpectedExternalCode + end + + def git_source?(dependencies) + dependencies.any? do |dep| + dep.requirements.any? { |req| req.fetch(:source)&.fetch(:type) == "git" } + end + end + def gemfile_dependencies dependencies = DependencySet.new diff --git a/bundler/spec/dependabot/bundler/file_parser_spec.rb b/bundler/spec/dependabot/bundler/file_parser_spec.rb index 1895ea9cf38..751ef4b9eba 100644 --- a/bundler/spec/dependabot/bundler/file_parser_spec.rb +++ b/bundler/spec/dependabot/bundler/file_parser_spec.rb @@ -16,7 +16,13 @@ let(:lockfile) do Dependabot::DependencyFile.new(name: "Gemfile.lock", content: lockfile_body) end - let(:parser) { described_class.new(dependency_files: files, source: source) } + let(:parser) do + described_class.new( + dependency_files: files, + source: source, + reject_external_code: reject_external_code + ) + end let(:source) do Dependabot::Source.new( provider: "github", @@ -24,6 +30,7 @@ directory: "/" ) end + let(:reject_external_code) { false } let(:gemfile_body) { fixture("ruby", "gemfiles", gemfile_fixture_name) } let(:lockfile_body) { fixture("ruby", "lockfiles", lockfile_fixture_name) } let(:gemfile_fixture_name) { "version_specified" } @@ -281,6 +288,36 @@ end end + context "rejecting external code" do + let(:reject_external_code) { true } + + context "with no git sources" do + let(:gemfile_fixture_name) { "version_specified" } + + it "does not raise exception" do + expect { parser.parse }.not_to raise_error + end + end + + context "with a git source" do + let(:gemfile_fixture_name) { "git_source" } + let(:lockfile_fixture_name) { "git_source.lock" } + + it "raises exception" do + expect { parser.parse }.to raise_error(::Dependabot::UnexpectedExternalCode) + end + end + + context "with a subdependency of a git source" do + let(:lockfile_fixture_name) { "git_source_undeclared.lock" } + let(:gemfile_fixture_name) { "git_source_undeclared" } + + it "raises exception" do + expect { parser.parse }.to raise_error(::Dependabot::UnexpectedExternalCode) + end + end + end + context "with a dependency that only appears in the lockfile" do let(:gemfile_fixture_name) { "subdependency" } let(:lockfile_fixture_name) { "subdependency.lock" } diff --git a/common/lib/dependabot/errors.rb b/common/lib/dependabot/errors.rb index 9af489f7e62..d9b75db0d00 100644 --- a/common/lib/dependabot/errors.rb +++ b/common/lib/dependabot/errors.rb @@ -217,4 +217,7 @@ def initialize(go_mod, declared_path, discovered_path) # Raised by UpdateChecker if all candidate updates are ignored class AllVersionsIgnored < DependabotError; end + + # Raised by FileParser if processing may execute external code in the update context + class UnexpectedExternalCode < DependabotError; end end diff --git a/common/lib/dependabot/file_parsers/base.rb b/common/lib/dependabot/file_parsers/base.rb index 1fcd45318ea..974e6fde114 100644 --- a/common/lib/dependabot/file_parsers/base.rb +++ b/common/lib/dependabot/file_parsers/base.rb @@ -6,11 +6,12 @@ class Base attr_reader :dependency_files, :repo_contents_path, :credentials, :source def initialize(dependency_files:, repo_contents_path: nil, source:, - credentials: []) + credentials: [], reject_external_code: false) @dependency_files = dependency_files @repo_contents_path = repo_contents_path @credentials = credentials @source = source + @reject_external_code = reject_external_code check_required_files end From 03e620cd7b2a1421c2ddcd7af7d1d99bfa00fd4d Mon Sep 17 00:00:00 2001 From: Peter Wagner <1559510+thepwagner@users.noreply.github.com> Date: Mon, 8 Feb 2021 13:54:53 -0500 Subject: [PATCH 2/2] dry-run --reject-external-code --- bin/dry-run.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bin/dry-run.rb b/bin/dry-run.rb index 59910422f74..ea24f88a904 100755 --- a/bin/dry-run.rb +++ b/bin/dry-run.rb @@ -103,6 +103,7 @@ write: false, clone: false, lockfile_only: false, + reject_external_code: false, requirements_update_strategy: nil, commit: nil, updater_options: {}, @@ -167,6 +168,10 @@ $options[:lockfile_only] = true end + opts.on("--reject-external-code", "Reject external code") do |_value| + $options[:reject_external_code] = true + end + opts_req_desc = "Options: auto, widen_ranges, bump_versions or "\ "bump_versions_if_necessary" opts.on("--requirements-update-strategy STRATEGY", opts_req_desc) do |value| @@ -454,7 +459,8 @@ def handle_dependabot_error(error:, dependency:) dependency_files: $files, repo_contents_path: $repo_contents_path, source: source, - credentials: $options[:credentials] + credentials: $options[:credentials], + reject_external_code: $options[:reject_external_code], ) dependencies = cached_read("dependencies") { parser.parse }