Commit Graph
287 Commits
Author SHA1 Message Date
TheWitness 0055dd00cc security: consolidate verified GHSA fixes for develop (batch 3) + CHANGELOG CVE reconciliation (#8141)
* security: consolidate verified GHSA fixes for develop (batch 3)

Develop counterparts of the 1.2.x fixes in #8138, each re-verified against
develop HEAD.

- GHSA-77pr-g949-4v5f: spikekill.php also requires is_graph_allowed() on the
  target graph, not just the Spike Kill realm.
- GHSA-w3qw-762w-6g9v: the include/global.php GET action guard fails closed on
  an array-form action[] before the scalar denylist comparison.
- GHSA-xhpr-w454-cc9w: update_poller_cache() casts the stored output_type to
  int before interpolation.
- GHSA-j3px-vw6r-g25x: graphs_new.php escapes stored snmp_index via db_qstr()
  in the data-query IN() pivot.
- GHSA-h745-3r76-g253: graphs_new.php escapes the data-query and field names in
  the XML parse error messages (__ -> __esc).

GHSA-297h-95ff-r767 does not apply on develop: the clog log-file selector
renders through the drop_array filter framework, which escapes.

tests/Unit/Security/ConsolidatedBatch3Test.php adds source-scanning regression
guards. CHANGELOG (1.3.0-dev) updated; php -l, the security baselines, and
check-changelog.py all pass.

* chore: lead 1.3.0-dev security CHANGELOG entries with the GHSA id

Normalize the 1.3.0-dev -security entries to the approved
"-security: GHSA-xxxx [CVE-xxxx] Description" form, moving the advisory id to
the front. CVE tokens are omitted because none of these advisories have a CVE
issued yet (verified via the repository security-advisories API). Covers the
five batch-3 entries plus the pre-existing entries that still used the trailing
(GHSA-xxxx) form.

* chore: use the -security#GHSA-xxxx: form for the 1.3.0-dev CHANGELOG entries

Correct the batch-3 and reconciled 1.3.0-dev -security entries to the approved
"-security#GHSA-xxxx: CVE-yyyy Description" form (GHSA id in the prefix). No CVE
tokens are added because none of these advisories have a CVE issued yet
(verified via the repository security-advisories API).

* test: split batch-3 security guards into vulnerability-class files; reconcile CHANGELOG

- Move the ConsolidatedBatch3Test.php guards into the matching Auth, Csrf,
  SqlInjection and Xss folders with behavior-based filenames, and strengthen
  them per review: the GET-guard test asserts the array branch actually returns
  405 and exits; the snmp_index test rejects both raw concatenation branches;
  the parse-error test asserts every name-bearing message uses __esc().
- Append this pull request number to the five new 1.3.0-dev security entries.
- Add issued CVE identifiers to the older -security#GHSA CHANGELOG entries
  (verified via the repository security-advisories API). Entries with no CVE
  yet, entries that already carried a CVE, and non-GHSA lines are unchanged.
2026-10-02 11:03:52 -04:00
TheWitnessandTheWitness 3ab4c69fb7 PHPStan Level 8 - Phase 1 Conversion (#8135)
* 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>
2026-10-02 10:08:56 -04:00
TheWitnessandcopilot-swe-agent 2d516ca0a9 security: consolidate additional verified GHSA fixes for develop (batch 2) (#8065)
* security: contain data source paths within the RRA directory (GHSA-9qff-xccr-rrc5)

* security: escape executable path in exec_background on non-win32 (GHSA-cv7j-p6fh-j4q7)

* security: stop disabled accounts authenticating via remember-me cookie and revoke on disable (GHSA-pqrp-v752-mc4f)

* security: prefer base_url over client Host for the force_https redirect (GHSA-3466-fp7g-c9rf)

* changelog: add fragments for GHSA-9qff-xccr-rrc5, GHSA-cv7j-p6fh-j4q7, GHSA-pqrp-v752-mc4f, GHSA-3466-fp7g-c9rf

* security: address review findings on GHSA batch 2 (develop)

- lib/dsstats.php, poller_maintenance.php: route through
  get_data_source_path() instead of expanding data_source_path directly,
  so the RRA containment check applies to these consumers too (GHSA-9qff)
- lib/functions.php: data_source_path_within_rra() now also validates
  the realpath of each existing ancestor segment, closing a symlink
  pivot that the purely lexical check missed (GHSA-9qff)
- include/global.php: fail closed instead of falling back to SERVER_NAME
  when base_url is unset/invalid, removing the residual open-redirect
  path (GHSA-3466)
- lib/html_utility.php: cacti_force_https_host()/
  cacti_build_https_redirect_url() now parse a scheme-less base_url
  (matching lib/reports.php's own normalization) and preserve a
  configured non-default port instead of silently dropping it (GHSA-3466)
- tests: move DataSourcePathContainmentTest.php into
  tests/Unit/Security/PathTraversal/, and switch its fixtures from a
  fictional CACTI_PATH_RRA path to a real temp directory so the new
  realpath()-based checks (including a symlink-pivot regression case)
  can actually run

* test: fix CACTI_PATH_RRA constant collision in DataSourcePathContainmentTest

CI showed 2 failures: RrdProxyProtocolTest.php runs earlier in the same
Pest process and already defines CACTI_PATH_RRA to a fictional,
non-existent '/var/lib/cacti/rra', so our own definition guard never
ran and realpath(CACTI_PATH_RRA) returned false for every assertion,
failing the two tests that expect containment to be true.

Now ensure the directory backing CACTI_PATH_RRA actually exists on
disk regardless of who defined the constant first, and gracefully
skip only the fixture-dependent assertions if this process can't
create it (e.g. no permission on a system path).

* test: refresh sink inventory baseline for exec_background rename

5fbe90fb4 renamed \ to \ in lib/poller.php's
exec() calls (GHSA-cv7j-p6fh-j4q7) but never regenerated the tracked
sink inventory, so the 'sink and architectural hotspot guard' CI check
has been failing on this branch since that commit. Regenerated via
tests/security/build_sink_inventory.sh | tr -d '\r' | LC_ALL=C sort;
diff confirms only those two lines changed.

* fix: address CI failures on GHSA batch 2 (develop)

- lib/auth.php: verify the remember-me token before revoking a disabled
  account's cache row, so a forged token no longer triggers a database
  write (fixes AuthSystemRegressionIntegrationTest)
- apply php-cs-fixer coding standard fixes (comment style, phpdoc
  alignment, list-to-short-array syntax) flagged by the locked
  integration check
- refresh the architectural hotspots baseline for the exec_background
  safe_filename rename

* fix: update PHPStan baseline for reordered check_auth_cookie and get_data_source_path

- lib/auth.php: reordering the disabled-account check after the token
  match moved one 'enabled' offset access to a new line, bumping the
  file's expected occurrence count from 3 to 4
- lib/functions.php: the GHSA-9qff-xccr-rrc5 containment log line reads
  \['data_source_path'], a genuinely new array|bool access
  not previously tracked

---------

Co-authored-by: copilot-swe-agent <copilot-swe-agent[bot]@users.noreply.github.com>
2026-09-24 10:06:01 -04:00
1e5329dd89 security: consolidate verified GHSA-rjvj win32 cmd.exe hardening for develop (#8056)
* 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>
2026-09-23 09:45:42 -04:00
Thomas VincentandTheWitness cb49f31ff9 Replace csrf-magic with maintained CSRF protection (#7744)
* build: add symfony/security-csrf

Backs the CSRF token core that replaces the vendored csrf-magic fork.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: fix security-csrf package path assertion

symfony/security-csrf ships CsrfTokenManager.php at the package root, not under src/.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* refactor: register the composer autoloader before the CSRF include

CSRF validation ran before the autoloader existed, which blocks any class-based guard.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* feat(csrf): add CactiCsrfGuard over symfony/security-csrf

Issues and validates session-scoped tokens; not yet wired into the request path.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* feat(csrf): accept pre-upgrade csrf-magic tokens during the grace window

Legacy tokens carry their own expiry, so the fallback closes itself without a config flag.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* feat(csrf): port the output buffer rewriter to CactiCsrfGuard

Keeps the hidden field name and JS globals that third-party plugins rely on.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): use json_encode for the JS script context in rewriteBuffer

htmlspecialchars() protects the HTML attribute context but not the
<script> string literal, which relied on Symfony's token generator
never emitting a quote rather than being safe by construction.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* refactor(csrf): move the XHR shim to include/js as Cacti-owned code

It has been maintained by the Cacti Group since 2020 and is not an upstream package.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* refactor(csrf): retire the vendored csrf-magic fork

The procedural csrf_* surface stays as a deprecated shim so plugins keep working.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* feat(csrf): validate requests through CactiCsrfGuard

csrf-magic self-invoked from its own file; the guard is called explicitly.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(install): stop gating the permission step on the csrf secret

Retiring csrf-magic removed the directory its default path lives in, and setStep() clamps to the first errored step, so no fresh install could advance.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): key the legacy fallback on the raw secret bytes

refresh_csrf.php writes PHP source with a trailing newline, and trimming that yields a different HMAC key than csrf-magic used.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): log when the guard degrades to disabled in a web context

The installer is the only web path that can reach it, and failing open in silence is not something csrf-magic could do.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* docs: changelog for the CSRF library replacement

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): mint the token before the session write callback closes it

csrf_startup() minted lazily from the output buffer callback, which runs after session_write_close()'s shutdown-function registration, so a token minted there on a legacy-fallback POST was silently dropped.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): make refresh_csrf.php a no-op

Its default secret path no longer ships and live tokens are session-bound, not HMACs over the secret, so rotation neither worked nor did anything meaningful; running it during the upgrade grace window used to delete the legacy secret and kill validation for pre-upgrade tokens outright.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): fall back to the old default secret path when reading it

csrf-magic tried its own directory as a second candidate regardless of $path_csrf_secret; the new reader only tried the configured path, so a site pointing $path_csrf_secret at a file that was never created lost the legacy-token grace window entirely.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* docs(csrf): point the JS header at its BSD license sidecar

The header reduces the original BSD terms to prose and the actual text sits in an unreferenced csrf.js.LICENSE, so add a pointer to it.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): restore csrf_check()'s bool return and $fatal contract

The shim ignored $fatal and always returned void, so audit/audit.php's `!csrf_check(false)` evaluated `!null` (always true) and denied every request instead of getting a real pass/fail result back.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* docs: reference the pull request number in the changelog entry

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: resolve includes through Cacti path constants

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(csrf): enforce full coverage

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* Harden CSRF fallback handling

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(csrf): assert the component stays out of the index

Committing it would leave a second copy to drift against the lock.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* ci: align CSRF checks with PHP 8.2

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* docs: normalize the CSRF changelog entry

* Fix CHANGELOG entry ordering

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(csrf): support constrained test environments

* fix(csrf): narrow nullable guard state

* test: place CSRF coverage in security domain

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* security(test): match disabled exec exactly

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* ci: run CSRF coverage on PHP 8.3

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-09-18 00:16:39 -04:00
Thomas Vincent a2c8df612e fix(csrf): require POST tokens for state-changing actions (#7991)
* fix(csrf): require POST tokens for item actions

* test(csrf): cover every guarded action path

* test(csrf): keep legacy guard contract extensible

* fix(csrf): hide POST-only targets from crawlers

* test(csrf): reject every crawlable state-action link

* fix(csrf): fail closed without logging crawler probes

* fix(csrf): post data-query item actions safely

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): normalize stripped token separators

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): distinguish method and token failures

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): escape POST action URL attributes

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): delegate POST action events

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): post data template add action

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* style(html): align POST button class

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-09-12 15:09:25 -04:00
Thomas Vincent 5da1a2d5e8 security: restrict graph input fields across handoffs (CVE-2024-31458) (#7669) 2026-08-30 10:56:16 -07:00
Thomas Vincent 21708ab599 security: stop trusting Host for HTTPS redirects (#7833)
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-08-29 16:01:36 -07:00
Thomas Vincent ff475bef6d feat(installer): add installation domain foundation (#7291) 2026-08-22 19:05:50 -07:00
Thomas Vincent 298bd51eca security: require POST + CSRF token for GET-accessible delete actions (#7673) 2026-08-16 22:15:39 -07:00
Thomas Vincent d4e255b5d9 security: stage nonce CSP migration and gated enforcement (#7662)
* security: add nonce CSP mode for inline JavaScript protection

Reworks the content_security_policy_script setting into a three-mode Inline JavaScript Protection dropdown (HTMLX default, Nonce, None) and ports the per-request nonce, strict-dynamic policy, and CSP violation report endpoint from 1.2.x. The stored option key and its default are unchanged, so existing installs keep their current policy byte-for-byte; nonce mode is opt-in.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* security: stage nonce CSP enforcement safely

Add report-only migration, gated enforcement, nonce-aware shared emitters, Linux Docker validation, and operator documentation.\n\nRefs #7703

* security: validate CSP header sources

Reject malformed alternate-source configurations at the CSP boundary, validate fallback report URIs, and remove the obsolete Psalm header-taint baseline entry.\n\nRefs #7705

* chore: drop the composer.json change from this PR

Dependency and manifest changes are a maintainer call and do not belong in
this one.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-08-16 21:38:46 -04:00
Thomas VincentandTheWitness 5eb058961b fix(csp): honor alternate frame ancestors on develop (#7597)
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-08-10 12:09:02 -04:00
Thomas VincentandTheWitness a75bf823bb feat: consolidate htmx loader and renderer abstraction global.php includes (#7517)
* feat(rendering): add view renderer abstraction

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* isolate renderer traversal test file

* Standardize renderer headers and endpoint guard

* style(renderer): use standard unit-test header in CactiRendererTest

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(renderer): reject null-byte paths before realpath

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* ci: refresh sink and hotspot baselines for line shifts

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(renderer): guard root base path and template-dir redirect

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(renderer): canonicalize both paths in index.php direct-access guard

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* hardening: normalize renderer base path to one separator

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(renderer): reject filesystem root as template path

A /-rooted renderer collapsed the containment prefix to /, which
str_starts_with() matches for every absolute path, so any file was
includable. Fail closed by refusing a root template path.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* hardening: reject windows drive-root template path

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* refactor: adopt PHP 8.4 idioms in view renderer

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: cover renderer abstraction to full coverage

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* hardening(renderer): reject UNC template names and fix sink baselines

Refuse a leading-backslash (UNC) template name the same way absolute
POSIX paths are refused, skip the direct-request redirect when
SCRIPT_FILENAME carries a null byte, and correct the renderer include
and views redirect line numbers in the security baselines.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* style: apply php-cs-fixer

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(renderer): use PHP 8.1-compatible readonly property in CactiRenderer

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* hardening(rendering): drop template context array before include

Unset the internal context array after extract so an included template
only sees the extracted variables, not the full context.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* ci: refresh security baselines for renderer line shift

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(renderer): simplify containment check and drain nested output buffers

The redundant real_path !== template_path comparison in
validateTemplateFile() can never be false once is_file()/is_dir() have
run, so drop it and keep the str_starts_with() prefix check as the
actual containment guard. renderFile() also only unwound a single
output buffer, so a template that opens (and leaves) its own buffer
could leak an ob level or drop its content; unwind every buffer opened
during rendering and reassemble them in the order they were written.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* ci: refresh security baselines for renderer buffer/containment fix

The dynamic_include line for lib/renderer.php's include shifted again
after adding drainOutputBuffers().

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* feat(htmx): vendor htmx 2.0.6 and add opt-in filter loader

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: cover htmx filter loader

Add missing-vendored-file case for htmx_script_tag and mark the pre-bootstrap read_config_option guard as uncoverable.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: harden htmx loader harness and tighten enable check

Skip anything but an exact 'on' for htmx_enabled, reject 4xx/5xx fixture
responses, fail fast when the fixture base path is unresolved, pin
setup-php to a commit SHA, and reindent the e2e specs to tabs.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* style: apply php-cs-fixer to htmx test files

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(htmx): drop duplicate script-tag src assertion

The subdir-install test repeated the preceding assertion; CACTI_PATH_URL is
fixed at bootstrap so it could not exercise a distinct scenario.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(e2e): drop sqlite3 from docker-php-ext-install

php:8.2-apache already ships sqlite3 built in, and its ext/sqlite3
source only carries config0.m4 (not config.m4), so building it
standalone always fails with "Cannot find config.m4", breaking the
e2e image build before Playwright ever runs.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(htmx): clear stat cache around version-file rename

Earlier tests already stat the version file via file_exists()/
file_get_contents(), so PHP's stat cache can report it present right
after these tests rename it away, making the assertions flaky.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(htmx): recompute script-tag cache-buster per call

The md5 cache-buster was cached in a static, so FPM/script_server
processes kept serving a stale ?v=... hash after an htmx.min.js
upgrade. Recompute it per call, matching get_md5_include_js() and
get_theme_paths() in lib/functions.php, and drop the 'v=' parameter
name to match their path + '?' + hash shape.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(e2e): drop obsolete MariaDB options

* ci(htmx): run the htmx integration test by path instead of a testsuite filter

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* build: vendor htmx unminified

Debian rejects minified sources. The file is the htmx 2.0.6 dist source; its
minified sibling hashes identically to what was committed, and the pinned SRI
is recomputed over the unminified file.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: address htmx review feedback

* test(e2e): resolve Cacti version through PHP

* test(htmx): harden portable server cleanup

* ci(actions): update checkout action pins

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-08-10 11:45:24 -04:00
Thomas Vincent 533d883f56 fix(headers): stop overriding no-store with a one year max-age (#7616)
header() replaces by default, so the second Cache-Control left every
authenticated page cacheable for a year.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-08-08 11:29:59 -07:00
Thomas VincentandTheWitness ccfe04a20c Harden remote-agent FCRDNS and basic auth identity (#7324)
* Harden remote agent and basic auth identity handling

* fix(auth): gate forwarded identity headers behind trusted proxies

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* chore: Update CHANGELOG for new feature, and reorder per rules

* chore: Add the issue number to CHANGELOG

* chore: Update hotspots

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-07-21 13:01:33 -04:00
Thomas VincentandTheWitness 53d7962b88 Allow installer to repair missing Composer vendor tree (#7325)
* Allow installer to repair missing vendor dependencies

* Apply project formatting to guarded autoload

---------

Co-authored-by: TheWitness <thewitness@cacti.net>
2026-07-20 15:36:31 -04:00
Thomas Vincent 10fba168a0 Guard startup on missing vendor and repair incomplete trees at install (#7272) 2026-07-13 14:30:46 -07:00
Thomas VincentandTheWitness 364be26ffa chore(test): gate test-only DB short-circuit behind CACTI_TEST_BOOTSTRAP env (#7128)
* chore(test): gate test-only DB short-circuit behind CACTI_TEST_BOOTSTRAP env

The previous `!defined('PHP_TESTING')` guard skipped db_connect_real
whenever PHP_TESTING was set anywhere in the include chain. A stray
define from a CLI tool or third-party script could disable real DB
connection logic in a deployed environment.

Switch the guard to require both `PHP_TESTING` and an explicit
`CACTI_TEST_BOOTSTRAP=1` env var, and replace the silent-no-connect
path with a `Cacti_TestDbSentinel` whose every method throws. Any
production code path that accidentally treats the sentinel as a real
DB handle will fail loudly instead of silently misbehaving.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(test): apply Copilot review feedback for test DB sentinel guard

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: collapse remaining test-bootstrap guard to $is_test_bootstrap

The previous review fix introduced a local $is_test_bootstrap flag but left
two call sites still inlining defined('PHP_TESTING') && getenv(...). Replace
the long-form check and guard the single-poller cacti_db_version fetch so the
sentinel branch finishes initialising without invoking the DB.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* chore(test): arm CACTI_TEST_BOOTSTRAP in test bootstraps and fail closed on PHP_TESTING alone

Set the env var where PHP_TESTING is defined so legitimate test runs short-circuit the DB connect again, gate the config.php.dist swap and schema-validation skip on the combined predicate, read it with local_only, and always seed cacti_db_version.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* ci: ignore phpstan function.alreadyNarrowedType identifier

Clears the level 6 alreadyNarrowedType error on lib/data_query.php:2591 that
develop currently emits, matching the sibling narrowing identifiers already ignored.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-06-24 12:34:16 -04:00
Thomas Vincent 61d8e93592 hardening: standardize input handling with request variable wrappers (#6865) (#6866)
* hardening: standardize input handling with request variable wrappers (#6865)

* fix: use !isempty_request_var() for language check to match !empty() semantics

isrv() (isset) returns true for empty strings, while the original
!empty() returned false. This caused repair_locale() and apply_locale()
to be called with empty language strings.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-03-23 13:07:49 -04:00
TheWitness 82f0204666 Comment out shell lint and coverage steps (#6874)
* Comment out shell lint and coverage steps

We will remove these checks until all the issues indenified by them have first been resolved.

* Disable shell-lint job in quality-extended.yml

Completely remove the step

* Disable spell check job in syntax.yml

Comment out the spelling job in the workflow.

* Fixing Syntax issue in lib/html_graph.php

* Fixing PHPDoc in lib/html_graph.php

* Fixing some style issues in syntax.xml

* Fix more style issues.

* Commenting out actionlint for the moment

* Fixing last issue with html_graph.php

* Fixing issue with auth_profile.php

* Fixing PHPStan issues

* Fix another PHPStan issue

* Fixing Unit Test

* Fix config.php prime step

* Fixing syntax check style

* Going back to old approach

* Moving csrf.php inclusion back to main line for automation

* Fixing issue with create logic

* Fix issue where poller exits with non-zero exit code

* Fix logging only for *NIX

* Fix poller completion detection by reverting code

* Fixing syslog change

* Fixing syslog change
2026-03-21 17:51:52 -04:00
Thomas Vincent a55d72f358 fix: use require_once in plugin loader paths (#6792)
* fix: replace include_once with require_once for core and plugin loader paths

Core library files in include/global.php are unconditionally required for
Cacti to function; a missing file should produce an immediate fatal error
rather than silently continuing with undefined functions. Convert all
include_once calls to require_once.

Plugin setup.php loaders in lib/plugins.php and cli/audit_database.php
already have file_exists() guards at the two sites that need them. Add a
guard + explicit error/redirect in api_plugin_install() where none existed.
Convert include_once to require_once at all three sites so a file that
passes the existence check but fails to load produces a clear error at
the point of failure.

Closes #6788

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: guard header() in install path with is_web check; tighten test assertion

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: convert remaining include_once to require_once in hook dispatch path

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-03-09 09:39:04 -04:00
TheWitnessandCopilot 9d8bf959f5 fix: Fixing Unit tests (#6790)
* fix: Fixing Unit tests

* fix: Include helper remove the defines

* Enable Pest tests for validation

* Update tests/Unit/ScriptsHardeningTest.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update tests/Helpers/CactiStubs.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update tests/Unit/SsHstatsTest.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update rector.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update tests/Unit/HtmlEscapeTest.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* fix: Remove comments that are no longer required

* fix: Move xdebug to the correct location

* fix: Be consistent with includes

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2026-03-07 12:57:00 -05:00
cd7d086384 fix: unify HTTPS detection with cacti_is_https() helper (#6715)
* cleanup: remove unreachable defensive error logs in aggregate graph item validation

* fix: unify HTTPS detection with cacti_is_https() helper

Replace 8 inline $_SERVER['HTTPS'] checks across 4 files with a
single cacti_is_https() helper in lib/functions.php. Handles edge
cases: 'off' (case-insensitive), '0', empty string, and unset.

Add 10 Pest tests covering all HTTPS detection edge cases.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* chore: remove unintended api_aggregate delta from PR scope

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* docs: add header comment to CactiIsHttpsTest explaining consolidation scope

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>
2026-03-06 09:28:44 -05:00
TheWitness 55d9e360b9 Fixing .phpstan.neon for pre-checks (#6754)
* Fixing .phpstan.neon for pre-checks

* Fixing composer issues

* Diagnosing cactid issues

* Diagnosing cactid issues

* Diagnosing cactid issues

* Diagnosing cactid issues

* Fixing issues with new cacti functions

* Re-adding cactid changes

* Switch to the mb function to workaround race condition
2026-03-05 22:40:18 -05:00
3e1dc3c886 fix: replace strtolower/strtoupper with locale-independent wrappers (#6717)
* 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>
2026-03-05 19:51:02 -05:00
d81e573e5b fix: remove PHP_EOL from force_https redirect header (#6659)
* fix: remove PHP_EOL from force_https redirect header

PHP 8.1+ rejects header() values containing \r or \n, which causes
the force_https redirect to silently fail. Remove the trailing
PHP_EOL . PHP_EOL so the Location header is valid.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* Update include/global.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2026-02-25 20:45:35 -05:00
TheWitness dd98bd0368 Remove legacy error reflection masking (#6619) 2026-02-14 11:35:12 -05:00
TheWitness 27919c50d7 Updating Copyrights to 2026 (#6578) 2026-01-28 16:11:10 -05:00
TheWitnessandCopilot b993471579 Fix #6539 - Allow Standard Cacti Log Files to be Rotated on Demand,PHPdoc, and Typing (#6540)
* 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>
2026-01-25 10:13:56 -05:00
TheWitness c0afb6a976 Preparing for phpstan analysis by adding bootstrapFiles 2025-11-18 21:47:55 -05:00
TheWitness 4fc606e9f4 Fix:#6450 - TimeZone display issues
This change simplifies the call and includes the To and From data that is displayed in the legend that was missed.
2025-11-14 09:54:48 -05:00
TheWitness cc19d29cfd Change the path of PHPStan 2025-03-23 16:26:58 -04:00
TheWitness 44bdd44466 Minor Lint Checking Fixes 2025-03-23 12:51:35 -04:00
MSSandTheWitness 724cf74901 Replace strstr(PHP_OS, 'WIN') with str_starts_with(PHP_OS, 'WIN') (#6196)
* Update global.php

str_starts_with(PHP_OS, 'WIN')

* Update global_path.php

str_starts_with(PHP_OS, 'WIN')

* Update global.php

---------

Co-authored-by: TheWitness <thewitness@cacti.net>
2025-03-22 16:54:43 -04:00
TheWitness e5c4015e0e QA: Remove most references to $config
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.
2025-03-22 16:48:59 -04:00
TheWitness 2c779273f9 Copyright Updates and PHP-CS-Fixer updates 2025-03-09 14:34:19 -04:00
Jimmy ConnerandTheWitness 65ab16f5cf Start converting to newer PHP Syntax and Functions (#6103)
* 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>
2025-02-23 15:14:54 -05:00
TheWitness e43e32321d QA: Merge changes from #6024 - Oauth2
* This merges @xmacan's changes and performs additional functions.
* Adds composer.json and composer.lock to include folder
* Places the flag-icons and phpmailer into that file
* Updates flag-icons and phpmailer to latest
* Adds the autoloader.php to global.php
2025-01-08 09:30:07 -05:00
TheWitness 26abeaae79 QA: #1773 - Light Redesign to Allow All Primary Admin Logins 2024-12-27 13:40:11 -05:00
TheWitness be1b1200ce Fix: #1773 - Lock Cacti Install
* This was fixed by introducing a setting to lock the Cacti install to a single user mode from the Support > General page, and only for the primary administrator.
2024-12-22 12:30:37 -05:00
TheWitness 7040f07b07 QA: Fix: #6005 - Undefined variables during install
* This is likely the fix as the warnings, depending on your setup will block the redirect.  If you have display_errors = on (a typical debug mode), you won't be able to redirect.
2024-12-16 13:13:04 -05:00
TheWitness 3af4a43838 QA: Use Cacti Log Level for Remote Data Collectors Not Working
* When you set the Log Level for the Data collector to use Cacti Log level, the remote poller does not respect selective log level properly.
2024-11-30 08:46:59 -05:00
TheWitness fac58e597d QA: Fix #5894 - Cacti Tab per Session Setting
* We were unable to login otherwise the way it was coded.
* Also change CactiTab, CactiTimeZone, and CactiDateTime to session Cookies to prevent clutter
2024-11-01 11:13:36 -04:00
TheWitness 6a283bb569 Fixing #5861 - Add Processes and Queries to Tech Support
* In Cacti Utilities, be able to view the MariaDB/MySQL processeslist and active background tasks
* Restructuring this page to separate the various calls into functions for readability.
2024-10-16 10:54:05 -04:00
TheWitness 42e8ff3510 QA: Improve logging when a page uses get instead of post 2024-10-13 15:12:24 -04:00
TheWitness 5dcda9f670 Fixing Issues #5783 and #5784 - Database Issues
* These issues are to fix an issue encountered when trying to support technologies such as MaxScale readonly and readwrite split connections from the same PHP session.

The first issue is to fix a bug where the main Cacti connection was being closed even when you were trying to close another connection.  The second enhancement is to enable logging of connects, disconnects and the first SQL execution from a connection.

* issue#5783: Cacti improperly handles connection closing leading to issues with third party connections
* feature#5784: Provide a means by which a user can track Cacti connections and disconnects from the php error_log
2024-07-04 19:35:04 -04:00
TheWitness 5f78605e28 Fixing #5723 - Remote poller install does not finish
Remote Poller installation is finished successfully but no "finish" button
2024-04-15 18:28:31 -04:00
TheWitness 4f020d18b2 Minor QA to Browser Command Issues
a56b28e344
2024-02-18 12:11:06 -05:00
TheWitness a56b28e344 QA: Prevent CLI scripts from running under the browser 2024-02-18 11:41:15 -05:00
TheWitness 1aa72d24c9 QA: Update Copyright to 2024 2024-02-03 14:18:57 -05:00