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

Pilot scorecards GitHub Action #35

Closed
varunsh-coder opened this issue Nov 19, 2021 · 10 comments
Closed

Pilot scorecards GitHub Action #35

varunsh-coder opened this issue Nov 19, 2021 · 10 comments

Comments

@varunsh-coder
Copy link
Member

varunsh-coder commented Nov 19, 2021

https://github.com/step-security/agent/blob/main/.github/workflows/scorecard-analysis.yml

@varunsh-coder
Copy link
Member Author

varunsh-coder commented Nov 19, 2021

  • Signed-Releases - valid finding - created Sign agent release #37
  • Code-Review - valid finding
  • Branch Protection - false positive. This repo has branch protection for default branch. Error from Scorecard: internal error: error during branchesHandler.setup: internal error: githubv4.Query: Resource not accessible by integration. It should not create an alert on error.
  • Token-Permissions - raised high severity issue since top level permissions are missing. Will address via secure-workflows
  • Dependency-Update-Tool - false positive. I do have dependabot enabled, though for some reason, dependabot has not created a PR as of now.
  • Security-Policy - valid finding though Scorecards got an error: repo unreachable: GET https://api.github.com/repos/step-security/.github: 404 Not Found []. Created Add security policy to repo #38 to address
  • SAST Tool - SAST tool detected but not run on all commmits - I think requirement to run it on all commits may be too high. CodeQL takes time to run. Running on PR to main and running once a week should be ok IMHO. Created Configure CodeQL to run on PR #39 to adjust CodeQL runs.
  • Packaging - false positive - Not applicable as of now.
  • Fuzzing - valid finding. created Add fuzzing tool #41
  • Pinned Dependency - valid finding. created Pin dependencies in action workflows #40. Will also add feature to secure-workflows.
  • Contributors - valid finding
  • CII badge - valid finding - created Add CII badge #42

@varunsh-coder
Copy link
Member Author

Completed pilot

@laurentsimon
Copy link

  • Signed-Releases - valid finding - created Sign agent release #37
  • Code-Review - valid finding
  • Branch Protection - false positive. This repo has branch protection for default branch. Error from Scorecard: internal error: error during branchesHandler.setup: internal error: githubv4.Query: Resource not accessible by integration. It should not create an alert on error.

We've seen this before ossf/scorecard#1097 and we're not sure why it's happening. How often does it happen?

  • Token-Permissions - raised high severity issue since top level permissions are missing. Will address via secure-workflows
  • Dependency-Update-Tool - false positive. I do have dependabot enabled, though for some reason, dependabot has not created a PR as of now.

We currently only check for the dependabot config file being present. How would you like us to change this behavior?

The NotFound was fixed recently in ossf/scorecard#1277, but is not used in the action code you used yet.

  • SAST Tool - SAST tool detected but not run on all commmits - I think requirement to run it on all commits may be too high. CodeQL takes time to run. Running on PR to main and running once a week should be ok IMHO. Created Configure CodeQL to run on PR #39 to adjust CodeQL runs.

we have a relevant tracking issue on this ossf/scorecard#1268. Feel free to chime in.

  • Packaging - false positive - Not applicable as of now.

this check is unreliable, agreed.

Thanks for the feedback!

@evverx
Copy link

evverx commented Nov 20, 2021

I do have dependabot enabled, though for some reason, dependabot has not created a PR as of now.

As far as I know there are two types of Dependabot. One of them is configured with .github/dependabot.yml and the other one is responsible for security updates: https://docs.github.com/en/code-security/supply-chain-security/managing-vulnerabilities-in-your-projects-dependencies/configuring-dependabot-security-updates. My guess would be that the "security" dependabot is enabled for this repository. I'm not sure how to figure out whether it's enabled using GitHub API though

@varunsh-coder
Copy link
Member Author

I do have dependabot enabled, though for some reason, dependabot has not created a PR as of now.

As far as I know there are two types of Dependabot. One of them is configured with .github/dependabot.yml and the other one is responsible for security updates: https://docs.github.com/en/code-security/supply-chain-security/managing-vulnerabilities-in-your-projects-dependencies/configuring-dependabot-security-updates. My guess would be that the "security" dependabot is enabled for this repository. I'm not sure how to figure out whether it's enabled using GitHub API though

Yes, that is right - the "security" dependabot is enabled for this repo. I did actually get a PR from dependabot today. One way to figure out if dependabot is enabled could be to check the last X PRs to see if one is from dependabot. I have seen similar other checks in Scorecards. While not perfect, if dependabot is enabled, there is high likelihood that it would have created some PRs...

@varunsh-coder
Copy link
Member Author

  • Signed-Releases - valid finding - created Sign agent release #37
  • Code-Review - valid finding
  • Branch Protection - false positive. This repo has branch protection for default branch. Error from Scorecard: internal error: error during branchesHandler.setup: internal error: githubv4.Query: Resource not accessible by integration. It should not create an alert on error.

We've seen this before ossf/scorecard#1097 and we're not sure why it's happening. How often does it happen?

Its happened every time the Scorecards GitHub action has run today on this repo - 5 odd times.

  • Token-Permissions - raised high severity issue since top level permissions are missing. Will address via secure-workflows
  • Dependency-Update-Tool - false positive. I do have dependabot enabled, though for some reason, dependabot has not created a PR as of now.

We currently only check for the dependabot config file being present. How would you like us to change this behavior?

One way to figure out if dependabot is enabled could be to check the last X PRs to see if one is from dependabot. I have seen similar other checks in Scorecards. While not perfect, if dependabot is enabled, there is high likelihood that it would have created some PRs...

The NotFound was fixed recently in ossf/scorecard#1277, but is not used in the action code you used yet.

  • SAST Tool - SAST tool detected but not run on all commmits - I think requirement to run it on all commits may be too high. CodeQL takes time to run. Running on PR to main and running once a week should be ok IMHO. Created Configure CodeQL to run on PR #39 to adjust CodeQL runs.

we have a relevant tracking issue on this ossf/scorecard#1268. Feel free to chime in.

  • Packaging - false positive - Not applicable as of now.

this check is unreliable, agreed.

Thanks for the feedback!

@varunsh-coder
Copy link
Member Author

varunsh-coder commented Nov 24, 2021

Hi @laurentsimon yesterday when I added the harden-runner GitHub Action to the workflow for piloting scorecards, I observed calls to bestpractices.coreinfrastructure.org and api.osv.dev. Are these expected? You can see the insights here. Request you to please review the KB for this action here. Please let me know what permissions it needs and the reasons for calling these endpoints.

Also, while I pinned all GitHub actions to their commit SHAs, I am still getting this issue flagged when one of the actions is using docker://. I believe this is a false positive. Can you please confirm? Thanks!

@laurentsimon
Copy link

Hi @laurentsimon yesterday when I added the harden-runner GitHub Action to the workflow for piloting scorecards, I observed calls to bestpractices.coreinfrastructure.org and api.osv.dev. Are these expected?

yes, those are expected. bestpractices.coreinfrastructure.org is or the CII check, api.osv.dev for the Vulnerabilities check.

You can see the insights here. Request you to please review the KB for this action here. Please let me know what permissions it needs and the reasons for calling these endpoints.

read-all should be enough.

Also, while I pinned all GitHub actions to their commit SHAs, I am still getting this issue flagged when one of the actions is using docker://. I believe this is a false positive. Can you please confirm? Thanks!

I think this is intended because you have not pinned by hash, only by latest which is mutable.

@varunsh-coder
Copy link
Member Author

Hi @laurentsimon yesterday when I added the harden-runner GitHub Action to the workflow for piloting scorecards, I observed calls to bestpractices.coreinfrastructure.org and api.osv.dev. Are these expected?

yes, those are expected. bestpractices.coreinfrastructure.org is or the CII check, api.osv.dev for the Vulnerabilities check.

Thanks! I added the info to the KB.

You can see the insights here. Request you to please review the KB for this action here. Please let me know what permissions it needs and the reasons for calling these endpoints.

read-all should be enough.

I tried to set it to read-all, but if there is another Action in the workflow that needs a write, then I could not get it to work. permissions can either be a string or a map of strings, but with read-all and security-events:write, it needed to be both string and a map, which was not working...Let me know if I am missing something. For the time being, I reviewed the code and have set the explicit read permissions. Feel free to review here.

Also, while I pinned all GitHub actions to their commit SHAs, I am still getting this issue flagged when one of the actions is using docker://. I believe this is a false positive. Can you please confirm? Thanks!

I think this is intended because you have not pinned by hash, only by latest which is mutable.

Got it. My bad. Will fix it...

@laurentsimon
Copy link

Hi @laurentsimon yesterday when I added the harden-runner GitHub Action to the workflow for piloting scorecards, I observed calls to bestpractices.coreinfrastructure.org and api.osv.dev. Are these expected?

yes, those are expected. bestpractices.coreinfrastructure.org is or the CII check, api.osv.dev for the Vulnerabilities check.

Thanks! I added the info to the KB.

You can see the insights here. Request you to please review the KB for this action here. Please let me know what permissions it needs and the reasons for calling these endpoints.
read-all should be enough.

I tried to set it to read-all, but if there is another Action in the workflow that needs a write, then I could not get it to work. permissions can either be a string or a map of strings, but with read-all and security-events:write, it needed to be both string and a map, which was not working...

my bad: you're right. security-events:write is needed to upload the results to the GitHub scanning dashboard

Let me know if I am missing something. For the time being, I reviewed the code and have set the explicit read permissions. Feel free to review here.

Also, while I pinned all GitHub actions to their commit SHAs, I am still getting this issue flagged when one of the actions is using docker://. I believe this is a false positive. Can you please confirm? Thanks!

I think this is intended because you have not pinned by hash, only by latest which is mutable.

Got it. My bad. Will fix it...

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

No branches or pull requests

3 participants