3 Commits

Include renames

Author	SHA1	Message	Date
TheWitnessandCopilot	9b217e0e3a	INFO: set compat to 1.2.29 (#838) ... * INFO: set compat to 1.2.32 * docs: update Copilot instructions Cacti baseline to 1.2.32 (matches INFO compat) * INFO: back off compat to 1.2.29 * Add manifest.json + upgrade-time file pruning * 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. * Move thold_functions.php and thold_webapi.php into includes/; flatten css The main threshold library becomes includes/functions.php and the web API helper becomes includes/webapi.php; every caller (setup.php, includes/*, the CLI/poller/notify entry points and the test suite) loads them from includes/. Each per-theme stylesheet css/<theme>/main.css becomes css/<theme>.css so users can drop their own css/<theme>.css into the css base, and the page_head hook looks there. Old paths are tombstoned so existing installs drop the stale copies on upgrade. * Regenerate cacti.pot for the includes/ file relocations Moving the plugin's library files into includes/ changed the source-reference lines in the translation template; regenerate it so the 'Verify translation template' CI check passes. * Harden the hook-include test and make RPN integer ops explicit The HookFunctionsIncludeTest invokes real hooks with placeholder input purely to execute their relocated include_once lines; it now swallows engine notices/warnings/deprecations during those calls (and restores the error handler and config afterward) so it cannot trip the suite's failOnDeprecation or leak state into other tests. The RPN % and ^ operators now cast their operands to int explicitly, matching their long-standing integer-truncation behaviour while silencing the implicit float-to-int conversion deprecation. * 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. * docs: update and align the Project Structure block Point the structure tree at the relocated files (libraries now under includes/, data files under docs/, stylesheets under css/) and align the trailing '# ...' comments to a single column so they no longer drift right. * 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. * refactor: rename prune helpers to the plugin-slug function prefix The repository naming contract reserves the plugin_<name>_ prefix for plugin lifecycle / hook-registration functions; all other functions use the plain <name>_ prefix. Rename the internal upgrade helpers accordingly: plugin_<name>_prune_files() -> <name>_prune_files() plugin_<name>_rmtree() -> <name>_rmtree() The call site, unit tests, and the copilot-instructions.md references are updated to match. No behavioral change. * chore: set the Cacti compatibility floor to 1.2.29 Align the compat metadata (and the contributor-guide references) with the team decision to standardize the minimum supported Cacti version at 1.2.29; the previously proposed 1.2.32 floor was not adopted. --------- Co-authored-by: Copilot <copilot@users.noreply.github.com>	2026-10-01 01:40:00 -05:00
3 people	dac4a8684e	security: replace eval() in RPN unary math functions with safe dispatch (GHSA-4mmp-mv2x-m9f6) (#808) ... * security: replace eval() in RPN unary math functions with safe dispatch (S1523) Replace the eval()-based unary math function evaluation at line 403 with a new thold_rpn_math_unary() helper function that uses a switch statement to dispatch SIN, COS, TAN, ATAN, SQRT, FLOOR, CEIL, DEG2RAD, RAD2DEG, ABS, EXP, LOG to their native PHP equivalents. The operator was already whitelisted via in_array(, , true) and the operand validated with is_numeric(), but removing eval() eliminates the code injection risk entirely (CWE-95). Addresses SonarQube finding: php:S1523 at thold_functions.php:403 * ci: retry transient dependency downloads * test(rpn): cover safe unary dispatch * fix: restore SQL binding, trigger-command quoting, and RPN safety fixes Same set of pre-existing gaps as develop (covered by tests but never implemented in thold_functions.php): - get_allowed_thresholds()/get_allowed_threshold_logs(): bind graph_id and caller-supplied params instead of interpolating them into the SQL text, and use the *_prepared fetchers. - thold_replace_threshold_tags(): add a \ flag that quotes device/threshold free-text values with cacti_escapeshellarg() when the substituted text is a trigger command. - thold_command_execution(): pass shell=true for the three trigger commands, and fix a topic-string typo ('thold' vs 'thold_cmd') that silently suppressed exit-status logging for inline command runs. - thold_expression_math_rpn(): remove a stray break that exited the operator switch before pushing the 0/0 result, flag modulo-by-zero instead of letting PHP throw DivisionByZeroError, and reject non-numeric binary operands the same way for '%' as for '/' (binary math here is still eval()-based; unrelated to this PR's unary-math dispatcher). - thold_rpn_math_unary()/thold_expression_math_rpn(): reject non-numeric operands before dispatching, and flag SQRT/LOG results that are NAN or INF instead of pushing them onto the stack. * fix(tests): stop includes/arrays.php loading from silently no-op'ing thold_functions.php includes includes/arrays.php with a plain include() (not include_once()) from several of its own functions (e.g. thold_log()), keyed off \['base_path']. Once any test in the shared Pest process exercised one of those call sites, PHP's include-once registry considered the file already included: a later thold_test_load() (require_once) on the same resolved path silently no-op'd and never (re)published \ to \, depending on which order the test files happened to run in. This intermittently broke every test that reads \ (TholdReplaceThresholdTagsTest's THOLDTYPE substitution) as well as tests several call frames downstream of thold_log()/thold_check_threshold() (NotificationEmailDeduplicationTest, ThresholdTimeBasedCharacterizationTest), depending on file execution order. Add thold_test_load_always()/loadPluginSourceAlways(), which use a plain require() instead of require_once(), for plugin files that only assign file-scope variables (no function/class declarations) and are therefore safe to load more than once. Switch the four test classes that load includes/arrays.php to the new helper. --------- Co-authored-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net>	2026-09-20 11:04:20 -04:00
Thomas Vincent	4dd0f87f24	refactor(thold): lift the evaluation settings out of the threshold check (#800) ... * test: pin the hi/low threshold evaluator's behaviour thold_check_threshold() has no return value: everything it decides is a side effect through seven global Cacti functions. ThresholdScenario builds the fixture those need and runs one poll; ThresholdOutcome reads back what was emitted, so a test asserts on behaviour rather than on the SQL text. No production file is touched. Several assertions record behaviour that is wrong rather than intended, each with a comment saying so, so that fixing it later is a deliberate edit here. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: pin the time-based threshold evaluator's behaviour Records where this arm has drifted from the hi/low one, notably that a restoral writes the log row and clears the state but sends no mail, so an operator sees the alert and never the all-clear. Cacti's cell fetchers return false rather than '' when a query matches no row. The stub now does the same: on PHP 8 the difference is a TypeError in the re-alert arithmetic, so the old default invented a failure production does not have. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: follow Cacti's test layout and composer scripts Adopts the conventions from Cacti core: tests/bootstrap-unit.php, tests/Helpers for the stubs, a phpunit.xml carrying error_reporting -1 and CACTI_TEST_BOOTSTRAP, and composer lint / test / test:coverage scripts so CI runs the same commands a developer does. The dev toolchain is Cacti's, pinned to the same platform php 8.1.0. Cacti core runs Pest and this suite does not, because pest ^2 does not resolve on PHP 8.1 -- the platform Cacti's own composer.json pins. Releases up to v2.36.0 conflict with phpunit 10.5.62 and later, every earlier 10.x release is blocked by advisory PKSA-z3gr-8qht-p93v, and v2.36.1, which does resolve, requires PHP 8.2. The stack installs on 8.2 and above; 8.1 is the floor this plugin's CI matrix targets. The tests are written in the plain PHPUnit class style that Cacti's tests/Pest.php explicitly supports, so they run unchanged under Pest wherever it is installable. * test: pin the baseline threshold evaluator's behaviour This arm compares the reading against statistics rrdtool reports for a reference window, so the scenario supplies those rather than static bounds. Reaching them means answering the three rrdtool calls thold makes -- file existence, an info block describing the data sources and consolidation functions, and a graph command whose printed values are decoded by position -- which the helper now does. Completes the three arms, so Phase 2 can start moving code. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * perf(thold): drop eight write-only locals from the threshold evaluator Each was assigned and never read. One of them, $baseu, ran a query against graph_templates_graph and discarded the result, so every threshold on every poll cycle paid for a round trip that fed nothing. $show_datasource likewise called thold_datasource_required() for a value nobody used. The others are $global_alert_address, $deadnotify (issue #751), $suffix, $show_units, $units_suffix and $decimals. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * refactor(thold): lift the evaluation settings out of the threshold check The preamble computed the settings the three threshold arms read, so reaching any of that logic meant driving a whole poll. It moves to thold_evaluation_context(), which derives everything from the threshold row and the Cacti settings and decides nothing, and the check unpacks what it needs. Pure motion. The characterization tests from the previous commits pass unchanged, which is what they were written for, and the rules that were buried in the preamble now have direct tests: trigger fallback, whether alerts also reach the warning recipients, and when a graph is attached. thold_check_threshold() goes from 1432 lines to 1378. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: load branch helpers without plugin Composer autoload * ci: keep plugin PR integration checks on pinned Cacti * test: tighten characterization fixture metadata and permissions * test: match the optional Cacti database stub signature * ci: bound package index refreshes --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>	2026-08-17 19:13:24 -07:00