Fix four review defects, two CI findings, and the P4 boot loop #68
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| name: CodeQL | |
| # Layer 4 of the analysis stack (docs/testing.md § Static analysis) — the only one that sees | |
| # whole-program taint. It earns its place on one question: we parse six network packet formats | |
| # (ArtNet, DDP, E1.31, WLED audio sync, MQTT, WLED) plus HTTP, doing ~22 memcpy operations on | |
| # data arriving from the LAN, on a device with no MMU and no process isolation. Nothing else in | |
| # the stack looks for that class of bug — the compiler checks effects, lizard counts branches, | |
| # the Python checks read text. | |
| # | |
| # It has paid for itself: 3 critical `localtime` findings in main_desktop.cpp (shared static, | |
| # not thread-safe), fixed via a portable isoTimestamp helper. The six packet parsers produced | |
| # no taint-flow findings, which is positive evidence nothing else in the stack could give. | |
| # | |
| # `build-mode: none` scans C/C++ WITHOUT building it, inferring compile flags per file. That | |
| # matters here because the real firmware build is an ESP-IDF cross-compile for Xtensa/RISC-V a | |
| # runner would have to reproduce. The trade is some extraction noise on macro/template-dense | |
| # headers — and, more sharply, that what it analyzes is the DESKTOP configuration: anything whose | |
| # truth depends on a target typedef (`nrOfLightsType` is uint32_t here, uint16_t on a no-PSRAM | |
| # ESP32) is judged against the wrong build. Read findings of that shape with the target in mind. | |
| # | |
| # Free on public repos, so the cost is wall-clock only. | |
| on: | |
| # NOT A GATE: no `pull_request` trigger, so it cannot block a merge. The reasoning is | |
| # docs/testing.md § Static analysis, which every analyser here follows. The Security tab | |
| # keeps the open/fixed alert lifecycle, which is the baselining we would otherwise build. | |
| # | |
| # Both branches are listed on purpose. A workflow whose trigger names a branch nobody is | |
| # pushing to runs never, and reports nothing while looking healthy — this job sat idle for | |
| # two commits when the working branch was called `next` instead of `next-iteration` | |
| # (docs/history/lessons.md). `main` keeps it firing once this branch is merged and retired. | |
| push: | |
| branches: | |
| - main | |
| - next-iteration | |
| paths: | |
| # Only when C/C++ or the analysis setup itself changes — a docs commit has nothing new to | |
| # analyze and would just burn a runner. Both setup files are listed: a change to the query | |
| # set or the ignore list changes what a scan FINDS, so it has to trigger one. Listing only | |
| # the workflow left an edit to codeql-config.yml silently unscanned until something under | |
| # src/ happened to change. | |
| - 'src/**' | |
| - '.github/workflows/codeql.yml' | |
| - '.github/codeql-config.yml' | |
| workflow_dispatch: | |
| schedule: | |
| # Monday 04:00 UTC. Weekly is enough for a codebase this size, and keeps the signal | |
| # from becoming background noise nobody reads. | |
| - cron: '0 4 * * 1' | |
| permissions: | |
| contents: read | |
| # Required to upload results to the Security tab (this is what buys the alert lifecycle: | |
| # open/fixed/dismissed tracked across runs, i.e. baselining we would otherwise build). | |
| security-events: write | |
| jobs: | |
| analyze: | |
| name: CodeQL C/C++ | |
| runs-on: ubuntu-latest | |
| timeout-minutes: 60 | |
| steps: | |
| - uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false | |
| - name: Initialize CodeQL | |
| uses: github/codeql-action/init@v3 | |
| with: | |
| languages: c-cpp | |
| build-mode: none | |
| # security-extended, NOT security-and-quality. The quality pack was included to answer | |
| # one question — does CodeQL see anything our existing checks miss — and after two runs | |
| # the answer is no: its findings land in territory that already has an owner (unused | |
| # variables and commented-out code are clang-tidy's, long/trivial switches are lizard's), | |
| # which is the one-rule-one-owner rule this stack is built on. | |
| # | |
| # It was also actively misleading here. `build-mode: none` analyzes the DESKTOP | |
| # configuration only, so 7 `cpp/constant-comparison` alerts flagged the layouts' overflow | |
| # clamps as always-false — correct for that build, wrong for the target: `nrOfLightsType` | |
| # is uint16_t on a no-PSRAM ESP32, where those guards are load-bearing. Build-dependent | |
| # dead code is exactly what the quality pack hunts and exactly what it cannot judge here. | |
| # | |
| # The security queries are untouched, and they are the reason CodeQL has a slot at all: | |
| # whole-program taint across the six packet parsers on a device with no MMU. | |
| queries: security-extended | |
| # Excludes vendored code (test/doctest.h) — see the file for why. | |
| config-file: ./.github/codeql-config.yml | |
| - name: Perform CodeQL analysis | |
| uses: github/codeql-action/analyze@v3 | |
| with: | |
| category: "/language:c-cpp" |