[somethingwithproof]

Description

Fixes found in a review of the plugin, with end-to-end coverage for every changed path. The branch contains security-relevant changes: the Query Tool no longer stores or redisplays the password it is given and now runs its query only for POST requests, and account saving validates its input before writing. No fix here is a pre-authentication issue; each needs a logged-in user or access to the Cacti server.

The commits are independent and can be reviewed one at a time. Each fix has its own regression test and CHANGELOG line.

Related Issue

No existing issue covers these. Open issue #15 (Windows hosts) is not touched.

Motivation and Context

Per-finding summary:

• Stale collector locks (poller_wmi.php, includes/functions.php): a collector that died left its wmi_processes row behind, so the master poller waited forever and every later run exited with "still running". Rows older than one hour are now deleted at start, and the master stops waiting for its children after one hour.
• Cache purge (run_store_wmi_query): rows were flagged present = 0 on every poll, but the delete that follows only ran when the host did not exist. Rows for instances that disappeared were never removed, and a failed query left every row flagged. The flag, insert and delete now happen only after a successful refresh.
• Multi-column primary key: a comma separated key never matched a column, so every row got the index N/A and only the last row survived. The index is now built from every listed column.
• Failure reason: the reason a query failed was printed to a discarded stdout. It is now written to the Cacti log.
• Namespace: the default root\\CIMV2 is single-quoted for wmic and arrived with two backslashes. Doubled backslashes are collapsed to one.
• Query Tool password (security): the password typed into the tool was stored in the session filter store and written back into the form. It is no longer stored or redisplayed.
• Query Tool method (security): wmi_tools.php?action=query ran for a GET request, so credentials could travel in a URL, an access log or the browser history, and the CSRF token did not apply. The query action now runs only for POST; any other method gets the empty form and runs nothing. The Run button already submits by POST, so the form works as before.
• Pagers: the Queries page counted only the current page and the Accounts page ignored its search filter. Both count matching rows.
• Account save: a mismatched or missing password, an empty name or username, or a password too long for the column still reached sql_save. The input is validated first.
• Remove-query dialog: the form action was missing = in &id=.
• Breadcrumbs: the Edit Account and Query Tool breadcrumbs pointed at a parent that does not exist, which made the account edit page fail on Cacti develop. They now point at the page's real parent.
• Poller scheduling: last_started was written from PHP's clock and compared with the database's NOW(). When PHP and the database session use different time zones, devices were polled again on every cycle (or delayed by the offset). The start time now comes from the database clock. The CI job runs with America/New_York for PHP and a UTC database, which exposed it.
• Concurrency default: when the wmi_processes setting was empty the master used its default only for the stored setting, not for its launch loop, and waited forever. It now uses the default.

Not changed, and why:

• Password on the wmic command line (--password=): visible to local users in ps. The repository gives no confirmed wmic option to pass credentials another way, so this needs a decision (an authentication file if the installed wmic supports one, or documenting the exposure). The PowerShell transport already avoids it.
• Script-server paths (wmi_script.php, script/wmi-script.php, the Exchange resource XML, the path_wmi setting): the intended install location of the scripts is not clear from the repository, so the paths and the setting are left for the maintainers to decide.
• INFO compat and version: the right floor and release number are not settled by the README, CHANGELOG and tests.
• wmic stderr is not captured, and a multi-row query whose primary key is None still shares one index. Both need a product decision.

How Has This Been Tested?

Environment: PHP 8.4.27 with Xdebug in Docker, MariaDB 11.8.9 in a throwaway container, Cacti 1.2.x (1.2.32) and Cacti develop (1.3.0) checkouts with composer install as the workflow does it. The install state is simulated where noted below.

Test layers:

• Unit tests with the existing stubs, now with the same signatures as Cacti core.
• End-to-end tests under tests/E2E, which need an installed Cacti and its database (the CI workflow provides both and sets WMI_E2E=1 on the Pest step; elsewhere they skip with a reason):
  • CollectorFlowTest runs the cache refresh and the real poller_wmi.php master and child against the database with a fake wmic: first refresh, dropped instances, failures (partial output, empty output, silent non-zero exit, header only), single, duplicate, None, composite and unknown primary keys, namespace forms, missing account, query or host, a live lock, a stale lock, an empty or missing concurrency setting, a child that cannot start (the master is killed by a timeout, the lock survives, and a later run recovers once the lock is old), down and disabled devices, a disabled plugin, a child run alone, and bad command lines.
  • PagesFlowTest sends real HTTP requests to PHP's built-in server serving Cacti, logged in as real users: anonymous and no-WMI-realm refusals, account create, edit and reject cases (mismatch, empty password, empty name or username, over-long password, 64-character boundary), no CSRF token, GET, pager totals with filters across pages, bulk delete, query create and edit, the Query Tool (rows, escaped output, missing fields, failed host, namespace forms, password submitted twice never stored or echoed, no CSRF, no realm, GET refused), and the Device Template hook.
  • EntryScriptsTest checks that every helper under tests/E2E does nothing over the web.
• Entry points that cannot be loaded in the unit process are measured by running them for real: tests/E2E/coverage_prepend.php records executed lines when WMI_E2E_COV names a directory, and tests/bin/e2e-coverage-report.php merges that with the unit Clover report and lists each changed line with the suites that ran it. Every changed executable line ran in at least one suite.

Executed locally:

• php -l on every changed PHP file, tests/bin/validate-manifest.php, tests/bin/check-i18n-pot.php (no string changes): pass.
• Pest on the develop tree, without the database: 94 passed, 64 skipped (63 are the E2E datasets and one is the existing permission test that skips as root) passed.
• Pest on the develop tree with the database and WMI_E2E=1: 157 passed, 608 assertions passed.
• CI-order simulation (checkout, composer install as the workflow does it, the pinned Pest harness for 1.2.x, install_cacti, plugin_manage --install --enable on a bare Cacti with MariaDB and no other plugin, the log-writable step, then Pest as a non-root user with WMI_E2E=1, then the workflow's log grep): 157 passed, 1 warning (the existing PruneFilesTest unlink warning), 609 assertions passed on both Cacti 1.2.x and develop, including with PHP set to America/New_York, Asia/Kolkata and UTC and with eight busy containers competing for the CPU. The log grep found nothing.
• php tests/bin/patch-coverage.php <clover> origin/develop 100: 100.00 percent (32 of 32 changed measured lines); tests/bin/e2e-coverage-report.php reports every changed executable line in the four unmeasured entry points as executed.
• The new tests were run against the code before each fix and failed, then passed after it; the E2E suite on origin/develop failed 5 of 8 of its first tests, and the Query Tool GET test and the time zone test fail without their fixes.

The install check also covers the "plain Cacti" requirement: INFO, the install hooks and the schema are unchanged, and nothing here adds a requires entry or a dependency.

Whole-file line coverage of the changed files, before (origin/develop, unit tests only) and after (unit plus end-to-end):

File	Before	After
includes/functions.php	0.0%	41.9%
includes/linux_wmi.php	45.8%	54.5%
setup.php	46.4%	73.8%
poller_wmi.php	not measured	89.8%
wmi_accounts.php	not measured	93.9%
wmi_queries.php	not measured	94.1%
wmi_tools.php	not measured	46.7%

Not run: the GitHub workflow itself, PHP 8.2 and 8.3 locally, the Cacti installer to completion (the image lacks extensions it checks for, so the install state was finished by hand after plugin_manage ran for real), PHPStan and the coding-standards jobs beyond what origin/develop already reports, Sonar duplication, a wmic that hangs, a real Windows host or wmic binary, and the pages in a browser. The one-hour lock deadline is covered by unit tests only.

Performance

Fixture: a fake wmic printing 3,000 and 10,000 rows of six columns (18,000 and 60,000 cache rows), one host, one query. Environment: Docker on a laptop, PHP 8.4 with Xdebug loaded, MariaDB 11.8.9 in a second container. Median of 5 runs per case, same fixture for origin/develop and this branch. Query counts are the difference in the session Questions counter.

Cache refresh, unchanged data (steady state), 10,000 rows (two runs of each):

	wall ms	CPU ms	queries	peak MB
develop	1669, 1566	140, 145	304	28
branch	1498, 1572	151, 161	305	28

The refresh is within run-to-run noise. The branch issues one more query (the delete now runs on every successful refresh). At 3,000 rows: 525 ms vs 520 ms, 94 vs 95 queries.

Cache refresh when wmic fails, 10,000 rows: develop 35 ms and 4 queries, leaving 60,000 of 60,000 rows flagged present = 0; branch 1 ms and 3 queries, leaving 0 flagged. This is a correctness change; the time saved is the skipped update of the whole table.

List pager: the Queries page gains one COUNT(*) with the search filter. On 20,000 rows with a filter matching 10 percent it took about 2.0 ms against about 2.7 ms for the page query itself. The Accounts page replaces its unfiltered count with a filtered one at the same cost.

Poller start time: one extra SELECT NOW() per device run. Collector wait loop and stale-lock delete: not benchmarked. By reading, the master issues one extra DELETE per run, and the loop issues the same one SELECT COUNT(*) per two-second iteration as before. Not measured: web request timings of the three pages, memory of the full master run, the cost under many devices, or cachegrind profiles.

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.
• I have updated the documentation repository accordingly.

Rollback: each commit reverts cleanly on its own. No schema change. The collector lock timeout is a function default (3600 seconds) in includes/functions.php. Operators who relied on wmi_processes rows lasting longer than one hour will see them expire. A script that called wmi_tools.php?action=query by GET will now get the form instead; the Run button is unaffected.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud