search

nccgroup / sobelow

Security-focused static analysis for the Phoenix Framework
Apache License 2.0
1.66k stars 92 forks source link

sobelow should have better default flags #163

Open marcandre opened 4 months ago

marcandre commented 4 months ago

Options --config and --skip should be the default, they should not be necessary.

I know of no other program (none!) that does not take their .some-config into account by default.

Feel free to add --no-config and --no-skip, although I don't see a utility for them.

houllette commented 3 months ago

Hey @marcandre - while I don't disagree, just wanted to shed some light on why the design of Sobelow is the way it is currently:

Sobelow was originally made as a security tool to aide in evaluating codebases locally (whether that was a Security professional or a developer looking to secure their code) - I can't say for certain, but I believe it's because of that specific use case in mind that the architecture of the codebase isn't as natively conducive to the CI/CD use case that having better defaults would allow for.

In #159 you brought up the following point:

The question is: are there any users calling sobelow without the --skip option that would be impacted?

And the problem is, frankly I have no idea if any users would be impacted and its hard to figure out if that would be the case or not. To me that sounds like a potentially breaking change, while I'm not adverse to those (namely Elixir/OTP version bumps), I'm always still hesitant to introduce them given the fact that many abstractions are built on top of Sobelow (GitLab Security Scanning, Paraxial.io, etc.) that I don't know exactly how they're anticipating how functionality works beyond how it exists today.

Again, not saying this isn't a worthwhile change or something to pursue (PRs are always welcome from folks besides me) - just wanted to lay out all the context I have and outline my concerns.

marcandre commented 3 months ago

Thanks for the reply.

I agree these are breaking changes.

I disagree that this request has anything to do with CI/CD. I'd say its the contrary. Current setup makes it difficult to setup CI/CD as I believe noone would think to initially use --config and --skip options. But that's a one time thing. Once it is setup, it will work for that project.

But anytime a user calls sobelow locally, they have to type in these options. I don't have a sufficient memory to remember that. So every single time I'm using sobelow outside of CI/CD, it will not give me the correct results and I have to figure out why.

I'll be glad to produce a PR if you greenlight the changes