[somethingwithproof]

fix: harden the import, the views and the schema upgrade (2.7)

Description

This PR fixes a set of defects found in a read-through of the whole plugin. It contains security-relevant fixes: a SQL injection in the import path, an unescaped value in the Details filter, and a delete confirmation whose No button deleted the selected logs. The first two need a signed-in user who holds the Slowlog realm; none is reachable without authentication. The details are below so reviewers can judge severity themselves.

The change is 42 small commits, one logical change each, so it can be reviewed and reverted commit by commit. The plugin version goes from 2.6 to 2.7 so the column widening in includes/database.php runs once through the existing version-drift upgrade path.

Related issue

No open issue covers these findings. The nearest are #14 (security audit) and #11 (prepared statements); this PR does not claim to close either.

Motivation and context

Per finding. "Needs" says who can trigger it.

#	Area	Problem	Fix	Needs
1	import	The numeric header fields (Query_time, Lock_time, Rows_*, Bytes_sent, Thread_id) were concatenated unquoted into the bulk INSERT. A value such as (SELECT ...) ran as SQL with Cacti's database account.	slowlog_log_number() keeps plain decimals and stores 0 for anything else.	Slowlog realm, upload
2	Details filter	The table name was written unescaped into <option value="...">. Names come from tokenized slow log SQL, so a quoted identifier could break out of the attribute.	html_escape() on the value.	Slowlog realm, a query in an imported log
3	delete confirmation	No and Return were submit buttons carrying the same selected_items field as Yes, so No deleted the selected logs.	Both are plain cactiReturnTo buttons, as in Cacti core.	Slowlog realm
4	import	strpos() header matching: a query containing SET timestamp= was read as a header and dropped.	Match header lines only at the start of the line.	any log
5	import	The User@Host line was split on every colon: MySQL's Id: suffix ended up in the address and IPv6 addresses were lost.	slowlog_parse_user_host() with one pattern.	MySQL 5.7/8 or IPv6 logs
6	table detection	Queries are stored with line breaks collapsed, so a trailing -- or # comment hid the rest of the statement from table detection.	Detect from the original text (oquery), falling back to query.	multi-line queries with comments
7	table detection	get_table_associations() read every column of every row of a log in one SELECT.	2000-row chunks, only logentry, query, oquery.	large logs
8	import	substr() could cut a multibyte character; a Max Query Length of 0 or less stored empty or truncated queries.	slowlog_truncate_query(): character boundary, ignores values below 1.	length option
9	import	The result of the bulk INSERT was ignored, so a failed batch (for example one over max_allowed_packet) still finished as a complete import.	The import stops and is marked failed (status 3, "Import Failed - Row Insert Error").	server side failure
10	import	The Tables of Interest list was split on a single space only.	Split on any whitespace.	pasted lists
11	import	SHOW TABLES FROM <db> with an unquoted name failed for names such as my-db.	Quote the name.	such databases
12	schema	user/schema 20, ip_address 15, table names 45, import_text_status 40 are shorter than real data. Cacti's session clears strict SQL mode, so the values were silently truncated and distinct users or tables merged (in strict mode the INSERT fails).	Widen to 128/64/45, 64, 128. New installs get them from the table definitions; existing installs are altered once on the 2.6 to 2.7 upgrade (slowlog_widen_columns(), db_update_table()).	any
13	upgrade	plugin_config.version was written before the schema upgrade ran, so a failed ALTER was never retried.	Written after slowlog_upgrade_tables() succeeds; failure is logged and retried on the next page load.	upgrade
14	tab	The tab read user_auth_realm directly (missing group grants) and cached the answer in the session.	is_realm_allowed(); only the realm id is cached.	group-granted users
15	Details list	rows and page were validated only as integers: a huge page size loaded every row, including both query columns, into memory; a negative value was a SQL error.	slowlog_page_bounds(): 1 to 1000 rows (30 when not positive), page at least 1.	any viewer
16	stats	Past 20000 entries per scope the values are sampled, so "Include Max" showed the sample's maximum.	Keep the exact min and max in the running totals.	large logs
17	filters	Filter values were appended to the request URL without encodeURIComponent, so &, # or + truncated them.	Encode them.	any viewer
18	import	Entry times were formatted with PHP's date() in PHP's time zone and written into a timestamp column that the database reads in its own session zone, so with PHP and the database in different zones every stored time was shifted (5 hours with PHP in America/New_York and a UTC database). Digit-padded MariaDB # Time: headers also lost the time of day.	The epoch from a SET timestamp= line is stored through FROM_UNIXTIME(); a MySQL 8 ISO # Time: with Z or an offset is parsed with its zone; padded headers keep the time of day; out-of-range values are clamped. TimeExpressionTest runs under four PHP time zones.	any import

Not in this PR (needs a decision or is larger than a fix):

• A separate realm for upload and delete. Viewing, uploading and deleting share one realm today. Splitting it changes what existing users can do. Options: (a) add a second realm and grant it on upgrade to every user and group that holds the current one, so nothing changes until an admin narrows it; (b) add it without a grant and accept that upload and delete stop working until it is assigned; (c) leave as is and document it. I would take (a).
• memory_limit is still set to -1 for every slowlog.php request. The upload form describes that override to the user. With the page size bounded, the view that could use unbounded memory is limited; removing the override means changing that screen and its warnings too.

How has this been tested?

• New and changed unit, regression and integration tests under tests/Unit, tests/Integration and tests/Security, using the repo's stubbed-database bootstrap. Run against the pre-change source (main), 48 of them fail, covering each finding above; all pass on this branch.
• End-to-end tests in tests/E2E run against an installed Cacti and a real MariaDB:
  • ImportFlowTest: imports a sample log through import_log.php, checks the rows, the tables and stats, the failed-insert path (a trigger stands in for a server side failure), the multibyte length cap, and the 2.6 to 2.7 upgrade of an old schema. The upgrade runs against a scratch copy of the plugin directory, because the upgrade prunes tests/ from the directory it runs in; the test also checks that the real tree is intact.
  • DeleteConfirmationFlowTest: signs in over HTTP on PHP's built-in server, selects two logs, opens the confirmation page, checks that No is a plain button and nothing is deleted, confirms with Yes, then revokes the realm and checks the page is denied.
  • DriverGuardTest: the driver scripts under tests/E2E do nothing (exit 404 / print nothing) unless started from the command line with SLOWLOG_E2E=1 and a temp directory from the environment. They take no path from a request.
  • The CI integration job sets SLOWLOG_E2E=1 on the Pest step; without it the E2E tests are skipped. A new step before it makes cacti/log writable for the runner user (the sudo install leaves cacti.log root-owned and Cacti's CLI bootstrap refuses to start then); the tests skip with that reason if the log is still not writable, and cut cacti.log back to its earlier length after each driver. The drivers move Cacti's log file to a temp directory while they run, so the deliberate failures do not reach the job's final scan of cacti.log.
• tests/bin/patch-coverage.php against main: 100 percent of the 131 changed measured lines (slowlog.php is allowlisted there, as before; its changes are covered by source-shape tests and the E2E tests).
• tests/bin/check-i18n-pot.php main passes (one new string added to locales/po/cacti.pot); tests/bin/validate-manifest.php passes.
• Install check on a plain Cacti (no other plugins, MariaDB 11.8): plugin_manage.php --plugin=slowlog --install --enable succeeds and creates the 8 tables; no requires entry or dependency was added.

What I could not run locally: the CI matrix itself (Cacti 1.2.x with PHP 8.2 to 8.4). Local runs used a Cacti 1.3 development tree and PHP 8.4. The E2E drivers rely on the 1.2.x login form and user_auth columns being the same as in that tree; if they differ the CI run will show it.

Performance

Fixture: a synthetic MariaDB-style slow log of 200,000 entries (75 MB, 30 table names, joins, multi-line queries with comments), imported with import_logfile() in table mode "all" (inline post-processing), producing 200,000 detail rows and 266,667 table associations. 3 runs per case, alternating main and this branch, medians reported (run values in brackets). Environment: PHP 8.4.27 CLI (Xdebug module loaded, no coverage mode) in one Docker container, MariaDB 11.8.9 in another on the same host, a laptop running other containers at the same time, so timings are noisy at the few-percent level.

Full import (parse, insert, method and table classification, stats cache):

	main	this branch
wall time	51.0 s (51.6, 50.5, 51.0)	53.0 s (53.1, 50.9, 53.0)
PHP CPU time	12.7 s	13.0 s
peak PHP memory	480 MB (RSS 497 MB)	64 MB (RSS 81 MB)
SQL statements	1632	1798

get_table_associations() alone on the same log:

	main	this branch
wall time	10.6 s (10.8, 10.6, 9.7)	10.4 s (10.8, 10.4, 10.2)
PHP CPU time	5.9 s	5.8 s
peak PHP memory	450 MB (RSS 469 MB)	14 MB (RSS 38 MB)
SQL statements	540	706

What this shows: peak memory no longer grows with the size of the log (about 7 times lower for the whole import and 30 times lower for table association on this fixture). Wall and CPU time did not change measurably; the import is 4 percent slower in the median, which is inside the spread of the individual runs, so I am not claiming either direction. The cost is 166 more statements (about 10 percent for the import): one SELECT per 2000 rows plus a smaller bulk INSERT at the end of each chunk.

Not measured: database server CPU and time, the Details and chart views (the page bound adds one function call and removes the unbounded case; no timing taken), the tab check (one is_realm_allowed() call per page instead of a cached session value), the 2.6 to 2.7 ALTER TABLE on a large plugin_slowlog_details table, and Xdebug cachegrind profiles.

Risk

• Upgrade: on the first page load after upgrading to 2.7, db_update_table() alters plugin_slowlog_details (user, ip_address, schema) and four small ALTER TABLE ... MODIFY COLUMN statements widen the other columns. On a very large details table the first alter rebuilds the table; do it in a quiet period. If any step fails, the recorded version stays at 2.6, a warning is written to the Cacti log, and the upgrade is retried on the next load of plugins.php or slowlog.php.
• Behaviour changes: an import now stops at the first failed INSERT (before, it continued and reported success); rows with a non-numeric header field store 0 instead of whatever the server evaluated; table detection reads the original query text, so results for queries with trailing comments can differ from earlier imports (re-run import_log.php --reprocess=N to refresh an existing log).
• Compatibility: the compat floor in INFO is unchanged (1.2.29). The new code uses features available in PHP 7.1 and later; mb_strcut() is used only when mbstring is loaded.

Rollback

Revert the commits or the merge. Nothing needs undoing in the database: the widened columns hold the same data and are compatible with the 2.6 code. If plugin_config.version already says 2.7 after a rollback, run the plugin's upgrade or set it back to 2.6 in the Plugin Management page; no schema step depends on it.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation. (CHANGELOG updated; no other documentation change)
• I have updated the documentation repository accordingly.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.4% Duplication on New Code

See analysis details on SonarQube Cloud