Skip to content

Commit 4208a9c

Browse files
kbukum1kbukum1
committed
update security advisory filter
1 parent c7c8ed2 commit 4208a9c

File tree

3 files changed

+131
-80
lines changed

3 files changed

+131
-80
lines changed

‎maven/lib/dependabot/maven/package/package_details_fetcher.rb

+3-3
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def initialize(dependency:, dependency_files:, credentials:) # rubocop:disable M
4343
@dependency_metadata_from_html = T.let({}, T::Hash[T.untyped, Nokogiri::HTML::Document])
4444
@repository_finder = T.let(nil, T.nilable(Maven::FileParser::RepositoriesFinder))
4545
@repositories = T.let(nil, T.nilable(T::Array[T::Hash[String, T.untyped]]))
46-
@released_check = T.let({}, T::Hash[Version, T::Boolean])
46+
@released_check = T.let({}, T::Hash[String, T::Boolean])
4747
@auth_headers_finder = T.let(nil, T.nilable(Utils::AuthHeadersFinder))
4848
@dependency_parts = T.let([], T::Array[String])
4949
@dependency_classifier = T.let(nil, T.nilable(String))
@@ -194,7 +194,7 @@ def fetch_dependency_metadata(repository_details)
194194
nil
195195
end
196196

197-
sig { params(version: Version).returns(T::Boolean) }
197+
sig { params(version: String).returns(T::Boolean) }
198198
def released?(version)
199199
@released_check[version] ||=
200200
repositories.any? do |repository_details|
@@ -337,7 +337,7 @@ def dependency_metadata_url(repository_url)
337337
# repository_url: https://repo.maven.apache.org/maven2
338338
# version: 5.0.0
339339
# returns: https://repo.maven.apache.org/maven2/org/junit/jupiter/junit-jupiter-api/5.0.0/junit-jupiter-api-5.0.0.jar
340-
sig { params(repository_url: String, version: Version).returns(String) }
340+
sig { params(repository_url: String, version: String).returns(String) }
341341
def dependency_files_url(repository_url, version)
342342
_, artifact_id = @dependency_parts
343343
url = dependency_base_url(repository_url)

‎maven/lib/dependabot/maven/update_checker/version_finder.rb

+72-23
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# typed: strict
1+
# typed: strong
22
# frozen_string_literal: true
33

44
require "dependabot/update_checkers/version_filters"
@@ -52,36 +52,68 @@ def package_details_fetcher
5252
)
5353
end
5454

55-
sig { returns(T::Array[T::Hash[Symbol, T.untyped]]) }
56-
def versions
57-
package_details_fetcher.versions
55+
sig { returns(Dependabot::Package::PackageDetails) }
56+
def package_details
57+
package_details_fetcher.package_details
5858
end
5959

6060
sig { returns(T.nilable(T::Hash[T.untyped, T.untyped])) }
6161
def latest_version_details
62-
possible_versions = versions
63-
62+
possible_versions = package_details.releases
6463
possible_versions = filter_prereleases(possible_versions)
6564
possible_versions = filter_date_based_versions(possible_versions)
6665
possible_versions = filter_version_types(possible_versions)
6766
possible_versions = filter_ignored_versions(possible_versions)
6867

69-
possible_versions.reverse.find { |v| package_details_fetcher.released?(v.fetch(:version)) }
68+
release = possible_versions.reverse.find do |r|
69+
package_details_fetcher.released?(r.version.version)
70+
end
71+
72+
return unless release
73+
74+
{ version: release.version, source_url: release.url }
75+
end
76+
77+
sig do
78+
params(
79+
possible_versions: T::Array[Dependabot::Package::PackageRelease]
80+
).returns(T::Array[Dependabot::Package::PackageRelease])
81+
end
82+
def filter_vulnerable_versions(possible_versions)
83+
return possible_versions if @security_advisories.empty?
84+
85+
filtered = possible_versions.reject do |release|
86+
security_advisories.any? do |advisory|
87+
advisory.vulnerable?(release.version)
88+
end
89+
end
90+
91+
if possible_versions.count > filtered.count
92+
diff = possible_versions.count - filtered.count
93+
Dependabot.logger.info("Filtered out #{diff} vulnerable versions")
94+
end
95+
96+
filtered
7097
end
7198

7299
sig { returns(T.nilable(T::Hash[T.untyped, T.untyped])) }
73100
def lowest_security_fix_version_details
74-
possible_versions = versions
101+
possible_versions = package_details.releases
75102

76103
possible_versions = filter_prereleases(possible_versions)
77104
possible_versions = filter_date_based_versions(possible_versions)
78105
possible_versions = filter_version_types(possible_versions)
79-
possible_versions = Dependabot::UpdateCheckers::VersionFilters.filter_vulnerable_versions(possible_versions,
80-
security_advisories)
106+
possible_versions = filter_vulnerable_versions(possible_versions)
81107
possible_versions = filter_ignored_versions(possible_versions)
82108
possible_versions = filter_lower_versions(possible_versions)
83109

84-
possible_versions.find { |v| package_details_fetcher.released?(v.fetch(:version)) }
110+
release = possible_versions.find do |r|
111+
package_details_fetcher.released?(r.version.version)
112+
end
113+
114+
return unless release
115+
116+
{ version: release.version, source_url: release.url }
85117
end
86118

87119
private
@@ -99,31 +131,42 @@ def lowest_security_fix_version_details
99131
sig { returns(T::Array[Dependabot::SecurityAdvisory]) }
100132
attr_reader :security_advisories
101133

102-
sig { params(possible_versions: T::Array[T.untyped]).returns(T::Array[T.untyped]) }
134+
sig do
135+
params(possible_versions: T::Array[Dependabot::Package::PackageRelease])
136+
.returns(T::Array[Dependabot::Package::PackageRelease])
137+
end
103138
def filter_prereleases(possible_versions)
104139
return possible_versions if wants_prerelease?
105140

106-
filtered = possible_versions.reject { |v| v.fetch(:version).prerelease? }
141+
filtered = possible_versions.reject { |release| release.version.prerelease? }
107142
if possible_versions.count > filtered.count
108143
Dependabot.logger.info("Filtered out #{possible_versions.count - filtered.count} pre-release versions")
109144
end
110145
filtered
111146
end
112147

113-
sig { params(possible_versions: T::Array[T.untyped]).returns(T::Array[T.untyped]) }
148+
sig do
149+
params(possible_versions: T::Array[Dependabot::Package::PackageRelease])
150+
.returns(T::Array[Dependabot::Package::PackageRelease])
151+
end
114152
def filter_date_based_versions(possible_versions)
115153
return possible_versions if wants_date_based_version?
116154

117-
filtered = possible_versions.reject { |v| v.fetch(:version) > version_class.new(1900) }
155+
filtered = possible_versions.reject { |release| release.version > version_class.new(1900) }
118156
if possible_versions.count > filtered.count
119157
Dependabot.logger.info("Filtered out #{possible_versions.count - filtered.count} date-based versions")
120158
end
121159
filtered
122160
end
123161

124-
sig { params(possible_versions: T::Array[T.untyped]).returns(T::Array[T.untyped]) }
162+
sig do
163+
params(possible_versions: T::Array[Dependabot::Package::PackageRelease])
164+
.returns(T::Array[Dependabot::Package::PackageRelease])
165+
end
125166
def filter_version_types(possible_versions)
126-
filtered = possible_versions.select { |v| matches_dependency_version_type?(v.fetch(:version)) }
167+
filtered = possible_versions.select do |release|
168+
matches_dependency_version_type?(release.version)
169+
end
127170
if possible_versions.count > filtered.count
128171
diff = possible_versions.count - filtered.count
129172
classifier = dependency.version&.split(/[.\-]/)&.last
@@ -132,15 +175,18 @@ def filter_version_types(possible_versions)
132175
filtered
133176
end
134177

135-
sig { params(possible_versions: T::Array[T.untyped]).returns(T::Array[T.untyped]) }
178+
sig do
179+
params(possible_versions: T::Array[Dependabot::Package::PackageRelease])
180+
.returns(T::Array[Dependabot::Package::PackageRelease])
181+
end
136182
def filter_ignored_versions(possible_versions)
137183
filtered = possible_versions
138184

139185
ignored_versions.each do |req|
140186
ignore_requirements = Maven::Requirement.requirements_array(req)
141187
filtered =
142188
filtered
143-
.reject { |v| ignore_requirements.any? { |r| r.satisfied_by?(v.fetch(:version)) } }
189+
.reject { |release| ignore_requirements.any? { |r| r.satisfied_by?(release.version) } }
144190
end
145191

146192
if @raise_on_ignored && filter_lower_versions(filtered).empty? &&
@@ -156,12 +202,15 @@ def filter_ignored_versions(possible_versions)
156202
filtered
157203
end
158204

159-
sig { params(possible_versions: T::Array[T.untyped]).returns(T::Array[T.untyped]) }
205+
sig do
206+
params(possible_versions: T::Array[Dependabot::Package::PackageRelease])
207+
.returns(T::Array[Dependabot::Package::PackageRelease])
208+
end
160209
def filter_lower_versions(possible_versions)
161210
return possible_versions unless dependency.numeric_version
162211

163-
possible_versions.select do |v|
164-
v.fetch(:version) > dependency.numeric_version
212+
possible_versions.select do |release|
213+
release.version > dependency.numeric_version
165214
end
166215
end
167216

@@ -179,7 +228,7 @@ def wants_date_based_version?
179228
T.must(dependency.numeric_version) >= version_class.new(100)
180229
end
181230

182-
sig { params(comparison_version: Version).returns(T::Boolean) }
231+
sig { params(comparison_version: Dependabot::Version).returns(T::Boolean) }
183232
def matches_dependency_version_type?(comparison_version)
184233
return true unless dependency.version
185234

‎maven/spec/dependabot/maven/update_checker/version_finder_spec.rb

+56-54
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,9 @@
658658
let(:raise_on_ignored) { true }
659659

660660
it "raises an error" do
661-
expect { lowest_security_fix_version_details }.to raise_error(Dependabot::AllVersionsIgnored)
661+
expect { lowest_security_fix_version_details }.to raise_error(
662+
Dependabot::AllVersionsIgnored
663+
)
662664
end
663665
end
664666
end
@@ -676,35 +678,43 @@
676678
end
677679
end
678680

679-
describe "#versions" do
680-
subject(:versions) { finder.versions }
681+
describe "#package_details" do
682+
subject(:package_details) { finder.package_details }
681683

682-
its(:count) { is_expected.to eq(70) }
684+
it "returns package details" do
685+
expect(package_details).to be_a(Dependabot::Package::PackageDetails)
686+
end
683687

684-
describe "the first version" do
685-
subject { versions.first }
688+
# Since the `releases` is the new `versions`, we assert the count of releases
689+
its(:releases) { is_expected.to have(70).items }
686690

687-
its([:version]) { is_expected.to eq(version_class.new("r03")) }
691+
describe "the first release" do
692+
subject { package_details.releases.first }
688693

689-
its([:source_url]) do
690-
is_expected.to eq("https://repo.maven.apache.org/maven2")
691-
end
694+
its(:version) { is_expected.to eq(version_class.new("r03")) }
695+
696+
its(:url) { is_expected.to eq("https://repo.maven.apache.org/maven2") }
697+
its(:released_at) { is_expected.to eq(Time.parse("2017-09-10 18:08")) }
698+
its(:yanked?) { is_expected.to be(false) }
699+
its(:latest) { is_expected.to be(false) }
692700
end
693701

694-
describe "the last version" do
695-
subject { versions.last }
702+
describe "the last release" do
703+
subject { package_details.releases.last }
696704

697-
its([:version]) { is_expected.to eq(version_class.new("23.7-rc1-jre")) }
705+
its(:version) { is_expected.to eq(version_class.new("23.7-rc1-jre")) }
698706

699-
its([:source_url]) do
700-
is_expected.to eq("https://repo.maven.apache.org/maven2")
701-
end
707+
its(:url) { is_expected.to eq("https://repo.maven.apache.org/maven2") }
708+
its(:released_at) { is_expected.to eq(Time.parse("2020-01-01 00:00")) }
709+
its(:yanked?) { is_expected.to be(false) }
710+
its(:latest) { is_expected.to be(false) }
702711
end
703712

704713
context "with a dependency name that needs URI encoding" do
705714
let(:dependency_name) { "bad com.google.guava:guava" }
706715

707-
its(:count) { is_expected.to eq(0) }
716+
# Expect that no releases exist for this dependency
717+
its(:releases) { is_expected.to be_empty }
708718
end
709719

710720
context "with a custom repository" do
@@ -739,58 +749,50 @@
739749
.to_return(status: 404, body: "")
740750
end
741751

742-
describe "the first version" do
743-
subject { versions.first }
752+
describe "the first release" do
753+
subject { package_details.releases.first }
744754

745-
its([:version]) { is_expected.to eq(version_class.new("r03")) }
755+
its(:version) { is_expected.to eq(version_class.new("r03")) }
746756

747-
its([:source_url]) do
748-
is_expected.to eq("http://repository.jboss.org/maven2")
749-
end
757+
its(:url) { is_expected.to eq("http://repository.jboss.org/maven2") }
750758
end
751759

752-
describe "the last version" do
753-
subject { versions.last }
760+
describe "the last release" do
761+
subject { package_details.releases.last }
754762

755-
its([:version]) { is_expected.to eq(version_class.new("23.7-rc1-jre")) }
763+
its(:version) { is_expected.to eq(version_class.new("23.7-rc1-jre")) }
756764

757-
its([:source_url]) do
758-
is_expected.to eq("http://repository.jboss.org/maven2")
759-
end
765+
its(:url) { is_expected.to eq("http://repository.jboss.org/maven2") }
760766
end
767+
end
761768

762-
context "when augmenting the central repo" do
763-
before do
764-
body =
765-
fixture("maven_central_metadata", "with_date_releases.xml")
766-
stub_request(:get, maven_central_metadata_url)
767-
.to_return(status: 200, body: body)
768-
# 404 causes Dependabot to fall back to the central repo
769-
stub_request(:get, jboss_metadata_url)
770-
.to_return(status: 404)
771-
end
769+
context "when augmenting the central repo" do
770+
before do
771+
body =
772+
fixture("maven_central_metadata", "with_date_releases.xml")
773+
stub_request(:get, maven_central_metadata_url)
774+
.to_return(status: 200, body: body)
775+
# 404 causes Dependabot to fall back to the central repo
776+
stub_request(:get, jboss_metadata_url)
777+
.to_return(status: 404)
778+
end
772779

773-
its(:count) { is_expected.to eq(17) }
780+
its(:releases) { is_expected.to have(17).items }
774781

775-
describe "the first version" do
776-
subject { versions.first }
782+
describe "the first release" do
783+
subject { package_details.releases.first }
777784

778-
its([:version]) { is_expected.to eq(version_class.new("r01")) }
785+
its(:version) { is_expected.to eq(version_class.new("r01")) }
779786

780-
its([:source_url]) do
781-
is_expected.to eq("https://repo.maven.apache.org/maven2")
782-
end
783-
end
787+
its(:url) { is_expected.to eq("https://repo.maven.apache.org/maven2") }
788+
end
784789

785-
describe "the last version" do
786-
subject { versions.last }
790+
describe "the last release" do
791+
subject { package_details.releases.last }
787792

788-
its([:version]) { is_expected.to eq(version_class.new("20040616")) }
793+
its(:version) { is_expected.to eq(version_class.new("20040616")) }
789794

790-
its([:source_url]) do
791-
is_expected.to eq("https://repo.maven.apache.org/maven2")
792-
end
793-
end
795+
its(:url) { is_expected.to eq("https://repo.maven.apache.org/maven2") }
794796
end
795797
end
796798
end

0 commit comments

Comments
 (0)