Build ErsatzTV Image / Docs update reminder (push) Has been skipped
Build ErsatzTV Image / decisions.md append-only (push) Has been skipped
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Has been skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Has been skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Has been cancelled
Build ErsatzTV Image / Build & test (.NET) (push) Has been cancelled
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Has been cancelled
Build ErsatzTV Image / Functional E2E (curl contracts) (push) Has been cancelled
Consolidate docs/decisions.md (1923 -> 1028) by extracting four cohesive topic clusters into docs/decisions/ (optimistic-concurrency, api-auth-security, release-ci-governance, spa-modularization) — content relocated verbatim (lossless; all rationale + reversals preserved), main Index rebuilt to reference the topic files plus the remaining in-file entries, docs/README.md points back at the decisions Index. Add the v26.9.0 row to the ci-cd.md version table. refs #340 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
393 lines
33 KiB
Markdown
393 lines
33 KiB
Markdown
# API auth & security posture (#197, #206, #279, #283, #292, #295, #301, #319, #330)
|
||
|
||
Why ErsatzTV's REST/SPA surface is gated the way it is: the #197 cold-review remediation and
|
||
its bundles, the Blazor-removal auth sign-off, the artwork stored-XSS fix, fail-closed API
|
||
auth, browser-session auth + CSRF, the side-effecting-GET POST-ification, and the response
|
||
security headers (CSP/Permissions-Policy/CORP). Rationale relocated from the append-only
|
||
`docs/decisions.md` at the v26.9.0 consolidation; enforcement/mechanics cross-link to
|
||
`docs/api-conventions.md` §5/§9 and `docs/spa-conventions.md` §5e.
|
||
|
||
Issue trail: #197 (Phase-0 headers/PR #279; Bundle A fail-closed/PR #292; Bundle C
|
||
contract-freeze), #206 (Blazor-removal auth posture), #283 (artwork content-type), #295
|
||
(PR1 server session auth + PR2 SPA cutover), #301 (side-effecting GETs), #319 (CSP), #330 (CORP).
|
||
|
||
## Contents
|
||
|
||
- [2026-07-11 — Blazor removal auth posture: no new exposure beyond phase (a); real auth deferred to #197 (#206)](#2026-07-11--blazor-removal-auth-posture-no-new-exposure-beyond-phase-a-real-auth-deferred-to-197-206)
|
||
- [2026-07-11 — Baseline security response headers + Phase-0 API hardening (#197, PR #279)](#2026-07-11--baseline-security-response-headers--phase-0-api-hardening-197-pr-279)
|
||
- [2026-07-12 — Artwork content-type is sniffed, never reflected (#283, S4/S9 stored XSS)](#2026-07-12--artwork-content-type-is-sniffed-never-reflected-283-s4s9-stored-xss)
|
||
- [2026-07-12 — Fail-closed API auth + sensitive-read tier + CORS/ForwardedHeaders lockdown (#197 Bundle A, PR #292)](#2026-07-12--fail-closed-api-auth--sensitive-read-tier--corsforwardedheaders-lockdown-197-bundle-a-pr-292)
|
||
- [2026-07-12 (#197 Bundle C — contract-freeze honesty)](#2026-07-12-197-bundle-c--contract-freeze-honesty)
|
||
- [2026-07-12 — Browser SPA session auth: `/api` accepts session OR machine key (#295 PR1, server-only)](#2026-07-12--browser-spa-session-auth-api-accepts-session-or-machine-key-295-pr1-server-only)
|
||
- [2026-07-12 — #295 PR2: SPA session cutover + #301 side-effecting-GET POST-ification](#2026-07-12--295-pr2-spa-session-cutover--301-side-effecting-get-post-ification)
|
||
- [2026-07-12 — Enforcing CSP + Permissions-Policy on the host (#319, ZAP baseline)](#2026-07-12--enforcing-csp--permissions-policy-on-the-host-319-zap-baseline)
|
||
- [2026-07-13 — Cross-origin resource policy: `same-origin` on every response (#330)](#2026-07-13--cross-origin-resource-policy-same-origin-on-every-response-330)
|
||
|
||
---
|
||
|
||
## 2026-07-11 — Blazor removal auth posture: no new exposure beyond phase (a); real auth deferred to #197 (#206)
|
||
|
||
Sign-off for the #91 phase (b) removal-gate item #206 ("deleting the last challenged Blazor page leaves
|
||
only the open SPA"). The actual authorization wiring in `ErsatzTV/Startup.cs` + `ErsatzTV/Pages` was
|
||
enumerated in code (not assumed) before clearing the gate.
|
||
|
||
**What is gated today**
|
||
- **OIDC** (`OidcHelper.IsEnabled` — active only when `Authority`/`ClientId`/`ClientSecret` are configured):
|
||
`AddAuthentication` (cookie default, `oidc` challenge) + `AddAuthorization` `DefaultPolicy =
|
||
RequireAuthenticatedUser` + **`AddRazorPages(… AuthorizeFolder("/"))`** (Startup.cs:379-385) +
|
||
`blazor.UseAuthentication()/UseAuthorization()` inside the Blazor `MapWhen` branch (Startup.cs:764-770).
|
||
`AuthorizeFolder("/")` gates **Razor Pages only**, and the sole user-facing Razor Page is
|
||
`Pages/_Host.cshtml` — the Blazor Server host (the other `.cshtml`, `Shared/_Favicons.cshtml`, is a
|
||
cosmetic partial). **So the OIDC challenge protects exactly the Blazor UI and nothing else.**
|
||
- **`/app` (SPA)** is served by its own `MapWhen(path=/app)` static-file branch (Startup.cs:701-714) with
|
||
**no authentication/authorization middleware** — open since phase (a) (`/`→`/app`, PR #148).
|
||
- **`/api/*` controllers** carry no `[Authorize]` (verified: zero attributes in `Controllers/`); the
|
||
Razor-Pages `AuthorizeFolder`/`DefaultPolicy` never reach them. Their only optional gate is the
|
||
per-endpoint `ApiKeyAuthorizationFilter` (API-key on mutating JSON endpoints), independent of OIDC/Blazor.
|
||
- **`/iptv/*`** is gated by `ConditionalIptvAuthorizeFilter` (JWT `JwtOnlyScheme`, active only when
|
||
`JwtHelper.IsEnabled`) in its own `MapWhen` branch (Startup.cs:797-803) — independent of Blazor.
|
||
|
||
**Posture after Blazor removal.** Removing `Pages/_Host.cshtml`, `AddRazorPages`/`AuthorizeFolder("/")`,
|
||
`blazor.UseAuthentication/UseAuthorization`, `MapBlazorHub`, and `MapFallbackToPage("/_Host")` deletes the
|
||
OIDC challenge's **only attachment point** — no user-facing surface remains challenged. **No capability is
|
||
lost:** every Blazor-served capability already has an open SPA equivalent (the #91 parity effort), and the
|
||
SPA was already the unauthenticated path since phase (a), so removal exposes nothing a user could not already
|
||
reach via `/app`.
|
||
|
||
**The one honest caveat (not a regression introduced by removal):** an OIDC-configured operator's *Blazor*
|
||
admin UI sits behind a login today; after removal there is no login-gated admin UI at all (the SPA admin UI
|
||
is open). That exposure delta already happened at **phase (a)** (the open SPA became the default admin
|
||
surface); removal only deletes the now-redundant challenged duplicate. Designing real SPA/API authentication
|
||
is deliberately deferred to **#197** (cold API security review — a HARD GATE before any remote exposure).
|
||
|
||
**Removal-PR must-not-break (independent gates that survive):** `ConditionalIptvAuthorizeFilter` (`/iptv/*`
|
||
JWT), `ApiKeyAuthorizationFilter` (mutating `/api/*`), and `JwtHelper` access_token query support. **Leave
|
||
the OIDC service registrations in place** (conditional on config, inert once no Razor Page consumes them) —
|
||
ripping OIDC out is a #197 decision, not a removal-PR one. The removal PR removes only the Blazor-attached
|
||
pieces above; `MapControllers()` + `/docs` (Scalar), currently co-hosted in the Blazor `MapWhen` branch, must
|
||
survive the surgical reduction.
|
||
|
||
## 2026-07-11 — Baseline security response headers + Phase-0 API hardening (#197, PR #279)
|
||
|
||
Phase-0 of the #197 remediation — the posture-**independent** safe subset, shipped ahead of the
|
||
fail-closed/CORS/versioning posture work tracked in #280–#289.
|
||
|
||
- **Baseline security headers on every response.** New `ErsatzTV/Middleware/SecurityHeadersMiddleware`,
|
||
registered **first** in the pipeline (before the `/iptv` `MapWhen` branch and `UseCors`), so it covers
|
||
`/api`, `/iptv`, `/artwork`, static, the SPA fallback, and filter-produced 4xx alike — which is why it's
|
||
middleware, not an MVC filter. It sets `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY`, and
|
||
`Referrer-Policy: strict-origin-when-cross-origin`. `nosniff` is the standing backstop for the artwork
|
||
content-type MIME-sniffing risk (#283). **CSP and HSTS are deliberately NOT included here**: CSP must be
|
||
validated against the ChicoryTV SPA's inline assets, and HSTS is a proxy/TLS-termination decision — both
|
||
belong to the #197 posture design (#284/roadmap), not this baseline. Headers are set eagerly (not via
|
||
`Response.OnStarting`); safe today because the pipeline has no `UseExceptionHandler`/`UseStatusCodePages`
|
||
that would `Response.Clear()` — switch to `OnStarting` if one is ever added.
|
||
- **Constant-time API-key comparison.** `ApiKeyAuthorizationFilter` compares `X-Api-Key` with
|
||
`CryptographicOperations.FixedTimeEquals` (over UTF-8 bytes) instead of ordinal `string.Equals`, removing
|
||
the response-timing oracle on the write key. Accept/reject behavior is otherwise identical.
|
||
- **Playout pagination clamped.** `GET /api/playouts` and `GET /api/playouts/{id}/items` now clamp
|
||
`Math.Clamp(pageSize, 1, 100)` + `Math.Max(0, pageNum)` before the query — applying the api-conventions §1
|
||
clamp convention the other paged endpoints already follow (these two were passing the raw client value
|
||
straight to EF `Take()`).
|
||
|
||
The larger #197 posture (fail-closed writes, sensitive-read auth tier, CORS lockdown, `/api/v1` versioning,
|
||
the OpenAPI security scheme) is decomposed into #280–#289 with the phased roadmap on #197; those PRs will
|
||
append their own decisions here as they land.
|
||
|
||
## 2026-07-12 — Artwork content-type is sniffed, never reflected (#283, S4/S9 stored XSS)
|
||
|
||
The artwork upload/serve path trusted client-supplied content types at both ends, giving a stored-XSS
|
||
chain on **unauthenticated** GET sinks: upload `<script>` bytes declared `image/png` →
|
||
`GET /iptv/logos/{hash}?contentType=text/html` served them as HTML in the ErsatzTV origin. The #279
|
||
`nosniff` header is not a fix here — the server was *explicitly declaring* `text/html`, which the browser
|
||
honors regardless of `nosniff`. The trust was the bug; the fix removes it at both ends.
|
||
|
||
- **Upload derives the content type from the bytes, never the declared value.** `UploadArtworkHandler`
|
||
buffers the (size-bounded) upload and calls `ErsatzTV.Core/Images/ImageContentTypes.DetectContentType`,
|
||
which uses SkiaSharp's `SKCodec` to identify the format from the image header only — pixels are **not**
|
||
decoded, so this can't be turned into a decompression-bomb vector. A payload that isn't one of the
|
||
accepted raster formats (png/jpeg/gif/webp) is rejected 422; the declared `Content-Type` is no longer
|
||
read at all (the field was dropped from the `UploadArtwork` command).
|
||
- **Serve sniffs the stored file; the `?contentType=` reflection is gone.** `GetCachedImagePath` no longer
|
||
carries a `ContentType`, and `GetImage` (`/iptv/logos`) / `GetWatermark` (`/artwork/watermarks`) dropped
|
||
their `[FromQuery] contentType` binding. `GetCachedImagePathHandler` always derives the MIME type from the
|
||
file (`MimeTypes.GetMimeTypeFromFile`) and **clamps it to the image allow-list** (`ImageContentTypes.IsAccepted`),
|
||
serving `application/octet-stream` for anything else — so a file whose bytes are not an accepted image (a
|
||
legacy cache entry poisoned before the upload sniff landed, or a hypothetical polyglot) is a non-renderable
|
||
download, never HTML/script. The removal is **structural** — there is no longer any request path that lets a
|
||
client choose the served `Content-Type`. `ArtworkContentTypeModel.UrlWithContentType` now returns the bare path, and the SPA
|
||
watermark/logo previews no longer append the query.
|
||
- **Defense-in-depth on the persisted JSON DTOs.** The `{path, contentType}` bodies (channel logo, watermark)
|
||
run their content type through `ArtworkContentTypeModel.Sanitized()`, which blanks anything outside the
|
||
image allow-list before it is stored — so a stale/hostile value can't be reflected by any future code path
|
||
even though the serve route already ignores it.
|
||
- **S9 upload-size DoS.** Kestrel `Limits.MaxRequestBodySize` is now set from `ETV_MAXIMUM_UPLOAD_MB`, so an
|
||
oversized body is rejected as it is read rather than only after the controller's post-binding `file.Length`
|
||
check (kept as the friendly-error backstop). This is a global bound; the app has no other large inbound
|
||
body (streaming is outbound GET).
|
||
|
||
`ImageContentTypes` is the single source of truth for the accepted image types (the allow-list previously
|
||
duplicated in `UploadArtworkHandler`). Both serve sinks are `[ApiExplorerSettings(IgnoreApi = true)]`, so
|
||
none of this changes the OpenAPI document.
|
||
|
||
## 2026-07-12 — Fail-closed API auth + sensitive-read tier + CORS/ForwardedHeaders lockdown (#197 Bundle A, PR #292)
|
||
|
||
Phase-1 of the #197 remediation — the auth posture that must land before any remote exposure.
|
||
Owner decisions (confirmed this session): **single API key** (no read/write split), and
|
||
**`Api:RequireKeyForReads` defaults `true`** (the whole `/api` surface requires the key). This does
|
||
**not** affect Jellyfin/streaming: `/iptv/*` (playlist/guide/streams/logos) and `/artwork/*` are outside
|
||
the filter's `/api` scope and keep their own optional access-token; only the management API the SPA talks
|
||
to is gated.
|
||
|
||
- **Fail-closed writes (#280, S1).** The empty-key "open" branch is deleted; there is no open mode. New
|
||
`IApiKeyProvider` (`ErsatzTV/Services/ApiKeyProvider.cs`, singleton, resolved once at startup) yields a
|
||
never-empty key: `Api:WriteKey` if set, else a key persisted at `FileSystemLayout.ApiKeyPath`
|
||
(`/config/api.key`, `0600`, path logged not value), else a generated 256-bit hex key. Every mutating
|
||
`/api` request now requires `X-Api-Key`.
|
||
- **Sensitive-read tier (#282, S3/S5).** Reads are gated by `Api:RequireKeyForReads` (default true) OR a
|
||
new `[RequiresApiKey]` marker (mirror of `[SkipApiKeyAuthorization]`) applied to
|
||
`Troubleshoot`/`Logs`/`Settings`/`Maintenance`, so that tier stays gated even if an operator opts reads
|
||
open. `OPTIONS` preflight is exempt (CORS middleware owns it). `ApiControllerSecurityTests` asserts the
|
||
tier reflectively.
|
||
- **Delete dead non-`/api` mutation surfaces (#281, S2).** `SortController`
|
||
(`POST media/collections/{id}/items`, dead Blazor SortableJS residue — the SPA uses
|
||
`PUT /api/collections/{id}/custom-order`) and `AccountController` (`POST account/logout`, dead OIDC)
|
||
bypassed the key because they sat outside `/api`. Removed rather than guarded.
|
||
- **CORS opt-in (#284, S6).** `AllowAnyOrigin/Method/Header` is replaced by the `ApiCors` policy: an
|
||
exact-origin allowlist from `Api:CorsAllowedOrigins` (semicolon list) that permits `X-Api-Key`/`If-Match`
|
||
and exposes `ETag`; with no origins configured there is no cross-origin access (the SPA is same-origin).
|
||
- **ForwardedHeaders trust + scanner loopback (#285, S7/S10).** `GET /api/maintenance/gc` → `POST`
|
||
(crawler-triggerable GC; spec regenerated). `ForwardedHeaders` trust is configurable via
|
||
`ForwardedHeaders:KnownProxies`/`KnownNetworks` — **unconfigured preserves the current trust-all
|
||
behavior but logs a warning** (flipping the default to loopback-only would break reverse-proxy scheme/host
|
||
detection and thus M3U/XMLTV absolute URLs — the operator must name their proxy network). `ScannerController`
|
||
gains `[LocalhostOnly]` (the scanner always calls back over `http://localhost:{UiPort}/api/scan/...`), which
|
||
is only spoof-resistant once ForwardedHeaders trust is restricted — the two interlock. `search/all-items`
|
||
DoS-paging is **deferred** (it feeds the SPA "add all" flow and needs coordinated pagination; the unauth
|
||
exposure is already closed by read-gating).
|
||
- **SPA (`web/`).** The client sends the stored key (`ctv-api-key`) on **every** method (not just
|
||
mutations); a new keyless **API Key** screen (`/app/api-key`) lets the user paste the generated key, and a
|
||
shell-level banner points there on any 401. See spa-conventions §5e. **First-run/upgrade UX:** with reads
|
||
gated by default, the SPA shows no data until the key (from `/config/api.key`) is entered — an intended
|
||
consequence of the strict default.
|
||
|
||
Phase-2 (contract freeze) — the declarative OpenAPI security scheme, global 401 docs, and `/api/v1`
|
||
versioning — remains #286/#287/#288. Phase-3 follow-ups: #265, #269, #172 remainder, `search/all-items`
|
||
paging, per-key rate limiting.
|
||
|
||
## 2026-07-12 (#197 Bundle C — contract-freeze honesty)
|
||
|
||
**#287 — OpenAPI contract honesty by construction.** The "v1" document now emits the `ApiKey` security
|
||
scheme plus per-operation `security`/`401` derived from the *same*
|
||
`ApiKeyAuthorizationFilter.EndpointRequiresKey` predicate the runtime filter enforces, so declared auth
|
||
can never drift from enforced auth. Every operation also gets a synthesized stable `operationId` (the
|
||
framework only assigned one when `Name=` was set — ~90 were missing), and body/param-binding operations
|
||
get the documented `400 ValidationProblemDetails` they actually return. `DayOfWeek` is now a string enum
|
||
in the schema (added to `Startup.UseStringEnumSchemas`), removing the SPA's `WithDayNames` wart. Pinned by
|
||
in-process document generation in tests (`OpenApiContractHonestyTests`) rather than the committed `v1.json`.
|
||
|
||
**#288 — Wrap the last raw ViewModels; reverse the §7a "intentional `version` leak."** Minted
|
||
`MediaCollectionResponseModel`, `ProgramScheduleResponseModel`, and `ChannelDetailResponseModel` (all
|
||
`#nullable enable`) and routed `CollectionController` / `ScheduleController` / `SmartCollectionController` /
|
||
`ResolutionController.GetResolutionByName` / the channel detail GET+writes through ResponseModels, so no
|
||
`/api/*` action returns an Application VM. This reverses the earlier §7a judgment that a ResponseModel
|
||
"purely to hide one field was disproportionate": `Version` is now header-only (ETag) on every aggregate
|
||
body — confirmed safe by grepping `web/src` (the SPA reads `version` from the ETag header, never the
|
||
response body). `ChannelDetailResponseModel` is the *full editable* field set the channel editor needs
|
||
(distinct from the lean list `ChannelResponseModel`; drops only the derived `webEncodedName`). Also flipped
|
||
`#nullable enable` onto the remaining 24 lagging `ErsatzTV.Core/Api/` files for schema honesty, and added
|
||
`pageNum` paging to `GET /api/search`.
|
||
|
||
**Channel REST resources are keyed by database `Id`, never by `Number`.** `Channel.Number` is user-mutable
|
||
(editable on update, bulk-renumbered via `/api/channels/bulk/renumber`, transiently invalid mid-renumber),
|
||
so the immutable int PK is the canonical key for all `/api/channels/*` single-item routes, sub-resources
|
||
(including `playout/reset`, re-keyed from `{channelNumber}` to `{id:int}` in Bundle C), and `Location`
|
||
headers. `Number` remains the identity on broadcast surfaces only (IPTV/M3U/XMLTV), a separate contract. A
|
||
number-based lookup endpoint may be added additively later; `UniqueId` (Guid) stays out of the REST
|
||
contract absent a federation requirement.
|
||
|
||
## 2026-07-12 — Browser SPA session auth: `/api` accepts session OR machine key (#295 PR1, server-only)
|
||
|
||
Implements the ratified #295 design (Fable [PLAN-MODE] pass, issue comment 9548). Supersedes the #206
|
||
"OIDC wiring stays inert until #197" note: the retained OIDC service registration is now **revived**, and a
|
||
cookie session becomes a first-class `/api` credential alongside the machine `X-Api-Key`. **PR1 is
|
||
server-only and backward compatible** — the SPA keeps sending its stored key; the SPA login flow, the
|
||
`ApiKeyScreen`→machine-key repurpose, and `spa-conventions §5e` land in **PR2**.
|
||
|
||
**One gate, evolved (not `[Authorize]`-per-controller).** `ApiKeyAuthorizationFilter` → `ApiAuthorizationFilter`,
|
||
same fail-closed-by-omission logic (a forgotten `[Authorize]` fails *open* — the #280 failure mode — so the
|
||
global filter stays the gate). It now accepts a request when a valid `X-Api-Key` matches **OR** the principal
|
||
is an authenticated session; the "does this endpoint need auth?" decision is still the single shared
|
||
`EndpointRequiresKey(...)` predicate (also drives OpenAPI, so the spec can't drift). Attributes renamed to
|
||
match the widened meaning: `[RequiresApiKey]`→`[RequiresAuthentication]`, `[SkipApiKeyAuthorization]`→
|
||
`[SkipApiAuthorization]`. `IApiKeyProvider`, the `X-Api-Key` header, and `Api:WriteKey`/`Api:RequireKeyForReads`
|
||
are unchanged — **machine/key behavior is byte-identical** (verified: no OpenAPI drift, existing filter tests
|
||
still green).
|
||
|
||
**CSRF (session only).** The machine key is CSRF-immune (a browser can't set a custom header cross-origin
|
||
without a credentialed CORS grant we never issue). A cookie session is not: a session-authenticated **mutation**
|
||
must carry the `X-CSRF` header (presence-only — a custom header forces a CORS preflight a cross-site page can't
|
||
satisfy) or is rejected **403**. Reinforced by `SameSite=Lax` + CORS without `AllowCredentials` (cross-origin
|
||
cookie auth is impossible by design). No antiforgery-token machinery.
|
||
|
||
**Cookie `ctv-session`.** Always registered (local login works with no IdP); OIDC handler added only when
|
||
`OIDC:*` is configured. `HttpOnly`, `SameSite=Lax`, `SecurePolicy=SameAsRequest` (so a plain-HTTP LAN isn't
|
||
bricked), 14-day sliding. `/api` XHR gets **401/403, not a redirect** (`OnRedirectToLogin`/`AccessDenied`).
|
||
The `UseAuthentication`/`UseAuthorization` middleware — deleted with Blazor in #91b — is **revived in the
|
||
`legacy` `MapWhen` branch only** (hosts `/api` + OIDC `/callback` + `/docs`; `/iptv` and `/app` untouched).
|
||
|
||
**Local store = `ConfigElement` rows, single admin, NO migration** (owner ruling F2):
|
||
`AuthLocalAdminUsername`, `AuthLocalAdminPasswordHash` (ASP.NET `PasswordHasher`, PBKDF2, via
|
||
`Microsoft.Extensions.Identity.Core`), `AuthSecurityStamp`. A password change rotates the stamp; the cookie
|
||
`OnValidatePrincipal` (`CookieSecurityStampValidator`) compares the claim to the stored stamp and rejects a
|
||
stale session (revocation). OIDC sessions carry an `etv:auth_method=oidc` claim and skip the stamp check
|
||
(governed by the IdP).
|
||
|
||
**Fail-closed out of the box + recovery.** An unconfigured instance keeps `/api` gated (the key still works);
|
||
first-run is a **setup-claim** (`POST /api/auth/setup`, first-claim-wins, only valid while unconfigured —
|
||
owner ruling F1). Recovery without the browser: `Auth:LocalAdmin:Password` env seed (`LocalAdminSeedService`,
|
||
overwrites + rotates the stamp on startup) or the machine key. Login hardening: per-IP rate limit
|
||
(`[EnableRateLimiting("auth")]`, 10 / 5 min) on login/setup/password, dummy-hash verify on unknown/unconfigured
|
||
user (no enumeration).
|
||
|
||
**Authelia = app-owned OIDC session; never trust proxy identity headers** (owner ruling F3): the container is
|
||
LAN-reachable bypassing the proxy, so `Remote-User`/`Remote-Email` header trust is spoofable. OIDC→Authelia
|
||
gives SSO without a double login. **`ForwardedHeaders` behaviour is kept unchanged from #285** (trust any peer
|
||
with a warning; restrict via `KnownProxies`/`:KnownNetworks`). A stricter "ignore `X-Forwarded-*` unless a proxy
|
||
is configured" default was implemented and then **reverted** after review (cold fork M1): the forwarded
|
||
scheme/host feed `/iptv` M3U/XMLTV/HLS absolute-URL generation (`Request.Scheme` in `GetChannelGuideHandler`/
|
||
`IptvController`), so ignoring them would regress stream URLs to `http`/internal-host for a proxied deployment
|
||
that hasn't set `KnownProxies`. **Deployment coordination:** operators behind a proxy should set
|
||
`ForwardedHeaders:KnownProxies`/`:KnownNetworks` — it gives the login rate limiter an unspoofable client IP and
|
||
marks the session cookie `Secure` behind TLS. The residual (a direct LAN peer can spoof `X-Forwarded-For` to
|
||
evade the per-IP login limit when unrestricted) is accepted defense-in-depth loss, mitigated by PBKDF2 +
|
||
no-enumeration.
|
||
|
||
**Review hardening (fork + independent Codex pass, folded into PR1).** Codex caught concurrency defects the
|
||
fork missed — folded in: (a) **atomic first-claim-wins** — setup writes the three credential rows in one
|
||
transaction guarded by the unique `ConfigElement.Key` index (a lost race → `DbUpdateException` → 409), so a
|
||
concurrent claim can't produce a mixed-state credential; (b) **consistent login snapshot** — login reads the
|
||
hash + stamp in one query and no longer rehashes-on-verify, so a login racing a password change can't capture a
|
||
newer stamp than the hash it verified (a concurrent change either fails the old password or leaves the issued
|
||
cookie carrying the pre-change stamp → revoked next request); (c) **env-seed waits on
|
||
`SystemStartup.WaitForDatabase`** (the migrator is a `BackgroundService`, so registration order alone didn't
|
||
guarantee the schema existed) — moved to `Services/RunOnce/`. Also: **logout + password require `X-CSRF`**
|
||
(the `[SkipApiAuthorization]` auth surface isn't covered by the filter's CSRF check → forced-logout CSRF), and
|
||
input length caps on username/password. **Logout rotates the security stamp** when called from a local session
|
||
(E2E-caught: `SignOutAsync` alone only clears the *client* cookie, leaving the stateless encrypted ticket
|
||
replayable server-side) — so signing out actually ends the session server-side; for the single admin this
|
||
revokes all local sessions ("log out everywhere"). Gated on an authenticated session so an unauthenticated
|
||
caller can't force-revoke the admin. **Deferred with a tracked gate:** side-effecting `[RequiresAuthentication]`
|
||
GETs (troubleshoot playback/archive) aren't CSRF-covered — **#301**, gating PR2 (latent in PR1: the SPA still
|
||
uses the machine key). OIDC-session revocation lever (no local stamp) noted for PR3 operator docs.
|
||
|
||
A **fix-commit re-review** (Codex, #242 discipline) then confirmed the above resolved and caught a second round:
|
||
(a) **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's credential persists,
|
||
defeating the env recovery path). Fixed structurally: **the setup-claim endpoint is closed whenever
|
||
`Auth:LocalAdmin:Password` is configured** — the env seed owns the credential, so there is no claim to race
|
||
(this also strengthens the setup-claim TOFU posture: an operator on an untrusted network sets the env password
|
||
and browser setup is disabled). (b) **LOW**: a concurrent setup race-loser now returns **409** (not 422), and
|
||
`ClaimLocalAdmin`'s `DbUpdateException` catch re-checks existence and **rethrows genuine/transient DB errors**
|
||
rather than masking them as "already configured". (c) **MEDIUM — accepted**: two *simultaneous* authenticated
|
||
password changes are a non-serializable lost-update (last-write-wins; the loser's cookie may be immediately
|
||
revoked). Accepted for a **single-admin** system: it needs two concurrent authenticated sessions both submitting
|
||
the correct current password at the same instant, and the outcome is self-healing (re-login). Adding EF
|
||
optimistic concurrency to the credential rows is disproportionate here.
|
||
|
||
**OpenAPI = `ApiKey`-only; `/api/auth/*` excluded** (owner ruling F4): the spec's audience is machine/MCP
|
||
clients, and a browser-interactive cookie login isn't something a generated client drives, so the cookie path
|
||
is an additional accepted credential the doc needn't express. `AuthController` is `[ApiExplorerSettings(IgnoreApi
|
||
= true)]`. Verified: no `v1.json`/`v1.d.ts`/`endpoint-index` drift from this PR.
|
||
|
||
**Phasing.** PR1 = this (server only, no migration). PR2 = SPA (drop the key header for browser calls + add
|
||
`X-CSRF`, `AuthContext` + boot gate, login/setup screens, `ApiKeyScreen`→machine-key management, E2E,
|
||
`spa-conventions §5e`). PR3 = key rotation + operator docs (Authelia client + env reference). Rollout: PR1→PR2
|
||
same release, then a manual Authelia round-trip checklist before the prod pin bump.
|
||
|
||
## 2026-07-12 — #295 PR2: SPA session cutover + #301 side-effecting-GET POST-ification
|
||
|
||
PR1 shipped the server side (previous entry): `/api` accepts a session cookie OR the machine `X-Api-Key`, with
|
||
`X-CSRF` required on session-authenticated mutations. **PR2 is the SPA cutover** — the browser now authenticates
|
||
with the session only — plus **#301** (a session-cookie CSRF hole in side-effecting GETs).
|
||
|
||
**Browser is cookie-only; the machine key is external/MCP-only.** `web/src/api/client.ts` no longer attaches
|
||
`X-Api-Key`; it relies on the same-origin session cookie and sets `X-Csrf: '1'` on every mutating verb centrally.
|
||
The former "paste your key" `ApiKeyScreen` is repurposed to **machine-key management**: it reads the server key
|
||
from the new `GET /api/auth/machine-key` (session-gated; masked with Reveal + Copy) so an operator can hand it to
|
||
MCP / external REST clients — the browser itself never sends it again. *Why:* one credential per audience (the
|
||
ratified #295 model); leaving a browser key path alive would keep a CSRF-immune bypass around and defeat the
|
||
point.
|
||
|
||
**Boot gate, not a route** (`web/src/AuthGate.tsx`, wrapping `<App/>` in `main.tsx`): on load it calls the public
|
||
`GET /api/auth/config` then `GET /api/auth/session` and renders Setup (first-run local-admin claim) / Login
|
||
(local form + an OIDC "Sign in with SSO" button when `oidcEnabled`) / the app. Login and Setup mint **no URL** —
|
||
the gate renders them at whatever `/app/*` path was requested, so a deep link survives login for free and **no
|
||
`blazor-route-parity.md`/`domain-model.md` route rows are added**. It publishes `AuthContext`
|
||
(`{ username, method, signOut, requireLogin }`); the 401 signal (`notifyUnauthorized`) now drives re-login via a
|
||
passive shell banner (never yanks a dirty draft — it consults the navigation guard first). Auth flows that expect
|
||
a 401 inline (login, change-password) pass `suppressUnauthorizedSignal`.
|
||
|
||
**#301 — POST-ify, don't gate-the-GET.** A side-effecting GET is a CSRF vector once a `SameSite=Lax` cookie is a
|
||
normal credential (it rides a cross-site top-level navigation). The three offenders became mutating verbs so the
|
||
existing filter CSRF gate covers them with zero new machinery: `GET /api/troubleshoot/playback.m3u8` →
|
||
**`POST /api/troubleshoot/playback/start`** returning `200 { url }` (the open `/iptv` manifest the player then
|
||
loads — so hls.js/native-HLS needs no header injection, strictly better than X-CSRF-on-GET); the archive and
|
||
sample GETs → **POST** (SPA downloads them via a fetch-blob helper, never `window.open`). Removing the HEAD
|
||
variants also fixed a latent bug: a HEAD opened the `DeleteOnClose` stream and destroyed the artifact. Standing
|
||
rule added to `api-conventions.md §9`: **never add a side-effecting GET/HEAD under `/api`.**
|
||
|
||
**Machine-key GET discloses the key to any authenticated session** — deliberate: the session principal is the
|
||
single admin (local or OIDC), same-origin policy blocks a cross-site page from reading the response body, and it
|
||
is how the "copy the key for MCP" UX works without a rotation endpoint (rotation is a later PR). **Accepted
|
||
residual (OIDC logout):** `POST /api/auth/logout` ends the *app* cookie but not the IdP session, so an OIDC user
|
||
who clicks "Sign out" then "Sign in with SSO" returns without re-entering credentials — a `returnUrl`/RP-initiated
|
||
logout is a future nicety. Docs: `spa-conventions.md §5e` (SPA seams), `api-conventions.md §9`, `e2e-local.md`
|
||
(browser setup/login flow). Refs #295 #301 #197.
|
||
|
||
## 2026-07-12 — Enforcing CSP + Permissions-Policy on the host (#319, ZAP baseline)
|
||
|
||
Completes the CSP that the #279 baseline-headers entry deferred ("CSP must be validated against the ChicoryTV
|
||
SPA's inline assets"). Surfaced by the #314 out-of-ecosystem ZAP baseline (missing CSP/Permissions-Policy WARNs);
|
||
a **#197 exit item**. `SecurityHeadersMiddleware` now also sets `Permissions-Policy` (deny-all for
|
||
camera/microphone/geolocation/payment/usb) and an **enforcing** `Content-Security-Policy`.
|
||
|
||
- **Enforce, not report-only.** Report-only was the issue's acceptable fallback, but the SPA's asset graph is
|
||
small and fully knowable, so we ship an enforcing policy (report-only leaves the ZAP WARN and provides no real
|
||
protection). The policy: `default-src 'self'`; `script-src 'self' '<sha256 of the inline theme-bootstrap
|
||
script>'` (**no** `'unsafe-inline'`/`'unsafe-eval'` — the real XSS win); `style-src 'self' 'unsafe-inline'
|
||
https://fonts.googleapis.com`; `img-src 'self' data: blob:`; `font-src 'self' data: https://fonts.gstatic.com`;
|
||
`connect-src 'self'`; `object-src 'none'`; `base-uri 'self'`; `frame-ancestors 'none'`; `form-action 'self'`.
|
||
- **Why each relaxation.** The SPA is a static file, so a per-response nonce is impossible → the one inline
|
||
theme-bootstrap `<script>` is allow-listed **by hash**; `SecurityHeadersMiddlewareTests.Csp_Script_Hash_Should_
|
||
Match_The_Spa_Index` hashes the built `wwwroot/app/index.html` when present (else the committed `web/index.html`
|
||
source, since the built artifact is gitignored/absent in CI — Vite copies the inline script verbatim) and fails
|
||
if it drifts from the middleware constant. `style-src 'unsafe-inline'` covers
|
||
React's inline `style=""` attributes (no CSS-in-JS lib to hash). The **Google Fonts** hosts are required — the
|
||
SPA CSS `@import`s the Geist web font (caught by **live-E2E**, which the static grep missed); self-hosting the
|
||
font to drop the Google dependency is a follow-on hardening, not this issue. `img-src data: blob:` covers
|
||
favicon/generated-image data URIs and object-URL upload previews.
|
||
- **Scoped: `/docs` (Scalar) and `/openapi` are excluded.** The Scalar API-reference UI relies on inline bootstrap
|
||
scripts/styles a strict CSP would break; it keeps the baseline headers (nosniff/frame/referrer) but no CSP.
|
||
Hardening that admin surface (self-hosted Scalar or a Scalar-tuned CSP) is a #197 follow-up. Everything else —
|
||
SPA, `/api`, `/artwork`, `/iptv` — gets the CSP (non-HTML responses simply never exercise the script/style
|
||
directives). Verified by live-E2E (SPA renders clean, zero CSP violations) + curl (CSP present on `/app`/`/api`,
|
||
absent on `/docs`/`/openapi`). HSTS remains out (proxy/TLS decision). Refs #319 #314 #197.
|
||
|
||
## 2026-07-13 — Cross-origin resource policy: `same-origin` on every response (#330)
|
||
|
||
The authenticated #314 ZAP scan found that ErsatzTV's baseline response posture omitted
|
||
`Cross-Origin-Resource-Policy`. `SecurityHeadersMiddleware` now sends
|
||
`Cross-Origin-Resource-Policy: same-origin` on every response, including `/docs` and `/openapi`.
|
||
Those two paths remain exempt only from the strict CSP that would break Scalar's inline bootstrap;
|
||
CORP has no equivalent rendering conflict and belongs with the middleware's path-independent baseline
|
||
headers.
|
||
|
||
`same-origin` requires the browser request and response to share the exact scheme, host, and port. It
|
||
blocks cross-origin `no-cors` loads, so direct browser embedding of ErsatzTV artwork or media from an
|
||
alternate origin is deliberately unsupported. It does not reject an allowed CORS-mode API fetch, so the
|
||
explicit `Api:CorsAllowedOrigins` machine-client path continues to work. It is also not enforced by
|
||
server-side HTTP clients, so Jellyfin's `/iptv/*` requests are unaffected; same-origin SPA artwork and
|
||
IPTV requests remain allowed. This is defense in depth for browser embedding and does not replace CORS
|
||
or authentication. Refs #330 #319 #314.
|