fix(config): length-bounded tokenizer, range checks, umask, credential scrub by somethingwithproof · Pull Request #539 · Cacti/spine

somethingwithproof · 2026-04-17T19:27:37Z

Three fixes for config parsing and startup hygiene.

Scope

Replace sscanf("%15s %255s", ...) in read_spine_config with a length-bounded tokenizer. Previous parser silently truncated keywords > 15 bytes, values > 255 bytes, and passwords with spaces. Adds range checks for DB_Port, RDB_Port (1-65535), CircuitBreakerThreshold (1-1000000), and DB_UseSSL/RDB_UseSSL (0 or 1). Opens spine.conf with O_NOFOLLOW|O_CLOEXEC.
umask(027) at startup before any file creation.
Extract spine_scrub_secrets() and call from both the clean-exit path and die(). Uses explicit_bzero where available.

Verification

21/21 tests pass. New tests: test_config_parser (keyword length, spaces in value, overlong line, embedded NUL, port range, symlink), test_startup_umask, test_scrub_secrets.

Risk

Operators with non-standard config files that rely on the prior silent truncation will now see a clear rejection. This is the intended behavior.

- Add CMakeLists.txt mirroring configure.ac feature checks - Add config/config.h.cmake.in template for cmake builds - Add build-cmake-linux CI job (gcc/clang matrix) - Add build-windows CI job (MSYS2/MinGW-w64, continue-on-error) - Windows crash dump collection via WER LocalDumps - Gate -Wall by compiler ID (GNU/Clang vs MSVC) - Use CMAKE_DL_LIBS instead of hardcoded -ldl - Parse net-snmp-config --libs into proper NETSNMP_LIBRARIES - Add SNMP_LOCALNAME compile check for feature parity - No C source files modified Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

spine.h depends on types from common.h (MYSQL, pid_t, size_t, pthread types, RESULTS_BUFFER from config.h). Add a compile-time guard that produces a clear error if spine.h is included without common.h, rather than cascading type errors. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

strncasecmp() returns 0 on match, but the comparisons used the raw return value as truthy, inverting the logic. TCP matched non-TCP strings and vice versa. Also fix: comparisons against 'hostname' instead of 'token' (lines 1020, 1032), and strncasecmp length 3 for 4-char strings "TCP6"/"UDP6" (should be 4). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

getarg(opt, &argv) advances the argv pointer on each call. Three successive calls in the --mode handler consumed three argv entries instead of one, corrupting subsequent argument parsing when using the space-separated form (--mode online). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

…t vars Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

…ncation Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

platform_sandbox_linux.c uses uint64_t in add_path_rule() and the apply_landlock() ro_roots loop but the HAVE_LANDLOCK block only included linux/landlock.h and sys/syscall.h. On gcc without a transitively-included stdint.h the type is unknown, which cascades into a parse failure on the add_path_rule signature and then an implicit-declaration error at the first call site (lines 110, 221, 247 in the CI error report). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

glibc's <features.h> freezes the feature-test bitmap on first inclusion. platform.h pulls <time.h> and platform_process.h pulls <sys/types.h>, both of which reach <features.h> before CMake's -D_GNU_SOURCE=1 flag can take effect in the preprocessor pipeline on clang/Linux. Without the macro visible at that point, pthread_setname_np and pipe2 remain undeclared, producing -Werror=implicit-function-declaration failures. Add a `#if defined(__linux__) && !defined(_GNU_SOURCE)` guard at the top of each affected TU so the macro is always set before any system header is included, regardless of include order or compiler. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Replace the bash case pattern *[^A-Za-z0-9._+-\ \t]*) with a tr-based check. The old pattern treated +-\ as a character range (0x2B-0x5C), which excluded space and tab and caused all multi-package strings like 'cmake make pkg-config gcc' to be rejected. The new approach pipes the input through LC_ALL=C tr -d with the explicit allowed set (alnum, . _ + - space tab); any bytes surviving the deletion are disallowed characters, reported verbatim in the error message. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

…rns -1 nft_pchild returns -1 on lookup failure. kill(-1, SIGKILL) signals every process owned by the spine uid; kill(0, ...) hits the whole process group. Accept only pids > 1. Type the local to spine_pid_t so Windows stays aligned with the POSIX pid_t width. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

SECURITY.md promises spine refuses to start when spine.conf has any group or world bit set or is owned by someone other than root or the startup euid. The old soft-warn path leaked credentials on a misconfigured deploy. Open the file with O_NOFOLLOW so a planted symlink at /etc/spine.conf cannot redirect credential loading, add O_CLOEXEC so the fd does not leak into poll-script children, and treat ELOOP, mode drift, and owner mismatch as fatal. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

The previous handler called stdio, time(3), localtime(3), strftime(3), and exit(3). None are async-signal-safe, so a SIGSEGV mid-allocation could deadlock inside libc. Pre-format per-signal messages at startup, emit with write(2), and use _exit(128 + signo) on SIGSEGV/SIGBUS/SIGFPE/ SIGABRT to skip atexit handlers. Drop SIGPIPE from the fatal set and ignore it process-wide so a poll script closing stdout early no longer kills spine. Remove the redundant legacy signal() loop. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

…e into children Every nft_popen'd poll script used to inherit the full descriptor table, including privileged ICMP sockets, the authenticated MySQL connection, the audit netlink fd, and the log file. A poll script that shells out could reuse any of them. Set cloexec at creation time: SOCK_CLOEXEC atomically on Linux/BSD with fcntl fallback for macOS, fcntl after each raw ICMP socket(), the MySQL net.fd after connect, the audit fd after audit_open(), and O_CLOEXEC on the log open(). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Two bugs: strncpy(name->hostname, hostname, sizeof(name->hostname)) leaves the buffer unterminated when the source equals or exceeds the bound, and strncpy(...) followed by name->hostname[strlen(token)] = 0 writes one-past-the-buffer for tokens >= sizeof. Funnel both branches through strncopy(), which caps at the bound and always NUL-terminates. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

SNMPv3 engine IDs are opaque octet strings (RFC 3411). strlen() on the engine-ID buffer truncates at the first 0x00, which any RFC-compliant engine ID is free to contain. Decode the Cacti hex-string form at DB load time, carry bytes + length in target_t and host_t, and pass both to snmp_host_init(). The hex path stays as a transitional fallback. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR hardens Spine’s config/startup/credential handling while expanding platform portability infrastructure (POSIX/Windows abstractions, sandbox hooks) and strengthening CI, packaging, and integration test coverage.

Changes:

Improves startup/config hygiene (secret scrubbing hooks, safer signal handling, tighter env handling for spawned children, systemd notify helpers).
Introduces/expands cross-platform abstraction layers (platform_*, ICMP/socket/fd/process/error wrappers, sandbox stubs/implementations).
Updates packaging/CI/test infrastructure (CMake+ninja packaging, new workflows, new unit + integration tests, SNMPv3 fixture tweaks).

Reviewed changes

Copilot reviewed 137 out of 182 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tests/unit/test_build_fixes.c	Adjusts unit test assertions and struct layout expectations.
tests/unit/test_arc4random.c	Adds BSD-only arc4random smoke test.
tests/unit/json_escape_tu.c	Adds standalone json escape helper for unit tests.
tests/unit/build_child_env_tu.c	Adds standalone env-scrub helper for unit tests.
tests/unit/Makefile	Reworks unit test makefile to build platform smoke tests.
tests/snmpv3/snmpd/snmpv3.conf	Aligns SNMPv3 auth protocol with fixtures (SHA).
tests/snmpv3/scripts/run_test.sh	Improves script structure/robustness for SNMPv3 test phases.
tests/snmpv3/docker-compose.yml	Updates healthcheck SNMP auth protocol to SHA.
tests/snmpv3/db/init.sql	Updates seeded SNMP auth protocol to SHA.
tests/integration/test_output_regex.sh	Improves script structure and uses `--no-deps` for docker runs.
tests/integration/test_ipv6_transport.sh	Adds IPv6 transport integration test.
tests/integration/test_db_column_detect.sh	Improves script structure and uses `--no-deps` for docker runs.
tests/integration/smoke_test.sh	Improves script structure; adjusts SNMP auth protocol; modifies error handling.
src/util.h	Adds declarations for secret scrubbing/startup euid capture; adds CLI helpers.
src/systemd_notify.h	Adds systemd sd_notify integration interface.
src/systemd_notify.c	Adds sd_notify implementation with no-op fallback.
src/sql.h	Fixes macro termination formatting.
src/spine_probes.h	Adds USDT probe macros (Linux sys/sdt.h optional).
src/spine_audit.h	Adds audit event interface with rate limiting + test seams.
src/spine_audit.c	Implements audit logging with token-bucket limiting and cached fd.
src/spine.h	Enforces include order; adds logging format constants and config fields; widens SNMP protocol buffers; adds binary engine-id storage; switches php_pid type.
src/snmp_engine_id.c	Adds standalone SNMPv3 engine-id hex decoder.
src/snmp.h	Extends snmp_host_init signature to accept binary engine-id path; exposes decoder prototype.
src/snmp.c	Uses platform setenv; adds probes; zeroes sensitive buffers; prefers binary engine-id; fixes localname cleanup on peername alloc failure.
src/platform/platform_win.c	Adds Windows platform backend (sleep/env/thread-name/job object).
src/platform/platform_socket_win.c	Adds Windows socket wrapper API.
src/platform/platform_socket_posix.c	Adds POSIX socket wrapper API with CLOEXEC handling.
src/platform/platform_socket.h	Adds cross-platform socket wrapper API types/prototypes.
src/platform/platform_sandbox_win.c	Adds Windows sandbox no-op stubs.
src/platform/platform_sandbox_posix.c	Adds generic POSIX sandbox no-op stub TU.
src/platform/platform_sandbox_openbsd.c	Adds OpenBSD unveil/pledge implementation.
src/platform/platform_sandbox_freebsd.c	Adds FreeBSD sandbox no-op with Capsicum rationale.
src/platform/platform_sandbox.h	Adds sandbox abstraction header.
src/platform/platform_process_posix.c	Adds POSIX process wrapper (pipe CLOEXEC, spawn retry, wait/terminate).
src/platform/platform_process.h	Adds cross-platform process wrapper API and spine_pid_t.
src/platform/platform_posix.c	Adds POSIX platform backend (sleep, localtime, thread naming).
src/platform/platform_icmp_posix.c	Adds POSIX ICMP wrappers forwarding into ping.c helpers.
src/platform/platform_icmp.h	Adds cross-platform ICMP abstraction API.
src/platform/platform_fd_win.c	Adds Windows fd read/write/wait wrapper.
src/platform/platform_fd_posix.c	Adds POSIX fd read/write/wait wrapper.
src/platform/platform_fd.h	Adds cross-platform fd wrapper header.
src/platform/platform_error_win.c	Adds Windows error-to-string helper.
src/platform/platform_error_posix.c	Adds POSIX error-to-string helper supporting GNU/XSI strerror_r.
src/platform/platform_error.h	Adds error-to-string helper header.
src/platform/platform_common.c	Adds init/cleanup refcounted platform initializer.
src/platform/platform.h	Adds general platform abstraction header.
src/ping_wire.h	Centralizes ICMP wire payload layout + validator API contract.
src/ping_validate.c	Adds standalone ICMP payload validator TU.
src/ping_ipv6_scope.c	Adds standalone IPv6 scope_id resolver TU.
src/ping.h	Removes large Cygwin ICMP struct block; clarifies ping_init.
src/php.c	Switches to platform fd/process wrappers; adds env scrubbing for spawned PHP; improves spawn error reporting.
src/nft_popen.h	Documents/export spine_build_child_env().
src/error.h	Exposes spine_signal_handler_init().
src/error.c	Reworks signal handling to async-signal-safe write/_exit model + SIGPIPE ignore.
src/common.h	Removes Cygwin-specific hacks; includes platform header.
src/circuit_breaker.h	Adds per-host circuit breaker interface.
src/circuit_breaker.c	Implements per-host breaker with mutex + audit logging and cooldown.
scripts/verify.sh	Updates scan-build to run through CMake build dir; adjusts binary path.
scripts/test-workflows.sh	Adds local workflow testing helper (act/policy/distro wrapper).
scripts/test-vagrant.sh	Adds Vagrant runner for BSD/niche OS testing.
scripts/test-distros.sh	Adds Docker distro build matrix script with reporting.
packaging/rpm/spine.spec	Migrates RPM build to CMake + Ninja macros.
packaging/rpm/README	Notes RPM spec uses CMake + Ninja.
packaging/debian/rules	Migrates Debian buildsystem to cmake+ninja with flags.
packaging/debian/control	Replaces autotools deps with cmake/ninja build deps.
packaging/debian/README	Notes Debian packaging uses CMake + Ninja.
packaging/README.md	Updates packaging docs to reflect CMake-based builds.
package	Updates packaging script checks/version extraction; validates CMake configure.
flake.nix	Adds Nix flake for build + dev shell.
etc/systemd/spine.timer	Adds systemd timer unit for periodic polling.
etc/systemd/spine.service	Adds hardened systemd service unit leveraging sd_notify, caps, and sandboxing.
etc/spine.conf.dist	Documents Script_Policy option in sample config.
etc/selinux/spine.te	Adds SELinux skeleton policy module.
etc/selinux/spine.if	Adds SELinux interfaces for domain transition/log/pid.
etc/selinux/spine.fc	Adds SELinux file contexts for spine paths.
etc/apparmor.d/usr.local.spine.bin.spine	Adds AppArmor profile for spine runtime restriction.
error.c	Removes legacy top-level error.c (moved/rewritten under src/).
docs/security-selinux.md	Adds SELinux enablement documentation.
docs/security-hardening.md	Adds runtime hardening guidance (allocators, profiles, sandbox interplay).
docs/security-alerts.md	Adds audit trail for dismissed static analysis alerts.
docs/platform-idioms.md	Documents platform-specific coding/runtime idioms.
docs/debugging.md	Adds debugging guidance (gdbserver, systemd overrides, USDT).
debug	Minor shell style cleanup for SPINE_CONFIG export.
config/config.h.cmake.in	Adds CMake-configured config.h template.
bootstrap	Removes autotools bootstrap script.
Vagrantfile	Adds Vagrant matrix for BSD/Alpine VM testing.
VERSION	Adds version file (1.3.0).
SECURITY.md	Adds security policy/trust model/deployment guidance.
Makefile.am	Removes autotools build manifest.
Dockerfile.dev	Migrates dev image build to CMake + Ninja and sanitizer flags.
Dockerfile	Migrates build to CMake + Ninja; adds non-root user and config copy.
CONTRIBUTING.md	Adds contribution/testing guidance and local CI reproduction instructions.
CMakePresets.json	Adds CI-oriented CMake presets for smoke/main builds.
CHANGELOG	Adds notes about platform and testing direction.
.github/workflows/weekly.yml	Adds weekly deeper checks workflow.
.github/workflows/security-posture.yml	Adds security posture workflow (TruffleHog/Semgrep/Scorecard/policy).
.github/workflows/release.yml	Adds release build/SBOM/cosign signing workflow.
.github/workflows/release-verification.yml	Adds hardened release verification pipeline and attestations.
.github/workflows/oci.yml	Adds GHCR OCI publish workflow with cosign signing.
.github/workflows/integration.yml	Adds/updates integration matrix and Docker integration test lane.
.github/workflows/fuzzing.yml	Adds sanitizer-based CLI fuzz smoke workflow.
.github/workflows/coverage.yml	Adds gcc+lcov coverage workflow and gate.
.github/workflows/codeql.yml	Adds CodeQL analysis workflow.
.github/scripts/cppcheck_to_sarif.py	Adds converter from cppcheck output to SARIF.
.github/scripts/clang_tidy_to_sarif.py	Adds converter from clang-tidy output to SARIF.
.github/scripts/check-workflow-policy.py	Adds workflow hygiene policy checker.
.github/scripts/check-unsafe-api-additions.sh	Adds CI guard to block newly introduced unsafe C APIs.
.github/scripts/check-leak-trend.py	Adds log parser/enforcer for leak trend baselines.
.github/perf-baseline.json	Adds perf baseline thresholds for smoke commands.
.github/nightly-leak-baseline.json	Adds nightly leak baseline thresholds (valgrind/asan).
.github/instructions/instructions.md	Updates contributor guidelines with API/documentation rules + CI guardrails.
.github/cppcheck-baseline.txt	Adds cppcheck baseline placeholder.
.github/actions/install-apt-deps/action.yml	Adds hardened composite action for apt dependency install.
.dockerignore	Broadens ignored paths to reduce build context size.
.devcontainer/devcontainer.json	Adds devcontainer config for Ubuntu 24.04 C++ environment.
.clang-tidy	Adds clang-tidy configuration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-17T19:30:29Z

+  fi
 }
-trap cleanup EXIT
+# trap cleanup EXIT


Commenting out the EXIT trap disables cleanup on early exits (including failures), which can leave Docker resources (containers/volumes/networks) behind and make subsequent test runs flaky. Re-enable trap cleanup EXIT (or ensure cleanup runs reliably on all exit paths).

Suggested change

# trap cleanup EXIT

trap cleanup EXIT

Copilot · 2026-04-17T19:30:29Z

+static void spine_nanosleep_us(unsigned long microseconds) {
+	struct timespec req;
+	req.tv_sec  = (time_t)(microseconds / 1000000UL);
+	req.tv_nsec = (long)((microseconds % 1000000UL) * 1000UL);
+	while (nanosleep(&req, &req) == -1 && (req.tv_sec > 0 || req.tv_nsec > 0)) {
+		/* Resume after EINTR with remaining time. */
+	}
+}


The nanosleep retry loop currently retries on any nanosleep failure as long as req remains non-zero; if nanosleep fails with a non-EINTR error (e.g., EINVAL), req may remain unchanged and this can loop indefinitely. Gate the retry on errno == EINTR and break/return on other errors.

Copilot · 2026-04-17T19:30:30Z

+	int    notified;
+};
+
+static struct rate_bucket g_buckets[AUDIT_BUCKET__COUNT];


spine_audit_event()/rate_allow() mutate shared globals (g_buckets and g_audit_fd) without any synchronization. If audit events can be emitted from multiple worker threads, this introduces data races (lost increments, multiple concurrent audit_open calls, inconsistent rate limiting). Add a mutex around rate limiting + fd initialization, or switch to atomic counters and a thread-safe one-time init for g_audit_fd.

Copilot · 2026-04-17T19:30:30Z

+static int rate_allow(enum audit_bucket b, const char *op) {
+	time_t now = g_clock();
+	struct rate_bucket *rb = &g_buckets[b];
+
+	if (rb->window_start == 0 || now - rb->window_start >= SPINE_AUDIT_RATE_WINDOW_SECS) {
+		rb->window_start = now;
+		rb->count = 0;
+		rb->notified = 0;
+	}
+
+	if (rb->count < SPINE_AUDIT_RATE_MAX) {
+		rb->count++;
+		return 1;
+	}
+
+	if (!rb->notified) {
+		fprintf(stderr, "NOTE: audit rate limit reached for op=%s\n",
+		        op ? op : "(null)");
+		rb->notified = 1;
+	}
+	return 0;
+}
+
+void spine_audit_event(const char *op, const char *detail, int result) {
+	if (!rate_allow(classify(op), op)) return;
+
+#ifdef HAVE_LIBAUDIT
+	if (g_audit_fd == -2) return;
+	if (g_audit_fd == -1) {
+		g_audit_fd = audit_open();
+		if (g_audit_fd < 0) {
+			g_audit_fd = -2;
+			return;
+		}


spine_audit_event()/rate_allow() mutate shared globals (g_buckets and g_audit_fd) without any synchronization. If audit events can be emitted from multiple worker threads, this introduces data races (lost increments, multiple concurrent audit_open calls, inconsistent rate limiting). Add a mutex around rate limiting + fd initialization, or switch to atomic counters and a thread-safe one-time init for g_audit_fd.

Copilot · 2026-04-17T19:30:30Z

+          export SPINE_DB_NAME="${DB_NAME}"
+          export SPINE_DB_USER="${DB_USER}"
+          export SPINE_DB_PASS="${DB_PASS}"
+          ctest --test-dir build --output-on-failure || echo "::notice::ctest returned non-zero."


This step currently masks test failures: ctest ... || echo will return success even when tests fail, so the Integration workflow won't gate regressions. If failures should fail CI, remove the || echo ... or replace it with a block that emits the notice and exits non-zero.

Suggested change

ctest --test-dir build --output-on-failure || echo "::notice::ctest returned non-zero."

if ! ctest --test-dir build --output-on-failure; then

echo "::notice::ctest returned non-zero."

exit 1

fi

Copilot · 2026-04-17T19:30:31Z

+/* Send STOPPING=1 with an optional human-readable reason. Called from the
+ * shutdown path and from fatal signal handlers. reason may be NULL. */


The comment states this function is called from fatal signal handlers, but sd_notifyf() (and the formatting/stdio underneath it) is not async-signal-safe. Either update the comment to explicitly state it must not be called from raw signal handlers, or provide a separate async-signal-safe stopping notifier that only uses signal-safe syscalls.

Suggested change

/* Send STOPPING=1 with an optional human-readable reason. Called from the

* shutdown path and from fatal signal handlers. reason may be NULL. */

/* Send STOPPING=1 with an optional human-readable reason. Call from the

* normal shutdown path only; this helper is not async-signal-safe and must

* not be invoked from raw signal handlers. reason may be NULL. */

Copilot · 2026-04-17T19:30:31Z

+		} else if (c == '\t') {
+			dst[i++] = '\\'; dst[i++] = 't';
+		} else if (c < 0x20) {
+			i += (size_t)snprintf(dst + i, dst_len - i, "\\u%04x", c);


If snprintf returns a negative value (allowed by the standard on encoding/other failures), casting to size_t will wrap and can make i jump forward, leading to out-of-bounds writes at dst[i] = '\\0'. Handle snprintf returning < 0 by breaking/terminating the output safely (or treating it as 0 chars written).

Suggested change

i += (size_t)snprintf(dst + i, dst_len - i, "\\u%04x", c);

int written = snprintf(dst + i, dst_len - i, "\\u%04x", c);

if (written < 0) {

break;

}

i += (size_t)written;

Copilot · 2026-04-17T19:30:31Z

+		if (fcntl(pipe_fds[0], F_SETFD, FD_CLOEXEC) == -1 ||
+			fcntl(pipe_fds[1], F_SETFD, FD_CLOEXEC) == -1) {


Setting FD flags with F_SETFD, FD_CLOEXEC overwrites any existing descriptor flags. While typically only FD_CLOEXEC is used, the more robust pattern is F_GETFD then F_SETFD with old_flags | FD_CLOEXEC (as done elsewhere in this PR) to avoid accidentally clearing other FD flags.

Suggested change

if (fcntl(pipe_fds[0], F_SETFD, FD_CLOEXEC) == -1 ||

fcntl(pipe_fds[1], F_SETFD, FD_CLOEXEC) == -1) {

int fd0_flags = fcntl(pipe_fds[0], F_GETFD);

int fd1_flags = fcntl(pipe_fds[1], F_GETFD);

if (fd0_flags == -1 ||

fd1_flags == -1 ||

fcntl(pipe_fds[0], F_SETFD, fd0_flags | FD_CLOEXEC) == -1 ||

fcntl(pipe_fds[1], F_SETFD, fd1_flags | FD_CLOEXEC) == -1) {

+	size_t body_len = BUFSIZE + 64;
+	char *body = malloc(body_len + 2);
+	ASSERT_TRUE(body != NULL);
+	memcpy(body, "DB_Host ", 8);


 char *exec_poll(host_t *current_host, char *command, int id, const char *type) {
 	int cmd_fd;
-	int pid;
+	spine_pid_t pid;


+			file, lineno, p1, p2_cap - 1);
+		return -1;
+	}
+	if (vlen > 0) {


+	char *body = malloc(body_len + 2);
+	ASSERT_TRUE(body != NULL);
+	memcpy(body, "DB_Host ", 8);
+	memset(body + 8, 'x', body_len - 8);


+	ASSERT_TRUE(body != NULL);
+	memcpy(body, "DB_Host ", 8);
+	memset(body + 8, 'x', body_len - 8);
+	body[body_len] = '\n';


+	memcpy(body, "DB_Host ", 8);
+	memset(body + 8, 'x', body_len - 8);
+	body[body_len] = '\n';
+	body[body_len + 1] = '\0';


H1 (SECURITY.md) refuses startup when spine.conf has any group or world bit set or is owned by someone other than root or the startup euid. The integration fixtures shipped the file at the COPY/checkout default of 0644, which H1 correctly rejects. Dockerfile: add RUN chmod 0600 after the COPY so the baked conf (owned by root, mode 0600) satisfies both H1 checks. This fixes the Docker Integration Tests, Poll Timing Benchmark, and build-cmake-linux- sanitizers jobs that use the production image without a bind-mount override. docker-compose.yml: the bind-mount of tests/snmpv3/spine/spine.conf arrives at 0644 from the git checkout and cannot be chmod'd through a :ro mount. Switch to a staging mount at spine.conf.src, set user: "0:0" so the process euid is root, and add a wrapper entrypoint that installs the staged conf to /tmp/spine.conf at 0600 before exec-ing spine. ci.yml / distro-matrix.yml: add chmod 0600 tests/snmpv3/spine/spine.conf before the cmake build in both FreeBSD jobs so ctest passes on FreeBSD 14 (Tier 1). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

…hecks read_spine_config() relied on sscanf("%15s %255s"), which silently truncated keywords > 15 bytes and values > 255 bytes and dropped the remainder of any value containing whitespace. The new tokenizer reads whole lines, rejects over-length lines, embedded NULs, and overlong keywords with a warning, and preserves interior whitespace so passwords with spaces round-trip. DB_Port, RDB_Port, DB_UseSSL, RDB_UseSSL, and CircuitBreakerThreshold now range-check via strtoul. Config open uses O_NOFOLLOW|O_CLOEXEC so a swapped symlink cannot redirect credential reads, and ELOOP is reported distinctly. Security-relevant warnings route through spine_config_warn() so systemd captures them unconditionally. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Spine inherits whatever umask the service manager hands it, commonly 0022 on systemd. A 0022 mask leaves log files group-readable, which matters because spine.log captures query errors that can include hosts, user names, and SQL fragments. umask(027) is installed immediately after spine_capture_startup_euid(), before any fd is opened, so the log file, pid file, and any temp files land as 0640. The prior umask is stashed and logged at debug level so operators can see what they inherited. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

The inline scrub in main() only fired on the happy path. die() bypassed it, leaving credentials in memory for any post-mortem that catches the fatal-error exit (core dump, journal rotation, etc.). Extracted the logic to spine_scrub_secrets() in util.c so both paths go through the same code. The helper uses explicit_bzero() when available (glibc >= 2.25, musl, *BSD) and a volatile-pointer memset fallback elsewhere. Signal handlers remain untouched; they must stay AS-safe and scrubbing from them is not. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

mkstemp(3) honours the process umask, so on Linux with umask 022 temp files land at 0644. open(2) mode applies only when O_CREAT creates a new file; re-opening an existing mkstemp path with O_CREAT|O_TRUNC,0600 leaves the inode mode untouched. H1 then rejects every test config with rc=-1 before the parser is even reached. Replace write_conf with make_test_conf, which calls fchmod(fd, 0600) on the mkstemp fd before writing content, matching the pattern used in test_config_perms.c. Also add umask(077) and spine_capture_startup_euid() at the top of main so the owner check passes (the test process creates the file, so geteuid() matches st_uid). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

somethingwithproof · 2026-04-18T00:47:17Z

Superseded by consolidated PR #535. Config parser tokenizer + range checks + umask + credential scrub are now on feat/distro-test-matrix via fast-forward.

somethingwithproof added 30 commits April 8, 2026 00:28

ci(windows): fail PRs on broken windows builds

63491a9

ci(actions): enable fork branch and manual runs

b7ebf54

ci(windows): fix broken msys2 action pin

025b6ae

ci(windows): skip build when net-snmp is unavailable

718fa56

ci(actions): repin to node24-compatible actions

122286d

ci(actions): repin upload-artifact to v6

bbfd67a

test(platform): add ctest smoke and windows runtime layer

c812844

refactor(platform): split posix and windows backends

f68d151

test(platform): add socket layer and focused ctest matrix

cf63246

refactor(platform): extract process and error helpers

cf39d72

feat(platform): implement native process backend coverage

e8e008b

docs(ci): clarify platform support matrix

6f6a765

fix(platform): use native process handles on windows

8ecc5b0

refactor(platform): abstract pipe io across runtimes

4501504

docs(windows): prefer msys2 over cygwin

007a811

refactor(ping): hide cygwin socket quirks behind platform helpers

04ee6be

feat(ping): allow ipv6 transport for tcp and udp

41ff3bd

feat(ping): add icmpv6 echo support

a8b05a5

test(socket): add ipv6 loopback smoke coverage

86b9531

test(socket): add loopback integration coverage

eb0347c

build(cmake): make target graph and ci more idiomatic

2864526

build: remove legacy autotools and cygwin paths

37cbeaf

fix(ping/php): restore tcp/udp path and correct script buffer bound

8ef4414

ci: add comprehensive CI/CD pipeline with security hardening (Cacti#462)

335243a

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

fix(cli): check strchr return for NULL before dereference in -O handler

0482cb3

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

fix(log): add missing break statements in get_date_format switch

bcde312

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

somethingwithproof added 17 commits April 17, 2026 02:18

fix(apparmor): drop /tmp/** catch-all, add /root and /etc/shadow denies

bf44e35

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

feat(sandbox-linux): narrow landlock rw roots to /tmp/spine prefix

f63861a

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

docs(selinux): warn against enforcing mode until audit2allow pass

6a3a736

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

docs(platforms): record FreeBSD Capsicum as a documented no-op

0f7436e

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

feat(env-scrub): strip entire LD_/DYLD_ namespace and interpreter ini…

1cef194

…t vars Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

fix(build): include sys/prctl.h on Linux for prctl/PR_SET_DUMPABLE

eb6aa27

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

fix(build): replace strncat/strncpy with memcpy to fix -Wstringop-tru…

7bf9d4b

…ncation Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

fix(build): clamp trim_limit to buffer size to fix -Wformat-truncation

82c8f7e

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Copilot AI review requested due to automatic review settings April 17, 2026 19:27

Copilot AI reviewed Apr 17, 2026

View reviewed changes

github-advanced-security AI found potential problems Apr 17, 2026

View reviewed changes

somethingwithproof added 5 commits April 17, 2026 17:17

somethingwithproof force-pushed the fix/config-parser-hardening branch from ae804a3 to 77b31ba Compare April 18, 2026 00:31

somethingwithproof closed this Apr 18, 2026

somethingwithproof mentioned this pull request Apr 18, 2026

feat: cross-platform CI matrix, sandbox hardening, and operator tooling #535

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(config): length-bounded tokenizer, range checks, umask, credential scrub#539

fix(config): length-bounded tokenizer, range checks, umask, credential scrub#539
somethingwithproof wants to merge 199 commits intoCacti:developfrom
somethingwithproof:fix/config-parser-hardening

somethingwithproof commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

somethingwithproof commented Apr 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		/* Send STOPPING=1 with an optional human-readable reason. Called from the
		* shutdown path and from fatal signal handlers. reason may be NULL. */

-/* Send STOPPING=1 with an optional human-readable reason. Called from the
- * shutdown path and from fatal signal handlers. reason may be NULL. */
+/* Send STOPPING=1 with an optional human-readable reason. Call from the
+ * normal shutdown path only; this helper is not async-signal-safe and must
+ * not be invoked from raw signal handlers. reason may be NULL. */

-			i += (size_t)snprintf(dst + i, dst_len - i, "\\u%04x", c);
+			int written = snprintf(dst + i, dst_len - i, "\\u%04x", c);
+			if (written < 0) {
+				break;
+			}
+			i += (size_t)written;

		if (fcntl(pipe_fds[0], F_SETFD, FD_CLOEXEC) == -1 \|\|
		fcntl(pipe_fds[1], F_SETFD, FD_CLOEXEC) == -1) {

-		if (fcntl(pipe_fds[0], F_SETFD, FD_CLOEXEC) == -1 ||
-			fcntl(pipe_fds[1], F_SETFD, FD_CLOEXEC) == -1) {
+		int fd0_flags = fcntl(pipe_fds[0], F_GETFD);
+		int fd1_flags = fcntl(pipe_fds[1], F_GETFD);
+		if (fd0_flags == -1 ||
+			fd1_flags == -1 ||
+			fcntl(pipe_fds[0], F_SETFD, fd0_flags | FD_CLOEXEC) == -1 ||
+			fcntl(pipe_fds[1], F_SETFD, fd1_flags | FD_CLOEXEC) == -1) {

Conversation

somethingwithproof commented Apr 17, 2026

Scope

Verification

Risk

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

somethingwithproof commented Apr 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants