fix(configdb): opt-in config-audit retention purge + after-state size cap (review 2026-10-21 08/G1)

2026-10-22 08:32:40 -04:00
parent 394234b450
commit 036f341a9d
7 changed files with 292 additions and 2 deletions
@@ -127,7 +127,7 @@ The configuration database stores all central system data, organized by domain a
 - **Bundle Unlock Attempts**: One row per permitted bundle-unlock passphrase attempt (`ClientKey`, `AttemptedAtUtc`), forming the fleet-wide sliding window behind Transport's `MaxUnlockAttemptsPerIpPerHour` throttle — see `Component-Transport.md` (register row 52). Operational history, not audit: non-partitioned, no DB-role restriction, and rows are pruned opportunistically by the application once they age past twice the window. **Denied** attempts are deliberately not recorded (a denial burns no budget), and no bundle identity is stored — the durable record of a failed unlock is the `BundleImportUnlockFailed` audit row. Central-only: written only by the Transport component, which the host registers for central roles.

 ### Audit Logging
- **Audit Log Entries**: Append-only audit trail (timestamp, user, action, entity type, entity ID, entity name, state as JSON). Stores only the after-state — change history is reconstructed by comparing consecutive entries. Entries are never modified or deleted. No retention policy — retained indefinitely. Indexed on timestamp, user, entity type, entity ID, and action for efficient filtering.
+- **Audit Log Entries**: Append-only audit trail (timestamp, user, action, entity type, entity ID, entity name, state as JSON). Stores only the after-state — change history is reconstructed by comparing consecutive entries. Entries are never modified. Retained indefinitely by default; an optional `ScadaBridge:ConfigAudit:RetentionDays` (default `0` = off, validated `>= 0`) enables a daily, time-sliced batched purge (`ConfigAuditPurgeService`: `PurgeBatchSize` 5000 rows/batch, at most `MaxBatchesPerTick` 200 batches per daily tick, oldest first) of entries older than the window. `AfterStateJson` is capped at 256 K characters — a larger serialized state is replaced by a small valid-JSON truncation marker (`AuditTruncated`, `OriginalLength`, `StateType`, preview) so one row cannot grow without bound. The viewer query orders by `(Timestamp DESC, Id DESC)` so pages are deterministic when many rows share a timestamp (bundle imports). Indexed on timestamp, user, entity type, entity ID, and action for efficient filtering.

 ---

@@ -0,0 +1,47 @@
+using Microsoft.Extensions.Options;
+
+namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Maintenance;
+
+/// <summary>
+/// Options for the configuration audit trail (<c>AuditLogEntries</c>), bound from
+/// <c>ScadaBridge:ConfigAudit</c>.
+/// </summary>
+public class ConfigAuditOptions
+{
+    /// <summary>The configuration section name.</summary>
+    public const string SectionName = "ScadaBridge:ConfigAudit";
+
+    /// <summary>
+    /// Entries older than this many days are purged daily. <c>0</c> (the default) keeps config
+    /// audit entries indefinitely — retention is strictly opt-in.
+    /// </summary>
+    public int RetentionDays { get; set; }
+
+    /// <summary>Maximum rows deleted per batch (keeps each DELETE short and the log small).</summary>
+    public int PurgeBatchSize { get; set; } = 5000;
+
+    /// <summary>Upper bound on batches per purge tick; the remainder is picked up on the next tick.</summary>
+    public int MaxBatchesPerTick { get; set; } = 200;
+
+    /// <summary>How often the purge runs.</summary>
+    public TimeSpan PurgeInterval { get; set; } = TimeSpan.FromDays(1);
+}
+
+/// <summary>Validates <see cref="ConfigAuditOptions"/>.</summary>
+public sealed class ConfigAuditOptionsValidator : IValidateOptions<ConfigAuditOptions>
+{
+    /// <inheritdoc />
+    public ValidateOptionsResult Validate(string? name, ConfigAuditOptions options)
+    {
+        var failures = new List<string>();
+        if (options.RetentionDays < 0)
+            failures.Add($"{ConfigAuditOptions.SectionName}:RetentionDays must be >= 0 (0 = retain indefinitely).");
+        if (options.PurgeBatchSize <= 0)
+            failures.Add($"{ConfigAuditOptions.SectionName}:PurgeBatchSize must be > 0.");
+        if (options.MaxBatchesPerTick <= 0)
+            failures.Add($"{ConfigAuditOptions.SectionName}:MaxBatchesPerTick must be > 0.");
+        if (options.PurgeInterval <= TimeSpan.Zero)
+            failures.Add($"{ConfigAuditOptions.SectionName}:PurgeInterval must be > 0.");
+        return failures.Count == 0 ? ValidateOptionsResult.Success : ValidateOptionsResult.Fail(failures);
+    }
+}
@@ -0,0 +1,122 @@
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
+
+namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Maintenance;
+
+/// <summary>
+/// Central hosted service that purges <c>AuditLogEntries</c> (the configuration audit trail)
+/// older than <see cref="ConfigAuditOptions.RetentionDays"/>. A no-op while
+/// <c>RetentionDays</c> is 0 (the default — retain indefinitely).
+/// </summary>
+/// <remarks>
+/// Deletes in bounded batches (<see cref="ConfigAuditOptions.PurgeBatchSize"/>, at most
+/// <see cref="ConfigAuditOptions.MaxBatchesPerTick"/> per tick) so a first purge over a large
+/// backlog is time-sliced across ticks instead of one long-running DELETE. Both central nodes may
+/// run it; deleting an already-deleted row is harmless, so no singleton is needed. A failed tick
+/// is logged and retried on the next.
+/// </remarks>
+public sealed class ConfigAuditPurgeService : BackgroundService
+{
+    private readonly IServiceScopeFactory _scopeFactory;
+    private readonly IOptions<ConfigAuditOptions> _options;
+    private readonly TimeProvider _time;
+    private readonly ILogger<ConfigAuditPurgeService> _logger;
+
+    /// <summary>Initializes the purge service.</summary>
+    /// <param name="scopeFactory">Creates a DI scope per tick (the DbContext is scoped).</param>
+    /// <param name="options">Retention options.</param>
+    /// <param name="time">Clock.</param>
+    /// <param name="logger">Logger.</param>
+    public ConfigAuditPurgeService(
+        IServiceScopeFactory scopeFactory,
+        IOptions<ConfigAuditOptions> options,
+        TimeProvider time,
+        ILogger<ConfigAuditPurgeService> logger)
+    {
+        _scopeFactory = scopeFactory;
+        _options = options;
+        _time = time;
+        _logger = logger;
+    }
+
+    /// <inheritdoc />
+    protected override async Task ExecuteAsync(CancellationToken stoppingToken)
+    {
+        var opts = _options.Value;
+        if (opts.RetentionDays <= 0)
+        {
+            _logger.LogInformation("Config audit retention is off (RetentionDays=0); entries are kept indefinitely.");
+            return;
+        }
+
+        using var timer = new PeriodicTimer(opts.PurgeInterval, _time);
+        do
+        {
+            try
+            {
+                await PurgeOnceAsync(stoppingToken);
+            }
+            catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
+            {
+                return;
+            }
+            catch (Exception ex)
+            {
+                _logger.LogError(ex, "Config audit purge tick failed; will retry next interval.");
+            }
+        }
+        while (await WaitNextAsync(timer, stoppingToken));
+    }
+
+    private static async Task<bool> WaitNextAsync(PeriodicTimer timer, CancellationToken ct)
+    {
+        try { return await timer.WaitForNextTickAsync(ct); }
+        catch (OperationCanceledException) { return false; }
+    }
+
+    /// <summary>
+    /// Runs one purge pass. Returns the number of rows deleted. Exposed for tests.
+    /// </summary>
+    /// <param name="cancellationToken">Cancellation token.</param>
+    /// <returns>Total rows deleted this pass.</returns>
+    public async Task<int> PurgeOnceAsync(CancellationToken cancellationToken = default)
+    {
+        var opts = _options.Value;
+        if (opts.RetentionDays <= 0)
+            return 0;
+
+        var cutoff = _time.GetUtcNow().AddDays(-opts.RetentionDays);
+        await using var scope = _scopeFactory.CreateAsyncScope();
+        var db = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
+
+        var total = 0;
+        for (var batch = 0; batch < opts.MaxBatchesPerTick; batch++)
+        {
+            // Oldest-first, bounded: select the batch's keys then delete by key so the DELETE
+            // stays a short, index-ordered operation on SQL Server.
+            var ids = await db.AuditLogEntries
+                .AsNoTracking()
+                .Where(a => a.Timestamp < cutoff)
+                .OrderBy(a => a.Timestamp)
+                .Select(a => a.Id)
+                .Take(opts.PurgeBatchSize)
+                .ToListAsync(cancellationToken);
+            if (ids.Count == 0)
+                break;
+
+            total += await db.AuditLogEntries
+                .Where(a => ids.Contains(a.Id))
+                .ExecuteDeleteAsync(cancellationToken);
+
+            if (ids.Count < opts.PurgeBatchSize)
+                break;
+        }
+
+        if (total > 0)
+            _logger.LogInformation("Config audit purge removed {Count} entries older than {Cutoff:o}.", total, cutoff);
+        return total;
+    }
+}
@@ -110,6 +110,14 @@ public static class ServiceCollectionExtensions
        // fresh scope on each tick (mirrors AuditLogPurgeActor / AuditLogIngestActor).
        services.AddScoped<IPartitionMaintenance, AuditLogPartitionMaintenance>();

+        // Opt-in config-audit retention (ScadaBridge:ConfigAudit:RetentionDays, default 0 = off).
+        // Options binding to configuration is done by the host; without it the defaults apply
+        // and the purge service is a no-op.
+        services.TryAddEnumerable(ServiceDescriptor.Singleton<
+            Microsoft.Extensions.Options.IValidateOptions<ConfigAuditOptions>, ConfigAuditOptionsValidator>());
+        services.TryAddSingleton(TimeProvider.System);
+        services.AddHostedService<ConfigAuditPurgeService>();
+
        services.AddDataProtection()
            .PersistKeysToDbContext<ScadaBridgeDbContext>();

@@ -23,6 +23,15 @@ public class AuditService : IAuditService
        MaxDepth = 32
    };

+    /// <summary>
+    /// Upper bound (characters) on a serialized <c>AfterStateJson</c>. A larger payload (e.g. a
+    /// whole template graph with every script body) is replaced by a small truncation marker so
+    /// one audit row can never grow without bound. Valid JSON is preserved either way.
+    /// </summary>
+    public const int MaxAfterStateChars = 256 * 1024;
+
+    private const int TruncatedPreviewChars = 4 * 1024;
+
    /// <summary>
    /// Initializes the audit service with the EF Core context and correlation context.
    /// </summary>
@@ -72,7 +81,17 @@ public class AuditService : IAuditService
    {
        try
        {
-            return JsonSerializer.Serialize(afterState, AuditSerializerOptions);
+            var json = JsonSerializer.Serialize(afterState, AuditSerializerOptions);
+            if (json.Length <= MaxAfterStateChars)
+                return json;
+
+            return JsonSerializer.Serialize(new
+            {
+                AuditTruncated = true,
+                OriginalLength = json.Length,
+                StateType = afterState.GetType().FullName,
+                Preview = json[..TruncatedPreviewChars]
+            });
        }
        catch (Exception ex) when (ex is JsonException or NotSupportedException)
        {
@@ -212,4 +212,20 @@ public class AuditServiceTests : IDisposable
        Assert.Contains("node", audit.AfterStateJson);
        Assert.Single(await _context.Templates.Where(t => t.Name == "CyclicAuditTemplate").ToListAsync());
    }
+
+    /// <summary>An oversized after-state is replaced by a small valid-JSON truncation marker (review 08/G1).</summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task LogAsync_OversizedAfterState_IsTruncatedToMarker()
+    {
+        await _auditService.LogAsync("admin", "Update", "Template", "1", "Big",
+            new { Code = new string('x', AuditService.MaxAfterStateChars + 10) });
+        await _context.SaveChangesAsync();
+
+        var json = _context.AuditLogEntries.Single().AfterStateJson!;
+        Assert.True(json.Length < 16 * 1024);
+        using var doc = JsonDocument.Parse(json);
+        Assert.True(doc.RootElement.GetProperty("AuditTruncated").GetBoolean());
+        Assert.True(doc.RootElement.GetProperty("OriginalLength").GetInt32() > AuditService.MaxAfterStateChars);
+    }
 }
@@ -0,0 +1,78 @@
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging.Abstractions;
+using Microsoft.Extensions.Options;
+using ZB.MOM.WW.ScadaBridge.Commons.Entities.Audit;
+using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Maintenance;
+
+namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests.Maintenance;
+
+/// <summary>Tests for the opt-in config-audit retention purge (review 2026-10-21 08/G1).</summary>
+public class ConfigAuditPurgeServiceTests : IDisposable
+{
+    private readonly ScadaBridgeDbContext _context = SqliteTestHelper.CreateInMemoryContext();
+
+    public void Dispose()
+    {
+        _context.Database.CloseConnection();
+        _context.Dispose();
+    }
+
+    private ConfigAuditPurgeService Create(ConfigAuditOptions opts)
+    {
+        var services = new ServiceCollection();
+        services.AddSingleton(_context);
+        return new ConfigAuditPurgeService(
+            services.BuildServiceProvider().GetRequiredService<IServiceScopeFactory>(),
+            Options.Create(opts),
+            TimeProvider.System,
+            NullLogger<ConfigAuditPurgeService>.Instance);
+    }
+
+    private async Task SeedAsync(int oldCount, int recentCount)
+    {
+        var now = DateTimeOffset.UtcNow;
+        for (var i = 0; i < oldCount; i++)
+            _context.AuditLogEntries.Add(new AuditLogEntry("u", "Create", "Template", $"o{i}", $"Old{i}") { Timestamp = now.AddDays(-100).AddMinutes(i) });
+        for (var i = 0; i < recentCount; i++)
+            _context.AuditLogEntries.Add(new AuditLogEntry("u", "Create", "Template", $"r{i}", $"New{i}") { Timestamp = now.AddDays(-1) });
+        await _context.SaveChangesAsync();
+    }
+
+    [Fact]
+    public async Task RetentionZero_DeletesNothing()
+    {
+        await SeedAsync(5, 2);
+        var deleted = await Create(new ConfigAuditOptions { RetentionDays = 0 }).PurgeOnceAsync();
+        Assert.Equal(0, deleted);
+        Assert.Equal(7, _context.AuditLogEntries.Count());
+    }
+
+    [Fact]
+    public async Task Retention_DeletesOnlyOlderThanWindow_AcrossBatches()
+    {
+        await SeedAsync(25, 3);
+        var deleted = await Create(new ConfigAuditOptions { RetentionDays = 30, PurgeBatchSize = 10 }).PurgeOnceAsync();
+        Assert.Equal(25, deleted);
+        Assert.Equal(3, _context.AuditLogEntries.Count());
+    }
+
+    [Fact]
+    public async Task Retention_MaxBatchesPerTick_TimeSlicesTheBacklog()
+    {
+        await SeedAsync(25, 0);
+        var svc = Create(new ConfigAuditOptions { RetentionDays = 30, PurgeBatchSize = 10, MaxBatchesPerTick = 2 });
+        Assert.Equal(20, await svc.PurgeOnceAsync());
+        Assert.Equal(5, await svc.PurgeOnceAsync());
+    }
+
+    [Fact]
+    public void Validator_RejectsNegativeRetention_AcceptsZero()
+    {
+        var v = new ConfigAuditOptionsValidator();
+        Assert.True(v.Validate(null, new ConfigAuditOptions { RetentionDays = 0 }).Succeeded);
+        Assert.True(v.Validate(null, new ConfigAuditOptions { RetentionDays = 90 }).Succeeded);
+        Assert.False(v.Validate(null, new ConfigAuditOptions { RetentionDays = -1 }).Succeeded);
+        Assert.False(v.Validate(null, new ConfigAuditOptions { PurgeBatchSize = 0 }).Succeeded);
+    }
+}