From fce0526d0b7820fad90ca50c48a22d6065438945 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Wed, 8 Apr 2026 22:59:38 -0700 Subject: [PATCH 1/2] refactor(php74): use null coalescing (??) and ??= operators Signed-off-by: Thomas Vincent --- RouterOS/routeros_api.class.php | 4 ++-- mikrotik_users.php | 11 +++++++-- poller_mikrotik.php | 42 ++++++++++++++++----------------- 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/RouterOS/routeros_api.class.php b/RouterOS/routeros_api.class.php index 28e242d..5490431 100644 --- a/RouterOS/routeros_api.class.php +++ b/RouterOS/routeros_api.class.php @@ -175,7 +175,7 @@ public function parseResponse($response) { if ($MATCHES[0][0] == 'ret') { $singlevalue = $MATCHES[0][1]; } - $CURRENT[$MATCHES[0][0]] = (isset($MATCHES[0][1]) ? $MATCHES[0][1] : ''); + $CURRENT[$MATCHES[0][0]] = ($MATCHES[0][1] ?? ''); } } } @@ -215,7 +215,7 @@ public function parseResponse4Smarty($response) { if ($MATCHES[0][0] == 'ret') { $singlevalue = $MATCHES[0][1]; } - $CURRENT[$MATCHES[0][0]] = (isset($MATCHES[0][1]) ? $MATCHES[0][1] : ''); + $CURRENT[$MATCHES[0][0]] = ($MATCHES[0][1] ?? ''); } } } diff --git a/mikrotik_users.php b/mikrotik_users.php index 4ac641c..70e5fe4 100644 --- a/mikrotik_users.php +++ b/mikrotik_users.php @@ -60,7 +60,10 @@ function form_actions() { /* if we are to save this form, instead of display it */ if (isset_request_var('selected_items')) { - $selected_items = unserialize(stripslashes(get_request_var('selected_items'))); + $selected_items = unserialize(stripslashes(get_request_var('selected_items')), ['allowed_classes' => false]); + if (!is_array($selected_items)) { + $selected_items = []; + } if (get_request_var('drp_action') == '1') { /* delete */ if (!isset_request_var('delete_type')) { set_request_var('delete_type', 2); } @@ -105,7 +108,11 @@ function form_actions() { api_graph_remove_multi($graphs_to_act_on); - db_execute("DELETE FROM plugin_mikrotik_users WHERE name IN ('" . implode("','", $devices_to_act_on) . "')"); + // Issue QUEUE-6: use prepared statement for IN() — prevents name-column SQL injection + if (!empty($devices_to_act_on)) { + $placeholders = implode(',', array_fill(0, count($devices_to_act_on), '?')); + db_execute_prepared("DELETE FROM plugin_mikrotik_users WHERE name IN ($placeholders)", $devices_to_act_on); + } } header('Location: mikrotik_users.php?header=false'); diff --git a/poller_mikrotik.php b/poller_mikrotik.php index abacc43..144b65c 100755 --- a/poller_mikrotik.php +++ b/poller_mikrotik.php @@ -1045,7 +1045,7 @@ function mikrotik_dateParse($value) { $value[1] = substr($value[1], 0, strpos($value[1], '.')); } - $date1 = trim($value[0] . ' ' . (isset($value[1]) ? $value[1]:'')); + $date1 = trim($value[0] . ' ' . ($value[1] ?? '')); if (strtotime($date1) === false) { $value = date('Y-m-d H:i:s'); } else { @@ -1304,10 +1304,10 @@ function collect_list_details(&$host) { if (cacti_sizeof($array) && $noServer === false) { foreach($array as $row) { $list['host_id'] = $host['id']; - $list['dynamic'] = isset($row['dynamic']) ? $row['dynamic']:'0'; - $list['disabled']= isset($row['disabled']) ? $row['disabled']:'0'; - $list['list'] = isset($row['list']) ? $row['list']:'N/A'; - $list['address'] = isset($row['address']) ? $row['address']:''; + $list['dynamic'] = $row['dynamic'] ?? '0'; + $list['disabled']= $row['disabled'] ?? '0'; + $list['list'] = $row['list'] ?? 'N/A'; + $list['address'] = $row['address'] ?? ''; $list['created'] = isset($row['creation-time']) ? date('Y-m-d H:i:s', strtotime(str_replace('/', ' ', $row['creation-time']))):date('Y-m-d H:i:s'); $list['timeout'] = isset($row['timeout']) ? mikrotik_parse_ttl($row['timeout']) :'-1'; @@ -1405,11 +1405,11 @@ function collect_dns_details(&$host) { foreach($array as $row) { if (isset($row['type']) && $row['type'] != '-1') { $dns['host_id'] = $host['id']; - $dns['type'] = isset($row['type']) ? $row['type']:'-1'; - $dns['data'] = isset($row['data']) ? $row['data']:'-'; - $dns['name'] = isset($row['name']) ? $row['name']:''; + $dns['type'] = $row['type'] ?? '-1'; + $dns['data'] = $row['data'] ?? '-'; + $dns['name'] = $row['name'] ?? ''; $dns['ttl'] = isset($row['ttl']) ? mikrotik_parse_ttl($row['ttl']) :'0'; - $dns['static'] = isset($row['static']) ? $row['static']:'false'; + $dns['static'] = $row['static'] ?? 'false'; $sql[] = '(' . $dns['host_id'] . ',' . @@ -1552,7 +1552,7 @@ function collect_dhcp_details(&$host) { if (cacti_sizeof($array) && $noServer === false) { foreach($array as $row) { - $mac_address = isset($row['mac-address']) ? $row['mac-address']:'-99'; + $mac_address = $row['mac-address'] ?? '-99'; if (isset($entries[$mac_address]) && $mac_address != 99) { $dhcp = $entries[$mac_address]; } @@ -1564,20 +1564,20 @@ function collect_dhcp_details(&$host) { } $dhcp['host_id'] = $host['id']; - $dhcp['address'] = isset($row['address']) ? $row['address']:'N/A'; - $dhcp['mac_address'] = isset($row['mac-address']) ? $row['mac-address']:'N/A'; - $dhcp['client_id'] = isset($row['client-id']) ? $row['client-id']:''; - $dhcp['address_lists'] = isset($row['address-lists']) ? $row['address-lists']:'N/A'; - $dhcp['server'] = isset($row['server']) ? $row['server']:'N/A'; - $dhcp['dhcp_option'] = isset($row['dhcp-option']) ? $row['dhcp-option']:'N/A'; - $dhcp['status'] = isset($row['status']) ? $row['status']:'N/A'; + $dhcp['address'] = $row['address'] ?? 'N/A'; + $dhcp['mac_address'] = $row['mac-address'] ?? 'N/A'; + $dhcp['client_id'] = $row['client-id'] ?? ''; + $dhcp['address_lists'] = $row['address-lists'] ?? 'N/A'; + $dhcp['server'] = $row['server'] ?? 'N/A'; + $dhcp['dhcp_option'] = $row['dhcp-option'] ?? 'N/A'; + $dhcp['status'] = $row['status'] ?? 'N/A'; $dhcp['expires_after'] = isset($row['expires-after']) ? uptimeToSeconds($row['expires-after']):0; $dhcp['last_seen'] = isset($row['last-seen']) ? uptimeToSeconds($row['last-seen']):0; $dhcp['active_address'] = isset($row['active_address']) ? $row['active-address']:''; - $dhcp['active_mac_address'] = isset($row['active-mac-address']) ? $row['active-mac-address']:''; - $dhcp['active_client_id'] = isset($row['active-client-id']) ? $row['active-client-id']:''; - $dhcp['active_server'] = isset($row['active-server']) ? $row['active-server']:''; - $dhcp['hostname'] = isset($row['host-name']) ? $row['host-name']:''; + $dhcp['active_mac_address'] = $row['active-mac-address'] ?? ''; + $dhcp['active_client_id'] = $row['active-client-id'] ?? ''; + $dhcp['active_server'] = $row['active-server'] ?? ''; + $dhcp['hostname'] = $row['host-name'] ?? ''; $dhcp['radius'] = isset($row['radius']) ? ($row['radius'] == 'true' ? 1:0):0; $dhcp['dynamic'] = isset($row['dynamic']) ? ($row['dynamic'] == 'true' ? 1:0):0; $dhcp['blocked'] = isset($row['blocked']) ? ($row['blocked'] == 'true' ? 1:0):0; From a1d3d9b827f0e110453e59cec60d4a955578eaad Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Thu, 9 Apr 2026 01:58:04 -0700 Subject: [PATCH 2/2] chore: add copilot instructions and CI workflow Signed-off-by: Thomas Vincent --- .github/copilot-instructions.md | 23 +++ .github/workflows/plugin-ci-workflow.yml | 225 +++++++++++++++++++++++ 2 files changed, 248 insertions(+) create mode 100644 .github/copilot-instructions.md create mode 100644 .github/workflows/plugin-ci-workflow.yml diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000..54a69ad --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,23 @@ +# Cacti mikrotik Plugin AI Instructions + +## Project Overview +This is a Cacti plugin. It integrates with the Cacti monitoring platform via the plugin hook architecture. + +## Technology Stack +- PHP 7.4+ (targeting Cacti 1.2.x compatibility) +- MySQL/MariaDB via Cacti's DB abstraction layer +- PSR-12 coding standards + +## Key Rules +- Use prepared statements (db_execute_prepared, db_fetch_row_prepared, etc.) for ALL queries with variables +- Use get_request_var() / get_filter_request_var() for ALL user input, never raw $_REQUEST/$_GET/$_POST +- Use html_escape() / htmlspecialchars() for ALL output of DB/user values in HTML context +- Use cacti_escapeshellarg() for ALL shell command arguments +- No PHP 8.0+ features (str_contains, match, union types, named args) - target PHP 7.4 +- Use ?? and ??= operators (PHP 7.4) instead of isset() ternary patterns +- All unserialize() calls must use allowed_classes => false + +## Testing +- Tests in tests/ directory +- Use Pest PHP or PHPUnit +- php -l lint check required before commit diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml new file mode 100644 index 0000000..f7e650c --- /dev/null +++ b/.github/workflows/plugin-ci-workflow.yml @@ -0,0 +1,225 @@ +# +-------------------------------------------------------------------------+ +# | 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: Plugin Integration Tests + +on: + push: + branches: + - main + - develop + pull_request: + branches: + - main + - develop + +jobs: + integration-test: + runs-on: ${{ matrix.os }} + + strategy: + fail-fast: false + matrix: + php: ['8.1', '8.2', '8.3', '8.4'] + os: [ubuntu-latest] + + services: + mariadb: + image: mariadb:10.6 + env: + MYSQL_ROOT_PASSWORD: cactiroot + MYSQL_DATABASE: cacti + MYSQL_USER: cactiuser + MYSQL_PASSWORD: cactiuser + ports: + - 3306:3306 + options: >- + --health-cmd="mysqladmin ping" + --health-interval=10s + --health-timeout=5s + --health-retries=3 + + name: PHP ${{ matrix.php }} Integration Test on ${{ matrix.os }} + + steps: + - name: Checkout Cacti + uses: actions/checkout@v4 + with: + repository: Cacti/cacti + path: cacti + + - name: Checkout mikrotik Plugin + uses: actions/checkout@v4 + with: + path: cacti/plugins/mikrotik + + - name: Install PHP ${{ matrix.php }} + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + extensions: intl, mysql, gd, ldap, gmp, xml, curl, json, mbstring + ini-values: "post_max_size=256M, max_execution_time=60, date.timezone=America/New_York" + + - 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${{ matrix.php }} + + - 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 + 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' + 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 + + - name: Validate composer files + run: | + cd ${{ github.workspace }}/cacti + if [ -f composer.json ]; then + composer validate --strict || true + fi + + - name: Install Composer Dependencies + run: | + cd ${{ github.workspace }}/cacti + if [ -f composer.json ]; then + sudo composer install --prefer-dist --no-progress + fi + + - name: Create Cacti config.php + run: | + cat ${{ github.workspace }}/cacti/include/config.php.dist | \ + sed -r "s/localhost/127.0.0.1/g" | \ + sed -r "s/'cacti'/'cacti'/g" | \ + 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 cli/install_cacti.php --accept-eula --install --force + + - name: Install mikrotik Plugin + run: | + cd ${{ github.workspace }}/cacti + sudo php cli/plugin_manage.php --plugin=mikrotik --install --enable + +# - name: import mikrotik Plugin Sample Data +# run: | +# cd ${{ github.workspace }}/cacti/plugins/mikrotik +# sudo php cli_import.php --filename=.github/workflows/mikrotik_sample_data.xml +# if [ $? -ne 0 ]; then +# echo "Failed to import Thold sample data" +# exit 1 +# fi + + - name: Check PHP Syntax for Plugin + run: | + cd ${{ github.workspace }}/cacti/plugins/mikrotik + if find . -name '*.php' -exec php -l {} 2>&1 \; | grep -iv 'no syntax errors detected'; then + echo "Syntax errors found!" + exit 1 + fi + + - name: Remove the plugins directory exclusion from the .phpstan.neon + run: sed '/plugins/d' -i .phpstan.neon + working-directory: ${{ github.workspace }}/cacti + + - name: Mark composer scripts executable + run: sudo chmod +x ${{ github.workspace }}/cacti/include/vendor/bin/* + + - name: Run Linter on base code + run: composer run-script lint ${{ github.workspace }}/cacti/plugins/mikrotik + working-directory: ${{ github.workspace }}/cacti + + - name: Checking coding standards on base code + run: composer run-script phpcsfixer ${{ github.workspace }}/cacti/plugins/mikrotik + 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/mikrotik +# working-directory: ${{ github.workspace }}/cacti + + - name: Run Cacti Poller + run: | + cd ${{ github.workspace }}/cacti + sudo 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: | + if [ -f ${{ github.workspace }}/cacti/log/cacti.log ]; then + echo "=== Cacti Log ===" + sudo cat ${{ github.workspace }}/cacti/log/cacti.log + fi