From 1e15b84dce7b5c5069f3b22e8c5a3e0f24a07fa2 Mon Sep 17 00:00:00 2001 From: cesnimda Date: Wed, 1 Jul 2026 00:29:45 +0200 Subject: [PATCH] fix(search): move pagination clamp to controller (fixes cleanup regression) Phase 5 re-validation caught a functional regression: the V-10 clamp in SearchService.SearchAsync also capped CleanupService's internal target resolution (pageSize 10000 -> 200), silently limiting bulk cleanup-by-query to 200 emails. The clamp belongs at the user-facing trust boundary, not the shared service: move MaxPageSize (200) enforcement into SearchController (both the POST body and GET query paths). Internal callers of ISearchService now request large pages unhindered, while user requests are still bounded. Adds a regression test proving SearchService returns a 250-row page uncapped. No security regressions per Phase 5. All 39 tests green. Co-Authored-By: Claude Opus 4.8 --- .../Controllers/SearchController.cs | 16 +++++- .../Search/SearchService.cs | 12 ++--- .../SearchPaginationTests.cs | 51 +++++++++++++++++++ 3 files changed, 68 insertions(+), 11 deletions(-) create mode 100644 tests/InboxIntel.IntegrationTests/SearchPaginationTests.cs diff --git a/src/InboxIntel.Api/Controllers/SearchController.cs b/src/InboxIntel.Api/Controllers/SearchController.cs index 5b37d57..6716518 100644 --- a/src/InboxIntel.Api/Controllers/SearchController.cs +++ b/src/InboxIntel.Api/Controllers/SearchController.cs @@ -7,19 +7,31 @@ namespace InboxIntel.Api.Controllers; public class SearchController : ApiControllerBase { + /// Hard upper bound on a user-requested page (V-10: DoS via huge pageSize). + /// Enforced HERE, at the user-facing trust boundary, so internal callers of + /// ISearchService (e.g. cleanup target resolution) can still request large pages. + private const int MaxPageSize = 200; + private readonly ISearchService _search; public SearchController(ISearchService search) => _search = search; /// Structured search via JSON body. [HttpPost] public async Task Search([FromBody] SearchRequestDto request, CancellationToken ct) - => Ok(await _search.SearchAsync(UserId, request, ct)); + { + var clamped = request with + { + Page = Math.Max(1, request.Page), + PageSize = Math.Clamp(request.PageSize, 1, MaxPageSize) + }; + return Ok(await _search.SearchAsync(UserId, clamped, ct)); + } /// Gmail-like query string search, e.g. ?q=from:github.com is:unread. [HttpGet] public async Task Query([FromQuery] string? q, [FromQuery] int page = 1, [FromQuery] int pageSize = 50, CancellationToken ct = default) { - var parsed = GmailQueryParser.Parse(q, page, pageSize); + var parsed = GmailQueryParser.Parse(q, Math.Max(1, page), Math.Clamp(pageSize, 1, MaxPageSize)); return Ok(await _search.SearchAsync(UserId, parsed, ct)); } } diff --git a/src/InboxIntel.Infrastructure/Search/SearchService.cs b/src/InboxIntel.Infrastructure/Search/SearchService.cs index 79b9e91..ef3b0b4 100644 --- a/src/InboxIntel.Infrastructure/Search/SearchService.cs +++ b/src/InboxIntel.Infrastructure/Search/SearchService.cs @@ -17,17 +17,11 @@ public class SearchService : ISearchService private readonly AppDbContext _db; public SearchService(AppDbContext db) => _db = db; - /// Hard upper bound on a user-facing page of results (V-10: DoS via huge pageSize). - public const int MaxPageSize = 200; - public async Task> SearchAsync(Guid userId, SearchRequestDto r, CancellationToken ct = default) { - // V-10: clamp pagination at the user-facing chokepoint (covers both the GET - // query-string path and the POST body path) so a caller cannot request an - // unbounded materialisation. Internal callers (e.g. cleanup target resolution) - // do not go through this service, so their larger pages are unaffected. - r = r with { Page = Math.Max(1, r.Page), PageSize = Math.Clamp(r.PageSize, 1, MaxPageSize) }; - + // NOTE: pagination is clamped at the user-facing trust boundary (SearchController), + // NOT here, so internal callers (e.g. CleanupService target resolution, which + // legitimately requests large pages) are unaffected. See V-10 fix. var q = _db.Emails.AsNoTracking().Where(e => e.UserId == userId); if (!string.IsNullOrWhiteSpace(r.Sender)) diff --git a/tests/InboxIntel.IntegrationTests/SearchPaginationTests.cs b/tests/InboxIntel.IntegrationTests/SearchPaginationTests.cs new file mode 100644 index 0000000..ee5294c --- /dev/null +++ b/tests/InboxIntel.IntegrationTests/SearchPaginationTests.cs @@ -0,0 +1,51 @@ +using FluentAssertions; +using InboxIntel.Application.Abstractions; +using InboxIntel.Application.Search; +using InboxIntel.Domain.Entities; +using InboxIntel.Infrastructure.Persistence; +using InboxIntel.Infrastructure.Search; +using Microsoft.EntityFrameworkCore; +using Xunit; + +namespace InboxIntel.IntegrationTests; + +/// +/// Regression guard for the V-10 pagination clamp: the clamp must live at the +/// user-facing controller boundary, NOT in SearchService — otherwise internal +/// callers (CleanupService resolves targets with pageSize 10000) would be silently +/// capped, breaking bulk cleanup-by-query. This proves the service itself honours a +/// large page. +/// +public class SearchPaginationTests +{ + private sealed class FakeCurrentUser : ICurrentUser + { + public Guid UserId { get; set; } + public bool IsAuthenticated => UserId != Guid.Empty; + } + + [Fact] + public async Task SearchService_does_not_clamp_large_internal_page() + { + var user = Guid.NewGuid(); + var opts = new DbContextOptionsBuilder() + .UseInMemoryDatabase(nameof(SearchService_does_not_clamp_large_internal_page)).Options; + + using (var seed = new AppDbContext(opts, new FakeCurrentUser())) + { + var sender = new Sender { UserId = user, Address = "sender@example.com", DisplayName = "Sender" }; + seed.Senders.Add(sender); + for (var i = 0; i < 250; i++) + seed.Emails.Add(new Email { UserId = user, GmailMessageId = $"m{i}", Subject = $"s{i}", SenderId = sender.Id, Sender = sender }); + await seed.SaveChangesAsync(); + } + + using var ctx = new AppDbContext(opts, new FakeCurrentUser { UserId = user }); + // Mirrors how CleanupService resolves targets: a large page via the parser. + var request = GmailQueryParser.Parse(null, page: 1, pageSize: 10_000); + var result = await new SearchService(ctx).SearchAsync(user, request); + + result.Items.Should().HaveCount(250); // not capped at 200 + result.TotalCount.Should().Be(250); + } +}