From 6bf16604f71c571f247da622644bb962be0740a4 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Fri, 30 Oct 2026 16:47:58 -0500 Subject: [PATCH 1/3] feat(audit): weekly CodeRabbit Deep Scan audit that files findings as issues Reads the org's Deep Scan inventory from the CodeRabbit API as SARIF and keeps one issue per finding, keyed on findingFingerprint, in the repo that owns the code. A finding in a caller stub goes to the repo whose reusable workflow it calls. Every org repo is public and these findings can describe exploit chains, so issues, logs, the summary and the artifact carry only opaque ids, severity, state and repo names. Findings are unverified LLM output, so issues get needs-verification and never dev-lead. Issues close only when CodeRabbit reports fixed or dismissed, since a partial scan omits findings it did not reach. Any failed read or unexpected export fails the run with no issue writes. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/coderabbit-audit-tests.yml | 57 ++ .github/workflows/coderabbit-audit.yml | 85 +++ scripts/coderabbit-audit.sh | 575 ++++++++++++++++++ standards/ci-standards.md | 61 ++ standards/github-settings.md | 1 + test/scripts/coderabbit-audit/audit-run.bats | 294 +++++++++ .../coderabbit-audit/pure-helpers.bats | 126 ++++ test/scripts/coderabbit-audit/stubs/curl | 14 + test/scripts/coderabbit-audit/stubs/gh | 25 + 9 files changed, 1238 insertions(+) create mode 100644 .github/workflows/coderabbit-audit-tests.yml create mode 100644 .github/workflows/coderabbit-audit.yml create mode 100644 scripts/coderabbit-audit.sh create mode 100644 test/scripts/coderabbit-audit/audit-run.bats create mode 100644 test/scripts/coderabbit-audit/pure-helpers.bats create mode 100755 test/scripts/coderabbit-audit/stubs/curl create mode 100755 test/scripts/coderabbit-audit/stubs/gh diff --git a/.github/workflows/coderabbit-audit-tests.yml b/.github/workflows/coderabbit-audit-tests.yml new file mode 100644 index 000000000..bbb171e5f --- /dev/null +++ b/.github/workflows/coderabbit-audit-tests.yml @@ -0,0 +1,57 @@ +# Quality gates for the CodeRabbit audit script and its tests. +# +# Gates (all must pass before merge): +# 1. shellcheck — static analysis for the audit script +# 2. bats — pure helpers (severity mapping, routing, identity validation) +# and full runs against stubbed gh/curl (no finding detail in +# public output, fail-closed reads, issue lifecycle, no dev-lead) +name: CodeRabbit Audit Tests + +on: + pull_request: + paths: + - 'scripts/coderabbit-audit.sh' + - 'test/scripts/coderabbit-audit/**' + - '.github/workflows/coderabbit-audit.yml' + - '.github/workflows/coderabbit-audit-tests.yml' + push: + branches: + - main + paths: + - 'scripts/coderabbit-audit.sh' + - 'test/scripts/coderabbit-audit/**' + - '.github/workflows/coderabbit-audit.yml' + - '.github/workflows/coderabbit-audit-tests.yml' + +permissions: + contents: read + +concurrency: + group: coderabbit-audit-tests-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + name: Lint and bats + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 1 + persist-credentials: false + + - name: Install bats, shellcheck, and jq + run: | + set -euo pipefail + sudo apt-get update -qq + sudo apt-get install -y --no-install-recommends bats shellcheck jq + + - name: shellcheck + run: | + set -euo pipefail + shellcheck --severity=warning -x scripts/coderabbit-audit.sh + + - name: Run bats suite + run: bats --print-output-on-failure test/scripts/coderabbit-audit/ diff --git a/.github/workflows/coderabbit-audit.yml b/.github/workflows/coderabbit-audit.yml new file mode 100644 index 000000000..2c318557d --- /dev/null +++ b/.github/workflows/coderabbit-audit.yml @@ -0,0 +1,85 @@ +# Weekly org-wide CodeRabbit Deep Scan audit. +# +# Reads the org's Deep Scan findings from the CodeRabbit API, routes each one to +# the repo that owns the code, and idempotently creates/updates/closes one GitHub +# issue per finding. Findings are unverified LLM output, so issues get +# needs-verification and never dev-lead; a person adds dev-lead after confirming. +# +# Runs Thursdays, after the Wednesday CodeQL audit and before the Friday +# compliance audit, so no two audits file into the same repos on the same day. +# The API only reads results: Deep Scans run on CodeRabbit's own schedule. +# +# Every org repo is public, so this run's logs, summary and artifact are too. The +# script writes only opaque ids, severities and counts to any of them. +# +# Idempotency, labeling, routing and lifecycle live in scripts/coderabbit-audit.sh. +# Standard: standards/ci-standards.md#coderabbit-audit-coderabbit-audityml +name: CodeRabbit Audit + +on: + schedule: + # Minute 1 = cron_offset_minute coderabbit-audit.yml (scripts/lib/cron-timing.sh). + - cron: '1 17 * * 4' # Every Thursday at 17:01 UTC + workflow_dispatch: + inputs: + target_repo: + description: 'Limit to a specific repo (owner/name or name). Leave blank to read all.' + required: false + type: string + dry_run: + description: 'Dry run — read and report only, skip issue writes' + required: false + default: false + type: boolean + +permissions: {} + +concurrency: + group: coderabbit-audit + cancel-in-progress: false # let a running audit finish to avoid partial issue state + +jobs: + audit: + name: CodeRabbit Audit + runs-on: ubuntu-latest + timeout-minutes: 30 + permissions: + contents: read + steps: + - name: Checkout .github repo + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Run CodeRabbit audit + env: + # Needs, on every org repo: Contents (read), Issues (read/write), + # Metadata (read); plus Organization metadata (read) to list repos. + GH_TOKEN: ${{ secrets.ORG_SCORECARD_TOKEN }} + # Non-agentic key with security:read. + CODERABBIT_API_KEY: ${{ secrets.CODERABBIT_API_KEY }} + ORG: ${{ github.repository_owner }} + REPORT_DIR: ${{ runner.temp }}/coderabbit-report + TARGET_REPO: ${{ inputs.target_repo || '' }} + DRY_RUN: ${{ inputs.dry_run && 'true' || 'false' }} + run: bash scripts/coderabbit-audit.sh + + - name: Write step summary + if: always() + env: + SUMMARY: ${{ runner.temp }}/coderabbit-report/summary.md + run: | + if [ -f "$SUMMARY" ]; then + cat "$SUMMARY" >> "$GITHUB_STEP_SUMMARY" + else + echo "CodeRabbit audit did not produce a summary." >> "$GITHUB_STEP_SUMMARY" + fi + + - name: Upload audit report + if: always() + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: coderabbit-report + # Summary only: this repo is public, and the summary holds counts alone. + path: ${{ runner.temp }}/coderabbit-report/summary.md + retention-days: 90 diff --git a/scripts/coderabbit-audit.sh b/scripts/coderabbit-audit.sh new file mode 100644 index 000000000..da5c1f259 --- /dev/null +++ b/scripts/coderabbit-audit.sh @@ -0,0 +1,575 @@ +#!/usr/bin/env bash +# Org-wide CodeRabbit Deep Scan audit → idempotent GitHub issues. +# +# Reads the org's Deep Scan inventory from the CodeRabbit API as SARIF (the +# paginated JSON format has no findingFingerprint), routes each finding to the +# repo that owns the code, and creates/updates/closes one GitHub issue per +# finding. Issues carry: +# - coderabbit-audit → marker label for idempotent listing/closing +# - needs-verification → a person has not confirmed this LLM finding yet +# - security → every Deep Scan finding is a security finding +# - priority: → prioritization from the finding's severity +# +# Never dev-lead: findings are unverified LLM output. A person adds dev-lead +# after confirming the finding; this script neither adds nor removes it. +# +# Public-safe by construction: every org repo is public, so issue bodies, logs, +# the run summary and the uploaded artifact are public. Only allow-listed opaque +# fields (fingerprint, finding id, severity, state, repo) ever leave parse_sarif. +# Description, category, CWE, subsystem, path, line and call stack do not. +# +# Lifecycle: an issue closes only when the export explicitly reports its +# finding as fixed or dismissed. A finding merely absent from the export never +# closes one: the export reflects each repo's latest scan, which may have only +# partial coverage. A failed read or an unexpected export writes nothing. +# +# Env: +# CODERABBIT_API_KEY non-agentic key with security:read (required) +# ORG GitHub owner (default: combsmith) +# REPORT_DIR output dir (default: mktemp -d) +# TARGET_REPO limit to one repo (owner/name or name); blank = all +# MIN_SEVERITY lowest severity that opens a new issue (default: low) +# DRY_RUN "true" → scan + report, no issue writes (default: false) +# CODERABBIT_API_URL API base URL (default: https://api.coderabbit.ai) +# +# Standard: standards/ci-standards.md#coderabbit-audit-coderabbit-audityml +set -euo pipefail + +ORG="${ORG:-combsmith}" +CODERABBIT_AUDIT_LABEL="coderabbit-audit" +VERIFY_LABEL="needs-verification" +DRY_RUN="${DRY_RUN:-false}" +TARGET_REPO="${TARGET_REPO:-}" +MIN_SEVERITY="${MIN_SEVERITY:-low}" +CODERABBIT_API_URL="${CODERABBIT_API_URL:-https://api.coderabbit.ai}" +REPORT_DIR="${REPORT_DIR:-$(mktemp -d)}" + +FINDINGS_FILE="$REPORT_DIR/findings.json" +SUMMARY_FILE="$REPORT_DIR/summary.md" +COUNTS_FILE="$REPORT_DIR/issue-counts.json" + +ISSUES_ADDED=0 +ISSUES_EXISTING=0 +ISSUES_REMOVED=0 +BELOW_THRESHOLD=0 +NOT_IN_EXPORT=0 +UNMAPPED=0 +RUN_ERROR="" + +info() { echo "[info] $*" >&2; } +warn() { echo "[warn] $*" >&2; } + +# --------------------------------------------------------------------------- +# Pure helpers (unit-tested via bats; keep free of side effects) +# --------------------------------------------------------------------------- + +# Lowercase a severity and reject anything outside CodeRabbit's scale, so no +# free text from the export can reach a label, title or body. +normalize_severity() { + local sev + sev=$(printf '%s' "${1:-}" | tr '[:upper:]' '[:lower:]') + case "$sev" in + critical|high|medium|low) echo "$sev" ;; + *) echo "unknown" ;; + esac +} + +# Numeric rank for a normalized severity (higher = worse). An unknown severity +# ranks as medium so a scale change files issues rather than hiding them. +severity_rank() { + case "$1" in + critical) echo 4 ;; + high) echo 3 ;; + medium|unknown) echo 2 ;; + low) echo 1 ;; + *) echo 2 ;; + esac +} + +# Map a normalized severity → the org's shared priority scale (the same labels +# the CodeQL and SonarCloud audits use, so one triage view covers all three). +severity_priority_label() { + case "$1" in + critical) echo "priority:blocker" ;; + high) echo "priority:critical" ;; + medium|unknown) echo "priority:major" ;; + low) echo "priority:minor" ;; + *) echo "priority:major" ;; + esac +} + +# findingFingerprint is the idempotency key, so anything but its known shape +# means the export changed and nothing can be tracked safely. +valid_fingerprint() { + [[ "${1:-}" =~ ^[0-9a-f]{64}$ ]] +} + +# findingId is opaque too, but it is printed in public issues: only a UUID passes. +valid_finding_id() { + [[ "${1:-}" =~ ^[0-9a-fA-F]{8}(-[0-9a-fA-F]{4}){3}-[0-9a-fA-F]{12}$ ]] +} + +# Stable idempotency key, embedded as a hidden HTML comment in each issue body. +issue_marker() { + echo "" +} + +# Stable issue title; severity stays out so a re-rating keeps the same title. +issue_title() { + echo "CodeRabbit Deep Scan: finding ${1:0:12}" +} + +# Echo the org repo a caller stub delegates to, or nothing when the workflow is +# not a stub. A stub has no steps of its own, so a finding located in one can +# only have its sink in the called workflow. Call-stack frames cannot decide +# this: CodeRabbit guarantees neither their order nor their grammar. A stub +# calling more than one org repo is ambiguous and echoes nothing. +# args: +stub_callee_repo() { + local org="$1" text="$2" callees + grep -Eq '^[[:space:]]+steps:' <<< "$text" && return 0 + callees=$(grep -E "^[[:space:]]*(-[[:space:]]+)?uses:[[:space:]]*[\"']?${org}/" <<< "$text" \ + | sed -nE "s#.*${org}/([A-Za-z0-9._-]+)/\.github/workflows/.*#\1#p" \ + | sort -u) || true + [[ -n "$callees" ]] && [[ "$(grep -c . <<< "$callees")" = "1" ]] && echo "$callees" + return 0 +} + +# The repo a finding's issue belongs in. Only a stub under .github/workflows/ +# moves a finding away from the repo that was scanned. +# args: +route_owner() { + local org="$1" scanned="$2" path="$3" text="${4:-}" callee="" + case "$path" in + .github/workflows/*.yml|.github/workflows/*.yaml) + callee=$(stub_callee_repo "$org" "$text") ;; + esac + echo "${callee:-$scanned}" +} + +# Classify the export's HTTP status into the operator hint for the run log. +api_error_hint() { + case "$1" in + 401) echo "invalid or inactive API key" ;; + 403) echo "key type not allowed, security:read missing, or repo outside the key's scope" ;; + 410) echo "legacy organization API key: create a current non-agentic key" ;; + 413) echo "export over 50,000 records or 16 MB: run per repo with TARGET_REPO" ;; + 429) echo "rate limited" ;; + *) echo "unexpected response" ;; + esac +} + +# --------------------------------------------------------------------------- +# CodeRabbit API +# --------------------------------------------------------------------------- + +# Download the SARIF inventory to a file. Echoes nothing from the response: +# error bodies come from the export endpoint and logs here are public. +# args: +fetch_sarif() { + local out="$1" repo_id="${2:-}" status url + url="$CODERABBIT_API_URL/v1/security/scans/code?format=sarif&state=all" + [[ -n "$repo_id" ]] && url="$url&repo_id=$repo_id" + # Key on stdin, not argv, so it stays out of the process list. + if ! status=$(printf 'header = "x-coderabbitai-api-key: %s"\n' "$CODERABBIT_API_KEY" \ + | curl --silent --show-error --location --max-time 180 --retry 3 \ + --config - --output "$out" --write-out '%{http_code}' "$url"); then + warn "CodeRabbit API request failed" + return 1 + fi + if [[ "$status" != "200" ]]; then + warn "CodeRabbit API returned HTTP $status: $(api_error_hint "$status")" + return 1 + fi +} + +# --------------------------------------------------------------------------- +# Parse: the only code that reads finding content +# --------------------------------------------------------------------------- + +# Reduce the SARIF to the fields routing and tracking need, as NDJSON. Path and +# revision are for routing only and stay in the private work dir. Fails on any +# result without the expected identity, so a changed export writes nothing. +# args: +parse_sarif() { + local sarif="$1" out="$2" + jq -e '.runs | type == "array"' "$sarif" >/dev/null 2>&1 || { + warn "export is not SARIF with a runs array" + return 1 + } + jq -c '.runs[].results[]? | { + fingerprint: (.properties.findingFingerprint // ""), + finding_id: (.properties.findingId // ""), + severity: (.properties.severity // ""), + state: (.properties.state // ""), + repo_id: ((.properties.scanProvenance.scope.primary.repositoryId // "") | tostring), + revision: (.properties.scanProvenance.scope.primary.revision // ""), + path: (.locations[0].physicalLocation.artifactLocation.uri // "") + }' "$sarif" > "$out" || { + warn "export could not be parsed" + return 1 + } + + local row fp id state repo_id + local bad=0 + while IFS= read -r row; do + fp=$(jq -r '.fingerprint' <<< "$row") + id=$(jq -r '.finding_id' <<< "$row") + state=$(jq -r '.state' <<< "$row") + repo_id=$(jq -r '.repo_id' <<< "$row") + valid_fingerprint "$fp" || bad=$((bad + 1)) + valid_finding_id "$id" || bad=$((bad + 1)) + [[ "$repo_id" =~ ^[0-9]+$ ]] || bad=$((bad + 1)) + case "$state" in open|fixed|dismissed) ;; *) bad=$((bad + 1)) ;; esac + done < "$out" + if [[ "$bad" -gt 0 ]]; then + warn "export has $bad invalid identity field(s): the export format may have changed" + return 1 + fi +} + +# --------------------------------------------------------------------------- +# GitHub API +# --------------------------------------------------------------------------- + +# All org repos as NDJSON {id,name,archived}. Repos the token cannot see are not +# listed, so ORG_SCORECARD_TOKEN must cover all repositories (ci-standards.md). +list_org_repos() { + gh api --paginate "orgs/$ORG/repos?type=all&per_page=100" \ + --jq '.[] | {id: (.id | tostring), name, archived}' +} + +# Text of a workflow file at the scanned revision. +# args: +read_workflow() { + gh api "repos/$ORG/$1/contents/$2?ref=$3" --jq '.content' | base64 -d +} + +# Create labels only when missing: priority:* and security are shared with the +# other audits and the standard label set (no --force). +ensure_labels() { + local repo="$1" + local specs=( + "$CODERABBIT_AUDIT_LABEL|5c3bd6|CodeRabbit Deep Scan audit finding (idempotent marker)" + "$VERIFY_LABEL|fbca04|LLM finding not yet confirmed by a person" + "security|d93f0b|Security-related PRs and issues" + "priority:blocker|b60205|Blocker severity" + "priority:critical|d93f0b|Critical severity" + "priority:major|fbca04|Major severity" + "priority:minor|0e8a16|Minor severity" + ) + local name color desc + for spec in "${specs[@]}"; do + IFS='|' read -r name color desc <<< "$spec" + gh label create "$name" --repo "$ORG/$repo" --color "$color" \ + --description "$desc" 2>/dev/null || true + done +} + +# Open audit issues across the org, as NDJSON {fingerprint,repo,number}. Keyed +# org-wide so a finding whose route changes keeps its issue instead of gaining a +# second one. Returns 1 on any failed listing: an incomplete index could file +# duplicates. +# args: +build_issue_index() { + local names="$1" out="$2" repo list failed=0 + : > "$out" + while IFS= read -r repo; do + [[ -z "$repo" ]] && continue + if ! list=$(gh issue list --repo "$ORG/$repo" --label "$CODERABBIT_AUDIT_LABEL" --state open \ + --json number,body --limit 1000); then + warn "$repo: could not list audit issues" + failed=1 + continue + fi + jq -c --arg repo "$repo" '.[] + | ((.body // "") | try capture("coderabbit-audit:key=(?[0-9a-f]{64})").fp catch "") as $fp + | select($fp != "") + | {fingerprint: $fp, repo: $repo, number}' <<< "$list" >> "$out" + done <<< "$names" + return "$failed" +} + +# --------------------------------------------------------------------------- +# Scan: findings.json (allow-listed fields only) +# --------------------------------------------------------------------------- + +# Resolve repo names and routes for every parsed finding. Returns 1 when a +# workflow needed for routing cannot be read: a guessed route could file the +# finding in the wrong public repo. +# args: +resolve_findings() { + local parsed="$1" repos="$2" row id name archived path revision owner text sev + local tmp failed=0 + tmp=$(mktemp) + while IFS= read -r row; do + id=$(jq -r '.repo_id' <<< "$row") + name=$(jq -r --arg id "$id" 'select(.id == $id) | .name' "$repos" | head -1) + archived=$(jq -r --arg id "$id" 'select(.id == $id) | .archived' "$repos" | head -1) + if [[ -z "$name" ]] || [[ "$archived" = "true" ]]; then + UNMAPPED=$((UNMAPPED + 1)) + continue + fi + path=$(jq -r '.path' <<< "$row") + revision=$(jq -r '.revision' <<< "$row") + text="" + case "$path" in + .github/workflows/*.yml|.github/workflows/*.yaml) + if ! text=$(read_workflow "$name" "$path" "$revision"); then + warn "$name: could not read a workflow to route a finding" + failed=1 + continue + fi ;; + esac + owner=$(route_owner "$ORG" "$name" "$path" "$text") + if ! jq -e --arg owner "$owner" 'select(.name == $owner and .archived == false)' "$repos" >/dev/null; then + owner="$name" + fi + sev=$(normalize_severity "$(jq -r '.severity' <<< "$row")") + jq -c --arg scanned "$name" --arg owner "$owner" --arg sev "$sev" \ + '{fingerprint, finding_id, state, severity: $sev, scanned_repo: $scanned, owner_repo: $owner}' \ + <<< "$row" >> "$tmp" + done < "$parsed" + jq -s 'sort_by(.owner_repo, .fingerprint)' "$tmp" > "$FINDINGS_FILE" + rm -f "$tmp" + return "$failed" +} + +# --------------------------------------------------------------------------- +# Issue management +# --------------------------------------------------------------------------- + +# Public text. Takes only allow-listed fields, so it cannot leak finding detail. +# args: +render_body() { + local fp="$1" id="$2" sev="$3" scanned="$4" owner="$5" route="" + if [[ "$scanned" != "$owner" ]]; then + route=" +Found by scanning \`${ORG}/${scanned}\`. Filed here because that repo's workflow is a stub calling this repo's reusable workflow; fix it here, not in the caller." + fi + cat < Findings** for \`${ORG}/${scanned}\`. This repo is public, so keep them out of this issue, its comments, and any pull request or commit message that fixes it. + +### Priority +Labeled \`$(severity_priority_label "$sev")\` from the finding's severity (\`${sev}\`). + +### Acceptance +- [ ] A person confirms the finding against current code in CodeRabbit, then removes \`${VERIFY_LABEL}\` +- [ ] If real: fixed, so the next Deep Scan reports it as fixed. Add \`dev-lead\` only after confirming it +- [ ] If false: use **Dismiss finding** in CodeRabbit. Closing only this issue files it again next run + +--- +*Idempotent issue, updated by each audit run; closed automatically when CodeRabbit reports the finding as fixed or dismissed.* + +$(issue_marker "$fp") +BODY +} + +# args: +reconcile_open() { + local finding="$1" existing="$2" + local fp id sev scanned owner title prio body min_rank + fp=$(jq -r '.fingerprint' <<< "$finding") + id=$(jq -r '.finding_id' <<< "$finding") + sev=$(jq -r '.severity' <<< "$finding") + scanned=$(jq -r '.scanned_repo' <<< "$finding") + owner=$(jq -r '.owner_repo' <<< "$finding") + title=$(issue_title "$fp") + prio=$(severity_priority_label "$sev") + + if [[ -z "$existing" ]]; then + min_rank=$(severity_rank "$MIN_SEVERITY") + if [[ "$(severity_rank "$sev")" -lt "$min_rank" ]]; then + BELOW_THRESHOLD=$((BELOW_THRESHOLD + 1)) + return 0 + fi + fi + if [[ "$DRY_RUN" = "true" ]]; then + info "[dry-run] ${existing:-create in $owner} :: $title ($sev)" + return 0 + fi + + if [[ -n "$existing" ]]; then + local repo number other + read -r repo number <<< "$existing" + body=$(render_body "$fp" "$id" "$sev" "$scanned" "$repo") + # Only the body and priority change: needs-verification and dev-lead are a + # person's decisions, so a re-run never restores or adds them. + if ! gh issue edit "$number" --repo "$ORG/$repo" --body "$body" --add-label "$prio" >/dev/null; then + warn "$repo: could not update #$number" + return 1 + fi + for other in priority:blocker priority:critical priority:major priority:minor; do + [[ "$other" != "$prio" ]] && gh issue edit "$number" --repo "$ORG/$repo" \ + --remove-label "$other" >/dev/null 2>&1 || true + done + ISSUES_EXISTING=$((ISSUES_EXISTING + 1)) + info "Updated #$number in $repo :: $title" + return 0 + fi + + ensure_labels "$owner" + body=$(render_body "$fp" "$id" "$sev" "$scanned" "$owner") + local url + if ! url=$(gh issue create --repo "$ORG/$owner" --title "$title" \ + --label "$CODERABBIT_AUDIT_LABEL,$VERIFY_LABEL,security,$prio" --body "$body"); then + warn "$owner: could not create an issue :: $title" + return 1 + fi + ISSUES_ADDED=$((ISSUES_ADDED + 1)) + info "Created $url" +} + +# args: +reconcile_closed() { + local state="$1" existing="$2" repo number + read -r repo number <<< "$existing" + if [[ "$DRY_RUN" = "true" ]]; then + info "[dry-run] close #$number in $repo ($state)" + return 0 + fi + if ! gh issue close "$number" --repo "$ORG/$repo" \ + --comment "CodeRabbit reports this finding as ${state} as of $(date -u +%Y-%m-%d). Closing automatically." >/dev/null; then + warn "$repo: could not close #$number" + return 1 + fi + ISSUES_REMOVED=$((ISSUES_REMOVED + 1)) + info "Closed #$number in $repo ($state)" +} + +# args: +reconcile_all() { + local index="$1" finding fp state existing failed=0 + while IFS= read -r finding; do + fp=$(jq -r '.fingerprint' <<< "$finding") + state=$(jq -r '.state' <<< "$finding") + existing=$(jq -r --arg fp "$fp" 'select(.fingerprint == $fp) | "\(.repo) \(.number)"' "$index" | head -1) + if [[ "$state" = "open" ]]; then + reconcile_open "$finding" "$existing" || failed=1 + elif [[ -n "$existing" ]]; then + reconcile_closed "$state" "$existing" || failed=1 + fi + done < <(jq -c '.[]' "$FINDINGS_FILE") + + # Absent from the export: left open and counted. A partial scan omits findings + # it never reached. A sustained count here would mean fingerprints are not + # stable across scans, so it is surfaced in the summary. + if [[ -z "$TARGET_REPO" ]]; then + NOT_IN_EXPORT=$(jq -r '.fingerprint' "$index" | sort -u \ + | grep -cvxFf <(jq -r '.[].fingerprint' "$FINDINGS_FILE"; echo "-") || true) + fi + return "$failed" +} + +# --------------------------------------------------------------------------- +# Summary (public: counts only) +# --------------------------------------------------------------------------- +generate_summary() { + local total=0 open=0 + if [[ -f "$FINDINGS_FILE" ]]; then + total=$(jq 'length' "$FINDINGS_FILE") + open=$(jq '[.[] | select(.state == "open")] | length' "$FINDINGS_FILE") + fi + { + echo "# CodeRabbit Audit — $(date -u +%Y-%m-%d)" + echo "" + echo "**Org:** \`${ORG}\` · **Findings read:** ${total} · **Open:** ${open} · **Minimum severity to file:** ${MIN_SEVERITY}" + if [[ -n "$RUN_ERROR" ]]; then + echo "" + echo "> [!WARNING]" + echo "> ${RUN_ERROR} Issues were left untouched. Check the workflow logs." + fi + echo "" + [[ "$DRY_RUN" = "true" ]] && { echo "Dry run: no issues written."; echo ""; } + echo "## Open findings by severity" + echo "| Severity | Findings |" + echo "|---|---:|" + if [[ -f "$FINDINGS_FILE" ]]; then + jq -r '[.[] | select(.state == "open")] | group_by(.severity) | .[] | "| \(.[0].severity) | \(length) |"' "$FINDINGS_FILE" + fi + echo "" + echo "## Issue actions" + echo "| Action | Count |" + echo "|---|---:|" + echo "| Added | $ISSUES_ADDED |" + echo "| Updated | $ISSUES_EXISTING |" + echo "| Closed (fixed or dismissed in CodeRabbit) | $ISSUES_REMOVED |" + echo "| Not filed (below minimum severity) | $BELOW_THRESHOLD |" + echo "| Left open (finding absent from this export) | $NOT_IN_EXPORT |" + echo "| Skipped (repo archived or outside this org) | $UNMAPPED |" + echo "" + echo "---" + echo "*Generated by the CodeRabbit Audit workflow. Finding detail is only in CodeRabbit.*" + } > "$SUMMARY_FILE" +} + +# --------------------------------------------------------------------------- +fail_run() { + RUN_ERROR="$1" + generate_summary + warn "$1" + return 1 +} + +main() { + mkdir -p "$REPORT_DIR" + info "CodeRabbit audit — org=$ORG dry_run=$DRY_RUN target=${TARGET_REPO:-all} min_severity=$MIN_SEVERITY" + if [[ -z "${CODERABBIT_API_KEY:-}" ]]; then + fail_run "CODERABBIT_API_KEY is not set." + return 1 + fi + + # Raw SARIF and routing paths stay here, outside REPORT_DIR. + local work + work=$(mktemp -d) + # shellcheck disable=SC2064 # expand now: $work is local to main + trap "rm -rf '$work'" EXIT + + local repos="$work/repos.ndjson" + list_org_repos > "$repos" || { fail_run "Could not list repos in $ORG."; return 1; } + [[ -s "$repos" ]] || { fail_run "The token lists no repos in $ORG."; return 1; } + + local target_id="" + if [[ -n "$TARGET_REPO" ]]; then + target_id=$(jq -r --arg n "${TARGET_REPO#"$ORG/"}" 'select(.name == $n) | .id' "$repos") + [[ -n "$target_id" ]] || { fail_run "TARGET_REPO is not a repo in $ORG."; return 1; } + fi + + fetch_sarif "$work/export.sarif" "$target_id" || { fail_run "The CodeRabbit export could not be read."; return 1; } + parse_sarif "$work/export.sarif" "$work/parsed.ndjson" || { fail_run "The CodeRabbit export was not in the expected format."; return 1; } + rm -f "$work/export.sarif" + + resolve_findings "$work/parsed.ndjson" "$repos" || { fail_run "A workflow needed for routing could not be read."; return 1; } + + local names + names=$(jq -r 'select(.archived == false) | .name' "$repos") + build_issue_index "$names" "$work/index.ndjson" || { fail_run "Audit issues could not be listed, so duplicates could not be ruled out."; return 1; } + + local issues_failed=0 + reconcile_all "$work/index.ndjson" || issues_failed=1 + + jq -n --argjson added "$ISSUES_ADDED" --argjson existing "$ISSUES_EXISTING" --argjson removed "$ISSUES_REMOVED" \ + '{added:$added,existing:$existing,removed:$removed}' > "$COUNTS_FILE" + generate_summary + info "Done. findings=$(jq length "$FINDINGS_FILE") added=$ISSUES_ADDED updated=$ISSUES_EXISTING closed=$ISSUES_REMOVED" + + if [[ "$issues_failed" = "1" ]]; then + warn "one or more audit issues could not be created, updated or closed — see the log" + return 1 + fi +} + +# Guard: only run when executed directly (sourcing defines functions for bats). +if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then + main "$@" +fi diff --git a/standards/ci-standards.md b/standards/ci-standards.md index dedd2ca51..04ba71687 100644 --- a/standards/ci-standards.md +++ b/standards/ci-standards.md @@ -2125,6 +2125,67 @@ org-level workflows that run across all repositories: - **Code and tests:** `scripts/codeql-audit.sh`; pure helpers gated by `test/scripts/codeql-audit/` via `codeql-audit-tests.yml`. +### CodeRabbit Audit (`coderabbit-audit.yml`) + +- **Schedule:** Weekly, Thursday 17:01 UTC — between the Wednesday CodeQL audit + and the Friday compliance audit. Minute from `cron_offset_minute`. +- **Purpose:** Gives CodeRabbit Deep Scan findings an owner. They reach neither + the SonarCloud nor the CodeQL audit otherwise. +- **Source:** `GET https://api.coderabbit.ai/v1/security/scans/code?format=sarif&state=all` + ([API reference](https://docs.coderabbit.ai/api-reference/security-scans-code-list)). + SARIF because the paginated JSON format has no `findingFingerprint`; `state=all` + because closing an issue requires an explicit `fixed` or `dismissed`. The API + only reads results. Deep Scans run on the schedule set per repo in CodeRabbit + (**Security > Repositories**), are usage-billed, and should finish before Thursday. +- **One issue per finding**, titled `CodeRabbit Deep Scan: finding ` + and keyed org-wide by a hidden `` + body marker. Labels: `coderabbit-audit` (marker), `needs-verification`, `security`, + and `priority:`. Every severity is filed by default; findings below `MIN_SEVERITY` + (default `low`) would be counted in the summary instead. +- **No finding detail in public output.** Every org repo is public, so issues, + workflow logs, the step summary and the artifact are public, and these findings + can describe exploit chains. Only the fingerprint, finding id, severity, state and + repo names are written. Description, category, CWE, subsystem, path, line and call + stack stay in CodeRabbit (**Security > Findings**), and the artifact is the + counts-only summary. +- **Never `dev-lead`.** Findings are unverified LLM output, and the API exposes no + verification result to gate on. A person confirms the finding, removes + `needs-verification`, and adds `dev-lead` if an agent should fix it. Re-runs + change neither label. Fixes must keep exploit detail out of the PR and commit + message too. +- **Routing:** an issue is filed in the repo that owns the code. A finding located + in a caller stub (under `.github/workflows/`, no `steps:`, calling one org repo's + reusable workflow) goes to that repo, because a stub has no code of its own. All + other findings stay with the scanned repo, including a stub calling two org repos. + The stub is read at the scanned revision. Call-stack frames are not used: + CodeRabbit guarantees neither their order nor their format. An issue that already + exists stays where it is if the route later changes. +- **Priority** uses the shared scale of the other audits: + + | Deep Scan severity | Label | + |---|---| + | `critical` | `priority:blocker` | + | `high` | `priority:critical` | + | `medium`, or any unrecognized value | `priority:major` | + | `low` | `priority:minor` | + +- **Closing:** an issue closes only when the export reports its finding as `fixed` or + `dismissed`. A finding absent from the export leaves its issue open and is counted + in the summary, because a partial-coverage scan omits findings it did not reach. To + reject a finding, use **Dismiss finding** in CodeRabbit: an issue closed only on + GitHub is filed again on the next run. +- **Fail closed:** a failed read, an export without the expected fingerprint, finding + id, state or repo id, an unreadable stub, or an unlistable repo fails the run with + no issue writes. API error bodies are never logged. +- **Dispatch inputs:** `target_repo` (one repo) and `dry_run` (read and report, no + issue writes). +- **Tokens:** `CODERABBIT_API_KEY` is a repository secret on `combsmith/.github`, a + non-agentic CodeRabbit API key with the `security:read` permission. + `ORG_SCORECARD_TOKEN` needs no permissions beyond those + [OpenSSF Scorecard](#openssf-scorecard-org-scorecardyml) already requires. +- **Code and tests:** `scripts/coderabbit-audit.sh`; helpers and stubbed end-to-end + runs gated by `test/scripts/coderabbit-audit/` via `coderabbit-audit-tests.yml`. + --- ## CI Auto-Fix Pattern diff --git a/standards/github-settings.md b/standards/github-settings.md index e6068d180..29d8b3f2a 100644 --- a/standards/github-settings.md +++ b/standards/github-settings.md @@ -747,6 +747,7 @@ The org runs several automated workflows across all repositories: | **Actions Fleet Monitor** (`actions-fleet-monitor.yml` in `combsmith/.github-private`) | Daily, 6:00 UTC | Monitor health of all GitHub Actions workflows across the org | Tracks success/failure rates, duration percentiles (p50/p95), and assigns status (HEALTHY/WARNING/DEGRADED/CRITICAL) per workflow; creates issues when workflows have failures | | **Org Scorecard Review** (`org-scorecard.yml`) | Weekly, Monday 9:00 UTC | Security posture scoring for all public repos via OpenSSF Scorecard | Creates/updates/closes GitHub Issues with findings; auto-closes when resolved | | **CodeQL Audit** (`codeql-audit.yml`) | Weekly, Wednesday 17:02 UTC | Turns open CodeQL code-scanning alerts into one issue per repo × language family | Labeled `codeql-audit` + `dev-lead` + `priority:`; auto-closes when resolved; skips alerts the compliance audit already files. See [ci-standards.md](ci-standards.md#codeql-audit-codeql-audityml) | +| **CodeRabbit Audit** (`coderabbit-audit.yml`) | Weekly, Thursday 17:01 UTC | Turns CodeRabbit Deep Scan findings into one issue per finding, in the repo that owns the code | Labeled `coderabbit-audit` + `needs-verification` + `priority:`, never `dev-lead`; no finding detail in public output; closes only when CodeRabbit reports fixed or dismissed. See [ci-standards.md](ci-standards.md#coderabbit-audit-coderabbit-audityml) | | **Org Standards Compliance Audit** (`compliance-audit-and-improvement.yml`) | Weekly, Friday 12:00 UTC | Deterministic audit of all repos against org standards + runtime health survey | Identifies missing workflows, misconfigured settings, stale PRs, security alerts; creates actionable issues labeled `dev-lead` for agent remediation | | **Daily Org Status** (`daily-org-status.yml`) | Daily, 6:00 UTC | Org-wide health snapshot — PR counts, CI failures, dependency vulnerabilities | Reports via PR comments and workflow summary | | **Dependency Audit** (`dependency-audit.yml`) | Per-repo CI (push/PR to `main`) | Multi-ecosystem dependency vulnerability scan (npm, pnpm, go, cargo, pip) | Fails the build when dependencies have known security advisories; adopted per-repo via the standard caller stub | diff --git a/test/scripts/coderabbit-audit/audit-run.bats b/test/scripts/coderabbit-audit/audit-run.bats new file mode 100644 index 000000000..2f47559ec --- /dev/null +++ b/test/scripts/coderabbit-audit/audit-run.bats @@ -0,0 +1,294 @@ +#!/usr/bin/env bats +# End-to-end runs of scripts/coderabbit-audit.sh against stubbed gh and curl. +# These pin the outcomes that matter: no finding detail reaches public output, +# unverified findings never get dev-lead, routing, and fail-closed lifecycle. +# Every finding here is synthetic. + +bats_require_minimum_version 1.5.0 + +ROOT="$(cd "$BATS_TEST_DIRNAME/../../.." && pwd)" +SCRIPT="$ROOT/scripts/coderabbit-audit.sh" +CANARY="CANARYSECRET" + +fp() { printf "$1%.0s" {1..64}; } + +# args: +finding() { + jq -n --arg fp "$(fp "$1")" --arg repo "$2" --arg path "$3" --arg sev "$4" --arg state "$5" \ + --arg c "$CANARY" '{ + guid: "00000000-0000-0000-0000-000000000000", + ruleId: ($c + "-rule"), + level: "warning", + message: {text: ($c + " exploit chain")}, + locations: [{physicalLocation: {artifactLocation: {uri: $path}, region: {startLine: 4242}}}], + properties: { + findingFingerprint: $fp, + findingId: "11111111-2222-3333-4444-555555555555", + severity: $sev, state: $state, source: "LLM", + cwe: ($c + "-cwe"), category: ($c + "-category"), subsystemName: ($c + "-subsystem"), + reachabilityCallStack: [($c + "-frame")], + scanProvenance: {scope: {primary: {repositoryId: $repo, revision: "abc123"}}} + }}' +} + +sarif_of() { jq -s '{version: "2.1.0", runs: [{tool: {driver: {name: "CodeRabbit"}}, results: .}]}'; } + +setup() { + STUB_DIR="$(mktemp -d)" + REPORT_DIR="$(mktemp -d)" + STUB_LOG="$STUB_DIR/calls.log" + export STUB_DIR REPORT_DIR STUB_LOG + export PATH="$BATS_TEST_DIRNAME/stubs:$PATH" + export CODERABBIT_API_KEY="test-key-value" ORG="combsmith" + : > "$STUB_LOG" + mkdir -p "$STUB_DIR/workflows" "$STUB_DIR/issues" + + cat > "$STUB_DIR/repos.ndjson" <<'JSON' +{"id":"101","name":"alpha","archived":false} +{"id":"102","name":"bravo","archived":false} +{"id":"103","name":".github","archived":false} +{"id":"104","name":"old","archived":true} +JSON + printf 'jobs:\n ci:\n uses: combsmith/.github/.github/workflows/ci-reusable.yml@v1\n' \ + > "$STUB_DIR/workflows/stub.yml" + printf 'jobs:\n b:\n runs-on: ubuntu-latest\n steps:\n - run: make\n' \ + > "$STUB_DIR/workflows/local.yml" + + { + finding a 101 "src/$CANARY-path.ts" medium open # new issue in alpha + finding b 101 src/low.ts low open # filed by default + finding c 102 .github/workflows/stub.yml high open # routed to .github + finding d 102 .github/workflows/local.yml high open # stays in bravo + finding e 101 src/gone.ts medium dismissed # closes #5 + finding f 101 src/done.ts medium fixed # no issue: nothing to do + finding 7 101 src/kept.ts high open # updates #7 + finding 9 999 src/foreign.ts high open # not an org repo + finding 0 104 src/archived.ts high open # archived repo + } | sarif_of > "$STUB_DIR/export.sarif" + + # fingerprint 8 is open in GitHub but absent from the export. + jq -n --arg e "$(fp e)" --arg g "$(fp 7)" --arg h "$(fp 8)" '[ + {number: 5, body: ("x ")}, + {number: 7, body: ("x ")}, + {number: 9, body: ("x ")}, + {number: 11, body: "an issue with the label but no marker"} + ]' > "$STUB_DIR/issues/alpha.json" +} + +teardown() { + rm -rf "$STUB_DIR" "$REPORT_DIR" +} + +writes() { grep -E '^gh issue (create|edit|close)' "$STUB_LOG" || true; } + +# ── public output ────────────────────────────────────────────────────────── +@test "no finding detail reaches logs, issue text or the report dir" { + run --separate-stderr bash "$SCRIPT" + [ "$status" -eq 0 ] + ! grep -q "$CANARY" <<< "$output" || false + ! grep -q "$CANARY" <<< "$stderr" || false + ! grep -q "$CANARY" "$STUB_LOG" || false + ! grep -rq "$CANARY" "$REPORT_DIR" || false + ! grep -q "4242" "$STUB_LOG" || false +} +@test "routing paths stay out of logs and issue text" { + run --separate-stderr bash "$SCRIPT" + ! grep -q "stub.yml" <<< "$stderr" || false + ! grep -E '^gh issue' "$STUB_LOG" | grep -q "stub.yml" || false + ! grep -rq "stub.yml" "$REPORT_DIR" || false +} +@test "the workflow uploads only the summary and pins its cron to the offset" { + wf="$ROOT/.github/workflows/coderabbit-audit.yml" + [ "$(grep -c 'path: \${{ runner.temp }}/coderabbit-report/summary.md' "$wf")" -eq 1 ] + [ "$(grep -cE '^[[:space:]]+path:' "$wf")" -eq 1 ] + source "$ROOT/scripts/lib/cron-timing.sh" + grep -q "cron: '$(cron_offset_minute coderabbit-audit.yml) 17 " "$wf" +} +@test "the API key travels on stdin, never in argv or logs" { + run --separate-stderr bash "$SCRIPT" + ! grep -q "test-key-value" "$STUB_LOG" || false + ! grep -q "test-key-value" <<< "$stderr" || false + grep -q 'x-coderabbitai-api-key: test-key-value' "$STUB_DIR/curl-stdin" +} +@test "the export is requested as SARIF with every state" { + run bash "$SCRIPT" + grep -E '^curl ' "$STUB_LOG" | grep -q 'format=sarif' + grep -E '^curl ' "$STUB_LOG" | grep -q 'state=all' +} + +# ── verification gate ────────────────────────────────────────────────────── +@test "no write ever adds dev-lead" { + run bash "$SCRIPT" + [ "$status" -eq 0 ] + writes | grep -q 'issue create' + ! grep -E '^gh issue (create|edit)' "$STUB_LOG" | grep -qE -- '--(add-)?label [^ ]*dev-lead' || false +} +@test "new issues carry needs-verification, security and the priority" { + run bash "$SCRIPT" + grep -E "^gh issue create .*$(fp a | cut -c1-12)" "$STUB_LOG" \ + | grep -qE -- '--label coderabbit-audit.{1,2}needs-verification.{1,2}security.{1,2}priority:major ' +} +@test "an update does not restore needs-verification after a person removed it" { + run bash "$SCRIPT" + grep -E '^gh issue edit 7 ' "$STUB_LOG" | grep -q -- '--add-label priority:critical' + ! grep -E '^gh issue edit 7 ' "$STUB_LOG" | grep -qE -- '--add-label [^ ]*needs-verification' || false +} + +# ── routing ──────────────────────────────────────────────────────────────── +@test "a finding in a caller stub is filed in the repo that owns the workflow" { + run bash "$SCRIPT" + line=$(grep -E "^gh issue create .*$(fp c | cut -c1-12)" "$STUB_LOG") + grep -q -- '--repo combsmith/.github ' <<< "$line" + grep -q 'combsmith/bravo' <<< "$line" +} +@test "a finding in a workflow with its own steps stays in the scanned repo" { + run bash "$SCRIPT" + grep -E "^gh issue create .*$(fp d | cut -c1-12)" "$STUB_LOG" | grep -q -- '--repo combsmith/bravo ' +} +@test "routing reads the workflow at the scanned revision" { + run bash "$SCRIPT" + grep -qF 'stub.yml\?ref=abc123' "$STUB_LOG" +} +@test "an existing issue stays where it is when the route changes" { + jq -n --arg c "$(fp c)" '[{number: 3, body: ("")}]' \ + > "$STUB_DIR/issues/bravo.json" + run bash "$SCRIPT" + grep -E '^gh issue edit 3 ' "$STUB_LOG" | grep -q -- '--repo combsmith/bravo ' + ! grep -E "^gh issue create .*$(fp c | cut -c1-12)" "$STUB_LOG" || false +} + +# ── lifecycle ────────────────────────────────────────────────────────────── +@test "a dismissed finding closes its issue" { + run bash "$SCRIPT" + grep -E '^gh issue close 5 ' "$STUB_LOG" | grep -q 'dismissed' +} +@test "a finding absent from the export never closes its issue, and is counted" { + run bash "$SCRIPT" + ! grep -qE '^gh issue close 9 ' "$STUB_LOG" || false + grep -q 'Left open (finding absent from this export) | 1 |' "$REPORT_DIR/summary.md" +} +@test "an issue without a marker is left alone" { + run bash "$SCRIPT" + ! grep -qE '^gh issue (close|edit) 11 ' "$STUB_LOG" || false +} +@test "a finding that already has an issue is updated, not filed again" { + run bash "$SCRIPT" + grep -qE '^gh issue edit 7 ' "$STUB_LOG" + ! grep -E "^gh issue create .*$(fp 7 | cut -c1-12)" "$STUB_LOG" || false +} +@test "a fixed finding with no issue writes nothing" { + run bash "$SCRIPT" + ! grep -E '^gh issue' "$STUB_LOG" | grep -q "$(fp f | cut -c1-12)" || false +} +@test "every severity is filed by default" { + run bash "$SCRIPT" + grep -qE "^gh issue create .*$(fp b | cut -c1-12)" "$STUB_LOG" + grep -q 'Not filed (below minimum severity) | 0 |' "$REPORT_DIR/summary.md" +} +@test "MIN_SEVERITY=medium counts low findings instead of filing them" { + MIN_SEVERITY=medium run bash "$SCRIPT" + ! grep -E "^gh issue create .*$(fp b | cut -c1-12)" "$STUB_LOG" || false + grep -q 'Not filed (below minimum severity) | 1 |' "$REPORT_DIR/summary.md" +} +@test "findings in archived or foreign repos are skipped and counted" { + run bash "$SCRIPT" + ! grep -E "^gh issue create " "$STUB_LOG" | grep -qE "$(fp 9 | cut -c1-12)|$(fp 0 | cut -c1-12)" || false + grep -q 'Skipped (repo archived or outside this org) | 2 |' "$REPORT_DIR/summary.md" +} +@test "a second run with the first run's issues writes no new issues" { + jq -n --arg a "$(fp a)" --arg b "$(fp b)" --arg g "$(fp 7)" '[ + {number: 1, body: ("")}, + {number: 4, body: ("")}, + {number: 7, body: ("")}]' > "$STUB_DIR/issues/alpha.json" + jq -n --arg c "$(fp c)" '[{number: 2, body: ("")}]' > "$STUB_DIR/issues/.github.json" + jq -n --arg d "$(fp d)" '[{number: 3, body: ("")}]' > "$STUB_DIR/issues/bravo.json" + run bash "$SCRIPT" + [ "$status" -eq 0 ] + ! grep -qE '^gh issue create' "$STUB_LOG" || false +} + +# ── fail closed ──────────────────────────────────────────────────────────── +assert_no_writes_and_red() { + [ "$status" -ne 0 ] + [ -z "$(writes)" ] + grep -q 'Issues were left untouched' "$REPORT_DIR/summary.md" +} + +@test "a missing API key fails and writes nothing" { + unset CODERABBIT_API_KEY + run bash "$SCRIPT" + assert_no_writes_and_red + ! grep -q '^curl' "$STUB_LOG" || false +} +@test "an API error fails, writes nothing, and logs no response body" { + echo "$CANARY error body" > "$STUB_DIR/error.body" + STUB_HTTP_STATUS=403 STUB_SARIF="$STUB_DIR/error.body" run --separate-stderr bash "$SCRIPT" + assert_no_writes_and_red + grep -q 'HTTP 403' <<< "$stderr" + ! grep -q "$CANARY" <<< "$stderr" || false +} +@test "a network failure fails and writes nothing" { + STUB_CURL_FAIL=1 run bash "$SCRIPT" + assert_no_writes_and_red +} +@test "a body that is not SARIF fails and writes nothing" { + echo '{"data":[]}' > "$STUB_DIR/not-sarif" + STUB_SARIF="$STUB_DIR/not-sarif" run bash "$SCRIPT" + assert_no_writes_and_red +} +@test "a result with no fingerprint fails and writes nothing" { + { finding a 101 src/a.ts medium open | jq 'del(.properties.findingFingerprint)'; } | sarif_of > "$STUB_DIR/bad" + STUB_SARIF="$STUB_DIR/bad" run bash "$SCRIPT" + assert_no_writes_and_red +} +@test "an unknown state fails and writes nothing" { + finding a 101 src/a.ts medium triaged | sarif_of > "$STUB_DIR/bad" + STUB_SARIF="$STUB_DIR/bad" run bash "$SCRIPT" + assert_no_writes_and_red +} +@test "a result with no scanned repo fails and writes nothing" { + { finding a 101 src/a.ts medium open | jq 'del(.properties.scanProvenance)'; } | sarif_of > "$STUB_DIR/bad" + STUB_SARIF="$STUB_DIR/bad" run bash "$SCRIPT" + assert_no_writes_and_red +} +@test "an unreadable workflow fails and writes nothing rather than guess a route" { + STUB_FAIL_CONTENTS=1 run bash "$SCRIPT" + assert_no_writes_and_red +} +@test "an unlistable repo fails and writes nothing rather than risk duplicates" { + STUB_FAIL_ISSUE_LIST=1 run bash "$SCRIPT" + assert_no_writes_and_red +} +@test "an unreadable repo list fails and writes nothing" { + STUB_FAIL_REPOS=1 run bash "$SCRIPT" + assert_no_writes_and_red +} +@test "a failed issue create turns the run red but still writes the summary" { + STUB_FAIL_ISSUE_CREATE=1 run bash "$SCRIPT" + [ "$status" -ne 0 ] + [ -f "$REPORT_DIR/summary.md" ] +} +@test "an empty export closes nothing" { + sarif_of < /dev/null > "$STUB_DIR/empty" + STUB_SARIF="$STUB_DIR/empty" run bash "$SCRIPT" + [ "$status" -eq 0 ] + [ -z "$(writes)" ] +} + +# ── dispatch inputs ──────────────────────────────────────────────────────── +@test "dry run reads everything and writes nothing" { + DRY_RUN=true run bash "$SCRIPT" + [ "$status" -eq 0 ] + [ -z "$(writes)" ] + ! grep -q '^gh label' "$STUB_LOG" || false + grep -q 'Dry run: no issues written' "$REPORT_DIR/summary.md" +} +@test "TARGET_REPO narrows the export to that repo's id" { + TARGET_REPO=combsmith/bravo run bash "$SCRIPT" + grep -E '^curl ' "$STUB_LOG" | grep -q 'repo_id=102' + grep -q 'absent from this export) | 0 |' "$REPORT_DIR/summary.md" +} +@test "an unknown TARGET_REPO fails and writes nothing" { + TARGET_REPO=nope run bash "$SCRIPT" + assert_no_writes_and_red +} diff --git a/test/scripts/coderabbit-audit/pure-helpers.bats b/test/scripts/coderabbit-audit/pure-helpers.bats new file mode 100644 index 000000000..a34908d84 --- /dev/null +++ b/test/scripts/coderabbit-audit/pure-helpers.bats @@ -0,0 +1,126 @@ +#!/usr/bin/env bats +# Unit tests for the pure helpers in scripts/coderabbit-audit.sh: +# normalize_severity, severity_rank, severity_priority_label, +# valid_fingerprint, valid_finding_id, issue_marker, issue_title, +# stub_callee_repo, route_owner, api_error_hint. +# +# The script's `main` is guarded, so sourcing only defines functions — the real +# helpers are exercised directly in an isolated subshell. + +bats_require_minimum_version 1.5.0 + +SCRIPT="$(cd "$BATS_TEST_DIRNAME/../../.." && pwd)/scripts/coderabbit-audit.sh" +FP="$(printf 'a%.0s' {1..64})" + +call() { + local func="$1" + shift + run bash -c 'source "$1" >/dev/null 2>&1; shift; "$@"' _ "$SCRIPT" "$func" "$@" +} + +# ── normalize_severity ───────────────────────────────────────────────────── +@test "known severities pass through lowercased" { + call normalize_severity "HIGH"; [ "$output" = "high" ] + call normalize_severity "critical"; [ "$output" = "critical" ] + call normalize_severity "low"; [ "$output" = "low" ] +} +@test "free text from the export never survives as a severity" { + call normalize_severity "high; see src/auth.ts"; [ "$output" = "unknown" ] + call normalize_severity ""; [ "$output" = "unknown" ] +} + +# ── severity_rank / severity_priority_label ──────────────────────────────── +@test "rank orders critical > high > medium > low" { + call severity_rank critical; [ "$output" = "4" ] + call severity_rank high; [ "$output" = "3" ] + call severity_rank medium; [ "$output" = "2" ] + call severity_rank low; [ "$output" = "1" ] +} +@test "an unknown severity ranks as medium, so it is filed rather than hidden" { + call severity_rank unknown; [ "$output" = "2" ] + call severity_priority_label unknown; [ "$output" = "priority:major" ] +} +@test "priority labels match the scale the CodeQL audit uses" { + call severity_priority_label critical; [ "$output" = "priority:blocker" ] + call severity_priority_label high; [ "$output" = "priority:critical" ] + call severity_priority_label medium; [ "$output" = "priority:major" ] + call severity_priority_label low; [ "$output" = "priority:minor" ] +} + +# ── valid_fingerprint / valid_finding_id ─────────────────────────────────── +@test "a 64-char lowercase hex fingerprint is valid" { + call valid_fingerprint "$FP"; [ "$status" -eq 0 ] +} +@test "other fingerprint shapes are rejected" { + call valid_fingerprint ""; [ "$status" -ne 0 ] + call valid_fingerprint "abc123"; [ "$status" -ne 0 ] + call valid_fingerprint "${FP:0:63}Z"; [ "$status" -ne 0 ] + call valid_fingerprint "${FP} injected"; [ "$status" -ne 0 ] +} +@test "only a UUID is accepted as a finding id" { + call valid_finding_id "11111111-2222-3333-4444-555555555555"; [ "$status" -eq 0 ] + call valid_finding_id "not a uuid"; [ "$status" -ne 0 ] + call valid_finding_id ""; [ "$status" -ne 0 ] +} + +# ── issue_marker / issue_title ───────────────────────────────────────────── +@test "the marker holds the fingerprint in an HTML comment" { + call issue_marker "$FP" + [[ "$output" == "" ]] +} +@test "the title holds a short fingerprint and no severity" { + call issue_title "$FP" + [ "$output" = "CodeRabbit Deep Scan: finding aaaaaaaaaaaa" ] +} + +# ── stub_callee_repo ─────────────────────────────────────────────────────── +@test "a stub calling one org reusable workflow yields that repo" { + text=$'jobs:\n ci:\n uses: combsmith/.github/.github/workflows/ci-reusable.yml@v1\n' + call stub_callee_repo combsmith "$text"; [ "$output" = ".github" ] +} +@test "a quoted ref and a stub in .github-private both resolve" { + text=$'jobs:\n ci:\n uses: "combsmith/.github-private/.github/workflows/x.yml@abc"\n' + call stub_callee_repo combsmith "$text"; [ "$output" = ".github-private" ] +} +@test "a workflow with steps is not a stub, even when it calls a reusable workflow" { + text=$'jobs:\n a:\n uses: combsmith/.github/.github/workflows/x.yml@v1\n b:\n runs-on: ubuntu-latest\n steps:\n - run: make\n' + call stub_callee_repo combsmith "$text"; [ -z "$output" ] +} +@test "a commented-out uses line does not make a stub" { + text=$'jobs:\n a:\n # uses: combsmith/.github/.github/workflows/x.yml@v1\n uses: other/repo/.github/workflows/x.yml@v1\n' + call stub_callee_repo combsmith "$text"; [ -z "$output" ] +} +@test "a reusable workflow from another org is not routed" { + text=$'jobs:\n a:\n uses: other/repo/.github/workflows/x.yml@v1\n' + call stub_callee_repo combsmith "$text"; [ -z "$output" ] +} +@test "a stub calling two org repos is ambiguous and yields nothing" { + text=$'jobs:\n a:\n uses: combsmith/.github/.github/workflows/x.yml@v1\n b:\n uses: combsmith/.github-private/.github/workflows/y.yml@v1\n' + call stub_callee_repo combsmith "$text"; [ -z "$output" ] +} + +# ── route_owner ──────────────────────────────────────────────────────────── +@test "a finding in a stub routes to the callee repo" { + text=$'jobs:\n a:\n uses: combsmith/.github/.github/workflows/x.yml@v1\n' + call route_owner combsmith TalkTerm .github/workflows/x.yml "$text" + [ "$output" = ".github" ] +} +@test "a finding outside .github/workflows stays with the scanned repo" { + text=$'jobs:\n a:\n uses: combsmith/.github/.github/workflows/x.yml@v1\n' + call route_owner combsmith TalkTerm src/app.ts "$text" + [ "$output" = "TalkTerm" ] +} +@test "a finding in a workflow with its own steps stays with the scanned repo" { + text=$'jobs:\n b:\n steps:\n - run: make\n' + call route_owner combsmith TalkTerm .github/workflows/x.yml "$text" + [ "$output" = "TalkTerm" ] +} + +# ── api_error_hint ───────────────────────────────────────────────────────── +@test "documented API errors get an operator hint" { + call api_error_hint 401; [[ "$output" == *"API key"* ]] + call api_error_hint 403; [[ "$output" == *"security:read"* ]] + call api_error_hint 410; [[ "$output" == *"legacy"* ]] + call api_error_hint 413; [[ "$output" == *"TARGET_REPO"* ]] + call api_error_hint 500; [ "$output" = "unexpected response" ] +} diff --git a/test/scripts/coderabbit-audit/stubs/curl b/test/scripts/coderabbit-audit/stubs/curl new file mode 100755 index 000000000..4f3e7f735 --- /dev/null +++ b/test/scripts/coderabbit-audit/stubs/curl @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +# Fake `curl` for the CodeRabbit audit tests. Logs argv and stdin separately so +# tests can show the API key travels on stdin only. +printf '%q ' curl "$@" >> "$STUB_LOG"; echo >> "$STUB_LOG" +cat > "$STUB_DIR/curl-stdin" + +[[ -n "${STUB_CURL_FAIL:-}" ]] && { echo "curl: (7) connection failed" >&2; exit 7; } +out="" +while [[ $# -gt 0 ]]; do + [[ "$1" = "--output" ]] && out="$2" + shift +done +cp "${STUB_SARIF:-$STUB_DIR/export.sarif}" "$out" +printf '%s' "${STUB_HTTP_STATUS:-200}" diff --git a/test/scripts/coderabbit-audit/stubs/gh b/test/scripts/coderabbit-audit/stubs/gh new file mode 100755 index 000000000..5f69ea549 --- /dev/null +++ b/test/scripts/coderabbit-audit/stubs/gh @@ -0,0 +1,25 @@ +#!/usr/bin/env bash +# Fake `gh` for the CodeRabbit audit tests. Logs each call to $STUB_LOG as one +# shell-quoted line and answers from the files setup() writes under $STUB_DIR. +printf '%q ' gh "$@" >> "$STUB_LOG"; echo >> "$STUB_LOG" + +case "$1 $2" in + "api --paginate") + [[ -n "${STUB_FAIL_REPOS:-}" ]] && { echo "gh: forbidden" >&2; exit 1; } + cat "$STUB_DIR/repos.ndjson" ;; + api\ repos/*) + [[ -n "${STUB_FAIL_CONTENTS:-}" ]] && { echo "gh: Not Found (HTTP 404)" >&2; exit 1; } + path="${2%%\?*}" + base64 < "$STUB_DIR/workflows/$(basename "$path")" ;; + "issue list") + [[ -n "${STUB_FAIL_ISSUE_LIST:-}" ]] && { echo "gh: forbidden" >&2; exit 1; } + repo="${4#*/}" + cat "$STUB_DIR/issues/$repo.json" 2>/dev/null || echo "[]" ;; + "issue create") + [[ -n "${STUB_FAIL_ISSUE_CREATE:-}" ]] && { echo "gh: forbidden" >&2; exit 1; } + echo "https://github.invalid/issues/1" ;; + "issue edit"|"issue close"|"label create") + : ;; + *) + echo "gh stub: unexpected call: $*" >&2; exit 64 ;; +esac From 5b17b3dd89f0c543fe74c9cc5a5ffe76f72f9086 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 13:00:08 +0000 Subject: [PATCH 2/3] fix(reviews): dedupe findings, fail closed on truncated index, drop --location, memoize labels, keep issue body Co-Authored-By: Claude Sonnet 5.5 --- scripts/coderabbit-audit.sh | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/scripts/coderabbit-audit.sh b/scripts/coderabbit-audit.sh index da5c1f259..fb474fa6f 100644 --- a/scripts/coderabbit-audit.sh +++ b/scripts/coderabbit-audit.sh @@ -172,7 +172,7 @@ fetch_sarif() { [[ -n "$repo_id" ]] && url="$url&repo_id=$repo_id" # Key on stdin, not argv, so it stays out of the process list. if ! status=$(printf 'header = "x-coderabbitai-api-key: %s"\n' "$CODERABBIT_API_KEY" \ - | curl --silent --show-error --location --max-time 180 --retry 3 \ + | curl --silent --show-error --max-time 180 --retry 3 \ --config - --output "$out" --write-out '%{http_code}' "$url"); then warn "CodeRabbit API request failed" return 1 @@ -247,8 +247,10 @@ read_workflow() { # Create labels only when missing: priority:* and security are shared with the # other audits and the standard label set (no --force). +declare -A LABELS_READY=() ensure_labels() { local repo="$1" + [[ -n "${LABELS_READY[$repo]:-}" ]] && return 0 local specs=( "$CODERABBIT_AUDIT_LABEL|5c3bd6|CodeRabbit Deep Scan audit finding (idempotent marker)" "$VERIFY_LABEL|fbca04|LLM finding not yet confirmed by a person" @@ -264,6 +266,7 @@ ensure_labels() { gh label create "$name" --repo "$ORG/$repo" --color "$color" \ --description "$desc" 2>/dev/null || true done + LABELS_READY[$repo]=1 } # Open audit issues across the org, as NDJSON {fingerprint,repo,number}. Keyed @@ -282,6 +285,13 @@ build_issue_index() { failed=1 continue fi + # At the limit the listing may be truncated; an incomplete index would file + # duplicates and leave closed findings open, so fail closed. + if [[ "$(jq 'length' <<< "$list")" -ge 1000 ]]; then + warn "$repo: 1000 or more open audit issues, the index may be incomplete" + failed=1 + continue + fi jq -c --arg repo "$repo" '.[] | ((.body // "") | try capture("coderabbit-audit:key=(?[0-9a-f]{64})").fp catch "") as $fp | select($fp != "") @@ -330,7 +340,11 @@ resolve_findings() { '{fingerprint, finding_id, state, severity: $sev, scanned_repo: $scanned, owner_repo: $owner}' \ <<< "$row" >> "$tmp" done < "$parsed" - jq -s 'sort_by(.owner_repo, .fingerprint)' "$tmp" > "$FINDINGS_FILE" + # One row per fingerprint, preferring an open row so a conflicting fixed or + # dismissed row cannot close an open finding or file a duplicate issue. + jq -s 'group_by(.fingerprint) + | map((map(select(.state == "open")) | first) // .[0]) + | sort_by(.owner_repo, .fingerprint)' "$tmp" > "$FINDINGS_FILE" rm -f "$tmp" return "$failed" } @@ -402,10 +416,10 @@ reconcile_open() { if [[ -n "$existing" ]]; then local repo number other read -r repo number <<< "$existing" - body=$(render_body "$fp" "$id" "$sev" "$scanned" "$repo") - # Only the body and priority change: needs-verification and dev-lead are a - # person's decisions, so a re-run never restores or adds them. - if ! gh issue edit "$number" --repo "$ORG/$repo" --body "$body" --add-label "$prio" >/dev/null; then + # Only the priority label changes: the body holds the human-maintained + # Acceptance checklist, and needs-verification and dev-lead are a person's + # decisions, so a re-run never rewrites the body or restores/adds them. + if ! gh issue edit "$number" --repo "$ORG/$repo" --add-label "$prio" >/dev/null; then warn "$repo: could not update #$number" return 1 fi From 52aa7a80bd9b0c1ba41847afad4da30a5dd3ac33 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:54:00 +0000 Subject: [PATCH 3/3] fix(reviews): stop gh stderr leaking workflow paths in coderabbit-audit Co-Authored-By: Claude Sonnet 5.5 --- scripts/coderabbit-audit.sh | 5 +++-- test/scripts/coderabbit-audit/audit-run.bats | 5 +++++ test/scripts/coderabbit-audit/stubs/gh | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/scripts/coderabbit-audit.sh b/scripts/coderabbit-audit.sh index fb474fa6f..1fc8bdab8 100644 --- a/scripts/coderabbit-audit.sh +++ b/scripts/coderabbit-audit.sh @@ -239,10 +239,11 @@ list_org_repos() { --jq '.[] | {id: (.id | tostring), name, archived}' } -# Text of a workflow file at the scanned revision. +# Text of a workflow file at the scanned revision. gh's stderr is discarded: a +# transport error echoes the request URL (the finding's path) into public logs. # args: read_workflow() { - gh api "repos/$ORG/$1/contents/$2?ref=$3" --jq '.content' | base64 -d + gh api "repos/$ORG/$1/contents/$2?ref=$3" --jq '.content' 2>/dev/null | base64 -d } # Create labels only when missing: priority:* and security are shared with the diff --git a/test/scripts/coderabbit-audit/audit-run.bats b/test/scripts/coderabbit-audit/audit-run.bats index 2f47559ec..e9afb8974 100644 --- a/test/scripts/coderabbit-audit/audit-run.bats +++ b/test/scripts/coderabbit-audit/audit-run.bats @@ -255,6 +255,11 @@ assert_no_writes_and_red() { STUB_FAIL_CONTENTS=1 run bash "$SCRIPT" assert_no_writes_and_red } +@test "a workflow-fetch error never leaks the finding path into the log" { + STUB_FAIL_CONTENTS=1 run bash "$SCRIPT" + [[ "$output" != *CANARYSECRET* ]] + [[ "$output" != *".github/workflows/"* ]] +} @test "an unlistable repo fails and writes nothing rather than risk duplicates" { STUB_FAIL_ISSUE_LIST=1 run bash "$SCRIPT" assert_no_writes_and_red diff --git a/test/scripts/coderabbit-audit/stubs/gh b/test/scripts/coderabbit-audit/stubs/gh index 5f69ea549..19247c320 100755 --- a/test/scripts/coderabbit-audit/stubs/gh +++ b/test/scripts/coderabbit-audit/stubs/gh @@ -8,7 +8,7 @@ case "$1 $2" in [[ -n "${STUB_FAIL_REPOS:-}" ]] && { echo "gh: forbidden" >&2; exit 1; } cat "$STUB_DIR/repos.ndjson" ;; api\ repos/*) - [[ -n "${STUB_FAIL_CONTENTS:-}" ]] && { echo "gh: Not Found (HTTP 404)" >&2; exit 1; } + [[ -n "${STUB_FAIL_CONTENTS:-}" ]] && { echo "gh: Get \"https://api.github.com/$2\": CANARYSECRET connection refused" >&2; exit 1; } path="${2%%\?*}" base64 < "$STUB_DIR/workflows/$(basename "$path")" ;; "issue list")