refactor: safe PHP 7.4 modernization by somethingwithproof · Pull Request #69 · Cacti/plugin_webseer

somethingwithproof · 2026-04-09T21:16:39Z

This PR adds strict typing, short array syntax, and null coalescing operators across the plugin. Standalone infrastructure files were removed per architectural mandate.

Copilot

Pull request overview

This PR modernizes the WebSeer plugin’s PHP code style (strict typing + short arrays) and adds initial security hardening scaffolding (audit docs, URL validator, tests, and CI config).

Changes:

Add declare(strict_types=1); across plugin entrypoints and supporting files, and refactor many array() usages to [] (including prepared-statement parameter arrays).
Introduce security documentation and a new UrlValidator seam plus a new security-focused test file.
Add CI/static-analysis config (GitHub Actions, PHPUnit config, PHPStan config, Infection config).

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
webseer.php	Adds strict types; converts prepared-statement params and arrays to short syntax
webseer_servers.php	Adds strict types; converts prepared-statement params and arrays to short syntax
webseer_proxies.php	Adds strict types; converts arrays to short syntax
webseer_process.php	Adds strict types; converts prepared-statement params/arrays to short syntax
setup.php	Adds strict types; converts arrays/prepared-statement params to short syntax
poller_webseer.php	Adds strict types; converts prepared-statement params/arrays to short syntax
remote.php	Adds strict types; converts some arrays to short syntax; touches remote actions
includes/functions.php	Adds strict types; converts arrays to short syntax; touches remote-sync helpers
includes/arrays.php	Adds strict types; converts config/field definition arrays to short syntax
includes/constants.php	Adds strict types
includes/index.php	Adds strict types
index.php	Adds strict types
classes/cURL.php	Adds strict types; refactors some arrays and signature defaults
classes/mxlookup.php	Adds strict types; converts arrays to short syntax in class internals
classes/index.php	Adds strict types
images/index.php	Adds strict types
locales/index.php	Adds strict types
locales/po/index.php	Adds strict types
locales/LC_MESSAGES/index.php	Adds strict types
src/Security/UrlValidator.php	Adds SSRF-oriented URL validation helper + shim function
tests/Security/SsrfTest.php	Adds security regression/spec tests (Pest-style)
phpunit.xml	Adds PHPUnit configuration
phpstan.neon	Adds PHPStan configuration
infection.json	Adds Infection mutation-testing configuration
.github/workflows/ci.yml	Adds GitHub Actions workflow to run tests + PHPStan
SECURITY.md	Adds vulnerability disclosure policy
SECURITY-AUDIT.md	Adds detailed security audit findings and remediation notes
BACKLOG.md	Adds security hardening backlog and acceptance criteria
.omc/state/subagent-tracking.json	Adds tool state artifact (appears non-source)
.omc/state/checkpoints/checkpoint-2026-03-10T08-14-41-766Z.json	Adds tool checkpoint artifact (appears non-source)

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

Copilot · 2026-04-09T21:20:56Z


 		$this->debug('cURL options: ' . clean_up_lines(var_export($options, true)));
-		curl_setopt_array($process, $options);
+		curl_setopt_[$process, $options];


curl_setopt_[$process, $options]; is invalid PHP syntax and will cause a parse error at runtime. This should be curl_setopt_array($process, $options) (or equivalent per-option curl_setopt calls).

Suggested change

curl_setopt_[$process, $options];

curl_setopt_array($process, $options);

Copilot · 2026-04-09T21:20:56Z


 		$this->debug('cURL options: ' . clean_up_lines(var_export($options, true)));
-		curl_setopt_array($process,$options);
+		curl_setopt_[$process,$options];


curl_setopt_[$process,$options]; is invalid PHP syntax and will prevent this file from loading. Replace with curl_setopt_array($process, $options) (note the missing underscore and the bracket typo).

Suggested change

curl_setopt_[$process,$options];

curl_setopt_array($process, $options);

Copilot · 2026-04-09T21:20:56Z

 	foreach ($contacts as $c) {
-		if (!in_array($c['user_id'], $u)) {
-			db_execute_prepared('DELETE FROM plugin_webseer_contacts WHERE user_id = ?', array($c['user_id']));
+		if (!in_[$c['user_id'], $u]) {


if (!in_[$c['user_id'], $u]) is not valid PHP and changes behavior from in_array(...) to a parse error. It should remain an in_array($c['user_id'], $u, true/false) call (choose strictness intentionally).

Suggested change

if (!in_[$c['user_id'], $u]) {

if (!in_array($c['user_id'], $u, false)) {

Copilot · 2026-04-09T21:20:57Z

+					$ip = str_replace(["'", '\\'], '', $_POST['ip']);
 					$row = db_fetch_row("SELECT * FROM plugin_webseer_servers WHERE ip = '$ip'");


$ip is still interpolated directly into SQL in db_fetch_row("SELECT * FROM plugin_webseer_servers WHERE ip = '$ip'"). The preceding str_replace is not sufficient sanitization. Use db_fetch_row_prepared('SELECT * FROM plugin_webseer_servers WHERE ip = ?', [$ip]) and drop the manual stripping.

Suggested change

$ip = str_replace(["'", '\\'], '', $_POST['ip']);

$row = db_fetch_row("SELECT * FROM plugin_webseer_servers WHERE ip = '$ip'");

$ip = $_POST['ip'];

$row = db_fetch_row_prepared('SELECT * FROM plugin_webseer_servers WHERE ip = ?', [$ip]);

Copilot · 2026-04-09T21:20:57Z

+        expect(in_[$scheme, $allowedSchemes, true])->toBeFalse();
+    })->todo('Extract scheme allowlist to src/Security/UrlValidator.php first');
+
+    it('rejects gopher:// scheme', function (): void {
+        $url = 'gopher://127.0.0.1:6379/_FLUSHALL';
+        $scheme = parse_url($url, PHP_URL_SCHEME);
+        $allowedSchemes = ['http', 'https'];
+        expect(in_[$scheme, $allowedSchemes, true])->toBeFalse();


expect(in_[$scheme, $allowedSchemes, true]) is not valid PHP syntax and will fail test parsing. This should use in_array($scheme, $allowedSchemes, true) (and then negate/assert accordingly).

Suggested change

expect(in_[$scheme, $allowedSchemes, true])->toBeFalse();

})->todo('Extract scheme allowlist to src/Security/UrlValidator.php first');

it('rejects gopher:// scheme', function (): void {

$url = 'gopher://127.0.0.1:6379/_FLUSHALL';

$scheme = parse_url($url, PHP_URL_SCHEME);

$allowedSchemes = ['http', 'https'];

expect(in_[$scheme, $allowedSchemes, true])->toBeFalse();

expect(in_array($scheme, $allowedSchemes, true))->toBeFalse();

})->todo('Extract scheme allowlist to src/Security/UrlValidator.php first');

it('rejects gopher:// scheme', function (): void {

$url = 'gopher://127.0.0.1:6379/_FLUSHALL';

$scheme = parse_url($url, PHP_URL_SCHEME);

$allowedSchemes = ['http', 'https'];

expect(in_array($scheme, $allowedSchemes, true))->toBeFalse();

Copilot · 2026-04-09T21:20:58Z

+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd"
+         bootstrap="tests/Helpers/GlobalStubs.php"


bootstrap="tests/Helpers/GlobalStubs.php" points to a file that does not exist in this PR (there is no tests/Helpers/ directory). PHPUnit/Pest will fail to start unless this file is added or the bootstrap path is updated to an existing bootstrap file.

Suggested change

bootstrap="tests/Helpers/GlobalStubs.php"

Copilot · 2026-04-09T21:20:58Z

+      - name: Install dependencies
+        run: composer install --prefer-dist --no-progress --no-interaction
+
+      - name: Run tests with coverage
+        run: vendor/bin/pest --coverage --min=80
+
+      - name: Run PHPStan
+        run: vendor/bin/phpstan analyse --no-progress


This workflow runs composer install and then executes vendor/bin/pest and vendor/bin/phpstan, but the repository (as currently structured in this PR) does not include a composer.json/lockfile or committed vendor/. CI will fail on a clean checkout unless Composer metadata and dependencies are added (or these steps are replaced with the project’s actual install/test commands).

Copilot · 2026-04-09T21:20:59Z

+    level: 6
+    paths:
+        - src
+        - tests
+    phpVersion: 80400
+    checkGenericClassInNonGenericObjectType: true


phpVersion: 80400 configures PHPStan for PHP 8.4, but this PR is described as a “PHP 7.4 modernization”. Please align the configured PHP version (and the CI PHP version) with the plugin’s supported/runtime PHP version, or update the PR description/support policy to reflect an 8.4 baseline.

Copilot · 2026-04-09T21:20:59Z

+  "agents": [
+    {
+      "agent_id": "a699400d97bee152d",
+      "agent_type": "Explore",
+      "started_at": "2026-03-10T08:10:14.616Z",
+      "parent_mode": "none",
+      "status": "failed",
+      "completed_at": "2026-03-10T08:11:31.007Z",
+      "duration_ms": 76391
+    },
+    {
+      "agent_id": "a308486ee44f9c951",
+      "agent_type": "Explore",
+      "started_at": "2026-03-10T08:12:45.759Z",
+      "parent_mode": "none",
+      "status": "failed",
+      "completed_at": "2026-03-10T08:14:41.642Z",
+      "duration_ms": 115883
+    }
+  ],
+  "total_spawned": 2,
+  "total_completed": 0,
+  "total_failed": 2,
+  "last_updated": "2026-03-10T08:16:10.392Z"


These .omc/state/* files look like local tool/agent state (including failed run metadata) and are unlikely to be intended as source-controlled artifacts. Consider removing them from the repo and adding .omc/ (or at least .omc/state/) to .gitignore to avoid churn and leaking local environment details.

Suggested change

"agents": [

{

"agent_id": "a699400d97bee152d",

"agent_type": "Explore",

"started_at": "2026-03-10T08:10:14.616Z",

"parent_mode": "none",

"status": "failed",

"completed_at": "2026-03-10T08:11:31.007Z",

"duration_ms": 76391

},

{

"agent_id": "a308486ee44f9c951",

"agent_type": "Explore",

"started_at": "2026-03-10T08:12:45.759Z",

"parent_mode": "none",

"status": "failed",

"completed_at": "2026-03-10T08:14:41.642Z",

"duration_ms": 115883

}

],

"total_spawned": 2,

"total_completed": 0,

"total_failed": 2,

"last_updated": "2026-03-10T08:16:10.392Z"

"agents": [],

"total_spawned": 0,

"total_completed": 0,

"total_failed": 0,

"last_updated": null

Copilot · 2026-04-09T21:20:59Z

+{
+  "created_at": "2026-03-10T08:14:41.765Z",
+  "trigger": "auto",
+  "active_modes": {},
+  "todo_summary": {
+    "pending": 0,
+    "in_progress": 0,
+    "completed": 0
+  },
+  "wisdom_exported": false
+}


This checkpoint file appears to be generated tool state rather than project source/configuration. Keeping it under version control will create noisy diffs and doesn’t seem actionable for runtime behavior; consider deleting it and ignoring .omc/state/checkpoints/ in git.

Suggested change

{

"created_at": "2026-03-10T08:14:41.765Z",

"trigger": "auto",

"active_modes": {},

"todo_summary": {

"pending": 0,

"in_progress": 0,

"completed": 0

},

"wisdom_exported": false

}

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

- Use inet_pton() to normalize IPv6 before checking loopback/link-local/ULA - Catch expanded notation like 0:0:0:0:0:0:0:1 as loopback - Detect IPv4-mapped IPv6 (::ffff:a.b.c.d) and check the inner v4 address - Resolve both A and AAAA records via dns_get_record so IPv6-only hosts aren't blocked as 'unresolvable' - Split IPv4/IPv6 internal checks into dedicated helpers Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

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

somethingwithproof · 2026-04-11T00:10:57Z

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

Copilot AI review requested due to automatic review settings April 9, 2026 21:16

Copilot started reviewing on behalf of somethingwithproof April 9, 2026 21:17 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

somethingwithproof added 3 commits April 9, 2026 15:23

refactor: add strict typing and clean up standalone infra

3cf2bb2

refactor: safe PHP 7.4 modernization (arrays, null coalescing)

4adb718

fix: restore corrupted in_array calls

d45bbaa

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

somethingwithproof force-pushed the refactor/modernization branch from a6dd110 to d45bbaa Compare April 10, 2026 05:59

somethingwithproof added 4 commits April 10, 2026 02:01

fix: restore corrupted curl_setopt_array and is_array calls

5c1a83a

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

fix(security): use prepared statement for SETMASTER ip lookup

f04b289

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

style: remove trailing double semicolons

a0a3cd6

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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: safe PHP 7.4 modernization#69

refactor: safe PHP 7.4 modernization#69
somethingwithproof wants to merge 7 commits intoCacti:developfrom
somethingwithproof:refactor/modernization

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!

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

	curl_setopt_[$process, $options];
	curl_setopt_array($process, $options);

	curl_setopt_[$process,$options];
	curl_setopt_array($process, $options);

	if (!in_[$c['user_id'], $u]) {
	if (!in_array($c['user_id'], $u, false)) {

		$ip = str_replace(["'", '\\'], '', $_POST['ip']);
		$row = db_fetch_row("SELECT * FROM plugin_webseer_servers WHERE ip = '$ip'");

Conversation

somethingwithproof commented Apr 9, 2026

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!

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