Skip to content

Sync with upstream and update README and dependencies #18

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.

Sign up for GitHub

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

Open
reactive-firewall wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Sync with upstream and update README and dependencies #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c5150a2
Prepare to merge with upstream fork from shundor/bandit-scan#2
reactive-firewall Apr 11, 2025
e2e29b9
Merge branch 'main' into sync-with-upstream-10
reactive-firewall Apr 11, 2025
ad6fae3
Backport of dependency automation from reactive-firewall/python-bandi…
reactive-firewall Apr 11, 2025
11a72c7
[REVIEW] Simpler defaults for backported dependabot.yml
reactive-firewall Apr 11, 2025
919c5b3
[UPDATE] Sync Dev with main (#15)
reactive-firewall Jul 28, 2026
945f6f6
Apply batched suggestions from code review
abirismyname Jul 29, 2026
9f79db2
[PATCH] Resolve backport fixes with PR shundor/python-bandit-scan#6
reactive-firewall Aug 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ To run a bandit scan include a step like this:
level: low
confidence: high
# exit with 0, even with results found
# exit_zero: true # optional, default is DEFAULT
# exit_zero: true # optional, default is DEFAULT (exit with results-based value)
```

## Inputs
Expand Down
12 changes: 10 additions & 2 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,20 @@ runs:
INI_PATH="--ini $INPUT_INI_PATH"
fi

CONFIG_PATH_ARG=""
if [ "$INPUT_CONFIG_PATH" == "DEFAULT" ]; then
# Clear both
CONFIG_PATH=""
CONFIG_PATH_ARG=""
unset CONFIG_PATH 2>dev/null || true # also try to unset
else
CONFIG_PATH="-c $INPUT_CONFIG_PATH"
if [ -n "$INPUT_CONFIG_PATH" ] ; then
# Set both (but let bandit validate its own arguments)
CONFIG_PATH="$INPUT_CONFIG_PATH" # quote paths on assignment
CONFIG_PATH_ARG="-c"
fi
fi
bandit -f sarif -o results.sarif -r $INPUT_PATH $LEVEL $CONFIDENCE $EXCLUDED_PATHS $EXIT_ZERO $SKIPS $INI_PATH $CONFIG_PATH
bandit -f sarif -o results.sarif -r $INPUT_PATH $LEVEL $CONFIDENCE $EXCLUDED_PATHS $EXIT_ZERO $SKIPS $INI_PATH $CONFIG_PATH_ARG ${CONFIG_PATH:-}
Comment on lines +117 to +123

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail
rg -n -C 2 'config_path|ini_path|INPUT_CONFIG_PATH' action.yml README.md

Repository: reactive-firewall/python-bandit-scan

Length of output: 1768

Add config_path to the README.

action.yml defines and passes config_path to Bandit with -c, including the YAML/TOML support. The README currently documents only ini_path for .bandit input. Add a config_path entry so users know both configuration mechanisms are available.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@action.yml` around lines 117 - 123, Update the README configuration-input
documentation to add the config_path option alongside ini_path, describing that
it passes a YAML/TOML Bandit configuration file via -c. Keep the existing
ini_path documentation unchanged and accurately reflect the action.yml
interface.

Comment on lines +119 to +123

@coderabbitai coderabbitai Bot Aug 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail
CONFIG_PATH='configs/security policy.yaml'
CONFIG_PATH_ARG='-c'
printf '<%s>\n' $CONFIG_PATH_ARG ${CONFIG_PATH:-}

Repository: reactive-firewall/python-bandit-scan

Length of output: 212

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail
printf 'Files:\n'
git ls-files | rg '(^|/)action\.y(a)?ml$|(^|/)README\.md$' || true
printf '\nRelated action.yml section:\n'
sed -n '90,145p' action.yml
printf '\nSearch config options:\n'
rg -n "CONFIG_PATH|input_config_path|config:" README.md action.yml || true

Repository: reactive-firewall/python-bandit-scan

Length of output: 2743

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail
printf 'Demonstrating Bash expansion behavior:\n'
for VALUE in 'configs/security policy.yaml' 'configs/*.yaml' ''; do
  CONFIG_PATH_ARG='-c'
  CONFIG_PATH="$VALUE"
  printf 'Input: <%s>\n' "$VALUE"
  printf 'printf outputs:\n'
  printf '<%s>\n' $CONFIG_PATH_ARG ${CONFIG_PATH:-} || true
  printf '\n'
done

printf 'Demonstrating Bash array behavior:\n'
CONFIG_PATH_ARGS=(-c 'configs/security policy.yaml')
printf 'printf outputs:\n'
printf '<%s>\n' "${CONFIG_PATH_ARGS[@]}"
printf '\n'

Repository: reactive-firewall/python-bandit-scan

Length of output: 489

Preserve INPUT_CONFIG_PATH as one Bandit argument.

Line 119 quotes the assignment, but line 123 expands ${CONFIG_PATH:-} without quotes. Bash will split whitespace- or wildcard-containing path values into multiple arguments. A path such as configs/security policy.yaml then sends configs/security and policy.yaml to Bandit, and an extra token can be mistaken for a Bandit option. Use a Bash array for the optional option and path, and expand it with "${CONFIG_PATH_ARGS[@]}".

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@action.yml` around lines 119 - 123, Update the Bandit invocation around
CONFIG_PATH and CONFIG_PATH_ARG to store the optional config flag and path as a
Bash array, preserving INPUT_CONFIG_PATH as one argument even when it contains
spaces or wildcard characters. Expand that array with quoted
"${CONFIG_PATH_ARGS[@]}" in the bandit command, while keeping the existing
behavior when no config path is provided.

@reactive-firewall reactive-firewall Aug 4, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

🙅🏻 While I appreciate the idea, your suggestion seems to introduce abstractions that may violate the Avoid Hasty Abstraction principle. Namely you assume it is desirable to use a bash array, and that bandit will handle bandit -f sarif -o results.serif -r . -ll -i "" (defaults with your suggestion) which will introduce calls where "" (e.g., quoted empty string) is passed after arguments that do not take meta-vars during the bandit invocation.

This causes a regression. The current logic ensures -c when passed a file path that the file path has been re-quoted already so AFAIK bash will see the ${...} pattern and treat the value as a single token (e.g. "example with spaces" -> example\ with\ spaces) albeit a little fragile. Does that make sense?

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@reactive-firewall, the concern about an empty argument is valid to check, but it does not occur with the proposed array form.

If CONFIG_PATH_ARGS=(), then:

"${CONFIG_PATH_ARGS[@]}"

expands to zero arguments. It does not pass "" to bandit. The command therefore retains the current no-config-path behavior.

The current ${CONFIG_PATH:-} expansion remains unsafe. Quotes in the value do not become shell quotes during a later expansion. Bash treats them as ordinary characters. It then performs word splitting and pathname expansion because the parameter expansion is unquoted.

For example, if the input is configs/security policy.yaml, this:

${CONFIG_PATH:-}

becomes two arguments. It does not become one escaped argument.

The array is not an additional abstraction for its own sake. It models the required command arguments: either no config arguments, or exactly -c plus one path argument. That preserves the argument boundary without passing an empty argument.

The original finding remains applicable.
🐇‍⬛

You are interacting with an AI system.

@reactive-firewall reactive-firewall Aug 4, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Ok, I see where you're going with this, but let's prove it with test code before making any changes.

Test Requirements:

  • demonstrate how passing arguments actual expands (both on the bash side and the bandit (python) side)
  • verify that the expected value would reach the bandit (python) side exactly (use monkey patching on bandit's CLI code if you have to)
  • hermetic test
  • proves your suggestion

@coderabbitai coderabbitai Bot Aug 4, 2026 •
edited by reactive-firewall
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' 'Repository files relevant to tests and tooling:'
fd -HI -t f \
  -e yml -e yaml -e py -e sh -e toml -e ini -e cfg \
  -g '!*node_modules*' -g '!*dist*' -g '!*build*' \
  | sort

printf '%s\n' 'Root-level metadata:'
fd -HI -t f -d 1 | sort

printf '%s\n' 'Test and Bandit references:'
rg -n -C 3 \
  -e 'pytest|unittest|bats|shellcheck|bandit|action\.yml|composite' \
  -g '!*node_modules*' -g '!*dist*' -g '!*build*' \
  . || true

Length of output: 990

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' 'Repository files relevant to tests and tooling:'
fd -HI -t f . \
  --exclude node_modules --exclude dist --exclude build \
  -e yml -e yaml -e py -e sh -e toml -e ini -e cfg \
  | sort

printf '%s\n' 'Root-level metadata:'
fd -HI -t f -d 1 . | sort

printf '%s\n' 'Test and Bandit references:'
rg -n -C 3 \
  -e 'pytest|unittest|bats|shellcheck|bandit|action\.yml|composite' \
  -g '!node_modules/**' -g '!dist/**' -g '!build/**' \
  . || true

Length of output: 4264

@reactive-firewall, the repository has no existing test framework or test workflow. I recommend one self-contained Bash regression test and one small workflow to run it.

The test will execute the actual Run Bandit scan script extracted from action.yml. It will not copy the config-path logic into the test.

The test will place a local bandit shim first in PATH:

  • The Bash shim records its received "$@" argument vector.
  • The shim starts a local Python recorder.
  • The Python recorder records sys.argv.
  • The test compares both records with the expected argument vector.

This proves the Bash boundary and the Python CLI boundary without installing Bandit or accessing the network.

  • Add a hermetic test that executes the current action command through local Bash and Python shims.
  • Cover a config path containing whitespace and a literal wildcard.
  • Cover the DEFAULT config-path case, which must pass no -c option and no empty argument.
  • Update action.yml only after the test shows the current split-path failure.
  • Add a minimal workflow that runs the test on pull requests.
  • 📌 Create a pull request with these changes

You are interacting with an AI system.

env:
INPUT_PATH: ${{ inputs.path }}
INPUT_LEVEL: ${{ inputs.level }}
Expand Down