From 190abec5348f508662544c142c90267393c332f1 Mon Sep 17 00:00:00 2001 From: Sadeq Abu-Hattem Date: Sat, 10 Oct 2026 21:58:02 +0300 Subject: [PATCH 1/2] Run CodeQL's security queries, not security-and-quality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code scanning posts its findings as pull request review comments, and the ruleset on main sets required_review_thread_resolution, so every PR arrived with the merge button already blocked by whatever the quality suite had to say. #7 carried eleven such threads. Three were real and fixed; the other eight were deliberate code or false positives and had to be dismissed by hand: - three "call to obsolete method" on calls inside a #pragma warning disable CS0618 whose entire purpose is to exercise the obsolete forwarder - two "useless upcast" where the cast is load-bearing, because Task.FromResult infers the wrong tuple type without it - one "missed opportunity to use Where" - one "generic catch clause" on the observer catch that is documented as logged-and-swallowed by design - one "string concatenation in loop" in a test helper That is eight dismissals per PR as the steady state, which trains everyone to click through the one list that is supposed to be read. This narrows what runs, not what is reported, so it is a real loss and worth naming: the bare-catch bug fixed in 0f2ef53 — a write replacing an undecryptable secret with its default — was found by a quality query, not a security one. Against that, the quality rules are the analyser's weakest signal here, the compiler already enforces more than they do (warnings-as-errors, AnalysisLevel latest-recommended, and CA rules that rejected a return type during this very change), and the friction landed on every PR rather than occasionally. If the quality queries are wanted back, the better shape is to keep them and stop them blocking: a codeql-config with query-filters excluding the specific noisy rules, or paths-ignore for tests/, which is where seven of the eight sat. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e1fa294..1a03de3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -259,8 +259,18 @@ jobs: 8.0.x 10.0.x + # security, not security-and-quality. The quality half posts its findings as pull request + # review comments, and the ruleset on main requires every conversation resolved, so each PR + # arrived pre-blocked by style notes that had to be dismissed by hand — eleven of them on + # #7, of which eight were deliberate code or false positives. + # + # This narrows what runs rather than what is reported, so it is a real loss: the bare-catch + # bug fixed in 0f2ef53 was found by a quality query, not a security one. The trade accepted + # here is that quality rules are the analyser's weakest signal on this codebase and the + # friction was landing on every PR, while the compiler already enforces more than CodeQL's + # quality suite does — warnings-as-errors with latest-recommended analysis. - uses: github/codeql-action/init@v3 - with: { languages: csharp, queries: security-and-quality } + with: { languages: csharp, queries: security } - run: dotnet restore - run: dotnet build --configuration Release --no-restore From e6be2665987924f19c478734e1038c2fb7764464 Mon Sep 17 00:00:00 2001 From: Sadeq Abu-Hattem Date: Sat, 10 Oct 2026 22:01:48 +0300 Subject: [PATCH 2/2] Select the default suite by omitting queries, not by naming it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 190abec passed `queries: security`, which fails init outright: A fatal error occurred: Query pack security cannot be found. Check the spelling of the pack. There is no pack by that name. The named suites are security-extended, security-and-quality and security-experimental; the security-only suite is CodeQL's default and is selected by passing no `queries` input at all. A wrong name is not ignored and does not fall back — unlike the minimumCoverage input two commits earlier, which GitHub dropped with a warning. Here the whole job failed in 22 seconds, which is the better behaviour and is how this was caught. security-extended would also have excluded the quality rules, but it adds lower-precision security queries, which is new noise in place of the noise being removed. The default suite is the one that matches the intent. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1a03de3..5fbfe24 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -259,18 +259,23 @@ jobs: 8.0.x 10.0.x - # security, not security-and-quality. The quality half posts its findings as pull request - # review comments, and the ruleset on main requires every conversation resolved, so each PR - # arrived pre-blocked by style notes that had to be dismissed by hand — eleven of them on - # #7, of which eight were deliberate code or false positives. + # No `queries:` on purpose — that selects CodeQL's default suite, which is the + # security-focused one. There is no pack named `security` to ask for: the named suites are + # security-extended, security-and-quality and security-experimental, and naming a + # non-existent one fails init outright rather than falling back. # - # This narrows what runs rather than what is reported, so it is a real loss: the bare-catch - # bug fixed in 0f2ef53 was found by a quality query, not a security one. The trade accepted - # here is that quality rules are the analyser's weakest signal on this codebase and the - # friction was landing on every PR, while the compiler already enforces more than CodeQL's - # quality suite does — warnings-as-errors with latest-recommended analysis. + # This replaces security-and-quality, whose quality half posts findings as pull request + # review comments. The ruleset on main requires every conversation resolved, so each PR + # arrived pre-blocked by style notes to dismiss by hand — eleven on #7, of which eight were + # deliberate code or false positives. + # + # It narrows what runs rather than what is reported, so it is a real loss: the bare-catch + # bug fixed in 0f2ef53 was found by a quality query, not a security one. The trade is that + # quality rules are the analyser's weakest signal here and the friction landed on every PR, + # while the compiler already enforces more than they do — warnings-as-errors with + # latest-recommended analysis. - uses: github/codeql-action/init@v3 - with: { languages: csharp, queries: security } + with: { languages: csharp } - run: dotnet restore - run: dotnet build --configuration Release --no-restore