Skip to content

Generate bot comment for license compliance violations

Martin Čavoj requested to merge 415086-approval-notification-for-license-compliance-violations into master

What does this MR do and why?

This MR expands on bot violations for scan_finding from Add security bot comment for policy violations ... (!121732 - merged) and adds a bot comment also for license scanning violations.

The MR also solves #413975 (closed) by identifying the comments using hidden header. I opted for this approach because:

  1. We want to validate whether the comment is the best approach for the problem stated in Bot comment nudge when merge requests require a... (&10617).
  2. The notes are looked up in the context of an MR, so the performance shouldn't be a big concern to look up notes by prefix
  3. Using DB to identify the comment would require adding two new columns in note_metadata table and these columns would only be used for this specific use case

Screenshots or screen recordings

Raw comment Rendered comment
CleanShot_2023-06-19_at_16.48.45_2x CleanShot_2023-06-19_at_16.48.51_2x

How to set up and validate locally

  1. Enable the feature flags in the rails console:

    Feature.enable(:security_policy_approval_notification)

  2. Add license scanning into .gitlab-ci.yml:

    test-job:
  script:
  - echo "Test Job..."
license_scanning:
  image: "busybox:latest"
  stage: test
  allow_failure: true
  artifacts:
    reports:
      license_scanning: gl-license-scanning-report.json
    paths: [gl-license-scanning-report.json]
  dependencies: []
  script:
    - wget -O gl-license-scanning-report.json https://gitlab.com/gitlab-org/govern/demos/license-approval-policies-test-cases/-/raw/main/report-0.json

  3. Create a new project with a security policy. Example:

    type: scan_result_policy
name: License
description: ''
enabled: true
rules:
  - type: license_finding
    branches: []
    match_on_inclusion: true
    license_types:
      - MIT License
    license_states:
      - newly_detected
actions:
  - type: require_approval
    approvals_required: 1
    user_approvers_ids:
      - 4

  4. Introduce a policy violation in an MR by updating the report file used for artifacts in .gitlab-ci.yaml. report-2.json introduces an MIT license:

    - wget -O gl-license-scanning-report.json https://gitlab.com/gitlab-org/govern/demos/license-approval-policies-test-cases/-/raw/main/report-2.json

  5. After the CI has finished, observe an automatic comment being added.

  6. Add a new commit which doesn't fix the violation yet. There shouldn't be any new automatic comment.

  7. Add a new commit, fixing the violation - switch to report-1.json which introduces GPLv3, which is within the policy.

  8. The comment should get updated and say that violations have been fixed.

Database query plan

Method

Before

SELECT 
  "notes"."note", 
  "notes"."noteable_type", 
  "notes"."author_id", 
  "notes"."created_at", 
  "notes"."updated_at", 
  "notes"."project_id", 
  "notes"."attachment", 
  "notes"."line_code", 
  "notes"."commit_id", 
  "notes"."noteable_id", 
  "notes"."system", 
  "notes"."st_diff", 
  "notes"."updated_by_id", 
  "notes"."type", 
  "notes"."position", 
  "notes"."original_position", 
  "notes"."resolved_at", 
  "notes"."resolved_by_id", 
  "notes"."discussion_id", 
  "notes"."note_html", 
  "notes"."cached_markdown_version", 
  "notes"."change_position", 
  "notes"."resolved_by_push", 
  "notes"."review_id", 
  "notes"."confidential", 
  "notes"."last_edited_at", 
  "notes"."internal", 
  "notes"."id" 
FROM 
  "notes" 
WHERE 
  "notes"."noteable_id" = 231313974
  AND "notes"."noteable_type" = 'MergeRequest' 
  AND "notes"."author_id" = 16943
ORDER BY 
  "notes"."id" ASC 
LIMIT 
  1;

Plan console.postgres.ai

 Limit  (cost=62.65..62.66 rows=1 width=3240) (actual time=13.457..13.462 rows=1 loops=1)
   Buffers: shared hit=189
   I/O Timings: read=0.000 write=0.000
   ->  Sort  (cost=62.65..62.66 rows=1 width=3240) (actual time=13.454..13.458 rows=1 loops=1)
         Sort Key: notes.id
         Sort Method: top-N heapsort  Memory: 27kB
         Buffers: shared hit=189
         I/O Timings: read=0.000 write=0.000
         ->  Bitmap Heap Scan on public.notes  (cost=61.13..62.64 rows=1 width=3240) (actual time=13.262..13.363 rows=13 loops=1)
               Buffers: shared hit=186
               I/O Timings: read=0.000 write=0.000
               ->  BitmapAnd  (cost=61.13..61.13 rows=1 width=0) (actual time=13.242..13.245 rows=0 loops=1)
                     Buffers: shared hit=174
                     I/O Timings: read=0.000 write=0.000
                     ->  Bitmap Index Scan using index_notes_on_noteable_id_and_noteable_type_and_system  (cost=0.00..3.16 rows=96 width=0) (actual time=0.071..0.072 rows=53 loops=1)
                           Index Cond: ((notes.noteable_id = 231313974) AND ((notes.noteable_type)::text = 'MergeRequest'::text))
                           Buffers: shared hit=5
                           I/O Timings: read=0.000 write=0.000
                     ->  Bitmap Index Scan using index_notes_on_author_id_and_created_at_and_id  (cost=0.00..57.71 rows=3618 width=0) (actual time=12.921..12.921 rows=32689 loops=1)
                           Index Cond: (notes.author_id = 16943)
                           Buffers: shared hit=169
                           I/O Timings: read=0.000 write=0.000

Time: 17.858 ms
  - planning: 4.306 ms
  - execution: 13.552 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

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

After

SELECT 
  "notes"."note", 
  "notes"."noteable_type", 
  "notes"."author_id", 
  "notes"."created_at", 
  "notes"."updated_at", 
  "notes"."project_id", 
  "notes"."attachment", 
  "notes"."line_code", 
  "notes"."commit_id", 
  "notes"."noteable_id", 
  "notes"."system", 
  "notes"."st_diff", 
  "notes"."updated_by_id", 
  "notes"."type", 
  "notes"."position", 
  "notes"."original_position", 
  "notes"."resolved_at", 
  "notes"."resolved_by_id", 
  "notes"."discussion_id", 
  "notes"."note_html", 
  "notes"."cached_markdown_version", 
  "notes"."change_position", 
  "notes"."resolved_by_push", 
  "notes"."review_id", 
  "notes"."confidential", 
  "notes"."last_edited_at", 
  "notes"."internal", 
  "notes"."id" 
FROM 
  "notes" 
WHERE 
  "notes"."noteable_id" = 231313974
  AND "notes"."noteable_type" = 'MergeRequest' 
  AND "notes"."author_id" = 16943
  AND (
    note LIKE '<!-- policy_violation_comment -->%'
  ) 
ORDER BY 
  "notes"."id" ASC 
LIMIT 
  1;

Plan console.postgres.ai

 Limit  (cost=62.66..62.66 rows=1 width=3240) (actual time=66.165..66.170 rows=1 loops=1)
   Buffers: shared hit=209 read=6
   I/O Timings: read=50.165 write=0.000
   ->  Sort  (cost=62.66..62.66 rows=1 width=3240) (actual time=66.162..66.166 rows=1 loops=1)
         Sort Key: notes.id
         Sort Method: quicksort  Memory: 25kB
         Buffers: shared hit=209 read=6
         I/O Timings: read=50.165 write=0.000
         ->  Bitmap Heap Scan on public.notes  (cost=61.13..62.65 rows=1 width=3240) (actual time=15.488..66.100 rows=1 loops=1)
               Filter: (notes.note ~~ '<!-- policy_violation_comment -->%'::text)
               Rows Removed by Filter: 12
               Buffers: shared hit=206 read=6
               I/O Timings: read=50.165 write=0.000
               ->  BitmapAnd  (cost=61.13..61.13 rows=1 width=0) (actual time=15.434..15.437 rows=0 loops=1)
                     Buffers: shared hit=174
                     I/O Timings: read=0.000 write=0.000
                     ->  Bitmap Index Scan using index_notes_on_noteable_id_and_noteable_type_and_system  (cost=0.00..3.16 rows=96 width=0) (actual time=0.078..0.079 rows=53 loops=1)
                           Index Cond: ((notes.noteable_id = 231313974) AND ((notes.noteable_type)::text = 'MergeRequest'::text))
                           Buffers: shared hit=5
                           I/O Timings: read=0.000 write=0.000
                     ->  Bitmap Index Scan using index_notes_on_author_id_and_created_at_and_id  (cost=0.00..57.71 rows=3618 width=0) (actual time=14.987..14.987 rows=32689 loops=1)
                           Index Cond: (notes.author_id = 16943)
                           Buffers: shared hit=169
                           I/O Timings: read=0.000 write=0.000

Time: 67.435 ms
  - planning: 1.173 ms
  - execution: 66.262 ms
    - I/O read: 50.165 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 209 (~1.60 MiB) from the buffer pool
  - reads: 6 (~48.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

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

Edited by Martin Čavoj

Merge request reports