Skip to content

ci(checkov): commit skip config instead of reading a repo variable - #968

Open
marcleblanc2 wants to merge 1 commit into
mainfrom
marc/checkov-commit-skip-config
Open

marcleblanc2 wants to merge 1 commit into
mainfrom
marc/checkov-commit-skip-config

Conversation

@marcleblanc2

Copy link
Copy Markdown
Contributor

Problem

The Checkov workflow writes .checkov.yaml from ${{ vars.CHECKOV_SKIP_CHECKS }} at runtime. Fork PRs don't receive repository variables, so the file ends up empty and checkov exits 2:

checkov: error: The config file doesn't appear to contain 'key: value' pairs (aka. a YAML mapping). yaml.load('.../.checkov.yaml') returned type 'NoneType' instead of 'dict'.

The SARIF upload step then fails because results.sarif was never written. Example: https://github.com/sourcegraph/deploy-sourcegraph-helm/actions/runs/39357525392/job/118415955536?pr=902

Fix

Commit the skip list as .checkov.yaml (same contents as the repo variable, sorted) and drop the runtime step. Checkov loads .checkov.yaml from the working directory automatically. This matches how deploy-sourcegraph-managed already does it.

The CHECKOV_SKIP_CHECKS repo variable can be deleted once this merges.

Test plan

Fork PRs do not receive repository variables, so the
'Write skip checks config' step wrote an empty .checkov.yaml and
checkov exited 2 with 'config file ... returned type NoneType'.

Move the skip list into a committed .checkov.yaml, which checkov loads
automatically, and drop the step and the CHECKOV_SKIP_CHECKS variable.

Verified locally with checkov 3.3.29: --show-config loads the file and
the chart scan reports 1442 passed / 0 failed, same as main.

Amp-Thread-ID: https://ampcode.com/threads/T-01a1571c-39af-708a-a421-a575d606dae7
Co-authored-by: Amp <amp@ampcode.com>
@github-actions

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant