From b0679d4f6743206557ca1f4dff8fb9d871afdfbb Mon Sep 17 00:00:00 2001 From: Mohd Kaif <98801504+KaifAhmad1@users.noreply.github.com> Date: Mon, 31 Aug 2026 19:29:24 +0530 Subject: [PATCH] fix(ci): stop checkov's suppressed checks from reopening as new alerts (#1346) * fix(ci): drop unpinnable benchmarks/requirements.txt install Scorecard flagged this pip install as unpinned-by-hash (#6082). Can't hash-pin it - benchmarks/requirements.txt doesn't exist in this repo, so there's nothing to compile a lockfile from. Dropping it instead of leaving it unpinned: the job already fails on the next real step (benchmarks/benchmarks_runner.py, also missing), so this line wasn't doing anything useful to begin with. * fix(ci): hash-pin the spacy model download in benchmark.yml Qodo review on this PR: dropping the benchmarks/requirements.txt install (the previous failure point) let the job actually reach `python -m spacy download en_core_web_sm`, which fetches an unpinned, unhashed wheel from spacy-models' GitHub releases - undoing the point of this PR by exposing a real unpinned-install path instead of a dead one. Replaced with a hash-pinned direct-URL entry in benchmark-extra.in/.txt for en_core_web_sm-3.8.0 (matches the spacy==3.8.15 already pinned in base-deps.txt). uv independently computed the same sha256 I got via a manual curl+sha256 of the release asset, and a --require-hashes dry-run install verifies clean. * fix(ci): stop checkov's suppressed checks from reopening as new alerts Root cause found, not just worked around: checkov's SARIF exporter includes every evaluated check as an ordinary result, including ones it internally marked SKIPPED via the inline # checkov:skip= comments and checkov.io/skipN annotations already on the Helm chart. It never uses SARIF's own `suppressions` field and never drops them - so the exact same already-suppressed finding reopens as a brand-new code scanning alert number on every single run, forever (#6035/#6036, #6112-6115, #6128-6131 are all the same 4 findings, manually dismissed 3 times now). checkov's JSON output *does* correctly record which checks were skipped. Added .github/scripts/filter_checkov_skipped.py, which cross-references the JSON's skipped_checks against the SARIF's results (matched by check ID + the last two path segments, since the two outputs use different path roots) and drops anything checkov itself already decided to suppress, before upload. Verified locally against a real checkov+helm run: removed exactly the 4 known-suppressed helm chart results, left the 2 genuinely real findings (deploy/gcp/cloudrun-service.yaml, deploy/kubernetes/ deployment.yaml) untouched. --- .github/scripts/filter_checkov_skipped.py | 76 +++++++++++++++++++++++ .github/workflows/defender-for-devops.yml | 22 ++++++- 2 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 .github/scripts/filter_checkov_skipped.py diff --git a/.github/scripts/filter_checkov_skipped.py b/.github/scripts/filter_checkov_skipped.py new file mode 100644 index 00000000..f0f366a0 --- /dev/null +++ b/.github/scripts/filter_checkov_skipped.py @@ -0,0 +1,76 @@ +"""Drop checkov-suppressed results from its SARIF output before upload. + +checkov's SARIF exporter includes every evaluated check as an ordinary +result, including ones it internally marked SKIPPED via an inline +`# checkov:skip=` comment or a `checkov.io/skipN` resource annotation - it +never uses SARIF's `suppressions` field, and never drops them. checkov's +JSON output *does* correctly record which checks were skipped, so this +cross-references the two: any SARIF result whose (check_id, file) pair +appears in the JSON's skipped_checks is removed before GitHub ever sees it. + +Without this, every already-suppressed finding reopens as a brand new code +scanning alert on every run, forever (see #6035/#6036, #6112-6115, +#6128-6131 for the pattern this was chasing before this script existed). + +Usage: filter_checkov_skipped.py +""" + +import json +import sys + + +def path_suffix(path: str, segments: int = 2) -> str: + """Last N path segments, normalized to forward slashes, lowercased. + + checkov's JSON file_path and SARIF artifactLocation.uri are relative to + different roots (the scanned directory vs. a temp helm-render dir), so + they can't be compared directly - but the last couple of segments + (e.g. "templates/service.yaml") are stable across both and specific + enough in practice to avoid cross-file collisions. + """ + normalized = path.replace("\\", "/").strip("/") + return "/".join(normalized.split("/")[-segments:]).lower() + + +def main() -> None: + json_path, sarif_in_path, sarif_out_path = sys.argv[1:4] + + with open(json_path, encoding="utf-8") as f: + checkov_json = json.load(f) + if isinstance(checkov_json, dict): + checkov_json = [checkov_json] + + skipped = set() + for block in checkov_json: + for check in block.get("results", {}).get("skipped_checks", []): + skipped.add((check["check_id"], path_suffix(check["file_path"]))) + + with open(sarif_in_path, encoding="utf-8") as f: + sarif = json.load(f) + + removed = 0 + for run in sarif.get("runs", []): + kept = [] + for result in run.get("results", []): + rule_id = result.get("ruleId") + locations = result.get("locations") or [{}] + uri = ( + locations[0] + .get("physicalLocation", {}) + .get("artifactLocation", {}) + .get("uri", "") + ) + if (rule_id, path_suffix(uri)) in skipped: + removed += 1 + continue + kept.append(result) + run["results"] = kept + + with open(sarif_out_path, "w", encoding="utf-8") as f: + json.dump(sarif, f) + + print(f"Removed {removed} checkov-suppressed result(s) from the SARIF before upload.") + + +if __name__ == "__main__": + main() diff --git a/.github/workflows/defender-for-devops.yml b/.github/workflows/defender-for-devops.yml index 8d3b8db1..8b48b5fb 100644 --- a/.github/workflows/defender-for-devops.yml +++ b/.github/workflows/defender-for-devops.yml @@ -76,12 +76,28 @@ jobs: PYTHONUTF8: "1" run: | New-Item -ItemType Directory -Force reports | Out-Null - checkov --directory . --framework kubernetes helm dockerfile github_actions secrets bicep arm --soft-fail --output sarif --output-file-path reports/checkov.sarif - if (-not (Test-Path reports/checkov.sarif)) { + checkov --directory . --framework kubernetes helm dockerfile github_actions secrets bicep arm --soft-fail --output sarif --output json --output-file-path reports + if (-not (Test-Path reports/results_sarif.sarif)) { $sarif = Get-ChildItem -Path reports -Recurse -Filter *.sarif | Select-Object -First 1 if ($null -eq $sarif) { throw "Checkov did not produce a SARIF file" } - Copy-Item $sarif.FullName reports/checkov.sarif + Copy-Item $sarif.FullName reports/results_sarif.sarif } + if (-not (Test-Path reports/results_json.json)) { + $json = Get-ChildItem -Path reports -Recurse -Filter *.json | Select-Object -First 1 + if ($null -eq $json) { throw "Checkov did not produce a JSON file" } + Copy-Item $json.FullName reports/results_json.json + } + + # checkov's SARIF exporter includes checks it internally marked SKIPPED + # (via the inline `# checkov:skip=` comments / `checkov.io/skipN` + # annotations already on the Helm chart) as ordinary un-suppressed + # results - it never uses SARIF's own `suppressions` field, so GitHub + # opens a fresh alert for the same already-suppressed finding on every + # single run (see #6035/#6036, #6112-6115, #6128-6131). checkov's JSON + # output does correctly record the skip, so cross-reference it here + # instead of re-dismissing the same alerts by hand forever. + - name: Filter checkov's own suppressed checks out of the SARIF + run: python .github/scripts/filter_checkov_skipped.py reports/results_json.json reports/results_sarif.sarif reports/checkov.sarif - name: Upload Checkov results to Security tab uses: github/codeql-action/upload-sarif@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4