Skip to content

test: add Pest v1 security test infrastructure #251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
somethingwithproof wants to merge 6 commits into
base: develop
Choose a base branch
from
Draft

test: add Pest v1 security test infrastructure #251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3334853
ci: add dependabot config with npm and github-actions
somethingwithproof Mar 15, 2026
c33d361
ci: add CodeQL (JS/TS, excludes PHP) with workflow_dispatch
somethingwithproof Mar 15, 2026
26a30ff
ci: remove CodeQL JS/TS workflow (no JavaScript in this repo)
somethingwithproof Mar 16, 2026
0bae121
test: add Pest v1 security test infrastructure
somethingwithproof Apr 9, 2026
a5c8c58
fix(tests): harden Pest infra - fail-fast on missing files, fix depen…
somethingwithproof Apr 10, 2026
da23982
style: remove trailing double semicolons
somethingwithproof Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/dependabot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
version: 2
updates:
- package-ecosystem: "composer"
directory: "/"
schedule:
interval: "weekly"
open-pull-requests-limit: 10
- package-ecosystem: "github-actions"
Comment on lines +1 to +8
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependabot is configured for the npm ecosystem, but this repository has no package.json / npm dependencies. With the new composer.json, it would be more accurate to switch this entry to the composer ecosystem (and keep github-actions if desired) to avoid dependabot configuration errors/no-op updates.

Copilot uses AI. Check for mistakes.
directory: "/"
schedule:
interval: "weekly"
open-pull-requests-limit: 10
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@
.git*
config.php
locales/po/*.mo
.omc/
18 changes: 18 additions & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"name": "cacti/plugin_flowview",
"description": "plugin_flowview plugin for Cacti",
"license": "GPL-2.0-or-later",
"require-dev": {
"pestphp/pest": "^1.23"
},
"config": {
"allow-plugins": {
"pestphp/pest-plugin": true
}
},
"autoload-dev": {
"files": [
"tests/bootstrap.php"
]
}
}
40 changes: 20 additions & 20 deletions database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ function flowview_db_close(&$flowview_cnn) {
* @return '1' for success, '0' for error
*/
function flowview_db_execute($sql, $log = true, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_execute($sql, $log, $flowview_cnn);
}
Expand All @@ -80,7 +80,7 @@ function flowview_db_execute($sql, $log = true, $cnn_id = false) {
* @return '1' for success, '0' for error
*/
function flowview_db_execute_prepared($sql, $parms = array(), $log = true, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_execute_prepared($sql, $parms, $log, $flowview_cnn);
}
Expand All @@ -97,7 +97,7 @@ function flowview_db_execute_prepared($sql, $parms = array(), $log = true, $cnn_
* @return (bool) the output of the sql query as a single variable
*/
function flowview_db_fetch_cell($sql, $col_name = '', $log = true, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_fetch_cell($sql, $col_name, $log, $flowview_cnn);
}
Expand All @@ -115,7 +115,7 @@ function flowview_db_fetch_cell($sql, $col_name = '', $log = true, $cnn_id = fal
* @return (bool) the output of the sql query as a single variable
*/
function flowview_db_fetch_cell_prepared($sql, $params = array(), $col_name = '', $log = true, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_fetch_cell_prepared($sql, $params, $col_name, $log, $flowview_cnn);
}
Expand All @@ -130,7 +130,7 @@ function flowview_db_fetch_cell_prepared($sql, $params = array(), $col_name = ''
* @return the first row of the result as a hash
*/
function flowview_db_fetch_row($sql, $log = true, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_fetch_row($sql, $log, $flowview_cnn);
}
Expand All @@ -146,7 +146,7 @@ function flowview_db_fetch_row($sql, $log = true, $cnn_id = false) {
* @return the first row of the result as a hash
*/
function flowview_db_fetch_row_prepared($sql, $params = array(), $log = true, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_fetch_row_prepared($sql, $params, $log, $flowview_cnn);
}
Expand All @@ -161,7 +161,7 @@ function flowview_db_fetch_row_prepared($sql, $params = array(), $log = true, $c
* @return the entire result set as a multi-dimensional hash
*/
function flowview_db_fetch_assoc($sql, $log = true, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_fetch_assoc($sql, $log, $flowview_cnn);
}
Expand All @@ -177,7 +177,7 @@ function flowview_db_fetch_assoc($sql, $log = true, $cnn_id = false) {
* @return the entire result set as a multi-dimensional hash
*/
function flowview_db_fetch_assoc_prepared($sql, $params = array(), $log = true, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_fetch_assoc_prepared($sql, $params, $log, $flowview_cnn);
}
Expand All @@ -190,7 +190,7 @@ function flowview_db_fetch_assoc_prepared($sql, $params = array(), $log = true,
* @return the id of the last auto incriment row that was created
*/
function flowview_db_fetch_insert_id($cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_fetch_insert_id($flowview_cnn);
}
Expand All @@ -206,7 +206,7 @@ function flowview_db_fetch_insert_id($cnn_id = false) {
* @return the auto incriment id column (if applicable)
*/
function flowview_db_replace($table_name, $array_items, $keyCols, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_replace($table_name, $array_items, $keyCols, $flowview_cnn);
}
Expand All @@ -223,7 +223,7 @@ function flowview_db_replace($table_name, $array_items, $keyCols, $cnn_id = fals
* @return the auto incriment id column (if applicable)
*/
function flowview_sql_save($array_items, $table_name, $key_cols = 'id', $autoinc = true, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return sql_save($array_items, $table_name, $key_cols, $autoinc, $flowview_cnn);
}
Expand All @@ -238,7 +238,7 @@ function flowview_sql_save($array_items, $table_name, $key_cols = 'id', $autoinc
* @return (bool) the output of the sql query as a single variable
*/
function flowview_db_table_exists($table, $log = true, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

preg_match("/([`]{0,1}(?<database>[\w_]+)[`]{0,1}\.){0,1}[`]{0,1}(?<table>[\w_]+)[`]{0,1}/", $table, $matches);
if ($matches !== false && array_key_exists('table', $matches)) {
Expand All @@ -250,7 +250,7 @@ function flowview_db_table_exists($table, $log = true, $cnn_id = false) {
}

function flowview_db_table_create($table, $data, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

$result = flowview_db_fetch_assoc('SHOW TABLES');
$tables = array();
Expand Down Expand Up @@ -352,13 +352,13 @@ function flowview_db_table_create($table, $data, $cnn_id = false) {
}

function flowview_db_column_exists($table, $column, $log = true, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_column_exists($table, $column, $log, $flowview_cnn);
}

function flowview_db_add_column($table, $column, $log = true, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_add_column($table, $column, $log, $flowview_cnn);
}
Expand All @@ -372,9 +372,9 @@ function flowview_db_add_column($table, $column, $log = true, $cnn_id = false) {
* or false on error
*/
function flowview_db_affected_rows($cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_affected_rows($flowview_cnn);;
return db_affected_rows($flowview_cnn);
}

/**
Expand All @@ -388,7 +388,7 @@ function flowview_db_affected_rows($cnn_id = false) {
* @return bool The output of the sql query as a single variable
*/
function flowview_db_index_exists($table, $index, $log = true, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

return db_index_exists($table, $index, $log, $flowview_cnn);
}
Expand All @@ -412,13 +412,13 @@ function flowview_get_connection($cnn_id) {
* @return (array) An array of column types indexed by the column names
*/
function flowview_db_get_table_column_types($table, $cnn_id = false) {
$flowview_cnn = flowview_get_connection($cnn_id);;
$flowview_cnn = flowview_get_connection($cnn_id);

$columns = db_fetch_assoc("SHOW COLUMNS FROM $table", false, $flowview_cnn);
$cols = array();
if (cacti_sizeof($columns)) {
foreach($columns as $col) {
$cols[$col['Field']] = array('type' => $col['Type'], 'null' => $col['Null'], 'default' => $col['Default'], 'extra' => $col['Extra']);;
$cols[$col['Field']] = array('type' => $col['Type'], 'null' => $col['Null'], 'default' => $col['Default'], 'extra' => $col['Extra']);
}
}

Expand Down
14 changes: 14 additions & 0 deletions tests/Pest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/

/*
* Pest configuration file.
*/

require_once __DIR__ . '/bootstrap.php';
103 changes: 103 additions & 0 deletions tests/Security/Php74CompatibilityTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/

/*
* Verify plugin source files do not use PHP 8.0+ syntax.
* Cacti 1.2.x plugins must remain compatible with PHP 7.4.
*/

describe('PHP 7.4 compatibility in flowview', function () {
$files = array(
'flowview.php',
'flowview_devices.php',
'flowview_filters.php',
'flowview_schedules.php',
'setup.php',
);

it('does not use str_contains (PHP 8.0)', function () use ($files) {
foreach ($files as $relativeFile) {
$path = realpath(__DIR__ . '/../../' . $relativeFile);

if ($path === false) {
continue;
}

$contents = file_get_contents($path);

if ($contents === false) {
continue;
}

expect(preg_match('/\bstr_contains\s*\(/', $contents))->toBe(0,
"{$relativeFile} uses str_contains() which requires PHP 8.0"
);
}
});

it('does not use str_starts_with (PHP 8.0)', function () use ($files) {
foreach ($files as $relativeFile) {
$path = realpath(__DIR__ . '/../../' . $relativeFile);

if ($path === false) {
continue;
}

$contents = file_get_contents($path);

if ($contents === false) {
continue;
}

expect(preg_match('/\bstr_starts_with\s*\(/', $contents))->toBe(0,
"{$relativeFile} uses str_starts_with() which requires PHP 8.0"
);
}
});

it('does not use str_ends_with (PHP 8.0)', function () use ($files) {
foreach ($files as $relativeFile) {
$path = realpath(__DIR__ . '/../../' . $relativeFile);

if ($path === false) {
continue;
}

$contents = file_get_contents($path);

if ($contents === false) {
continue;
}

expect(preg_match('/\bstr_ends_with\s*\(/', $contents))->toBe(0,
"{$relativeFile} uses str_ends_with() which requires PHP 8.0"
);
}
});

it('does not use nullsafe operator (PHP 8.0)', function () use ($files) {
foreach ($files as $relativeFile) {
$path = realpath(__DIR__ . '/../../' . $relativeFile);

if ($path === false) {
continue;
}

$contents = file_get_contents($path);

if ($contents === false) {
continue;
}

Comment on lines +24 to +97
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each loop continues when a target file is missing/unreadable, which can produce false-green results (compatibility constraints aren’t actually enforced for that file). Consider asserting that realpath() and file_get_contents() succeed for every expected plugin file so the test fails if it can’t validate the source.

Suggested change
it('does not use str_contains (PHP 8.0)', function () use ($files) {
foreach ($files as $relativeFile) {
$path = realpath(__DIR__ . '/../../' . $relativeFile);
if ($path === false) {
continue;
}
$contents = file_get_contents($path);
if ($contents === false) {
continue;
}
expect(preg_match('/\bstr_contains\s*\(/', $contents))->toBe(0,
"{$relativeFile} uses str_contains() which requires PHP 8.0"
);
}
});
it('does not use str_starts_with (PHP 8.0)', function () use ($files) {
foreach ($files as $relativeFile) {
$path = realpath(__DIR__ . '/../../' . $relativeFile);
if ($path === false) {
continue;
}
$contents = file_get_contents($path);
if ($contents === false) {
continue;
}
expect(preg_match('/\bstr_starts_with\s*\(/', $contents))->toBe(0,
"{$relativeFile} uses str_starts_with() which requires PHP 8.0"
);
}
});
it('does not use str_ends_with (PHP 8.0)', function () use ($files) {
foreach ($files as $relativeFile) {
$path = realpath(__DIR__ . '/../../' . $relativeFile);
if ($path === false) {
continue;
}
$contents = file_get_contents($path);
if ($contents === false) {
continue;
}
expect(preg_match('/\bstr_ends_with\s*\(/', $contents))->toBe(0,
"{$relativeFile} uses str_ends_with() which requires PHP 8.0"
);
}
});
it('does not use nullsafe operator (PHP 8.0)', function () use ($files) {
foreach ($files as $relativeFile) {
$path = realpath(__DIR__ . '/../../' . $relativeFile);
if ($path === false) {
continue;
}
$contents = file_get_contents($path);
if ($contents === false) {
continue;
}
$readPluginFile = function (string $relativeFile): string {
$path = realpath(__DIR__ . '/../../' . $relativeFile);
if ($path === false) {
throw new \RuntimeException("Failed to resolve expected plugin file: {$relativeFile}");
}
$contents = file_get_contents($path);
if ($contents === false) {
throw new \RuntimeException("Failed to read expected plugin file: {$relativeFile}");
}
return $contents;
};
it('does not use str_contains (PHP 8.0)', function () use ($files, $readPluginFile) {
foreach ($files as $relativeFile) {
$contents = $readPluginFile($relativeFile);
expect(preg_match('/\bstr_contains\s*\(/', $contents))->toBe(0,
"{$relativeFile} uses str_contains() which requires PHP 8.0"
);
}
});
it('does not use str_starts_with (PHP 8.0)', function () use ($files, $readPluginFile) {
foreach ($files as $relativeFile) {
$contents = $readPluginFile($relativeFile);
expect(preg_match('/\bstr_starts_with\s*\(/', $contents))->toBe(0,
"{$relativeFile} uses str_starts_with() which requires PHP 8.0"
);
}
});
it('does not use str_ends_with (PHP 8.0)', function () use ($files, $readPluginFile) {
foreach ($files as $relativeFile) {
$contents = $readPluginFile($relativeFile);
expect(preg_match('/\bstr_ends_with\s*\(/', $contents))->toBe(0,
"{$relativeFile} uses str_ends_with() which requires PHP 8.0"
);
}
});
it('does not use nullsafe operator (PHP 8.0)', function () use ($files, $readPluginFile) {
foreach ($files as $relativeFile) {
$contents = $readPluginFile($relativeFile);

Copilot uses AI. Check for mistakes.
expect(preg_match('/\?->/', $contents))->toBe(0,
"{$relativeFile} uses nullsafe operator which requires PHP 8.0"
);
}
});
});
61 changes: 61 additions & 0 deletions tests/Security/PreparedStatementConsistencyTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/

/*
* Verify migrated files use prepared DB helpers exclusively.
* Catches regressions where raw db_execute/db_fetch_* calls creep back in.
*/

describe('prepared statement consistency in flowview', function () {
it('uses prepared DB helpers in all plugin files', function () {
$targetFiles = array(
'flowview.php',
'flowview_devices.php',
'flowview_filters.php',
'flowview_schedules.php',
'setup.php',
Comment on lines +16 to +22
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setup.php is included in $targetFiles, but the current setup.php in this repo contains raw db_fetch_cell( / db_execute( calls (e.g. setup.php:110, setup.php:324, setup.php:1024). This test will fail immediately under vendor/bin/pest even without any regressions. Either (a) migrate those calls to prepared variants / wrappers, or (b) narrow the check (e.g. exclude setup.php and/or ignore DDL-only db_execute() usage) so the new test suite is green on the current codebase.

Suggested change
it('uses prepared DB helpers in all plugin files', function () {
$targetFiles = array(
'flowview.php',
'flowview_devices.php',
'flowview_filters.php',
'flowview_schedules.php',
'setup.php',
it('uses prepared DB helpers in all migrated plugin files', function () {
$targetFiles = array(
'flowview.php',
'flowview_devices.php',
'flowview_filters.php',
'flowview_schedules.php',

Copilot uses AI. Check for mistakes.
);

$rawPattern = '/\bdb_(?:execute|fetch_row|fetch_assoc|fetch_cell)\s*\(/';
$preparedPattern = '/\bdb_(?:execute|fetch_row|fetch_assoc|fetch_cell)_prepared\s*\(/';

foreach ($targetFiles as $relativeFile) {
$path = realpath(__DIR__ . '/../../' . $relativeFile);

if ($path === false) {
continue;
}

$contents = file_get_contents($path);

if ($contents === false) {
continue;
}
Comment on lines +31 to +39
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These continue branches mean the test can pass while silently skipping missing/unreadable files (e.g. if the repo layout changes or a file is deleted), which undermines the purpose of a regression/security check. Prefer failing fast with an assertion when realpath() or file_get_contents() fails so the test suite can’t go green without actually validating the files.

Suggested change
if ($path === false) {
continue;
}
$contents = file_get_contents($path);
if ($contents === false) {
continue;
}
expect($path)->not->toBeFalse(
"Failed to resolve expected plugin file: {$relativeFile}"
);
$contents = file_get_contents($path);
expect($contents)->not->toBeFalse(
"Failed to read expected plugin file: {$relativeFile}"
);

Copilot uses AI. Check for mistakes.

$lines = explode("\n", $contents);
$rawCallsOutsideComments = 0;

foreach ($lines as $line) {
$trimmed = ltrim($line);

if (strpos($trimmed, '//') === 0 || strpos($trimmed, '*') === 0 || strpos($trimmed, '#') === 0) {
continue;
}

if (preg_match($rawPattern, $line) && !preg_match($preparedPattern, $line)) {
$rawCallsOutsideComments++;
}
}

Comment on lines +41 to +55
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line-based comment skipping here is not robust: block comments starting with /* (or inline /* ... */) and string literals containing db_execute( will still be scanned and can cause false positives. Consider tokenizing with token_get_all() and ignoring T_COMMENT, T_DOC_COMMENT, and T_CONSTANT_ENCAPSED_STRING tokens before applying the regex, to make this test stable.

Suggested change
$lines = explode("\n", $contents);
$rawCallsOutsideComments = 0;
foreach ($lines as $line) {
$trimmed = ltrim($line);
if (strpos($trimmed, '//') === 0 || strpos($trimmed, '*') === 0 || strpos($trimmed, '#') === 0) {
continue;
}
if (preg_match($rawPattern, $line) && !preg_match($preparedPattern, $line)) {
$rawCallsOutsideComments++;
}
}
$tokens = token_get_all($contents);
$sanitizedContents = '';
foreach ($tokens as $token) {
if (is_array($token)) {
$tokenId = $token[0];
$tokenText = $token[1];
if ($tokenId === T_COMMENT || $tokenId === T_DOC_COMMENT || $tokenId === T_CONSTANT_ENCAPSED_STRING) {
$sanitizedContents .= preg_replace('/[^\r\n]/', ' ', $tokenText);
} else {
$sanitizedContents .= $tokenText;
}
} else {
$sanitizedContents .= $token;
}
}
preg_match_all($rawPattern, $sanitizedContents, $matches);
$rawCallsOutsideComments = count($matches[0]);

Copilot uses AI. Check for mistakes.
expect($rawCallsOutsideComments)->toBe(0,
"File {$relativeFile} contains raw (unprepared) DB calls"
);
}
});
});
Loading