tokio-rs / prost

PROST! a Protocol Buffers implementation for the Rust Language
Apache License 2.0
3.95k stars 506 forks source link

Repository security settings can be strengthened. #1184

Open amaranthjinn opened 2 weeks ago

amaranthjinn commented 2 weeks ago

Our team wants to use prost for an ongoing project, however, we are concerned about the risk of bad changes making into the repository, introducing vulnerabilities into our project given how prevalent software supply chain attacks have become.

We used the tool https://github.com/ossf/scorecard?tab=readme-ov-file#using-scorecard to help us assess the risk of using tonic. It suggested that some areas seem to be weak against bad behaviors:

branch protection - Warn: branch 'master' does not require approvers Warn: codeowners review is not required on branch 'master'. See https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection for more details.

token permission - Warn: no topLevel permission defined: .github/workflows/ci.yml:1 Warn: no topLevel permission defined: .github/workflows/cifuzz.yml:1. See https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions for more details.

Those seem to be concerns that can be addressed fairly quickly, and can help increase the trust of the package so much. Really appreciate it if the settings can be strengthened soon.

Steps To Reproduce See https://github.com/ossf/scorecard/tree/main?tab=readme-ov-file#scorecard-command-line-interface for instruction on running the tool.

Run security scan against the prost repo: scorecard --repo=https://github.com/tokio-rs/prost --checks=Dangerous-Workflow,Maintained,Vulnerabilities,Binary-Artifacts,Branch-Protection,Code-Review,Token-Permissions,Signed-Releases,Dependency-Update-Tool --show-details

caspermeijn commented 1 week ago

We used the tool https://github.com/ossf/scorecard?tab=readme-ov-file#using-scorecard to help us assess the risk of using tonic. It suggested that some areas seem to be weak against bad behaviors:

You mention tonic here. I assume you assessed both prost and tonic and this is a copy-paste error.

branch protection - Warn: branch 'master' does not require approvers Warn: codeowners review is not required on branch 'master'. See https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection for more details.

You are right, we don't have this kind of branch protection enabled. Mainly because there is not enough review capacity. I try to keep my PRs open for at least a week, but I almost never recieve any comment on them. If you are willing to provide code reviews, then we can consider enabling this protection.

token permission - Warn: no topLevel permission defined: .github/workflows/ci.yml:1 Warn: no topLevel permission defined: .github/workflows/cifuzz.yml:1. See https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions for more details.

I opened a PR to restrict the token permissions. Can you verify this fixes the warning? https://github.com/tokio-rs/prost/pull/1189

amaranthjinn commented 4 days ago

Thank you for addressing the issues! Yes, 'tonic' is a copy/paste error. https://github.com/tokio-rs/prost/pull/1189/ looks good to me, merge it! I will run the tool after the fix in and let you know the updated score?

amaranthjinn commented 20 hours ago

The repo security score is 8.2/10 now, after the token permission fix. Thank you!