Skip to content

Persist scan_finding policy violations

Martin Čavoj requested to merge 433396-persist-scan_finding-violation-data into master

What does this MR do and why?

With this MR, we start persisting uuids that caused policy violations for users to better understand how to resolve them (behind feature flag).

This MR also optimizes UpdateApprovalsService specs in !145504 (2f5967c4) and bring the time down from ~14 minutes to ~30 seconds (related issue from some time ago)

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

How to set up and validate locally

  1. In rails console enable the feature flag
    Feature.enable(:save_policy_violation_data)
  2. Create a new project
  3. Add CI configuration:
    include:
      - template: Jobs/Secret-Detection.gitlab-ci.yml
    
    build-job:
      script:
        - echo "Compiling the code..."
        - echo "Compile complete."
  4. Go to Secure -> Policies and create a new policy:
    type: approval_policy
    name: Scans
    description: ''
    enabled: true
    rules:
      - type: scan_finding
        scanners: []
        vulnerabilities_allowed: 0
        severity_levels: []
        vulnerability_states:
          - new_needs_triage
          - new_dismissed
          - detected
          - confirmed
          - dismissed
          - resolved
        branch_type: protected
    actions:
      - type: require_approval
        approvals_required: 1
        role_approvers:
          - developer
  5. Create MR adding a new leaked secret (to test persisting from UpdateApprovalsService). Example:
    diff --git a/.env b/.env
    new file mode 100644
    index 0000000000000000000000000000000000000000..ee4bf74ac3b632173dafc09e74ecd68c298bdfa1
    --- /dev/null
    +++ b/.env
    @@ -0,0 +1 @@
    +AWS_TOKEN=AKIAZYONPI3G4JNCCWGQ
    \ No newline at end of file
  6. After the pipeline completes, verify that jobs pass, artifacts are present and approvals are required
  7. In rails console, check that violations_data have been persisted and uuid is present:
    MergeRequest.last.scan_result_policy_violations
  8. Merge the secret so that it becomes a previously-existing vulnerability. The vulnerability should show up in Secure -> Vulnerability report
  9. Create a new MR, adding a new leaked secret. Example:
    diff --git a/.env b/.env
    index ee4bf74ac3b632173dafc09e74ecd68c298bdfa1..d791cd2180779bf094bd017e07b12c1fad506c2c 100644
    --- a/.env
    +++ b/.env
    @@ -1 +1,2 @@
    -AWS_TOKEN=AKIAZYONPI3G4JNCCWGQ
    \ No newline at end of file
    +AWS_TOKEN=AKIAZYONPI3G4JNCCWGQ
    +AWS_TOKEN2=AKIAZYONPI3G4JNCCWGZ
    \ No newline at end of file
  10. After the pipeline completes, check that violations_data have been persisted and there are two uuids, one for newly_detected and one for previously_existing. Example:
    MergeRequest.last.scan_result_policy_violations.last.violation_data
    
    # => {"context"=>{"pipeline_ids"=>[42373], "target_pipeline_ids"=>[42118]}, "violations"=>{"scan_finding"=>{"uuids"=>{"newly_detected"=>["c9683e04-7a8c-5258-a489-f64e52fd1175"], "previously_existing"=>["625e5196-fb52-55a1-a079-d618c0541b1d"]}}}}
  11. Update the existing policy to only include newly detected vulnerabilities:
    type: approval_policy
    name: New scans
    description: ''
    enabled: true
    rules:
      - type: scan_finding
        scanners: []
        vulnerabilities_allowed: 0
        severity_levels: []
        vulnerability_states:
          - new_needs_triage
          - new_dismissed
        branch_type: protected
    actions:
      - type: require_approval
        approvals_required: 1
        role_approvers:
          - developer
  12. Create a second policy to only match previously-existing vulnerabilities (to verify persisting from SyncPreexistingStatesApprovalRulesService):
    type: approval_policy
    name: Previously existing scans
    description: ''
    enabled: true
    rules:
      - type: scan_finding
        scanners: []
        vulnerabilities_allowed: 0
        severity_levels: []
        vulnerability_states:
          - detected
          - confirmed
          - dismissed
          - resolved
        branch_type: protected
    actions:
      - type: require_approval
        approvals_required: 1
        role_approvers:
          - developer
  13. Create MR which updates README (assuming there is still a detected secret in .env):
  14. Verify that violation data is persisted and there is a uuid in the previously_existing key. Example:
      MergeRequest.last.scan_result_policy_violations.last
      => #<Security::ScanResultPolicyViolation:0x00000002c2631540
       id: 354333,
       scan_result_policy_id: 204523,
       merge_request_id: 2060,
       project_id: 2298,
       created_at: Thu, 22 Feb 2024 12:25:49.478998000 UTC +00:00,
       updated_at: Thu, 22 Feb 2024 12:25:50.259272000 UTC +00:00,
       violation_data: {"violations"=>{"scan_finding"=>{"uuids"=>{"previously_existing"=>["625e5196-fb52-55a1-a079-d618c0541b1d"]}}}}>

Related to #433396 (closed)

Edited by Martin Čavoj

Merge request reports