mirror of
https://github.com/Cacti/cacti.git
synced 2026-10-23 03:27:02 +09:00
develop
11
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
5403cf75bb |
feature: user-defined filter layouts with administrator publishing (#8185)
* feature: user-defined filter layouts with administrator publishing
Add a render_layouts toggle to the CactiTableFilter class so list pages
offer Save / New / Rename / Delete controls for saved filter layouts,
modelled on the monitor plugin. Layouts persist to a new user_layouts
table (id, user_id, page, name, url). The toggle defaults on for every
page except the Console > Presets pages (CDEFs, VDEFs, Colors, GPRINTs,
Data Profiles, SNMP), where it is off.
Each user manages their own layouts from any filter. Administrators who
hold the Settings/Utilities realm can publish a layout to all users
(moving it to user_id = 0) from the filter, and from a new
Console > Presets > Filters management page (layouts.php) where they can
view and manage every user's layouts. Published layouts appear for all
users on that page.
Layout create/rename/delete/publish requests are handled centrally by
filter_layouts_handle_request(), invoked from include/auth.php after the
user has been authorized for the current page, so the controls work on any
filter page while publishing stays gated on the realm. layouts.php itself
is mapped to the Settings/Utilities realm so the Presets > Filters menu
pick is administrator-only.
Schema added to cacti.sql, install/upgrades/1_3_0.php, and
docs/audit_schema.sql. Adds tests/Unit/FilterLayoutsTest.php covering the
URL-safety validation, preset-page defaults, and the request/permission
wiring.
* changelog: 8185.feature fragment for filter layouts
* chore: enforce LF line endings via .gitattributes and normalize new files
* fix: address review - attribute-escape layout options, honor failed/non-editable layout ops, regenerate cacti.pot, move test to Unit/Ui/Html
* chore: renormalize pre-existing CRLF files to LF per .gitattributes
* chore: keep vendored JS (big.js, tablesorter) byte-identical to upstream and exclude from LF normalization
* style: apply php-cs-fixer formatting and resolve PHPStan level 8 findings in filter layouts
* ci: re-trigger checks on current head
* refactor: store filter layouts as a versioned JSON document in a TEXT data column
Replace the user_layouts.url varchar(1024) column with a data TEXT column holding a versioned {version,page,filter} document. The navigable url is rebuilt server-side from the document (page basename + rebuilt query) plus the filter_layout marker, removing the stored-url redirect/XSS surface and making the backend forward-compatible with a structured query builder without a further schema change.
* feature: add Modern/Legacy page filter format with an Edit dialog for filters
Modern (default) collapses the filter to a Layouts selector plus an Edit dialog holding the Filter Name and all filter fields; Save and Publish live in the dialog, with Edit/Rename/Delete/Save As on the bar and time controls/refresh kept on the bar. Legacy keeps the classic inline filter. The format is a per-user General setting (page_filter_format). Field rendering is factored into emit_field() so both layouts share one source.
* fix: render page-action buttons (import, export, sort) on the modern filter bar
Import, Export, Purge, Save, and sort ascending/descending are page actions, not filter inputs, so they render on the bar beside the Layouts selector instead of inside the Edit dialog. Only go/clear are dropped from the bar; the dialog keeps the Search/Clear footer.
* feature: render the modern Edit dialog one filter variable per row
The dialog no longer mirrors the inline filter's multi-field row packing; each filter variable renders on its own row (label + control) for a clean, responsive form. Hidden/validate fields stay in the form without an empty visible row. Rows carry a cactiFilterEditRow hook for label-column alignment styling.
* feature: align the modern edit dialog one-per-row and skin it per theme
Render all dialog fields in a single table so the label column aligns via the existing table-cell layout. Tag the dialog widget with cactiFilterDialog and add a scoped skin to the modern theme (slate titlebar, styled button pane, accented primary action). Add a bar separator between layout and page-action buttons.
* feature: skin the modern filter Edit dialog across the classic themes
Add the scoped .cactiFilterDialog skin (accent titlebar, white title/close, styled button pane with an accented primary action) to cacti, carrot, dark, hollyberry, paper-plane, paw, raspberry, and sunrise, each using its own brand accent. The skin is accent-only and inherits each theme's dialog surface, so it works on light and dark themes. MidWinter is intentionally skipped (its variable-based CSS is handled in a separate Epic).
* fix: delete a user's saved filter layouts when the user is removed
user_remove() now deletes the user's own user_layouts rows alongside their other per-user data. Scoped to user_id so published (user_id = 0) layouts remain available to everyone.
* fix: address PR review — PHPStan return type, escaping, callbacks, SQL columns, mb truncation, contrast, POT
- create_filter(): cast both ob_get_clean() returns to string and drop the now-unused PHPStan baseline entry (fixes Level 6 failure).
- Edit dialog title now uses html_escape_attr() (attribute-context double-encoding).
- Modern Search/Clear honor the page's configured go/clear callbacks instead of hardcoding applyFilter()/clearFilter().
- filter_layouts_get() and layouts.php select explicit columns instead of *.
- Layout name truncation uses mb_substr() to avoid cutting multibyte codepoints.
- layouts.php management callback checks result.ok before reloading.
- carrot/paper-plane dialog accents darkened to meet WCAG AA (>=4.5:1) with white text.
- Added the new translatable msgids (Page Filter Format, Filters, Edit Filter, Filter Name, Save As, ...) to cacti.pot.
* fix: resolve Semgrep reflected-xss by reading filter_layout inside layout_select
The request var is no longer passed into layout_select() as a parameter (which Semgrep tracked parameter-to-return to the print sink); it is read internally like create_filter(), and the option markup remains escaped. No behavior change.
---------
Co-authored-by: TheWitness <copilot@users.noreply.github.com>
Co-authored-by: TheWitness <TheWitness@users.noreply.github.com>
|
||
|
TheWitness
|
f79dc15c08 |
Port #8166 SNMP credential cache + procedural SNMPv3 (with #8171/#8172 follow-ups) to develop (#8174)
* Port #8166 SNMP credential cache + procedural SNMPv3 to develop Ports the shared SNMPv3 credential cache and procedural SNMPv3 routing from the 1.2.x feature (merged as #8166) onto develop, adapted to develop's refactored SNMP layer. Parts of the 1.2.x change that do not apply to develop are omitted: the cacti_exec() idle-timeout rewrite (develop has no cacti_exec()), the PHP floor bump and composer/doc changes (develop is already 8.3+), and the cmd.php max_oids batching (develop's cmd.php already collects SNMP in-process through persistent sessions, so the per-item spawn the batching removed does not occur). - lib/cache.php: new cross-process \Cacti\Cache\SharedCache (file + POSIX shared-memory + process-local tiers, optional AES-256-GCM sealing). Loaded via lib/snmp.php (the Cacti\Cache\ namespace is not PSR-4 autoloaded). The crypto helpers it seals with already live in develop's lib/auth.php, loaded by global.php, so no duplicate was added. - lib/snmp.php: snmp_get_method() now routes SNMPv3 get/getnext to the procedural snmp3_*() calls when the running PHP supports the configured auth/privacy tokens and no context/engine id is set (SHA-224/384 and AES-192/256[C] stay on the binary before PHP 8.6); the three single-get functions gain the procedural v3 path with \Throwable handling; adds the credential-cache subsystem, protocol-option/MD5-DES helpers, snmp_native_protocol(), snmp_format_agent() and cacti_snmp_get_multi(); the binary v3 auth builder consults the cache when enabled. - poller.php / script_server.php: prime / load the shared cache. - poller_maintenance.php: daily out-of-band cache rebuild on every collector (per-poller lastrun marker). - lib/api_device.php / lib/utility.php: bump snmp_cred_version only on v3 credential add/change (and propagate the token to the affected remote poller). - include/global.php: resolve and create the private cache/ directory. - include/global_settings.php: add 'Enable Credential Cache' and 'Enable MD5 and DES for SNMPv3' settings (both off by default); default new-device v3 auth/privacy to SHA/AES instead of legacy MD5/DES. - host.php / automation_snmp.php: drop MD5/DES from the pickers when disabled and default new records to SHA/AES on upgraded installs. - include/config.php.dist: document $config['cache_dir']. - tests/Unit/Core/Cache/SharedCacheTest.php: SharedCache behaviour coverage. - tests/security/baselines: baseline the two new lib/cache.php sinks. * Incorporate #8171: binary fallback for unsupported SNMPv3 protocols Layers PR #8171 onto the develop port so an SNMPv3 device whose auth/privacy protocols the running PHP's ext-snmp cannot service natively (e.g. AES-256 before PHP 8.6) no longer fatals the poller or device-edit screen. - lib/snmp.php: add CactiSnmpBinarySession, a session stand-in exposing the get/getnext/walk/close interface the cacti_snmp_session_*() helpers use and delegating each call to cacti_snmp_*() -> the Net-SNMP binary. cacti_snmp_session() returns it when php-snmp is absent or the v3 protocol is not natively serviceable, so cmd.php/host.php collect these devices via the binary on pre-8.6 and switch to the in-process path on 8.6 with no caller change. v1/v2 snmpget/snmp2_get now go through snmp_format_agent(). The ERRNO_TIMEOUT check tolerates php-snmp being absent. Drop the vendored classSNMP.php compatibility shim and its include (and the Yandex attribution). - cmd.php: open_snmp_session() returns the down sentinel when a host is already marked down this run, instead of returning an unset (null) [] that cacti_snmp_session_get() then dereferenced (TypeError). - lib/ping.php: ping_snmp() runs the availability check through the procedural cacti_snmp_get()/getnext() path so an unsupported protocol falls back to the binary instead of raising a ValueError in SNMP::setSecurity(). * Incorporate #8172: retire include/vendor/phpsnmp, relocate into lib Follow-up to #8171 on the develop port. After classSNMP.php was removed, the include/vendor/phpsnmp/ directory held only Cacti-owned files; this retires it. - lib/snmp.php: the ext-snmp subclass that carried Cacti's bulk_walk_size / value_output_format (previously vendor/phpsnmp/extension.php) is now declared directly as CactiSnmpNativeSession, guarded by CACTI_PHP_SNMP; the include and the 'use phpsnmp\SNMP' alias are gone, and new SNMP(...) / SNMP::VERSION_* now reference the native \SNMP and the new subclass. - lib/mib_parser.php: moved from include/vendor/phpsnmp/mib_parser.php; lib/mib_cache.php includes it via __DIR__. - Remove include/vendor/phpsnmp/extension.php and index.php (directory retired). - tests: SnmpCoverageTest uses \SNMP::ERRNO_TIMEOUT now that phpsnmp\SNMP is gone; drop the stale include/vendor/phpsnmp/ entry from the vendor-policy allowlist. develop's sink baseline tracks constant-based includes differently than 1.2.x, so no baseline refresh was needed here. * Address PR #8174 review: snmp_get_method params, v3 walk, getnext agent, cache bump, CHANGELOG - lib/snmp.php: restore $php_snmp as the 6th snmp_get_method() parameter and move the new $auth_proto/$priv_proto after it (callers pass them by name), so the positional $php_snmp used by SnmpCoverageTest and others keeps working. - lib/snmp.php: keep every SNMPv3 operation except get/getnext on the Net-SNMP binary - the native walk branch implements only v1/v2, so a v3 walk reaching the procedural gate would have silently returned empty when path_snmpbulkwalk is missing. - lib/snmp.php: cacti_snmp_getnext() v1/v2 branches now use snmp_format_agent() like get/get_raw, so an IPv6 literal / non-default port is bracketed. - lib/api_device.php: bump snmp_cred_version only when the host's v3 membership or one of the cached credential fields actually changed, instead of on every save of a v3 device. - CHANGELOG: move the #8171 entry into the issue block and the #8166 entries into the feature block, matching the established section ordering. * Fix CI: unbracket IPv4/DNS SNMP targets, inline auth-cache SQL columns, apply phpcsfixer standard * Refresh sink inventory baseline after short-array normalization * Restore lib/snmp.php 100% coverage after aligning v3 routing to 1.2.x - update the three SnmpCoverageTest assertions that still encoded develop's pre-port behavior: v3 snmp_get_method now returns PHP, an unsupported- protocol v3 session returns the binary-delegating session object, and the v3 cacti_snmp_get path runs natively - add a v3 noAuthNoPriv user to the snmp-coverage snmpd fixture so native snmp3_get is exercised deterministically - add tests covering the ported subsystems to 100%: shared auth cache, binary-delegating session, protocol pickers, v3 credential args, cacti_snmp_get_multi, and the cacti_get_snmpv3_auth cache path - snmp_auth_cache_load uses a resettable process guard so its enabled and disabled branches are both coverable in a single process * Resolve phpstan errors in the relocated SNMP MIB parser and ported snmp.php - lib/mib_parser.php: type the MibParser properties, add parameter/return types, guard file_get_contents()/preg_split() against false, correct the parse_bracket_token delimiter params (string, not integer), and drop the unreachable return after trigger_error(E_USER_ERROR) - now phpstan clean - lib/mib_cache.php: drop the redundant isset($mp->oids) guard (the typed property is always initialized; $mp->mib is the real check) - lib/snmp.php: type the CactiSnmpBinarySession and CactiSnmpNativeSession properties/methods and pass bool to snmp_set_quick_print() - .phpstan.neon: ignore greaterOrEqual.alwaysFalse alongside the existing greater.alwaysFalse (the PHP_VERSION_ID >= 80600 forward-compat gate) - regenerate phpstan-baseline.neon for the irreducible ext-snmp stub strictness (snmp3_get literal-union protocols, int<-1,max> timeouts) in the ported procedural v3 path, the new lib/cache.php shmop stubs, and the $config array-shape offsets in include/global.php * Make the v3 getter catch coverage net-snmp-build-independent The DES-rejection trick that drove the procedural snmp3_get()/snmp3_getnext() try/catch blocks only works on net-snmp builds compiled without DES; CI's Debian net-snmp accepts DES, so those catches went uncovered (98.3%). - extend the existing native_get injection seam (already on cacti_snmp_get) to cacti_snmp_get_raw, cacti_snmp_getnext, and cacti_snmp_get_multi, and drive the catch blocks with a throwing native getter in the test - mark the seam-less cacti_snmp_session() setSecurity catch with @codeCoverageIgnore (it only throws for a protocol the running net-snmp rejects, which is not portably reproducible in the fixture) * Exclude ext-snmp/PHP-8.6-gated defensive branches from snmp coverage Two branches in lib/snmp.php are unreachable on the PHP 8.3 + ext-snmp coverage runner and were only "covered" locally via opcache dead-code elimination: - snmp_get_method()'s !function_exists('snmp3_get') fallback (ext-snmp always provides snmp3_get) - snmp_php_v3_protocols_supported()'s PHP_VERSION_ID >= 80600 block (SHA-224/384 and AES-192/256 tokens are only accepted on PHP 8.6+) Mark both with @codeCoverageIgnore so the 100% gate is deterministic across runners regardless of opcache settings. * Account for the #8166 SNMPv3 credential-cache gate query in the batching test poller_update_poller_cache_from_buffer() now runs one SELECT per flush to decide whether the flush touched SNMPv3 poller items before bumping the shared credential-cache version (matching 1.2.x). UpdateDataQueryCacheBatchingIntegrationTest counted 4 statements per flush; update it to 5 (and 3 per-row flushes to 15, 1 batched flush to 5). The guard's intent - batching issues fewer statements than per-row flushes - still holds. * Port #8175: group SNMP options under an SNMP Behavior section Mirror the 1.2.x settings reorganization (#8175) on develop. develop already keeps the SNMP options in a dedicated SNMP tab, so add an "SNMP Behavior" spacer there, order the behavior options as md5/des, credential cache, max get size, bulkwalk fetch size, and move "Disable increasing OID Check" out of the Poller tab to sit last in that group. Display-only; no settings added or removed and no behavior change. Adds the "SNMP Behavior" heading to cacti.pot. * fix: complete dependabot htmx 2.0.10 to 2.0.11 bump Dependabot bumped package.json and package-lock.json to htmx.org 2.0.11 (PR 8179) but left the vendored include/js/htmx.js, its version marker, and the pinned SRI constant at 2.0.10. The htmx-filters CI workflow runs npm ci, which regenerates include/js/htmx.js from the 2.0.11 package, so HtmxTest's runtime sha384 no longer matches the HTMX_2_0_10_SRI constant and the integrity assertion fails. Complete the bump: regenerate the vendored include/js/htmx.js to the 2.0.11 build, bump include/js/htmx.js.version to 2.0.11, rename HTMX_2_0_10_SRI to HTMX_2_0_11_SRI with the recomputed sha384 hash, and update the version/integrity assertions in tests/Unit/Ui/Html/HtmxTest.php and tests/HandOff/HtmxLoaderHandOffTest.php. package.json and package-lock.json match the values dependabot already landed on develop. * settings: move SNMP Agent Support under the SNMP Behavior spacer Relocate the enable_snmp_agent setting (SNMP Agent Support Enabled) so it is the first item under the SNMP Behavior spacer instead of sitting in the Poller General group, grouping it with the other SNMP behavior options. * Address PR #8174 review: keep SHA-224/384 on the binary, honor port on transport targets Resolves the two still-valid Copilot review findings on lib/snmp.php that were left unresolved (only marked outdated after later commits shifted their lines). - snmp_php_v3_protocols_supported(): drop SHA224/SHA384 from the native auth allowlist. ext-snmp's SNMP::setSecurity()/snmp3_*() only map MD5/SHA/SHA256/ SHA512 - there is no SHA-224/SHA-384 mapping on any build, including PHP 8.6 (verified against php-src ext/snmp/snmp.c), so routing those tokens to the procedural path raised a ValueError instead of using the binary. They now always fall back to the Net-SNMP binary. The AES-192/256 privacy tokens that ext-snmp does add on 8.6 stay gated behind PHP_VERSION_ID >= 80600. - snmp_format_agent(): a udp/tcp transport-qualified target that omits a port now has the configured non-default port appended (udp6:[2001:db8::1] + 1161 -> udp6:[2001:db8::1]:1161), matching the Net-SNMP binary path. Targets that already carry a port, and unix: sockets (no port), are left untouched. - SnmpCoverageTest: cover the new transport-target branches; lib/snmp.php stays at 100%. |
||
|
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
|
bb92c46e78 |
feat(auth): automatic User Group assignment from Login Provider groups (#8129)
* feat(auth): automatic User Group assignment from Login Provider groups Add per-Login-Provider "Auto Assignment" rules to the User Group editor so a user is added to (or removed from) a Cacti User Group at every login based on the groups their Login Provider reports. User Group editor (user_group_admin.php): - Add General / Auto Assignment / Rights spacers to the General tab and rename "General Group Options" to "Group Options". - Render one text field per eligible Login Provider under Auto Assignment (label = provider name). The admin enters a plain Group Name (e.g. "Cacti Admins"), never a DN. - Store the rules as JSON in the new user_auth_group.auto_assignments column, keyed by login_providers.id. Group resolution: - SAML2 and OpenID match the configured name against the group attribute / group claim values. - LDAP/AD resolve membership live against the directory using a raw group name plus the Search Base, trying AD (memberOf + nested LDAP_MATCHING_RULE_IN_CHAIN), FreeIPA / 389 DS (groupOfNames/groupOfUniqueNames) and RFC2307 posixGroup shapes so one plain name works everywhere. This requires a directory-read bind, so the field is only offered for Anonymous Searching or Specific Searching with a stored service account. - A directory that cannot be reached returns "undetermined" so existing memberships are left untouched rather than revoked on a transient outage. Lifecycle: - auto_assignments is reconciled on every login (lib/auth.php credential flow and login_sso.php redirect flow). - Deleting a Login Provider strips its key from every group's auto_assignments. Schema: add user_auth_group.auto_assignments (cacti.sql + 1_3_0 upgrade). * ci: document free-text group-name validateInput for constraint ratchet * fix(auth): address PR review on automatic User Group assignment - LDAP: add an Active Directory bare-name query (objectClass=group with the member:1.2.840.113556.1.4.1941 nested-membership rule) so a plain group name resolves on AD, which neither the posixGroup nor groupOfNames probe could match. - LDAP: make single-group membership tri-state and propagate an "undetermined" result, so a search that fails after binding (timeout/dropped connection) no longer looks like confirmed non-membership and revokes assignments. - SAML2/OpenID: supportsAutoAssignment() now requires a configured group claim, and applyAutoAssignments() skips any provider that is not currently capable, so a blank claim can never strip users from their groups. - Only reset the permission cache when a membership row actually changed (accumulate db_affected_rows) rather than on every login that has a rule. - Add unit tests for the reconciliation boundaries (add / remove / no-op / incapable / undetermined) and for the LDAP group-query construction (AD plain + nested, posixGroup, groupOfNames, DN mode). - Regenerate locales/po/cacti.pot for the new UI strings. * fix(phpstan): resolve level-8 errors from auto group assignment - Guard supportsAutoAssignment() behind an instanceof AbstractLoginProvider check so it is not called on the base LoginProviderInterface. - Assign the injected field list to a variable (inject_form_variables takes it by reference) and pass an is_array-guarded group row. - Bump the lib/ldap.php ldap_get_entries baseline count 3->6 for the three new group-membership query helpers. |
||
|
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
|
4afe89966e |
build: analyse at PHPStan level 8 and unpin the analyser (#7918)
* build: analyse at PHPStan level 8 and unpin the analyser The level lived in the composer script as --level 6 while .phpstan.neon declared none, so anyone running phpstan directly analysed at level 0 and saw nothing. The level now sits in the config where both CI and a developer get the same answer. Level 8 finds 2787 issues on the current tree, so the baseline is regenerated rather than the debt fixed here. The gate passes on what exists and fails on anything new, which is the point: a fresh offset access on a db_fetch_row() result is now caught in review instead of at runtime. The debt is not 2787 unrelated problems. 1556 of them are the same shape, "Cannot access offset X on array|bool", from the db_fetch_* family declaring bool|array and call sites indexing the result without testing it. Narrowing those return types would clear more than half in one change, which is worth doing on its own. phpstan was pinned to an exact 2.2.9, so patch releases were not reaching CI. It now floats on ^2.2.12. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * build: refresh the PHPStan baseline against current develop The baseline was generated while phpseclib was temporarily pinned back to the 3.x line. develop has since migrated the call sites to phpseclib4, so three entries no longer match anything. reportUnmatchedIgnoredErrors is false in .phpstan.neon, so stale entries are ignored rather than reported and would have sat there indefinitely. The migration introduced no new level 8 findings: 1483 entries become 1480, with nothing added. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * build: refresh PHPStan baseline after rebase * build: refresh baseline after transaction fix Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas VincentandTheWitness
|
364be26ffa |
chore(test): gate test-only DB short-circuit behind CACTI_TEST_BOOTSTRAP env (#7128)
* chore(test): gate test-only DB short-circuit behind CACTI_TEST_BOOTSTRAP env
The previous `!defined('PHP_TESTING')` guard skipped db_connect_real
whenever PHP_TESTING was set anywhere in the include chain. A stray
define from a CLI tool or third-party script could disable real DB
connection logic in a deployed environment.
Switch the guard to require both `PHP_TESTING` and an explicit
`CACTI_TEST_BOOTSTRAP=1` env var, and replace the silent-no-connect
path with a `Cacti_TestDbSentinel` whose every method throws. Any
production code path that accidentally treats the sentinel as a real
DB handle will fail loudly instead of silently misbehaving.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* fix(test): apply Copilot review feedback for test DB sentinel guard
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* test: collapse remaining test-bootstrap guard to $is_test_bootstrap
The previous review fix introduced a local $is_test_bootstrap flag but left
two call sites still inlining defined('PHP_TESTING') && getenv(...). Replace
the long-form check and guard the single-poller cacti_db_version fetch so the
sentinel branch finishes initialising without invoking the DB.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* chore(test): arm CACTI_TEST_BOOTSTRAP in test bootstraps and fail closed on PHP_TESTING alone
Set the env var where PHP_TESTING is defined so legitimate test runs short-circuit the DB connect again, gate the config.php.dist swap and schema-validation skip on the combined predicate, read it with local_only, and always seed cacti_db_version.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
* ci: ignore phpstan function.alreadyNarrowedType identifier
Clears the level 6 alreadyNarrowedType error on lib/data_query.php:2591 that
develop currently emits, matching the sibling narrowing identifiers already ignored.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
|
||
|
Thomas VincentandTheWitness
|
ae4c147f04 |
chore(test): unified Pest test infrastructure with HandOff support (#7124)
* test: shared hand-off + mutation infrastructure for feature PRs Consolidates infection/infection dev dep, allow-plugins entry, baseline infection.json5, tests/HandOff/HandOffHelpers.php with reusable stubs (cacti_log capture, temp-file fixtures, minimal-zip builder), HandOff testsuite registration, and the documented pattern. Feature PRs that add hand-off coverage rebase onto this and contribute only ONE HandOffTest file plus an optional infection.json5 filter override. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: consolidate per-feature hand-off suites into shared infra Each test guards on its feature file and skips when not present on develop. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(test): replace eval stub, unify log buffer ref, guard ZipArchive add Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(ci): correct infection vendor path in infection.json5 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci(phpstan): baseline upstream undefined-variable and isset errors Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: add HandOff and Integration testsuites; add regression guards - phpunit.xml: add Integration testsuite alongside existing Unit/HandOff - tests/Integration/: DbDumpIntegrationTest, PingIntegrationTest (from develop), CactiProcessIntegrationTest, SqlScriptsIntegrationTest - tests/HandOff/RegressionGuardTest: guards against backsliding on shell_exec, password-in-argv, RLIKE concat, and open-redirect patterns Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * chore(test): pest test infrastructure and PSR-4 autoload Add Pest 2 dev dependency, regenerate composer.lock under PHP 8.1, declare PSR-4 autoload for the Cacti namespace, and seed the test bootstrap with three unit tests plus an orb integration check. Pest stays on the v2 line because v3 transitively requires PHP 8.2 and breaks the 8.1 matrix entry. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * chore(phpstan): catch up baseline with 11 upstream-detected entries Cacti Commit Audit fails on develop with 11 PHPStan errors that are not yet in phpstan-baseline.neon. Append the matching entries so the PR-A branch passes Run PHPStan at Level 6. Same set of entries already exists on PR #7077; this is a transient catch-up that upstream will absorb when those entries land on develop. Files: aggregate_graphs.php, color_templates.php, graph_templates.php, graphs.php, lib/html.php (3 entries). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * chore(test): drop composer.lock; let CI resolve per PHP version The lock file pinned brianium/paratest v7.3.1 which only supports PHP 8.1-8.3, so the 8.4 matrix entry rejected the lock with "Your lock file does not contain a compatible set of packages". A single lock cannot satisfy 8.1 (paratest 7.3.x) and 8.4 (paratest 7.4.x) at the same time because pestphp/pest 2.36.0 is the last 2.x release that supports 8.1 and the 8.1-compatible paratest tag predates 8.4 support. Match upstream develop's behaviour: no committed lock; let `composer install` in CI resolve per matrix entry. Pest stays on the v2 line in composer.json so 8.1 still gets a compatible Pest. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(test): apply Copilot review feedback for test infra Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * chore(test): drop psr-4 autoload and align deps with canonical PRs Match the flat un-namespaced lib/CactiX.php convention used by #7088, #7073, #7077, #7075. Drop symfony/process (added by #7073) and symfony/validator (added by #7077) to avoid composer.json conflicts at merge time. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: skip integration suites and regression guards pending feature PRs Guards integration tests and regression guards from #7083 against absence of CactiProcess and other feature-PR hardening, so the consolidated foundation PR is green on develop. Suites activate automatically once their feature PRs merge. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(ci): drop infection/infection to unbreak PHP 8.4 matrix infection/infection ^0.27 transitively pins thecodingmachine/safe ^2.1.2, resolving to v2.5.0. Its generated stubs use implicit-nullable parameter declarations, which PHP 8.4 emits as deprecations into cacti.log and trips the log-quiet assertion. infection has no config or test wiring in this repo, so removing the dev dep clears the transitive pin without losing functionality. 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
|
44ad795cdc |
fix: prevent empty CDEF RPN expressions in aggregate graphs (#6842)
* fix: prevent empty CDEF RPN expressions in aggregate graphs aggregate_cdef_totalling() processed GPRINT/LEGEND items (graph_type_id 9-15), assigning them aggregate CDEFs with SIMILAR_DATA_SOURCES_NODUPS. The expansion loop in rrd.php only resolves those magic variables for AREA/STACK/LINE items, so when the consolidation function did not match, the RPN expression was empty, producing "CDEF:cdefX=" which rrdtool rejects with "cannot parse an empty rpn expression". Two fixes: 1. Filter GPRINT and LEGEND graph types from the aggregate_cdef_totalling query so they never receive aggregate CDEFs they cannot use. 2. Guard against empty CDEF strings in rrd.php before emission as a safety net for any other code path that might produce one. Fixes #6451 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: review improvements for aggregate CDEF fix - Use db_fetch_assoc_prepared() with parameterized placeholders - Add cacti_log() debug trace when empty CDEF guard triggers - Add drift detection, boundary, and edge case tests Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: address review findings for aggregate CDEF fix - Convert remaining db_fetch_assoc to db_fetch_assoc_prepared - Convert db_execute to db_execute_prepared for UPDATE query - Add null CDEF guard documentation test - Add edge case tests for drift detection and boundary values Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * style: apply php-cs-fixer formatting Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: add sync warnings to test helpers Test helpers replicate production logic from lib/aggregate.php and lib/rrd.php because the production functions require DB/rrdtool state. Add comments documenting this dependency and the source lines that must stay in sync. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * revert: remove unrelated CS changes from review hook 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
|
6112aa12a0 |
hardening: escape hash parameter in auth_resetpassword.php form (#6771)
* fix: escape hash output in auth_resetpassword.php Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: add AuthResetpasswordXssTest for XSS fix * fix: use htmlerv() for hash parameter in reset password form TheWitness review: htmlerv() is the canonical alias for html_escape_request_var(), consistent with Cacti request-var conventions. Replace htmle(gnrv('hash')) with htmlerv('hash') at both form inputs. Also add phpstan-baseline.neon for 4 pre-existing level-6 errors so the pre-commit hook passes on a clean checkout. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |