Skip to content

Disable deleting approval rules from scan result policies

Sashi Kumar Kumaresan requested to merge sk/420335-fix-delete into master

What does this MR do and why?

Addresses #420335 (closed)

This MR fixes a bug in showing and deleting MR approval rules in settings page where approval rules created from :license_finding scan result policy are allowed to be deleted from UI. This change fixes the behaviour by not exposing the approval rules created from scan result policy in the list API and not allowing the approval rules to be deleted/updated from PUT/DELETE API.

Database query plan

Query

SELECT
    "approval_project_rules".* 
FROM
    "approval_project_rules" 
WHERE
    "approval_project_rules"."project_id" = 48135606 
    AND (
        "approval_project_rules"."report_type" IS NULL 
        OR "approval_project_rules"."report_type" NOT IN (
            4, 2
        )
    ) 
    AND "approval_project_rules"."id" = 18298464 LIMIT 1

Plan

 Limit  (cost=0.42..3.45 rows=1 width=182) (actual time=7.264..7.264 rows=0 loops=1)
   Buffers: shared read=4
   I/O Timings: read=7.192 write=0.000
   ->  Index Scan using index_approval_project_rules_on_project_id on public.approval_project_rules  (cost=0.42..3.45 rows=1 width=182) (actual time=7.262..7.262 rows=0 loops=1)
         Index Cond: (approval_project_rules.project_id = 48135606)
         Filter: (((approval_project_rules.report_type IS NULL) OR (approval_project_rules.report_type <> ALL ('{4,2}'::integer[]))) AND (approval_project_rules.id = 18298464))
         Rows Removed by Filter: 1
         Buffers: shared read=4
         I/O Timings: read=7.192 write=0.000

Time: 9.777 ms
  - planning: 2.414 ms
  - execution: 7.363 ms
    - I/O read: 7.192 ms
    - I/O write: 0.000 ms

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

Screenshots or screen recordings

Before After
Screenshot_2023-08-15_at_11.16.54_PM Screenshot_2023-08-15_at_11.18.21_PM

How to set up and validate locally

  1. Create a scan result policy that contains both license_finding and scan_finding rules in a same policy
type: scan_result_policy
name: Scan
description: ''
enabled: true
rules:
  - type: license_finding
    match_on_inclusion: true
    license_types:
      - GNU General Public License v2.0 or later
      - GNU Affero General Public License v3.0
    license_states:
      - newly_detected
    branches:
      - main
  - type: scan_finding
    scanners:
      - dependency_scanning
      - dast
      - sast
    vulnerabilities_allowed: 0
    severity_levels:
      - critical
    vulnerability_states:
      - newly_detected
    branches:
      - main
actions:
  - type: require_approval
    approvals_required: 1
    group_approvers_ids:
      - 22
  1. Go to Settings -> Merge Requests -> Merge request approvals and verify that approval rules from policy are not listed
  2. Try calling the API to delete the approval rule and verify if it returns 404
curl 'http://gdk.test:3000/api/v4/projects/<project_id>/approval_rules/<approval_rule_id>' \
  -X 'DELETE' \
  -H 'Accept: application/json, text/plain, */*' \
  --compressed \
  --insecure

MR acceptance checklist

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

Edited by Sashi Kumar Kumaresan

Merge request reports