From 7b8cdeba1896084b9285afedeb4362f9246eddef Mon Sep 17 00:00:00 2001 From: Alpaq92 Date: Tue, 4 Aug 2026 10:25:39 +0200 Subject: [PATCH] docs: correct analyzer-promotion, release trigger, and TPS003 references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three claims in the doc set did not match the repo: - README (CI-matrix row and maintainer notes) and the AnalyzerReleases.Unshipped.md runbook all state that release.yml auto-promotes analyzer rules into Shipped.md via a "Promote analyzer rules to Shipped" step. No such step exists — release.yml's own header comment describes promotion as a manual maintainer commit, and that is what actually happens. Shipped.md is empty while TPS001-TPS003 have shipped in released NuGets from Unshipped.md, which is fine: rules are valid there indefinitely. The runbook's RS2007 claim is corrected too — RS2007 fires for an implemented rule that is listed in neither file, not for one that sits unshipped. Also notes that promotion should use the version that first shipped each rule rather than sweeping all three under whatever release is being cut. - The CI-matrix row listed release.yml's trigger as "scheduled"; it is `push` to master (plus workflow_dispatch), and only the release PR merging reaches the pack-and-publish job. - TPS003 shipped in 2.4.0 but README (repo layout, FAQ 12) and CONTRIBUTING still listed the analyzer as TPS001/TPS002 only. Adds a maintainer note for the codeql-action init/analyze SHA-pair invariant and the dependabot group that now enforces it. Co-Authored-By: Claude Opus 5 --- CONTRIBUTING.md | 2 +- README.md | 9 +++--- .../AnalyzerReleases.Unshipped.md | 32 +++++++++++-------- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 89eb48b..304e4a6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,7 +25,7 @@ A solution-wide build attempts every TFM and needs all workloads; CI builds leaf | --- | --- | | `TopSecret.ProtectedString` | Core library (multi-TFM). | | `TopSecret.ProtectedBlob` | Large-blob sibling package (net10.0; consumes core internals via `InternalsVisibleTo`). | -| `TopSecret.ProtectedString.Analyzers` | Roslyn analyzer (TPS001/TPS002), packed inside both library NuGets. | +| `TopSecret.ProtectedString.Analyzers` | Roslyn analyzer (TPS001/TPS002/TPS003), packed inside both library NuGets. | | `TopSecret.ProtectedString.{WindowsTpm,LinuxTpm}` | Optional TPM 2.0 key-wrap providers. | | `TopSecret.ProtectedString.Configuration` | Optional `IConfiguration` binder. | | `*.Tests` | NUnit 4 suites, one per package. | diff --git a/README.md b/README.md index fc20bcd..d2848da 100644 --- a/README.md +++ b/README.md @@ -879,7 +879,7 @@ GitHub-hosted runners exist for **Windows, Linux, and macOS only** — not for i | --- | --- | --- | --- | | `ci.yml` | push / pull_request | `net10.0` on all three OSes; `net10.0-macos` and `net10.0-maccatalyst` on the macOS leg | These TFMs target platforms with native runners (Windows / Linux / macOS), so the CI compile-check is matched by the same OS we'd run tests on. The macOS leg is pinned to `macos-14` for stable Apple-Silicon SEP availability. Tests run, AOT publish dry-runs on Linux, TPM smoke suites on Windows and Linux. | | `build-platform-tfms.yml` | manual (`workflow_dispatch`) | `net10.0-ios` (macOS), `net10.0-android` (Linux), `net10.0-browser` (Linux) | These TFMs target platforms with no GitHub-hosted runner. Building them is cross-compilation only — no test value beyond the compile-check — so they live behind a manual trigger to keep per-PR pipeline cost down. | -| `release.yml` | scheduled / `workflow_dispatch` | every TFM (`net10.0;net10.0-android;net10.0-ios;net10.0-macos;net10.0-maccatalyst;net10.0-browser`) on `macos-14` | Release-time build runs natively on the only runner that has every Apple SDK + the wasm-tools + android workloads. The published NuGet contains all six platform-specific binaries; running on `macos-14` is the path that produces them all. Also auto-promotes `AnalyzerReleases.Unshipped.md` rules into `Shipped.md` under the new version's heading before the build, so RS2007 doesn't fail the analyzer build. | +| `release.yml` | push to `master` / `workflow_dispatch` | every TFM (`net10.0;net10.0-android;net10.0-ios;net10.0-macos;net10.0-maccatalyst;net10.0-browser`) on `macos-14` | Release-time build runs natively on the only runner that has every Apple SDK + the wasm-tools + android workloads. The published NuGet contains all six platform-specific binaries; running on `macos-14` is the path that produces them all. Every push to `master` runs release-please, which maintains the release PR; the pack-and-publish job only runs once that PR merges (or when dispatched manually with an existing tag). | The `ci.yml` Linux leg additionally runs an AOT publish dry-run with warnings escalated to errors (catches IL2026 / IL2050 / IL3050 / transitive trim warnings before they ship), and the Linux TPM smoke tests run against a `swtpm` software TPM 2.0 simulator for end-to-end coverage of the TSS.MSR call shape. @@ -1132,7 +1132,7 @@ Materializing a `string` inside `Access` is the **best** the library can do at a What you do get by keeping the string narrow: the `ProtectedString` itself stays encrypted at rest between uses, the `string` is unreachable as soon as the surrounding scope exits, and a heap dump captured *between* operations sees only ciphertext. -#### 12. My analyzer is firing TPS001 / TPS002. How do I tell what's wrong? +#### 12. My analyzer is firing TPS001 / TPS002 / TPS003. How do I tell what's wrong? The most common cause is `new string(plain)` or `plain.ToString()` inside an `Access` callback — the resulting `string` is on the heap, unwipeable, and visible to a heap dump. Replace it with a `Span` sink (`CopyTo`, `WriteUtf8To`) or transform the data inside the callback. See [Build-time analyzer](#build-time-analyzer) for the full trigger list and unavoidable-boundary suppressions. @@ -1152,7 +1152,8 @@ This library was built with an audit-driven workflow assisted by an AI pair prog ### Maintainer notes — version pins worth knowing - **`Microsoft.CodeAnalysis.CSharp` in the analyzer project is pinned to a Roslyn version that requires a matching SDK floor.** An analyzer that references a *newer* Roslyn than the host compiler fails to load with `CS9057` ("analyzer references compiler version X newer than running version Y"), breaking every consumer's build. Current pin: **5.6.0**, bumped 2026-07-04 from the prior 5.3.0 pin. There is no cleanly documented public mapping from a Roslyn package version to its minimum host SDK, so the floor was set empirically — verified by a full solution rebuild with the analyzer self-applying via `ProjectReference` (the actual CS9057 failure path, not just the analyzer project's own build) — rather than assumed. The repo's `global.json` is set to `"version": "10.0.300"` with `rollForward: latestFeature`, so a contributor on an older SDK gets a clear "SDK 10.0.300 not found" error at restore time instead of a cryptic CS9057 mid-build. Consumers of the published NuGet must also be on SDK 10.0.300+ for the analyzer to load. Bumping the Roslyn pin further requires a corresponding SDK-floor bump (verified the same way) and a major-version release of this package (consumer-breaking change). **Enforced by `.github/dependabot.yml`**, which ignores `version-update:semver-minor` and `version-update:semver-major` bumps for that package — patch bumps inside the pinned major.minor still propose PRs. -- **Analyzer release-tracking files** (`AnalyzerReleases.Shipped.md`, `AnalyzerReleases.Unshipped.md`) are auto-promoted by `release.yml`. The runbook is in `AnalyzerReleases.Unshipped.md`'s header comment. Never hand-edit `Shipped.md` after a release lands; only add new rules to `Unshipped.md`. +- **Analyzer release-tracking files** (`AnalyzerReleases.Shipped.md`, `AnalyzerReleases.Unshipped.md`) are promoted **by hand**, not by `release.yml` — there is no workflow step that moves them, and rules left in `Unshipped.md` stay valid for the build indefinitely, so nothing fails if a release ships without promoting. Promotion is a deliberate maintainer commit folded into the release PR before it merges; the runbook is in `AnalyzerReleases.Unshipped.md`'s header comment. Never hand-edit `Shipped.md` after a release lands; only add new rules to `Unshipped.md`. +- **`github/codeql-action/init` and `github/codeql-action/analyze` must be pinned to the same SHA.** `init` writes a version-stamped config that `analyze` reads back, and a mismatch fails the job outright: *"Loaded a configuration file for version '4.37.4', but running version '4.36.3'"*. Dependabot treats the two sub-actions as separate dependencies and, left alone, opens one PR per half — each individually red, neither individually wrong. **Enforced by `.github/dependabot.yml`**, which groups `github/codeql-action*` into a single PR so the pair always moves together. Current pin: **v4.37.5** (both), set 2026-08-04 after that exact split happened. - **`coverlet.msbuild` requires re-building** (no `--no-build`) because instrumentation runs at build time. CI's coverage step is ordered last on the Linux leg specifically so the instrumented binaries don't affect downstream test steps. If you reorder, keep it last. ## Inspiration @@ -1207,7 +1208,7 @@ References used while auditing this implementation. Everything beyond *"this enc ``` TopSecret.ProtectedString/ # main cross-platform library (NuGet) -TopSecret.ProtectedString.Analyzers/ # Roslyn analyzer (TPS001 / TPS002), packed into the main NuGet +TopSecret.ProtectedString.Analyzers/ # Roslyn analyzer (TPS001 / TPS002 / TPS003), packed into the main NuGet TopSecret.ProtectedString.Configuration/ # optional appsettings.json binder subpackage (NuGet) TopSecret.ProtectedString.Json/ # optional System.Text.Json converter subpackage (NuGet) TopSecret.ProtectedString.Xml/ # optional XmlReader helper subpackage (NuGet) diff --git a/TopSecret.ProtectedString.Analyzers/AnalyzerReleases.Unshipped.md b/TopSecret.ProtectedString.Analyzers/AnalyzerReleases.Unshipped.md index 1a677a0..0db7b3d 100644 --- a/TopSecret.ProtectedString.Analyzers/AnalyzerReleases.Unshipped.md +++ b/TopSecret.ProtectedString.Analyzers/AnalyzerReleases.Unshipped.md @@ -4,23 +4,29 @@ ; ───────────────────────────────────────────────────────────────────────── ; Release-time runbook ; ───────────────────────────────────────────────────────────────────────── -; When cutting a NuGet release, every rule listed below must move into -; AnalyzerReleases.Shipped.md under a `## Release X.Y.Z` heading, and this -; file must be left empty (with the header preserved). Forgetting either -; step makes the next analyzer build fail with diagnostic RS2007 (or -; RS2008 for "rule changed without release"). +; What actually fails the build is an implemented rule that is *listed +; nowhere*: RS2007 ("missing or invalid entry") fires when a DiagnosticId +; the analyzer reports has no row in either tracking file, and RS2008 when +; a shipped rule's severity/category changes without a new release entry. +; Rules sitting in *this* file are fully valid for the build and may stay +; here indefinitely — TPS001-TPS003 have shipped in released NuGets from +; here, and nothing broke. ; -; The release.yml workflow performs this move automatically before -; `dotnet pack` — see the `Promote analyzer rules to Shipped` step. -; That step is idempotent: a re-run on the same version shifts nothing -; if the rules are already in Shipped.md under the matching heading. If -; you are cutting a release manually (no workflow), run: +; Promoting them into AnalyzerReleases.Shipped.md under a `## Release X.Y.Z` +; heading is therefore a deliberate maintainer choice, not a hard gate. No +; workflow does it for you: release.yml has no promotion step, so a release +; that should record its rules as shipped needs this done by hand and +; folded into the release PR before it merges. Promote under the version +; that first shipped each rule — do not sweep long-standing rules under +; whatever version happens to be cutting now, since that misdates them. ; -; PWSH: +; To promote by hand: ; $unshipped = "TopSecret.ProtectedString.Analyzers/AnalyzerReleases.Unshipped.md" ; $shipped = "TopSecret.ProtectedString.Analyzers/AnalyzerReleases.Shipped.md" -; # 1. Append "## Release vX.Y.Z" + the rules block to $shipped -; # 2. Truncate $unshipped to just the header lines (lines starting with `;`) +; # 1. Append "## Release X.Y.Z" + a `### New Rules` block with the rows +; # that first shipped in X.Y.Z to $shipped +; # 2. Remove those rows from $unshipped, preserving the `;` header and +; # the `### New Rules` table header ; # 3. Commit ; ; ─────────────────────────────────────────────────────────────────────────