From 938b92d76a8582f9278cf87b6efba588280f35f3 Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 30 Sep 2026 10:47:02 -0400 Subject: [PATCH 1/8] Back off compat to 1.2.29 and re-introduce dropped unique keys - INFO: lower the minimum Cacti compat from 1.2.32 to 1.2.29. - Add webseer_reintroduce_unique_keys(), called after db_update_table() in webseer_upgrade_tables(). Cacti 1.2.29-1.2.31 shipped a db_update_table() that rebuilds indexes only from 'keys' and silently drops UNIQUE keys declared via 'unique_keys' (here plugin_webseer_contacts.user_id_type). On those releases only, re-add the key when it is missing; 1.2.32+ preserves it, so the helper is a no-op there. - Test the in-range re-add, the out-of-range no-ops, and the already-exists guard; make the db_index_exists bootstrap stub mockable and add a cacti_version_compare stub. --- INFO | 2 +- includes/database.php | 32 ++++++++++++ tests/Unit/ReintroduceUniqueKeysTest.php | 65 ++++++++++++++++++++++++ tests/bootstrap-unit.php | 8 ++- 4 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 tests/Unit/ReintroduceUniqueKeysTest.php diff --git a/INFO b/INFO index b21476f..5329dad 100644 --- a/INFO +++ b/INFO @@ -26,5 +26,5 @@ longname = Service Monitor author = The Cacti Group email = jimmy@sqmail.org homepage = http://cactiusers.org -compat = 1.2.32 +compat = 1.2.29 capabilities = online_view:1, online_mgmt:1, offline_view:0, offline_mgmt:0, remote_collect:1 diff --git a/includes/database.php b/includes/database.php index 58fbdaa..510fc34 100644 --- a/includes/database.php +++ b/includes/database.php @@ -296,6 +296,38 @@ function webseer_upgrade_tables() { api_plugin_db_table_create('webseer', $table, $data); } } + + webseer_reintroduce_unique_keys(); +} + +/** + * Cacti 1.2.29 through 1.2.31 shipped a db_update_table() that silently + * dropped UNIQUE keys declared via a table definition's 'unique_keys' (it + * rebuilds indexes only from 'keys'). Re-add this plugin's unique keys when + * running on one of those releases; 1.2.32+ preserves them, so this is a + * no-op there. + * + * @return void + */ +function webseer_reintroduce_unique_keys(): void { + $running = trim((string) db_fetch_cell('SELECT cacti FROM version LIMIT 1')); + + if ($running === '' || + !cacti_version_compare($running, '1.2.29', '>=') || + !cacti_version_compare($running, '1.2.32', '<')) { + return; + } + + $unique_keys = [ + ['table' => 'plugin_webseer_contacts', 'name' => 'user_id_type', 'columns' => ['user_id', 'type']], + ]; + + foreach ($unique_keys as $uk) { + if (!db_index_exists($uk['table'], $uk['name'])) { + db_execute('ALTER TABLE `' . $uk['table'] . '` ADD UNIQUE KEY `' . + $uk['name'] . '` (`' . implode('`,`', $uk['columns']) . '`)'); + } + } } /** diff --git a/tests/Unit/ReintroduceUniqueKeysTest.php b/tests/Unit/ReintroduceUniqueKeysTest.php new file mode 100644 index 0000000..471d9bf --- /dev/null +++ b/tests/Unit/ReintroduceUniqueKeysTest.php @@ -0,0 +1,65 @@ +toHaveCount(1); + expect($added[0]['sql'])->toContain('plugin_webseer_contacts') + ->and($added[0]['sql'])->toContain('user_id_type') + ->and($added[0]['sql'])->toContain('`user_id`,`type`'); +}); + +it('does nothing on Cacti 1.2.32 and later', function () { + webseer_test_mock_db('db_fetch_cell', 'SELECT cacti FROM version', '1.2.32'); + + webseer_reintroduce_unique_keys(); + + expect(webseer_uk_added_indexes())->toHaveCount(0); +}); + +it('does nothing on Cacti releases before 1.2.29', function () { + webseer_test_mock_db('db_fetch_cell', 'SELECT cacti FROM version', '1.2.28'); + + webseer_reintroduce_unique_keys(); + + expect(webseer_uk_added_indexes())->toHaveCount(0); +}); + +it('does not re-add a unique key that already exists', function () { + webseer_test_mock_db('db_fetch_cell', 'SELECT cacti FROM version', '1.2.30'); + webseer_test_mock_db('db_index_exists', 'plugin_webseer_contacts', true); + + webseer_reintroduce_unique_keys(); + + expect(webseer_uk_added_indexes())->toHaveCount(0); +}); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index d1874bf..97bc3f8 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -195,7 +195,13 @@ function db_fetch_cell_prepared($sql, $params = []) { if (!function_exists('db_index_exists')) { function db_index_exists($table, $index) { - return false; + return webseer_test_db_result('db_index_exists', (string) $table . '.' . (string) $index, [], false); + } +} + +if (!function_exists('cacti_version_compare')) { + function cacti_version_compare($version1, $version2, $operator = '>') { + return version_compare($version1, $version2, $operator); } } From 713b5b77f54dbd2a58f94d4d452ae398c0300260 Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 30 Sep 2026 16:07:56 -0400 Subject: [PATCH 2/8] Add manifest.json + upgrade-time file pruning --- .github/copilot-instructions.md | 4 + .github/workflows/plugin-ci-workflow.yml | 3 + README.md | 10 ++ manifest.json | 25 ++++ setup.php | 158 ++++++++++++++++++++ tests/Integration/PluginLifecycleTest.php | 22 +++ tests/Unit/PruneFilesTest.php | 173 ++++++++++++++++++++++ tests/Unit/SetupHelpersGapsTest.php | 19 +++ tests/bin/validate-manifest.php | 73 +++++++++ tests/bootstrap-unit.php | 1 + 10 files changed, 488 insertions(+) create mode 100644 manifest.json create mode 100644 tests/Unit/PruneFilesTest.php create mode 100644 tests/bin/validate-manifest.php diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 0c545e5..a686a06 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -219,3 +219,7 @@ existing code or adding new code, not just in dedicated cleanup passes: line, `@param` lines, a blank comment line, then `@return`. Infer parameter/return types from actual usage; don't change the function's real type-hints in the same pass (let static analysis flag mismatches separately). Skip vendored third-party library files. + +## File manifest & upgrade pruning + +The plugin ships a root `manifest.json` with three arrays: `tombstones` (files/directories older versions shipped that have since moved or been removed), `expected` (the top-level files and directories that ship today, directories written with a trailing `/`), and `whitelist` (paths holding user data that must never be touched). Keep `expected` current: CI runs `tests/bin/validate-manifest.php`, which fails on any drift between `expected` and the real top-level tree (it ignores `tests/`, `.git*`, and whitelisted paths). Custom customer CSS/theme files belong in `expected`, and stylesheets live in `css/` (not `themes/`). On upgrade, `plugin_webseer_prune_files()` deletes the tombstoned paths and the dev-only `tests/` tree, leaves `whitelist` and `.git*` alone, and logs (without removing) any top-level entry the manifest does not account for. As a safety measure it refuses any tombstone that resolves outside the plugin directory (a tampered manifest.json) and logs a warning for any file or directory it cannot remove. When you move or delete a shipped file, add its old path to `tombstones` and update `expected` in the same change. diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index ab0d9698..430e0f1 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -89,6 +89,9 @@ jobs: - name: Check PHP version run: php -v + - name: Validate plugin manifest (expected-file drift) + run: php cacti/plugins/webseer/tests/bin/validate-manifest.php + - name: Run apt-get update run: sudo apt-get update diff --git a/README.md b/README.md index 2da0095..e0b90c3 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,16 @@ this, goto Cacti's Plugin Management page, Install and Enable the webseer. Once this is complete, you can grant users permission to create service checks for various Web Sites and Services. +## Upgrading + +`poller_webseer.php` runs as a long-lived background process, so a running copy +keeps the old code in memory across a plugin upgrade. After upgrading the plugin +files, restart the webseer background process (stop the running +`poller_webseer.php`; Cacti's poller relaunches it on the next cycle) so it +picks up the new code. On upgrade the plugin also prunes its own bundled +development-only files (for example the `tests/` directory) from the installed +tree. + ## Bugs and Feature Enhancements Bug and feature enhancements for the webseer plugin are handled in GitHub. If diff --git a/manifest.json b/manifest.json new file mode 100644 index 0000000..0dd3251 --- /dev/null +++ b/manifest.json @@ -0,0 +1,25 @@ +{ + "tombstones": [], + "expected": [ + "CHANGELOG.md", + "INFO", + "LICENSE", + "README.md", + "ca-bundle.crt", + "classes/", + "images/", + "includes/", + "index.php", + "locales/", + "manifest.json", + "phpunit.xml", + "poller_webseer.php", + "remote.php", + "setup.php", + "webseer.php", + "webseer_process.php", + "webseer_proxies.php", + "webseer_servers.php" + ], + "whitelist": [] +} diff --git a/setup.php b/setup.php index 8bc6044..7d7314d 100644 --- a/setup.php +++ b/setup.php @@ -154,6 +154,8 @@ function plugin_webseer_upgrade() { ['webseer.php,webseer_servers.php,webseer_proxies.php']); api_plugin_register_hook('webseer', 'replicate_out', 'webseer_replicate_out', 'setup.php', '1'); + + plugin_webseer_prune_files(); } return true; @@ -350,3 +352,159 @@ function webseer_replicate_out($data) { return $data; } + +/** + * Removes files and directories that a previous version of this plugin + * shipped but that have since moved or been deleted, using the tombstone + * and whitelist lists in manifest.json. Whitelisted (user-data) paths and + * any VCS metadata (.git*) are never touched; the dev-only tests/ tree is + * removed. Any path that resolves outside the plugin directory (a tampered + * manifest.json) is refused, and any file/directory that cannot be removed + * (e.g. read-only) is reported to the Cacti log. Any top-level entry that is + * neither expected nor a tombstone nor whitelisted is logged to the Cacti + * log and left in place. Called on a plugin version change. + * + * @return void + * + * @global array $config Cacti global configuration array; used to resolve + * the plugin directory. + */ +function plugin_webseer_prune_files(): void { + global $config; + + $plugin_dir = $config['base_path'] . '/plugins/webseer'; + $manifest_path = $plugin_dir . '/manifest.json'; + + if (!is_readable($manifest_path)) { + return; + } + + $manifest = json_decode((string) file_get_contents($manifest_path), true); + + if (!is_array($manifest)) { + cacti_log('WARNING: webseer manifest.json could not be parsed; skipping file prune', false, 'WEBSEER'); + + return; + } + + $tombstones = isset($manifest['tombstones']) && is_array($manifest['tombstones']) ? $manifest['tombstones'] : []; + $expected = isset($manifest['expected']) && is_array($manifest['expected']) ? $manifest['expected'] : []; + $whitelist = isset($manifest['whitelist']) && is_array($manifest['whitelist']) ? $manifest['whitelist'] : []; + + $protected = function (string $rel) use ($whitelist): bool { + if (strncmp($rel, '.git', 4) === 0) { + return true; + } + + foreach ($whitelist as $entry) { + $entry = trim((string) $entry, '/'); + + if ($entry !== '' && ($rel === $entry || strncmp($rel, $entry . '/', strlen($entry) + 1) === 0)) { + return true; + } + } + + return false; + }; + + // Security: resolve the plugin directory so a tampered manifest.json + // cannot steer the prune outside of it. + $plugin_real = realpath($plugin_dir); + + // Remove tombstoned (moved/deleted) paths plus the dev-only tests/ tree. + $remove = $tombstones; + $remove[] = 'tests/'; + + foreach ($remove as $rel) { + $rel = trim((string) $rel, '/'); + + if ($rel === '' || $protected($rel)) { + continue; + } + + $path = $plugin_dir . '/' . $rel; + + if (!is_link($path) && !file_exists($path)) { + continue; + } + + // Refuse any path that, after resolving symlinks and ../ segments, + // escapes the plugin directory (protects user data from a tampered + // manifest.json). + $anchor = is_link($path) ? dirname($path) : $path; + $real = realpath($anchor); + + if ($real === false || ($real !== $plugin_real && strncmp($real, $plugin_real . DIRECTORY_SEPARATOR, strlen((string) $plugin_real) + 1) !== 0)) { + cacti_log(sprintf('WARNING: webseer prune refused to remove %s: path resolves outside the plugin directory (tampered manifest.json?)', $rel), false, 'WEBSEER'); + + continue; + } + + if (is_dir($path) && !is_link($path)) { + $removed = plugin_webseer_rmtree($path); + } else { + $removed = @unlink($path); + } + + if (!$removed) { + cacti_log(sprintf('WARNING: webseer upgrade could not remove %s (check file/directory permissions)', $rel), false, 'WEBSEER'); + } + } + + // Surface any top-level entry the manifest does not account for. + $known = []; + + foreach (array_merge($expected, $tombstones) as $entry) { + $top = explode('/', trim((string) $entry, '/'))[0]; + + if ($top !== '') { + $known[$top] = true; + } + } + + $entries = scandir($plugin_dir); + + foreach (($entries !== false ? $entries : []) as $entry) { + if ($entry === '.' || $entry === '..' || $entry === 'tests' || $protected($entry) || isset($known[$entry])) { + continue; + } + + cacti_log(sprintf('WARNING: webseer upgrade found a file/directory not described in manifest.json: %s (left in place)', $entry), false, 'WEBSEER'); + } +} + +/** + * Recursively deletes a directory and its contents. Symlinks are removed + * without being followed. Helper for plugin_webseer_prune_files(). + * + * @param string $dir Absolute path to the directory to remove. + * + * @return bool True if the directory and everything under it was removed; + * false if any entry could not be deleted. + */ +function plugin_webseer_rmtree(string $dir): bool { + $entries = scandir($dir); + $ok = true; + + foreach (($entries !== false ? $entries : []) as $entry) { + if ($entry === '.' || $entry === '..') { + continue; + } + + $path = $dir . '/' . $entry; + + if (is_dir($path) && !is_link($path)) { + if (!plugin_webseer_rmtree($path)) { + $ok = false; + } + } elseif (!@unlink($path)) { + $ok = false; + } + } + + if (!@rmdir($dir)) { + $ok = false; + } + + return $ok; +} diff --git a/tests/Integration/PluginLifecycleTest.php b/tests/Integration/PluginLifecycleTest.php index b3931eb..cad32c4 100644 --- a/tests/Integration/PluginLifecycleTest.php +++ b/tests/Integration/PluginLifecycleTest.php @@ -17,12 +17,34 @@ */ require_once dirname(__DIR__, 2) . '/setup.php'; +// Define webseer_upgrade_tables() from the real checkout so +// plugin_webseer_upgrade() runs while base_path is sandboxed below. +require_once dirname(__DIR__, 2) . '/includes/database.php'; // tests/Pest.php's beforeEach isn't reliably discovered under this plugin's CI // invocation (pest run from the cacti root via --configuration=plugins/...), // so register it here too to guarantee a clean call log between tests in this file. beforeEach(function () { webseer_test_reset_db_mocks(); + + // Sandbox base_path so the version-drift branch runs + // plugin_webseer_prune_files() against a throwaway tree with no + // manifest.json (prune no-ops), never the real checkout. The temp tree + // carries a copy of the real INFO (so plugin_webseer_version() still + // matches) and an empty includes/database.php the upgrade's top-level + // require_once can load harmlessly. + $GLOBALS['__webseer_base_restore'] = $GLOBALS['config']['base_path']; + $base = sys_get_temp_dir() . '/webseer-itest-' . uniqid(); + mkdir($base . '/plugins/webseer/includes', 0777, true); + copy(dirname(__DIR__, 2) . '/INFO', $base . '/plugins/webseer/INFO'); + file_put_contents($base . '/plugins/webseer/includes/database.php', " ['include/', 'oldfile.php', 'userdata/', 'gone.png'], + 'expected' => ['INFO', 'setup.php', 'includes/', 'manifest.json'], + 'whitelist' => ['userdata/'], + ]; + + $base = webseer_prune_fixture($manifest); + $plugin = $base . '/plugins/webseer'; + $restore = $GLOBALS['config']['base_path']; + + $GLOBALS['config']['base_path'] = $base; + + try { + plugin_webseer_prune_files(); + } finally { + $GLOBALS['config']['base_path'] = $restore; + } + + // Tombstone and the dev-only tests/ tree are gone. + expect(is_dir($plugin . '/include'))->toBeFalse(); + expect(is_dir($plugin . '/tests'))->toBeFalse(); + expect(is_file($plugin . '/oldfile.php'))->toBeFalse(); + + // Whitelisted user data, VCS metadata, and expected files are untouched. + // (userdata/ is even listed as a tombstone, but the whitelist wins.) + expect(is_file($plugin . '/userdata/keep.dat'))->toBeTrue(); + expect(is_dir($plugin . '/.git'))->toBeTrue(); + expect(is_file($plugin . '/INFO'))->toBeTrue(); + expect(is_dir($plugin . '/includes'))->toBeTrue(); + + // An unexpected, non-whitelisted stray is left in place but logged. + expect(is_file($plugin . '/stray.php'))->toBeTrue(); + + $logged = implode("\n", $GLOBALS['__test_cacti_log']); + expect($logged)->toContain('stray.php'); + expect($logged)->not->toContain('userdata'); + expect($logged)->not->toContain('.git'); +}); + +it('is a safe no-op when the manifest is missing', function () { + $base = sys_get_temp_dir() . '/webseer-prune-missing-' . uniqid(); + mkdir($base . '/plugins/webseer', 0777, true); + $restore = $GLOBALS['config']['base_path']; + + $GLOBALS['config']['base_path'] = $base; + + try { + plugin_webseer_prune_files(); + } finally { + $GLOBALS['config']['base_path'] = $restore; + } + + expect($GLOBALS['__test_cacti_log'])->toBe([]); +}); + +it('logs and skips pruning when the manifest is malformed', function () { + $base = sys_get_temp_dir() . '/webseer-prune-bad-' . uniqid(); + $plugin = $base . '/plugins/webseer'; + mkdir($plugin, 0777, true); + file_put_contents($plugin . '/manifest.json', 'not json'); + mkdir($plugin . '/tests', 0777, true); + $restore = $GLOBALS['config']['base_path']; + + $GLOBALS['config']['base_path'] = $base; + + try { + plugin_webseer_prune_files(); + } finally { + $GLOBALS['config']['base_path'] = $restore; + } + + // A malformed manifest must not delete anything. + expect(is_dir($plugin . '/tests'))->toBeTrue(); + expect(implode("\n", $GLOBALS['__test_cacti_log']))->toContain('could not be parsed'); +}); + +it('refuses to remove a tombstone that resolves outside the plugin directory', function () { + $manifest = [ + 'tombstones' => ['../escapee.txt'], + 'expected' => ['manifest.json'], + 'whitelist' => [], + ]; + + $base = webseer_prune_fixture($manifest); + $plugin = $base . '/plugins/webseer'; + $outside = $base . '/plugins/escapee.txt'; + file_put_contents($outside, 'precious user data'); + $restore = $GLOBALS['config']['base_path']; + + $GLOBALS['config']['base_path'] = $base; + + try { + plugin_webseer_prune_files(); + } finally { + $GLOBALS['config']['base_path'] = $restore; + } + + // The out-of-tree file is untouched and the refusal is logged. + expect(is_file($outside))->toBeTrue(); + expect(implode("\n", $GLOBALS['__test_cacti_log']))->toContain('outside the plugin directory'); +}); + +it('warns when a tombstoned path cannot be removed', function () { + $manifest = [ + 'tombstones' => ['locked/'], + 'expected' => ['manifest.json'], + 'whitelist' => [], + ]; + + $base = webseer_prune_fixture($manifest); + $plugin = $base . '/plugins/webseer'; + mkdir($plugin . '/locked/sub', 0777, true); + file_put_contents($plugin . '/locked/sub/data', 'x'); + chmod($plugin . '/locked/sub', 0500); // read-only dir: its child cannot be unlinked + $restore = $GLOBALS['config']['base_path']; + + $GLOBALS['config']['base_path'] = $base; + + set_error_handler(static fn () => true); // swallow the expected unlink warning + + try { + plugin_webseer_prune_files(); + } finally { + restore_error_handler(); + $GLOBALS['config']['base_path'] = $restore; + @chmod($plugin . '/locked/sub', 0700); + } + + expect(implode("\n", $GLOBALS['__test_cacti_log']))->toContain('could not remove'); +})->skip(function () { + return function_exists('posix_getuid') && posix_getuid() === 0; +}, 'permission checks are bypassed for the root user'); diff --git a/tests/Unit/SetupHelpersGapsTest.php b/tests/Unit/SetupHelpersGapsTest.php index 53d1d15..8aede8c 100644 --- a/tests/Unit/SetupHelpersGapsTest.php +++ b/tests/Unit/SetupHelpersGapsTest.php @@ -29,6 +29,25 @@ beforeEach(function () { webseer_test_reset_db_mocks(); $GLOBALS['__test_exec_calls'] = []; + + // Sandbox base_path so the version-drift branch runs + // plugin_webseer_prune_files() against a throwaway tree with no + // manifest.json (prune no-ops), never the real checkout. The temp tree + // carries a copy of the real INFO (so plugin_webseer_version() still + // matches) and an empty includes/database.php the upgrade's top-level + // require_once can load harmlessly. + $GLOBALS['__webseer_base_restore'] = $GLOBALS['config']['base_path']; + $base = sys_get_temp_dir() . '/webseer-test-' . uniqid(); + mkdir($base . '/plugins/webseer/includes', 0777, true); + copy(__DIR__ . '/../../INFO', $base . '/plugins/webseer/INFO'); + file_put_contents($base . '/plugins/webseer/includes/database.php', " $message, 'also_print' => $also_print, 'log_type' => $log_type, 'level' => $level]; + $GLOBALS['__test_cacti_log'][] = (string) $message; } } From bb910d33791033e744dbe180c7ba642113c941cf Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 30 Sep 2026 17:06:09 -0400 Subject: [PATCH 3/8] review: propagate unique-key repair failure + docs/tests --- CHANGELOG.md | 3 ++ includes/database.php | 41 +++++++++++++---- setup.php | 13 +++++- tests/Integration/PluginLifecycleTest.php | 18 ++++++++ tests/Unit/ReintroduceUniqueKeysTest.php | 56 +++++++++++++++++++---- tests/bootstrap-unit.php | 6 +++ 6 files changed, 115 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ad9ccb..0c0e770 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ --- 3.3 --- +* compat: Lower the minimum Cacti compatibility floor from 1.2.32 to 1.2.29 +* issue: Re-introduce the `plugin_webseer_contacts` `user_id_type` UNIQUE key (on `user_id`, `type`) that Cacti 1.2.29-1.2.31's `db_update_table()` silently dropped - those releases rebuild a table's indexes only from its `keys` definition and ignore `unique_keys`. The repair runs automatically on upgrade when running one of those releases and is a no-op on 1.2.32+. WARNING: if duplicate `(user_id, type)` contact rows were inserted while the constraint was absent, re-adding the key fails; the upgrade logs the affected table and leaves the recorded version unchanged so it retries on the next request. To recover, de-duplicate `plugin_webseer_contacts` (keep a single row per `user_id`/`type`) and re-run the upgrade. Sites already upgraded to 1.2.32+ that lost the key on an earlier 1.2.29-1.2.31 run are not auto-repaired +* feature: Add a root `manifest.json` and an upgrade-time file prune that removes dev-only bundled files (e.g. `tests/`) on a version change, refuses any path resolving outside the plugin directory, and logs anything it cannot remove * refactor: Move all schema management into includes/database.php (the thold model) and manage the seven plugin_webseer_* tables through Cacti's plugin table API - create with api_plugin_db_table_create() and refresh existing tables with db_update_table() from a single shared definition, replacing the raw CREATE TABLE/version-gated ALTER TABLE migrations (the historical plugin_webseer_url_log -> plugin_webseer_urls_log rename is kept as a guarded pre-step); the upgrade path now also updates the full plugin_config row. Also switches every file inclusion from include/include_once to require/require_once * dev: Measure CI coverage with xdebug instead of pcov so the plugin's own sources are instrumented (pcov auto-scopes to the Composer root and skipped cacti/plugins/, leaving the patch-coverage gate with nothing to measure) * dev: Enforce patch coverage of changed lines in CI and remove the inert COMPOSER_ROOT_VERSION env from the Pest step diff --git a/includes/database.php b/includes/database.php index 510fc34..47bbb58 100644 --- a/includes/database.php +++ b/includes/database.php @@ -280,9 +280,11 @@ function plugin_webseer_setup_table() { * is applied as a guarded pre-step first. Called from * plugin_webseer_upgrade() when the stored version changes. * - * @return void + * @return bool True if the refresh (including any compensating UNIQUE-key + * re-add) fully succeeded; false if a unique key could not be + * restored, so the caller can defer version bookkeeping. */ -function webseer_upgrade_tables() { +function webseer_upgrade_tables(): bool { // db_update_table() can not rename, so carry the historical URL-log // table rename (and its data) forward before the schema refresh. if (db_table_exists('plugin_webseer_url_log') && !db_table_exists('plugin_webseer_urls_log')) { @@ -297,7 +299,7 @@ function webseer_upgrade_tables() { } } - webseer_reintroduce_unique_keys(); + return webseer_reintroduce_unique_keys(); } /** @@ -305,29 +307,48 @@ function webseer_upgrade_tables() { * dropped UNIQUE keys declared via a table definition's 'unique_keys' (it * rebuilds indexes only from 'keys'). Re-add this plugin's unique keys when * running on one of those releases; 1.2.32+ preserves them, so this is a - * no-op there. + * no-op there. Uses Cacti's db_add_index() (available since 1.2.29) and + * skips any key that already exists. When a key cannot be re-added - almost + * always because duplicate rows were inserted while the constraint was + * absent - the failure is logged with remediation steps and reported to the + * caller so version bookkeeping can be deferred and the repair retried. * - * @return void + * @return bool True if every applicable unique key is present (or the gate + * does not apply); false if any key could not be re-added. */ -function webseer_reintroduce_unique_keys(): void { +function webseer_reintroduce_unique_keys(): bool { $running = trim((string) db_fetch_cell('SELECT cacti FROM version LIMIT 1')); if ($running === '' || !cacti_version_compare($running, '1.2.29', '>=') || !cacti_version_compare($running, '1.2.32', '<')) { - return; + return true; } $unique_keys = [ ['table' => 'plugin_webseer_contacts', 'name' => 'user_id_type', 'columns' => ['user_id', 'type']], ]; + $ok = true; + foreach ($unique_keys as $uk) { - if (!db_index_exists($uk['table'], $uk['name'])) { - db_execute('ALTER TABLE `' . $uk['table'] . '` ADD UNIQUE KEY `' . - $uk['name'] . '` (`' . implode('`,`', $uk['columns']) . '`)'); + if (db_index_exists($uk['table'], $uk['name'])) { + continue; + } + + if (!db_add_index($uk['table'], 'UNIQUE', $uk['name'], $uk['columns'])) { + $ok = false; + + cacti_log(sprintf('ERROR: webseer could not re-add UNIQUE key `%s` on `%s` (%s). ' + . 'This happens only when duplicate (%s) rows were inserted while the constraint ' + . 'was absent on Cacti 1.2.29-1.2.31; de-duplicate the table, then re-run the upgrade ' + . 'to restore it.', + $uk['name'], $uk['table'], implode(', ', $uk['columns']), implode(', ', $uk['columns'])), + false, 'WEBSEER'); } } + + return $ok; } /** diff --git a/setup.php b/setup.php index 7d7314d..7ea3b3e 100644 --- a/setup.php +++ b/setup.php @@ -103,7 +103,9 @@ function plugin_webseer_check_config() { * proxies tables, renaming the URL log table, adding compression/ * notify-format/poller-id columns) and updates the recorded plugin * version/realm file list, based on comparing the installed version - * against the current INFO file version. Called from + * against the current INFO file version. If webseer_upgrade_tables() + * reports a failed compensating UNIQUE-key repair, the recorded version is + * left unchanged so the repair retries on a later request. Called from * plugin_webseer_check_config(), which is itself only invoked when the * current page is index.php, plugins.php, or webseer.php. * @@ -134,7 +136,14 @@ function plugin_webseer_upgrade() { // create any missing tables, db_update_table() diff the rest. The historical // plugin_webseer_url_log -> plugin_webseer_urls_log rename (and the column // additions that were previously hand-written ALTERs) are handled there. - webseer_upgrade_tables(); + if (!webseer_upgrade_tables()) { + // A compensating repair (re-adding a UNIQUE key that Cacti + // 1.2.29-1.2.31 dropped) failed - almost always because of + // pre-existing duplicate rows. Leave the stored version unchanged + // so the upgrade retries on the next request once the operator + // de-duplicates; the actionable details are already in the Cacti log. + return true; + } db_execute_prepared('UPDATE plugin_config SET version = ?, name = ?, author = ?, webpage = ? diff --git a/tests/Integration/PluginLifecycleTest.php b/tests/Integration/PluginLifecycleTest.php index cad32c4..49d16cd 100644 --- a/tests/Integration/PluginLifecycleTest.php +++ b/tests/Integration/PluginLifecycleTest.php @@ -158,3 +158,21 @@ expect($version_updates)->not->toBeEmpty(); }); + +it('leaves the recorded version unchanged when a unique-key repair fails on upgrade', function () { + // Drift plus a Cacti 1.2.29-1.2.31 run where the compensating UNIQUE-key + // re-add fails (as it would against pre-existing duplicate rows). + webseer_test_mock_db('db_fetch_cell', 'plugin_config', '1.0'); + webseer_test_mock_db('db_fetch_cell', 'SELECT cacti FROM version', '1.2.30'); + webseer_test_mock_db('db_add_index', 'plugin_webseer_contacts', false); + + plugin_webseer_upgrade(); + + $version_updates = array_filter( + $GLOBALS['__test_db_calls'], + fn ($call) => $call['fn'] === 'db_execute_prepared' && strpos($call['sql'], 'UPDATE plugin_config') !== false + ); + + expect($version_updates)->toBeEmpty(); + expect(implode("\n", $GLOBALS['__test_cacti_log'] ?? []))->toContain('could not re-add UNIQUE key'); +}); diff --git a/tests/Unit/ReintroduceUniqueKeysTest.php b/tests/Unit/ReintroduceUniqueKeysTest.php index 471d9bf..e160d21 100644 --- a/tests/Unit/ReintroduceUniqueKeysTest.php +++ b/tests/Unit/ReintroduceUniqueKeysTest.php @@ -2,13 +2,30 @@ /* +-------------------------------------------------------------------------+ | 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. | + | | + | This program is distributed in the hope that it will be useful, | + | but WITHOUT ANY WARRANTY; without even the implied warranty of | + | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | + | GNU General Public License for more details. | + +-------------------------------------------------------------------------+ + | Cacti: The Complete RRDtool-based Graphing Solution | + +-------------------------------------------------------------------------+ + | This code is designed, written, and maintained by the Cacti Group. See | + | about.php and/or the AUTHORS file for specific developer information. | + +-------------------------------------------------------------------------+ + | http://www.cacti.net/ | +-------------------------------------------------------------------------+ */ /* * Unit coverage for webseer_reintroduce_unique_keys() in includes/database.php: * the compensating re-add of UNIQUE keys that Cacti 1.2.29-1.2.31 dropped via - * db_update_table(). + * db_update_table(), and its success/failure reporting. */ beforeAll(function () { @@ -19,39 +36,48 @@ beforeEach(function () { webseer_test_reset_db_mocks(); $GLOBALS['__test_db_calls'] = []; + $GLOBALS['__test_cacti_log'] = []; }); +/** + * Collects the db_add_index() calls the bootstrap recorded during a test. + * Each entry is shaped as the stub records it: + * ['fn' => 'db_add_index', 'sql' => '.', + * 'params' => ['type' => , 'columns' => ]]. + * + * @return array The + * matching call records, re-indexed from zero. + */ function webseer_uk_added_indexes(): array { return array_values(array_filter($GLOBALS['__test_db_calls'], function ($call) { - return $call['fn'] === 'db_execute' && stripos($call['sql'], 'ADD UNIQUE KEY') !== false; + return $call['fn'] === 'db_add_index'; })); } it('re-adds the contacts unique key when running Cacti 1.2.30', function () { webseer_test_mock_db('db_fetch_cell', 'SELECT cacti FROM version', '1.2.30'); - webseer_reintroduce_unique_keys(); + expect(webseer_reintroduce_unique_keys())->toBeTrue(); $added = webseer_uk_added_indexes(); expect($added)->toHaveCount(1); expect($added[0]['sql'])->toContain('plugin_webseer_contacts') ->and($added[0]['sql'])->toContain('user_id_type') - ->and($added[0]['sql'])->toContain('`user_id`,`type`'); + ->and($added[0]['params']['type'])->toBe('UNIQUE') + ->and($added[0]['params']['columns'])->toBe(['user_id', 'type']); }); it('does nothing on Cacti 1.2.32 and later', function () { webseer_test_mock_db('db_fetch_cell', 'SELECT cacti FROM version', '1.2.32'); - webseer_reintroduce_unique_keys(); - + expect(webseer_reintroduce_unique_keys())->toBeTrue(); expect(webseer_uk_added_indexes())->toHaveCount(0); }); it('does nothing on Cacti releases before 1.2.29', function () { webseer_test_mock_db('db_fetch_cell', 'SELECT cacti FROM version', '1.2.28'); - webseer_reintroduce_unique_keys(); - + expect(webseer_reintroduce_unique_keys())->toBeTrue(); expect(webseer_uk_added_indexes())->toHaveCount(0); }); @@ -59,7 +85,17 @@ function webseer_uk_added_indexes(): array { webseer_test_mock_db('db_fetch_cell', 'SELECT cacti FROM version', '1.2.30'); webseer_test_mock_db('db_index_exists', 'plugin_webseer_contacts', true); - webseer_reintroduce_unique_keys(); - + expect(webseer_reintroduce_unique_keys())->toBeTrue(); expect(webseer_uk_added_indexes())->toHaveCount(0); }); + +it('reports failure and logs remediation when the unique key cannot be re-added', function () { + webseer_test_mock_db('db_fetch_cell', 'SELECT cacti FROM version', '1.2.30'); + webseer_test_mock_db('db_add_index', 'plugin_webseer_contacts', false); + + expect(webseer_reintroduce_unique_keys())->toBeFalse(); + + $logged = implode("\n", $GLOBALS['__test_cacti_log']); + expect($logged)->toContain('could not re-add UNIQUE key') + ->and($logged)->toContain('de-duplicate'); +}); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index c836a71..bdb9a97 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -199,6 +199,12 @@ function db_index_exists($table, $index) { } } +if (!function_exists('db_add_index')) { + function db_add_index($table, $type, $key, $columns, $log = true, $db_conn = false) { + return webseer_test_db_result('db_add_index', (string) $table . '.' . (string) $key, ['type' => $type, 'columns' => $columns], true); + } +} + if (!function_exists('cacti_version_compare')) { function cacti_version_compare($version1, $version2, $operator = '>') { return version_compare($version1, $version2, $operator); From c40b545bdbaeaeec7a281d9165ab1dca518e417f Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 30 Sep 2026 19:58:02 -0400 Subject: [PATCH 4/8] Prune phpunit.xml on upgrade and ignore markdown-lint configs phpunit.xml ships in the repo for CI but is a dev-only test artifact, so it is removed from manifest.json 'expected', pruned from installs on upgrade (like tests/), and excluded from the manifest drift check. The retired markdown-lint configs (.mdlrc, .md_style.rb) are deleted, and .md* is now ignored like .git*: protected from pruning and excluded from drift if it reappears. --- manifest.json | 1 - setup.php | 8 +++++--- tests/Unit/PruneFilesTest.php | 8 ++++++++ tests/bin/validate-manifest.php | 8 ++++---- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/manifest.json b/manifest.json index 0dd3251..80ee0b8 100644 --- a/manifest.json +++ b/manifest.json @@ -12,7 +12,6 @@ "index.php", "locales/", "manifest.json", - "phpunit.xml", "poller_webseer.php", "remote.php", "setup.php", diff --git a/setup.php b/setup.php index 7ea3b3e..a6c795a 100644 --- a/setup.php +++ b/setup.php @@ -401,7 +401,7 @@ function plugin_webseer_prune_files(): void { $whitelist = isset($manifest['whitelist']) && is_array($manifest['whitelist']) ? $manifest['whitelist'] : []; $protected = function (string $rel) use ($whitelist): bool { - if (strncmp($rel, '.git', 4) === 0) { + if (strncmp($rel, '.git', 4) === 0 || strncmp($rel, '.md', 3) === 0) { return true; } @@ -420,9 +420,11 @@ function plugin_webseer_prune_files(): void { // cannot steer the prune outside of it. $plugin_real = realpath($plugin_dir); - // Remove tombstoned (moved/deleted) paths plus the dev-only tests/ tree. + // Remove tombstoned (moved/deleted) paths plus the dev-only tests/ + // tree and the phpunit.xml test configuration. $remove = $tombstones; $remove[] = 'tests/'; + $remove[] = 'phpunit.xml'; foreach ($remove as $rel) { $rel = trim((string) $rel, '/'); @@ -474,7 +476,7 @@ function plugin_webseer_prune_files(): void { $entries = scandir($plugin_dir); foreach (($entries !== false ? $entries : []) as $entry) { - if ($entry === '.' || $entry === '..' || $entry === 'tests' || $protected($entry) || isset($known[$entry])) { + if ($entry === '.' || $entry === '..' || $entry === 'tests' || $entry === 'phpunit.xml' || $protected($entry) || isset($known[$entry])) { continue; } diff --git a/tests/Unit/PruneFilesTest.php b/tests/Unit/PruneFilesTest.php index d07bbd9..a141248 100644 --- a/tests/Unit/PruneFilesTest.php +++ b/tests/Unit/PruneFilesTest.php @@ -27,6 +27,9 @@ function webseer_prune_fixture(array $manifest): string { file_put_contents($plugin . '/setup.php', "toBeFalse(); expect(is_dir($plugin . '/tests'))->toBeFalse(); expect(is_file($plugin . '/oldfile.php'))->toBeFalse(); + expect(is_file($plugin . '/phpunit.xml'))->toBeFalse(); // Whitelisted user data, VCS metadata, and expected files are untouched. // (userdata/ is even listed as a tombstone, but the whitelist wins.) @@ -68,6 +72,8 @@ function webseer_prune_fixture(array $manifest): string { expect(is_dir($plugin . '/.git'))->toBeTrue(); expect(is_file($plugin . '/INFO'))->toBeTrue(); expect(is_dir($plugin . '/includes'))->toBeTrue(); + expect(is_file($plugin . '/.mdlrc'))->toBeTrue(); + expect(is_file($plugin . '/.md_style.rb'))->toBeTrue(); // An unexpected, non-whitelisted stray is left in place but logged. expect(is_file($plugin . '/stray.php'))->toBeTrue(); @@ -76,6 +82,8 @@ function webseer_prune_fixture(array $manifest): string { expect($logged)->toContain('stray.php'); expect($logged)->not->toContain('userdata'); expect($logged)->not->toContain('.git'); + expect($logged)->not->toContain('.mdlrc'); + expect($logged)->not->toContain('.md_style.rb'); }); it('is a safe no-op when the manifest is missing', function () { diff --git a/tests/bin/validate-manifest.php b/tests/bin/validate-manifest.php index aa9858b..c54fab5 100644 --- a/tests/bin/validate-manifest.php +++ b/tests/bin/validate-manifest.php @@ -5,8 +5,8 @@ +-------------------------------------------------------------------------+ | Validates that manifest.json's "expected" array matches the plugin's | | actual top-level tree, so the manifest that drives upgrade-time pruning | - | cannot silently drift. tests/, .git* and whitelisted (user-data) paths | - | are intentionally excluded. Exits non-zero on any drift. | + | cannot silently drift. tests/, phpunit.xml, .git* and .md* are | + | excluded, as are whitelisted (user-data) paths. Non-zero exit on drift. | +-------------------------------------------------------------------------+ */ @@ -39,11 +39,11 @@ $actual = []; foreach (scandir($root) as $entry) { - if ($entry === '.' || $entry === '..' || $entry === 'tests') { + if ($entry === '.' || $entry === '..' || $entry === 'tests' || $entry === 'phpunit.xml') { continue; } - if (strncmp($entry, '.git', 4) === 0 || isset($whitelistTop[$entry])) { + if (strncmp($entry, '.git', 4) === 0 || strncmp($entry, '.md', 3) === 0 || isset($whitelistTop[$entry])) { continue; } From 5fc7b82f28c530689e2385a3f623558c73387c52 Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 30 Sep 2026 22:18:12 -0400 Subject: [PATCH 5/8] Harden prune against tampered-manifest path traversal The upgrade-time prune now rejects any tombstone containing '.'/'..' segments (which could escape the plugin directory or resolve to its root) and treats ancestors of whitelist entries as protected, so a tombstone on a parent directory can no longer delete a whitelisted file beneath it. --- setup.php | 14 ++++++++- tests/Unit/PruneFilesTest.php | 59 ++++++++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/setup.php b/setup.php index a6c795a..773418d 100644 --- a/setup.php +++ b/setup.php @@ -408,7 +408,9 @@ function plugin_webseer_prune_files(): void { foreach ($whitelist as $entry) { $entry = trim((string) $entry, '/'); - if ($entry !== '' && ($rel === $entry || strncmp($rel, $entry . '/', strlen($entry) + 1) === 0)) { + if ($entry !== '' && ($rel === $entry + || strncmp($rel, $entry . '/', strlen($entry) + 1) === 0 + || strncmp($entry, $rel . '/', strlen($rel) + 1) === 0)) { return true; } } @@ -433,6 +435,16 @@ function plugin_webseer_prune_files(): void { continue; } + // A tombstone must never contain '.'/'..' segments; a tampered manifest + // could use them to escape the plugin directory or target its root. + $segments = explode('/', $rel); + + if (in_array('.', $segments, true) || in_array('..', $segments, true)) { + cacti_log(sprintf('WARNING: webseer prune refused to remove %s: path contains a traversal segment (tampered manifest.json?)', $rel), false, 'WEBSEER'); + + continue; + } + $path = $plugin_dir . '/' . $rel; if (!is_link($path) && !file_exists($path)) { diff --git a/tests/Unit/PruneFilesTest.php b/tests/Unit/PruneFilesTest.php index a141248..f479d43 100644 --- a/tests/Unit/PruneFilesTest.php +++ b/tests/Unit/PruneFilesTest.php @@ -146,7 +146,7 @@ function webseer_prune_fixture(array $manifest): string { // The out-of-tree file is untouched and the refusal is logged. expect(is_file($outside))->toBeTrue(); - expect(implode("\n", $GLOBALS['__test_cacti_log']))->toContain('outside the plugin directory'); + expect(implode("\n", $GLOBALS['__test_cacti_log']))->toContain('a traversal segment'); }); it('warns when a tombstoned path cannot be removed', function () { @@ -179,3 +179,60 @@ function webseer_prune_fixture(array $manifest): string { })->skip(function () { return function_exists('posix_getuid') && posix_getuid() === 0; }, 'permission checks are bypassed for the root user'); + +it('refuses a tombstone that escapes through a symlinked directory', function () { + $manifest = [ + 'tombstones' => ['escdir/secret.txt'], + 'expected' => ['manifest.json'], + 'whitelist' => [], + ]; + + $base = webseer_prune_fixture($manifest); + $plugin = $base . '/plugins/webseer'; + $outside = $base . '/outside'; + mkdir($outside, 0777, true); + file_put_contents($outside . '/secret.txt', 'precious user data'); + @symlink($outside, $plugin . '/escdir'); + $restore = $GLOBALS['config']['base_path']; + + $GLOBALS['config']['base_path'] = $base; + + try { + plugin_webseer_prune_files(); + } finally { + $GLOBALS['config']['base_path'] = $restore; + } + + // The out-of-tree file reached through the symlink is untouched and logged. + expect(is_file($outside . '/secret.txt'))->toBeTrue(); + expect(implode("\n", $GLOBALS['__test_cacti_log']))->toContain('outside the plugin directory'); +})->skip(function () { + $probe = sys_get_temp_dir() . '/.prune-symlink-probe-' . uniqid(); + $ok = @symlink(__FILE__, $probe); + @unlink($probe); + + return $ok === false; +}, 'symlinks are not supported on this filesystem'); + +it('protects a whitelisted file from a tombstone on its parent directory', function () { + $manifest = [ + 'tombstones' => ['userdata/'], + 'expected' => ['manifest.json'], + 'whitelist' => ['userdata/keep.dat'], + ]; + + $base = webseer_prune_fixture($manifest); + $plugin = $base . '/plugins/webseer'; + $restore = $GLOBALS['config']['base_path']; + + $GLOBALS['config']['base_path'] = $base; + + try { + plugin_webseer_prune_files(); + } finally { + $GLOBALS['config']['base_path'] = $restore; + } + + // A whitelisted file shields its parent directory from a tombstone. + expect(is_file($plugin . '/userdata/keep.dat'))->toBeTrue(); +}); From 91524b82db5ea6f1ef6b93a3d13cac54ca2b322b Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 30 Sep 2026 22:33:16 -0400 Subject: [PATCH 6/8] docs: align the Project Structure comment column The trailing '# ...' comments in the Project Structure block drifted further right down the tree; align them all to a single column. --- .github/copilot-instructions.md | 36 ++++++++++++++++----------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a686a06..7796467 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -24,25 +24,25 @@ When generating code for this repository: ## Project Structure ``` -webseer/ # Repository root (install to plugins/webseer/ in Cacti) -├── classes/ # Supporting PHP classes (cURL, mxlookup) -├── includes/ # Library/helper files, require_once'd from the entry points -│ ├── database.php # Schema management: table defs + create/upgrade/drop helpers -│ ├── functions.php # Shared plugin functions -│ ├── arrays.php # Shared option/label arrays -│ └── constants.php # Shared constants -├── locales/ # Internationalization files -├── tests/ # Test suite -├── ca-bundle.crt # CA bundle for HTTPS endpoint verification -├── poller_webseer.php # Background poller entry point (CLI) -├── remote.php # Remote poller support endpoint -├── webseer.php # Main viewer/administration UI -├── webseer_process.php # Check execution logic -├── webseer_proxies.php # Proxy administration -├── webseer_servers.php # Monitored server/service administration -├── INFO # Plugin metadata (name, version, compat) +webseer/ # Repository root (install to plugins/webseer/ in Cacti) +├── classes/ # Supporting PHP classes (cURL, mxlookup) +├── includes/ # Library/helper files, require_once'd from the entry points +│ ├── database.php # Schema management: table defs + create/upgrade/drop helpers +│ ├── functions.php # Shared plugin functions +│ ├── arrays.php # Shared option/label arrays +│ └── constants.php # Shared constants +├── locales/ # Internationalization files +├── tests/ # Test suite +├── ca-bundle.crt # CA bundle for HTTPS endpoint verification +├── poller_webseer.php # Background poller entry point (CLI) +├── remote.php # Remote poller support endpoint +├── webseer.php # Main viewer/administration UI +├── webseer_process.php # Check execution logic +├── webseer_proxies.php # Proxy administration +├── webseer_servers.php # Monitored server/service administration +├── INFO # Plugin metadata (name, version, compat) ├── README.md -└── setup.php # Plugin install/uninstall/upgrade hooks +└── setup.php # Plugin install/uninstall/upgrade hooks ``` ## Naming Conventions From faf22d73f149355c3e860149e92e7c9d214c7550 Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 30 Sep 2026 23:01:06 -0400 Subject: [PATCH 7/8] docs/tests: standardize test header and correct manifest-contract notes - Use the full license header (from the plugin's own setup.php) in tests/Unit/PruneFilesTest.php instead of the abbreviated copyright banner. - Document that the manifest drift check and the upgrade-time prune also handle phpunit.xml and .md* files, matching the implemented behavior. --- .github/copilot-instructions.md | 2 +- tests/Unit/PruneFilesTest.php | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 7796467..9ec867e 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -222,4 +222,4 @@ existing code or adding new code, not just in dedicated cleanup passes: ## File manifest & upgrade pruning -The plugin ships a root `manifest.json` with three arrays: `tombstones` (files/directories older versions shipped that have since moved or been removed), `expected` (the top-level files and directories that ship today, directories written with a trailing `/`), and `whitelist` (paths holding user data that must never be touched). Keep `expected` current: CI runs `tests/bin/validate-manifest.php`, which fails on any drift between `expected` and the real top-level tree (it ignores `tests/`, `.git*`, and whitelisted paths). Custom customer CSS/theme files belong in `expected`, and stylesheets live in `css/` (not `themes/`). On upgrade, `plugin_webseer_prune_files()` deletes the tombstoned paths and the dev-only `tests/` tree, leaves `whitelist` and `.git*` alone, and logs (without removing) any top-level entry the manifest does not account for. As a safety measure it refuses any tombstone that resolves outside the plugin directory (a tampered manifest.json) and logs a warning for any file or directory it cannot remove. When you move or delete a shipped file, add its old path to `tombstones` and update `expected` in the same change. +The plugin ships a root `manifest.json` with three arrays: `tombstones` (files/directories older versions shipped that have since moved or been removed), `expected` (the top-level files and directories that ship today, directories written with a trailing `/`), and `whitelist` (paths holding user data that must never be touched). Keep `expected` current: CI runs `tests/bin/validate-manifest.php`, which fails on any drift between `expected` and the real top-level tree (it ignores `tests/`, `phpunit.xml`, `.git*`, `.md*`, and whitelisted paths). Custom customer CSS/theme files belong in `expected`, and stylesheets live in `css/` (not `themes/`). On upgrade, `plugin_webseer_prune_files()` deletes the tombstoned paths, the dev-only `tests/` tree, and the `phpunit.xml` test config, leaves `whitelist`, `.git*`, and `.md*` alone, and logs (without removing) any top-level entry the manifest does not account for. As a safety measure it refuses any tombstone that resolves outside the plugin directory (a tampered manifest.json) and logs a warning for any file or directory it cannot remove. When you move or delete a shipped file, add its old path to `tombstones` and update `expected` in the same change. diff --git a/tests/Unit/PruneFilesTest.php b/tests/Unit/PruneFilesTest.php index f479d43..6868a3b 100644 --- a/tests/Unit/PruneFilesTest.php +++ b/tests/Unit/PruneFilesTest.php @@ -2,6 +2,23 @@ /* +-------------------------------------------------------------------------+ | 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. | + | | + | This program is distributed in the hope that it will be useful, | + | but WITHOUT ANY WARRANTY; without even the implied warranty of | + | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | + | GNU General Public License for more details. | + +-------------------------------------------------------------------------+ + | Cacti: The Complete RRDtool-based Graphing Solution | + +-------------------------------------------------------------------------+ + | This code is designed, written, and maintained by the Cacti Group. See | + | about.php and/or the AUTHORS file for specific developer information. | + +-------------------------------------------------------------------------+ + | http://www.cacti.net/ | +-------------------------------------------------------------------------+ */ From 74a0f9ddb594efccd4ad9322c9487fd8e54b0caa Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 30 Sep 2026 23:19:02 -0400 Subject: [PATCH 8/8] refactor: rename prune helpers to the plugin-slug function prefix The repository naming contract reserves the plugin__ prefix for plugin lifecycle / hook-registration functions; all other functions use the plain _ prefix. Rename the internal upgrade helpers accordingly: plugin__prune_files() -> _prune_files() plugin__rmtree() -> _rmtree() The call site, unit tests, and the copilot-instructions.md references are updated to match. No behavioral change. --- .github/copilot-instructions.md | 2 +- setup.php | 12 ++++++------ tests/Integration/PluginLifecycleTest.php | 2 +- tests/Unit/PruneFilesTest.php | 16 ++++++++-------- tests/Unit/SetupHelpersGapsTest.php | 2 +- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 9ec867e..b1f34ab 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -222,4 +222,4 @@ existing code or adding new code, not just in dedicated cleanup passes: ## File manifest & upgrade pruning -The plugin ships a root `manifest.json` with three arrays: `tombstones` (files/directories older versions shipped that have since moved or been removed), `expected` (the top-level files and directories that ship today, directories written with a trailing `/`), and `whitelist` (paths holding user data that must never be touched). Keep `expected` current: CI runs `tests/bin/validate-manifest.php`, which fails on any drift between `expected` and the real top-level tree (it ignores `tests/`, `phpunit.xml`, `.git*`, `.md*`, and whitelisted paths). Custom customer CSS/theme files belong in `expected`, and stylesheets live in `css/` (not `themes/`). On upgrade, `plugin_webseer_prune_files()` deletes the tombstoned paths, the dev-only `tests/` tree, and the `phpunit.xml` test config, leaves `whitelist`, `.git*`, and `.md*` alone, and logs (without removing) any top-level entry the manifest does not account for. As a safety measure it refuses any tombstone that resolves outside the plugin directory (a tampered manifest.json) and logs a warning for any file or directory it cannot remove. When you move or delete a shipped file, add its old path to `tombstones` and update `expected` in the same change. +The plugin ships a root `manifest.json` with three arrays: `tombstones` (files/directories older versions shipped that have since moved or been removed), `expected` (the top-level files and directories that ship today, directories written with a trailing `/`), and `whitelist` (paths holding user data that must never be touched). Keep `expected` current: CI runs `tests/bin/validate-manifest.php`, which fails on any drift between `expected` and the real top-level tree (it ignores `tests/`, `phpunit.xml`, `.git*`, `.md*`, and whitelisted paths). Custom customer CSS/theme files belong in `expected`, and stylesheets live in `css/` (not `themes/`). On upgrade, `webseer_prune_files()` deletes the tombstoned paths, the dev-only `tests/` tree, and the `phpunit.xml` test config, leaves `whitelist`, `.git*`, and `.md*` alone, and logs (without removing) any top-level entry the manifest does not account for. As a safety measure it refuses any tombstone that resolves outside the plugin directory (a tampered manifest.json) and logs a warning for any file or directory it cannot remove. When you move or delete a shipped file, add its old path to `tombstones` and update `expected` in the same change. diff --git a/setup.php b/setup.php index 773418d..f0ad348 100644 --- a/setup.php +++ b/setup.php @@ -164,7 +164,7 @@ function plugin_webseer_upgrade() { api_plugin_register_hook('webseer', 'replicate_out', 'webseer_replicate_out', 'setup.php', '1'); - plugin_webseer_prune_files(); + webseer_prune_files(); } return true; @@ -378,7 +378,7 @@ function webseer_replicate_out($data) { * @global array $config Cacti global configuration array; used to resolve * the plugin directory. */ -function plugin_webseer_prune_files(): void { +function webseer_prune_files(): void { global $config; $plugin_dir = $config['base_path'] . '/plugins/webseer'; @@ -464,7 +464,7 @@ function plugin_webseer_prune_files(): void { } if (is_dir($path) && !is_link($path)) { - $removed = plugin_webseer_rmtree($path); + $removed = webseer_rmtree($path); } else { $removed = @unlink($path); } @@ -498,14 +498,14 @@ function plugin_webseer_prune_files(): void { /** * Recursively deletes a directory and its contents. Symlinks are removed - * without being followed. Helper for plugin_webseer_prune_files(). + * without being followed. Helper for webseer_prune_files(). * * @param string $dir Absolute path to the directory to remove. * * @return bool True if the directory and everything under it was removed; * false if any entry could not be deleted. */ -function plugin_webseer_rmtree(string $dir): bool { +function webseer_rmtree(string $dir): bool { $entries = scandir($dir); $ok = true; @@ -517,7 +517,7 @@ function plugin_webseer_rmtree(string $dir): bool { $path = $dir . '/' . $entry; if (is_dir($path) && !is_link($path)) { - if (!plugin_webseer_rmtree($path)) { + if (!webseer_rmtree($path)) { $ok = false; } } elseif (!@unlink($path)) { diff --git a/tests/Integration/PluginLifecycleTest.php b/tests/Integration/PluginLifecycleTest.php index 49d16cd..0035598 100644 --- a/tests/Integration/PluginLifecycleTest.php +++ b/tests/Integration/PluginLifecycleTest.php @@ -28,7 +28,7 @@ webseer_test_reset_db_mocks(); // Sandbox base_path so the version-drift branch runs - // plugin_webseer_prune_files() against a throwaway tree with no + // webseer_prune_files() against a throwaway tree with no // manifest.json (prune no-ops), never the real checkout. The temp tree // carries a copy of the real INFO (so plugin_webseer_version() still // matches) and an empty includes/database.php the upgrade's top-level diff --git a/tests/Unit/PruneFilesTest.php b/tests/Unit/PruneFilesTest.php index 6868a3b..015d6df 100644 --- a/tests/Unit/PruneFilesTest.php +++ b/tests/Unit/PruneFilesTest.php @@ -23,7 +23,7 @@ */ /* - * Unit coverage for plugin_webseer_prune_files(): tombstone/tests removal, + * Unit coverage for webseer_prune_files(): tombstone/tests removal, * whitelist and .git protection, and logging of unaccounted-for entries. */ @@ -72,7 +72,7 @@ function webseer_prune_fixture(array $manifest): string { $GLOBALS['config']['base_path'] = $base; try { - plugin_webseer_prune_files(); + webseer_prune_files(); } finally { $GLOBALS['config']['base_path'] = $restore; } @@ -111,7 +111,7 @@ function webseer_prune_fixture(array $manifest): string { $GLOBALS['config']['base_path'] = $base; try { - plugin_webseer_prune_files(); + webseer_prune_files(); } finally { $GLOBALS['config']['base_path'] = $restore; } @@ -130,7 +130,7 @@ function webseer_prune_fixture(array $manifest): string { $GLOBALS['config']['base_path'] = $base; try { - plugin_webseer_prune_files(); + webseer_prune_files(); } finally { $GLOBALS['config']['base_path'] = $restore; } @@ -156,7 +156,7 @@ function webseer_prune_fixture(array $manifest): string { $GLOBALS['config']['base_path'] = $base; try { - plugin_webseer_prune_files(); + webseer_prune_files(); } finally { $GLOBALS['config']['base_path'] = $restore; } @@ -185,7 +185,7 @@ function webseer_prune_fixture(array $manifest): string { set_error_handler(static fn () => true); // swallow the expected unlink warning try { - plugin_webseer_prune_files(); + webseer_prune_files(); } finally { restore_error_handler(); $GLOBALS['config']['base_path'] = $restore; @@ -215,7 +215,7 @@ function webseer_prune_fixture(array $manifest): string { $GLOBALS['config']['base_path'] = $base; try { - plugin_webseer_prune_files(); + webseer_prune_files(); } finally { $GLOBALS['config']['base_path'] = $restore; } @@ -245,7 +245,7 @@ function webseer_prune_fixture(array $manifest): string { $GLOBALS['config']['base_path'] = $base; try { - plugin_webseer_prune_files(); + webseer_prune_files(); } finally { $GLOBALS['config']['base_path'] = $restore; } diff --git a/tests/Unit/SetupHelpersGapsTest.php b/tests/Unit/SetupHelpersGapsTest.php index 8aede8c..c9d4dbd 100644 --- a/tests/Unit/SetupHelpersGapsTest.php +++ b/tests/Unit/SetupHelpersGapsTest.php @@ -31,7 +31,7 @@ $GLOBALS['__test_exec_calls'] = []; // Sandbox base_path so the version-drift branch runs - // plugin_webseer_prune_files() against a throwaway tree with no + // webseer_prune_files() against a throwaway tree with no // manifest.json (prune no-ops), never the real checkout. The temp tree // carries a copy of the real INFO (so plugin_webseer_version() still // matches) and an empty includes/database.php the upgrade's top-level