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

.github/workflows/test-betterleaks.yml

@@ -37,12 +37,25 @@ jobs:
 
       - name: "git mode: assert leak detected and redacted"
         env:
+          RESULT: ${{ steps.git_leak.outputs.result }}
           LEAKS: ${{ steps.git_leak.outputs.leaks }}
           REPORT: ${{ steps.git_leak.outputs.report-path }}
+          COMMIT: ${{ steps.git_leak.outputs.commit }}
         run: |
+          test "$RESULT" = "leaks" || { echo "expected result=leaks, got '$RESULT'"; exit 1; }
           test "$LEAKS" -ge 1 || { echo "expected a leak, got '$LEAKS'"; exit 1; }
           jq -e 'any(.[]; .RuleID == "github-pat")' "$REPORT"
           ! grep -q 'ghp_' "$REPORT" || { echo "token not redacted"; exit 1; }
+          test "$COMMIT" = "$(git -C fixture rev-parse HEAD)" || { echo "commit output '$COMMIT' is not the fixture HEAD"; exit 1; }
+
+      - name: "git mode: assert report is outside the workspace"
+        env:
+          REPORT: ${{ steps.git_leak.outputs.report-path }}
+        run: |
+          [[ "$REPORT" == "$RUNNER_TEMP"/* ]] || { echo "report not under RUNNER_TEMP: $REPORT"; exit 1; }
+          if find "$GITHUB_WORKSPACE" -name betterleaks-report.json | grep -q .; then
+            echo "report written into the workspace"; exit 1
+          fi
 
       - name: Add clean commit on top
         working-directory: fixture
@@ -131,6 +144,82 @@ jobs:
           LEAKS: ${{ steps.nobase.outputs.leaks }}
         run: test "$LEAKS" = "0" || { echo "expected 0 leaks without base config, got $LEAKS"; exit 1; }
 
+      - name: Create fixture with $PWD bind mounts and a real pwd secret
+        run: |
+          mkdir -p pwdmount
+          cat > pwdmount/run.sh <<'SCRIPT'
+          docker run --rm -v "$PWD:/mnt:ro" -w /mnt image
+          SCRIPT
+          key="pwd"   # built at runtime so this workflow holds no secret-like literal
+          echo "${key}=$(head -c 400 /dev/urandom | tr -dc 'A-Za-z0-9' | head -c 16)" >> pwdmount/run.sh
+
+      - name: "bind mount: scan"
+        id: pwdmount
+        uses: ./betterleaks
+        with:
+          path: pwdmount
+          fail-on-leak: "false"
+          upload-report: "false"
+
+      - name: "bind mount: assert the mount is allowed and the secret is not"
+        env:
+          REPORT: ${{ steps.pwdmount.outputs.report-path }}
+        run: |
+          jq -e '[.[] | .StartLine] == [2]' "$REPORT" || { jq '.[] | {RuleID, StartLine}' "$REPORT"; exit 1; }
+
+      # --- invalid input and infrastructure errors ----------------------------
+
+      - name: "invalid input: mode"
+        id: bad_mode
+        continue-on-error: true
+        uses: ./betterleaks
+        with:
+          mode: foo
+          path: fixture
+          upload-report: "false"
+
+      - name: "invalid input: fail-on-leak"
+        id: bad_bool
+        continue-on-error: true
+        uses: ./betterleaks
+        with:
+          path: fixture
+          fail-on-leak: "yes"
+          upload-report: "false"
+
+      - name: "invalid input: path outside the workspace"
+        id: bad_path
+        continue-on-error: true
+        uses: ./betterleaks
+        with:
+          path: ../..
+          upload-report: "false"
+
+      - name: "invalid input: assert each one failed"
+        env:
+          MODE: ${{ steps.bad_mode.outcome }}
+          BOOL: ${{ steps.bad_bool.outcome }}
+          PATH_OUTCOME: ${{ steps.bad_path.outcome }}
+        run: |
+          for o in "mode=$MODE" "fail-on-leak=$BOOL" "path=$PATH_OUTCOME"; do
+            [[ "${o#*=}" == "failure" ]] || { echo "invalid ${o%%=*} was accepted (${o#*=})"; exit 1; }
+          done
+
+      - name: "infrastructure: image pull failure is an error, not a leak"
+        run: |
+          set +e
+          out="$(REPO_DIR="$GITHUB_WORKSPACE" OUT_DIR="$RUNNER_TEMP/pull-test" SCAN_PATH=fixture \
+            BETTERLEAKS_IMAGE=ghcr.io/betterleaks/betterleaks:v0.0.0-does-not-exist \
+            PULL_ATTEMPTS=2 PULL_DELAY=1 GITHUB_OUTPUT="$RUNNER_TEMP/pull-test.out" \
+            betterleaks/scripts/scan.sh 2>&1)"
+          rc=$?
+          set -e
+          echo "$out"
+          test "$rc" -ne 0 || { echo "expected a failure"; exit 1; }
+          grep -q 'infrastructure problem, not a leak' <<<"$out"
+          test "$(grep -c 'retrying' <<<"$out")" = "1"
+          ! grep -q '^result=' "$RUNNER_TEMP/pull-test.out" 2>/dev/null || { echo "a result was reported"; exit 1; }
+
   # Exercises the steps the scan tests skip: artifact upload, committer email
   # (sent to a throwaway Mailpit SMTP server) and the final failing step.
   notify:
@@ -194,6 +283,7 @@ jobs:
           echo "$text"
           grep -q 'github-pat at values.yaml:2' <<<"$text" || { echo "finding missing from email"; exit 1; }
           ! grep -q 'ghp_' <<<"$text" || { echo "token not redacted in email"; exit 1; }
+          grep -q "^Commit: $(git -C fixture rev-parse HEAD)" <<<"$text" || { echo "email does not name the scanned commit"; exit 1; }
 
       - name: Assert report artifact uploaded and redacted
         env:

CHANGELOG.md

@@ -25,8 +25,23 @@ Actions are released together under the major tag (`v1`).
 - 2026-11-03 — `betterleaks` README: example workflow skips the duplicate `pull_request` scan for
   branches of the same repository (`push` covers them); `pull_request` only runs for forks.
 
-### Fixed
+### Changed (review of the action's shell code)
+
+- 2026-11-03 — `betterleaks`: scan logic moved to `scripts/scan.sh`, shared by the action and
+  `tests/scan-local.sh` (no more duplicated config merge); shellcheck-clean, `set -euo pipefail`.
+- 2026-11-03 — `betterleaks`: the repository is mounted read-only into the scanner; the report is
+  written under `RUNNER_TEMP` instead of the checkout.
+- 2026-11-03 — `betterleaks`: the image pull is retried 3 times; pull failures and scanner errors fail
+  the step as errors and no longer email the committer as if a leak was found.
+- 2026-11-03 — `betterleaks`: the email names the scanned commit and the PR head branch (was the merge
+  commit / `N/merge` on `pull_request`). New outputs `result` and `commit`.
+- 2026-11-03 — `betterleaks`: inputs are validated (`mode`, boolean inputs, `path` and `config` must
+  stay inside the workspace); an invalid `fallback-email` is reported.
 
+### Fixed
+- 2026-11-03 — `betterleaks` base config: `docker run -v "$PWD:/path"` was reported by
+  `secret-literal-assignment` (`PWD` read as a password name). Allowlisted when the match is
+  `PWD:/…` only; `pwd=…`, `DB_PWD: …` and similar are still reported.
 - 2026-11-03 — `betterleaks`: the committer email was sent to `noreply@github.com` for commits made in
   the GitHub web UI. All no-reply addresses are now skipped and the chosen recipient is logged.
 

betterleaks/README.md

@@ -71,7 +71,12 @@ paths = ['''(^|/)develop/etc/.*\.conf$''']
 
 The action replaces the repo file's `[extend]` table with a reference to the base config, so the same
 file still works on its own for local runs (`betterleaks dir . -c .gitleaks.toml`: defaults + repo rules).
-To run locally with the EEA rules too, use [`tests/scan-local.sh`](tests/scan-local.sh).
+To run the same scan locally (same script, pinned image and config layering as CI):
+
+```sh
+betterleaks/tests/scan-local.sh /path/to/repo        # dir mode
+betterleaks/tests/scan-local.sh /path/to/repo git    # latest commit only
+```
 
 Single findings can still be silenced inline with a `betterleaks:allow` (or `gitleaks:allow`) comment.
 
@@ -90,9 +95,27 @@ Single findings can still be silenced inline with a `betterleaks:allow` (or `git
 | `smtp-url`, `smtp-port`, `smtp-email`, `smtp-password` | empty / `25` | Email notification; skipped when `smtp-url` or `smtp-email` is empty |
 | `fallback-email` | empty | Recipient when the committer has no deliverable address (see below) |
 
+Inputs are validated: `mode` must be `dir` or `git`, the `true`/`false` inputs must be exactly that,
+and `path` / `config` must stay inside the workspace.
+
 ## Outputs
 
-`leaks` (count), `exit-code`, `report-path`.
+| Output | Description |
+|---|---|
+| `result` | `clean` or `leaks` |
+| `leaks` | Number of findings |
+| `exit-code` | Betterleaks exit code |
+| `report-path` | JSON report, under `RUNNER_TEMP` (never written into the checkout) |
+| `commit` | Commit that was scanned; also used in the email |
+
+## Errors vs leaks
+
+Leaks set `result=leaks`, email the committer and fail the job in the last step. Problems that are not
+leaks fail the scan step itself, without an email: invalid inputs, the Betterleaks image not being
+pullable (retried 3 times first) and scanner errors. Re-run the job for registry hiccups.
+
+The scan logic lives in [`scripts/scan.sh`](scripts/scan.sh); the repository is mounted read-only into
+the scanner container.
 
 ## Who gets the email
 
@@ -118,4 +141,3 @@ trigger `push` here (they get no secrets, so no email, but the check still fails
   with `fetch-depth: 0` to scan the whole push.
 - `mode: dir` scans the files as checked out, so an existing leak keeps failing until it is removed
   or allowlisted.
-- A non-zero exit without findings is a scan error and also fails the job (and emails "No JSON report").