Skip to content

Fix dismissal_reason updates

What does this MR do and why?

Enable modification of the dismissal reason on already dismissed security findings. A fairly straightforward change, however the existing code allowed the modification of a comment on the existing vulnerability state transition. (An outstanding conversation as strictly state transitions should be Noteable and not have comments attached to them directly). To unblock Provide & edit dismissal reason in finding modal (#412841 - closed) • Lorenz van Herwaarden • 16.4 • On track is it simplest to allow the modification of the dismissal_reason, but we had already made the technical decision that state transitions should behave as audit trails and should not be modifiable, which is contravened by this code.

I've created Consider if Vulnerability::StateTransitions sho... (#423693) • Unassigned • Backlog to discuss this further while unblocking current work.

New Query

Query
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"."solution" AS t1_r18,
    "findings"."cve" AS t1_r19,
    "findings"."location" AS t1_r20,
    "findings"."detection_method" AS t1_r21,
    "state_transitions"."id" AS t2_r0,
    "state_transitions"."vulnerability_id" AS t2_r1,
    "state_transitions"."to_state" AS t2_r2,
    "state_transitions"."from_state" AS t2_r3,
    "state_transitions"."created_at" AS t2_r4,
    "state_transitions"."updated_at" AS t2_r5,
    "state_transitions"."author_id" AS t2_r6,
    "state_transitions"."comment" AS t2_r7,
    "state_transitions"."dismissal_reason" AS t2_r8,
    "state_transitions"."state_changed_at_pipeline_id" AS t2_r9
FROM
    "vulnerabilities"
    LEFT OUTER JOIN "vulnerability_occurrences" "findings" ON "findings"."vulnerability_id" = "vulnerabilities"."id"
    LEFT OUTER JOIN "vulnerability_state_transitions" "state_transitions" ON "state_transitions"."vulnerability_id" = "vulnerabilities"."id"
WHERE
    "findings"."uuid" = '01f10c21-15df-5475-81df-2571f7dd493b'
    AND "state_transitions"."id" IN (
        SELECT
            "vulnerability_state_transitions"."id"
        FROM
            "vulnerability_state_transitions"
        WHERE
            "vulnerabilities"."id" = "vulnerability_state_transitions"."vulnerability_id"
        ORDER BY
            "vulnerability_state_transitions"."id" DESC
        LIMIT 1)
Time: 26.175 ms  
  - planning: 1.176 ms  
  - execution: 24.999 ms  
    - I/O read: 24.458 ms  
    - I/O write: 0.000 ms  
  
Shared buffers:  
  - hits: 7 (~56.00 KiB) from the buffer pool  
  - reads: 11 (~88.00 KiB) from the OS file cache, including disk I/O  
  - dirtied: 0  
  - writes: 0  

https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/21883/commands/70884 10 Findings: https://postgres.ai/console/gitlab/gitlab-production-tunnel-pg12/sessions/21914/commands/70962 Roughly 130KB read per Security Finding queried.

How to set up and validate locally

  1. Find a Security Finding in your console with Security::Finding.last, grab it's UUID.
  2. Use the UUID in the following GQL mutation.
mutation {
  securityFindingDismiss(input: {uuid: "af7787c3-7a9d-5569-96a5-96363ca7924a", dismissalReason: USED_IN_TESTS}) {
    errors
    uuid
    clientMutationId
  }
}
  1. Check the dismissal_reason with Security::Finding.last.vulnerability.latest_state_transition
  2. Use the UUID in the following GQL mutation.
mutation {
  securityFindingDismiss(input: {uuid: "af7787c3-7a9d-5569-96a5-96363ca7924a", dismissalReason: ACCEPTABLE_RISK}) {
    errors
    uuid
    clientMutationId
  }
}
  1. Check the dismissal_reason with Security::Finding.last.vulnerability.latest_state_transition has changed.

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 #411210 (closed)

Edited by Gregory Havenga

Merge request reports