mirror of
https://github.com/Cacti/cacti.git
synced 2026-11-02 18:17:00 +09:00
* Add one-click RRDfile repair to Data Source Info mode The Data Source Info mode already compares the RRDfile against the Cacti definition and prints the rrdtool tune recommendations (add a missing Data Source with DS:name:type:heartbeat:min:max, remove an extra one with DEL:name, and correct the type, heartbeat, minimum and maximum). It did not, however, offer a way to apply them from the page the way data_debug.php does. - Add an "Apply Suggested Fixes" link to the "RRDtool Tune Info" box header when the web server can write the RRDfile; when it cannot, the header falls back to "Run Fix from Command Line" and the commands remain listed for manual use. - Add a ds_repair action that runs the same tune recommendations server-side (gated on data_source_authorized() and on the RRDfile being writable, and skipping any recommendation carrying control characters), logs each command to DSDEBUG, then reloads the Data Source so the result is visible. This mirrors data_debug.php's run_repair / "Apply Suggested Fixes". - Harden the Info-mode guard so a missing/unreadable RRDfile (rrdtool info returning false) no longer triggers an array-offset warning. * Address review: POST-guard ds_repair and harden repair result check - Route the "Apply Suggested Fixes" action through a CSRF-protected POST: the link now uses the cactiPostAction handler (href="#", data-url) instead of a GET navigation, and ds_repair is added to the $bad_actions guard in include/global.php so a tokenless request fails closed with 405. ds_repair is added to the curated CSRF action test for regression coverage. - Use a strict comparison on the rrdtool_execute() result: it can return null on a failed exec/proxy connection, and null == '' would have reported a failed repair as "All RRDfile repairs succeeded." - Refresh the sink inventory baseline for the new header redirects in ds_repair. * Add request-coverage probe for the ds_repair RRDfile repair handler The Data Source Info mode repair handler ds_repair() is a POST action that the page-level e2e suites do not drive (it needs a Data Source whose RRDfile mismatches its Cacti definition). Add a request-coverage fixture that loads the handler in isolation via cacti_fixture_source_function(), stubs its I/O boundaries, and drives every branch (unauthorized, unreadable RRDfile, no RRA, nothing to tune, not writable, a control-character recommendation that is skipped, and both tune success and failure). Register it in run_request_coverage.sh for data_sources.php changes so the changed-line coverage gate sees the handler exercised. * Address review: use defined helpers and boolean repair result - Replace the undefined cacti_has_control_chars() check with a comparison against rrd_strip_control_chars() (loaded from lib/rrd.php) to skip any tune recommendation that carries control characters. - Execute each tune with RRDTOOL_OUTPUT_BOOLEAN and treat a strict true as success so proxy storage (which returns the raw 'OK u:...' status under RRDTOOL_OUTPUT_RETURN_STDERR) is not misreported as a failure. - Add the new translatable strings to locales/po/cacti.pot and reuse the existing failure msgid. - Update the request-coverage fixture to the new boundary contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
225 lines
7.5 KiB
PHP
225 lines
7.5 KiB
PHP
<?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 |
|
|
+-------------------------------------------------------------------------+
|
|
*/
|
|
|
|
/*
|
|
* Item, tree, RRA, and ordering actions change state and were reachable by GET,
|
|
* so a cross-origin request could delete or reorder objects using the victim's
|
|
* authentication state.
|
|
*
|
|
* The guard and the links have to move together: adding the actions to
|
|
* $bad_actions while a page still links them by GET breaks the delete it is
|
|
* meant to protect. These two assertions fail in opposite directions, so
|
|
* neither half can land alone.
|
|
*/
|
|
|
|
/**
|
|
* Every GET reachable action whose handler mutates state. Read-only actions are
|
|
* deliberately absent: item_edit, readme, changelog, latest, avail and the
|
|
* *_confirm dialogs render a page and change nothing.
|
|
*
|
|
* @return array<int, string> Action names.
|
|
*/
|
|
function guarded_actions() {
|
|
return [
|
|
'item_remove', 'item_moveup', 'item_movedown',
|
|
'item_remove_gsv', 'item_remove_dssv',
|
|
'item_moveup_gsv', 'item_moveup_dssv',
|
|
'item_movedown_gsv', 'item_movedown_dssv',
|
|
'delete_node', 'gt_remove', 'query_remove', 'remove', 'change_leaf',
|
|
'moveup', 'movedown',
|
|
'tree_up', 'tree_down', 'move_page_up', 'move_page_down',
|
|
'rrd_add', 'rrd_remove',
|
|
'ds_repair',
|
|
];
|
|
}
|
|
|
|
/**
|
|
* A pattern matching any guarded action, longest name first so that
|
|
* item_remove_gsv is not matched as item_remove.
|
|
*
|
|
* @return string A regex alternation.
|
|
*/
|
|
function guarded_action_pattern() {
|
|
$actions = guarded_actions();
|
|
|
|
usort($actions, function ($a, $b) {
|
|
return strlen($b) - strlen($a);
|
|
});
|
|
|
|
return 'action=(?:' . implode('|', array_map('preg_quote', $actions)) . ')(?![a-z_])';
|
|
}
|
|
|
|
function item_action_files() {
|
|
$root = dirname(__DIR__, 4);
|
|
$found = [];
|
|
|
|
$it = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($root, FilesystemIterator::SKIP_DOTS));
|
|
|
|
foreach ($it as $file) {
|
|
$path = $file->getPathname();
|
|
|
|
if (substr($path, -4) !== '.php') {
|
|
continue;
|
|
}
|
|
|
|
if (strpos($path, '/include/vendor/') !== false || strpos($path, '/tests/') !== false) {
|
|
continue;
|
|
}
|
|
|
|
$src = file_get_contents($path);
|
|
|
|
if ($src !== false && preg_match('/' . guarded_action_pattern() . '/', $src)) {
|
|
$found[] = substr($path, strlen($root) + 1);
|
|
}
|
|
}
|
|
|
|
sort($found);
|
|
|
|
return $found;
|
|
}
|
|
|
|
test('every state changing action is refused without a POST token', function () {
|
|
$src = file_get_contents(dirname(__DIR__, 4) . '/include/global.php');
|
|
|
|
expect($src)->not->toBeFalse();
|
|
|
|
preg_match('/\$bad_actions\s*=\s*(?:array\(|\[)(.*?)(?:\);|\];)/s', $src, $matches);
|
|
|
|
expect($matches)->toHaveKey(1);
|
|
|
|
$list = $matches[1];
|
|
|
|
$missing = [];
|
|
|
|
foreach (guarded_actions() as $action) {
|
|
if (strpos($list, "'" . $action . "'") === false) {
|
|
$missing[] = $action;
|
|
}
|
|
}
|
|
|
|
expect($missing)->toBe([]);
|
|
expect($src)->toContain("(\$_SERVER['REQUEST_METHOD'] ?? 'GET') !== 'POST'")
|
|
->and($src)->toContain("header('Allow: POST')")
|
|
->and($src)->toContain('http_response_code(405)')
|
|
->and($src)->toContain('http_response_code(403)');
|
|
});
|
|
|
|
test('no page still links a state changing action by GET', function () {
|
|
$files = item_action_files();
|
|
|
|
expect($files)->not->toBe([]);
|
|
|
|
$unconverted = [];
|
|
|
|
foreach ($files as $file) {
|
|
$src = file_get_contents(dirname(__DIR__, 4) . '/' . $file);
|
|
|
|
/* Anchors are emitted across two lines in places, so collapse the file
|
|
before pairing an href with the class that posts it. */
|
|
$flat = preg_replace('/\s+/', ' ', $src);
|
|
|
|
if (preg_match_all('/<a\b[^>]*?' . guarded_action_pattern() . '[^>]*>/', $flat, $matches)) {
|
|
foreach ($matches[0] as $anchor) {
|
|
$has_safe_href = strpos($anchor, "href='#'") !== false
|
|
|| strpos($anchor, 'href="#"') !== false;
|
|
$uses_global_handler = strpos($anchor, 'cactiPostAction') !== false;
|
|
$uses_page_handler = strpos($anchor, 'remover') !== false
|
|
&& strpos($src, "$('.remover').on('click'") !== false
|
|
&& strpos($src, 'cactiPreparePostRequestFromUrl') !== false;
|
|
|
|
if (!$has_safe_href || (!$uses_global_handler && !$uses_page_handler)) {
|
|
$unconverted[] = $file;
|
|
|
|
break;
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
expect($unconverted)->toBe([]);
|
|
});
|
|
|
|
test('the cactiPostAction handler posts the token and refuses another origin', function () {
|
|
$js = file_get_contents(dirname(__DIR__, 4) . '/include/layout.js');
|
|
|
|
expect($js)->not->toBeFalse();
|
|
|
|
/* The class has to be bound on its own, not only through the ajaxAnchors
|
|
selector, because several tagged anchors carry none of the classes that
|
|
selector names. */
|
|
expect($js)->toContain("$(document).rebind('click.cactiPostAction', 'a.cactiPostAction'")
|
|
->and($js)->toContain(".not('.cactiPostAction').rebind('click'")
|
|
->and($js)->toContain('submitPageUsingPost')
|
|
->and($js)->toContain('cactiPreparePostRequestFromUrl');
|
|
|
|
$start = strpos($js, 'function cactiPreparePostRequest(');
|
|
|
|
expect($start)->not->toBeFalse();
|
|
|
|
$body = substr($js, $start, 600);
|
|
|
|
expect($body)->toContain('csrfMagicToken')
|
|
->and($body)->toContain('__csrf_magic')
|
|
->and($body)->toContain('Refusing to send a CSRF token to a different origin');
|
|
});
|
|
|
|
test('state action URLs use attribute-safe escaping and the page handler posts', function () {
|
|
$data_queries = file_get_contents(dirname(__DIR__, 4) . '/data_queries.php');
|
|
$html = file_get_contents(dirname(__DIR__, 4) . '/lib/html.php');
|
|
|
|
expect($data_queries)->not->toBeFalse()
|
|
->and($html)->not->toBeFalse()
|
|
->and(substr_count($data_queries, "data-url='<?php print html_escape_url("))->toBeGreaterThanOrEqual(6)
|
|
->and(substr_count($html, "data-url='\" . html_escape_url("))->toBeGreaterThanOrEqual(3)
|
|
->and($data_queries)->toContain("$('.remover').on('click'")
|
|
->and($data_queries)->toContain('cactiPreparePostRequestFromUrl(href)')
|
|
->and($data_queries)->toContain('$.post(request.url, request.data)');
|
|
});
|
|
|
|
test('every guarded data URL uses attribute-safe URL escaping', function () {
|
|
$unsafe = [];
|
|
|
|
foreach (item_action_files() as $file) {
|
|
$lines = file(dirname(__DIR__, 4) . '/' . $file);
|
|
|
|
foreach ($lines as $line_number => $line) {
|
|
$attribute_start = strpos($line, 'data-url=');
|
|
|
|
if ($attribute_start === false || !preg_match('/' . guarded_action_pattern() . '/', $line)) {
|
|
continue;
|
|
}
|
|
|
|
$attribute = substr($line, $attribute_start);
|
|
|
|
if (strpos($attribute, 'html_escape_url(') === false
|
|
|| strpos($attribute, 'htmle(') !== false
|
|
|| strpos($attribute, 'htmlspecialchars(') !== false) {
|
|
$unsafe[] = $file . ':' . ($line_number + 1);
|
|
}
|
|
}
|
|
}
|
|
|
|
expect($unsafe)->toBe([]);
|
|
});
|
|
|
|
test('the data template add button submits its guarded action by POST', function () {
|
|
$data_templates = file_get_contents(dirname(__DIR__, 4) . '/data_templates.php');
|
|
$html = file_get_contents(dirname(__DIR__, 4) . '/lib/html.php');
|
|
|
|
expect($data_templates)->toContain("'data_url' => 'data_templates.php?action=rrd_add&id='")
|
|
->and($html)->toContain("isset(\$icon['data_url'])")
|
|
->and($html)->toContain("html_escape_url(\$icon['data_url'])")
|
|
->and($html)->toContain("linkOverDark cactiPostAction");
|
|
});
|