false positive when the endpoint is not accessible (e.g. not enough permissions)

[rgreinho]

Description

Running ./chain-bench scan --repository-url https://github.com/buildsec/frsca --access-token $GITHUB_TOKEN -o ./results, some checks where marked as failed due to not having enough permissions associated with the GitHub token.

For instance:

1.1.3 Ensure any change to code receives approval of two strongly authenticated
users Failed

The problem was that the GITHUB_TOKEN did not have permissions to read the branch protection settings, therefore marking this check as failed.

However it should in this case show another status, like Unknown or not evaluated for instance, as the check was not able to read the results (the endpoint returned a 404).

The failed status implies that the requirement was not met, and should be reserved for cases where the number of required reviewers was strictly less than 2.

The same problem applies to all the checks that returned a 404.

Additional details in case it does not expose sensitive data (scanned pipeline files, PR, etc):

[naortalmor1]

@MorAlon1 Please take a look and fix it 🙏🏽

[MorAlon1]

@rgreinho can you please give us more details about the scenario? was it public repository or private and what permissions your token includes?

[rgreinho]

@MorAlon1 Yes, the project is open source, and you should be able to to reproduce the behavior by re-running the command I pasted above.

The permissions for this particular token are:

• repo
• workflow
• admin:org -> read:org
• admin:repo_hook -> read:repo_hook
• gist
• user
• project -> read:project

[MorAlon1]

@rgreinho I created a token with the permissions you mentioned before and got Unknown for rule 1.1.3, can you check again if you gave me the right permissions?

[rgreinho]

Yup, this is what I get, from the main branch:

[ghost]

@rgreinho - can you please check that the fix works for your case?

[rgreinho]

@morwn @MorAlon1 I just checked with the latest commit from the main branch and it did not seem to solve my issue:

But we do use 2 reviewer approvals in order for a PR to be merged.

For instance with https://github.com/buildsec/frsca/pull/242, you can see that we needed an approval from me and from bradbeck before our bot (kodiak) merged the PR.

If you want to re-open this PR, I'd be happy to help you dig.

[morwn]

@rgreinho I'm not sure how kodiak bot enforcing the policy. could you please enlighten me? any case, we currently enforce the checks based on the branch protection API call:

curl \
  -H "Accept: application/vnd.github+json" \ 
  -H "Authorization: token <TOKEN>" \
  https://api.github.com/repos/OWNER/REPO/branches/BRANCH/protection

so if .required_pull_request_reviews.required_approving_review_count < 2 we fail the check. let me know if you set the required_approving_review_count corresponde

[rgreinho]

Sure thing! Kodiak uses the branch protection settings to decide when to merge or rebase a branch. Therefore as soon as all the branch protection criterias are met, the bot acts accordingly.

Requiring 2 reviewers is part of our branch protection settings.

[morwn]

What do you get when you run this api call?

curl \
  -H "Accept: application/vnd.github+json" \ 
  -H "Authorization: token <TOKEN>" \
  https://api.github.com/repos/OWNER/REPO/branches/BRANCH/protection

[rgreinho]

$ curl -s  -H "Accept: application/vnd.github.v3+json" -H "Authorization: token $GITHUB_TOKEN" https://api.github.com/repos/buildsec/frsca/branches/main/protection
{
  "message": "Not Found",
  "documentation_url": "https://docs.github.com/rest/reference/repos#get-branch-protection"
}

and

$ curl -s  -H "Accept: application/vnd.github.v3+json" -H "Authorization: token $GITHUB_TOKEN" https://api.github.com/repos/buildsec/frsca/branches/main/protection/required_pull_request_reviews
{
  "message": "Not Found",
  "documentation_url": "https://docs.github.com/rest/reference/repos#get-pull-request-review-protection"
}

[morwn]

You might miss the permission for repo settings. please try to generate a scoped token based on an admin user within this repository

[rgreinho]

Oh, that is the whole point, I am not admin for this repo 🙃 So since I cannot read this property, chain-bench should return "Unknown" instead of "Failed" (since technically it is no a configuration failure, it is just that I don not have the permission to read this value).

[rgreinho]

Here is a screenshot of the branch protection settings:

[morwn]

Oh my bad, I reopen the issue and pushed new PR that should fix it

[rgreinho]

@morwn It worked like a charm! Great job!