The runtime/probe changes from #642 and GNU C17/build changes from #643
were squash-merged into #641's source branch, which made the
result-normalization review include later stages. This follow-up
preserves those approved changes while restoring #641's focused diff
against develop. No force-push or approved-code deletion is required.
Merge after #641 and before #644. #644 will target this branch. This
combines the already reviewed steps 5 and 6; it introduces no new
worker-stack or module-extraction changes.
The UDP follow-up also corrects a validated finding: EHOSTUNREACH on a
connected socket is ambiguous and can represent an unreachable host or
failed neighbour lookup. It now reports down; ECONNREFUSED/ECONNRESET
and actual peer datagrams retain their existing alive behavior. The
production-linked test rejects the original EHOSTUNREACH behavior.
Validation: Ubuntu 24.04/aarch64, clean GCC build and make check: 17
passed, one existing setuid prerequisite skip, zero failures. The full
tracked source tree is byte-identical to the independently validated
#641 functional-fix snapshot. The UDP and poll-host regressions fail
against the original code and pass after the fixes. The isolated step-4
tree independently passes 15 tests with one existing skip. No live
endpoints, host installation, privilege changes, or test suppression.
Repository squash merges still require restacking/retargeting the next
child after each merge; clean ordinary merge simulations are
insufficient by themselves.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Classify numeric, undefined, multipart and Cacti octet results through
one parser, normalize undefined agent values before persistence and
flush every SNMP batch consistently. Validate whole non-negative
uptimes, retain unknown reindex baselines and free script results
through their owning pointer. The mocked poll_host suite checks
session-profile boundaries, spike handling, output_regex input and
completion writes.
Originally step 4/14 of the C17/native-correctness series; its preceding
steps have merged. The current base is develop.
Earlier validation, before this review follow-up, in isolated copied
snapshots on Linux arm64:
- Clean GCC build and make check: 15 passed, 1 existing prerequisite
skips, 0 failures.
- ASan/UBSan: 14 passed, 2 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.
Review follow-up (2026-10-09): Restacked on the corrected preceding
branches. The SNMP status fix is introduced in #638, the
reindex/completion fixes in #640, and focused regression coverage in
#650.
Restacked tip validation: isolated Linux arm64 build and make check
passed (18 passed, 1 setuid prerequisite skip, no failures).
GitHub checks validate the updated published commits; earlier
profile/archive/sanitizer evidence above belongs to the previous
revisions.
## Current merge dependencies
Current base: `develop`. Immediate dependency: `develop` (earlier steps
already merged). Next open PR:
[#644](https://github.com/Cacti/spine/pull/644).
Merge order: #641 → #644 → #654 → #646 → #648 → #649 → #650 → #651 →
#655 → #658 → #659.
This repository uses squash-only merges and deletes merged branches.
Record the old parent SHA, squash one PR at a time, then restack and
retarget dependents before merging the next PR. Re-run merge validation
and checks on the resulting heads; an ordinary clean merge does not
replace squash/restack validation.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Open and own the required ICMP sockets before permanently dropping
setuid root, then parse input without elevated identity. Close-on-exec
socket ownership, shared IPv4/IPv6 dispatch and fault-injected
identity/capability calls are covered. The owned fixture tests check
deny/allow behavior without contacting external devices.
Step 2/14 of the complete C17/native-correctness series. Base: the
preceding series branch; merge/review in order so this PR shows only its
own step.
Earlier validation, before this review follow-up, in isolated copied
snapshots on Linux arm64:
- Clean GCC build and make check: 13 passed, 1 existing prerequisite
skips, 0 failures.
- ASan/UBSan: 12 passed, 2 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.
Series navigation: [Series
index](https://github.com/Cacti/spine/pull/638); [previous
step](https://github.com/Cacti/spine/pull/638). [Next
step](https://github.com/Cacti/spine/pull/640).
Review follow-up (2026-10-09): Restacked on the corrected preceding
branches. The SNMP status fix is introduced in #638, the
reindex/completion fixes in #640, and focused regression coverage in
#650.
Restacked tip validation: isolated Linux arm64 build and make check
passed (18 passed, 1 setuid prerequisite skip, no failures).
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>
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>
## What
Follow-up to #633 (already merged), from the code review findings on the
equivalent 1.2.x change (#634):
- **SOLAR_THREAD sleep budget**: `spine_reap_child_bounded()` has no
`usleep()`-paced spin phase on `SOLAR_THREAD` builds - it sleeps a flat
1s per attempt. #633 raised the shared attempt budget
(`NFT_PCLOSE_SPIN_ATTEMPTS`) to 100 for `nft_pclose()`'s brief
allowance, which made it sleep up to **~100 seconds** on that platform
instead of the intended ~20ms, reintroducing the exact poller stall #633
was fixing. `NFT_PCLOSE_SPIN_ATTEMPTS` is now platform-specific (100
elsewhere, 2 under `SOLAR_THREAD`).
- **Unbounded EINTR retries**: bounded the `EINTR` retry loop in
`spine_reap_child_bounded()` and in `nft_sweep_abandoned()` (which runs
with `ListMutex` held), so a stream of caught signals can no longer spin
either indefinitely.
- **Kill before park on waitpid() error**: `nft_pclose()` now SIGKILLs
the child before parking it when `waitpid()` itself errors (previously
only done when the child was confirmed still running), so a
still-running child isn't left outside the abandoned-pid sweep's reach.
Preserves `waitpid()`'s errno across the call.
- **Docs**: `nft_popen.h`'s `nft_pclose()` comment still described the
old blocking-wait behavior and didn't mention `ETIMEDOUT` - #633 updated
`nft_popen.c`'s comment but missed the header. Both now document the
bounded-check/parked-child contract.
## Why
These were flagged by review of #634 (the same fix ported to 1.2.x,
where the affected code showed up as new rather than a diff against
pre-existing lines). Fixing here too since develop has the identical
code.
## What was tested
- `./bootstrap && ./configure --enable-warnings && make` - clean build,
no new warnings.
- `make check` - all 11 suites pass.
- `cppcheck --enable=all --std=c11` on `nft_popen.c` - no new errors.
Closes#632.
---------
Signed-off-by: TheWitness <thewitness@cacti.net>
## 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>
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#564Closes#565Closes#566Closes#567Closes#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>
Extracted from #597 into a focused SNMPv3 correctness and safety change.
This restores noAuthNoPriv and authNoPriv session construction, derives
authentication keys on the authNoPriv path, validates configured
algorithms when they are used, wipes temporary passphrase copies, and
makes the effective security-level decision agree with Cacti. It also
prevents a failed mid-loop session rebuild from passing NULL into
Net-SNMP multi-get.
Compatibility and failure policy:
- [None] is the explicit no-protocol sentinel; stale passphrases paired
with it downgrade with a warning to the Cacti effective level.
- A selected protocol missing its passphrase downgrades with an explicit
warning, preserving existing polling behavior.
- A blank protocol field paired with a passphrase is ambiguous and now
fails closed with an error.
- Complete privacy credentials without usable authentication fail closed
because USM cannot honor privacy without authentication.
- Unknown authentication algorithms fail closed instead of silently
substituting a default; unused privacy algorithms do not block
authNoPriv.
Verification:
- warning-enabled C99 build on macOS arm64
- make check: all 5 suites passed
- deterministic SNMPv3 session suite: 22/22 pass, including literal OID
mapping, effective-level combinations, refusal paths, key derivation,
caller-buffer ownership, and NULL multi-get handling
- mandatory independent pre-push review: PASS
Closes#582
Refs #597
---------
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>
## 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>
## 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>
## 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>
## Summary
A batch of small, independent, unaddressed bugs from the open-issue
backlog. None of these overlapped with any currently open PR's actual
diff (verified by inspecting each candidate PR's changes directly, not
just its declared `Closes` list).
## Fixes
- **`poll_host()` skips `mysql_thread_end()` when the device row is
missing** — the early `return` on `num_rows != 1` released both DB
connections and freed six allocations, but never called
`mysql_thread_end()`, unlike the other two exit paths. Added the missing
call.
- **SNMPv3 passphrase scrubbing flag never set** — `zero_sensitive` was
a function-local initialized to 0 and never assigned, so every guarded
`memset()` was dead code. Made scrubbing of this module's own heap
copies (`Apsz`/`Xpsz`) unconditional, and moved the wipe to immediately
precede each free (including both `generate_Ku()` error paths) after
review found the first pass still scrubbed before assignment. Left the
caller's `snmp_password`/`snmp_priv_passphrase` alone rather than also
scrubbing those in place — those are `host_t`-owned and get reused for
later session rebuilds against the same host, so zeroing them would have
broken subsequent polls.
- **`php_readpipe()` FD_SET without an FD_SETSIZE check** — `FD_SET()`
doesn't range-check, so a read descriptor at or above `FD_SETSIZE` (1024
on glibc) wrote past the end of the on-stack `fd_set`. Added a bounds
check; on an out-of-range descriptor it marks the result undefined and
closes the slot (a review finding caught the first version recursing
without bound through `php_init()` → `php_readpipe()` under sustained
descriptor exhaustion, so it no longer restarts synchronously from
here).
- **Signal handler never re-arms, so the second SIGPIPE kills the
poller** — every fatal signal reset itself to `SIG_DFL` and never
re-armed. That's the right idiom for a real fault (SIGSEGV and friends
re-execute and die with a core), but SIGPIPE is a routine, recurring
condition — a second one previously killed the whole poller. A review
finding pointed out that re-arming only at the end still left a window
(signal dispositions are process-wide) where another thread's SIGPIPE
could hit `SIG_DFL`; fixed by never resetting SIGPIPE's disposition in
the first place.
- **spine fails to link on Fedora: undefined reference to
`usmDESPrivProtocol`** — some distributions ship a `net-snmp-config.h`
that advertises DES as the default privacy protocol while the shipped
`libnetsnmp` doesn't export the symbol. Added an `AC_LINK_IFELSE` probe
for the actual symbol and guarded the fallback to use
`usmAESPrivProtocol` when it's absent.
- **Poller results ≥1024 bytes silently truncated to 1022** —
`db_escape()` capped its internal trim buffer at a fixed `DBL_BUFSIZE`
regardless of the caller's `max_size`, so raising
`--with-results-buffer` never helped. The trim buffer now scales with
`max_size` (a review finding caught the trim-size formula going
negative/zero for `max_size` 1-3), and `escaped_result` in `poll_host()`
is sized to `RESULTS_BUFFER * 2 + 1` instead of the fixed `DBL_BUFSIZE`.
(A very large escaped result can still hit the separate, already-tracked
row-size limit in #598/#618 — this fix only addresses the escaping-stage
truncation.)
## Also included
- **Seed `spine.conf` from `spine.conf.dist` on first install** — `make
install` only ever installed `spine.conf.dist`; nothing produced a live
`spine.conf`. Added an `install-data-hook` that copies it over only when
`spine.conf` doesn't already exist, plus a matching `uninstall-hook`
that only removes it again when it's still byte-identical to the `.dist`
(so a real uninstall never discards a customized config, while `make
distcheck`'s install/uninstall symmetry check still passes).
## Dropped from this PR
- A custom `codeql.yml` workflow (originally added for #580) was removed
— the repo already has GitHub's default CodeQL setup enabled and running
(`gh api repos/Cacti/spine/code-scanning/default-setup` → `state:
configured`), and a custom "advanced configuration" workflow cannot
coexist with it; GitHub rejected the SARIF upload and failed CI. #580's
underlying gap was already resolved out of band, so this PR no longer
claims to close it.
## Testing
Full build via WSL (`./bootstrap && ./configure --enable-warnings &&
make`) succeeds with no new warnings in `poller.c`, `snmp.c`, `php.c`,
`error.c`, or `sql.c` beyond two pre-existing, unrelated
`-Wformat-truncation` warnings in `php_init()`. Confirmed the
`usmDESPrivProtocol` configure probe correctly detects the missing
symbol in this build environment and takes the AES fallback path without
a link error. `make distcheck` passes end to end.
Closes#594Closes#600Closes#605Closes#607Closes#575Closes#583
The three unit binaries in `tests/unit/` do not test the code that
ships. `test_safety_fixes.c` says so in its own header:
> the functions under test are inlined (or their exact predicate
replicated) so this translation unit compiles standalone without linking
util.o/ping.o/php.o and dragging in MySQL/SNMP
That keeps them cheap, but a fix can land in `util.c` while the test
carries on passing against its private copy. The 39 assertions currently
guarantee less than they appear to.
This adds a fourth binary that links the real translation units.
`tests/fuzz/stubs.c` already solves the hard part for the fuzz targets
by supplying the globals `spine.c` would define, so the same file is
reused here. Everything except `spine.c` is linked as built.
**Demonstration.** Reverting `strncopy()` to the issue#447 off-by-one
and running the suite:
```
PASS: tests/unit/test_util_strings
PASS: tests/unit/test_build_fixes
PASS: tests/unit/test_safety_fixes
FAIL: tests/unit/test_linked
# TOTAL: 4 # PASS: 3 # FAIL: 1
```
The three existing suites do not notice. Only the linked one does.
Seven cases to start, covering `strncopy()` bounds and termination and
`regex_replace()` match/no-match/bad-pattern. The point of the PR is the
harness rather than the case count: with this in place, future tests can
assert against real behaviour, and the existing binaries can migrate
over one at a time.
Verified in a clean `ubuntu:24.04` container: builds at 4 warnings
matching develop, `make check` passes 4/4, `make distcheck` passes.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
The suite that landed in #563 is built by its own `tests/unit/Makefile`
and executed by nothing. `Makefile.am` has no `check_PROGRAMS`,
`configure.ac` has no cmocka detection, and no workflow invokes it. So
`make check` is a no-op today, and `make distcheck` from #556 validates
only that the tarball builds.
This wires the three binaries in:
- `configure.ac` probes for `-lcmocka` and `cmocka.h` and defines
`HAVE_CMOCKA`
- `Makefile.am` lists the three as `check_PROGRAMS` and sets `TESTS`
- the `build` job runs `make check`, and `libcmocka-dev` is added where
the workflow needs it
Verified in a clean `ubuntu:24.04` container.
```
PASS: tests/unit/test_util_strings
PASS: tests/unit/test_build_fixes
PASS: tests/unit/test_safety_fixes
# TOTAL: 3 # PASS: 3 # FAIL: 0
```
`make distcheck` still passes, so the test sources are distributed
correctly.
Absence of cmocka is not fatal. Building without `libcmocka-dev`
installed:
```
configure OK without cmocka
build OK without cmocka
# TOTAL: 0 # PASS: 0 # FAIL: 0
make check exit: 0
```
The tests drop out of `check_PROGRAMS` rather than breaking the build,
which matters for platforms where cmocka is not packaged.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Two macros that autoconf removed in 2.70 and warns about on every
`autoreconf`.
- `AC_HEADER_TIME` — obsolete. Every system that provides `sys/time.h`
allows including it alongside `time.h`, so the `TIME_WITH_SYS_TIME`
ladder in `common.h` collapses to a single `HAVE_SYS_TIME_H` guard.
- `AC_TRY_COMPILE` — replaced by
`AC_COMPILE_IFELSE([AC_LANG_PROGRAM(...)])`. Mechanical; the Net-SNMP
crypto probe behaves identically (`checking if Net-SNMP needs crypto
support... yes`).
`AC_PREREQ` stays at 2.69 deliberately, so the tree still bootstraps on
EL7-era autoconf.
Verified in a clean ubuntu:24.04 container: `autoreconf -Wall` is quiet,
build is 39 warnings matching develop, and `make dist` still produces a
correct tarball.
**Note on #500**, which proposed this cleanup — both of its scope items
are wrong and I am not doing them:
- *"remove `ACLOCAL_AMFLAGS = -I m4`"* — `m4/` exists and holds the
libtool macros (`libtool.m4`, `ltoptions.m4`, `ltsugar.m4`,
`ltversion.m4`, `lt~obsolete.m4`). Removing the flag breaks `aclocal`.
- *"remove redundant `EXTRA_DIST = spine.1`"* — not redundant. I removed
it and `make dist` silently dropped the man page from the tarball;
`man_MANS` does not distribute it here. Restored.
Refs #500
---
**Release-blocking bug found while testing this.** `make distcheck`
failed on develop: the dist tarball was missing twelve headers
(`common.h`, `spine.h`, `util.h`, `sql.h`, `snmp.h`, `poller.h`,
`php.h`, `ping.h`, `locks.h`, `error.h`, `keywords.h`, `nft_popen.h`)
and `spine.conf.dist`, so a release tarball could not be compiled from.
Headers are now declared via `noinst_HEADERS` and the sample config is
in `EXTRA_DIST`. `make distcheck` passes.
**Also added**
- `--enable-sanitizers` configure option (ASan + UBSan), so a sanitizer
build is one flag rather than hand-rolled `CFLAGS`.
- `AM_SILENT_RULES([yes])` — the normal build is readable, `make V=1`
restores the full command lines.
- A `distcheck` CI job, so a tarball that cannot be compiled fails the
PR instead of shipping.
(`AC_CONFIG_MACRO_DIR([m4])` was already present at line 46; I added a
duplicate, autoconf rejected it, and I removed it.)
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Adds a README.md to the packaging directory providing an overview of
supported platforms and links to OS-specific instructions.
## Linked issue
Fixes#461
---------
Co-authored-by: netniV <netniv@hotmail.com>
Fixes#443
Companion to #401. Adds four .PHONY make targets:
make docker # build the runtime image
make docker-dev # build the dev image (ASan, cppcheck, scan-build,
valgrind)
make verify # build dev image + run the full analysis suite
make cppcheck # just cppcheck
None interfere with the normal make / make install workflow. If Docker
is not installed they fail with a clear error. Depends on #401 being
merged first.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* Correct segfault due to use of wrong variable
* Remove some whitespace
* More declarations to headers
* Add 'Device[]' to some log messages
* PID[] instead of Process[]
* Some block formatting
* Remove an automake warning
* Use stdbool.h
-bug#0000736: Solaris 8 : Alarm Clock message
-feature: enabled signal handling in cactid
plus significant header rearrangement and correction of some compile issue.
git-svn-id: svn+ssh://repo.cacti.net/var/data/svnroot/cacti/branches/BRANCH_0_8_6/cactid@3613 860744bd-22fc-0310-8c96-e9fe5004b5ca
Thomas Vincent