fix(security): scope SONAR_TOKEN to scan steps and fail unanalysed fork PRs (#1390, #1392)

.github/workflows/required-checks-tests.yml

@@ -14,12 +14,15 @@
 #       See test/workflows/required-checks/merge-group.bats. Also: a failed
 #       SonarCloud quality gate fails the required `SonarCloud` check, in the
 #       repo workflow and the template (sonarcloud-quality-gate.bats).
+#       sonarcloud-token-scope.bats: SONAR_TOKEN never shares a job with the
+#       code under test, and a fork PR it can't analyse fails SonarCloud (#1390, #1392).
 name: Required Checks Tests
 
 on:
   pull_request:
     paths:
       - '.github/workflows/sonarcloud.yml'
+      # sonarcloud-token-scope.bats keeps this repo's token gate identical to the template's.
       - 'standards/workflows/sonarcloud.yml'
       - '.github/workflows/codeql.yml'
       - '.github/workflows/agent-shield.yml'
@@ -34,6 +37,7 @@ on:
     branches: [main]
     paths:
       - '.github/workflows/sonarcloud.yml'
+      # sonarcloud-token-scope.bats keeps this repo's token gate identical to the template's.
       - 'standards/workflows/sonarcloud.yml'
       - '.github/workflows/codeql.yml'
       - '.github/workflows/agent-shield.yml'

.github/workflows/sonarcloud.yml

@@ -15,27 +15,69 @@ on:
   merge_group:
 
 jobs:
-  sonarcloud:
-    name: SonarCloud
+  # Runs the PR / merge-queue revision's Python (dependency install + pytest)
+  # to produce coverage. It gets NO secrets: SONAR_TOKEN must never share a job
+  # with code from the revision under test (#1392). The report reaches the
+  # SonarCloud job below as an artifact; that job executes no repo code.
+  coverage:
+    name: SonarCloud coverage
     runs-on: ubuntu-latest
     permissions:
       contents: read
-      pull-requests: read
-    env:
-      SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
         with:
-          fetch-depth: 0
+          persist-credentials: false
 
       - name: Install pytest and dependencies
         run: 'pip install --quiet --require-hashes --only-binary :all: -r scripts/pytest-requirements.txt'
 
       - name: Generate coverage report
+        env:
+          COVERAGE_XML: ${{ runner.temp }}/sonar-coverage/coverage.xml
         run: |
           set -euo pipefail
           python3 -m pytest test/feature-ideation/test_validate_signals.py \
-            --cov --cov-report=xml:test/feature-ideation/coverage.xml
+            --cov --cov-report="xml:${COVERAGE_XML}"
+
+      - name: Upload coverage report
+        uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
+        with:
+          name: sonar-coverage
+          path: ${{ runner.temp }}/sonar-coverage/coverage.xml
+          if-no-files-found: error
+          retention-days: 1
+
+  sonarcloud:
+    name: SonarCloud
+    needs: coverage
+    # Run even when coverage failed: a skipped required job reports success, so
+    # this job has to run and fail on a bad coverage result instead.
+    if: ${{ !cancelled() }}
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      pull-requests: read
+    steps:
+      - name: Require a successful coverage job
+        if: ${{ needs.coverage.result != 'success' }}
+        env:
+          COVERAGE_RESULT: ${{ needs.coverage.result }}
+        run: |
+          echo "::error::The coverage job finished with result '${COVERAGE_RESULT}'; see the 'SonarCloud coverage' job."
+          exit 1
+
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          fetch-depth: 0
+          persist-credentials: false
+
+      # sonar.python.coverage.reportPaths (sonar-project.properties) points here.
+      - name: Download coverage report
+        uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2
+        with:
+          name: sonar-coverage
+          path: test/feature-ideation
 
       # On a merge queue run GITHUB_REF is the temporary gh-readonly-queue/* ref.
       # Left alone, the scanner would record it as a new long-lived branch in
@@ -54,6 +96,40 @@ jobs:
             echo "name=" >> "$GITHUB_OUTPUT"
           fi
 
+      # The required check must not pass a fork PR just because the token is
+      # unavailable to it (#1390). Same script as the standards template
+      # (standards/workflows/sonarcloud.yml); keep the two identical.
+      - name: Check SonarCloud token
+        id: token
+        env:
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
+          SONAR_BOOTSTRAP: ${{ vars.SONAR_BOOTSTRAP }}
+          EVENT_NAME: ${{ github.event_name }}
+          HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }}
+          BASE_REPO: ${{ github.repository }}
+        run: |
+          if [ -n "${SONAR_TOKEN:-}" ]; then
+            echo "present=true" >> "$GITHUB_OUTPUT"
+            exit 0
+          fi
+          echo "present=false" >> "$GITHUB_OUTPUT"
+          # Forks first: SONAR_BOOTSTRAP must never let an unanalysed fork PR pass.
+          if [ "${EVENT_NAME:-}" = "pull_request" ] && [ "${HEAD_REPO:-}" != "${BASE_REPO:-}" ]; then
+            msg="This pull request comes from a fork. GitHub does not pass SONAR_TOKEN to fork runs,"
+            msg="${msg} so SonarCloud cannot analyse it and this required check fails."
+            msg="${msg} A maintainer can push the branch to this repository and open a same-repo"
+            msg="${msg} pull request so the analysis runs (combsmith/.github standards/ci-standards.md, section 3)."
+            echo "::error title=SonarCloud not run::${msg}"
+            exit 1
+          fi
+          if [ "${SONAR_BOOTSTRAP:-}" = "true" ]; then
+            echo "::notice title=SonarCloud skipped::SONAR_BOOTSTRAP is 'true' and SONAR_TOKEN is unavailable, so no analysis ran. Delete the SONAR_BOOTSTRAP variable once SonarCloud is onboarded."
+            exit 0
+          fi
+          msg="SONAR_TOKEN is not available to this run, so no analysis ran. Add the SONAR_TOKEN secret"
+          msg="${msg} (see BOOTSTRAP.md); Dependabot runs only see it if it is also a Dependabot secret."
+          echo "::warning title=SonarCloud skipped::${msg}"
+
       # The scan waits for the SonarCloud quality gate on pull requests
       # (sonar.qualitygate.wait), so a failed gate fails this job and the
       # required `SonarCloud` check. Without it the job passed whenever the
@@ -64,10 +140,13 @@ jobs:
       # First attempt. continue-on-error lets the retry step below recover from
       # transient 403/5xx responses from binaries.sonarsource.com (the scanner
       # CLI download), without failing the job on a single CDN blip.
+      # SONAR_TOKEN is scoped to the two scan steps, never the job env (#1392).
       - name: SonarCloud Scan
         id: sonar
-        if: ${{ env.SONAR_TOKEN != '' }}
+        if: ${{ steps.token.outputs.present == 'true' }}
         uses: SonarSource/sonarqube-scan-action@4d1dc0c91ee400dcf8a6dddf2f705ac36542e304 # v8.4.0
+        env:
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
         with:
           args: >-
             ${{ steps.branch.outputs.name != '' &&
@@ -82,8 +161,10 @@ jobs:
       # (or the gate wait timed out), not on a transient download error.
       - name: SonarCloud Scan (retry)
         id: sonar_retry
-        if: ${{ env.SONAR_TOKEN != '' && steps.sonar.outcome == 'failure' && hashFiles('.scannerwork/report-task.txt') == '' }}
+        if: ${{ steps.token.outputs.present == 'true' && steps.sonar.outcome == 'failure' && hashFiles('.scannerwork/report-task.txt') == '' }}
         uses: SonarSource/sonarqube-scan-action@4d1dc0c91ee400dcf8a6dddf2f705ac36542e304 # v8.4.0
+        env:
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
         with:
           args: >-
             ${{ steps.branch.outputs.name != '' &&
@@ -94,7 +175,7 @@ jobs:
       # continue-on-error keeps a failed first attempt from failing the job, so
       # a gate failure that skipped the retry has to fail the job here.
       - name: Fail on SonarCloud quality gate
-        if: ${{ env.SONAR_TOKEN != '' && steps.sonar.outcome == 'failure' && steps.sonar_retry.outcome == 'skipped' }}
+        if: ${{ steps.token.outputs.present == 'true' && steps.sonar.outcome == 'failure' && steps.sonar_retry.outcome == 'skipped' }}
         run: |
           msg="The analysis was uploaded but the quality gate failed or did not "
           msg="${msg}finish. See the SonarCloud Scan step log and the 'SonarCloud "

standards/ci-standards.md

@@ -755,30 +755,46 @@ jobs:
     permissions:
       contents: read
       pull-requests: read
-    env:
-      SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
+    # No job-level env: SONAR_TOKEN is scoped to the gate and scan steps only.
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
         with:
           fetch-depth: 0
+          persist-credentials: false
+      # Token gate: outputs present=true|false; fails a fork PR that has no
+      # token, whatever vars.SONAR_BOOTSTRAP says. Full script:
+      # standards/workflows/sonarcloud.yml (the canonical copy).
+      - name: Check SonarCloud token
+        id: token
+        env:
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
+          SONAR_BOOTSTRAP: ${{ vars.SONAR_BOOTSTRAP }}
+          EVENT_NAME: ${{ github.event_name }}
+          HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }}
+          BASE_REPO: ${{ github.repository }}
+        run: ...  # see standards/workflows/sonarcloud.yml
       # First attempt. continue-on-error lets the retry step below recover from
       # transient 403/5xx responses from binaries.sonarsource.com (the scanner
       # CLI download), without failing the job on a single CDN blip.
       - name: SonarCloud Scan
         id: sonar
-        if: env.SONAR_TOKEN != ''
+        if: steps.token.outputs.present == 'true'
         uses: SonarSource/sonarqube-scan-action@d209202bc7d53ff1cc128f7f907dac145c9d6ae9 # v8.3.0
         with:
           args: ${{ github.event_name == 'pull_request' && '-Dsonar.qualitygate.wait=true' || '' }}
         continue-on-error: true
+        env:
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
       - name: SonarCloud Scan (retry)
         id: sonar_retry
-        if: env.SONAR_TOKEN != '' && steps.sonar.outcome == 'failure' && hashFiles('.scannerwork/report-task.txt') == ''
+        if: steps.token.outputs.present == 'true' && steps.sonar.outcome == 'failure' && hashFiles('.scannerwork/report-task.txt') == ''
         uses: SonarSource/sonarqube-scan-action@d209202bc7d53ff1cc128f7f907dac145c9d6ae9 # v8.3.0
+        env:
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
         with:
           args: ${{ github.event_name == 'pull_request' && '-Dsonar.qualitygate.wait=true' || '' }}
       - name: Fail on SonarCloud quality gate
-        if: env.SONAR_TOKEN != '' && steps.sonar.outcome == 'failure' && steps.sonar_retry.outcome == 'skipped'
+        if: steps.token.outputs.present == 'true' && steps.sonar.outcome == 'failure' && steps.sonar_retry.outcome == 'skipped'
         run: exit 1
 ```
 
@@ -788,6 +804,27 @@ verbatim rather than this excerpt.
 
 **Required secrets:** `SONAR_TOKEN`
 
+**Keep `SONAR_TOKEN` away from the code under test.** Never set `SONAR_TOKEN`
+in a job- or workflow-level `env:`. Only the token gate and the two scan steps
+may reference it. A repo that builds or tests its code to produce coverage
+(dependency install, test runner, build tool) does that in a **separate job
+with no secrets** and passes the report to the `SonarCloud` job as an artifact.
+That job must use `if: ${{ !cancelled() }}` and fail explicitly when the
+coverage job did not succeed, because a skipped required job counts as passing.
+This repo's own `.github/workflows/sonarcloud.yml` is the reference
+implementation. Guards: `test/scripts/standards-templates/sonarcloud-token-gate.bats`
+and `test/workflows/required-checks/sonarcloud-token-scope.bats`.
+
+**When the token is unavailable.** `SonarCloud` is a required check, so the
+gate decides whether a run that cannot analyse may still pass:
+
+| Situation | Result |
+|-----------|--------|
+| `SONAR_TOKEN` available | Analysis runs |
+| No token, pull request from a fork | **Fails**, even when `vars.SONAR_BOOTSTRAP == 'true'`. GitHub never passes secrets to fork runs, so the change cannot be analysed. A maintainer can push the branch to the repo and open a same-repo PR |
+| No token, same-repo run, `vars.SONAR_BOOTSTRAP == 'true'` | Passes with a notice. This is an explicit opt-in for a repo not yet onboarded to SonarCloud; delete the variable once it is |
+| No token, same-repo run (push, same-repo PR, merge queue; this includes Dependabot PRs, which do not receive Actions secrets) | Passes with a warning |
+
 Each repo needs a `sonar-project.properties` file at root with project key and org.
 
 **The required `SonarCloud` check carries the quality gate.** On

standards/workflows/sonarcloud.yml

@@ -7,12 +7,23 @@
 #   2. Set sonar.projectKey / sonar.organization in sonar-project.properties to
 #      match your repo (see BOOTSTRAP.md), and add the SONAR_TOKEN repo secret.
 #   3. The job name `SonarCloud` is the required status check — keep it.
-#   4. The scan is guarded on SONAR_TOKEN, so the job is a green no-op until the
-#      secret is set — a fresh repo is not blocked on day 0.
+#   4. Without SONAR_TOKEN the job cannot analyse. The `Check SonarCloud token`
+#      gate decides what that means for the required check:
+#        • fork pull request (secrets are never passed to forks) → FAIL, so an
+#          unanalysed fork change can't pass the required check, whatever
+#          SONAR_BOOTSTRAP says
+#        • SONAR_BOOTSTRAP repo variable = "true" → green no-op for same-repo
+#          runs (explicit bootstrap; delete the variable once onboarded)
+#        • same-repo run (push, same-repo PR) → green no-op with a warning: the
+#          repo has no token yet, so a fresh repo is not blocked on day 0
 #
 # CONVENTIONS (enforced by the standard):
 #   • permissions: {} at top, least-privilege per job (contents + pull-requests: read)
 #   • triggers: push + pull_request to main; checkout with fetch-depth: 0
+#   • SONAR_TOKEN is scoped to the gate and scan steps ONLY — never job- or
+#     workflow-level `env:`. A step that builds or tests the checked-out code
+#     must not see it; generate coverage in a separate job without secrets and
+#     hand the report over as an artifact.
 #   • on pull_request the scan waits for the quality gate
 #     (-Dsonar.qualitygate.wait=true), so a failed gate fails the required check
 #   • first scan is continue-on-error with a single retry, taken only when no
@@ -37,28 +48,65 @@ jobs:
     permissions:
       contents: read
       pull-requests: read
-    env:
-      SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
     steps:
       - name: Checkout repository
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
         with:
           fetch-depth: 0
+          persist-credentials: false
       - name: Set up JDK 21
         id: setup_java
         uses: actions/setup-java@b843ebf9b913146b38da2eaa7aeddf76074ae1ad # v6.1.0
         with:
           distribution: temurin
           java-version: '21'
+      # The required check must not pass a fork PR just because the token is
+      # unavailable to it. Outputs present=true|false; fails on a fork PR
+      # without the token, whatever SONAR_BOOTSTRAP says. Never echoes the token.
+      - name: Check SonarCloud token
+        id: token
+        env:
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
+          SONAR_BOOTSTRAP: ${{ vars.SONAR_BOOTSTRAP }}
+          EVENT_NAME: ${{ github.event_name }}
+          HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }}
+          BASE_REPO: ${{ github.repository }}
+        run: |
+          if [ -n "${SONAR_TOKEN:-}" ]; then
+            echo "present=true" >> "$GITHUB_OUTPUT"
+            exit 0
+          fi
+          echo "present=false" >> "$GITHUB_OUTPUT"
+          # Forks first: SONAR_BOOTSTRAP must never let an unanalysed fork PR pass.
+          if [ "${EVENT_NAME:-}" = "pull_request" ] && [ "${HEAD_REPO:-}" != "${BASE_REPO:-}" ]; then
+            msg="This pull request comes from a fork. GitHub does not pass SONAR_TOKEN to fork runs,"
+            msg="${msg} so SonarCloud cannot analyse it and this required check fails."
+            msg="${msg} A maintainer can push the branch to this repository and open a same-repo"
+            msg="${msg} pull request so the analysis runs (combsmith/.github standards/ci-standards.md, section 3)."
+            echo "::error title=SonarCloud not run::${msg}"
+            exit 1
+          fi
+          if [ "${SONAR_BOOTSTRAP:-}" = "true" ]; then
+            echo "::notice title=SonarCloud skipped::SONAR_BOOTSTRAP is 'true' and SONAR_TOKEN is unavailable, so no analysis ran. Delete the SONAR_BOOTSTRAP variable once SonarCloud is onboarded."
+            exit 0
+          fi
+          msg="SONAR_TOKEN is not available to this run, so no analysis ran. Add the SONAR_TOKEN secret"
+          msg="${msg} (see BOOTSTRAP.md); Dependabot runs only see it if it is also a Dependabot secret."
+          echo "::warning title=SonarCloud skipped::${msg}"
       # The scan waits for the SonarCloud quality gate on pull requests
       # (sonar.qualitygate.wait), so a failed gate fails this job and the
       # required `SonarCloud` check. push runs do not wait: the gate was
       # enforced on the PR, and a main-branch verdict would only fail
       # post-merge runs.
+      # First attempt. continue-on-error lets the retry step below recover from
+      # transient 403/5xx responses from binaries.sonarsource.com (the scanner
+      # CLI download), without failing the job on a single CDN blip.
       - name: SonarCloud Scan
         id: sonar
-        if: env.SONAR_TOKEN != ''
+        if: steps.token.outputs.present == 'true'
         uses: SonarSource/sonarqube-scan-action@4d1dc0c91ee400dcf8a6dddf2f705ac36542e304 # v8.4.0
+        env:
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
         with:
           args: ${{ github.event_name == 'pull_request' && '-Dsonar.qualitygate.wait=true' || '' }}
         continue-on-error: true
@@ -68,14 +116,16 @@ jobs:
       # not on a transient download error.
       - name: SonarCloud Scan (retry)
         id: sonar_retry
-        if: env.SONAR_TOKEN != '' && steps.sonar.outcome == 'failure' && hashFiles('.scannerwork/report-task.txt') == ''
+        if: steps.token.outputs.present == 'true' && steps.sonar.outcome == 'failure' && hashFiles('.scannerwork/report-task.txt') == ''
         uses: SonarSource/sonarqube-scan-action@4d1dc0c91ee400dcf8a6dddf2f705ac36542e304 # v8.4.0
+        env:
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
         with:
           args: ${{ github.event_name == 'pull_request' && '-Dsonar.qualitygate.wait=true' || '' }}
       # continue-on-error keeps a failed first attempt from failing the job, so
       # a gate failure that skipped the retry has to fail the job here.
       - name: Fail on SonarCloud quality gate
-        if: env.SONAR_TOKEN != '' && steps.sonar.outcome == 'failure' && steps.sonar_retry.outcome == 'skipped'
+        if: steps.token.outputs.present == 'true' && steps.sonar.outcome == 'failure' && steps.sonar_retry.outcome == 'skipped'
         run: |
           msg="The analysis was uploaded but the quality gate failed or did not "
           msg="${msg}finish. See the SonarCloud Scan step log and the 'SonarCloud "