feat(backend): /api/persons — query length cap, limit enforcement, response DTO #633

New Issue

marcel · 2026-05-19T23:31:52+02:00

marcel commented

2026-05-19 23:31:52 +02:00

Context

The frontend at MentionDropdown.svelte and PersonMentionEditor.svelte (PR #629) defends against amplification attacks with client-side maxlength="100" on the search input and &limit=5 on the fetch URL. Both are advisory only — any script in a compromised tab can call fetch('/api/persons?q=' + 'A'.repeat(100000)). The backend PersonController is currently unbounded.

Additionally, the response shape leaks notes, alias, etc. to clients that only need name/dates for typeahead (CWE-200).

Required changes

PersonController.search: add @Size(max = 100) @RequestParam String q (or equivalent Bean Validation). 414/400 on overlong queries (CWE-400, CWE-602).
?limit= enforcement: read @RequestParam(defaultValue = "20") @Min(1) @Max(50) Integer limit and slice service result accordingly.
Response DTO: introduce PersonSummaryDTO { id, displayName, birthYear, deathYear, personType, familyMember } — strip notes, alias, title from the search response. Update OpenAPI annotations to drive frontend type regeneration.
Test coverage: unit + slice test asserting 400 on 101-char query, 50-cap honoured, response shape excludes private fields.

Acceptance

Query cap enforced; integration test asserts 400 with code = INVALID_INPUT.
Limit param honoured up to 50.
PersonSummaryDTO exposed via OpenAPI; frontend regenerates and uses it.
Frontend Person cast in runSearch updated to PersonSummaryDTO (separate PR).

Reviewer rationale: Markus #10919/#10959, Nora #10933/#10969.

Additional concern — CWE-770 (Resource throttling)

Capping query length blocks one DoS vector but /api/persons itself has no per-principal rate limit. Even with a 100-char cap, an authenticated tab (or a compromised one) can call the endpoint 10,000 times in a tight loop.

Required: per-session rate limit on /api/persons — e.g. Bucket4j filter or a Spring Security OncePerRequestFilter keyed on principal/session. Suggested envelope: 20 rps burst, 5 rps sustained, tunable. Frontend self-limits at ~6.7 rps (150 ms debounce, one inflight), so this allows ~3× headroom for autofocus/batch use while still throttling abuse.
- Log each 429 at WARN with the throttled principal id and the rolling rate that triggered it — silent throttling makes incidents invisible (CWE-778). Sample to 1/10 if the volume becomes noisy.
Acceptance: integration test asserts the 21st request inside 1 s returns 429 with code = TOO_MANY_REQUESTS (or the existing rate-limit error code), and burst recovers after the bucket refills.
Out of scope here: a project-wide rate-limit framework. If this turns into a chore, file a sibling issue and reduce this entry to "wire /api/persons into whatever the framework exposes."

Reviewer rationale: Nora #11076 (CWE-770) on PR #629 round 3; WARN-logging sub-bullet per Markus polish on PR #629 round 4 (#11204).

## Context The frontend at `MentionDropdown.svelte` and `PersonMentionEditor.svelte` (PR #629) defends against amplification attacks with client-side `maxlength="100"` on the search input and `&limit=5` on the fetch URL. Both are advisory only — any script in a compromised tab can call `fetch('/api/persons?q=' + 'A'.repeat(100000))`. The backend `PersonController` is currently unbounded. Additionally, the response shape leaks `notes`, `alias`, etc. to clients that only need name/dates for typeahead (CWE-200). ## Required changes 1. **`PersonController.search`**: add `@Size(max = 100) @RequestParam String q` (or equivalent Bean Validation). 414/400 on overlong queries (CWE-400, CWE-602). 2. **`?limit=` enforcement**: read `@RequestParam(defaultValue = "20") @Min(1) @Max(50) Integer limit` and slice service result accordingly. 3. **Response DTO**: introduce `PersonSummaryDTO { id, displayName, birthYear, deathYear, personType, familyMember }` — strip `notes`, `alias`, `title` from the search response. Update OpenAPI annotations to drive frontend type regeneration. 4. **Test coverage**: unit + slice test asserting 400 on 101-char query, 50-cap honoured, response shape excludes private fields. ## Acceptance - [ ] Query cap enforced; integration test asserts 400 with `code = INVALID_INPUT`. - [ ] Limit param honoured up to 50. - [ ] `PersonSummaryDTO` exposed via OpenAPI; frontend regenerates and uses it. - [ ] Frontend `Person` cast in `runSearch` updated to `PersonSummaryDTO` (separate PR). Reviewer rationale: Markus #10919/#10959, Nora #10933/#10969. ### Additional concern — CWE-770 (Resource throttling) Capping query length blocks one DoS vector but `/api/persons` itself has no per-principal rate limit. Even with a 100-char cap, an authenticated tab (or a compromised one) can call the endpoint 10,000 times in a tight loop. - **Required**: per-session rate limit on `/api/persons` — e.g. Bucket4j filter or a Spring Security `OncePerRequestFilter` keyed on principal/session. Suggested envelope: **20 rps burst, 5 rps sustained**, tunable. Frontend self-limits at ~6.7 rps (150 ms debounce, one inflight), so this allows ~3× headroom for autofocus/batch use while still throttling abuse. - Log each 429 at WARN with the throttled principal id and the rolling rate that triggered it — silent throttling makes incidents invisible (CWE-778). Sample to 1/10 if the volume becomes noisy. - **Acceptance**: integration test asserts the 21st request inside 1 s returns 429 with `code = TOO_MANY_REQUESTS` (or the existing rate-limit error code), and burst recovers after the bucket refills. - **Out of scope here**: a project-wide rate-limit framework. If this turns into a chore, file a sibling issue and reduce this entry to "wire `/api/persons` into whatever the framework exposes." Reviewer rationale: Nora #11076 (CWE-770) on PR #629 round 3; WARN-logging sub-bullet per Markus polish on PR #629 round 4 (#11204).

marcel added the P2-medium feature security labels 2026-05-19 23:31:55 +02:00