timothy/ersatztv
1
0
Fork 0
Code Issues 54 Pull Requests 6 Actions Packages Projects Releases 9 Wiki Activity

Page/cap GET /api/search/all-items (DoS hardening — deferred from #285) #293

New Issue
Closed
opened 2026-07-12 00:36:51 +02:00 by timothy · 3 comments
timothy commented 2026-07-12 00:36:51 +02:00
Owner
Copy Link
Copy Source

Deferred from #285 (Bundle A, PR #292).

GET /api/search/all-items (SearchController, QuerySearchIndexAllItems) returns every matching item id with no paging. #285 called for paging/capping it. It was deferred from Bundle A because it feeds the SPA "add all to collection/playlist" flow (web/src/media/addTo/), which materializes the full id set before calling the add endpoints — a naive hard cap would silently truncate "add all".

Why not urgent: the original S10 concern was unauthenticated unbounded exposure; that is now closed — the endpoint is gated by read-gating (Api:RequireKeyForReads default true, #282). The residual is DoS-hardening against an authenticated caller with a very broad query.

Fix options: (a) coordinated pagination — page the endpoint AND teach the SPA add-all flow to iterate pages; or (b) a generous safety cap (e.g. configurable Search:AllItemsMaxResults) with a documented truncation signal the SPA surfaces. Prefer (a).

Triage: backlog (Phase-3 / #197 follow-up). Not a gate.

Resolved via option (a) (operator-confirmed) in PR #442.

Done-when

  • Adversarial/independent review passed
  • Endpoint paged + response carries per-kind totals; params clamped (backend tests green)
  • SPA add-all pages to completeness (SPA tests green)
  • Docs updated (decisions.md, api-conventions.md) + OpenAPI regenerated
  • CI green
Deferred from #285 (Bundle A, PR #292). `GET /api/search/all-items` (`SearchController`, `QuerySearchIndexAllItems`) returns **every** matching item id with no paging. #285 called for paging/capping it. It was deferred from Bundle A because it feeds the SPA "add all to collection/playlist" flow (`web/src/media/addTo/`), which materializes the full id set before calling the add endpoints — a naive hard cap would silently truncate "add all". **Why not urgent**: the original S10 concern was *unauthenticated* unbounded exposure; that is now closed — the endpoint is gated by read-gating (`Api:RequireKeyForReads` default true, #282). The residual is DoS-hardening against an *authenticated* caller with a very broad query. **Fix options**: (a) coordinated pagination — page the endpoint AND teach the SPA add-all flow to iterate pages; or (b) a generous safety cap (e.g. configurable `Search:AllItemsMaxResults`) with a documented truncation signal the SPA surfaces. Prefer (a). **Triage**: backlog (Phase-3 / #197 follow-up). Not a gate. Resolved via **option (a)** (operator-confirmed) in PR #442. ## Done-when - [x] Adversarial/independent review passed - [x] Endpoint paged + response carries per-kind totals; params clamped (backend tests green) - [x] SPA add-all pages to completeness (SPA tests green) - [x] Docs updated (decisions.md, api-conventions.md) + OpenAPI regenerated - [x] CI green
timothy added the apipriority: lowsecurity labels 2026-07-12 00:36:51 +02:00
timothy added priority: medium and removed priority: low labels 2026-07-17 00:25:09 +02:00
timothy commented 2026-07-17 00:25:35 +02:00
Author
Owner
Copy Link
Copy Source

🔗 Session bundle — security hardening: #293, #376. Both are endpoint-hardening; work together. (Backlog-grooming 2026-07-17.)

🔗 **Session bundle — security hardening**: #293, #376. Both are endpoint-hardening; work together. (Backlog-grooming 2026-07-17.)
timothy added the in-progress label 2026-07-18 02:06:12 +02:00
timothy commented 2026-07-18 02:06:21 +02:00
Author
Owner
Copy Link
Copy Source

Claiming (Claude Code / Opus 4.8 orchestrator session).

Selection trace — arc frontier empty (all six arc items closed); tier-2 (open milestones) exhausted for pickup: #383 is the Auto-Tune DetailPanel epic container (children #384/#385/#386 all closed), #425 + #176 are in-progress (live parallel sessions), #395 is gated on open #381 (goldens). Tier-3 review pool empty: no review-labeled issue open, and all five ersatztv reviewer audits (adversarial-reviewer #20–#24) have posted deliverables. No priority: high issues exist. Fell through to priority: medium, lowest eligible number → #293. Verified directly: state open, deps [] (unblocked), not in-progress; bundle sibling #376 already closed so this is now standalone (no collision with #425/#176).

Plan (design-first, per repo rules): recon SearchController.QuerySearchIndexAllItems + the SPA add-all flow (web/src/media/addTo/) to pin the exact contract, decide between the issue's option (a) coordinated pagination vs (b) generous safety cap with a truncation signal, record the decision in docs/decisions.md, then implement backend + SPA + tests + live-E2E + OpenAPI regen + docs. No code until the design decision is recorded.

Claiming (Claude Code / Opus 4.8 orchestrator session). **Selection trace** — arc frontier empty (all six arc items closed); tier-2 (open milestones) exhausted for pickup: #383 is the Auto-Tune DetailPanel **epic container** (children #384/#385/#386 all closed), #425 + #176 are `in-progress` (live parallel sessions), #395 is gated on open #381 (goldens). Tier-3 review pool empty: no `review`-labeled issue open, and all five `ersatztv` reviewer audits (adversarial-reviewer #20–#24) have posted deliverables. No `priority: high` issues exist. Fell through to `priority: medium`, lowest eligible number → **#293**. Verified directly: `state open`, deps `[]` (unblocked), not `in-progress`; bundle sibling #376 already closed so this is now standalone (no collision with #425/#176). **Plan (design-first, per repo rules):** recon `SearchController.QuerySearchIndexAllItems` + the SPA add-all flow (`web/src/media/addTo/`) to pin the exact contract, decide between the issue's option (a) coordinated pagination vs (b) generous safety cap with a truncation signal, record the decision in `docs/decisions.md`, then implement backend + SPA + tests + live-E2E + OpenAPI regen + docs. No code until the design decision is recorded.
timothy removed the in-progress label 2026-07-18 14:18:01 +02:00
timothy commented 2026-07-18 14:18:04 +02:00
Author
Owner
Copy Link
Copy Source

Closed by PR #442 (merged to main).

What was done — paginated GET /api/v1/search/all-items (was ten index searches at limit: 0 = every hit, so a broad authenticated query materialized the whole index into one response). Design option (a) full pagination (operator-confirmed): optional pageNum/pageSize (clamped pageSize 1–1000, pageNum 0–2_000_000), an additive per-kind Totals on the response, and the SPA add-all flow (getAllSearchItemIds) pages to completeness instead of a single unbounded fetch.

Root cause (why it existed): the endpoint was built for the SPA's "add all" convenience and passed limit: 0 to ISearchIndex.Search to fetch every id in one shot — fine when read-gating blocked anonymous callers, but it left unbounded per-request work for an authenticated broad query once #282 closed the anon hole. Fix threads a real skip/limit (native Lucene support) and returns per-kind TotalCount so the client can page.

Files changed: SearchController.SearchAllItems, QuerySearchIndexAllItems(Handler), SearchResultAllItemsViewModel, SearchResultAllItemsResponseModel + new SearchResultAllItemsTotalsResponseModel; web/src/api/search.ts (+SearchScreen.addAll); controller/handler/SPA tests; docs/decisions.md, docs/api-conventions.md §5; regenerated OpenAPI trio.

Deferred / follow-up: the add POST still accepts the full merged id set in one body — bounding that surface is out of scope here and tracked under the add-path idempotency work (#308). No new issues filed.

Docs updated: decisions.md (2026-07-18 #293) + api-conventions.md §5.

Review: two cold-context adversarial passes → MERGEABLE; both LOW findings (pageNum overflow, missing-totals under-fetch) fixed in-diff. CI green on the merged head.

Closed by PR #442 (merged to `main`). **What was done** — paginated `GET /api/v1/search/all-items` (was ten index searches at `limit: 0` = every hit, so a broad *authenticated* query materialized the whole index into one response). Design **option (a) full pagination** (operator-confirmed): optional `pageNum`/`pageSize` (clamped `pageSize` 1–1000, `pageNum` 0–2_000_000), an additive per-kind `Totals` on the response, and the SPA add-all flow (`getAllSearchItemIds`) pages to completeness instead of a single unbounded fetch. **Root cause** (why it existed): the endpoint was built for the SPA's "add all" convenience and passed `limit: 0` to `ISearchIndex.Search` to fetch every id in one shot — fine when read-gating blocked anonymous callers, but it left unbounded per-request work for an authenticated broad query once #282 closed the anon hole. Fix threads a real `skip`/`limit` (native Lucene support) and returns per-kind `TotalCount` so the client can page. **Files changed**: `SearchController.SearchAllItems`, `QuerySearchIndexAllItems(Handler)`, `SearchResultAllItemsViewModel`, `SearchResultAllItemsResponseModel` + new `SearchResultAllItemsTotalsResponseModel`; `web/src/api/search.ts` (+`SearchScreen.addAll`); controller/handler/SPA tests; `docs/decisions.md`, `docs/api-conventions.md` §5; regenerated OpenAPI trio. **Deferred / follow-up**: the add POST still accepts the full merged id set in one body — bounding *that* surface is out of scope here and tracked under the add-path idempotency work (#308). No new issues filed. **Docs updated**: `decisions.md` (2026-07-18 #293) + `api-conventions.md` §5. **Review**: two cold-context adversarial passes → MERGEABLE; both LOW findings (pageNum overflow, missing-totals under-fetch) fixed in-diff. CI green on the merged head.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ad-hoc
One-off / ad-hoc work not tracked by a dedicated issue
 api
REST API / HTTP endpoints
 bug
Something isn't working
 ci-cd
Build, test, deploy pipeline
 content
Channel content / schedules / playlists
 dependencies
Dependency updates (Renovate)
 enhancement
New feature or improvement
 frontend
ChicoryTV React SPA frontend
 in-progress
Claimed by an active session — do not pick up
 jellyfin
Jellyfin tuner / IPTV integration
 parked
Excluded from automatic queue pickup; work only when explicitly selected
 priority: high
priority: low
priority: medium
review
Adversarial review finding
 security
Security / vulnerability fix
No labels api priority: medium security
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#293