From a91b05b99ce6bf312cd8205098ad894f588c836d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 22:45:29 +0000 Subject: [PATCH 1/4] ci: refuse plaintext redirects on every curl that follows them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sonar reports githubactions:S6506 on the actionlint download in lint.yml, and it is the VULNERABILITY currently failing the project's Quality Gate — new_security_rating 3 where the gate wants 1. It was introduced by the pull request that added that workflow, in which I wrote at length about pinning and supply chain, and verified with actionlint — a tool whose own documentation, written in that same pull request, says it audits correctness and not security posture. The caveat was accurate and I still walked into it. `curl -sSfL` follows redirects with no restriction on the protocol, so a redirect to plaintext http is possible. --proto '=https' --proto-redir '=https' refuse anything else, on the initial request and on every hop. The checksum verification that follows would still catch tampering, but only after the bytes had crossed the network in clear. The same flaw is in tools/sonar-profile/sync-profile.sh, added on this branch and not yet scanned — and worse there: its authenticated branch passes --user "$SONAR_TOKEN:", so a redirect to http would have put the credential on the wire in clear. Fixed in both, because fixing only what the report happened to see is how the next one gets written. Verified: the hardened download still succeeds and its checksum still verifies; the profile script still reaches the API and --check still passes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017bxVrnCNsXW3RvwmLc9Kvy --- .github/workflows/lint.yml | 7 ++++++- tools/sonar-profile/sync-profile.sh | 9 +++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 754c50f6..5a5cb41c 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -78,7 +78,12 @@ jobs: run: | set -eu archive="actionlint_${ACTIONLINT_VERSION}_linux_amd64.tar.gz" - curl -sSfL -o "$archive" \ + # --proto '=https' --proto-redir '=https' refuse anything but HTTPS, on the initial request + # AND on every redirect. Without them -L would follow a redirect to plaintext http, which is + # what Sonar's githubactions:S6506 flags, correctly: a release URL is attacker-influenced the + # moment DNS or the CDN is. The checksum below would still catch tampering, but only after the + # bytes had crossed the network in clear. + curl --proto '=https' --proto-redir '=https' -sSfL -o "$archive" \ "https://github.com/rhysd/actionlint/releases/download/v${ACTIONLINT_VERSION}/${archive}" echo "${ACTIONLINT_SHA256} ${archive}" | sha256sum --check --strict tar xzf "$archive" actionlint diff --git a/tools/sonar-profile/sync-profile.sh b/tools/sonar-profile/sync-profile.sh index 238f63a4..eaa252c0 100755 --- a/tools/sonar-profile/sync-profile.sh +++ b/tools/sonar-profile/sync-profile.sh @@ -71,11 +71,16 @@ fi # fetch — GET one endpoint. The project is public, so the API answers unauthenticated; # SONAR_TOKEN is honoured when set, which is what keeps this working the day it stops being # public. Retries cover a transient blip; a real outage aborts before anything is written. +# --proto '=https' --proto-redir '=https' matter more here than anywhere: the authenticated branch +# sends SONAR_TOKEN, and -L without them would follow a redirect to plaintext http and put the +# credential on the wire in clear. Refusing every non-HTTPS hop is the only version of this call +# that is safe to give a token to. fetch() { if [ -n "${SONAR_TOKEN:-}" ]; then - curl -sSfL --retry 3 --retry-delay 2 --max-time 60 --user "${SONAR_TOKEN}:" "$1" + curl --proto '=https' --proto-redir '=https' -sSfL --retry 3 --retry-delay 2 --max-time 60 \ + --user "${SONAR_TOKEN}:" "$1" else - curl -sSfL --retry 3 --retry-delay 2 --max-time 60 "$1" + curl --proto '=https' --proto-redir '=https' -sSfL --retry 3 --retry-delay 2 --max-time 60 "$1" fi } From 4243cce55fff7b51e3412d196d201f483fa83059 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 22:45:29 +0000 Subject: [PATCH 2/4] refactor(justdummies): drop the loop guard that could never be false MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sonar reports csharpsquid:S2583 on the chain walk in AnyChainFacts, and it is the BUG failing the project's Quality Gate — new_reliability_rating 3 where the gate wants 1. It is right. Every branch of the loop either returns or assigns `current = next`, where `next` has just been pattern-matched non-null, so `current is not null` could never be false and the `return false` after the loop was unreachable. The C# compiler agrees once the guard is removed: it reports CS0162 on exactly that line, which is the second opinion worth having. The loop becomes `for (…;;)` — the shape it always had — with the trailing return deleted. Termination is unchanged and now stated in a comment: every branch either returns or steps one link down a receiver chain, which a syntax tree makes finite. Same idiom as the redraw loop in AnyPattern. No behaviour changes, which is why this is a refactor and not a fix: the 246 analyzer tests pass untouched, and the full suite stays at 2168. Worth recording for whoever relies on the rule set this branch introduces: S2583 is enforced at `warning` in the generated config, this code violated it, and the local build reported NOTHING. It is a symbolic-execution rule that SonarCloud's engine runs and the NuGet analyzer does not. The build-side enforcement has a blind spot on precisely the rule class that fails this gate. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017bxVrnCNsXW3RvwmLc9Kvy --- JustDummies.Analyzers/AnyChainFacts.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/JustDummies.Analyzers/AnyChainFacts.cs b/JustDummies.Analyzers/AnyChainFacts.cs index 08a38452..3f94000f 100644 --- a/JustDummies.Analyzers/AnyChainFacts.cs +++ b/JustDummies.Analyzers/AnyChainFacts.cs @@ -35,7 +35,12 @@ public static bool TryGetChain(IInvocationOperation invocation, KnownSymbols sym outermost = parent; } - for (IInvocationOperation? current = outermost; current is not null;) { + // Unbounded on purpose, and it terminates: every branch below either returns or steps `current` one + // link down the receiver chain, which a syntax tree makes finite. Written `;;` rather than with a + // `current is not null` guard because that guard could never be false — `next` is pattern-matched + // non-null — which left the exit path unreachable (Sonar csharpsquid:S2583). Same shape as the + // redraw loop in AnyPattern. + for (IInvocationOperation current = outermost;;) { if (current.Instance is null) { // A static call roots the chain: it is the factory when it belongs to Any, otherwise this is not a // JustDummies chain at all. @@ -64,8 +69,6 @@ public static bool TryGetChain(IInvocationOperation invocation, KnownSymbols sym collected.Add(current); current = next; } - - return false; } private static bool IsFactoryOwner(INamedTypeSymbol? type, KnownSymbols symbols) { From 32a8e6a6edc39e9338e79f52f981d11f3040028e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 07:41:16 +0000 Subject: [PATCH 3/4] docs: accept ADR-0062 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the maintainer's decision, per the header format ADR-0057 prescribes: an `Accepted:` line is ADDED below `Proposed:` and the first date is left untouched, so the record keeps both facts — when the thinking happened and when it was ratified. They differ here by a day, and that gap is now part of the log rather than flattened out of it. Status and the index row follow. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017bxVrnCNsXW3RvwmLc9Kvy --- ...62-derive-the-build-rule-set-from-the-quality-profile.fr.md | 3 ++- .../0062-derive-the-build-rule-set-from-the-quality-profile.md | 3 ++- doc/handwritten/for-maintainers/adr/README.md | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/handwritten/for-maintainers/adr/0062-derive-the-build-rule-set-from-the-quality-profile.fr.md b/doc/handwritten/for-maintainers/adr/0062-derive-the-build-rule-set-from-the-quality-profile.fr.md index ff76313d..74330d51 100644 --- a/doc/handwritten/for-maintainers/adr/0062-derive-the-build-rule-set-from-the-quality-profile.fr.md +++ b/doc/handwritten/for-maintainers/adr/0062-derive-the-build-rule-set-from-the-quality-profile.fr.md @@ -2,8 +2,9 @@ 🌍 🇫🇷 Français (ce fichier) · 🇬🇧 [English](0062-derive-the-build-rule-set-from-the-quality-profile.md) -**Statut :** Proposé +**Statut :** Accepté **Proposé :** 2026-07-29 +**Accepté :** 2026-07-30 **Décideurs :** Reefact ## Contexte diff --git a/doc/handwritten/for-maintainers/adr/0062-derive-the-build-rule-set-from-the-quality-profile.md b/doc/handwritten/for-maintainers/adr/0062-derive-the-build-rule-set-from-the-quality-profile.md index f9bfb0f2..eee7b210 100644 --- a/doc/handwritten/for-maintainers/adr/0062-derive-the-build-rule-set-from-the-quality-profile.md +++ b/doc/handwritten/for-maintainers/adr/0062-derive-the-build-rule-set-from-the-quality-profile.md @@ -2,8 +2,9 @@ 🌍 🇬🇧 English (this file) · 🇫🇷 [Français](0062-derive-the-build-rule-set-from-the-quality-profile.fr.md) -**Status:** Proposed +**Status:** Accepted **Proposed:** 2026-07-29 +**Accepted:** 2026-07-30 **Decision Makers:** Reefact ## Context diff --git a/doc/handwritten/for-maintainers/adr/README.md b/doc/handwritten/for-maintainers/adr/README.md index 08cff626..1d5ba2be 100644 --- a/doc/handwritten/for-maintainers/adr/README.md +++ b/doc/handwritten/for-maintainers/adr/README.md @@ -263,4 +263,4 @@ Optional supporting material: | [ADR-0059](0059-guard-the-recipe-versus-value-boundary-with-analyzers.md) | Guard the recipe-versus-value boundary with analyzers where the type system cannot reach it | Proposed | | [ADR-0060](0060-let-stated-intent-outrank-generic-analyzer-advice.md) | Let stated intent outrank generic analyzer advice, and record the refusal beside the rule | Proposed | | [ADR-0061](0061-run-the-justdummies-analyzers-on-the-repository-s-own-code.md) | Run the JustDummies analyzers on the repository's own code, so the rules are verified against code not written to please them | Accepted | -| [ADR-0062](0062-derive-the-build-rule-set-from-the-quality-profile.md) | Derive the build's Sonar rule set from the quality profile: generated membership, hand-written exceptions, weekly drift check | Proposed | +| [ADR-0062](0062-derive-the-build-rule-set-from-the-quality-profile.md) | Derive the build's Sonar rule set from the quality profile: generated membership, hand-written exceptions, weekly drift check | Accepted | From 92fc26d0f7c409085c5fd451c4e485a40e5e44ea Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 07:41:16 +0000 Subject: [PATCH 4/4] ci: read the SonarCloud quality gate every night MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate was computed by SonarCloud and enforced by nothing. `dotnet-sonarscanner end` uploads the analysis and returns; it neither waits for the verdict nor reads it, so the `sonar` job is green as soon as the upload succeeds. No GitHub check carried the verdict either. Two findings typed VULNERABILITY reached main behind that permanently green workflow, and the report showing ERROR was visible to nobody who was not looking at sonarcloud.io. This reads it: one API call, nightly, failing when the status is not OK and listing the failing conditions. Ratings are translated from the raw 1..5 the API returns to the A..E the dashboard shows, because "3" tells a reader nothing and "C" tells them where they are, and the message states what a rating worse than A means — reliability is a Bug, security a Vulnerability. Why not `sonar.qualitygate.wait` instead. That bundles two decisions: whether `sonar` should be a REQUIRED check, and whether the verdict should be READ. `sonar` is required and already calls the service, so an outage already blocks every merge while a red gate does not; the wait would extend that dependency instead of removing it. A scheduled reader separates them and needs neither decision: the verdict gets enforced, nothing waits on a third party, and `sonar`'s required status stays exactly as it was. ADR-0062 recorded that combination as unevaluated; this is it, and it turns out not to require de-requiring anything. Why this is not redundant with build/sonar-profile.globalconfig, which enforces 342 C# rules. That set is a strict subset of what the gate measures, and the gap is where the failures live: symbolic-execution rules the NuGet analyzer does not run — measured, an S2583 violation sat in this repository with the rule enforced at `warning` and the local build said nothing — every non-C# family (githubactions, shell, secrets, xml, json, yaml; Sonar analyses five languages here and C# is 85% of the lines), and coverage, duplication and hotspot review, which no analyzer can answer. Both findings that last turned this gate red were in those classes. Nightly rather than weekly like `sonar-profile`: a quality profile moves on a vendor's release cadence, the gate moves with every merge. Read-only, permissions on the job, no secret required (the project is public), and every request refuses non-HTTPS including on redirects — the authenticated branch sends the token. It never blocks a merge, by design, and the documentation says plainly that this makes it an alarm that can be muted into uselessness. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017bxVrnCNsXW3RvwmLc9Kvy --- .github/workflows/sonar-gate.yml | 79 ++++++++++++ .../for-maintainers/workflows/README.fr.md | 3 +- .../for-maintainers/workflows/README.md | 3 +- .../workflows/sonar-gate.en.md | 106 ++++++++++++++++ .../workflows/sonar-gate.fr.md | 114 ++++++++++++++++++ tools/sonar-profile/check-gate.sh | 84 +++++++++++++ 6 files changed, 387 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/sonar-gate.yml create mode 100644 doc/handwritten/for-maintainers/workflows/sonar-gate.en.md create mode 100644 doc/handwritten/for-maintainers/workflows/sonar-gate.fr.md create mode 100755 tools/sonar-profile/check-gate.sh diff --git a/.github/workflows/sonar-gate.yml b/.github/workflows/sonar-gate.yml new file mode 100644 index 00000000..10a1e99d --- /dev/null +++ b/.github/workflows/sonar-gate.yml @@ -0,0 +1,79 @@ +name: sonar-gate + +# Reads the SonarCloud Quality Gate verdict every night and fails when it is not green. +# +# The gap this closes. `dotnet-sonarscanner end` uploads the analysis and returns; it neither waits +# for the gate nor reads it, and no GitHub check carries the verdict — of the 28 checks on a recent +# pull request, the only Sonar one was the repository's own analysis job. The gate was therefore +# computed by SonarCloud and enforced by nothing. Two findings typed VULNERABILITY reached `main` +# behind a permanently green `sonar` workflow. +# +# Why not simply make the scanner wait (`sonar.qualitygate.wait`). Because `sonar` is a REQUIRED +# check that calls the service, so a SonarCloud outage already blocks every merge while a red gate +# does not; adding the wait would deepen that coupling rather than remove it. Reading the verdict +# from a SCHEDULED job separates the two questions: the verdict gets enforced, and an outage costs +# a red nightly instead of a frozen repository. That combination — not blocking a merge, and +# reading the gate — is the one ADR-0062 left open, and this is it. +# +# Why this is not redundant with the build. build/sonar-profile.globalconfig enforces the C# rules +# the SonarAnalyzer NuGet package implements, which is a strict SUBSET of what the gate measures: +# - symbolic-execution rules the package does not run. Measured: an S2583 violation sat in this +# repository with the rule enforced at `warning`, and the local build reported nothing; +# - every non-C# family — githubactions, shell, secrets, xml, json, yaml; +# - coverage, duplication and hotspot review, which no analyzer can answer. +# Both of the findings that last turned this gate red were in those classes. The build hardening +# and this check are complements. +# +# Nightly, unlike the weekly `sonar-profile` check: a quality profile moves on a vendor's release +# cadence, but the gate moves with every merge. + +on: + schedule: + # 04:11 UTC daily — in the gap left by the weekly jobs, which run Monday between 03:00 and 06:30. + - cron: '11 4 * * *' + workflow_dispatch: + +# Cancel superseded runs (a dispatch on top of a scheduled one). +concurrency: + group: sonar-gate-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + gate: + name: Quality gate verdict + runs-on: ubuntu-latest + # One API call; the cap only guards a hung request. + timeout-minutes: 10 + # Declared per job so a job added later inherits nothing it did not ask for (Sonar + # githubactions:S8264). This job reads the checkout and calls a public API — nothing else. + permissions: + contents: read + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + + - name: Read the quality gate + # The project is public, so this needs no secret. SONAR_TOKEN is passed from the same secret + # sonar.yml uses so the check keeps working the day the project stops being public; a missing + # secret is an empty string, which the script treats as unauthenticated. + env: + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + run: tools/sonar-profile/check-gate.sh + + - name: Say what to do about it + # The failing conditions are already in the log above; this is the pointer, not a repeat. + if: failure() + run: | + # shellcheck disable=SC2016 # the backticks are Markdown for the step summary, not command substitution + { + echo '### SonarCloud quality gate is red' + echo + echo 'The failing conditions are in the previous step.' + echo + echo 'A rating worse than A means the new-code period carries at least one issue of that' + echo 'kind — reliability is a Bug, security is a Vulnerability, maintainability a Code Smell.' + echo 'Coverage and duplication conditions read directly.' + echo + echo 'This check does NOT block any merge, by design. It is the standing alarm that the' + echo 'gate is not green, and it will stay red until someone acts on it.' + } >> "$GITHUB_STEP_SUMMARY" diff --git a/doc/handwritten/for-maintainers/workflows/README.fr.md b/doc/handwritten/for-maintainers/workflows/README.fr.md index e4ed5c1f..d5fdf953 100644 --- a/doc/handwritten/for-maintainers/workflows/README.fr.md +++ b/doc/handwritten/for-maintainers/workflows/README.fr.md @@ -77,7 +77,8 @@ documentées une seule fois ici plutôt que répétées sur chaque page. | [`analyzers`](analyzers.fr.md) | Dogfood des analyzers Roslyn embarqués, y compris sur le plus vieux compilateur supporté (le floor Roslyn). | | [`commit-lint`](commit-lint.fr.md) | Impose la convention Conventional Commits sur chaque commit de PR, via le même script que le hook local. | | [`lint`](lint.fr.md) | shellcheck et actionlint sur les fichiers que le compilateur C# ne voit jamais — les scripts POSIX et les définitions de workflow. Zéro constat, `info` compris. | -| [`sonar-profile`](sonar-profile.fr.md) | Chaque nuit : échoue quand la liste de règles C# Sonar committée s'est écartée du profil qualité SonarCloud. Signale, ne répare jamais. | +| [`sonar-profile`](sonar-profile.fr.md) | Chaque semaine : échoue quand la liste de règles C# Sonar committée s'est écartée du profil qualité SonarCloud. Signale, ne répare jamais. | +| [`sonar-gate`](sonar-gate.fr.md) | Chaque nuit : lit le verdict du Quality Gate SonarCloud et échoue s'il est rouge. Ne bloque jamais un merge — l'alarme que le téléversement `sonar` n'a jamais été. | | [`adr-check`](adr-check.fr.md) | Consultatif, dispatch manuel : confronte une branche à la base d'ADR (nouvelle décision / remplacement / conflit). Le repli pour les contributeurs sans Claude Code ; ne bloque jamais. | ### Sécurité & chaîne d'approvisionnement diff --git a/doc/handwritten/for-maintainers/workflows/README.md b/doc/handwritten/for-maintainers/workflows/README.md index f27cd1cc..e7723b35 100644 --- a/doc/handwritten/for-maintainers/workflows/README.md +++ b/doc/handwritten/for-maintainers/workflows/README.md @@ -73,7 +73,8 @@ here instead of being repeated on every page. | [`analyzers`](analyzers.en.md) | Dogfood the bundled Roslyn analyzers, including on the oldest supported compiler (the Roslyn floor). | | [`commit-lint`](commit-lint.en.md) | Enforce the Conventional Commits convention on every PR commit, using the same script as the local hook. | | [`lint`](lint.en.md) | shellcheck and actionlint over the files the C# compiler never sees — the POSIX scripts and the workflow definitions. Zero findings, `info` included. | -| [`sonar-profile`](sonar-profile.en.md) | Nightly: fails when the committed Sonar C# rule list has drifted from the SonarCloud quality profile. Reports, never repairs. | +| [`sonar-profile`](sonar-profile.en.md) | Weekly: fails when the committed Sonar C# rule list has drifted from the SonarCloud quality profile. Reports, never repairs. | +| [`sonar-gate`](sonar-gate.en.md) | Nightly: reads the SonarCloud Quality Gate verdict and fails when it is red. Never blocks a merge — the alarm the `sonar` upload never was. | | [`adr-check`](adr-check.en.md) | Advisory, manual dispatch: check a branch against the ADR base (new decision / supersede / conflict). The fallback for contributors without Claude Code; never blocks. | ### Security & supply chain diff --git a/doc/handwritten/for-maintainers/workflows/sonar-gate.en.md b/doc/handwritten/for-maintainers/workflows/sonar-gate.en.md new file mode 100644 index 00000000..fd9dfbc9 --- /dev/null +++ b/doc/handwritten/for-maintainers/workflows/sonar-gate.en.md @@ -0,0 +1,106 @@ +# `sonar-gate` workflow + +🌍 🇬🇧 English (this file) · 🇫🇷 [Français](sonar-gate.fr.md) + +> Maintainer documentation — part of the [workflow reference](README.md). +> Not part of the user documentation under `doc/`. + +**Workflow file:** [`.github/workflows/sonar-gate.yml`](../../../../.github/workflows/sonar-gate.yml) +**Script:** [`tools/sonar-profile/check-gate.sh`](../../../../tools/sonar-profile/check-gate.sh) + +## What it is for + +It reads the **SonarCloud Quality Gate verdict** every night and fails when it is not green. + +Until this existed, the gate was computed by SonarCloud and **enforced by nothing**. +`dotnet-sonarscanner end` uploads the analysis and returns; it neither waits for the gate nor +reads it, so the [`sonar`](sonar.en.md) job is green as soon as the upload succeeds, whatever +the verdict. No GitHub check carried the verdict either — of the 28 checks on a recent pull +request, the only Sonar one was the repository's own analysis job. Two findings typed +VULNERABILITY reached `main` behind that permanently green workflow. + +## Why not make the scanner wait instead + +`sonar.qualitygate.wait=true` looks like the obvious fix and is the wrong one *as posed*, +because it bundles two decisions: whether `sonar` should be a **required** check, and whether +the gate's verdict should be **read**. + +As things stand `sonar` is required and already calls SonarCloud, so an outage already blocks +every merge while a red gate does not. Adding the wait extends that dependency instead of +removing it: the failure mode becomes "nobody can merge because a SaaS is down". + +Reading the verdict from a **scheduled** job separates the two. The verdict gets enforced, and +an outage costs a red nightly rather than a frozen repository. That is the combination ADR-0062 +recorded as never having been evaluated on its own merits — this workflow is it. + +## Why this is not redundant with the build + +`build/sonar-profile.globalconfig` enforces the C# rules the `SonarAnalyzer` NuGet package +implements. That is a strict **subset** of what the gate measures, and the gap is not academic: + +* **Symbolic-execution rules the package does not run.** Measured: an `S2583` violation sat in + this repository with the rule enforced at `warning` in the generated config, and the local + build reported *nothing*. SonarCloud's engine found it; the analyzer package cannot. +* **Every non-C# family** — `githubactions`, `shell`, `secrets`, `xml`, `json`, `yaml`. Sonar + analyses five languages here; C# is 85% of the lines and none of the others are covered by a + Roslyn analyzer. +* **Coverage, duplication and hotspot review**, which no analyzer can answer at all. + +Both findings that last turned this gate red were in those classes: a symbolic-execution bug +and a `githubactions` vulnerability. The build hardening and this check are complements, not +alternatives — and it was the absence of this check that let them sit unnoticed. + +## When it runs + +- **Nightly**, 04:11 UTC — in the gap left by the weekly jobs, which run Monday between 03:00 + and 06:30. +- On demand via **`workflow_dispatch`**. + +Nightly, unlike the weekly [`sonar-profile`](sonar-profile.en.md) drift check: a quality profile +moves on a vendor's release cadence, but the gate moves with **every merge**. + +It deliberately does **not** run on pull requests. That is the whole point: the verdict is +read and reported, and no merge ever waits on a third party. + +## How it runs + +One job, `Quality gate verdict`: checkout, then `tools/sonar-profile/check-gate.sh`, which +calls `/api/qualitygates/project_status` and exits non-zero when the status is not `OK`, +listing the failing conditions. + +Ratings are translated from the raw `1..5` the API returns to the `A..E` the dashboard shows — +"C" tells a reader where they are, "3" does not — and the message states what a rating worse +than A actually means: reliability is a **Bug**, security a **Vulnerability**, maintainability a +**Code Smell**. + +## Permissions & security + +`contents: read`, declared **on the job** (Sonar `githubactions:S8264`). + +The project is public, so this needs **no secret**. `SONAR_TOKEN` is passed from the same secret +`sonar.yml` uses so the check keeps working the day the project stops being public; a missing +secret is an empty string, which the script treats as unauthenticated. Every request refuses +non-HTTPS on the initial call *and* on redirects, which matters because the authenticated branch +sends the token. + +## Handle with care + +- **It never blocks a merge, and that is deliberate.** It is a standing alarm, not a gate. A red + gate will produce a red run every night until somebody acts, which is the intended behaviour + and also the way this check can be muted into uselessness. +- **It reads the project, not the branch.** The verdict reflects the last analysis of `main`, so a + fix on an unmerged branch does not turn it green — only merging does. Expect a lag of one + analysis after any fix lands. +- **A rating condition names a class, not a count.** `new_reliability_rating: C` means "at least + one Major bug in the new-code period"; it does not say how many. Follow the link the script + prints to see them. +- **It shares its script directory with `sonar-profile` but answers a different question.** Drift + means "the rule list is stale, regenerate it"; a red gate means "something got through, go look". + Different actions, which is why they are separate workflows on separate cadences. + +## Related + +- [`sonar`](sonar.en.md) — produces the analysis this reads. It uploads; it has never enforced. +- [`sonar-profile`](sonar-profile.en.md) — the weekly check that the committed rule list still + matches the server's profile. +- [`ci`](ci.en.md) — where the warning ratchet enforces the C# rules the build *can* see. diff --git a/doc/handwritten/for-maintainers/workflows/sonar-gate.fr.md b/doc/handwritten/for-maintainers/workflows/sonar-gate.fr.md new file mode 100644 index 00000000..83c26709 --- /dev/null +++ b/doc/handwritten/for-maintainers/workflows/sonar-gate.fr.md @@ -0,0 +1,114 @@ +# Workflow `sonar-gate` + +🌍 🇫🇷 Français (ce fichier) · 🇬🇧 [English](sonar-gate.en.md) + +> Documentation mainteneur — fait partie de la [référence des workflows](README.fr.md). +> Ne fait pas partie de la documentation utilisateur sous `doc/`. + +**Fichier du workflow :** [`.github/workflows/sonar-gate.yml`](../../../../.github/workflows/sonar-gate.yml) +**Script :** [`tools/sonar-profile/check-gate.sh`](../../../../tools/sonar-profile/check-gate.sh) + +## À quoi il sert + +Il lit chaque nuit le **verdict du Quality Gate SonarCloud** et échoue quand il n'est pas vert. + +Jusqu'ici, le gate était calculé par SonarCloud et **appliqué par rien**. +`dotnet-sonarscanner end` téléverse l'analyse et rend la main ; il n'attend pas le gate et ne le +lit pas, si bien que le job [`sonar`](sonar.fr.md) est vert dès que le téléversement réussit, +quel que soit le verdict. Aucune *check* GitHub ne portait ce verdict non plus — sur les 28 +*checks* d'une *pull request* récente, la seule Sonar était le job d'analyse du dépôt lui-même. +Deux constats typés VULNERABILITY ont atteint `main` derrière ce workflow durablement vert. + +## Pourquoi ne pas faire attendre le scanner + +`sonar.qualitygate.wait=true` ressemble à la correction évidente et n'est pas la bonne *telle que +posée*, parce qu'elle fond deux décisions : `sonar` doit-elle être une *check* **requise**, et le +verdict du gate doit-il être **lu**. + +En l'état, `sonar` est requise et appelle déjà SonarCloud : une panne bloque donc déjà tous les +merges alors qu'un gate rouge ne les bloque pas. Ajouter l'attente étend cette dépendance au lieu +de la supprimer : le mode de défaillance devient « personne ne peut merger parce qu'un SaaS est +tombé ». + +Lire le verdict depuis un job **planifié** sépare les deux. Le verdict est appliqué, et une panne +coûte un nocturne rouge plutôt qu'un dépôt gelé. C'est la combinaison qu'ADR-0062 a consignée +comme jamais évaluée pour elle-même — ce workflow, c'est elle. + +## Pourquoi ce n'est pas redondant avec le build + +`build/sonar-profile.globalconfig` applique les règles C# que le paquet `SonarAnalyzer` +implémente. C'est un **sous-ensemble strict** de ce que mesure le gate, et l'écart n'est pas +théorique : + +* **Les règles d'exécution symbolique que le paquet n'exécute pas.** Mesuré : une violation de + `S2583` était présente dans ce dépôt avec la règle appliquée en `warning` dans la configuration + générée, et le build local n'a **rien** signalé. Le moteur de SonarCloud l'a trouvée ; le paquet + ne peut pas. +* **Toutes les familles non-C#** — `githubactions`, `shell`, `secrets`, `xml`, `json`, `yaml`. + Sonar analyse cinq langages ici ; le C# est 85 % des lignes et aucun des autres n'est couvert + par un analyseur Roslyn. +* **Couverture, duplication et revue des *hotspots***, auxquelles aucun analyseur ne peut répondre. + +Les deux constats qui ont mis ce gate au rouge la dernière fois relevaient de ces classes : un bug +d'exécution symbolique et une vulnérabilité `githubactions`. Le durcissement du build et ce +contrôle sont complémentaires, non alternatifs — et c'est l'absence de ce contrôle qui les a +laissés passer inaperçus. + +## Quand il s'exécute + +- **Chaque nuit**, à 04h11 UTC — dans le creux laissé par les jobs hebdomadaires, qui tournent le + lundi entre 03h00 et 06h30. +- À la demande via **`workflow_dispatch`**. + +Chaque nuit, contrairement au contrôle de dérive hebdomadaire +[`sonar-profile`](sonar-profile.fr.md) : un profil qualité bouge à la cadence de livraison d'un +éditeur, mais le gate bouge à **chaque merge**. + +Il ne s'exécute délibérément **pas** sur les *pull requests*. C'est tout le propos : le verdict +est lu et signalé, et aucun merge n'attend jamais un tiers. + +## Comment il s'exécute + +Un job, `Quality gate verdict` : checkout, puis `tools/sonar-profile/check-gate.sh`, qui appelle +`/api/qualitygates/project_status` et sort en code non nul quand le statut n'est pas `OK`, en +listant les conditions en échec. + +Les notes sont traduites du `1..5` brut renvoyé par l'API vers le `A..E` qu'affiche le tableau de +bord — « C » dit au lecteur où il en est, « 3 » non — et le message précise ce qu'une note pire +que A signifie réellement : la fiabilité, c'est un **Bug** ; la sécurité, une **Vulnérabilité** ; +la maintenabilité, un **Code Smell**. + +## Permissions & sécurité + +`contents: read`, déclaré **sur le job** (Sonar `githubactions:S8264`). + +Le projet est public : aucun secret n'est nécessaire. `SONAR_TOKEN` est transmis depuis le même +secret que `sonar.yml` pour que le contrôle survive au jour où le projet cessera d'être public ; +un secret absent est une chaîne vide, que le script traite comme « non authentifié ». Chaque +requête refuse le non-HTTPS à l'appel initial **et** sur les redirections, ce qui importe parce +que la branche authentifiée envoie le token. + +## À manier avec précaution + +- **Il ne bloque jamais un merge, et c'est délibéré.** C'est une alarme permanente, pas un + garde-fou. Un gate rouge produira un run rouge chaque nuit jusqu'à ce que quelqu'un agisse : + c'est le comportement voulu, et aussi la façon dont ce contrôle peut être coupé jusqu'à + l'inutilité. +- **Il lit le projet, pas la branche.** Le verdict reflète la dernière analyse de `main` : un + correctif sur une branche non mergée ne le verdit pas, seul le merge le fait. Compter un + décalage d'une analyse après chaque correctif. +- **Une condition de note nomme une classe, pas un nombre.** `new_reliability_rating: C` signifie + « au moins un bug majeur dans la fenêtre de code neuf » ; elle ne dit pas combien. Suivre le + lien que le script imprime. +- **Il partage son répertoire de script avec `sonar-profile` mais répond à une autre question.** + Une dérive veut dire « la liste de règles est périmée, régénère-la » ; un gate rouge veut dire + « quelque chose est passé, va voir ». Actions différentes, d'où deux workflows sur deux + cadences. + +## Voir aussi + +- [`sonar`](sonar.fr.md) — produit l'analyse que celui-ci lit. Il téléverse ; il n'a jamais + appliqué. +- [`sonar-profile`](sonar-profile.fr.md) — le contrôle hebdomadaire que la liste de règles + committée correspond encore au profil du serveur. +- [`ci`](ci.fr.md) — là où le ratchet de warnings applique les règles C# que le build *peut* voir. diff --git a/tools/sonar-profile/check-gate.sh b/tools/sonar-profile/check-gate.sh new file mode 100755 index 00000000..0a086af7 --- /dev/null +++ b/tools/sonar-profile/check-gate.sh @@ -0,0 +1,84 @@ +#!/bin/sh +# Read the SonarCloud Quality Gate verdict for this project and fail when it is not green. +# +# Usage: +# tools/sonar-profile/check-gate.sh +# +# Why this exists. `dotnet-sonarscanner end` uploads the analysis and returns; it neither waits +# for the gate nor reads it, and no GitHub check carries the verdict. The gate was therefore +# computed and enforced by nothing, while the workflow that produced it stayed green as long as +# the upload succeeded. +# +# Why here and not in the sonar workflow. Making the scanner wait would couple MERGE +# AVAILABILITY to a third-party service: the sonar job is a required check, so a SonarCloud +# outage would stop every merge while a red gate still would not. Reading the verdict from a +# SCHEDULED job separates the two — the verdict is enforced, and an outage costs a red nightly +# instead of a frozen repository (decision: ADR-0062, follow-up on the gate questions). +# +# What this catches that the build cannot. build/sonar-profile.globalconfig enforces the C# rules +# the NuGet analyzer implements, and that is a strict subset of what the gate measures: +# - symbolic-execution rules (S2583 and its family) that SonarCloud's engine runs and the +# analyzer package does not — measured: a violation of S2583 sat in this repository with the +# rule enforced at `warning` and the local build reported nothing; +# - every non-C# rule family: githubactions, shell, secrets, xml, json, yaml; +# - coverage, duplication and security-hotspot review, which no analyzer can answer. +# Those are the classes that actually turn this gate red, so the build hardening and this check +# are complements, not alternatives. +# +# The project is public, so no token is required; SONAR_TOKEN is honoured when set. + +set -eu + +PROJECT="${SONAR_PROJECT_KEY:-reefact_first-class-errors}" +API="${SONAR_API_BASE:-https://sonarcloud.io/api}" + +fail() { printf 'check-gate: %s\n' "$1" >&2; exit "${2:-1}"; } + +command -v curl >/dev/null || fail "curl is required" +command -v jq >/dev/null || fail "jq is required" + +# --proto '=https' --proto-redir '=https' refuse every non-HTTPS hop, including on a redirect, +# which matters because the token below would otherwise be sent in clear (Sonar +# githubactions:S6506 flagged exactly this shape elsewhere in the repository). +if [ -n "${SONAR_TOKEN:-}" ]; then + body="$(curl --proto '=https' --proto-redir '=https' -sSfL --retry 3 --retry-delay 2 --max-time 60 \ + --user "${SONAR_TOKEN}:" "${API}/qualitygates/project_status?projectKey=${PROJECT}")" \ + || fail "could not reach ${API}" +else + body="$(curl --proto '=https' --proto-redir '=https' -sSfL --retry 3 --retry-delay 2 --max-time 60 \ + "${API}/qualitygates/project_status?projectKey=${PROJECT}")" \ + || fail "could not reach ${API}" +fi + +status="$(printf '%s' "$body" | jq -r '.projectStatus.status // empty')" +[ -n "$status" ] || fail "no gate status in the response; the project key may be wrong: ${PROJECT}" + +# A rating is reported as 1..5 where the dashboard shows A..E, so the raw number is translated: +# "3" tells a reader nothing, "C" tells them where they are. +render() { + printf '%s' "$body" | jq -r ' + def letter: {"1":"A","2":"B","3":"C","4":"D","5":"E"}; + .projectStatus.conditions[]? + | select(.status != "OK") + | . as $c + | ($c.metricKey | test("_rating$")) as $isRating + | " \($c.metricKey): \(if $isRating then (letter[$c.actualValue] // $c.actualValue) else $c.actualValue end)" + + " (needs \(if $c.comparator == "GT" then "at most" else "at least" end)" + + " \(if $isRating then (letter[$c.errorThreshold] // $c.errorThreshold) else $c.errorThreshold end))" + ' +} + +if [ "$status" = "OK" ]; then + printf 'check-gate: the SonarCloud quality gate is green for %s.\n' "$PROJECT" + exit 0 +fi + +printf 'check-gate: the SonarCloud quality gate is %s for %s.\n' "$status" "$PROJECT" >&2 +printf '\n' >&2 +printf 'Failing conditions:\n' >&2 +render >&2 +printf '\n' >&2 +printf 'A rating worse than A means the new-code period carries at least one issue of that kind:\n' >&2 +printf ' reliability -> a Bug, security -> a Vulnerability, maintainability -> a Code Smell.\n' >&2 +printf 'Open https://sonarcloud.io/project/issues?id=%s&resolved=false&inNewCodePeriod=true\n' "$PROJECT" >&2 +exit 1