5 Commits
Author SHA1 Message Date
Thomas Vincent 3065c6fc23 refactor(core): add bounded native interfaces and polling contracts (#638)
Introduce bounded native interfaces and grouped runtime state, then link
regression and fault contracts against the real poller implementations.
The contracts capture CLI/configuration input, availability
combinations, SNMP request/response ownership, script framing,
reindexing, output persistence, worker completion and shutdown. This
foundational change is larger because the shared interfaces and their
existing production contracts must move together; later changes have
separate review boundaries.

Step 1/14 of the complete C17/native-correctness series. Base: develop.

Earlier validation, before this review follow-up, in isolated copied
snapshots on Linux arm64:

- Clean GCC build and make check: 13 passed, 0 existing prerequisite
skips, 0 failures.
- ASan/UBSan: 12 passed, 1 existing privilege/allocator-interposition
prerequisite skips, 0 failures; no sanitizer findings.
- Production source/profile integrity verification against disposable
MariaDB and loopback SNMP fixtures passed.
- Release archive extracted into a separate source tree and built/tested
out of tree; fuzz checks passed.

Earlier local profiles retain source hashes for the copied validation
tree; GitHub checks validate the published commit. No production
database/device, host installation or live privilege change is part of
validation. Existing prerequisite skips were retained; no test or
finding was suppressed to obtain a pass.


The worker contracts publish owned heap state and release it only after
joining workers and restoring the runtime table. Coverage counters use
atomic updates; cppcheck receives an explicit model of the fatal exit
contract. Fixture images use public mirrors to avoid shared Docker Hub
pull limits.


Series navigation: review and merge in order. Every PR targets the
preceding branch so its diff contains one step.

1. [refactor(core): add bounded native interfaces and polling
contracts](https://github.com/Cacti/spine/pull/638)
2. [fix(privileges): drop root before input and retain owned ICMP
sockets](https://github.com/Cacti/spine/pull/639)
3. [fix(poller): isolate worker failures and bound database
reconnects](https://github.com/Cacti/spine/pull/640)
4. [fix(results): normalize output and unify SNMP batch
persistence](https://github.com/Cacti/spine/pull/641)
5. [fix(runtime): bound probe retries and validate CLI and
configuration](https://github.com/Cacti/spine/pull/642)
6. [build: select GNU C17 explicitly and probe hardening
support](https://github.com/Cacti/spine/pull/643)
7. [fix(threads): size worker stacks and guard size
arithmetic](https://github.com/Cacti/spine/pull/644)
8. [refactor(c17): simplify ownership guards and share test
support](https://github.com/Cacti/spine/pull/645)
9. [refactor(core): extract configuration polling and runtime
modules](https://github.com/Cacti/spine/pull/646)
10. [fix(boundaries): validate script slots and make cleanup
portable](https://github.com/Cacti/spine/pull/647)
11. [refactor(layout): organize responsibilities and separate main from
runtime](https://github.com/Cacti/spine/pull/648)
12. [docs(license): adopt SPDX notices and retain contributor
credits](https://github.com/Cacti/spine/pull/649)
13. [test(runtime): measure all suites and cover protocol
boundaries](https://github.com/Cacti/spine/pull/650)
14. [build(ci): validate connectors time width and native C17
modules](https://github.com/Cacti/spine/pull/651)

Review follow-up (2026-10-09): Publish the decoded GET/GETNEXT status
back to the host so availability checks observe exception and
missing-response failures, while preserving the legacy transport-success
behavior for agent PDU errors.

Updated step 1 snapshot: isolated Linux arm64 build and make check
passed (13 passed, no skips or failures).
The restacked tip passes all five clang-tidy gate regression tests and
real clang-tidy analysis (26 reviewed baseline findings, no new
findings). Actionlint and matching DCO trailers pass. The setuid test
also passes when run in the separate disposable root container.
GitHub checks validate the updated published commits; earlier
profile/archive/sanitizer evidence above belongs to the previous
revisions.

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-10-10 20:03:21 -04:00
86a062dd09 ci: enforce cross-platform quality gates and connector coverage (#618)
## Summary

Consolidates the connector/build work from #613 and the broader
test-infrastructure work extracted from #597 into one review:

- discover MariaDB Connector/C first while retaining MySQL and
Percona-compatible support
- test MariaDB and MySQL Connector/C with strict GCC and Clang builds on
Ubuntu 24.04
- add a real macOS 15 build and unit-test lane with portable Autotools
platform detection
- run unit tests under ASan/UBSan and ThreadSanitizer
- enforce the measured 7% global coverage baseline and 70% changed-line
coverage
- add libFuzzer coverage for output parsing alongside the existing
hostname parser
- exercise the SNMPv3 integration suite against MariaDB and MySQL 8.4
- add cppcheck, shellcheck, DCO verification, CODEOWNERS, distcheck, and
Linux-capability checks while retaining the repository-managed default
CodeQL setup
- expose a stable `CI / required` aggregate check for branch protection
- pin runners and third-party actions and keep workflow permissions
read-only by default

## Correctness fixes found by the new gates

- reject truncated capability strings and poller-output SQL tuples
instead of publishing partial data
- make `bootstrap` fail closed and select `libtoolize`/`glibtoolize`
portably
- use Autoconf's canonical host for Darwin detection instead of an empty
native-build alias
- remove stale initialization/name-shadow findings and make read-only
interfaces const-correct
- make the ICMP parser test portable across Linux and BSD header names

## Verification

- strict GCC and Clang builds: 4/4 unit targets passed
- ASan/UBSan: 4/4 passed
- ThreadSanitizer: 4/4 passed
- macOS Clang/Homebrew build: 4/4 passed
- MariaDB integration: passed
- MySQL 8.4 integration: passed
- both libFuzzer targets: passed sustained local runs
- coverage: 7.2% global and 84% on changed lines
- cppcheck, shellcheck, actionlint, DCO verification, and `git diff
--check`: passed locally

## Post-merge repository settings

After these workflows exist on `develop`, an administrator should
require `CI / required`, Fuzz, Integration, Rocky Linux, and the
repository-managed CodeQL/code-scanning result in the active ruleset;
enable code-owner approval; and require the branch to be current (or use
a merge queue). Enabling those settings before this PR merges would
create required checks that the base branch cannot yet emit.

Consolidates and supersedes #613. Closes #612.

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: TheWitness <thewitness@cacti.net>
Signed-off-by: TheWitness <copilot@users.noreply.github.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
Co-authored-by: TheWitness <copilot@users.noreply.github.com>
2026-09-24 05:52:56 -04:00
Thomas VincentandTheWitness f989722458 build: establish src/core layout for support modules (#617)
## What

- move `keywords.c`/`.h` and `locks.c`/`.h` into `src/core/`
- enable Automake `subdir-objects` for collision-safe nested object
placement
- update the Autotools binary, linked-test, standalone unit-test, and
fuzz paths
- make both cppcheck entry points share one complete nested-source
discovery path
- ignore nested Automake and Docker build artifacts
- retain existing header include names and the Autotools build

The four production source/header files are 100% renames with no content
changes. The remaining changes are build and verification plumbing
required by the new paths.

## Why

Spine production sources currently live in the repository root. This
starts the domain layout in a small, reviewable batch instead of
repeating #523, where the source move was coupled to an 11k-line
CMake/platform rewrite. `keywords` and `locks` were selected because
active PRs #597 and #614 do not modify their implementation files; only
shared build manifests overlap #597.

`subdir-objects` is enabled in this first production move so future
modules with the same basename cannot write the same top-level object
under parallel builds.

Closes #616.

## Verification

- `./bootstrap` with no subdirectory-source warnings
- `./configure --with-mysql=$(brew --prefix mariadb-connector-c)
--with-snmp=$(brew --prefix net-snmp) CPPFLAGS=-I$(brew --prefix
openssl@3)/include LDFLAGS=-L$(brew --prefix openssl@3)/lib`
- warning-enabled `make -j$(sysctl -n hw.ncpu)`; project sources
compile, with Homebrew Net-SNMP header warnings only
- `make check` (cmocka unavailable to Autoconf locally; 0 local tests)
- `make distcheck`; archive verified to contain all four `src/core`
files
- cppcheck source discovery verified under macOS Bash 3 and includes all
four relocated files
- standalone `tests/unit/Makefile` compiles through header resolution
with `-I../../src/core` (the local macOS link retains unrelated existing
client-library/toolchain issues)
- GitHub checks pass, including fuzz, sanitizers, coverage, `distcheck`,
gcc/clang, SNMPv3, and Rocky Linux

The local macOS fuzz build reaches both relocated files, then fails
identically on untouched `util.c` (`INT_MAX`) and Net-SNMP API
declarations on both this branch and `develop`. Linux fuzz CI is the
authoritative fuzz-build result.

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-09-22 11:07:10 -04:00
Thomas VincentandTheWitness 3570e89098 fix(C): correct printf format specifiers, guard calloc, and fix shell quoting (#449)
Twelve bugs found by running cppcheck 2.20
(--enable=warning,portability) and shellcheck v0.10.0.1 across the full
source tree.

**poller.c**

- `%llu` used for `atoll()` return (signed `long long`) at line 970 —
changed to `%lld`
- `%lld` used for `hex2dec()` return (unsigned `long long`) at five call
sites (lines 1414, 1515, 1597, 1655, 1727) — changed to `%llu`

Mismatched sign in printf format specifiers is undefined behaviour under
C99 §7.19.6.1.

**snmp.c**

- `session.localname` is `strdup()`'d at line 140 (when `SNMP_LOCALNAME
== 1`) but the early-return path for an unknown SNMP version at line 190
did not free it. Added `free(session.localname)` before that return.
`free(NULL)` is a no-op so the call is safe when `SNMP_LOCALNAME != 1`.
- `calloc(num_oids, sizeof(*name))` at line 999 has no NULL check. If
allocation fails, the immediately following `namep->name_len =
MAX_OID_LEN` dereferences a null pointer. Added a NULL guard that logs
via `SPINE_LOG` and returns early before the PDU is created.

**sql.c**

- `attempts` is `int` but printed with `%u` — changed to `%d`.
- `error` is `int` but printed with `%u` — changed to `%d`.
- `mysql = NULL` in `db_disconnect()` assigns to a by-value parameter;
the write has no effect on the caller. Removed the dead assignment.

**util.c**

- `pthread_self()` is cast to `unsigned long int` then printed with
`%ld` (signed) — changed to `%lu`.

**copyright_year.sh / scripts/verify.sh**

- `$1` unquoted in `sed` invocation in `copyright_year.sh` — word-splits
on filenames with spaces.
- `*.c *.h` glob in `scripts/verify.sh` passed without `--` separator —
filenames starting with `-` would be misinterpreted as cppcheck options.

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-03-10 09:50:05 -04:00
Thomas Vincent bae4dd32c0 Add Dockerfile for building and verifying spine (#401)
Fixes #444

- **Dockerfile**: multi-stage build. Builder stage compiles spine; final
stage is minimal, non-root, with only required shared libs.
- **Dockerfile.dev**: single-stage dev image with ASan, cppcheck,
scan-build, valgrind, and gdb. Defaults to running scripts/verify.sh
(cppcheck + scan-build + smoke tests).
- **scripts/verify.sh**: runs cppcheck, scan-build, and basic
--help/--version smoke tests.
- **.dockerignore**: excludes .git and build artifacts.

Nothing changes about the existing build. Purely additive.

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-03-08 09:40:38 -04:00