diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..0b89da2 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,26 @@ +# +-------------------------------------------------------------------------+ +# | Copyright (C) 2004-2026 The Cacti Group | +# +-------------------------------------------------------------------------+ +# | Cacti: The Complete RRDtool-based Graphing Solution | +# +-------------------------------------------------------------------------+ +# | http://www.cacti.net/ | +# +-------------------------------------------------------------------------+ + +# No composer entry: the plugin ships no composer.json, and its tests run from +# a plain PHP CLI by design. + +version: 2 +updates: + - package-ecosystem: "github-actions" + directory: "/" + target-branch: "develop" + schedule: + interval: "weekly" + open-pull-requests-limit: 5 + groups: + github-actions: + patterns: + - "*" + commit-message: + prefix: "ci" + include: "scope" diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 0000000..9dd80d5 --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,59 @@ +# +-------------------------------------------------------------------------+ +# | 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. The PHP is +# covered by the syntax and integration jobs in plugin-ci-workflow.yml. + +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..1bbd1a8 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -31,7 +31,54 @@ on: - main - develop +permissions: + contents: read + 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. + unit-test: + name: Standalone test suite + runs-on: ubuntu-latest + steps: + - name: Checkout monitor Plugin + uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 + + - name: Install PHP + uses: shivammathur/setup-php@b604ade2a87db23f8871b7182e69ec5e75effb45 # v2 + with: + php-version: '8.1' + tools: composer + + - name: Install dev dependencies + run: composer install --prefer-dist --no-progress --no-interaction --no-plugins --no-scripts + + - name: Run the test suite + run: composer test + + # compat = 1.2.15 implies Cacti 1.2.x, which requires PHP 8.0. The matrix + # below starts at 8.1, so it cannot catch syntax the declared floor rejects. + 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' -print \ + | xargs -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 }} @@ -61,18 +108,18 @@ jobs: steps: - name: Checkout Cacti - uses: actions/checkout@v5 + uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 with: repository: Cacti/cacti 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 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/composer.json b/composer.json new file mode 100644 index 0000000..8d5f995 --- /dev/null +++ b/composer.json @@ -0,0 +1,21 @@ +{ + "name": "cacti/plugin_monitor", + "description": "Monitor Plugin for Cacti", + "type": "cacti-plugin", + "license": "GPL-2.0-or-later", + "require": { + "php": ">=8.0" + }, + "require-dev": { + "pestphp/pest": "^2" + }, + "config": { + "allow-plugins": { + "pestphp/pest-plugin": true + } + }, + "scripts": { + "test": "pest", + "test:integration": "pest tests/Integration" + } +} diff --git a/phpunit.xml.dist b/phpunit.xml.dist new file mode 100644 index 0000000..d42e075 --- /dev/null +++ b/phpunit.xml.dist @@ -0,0 +1,8 @@ + + + + + ./tests/Integration + + + diff --git a/tests/Integration/OutputEscapingTest.php b/tests/Integration/OutputEscapingTest.php new file mode 100644 index 0000000..6d71703 --- /dev/null +++ b/tests/Integration/OutputEscapingTest.php @@ -0,0 +1,49 @@ +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'); + +it('escapes a payload that would break out of a hidden input', function () { + $payload = '" autofocus onfocus="alert(1)'; + $escaped = htmlspecialchars($payload, ENT_QUOTES, 'UTF-8'); + + expect($escaped)->not->toContain('"')->and($escaped)->toContain('"'); +}); 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/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 @@ -