From 60eaed3096486b8c14b7100f48ea7ca469daee56 Mon Sep 17 00:00:00 2001 From: bluezr Date: Wed, 5 Aug 2026 15:21:46 -0700 Subject: [PATCH 1/2] ci: gate cppcheck on error severity, as the workflow already documents The cppcheck job fails on 0.1.5-dev itself. Reproduced on an untouched checkout, so it is red for every open pull request and none of them caused it. The header of the workflow states the phase-0 policy plainly: Gating policy (phase 0): only `error` severity fails the job. The full report (all severities, CWE IDs included) is uploaded as an artifact for triage. Gating widens to `warning` once the initial backlog is dispositioned in cppcheck-suppressions.txt. but the step named "gate on error severity" invoked cppcheck with --enable=warning --error-exitcode=1, so warnings were fatal too. On the CI image (ubuntu-24.04, cppcheck 2.13) that is 26 warnings against 2 errors. Drops --enable=warning from the gating step only. The reporting step above still runs --enable=warning,style,performance,portability and uploads the full XML, and the CWE summary step is untouched, so nothing stops being visible -- it stops blocking. That leaves the two error-severity findings, both false positives, both in OP-TEE code and both now suppressed with a rationale as the suppressions file requires: - src/optee/host/main.c: `seed` is passed as a TEEC_MEMREF_TEMP_OUTPUT buffer that the trusted application fills. cppcheck reads taking its address as a use of an uninitialised value. - src/optee/ta/libdogecoin_ta.c: cppcheck cannot build an AST for assert() as expanded there. Deliberately not suppressed: three further internalAstError hits on HASH_DEL that cppcheck 2.7 reports locally and 2.13 on the CI image does not. Suppressing findings the gate does not produce would be writing the file against a stale tool version. Worth noting 2.13 already cleared three of the four internalAstError that 2.7 reports, so a version bump beats a suppression here; pinning the cppcheck version so local and CI agree is a sensible follow-up, since `apt-get install cppcheck` tracks whatever the runner image ships. The backlog this leaves for phase 1, from the CI run: 18 invalidPrintfArgType_sint (%d with unsigned -- real, mechanical, should be fixed rather than suppressed), 4 nullPointerRedundantCheck and 2 ctunullpointer in wallet.c/utils.c which need actual review since wallet.c dereferences tx before the !tx check, and 1 literalWithCharPtrCompare in bip32.c. The gate has already paid for itself: it is what surfaced the unbounded fscanf fixed in #401, which had been shipping since v0.1.2. --- .github/workflows/cppcheck.yml | 9 ++++++++- contrib/analysis/cppcheck-suppressions.txt | 11 +++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/.github/workflows/cppcheck.yml b/.github/workflows/cppcheck.yml index 8cca70113..0e6f31c17 100644 --- a/.github/workflows/cppcheck.yml +++ b/.github/workflows/cppcheck.yml @@ -79,9 +79,16 @@ jobs: path: cppcheck-report.xml - name: gate on error severity + # Only `error` severity is fatal, per the phase-0 policy at the top of + # this file. The step previously passed --enable=warning here, which + # made every warning fatal too and left the job red on 0.1.5-dev itself. + # The full warning/style/performance/portability set is still produced + # by the reporting step above and uploaded as an artifact -- nothing + # stops being visible, it just stops blocking. Widen this to + # --enable=warning once the backlog in the suppressions file is + # dispositioned, which is what the header has always intended. run: | cppcheck \ - --enable=warning \ --std=c99 \ -D__GNUC__ -D__linux__ \ --inline-suppr \ diff --git a/contrib/analysis/cppcheck-suppressions.txt b/contrib/analysis/cppcheck-suppressions.txt index 9278d5925..03103173f 100644 --- a/contrib/analysis/cppcheck-suppressions.txt +++ b/contrib/analysis/cppcheck-suppressions.txt @@ -16,3 +16,14 @@ unusedFunction // Informational check with a very high false-positive rate on C99 designated // initializers; revisit after the Phase 1 backlog triage. missingIncludeSystem + +// OP-TEE client passes `seed` as a TEEC_MEMREF_TEMP_OUTPUT buffer, which the +// trusted application fills; cppcheck reads taking its address as a use of an +// uninitialised value. disposition: false-positive +uninitvar:src/optee/host/main.c + +// cppcheck 2.13 cannot build an AST for the assert() macro as expanded here; +// this is a parser limitation, not a defect in the code. Re-check on the next +// cppcheck bump -- 2.13 already cleared three of the four such errors that +// 2.7 reported. disposition: false-positive +internalAstError:src/optee/ta/libdogecoin_ta.c From e0a56b442e14e8676de9993fad7eeb7e67852130 Mon Sep 17 00:00:00 2001 From: bluezr Date: Wed, 5 Aug 2026 16:57:31 -0700 Subject: [PATCH 2/2] ci: run cppcheck and clang-tidy on pull requests at all Both jobs filter on '*-dev-*', for push and for pull_request. That pattern never matches `0.1.5-dev` itself, because there is nothing after "-dev". The consequence is that neither has ever run on a pull request upstream: the only runs are pushes to fork branches that happen to carry a suffix, so the failures land in one author's inbox and gate nothing. This is the same defect #377 fixes in ql.yml, and the same one #371 already fixed in ci.yml -- `*` does not cross `/`, and `*-dev-*` requires a trailing segment that the release branch does not have. All three phase-0 static-analysis workflows shipped with it. Aligns both with ci.yml: push on '*-dev*' (which matches the bare release branch as well as suffixed work branches) and pull_request on '*', because a pull request must be analysed whichever branch it targets. Without this, narrowing the severity gate in the previous commit would make the job green somewhere nobody is looking. --- .github/workflows/clang-tidy.yml | 9 +++++++-- .github/workflows/cppcheck.yml | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml index c9f7028af..27dd48233 100644 --- a/.github/workflows/clang-tidy.yml +++ b/.github/workflows/clang-tidy.yml @@ -12,11 +12,16 @@ name: clang-tidy on: push: branches: - - '*-dev-*' + - '*-dev*' - 'main' pull_request: + # Deliberately wider than the push filter above: a pull request must be + # analysed whichever branch it targets. '*-dev-*' never matched + # `0.1.5-dev` itself -- there is nothing after "-dev" -- so this job has + # never run on a pull request upstream, only on pushes to fork branches + # that happen to carry a suffix. Matches ci.yml's filter. branches: - - '*-dev-*' + - '*' paths-ignore: - '**/*.md' - '**/*.txt' diff --git a/.github/workflows/cppcheck.yml b/.github/workflows/cppcheck.yml index 0e6f31c17..b0f3685d2 100644 --- a/.github/workflows/cppcheck.yml +++ b/.github/workflows/cppcheck.yml @@ -14,11 +14,16 @@ name: cppcheck on: push: branches: - - '*-dev-*' + - '*-dev*' - 'main' pull_request: + # Deliberately wider than the push filter above: a pull request must be + # analysed whichever branch it targets. '*-dev-*' never matched + # `0.1.5-dev` itself -- there is nothing after "-dev" -- so this job has + # never run on a pull request upstream, only on pushes to fork branches + # that happen to carry a suffix. Matches ci.yml's filter. branches: - - '*-dev-*' + - '*' paths-ignore: - '**/*.md' - '**/*.txt'