From cbb856b0d4f4c8e52014b14a93a957c3179c4f45 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sat, 10 Oct 2026 02:29:03 -0700 Subject: [PATCH 1/5] security: Protect QuickTree add and clear actions against CSRF (#40) Signed-off-by: Thomas Vincent --- CHANGELOG.md | 1 + js/quicktree.js | 12 ++++---- quicktree.php | 15 ++++++++++ tests/client-post.test.mjs | 25 ++++++++++++++++ tests/csrf-actions.php | 60 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 106 insertions(+), 7 deletions(-) create mode 100644 tests/client-post.test.mjs create mode 100644 tests/csrf-actions.php diff --git a/CHANGELOG.md b/CHANGELOG.md index c65bf20..aef2d49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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, 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) * dev: Enforce patch coverage of changed lines in CI and remove the inert COMPOSER_ROOT_VERSION env from the Pest step diff --git a/js/quicktree.js b/js/quicktree.js index 70dafd0..083340e 100644 --- a/js/quicktree.js +++ b/js/quicktree.js @@ -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; } @@ -39,7 +37,7 @@ function addQuickTree(local_graph_id, rra_id) { } displayMessages(); - }); + }}); } $(function() { diff --git a/quicktree.php b/quicktree.php index e9b40a0..429a2b7 100644 --- a/quicktree.php +++ b/quicktree.php @@ -59,6 +59,21 @@ $action = $code_actions[$drp_action]; } +// Validate the effective action, including the bulk-action clear alias. +// remove stays GET until its row links become POST forms. +if (in_array($action, array('add', 'clear', 'save'), true)) { + if (($_SERVER['REQUEST_METHOD'] ?? 'GET') !== 'POST') { + header('Allow: POST'); + http_response_code(405); + exit; + } + + if (!function_exists('csrf_check') || !csrf_check(false)) { + http_response_code(403); + exit; + } +} + switch ($action) { case 'add': $graph = 0; diff --git a/tests/client-post.test.mjs b/tests/client-post.test.mjs new file mode 100644 index 0000000..d078e6d --- /dev/null +++ b/tests/client-post.test.mjs @@ -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/', + $: { ajax: (options) => { request = options; } }, + displayMessages: () => { notifications++; }, + }; + runInNewContext(readFileSync(new URL('../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); +}); diff --git a/tests/csrf-actions.php b/tests/csrf-actions.php new file mode 100644 index 0000000..f6efa17 --- /dev/null +++ b/tests/csrf-actions.php @@ -0,0 +1,60 @@ + $method, 'token' => $token, 'action' => $route[0], 'drp_action' => $route[1]), true); + $script = ' 'add_tree', 2 => 'add_branch', 3 => 'clear'); +$mutated = false; +function csrf_check($fatal) { return $GLOBALS['seed']['token'] === 'valid'; } +register_shutdown_function(function () { echo json_encode(array('status' => http_response_code(), 'mutated' => $GLOBALS['mutated'])); }); +PHP; + $script .= "\n" . $dispatch_guard . "\n\$mutated = true;"; + $process = proc_open(array(PHP_BINARY), array(0 => array('pipe', 'r'), 1 => array('pipe', 'w'), 2 => array('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]); + if (proc_close($process) !== 0 || $stderr !== '' || $result['status'] !== $expected || $result['mutated'] !== ($expected === 200)) { + throw new RuntimeException('QuickTree mutation guard regression: ' . $stderr); + } + } +} +echo "20 mutation guard cases passed\n"; From 778a07dd1d7a1f23d700fc0899c8dec4e1af65ad Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 18 Oct 2026 14:28:32 -0700 Subject: [PATCH 2/5] refactor: route QuickTree mutation checks through a named guard Signed-off-by: Thomas Vincent --- includes/security.php | 23 ++++++++++++++++++ quicktree.php | 15 ++++++------ tests/Unit/MutationStatusTest.php | 39 +++++++++++++++++++++++++++++++ tests/client-post.test.mjs | 4 ++-- tests/csrf-actions.php | 26 +++++++++++++++++---- 5 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 tests/Unit/MutationStatusTest.php diff --git a/includes/security.php b/includes/security.php index 011aa36..db5827c 100644 --- a/includes/security.php +++ b/includes/security.php @@ -40,3 +40,26 @@ function quicktree_normalize_location($value): string { return 'tab'; } + +/** + * Reports why a QuickTree request may not change state, or 0 when it may. + * Returns 405 when the request is not a POST, 403 when the CSRF token is + * missing or invalid, and 0 for a CSRF-validated POST. The caller sends the + * response and ends the request. Called from quicktree.php's dispatcher for + * the add, clear and save actions, including the clear alias selected by + * the bulk drp_action. The remove action stays GET until its row links + * become POST forms. + * + * @return int An HTTP status code, or 0 when the request is allowed. + */ +function quicktree_mutation_status(): int { + if (($_SERVER['REQUEST_METHOD'] ?? 'GET') !== 'POST') { + return 405; + } + + if (!function_exists('csrf_check') || !csrf_check(false)) { + return 403; + } + + return 0; +} diff --git a/quicktree.php b/quicktree.php index 429a2b7..f95633b 100644 --- a/quicktree.php +++ b/quicktree.php @@ -59,17 +59,16 @@ $action = $code_actions[$drp_action]; } -// Validate the effective action, including the bulk-action clear alias. // remove stays GET until its row links become POST forms. if (in_array($action, array('add', 'clear', 'save'), true)) { - if (($_SERVER['REQUEST_METHOD'] ?? 'GET') !== 'POST') { - header('Allow: POST'); - http_response_code(405); - exit; - } + $status = quicktree_mutation_status(); + + if ($status !== 0) { + if ($status === 405) { + header('Allow: POST'); + } - if (!function_exists('csrf_check') || !csrf_check(false)) { - http_response_code(403); + http_response_code($status); exit; } } diff --git a/tests/Unit/MutationStatusTest.php b/tests/Unit/MutationStatusTest.php new file mode 100644 index 0000000..1691a31 --- /dev/null +++ b/tests/Unit/MutationStatusTest.php @@ -0,0 +1,39 @@ +toBe(405); + + unset($_SERVER['REQUEST_METHOD']); + expect(quicktree_mutation_status())->toBe(405); + + $_SERVER['REQUEST_METHOD'] = 'POST'; + expect(quicktree_mutation_status())->toBe(403); + + function csrf_check($fatal) { + return $GLOBALS['__test_csrf_valid']; + } + + $GLOBALS['__test_csrf_valid'] = false; + expect(quicktree_mutation_status())->toBe(403); + + $GLOBALS['__test_csrf_valid'] = true; + expect(quicktree_mutation_status())->toBe(0); + + $_SERVER['REQUEST_METHOD'] = 'GET'; + expect(quicktree_mutation_status())->toBe(405); + }); +}); diff --git a/tests/client-post.test.mjs b/tests/client-post.test.mjs index d078e6d..63888d9 100644 --- a/tests/client-post.test.mjs +++ b/tests/client-post.test.mjs @@ -9,10 +9,10 @@ test('adding a graph sends a POST body and preserves the success notification', let notifications = 0; const context = { urlPath: '/cacti/', - $: { ajax: (options) => { request = options; } }, + $: Object.assign(() => ({}), { ajax: (options) => { request = options; } }), displayMessages: () => { notifications++; }, }; - runInNewContext(readFileSync(new URL('../quicktree.js', import.meta.url), 'utf8'), context); + 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'); diff --git a/tests/csrf-actions.php b/tests/csrf-actions.php index f6efa17..edd4e2f 100644 --- a/tests/csrf-actions.php +++ b/tests/csrf-actions.php @@ -28,9 +28,25 @@ $start = strpos($source, 'if ($drp_action != null)'); $end = strpos($source, 'switch ($action) {', $start); $dispatch_guard = substr($source, $start, $end - $start); +$security = file_get_contents(__DIR__ . '/../includes/security.php'); +$start = strpos($security, 'function quicktree_mutation_status()'); +$end = strpos($security, "\n}\n", $start) + 3; +$guard = substr($security, $start, $end - $start); -foreach (array(array('add', null), array('clear', null), array('save', null), array('', 3), array('view', 3)) as $route) { - foreach (array(array('GET', null, 405), array('POST', null, 403), array('POST', 'invalid', 403), array('POST', 'valid', 200)) as $case) { +$mutation_cases = array(array('GET', null, 405), array('POST', null, 403), array('POST', 'invalid', 403), array('POST', 'valid', 200)); +$remove_cases = array(array('GET', null, 200), array('POST', null, 200), array('POST', 'invalid', 200)); +$routes = array( + array('add', null, $mutation_cases), + array('clear', null, $mutation_cases), + array('save', null, $mutation_cases), + array('', 3, $mutation_cases), + array('view', 3, $mutation_cases), + array('remove', null, $remove_cases), +); +$count = 0; + +foreach ($routes as $route) { + foreach ($route[2] as $case) { list($method, $token, $expected) = $case; $seed = var_export(array('method' => $method, 'token' => $token, 'action' => $route[0], 'drp_action' => $route[1]), true); $script = ' http_response_code(), 'mutated' => $GLOBALS['mutated'])); }); PHP; - $script .= "\n" . $dispatch_guard . "\n\$mutated = true;"; + $script .= "\n" . $guard . "\n" . $dispatch_guard . "\n\$mutated = true;"; $process = proc_open(array(PHP_BINARY), array(0 => array('pipe', 'r'), 1 => array('pipe', 'w'), 2 => array('pipe', 'w')), $pipes); fwrite($pipes[0], $script); fclose($pipes[0]); @@ -55,6 +71,8 @@ function csrf_check($fatal) { return $GLOBALS['seed']['token'] === 'valid'; } if (proc_close($process) !== 0 || $stderr !== '' || $result['status'] !== $expected || $result['mutated'] !== ($expected === 200)) { throw new RuntimeException('QuickTree mutation guard regression: ' . $stderr); } + + $count++; } } -echo "20 mutation guard cases passed\n"; +echo "$count mutation guard cases passed\n"; From db050bb2640ca27e10f7b7f3465a1b3196c6c5db Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Fri, 23 Oct 2026 11:57:24 -0600 Subject: [PATCH 3/5] fix(csrf): use the shared Cacti POST guard with legacy fallback Signed-off-by: Thomas Vincent --- CHANGELOG.md | 2 +- includes/security.php | 23 -------- quicktree.php | 21 ++++---- tests/Unit/MutationStatusTest.php | 39 ++++---------- tests/csrf-actions.php | 88 ++++++++++++++++++------------- 5 files changed, 74 insertions(+), 99 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e90d3a8..ba6cdb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +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, and send the add request from the graph button as a POST +* 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 diff --git a/includes/security.php b/includes/security.php index 7472372..97c51ae 100644 --- a/includes/security.php +++ b/includes/security.php @@ -41,29 +41,6 @@ function quicktree_normalize_location($value): string { return 'tab'; } -/** - * Reports why a QuickTree request may not change state, or 0 when it may. - * Returns 405 when the request is not a POST, 403 when the CSRF token is - * missing or invalid, and 0 for a CSRF-validated POST. The caller sends the - * response and ends the request. Called from quicktree.php's dispatcher for - * the add, clear and save actions, including the clear alias selected by - * the bulk drp_action. The remove action stays GET until its row links - * become POST forms. - * - * @return int An HTTP status code, or 0 when the request is allowed. - */ -function quicktree_mutation_status(): int { - if (($_SERVER['REQUEST_METHOD'] ?? 'GET') !== 'POST') { - return 405; - } - - if (!function_exists('csrf_check') || !csrf_check(false)) { - return 403; - } - - return 0; -} - /** * Authorize an existing tree before QuickTree writes any items to it. * diff --git a/quicktree.php b/quicktree.php index 553b249..f876523 100644 --- a/quicktree.php +++ b/quicktree.php @@ -60,15 +60,18 @@ } // remove stays GET until its row links become POST forms. -if (in_array($action, array('add', 'clear', 'save'), true)) { - $status = quicktree_mutation_status(); - - if ($status !== 0) { - if ($status === 405) { - header('Allow: POST'); - } +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; + } - http_response_code($status); + // 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; } } @@ -208,7 +211,7 @@ break; case 'clear': - $SQL = db_execute_prepared('DELETE FROM quicktree_graphs WHERE userid = ?', array($user)); + $SQL = db_execute_prepared('DELETE FROM quicktree_graphs WHERE userid = ?', [$user]); header('Location: quicktree.php?header=false&drp_action=&action=&location=' . $location); exit; diff --git a/tests/Unit/MutationStatusTest.php b/tests/Unit/MutationStatusTest.php index 1691a31..68046bf 100644 --- a/tests/Unit/MutationStatusTest.php +++ b/tests/Unit/MutationStatusTest.php @@ -7,33 +7,14 @@ +-------------------------------------------------------------------------+ */ - -require_once __DIR__ . '/../../includes/security.php'; - -describe('quicktree mutation status', function () { - // A function cannot be undefined once declared, so the case without - // csrf_check has to run before the stub below exists. - it('rejects non-POST requests and POSTs that fail the CSRF check', function () { - $_SERVER['REQUEST_METHOD'] = 'GET'; - expect(quicktree_mutation_status())->toBe(405); - - unset($_SERVER['REQUEST_METHOD']); - expect(quicktree_mutation_status())->toBe(405); - - $_SERVER['REQUEST_METHOD'] = 'POST'; - expect(quicktree_mutation_status())->toBe(403); - - function csrf_check($fatal) { - return $GLOBALS['__test_csrf_valid']; - } - - $GLOBALS['__test_csrf_valid'] = false; - expect(quicktree_mutation_status())->toBe(403); - - $GLOBALS['__test_csrf_valid'] = true; - expect(quicktree_mutation_status())->toBe(0); - - $_SERVER['REQUEST_METHOD'] = 'GET'; - expect(quicktree_mutation_status())->toBe(405); - }); +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'); }); diff --git a/tests/csrf-actions.php b/tests/csrf-actions.php index edd4e2f..2d77160 100644 --- a/tests/csrf-actions.php +++ b/tests/csrf-actions.php @@ -23,33 +23,28 @@ +-------------------------------------------------------------------------+ */ - -$source = file_get_contents(__DIR__ . '/../quicktree.php'); -$start = strpos($source, 'if ($drp_action != null)'); -$end = strpos($source, 'switch ($action) {', $start); +$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); -$security = file_get_contents(__DIR__ . '/../includes/security.php'); -$start = strpos($security, 'function quicktree_mutation_status()'); -$end = strpos($security, "\n}\n", $start) + 3; -$guard = substr($security, $start, $end - $start); - -$mutation_cases = array(array('GET', null, 405), array('POST', null, 403), array('POST', 'invalid', 403), array('POST', 'valid', 200)); -$remove_cases = array(array('GET', null, 200), array('POST', null, 200), array('POST', 'invalid', 200)); -$routes = array( - array('add', null, $mutation_cases), - array('clear', null, $mutation_cases), - array('save', null, $mutation_cases), - array('', 3, $mutation_cases), - array('view', 3, $mutation_cases), - array('remove', null, $remove_cases), -); +$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 ($routes as $route) { - foreach ($route[2] as $case) { - list($method, $token, $expected) = $case; - $seed = var_export(array('method' => $method, 'token' => $token, 'action' => $route[0], 'drp_action' => $route[1]), true); - $script = ' $method, 'token' => $token, 'action' => $route[0], 'drp_action' => $route[1]], true); + $script = ' http_response_code(), 'mutated' => $GLOBALS['mutated'])); }); PHP; - $script .= "\n" . $guard . "\n" . $dispatch_guard . "\n\$mutated = true;"; - $process = proc_open(array(PHP_BINARY), array(0 => array('pipe', 'r'), 1 => array('pipe', 'w'), 2 => array('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]); - if (proc_close($process) !== 0 || $stderr !== '' || $result['status'] !== $expected || $result['mutated'] !== ($expected === 200)) { - throw new RuntimeException('QuickTree mutation guard regression: ' . $stderr); - } - $count++; + if ($host !== 'legacy') { + $script .= <<<'PHP' + +function csrf_require_post() { + 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]); + + if (proc_close($process) !== 0 || $stderr !== '' || $result['status'] !== $expected || $result['mutated'] !== ($expected === 200)) { + throw new RuntimeException('QuickTree mutation guard regression: ' . $stderr); + } + + $count++; + } } } -echo "$count mutation guard cases passed\n"; +print "$count mutation guard cases passed\n"; From 965e933cae5f50e6491e6efe942c2eb510bbc171 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Fri, 23 Oct 2026 14:31:32 -0600 Subject: [PATCH 4/5] test(csrf): require shared guard invocation on supported hosts Signed-off-by: Thomas Vincent --- tests/csrf-actions.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/csrf-actions.php b/tests/csrf-actions.php index 2d77160..5e0dbbb 100644 --- a/tests/csrf-actions.php +++ b/tests/csrf-actions.php @@ -52,14 +52,17 @@ $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'])); }); +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); @@ -81,7 +84,9 @@ function csrf_require_post() { fclose($pipes[1]); fclose($pipes[2]); - if (proc_close($process) !== 0 || $stderr !== '' || $result['status'] !== $expected || $result['mutated'] !== ($expected === 200)) { + $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); } From cb49cbeffa8db924d6867bd720850be1f6ac7c1f Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Fri, 23 Oct 2026 20:43:57 -0600 Subject: [PATCH 5/5] fix(tests): preserve the existing prepared statement parameter form Signed-off-by: Thomas Vincent --- quicktree.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quicktree.php b/quicktree.php index f876523..242b25d 100644 --- a/quicktree.php +++ b/quicktree.php @@ -211,7 +211,7 @@ break; case 'clear': - $SQL = db_execute_prepared('DELETE FROM quicktree_graphs WHERE userid = ?', [$user]); + $SQL = db_execute_prepared('DELETE FROM quicktree_graphs WHERE userid = ?', array($user)); header('Location: quicktree.php?header=false&drp_action=&action=&location=' . $location); exit;