I think this is related to the reporting in general and should be fixed as one issue. Also, we may think about the config design then more, e.g., whether we should have a new dedicated job type and whether this config option couldn't be more general (applied also for the other jobs).

@siteshwar is the fail_ci_on_build_failure even though we trigger this after the successful Copr build?

We can use the term "scan" instead of "build" then.

Can you please open an issue for this?

packit/packit-service#2515

We may need two different options here, fail_ci_on_build_failure and fail_ci_on_new_findings. Both the options should be kept false by default

I agree that those options should be false by default but I think fail_ci_on_new_findings can be too broad in the sense that as far as I understand it should always fail regardless of what OpenScanHub has found. In my case I'm OK with cppcheck false positives and I'll go and look at the diff tab anyway but I think it would be great if it failed only when new compiler warnings were introduced (mostly to avoid annoying contributors).

We may need two different options here, fail_ci_on_build_failure and fail_ci_on_new_findings. Both the options should be kept false by default

I agree that those options should be false by default but I think fail_ci_on_new_findings can be too broad in the sense that as far as I understand it should always fail regardless of what OpenScanHub has found. In my case I'm OK with cppcheck false positives and I'll go and look at the diff tab anyway but I think it would be great if it failed only when new compiler warnings were introduced (mostly to avoid annoying contributors).

Your use case is specific, so you can turn this option on in your Packit configurations. Other possibility is to use action_required status for new findings, but I still believe it can be too annoying for projects with high false positive rate and developers may start ignoring this status.

I still believe it can be too annoying for projects with high false positive rate and developers may start ignoring this status

Agreed. I think another option would probably be to show something like "10 new alerts" in the check status (by analogy with how coverage services report their findings) without the whole check turning red. It seems to be safer in general. Then again it could be I'm overthinking this.

Could you please provide the exact steps to detect this bug with GCC/Cppcheck?

With cppcheck it's included in the "style" checks:

$ cppcheck --enable=style examples |& grep clari
examples/client-publish-service.c:135:12: style: Suspicious condition (assignment + comparison); Clarify expression with parentheses. [clarifyCondition]
examples/core-publish-service.c:115:8: style: Suspicious condition (assignment + comparison); Clarify expression with parentheses. [clarifyCondition]

For some reason in the avahi repository it pops up from time to time: avahi/avahi#521. I'm guessing it's because of its coding style.

OSH can transparently change the flags for both GCC and Cppcheck. However, we need to be careful with overriding the default configuration in order to keep the signal-to-noise ratio acceptable.

Got it. I agree that false positives can be annoying.

I'll take a closer look at what's going on there. I learnt about this today so It could be I'm missing something.

As far as I can see those build errors should were fixed in systemd/systemd#31709 and systemd/systemd#33108 so it should probably no longer fail to build.

A sample report can be seen on the staging instance however

I'll take a look. Thank you!

The latest full scan report for systemd can be seen here. To list context of the findings, you can run below command to get a longer report:

curl -s 'https://openscanhub.fedoraproject.org/task/10265/log/systemd-256.5-1.fc42/scan-results.js?format=raw' | csgrep

@siteshwar got it. I wonder if those reports are published somewhere periodically? As far as I can remember I followed a bunch of links mentioned in some thread where critical (or something like that) Fedora projects were discussed and saw a couple of links pointing to two reports built 2 weeks apart (or something like that). I can't seem to find those links now for some reason.

@siteshwar got it. I wonder if those reports are published somewhere periodically? As far as I can remember I followed a bunch of links mentioned in some thread where critical (or something like that) Fedora projects were discussed and saw a couple of links pointing to two reports built 2 weeks apart (or something like that). I can't seem to find those links now for some reason.

I have performed couple of mass scans this year on rawhide. Related reports can be seen here:

https://www.mail-archive.com/[email protected]/msg200973.html

https://www.mail-archive.com/[email protected]/msg199411.html

@evverx Another mass scan was performed this week. You can see the report at https://www.mail-archive.com/[email protected]/msg203529.html

@mrc0mmand @jamacku I wonder if it would make sense to turn it on for systemd?

It seems unlikely it would be turned on in systemd due to large amount of false positives and LTO support in GCC not being ready for production.

Hi! Thanks for the feedback, I would say what you wrote makes sense, we should store and display the HTML versions. I will fix it as part of packit/dashboard#441 where we would like to also process the data further (using the mentioned csgrep) and report using GitHub security UI capabilities.

Thank you!

Is this expected? It's a bit annoying from the UX point of view, since the HTML versions are much more human friendly, and right now you have to click through the first link to get to them.

This has been already fixed in production. Now you get a direct link to added findings html report if a new finding is identified.

It has been fixed now. For example, you can see the scan for the packit package was successful and it uses dynamic dependencies.

I tried to workaround this issue from the configuration side, but it seems that is not sufficient for all the builds to be successful. For example, tmt scans are still failing. --calculate-build-deps option is more suited to be used from the command line and should be fixed in csmock.

I have deployed the fix from csmock in production and the scans are passing now.

Hi, now there is no way for the user to check, I looked into our internal logs:

2024-11-12T08:17:35.479607588+00:00 stderr F [2024-11-12 08:17:35,479: DEBUG/MainProcess] task.run_copr_build_end_handler[43127464-640b-4675-b2ab-f6bf43c29db3] Failed to download file from https://download.copr.fedorainfracloud.org/results/@codescan/csutils/srpm-builds/07776695/cscppc-2.2.5.20240722.150725.g1bbbfa0-1.src.rpm: HTTPError('404 Client Error: Not Found for url: https://download.copr.fedorainfracloud.org/results/@codescan/csutils/srpm-builds/07776695/cscppc-2.2.5.20240722.150725.g1bbbfa0-1.src.rpm')

looking into commits in main, that makes sense as the previously last commit was from July so I assume Copr deleted the artifacts after that time.

We could improve the UX by displaying the URLs of the SRPMs we try to download, WDYT?

We could improve the UX by displaying the URLs of the SRPMs we try to download, WDYT?

Can you display the error message to the user?

We could improve the UX by displaying the URLs of the SRPMs we try to download, WDYT?

Yes, that would help for sure!

Can you display the error message to the user?

yes we could do both

I am still getting these failures without any details about which SRPMs Packit tried to download. For example, on a today's CI run on a csdiff pull request: https://github.com/csutils/csdiff/pull/217/checks?check_run_id=36715144190

yes, thanks for reminding, updated!