diff --git a/.github/workflows/test-github-action.yml b/.github/workflows/test-github-action.yml
index 3887552..7a7665d 100644
--- a/.github/workflows/test-github-action.yml
+++ b/.github/workflows/test-github-action.yml
@@ -227,6 +227,183 @@ jobs:
fi
echo "PASS: the results fetch retried and completed."
+ # sable-fgk7 — the results step used to coerce EVERY failure to read the
+ # report into findings_count=0, which passes every severity threshold and
+ # renders ":white_check_mark: No security findings detected". A report the
+ # action cannot read is not a clean scan. Three shapes, each of which used
+ # to land as a clean green: schema-valid-but-wrong (parses, no key), not
+ # JSON at all, and a 200 whose body is an error object.
+ test-results-unreadable-is-not-clean:
+ name: "Results: an unreadable report is not a clean scan (${{ matrix.shape }})"
+ runs-on: ubuntu-latest
+ strategy:
+ fail-fast: false
+ matrix:
+ shape: [missing-key, not-json, error-object]
+ steps:
+ - uses: actions/checkout@v4
+
+ - name: Start mock backend (poll completes; results body is ${{ matrix.shape }})
+ env:
+ PORT: '8791'
+ FAIL_COUNT: '0'
+ COMPLETE_AFTER: '1'
+ RESULTS_SHAPE: ${{ matrix.shape }}
+ run: |
+ nohup python3 github-action/tests/mock-rafter-api.py > mock.log 2>&1 &
+ for _ in $(seq 1 30); do
+ curl -sf -X POST -d '{}' http://127.0.0.1:8791/api/static/scan >/dev/null && break
+ sleep 1
+ done
+ curl -sf -X POST -d '{}' http://127.0.0.1:8791/api/static/scan >/dev/null || {
+ echo "FAIL: mock backend never started"; cat mock.log; exit 1; }
+
+ - name: Run the action against the mock
+ id: scan
+ continue-on-error: true
+ uses: ./github-action
+ with:
+ api-key: 'not-a-real-key'
+ rafter-url: 'http://127.0.0.1:8791'
+ timeout-minutes: '2'
+ upload-sarif: 'false'
+ comment-on-pr: 'false'
+
+ - name: Assert the build failed and no count was fabricated
+ run: |
+ cat mock.log
+ FAIL=0
+ if [ "${{ steps.scan.outcome }}" != "failure" ]; then
+ echo "FAIL: an unreadable report must fail the build (outcome='${{ steps.scan.outcome }}')."
+ FAIL=1
+ fi
+ if [ "${{ steps.scan.outputs.status }}" != "unreadable" ]; then
+ echo "FAIL: expected status=unreadable, got '${{ steps.scan.outputs.status }}'."
+ FAIL=1
+ fi
+ # The floor: a count that was never computed must be ABSENT, not 0.
+ # '0' here is the bug — it is what a consumer gating on the output
+ # reads as a clean scan.
+ if [ -n "${{ steps.scan.outputs.findings-count }}" ]; then
+ echo "FAIL: findings-count was fabricated as '${{ steps.scan.outputs.findings-count }}' from an unreadable report."
+ FAIL=1
+ fi
+ [ "$FAIL" -eq 0 ] && echo "PASS: unreadable report (${{ matrix.shape }}) failed the build with status=unreadable and no counts."
+ exit $FAIL
+
+ # The other half of the floor: when the report IS readable the counts must be
+ # exactly the report's and the build must pass. Without this, a "validation"
+ # that rejected everything would also land green above.
+ test-results-counts-exact:
+ name: "Results: counts are exactly the report's"
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v4
+
+ - name: Start mock backend (poll completes; report has 3 findings)
+ env:
+ PORT: '8792'
+ FAIL_COUNT: '0'
+ COMPLETE_AFTER: '1'
+ RESULTS_SHAPE: 'with-findings'
+ run: |
+ nohup python3 github-action/tests/mock-rafter-api.py > mock.log 2>&1 &
+ for _ in $(seq 1 30); do
+ curl -sf -X POST -d '{}' http://127.0.0.1:8792/api/static/scan >/dev/null && break
+ sleep 1
+ done
+ curl -sf -X POST -d '{}' http://127.0.0.1:8792/api/static/scan >/dev/null || {
+ echo "FAIL: mock backend never started"; cat mock.log; exit 1; }
+
+ - name: Run the action against the mock
+ id: scan
+ continue-on-error: true
+ uses: ./github-action
+ with:
+ api-key: 'not-a-real-key'
+ rafter-url: 'http://127.0.0.1:8792'
+ timeout-minutes: '2'
+ upload-sarif: 'false'
+ comment-on-pr: 'false'
+
+ - name: Assert every count is the report's, not a default
+ run: |
+ cat mock.log
+ FAIL=0
+ check() {
+ if [ "$2" != "$3" ]; then
+ echo "FAIL: $1 expected '$3', got '$2'"
+ FAIL=1
+ fi
+ }
+ check outcome "${{ steps.scan.outcome }}" "success"
+ check status "${{ steps.scan.outputs.status }}" "completed"
+ check findings-count "${{ steps.scan.outputs.findings-count }}" "3"
+ check critical-count "${{ steps.scan.outputs.critical-count }}" "1"
+ check high-count "${{ steps.scan.outputs.high-count }}" "1"
+ check medium-count "${{ steps.scan.outputs.medium-count }}" "0"
+ check low-count "${{ steps.scan.outputs.low-count }}" "1"
+ [ "$FAIL" -eq 0 ] && echo "PASS: counts are exactly the report's (3/1/1/0/1)."
+ exit $FAIL
+
+ # sable-1drb — the threshold gate is now unit-tested against the real
+ # lib/severity.sh, but a unit test cannot prove action.yml WIRES it: that
+ # the counts reach the gate and the gate's verdict reaches the job. One
+ # end-to-end run with real findings and a threshold they exceed does.
+ test-threshold-gate-end-to-end:
+ name: "Threshold gate: real findings above the threshold fail the build"
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v4
+
+ - name: Start mock backend (report has 1 critical, 1 high, 1 low)
+ env:
+ PORT: '8793'
+ FAIL_COUNT: '0'
+ COMPLETE_AFTER: '1'
+ RESULTS_SHAPE: 'with-findings'
+ run: |
+ nohup python3 github-action/tests/mock-rafter-api.py > mock.log 2>&1 &
+ for _ in $(seq 1 30); do
+ curl -sf -X POST -d '{}' http://127.0.0.1:8793/api/static/scan >/dev/null && break
+ sleep 1
+ done
+ curl -sf -X POST -d '{}' http://127.0.0.1:8793/api/static/scan >/dev/null || {
+ echo "FAIL: mock backend never started"; cat mock.log; exit 1; }
+
+ - name: Run the action with severity-threshold high
+ id: scan
+ continue-on-error: true
+ uses: ./github-action
+ with:
+ api-key: 'not-a-real-key'
+ rafter-url: 'http://127.0.0.1:8793'
+ timeout-minutes: '2'
+ upload-sarif: 'false'
+ comment-on-pr: 'false'
+ severity-threshold: 'high'
+
+ - name: Assert the gate, not an error, failed the build
+ run: |
+ cat mock.log
+ FAIL=0
+ # status=completed AND outcome=failure is the gate's signature: the
+ # report was read and counted, then the threshold rejected it.
+ if [ "${{ steps.scan.outputs.status }}" != "completed" ]; then
+ echo "FAIL: expected status=completed (report read), got '${{ steps.scan.outputs.status }}'"
+ FAIL=1
+ fi
+ if [ "${{ steps.scan.outcome }}" != "failure" ]; then
+ echo "FAIL: 1 critical + 1 high with severity-threshold=high must fail the build (outcome='${{ steps.scan.outcome }}')"
+ FAIL=1
+ fi
+ if [ "${{ steps.scan.outputs.findings-count }}" != "3" ]; then
+ echo "FAIL: findings-count expected 3, got '${{ steps.scan.outputs.findings-count }}'"
+ FAIL=1
+ fi
+ [ "$FAIL" -eq 0 ] && echo "PASS: counts reached the gate and the gate failed the build."
+ exit $FAIL
+
test-yaml-validity:
name: action.yml is valid YAML
runs-on: ubuntu-latest
diff --git a/github-action/README.md b/github-action/README.md
index e9d94c1..eec9bf0 100644
--- a/github-action/README.md
+++ b/github-action/README.md
@@ -53,12 +53,12 @@ jobs:
| Output | Description |
|--------|-------------|
| `scan-id` | The Rafter scan ID |
-| `findings-count` | Total findings |
+| `findings-count` | Total findings. Empty, never `0`, when the report could not be read (see `status`) |
| `critical-count` | Critical severity findings |
| `high-count` | High severity findings |
| `medium-count` | Medium severity findings |
| `low-count` | Low severity findings |
-| `status` | Scan status |
+| `status` | `completed`, `failed`, `timeout`, `unreadable` (the scan may have finished but its report could not be read or parsed), or `unreachable` (the API could not be contacted). Count outputs are only written when `completed` |
## Examples
diff --git a/github-action/action.yml b/github-action/action.yml
index e096efa..8fe5ee3 100644
--- a/github-action/action.yml
+++ b/github-action/action.yml
@@ -303,13 +303,28 @@ runs:
fetch_results "${{ runner.temp }}/rafter-results.md" "${RAFTER_URL}/api/static/scan?scan_id=${SCAN_ID}&format=md"
fetch_results "${{ runner.temp }}/rafter-results.sarif" "${RAFTER_URL}/api/static/scan?scan_id=${SCAN_ID}&format=sarif"
- # Extract counts
+ # A report this step cannot read is NOT a clean scan (sable-fgk7).
+ # Every count below used to fall back to 0 on any jq failure, so a
+ # malformed body, a truncated write, or a 200 carrying an error object
+ # rendered as ":white_check_mark: No security findings detected" and
+ # passed every severity threshold. Validate the shape first. Once it
+ # holds, the count expressions cannot fail and need no fallback; if
+ # jq itself is broken the step fails, which is the correct outcome.
RESULTS="${{ runner.temp }}/rafter-results.json"
- FINDINGS_COUNT=$(jq '.vulnerabilities | length' "$RESULTS" 2>/dev/null || echo "0")
- CRITICAL_COUNT=$(jq '[.vulnerabilities[] | select(.severity == "critical")] | length' "$RESULTS" 2>/dev/null || echo "0")
- HIGH_COUNT=$(jq '[.vulnerabilities[] | select(.severity == "error" or .severity == "high")] | length' "$RESULTS" 2>/dev/null || echo "0")
- MEDIUM_COUNT=$(jq '[.vulnerabilities[] | select(.severity == "warning" or .severity == "medium")] | length' "$RESULTS" 2>/dev/null || echo "0")
- LOW_COUNT=$(jq '[.vulnerabilities[] | select(.severity == "note" or .severity == "low")] | length' "$RESULTS" 2>/dev/null || echo "0")
+ if ! jq -e 'type == "object" and (.vulnerabilities | type == "array") and all(.vulnerabilities[]; type == "object")' "$RESULTS" >/dev/null 2>&1; then
+ SNIPPET=$(head -c 300 "$RESULTS" 2>/dev/null | tr -d '\r\n' | cut -c1-200 || true)
+ echo "::error::Rafter returned a report for scan ${SCAN_ID} that this action cannot read: no 'vulnerabilities' array."
+ echo "::error::A report that cannot be parsed is not a clean scan, so no counts were produced. Check the scan in your dashboard at ${RAFTER_URL}/dashboard or retry with: rafter get ${SCAN_ID}"
+ echo "::error::Body started with: ${SNIPPET}"
+ echo "status=unreadable" >> "$GITHUB_OUTPUT"
+ exit 1
+ fi
+
+ FINDINGS_COUNT=$(jq '.vulnerabilities | length' "$RESULTS")
+ CRITICAL_COUNT=$(jq '[.vulnerabilities[] | select(.severity == "critical")] | length' "$RESULTS")
+ HIGH_COUNT=$(jq '[.vulnerabilities[] | select(.severity == "error" or .severity == "high")] | length' "$RESULTS")
+ MEDIUM_COUNT=$(jq '[.vulnerabilities[] | select(.severity == "warning" or .severity == "medium")] | length' "$RESULTS")
+ LOW_COUNT=$(jq '[.vulnerabilities[] | select(.severity == "note" or .severity == "low")] | length' "$RESULTS")
echo "findings_count=${FINDINGS_COUNT}" >> "$GITHUB_OUTPUT"
echo "critical_count=${CRITICAL_COUNT}" >> "$GITHUB_OUTPUT"
@@ -341,6 +356,9 @@ runs:
LOW_COUNT: ${{ steps.results.outputs.low_count }}
SEVERITY_THRESHOLD: ${{ inputs.severity-threshold }}
run: |
+ # Shared with the tests under tests/ — see lib/severity.sh (sable-1drb).
+ source "${{ github.action_path }}/lib/severity.sh"
+
MD_REPORT=$(jq -r '.markdown // empty' "${{ runner.temp }}/rafter-results.md" 2>/dev/null || cat "${{ runner.temp }}/rafter-results.md")
if [ "$FINDINGS_COUNT" -eq 0 ]; then
@@ -379,10 +397,7 @@ runs:
echo ""
echo ""
echo ""
- if [ "$FINDINGS_COUNT" -gt 0 ] && [ "$SEVERITY_THRESHOLD" = "none" ]; then
- echo "> :information_source: This run is report-only. To fail the build on critical/high findings, set \`severity-threshold: high\` in your workflow."
- echo ""
- fi
+ rafter_report_only_tip "$FINDINGS_COUNT" "$SEVERITY_THRESHOLD"
echo "---"
echo "Scan ID: ${SCAN_ID} | Powered by [Rafter](https://rafter.so)"
} >> "$COMMENT_FILE"
@@ -417,31 +432,11 @@ runs:
MEDIUM_COUNT: ${{ steps.results.outputs.medium_count }}
LOW_COUNT: ${{ steps.results.outputs.low_count }}
run: |
- FAIL=0
-
- case "$SEVERITY_THRESHOLD" in
- critical)
- [ "$CRITICAL_COUNT" -gt 0 ] && FAIL=1
- ;;
- high)
- [ "$CRITICAL_COUNT" -gt 0 ] || [ "$HIGH_COUNT" -gt 0 ] && FAIL=1
- ;;
- medium)
- [ "$CRITICAL_COUNT" -gt 0 ] || [ "$HIGH_COUNT" -gt 0 ] || [ "$MEDIUM_COUNT" -gt 0 ] && FAIL=1
- ;;
- low)
- [ "$CRITICAL_COUNT" -gt 0 ] || [ "$HIGH_COUNT" -gt 0 ] || [ "$MEDIUM_COUNT" -gt 0 ] || [ "$LOW_COUNT" -gt 0 ] && FAIL=1
- ;;
- none)
- FAIL=0
- ;;
- *)
- echo "::warning::Unknown severity threshold '${SEVERITY_THRESHOLD}', defaulting to 'high'"
- [ "$CRITICAL_COUNT" -gt 0 ] || [ "$HIGH_COUNT" -gt 0 ] && FAIL=1
- ;;
- esac
+ # The case statement lives in lib/severity.sh so the unit tests under
+ # tests/ run the same code, not a transcription of it (sable-1drb).
+ source "${{ github.action_path }}/lib/severity.sh"
- if [ "$FAIL" -eq 1 ]; then
+ if rafter_threshold_fails "$SEVERITY_THRESHOLD" "$CRITICAL_COUNT" "$HIGH_COUNT" "$MEDIUM_COUNT" "$LOW_COUNT"; then
echo "::error::Security findings exceed severity threshold '${SEVERITY_THRESHOLD}'"
exit 1
fi
diff --git a/github-action/lib/severity.sh b/github-action/lib/severity.sh
new file mode 100644
index 0000000..ab38d2f
--- /dev/null
+++ b/github-action/lib/severity.sh
@@ -0,0 +1,54 @@
+#!/usr/bin/env bash
+#
+# Severity-threshold logic shared by github-action/action.yml and the tests
+# under github-action/tests/. ONE copy, sourced by both, so the tests exercise
+# the code the action runs rather than a transcription of it (sable-1drb).
+#
+# Sourced, never executed: no `set -e`, no side effects at load time. Every
+# function takes explicit arguments so a test can call it without staging
+# environment variables, and prints only what the action wants in its log.
+
+# rafter_threshold_fails THRESHOLD CRITICAL HIGH MEDIUM LOW
+#
+# Returns 0 when the findings exceed THRESHOLD (the build should fail) and 1
+# otherwise. 'none' never fails. An unrecognised threshold behaves like
+# 'high' and says so with a ::warning:: annotation.
+rafter_threshold_fails() {
+ local threshold="$1" critical="$2" high="$3" medium="$4" low="$5"
+ local fail=0
+ case "$threshold" in
+ critical)
+ [ "$critical" -gt 0 ] && fail=1
+ ;;
+ high)
+ [ "$critical" -gt 0 ] || [ "$high" -gt 0 ] && fail=1
+ ;;
+ medium)
+ [ "$critical" -gt 0 ] || [ "$high" -gt 0 ] || [ "$medium" -gt 0 ] && fail=1
+ ;;
+ low)
+ [ "$critical" -gt 0 ] || [ "$high" -gt 0 ] || [ "$medium" -gt 0 ] || [ "$low" -gt 0 ] && fail=1
+ ;;
+ none)
+ fail=0
+ ;;
+ *)
+ echo "::warning::Unknown severity threshold '${threshold}', defaulting to 'high'"
+ [ "$critical" -gt 0 ] || [ "$high" -gt 0 ] && fail=1
+ ;;
+ esac
+ [ "$fail" -eq 1 ]
+}
+
+# rafter_report_only_tip FINDINGS_COUNT THRESHOLD
+#
+# Prints the report-only tip block for the PR comment iff there are findings
+# AND the threshold is 'none' (the default), i.e. the run reported problems
+# but was configured never to fail on them. Prints nothing otherwise.
+rafter_report_only_tip() {
+ local findings="$1" threshold="$2"
+ if [ "$findings" -gt 0 ] && [ "$threshold" = "none" ]; then
+ echo "> :information_source: This run is report-only. To fail the build on critical/high findings, set \`severity-threshold: high\` in your workflow."
+ echo ""
+ fi
+}
diff --git a/github-action/tests/mock-rafter-api.py b/github-action/tests/mock-rafter-api.py
index 2b77ab9..23af6cc 100644
--- a/github-action/tests/mock-rafter-api.py
+++ b/github-action/tests/mock-rafter-api.py
@@ -26,6 +26,19 @@
i.e. as soon as the injected failures are done). Set it higher
than the failure window to make the RESULTS fetch fail rather
than the poll.
+ RESULTS_SHAPE what the completed JSON body looks like (sable-fgk7). Default
+ "ok": {"status":"completed","vulnerabilities":[]}. Others:
+ with-findings three findings: one critical, one high, one low
+ missing-key {"scan_id":..,"status":"completed"} — parses,
+ has no vulnerabilities array at all
+ not-json a 200 whose body is an HTML error page
+ error-object a 200 whose body is {"error": ...}
+ Each of the last three used to make the action report
+ "No security findings detected" and pass every threshold.
+ SHAPE_FROM GET index from which the JSON body takes RESULTS_SHAPE
+ (default COMPLETE_AFTER + 1, so the poll loop sees one healthy
+ "completed" and the RESULTS fetch gets the shaped body).
+ md/sarif fetches are never shaped.
"""
import json
import os
@@ -38,21 +51,47 @@
FAIL_FOREVER = os.environ.get("FAIL_FOREVER") == "1"
FAIL_COUNT = int(os.environ.get("FAIL_COUNT", "1"))
COMPLETE_AFTER = int(os.environ.get("COMPLETE_AFTER", str(FAIL_ON)))
+RESULTS_SHAPE = os.environ.get("RESULTS_SHAPE", "ok")
+SHAPE_FROM = int(os.environ.get("SHAPE_FROM", str(COMPLETE_AFTER + 1)))
SCAN_ID = "repro-sable-l10k-0001"
+# Severities chosen so every count output is pinned to a distinct value:
+# findings=3, critical=1, high=1, medium=0, low=1.
+WITH_FINDINGS = [
+ {"rule_id": "sql-injection", "severity": "critical", "file_path": "db.php", "line_start": 12},
+ {"rule_id": "xss-echo", "severity": "high", "file_path": "view.php", "line_start": 40},
+ {"rule_id": "weak-hash", "severity": "low", "file_path": "auth.php", "line_start": 7},
+]
+
state = {"polls": 0}
class Handler(BaseHTTPRequestHandler):
def _send(self, code, payload):
- body = json.dumps(payload).encode()
+ self._send_raw(code, json.dumps(payload).encode(), "application/json")
+
+ def _send_raw(self, code, body, content_type):
self.send_response(code)
- self.send_header("Content-Type", "application/json")
+ self.send_header("Content-Type", content_type)
self.send_header("Content-Length", str(len(body)))
self.end_headers()
self.wfile.write(body)
+ def _send_shaped_results(self):
+ """The completed JSON body under RESULTS_SHAPE (never for md/sarif)."""
+ if RESULTS_SHAPE == "with-findings":
+ return self._send(200, {"scan_id": SCAN_ID, "status": "completed",
+ "vulnerabilities": WITH_FINDINGS})
+ if RESULTS_SHAPE == "missing-key":
+ return self._send(200, {"scan_id": SCAN_ID, "status": "completed"})
+ if RESULTS_SHAPE == "not-json":
+ return self._send_raw(200, b"
502 Bad Gateway
",
+ "text/html")
+ if RESULTS_SHAPE == "error-object":
+ return self._send(200, {"error": "Failed to fetch report from storage: Object not found"})
+ raise SystemExit(f"unknown RESULTS_SHAPE {RESULTS_SHAPE!r}")
+
def do_POST(self):
if urlparse(self.path).path != "/api/static/scan":
return self._send(404, {"error": "not found"})
@@ -84,6 +123,9 @@ def do_GET(self):
if n < COMPLETE_AFTER:
return self._send(200, {"scan_id": SCAN_ID, "status": "processing"})
+ if fmt == "json" and RESULTS_SHAPE != "ok" and n >= SHAPE_FROM:
+ return self._send_shaped_results()
+
completed = {"scan_id": SCAN_ID, "status": "completed", "vulnerabilities": []}
if fmt == "md":
completed["markdown"] = "# Rafter\n\nNo findings.\n"
diff --git a/github-action/tests/test-action-yml-defaults.sh b/github-action/tests/test-action-yml-defaults.sh
index 59444a3..d68b446 100755
--- a/github-action/tests/test-action-yml-defaults.sh
+++ b/github-action/tests/test-action-yml-defaults.sh
@@ -45,25 +45,49 @@ else
failures=$((failures+1))
fi
-# 3. The report-only tip block must be present and gated on both conditions.
-if grep -qE '\[ "\$FINDINGS_COUNT" -gt 0 \] && \[ "\$SEVERITY_THRESHOLD" = "none" \]' "$ACTION_YML"; then
- echo "PASS: report-only tip block gated on (findings > 0) AND (threshold == 'none')"
+# The threshold case statement and the report-only tip live in lib/severity.sh
+# (sable-1drb), sourced by action.yml AND by the unit tests, so checks 3, 4
+# and 17 look there. Check 17 is what stops a "simplification" from inlining
+# a copy back into action.yml, which would silently detach the tests again.
+SEVERITY_LIB="$(cd "$(dirname "$0")/.." && pwd)/lib/severity.sh"
+if [ ! -f "$SEVERITY_LIB" ]; then
+ echo "FAIL: $SEVERITY_LIB not found"
+ exit 1
+fi
+
+# 3. The report-only tip must be gated on both conditions.
+if grep -qE '\[ "\$findings" -gt 0 \] && \[ "\$threshold" = "none" \]' "$SEVERITY_LIB"; then
+ echo "PASS: report-only tip gated on (findings > 0) AND (threshold == 'none')"
else
- echo "FAIL: report-only tip block missing or mis-gated in $ACTION_YML"
+ echo "FAIL: report-only tip missing or mis-gated in $SEVERITY_LIB"
failures=$((failures+1))
fi
-# 4. The threshold-eval step must still handle 'none' as a no-op
-# (no FAIL=1 in the none branch).
+# 4. The threshold-eval must still handle 'none' as a no-op
+# (no fail=1 in the none branch).
if awk '
/none\)/ { in_none=1; next }
in_none && /;;/ { in_none=0; next }
in_none { print }
-' "$ACTION_YML" | grep -qE "FAIL *= *1"; then
- echo "FAIL: 'none' branch of threshold-eval sets FAIL=1 — that would break the default"
+' "$SEVERITY_LIB" | grep -qE "fail *= *1"; then
+ echo "FAIL: 'none' branch of threshold-eval sets fail=1 — that would break the default"
failures=$((failures+1))
else
- echo "PASS: 'none' branch of threshold-eval does not set FAIL=1"
+ echo "PASS: 'none' branch of threshold-eval does not set fail=1"
+fi
+
+# 17. action.yml must SOURCE the library in both steps that use it, and must
+# not carry its own copy of the case statement. If either regresses, the
+# unit tests go back to testing a transcription.
+lib_sources=$(grep -cF 'source "${{ github.action_path }}/lib/severity.sh"' "$ACTION_YML" || true)
+inline_cases=$(grep -cE '^\s*(critical|medium|low)\)\s*$' "$ACTION_YML" || true)
+if [ "$lib_sources" -ge 2 ] && [ "$inline_cases" -eq 0 ] \
+ && grep -q 'rafter_threshold_fails "\$SEVERITY_THRESHOLD"' "$ACTION_YML" \
+ && grep -q 'rafter_report_only_tip "\$FINDINGS_COUNT" "\$SEVERITY_THRESHOLD"' "$ACTION_YML"; then
+ echo "PASS: action.yml sources lib/severity.sh in both steps and carries no inline copy"
+else
+ echo "FAIL: action.yml sources=${lib_sources} (need >=2), inline case branches=${inline_cases} (need 0), or a call site is missing"
+ failures=$((failures+1))
fi
# ── sable-l10k: poll-path retry contract ─────────────────────────────────
@@ -163,6 +187,43 @@ else
failures=$((failures+1))
fi
+# ── sable-fgk7: an unreadable report is not a clean scan ─────────────────
+# The results step used to coerce every jq failure into findings_count=0,
+# which passed every threshold and rendered "No security findings detected".
+# Reproduced with a 200 whose body was not JSON, a 200 carrying an error
+# object, and a parseable payload with no vulnerabilities key.
+
+# 14. The payload shape must be validated before any count is computed.
+if grep -qF "jq -e 'type == \"object\" and (.vulnerabilities | type == \"array\") and all(.vulnerabilities[]; type == \"object\")'" "$ACTION_YML"; then
+ echo "PASS: results step validates the payload shape before counting"
+else
+ echo "FAIL: results step no longer validates that .vulnerabilities is an array of objects"
+ failures=$((failures+1))
+fi
+
+# 15. No count may fall back to 0 on a jq failure. That fallback IS the bug:
+# the error path and the clean path produced the same number.
+zero_fallbacks=$(grep -c '|| echo "0"' "$ACTION_YML" || true)
+if [ "$zero_fallbacks" -eq 0 ]; then
+ echo "PASS: no count falls back to 0 on a parse failure"
+else
+ echo "FAIL: ${zero_fallbacks} count(s) still fall back to 0 on a jq failure — an unreadable report would render as clean"
+ failures=$((failures+1))
+fi
+
+# 16. The unreadable-payload path must record status=unreadable and exit 1,
+# so the declared status output cannot fall back to the poll step's
+# 'completed' for a report that was never read.
+if awk '/no .vulnerabilities. array/,/^ fi$/' "$ACTION_YML" \
+ | grep -q 'status=unreadable' \
+ && awk '/no .vulnerabilities. array/,/^ fi$/' "$ACTION_YML" \
+ | grep -q 'exit 1'; then
+ echo "PASS: unreadable payload records status=unreadable and fails the step"
+else
+ echo "FAIL: unreadable-payload branch no longer records status=unreadable and exits 1"
+ failures=$((failures+1))
+fi
+
echo ""
echo "── results ───────────────────────────────────────────────────────────"
echo "Failures: $failures"
diff --git a/github-action/tests/test-pr-comment-tip.sh b/github-action/tests/test-pr-comment-tip.sh
index 01e8ba6..1ac97a1 100755
--- a/github-action/tests/test-pr-comment-tip.sh
+++ b/github-action/tests/test-pr-comment-tip.sh
@@ -1,39 +1,38 @@
#!/usr/bin/env bash
#
-# Unit test for the new PR-comment "report-only tip" block in
-# github-action/action.yml. Re-implements the if block verbatim and
-# exercises every input combination.
+# Unit test for the PR-comment "report-only tip" block in
+# github-action/action.yml. Sources github-action/lib/severity.sh — the SAME
+# file action.yml sources at run time — and exercises every input
+# combination of rafter_report_only_tip.
#
# The tip should appear iff (FINDINGS_COUNT > 0) AND (SEVERITY_THRESHOLD == 'none').
+#
+# This test used to carry its own copy of the if block (sable-1drb). It now
+# runs the code the action runs.
set -u
+# shellcheck source=../lib/severity.sh
+source "$(cd "$(dirname "$0")/.." && pwd)/lib/severity.sh"
+
failures=0
total=0
-# Mirror of the new block under the "Comment on PR" step's COMMENT_FILE builder.
-emit_tip_if_applicable() {
- if [ "$FINDINGS_COUNT" -gt 0 ] && [ "$SEVERITY_THRESHOLD" = "none" ]; then
- echo "> :information_source: This run is report-only. To fail the build on critical/high findings, set \`severity-threshold: high\` in your workflow."
- echo ""
- fi
-}
-
TIP_NEEDLE="report-only"
# assert_tip
assert_tip() {
local name="$1"; local expected="$2"
- FINDINGS_COUNT="$3"; SEVERITY_THRESHOLD="$4"
+ local findings="$3" threshold="$4"
total=$((total+1))
local out
- out=$(emit_tip_if_applicable)
+ out=$(rafter_report_only_tip "$findings" "$threshold")
local has_tip="no"
if echo "$out" | grep -q "$TIP_NEEDLE"; then has_tip="yes"; fi
if [ "$has_tip" != "$expected" ]; then
- echo "FAIL: $name — findings=$FINDINGS_COUNT threshold=$SEVERITY_THRESHOLD → expected tip=$expected got $has_tip"
+ echo "FAIL: $name — findings=$findings threshold=$threshold → expected tip=$expected got $has_tip"
failures=$((failures+1))
else
echo "PASS: $name (tip=$has_tip)"
@@ -53,6 +52,15 @@ assert_tip "findings + low" no 5 low
assert_tip "no findings + high" no 0 high
assert_tip "no findings + critical" no 0 critical
+echo "── the tip must tell the reader what to set ─────────────────────────"
+total=$((total+1))
+if rafter_report_only_tip 5 none | grep -q 'severity-threshold: high'; then
+ echo "PASS: tip names the input to set"
+else
+ echo "FAIL: tip no longer names severity-threshold: high"
+ failures=$((failures+1))
+fi
+
echo ""
echo "── results ───────────────────────────────────────────────────────────"
echo "Total: $total Failures: $failures"
diff --git a/github-action/tests/test-threshold-eval.sh b/github-action/tests/test-threshold-eval.sh
index 99dbfc3..eddb5c6 100755
--- a/github-action/tests/test-threshold-eval.sh
+++ b/github-action/tests/test-threshold-eval.sh
@@ -1,108 +1,95 @@
#!/usr/bin/env bash
#
# Unit test for the "Evaluate severity threshold" step in
-# github-action/action.yml. Re-implements the case statement verbatim and
-# exercises every branch with deliberate inputs.
+# github-action/action.yml. Sources github-action/lib/severity.sh — the SAME
+# file action.yml sources at run time — and exercises every branch of
+# rafter_threshold_fails with deliberate inputs.
#
-# If you change the case body in action.yml, you MUST change it here too —
-# the test-action-yml-defaults check enforces drift detection on the default
-# value, but the case body itself is duplicated by design (sourcing bash out
-# of YAML at test time is fragile).
+# This test used to carry its own copy of the case statement, so it could
+# pass in full while action.yml was broken (sable-1drb). It now runs the code
+# the action runs. The drift detector (test-action-yml-defaults.sh) separately
+# asserts that action.yml still sources the library rather than inlining a
+# copy again.
#
# Exit 0 = all cases pass. Exit 1 = at least one case failed.
set -u
+# shellcheck source=../lib/severity.sh
+source "$(cd "$(dirname "$0")/.." && pwd)/lib/severity.sh"
+
failures=0
total=0
-# Mirror of the case body in github-action/action.yml under the
-# "Evaluate severity threshold" step. Returns 1 if the threshold would
-# fail the build given the current *_COUNT envs, else 0.
-evaluate_threshold() {
- local FAIL=0
- case "$SEVERITY_THRESHOLD" in
- critical)
- [ "$CRITICAL_COUNT" -gt 0 ] && FAIL=1
- ;;
- high)
- [ "$CRITICAL_COUNT" -gt 0 ] || [ "$HIGH_COUNT" -gt 0 ] && FAIL=1
- ;;
- medium)
- [ "$CRITICAL_COUNT" -gt 0 ] || [ "$HIGH_COUNT" -gt 0 ] || [ "$MEDIUM_COUNT" -gt 0 ] && FAIL=1
- ;;
- low)
- [ "$CRITICAL_COUNT" -gt 0 ] || [ "$HIGH_COUNT" -gt 0 ] || [ "$MEDIUM_COUNT" -gt 0 ] || [ "$LOW_COUNT" -gt 0 ] && FAIL=1
- ;;
- none)
- FAIL=0
- ;;
- *)
- [ "$CRITICAL_COUNT" -gt 0 ] || [ "$HIGH_COUNT" -gt 0 ] && FAIL=1
- ;;
- esac
- return $FAIL
-}
-
-# assert_threshold
+# assert_threshold
assert_threshold() {
local name="$1"; local expected="$2"
- SEVERITY_THRESHOLD="$3"
- CRITICAL_COUNT="$4"; HIGH_COUNT="$5"; MEDIUM_COUNT="$6"; LOW_COUNT="$7"
+ local threshold="$3" crit="$4" high="$5" med="$6" low="$7"
total=$((total+1))
- evaluate_threshold
- local actual=$?
+ local actual="pass"
+ if rafter_threshold_fails "$threshold" "$crit" "$high" "$med" "$low" >/dev/null; then
+ actual="fail"
+ fi
if [ "$actual" != "$expected" ]; then
- echo "FAIL: $name — threshold=$SEVERITY_THRESHOLD crit=$CRITICAL_COUNT high=$HIGH_COUNT med=$MEDIUM_COUNT low=$LOW_COUNT → expected exit=$expected got $actual"
+ echo "FAIL: $name — threshold=$threshold crit=$crit high=$high med=$med low=$low → expected build=$expected got $actual"
failures=$((failures+1))
else
echo "PASS: $name"
fi
}
-echo "── 'none' threshold (the new default) — must never fail ─────────────"
-assert_threshold "none + no findings" 0 none 0 0 0 0
-assert_threshold "none + low only" 0 none 0 0 0 7
-assert_threshold "none + medium only" 0 none 0 0 3 0
-assert_threshold "none + high only" 0 none 0 5 0 0
-assert_threshold "none + critical only" 0 none 2 0 0 0
-assert_threshold "none + everything" 0 none 9 9 9 9
+echo "── 'none' threshold (the default) — must never fail ────────────────"
+assert_threshold "none + no findings" pass none 0 0 0 0
+assert_threshold "none + low only" pass none 0 0 0 7
+assert_threshold "none + medium only" pass none 0 0 3 0
+assert_threshold "none + high only" pass none 0 5 0 0
+assert_threshold "none + critical only" pass none 2 0 0 0
+assert_threshold "none + everything" pass none 9 9 9 9
echo "── 'critical' threshold — fail only on critical ────────────────────"
-assert_threshold "critical + clean" 0 critical 0 0 0 0
-assert_threshold "critical + only high" 0 critical 0 4 0 0
-assert_threshold "critical + only medium" 0 critical 0 0 4 0
-assert_threshold "critical + only low" 0 critical 0 0 0 4
-assert_threshold "critical + critical=1" 1 critical 1 0 0 0
-assert_threshold "critical + critical+high" 1 critical 1 5 0 0
+assert_threshold "critical + clean" pass critical 0 0 0 0
+assert_threshold "critical + only high" pass critical 0 4 0 0
+assert_threshold "critical + only medium" pass critical 0 0 4 0
+assert_threshold "critical + only low" pass critical 0 0 0 4
+assert_threshold "critical + critical=1" fail critical 1 0 0 0
+assert_threshold "critical + critical+high" fail critical 1 5 0 0
echo "── 'high' threshold — fail on critical or high ─────────────────────"
-assert_threshold "high + clean" 0 high 0 0 0 0
-assert_threshold "high + only medium" 0 high 0 0 4 0
-assert_threshold "high + only low" 0 high 0 0 0 4
-assert_threshold "high + critical only" 1 high 1 0 0 0
-assert_threshold "high + high only" 1 high 0 1 0 0
-assert_threshold "high + critical+high" 1 high 1 1 0 0
+assert_threshold "high + clean" pass high 0 0 0 0
+assert_threshold "high + only medium" pass high 0 0 4 0
+assert_threshold "high + only low" pass high 0 0 0 4
+assert_threshold "high + critical only" fail high 1 0 0 0
+assert_threshold "high + high only" fail high 0 1 0 0
+assert_threshold "high + critical+high" fail high 1 1 0 0
echo "── 'medium' threshold — fail on crit/high/medium ───────────────────"
-assert_threshold "medium + clean" 0 medium 0 0 0 0
-assert_threshold "medium + only low" 0 medium 0 0 0 4
-assert_threshold "medium + critical only" 1 medium 1 0 0 0
-assert_threshold "medium + high only" 1 medium 0 1 0 0
-assert_threshold "medium + medium only" 1 medium 0 0 1 0
+assert_threshold "medium + clean" pass medium 0 0 0 0
+assert_threshold "medium + only low" pass medium 0 0 0 4
+assert_threshold "medium + critical only" fail medium 1 0 0 0
+assert_threshold "medium + high only" fail medium 0 1 0 0
+assert_threshold "medium + medium only" fail medium 0 0 1 0
echo "── 'low' threshold — fail on anything ──────────────────────────────"
-assert_threshold "low + clean" 0 low 0 0 0 0
-assert_threshold "low + only low" 1 low 0 0 0 1
-assert_threshold "low + critical only" 1 low 1 0 0 0
+assert_threshold "low + clean" pass low 0 0 0 0
+assert_threshold "low + only low" fail low 0 0 0 1
+assert_threshold "low + critical only" fail low 1 0 0 0
echo "── unknown threshold — falls back to 'high' behavior ───────────────"
-assert_threshold "unknown + clean" 0 badvalue 0 0 0 0
-assert_threshold "unknown + critical" 1 badvalue 1 0 0 0
-assert_threshold "unknown + high" 1 badvalue 0 1 0 0
-assert_threshold "unknown + medium only" 0 badvalue 0 0 3 0
+assert_threshold "unknown + clean" pass badvalue 0 0 0 0
+assert_threshold "unknown + critical" fail badvalue 1 0 0 0
+assert_threshold "unknown + high" fail badvalue 0 1 0 0
+assert_threshold "unknown + medium only" pass badvalue 0 0 3 0
+
+echo "── unknown threshold — must say so in the log ──────────────────────"
+total=$((total+1))
+if rafter_threshold_fails badvalue 0 0 0 0 | grep -q "::warning::Unknown severity threshold 'badvalue'"; then
+ echo "PASS: unknown threshold emits a ::warning:: naming the value"
+else
+ echo "FAIL: unknown threshold no longer warns"
+ failures=$((failures+1))
+fi
echo ""
echo "── results ───────────────────────────────────────────────────────────"
diff --git a/node/src/commands/issues/from-scan.ts b/node/src/commands/issues/from-scan.ts
index 38852bc..aca9f0f 100644
--- a/node/src/commands/issues/from-scan.ts
+++ b/node/src/commands/issues/from-scan.ts
@@ -164,6 +164,36 @@ async function runFromScan(opts: {
}
}
+/**
+ * The findings list from a scan payload, or an error — never a silent [].
+ *
+ * A payload without a `vulnerabilities` array is not "no findings". It is a
+ * scan that has not completed, a failed scan, or a report this client cannot
+ * read; filing zero issues from it would report a clean codebase for work
+ * that was never done (sable-fgk7). An empty array IS a legitimate clean
+ * result and is returned as such.
+ */
+export function vulnerabilitiesFromPayload(
+ data: unknown,
+ scanId: string
+): BackendVulnerability[] {
+ const payload = data as { vulnerabilities?: unknown; status?: unknown } | null;
+ if (payload && Array.isArray(payload.vulnerabilities)) {
+ return payload.vulnerabilities as BackendVulnerability[];
+ }
+ const status = payload && typeof payload.status === "string" ? payload.status : undefined;
+ if (status && status !== "completed") {
+ throw new Error(
+ `Scan ${scanId} is ${status}, not completed — there are no findings to file yet. ` +
+ `Retry once it completes: rafter get ${scanId}`
+ );
+ }
+ throw new Error(
+ `Scan ${scanId} returned no 'vulnerabilities' array; refusing to treat an unreadable ` +
+ `report as zero findings. Check it with: rafter get ${scanId}`
+ );
+}
+
async function draftsFromBackendScan(
scanId: string,
apiKey?: string
@@ -174,11 +204,10 @@ async function draftsFromBackendScan(
headers: { "x-api-key": key },
});
- const vulns: BackendVulnerability[] = data.vulnerabilities || [];
- return vulns.map(buildFromBackendVulnerability);
+ return vulnerabilitiesFromPayload(data, scanId).map(buildFromBackendVulnerability);
}
-function draftsFromLocalScan(filePath: string): IssueDraft[] {
+export function draftsFromLocalScan(filePath: string): IssueDraft[] {
const raw = fs.readFileSync(filePath, "utf-8");
const parsed = JSON.parse(raw);
// New shape: { _note, scan_mode, triage_applied, results: [...] }
diff --git a/node/src/commands/issues/from-text.ts b/node/src/commands/issues/from-text.ts
index 1298263..947f2e0 100644
--- a/node/src/commands/issues/from-text.ts
+++ b/node/src/commands/issues/from-text.ts
@@ -130,7 +130,7 @@ async function readInput(opts: {
* - File paths → mentioned in body
* - Security keywords → security label
*/
-function parseNaturalText(text: string): ParsedIssue {
+export function parseNaturalText(text: string): ParsedIssue {
const lines = text.trim().split("\n");
const labels: string[] = [];
diff --git a/node/src/commands/issues/issue-builder.ts b/node/src/commands/issues/issue-builder.ts
index 04853ab..b9209d5 100644
--- a/node/src/commands/issues/issue-builder.ts
+++ b/node/src/commands/issues/issue-builder.ts
@@ -35,7 +35,7 @@ export interface LocalScanResult {
}>;
}
-function severityLabel(level: string): string {
+export function severityLabel(level: string): string {
const map: Record = {
error: "critical",
critical: "critical",
diff --git a/node/tests/issues.test.ts b/node/tests/issues.test.ts
index 4e81f42..77fe6a4 100644
--- a/node/tests/issues.test.ts
+++ b/node/tests/issues.test.ts
@@ -1,44 +1,29 @@
-import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
-import crypto from "crypto";
+import { describe, it, expect, beforeEach, afterEach } from "vitest";
import fs from "fs";
-import { execFileSync } from "child_process";
-
-// ── dedup logic (mirrored from src/commands/issues/dedup.ts) ─────────
-
-const FINGERPRINT_PREFIX = "";
-
-function fingerprint(file: string, ruleId: string): string {
- return crypto
- .createHash("sha256")
- .update(`${file}:${ruleId}`)
- .digest("hex")
- .slice(0, 12);
-}
-
-function embedFingerprint(body: string, fp: string): string {
- return `${body}\n\n${FINGERPRINT_PREFIX}${fp}${FINGERPRINT_SUFFIX}`;
-}
-
-function extractFingerprint(body: string): string | null {
- const idx = body.indexOf(FINGERPRINT_PREFIX);
- if (idx === -1) return null;
- const start = idx + FINGERPRINT_PREFIX.length;
- const end = body.indexOf(FINGERPRINT_SUFFIX, start);
- if (end === -1) return null;
- return body.slice(start, end);
-}
-
-type GHIssue = { number: number; title: string; body: string; labels: string[]; html_url: string; state: string };
-
-function findDuplicates(existingIssues: GHIssue[], newFingerprints: string[]): Set {
- const existingFps = new Set();
- for (const issue of existingIssues) {
- const fp = extractFingerprint(issue.body);
- if (fp) existingFps.add(fp);
- }
- return new Set(newFingerprints.filter((fp) => existingFps.has(fp)));
-}
+// The real functions, not mirrors. This file used to carry hand-copied
+// re-implementations of dedup, issue-builder, from-text and from-scan and
+// tested those; it could pass in full while the source was broken
+// (sable-1drb, specimen 11 of sable-d2x2). Every describe below now
+// exercises the code the CLI ships.
+import {
+ fingerprint,
+ embedFingerprint,
+ extractFingerprint,
+ findDuplicates,
+} from "../src/commands/issues/dedup.js";
+import type { GitHubIssue as GHIssue } from "../src/commands/issues/github-client.js";
+import {
+ severityLabel,
+ buildFromBackendVulnerability,
+ buildFromLocalMatch,
+ type IssueDraft,
+ type BackendVulnerability,
+ type LocalScanResult,
+} from "../src/commands/issues/issue-builder.js";
+import { parseNaturalText } from "../src/commands/issues/from-text.js";
+import { vulnerabilitiesFromPayload, draftsFromLocalScan } from "../src/commands/issues/from-scan.js";
+
+type LocalMatch = LocalScanResult["matches"][number];
describe("fingerprint", () => {
it("produces deterministic 12-char hex hash", () => {
@@ -108,91 +93,6 @@ describe("findDuplicates", () => {
});
});
-// ── issue-builder logic (mirrored from src/commands/issues/issue-builder.ts) ──
-
-function severityLabel(level: string): string {
- const map: Record = {
- error: "critical",
- critical: "critical",
- warning: "high",
- high: "high",
- note: "medium",
- medium: "medium",
- low: "low",
- };
- return map[level.toLowerCase()] || "medium";
-}
-
-function severityEmoji(level: string): string {
- const sev = severityLabel(level);
- const emojis: Record = {
- critical: "\u{1F534}",
- high: "\u{1F7E0}",
- medium: "\u{1F7E1}",
- low: "\u{1F7E2}",
- };
- return emojis[sev] || "\u{1F7E1}";
-}
-
-interface IssueDraft {
- title: string;
- body: string;
- labels: string[];
- fingerprint: string;
-}
-
-interface BackendVulnerability {
- ruleId: string;
- level: string;
- message: string;
- file: string;
- line?: number;
-}
-
-function buildFromBackendVulnerability(vuln: BackendVulnerability): IssueDraft {
- const sev = severityLabel(vuln.level);
- const emoji = severityEmoji(vuln.level);
- const fp = fingerprint(vuln.file, vuln.ruleId);
- const title = `${emoji} [${sev.toUpperCase()}] ${vuln.ruleId}: ${vuln.message.length > 80 ? vuln.message.slice(0, 77) + "..." : vuln.message}`;
- let body = `## Security Finding\n\n`;
- body += `**Rule:** \`${vuln.ruleId}\`\n`;
- body += `**Severity:** ${sev}\n`;
- body += `**File:** \`${vuln.file}\``;
- if (vuln.line) body += ` (line ${vuln.line})`;
- body += `\n\n`;
- body += `### Description\n\n${vuln.message}\n\n`;
- body += `### Remediation\n\nReview and fix the finding in \`${vuln.file}\`.\n`;
- body += `\n---\n*Created by [Rafter CLI](https://rafter.so) — security for AI builders*\n`;
- const labels = ["security", `severity:${sev}`, `rule:${vuln.ruleId}`];
- return { title, body: embedFingerprint(body, fp), labels, fingerprint: fp };
-}
-
-type LocalMatch = { pattern: { name: string; severity: string; description?: string }; line?: number; column?: number; redacted?: string };
-
-function buildFromLocalMatch(file: string, match: LocalMatch): IssueDraft {
- const sev = severityLabel(match.pattern.severity);
- const emoji = severityEmoji(match.pattern.severity);
- const fp = fingerprint(file, match.pattern.name);
- const basename = file.split("/").pop() || file;
- const title = `${emoji} [${sev.toUpperCase()}] Secret detected: ${match.pattern.name} in ${basename}`;
- let body = `## Secret Detection\n\n`;
- body += `**Pattern:** \`${match.pattern.name}\`\n`;
- body += `**Severity:** ${sev}\n`;
- body += `**File:** \`${file}\``;
- if (match.line) body += ` (line ${match.line})`;
- body += `\n`;
- if (match.redacted) body += `**Match:** \`${match.redacted}\`\n`;
- body += `\n`;
- if (match.pattern.description) body += `### Description\n\n${match.pattern.description}\n\n`;
- body += `### Remediation\n\n`;
- body += `1. Rotate the exposed credential immediately\n`;
- body += `2. Remove the secret from source code\n`;
- body += `3. Use environment variables or a secrets manager instead\n`;
- body += `\n---\n*Created by [Rafter CLI](https://rafter.so) — security for AI builders*\n`;
- const labels = ["security", "secret-detected", `severity:${sev}`];
- return { title, body: embedFingerprint(body, fp), labels, fingerprint: fp };
-}
-
describe("severityLabel", () => {
it("maps error to critical", () => expect(severityLabel("error")).toBe("critical"));
it("maps warning to high", () => expect(severityLabel("warning")).toBe("high"));
@@ -285,63 +185,6 @@ describe("buildFromLocalMatch", () => {
});
});
-// ── from-text parsing logic (mirrored from src/commands/issues/from-text.ts) ──
-
-interface ParsedIssue {
- title: string;
- body: string;
- labels: string[];
-}
-
-function parseNaturalText(text: string): ParsedIssue {
- const lines = text.trim().split("\n");
- const labels: string[] = [];
- let title = "";
- let bodyStart = 0;
- for (let i = 0; i < lines.length; i++) {
- const line = lines[i].trim();
- if (line) {
- title = line.replace(/^#+\s*/, "").trim();
- bodyStart = i + 1;
- break;
- }
- }
- if (!title) title = "Security issue reported via Rafter CLI";
- if (title.length > 120) title = title.slice(0, 117) + "...";
- const bodyLines = lines.slice(bodyStart);
- let body = bodyLines.join("\n").trim();
- if (!body) body = text.trim();
- const textLower = text.toLowerCase();
- if (textLower.includes("critical") || textLower.includes("p0")) {
- labels.push("severity:critical");
- } else if (textLower.includes("high severity") || textLower.includes("high risk") || textLower.includes("p1")) {
- labels.push("severity:high");
- } else if (textLower.includes("medium") || textLower.includes("p2")) {
- labels.push("severity:medium");
- } else if (textLower.includes("low") || textLower.includes("p3")) {
- labels.push("severity:low");
- }
- const securityKeywords = [
- "security", "vulnerability", "cve", "cwe", "owasp", "secret",
- "credential", "token", "password", "injection", "xss", "csrf", "ssrf", "exploit",
- ];
- if (securityKeywords.some((kw) => textLower.includes(kw))) {
- labels.push("security");
- }
- const fileRefs = text.match(/(?:^|\s)([a-zA-Z0-9_./-]+\.[a-zA-Z]{1,10})(?::(\d+))?/gm);
- if (fileRefs && fileRefs.length > 0) {
- const files = fileRefs.map((f) => f.trim()).filter((f) => f.includes("/") || f.includes("."));
- if (files.length > 0) {
- body += `\n\n### Referenced Files\n\n`;
- for (const f of files.slice(0, 10)) {
- body += `- \`${f}\`\n`;
- }
- }
- }
- body += `\n\n---\n*Created by [Rafter CLI](https://rafter.so) — security for AI builders*\n`;
- return { title, body, labels: [...new Set(labels)] };
-}
-
describe("parseNaturalText", () => {
it("extracts first line as title", () => {
const result = parseNaturalText("SQL injection in login form\nDetails here");
@@ -413,20 +256,6 @@ describe("parseNaturalText", () => {
});
});
-// ── from-scan command logic (mirrored from src/commands/issues/from-scan.ts) ──
-
-function draftsFromLocalScan(filePath: string): IssueDraft[] {
- const raw = fs.readFileSync(filePath, "utf-8");
- const results: Array<{ file: string; matches: LocalMatch[] }> = JSON.parse(raw);
- const drafts: IssueDraft[] = [];
- for (const result of results) {
- for (const match of result.matches) {
- drafts.push(buildFromLocalMatch(result.file, match));
- }
- }
- return drafts;
-}
-
describe("from-scan: draftsFromLocalScan", () => {
const sampleScanJson: Array<{ file: string; matches: LocalMatch[] }> = [
{
@@ -657,6 +486,61 @@ describe("from-scan: backend vulnerability drafts", () => {
});
});
+// ── from-scan: a payload with no findings list is an error, not zero ──
+//
+// sable-fgk7. `data.vulnerabilities || []` turned a still-running scan, a
+// failed scan, a 200 carrying an error object, and a schema-valid payload with
+// no key into "No findings to create issues for". Delete-the-subject test:
+// with the fallback restored, every case below except the first two passes
+// with [] and this suite fails.
+
+describe("from-scan: vulnerabilitiesFromPayload (sable-fgk7)", () => {
+ const SCAN = "scan-abc";
+
+ it("returns the list when present", () => {
+ const v = [{ ruleId: "r", level: "error", message: "m", file: "f" }];
+ expect(vulnerabilitiesFromPayload({ status: "completed", vulnerabilities: v }, SCAN)).toBe(v);
+ });
+
+ it("returns an empty list as a legitimate clean result", () => {
+ expect(vulnerabilitiesFromPayload({ status: "completed", vulnerabilities: [] }, SCAN)).toEqual([]);
+ });
+
+ it("refuses a completed payload with no vulnerabilities key", () => {
+ expect(() => vulnerabilitiesFromPayload({ scan_id: SCAN, status: "completed" }, SCAN))
+ .toThrow(/no 'vulnerabilities' array/);
+ });
+
+ it("names the status when the scan has not completed", () => {
+ expect(() => vulnerabilitiesFromPayload({ status: "processing" }, SCAN))
+ .toThrow(/is processing, not completed/);
+ expect(() => vulnerabilitiesFromPayload({ status: "failed" }, SCAN))
+ .toThrow(/is failed, not completed/);
+ });
+
+ it("refuses a 200 whose body is an error object", () => {
+ expect(() =>
+ vulnerabilitiesFromPayload({ error: "Failed to fetch report from storage: Object not found" }, SCAN)
+ ).toThrow(/no 'vulnerabilities' array/);
+ });
+
+ it("refuses a vulnerabilities value that is not a list", () => {
+ expect(() => vulnerabilitiesFromPayload({ vulnerabilities: "3" }, SCAN)).toThrow();
+ expect(() => vulnerabilitiesFromPayload({ vulnerabilities: null }, SCAN)).toThrow();
+ });
+
+ it("refuses non-object payloads", () => {
+ expect(() => vulnerabilitiesFromPayload(null, SCAN)).toThrow();
+ expect(() => vulnerabilitiesFromPayload("502", SCAN)).toThrow();
+ });
+
+ it("points at the scan id in every refusal", () => {
+ for (const bad of [{ status: "completed" }, { status: "processing" }, {}]) {
+ expect(() => vulnerabilitiesFromPayload(bad, SCAN)).toThrow(new RegExp(`rafter get ${SCAN}`));
+ }
+ });
+});
+
// ── from-scan: --repo flag override ──────────────────────────────────
describe("from-scan: repo flag", () => {
diff --git a/python/rafter_cli/commands/issues/issues_app.py b/python/rafter_cli/commands/issues/issues_app.py
index ec198c4..2e74bc2 100644
--- a/python/rafter_cli/commands/issues/issues_app.py
+++ b/python/rafter_cli/commands/issues/issues_app.py
@@ -14,7 +14,7 @@
import requests
import typer
-from ...utils.api import api_url, API_BASE, api_get, EXIT_GENERAL_ERROR, EXIT_SUCCESS, resolve_key
+from ...utils.api import api_url, api_get, EXIT_GENERAL_ERROR, resolve_key
from ...utils.formatter import fmt, print_stderr
from ...utils.git import detect_repo
from .dedup import find_duplicates
@@ -74,7 +74,13 @@ def from_scan(
# Build drafts
if scan_id:
- drafts = _drafts_from_backend(scan_id, api_key)
+ try:
+ drafts = _drafts_from_backend(scan_id, api_key)
+ except (UnreadableScanPayload, requests.RequestException, ValueError) as e:
+ # ValueError covers a non-JSON body from resp.json(). None of these
+ # is "no findings"; each is a scan whose report could not be read.
+ print_stderr(fmt.error(str(e)))
+ raise typer.Exit(code=EXIT_GENERAL_ERROR)
else:
drafts = _drafts_from_local(from_local) # type: ignore[arg-type]
@@ -211,6 +217,33 @@ def from_text(
# ── Internal helpers ──────────────────────────────────────────────────
+class UnreadableScanPayload(ValueError):
+ """The scan payload carries no findings list this command can file from."""
+
+
+def vulnerabilities_from_payload(data: object, scan_id: str) -> list[dict]:
+ """The findings list from a scan payload, or an error — never a silent [].
+
+ A payload without a ``vulnerabilities`` list is not "no findings". It is a
+ scan that has not completed, a failed scan, or a report this client cannot
+ read; filing zero issues from it would report a clean codebase for work
+ that was never done (sable-fgk7). An empty list IS a legitimate clean
+ result and is returned as such.
+ """
+ if isinstance(data, dict) and isinstance(data.get("vulnerabilities"), list):
+ return data["vulnerabilities"]
+ status = data.get("status") if isinstance(data, dict) else None
+ if isinstance(status, str) and status != "completed":
+ raise UnreadableScanPayload(
+ f"Scan {scan_id} is {status}, not completed — there are no findings to "
+ f"file yet. Retry once it completes: rafter get {scan_id}"
+ )
+ raise UnreadableScanPayload(
+ f"Scan {scan_id} returned no 'vulnerabilities' array; refusing to treat an "
+ f"unreadable report as zero findings. Check it with: rafter get {scan_id}"
+ )
+
+
def _drafts_from_backend(scan_id: str, api_key: str | None) -> list[IssueDraft]:
key = resolve_key(api_key)
resp = api_get(
@@ -222,7 +255,7 @@ def _drafts_from_backend(scan_id: str, api_key: str | None) -> list[IssueDraft]:
resp.raise_for_status()
data = resp.json()
- vulns = data.get("vulnerabilities", [])
+ vulns = vulnerabilities_from_payload(data, scan_id)
return [
build_from_backend_vulnerability(
BackendVulnerability(
diff --git a/python/tests/test_issues.py b/python/tests/test_issues.py
index f24bdf2..5497d0d 100644
--- a/python/tests/test_issues.py
+++ b/python/tests/test_issues.py
@@ -683,3 +683,122 @@ def _parse(self, text):
def test_labels_are_unique(self):
result = self._parse("Critical security vulnerability with credentials and tokens")
assert len(result["labels"]) == len(set(result["labels"]))
+
+
+# ── from-scan: a payload with no findings list is an error, not zero ──
+#
+# sable-fgk7. `data.get("vulnerabilities", [])` turned a still-running scan, a
+# failed scan, a 200 carrying an error object, and a schema-valid payload with
+# no key into "No findings to create issues for". Delete-the-subject test:
+# with the fallback restored, every case below except the first two passes
+# with [] and this class fails. Mirrors the Node suite of the same name.
+
+
+class TestVulnerabilitiesFromPayload:
+ SCAN = "scan-abc"
+
+ def _call(self, data):
+ from rafter_cli.commands.issues.issues_app import vulnerabilities_from_payload
+
+ return vulnerabilities_from_payload(data, self.SCAN)
+
+ def test_returns_the_list_when_present(self):
+ v = [{"ruleId": "r", "level": "error", "message": "m", "file": "f"}]
+ assert self._call({"status": "completed", "vulnerabilities": v}) is v
+
+ def test_empty_list_is_a_legitimate_clean_result(self):
+ assert self._call({"status": "completed", "vulnerabilities": []}) == []
+
+ def test_refuses_completed_payload_with_no_key(self):
+ from rafter_cli.commands.issues.issues_app import UnreadableScanPayload
+
+ with pytest.raises(UnreadableScanPayload, match="no 'vulnerabilities' array"):
+ self._call({"scan_id": self.SCAN, "status": "completed"})
+
+ def test_names_the_status_when_not_completed(self):
+ from rafter_cli.commands.issues.issues_app import UnreadableScanPayload
+
+ with pytest.raises(UnreadableScanPayload, match="is processing, not completed"):
+ self._call({"status": "processing"})
+ with pytest.raises(UnreadableScanPayload, match="is failed, not completed"):
+ self._call({"status": "failed"})
+
+ def test_refuses_error_object(self):
+ from rafter_cli.commands.issues.issues_app import UnreadableScanPayload
+
+ with pytest.raises(UnreadableScanPayload, match="no 'vulnerabilities' array"):
+ self._call({"error": "Failed to fetch report from storage: Object not found"})
+
+ def test_refuses_vulnerabilities_that_is_not_a_list(self):
+ from rafter_cli.commands.issues.issues_app import UnreadableScanPayload
+
+ with pytest.raises(UnreadableScanPayload):
+ self._call({"vulnerabilities": "3"})
+ with pytest.raises(UnreadableScanPayload):
+ self._call({"vulnerabilities": None})
+
+ def test_refuses_non_dict_payloads(self):
+ from rafter_cli.commands.issues.issues_app import UnreadableScanPayload
+
+ with pytest.raises(UnreadableScanPayload):
+ self._call(None)
+ with pytest.raises(UnreadableScanPayload):
+ self._call("502")
+
+ def test_points_at_the_scan_id_in_every_refusal(self):
+ from rafter_cli.commands.issues.issues_app import UnreadableScanPayload
+
+ for bad in ({"status": "completed"}, {"status": "processing"}, {}):
+ with pytest.raises(UnreadableScanPayload, match=f"rafter get {self.SCAN}"):
+ self._call(bad)
+
+
+class TestFromScanCommandRefusesUnreadablePayload:
+ """The command surface: an unreadable payload exits 1 with the message on
+ stderr, and never reaches the "No findings to create issues for" path."""
+
+ def test_unreadable_payload_exits_1(self, monkeypatch, capsys):
+ from unittest.mock import MagicMock
+
+ import typer
+
+ from rafter_cli.commands.issues import issues_app as mod
+
+ resp = MagicMock()
+ resp.status_code = 200
+ resp.json.return_value = {"scan_id": "scan-abc", "status": "completed"}
+ resp.raise_for_status.return_value = None
+ monkeypatch.setattr(mod, "api_get", lambda *a, **k: resp)
+ monkeypatch.setattr(mod, "resolve_key", lambda _k: "key")
+
+ with pytest.raises(typer.Exit) as exc:
+ mod.from_scan(
+ scan_id="scan-abc", from_local=None, repo="org/repo", api_key="k",
+ no_dedup=True, dry_run=True, quiet=False,
+ )
+ assert exc.value.exit_code == 1
+ err = capsys.readouterr().err
+ assert "no 'vulnerabilities' array" in err
+ assert "No findings to create issues for" not in err
+
+ def test_non_json_body_exits_1(self, monkeypatch, capsys):
+ from unittest.mock import MagicMock
+
+ import typer
+
+ from rafter_cli.commands.issues import issues_app as mod
+
+ resp = MagicMock()
+ resp.status_code = 200
+ resp.json.side_effect = ValueError("Expecting value: line 1 column 1 (char 0)")
+ resp.raise_for_status.return_value = None
+ monkeypatch.setattr(mod, "api_get", lambda *a, **k: resp)
+ monkeypatch.setattr(mod, "resolve_key", lambda _k: "key")
+
+ with pytest.raises(typer.Exit) as exc:
+ mod.from_scan(
+ scan_id="scan-abc", from_local=None, repo="org/repo", api_key="k",
+ no_dedup=True, dry_run=True, quiet=False,
+ )
+ assert exc.value.exit_code == 1
+ assert "No findings to create issues for" not in capsys.readouterr().err
diff --git a/shared-docs/CLI_SPEC.md b/shared-docs/CLI_SPEC.md
index cd23373..ba6e070 100644
--- a/shared-docs/CLI_SPEC.md
+++ b/shared-docs/CLI_SPEC.md
@@ -149,6 +149,7 @@ After either budget is exhausted the command exits `1`. If the failures reached
- It has no "first poll" distinction: by the time it polls, the trigger step has already returned a `scan_id`, so **every** 404 there is treated as read-after-write lag. A scan id the backend accepted but never persisted therefore fails after the 5-failure budget rather than immediately.
- Its poll loop is additionally bounded by a wall-clock deadline derived from `timeout-minutes`. Before v0.11 that input was a poll *count*, so a slow API could overrun it; it is now a real deadline.
- Its `status` output is `completed`, `failed`, `timeout`, `unreadable` (the scan may have finished but its report could not be read), or `unreachable` (the API could not be contacted).
+- Its results step validates the payload **before** counting. A body with no `vulnerabilities` array — not JSON, a `200` carrying an error object, or a parseable payload missing the key — is `status=unreadable`, the job fails, and **no count outputs are written**: a consumer reading `findings-count` sees an empty string, never a fabricated `0`. A report the action cannot read is not a clean scan. An empty array is a clean scan and counts as `0`.
### rafter usage [OPTIONS]
@@ -1245,6 +1246,8 @@ Create GitHub issues from scan results.
- `--dry-run` — show issues that would be created without actually creating them
- `--quiet` — suppress status messages
+**A scan payload without a `vulnerabilities` array is an error, not zero findings.** With `--scan-id`, a response that has no `vulnerabilities` list — the scan is still `processing`, it `failed`, the body is not JSON, or it is an error object — exits `1` with a message on stderr that names the scan id and `rafter get `. It never prints "No findings to create issues for". An empty list is a legitimate clean result. Both runtimes.
+
#### rafter issues create from-text [OPTIONS]
Create a GitHub issue from natural language text (stdin, file, or inline).