Skip to content

fix(sql): migrate interpolated queries to prepared statements#50

Closed
somethingwithproof wants to merge 1 commit intoCacti:developfrom
somethingwithproof:fix/prepared-statements
Closed

fix(sql): migrate interpolated queries to prepared statements#50
somethingwithproof wants to merge 1 commit intoCacti:developfrom
somethingwithproof:fix/prepared-statements

Conversation

@somethingwithproof
Copy link
Copy Markdown

Migrate remaining non-prepared SQL queries that interpolate PHP variables to use prepared statement variants (db_execute_prepared, db_fetch_cell_prepared, etc.). Static SQL without variable interpolation is left as-is.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Copilot AI review requested due to automatic review settings April 9, 2026 04:48
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Audit plugin’s poller retention cleanup to avoid SQL string interpolation by using a prepared statement, aligning the plugin with Cacti’s safer DB access patterns.

Changes:

  • Replace interpolated DELETE ... FROM_UNIXTIME(<expr>) query with db_execute_prepared(...) in the daily purge logic.

Comment thread setup.php

if ($retention > 0) {
db_execute('DELETE FROM audit_log WHERE event_time < FROM_UNIXTIME(' . (time() - ($retention * 86400)) . ')');
db_execute_prepared('DELETE FROM audit_log WHERE event_time < FROM_UNIXTIME(?)', array(time() - ($retention * 86400)));
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description says it migrates the remaining interpolated SQL queries to prepared statements, but this file still has variable-interpolated UPDATEs to plugin_config (e.g., SET version='$current' and string-concatenated name/author/webpage updates in audit_check_upgrade()). Either convert those to prepared statements as well, or adjust the PR description/scope so it matches the actual change set.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. Only the one query with variable interpolation was converted. The others are static SQL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants