mirror of
https://github.com/Cacti/spine.git
synced 2026-11-02 01:56:51 +09:00
develop
6
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
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> |
||
|
|
dbbccc5263 |
fix: extract core safety fixes from #597 (#621)
Extracted from #597 into a focused core correctness and memory-safety change. This checks startup allocations, bounds full log lines and query construction, frees settings result sets on NULL-row paths, caches the configured timestamp format, replaces advancing snprintf cursors with a bounded append contract, fixes remote batch boundary loss, and converts delimited hexadecimal OctetStrings without overflow. poller_output and remote status pushes use the cross-vendor VALUES() form until #590 supplies an independently detected main-server capability. Behavior worth calling out: - colon/dash-delimited hexadecimal values that were previously stored as zero now convert correctly - values wider than 64 bits become U and count as collection errors instead of producing undefined numeric conversion - remote batches flush before the boundary-triggering row and refuse truncated statements Verification on macOS arm64: - warning-enabled C99 configure/build - forced make -B check on the final tree: 6 pass, 1 expected skip - the skipped allocation-failure integration uses Linux LD_PRELOAD and runs in Linux CI - full VPATH make distcheck passed earlier in this branch - mandatory independent pre-push review: PASS Closes #564 Closes #565 Closes #566 Closes #567 Closes #588 Refs #590 Refs #597 --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Signed-off-by: TheWitness <copilot@users.noreply.github.com> Co-authored-by: TheWitness <copilot@users.noreply.github.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
|
478d330985 |
fix(ping): make IPv4 and IPv6 ICMP polling safe (#619)
## Summary Consolidates the overlapping ICMP work from #555 and #619 into one review of the complete socket and resolver state machine. - add ICMPv6 echo polling and dispatch IPv6 devices to it - prefer unprivileged datagram ICMP sockets when the kernel permits them - retain bounded raw-socket fallback for IPv4 and IPv6 - give packet and socket resources one cleanup owner - leave caller-owned resolver state untouched on lookup failure - keep every retry inside one cached capability and privilege-transition decision - prevent LOCK_SETEUID retry deadlocks - validate raw and datagram reply layouts, source, sequence, identifiers, and payload - retain fuzz and unprivileged-socket integration coverage The consolidation also fixed an integration defect found locally: the controlled successful-reply test constructed only a raw IPv4 packet even when the production path selected SOCK_DGRAM. It now models the selected socket format, and IPv6 raw retries use the same privilege-safety invariant as IPv4. ## Verification - warning-enabled local build completed with MariaDB Connector/C, Net-SNMP, OpenSSL 3, and cmocka - make check: 5/5 test binaries passed - focused ping_icmp suite: 9/9 cases passed - actionlint and git diff --check passed - all commits are signed - GitHub CI is authoritative for Linux capability, sanitizer, fuzz, and unprivileged-network coverage Closes #127. Closes #250. Closes #593. Closes #611. Supersedes #555. Refs #597 and #618. --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> 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> |
||
|
Thomas VincentandTheWitness
|
90da94b94f |
fix(process): unify subprocess ownership and child lifecycle safety (#614)
## Summary - make subprocess-registry detach atomic so exactly one closer owns each entry - make cancellation cleanup release only the detached node it owns - replace `vfork`/`execve` child setup with `posix_spawn` and explicit file actions - mark every pipe and stdio-collision duplicate close-on-exec - restore the child's default SIGPIPE disposition while preserving the parent's handling - bound TERM/KILL reaping and park unreaped children in a fixed-capacity sweep list - harden PHP script-server slot ownership, startup, restart, signal, descriptor, and cleanup paths - retire and restart failed or oversized reads so stale response tails cannot poison later data sources - revalidate every command retry and cap recovery handshakes at two seconds This folds the child-process lifecycle portion of #597 into the narrower ownership PR. It does not absorb unrelated poll-host, SQL, SNMP, ICMP, or build-infrastructure work. ## Review-driven corrections - close-on-exec is explicitly restored after `dup()` in both read and write stdio-collision branches - the source invariant check now uses portable POSIX character classes - direct tests cover duplicate flags, both collision branches, failure after pipe creation, abandoned-child sweeping, and capacity overflow - collision cleanup preserves current-child redirects even when an older registry entry occupies fd 0 or 1 - script children receive SIGTERM before SIGKILL and unreaped PHP children transfer to the bounded sweep registry - the Docker integration suite now starts a real PHP script server, verifies its startup protocol and database result, and checks that shutdown leaves no zombie PHP process ## Validation - Ubuntu 24.04 container with GCC and GNU ld: `make check`, 6/6 targets passed with no skips, including `test_php_runtime` - real PHP script-server Docker integration: 6/6 checks passed; exact value `42` reached `poller_output` and no zombie remained - macOS Homebrew dependency build passed - macOS `make check`: 5/5 available binaries passed; GNU `--wrap` runtime suite is Linux-only - PHP fixture syntax checked through mise PHP 8.3 - actionlint, shellcheck, and `git diff --check` passed Closes #610. --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
Thomas Vincent
|
d63e136b89 |
test: add the unit, fuzz and integration suites (#563)
Split out of #542. Tests and CI only, no source file is touched, so this cannot conflict with anything currently open. Verified in a clean `ubuntu:24.04` container against develop as it stands. **Unit** — 39 cmocka assertions across three binaries, run with `make -C tests/unit`: ``` tests: 16 test(s) run. PASSED tests: 20 test(s) run. PASSED tests: 3 test(s) run. PASSED ``` `test_safety_fixes.c` covers the allocation, bounds and reap paths; `test_build_fixes.c` covers `regex_replace` and the config defaults; `test_util_strings.c` covers `strpos`. **Fuzz** — a `get_namebyhost` target with a 26-input corpus, built with `-fsanitize=fuzzer,address,undefined`: ``` #2000 DONE cov: 64 ft: 85 corp: 11/48b ``` **Integration** — the SNMPv3 fixture polls a live `snmpd` container and asserts on the rows spine wrote to `poller_output` rather than on its exit code, plus scripted checks for CLI argument handling, the script server reap, and `output_regex` column detection. **Not included.** The `fuzz_icmp_reply` target stays on #542, because it calls `spine_icmp_classify_reply()`, which that PR adds. Everything else stands on its own here. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas Vincent
|
70459ac82c |
fix(build): resolve compilation failures on develop (#514)
## Summary Fixes #515. Spine on `develop` fails to compile after recent feature branch merges (everything after `4837ddd`). This PR fixes 6 build/runtime bugs, replaces deprecated `vfork`, and adds 68 automated tests. ## Fixes ### Build-breaking (prevented compilation) 1. **Missing uthash.h** - vendored v2.3.0 (header-only, BSD license). The O(1) hash table feature added `#include "uthash.h"` but never committed the file. 2. **Extra closing braces in poller.c** - duplicate `}` in POLLER_ACTION_SCRIPT and POLLER_ACTION_PHP_SCRIPT_SERVER cases broke the switch statement. ### Runtime bugs 3. **SNMP_FREE macro collision** - spine's `SNMP_FREE` (calls `snmp_host_cleanup`) collided with net-snmp's `SNMP_FREE` (calls `free`). Renamed to `SNMP_SESSION_FREE`. The `errstr` from `snmp_sess_error()` now correctly uses net-snmp's `free()` instead of `snmp_host_cleanup()`. 4. **output_regex column detection** - the output_regex feature (#210) assumed the column exists, but baseline Cacti does not have it. Added `db_column_exists()` check at startup to conditionally include it in queries. ### Deprecation warnings 5. **vfork in nft_popen.c** - replaced with `posix_spawn` using file actions, matching the approach already used in `php.c`. Eliminates macOS deprecation warning. ### Test fixture 6. **snmp_auth_protocol column width** - `char(6)` too narrow for `SHA-256` (7 chars). Fixed to `char(7)` in both host and poller_item tables. Added `rocommunity public` to snmpd.conf for SNMPv2c test coverage. ## Test Results (68/68 pass) | Suite | Count | Coverage | |-------|-------|----------| | Unit (cmocka) | 16 | uthash macros, config init, regex_replace, strncopy | | Smoke | 21 | SNMPv3 + SNMPv2c polling, DB writes, multi-device, runtime validation | | Output regex | 15 | Column absent, present, regex filtering, removal fallback | | DB column detect | 16 | 3 scenarios: absent, added via ALTER TABLE, dropped | ## Files changed (15) | File | Change | |------|--------| | `uthash.h` | New: vendored header | | `Makefile.am` | Add uthash.h to EXTRA_DIST | | `common.h` | Add uthash.h include | | `poller.c` | Fix braces, conditional output_regex in queries | | `spine.c` | Add has_output_regex detection at startup | | `spine.h` | Add has_output_regex to config struct | | `snmp.h` | Add SNMP_SESSION_FREE macro | | `sql.c` | Fix indentation in db_query(), restore missing continue | | `nft_popen.c` | Replace vfork+execve with posix_spawn | | `tests/snmpv3/db/init.sql` | Fix snmp_auth_protocol char(7) | | `tests/snmpv3/snmpd/snmpd.conf` | Add rocommunity public | | `tests/integration/smoke_test.sh` | New: e2e smoke test | | `tests/integration/test_output_regex.sh` | New: output_regex scenarios | | `tests/integration/test_db_column_detect.sh` | New: db_column_exists scenarios | | `tests/unit/test_build_fixes.c` + `Makefile` | New: cmocka unit tests | ## Follow-up PRs - #517: Replace deprecated POSIX semaphores with pthread wrapper (9 remaining warnings) - SNMP error handling, spike kill, script server path testing Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |