fix(boundaries): validate script slots and make cleanup portable

Dockerfile

@@ -1,4 +1,4 @@
-FROM mirror.gcr.io/library/debian:bookworm-slim AS builder
+FROM mirror.gcr.io/library/debian:bookworm-slim@sha256:7c7b2c966bc9ee8cedfeef67e0e279108992c77681fa595db4a9d65c06ccc587 AS builder
 
 RUN apt-get update && apt-get install -y --no-install-recommends \
         gcc \
@@ -19,7 +19,7 @@ RUN autoreconf -fi \
     && ./configure --prefix=/usr/local \
     && make -j"$(nproc)" spine
 
-FROM mirror.gcr.io/library/debian:bookworm-slim
+FROM mirror.gcr.io/library/debian:bookworm-slim@sha256:7c7b2c966bc9ee8cedfeef67e0e279108992c77681fa595db4a9d65c06ccc587
 
 RUN apt-get update && apt-get install -y --no-install-recommends \
         libmariadb3 \

Dockerfile.coverage

@@ -1,6 +1,6 @@
 # Single stage: the .gcda files land beside the objects, so /src must survive
 # into the running container.
-FROM debian:bookworm-slim
+FROM debian:bookworm-slim@sha256:7c7b2c966bc9ee8cedfeef67e0e279108992c77681fa595db4a9d65c06ccc587
 RUN apt-get update && apt-get install -y --no-install-recommends \
         gcc make autoconf automake libtool pkg-config \
         libmariadb-dev libsnmp-dev libssl-dev gcovr \

Dockerfile.dev

@@ -5,7 +5,7 @@
 # | Builds with ASan + full static analysis tooling.                       |
 # +-------------------------------------------------------------------------+
 
-FROM debian:bookworm-slim
+FROM debian:bookworm-slim@sha256:7c7b2c966bc9ee8cedfeef67e0e279108992c77681fa595db4a9d65c06ccc587
 
 RUN apt-get update && apt-get install -y --no-install-recommends \
     build-essential \

Makefile.am

@@ -252,7 +252,7 @@ tests_unit_test_php_runtime_SOURCES = tests/unit/test_php_runtime.c tests/suppor
 tests_unit_test_php_runtime_LDADD = $(CMOCKA_LIBS) $(LIBS)
 tests_unit_test_php_runtime_LDFLAGS = \
 	-Wl,--wrap=posix_spawn -Wl,--wrap=dup -Wl,--wrap=fcntl \
-	-Wl,--wrap=pthread_sigmask -Wl,--wrap=write \
+	-Wl,--wrap=write \
 	-Wl,--wrap=posix_spawnattr_setsigdefault -Wl,--wrap=posix_spawnattr_destroy
 tests_unit_test_php_runtime_CPPFLAGS = $(AM_CPPFLAGS) \
 	-DPHP_TEST_SERVER_PATH='"$(abs_builddir)/tests/support/php_test_server"' \
@@ -282,4 +282,5 @@ noinst_HEADERS += src/process/process_internal.h
 
 EXTRA_DIST += docs/architecture/source-modules.md
 
+
 EXTRA_DIST += scripts/cppcheck/spine.cfg

Makefile.in

@@ -1321,7 +1321,7 @@ tests_unit_test_php_runtime_SOURCES = tests/unit/test_php_runtime.c tests/suppor
 tests_unit_test_php_runtime_LDADD = $(CMOCKA_LIBS) $(LIBS)
 tests_unit_test_php_runtime_LDFLAGS = \
 	-Wl,--wrap=posix_spawn -Wl,--wrap=dup -Wl,--wrap=fcntl \
-	-Wl,--wrap=pthread_sigmask -Wl,--wrap=write \
+	-Wl,--wrap=write \
 	-Wl,--wrap=posix_spawnattr_setsigdefault -Wl,--wrap=posix_spawnattr_destroy
 
 tests_unit_test_php_runtime_CPPFLAGS = $(AM_CPPFLAGS) \

php.c

@@ -117,7 +117,11 @@ ssize_t php_write_no_sigpipe(int fd, const void *buffer, size_t length) {
 
 	sigemptyset(&blocked);
 	sigaddset(&blocked, SIGPIPE);
+#ifdef SPINE_PHP_RUNTIME_TESTING
+	mask_error = spine_php_test_sigmask(SIG_BLOCK, &blocked, &old_mask);
+#else
 	mask_error = pthread_sigmask(SIG_BLOCK, &blocked, &old_mask);
+#endif
 	if (mask_error != 0) {
 		errno = mask_error;
 		return -1;

scripts/package.sh

@@ -85,14 +85,19 @@ fi
 
 # Copy repository
 mkdir -p "${TMP_DIR}/cacti-spine-${VERSION}"
-if ! tar -cf - --exclude 'scripts/package.sh' --exclude '.svn' --exclude '.travis.yml' -- * | (
+if ! tar -cf - ./* | (
   cd "${TMP_DIR}/cacti-spine-${VERSION}" || exit 1
   tar -xf -
 ); then
   echo "ERROR: Unable to copy repository to ${TMP_DIR}/cacti-spine-${VERSION}" >&2
   exit 1
 fi
 
+# Native BSD tar has no GNU --exclude option. Prune obsolete metadata only
+# from the staged copy, after copying with portable tar arguments.
+find "${TMP_DIR}/cacti-spine-${VERSION}" \
+  \( -name .svn -o -name .travis.yml \) -prune -exec rm -rf -- {} +
+
 # Change working directory
 pushd "${TMP_DIR}/cacti-spine-${VERSION}" > /dev/null || exit 1
 

src/core/log.c

@@ -196,7 +196,7 @@ static bool log_to_file(const char *message) {
 	 * message preserves log rotation. */
 	int oldstate;
 	pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldstate);
-	int fd = open(set.logging.path_logfile, O_WRONLY | O_CREAT | O_APPEND, 0666);
+	int fd = open(set.logging.path_logfile, O_WRONLY | O_CREAT | O_APPEND, 0640);
 	bool success = FALSE;
 	if (fd >= 0) {
 		size_t length = strlen(message);

src/core/result.c

@@ -50,7 +50,7 @@ int all_digits(const char *string) {
 	/* empty string is not all digits */
 	if (*string == '\0') return FALSE;
 
-	while (isdigit((int) *string))
+	while (isdigit((unsigned char) *string))
 		string++;
 
 	return *string == '\0';
@@ -68,7 +68,7 @@ int all_digits(const char *string) {
  */
 int is_ipaddress(const char *string) {
 	while (*string) {
-		if ((isdigit((int) *string)) ||
+		if ((isdigit((unsigned char) *string)) ||
 			(*string == '.') ||
 			(*string == ':')) {
 			string++;

src/php/php_internal.h

@@ -43,4 +43,8 @@ extern char **environ;
 char *php_read_result(int php_process, const char *command, int allow_restart);
 void php_fail_read(int php_process, int allow_restart);
 ssize_t php_write_no_sigpipe(int fd, const void *buffer, size_t length);
+#ifdef SPINE_PHP_RUNTIME_TESTING
+/* Test seam avoids libc symbol aliases defeating linker interposition. */
+int spine_php_test_sigmask(int how, const sigset_t *mask, sigset_t *previous);
+#endif
 #endif

src/php/read.c

@@ -68,6 +68,13 @@ char *php_read_result(int php_process, const char *command, int allow_restart) {
 	}
 	result_string[0] = '\0';
 
+	/* Public readers may be called without php_cmd()'s slot validation. */
+	if (php_processes == NULL || php_process < 0 ||
+		php_process >= set.php.php_servers || php_process >= MAX_PHP_SERVERS) {
+		SET_UNDEFINED(result_string);
+		return result_string;
+	}
+
 	/* record start time */
 	begin_time = get_time_as_double();
 

src/process/fd.c

@@ -200,19 +200,30 @@ int spine_spawnattr_sigpipe_default(posix_spawnattr_t *attr) {
 int spine_reap_child_bounded(pid_t pid, int *pstat, int attempts) {
 	int attempt;
 	int eintr_budget;
+	int fast_attempts;
+	double deadline;
 	pid_t waited;
 
 	if (pstat == NULL) {
 		return -1;
 	}
 
+	if (attempts <= 0) return 1;
+	fast_attempts = attempts < NFT_PCLOSE_SPIN_ATTEMPTS ? attempts : NFT_PCLOSE_SPIN_ATTEMPTS;
+	/* BSD kernels may round a 200 us sleep to an entire scheduler tick.
+	 * Bound elapsed grace time as well as syscall attempts. */
+	deadline = spine_monotonic_time() +
+		((double) fast_attempts * NFT_PCLOSE_SPIN_USEC +
+			(double) (attempts - fast_attempts) * NFT_PCLOSE_REAP_USEC) /
+			1000000.0;
+
 	for (attempt = 0; attempt < attempts; attempt++) {
 		/* Bounded so a stream of caught signals cannot spin this call
 		 * forever instead of returning within its attempt budget. */
 		eintr_budget = 1000;
 		do {
 			waited = waitpid(pid, pstat, WNOHANG);
-		} while (waited < 0 && errno == EINTR && --eintr_budget > 0);
+		} while (waited < 0 && errno == EINTR && --eintr_budget > 0 && spine_monotonic_time() < deadline);
 
 		if (waited == pid) {
 			return 0;
@@ -229,6 +240,8 @@ int spine_reap_child_bounded(pid_t pid, int *pstat, int attempts) {
 			return -1;
 		}
 
+		if (spine_monotonic_time() >= deadline) return 1;
+
 /* The delay is load-bearing: without it the attempts are spent in
 		   nanoseconds and the caller's kill lands before the child can exit.
 

tests/regression/test_helper_scripts.sh

@@ -5,7 +5,10 @@
 set -eu
 root=$(CDPATH='' cd -- "$(dirname -- "$0")/../.." && pwd)
 work=$(mktemp -d "${TMPDIR:-/tmp}/spine-helper-contract.XXXXXX")
-trap 'rm -rf -- "$work"' EXIT HUP INT TERM
+package_version="helper-contract-$(basename "$work")"
+package_stage="/tmp/cacti-spine-${package_version}"
+package_archive="${package_stage}.tar.gz"
+trap 'rm -rf -- "$work" "$package_stage"; rm -f -- "$package_archive"' EXIT HUP INT TERM
 cd "$root"
 bash scripts/package.sh --help > "$work/help"
 grep -q 'Spine Package Script' "$work/help"
@@ -18,6 +21,22 @@ if bash scripts/package.sh > "$work/missing" 2>&1; then
     echo 'package helper accepted a missing version' >&2
     exit 1
 fi
+# Exercise the packaging archive with a local bootstrap stub.
+mkdir -p "$work/source/scripts" "$work/source/nested/.svn"
+printf obsolete > "$work/source/nested/.svn/entries"
+printf obsolete > "$work/source/nested/.travis.yml"
+cp scripts/package.sh "$work/source/scripts/package.sh"
+printf 'AC_INIT(Spine, %s, example.invalid)\n' "$package_version" > "$work/source/configure.ac"
+printf '#!/bin/sh\nexit 0\n' > "$work/source/bootstrap"
+chmod +x "$work/source/bootstrap"
+(cd "$work/source" && bash scripts/package.sh "$package_version") > "$work/package"
+tar -tzf "$package_archive" > "$work/contents"
+grep -Fxq "cacti-spine-${package_version}/scripts/package.sh" "$work/contents"
+if grep -Eq '/(\.svn|\.travis\.yml)(/|$)' "$work/contents"; then
+    echo 'package helper included obsolete checkout metadata' >&2
+    exit 1
+fi
+
 mkdir "$work/bin"
 cat > "$work/bin/make" <<'SH'
 #!/bin/sh

tests/snmpv3/Dockerfile.coverage

@@ -1,6 +1,6 @@
 # Single stage: the .gcda files land beside the objects, so /src must survive
 # into the running container.
-FROM mirror.gcr.io/library/debian:bookworm-slim
+FROM mirror.gcr.io/library/debian:bookworm-slim@sha256:7c7b2c966bc9ee8cedfeef67e0e279108992c77681fa595db4a9d65c06ccc587
 RUN apt-get update && apt-get install -y --no-install-recommends \
         gcc make autoconf automake libtool pkg-config \
         libmariadb-dev libsnmp-dev libssl-dev gcovr \

tests/snmpv3/snmpd/Dockerfile

@@ -1,4 +1,4 @@
-FROM mirror.gcr.io/library/debian:bookworm-slim
+FROM mirror.gcr.io/library/debian:bookworm-slim@sha256:7c7b2c966bc9ee8cedfeef67e0e279108992c77681fa595db4a9d65c06ccc587
 
 RUN apt-get update && apt-get install -y --no-install-recommends \
     iproute2 \

tests/test_spine_regressions.c

@@ -1375,7 +1375,12 @@ static void test_php_startup(const char *executable) {
 	STRNCOPY(set.php.path_php_server, "regression-server");
 	assert(!php_init(-2) && !php_init(set.php.php_servers));
 	STRNCOPY(set.php.path_php, "/nonexistent-spine-regression-executable");
-	assert(!php_init(0));
+	/* Some libcs return an exec error to the parent; OpenBSD reports it
+	 * through the child exit instead. Neither path may produce a READY slot. */
+	if (php_init(0)) {
+		assert(processes[0].php_state == PHP_BUSY);
+		php_close(0);
+	}
 	assert(processes[0].php_state == PHP_BUSY);
 	assert(processes[0].php_read_fd == -1 && processes[0].php_write_fd == -1);
 	/* posix_spawn reports a missing executable to the parent (or the child

tests/unit/test_log_config.c

@@ -17,6 +17,7 @@
 #include <pthread.h>
 #include <signal.h>
 #include <sys/wait.h>
+#include <sys/stat.h>
 
 #include "common.h"
 #include "spine.h"
@@ -89,6 +90,33 @@ static void test_spine_log_fits_a_musl_thread_stack(void **state) {
 	assert_int_equal(WEXITSTATUS(status), 0);
 }
 
+static void test_new_log_permissions(void **state) {
+	char directory[] = "spine-log-mode-XXXXXX";
+	char path[128];
+	struct stat info;
+	mode_t previous_umask;
+	int result;
+
+	(void) state;
+	assert_non_null(mkdtemp(directory));
+	snprintf(path, sizeof(path), "%s/poller.log", directory);
+	memset(&set, 0, sizeof(set));
+	config_defaults();
+	set.logging.log_destination = LOGDEST_FILE;
+	set.logging.logfile_processed = TRUE;
+	set.console.stdout_notty = FALSE;
+	set.console.stderr_notty = TRUE;
+	STRNCOPY(set.logging.path_logfile, path);
+	previous_umask = umask(0);
+	spine_log("permission check");
+	umask(previous_umask);
+	result = stat(path, &info);
+	unlink(path);
+	rmdir(directory);
+	assert_int_equal(result, 0);
+	assert_int_equal(info.st_mode & 0777, 0640);
+}
+
 /* --- spine.conf parsing -------------------------------------------------- */
 
 static char config_path[64];
@@ -242,6 +270,7 @@ static void test_config_last_line_without_newline_is_read(void **state) {
 int main(void) {
 	const struct CMUnitTest tests[] = {
 		cmocka_unit_test(test_spine_log_fits_a_musl_thread_stack),
+		cmocka_unit_test(test_new_log_permissions),
 		cmocka_unit_test_setup_teardown(test_config_long_key_is_not_truncated_into_a_known_key, config_setup, config_teardown),
 		cmocka_unit_test_setup_teardown(test_config_every_known_key_is_accepted, config_setup, config_teardown),
 		cmocka_unit_test_setup_teardown(test_config_long_value_is_kept_whole, config_setup, config_teardown),

tests/unit/test_php_runtime.c

@@ -70,13 +70,12 @@ static int duplicate_at_fdsetsize(int fd) {
 	return duplicate;
 }
 
-int __real_pthread_sigmask(int how, const sigset_t *set, sigset_t *oldset);
-int __wrap_pthread_sigmask(int how, const sigset_t *set, sigset_t *oldset) {
+int spine_php_test_sigmask(int how, const sigset_t *set, sigset_t *oldset) {
 	if (fail_next_sigmask && how == SIG_BLOCK) {
 		fail_next_sigmask = FALSE;
 		return EAGAIN;
 	}
-	return __real_pthread_sigmask(how, set, oldset);
+	return pthread_sigmask(how, set, oldset);
 }
 
 ssize_t __real_write(int fd, const void *buffer, size_t length);
@@ -856,7 +855,9 @@ static void test_spawn_failure_releases_every_resource(void **state) {
 	/* calloc-like zeroes reproduce the process-wide initialization that used to
 	 * let a failed slot masquerade as stdin. php_init() must replace them. */
 	memset(&php_processes[0], 0, sizeof(php_processes[0]));
-	snprintf(set.php.path_php, sizeof(set.php.path_php), "%s", "/does/not/exist/spine-php-test");
+	/* POSIX allows exec errors to appear in the child. Inject an actual
+	 * parent-side spawn failure so this exercises the cleanup on every OS. */
+	fail_php_spawn_call = 1;
 	assert_int_equal(php_init(0), FALSE);
 	assert_int_equal(php_processes[0].php_pid, -1);
 	assert_int_equal(php_processes[0].php_read_fd, -1);
@@ -881,6 +882,28 @@ static void test_command_rejects_a_writable_poisoned_slot(void **state) {
 	php_processes[0].php_read_fd = php_processes[0].php_write_fd = -1;
 }
 
+static void test_readpipe_rejects_invalid_slots(void **state) {
+	const int slots[] = {-1, MAX_PHP_SERVERS, set.php.php_servers};
+	size_t index;
+	php_t *saved_processes;
+	char *result;
+
+	(void) state;
+	for (index = 0; index < sizeof(slots) / sizeof(slots[0]); index++) {
+		result = php_readpipe(slots[index], "test");
+		assert_non_null(result);
+		assert_string_equal(result, "U");
+		free(result);
+	}
+	saved_processes = php_processes;
+	php_processes = NULL;
+	result = php_readpipe(0, "test");
+	php_processes = saved_processes;
+	assert_non_null(result);
+	assert_string_equal(result, "U");
+	free(result);
+}
+
 static void test_readpipe_rejects_fd_at_fd_setsize(void **state) {
 	char command[] = "test";
 	char *result;
@@ -1101,6 +1124,7 @@ int main(void) {
 		cmocka_unit_test_setup_teardown(test_init_timeout_does_not_recurse, php_setup, php_teardown),
 		cmocka_unit_test_setup_teardown(test_spawn_failure_releases_every_resource, php_setup, php_teardown),
 		cmocka_unit_test_setup_teardown(test_command_rejects_a_writable_poisoned_slot, php_setup, php_teardown),
+		cmocka_unit_test_setup_teardown(test_readpipe_rejects_invalid_slots, php_setup, php_teardown),
 		cmocka_unit_test_setup_teardown(test_readpipe_rejects_fd_at_fd_setsize, php_setup, php_teardown),
 		cmocka_unit_test_setup_teardown(test_startup_read_rejects_fd_at_fd_setsize_without_restart, php_setup, php_teardown),
 		cmocka_unit_test_setup_teardown(test_command_retires_fd_at_fd_setsize, php_setup, php_teardown),