diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 0000000..8bc3f3a --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,60 @@ +# +-------------------------------------------------------------------------+ +# | Copyright (C) 2004-2026 The Cacti Group | +# +-------------------------------------------------------------------------+ +# | Cacti: The Complete RRDtool-based Graphing Solution | +# +-------------------------------------------------------------------------+ +# | http://www.cacti.net/ | +# +-------------------------------------------------------------------------+ +# +# CodeQL has no PHP analysis, so this covers the plugin JavaScript only. Nothing in +# CI performs taint analysis on the PHP: plugin-ci-workflow.yml runs a parser check +# and an install smoke test, neither of which tracks a value from request to sink. + +name: "CodeQL" + +on: + push: + branches: [main, develop] + paths-ignore: + - "**/*.md" + pull_request: + branches: [main, develop] + paths-ignore: + - "**/*.md" + schedule: + - cron: "30 1 * * 1" + workflow_dispatch: + +concurrency: + group: codeql-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + actions: read + contents: read + security-events: write + strategy: + fail-fast: false + matrix: + language: ["javascript-typescript"] + steps: + - name: Checkout repository + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 + - name: Initialize CodeQL + uses: github/codeql-action/init@b56ba49b26e50535fa1e7f7db0f4f7b4bf65d80d # v3 + with: + languages: ${{ matrix.language }} + - name: Autobuild + uses: github/codeql-action/autobuild@b56ba49b26e50535fa1e7f7db0f4f7b4bf65d80d # v3 + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@b56ba49b26e50535fa1e7f7db0f4f7b4bf65d80d # v3 + with: + category: "/language:${{ matrix.language }}" diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index 1b63343..a57245e 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -19,19 +19,116 @@ # | http://www.cacti.net/ | # +-------------------------------------------------------------------------+ -name: Plugin Integration Tests +name: Monitor CI on: push: branches: - main - develop + paths: + - '**.php' + - '.github/workflows/**' + - 'phpunit.xml.dist' + - 'tests/**' pull_request: branches: - main - develop + paths: + - '**.php' + - '.github/workflows/**' + - 'phpunit.xml.dist' + - 'tests/**' + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: monitor-ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true jobs: + # The plugin ships tests that nothing ran: three files under tests/ that pass + # from a plain PHP CLI. They cover request-variable output escaping, which is + # the class of defect most likely to regress here. + # A duplicate key or schema error stops GitHub loading the file, which + # presents as a workflow that silently never runs. Catch it here instead. + workflow-lint: + name: Workflow syntax + runs-on: ubuntu-latest + steps: + - name: Checkout Monitor Plugin + uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 + + - name: Run actionlint + uses: docker://rhysd/actionlint@sha256:887a259a5a534f3c4f36cb02dca341673c6089431057242cdc931e9f133147e9 + with: + args: -color + + unit-test: + name: Pest using Cacti Composer (Docker) + runs-on: ubuntu-latest + timeout-minutes: 20 + + steps: + - name: Checkout monitor Plugin + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 + + - name: Checkout Cacti runtime + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 + 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@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 + with: + repository: Cacti/cacti + # Pins Composer, Pest, the lock file and Cacti's Docker test image. + ref: 298bd51eca843490fb90b27ada6b3fecc9b9a7d8 + path: cacti-toolchain + + # The same image Cacti's own tests/tools/docker_pest.sh builds. Composer and + # Pest come from Cacti's locked toolchain, so the plugin ships no + # composer.json and no parallel vendor directory. + - name: Build Cacti test image + run: | + docker build --tag cacti-web --file cacti-toolchain/docker/Dockerfile cacti-toolchain/docker + docker build --tag cacti-monitor-test --file cacti-toolchain/docker/Dockerfile.test cacti-toolchain + + - name: Run Pest + run: | + docker run --rm \ + --volume "$PWD/cacti-runtime":/cacti \ + --volume "$PWD":/cacti/plugins/monitor \ + --user root \ + --entrypoint composer cacti-monitor-test \ + test -- --configuration=/cacti/plugins/monitor/phpunit.xml.dist + + syntax-floor: + name: PHP 8.0 syntax floor + runs-on: ubuntu-latest + steps: + - name: Checkout monitor Plugin + uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 + + - name: Install PHP 8.0 + uses: shivammathur/setup-php@b604ade2a87db23f8871b7182e69ec5e75effb45 # v2 + with: + php-version: '8.0' + + - name: Lint every PHP file + run: | + if find . -path ./vendor -prune -o -name '*.php' -print0 \ + | xargs -0 -n1 php -l 2>&1 | grep -iv 'no syntax errors detected'; then + echo "Syntax errors found at the declared PHP floor" + exit 1 + fi + integration-test: runs-on: ${{ matrix.os }} @@ -43,7 +140,7 @@ jobs: services: mariadb: - image: mariadb:10.6 + image: mariadb:10.11 env: MYSQL_ROOT_PASSWORD: cactiroot MYSQL_DATABASE: cacti @@ -61,18 +158,19 @@ jobs: steps: - name: Checkout Cacti - uses: actions/checkout@v5 + uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 with: repository: Cacti/cacti + ref: release/1.2.31 path: cacti - name: Checkout Monitor Plugin - uses: actions/checkout@v5 + uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 with: path: cacti/plugins/monitor - name: Install PHP ${{ matrix.php }} - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@b604ade2a87db23f8871b7182e69ec5e75effb45 # v2 with: php-version: ${{ matrix.php }} extensions: intl, mysql, gd, ldap, gmp, xml, curl, json, mbstring @@ -81,11 +179,10 @@ jobs: - name: Check PHP version run: php -v - - name: Run apt-get update - run: sudo apt-get update - - name: Install System Dependencies - run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping libapache2-mod-php + run: | + sudo apt-get update + sudo apt-get install --yes --no-install-recommends apache2 snmp snmpd rrdtool fping libapache2-mod-php - name: Start SNMPD Agent and Test run: | @@ -95,40 +192,37 @@ jobs: - name: Setup Permissions run: | sudo chown -R www-data:runner ${{ github.workspace }}/cacti - sudo find ${{ github.workspace }}/cacti -type d -exec chmod 775 {} \; - sudo find ${{ github.workspace }}/cacti -type f -exec chmod 664 {} \; + sudo find ${{ github.workspace }}/cacti -type d -exec chmod 775 {} + + 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' + MYSQL_PWD: cactiroot run: | - mysql $MYSQL_AUTH_USR -e 'CREATE DATABASE IF NOT EXISTS cacti;' - mysql $MYSQL_AUTH_USR -e "CREATE USER IF NOT EXISTS 'cactiuser'@'localhost' IDENTIFIED BY 'cactiuser';" - mysql $MYSQL_AUTH_USR -e "GRANT ALL PRIVILEGES ON cacti.* TO 'cactiuser'@'localhost';" - 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', '/usr/bin/php')" cacti + mysql --host=127.0.0.1 --user=root -e 'CREATE DATABASE IF NOT EXISTS cacti;' + mysql --host=127.0.0.1 --user=root -e "CREATE USER IF NOT EXISTS 'cactiuser'@'localhost' IDENTIFIED BY 'cactiuser';" + mysql --host=127.0.0.1 --user=root -e "GRANT ALL PRIVILEGES ON cacti.* TO 'cactiuser'@'localhost';" + mysql --host=127.0.0.1 --user=root -e "GRANT SELECT ON mysql.time_zone_name TO 'cactiuser'@'localhost';" + mysql --host=127.0.0.1 --user=root -e "FLUSH PRIVILEGES;" + mysql --host=127.0.0.1 --user=root cacti < ${{ github.workspace }}/cacti/cacti.sql + mysql --host=127.0.0.1 --user=root -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 + composer validate --no-check-publish fi - name: Install Composer Dependencies run: | cd ${{ github.workspace }}/cacti if [ -f composer.json ]; then - sudo composer install --prefer-dist --no-progress + # Cacti core ships no composer.lock and declares no scripts, so nothing + # is lost by refusing to run package-supplied code as root. + sudo composer install --prefer-dist --no-interaction --no-progress --no-plugins --no-scripts fi - name: Create Cacti config.php @@ -173,24 +267,37 @@ jobs: - name: Check PHP Syntax for Plugin run: | cd ${{ github.workspace }}/cacti/plugins/monitor - if find . -name '*.php' -exec php -l {} 2>&1 \; | grep -iv 'no syntax errors detected'; then - echo "Syntax errors found!" - exit 1 - fi + find . -path './vendor' -prune -o -name '*.php' -print0 | xargs -0 -n1 php -l - name: Remove the plugins directory exclusion from the .phpstan.neon - run: sed '/plugins/d' -i .phpstan.neon + run: | + if [ -f .phpstan.neon ]; then + sed '/plugins/d' -i .phpstan.neon + fi working-directory: ${{ github.workspace }}/cacti - name: Mark composer scripts executable - run: sudo chmod +x ${{ github.workspace }}/cacti/include/vendor/bin/* + 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 - name: Run Linter on base code - run: composer run-script lint ${{ github.workspace }}/cacti/plugins/monitor + run: | + if composer run-script --list | grep -qE '^ lint'; then + composer run-script lint ${{ github.workspace }}/cacti/plugins/monitor + else + echo 'Composer lint script is not defined; skipping.' + fi working-directory: ${{ github.workspace }}/cacti - name: Checking coding standards on base code - run: composer run-script phpcsfixer ${{ github.workspace }}/cacti/plugins/monitor + run: | + if composer run-script --list | grep -qE '^ phpcsfixer'; then + composer run-script phpcsfixer ${{ github.workspace }}/cacti/plugins/monitor + else + echo 'Composer phpcsfixer script is not defined; skipping.' + fi working-directory: ${{ github.workspace }}/cacti # - name: Run PHPStan at Level 6 on base code outside of Composer due to technical issues @@ -214,3 +321,11 @@ jobs: echo "=== Cacti Log ===" sudo cat ${{ github.workspace }}/cacti/log/cacti.log fi + + - name: Upload Cacti log + if: failure() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + with: + name: cacti-log-php-${{ matrix.php }} + path: ${{ github.workspace }}/cacti/log/cacti.log + if-no-files-found: ignore diff --git a/.gitignore b/.gitignore index 7a6c551..6b564ca 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,8 @@ locales/po/*.mo vendor/ .omc/ + +# Lockfile is not committed: the plugin declares a range, and thold does the same. +composer.lock +/vendor/ +.phpunit.result.cache diff --git a/phpunit.xml.dist b/phpunit.xml.dist new file mode 100644 index 0000000..72df35b --- /dev/null +++ b/phpunit.xml.dist @@ -0,0 +1,11 @@ + + + + + ./tests/unit + + + ./tests/Integration + + + diff --git a/tests/Integration/OutputEscapingTest.php b/tests/Integration/OutputEscapingTest.php new file mode 100644 index 0000000..05f258b --- /dev/null +++ b/tests/Integration/OutputEscapingTest.php @@ -0,0 +1,44 @@ +toContain($pattern); +})->with('escaped_outputs'); + +dataset('raw_reuse', [ + ['monitor_controller.php', "get_request_var('tree') . '\">'"], + ['monitor_controller.php', "get_request_var('site') . '\">'"], + ['monitor_controller.php', "get_request_var('template') . '\">'"], + ['monitor_controller.php', "get_request_var('size') . '\">'"], + ['monitor_controller.php', "get_request_var('trim') . '\">'"], + ['monitor_render.php', "monitor.php?rfilter=' . get_request_var('rfilter')"], +]); + +it('never concatenates a raw request value into markup', function (string $file, string $pattern) use ($root) { + expect(file_get_contents($root . '/' . $file))->not->toContain($pattern); +})->with('raw_reuse'); diff --git a/tests/Integration/test_monitor_request_output_wiring.php b/tests/Integration/test_monitor_request_output_wiring.php deleted file mode 100644 index e20dadd..0000000 --- a/tests/Integration/test_monitor_request_output_wiring.php +++ /dev/null @@ -1,41 +0,0 @@ - [ - "html_escape(get_request_var('downhosts'))", - "html_escape(get_request_var('mute'))", - "html_escape(get_request_var('tree'))", - "html_escape(get_request_var('site'))", - "html_escape(get_request_var('template'))", - "html_escape(get_request_var('size'))", - "html_escape(get_request_var('trim'))", - ], - __DIR__ . '/../../monitor_render.php' => [ - "rawurlencode(get_request_var('rfilter'))", - ], -]; - -foreach ($checks as $path => $patterns) { - $contents = file_get_contents($path); - - if ($contents === false) { - fwrite(STDERR, "Unable to read {$path}\n"); - exit(1); - } - - foreach ($patterns as $pattern) { - if (strpos($contents, $pattern) === false) { - fwrite(STDERR, "Missing expected output hardening: {$pattern}\n"); - exit(1); - } - } -} - -print "OK\n"; diff --git a/tests/Pest.php b/tests/Pest.php new file mode 100644 index 0000000..8434908 --- /dev/null +++ b/tests/Pest.php @@ -0,0 +1,14 @@ +beforeEach(function () { + $GLOBALS['__test_sql'] = []; + $GLOBALS['__test_settings'] = []; + test_set_request([]); +})->in(__DIR__); diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000..16d4009 --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,101 @@ + dirname(__DIR__), + 'url_path' => '/cacti/', + 'cacti_version' => '1.2.999', +]; + +$GLOBALS['__test_request'] = []; +$GLOBALS['__test_settings'] = []; +$GLOBALS['__test_sql'] = []; + +if (!function_exists('get_request_var')) { + function get_request_var($name, $default = '') { + return $GLOBALS['__test_request'][$name] ?? $default; + } +} + +if (!function_exists('get_nfilter_request_var')) { + function get_nfilter_request_var($name, $default = '') { + return get_request_var($name, $default); + } +} + +if (!function_exists('read_config_option')) { + function read_config_option($name, $force = false) { + return $GLOBALS['__test_settings'][$name] ?? ''; + } +} + +if (!function_exists('read_user_setting')) { + function read_user_setting($name, $default = '', $force = false) { + return $GLOBALS['__test_settings'][$name] ?? $default; + } +} + +if (!function_exists('cacti_sizeof')) { + function cacti_sizeof($a) { return is_array($a) ? count($a) : 0; } +} + +if (!function_exists('db_qstr')) { + function db_qstr($s, $db_conn = false) { + return "'" . str_replace(["\\", "'"], ["\\\\", "\\'"], (string) $s) . "'"; + } +} + +if (!function_exists('db_qstr_rlike')) { + /* Mirrors Cacti core: cap the length, strip alternation and bounded + * repeats, then quote. */ + function db_qstr_rlike($s, $db_conn = false) { + $s = (string) $s; + + if (strlen($s) > 255) { + $s = substr($s, 0, 255); + } + + $s = str_replace(["\0", '|', '{', '}'], '', $s); + + return 'RLIKE ' . db_qstr($s, $db_conn); + } +} + +if (!function_exists('db_fetch_cell_prepared')) { + function db_fetch_cell_prepared($sql, $params = [], $col = '', $log = true, $db_conn = false) { + $GLOBALS['__test_sql'][] = ['sql' => $sql, 'params' => $params]; + + return $GLOBALS['__test_db_cell'] ?? ''; + } +} + +if (!function_exists('db_fetch_cell')) { + function db_fetch_cell($sql, $col = '', $log = true, $db_conn = false) { + $GLOBALS['__test_sql'][] = ['sql' => $sql, 'params' => []]; + + return $GLOBALS['__test_db_cell'] ?? ''; + } +} + +if (!function_exists('__')) { + function __($format, ...$args) { + if (count($args) > 1) { array_pop($args); } + + return $args === [] ? $format : vsprintf($format, $args); + } +} + +function test_set_request(array $vars): void { + $GLOBALS['__test_request'] = $vars + [ + 'crit' => 0, 'site' => 0, 'tree' => 0, 'grouping' => '', 'rfilter' => '', 'status' => '0', + ]; +} diff --git a/tests/e2e/test_monitor_no_raw_request_reuse.php b/tests/e2e/test_monitor_no_raw_request_reuse.php deleted file mode 100644 index 73be1e9..0000000 --- a/tests/e2e/test_monitor_no_raw_request_reuse.php +++ /dev/null @@ -1,40 +0,0 @@ - [ - "get_request_var('downhosts') . '\">'", - "get_request_var('site') . '\">'", - "get_request_var('template') . '\">'", - "get_request_var('size') . '\">'", - "get_request_var('trim') . '\">'", - ], - __DIR__ . '/../../monitor_render.php' => [ - "monitor.php?rfilter=' . get_request_var('rfilter')", - ], -]; - -foreach ($checks as $path => $patterns) { - $contents = file_get_contents($path); - - if ($contents === false) { - fwrite(STDERR, "Unable to read {$path}\n"); - exit(1); - } - - foreach ($patterns as $pattern) { - if (strpos($contents, $pattern) !== false) { - fwrite(STDERR, "Raw request reuse remains: {$pattern}\n"); - exit(1); - } - } -} - -print "OK\n"; diff --git a/tests/unit/DbFunctionsTest.php b/tests/unit/DbFunctionsTest.php new file mode 100644 index 0000000..216a384 --- /dev/null +++ b/tests/unit/DbFunctionsTest.php @@ -0,0 +1,59 @@ + '2']); + + expect(getTholdWhere())->toContain('td.thold_alert != 0 OR td.bl_alert > 0'); +}); + +it('selects the triggered clause for any other status', function () { + test_set_request(['status' => '0']); + + expect(getTholdWhere())->toContain('thold_fail_count >= td.thold_fail_trigger'); +}); + +it('builds an IN clause from a concatenated id list', function () { + $where = ''; + renderGroupConcat($where, ' AND ', 'h.id', '4,9,17'); + + expect($where)->toBe('(h.id IN(4,9,17) )'); +}); + +it('joins onto an existing clause rather than replacing it', function () { + $where = 'h.disabled = ""'; + renderGroupConcat($where, ' AND ', 'h.id', '4'); + + expect($where)->toStartWith('h.disabled = ""')->and($where)->toContain(' AND '); +}); + +it('adds nothing when the id list is empty', function () { + $where = ''; + renderGroupConcat($where, ' AND ', 'h.id', ''); + + expect($where)->toBe(''); +}); + +it('collapses the doubled commas GROUP_CONCAT can produce', function () { + $where = ''; + renderGroupConcat($where, ' AND ', 'h.id', ',,4,,9,,'); + + expect($where)->toBe('(h.id IN(4,9) )'); +}); + +it('appends the optional suffix', function () { + $where = ''; + renderGroupConcat($where, ' AND ', 'h.id', '4', 'OR h.id IS NULL'); + + expect($where)->toContain('OR h.id IS NULL'); +}); diff --git a/tests/unit/test_request_output_escaping.php b/tests/unit/test_request_output_escaping.php deleted file mode 100644 index a9cd09b..0000000 --- a/tests/unit/test_request_output_escaping.php +++ /dev/null @@ -1,19 +0,0 @@ -