Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
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"
directory: "/"
schedule:
interval: "weekly"
open-pull-requests-limit: 10
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@
# +-------------------------------------------------------------------------+

locales/po/*.mo
.omc/
18 changes: 18 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"name": "cacti/plugin_maint",
"description": "plugin_maint 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"
]
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.

This PR adds a Pest test suite, but the repo’s CI workflow doesn’t run vendor/bin/pest today (it only runs the Cacti integration workflow). Consider adding a scripts entry (e.g., a test script) and/or updating CI to execute the Pest suite so these security/compatibility checks are actually enforced.

Suggested change
]
]
},
"scripts": {
"test": "vendor/bin/pest"

Copilot uses AI. Check for mistakes.
}
}
14 changes: 14 additions & 0 deletions tests/Pest.php
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
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 maint', function () {
$files = array(
'functions.php',
'maint.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 +22 to +95
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 tests continue when realpath()/file_get_contents() fails, which can silently skip a file and still pass. Since the goal is to enforce PHP 7.4 compatibility across specific required files, assert the path is resolvable and contents are readable (fail otherwise).

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;
}
$readRequiredFile = 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;
};
it('does not use str_contains (PHP 8.0)', function () use ($files, $readRequiredFile) {
foreach ($files as $relativeFile) {
$contents = $readRequiredFile($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, $readRequiredFile) {
foreach ($files as $relativeFile) {
$contents = $readRequiredFile($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, $readRequiredFile) {
foreach ($files as $relativeFile) {
$contents = $readRequiredFile($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, $readRequiredFile) {
foreach ($files as $relativeFile) {
$contents = $readRequiredFile($relativeFile);

Copilot uses AI. Check for mistakes.
expect(preg_match('/\?->/', $contents))->toBe(0,
"{$relativeFile} uses nullsafe operator which requires PHP 8.0"
);
}
});
});
59 changes: 59 additions & 0 deletions tests/Security/PreparedStatementConsistencyTest.php
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 maint', function () {
it('uses prepared DB helpers in all plugin files', function () {
$targetFiles = array(
'functions.php',
'maint.php',
'setup.php',
Comment on lines +16 to +20
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.

This test currently fails against the repository’s current code: maint.php and setup.php contain raw db_fetch_assoc( calls (e.g. maint.php:927, 1137, 1161 and setup.php:301). Either migrate those calls to db_fetch_assoc_prepared() (preferred) or adjust targetFiles/add an allowlist for known-safe cases so the test reflects the intended enforcement scope.

Suggested change
it('uses prepared DB helpers in all plugin files', function () {
$targetFiles = array(
'functions.php',
'maint.php',
'setup.php',
it('uses prepared DB helpers in migrated plugin files', function () {
$targetFiles = array(
'functions.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 +30 to +36
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.

Skipping missing/unreadable files (continue when realpath()/file_get_contents() fails) can make this test pass while silently not scanning a target file. Since these files are required for the security guarantee, fail the test with a clear message when a file can’t be resolved or read.

Suggested change
continue;
}
$contents = file_get_contents($path);
if ($contents === false) {
continue;
throw new RuntimeException("Unable to resolve required plugin file: {$relativeFile}");
}
$contents = file_get_contents($path);
if ($contents === false) {
throw new RuntimeException("Unable to read required 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++;
}
}

expect($rawCallsOutsideComments)->toBe(0,
"File {$relativeFile} contains raw (unprepared) DB calls"
);
}
});
});
36 changes: 36 additions & 0 deletions tests/Security/SetupStructureTest.php
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('maint setup.php structure', function () {
$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));

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

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

it('defines plugin_maint_uninstall function', function () use ($source) {
expect($source)->toContain('function plugin_maint_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*=>/');
});
Comment on lines +15 to +35
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.

file_get_contents(realpath(...)) can return false (missing file/realpath failure). Add explicit expectations that the resolved path is not false and that file_get_contents() succeeded, so the test fails with a clear message instead of passing a non-string into Pest matchers.

Suggested change
$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));
it('defines plugin_maint_install function', function () use ($source) {
expect($source)->toContain('function plugin_maint_install');
});
it('defines plugin_maint_version function', function () use ($source) {
expect($source)->toContain('function plugin_maint_version');
});
it('defines plugin_maint_uninstall function', function () use ($source) {
expect($source)->toContain('function plugin_maint_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*=>/');
});
beforeEach(function () {
$this->setupPath = realpath(__DIR__ . '/../../setup.php');
expect($this->setupPath)->not->toBeFalse();
$this->source = file_get_contents($this->setupPath);
expect($this->source)->not->toBeFalse();
});
it('defines plugin_maint_install function', function () {
expect($this->source)->toContain('function plugin_maint_install');
});
it('defines plugin_maint_version function', function () {
expect($this->source)->toContain('function plugin_maint_version');
});
it('defines plugin_maint_uninstall function', function () {
expect($this->source)->toContain('function plugin_maint_uninstall');
});
it('returns version array with name key', function () {
expect($this->source)->toMatch('/[\'\""]name[\'\""]\s*=>/');
});
it('returns version array with version key', function () {
expect($this->source)->toMatch('/[\'\""]version[\'\""]\s*=>/');
});

Copilot uses AI. Check for mistakes.
Comment on lines +29 to +35
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 checks for 'name' => / 'version' => in setup.php don't match the current implementation: plugin_maint_version() reads metadata from the INFO file (so setup.php doesn’t contain a 'version' => literal, and this test fails). Consider parsing INFO in the test (or include setup.php with a stubbed $config and assert the returned array has name/version).

Copilot uses AI. Check for mistakes.
});
Loading
Loading