From 86bc3ede7490f00e9cb1980c2b950467192555a3 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Thu, 8 Oct 2026 14:55:43 -0400 Subject: [PATCH 1/3] Feature-detect Cacti 1.3 instead of CACTI_VERSION in audit_utilities_array audit_utilities_array() gated its Utilities-page entry on version_compare(CACTI_VERSION,'1.3.0','<'). A develop build can report < 1.3.0 while surfacing the Audit Log link via config_arrays, so the legacy block also ran: the entry was doubled and array_merge() could run against an unset Technical Support key. Detect 1.3 via class_exists('CactiTableFilter') instead and leave the translatable strings unchanged. --- CHANGELOG.md | 1 + setup.php | 6 +++++- tests/Unit/AuditUtilitiesArrayTest.php | 24 ++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 tests/Unit/AuditUtilitiesArrayTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index c3c3172..9c9695b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ --- develop --- * bug: Pass a boolean (not the integer 1) to api_plugin_register_hook()'s $enable argument so the hook registration stays type-correct under Cacti 1.3's strict bool type-hint (the int only coerces while the file lacks declare(strict_types=1)) +* bug: Replace the audit_utilities_array() CACTI_VERSION gate with a 1.3 feature-detect (CactiTableFilter), so a develop build that reports a version comparing < 1.3.0 no longer adds the Audit Log Utilities-page entry a second time (nor runs array_merge() against an unset Technical Support section) when 1.3 already lists it via config_arrays * bug: Skip audit logging on CLI/poller requests until the `request_status` column exists, instead of emitting "Unknown column 'request_status'" errors when the audit_log schema migration (which only runs on plugins.php/audit.php page loads) has not yet applied; likewise make the shutdown-time request finalizer (`audit_finalize_request()`), which writes `request_status`, best-effort (its writes pass `$log = false`) so it fails quietly instead of logging "Table 'audit_log' doesn't exist" when the table was dropped (e.g. plugin uninstall) after `db_table_exists()`'s request-scoped cache was primed * dev: Enforce patch coverage of changed lines in CI and remove the inert COMPOSER_ROOT_VERSION env from the Pest step * dev: Relocate the schema/DB provisioning helpers (`audit_setup_table()`, `audit_setup_syslog_table()`, `audit_setup_user_log_state_table()`, `audit_upgrade_event_schema()`, and the user_log index helpers) from `setup.php` into `includes/database.php` to align with the fleet schema/includes convention, and add a root `manifest.json` with upgrade-time file pruning (`audit_prune_files()`) plus a CI manifest drift check (`tests/bin/validate-manifest.php`) diff --git a/setup.php b/setup.php index 334676d..18c2776 100644 --- a/setup.php +++ b/setup.php @@ -633,7 +633,11 @@ function audit_log_valid_event(): bool { function audit_utilities_array(): void { global $utilities; - if (version_compare(CACTI_VERSION, '1.3.0', '<')) { + // Cacti 1.3+ adds the Audit Log link through config_arrays instead of this + // legacy Utilities hook. Detect 1.3 via a class it introduced rather than a + // CACTI_VERSION string (a develop build can report < 1.3.0) so the entry is + // not added a second time. + if (!class_exists('CactiTableFilter')) { if (api_plugin_user_realm_auth('audit.php')) { $utilities[__('Technical Support', 'audit')] = array_merge( $utilities[__('Technical Support', 'audit')], diff --git a/tests/Unit/AuditUtilitiesArrayTest.php b/tests/Unit/AuditUtilitiesArrayTest.php new file mode 100644 index 0000000..58bb5c7 --- /dev/null +++ b/tests/Unit/AuditUtilitiesArrayTest.php @@ -0,0 +1,24 @@ + []]; + + audit_utilities_array(); + + expect($GLOBALS['utilities'])->toHaveKey($section); +}); From 50627e3083da24885ee2e536592d335bf87a26f8 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Thu, 8 Oct 2026 15:35:41 -0400 Subject: [PATCH 2/3] Harden audit_utilities_array() against a missing section and cover its branches Guard the legacy array_merge() with a null-coalesce so the Utilities hook never runs against an unset Technical Support section (missing-key protection), and replace the single placeholder test with focused coverage for the absent-section, populated-section-with-access, and access-denied paths. --- setup.php | 6 ++- tests/Unit/AuditUtilitiesArrayTest.php | 55 ++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/setup.php b/setup.php index 18c2776..38d163c 100644 --- a/setup.php +++ b/setup.php @@ -639,8 +639,10 @@ function audit_utilities_array(): void { // not added a second time. if (!class_exists('CactiTableFilter')) { if (api_plugin_user_realm_auth('audit.php')) { - $utilities[__('Technical Support', 'audit')] = array_merge( - $utilities[__('Technical Support', 'audit')], + $section = __('Technical Support', 'audit'); + + $utilities[$section] = array_merge( + $utilities[$section] ?? [], [ __('View Audit Log', 'audit') => [ 'link' => 'plugins/audit/audit.php', diff --git a/tests/Unit/AuditUtilitiesArrayTest.php b/tests/Unit/AuditUtilitiesArrayTest.php index 58bb5c7..1a82f36 100644 --- a/tests/Unit/AuditUtilitiesArrayTest.php +++ b/tests/Unit/AuditUtilitiesArrayTest.php @@ -7,18 +7,67 @@ /* * Unit coverage for audit_utilities_array() in setup.php. + * + * The CI matrix runs against a Cacti build without the 1.3-only + * CactiTableFilter class, so these tests exercise the legacy Utilities + * hook body: the section-preserving merge and the realm-authorization + * gate. Realm access is driven through the same api_plugin_user_realm_auth() + * stub used by the security suite (controllable via $__audit_test_realms); + * the guard below keeps this file runnable on its own. */ beforeAll(function () { require_once dirname(__DIR__, 2) . '/setup.php'; }); -it('runs the legacy Utilities hook and preserves the Technical Support section', function () { - $section = __('Technical Support', 'audit'); +if (!function_exists('api_plugin_user_realm_auth')) { + function api_plugin_user_realm_auth(string $filename = ''): bool { + return !empty($GLOBALS['__audit_test_realms'][$filename]); + } +} + +beforeEach(function () { + $GLOBALS['__audit_test_realms'] = []; + $GLOBALS['utilities'] = []; +}); - $GLOBALS['utilities'] = [$section => []]; +it('adds the Audit Log entry when the Technical Support section is absent', function () { + $GLOBALS['__audit_test_realms']['audit.php'] = true; audit_utilities_array(); + $section = __('Technical Support', 'audit'); + expect($GLOBALS['utilities'])->toHaveKey($section); + expect($GLOBALS['utilities'][$section])->toHaveKey(__('View Audit Log', 'audit')); + expect($GLOBALS['utilities'][$section][__('View Audit Log', 'audit')]['link']) + ->toBe('plugins/audit/audit.php'); +}); + +it('preserves existing Technical Support entries when access is granted', function () { + $GLOBALS['__audit_test_realms']['audit.php'] = true; + + $section = __('Technical Support', 'audit'); + $existing = __('Existing Entry', 'audit'); + + $GLOBALS['utilities'][$section] = [ + $existing => ['link' => 'plugins/other/other.php'], + ]; + + audit_utilities_array(); + + expect($GLOBALS['utilities'][$section])->toHaveKey($existing); + expect($GLOBALS['utilities'][$section][$existing]['link'])->toBe('plugins/other/other.php'); + expect($GLOBALS['utilities'][$section])->toHaveKey(__('View Audit Log', 'audit')); +}); + +it('adds nothing when the user lacks audit.php realm access', function () { + $section = __('Technical Support', 'audit'); + + $GLOBALS['utilities'][$section] = []; + + audit_utilities_array(); + + expect($GLOBALS['utilities'][$section])->toBe([]); + expect($GLOBALS['utilities'][$section])->not->toHaveKey(__('View Audit Log', 'audit')); }); From e1b933a6ec19c63d11e05fa3058d0ed9fcd303cc Mon Sep 17 00:00:00 2001 From: TheWitness Date: Thu, 8 Oct 2026 15:40:58 -0400 Subject: [PATCH 3/3] Keep both Technical Support __() occurrences so the i18n multiset matches develop The CI i18n check compares every __() call occurrence, not just unique strings. Extracting the section into a single \ variable dropped __('Technical Support', 'audit') from two calls to one, tripping the pot-sync gate even though the unique-string set is unchanged. Apply the missing-key null-coalesce in place instead. --- setup.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/setup.php b/setup.php index 38d163c..4be257d 100644 --- a/setup.php +++ b/setup.php @@ -639,10 +639,8 @@ function audit_utilities_array(): void { // not added a second time. if (!class_exists('CactiTableFilter')) { if (api_plugin_user_realm_auth('audit.php')) { - $section = __('Technical Support', 'audit'); - - $utilities[$section] = array_merge( - $utilities[$section] ?? [], + $utilities[__('Technical Support', 'audit')] = array_merge( + $utilities[__('Technical Support', 'audit')] ?? [], [ __('View Audit Log', 'audit') => [ 'link' => 'plugins/audit/audit.php',