beanshell / beanshell

Beanshell scripting language
Apache License 2.0
845 stars 182 forks source link

Coverity automation #83

Closed nickl- closed 1 year ago

nickl- commented 6 years ago

Created on behalf of @pgiffuni

It would be really nice to get the Coverity automation working on github. Just to give ourselves something else to co ;).

nickl- commented 6 years ago

I can't find coverity are you referring to synopsys?

Wasted quite a bit of time trying to get scrutinizer configured... got to love it when things just don't work =( Basically I wanted test coverage reports and finally found a solution with codecov which required zero configuration... booyah!!! See README for banner.

Please point me in the right direction and I will gladly give it a shot.

nickl- commented 6 years ago

Ok so if it was synopsis (Coverity Scan) then I've tried.

I registered with GitHub account went to DashBoard to add project, everything but beanshell appears as my GitHub projects. The tip says to see if my member status is public, chased that wild goose too.

Proceeded to manually add the project... hit a What The?!?!? it expects a reference URL which from what I can tell is a link to a commit in my name, supposedly to prove my involvement or something... what's the point of this? I can have commits in a repository if a PR of mine was committed. So what is this trying to prove exactly.

Just before I submitted it caught my eye that there will be a 48 hour approval delay... now who has time for that? Screw this...

Is it too much to ask to expect a public and live system to work? There really isn't much more to it.

Now I have to waste more time to try and find and remove whatever the hell this dodgy service provider (obviously dodgy or their crap would work) installed on my GitHub account for no reason.

Do you think I am likely to return or suggest this service to anyone else?

pgiffuni commented 6 years ago

Sorry for the delay ... Indeed I meant coverity-scan which was bought by Synopsis It is supposed to be relatively easy to setup from Travis, which we have, following the instructions here.

I tried once to add the project logging with my github account but failed miserably. What you mention sounds familiar, so I guess we need someone with patience :(. The service is rather nice once you get it working.

nickl- commented 6 years ago

@pgiffuni Sweet... at least we are talking about dim same apples.

If you also struggled getting the project configured then indeed, it appears to be for everyone and not just for us. This probably means the next approach should be manually adding a project and waiting the 48 hrs for moderation.

Kinda lost the warm and fuzzies after the initial struggle which now unfortunately feels like a chore. If I do find myself needing a challenge I will certainly give it another whirl. Watch this space for more...

nickl- commented 6 years ago

Not quite sure what to report back, it seems to be doing something.

Had to remove "application access restrictions" under the project settings, "Third-party application access policy" for coverity to list our project. It appears that we only have coverity application access on a personal level and this restriction appears to prevent personal applications from accessing project data. The status now reads:

All applications authorized by organization members have access to beanshell’s data.

And adds a less comforting red exclamation mark icon on the page which leaves much to be desired.

Only took 3 attempts to configure travis before coverity accepted the token and allowed the magic to happen... a build was uploaded to the project page and after a little patience it was analysed.

The report states we have 39 defects which is very surprising I expected much worse. However it appears they are preventing me from accessing the details. (Hurry up and wait...) From what I can tell this is pending a 1 to 2 business day lock for someone to review my association as well as our projects open source-ability or some such.

I invited @pgiffuni and @stain, since I can confirm you are active but should anyone else require access please don't hesitate to shout.

Watch this space for more feedback... in or around 1 or 2 business days give or take =)

pgiffuni commented 6 years ago

I can't access yet .. apparently beanshell is not registered yet within coverity, but I'll keep an eye on the account.

Great job!

nickl- commented 6 years ago

@pgiffuni did they deliver your invite?

Still waiting on the project review perhaps that is why it is not visible. I also see no change to the members section which I assume will also update at some point.

Great job!

Tx but I still can't be sure everything is in order yet. At least something is happening/happened which is more than we had before. =)

pgiffuni commented 6 years ago

No invite. I logged in with my github account and I get this: "You are not authorized to access this page."

nickl- commented 6 years ago

Ok something just happened which hasn't happened before, your user was added to the project member list and I sent the invite again, using the apache.org email address that I have of yours.

The project status however still remains pending their review so we wait some more. Let me know what you find...

nickl- commented 6 years ago

Invitations created successfully. 1 users added to project. 1 invitation email sent successfully.

nickl- commented 6 years ago

And we have liftoff... project reviewed.

pgiffuni commented 6 years ago

I added a badge to the readme! Great job, really!

nickl- commented 6 years ago

👍

pgiffuni commented 6 years ago

Small note: I activated the Findbugs(TM) reports, so we may see a sudden surge in bugs, including false positives, for the next sweep.

nickl- commented 6 years ago

Oh great yeah! =)

@pgiffuni I am reopening this thread, something tells me there is still some to discuss.

We already have 2 false positives but I was not able to create the appropriate "modelling file" for this. Their documentation shows the C versions of what they require but for java they don't have too much to say and what they do say is how to increase detection not how to avoid already detected false positives.

The option of adding annotations makes zero sense. If they did this the same way as with C by identifying simple comments I would agree. Instead they implemented this with java annotations which require us to have a coverity dependency in order to describe the annotation objects. Unless you disagree with me, I don't believe the costs involved in adding additional dependencies is justified for code analysis.

You obviously noticed I managed to clear the majority of the defects but the most alarming ones they don't seem to identify. The CWE-22 which they claim they found 2 occurrences of and has it flagged as "High impact security" but they fail to give us any indication of how or where we are supposed to find this.

I completely understand what CWE-22 is referring to, it basically needs us to prevent file traversal access outside of a predefined sand box. ie. if current working directory is /home/username then you should not be able to supply a path ../../etc and gain access to the system configuration.

This is all fine and well but without telling me where this is happening patching against their panic is going to be very difficult. There are several places in our code where this could be raised some of them will not be a concern others a necessity ie. class path needs to be able to go anywhere you please but unless you are pointing to java classes (jar files, modules, etc) it will not be of any use to you.

Before applying any restrictions we first need to identify if it is a risk and whether we need to fix it and how we want to restrict traversal (ie what should be the sandbox). Without knowing what exactly they have flagged I don't know how we are going to address this. Your thoughts?

pgiffuni commented 6 years ago

Sorry for the delay in answering: my desktop needed some repairs :(.

I tried to look for the Java modelling examples but they aren't really there (?). For C, Coverity accepts the classic LINT comments but on Java I don't think there is an explicit equivalent. I agree that annotating the code doesn't make much sense. We also certainly don't want to start adding coverity-specific comments that no one else can use without a Coverity license.. You can comment and tag false positives in the Coverity platform itself and that should make such issues appeared as "analyzed".

IMHO it is not really important to get clean builds in Coverity: they are basically warnings and they are not necessarily right. I will disable the optional "FindBugs reports": it already found what it was supposed to find. We probably should re-enable it briefly before the next release but in general I don't think we should pay too much attention to Coverity beyond the initial sweep or the codebase.

I do think it is a rather bad practice to hack around potential issues on the Coverity branch: It is important to keep the Coverity branch synced with the master version and let the tool do its job passively.

pgiffuni commented 5 years ago

FWIW, Coverity is temporarily down:

https://community.synopsys.com/s/article/Coverity-Scan-Update

In short:

On Monday, January 7, 2019 our team was notified that our Coverity Scan hosting service provider unexpectedly ceased operations, making the servers that support Scan unavailable. We are working to stand up new servers to restore the service as soon as possible. We apologize for any inconvenience.

nickl- commented 5 years ago

@pgiffuni Thanks for the heads up!

nickl- commented 1 year ago

Closed resolved we have coverity coverage automation