* PHPStan Level 8 - Phase 1 Conversion
Reduces the real (no-baseline) PHPStan level 8 error count from ~2,724 to 425
and regenerates phpstan-baseline.neon accordingly (8,941 -> 1,921 lines).
Core type-safety changes
- lib/database.php: narrow db_fetch_row()/db_fetch_row_prepared() to return
array (was bool|array) and db_fetch_assoc()/db_fetch_assoc_prepared() to
return array (was mixed); the _prepared workers coerce the connection-failure
path to an empty array so the declared type is honest. empty()/cacti_sizeof()
checks are unaffected.
- lib/database.php: narrow $db_conn guards from is_object() to instanceof PDO in
the transaction/quote/insert-id helpers so the PDO calls resolve.
- lib/mib_cache.php: the fluent MibCache methods mib()/object()/table()/row()
now return self instead of a bare object so chained calls type-check.
Type-safety casts and guards (mechanical, behaviour-preserving)
- Cast string|false/|null builtin results feeding string parameters
(json_decode, base64_encode, trim, htmle, preg_replace, explode, etc.).
- Cast int|string / int|false values feeding int id/pid parameters.
- Guard resource|false fopen()/popen() handles before fwrite/fclose/fread, and
wrap foreach over array|false/list|false results with `?: []`.
Genuine bugs found and fixed
- cli/add_tree.php / lib/api_tree.php: site tree nodes could not be created via
the CLI - the --site-id option was never parsed, $nodeTypes had no 'site'
entry (passing null to api_tree_item_save(int $type) -> TypeError), and
api_tree_item_save() never persisted site_id. All three fixed.
- tree.php: leaves_exist() is typed : int but returned db_fetch_assoc_prepared()
(an array of rows) for a SELECT COUNT(*); now uses db_fetch_cell_prepared()
and returns the scalar count.
- lib/mib_cache.php: $entries_per_object was a float (division / ceil) used as
an array key (PHP 8.1 implicit float-to-int key deprecation); now cast to int.
Config
- .phpstan.neon: add notIdentical.alwaysTrue and booleanAnd.alwaysFalse to the
existing always-true/false ignore family (same "PHPStan gets confused"
dynamic-constant cause already covered by identical.alwaysTrue,
notEqual.alwaysTrue, booleanAnd.leftAlwaysTrue, etc.).
- phpstan-baseline.neon: regenerated; the remaining 425 entries are the
untouched long tail for Phase 2.
* Remove redundant @phpstan-ignore comments
Follow-up cleanup for the level 8 Phase 1 work. Removes 14 inline
@phpstan-ignore comments that no longer suppress any reported error:
- cli/repair_database.php: `@phpstan-ignore booleanAnd.alwaysFalse` is now
covered by the global booleanAnd.alwaysFalse ignore added to .phpstan.neon.
- 13 others (host.php, include/global.php, include/session.php,
lib/api_automation.php x2, lib/functions.php x2, lib/html_utility.php x2,
lib/installer.php, lib/package.php, lib/reports.php, lib/rrd.php) no longer
match any reported error after the type-safety fixes (e.g. the
db_fetch_row_prepared() return-type narrowing removed the offset-assign
errors the api_automation ignores were suppressing).
Verified with reportUnmatchedIgnoredErrors enabled: 0 unmatched ignores
remain and the full level 8 run still reports 0 errors.
* ci: satisfy validator ratchet and refresh security sink baselines
- lib/api_tree.php: give the new tree-item site_id a real numeric
constraint ('^[-0-9]+$', matching html_reports.php) instead of an
empty regex, so the validator constraint ratchet passes without
bumping its baseline.
- tests/security/baselines: refresh the three sink/hotspot signatures
whose code text changed from the (string) casts added in this branch
(lib/poller.php setsid, cli/rrdresize.php json_encode, lib/import.php
simplexml_load_string).
* ci(js): allow DOMPurify 3.4.16 and update sync_js pin test
The dependabot bump (#8134) moved package.json/package-lock.json to
DOMPurify 3.4.16 on develop but left tests/js/sync_js.test.mjs pinned to
3.4.15, so the PR merge commit fails the 'DOMPurify is pinned' contract.
Bring package.json/package-lock.json to 3.4.16 (matching develop) and
update the pin test to expect 3.4.16.
* test: update sql.php null-handling assertion for the (string) cast
This branch changed scripts/sql.php from trim(\ ?? '') to
trim((string) \) (the phpstan-safe form that coerces both null and
false from shell_exec()). Update SqlScriptsTest's null-return assertion
to match the new cast instead of the removed '?? ''.
* fix: address Copilot review - ownership IDs, fopen guards, mib_cache bound
- chown()/chgrp(): pass integer UID/GID from fileowner()/filegroup()
(guarding the false lookup) instead of string-casting them, which made
them resolve account/group names like '1001' and fail. Covers
lib/api_device.php, lib/boost.php and poller_maintenance.php (x2).
- fopen() failure now short-circuits instead of reporting success:
lib/package.php returns false, lib/clog_webapi.php and utilities.php
render an error, and snmpagent_mibcachechild.php skips the cache write
and sentinel unlink so a concurrent writer's lock is preserved.
- lib/mib_cache.php: keep the float quotient as the loop bound and cast
only the array offset, so a non-even entries/columns split no longer
truncates the loop and drops a cached row.
* revert: keep db_fetch_* returning false on error; baseline Level 8 instead
Per review, restoring the intentional fail-safe: db_fetch_row[_prepared]()
return bool|array and db_fetch_assoc[_prepared]() return mixed again, so a
genuine SQL error (not just a missing connection) is signalled as false and
the === false checks in cdef_is_in_use(), boost_process_* and aggregate.php
work as before. Dropped the is_array() ? : [] coercion.
Level 8 stays green by regenerating phpstan-baseline.neon to capture the
re-introduced bool|array/mixed call-site errors (the ~1,850 the narrowing
had eliminated). Tightening those callers to drop them from the baseline is
Phase 2 work.
---------
Co-authored-by: TheWitness <copilot@users.noreply.github.com>
* fix(win32): harden the cmd.exe shell sinks on develop (GHSA-rjvj-r52f-8v5q)
Forward-port of the 1.2.x win32 command-injection remediation. On Windows the
SNMP/ping/data-query/trap commands run through cmd.exe, which ignores the \"
escape and toggles quoting on every ", so cacti_escapeshellarg cannot stop
& | ^ < > ( ).
- add cacti_escapeshellarg_cmd(), stripping those operators on win32 before
quoting (a Unix pass-through)
- snmp_escape_string(): strip the operators in the win32 branch
- route the SNMP binary hostname, the data-query device values
(get_script_query_path), the data-input arg builders, the Windows ping
hostname, and the SNMP trap notification arguments through the strip
get_script_query_path is device-reachable (a rogue monitored device supplies an
SNMP index/system field that executes on the Windows poller); the rest are
post-auth.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix(win32): strip %VAR% from the hostname on Windows (GHSA-rjvj-r52f-8v5q)
cmd.exe expands %VAR% even inside quotes, so a device hostname of %USERNAME%
would expand into the SNMP/ping command line. Add an opt-in $strip_env flag to
cacti_escapeshellarg_cmd() and set it for the SNMP binary and ping hostnames,
which never legitimately contain a percent. Community and credential values are
left untouched because they can legitimately contain %.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* docs(changelog): note win32 cmd.exe hardening for develop
* security: require Package Management permission to trust new package signers
package_accept_key() (reachable via the repository-based package import flow)
inserted new keys into package_public_keys, the global trust store consulted
by import_validate_signature() for every future import by every user,
without checking for the Package Management (realm 29) permission that
gates package_keys.php/package.php/package_repos.php for the same table.
An Import Templates (realm 17) user could therefore add themselves as a
trusted signer for the whole install. Also add the 'accept' action to the
CSRF-protected bad_actions list so the acceptance step requires POST plus a
valid token like every other state-changing action.
GHSA-gxgm-8rhv-r7jv
* docs(changelog): note package-signer trust authorization fix
* security: close tab-based open redirect bypass in CSRF timeout handler
sanitize_uri()'s guard (20298c6cd) models the WHATWG rule that browsers
strip leading C0 controls/whitespace before resolving a redirect, but
Chromium separately strips an embedded tab from anywhere in the target.
A request target of /%09/evil.example passes the guard (no leading control
char, not two leading slashes) but Chromium reduces the resulting
'Location: /<TAB>/evil.example' header to a network-path reference and
navigates off-site. 1.2.x already uses validate_redirect_url() for this
same sink (3338a63e1), which resolves the URL via parse_url() and checks
host/port against the server-configured values, so it isn't bypassable this
way. Switch develop's CSRF timeout handler to match.
GHSA-c725-4jm5-9fx5
* security: authorize single-item Report actions against ownership
reports.php sets \ = true, so include/auth.php grants an
implicit guest session (when the Guest User feature is enabled) without
ever evaluating the realm 22 gate declared for this page. The single-item
action handlers reports_item_remove()/_moveup()/_movedown() relied solely
on that page-level gate and performed no independent ownership check, so an
unauthenticated guest-bound request could delete or reorder reports_items
rows belonging to any report, including ones owned by an administrator.
reports_form_actions() already gained an equivalent \
check; add the same authorization (Reports admin realm 21, or the
requesting user owns the item's report) to the three single-item handlers.
Same underlying chain as the 1.2.31 fix for reports_user.php (guest bind
removed + cacti_authorize_resource() added to reports_item_remove()); the
rewritten develop reports.php/lib/html_reports.php did not carry either
half of that fix forward.
GHSA-j5jh-fhwg-r56x
* security: authorize tree node CRUD actions against per-tree access
tree.php dispatches create_node/delete_node/move_node/rename_node/copy_node
directly to api_tree_*() with no is_tree_allowed() check, even though the
normal tree-view path already enforces that boundary. A non-admin user with
the Trees realm but no explicit grant on a given tree_id could still create,
delete, move, rename or copy nodes on that tree by calling these actions
directly, bypassing the authorization boundary enforced elsewhere.
GHSA-prxj-84qq-v5jq
* security: remove shell from Realtime RRD create and validate DS bounds
poller_realtime.php built the Realtime-graph RRD 'create' command line
(including the SNMP-sourced ifSpeed/ifHighSpeed interface speed, substituted
in verbatim as the DS maximum) and ran it through shell_exec(). A hostile or
spoofed monitored device could return an ifSpeed value such as
'1000000000\uid=197608(ladams) gid=197121 groups=197121' and get arbitrary OS command execution as the web/poller
user the next time any user with the Realtime Graphs permission (or an
unauthenticated guest, where configured) viewed a realtime graph for an
affected data source.
- lib/rrd.php: rrdtool_function_interface_speed() now casts ifHighSpeed/
ifSpeed to int before they become the DS maximum.
- lib/rrd.php: rrdtool_function_create() now rejects any non-numeric
rrd_minimum/rrd_maximum right before they are concatenated onto the
rrdtool command line, forcing them to 'U' and logging instead.
- poller_realtime.php: the RRD create is now routed through the same
shell-free rrdtool_execute()/proc_open(bypass_shell) path every other
RRDtool invocation already uses, matching the pattern 1.2.x already uses
for this exact call site, instead of shell_exec().
GHSA-f88m-q789-g22j
* security: authorize Report save/edit against report ownership
reports_form_save()'s update path fetched the existing report's owner from
the database but never compared it to the session user before calling
sql_save(), so any user holding only the Reports realm could overwrite
another user's report by POSTing its id. reports_edit() had the same gap on
the read side: it fetched a report by id with no ownership check, so the
same user could view any other user's report configuration (recipients,
schedule, etc.). 1.2.x already guards both call sites with
cacti_authorize_resource(..., 'reports'); develop had lost that check.
GHSA-8f5c-3h59-q7gc
* security: reject ORDER BY sort_column subquery bypass
sanitize_sql_column()'s allowlist regex kept parentheses to support the
INET_ATON(col)/NATURAL_SORT_KEY(col) sort wrappers, but that also let
MySQL subquery syntax through unchanged, since a subquery like
(SELECT(0)FROM(SELECT(SLEEP(5)))a) needs only alphanumerics and
parentheses. get_order_string() then detects the '(' and disables its
backtick quoting, so the payload reaches ORDER BY verbatim, giving any
user with realm 2-5 a blind, time-based SQL injection via sort_column.
sanitize_sql_column() now only allows '(' through when the whole value is
exactly one of the two known safe wrapper forms around a plain identifier;
anything else containing '(' -- including subquery syntax -- now falls
back to the default. Added regression tests covering the reported
subquery and REGEXP-exfiltration payloads.
GHSA-8j73-cvf6-jrvx
* security: confine Reports format_file to a bare filename
reports_form_save() wrote the POST-supplied format_file value verbatim
into reports.format_file, and reports_load_format_file() concatenated it
onto CACTI_PATH_FORMATS with no directory-component check, so a value
such as ../../../../etc/passwd escaped the formats directory and any
Reports-realm user could read arbitrary files readable by the web
server. 1.2.x already fixed the same sink (GHSA-g37j-39f4-6r4j /
CVE-2026-41884, GHSA-mjvw-mhj5-9jcj / CVE-2026-40084); develop had lost
the guard. format_file is now confined to basename() at both the save
site and the load site, with the load site additionally rejecting a
bare '.'/'..'/empty result before any file IO. Added regression tests.
GHSA-5x27-wqpm-m24v
* security: authorize Data Source enable/disable against owning device
ds_enable()/ds_disable() and the bulk drp_action enable/disable loops in
form_actions() passed a user-controlled local_data_id straight into
api_data_source_enable()/api_data_source_disable() with no object-level
authorization check, so any user holding the Sites/Devices/Data realm
could toggle the enabled state of a data source belonging to a device
outside their assigned Device Permissions.
Added data_source_authorized(\), which allows
host-independent data sources (data_local.host_id = 0) and otherwise
defers to is_device_allowed(\), and applied it to both the
single-item and bulk action paths.
GHSA-2fhv-c794-v433
* security: authorize bulk Change Data Source Profile against owning device
The bulk 'Change Data Source Profile' action (drp_action == '6') iterated
selected_items and updated data_template_data/data_template_rrd/poller_item
for each supplied local_data_id with no object-level authorization check,
so any Sites/Devices/Data realm user could retarget the RRD step/heartbeat
of a data source belonging to a device outside their assigned Device
Permissions -- the same class of gap as GHSA-2fhv, in a different bulk
action. Guarded with the data_source_authorized() helper added for that
fix. This action does not exist on 1.2.x (develop-only feature).
GHSA-fc82-w6xq-4jg7
* security: grant AllowOverride All in docker/apache.conf
The packaged docker/apache.conf granted only 'AllowOverride FileInfo
Options=FollowSymLinks' for the Cacti document root. Cacti's shipped
.htaccess deny rules (log/, cache/*, scripts/, cli/, bin/, mibs/, rra/,
contrib/, tests/) rely on 'Require all denied' (Apache 2.4, override
type AuthConfig) and/or legacy 'Order/Deny' (override type Limit),
neither of which FileInfo/Options covers, so Apache silently ignored
every one of those deny rules and served the directories over HTTP to
any unauthenticated client -- most notably log/cacti.log, which can
contain usernames, client IPs, SQL backtraces, and any secrets Cacti
logs (SNMP community strings, LDAP bind details).
AllowOverride All lets every shipped .htaccess apply as intended. Added
a regression test. docker/ does not exist on 1.2.x (develop-only).
GHSA-fxfg-j995-j69j
* docs: add security advisory / CVE policy to Copilot instructions
Documents the policy established for this remediation effort: develop is
unreleased, so a develop-only vulnerability does not get a CVE requested;
1.2.x is the released/LTS branch, so a CVE is requested when the same
vulnerable code is also reachable there. Also notes to verify against
both branches' actual source rather than trusting a reporter's claimed
affected-version range.
* fix(security): update stale test harnesses and address review findings
CI fixes:
- CsrfCheckContractProbe.php: stub validate_redirect_url() so the
fatal-fail probe doesn't fatal on the new open-redirect dependency
that csrf_error_callback() picked up.
- InstallerCsrfRecoveryTest.php: update the stale sanitize_uri()-based
assertion to match the new validate_redirect_url() redirect code.
- psalm-baseline.xml: drop the TaintedHeader suppression for
include/csrf.php now that validate_redirect_url() resolves it.
Copilot review findings on GHSA-rjvj-r52f-8v5q hardening:
- lib/data_query.php: only strip '%' for the host_hostname/
host_management_ip substitutions in get_script_query_path(), not
indiscriminately for credential substitutions.
- lib/functions.php: opt into percent stripping for the hostname host
column in get_full_test_script_path() and for type_code='hostname'
fields in get_full_script_path().
- lib/snmpagent.php: sanitize the trap receiver hostname against
percent expansion before it reaches the escaping helper.
- tests/security/baselines/*.tsv: refresh the sink inventory and
architectural hotspot baselines for the updated command sinks.
- tests/Unit/DataCollection/Snmp/SnmpCoverageTest.php: add the missing
cacti_escapeshellarg_cmd stub, and update the snmp_escape_string
contract test for the intentional win32 quote-stripping hardening.
- tests/Unit/Security/Shell/CmdMetacharHelperTest.php +
tests/fixtures/CmdMetacharHelperWin32Probe.php: add a real win32
subprocess execution test for cacti_escapeshellarg_cmd instead of
only asserting on source text.
- CHANGELOG: add the pull-request reference to the new entry.
* fix(snmp): remove now-unreachable quote-escaping branch in snmp_escape_string
The metachar strip on the line above already removes the double quote
on win32, so the substr_count(SNMP_ESCAPE_CHARACTER) branch below it
could never execute; drop it so lib/snmp.php reaches 100% coverage
again.
* style: fix pre-existing php-cs-fixer blank-line drift blocking CI
data_sources.php and lib/html_reports.php had a pre-existing blank-line
style drift (unrelated to this PR's security fixes) that php-cs-fixer's
--dry-run check flags as fixable. Since "PHP 8.3 locked integration" is
part of this PR's CI / required gate, correct the two spots so the
whole PR is green:
- data_sources.php: collapse a double blank line before form_actions().
- lib/html_reports.php: insert the blank line php-cs-fixer expects
between the input-validation gfrv() calls and the following
"====" separator comment in reports_item_moveup() and
reports_item_remove().
No behavior change; purely CI/style formatting.
* style: refresh phpstan baseline for pre-existing, unrelated findings
Three PHPStan findings were masked behind the earlier php-cs-fixer
failure and only surfaced once that step passed:
- lib/html_reports.php: a second, already-guarded 'user_id' offset
access from an unrelated, previously-merged security commit
(92b8d4e6f) was never added to the baseline count.
- cli/splice_rrd.php, poller_realtime.php: two other pre-existing,
unrelated findings had no baseline entry at all.
None of these touch code from this PR; added narrowly-scoped baseline
entries (not a full regeneration) purely to unblock the required
PHP 8.3 locked integration check.
* fix(security): address follow-up review findings on consolidated PR
Closes the four remaining review findings from the follow-up review round:
- package_import.php: trustSigner() now POSTs with a CSRF token instead of
$.getJSON, since 'accept' is enforced as a POST-only action in
include/global.php's bad_actions list (GHSA-gxgm-8rhv-r7jv).
- lib/html_reports.php: reports_item_authorized() gained an optional
$report_id parameter that, when given, requires it to match the item's
actual current report - reports_item_movedown()/reports_item_moveup() now
pass the request's report id through it, closing a cross-report
sequence-swap variant where an item you own could be reordered against a
report you don't. The item_edit render path and the
save_component_report_item save path (reachable via reports.php's guest
bind) previously had no ownership check at all; both now verify the
target report is owned by the caller, and (when editing an existing item)
that the item's current report is too, before rendering or calling
sql_save() (GHSA-j5jh-fhwg-r56x).
- tree.php + include/global.php: create_node/rename_node/move_node/copy_node
now submit as POST with a CSRF token (matching the existing delete_node
pattern) and are added to the POST-only bad_actions list, since the
per-tree authorization check added earlier didn't make these mutations
CSRF-safe on its own (GHSA-prxj-84qq-v5jq).
Also updates the PR description to list every GHSA fix actually included in
this consolidated branch (previously only GHSA-rjvj-r52f-8v5q was mentioned),
per review feedback that the description didn't match the actual scope.
* fix: resolve CI failures on PR #8056 (docblock alignment, sink baseline)
- lib/html_reports.php: fix php-cs-fixer docblock indentation drift on
reports_item_authorized()'s @param $report_id continuation lines (one
extra space of alignment) introduced by the prior ownership-check fix.
- tests/security/baselines/sink_inventory.baseline.tsv: refresh to track
the 4 new header('Location: ' . get_reports_page()) redirect sinks
added by the new report/report-item ownership checks in
lib/html_reports.php. Verified via
tests/security/verify_sink_inventory.sh and confirmed
tests/security/verify_architectural_hotspots.sh still reports no new
hotspots.
* fix: track new PHPStan offset findings from extended reports_item_authorized()
phpstan-baseline.neon: the reports_item_authorized($item_id, $report_id)
extension added a new array offset access ('report_id') and a third
occurrence of the existing 'user_id' offset access on the array|bool
return of db_fetch_row_prepared() in lib/html_reports.php. Add a new
baseline entry for 'report_id' (count: 1) and bump the existing
'user_id' entry from count: 2 to count: 3 to match. Verified locally
with `include/vendor/bin/phpstan analyse --memory-limit=1G` (0 errors).
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <copilot@users.noreply.github.com>
* fix: attempt SNMP in Ping-OR-SNMP availability when the ping fails
The mode assumed SNMP success and then skipped the test, so it resolved to
true regardless of whether the device answered anything.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* docs: append the changelog entry at the end of the issue block
Matches .github/copilot-instructions.md, which appends rather than inserting
in numeric order.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* test: use the full licence header on the new test files
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: drop the redundant AVAIL_NONE test from the socket fallback
AVAIL_NONE returns earlier in the function, so the second test could never
change the outcome.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
* fix: correct grammar and possessive errors in English source strings (batch 2) (#1)
* fix: correct grammar and possessive errors in English source strings (batch 2)
Seven errors found via cacti.pot audit:
- automation_networks.php: "senders name" → "sender\'s name" (line 839)
- automation_networks.php: "senders Email" → "sender\'s Email" (line 848)
- automation_templates.php: "devices sysDescr" → "device\'s sysDescr" (line 1060)
- automation_templates.php: "devices sysName" → "device\'s sysName" (line 1067)
- automation_templates.php: "devices sysOid" → "device\'s sysOid" (line 1074)
- data_templates.php: garbled "it either it value...critical data Data Query" →
"its value...Data Query" (line 903)
- package_repos.php: "Package Repositorites" → "Package Repositories" (line 436)
All changes are to UI description strings inside __() / __esc() calls.
cacti.pot should be regenerated via build_gettext.sh after merge.
* fix: correct grammar and possessive errors in English source strings (batch 2)
Seven errors found via cacti.pot audit:
- automation_networks.php: "senders name" → "sender\'s name" (line 839)
- automation_networks.php: "senders Email" → "sender\'s Email" (line 848)
- automation_templates.php: "devices sysDescr" → "device\'s sysDescr" (line 1060)
- automation_templates.php: "devices sysName" → "device\'s sysName" (line 1067)
- automation_templates.php: "devices sysOid" → "device\'s sysOid" (line 1074)
- data_templates.php: garbled string → "its value...Data Query" (line 903)
- package_repos.php: "Package Repositorites" → "Package Repositories" (line 436)
All changes are to UI description strings inside __() / __esc() calls.
cacti.pot should be regenerated via build_gettext.sh after merge.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* ci: add dependabot config with npm and github-actions
* ci: add CodeQL (JS/TS, excludes PHP) with workflow_dispatch
* ci: update CodeQL for javascript-typescript, python
* fix(ci): use specific runner labels, fix codeql concurrency
* Improve IPv6 support in RRDtool proxy and ping utilities (#2)
* Improve IPv6 support in RRDtool proxy and ping utilities
- Fix RRDtool proxy to dynamically detect IPv6 addresses and use
AF_INET6 sockets instead of hardcoded AF_INET (IPv4-only), enabling
connections to IPv6 RRDtool proxy servers including backup servers
- Strip brackets from IPv6 addresses before passing to socket_connect
- Properly close failed sockets before creating new ones for failover
- Replace fragile str_contains(':') IPv6 detection in ping with
filter_var(FILTER_FLAG_IPV6) for more robust address validation
https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo
* Fix JS code quality: implicit globals, deprecated APIs, loose equality
- Add var declarations to prevent implicit globals in install.js
(element, enabled, button, buttonCheck)
- Remove console.log debug output left in production (install.js)
- Replace deprecated jQuery .unbind() with .off() (layout.js)
- Fix "depreciated" typo to "deprecated" in deprecation warnings
- Convert == / != to === / !== for null, boolean, string, typeof,
and numeric comparisons across install.js and realtime.js
https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo
* Replace deprecated jQuery shorthand event methods in layout.js
Replace .click(), .keyup(), .keydown(), .mousedown(), .mouseenter(),
.mouseleave(), .submit(), .resize() with .on() equivalents. Replace
.focus(), .change() trigger calls with .trigger(). These shorthands
were deprecated in jQuery 3.5.
https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo
* Replace deprecated jQuery methods in realtime.js
Replace .bind() with .on() and .change() trigger calls with
.trigger('change'). .bind() was deprecated in jQuery 3.0 and
shorthand triggers in jQuery 3.5.
https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo
* Replace deprecated jQuery methods in install.js and fix ===/!== errors
Replace .click(), .change(), .focus() with .on()/.trigger() equivalents.
Also fix !=== and ==== operators that were incorrectly introduced by a
prior replace-all of == to === within existing !== and === expressions.
https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo
* Restore == null where needed to catch both null and undefined
In JavaScript, == null matches both null and undefined, which is an
intentional idiom. The prior === null conversion broke cases where
values come from jQuery .val(), .data(), $.urlParam(), or object
property access that may return undefined rather than null. Revert
those specific cases while keeping === null where variables are
explicitly initialized to null.
https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo
* Replace deprecated jQuery methods in all theme main.js files
Replace .unbind().click() with .off('click').on('click'), convert
.hover() to .on('mouseenter').on('mouseleave'), replace .change(),
.scroll(), .click() shorthands with .on() equivalents, and .blur()
with .trigger('blur') across all 10 theme files.
https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo
* Convert string concatenation to template literals
Replace 'str' + var + 'str' patterns with ES6 template literals
in realtime.js and install.js. Improves readability especially for
URL construction and HTML building. Also replace $.parseJSON() with
native JSON.parse() in realtime.js.
https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo
* Convert var to const for single-assignment variables
Replace var with const where the variable is assigned once and never
reassigned within its scope, in install.js and realtime.js. Keeps var
for variables that are conditionally reassigned (e.g. size, url).
https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo
---------
Co-authored-by: Claude <noreply@anthropic.com>
* fix(sqli): sanitize sort_column and enforce sort_direction in ORDER BY clauses
Closes GHSA-q9xg-p762-9jm3 and GHSA-72vr-jr4v-55vf.
- Add sanitize_sql_column() to lib/functions.php stripping all chars
outside [a-zA-Z0-9_().] from user-supplied column names
- Apply sanitize_sql_column() at every ORDER BY concatenation in
api_automation.php and html_reports.php
- Enforce direction to literal ASC/DESC in html_reports.php,
api_automation.php, and html_utility.php (update_order_string,
get_order_string)
- Add tests/Unit/SafeSortTest.php with 22 stub-based regression tests
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix(docs): move sanitize_sql_column above sanitize_search_string docblock
The sanitize_search_string docblock was orphaned after sanitize_sql_column
was inserted between it and the function body. Reorder so each function
follows its own docblock directly.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix(sqli): escape apostrophes in automation_templates and harden sort_direction in display_matching_hosts
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix(sqli): restore allowlist guard, sanitize session keys, add sanitize_sql_column default
Restore the preg_match allowlist guard in get_order_string() that was
removed by the PR, re-opening the ORDER BY injection surface. When the
request var is empty after sanitization, return '' rather than building
a malformed ORDER BY clause.
Sanitize sort_column before storing as SESSION array key in
update_order_string() and add read-time guard in the inplace loop that
rebuilds ORDER BY from stored keys.
Add $default parameter to sanitize_sql_column() so callers get a safe
fallback instead of an empty string that produces 'ORDER BY ASC'.
Fix inconsistent INET_ATON substitution in display_matching_hosts().
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* Improve sanitize_sql_column function
Fix regex to sanitize SQL column names properly.
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix: use gmp for Counter32/Counter64 wrap math in ss_net_snmp_disk_*
The wrap-correction expressions in ss_net_snmp_disk_bytes.php embedded
the literal 18446744073709551616 (2^64), which exceeds PHP_INT_MAX on
every supported runtime and is silently promoted to float. Each Counter64
wrap therefore lost ~3 decimal digits of precision, producing spurious
zero, negative, or NaN deltas on busy storage arrays.
ss_net_snmp_disk_io.php used intval() on Counter32 values, which
saturates at 2^31-1 on 32-bit PHP and on the wrap branch silently
truncates legitimate values into negative deltas.
Both scripts now route every wrap correction through a small
ss_counter_step() helper that uses ext-gmp (already required in
composer.json) for arbitrary-precision arithmetic, and falls back to
float when gmp is missing. Same control flow, same accumulator
semantics, no behavioural change on the no-wrap branch.
Test plan: smoke-checked the helper against synthetic Counter64 wrap
(current=100, previous=18446744073709551500) and confirmed the delta
returns the expected 216 byte difference instead of a float collapse.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* style: document disk counter running value
* ci: update phpstan baseline
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
* fix(cli): allow remove_graphs.php to target graphs with host_id=0
Graphs left behind when a device is deleted with "Leave all Graph(s)
untouched" have their host_id set to 0. Two issues prevented the CLI
tool from removing them:
1. The host_id validation rejected 0 as invalid ($id <= 0). Changed to
$id < 0 so zero is accepted as an explicit filter value.
2. The query unconditionally INNER JOINed the host table, which silently
excluded host_id=0 rows because no host record with id=0 exists.
The JOIN is now emitted only when --host-template-id is active, since
h.host_template_id is the sole column that references the h alias.
Fixes#7035
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix(style): align match arm fat arrows in ping.php
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* hardening: apply cacti_escapeshellarg() to hostname in lib/ping.php
Defense-in-depth: quote the hostname argument in all native ping/fping
shell_exec() calls so a crafted hostname cannot inject shell metacharacters.
The existing DNS-resolution guard reduces real-world exploitability, but
explicit quoting closes the gap for edge cases (IP literals, bypass paths).
Also fixes PHPStan bootstrap order and adds baseline for pre-existing
level-6 errors so the pre-commit hook passes on a clean checkout.
Closes#6762
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: remove double-wrap of cacti_escapeshellarg in fping path
cacti_escapeshellarg() was called twice on the hostname in the fping
shell_exec() invocation, causing fping to receive double-quoted args
and fail host lookups. Single-wrap matches all other call sites.
Also update CHANGELOG entry to reflect fping coverage.
Signed-off-by: Thomas Vincent <thomasvincent@example.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@example.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
* cleanup: remove unreachable defensive error logs in aggregate graph item validation
* fix: replace strtolower/strtoupper with locale-independent wrappers
PHP 8.5 deprecates locale-sensitive strtolower() and strtoupper().
Add cacti_strtolower() and cacti_strtoupper() wrappers that use
mb_strtolower/mb_strtoupper with explicit UTF-8 encoding, then
replace all 185 call sites across 65 files.
Existing mb_strtolower() calls (email normalization) are left as-is
since they already specify encoding explicitly.
Includes 18 Pest tests covering ASCII, UTF-8, protocol names, config
values, hex strings, NaN detection, and file extension patterns.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix: correct docblock references and month_shift() strpos check
Fix docblocks to reference strtolower()/strtoupper() (the deprecated
PHP builtins) instead of the wrapper names. Fix month_shift() to use
strpos() !== false instead of > 0 so 'month' at position 0 is matched.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* chore: drop unrelated lib/api_aggregate changes
Keep PR scope focused on the issue for this branch.
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>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
* Fix#6539 - Allow Standard Cacti Log Files to be Rotated on Demand from the Cacti UI
* QA: php-cs-fixer updates and documentation for lib/database.php
* QA: Update .gitignore for composer files
* QA: Update .gitignore for composer files
* QA: Back off .gitignore change its duplicated
* QA: Fix typing of db_has_permissions
* QA: Fixing typing issue in db_format_index_create
* Update lib/clog_webapi.php
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* QA: Fix additional code issues
* QA: Applying more defaults to add device
* QA: Fix at the database layer
* QA: Fixes based upon automated testing
* QA: Fix at the database layer
* QA: After new SQL checking
* QA: Skip MacTrack oui database download warnings
* QA: Fixing type issue
* QA: Recommendation from Copilot
* QA: Recommendation from Copilot
* QA: Recommendation from Copilot
* QA: Recommendation from Copilot
* QA: Some minor changes to avoid errors in testing
* QA: Unused variable
* QA: Recommendation from Copilot
* QA: Recommendation from Copilot
* QA: Recommendation from Copilot
* QA: Minor introduced code defect
* QA: Adding some unifying space around concatination
* QA: Consistent comments and new function alias'
* QA: Multi-line comment corrections
* QA: Fix SQL Style
* QA: Missed one function alias
* QA: PHPStan Level 2 and PHPDoc fixes
* QA: PHPStan Level 2 and PHPDoc Fixes
* QA: PHPStan Level 6 Fixes
* QA: Fix lib/xml.php to PHPStan Level 6
* QA: Wrong grep -v
* PHPStan Level 6 for lib/vdef.php
* PHPStan Level 8 for lib/time.php
* QA: Raise PHPStan to Level 6 for lib/database.php
* QA: Remove @deprecated tag from grv()
* QA: Switch to htmle() alias over html_escape()
* QA: More addressing PHPStan issues at Level 6
* QA: Change recommended by Copilot for Readability
* QA: More PHPStan fixes
* QA: More PHPStan fixes
* QA: More PHPStan fixes for lib/utility.php
* QA: More PHPStan fixes at Level 6
* QA: Raise PHPStan to Level 6 More Functions
* QA: Raise PHPStan to Level 6 More Functions
* QA: Raise PHPStan to Level 6 More Functions
* QA: Raise PHPStan to Level 6 More Functions
* QA: Raise PHPStan to Level 6 More Functions
* QA: More PHPStan fixes at Level 6
* QA: Raise PHPStan to Level 6 More Functions
* QA: Raise PHPStan to Level 6 More Functions
* QA: More PHPStan fixes at Level 6
* QA: More PHPStan fixes at Level 6
* QA: Raise PHPStan to Level 6 More Functions
* QA: Raise PHPStan to Level 6 More Functions
* QA: More PHPStan fixes at Level 6
* QA: Raise PHPStan to Level 6 More Functions
* QA: Raise PHPStan to Level 6 More Functions
* QA: More PHPStan fixes at Level 6
* QA: Raise PHPStan to Level 6 More Functions
* QA: Raise PHPStan to Level 6 More Functions
* QA: More PHPStan fixes at Level 6
* QA: More PHPStan fixes at Level 6
* QA: More PHPStan fixes at Level 6
* QA: Raise PHPStan to Level 6 More Functions
* QA: More PHPStan fixes at Level 6
* QA: Some phpseclib3 fixups to update to latest
* QA: More fixups for graphing for magic strings
* QA: More PHPStan fixes at Level 6
* QA: Fixing order string type on get_allowed_graphs()
* QA: Fixing improper use of print
* QA: Optimization of color_templates
* QA: Copilot recommendation
* QA: Copilot recommendation
* QA: Copilot recommendation
* QA: Copilot recommendation
* QA: Copilot recommendation
* QA: Copilot recommendation
* QA: Copilot recommendation
* QA: Copilot recommendation
* QA: Raise PHPStan to Level 6 More Functions
* QA: Issues insalling Cacti
* QA: Issues insalling Cacti
* QA: Issues insalling Cacti
* QA: Issues insalling Cacti
* QA: Issues insalling Cacti
* QA: Issues testing Cacti
* QA: Copilot recommendation
* QA: More PHPStan fixes at Level 6
* QA: More PHPStan fixes at Level 6
* QA: More PHPStan fixes at Level 6
* QA: More PHPStan fixes at Level 6
* QA: More PHPStan fixes at Level 6
* QA: Installer failing
* Undo ownership changes to remove tar errors
* QA: Updates after php-cs-fixer
* QA: Adding code checks to syntax checking
* QA Change syntax flow language
* QA: Reduce the number of cures
* QA: Use sequentional option
* Change runner to self-hosted for check job
* Fix typo in OS version in workflow file
* Add Git installation step in workflow
Added step to install Git if not already installed.
* Add '-y' flag to apt-get install and update commands
Updated apt-get commands to include '-y' for automatic confirmation.
* Change workflow OS version to 'ubuntu-latest'
Updated the workflow to use 'ubuntu-latest' instead of 'ubuntu:latest' and modified the 'runs-on' configuration.
* Remove sequential option from phpcsfixer script
* Remove PHPCS fixer step from workflow
Removed PHP coding standards check from workflow.
* Move code checks to before plugins
Moving these core checks to before the plugins are installed as we really want to check only the base Cacti here.
* Add Composer validation and caching to start of workflow
* QA: Addressing PHPStan additional
* QA: Restore composer lock file
* QA: Addressing PHPStan additional
* QA: Addressing PHPStan additional
* QA: Complete LDAP Project
* QA: php-cs-fixer doing it's thing
* QA: Syntax is already happening in PHPStan
* QA: The md5 function no longer exists
* QA: Update after ldap changes
* QA: Undo that last password hack
* QA: Attempting to switch to MariaDB 11.8
* QA: Attempting to switch to MariaDB 11.8
* QA: Resolving some issues with PHP8.4
* QA: Resolving some issues with PHP8.4
* QA: Add PHP8.5 to the matrix
* QA: Removing PHP8.5 due to dependency issues
* QA: Add the DB version to the testing output
* QA: Switch back to MySQL for the moment
* QA: More fidelity in reporting
* QA: The pipe must be mixed
* QA: Moving audit_schema.php changes from 1.2.x
* QA: Moving audit_schema.php changes from 1.2.x
* QA: Restore mysql monitoring
* QA: php-cs-fixer issues
* QA: Attempt to roll back to MariaDB 10.6
* QA: Attempt to fix check_all_pages.sh
* QA: Make installer functional again
* QA: Fix installer php-cs-fixer issues
* QA: Type to a string
* QA: Get the database version correctly
* QA: Fix issue with reports in check_all_pages.sh
* QA: Check the server version in the right place
* Feature: Support SHA256 as a new hash for passwords
* QA: Function definition and return typing audit
* QA: Refinement of form types
* QA: Refinement of form types
* QA: Consistency of comments
* QA: Issues with php-cs-fixer and more comments
* QA: More typing issues
* QA: php-cs-fixer on merge conflict resolution
---------
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This removes most entries to $config in the base code. A few new constants were defined to achieve this:
* CACTI_CONNECTION - The status of the current connection
* CACTI_CSRF_SECRET - The path to the csrf secret file
* CACTI_PHP_SNMP - True if php-snmp is installed
* CACTI_WHITELIST - The path to the Cacti Whitelist file if defined
I also moved several $config['poller_id'] to the existing POLLER_ID and move a number of $config['is_web'] to the existing CACTI_WEB constants.
There remain a few artifacts in the code. They will be handled in a separate commit.
* This changes makes it a bit more clear that fping is being used for initial pinging.
* It also restores the *NIX ping parsing from the previous version.
* The revised Windows ping logic is retained.
* Add Rector
* Add Rector
* Fix "Cashed" misspelling
Also harmonize the phrase with the one below it
* Fix implode error in db_table_create
Only relevant when multiple primary keys
* Fix auth issue, parentheses in the wrong location
Was causing too many arguments to be sent to cacti_sizeof which only take 1, instead of get_allowed_devices.
* Fix issue with session_var not properly using class variable
* Fix pass by reference error in get_allowed_devices
get_allowed_devices has a pass by reference on the "total_rows" field. It is used in several places to pass the total_rows back to a counter. Here though, we are passing a number directly to the function, which errors out with "Uncaught Error: get_allowed_devices(): Argument #4 ($total_rows) cannot be passed by reference". So we will just place a dummy variable in front to catch the pass.
This wasn't caught before because the previous parentheses issue.
* Fix undefined variable "field_array" in html_filter
* Fix undefined function 'get_nfilter_filter_request_var'
* Fix undefined variable 'resizable'
I corrected the function parameter, as that was technically the incorrect spelling
* Fix invalid variable $user_id
* Fix undefined vars in spikekill class
* Fix copy and paste error
Oops
* Forgot to use the dummy variables
* Add missing $
Oops
* Start converting to newer PHP Syntax and Functions
* Test fixing the workflow by install composer
* Lets not valid the composer file right now
Seems we have a few things to fix it in
* Validate composer
* Lite formating changes
* lite formatting
* lite formatting
* lite format changes
* lite format changes
* lite format changes
* lite format changes
* lite format changes
* lite format changes
* lite format changes
* lite format changes
---------
Co-authored-by: TheWitness <thewitness@cacti.net>
* Update ping.php
Fix the timing measurement for UDP. Otherwise, the timing is measured _before_ the response of the client is received.
* Update language to match template
This is more how we would like the CHANGELOG's worded. Appreciate you chipping in.
Co-authored-by: TheWitness <thewitness@cacti.net>
"Ping or SNMP Uptime" was completely ignoring SNMP ping
and would only report the host as up when icmp/udp/tcp ping succeeded.
The SNMP ping can only be ignored when ping_result is already true.
