Add GitHub Actions CI and SECURITY.md - #67
Open
austek wants to merge 8 commits into
Open
Conversation
tests/sonic_api_test.c had an unused variable warning under -Wall. sonic.c's findSincCoefficient left-shifted a value that can be negative (sincTable has negative entries), which is undefined behavior in C; replaced with the behaviorally-identical *2. Both were found while building out CI (see following commits) that enables -Werror and UBSan, and block those jobs from being meaningfully green. Also extends .gitignore to cover the various build artifacts (test binaries, .gcda/.gcno/.gcov, libsonic*) the new CI jobs' make invocations produce, none of which were previously ignored.
tests/fuzz_main.c declared short outBuffer[1024] (2048 bytes) and called sonicReadShortFromStream(stream, outBuffer, 1024). That maxSamples argument is a per-channel count everywhere in sonic.c, but the harness always passed the fixed cap regardless of the numChannels (1 or 2) it randomly generates from fuzz input -- for numChannels=2, the library correctly tries to write up to 1024*2 shorts into a 1024-short buffer. Confirmed via a symbolized ASan trace: stack-buffer-overflow, WRITE of size 2144 into outBuffer, in sonicReadShortFromStream at sonic.c:645. This is a harness bug, not a sonic.c bug -- fix caps the read request by dividing the fixed buffer size by numChannels. Found while wiring the existing fuzz harness into CI as a smoke test; without this fix, every stereo-generating CI fuzz run would crash immediately on a harness artifact rather than a real library bug.
Points reporters at GitHub's private security advisory flow, since there's no published security contact email. Closes waywardgeek#59.
Matrix across ubuntu-latest and macos-latest, running make && make test -- covers both Makefile platform branches and both platforms' default compilers (gcc on Ubuntu, Apple clang on macOS) without an explicit compiler matrix dimension. First job of a CI setup that had no automation at all before this; more jobs land in following commits.
Single ubuntu-latest job, CC=clang explicit so a macOS-specific warning difference in job 1 can never trip this job's -Werror. Targets exactly the bug class already hand-found and fixed this session (signed overflow, UB float-to-short conversion, UB left-shift).
Time-bounded (2 minute) regression run of the existing tests/fuzz_main.c harness via make fuzz -- a smoke test, not an open-ended campaign, so CI can't hang. This is exactly how waywardgeek#43's three bugs were originally found; until now it only ran when a human manually invoked make fuzz.
Runs the existing make coverage target (already builds with --coverage and invokes gcov) and uploads the resulting .gcov files as a build artifact. No external service/token -- a PR from a fork can't provision the upstream repo's secrets.
- push trigger now scoped to branches: [master], plus a concurrency group with cancel-in-progress, so post-merge pushes to in-repo branches don't double-run every job via both push and pull_request. - Add permissions: contents: read at the workflow level (nothing needs write). - Add timeout-minutes to all 4 jobs so a hanging job can't run to GitHub's 6-hour default (libFuzzer's own per-input timeout alone is 1200s). - fuzz-smoke now uploads any crash-* file on failure, so a red job leaves the maintainer a reproducer instead of just a stack trace.
This was referenced Aug 23, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #59.
The repo had no CI at all -- no
.github/workflows, nothing -- despitemake test,make coverage, andmake fuzzall already existing and working locally. This adds a.github/workflows/ci.ymlwith four jobs, all invoking those existing Makefile targets rather than inventing new build logic:build-and-test-- matrix[ubuntu-latest, macos-latest],make && make test. Covers both Makefile Linux/Darwin branches and both platforms' default compilers (gcc, Apple clang) without an explicit compiler matrix.strict-and-sanitized--ubuntu-latest,clang,-Werror+ ASan/UBSan. Targets exactly the bug class already found and fixed in Potential Integer Overflow at functioninsertPitchPeriod#50, Numerical truncation bug in functionaddFloatSamplesToInputBuffer#48, and one more (below).fuzz-smoke--ubuntu-latest, a 2-minute bounded run of the existingtests/fuzz_main.charness viamake fuzz. This is how 3 bugs found using afl #43's three bugs were originally found; previously only run manually. Uploads the crash reproducer as an artifact if it ever goes red.coverage--ubuntu-latest, runsmake coverage, uploads the.gcovoutput as a build artifact. No external service/token, since a PR from a fork can't provision this repo's secrets.Also adds
SECURITY.md-- points reporters at GitHub's private security advisory flow. And extends.gitignore, which previously only covered*.obj, to cover the various build artifacts (.o,.a,.so*, test/coverage/fuzz binaries,.gcda/.gcno/.gcov) these new jobs'makeinvocations produce.Three pre-existing bugs were blocking these jobs from being meaningfully green, fixed here rather than worked around:
tests/sonic_api_test.c(blocked-Werror).findSincCoefficientleft-shifted a value that can be negative (sincTablehas negative entries) -- undefined behavior in C. Fixed with the behaviorally-identical* 2.tests/fuzz_main.c's read loop passed a fixedmaxSamplescap without accounting fornumChannels, even though that parameter is a per-channel count throughoutsonic.c-- a real stack-buffer-overflow on stereo streams, confirmed via a symbolized ASan trace. This is a harness bug, not asonic.cbug; without the fix, every stereo-generating fuzz-smoke run would crash on the harness itself rather than finding real issues.Deliberately out of scope, tracked separately: formatting/clang-format (#64, since enforcing one now means reformatting every source file in this PR), Java build/test setup (#65, since
Sonic.java/Main.javahave no build system to run in CI yet), and #66, a separate pre-existing signed-integer-overflow ininterpolate()plus themake fuzztarget's lack of ASan/UBSan instrumentation.strict-and-sanitizeddoes compile and exercise that same code path (viatests/input_clamping_test.c's rate-change tests) but doesn't currently trip it -- the fixture's amplitude stays under the threshold that triggers the overflow, not because the path is untested. Worth knowing if that fixture's amplitude ever changes.make,make test, the sanitizedmake CC=clang CFLAGS=... test,make fuzz+ a 2-minute ASan+UBSan smoke run, andmake coverageall verified locally before this PR was opened, and independently re-verified in a second pass against the final HEAD.