From c88d1dc2fa64eb52cf93442eedb9c1133f77009a Mon Sep 17 00:00:00 2001 From: Jeff Widman Date: Tue, 22 Nov 2022 20:49:31 -0800 Subject: [PATCH] Add Security Update Support for Pub This implements basic security update support for Pub. For now, it's a naive implementation that bumps to the latest available version, and then filters it out if it's vulnerable. In other ecosystems, we only bump to the minimum version required to get a non-vulnerable version. We're currently discussing with the `Pub` team how best to do that over in `https://github.com/dependabot/dependabot-core/issues/5391`, but that will come later. --- pub/README.md | 8 +- pub/lib/dependabot/pub/update_checker.rb | 24 +++++ .../dependabot/pub/update_checker_spec.rb | 94 +++++++++++++++++++ 3 files changed, 122 insertions(+), 4 deletions(-) diff --git a/pub/README.md b/pub/README.md index b90ad1178a4..79b8b019345 100644 --- a/pub/README.md +++ b/pub/README.md @@ -8,10 +8,10 @@ Dart (pub) support for [`dependabot-core`][core-repo]. * `dart pub` in general doesn't read versions numbers from git, so upgrade logic is limited to upgrading to what the 'ref' is pointing to. * If you pin to a specific revision in `pubspec.yaml` dependabot will not find upgrades. * If you give a branch in `pubspec.yaml` dependabot will upgrade to the - latest revision that branch is pointing to, and update pubspec.lock + latest revision that branch is pointing to, and update `pubspec.lock` accordingly. - - No support for security advisory integration. - - If the version found is ignored (by dependabot config) no update will happen (even if, an earlier version could be used) + - Security updates currently bump to the latest version. If the latest version is vulnerable, no update will happen (even if an earlier version could be used). Changing the upgrade strategy to use the minimum non-vulnerable version is tracked in https://github.com/dependabot/dependabot-core/issues/5391. + - If the version found is ignored (by dependabot config) no update will happen (even if an earlier version could be used) - Limited metadata support (just retrieves the repository link). - No support for auhtentication of private package repositories (mostly a configuration issue). - `updated_dependencies_after_full_unlock` only allows updating to a later version, if the latest version that is mutually compatible with other dependencies is the latest version of the said package. This is a dependabot limitation. @@ -246,4 +246,4 @@ available releases: * The latest stable release that matches the SDK constraints will be chosen * If there is no stable release it will choose the newest beta that matches the -SDK constraints. \ No newline at end of file +SDK constraints. diff --git a/pub/lib/dependabot/pub/update_checker.rb b/pub/lib/dependabot/pub/update_checker.rb index f23a9ec4e60..1fecf6f04d1 100644 --- a/pub/lib/dependabot/pub/update_checker.rb +++ b/pub/lib/dependabot/pub/update_checker.rb @@ -2,6 +2,7 @@ require "dependabot/update_checkers" require "dependabot/update_checkers/base" +require "dependabot/update_checkers/version_filters" require "dependabot/pub/helpers" require "yaml" module Dependabot @@ -34,6 +35,29 @@ def latest_resolvable_version version_unless_ignored(entry["version"]) end + def lowest_resolvable_security_fix_version + raise "Dependency not vulnerable!" unless vulnerable? + + lowest_security_fix_version + end + + def lowest_security_fix_version + # TODO: Pub lacks a lowest-non-vulnerable version strategy, for now we simply bump to latest resolvable: + # https://github.com/dependabot/dependabot-core/issues/5391 + relevant_version = latest_resolvable_version + return unless relevant_version + + # NOTE: in other ecosystems, the native helpers return a list of possible versions, to which we apply + # post-filtering. Ideally we move toward a world where we hand the native helper a list of ignored versions + # and possibly a flag indicating "use min version rather than max". The pub team is interested in supporting + # that. But in the meantime for internal consistency with other dependabot ecosystem implementations I kept + # `relevant_versions` as an array. + relevant_versions = [relevant_version] + relevant_versions = Dependabot::UpdateCheckers::VersionFilters.filter_vulnerable_versions(relevant_versions, + security_advisories) + relevant_versions.min + end + def updated_requirements # Requirements that need to be changed, if obtain: # latest_resolvable_version diff --git a/pub/spec/dependabot/pub/update_checker_spec.rb b/pub/spec/dependabot/pub/update_checker_spec.rb index 2fe6b334fc8..65e7ebb58be 100644 --- a/pub/spec/dependabot/pub/update_checker_spec.rb +++ b/pub/spec/dependabot/pub/update_checker_spec.rb @@ -63,12 +63,14 @@ flutter_releases_url: "http://localhost:#{@server[:Port]}/flutter_releases.json" }, raise_on_ignored: raise_on_ignored, + security_advisories: security_advisories, requirements_update_strategy: requirements_update_strategy ) end let(:ignored_versions) { [] } let(:raise_on_ignored) { false } + let(:security_advisories) { [] } let(:dependency) do Dependabot::Dependency.new( @@ -243,6 +245,7 @@ end end end + context "given an outdated dependency, requiring unlock" do let(:dependency_name) { "retry" } @@ -465,6 +468,81 @@ end end + describe "#lowest_resolvable_security_fix_version" do + subject(:lowest_resolvable_security_fix_version) { checker.lowest_resolvable_security_fix_version } + let(:dependency_name) { "retry" } + let(:security_advisories) do + [ + Dependabot::SecurityAdvisory.new( + dependency_name: dependency_name, + package_manager: "pub", + vulnerable_versions: ["<3.0.0"] + ) + ] + end + + context "when a newer non-vulnerable version is available" do + # TODO: Implement https://github.com/dependabot/dependabot-core/issues/5391, then flip "highest" to "lowest" + it "updates to the highest non-vulnerable version" do + is_expected.to eq(Gem::Version.new("3.1.0")) + end + end + + # TODO: should it update indirect deps for security vulnerabilities? I assume Pub has these? + # examples of how to write tests in go_modules/update_checker_spec + + context "when the current version is not newest but also not vulnerable" do + let(:dependency_version) { "3.0.0" } # 3.1.0 is latest + it "raises an error " do + expect { lowest_resolvable_security_fix_version.to }.to raise_error(RuntimeError) do |error| + expect(error.message).to eq("Dependency not vulnerable!") + end + end + end + end + + describe "#lowest_security_fix_version" do + subject(:lowest_security_fix_version) { checker.lowest_security_fix_version } + let(:dependency_name) { "retry" } + + # TODO: Implement https://github.com/dependabot/dependabot-core/issues/5391, then flip "highest" to "lowest" + it "finds the highest available non-vulnerable version" do + is_expected.to eq(Gem::Version.new("3.1.0")) + end + + context "with a security vulnerability on older versions" do + let(:security_advisories) do + [ + Dependabot::SecurityAdvisory.new( + dependency_name: dependency_name, + package_manager: "pub", + vulnerable_versions: ["< 3.0.0"] + ) + ] + end + + # TODO: Implement https://github.com/dependabot/dependabot-core/issues/5391, then flip "highest" to "lowest" + it "finds the highest available non-vulnerable version" do + is_expected.to eq(Gem::Version.new("3.1.0")) + end + + # it "returns nil for git versions" # tested elsewhere under `context "With a git dependency"` + end + + context "with a security vulnerability on all newer versions" do + let(:security_advisories) do + [ + Dependabot::SecurityAdvisory.new( + dependency_name: dependency_name, + package_manager: "pub", + vulnerable_versions: ["< 4.0.0"] + ) + ] + end + it { is_expected.to be_nil } + end + end + context "mono repo" do let(:project) { "mono_repo_main_at_root" } let(:dependency_name) { "dep" } @@ -564,6 +642,22 @@ "version" => new_ref } ] end + + context "with a security vulnerability on older versions" do + let(:security_advisories) do + [ + Dependabot::SecurityAdvisory.new( + dependency_name: dependency_name, + package_manager: "pub", + vulnerable_versions: ["< 3.0.0"] + ) + ] + end + + it "returns no version" do + expect(checker.lowest_security_fix_version).to be_nil + end + end end context "works for a flutter project" do