feat(security): add cacti_* validation helper module (lib/security_validation.php)

[somethingwithproof]

Adds a side-effect-free validation helper module with six pure functions callers can use instead of open-coding their own regex allowlists.

Exported

• cacti_input_string_is_safe(string): bool — data-input command-string safety. Strips <tag> placeholders, then blocks ;, `, $, and newlines.
• cacti_validate_sort_column(string, string = ''): string — sort-column allowlist for ORDER BY. Blocks ;, --, /*, UNION/DROP/DELETE/INSERT/UPDATE; requires [a-zA-Z_``] prefix; allows dotted columns, backticks, parentheses, spaces for FIELD(status,1,3,2) style.
• cacti_validate_sort_direction(string, string = 'ASC'): string — ASC|DESC allowlist.
• cacti_extract_positive_int_ids(array): array — filter-casts an array to strictly positive ints, drops everything else.
• cacti_remote_agent_forward_matches(string, string, callable): bool — forward-DNS match helper for poller ingress.
• cacti_remote_agent_is_authorized_host(string, string, array, array): bool — ingress authorization combining the above with a poller list + whitelist.

Tests

tests/Unit/SecurityValidationHelpersTest.php (80 lines, Pest) covers each helper's positive case, negative case, and at least one edge case.

Scope

• No existing function signatures change.
• No existing caller is migrated in this PR — that lands page-by-page.
• Zero overlap with fix(import): consolidate path hardening for import and package writes #6989 or fix(xml): disable external entity loader in xml2array paths #7067.

Re-file from closed batch PR #7036.

[Copilot]

Pull request overview

This PR introduces a new reusable security validation helper module for Cacti (PHP) with accompanying unit tests, and also includes several unrelated UX/JS event-binding refactors plus some IPv6-related networking adjustments.

Changes:

• Add lib/security_validation.php with pure helper functions for input-string safety checks, sort validation, id extraction, and remote-agent host checks.
• Add Pest unit tests covering the new validation helpers.
• Refactor various JavaScript event bindings (unbind/click/hover → off/on) and make minor wording/typo fixes; adjust RRDtool proxy socket initialization for IPv6.

Reviewed changes

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

Show a summary per file

File	Description
lib/security_validation.php	Adds new security/validation helper functions intended for reuse by callers.
tests/Unit/SecurityValidationHelpersTest.php	Adds Pest unit tests for the new helper module.
lib/rrd.php	Updates RRDtool proxy socket creation/connection flow to better support IPv6.
lib/ping.php	Switches IPv6 detection to filter_var(..., FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) in multiple ping paths.
include/realtime.js	Refactors string building to template literals and tightens comparisons / event triggering in realtime graph code.
include/layout.js	Refactors several event bindings to on(...)/off(...) and fixes “deprecated” spelling in console logs.
install/install.js	Refactors event bindings and string building to template literals; minor null/undefined handling tweaks.
include/themes/sunrise/main.js	Replaces unbind()/hover()/click() patterns with off()/on() equivalents.
include/themes/raspberry/main.js	Replaces unbind()/hover()/click() patterns with off()/on() equivalents.
include/themes/paw/main.js	Replaces unbind()/hover()/click() patterns with off()/on() equivalents.
include/themes/paper-plane/main.js	Replaces unbind()/hover()/click() patterns with off()/on() equivalents.
include/themes/modern/main.js	Replaces change() handlers with on('change', ...) equivalents.
include/themes/midwinter/main.js	Replaces click()/hover() bindings with on(...) equivalents; minor trigger adjustments.
include/themes/hollyberry/main.js	Replaces unbind()/hover()/click() patterns with off()/on() equivalents.
include/themes/dark/main.js	Replaces change()/hover() bindings with on(...) equivalents.
include/themes/carrot/main.js	Replaces unbind()/hover()/click() patterns with off()/on() equivalents.
include/themes/cacti/main.js	Replaces unbind()/hover()/click() patterns with off()/on() equivalents.
package_repos.php	Fixes UI spelling (“Repositories”).
data_templates.php	Improves wording/grammar in a UI message string.
automation_templates.php	Fixes grammar in template description strings (“device’s …”).
automation_networks.php	Fixes grammar/possessives in notification description strings (“sender’s …”).
.github/workflows/codeql.yml	Adds a CodeQL workflow (JS/TS + Python matrix).
.github/dependabot.yml	Adds Dependabot configuration for npm and GitHub Actions updates.

Comments suppressed due to low confidence (1)

include/realtime.js:184

• In stopRealtime(), the selector passed to zoom() is missing the # prefix (currently graph_${graph_id}). This will not match the intended id="graph_<id>" element and will prevent zoom from being re-initialized when realtime stops. Use an id selector consistent with the rest of the file (e.g., select #graph_${graph_id}).

		$(`graph_${graph_id}`).zoom({
			inputfieldStartTime : 'date1',
			inputfieldEndTime : 'date2',
			serverTimeOffset : timeOffset
		});

[Copilot]

cacti_extract_positive_int_ids() is documented as returning “strictly positive integers”, but using intval will accept strings like "9 OR 1=1" as 9 (and "4.2" as 4). If this is meant for sanitizing request id lists, it should reject non-integer tokens instead of truncating them. Consider filtering by a full-match integer regex (optionally allowing surrounding whitespace / leading +) before casting.

Suggested change

	return array_values(array_filter(array_map('intval', $items), function ($v) {
	return $v > 0;
	}));
	$ids = [];
	foreach ($items as $item) {
	if (is_int($item)) {
	$value = $item;
	} elseif (is_string($item)) {
	$item = trim($item);
	if (!preg_match('/^\+?[0-9]+$/', $item)) {
	continue;
	}
	$value = (int) $item;
	} else {
	continue;
	}
	if ($value > 0) {
	$ids[] = $value;
	}
	}
	return array_values($ids);

[Copilot]

The $dns_lookup test double is declared with no parameters, but cacti_remote_agent_forward_matches() calls it with $client_name. In newer PHP versions this can emit warnings (or errors depending on configuration). Update the closures to accept the host parameter to match the documented callable signature.

[Copilot]

Same as above: this $dns_lookup closure should accept the host parameter since cacti_remote_agent_forward_matches() passes one argument.

[Copilot]

This test asserts that cacti_extract_positive_int_ids() keeps values like "9 OR 1=1" by truncating them to 9. That contradicts the function’s “strictly positive integers” contract and encourages unsafe usage. If the helper is meant for sanitizing request ids, update the expected output to drop non-integer tokens and adjust the helper accordingly.

Suggested change

	expect($ids)->toBe([1, 4, 7, 9, 7, 8]);
	expect($ids)->toBe([1, 7, 8]);

[Copilot]

In the load-balancing fallback, $rrdp_id = ($rrdp_id + 1) % 2; produces 0 or 1, while elsewhere $rrdp_id is treated like 1/2 (e.g., for logging and selecting primary vs backup config). Use a toggle that preserves the 1/2 convention (e.g., 3 - $rrdp_id) to avoid confusing log output and potential future logic errors.

Suggested change

	$rrdp_id = ($rrdp_id + 1) % 2;
	$rrdp_id = 3 - $rrdp_id;

[Copilot]

PR description focuses on adding lib/security_validation.php + tests and says no callers are migrated, but this PR also includes functional changes in unrelated areas (e.g., RRDtool proxy socket handling/IPv6 logic here, ping IPv6 detection, and multiple UI JS event handler refactors). If these are intentional, please reflect them in the PR description or split them into separate PRs to keep the security-helper change easy to review and backport.

[Copilot]

cacti_validate_sort_column() is still permissive enough to allow arbitrary SQL expressions/functions (e.g., SLEEP(5), RAND()) because parentheses, commas, spaces, and * are allowed and only a few keywords are blocked. If callers use this to validate user-provided ORDER BY, it can still enable time-based attacks or expensive sorts. Consider restricting to identifiers / dotted identifiers (and, if needed, explicitly allow only specific safe functions like FIELD(...) via dedicated patterns) rather than a broad character allowlist.

Suggested change

	* Allows dotted columns (h.hostname), backtick-quoted identifiers,
	* aggregate functions (COUNT(*), FIELD(status,1,3,2)), and spaces
	* inside function calls. Blocks semicolons, comments, and UNION.
	*/
	function cacti_validate_sort_column(string $value, string $default = ''): string {
	if (preg_match('/;|--|\/\*|\bUNION\b|\bDROP\b|\bDELETE\b|\bINSERT\b|\bUPDATE\b/i', $value)) {
	return $default;
	}
	return preg_match('/^[a-zA-Z_`][a-zA-Z0-9_`()., *]*$/', $value) ? $value : $default;
	* Allows bare identifiers, dotted identifiers (h.hostname), and
	* backtick-quoted identifiers. Does not allow SQL expressions or
	* function calls.
	*/
	function cacti_validate_sort_column(string $value, string $default = ''): string {
	$value = trim($value);
	if ($value === '') {
	return $default;
	}
	return preg_match('/^(?:`[a-zA-Z_][a-zA-Z0-9_]*`|[a-zA-Z_][a-zA-Z0-9_]*)(?:\.(?:`[a-zA-Z_][a-zA-Z0-9_]*`|[a-zA-Z_][a-zA-Z0-9_]*))*$/', $value) ? $value : $default;

[Copilot]

PR description says cacti_remote_agent_is_authorized_host() “combines the above” (including forward-DNS matching), but the current implementation only checks the poller hostname / client IP against lists and never calls cacti_remote_agent_forward_matches(). Either update the implementation to incorporate forward-lookup matching (likely via an injected $dns_lookup callable for testability) or adjust the PR description/API to reflect the actual behavior.

[somethingwithproof]

Closing. Copilot flagged two substantive bugs plus the fork-stale-files issue:

Function-contract bugs

• cacti_extract_positive_int_ids() uses intval(), which accepts "9 OR 1=1" as 9 and "4.2" as 4. That contradicts the docblock's "strictly positive integers" contract and would invite SQL-injection-style inputs through. Needs filter_var(..., FILTER_VALIDATE_INT) with explicit rejection of non-numeric strings, and the paired test needs to assert rejection, not acceptance.
• cacti_validate_sort_column() allows (, ), ,, spaces, *. With those plus an [a-zA-Z_] prefix, SLEEP(5) and RAND() pass. The allowlist needs tightening: either reject parentheses/commas entirely (plain column references + . for table qualifier), or have an explicit short list of permitted aggregate functions.
• cacti_remote_agent_is_authorized_host() docblock advertises forward-DNS matching but the implementation only does a poller-table compare; the forward-match helper is defined but not wired through.
• Test closures for $dns_lookup are declared with no parameters but the production code calls them with $host, which errors on PHP 8.

Fork-stale contamination

My fork's develop was 10 commits behind Cacti/cacti:develop when I branched, which bloated the diff to 23 files (dependabot, codeql, themes/*, rrd.php, ping.php, JS refactors). That's my pilot error, now corrected by registering upstream and branching off upstream/develop.

No salvageable path that isn't "rewrite the helpers with stricter contracts and re-file against upstream." Closing.