Skip to content
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

Implementation of GitHub Bandit action #6

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lukehinds
Copy link
Member

A lot to say on this, so likely better to discuss in comments.

Initially this Action will run on itself, to allow easier preview /review. Once review is satisified, we can remove the workflow file or point it to run against post_comment.py only

A lot to say on this, so likely better to discuss in comments.

Initially this Action will run on itself, to allow easier preview
/review. Once review is satisified, we can remove the workflow file
or point it to run against `post_comment.py` only

Signed-off-by: Luke Hinds <[email protected]>
@lukehinds lukehinds requested a review from ericwb as a code owner March 2, 2024 19:39
bandit-action:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Copy link
Member

Choose a reason for hiding this comment

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

The latest version for checkout is v4

Suggested change
- uses: actions/checkout@v2
- uses: actions/checkout@v4

with:
path: "."
recursive: "true"

Copy link
Member

Choose a reason for hiding this comment

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

Nit: extra blank line

Dockerfile Outdated
# Assuming the Dockerfile is located at the root of the repository
COPY post_comment.py /post_comment.py

ENTRYPOINT ["/entrypoint.sh"]
Copy link
Member

Choose a reason for hiding this comment

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

Nit: no endline

README.md Outdated
Comment on lines 3 to 5
This is the official GitHub Action for running [Bandit](https://bandit.readthedocs.io/en/latest/),
developed by the maintainers of Bandit. It is designed to be configurable and
easy to use.
Copy link
Member

Choose a reason for hiding this comment

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

The line breaks here are a little off. Maybe limit to 80 chars.

README.md Outdated
bandit-action:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- uses: actions/checkout@v2
- uses: actions/checkout@v4

README.md Outdated
bandit-action:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- uses: actions/checkout@v2
- uses: actions/checkout@v4

action.yml Outdated
targets:
description: |
Source file(s) or directory(s) to be tested
name: 'Bandit Code Scan'
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the YAML file here would require single quotes for strings.

description: 'Run Bandit code scans on your Python codebase'
inputs:
GITHUB_TOKEN:
description: 'GitHub token'
Copy link
Member

Choose a reason for hiding this comment

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

I believe GitHub now refers these to "personal access tokens". And the latest version have fine-grained permissions that can be assigned. Therefore guidance on what minimal set of permissions needed here would be useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah right, this is an inbuilt Token that is created for each workflow run and then destroyed at the end. No need for the user to do anything:

https://docs.github.com/en/actions/security-guides/automatic-token-authentication#about-the-github_token-secret

Dockerfile Outdated
RUN pip install bandit
# Install additional dependencies if necessary
RUN apk add --no-cache git bash python3 py3-pip && \
pip install pygithub
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pip install pygithub
pip install PyGithub

entrypoint.sh Outdated
echo "Constructed command: $cmd"


# Force the output format as JSON and output file, we json and to report.json
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Force the output format as JSON and output file, we json and to report.json
# Force the output format as JSON and output file, we default to json and to report.json

Signed-off-by: Luke Hinds <[email protected]>
@lukehinds lukehinds requested a review from sigmavirus24 as a code owner March 3, 2024 08:39
Copy link
Member

@sigmavirus24 sigmavirus24 left a comment

Choose a reason for hiding this comment

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

Should this leverage the new SARIF format?

@ericwb
Copy link
Member

ericwb commented Mar 9, 2024

Should this leverage the new SARIF format?

Yes, I think we can change this action to be a composite-type action and output to SARIF so it automatically post results to the Code Scanning tab of a repository.

@sigmavirus24
Copy link
Member

I wonder if we should have a bandit to SARIF converter too so people can get the output in a comment and as SARIF so people who pr against a repo can see the results too in CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants