fix(action): a fork's branch name must not reach the shell holding the API key (rf-7xv0, rf-v2mj)

.github/workflows/test-github-action.yml

@@ -14,10 +14,15 @@ on:
       - main
     paths:
       - 'github-action/**'
+      - 'action.yml'
       - '.github/workflows/test-github-action.yml'
   pull_request:
     paths:
       - 'github-action/**'
+      # The ROOT action.yml is a SECOND action file, and rf-v2mj lived in it.
+      # Without this line the probe that guards it would never fire on a change
+      # to it — a check watching a path its subject is not on.
+      - 'action.yml'
       - '.github/workflows/test-github-action.yml'
   workflow_dispatch:
 
@@ -404,6 +409,29 @@ jobs:
           [ "$FAIL" -eq 0 ] && echo "PASS: counts reached the gate and the gate failed the build."
           exit $FAIL
 
+  # sable-oubg / rf-7xv0 — a fork's branch name reached a shell holding
+  # RAFTER_API_KEY. Both probes FAIL on the pre-fix file and pass after, and
+  # both carry controls, so neither can pass by doing nothing.
+  test-trigger-injection:
+    name: "Trigger: a branch name cannot reach the key's shell"
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Run the injection probe
+        run: bash github-action/tests/test-trigger-injection.sh
+
+  # sable-oubg / rf-v2mj — the ROOT action.yml derived its finding count with
+  # `|| echo "0"`, so an unreadable report rendered as a clean scan. Note this
+  # is the OTHER action.yml: the same defect class in github-action/action.yml
+  # was fixed under sable-fgk7 and this file was never touched by it.
+  test-root-action-counts:
+    name: "Root action: an unreadable report is not a clean scan"
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Run the count-derivation probe
+        run: bash github-action/tests/test-root-action-counts.sh
+
   test-yaml-validity:
     name: action.yml is valid YAML
     runs-on: ubuntu-latest

action.yml

@@ -98,10 +98,30 @@ runs:
         # ({_note, scan_mode, triage_applied, results: [...]}); older versions
         # emitted a bare array. Handle both so users pinning `version:` to an
         # older release don't break.
+        # rf-v2mj — `|| echo "0"` here was the whole bug: jq fails on a truncated
+        # report, an HTML error page or an auth failure, and the fallback turned
+        # every one of those into finding-count=0. Zero findings and "I could not
+        # read the output" became the same value, so the gate passed PRECISELY
+        # when it could not see its input. An unreadable report is not a clean
+        # scan; the count is emitted only when a parse actually succeeded.
         if [ "${{ inputs.format }}" = "json" ]; then
-          COUNT=$(echo "${OUTPUT}" | jq '[(if type == "array" then . else .results end) | .[]?.matches[]?] | length' 2>/dev/null || echo "0")
+          if ! COUNT=$(printf '%s' "${OUTPUT}" | jq -e '[(if type == "array" then . else .results end) | .[]?.matches[]?] | length' 2>/dev/null); then
+            echo "::error::Rafter's output could not be parsed as JSON, so the finding count is unknown."
+            echo "::error::A report this action cannot read is not a clean scan. Re-run, or check the scanner's stderr above."
+            # Deliberately NOT written as 0: a consumer reading finding-count
+            # gets an empty string, never a fabricated zero.
+            echo "finding-count=" >> "$GITHUB_OUTPUT"
+            exit 1
+          fi
         else
-          COUNT=$(echo "${OUTPUT}" | grep -c 'Secret:' 2>/dev/null || echo "0")
+          # grep exits 1 for "no matches", which is a CLEAN result and must not
+          # be confused with grep failing (exit >1).
+          COUNT=$(printf '%s' "${OUTPUT}" | grep -c 'Secret:'); GREP_STATUS=$?
+          if [ "$GREP_STATUS" -gt 1 ]; then
+            echo "::error::Could not scan Rafter's output for findings (grep exited ${GREP_STATUS})."
+            echo "finding-count=" >> "$GITHUB_OUTPUT"
+            exit 1
+          fi
         fi
         echo "finding-count=${COUNT}" >> "$GITHUB_OUTPUT"
 

github-action/action.yml

@@ -68,20 +68,35 @@ runs:
         RAFTER_API_KEY: ${{ inputs.api-key }}
         RAFTER_URL: ${{ inputs.rafter-url }}
         SCAN_MODE: ${{ inputs.scan-mode }}
+        # rf-7xv0 — these two reach the script through the ENVIRONMENT, never
+        # through `${{ }}` inside `run:`. The runner expands `${{ }}` into the
+        # script TEXT before bash sees it, so an interpolated value is not data,
+        # it is source code — and `github.head_ref` is the branch name chosen by
+        # whoever opened the pull request. Any stranger can open one from a fork.
+        # This step's env carries RAFTER_API_KEY, so the branch name was running
+        # in a shell that could read the key.
+        GH_REPOSITORY: ${{ github.repository }}
+        GH_BRANCH: ${{ github.head_ref || github.ref_name }}
       run: |
         # --fail-with-body: non-2xx → body printed to stdout AND exit nonzero.
         # We capture body+status separately so future failures self-explain
         # (instead of just "curl exit 22"). API key never echoed.
         BODY_FILE="$(mktemp)"
+        # Built by jq from environment variables, not by pasting values into a
+        # quoted string. That fixes two things at once: the shell never sees the
+        # branch name as code, and a branch name containing a quote or backslash
+        # produces VALID JSON instead of a malformed body. Hand-quoting would
+        # have to get both right; jq --arg gets both right by construction.
+        PAYLOAD=$(jq -nc \
+          --arg repository_name "$GH_REPOSITORY" \
+          --arg branch_name "$GH_BRANCH" \
+          --arg scan_mode "$SCAN_MODE" \
+          '{repository_name: $repository_name, branch_name: $branch_name, scan_mode: $scan_mode}')
         HTTP_CODE=$(curl -sS --connect-timeout 10 --max-time 60 \
           -o "$BODY_FILE" -w "%{http_code}" -X POST \
           -H "Content-Type: application/json" \
           -H "x-api-key: ${RAFTER_API_KEY}" \
-          -d "{
-            \"repository_name\": \"${{ github.repository }}\",
-            \"branch_name\": \"${{ github.head_ref || github.ref_name }}\",
-            \"scan_mode\": \"${SCAN_MODE}\"
-          }" \
+          -d "$PAYLOAD" \
           "${RAFTER_URL}/api/static/scan") || {
             echo "::error::curl transport error contacting ${RAFTER_URL}/api/static/scan"
             cat "$BODY_FILE" || true

github-action/tests/test-root-action-counts.sh

@@ -0,0 +1,108 @@
+#!/usr/bin/env bash
+#
+# rf-v2mj — an unreadable report must not render as a clean scan.
+#
+# The root action derives `finding-count` from the scanner's stdout. It used to
+# end that derivation with `|| echo "0"`, so a truncated report, an HTML error
+# page or an auth failure all produced finding-count=0: the value that means
+# CLEAN. A consumer gating on it was told "no findings" precisely when the
+# action could not see any output at all.
+#
+# Corpus: the ROOT action.yml. The separate github-action/action.yml had the
+# same class of defect in its five severity counts and was fixed under
+# sable-fgk7; this file was never touched by that work. Two action.yml files,
+# and only one of them had been fixed.
+#
+# The control is the point: a fix that fails on everything would pass the first
+# case and be useless. A genuinely clean report must still succeed with count 0.
+set -uo pipefail
+
+ROOT="$(cd "$(dirname "$0")/../.." && pwd)/action.yml"
+TMP="$(mktemp -d)"; trap 'rm -rf "$TMP"' EXIT
+failures=0
+
+# The scan step, with the runner's `${{ inputs.* }}` expansion applied.
+render() {
+  python3 - "$ROOT" "$1" <<'PY'
+import sys, yaml, re
+d = yaml.safe_load(open(sys.argv[1])); fmt = sys.argv[2]
+for s in d["runs"]["steps"]:
+    if s.get("id") == "scan":
+        r = s["run"]
+        r = re.sub(r"\$\{\{\s*inputs\.format\s*\}\}", fmt, r)
+        r = re.sub(r"\$\{\{\s*inputs\.(scan-path|args)\s*\}\}", "", r)
+        r = re.sub(r"\$\{\{.*?\}\}", "", r)
+        print(r); break
+else:
+    sys.exit("no step id 'scan'")
+PY
+}
+
+# A stub `rafter` that emits exactly what we want to test, with a chosen exit code.
+stub() {
+  mkdir -p "$TMP/bin"
+  { echo '#!/usr/bin/env bash'; echo "cat <<'RAFTER_STUB_EOF'"; printf '%s\n' "$1"; echo 'RAFTER_STUB_EOF'; echo "exit ${2}"; } > "$TMP/bin/rafter"
+  chmod +x "$TMP/bin/rafter"
+}
+
+run_scan() {
+  render "$1" > "$TMP/scan.sh"
+  : > "$TMP/gh_output"
+  ( cd "$TMP" && PATH="$TMP/bin:$PATH" GITHUB_OUTPUT="$TMP/gh_output" \
+      timeout 30 bash "$TMP/scan.sh" >"$TMP/out" 2>&1 )
+  echo $?
+}
+count_written() { sed -n 's/^finding-count=\(.*\)$/\1/p' "$TMP/gh_output" | tail -1; }
+
+echo "== rf-v2mj: an unreadable report is not a clean scan =="
+
+# 1. THE BUG. Unparseable stdout with a success exit code — a truncated report,
+#    an error page, anything jq cannot read.
+stub '<html><body>502 Bad Gateway</body></html>' 0
+rc=$(run_scan json); c=$(count_written)
+if [ "$rc" -eq 0 ] && [ "$c" = "0" ]; then
+  echo "FAIL: an unparseable report produced finding-count=0 and exit 0 — a clean scan"
+  failures=$((failures+1))
+elif [ "$rc" -eq 0 ]; then
+  echo "FAIL: an unparseable report exited 0 (count written: '${c}')"
+  failures=$((failures+1))
+else
+  echo "PASS: an unparseable report fails the step (exit ${rc}, count '${c}')"
+fi
+
+# 2. Truncated JSON — the likeliest real shape, and still not a clean scan.
+stub '{"results": [{"matches": [{"rule":' 0
+rc=$(run_scan json); c=$(count_written)
+if [ "$rc" -eq 0 ]; then
+  echo "FAIL: truncated JSON exited 0 (count written: '${c}')"
+  failures=$((failures+1))
+else
+  echo "PASS: truncated JSON fails the step (exit ${rc})"
+fi
+
+# 3. THE CONTROL. A genuinely clean report must still succeed, with count 0.
+#    Without this, "fail on everything" would pass the two cases above.
+stub '{"_note":"x","scan_mode":"fast","triage_applied":false,"results":[]}' 0
+rc=$(run_scan json); c=$(count_written)
+if [ "$rc" -eq 0 ] && [ "$c" = "0" ]; then
+  echo "PASS: a genuinely clean report still passes, count 0"
+else
+  echo "FAIL: CONTROL — a clean report no longer passes (exit ${rc}, count '${c}')"
+  failures=$((failures+1))
+fi
+
+# 4. Second control: real findings must still be counted, not just tolerated.
+stub '{"results":[{"matches":[{"rule":"aws"},{"rule":"gh"}]}]}' 1
+rc=$(run_scan json); c=$(count_written)
+if [ "$c" = "2" ]; then
+  echo "PASS: real findings are counted (2)"
+else
+  echo "FAIL: CONTROL — findings miscounted: got '${c}', expected 2"
+  failures=$((failures+1))
+fi
+
+echo ""
+echo "── results ──────────────────────────────────────────────"
+echo "Failures: $failures"
+[ "$failures" -eq 0 ] || exit 1
+echo "OK: the count is emitted only when the report was actually read"