Skip to content

Commit

Permalink
Merge pull request #5837 from dependabot/mctofu/2855-vuln-multi-version
Browse files Browse the repository at this point in the history
Consider all dependency versions in Job.vulnerable?
  • Loading branch information
mctofu authored Oct 6, 2022
2 parents 5efcefa + 2718fda commit 742fe5b
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 7 deletions.
9 changes: 9 additions & 0 deletions common/lib/dependabot/dependency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,15 @@ def display_name
display_name_builder.call(name)
end

# Returns all detected versions of the dependency. Only ecosystems that
# support this feature will return more than the current version.
def all_versions
all_versions = metadata[:all_versions]
return [version].compact unless all_versions

all_versions.filter_map(&:version)
end

def ==(other)
other.instance_of?(self.class) && to_h == other.to_h
end
Expand Down
66 changes: 66 additions & 0 deletions common/spec/dependabot/dependency_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -303,5 +303,71 @@
)
expect(dependency.to_h.keys).not_to include("metadata")
end

it "isn't utilized by the equality operator" do
dependency1 = described_class.new(
name: "dep",
requirements: [],
package_manager: "dummy",
metadata: { foo: 42 }
)
dependency2 = described_class.new(
name: "dep",
requirements: [],
package_manager: "dummy",
metadata: { foo: 43 }
)
expect(dependency1).to eq(dependency2)
end
end

describe "#all_versions" do
it "returns an empty array by default" do
dependency = described_class.new(
name: "dep",
requirements: [],
package_manager: "dummy"
)

expect(dependency.all_versions).to eq([])
end

it "returns the dependency version if all_version metadata isn't present" do
dependency = described_class.new(
name: "dep",
requirements: [],
package_manager: "dummy",
version: "1.0.0"
)

expect(dependency.all_versions).to eq(["1.0.0"])
end

it "returns all_version metadata if present" do
dependency = described_class.new(
name: "dep",
requirements: [],
package_manager: "dummy",
version: "1.0.0",
metadata: {
all_versions: [
described_class.new(
name: "dep",
requirements: [],
package_manager: "dummy",
version: "1.0.0"
),
described_class.new(
name: "dep",
requirements: [],
package_manager: "dummy",
version: "2.0.0"
)
]
}
)

expect(dependency.all_versions).to eq(["1.0.0", "2.0.0"])
end
end
end
7 changes: 2 additions & 5 deletions npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ def conflicting_dependencies
dependency: dependency,
target_version: lowest_security_fix_version
)
return conflicts unless defined?(@vulnerability_audit)

vulnerable = [vulnerability_audit].select do |hash|
!hash["fix_available"] && hash["explanation"]
Expand All @@ -144,10 +143,8 @@ def vulnerability_audit
def vulnerable_versions
@vulnerable_versions ||=
begin
all_versions = Set.new([
dependency.version,
*dependency.metadata.fetch(:all_versions, []).filter_map(&:version)
]).filter_map { |v| version_class.new(v) if version_class.correct?(v) }
all_versions = dependency.all_versions.
filter_map { |v| version_class.new(v) if version_class.correct?(v) }

all_versions.select do |v|
security_advisories.any? { |advisory| advisory.vulnerable?(v) }
Expand Down
5 changes: 3 additions & 2 deletions updater/lib/dependabot/job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ def vulnerable?(dependency)
version_class_for_package_manager(dependency.package_manager)
return false unless version_class.correct?(dependency.version)

version = version_class.new(dependency.version)
security_advisories.any? { |a| a.vulnerable?(version) }
all_versions = dependency.all_versions.
filter_map { |v| version_class.new(v) if version_class.correct?(v) }
security_advisories.any? { |a| all_versions.any? { |v| a.vulnerable?(v) } }
end

def security_fix?(dependency)
Expand Down
47 changes: 47 additions & 0 deletions updater/spec/dependabot/job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,53 @@

it { is_expected.to eq(true) }
end

context "for a security fix that doesn't apply" do
let(:security_advisories) do
[
{
"dependency-name" => "business",
"affected-versions" => ["> 1.8.0"],
"patched-versions" => [],
"unaffected-versions" => []
}
]
end

it { is_expected.to eq(false) }
end

context "for a security fix that doesn't apply to some versions" do
let(:security_advisories) do
[
{
"dependency-name" => "business",
"affected-versions" => ["> 1.8.0"],
"patched-versions" => [],
"unaffected-versions" => []
}
]
end

it "should be allowed" do
dependency.metadata[:all_versions] = [
Dependabot::Dependency.new(
name: dependency_name,
package_manager: "bundler",
version: "1.8.0",
requirements: []
),
Dependabot::Dependency.new(
name: dependency_name,
package_manager: "bundler",
version: "1.9.0",
requirements: []
)
]

is_expected.to eq(true)
end
end
end

context "and a dependency whitelist that includes the dependency" do
Expand Down

0 comments on commit 742fe5b

Please sign in to comment.