Skip to content

Find feedback based on UUID

What does this MR do and why?

This MR was created to use the finding_uuid field to connect a vulnerability feedback with a finding. The current usage of project_fingerprint is problematic as it can associate multiple findings to the same feedback, resulting in bugs as described in Dismissal of single SAST IaC Pipeline Finding r... (#350683 - closed).

Screenshots or screen recordings

Before change

Screen_Shot_2022-03-11_at_4.00.19_PM Screen_Shot_2022-03-11_at_4.01.13_PM

After Change

Screen_Shot_2022-03-11_at_3.58.37_PM Screen_Shot_2022-03-11_at_3.58.58_PM

Query Plans

ee/app/finders/security/findings_finder.rb

existing_vulnerabilities

https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/9526/commands/33992

explain SELECT "vulnerabilities"."id" AS t0_r0, "vulnerabilities"."milestone_id" AS t0_r1, "vulnerabilities"."epic_id" AS t0_r2, "vulnerabilities"."project_id" AS t0_r3, "vulnerabilities"."author_id" AS t0_r4, "vulnerabilities"."updated_by_id" AS t0_r5, "vulnerabilities"."last_edited_by_id" AS t0_r6, "vulnerabilities"."start_date" AS t0_r7, "vulnerabilities"."due_date" AS t0_r8, "vulnerabilities"."last_edited_at" AS t0_r9, "vulnerabilities"."created_at" AS t0_r10, "vulnerabilities"."updated_at" AS t0_r11, "vulnerabilities"."title" AS t0_r12, "vulnerabilities"."title_html" AS t0_r13, "vulnerabilities"."description" AS t0_r14, "vulnerabilities"."description_html" AS t0_r15, "vulnerabilities"."start_date_sourcing_milestone_id" AS t0_r16, "vulnerabilities"."due_date_sourcing_milestone_id" AS t0_r17, "vulnerabilities"."state" AS t0_r18, "vulnerabilities"."severity" AS t0_r19, "vulnerabilities"."severity_overridden" AS t0_r20, "vulnerabilities"."confidence" AS t0_r21, "vulnerabilities"."confidence_overridden" AS t0_r22, "vulnerabilities"."resolved_by_id" AS t0_r23, "vulnerabilities"."resolved_at" AS t0_r24, "vulnerabilities"."report_type" AS t0_r25, "vulnerabilities"."cached_markdown_version" AS t0_r26, "vulnerabilities"."confirmed_by_id" AS t0_r27, "vulnerabilities"."confirmed_at" AS t0_r28, "vulnerabilities"."dismissed_at" AS t0_r29, "vulnerabilities"."dismissed_by_id" AS t0_r30, "vulnerabilities"."resolved_on_default_branch" AS t0_r31, "vulnerabilities"."present_on_default_branch" AS t0_r32, "vulnerabilities"."detected_at" AS t0_r33, "findings"."id" AS t1_r0, "findings"."created_at" AS t1_r1, "findings"."updated_at" AS t1_r2, "findings"."severity" AS t1_r3, "findings"."confidence" AS t1_r4, "findings"."report_type" AS t1_r5, "findings"."project_id" AS t1_r6, "findings"."scanner_id" AS t1_r7, "findings"."primary_identifier_id" AS t1_r8, "findings"."project_fingerprint" AS t1_r9, "findings"."location_fingerprint" AS t1_r10, "findings"."uuid" AS t1_r11, "findings"."name" AS t1_r12, "findings"."metadata_version" AS t1_r13, "findings"."raw_metadata" AS t1_r14, "findings"."vulnerability_id" AS t1_r15, "findings"."details" AS t1_r16, "findings"."description" AS t1_r17, "findings"."message" AS t1_r18, "findings"."solution" AS t1_r19, "findings"."cve" AS t1_r20, "findings"."location" AS t1_r21, "findings"."detection_method" AS t1_r22, "findings"."migrated_to_new_structure" AS t1_r23 FROM "vulnerabilities" LEFT OUTER JOIN "vulnerability_occurrences" "findings" ON "findings"."vulnerability_id" = "vulnerabilities"."id" WHERE "vulnerabilities"."project_id" = 23 AND "findings"."uuid" IN  (SELECT "vulnerability_occurrences"."uuid" FROM "vulnerability_occurrences" WHERE "vulnerability_occurrences"."project_id" = 33851038)
Time: 1.808 s  
  - planning: 1.512 ms  
  - execution: 1.807 s  
    - I/O read: 1.791 s  
    - I/O write: 0.000 ms  
  
Shared buffers:  
  - hits: 1401 (~10.90 MiB) from the buffer pool  
  - reads: 363 (~2.80 MiB) from the OS file cache, including disk I/O  
  - dirtied: 0  
  - writes: 0  

ee/app/models/security/finding.rb

undismissed

https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/9526/commands/33968

SELECT
    "security_findings".*
FROM
    "security_findings"
    INNER JOIN "security_scans" ON "security_findings"."scan_id" = "security_scans"."id"
WHERE
    "security_scans"."pipeline_id" = 251
    AND (NOT EXISTS (
            SELECT
                1
            FROM
                "security_scans"
                INNER JOIN "projects" ON "projects"."id" = "security_scans"."project_id"
                INNER JOIN "vulnerability_feedback" ON "vulnerability_feedback"."project_id" = "projects"."id"
            WHERE (vulnerability_feedback.category = (security_scans.scan_type - 1))
            AND "vulnerability_feedback"."feedback_type" = 0
            AND (security_scans.id = security_findings.scan_id)
            AND (vulnerability_feedback.finding_uuid = security_findings.uuid)))
Time: 25.149 ms  
  - planning: 22.899 ms  
  - execution: 2.250 ms  
    - I/O read: 2.042 ms  
    - I/O write: 0.000 ms  
  
Shared buffers:  
  - hits: 4 (~32.00 KiB) from the buffer pool  
  - reads: 3 (~24.00 KiB) from the OS file cache, including disk I/O  
  - dirtied: 0  
  - writes: 0  

ee/app/models/vulnerabilities/finding.rb

self.related_dismissal_feedback

https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/9526/commands/33969

SELECT
    1
FROM
    "vulnerability_feedback",
    "vulnerability_occurrences"
WHERE (vulnerability_occurrences.uuid = vulnerability_feedback.finding_uuid::text)
    AND "vulnerability_feedback"."feedback_type" = 0
Time: 1.366 min  
  - planning: 3.160 ms  
  - execution: 1.366 min  
    - I/O read: 3.923 min  
    - I/O write: 0.000 ms  
  
Shared buffers:  
  - hits: 1284470 (~9.80 GiB) from the buffer pool  
  - reads: 199435 (~1.50 GiB) from the OS file cache, including disk I/O  
  - dirtied: 15113 (~118.10 MiB)  
  - writes: 0  

load_feedback

NOTE: Because I had to pull the UUIDs from the DB in the query (where normally the uuids are provided already), it added additional time and complexity that wouldn't exist in the actual call from the method.

https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/9526/commands/33976

SELECT
    "vulnerability_feedback".*
FROM
    "vulnerability_feedback"
WHERE
    "vulnerability_feedback"."finding_uuid" IN (
        SELECT
            uuid("vulnerability_occurrences"."uuid")
        FROM
            "vulnerability_occurrences"
        WHERE
            "vulnerability_occurrences"."project_id" = 33851038)
Time: 14.291 ms  
  - planning: 3.850 ms  
  - execution: 10.441 ms  
    - I/O read: 8.662 ms  
    - I/O write: 0.000 ms  
  
Shared buffers:  
  - hits: 332 (~2.60 MiB) from the buffer pool  
  - reads: 312 (~2.40 MiB) from the OS file cache, including disk I/O  
  - dirtied: 0  
  - writes: 0  

How to set up and validate locally

  1. Switch to master branch.
  2. Import https://gitlab.com/gitlab-org/security-products/tests/ansible
  3. Simplify CI (simultaneously triggering new pipeline) https://gitlab.com/theoretick/ansible-goop/-/commit/a352885d42fc5535b587ecfb6d6d6da99eecfe00
  4. Once pipeline has completed, dismiss a single finding on pipeline security tab
  5. Reload page
  6. Note all findings have been dismissed
  7. Switch to 350683-dismissal-of-single-sast-iac-pipeline-finding-results-in-mass-dismissal branch
  8. Reload page
  9. Note only the previously dismissed finding has been dismissed

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #350683 (closed)

Edited by Jonathan Schafer

Merge request reports