From 4b2dd6d08d9ce1704f895bfdfbc8560ca908aec6 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Thu, 15 Oct 2026 10:43:45 -0400 Subject: [PATCH 1/2] Guard maint maintenance-window checks against a missing maint functions file #858 corrected the maint functions include path but left three plugin_maint_check_cacti_host() calls guarded only by api_plugin_is_enabled('maint'). When maint is enabled but its functions file cannot be included (the include block now logs and continues rather than fataling), those calls hit an undefined function and fatal the poll. Add a function_exists('plugin_maint_check_cacti_host') guard to all three call sites (thold_check_threshold() and both the recovering- and down-device loops in thold_update_host_status()) so a missing maint include degrades gracefully, matching the guard other plugins use. Cover both maintenance-gated notification branches with a new MaintCheckGuardTest; add an html_split_string() test stub so the recovering-device path can be driven in the isolated unit process. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- includes/functions.php | 2 +- includes/polling.php | 4 +- tests/Unit/MaintCheckGuardTest.php | 122 +++++++++++++++++++++++++++++ tests/bootstrap-unit.php | 6 ++ 4 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 tests/Unit/MaintCheckGuardTest.php diff --git a/includes/functions.php b/includes/functions.php index b04a5f30..1504bac3 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -3423,7 +3423,7 @@ function thold_check_threshold(&$thold_data) { } } - if (api_plugin_is_enabled('maint') && plugin_maint_check_cacti_host($thold_data['host_id'])) { + if (api_plugin_is_enabled('maint') && function_exists('plugin_maint_check_cacti_host') && plugin_maint_check_cacti_host($thold_data['host_id'])) { $maint_dev = true; } else { $maint_dev = false; diff --git a/includes/polling.php b/includes/polling.php index 51b68988..202ed2fd 100644 --- a/includes/polling.php +++ b/includes/polling.php @@ -637,7 +637,7 @@ function thold_update_host_status() { $msg = thold_str_replace('', $host['notes'], $msg); $msg = thold_str_replace("\n", '
', $msg); - if (api_plugin_is_enabled('maint') && plugin_maint_check_cacti_host($host['id'])) { + if (api_plugin_is_enabled('maint') && function_exists('plugin_maint_check_cacti_host') && plugin_maint_check_cacti_host($host['id'])) { cacti_log('WARNING: Device[' . $host['id'] . '] Hostname[' . $host['hostname'] . '] is recovering! Only logging, maint device', true, 'THOLD'); } else { switch ($host['thold_send_email']) { @@ -764,7 +764,7 @@ function thold_update_host_status() { $alert_email = read_config_option('alert_email'); - if (api_plugin_is_enabled('maint') && plugin_maint_check_cacti_host($host['id'])) { + if (api_plugin_is_enabled('maint') && function_exists('plugin_maint_check_cacti_host') && plugin_maint_check_cacti_host($host['id'])) { cacti_log('WARNING: Device[' . $host['id'] . '] Hostname[' . $host['hostname'] . '] is DOWN. Only logging, maint device', true, 'THOLD'); } else { if (strtotime($host['status_fail_date']) > 192800 || strtotime($host['status_rec_date']) > 192800) { diff --git a/tests/Unit/MaintCheckGuardTest.php b/tests/Unit/MaintCheckGuardTest.php new file mode 100644 index 00000000..9636a4ee --- /dev/null +++ b/tests/Unit/MaintCheckGuardTest.php @@ -0,0 +1,122 @@ +maintIncludesRoot(); + + set_error_handler(static fn (): bool => true); + ob_start(); + + try { + thold_update_host_status(); + } catch (\Throwable $e) { + // Reaching the maintenance-gated branch is the point. + } finally { + ob_end_clean(); + restore_error_handler(); + } + } + + /** + * A recovered device inside a maintenance window is logged, not alerted. + * + * @return void + */ + public function testRecoveringDeviceInMaintenanceIsLoggedNotAlerted(): void { + CactiStubs::willReturnFor('db_fetch_assoc', 'plugin_thold_host_failed', [['host_id' => 42]]); + CactiStubs::willReturnFor('db_fetch_row_prepared', 'FROM host WHERE id', [ + 'id' => 42, + 'hostname' => 'dev42', + 'description' => 'Device 42', + 'status' => HOST_UP, + 'snmp_community' => '', + 'snmp_username' => '', + 'snmp_version' => 0, + 'site_id' => 0, + 'location' => '', + 'notes' => '', + 'status_fail_date' => '0000-00-00 00:00:00', + 'availability' => 100, + 'total_polls' => 1, + 'failed_polls' => 0, + 'cur_time' => 0, + 'avg_time' => 0, + 'thold_send_email' => '0', + ]); + + $this->runHostStatus(); + + $this->assertContains( + 'WARNING: Device[42] Hostname[dev42] is recovering! Only logging, maint device', + CactiStubs::$log + ); + } + + /** + * A down device inside a maintenance window is logged, not alerted. + * + * @return void + */ + public function testDownDeviceInMaintenanceIsLoggedNotAlerted(): void { + CactiStubs::willReturnFor('db_fetch_assoc_prepared', 'availability_method', [[ + 'id' => 77, + 'hostname' => 'dev77', + ]]); + + $this->runHostStatus(); + + $this->assertContains( + 'WARNING: Device[77] Hostname[dev77] is DOWN. Only logging, maint device', + CactiStubs::$log + ); + } +} diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index ae80db4c..bfffd08b 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -669,6 +669,12 @@ function number_format_i18n($number, $decimals = 0, $baseu = 1000) { } } +if (!function_exists('html_split_string')) { + function html_split_string($string, $length = 76, $forgiving = 10) { + return $string; + } +} + if (!defined('FILTER_VALIDATE_IS_REGEX')) { define('FILTER_VALIDATE_IS_REGEX', 99999); } From 712dfec92da02cf18a23c22be2a4d5fb152577f4 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Thu, 15 Oct 2026 13:22:22 -0400 Subject: [PATCH 2/2] Let unexpected errors fail the maint-guard tests Drop the broad catch(Throwable) in the host-status test helper. A device inside a maintenance window is logged and the loop continues without the downstream notification work, so the call returns on its own; removing the catch means an unexpected error (such as a removed guard fataling on an undefined maint function) now fails the test instead of being swallowed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/Unit/MaintCheckGuardTest.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/Unit/MaintCheckGuardTest.php b/tests/Unit/MaintCheckGuardTest.php index 9636a4ee..22b2531a 100644 --- a/tests/Unit/MaintCheckGuardTest.php +++ b/tests/Unit/MaintCheckGuardTest.php @@ -41,9 +41,12 @@ private function maintIncludesRoot() { } /** - * Run thold_update_host_status() against the newer maint layout, swallowing - * engine output and any throw past the maintenance-gated branch, which is - * all these tests need to reach. + * Run thold_update_host_status() against the newer maint layout. A device + * inside a maintenance window is logged and the loop moves on without the + * downstream notification work, so the call returns on its own. Expected + * engine warnings are suppressed, but no Throwable is swallowed, so an + * unexpected error (e.g. a removed guard fataling on an undefined maint + * function) fails the test. * * @return void */ @@ -58,8 +61,6 @@ private function runHostStatus() { try { thold_update_host_status(); - } catch (\Throwable $e) { - // Reaching the maintenance-gated branch is the point. } finally { ob_end_clean(); restore_error_handler();