diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml deleted file mode 100644 index f3adf58..0000000 --- a/.github/workflows/sonar.yml +++ /dev/null @@ -1,114 +0,0 @@ -name: SonarCloud - -# Coverage + static analysis, deliberately separate from ci.yml: ci.yml is the pre-merge gate, and a -# Sonar outage or an expired token must not be able to block a merge. -# -# This REPLACES SonarCloud's automatic analysis. That mode never builds the project and never runs -# the tests, so per its documented limitations "Code coverage information is not supported" — which -# is why the coverage metric was missing entirely. -# -# Automatic analysis therefore has to be turned OFF, or `sonarscanner end` fails the job with -# ERROR: You are running CI analysis while Automatic Analysis is enabled. -# That is a server-side project setting — nothing in this repo can switch it off (a -# `sonar-project.properties` file, sometimes suggested for this, is rejected outright by the scanner -# for .NET). A PROJECT admin has to flip it once, at -# https://sonarcloud.io/project/settings?id=Ktechen_Trackify -> Analysis Method -# after which every run here is accepted. -# -# !! THAT HAS NOT HAPPENED YET (`sonar.autoscan.enabled=true`), so every run of this workflow to date -# has failed at `Sonar end` and NONE of the /d: settings below have ever taken effect — the coverage -# report is produced and never consumed, and the scope exclusions are inert. What actually scopes the -# published results today is .sonarcloud.properties, the file automatic analysis reads. Keep its -# exclusion list identical to the one here, or a scope change silently does nothing. See R-9 in -# docs/arc42/11-risks-and-technical-debt.md. -# -# Scope note: the scanner for .NET only sees projects that are built between `begin` and `end`. As in -# ci.yml the Uno app (Source/Trackify, five heads) is not buildable on a Linux runner, so the -# analysed scope here is the shared core + CLI + tests. -# -# Scope exclusions (`begin` below): -# - docs/** is the vendored lego-ble-wireless-protocol-docs submodule — third-party HTML/JS we don't -# own and won't fix; it accounted for 51 of 61 "bugs" and most of the ncloc. (It now also covers -# docs/arc42/, which is harmless — Sonar has no markdown analyser.) -# - Source/Trackify/**/*.cs is the Uno app, which per the scope note above is never built here and -# therefore never analysed. Without the exclusion its files stay INDEXED, so 77 findings left over -# from the old automatic-analysis mode were frozen OPEN: nothing re-evaluates them, so neither a -# fix nor a false positive can ever close them. 60 of those 77 were false positives caused by -# analysis without a compilation — S2325 "make it static" on [ObservableProperty]-generated -# property reads and on IValueConverter members, and S8970 "nullable warnings are disabled here" -# when Directory.Build.props sets enable. Only the .cs is excluded, so the -# WasmScripts JS and any XML still get analysed. Drop this exclusion if the Uno heads are ever -# built inside begin/end (that needs a windows runner) — then the analysis becomes real. -# -# There is deliberately NO issue-exclusion here. A `sonar.issue.ignore.multicriteria` for docker:S6471 -# used to be passed as a /d: parameter and silently did nothing: issue exclusions are multi-value -# SERVER-side settings, so they only apply when set in the SonarCloud UI (Project -> Administration -> -# Analysis Scope), not from the scanner command line. The analysis of 9299be9 ran with that parameter -# in place and still reported the issue as OPEN. The Dockerfile now runs as non-root instead, which -# fixes the finding at the source — prefer that over reaching for an exclusion. -on: - pull_request: - branches: [ master ] - push: - branches: [ master ] - -jobs: - sonar: - name: Scan + coverage - runs-on: ubuntu-latest - # Forks never receive SONAR_TOKEN; skip instead of failing their PR with an auth error. - if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository - steps: - - uses: actions/checkout@v4 - with: - # Sonar needs the full history to attribute issues and resolve the new-code period. - fetch-depth: 0 - - # No JDK step: `begin` runs a Roslyn analyzer inside MSBuild (pure .NET), and the Java-based - # engine that `end` uses to aggregate and upload provisions its own JRE — "from version 7.0, - # Java is no longer required because the scanner will download it automatically". - - # Same as ci.yml: global.json pins 9.0.100 and rolls forward to the newest installed major. - - name: Setup .NET (8, 9, 10) - uses: actions/setup-dotnet@v4 - with: - dotnet-version: | - 8.0.x - 9.0.x - 10.0.x - - - name: Install Sonar scanner - run: | - dotnet tool install --global dotnet-sonarscanner - echo "$HOME/.dotnet/tools" >> "$GITHUB_PATH" - - # projectBaseDir is pinned on purpose: since scanner v8 it is auto-detected, and everything - # resolved relative to it (notably sonar.exclusions below) moves with it. The repo root is what - # those paths are written against, so say so instead of relying on the detection heuristic. - - name: Sonar begin - env: - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - run: > - dotnet sonarscanner begin - /k:"Ktechen_Trackify" - /o:"trackify1" - /d:sonar.token="$SONAR_TOKEN" - /d:sonar.host.url="https://sonarcloud.io" - /d:sonar.projectBaseDir="${{ github.workspace }}" - /d:sonar.cs.opencover.reportsPaths="${{ github.workspace }}/coverage/coverage.opencover.xml" - /d:sonar.exclusions="docs/**,Source/Trackify/**/*.cs" - - # A single step builds everything the scanner should see (the test project references the core - # libs and the CLI) and lets coverlet instrument that same build — no `--no-build`, otherwise - # nothing is instrumented. OpenCover is the report format Sonar's C# analyser consumes. - - name: Test + coverage - run: > - dotnet test Test/Trackify.Tests/Trackify.Tests.csproj -c Release - /p:CollectCoverage=true - /p:CoverletOutputFormat=opencover - /p:CoverletOutput="${{ github.workspace }}/coverage/" - - - name: Sonar end - env: - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - run: dotnet sonarscanner end /d:sonar.token="$SONAR_TOKEN" diff --git a/.sonarcloud.properties b/.sonarcloud.properties index a407fd7..0ba041a 100644 --- a/.sonarcloud.properties +++ b/.sonarcloud.properties @@ -1,11 +1,11 @@ -# Scope for SonarCloud AUTOMATIC ANALYSIS only. +# Scope for SonarCloud AUTOMATIC ANALYSIS — the only analysis this project runs. # -# The CI scan in .github/workflows/sonar.yml is the real analysis (it builds the projects and uploads -# coverage) and carries its own /d:sonar.* settings; automatic analysis has to be switched off for that -# scan to be accepted, at which point this file is simply ignored. It is kept because automatic -# analysis is still the active mode (`sonar.autoscan.enabled=true` on the project), so today this file -# — not the workflow — is what actually scopes the published results. Keep the two exclusion lists -# identical; anything only in the workflow has no effect until the mode is switched. +# There is deliberately no scanner workflow. A CI scan (`dotnet sonarscanner`) cannot coexist with +# automatic analysis: `sonarscanner end` is rejected with "You are running CI analysis while Automatic +# Analysis is enabled", which is a server-side project setting no file in this repo can change. The +# workflow that used to live at .github/workflows/sonar.yml therefore failed on every run it ever made +# and published nothing, so it was removed. See R-9 in docs/arc42/11-risks-and-technical-debt.md for +# what that costs (no coverage metric, no compiled analysis) and how to reverse the decision. # # docs/ is the vendored `lego-ble-wireless-protocol-docs` submodule: third-party LEGO protocol # documentation (HTML/JS) that we neither own nor maintain. @@ -15,8 +15,7 @@ # references are unresolved. Every member that reads an [ObservableProperty]-generated property, and # every IValueConverter implementation, therefore looks like it uses no instance state — 57 bogus # S2325 "make it static" findings (uncompilable if applied), plus S8970 "nullable warnings are -# disabled here" although Directory.Build.props sets enable. The app is not -# analysable in either mode — the CI scan can't build its five heads on a Linux runner either, and -# excludes the same path — so exclude it rather than leave unfixable findings open. -# Only the .cs is excluded: the WasmScripts JS and any XML still get analysed. +# disabled here" although Directory.Build.props sets enable. Excluding the app is +# the only way to close findings that cannot be fixed in code. +# Only the .cs is excluded, so the WasmScripts JS and any XML still get analysed. sonar.exclusions=docs/**,Source/Trackify/**/*.cs diff --git a/README.md b/README.md index 5261396..5f53230 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ | Category | Badge | | --- | --- | | CI | [![CI](https://github.com/Ktechen/Trackify/actions/workflows/ci.yml/badge.svg)](https://github.com/Ktechen/Trackify/actions/workflows/ci.yml) | -| CodeQL | [![CodeQL](https://github.com/Ktechen/Trackify/actions/workflows/codeql.yml/badge.svg)](https://github.com/Ktechen/Trackify/security/code-scanning) | +| CodeQL | [![CodeQL](https://img.shields.io/badge/CodeQL-default%20setup-2088FF?logo=github)](https://github.com/Ktechen/Trackify/security/code-scanning) | | Android APK | [![Android APK](https://github.com/Ktechen/Trackify/actions/workflows/android-apk.yml/badge.svg)](https://github.com/Ktechen/Trackify/actions/workflows/android-apk.yml) | | CLI linux-arm64 | [![CLI linux-arm64](https://github.com/Ktechen/Trackify/actions/workflows/cli-arm64.yml/badge.svg)](https://github.com/Ktechen/Trackify/actions/workflows/cli-arm64.yml) | | License | [![License](https://img.shields.io/github/license/Ktechen/Trackify)](LICENSE) | @@ -20,7 +20,6 @@ | Maintainability Rating | [![Maintainability Rating](https://sonarcloud.io/api/project_badges/measure?project=Ktechen_Trackify&metric=sqale_rating)](https://sonarcloud.io/summary/new_code?id=Ktechen_Trackify) | | Technical Debt | [![Technical Debt](https://sonarcloud.io/api/project_badges/measure?project=Ktechen_Trackify&metric=sqale_index)](https://sonarcloud.io/summary/new_code?id=Ktechen_Trackify) | | Duplicated Lines | [![Duplicated Lines (%)](https://sonarcloud.io/api/project_badges/measure?project=Ktechen_Trackify&metric=duplicated_lines_density)](https://sonarcloud.io/summary/new_code?id=Ktechen_Trackify) | -| Coverage | [![Coverage](https://sonarcloud.io/api/project_badges/measure?project=Ktechen_Trackify&metric=coverage)](https://sonarcloud.io/summary/new_code?id=Ktechen_Trackify) | A Clean Architecture solution with **two front-ends over one shared core**: an [Uno Platform](https://platform.uno) app (App / HMI / Web) and a [Spectre.Console](https://spectreconsole.net) CLI for a Raspberry Pi / Linux server. diff --git a/docs/arc42/02-architecture-constraints.md b/docs/arc42/02-architecture-constraints.md index d60faee..2028046 100644 --- a/docs/arc42/02-architecture-constraints.md +++ b/docs/arc42/02-architecture-constraints.md @@ -37,7 +37,7 @@ technology choices, then the rules the project imposes on itself. | OC-1 | **Single maintainer, hobby cadence.** Anything that only works because someone remembers it will break; conventions must be machine-checked. | | OC-2 | **CI runs on hosted GitHub runners.** The Uno app's five heads cannot be restored on one runner, so `ci.yml` gates the shared core + CLI + tests only; the Android head is covered by `android-apk.yml`. | | OC-3 | **Hardware verification is manual.** BLE cannot run in CI or in an agent environment — real behaviour is confirmed on a phone and on a Raspberry Pi. | -| OC-4 | **SonarCloud + CodeQL** run on every PR. `docs/**` (the vendored LEGO submodule) is excluded — third-party HTML/JS the project neither owns nor fixes. | +| OC-4 | **SonarCloud + CodeQL** run on every PR, both configured outside the workflow files. Excluded from Sonar: `docs/**` (the vendored LEGO submodule — third-party HTML/JS the project neither owns nor fixes) and `Source/Trackify/**/*.cs` (the Uno app, which the build-less analysis cannot judge — [R-9](11-risks-and-technical-debt.md)). | | OC-5 | The vendored LEGO protocol documentation is a **git submodule**; it is read-only reference material. | ## 2.4 Self-imposed conventions (enforced by the build) diff --git a/docs/arc42/04-solution-strategy.md b/docs/arc42/04-solution-strategy.md index 28caad3..6c9fd99 100644 --- a/docs/arc42/04-solution-strategy.md +++ b/docs/arc42/04-solution-strategy.md @@ -86,7 +86,7 @@ restart and no UI changes. | Any warning at all | `TreatWarningsAsErrors` + `EnforceCodeStyleInBuild` | | Package version drift | Central Package Management (`Directory.Packages.props`) | | Uno version drift | `Uno.Sdk` pinned in `global.json`, not in package props | -| Bugs, coverage, security rating | SonarCloud (`sonar.yml`) + CodeQL, per PR | +| Bugs, security rating | SonarCloud automatic analysis + CodeQL, per PR (no coverage — see [R-9](11-risks-and-technical-debt.md)) | What *cannot* be enforced automatically is stated as a convention in [§2.4–2.5](02-architecture-constraints.md#24-self-imposed-conventions-enforced-by-the-build) and diff --git a/docs/arc42/07-deployment-view.md b/docs/arc42/07-deployment-view.md index 9dfd945..ab5f127 100644 --- a/docs/arc42/07-deployment-view.md +++ b/docs/arc42/07-deployment-view.md @@ -141,7 +141,9 @@ confirm that a failure really is the non-root switch. > `docker:S6471` as a scanner `/d:` parameter. It silently did nothing — issue exclusions are > multi-value *server-side* settings that only apply when configured in the SonarCloud UI. The > parameter was live for the analysis of `9299be9` and the issue still came back `OPEN`. Fixing the -> finding at the source was the durable answer. +> finding at the source was the durable answer. In hindsight it was doubly doomed: the scanner +> workflow carrying that parameter never published an analysis at all ([R-9](11-risks-and-technical-debt.md)), +> so nothing it passed could have taken effect either way. The scanner has since been removed. ## 7.4 Server mode (client/server) @@ -180,8 +182,8 @@ for a **trusted home LAN** and must not be exposed to the internet — see ```mermaid graph LR PR["PR → master"] --> CI["ci.yml
ubuntu-latest"] - PR --> SON["sonar.yml
scan + coverage"] - PR --> CQL["codeql.yml"] + PR --> SON["SonarCloud
automatic analysis"] + PR --> CQL["CodeQL
default setup"] TAG["tag v*"] --> APK["android-apk.yml
windows-latest"] TAG --> ARM["cli-arm64.yml
self-contained linux-arm64"] @@ -195,19 +197,25 @@ graph LR | Workflow | Trigger | Runner | Does | |---|---|---|---| | `ci.yml` | PR / push to `master` | ubuntu-latest | Builds the CLI + shared core, runs the tests. **The pre-merge gate** | -| `sonar.yml` | PR / push to `master` | ubuntu-latest | SonarCloud scan with coverage (OpenCover via coverlet). Deliberately separate from the gate so a Sonar outage cannot block a merge; skipped for fork PRs, which never receive `SONAR_TOKEN` | -| `codeql.yml` | PR / push | — | Security code scanning | | `android-apk.yml` | tag `v*` / manual | windows-latest | JDK 17 + `android` workload, `-f net10.0-android` → `trackify-apk` | | `cli-arm64.yml` | tag `v*` / manual | ubuntu-latest | `-f net10.0 -r linux-arm64 --self-contained` → `trackify-cli-linux-arm64` | +Two analyses run **without a workflow file in this repo**, so they have no `.yml` to read: + +| Analysis | Configured by | Notes | +|---|---|---| +| SonarCloud | [`.sonarcloud.properties`](../../.sonarcloud.properties) + the GitHub app | **Automatic analysis** — never builds, so it reports no coverage and analyses C# without a compilation. A scanner workflow cannot coexist with it → [R-9](11-risks-and-technical-debt.md) | +| CodeQL | GitHub **default setup** (`dynamic/github-code-scanning/codeql`) | Runs `csharp`, `javascript-typescript` and `actions`. Configured in the repo's Security settings, not in version control | + All workflows provision the .NET 8, 9 and 10 SDKs; `global.json` pins `9.0.100` with `rollForward: latestMajor`, so the newest installed major is used. **The Uno app is intentionally not built by `ci.yml`.** Its five heads trigger workload imports and OS-locked TFMs (iOS needs macOS, Windows needs Windows) during *restore*, even with `-f ` — so gating one head reliably would need a per-OS + workload matrix. The Android head is covered by -`android-apk.yml`; the remaining heads are verified locally. The same limitation scopes the -SonarCloud analysis to the shared core + CLI + tests. → [§11](11-risks-and-technical-debt.md) +`android-apk.yml`; the remaining heads are verified locally. The app is out of the SonarCloud scope +too, for a related but distinct reason: automatic analysis never builds anything, so its C# findings +on the app are noise rather than results. → [§11](11-risks-and-technical-debt.md) **Local verification** (what a change is expected to pass before it is pushed): @@ -222,10 +230,10 @@ Buildable heads outside macOS: `net10.0-android`, `net10.0-desktop`, `net10.0-br `net10.0-windows10.0.19041.0`. Real BLE behaviour is confirmed on a device and on a Pi — it cannot be exercised in CI. -**Checking SonarCloud's C# rules locally.** The scan needs a `SONAR_TOKEN` and only runs in CI, so a -fix to a Sonar finding is otherwise unverifiable before pushing — and while -[R-9](11-risks-and-technical-debt.md) stands, the published findings are not trustworthy anyway. The -same rules can be run offline by injecting the analyzer package without touching any repo file: write +**Checking SonarCloud's C# rules locally — the authoritative check.** Because the hosted analysis +never compiles the code ([R-9](11-risks-and-technical-debt.md)), its C# findings cannot be trusted and +a fix cannot be confirmed there. Run the same rules offline instead, by injecting the analyzer package +without touching any repo file: write ```xml diff --git a/docs/arc42/09-architecture-decisions.md b/docs/arc42/09-architecture-decisions.md index 793a310..96066aa 100644 --- a/docs/arc42/09-architecture-decisions.md +++ b/docs/arc42/09-architecture-decisions.md @@ -234,7 +234,9 @@ Windows) — **even with `-f `**, because restore imports workloads for al **Decision.** `ci.yml` gates the shared core + CLI + tests on ubuntu-latest. The Android head is built by `android-apk.yml` on windows-latest for tags. The other heads are verified locally by developers. -The same limitation scopes the SonarCloud analysis. +The app is also outside the SonarCloud scope, for a related but separate reason +([R-9](11-risks-and-technical-debt.md)): the analysis never builds, so its findings on the app are +artefacts of the missing compilation rather than results. **Consequences.** The gate is fast and reliable on a single runner. The gap is real and accepted: a change that breaks only the desktop, WASM or iOS head **can merge**. Mitigation is the documented diff --git a/docs/arc42/10-quality-requirements.md b/docs/arc42/10-quality-requirements.md index c495fbe..7798248 100644 --- a/docs/arc42/10-quality-requirements.md +++ b/docs/arc42/10-quality-requirements.md @@ -101,8 +101,8 @@ suspect. | Build health, warnings, code style | `TreatWarningsAsErrors` + `EnforceCodeStyleInBuild` | every build | | Layer integrity | NetArchTest architecture tests | `ci.yml` | | Logic correctness | xUnit tests foldered by layer | `ci.yml` | -| Coverage, bugs, security & maintainability rating | SonarCloud (coverlet → OpenCover) | `sonar.yml`, badges in the README | -| Security patterns | CodeQL | `codeql.yml` | +| Bugs, security & maintainability rating | SonarCloud automatic analysis | `.sonarcloud.properties`, badges in the README | +| Security patterns | CodeQL | GitHub default setup (no workflow file) | | Control latency, BLE behaviour, UI rendering | **Manual, on real hardware** | phone + Raspberry Pi | The last row is the honest gap: the top-priority quality goal (control responsiveness) has **no diff --git a/docs/arc42/11-risks-and-technical-debt.md b/docs/arc42/11-risks-and-technical-debt.md index 1756489..944037b 100644 --- a/docs/arc42/11-risks-and-technical-debt.md +++ b/docs/arc42/11-risks-and-technical-debt.md @@ -89,33 +89,39 @@ have no hub address recorded yet will be merged into one local row. - **Realistic trigger:** trains created in the app before discovery has run, then synced from a Pi. - **Fix if it bites:** drop the name fallback and require a hub identity for matching. -### R-9: `sonar.yml` has never published an analysis (high impact, needs a one-off admin action) - -The project still has **Automatic Analysis** enabled server-side (`sonar.autoscan.enabled=true`), and -the two modes are mutually exclusive: every `sonarscanner end` step is rejected with *"You are running -CI analysis while Automatic Analysis is enabled"*, so the workflow has failed on **every** run to date -while the `begin` and test steps pass. Three consequences, all of them easy to misread: - -- **Coverage is not measured at all.** The OpenCover report is produced and never consumed, so the - coverage metric and its badge come from nothing. -- **Every `/d:sonar.*` setting in `sonar.yml` is inert**, including the scope exclusions. What actually - scopes the published results is [`.sonarcloud.properties`](../../.sonarcloud.properties), the file - Automatic Analysis reads — which is why the two exclusion lists must be kept identical. -- **The published C# findings are analysed without a compilation.** Source generators have not run and - package references are unresolved, so the results are unreliable in exactly the places that depend on - either — most visibly the Uno app, where every read of a `[ObservableProperty]`-generated property - and every `IValueConverter` member drew a bogus `S2325` "make it static", and `S8970` claimed nullable - warnings were disabled although `Directory.Build.props` sets `enable`. Applying - those "fixes" would not compile. The app is excluded for this reason. - -- **Mitigation today:** C# rules are reproduced locally against the same analyzer version by injecting +### R-9: SonarCloud runs without a compilation, so its C# findings are weak (accepted) + +The project uses SonarCloud's **Automatic Analysis** (`sonar.autoscan.enabled=true`). That mode and a +CI scanner are mutually exclusive — `sonarscanner end` is rejected with *"You are running CI analysis +while Automatic Analysis is enabled"* — and switching between them is a **server-side setting that no +file in this repository can change**. A `.github/workflows/sonar.yml` did exist for exactly that +purpose; because the setting was never flipped it failed on **every** run it ever made, published +nothing, and was removed rather than left as a permanently red check. Two things follow: + +- **Coverage is not measured.** Automatic analysis never runs the tests, so per its documented + limitations coverage is unsupported. The metric has in fact never had a value, and the README + coverage badge was removed for that reason. +- **C# is analysed without a compilation.** Source generators have not run and package references are + unresolved, so findings are unreliable wherever either matters — most visibly the Uno app, where + every read of an `[ObservableProperty]`-generated property and every `IValueConverter` member drew a + bogus `S2325` "make it static", and `S8970` claimed nullable warnings were disabled although + `Directory.Build.props` sets `enable`. Applying those "fixes" would not compile. + The app is excluded from analysis for this reason — see + [`.sonarcloud.properties`](../../.sonarcloud.properties), now the only Sonar configuration in the repo. + +What is still worth having: the non-C# analysers (Docker, YAML, XML, JS, secrets) do not need a build +and are unaffected, and the quality-gate badges stay meaningful for those. + +- **Mitigation:** C# rules are reproduced locally against the same analyzer version by injecting `SonarAnalyzer.CSharp` through `CustomAfterMicrosoftCommonProps` — - see [§7.6](07-deployment-view.md#76-build-and-delivery-pipeline). -- **Fix:** a project admin turns Automatic Analysis off once, at - [Project Settings → Analysis Method](https://sonarcloud.io/project/settings?id=Ktechen_Trackify). - Nothing in this repository can do it — it is a server-side setting, and a `sonar-project.properties` - file (sometimes suggested) is rejected outright by the scanner for .NET. After the switch the CI scan - is accepted, coverage starts reporting, and the workflow's own exclusions take over. + see [§7.6](07-deployment-view.md#76-build-and-delivery-pipeline). This is the accurate view of the C# + rules, and it should be trusted over the SonarCloud UI. +- **Reverse it** by turning Automatic Analysis off at + [Project Settings → Analysis Method](https://sonarcloud.io/project/settings?id=Ktechen_Trackify) and + restoring a scanner workflow (`git log -- .github/workflows/sonar.yml` has a working one, including + the coverlet → OpenCover coverage wiring). Do both or neither: a scanner workflow without the setting + change is dead weight, which is what it was. Note a `sonar-project.properties` file, often suggested + for this, is rejected outright by the scanner for .NET. ## 11.2 Technical debt