Skip to content
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* feature: Shorten the graph 'Add to QuickTree' action icon's title to "Add to Quicktree" so it reads cleanly as a label in the Cacti graph drilldown action menu
* bug: Pass a boolean (not the integer 1) to api_plugin_register_hook()'s $enable argument so the hook registration stays type-correct under Cacti 1.3's strict bool type-hint (the int only coerces while the file lacks declare(strict_types=1))
* security: Move the graph 'Add to QuickTree' icon's inline `onClick='addQuickTree()'` handler into a delegated jQuery binding in `js/quicktree.js` (via a `quicktreeAddLink` class and `data-` attributes) and replace the confirmation pages' inline `onClick='cactiReturnTo()'` Cancel buttons with the CSP-safe `cactiReturnTo` class, so the pages no longer trip Cacti's Content-Security-Policy script-src-attr directive
* security: Require a POST request with a valid CSRF token for the add, clear and save actions using Cacti's shared guard with a fallback for older hosts, and send the add request from the graph button as a POST
* dev: Replace the translation-template CI check that regenerated locales/po/cacti.pot and compared it with a diff-based gate (tests/bin/check-i18n-pot.php) requiring cacti.pot to be updated only when a pull request adds, removes, or modifies an i18n function call
* dev: Measure CI coverage with xdebug instead of pcov so the plugin's own sources are instrumented (pcov auto-scopes to the Composer root and skipped cacti/plugins/, leaving the patch-coverage gate with nothing to measure)
* security: Normalize the requested location before building the breadcrumb URL so a crafted location value is not reflected into the navigation
Expand Down
12 changes: 5 additions & 7 deletions js/quicktree.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,11 @@
*/

function addQuickTree(local_graph_id, rra_id) {
var strURL = urlPath +
'plugins/quicktree/quicktree.php' +
'?action=add' +
'&rra_id=' + rra_id +
'&graph_id=' + local_graph_id;
const strURL = urlPath + 'plugins/quicktree/quicktree.php';

$.getJSON(strURL, function(message) {
$.ajax({type: 'POST', url: strURL, dataType: 'json',
data: {action: 'add', rra_id: rra_id, graph_id: local_graph_id},
success: function(message) {
if (message.title) {
sessionMessageTitle = message.title;
}
Expand All @@ -39,7 +37,7 @@ function addQuickTree(local_graph_id, rra_id) {
}

displayMessages();
});
}});
}

$(function() {
Expand Down
17 changes: 17 additions & 0 deletions quicktree.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,23 @@
$action = $code_actions[$drp_action];
}

// remove stays GET until its row links become POST forms.
if (in_array($action, ['add', 'clear', 'save'], true)) {
if (function_exists('csrf_require_post')) {
csrf_require_post();
} elseif (($_SERVER['REQUEST_METHOD'] ?? 'GET') !== 'POST') {
header('Allow: POST');
http_response_code(405);
exit;
}

// The 1.2.x helper checks the method only; validate the token on every host.
if (!function_exists('csrf_check') || !csrf_check(false)) {
http_response_code(403);
exit;
}
}

switch ($action) {
case 'add':
$graph = 0;
Expand Down
20 changes: 20 additions & 0 deletions tests/Unit/MutationStatusTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/

it('protects mutation routes with legacy, method-only and token-checking Cacti guards', function () {
$process = proc_open([PHP_BINARY, __DIR__ . '/../csrf-actions.php'], [1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes);
$stdout = stream_get_contents($pipes[1]);
$stderr = stream_get_contents($pipes[2]);
fclose($pipes[1]);
fclose($pipes[2]);

expect(proc_close($process))->toBe(0)
->and($stderr)->toBe('')
->and(trim($stdout))->toBe('69 mutation guard cases passed');
});
25 changes: 25 additions & 0 deletions tests/client-post.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/* Copyright (C) 2026 The Cacti Group. GPL version 2 or later. */
import assert from 'node:assert/strict';
import { readFileSync } from 'node:fs';
import { runInNewContext } from 'node:vm';
import test from 'node:test';

test('adding a graph sends a POST body and preserves the success notification', () => {
let request;
let notifications = 0;
const context = {
urlPath: '/cacti/',
$: Object.assign(() => ({}), { ajax: (options) => { request = options; } }),
displayMessages: () => { notifications++; },
};
runInNewContext(readFileSync(new URL('../js/quicktree.js', import.meta.url), 'utf8'), context);
context.addQuickTree(42, 7);
assert.equal(request.type, 'POST');
assert.equal(request.url, '/cacti/plugins/quicktree/quicktree.php');
assert.equal(request.dataType, 'json');
assert.deepEqual(JSON.parse(JSON.stringify(request.data)), {action: 'add', rra_id: 7, graph_id: 42});
request.success({title: 'Added', message: 'Queued'});
assert.equal(context.sessionMessageTitle, 'Added');
assert.equal(context.sessionMessage.message, 'Queued');
assert.equal(notifications, 1);
});
97 changes: 97 additions & 0 deletions tests/csrf-actions.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2023 Howard Jones |
| Copyright (C) 2022 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. |
| |
| This program is distributed in the hope that it will be useful, |
| but WITHOUT ANY WARRANTY; without even the implied warranty of |
| MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
| GNU General Public License for more details. |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
| This code is designed, written, and maintained by the Cacti Group. See |
| about.php and/or the AUTHORS file for specific developer information. |
+-------------------------------------------------------------------------+
| http://www.cacti.net/ |
+-------------------------------------------------------------------------+
*/

$source = file_get_contents(__DIR__ . '/../quicktree.php');
$start = strpos($source, 'if ($drp_action != null)');
$end = strpos($source, 'switch ($action) {', $start);
$dispatch_guard = substr($source, $start, $end - $start);
$mutation_cases = [['GET', null, 405], ['POST', null, 403], ['POST', 'invalid', 403], ['POST', 'valid', 200]];
$remove_cases = [['GET', null, 200], ['POST', null, 200], ['POST', 'invalid', 200]];
$routes = [
['add', null, $mutation_cases],
['clear', null, $mutation_cases],
['save', null, $mutation_cases],
['', 3, $mutation_cases],
['view', 3, $mutation_cases],
['remove', null, $remove_cases],
];
$count = 0;

foreach (['legacy', 'method-only', 'method-and-token'] as $host) {
foreach ($routes as $route) {
foreach ($route[2] as $case) {
[$method, $token, $expected] = $case;
$seed = var_export(['method' => $method, 'token' => $token, 'action' => $route[0], 'drp_action' => $route[1]], true);
$script = '<?php $seed = ' . $seed . ';' . <<<'PHP'

$_SERVER['REQUEST_METHOD'] = $seed['method'];
http_response_code(200);
$action = $seed['action'];
$drp_action = $seed['drp_action'];
$code_actions = array(1 => 'add_tree', 2 => 'add_branch', 3 => 'clear');
$mutated = false;
$core_guard_calls = 0;
function csrf_check($fatal) { return $GLOBALS['seed']['token'] === 'valid'; }
register_shutdown_function(function () { echo json_encode(array('status' => http_response_code(), 'mutated' => $GLOBALS['mutated'], 'core_guard_calls' => $GLOBALS['core_guard_calls'])); });
PHP;
$script .= "\nrequire " . var_export(__DIR__ . '/../includes/security.php', true) . ';';

if ($host !== 'legacy') {
$script .= <<<'PHP'

function csrf_require_post() {
$GLOBALS['core_guard_calls']++;
if ($_SERVER['REQUEST_METHOD'] !== 'POST') {
header('Allow: POST');
http_response_code(405);
exit;
}
PHP;

if ($host === 'method-and-token') {
$script .= 'if (!csrf_check(false)) { http_response_code(403); exit; }';
}
$script .= "\n}\n";
}
$script .= "\n" . $dispatch_guard . "\n\$mutated = true;";
$process = proc_open([PHP_BINARY], [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes);
fwrite($pipes[0], $script);
fclose($pipes[0]);
$result = json_decode(stream_get_contents($pipes[1]), true);
$stderr = stream_get_contents($pipes[2]);
fclose($pipes[1]);
fclose($pipes[2]);

$uses_core_guard = $host !== 'legacy' && (in_array($route[0], ['add', 'clear', 'save'], true) || $route[1] === 3);

if (proc_close($process) !== 0 || $stderr !== '' || $result['status'] !== $expected || $result['mutated'] !== ($expected === 200) || $result['core_guard_calls'] !== (int) $uses_core_guard) {
throw new RuntimeException('QuickTree mutation guard regression: ' . $stderr);
}

$count++;
}
}
}
print "$count mutation guard cases passed\n";
Loading