Skip to content

Add CodeQL and Bandit Static Analysis Scans#560

Merged
kkraus14 merged 6 commits into
NVIDIA:mainfrom
kkraus14:static_analysis
Apr 21, 2025
Merged

Add CodeQL and Bandit Static Analysis Scans#560
kkraus14 merged 6 commits into
NVIDIA:mainfrom
kkraus14:static_analysis

Conversation

@kkraus14

Copy link
Copy Markdown
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-pr-bot

copy-pr-bot Bot commented Apr 15, 2025

Copy link
Copy Markdown
Contributor

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.

Comment thread .github/workflows/codeql.yml
@kkraus14

Copy link
Copy Markdown
Collaborator Author

/ok to test b8d0441

@kkraus14

Copy link
Copy Markdown
Collaborator Author

/ok to test b8d0441

@github-actions

This comment has been minimized.

cryos
cryos previously approved these changes Apr 16, 2025

@cryos cryos left a comment

Copy link
Copy Markdown
Collaborator

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.

Comment thread .github/workflows/bandit.yml
Comment thread .github/workflows/codeql.yml
@cryos

cryos commented Apr 16, 2025

Copy link
Copy Markdown
Collaborator

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

Comment thread .github/workflows/codeql.yml
@cryos

cryos commented Apr 16, 2025

Copy link
Copy Markdown
Collaborator

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

@kkraus14

Copy link
Copy Markdown
Collaborator Author

/ok to test 634f56a

@kkraus14

Copy link
Copy Markdown
Collaborator Author

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

@kkraus14

Copy link
Copy Markdown
Collaborator Author

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

@leofang

leofang commented Apr 17, 2025

Copy link
Copy Markdown
Member

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 thread .github/workflows/bandit.yml
Comment thread .pre-commit-config.yaml
Comment on lines +19 to +22
- repo: https://github.com/PyCQA/bandit
rev: 8ff25e07e487f143571cc305e56dd0253c60bc7b #v1.8.3
hooks:
- id: bandit

Copy link
Copy Markdown
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
Copy Markdown
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
Comment thread .github/workflows/codeql.yml Outdated
@kkraus14

Copy link
Copy Markdown
Collaborator Author

/ok to test 4d7632c

@leofang leofang added this to the cuda-python 12.9.0 & 11.8.7 milestone Apr 21, 2025
Comment thread .pre-commit-config.yaml
@kkraus14

Copy link
Copy Markdown
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
@leofang

leofang commented Apr 21, 2025

Copy link
Copy Markdown
Member

Thanks, Keith!

@github-actions

Copy link
Copy Markdown
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