Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pub/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
SDK constraints.
24 changes: 24 additions & 0 deletions pub/lib/dependabot/pub/update_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
94 changes: 94 additions & 0 deletions pub/spec/dependabot/pub/update_checker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -243,6 +245,7 @@
end
end
end

context "given an outdated dependency, requiring unlock" do
let(:dependency_name) { "retry" }

Expand Down Expand Up @@ -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?
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.

Good question! Yes, it does but I'm not sure if:

  • we parse the transitive dependencies
  • there is a capability to update them
  • there is a capability to update them if they are locked by a parent dependency

I think it's worth confirming and then documenting if we don't support them yet.

Copy link
Copy Markdown
Member Author

@jeffwidman jeffwidman Nov 29, 2022

Choose a reason for hiding this comment

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

Here's a manual run of a public repo with a transitive dep:

[dependabot-core-dev] ~/dependabot-core $ SECURITY_ADVISORIES='[{"dependency-name":"analyzer","patched-versions":[],"unaffected-versions":[],"affected-versions":["<= 6.0.0"]}]' bin/dry-run.rb pub dart-lang/pub-dev --dir "/app" --cache=files --dep="analyzer" # transitive dep
=> reading cloned repo from /home/dependabot/dependabot-core/tmp/dart-lang/pub-dev
=> parsing dependency files
I, [2022-11-29T17:45:28.571710 #9427]  INFO -- : Failed to infer the flutter version. Attempting to use latest stable release.
I, [2022-11-29T17:45:28.571838 #9427]  INFO -- : Checking out Flutter version stable
I, [2022-11-29T17:45:29.774632 #9427]  INFO -- : Running `flutter doctor` to install artifacts and create flutter/version.
I, [2022-11-29T17:45:32.909847 #9427]  INFO -- : Running `flutter --version`
I, [2022-11-29T17:45:33.864389 #9427]  INFO -- : Installed the Flutter SDK version: 3.3.9 with Dart 2.18.5.
=> updating 1 dependencies: analyzer

=== analyzer (4.7.0) (vulnerable 🚨)
 => checking for updates 1/1
I, [2022-11-29T17:45:34.300923 #9427]  INFO -- : Failed to infer the flutter version. Attempting to use latest stable release.
I, [2022-11-29T17:45:34.301044 #9427]  INFO -- : Checking out Flutter version stable
I, [2022-11-29T17:45:35.498491 #9427]  INFO -- : Running `flutter doctor` to install artifacts and create flutter/version.
I, [2022-11-29T17:45:38.642229 #9427]  INFO -- : Running `flutter --version`
I, [2022-11-29T17:45:39.581409 #9427]  INFO -- : Installed the Flutter SDK version: 3.3.9 with Dart 2.18.5.
 => latest available version is 5.2.0
 => there is no available non-vulnerable version
 => latest allowed version is 4.7.0
 => requirements to unlock: update_not_possible
 => requirements update strategy:
    (no security update possible 🙅‍♀️)
🌍 Total requests made: '0'

As expected, it throws update_not_possible.

Here's a similar run against that repo, but for a direct dep:

[dependabot-core-dev] ~/dependabot-core $ SECURITY_ADVISORIES='[{"dependency-name":"buffer","patched-versions":[],"unaffected-versions":[],"affected-versions":["<= 2.0.0"]}]' bin/dry-run.rb pub dart-lang/pub-dev --dir "/app" --cache=files --dep="buffer" # direct dep
=> reading cloned repo from /home/dependabot/dependabot-core/tmp/dart-lang/pub-dev
=> parsing dependency files
I, [2022-11-29T17:44:05.782429 #8784]  INFO -- : Failed to infer the flutter version. Attempting to use latest stable release.
I, [2022-11-29T17:44:05.782594 #8784]  INFO -- : Checking out Flutter version stable
I, [2022-11-29T17:44:06.918204 #8784]  INFO -- : Running `flutter doctor` to install artifacts and create flutter/version.
I, [2022-11-29T17:44:12.748310 #8784]  INFO -- : Running `flutter --version`
I, [2022-11-29T17:44:13.784197 #8784]  INFO -- : Installed the Flutter SDK version: 3.3.9 with Dart 2.18.5.
=> updating 1 dependencies: buffer

=== buffer (1.1.1) (vulnerable 🚨)
 => checking for updates 1/1
I, [2022-11-29T17:44:14.243087 #8784]  INFO -- : Failed to infer the flutter version. Attempting to use latest stable release.
I, [2022-11-29T17:44:14.243212 #8784]  INFO -- : Checking out Flutter version stable
I, [2022-11-29T17:44:15.484309 #8784]  INFO -- : Running `flutter doctor` to install artifacts and create flutter/version.
I, [2022-11-29T17:44:18.590065 #8784]  INFO -- : Running `flutter --version`
I, [2022-11-29T17:44:19.892184 #8784]  INFO -- : Installed the Flutter SDK version: 3.3.9 with Dart 2.18.5.
 => latest available version is 1.1.1
    (no update needed as it's already up-to-date)
🌍 Total requests made: '0'

I was a little surprised that ☝️ says "no update needed as already up-to-date" rather than "no non-vulnerable version available"... but turns out that's because of the dry-run script logging not first checking for vulnerable:

if checker.up_to_date?
puts " (no update needed as it's already up-to-date)"
next
end
if checker.vulnerable?
if checker.lowest_security_fix_version
puts " => earliest available non-vulnerable version is " \
"#{checker.lowest_security_fix_version}"
else
puts " => there is no available non-vulnerable version"
end
end

I'll open a separate PR to tweak that shortly.

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.

I believe we can do all of those to varying degrees.

The native helper report command returns multiple packages that are unlocked simultaneously if needed, see:

# dart pub global run pub:dependency_services report

The native helper apply command takes a list of dependencies to update:

# dart pub global run pub:dependency_services apply << EOF

We use that for the full unlock here:

parse_updated_dependency(d, requirements_update_strategy: resolved_requirements_update_strategy)

See the tests here:

let(:requirements_to_unlock) { :all }

However, as eg. updated_requirements only allows us to return a single requirement update the native helpers apply command does its own resolution to re-find any transitive dependencies that has to be unlocked along with the one we are interested in currently: https://github.com/dart-lang/pub/blob/4c9ebd0e53796f0d45f9fcb79a14a3fa149beac2/lib/src/command/dependency_services.dart#L461

If updated_requirements allowed us to return the full set of updates, not just a single one we could (potentially) save doing this resolution.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was a little surprised that ☝️ says "no update needed as already up-to-date" rather than "no non-vulnerable version available"... but turns out that's because of the dry-run script logging not first checking for vulnerable... I'll open a separate PR to tweak that shortly.

PR here: #6258

# 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" }
Expand Down Expand Up @@ -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
Expand Down