Centralize SDK and threading analyzers, baseline the .NET 10 All rule inventory at suggestion severity, and promote S3981 repo-wide. Fix the always-true worker count predicate and cover the idle/active branches.
Fixes#15
Co-Authored-By: Codex <codex@openai.com>
Independent review found that AdvertisedBaseUrl.TryParse accepts a path
prefix containing '&' (a legal URL-path char kept out of uri.Query), but
GetChannelGuideHandler substituted {RequestBase} raw into pre-built XML
written unescaped — so a configured base like https://host/a&b emitted a
bare '&', malforming the entire XMLTV guide (clients reject the document).
Escape the substituted base with SecurityElement.Escape before the raw
replace, mirroring how {AccessTokenUri} is already pre-escaped (&).
No-op for normal URLs (goldens unchanged); M3U output is untouched (M3U
isn't XML). Adds a regression test asserting '&' → '&' in the guide.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ErsatzTV built every absolute M3U/XMLTV URL from the incoming request's
Scheme/Host/PathBase, so a client fetching via a host that downstream
consumers can't resolve (e.g. Dispatcharr over Docker DNS → Kodi) baked
that internal host into programme-image/stream URLs.
Add an optional advertised IPTV base URL, backed by the existing
ConfigElement key/value store (key `iptv.base_url`, no EF migration):
- Central pure Core helper `AdvertisedBaseUrl` (TryParse/Resolve):
validates absolute http(s), no credentials/query/fragment, preserves
port + path prefix, normalizes trailing slash. Blank/invalid falls
back to the request-derived values, so unset output is byte-identical.
- Resolved inside `GetChannelPlaylistHandler` (M3U guide/logo/stream) and
`GetChannelGuideHandler` (both XMLTV {RequestBase} sites) — controllers
stay thin, golden tests untouched.
- New `iptv` settings group: GET/PUT /api/v1/settings/iptv (blank clears,
malformed → 422) + a new IPTV section on the SPA Settings screen.
- Scoped to M3U + XMLTV; HDHomeRun deliberately out of scope. Distinct
from ETV_BASE_URL (which only sets ASP.NET PathBase).
Tests: AdvertisedBaseUrl unit tests (override/fallback/port/path/invalid),
handler override tests for both generators, settings controller + handler
tests, SPA client + screen tests. Docs: m3u-xmltv, decisions, domain-model,
regenerated OpenAPI v1.json + v1.d.ts + endpoint-index.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Clears the still-live findings from #172 (verified against main; #2/#4/#7 and the
auth/search/Trakt tail were already deliberate-documented or fixed since 2026-07-07).
- Null/empty Name → 500 (10 create/replace handlers). Block/Template/DecoTemplate/Deco
Create+Replace/Update + UpdateFFmpegProfile did `request.Name.Length > 50` on a
client-nullable string → unhandled NullReferenceException → HTTP 500 (no global
exception filter). Now `string.IsNullOrWhiteSpace(request.Name) || .Length > 50` →
422; also rejects empty/whitespace names, matching the group-create handlers'
NotEmpty behavior. CreatePlaylist coalesces null→"" at the DTO so it was an
empty-name persist, not a 500; guarded the same way.
- ReplaceTemplateItems overlap validation iterated with an `item == otherItem`
record value-equality skip, so two exact-duplicate items were value-equal and
bypassed the intersection check (both persisted). Now index-based (i != j) so
duplicates register as a self-intersection and are rejected 422.
- Trimmed the unreachable 404 ProducesResponseType from POST /api/blocks/groups and
POST /api/templates/groups (a create has no parent lookup that can 404); v1.json
regenerated.
- Regression tests: all 10 name-guard paths + the duplicate-items path (19 cases).
- Docs: decisions.md entry + api-conventions.md §3b null-safe-validation bullet.
fixes#172
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Version every /api route to /api/v1 (251 controller routes + ~24 Location
headers + the scanner callback URL + the Startup request-log literal),
uniform across the machine API, auth, scanner and scripted-build surfaces.
Add ApiVersionRewriteMiddleware: a legacy unversioned /api/* request is
rewritten (NOT redirected) to /api/v1/* in-pipeline — method, body, auth
headers and query survive — carrying RFC 8594 Deprecation/Sunset headers,
so curl / the future MCP server / bookmarks keep working. An already-
versioned path passes through; a future /api/v2 is never forced to v1.
Standardize the route convention (leading-slash absolute route per method,
no class-[Route] — except the two Scanner/Scripted controllers whose ~all
actions share a parametrized {id} prefix), enforced by ApiRouteVersioningTests
(^/api/v\d+/ over the whole Controllers.Api surface; browser-nav
/auth/oidc/login is out of scope).
Regenerate v1.json (160 paths, all /api/v1)/endpoint-index/v1.d.ts; sweep 945
SPA request literals + the test mocks (regex + positional URL parsers). /api/v1
is additive-only after freeze; the legacy-rewrite shim sunsets in ~2 releases
(owner decision) with removal tracked as a Phase-3 follow-up.
Docs: decisions.md 2026-07-13, api-conventions §1/§9, rest-api/spa-conventions/
blazor-route-parity/e2e-local/domain-model.
fixes#286
refs #197
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Independent review fix commit (cold fork MERGEABLE-WITH-NITS + Codex BLOCKED, 2 Highs):
- Codex H1: a comma (0x2C) is a valid etagc and can appear INSIDE a quoted opaque-tag
("3,5" is ONE tag). The old Split(',') broke it into two malformed tokens → 400. Replaced
with a quote-aware position scanner that treats a comma as a separator only outside the
quotes; "3,5" is now one valid non-canonical tag → 412.
- Codex H2: RFC 7230 OWS is SP/HTAB only. string.Trim() also strips NBSP and other Unicode
whitespace, letting " * " masquerade as the "*" force-write escape. Trim only
(' ', '\t'); such input is now Malformed → 400.
- Fork nit: corrected the canonical-guard comment (interior-whitespace tags are rejected by
IsEtagc, not NumberStyles.None).
- CI Formatting gate: de-BOM the 8 touched legacy Application .cs (charset=utf-8, #311/#310).
- Tests: added comma-in-tag ("3,5", "x,y","3"), empty-element tolerance, NBSP-not-OWS,
trailing-junk, lowercase-weak, wildcard-in-list cases. Full ErsatzTV.Tests green (1556).
Refs #253#197
The shared optimistic-concurrency parser (ConcurrencyHeaders.ParseIfMatch) classified any
non-canonical/weak/list If-Match value as Malformed → 400. Per RFC 7232 §3.1 a syntactically
-valid entity-tag that simply doesn't strong-match must be 412; 400 is only for a genuine
grammar violation.
- Rewrite ParseIfMatch as a real RFC 7232 entity-tag/list parser: walks the comma-separated
1#entity-tag list, validates each [W/]DQUOTE *etagc DQUOTE member, and collects the strong
members whose opaque text is our canonical decimal. Weak / empty / non-canonical /
out-of-range tags are valid but contribute no version (→ empty set → 412); genuine grammar
violations (unquoted, SP-in-tag, unterminated, garbage) → 400.
- Reshape IfMatchCondition.ExpectedVersion : Option<int> → ExpectedVersions : Option<Seq<int>>
and VersionedAggregateExtensions.CheckVersion → set membership (any strong match proceeds;
empty set always 412). Threads through 10 replace/update commands + handlers + request
mappers + 9 controllers.
- No wire-contract change (400 + 412 already declared on every PUT; the field is header-derived
and internal — no DTO/route/response-type/OpenAPI change).
- Tests: ConcurrencyHeadersTests rewritten for the new classification (lists, weak, empty,
non-canonical → Version/empty-set; grammar violations → Malformed) + new
VersionedAggregateExtensionsTests for CheckVersion membership/empty-set/force-write.
- Docs: api-conventions.md §7a rewritten; decisions.md entry appended.
Refs #253#197fixes#265
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- GET /api/graphics-elements no longer side-effects; refresh moved to
POST /api/graphics-elements/refresh (204), closing a CSRF vector on a GET.
- PrepareTroubleshootingPlaybackHandler now returns a typed LockedError from
both atomic lock-acquire failures; ApiResults.ToErrorResult maps it to 409
instead of falling through to 422, so a lock lost in the race between the
controller's pre-check and the handler's atomic acquire still reports 409.
- AuthController.MachineKey sets Cache-Control: no-store + Pragma: no-cache
on the 200 response carrying the master API key.
- Reworded the stale "subtitleId query parameter" endpoint description now
that playback/start takes a JSON body.
- Regenerated openapi/v1.json + docs/endpoint-index.md; docs/api-conventions.md
updated with the LockedError pattern (§3a) and the ToErrorResult table row.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Independent Codex review (reconciled by Fable against a MERGEABLE fork verdict) found
SaveChangesForcingVersion silently DROPPED a pending Version++ under a concurrent
versioned-write race: on DbUpdateConcurrencyException it adopted the DB's current
Version verbatim (original = current = dbVersion), so a bumping sibling committed at
dbVersion instead of dbVersion+1. Net: an editor holding the concurrent writer's ETag
was never invalidated by the sibling's change — the exact lost-update the #253/#269
contract exists to close, lost under the very condition the helper handles.
F1 fix (shared helper, corrects all 25 bumpers incl. the pre-existing Add*ToPlaylist /
schedule-item writers): rebase the pending delta on top of the stored token —
pendingDelta = current - original; original = dbVersion; current = dbVersion + pendingDelta
Bumpers (delta 1) advance to dbVersion+1; non-bumpers/deletes (delta 0, e.g.
ErasePlayoutHistory) still adopt the stored token unchanged, so RootWriterForceVersionTests
is unaffected. Idempotent across the bounded retry loop.
F3: the force-race tests now assert Version==3 (rebase), not just membership survival;
added the missing Playout force-race+rotate test. Negative-controlled: with the helper
fix reverted, both strengthened tests go red.
F2 (Medium, deferred → #308): two concurrent same-item Add*ToCollection can both pass the
membership check and the loser 500s on the composite-PK violation (DbUpdateException, which
the helper doesn't catch). Pre-existing and narrow (no corruption); doc claims softened to
name it. Filed #308.
Docs: api-conventions §7a + decisions.md prose corrected from "adopt the stored token" to
the rebase semantics.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Complete the #253 optimistic-concurrency contract's cross-editor ETag
rotation tail. The non-If-Match config siblings mutated editor-visible
state without bumping Version, so a concurrent editor of the same root
never invalidated. Now the Collection Add*/Remove handlers bump
Collection.Version, and UpdateCollection / UpdatePlayout / the three
ScheduleFile writers (which already force-wrote past a concurrent bump)
now bump too — all via SaveChangesForcingVersion (no If-Match → force
write, never 412/500).
No-op idempotence (Fable-caught trap): these gate reindex/BuildPlayout
fan-out on SaveChanges()>0, so an unconditional bump would fire spurious
rebuilds on an idempotent re-add / same-value re-submit. Each now
short-circuits a genuine no-op before the bump — Add handlers by an
explicit membership check (also fixing a latent duplicate-CollectionItem
insert), scalar writers by ChangeTracker.HasChanges().
Corrects #269's framing: the Add*ToCollection family is not
repository-mediated (IMediaCollectionRepository is read-only); each
handler writes via its own dbContext, so the scanner's separate
membership path is unaffected (a background scan does not rotate the
editor ETag).
Tests: CollectionEtagRotationTests + PlayoutScheduleFileEtagRotationTests
(rotation, no-op-without-bump-or-rebuild, force-write-past-concurrent-bump),
no-op guard proven non-vacuous by inverting the membership check.
Docs: api-conventions §7a + decisions.md. No new status codes / no
OpenAPI change (these endpoints take no If-Match, never 412).
The #265 RFC-7232 If-Match parser refinement is a separate PR.
fixes#269
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Independent adversarial review (cold fork + Codex) of the first cut converged
on one real miss and two boundaries to document.
- **ErasePlayoutHistoryHandler** (HIGH, both reviewers): modifies Playout ROOT
scalars (Seed/Anchor/OnDemandCheckpoint) *without* bumping Version, inside an
explicit transaction with no try/catch, so it 500s on a concurrent bump —
reachable via POST /api/playouts/{id}/erase-items-and-history. My first sweep
filtered on "Version-bumpers + deletes"; the true exposure surface is "any
handler leaving a versioned root Modified/Deleted", so this slipped through.
Now routes through SaveChangesForcingVersion (+ a non-vacuous through-handler
test that exercises the explicit-transaction path). Re-swept with the correct
filter: ErasePlayoutItems (AsNoTracking + ExecuteDelete children only) and
ResetAllPlayouts (read-only + enqueue) are NOT exposed.
- **Background build/time-shift Playout-scalar writers** (BuildPlayout via
PlayoutBuilder, PlayoutTimeShifter): token-guarded too, but intentionally left
on plain save — they already catch (build-failure, not 500), and force-writing
would persist output built from stale config (the concurrent config bump already
enqueues a rebuild). Documented as a deliberate boundary, not a gap.
- **Item-add index collision** under force-write: documented as an accepted
Phase-1 effect (non-corrupting, self-correcting; reload-recompute refinement
is a #197 candidate).
Also corrects the docs' "every Version bumper" framing to the true filter and the
test docstring's over-broad non-vacuity claim. Full ErsatzTV.Tests green (1483).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fix-commit re-review confirmed the 1st-round fixes resolved and caught a 2nd round:
- HIGH — env-seed vs. setup race: an attacker could claim admin in the startup
window before LocalAdminSeedService runs, and the seed's insert would then be
swallowed (attacker credential persists, defeating env recovery). Fixed
structurally: the setup-claim endpoint is CLOSED (409) whenever
Auth:LocalAdmin:Password is configured — the env seed owns the credential, so
there's no claim to race (also strengthens the setup-claim TOFU posture).
Config.setupRequired reflects it.
- LOW — a concurrent setup race-loser now returns 409 (not 422); ClaimLocalAdmin's
DbUpdateException catch re-checks existence and rethrows genuine/transient DB
errors instead of masking them as "already configured".
- MEDIUM (accepted, documented) — two simultaneous authenticated password changes
are a non-serializable lost-update; accepted for a single-admin system
(self-healing via re-login, implausible timing).
+3 AuthController tests (env-seed closes setup / setupRequired gating). Full
ErsatzTV.Tests green (1506); no generated drift. Docs updated.
Refs #295
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Live E2E found that replaying a pre-logout cookie still authenticated (200, not
401): SignOutAsync only clears the CLIENT cookie, but the stateless encrypted
cookie ticket stays valid server-side because its security stamp is unchanged —
a captured cookie was replayable after logout until ticket expiry.
Fix: logout now rotates the local-admin security stamp (RotateLocalAdminSecurityStamp),
so every outstanding local session (old stamp) fails OnValidatePrincipal on its
next request. For the single admin this is "log out everywhere". Gated on an
authenticated local session so an unauthenticated caller can't force-revoke the
admin. OIDC sessions (no stamp) are unaffected; SignOutAsync still clears the
client cookie for UX.
+2 handler tests (rotate-when-configured / no-op-when-unconfigured). Auth suite
green (21). Docs: decisions.md note updated.
Refs #295
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Independent review (cold fork = MERGEABLE-WITH-NITS; Codex = BLOCKED, caught
concurrency defects the fork missed). All actionable findings folded in:
- HIGH (Codex) atomic first-claim-wins: ClaimLocalAdmin now writes the three
credential rows in ONE transaction guarded by the unique ConfigElement.Key
index (lost race -> DbUpdateException -> 409), so concurrent claims can't
produce a mixed-state credential.
- HIGH (Codex) consistent login snapshot: VerifyLocalAdminLogin reads hash+stamp
in one query and drops rehash-on-verify, so a login racing a password change
can't capture a stamp newer than the hash it verified (concurrent change ->
old password fails, or the issued cookie carries the pre-change stamp ->
revoked next request).
- MEDIUM (Codex) env-seed migration race: LocalAdminSeedService is now a RunOnce
BackgroundService that awaits SystemStartup.WaitForDatabase (the migrator is a
BackgroundService; registration order didn't guarantee the schema) + try/catch.
- MEDIUM (fork M1) ForwardedHeaders: reverted the strict-opt-in flip — it would
regress /iptv M3U/XMLTV/HLS absolute-URL generation (Request.Scheme) behind a
proxy without KnownProxies. Kept #285 behavior; KnownProxies still recommended.
- LOW (Codex/fork) require X-CSRF on /api/auth/logout + /password (the
[SkipApiAuthorization] surface isn't covered by the filter's CSRF check;
closes forced-logout CSRF).
- ChangeLocalAdminPassword also writes hash+stamp atomically. Input length caps
on username/password.
Deferred with a tracked gate: MEDIUM (Codex) side-effecting [RequiresAuthentication]
GETs (troubleshoot playback/archive) aren't CSRF-covered -> #301, gates PR2
(latent in PR1: the SPA still uses the machine key).
Verify: full ErsatzTV.Tests green (1501); no OpenAPI/generated drift. Docs updated
(api-conventions §9, decisions.md).
Refs #295#301
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Activating #253's `Version` as an `IsConcurrencyToken` made EF guard every
UPDATE *and DELETE* of a versioned root with `WHERE Version=@orig`, so any
writer outside the If-Match contract that saves via plain `SaveChangesAsync`
throws an unhandled `DbUpdateConcurrencyException`->500 when a replace-all
editor bumps the row in its narrow load->save window (ordinary two-tab UI).
A completeness sweep (grep every `Version` bumper + every root delete, not
just the handlers PR3's close note named) found 17 exposed writers, all now
routed through `ConcurrencyExtensions.SaveChangesForcingVersion` (Phase-1
force-write: adopt the stored token and retry; rethrow only on genuine
row-deletion):
- 9 versioned-root delete handlers (a delete has no ETag to rotate -> force
only, no bump)
- UpdateProgramScheduleHandler (bumps then saved plainly - the ProgramSchedule
case PR3 only suspected; its post-commit query/enqueue also moved to
CancellationToken.None per section 7b)
- 7 item add/remove bumpers PR2 left on plain save:
Add/DeleteProgramScheduleItem + Add{Items,Movie,Show,Season,Episode}ToPlaylist
Force-write (not 412) is correct: these endpoints take no If-Match, so an
unconditional delete/edit should win. No API contract change (no new response
codes) -> no OpenAPI regen.
Still deferred to #197 (cross-editor ETag rotation only, not a 500): the
non-bumping config siblings + the scanner-shared Add*ToCollection family.
Tests: RootWriterForceVersionTests races a bump *through the handler* via a
pre-tracked context (non-vacuous - reverting a handler to plain save fails the
test, verified) for the Option-delete / Either-delete / bump+update shapes,
plus the genuine-conflict rethrow branch and an explicit negative control
proving the plain-save path throws. Full ErsatzTV.Tests green (1482).
Also strips a pre-existing UTF-8 BOM from the touched handlers to satisfy the
.editorconfig `charset=utf-8` rule the pre-commit format hook enforces.
Docs: api-conventions section 7a (fan-out completeness) + decisions.md entry.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implements the ratified #295 design (PR1, server-only, backward compatible). The
/api surface now accepts a valid X-Api-Key (machine) OR an authenticated session
(browser cookie, local login or OIDC), gated by the evolved ApiAuthorizationFilter
(renamed from ApiKeyAuthorizationFilter; same fail-closed EndpointRequiresKey
predicate). Machine/key behavior is byte-identical and the SPA keeps working via
its stored key — the SPA login flow lands in PR2.
- ApiAuthorizationFilter: key-first (CSRF-immune) then session; session-authed
mutations require the X-CSRF header (403 otherwise). Attributes renamed
[RequiresApiKey]->[RequiresAuthentication], [SkipApiKeyAuthorization]->[SkipApiAuthorization].
- Cookie scheme ctv-session always registered (Lax/SameAsRequest/14d sliding, 401 not
redirect for /api); OIDC handler revived when configured (profile scope, userinfo,
auth-method claim); UseAuthentication/UseAuthorization/UseRateLimiter revived in the
legacy MapWhen branch.
- Local admin = single credential in ConfigElement rows (username / PBKDF2 hash via
Microsoft.Extensions.Identity.Core / rotating security stamp) — NO DB migration.
Password change rotates the stamp; CookieSecurityStampValidator revokes stale local
sessions. Env-seed recovery (Auth:LocalAdmin:*) via LocalAdminSeedService.
- AuthController /api/auth/{config,session,setup,login,logout,password} + browser-nav
GET /auth/oidc/login; excluded from OpenAPI (machine-audience spec). Per-IP rate limit
on login/setup/password; dummy-hash verify (no user enumeration).
- ForwardedHeaders now strict opt-in: X-Forwarded-* ignored unless KnownProxies/Networks
configured (rate-limiter IP + cookie-Secure integrity). Deployment: operators behind a
proxy must set ForwardedHeaders:KnownProxies.
- Tests: session/CSRF filter cases + 17 Application/Auth handler tests; full ErsatzTV.Tests
green (1499). No OpenAPI/generated-artifact drift.
- Docs: api-conventions section 9 rewritten; decisions.md entry (supersedes #206 inert-OIDC note).
Refs #295#197#206#58
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add GET /api/media-sources/collections-scan-status (MediaSourcesController →
GetCollectionsScanStatus handler) reporting which media-source families
(plex/jellyfin/emby) currently hold their external-collections scan lock,
reading IEntityLocker.Are{X}CollectionsLocked(). The lock is family-global
(no source id) and boolean (no percent), so the DTO carries just {family} and
returns only active families — the counterpart to GET /api/libraries/scan-status.
SPA: useCollectionsScan now polls this endpoint and reconciles optimistic
pending against the active-family set (seeding on mount so an in-progress scan
disables buttons immediately), using the same grace-tick helper as library
scans (now generic over the pending key type). Drops COLLECTIONS_PENDING_TIMEOUT_MS
— a long deep scan no longer re-enables the button early, and a fast scan no
longer wedges it disabled for the full timeout. A row shows Scanning when its
family is active or it has an in-grace optimistic pending key.
Tests: handler (3), controller route+delegation (2), SPA api fn + hook reconcile
(mount-seed / 202-promote / 409-keeps-disabled / 404-error). OpenAPI + TS types
regenerated. Docs: api-conventions §3b, blazor-route-parity §5, decisions.md.
Unblocks #91b (arc item 4): Libraries.razor's collections-scan affordance now
has full authoritative parity.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mint ChannelDetailResponseModel (faithful detail DTO exposing the raw editable
field set the channel editor reads: raw FFmpegProfileId/WatermarkId/FallbackFillerId
ids, the mode enums, logo, playoutCount, id) and route GetById/Create/Update through
it, replacing the lean list ChannelResponseModel that resolved the profile to a name
and dropped the editable ids (a functional regression for draftFromChannel). The lean
ChannelResponseModel stays unchanged for GET /api/channels. webEncodedName dropped
(SPA never reads it). Logo is mirrored as a Core ChannelLogoResponseModel since the
Application ArtworkContentTypeModel can't be referenced from Core.
Repoint the hand-written SPA client aliases now that the VMs are gone from the schema:
Channel -> ChannelDetailResponseModel, MediaCollection/SmartCollection -> *ResponseModel,
ProgramSchedule -> ProgramScheduleResponseModel. Fix#288 honest-nullability test fallout
in search.test.ts (null -> [] for now-non-null id arrays). Include the already-on-disk
playouts.ts WithDayNames removal and regenerate v1.json + v1.d.ts + endpoint-index.md
(authoritative final regen; the reset endpoint's {channelNumber}->{id} re-key surfaces
in the generated docs and the OpenApi error-contract test).
Refs #288#197
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GetById/Create/Update return ChannelResponseModel via new GetChannelByIdForApi
read-side query; POST /api/channels/{id:int}/playout/reset (new
GetPlayoutIdByChannelId; by-number kept for HlsSessionWorker broadcast).
Refs #288#197
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
24 #nullable enable flips across ErsatzTV.Core/Api; Collection/Schedule/
SmartCollection/Resolution VMs wrapped in ResponseModels (Version now
header-only, SPA-verified); pageNum threaded into GET /api/search.
Refs #288#197
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cold-review LOW (defense-in-depth): the serve path derived the Content-Type
from the stored file via Winista but only defaulted application/octet-stream on
a NULL sniff. A cache file whose bytes are HTML — a legacy entry poisoned before
the upload-sniff landed, or a hypothetical image/script polyglot — could still be
sniffed as text/html and served renderable (nosniff does not stop an explicitly
declared text/html). Clamp the sniffed type to ImageContentTypes.IsAccepted,
serving application/octet-stream for anything else, so the serve path can never
emit a renderable non-image type regardless of what bytes are on disk.
Refs #283
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
S4 stored-XSS + S9 upload-size DoS from the #197 cold API review.
The artwork path trusted client-supplied content types at both ends: upload
validated only the declared multipart Content-Type (never decoded the bytes),
and serving reflected a client `?contentType=` straight into the response
Content-Type on unauthenticated GET sinks (/iptv/logos, /artwork/watermarks).
Chain: upload <script> bytes as image/png -> GET ...?contentType=text/html
serves them as HTML in-origin. nosniff (#279) does not help because the server
explicitly declares text/html.
- Upload: derive the content type from the bytes via SkiaSharp SKCodec
(header-only, no decode -> no decompression-bomb path); reject non-images 422.
New ErsatzTV.Core/Images/ImageContentTypes as the single allow-list source.
Dropped the untrusted declared Content-Type from the UploadArtwork command.
- Serve: removed the ?contentType= reflection structurally -- dropped ContentType
from GetCachedImagePath and the [FromQuery] binding on GetImage/GetWatermark;
the handler always sniffs the file, defaulting application/octet-stream.
ArtworkContentTypeModel.UrlWithContentType is now the bare path; SPA previews
no longer append the query.
- Defense-in-depth: channel-logo / watermark {path, contentType} DTOs run through
ArtworkContentTypeModel.Sanitized(), blanking non-allow-listed types on write.
- S9: Kestrel MaxRequestBodySize from ETV_MAXIMUM_UPLOAD_MB rejects oversized
bodies during read (controller file.Length check kept as friendly-error backstop).
Both serve sinks are IgnoreApi, so no OpenAPI change. Tests: byte-sniff accept/
reject, Sanitized() allow-list, Location no longer carries ?contentType=.
Docs: api-conventions §4a + decisions.md 2026-07-12.
Refs #283#197#66
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add optional `int? Id` to ScheduleItemRequest/ReplaceProgramScheduleItem so
a client can round-trip each existing item's server id. When ids are present,
ReplaceProgramScheduleItemsHandler reconciles by id (not array position), so an
item's persisted fill-group/shuffle state (PlayoutScheduleItemFillGroupIndex,
FK OnDelete Cascade) follows the logical item across reorders/inserts instead of
being inherited by whatever previously occupied its new slot (#259, split from
#252/#253). A fully id-less payload keeps the verbatim positional fallback.
Guards (inside PersistItems, after CheckVersion so 412 precedes 422): duplicate
id -> 422; id not in this schedule -> 422 (a stale id under Phase-1 force-write is
a live lost-update signal, not a new item). Index stays array-position derived.
Regenerated v1.json + TS client.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adversarial review caught a HIGH the fan-out introduced: activating Version as an
IsConcurrencyToken on Playout/Collection makes EF append `WHERE Version=@orig` to
EVERY root UPDATE, so a non-If-Match writer that saves via plain SaveChangesAsync
now throws DbUpdateConcurrencyException → 500 when a replace-all editor bumps the
row between its load and save. Realistic two-tab trigger (edit playout settings while
editing its alt-schedules; edit a collection's name while reordering) — a new crash,
previously silent last-write-wins.
Fix: shared ConcurrencyExtensions.SaveChangesForcingVersion — on a concurrency
failure it adopts the stored token as original+current (client-wins merge scoped to
the token, never reverting the concurrent bump) and retries, i.e. Phase-1 force-write
semantics for a missing If-Match. Applied to the exposed UPDATE writers:
UpdatePlayout, Update{Sequential,Scripted,ExternalJson}Playout, UpdateOnDemandCheckpoint,
UpdateCollection. Non-vacuous test proves the write lands and the bump survives.
Deletes + repo-mediated Add* writers (rarer / join-rows-only) re-scoped onto #269.
Refs #253#269
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The new POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections
endpoints acquire a per-provider collections lock (409 if held) and hand the
single release to the ScannerService finally. But SchedulerService's periodic
collection scans were enqueued WITHOUT the lock, and ScannerService's finally
released the collections lock whenever held with no ownership check. A
scheduler-queued scan running while an API request held the lock cross-released
the API's lock (#250 bug class), letting a second API request get a spurious
202 instead of 409.
Fix (mirrors the SynchronizePlexLibraryByIdIfNeeded(Unlock: !networksFollow)
library-scan precedent):
- Add `bool Unlock = true` (4th positional param) to the three
Synchronize{Plex,Jellyfin,Emby}Collections records; default keeps the
controller + Libraries.razor call sites compiling and releasing on run.
- ScannerService: the three collection finallys now honor `request.Unlock`
(the concrete typed request is in scope in each method) so a batch member
with Unlock:false never releases a lock it doesn't own.
- SchedulerService: replace the unlocked per-source enqueue with a lock-once
per-provider batch — LockX Collections() once, enqueue each source with
Unlock:isLast (last message owns the release), compensating unlock in catch,
and SKIP the whole provider loop if the lock is already held. A naive
"lock-per-source, skip if held" would deterministically starve the 2nd+
source; lock-once-batch does not.
Tests (ErsatzTV.Tests/Services/): ScannerServiceCollectionLockTests drives the
real ScannerService read loop + real EntityLocker and asserts Unlock:false
leaves a held lock intact while Unlock:true releases (all three providers);
SchedulerServiceCollectionLockTests reflect-invokes ScanPlexMediaSources and
asserts it locks once + skips the enqueue when held, and hands the release to
the last message when acquired. Proven non-vacuous: reverting the Plex fix
fails exactly the three Plex tests.
No OpenAPI/v1.json change (internal channel-message record, not a DTO).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wire the frozen ETag/If-Match/412 recipe (Block reference implementation)
onto the Template and DecoTemplate aggregates:
- ReplaceTemplateItems / ReplaceDecoTemplateItems commands gain
Option<int> ExpectedVersion; ToCommand() on the request DTOs threads it
through from If-Match.
- Handlers introduce the version check as a standalone Either after
validation (never via Apply), bump Version unconditionally before
saving, and persist through SaveChangesWithConcurrencyGuard so a losing
writer maps to 412 instead of 500. DecoTemplate's post-commit playout
Reset enqueue now only runs after a successful save.
- TemplateViewModel / DecoTemplateViewModel carry Version (header-only,
not echoed in the response body), populated in Mapper.
- TemplateController / DecoTemplateController: GET items emits a strong
ETag of the root's version; PUT parses If-Match (400 on malformed),
threads the expected version into the command, and returns the new
ETag from the refreshed root on success. Both PUT actions now use the
handler's returned item list directly instead of re-querying items.
- SPA: templates.ts / decoTemplates.ts gain getXItemsWithMeta and an
If-Match-aware replaceX; TemplateEditor / DecoTemplateEditor hold the
ETag in a ref, read items-with-meta first on load, and open a
"changed elsewhere" ConfirmDialog on a 412 instead of navigating away.
Tests: new ReplaceTemplateItemsHandlerConcurrencyTests /
ReplaceDecoTemplateItemsHandlerConcurrencyTests mirror the Block
concurrency contract tests (stale/matching/absent If-Match, no-op bump,
racing-save 412, non-vacuous backstop). TemplateControllerTests /
DecoTemplateControllerTests gain ETag/If-Match/412 coverage.
TemplatesScreen.test.tsx / DecoTemplatesScreen.test.tsx gain a 412
conflict-dialog test mirroring BlocksScreen's.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fans the frozen ETag/If-Match/412 recipe (Block reference, #253) onto the
Playlist aggregate:
- ReplacePlaylistItems command carries ExpectedVersion; the handler runs
CheckVersion as a standalone Either after validation (so a stale write
survives as 412, not flattened to 422 by Apply/Join), bumps Version
unconditionally before saving, and persists via
SaveChangesWithConcurrencyGuard (EF concurrency-token backstop).
- PlaylistViewModel carries Version; the items GET sets a strong ETag and
the PUT parses If-Match, threads it into the command, and returns the
refreshed ETag on success (400 on a malformed If-Match).
- Sibling item-adding handlers (AddItemsToPlaylist, AddMovie/Episode/
Season/ShowToPlaylist) bump Version too, since they mutate the same
editor-visible item list.
- SPA: playlists.ts exposes getPlaylistItemsWithMeta and an
If-Match-aware updatePlaylist; PlaylistEditor holds the ETag in a ref,
round-trips it on save, and opens a "changed elsewhere" ConfirmDialog on
412 (mirrors BlockEditor).
Tests: new ReplacePlaylistItemsHandlerConcurrencyTests (stale/match/
force-write/no-op-bump/racing-save), new PlaylistController tests
(ETag on GET items, 400/412/thread-version/force-write on PUT), and a
vitest 412-conflict-dialog test for PlaylistsScreen. dotnet test:
1304/1304 green. web: npm run typecheck clean, npm run build clean,
vitest 664/664 green.
Ref #253 PR2.
Closes the two F9 Libraries.razor parity gaps and normalizes scan-show error
mapping to ProblemDetails.
TASK 1 — library-wide deep scan:
- QueueLibraryScanByLibraryId gains optional `bool DeepScan = false`; handler
threads it into ForceSynchronize{Plex,Jellyfin,Emby}LibraryById.
- POST /api/libraries/{id}/scan?deep=false binds it via [FromQuery].
TASK 2 — external-collections scan (new endpoints):
- POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=false
acquires the per-source collections lock (§3b: lock IS the running scan → 409),
enqueues Synchronize{X}Collections(id, ForceScan:true, deep) to the scanner
channel, returns 202; compensating-unlock on enqueue throw.
TASK 3 — scan-show normalization:
- New QueueShowScanResult enum; handler returns it instead of bool.
- POST /api/libraries/{id}/scan-show now maps 202/404/409/422 (all errors
ProblemDetails) instead of 200/404/400-anonymous-object.
- Updated the lone Blazor caller (TelevisionSeasonList.razor).
Tests: LibrariesController (scan deep=true, scan-show enum→status), the three
media-source controllers (scan-collections route/404/409/202/compensating-unlock),
and handler tests for both changed handlers (deep threading + show-scan outcomes).
Docs: api-conventions §3b exemplar + blazor-route-parity §5 F9 gate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Slice C of the async-op contract normalization:
- channel reset (POST /api/channels/{channelNumber}/playout/reset) now
returns 202 Accepted (was 200 Ok) — it only queues a background rebuild
- reset-all (POST /api/playouts/reset-all) still 202 but now returns a
ResetAllPlayoutsResponseModel body reporting QueuedPlayoutIds /
SkippedLocked / SkippedUnsupported instead of silently swallowing skips;
handler returns a new ResetAllPlayoutsResult record
- single-playout GET (GET /api/playouts/{id}) now exposes IsLocked on
PlayoutResponseModel, set from IEntityLocker.IsPlayoutLocked mirroring
the list projection — gives a polling client the lock flag
Tests: channel reset asserts 202; reset-all asserts 202 + skipped-body
shape; single GET asserts IsLocked; new ResetAllPlayoutsHandlerTests
(in-memory SQLite) asserts locked/ExternalJson/None land in skipped lists
and eligible playouts in queued. docs/api-conventions.md §3a updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Independent adversarial review of PR #266 found two sibling handlers with the
identical post-commit `_smartCollectionCache.Refresh(cancellationToken)` pattern
that the sweep missed (only UpdateSmartCollection was caught). Same audit#22 F4
class: a late client-disconnect after the commit lands would throw and leave the
in-memory smart-collection cache stale vs the committed DB. → CancellationToken.None.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Backend mutation-hardening cluster from the 2026-07-11 mutation-safety audit
sweep (adversarial-reviewer #22/#23), the parallel-safe backend-isolated slice.
audit#22 F4 — standardize post-commit enqueues on CancellationToken.None:
20 command handlers under MediaCollections/, ProgramSchedules/, Playouts/,
Channels/ threaded the request cancellationToken into work that runs AFTER
SaveChangesAsync commits (WriteAsync rebuild/refresh enqueues, mediator.Publish,
reindex, cache Refresh, and post-commit lookups that gate an enqueue). A late
client-disconnect then turns an already-durable commit into a thrown request AND
drops the side effect. Generalizes the #251 deco-handler fix. Excludes
BuildPlayoutHandler (worker/background token, not a client-disconnect token),
the config/FFmpeg multi-upsert handlers (partial-commit case, separate
follow-up), and response-projection reloads (correctly keep the request token).
audit#22 F2 — DeleteChannelHandler/DeletePlayoutHandler now delete the channel
guide {number}.xml through IFileSystem.File.Delete (observable under
MockFileSystem) and BEFORE the commit (a post-commit delete orphans the xml on a
crash; the xml is regenerable on demand, so pre-commit delete is the safe order).
audit#23 F4 — ReplacePlayoutAlternateScheduleItemsHandler rejects an empty item
list in the handler (not only the controller pre-guard) so a direct caller can't
trip the Max()-on-empty crash.
Docs: api-conventions.md §7a (post-commit token convention + boundaries),
decisions.md entry (rationale, sweep scope, #253 PR2-4 coordination note).
Tests: guide-cache-delete-through-FS for both delete handlers, empty-list guard.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the shared optimistic-concurrency contract so a stale second tab can no longer
silently overwrite a fresher edit. PR1 lands the infra + the Block reference aggregate;
PRs 2–4 fan the same recipe across the other 8 roots (design: #253#issuecomment-8472).
Contract
- `IVersionedAggregate` (`int Version`) on all 9 replace-all roots (ProgramSchedule,
Block, Template, DecoTemplate, Playlist, Collection, Playout, MultiCollection,
RerunCollection), EF-mapped `.IsConcurrencyToken()`; one dual-provider migration
`AddAggregateVersions` (nullable:false, default 0).
- Strong `ETag` of `Version` on the aggregate GET; `If-Match` on the PUT; mismatch →
412 (distinct from the §3a 409 build-lock guard). Successful PUT returns the new ETag.
- `PreconditionFailedError : BaseError` → 412 in `ApiResults.ToErrorResult`;
`ConcurrencyHeaders.ParseIfMatch/SetETag`; malformed If-Match → 400; `*`/absent =
Phase-1 force-write.
Block reference wiring
- Handler: standalone `Either` via `CheckVersion` AFTER validation (never through
`Apply`, which Join()-flattens the subtype to 422), unconditional `Version++`,
`SaveChangesWithConcurrencyGuard` backstop (DbUpdateConcurrencyException → 412).
- `BlockViewModel.Version` (header-only, not echoed in the body); controller sets the
ETag on GET items and on the successful PUT.
- SPA: `client.requestWithMeta` seam; `blocks.getBlockItemsWithMeta` + `replaceBlock`
If-Match/ETag round-trip; `BlockEditor` holds the ETag, sends If-Match, and on 412
opens a blocking "changed elsewhere — reload" dialog.
Tests
- Handler contract tests: stale-If-Match → 412 (no mutation), matching/absent → success
+ bump, no-op save still bumps, and a two-context racing save → 412; proven
non-vacuous (drop `.IsConcurrencyToken()` → the race test fails).
- Controller tests: malformed If-Match → 400, If-Match threaded to the command, ETag on
GET/PUT, 412 passthrough. SPA: requestWithMeta ETag, replaceBlock If-Match, 412 dialog.
Docs: api-conventions §7a, spa-conventions §4a, domain-model glossary, decisions log.
Refs #253
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
New JellyfinMediaSourcesController (/api/media-sources/jellyfin, J1-J9) and
EmbyMediaSourcesController (/api/media-sources/emby, E1-E9), wrapping the
existing Jellyfin/Emby MediatR commands per the #202 design doc §A.3/§A.4.
Secure connection contract (§C3/§B, finding 1): the connection GET returns
only { address, hasApiKey } — the API key never crosses the wire. The PUT
retains the existing key when the incoming key is blank, sets a new one when
non-blank, and 422s "API key is required" on a blank first connect.
Finding 7 (lock-release discipline): DisconnectJellyfinHandler and
DisconnectEmbyHandler now wrap their work in try/finally so a throw from any
awaited dependency (repo delete, search-index commit, secret store) still
releases the family lock instead of wedging every future disconnect at 409.
Findings 2c/8 (path-replacement cross-source guard): UpdateJellyfinPathReplacementsHandler
and UpdateEmbyPathReplacementsHandler now reject, before any write, an incoming
positive Id that isn't owned by the route's media source, a null item, or a
blank RemotePath/LocalPath — all 422 with no partial mutation. Defense-in-depth
repo fix: the Jellyfin/Emby path-replacement UPDATE SQL in MediaSourceRepository
now scopes by {Jellyfin,Emby}MediaSourceId (was previously unscoped by Id alone,
allowing a PUT to one source to silently overwrite another source's row). The
Plex path-replacement method (~line 397) is untouched — that's slice S2's file.
Library preferences (§C4a): the controller validates the incoming id set
against the source's known libraries (reject foreign ids, require full
coverage, no Id=0) before dispatch, then — for §C7 — LockLibrary + enqueues
the SynchronizeXLibraries/SynchronizeXLibraryByIdIfNeeded pair per enabled
library (compensating unlock if the enqueue throws), and returns the reloaded
list (ids are not stable across a disable).
404s on id-taking endpoints come from a controller pre-check (GetXMediaSourceById
is None), not a handler NotFoundError, since Either.Apply/ToEitherAsync join any
NotFoundError into a flat 422 (finding 9).
Tests: controller route/404/409/422 tests for both families; disconnect
fault-injection tests proving the lock releases even when a dependency throws;
path-replacement handler tests for cross-source-id/blank/null-item rejection
and correct add/update/delete merge; a repository-level test proving the SQL
fix stops a same-family cross-source path-replacement overwrite.
No new commands, no DB migration, no OpenAPI regen (gated until S1-S3 merge
per the design doc's build-slice plan).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
timothyandCodex