PR #1417 - combsmith/.github

Summary

Two SonarCloud workflow fixes:

#1392: SONAR_TOKEN no longer shares a job with code from the revision under test.
#1390: the required SonarCloud check no longer passes a fork pull request that it could not analyse.

Both fixes are applied to this repo's .github/workflows/sonarcloud.yml and to the standards/workflows/sonarcloud.yml template. A test keeps the token gate script identical in the two files.

[!IMPORTANT]
Needs a human decision: fork PRs now fail the required SonarCloud check (#1390).
GitHub never passes secrets to fork runs. Once the hole is closed, a fork PR without vars.SONAR_BOOTSTRAP == 'true' cannot get a green SonarCloud check, so contributors see a behaviour change. This PR implements the most conservative secure version, described under #1390 below. The alternatives are listed there too.

#1392: SONAR_TOKEN reachable by the code under test

Fixes #1392

Confirmed. At .github/workflows/sonarcloud.yml:24-25 on main, SONAR_TOKEN was set in the job-level env. The same job ran pip install -r scripts/pytest-requirements.txt (L32) and pytest test/feature-ideation/test_validate_signals.py (L37) on the checked-out PR or merge_group revision, so every step inherited the token. The new guard test/workflows/required-checks/sonarcloud-token-scope.bats fails on main with "job 'sonarcloud' runs repo code and references a secret". The template (standards/workflows/sonarcloud.yml:37-38) used the same job-level env pattern. The template runs no repo code itself, but any build or test step an adopting repo added would inherit the token.

Fix

Coverage moves to a new coverage job (SonarCloud coverage). That job has contents: read, persist-credentials: false and no secrets. It uploads coverage.xml as a 1-day artifact.
The SonarCloud job (needs: coverage) downloads the artifact to the path sonar-project.properties already points at. It runs no repo code.
SONAR_TOKEN now appears only in the step env of the token gate and of the two scan steps, never in a job- or workflow-level env. The same change is made in the template.
A skipped required job reports success. The SonarCloud job therefore runs if: ${{ !cancelled() }} and fails explicitly when needs.coverage.result != 'success'. As before, failing tests still fail SonarCloud.
actions/upload-artifact v7.0.2 and actions/download-artifact v8.0.2 reuse SHAs already pinned in this repo. Both were re-verified with gh api repos/actions/<name>/git/ref/tags/<tag>.

#1390: required SonarCloud check passes fork PRs without analysis

Fixes #1390

Confirmed. In standards/workflows/sonarcloud.yml on main, both scan steps are guarded by if: env.SONAR_TOKEN != '' (L52, L56) and nothing else fails the job. A fork PR receives an empty token, both steps are skipped, and the job reports success to the required SonarCloud context (standards/rulesets/code-quality.json). This repo's own workflow behaved the same way. The new template guard test/scripts/standards-templates/sonarcloud-token-gate.bats fails on main: there is no gate step and the token sits in the job env.

Fix: a Check SonarCloud token gate step. It outputs present=true|false and never echoes the token. Both scan steps now run only when steps.token.outputs.present == 'true'.

Situation	Before	After
Token available	scan	scan (unchanged)
No token, `vars.SONAR_BOOTSTRAP == 'true'`	pass	pass with a `::notice` (new explicit bootstrap opt-in)
No token, fork PR (head repo ≠ base repo, or head repo deleted)	pass	fail with a `::error` that explains why and how a maintainer can get the change analysed
No token, same-repo run (push, same-repo PR, merge queue)	pass	pass with a `::warning` (unchanged)

Why same-repo runs without a token still pass. Per standards/ci-standards.md, SONAR_TOKEN is an org-level secret, so same-repo runs normally have it. The main same-repo run without a token is a Dependabot PR, because Dependabot runs do not receive Actions secrets. Failing that case would block Dependabot auto-merge org-wide. The other case is a genuine day-0 repo. Neither path is reachable by someone without write access.

Decision needed / alternatives

This PR (recommended): fork PRs fail. A maintainer can push the contributor's branch to the repo and open a same-repo PR so the analysis runs, or use the ruleset bypass after review. Re-running the fork PR does not help, because the token is still withheld.
Stricter: also fail same-repo runs without a token unless SONAR_BOOTSTRAP is set. This ends the bootstrap exception for real, but breaks Dependabot PRs unless SONAR_TOKEN is also added as an org Dependabot secret.
Keep status quo for forks: pass with a ::warning. This is non-breaking but leaves #1390 open.
Follow-up, larger: a separate credentialed fork-analysis path (workflow_run / pull_request_target that scans without executing fork code). That is a bigger design and is out of scope here.

I did not verify whether vars are exposed to fork pull_request runs. If they are not, SONAR_BOOTSTRAP=true will not rescue fork PRs on a repo that is still bootstrapping, and those PRs fail, which is the fail-closed direction.

Docs

standards/ci-standards.md §3 snippet and prose are updated. The doc now covers: step-scoped token, the separate secret-free coverage job with a !cancelled() guard, and the gate's behaviour table. The doc snippet's older action SHAs are left as they were.

Tests

New bats files. Each one fails on origin/main and passes with this change.

test/scripts/standards-templates/sonarcloud-token-gate.bats (11 tests). It extracts the template's gate run: with yq and executes it for each row of the table above, including deleted head repo, SONAR_BOOTSTRAP other than exactly true, and the token never printed or written to GITHUB_OUTPUT. It also asserts there is no job- or workflow-level SONAR_TOKEN, that only the gate and the scan steps reference the secret, that the scans are gated on present, and that the job name is unchanged. It runs in Standards Templates Tests.
test/workflows/required-checks/sonarcloud-token-scope.bats (10 tests). It covers the same scoping and gate assertions for this repo's workflow. It also checks that the job running pip/pytest references no secrets, that coverage arrives via an artifact, that a failed coverage job fails SonarCloud, and that the gate script is identical to the template's. It runs in Required Checks Tests. required-checks-tests.yml now also triggers on standards/workflows/sonarcloud.yml.
Both rely on yq (mikefarah), which is preinstalled on ubuntu-latest. If yq is missing, they fail loudly instead of skipping.

Local runs:

bats test/workflows/required-checks/ test/scripts/standards-templates/: 52/52 ok. This includes the existing merge-group.bats, which still finds name: SonarCloud.
bats tests/test_emit_standard_file_workflows.bats test/workflows/dependency-hardening.bats test/scripts/deploy-standard-workflows/, run under GNU bash 5: 130/131 ok. The one failure is dry-run.bats "dry-run skips a stub that is already compliant". It is a macOS-only base64 -w incompatibility on add-to-project.yml and is unrelated to this change.
actionlint: no findings in the changed workflows (pre-existing findings elsewhere unchanged). actionlint's shellcheck pass covers the gate script.
yamllint with the ci.yml config (pinned scripts/ci-yamllint-requirements.txt): clean for the changed files.
markdownlint-cli2 standards/ci-standards.md: 0 errors.
shellcheck on the new bats files: clean.

Follow-ups / release notes

Template consumers: sonarcloud.yml is not deployed by deploy-standard-workflows.sh. It ships via combsmith/repo-template, so the template copy there needs a sync PR. Repos that already adopted it keep their copy until updated.
test/feature-ideation/test_validate_signals.py (touched by open PR #1383) is not modified. Only the job that runs it changed.
Not audited: the scan job still reads the repo-controlled sonar-project.properties. The scanner is not meant to execute repo code, but its scanner-side properties were not reviewed one by one.

🤖 Generated with Claude Code

Problem

See the linked issue.

Risk

Medium — changes GitHub Actions workflow behavior, which is exercised only post-merge; verify via the affected workflow runs.

Test plan

Tests added/updated: test/scripts/standards-templates/sonarcloud-token-gate.bats,test/workflows/required-checks/sonarcloud-token-scope.bats. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Monitoring

Watch the affected workflow run(s) in the Actions tab and this PR's Lint check for regressions.