diff --git a/config/feature_flags/development/vulnerability_location_image_filter.yml b/config/feature_flags/development/vulnerability_location_image_filter.yml new file mode 100644 index 0000000000000000000000000000000000000000..4b373b76ff69c185b2a945f0fca47dd2db800332 --- /dev/null +++ b/config/feature_flags/development/vulnerability_location_image_filter.yml @@ -0,0 +1,8 @@ +--- +name: vulnerability_location_image_filter +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69867 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/340915 +milestone: '14.4' +type: development +group: group::container security +default_enabled: false diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 93e165208fb1396564eacddd722f7df835ef8df3..6ffc4de5eff577734e12186a621e3fd80cec2bc8 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -469,6 +469,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | <a id="queryvulnerabilitieshasissues"></a>`hasIssues` | [`Boolean`](#boolean) | Returns only the vulnerabilities which have linked issues. | | <a id="queryvulnerabilitieshasresolution"></a>`hasResolution` | [`Boolean`](#boolean) | Returns only the vulnerabilities which have been resolved on default branch. | +| <a id="queryvulnerabilitiesimage"></a>`image` | [`[String!]`](#string) | Filter vulnerabilities by location image. When this filter is present, the response only matches entries for a `reportType` that includes `container_scanning`, `cluster_image_scanning`. | | <a id="queryvulnerabilitiesprojectid"></a>`projectId` | [`[ID!]`](#id) | Filter vulnerabilities by project. | | <a id="queryvulnerabilitiesreporttype"></a>`reportType` | [`[VulnerabilityReportType!]`](#vulnerabilityreporttype) | Filter vulnerabilities by report type. | | <a id="queryvulnerabilitiesscanner"></a>`scanner` | [`[String!]`](#string) | Filter vulnerabilities by VulnerabilityScanner.externalId. | @@ -10527,6 +10528,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | <a id="groupvulnerabilitieshasissues"></a>`hasIssues` | [`Boolean`](#boolean) | Returns only the vulnerabilities which have linked issues. | | <a id="groupvulnerabilitieshasresolution"></a>`hasResolution` | [`Boolean`](#boolean) | Returns only the vulnerabilities which have been resolved on default branch. | +| <a id="groupvulnerabilitiesimage"></a>`image` | [`[String!]`](#string) | Filter vulnerabilities by location image. When this filter is present, the response only matches entries for a `reportType` that includes `container_scanning`, `cluster_image_scanning`. | | <a id="groupvulnerabilitiesprojectid"></a>`projectId` | [`[ID!]`](#id) | Filter vulnerabilities by project. | | <a id="groupvulnerabilitiesreporttype"></a>`reportType` | [`[VulnerabilityReportType!]`](#vulnerabilityreporttype) | Filter vulnerabilities by report type. | | <a id="groupvulnerabilitiesscanner"></a>`scanner` | [`[String!]`](#string) | Filter vulnerabilities by VulnerabilityScanner.externalId. | @@ -13203,6 +13205,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | <a id="projectvulnerabilitieshasissues"></a>`hasIssues` | [`Boolean`](#boolean) | Returns only the vulnerabilities which have linked issues. | | <a id="projectvulnerabilitieshasresolution"></a>`hasResolution` | [`Boolean`](#boolean) | Returns only the vulnerabilities which have been resolved on default branch. | +| <a id="projectvulnerabilitiesimage"></a>`image` | [`[String!]`](#string) | Filter vulnerabilities by location image. When this filter is present, the response only matches entries for a `reportType` that includes `container_scanning`, `cluster_image_scanning`. | | <a id="projectvulnerabilitiesprojectid"></a>`projectId` | [`[ID!]`](#id) | Filter vulnerabilities by project. | | <a id="projectvulnerabilitiesreporttype"></a>`reportType` | [`[VulnerabilityReportType!]`](#vulnerabilityreporttype) | Filter vulnerabilities by report type. | | <a id="projectvulnerabilitiesscanner"></a>`scanner` | [`[String!]`](#string) | Filter vulnerabilities by VulnerabilityScanner.externalId. | diff --git a/ee/app/finders/security/vulnerabilities_finder.rb b/ee/app/finders/security/vulnerabilities_finder.rb index 78532ddc31a50f21c32c56f2dd569df42c9d6591..d2445772d6e86c43f72e4dd58522260b3ed418b1 100644 --- a/ee/app/finders/security/vulnerabilities_finder.rb +++ b/ee/app/finders/security/vulnerabilities_finder.rb @@ -9,10 +9,11 @@ # params: optional! a hash with one or more of the following: # project_ids: if `vulnerable` includes multiple projects (like a Group), this filter will restrict # the vulnerabilities returned to those in the group's projects that also match these IDs +# image: only return vulnerabilities with these location images # report_types: only return vulnerabilities from these report types # severities: only return vulnerabilities with these severities # states: only return vulnerabilities in these states -# has_resolution: only return vulnerabilities thah have resolution +# has_resolution: only return vulnerabilities that have resolution # has_issues: only return vulnerabilities that have issues linked # sort: return vulnerabilities ordered by severity_asc or severity_desc @@ -22,11 +23,13 @@ class VulnerabilitiesFinder def initialize(vulnerable, params = {}) @params = params + @vulnerable = vulnerable @vulnerabilities = vulnerable.vulnerabilities end def execute filter_by_projects + filter_by_image filter_by_report_types filter_by_severities filter_by_states @@ -40,7 +43,7 @@ def execute private - attr_reader :params, :vulnerabilities + attr_reader :params, :vulnerable, :vulnerabilities def filter_by_projects if params[:project_id].present? @@ -90,6 +93,15 @@ def filter_by_issues end end + def filter_by_image + # This filter will not work for InstanceSecurityDashboard, because InstanceSecurityDashboard could have multiple projects. + return if vulnerable.is_a?(InstanceSecurityDashboard) + + if params[:image].present? && Feature.enabled?(:vulnerability_location_image_filter, vulnerable, default_enabled: :yaml) + @vulnerabilities = vulnerabilities.with_container_image(params[:image]) + end + end + def sort(items) items.order_by(params[:sort]) end diff --git a/ee/app/graphql/resolvers/vulnerabilities_resolver.rb b/ee/app/graphql/resolvers/vulnerabilities_resolver.rb index eea15057689c8ddb62ae7e0ca4142b6484438bbd..215baf52836bf98753ba97bd19ebdc86b6d8012c 100644 --- a/ee/app/graphql/resolvers/vulnerabilities_resolver.rb +++ b/ee/app/graphql/resolvers/vulnerabilities_resolver.rb @@ -43,6 +43,12 @@ class VulnerabilitiesResolver < VulnerabilitiesBaseResolver required: false, description: 'Returns only the vulnerabilities which have linked issues.' + argument :image, [GraphQL::Types::String], + required: false, + description: "Filter vulnerabilities by location image. When this filter is present, "\ + "the response only matches entries for a `reportType` "\ + "that includes #{::Vulnerabilities::Finding::REPORT_TYPES_WITH_LOCATION_IMAGE.map { |type| "`#{type}`" }.join(', ')}." + def resolve(**args) return Vulnerability.none unless vulnerable diff --git a/ee/app/models/ee/vulnerability.rb b/ee/app/models/ee/vulnerability.rb index fe61c5111dafffa01561b6716f3a98882a25463c..0c5cdb790a5272ba912a3f927731a37d7476e9ad 100644 --- a/ee/app/models/ee/vulnerability.rb +++ b/ee/app/models/ee/vulnerability.rb @@ -119,6 +119,9 @@ def with_vulnerability_links scope :order_id_desc, -> { reorder(id: :desc) } scope :with_limit, -> (maximum) { limit(maximum) } + scope :with_container_image, -> (images) do + joins(:findings).merge(Vulnerabilities::Finding.by_location_image(images)) + end delegate :scanner_name, :scanner_external_id, :scanner_id, :metadata, :message, :description, :details, to: :finding, prefix: true, allow_nil: true diff --git a/ee/app/models/vulnerabilities/finding.rb b/ee/app/models/vulnerabilities/finding.rb index cfe91e633084e0f50dde8a64212f500e8d46ed89..d6c0a6fcd8ac8008244ac48e471f734a4d1091aa 100644 --- a/ee/app/models/vulnerabilities/finding.rb +++ b/ee/app/models/vulnerabilities/finding.rb @@ -13,6 +13,7 @@ class Finding < ApplicationRecord FINDINGS_PER_PAGE = 20 MAX_NUMBER_OF_IDENTIFIERS = 20 + REPORT_TYPES_WITH_LOCATION_IMAGE = %w[container_scanning cluster_image_scanning].freeze paginates_per FINDINGS_PER_PAGE @@ -95,6 +96,10 @@ class Finding < ApplicationRecord scope :scoped_project, -> { where('vulnerability_occurrences.project_id = projects.id') } scope :eager_load_vulnerability_flags, -> { includes(:vulnerability_flags) } + scope :by_location_image, -> (images) do + where(report_type: REPORT_TYPES_WITH_LOCATION_IMAGE) + .where("vulnerability_occurrences.location -> 'image' ?| array[:images]", images: images) + end def self.for_pipelines(pipelines) joins(:finding_pipelines) diff --git a/ee/spec/factories/vulnerabilities/findings.rb b/ee/spec/factories/vulnerabilities/findings.rb index a5082fd451f1cda9e0becb1b5c11e8227abeb98a..ce3b18261f96de744b7e3a417543ada3da0466eb 100644 --- a/ee/spec/factories/vulnerabilities/findings.rb +++ b/ee/spec/factories/vulnerabilities/findings.rb @@ -520,6 +520,55 @@ end end + trait :with_cluster_image_scanning_scanning_metadata do + after(:build) do |finding, _| + finding.report_type = "cluster_image_scanning" + finding.name = "CVE-2017-16997 in libc" + finding.metadata_version = "2.3" + finding.location = { + "dependency": { + "package": { + "name": "glibc" + }, + "version": "2.24-11+deb9u3" + }, + "operating_system": "alpine 3.7", + "image": "alpine:3.7" + } + finding.raw_metadata = { + "category": "cluster_image_scanning", + "name": "CVE-2017-16997 in libc", + "severity": "high", + "solution": "Upgrade glibc from 2.24-11+deb9u3 to 2.24-11+deb9u4", + "scanner": { + "id": "starboard", + "name": "Starboard" + }, + "location": { + "dependency": { + "package": { + "name": "glibc" + }, + "version": "2.24-11+deb9u3" + }, + "operating_system": "alpine 3.7", + "image": "alpine:3.7" + }, + "identifiers": [{ + "type": "cve", + "name": "CVE-2017-16997", + "value": "CVE-2017-16997", + "url": "https://security-tracker.debian.org/tracker/CVE-2017-16997" + }], + "links": [ + { + "url": "https://security-tracker.debian.org/tracker/CVE-2017-16997" + } + ] + }.to_json + end + end + trait :identifier do after(:build) do |finding| identifier = build( diff --git a/ee/spec/finders/security/vulnerabilities_finder_spec.rb b/ee/spec/finders/security/vulnerabilities_finder_spec.rb index 909b9a1008360415f49cb4ba707491eb1b125b0c..4a26b38b820db5490d824bb0f8320414eb6e71d9 100644 --- a/ee/spec/finders/security/vulnerabilities_finder_spec.rb +++ b/ee/spec/finders/security/vulnerabilities_finder_spec.rb @@ -157,4 +157,46 @@ is_expected.to contain_exactly(vulnerability4) end end + + context 'when filtered by image' do + let_it_be(:cluster_vulnerability) { create(:vulnerability, :cluster_image_scanning, project: project) } + let_it_be(:finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, vulnerability: cluster_vulnerability) } + + let(:filters) { { image: [finding.location['image']] } } + let(:feature_enabled) { true } + + before do + stub_feature_flags(vulnerability_location_image_filter: feature_enabled) + end + + context 'when vulnerability_location_image_filter is disabled' do + let(:feature_enabled) { false } + + it 'does not include cluster vulnerability' do + is_expected.not_to contain_exactly(cluster_vulnerability) + end + end + + context 'when vulnerability_location_image_filter is enabled' do + it 'only returns vulnerabilities matching the given image' do + is_expected.to contain_exactly(cluster_vulnerability) + end + + context 'when different report_type is passed' do + let(:filters) { { report_type: %w[dast], image: [finding.location['image']] }} + + it 'returns empty list' do + is_expected.to be_empty + end + end + end + + context 'when vulnerable is InstanceSecurityDashboard' do + let(:vulnerable) { InstanceSecurityDashboard.new(project.users.first) } + + it 'does not include cluster vulnerability' do + is_expected.not_to contain_exactly(cluster_vulnerability) + end + end + end end diff --git a/ee/spec/graphql/resolvers/vulnerabilities_resolver_spec.rb b/ee/spec/graphql/resolvers/vulnerabilities_resolver_spec.rb index 513b491b9dfe3d3747588214a32ba73f8b144fd2..b854661a82ba1a3627d661de86265d5a49e897e8 100644 --- a/ee/spec/graphql/resolvers/vulnerabilities_resolver_spec.rb +++ b/ee/spec/graphql/resolvers/vulnerabilities_resolver_spec.rb @@ -191,5 +191,24 @@ end end end + + context 'when image is given' do + let_it_be(:cluster_vulnerability) { create(:vulnerability, :cluster_image_scanning, project: project) } + let_it_be(:cluster_finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, vulnerability: cluster_vulnerability) } + + let(:params) { { image: [cluster_finding.location['image']] } } + + it 'only returns vulnerabilities with given image' do + is_expected.to contain_exactly(cluster_vulnerability) + end + + context 'when different report_type is given along with image' do + let(:params) { { report_type: %w[sast], image: [cluster_finding.location['image']] } } + + it 'returns empty list' do + is_expected.to be_empty + end + end + end end end diff --git a/ee/spec/models/ee/vulnerability_spec.rb b/ee/spec/models/ee/vulnerability_spec.rb index 291535fdc1ef1728b6a1468060ec5a9255c16b6a..7e2b7a68f0e48be5e97dcee6d685ba0633d56e49 100644 --- a/ee/spec/models/ee/vulnerability_spec.rb +++ b/ee/spec/models/ee/vulnerability_spec.rb @@ -579,6 +579,28 @@ it { is_expected.not_to match("gitlab-org/gitlab-foss/milestones/123") } end + describe '.with_container_image' do + let_it_be(:vulnerability) { create(:vulnerability, project: project, report_type: 'cluster_image_scanning') } + let_it_be(:finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, vulnerability: vulnerability) } + let_it_be(:image) { finding.location['image'] } + + before do + finding_with_different_image = create( + :vulnerabilities_finding, + :with_cluster_image_scanning_scanning_metadata, + vulnerability: create(:vulnerability, report_type: 'cluster_image_scanning') + ) + finding_with_different_image.location['image'] = 'alpine:latest' + finding_with_different_image.save! + end + + subject(:cluster_vulnerabilities) { described_class.with_container_image(image) } + + it 'returns vulnerabilities with given image' do + expect(cluster_vulnerabilities).to contain_exactly(vulnerability) + end + end + describe 'created_in_time_range' do it 'returns vulnerabilities created in given time range', :aggregate_failures do record1 = create(:vulnerability, created_at: 1.day.ago) diff --git a/ee/spec/models/vulnerabilities/finding_spec.rb b/ee/spec/models/vulnerabilities/finding_spec.rb index e6ffa635ceb0c98705c50ace0ea20467f5137598..b714fd066b5229d421c459d9a28c6c90c10f3ee8 100644 --- a/ee/spec/models/vulnerabilities/finding_spec.rb +++ b/ee/spec/models/vulnerabilities/finding_spec.rb @@ -340,6 +340,30 @@ end end + describe '.by_location_image' do + let_it_be(:vulnerability) { create(:vulnerability, report_type: 'cluster_image_scanning') } + let_it_be(:finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, vulnerability: vulnerability) } + let_it_be(:image) { finding.location['image'] } + + before do + finding_with_different_image = create( + :vulnerabilities_finding, + :with_cluster_image_scanning_scanning_metadata, + vulnerability: create(:vulnerability, report_type: 'cluster_image_scanning') + ) + finding_with_different_image.location['image'] = 'alpine:latest' + finding_with_different_image.save! + + create(:vulnerabilities_finding, report_type: :dast) + end + + subject(:cluster_findings) { described_class.by_location_image(image) } + + it 'returns findings with given image' do + expect(cluster_findings).to contain_exactly(finding) + end + end + describe '#false_positive?' do let_it_be(:finding) { create(:vulnerabilities_finding) } let_it_be(:finding_with_fp) { create(:vulnerabilities_finding, vulnerability_flags: [create(:vulnerabilities_flag)]) }