fix(github): bug fixes in CKV_GITHUB_6, CKV_GITHUB_7, CKV_GITHUB_9 #3605
Conversation
marynaKK
changed the title
checkov/github/dal.py
Outdated
| @@ -85,28 +82,34 @@ def get_organization_webhooks(self) -> dict[str, Any] | None: | |||
| return data | |||
|
|
|||
| def get_repository_collaborators(self) -> dict[str, Any] | None: | |||
| data = self._request(endpoint="repos/{}/{}/collaborators".format(self.org, self.current_repository), | |||
| allowed_status_codes=[200]) | |||
| if self.org: | |||
There was a problem hiding this comment.
This is not needed, org can exist (to retrieve org security info) but the repository collaborators should be queried with repo-related details, i.e. repo_owner.
There was a problem hiding this comment.
checkov/github/dal.py
Outdated
|
|
||
| def get_repository_webhooks(self) -> dict[str, Any] | None: | ||
| data = self._request(endpoint="repos/{}/{}/hooks".format(self.org, self.current_repository), | ||
| allowed_status_codes=[200]) | ||
| if self.org: |
There was a problem hiding this comment.
Same here, it is not needed, org can exist (to retrieve org security info) but the repository webhooks should be queried with repo-related details, i.e. repo_owner.
There was a problem hiding this comment.
there could be hooks for org, for repo of org, and for repo not under an org.
hence to get the correct hooks of the repo - there must be a specification if it's under a user or an org.
These 2 URLs will retrieve different data:
https://api.github.com/repos/marynaKK/terragoat/hooks - The private repo under my account
https://api.github.com/repos/KiteFlyingInTheSky/terragoat/hooks - The repo under the org 'KiteFlyingInTheSky', that is different from the previous repo.
The get endpoint is: /repos/{owner}/{repo}/hooks, It is not mentioned in the documentation but I guess the owner could be the org or the user.
There was a problem hiding this comment.
If the user's org is also repo owner and he's interested in repo hooks under that org, he can provide org name as repo_owner env var.
The current solution will fail if I do provide org env var, but my org is not listed as repo owner (i.e. I'm the sole repo owner).
| report = runner.run( | ||
| root_folder=valid_dir_path, | ||
| runner_filter=RunnerFilter(checks=checks) | ||
| ) | ||
| self.assertEqual(len(report.failed_checks), 1) | ||
| self.assertEqual(report.parsing_errors, []) | ||
| self.assertEqual(len(report.passed_checks), 3) | ||
| self.assertEqual(len(report.passed_checks), 2) |
There was a problem hiding this comment.
Did we lose a check here? I would expect that if we have less passing checks we'll have more failed checks or skipped checks.
There was a problem hiding this comment.
We didn't lose a check. initially, there was an error - see bug 2 in my description.
…ridgecrewio#3605) * bug fixes * fix test * added test to differentiate repo & org webhooks * fix lint, mypy * changed to repo owner in hooks and collab endpoint * removed unused var from test * remove all files of github_conf dir tree * fix lint
…ridgecrewio#3605) * bug fixes * fix test * added test to differentiate repo & org webhooks * fix lint, mypy * changed to repo owner in hooks and collab endpoint * removed unused var from test * remove all files of github_conf dir tree * fix lint
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Description
Fix of couple bugs in GitHub checks
Description
The bugs:
The fix:
GITHUB_REPO_OWNERto access webhooks/collaborators for a repo, instead ofGITHUB_ORGChecklist: