sett-and-hive / sarif-to-comment-action

A GitHub action for @security-alert/sarif-to-comment
MIT License
7 stars 5 forks source link

bug: Suppressed findings displaying under results. #333

Open XargsUK opened 1 month ago

XargsUK commented 1 month ago

Version

v2.0.1

Current Behavior

I attempted to use this Action alongside Checkov, which outputs a SARIF report in a workflow. When doing so, suppressed results showed under the Results section and under the Suppressed section.

Below is a finding which was under both sections, despite being a suppressed finding:

JSON Selector: $.runs[0].results[0]

{
   "ruleId": "CKV_AWS_356",
   "ruleIndex": 0,
   "level": "warning",
   "attachments": [],
   "message": {
      "text": "Ensure no IAM policies documents allow \"*\" as a statement's resource for restrictable actions"
   },
   "locations": [
      {
         "physicalLocation": {
            "artifactLocation": {
               "uri": "terraform/modules/eks_cluster/iam_policies.tf"
            },
            "region": {
               "startLine": 1,
               "endLine": 16,
               "snippet": {
                  "text": "data \"aws_iam_policy_document\" \"ecr\" {\n  # checkov:skip=CKV_AWS_111: Required for karpenter\n  # checkov:skip=CKV_AWS_109: Required for karpenter\n  # checkov:skip=CKV_AWS_356: Required for karpenter\n  statement {\n    effect = \"Allow\"\n\n    actions = [\n      \"ecr:*\",\n    ]\n\n    resources = [\n      \"*\"\n    ]\n  }\n}\n"
               }
            }
         }
      }
   ],
   "suppressions": [
      {
         "kind": "inSource",
         "justification": " Required for karpenter"
      }
   ]
}

Expected Behavior

If a finding is suppressed, it should not show under the Results section, but should show under the Suppressed section in the comment.

Steps to Reproduce

name: checkov

on:
  push:
    branches: [ "main" ]
  pull_request:
    branches: [ "main"  ]
  workflow_dispatch:

jobs:

  security-scan:
    runs-on: ubuntu-latest
    permissions:
      contents: read
      security-events: write
      actions: read
      statuses: read
      pull-requests: write
    steps:
      - name: Harden Runner
        uses: step-security/harden-runner@v2
        with:
          disable-sudo: true
          egress-policy: block
          disable-telemetry: true
          allowed-endpoints: >
            github.com:443

      - uses: actions/checkout@v4
        with:
          fetch-depth: 0

      - name: Checkov GitHub Action
        uses: bridgecrewio/checkov-action@v12
        with:
          directory: terraform/
          output_format: cli,sarif
          output_file_path: console,results.sarif
          quiet: true
          soft_fail: true

      - name: Post SARIF findings in the pull request
        if: github.event_name == 'pull_request'
        uses: sett-and-hive/sarif-to-comment-action@v2.0.1
        with:
          token: ${{ secrets.GITHUB_TOKEN }}
          repository: ${{ github.repository }}
          branch: ${{ github.head_ref }}
          pr-number: ${{ github.event.number }}
          sarif-file: results.sarif
          title: Checkov Scan Results
          dry-run: false

Additional Information

I noticed that when a finding in Checkov is suppressed, it changes the level from error to warning. Potentially creating the addition of a minimum level threshold would prevent results which are supressed from showing under the results section, and configuration for a minimum level threshold for under the suppressed section.

For example:

      - name: Post SARIF findings in the pull request
        if: github.event_name == 'pull_request'
        uses: sett-and-hive/sarif-to-comment-action@v2.0.1
        with:
          token: ${{ secrets.GITHUB_TOKEN }}
          repository: ${{ github.repository }}
          branch: ${{ github.head_ref }}
          pr-number: ${{ github.event.number }}
          sarif-file: results.sarif
          title: Checkov Scan Results
          dry-run: false
          min-result-level: "error"
          min-suppressed-level: "warning"

I understand that this is likely due to the implementation of SARIF report generation from Checkov.

Note that also in the SARIF is $.runs[0].tool.driver.rules which are actually the results which are populated under the Results section in the comment. Here is the contents of $.runs[0].tool.driver.rules[0] as an example:

{
   "id": "CKV_AWS_356",
   "name": "Ensure no IAM policies documents allow \"*\" as a statement's resource for restrictable actions",
   "shortDescription": {
      "text": "Ensure no IAM policies documents allow \"*\" as a statement's resource for restrictable actions"
   },
   "fullDescription": {
      "text": "Ensure no IAM policies documents allow \"*\" as a statement's resource for restrictable actions"
   },
   "help": {
      "text": "Ensure no IAM policies documents allow \"*\" as a statement's resource for restrictable actions\nResource: module.eks_cluster.aws_iam_policy_document.ecr"
   },
   "defaultConfiguration": {
      "level": "error"
   },
   "helpUri": "https://docs.prismacloud.io/en/enterprise-edition/policy-reference/aws-policies/aws-iam-policies/bc-aws-356"
}