Feature/auth session audit - #64
Open
bmfmancini wants to merge 40 commits into
Open
Conversation
security: harden audit logging and management - prevent stored XSS in audit views - require CSRF-protected POST and management permission for purging - recursively redact web and CLI credentials - generate safe, standards-compliant CSV exports - record hook events explicitly as attempted actions - harden retention, replication, and external logging - add security regression checks - bump plugin version to 1.3
security: complete audit logging hardening - bound request depth, field counts, string sizes, and JSON parsing - detect and redact additional secret-shaped values - finalize request outcomes as completed or failed - track and retry failed external log delivery - expose delivery status in the UI and CSV exports - upgrade existing remote-poller schemas - add retention, schema, delivery, and security regression tests - update documentation and translation template - bump plugin version to 1.4
This will make it easier for some SIEMS like splunk to ingest the log file
feat: add normalized compliance audit event capture - add event and correlation UUIDs with integrity metadata - record actor, target, outcome, timing, and event classification - deliver finalized request outcomes to external log consumers - audit log views, searches, details, exports, and purges - capture logout and session-timeout events on Cacti 1.2.x - finalize CLI audit events and expand CSV exports - add schema migration, documentation, and tests
…ce-event-capture # Conflicts: # .github/workflows/plugin-ci-workflow.yml # CHANGELOG.md # README.md # audit.php # audit_functions.php # setup.php
…-integrity-governance
- composer.json with php-cs-fixer ^3.86, phpstan ^2.2, pest ^2 - .php-cs-fixer.php copied verbatim from Cacti develop (tabs, custom ruleset) - .phpstan.neon matching Cacti shape, paths adapted, level 8 - phpstan/stubs/cacti.stubs.php typed Cacti function signatures - .editorconfig (tabs for PHP/JS/SH, binary for po/pot/mo) - .gitignore updated for vendor/, caches, baseline Phase 1 of code quality sweep. No functionality changes.
- Convert array() to short array syntax [] - Normalize switch/case indentation (PSR-12) - Convert dirname(__FILE__) to __DIR__ - Normalize else if to elseif - Apply binary operator alignment (=> = ===) - Single quotes, concat spaces, trailing whitespace - Fix controller_security_test assertions for short array syntax - Fix SetupStructureTest to check INFO file instead of source regex Phase 2 of code quality sweep. No functionality changes.
- Add declare(strict_types=1) to all in-scope PHP files - Replace Php74CompatibilityTest with Php81SyntaxTest - Php81SyntaxTest asserts strict_types and short array syntax - CS-Fixer normalizes declare spacing to Cacti convention Phase 3 (partial) of code quality sweep. No functionality changes.
- Remove declare(strict_types=1) from all PHP files per team lead directive - Update Php81SyntaxTest to not assert strict_types - strict_types will not be supported until Cacti 1.3 is released No functionality changes.
- audit_syslog.php: 38 functions typed (params + return types) - audit_functions.php: 30 functions typed (params + return types) - setup.php: 19 functions typed (params + return types) - audit.php: 7 functions typed (params + return types) - Uses union types (array|false, int|false), mixed, nullable (?T), void - No strict_types (deferred until Cacti 1.3) - No functionality changes — types match existing runtime values Phase 3 of code quality sweep.
- Add @param/@return array value type docblocks (40+ missingType.iterableValue) - Add is_array() guards for foreach on db_fetch results (7 foreach.nonIterable) - Add is_array/null-coalescing guards for array|false offset access (6 errors) - Add null coalescing for string|null arguments (preg_replace, trim, substr) - Add resource|false guards for fputcsv/fclose - Fix audit_json_encode fallback to guarantee string return - Fix audit_syslog_config null coalescing for optional keys (20 offsetAccess.notFound) - Fix stubs: __() variadic, api_plugin_register_hook 5th param, db_add_index mixed - Add ignoreErrors for includeOnce.fileNotFound, treatPhpDocTypesAsCertain: false - Regenerate baseline (210 errors, all in test files / pre-existing debt) - PHPStan with baseline: 0 errors. Without baseline: 7 source errors (all ignored) Phase 4 of code quality sweep. No functionality changes.
- .github/workflows/code-quality.yml: PHP 8.1/8.2/8.3/8.4 matrix - Runs PHP-CS-Fixer (dry-run), PHPStan, Pest, and plain-PHP tests - Existing plugin-ci-workflow.yml remains unchanged Phase 5 of code quality sweep. No functionality changes.
- Bump minimum PHP to ^8.2 (pest ^2 requires PHP 8.2+) - Pin symfony components to ^7.0 (avoid PHP 8.4-only symfony 8.x) - Set composer platform.php to 8.2 for reproducible lock - Regenerate composer.lock with 8.2-compatible versions - Update CI matrix to PHP 8.2/8.3/8.4 Fixes CI composer install failure on PHP 8.1.
- Keep require php ^8.1; pest ^2.0 requires PHP 8.2 so: - composer install uses --ignore-platform-reqs (lock resolves on any PHP) - platform pinned to 8.2 for reproducible lock - symfony pinned to ^7.0 - Pest tests skipped on PHP 8.1 in CI (pest 2.x needs 8.2+) - PHP-CS-Fixer and PHPStan run on all PHP versions (8.1-8.4) - Regenerate baseline (210 errors, unchanged) Fixes CI composer install failure while maintaining PHP 8.1 support.
- ingest Cacti user_log events with transactional deduplication - capture logout completion and authorization-denied events - add brute-force detection with atomic alert throttling - preserve deduplication state across audit-log purges - add authentication settings and upgrade handling - test Cacti 1.2.x and develop compatibility in CI - add behavioral coverage for races, retries, paging, and retention
And fix bruteforce detection settings
There was a problem hiding this comment.
Pull request overview
This pull request extends the Cacti Audit plugin to cover authentication/session-related audit events and adds supporting infrastructure (schema, settings, tests, and CI) to close identified security audit gaps while keeping behavior consistent across supported Cacti branches.
Changes:
- Adds auth/session auditing support via new hooks, poller-cycle ingestion, and a durable deduplication table (
audit_user_log_state), plus brute-force detection/throttling defaults. - Improves/extends syslog delivery and controller security assertions, and standardizes tests/code to PHP 8.1+ conventions (short array syntax, typed signatures).
- Introduces code-quality tooling (Composer, PHPStan, PHP-CS-Fixer, Pest) and expands CI to validate both
1.2.xanddevelopCacti branches.
Reviewed changes
Copilot reviewed 26 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/syslog_queue_test.php | Modernizes test fixtures (short arrays) and updates table-existence stubbing. |
| tests/syslog_functions_test.php | Refactors syslog integration tests toward PHP 8.1+ style and short arrays. |
| tests/Security/SetupStructureTest.php | Updates structure checks to assert plugin metadata is sourced from INFO. |
| tests/Security/PreparedStatementConsistencyTest.php | Minor refactor to short array syntax for target file list. |
| tests/Security/Php81SyntaxTest.php | Adds a new test enforcing PHP 8.1+ short array syntax in plugin sources. |
| tests/Security/Php74CompatibilityTest.php | Removes the PHP 7.4 compatibility enforcement tests. |
| tests/security_functions_test.php | Refactors security helper tests to short arrays and newer syntax. |
| tests/Pest.php | Converts file header comment to single-line comment. |
| tests/controller_security_test.php | Extends controller/schema/static requirements to include new auth/session auditing capabilities. |
| tests/bootstrap.php | Refactors bootstrap DB stubs to short arrays/default params. |
| tests/auth_audit_test.php | Adds extensive behavioral tests for auth/session ingestion, dedupe, and brute-force logic. |
| setup.php | Registers new hooks, adds auth default persistence, adds dedupe table creation, and cleans up settings on uninstall. |
| README.md | Documents new auth/session auditing approach (user_log polling + hooks), dedupe model, and brute-force detection. |
| phpstan/stubs/cacti.stubs.php | Adds PHPStan stubs for Cacti core APIs/constants used by the plugin. |
| phpstan-baseline.neon | Introduces a PHPStan baseline capturing known/static-analysis exceptions. |
| index.php | Switches header string to single quotes. |
| composer.json | Adds Composer configuration and dev tooling dependencies/scripts. |
| CHANGELOG.md | Documents new auth/session auditing features and related security behaviors. |
| audit.php | Refactors controller/actions and rendering with additional syslog gating and safer field handling. |
| .phpstan.neon | Adds PHPStan configuration pointing at plugin sources and Cacti stubs. |
| .php-cs-fixer.php | Adds PHP-CS-Fixer configuration aligned with Cacti’s coding standard. |
| .gitignore | Ignores vendor and tool caches (Composer/PHPStan/PHP-CS-Fixer/PHPUnit). |
| .github/workflows/plugin-ci-workflow.yml | Expands integration CI matrix to test both 1.2.x and develop, and adds auth-ingestion/uninstall checks. |
| .github/workflows/code-quality.yml | Adds a new code-quality workflow (lint, CS fixer, PHPStan, Pest/security tests). |
| .editorconfig | Introduces consistent editor formatting rules (tabs for PHP/JS/SH, etc.). |
Comment on lines
+125
to
+136
| case 'getdata': | ||
| $data = db_fetch_row_prepared('SELECT * | ||
| FROM audit_log | ||
| WHERE id = ?', | ||
| array(get_filter_request_var('id'))); | ||
| [get_filter_request_var('id')]); | ||
|
|
||
| if (!cacti_sizeof($data)) { | ||
| http_response_code(404); | ||
| print html_escape(__('Audit event not found.', 'audit')); | ||
| break; | ||
| } | ||
| if (!is_array($data)) { | ||
| http_response_code(404); | ||
| print html_escape(__('Audit event not found.', 'audit')); | ||
|
|
||
| audit_record_event('audit.event.viewed', array( | ||
| 'event_category' => 'audit', | ||
| 'target_type' => 'audit_event', | ||
| 'target_id' => $data['event_uuid'] != '' ? $data['event_uuid'] : $data['id'], | ||
| 'details' => array('record_id' => $data['id']) | ||
| )); | ||
|
|
||
| $output = audit_render_event_details($data); | ||
| echo $output; | ||
|
|
||
| break; | ||
| default: | ||
| top_header(); | ||
| audit_log(); | ||
| bottom_footer(); | ||
| break; | ||
| } |
| $info = parse_ini_file($config['base_path'] . '/plugins/audit/INFO', true); | ||
| return $info['info']; | ||
|
|
||
| return is_array($info) ? $info['info'] : []; |
somethingwithproof
suggested changes
Aug 17, 2026
somethingwithproof
left a comment
somethingwithproof
left a comment
Member
There was a problem hiding this comment.
This is not reviewable/mergeable as one unit yet: it is conflicting and combines auth auditing with repository-wide formatting, native type changes, a 5,488-line plugin-local composer.lock, PHPStan baseline/tooling, and CI. Per the current plugin direction, tests should use Cacti's Composer/Pest toolchain instead of a second plugin dependency graph; --ignore-platform-reqs plus skipping Pest on PHP 8.1 does not demonstrate declared PHP 8.1 support. Please rebase and split the mechanical/toolchain work from the auth feature. One functional blocker also needs resolution: audit_detect_brute_force() counts failures globally across every username and IP, then emits a critical event without identifying a source, so normal aggregate failures on a large installation can be classified as one brute-force actor. Group/threshold by a defensible identity (and record it), or explicitly model and name this as a global anomaly, with concurrency and multi-source tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds authentication and session event auditing to the Cacti audit plugin, closing the Group A gaps identified in the security audit. Captures login outcomes (success, failure, token, password-change/2FA), authorization-denied events, brute-force detection, and completes the logout audit pair — all while remaining compatible with both Cacti 1.2.x and the develop branch.