diff --git a/.github/workflows/php-unit-tests.yml b/.github/workflows/php-unit-tests.yml deleted file mode 100644 index 43e96601..00000000 --- a/.github/workflows/php-unit-tests.yml +++ /dev/null @@ -1,127 +0,0 @@ -# +-------------------------------------------------------------------------+ -# | Copyright (C) 2004-2026 The Cacti Group | -# | | -# | This program is free software; you can redistribute it and/or | -# | modify it under the terms of the GNU General Public License | -# | as published by the Free Software Foundation; either version 2 | -# | of the License, or (at your option) any later version. | -# | | -# | This program is distributed in the hope that it will be useful, | -# | but WITHOUT ANY WARRANTY; without even the implied warranty of | -# | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | -# | GNU General Public License for more details. | -# +-------------------------------------------------------------------------+ -# | Cacti: The Complete RRDtool-based Graphing Solution | -# +-------------------------------------------------------------------------+ -# | This code is designed, written, and maintained by the Cacti Group. See | -# | about.php and/or the AUTHORS file for specific developer information. | -# +-------------------------------------------------------------------------+ -# | http://www.cacti.net/ | -# +-------------------------------------------------------------------------+ - - -name: Pest Tests - -on: - push: - branches: - - main - - develop - pull_request: - branches: - - main - - develop - -permissions: - contents: read - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -jobs: - unit-test: - name: Pest using Cacti Composer (Docker) - runs-on: ubuntu-latest - - steps: - - name: Checkout Thold Plugin - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - # patch-coverage.php diffs against the base branch. - fetch-depth: 0 - - - name: Checkout Cacti 1.2.31 runtime - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - repository: Cacti/cacti - # Commit behind the annotated release/1.2.31 tag. - ref: 1e8eaca26b84b128c39ce8cc8ece42d7ff76aac1 - path: cacti-runtime - - - name: Checkout Cacti test toolchain - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - repository: Cacti/cacti - # Pin Composer, Pest, the lock file, and Cacti's Docker test image. - ref: 298bd51eca843490fb90b27ada6b3fecc9b9a7d8 - path: cacti-toolchain - - # Build the same Linux image used by Cacti's tests/tools/docker_pest.sh. - # Composer installs and executes Cacti's locked Pest toolchain in /work; - # Thold contributes no parallel vendor directory or PHPUnit dependency. - - name: Build Cacti test image - run: | - for attempt in 1 2 3; do - if docker build --tag cacti-web --file cacti-toolchain/docker/Dockerfile cacti-toolchain/docker && \ - docker build --tag cacti-thold-test --file cacti-toolchain/docker/Dockerfile.test cacti-toolchain; then - exit 0 - fi - - if [ "$attempt" -lt 3 ]; then - sleep 10 - fi - done - - echo 'Cacti test image build failed after three attempts.' >&2 - exit 1 - - - name: Lint every PHP source file - run: | - docker run --rm --volume "$PWD":/work/plugins/thold \ - --env COMPOSER_ROOT_VERSION=1.3.0-dev \ - --entrypoint composer cacti-thold-test \ - run-script lint /work/plugins/thold - - - name: Run Pest with coverage - run: | - docker run --rm \ - --volume "$PWD/cacti-runtime":/cacti \ - --volume "$PWD":/cacti/plugins/thold \ - --env COMPOSER_ROOT_VERSION=1.3.0-dev \ - --env XDEBUG_MODE=coverage \ - --user root \ - --entrypoint composer cacti-thold-test \ - test -- --configuration=/cacti/plugins/thold/phpunit.xml \ - --coverage-clover=/cacti/plugins/thold/coverage/clover.xml \ - /cacti/plugins/thold/tests/Unit - - # Whole-file coverage is meaningless here: most of the plugin only runs - # inside a live Cacti. What is enforceable is that a change covers the - # lines it adds. - - name: Enforce coverage of changed lines - if: github.event_name == 'pull_request' - env: - BASE_REF: ${{ github.event.pull_request.base.sha }} - run: | - docker run --rm --volume "$PWD":/plugin --workdir /plugin \ - --env BASE_REF --user root --entrypoint sh cacti-thold-test \ - -c 'git config --global --add safe.directory /plugin && php tests/bin/patch-coverage.php coverage/clover.xml "$BASE_REF" 100' - - - name: Upload coverage report - if: always() - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 - with: - name: coverage - path: coverage/ - if-no-files-found: warn diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index e6775750..119d9298 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -31,17 +31,20 @@ on: - main - develop +env: + CACTI: 1.2.x + COMPOSER_ALLOW_SUPERUSER: 1 + jobs: integration-test: runs-on: ${{ matrix.os }} - + strategy: fail-fast: false matrix: - php: ['8.1', '8.2', '8.3', '8.4'] + php: ['8.2', '8.3', '8.4'] os: [ubuntu-latest] - cacti: ['release/1.2.31'] - + services: mariadb: image: mariadb:10.6 @@ -57,62 +60,46 @@ jobs: --health-interval=10s --health-timeout=5s --health-retries=3 - - name: PHP ${{ matrix.php }} Integration Test on ${{ matrix.os }} against Cacti ${{ matrix.cacti }} - + + name: PHP ${{ matrix.php }} Integration Test on ${{ matrix.os }} + steps: - name: Checkout Cacti - uses: actions/checkout@v7 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: repository: Cacti/cacti - ref: ${{ matrix.cacti }} + ref: ${{ env.CACTI }} path: cacti - + - name: Checkout Thold Plugin - uses: actions/checkout@v7 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: path: cacti/plugins/thold - + # patch-coverage.php diffs against the base branch below. + fetch-depth: 0 + - name: Install PHP ${{ matrix.php }} - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ matrix.php }} - extensions: intl, mysql, gd, ldap, gmp, xml, curl, json, mbstring + extensions: intl, mysql, gd, ldap, gmp, xml, curl, json, mbstring, snmp ini-values: "post_max_size=256M, max_execution_time=60, date.timezone=America/New_York" - + coverage: xdebug + - name: Check PHP version - run: | - php -v - echo "PHP_BINARY=$(command -v php)" >> "$GITHUB_ENV" - - - name: Run apt-get update - run: | - for attempt in 1 2 3; do - if sudo timeout 3m apt-get \ - -o Dpkg::Lock::Timeout=60 \ - -o Acquire::Retries=3 \ - -o Acquire::http::Timeout=30 \ - -o Acquire::https::Timeout=30 \ - update; then - exit 0 - fi + run: php -v - if [ "$attempt" -lt 3 ]; then - sleep 10 - fi - done + - name: Run apt-get update + run: sudo apt-get update - echo 'apt-get update failed after three bounded attempts.' >&2 - exit 1 - - name: Install System Dependencies - run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping libapache2-mod-php - + run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping + - name: Start SNMPD Agent and Test run: | sudo systemctl start snmpd sudo snmpwalk -c public -v2c -On localhost .1.3.6.1.2.1.1 - + - name: Setup Permissions run: | sudo chown -R www-data:runner ${{ github.workspace }}/cacti @@ -120,12 +107,12 @@ jobs: sudo find ${{ github.workspace }}/cacti -type f -exec chmod 664 {} \; sudo chmod +x ${{ github.workspace }}/cacti/cmd.php sudo chmod +x ${{ github.workspace }}/cacti/poller.php - + - name: Create MySQL Config run: | echo -e "[client]\nuser = root\npassword = cactiroot\nhost = 127.0.0.1\n" > ~/.my.cnf cat ~/.my.cnf - + - name: Initialize Cacti Database env: MYSQL_AUTH_USR: '--defaults-file=~/.my.cnf' @@ -136,33 +123,30 @@ jobs: mysql $MYSQL_AUTH_USR -e "GRANT SELECT ON mysql.time_zone_name TO 'cactiuser'@'localhost';" mysql $MYSQL_AUTH_USR -e "FLUSH PRIVILEGES;" mysql $MYSQL_AUTH_USR cacti < ${{ github.workspace }}/cacti/cacti.sql - mysql $MYSQL_AUTH_USR -e "INSERT INTO settings (name, value) VALUES ('path_php_binary', '$PHP_BINARY')" cacti - + mysql $MYSQL_AUTH_USR -e "INSERT INTO settings (name, value) VALUES ('path_php_binary', '/usr/bin/php')" cacti + - name: Validate composer files run: | cd ${{ github.workspace }}/cacti if [ -f composer.json ]; then - composer validate --strict || true + sudo composer validate --strict || true fi - + - name: Install Composer Dependencies run: | cd ${{ github.workspace }}/cacti if [ -f composer.json ]; then - for attempt in 1 2 3; do - if sudo composer install --prefer-dist --no-progress --no-interaction; then - exit 0 - fi + sudo composer config --no-plugins allow-plugins.pestphp/pest-plugin true + sudo composer require --no-progress --no-interaction "pestphp/pest: ^3" "pestphp/pest-plugin-drift: ^3.0" + sudo rm -f composer.lock + sudo composer install --dev --no-progress + fi - if [ "$attempt" -lt 3 ]; then - sleep 10 - fi - done + - name: Restore vendor ownership for Pest + run: | + cd ${{ github.workspace }}/cacti + sudo chown -R runner:runner include/vendor composer.lock - echo 'Composer install failed after three attempts.' >&2 - exit 1 - fi - - name: Create Cacti config.php run: | cat ${{ github.workspace }}/cacti/include/config.php.dist | \ @@ -171,46 +155,37 @@ jobs: sed -r "s/'cactiuser'/'cactiuser'/g" | \ sed -r "s/'cactiuser'/'cactiuser'/g" > ${{ github.workspace }}/cacti/include/config.php sudo chmod 664 ${{ github.workspace }}/cacti/include/config.php - + - name: Configure Apache run: | cat << 'EOF' | sed 's#GITHUB_WORKSPACE#${{ github.workspace }}#g' > /tmp/cacti.conf ServerAdmin webmaster@localhost DocumentRoot GITHUB_WORKSPACE/cacti - + Options Indexes FollowSymLinks AllowOverride All Require all granted - + ErrorLog ${APACHE_LOG_DIR}/error.log CustomLog ${APACHE_LOG_DIR}/access.log combined EOF sudo cp /tmp/cacti.conf /etc/apache2/sites-available/000-default.conf sudo systemctl restart apache2 - + - name: Install Cacti via CLI run: | cd ${{ github.workspace }}/cacti - sudo "$PHP_BINARY" cli/install_cacti.php --accept-eula --install --force - + sudo php cli/install_cacti.php --accept-eula --install --force + - name: Install Thold Plugin run: | cd ${{ github.workspace }}/cacti - sudo "$PHP_BINARY" cli/plugin_manage.php --plugin=thold --install --enable - -# - name: import Thold Plugin Sample Data -# run: | -# cd ${{ github.workspace }}/cacti/plugins/thold -# sudo php cli_import.php --filename=.github/workflows/thold_sample_data.xml -# if [ $? -ne 0 ]; then -# echo "Failed to import Thold sample data" -# exit 1 -# fi - + sudo php cli/plugin_manage.php --plugin=thold --install --enable + - name: Check PHP Syntax for Plugin run: | cd ${{ github.workspace }}/cacti/plugins/thold @@ -218,42 +193,62 @@ jobs: echo "Syntax errors found!" exit 1 fi - - - name: Remove the plugins directory exclusion from the .phpstan.neon + + - name: Set expected Cacti version for unit tests + run: echo -n "${{ env.CACTI }}" | sudo tee ${{ github.workspace }}/cacti/plugins/thold/tests/.cacti-version > /dev/null + + - name: Run Pest Unit Tests + env: + COMPOSER_ROOT_VERSION: 1.3.0-dev run: | - if [ -f .phpstan.neon ]; then - sed -i '/plugins/d' .phpstan.neon - fi + cd ${{ github.workspace }}/cacti + include/vendor/bin/pest --configuration=plugins/thold/phpunit.xml \ + --coverage-clover=plugins/thold/coverage/clover.xml + + # Whole-file coverage is meaningless here: most of the plugin only runs + # inside a live Cacti. What is enforceable is that a change covers the + # lines it adds. + - name: Enforce coverage of changed lines + if: github.event_name == 'pull_request' + env: + BASE_REF: ${{ github.event.pull_request.base.sha }} + run: | + cd ${{ github.workspace }}/cacti/plugins/thold + git config --global --add safe.directory ${{ github.workspace }}/cacti/plugins/thold + php tests/bin/patch-coverage.php coverage/clover.xml "$BASE_REF" 100 + + - name: Upload coverage report + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: coverage-php${{ matrix.php }} + path: ${{ github.workspace }}/cacti/plugins/thold/coverage/ + if-no-files-found: warn + + - name: Remove the plugins directory exclusion from the .phpstan.neon + if: ${{ env.CACTI != '1.2.x' }} + run: sed '/plugins/d' -i .phpstan.neon working-directory: ${{ github.workspace }}/cacti - name: Mark composer scripts executable - run: | - if [ -d "${{ github.workspace }}/cacti/include/vendor/bin" ]; then - sudo find "${{ github.workspace }}/cacti/include/vendor/bin" -maxdepth 1 -type f -exec chmod +x {} + - fi + if: ${{ env.CACTI != '1.2.x' }} + run: sudo chmod +x ${{ github.workspace }}/cacti/include/vendor/bin/* - name: Run Linter on base code - run: | - if composer --no-ansi run-script --list | grep -qE '^[[:space:]]+lint([[:space:]]|$)'; then - composer run-script lint ${{ github.workspace }}/cacti/plugins/thold - else - echo 'Composer lint script is not defined; skipping.' - fi + if: ${{ env.CACTI != '1.2.x' }} + run: sudo composer run-script lint ${{ github.workspace }}/cacti/plugins/thold working-directory: ${{ github.workspace }}/cacti - name: Checking coding standards on base code - run: | - if composer --no-ansi run-script --list | grep -qE '^[[:space:]]+phpcsfixer([[:space:]]|$)'; then - composer run-script phpcsfixer ${{ github.workspace }}/cacti/plugins/thold - else - echo 'Composer phpcsfixer script is not defined; skipping.' - fi + if: ${{ env.CACTI != '1.2.x' }} + run: sudo composer run-script phpcsfixer ${{ github.workspace }}/cacti/plugins/thold + working-directory: ${{ github.workspace }}/cacti + + - name: Run PHPStan at Level 6 on base code outside of Composer due to technical issues + if: ${{ env.CACTI != '1.2.x' }} + run: sudo ./include/vendor/bin/phpstan analyze --level 6 ${{ github.workspace }}/cacti/plugins/thold working-directory: ${{ github.workspace }}/cacti -# - name: Run PHPStan at Level 6 on base code outside of Composer due to technical issues -# run: ./include/vendor/bin/phpstan analyze --level 6 ${{ github.workspace }}/cacti/plugins/thold -# working-directory: ${{ github.workspace }}/cacti - - name: Re-apply web user ownership before polling run: | # Ancestor directories above the checkout (e.g. /home/runner, .../work) @@ -274,13 +269,14 @@ jobs: - name: Run Cacti Poller run: | cd ${{ github.workspace }}/cacti - sudo -u www-data "$PHP_BINARY" poller.php --poller=1 --force --debug + sudo -u www-data php poller.php --poller=1 --force --debug + if ! grep -q "SYSTEM STATS" log/cacti.log; then echo "Cacti poller did not finish successfully" cat log/cacti.log exit 1 fi - + - name: View Cacti Logs if: always() run: | diff --git a/CHANGELOG.md b/CHANGELOG.md index 1aeb9837..33c31b59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ * issue#710: Fixing Typo in thold_daemons.service File * issue#714: Increase the Name column to 255 characters * issue#719: Plugin Disabled due to mix of string and int +* issue#815: Keep counter sample values and timestamps synchronized, recover from backward sample clocks, preserve alert state while samples are unavailable, and fail closed when expression sources cannot be resolved * issue#814: Normalize nullable notification template text before replacement * issue#784: Retry failed queued email notifications with bounded exponential backoff * issue#812: Recover stale notification claims, scope worker drains, and deprecate the ignored notification --thread option diff --git a/README.md b/README.md index 4f355898..a2676a09 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,24 @@ The legacy `thold_notify.php --thread=N` option remains accepted for operator compatibility but is deprecated and ignored; queue ownership and worker serialization are automatic. +Counter, derive, and absolute rate thresholds preserve the previous value and +timestamp together when a poll has no numeric sample, preventing the next rate +from using mismatched interval data. A gap of more than two effective sampling +intervals (the greater of the RRD step and poller interval), or the configured +RRD heartbeat when available, is treated as unknown while the current sample +starts a fresh baseline instead of producing a stale rate. Thresholds with an +unknown rate remain eligible for the next poll, but that poll preserves the +existing alert state and writes a warning instead of treating the missing +sample as a restoral. Warnings are emitted only when the threshold enters the +unavailable state. A backward sample clock is re-anchored without calculating a +rate for that cycle, so later samples can recover normally. Expression +thresholds require a numeric sibling value in the current poll and use its +cached DSStats rate, with an RRD fallback, even when the sibling has no +threshold row. This preserves COUNTER and DERIVE rate units without adding an +rrdtool process per expression during normal operation. They fail closed when +the current sibling value is unavailable. Gauge readings remain valid across +such a gap. + As with much of Cacti, settings should be documented in line with the actual setting. If you find that any of these settings are ambiguous, please create a pull request with your proposed changes. diff --git a/includes/polling.php b/includes/polling.php index 0e0ca5a1..d660cc7d 100644 --- a/includes/polling.php +++ b/includes/polling.php @@ -133,7 +133,7 @@ function thold_poller_output(&$rrd_update_array) { td.cdef, td.local_data_id, td.data_template_rrd_id, td.lastread, UNIX_TIMESTAMP(td.lasttime) AS lasttime, td.oldvalue, td.data_source_name AS name, dtr.data_source_type_id, - dtd.rrd_step, dtr.rrd_maximum + dtd.rrd_step, dtr.rrd_maximum, dtr.rrd_heartbeat FROM thold_data AS td LEFT JOIN data_template_rrd AS dtr ON dtr.id = td.data_template_rrd_id @@ -143,7 +143,8 @@ function thold_poller_output(&$rrd_update_array) { AND td.local_data_id IN($local_data_ids)"); if (cacti_sizeof($tholds)) { - $sql = []; + $sql = []; + $status_sql = []; foreach ($tholds as $thold_data) { thold_debug("Checking Threshold: Name: '" . $thold_data['thold_name'] . "', Graph: '" . $thold_data['local_graph_id'] . "'"); @@ -184,57 +185,80 @@ function thold_poller_output(&$rrd_update_array) { if (!is_numeric($currentval)) { if (read_config_option('thold_consider_unknown_zero') == 'on') { - $currentval = strtolower($currentval); + $normalized = strtolower($currentval); - if ($currentval == 'u' || $currentval == 'nan' || $currentval == '') { - $currentval = 0; - thold_debug('Threshold: ' . $thold_data['thold_name'] . ' changing unknown value to zero', 'thold'); + if ($normalized == 'u' || $normalized == 'nan' || $normalized == '') { + // Fail closed: an unavailable sample must not be persisted as a + // manufactured zero, or thold_check_threshold()'s unavailable-sample + // guard never sees it and can alert/recover on the fabricated value. + thold_debug('Threshold: ' . $thold_data['thold_name'] . ' unknown value would have been treated as zero; preserving unavailable state instead', 'thold'); if (read_config_option('thold_log_unknown_to_zero') == 'on') { - cacti_log('NOTE: Threshold \'' . $thold_data['thold_name'] . '\' changing unknown value to zero', true, 'THOLD'); + cacti_log('NOTE: Threshold \'' . $thold_data['thold_name'] . '\' unknown value would have been treated as zero; preserving unavailable state instead', true, 'THOLD'); } - } else { - $currentval = ''; } - } else { - $currentval = ''; } - } - // This stores the raw value into the data source and is important for - // Counters, where calculating the difference is important. - // The unset case is problematic and may lead to false triggering - // events. So, in those cases, we will store the 'oldvalue'. - if (isset($item[$thold_data['name']])) { - $rawvalue = $item[$thold_data['name']]; - } else { - $rawvalue = $thold_data['oldvalue']; + $currentval = ''; } - $sql[] = '(' . $thold_data['id'] . ', 1, ' . db_qstr($currentval) . ', FROM_UNIXTIME(' . $currenttime . '), ' . db_qstr($rawvalue) . ')'; + $sample_rows = thold_polling_sample_row($thold_data, $item, $currentval, $currenttime); + + if ($sample_rows['sample_row'] !== null) { + $sql[] = $sample_rows['sample_row']; + } elseif ($sample_rows['status_row'] !== null) { + $status_sql[] = $sample_rows['status_row']; + } } if (cacti_sizeof($sql)) { - $chunks = array_chunk($sql, 400); + foreach (array_chunk($sql, 400) as $chunk) { + $placeholders = implode(', ', array_fill(0, cacti_sizeof($chunk), '(?, ?, ?, FROM_UNIXTIME(?), ?)')); + $params = []; + + foreach ($chunk as $row) { + $params[] = $row['id']; + $params[] = $row['tcheck']; + $params[] = $row['lastread']; + $params[] = $row['lasttime']; + $params[] = $row['oldvalue']; + } - foreach ($chunks as $c) { - db_execute('INSERT INTO thold_data + db_execute_prepared('INSERT INTO thold_data (id, tcheck, lastread, lasttime, oldvalue) - VALUES ' . implode(', ', $c) . ' + VALUES ' . $placeholders . ' ON DUPLICATE KEY UPDATE tcheck = VALUES(tcheck), lastread = VALUES(lastread), lasttime = VALUES(lasttime), - oldvalue = VALUES(oldvalue)'); + oldvalue = VALUES(oldvalue)', + $params); } - // accommodate deleted tholds - db_execute('DELETE FROM thold_data WHERE local_data_id = 0'); + } + + if (cacti_sizeof($status_sql)) { + foreach (array_chunk($status_sql, 400) as $chunk) { + $placeholders = implode(', ', array_fill(0, cacti_sizeof($chunk), '(?, ?, ?)')); + $params = []; + + foreach ($chunk as $row) { + $params[] = $row['id']; + $params[] = $row['tcheck']; + $params[] = $row['lastread']; + } - if (db_affected_rows() > 0) { - set_config_option('time_last_change_thold', time()); + db_execute_prepared('INSERT INTO thold_data + (id, tcheck, lastread) + VALUES ' . $placeholders . ' + ON DUPLICATE KEY UPDATE + tcheck = VALUES(tcheck), + lastread = VALUES(lastread)', + $params); } } + + thold_polling_cleanup(cacti_sizeof($sql) || cacti_sizeof($status_sql)); } return $rrd_update_array; diff --git a/phpunit.xml b/phpunit.xml index b68405ad..bc33485b 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -14,7 +14,9 @@ + ./tests/Security ./tests/Unit + ./tests/Integration @@ -22,13 +24,17 @@ Pest reads this PHPUnit-compatible configuration through Cacti's Composer-managed vendor tree. - thold_functions.php is the only plugin source that loads without a - running Cacti; every other file includes ./include/auth.php and - dispatches on the request at top level. + thold_functions.php, includes/polling.php, and setup.php are the only + plugin sources that load without a running Cacti (no top-level side + effects); every other file includes ./include/auth.php and dispatches + on the request at top level. tests/Integration/PluginLifecycleTest.php + is what requires setup.php directly. --> thold_functions.php + includes/polling.php + setup.php diff --git a/tests/Helpers/ThresholdOutcome.php b/tests/Helpers/ThresholdOutcome.php index 4642467c..3b70faaa 100644 --- a/tests/Helpers/ThresholdOutcome.php +++ b/tests/Helpers/ThresholdOutcome.php @@ -117,6 +117,21 @@ public function acknowledged() { return false; } + /** + * Whether the run cleared the acknowledgment flag. + * + * @return bool + */ + public function acknowledgmentCleared() { + foreach (CactiStubs::callsTo('db_execute_prepared') as $call) { + if (preg_match('/SET\s+acknowledgment\s*=\s*""/', $call['sql'])) { + return true; + } + } + + return false; + } + /** * Columns the run wrote to thold_data, resolved to their values. * diff --git a/tests/Integration/PluginLifecycleTest.php b/tests/Integration/PluginLifecycleTest.php new file mode 100644 index 00000000..7772efd7 --- /dev/null +++ b/tests/Integration/PluginLifecycleTest.php @@ -0,0 +1,62 @@ +toHaveKey('name'); + expect($info)->toHaveKey('version'); + expect($info['name'])->toBe('thold'); +}); + +it('removes thold settings on uninstall', function () { + plugin_thold_uninstall(); + + $calls = CactiStubs::callsTo('db_execute'); + + expect($calls)->not->toBeEmpty(); + expect($calls[0]['sql'])->toContain('DELETE FROM settings'); + expect($calls[0]['sql'])->toContain('thold'); +}); + +it('does not check for an upgrade on pages outside its allow-list', function () { + CactiStubs::willReturn('get_current_page', 'graphs.php'); + + expect(plugin_thold_upgrade())->toBeFalse(); + expect(CactiStubs::callsTo('db_fetch_cell'))->toBeEmpty(); +}); + +it('does not reinstall when the installed version already matches', function () { + CactiStubs::willReturn('get_current_page', 'thold.php'); + CactiStubs::willReturnFor('db_fetch_cell', 'plugin_config', plugin_thold_version()['version']); + + expect(plugin_thold_upgrade())->toBeTrue(); + expect(CactiStubs::callsTo('api_plugin_register_hook'))->toBeEmpty(); +}); diff --git a/tests/Security/Php82CompatibilityTest.php b/tests/Security/Php82CompatibilityTest.php new file mode 100644 index 00000000..f398fac7 --- /dev/null +++ b/tests/Security/Php82CompatibilityTest.php @@ -0,0 +1,105 @@ +getExtension() !== 'php') { + continue; + } + + $relativeFile = ltrim(str_replace($pluginRoot, '', $file->getPathname()), DIRECTORY_SEPARATOR); + $relativeFile = str_replace(DIRECTORY_SEPARATOR, '/', $relativeFile); + + if (strpos($relativeFile, 'tests/') === 0) { + continue; + } + + $files[] = $relativeFile; +} + +sort($files); + +it('does not use typed class constants (PHP 8.3)', function () use ($files) { + foreach ($files as $relativeFile) { + $contents = plugin_test_read_source($relativeFile); + + expect(preg_match('/\bconst\s+(?:int|string|float|bool|array|self|static|mixed)\s+[A-Z_][A-Za-z0-9_]*\s*=/', $contents))->toBe(0, + "{$relativeFile} uses typed class constants which require PHP 8.3" + ); + } +}); + +it('does not use json_validate (PHP 8.3)', function () use ($files) { + foreach ($files as $relativeFile) { + $contents = plugin_test_read_source($relativeFile); + + expect(preg_match('/\bjson_validate\s*\(/', $contents))->toBe(0, + "{$relativeFile} uses json_validate() which requires PHP 8.3" + ); + } +}); + +it('does not use the #[Override] attribute (PHP 8.3)', function () use ($files) { + foreach ($files as $relativeFile) { + $contents = plugin_test_read_source($relativeFile); + + expect(preg_match('/#\[\s*Override\s*\]/i', $contents))->toBe(0, + "{$relativeFile} uses the #[Override] attribute which requires PHP 8.3" + ); + } +}); + +it('does not use array_find, array_any, or array_all (PHP 8.4)', function () use ($files) { + foreach ($files as $relativeFile) { + $contents = plugin_test_read_source($relativeFile); + + expect(preg_match('/\barray_(?:find|any|all)\s*\(/', $contents))->toBe(0, + "{$relativeFile} uses array_find()/array_any()/array_all() which require PHP 8.4" + ); + } +}); + +it('does not use asymmetric visibility (PHP 8.4)', function () use ($files) { + foreach ($files as $relativeFile) { + $contents = plugin_test_read_source($relativeFile); + + expect(preg_match('/\bpublic\(set\)|\bprotected\(set\)|\bprivate\(set\)/', $contents))->toBe(0, + "{$relativeFile} uses asymmetric visibility which requires PHP 8.4" + ); + } +}); + +it('does not use the #[Deprecated] attribute (PHP 8.4)', function () use ($files) { + foreach ($files as $relativeFile) { + $contents = plugin_test_read_source($relativeFile); + + expect(preg_match('/#\[\s*Deprecated\b/i', $contents))->toBe(0, + "{$relativeFile} uses the #[Deprecated] attribute which requires PHP 8.4" + ); + } +}); diff --git a/tests/Security/SetupStructureTest.php b/tests/Security/SetupStructureTest.php new file mode 100644 index 00000000..2510d110 --- /dev/null +++ b/tests/Security/SetupStructureTest.php @@ -0,0 +1,49 @@ +toContain('function plugin_thold_install'); +}); + +it('defines plugin_thold_uninstall function', function () use ($source) { + expect($source)->toContain('function plugin_thold_uninstall'); +}); + +it('defines plugin_thold_version function', function () use ($source) { + expect($source)->toContain('function plugin_thold_version'); +}); + +it('declares a plugin name in INFO', function () use ($info) { + expect($info)->toHaveKey('name'); +}); + +it('declares a plugin version in INFO', function () use ($info) { + expect($info)->toHaveKey('version'); +}); diff --git a/tests/Unit/GetCurrentValueTest.php b/tests/Unit/GetCurrentValueTest.php index f467b9bc..e677a7d5 100644 --- a/tests/Unit/GetCurrentValueTest.php +++ b/tests/Unit/GetCurrentValueTest.php @@ -96,6 +96,12 @@ public function testMissingDataSourceNamesReturnsZero(): void { $this->rrdReturns([]); $this->assertSame(0, get_current_value(4, 'traffic_in')); + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); + CactiStubs::willReturn('rrdtool_execute', '1700000000'); + $this->rrdReturns([]); + $this->assertSame('', get_current_value(4, 'traffic_in', 0, '')); } /** @@ -105,6 +111,12 @@ public function testMissingValuesReturnsZero(): void { $this->rrdReturns(['data_source_names' => ['traffic_in']]); $this->assertSame(0, get_current_value(4, 'traffic_in')); + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); + CactiStubs::willReturn('rrdtool_execute', '1700000000'); + $this->rrdReturns(['data_source_names' => ['traffic_out'], 'values' => [['1700000000' => 20.0]]]); + $this->assertSame('', get_current_value(4, 'traffic_in', 0, '')); } /** @@ -145,4 +157,25 @@ public function testValueIsRoundedToFourDecimals(): void { $this->assertSame(1.2346, get_current_value(4, 'traffic_in')); } + + /** + * A stored 'U'/'nan'/blank reading must stay missing rather than coerce + * to zero, the same contract as an empty value series above. + * + * @return void + */ + public function testNonNumericStoredValueReturnsTheMissingValue(): void { + $this->rrdReturns([ + 'data_source_names' => ['traffic_in'], + 'values' => [['1700000000' => 'U']], + ]); + + $this->assertSame(0, get_current_value(4, 'traffic_in')); + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); + CactiStubs::willReturn('rrdtool_execute', '1700000000'); + $this->rrdReturns(['data_source_names' => ['traffic_in'], 'values' => [['1700000000' => 'U']]]); + $this->assertSame('', get_current_value(4, 'traffic_in', 0, '')); + } } diff --git a/tests/Unit/NotificationEmailDeduplicationTest.php b/tests/Unit/NotificationEmailDeduplicationTest.php index ba618eea..97612bb4 100644 --- a/tests/Unit/NotificationEmailDeduplicationTest.php +++ b/tests/Unit/NotificationEmailDeduplicationTest.php @@ -38,7 +38,7 @@ 'html' => true, ]; - CactiStubs::willReturn('db_fetch_assoc', [[ + CactiStubs::willReturn('db_fetch_assoc_prepared', [[ 'id' => 42, 'topic' => 'thold_dhost_mail', 'event_data' => json_encode($event), diff --git a/tests/Unit/NotificationQueueClaimTest.php b/tests/Unit/NotificationQueueClaimTest.php index 75a639f1..3b5a1621 100644 --- a/tests/Unit/NotificationQueueClaimTest.php +++ b/tests/Unit/NotificationQueueClaimTest.php @@ -196,7 +196,7 @@ public function testRegistrationRequiresTheLeaseAndHandlesQueryFailures(): void $this->assertFalse(thold_notification_register_process(2, 300, static function () { return false; })); - $this->assertSame([], CactiStubs::$calls); + $this->assertSame(['cacti_log'], array_column(CactiStubs::$calls, 'fn')); $this->assertNotEmpty(CactiStubs::$log); CactiStubs::reset(); @@ -432,7 +432,7 @@ public function testDefaultUnixProbeKeepsALiveWorkerRegistered(): void { return true; })); $this->assertSame( - ['db_fetch_row_prepared', 'db_fetch_cell_prepared'], + ['db_fetch_row_prepared', 'db_fetch_cell_prepared', 'cacti_log'], array_column(CactiStubs::$calls, 'fn') ); } @@ -882,7 +882,7 @@ static function () { $this->assertStringContainsString('registration failed', end(CactiStubs::$log)); $this->assertFalse($GLOBALS['notification_registered']); $this->assertSame( - ['db_execute_prepared', 'unregister_process', 'db_fetch_cell_prepared'], + ['cacti_log', 'db_execute_prepared', 'unregister_process', 'db_fetch_cell_prepared'], array_column(CactiStubs::$calls, 'fn') ); } diff --git a/tests/Unit/PollerSchedulingTest.php b/tests/Unit/PollerSchedulingTest.php index 9a3b7be4..3d14c923 100644 --- a/tests/Unit/PollerSchedulingTest.php +++ b/tests/Unit/PollerSchedulingTest.php @@ -225,6 +225,158 @@ public function testTheDaemonWritesReadingsInBatchesOfFifty(): void { $this->assertCount(3, $inserts); } + /** + * A threshold row shaped like thold_poller_output()'s non-daemon query + * (td/dtr/dtd columns), evaluating quietly as a gauge. + * + * @param array $overrides + * + * @return array + */ + private function pollerThreshold(array $overrides = []) { + return $overrides + [ + 'id' => 9, + 'thold_name' => 'CPU', + 'local_graph_id' => 9, + 'percent_ds' => '', + 'expression' => '', + 'upper_ds' => '', + 'data_type' => 0, + 'cdef' => 0, + 'local_data_id' => 4, + 'data_template_rrd_id' => 3, + 'lastread' => '', + 'lasttime' => 0, + 'oldvalue' => '', + 'name' => 'traffic_in', + 'data_source_type_id' => 1, + 'rrd_step' => 300, + 'rrd_maximum' => 0, + 'rrd_heartbeat' => 600, + ]; + } + + /** + * A numeric reading turns into a sample_row, which is written through the + * FROM_UNIXTIME() upsert - not the status-only insert added alongside it. + * + * @return void + */ + public function testNonDaemonPathPersistsANumericSampleRow(): void { + CactiStubs::willReturnFor('db_fetch_assoc', 'dtr.rrd_maximum, dtr.rrd_heartbeat', [ + $this->pollerThreshold(), + ]); + + $readings = [ + ['local_data_id' => 4, 'times' => [1700000300 => ['traffic_in' => 55]]], + ]; + + thold_poller_output($readings); + + $upserts = array_values(array_filter(CactiStubs::$calls, static function ($call) { + return strpos($call['sql'], 'FROM_UNIXTIME(?)') !== false + && in_array(1700000300, $call['params'], true); + })); + + $this->assertCount(1, $upserts); + + $status_inserts = array_filter(CactiStubs::callsTo('db_execute_prepared'), static function ($call) { + return strpos($call['sql'], 'INSERT INTO thold_data') !== false && strpos($call['sql'], 'lasttime') === false; + }); + + $this->assertSame([], array_values($status_inserts)); + } + + /** + * An unavailable reading (non-numeric raw value) turns into a status-only + * row instead - persisting tcheck/lastread without fabricating a sample + * time, and without touching the FROM_UNIXTIME() upsert. + * + * @return void + */ + public function testNonDaemonPathPersistsAStatusOnlyRowForAnUnavailableReading(): void { + CactiStubs::willReturnFor('db_fetch_assoc', 'dtr.rrd_maximum, dtr.rrd_heartbeat', [ + $this->pollerThreshold(), + ]); + + $readings = [ + ['local_data_id' => 4, 'times' => [1700000300 => ['traffic_in' => 'U']]], + ]; + + thold_poller_output($readings); + + $status_inserts = array_values(array_filter(CactiStubs::callsTo('db_execute_prepared'), static function ($call) { + return strpos($call['sql'], 'INSERT INTO thold_data') !== false && strpos($call['sql'], 'lasttime') === false; + })); + + $this->assertCount(1, $status_inserts); + $this->assertSame([9, 1, ''], $status_inserts[0]['params']); + + $sample_upserts = array_filter(CactiStubs::$calls, static function ($call) { + return strpos($call['sql'], 'FROM_UNIXTIME') !== false; + }); + + $this->assertSame([], array_values($sample_upserts)); + } + + /** + * With thold_consider_unknown_zero enabled, an unavailable reading is + * still preserved as unavailable (never manufactured into a zero) - this + * setting only controls whether that fact gets logged, not the outcome. + * + * @return void + */ + public function testNonDaemonPathLogsAndStillPreservesUnavailableStateWhenConsiderUnknownZeroIsOn(): void { + CactiStubs::willReturnFor('db_fetch_assoc', 'dtr.rrd_maximum, dtr.rrd_heartbeat', [ + $this->pollerThreshold(), + ]); + CactiStubs::$configOptions['thold_consider_unknown_zero'] = 'on'; + CactiStubs::$configOptions['thold_log_unknown_to_zero'] = 'on'; + + $readings = [ + ['local_data_id' => 4, 'times' => [1700000300 => ['traffic_in' => 'U']]], + ]; + + thold_poller_output($readings); + + $notes = array_values(array_filter(CactiStubs::$log, static function ($message) { + return strpos($message, 'would have been treated as zero') !== false; + })); + + $this->assertNotEmpty($notes); + + $status_inserts = array_values(array_filter(CactiStubs::callsTo('db_execute_prepared'), static function ($call) { + return strpos($call['sql'], 'INSERT INTO thold_data') !== false && strpos($call['sql'], 'lasttime') === false; + })); + + $this->assertCount(1, $status_inserts); + $this->assertSame([9, 1, ''], $status_inserts[0]['params']); + } + + /** + * Either write path counts as an update, so the deleted-tholds cleanup + * still runs afterwards. + * + * @return void + */ + public function testNonDaemonPathRunsCleanupAfterWriting(): void { + CactiStubs::willReturnFor('db_fetch_assoc', 'dtr.rrd_maximum, dtr.rrd_heartbeat', [ + $this->pollerThreshold(), + ]); + + $readings = [ + ['local_data_id' => 4, 'times' => [1700000300 => ['traffic_in' => 55]]], + ]; + + thold_poller_output($readings); + + $cleanup = array_filter(CactiStubs::callsTo('db_execute_prepared'), static function ($call) { + return strpos($call['sql'], 'DELETE FROM thold_data WHERE local_data_id = 0') !== false; + }); + + $this->assertCount(1, $cleanup); + } + /** * A structural guard rather than a behavioural one: the defect is SQL * operator precedence, and proving it needs a database to run the query diff --git a/tests/Unit/TholdBuildCdefTest.php b/tests/Unit/TholdBuildCdefTest.php new file mode 100644 index 00000000..08157a3e --- /dev/null +++ b/tests/Unit/TholdBuildCdefTest.php @@ -0,0 +1,79 @@ +assertSame('', thold_build_cdef(1, '', 4, 5)); + } + + /** + * @return void + */ + public function testEvaluatesALiteralAdditionCdef(): void { + CactiStubs::willReturn('db_fetch_assoc_prepared', [ + ['id' => 1, 'type' => 6, 'value' => 10], + ['id' => 2, 'type' => 6, 'value' => 5], + ['id' => 3, 'type' => 2, 'value' => 1], + ]); + + $this->assertSame(15, thold_build_cdef(1, 100, 4, 5)); + } + + /** + * @return void + */ + public function testFailsClosedWhenAQueryOperandDoesNotResolve(): void { + CactiStubs::willReturn('db_fetch_assoc_prepared', [ + ['id' => 1, 'type' => 6, 'value' => 10], + ['id' => 2, 'type' => 6, 'value' => '|query_ifOperStatus|'], + ['id' => 3, 'type' => 2, 'value' => 1], + ]); + CactiStubs::willReturn('db_fetch_cell_prepared', ''); + + $this->assertSame('', thold_build_cdef(1, 100, 4, 5)); + } + + /** + * @return void + */ + public function testEvaluatesWhenAQueryOperandResolves(): void { + CactiStubs::willReturn('db_fetch_assoc_prepared', [ + ['id' => 1, 'type' => 6, 'value' => 10], + ['id' => 2, 'type' => 6, 'value' => '|query_ifOperStatus|'], + ['id' => 3, 'type' => 2, 'value' => 1], + ]); + CactiStubs::willReturn('db_fetch_cell_prepared', 5); + + $this->assertSame(15, thold_build_cdef(1, 100, 4, 5)); + } +} diff --git a/tests/Unit/TholdCalculateExpressionTest.php b/tests/Unit/TholdCalculateExpressionTest.php index b3a13714..3611fbe8 100644 --- a/tests/Unit/TholdCalculateExpressionTest.php +++ b/tests/Unit/TholdCalculateExpressionTest.php @@ -79,7 +79,10 @@ public function testExpressionsReduceToTheTopOfTheStack($expression, $expected): * @return void */ public function testUnbalancedExpressionIsRejectedRatherThanReturningAnOperand(): void { - $this->assertSame(0, $this->evaluate('1,2,3,+')); + // '' (not numeric 0) is this function's unavailable-sample sentinel: + // polling.php's fail-closed guard only catches non-numeric values, so + // a manufactured 0 here would be persisted/alerted on as a real reading. + $this->assertSame('', $this->evaluate('1,2,3,+')); $this->assertTrue($GLOBALS['rpn_error']); } @@ -87,7 +90,53 @@ public function testUnbalancedExpressionIsRejectedRatherThanReturningAnOperand() * @return void */ public function testUnsupportedTokenFailsTheExpression(): void { - $this->assertSame(0, $this->evaluate('2,NOSUCHOP')); + $this->assertSame('', $this->evaluate('2,NOSUCHOP')); + $this->assertTrue($GLOBALS['rpn_error']); + } + + /** + * Division/modulo by zero (with a non-zero numerator) is flagged by + * thold_expression_math_rpn() as an rpn_error. The whole-expression + * evaluator must fail closed with '' rather than a manufactured 0. + * + * @return array + */ + public static function zeroDivisorProvider() { + return [ + 'division by zero' => ['5,0,/'], + 'modulo by zero' => ['5,0,%'], + ]; + } + + /** + * @dataProvider zeroDivisorProvider + * + * @param string $expression + * + * @return void + */ + public function testZeroDivisorFailsClosedRatherThanReturningZero($expression): void { + $this->assertSame('', $this->evaluate($expression)); + $this->assertTrue($GLOBALS['rpn_error']); + } + + /** + * A |pipe| token thold_expand_string() cannot resolve returns ''. + * Converting that to '0' would let a manufactured zero flow through the + * RPN stack as a valid operand instead of failing the expression closed. + * + * @return void + */ + public function testUnresolvedPipeTokenFailsClosedRatherThanReturningZero(): void { + $graph = ['id' => 7, 'host_id' => 2, 'snmp_query_id' => '0', 'snmp_index' => '']; + + // thold_calculate_expression() and thold_expand_string() each look up + // graph_local independently. + CactiStubs::willReturn('db_fetch_row_prepared', $graph); + CactiStubs::willReturn('db_fetch_row_prepared', $graph); + CactiStubs::willReturn('expand_title', ''); + + $this->assertSame('', $this->evaluate('|query_ifName|')); $this->assertTrue($GLOBALS['rpn_error']); } diff --git a/tests/Unit/TholdCalculateLowerUpperTest.php b/tests/Unit/TholdCalculateLowerUpperTest.php index b76ba75b..0a1ccb59 100644 --- a/tests/Unit/TholdCalculateLowerUpperTest.php +++ b/tests/Unit/TholdCalculateLowerUpperTest.php @@ -34,26 +34,26 @@ public function testHighWordIsShiftedAndCombinedWithTheLowWord(): void { $thold = ['upper_ds' => 'octets_hi', 'local_data_id' => 4]; $rrd = [4 => ['octets_hi' => 2]]; - $this->assertSame((2 << 32) + 100, thold_calculate_lower_upper($thold, 100, $rrd)); + $this->assertSame((float) ((2 << 32) + 100), thold_calculate_lower_upper($thold, 100, $rrd)); } /** * @return void */ - public function testValuePassesThroughWhenTheHighWordIsAbsent(): void { + public function testFailsClosedWhenTheHighWordIsAbsent(): void { $thold = ['upper_ds' => 'octets_hi', 'local_data_id' => 4]; $rrd = [4 => ['octets_lo' => 5]]; - $this->assertSame(100, thold_calculate_lower_upper($thold, 100, $rrd)); + $this->assertSame('', thold_calculate_lower_upper($thold, 100, $rrd)); } /** * @return void */ - public function testValuePassesThroughWhenTheDataSourceHasNoReadings(): void { + public function testFailsClosedWhenTheDataSourceHasNoReadings(): void { $thold = ['upper_ds' => 'octets_hi', 'local_data_id' => 4]; - $this->assertSame(100, thold_calculate_lower_upper($thold, 100, [])); + $this->assertSame('', thold_calculate_lower_upper($thold, 100, [])); } /** @@ -63,6 +63,26 @@ public function testHighWordOfZeroLeavesTheValueUnchanged(): void { $thold = ['upper_ds' => 'octets_hi', 'local_data_id' => 4]; $rrd = [4 => ['octets_hi' => 0]]; - $this->assertSame(100, thold_calculate_lower_upper($thold, 100, $rrd)); + $this->assertSame(100.0, thold_calculate_lower_upper($thold, 100, $rrd)); + } + + /** + * @return void + */ + public function testFailsClosedWhenTheCurrentValueIsNotNumeric(): void { + $thold = ['upper_ds' => 'octets_hi', 'local_data_id' => 4]; + $rrd = [4 => ['octets_hi' => 2]]; + + $this->assertSame('', thold_calculate_lower_upper($thold, '', $rrd)); + } + + /** + * @return void + */ + public function testFailsClosedWhenTheHighWordIsOutOfRange(): void { + $thold = ['upper_ds' => 'octets_hi', 'local_data_id' => 4]; + $rrd = [4 => ['octets_hi' => 4294967296]]; + + $this->assertSame('', thold_calculate_lower_upper($thold, 100, $rrd)); } } diff --git a/tests/Unit/TholdCalculatePercentTest.php b/tests/Unit/TholdCalculatePercentTest.php index 4266fa39..3231ff32 100644 --- a/tests/Unit/TholdCalculatePercentTest.php +++ b/tests/Unit/TholdCalculatePercentTest.php @@ -80,8 +80,8 @@ public function testZeroDenominatorGivesZeroRatherThanDividingByZero(): void { /** * @return void */ - public function testNonNumericDenominatorGivesZero(): void { - $this->assertSame(0, $this->percent('U')); + public function testNonNumericDenominatorYieldsTheNoValueSentinel(): void { + $this->assertSame('', $this->percent('U')); } /** diff --git a/tests/Unit/TholdCommandExecutionTest.php b/tests/Unit/TholdCommandExecutionTest.php index d0f3e648..fb0dec55 100644 --- a/tests/Unit/TholdCommandExecutionTest.php +++ b/tests/Unit/TholdCommandExecutionTest.php @@ -147,6 +147,25 @@ public function testShellMetacharactersInDeviceDataAreQuoted($column, array $bre $this->assertSame("/usr/bin/alert '; touch /tmp/pwned'", $this->queuedCommand()); } + /** + * thold_command_execution() resolves |pipe| tokens (thold_expand_string()) + * and tokens (thold_replace_threshold_tags()) through a shared + * deferred-placeholder map, so a device value that happens to contain the + * literal text of the other family's token can't be expanded a second + * time and break out of its own quoting. + * + * @return void + */ + public function testADeviceValueContainingAPipeTokenIsNotReExpandedAsAHostToken(): void { + $malicious = "'; touch /tmp/pwned; echo '"; + $thold = $this->threshold(['trigger_cmd_high' => '/usr/bin/alert ']); + $device = $this->device(['description' => '|host_hostname|', 'hostname' => $malicious]); + + thold_command_execution($thold, $device, true, false, false); + + $this->assertSame('/usr/bin/alert ' . escapeshellarg('|host_hostname|'), $this->queuedCommand()); + } + /** * @return void */ diff --git a/tests/Unit/TholdExpandStringTest.php b/tests/Unit/TholdExpandStringTest.php index 8f1415d8..13235334 100644 --- a/tests/Unit/TholdExpandStringTest.php +++ b/tests/Unit/TholdExpandStringTest.php @@ -29,6 +29,46 @@ public static function setUpBeforeClass(): void { self::loadPluginSource('thold_functions.php'); } + /** + * Outside the per-graph SNMP substitution below (no graph found for this + * threshold), a |host_management_ip| token is still resolved against + * whatever device the current request is for. Real Cacti's + * lib/variables.php sets $device_id as a side effect of the include() + * inside thold_expand_string(), so this points library_path at a + * one-off fixture that does the same, rather than the shared + * tests/fixtures/cacti-lib one every other test here uses - which + * every other test in this file has typically already include_once()'d + * by the time this runs, so reusing it would silently no-op. + * + * @return void + */ + public function testShellModeEscapesTheTopLevelHostTokenForTheRequestDevice(): void { + $malicious = "10.0.0.1'; touch /tmp/pwned; echo '"; + + $fixtureDir = sys_get_temp_dir() . '/thold-variables-' . uniqid(); + mkdir($fixtureDir, 0777, true); + file_put_contents($fixtureDir . '/variables.php', " $malicious]); + + try { + $result = thold_expand_string($this->thresholdData(), 'alert |host_management_ip|', true); + } finally { + $GLOBALS['config']['library_path'] = $originalLibraryPath; + unlink($fixtureDir . '/variables.php'); + rmdir($fixtureDir); + } + + $this->assertSame('alert ' . escapeshellarg($malicious), $result); + } + /** * @return array */ @@ -105,6 +145,19 @@ public function testInterfaceSpeedFallsBackToTheConfiguredDefaultWhenUnknown(): $this->assertStringNotContainsString('|query_ifHighSpeed|', $result); } + /** + * @return void + */ + public function testUnthrottledInterfaceSpeedFallsBackToTheConfiguredDefaultWhenUnknown(): void { + $this->graphExists(); + CactiStubs::$configOptions['thold_empty_if_speed_default'] = '1000000000'; + CactiStubs::willReturn('db_fetch_cell_prepared', ''); + + $result = thold_expand_string($this->thresholdData(), '|query_ifSpeed|'); + + $this->assertStringNotContainsString('|query_ifSpeed|', $result); + } + /** * @return void */ @@ -137,4 +190,145 @@ public function testSurroundingWhitespaceIsTrimmed(): void { $this->assertSame('alert', thold_expand_string($this->thresholdData(), ' alert ')); } + + /** + * In $shell mode (used when building a trigger command line), host and + * data-query token values come from the polled device and must be shell- + * escaped rather than substituted raw, or a malicious sysDescr/community/ + * custom field could inject shell syntax into the command. + * + * @return void + */ + public function testShellModeEscapesHostTokenValues(): void { + CactiStubs::willReturn('db_fetch_row_prepared', [ + 'id' => 7, + 'host_id' => 2, + 'snmp_query_id' => '0', + 'snmp_index' => '', + ]); + + $malicious = "evil'; touch /tmp/pwned; echo '"; + CactiStubs::willReturn('substitute_host_data', $malicious); + + $result = thold_expand_string($this->thresholdData(), 'alert |host_description|', true); + + $this->assertSame('alert ' . escapeshellarg($malicious), $result); + } + + /** + * When the graph has an SNMP data query attached, host/query tokens are + * resolved through substitute_snmp_query_data() (so query-indexed fields + * work too) rather than through substitute_host_data() alone - and the + * result is still escaped before landing in the command. + * + * @return void + */ + public function testShellModeResolvesHostTokensThroughSnmpQueryDataWhenAGraphQueryIsAttached(): void { + $this->graphExists(); + + $malicious = "evil'; touch /tmp/pwned; echo '"; + CactiStubs::willReturn('substitute_snmp_query_data', $malicious); + + $result = thold_expand_string($this->thresholdData(), 'alert |host_description|', true); + + $this->assertSame('alert ' . escapeshellarg($malicious), $result); + $this->assertNotEmpty(CactiStubs::callsTo('substitute_snmp_query_data')); + } + + /** + * Non-shell callers (email/HTML rendering) must not be affected by the + * shell-escaping added for the trigger-command path. + * + * @return void + */ + public function testNonShellModeLeavesHostTokenValuesUnescaped(): void { + $this->graphExists(); + CactiStubs::willReturn('expand_title', 'alert eth0'); + + $this->assertSame('alert eth0', thold_expand_string($this->thresholdData(), 'alert |query_ifName|', false)); + } + + /** + * A single strtr() pass replaces host/query tokens against the original + * string only and never re-scans inserted values, so one token's + * resolved value containing another token's literal text can't be + * substituted a second time outside of its own quoting. + * + * @return void + */ + public function testShellModeDoesNotReSubstituteAHostTokenLiteralInsideAnotherValue(): void { + CactiStubs::willReturn('db_fetch_row_prepared', [ + 'id' => 7, + 'host_id' => 2, + 'snmp_query_id' => '0', + 'snmp_index' => '', + ]); + + $malicious = "'; touch /tmp/pwned; echo '"; + CactiStubs::willReturn('substitute_host_data', '|host_bar|'); + CactiStubs::willReturn('substitute_host_data', $malicious); + + $result = thold_expand_string($this->thresholdData(), 'cmd |host_foo| |host_bar|', true); + + $this->assertSame( + 'cmd ' . escapeshellarg('|host_bar|') . ' ' . escapeshellarg($malicious), + $result + ); + } + + /** + * A single strtr() pass also protects |graph_title|, + * |data_source_description|, and |data_source_name|: a resolved value + * containing another one of these tokens' literal text can't be + * substituted a second time outside of its own quoting. + * + * @return void + */ + public function testShellModeDoesNotReSubstituteADirectTokenLiteralInsideAnotherValue(): void { + $this->graphExists(); + + $malicious = "'; touch /tmp/pwned; echo '"; + CactiStubs::willReturn('get_graph_title', '|data_source_name|'); + CactiStubs::willReturn('db_fetch_cell_prepared', $malicious); + + $result = thold_expand_string( + $this->thresholdData(['data_source_name' => $malicious]), + 'cmd |graph_title| |data_source_description|', + true + ); + + $this->assertSame( + 'cmd ' . escapeshellarg('|data_source_name|') . ' ' . escapeshellarg($malicious), + $result + ); + } + + /** + * Every phase (host/query tokens, custom data, direct tokens) defers its + * quoted value behind an opaque placeholder and only reveals it in one + * final pass, so an earlier phase's resolved value containing a later + * phase's literal token text can't be re-substituted by that later + * phase and have its quoting broken. + * + * @return void + */ + public function testShellModeDoesNotReSubstituteAcrossPhases(): void { + CactiStubs::willReturn('db_fetch_row_prepared', [ + 'id' => 7, + 'host_id' => 2, + 'snmp_query_id' => '0', + 'snmp_index' => '', + ]); + + $malicious = "'; touch /tmp/pwned; echo '"; + CactiStubs::willReturn('substitute_host_data', '|data_source_name|'); + + $result = thold_expand_string( + $this->thresholdData(['data_source_name' => $malicious]), + 'cmd |host_description|', + true + ); + + $this->assertSame('cmd ' . escapeshellarg('|data_source_name|'), $result); + } } diff --git a/tests/Unit/TholdExpressionMathRpnTest.php b/tests/Unit/TholdExpressionMathRpnTest.php index 6c9fdbf1..b0f16de0 100644 --- a/tests/Unit/TholdExpressionMathRpnTest.php +++ b/tests/Unit/TholdExpressionMathRpnTest.php @@ -128,6 +128,19 @@ public function testModuloByZeroFlagsErrorInsteadOfThrowing(): void { $this->assertTrue($GLOBALS['rpn_error']); } + /** + * The % operator truncates both operands to int before dividing, so a + * divisor such as 0.5 becomes 0 even though it is not "== 0" as a float. + * Without a dedicated int-cast check, this previously reached eval() and + * threw an uncaught DivisionByZeroError instead of failing closed. + * + * @return void + */ + public function testModuloByFractionalZeroDivisorFlagsErrorInsteadOfThrowing(): void { + $this->assertSame([], $this->evaluate([8, 0.5], '%')); + $this->assertTrue($GLOBALS['rpn_error']); + } + /** * @return array, 1: string}> */ diff --git a/tests/Unit/TholdGetCurrentvalTest.php b/tests/Unit/TholdGetCurrentvalTest.php index 4c1472cf..30b7168d 100644 --- a/tests/Unit/TholdGetCurrentvalTest.php +++ b/tests/Unit/TholdGetCurrentvalTest.php @@ -38,12 +38,14 @@ public static function setUpBeforeClass(): void { */ private function threshold(array $overrides = []) { return $overrides + [ + 'id' => 9, + 'thold_id' => 9, 'local_data_id' => 4, 'name' => 'traffic_in', 'data_source_type_id' => self::COUNTER, 'rrd_step' => 300, 'rrd_maximum' => 0, - 'lasttime' => 0, + 'lasttime' => 1700000000, 'oldvalue' => 100, ]; } @@ -81,6 +83,34 @@ public function testAbsoluteDividesTheReadingByTheStep(): void { $this->assertEqualsWithDelta(2, $this->currentValue($thold, 600), 1.0e-9); } + /** + * @return void + */ + public function testAbsoluteRejectsStaleAndOutOfOrderIntervals(): void { + foreach ([1000 + 86400, 900] as $sample_time) { + $thold = $this->threshold(['data_source_type_id' => self::ABSOLUTE, 'lasttime' => 1000]); + $reindexed = [4 => ['traffic_in' => 600]]; + $time_reindexed = [4 => $sample_time]; + $item = []; + $currenttime = 0; + + $this->assertSame('', thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime)); + } + } + + /** + * @return void + */ + public function testGaugeRemainsValidAcrossAGap(): void { + $thold = $this->threshold(['data_source_type_id' => self::GAUGE, 'lasttime' => 1000]); + $reindexed = [4 => ['traffic_in' => 42]]; + $time_reindexed = [4 => 1000 + 7 * 86400]; + $item = []; + $currenttime = 0; + + $this->assertSame(42, thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime)); + } + /** * @return void */ @@ -106,10 +136,10 @@ public function testCounterTreatsAPreviousReadingOfZeroAsReal(): void { /** * @return void */ - public function testCounterWithNoPreviousReadingYieldsZero(): void { - $thold = $this->threshold(['oldvalue' => '']); + public function testCounterWithNoPreviousReadingYieldsUnknown(): void { + $thold = $this->threshold(['lasttime' => 0, 'oldvalue' => '']); - $this->assertSame(0, $this->currentValue($thold, 600)); + $this->assertSame('', $this->currentValue($thold, 600)); } /** @@ -119,7 +149,7 @@ public function testCounterWithNoPreviousReadingYieldsZero(): void { * @return void */ public function testThirtyTwoBitWrapUsesTheCorrectModulus(): void { - $thold = $this->threshold(['oldvalue' => 4294967290, 'rrd_step' => 1]); + $thold = $this->threshold(['oldvalue' => 4294967290, 'rrd_step' => 1, 'lasttime' => 1700000299]); $this->assertEqualsWithDelta(11, $this->currentValue($thold, 5), 1.0e-9); } @@ -128,7 +158,7 @@ public function testThirtyTwoBitWrapUsesTheCorrectModulus(): void { * @return void */ public function testSixtyFourBitWrapUsesTheCorrectModulus(): void { - $thold = $this->threshold(['oldvalue' => '18446744073709551610', 'rrd_step' => 1]); + $thold = $this->threshold(['oldvalue' => '18446744073709551610', 'rrd_step' => 1, 'lasttime' => 1700000299]); $this->assertEqualsWithDelta(11, $this->currentValue($thold, 5), 1.0e-9); } @@ -140,7 +170,7 @@ public function testSixtyFourBitWrapUsesTheCorrectModulus(): void { * @return void */ public function testSixtyFourBitWrapAcceptsScientificNotation(): void { - $thold = $this->threshold(['oldvalue' => '1.8446744073709552E+19', 'rrd_step' => 1]); + $thold = $this->threshold(['oldvalue' => '1.8446744073709552E+19', 'rrd_step' => 1, 'lasttime' => 1700000299]); $this->assertEqualsWithDelta(5, $this->currentValue($thold, 5), 1.0e-9); } @@ -173,4 +203,653 @@ public function testMissingDataSourceYieldsTheNoValueSentinel(): void { $this->assertSame('', thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime)); } + + /** + * @return void + */ + public function testLowerUpperCombinationRejectsUnknownInputs(): void { + $thold = ['local_data_id' => 4, 'upper_ds' => 'upper']; + + $this->assertSame('', thold_calculate_lower_upper($thold, '', [4 => ['upper' => 5]])); + $this->assertSame('', thold_calculate_lower_upper($thold, 7, [4 => ['upper' => 'U']])); + $this->assertSame('', thold_calculate_lower_upper($thold, 7, [4 => []])); + $this->assertEqualsWithDelta((5 * 4294967296) + 7, thold_calculate_lower_upper($thold, 7, [4 => ['upper' => 5]]), 1.0e-9); + $this->assertEqualsWithDelta((2147483648.0 * 4294967296) + 7, thold_calculate_lower_upper($thold, 7, [4 => ['upper' => 2147483648]]), 1); + $this->assertEqualsWithDelta((4294967295.0 * 4294967296) + 7, thold_calculate_lower_upper($thold, 7, [4 => ['upper' => 4294967295]]), 1); + $this->assertSame('', thold_calculate_lower_upper($thold, 7, [4 => ['upper' => -1]])); + $this->assertSame('', thold_calculate_lower_upper($thold, 7, [4 => ['upper' => 4294967296]])); + } + + /** + * @return void + */ + public function testCdefAndNestedExpressionPreserveUnknownValues(): void { + $this->assertSame('', thold_build_cdef(1, '', 4, 5)); + $this->assertSame( + ['sample_row' => ['id' => 9, 'tcheck' => 1, 'lastread' => '', 'lasttime' => 1700000300, 'oldvalue' => 700], 'status_row' => null], + thold_polling_sample_row($this->threshold(), ['traffic_in' => 700], '', 1700000300) + ); + + $nested = $this->threshold([ + 'lasttime' => 1000, + 'rrd_heartbeat' => 600, + ]); + CactiStubs::willReturn('db_fetch_row_prepared', $nested); + $outer = $this->threshold(['expression' => '|ds:traffic_in|', 'lastread' => 2]); + $reindexed = [4 => ['traffic_in' => 700]]; + $time_reindexed = [4 => 1900]; + + $this->assertSame('', thold_calculate_expression($outer, '', $reindexed, $time_reindexed)); + $this->assertSame([], CactiStubs::$log); + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', $nested); + $time_reindexed[4] = 1300; + $this->assertEqualsWithDelta(2, thold_calculate_expression($outer, '', $reindexed, $time_reindexed), 1.0e-9); + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', []); + CactiStubs::willReturn('db_fetch_row_prepared', ['data_source_type_id' => self::COUNTER]); + CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); + CactiStubs::willReturn('rrdtool_execute', '1700000000'); + CactiStubs::willReturn('rrdtool_function_fetch', [ + 'data_source_names' => ['traffic_in'], + 'values' => [['1700000000' => 2.0]], + ]); + $this->assertEqualsWithDelta(2.0, thold_calculate_expression($outer, '', $reindexed, $time_reindexed), 1.0e-9); + $this->assertSame([], CactiStubs::$log); + + CactiStubs::reset(); + CactiStubs::$configOptions['dsstats_enable'] = 'on'; + CactiStubs::willReturn('db_fetch_row_prepared', []); + CactiStubs::willReturn('db_fetch_row_prepared', ['data_source_type_id' => self::COUNTER]); + CactiStubs::willReturn('db_fetch_cell_prepared', 3.5); + $this->assertEqualsWithDelta(3.5, thold_calculate_expression($outer, '', $reindexed, $time_reindexed), 1.0e-9); + $this->assertSame([], CactiStubs::callsTo('rrdtool_function_fetch')); + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', []); + CactiStubs::willReturn('db_fetch_row_prepared', ['data_source_type_id' => self::GAUGE]); + $this->assertSame('700', thold_calculate_expression($outer, '', $reindexed, $time_reindexed)); + $this->assertSame([], CactiStubs::callsTo('rrdtool_function_fetch')); + + foreach ([ + [], + ['data_source_names' => ['traffic_out'], 'values' => [['1700000000' => 2.0]]], + ] as $missing_fetch) { + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', []); + CactiStubs::willReturn('db_fetch_row_prepared', ['data_source_type_id' => self::COUNTER]); + CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); + CactiStubs::willReturn('rrdtool_execute', '1700000000'); + CactiStubs::willReturn('rrdtool_function_fetch', $missing_fetch); + $this->assertSame('', thold_calculate_expression($outer, '', $reindexed, $time_reindexed)); + } + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', []); + CactiStubs::willReturn('db_fetch_row_prepared', []); + $this->assertSame('', thold_calculate_expression($outer, '', $reindexed, $time_reindexed)); + + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', []); + $reindexed = []; + $this->assertSame('', thold_calculate_expression($outer, '', $reindexed, $time_reindexed)); + $this->assertStringContainsString('expression source traffic_in is unavailable', CactiStubs::$log[0]); + $log_call = CactiStubs::callsTo('cacti_log')[0]; + $this->assertSame('THOLD', $log_call['params'][2]); + $this->assertSame(POLLER_VERBOSITY_MEDIUM, $log_call['params'][3]); + } + + /** + * @return void + */ + public function testMissedSampleCarriesTheValueAndTimestampTogether(): void { + $thold = $this->threshold(['lasttime' => 1000, 'oldvalue' => 100]); + + $missed = thold_sample_persistence($thold, [], 1300); + $this->assertSame(['lasttime' => 1000, 'oldvalue' => 100], $missed); + $this->assertSame($missed, thold_sample_persistence($thold, ['traffic_in' => 'U'], 1300)); + $this->assertSame($missed, thold_sample_persistence($thold, ['traffic_in' => 'nan'], 1300)); + $this->assertSame($missed, thold_sample_persistence($thold, ['traffic_in' => ''], 1300)); + $this->assertSame( + ['lasttime' => 1600, 'oldvalue' => 700], + thold_sample_persistence($thold, ['traffic_in' => 700], 1600) + ); + $this->assertSame( + ['lasttime' => 1600, 'oldvalue' => '700'], + thold_sample_persistence($thold, ['traffic_in' => '700'], 1600) + ); + + $thold['lasttime'] = $missed['lasttime']; + $thold['oldvalue'] = $missed['oldvalue']; + $reindexed = [4 => ['traffic_in' => 700]]; + $time_reindexed = [4 => 1600]; + $item = []; + $currenttime = 0; + + $this->assertEqualsWithDelta( + 1, + thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime), + 1.0e-9 + ); + } + + /** + * A numeric raw sample that arrives after a gap beyond the effective + * heartbeat still re-anchors the pair to the current sample and time: + * thold_get_currentval() already rejected this cycle's rate using the + * pair as it stood before this call, so freezing lasttime/oldvalue here + * would buy no extra protection for the current read while permanently + * disqualifying every later poll, whose elapsed time is measured against + * that same frozen lasttime and only grows. A gauge has no rate baseline + * to protect, and the first-ever sample and a backward-clock re-anchor + * always advance the pair regardless of the gap. + * + * @return void + */ + public function testStaleGapReanchorsARateBearingPairAndAllOtherCasesStillAdvance(): void { + $thold = $this->threshold(['lasttime' => 1000, 'oldvalue' => 100, 'rrd_step' => 300]); + + // 1000 -> 1601: a 601s gap exceeds the 600s effective heartbeat (2 * 300s), + // but the pair still re-anchors so the next poll can recover. + $this->assertSame( + ['lasttime' => 1601, 'oldvalue' => 700], + thold_sample_persistence($thold, ['traffic_in' => 700], 1601) + ); + + // Right at the heartbeat boundary, the pair still advances. + $this->assertSame( + ['lasttime' => 1600, 'oldvalue' => 700], + thold_sample_persistence($thold, ['traffic_in' => 700], 1600) + ); + + // A gauge has no baseline to protect, so the same gap still advances it. + $gauge = $this->threshold(['data_source_type_id' => self::GAUGE, 'lasttime' => 1000, 'oldvalue' => 100, 'rrd_step' => 300]); + $this->assertSame( + ['lasttime' => 1601, 'oldvalue' => 700], + thold_sample_persistence($gauge, ['traffic_in' => 700], 1601) + ); + + // No prior sample at all: the gap check does not apply. + $firstSample = $this->threshold(['lasttime' => 0, 'oldvalue' => null, 'rrd_step' => 300]); + $this->assertSame( + ['lasttime' => 1601, 'oldvalue' => 700], + thold_sample_persistence($firstSample, ['traffic_in' => 700], 1601) + ); + + // A backward clock always re-anchors, even across a heartbeat-exceeding gap. + $backward = $this->threshold(['lasttime' => 5000, 'oldvalue' => 100, 'rrd_step' => 300]); + $this->assertSame( + ['lasttime' => 1601, 'oldvalue' => 700], + thold_sample_persistence($backward, ['traffic_in' => 700], 1601) + ); + } + + /** + * issue #815: a stale gap must not lock a rate-bearing threshold out of + * ever recovering. The first (gap) poll re-anchors the pair; the second + * poll, arriving at a normal interval from that new anchor, must produce + * a real numeric rate rather than being rejected as stale again. + * + * @return void + */ + public function testASecondPollAtANormalIntervalRecoversAfterAStaleGapReanchors(): void { + $thold = $this->threshold(['lasttime' => 1000, 'oldvalue' => 100, 'rrd_step' => 300]); + + // First (gap) poll: re-anchors instead of freezing. + $afterGap = thold_sample_persistence($thold, ['traffic_in' => 700], 1601); + $this->assertSame(['lasttime' => 1601, 'oldvalue' => 700], $afterGap); + + // Second poll, a normal 300s interval after the re-anchored pair. + $recovered = $this->threshold(['lasttime' => $afterGap['lasttime'], 'oldvalue' => $afterGap['oldvalue'], 'rrd_step' => 300]); + + $reindexed = [4 => ['traffic_in' => 900]]; + $timeReindexed = [4 => 1901]; + $item = []; + $currenttime = 0; + + // A COUNTER's delta is a per-second rate: (900 - 700) / (1901 - 1601). + $this->assertEqualsWithDelta( + 200 / 300, + thold_get_currentval($recovered, $reindexed, $timeReindexed, $item, $currenttime), + 1.0e-9 + ); + } + + /** + * The re-anchor warning fires only on the poll that first crosses the + * heartbeat, while the prior cycle's persisted rate (lastread) is still + * numeric. A source that stays stale poll after poll has already had its + * rate replaced with an unavailable state, so it must not keep logging, + * or a persistently flaky device floods medium-verbosity logs. + * + * @return void + */ + public function testStaleGapReanchorLogsOnlyOnceOnTheTransitionIntoUnavailable(): void { + $thold = $this->threshold(['lasttime' => 1000, 'oldvalue' => 100, 'rrd_step' => 300, 'lastread' => 50]); + + thold_sample_persistence($thold, ['traffic_in' => 700], 1601); + + $this->assertCount(1, CactiStubs::$log); + $this->assertStringContainsString('sample gap exceeded', CactiStubs::$log[0]); + + CactiStubs::reset(); + + // Same kind of gap, but the previous cycle already persisted the + // rate as unavailable: no further warning. + $stillStale = $this->threshold(['lasttime' => 1601, 'oldvalue' => 700, 'rrd_step' => 300, 'lastread' => '']); + + thold_sample_persistence($stillStale, ['traffic_in' => 900], 2500); + + $this->assertSame([], CactiStubs::$log); + } + + /** + * The effective heartbeat thold_sample_interval_eligible() compares + * against honors an explicitly configured rrd_heartbeat, and falls back + * to the poller interval alone when rrd_step is not a usable positive + * number - mirroring thold_get_currentval()'s own fallbacks. The pair + * always advances; the heartbeat comparison now only affects whether a + * re-anchor is logged. + * + * @return void + */ + public function testEligibilityHonorsAConfiguredHeartbeatAndFallsBackForAnInvalidRrdStep(): void { + $withHeartbeat = $this->threshold(['lasttime' => 1000, 'oldvalue' => 100, 'rrd_step' => 300, 'rrd_heartbeat' => 1800]); + + $this->assertSame( + ['lasttime' => 2800, 'oldvalue' => 700], + thold_sample_persistence($withHeartbeat, ['traffic_in' => 700], 2800) + ); + $this->assertSame( + ['lasttime' => 2801, 'oldvalue' => 700], + thold_sample_persistence($withHeartbeat, ['traffic_in' => 700], 2801) + ); + + $invalidStep = $this->threshold(['lasttime' => 1000, 'oldvalue' => 100, 'rrd_step' => 0]); + + $this->assertSame( + ['lasttime' => 1600, 'oldvalue' => 700], + thold_sample_persistence($invalidStep, ['traffic_in' => 700], 1600) + ); + $this->assertSame( + ['lasttime' => 1601, 'oldvalue' => 700], + thold_sample_persistence($invalidStep, ['traffic_in' => 700], 1601) + ); + } + + /** + * @return array + */ + public static function emptyMaximumProvider() { + return [ + 'integer zero' => [0, 1009000000100, 1009000000100 / 600], + 'empty string' => ['', 1009000000100, 1009000000100 / 600], + 'null maximum' => [null, 1009000000100, 1009000000100 / 600], + 'unknown maximum' => ['U', 1009000000100, 1009000000100 / 600], + 'unresolved if speed' => ['|query_ifSpeed|', 1009000000100, 1009000000100 / 600], + 'explicit maximum' => [20000000, 1015000000000, 1015000000000 / 600], + ]; + } + + /** + * @dataProvider emptyMaximumProvider + * + * @param int|string|null $maximum + * @param int $reading + * @param float $expected + * + * @return void + */ + public function testMultiIntervalDeltaScalesTheResetGuard($maximum, $reading, $expected): void { + if ($maximum === '|query_ifSpeed|') { + CactiStubs::willReturn('db_fetch_row_prepared', ['host_id' => 1, 'snmp_query_id' => 2, 'snmp_index' => 'eth0']); + CactiStubs::willReturn('db_fetch_cell_prepared', ''); + } + + $thold = $this->threshold(['lasttime' => 1000, 'oldvalue' => 1000000000000, 'rrd_maximum' => $maximum]); + $reindexed = [4 => ['traffic_in' => $reading]]; + $time_reindexed = [4 => 1600]; + $item = []; + $currenttime = 0; + + $this->assertEqualsWithDelta($expected, thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime), 1.0e-9); + } + + /** + * @return void + */ + public function testMultiIntervalWrapUsesTheWholeElapsedTime(): void { + $thold = $this->threshold(['lasttime' => 1000, 'oldvalue' => 4294967290, 'rrd_maximum' => 0]); + $reindexed = [4 => ['traffic_in' => 5]]; + $time_reindexed = [4 => 1600]; + $item = []; + $currenttime = 0; + + $this->assertEqualsWithDelta( + 11 / 600, + thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime), + 1.0e-9 + ); + } + + /** + * @return void + */ + public function testWrapResetGuardUsesTheEffectiveSampleInterval(): void { + CactiStubs::$configOptions['poller_interval'] = 300; + $thold = $this->threshold([ + 'lasttime' => 1700000000, + 'oldvalue' => 1000, + 'rrd_step' => 60, + 'rrd_maximum' => 0, + ]); + + $this->assertEqualsWithDelta(999 / 300, $this->currentValue($thold, 999), 1.0e-9); + } + + /** + * @return void + */ + public function testStaleCounterAndDeriveSamplesAreDiscarded(): void { + foreach ([self::COUNTER, self::DERIVE] as $type) { + $thold = $this->threshold(['data_source_type_id' => $type, 'lasttime' => 1000, 'oldvalue' => 100]); + $reindexed = [4 => ['traffic_in' => 700]]; + $time_reindexed = [4 => 1000 + 7 * 86400]; + $item = []; + $currenttime = 0; + + $this->assertSame('', thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime)); + } + } + + /** + * @return array + */ + public static function invalidRrdStepProvider() { + return [ + 'zero' => [0], + 'null' => [null], + 'non-numeric' => ['invalid'], + ]; + } + + /** + * @dataProvider invalidRrdStepProvider + * + * @param mixed $rrd_step + * + * @return void + */ + public function testInvalidRrdStepFallsBackToThePollerInterval($rrd_step): void { + CactiStubs::$configOptions['poller_interval'] = 300; + $thold = $this->threshold([ + 'data_source_type_id' => self::ABSOLUTE, + 'lasttime' => 0, + 'rrd_step' => $rrd_step, + ]); + + $this->assertEqualsWithDelta(2, $this->currentValue($thold, 600), 1.0e-9); + } + + /** + * @return void + */ + public function testEvaluationCadenceMayExceedTheRrdStep(): void { + CactiStubs::$configOptions['poller_interval'] = 300; + $thold = $this->threshold(['rrd_step' => 60]); + + $this->assertEqualsWithDelta(2, $this->currentValue($thold, 700), 1.0e-9); + } + + /** + * @return void + */ + public function testRrdHeartbeatControlsGapAcceptanceWithASafeFloor(): void { + $accepted = $this->threshold(['lasttime' => 1000, 'rrd_heartbeat' => 1800]); + $reindexed = [4 => ['traffic_in' => 700]]; + $time_reindexed = [4 => 1900]; + $item = []; + $currenttime = 0; + + $this->assertEqualsWithDelta( + 600 / 900, + thold_get_currentval($accepted, $reindexed, $time_reindexed, $item, $currenttime), + 1.0e-9 + ); + + $floored = $accepted; + $floored['rrd_heartbeat'] = 120; + $time_reindexed[4] = 1300; + $this->assertEqualsWithDelta( + 2, + thold_get_currentval($floored, $reindexed, $time_reindexed, $item, $currenttime), + 1.0e-9 + ); + + $time_reindexed[4] = 1700; + $this->assertSame('', thold_get_currentval($floored, $reindexed, $time_reindexed, $item, $currenttime)); + } + + /** + * @return void + */ + public function testNonNumericSampleTimeUsesTheRrdStep(): void { + $thold = $this->threshold(); + $reindexed = [4 => ['traffic_in' => 700]]; + $time_reindexed = [4 => 'U']; + $item = []; + $currenttime = 0; + + $this->assertEqualsWithDelta(2, thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime), 1.0e-9); + } + + /** + * @return void + */ + public function testFirstAbsoluteSampleUsesTheValidatedPollerInterval(): void { + CactiStubs::$configOptions['poller_interval'] = 300; + $thold = $this->threshold([ + 'data_source_type_id' => self::ABSOLUTE, + 'lasttime' => 0, + 'rrd_step' => 0, + ]); + + $this->assertEqualsWithDelta(2, $this->currentValue($thold, 600), 1.0e-9); + } + + /** + * @return void + */ + public function testOutOfOrderCounterAndDeriveSamplesAreUnknown(): void { + foreach ([self::COUNTER, self::DERIVE] as $type) { + $thold = $this->threshold(['data_source_type_id' => $type, 'lasttime' => 1700000400]); + $reindexed = [4 => ['traffic_in' => 700]]; + $time_reindexed = [4 => 1700000300]; + $item = []; + $currenttime = 0; + + $this->assertSame('', thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime)); + } + + $this->assertSame( + ['lasttime' => 1700000300, 'oldvalue' => 700], + thold_sample_persistence($thold, ['traffic_in' => 700], 1700000300) + ); + $this->assertSame( + ['lasttime' => 1700000400, 'oldvalue' => 100], + thold_sample_persistence($thold, ['traffic_in' => 700], 1700000400) + ); + + CactiStubs::reset(); + $this->assertSame([ + 'sample_row' => ['id' => 9, 'tcheck' => 1, 'lastread' => '', 'lasttime' => 1700000300, 'oldvalue' => 700], + 'status_row' => null, + ], thold_polling_sample_row($thold, ['traffic_in' => 700], '', 1700000300)); + $this->assertCount(1, CactiStubs::$log); + $this->assertStringContainsString('clock moved backwards', CactiStubs::$log[0]); + + CactiStubs::reset(); + $this->assertTrue(thold_daemon_persist_sample($thold, ['traffic_in' => 700], '', 1700000300)); + $this->assertCount(1, CactiStubs::$log); + $this->assertStringContainsString('clock moved backwards', CactiStubs::$log[0]); + $call = end(CactiStubs::$calls); + $this->assertSame([1, '', 1700000300, 700, 9], $call['params']); + + $reanchored = $this->threshold(['lasttime' => 1700000300, 'oldvalue' => 700]); + $reindexed = [4 => ['traffic_in' => 1000]]; + $time_reindexed = [4 => 1700000600]; + $item = []; + $currenttime = 0; + $this->assertEqualsWithDelta( + 1, + thold_get_currentval($reanchored, $reindexed, $time_reindexed, $item, $currenttime), + 1.0e-9 + ); + } + + /** + * @return void + */ + public function testDeriveWithAnInvalidPriorValueIsUnknown(): void { + $thold = $this->threshold([ + 'data_source_type_id' => self::DERIVE, + 'oldvalue' => 'U', + ]); + + $this->assertSame('', $this->currentValue($thold, 700)); + } + + /** + * @return void + */ + public function testDaemonPersistsThePairWithBoundParameters(): void { + $thold = $this->threshold(['lasttime' => 1000, 'oldvalue' => 100]); + + $this->assertTrue(thold_daemon_persist_sample($thold, [], '', 1300)); + $call = end(CactiStubs::$calls); + $this->assertStringContainsString('lasttime = FROM_UNIXTIME(?)', $call['sql']); + $this->assertSame([1, '', 1000, 100, 9], $call['params']); + + CactiStubs::reset(); + $this->assertTrue(thold_daemon_persist_sample($thold, ['traffic_in' => 700], 2, 1600)); + $call = end(CactiStubs::$calls); + $this->assertSame([1, 2, 1600, 700, 9], $call['params']); + } + + /** + * @return void + */ + public function testDaemonPropagatesPersistenceFailure(): void { + CactiStubs::willReturn('db_execute_prepared', false); + $this->assertFalse(thold_daemon_persist_sample( + $this->threshold(['lasttime' => 1000]), + [], + '', + 1300 + )); + + CactiStubs::reset(); + CactiStubs::willReturn('db_execute_prepared', false); + $this->assertFalse(thold_daemon_persist_sample( + $this->threshold(['lasttime' => 0]), + [], + '', + 1300 + )); + } + + /** + * @return void + */ + public function testNeverSampledThresholdLeavesTheTimestampPairUntouched(): void { + $thold = $this->threshold(['lasttime' => 0, 'oldvalue' => null]); + + $this->assertSame( + ['lasttime' => 0, 'oldvalue' => null], + thold_sample_persistence($thold, ['traffic_in' => 'U'], 1300) + ); + $this->assertTrue(thold_daemon_persist_sample($thold, ['traffic_in' => 'U'], '', 1300)); + $call = end(CactiStubs::$calls); + $this->assertStringNotContainsString('FROM_UNIXTIME', $call['sql']); + $this->assertStringNotContainsString('oldvalue', $call['sql']); + $this->assertSame([1, '', 9], $call['params']); + + CactiStubs::reset(); + $this->assertSame( + ['sample_row' => null, 'status_row' => ['id' => 9, 'tcheck' => 1, 'lastread' => '']], + thold_polling_sample_row($thold, ['traffic_in' => 'U'], '', 1300) + ); + $this->assertSame([], CactiStubs::$calls); + } + + /** + * @return void + */ + public function testPollerBuildsTheSamePersistedPair(): void { + $thold = $this->threshold(['lasttime' => 1000, 'oldvalue' => 100]); + + $this->assertSame([ + 'sample_row' => ['id' => 9, 'tcheck' => 1, 'lastread' => 2, 'lasttime' => 1000, 'oldvalue' => 100], + 'status_row' => null, + ], thold_polling_sample_row($thold, [], 2, 1300)); + $this->assertSame([ + 'sample_row' => ['id' => 9, 'tcheck' => 1, 'lastread' => 2, 'lasttime' => 1600, 'oldvalue' => 700], + 'status_row' => null, + ], thold_polling_sample_row($thold, ['traffic_in' => 700], 2, 1600)); + $this->assertSame([ + 'sample_row' => ['id' => 9, 'tcheck' => 1, 'lastread' => 2, 'lasttime' => 1000, 'oldvalue' => null], + 'status_row' => null, + ], thold_polling_sample_row($this->threshold(['lasttime' => 1000, 'oldvalue' => null]), [], 2, 1300)); + } + + /** + * @return void + */ + public function testMissingPersistenceKeysFailClosed(): void { + $this->assertSame( + ['lasttime' => 0, 'oldvalue' => null], + thold_sample_persistence([], ['traffic_in' => 700], 1600) + ); + $this->assertFalse(thold_daemon_persist_sample([], ['traffic_in' => 700], 2, 1600)); + $this->assertSame( + ['sample_row' => null, 'status_row' => null], + thold_polling_sample_row([], ['traffic_in' => 700], 2, 1600) + ); + } + + /** + * @return void + */ + public function testUnavailableSampleLogsOnlyOnTheStateTransition(): void { + $thold = $this->threshold([ + 'lastread' => 12, + 'name_cache' => 'Traffic in', + ]); + + thold_polling_sample_row($thold, [], '', 1700000300); + $this->assertCount(1, CactiStubs::$log); + $log_call = CactiStubs::callsTo('cacti_log')[0]; + $this->assertSame('THOLD', $log_call['params'][2]); + $this->assertSame(POLLER_VERBOSITY_MEDIUM, $log_call['params'][3]); + + $thold['lastread'] = ''; + thold_polling_sample_row($thold, [], '', 1700000600); + $this->assertCount(1, CactiStubs::$log); + } + + /** + * @return void + */ + public function testPollerCleanupRunsForEitherBatchType(): void { + thold_polling_cleanup(false); + $this->assertSame([], CactiStubs::$calls); + + CactiStubs::willReturn('db_affected_rows', 1); + thold_polling_cleanup(true); + $this->assertSame('db_execute_prepared', CactiStubs::$calls[0]['fn']); + $this->assertStringContainsString('local_data_id = 0', CactiStubs::$calls[0]['sql']); + $this->assertArrayHasKey('time_last_change_thold', CactiStubs::$configOptions); + } } diff --git a/tests/Unit/TholdReplaceThresholdTagsTest.php b/tests/Unit/TholdReplaceThresholdTagsTest.php index 3bcc6ec7..c43fc1b0 100644 --- a/tests/Unit/TholdReplaceThresholdTagsTest.php +++ b/tests/Unit/TholdReplaceThresholdTagsTest.php @@ -75,11 +75,12 @@ private function device(array $overrides = []) { * @param array $device * @param bool $shell * @param mixed $currentval + * @param string $dataSourceName * * @return string */ - private function substitute($text, array $thold, array $device, $shell, $currentval = 42) { - return thold_replace_threshold_tags($text, $thold, $device, $currentval, 7, 'traffic_in', $shell); + private function substitute($text, array $thold, array $device, $shell, $currentval = 42, $dataSourceName = 'traffic_in') { + return thold_replace_threshold_tags($text, $thold, $device, $currentval, 7, $dataSourceName, $shell); } /** @@ -117,6 +118,30 @@ public function testShellModeQuotesEveryDeviceDerivedTag($tag, $column, $source) $this->assertStringNotContainsString('alert ; touch', $result); } + /** + * A device-controlled value (e.g. description) can contain another tag's + * literal placeholder text (e.g. ""). Every value is + * substituted in a single strtr() pass over the original text, so that + * literal text is never re-scanned and substituted a second time - it + * can't land a later value's shell metacharacters outside of its own + * quoting. + * + * @return void + */ + public function testShellModeDoesNotReSubstituteATagLiteralInsideAnotherValue(): void { + $thold = $this->threshold(); + $device = $this->device([ + 'description' => '', + 'hostname' => '; touch /tmp/pwned', + ]); + + $result = $this->substitute('/usr/bin/alert ', $thold, $device, true); + + $this->assertStringContainsString(escapeshellarg(''), $result); + $this->assertStringContainsString(escapeshellarg('; touch /tmp/pwned'), $result); + $this->assertStringNotContainsString("''; touch", $result); + } + /** * @dataProvider deviceDerivedTagProvider * @@ -183,6 +208,29 @@ public function testEmailModeLeavesTheCurrentValueUnquoted(): void { $this->assertSame('value=42', $result); } + /** + * The data source name is read straight from the data source table; a + * name containing shell metacharacters must not be able to break out of + * the configured trigger command. + * + * @return void + */ + public function testShellModeQuotesTheDataSourceName(): void { + $result = $this->substitute('/usr/bin/alert ', $this->threshold(), $this->device(), true, 42, '; touch /tmp/pwned'); + + $this->assertStringContainsString(escapeshellarg('; touch /tmp/pwned'), $result); + $this->assertStringNotContainsString('alert ; touch', $result); + } + + /** + * @return void + */ + public function testEmailModeLeavesTheDataSourceNameUnquoted(): void { + $result = $this->substitute('ds=', $this->threshold(), $this->device(), false, 42, "O'Brien"); + + $this->assertSame("ds=O'Brien", $result); + } + /** * @return void */ @@ -245,6 +293,25 @@ public function testUrlTagRendersALinkToTheGraph(): void { $this->assertStringContainsString('graph.php?local_graph_id=7', $result); } + /** + * The rendered ... markup contains literal single + * quotes, so in $shell mode the whole substituted value must be quoted + * as one token or those quotes would terminate the command early. + * + * @return void + */ + public function testShellModeQuotesTheUrlTag(): void { + CactiStubs::$configOptions['base_url'] = 'http://cacti.example.org'; + + $result = $this->substitute('/usr/bin/alert ', $this->threshold(), $this->device(), true); + + $this->assertStringContainsString( + escapeshellarg("" . __('Link to Graph in Cacti', 'thold') . ''), + $result + ); + $this->assertStringNotContainsString("alert 'router1', 'hostname' => '10.0.0.1']); + + $this->assertSame('router1', thold_substitute_host_data('|host_description|', '|', '|', 2)); + + // The second call must not query again: it is served from the cache + // populated by the first. + $this->assertCount(1, CactiStubs::callsTo('db_fetch_row_prepared')); + $this->assertSame('router1', thold_substitute_host_data('|host_description|', '|', '|', 2)); + $this->assertCount(1, CactiStubs::callsTo('db_fetch_row_prepared')); + } + + /** + * @return void + */ + public function testCachedHostColumnIsEscapedInShellMode(): void { + $malicious = "evil'; touch /tmp/pwned; echo '"; + CactiStubs::willReturn('db_fetch_row_prepared', ['description' => $malicious, 'hostname' => '10.0.0.1']); + + $result = thold_substitute_host_data('|host_description|', '|', '|', 3, true); + + $this->assertSame(escapeshellarg($malicious), $result); + } + + /** + * |host_management_ip| is not a plain device column: it falls through to + * the hostname-substitution path rather than the cache-hit return. + * + * @return void + */ + public function testManagementIpFallsThroughToTheHostnameSubstitution(): void { + CactiStubs::willReturn('db_fetch_row_prepared', ['description' => 'router1', 'hostname' => '10.0.0.1']); + + $this->assertSame('ping 10.0.0.1', thold_substitute_host_data('ping |host_management_ip|', '|', '|', 4)); + } + + /** + * @return void + */ + public function testManagementIpIsEscapedInShellMode(): void { + $malicious = "10.0.0.1'; touch /tmp/pwned; echo '"; + CactiStubs::willReturn('db_fetch_row_prepared', ['description' => 'router1', 'hostname' => $malicious]); + + $result = thold_substitute_host_data('ping |host_management_ip|', '|', '|', 5, true); + + $this->assertSame('ping ' . escapeshellarg($malicious), $result); + } + + /** + * The custom_* tokens come from data source input fields (e.g. an SNMP + * community string), which is data an operator, not an admin, controls. + * + * @return void + */ + public function testCustomDataValueIsEscapedInShellMode(): void { + $malicious = "public'; touch /tmp/pwned; echo '"; + + CactiStubs::willReturn('db_fetch_assoc', [['id' => 9]]); + CactiStubs::willReturn('db_fetch_assoc_prepared', [ + ['name' => 'snmp_community', 'value' => $malicious], + ]); + + $result = thold_substitute_custom_data('alert |custom_snmp_community|', '|', '|', 4, true); + + $this->assertSame('alert ' . escapeshellarg($malicious), $result); + } + + /** + * @return void + */ + public function testCustomDataValueIsUnescapedOutsideShellMode(): void { + CactiStubs::willReturn('db_fetch_assoc', [['id' => 9]]); + CactiStubs::willReturn('db_fetch_assoc_prepared', [ + ['name' => 'snmp_community', 'value' => 'public'], + ]); + + $result = thold_substitute_custom_data('alert |custom_snmp_community|', '|', '|', 4, false); + + $this->assertSame('alert public', $result); + } + + /** + * A single strtr() pass replaces against the original string only and + * never re-scans inserted values, so one field's value containing + * another field's literal token can't be substituted a second time + * outside of its own quoting. + * + * @return void + */ + public function testShellModeDoesNotReSubstituteATokenLiteralInsideAnotherValue(): void { + $malicious = "'; touch /tmp/pwned; echo '"; + + CactiStubs::willReturn('db_fetch_assoc', [['id' => 9]]); + CactiStubs::willReturn('db_fetch_assoc_prepared', [ + ['name' => 'description', 'value' => '|custom_secret|'], + ['name' => 'secret', 'value' => $malicious], + ]); + + $result = thold_substitute_custom_data('cmd |custom_description| |custom_secret|', '|', '|', 4, true); + + $this->assertSame( + 'cmd ' . escapeshellarg('|custom_secret|') . ' ' . escapeshellarg($malicious), + $result + ); + } +} diff --git a/tests/Unit/ThresholdHiLowCharacterizationTest.php b/tests/Unit/ThresholdHiLowCharacterizationTest.php index a392d9ab..91b18a23 100644 --- a/tests/Unit/ThresholdHiLowCharacterizationTest.php +++ b/tests/Unit/ThresholdHiLowCharacterizationTest.php @@ -255,9 +255,14 @@ public function testMaintenanceWindowSuppressesNotification(): void { * @return void */ public function testUnknownReadingEmitsNoAlert(): void { - $outcome = $this->bounded(['lastread' => 'U'])->poll(); + $outcome = $this->bounded([ + 'lastread' => 'U', + 'thold_alert' => 2, + 'thold_fail_count' => 3, + ])->poll(); $this->assertSame(0, $outcome->mailCount()); + $this->assertNull($outcome->persistedAlertState()); } /** diff --git a/tests/Unit/ThresholdTimeBasedCharacterizationTest.php b/tests/Unit/ThresholdTimeBasedCharacterizationTest.php index c231072c..6a531213 100644 --- a/tests/Unit/ThresholdTimeBasedCharacterizationTest.php +++ b/tests/Unit/ThresholdTimeBasedCharacterizationTest.php @@ -106,6 +106,26 @@ public function testRestoralResetsTheFailCounts(): void { $this->assertSame(['alert' => 0, 'warning' => 0], $outcome->persistedFailCounts()); } + /** + * The early-restoral branch (neither fail count reached its trigger) must + * apply the same acknowledgment-reset side effect as the two branches + * above it, or an acknowledged threshold that recovers before retriggering + * stays acknowledged forever. + * + * @return void + */ + public function testRestoralClearsAcknowledgmentWhenResetAckEnabled(): void { + $outcome = $this->bounded([ + 'lastread' => 50, + 'thold_alert' => STAT_HI, + 'thold_fail_count' => 3, + 'acknowledgment' => 'on', + 'reset_ack' => 'on', + ])->poll(); + + $this->assertTrue($outcome->acknowledgmentCleared()); + } + /** * @return void */ @@ -141,9 +161,14 @@ public function testAcknowledgedThresholdDoesNotMailOnBreach(): void { * @return void */ public function testUnknownReadingEmitsNoAlert(): void { - $outcome = $this->bounded(['lastread' => 'U'])->poll(); + $outcome = $this->bounded([ + 'lastread' => 'U', + 'thold_alert' => 2, + 'thold_fail_count' => 3, + ])->poll(); $this->assertSame(0, $outcome->mailCount()); + $this->assertNull($outcome->persistedAlertState()); } /** diff --git a/tests/bin/patch-coverage.php b/tests/bin/patch-coverage.php index 07d45250..c23aa4b1 100644 --- a/tests/bin/patch-coverage.php +++ b/tests/bin/patch-coverage.php @@ -163,6 +163,11 @@ function changed_lines($base_ref) { // thold_notification_queue_status_cells() helper. 'notify_queue.php', 'thold_notify.php', + // Daemon entry point: does CLI bootstrap, pcntl signal handling, and + // argv parsing at the top level, so it cannot be require()'d into the + // isolated unit process. Its persistence/evaluation logic lives in the + // covered thold_daemon_persist_sample()/thold_check_threshold() helpers. + 'thold_process.php', ]; $unmeasured = array_values(array_diff(array_keys($changed), array_keys($measured))); $unexpected_unmeasured = array_values(array_diff($unmeasured, $unmeasured_allowlist)); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 07523500..b5fab151 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -55,7 +55,8 @@ throw new RuntimeException("Expected Cacti version file is empty: $expected"); } -if ($cacti_version !== $expected_version) { +// The CI workflow tracks a moving branch (1.2.x or develop) rather than a pinned release, so any actual version is accepted. +if (!in_array($expected_version, ['1.2.x', 'develop'], true) && $cacti_version !== $expected_version) { throw new RuntimeException("Expected Cacti $expected_version, found $cacti_version in $version"); } @@ -65,6 +66,10 @@ require_once __DIR__ . '/Helpers/ThresholdOutcome.php'; require_once __DIR__ . '/Helpers/ThresholdScenario.php'; +if (!defined('POLLER_VERBOSITY_MEDIUM')) { + define('POLLER_VERBOSITY_MEDIUM', 3); +} + /* * base_path has to point at the Cacti root two levels above this plugin: * thold_functions.php builds include paths from it at runtime. @@ -275,6 +280,7 @@ function __esc($text) { if (!function_exists('cacti_log')) { function cacti_log($message, $output = false, $environ = 'CMDPHP', $level = 0) { CactiStubs::$log[] = $message; + CactiStubs::record('cacti_log', '', [$message, $output, $environ, $level]); } } @@ -354,6 +360,20 @@ function expand_title($host_id, $snmp_query_id, $snmp_index, $title) { } } +if (!function_exists('substitute_host_data')) { + function substitute_host_data($string, $l_escape_string, $r_escape_string, $host_id) { + CactiStubs::record('substitute_host_data', $string); + + return CactiStubs::nextReturn('substitute_host_data', $string); + } +} + +if (!function_exists('null_out_substitutions')) { + function null_out_substitutions($string) { + return CactiStubs::nextReturn('null_out_substitutions', $string); + } +} + if (!function_exists('get_graph_title')) { function get_graph_title($local_graph_id) { return CactiStubs::nextReturn('get_graph_title', 'Traffic - eth0'); @@ -493,6 +513,34 @@ function api_plugin_hook($name, $data = '') { } } +if (!function_exists('api_plugin_register_hook')) { + function api_plugin_register_hook($name, $hook, $function, $file, $status = '') { + CactiStubs::record('api_plugin_register_hook', $hook, ['name' => $name, 'function' => $function, 'file' => $file]); + + return true; + } +} + +if (!function_exists('api_plugin_register_realm')) { + function api_plugin_register_realm($name, $files, $title, $navigate = 0) { + CactiStubs::record('api_plugin_register_realm', $files, ['name' => $name, 'title' => $title]); + + return true; + } +} + +if (!function_exists('api_plugin_enable_hooks')) { + function api_plugin_enable_hooks($name) { + CactiStubs::record('api_plugin_enable_hooks', $name); + } +} + +if (!function_exists('get_current_page')) { + function get_current_page() { + return CactiStubs::nextReturn('get_current_page', ''); + } +} + if (!function_exists('api_user_realm_auth')) { function api_user_realm_auth($filename = '') { return CactiStubs::nextReturn('api_user_realm_auth', true); @@ -519,6 +567,14 @@ function rrdtool_function_interface_speed($data_local) { } } +if (!function_exists('substitute_snmp_query_data')) { + function substitute_snmp_query_data($value, $host_id, $snmp_query_id, $snmp_index) { + CactiStubs::record('substitute_snmp_query_data', (string) $value, [$host_id, $snmp_query_id, $snmp_index]); + + return CactiStubs::nextReturn('substitute_snmp_query_data', $value); + } +} + if (!function_exists('get_timeinstate')) { function get_timeinstate($host) { return CactiStubs::nextReturn('get_timeinstate', '1 day'); @@ -565,6 +621,31 @@ function number_format_i18n($number, $decimals = 0, $baseu = 1000) { define('CACTI_PATH_BASE', $GLOBALS['config']['base_path']); } +if (!function_exists('plugin_test_read_source')) { + /** + * Read a plugin source file's raw contents, relative to the plugin root. + * + * @param string $relative_file Plugin file, relative to the plugin root. + * + * @return string + */ + function plugin_test_read_source($relative_file) { + $path = realpath(__DIR__ . '/../' . $relative_file); + + if ($path === false) { + throw new RuntimeException("Unable to resolve required file: {$relative_file}"); + } + + $contents = file_get_contents($path); + + if ($contents === false) { + throw new RuntimeException("Unable to read required file: {$relative_file}"); + } + + return $contents; + } +} + /** * Load a plugin source file at global scope. * diff --git a/thold_functions.php b/thold_functions.php index 37f19fa3..4461475b 100644 --- a/thold_functions.php +++ b/thold_functions.php @@ -415,7 +415,12 @@ function thold_expression_math_rpn($operator, &$stack) { // enclosing switch($operator) and skip the array_push below). $v3 = 0; $rpn_evaled = true; - } elseif ($v1 == 0 && ($operator == '/' || $operator == '%')) { + } elseif ($v1 == 0 && $operator == '/') { + cacti_log('ERROR: RPN value: v1 can not be "0" when the operator is "' . $operator . '". Stack:"' . implode(',', $orig_stack) . '"', false, 'THOLD'); + $rpn_error = true; + } elseif ($operator == '%' && (int) $v1 == 0) { + // The % operator truncates both operands to int, so a fractional + // divisor such as 0.5 becomes 0 even though $v1 == 0 is false. cacti_log('ERROR: RPN value: v1 can not be "0" when the operator is "' . $operator . '". Stack:"' . implode(',', $orig_stack) . '"', false, 'THOLD'); $rpn_error = true; } @@ -896,9 +901,246 @@ function thold_counter_wrap_delta($oldvalue, $newvalue) { return (4294967296 - $oldvalue) + $newvalue; } +/** + * Whether a valid current sample predates the stored sample clock. + * + * @param array $thold_data + * @param array $item + * @param int $currenttime + * + * @return bool + */ +function thold_sample_clock_moved_backward(array $thold_data, array $item, $currenttime) { + $name = (string) ($thold_data['name'] ?? ''); + $currenttime = (int) $currenttime; + $lasttime = (int) ($thold_data['lasttime'] ?? 0); + + return $name !== '' + && $currenttime > 0 + && $lasttime > 0 + && $currenttime < $lasttime + && isset($item[$name]) + && is_numeric($item[$name]); +} + +/** + * Whether the interval ending at $currenttime is within the effective + * heartbeat thold_get_currentval() uses to trust a counter/derive/absolute + * baseline. thold_sample_persistence() uses this only to decide whether to + * log a stale-gap re-anchor; the pair itself always advances. + * + * The caller must already know $currenttime is strictly after a real prior + * sample (lasttime > 0, currenttime > lasttime); this only computes the + * heartbeat comparison thold_get_currentval() applies in that situation. + * + * @param array $thold_data + * @param int $currenttime + * + * @return bool + */ +function thold_sample_interval_eligible(array $thold_data, $currenttime) { + $elapsed = ((int) $currenttime) - (int) ($thold_data['lasttime'] ?? 0); + + $poller_interval = read_config_option('poller_interval'); + + if (!is_numeric($poller_interval) || $poller_interval <= 0) { + $poller_interval = 300; + } + + $rrd_step = is_numeric($thold_data['rrd_step'] ?? null) && $thold_data['rrd_step'] > 0 + ? (float) $thold_data['rrd_step'] + : 0.0; + + $sample_interval = max($rrd_step, (float) $poller_interval); + $rrd_heartbeat = is_numeric($thold_data['rrd_heartbeat'] ?? null) && $thold_data['rrd_heartbeat'] > 0 + ? max((float) $thold_data['rrd_heartbeat'], 2 * $sample_interval) + : 2 * $sample_interval; + + return $elapsed <= $rrd_heartbeat; +} + +/** + * Persist a raw sample and its timestamp as one causal pair. + * + * `$thold_data` must provide `name`, `lasttime`, and `oldvalue`; absent values + * fail closed to an unavailable prior sample. The pair always re-anchors to + * the current sample when one is available: thold_get_currentval() already + * rejected the rate for this cycle using the pair as it stood before this + * call, so freezing lasttime/oldvalue here would buy no extra protection for + * the current read while permanently disqualifying every later poll, whose + * elapsed time is measured against that same frozen lasttime and only grows. + * Re-anchoring instead lets the very next poll compute a fresh, valid rate. + * + * @param array $thold_data + * @param array $item + * @param int $currenttime + * + * @return array{lasttime:mixed,oldvalue:mixed} + */ +function thold_sample_persistence(array $thold_data, array $item, $currenttime) { + $name = (string) ($thold_data['name'] ?? ''); + $currenttime = (int) $currenttime; + + $lasttime = (int) ($thold_data['lasttime'] ?? 0); + + if ($name !== '' && $currenttime > 0 && $currenttime !== $lasttime && isset($item[$name]) && is_numeric($item[$name])) { + $is_rate_bearing = in_array((int) ($thold_data['data_source_type_id'] ?? 0), [2, 3, 4], true); + $clock_moved_back = thold_sample_clock_moved_backward($thold_data, $item, $currenttime); + + if ($clock_moved_back) { + cacti_log(sprintf( + 'WARNING: Threshold %s sample clock moved backwards; re-anchoring its value and timestamp.', + $thold_data['id'] ?? ($thold_data['thold_id'] ?? 'unknown') + ), false, 'THOLD', POLLER_VERBOSITY_MEDIUM); + } elseif ($lasttime > 0 && $is_rate_bearing && !thold_sample_interval_eligible($thold_data, $currenttime) && is_numeric($thold_data['lastread'] ?? null)) { + // Only the poll that first crosses the heartbeat logs: once this + // source is already persisted as unavailable, every later poll + // re-anchors again (its own gap is now measured from here) but + // stays quiet, or a persistently flaky device would flood the + // log at medium verbosity. + cacti_log(sprintf( + 'WARNING: Threshold %s sample gap exceeded the effective heartbeat; re-anchoring its rate baseline so the next poll can recover.', + $thold_data['id'] ?? ($thold_data['thold_id'] ?? 'unknown') + ), false, 'THOLD', POLLER_VERBOSITY_MEDIUM); + } + + return ['lasttime' => $currenttime, 'oldvalue' => $item[$name]]; + } + + return [ + 'lasttime' => $lasttime, + 'oldvalue' => $thold_data['oldvalue'] ?? null, + ]; +} + +/** + * Log only the transition from a numeric result to an unavailable result. + * + * @param array $thold_data + * @param mixed $currentval + * + * @return void + */ +function thold_log_unavailable_transition(array $thold_data, $currentval) { + if (is_numeric($currentval) || !is_numeric($thold_data['lastread'] ?? null)) { + return; + } + + cacti_log(sprintf( + 'WARNING: Threshold %s (%s) current sample is unavailable; preserving its alert state.', + $thold_data['id'] ?? ($thold_data['thold_id'] ?? 'unknown'), + $thold_data['name_cache'] ?? ($thold_data['thold_name'] ?? ($thold_data['name'] ?? 'unknown')) + ), false, 'THOLD', POLLER_VERBOSITY_MEDIUM); +} + +/** + * Persist one daemon sample without manufacturing a zero SQL timestamp. + * + * @param array $thold_data + * @param array $item + * @param mixed $currentval + * @param int $currenttime + * + * @return bool + */ +function thold_daemon_persist_sample(array $thold_data, array $item, $currentval, $currenttime) { + $id = (int) ($thold_data['thold_id'] ?? 0); + $tcheck = 1; + + if ($id <= 0) { + return false; + } + + $sample = thold_sample_persistence($thold_data, $item, $currenttime); + + if (!thold_sample_clock_moved_backward($thold_data, $item, $currenttime)) { + thold_log_unavailable_transition($thold_data, $currentval); + } + + if ($sample['lasttime'] <= 0) { + return db_execute_prepared('UPDATE thold_data + SET tcheck = ?, lastread = ? + WHERE id = ?', + [$tcheck, $currentval, $id]); + } + + return db_execute_prepared('UPDATE thold_data + SET tcheck = ?, lastread = ?, + lasttime = FROM_UNIXTIME(?), oldvalue = ? + WHERE id = ?', + [$tcheck, $currentval, $sample['lasttime'], $sample['oldvalue'], $id]); +} + +/** + * Build one pure poller batching result for sample or status-only updates. + * + * @param array $thold_data + * @param array $item + * @param mixed $currentval + * @param int $currenttime + * + * @return array{sample_row:array{id:int,tcheck:int,lastread:mixed,lasttime:mixed,oldvalue:mixed}|null,status_row:array{id:int,tcheck:int,lastread:mixed}|null} + */ +function thold_polling_sample_row(array $thold_data, array $item, $currentval, $currenttime) { + $id = (int) ($thold_data['id'] ?? 0); + $tcheck = 1; + + if ($id <= 0) { + return ['sample_row' => null, 'status_row' => null]; + } + + $sample = thold_sample_persistence($thold_data, $item, $currenttime); + + if (!thold_sample_clock_moved_backward($thold_data, $item, $currenttime)) { + thold_log_unavailable_transition($thold_data, $currentval); + } + + if ($sample['lasttime'] <= 0) { + return [ + 'sample_row' => null, + 'status_row' => ['id' => $id, 'tcheck' => $tcheck, 'lastread' => $currentval], + ]; + } + + return [ + 'sample_row' => [ + 'id' => $id, + 'tcheck' => $tcheck, + 'lastread' => $currentval, + 'lasttime' => $sample['lasttime'], + 'oldvalue' => $sample['oldvalue'], + ], + 'status_row' => null, + ]; +} + +/** + * Remove rows deleted during polling after either update batch ran. + * + * @param bool $has_updates + * + * @return void + */ +function thold_polling_cleanup($has_updates) { + if (!$has_updates) { + return; + } + + db_execute_prepared('DELETE FROM thold_data WHERE local_data_id = 0'); + + if (db_affected_rows() > 0) { + set_config_option('time_last_change_thold', time()); + } +} + function thold_get_currentval(&$thold_data, &$rrd_reindexed, &$rrd_time_reindexed, &$item, &$currenttime) { // adjust the polling interval by the last read, if applicable $currenttime = $rrd_time_reindexed[$thold_data['local_data_id']]; + $poller_interval = read_config_option('poller_interval'); + + if (!is_numeric($poller_interval) || $poller_interval <= 0) { + $poller_interval = 300; + } if ($thold_data['lasttime'] > 0) { if (is_numeric($currenttime)) { @@ -910,8 +1152,32 @@ function thold_get_currentval(&$thold_data, &$rrd_reindexed, &$rrd_time_reindexe $step = $thold_data['rrd_step']; } - if (empty($step)) { - $step = read_config_option('poller_interval'); + $elapsed_step = $step; + + if (!is_numeric($step) || $step <= 0) { + $step = $poller_interval; + } + + $rrd_step = is_numeric($thold_data['rrd_step']) && $thold_data['rrd_step'] > 0 + ? (float) $thold_data['rrd_step'] + : 0.0; + $sample_interval = max($rrd_step, (float) $poller_interval); + // Use a two-cycle floor so normal scheduler jitter does not discard the + // only usable pair even when an RRD heartbeat is tighter than poll cadence. + $rrd_heartbeat = is_numeric($thold_data['rrd_heartbeat'] ?? null) && $thold_data['rrd_heartbeat'] > 0 + ? max((float) $thold_data['rrd_heartbeat'], 2 * $sample_interval) + : 2 * $sample_interval; + $previous_sample_usable = $thold_data['lasttime'] > 0 + && is_numeric($elapsed_step) + && $elapsed_step > 0 + && $elapsed_step <= $rrd_heartbeat; + + if ($thold_data['lasttime'] > 0 && is_numeric($elapsed_step) && $elapsed_step > $rrd_heartbeat && function_exists('thold_debug')) { + thold_debug(sprintf( + 'Threshold sample gap of %s seconds exceeds the effective heartbeat of %s seconds.', + $elapsed_step, + $rrd_heartbeat + ), 'thold'); } $currentval = ''; @@ -923,7 +1189,7 @@ function thold_get_currentval(&$thold_data, &$rrd_reindexed, &$rrd_time_reindexe switch ($thold_data['data_source_type_id']) { case 2: // COUNTER // A previous reading of zero is a real reading, not a missing one. - if (is_numeric($thold_data['oldvalue']) && $thold_data['oldvalue'] !== '') { + if ($previous_sample_usable && is_numeric($thold_data['oldvalue']) && $thold_data['oldvalue'] !== '') { if ($item[$thold_data['name']] >= $thold_data['oldvalue']) { // Everything is Normal $currentval = $item[$thold_data['name']] - $thold_data['oldvalue']; @@ -932,7 +1198,7 @@ function thold_get_currentval(&$thold_data, &$rrd_reindexed, &$rrd_time_reindexe $currentval = thold_counter_wrap_delta($thold_data['oldvalue'], $item[$thold_data['name']]); } - if (strpos($thold_data['rrd_maximum'], '|query_') !== false) { + if (strpos((string) $thold_data['rrd_maximum'], '|query_') !== false) { $data_local = db_fetch_row_prepared('SELECT * FROM data_local WHERE id = ?', @@ -957,25 +1223,34 @@ function thold_get_currentval(&$thold_data, &$rrd_reindexed, &$rrd_time_reindexe } } + $maximum_value = $thold_data['rrd_maximum'] ?? ''; + $rrd_maximum = is_numeric($maximum_value) ? (float) $maximum_value : 0.0; + // assume counter reset if greater than max value - if ($thold_data['rrd_maximum'] > 0 && ($currentval / $step) > $thold_data['rrd_maximum']) { + if ($rrd_maximum > 0 && ($currentval / $step) > $rrd_maximum) { $currentval = $item[$thold_data['name']] / $step; - } elseif ($thold_data['rrd_maximum'] == 0 && $currentval > 4.25E+9) { + } elseif ($rrd_maximum === 0.0 && $currentval > 4.25E+9 * max(1, $step / $sample_interval)) { $currentval = $item[$thold_data['name']] / $step; } else { $currentval = $currentval / $step; } } else { - $currentval = 0; + $currentval = ''; } break; case 3: // DERIVE - $currentval = ($item[$thold_data['name']] - $thold_data['oldvalue']) / $step; + if ($previous_sample_usable && is_numeric($thold_data['oldvalue'])) { + $currentval = ($item[$thold_data['name']] - $thold_data['oldvalue']) / $step; + } else { + $currentval = ''; + } break; case 4: // ABSOLUTE - $currentval = $item[$thold_data['name']] / $step; + $currentval = ($thold_data['lasttime'] <= 0 || $previous_sample_usable) + ? $item[$thold_data['name']] / $step + : ''; break; case 1: // GAUGE @@ -1031,7 +1306,7 @@ function thold_calculate_expression($thold, $currentval, &$rrd_reindexed, &$rrd_ td.host_id, td.cdef, td.local_data_id, td.data_template_rrd_id, td.lastread, UNIX_TIMESTAMP(td.lasttime) AS lasttime, td.oldvalue, dtr.data_source_name as name, - dtr.data_source_type_id, dtd.rrd_step, dtr.rrd_maximum + dtr.data_source_type_id, dtd.rrd_step, dtr.rrd_maximum, dtr.rrd_heartbeat FROM thold_data AS td LEFT JOIN data_template_rrd AS dtr ON dtr.id = td.data_template_rrd_id @@ -1041,34 +1316,64 @@ function thold_calculate_expression($thold, $currentval, &$rrd_reindexed, &$rrd_ AND td.local_data_id = ?', [$dsname, $thold['local_data_id']]); - $value = ''; - - if (cacti_sizeof($thold_item)) { - $item = []; - $currenttime = 0; - $value = thold_get_currentval($thold_item, $rrd_reindexed, $rrd_time_reindexed, $item, $currenttime); - } + if (!cacti_sizeof($thold_item)) { + $current_sample = $rrd_reindexed[$thold['local_data_id']][$dsname] ?? ''; - // Previous returns 'U' after device recovers. Try alternate - if (empty($value) || $value == 'U') { - if (read_config_option('dsstats_enable') == 'on') { - $value = db_fetch_cell_prepared('SELECT calculated - FROM data_source_stats_hourly_last + if (is_numeric($current_sample)) { + $source = db_fetch_row_prepared('SELECT data_source_type_id + FROM data_template_rrd WHERE local_data_id = ? - AND rrd_name = ?', + AND data_source_name = ?', [$thold['local_data_id'], $dsname]); + + if (cacti_sizeof($source) && $source['data_source_type_id'] == 1) { + $value = $current_sample; + } elseif (cacti_sizeof($source)) { + $value = ''; + + if (read_config_option('dsstats_enable') == 'on') { + $value = db_fetch_cell_prepared('SELECT calculated + FROM data_source_stats_hourly_last + WHERE local_data_id = ? + AND rrd_name = ?', + [$thold['local_data_id'], $dsname]); + } + + if (!is_numeric($value) || $value == -90909090909) { + $value = get_current_value($thold['local_data_id'], $dsname, 0, ''); + } + } else { + $value = ''; + } + + if (is_numeric($value)) { + $expression[$key] = $value; + + continue; + } } - if (empty($value) || $value == 'U' || $value == '-90909090909') { - $value = get_current_value($thold['local_data_id'], $dsname); + if (is_numeric($thold['lastread'] ?? null)) { + cacti_log(sprintf( + 'WARNING: Threshold %s expression source %s is unavailable for local data ID %s.', + $thold['id'] ?? 'unknown', + $dsname, + $thold['local_data_id'] ?? 'unknown' + ), false, 'THOLD', POLLER_VERBOSITY_MEDIUM); } + + return ''; } - $expression[$key] = $value; + $item = []; + $currenttime = 0; + $value = thold_get_currentval($thold_item, $rrd_reindexed, $rrd_time_reindexed, $item, $currenttime); - if ($expression[$key] == '') { - $expression[$key] = '0'; + if (!is_numeric($value)) { + return ''; } + + $expression[$key] = $value; } elseif (strpos($item, '|') !== false) { // Remove invalid characters $item = str_replace('\\', '', $item); @@ -1084,10 +1389,6 @@ function thold_calculate_expression($thold, $currentval, &$rrd_reindexed, &$rrd_ $expression[$key] = '0'; cacti_log("WARNING: Query Replacement for '$item' Does Not Exist"); } - - if ($expression[$key] == '') { - $expression[$key] = '0'; - } } else { // normal operator } @@ -1166,7 +1467,11 @@ function thold_calculate_expression($thold, $currentval, &$rrd_reindexed, &$rrd_ if ($rpn_error) { cacti_log("ERROR: RPN Expression is invalid! THold:'" . $thold['name'] . "', Value:'" . $currentval . "', Expression:'" . $thold['expression'] . "', Processed:'" . implode(',', $processed_expression) . "'", false, 'THOLD'); - return 0; + // Fail closed: '' is this function's established unavailable-sample + // sentinel (see the early returns above). Returning a numeric 0 here + // would pass is_numeric() in the polling fail-closed guard and be + // persisted/alerted on as a real, valid zero reading. + return ''; } } @@ -1176,7 +1481,7 @@ function thold_calculate_expression($thold, $currentval, &$rrd_reindexed, &$rrd_ cacti_log("ERROR: RPN Expression did not reduce to a single value! THold:'" . $thold['name'] . "', Expression:'" . $thold['expression'] . "', Stack:'" . implode(',', $stack) . "'", false, 'THOLD'); $rpn_error = true; - return 0; + return ''; } return end($stack); @@ -1216,7 +1521,32 @@ function thold_substitute_data_source_description($string, $local_data_id, $max_ } } -function thold_substitute_host_data($string, $l_escape_string, $r_escape_string, $device_id) { +/** + * A token that cannot appear in any admin-authored template and cannot match + * any of this file's own token patterns (, |pipe|), so a later + * substitution phase can never mistake it for real token syntax. + * + * Used to defer inserting an already-quoted, device/operator-controlled + * value until every phase that scans for token syntax has run; only then is + * it safe to reveal the value without a later phase re-scanning it and + * breaking out of its quoting. + * + * @return string + */ +function thold_defer_placeholder() { + static $n = 0; + $n++; + + return "\x01THOLD_DEFERRED_{$n}\x01"; +} + +function thold_substitute_host_data($string, $l_escape_string, $r_escape_string, $device_id, $shell = false, ?array &$deferred = null) { + $defer_internally = $deferred === null; + + if ($defer_internally) { + $deferred = []; + } + $field_name = trim(str_replace('|host_', '', $string),"| \n\r"); if (!isset($_SESSION['sess_host_cache_array'][$device_id])) { @@ -1228,12 +1558,34 @@ function thold_substitute_host_data($string, $l_escape_string, $r_escape_string, } if (isset($_SESSION['sess_host_cache_array'][$device_id][$field_name])) { - return $_SESSION['sess_host_cache_array'][$device_id][$field_name]; + $field_value = $_SESSION['sess_host_cache_array'][$device_id][$field_name]; + + if (!$shell) { + return $field_value; + } + + $placeholder = thold_defer_placeholder(); + $deferred[$placeholder] = cacti_escapeshellarg((string) $field_value); + + return $defer_internally ? $deferred[$placeholder] : $placeholder; + } + + $hostname = $_SESSION['sess_host_cache_array'][$device_id]['hostname']; + + if ($shell) { + $placeholder = thold_defer_placeholder(); + $deferred[$placeholder] = cacti_escapeshellarg((string) $hostname); + $string = str_replace($l_escape_string . 'host_management_ip' . $r_escape_string, $placeholder, $string); + } else { + $string = str_replace($l_escape_string . 'host_management_ip' . $r_escape_string, $hostname, $string); } - $string = str_replace($l_escape_string . 'host_management_ip' . $r_escape_string, $_SESSION['sess_host_cache_array'][$device_id]['hostname'], $string); - $temp = api_plugin_hook_function('substitute_host_data', ['string' => $string, 'l_escape_string' => $l_escape_string, 'r_escape_string' => $r_escape_string, 'host_id' => $device_id]); - $string = $temp['string']; + $temp = api_plugin_hook_function('substitute_host_data', ['string' => $string, 'l_escape_string' => $l_escape_string, 'r_escape_string' => $r_escape_string, 'host_id' => $device_id]); + $string = $temp['string']; + + if ($defer_internally) { + $string = strtr($string, $deferred); + } return $string; } @@ -1250,7 +1602,13 @@ function thold_substitute_host_data($string, $l_escape_string, $r_escape_string, * * @return - the original string with all of the variable substitutions made */ -function thold_substitute_custom_data($string, $l_escape, $r_escape, $local_data_id) { +function thold_substitute_custom_data($string, $l_escape, $r_escape, $local_data_id, $shell = false, ?array &$deferred = null) { + $defer_internally = $deferred === null; + + if ($defer_internally) { + $deferred = []; + } + if (is_array($local_data_id)) { $local_data_ids = $local_data_id; } elseif ($local_data_id == '') { @@ -1304,15 +1662,34 @@ function thold_substitute_custom_data($string, $l_escape, $r_escape, $local_data $query_array); if (cacti_sizeof($custom_data_array)) { + $replacements = []; + foreach ($custom_data_array as $custom_data) { - $custom_name = $custom_data['name']; - $custom_value = $custom_data['value']; - $string = str_replace($l_escape . 'custom_' . $custom_name . $r_escape, $custom_value, $string); + $custom_name = $custom_data['name']; + $token = $l_escape . 'custom_' . $custom_name . $r_escape; + + if ($shell) { + $placeholder = thold_defer_placeholder(); + $deferred[$placeholder] = cacti_escapeshellarg((string) $custom_data['value']); + $replacements[$token] = $placeholder; + } else { + $replacements[$token] = $custom_data['value']; + } } + + // A single strtr() pass replaces against the original string only and + // never re-scans inserted values, so one custom field's value cannot + // contain another field's literal token and have it substituted again + // outside of its own quoting. + $string = strtr($string, $replacements); } } } + if ($defer_internally) { + $string = strtr($string, $deferred); + } + return $string; } @@ -1328,7 +1705,9 @@ function thold_calculate_percent($thold, $currentval, $rrd_reindexed) { // forced the percentage to zero and kept a low threshold alerting. $t = $rrd_reindexed[$thold['local_data_id']][$thold['percent_ds']]; - if (is_numeric($t) && $t != 0) { + if (!is_numeric($t)) { + $currentval = ''; + } elseif ($t != 0) { $currentval = ($currentval / $t) * 100; } else { $currentval = 0; @@ -1343,15 +1722,26 @@ function thold_calculate_percent($thold, $currentval, $rrd_reindexed) { function thold_calculate_lower_upper($thold, $currentval, $rrd_reindexed) { $ds = $thold['upper_ds']; - if (isset($rrd_reindexed[$thold['local_data_id']][$ds])) { - $t = $rrd_reindexed[$thold['local_data_id']][$thold['upper_ds']]; - $currentval = ($t << 32) + $currentval; + if (!is_numeric($currentval)) { + return ''; } - return $currentval; + if (!isset($rrd_reindexed[$thold['local_data_id']][$ds])) { + return ''; + } + + $t = $rrd_reindexed[$thold['local_data_id']][$thold['upper_ds']]; + + if (!is_numeric($t) || $t < 0 || $t > 4294967295) { + return ''; + } + + return ((float) $t * 4294967296) + $currentval; } function get_allowed_thresholds($sql_where = '', $order_by = 'td.name', $sql_limit = '', &$total_rows = 0, $user_id = 0, $graph_id = 0, $sql_params = []) { + $graph_id = (int) $graph_id; + if ($sql_limit != '') { $sql_limit = "LIMIT $sql_limit"; } @@ -1363,7 +1753,7 @@ function get_allowed_thresholds($sql_where = '', $order_by = 'td.name', $sql_lim $params = $sql_params; if ($graph_id > 0) { - $sql_where .= (strlen($sql_where) ? ' AND ' : ' ') . " gl.id = ?"; + $sql_where .= (strlen($sql_where) ? ' AND ' : ' ') . ' gl.id = ?'; $params[] = $graph_id; } @@ -1447,6 +1837,8 @@ function get_allowed_thresholds($sql_where = '', $order_by = 'td.name', $sql_lim } function get_allowed_threshold_logs($sql_where = '', $order_by = 'td.name', $sql_limit = '', &$total_rows = 0, $user_id = 0, $graph_id = 0, $sql_params = []) { + $graph_id = (int) $graph_id; + if ($sql_limit != '') { $sql_limit = "LIMIT $sql_limit"; } @@ -1458,7 +1850,7 @@ function get_allowed_threshold_logs($sql_where = '', $order_by = 'td.name', $sql $params = $sql_params; if ($graph_id > 0) { - $sql_where .= (strlen($sql_where) ? ' AND ' : ' ') . " gl.id = ?"; + $sql_where .= (strlen($sql_where) ? ' AND ' : ' ') . ' gl.id = ?'; $params[] = $graph_id; } @@ -2425,6 +2817,11 @@ function thold_check_threshold(&$thold_data) { return; } + // An unavailable sample is not evidence that an active alert recovered. + if (!is_numeric($thold_data['lastread'])) { + return; + } + $alert_exempt = read_config_option('alert_exempt'); // check for exemptions $weekday = date('l'); @@ -3742,6 +4139,42 @@ function thold_check_threshold(&$thold_data) { WHERE id = ?', [$thold_data['id']]); } + } elseif ($alertstat != 0 && $thold_data['restored_alert'] != 'on') { + /* + * Neither the warning nor the alert failure counts reached + * their trigger, but the threshold was alerting last poll + * and is normal now: log the restoral so it is visible in + * the log, without re-sending a notification for a state + * transition that never crossed its own trigger. + */ + thold_log([ + 'type' => 2, + 'time' => time(), + 'host_id' => $thold_data['host_id'], + 'local_graph_id' => $thold_data['local_graph_id'], + 'threshold_id' => $thold_data['id'], + 'threshold_value' => '', + 'current' => $thold_data['lastread'], + 'status' => ST_NOTIFYRS, + 'description' => $subject, + 'emails' => '', + 'bcc_emails' => ''] + ); + + db_execute_prepared('UPDATE thold_data + SET thold_alert = 0, + lastchanged = NOW(), + thold_warning_fail_count = ?, + thold_fail_count = ? + WHERE id = ?', + [$warning_failures, $failures, $thold_data['id']]); + + if ($thold_data['reset_ack'] == 'on') { + db_execute_prepared('UPDATE thold_data + SET acknowledgment = "" + WHERE id = ?', + [$thold_data['id']]); + } } else { db_execute_prepared('UPDATE thold_data SET thold_fail_count = ?, @@ -4028,11 +4461,37 @@ function get_thold_snmp_data($data_source_name, $thold, $h, $currentval) { return $thold_snmp_data; } -function thold_expand_string($thold_data, $string) { +function thold_expand_string($thold_data, $string, $shell = false, ?array &$deferred = null) { global $config; include_once($config['library_path'] . '/variables.php'); + $defer_internally = $deferred === null; + + if ($defer_internally) { + $deferred = []; + } + + // Values substituted below (data source names/descriptions, graph + // titles, and host/data-query fields sourced from the polled device + // itself) are not admin-controlled, so in $shell mode they must be + // quoted before landing on a command line, same as + // thold_replace_threshold_tags()'s $q(). The quoted value is deferred + // behind an opaque placeholder rather than inserted immediately, so a + // later phase's token scan (or a caller chaining another substitution + // pass afterward) can never mistake it for real token syntax and + // re-substitute inside its quoting. + $q = function ($value) use ($shell, &$deferred) { + if (!$shell) { + return $value; + } + + $placeholder = thold_defer_placeholder(); + $deferred[$placeholder] = cacti_escapeshellarg((string) $value); + + return $placeholder; + }; + $str = $string; // Handle the blank string case @@ -4068,7 +4527,7 @@ function thold_expand_string($thold_data, $string) { $value = read_config_option('thold_empty_if_speed_default'); } - $str = str_replace('|query_ifHighSpeed|', $value, $str); + $str = str_replace('|query_ifHighSpeed|', $q($value), $str); } elseif (strpos($str, '|query_ifSpeed|') !== false) { $value = thold_substitute_snmp_query_data('|query_ifSpeed|', $lg['host_id'], $lg['snmp_query_id'], $lg['snmp_index'], read_config_option('max_data_query_field_length')); @@ -4076,11 +4535,49 @@ function thold_expand_string($thold_data, $string) { $value = read_config_option('thold_empty_if_speed_default'); } - $str = str_replace('|query_ifSpeed|', $value, $str); + $str = str_replace('|query_ifSpeed|', $q($value), $str); + } + + if ($shell) { + // expand_title() (Cacti core) substitutes |host_*|/|query_*| + // tokens with raw values -- hostname, SNMP community/ + // password, sysDescr/sysContact/sysLocation, polled data + // query fields, etc -- with no shell escaping, and any of + // them can be set by the polled device itself. Resolve each + // token in isolation through the same core helpers so the + // substituted value matches what expand_title() would have + // produced, but escape it before it lands in the command. + preg_match_all('/\|(?:host|query)_[A-Za-z0-9_]+\|/', $str, $host_query_tokens); + + $host_query_replacements = []; + + foreach (array_unique($host_query_tokens[0]) as $token) { + if ($lg['snmp_query_id'] != '0' && $lg['snmp_index'] != '') { + $resolved = substitute_snmp_query_data( + null_out_substitutions(substitute_host_data($token, '|', '|', $lg['host_id'])), + $lg['host_id'], $lg['snmp_query_id'], $lg['snmp_index'], + intval(read_config_option('max_data_query_field_length')) + ); + } else { + $resolved = null_out_substitutions(substitute_host_data($token, '|', '|', $lg['host_id'])); + } + + if ($resolved !== $token) { + $host_query_replacements[$token] = $q($resolved); + } + } + + // A single strtr() pass replaces against the original string only and + // never re-scans inserted values, so a resolved host/query value that + // happens to contain another token's literal text can't be substituted + // a second time outside of its own quoting. + $str = strtr($str, $host_query_replacements); + $str = null_out_substitutions($str); + } else { + $str = expand_title($lg['host_id'], $lg['snmp_query_id'], $lg['snmp_index'], $str); } - $str = expand_title($lg['host_id'], $lg['snmp_query_id'], $lg['snmp_index'], $str); - $str = thold_substitute_custom_data($str, '|', '|', $thold_data['local_data_id']); + $str = thold_substitute_custom_data($str, '|', '|', $thold_data['local_data_id'], $shell, $deferred); $data = [ 'str' => $str, @@ -4100,29 +4597,37 @@ function thold_expand_string($thold_data, $string) { } if (strpos($str, '|host_') !== false && !empty($device_id)) { - $str = thold_substitute_host_data($str, '|', '|', $device_id); + $str = thold_substitute_host_data($str, '|', '|', $device_id, $shell, $deferred); } - // Replace |graph_title| + // Replace |graph_title|, |data_source_description|, and |data_source_name| + // in a single strtr() pass so a resolved value containing another + // token's literal text can't be substituted a second time outside of + // its own quoting. + $direct_replacements = []; + if (strpos($str, '|graph_title|') !== false) { - $title = get_graph_title($thold_data['local_graph_id']); - $str = str_replace('|graph_title|', $title, $str); + $direct_replacements['|graph_title|'] = $q(get_graph_title($thold_data['local_graph_id'])); } - // Replace |data_source_description| if (strpos($str, '|data_source_description|') !== false) { $data_source_desc = db_fetch_cell_prepared('SELECT name_cache FROM data_template_data WHERE local_data_id = ?', [$thold_data['local_data_id']]); - $str = str_replace('|data_source_description|', $data_source_desc, $str); + $direct_replacements['|data_source_description|'] = $q($data_source_desc); } - // Replace |data_source_name| if (strpos($str, '|data_source_name|') !== false) { - $str = str_replace('|data_source_name|', $thold_data['data_source_name'], $str); + $direct_replacements['|data_source_name|'] = $q($thold_data['data_source_name']); } + + $str = strtr($str, $direct_replacements); + } + + if ($defer_internally) { + $str = strtr($str, $deferred); } return trim($str); @@ -4137,9 +4642,17 @@ function thold_command_execution(&$thold_data, &$h, $breach_up, $breach_down, $b $queue = read_config_option('thold_notification_queue'); if ($breach_up && $thold_data['trigger_cmd_high'] != '') { - $cmd = thold_replace_threshold_tags($thold_data['trigger_cmd_high'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id'], $data_source_name, true); + $deferred = []; - $cmd = thold_expand_string($thold_data, $cmd); + $cmd = thold_expand_string($thold_data, $thold_data['trigger_cmd_high'], true, $deferred); + $cmd = thold_replace_threshold_tags($cmd, $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id'], $data_source_name, true, $deferred); + + // Both calls above deferred their quoted values behind opaque + // placeholders instead of inserting them immediately, so neither + // one's value could be re-scanned (and have its quoting broken) by + // the other's token substitution. Resolve every placeholder now + // that no further token scanning will happen. + $cmd = strtr($cmd, $deferred); $environment = thold_set_environ($thold_data['trigger_cmd_high'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id'], $data_source_name); @@ -4157,8 +4670,11 @@ function thold_command_execution(&$thold_data, &$h, $breach_up, $breach_down, $b $command_executed = true; } elseif ($breach_down && $thold_data['trigger_cmd_low'] != '') { - $cmd = thold_replace_threshold_tags($thold_data['trigger_cmd_low'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id'], $data_source_name, true); - $cmd = thold_expand_string($thold_data, $cmd); + $deferred = []; + + $cmd = thold_expand_string($thold_data, $thold_data['trigger_cmd_low'], true, $deferred); + $cmd = thold_replace_threshold_tags($cmd, $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id'], $data_source_name, true, $deferred); + $cmd = strtr($cmd, $deferred); $environment = thold_set_environ($thold_data['trigger_cmd_high'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id'], $data_source_name); @@ -4176,8 +4692,11 @@ function thold_command_execution(&$thold_data, &$h, $breach_up, $breach_down, $b $command_executed = true; } elseif ($breach_norm && $thold_data['trigger_cmd_norm'] != '') { - $cmd = thold_replace_threshold_tags($thold_data['trigger_cmd_norm'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id'], $data_source_name, true); - $cmd = thold_expand_string($thold_data, $cmd); + $deferred = []; + + $cmd = thold_expand_string($thold_data, $thold_data['trigger_cmd_norm'], true, $deferred); + $cmd = thold_replace_threshold_tags($cmd, $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id'], $data_source_name, true, $deferred); + $cmd = strtr($cmd, $deferred); $environment = thold_set_environ($thold_data['trigger_cmd_high'], $thold_data, $h, $thold_data['lastread'], $thold_data['local_graph_id'], $data_source_name); @@ -4311,9 +4830,15 @@ function thold_set_environ($text, &$thold, &$h, $currentval, $local_graph_id, $d return $environment; } -function thold_replace_threshold_tags($text, &$thold, &$h, $currentval, $local_graph_id, $data_source_name, $shell = false) { +function thold_replace_threshold_tags($text, &$thold, &$h, $currentval, $local_graph_id, $data_source_name, $shell = false, ?array &$deferred = null) { global $thold_types; + $defer_internally = $deferred === null; + + if ($defer_internally) { + $deferred = []; + } + if (substr(read_config_option('base_url'), 0, 4) != 'http') { if (read_config_option('force_https') == 'on') { $prefix = 'https://'; @@ -4337,54 +4862,72 @@ function thold_replace_threshold_tags($text, &$thold, &$h, $currentval, $local_g // Device and threshold free-text values are admin/user editable. When $text // is a trigger command template ($shell), quote them so they cannot - // terminate the command and start another. - $quote = function ($value) use ($shell) { - return $shell ? cacti_escapeshellarg((string) $value) : $value; - }; + // terminate the command and start another. The quoted value is deferred + // behind an opaque placeholder rather than inserted immediately, so a + // caller chaining another substitution pass afterward (thold_expand_string()) + // can never mistake it for real token syntax and re-substitute inside its + // quoting. + $q = function ($value) use ($shell, &$deferred) { + if (!$shell) { + return $value; + } - // Do some replacement of variables - $text = thold_str_replace('', $quote($h['description']), $text); - $text = thold_str_replace('', $quote($h['hostname']), $text); - $text = thold_str_replace('', $quote($h['location']), $text); - $text = thold_str_replace('', $quote($site), $text); - $text = thold_str_replace('', $local_graph_id, $text); - $text = thold_str_replace('', $thold['id'], $text); + $placeholder = thold_defer_placeholder(); + $deferred[$placeholder] = cacti_escapeshellarg((string) $value); + + return $placeholder; + }; - $text = thold_str_replace('', $quote($currentval), $text); - $text = thold_str_replace('', $quote($thold['name_cache']), $text); - $text = thold_str_replace('', $data_source_name, $text); + // Do some replacement of variables. Every tag/value pair is collected up + // front and substituted in a single strtr() pass: strtr() replaces against + // the original text only and never re-scans inserted values, so a value + // that happens to contain another tag's literal placeholder (for example + // a of "") can't be substituted a second time + // outside of $q()'s quoting. + $replacements = [ + '' => $q($h['description']), + '' => $q($h['hostname']), + '' => $q($h['location']), + '' => $q($site), + '' => $local_graph_id, + '' => $thold['id'], + '' => $q($currentval), + '' => $q($thold['name_cache']), + '' => $q($data_source_name), + '' => $q($thold['notes']), + '' => $q($thold['dnotes']), + '' => $q($thold['dnotes']), + '' => $q($thold['external_id']), + '" . __('Link to Graph in Cacti', 'thold') . ''), + ]; if (isset($thold_types[$thold['thold_type']])) { - $text = thold_str_replace('', $thold_types[$thold['thold_type']], $text); + $replacements[''] = $thold_types[$thold['thold_type']]; } - $text = thold_str_replace('', $quote($thold['notes']), $text); - $text = thold_str_replace('', $quote($thold['dnotes']), $text); - $text = thold_str_replace('', $quote($thold['dnotes']), $text); - $text = thold_str_replace('', $quote($thold['external_id']), $text); - if ($thold['thold_type'] == 0) { - $text = thold_str_replace('', $thold['thold_hi'], $text); - $text = thold_str_replace('', $thold['thold_low'], $text); - $text = thold_str_replace('', $thold['thold_fail_trigger'], $text); - $text = thold_str_replace('', '', $text); + $replacements[''] = $thold['thold_hi']; + $replacements[''] = $thold['thold_low']; + $replacements[''] = $thold['thold_fail_trigger']; + $replacements[''] = ''; } elseif ($thold['thold_type'] == 2) { - $text = thold_str_replace('', $thold['time_hi'], $text); - $text = thold_str_replace('', $thold['time_low'], $text); - $text = thold_str_replace('', $thold['time_fail_trigger'], $text); - $text = thold_str_replace('', plugin_thold_duration_convert($thold['local_data_id'], $thold['time_fail_length'], 'time'), $text); + $replacements[''] = $thold['time_hi']; + $replacements[''] = $thold['time_low']; + $replacements[''] = $thold['time_fail_trigger']; + $replacements[''] = plugin_thold_duration_convert($thold['local_data_id'], $thold['time_fail_length'], 'time'); } else { - $text = thold_str_replace('', '', $text); - $text = thold_str_replace('', '', $text); - $text = thold_str_replace('', '', $text); - $text = thold_str_replace('', '', $text); + $replacements[''] = ''; + $replacements[''] = ''; + $replacements[''] = ''; + $replacements[''] = ''; } - $text = thold_str_replace('