Skip to content

refactor(betterleaks): move scan to scripts/scan.sh and harden it - #7

Merged
valentinab25 merged 2 commits into
mainfrom
refactor/betterleaks-scan-script
Nov 3, 2026
Merged

valentinab25 merged 2 commits into
mainfrom
refactor/betterleaks-scan-script

Conversation

@valentinab25

Copy link
Copy Markdown
Contributor

Review of the action's shell code. actionlint runs shellcheck on workflow run: blocks but not on a composite action.yml, so the action's Bash had never been linted. Findings were checked against shellcheck 0.10 (all optional checks) and GitHub's shell.instructions.md guidelines.

# Change Why
1 Scan logic moved to betterleaks/scripts/scan.sh; action.yml and tests/scan-local.sh both call it config merge and docker args were duplicated and would drift; the script is now shellchecked and testable on its own
2 Repository mounted read-only into the scanner; report written to $RUNNER_TEMP/betterleaks/ the scanner only needs to read; no report file left in the checkout
3 Image pull retried (3 attempts, back-off); pull failures and scanner errors fail the scan step as errors, without emailing the committer a ghcr.io hiccup across ~800 repos used to look like a leak ("No JSON report" email)
4 Email uses the scanned commit (git rev-parse HEAD) and github.head_ref on pull_request it showed the merge commit and N/merge
5 set -euo pipefail in all scripts composite steps run bash -e -o pipefail without -u
6 Inputs validated: mode, true/false inputs, path/config must stay inside the workspace; invalid fallback-email reported typos such as fail-on-leak: yes used to disable failing silently
7 Committer step: no global shopt inside a function (${1,,} instead), no variable named source readability

New outputs: result (clean/leaks) and commit. Existing outputs and inputs unchanged.

Tests

  • shellcheck: clean (default, and -o all minus brace-style / Actions env-var / function-in-if notes)
  • actionlint: clean
  • Local run of scan.sh: leak/clean in git and dir mode, layered config, base off, 5 invalid-input cases, image pull failure with retry — all as expected; scan-local.sh on helm5 and this repo
  • Self-test additions: result/commit outputs, report outside the workspace, invalid mode / fail-on-leak / path fail the step, pull failure is an error with one retry and no result, email contains Commit: <scanned sha>

After merging

Move v1 (on main): git checkout main && git pull && git tag -f v1 && git push -f origin v1

Review of the action's shell code (shellcheck, shell guidelines):
- single scan script shared by the action and tests/scan-local.sh
- repository mounted read-only; report written under RUNNER_TEMP
- image pull retried; pull/scanner errors fail without a leak email
- email names the scanned commit and PR head branch
- set -euo pipefail, validated inputs (mode, booleans, paths inside workspace)
- readability fixes in the committer email step
docker run -v "$PWD:/path" matched secret-literal-assignment (PWD read as a password name). Allowlist matches of the form PWD:/... only; pwd=..., DB_PWD: ... are still reported. Self-test covers both.
@valentinab25

valentinab25 commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

Follow-up commit 52b5ec6: fix(betterleaks): allow $PWD bind mounts in the base config. docker run -v "$PWD:/path" was reported by secret-literal-assignment (found while scanning the new shell-scripting skill in eea.agent.skills). The allowlist only matches PWD:/…; pwd=…, DB_PWD: …, mysql_pwd: … are still reported. A new self-test step checks both.

@valentinab25
valentinab25 merged commit beb8f26 into main Nov 3, 2026
4 checks passed
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.

1 participant