* issue: shorten thold VRULE graph drilldown button title
The graph action menu now renders each drilldown button's title as a visible label, so 'Toggle Threshold VRULES %s' reads as an overlong menu entry. Shorten it to 'Toggle VRULES %s', add a unit test exercising thold_graph_button()'s VRULE branch (with the two missing request-var stubs it needs) so the changed line stays covered, and regenerate locales/po/cacti.pot.
* Update copyright notice in TholdGraphButtonTest.php
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix Last-Translator entry in cacti.pot
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* test: cover the VRULEs-enabled 'Off' title branch for patch coverage
---------
Co-authored-by: TheWitness <1439914+TheWitness@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The status pills were being clipped to a fixed width because the single-class theme rules (e.g. .deviceUpFull width:14%, padding:3px) outranked the equal-specificity .thold-pill shape rule, so long labels such as 'Not Monitored' overflowed the coloured pill on the Device Status page. Element-qualify the selector (span.thold-pill) so the shape rule wins and the pill hugs its label in every theme.
Also make a second click on an already-selected status pill clear that filter (reset the Status select to All, value -1) instead of re-applying the same value. Applied to all three thold_graph.php sub-tabs (Thresholds, Device Status, Threshold Log) and the matching Thresholds console page (thold.php).
* Add clickable status pills + zebra rows to Device Status and Threshold Log pages
Mirror the Thresholds-page pill treatment on the two remaining thold_graph.php sub-views: the Device Status (hoststat) list now renders each device's status as a clickable pill and zebra-stripes its rows instead of colouring the whole row, and the Threshold Log gains a clickable Status pill column right after Type (rows zebra-striped). Clicking a pill sets the page's Status filter and auto-applies. Regenerated cacti.pot for the new Log Status header.
* Make Device Status pill filters match displayed states
Address review feedback on PR #853: the clicked status pills did not consistently filter to the state shown on the device row.
- Map the Error pill to an exact Error filter (host_status=4) instead of the aggregate Not Up (-4), and add an Error option to the Status dropdown.
- Rework the host_status WHERE clause so each state reuses the same failure-count override classification as thold_device_status_pill(): the override now uses >= (matching the pill, was >) and only reclassifies hosts to Down, so the Down filter includes overridden hosts and the Up/Unknown/Error filters exclude them.
* bug: pass bool (not int) to api_plugin_register_hook() enable argument
Cacti 1.3 types api_plugin_register_hook() 5th arg as bool enable. These calls passed int 1, which only coerces while the file lacks declare(strict_types=1); pass true so they stay type-correct. php -l clean.
* Exclude live-DB reinstall body of plugin_thold_install() from coverage
* bug: pass bool true (not string '1') at remaining register-hook upgrade sites
Convert the 17 other api_plugin_register_hook() 5th arguments in includes/database.php from the string '1' to boolean true so the upgrade/re-registration paths are strict_types-safe against Cacti develop's bool \ signature.
* thold: status pills with sortable/clickable Status column and zebra rows
Replace the whole-row status colour on the Thresholds (thold.php) and Device Status (thold_graph.php) pages with a rounded status pill in a new Status column right of Name; rows now fall back to zebra striping. The column is server-side sortable by severity via a new thold_severity_sql() CASE (a faithful SQL mirror of get_thold_severity(), legend-order rank 1=Alert..7=Disabled), the Status filter gains the seven per-severity options (values 101-107 handled in thold_get_state_filter()), and clicking a pill sets that filter and reloads. Pill shape lives in a theme-agnostic css/pills.css; all colours stay theme-aware (reusing each theme's .thold<State> backgrounds, with light pill text added to the saturated classic/modern/paw themes). Adds Pest coverage for thold_severity_sql() and the severity filter branch.
* thold: regenerate cacti.pot for the status-pill i18n strings
* thold: address review - pill contrast, time_warning_low '0' parity, keyboard a11y, stronger test
Light themes (classic/modern/paw all have white/snow backgrounds) now use dark pill text instead of white, which failed 4.5:1 at 11px. thold_severity_sql() treats time_warning_low '0' as falsy (NOT IN ('','0')) to match get_thold_severity(), so the pill never disagrees with sort/filter. Pills are now focusable role=button spans with Enter/Space activation on both pages. TholdSeveritySqlTest now executes the CASE over an in-memory SQLite row and asserts the SQL rank equals get_thold_severity() across branch/boundary scenarios incl. the '0' case.
* security: move filter inline onChange/onClick handlers into ready() for CSP
The threshold, notification list, notification queue, template and
device/log status filter forms still carried inline onChange/onClick
attributes on their selects and Clear/Go/Import/Export controls. Under
Cacti's Content-Security-Policy these trip the script-src-attr directive
(inline event handlers). Bind them in each view's existing
$(function(){...}) ready block instead.
Scope: thold.php, notify_queue.php, thold_templates.php, notify_lists.php,
thold_graph.php (all gate-allowlisted UI files; no measured source changed).
The cactiReturnTo() cancel buttons on the bulk-action confirmation pages are
left as-is (shared Cacti-core pattern). Regenerated locales/po/cacti.pot for
the shifted source line references.
* security: convert cactiReturnTo cancel buttons to the CSP-safe class
The confirmation pages' Cancel buttons used an inline
onClick='cactiReturnTo()' handler, which trips Cacti's Content-Security-Policy
script-src-attr directive. Switch them to the CSP-safe cactiReturnTo CSS
class (class='cactiReturnTo', plus an optional data-url when a target is
passed).
Cacti 1.2.31 and later bind this class automatically. The README documents a
one-time include/layout.js applySkin() snippet for operators still on an
earlier release - no shim is baked into the core or the plugin.
* fix: give Thresholds/Devices filter Clear buttons the id the ready() handler binds
The hosts() and tholds() filter views used name='clear' on the Clear button, but the ready() handler binds to #clear, so Clear was a no-op on those two views. Switch them to id='clear' to match the other views and the handler.
---------
Co-authored-by: TheWitness <larry@thewitness.org>
* thold: keep legend chips equal width using the longest label
The Thold/Host/Log status legends used flex:1 1 0 with no shared minimum, so chips grew to unequal widths (and could clip their labels when tight). CSS can't measure the widest sibling, so the three legend renderers now compute the longest label and emit it as a --thold-chip-min CSS variable (calc(<longest>ch + 1.5rem)); every theme's .tholdLegendItem uses it as a shared min-width (falling back to max-content). All chips share one minimum, so they stay equal width as the legend wraps. Updated TholdLegendTest assertions to match the new container markup.
* thold: use CSS Grid for truly equal-width legend chips; assert exact chip min
Review follow-up (#847). The flex: 1 1 0 + flex-wrap layout distributes free space per wrapped line, so a partial last row stretched its chips wider than full rows - chips were not equal across rows. Switch .tholdLegend to a grid (repeat(auto-fill, minmax(var(--thold-chip-min), 1fr))) so every chip shares an equal column across all rows. The longest-label seed still drives the column minimum. Tests now assert the exact computed calc(Nch + 1.5rem) value instead of mere presence.
* thold: use auto-fit so chips fill the legend width when they fit on one row
Review follow-up: auto-fill keeps empty implicit tracks, so on a wide viewport where all chips fit in one row they no longer stretch to fill the legend (as they did under flex). auto-fit collapses the empty tracks while still sharing equal columns when the legend wraps to multiple rows.
* thold: escape status-legend chip labels at output
Defence-in-depth: wrap the legend chip display text in html_escape() so markup or HTML entities in a translated label cannot alter the legend DOM. Applied consistently across the thold/monitor/servcheck/mactrack/syslog fleet legends. The chip-min sizing still measures the unescaped label length, so layout is unchanged.
COMPOSER_ROOT_VERSION only influences Composer root-version detection, which
does not run in the "Run Pest Unit Tests" step (Composer install happens in
an earlier step). Removing it from that step's env; no behaviour change.
Part of harmonizing the patch-coverage CI convention across the plugin fleet.
Co-authored-by: TheWitness <copilot@users.noreply.github.com>
The monitored branch renders the Status column with 'center' alignment, but the not-monitored branch passed 'right' to form_selectable_cell(), so 'Not Monitored' appeared right-aligned. Use 'center' to match.
* thold: define plugin_thold_csp_nonce() in thold_functions.php
Fixes a fatal 'Call to undefined function plugin_thold_csp_nonce()' in
thold.php (and the other plugin pages) when they load without setup.php
(e.g. after the plugin is disabled by an error). The helper lived only in
setup.php, but every plugin page includes thold_functions.php - not
setup.php - so move the definition there. setup.php now require_once's
thold_functions.php so its page_head/device/settings hook callbacks, which
also run on core Cacti pages, keep resolving the helper.
* thold: scope-load thold_functions.php in the nonce hooks, not at setup.php top
Address review: don't require the full ~11k-line thold_functions.php at
setup.php top level (it loaded on every core page via page_head and during
CLI/poller). Instead each nonce-emitting hook (page_head, host/device/
template edit, settings) require_once()'s it when it actually fires;
require_once dedupes so the first hook on a page loads it once. Point
CspNonceTest at thold_functions.php directly so it guards the inclusion-
path regression.
UNIX_TIMESTAMP() AS current_timestamp used current_timestamp, a MariaDB/MySQL reserved word, as a bare column alias in thold_notification_register_process(). MariaDB rejected it with a syntax error, so db_fetch_row_prepared() failed and notification worker registration never completed, emitting a CMDPHP SQL backtrace every run. Backtick-quote the alias; the result key stays 'current_timestamp' so no other code changes.
* thold: restyle status legends as rounded, spaced, solid-color chips (servcheck-style)
* thold: let legend chips keep intrinsic width so the flex row wraps; add CHANGELOG entry
* thold: add tests covering the restyled status legend renderers
Covers thold_legend(), host_legend() and log_legend() so the changed lines meet the 100% patch-coverage gate. Each test loads includes/arrays.php for the state globals, captures the rendered output, and asserts one .tholdLegendItem chip per state with its class and label.
* security: replace rand() with hrtime(true) for graph cache-buster (S2245)
Replaces the non-cryptographic PRNG rand() with hrtime(true), a
monotonic nanosecond clock that never throws. This URL parameter is
a cache-buster, not a security boundary, so a CSPRNG (random_int)
is the wrong reliability tradeoff — it can throw Random\RandomException
on CSPRNG failure, making threshold editing fatal.
hrtime(true) provides:
- Never throws (no CSPRNG dependency)
- Monotonic (never goes backwards on NTP step)
- Nanosecond resolution (no collisions on fast page renders)
- Returns int (clean URL query parameter)
Fixes GHSA-vhwj-hfwg-gfg3
Rule: php:S2245 (CWE-338)
* fix: restore SQL binding, trigger-command quoting, and RPN safety fixes
These security/robustness fixes were already documented in CHANGELOG.md and
covered by tests (GetAllowedThresholdsTest, TholdCommandExecutionTest,
TholdReplaceThresholdTagsTest, TholdExpressionMathRpnTest,
OptionalCoreFunctionTest), but the corresponding thold_functions.php changes
were missing, leaving the Pest suite failing on every PR built on develop.
- 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; email/notification callers are
unaffected.
- thold_command_execution(): pass shell=true when building the three
trigger commands, and fix a topic-string typo ('thold' vs 'thold_cmd')
that silently suppressed exit-status logging for inline (non-queued)
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, reject non-numeric
unary operands before eval(), 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.
* debug: temporary CI diagnostics for notification/restoral test failures
Adds stderr diagnostics behind THOLD_CI_DEBUG to see the real values
computed on the Linux Pest CI run for the two failures that do not
reproduce locally on Windows. Will be removed once the root cause is
identified.
* debug: make CI diagnostics fire unconditionally for this run
* Revert "debug: make CI diagnostics fire unconditionally for this run"
This reverts commit 71877471e7.
* Revert "debug: temporary CI diagnostics for notification/restoral test failures"
This reverts commit 76d05ad300.
* debug: temporary CI diagnostics (round 2)
* chore: retrigger CI
* Revert "debug: temporary CI diagnostics (round 2)"
This reverts commit 7ce80b2d1c.
* debug: temporary CI diagnostics (round 3)
* chore: retrigger CI for diagnostics
* Revert "debug: temporary CI diagnostics (round 3)"
This reverts commit 9aff7a428e.
* debug: temporary CI diagnostics (round 4)
* chore: retrigger CI (2)
* Revert "debug: temporary CI diagnostics (round 4)"
This reverts commit 4bb7f44ecf.
* debug: temporary CI diagnostics (round 5)
* chore: retrigger CI (3)
---------
Co-authored-by: TheWitness <thewitness@cacti.net>
* fix: preserve counter sample time pairs
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: preserve samples in daemon and poller paths
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* test: exercise poller sample persistence
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: bound counter gap recovery
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* style: format counter recovery tests
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: treat unusable counter baselines as unknown
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: preserve unknown counter maximum semantics
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* test: cover validated poller interval fallback
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: retain trustworthy counter baselines
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: propagate unknown rate samples safely
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: validate effective counter step
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: preserve alerts across unknown rate gaps
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: preserve unknown values through transforms
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: fail closed on missing expression sources
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: keep unavailable thresholds eligible
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: preserve cached expression rates
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: deduplicate backward-clock warnings
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* test: adopt shared Cacti plugin CI workflow, add Security/Integration coverage
- Replace plugin-ci-workflow.yml with the Cacti/plugin_evidence-based template: pinned actions, CACTI/COMPOSER_ALLOW_SUPERUSER global env vars, sudo on every composer command, and an actual Pest execution step (previously missing - this workflow never ran Pest at all).
- Remove php-unit-tests.yml; its Docker-based Pest run is now redundant with plugin-ci-workflow.yml's Pest step.
- Preserve this plugin's existing, more advanced tests/bootstrap-unit.php, TestCase.php, CactiStubs helper and pinned tests/.cacti-version (1.2.31, strict match) rather than overwriting with plugin_evidence's simpler generic versions - only additive stubs (api_plugin_register_hook/realm, get_current_page, plugin_test_read_source) were appended.
- Add tests/Security (SetupStructureTest, Php82CompatibilityTest) and tests/Integration (PluginLifecycleTest covering plugin_thold_version/uninstall/upgrade gating) to round out the Security/Unit/Integration directory convention used across other plugins.
- Update phpunit.xml to run all three suites and include setup.php as covered source.
* ci: retrigger checks
* fix: address Copilot review findings
- includes/polling.php: stop manufacturing a numeric zero for an unavailable current sample when thold_consider_unknown_zero is enabled - it was bypassing thold_check_threshold()'s new unavailable-sample guard entirely. Preserve the informational NOTE/debug logging.
- thold_functions.php: thold_build_cdef() now fails closed (returns '') when a |query_...| CDEF operand cannot be resolved, instead of silently continuing arithmetic on a manufactured zero and returning a fabricated numeric result. Added TholdBuildCdefTest.php covering the literal, unresolved-operand, and resolved-operand paths.
- tests/Unit/TholdCalculateLowerUpperTest.php: updated to match thold_calculate_lower_upper()'s current fail-closed contract (returns '' when the high word is absent/out of range, previously asserted stale pass-through values) and its float return type; added coverage for a non-numeric current value and an out-of-range high word.
- .github/workflows/plugin-ci-workflow.yml: drop PHP 8.1 from the matrix (Pest 3 requires PHP >= 8.2) and add a pcov coverage driver to the setup-php step so --coverage-clover does not fail with 'No code coverage driver available'.
* ci: replace plugin-ci-workflow.yml whole-cloth from plugin_evidence
A prior merge did not cleanly bring in plugin_evidence's current CI
template, leaving this file in an inconsistent state. Replaced it
entirely with plugin_evidence's .github/workflows/plugin-ci-workflow.yml,
with only the light corrections needed for this plugin:
- evidence -> thold in checkout path, plugin_manage --plugin, .cacti-version
path, and coverage upload path
- Install System Dependencies keeps apache2 (needed by the later
Configure Apache step; evidence's own template omits it)
- Run Pest Unit Tests step drops the trailing tests/Security path arg
since thold's phpunit.xml testsuite already covers
tests/Security, tests/Unit, and tests/Integration
- Actions pinned to their latest release commit SHA
(actions/checkout, actions/upload-artifact, shivammathur/setup-php),
COMPOSER_ALLOW_SUPERUSER env var and an explicit coverage: xdebug
driver added, and sudo added to the composer validate/lint/phpcsfixer/
phpstan invocations for consistency with the rest of the workflow
* fix(tests): tolerate the moving 1.2.x/develop branch in bootstrap-unit.php
The new workflow's 'Set expected Cacti version' step writes the literal
branch name (env.CACTI, e.g. '1.2.x') into tests/.cacti-version, but this
file's version check compared it verbatim against the checked-out
cacti_version file (e.g. '1.2.32'), throwing on every run. Matches the
exception plugin_evidence's bootstrap-unit.php already uses.
* fix(ci): run composer unprivileged so Pest's vendor cache is writable
Setup Permissions chowns the checkout to www-data:runner (group-writable) before Composer runs. Running composer install as root via sudo left include/vendor owned by root, so the later non-root 'Run Pest Unit Tests' step couldn't create pestphp/pest-plugin-mutate's cache directory (mkdir(): Permission denied). Composer no longer needs sudo here: the runner user is already a member of the runner group with write access from Setup Permissions, so dropping sudo leaves vendor/ owned by runner and writable by the later Pest step. Applied the same fix to the (currently dead, CACTI==1.2.x-gated) lint/phpcsfixer/phpstan steps for consistency.
* fix(ci): restore vendor ownership for Pest instead of running composer unprivileged
Running composer without sudo failed differently: Setup Permissions chowns the whole checkout to www-data:runner before Composer runs, and composer's own file-touching (utime) on files it does not own fails even with group-write access (touch(): Utime failed: Operation not permitted) - group write only covers content changes, not mtime updates, which require file ownership. Revert to running composer as root (matching Setup Permissions/Install Cacti via CLI's existing pattern) and add an explicit 'Restore vendor ownership for Pest' step that chowns include/vendor and composer.lock to runner:runner right after Composer finishes, so the later non-root 'Run Pest Unit Tests' step can write pestphp/pest-plugin-mutate's cache directory and Pest's own result cache under vendor/.
* fix(thold): bind graph_id parameter, quote shell tag substitutions
Root-caused and fixed the actual source bugs behind ~40 of the 55 failing
Pest tests (the rest are pre-existing/unrelated or test-stub mismatches
fixed separately):
- get_allowed_thresholds()/get_allowed_threshold_logs(): $graph_id was
still interpolated directly into the WHERE clause (`gl.id=$graph_id`)
and the row/count queries used unprepared db_fetch_assoc()/
db_fetch_cell(), even though GetAllowedThresholdsTest and
OptionalCoreFunctionTest were already written expecting a bound `gl.id
= ?` placeholder via db_fetch_assoc_prepared()/db_fetch_cell_prepared().
Added a new $sql_params parameter for the caller's own WHERE-clause
bindings (appended before the graph_id placeholder), matching the
tests' expected bind order and query text exactly. No existing caller
passes $graph_id, so this is behavior-preserving for them.
- thold_replace_threshold_tags(): added the $shell parameter
TholdReplaceThresholdTagsTest/TholdCommandExecutionTest already call it
with. Device/threshold-derived values (DESCRIPTION, HOSTNAME, LOCATION,
SITE, CURRENTVALUE, NOTES, DNOTES/DEVICENOTE, EXTERNALID, THRESHOLDNAME)
are now escapeshellarg()'d when $shell is true; email/HTML callers are
unaffected. thold_command_execution() now passes $shell=true when
building the exec()/queued command for all three breach directions -
the exec() call sites already carried a comment claiming
"cacti_escapeshellarg protection" that did not actually exist.
- NotificationQueueClaimTest: cacti_log() calls are now recorded into
CactiStubs::$calls (added elsewhere in this PR, needed by
TholdGetCurrentvalTest), which added an extra 'cacti_log' entry to 3
assertions that expected an exact list/emptiness of $calls. Updated the
3 expectations to include it.
- NotificationEmailDeduplicationTest: stubbed the wrong function
(db_fetch_assoc instead of db_fetch_assoc_prepared, which is what
process_device_notifications() actually calls), so the fixture record
was never returned and no mail was ever built. Fixed the stub name; the
sha256 dedup path it asserts on was already implemented correctly.
Verified locally against a real Cacti 1.2.x checkout with a Windows PHP
CLI (GetAllowedThresholdsTest, OptionalCoreFunctionTest,
NotificationEmailDeduplicationTest, TholdReplaceThresholdTagsTest all
green; TholdCommandExecutionTest and 3 NotificationQueueClaimTest cases
only fail locally on Windows due to escapeshellarg()'s CRT-quoting
convention and missing /bin/*, posix_*, LOG_LOCAL0 - all Linux-only,
matching the actual CI environment).
* fix(thold): correct command-output log topic, harden THOLDTYPE test isolation
- thold_command_execution() called thold_process_command_output() with
topic 'thold' but the function's switch only matches 'thold_cmd'/
'thold_dhost_cmd'/'thold_uhost_cmd', so cacti_log() was silently never
called for inline (non-queued) command execution. Fixed the topic
string, restoring the exit-status/output logging
TholdCommandExecutionTest's inline-execution tests check for.
- TholdReplaceThresholdTagsTest::setUpBeforeClass() loaded
includes/arrays.php via the shared require_once-based test loader,
which is a no-op if an earlier test class in the same run already
loaded the same real path, so \ is not guaranteed to reach
this class's global scope. Force a fresh include as a fallback so
<THOLDTYPE> substitution is not dependent on suite run order.
* fix(thold): fix RPN evaluator crashes and silent bad-result pushes
- The v1==0 && v2==0 && operator=='/' special case (0/0 -> 0) had a stray
break that exited the switch before the array_push() below it ran, so
the special-cased result was popped off but never pushed back, leaving
the stack empty instead of [0].
- The zero-divisor guard only covered '/', so '%' with a zero divisor fell
through to eval('... % 0'), which throws an uncaught DivisionByZeroError
on PHP 8 instead of flagging rpn_error. Extended the guard to also cover
modulo.
- Unary functions (SQRT, LOG, etc.) evaled their operand without checking
it was numeric first, so a non-numeric operand (e.g. the 'U'/unknown
sentinel) produced an undefined-constant Error instead of flagging
rpn_error. Added the same is_numeric guard the binary path already had.
- Unary functions that produce a mathematically undefined result (sqrt of
a negative, log of zero/a negative) silently pushed NAN/-INF onto the
stack instead of flagging an error, which would have compared as false
against every threshold. Now flags rpn_error and drops the result
instead of pushing it.
* fix(thold): log time-based restoral even when it does not cross the retrigger threshold
thold_check_threshold()'s time-based arm (case 2) only logged a restoral
(ST_NOTIFYRS) inside the two branches that also send a notification, each
gated on the freshly re-queried failure/warning-failure counts reaching
their trigger. When a threshold had been alerting but the fresh counts
did not reach trigger again (the common case, since those counts measure
consecutive alert rows in the log rather than the prior persisted state),
execution fell into the bare final else, which reset the fail counters
but never logged the restoral or cleared thold_alert - so an operator
never saw the all-clear in the log and the threshold stayed marked as
alerting. Added a branch that logs ST_NOTIFYRS and resets thold_alert to
normal whenever the threshold was previously alerting, without sending a
notification, matching this arm's documented behavior of logging every
restoral while only mailing on retrigger.
* fix: address remaining Copilot review findings on PR #817
- get_current_value(): return missing_value instead of round()-ing a
non-numeric ('U'/'nan'/blank or fail-closed CDEF) sample, avoiding a
PHP 8 TypeError and manufactured-zero alerting.
- PluginLifecycleTest: reset CactiStubs in beforeEach so assertions on
global call history/queued returns do not depend on suite order.
- README: restore the notification-queue retry/worker-claim docs that
were dropped instead of retained alongside the new sampling section.
* security: fail-closed RPN evaluator errors; shell-escape thold_expand_string()
thold_functions.php:
- thold_calculate_expression() converted an RPN evaluation error (unbalanced
expression, unsupported token, divide/modulo-by-zero) into a numeric 0
return. 0 passes is_numeric(), so includes/polling.php's fail-closed
"unavailable sample" guard (which only catches non-numeric values) never
saw it, and the poller could alert/recover on a manufactured zero instead
of treating the reading as unavailable. Both error returns now return ''
instead, matching every other early-return failure path already in this
function.
- thold_expand_string() gained a $shell parameter (mirroring
thold_replace_threshold_tags()'s existing $q()/escapeshellarg() pattern).
It previously called Cacti core's expand_title() unconditionally, which
substitutes |host_*|/|query_*| tokens (hostname, SNMP community/password,
sysDescr/sysContact/sysLocation, polled data-query fields, etc) with no
shell escaping. Any of those values can be set by the polled device
itself, so thold_command_execution() calling thold_expand_string() after
thold_replace_threshold_tags($shell=true) reintroduced shell injection
despite the initial hardening, since expand_title() ran unescaped on the
already-quoted command string.
In $shell mode, each |host_*|/|query_*| token in the string is now
resolved in isolation through the same core substitute_host_data()/
substitute_snmp_query_data() helpers expand_title() itself uses (so the
resolved value matches), then escapeshellarg()'d before it's substituted
into the command. The plugin's own direct substitutions (data source
name/description, graph title, interface speed) and its
thold_substitute_custom_data()/thold_substitute_host_data() helpers are
now shell-aware the same way. Non-shell callers (email/HTML rendering)
are unaffected; thold_command_execution()'s three exec() paths now pass
$shell = true.
tests/Unit/TholdCalculateExpressionTest.php: updated the two existing
error-path assertions from 0 to '', and added divide-by-zero/modulo-by-zero
coverage.
tests/Unit/TholdExpandStringTest.php + tests/bootstrap-unit.php: added
substitute_host_data()/null_out_substitutions() stubs and a test asserting
a malicious |host_description| value is escapeshellarg()'d in shell mode
and left untouched otherwise.
* security: shell-escape the <URL> tag in trigger commands
The <a href='...'>...</a> markup substituted for <URL> contains literal
single quotes. In \ mode this landed in the command unquoted, so
the quotes could terminate the shell argument early (and any
metacharacters in the configured base_url would remain executable)
despite the rest of thold_replace_threshold_tags()'s hardening. The
whole substituted value is now passed through the same \() helper as
every other tag, quoting it as one token in shell mode while leaving
email/HTML output unchanged. Added a regression test.
* fix: address remaining Copilot review findings on PR #817 (round 2)
- Guard modulo against fractional zero divisors: the % operator truncates
both operands to int, so a divisor like 0.5 became 0 and reached eval(),
throwing an uncaught DivisionByZeroError instead of failing closed.
- Shell-escape <DSNAME> in thold_replace_threshold_tags(); every other
substituted value already went through the $q() helper in shell mode.
- Apply the same lastchanged/acknowledgment restoral side effects in the
early-restoral branch of thold_check_threshold() as the two full restoral
branches above it, so an acknowledged time-based alert that recovers
before either failure count retriggers no longer stays acknowledged.
- Log daemon sample-persistence failures unconditionally via
thold_cacti_log() instead of the --debug-gated thold_daemon_debug(), so
sample loss is visible in normal daemon operation.
- Restore the changed-line coverage gate the shared CI workflow dropped
when it replaced the old Docker-based Pest workflow: fetch full history
for the plugin checkout and run tests/bin/patch-coverage.php against the
PR base SHA. Allowlist thold_process.php (daemon entry point, cannot be
required into the isolated unit test process).
- Add/extend unit tests covering each of the above.
* fix(ci): measure includes/polling.php for changed-line coverage
phpunit.xml's <source><include> only listed thold_functions.php and
setup.php, so Clover never recorded hits for includes/polling.php even
though it has no top-level side effects and is already require()'d
directly by PollerSchedulingTest. patch-coverage.php then correctly
flagged this PR's changes to thold_poller_output() (the new
thold_polling_sample_row()/status-row insert path, rrd_heartbeat column)
as changed-but-unmeasured production lines and failed the build.
Add includes/polling.php to the source include list, and add tests
exercising thold_poller_output()'s non-daemon path for both outcomes:
a numeric reading persisted as a sample_row, and an unavailable reading
persisted as a status-only row, plus the post-write cleanup call.
* test: cover remaining changed lines flagged by patch-coverage
Adds/extends unit tests for lines the coverage gate flagged as changed
but unexercised:
- includes/polling.php: the thold_consider_unknown_zero "would have
been zero" debug/log branch in thold_poller_output()'s non-daemon
path (still preserves the unavailable state either way).
- thold_functions.php: thold_substitute_host_data()'s cache-hit and
build-fresh (|host_management_ip|) paths, and
thold_substitute_custom_data()'s shell-escaped custom value - neither
had any direct test coverage before. New TholdSubstituteHostDataTest.
- thold_functions.php: thold_expand_string()'s |query_ifSpeed| fallback,
the SNMP-query-resolved host/query token branch in $shell mode, and
the top-level |host_management_ip| fallback that reads $device_id
from Cacti's real lib/variables.php (a side effect of an include()
inside the function's own scope). The last of those needs its own
one-off library_path fixture rather than the shared one, since
include_once() means the shared fixture's injected value would only
ever take effect on the first call across the whole test run.
- thold_functions.php: get_current_value()'s fail-closed return for a
non-numeric stored RRD value (matches the existing empty-value-series
contract already covered next to it).
* fix: gate sample pair advancement on interval eligibility; resolve tags before quoting; add complete headers
* test: cover rrd_heartbeat/rrd_step fallback branches in sample eligibility
* fix: re-anchor stale rate baselines and stop re-scanning substituted values
thold_sample_persistence() froze lasttime/oldvalue once a sample arrived
after a heartbeat-exceeding gap, but thold_get_currentval() already fails
that cycle's rate closed using the pair as it stood before this call, so
freezing bought no extra protection while permanently disqualifying every
later poll (whose elapsed time is measured against that same frozen
lasttime and only grows). Always re-anchor to the current sample instead;
thold_sample_interval_eligible() now only decides whether to log the
re-anchor.
thold_substitute_custom_data() and thold_expand_string() (both its
host/query token loop and its |graph_title|/|data_source_description|/
|data_source_name| substitutions) quoted each value and then applied it
with str_replace(), one token at a time. A later str_replace() call
re-scans the whole string, including text just inserted by an earlier
one, so a value that happens to contain another token's literal
placeholder could be substituted a second time outside of its own
quoting. Build a token => quoted-value map per substitution site and
apply it in a single strtr() pass instead, same as
thold_replace_threshold_tags(). Also switched thold_expand_string()'s and
thold_substitute_custom_data()'s shell-escaping to the established
cacti_escapeshellarg() wrapper instead of raw escapeshellarg(), matching
every other quoting site in this file.
Added regression coverage: a second poll recovering a real rate after a
stale-gap re-anchor, and a re-scan test for each of the three affected
substitution sites.
* fix: close cross-phase/cross-function shell re-scan and log flooding
thold_expand_string()'s per-phase strtr() passes were each individually
re-scan-safe, but a value resolved in one phase (host/query, custom data)
could still contain another phase's token literal text (e.g. a device
description of |data_source_name|) and get re-substituted by that later
phase, breaking out of the earlier phase's quoting. Separately,
thold_command_execution() called thold_replace_threshold_tags() (quoting
<TAG> values) and then passed the result into thold_expand_string(), which
re-scans its input for |pipe| tokens - a <TAG> value containing a literal
pipe token (e.g. a description of |host_hostname|) was expanded a second
time with the real, unescaped device value spliced inside the tag's
quoting.
Introduce thold_defer_placeholder(): every substitution that needs shell
quoting now inserts an opaque, non-token placeholder instead of its quoted
value immediately, and records the real value in a map. Thread
that map through thold_expand_string(), thold_substitute_custom_data(),
thold_substitute_host_data(), and thold_replace_threshold_tags(); each
resolves its own map internally when called standalone (preserving
existing behavior/tests), or accepts a caller-supplied map by reference to
defer resolution further. thold_command_execution() now expands pipe
tokens first, then threshold tags, sharing one deferred map across both
calls and resolving every placeholder in a single final strtr() only after
all token scanning for the command is complete.
Also stop thold_sample_persistence()'s stale-gap re-anchor warning from
firing on every poll of a persistently stale source: it now only logs on
the transition (prior lastread was still numeric), matching
thold_log_unavailable_transition()'s own transition-only pattern.
Renamed testStaleGapDoesNotAdvanceARateBearingPairButOtherCasesStillAdvance
to testStaleGapReanchorsARateBearingPairAndAllOtherCasesStillAdvance, since
the pair now always re-anchors.
Added regression coverage for the cross-phase reorder within
thold_expand_string(), the cross-function reorder in
thold_command_execution(), and the transition-only re-anchor warning.
* fix: preserve unavailable sentinel, prepared sample inserts, PHP 8.4 nullability
thold_calculate_expression()'s pipe-token branch converted an empty (failed)
thold_expand_string() result to '0' after the resolution attempt, so an
unresolved |query_*| or host token became a manufactured valid zero instead
of failing the expression closed like every other branch in this function.
Drop that fallback; an empty operand now falls through the RPN loop's
unsupported-field case and returns '' as intended.
thold_polling_sample_row() built its sample row as a raw SQL value tuple
string (id, tcheck, quoted lastread, FROM_UNIXTIME(lasttime), quoted
oldvalue) for direct concatenation into an unprepared db_execute() call,
unlike the status-only row it introduced alongside it. It now returns the
same structured fields as the status row, and includes/polling.php builds
'(?, ?, ?, FROM_UNIXTIME(?), ?)' placeholders with a flattened parameter
list for db_execute_prepared(), matching the status-only path.
rray & = null is an implicitly nullable parameter, deprecated
in PHP 8.4; phpunit.xml enables failOnDeprecation, so loading this file
could fail the PHP 8.4 CI job. Declared explicitly as ?array in all 4
functions that take it.
Added a regression test for the unresolved-pipe-token fail-closed fix, and
updated the sample-row assertions in TholdGetCurrentvalTest and
PollerSchedulingTest for the new structured/prepared-statement shape.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
* security: replace eval() in RPN math binary ops with safe dispatch (S1523)
Replace the eval()-based binary arithmetic evaluation at line 378
with a new thold_rpn_math_binary() helper function that uses a
switch statement for +, -, *, /, %, ^ operators.
The operator was already whitelisted via in_array(, , true)
and operands validated with is_numeric(), but removing eval() eliminates
the code injection risk entirely (CWE-95).
Addresses SonarQube finding: php:S1523 at thold_functions.php:378
* ci: retry transient dependency downloads
* test(rpn): cover safe binary 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), plus one bug specific to this PR:
- 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, reject non-numeric
unary operands before eval(), and flag SQRT/LOG results that are NAN or
INF instead of pushing them onto the stack (unary math here is still
eval()-based; unrelated to this PR's binary-math dispatcher).
- thold_rpn_math_binary(): this PR's new dispatcher mapped '^' to pow(),
silently changing it from the pre-existing bitwise XOR behaviour
(eval('... ^ ...') always computed XOR). Restored XOR to avoid breaking
existing user thresholds, and corrected this PR's own
TholdRpnBinaryMathTest 'power' dataset, which had asserted the
incorrect pow() behaviour.
* 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.
* debug: temporary CI diagnostics for notification/restoral failures
* Revert "debug: temporary CI diagnostics for notification/restoral failures"
This reverts commit 21fbd16030.
* debug: temporary CI diagnostics for notification/restoral failures
* test: fix restoral tests for the corrected type-2 restoral operator
Merging develop brought in #796's fix for thold_check_threshold()'s type-2
(time-based) restoral logic (`<` -> `>=` for the warning/alert fail-count
comparisons). This test harness always stubs the log-derived fail count
to 0, and the two restoral tests here relied on the default trigger of 1,
so under the corrected operator (0 >= 1 is false) neither restoral branch
fired anymore and both tests broke.
Override time_fail_trigger/time_warning_fail_trigger to 0 in both tests,
representing a threshold with no minimum streak requirement, so the
harness's stubbed zero count still satisfies the corrected `>=` check and
the tests exercise real restoral behavior again instead of the inverted
bug.
---------
Co-authored-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
* fix: normalize nullable notification text
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: preserve notification replacement type checks
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* test: pin nullable subject deprecation handling
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* 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, reject non-numeric
unary operands before eval(), 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.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
* 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>
* 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>
* security: replace md5() with sha256 for email dedup key (S4790)
Replace md5() with hash('sha256', ...) for the email grouping
deduplication key. While this usage is not security-sensitive (it's
a cache key for grouping emails by recipient list, not password
hashing or integrity verification), using a modern hash algorithm
eliminates the weak hashing finding (CWE-1240).
Addresses SonarQube finding: php:S4790 at thold_functions.php:7311
* ci: retry transient dependency downloads
* test(notification): cover sha256 deduplication
---------
Co-authored-by: Thomas Vincent <thomasvincent@gmail.com>
This is partially solved here, and also solved in mainline Cacti. It will be fixed in Cacti 1.2.31 and Cacti 1.3.
This patch includes some PSR on array structuring indentation.
* issue#673: Device up/down notification emails don't appear to be sent if the notification queue is enabled
* issue#674: Thold Current Value is incorrect in 1.8.1
* issue#678: Unable to remove suspended notifications one by one
* feature#677: Allow the Administrator to create a default Single Email Notification Subject
* feature: Better validation of Threshold values to reduce human error
* feature: Validate Threshold Template values identical to normal Threshold values
