Skip to content

ci: weekly ASan/UBSan/TSan workflow with sticky issue tracking#2866

Merged
ten9876 merged 2 commits into
mainfrom
claude/review-project-SkDoW
May 21, 2026
Merged

ci: weekly ASan/UBSan/TSan workflow with sticky issue tracking#2866
ten9876 merged 2 commits into
mainfrom
claude/review-project-SkDoW

Conversation

@ten9876

@ten9876 ten9876 commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds .github/workflows/sanitizers.yml — a scheduled debug-heavy build that runs the test suite under sanitizers and auto-tracks findings in GitHub issues.

  • Schedule: every Monday 07:00 UTC, plus on-demand via workflow_dispatch.
  • Matrix: two jobs against the existing ghcr.io/ten9876/aethersdr-ci:latest image.
    • ASan + UBSanDebug build, picks up the existing -fsanitize=address from CMakeLists.txt:1260 and layers UBSan on top via CXXFLAGS/LDFLAGS.
    • TSan — separate RelWithDebInfo build (TSan can't coexist with ASan), thread-sanitizer flags injected directly.
  • Both run ctest under QT_QPA_PLATFORM=offscreen with halt_on_error=0 so one run captures every failure, not just the first.

Issue auto-creation

On failure each job:

  1. Extracts each sanitizer error block (grep -B 80 '^SUMMARY:'), capped at ~45 KB to fit a GitHub issue body.
  2. Uploads the full sanitizer.log + trimmed report.txt as artifacts (30-day retention).
  3. Opens or appends to a sticky tracking issue per sanitizer kind, matched by exact title + labels (sanitizer + asan-ubsan or tsan). Repeated weekly failures accumulate as comments instead of duplicating the queue. Dev team closes the issue when fixed; the next failing run reopens it.

Each comment includes a 12-char fingerprint of the first SUMMARY: line so reviewers can tell at a glance whether a re-run is the same bug or a new one stacked on top.

Why this fits AetherSDR's threat model

The "70% of CVEs are C++ memory-safety bugs" stat is real but measured on browsers/kernels. AetherSDR's attack surface is narrow (one TCP/UDP socket to a known FlexRadio on a trusted LAN). Memory bugs here mostly cause crashes, not RCEs. ASan + UBSan + TSan close the same correctness gap a Rust port would close, for ~zero engineering cost and without giving up Qt.

Prerequisites for a clean first run

  • Create the three labels: sanitizer, asan-ubsan, tsan (or let the first failing run create them via the create issue call).
  • Verify the Docker CI image has the sanitizer runtimes — almost certainly yes for any modern gcc/clang, but worth a one-off workflow_dispatch run before relying on the cron.
  • Expect a noisy first run. Sanitizers always find something on first contact with a mature C++ codebase. May need a .asan-suppressions.txt for known false positives in Qt plugin loading — common, well-documented, fixable in a follow-up.

Test plan

  • Manually trigger via Actions → Sanitizers → Run workflow against claude/review-project-SkDoW and confirm the image has ASan/UBSan/TSan runtimes.
  • Confirm a deliberately-broken test (temporarily add a heap-use-after-free in a test file) produces a non-zero exit, an artifact upload, and a new tracking issue with labels.
  • Re-run the broken build and confirm the second run appends a comment to the existing issue instead of opening a duplicate.
  • Fix the broken test, manually close the tracking issue, confirm next clean run produces no new issue.

Follow-ups (not in this PR)

  • Add a --self-test startup mode that constructs MainWindow, processes one offscreen frame, and tears down — would expose the largest files (MainWindow, SpectrumWidget, AudioEngine) to ASan, which the current unit suite doesn't cover.
  • Replace the workflow's CXXFLAGS injection with a proper -DAETHERSDR_SANITIZER=asan|ubsan|tsan|all cache var in CMakeLists.txt for cleaner local repro.

https://claude.ai/code/session_01PSLHgUjYZsq67v3Fz4peAE


Generated by Claude Code

Adds .github/workflows/sanitizers.yml — runs every Monday 07:00 UTC plus
on-demand via workflow_dispatch. Matrix runs two jobs against the existing
ghcr.io/ten9876/aethersdr-ci image:

- ASan + UBSan: Debug build (picks up the existing -fsanitize=address from
  CMakeLists.txt:1260) with UBSan layered via CXXFLAGS/LDFLAGS.
- TSan: RelWithDebInfo build with TSan flags injected (incompatible with
  ASan, hence a separate matrix entry).

Both run ctest under QT_QPA_PLATFORM=offscreen with halt_on_error=0 so a
single run captures every failure, not just the first.

On failure each job:
- Extracts each sanitizer error block (grep -B 80 '^SUMMARY:'), capped
  at 45KB to fit a GitHub issue body.
- Uploads the full sanitizer.log + trimmed report.txt as artifacts
  (30-day retention).
- Opens or appends to a sticky tracking issue per sanitizer kind,
  matched by exact title + labels (sanitizer + asan-ubsan|tsan), so
  repeated weekly failures accumulate as comments instead of
  duplicating the queue.

Each comment includes a 12-char fingerprint derived from the first
SUMMARY line so reviewers can tell at a glance whether a re-run is the
same bug or a new one stacked on top.

https://claude.ai/code/session_01PSLHgUjYZsq67v3Fz4peAE
@ten9876 ten9876 requested a review from AetherClaude May 19, 2026 00:48

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ten9876 — the overall shape is good: matrix split for ASan/UBSan vs. TSan, halt_on_error=0 to capture every failure per run, artifact upload, sticky-issue dedup with a fingerprint, and a thoughtful continue-on-error + re-fail pattern. The grep-based 80-line context capture with the 45 KB cap is a nice touch for fitting GitHub issue bodies.

One real coverage gap and a few smaller things worth addressing before the first cron fire.

Main finding: ASan won't actually instrument the test binaries

The PR description says the asan-ubsan job "picks up the existing -fsanitize=address from CMakeLists.txt:1260". That flag is scoped to target_compile_options(AetherSDR PRIVATE ...) — it applies only to the AetherSDR main target.

The unit tests at CMakeLists.txt:1273+ are standalone add_executable targets that pull in specific source files directly (e.g., client_eq_test compiles src/core/ClientEq.cpp) and are not linked against the AetherSDR target. So:

  • The asan-ubsan job only injects -fsanitize=undefined via CXXFLAGS/LDFLAGS → tests get UBSan only, no ASan.
  • ASan compiles into the AetherSDR binary, but ctest never executes that binary, so no allocations under ASan ever happen.
  • Net result: the "ASan + UBSan" job effectively only exercises UBSan on the unit tests.

The TSan job is fine — -fsanitize=thread is in extra_cxx/extra_ld, so it reaches the test compilations.

Simplest fix — add ASan to the matrix entry the same way TSan handles it:

- id: asan-ubsan
  label: ASan + UBSan
  build_type: Debug
  extra_cxx: "-fsanitize=address,undefined -fno-sanitize-recover=undefined -fno-omit-frame-pointer"
  extra_ld:  "-fsanitize=address,undefined"

That also makes the workflow self-contained — it no longer relies on the Debug-build side effect in CMakeLists.txt:1260, which would let you drop that coupling when you do the follow-up -DAETHERSDR_SANITIZER=... cache var.

Smaller items

  • actions/checkout@v4ci.yml and the other workflows in this repo use @v6. Worth matching for consistency.
  • credentials: block on the container.github/workflows/ci.yml:18 pulls the same ghcr.io/ten9876/aethersdr-ci:latest image with no credentials, which suggests it's public. If so, the credentials: block here is dead weight; if the image were private, GITHUB_TOKEN scoped to aethersdr/AetherSDR wouldn't have access to a package under the ten9876 user namespace anyway. Recommend dropping it.
  • Title-based sticky-issue matchopen.data.find(i => i.title === title) is fragile if a maintainer ever renames the tracking issue (or appends [blocked], etc.). Not a blocker since the state: 'open' + label filter already narrows the search, but you could match on the label combination alone given the labels are kind-specific (sanitizer + asan-ubsan/tsan already uniquely identifies the bucket).

Nothing else stands out — the scope is clean (one workflow file), no AetherSDR conventions in play here, and the safety posture (permissions: minimal, no token write-back to code) is right.

ten9876 pushed a commit that referenced this pull request May 21, 2026
After publishing a new CI image, docker-ci-image.yml now captures the
sha256 digest from docker/build-push-action and opens an auto-PR that
rewrites the image reference in the workflow consumers from the tag
form to the digest form.

Mirrors the existing auto-PR pattern from update-rnnoise.yml
(peter-evans/create-pull-request, branch deleted on close,
labels: ci). Permissions block extended from packages:write to also
include contents:write and pull-requests:write — the minimum the
create-pull-request action needs to push a branch and open the PR.

Consumer set covers ci.yml, codeql.yml, and sanitizers.yml. A `[ -f ]`
guard lets the loop run cleanly when a consumer file isn't on main
yet (sanitizers.yml until PR #2866 lands).

peter-evans/create-pull-request is pinned to commit SHA inline with
the rest of the third-party action references on main.

Idempotent both ways:
  - Bootstrap run on a ":latest" reference swaps in the digest.
  - Subsequent re-runs producing the same digest leave files untouched.
  - A re-run with a new digest cleanly replaces the previous one.

This PR adds the mechanism only — it does not modify ci.yml or
codeql.yml. After merge, one workflow_dispatch run produces the
bootstrap PR that lands the first pinned digest.

https://claude.ai/code/session_01PSLHgUjYZsq67v3Fz4peAE
Four fixes from the bot review:

1. ASan now actually instruments the test binaries.
   The previous comment claimed CMakeLists.txt:1260 carried ASan onto
   every Debug build, but that flag is scoped to
   `target_compile_options(AetherSDR PRIVATE ...)` and applies only to
   the AetherSDR executable. The unit tests at CMakeLists.txt:1273+ are
   standalone add_executable targets that don't link against
   AetherSDR, so they inherit nothing from that line. With
   extra_cxx="-fsanitize=undefined" alone, the asan-ubsan job was
   effectively a UBSan-only job. Switch the asan-ubsan matrix entry
   to inject "-fsanitize=address,undefined ..." through CXXFLAGS /
   LDFLAGS the same way TSan does, so the flags reach every target
   the build produces.

2. Pin third-party actions to commit SHAs matching main:
     actions/checkout@v4         -> @de0fac2  # v6
     actions/upload-artifact@v4  -> @043fb46  # v7
     actions/github-script@v7    -> @f28e40c  # v7
   Brings this workflow in line with the SHA pins applied across the
   rest of .github/workflows/.

3. Drop the credentials: block on the container. The same image is
   pulled by ci.yml with no credentials, which means the package is
   public; the block was dead weight.

4. Simplify sticky-issue match. The (sanitizer + asan-ubsan|tsan)
   label combo already uniquely identifies the bucket, so a
   title-equality check on top is redundant and fragile to manual
   issue renames. Use open.data[0] from the label-filtered result.

https://claude.ai/code/session_01PSLHgUjYZsq67v3Fz4peAE

ten9876 commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed a07536a addressing all four points:

  1. ASan now actually runs. Moved -fsanitize=address from the (incorrectly assumed) CMakeLists.txt Debug-side-effect into the matrix entry's extra_cxx/extra_ld, the same way TSan already injects its flags. The asan-ubsan job will now instrument every test executable, not just the AetherSDR target it never executes. Good catch — that one would've shipped a workflow that didn't actually do what it claimed.

  2. Third-party actions pinned to commit SHA matching main:

    • actions/checkout@v4@de0fac2… # v6
    • actions/upload-artifact@v4@043fb46… # v7
    • actions/github-script@v7@f28e40c… # v7
  3. Dropped the credentials: block on the container. Confirmed the image is public — ci.yml pulls it with no credentials and works.

  4. Simplified the sticky-issue match to open.data[0] from the label-filtered list. The sanitizer + (asan-ubsan|tsan) label combo already uniquely identifies the bucket.

The branch also pre-dates a few things on main now (no permissions: blocks, no SHA pins, no CODEOWNERS tier-4 setup) — but since this PR only adds a new file, there are no merge conflicts.


Generated by Claude Code

@ten9876 ten9876 merged commit 4fa407f into main May 21, 2026
3 checks passed
@ten9876 ten9876 deleted the claude/review-project-SkDoW branch May 21, 2026 16:29
ten9876 added a commit that referenced this pull request May 25, 2026
…ork (#3155)

## Summary

The Sanitizers workflow ([first run
#26397650182](https://github.com/aethersdr/AetherSDR/actions/runs/26397650182),
2026-05-25 07:00 UTC) reported green — but it didn't actually run
anything under the sanitizers. The "Run tests under sanitizer" step
exited at code 2 on its very first line, before `ctest` ran.
\`continue-on-error: true\` silenced the failure at the job level, and
every downstream gate on \`steps.ctest.outputs.exit != '0'\` then
matched against an unset output, so the workflow walked through to
completion. Every scheduled run since the workflow landed (PR #2866,
2026-05-21) has produced a false-clean result with zero sanitizer
coverage.

## Root cause

The container image \`ghcr.io/ten9876/aethersdr-ci:latest\` defaults
\`/bin/sh\` to **Dash**, which doesn't understand \`set -o pipefail\` or
\`\${PIPESTATUS[0]}\`. Two bashisms, both at line 75/83 of
[`.github/workflows/sanitizers.yml`](.github/workflows/sanitizers.yml).

Run log signature:
```
/__w/_temp/...sh: 1: set: Illegal option -o pipefail
##[error]Process completed with exit code 2.
```

## Fix

One-line: add \`shell: bash\` to the "Run tests under sanitizer" step.
Bash is present in the container; only this step uses non-POSIX
features, so leaving the other steps on the default shell keeps the
patch minimal.

## Verification

Triggered \`workflow_dispatch\` on this branch: [run
#26415106480](https://github.com/aethersdr/AetherSDR/actions/runs/26415106480).
With the fix in place, the step should now reach \`ctest\` and either
pass cleanly or surface real sanitizer findings (and file/update the
sticky tracking issue). Result will appear there before this PR merges.

## Test plan

- [ ] Workflow dispatch run on this branch executes \`ctest\`
(verifiable in the step log)
- [ ] If tests pass under sanitizers: no behavior change vs. \"green\"
appearance, but now meaningful
- [ ] If tests fail: sticky issue gets opened or appended, artifacts
uploaded
- [ ] CI green on the PR itself

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
G6PWY-Chris pushed a commit to G6PWY-Chris/AetherSDR that referenced this pull request Jun 22, 2026
G6PWY-Chris pushed a commit to G6PWY-Chris/AetherSDR that referenced this pull request Jun 22, 2026
…ork (aethersdr#3155)

## Summary

The Sanitizers workflow ([first run
#26397650182](https://github.com/aethersdr/AetherSDR/actions/runs/26397650182),
2026-05-25 07:00 UTC) reported green — but it didn't actually run
anything under the sanitizers. The "Run tests under sanitizer" step
exited at code 2 on its very first line, before `ctest` ran.
\`continue-on-error: true\` silenced the failure at the job level, and
every downstream gate on \`steps.ctest.outputs.exit != '0'\` then
matched against an unset output, so the workflow walked through to
completion. Every scheduled run since the workflow landed (PR aethersdr#2866,
2026-05-21) has produced a false-clean result with zero sanitizer
coverage.

## Root cause

The container image \`ghcr.io/ten9876/aethersdr-ci:latest\` defaults
\`/bin/sh\` to **Dash**, which doesn't understand \`set -o pipefail\` or
\`\${PIPESTATUS[0]}\`. Two bashisms, both at line 75/83 of
[`.github/workflows/sanitizers.yml`](.github/workflows/sanitizers.yml).

Run log signature:
```
/__w/_temp/...sh: 1: set: Illegal option -o pipefail
##[error]Process completed with exit code 2.
```

## Fix

One-line: add \`shell: bash\` to the "Run tests under sanitizer" step.
Bash is present in the container; only this step uses non-POSIX
features, so leaving the other steps on the default shell keeps the
patch minimal.

## Verification

Triggered \`workflow_dispatch\` on this branch: [run
#26415106480](https://github.com/aethersdr/AetherSDR/actions/runs/26415106480).
With the fix in place, the step should now reach \`ctest\` and either
pass cleanly or surface real sanitizer findings (and file/update the
sticky tracking issue). Result will appear there before this PR merges.

## Test plan

- [ ] Workflow dispatch run on this branch executes \`ctest\`
(verifiable in the step log)
- [ ] If tests pass under sanitizers: no behavior change vs. \"green\"
appearance, but now meaningful
- [ ] If tests fail: sticky issue gets opened or appended, artifacts
uploaded
- [ ] CI green on the PR itself

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants