chore(test): gate test-only DB short-circuit behind CACTI_TEST_BOOTSTRAP env (#7128)

* chore(test): gate test-only DB short-circuit behind CACTI_TEST_BOOTSTRAP env

The previous `!defined('PHP_TESTING')` guard skipped db_connect_real
whenever PHP_TESTING was set anywhere in the include chain. A stray
define from a CLI tool or third-party script could disable real DB
connection logic in a deployed environment.

Switch the guard to require both `PHP_TESTING` and an explicit
`CACTI_TEST_BOOTSTRAP=1` env var, and replace the silent-no-connect
path with a `Cacti_TestDbSentinel` whose every method throws. Any
production code path that accidentally treats the sentinel as a real
DB handle will fail loudly instead of silently misbehaving.

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

* fix(test): apply Copilot review feedback for test DB sentinel guard

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

* test: collapse remaining test-bootstrap guard to $is_test_bootstrap

The previous review fix introduced a local $is_test_bootstrap flag but left
two call sites still inlining defined('PHP_TESTING') && getenv(...). Replace
the long-form check and guard the single-poller cacti_db_version fetch so the
sentinel branch finishes initialising without invoking the DB.

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

* chore(test): arm CACTI_TEST_BOOTSTRAP in test bootstraps and fail closed on PHP_TESTING alone

Set the env var where PHP_TESTING is defined so legitimate test runs short-circuit the DB connect again, gate the config.php.dist swap and schema-validation skip on the combined predicate, read it with local_only, and always seed cacti_db_version.

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

* ci: ignore phpstan function.alreadyNarrowedType identifier

Clears the level 6 alreadyNarrowedType error on lib/data_query.php:2591 that
develop currently emits, matching the sibling narrowing identifiers already ignored.

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

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>

.phpstan-bootstrap.php

@@ -1,3 +1,8 @@
 <?php
 
 define('PHP_TESTING', true);
+
+// PHP_TESTING alone no longer arms the DB short-circuit in include/global.php;
+// the CACTI_TEST_BOOTSTRAP env var must also be set.
+putenv('CACTI_TEST_BOOTSTRAP=1');
+$_ENV['CACTI_TEST_BOOTSTRAP'] = '1';

include/global.php

@@ -123,7 +123,18 @@ if (file_exists(__DIR__ . '/config.php')) {
 	include(__DIR__ . '/config.php');
 }
 
-if (defined('PHP_TESTING') && file_exists(__DIR__ . '/config.php.dist')) {
+// Combined test-bootstrap gate: PHP_TESTING alone is not enough. The
+// CACTI_TEST_BOOTSTRAP env var (local_only so per-request SAPI params under
+// FPM/FastCGI cannot set it) must also be present. A stray PHP_TESTING define
+// in a deployed environment therefore cannot swap to the default config, skip
+// schema validation, or disable real DB connection logic. Fail-closed.
+if (!function_exists('cacti_is_test_bootstrap')) {
+	function cacti_is_test_bootstrap(): bool {
+		return defined('PHP_TESTING') && getenv('CACTI_TEST_BOOTSTRAP', true) === '1';
+	}
+}
+
+if (cacti_is_test_bootstrap() && file_exists(__DIR__ . '/config.php.dist')) {
 	if (!is_readable(__DIR__ . '/config.php.dist')) {
 		die('Configuration file include/config.php is present, but unreadable.' . PHP_EOL);
 	}
@@ -300,6 +311,37 @@ if ($config['is_web'] && ini_get('session.auto_start') == 1) {
 	exit;
 }
 
+// Test-mode DB sentinel: any method invocation on the sentinel throws,
+// so production-mode code paths can never silently treat a non-handle as
+// a real connection. Gated by the PHP_TESTING constant AND the
+// CACTI_TEST_BOOTSTRAP env var so a stale define alone cannot disable
+// real DB connection logic in a deployed environment.
+// Pass false to class_exists() so this guard never triggers autoload.
+if (!class_exists('Cacti_TestDbSentinel', false)) {
+	final class Cacti_TestDbSentinel {
+		/**
+		 * @param  string           $name
+		 * @param  array<int,mixed> $args
+		 * @return never
+		 */
+		public function __call($name, $args) {
+			throw new \RuntimeException('PHP_TESTING DB sentinel called: ' . $name);
+		}
+	}
+}
+
+// Helper for "is this a real DB handle" checks that must reject the sentinel.
+// Defined inline so include/global.php remains self-contained.
+if (!function_exists('_cacti_is_real_db_conn')) {
+	function _cacti_is_real_db_conn(mixed $x): bool {
+		return is_object($x) && !($x instanceof Cacti_TestDbSentinel);
+	}
+}
+
+// Resolve the test-bootstrap predicate once; both PHP_TESTING and
+// CACTI_TEST_BOOTSTRAP=1 must be set for the sentinel branches to engage.
+$is_test_bootstrap = cacti_is_test_bootstrap();
+
 // set poller mode
 global $local_db_cnn_id, $remote_db_cnn_id, $conn_mode;
 
@@ -313,8 +355,10 @@ $lu = $config['is_web'] ? '</ul>' : '';
 $il = $config['is_web'] ? '</li>' : '';
 
 if ($config['poller_id'] > 1 || isset($rdatabase_hostname)) {
-	if (!defined('PHP_TESTING')) {
+	if (!$is_test_bootstrap) {
 		$local_db_cnn_id = db_connect_real($database_hostname, $database_username, $database_password, $database_default, $database_type, $database_port, $database_retries, $database_ssl, $database_ssl_key, $database_ssl_cert, $database_ssl_ca, $database_ssl_capath, $database_ssl_verify_server_cert);
+	} else {
+		$local_db_cnn_id = new Cacti_TestDbSentinel();
 	}
 
 	if (!isset($rdatabase_retries)) {
@@ -346,7 +390,7 @@ if ($config['poller_id'] > 1 || isset($rdatabase_hostname)) {
 	}
 
 	// Check for recovery
-	if (is_object($local_db_cnn_id)) {
+	if (_cacti_is_real_db_conn($local_db_cnn_id)) {
 		$boost_records = db_fetch_cell('SELECT COUNT(*)
 			FROM poller_output_boost', '', true, $local_db_cnn_id);
 
@@ -355,20 +399,29 @@ if ($config['poller_id'] > 1 || isset($rdatabase_hostname)) {
 		}
 	}
 
-	// gather the existing cactidb version
+	// gather the existing cactidb version (skip when running under the test sentinel).
+	// Seed '' first so the key is always set, matching pre-sentinel behavior where a
+	// failed db_fetch_cell returned ''; a remote poller with the local DB down then
+	// reads a defined value instead of raising an 'Undefined array key' warning.
+	$config['cacti_db_version'] = '';
+
+	if (_cacti_is_real_db_conn($local_db_cnn_id)) {
 		$config['cacti_db_version'] = db_fetch_cell('SELECT cacti FROM version LIMIT 1', '', false, $local_db_cnn_id);
+	}
 
 	/**
 	 * If we have not been forced offline by the $conn_mode global and since we are
 	 * a remote poller, let's attempt to get back online.
 	 */
 	if ($conn_mode != 'offline') {
-		if (!defined('PHP_TESTING')) {
+		if (!$is_test_bootstrap) {
 			$remote_db_cnn_id = db_connect_real($rdatabase_hostname, $rdatabase_username, $rdatabase_password, $rdatabase_default, $rdatabase_type, $rdatabase_port, $database_retries, $rdatabase_ssl, $rdatabase_ssl_key, $rdatabase_ssl_cert, $rdatabase_ssl_ca, $rdatabase_ssl_capath, $rdatabase_ssl_verify_server_cert);
+		} else {
+			$remote_db_cnn_id = new Cacti_TestDbSentinel();
 		}
 	}
 
-	if ($config['is_web'] && is_object($remote_db_cnn_id) && $config['connection'] != 'recovery' && $config['cacti_db_version'] != 'new_install' && !defined('IN_CACTI_INSTALL')) {
+	if ($config['is_web'] && _cacti_is_real_db_conn($remote_db_cnn_id) && $config['connection'] != 'recovery' && $config['cacti_db_version'] != 'new_install' && !defined('IN_CACTI_INSTALL')) {
 		// Connection worked, so now override the default settings so that it will always utilize the remote connection
 		$database_default                = $rdatabase_default;
 		$database_hostname               = $rdatabase_hostname;
@@ -381,7 +434,7 @@ if ($config['poller_id'] > 1 || isset($rdatabase_hostname)) {
 		$database_ssl_ca                 = $rdatabase_ssl_ca;
 		$database_ssl_capath             = $rdatabase_ssl_capath;
 		$database_ssl_verify_server_cert = $rdatabase_ssl_verify_server_cert;
-	} elseif (is_object($remote_db_cnn_id)) {
+	} elseif (_cacti_is_real_db_conn($remote_db_cnn_id)) {
 		if ($config['connection'] != 'recovery') {
 			$config['connection'] = 'online';
 		}
@@ -389,7 +442,7 @@ if ($config['poller_id'] > 1 || isset($rdatabase_hostname)) {
 		$config['connection'] = 'offline';
 	}
 } else {
-	if (!defined('PHP_TESTING')) {
+	if (!$is_test_bootstrap) {
 		if (!db_connect_real($database_hostname, $database_username, $database_password, $database_default, $database_type, $database_port, $database_retries, $database_ssl, $database_ssl_key, $database_ssl_cert, $database_ssl_ca, $database_ssl_capath, $database_ssl_verify_server_cert)) {
 			print $ps . 'FATAL: Connection to Cacti database failed. Please ensure: ' . $ul;
 			print $li . 'the PHP MySQL module is installed and enabled.' . $il;
@@ -408,9 +461,11 @@ if ($config['poller_id'] > 1 || isset($rdatabase_hostname)) {
 
 			exit;
 		}
+	} else {
+		$local_db_cnn_id = new Cacti_TestDbSentinel();
 	}
 
-	if (!defined('PHP_TESTING')) {
+	if (!$is_test_bootstrap) {
 		if (!db_table_exists('settings') || !db_table_exists('version')) {
 			print $ps . 'FATAL: Connection to Cacti database succeeded but `settings` table not found. Please ensure: ' . $ul;
 			print $li . 'the PHP MySQL module is installed and enabled.' . $il;
@@ -432,8 +487,13 @@ if ($config['poller_id'] > 1 || isset($rdatabase_hostname)) {
 		}
 	}
 
-	// gather the existing cactidb version
+	// gather the existing cactidb version (skip when running under the test sentinel).
+	// Seed '' first so the key is always set even under the test bootstrap.
+	$config['cacti_db_version'] = '';
+
+	if (!$is_test_bootstrap) {
 		$config['cacti_db_version'] = db_fetch_cell('SELECT cacti FROM version LIMIT 1');
+	}
 }
 
 define('CACTI_CONNECTION', $config['connection']);

lib/database.php

@@ -161,8 +161,14 @@ function db_connect_real(string $device, string $user, string $pass, string $db_
 				'database_persist'                  => $persist,
 			];
 
-			// test if cacti database is imported from SQL file
-			if (!defined('PHP_TESTING')) {
+			// test if cacti database is imported from SQL file. Skip only under the
+			// combined test bootstrap (PHP_TESTING + CACTI_TEST_BOOTSTRAP=1, local_only);
+			// PHP_TESTING alone must not bypass schema validation in a deployed setup.
+			$skip_schema_check = function_exists('cacti_is_test_bootstrap')
+				? cacti_is_test_bootstrap()
+				: (defined('PHP_TESTING') && getenv('CACTI_TEST_BOOTSTRAP', true) === '1');
+
+			if (!$skip_schema_check) {
 				$table_exists = db_fetch_cell("SELECT count(*) FROM information_schema.tables
 					WHERE table_schema = DATABASE() and TABLE_NAME = 'version'");
 

phpstan-baseline.neon

@@ -107,3 +107,9 @@ parameters:
 			identifier: isset.offset
 			count: 1
 			path: lib/html.php
+
+		-
+			message: '#^Call to function is_numeric\(\) with float\|int\|numeric\-string will always evaluate to true\.$#'
+			identifier: function.alreadyNarrowedType
+			count: 1
+			path: lib/data_query.php

phpunit.xml

@@ -1,6 +1,7 @@
 <phpunit bootstrap="include/vendor/autoload.php" beStrictAboutOutputDuringTests="false">
     <php>
         <ini name="error_reporting" value="-1"/>
+        <env name="CACTI_TEST_BOOTSTRAP" value="1"/>
     </php>
     <testsuites>
         <testsuite name="Unit">

tests/Helpers/CactiStubs.php

@@ -20,3 +20,9 @@
  */
 
 define('PHP_TESTING', true);
+
+// Arm the combined test-bootstrap gate in include/global.php. PHP_TESTING alone
+// no longer engages the DB short-circuit; CACTI_TEST_BOOTSTRAP must also be set
+// so a stray define in production cannot bypass real connection logic.
+putenv('CACTI_TEST_BOOTSTRAP=1');
+$_ENV['CACTI_TEST_BOOTSTRAP'] = '1';

tests/Unit/TestDbSentinelGuardTest.php

@@ -0,0 +1,112 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ |                                                                         |
+ | This program is free software; you can redistribute it and/or           |
+ | modify it under the terms of the GNU General Public License             |
+ | as published by the Free Software Foundation; either version 2          |
+ | of the License, or (at your option) any later version.                  |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+/*
+ * Tests for the test-mode DB sentinel guard. The short-circuit that skips a
+ * real database connection must engage only when PHP_TESTING is defined AND
+ * the CACTI_TEST_BOOTSTRAP env var is set. PHP_TESTING alone must fail closed:
+ * it must not swap to config.php.dist nor skip schema validation.
+ */
+
+require_once dirname(__DIR__) . '/Helpers/CactiStubs.php';
+require_once dirname(__DIR__, 2) . '/include/global.php';
+
+// Pure replica of cacti_is_test_bootstrap() so the false branches can be
+// exercised without mutating the process-global PHP_TESTING constant.
+function test_predicate(bool $php_testing, ?string $env): bool {
+	return $php_testing && $env === '1';
+}
+
+test('bootstrap arms the gate: CactiStubs sets CACTI_TEST_BOOTSTRAP=1', function () {
+	expect(getenv('CACTI_TEST_BOOTSTRAP', true))->toBe('1');
+});
+
+test('cacti_is_test_bootstrap is true under the test bootstrap', function () {
+	expect(defined('PHP_TESTING'))->toBeTrue();
+	expect(cacti_is_test_bootstrap())->toBeTrue();
+});
+
+test('predicate true only when PHP_TESTING and env are both set', function () {
+	expect(test_predicate(true, '1'))->toBeTrue();
+});
+
+test('predicate false when PHP_TESTING is set but env is missing', function () {
+	expect(test_predicate(true, null))->toBeFalse();
+	expect(test_predicate(true, '0'))->toBeFalse();
+	expect(test_predicate(true, ''))->toBeFalse();
+});
+
+test('predicate false when neither PHP_TESTING nor env is set', function () {
+	expect(test_predicate(false, null))->toBeFalse();
+});
+
+test('predicate false when env is set but PHP_TESTING is not', function () {
+	expect(test_predicate(false, '1'))->toBeFalse();
+});
+
+test('short-circuit installs the DB sentinel rather than a real handle', function () {
+	global $local_db_cnn_id;
+
+	// global.php ran above under the test bootstrap, so no real connection
+	// was attempted and the sentinel stands in for the handle.
+	expect($local_db_cnn_id)->toBeInstanceOf(Cacti_TestDbSentinel::class);
+	expect(_cacti_is_real_db_conn($local_db_cnn_id))->toBeFalse();
+});
+
+test('sentinel throws on any method call so it cannot pass as a real handle', function () {
+	$sentinel = new Cacti_TestDbSentinel();
+
+	expect(fn () => $sentinel->query('SELECT 1'))
+		->toThrow(RuntimeException::class);
+});
+
+test('global.php always assigns cacti_db_version with a safe default', function () {
+	$src = file_get_contents(dirname(__DIR__, 2) . '/include/global.php');
+
+	// Bug 4: the key must be seeded so isset()/comparison logic never raises
+	// an Undefined array key warning when the local DB handle is the sentinel
+	// or the connection is down. Both connect branches must seed the default.
+	$matches = preg_match_all(
+		"/\\\$config\\['cacti_db_version'\\] = '';/",
+		$src
+	);
+
+	expect($matches)->toBe(2);
+});
+
+// Source-level guards. These assert the fail-closed wiring is present in the
+// shipped files, independent of the runtime path the test itself takes.
+
+test('global.php gates config.php.dist swap on the combined predicate', function () {
+	$src = file_get_contents(dirname(__DIR__, 2) . '/include/global.php');
+
+	// The config.php.dist include must be gated by the helper, not PHP_TESTING alone.
+	expect($src)->toContain('if (cacti_is_test_bootstrap() && file_exists(__DIR__ . \'/config.php.dist\'))');
+});
+
+test('global.php reads CACTI_TEST_BOOTSTRAP with local_only=true', function () {
+	$src = file_get_contents(dirname(__DIR__, 2) . '/include/global.php');
+
+	expect($src)->toContain("getenv('CACTI_TEST_BOOTSTRAP', true)");
+	// No call site may read the env var without the local_only flag.
+	expect($src)->not->toContain("getenv('CACTI_TEST_BOOTSTRAP')");
+});
+
+test('database.php gates schema validation on the combined predicate with local_only', function () {
+	$src = file_get_contents(dirname(__DIR__, 2) . '/lib/database.php');
+
+	expect($src)->toContain("getenv('CACTI_TEST_BOOTSTRAP', true)");
+	expect($src)->not->toContain("if (!defined('PHP_TESTING')) {\n\t\t\t\t$table_exists");
+	expect($src)->toContain('$skip_schema_check');
+});