From 36a459754e0fa1818556f15e7d355594affda8b5 Mon Sep 17 00:00:00 2001 From: Copilot Date: Tue, 29 Sep 2026 22:45:44 -0400 Subject: [PATCH 01/13] INFO: set compat to 1.2.32 --- INFO | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/INFO b/INFO index 7099aeb2..ebff1502 100644 --- a/INFO +++ b/INFO @@ -26,5 +26,5 @@ longname = Thresholds author = The Cacti Group email = homepage = http://www.cacti.net -compat = 1.2.25 +compat = 1.2.32 capabilities = online_view:1, online_mgmt:1, offline_view:0, offline_mgmt:0, remote_collect:1 From c63fa0451dbcfe0e5a7d8c90535069a5d7348d0a Mon Sep 17 00:00:00 2001 From: TheWitness Date: Tue, 29 Sep 2026 23:55:50 -0400 Subject: [PATCH 02/13] docs: update Copilot instructions Cacti baseline to 1.2.32 (matches INFO compat) --- .github/copilot-instructions.md | 362 ++++++++++++++++---------------- 1 file changed, 181 insertions(+), 181 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 86f47c9f..275e0615 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,182 +1,182 @@ -# GitHub Copilot Instructions - -## Priority Guidelines - -When generating code for this repository: - -1. **Version Compatibility**: This is a Cacti plugin (`thold`, "Thresholds", version 1.8.2) targeting Cacti 1.2.25+ -2. **Context Files**: Prioritize patterns and standards defined in this file (`.github/copilot-instructions.md`) -3. **Codebase Patterns**: When context files don't provide specific guidance, scan the codebase for established patterns -4. **Architectural Consistency**: Maintain plugin-based architecture extending Cacti core -5. **Code Quality**: Prioritize security, maintainability, and compatibility in all generated code - -## Technology Stack - -### Core Technologies -- **PHP**: 8.1+ (CI matrix tests 8.1-8.4) -- **Platform**: Cacti Plugin Architecture (Cacti 1.2.25+) -- **Database**: MySQL/MariaDB with InnoDB engine -- **Alerting**: Email, Syslog, and SNMP Traps/Informs notification channels - -### Key Dependencies -- Cacti core framework (`api_plugin_*`, `db_*`) -- `CACTI-THOLD-MIB` for SNMP trap/inform definitions -- Optional: `gettext` for internationalization - -## Project Structure - -``` -thold/ # Repository root (install to plugins/thold/ in Cacti) -├── extras/ # Supplementary assets -├── includes/ # polling.php (poller hooks), settings.php (config UI), tab.php -├── service/ # systemd unit for thold_daemon -├── tests/ # Test suite (phpunit.xml) -├── themes/ # CSS theme overlays -├── cli_import.php / cli_thresholds.php # CLI threshold management utilities -├── notify_lists.php / notify_queue.php # Notification list/queue administration -├── thold.php # Main threshold administration UI -├── thold_daemon.php # Standalone high-scale daemon (bypasses poller hook) -├── thold_functions.php # Core utility/business logic -├── thold_graph.php / thold_notify.php # Graph-threshold view / notification dispatch -├── thold_process.php / thold_templates.php # Background processing / threshold templates -├── thold_webapi.php # Web API endpoints -├── poller_thold.php # Background poller entry point (CLI) -├── INFO # Plugin metadata (name, version, compat) -├── README.md -└── setup.php # Plugin install/uninstall/upgrade hooks -``` - -## Naming Conventions - -### Function Names -- **All functions and global variables** MUST be prefixed `thold_`: `thold_functions.php`, `thold_poller_output()`, `thold_config_settings()`. -- Match the existing prefix used by the function you are editing; do not introduce a new naming scheme. - -### Database Tables -All plugin tables are prefixed `plugin_thold_`. - -### Variables and Constants -- Access Cacti configuration via the global `$config` array; use `$config['base_path']` for absolute file paths. - -## Code Style - -### Indentation and Formatting -- **Tabs**: Use tabs (not spaces) for indentation throughout all PHP files. -- **Braces**: Opening brace on the same line for functions and control structures. -- **Spacing**: Space after control structure keywords (`if`, `foreach`, `while`). - -### File Headers -ALL PHP files MUST include the standard GPL v2 license header used throughout this repository (see `setup.php`), crediting "The Cacti Group". - -## Security Standards - -### SQL Query Security -**ALWAYS use prepared statements** for database operations: - -```php -// CORRECT -db_execute_prepared($sql, $params); -db_fetch_assoc($sql); // only for queries with no variable input -db_fetch_cell($sql); // only for queries with no variable input - -// WRONG - never concatenate request input into SQL -db_fetch_row("SELECT * FROM plugin_thold_thresholds WHERE id = $id"); -``` - -### Input Validation and Sanitization -Sanitize inputs using `sanitize_thold_sort_string()` or Cacti's built-in input validation functions (`get_filter_request_var()`, `get_nfilter_request_var()`); never read `$_GET`/`$_POST` directly. - -`get_filter_request_var()` (and its `gfrv()` shorthand, where available) called with only the -`$name` argument (no regex/filter as the 2nd/3rd argument) already validates the value as numeric -and returns it as a **string** -- it does not return an int, and it halts execution if the request -value is not numeric. Because of this, do NOT cast its output to `(int)` when the result is only -used for string output (e.g. `print`/`echo`, string concatenation, embedding in HTML/JS); the cast -is redundant. Only cast when the value is genuinely used in an integer/numeric context (e.g. -arithmetic, strict `===` comparisons). - -## Database Operations - -Use Cacti's global database functions: `db_execute_prepared($sql, $params)` for writes, `db_fetch_assoc($sql)` / `db_fetch_cell($sql)` for reads. - -## Internationalization - -Use `__('String', 'thold')` for all user-facing strings to support internationalization. - -## Plugin Architecture - -### Data Flow -1. **Data Collection**: Cacti poller collects data. -2. **Interception**: `thold_poller_output()` (in `includes/polling.php`) receives the data. -3. **Processing**: Standard mode processes immediately within the poller hook; Daemon mode queues data for `thold_daemon.php` to process asynchronously. -4. **Alerting**: If a threshold is breached, `thold_functions.php` handles notification dispatch. - -### Plugin Hooks -Register hooks in `setup.php` (see the full list of ~30 hooks covering device/graph/data-source actions, poller integration, and template change events); keep new hooks registered the same way via `api_plugin_register_hook($plugin, 'hook_name', 'callback', 'file.php')`. - -### Daemon Mode -`thold_daemon.php` is a standalone daemon for high-scalability environments, bypassing the standard poller hook — requires systemd service installation (`service/systemd/thold_daemon.service`). Keep daemon-mode processing logic in sync with the standard poller-hook processing path in `includes/polling.php`. - -## Best Practices - -1. Keep all new functions and globals under the single `thold_` prefix. -2. Prefer `db_*_prepared()` over string-concatenated SQL. -3. Keep daemon-mode and poller-hook-mode threshold evaluation logic consistent. -4. Wrap all user-facing strings with `__('text', 'thold')`. - -## Common Pitfalls to Avoid - -```php -// WRONG - concatenated SQL -$sql = "SELECT * FROM plugin_thold_thresholds WHERE id = $id"; - -// CORRECT -$row = db_fetch_row_prepared('SELECT * FROM plugin_thold_thresholds WHERE id = ?', array($id)); -``` - -## Version Control - -Testing changes in a safe environment is crucial, especially when dealing with database interactions and alerting mechanisms. Document all changes in `CHANGELOG.md`. - -## CI & Dependency Baselines - -- Do not commit a `composer.json` or `composer.lock` in this plugin's own repo root — the shared CI workflow installs Pest/dev dependencies into Cacti's own Composer-managed vendor tree (checked out alongside the plugin). Use Cacti's `composer.json`, not a plugin-local one. -- Do not add a plugin-local `.phpstan.neon`/`phpstan.neon` or `.php-cs-fixer.php`/`.php-cs-fixer.dist.php` — lint/static-analysis steps run against Cacti's own config from the Cacti core checkout, targeting this plugin's directory. Use the Cacti version, not a plugin-local config. -- Prefer Cacti's `cacti_count()`/`cacti_sizeof()` wrappers over the raw `count()`/`sizeof()` builtins in new or edited code. - -## Internationalization (i18n) - -- Translatable strings are managed with GNU gettext via `locales/build_gettext.sh`. `locales/po/cacti.pot` is the source template; Weblate owns syncing the per-language `.po`/`.mo` files from it. -- **Never commit the per-language `.po` or compiled `.mo` files** (`locales/po/*.po`, `locales/LC_MESSAGES/*.mo`) in a plugin PR. Weblate is the sole owner of those catalogs, and regenerating them here produces spurious diffs and merge conflicts. `locales/po/cacti.pot` is the ONLY translation artifact a PR may add or modify. +# GitHub Copilot Instructions + +## Priority Guidelines + +When generating code for this repository: + +1. **Version Compatibility**: This is a Cacti plugin (`thold`, "Thresholds", version 1.8.2) targeting Cacti 1.2.32+ +2. **Context Files**: Prioritize patterns and standards defined in this file (`.github/copilot-instructions.md`) +3. **Codebase Patterns**: When context files don't provide specific guidance, scan the codebase for established patterns +4. **Architectural Consistency**: Maintain plugin-based architecture extending Cacti core +5. **Code Quality**: Prioritize security, maintainability, and compatibility in all generated code + +## Technology Stack + +### Core Technologies +- **PHP**: 8.1+ (CI matrix tests 8.1-8.4) +- **Platform**: Cacti Plugin Architecture (Cacti 1.2.32+) +- **Database**: MySQL/MariaDB with InnoDB engine +- **Alerting**: Email, Syslog, and SNMP Traps/Informs notification channels + +### Key Dependencies +- Cacti core framework (`api_plugin_*`, `db_*`) +- `CACTI-THOLD-MIB` for SNMP trap/inform definitions +- Optional: `gettext` for internationalization + +## Project Structure + +``` +thold/ # Repository root (install to plugins/thold/ in Cacti) +├── extras/ # Supplementary assets +├── includes/ # polling.php (poller hooks), settings.php (config UI), tab.php +├── service/ # systemd unit for thold_daemon +├── tests/ # Test suite (phpunit.xml) +├── themes/ # CSS theme overlays +├── cli_import.php / cli_thresholds.php # CLI threshold management utilities +├── notify_lists.php / notify_queue.php # Notification list/queue administration +├── thold.php # Main threshold administration UI +├── thold_daemon.php # Standalone high-scale daemon (bypasses poller hook) +├── thold_functions.php # Core utility/business logic +├── thold_graph.php / thold_notify.php # Graph-threshold view / notification dispatch +├── thold_process.php / thold_templates.php # Background processing / threshold templates +├── thold_webapi.php # Web API endpoints +├── poller_thold.php # Background poller entry point (CLI) +├── INFO # Plugin metadata (name, version, compat) +├── README.md +└── setup.php # Plugin install/uninstall/upgrade hooks +``` + +## Naming Conventions + +### Function Names +- **All functions and global variables** MUST be prefixed `thold_`: `thold_functions.php`, `thold_poller_output()`, `thold_config_settings()`. +- Match the existing prefix used by the function you are editing; do not introduce a new naming scheme. + +### Database Tables +All plugin tables are prefixed `plugin_thold_`. + +### Variables and Constants +- Access Cacti configuration via the global `$config` array; use `$config['base_path']` for absolute file paths. + +## Code Style + +### Indentation and Formatting +- **Tabs**: Use tabs (not spaces) for indentation throughout all PHP files. +- **Braces**: Opening brace on the same line for functions and control structures. +- **Spacing**: Space after control structure keywords (`if`, `foreach`, `while`). + +### File Headers +ALL PHP files MUST include the standard GPL v2 license header used throughout this repository (see `setup.php`), crediting "The Cacti Group". + +## Security Standards + +### SQL Query Security +**ALWAYS use prepared statements** for database operations: + +```php +// CORRECT +db_execute_prepared($sql, $params); +db_fetch_assoc($sql); // only for queries with no variable input +db_fetch_cell($sql); // only for queries with no variable input + +// WRONG - never concatenate request input into SQL +db_fetch_row("SELECT * FROM plugin_thold_thresholds WHERE id = $id"); +``` + +### Input Validation and Sanitization +Sanitize inputs using `sanitize_thold_sort_string()` or Cacti's built-in input validation functions (`get_filter_request_var()`, `get_nfilter_request_var()`); never read `$_GET`/`$_POST` directly. + +`get_filter_request_var()` (and its `gfrv()` shorthand, where available) called with only the +`$name` argument (no regex/filter as the 2nd/3rd argument) already validates the value as numeric +and returns it as a **string** -- it does not return an int, and it halts execution if the request +value is not numeric. Because of this, do NOT cast its output to `(int)` when the result is only +used for string output (e.g. `print`/`echo`, string concatenation, embedding in HTML/JS); the cast +is redundant. Only cast when the value is genuinely used in an integer/numeric context (e.g. +arithmetic, strict `===` comparisons). + +## Database Operations + +Use Cacti's global database functions: `db_execute_prepared($sql, $params)` for writes, `db_fetch_assoc($sql)` / `db_fetch_cell($sql)` for reads. + +## Internationalization + +Use `__('String', 'thold')` for all user-facing strings to support internationalization. + +## Plugin Architecture + +### Data Flow +1. **Data Collection**: Cacti poller collects data. +2. **Interception**: `thold_poller_output()` (in `includes/polling.php`) receives the data. +3. **Processing**: Standard mode processes immediately within the poller hook; Daemon mode queues data for `thold_daemon.php` to process asynchronously. +4. **Alerting**: If a threshold is breached, `thold_functions.php` handles notification dispatch. + +### Plugin Hooks +Register hooks in `setup.php` (see the full list of ~30 hooks covering device/graph/data-source actions, poller integration, and template change events); keep new hooks registered the same way via `api_plugin_register_hook($plugin, 'hook_name', 'callback', 'file.php')`. + +### Daemon Mode +`thold_daemon.php` is a standalone daemon for high-scalability environments, bypassing the standard poller hook — requires systemd service installation (`service/systemd/thold_daemon.service`). Keep daemon-mode processing logic in sync with the standard poller-hook processing path in `includes/polling.php`. + +## Best Practices + +1. Keep all new functions and globals under the single `thold_` prefix. +2. Prefer `db_*_prepared()` over string-concatenated SQL. +3. Keep daemon-mode and poller-hook-mode threshold evaluation logic consistent. +4. Wrap all user-facing strings with `__('text', 'thold')`. + +## Common Pitfalls to Avoid + +```php +// WRONG - concatenated SQL +$sql = "SELECT * FROM plugin_thold_thresholds WHERE id = $id"; + +// CORRECT +$row = db_fetch_row_prepared('SELECT * FROM plugin_thold_thresholds WHERE id = ?', array($id)); +``` + +## Version Control + +Testing changes in a safe environment is crucial, especially when dealing with database interactions and alerting mechanisms. Document all changes in `CHANGELOG.md`. + +## CI & Dependency Baselines + +- Do not commit a `composer.json` or `composer.lock` in this plugin's own repo root — the shared CI workflow installs Pest/dev dependencies into Cacti's own Composer-managed vendor tree (checked out alongside the plugin). Use Cacti's `composer.json`, not a plugin-local one. +- Do not add a plugin-local `.phpstan.neon`/`phpstan.neon` or `.php-cs-fixer.php`/`.php-cs-fixer.dist.php` — lint/static-analysis steps run against Cacti's own config from the Cacti core checkout, targeting this plugin's directory. Use the Cacti version, not a plugin-local config. +- Prefer Cacti's `cacti_count()`/`cacti_sizeof()` wrappers over the raw `count()`/`sizeof()` builtins in new or edited code. + +## Internationalization (i18n) + +- Translatable strings are managed with GNU gettext via `locales/build_gettext.sh`. `locales/po/cacti.pot` is the source template; Weblate owns syncing the per-language `.po`/`.mo` files from it. +- **Never commit the per-language `.po` or compiled `.mo` files** (`locales/po/*.po`, `locales/LC_MESSAGES/*.mo`) in a plugin PR. Weblate is the sole owner of those catalogs, and regenerating them here produces spurious diffs and merge conflicts. `locales/po/cacti.pot` is the ONLY translation artifact a PR may add or modify. - When a pull request adds or changes a string wrapped in `__()`/`__n()`/`__esc()`/`__x()`/`__xn()`/`__gettext()`, run `locales/build_gettext.sh` before pushing and stage `locales/po/cacti.pot` only. `build_gettext.sh` also rewrites the `.po`/`.mo` files as a side effect; revert those before committing (`git checkout -- locales/po/*.po locales/LC_MESSAGES`), or run only the `xgettext` step that targets `cacti.pot`. - -## References - -- [Cacti DB Functions](https://github.com/Cacti/cacti/blob/1.2.x/lib/database.php) -- [Cacti Documentation](https://www.github.com/Cacti/documentation) -- `README.md` for feature descriptions -- `CHANGELOG.md` for version history - -## Security & Quality Conventions - -These conventions apply across the Cacti plugin fleet and should be followed whenever touching -existing code or adding new code, not just in dedicated cleanup passes: - -- **No hardcoded third-party hosts.** Never hardcode a third-party IP address, hostname, or URL - in plugin code (even for tooling/download helpers). Expose it as a plugin setting instead, with - secure-by-default values (e.g. an SSL-verification setting that defaults to verify-on). -- **Prepared statements over `db_qstr()`.** Build dynamic `WHERE` clauses using the - `$sql_where`/`$sql_params` prepared-statement pattern, not string concatenation via `db_qstr()`. -- **Use `html_escape_request_var()`.** Prefer it over the `html_escape(get_request_var(...))` call - chain. -- **Harden `unserialize()`.** Always pass `['allow_classes' => false]` as the second argument. -- **i18n text domain.** Every `__()`/`__esc()` call must include this plugin's text domain as the - final argument, except when deliberately comparing against a literal, untranslated Cacti-core - label. -- **Plugin table-creation API.** Use `api_plugin_db_table_create()`/`api_plugin_db_add_column()` - (from Cacti core's `lib/plugins.php`) instead of raw `CREATE TABLE`/`ALTER TABLE ... ADD COLUMN`. - Both are idempotent (safe no-ops when already applied), so the same call can run unconditionally - from both the install AND upgrade paths. -- **PHPDoc shape.** Every function gets a PHPDoc block: a one-line description, a blank comment - 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. + +## References + +- [Cacti DB Functions](https://github.com/Cacti/cacti/blob/1.2.x/lib/database.php) +- [Cacti Documentation](https://www.github.com/Cacti/documentation) +- `README.md` for feature descriptions +- `CHANGELOG.md` for version history + +## Security & Quality Conventions + +These conventions apply across the Cacti plugin fleet and should be followed whenever touching +existing code or adding new code, not just in dedicated cleanup passes: + +- **No hardcoded third-party hosts.** Never hardcode a third-party IP address, hostname, or URL + in plugin code (even for tooling/download helpers). Expose it as a plugin setting instead, with + secure-by-default values (e.g. an SSL-verification setting that defaults to verify-on). +- **Prepared statements over `db_qstr()`.** Build dynamic `WHERE` clauses using the + `$sql_where`/`$sql_params` prepared-statement pattern, not string concatenation via `db_qstr()`. +- **Use `html_escape_request_var()`.** Prefer it over the `html_escape(get_request_var(...))` call + chain. +- **Harden `unserialize()`.** Always pass `['allow_classes' => false]` as the second argument. +- **i18n text domain.** Every `__()`/`__esc()` call must include this plugin's text domain as the + final argument, except when deliberately comparing against a literal, untranslated Cacti-core + label. +- **Plugin table-creation API.** Use `api_plugin_db_table_create()`/`api_plugin_db_add_column()` + (from Cacti core's `lib/plugins.php`) instead of raw `CREATE TABLE`/`ALTER TABLE ... ADD COLUMN`. + Both are idempotent (safe no-ops when already applied), so the same call can run unconditionally + from both the install AND upgrade paths. +- **PHPDoc shape.** Every function gets a PHPDoc block: a one-line description, a blank comment + 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. From a2506ed99cb1be1ed30708462846a331304c5aaa Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 30 Sep 2026 10:53:21 -0400 Subject: [PATCH 03/13] INFO: back off compat to 1.2.29 --- INFO | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/INFO b/INFO index ebff1502..51e407fe 100644 --- a/INFO +++ b/INFO @@ -26,5 +26,5 @@ longname = Thresholds author = The Cacti Group email = homepage = http://www.cacti.net -compat = 1.2.32 +compat = 1.2.29 capabilities = online_view:1, online_mgmt:1, offline_view:0, offline_mgmt:0, remote_collect:1 From b95136686f85245c40e96c3c25fd16e98567d2a5 Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 30 Sep 2026 16:19:01 -0400 Subject: [PATCH 04/13] Add manifest.json + upgrade-time file pruning --- .github/copilot-instructions.md | 4 + .github/workflows/plugin-ci-workflow.yml | 3 + README.md | 6 + {themes => css}/classic/index.php | 0 {themes => css}/classic/main.css | 0 {themes => css}/dark/index.php | 0 {themes => css}/dark/main.css | 0 {themes => css}/index.php | 0 {themes => css}/midwinter/main.css | 0 {themes => css}/modern/index.php | 0 {themes => css}/modern/main.css | 0 {themes => css}/paper-plane/index.php | 0 {themes => css}/paper-plane/main.css | 0 {themes => css}/paw/index.php | 0 {themes => css}/paw/main.css | 0 {themes => css}/sunrise/index.php | 0 {themes => css}/sunrise/main.css | 0 manifest.json | 38 ++++ setup.php | 161 +++++++++++++++- .../thold/{themes => css}/modern/main.css | 0 tests/Integration/PluginLifecycleTest.php | 27 +++ tests/Unit/PruneFilesTest.php | 173 ++++++++++++++++++ tests/Unit/SetupCspHooksTest.php | 2 +- tests/bin/validate-manifest.php | 73 ++++++++ tests/bootstrap-unit.php | 1 + 25 files changed, 485 insertions(+), 3 deletions(-) rename {themes => css}/classic/index.php (100%) rename {themes => css}/classic/main.css (100%) rename {themes => css}/dark/index.php (100%) rename {themes => css}/dark/main.css (100%) rename {themes => css}/index.php (100%) rename {themes => css}/midwinter/main.css (100%) rename {themes => css}/modern/index.php (100%) rename {themes => css}/modern/main.css (100%) rename {themes => css}/paper-plane/index.php (100%) rename {themes => css}/paper-plane/main.css (100%) rename {themes => css}/paw/index.php (100%) rename {themes => css}/paw/main.css (100%) rename {themes => css}/sunrise/index.php (100%) rename {themes => css}/sunrise/main.css (100%) create mode 100644 manifest.json rename tests/Fixtures/cacti-root/plugins/thold/{themes => css}/modern/main.css (100%) 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 275e0615..26b38e22 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -180,3 +180,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_thold_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 9834da12..0998da03 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/thold/tests/bin/validate-manifest.php + - name: Run apt-get update run: sudo apt-get update diff --git a/README.md b/README.md index a2676a09..7642c599 100644 --- a/README.md +++ b/README.md @@ -129,6 +129,12 @@ thresholds, note that you must modify and install the thold_daemon.service file into your systemd configuration, and then start and test the service. If you fail to perform these steps, thold will appear to not work as expected. +When upgrading the plugin while the Threshold Daemon is running, restart the +`thold_daemon` service (for example `systemctl restart thold_daemon`) after +the upgrade so the long-lived process 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. + Lastly, please note that several forks of the thold plugin are available from different sources. These forks of thold are not necessarily compatible with the current version of Cacti's thold plugin. Please be aware of this when diff --git a/themes/classic/index.php b/css/classic/index.php similarity index 100% rename from themes/classic/index.php rename to css/classic/index.php diff --git a/themes/classic/main.css b/css/classic/main.css similarity index 100% rename from themes/classic/main.css rename to css/classic/main.css diff --git a/themes/dark/index.php b/css/dark/index.php similarity index 100% rename from themes/dark/index.php rename to css/dark/index.php diff --git a/themes/dark/main.css b/css/dark/main.css similarity index 100% rename from themes/dark/main.css rename to css/dark/main.css diff --git a/themes/index.php b/css/index.php similarity index 100% rename from themes/index.php rename to css/index.php diff --git a/themes/midwinter/main.css b/css/midwinter/main.css similarity index 100% rename from themes/midwinter/main.css rename to css/midwinter/main.css diff --git a/themes/modern/index.php b/css/modern/index.php similarity index 100% rename from themes/modern/index.php rename to css/modern/index.php diff --git a/themes/modern/main.css b/css/modern/main.css similarity index 100% rename from themes/modern/main.css rename to css/modern/main.css diff --git a/themes/paper-plane/index.php b/css/paper-plane/index.php similarity index 100% rename from themes/paper-plane/index.php rename to css/paper-plane/index.php diff --git a/themes/paper-plane/main.css b/css/paper-plane/main.css similarity index 100% rename from themes/paper-plane/main.css rename to css/paper-plane/main.css diff --git a/themes/paw/index.php b/css/paw/index.php similarity index 100% rename from themes/paw/index.php rename to css/paw/index.php diff --git a/themes/paw/main.css b/css/paw/main.css similarity index 100% rename from themes/paw/main.css rename to css/paw/main.css diff --git a/themes/sunrise/index.php b/css/sunrise/index.php similarity index 100% rename from themes/sunrise/index.php rename to css/sunrise/index.php diff --git a/themes/sunrise/main.css b/css/sunrise/main.css similarity index 100% rename from themes/sunrise/main.css rename to css/sunrise/main.css diff --git a/manifest.json b/manifest.json new file mode 100644 index 00000000..3005d3f8 --- /dev/null +++ b/manifest.json @@ -0,0 +1,38 @@ +{ + "tombstones": [ + "themes/" + ], + "expected": [ + ".mdl_style.rb", + ".mdlrc", + "CACTI-THOLD-MIB", + "CHANGELOG.md", + "INFO", + "LICENSE", + "README.md", + "cli_import.php", + "cli_thresholds.php", + "css/", + "extras/", + "images/", + "includes/", + "index.php", + "locales/", + "manifest.json", + "notify_lists.php", + "notify_queue.php", + "phpunit.xml", + "poller_thold.php", + "service/", + "setup.php", + "thold.php", + "thold_daemon.php", + "thold_functions.php", + "thold_graph.php", + "thold_notify.php", + "thold_process.php", + "thold_templates.php", + "thold_webapi.php" + ], + "whitelist": [] +} diff --git a/setup.php b/setup.php index e197d49c..8806772d 100644 --- a/setup.php +++ b/setup.php @@ -211,6 +211,7 @@ function plugin_thold_upgrade() { if ($current != $old) { plugin_thold_install(true); + plugin_thold_prune_files(); } return true; @@ -1603,8 +1604,8 @@ function thold_page_head() { // This hook can fire on core pages that have not loaded thold_functions.php, where plugin_thold_csp_nonce() lives. require_once(__DIR__ . '/thold_functions.php'); - if (file_exists($config['base_path'] . '/plugins/thold/themes/' . get_selected_theme() . '/main.css')) { - print get_md5_include_css('plugins/thold/themes/' . get_selected_theme() . '/main.css'); + if (file_exists($config['base_path'] . '/plugins/thold/css/' . get_selected_theme() . '/main.css')) { + print get_md5_include_css('plugins/thold/css/' . get_selected_theme() . '/main.css'); } ?> @@ -2239,3 +2240,159 @@ function thold_settings_bottom() { toBeTrue(); expect(CactiStubs::callsTo('api_plugin_register_hook'))->toBeEmpty(); }); + +it('prunes bundled dev-only files on a version-drift upgrade', function () { + CactiStubs::willReturn('get_current_page', 'thold.php'); + CactiStubs::willReturnFor('db_fetch_cell', 'plugin_config', '0.0.0'); + + // Force plugin_thold_install(true) to short-circuit immediately (its + // version_compare guard) so this exercises the drift branch + prune call + // without the full reinstall, which needs a live Cacti include/database.php. + $restoreCacti = $GLOBALS['config']['cacti_version']; + $GLOBALS['config']['cacti_version'] = '1.1'; + + // Sandbox base_path so plugin_thold_prune_files() runs against a throwaway + // tree with a copy of the real INFO (so plugin_thold_version() still + // matches) and no manifest.json (prune no-ops), never the real checkout. + $restoreBase = $GLOBALS['config']['base_path']; + $base = sys_get_temp_dir() . '/thold-itest-' . uniqid(); + mkdir($base . '/plugins/thold', 0777, true); + copy(dirname(__DIR__, 2) . '/INFO', $base . '/plugins/thold/INFO'); + $GLOBALS['config']['base_path'] = $base; + + try { + expect(plugin_thold_upgrade())->toBeTrue(); + } finally { + $GLOBALS['config']['base_path'] = $restoreBase; + $GLOBALS['config']['cacti_version'] = $restoreCacti; + } +}); diff --git a/tests/Unit/PruneFilesTest.php b/tests/Unit/PruneFilesTest.php new file mode 100644 index 00000000..880304d3 --- /dev/null +++ b/tests/Unit/PruneFilesTest.php @@ -0,0 +1,173 @@ + ['include/', 'oldfile.php', 'userdata/', 'gone.png'], + 'expected' => ['INFO', 'setup.php', 'includes/', 'manifest.json'], + 'whitelist' => ['userdata/'], + ]; + + $base = thold_prune_fixture($manifest); + $plugin = $base . '/plugins/thold'; + $restore = $GLOBALS['config']['base_path']; + + $GLOBALS['config']['base_path'] = $base; + + try { + plugin_thold_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() . '/thold-prune-missing-' . uniqid(); + mkdir($base . '/plugins/thold', 0777, true); + $restore = $GLOBALS['config']['base_path']; + + $GLOBALS['config']['base_path'] = $base; + + try { + plugin_thold_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() . '/thold-prune-bad-' . uniqid(); + $plugin = $base . '/plugins/thold'; + 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_thold_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 = thold_prune_fixture($manifest); + $plugin = $base . '/plugins/thold'; + $outside = $base . '/plugins/escapee.txt'; + file_put_contents($outside, 'precious user data'); + $restore = $GLOBALS['config']['base_path']; + + $GLOBALS['config']['base_path'] = $base; + + try { + plugin_thold_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 = thold_prune_fixture($manifest); + $plugin = $base . '/plugins/thold'; + 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_thold_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/SetupCspHooksTest.php b/tests/Unit/SetupCspHooksTest.php index dd0a99c3..89bc0d27 100644 --- a/tests/Unit/SetupCspHooksTest.php +++ b/tests/Unit/SetupCspHooksTest.php @@ -51,7 +51,7 @@ public function test_page_head_emits_theme_css_and_a_nonced_inline_script(): voi thold_page_head(); $output = ob_get_clean(); - $this->assertStringContainsString('themes/modern/main.css', $output); + $this->assertStringContainsString('css/modern/main.css', $output); $this->assertStringContainsString("