Skip to content
Snippets Groups Projects

Add approval notification for unenforceable policy rules

Merged Add approval notification for unenforceable policy rules
417598-generate-approval-notification-when-no-scanners-are-present-in-the-pipeline into master
Merged Martin Čavoj requested to merge 417598-generate-approval-notification-when-no-scanners-are-present-in-the-pipeline into master

What does this MR do and why?

With this change, a policy bot comment will be created for policy rules that could not be enforced due to missing scanners in the pipeline, or missing pipeline setup altogether.

flowchart TD
    A[MR is created] -->WithPipelines
    A[MR is created] -->WithoutPipelines
    subgraph WithPipelines[Pipelines are set up]
    A2[Pipeline finished] --> B{Has all security artifacts?}
    A3[Change target branch] -->|Applicable rules may change
    and become unenforceable| H 
    B -->|Yes| G[Bot comment handled by 
    SyncFindingsToApprovalRulesService and 
    SyncLicenseScanningRulesService]
    B -->|No| H[Enqueue worker]
    end
    subgraph WithoutPipelines[No pipelines]
    C0[MR] --> C1
    C0 --> C2
    C1[After create] -->|Applicable report_approver 
    rules are unenforceable| D[Enqueue worker]
    C2[Change target branch] -->|Applicable rules may change
    and become unenforceable| D
    end
    subgraph Worker[UnenforceablePolicyRulesNotificationService]
    W0[Execute] -->|scan_finding rules| W1[No pipeline or no corresponding artifact?]
    W0 -->|license_scanning rules| W1
    W1 --> Wend[Create/Update bot comment based on violations 
for the applicable rules]
    end
    H -->|Notify for reports with artifacts| Worker
    D -->|Notify for any report_approver rules| Worker

Screenshots or screen recordings
With pipelines; at first targeting a protected branch CleanShot_2023-11-13_at_13.15.44
With pipelines; at first targeting a non-protected branch CleanShot_2023-11-13_at_13.16.45
Without pipelines; update of comment when policy gets disabled CleanShot_2023-11-13_at_13.17.52_2_trimmed

Database

There is a new scope for_approval_rules.

Code

SELECT 1 AS one FROM "scan_result_policy_violations" WHERE "scan_result_policy_violations"."merge_request_id" = 263349793 AND "scan_result_policy_violations"."scan_result_policy_id" IN (945, 946);

Plan: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/23784/commands/76285

Note: I was not able to find a specific scan_result_policy_id for which any rows would be returned.

Other queries related to this MR:

Code

SELECT 1 AS one FROM "approval_merge_request_rules" WHERE "approval_merge_request_rules"."merge_request_id" = 260637620 AND "approval_merge_request_rules"."scan_result_policy_id" IS NOT NULL LIMIT 1;

Plan: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/23784/commands/76297

Code

SELECT 1 AS one FROM "approval_project_rules" WHERE "approval_project_rules"."project_id" = 50532553 AND "approval_project_rules"."scan_result_policy_id" IS NOT NULL LIMIT 1

Plan: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/23784/commands/76303

How to set up and validate locally

  1. In rails console enable the feature flag 
    Feature.enable(:security_policies_unenforceable_rules_notification)
  2. Go to Secure -> Policies and create a new scan result policy. Sample YAML: 
    type: scan_result_policy
name: Sec & Lic
description: ''
enabled: true
rules:
  - type: scan_finding
    scanners: []
    vulnerabilities_allowed: 0
    severity_levels: []
    vulnerability_states: []
    branch_type: protected
  - type: license_finding
    match_on_inclusion: true
    license_types:
      - BSD 3-Clause "New" or "Revised" License
    license_states:
      - newly_detected
    branch_type: protected
actions:
  - type: require_approval
    approvals_required: 1
    role_approvers:
    - developer
approval_settings:
  block_unprotecting_branches: true
  prevent_pushing_and_force_pushing: true

With no pipelines

  1. In a project where there is no .gitlab-ci.yml, create any MR (e.g. update README)
  2. Observe required approvals for Sec & Lic
  3. Observe a policy bot comment being created
  4. Change the target branch to a non-protected one (you can go to Code -> Branches and create a new branch from main)
  5. The policy bot comment should be updated to say the violations have been resolved
  6. Create a new MR targeting a non-protected branch from the beginning -> there should be no policy bot comment. Change the target branch to main and the bot comment should appear.

With missing scanners

  1. Create a .gitlab-ci.yml file and don't include dependency scanning. Sample YAML: 
    include:
  - template: Jobs/Secret-Detection.gitlab-ci.yml
#  - template: Jobs/Dependency-Scanning.gitlab-ci.yml

build-job:
  script:
    - echo "Compiling the code..."
    - echo "Compile complete."
  2. Create MR a policy bot comment being created and approvals should be still required after pipeline finishes
  3. Update .gitlab-ci.yml again and include also the commented-out dependency scanning template. Add empty requirements.txt file in the repo.
  4. Create another MR. There should be no policy bot comment and no required approvals after pipeline finishes.

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

Edited by Martin Čavoj

Merge request reports

Merged results pipeline #1091849733 passed with warnings

Stage: sync
Stage: preflight
Stage: prepare
Stage: build-images
Stage: fixtures
Stage: lint
Stage: test
Stage: post-test
Stage: qa
Stage: benchmark
Pipeline: E2E GDK

#1091854283

    Pipeline: TRIGGERED_EE_PIPELINE

    #1091852782

      Pipeline: E2E Omnibus GitLab EE

      #1091871442

        +2

        Merged results pipeline passed with warnings for babf988f

        Test coverage 82.51% from 2 jobs
        Approval is optional
        Ready to merge by members who can write to the target branch.

        Merge details

        • 1 commit and 1 merge commit will be added to master (squashes 1 commit).
        • Source branch will be deleted.

        • Mentions issue #417598 (closed)

        Activity

        Filter activity
        • Approvals
        • Assignees & reviewers
        • Comments (from bots)
        • Comments (from users)
        • Commits & branches
        • Edits
        • Labels
        • Lock status
        • Mentions
        • Merge request status
        • Tracking
        Please register or sign in to reply