-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Bug: AuditInterceptor feedback loop when audit entities implement IAuditable or interceptor captures all changes #1230
Copy link
Copy link
Open
Description
Description
BindDbContext<TContext> registers all ISaveChangesInterceptor instances (including AuditInterceptor) on every DbContext via:
options.AddInterceptors(sp.GetServices<ISaveChangesInterceptor>());
This means AuditInterceptor also fires when AuditPublishedEventHandler calls IdentityDbContext.SaveChangesAsync() to persist AuditTrail records. Currently this
doesn't cause a visible problem because AuditInterceptor filters on IAuditable:
foreach (var entry in eventData.Context.ChangeTracker.Entries<IAuditable>()
.Where(x => x.State is EntityState.Added or EntityState.Deleted or EntityState.Modified)
.ToList())
And AuditTrail does not implement IAuditable, so the interceptor finds no matching entries and exits early.
The Problem
This is a latent feedback loop. If anyone:
1. Makes AuditTrail implement IAuditable (to track when audit records are created/modified), or
2. Broadens the interceptor to capture all entity changes (e.g., ChangeTracker.Entries() without the <IAuditable> filter)
...the interceptor will detect the AuditTrail inserts, publish new AuditPublishedEvents, which triggers AuditPublishedEventHandler, which calls SaveChangesAsync()
again, creating an infinite loop.
Reproduction
When broadening the interceptor to capture all entity changes (replacing Entries<IAuditable>() with Entries()), the seeding process can generate audit records that recursively trigger additional audit records, resulting in exponential growth:
[10:37:35 INF] Wrote 84 audit records for tenant root.
[10:37:36 INF] Wrote 84 audit records for tenant root.
[10:37:37 INF] Wrote 84 audit records for tenant root.
...repeating indefinitely...
Location
- src/api/framework/Infrastructure/Persistence/Extensions.cs — BindDbContext<TContext> (line with AddInterceptors)
- src/api/framework/Infrastructure/Persistence/Interceptors/AuditInterceptor.cs — no guard against audit context
- src/api/framework/Infrastructure/Identity/Audit/AuditPublishedEventHandler.cs — saves to IdentityDbContext which has the interceptor
Proposed Fix
Add an early return in AuditInterceptor when the context is the one storing audit trails:
public override async ValueTask<InterceptionResult<int>> SavingChangesAsync(
DbContextEventData eventData, InterceptionResult<int> result,
CancellationToken cancellationToken = default)
{
if (eventData.Context is IdentityDbContext ctx && ctx.ChangeTracker.Entries<AuditTrail>().Any())
return await base.SavingChangesAsync(eventData, result, cancellationToken);
UpdateEntities(eventData.Context);
await PublishAuditTrailsAsync(eventData);
return await base.SavingChangesAsync(eventData, result, cancellationToken);
}
Alternatively, a simpler approach: exclude AuditTrail from the Entries<IAuditable>() filter explicitly, or document that AuditTrail must never implement IAuditable.
Impact Analysis
- Current impact: None visible (accidentally mitigated by IAuditable filter)
- Latent risk: Infinite loop, database bloat, and degraded startup performance if the interceptor is broadened or AuditTrail gains IAuditable
- Related: This is architecturally similar to the missing base.OnModelCreating() issue (#1226) — a working-by-coincidence pattern that breaks under reasonable
modificationsReactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels