Add Security Update Support for Pub by jeffwidman · Pull Request #6230 · dependabot/dependabot-core

jeffwidman · 2022-11-29T00:00:30Z

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.

jeffwidman · 2022-11-29T00:13:19Z

As mentioned in #6212 (comment), in addition to the unit tests, I also confirmed this is working locally using dry-run:

[dependabot-core-dev] ~/dependabot-core $ SECURITY_ADVISORIES='[{"dependency-name":"retry","patched-versions":[],"unaffected-versions":[],"affected-versions":["<= 3.0.2"]}]' bin/dry-run.rb pub org/test-repo --cache=files --dep=retry

=> reading cloned repo from /home/dependabot/dependabot-core/tmp/dsp-testing/dependabot-all-updates-test-staging
=> parsing dependency files
I, [2022-11-26T02:24:01.438558 #6936]  INFO -- : Installing the Flutter SDK version: 3.3.9 from channel stable with Dart 2.18.5
I, [2022-11-26T02:24:01.438688 #6936]  INFO -- : Checking out Flutter version refs/tags/3.3.9
I, [2022-11-26T02:24:04.025229 #6936]  INFO -- : Running `flutter doctor` to install artifacts and create flutter/version.
I, [2022-11-26T02:25:58.674844 #6936]  INFO -- : Running `flutter --version`
I, [2022-11-26T02:26:00.915668 #6936]  INFO -- : Installed the Flutter SDK version: 3.3.9 with Dart 2.18.5.
=> updating 1 dependencies: retry

=== retry (2.0.0) (vulnerable 🚨)
 => checking for updates 1/1
I, [2022-11-26T02:26:01.273093 #6936]  INFO -- : Installing the Flutter SDK version: 3.3.9 from channel stable with Dart 2.18.5
I, [2022-11-26T02:26:01.273206 #6936]  INFO -- : Checking out Flutter version refs/tags/3.3.9
I, [2022-11-26T02:26:02.841201 #6936]  INFO -- : Running `flutter doctor` to install artifacts and create flutter/version.
I, [2022-11-26T02:26:07.200580 #6936]  INFO -- : Running `flutter --version`
I, [2022-11-26T02:26:09.369335 #6936]  INFO -- : Installed the Flutter SDK version: 3.3.9 with Dart 2.18.5.
 => latest available version is 3.1.0
 => earliest available non-vulnerable version is 3.1.0
 => latest allowed version is 3.1.0
 => requirements to unlock: own
 => requirements update strategy:
 => updating retry from 2.0.0 to 3.1.0
I, [2022-11-26T02:26:10.227446 #6936]  INFO -- : Installing the Flutter SDK version: 3.3.9 from channel stable with Dart 2.18.5
I, [2022-11-26T02:26:10.227632 #6936]  INFO -- : Checking out Flutter version refs/tags/3.3.9
I, [2022-11-26T02:26:12.050901 #6936]  INFO -- : Running `flutter doctor` to install artifacts and create flutter/version.
I, [2022-11-26T02:26:15.777269 #6936]  INFO -- : Running `flutter --version`
I, [2022-11-26T02:26:17.972313 #6936]  INFO -- : Installed the Flutter SDK version: 3.3.9 with Dart 2.18.5.

    ± pubspec.yaml
    ~~~
    6c6
    <   retry: ^2.0.0 # Can update with updated constraint, no further constraints.
    ---
    >   retry: ^3.1.0 # Can update with updated constraint, no further constraints.
    ~~~

    ± pubspec.lock
    ~~~
    38c38
    <     version: "2.0.0"
    ---
    >     version: "3.1.0"
    ~~~
🌍 Total requests made: '0'

mctofu · 2022-11-29T00:47:03Z

pub/spec/dependabot/pub/update_checker_spec.rb

+      end
+    end
+
+    # TODO: should it update indirect deps for security vulnerabilities? I assume Pub has these?


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.

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:

dependabot-core/bin/dry-run.rb

Lines 682 to 694 in c2b0cf8

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.

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:

dependabot-core/pub/README.md

Line 67 in 575bb90

# dart pub global run pub:dependency_services report

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

dependabot-core/pub/README.md

Line 200 in 575bb90

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

We use that for the full unlock here:

dependabot-core/pub/lib/dependabot/pub/update_checker.rb

Line 92 in 4acd031

parse_updated_dependency(d, requirements_update_strategy: resolved_requirements_update_strategy)

See the tests here:

dependabot-core/pub/spec/dependabot/pub/update_checker_spec.rb

Line 250 in 575bb90

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.

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

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.

In #5950, this short-circuit was moved up to skip checking for the latest version if we already know we're on the latest version. During review, it was moved below the case for a security update, when a dep is already up-to-date but also _not_ vulnerable. However, there's yet another case, which is where a dep is up-to-date and _also_ vulnerable. I was surprised in #6230 (comment) to see that it spits out `no update needed as it's already up-to-date` when I had explicitly passed in a security advisor marking the latest version as vulnerable. It wasn't clear to me if the `dry-run` was prematurely bailing before checking for vulnerable versions. That was for the `pub` ecosystem, where both `checker.lowest_resolvable_security_fix_version` and `checker.latest_resolvable_version` return the same version. The very next `if` block handles the case when the dep _is_ vulnerable. So by moving this short-circuit below that, it will still short circuit for the common case where a version is not vulnerable and on latest. For reference, here's pre-change run: ``` [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 warning: parser/current is loading parser/ruby31, which recognizes3.1.3-compliant syntax, but you are running 3.1.2. Please see https://github.com/whitequark/parser#compatibility-with-ruby-mri. => reading cloned repo from /home/dependabot/dependabot-core/tmp/dart-lang/pub-dev => parsing dependency files I, [2022-12-04T22:26:35.101963 #38484] INFO -- : Failed to infer the flutter version. Attempting to use latest stable release. I, [2022-12-04T22:26:35.102107 #38484] INFO -- : Cloning the flutter repo https://github.com/flutter/flutter. I, [2022-12-04T22:27:40.833227 #38484] INFO -- : Checking out Flutter version stable I, [2022-12-04T22:27:46.471869 #38484] INFO -- : Running `flutter doctor` to install artifacts and create flutter/version. I, [2022-12-04T22:29:39.504293 #38484] INFO -- : Running `flutter --version` I, [2022-12-04T22:29:40.773354 #38484] 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-12-04T22:29:41.511587 #38484] INFO -- : Failed to infer the flutter version. Attempting to use latest stable release. I, [2022-12-04T22:29:41.511704 #38484] INFO -- : Checking out Flutter version stable I, [2022-12-04T22:29:42.735602 #38484] INFO -- : Running `flutter doctor` to install artifacts and create flutter/version. I, [2022-12-04T22:29:45.652134 #38484] INFO -- : Running `flutter --version` I, [2022-12-04T22:29:46.743673 #38484] 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' ``` And here's post-change run: ``` [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 warning: parser/current is loading parser/ruby31, which recognizes3.1.3-compliant syntax, but you are running 3.1.2. Please see https://github.com/whitequark/parser#compatibility-with-ruby-mri. => reading cloned repo from /home/dependabot/dependabot-core/tmp/dart-lang/pub-dev => parsing dependency files I, [2022-12-04T22:44:40.544797 #39357] INFO -- : Failed to infer the flutter version. Attempting to use latest stable release. I, [2022-12-04T22:44:40.544922 #39357] INFO -- : Checking out Flutter version stable I, [2022-12-04T22:44:41.954572 #39357] INFO -- : Running `flutter doctor` to install artifacts and create flutter/version. I, [2022-12-04T22:44:45.209362 #39357] INFO -- : Running `flutter --version` I, [2022-12-04T22:44:46.159503 #39357] 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-12-04T22:44:46.554930 #39357] INFO -- : Failed to infer the flutter version. Attempting to use latest stable release. I, [2022-12-04T22:44:46.555046 #39357] INFO -- : Checking out Flutter version stable I, [2022-12-04T22:44:47.709661 #39357] INFO -- : Running `flutter doctor` to install artifacts and create flutter/version. I, [2022-12-04T22:44:51.640507 #39357] INFO -- : Running `flutter --version` I, [2022-12-04T22:44:52.616051 #39357] INFO -- : Installed the Flutter SDK version: 3.3.9 with Dart 2.18.5. => latest available version is 1.1.1 => there is no available non-vulnerable version (no update needed as it's already up-to-date) 🌍 Total requests made: '0' ``` Note that the output above is slightly confusing because it's _not_ doing a security update only, but rather a normal update with a specified security advisory... so I think it's fine for debugging purposes that we get the information that there's no non-vulnerable version, and also that from a version-checking perspective there's no available update, regardless of vulnerability.

jeffwidman requested a review from a team as a code owner November 29, 2022 00:00

jeffwidman mentioned this pull request Nov 29, 2022

Additional updates to Security Update Support for Pub #6212

Closed

mctofu approved these changes Nov 29, 2022

View reviewed changes

jeffwidman force-pushed the add-security-updates-support-for-pub-basics branch from bdd273e to c88d1dc Compare November 29, 2022 17:24

jeffwidman merged commit dae1dcc into main Nov 29, 2022

jeffwidman deleted the add-security-updates-support-for-pub-basics branch November 29, 2022 19:42

jeffwidman mentioned this pull request Dec 4, 2022

Delay short-circuit in dry-run.rb if vulnerable #6258

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Security Update Support for Pub#6230

Add Security Update Support for Pub#6230
jeffwidman merged 1 commit intomainfrom
add-security-updates-support-for-pub-basics

jeffwidman commented Nov 29, 2022

Uh oh!

jeffwidman commented Nov 29, 2022 •

edited

Loading

Uh oh!

mctofu Nov 29, 2022

Uh oh!

jeffwidman Nov 29, 2022 •

edited

Loading

Uh oh!

sigurdm Nov 30, 2022

Uh oh!

jeffwidman Dec 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	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

Conversation

jeffwidman commented Nov 29, 2022

Uh oh!

jeffwidman commented Nov 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mctofu Nov 29, 2022

Choose a reason for hiding this comment

Uh oh!

jeffwidman Nov 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sigurdm Nov 30, 2022

Choose a reason for hiding this comment

Uh oh!

jeffwidman Dec 4, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jeffwidman commented Nov 29, 2022 •

edited

Loading

jeffwidman Nov 29, 2022 •

edited

Loading