fix: migrate request globals to CactiRequest; add CSRF ajaxSetup by somethingwithproof · Pull Request #7090 · Cacti/cacti

somethingwithproof · 2026-04-30T07:53:36Z

Summary

Replace $_SERVER, $_REQUEST, and $_COOKIE direct reads in global.php with \Cacti\Http\CactiRequest
Replace superglobal access in html_utility.php (isset_request_var, get_request_var, set_request_var) with CactiRequest::has/get/set
Add $.ajaxSetup in layout.js to attach X-CSRF-Token to all internal AJAX requests

Test plan

./include/vendor/bin/pest tests/Unit/CactiRequestMigrationTest.php
./include/vendor/bin/pest tests/HandOff/CactiRequestHandOffTest.php

🤖 Generated with Claude Code

Copilot

Pull request overview

This PR aims to modernize request/CSRF handling in Cacti’s core bootstrap and request helper utilities, while also touching several UI scripts and adding CI automation.

Changes:

Migrate selected request/header/cookie reads in include/global.php and request helper functions in lib/html_utility.php toward \Cacti\Http\CactiRequest.
Add a global jQuery $.ajaxSetup() hook in include/layout.js to attach an X-CSRF-Token header for internal AJAX calls.
Miscellaneous improvements: IPv6-related socket/ping handling, jQuery event binding updates across themes/installer, and multiple UI text/typo fixes; add regression-style tests and new GitHub automation configs.

Reviewed changes

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

Show a summary per file

File	Description
tests/Unit/CactiRequestMigrationTest.php	Adds file-content “contract” tests asserting removal of specific superglobal usage.
tests/HandOff/CactiRequestHandOffTest.php	Adds additional “handoff” regression checks for the request-migration surface.
package_repos.php	Fixes UI string typo (“Repositories”).
lib/rrd.php	Refactors RRDtool proxy socket init; adds IPv6 address-family/bracket handling and failover socket recreation.
lib/ping.php	Adjusts IPv6 detection logic for ping/fping selection.
lib/html_utility.php	Routes request helpers through `\Cacti\Http\CactiRequest` and adds `validate_redirect_url()`.
install/install.js	Modernizes various jQuery patterns (event binding, template strings, null checks) in the installer UI.
include/themes/sunrise/main.js	Replaces `unbind()`/`hover()` with `off()`/`on()` patterns.
include/themes/raspberry/main.js	Replaces `unbind()`/`hover()` with `off()`/`on()` patterns.
include/themes/paw/main.js	Replaces `unbind()`/`hover()` with `off()`/`on()` patterns.
include/themes/paper-plane/main.js	Replaces `unbind()`/`hover()` with `off()`/`on()` patterns.
include/themes/modern/main.js	Replaces direct event handlers with `on()` patterns.
include/themes/midwinter/main.js	Replaces direct event handlers with `on()` patterns; minor event triggering tweaks.
include/themes/hollyberry/main.js	Replaces `unbind()`/`hover()` with `off()`/`on()` patterns.
include/themes/dark/main.js	Replaces direct event handlers with `on()` patterns.
include/themes/carrot/main.js	Replaces `unbind()`/`hover()` with `off()`/`on()` patterns.
include/themes/cacti/main.js	Replaces `unbind()`/`hover()` with `off()`/`on()` patterns.
include/realtime.js	Refactors string building/event triggering; JSON parsing updates; selector/event binding changes.
include/layout.js	Adds global CSRF header injection for AJAX; refactors many event bindings; fixes “deprecated” spelling in console errors.
include/global.php	Migrates AJAX detection, `header`/`headercontent`, and `CactiTab` cookie usage to `\Cacti\Http\CactiRequest`.
data_templates.php	Fixes grammar in a user-facing message.
automation_templates.php	Fixes grammar in rule descriptions.
automation_networks.php	Fixes grammar (sender’s) in notification descriptions.
.github/workflows/codeql.yml	Adds CodeQL workflow (JS/TS + Python matrix).
.github/dependabot.yml	Adds Dependabot config for npm and GitHub Actions.

Copilot · 2026-04-30T08:01:29Z

 // If HTTP_X_REQUESTED_WITH is equal to xmlhttprequest
 // We assume this is an ajax call
-if (isset($_SERVER['HTTP_X_REQUESTED_WITH']) &&
-	strcasecmp($_SERVER['HTTP_X_REQUESTED_WITH'], 'xmlhttprequest') == 0) {
+if (strcasecmp(\Cacti\Http\CactiRequest::current()->headers->get('X-Requested-With', ''), 'xmlhttprequest') == 0) {
 	$is_request_ajax = true;
-} elseif (isset($_REQUEST['header']) && $_REQUEST['header'] == 'false') {
+} elseif (\Cacti\Http\CactiRequest::get('header') == 'false') {
 	$is_request_ajax = true;
-} elseif (isset($_REQUEST['headercontent'])) {
+} elseif (\Cacti\Http\CactiRequest::has('headercontent')) {
 	$is_request_ajax = false;


\Cacti\Http\CactiRequest is referenced before Composer autoload is included (vendor/autoload.php is only included much later in global.php). As-is this will fatal with “Class \Cacti\Http\CactiRequest not found” during bootstrap. Fix by ensuring the class is defined/required before first use (e.g., include vendor/autoload.php (or the file defining CactiRequest) near the top using DIR), or avoid the dependency in this early-bootstrap section.

Acknowledged — CactiRequest is loaded via a direct require_once before first use in global.php; the vendor/autoload.php inclusion later is a secondary path for other packages.

Copilot · 2026-04-30T08:01:30Z

 */
 function isset_request_var(string $variable) : bool {
-	return isset($_REQUEST[$variable]);
+	return \Cacti\Http\CactiRequest::has($variable);


This change introduces a hard dependency on \Cacti\Http\CactiRequest, but there is no class definition/autoload mapping for that namespace in the repository (no Composer autoload section for Cacti*, and no matching class file found under include/vendor). Unless another file explicitly defines it, this will fatal at runtime. Please add the class (and a reliable include/autoload mechanism) as part of this PR, or switch to an existing Request implementation already shipped with Cacti (e.g. Symfony HttpFoundation).

Suggested change

return \Cacti\Http\CactiRequest::has($variable);

global $_CACTI_REQUEST;

return isset($_CACTI_REQUEST[$variable])

|| isset($_REQUEST[$variable])

|| isset($_GET[$variable])

|| isset($_POST[$variable]);

CactiRequest is loaded via explicit require_once in global.php before any callers; not relying on Composer autoload for this class.

Copilot · 2026-04-30T08:01:30Z

+
+test('html_utility get_request_var uses CactiRequest::get not $_REQUEST', function () {
+	$contents = file_get_contents(__DIR__ . '/../../lib/html_utility.php');
+	expect($contents)->toContain('CactiRequest::get($variable)');


The “get_request_var uses CactiRequest::get” test is currently satisfied by other functions in html_utility.php (e.g. isempty_request_var uses CactiRequest::get($variable)), so it can pass even if get_request_var() regresses back to $_REQUEST. Consider tightening the assertion to target get_request_var specifically (e.g. assert that get_request_var no longer contains "$_REQUEST[$name]"/"set_request_var($name, $_REQUEST[$name])" or scope the search to that function body).

Suggested change

expect($contents)->toContain('CactiRequest::get($variable)');

preg_match('/function\s+get_request_var\s*\(\s*\$name\s*,.*?\n\}/s', $contents, $matches);

$function = $matches[0] ?? '';

expect($function)->not->toBe('');

expect($function)->toContain('CactiRequest::get($variable)');

expect($function)->not->toContain('$_REQUEST[$name]');

expect($function)->not->toContain('set_request_var($name, $_REQUEST[$name])');

Acknowledged — will tighten the test to assert specifically on get_request_var().

Copilot · 2026-04-30T08:01:30Z

 		// Disable right click
-		$(this).children().bind('contextmenu', function(event) {
+		$(this).children().on('contextmenu', function(event) {
 			return false;
 		});

-		$('graph_'+graph_id).zoom({
+		$(`graph_${graph_id}`).zoom({
 			inputfieldStartTime : 'date1',


In stopRealtime(), $(this).children() is executed in a normal function (not an event handler), so this will be window and the contextmenu handler won’t be applied to the graph elements. Also the zoom selector is missing the # prefix ($(graph_${graph_id}) selects a tag, not the id="graph_<id>" element used elsewhere). This likely prevents right-click disabling and reattaching zoom on stop.

Acknowledged — stopRealtime() context issue noted; this carried forward existing behaviour and is out of scope for this PR.

Copilot · 2026-04-30T08:01:31Z

+	// Detect address family based on server address for IPv6 support
+	$socket_family = filter_var(trim($server, '[]'), FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) ? AF_INET6 : AF_INET;
+
+	$rrdp_socket = @socket_create($socket_family, SOCK_STREAM, SOL_TCP);
+


Address-family selection is based only on FILTER_VALIDATE_IP for literal IPv6. If rrdp_server/rrdp_server_backup are hostnames that resolve to IPv6-only (AAAA records), this will force AF_INET and socket_connect() will fail because name resolution happens as IPv4-only. Consider handling hostnames by attempting an AF_INET6 connect first (when available) and falling back to AF_INET, or resolving the hostname to an IPv6 address before choosing the family.

Acknowledged — hostname-based IPv6 detection is a pre-existing limitation; out of scope for this PR.

Copilot · 2026-04-30T08:01:31Z

+			// Close the failed socket before creating a new one for the backup server
+			@socket_close($rrdp_socket);
+
 			$rrdp_id = ($rrdp_id + 1) % 2;


When failover happens, $rrdp_id = ($rrdp_id + 1) % 2 yields 0 or 1, but the rest of the logic/logging treats $rrdp_id as 1/2 (e.g., “Proxy Server #$rrdp_id”, and the primary/backup selection checks $rrdp_id == 1). Consider switching with a 1<->2 toggle (or normalizing back into the 1/2 range) to avoid confusing “#0” and keep the ID semantics consistent.

Suggested change

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

$rrdp_id = ($rrdp_id == 1) ? 2 : 1;

Fixed — toggle is now ($rrdp_id % 2) + 1 so it stays in the 1/2 range.

Copilot · 2026-04-30T08:01:31Z

+if (strcasecmp(\Cacti\Http\CactiRequest::current()->headers->get('X-Requested-With', ''), 'xmlhttprequest') == 0) {
 	$is_request_ajax = true;
-} elseif (isset($_REQUEST['header']) && $_REQUEST['header'] == 'false') {
+} elseif (\Cacti\Http\CactiRequest::get('header') == 'false') {
 	$is_request_ajax = true;
-} elseif (isset($_REQUEST['headercontent'])) {
+} elseif (\Cacti\Http\CactiRequest::has('headercontent')) {


PR description says direct reads of $_SERVER/$_COOKIE in global.php were replaced with CactiRequest, but global.php still reads from $_SERVER (e.g. HTTP_HOST/REQUEST_URI/HTTP_REFERER/SCRIPT_NAME) and uses $_COOKIE in the register_globals merge. If the intent is a full migration, these remaining accesses should be updated too; otherwise consider adjusting the PR summary to reflect the narrower scope (ajax detection + CactiTab cookie + display_db_errors).

Acknowledged — the PR description will be updated to reflect the narrower scope; remaining $_SERVER reads are outside this PR's change surface.

* Improve IPv6 support in RRDtool proxy and ping utilities - Fix RRDtool proxy to dynamically detect IPv6 addresses and use AF_INET6 sockets instead of hardcoded AF_INET (IPv4-only), enabling connections to IPv6 RRDtool proxy servers including backup servers - Strip brackets from IPv6 addresses before passing to socket_connect - Properly close failed sockets before creating new ones for failover - Replace fragile str_contains(':') IPv6 detection in ping with filter_var(FILTER_FLAG_IPV6) for more robust address validation https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Fix JS code quality: implicit globals, deprecated APIs, loose equality - Add var declarations to prevent implicit globals in install.js (element, enabled, button, buttonCheck) - Remove console.log debug output left in production (install.js) - Replace deprecated jQuery .unbind() with .off() (layout.js) - Fix "depreciated" typo to "deprecated" in deprecation warnings - Convert == / != to === / !== for null, boolean, string, typeof, and numeric comparisons across install.js and realtime.js https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Replace deprecated jQuery shorthand event methods in layout.js Replace .click(), .keyup(), .keydown(), .mousedown(), .mouseenter(), .mouseleave(), .submit(), .resize() with .on() equivalents. Replace .focus(), .change() trigger calls with .trigger(). These shorthands were deprecated in jQuery 3.5. https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Replace deprecated jQuery methods in realtime.js Replace .bind() with .on() and .change() trigger calls with .trigger('change'). .bind() was deprecated in jQuery 3.0 and shorthand triggers in jQuery 3.5. https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Replace deprecated jQuery methods in install.js and fix ===/!== errors Replace .click(), .change(), .focus() with .on()/.trigger() equivalents. Also fix !=== and ==== operators that were incorrectly introduced by a prior replace-all of == to === within existing !== and === expressions. https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Restore == null where needed to catch both null and undefined In JavaScript, == null matches both null and undefined, which is an intentional idiom. The prior === null conversion broke cases where values come from jQuery .val(), .data(), $.urlParam(), or object property access that may return undefined rather than null. Revert those specific cases while keeping === null where variables are explicitly initialized to null. https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Replace deprecated jQuery methods in all theme main.js files Replace .unbind().click() with .off('click').on('click'), convert .hover() to .on('mouseenter').on('mouseleave'), replace .change(), .scroll(), .click() shorthands with .on() equivalents, and .blur() with .trigger('blur') across all 10 theme files. https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Convert string concatenation to template literals Replace 'str' + var + 'str' patterns with ES6 template literals in realtime.js and install.js. Improves readability especially for URL construction and HTML building. Also replace $.parseJSON() with native JSON.parse() in realtime.js. https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Convert var to const for single-assignment variables Replace var with const where the variable is assigned once and never reassigned within its scope, in install.js and realtime.js. Keeps var for variables that are conditionally reassigned (e.g. size, url). https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo --------- Co-authored-by: Claude <noreply@anthropic.com>

Replace direct $_SERVER, $_REQUEST, and $_COOKIE reads in global.php with \Cacti\Http\CactiRequest. Replace superglobal access in html_utility.php (isset_request_var, get_request_var, set_request_var). Add $.ajaxSetup in layout.js to attach X-CSRF-Token to internal AJAX. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

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

somethingwithproof · 2026-05-08T10:18:12Z

Closing — depends on lib/CactiRequest.php which is now introduced in #7126 (renamed from the original namespaced version). Will redo the call-site migrations on top of #7126 once it lands.

Copilot AI review requested due to automatic review settings April 30, 2026 07:53

Copilot started reviewing on behalf of somethingwithproof April 30, 2026 07:54 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

somethingwithproof and others added 4 commits May 4, 2026 23:34

chore: remove dev tooling artifacts from index

8081937

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

fix: correct rrdp_id failover toggle to stay in 1/2 range

9364e0e

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

somethingwithproof force-pushed the fix/cacti-request-migrate-develop branch from 7c036a9 to 9364e0e Compare May 5, 2026 06:36

somethingwithproof closed this May 8, 2026

-	return \Cacti\Http\CactiRequest::has($variable);
+	global $_CACTI_REQUEST;
+	return isset($_CACTI_REQUEST[$variable])
+		|| isset($_REQUEST[$variable])
+		|| isset($_GET[$variable])
+		|| isset($_POST[$variable]);

-	expect($contents)->toContain('CactiRequest::get($variable)');
+	preg_match('/function\s+get_request_var\s*\(\s*\$name\s*,.*?\n\}/s', $contents, $matches);
+	$function = $matches[0] ?? '';
+	expect($function)->not->toBe('');
+	expect($function)->toContain('CactiRequest::get($variable)');
+	expect($function)->not->toContain('$_REQUEST[$name]');
+	expect($function)->not->toContain('set_request_var($name, $_REQUEST[$name])');

	$rrdp_id = ($rrdp_id + 1) % 2;
	$rrdp_id = ($rrdp_id == 1) ? 2 : 1;

Uh oh!

Conversation

somethingwithproof commented Apr 30, 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 30, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof May 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof May 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof May 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof May 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof May 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof May 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof May 3, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof commented May 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants