Evaluate the use of Sarif instead of Reviewdog

brave / security-action

Composite GitHub CI Action containing the minimal viable security lint for brave repositories

Mozilla Public License 2.0

17 stars 6 forks source link

Evaluate the use of Sarif instead of Reviewdog #65

Open thypon opened 1 year ago

thypon commented 1 year ago

Pro

It's supported by GitHub natively
Fancy new thingy

Cons

We need to port the checks we have
We need to rewrite the missing checks (xmllint one, maybe)
We need to rewrite and get the other scripts working:
- tfsec with multi-dir support, and proper diffing
- xmllint that does not support any output if not text
- blocklist for semgrep findings and custom rulesets

Open Qs

Rather limited console support, so slow iteration (maybe?)
How do we test that the serif output is fine? Is there any validator?

thypon commented 1 year ago

Another Q. Can we control how we output the findings? I'm good with the comments flow and find the less discursive approach a bit lacking.

thypon commented 1 year ago

Some reference: https://github.com/microsoft/sarif-tutorials/blob/main/docs/1-Introduction.md

thypon commented 1 year ago

Reviewdog incident: https://github.com/brave/security-action/commit/4ff345c1deeff6652163fd8cb2d232d411fa1aa5 <-- fixed

thypon commented 1 year ago

A major point is that besides the output format we should however use find/xargs to limit the scope of the tools. In-fact most of the tools don't support files level scanning, but sub-directories only (like tfsec). For this reason we should hack support for differential scanning.

Differential scanning reduces the cost on the github action runtime and for some repo allows to work properly. In-fact tfsec, while awesome, does not even do recursive scanning for the main .tf file which is delegated to the user, or a script.