From 181d6d155c4b147c3f75bea46132991bbcf239bc Mon Sep 17 00:00:00 2001 From: "Marcelo M. Maciel" <4993482+marcelo-maciel@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:51:32 -0300 Subject: [PATCH] fix(auditing): cap and page the security/exception audit lists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both audit list handlers did ToListAsync with no Skip/Take and no default window, and the validators never required a time range, so an unpaged call materialized a tenant's entire audit history into memory and over the wire — a latent OOM on an active tenant. Add optional Skip/Take to both queries and clamp server-side (page size 1..200, default 50), mirroring SessionService.GetTenantSessionsAsync. An unpaged call now returns at most the default page instead of the full table. --- .../GetExceptionAuditsQuery.cs | 4 + .../GetSecurityAuditsQuery.cs | 4 + .../GetExceptionAuditsQueryHandler.cs | 13 ++- .../GetSecurityAuditsQueryHandler.cs | 9 ++ .../Auditing/SecurityAuditListBoundsTests.cs | 87 +++++++++++++++++++ 5 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 src/Tests/Integration.Tests/Tests/Auditing/SecurityAuditListBoundsTests.cs diff --git a/src/Modules/Auditing/Modules.Auditing.Contracts/v1/GetExceptionAudits/GetExceptionAuditsQuery.cs b/src/Modules/Auditing/Modules.Auditing.Contracts/v1/GetExceptionAudits/GetExceptionAuditsQuery.cs index 5b15957c06..dbf8660bb3 100644 --- a/src/Modules/Auditing/Modules.Auditing.Contracts/v1/GetExceptionAudits/GetExceptionAuditsQuery.cs +++ b/src/Modules/Auditing/Modules.Auditing.Contracts/v1/GetExceptionAudits/GetExceptionAuditsQuery.cs @@ -17,4 +17,8 @@ public sealed class GetExceptionAuditsQuery : IQuery> { + private const int MaxPageSize = 200; + private const int DefaultPageSize = 50; + private readonly AuditDbContext _dbContext; public GetExceptionAuditsQueryHandler(AuditDbContext dbContext) @@ -26,7 +29,11 @@ public async ValueTask> Handle(GetExceptionAudits audits = ApplySeverityFilter(audits, query); audits = ApplyPayloadFilters(audits, query); - return await ProjectToDto(audits, cancellationToken); + // Cap server-side so an unpaged call can't materialize a tenant's whole exception history. + var take = query.Take is >= 1 and <= MaxPageSize ? query.Take.Value : DefaultPageSize; + var skip = query.Skip is > 0 ? query.Skip.Value : 0; + + return await ProjectToDto(audits, skip, take, cancellationToken); } private IQueryable GetBaseQuery() @@ -87,10 +94,12 @@ private static IQueryable ApplyPayloadFilters(IQueryable> ProjectToDto(IQueryable audits, CancellationToken cancellationToken) + private static async Task> ProjectToDto(IQueryable audits, int skip, int take, CancellationToken cancellationToken) { return await audits .OrderByDescending(a => a.OccurredAtUtc) + .Skip(skip) + .Take(take) .Select(a => new AuditSummaryDto { Id = a.Id, diff --git a/src/Modules/Auditing/Modules.Auditing/Features/v1/GetSecurityAudits/GetSecurityAuditsQueryHandler.cs b/src/Modules/Auditing/Modules.Auditing/Features/v1/GetSecurityAudits/GetSecurityAuditsQueryHandler.cs index 34e7c3185a..a014b2d919 100644 --- a/src/Modules/Auditing/Modules.Auditing/Features/v1/GetSecurityAudits/GetSecurityAuditsQueryHandler.cs +++ b/src/Modules/Auditing/Modules.Auditing/Features/v1/GetSecurityAudits/GetSecurityAuditsQueryHandler.cs @@ -10,6 +10,9 @@ namespace FSH.Modules.Auditing.Features.v1.GetSecurityAudits; public sealed class GetSecurityAuditsQueryHandler : IQueryHandler> { + private const int MaxPageSize = 200; + private const int DefaultPageSize = 50; + private readonly AuditDbContext _dbContext; public GetSecurityAuditsQueryHandler(AuditDbContext dbContext) @@ -49,8 +52,14 @@ public async ValueTask> Handle(GetSecurityAuditsQ EF.Functions.ILike(AsText(a.PayloadJson), $"%\"action\": \"{actionValue}\"%")); } + // Cap server-side so an unpaged call can't materialize a tenant's whole audit history. + var take = query.Take is >= 1 and <= MaxPageSize ? query.Take.Value : DefaultPageSize; + var skip = query.Skip is > 0 ? query.Skip.Value : 0; + var list = await audits .OrderByDescending(a => a.OccurredAtUtc) + .Skip(skip) + .Take(take) .Select(a => new AuditSummaryDto { Id = a.Id, diff --git a/src/Tests/Integration.Tests/Tests/Auditing/SecurityAuditListBoundsTests.cs b/src/Tests/Integration.Tests/Tests/Auditing/SecurityAuditListBoundsTests.cs new file mode 100644 index 0000000000..1ac42da29c --- /dev/null +++ b/src/Tests/Integration.Tests/Tests/Auditing/SecurityAuditListBoundsTests.cs @@ -0,0 +1,87 @@ +using Finbuckle.MultiTenant; +using Finbuckle.MultiTenant.Abstractions; +using FSH.Framework.Shared.Multitenancy; +using FSH.Modules.Auditing; +using FSH.Modules.Auditing.Contracts; +using FSH.Modules.Auditing.Contracts.Dtos; +using FSH.Modules.Auditing.Persistence; +using Integration.Tests.Infrastructure; +using Integration.Tests.Infrastructure.Extensions; + +namespace Integration.Tests.Tests.Auditing; + +/// +/// Runtime repro for audit finding DATA-01 (unbounded audit list). The security-audit list handler +/// does ToListAsync with no Skip/Take/cap and the validator does not require a time window, so an +/// unpaged call materializes the whole matching set. A bounded API would cap the page size. +/// +[Collection(FshCollectionDefinition.Name)] +public sealed class SecurityAuditListBoundsTests +{ + private const int SeededRows = 500; + + // Analogous to the 200-row server cap that SessionService.GetTenantSessionsAsync already applies + // "so an over-eager client can't pull a tenant's full session table in one round-trip". + private const int ExpectedServerCap = 200; + + private readonly FshWebApplicationFactory _factory; + private readonly AuthHelper _auth; + + public SecurityAuditListBoundsTests(FshWebApplicationFactory factory) + { + _factory = factory; + _auth = new AuthHelper(factory); + } + + [Fact] + public async Task GetSecurityAudits_Should_CapResults_When_NoPagingOrWindowProvided() + { + await SeedSecurityAuditsAsync(SeededRows); + + using var client = await _auth.CreateRootAdminClientAsync(); + using var response = await client.GetAsync($"{TestConstants.AuditsBasePath}/security"); + response.StatusCode.ShouldBe(HttpStatusCode.OK); + + var list = await response.DeserializeAsync>(); + + list.Count.ShouldBeLessThanOrEqualTo( + ExpectedServerCap, + $"an unpaged security-audit list must cap results; seeded {SeededRows} rows and the endpoint " + + $"returned {list.Count} with no Skip/Take/default page size."); + } + + private async Task SeedSecurityAuditsAsync(int count) + { + using var scope = _factory.Services.CreateScope(); + + // Tenant context is AsyncLocal — set inline in the same method as the DbContext call. + var tenantStore = scope.ServiceProvider.GetRequiredService>(); + var tenant = await tenantStore.GetAsync(TestConstants.RootTenantId); + scope.ServiceProvider.GetRequiredService().MultiTenantContext = + new MultiTenantContext(tenant); + + var db = scope.ServiceProvider.GetRequiredService(); + var now = DateTime.UtcNow; + var rows = new List(count); + for (int i = 0; i < count; i++) + { + rows.Add(new AuditRecord + { + Id = Guid.NewGuid(), + OccurredAtUtc = now.AddSeconds(-i), + ReceivedAtUtc = now, + EventType = (int)AuditEventType.Security, + Severity = (byte)AuditSeverity.Information, + TenantId = TestConstants.RootTenantId, + UserId = "data01-seed-user", + UserName = "data01-seed", + Source = "DATA-01-test", + Tags = 0, + PayloadJson = "{}", + }); + } + + db.AuditRecords.AddRange(rows); + await db.SaveChangesAsync(); + } +}