hardening: replace eval callback execution in autocomplete handling by somethingwithproof · Pull Request #268 · Cacti/plugin_syslog

somethingwithproof · 2026-03-06T23:28:18Z

Summary

replace eval(onChange) with safe function-reference execution in initSyslogAutocomplete()
validate callback names before invocation and execute only named global functions
add regression coverage for removing eval usage
run regression scripts from CI when tests/regression/*.php exist
add changelog entry for issue hardening: replace eval-based callback execution in syslog autocomplete JS #260

Validation

php -l tests/regression/issue260_remove_eval_callback_test.php
php tests/regression/issue260_remove_eval_callback_test.php

Fixes #260

Copilot

Pull request overview

This PR hardens the Syslog plugin’s autocomplete handling by removing eval() usage when executing onChange callbacks in js/functions.js, reducing client-side code execution risk and improving security posture.

Changes:

Replaced eval(onChange) with a safer helper (runSyslogAutocompleteOnChange) that validates and invokes only named global functions.
Added a regression script to ensure eval( no longer appears in js/functions.js and that the safe helper remains present.
Updated CI to run plugin regression scripts and added a changelog entry for issue #260.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
`js/functions.js`	Introduces safe callback execution helper and replaces `eval()` usage in autocomplete select handler.
`tests/regression/issue260_remove_eval_callback_test.php`	Adds a regression check to prevent reintroduction of `eval()` in autocomplete callback execution.
`.github/workflows/plugin-ci-workflow.yml`	Runs regression scripts during CI for the syslog plugin.
`CHANGELOG.md`	Documents the hardening change under develop for issue #260.

Copilot · 2026-03-06T23:33:41Z

js/functions.js

 /**
 * Initialize autocomplete for form dropdown fields
 * @param {string} formName - The name of the form field
 * @param {string} callback - The AJAX callback action
 * @param {string} onChange - The onChange callback to execute when selection changes
 */
+function runSyslogAutocompleteOnChange(onChange) {
+	if (typeof onChange !== 'string') {


The JSDoc block describes initSyslogAutocomplete(...), but it now sits directly above runSyslogAutocompleteOnChange(...), so the documentation no longer matches the function it is annotating. Please move this block to initSyslogAutocomplete (or split into two doc blocks so the helper is documented separately).

Copilot · 2026-03-06T23:33:41Z

tests/regression/issue260_remove_eval_callback_test.php

+if (strpos($javascript, 'function runSyslogAutocompleteOnChange(onChange)') === false) {
+	fwrite(STDERR, "Safe autocomplete callback helper is missing.\n");
+	exit(1);
+}
+
+if (strpos($javascript, "if (!callbackName.match(/^[A-Za-z_$][A-Za-z0-9_$]*$/))") === false) {
+	fwrite(STDERR, "Autocomplete callback name validation is missing.\n");
+	exit(1);
+}
+
+if (strpos($javascript, "if (typeof window[callbackName] === 'function')") === false ||
+	strpos($javascript, 'window[callbackName]();') === false) {
+	fwrite(STDERR, "Expected function-reference callback execution is missing.\n");
+	exit(1);
+}
+
+if (strpos($javascript, 'runSyslogAutocompleteOnChange(onChange);') === false) {


This regression test is very brittle because it asserts exact source-code substrings (including whitespace and exact function signature). Consider using preg_match() with more flexible patterns (e.g., look for runSyslogAutocompleteOnChange definition and the identifier-validation regex) so harmless refactors/formatting changes don’t break CI.

Suggested change

if (strpos($javascript, 'function runSyslogAutocompleteOnChange(onChange)') === false) {

fwrite(STDERR, "Safe autocomplete callback helper is missing.\n");

exit(1);

}

if (strpos($javascript, "if (!callbackName.match(/^[A-Za-z_$][A-Za-z0-9_$]*$/))") === false) {

fwrite(STDERR, "Autocomplete callback name validation is missing.\n");

exit(1);

}

if (strpos($javascript, "if (typeof window[callbackName] === 'function')") === false ||

strpos($javascript, 'window[callbackName]();') === false) {

fwrite(STDERR, "Expected function-reference callback execution is missing.\n");

exit(1);

}

if (strpos($javascript, 'runSyslogAutocompleteOnChange(onChange);') === false) {

if (!preg_match('/function\s+runSyslogAutocompleteOnChange\s*$\s*onChange\s*$/', $javascript)) {

fwrite(STDERR, "Safe autocomplete callback helper is missing.\n");

exit(1);

}

if (!preg_match('~callbackName\.match\s*$\s*/\^[A-Za-z_$][A-Za-z0-9_$]*\$/~', $javascript)) {

fwrite(STDERR, "Autocomplete callback name validation is missing.\n");

exit(1);

}

if (!preg_match('/if\s*\(\s*typeof\s+window\[callbackName\]\s*===\s*[\'"]function[\'"]\s*$/', $javascript) ||

!preg_match('/window\[callbackName\]\s*$\s*$\s*;/', $javascript)) {

fwrite(STDERR, "Expected function-reference callback execution is missing.\n");

exit(1);

}

if (!preg_match('/runSyslogAutocompleteOnChange\s*$\s*onChange\s*$\s*;/', $javascript)) {

Fixed -- test now uses preg_match() patterns.

Copilot · 2026-03-06T23:33:41Z

tests/regression/issue260_remove_eval_callback_test.php

+if (strpos($javascript, "if (typeof window[callbackName] === 'function')") === false ||
+	strpos($javascript, 'window[callbackName]();') === false) {


This assertion depends on an exact statement string (window[callbackName]();) which can easily change with formatting or minor refactors (e.g., adding arguments). Prefer matching the behavior more flexibly (regex) or asserting the absence of eval( plus presence of window[callbackName] invocation without hardcoding the full line.

Suggested change

if (strpos($javascript, "if (typeof window[callbackName] === 'function')") === false ||

strpos($javascript, 'window[callbackName]();') === false) {

$hasCallbackTypeCheck = preg_match(

'/if\s*$\s*typeof\s+window\s*\[\s*callbackName\s*\]\s*===\s*[\'"]function[\'"]\s*$/',

$javascript

) === 1;

$hasCallbackInvocation = preg_match(

'/window\s*\[\s*callbackName\s*\]\s*\(/',

$javascript

) === 1;

if (!$hasCallbackTypeCheck || !$hasCallbackInvocation) {

somethingwithproof · 2026-03-07T06:17:38Z

Fixed in ea7668d -- added JSDoc to runSyslogAutocompleteOnChange so both autocomplete helpers are documented; switched the function-signature assertion in the regression test to preg_match to tolerate minor whitespace variation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

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

Add JSDoc block to the runSyslogAutocompleteOnChange helper so both autocomplete functions are documented. Replace exact-substring strpos check in the regression test with preg_match to tolerate minor whitespace variations in the signature without weakening the security assertion. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

…test strpos() exact matches are brittle to whitespace/formatting changes. preg_match() with flexible patterns assert the same semantics without coupling to exact source formatting. Signed-off-by: Thomas Vincent <thomas@somethingwithproof.com> Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Copilot AI review requested due to automatic review settings March 6, 2026 23:28

Copilot started reviewing on behalf of somethingwithproof March 6, 2026 23:28 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

somethingwithproof and others added 4 commits March 7, 2026 05:32

hardening: remove eval from autocomplete callback execution

1fc24b1

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

fix: move JSDoc block to sit directly above initSyslogAutocomplete

a9e3ffb

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

somethingwithproof force-pushed the hardening/remove-eval-autocomplete branch from a050c63 to fef4ca7 Compare March 7, 2026 13:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hardening: replace eval callback execution in autocomplete handling#268

hardening: replace eval callback execution in autocomplete handling#268
somethingwithproof wants to merge 4 commits intoCacti:developfrom
somethingwithproof:hardening/remove-eval-autocomplete

somethingwithproof commented Mar 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

somethingwithproof Mar 7, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

somethingwithproof Mar 7, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

somethingwithproof Mar 7, 2026

Uh oh!

somethingwithproof commented Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		if (strpos($javascript, "if (typeof window[callbackName] === 'function')") === false \|\|
		strpos($javascript, 'window[callbackName]();') === false) {

-if (strpos($javascript, "if (typeof window[callbackName] === 'function')") === false ||
-	strpos($javascript, 'window[callbackName]();') === false) {
+$hasCallbackTypeCheck = preg_match(
+	'/if\s*\(\s*typeof\s+window\s*\[\s*callbackName\s*\]\s*===\s*[\'"]function[\'"]\s*\)/',
+	$javascript
+) === 1;
+$hasCallbackInvocation = preg_match(
+	'/window\s*\[\s*callbackName\s*\]\s*\(/',
+	$javascript
+) === 1;
+if (!$hasCallbackTypeCheck || !$hasCallbackInvocation) {

Conversation

somethingwithproof commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof commented Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

somethingwithproof commented Mar 6, 2026 •

edited

Loading