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

#197 S2 [HIGH]: persistent mutation outside /api bypasses the write key even when configured (SortController) #281

New Issue
Closed
opened 2026-07-11 23:20:30 +02:00 by timothy · 2 comments
timothy commented 2026-07-11 23:20:30 +02:00
Owner
Copy Link
Copy Source

From the #197 cold API security review. Phase 1 hard gate.

Finding (HIGH): POST /media/collections/{collectionId}/items (SortController, [ApiExplorerSettings(IgnoreApi=true)], [FromForm]) dispatches UpdateCollectionCustomOrder — a persistent mutation that commits + rebuilds affected playouts. It sits outside /api, so ApiKeyAuthorizationFilter skips it (ApiKeyAuthorizationFilter.cs:67) → it bypasses the key even when one is configured, and is reachable by a plain cross-origin HTML form (CORS not even needed). POST /account/logout (AccountController) is similarly non-/api + CSRF-reachable. (SortController.cs:15.)

Fix (Phase-1 #2): delete both. Confirmed against source: the SPA reorders collections via PUT /api/collections/{id}/custom-order (web/src/api/collections.ts:85, ETag-guarded) — SortController has zero SPA callers, it's dead Blazor SortableJS residue. /account/logout is dead OIDC/Blazor-era. Removing beats guarding. Refs #197.

From the #197 cold API security review. **Phase 1 hard gate.** **Finding (HIGH):** `POST /media/collections/{collectionId}/items` (`SortController`, `[ApiExplorerSettings(IgnoreApi=true)]`, `[FromForm]`) dispatches `UpdateCollectionCustomOrder` — a persistent mutation that commits + rebuilds affected playouts. It sits **outside `/api`**, so `ApiKeyAuthorizationFilter` skips it (`ApiKeyAuthorizationFilter.cs:67`) → it bypasses the key **even when one is configured**, and is reachable by a plain cross-origin HTML form (CORS not even needed). `POST /account/logout` (`AccountController`) is similarly non-`/api` + CSRF-reachable. (`SortController.cs:15`.) **Fix (Phase-1 #2): delete both.** Confirmed against source: the SPA reorders collections via `PUT /api/collections/{id}/custom-order` (`web/src/api/collections.ts:85`, ETag-guarded) — `SortController` has **zero SPA callers**, it's dead Blazor SortableJS residue. `/account/logout` is dead OIDC/Blazor-era. Removing beats guarding. Refs #197.
timothy added the priority: highreviewsecurity labels 2026-07-11 23:20:30 +02:00
timothy commented 2026-07-11 23:28:41 +02:00
Author
Owner
Copy Link
Copy Source

Bundling — Bundle A "auth posture" (single session / one PR): #280 + #281 + #282 + #284 + #285. Shared surfaces (ApiKeyAuthorizationFilter, Startup.cs, SPA client.ts, ApiControllerSecurityTests) → do together to avoid conflicts. This one is a trivial deletion (SortController + /account/logout) that closes the same auth-completeness gap. Parallel-safe against Bundle B (#283) and #289. Phase-1 gate. See #197 roadmap.

**Bundling — Bundle A "auth posture" (single session / one PR): #280 + #281 + #282 + #284 + #285.** Shared surfaces (`ApiKeyAuthorizationFilter`, `Startup.cs`, SPA `client.ts`, `ApiControllerSecurityTests`) → do together to avoid conflicts. This one is a trivial deletion (SortController + `/account/logout`) that closes the same auth-completeness gap. Parallel-safe against Bundle B (#283) and #289. Phase-1 gate. See #197 roadmap.
timothy added the in-progress label 2026-07-11 23:46:39 +02:00
timothy commented 2026-07-12 00:35:33 +02:00
Author
Owner
Copy Link
Copy Source

Closed — PR #292 (merged FF to main ee6be81c)

Done (S2). Deleted SortController (POST media/collections/{id}/items, dead Blazor SortableJS residue — the SPA reorders via PUT /api/collections/{id}/custom-order) and AccountController (POST account/logout, dead OIDC). Both were non-/api persistent mutations that bypassed the key even when configured. Grep confirmed zero remaining references (C# or web/); the cold review re-verified. Live E2E: both POSTs now hit no handler (302→/app catch-all).

Files: removed ErsatzTV/Controllers/SortController.cs, ErsatzTV/Controllers/AccountController.cs.

## Closed — PR #292 (merged FF to main `ee6be81c`) **Done (S2).** Deleted `SortController` (`POST media/collections/{id}/items`, dead Blazor SortableJS residue — the SPA reorders via `PUT /api/collections/{id}/custom-order`) and `AccountController` (`POST account/logout`, dead OIDC). Both were non-`/api` persistent mutations that bypassed the key even when configured. Grep confirmed zero remaining references (C# or `web/`); the cold review re-verified. Live E2E: both POSTs now hit no handler (302→/app catch-all). **Files**: removed `ErsatzTV/Controllers/SortController.cs`, `ErsatzTV/Controllers/AccountController.cs`.
timothy removed the in-progress label 2026-07-12 00:36:04 +02:00
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 priority: high review 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#281