Add v2 major release audit infrastructure - #9
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the v2 infrastructure for the Laravel Audit Log package, incorporating significant features such as native batch insert support, multi-tenancy context, append-only security mode, and relationship auditing. It also adds a comprehensive set of Artisan commands for configuration validation, migrations, and health checks. Feedback identifies critical issues regarding database connection consistency in management commands, potential sensitive data exposure in the snapshot service, and performance bottlenecks related to schema filtering during batch operations and row counting for snapshots.
| } | ||
|
|
||
| $table = $tables->resolve($entityClass); | ||
| $exists = Schema::connection(config('database.default'))->hasTable($table); |
There was a problem hiding this comment.
The health check uses the default database connection to verify the existence of audit tables. However, the audit logger can be configured to use a different connection via AUDIT_MYSQL_CONNECTION or AUDIT_PGSQL_CONNECTION. This check should use the connection defined in the audit logger configuration to be accurate.
| if (! Schema::hasTable($tableName)) { | ||
| $this->warn("Missing audit table [{$tableName}]."); | ||
| continue; | ||
| } | ||
|
|
||
| $missing = array_values(array_filter([ | ||
| 'audit_hash', | ||
| 'previous_hash', | ||
| 'tenant_type', | ||
| 'tenant_id', | ||
| 'changes', | ||
| ], fn (string $column): bool => ! Schema::hasColumn($tableName, $column))); |
There was a problem hiding this comment.
Similar to the doctor command, this upgrade check defaults to the primary database connection. It should respect the connection configured for each audit table to correctly identify missing columns in multi-database setups.
| entityId: $model->getKey(), | ||
| action: 'snapshot', | ||
| oldValues: null, | ||
| newValues: $model->getAttributes(), |
There was a problem hiding this comment.
Using $model->getAttributes() directly will include all model fields in the snapshot, including sensitive data (e.g., passwords, tokens) that should be excluded or redacted. This bypasses the standard audit log field filtering logic. Please ensure that the snapshot only includes attributes that are configured to be audited.
| $this->store($log); | ||
| $this->validateEntityType($log->getEntityType()); | ||
| $this->ensureStorageExists($log->getEntityType()); | ||
| $grouped[$this->getTableName($log->getEntityType())][] = $this->payloadForInsert($log); |
There was a problem hiding this comment.
The payloadForInsert method (which calls filterPayloadForTable) is executed for every row in a batch. filterPayloadForTable performs multiple Schema::hasColumn checks per row. While Laravel caches these results, iterating over the column list for every field in every row of a large batch (e.g., 500 rows) adds significant overhead. It is recommended to fetch the column list once per table before starting the batch loop.
| $this->store($log); | ||
| $this->validateEntityType($log->getEntityType()); | ||
| $this->ensureStorageExists($log->getEntityType()); | ||
| $grouped[$this->getTableName($log->getEntityType())][] = $this->payloadForInsert($log); |
There was a problem hiding this comment.
The payloadForInsert method is executed for every row in a batch, triggering repeated schema filtering. To improve performance during batch inserts, consider fetching the column listing once per table and reusing it for all rows in the batch instead of relying on individual hasColumn checks inside the loop.
| $count = \iamfarhad\LaravelAuditLog\Models\EloquentAuditLog::forEntity($model::class) | ||
| ->newQuery() | ||
| ->where('entity_id', (string) $model->getKey()) | ||
| ->count(); |
There was a problem hiding this comment.
Performing a count() on the audit table for every model update can become a major performance bottleneck as the audit log grows to millions of rows. Consider using a more efficient trigger for periodic snapshots, such as a dedicated counter or checking the ID of the last snapshot row.
1 participant
Summary
This PR builds the v2 major-release infrastructure on top of
feature/audit-advanced-apis/ PR #8.Included:
audit:make-migrationaudit:migrateAuditMigrationGeneratorAUDIT_BATCH_ENABLED=true, queueing is disabled, and hash-chain mode is offTenantResolverInterfaceTenantResolvertenant_type/tenant_idpayload columnsforTenant()audit scopeAUDIT_APPEND_ONLY=trueRelationshipAuditor::attached()RelationshipAuditor::detached()RelationshipAuditor::synced()audit:config-checkaudit:doctoraudit:statsaudit:timelineaudit:diffaudit:verifyaudit:partitionaudit:upgradechangescolumnAuditLogResourceAuditTimelineResourceAuditDiffResourceAuditCreatingAuditCreatedAuditVerificationFaileddocs/v2-major-release.mdExcluded by request:
Documentation updates
README.mdinto a full feature guide covering v2 installation, config, production table management, search/analytics/timeline/diff, restore safety, integrity/security, redaction, tenancy, batch inserts, relationship auditing, snapshots, API resources, commands, retention, events, upgrades, comparison, troubleshooting, and docs links.docs/v2-major-release.mdinto a deeper production guide with migration examples, PostgreSQL/MySQL upgrade examples, batch limitations, tenancy resolver examples, hash-chain behavior, relationship auditing, restore safety, API resources, operational commands, partition guidance, events, presets, production checklist, troubleshooting, and upgrade order.Hardening / review fixes
audit:migratenow explicitly creates storage instead of relying onensureStorageExists(), so it works even when runtime auto-migration is disabled.AuditCreatedfrom the queued job after storage succeeds instead of dispatching too early.audit:doctorandaudit:upgradeuse the configured audit connection rather than the app default database connection.AuditAuthorization, supports fillable filtering, and can save without creating a new audit row whenAUDIT_RESTORE_AUDIT=false.AuditVerificationFailedwhen verification fails.Notes
This is intentionally a stacked PR on top of PR #8 so the advanced audit APIs can stay reviewable and mergeable independently.
The v2 features are mostly opt-in through config flags. Existing tables are protected by payload filtering, so optional v2 columns are only written when present.
The partition command is guidance-only and does not run risky database partition DDL automatically.
Relationship auditing is explicit via
RelationshipAuditor; it does not monkey-patch Laravel relation methods.Verification
This PR is still draft because GitHub Actions do not run for this stacked PR target: the repository workflows are configured for pull requests into
main/master, while this PR targetsfeature/audit-advanced-apis.Recommended after PR #8 is merged or this PR is retargeted to
main:composer install composer test composer pint:testThen mark ready only after the full matrix is green.