From b3a4d5b45ad807af1e6eae8ff75ddee485669085 Mon Sep 17 00:00:00 2001 From: Stuart Meeks Date: Sat, 22 Aug 2026 00:35:37 +0000 Subject: [PATCH 1/2] fix: resolve CodeQL source findings (cs/complex-condition, cs/local-not-disposed) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the hand-rolled hex-digit range check in SplashColors.ValidateHex with char.IsAsciiHexDigit — same result, no complex condition, and the canonical BCL method on both net8.0 and net10.0. Dispose the two TestConsole instances in SplashScreenTests via `using`; TestConsole is IDisposable and both were leaking. Behaviour-preserving: no public API or rendered output changed. The two remaining open alerts are in xUnit's auto-generated obj/ entry point and are pending a Standards-level codeql.yml change (paths-ignore is inert for built C#). Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 10 ++++++++++ .../SplashColors.cs | 3 +-- .../SplashScreenTests.cs | 4 ++-- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddca025..ba68904 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **Two CodeQL `security-and-quality` findings resolved with genuine changes.** + `SplashColors.ValidateHex` no longer hand-rolls the hex-digit range test + (`cs/complex-condition`): the three chained comparisons are replaced by the canonical + `char.IsAsciiHexDigit`, which is available on both shipped target frameworks and reads + as the intent. The two `TestConsole` instances in `SplashScreenTests` are now disposed + via `using` (`cs/local-not-disposed`); `TestConsole` is `IDisposable` and both were + leaking. No behaviour, public API or rendered output changed. + --- ## [1.0.0] — 2026-08-21 diff --git a/src/NextIteration.SpectreConsole.Splash/SplashColors.cs b/src/NextIteration.SpectreConsole.Splash/SplashColors.cs index d6bfd4c..14499f1 100644 --- a/src/NextIteration.SpectreConsole.Splash/SplashColors.cs +++ b/src/NextIteration.SpectreConsole.Splash/SplashColors.cs @@ -57,8 +57,7 @@ private static void ValidateHex(string hex) for (var i = 1; i < 7; i++) { var c = hex[i]; - var isHex = (c >= '0' && c <= '9') || (c >= 'A' && c <= 'F') || (c >= 'a' && c <= 'f'); - if (!isHex) + if (!char.IsAsciiHexDigit(c)) { throw new ArgumentException( $"Hex colour '{hex}' contains non-hex character '{c}' at position {i}.", nameof(hex)); diff --git a/tests/NextIteration.SpectreConsole.Splash.Tests/SplashScreenTests.cs b/tests/NextIteration.SpectreConsole.Splash.Tests/SplashScreenTests.cs index 97e1219..4905c19 100644 --- a/tests/NextIteration.SpectreConsole.Splash.Tests/SplashScreenTests.cs +++ b/tests/NextIteration.SpectreConsole.Splash.Tests/SplashScreenTests.cs @@ -25,7 +25,7 @@ public void Show_writes_to_AnsiConsole() // Redirecting AnsiConsole.Console to a test console lets us // confirm the single-Markup call actually produces output. var prev = AnsiConsole.Console; - var test = new TestConsole(); + using var test = new TestConsole(); AnsiConsole.Console = test; try { @@ -49,7 +49,7 @@ public void Show_writes_to_AnsiConsole() public void Show_with_custom_tagline_emits_it() { var prev = AnsiConsole.Console; - var test = new TestConsole().Width(120); + using var test = new TestConsole().Width(120); AnsiConsole.Console = test; try { From 5787e05f1b1aa9cd14f233164d495326ff2fc472 Mon Sep 17 00:00:00 2001 From: Stuart Meeks Date: Sat, 22 Aug 2026 00:55:45 +0000 Subject: [PATCH 2/2] ci: align codeql.yml with updated Standards template (build-mode: none) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The explicit-build CodeQL workflow applied no path filter to the compiled C#, so the repo's paths-ignore: **/obj/** was inert and the xUnit auto-generated entry point in obj/ was analysed and flagged (two cs/missed-ternary-operator alerts). The updated §4.4 template switches to build-mode: none, under which buildless extraction honours paths-ignore, so the exclusion actually takes effect — and it reads source across all TFMs at once instead of one build's output. File is the template verbatim; Setup .NET / Restore / Build steps removed. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/codeql.yml | 38 ++++++++++++++++++++---------------- CHANGELOG.md | 12 ++++++++++++ 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 4e37bc7..776630f 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -32,23 +32,36 @@ jobs: - name: Checkout uses: actions/checkout@v7 - - name: Setup .NET - uses: actions/setup-dotnet@v6 - with: - dotnet-version: | - 8.0.x - 10.0.x - - name: Initialize CodeQL uses: github/codeql-action/init@v4 with: languages: csharp + # build-mode: none analyses the C# source directly, without a build. + # It is load-bearing, not a convenience, for two reasons: + # + # 1. paths-ignore (below) only takes effect in this mode. When CodeQL + # builds a compiled language, GitHub applies no path filter — every + # file the compiler sees is analysed, obj/ included — so under the + # explicit build this workflow used to run, paths-ignore was + # silently inert and the xUnit auto-generated entry point in obj/ + # was analysed and flagged in every repo. Buildless extraction + # honours the filter, so the exclusion the standard mandates + # actually happens. + # + # 2. It reads the source across every target framework at once. These + # repos multi-target, and autobuild has picked a single TFM in the + # past, silently analysing half the code; the explicit build existed + # to guard against that. Buildless extraction reads the source + # itself, not one TFM's build output, so it covers all of it with no + # build step to get wrong. + build-mode: none # security-and-quality is broader than the default security-extended; # these are small libraries, so the extra findings are affordable. queries: security-and-quality # Analyse source only. obj/ and bin/ hold generated and compiled # output — e.g. the xUnit auto-generated entry point — so findings - # there are noise against code no human maintains. + # there are noise against code no human maintains. Effective only + # under build-mode: none (above). # # query-filters excludes the two audit queries that fire on every # P/Invoke declaration and call site (cs/unmanaged-code, @@ -68,15 +81,6 @@ jobs: - exclude: id: cs/call-to-unmanaged-code - # Explicit build rather than autobuild: these repos multi-target, and - # autobuild has picked a single TFM in the past, silently analysing half - # the code. Restore is separate so a restore failure is legible. - - name: Restore - run: dotnet restore - - - name: Build - run: dotnet build --configuration Release --no-restore - - name: Perform CodeQL analysis uses: github/codeql-action/analyze@v4 with: diff --git a/CHANGELOG.md b/CHANGELOG.md index ba68904..87da8e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 via `using` (`cs/local-not-disposed`); `TestConsole` is `IDisposable` and both were leaking. No behaviour, public API or rendered output changed. +### Changed + +- **CodeQL now analyses with `build-mode: none`, per the updated `codeql.yml` template + (§4.4).** The workflow previously ran an explicit `dotnet build` before analysis, under + which GitHub applies no path filter to a compiled language — so the `paths-ignore: + **/obj/**` this repo already carried was silently inert, and CodeQL flagged the xUnit + auto-generated entry point in `obj/` (two `cs/missed-ternary-operator` alerts against + code no human maintains). Buildless extraction honours `paths-ignore`, so that + exclusion now actually takes effect, and it reads the source across every target + framework at once rather than one TFM's build output. The file is the Standards + template verbatim; the Setup .NET / Restore / Build steps are gone. + --- ## [1.0.0] — 2026-08-21