Skip to content

Commit

Permalink
Delay short-circuit if vulnerable
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jeffwidman committed Dec 5, 2022
1 parent 095ab2a commit a0909c7
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions bin/dry-run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -679,11 +679,6 @@ def security_fix?(dependency)
next
end

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 " \
Expand All @@ -693,6 +688,11 @@ def security_fix?(dependency)
end
end

if checker.up_to_date?
puts " (no update needed as it's already up-to-date)"
next
end

latest_allowed_version = if checker.vulnerable?
checker.lowest_resolvable_security_fix_version
else
Expand Down

0 comments on commit a0909c7

Please sign in to comment.