Skip to content

test: add Pest v1 security test infrastructure #55

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

Open
somethingwithproof wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

test: add Pest v1 security test infrastructure #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
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';
101 changes: 101 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,101 @@
<?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 audit', function () {
$files = array(
'audit.php',
'audit_functions.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;
}

expect(preg_match('/\?->/', $contents))->toBe(0,
"{$relativeFile} uses nullsafe operator which requires PHP 8.0"
);
}
Comment on lines +22 to +99
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

These compatibility checks silently skip files that can't be resolved/read (continue on $path === false / $contents === false). That can lead to false positives where the test suite passes without actually checking anything (and the same pattern repeats in each it(...) block). Prefer failing fast if a required file isn't readable, and consider extracting the repeated loop into a helper to avoid duplicated logic across the four tests.

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;
}
expect(preg_match('/\?->/', $contents))->toBe(0,
"{$relativeFile} uses nullsafe operator which requires PHP 8.0"
);
}
$readFileContents = function (string $relativeFile): string {
$path = realpath(__DIR__ . '/../../' . $relativeFile);
if ($path === false) {
throw new RuntimeException("Unable to resolve required file: {$relativeFile}");
}
$contents = file_get_contents($path);
if ($contents === false) {
throw new RuntimeException("Unable to read required file: {$relativeFile}");
}
return $contents;
};
$assertPatternAbsent = function (string $pattern, string $message) use ($files, $readFileContents) {
foreach ($files as $relativeFile) {
$contents = $readFileContents($relativeFile);
expect(preg_match($pattern, $contents))->toBe(0, sprintf($message, $relativeFile));
}
};
it('does not use str_contains (PHP 8.0)', function () use ($assertPatternAbsent) {
$assertPatternAbsent('/\bstr_contains\s*\(/', '%s uses str_contains() which requires PHP 8.0');
});
it('does not use str_starts_with (PHP 8.0)', function () use ($assertPatternAbsent) {
$assertPatternAbsent('/\bstr_starts_with\s*\(/', '%s uses str_starts_with() which requires PHP 8.0');
});
it('does not use str_ends_with (PHP 8.0)', function () use ($assertPatternAbsent) {
$assertPatternAbsent('/\bstr_ends_with\s*\(/', '%s uses str_ends_with() which requires PHP 8.0');
});
it('does not use nullsafe operator (PHP 8.0)', function () use ($assertPatternAbsent) {
$assertPatternAbsent('/\?->/', '%s uses nullsafe operator which requires PHP 8.0');

Copilot uses AI. Check for mistakes.
});
});
59 changes: 59 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,59 @@
<?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 audit', function () {
it('uses prepared DB helpers in all plugin files', function () {
$targetFiles = array(
'audit.php',
'audit_functions.php',
'setup.php',
);

$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 +29 to +37
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

This test silently continues when a target file can't be resolved/read. That makes the assertion meaningless in those cases (the test will pass even though nothing was checked). Since these are required plugin files, it would be better to fail fast with a clear expectation when $path/$contents is false.

Suggested change
if ($path === false) {
continue;
}
$contents = file_get_contents($path);
if ($contents === false) {
continue;
}
expect($path)->not->toBeFalse(
"Required plugin file {$relativeFile} could not be resolved"
);
$contents = file_get_contents($path);
expect($contents)->not->toBeFalse(
"Required plugin file {$relativeFile} could not be read"
);

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

expect($rawCallsOutsideComments)->toBe(0,
"File {$relativeFile} contains raw (unprepared) DB calls"
);
Comment on lines +23 to +56
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

The assertion expects zero raw db_execute/db_fetch_* calls in audit.php, audit_functions.php, and setup.php, but the current plugin code contains raw calls (e.g., audit.php uses db_execute('TRUNCATE TABLE audit_log') and setup.php has multiple db_execute(...)/db_fetch_cell(...)). As written, this test will fail as soon as Pest is run. Either narrow the rule to what the codebase actually enforces (e.g., only forbid raw calls when user input is involved) or update the plugin code in the same PR to satisfy this requirement.

Copilot uses AI. Check for mistakes.
}
});
});
36 changes: 36 additions & 0 deletions tests/Security/SetupStructureTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/

/*
* Verify setup.php defines required plugin hooks and info function.
*/

describe('audit setup.php structure', function () {
$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));

Comment on lines +15 to +16
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

realpath() can return false; passing that directly to file_get_contents() will emit warnings and $source may become false, which then causes confusing failures in later expectations. Consider asserting that the resolved path/content is not false (and failing the test with a clear message) before running the toContain/toMatch checks.

Suggested change
$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));
$setup_path = realpath(__DIR__ . '/../../setup.php');
if ($setup_path === false) {
throw new RuntimeException('Failed to resolve setup.php path for structure test.');
}
$source = file_get_contents($setup_path);
if ($source === false) {
throw new RuntimeException('Failed to read setup.php contents for structure test.');
}

Copilot uses AI. Check for mistakes.
it('defines plugin_audit_install function', function () use ($source) {
expect($source)->toContain('function plugin_audit_install');
});

it('defines plugin_audit_version function', function () use ($source) {
expect($source)->toContain('function plugin_audit_version');
});

it('defines plugin_audit_uninstall function', function () use ($source) {
expect($source)->toContain('function plugin_audit_uninstall');
});

it('returns version array with name key', function () use ($source) {
expect($source)->toMatch('/[\'\""]name[\'\""]\s*=>/');
});

it('returns version array with version key', function () use ($source) {
expect($source)->toMatch('/[\'\""]version[\'\""]\s*=>/');
});
});
Loading
Loading