Stop unblocking policy approvals when security jobs get canceled

Martin Cavojrequested to merge
585752-do-not-unblock-canceled-scans into master

What does this MR do and why?

Stop unblocking policy approvals when security jobs get canceled

When unblock_rules_using_execution_policies option is enabled for merge request approval policies, security scanners are not required to run as long as they are enforced by SEP or PEP. The approvals are required when the security job runs and fails, however, when the jobs run and get canceled, the approvals stay optional. We should treat it as a failure and don't unblock policy approvals in that case to prevent bypass.

References

Screenshots or screen recordings

Before After

Database

pipeline.builds.latest.with_secure_reports_from_config_options(scanners).without_status(:success).exists?

Query:

SELECT 1 AS one FROM "p_ci_builds" WHERE "p_ci_builds"."type" = 'Ci::Build' AND "p_ci_builds"."commit_id" = 2324399968 AND "p_ci_builds"."partition_id" = 108 AND ("p_ci_builds"."retried" = FALSE OR "p_ci_builds"."retried" IS NULL) AND (EXISTS (SELECT 1 FROM "p_ci_job_definition_instances" INNER JOIN "p_ci_job_definitions" ON "p_ci_job_definitions"."partition_id" IS NOT NULL AND "p_ci_job_definitions"."id" = "p_ci_job_definition_instances"."job_definition_id" AND "p_ci_job_definitions"."partition_id" = "p_ci_job_definition_instances"."partition_id" WHERE "p_ci_job_definition_instances"."job_id" = "p_ci_builds"."id" AND "p_ci_job_definition_instances"."partition_id" = "p_ci_builds"."partition_id" AND (config -> 'options' -> 'artifacts' -> 'reports' ?| array['sast']))) AND "p_ci_builds"."status" != 'success' LIMIT 1

Query plan: https://console.postgres.ai/gitlab/gitlab-production-ci/sessions/48865/commands/146329

 Limit  (cost=1.58..82.68 rows=1 width=4) (actual time=13.435..13.437 rows=1 loops=1)
   Buffers: shared read=16
   ->  Nested Loop Semi Join  (cost=1.58..163.79 rows=2 width=4) (actual time=13.434..13.435 rows=1 loops=1)
         Buffers: shared read=16
         ->  Index Scan using ci_builds_108_commit_id_type_ref_idx on gitlab_partitions_dynamic.ci_builds_108 p_ci_builds  (cost=0.57..61.48 rows=25 width=16) (actual time=4.556..4.557 rows=1 loops=1)
               Index Cond: ((p_ci_builds.commit_id = '2324399968'::bigint) AND ((p_ci_builds.type)::text = 'Ci::Build'::text))
               Filter: (((NOT p_ci_builds.retried) OR (p_ci_builds.retried IS NULL)) AND ((p_ci_builds.status)::text <> 'success'::text) AND (p_ci_builds.partition_id = 108))
               Rows Removed by Filter: 2
               Buffers: shared read=7
         ->  Nested Loop  (cost=1.01..4.08 rows=1 width=16) (actual time=8.875..8.875 rows=1 loops=1)
               Buffers: shared read=9
               ->  Index Scan using ci_job_definition_instances_108_pkey on gitlab_partitions_dynamic.ci_job_definition_instances_108 p_ci_job_definition_instances  (cost=0.57..3.59 rows=1 width=24) (actual time=6.249..6.249 rows=1 loops=1)
                     Index Cond: ((p_ci_job_definition_instances.job_id = p_ci_builds.id) AND (p_ci_job_definition_instances.partition_id = 108))
                     Buffers: shared read=5
               ->  Index Scan using ci_job_definitions_108_pkey on gitlab_partitions_dynamic.ci_job_definitions_108 p_ci_job_definitions  (cost=0.44..0.48 rows=1 width=16) (actual time=2.623..2.623 rows=1 loops=1)
                     Index Cond: ((p_ci_job_definitions.id = p_ci_job_definition_instances.job_definition_id) AND (p_ci_job_definitions.partition_id = 108))
                     Filter: ((((p_ci_job_definitions.config -> 'options'::text) -> 'artifacts'::text) -> 'reports'::text) ?| '{sast}'::text[])
                     Rows Removed by Filter: 0
                     Buffers: shared read=4
Settings: work_mem = '100MB', effective_cache_size = '338688MB', random_page_cost = '1.5', jit = 'off', seq_page_cost = '4'

Time: 18.717 ms
  - planning: 5.215 ms
  - execution: 13.502 ms
    - I/O read: N/A
    - I/O write: N/A

Shared buffers:
  - hits: 0 from the buffer pool
  - reads: 16 (~128.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

How to set up and validate locally

  1. Create a project
  2. In the project, create a merge request approval policy 
    approval_policy:
  - name: Require Approval for SAST Findings
    description: Block MRs that have SAST vulnerabilities without approval
    enabled: true
    actions:
      - type: require_approval
        approvals_required: 1
        role_approvers:
          - owner
          - maintainer
    rules:
      - type: scan_finding
        scanners:
          - sast
        vulnerabilities_allowed: 0
        severity_levels:
          - critical
          - high
          - medium
        vulnerability_states:
          - new_needs_triage
        branches:
          - main
    policy_tuning:
      unblock_rules_using_execution_policies: true
  3. In the project, create a pipeline execution policy which runs scanners conditionally based on rules:changes: 
    include:
  - template: Jobs/SAST.latest.gitlab-ci.yml

.sast-optimized:
  extends: .sast-analyzer
  allow_failure: false
  rules:
    - if: $CI_PIPELINE_SOURCE == "merge_request_event"
      changes:
        paths:
          - "**/*.rb"
        compare_to: 'refs/heads/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME'
    - when: never

semgrep-sast:
  extends: .sast-optimized
  rules: !reference [.sast-optimized, rules]
  4. In the project, add a Ruby file to trigger SAST scans: 
    # frozen_string_literal: true

# Sample Ruby application file
# Changes to this file WILL trigger SAST scanning

class Application
  def initialize(name)
    @name = name
  end

  def greet
    puts "Hello from #{@name}!"
  end

  def run
    greet
    process_data
  end

  private

  def process_data
    # Simulated data processing
    data = fetch_data
    transform(data)
  end

  def fetch_data
    { status: 'ok', items: [1, 2, 3] }
  end

  def transform(data)
    data[:items].map { |i| i * 2 }
  end
end

# Run the application
app = Application.new('TestApp')
app.run
  5. Create an MR which only updates README. Verify that SAST scan doesn't run and the MR gets unblocked thanks to unblock_rules_using_execution_policies
  6. Create an MR which updates the Ruby code. Verify that SAST scan runs and the MR gets unblocked due to no vulnerabilities found
  7. Create an MR, adding a vulnerability: 
    def execute_command(user_input)
   system("ls #{user_input}")  # Unsafe - directly interpolates user input
end
  8. Right after pushing, go to the pipeline and cancel the SAST job
  9. Verify that the MR gets blocked and the bot comment gets posted

MR acceptance checklist

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

Related to #585752

Edited by Martin Cavoj

Merge request reports