Skip to content

Add CodeQL and Bandit Static Analysis Scans #560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 21, 2025

Conversation

kkraus14
Copy link
Collaborator

Description

Resolves #534

Adds scans using both CodeQL and Bandit. Could use some discussion on what level of reporting we wish to have here and when we want to error. I have updated the repo settings to alert on any Security alert severity level and set the Standard alert severity level to "Errors and warnings" as a starting point.

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Copy link
Contributor

copy-pr-bot bot commented Apr 15, 2025

Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@kkraus14
Copy link
Collaborator Author

/ok to test b8d0441

@kkraus14
Copy link
Collaborator Author

/ok to test b8d0441

This comment has been minimized.

cryos
cryos previously approved these changes Apr 16, 2025
Copy link
Collaborator

@cryos cryos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, I added a proposal for concurrency groups that are pretty simple and should work well, choosing something that should work well for both merges to main and PRs.

@cryos
Copy link
Collaborator

cryos commented Apr 16, 2025

This needs the addition of bandit to the whitelist as noted in slack.

@cryos
Copy link
Collaborator

cryos commented Apr 16, 2025

Bandit should be allowed to run now if you want to retry it.

@kkraus14
Copy link
Collaborator Author

/ok to test 634f56a

@kkraus14
Copy link
Collaborator Author

@leofang do you want me to add bandit / codeql to pre-commit before we merge this?

@kkraus14
Copy link
Collaborator Author

Do not merge. Needs an internal discussion before moving forward.

@leofang
Copy link
Member

leofang commented Apr 17, 2025

do you want me to add bandit / codeql to pre-commit before we merge this?

I think it is fine to do it in a separate PR, so we only need to resolve the internal discussion before merging.

Comment on lines +19 to +22
- repo: https://github.com/PyCQA/bandit
rev: 8ff25e07e487f143571cc305e56dd0253c60bc7b #v1.8.3
hooks:
- id: bandit
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does pin the version of Bandit to v1.8.3 which could cause mismatches between this and the GitHub workflow.

@kkraus14
Copy link
Collaborator Author

An issue here:

I've also temporarily moved the CodeQL action to be manually triggered only until our internal discussion is completed.

@leofang leofang added support All things related to the project that can't be categorized P0 High priority - Must do! CI/CD CI/CD infrastructure labels Apr 18, 2025
@kkraus14
Copy link
Collaborator Author

/ok to test 4d7632c

@leofang leofang added this to the cuda-python 12.9.0 & 11.8.7 milestone Apr 21, 2025
@kkraus14
Copy link
Collaborator Author

Merging for now and will create issues for following up on Bandit version pinning for the Action and CodeQL pre-commit hook.

@kkraus14 kkraus14 merged commit d425a88 into NVIDIA:main Apr 21, 2025
75 checks passed
@leofang
Copy link
Member

leofang commented Apr 21, 2025

Thanks, Keith!

Copy link

Doc Preview CI
Preview removed because the pull request was closed or merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD CI/CD infrastructure P0 High priority - Must do! support All things related to the project that can't be categorized
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: Set up a static code analysis tool
3 participants