mirror of
https://github.com/Cacti/cacti.git
synced 2026-10-25 04:27:56 +09:00
develop
1149
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 TheWitness
|
b95226918e |
fix: Update the deepness favicon (#8419)
* fix: Update the deepness favicon (cherry picked from commit |
||
|
TheWitness
|
433af99661 |
Fix plugin archive restore skipping not-yet-created nested directories (#8398)
Restoring a plugin from an archive, or installing one downloaded from GitHub into the database, extracts the tgz to a temp file and writes each entry under plugins/<name>/. validate_relative_path_within() anchored a not-yet-created entry on its immediate parent directory, so every entry in a subdirectory the restore had not created yet (includes/, css/, tests/, docs/, ...) resolved against a missing path, failed the containment check and was skipped. The plugin was left with only its top-level files and setup.php then fatally required the missing includes/database.php. Judge a not-yet-created entry by its deepest existing ancestor instead, mirroring data_source_path_within_rra(). Every path segment is already a validated plain name and no walked segment may be a symlink, so the full candidate still stays within the base; traversal, absolute, drive-letter, null-byte and symlink entries remain rejected. |
||
|
Thomas Vincent
|
cf70ba4cf2 |
fix(plugins): [develop] restore lifecycle diagnostics, compatibility and error attribution (#8297)
* fix(plugins): honor version range boundaries and compatibility status Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(plugins): preserve null hook diagnostics and enforce dependency versions Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(plugins): report config failures and confine error attribution Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(plugins): guard entrypoint frames and bind realm filenames Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(plugins): propagate install hook failures on develop Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(plugins): match realm filenames literally Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs(changelog): defer release entries to final backlog PR Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(plugins): add standard GPL headers Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci(coverage): require 80 percent of changed PHP statements Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: share changed-code coverage contracts and enforce shell policy Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(ci): run request coverage with portable path matching Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: retain bootstrap coverage and validate request dispatch Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: synchronize native coverage fixture dispatch Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: measure coverage at the exact PR revision Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas Vincent
|
ebd16105c5 |
fix: drop backslash and line breaks in sanitize_uri (#8388)
* fix: drop backslash and line breaks in sanitize_uri Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: add changelog entry for sanitize_uri fix Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas Vincent
|
0f06608175 |
fix: reject dangling symlinks in validate_relative_path_within (#8389)
* fix: reject dangling symlinks in validate_relative_path_within Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: add changelog entry for dangling symlink fix Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas VincentandPetr Macek
|
930b085703 |
feat(auth): log administrative account actions after confirmed writes (#8360)
* log user action (create, delete, locked, ..) * change log * update changelog * style(auth): apply coding standards to user action logging Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Audit password changes only after successful account saves Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(auth): sanitize audit fields and confirm account removals Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: cover native account audit state transitions Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: synchronize native coverage fixture dispatch Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(auth): audit only confirmed account persistence Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: measure coverage at the exact PR revision Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: Petr Macek <petr.macek@kostax.cz> |
||
|
TheWitnessandThomas Vincent
|
34fc3f8642 |
Stop flagging a not-yet-created RRA subdirectory as a data_source_path escape (#8313)
* Stop data_source_path_within_rra() flagging a not-yet-created intermediate directory as an RRA escape get_data_source_path() routed a legitimate <path_rra>/<host_id>/<local_data_id>.rrd through the generated-path fallback and logged a bogus 'SECURITY: ... escapes the RRA directory' message whenever the host_id subdirectory did not exist yet (first poll, or host_id 0). data_source_path_within_rra() only tolerated a missing final RRD file: for a missing file it resolved realpath(dirname(path)), which returns false when that parent directory has not been created, so the lexically-contained path was wrongly rejected. Walk to the deepest ancestor that actually exists and confirm it resolves inside the RRA root instead. Lexical containment (no .., '.', or empty segments) and per-segment symlink rejection are unchanged, so traversal, lookalike-prefix, absolute, and symlink-pivot paths are still rejected. Adds a regression test for a not-yet-created intermediate directory. * Drop explanatory comments from the RRA containment fix * Add CHANGELOG fragment for the RRA containment fix * Reject a dangling-symlink segment independently of file_exists() * test(paths): cover RRA filesystem boundaries and gate PR coverage Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: measure committed PR changes on a policy-compliant runner Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
TheWitnessandTheWitness
|
ce707ccd79 |
Fix false-positive RRA-escape warning when path_rra has a trailing slash (#8258)
* Fix false-positive RRA-escape warning when path_rra has a trailing slash
A path_rra setting saved with a trailing slash makes CACTI_PATH_RRA end in
'/', so the '<path_rra>/' -> CACTI_PATH_RRA . '/' expansion that every data
source consumer performs yields a doubled slash ('<rra>//0/1457.rrd').
data_source_path_within_rra() split the expanded path on '/' and rejected any
empty segment, so the doubled slash was read as a traversal and a perfectly
legitimate RRD file was reported as escaping the RRA directory:
SECURITY: Data source 1457 has a data_source_path that escapes the RRA
directory (<path_rra>/0/1457.rrd). Using the generated path instead.
get_data_source_path() then returned the generated fallback path, which the
poller had never created, producing the follow-on 'RRD file does not exist'
graph warnings (note the tell-tale 'rra//0' doubled slash in those lines).
Collapse repeated slashes and strip a trailing slash from the RRA root before
the containment comparison. A '//' is an empty segment, not a parent
reference, so normalising it keeps the file contained while '..' traversal,
absolute paths elsewhere, lookalike prefixes and symlink pivots are still
rejected (asserted by the expanded regression test).
* Keep data_source_path_within_rra() normalisation strictly string-typed
preg_replace() is typed array<string>|string|null, so feeding its result to
rtrim()/strlen() tripped PHPStan (level 6/8). Cast CACTI_PATH_RRA and both
preg_replace() results to string; runtime behaviour is unchanged.
* Cover the trailing-slash CACTI_PATH_RRA half of the fix in an isolated process
The existing cases insert '//' into the target only; data_source_path_test_base()
uses realpath(), which strips the trailing slash, so they never exercise the
rtrim() on \. CACTI_PATH_RRA is immutable, so define it with a trailing
slash in a fresh PHP process and assert a clean/doubled-slash file stays
contained while a traversal is still rejected.
---------
Co-authored-by: TheWitness <1439914+TheWitness@users.noreply.github.com>
|
||
|
TheWitness
|
b5a388094d |
Honor @ error suppression in CactiErrorHandler on PHP 8 (#8197)
* Honor @ error suppression in CactiErrorHandler on PHP 8 CactiErrorHandler() guarded against the @ operator with error_reporting() == 0, which was the PHP 7 semantics. On PHP 8 the @ operator sets error_reporting() to a non-zero bitmask that merely excludes the suppressed level, so the guard never matched and deliberately suppressed warnings still reached the log. This surfaced after the Symfony mailer swap: Symfony's SendmailTransport writes to the sendmail pipe with @fwrite(), and when sendmail is misconfigured the broken-pipe notice leaked into the Cacti log instead of being swallowed and reported as the handled TransportException. Mask error_reporting() against the incoming level so the @ operator is honored on every supported PHP version. * Add regression test for CactiErrorHandler @-suppression mask Exercises both paths of the error_reporting() mask check: a notice and a warning raised under PHP 8's @-operator bitmask return before logging, while an enabled level is logged and reported back to PHP as unhandled. The suppressed cases would have logged under the previous error_reporting() == 0 guard, so the test fails closed if that regression returns. |
||
|
TheWitnessandLarry Adams
|
75d816a335 |
security: consolidate GHSA batch 11 fixes for develop (fq9x, hh7r, v99q, f5f5) (#8165)
* security: consolidate GHSA batch 11 fixes for develop (fq9x, hh7r, v99q, f5f5) Mirrors the 1.2.x batch-11 security consolidation. Items already fixed on the develop tip are credited on their advisories and omitted here: - GHSA-54hx-hxc4-jq7j: TICK/SHIFT values already pass through strip_alpha(). - GHSA-hh7r CSV portion: already guarded via cacti_csv_cell(); only the Content-Disposition filename needed sanitizing (included below). - GHSA-mx5c: REST scaffold already deny-by-default (404 unless api_enabled), enforced by tests/Unit/ApiDenyByDefaultTest.php. - GHSA-rjvj/pjq5/fmhq/mfw8: already addressed on develop. Fixes applied here: - GHSA-fq9x-x3vf-3vf2 (High): add substitute_script_path() single-pass token substitution and refactor get_full_script_path()/get_full_test_script_path() to use a pre-escaped value map, closing the second-order re-substitution OS command injection. - GHSA-hh7r-464j-rf39: sanitize the Content-Disposition filename in graph_xport.php. - GHSA-v99q-57w7-mpmg: escape the page title rendered in the HTML <title>. - GHSA-f5f5-h85v-6m3h: render user admin tooltip content as escaped text. * security: add batch-11 consolidation regression tests for develop Addresses PR #8165 review feedback by adding regression coverage for all four fixed sinks: - GHSA-fq9x: behavioral single-pass proof that substitute_script_path() does not re-substitute a field value containing another field token, plus source guards that both path builders use the helper. - GHSA-hh7r: Content-Disposition filename strips CR/LF/quote/backslash. - GHSA-v99q: the HTML <title> is html_escape()'d. - GHSA-f5f5: both the user_admin.php and user_group_admin.php tooltips render data-tooltip as escaped text. * style: apply php-cs-fixer formatting to substitute_script_path (CI coding standards) * docs: require composer run-script php-cs-fixit before creating any pull request * docs: require PHPStan (level 8, satisfies Level 6 floor) to pass before creating any pull request --------- Co-authored-by: Larry Adams <TheWitness@users.noreply.github.com> |
||
|
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 |
||
|
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>
|
||
|
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. |
||
|
TheWitness
|
38c1d05f08 |
perf: cache realpath(CACTI_PATH_RRA) in data_source_path_within_rra() (#8070)
* perf: cache realpath(CACTI_PATH_RRA) in data_source_path_within_rra() * perf: align RRA-root realpath cache with 1.2.x's array-keyed pattern |
||
|
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
|
||
|
|
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 ( |
||
|
Thomas Vincent
|
ac0b2c9515 |
Fix confirmed poller, database, automation, and UI defects (#7861)
* issue#7024: do not delete still-running poller_time rows on cycle start The overrun path deleted every poller_time row for the poller, including the in-flight ones it had just counted, so overrunning processes lost their tracking. Restrict the delete to completed rows. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7028: restore identifier and connection safety in the db layer _db_replace interpolated caller-supplied column keys as backtick identifiers without escaping, and db_commit_transaction checked @@in_transaction on the default connection rather than the one passed in. Both were correct on 1.2.x and regressed on develop. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7341: size the CSV export maxrows to the requested span The export hardcoded --maxrows=10000, truncating wide date ranges regardless of the data source profile step. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7421: offset automation_get_next_host through the 32-bit integer The hand-rolled octet walk subtracted the wrong magnitude per octet and emitted out-of-range octets for ranges wider than a /16. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7030: skip the poller_output ENGINE=MEMORY conversion when already applied The output-table refresh ran ALTER TABLE poller_output ENGINE=MEMORY every cycle even though the swapped-in copy already inherits the engine, adding a metadata-locking DDL to the hot path. Probe the current engine and convert only when needed. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7342: prefer snmpEngineTime only when it covers system uptime update_system_mibs preferred snmpEngineTime (x100) over sysUpTime whenever it was present, so an SNMP engine that restarts independently of the system (for example OpenBSD snmpd) made the stored uptime drop, which reads as a reboot and forces a perpetual reindex with mass 'U' results. Capture both values and use snmpEngineTime only when it is at least the system uptime, which still covers the 32-bit sysUpTime wrap. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7403: scale lmsensors readings in the get path like the query path The get path returned the raw milli-unit SNMP counter while the query path divided voltage and temperature by 1000 (and corrected negative voltages), so a get gave values 1000x too large. Apply the same scaling before returning. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7029: propagate the reconnected handle back to the caller db_check_reconnect() closed the dead connection, opened a new one, and returned true without giving the new handle back, so the db_execute_prepared retry ran its query against the connection that was just closed. Take $db_conn by reference and assign the fresh handle; the one caller that passed a literal false now passes a variable. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: refresh sink inventory baseline Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: refresh architectural hotspots baseline Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7474: return valid after correcting the theme to selected_theme is_valid_theme() corrected an invalid theme to selected_theme but left $valid false, so callers saw the theme as invalid even though it had been fixed. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7462: allow SELECT aliases as sortable automation columns site_name and host_template_name are aliases (s.name AS site_name), valid in ORDER BY but not real columns, so api_automation_column_exists() rejected them and the site-name sort silently failed. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7456: bind the OS value in the automation OS filter The OS filter built 'os = ?' but bound the network value, so filtering by OS matched on the wrong input. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7491: filter host_template_id against its own column html_make_device_where() compared h.location to the host_template_id value instead of h.host_template_id. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7479: apply the hostname and description filters in get_hosts The REST get_hosts() honored host_id/poller/site/template/location but ignored the hostname and description filters its callers pass; add them as bound LIKE. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7475: authorize a direct client-IP poller match before the hostname guard A poller configured by IP with no hostname-based pollers was rejected because the empty-hostname guard ran before the direct-match check, even though the client address exactly matched a poller's configured address. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Guard two divisors that PHP 8 makes fatal Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: consolidate functional fix changelog * test: cover consolidated functional fixes * docs: identify consolidated functional fixes --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas Vincent
|
964c1fb1f4 |
Harden RRDtool execution and data handoff (#7615)
* fix(rrdtool): quote RRDfile paths passed to the command line __rrd_execute() hands the assembled line to shell_exec(), and its docblock already required callers to quote each variable argument. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(tests): widen bare RRD path sweep Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(rrdtool): quote binary path as one argument Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(rrd): preserve caller logging context Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Make RRD XML mutations transactional Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Harden RRDtool proxy transport handling Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(rrdproxy): reject invalid command argument lists Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(rrdproxy): preserve binary graph payloads Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Keep RRDtool proxy sessions encrypted Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Harden local RRDtool command and output handoff Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Eliminate graph rendering metadata lookups Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Batch DSStats metadata and last-value reads Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Batch interface speed metadata reads Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Update architectural hotspot baseline for RRD execution Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Batch RRD resolution profile lookups Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Batch DSStats summary aggregates Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Narrow phpseclib key types for the proxy encrypt and decrypt calls Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(rrd): bound the forked proxy peers so a stall cannot hang the suite The children accepted and read with no deadline while the parent used a 2s socket timeout, so any mismatch left the child blocked in read and the parent blocked in a bare pcntl_waitpid(). Note that accept() ignores SO_RCVTIMEO, so readiness has to be waited on with socket_select() rather than a socket option. Also corrects the array-command expectation, which still assumed develop's escape-everything behaviour rather than rrdtool_build_command() leaving the operation unquoted. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(rrd): seed proxy settings where the real read_config_option looks UnitStubs only declares its stub when lib/functions.php has not been loaded yet, so during a full run the real database-backed function is the one in effect and $unit_config_options was being ignored. The parent then dialled an address the child was not listening on, which is what stalled the handshake. Seeding the option cache keeps the settings visible to either implementation. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(rrd): assert the shell-free RRDtool launch instead of the old quoting __rrd_init now hands proc_open an argv array with bypass_shell, so escaping the binary path would put the quotes inside the filename. The assertion still expected the pre-argv line and had been failing unnoticed behind the suite hang. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(dsstats): derive the expected stamps instead of assuming UTC dsstats_poller_output() formats with date(), so the hardcoded UTC strings only matched on a UTC runner. The failure was masked by the suite hang. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(rrd): keep RRDtool control codes intact in stdin commands Escaping the backslash when converting a shell-quoted argument to an RRDtool token left legends such as " IO Wait\:" ending in a dangling control code, so RRDtool refused every graph carrying an escaped colon. Verified against RRDtool 1.7.2 in pipe mode; values with no unambiguous quoted form are now refused rather than mangled. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: use Cacti test bootstrap and config Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: resolve includes through Cacti path constants Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: match the escaping helpers by token, not by text lib/rrd.php documents its escaping behaviour in a comment, and the regex over raw source counted that as a call site. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh sink inventory after develop rebase Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(rrd): prove argv isolation and concurrent pipe draining Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(security): refresh sink inventory baseline after rebase onto develop Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas Vincent
|
5727cebebd | security: forward-port 1.2.x hardening (dsstats, guest, permissions, cookie) (#7844) | ||
|
Thomas Vincent
|
cdf92b7c8d | Resolve the client address through the trusted proxy list (#7658) | ||
|
Thomas Vincent
|
4a98d27e6c |
security: escape external link titles and the tree data query index (#7815)
* hardening: escape the data query index in the tree graph header SNMP-collected values are stripped of <>"' by format_snmp_string(), so this is not exploitable through the SNMP path, but script-query data reaches host_snmp_cache.field_value without that filter. Escaping at the sink matches the sibling values on the same line. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * security: escape external link titles in the page title element external_links.title is stored with an empty validation regex and the title branch of draw_navigation_text() returned it raw. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas Vincent
|
f487bc3530 |
fix(functions): cast path_php_binary before escaping it (#7792)
The forward-port in
|
||
|
Thomas Vincent
|
619b33142e | feat(rendering): add HTML, URL, and inline-data output helpers (#7252) | ||
|
Thomas Vincent
|
02386f5006 | security: escape path_php_binary in test_data_source (#7786) | ||
|
Thomas Vincent
|
4d021a93e3 | Add plugin-extensible notification API (#7775) | ||
|
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> |
||
|
Thomas Vincent
|
d6f56fb51b | Keep an empty argument on the command line (#7695) | ||
|
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. |
||
|
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> |
||
|
Thomas Vincent
|
cb79286b18 |
Contain plugin archive writes and validate the repository URL (#7656)
* Contain plugin archive writes and validate the repository URL Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Use the full Cacti copyright header and reject non-base repository URLs Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Move the path containment helpers into lib/functions.php They live in lib/functions.php on 1.2.x; keeping a separate module on develop split the same helpers across two files. plugin_validate_repository_url() goes to lib/plugins.php with its caller. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh the sink baseline for the containment guard The archive guard shifts eleven sink line numbers in lib/plugins.php. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: drop the baseline refresh this branch did not need The sink and hotspot gates compare on a line-agnostic signature, so pure line drift never tripped them and refreshing only invites merge conflicts. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas VincentandTheWitness
|
c5fa3d2f75 |
Repair the string sanitisers in lib/functions.php (#7696)
* fix(functions): compare string offsets explicitly in sanitize_uri A graph_view.php URI opening with 'action=' or '?' matched at offset 0, which strpos() reports as a falsy 0, so it gained a duplicate token. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(functions): fall back when preg_replace returns null clean_up_lines() and sanitize_uri() were the two sites without the guard every sibling call in the file already had. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs(changelog): note the sanitiser repairs Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(functions): tighten sanitizer contracts --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
Thomas VincentandTheWitness
|
979c048ae3 |
security: block loopback/link-local SSRF targets in call_remote_data_collector (#7672)
* security: block loopback and link-local targets in call_remote_data_collector The function issued an outbound file_get_contents() to any hostname in the poller table with no address check, so an admin could point a remote collector at 127.0.0.1 or the cloud metadata endpoint (169.254.169.254) and turn a status poll into SSRF. The resolved target is now rejected when it falls in a reserved range (FILTER_FLAG_NO_RES_RANGE) before the fetch. RFC1918 private ranges stay allowed so distributed collectors on internal LANs keep working. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Drop the redundant null-coalesce on the resolved collector IP Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
|
3ee093a112 |
Guard two divisions PHP 8 made fatal (#7697)
* fix(poller): guard the average against a zero divisor Counters that disagree made the divide fatal on PHP 8, and the reverse lookup used a failed write as the offset it parses the reply from. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs(changelog): note the zero divisor guards Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: drop the baseline refresh this branch did not need The sink and hotspot gates compare on a line-agnostic signature, so pure line drift never tripped them and refreshing only invites merge conflicts. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs(poller): describe the PHP 7 divide result accurately It returned an infinity signed by the numerator rather than INF in every case. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> Co-authored-by: Sean Mancini <sean@seanmancini.com> |
||
|
Petr MacekandTheWitness
|
1f360b2420 |
fix log entry (two spaces) "fields which is NOT okay" (#7726)
Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
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> |
||
|
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> |
||
|
Thomas VincentandTheWitness
|
fd111a1900 |
fix: reset loop-scoped $oid and $script_path when building the poller cache (#7135) (#7570)
* fix: reset loop-scoped $oid and $script_path when building the poller cache Both output loops assigned inside a guard but read after it, so an output with no mapping was cached against the previous output's OID or script. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: append the changelog entry at the end of the issue block Matches .github/copilot-instructions.md, which appends rather than inserting in numeric order. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: use the full licence header on the new test files Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: treat CACTI_PATH_LOG as a directory and pass ctor args to the statement class Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: reset data-source output loop state * test(poller): isolate SQL dialect in cache regression --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
Thomas VincentandTheWitness
|
f2a5ef0d51 |
fix: cast path_php_binary before passing it to string parameters (#7576) (#7577)
* fix: cast path_php_binary before passing it to string parameters The setting has no default, so it is null on a fresh install, and PHP 9 turns the resulting coercion from a deprecation into a TypeError. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: append the changelog entry at the end of the issue block Matches .github/copilot-instructions.md, which appends rather than inserting in numeric order. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: use the full licence header on the new test files Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: cast path_php_binary at every site that escapes it The first pass covered three of them; a repo-wide scan found 21, and the test now fails if a new one appears. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: strengthen PHP binary path guard * fix: cover default-config PHP binary reads --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
Thomas VincentandTheWitness
|
453e747d45 |
fix: update device status by host id instead of hostname (#7427) (#7567)
* fix: update device status by host id instead of hostname Hostnames are not unique in Cacti, so keying the status write on hostname let sibling devices overwrite each other's availability every poll. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: add phpdoc to the issue 7427 test helpers Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: append the changelog entry at the end of the issue block Matches .github/copilot-instructions.md, which appends rather than inserting in numeric order. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: use the full licence header on the new test files Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: treat CACTI_PATH_LOG as a directory and pass ctor args to the statement class Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: restore host-status config state exactly --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
Thomas VincentandTheWitness
|
03f6241ad9 |
fix(errors): remove dead E_STRICT and always-true constant guards (#7546)
* fix(errors): drop E_STRICT references deprecated on PHP 8.4 No diagnostic has raised E_STRICT since PHP 8.0 and composer.json floors at 8.1, so the handler entries were dead weight that made 8.4 emit a deprecation for every error it handled. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * style: align GPL header border and drop PHP 4 comment in aggregate error handler Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(poller): drop the E_STRICT entry from the poller error handlers Nothing has raised that level since PHP 8.0 and reading the constant is deprecated on 8.4, so each handled error emitted a second deprecation. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: name the E_STRICT level numerically with a reason Spelling the constant would reintroduce the deprecation the change removes. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(errors): drop the E_RECOVERABLE_ERROR guards and fold the poller tests in The constant has existed since PHP 5.2 and the floor is 8.1, so both conditions are constant. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(poller): fold the always-true constant guards into the error maps Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
Thomas Vincent
|
46bc738bff |
fix(validator): guard null messages array in raise_message() (#7502)
* fix(validator): guard null messages array in raise_message() raise_message() reads the global $messages table (populated by include/global_arrays.php) and calls array_key_exists() on it unconditionally. Any caller that reaches raise_message() before that bootstrap file has run (e.g. CactiValidator::validateInput() invoked from a unit test) hits $messages === null, and PHP 8 throws a TypeError instead of the intended 'no predefined message' fallback. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(validator): stop asserting on the raise_message mock's side effect These tests conditionally defined a raise_message() stub only if the function did not already exist. PHPUnit/Pest loads every file under tests/Unit before running any test, so once one file requires lib/functions.php (or include/global.php), the real raise_message() wins the race for the rest of the process and the stub is silently never installed. The $GLOBALS['raised_message'] assertion then failed in a full-suite run even though CactiValidator::validateInput() itself behaved correctly. Keep the stub (needed so isolated runs don't hit 'call to undefined function'), drop the assertion that depends on which implementation actually ran. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas VincentandTheWitness
|
d8c5ef4b1a |
Fail gracefully when Symfony Mailer dependencies are stale (#7326)
Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
Thomas VincentandTheWitness
|
438ae8cdb8 |
Warn when data input commands contain unresolved substitution variables (#7270)
* Warn when data input commands contain unresolved substitution variables Unresolved <field> tokens were silently nulled out, shifting the script's positional arguments with no trace in the log. Ref #3744 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Add device context to the unresolved substitution warning Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: place CHANGELOG entry in release order --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
|
c741f71c5c |
refactor(mail): replace PHPMailer with Symfony Mailer (#7190)
* refactor(mail): replace phpmailer with symfony mailer Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * style(codex): fix coding standards in Symfony Mailer migration Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(codex): fix TLS downgrade bug and From address overwrite in Symfony Mailer Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(codex): guard SMTP stream setTimeout calls behind SocketStream check setTimeout() exists on SocketStream only; getStream() is typed AbstractStream, which fails PHPStan level 6. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(codex): correct inline graph CID, mandatory STARTTLS, and address validation in Symfony Mailer Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(codex): test inline CID, STARTTLS enforcement, and address-error handling Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(mail): qualify bare local addresses so sendmail recipients still work Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(mail): prune the stale PHPMailer vendor tree on upgrade Replacing PHPMailer with Symfony Mailer dropped phpmailer from composer.json, but the change also removed the prune_deprecated_files entries that cleaned up the vendored tree, so an upgraded install would keep stale PHPMailer code on disk. Add include/vendor/phpmailer (whole directory) to the prune list. Co-Authored-By: Oz <oz-agent@warp.dev> Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(mail): address transport review feedback Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(mail): drop setAutoTls() call missing from symfony/mailer 6.4 EsmtpTransport::setAutoTls() only exists in symfony/mailer >= 7.1, and the 6.4 series (needed for PHP 8.1) has no switch to disable opportunistic STARTTLS, so hide the advertised capability in a small subclass instead. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(mail): vendor the psr/log 1.1.4 sources include/vendor/psr/log was tracked with only its package metadata, so composer treats it as installed and never extracts the classes. That was harmless while nothing loaded them, but Symfony Mailer's AbstractTransport instantiates Psr\Log\NullLogger on every transport, which now fatals at runtime and in the test job. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(mail): match PHPMailer SMTP and attachment semantics Empty secure modes use no-STARTTLS transport; inline disposition is honoured; unreadable attachments fail closed. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: Thomas Vincent <thomas@vincent.dev> Co-authored-by: Oz <oz-agent@warp.dev> |
||
|
|
61a542a670 |
fix(auth): correct auth system bugs (#7187)
* fix(auth): correct auth system bugs Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Address auth PR review findings Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Guard remember cookie cache deletion Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(auth): prevent remote agent lockout on FCrDNS failure and add regression tests Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: update CHANGELOG for auth system correctness fixes Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(auth): stop returning 2fa secret from verification Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(auth): reject remote agent requests that fail forward-confirmed reverse DNS Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: ignore phpstan function.alreadyNarrowedType identifier Clears the level 6 alreadyNarrowedType error on lib/data_query.php:2591 that develop currently emits, matching the sibling narrowing identifiers already ignored. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: reword FCRDNS change as a refactor, not a security fix Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
|
b2bf39b8dc |
fix(sqli): sanitize sort_column and enforce sort_direction in ORDER BY clauses (#7162)
* fix: correct grammar and possessive errors in English source strings (batch 2) (#1) * fix: correct grammar and possessive errors in English source strings (batch 2) Seven errors found via cacti.pot audit: - automation_networks.php: "senders name" → "sender\'s name" (line 839) - automation_networks.php: "senders Email" → "sender\'s Email" (line 848) - automation_templates.php: "devices sysDescr" → "device\'s sysDescr" (line 1060) - automation_templates.php: "devices sysName" → "device\'s sysName" (line 1067) - automation_templates.php: "devices sysOid" → "device\'s sysOid" (line 1074) - data_templates.php: garbled "it either it value...critical data Data Query" → "its value...Data Query" (line 903) - package_repos.php: "Package Repositorites" → "Package Repositories" (line 436) All changes are to UI description strings inside __() / __esc() calls. cacti.pot should be regenerated via build_gettext.sh after merge. * fix: correct grammar and possessive errors in English source strings (batch 2) Seven errors found via cacti.pot audit: - automation_networks.php: "senders name" → "sender\'s name" (line 839) - automation_networks.php: "senders Email" → "sender\'s Email" (line 848) - automation_templates.php: "devices sysDescr" → "device\'s sysDescr" (line 1060) - automation_templates.php: "devices sysName" → "device\'s sysName" (line 1067) - automation_templates.php: "devices sysOid" → "device\'s sysOid" (line 1074) - data_templates.php: garbled string → "its value...Data Query" (line 903) - package_repos.php: "Package Repositorites" → "Package Repositories" (line 436) All changes are to UI description strings inside __() / __esc() calls. cacti.pot should be regenerated via build_gettext.sh after merge. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: add dependabot config with npm and github-actions * ci: add CodeQL (JS/TS, excludes PHP) with workflow_dispatch * ci: update CodeQL for javascript-typescript, python * fix(ci): use specific runner labels, fix codeql concurrency * Improve IPv6 support in RRDtool proxy and ping utilities (#2) * Improve IPv6 support in RRDtool proxy and ping utilities - Fix RRDtool proxy to dynamically detect IPv6 addresses and use AF_INET6 sockets instead of hardcoded AF_INET (IPv4-only), enabling connections to IPv6 RRDtool proxy servers including backup servers - Strip brackets from IPv6 addresses before passing to socket_connect - Properly close failed sockets before creating new ones for failover - Replace fragile str_contains(':') IPv6 detection in ping with filter_var(FILTER_FLAG_IPV6) for more robust address validation https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Fix JS code quality: implicit globals, deprecated APIs, loose equality - Add var declarations to prevent implicit globals in install.js (element, enabled, button, buttonCheck) - Remove console.log debug output left in production (install.js) - Replace deprecated jQuery .unbind() with .off() (layout.js) - Fix "depreciated" typo to "deprecated" in deprecation warnings - Convert == / != to === / !== for null, boolean, string, typeof, and numeric comparisons across install.js and realtime.js https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Replace deprecated jQuery shorthand event methods in layout.js Replace .click(), .keyup(), .keydown(), .mousedown(), .mouseenter(), .mouseleave(), .submit(), .resize() with .on() equivalents. Replace .focus(), .change() trigger calls with .trigger(). These shorthands were deprecated in jQuery 3.5. https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Replace deprecated jQuery methods in realtime.js Replace .bind() with .on() and .change() trigger calls with .trigger('change'). .bind() was deprecated in jQuery 3.0 and shorthand triggers in jQuery 3.5. https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Replace deprecated jQuery methods in install.js and fix ===/!== errors Replace .click(), .change(), .focus() with .on()/.trigger() equivalents. Also fix !=== and ==== operators that were incorrectly introduced by a prior replace-all of == to === within existing !== and === expressions. https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Restore == null where needed to catch both null and undefined In JavaScript, == null matches both null and undefined, which is an intentional idiom. The prior === null conversion broke cases where values come from jQuery .val(), .data(), $.urlParam(), or object property access that may return undefined rather than null. Revert those specific cases while keeping === null where variables are explicitly initialized to null. https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Replace deprecated jQuery methods in all theme main.js files Replace .unbind().click() with .off('click').on('click'), convert .hover() to .on('mouseenter').on('mouseleave'), replace .change(), .scroll(), .click() shorthands with .on() equivalents, and .blur() with .trigger('blur') across all 10 theme files. https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Convert string concatenation to template literals Replace 'str' + var + 'str' patterns with ES6 template literals in realtime.js and install.js. Improves readability especially for URL construction and HTML building. Also replace $.parseJSON() with native JSON.parse() in realtime.js. https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo * Convert var to const for single-assignment variables Replace var with const where the variable is assigned once and never reassigned within its scope, in install.js and realtime.js. Keeps var for variables that are conditionally reassigned (e.g. size, url). https://claude.ai/code/session_01SbuDigvAkYvPKvdcougsdo --------- Co-authored-by: Claude <noreply@anthropic.com> * fix(sqli): sanitize sort_column and enforce sort_direction in ORDER BY clauses Closes GHSA-q9xg-p762-9jm3 and GHSA-72vr-jr4v-55vf. - Add sanitize_sql_column() to lib/functions.php stripping all chars outside [a-zA-Z0-9_().] from user-supplied column names - Apply sanitize_sql_column() at every ORDER BY concatenation in api_automation.php and html_reports.php - Enforce direction to literal ASC/DESC in html_reports.php, api_automation.php, and html_utility.php (update_order_string, get_order_string) - Add tests/Unit/SafeSortTest.php with 22 stub-based regression tests Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(docs): move sanitize_sql_column above sanitize_search_string docblock The sanitize_search_string docblock was orphaned after sanitize_sql_column was inserted between it and the function body. Reorder so each function follows its own docblock directly. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(sqli): escape apostrophes in automation_templates and harden sort_direction in display_matching_hosts Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(sqli): restore allowlist guard, sanitize session keys, add sanitize_sql_column default Restore the preg_match allowlist guard in get_order_string() that was removed by the PR, re-opening the ORDER BY injection surface. When the request var is empty after sanitization, return '' rather than building a malformed ORDER BY clause. Sanitize sort_column before storing as SESSION array key in update_order_string() and add read-time guard in the inplace loop that rebuilds ORDER BY from stored keys. Add $default parameter to sanitize_sql_column() so callers get a safe fallback instead of an empty string that produces 'ORDER BY ASC'. Fix inconsistent INET_ATON substitution in display_matching_hosts(). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Improve sanitize_sql_column function Fix regex to sanitize SQL column names properly. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: TheWitness <thewitness@cacti.net> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Thomas VincentandTheWitness
|
c65f56d7e1 |
fix(graph): correct graph subsystem bugs (#7186)
* fix(graph): correct graph subsystem bugs * Address graph PR review findings * style: convert new code to short array syntax php-cs-fixer flagged array() in graph_realtime.php; convert to [] for the coding-standards gate. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * build: pin phpstan to 2.2.1 phpstan 2.2.2 added a stricter is_numeric() always-true inference that fails on pre-existing develop code (lib/data_query.php); pin until that is addressed upstream so CI is reproducible. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(rrd): key GPRINT CF cache per data source instance Two local data sources from the same data template shared a cache key, so a GPRINT could print another instance's consolidation function. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: ignore phpstan function.alreadyNarrowedType identifier Clears the level 6 alreadyNarrowedType error on lib/data_query.php:2591 that develop currently emits, matching the sibling narrowing identifiers already ignored. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
Thomas VincentandTheWitness
|
122bb88387 |
chore(security): harden report actions and theme handling (#7169)
* fix(security): align develop with advisory hardening Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(security): harden remote agent auth compatibility Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(security): reorder remote auth checks Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(security): harden remote_agent auth order and query scope Move whitelist check before the single-poller guard so single-poller installs relying on the whitelist are not incorrectly rejected. Narrow SELECT * to SELECT hostname; the loop only reads hostname. Drop function_exists guards around md5/hash; both are PHP core and the raw $cache_seed fallback is unsafe as an APCu key. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(theme): cast read_config_option result to string in cacti_validate_theme read_config_option can return false or null when the setting is absent. Without the cast, a strict comparison to '' would not match, leaving $default unset and risking a null-typed value flowing into callers. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(reports): skip invalid IDs in bulk action without aborting the loop Replace input_validate_input_number (which calls die_html_input_error on bad input, aborting the entire bulk action) with an inline guard. Cast $report_id to int at call sites where the signature requires int. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * feat(security): comprehensive UI hardening and 7.4 normalization - Refactored poller_spikekill.php to use array-based execution and sanitized SQL IN clauses. - Refactored script_server.php Windows process check to use cacti_exec. - Hardened snmpagent background process lifecycles. - Standardized version checks in support.php and utilities.php. - Replaced short array syntax [] with array() and verified 7.4 compatibility. - Synchronized all hardened files to primary developer path. * fix(snmpagent): replace undefined cacti_process_execute() with exec_background() cacti_process_execute() was called but never defined, causing a PHP fatal. Replaced with exec_background() from lib/poller.php. Also fix pre-existing parse error in snmpagent_persist.php ([ closed with ) instead of ]). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * style: convert new code to short array syntax php-cs-fixer (array_syntax/list_syntax short) flagged array() introduced on this branch; convert to [] so the coding-standards gate passes. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * revert off-topic exec changes to match develop poller_spikekill, script_server, snmpagent_persist/mibcache and support had been switched to 1.2.x-style cacti_exec(), which is undefined on develop and failed phpstan. Restore develop's native exec() idiom; these files are outside this PR's scope (report actions and theme handling). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * build: pin phpstan to 2.2.1 phpstan 2.2.2 added a stricter is_numeric() always-true inference that fails on pre-existing develop code (lib/data_query.php); pin until that is addressed upstream so CI is reproducible. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(reports): deserialize type-prefixed bulk items so actions run Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: ignore phpstan function.alreadyNarrowedType identifier Clears the level 6 alreadyNarrowedType error on lib/data_query.php:2591 that develop currently emits, matching the sibling narrowing identifiers already ignored. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: add missing PCRE delimiters to graph_template_id regexp filter Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(reports): parse the confirm list identically to the action branch Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * refactor(remote_agent): drop the APCu auth cache (uncertain dependency, cross-request only) Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Enhance security checks for remote agent access --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
Thomas VincentandTheWitness
|
20298c6cd6 |
fix(defensive): harden helper library input and output handling (#7197)
* fix(defensive): harden helper library input and output handling lib/html_utility.php: update_order_string() wrote sort_direction from the request directly into the session without enforcing ASC/DESC. Normalise to strtoupper and allowlist-check before every session write; default to ASC for any unrecognised value. lib/functions.php: - sanitize_uri(): add protocol-relative URL guard (leading '//' after ltrim). The function stripped most dangerous chars but had no entry for '//', which browsers treat as a scheme-relative redirect target. - is_valid_pathname(): the regex allows literal dots so '../..' passed validation. Reject any path containing '..' before the regex check. lib/database.php: db_qstr() fallback (no PDO connection) was missing \x1a (Ctrl+Z) from its escape map. PDO quote() handles it; the fallback now matches. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(defensive): correct hardening edge cases flagged in review Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(defensive): reject control-char redirect and drive-relative traversal Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: ignore phpstan function.alreadyNarrowedType identifier Clears the level 6 alreadyNarrowedType error on lib/data_query.php:2591 that develop currently emits, matching the sibling narrowing identifiers already ignored. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
Thomas VincentandTheWitness
|
7a1e9bbf92 |
harden CSV encoding and process execution handling (#7196)
* fix: harden CSV encoding and process execution handling Route CSV export cells through a shared cacti_csv_cell() helper so values with quotes, commas, or leading formula characters round-trip correctly, and add defensive guards on shell construction, unchecked proc_open handles, discarded exec exit codes, and gap values in nth-percentile fetch math. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: address review feedback on CSV and process hardening - escape poller_id in cmd_realtime proc_open command to match remote_agent Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: keep numeric CSV cells unquoted and guard snmptrap and rrdtool process failures - exempt numeric values from the CSV formula-injection prefix and preserve empty snmptrap positions while bailing out when the RRDtool process fails to spawn Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: ignore phpstan function.alreadyNarrowedType identifier Clears the level 6 alreadyNarrowedType error on lib/data_query.php:2591 that develop currently emits, matching the sibling narrowing identifiers already ignored. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |