test: add Pest v1 security test infrastructure by somethingwithproof · Pull Request #251 · Cacti/plugin_flowview

somethingwithproof · 2026-04-09T06:55:05Z

Summary

Add Pest v1 test scaffold with Cacti framework stubs
Source-scan tests for prepared statement consistency
PHP 7.4 compatibility verification tests
Plugin setup.php structure validation

Test plan

composer install && vendor/bin/pest passes
Tests verify security patterns match hardening PRs

Add source-scan tests verifying security patterns (prepared statements, output escaping, auth guards, PHP 7.4 compatibility) remain in place across refactors. Tests run with Pest v1 (PHP 7.3+) and stub the Cacti framework so plugins can be tested in isolation. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Copilot

Pull request overview

Adds a Pest v1-based test harness for the Flowview plugin, focused on static security/regression checks and basic PHP 7.4 compatibility guards.

Changes:

Introduces Pest bootstrap/config and security-focused Pest test suites.
Adds source-scanning tests for prepared-statement usage and setup.php structure.
Adds a minimal composer.json for dev dependencies and a Dependabot configuration file.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`tests/Pest.php`	Pest entrypoint that loads the test bootstrap.
`tests/bootstrap.php`	Defines Cacti/framework function stubs for isolated testing.
`tests/Security/SetupStructureTest.php`	Static checks that `setup.php` contains required plugin hook functions and version keys.
`tests/Security/PreparedStatementConsistencyTest.php`	Static scan intended to prevent reintroduction of raw `db_*` calls.
`tests/Security/Php74CompatibilityTest.php`	Static scan for a few PHP 8.0-only constructs.
`composer.json`	Adds Pest v1 as a dev dependency and dev autoloading.
`.github/dependabot.yml`	Adds Dependabot configuration (currently npm + GitHub Actions).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T06:59:52Z

tests/Security/PreparedStatementConsistencyTest.php

+	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',


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 · 2026-04-09T06:59:53Z

tests/Security/PreparedStatementConsistencyTest.php

+			$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++;
+				}
+			}
+


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 · 2026-04-09T06:59:53Z

tests/Security/SetupStructureTest.php

+	$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));
+


realpath() can return false and file_get_contents() can return false, which will emit warnings and then make the subsequent toContain() assertions fail with an unhelpful type/value. Add explicit assertions that the path exists and the file contents were read successfully (fail fast with a clear message) before running the string checks.

Suggested change

$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));

$setupPath = realpath(__DIR__ . '/../../setup.php');

\PHPUnit\Framework\Assert::assertNotFalse($setupPath, 'Failed to resolve setup.php path.');

$source = file_get_contents($setupPath);

\PHPUnit\Framework\Assert::assertNotFalse($source, 'Failed to read setup.php contents.');

Copilot · 2026-04-09T06:59:53Z

tests/bootstrap.php

+	function db_execute($sql) {
+		$GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute', 'sql' => $sql, 'params' => array());
+		return true;
+	}
+}
+
+if (!function_exists('db_execute_prepared')) {
+	function db_execute_prepared($sql, $params = array()) {
+		$GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute_prepared', 'sql' => $sql, 'params' => $params);
+		return true;
+	}
+}
+
+if (!function_exists('db_fetch_assoc')) {
+	function db_fetch_assoc($sql) {
+		return array();
+	}
+}
+
+if (!function_exists('db_fetch_assoc_prepared')) {
+	function db_fetch_assoc_prepared($sql, $params = array()) {
+		return array();
+	}
+}
+
+if (!function_exists('db_fetch_row')) {
+	function db_fetch_row($sql) {
+		return array();
+	}
+}
+
+if (!function_exists('db_fetch_row_prepared')) {
+	function db_fetch_row_prepared($sql, $params = array()) {
+		return array();
+	}
+}
+
+if (!function_exists('db_fetch_cell')) {
+	function db_fetch_cell($sql) {
+		return '';
+	}
+}
+
+if (!function_exists('db_fetch_cell_prepared')) {
+	function db_fetch_cell_prepared($sql, $params = array()) {


The db_execute() stub only accepts one parameter, but plugin code calls db_execute($sql, $log, $conn) via flowview_db_execute() (see database.php:70). If any test starts loading plugin code, this will raise an argument count error. Update the stub signatures to accept the optional parameters used in production (and similarly for fetch helpers) so the bootstrap is compatible with the plugin.

Suggested change

function db_execute($sql) {

$GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute', 'sql' => $sql, 'params' => array());

return true;

}

}

if (!function_exists('db_execute_prepared')) {

function db_execute_prepared($sql, $params = array()) {

$GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute_prepared', 'sql' => $sql, 'params' => $params);

return true;

}

}

if (!function_exists('db_fetch_assoc')) {

function db_fetch_assoc($sql) {

return array();

}

}

if (!function_exists('db_fetch_assoc_prepared')) {

function db_fetch_assoc_prepared($sql, $params = array()) {

return array();

}

}

if (!function_exists('db_fetch_row')) {

function db_fetch_row($sql) {

return array();

}

}

if (!function_exists('db_fetch_row_prepared')) {

function db_fetch_row_prepared($sql, $params = array()) {

return array();

}

}

if (!function_exists('db_fetch_cell')) {

function db_fetch_cell($sql) {

return '';

}

}

if (!function_exists('db_fetch_cell_prepared')) {

function db_fetch_cell_prepared($sql, $params = array()) {

function db_execute($sql, $log = true, $conn = false) {

$GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute', 'sql' => $sql, 'params' => array());

return true;

}

}

if (!function_exists('db_execute_prepared')) {

function db_execute_prepared($sql, $params = array(), $conn = false) {

$GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute_prepared', 'sql' => $sql, 'params' => $params);

return true;

}

}

if (!function_exists('db_fetch_assoc')) {

function db_fetch_assoc($sql, $log = true, $conn = false) {

return array();

}

}

if (!function_exists('db_fetch_assoc_prepared')) {

function db_fetch_assoc_prepared($sql, $params = array(), $conn = false) {

return array();

}

}

if (!function_exists('db_fetch_row')) {

function db_fetch_row($sql, $log = true, $conn = false) {

return array();

}

}

if (!function_exists('db_fetch_row_prepared')) {

function db_fetch_row_prepared($sql, $params = array(), $conn = false) {

return array();

}

}

if (!function_exists('db_fetch_cell')) {

function db_fetch_cell($sql, $log = true, $conn = false) {

return '';

}

}

if (!function_exists('db_fetch_cell_prepared')) {

function db_fetch_cell_prepared($sql, $params = array(), $conn = false) {

Copilot · 2026-04-09T06:59:53Z

tests/bootstrap.php

+}
+
+if (!function_exists('__esc')) {
+	function __esc($text, $domain = '') {


__esc() is used throughout the plugin with sprintf-style arguments (e.g. __esc('Listener [edit: %s]', $device['name'], 'flowview')), but the stub only accepts ($text, $domain = ''). This will break as soon as any test evaluates code paths that call __esc. Consider implementing the stub with a variadic signature and applying vsprintf when additional args are provided.

Suggested change

function __esc($text, $domain = '') {

function __esc($text, ...$args) {

$domain = '';

if (!empty($args) && is_string($args[count($args) - 1])) {

$domain = array_pop($args);

}

if (!empty($args)) {

$text = vsprintf($text, $args);

}

Copilot · 2026-04-09T06:59:54Z

.github/dependabot.yml

+version: 2
+updates:
+  - package-ecosystem: "npm"
+    directory: "/"
+    schedule:
+      interval: "weekly"
+    open-pull-requests-limit: 10
+  - package-ecosystem: "github-actions"


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 · 2026-04-09T06:59:54Z

tests/Security/PreparedStatementConsistencyTest.php

+			if ($path === false) {
+				continue;
+			}
+
+			$contents = file_get_contents($path);
+
+			if ($contents === false) {
+				continue;
+			}


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 · 2026-04-09T06:59:54Z

tests/Security/Php74CompatibilityTest.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;
+			}
+


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);

…dabot - Throw RuntimeException when realpath/file_get_contents fails (previously silent continue hid unscanned files) - Fix Dependabot ecosystem from npm to composer - Remove committed .omc session artifacts, add .omc/ to .gitignore Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

somethingwithproof · 2026-04-11T00:09:40Z

Converted to draft to serialize the stack in this repo. Blocked by #250; will un-draft after that merges to avoid cross-PR merge conflicts.

somethingwithproof added 4 commits March 15, 2026 16:58

ci: add dependabot config with npm and github-actions

3334853

ci: add CodeQL (JS/TS, excludes PHP) with workflow_dispatch

c33d361

ci: remove CodeQL JS/TS workflow (no JavaScript in this repo)

26a30ff

Copilot AI review requested due to automatic review settings April 9, 2026 06:55

Copilot started reviewing on behalf of somethingwithproof April 9, 2026 06:55 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

somethingwithproof added 2 commits April 9, 2026 23:02

style: remove trailing double semicolons

da23982

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

somethingwithproof marked this pull request as draft April 11, 2026 00:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add Pest v1 security test infrastructure#251

test: add Pest v1 security test infrastructure#251
somethingwithproof wants to merge 6 commits intoCacti:developfrom
somethingwithproof:test/add-security-test-infrastructure

somethingwithproof commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

somethingwithproof commented Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-			$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]);

		$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));

-	$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));
+	$setupPath = realpath(__DIR__ . '/../../setup.php');
+	\PHPUnit\Framework\Assert::assertNotFalse($setupPath, 'Failed to resolve setup.php path.');
+	$source = file_get_contents($setupPath);
+	\PHPUnit\Framework\Assert::assertNotFalse($source, 'Failed to read setup.php contents.');

-	function __esc($text, $domain = '') {
+	function __esc($text, ...$args) {
+		$domain = '';
+		if (!empty($args) && is_string($args[count($args) - 1])) {
+			$domain = array_pop($args);
+		}
+		if (!empty($args)) {
+			$text = vsprintf($text, $args);
+		}

Conversation

somethingwithproof commented Apr 9, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof commented Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants