Link to overcommit?

[lencioni]

overcommit is a fully extendable and configurable Git hook manager that contains out-of-the-box support for brakeman. Do you think there would be a good place to mention this in the brakeman documentation?

[presidentbeef]

Hi Joe,

We do not currently have a page linking to Brakeman-compatible tools. There is https://github.com/presidentbeef/brakeman/wiki/Brakeman-as-a-Service but that is not quite the same.

Would you like to start a page? I think the Brakeman wiki is probably the best place to start and then it can be moved to the site later.

[lencioni]

Cool. Not sure there would be enough content to justify a whole page quite yet. How about just a section on the main wiki page titled "Tools" just above "Brakeman Users"?

[lencioni]

I went ahead and did that. https://github.com/presidentbeef/brakeman/wiki#tools We can always move it if you like.

[presidentbeef]

Hi Joe,

The approach you are taking, like pronto-brakeman, is not recommended. Brakeman works best when analyzing the whole program. Limiting the scan to a set of changed files may cause it to miss vulnerabilities even in the specified files, or it may cause false positives. Changes in one file can affect how another file is interpreted.

Anyway, like I said it's not recommended but of course you are welcome to provide it.

[lencioni]

Thanks! I wasn't really involved with the brakeman overcommit integration (that was mostly @jfelchner), so I don't have a lot to say on the subject.

It seems like missing vulnerabilities is not a big deal for a pre-commit hook because pre-commit hooks should not be used to enforce rules--that is the role of CI--they are useful for early detection to prevent roundtrips to code review. However false positives seem like they might pose an actual problem here. Can you elaborate?

[jfelchner]

@presidentbeef I agree but it will certainly catch some things. It's not that it's the best way to run it, but it's certainly better than trying to remember running it in full before every commit.

My thoughts are:

• Let overcommit run on the single files. Catch what it can.
• Let CI always run the full test and fail if it finds any problems with the entire codebase

At least that's how we've done it and CI rarely (I actually can't even remember the last time) finds an error that wasn't caught by only running the changed files.

[presidentbeef]

That is a reasonable approach. I just worry people may be misled if they are relying solely on tools using --only-files.

[presidentbeef]

I've added https://github.com/presidentbeef/brakeman/wiki/Tools-Using-Brakeman

[lencioni]

:+1: