superset icon indicating copy to clipboard operation
superset copied to clipboard

feat: security ci to check vulnerability in superset added for superset

Open hash-data opened this issue 2 years ago • 7 comments

SUMMARY

Security ci contains the following modules which detect the vulnerabilities -> Bandit -> PIP Audit -> safety

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • [ ] Has associated issue:
  • [ ] Required feature flags:
  • [ ] Changes UI
  • [ ] Includes DB Migration (follow approval process in SIP-59)
    • [ ] Migration is atomic, supports rollback & is backwards-compatible
    • [ ] Confirm DB migration upgrade and downgrade tested
    • [ ] Runtime estimates and downtime expectations provided
  • [ ] Introduces new feature or API
  • [ ] Removes existing feature or API

hash-data avatar Jun 23 '23 04:06 hash-data

@dpgaspar I have created a workflow that provide information about the vulnerability. Mostly uses open source projects and database. Mentioning the issue here https://github.com/apache/superset/issues/23621

Allow the workflow to run to check the vulnerable issues.

Also thanks for the great project ❤️

hash-data avatar Jun 23 '23 05:06 hash-data

@villebro have you guys checked ?

hash-data avatar Jul 03 '23 10:07 hash-data

It seems this PR slipped off our collective radar. It's still an interesting idea, but seems to have failed CI because of the added test itself. Maybe we can set some stuff to warn instead of error, or clean up the errors to get it through? In any case, I'm closing and reopening to reboot CI, since the logs have vanished.

rusackas avatar Aug 19 '24 22:08 rusackas

Hi, @rusackas thanks for looking at it. @mistercrunch That is a great idea for sending alerts, for the second one: it must run on every pr as there can be vulnerabilities that are found later not when the dependency gets upgraded.

hash-data avatar Aug 20 '24 07:08 hash-data

it must run on every pr as there can be vulnerabilities that are found later not when the dependency gets upgraded.

So say if I open a super simple PR fixing a typo, it shouldn't be held back because a vulnerability was found in some package that's completely unrelated to my PR. If we run bandit on every PR it means all of the open PRs (say all 300+ of them open right now) would start failing CI for things totally out of scope/knowledge for all these contributors. Also, once it's fixed in master by some PR, all other 300+ PRs now need to be rebased to pass the checks. I also don't think we want master CI (happens every time we merge a PR) to start failing semi-randomly, in a way that's unrelated to what was merged.

That's why I'm suggesting a workflow on a schedule (say @daily, which is supported by GHA) that notifies committers through email ([email protected]), but doesn't break ongoing unrelated to PRs or the builds on master

mistercrunch avatar Aug 21 '24 01:08 mistercrunch

@mistercrunch agreed with it, a scheduled workflow is fine. So to summarize -> Must run on PRs that are doing some changes in dependencies -> A Scheduled action that checks on a daily or weekly basis for vulnerabilities and send mail or alerts.

I will do these changes

Should we use something like this reusable action to simplify things? -> https://github.com/PyCQA/bandit-action

hash-data avatar Aug 21 '24 10:08 hash-data

Yes exactly!

Here's an example of using paths: in GitHub actions https://github.com/apache/superset/blob/master/.github/workflows/check_db_migration_confict.yml#L4-L5

Given bandit is python-only, I think the path you want to add is requirements/**

mistercrunch avatar Aug 23 '24 00:08 mistercrunch

This one seems to have gone inactive for quite a while now. Any interest/bandwidth to get it through?

rusackas avatar Apr 22 '25 02:04 rusackas

hey @rusackas will update it

hash-data avatar Apr 22 '25 06:04 hash-data