dc2df628e3
The remediation reviews approved every task but left a tail of small notes. This lands the gateway-side half of them. Hardening (behavior changes, all narrow): - BuildFilteredWriteBulkCommand's unreachable default case failed OPEN: a fifth bulk-write kind added upstream without a filter case here would have shipped the DENIED entries to the worker while reporting them denied to the caller. It now throws UnreachableException. - SqliteCanonicalAuditStore.ListRecentAsync no longer throws on a row it cannot date. The retention sweep deliberately preserves such rows (SQLite's datetime() yields NULL, so the DELETE never matches), which guaranteed the dashboard's recent-audit view would meet one eventually and lose the whole page to it. The row is now reported at DateTimeOffset.MinValue with every other column intact, behind an optional logger. - The audit drain loop's finally now completes the channel writer alongside detaching the drain, so a producer that raced past the attached check takes the write-through branch instead of stranding its event in a buffer nobody reads until shutdown. TryComplete is idempotent, so StopAsync is unaffected. Tests: - MapCommandReply ownership (Assert.Same on the inner reply), mirroring the existing MapEvent ownership test. - Redactor key-id length boundary at exactly 64 and 65 characters, pinning which way it fails. Nothing validates key-id length at creation, so docs/Diagnostics.md's "which no issued key id does" is now stated as the heuristic it is. - ApiKeyFailureLimiter.Reset with a PartitionResolution whose partition was evicted between the Check and the Reset: inert, and clears nobody else's block. - Constraint-cache concurrency stress: the cap is enforced by the inserting thread, so overshoot must be transient and proportional to the in-flight inserters, and the cache must settle at or under the cap. - ListRecentAsync against a raw-SQL undateable row. Comment/doc accuracy: - EventsHubViewerRegistry.ReleaseConnection records that it relies on SignalR's default sequential per-connection dispatch (MaximumParallelInvocationsPerClient = 1). - A PERF(followup) note on Invoke's double session resolve and why removing it needs a SessionManager overload. - SessionEventDistributor: the volatile-field comment named the pump as the lock-free reader, but the pump's single capture point is inside _replayLock; the genuinely lock-free reader is SubscriberCount. OnSubscriberOverflow's "cannot be observed here" now excepts the DisposeAsync abandon path. The churn test names its ConcurrentDictionary bucket-order assumption and that a violation surfaces as a read timeout, not a silent pass. - The two "restores the sequential drain's behavior" claims (SessionManager, docs/Sessions.md) were wrong: the sequential drain leaked too, because KillWorkerAsync's entry ThrowIfCancellationRequested aborted the whole loop on the first session for zero kills. Reworded to "fixes a leak the sequential drain also had", with the sweep-bound/shutdown-unbound ParallelOptions asymmetry explained. - ISessionManager.ShutdownAsync's token doc: it degrades the drain to a kill sweep rather than cancelling it, with the bounded overrun stated. SessionShutdownHostedService.StopAsync records that its cancellation-logging branch is now unreachable.
140 lines
6.8 KiB
C#
140 lines
6.8 KiB
C#
using System.Threading.Channels;
|
|
using ZB.MOM.WW.Audit;
|
|
|
|
namespace ZB.MOM.WW.MxGateway.Server.Security.Audit;
|
|
|
|
/// <summary>
|
|
/// Bounded, non-blocking <see cref="IAuditWriter"/>: <see cref="WriteAsync"/> enqueues onto a
|
|
/// fixed-capacity channel and returns, leaving <see cref="AuditDrainService"/> to batch the
|
|
/// events onto the durable <see cref="IAuditEventSink"/>.
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// The canonical <see cref="IAuditWriter"/> contract is already best-effort — a failed audit
|
|
/// write is swallowed rather than propagated. The channel makes the <em>bound</em> on that
|
|
/// promise explicit: audit can cost the calling RPC at most one enqueue, never a SQLite
|
|
/// round-trip, and at most <see cref="ChannelCapacity"/> events of memory. This matters on the
|
|
/// constraint-denial path, where a partially denied bulk RPC previously awaited one insert per
|
|
/// denied tag, serially, against the same database file the authentication hot path reads.
|
|
/// <para>
|
|
/// When the channel is full the newest write is dropped (<see cref="BoundedChannelFullMode.DropWrite"/>)
|
|
/// and counted in <see cref="DroppedCount"/>. Dropping is the deliberate choice over blocking:
|
|
/// a stalled audit database must degrade audit completeness, not stall the gateway. Drops are
|
|
/// logged, and <see cref="AuditDrainService"/> reports the running total on its sweep.
|
|
/// </para>
|
|
/// <para>
|
|
/// Until a drain attaches (<see cref="AttachDrain"/>), and again after it detaches, writes go
|
|
/// straight through to <see cref="CanonicalAuditWriter"/>. Enqueueing into a channel nobody will
|
|
/// ever read would silently discard audit in the processes that have no hosted services — the
|
|
/// <c>apikey</c> admin CLI and the DI-only tests — so those keep the original synchronous path.
|
|
/// The same fallback covers a completed channel, so no combination of attach/detach can leave
|
|
/// producers writing into a buffer that will never be read.
|
|
/// </para>
|
|
/// </remarks>
|
|
public sealed class ChannelAuditWriter : IAuditWriter
|
|
{
|
|
/// <summary>
|
|
/// Maximum number of audit events buffered before writes start being dropped. Sized to
|
|
/// absorb a fully denied bulk RPC (the gateway's bulk request cap) plus headroom, so a
|
|
/// realistic burst is buffered rather than lost.
|
|
/// </summary>
|
|
public const int ChannelCapacity = 4096;
|
|
|
|
private readonly CanonicalAuditWriter _directWriter;
|
|
private readonly ILogger<ChannelAuditWriter> _logger;
|
|
private readonly Channel<AuditEvent> _channel;
|
|
|
|
private long _droppedCount;
|
|
private int _drainAttached;
|
|
private int _dropLogged;
|
|
|
|
/// <summary>Creates the writer and its bounded buffer.</summary>
|
|
/// <param name="directWriter">The synchronous writer used when no drain is attached.</param>
|
|
/// <param name="logger">Logger for drop diagnostics.</param>
|
|
public ChannelAuditWriter(CanonicalAuditWriter directWriter, ILogger<ChannelAuditWriter> logger)
|
|
{
|
|
_directWriter = directWriter;
|
|
_logger = logger;
|
|
|
|
// DropWrite discards the incoming item and still reports success to the producer, so the
|
|
// itemDropped callback is the only place a drop can be observed and counted.
|
|
_channel = Channel.CreateBounded<AuditEvent>(
|
|
new BoundedChannelOptions(ChannelCapacity)
|
|
{
|
|
FullMode = BoundedChannelFullMode.DropWrite,
|
|
SingleReader = true,
|
|
SingleWriter = false,
|
|
},
|
|
itemDropped: RecordDrop);
|
|
}
|
|
|
|
/// <summary>Gets the number of audit events dropped because the channel was full.</summary>
|
|
public long DroppedCount => Interlocked.Read(ref _droppedCount);
|
|
|
|
/// <summary>Gets the reader the drain service consumes buffered events from.</summary>
|
|
public ChannelReader<AuditEvent> Reader => _channel.Reader;
|
|
|
|
/// <summary>
|
|
/// Marks a drain as running, so subsequent writes enqueue instead of writing through.
|
|
/// Called by <see cref="AuditDrainService"/> once its one-time bootstrap has completed.
|
|
/// </summary>
|
|
public void AttachDrain() => Volatile.Write(ref _drainAttached, 1);
|
|
|
|
/// <summary>
|
|
/// Marks the drain as no longer running, so writes revert to the synchronous path. Called at
|
|
/// shutdown, and whenever the drain loop dies, so late audit is still persisted rather than
|
|
/// buffered into a channel with no reader.
|
|
/// </summary>
|
|
public void DetachDrain() => Volatile.Write(ref _drainAttached, 0);
|
|
|
|
/// <summary>
|
|
/// Enqueues a canonical audit event for the drain to persist. Never blocks and never throws.
|
|
/// It also never touches the store on the caller's thread while a drain is attached — with one
|
|
/// exception: once the channel has been completed (shutdown, or a drain loop that died), the
|
|
/// enqueue fails and this falls through to the synchronous write, which is what keeps the event
|
|
/// rather than stranding it in a buffer nobody reads.
|
|
/// </summary>
|
|
/// <param name="auditEvent">The canonical audit event to persist.</param>
|
|
/// <param name="cancellationToken">Token honoured only by the direct write-through path.</param>
|
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
|
public Task WriteAsync(AuditEvent auditEvent, CancellationToken cancellationToken = default)
|
|
{
|
|
ArgumentNullException.ThrowIfNull(auditEvent);
|
|
|
|
if (Volatile.Read(ref _drainAttached) == 0)
|
|
{
|
|
return _directWriter.WriteAsync(auditEvent, cancellationToken);
|
|
}
|
|
|
|
// Under DropWrite a full channel still reports SUCCESS — the discard surfaces through the
|
|
// itemDropped callback. So a false here does not mean "full", it means the channel has
|
|
// been completed and no drain will ever read it again (shutdown, or a re-attach onto a
|
|
// dead channel). Writing through is the only outcome that keeps the event.
|
|
if (!_channel.Writer.TryWrite(auditEvent))
|
|
{
|
|
return _directWriter.WriteAsync(auditEvent, cancellationToken);
|
|
}
|
|
|
|
return Task.CompletedTask;
|
|
}
|
|
|
|
/// <summary>Signals that no further events will be enqueued, so the drain loop can finish.</summary>
|
|
public void CompleteWriting() => _channel.Writer.TryComplete();
|
|
|
|
private void RecordDrop(AuditEvent auditEvent)
|
|
{
|
|
Interlocked.Increment(ref _droppedCount);
|
|
|
|
// Log the first drop only; the running total is reported on the drain's periodic sweep,
|
|
// so a sustained overload cannot turn audit pressure into a log flood.
|
|
if (Interlocked.Exchange(ref _dropLogged, 1) == 0)
|
|
{
|
|
_logger.LogWarning(
|
|
"Audit channel is full ({Capacity} events); dropping audit event {EventId} (action {Action}). "
|
|
+ "Audit is best-effort and bounded; further drops are reported in aggregate.",
|
|
ChannelCapacity,
|
|
auditEvent.EventId,
|
|
auditEvent.Action);
|
|
}
|
|
}
|
|
}
|