From ac0d3d6784c6af550b87dc4a71a49dcef2e6b4f7 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Fri, 16 Oct 2026 16:03:33 -0400 Subject: [PATCH 1/2] Log NUT query failures and save the UPS Poller ID A failed NUT query only wrote to the UPS error_message column and -d output, so an unreachable upsd left nothing in the Cacti log; log a WARNING with the UPS, host, port and reason (never credentials). The UPS form displayed a Poller ID field that form_save() never stored; validate and save it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 ++ poller_apcupsd.php | 4 ++++ tests/Unit/NutWiringTest.php | 22 ++++++++++++++++++++++ upses.php | 5 +++++ 4 files changed, 33 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4299280..e9dbbc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ --- 1.4.0 --- +* issue: Write a WARNING to the Cacti log (APCUPSD) naming the UPS, host, port and reason whenever a NUT query fails; before, the reason was only stored in the UPS's error message and shown with `-d`, so a NUT UPS that could not be reached left nothing in the log +* issue: Save the Poller ID chosen on the UPS form; the field was displayed but never stored, so every UPS was always assigned to poller 1 * feature: Add support for monitoring UPSes through Network UPS Tools (NUT) as a third collector type, "NUT Based". The poller talks to `upsd` (TCP 3493 by default) over a native PHP socket implementation of the NUT protocol, so no `upsc` or other CLI tool is needed. Optional username/password authentication and STARTTLS (with certificate verification) are supported, and one NUT server can serve several UPSes through the new NUT UPS Name setting. NUT status flags are translated to the existing apcupsd vocabulary (OL to ONLINE, OB to ONBATT, and so on) so the dashboards, fleet totals and alerts keep working unchanged * issue: Widen the status, model, serial number, firmware, driver and name columns of `apcupsd_ups_stats` so multi-flag NUT statuses and longer NUT strings are not truncated * feature: Let NUT STARTTLS work with self-signed certificates: a per-UPS CA / Certificate File to verify the server against (certificate and host name are still checked), or an Allow Self-Signed Certificate option that encrypts without verifying the server diff --git a/poller_apcupsd.php b/poller_apcupsd.php index 217c428..cc8f465 100644 --- a/poller_apcupsd.php +++ b/poller_apcupsd.php @@ -622,6 +622,10 @@ function collect_nut_ups_data($ups): bool { if ($result['error'] !== '') { debug('NUT query failed: ' . $result['error']); + // The poller runs in the background with its output discarded, so the Cacti log is + // the only place an operator can see why a NUT UPS is down. + cacti_log(sprintf('WARNING: NUT query for UPS %s (%s:%s) failed: %s', $ups['name'], $ups['hostname'], $ups['port'], $result['error']), false, 'APCUPSD'); + // These devices have availability checking disabled, so core polling cannot // notice the failure: mark the linked host down here. set_ups_host_state($ups['host_id'], false); diff --git a/tests/Unit/NutWiringTest.php b/tests/Unit/NutWiringTest.php index 4c68795..54bcd73 100644 --- a/tests/Unit/NutWiringTest.php +++ b/tests/Unit/NutWiringTest.php @@ -168,6 +168,28 @@ expect(strpos($poller, "set_ups_host_state(\$ups['host_id'], false);"))->toBeLessThan(strpos($poller, "substr(\$result['error'], 0, 255)")); }); + it('logs a failed NUT query to the Cacti log without exposing credentials', function () use ($read) { + $poller = $read('poller_apcupsd.php'); + $log = "cacti_log(sprintf('WARNING: NUT query for UPS %s (%s:%s) failed: %s', \$ups['name'], \$ups['hostname'], \$ups['port'], \$result['error']), false, 'APCUPSD');"; + + expect($poller)->toContain($log); + + // The failure is logged before the status is written and the function returns. + expect(strpos($poller, $log))->toBeLessThan(strpos($poller, "substr(\$result['error'], 0, 255)")); + + // Neither the NUT username nor password may reach the log line. + expect($log)->not->toContain('nut_password'); + expect($log)->not->toContain('nut_username'); + }); + + it('stores the Poller ID chosen on the UPS form', function () use ($read) { + $page = $read('upses.php'); + + expect($page)->toContain("if (isset_request_var('poller_id')) {"); + expect($page)->toContain("\$save['poller_id'] = form_input_validate(get_nfilter_request_var('poller_id'), 'poller_id', '^[0-9]+\$', false, 3);"); + expect(strpos($page, "\$save['poller_id']"))->toBeLessThan(strpos($page, "sql_save(\$save, 'apcupsd_ups')")); + }); + it('does not rewrite an existing UPS\'s port when the edit form opens', function () use ($read) { $page = $read('upses.php'); diff --git a/upses.php b/upses.php index fd83ff0..ed63940 100644 --- a/upses.php +++ b/upses.php @@ -352,6 +352,11 @@ function form_save(): void { $save['description'] = form_input_validate(get_nfilter_request_var('description'), 'description', '', true, 3); $save['site_id'] = form_input_validate(get_nfilter_request_var('site_id'), 'site_id', '^[0-9]+$', true, 3); + // Without this the Poller ID field is shown but never stored, so every UPS falls back to the column default. + if (isset_request_var('poller_id')) { + $save['poller_id'] = form_input_validate(get_nfilter_request_var('poller_id'), 'poller_id', '^[0-9]+$', false, 3); + } + if ($save['type_id'] == 2) { $save['hostname'] = form_input_validate(get_nfilter_request_var('snmp_hostname'), 'snmp_hostname', '', true, 3); } else { From 857bf65f94105ee12b6fc023134b915af4aa7c9a Mon Sep 17 00:00:00 2001 From: TheWitness Date: Fri, 16 Oct 2026 16:39:15 -0400 Subject: [PATCH 2/2] Refuse Poller ID changes on a UPS linked to a device; slim the tests Changing a UPS's Poller ID updated apcupsd_ups but left the linked device on the old poller, so collection and the device assignment could disagree. Moving a device between pollers needs Cacti's own device handling, so reject the change with an explanation and have the user change the poller on the device. Compare against the UPS's stored value so a device already moved elsewhere does not block unrelated edits. Restructure the new tests as a compact dataset to avoid duplicating the neighbouring source-assertion tests (SonarCloud duplication gate), and add the new message to cacti.pot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- locales/po/cacti.pot | 6 +++++- tests/Unit/NutWiringTest.php | 31 +++++++++++++------------------ upses.php | 11 +++++++++++ 4 files changed, 30 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9dbbc6..38d8c61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ --- 1.4.0 --- * issue: Write a WARNING to the Cacti log (APCUPSD) naming the UPS, host, port and reason whenever a NUT query fails; before, the reason was only stored in the UPS's error message and shown with `-d`, so a NUT UPS that could not be reached left nothing in the log -* issue: Save the Poller ID chosen on the UPS form; the field was displayed but never stored, so every UPS was always assigned to poller 1 +* issue: Save the Poller ID chosen on the UPS form; the field was displayed but never stored, so every UPS was always assigned to poller 1. Changing the Poller ID of a UPS that is linked to a Cacti Device is refused with an explanation, since the device's poller has to be changed on the device itself * feature: Add support for monitoring UPSes through Network UPS Tools (NUT) as a third collector type, "NUT Based". The poller talks to `upsd` (TCP 3493 by default) over a native PHP socket implementation of the NUT protocol, so no `upsc` or other CLI tool is needed. Optional username/password authentication and STARTTLS (with certificate verification) are supported, and one NUT server can serve several UPSes through the new NUT UPS Name setting. NUT status flags are translated to the existing apcupsd vocabulary (OL to ONLINE, OB to ONBATT, and so on) so the dashboards, fleet totals and alerts keep working unchanged * issue: Widen the status, model, serial number, firmware, driver and name columns of `apcupsd_ups_stats` so multi-flag NUT statuses and longer NUT strings are not truncated * feature: Let NUT STARTTLS work with self-signed certificates: a per-UPS CA / Certificate File to verify the server against (certificate and host name are still checked), or an Allow Self-Signed Certificate option that encrypts without verifying the server diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index f87c895..d08274c 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: Cacti 1.3.1\n" "Report-Msgid-Bugs-To: developers@cacti.net\n" -"POT-Creation-Date: 2026-10-14 16:33-0400\n" +"POT-Creation-Date: 2026-10-16 16:37-0400\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: Cacti Developers \n" "Language-Team: Cacti Developers \n" @@ -750,6 +750,10 @@ msgstr "" msgid "The hostname where the snmp agent is located." msgstr "" +#: upses.php +msgid "The Poller ID cannot be changed while the UPS is linked to a Cacti Device. Change the poller on the device instead." +msgstr "" + #: upses.php msgid "Template UPS was not found! Unable to duplicate." msgstr "" diff --git a/tests/Unit/NutWiringTest.php b/tests/Unit/NutWiringTest.php index 54bcd73..7b3d957 100644 --- a/tests/Unit/NutWiringTest.php +++ b/tests/Unit/NutWiringTest.php @@ -168,26 +168,21 @@ expect(strpos($poller, "set_ups_host_state(\$ups['host_id'], false);"))->toBeLessThan(strpos($poller, "substr(\$result['error'], 0, 255)")); }); - it('logs a failed NUT query to the Cacti log without exposing credentials', function () use ($read) { + it('keeps the NUT failure log line and the Poller ID save in place', function (string $file, string $needle) use ($read) { + expect($read($file))->toContain($needle); + })->with([ + 'failure logged' => ['poller_apcupsd.php', "cacti_log(sprintf('WARNING: NUT query for UPS %s (%s:%s) failed: %s', \$ups['name'], \$ups['hostname'], \$ups['port'], \$result['error']), false, 'APCUPSD');"], + 'poller id saved' => ['upses.php', "if (isset_request_var('poller_id')) {"], + 'poller id digits only' => ['upses.php', "get_nfilter_request_var('poller_id'), 'poller_id', '^[0-9]+\$', false, 3);"], + 'linked poller move held' => ['upses.php', "form_input_validate('', 'poller_id', '^\$', false, 3);"], + ]); + + it('logs the NUT failure before recording it and never logs credentials', function () use ($read) { $poller = $read('poller_apcupsd.php'); - $log = "cacti_log(sprintf('WARNING: NUT query for UPS %s (%s:%s) failed: %s', \$ups['name'], \$ups['hostname'], \$ups['port'], \$result['error']), false, 'APCUPSD');"; - expect($poller)->toContain($log); - - // The failure is logged before the status is written and the function returns. - expect(strpos($poller, $log))->toBeLessThan(strpos($poller, "substr(\$result['error'], 0, 255)")); - - // Neither the NUT username nor password may reach the log line. - expect($log)->not->toContain('nut_password'); - expect($log)->not->toContain('nut_username'); - }); - - it('stores the Poller ID chosen on the UPS form', function () use ($read) { - $page = $read('upses.php'); - - expect($page)->toContain("if (isset_request_var('poller_id')) {"); - expect($page)->toContain("\$save['poller_id'] = form_input_validate(get_nfilter_request_var('poller_id'), 'poller_id', '^[0-9]+\$', false, 3);"); - expect(strpos($page, "\$save['poller_id']"))->toBeLessThan(strpos($page, "sql_save(\$save, 'apcupsd_ups')")); + expect(preg_match("/WARNING: NUT query.*?substr\\(\\\$result\\['error'\\], 0, 255\\)/s", $poller))->toBe(1); + expect(preg_match('/WARNING: NUT query[^\n]*nut_(password|username)/', $poller))->toBe(0); + expect(preg_match("/\\\$current_poller != \\\$save\\['poller_id'\\].*?sql_save\\(\\\$save, 'apcupsd_ups'\\)/s", $read('upses.php')))->toBe(1); }); it('does not rewrite an existing UPS\'s port when the edit form opens', function () use ($read) { diff --git a/upses.php b/upses.php index ed63940..5d1e9d3 100644 --- a/upses.php +++ b/upses.php @@ -420,6 +420,17 @@ function form_save(): void { $host = []; } + // Moving a device to another poller needs Cacti's own device handling (poller cache and remote + // replication), which this form does not do, so a linked UPS keeps the poller its device is on. + if (cacti_sizeof($host) && !empty($save['id']) && isset($save['poller_id'])) { + $current_poller = (int) db_fetch_cell_prepared('SELECT poller_id FROM apcupsd_ups WHERE id = ?', [$save['id']]); + + if ($current_poller > 0 && $current_poller != $save['poller_id']) { + form_input_validate('', 'poller_id', '^$', false, 3); + raise_message('apcupsd_poller_linked', __('The Poller ID cannot be changed while the UPS is linked to a Cacti Device. Change the poller on the device instead.', 'apcupsd'), MESSAGE_LEVEL_ERROR); + } + } + if (!is_error_message()) { $ups_id = sql_save($save, 'apcupsd_ups');