5 Commits
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
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 860be297c1 fix(notify): recover unfinished queue claims (#813)
* fix: recover unfinished notification claims

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: cover notification claim recovery branches

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: harden notification worker recovery

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: heartbeat notification queue loops

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: fail closed on notification worker recovery

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: bound notification worker recovery

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* style: format notification CLI validation

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: prove notification claim ownership at completion

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: bound notification completion failures

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: recover unverifiable stale workers

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: keep disabled exec path measurable

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: verify stale worker age without wall clocks

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: fail closed on unverifiable live workers

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: cover portable process age fallback

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: reject invalid process age output

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* Retry failed queued email notifications (#811)

* Retry failed queued email notifications

* test: cover queued delivery retry paths

* style: document notification fixture parameters

* fix: batch grouped notification delivery updates

* fix: align notification retry status output

* refactor: centralize notification retry updates

* fix: retain retry queue ownership

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-09-19 21:07:29 -04:00
Thomas Vincent 5799446aee fix(thold): make the unit suffix conversions inverses of each other (#802)
* fix(thold): make the unit suffix conversions inverses of each other

thold_display_to_raw() and thold_raw_to_display() each carried their own
suffix table and disagreed. Entering 'p' scaled by 1e-9, while a value of
1e-12 rendered as 'f' and so parsed back as 1e-15: opening a threshold and
saving it again divided its bound by a thousand, every time.

Nano was missing from both, which is what pushed the rest of the small
suffixes one place out of step. Past the largest and smallest entries the
render read off the end of its pattern string and dropped the magnitude
entirely, so 5e-18 displayed as 5.

Both now read one shared table, so they cannot drift apart again, and a value
outside the table keeps its scale rather than losing it.

Stored bounds are untouched. What changes is the label they display under and
the meaning of a newly typed suffix: 'p' is pico, as it always claimed to be.

Refs #786

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* ci: keep plugin PR integration checks on pinned Cacti

* test: cover every suffix and isolate session state

* ci: bound package index refreshes

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-08-30 14:10:00 -07:00
Thomas Vincent 4edd1331eb fix(notify): drain only the notifications this run claimed (#804) 2026-08-17 13:01:03 -07:00