nccgroup / sobelow

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

Add new output format for sonarqube #106

Open juancgalvis opened 2 years ago

juancgalvis commented 2 years ago

It would be very useful if the sobelow task could generate the output in sonarqube format, following the generic issues structure of sonarqube, I am interested in contributing and I have advance work, what do you think of this feature? I would really appreciate it if you would accept it.

The basic idea is to have the next task parameter value for the option --format, -f

mix sobelow -f sonarqube
houllette commented 1 year ago

I really like the concept behind this (also thank you for the MR, looks awesome!), but I have one high level concern - it would probably be best to avoid vendor lock-in within the codebase.

Looking through your code changes, it seems like there is a lot of base level code implemented to support Sonarqube in Sobelow. I can think of a few other formats that I think it would be beneficial to support (e.g. SIEM integrations) and instead of going down the rabbit hole of supporting each one, it might be better to take a more generic approach by allowing for Sobelow configuration files to define the outputted object.

Still a rough idea in my head, so I'd be curious to hear your thoughts @juancgalvis.

juancgalvis commented 5 months ago

Hi @houllette,

Sorry for not responding sooner, I was on other topics, returning to this idea. If you look at the changed code base, what I proposed was to generate an abstraction using a behavior called formatter, which is why you see many changes. I made this proposal so that this module was not huge and loaded with the details of each formatter, but really the functional change is:

In reality the code base does not change, only the formatter changes given the format by parameter.

If you want, I can add strictly the sonarqube minimum required changes

houllette commented 5 months ago

Hey @juancgalvis - welcome back to the project! 😄

I had looked at the code at the time and what I was mainly referring to in my comment was the hardcoded references to Sonarqube in order to support functionality. I think the abstractions you made to general the formatter are awesome (plus I always appreciate cleaner code), but I'm avoidant to push in hardcoded support for a specific vendor.

I think if instead of it being specifically built out modules required for every SIEM format we want to support, if the formatter supported configurable output format via a yaml config file (or something like that) - it would be endlessly supportive of any SIEM that folks need on an individual basis without needing the codebase to explicitly support that SIEM. As a small project, this approach would push the responsibility of maintaining the correctness of the integration onto whomever needs that specific format type rather than the project maintainers being responsible for ensuring the validity of every new SIEM format we want to support.

Additionally, this could be a good candidate for something like a plugin mechanism as suggested in #94 - where new features may require more code changes to / lower level interactions with Sobelow to support it, but we wish to avoid clogging up core components with one-off functionality.

As a final note - it has been a hot minute since your original PR and the code base has undergone quite a bit of changes / upkeep since I've taken over maintaining it, so I would recommend starting from a fresh fork if this is something you wish to keep working on!

juancgalvis commented 5 months ago

That's clear, sure I can work from a new base of code having your position clear, thanks.