Render RRDtool Tune Info as balanced div boxes and POST the Data Source repair actions (#8412)

* data_sources: render RRDtool Tune Info as balanced div boxes

rrdtool_tune() emitted an unbalanced mix of html_header() and
html_end_box() calls wrapped around raw table rows inside the single
box opened by data_sources.php.  The premature and extra closing tags
corrupted the DOM, which (depending on theme) clipped the Save button
at the bottom of the Data Source edit page while in Data Source Info
mode.

- Separate the execute, CLI and HTML code paths in rrdtool_tune() and,
  for HTML, render each section as a self contained, balanced div box.
- Extract and rename the helper print_leaves() to the standalone
  rrdtool_tune_print_leaves(), matching the rrdtool_tune_* naming
  convention, and emit div rows instead of table rows.
- Switch the data_sources.php wrapper box to div mode to match.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* data_debug: require POST for the Data Source repair actions

Applying RRDfile repairs and rerunning the debugger are state changing
operations, so they should not be reachable with a GET.  Driving them
through a GET also meant the follow up redirect was pulled into the
content area as a full page, losing the theme and navigation on
rerender.

Register data_debug.php run_repair and run_debug in the
cactiIsPostActionUrl() allowlist so loadUrl() upgrades them to a
token-bearing POST, and add the matching csrf_require_post() guard so
the browser transport and server side guard stay in sync.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* rrd: split rrdtool_tune into testable render and execute helpers

Break rrdtool_tune() into small single-purpose helpers so each path can
be exercised directly by unit tests (the web display path is otherwise
unreachable under CACTI_CLI):

- rrdtool_tune_print_leaves() takes an explicit $html flag instead of
  branching on CACTI_CLI, so both the div and plain-text branches are
  testable.
- rrdtool_tune_execute() applies the tune/resize commands for a repair.
- rrdtool_tune_print_cli() prints the source commands as plain text.
- rrdtool_tune_print_html() renders the balanced div boxes.
- rrdtool_tune() becomes a thin dispatcher over the three.

Add tests/Unit/RrdTuneRenderTest.php covering the leaf printer, the HTML
and CLI printers, the execute path and the dispatcher, asserting the
output is balanced div markup with no table rows.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* review: post Data Source Troubleshooter actions via cactiPostAction

Address PR review feedback:

- Switch the Apply Suggested Fixes and Rerun Analysis links to the
  existing cactiPostAction/data-url idiom (as data_sources.php already
  does for ds_repair) instead of upgrading a loadUrl() GET through the
  cactiIsPostActionUrl allowlist. The allowlist path replayed the
  query-free mutation URL into browser history, so a refresh reopened
  the list instead of the selected Data Source; a full cactiPostAction
  submission follows the server redirect to the read-only view URL and
  records that in history. The run_repair / run_debug csrf_require_post
  guards are retained, and the data_debug.php allowlist entry (and its
  now redundant click handlers) are removed.

- Add a callable $executor test seam to rrdtool_tune_execute() (matching
  rrdtool_dump_document) so the unit tests assert the tune and resize
  commands are actually executed, with resize.rrd produced only when the
  resize command runs, rather than passing on the rename alone.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* review: move RrdTuneRenderTest into the Unit/Core/Rrd category folder

Per AGENTS.md, new unit tests belong in a category subfolder rather than
directly under tests/Unit. Move RrdTuneRenderTest there and update the
UnitStubs include depth accordingly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: TheWitness <copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

data_debug.php

@@ -37,6 +37,10 @@ set_default_action();
 
 draw_data_debug_filter(false);
 
+if (in_array(grv('action'), ['run_repair', 'run_debug'], true)) {
+	csrf_require_post();
+}
+
 switch (grv('action')) {
 	case 'actions':
 		form_actions();
@@ -769,7 +773,7 @@ function debug_view() : void {
 	} elseif ($debug_status == 'analysis') {
 		html_start_box(__('Data Source Troubleshooter [ Auto Refreshing till RRDfile Update ] %s', '<i class="reloadquery ti ti-refresh icon-rotate" data-id="' . $id . '" title="' . __esc('Refresh Now') . '"></i>'), '100%', false, 3, 'center', '');
 	} else {
-		html_start_box(__('Data Source Troubleshooter [ Analysis Complete! %s ]', '<a href="#" class="rerun linkEditMain" data-id="' . $id . '" style="cursor:pointer;">' . __('Rerun Analysis') . '</a>'), '100%', false, 3, 'center', '');
+		html_start_box(__('Data Source Troubleshooter [ Analysis Complete! %s ]', '<a href="#" class="cactiPostAction linkEditMain" data-url="' . html_escape_url('data_debug.php?action=run_debug&id=' . $id) . '" style="cursor:pointer;">' . __('Rerun Analysis') . '</a>'), '100%', false, 3, 'center', '');
 	}
 
 	html_header(
@@ -859,7 +863,7 @@ function debug_view() : void {
 			$path = get_data_source_path($id, true);
 
 			if (is_writeable($path)) {
-				html_start_box(__('Repair Steps [ %s ]', '<a href="#" class="repairme linkEditMain" data-id="' . $id . '" style="cursor:pointer;">' . __('Apply Suggested Fixes') . '</a>'), '100%', false, 3, 'center', '');
+				html_start_box(__('Repair Steps [ %s ]', '<a href="#" class="cactiPostAction linkEditMain" data-url="' . html_escape_url('data_debug.php?action=run_repair&id=' . $id) . '" style="cursor:pointer;">' . __('Apply Suggested Fixes') . '</a>'), '100%', false, 3, 'center', '');
 			} else {
 				html_start_box(__('Repair Steps [ Run Fix from Command Line ]', $path), '100%', false, 3, 'center', '');
 			}
@@ -889,22 +893,10 @@ function debug_view() : void {
 	?>
 	<script type='text/javascript'>
 	$(function() {
-		$('.repairme').click(function(event) {
-			event.preventDefault();
-			id = $(this).attr('data-id');
-			loadUrl({url:'data_debug.php?action=run_repair&id=' + id})
-		});
-
 		$('.reloadquery').click(function() {
 			id = $(this).attr('data-id');
 			loadUrl({url:'data_debug.php?action=view&id=' + id})
 		});
-
-		$('.rerun').click(function(event) {
-			event.preventDefault();
-			id = $(this).attr('data-id');
-			loadUrl({url:'data_debug.php?action=run_debug&id=' + id})
-		});
 	});
 	</script>
 	<?php

data_sources.php

@@ -1495,16 +1495,16 @@ function ds_edit() : void {
 				// offer a one-click repair when the web server can write the
 				// RRDfile, otherwise present the commands to run by hand
 				if ($has_tune && is_writeable(get_data_source_path(grv('id'), true))) {
-					html_start_box(__('RRDtool Tune Info [ %s ]', '<a href="#" class="cactiPostAction linkEditMain" data-url="' . html_escape_url('data_sources.php?action=ds_repair&id=' . grv('id')) . '" style="cursor:pointer;">' . __('Apply Suggested Fixes') . '</a>'), '100%', false, 3, 'center', '');
+					html_start_box(__('RRDtool Tune Info [ %s ]', '<a href="#" class="cactiPostAction linkEditMain" data-url="' . html_escape_url('data_sources.php?action=ds_repair&id=' . grv('id')) . '" style="cursor:pointer;">' . __('Apply Suggested Fixes') . '</a>'), '100%', true, 3, 'center', '');
 				} elseif ($has_tune) {
-					html_start_box(__('RRDtool Tune Info [ %s ]', __('Run Fix from Command Line')), '100%', false, 3, 'center', '');
+					html_start_box(__('RRDtool Tune Info [ %s ]', __('Run Fix from Command Line')), '100%', true, 3, 'center', '');
 				} else {
-					html_start_box(__('RRDtool Tune Info'), '100%', false, 3, 'center', '');
+					html_start_box(__('RRDtool Tune Info'), '100%', true, 3, 'center', '');
 				}
 
 				rrdtool_tune($rrd_info['filename'], $diff, true);
 
-				html_end_box();
+				html_end_box(true, true);
 			}
 		}
 

lib/rrd.php

@@ -4802,102 +4802,144 @@ function rrdtool_info2html(array $info_array, array $diff = []) : void {
 }
 
 /**
- * print_leaves - A function used by the rrdtool_tune function to output data
+ * rrdtool_tune_print_leaves - a helper used by rrdtool_tune_print_cli() and
+ * rrdtool_tune_print_html() to recursively output the leaf nodes of an rrdtool
+ * 'diff' array
  *
  * @param array $array The array of leaves
+ * @param bool  $html  Emit div rows for the web UI, or plain text for the CLI
  *
  * @return void
  */
-function print_leaves(array $array) : void {
+function rrdtool_tune_print_leaves(array $array, bool $html = true) : void {
 	foreach ($array as $key => $line) {
 		if (!is_array($line)) {
-			if (CACTI_CLI) {
-				print $line . PHP_EOL;
+			if ($html) {
+				print "<div class='formRow'>" . $line . '</div>';
 			} else {
-				print '<tr><td>' . $line . '</td></tr>';
+				print $line . PHP_EOL;
 			}
 		} else {
-			if ($key === 'tune') {
+			if ($key === 'tune' || $key === 'resize') {
 				continue;
 			}
 
-			if ($key === 'resize') {
-				continue;
-			}
-			print_leaves($line);
+			rrdtool_tune_print_leaves($line, $html);
 		}
 	}
 }
 
+/**
+ * rrdtool_tune_execute - apply the rrdtool tune/resize commands for a repair
+ *
+ * @param string        $rrd_file The rrd file name
+ * @param array         $diff     The array of discrepancies between cacti settings and rrd file info
+ * @param callable|null $executor Test seam for RRDtool execution
+ *
+ * @return void
+ */
+function rrdtool_tune_execute(string $rrd_file, array $diff, ?callable $executor = null) : void {
+	$executor ??= 'rrdtool_execute';
+
+	if (isset($diff['tune']) && cacti_sizeof($diff['tune'])) {
+		foreach ($diff['tune'] as $line) {
+			$executor("tune $line", true, RRDTOOL_OUTPUT_STDOUT);
+		}
+	}
+
+	if (isset($diff['resize']) && cacti_sizeof($diff['resize'])) {
+		foreach ($diff['resize'] as $line) {
+			$executor("resize $line", true, RRDTOOL_OUTPUT_STDOUT);
+			rename(dirname($rrd_file) . '/resize.rrd', $rrd_file);
+		}
+	}
+}
+
+/**
+ * rrdtool_tune_print_cli - print the rrdtool tune/resize source commands as plain text
+ *
+ * @param string $rrd_file The rrd file name
+ * @param array  $diff     The array of discrepancies between cacti settings and rrd file info
+ *
+ * @return void
+ */
+function rrdtool_tune_print_cli(string $rrd_file, array $diff) : void {
+	$rrd_path = read_config_option('path_rrdtool');
+
+	if (cacti_sizeof($diff)) {
+		rrdtool_tune_print_leaves($diff, false);
+	}
+
+	if (isset($diff['tune']) && cacti_sizeof($diff['tune'])) {
+		foreach ($diff['tune'] as $line) {
+			print $rrd_path . ' tune ' . $line . PHP_EOL;
+		}
+	}
+
+	if (isset($diff['resize']) && cacti_sizeof($diff['resize'])) {
+		foreach ($diff['resize'] as $line) {
+			print $rrd_path . ' resize ' . $line . PHP_EOL;
+			print __('rename %s to %s', dirname($rrd_file) . '/resize.rrd', $rrd_file) . PHP_EOL;
+		}
+	}
+}
+
+/**
+ * rrdtool_tune_print_html - render the rrdtool tune/resize output as balanced div boxes
+ *
+ * @param string $rrd_file The rrd file name
+ * @param array  $diff     The array of discrepancies between cacti settings and rrd file info
+ *
+ * @return void
+ */
+function rrdtool_tune_print_html(string $rrd_file, array $diff) : void {
+	$rrd_path = read_config_option('path_rrdtool');
+
+	if (cacti_sizeof($diff)) {
+		html_start_box(__('Errors Found'), '100%', true, 3, 'center', '');
+
+		rrdtool_tune_print_leaves($diff, true);
+
+		html_end_box(true, true);
+	}
+
+	if ((isset($diff['tune']) && cacti_sizeof($diff['tune'])) ||
+		(isset($diff['resize']) && cacti_sizeof($diff['resize']))) {
+		html_start_box(__('Optional Tuning Recommendations'), '100%', true, 3, 'center', '');
+
+		if (isset($diff['tune'])) {
+			foreach ($diff['tune'] as $line) {
+				print "<div class='formRow'>" . $rrd_path . ' tune ' . $line . '</div>';
+			}
+		}
+
+		if (isset($diff['resize'])) {
+			foreach ($diff['resize'] as $line) {
+				print "<div class='formRow'>" . $rrd_path . ' resize ' . $line . '</div>';
+				print "<div class='formRow'>" . __('rename %s to %s', dirname($rrd_file) . '/resize.rrd', $rrd_file) . '</div>';
+			}
+		}
+
+		html_end_box(true, true);
+	}
+}
+
 /**
  * rrdtool_tune - create rrdtool tune/resize commands html+cli enabled
  *
  * @param string $rrd_file    The rrd file name
  * @param array  $diff        The array of discrepancies between cacti settings and rrd file info
- * @param bool   $show_source The only show text+commands or execute all commands, execute is for cli mode only!
+ * @param bool   $show_source Only show text+commands, or execute all commands; execute is for cli mode only!
  *
  * @return void
  */
 function rrdtool_tune(string $rrd_file, array $diff, bool $show_source = true) : void {
-	$rrd_path = read_config_option('path_rrdtool');
-
-	$cmd = [];
-
-	if ($show_source && cacti_sizeof($diff)) {
-		if (!CACTI_CLI) {
-			html_header([__('Errors Found')]);
-		}
-
-		print_leaves($diff);
-
-		if (!CACTI_CLI) {
-			html_end_box();
-		}
-	}
-
-	if (isset($diff['tune']) || isset($diff['resize'])) {
-		if (!CACTI_CLI) {
-			html_header([__('Optional Tuning Recommendations')]);
-		}
-	}
-
-	if (isset($diff['tune']) && cacti_sizeof($diff['tune'])) {
-		// create tune commands
-		foreach ($diff['tune'] as $line) {
-			if ($show_source == true) {
-				if (CACTI_CLI) {
-					print $rrd_path . ' tune ' . $line . PHP_EOL;
-				} else {
-					print '<tr><td>' . $rrd_path . ' tune ' . $line . '</td></tr>';
-				}
-			} else {
-				rrdtool_execute("tune $line", true, RRDTOOL_OUTPUT_STDOUT);
-			}
-		}
-	}
-
-	if (isset($diff['resize']) && cacti_sizeof($diff['resize'])) {
-		// each resize goes into an extra line
-		foreach ($diff['resize'] as $line) {
-			if ($show_source == true) {
-				if (CACTI_CLI) {
-					print read_config_option('path_rrdtool') . ' resize ' . $line . PHP_EOL;
-					print __('rename %s to %s', dirname($rrd_file) . '/resize.rrd', $rrd_file) . PHP_EOL;
-				} else {
-					print '<tr><td>' . read_config_option('path_rrdtool') . ' resize ' . $line . '</td></tr>';
-					print '<tr><td>' . __('rename %s to %s', dirname($rrd_file) . '/resize.rrd', $rrd_file) . '</td></tr>';
-				}
-			} else {
-				rrdtool_execute("resize $line", true, RRDTOOL_OUTPUT_STDOUT);
-				rename(dirname($rrd_file) . '/resize.rrd', $rrd_file);
-			}
-		}
-	}
-
-	if (isset($diff['tune']) || isset($diff['resize'])) {
-		if (!CACTI_CLI) {
-			html_end_box();
-		}
+	if (!$show_source) {
+		rrdtool_tune_execute($rrd_file, $diff);
+	} elseif (CACTI_CLI) {
+		rrdtool_tune_print_cli($rrd_file, $diff);
+	} else {
+		rrdtool_tune_print_html($rrd_file, $diff);
 	}
 }
 

tests/Unit/Core/Rrd/RrdTuneRenderTest.php

@@ -0,0 +1,212 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ |                                                                         |
+ | This program is free software; you can redistribute it and/or           |
+ | modify it under the terms of the GNU General Public License             |
+ | as published by the Free Software Foundation; either version 2          |
+ | of the License, or (at your option) any later version.                  |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+require_once dirname(__DIR__, 3) . '/Helpers/UnitStubs.php';
+require_once CACTI_PATH_LIBRARY . '/rrd.php';
+require_once CACTI_PATH_LIBRARY . '/html.php';
+
+function rrd_tune_capture(callable $callback) : string {
+	ob_start();
+
+	try {
+		$callback();
+	} finally {
+		$output = ob_get_clean();
+	}
+
+	return $output;
+}
+
+test('rrdtool_tune_print_leaves emits balanced div rows for the web UI', function () {
+	$diff = [
+		'The Graph requires 1 Data Sources but the RRDfile includes 2.',
+		'tune'   => ['/rra/x.rrd DEL:timeafter'],
+		'resize' => ['/rra/x.rrd 0 GROW 10'],
+	];
+
+	$output = rrd_tune_capture(function () use ($diff) {
+		rrdtool_tune_print_leaves($diff, true);
+	});
+
+	expect($output)->toContain("<div class='formRow'>The Graph requires 1 Data Sources but the RRDfile includes 2.</div>");
+	// the tune and resize branches are intentionally skipped by the leaf printer
+	expect($output)->not->toContain('DEL:timeafter');
+	expect(substr_count($output, '<div'))->toBe(substr_count($output, '</div>'));
+});
+
+test('rrdtool_tune_print_leaves emits plain text for the CLI', function () {
+	$diff = [
+		'Errors were found',
+		'nested' => ['Deeper error'],
+		'tune'   => ['ignored'],
+	];
+
+	$output = rrd_tune_capture(function () use ($diff) {
+		rrdtool_tune_print_leaves($diff, false);
+	});
+
+	expect($output)->toContain('Errors were found' . PHP_EOL);
+	expect($output)->toContain('Deeper error' . PHP_EOL);
+	expect($output)->not->toContain('<div');
+	expect($output)->not->toContain('ignored');
+});
+
+test('rrdtool_tune_print_html renders self contained, balanced div boxes', function () {
+	$diff = [
+		'Data Source timeafter is included but is not required',
+		'tune'   => ['/rra/x.rrd DEL:timeafter'],
+		'resize' => ['/rra/x.rrd 0 GROW 10'],
+	];
+
+	$output = rrd_tune_capture(function () use ($diff) {
+		rrdtool_tune_print_html('/rra/x.rrd', $diff);
+	});
+
+	expect($output)->toContain('Errors Found');
+	expect($output)->toContain('Optional Tuning Recommendations');
+	expect($output)->toContain('tune /rra/x.rrd DEL:timeafter');
+	expect($output)->toContain('resize /rra/x.rrd 0 GROW 10');
+	expect($output)->toContain('rename');
+	// the panel must not emit raw table markup any more
+	expect($output)->not->toContain('<tr>');
+	expect($output)->not->toContain('<td>');
+	// every div that opens must close
+	expect(substr_count($output, '<div'))->toBe(substr_count($output, '</div>'));
+});
+
+test('rrdtool_tune_print_html is empty when there is nothing to report', function () {
+	$output = rrd_tune_capture(function () {
+		rrdtool_tune_print_html('/rra/x.rrd', []);
+	});
+
+	expect($output)->toBe('');
+});
+
+test('rrdtool_tune_print_cli prints the rrdtool source commands', function () {
+	$diff = [
+		'An error leaf',
+		'tune'   => ['/rra/x.rrd DEL:timeafter'],
+		'resize' => ['/rra/x.rrd 0 GROW 10'],
+	];
+
+	$output = rrd_tune_capture(function () use ($diff) {
+		rrdtool_tune_print_cli('/rra/x.rrd', $diff);
+	});
+
+	expect($output)->toContain('An error leaf' . PHP_EOL);
+	expect($output)->toContain('tune /rra/x.rrd DEL:timeafter' . PHP_EOL);
+	expect($output)->toContain('resize /rra/x.rrd 0 GROW 10' . PHP_EOL);
+	expect($output)->toContain('rename');
+	expect($output)->not->toContain('<div');
+});
+
+test('rrdtool_tune_execute runs the tune and resize commands and relocates the resized file', function () {
+	$dir = sys_get_temp_dir() . '/rrd_tune_exec_' . getmypid();
+	@mkdir($dir);
+
+	$target = $dir . '/data.rrd';
+	file_put_contents($target, 'target');
+
+	$diff = [
+		'tune'   => [cacti_escapeshellarg($target) . ' DEL:timeafter'],
+		'resize' => [cacti_escapeshellarg($target) . ' 0 GROW 10'],
+	];
+
+	// a recording executor stands in for rrdtool and, like the real resize,
+	// only produces resize.rrd when the resize command actually runs
+	$commands = [];
+	$executor = function ($command) use (&$commands, $dir) {
+		$commands[] = $command;
+
+		if (strpos($command, 'resize ') === 0) {
+			file_put_contents($dir . '/resize.rrd', 'resized');
+		}
+
+		return true;
+	};
+
+	rrdtool_tune_execute($target, $diff, $executor);
+
+	// both commands must have been executed, not just the rename
+	expect($commands)->toHaveCount(2);
+	expect($commands[0])->toBe('tune ' . cacti_escapeshellarg($target) . ' DEL:timeafter');
+	expect($commands[1])->toBe('resize ' . cacti_escapeshellarg($target) . ' 0 GROW 10');
+
+	// the resize step renames resize.rrd over the target
+	expect(file_exists($dir . '/resize.rrd'))->toBeFalse();
+	expect(file_get_contents($target))->toBe('resized');
+
+	@unlink($target);
+	@rmdir($dir);
+});
+
+test('rrdtool_tune_execute skips the resize rename when no resize is produced', function () {
+	$dir = sys_get_temp_dir() . '/rrd_tune_noresize_' . getmypid();
+	@mkdir($dir);
+
+	$target = $dir . '/data.rrd';
+	file_put_contents($target, 'target');
+
+	$commands  = [];
+	$executor  = function ($command) use (&$commands) {
+		$commands[] = $command;
+
+		return true;
+	};
+
+	rrdtool_tune_execute($target, ['tune' => [cacti_escapeshellarg($target) . ' DEL:timeafter']], $executor);
+
+	expect($commands)->toBe(['tune ' . cacti_escapeshellarg($target) . ' DEL:timeafter']);
+	expect(file_get_contents($target))->toBe('target');
+
+	@unlink($target);
+	@rmdir($dir);
+});
+
+test('rrdtool_tune dispatches to the execute path when not showing the source', function () {
+	$dir = sys_get_temp_dir() . '/rrd_tune_disp_' . getmypid();
+	@mkdir($dir);
+
+	$target = $dir . '/data.rrd';
+	file_put_contents($target, 'target');
+	file_put_contents($dir . '/resize.rrd', 'resized');
+
+	$diff = [
+		'resize' => [cacti_escapeshellarg($target) . ' 0 GROW 10'],
+	];
+
+	// no executor is supplied, so the real rrdtool_execute is invoked; it returns
+	// without side effects in this environment while the resize rename still runs
+	rrdtool_tune($target, $diff, false);
+
+	expect(file_get_contents($target))->toBe('resized');
+
+	@unlink($target);
+	@rmdir($dir);
+});
+
+test('rrdtool_tune dispatches to the CLI printer under CACTI_CLI', function () {
+	expect(CACTI_CLI)->toBeTrue();
+
+	$diff = [
+		'tune' => ['/rra/x.rrd DEL:timeafter'],
+	];
+
+	$output = rrd_tune_capture(function () use ($diff) {
+		rrdtool_tune('/rra/x.rrd', $diff, true);
+	});
+
+	expect($output)->toContain('tune /rra/x.rrd DEL:timeafter' . PHP_EOL);
+	expect($output)->not->toContain('<div');
+});