Multiple Fixes: Range Pruning, Callback Execution, Early Rotation, CSV Export Hardening, Prepared Statements, SQL Readability by TheWitness · Pull Request #313 · Cacti/plugin_syslog

TheWitness · 2026-04-10T16:29:30Z

Fix partitioning so that it happens an hour before the rotation
Fix CSV Export to be secure using built-in function
Fix Use logtime as a timestamp to improve range partition pruning
Fix properly execute the program callback when the value changes on the syslog page

* Fix partitioning so that it happens an hour before the rotation * Fix CSV Export to be secure using built-in function

Also missed a few prepared statements.

* Properly execute the syslog callback without using eval * Use a timestamp to improve the quality of range partition pruning * Better formatting on UNION queries

Copilot

Pull request overview

This PR targets the Cacti Syslog plugin’s data lifecycle and UI hardening: improving partition rotation behavior (incl. DST edge cases), securing CSV exports, and hardening client-side callback execution.

Changes:

Adjusts partition creation timing/logic and updates schema partitioning to use UNIX_TIMESTAMP(logtime) (with UTC-based date calculations).
Reworks CSV export to use fputcsv() for proper CSV escaping.
Removes eval()-based autocomplete callbacks and adds a string-based function dispatcher; adds additional HTML escaping in UI outputs.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`setup.php`	Changes partitioned syslog table definition to partition by `UNIX_TIMESTAMP(logtime)` and tweaks table DDL quoting.
`functions.php`	Updates partition management timing/signatures; switches export output to `fputcsv()`; converts many SQL strings to interpolated backtick form and more prepared statements.
`syslog.php`	Updates various SQL strings to interpolated backtick form and adds HTML escaping in select options; refactors some query formatting.
`syslog_reports.php`	Moves report enable/disable/remove operations to prepared statements; SQL quoting adjustments.
`syslog_removal.php`	SQL quoting adjustments for removal rule queries.
`syslog_alerts.php`	Fixes alert enable query to use `syslog_db_execute_prepared`; SQL quoting adjustments.
`js/functions.js`	Replaces `eval()` with a function-by-name executor; attempts to sanitize host dropdown option rendering.

syslog.php

functions.php

js/functions.js

functions.php

setup.php

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

functions.php

syslog.php

js/functions.js

Follow-up to Cacti#313. Addresses eight correctness and hardening findings: - Partition boundary math is now computed as integer epochs in PHP (next UTC midnight) and injected as numeric literals. strtotime() and UNIX_TIMESTAMP('date-literal') both pulled the PHP/MySQL session TZ into UTC-intended math and caused drift at day boundaries on non-UTC servers. - syslog_partition_create() now hard-fails with an error log instead of silently warning when SHOW CREATE TABLE does not expose either TO_DAYS or UNIX_TIMESTAMP. Silent no-ops caused rotations to stall unnoticed. Rewrote str_contains() to strpos() for portability. - syslog_manage_items() validates $from_table/$to_table against a three-value allowlist before interpolation. Defense-in-depth for the one caller that currently passes safe literals. - Alert, removal, and report rule handlers of type sql are gated on a new 'syslog_allow_sql_rules' setting (off by default). These handlers inline admin-defined SQL into the WHERE clause and cannot be parameterised; the previous removal handler also emitted invalid syntax ("WHERE message (expr)"). Added a Security Settings section to setup.php with a warning description. - CSV export replaces lossy trim($x, ' =+-@') with a syslog_csv_safe() helper that prepends a single quote only when the first character is one of =+-@, TAB, CR. Preserves content verbatim while defusing spreadsheet formula injection per OWASP guidance. - Host autocomplete dropdown builds <option> elements via jQuery's attr()/text() instead of HTML string concatenation wrapped in DOMPurify.sanitize(). DOMPurify does not escape attribute delimiters, so a host containing a double quote could still break out of the class/value attribute. - Autocomplete onChange dispatcher enforces a bare-identifier whitelist and looks the callback up as a direct window[name] property. Rejects dotted paths, arguments, and anything that is not a function, with a console.warn. Server side, the three initSyslogAutocomplete arguments now go through json_encode() instead of single-quote interpolation. - Ported tests/regression/issue254_partition_table_locking_test.php to the new syslog_partition_check/create two-argument signatures and extended it to assert the UTC-midnight boundary arithmetic, the hard-fail on unknown partition expression, and the syslog_manage_items table-name allowlist. Verification: - php -l on every touched file - node -c on js/functions.js - all eight regression tests in tests/regression/ that pass on develop continue to pass; issue254 was restored and passes; three pre-existing failures (issue253, issue269 x2) are unchanged on develop. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Addresses code review feedback on the PR Cacti#313 follow-up. The boundary epoch calculation ((int)($time / 86400) + 1) * 86400 assumes a non-negative UTC timestamp; non-numeric or pre-epoch values would silently underflow. Reject them at function entry with a logged error and a false return, matching the other guard clauses in the function. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

TheWitness added 4 commits April 10, 2026 09:20

fix: Rotate Partition Early - CSV Export

5d28295

* Fix partitioning so that it happens an hour before the rotation * Fix CSV Export to be secure using built-in function

fix: Increase readability of SQL statements

d7eec70

Also missed a few prepared statements.

fix: Error in syslog displaying rows

2ddef96

fix: Multiple fixes

91bcf64

* Properly execute the syslog callback without using eval * Use a timestamp to improve the quality of range partition pruning * Better formatting on UNION queries

TheWitness requested review from bmfmancini, Copilot, somethingwithproof and xmacan April 10, 2026 16:29

Copilot started reviewing on behalf of TheWitness April 10, 2026 16:30 View session

TheWitness changed the title ~~Partition export~~ Multiple Fixes: Range Pruning, Callback Execution, Early Rotation, CSV Export Hardening, Prepared Statements, SQL Readability Apr 10, 2026

Copilot AI reviewed Apr 10, 2026

View reviewed changes

xmacan previously approved these changes Apr 10, 2026

View reviewed changes

fix: Copilot reviews

29114fb

TheWitness dismissed xmacan’s stale review via 29114fb April 10, 2026 16:42

TheWitness added 4 commits April 10, 2026 12:43

fixes: Copilot reviews

12e8a37

fix: Make partition reorganization work with TO_DAYS legacy

18aa3f4

fix: Copilot reviews

210dd0f

fix: Missed DOMPurify fix

6278cf9

TheWitness requested review from Copilot and xmacan April 10, 2026 17:11

Copilot started reviewing on behalf of TheWitness April 10, 2026 17:12 View session

TheWitness mentioned this pull request Apr 10, 2026

fix(security): comprehensive bug and security cleanup #302

Closed

6 tasks

Copilot AI reviewed Apr 10, 2026

View reviewed changes

functions.php Outdated Show resolved Hide resolved

functions.php Show resolved Hide resolved

functions.php Show resolved Hide resolved

syslog.php Show resolved Hide resolved

js/functions.js Show resolved Hide resolved

xmacan previously approved these changes Apr 10, 2026

View reviewed changes

fix: More copilot comments

495c2e3

TheWitness dismissed xmacan’s stale review via 495c2e3 April 10, 2026 17:23

fix: Copilot review

a58d099

xmacan approved these changes Apr 10, 2026

View reviewed changes

TheWitness merged commit bb45b9b into develop Apr 10, 2026
8 checks passed

TheWitness deleted the partition-export branch April 10, 2026 17:37

This was referenced Apr 10, 2026

Security: defense-in-depth audit findings (51 items) #304

Closed

hardening: escape device hostname output in syslog view; parameterize alert API functions #252

Closed

This was referenced Apr 10, 2026

Follow-up to PR #313: partition correctness, CSV/XSS hardening, SQL rule gating #314

Open

DRAFT: fix(security): PR #313 follow-up - partition correctness, CSV/XSS hardening #315

Draft

TheWitness mentioned this pull request Apr 10, 2026

hardening: parameterize domain stripping update in syslog_strip_incoming_domains #261

Closed

somethingwithproof mentioned this pull request Apr 10, 2026

Cacti Syslog v2 — Design Principles (discussion / tracking) #317

Open

bmfmancini approved these changes Apr 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multiple Fixes: Range Pruning, Callback Execution, Early Rotation, CSV Export Hardening, Prepared Statements, SQL Readability#313

Multiple Fixes: Range Pruning, Callback Execution, Early Rotation, CSV Export Hardening, Prepared Statements, SQL Readability#313
TheWitness merged 11 commits intodevelopfrom
partition-export

TheWitness commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

TheWitness commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants