Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(github): add graph to GitHub Actions #3672

Merged
merged 17 commits into from
Nov 7, 2022

Conversation

gruebel
Copy link
Contributor

@gruebel gruebel commented Oct 16, 2022

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Description

  • added some boilerplate to the ObjectRunner to make it easier to add graphs to other runners
  • added a graph to GitHub Actions with jobs and steps as resource_types and edges between them
  • added an example for a simple attribute and a connection YAML check
  • added one regular graph check CKV2_GHA_1 which ensures the user doesn't give write-all permission on the top-level

ex.

-d [path to test file] --framework github_actions --external-checks-dir checkov/tests/github_actions/checks/extra_yaml_checks -c CKV2_GHA_CUSTOM_1

       _               _              
   ___| |__   ___  ___| | _______   __
  / __| '_ \ / _ \/ __| |/ / _ \ \ / /
 | (__| | | |  __/ (__|   < (_) \ V / 
  \___|_| |_|\___|\___|_|\_\___/ \_/  
                                      
By bridgecrew.io | version: 2.1.270 

github_actions scan results:

Passed checks: 1, Failed checks: 1, Skipped checks: 0

Check: CKV2_GHA_CUSTOM_1: "Ensure job permissions don't contain contents write access"
	PASSED for resource: jobs.spelling
	File: /.github/workflows/pr-test.yml:9-33
Check: CKV2_GHA_CUSTOM_1: "Ensure job permissions don't contain contents write access"
	FAILED for resource: jobs.update
	File: /.github/workflows/pr-test.yml:59-80

		59 |     name: Update PR
		60 |     permissions:
		61 |       contents: write
		62 |       pull-requests: write
		63 |     runs-on: ubuntu-latest
		64 |     if: ${{
		65 |         github.event_name == 'issue_comment' &&
		66 |         github.event.issue.pull_request &&
		67 |         contains(github.event.comment.body, '@check-spelling-bot apply')
		68 |       }}
		69 |     concurrency:
		70 |       group: spelling-update-${{ github.event.issue.number }}
		71 |       cancel-in-progress: false
		72 |     steps:
		73 |     - name: apply spelling updates
		74 |       uses: check-spelling/[email protected]
		75 |       with:
		76 |         experimental_apply_changes_via_bot: 1
		77 |         checkout: true
		78 |         ssh_key: "${{ secrets.CHECK_SPELLING }}"

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my feature, policy, or fix is effective and works
  • New and existing tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Copy link
Contributor

@nimrodkor nimrodkor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really great. I think we should add at least 1 or 2 checks and verify they are packaged correctly in checkov. WDYT?

checkov/json_doc/runner.py Outdated Show resolved Hide resolved
checkov/yaml_doc/runner.py Outdated Show resolved Hide resolved
@gruebel
Copy link
Contributor Author

gruebel commented Oct 27, 2022

This is really great. I think we should add at least 1 or 2 checks and verify they are packaged correctly in checkov. WDYT?

tought about it, but had no good idea, which were unique for a graph check, but I will add something simple 😄

Copy link
Contributor

@tronxd tronxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

checkov/common/runners/graph_builder/local_graph.py Outdated Show resolved Hide resolved
checkov/common/runners/graph_builder/local_graph.py Outdated Show resolved Hide resolved
Comment on lines 68 to 70
failing_resources = {
"jobs.attest.steps.5",
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that compatible with the resource ids already generated for steps with the python checks? I think that the format for step resources should be 'jobs.<job_name>.steps.<step_name>'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the name field is optional and you can name all your steps the same, so if we use this in Python checks, we probably should reconsider it. only the order is relevant at the end.

Copy link
Contributor

@Eliran-Turgeman Eliran-Turgeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool 🔥
Had only one comment regarding the resource ids

@gruebel gruebel merged commit 76ffbb0 into bridgecrewio:master Nov 7, 2022
@gruebel gruebel deleted the add-gha-graph branch November 7, 2022 07:53
ayajbara pushed a commit that referenced this pull request Nov 7, 2022
* add graph capabilities to ObjectRunner

* add graph to GitHub Actions

* fix Python 3.11 issue

* add permissions vertices and adjust docs

* fix PR comments

* add CKV2_GHA_1 to scan for top-level permissions

* fix argument name in kubernetes graph manager

* add GHA grpah checks to doc generator

* adjust test entities

* adjust the log level

* adjust GHA step name

* fix linting

* fix mypy
ayajbara pushed a commit that referenced this pull request Nov 7, 2022
* add graph capabilities to ObjectRunner

* add graph to GitHub Actions

* fix Python 3.11 issue

* add permissions vertices and adjust docs

* fix PR comments

* add CKV2_GHA_1 to scan for top-level permissions

* fix argument name in kubernetes graph manager

* add GHA grpah checks to doc generator

* adjust test entities

* adjust the log level

* adjust GHA step name

* fix linting

* fix mypy
ayajbara pushed a commit that referenced this pull request Nov 7, 2022
* add graph capabilities to ObjectRunner

* add graph to GitHub Actions

* fix Python 3.11 issue

* add permissions vertices and adjust docs

* fix PR comments

* add CKV2_GHA_1 to scan for top-level permissions

* fix argument name in kubernetes graph manager

* add GHA grpah checks to doc generator

* adjust test entities

* adjust the log level

* adjust GHA step name

* fix linting

* fix mypy
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.

4 participants