Commit Graph
3606 Commits
Author SHA1 Message Date
TheWitness 0a51c364ce Read graph date range from each wrapper during initialization
Port of #8161 to develop. Reads graph_start and graph_end from each graph wrapper in initializeGraphs(), with empty fallbacks when absent, instead of relying on the shared date-filter globals for every graph.
2026-10-03 13:35:46 -04:00
TheWitness 7b551eea72 chore: normalize CRLF line endings to LF across Cacti-owned files (#8156)
Excludes vendored packages, third-party JS, patches, and the Win32 probe fixture. Pure EOL change; guards pass and php -l is clean.
2026-10-02 17:22:42 -04:00
TheWitness 41e6d9e033 security: consolidate verified GHSA fixes for develop (batch 4) (#8145)
* security: consolidate verified GHSA fixes for develop (batch 4)

Develop counterparts of the 1.2.x batch-4 fixes in #8144, re-verified against
current develop HEAD.

- GHSA-4rmr-wvjq-qxc2: include/auth.php Basic Auth and guest branches fall
  through to the per-page realm check instead of returning early.
- GHSA-j67j-wpm4-9g3x: the pre-dispatch update_policy branch in user_admin.php
  and user_group_admin.php requires a CSRF-token POST. The denylist -> allowlist
  redesign is tracked for 1.3 in #8143.
- GHSA-2p3x-f5jm-rg6p: the View-Graphs -> poller RCE chain is broken by the
  realm check (4rmr), the update_policy guard (j67j) and shell-free webhits (x5pw).
- GHSA-x5pw-cg7j-mj4r: scripts/webhits.pl runs wc through a shell-free list-form open.
- GHSA-9737-rqh3-h7fh: names interpolated into raise_message() now use __esc()
  across the UI; URL- and HTML-bearing messages are excluded.
- GHSA-9x9h-2577-9w86: the colordropdown input is built as a node and the label
  is no longer round-tripped through parseHTML (the list item already used
  createTextNode on develop).
- GHSA-h96j-8xhc-2p3w: remote_agent.php authorizes on $_SERVER['REMOTE_ADDR'],
  not get_client_addr(), which honours client-settable proxy headers.

Regression guards under tests/Unit/Security/{Auth,Csrf,Xss,ShellInjection}.
php -l, check-changelog.py and the sink/architectural baselines all pass.

* chore: reference PR #8145 on the batch-4 1.3.0-dev CHANGELOG entries

* security: resolve develop merge - keep id-only xmlerror unescaped (matches #8141 h745 test)

* security(GHSA-j67j): use REQUEST_METHOD POST guard instead of raw \ (hardening-pattern-guard)

* security(GHSA-9737): fix review findings - drop double-encoding in __esc, escape missed name sinks, correct plugins format-arg count
2026-10-02 13:10:42 -04:00
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
TheWitness 6251fed1dc Make the drop_multi form method use Select2 by default (#8130)
* form_multi_dropdown: default drop_multi to select2, keep legacy opt-in

The drop_multi form method now renders a select2 'N Selected' multi dropdown by default instead of the legacy jquery-multiselect widget, closing a gap left from the earlier select2-multi-count work (html_graph/html_tree already opted in explicitly). A caller keeps the legacy jquery-multiselect widget by including 'multiselect' in its field class; the Scheduler fields used by Automation Networks (lib/api_scheduler.php), Data Source Profiles consolidation functions (global_form.php) and the Settings selective-debug/spikekill pickers (global_settings.php) are marked accordingly so their existing per-page .multiselect() initializers keep working unchanged. Also drops the duplicate class concatenation the old code emitted.

* settings: convert selective debug and spikekill template pickers to select2

Mirror of the 1.2.x conversion. Drop the inline .multiselect() initializers for the Logging-tab Selective File/Plugin Debug pickers and the Spikes-tab 'Graph Templates to Spike Kill' picker, and remove their 'multiselect' opt-in class, so applySkin() renders them as the select2 'N Selected' multi dropdown. Each keeps its wording via per-field select_all_text/select_count_text (the select2-multi-count data-attribute mechanism used for graph_template_id): 'Select File(s)'/'Files Selected', 'Select Plugin(s)'/'Plugins Selected', and 'Select Template(s)'/'Templates Selected'. Empty stays the placeholder text (poller_spikekill.php treats no selection as none).

* settings: align array operators in debug/spikekill defs (php-cs-fixer)

* html_form: legacy opt-in strips select2-multi-count; add drop_multi tests; fix stale docblock

When a drop_multi field carries both 'multiselect' and 'select2-multi-count', the legacy opt-in now wins: the select2-multi-count token is stripped so applySkin() does not also decorate the field and collide with the page's own .multiselect() init. Adds tests/Unit/Ui/Html/FormMultiDropdownSelect2Test.php covering the default injection, legacy opt-in, preserved plugin class, no duplication, and the both-token precedence (either order collapses to the legacy widget). Also drops the stale 'Settings selective debug' example from the helper comment since this PR converts those fields to select2.
2026-10-01 19:53:32 -04:00
TheWitnessandTheWitness d4b8cd3814 fix: align Force Uninstall icon (.piforceremove) on the Plugins page (#8110)
* fix: align Force Uninstall icon by adding .piforceremove to icon-padding rule

The orphaned-plugin (-5) control introduced in #8107 renders a .piforceremove anchor, but that class was missing from the plugin-action icon padding rule, so the icon lost its 0 3px padding and misaligned in the Actions column. Add .piforceremove alongside .pierror in every theme stylesheet.

* fix: align Force Uninstall confirm wording to standard 'Otherwise click Cancel.' pattern

develop counterpart of the #8109 dialog copy fix. develop already uses the non-modal displayDialog() with a plain-text message, so only the closing sentence is aligned to the house pattern (with trailing period). cacti.pot msgid updated to match.

* fix: bump midwinter core.css cache-buster hash to match the updated file

The Force Uninstall (.piforceremove) alignment change edited
include/themes/midwinter/css/media/core.css, changing its content md5 to
afd5bc6f4d45884846ab715f1583123c, but main.css still imported it with the old
?ad4ed321... query string. Browsers holding the pre-change stylesheet in cache
would keep the icon misaligned. Update the import hash so the change is picked up.

---------

Co-authored-by: TheWitness <copilot@users.noreply.github.com>
2026-09-29 16:32:46 -05:00
TheWitness 8a73fc6703 tree: Tabler glyphs for jsTree nodes with per-type theme colors and glyph carets (#8101)
* tree: replace jsTree node PNGs with Tabler glyphs, per-type theme colors, and glyph open/close carets

* tree: center carets to 20px node row, remove indent dotted lines, add dots:false to editor source tree

* tree: center jsTree carets with flexbox so alignment is glyph-height independent

* tree: vertically center node glyph with its label text (vertical-align middle)

* tree: preserve jsTree loading throbber and add default folder glyph

Scope the .jstree-ocl reset and caret glyph content to :not(.jstree-loading) so AJAX expansion keeps its loading indicator, and register a 'ti ti-folder' glyph (with per-theme color) for the untyped 'default' branch type so branch nodes are not left with a blank icon.
2026-09-29 10:42:29 -04:00
TheWitness e29581b68a Make package/diff viewer colors theme-aware (#8093)
* Make package/diff viewer colors theme-aware

The jfcherng/php-diff Inline renderer output is styled by each theme's
Diff.css, but every theme shipped an identical copy that hard-coded a
light palette (color: black, background: #fff, opaque #fbe1e1/#e1fbe1
add/remove tints). Inside the diff dialog this painted a light box with
dark text over dark themes (dark, sunrise, paper-plane) and over
midwinter's dark color-mode, so the modal did not match the theme.

Derive the base surface from the themed dialog instead of hard-coding it
(color: inherit, background: transparent, neutral translucent borders and
gutters) and express the add/remove/replace highlights as translucent
rgba overlays that read correctly on both light and dark surfaces. Light
themes render essentially as before; dark themes now inherit their dark
surface and light text. Applied to every theme so the shared stylesheet
stays identical.

* Band diff code columns on dark themes

On the dark themes the unchanged (context) code column and the
line-number gutter read too similarly, making the actual diff on the
right hard to pick out. Give the context old/new code cells a subtle
dark band (rgba(0,0,0,.25)) so the code reads as a panel distinct from
the lighter line-number gutter. Applied to dark, sunrise and paper-plane;
for midwinter the band is scoped to its dark color-mode only. Light
themes are unchanged.
2026-09-28 15:21:51 -04:00
TheWitness 46908e350b fix(ci): pin htmx.org back to 2.0.10 to match vendored asset and SRI (#8091)
* fix(ci): pin htmx.org back to 2.0.10 to match vendored asset and SRI

The npm group bump #8085 raised htmx.org to 4.0.0, but the vendored
include/js/htmx.js, the HTMX_2_0_10_SRI integrity constant, and the
htmx.js.version file were not regenerated. CI's 'Build managed JavaScript'
step (build/sync-js.mjs) copies node_modules/htmx.org/dist/htmx.js, so it
shipped 4.0.0 while htmx_script_tag() still emitted the 2.0.10 SRI, failing
tests/Unit/Ui/Html/HtmxTest. Revert htmx.org to the vendored/pinned 2.0.10;
a real htmx 4.x upgrade should be a separate, tested PR.

* fix(htmx): sync version file, tests and docs to vendored 2.0.10
2026-09-28 15:02:18 -04:00
MSS 66813e8b01 WEBUI WARNING: Key Cacti Include File "select2.js" missing (#8079)
WEBUI WARNING: Key Cacti Include File "select2.js" missing. Please locate and replace this file as we checked in "/var/www/cactiuat/include/js/"
2026-09-27 09:21:56 -04:00
TheWitness a52368125e feat(auth): rename User Domains to Login Providers, add SAML2 and OpenID Connect (#8066)
* feat(auth): rename User Domains to Login Providers

Collapses user_domains/user_domains_ldap into a single login_providers table with a JSON parameters column, migrated in place during the 1.3.0 upgrade. Renames the admin page, menu, breadcrumbs, and help links to match.

* feat(auth): rework login providers as Cacti\Auth classes

Introduces LoginProviderInterface with credential (LDAP/AD) and redirect (SAML2/OpenID) flavors, replacing the procedural domains_login_process()/domains_ldap_*() functions. Adds blank-field group membership gating shared by every provider type.

* feat(auth): add SAML2 and OpenID Connect login providers

Adds onelogin/php-saml and firebase/php-jwt, a new cacti_http() SSRF-hardened outbound request helper for every OpenID discovery/token/JWKS call, and login_sso.php to handle the redirect-based SSO flow. Login page now shows a button per enabled SAML2/OpenID provider, and per-provider auth cookie opt-out is enforced on both the password and SSO login paths.

* chore(i18n): regenerate cacti.pot

* feat(auth): add Local and Basic auth as Cacti\Auth providers

LocalAuthLoginProvider (CredentialLoginProviderInterface) and BasicAuthLoginProvider (new PreAuthenticatedLoginProviderInterface, since Basic Auth has no password for Cacti to check) are thin adapters over the existing local_auth_login_process()/basic_auth_login_process() functions, which are left untouched to avoid disturbing their regression-tested lockout/timing/rehash behavior.

* feat(auth): move local password verification into LocalAuthLoginProvider

secpass_login_process() and local_auth_login_process() are retired; their lockout, constant-time verification, forced-password-change, and rehash-on-login logic now live end to end in the class (verifyCredential/enforcePasswordPolicy/rehashIfNeeded). secpass_check_pass() stays a shared lib/auth.php function since password change/reset/user_admin also call it outside any login flow. Local auth still has no login_providers row: isEnabled() is always true and it cannot be disabled or deleted.

* feat(auth): move Login Provider CRUD into Cacti\Auth classes

AbstractLoginProvider gains static deleteById()/enableById()/disableById()/makeDefaultById()/persist(); each concrete provider owns its own collectParameters(), dispatched via LoginProviderFactory. login_providers.php's form_save()/form_actions() call these instead of doing raw SQL, and the retired provider_remove()/provider_disable()/provider_enable()/provider_default()/provider_collect_parameters() procedural functions are gone. Local/Basic auth are unaffected: they never extend AbstractLoginProvider, so they structurally cannot expose delete/disable.

* fix(auth): address Copilot review findings on SSO/SAML/OIDC security

Fixes callback/ACS/SLS/metadata URLs that used the raw provider id instead of the login realm (broke every SSO login), adds InResponseTo correlation for SAML and sub/azp validation for OIDC, requires HTTPS for OIDC discovery/endpoints, pins cacti_http() connections against DNS rebinding via CURLOPT_RESOLVE, exempts login_sso.php from the authenticated-page redirect guard and gates it on auth_method, wraps provider dispatch in a exception boundary, adds a dedicated SAML SLO path instead of routing it through login completion, makes remember-me an explicit opt-in for SSO, fixes disabled/deleted providers not revoking existing auth cookies, restricts the default-realm action to LDAP/AD, and fixes the 1.3.0 migration silently enabling group enforcement for installs that had it off. Also updates the E2E seed script for the login_providers schema.

* chore(auth): refresh CI security baselines and document no-op validation

Regenerate the sink inventory baseline for the new/renamed lib/Auth/* and login_providers.php files, and add '// no-validation:' comments on the intentionally-unconstrained admin/free-text/secret fields (LDAP DNs/filters, SAML certs/keys, OIDC client credentials, provider display fields) flagged by the validator constraint ratchet.

Refreshes the validator baseline to drop the deleted user_domains.php entries.

* refactor(ldap): remove dead DB-lookup path from Ldap::__construct

The domain_id>0 branch duplicated the JSON-parameter-to-property mapping already owned by LdapLoginProvider::buildLdap(), and nothing constructs Ldap with a real id anymore (all call sites pass 0). Drop the parameter and the branch so Ldap stays a pure connection/protocol class with no DB coupling.

Also fixes a latent bug: group_require is a typed bool property, but LdapLoginProvider::authenticate() assigned it an int (1/0) from a strict_types=1 file, which throws a TypeError on every LDAP/AD login attempt. Assign a real bool instead.

* fix(auth): fix LDAP login, upgrade ordering, and hardening guard failures

LdapLoginProvider compared LdapError::GetErrorDetails()'s int error_num against the string '0' with strict equality, so it never matched and every LDAP/AD login was rejected regardless of valid credentials. Compare against int 0 instead.

install/upgrades/1_3_0.php called login_providers_convert_1_3_0() (which reads and then drops user_domains/user_domains_ldap) before the pre-existing legacy ALTER TABLE statements that add the debug/network_timeout/bind_timeout columns those reads depend on, so upgrades failed once the tables were already gone. Moved the call to run after those ALTERs.

OpenIdLoginProvider::complete() read state/code from raw \ instead of gnrv(), tripping the hardening pattern guard; switched to the request helper.

* style(auth): apply php-cs-fixer formatting and normalize line endings to LF

Runs php-cs-fixer across the new lib/Auth/* classes, login_providers.php, login_sso.php, and the other touched files, fixing indentation/alignment/docblock style to match project convention. Also normalizes several files that had picked up CRLF line endings back to the LF the project's .php-cs-fixer.php config (setLineEnding) and the rest of the tracked tree actually use.

* fix(auth): address second Copilot review round (9 high/medium findings)

- Make login_providers_convert_1_3_0() resumable: CREATE TABLE IF NOT EXISTS and INSERT ... ON DUPLICATE KEY UPDATE so an interrupted upgrade can be re-run instead of leaving a half-migrated, unrecoverable table.

- SamlLoginProvider::complete() now fails closed when no saved AuthnRequest ID exists instead of passing null to processResponse(), which skipped InResponseTo validation and accepted unsolicited IdP-initiated assertions.

- LoginResult gains an isCredentialFailure flag; only a genuine wrong-password LDAP bind (LdapError::Failure) counts toward the lockout counter now, not connection errors or group-membership denials.

- Fixed button_label/debug being read from the JSON parameters column in OpenIdLoginProvider/SamlLoginProvider/LdapLoginProvider when they are actually top-level login_providers columns; AbstractLoginProvider now hydrates and exposes both.

- Removed SamlLoginProvider's wantMessagesSigned, which silently required the outer SAML message (not just the assertion) to be signed, rejecting IdPs that only sign the assertion as the UI promises.

- Added getTemplateUserId() to CredentialLoginProviderInterface and implemented it on LocalAuthLoginProvider so the contract (not just incidental AbstractLoginProvider inheritance) guarantees the method lib/auth.php calls.

- cacti_http()'s CURLOPT_RESOLVE entries now bracket IPv6 literals correctly, and safe-IP validation strips parse_url()'s brackets before checking, fixing IPv6 target pinning.

- login_providers.php's type dropdown now swaps the Full Name attribute default (cn/displayName) on an actual user-driven type change, not on page load, so a new Active Directory provider does not silently save the LDAP default.

- Extracted auth_apply_group_login_opts() and call it from both auth_login.php and login_sso.php, so SSO logins honor a user's group-derived login_opts the same way password logins already do.

* fix(auth): fix PHPStan level 8 errors and pest harness stub drift

Test harness fix: the LoginProvidersLoginRealmValidationTest stub LoginResult class was missing the isCredentialFailure property added to the real class, causing 'Undefined property' warnings that broke the harness's JSON output. Added the property and set it on the simulated bad-password failure.

PHPStan (48+ errors, all newly introduced by this branch's rewrite, never actually verified until now): narrowed db_fetch_row_prepared()/db_fetch_assoc()'s array|bool (including bare true, which survives a '?: []' short-circuit - fixed with explicit is_array() checks instead) at every call site across auth_login.php, login_sso.php, lib/auth.php, lib/Auth/LocalAuthLoginProvider.php, lib/Auth/LoginProviderFactory.php, and install/upgrades/1_3_0.php.

Fixed OpenIdLoginProvider::discover()'s return shape to match its declared docblock instead of returning the raw decoded JSON. Fixed lib/functions.php's cacti_http() to narrow curl_exec()'s string|bool result with is_string() (not ===false, which still leaves 'true' as a residual type) and guard / as non-empty for curl_setopt_array()'s stubs.

Fixed a real bug in SamlLoginProvider::processLogout(): processSLO(true) was passing 'true' as \ (the first parameter) instead of \ (the fifth), so it always redirected/exited internally with the default \=false, making Cacti's own logout.php never regain control. Now calls processSLO(keepLocalSession: true, stay: true) and is typed ': never' to match; login_sso.php's now-genuinely-unreachable break statements after processLogout()/initiate() are removed.

Fixed login_providers.php passing a expression (not a variable) to inject_form_variables()'s by-reference parameter.

Verified: PHPStan analyse clean (0 errors), full Pest Unit/Security suite passes (128 tests), hardening guard/sink inventory/validator ratchet all still clean.

* fix(auth): fix ambiguous column and array+bool crash in user_log filter

The user filter dropdown query selected a bare 'id' column after joining user_auth and login_providers, which is ambiguous now that login_providers also has an id column (unlike the old user_domains.domain_id) - MySQL rejected the query outright. Qualified as ua.id.

Also narrow db_fetch_assoc()'s array|bool result before the \ + \ + \ merge, which was throwing a TypeError whenever the query failed instead of surfacing the real SQL error.

* fix(auth): address third Copilot review round (5 findings + tests)

- AbstractLoginProvider::persist() now disables the template account for every provider type; SAML2/OpenID also copy it via user_copy() in login_sso_complete() and were left directly usable as a local login.

- cacti_http_resolve_safe_ips() now detects IPv4-mapped IPv6 literals (::ffff:0:0/96) via inet_pton() and validates the embedded IPv4 address, closing an SSRF bypass via e.g. ::ffff:169.254.169.254.

- login_providers_convert_1_3_0() now tracks db_install_execute()'s status for every row and only drops the legacy user_domains/user_domains_ldap tables once every row (and the final row count) is confirmed migrated.

- Ldap::Getcn() now looks up ldap_get_entries()'s always-lowercased attribute keys case-insensitively while still storing the result under the originally-requested key, fixing AD's displayName (and any other mixed-case configured attribute) always resolving to a blank full name.

- SamlLoginProvider::processLogout() now redirects to the LogoutResponse URL processSLO() returns for a LogoutRequest (stay:true), instead of discarding it and always redirecting to Cacti's own logout page - federated logout was never actually acknowledged to the IdP.

- Added tests/Unit/Security/Auth/OpenIdLoginProviderTest.php (13 tests) covering verifyIdToken() signature/issuer/audience/azp/nonce validation, discover()'s HTTPS enforcement, and complete()'s callback-state and UserInfo-subject-matching checks, against a locally-generated RSA keypair/JWKS.

- Regenerated the sink inventory baseline for the new header_redirect call in SamlLoginProvider::processLogout().

Verified: PHPStan clean, full Pest Security/Auth suite passes (830 tests including the 13 new ones), php-cs-fixer/hardening-guard/sink-inventory/validator-ratchet all clean.
2026-09-24 17:51:18 -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
TheWitness f956ea5a5c Convert generic <select> elements to select2(), with new class helpers (#8053)
* feat: convert generic <select> elements to select2, with new class helpers

Replaces the jQuery UI selectmenu() catch-all in setSelectMenus() with select2,
applied uniformly to single and multi selects. Fewer than 10 options disables
the search box (minimumResultsForSearch: Infinity); 10 or more enables it -
same threshold now applied to the existing select2/select2-multi classes.

New layout.js class helper: select2-multi-count, a multi-select whose
selection summary reads "N Selected"/"All Selected" instead of one chip per
option (generic multiSelectAllText/multiSelectCountText strings in html.php,
overridable per-field via data-select-all-text/data-select-count-text/
data-select-all-value). graph_template_id (html_graph.php x2, html_tree.php)
now uses it in place of the jquery.multiselect-based .graph-multiselect,
preserving the existing "All Graph Templates"/"N Templates Selected" wording
and the -1 "all" sentinel behavior (reimplemented via select2:select/unselect
events in setSelectMenus()). The old .graph-multiselect jquery.multiselect
init block is left in place (now unreachable for core's own markup) in case
a plugin/theme still emits that class itself.

select2-callback is reworked from its previously-unused static-URL contract
into an ajax-backed replacement for the legacy .drop-callback/makeCallbacks()
autocomplete widget: data-action + data-variables build the query the same
way makeCallbacks() already did (pageName + action + live values of other
named fields + search term), and data-callback still names a JS function to
invoke on selection, matching form_callback()'s existing $on_change/$class
contract. form_callback() only renders the new <select> markup when its
$class parameter contains select2-callback (opt-in); every existing caller
that doesn't request it keeps the exact legacy output. html_graph.php's
host_id filter (both places) opts in. form_multi_dropdown() gained optional
$select_all_text/$select_count_text/$select_all_value parameters (all
additive/backward compatible) to plumb the graph_template_id wording through.

Compatibility: an org-wide code search found no plugin depending on the
.select2-callback/.graph-multiselect/.drop-callback/makeCallbacks() internals
being changed here (form_callback()'s default output is unchanged; the
.graph-multiselect jquery.multiselect init stays available). Everywhere else
in core that assumed the old jQuery UI selectmenu widget on elements now
covered by the broadened catch-all (disableField()/enableField(), the SNMP
security-level/protocol refresh logic, realtime.js's ds_step/graph_start/size
refresh, api_aggregate.php's field enable/disable, and html_reports.php's
branch_id/site_id/host_template_id handling) now branches on
.select2-hidden-accessible and uses select2's own refresh mechanism
(.trigger('change.select2')) instead, alongside the untouched legacy
selectmenu branch. #user_language and #i18n_default_language (which get
their own bespoke flag-icon languageselect widget) are excluded from the
catch-all, same as before.

The install wizard (install/install.js) has its own independent selectmenu
initialization and is intentionally left untouched.

* fix: use json_encode for select2 multi-select JS translation strings

* fix: make select2 initializers idempotent to stop duplicate widgets

applySkin() (which calls setSelectMenus(), where every select2/select2-multi/
select2-multi-count/select2-callback/catch-all initializer lives) runs more
than once per page view - once from cactiReady() on initial load, and again
from the generic AJAX-completion handler after any loadUrl()/postUrl() call,
which most list/filter pages already trigger during their own initial load.

Since calling .select2() a second time on an element that already has it
initialized creates a second, independent .select2-container next to the
first (select2 does not no-op or tear down the previous instance), every
affected <select> ended up rendering twice. Guard every initializer (plus
the graph_template_id -1 sentinel's event bindings, now namespaced and
unbound first) so a second setSelectMenus() pass skips elements already
marked .select2-hidden-accessible.

* fix: match select2 dropdown styling to the dark theme's real button look

border-radius was 4px (should be 6px, matching .ui-corner-all used
everywhere else); the selection box and dropdown panel were flat black
instead of this theme's actual .ui-button/.ui-widget-content backgrounds
(a subtle gradient-highlight image and a repeating texture image,
respectively). Also add a min-width and force the selected text/dropdown
option list to a single line instead of wrapping.

* fix: match select2 dropdown styling to each theme's real button look

Same fix as the earlier dark theme commit, applied to the remaining themes.
Every theme's select2 override previously used generic placeholder colors
(border-radius: 4px, background: #ffffff/#222222) that did not match that
theme's actual .ui-state-default/.ui-button/.ui-widget-content styling:

- cacti, carrot, hollyberry, raspberry, modern, paw: selection box now uses
  the real button gradient-image background instead of flat white; dropdown
  panel background already matched and is unchanged.
- sunrise: selection box background corrected to transparent (matching its
  real button style) and the dropdown panel to its real black background,
  instead of flat white for both.
- paper-plane: selection box background corrected to this theme's real
  near-black button color instead of flat white.
- midwinter: switched from hardcoded generic colors to this theme's own
  CSS custom properties (var(--border-color), var(--background-content),
  var(--text-color-lighter), etc.), matching jquery-ui.midwinter.css.

border-radius is now 6px everywhere, matching .ui-corner-all. All themes
also gained a min-width and forced the selected text/dropdown option list
onto a single line instead of wrapping.

* fix: default select2's dropdownAutoWidth to true

The legacy jQuery UI selectmenu widget always sized its open dropdown to fit
the widest option, regardless of how narrow the closed control itself was.
Select2 defaults to matching the dropdown's width to the control's, so wide
option text was wrapping/truncating inside a narrow panel. Set this as a
global default once instead of touching every individual select2() call.

fix: destroy any existing select2 widget before re-initializing

applySkin() runs more than once per page view. The existing
:not(.select2-hidden-accessible) guards were meant to make re-running the
select2 initializers a no-op, but duplicate widgets were still being
reported. Explicitly tear down any select2 instance still attached right
before the initializers run again, so a widget can never end up duplicated
regardless of why applySkin() re-ran.

fix: don't select2-ify a select a plugin already widget-ified with selectmenu

Symmetric to the above: if a plugin's own script already deliberately
applied jQuery UI selectmenu to one of its selects (a few do, for their own
dynamically-created fields), skip converting it to select2 too.

* fix: convert the Device filter's legacy drop-callback widget to select2-callback

html_host_filter()'s autocomplete-enabled branch never got the select2
treatment applied to every other filter field - it still built its own
<input class='drop-callback'> wired up by the older makeCallbacks()/jQuery UI
autocomplete widget (styled with the legacy ui-selectmenu-button markup),
instead of opting into form_callback()'s newer select2-callback rendering like
everything else. Left side-by-side with now-modernized neighboring filters,
this old widget stood out as a leftover "old select". Switch it to the same
select2-callback select markup (data-action='ajax_hosts',
data-variables='site_id') the 1.2.x branch already uses, reusing the existing
ajax_hosts handlers and get_allowed_ajax_hosts()'s id/label response shape.

* fix: restore select2's gradient after closing without selecting, shrink its height, and drop the now-unneeded min-width

Every theme's active/open select2 style also matched .select2-container--focus,
but select2 leaves that class on the container after the dropdown closes (it
only drops --open), so the flat "active" background stuck around until the
next selection instead of the gradient returning right away. Scope the active
look to --open only.

Also shrink the single-select control from select2's stock 28px down to 24px
(and its arrow/rendered text to match) so it sits closer to the jQuery UI
button height it's replacing, and drop the min-width: 160px placeholder added
earlier to keep wide option text from being clipped - dropdownAutoWidth now
sizes the open dropdown to its content regardless of the closed control's
width, so the constraint is no longer needed.

* feat: add configurable select2 search threshold; drop autocomplete_enabled on develop

Add the same Select Search Minimum Rows setting added on 1.2.x, wiring it
through as the select2SearchRows JS global (include/global_session.php) and
replacing the hardcoded 10-option cutoff in every select2 initializer in
layout.js.

Remove Autocomplete Enabled entirely on develop: it only ever existed to let
the Classic theme opt out of the ajax-backed callback widgets, and Classic
doesn't exist as a develop theme. html_host_filter() and the Graph Items data
source field now always use the ajax-backed rendering (select2-callback /
drop_callback) unconditionally instead of branching on the removed setting.

* fix: keep the first 20 select2 results visible before scrolling

Select2's stock results list only guaranteed ~7-8 rows (200px) before
scrolling. Pin each result row to a consistent 24px (matching the shrunk
control height) and size the list's max-height to fit 20 of them before the
scrollbar kicks in.

* fix: repair layout.js corruption from the select2SearchRows commit, and exclude .multiselect from the catch-all

A faulty automated edit in 3cbc5a75 (feat: add configurable select2 search
threshold) matched the wrong text in 4 places and clobbered unrelated code
instead of the intended minimumResultsForSearch lines:
 - deleted the fa-remove icon-class swap and mangled the `if (!theme)` check
 - deleted setupPageTimeout()/CsrfMagic.end()
 - left a stray bare `select2SearchRows` statement before the themeReady() call
 - deleted the #password/#password_confirm delayKeyup wiring and broke the
   showPage deferred's closing brace

Restored all four to their pre-corruption behavior and applied the intended
select2SearchRows threshold to the actual 4 select2 init blocks this time
(select2, select2-multi, select2-multi-count, and the generic catch-all).

While in there: the catch-all's '.multi-select'/'.graph-multiselect'
exclusions never matched anything real - form_multi_dropdown() always tags
its <select multiple> with plain 'multiselect', so every field using the
older jquery.multiselect plugin (Automation's day/month pickers, Consolidation
Functions, Selective Debug, etc.) was getting select2-ified on top of its own
dedicated multiselect() init. Exclude '.multiselect' instead.

* feat: checkbox-styled option rows for select2-multi-count dropdowns

Gets closer to the old jquery-multiselect look the count-style multi-selects
(Graph Templates, etc.) replaced: each option row in the open dropdown now
shows a checkbox-style indicator instead of plain text, driven by
[aria-selected] via CSS so it stays in sync with select2's own selection
state without any extra JS bookkeeping. closeOnSelect stays off so multiple
options can still be picked without the dropdown closing between clicks; the
closed control still shows the "N Selected"/"All Selected" summary text.

* fix: match multi-select control height to single-selects and add its missing dropdown arrow

select2's stock multi-select box only had a min-height with no matching
override, so it stayed taller than every single-select control next to it;
select2-multi-count fields never actually show real per-option chips (the
rendered area always gets replaced with "N Selected"/"All Selected" text),
so it's safe to tighten this to match.

select2 also only renders a .select2-selection__arrow element for
single-selects - multi-selects (Templates, Days of Week, etc.) got none by
default, so they looked handle-less next to every single-select control. Add
a matching CSS-only arrow via ::after (flips on select2-container--open,
same as the single-select's).

* fix: stop select2's multi-select inline search box from collapsing to near-zero width

Select2 sizes the inline search input it embeds in .select2-selection__rendered
by measuring the box's natural layout; my earlier height-matching CSS
(zeroed padding, a fixed line-height, and extending the single-select's
overflow:hidden/text-overflow:ellipsis truncation to multi-selects too) broke
that measurement, so the search field rendered as a barely-visible sliver
sitting inline next to the "N Selected" text instead of expanding to a usable
width.

Back off to a lighter touch: only nudge min-height down a little (no padding/
line-height overrides), and keep the overflow/ellipsis truncation scoped to
single-selects, which is where it was actually needed (short "N Selected"/
"All Selected" labels on multi-selects don't need it and it wasn't there
before this session's changes).

* fix: hide the inline search box on select2-multi-count controls

Reducing padding/line-height (previous attempt) didn't stop the box from
showing - select2's inline search-while-typing input for multi-selects still
rendered as a tiny stray rectangle next to the "N Selected" text and kept
inflating the control's height, since that input's own margin/height are
part of the rendered area's natural content flow regardless of the parent's
box model.

select2-multi-count fields always replace the rendered area with summary
text and rely on checkboxes in the open dropdown for picking options, so the
inline search box serves no purpose here - hide it outright via the adjacent-
sibling selector (select.select2-multi-count + .select2-container), which
also lets the control's height settle back down to match single-selects
without needing to fight select2's own sizing math.

* fix: let select2-multi-count's search box work again once opened; fix vertical alignment

Hiding the inline search box outright also removed the only way to search a
multi-select's open dropdown (multi-select mode has no separate dropdown-only
search box - the inline one filters the open list too). Scope the hide to
:not(.select2-container--open) instead, so it stays out of the way while
closed but reappears once the dropdown is open.

Also wrap the "N Selected"/"All Selected" label in its own span
(select2-count-label) instead of writing bare text into
.select2-selection__rendered, so its line-height/truncation can be
controlled independently of whatever else (the search box) shares that
container - this was causing the text to sit off-center vertically.

* fix: keep the multi-select control's height fixed when its search box opens

Even scoped to select2-container--open, the inline search <li> was still
sitting inside .select2-selection__rendered as a normal block-level sibling
of the "N Selected" label span, so it pushed the label onto its own line and
grew/shifted the whole control upward whenever the dropdown opened - visible
proof the search box was structurally part of the "selection" element, not
just visually overlapping it.

Take it fully out of flow with position: absolute (sized to the control via
top/left/right/height: 100%, overriding select2's own inline width/height on
the actual <input>), so it can never affect the parent's layout regardless of
when it's shown. Also switch the control to flexbox with align-items: center
for reliable vertical centering of the label instead of a guessed
line-height, and give it the same 8px left inset the single-select's text
already has.

* fix: stop the multi-select's overlaid search field from painting over the label

The generic .select2-search--inline .select2-search__field rule gives search
boxes a solid background (matching the dropdown panel's search box, where
that's correct) - but once resized to fill the whole control via absolute
positioning, that same solid background completely hid the "N Selected"
label behind it while the dropdown was open. Make it transparent/borderless
instead, and hide the label (visibility, not display, so layout doesn't
shift) while open so there's no overlapping text underneath the cursor.

* fix: put the multi-select's search box on its own row instead of overlapping the selection

Overlaying it via position: absolute solved the layout-corruption bug but
traded it for a worse one - it now hid the "N Selected" label completely.
Let it flow normally below the label instead (its own full-width block, small
top margin for separation), matching how a search box should actually behave
- visible on its own line, not fighting the label for the same space. The
container only grows taller while the dropdown is actually open, since the
search row is hidden entirely while closed.

Also reset the stray margin select2 puts on #select2-<id>-container (the
same element as .select2-selection__rendered) - it was inflating the closed
control's height too.

* fix: multi-select search box wraps below label; vertically center label text

The row-direction flex box for select2-selection--multiple had no
flex-wrap, so once the search field was added as a second full-width flex
item its own line-item competed with the label item in the same row -
squeezing/overlapping both instead of the intended label-then-search
stacked layout. flex-wrap: wrap lets the label take row 1 and the
full-width search field naturally wrap to row 2, only while open.

Also give the label an explicit line-height matching the control's height
(same technique already used for the single-select boxes) - flex centering
alone left the text sitting a few pixels high due to the font's default
line-height metrics.

* fix: search select2-multi-count from the dropdown, not an inline box in the control

Root cause of the open-state height jump: select2 gives every multiple-select
a type-to-filter <input> INSIDE the selection control (its SelectionSearch
adapter). When the dropdown opens that input appears, growing the control and
pushing the surrounding filter rows down. No amount of CSS on an in-flow child
of the control avoids that - the search genuinely lives inside the control's
box.

Single-selects don't have this problem because their search lives in the
floating dropdown panel (DropdownSearch adapter), which is body-attached and
never affects page layout. So give the multi-count controls the same wiring:
a custom dropdownAdapter (Dropdown + DropdownSearch + MinimumResultsForSearch
+ AttachBody) that searches from the panel, and a selectionAdapter of just
MultipleSelection + EventRelay with no inline search box at all.

The control now keeps a fixed height whether open or closed, and the surrounding
form no longer shifts. Guarded so it falls back to default behavior if the
select2 AMD modules aren't reachable.

* fix: show 15 select2 dropdown rows before scrolling instead of 20

* fix: actually apply the multi-count dropdown-search adapters + trim control padding

Two bugs behind the "search still looks attached to the (green) control and the
control grows when open" report:

1. The adapters were required with the ARRAY form of select2's amd.require(),
   which almond defers via setTimeout - so multiCountDropdownAdapter/
   multiCountSelectionAdapter were still undefined when the init loop ran, and
   select2 silently fell back to its default inline search box inside the
   control. Fetch each module with the synchronous string form instead so the
   adapters are actually in place at init time. The search now lives in the
   floating dropdown (black), not inline in the green open-state control.

2. Vendor select2.css's .select2-container--default .select2-selection--multiple
   adds padding-bottom:5px (equal specificity, so it wasn't being reset), which
   left the count label sitting against the top of the control with a gap below.
   Set explicit padding: 0 20px 0 0 so the flex-centered label is truly centered
   and the closed control matches the single-select height.

* fix: size select2-multi-count control/dropdown to content, not the native listbox width

* fix: reload graphs after the Template select2 fully closes, not mid-teardown

* docs: track pending select2 upstream dropdown-search PR for future cleanup

* docs: link the actual select2 upstream PR (#6462)

* fix: sync checkbox visuals for graph_template_id's -1/others exclusivity; skip reload if nothing changed

Two bugs in the "All Templates" (-1) mutual-exclusivity sentinel:

1. Selecting -1 didn't visually uncheck the other checkboxes, and vice versa.
   The old code flipped the underlying <option>.selected directly and fired a
   plain 'change' event - but select2's own 'change' listener only refreshes
   the "N Selected" summary label, not the open dropdown's rendered results
   list. The checkbox marks are drawn from aria-selected on those already-
   rendered <li> elements, which never got touched. Fixed by routing through
   select2's own internal event bus (available via $el.data('select2')) with
   'select'/'unselect' triggers instead - the exact same path a real click
   takes, which also runs Results.setClasses() to redraw the checkboxes.

2. Opening the dropdown and closing it again without changing anything used
   to still reload the graphs. Now the selected value is snapshotted on open
   and compared on close; the reload only fires if it actually changed.

* fix: force checkbox refresh in graph_template_id sentinel to fix off-by-one visual lag

Reported symptom: click template T1 while "All Graphs & Templates" is
checked - the "All" box unchecks correctly, but T1 does not visually check.
Click T2 next - T1 (not T2) now shows checked. Click T3 - T2 shows checked.
The checkbox visual was always exactly one click behind the actual selection.

Rather than keep chasing the precise internal event-ordering that causes
this (select2's DataAdapter/Results adapters both listen to the same
internal 'select'/'unselect' events our sentinel re-triggers), force a
direct, unconditional call to the Results adapter's own setClasses() method
(exposed publicly as $el.data('select2').results) immediately after our
selection changes. This guarantees the open dropdown's checkboxes reflect
the current underlying <option> selected state before the handler returns,
independent of dispatch ordering.

* debug: temporary console diagnostics for graphTemplateSentinel checkbox lag

* fix: checkbox checkmark keys off --selected class, not aria-selected (hover was stealing it)

Root cause of the "checkbox always one click behind" bug, finally confirmed
via console diagnostics: our checkbox checkmark CSS keyed off the
aria-selected attribute, but select2's own results.js reuses that exact same
attribute for a second, unrelated purpose - tracking which row is currently
mouse-hovered/keyboard-highlighted (a legitimate ARIA listbox pattern). Every
time the mouse moves onto a new row, select2 clears aria-selected on whatever
row was previously "highlighted" and sets it on the newly-hovered row,
completely independent of actual selection state.

Since a user naturally moves the mouse toward the next item immediately after
clicking the current one, this hover-tracking code was stealing
aria-selected="true" away from the just-selected row and handing it to
whatever's now under the cursor - producing exactly the observed "previous
box shows checked, current one doesn't" pattern. Confirmed via console
logging: aria-selected was momentarily correct right after each click (when
our own forced setClasses() call ran), then immediately got reassigned by the
very next mouseenter as the user moved toward their next target.

Fix: key the checkmark off the select2-results__option--selected CSS class
instead. That class is exclusively added/removed by setClasses() based on the
real underlying <option>.selected state, and is never touched by the
mouseenter/highlight-tracking code - so it can't be hijacked by hovering.

Also removes the temporary console.log diagnostics added while tracking this
down; the forced setClasses() calls in the sentinel are kept as harmless extra
insurance.

* chore: apply php-cs-fixer alignment fixes

CI's php-cs-fixer dry-run flagged 4 files for pure whitespace/alignment
drift introduced by the select2 conversion: array `=>` operators and a
docblock's @param columns need to align to the widest key/name in their
respective blocks, and two inline PHP print tags were missing the space
before `?>`. No behavioral change.

* fix: address Copilot review findings on select2 conversion (develop)

- lib/html_form.php: form_callback()'s select2-callback opt-in path used
  htmle() (double_encode=false) for the select's id/name/class, its
  data-action/data-variables/data-callback attributes, and the
  pre-selected option's value attribute. A pre-encoded entity in any of
  these caller-supplied values would survive into the attribute and be
  decoded back into markup/quotes by the browser. Use html_escape_attr()
  (double_encode=true) for all of these attribute contexts; the option's
  text content stays on htmle() since that's body text, not an
  attribute. Same fix for form_multi_dropdown()'s three
  data-select-all-text/data-select-count-text/data-select-all-value
  attributes.
- lib/html_reports.php: changeTree()'s replaceWith(data) on #branch_id
  only swaps the <select> element itself, leaving a live Select2
  instance's sibling .select2-container orphaned in the DOM. Destroy the
  existing Select2 instance first so the re-initialization a few lines
  down doesn't produce a duplicate branch control.

include/layout.js's select2-callback CSRF finding: verified this is not
an actual bug. select2 4.0.13's AjaxAdapter.query() calls
options.data.call(...) and hands the result straight to $.ajax() via the
default transport; for type: 'post' with a plain-object `data` and no
processData:false/contentType override, jQuery serializes it as the POST
body via $.param(), and Cacti's csrf_check() reads the token via
filter_input(INPUT_POST, ...). The __csrf_magic data() callback already
present here (since the PR's first commit) does get sent correctly. No
global $.fn.select2.defaults override clobbers it. Replied with this
analysis rather than changing working code.

Verified: php -l on both changed PHP files; brace-balance check on
lib/html_reports.php (295 open / 295 close).

* fix: address remaining Copilot review findings on select2 conversion (develop)

- include/layout.js: move the select2-hidden-accessible teardown in
  applySkin() to run before setSelectMenus() instead of after it, so the
  catch-all initializer still re-enhances plain <select> elements that the
  teardown just destroyed (previously they stayed native after any
  post-initial-load applySkin() re-run).
- include/layout.js: namespace the select2-callback widget's
  select2:select listener and unbind it before rebinding, since destroying
  and recreating the Select2 instance on every AJAX filter reload does not
  remove a plain jQuery listener bound to the underlying <select>, which
  was accumulating duplicate invocations of the configured callback.
- include/layout.js, lib/auth.php, lib/html.php: thread $noany/$nonone
  from html_host_filter() through to its ajax-backed select2-callback
  widget (as data-noany/data-nonone attributes forwarded as request vars),
  and have get_allowed_ajax_hosts() honor them. $sql_where is intentionally
  left unapplied over the ajax path and documented as such: passing
  caller-supplied raw SQL across the client/server boundary would be a new
  SQL injection vector, unlike the boolean any/none toggles.
- lib/html.php: use html_escape_attr() instead of htmle() for the two
  attribute contexts in html_host_filter()'s markup (data-callback and the
  option value), since htmle()/html_escape() disables double_encode and so
  a pre-encoded quote entity in the input could survive and be decoded
  back into a live quote by the browser, breaking out of the attribute.
  htmle() is kept for the visible option text.
- lib/html.php: use cacti_js_encode() instead of plain json_encode() for
  the two new multi-select translation strings emitted into the inline
  <script> block, since json_encode() alone does not hex-escape </script>
  sequences.
- lib/html_reports.php: use the global select2SearchRows setting instead
  of a hard-coded 10-row threshold when reinitializing the branch_id
  Select2 control after an AJAX tree change, so the configured
  "Select Search Minimum Rows" setting (including "Always") is respected.
- include/themes/paper-plane/main.css, include/themes/sunrise/main.css:
  fix unreadable dark-on-dark Select2 control text (both the base control
  and its rendered value) by using each theme's existing light foreground
  convention instead of the near-black color left over from the plain
  jQuery UI selectmenu styling.

Verified: tests/Unit/InlineScriptDataEncodingTest,
tests/Unit/Security/Xss/HtmlEscapeTest, and reports/host/graph-filter
related Unit tests all pass; php -l and node --check clean on touched
files; empirically reproduced (jsdom + real select2 4.1.0 + jQuery 4.0)
that the select2-multi-count adapter combination does not create a
duplicate inline search box, contrary to one review comment - no code
change made for that finding, replied with the reproduction instead.
2026-09-23 11:58:48 -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
TheWitness 2e05b98b1f Add select2 for enhanced/searchable dropdowns (#8051)
* feat: add select2 for enhanced/searchable dropdowns

Vendors select2 4.1.0 (MIT, https://select2.org) as a new front-end
dependency, following this branch's existing npm-synced-to-include/js
convention:

- package.json: added "select2": "4.1.0" to dependencies (alongside the
  other npm-tracked front-end libs, so Dependabot can track it).
- build/sync-js.mjs: added node_modules/select2/dist/js/select2.full.js ->
  include/js/select2.js to the asset map (the "full" build bundles its
  jquery-mousewheel dependency, so no extra package is needed).
- include/js/select2.js: the synced output (select2.full.js, unminified,
  matching this repo's "vendor unminified where possible" convention).

CSS, following the existing per-theme vendor-CSS convention (jquery-ui.css,
jquery.multiselect.css, etc. - none of which are npm-synced either):

- include/themes/<theme>/select2.css: the upstream select2.css (structural/
  functional rules), copied unmodified into all 10 themes.
- lib/html.php: wired both the JS and (per-theme) CSS into the page header,
  next to the other jQuery UI plugin includes.
- include/themes/<theme>/main.css: since Select2 4.x publishes no jQuery UI
  theme/adapter, appended a small override block per theme (widget border/
  background/text, hover, and active/highlighted colors) reusing each
  theme's own jquery-ui.css/.ui-state-active colors, so Select2 dropdowns
  match that theme's native jQuery UI look instead of Select2's generic
  default styling.

* feat: initialize select2() widgets in applySkin()

Wires up the select2/select2-nosearch/select2-tags/select2-multi/
select2-multi-tags/select2-callback marker classes so pages can opt in
to a Select2-enhanced dropdown just by adding the class, matching the
existing markup-driven convention used by other jQuery UI widget
setup in this function.

Dialog-hosted selects pass dropdownParent so the dropdown attaches to
the ui-dialog widget instead of escaping under it (a well-known
Select2 + jQuery UI dialog z-index/stacking issue).

* fix: remove accidental theme:'bootstrap' from select2-multi init

* fix: address Copilot review findings on the select2 PR

- package-lock.json: added the select2@4.1.0 entry (registry resolved URL +
  integrity hash) so `npm ci` no longer rejects the package.json/lockfile
  mismatch.
- .gitignore / include/js/select2.js: select2.js is a generated npm-synced
  build output like the other managed libraries, so it belongs in the
  generated-asset ignore list, not committed. Untracked the copy that had
  been committed; `npm run build:js` regenerates it locally.
- build/README.md: documented select2 among the npm-managed libraries, and
  clarified that the per-theme select2.css copies are committed unmodified
  upstream CSS (unlike jquery-ui/jstree's Cacti-owned theme CSS).
- include/themes/paper-plane/main.css: highlighted/focused Select2 options
  were the same white as the dropdown background, giving no visible focus
  indicator; changed to the theme's own hover color (#dadada / #212121).
- include/themes/sunrise/main.css: same issue; changed to this theme's
  established menu focus color (#3C8A3C, matching .ui-menu .ui-state-focus)
  with white text.

* fix: address 3 material Copilot review findings on select2 init

- Exclude the select2/select2-nosearch/select2-tags/select2-multi/
  select2-multi-tags/select2-callback marker classes from
  setSelectMenus()'s generic $('select')... loop, so Select2-marked
  single-selects are no longer first hidden/wrapped by jQuery UI
  Selectmenu before Select2 wraps them again.
- select2-callback: Select2 calls the ajax.url callback with a context
  that is not the original <select>, so $(this).data('callback') read
  undefined inside it. Capture the element's callback URL into a local
  variable before calling .select2() and use that instead.
- select2-callback: it was the only initializer that never checked for a
  jQuery UI dialog ancestor, so its dropdown always appended to body and
  could be hidden behind a dialog's overlay. Set dropdownParent the same
  way the other five initializers do.

* fix: proactively apply 2 material findings from the 1.2.x PR review

Copilot's review of the companion 1.2.x PR (#8052) found 2 bugs in this
same select2-init code that also apply here unchanged:

- Select2 renders its replacement as <span class="select2 select2-container">,
  so the plain class selectors (.select2, .select2-nosearch, etc.) also
  matched Select2's own generated wrappers on every later applySkin() call
  (e.g. during AJAX navigation), re-initializing and nesting wrappers.
  Scoped all six selectors to `select.<marker-class>` so only the original
  <select> elements are ever matched.
- select2-callback's ajax response was never normalized: Cacti's existing
  callback endpoints (e.g. lib/auth.php) return a plain array of
  {label, value, id} objects, but Select2 requires {results: [{id, text}]}.
  Added a processResults mapping so callback-backed dropdowns actually
  populate.
- select2-callback now also sets dropdownParent unconditionally (dialog
  ancestor, or document.body otherwise) instead of leaving it unset outside
  dialogs.
2026-09-22 08:27:00 -04:00
TheWitness 1a37594ad6 Replace unmaintained phpdiff vendor lib with jfcherng/php-diff (develop) (#8050)
* chore: replace unmaintained phpdiff vendor lib with jfcherng/php-diff

The manually-vendored include/vendor/phpdiff (chrisboulton/php-diff v1.1,
2009 - unmaintained since ~2013) backed the package-import file diff
viewer (package_import.php's package_diff_file()), its only call site.

Replace it with jfcherng/php-diff (^6.16, BSD-3-Clause), an actively
maintained continuation of the same library, declared as a real Composer
dependency in composer.json/composer.lock (installed at build/CI time,
same as this branch's other composer.json dependencies - not committed
to include/vendor, unlike the old hand-vendored phpdiff).

package_import.php now uses Jfcherng\Diff\Differ + RendererFactory in
place of the old Diff/Diff_Renderer_Html_Inline classes. Preserved the
existing nosemgrep justification comment, updated to reference the new
library's AbstractHtml::htmlSafe() escaping.

Adopted the vendor's own diff-table.css as the Diff.css in all 10 themes,
replacing the bespoke per-theme stylesheets that were tied to the old
library's class names.

* fix: address review feedback on jfcherng/php-diff migration

- package_import.php: narrow $oldfile/$newfile to array before
  constructing Jfcherng\Diff\Differ so PHPStan can see the
  false branch is excluded (fixes level 6/8 analysis failure).
- tests/tools/check_vendor_policy.php: drop the removed
  include/vendor/phpdiff/ path from the legacy allowlist.
- tests/Unit/Security/Xss/PackageDiffRendererTest.php: rewrite
  against the new Jfcherng\Diff\Differ/RendererFactory API instead
  of the deleted vendored phpdiff classes.
2026-09-21 13:59:50 -04:00
TheWitness 2f14d78927 fix(js): regenerate DOMPurify 3.4.15 asset and keep node_modules during CI (#8043)
* fix(js): regenerate DOMPurify 3.4.15 asset and skip node_modules cleanup in CI

* chore: add PR number to CHANGELOG entry
2026-09-18 11:59:13 -04:00
TheWitness d34fbaacc4 fix: Restore base javascript files for testing (#8037)
* fix: Restore base javascript files for testing

* fix: screenfull 6.0.2 is missing screenfull.js

* fix: Restoring screenful after discovering how the files are managed

* fix: Remove node modules directory after installing modules

* fix: Change in screenfull.js behavior affects cacti

* fix: Restore jQuery 4 compatibility in the tablesorter pager plugin

$.isArray()/$.trim() were removed in jQuery 4.0, which this PR pins in
package.json. The restored jquery.tablesorter.pager.js still called
E.isArray(...)/E.trim(...), so any Ajax pager response threw before
rendering. Also fix a TypeError in the countChildRows branch, which
indexed into `a` while it was still an undeclared var.

Add patches/tablesorter+2.32.0.patch so the fix survives the next
`npm run build:js` sync from node_modules, following the existing
pace-js patch-package precedent.

* fix: Rewrite screenfull's ESM export during sync-js instead of downgrading

screenfull 6+ ships ESM-only (`export default screenfull;` in
node_modules/screenfull/index.js), which throws "export declarations
may only appear at top level of a module" when the copied file is
loaded as a classic <script>.

Add a post-copy transform step to build/sync-js.mjs that rewrites that
line to `window.screenfull = screenfull;` for include/js/screenfull.js,
so the 6.0.2 pin in package.json can be kept instead of reverting to
5.2.0. Throws if the expected export line isn't found, so a future
screenfull release changing its export shape fails the build loudly
rather than silently shipping a broken asset.

Updates tests/js/sync_js.test.mjs to cover the transform and its
failure mode.
2026-09-18 06:16:35 -07: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 428b56e04d chore: replace removed jQuery $.isFunction() with typeof checks (#7917)
* chore: replace removed jQuery $.isFunction() with typeof checks

jQuery 4.0 removes $.isFunction(). Swap the three call sites in navigationBox.js
and navigationBox.tree.js for behaviour-identical 'typeof x === function' checks,
removing the only jQuery-4-removed API in Cacti's own front-end code.

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

* fix: correct typeof negation in the mark.js availability check

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

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-09-12 15:25:01 -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 1ae479d4f5 feat(cli): add unified command frontend and normalize legacy failures (#7903)
* feat(cli): add unified command frontend and normalize legacy failures

Add the Symfony-backed bin/cacti entry point while preserving legacy argument, terminal, signal, and exit-status behavior. Fold in the focused legacy CLI correctness fixes from #7906 without adding its unused parser abstraction.

Keep path containment and the PHP 8.3 floor out of this review; the latter remains in #7939.

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

* fix(cli): enforce PHP floor and deny web dispatch

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-09-07 18:36:55 -07:00
Thomas Vincent 79a9ae0351 build: raise the 1.3 floor to PHP 8.3 (#7939)
* build: raise the 1.3 floor to PHP 8.3

RHEL 10 ships 8.3 in AppStream and dropped modularity, so an 8.4 floor would
put every RHEL, Alma and Rocky 10 install on Remi. Ubuntu 24.04 is 8.3 as
well, which makes it the lowest common denominator of the current LTS fleet.

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

* docs: raise the documented PHP floor to 8.3

The 1.2.x column keeps 8.1, which still matches that branch's composer constraint.

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

* ci: run process lock coverage on PHP 8.3

* fix: finish PHP 8.3 floor cleanup

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

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-09-06 08:44:40 -07:00
Thomas Vincent 1fdf9acf3b Fix detached select menus on scroll (#8000) 2026-09-05 18:37:42 -07:00
Thomas Vincent b489dbfab3 fix(build): stop tracking partial Composer vendor tree (#7960) 2026-09-04 08:57:01 -07:00
Thomas Vincent ca2e4f07c2 Build releases from managed dependencies (#7888)
* chore: manage front-end JavaScript libraries with npm and Dependabot

Move all third-party front-end JavaScript to npm so Dependabot tracks it, and
drop the committed copies in favour of a build. package.json pins jquery,
jquery-ui, jstree (3.3.12 -> 3.3.17), billboard.js, pace-js, tablesorter and
jquery-validation; build/sync-js.mjs copies their dist files into include/js/
and runs on npm install via postinstall.

pace-js carries a one-line local fix (a stray regex alternation that made it
match the empty string) applied through patch-package
(patches/pace-js+1.2.4.patch); the fix should be sent upstream so the patch can
be dropped. Only Cacti's own themed jquery-ui/jstree CSS stays committed.

An npm ecosystem is added to dependabot.yml for develop. The release pipeline
and any workflow that serves Cacti pages must run 'npm ci' (which builds via
postinstall) before use, since the generated files are no longer in the tree.

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

* hardening: remove vendored development artifacts

* feat(release): attach the built artifact to the release

The workflow already builds an attested, checksummed archive carrying the
Composer dependencies, then discards it after thirty days.

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

* ci: fail the publish step when the bundle is empty

An unmatched glob would otherwise reach gh as a literal path.

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

* ci: treat the maintenance lock notice as expected log output

The line records the lock working, so a second maintenance run overlapping
in CI failed unrelated pull requests.

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

* docs: consolidate release dependency changelog

* ci: build managed JavaScript in release artifacts

* test: cover managed JavaScript build end to end

* deps: complete phpseclib 4 migration

* deps: update patch-package to audited release

* ci: satisfy workflow shell policy

* chore: source DOMPurify from npm and update it to 3.4.14

Add dompurify to the npm-managed set (was committed at 3.4.7) and bump to 3.4.14.
DOMPurify is the client-side XSS sanitizer used on tree labels, device tooltips
and plugin messages; keeping it current with upstream bypass fixes matters more
than for the UI libraries.

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

* docs: preserve release changelog ordering

* test: preserve DOMPurify source map in npm build

* chore: source htmx, d3, big.js, lzjs and screenfull from npm

Add the remaining unmodified third-party libraries to the npm-managed build:
htmx.org 2.0.6, d3 7.8.2, big.js 6.2.2, lzjs 1.3.1 and screenfull 3.3.3. Each
was verified byte-identical (lzjs differs only in a JSDoc comment) to the copies
being removed. htmx in particular moves from a hand-tracked SHA pin to npm with
lockfile integrity.

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

* test: cover phpseclib 4 integration

* ci: build managed assets before htmx tests

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-09-01 23:11:49 -07:00
Thomas Vincent 668333b7c9 security(import): validate Data Source item fields coming from Template XML (#7720)
* security(import): validate Data Source item fields coming from Template XML

data_source_name, rrd_minimum and rrd_maximum reach an RRDtool command line
but never pass through the data_sources.php form on the import path, so the
same field rules are applied at the XML handoff instead.

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

* chore: add the CHANGELOG entry

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

* fix(import): name the rejection and keep the value out of the log lines

Code 7 reads as an XML parse error, which this is not, and the value comes
from the file being imported.

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

* style: use a line comment to satisfy php-cs-fixer

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

* test: resolve includes through Cacti path constants

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

* ci: restore changelog entry ordering

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

* fix(changelog): order the 7720 issue entry before features and drop a duplicate

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

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-08-31 17:08:26 -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 cdf92b7c8d Resolve the client address through the trusted proxy list (#7658) 2026-08-29 17:52:12 -07:00
Thomas Vincent 11117e812d fix(php85): remove test and authenticator deprecations (#7632) 2026-08-29 17:38:51 -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 80fbb802a3 fix: allow graph tree sidebar to shrink (#7837) 2026-08-29 16:00:35 -07:00
Thomas Vincent ff475bef6d feat(installer): add installation domain foundation (#7291) 2026-08-22 19:05:50 -07:00
Thomas Vincent 4d021a93e3 Add plugin-extensible notification API (#7775) 2026-08-21 14:29:33 -07:00
Thomas Vincent 49f5212181 fix(tests): restore paths and isolate test doubles (#7764)
* fix(tests): correct path depths after the domain reorganisation

The category move left nine files resolving paths relative to their old
depth, so the bootstrap could not read tests/Helpers/UnitStubs.php and no
test ran at all.

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

* fix tests without replacing production functions

* remove unrelated failing integration tests

* remove unrelated broken test workflows

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-08-19 12:28:45 -07:00
Thomas Vincent e61d5e274a fix: declare __() before the disabled-i18n fallback returns (#7751)
* fix: declare __() before the disabled-i18n fallback returns

include/global_languages.php returns early to a fallback locale when
i18n_language_support is off. __() sits inside a function_exists() guard, and
PHP does not hoist a conditional declaration, so that return left __()
undefined for the rest of the request. The other translation helpers are plain
top level declarations, so hoisting covered them and only __() went missing.

include/global.php defines CACTI_VERSION_BRIEF through get_cacti_version_text()
on the next line, so the first caller was Cacti's own startup:

  PHP Fatal error: Uncaught Error: Call to undefined function __()
    in lib/functions.php:7975
  #0 lib/functions.php(8042): format_cacti_version_text()
  #1 include/global.php(700): get_cacti_version_text()
  #2 include/cli_check.php(54): include()
  #3 cli/install_cacti.php(26): require()

Reproduced against a freshly loaded schema, where the setting has no row and
no default yet because global_settings.php loads ten lines later. Before the
change cli/install_cacti.php dies at global.php:700; after it, startup clears
that line and continues.

The declaration moves above the fallback. Nothing else changes.

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

* test: address i18n fallback review

* docs: record the i18n bootstrap fix

* Fix translation helper guard formatting

* test: reuse the i18n fallback probe

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-08-17 23:52:43 -04:00
Thomas Vincent 298bd51eca security: require POST + CSRF token for GET-accessible delete actions (#7673) 2026-08-16 22:15:39 -07:00
Sean Mancini 96033267cb refactor(tests): reorganize unit tests into domain category folders (#7733)
* refactor(tests): reorganize unit tests into domain category folders

- Consolidate per-advisory GHSA regression tests into Security/ subfolders by vuln class
- Group all 186 test files into domain folders: DataCollection/, Ui/, Core/,
  Database/, Installer/, Scripts/, Security/, Plugin/
- Strip GitHub issue/GHSA numbers from filenames; keep IDs in test descriptions
- Fix all __DIR__ relative paths to match new folder nesting depth
- Update CI workflow path triggers and phpunit XML configs for new file locations
- Update .github/copilot-instructions.md with unit test organization guidelines
- Add tests/vendor/ to .gitignore

* add gaurds

* fix up guard for unit tests

* fix redeclaration of cacti_log in tests

* fix updates to unit tests

* fix test patjhs

* fix: update test __DIR__ paths for reorganized test structure

* fix: conditionally define read_user_setting to prevent redeclaration in tests

* fix: conditionally define functions to prevent redeclaration in tests

- Wrap clean_up_lines, raise_message, cacti_debug_backtrace, and get_debug_prefix
  with function_exists() checks to prevent conflicts when test stubs are loaded
  before lib/functions.php in the same PHP process

* fix: add unit test bootstrap to prevent function redeclaration errors

- Create tests/bootstrap-unit.php to load core libraries before tests
- Ensures lib/functions.php loads before test stubs define stub functions
- Test stubs use function_exists() checks, so they detect pre-loaded functions
- Update phpunit.xml to use bootstrap-unit.php for Unit testsuite
- Fixes 'Cannot redeclare read_user_setting()' errors when tests run in same process

* fix: load test stubs before global.php in unit test bootstrap

- Reorder bootstrap to load CactiStubs.php and UnitStubs.php first
- Ensures stub functions like __() are available when global.php loads
- global.php calls get_cacti_version_text() which needs __() for translations
- Fixes 'Call to undefined function __()' error during bootstrap

* fix: remove function conflicts from test stubs

- Remove stub definitions for functions already in lib/functions.php:
  cacti_sizeof, cacti_log, cacti_strtolower, read_config_option,
  read_user_setting, cacti_count
- Keep translation function stubs (__(), __esc()) needed by global.php
- Update bootstrap to load lib/functions.php after stubs
- This prevents function redeclaration errors while maintaining test isolation

* fix: prevent constant and variable redeclaration in bootstrap

- Remove explicit PHP_TESTING and CACTI_TEST_BOOTSTRAP defines from bootstrap
  (CactiStubs.php now handles this with guards)
- Add guards to CactiStubs.php to prevent redefinition of PHP_TESTING
- Only set CACTI_TEST_BOOTSTRAP env vars if not already set
- Fixes PHP warnings about 'Constant X already defined'

* fix: remove redundant lib/functions.php load from bootstrap

include/global.php already loads lib/functions.php at lines 287-288.
The bootstrap was trying to load it again after global.php, but global.php
needs format_cacti_version() to be available on lines 293-294 to define
CACTI_VERSION and CACTI_VERSION_FULL constants.

Solution: Let global.php handle loading lib/functions.php itself, and don't
load it separately in the bootstrap.

* fix: remove more conflicting function stubs from test helpers

- Remove stubs for functions also defined in lib/functions.php:
  raise_message, clean_up_lines, cacti_debug_backtrace, get_debug_prefix
- Remove POLLER_ID constant stub (defined unconditionally by global.php)
- Keep only translation functions and test-specific helpers
- This prevents function/constant redeclaration errors

* fix: minimize test stubs to only translation functions

Remove ALL helper functions and constants from UnitStubs.php that are:
- Defined in lib/functions.php (raise_message, clean_up_lines, etc.)
- Defined in lib/html_utility.php (srv, set_request_var)
- Defined in global_constants.php (POLLER_VERBOSITY_*, MESSAGE_LEVEL_*)
- Defined in global.php (POLLER_ID)

Keep ONLY translation functions (__(), __esc()) that are:
- Called by global.php during initialization
- Not defined elsewhere
- Required for proper bootstrap sequence

This eliminates all constant and function redeclaration errors.

* fix: remove __esc stub - already defined in global_languages.php

Keep only __() translation function stub, which is needed by global.php
and not defined elsewhere in Cacti core.

* fix: handle no-arg case in __() translation stub

The __() function may be called with just a string and no format arguments.
Previously, vsprintf() would fail with 'arguments array must contain 2 items'.
Now, if no args are provided, return the text as-is.
2026-08-16 19:23:31 -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 Vincent 2e372e5b88 Make PHP 8.1 dependency builds reproducible (#7746)
* Make PHP 8.1 dependency builds reproducible

* Tighten integration runner permissions

* Harden and standardize GitHub Actions

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

* Restore ripgrep for security guards

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

* Wait for Cacti daemon readiness in CI

* Satisfy strict shell policy in diagnostics

* Preserve Cacti config group ownership

* Run stateful Cacti checks as the web user

* Run privileged page-crawl harness as root

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-08-16 22:06:36 +01:00
Thomas Vincent 78605b36b0 chore(deps): regenerate the committed vendor tree (#7736)
Dependabot raises the composer.json constraints without reinstalling, so the
committed tree still held psr/log 1.1.4 against a ^3.0 requirement and
league/oauth2-google 4.2.0 against ^5.0. Guzzle, phpseclib and fast-route pick
up their current patch releases in the same pass.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-08-15 11:55:39 -04:00
Thomas VincentandTheWitness a6da63ac89 Harden remote graph transport and response validation (#7631)
* Harden remote graph transport boundaries

* Tighten remote graph SVG validation

* fix: PHPStan Issues with emerging code

* Refresh security baselines after develop rebase

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

* Match transport test to the explicit response-header default

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

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-08-11 08:16:48 -04:00
Thomas VincentandTheWitness db656e8cb1 fix(auth): regenerate the session id when a session becomes authenticated (#7619)
* fix(auth): regenerate the session id when a session becomes authenticated

1.2.x closed this under GHSA-273r-qr93-wgcp; the helper never reached this
branch, so a session id planted before the login survived it.

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

* fix(auth): refuse a locked account before rotating its cookie

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

* fix(auth): fail closed when transition user is missing

* fix(auth): gate remembered login before token creation

* Fix auth transition cache invalidation

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-08-10 12:12:34 -04:00
Thomas VincentandTheWitness bb6b90e555 Recognize RRDtool 1.10 and 1.11 (#7636)
* Recognize RRDtool 1.10 and 1.11

* Handle failed RRDtool version detection

---------

Co-authored-by: TheWitness <thewitness@cacti.net>
2026-08-10 12:10:30 -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 VincentandTheWitness b9455eeb66 fix(session): centralize request URI fallback (#7592)
* fix(session): centralize request URI fallback

* refactor(auth): use basic auth method constant

* test(session): cover every guarded request URI site

---------

Co-authored-by: TheWitness <thewitness@cacti.net>
2026-08-10 11:33:31 -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 Vincent 407c82f625 fix(settings): default binary paths to empty strings (#7594)
* fix(settings): default binary paths to empty strings

* test(settings): scope binary path wording
2026-08-06 17:56:20 -07:00