test(929): only ImageSharpDecoding may choose the decode configuration, and the test map names the IL scan
Review found that the surface guard checked that every decode takes
DecoderOptions and that only ImageSharpDecoding constructs them, but not
that the TIFF-blind configuration is the one that picks the decoder. A
new rule rejects, outside ImageSharpDecoding, a DecoderOptions
Configuration assignment, a read of Configuration.Default, and any
ImageFormatManager reference (a decoder fetched from a registry decodes
its own format whatever the options carry).
Mutation sweep against ImageElementBase.LoadImage, restored from git
after each:
- Configuration.Default.ImageFormatsManager.GetDecoder(...).DecodeAsync(
ImageSharpDecoding.Options(), ...): only the new rule reds (get_Default,
get_ImageFormatsManager, GetDecoder sites).
- ImageSharpDecoding.Configuration.ImageFormatsManager.GetDecoder(...):
only the new rule reds (ImageFormatManager clause alone).
- new DecoderOptions { Configuration = ImageSharpDecoding.Configuration }:
the new rule reds on set_Configuration and the construction rule on .ctor.
- options.Configuration = ... on a returned instance does not compile
(CS8852: init-only), so the reviewer's reassignment shape is not
expressible in C#; the IL rule still covers the setter.
docs/testing.md: the Architecture.Tests row now names all three rule
families (layering, provider-statics wiring, the Mono.Cecil ImageSharp
scan) and the host reference, and drops the stale exact count; the
missing ErsatzTV.Infrastructure.Tests row is added, since this branch
adds ImageSharpDecodingTests there. The record and guard inventory name
the fourth shape and what an IL scan cannot see.
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: f9ce6bc6-5b36-491a-8d72-89c1f70dd3e1
This commit is contained in:
Claude Opus 5.5
@@ -25,6 +25,8 @@ public class ImageSharpSurfaceTests
|
||||
private const string ImageSharpNamespace = "SixLabors.ImageSharp.";
|
||||
private const string DecodingType = "ErsatzTV.Infrastructure.Images.ImageSharpDecoding";
|
||||
private const string DecoderOptionsType = "SixLabors.ImageSharp.Formats.DecoderOptions";
|
||||
private const string ConfigurationType = "SixLabors.ImageSharp.Configuration";
|
||||
private const string FormatManagerType = "SixLabors.ImageSharp.Formats.ImageFormatManager";
|
||||
|
||||
// ImageSharp's format-decoding entry points: Image.Load*/Identify*/DetectFormat* and IImageDecoder's
|
||||
// Decode*/Identify*. LoadPixelData wraps raw pixels and runs no format decoder.
|
||||
@@ -114,6 +116,33 @@ public class ImageSharpSurfaceTests
|
||||
+ string.Join("\n ", violations));
|
||||
}
|
||||
|
||||
// Passing ImageSharpDecoding's DecoderOptions is not enough on its own: the TIFF-blind configuration
|
||||
// must also be the one that picks the decoder. Outside ImageSharpDecoding nothing may assign a
|
||||
// DecoderOptions.Configuration, read Configuration.Default (which registers TIFF), or touch an
|
||||
// ImageFormatManager, the registry a decoder can be fetched from directly — a decoder fetched that
|
||||
// way decodes its own format whatever the options' configuration says.
|
||||
[Test]
|
||||
public void Only_ImageSharpDecoding_chooses_the_decode_Configuration()
|
||||
{
|
||||
List<Site> sites = Sites.Value.ToList();
|
||||
|
||||
// sanity: the one legitimate Configuration assignment is visible to the scan
|
||||
sites.ShouldContain(s => IsInDecodingType(s.CallerType) && IsCall(s, DecoderOptionsType, "set_Configuration"));
|
||||
|
||||
List<string> violations = sites
|
||||
.Where(s => !IsInDecodingType(s.CallerType))
|
||||
.Where(s => IsCall(s, DecoderOptionsType, "set_Configuration")
|
||||
|| IsCall(s, ConfigurationType, "get_Default")
|
||||
|| s.ReferencedTypes.Contains(FormatManagerType))
|
||||
.Select(s => s.Describe())
|
||||
.ToList();
|
||||
|
||||
violations.ShouldBeEmpty(
|
||||
"Code outside ImageSharpDecoding chooses an ImageSharp decode configuration or decoder. "
|
||||
+ "Configuration.Default and its ImageFormatManager register TIFF. Use ImageSharpDecoding.Options(...):\n "
|
||||
+ string.Join("\n ", violations));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void No_code_reaches_the_ImageSharp_paths_the_3x_advisories_live_in()
|
||||
{
|
||||
@@ -142,6 +171,11 @@ public class ImageSharpSurfaceTests
|
||||
&& DecodeEntryPrefixes.Any(p => callee.Name.StartsWith(p, StringComparison.Ordinal))
|
||||
&& !NonDecodingEntries.Contains(callee.Name);
|
||||
|
||||
private static bool IsCall(Site site, string declaringType, string methodName) =>
|
||||
site.Callee is not null
|
||||
&& site.Callee.Name == methodName
|
||||
&& TypeName(site.Callee.DeclaringType) == declaringType;
|
||||
|
||||
private static bool IsInDecodingType(string typeName) =>
|
||||
typeName == DecodingType || typeName.StartsWith(DecodingType + "/", StringComparison.Ordinal);
|
||||
|
||||
|
||||
@@ -184,7 +184,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
|
||||
| `security.corp-same-origin` | `SecurityHeadersMiddleware` sends `Cross-Origin-Resource-Policy: same-origin` on every response including `/docs`/`/openapi`, blocking cross-origin `no-cors` embedding without affecting allowed CORS-mode fetches or server-side Jellyfin `/iptv/*` requests. | 2026-07-13 | [link](records/security/corp-same-origin.md) |
|
||||
| `security.csp-permissions-policy` | `SecurityHeadersMiddleware` sends an enforcing (not report-only) `Content-Security-Policy` (no `unsafe-inline`/`unsafe-eval`; the one inline theme-bootstrap script allow-listed by hash) and a deny-all `Permissions-Policy` on the SPA/`/api`/`/artwork`/`/iptv`; `/docs` and `/openapi` keep only the baseline headers, excluded from CSP because Scalar needs inline bootstrap. | 2026-07-12 | [link](records/security/csp-permissions-policy.md) |
|
||||
| `security.fail-closed-api-auth` | Every mutating `/api` request requires `X-Api-Key` (no open mode); reads are gated by `Api:RequireKeyForReads` (default true) OR `[RequiresApiKey]` on sensitive controllers; CORS is an exact-origin allowlist (`ApiCors`); `ForwardedHeaders` trust stays configurable but defaults to trust-all-with-warning. | 2026-07-12 | [link](records/security/fail-closed-api-auth.md) |
|
||||
| `security.imagesharp-3x-tiff-decoder-disabled` | SixLabors.ImageSharp stays on 3.1.12 until a Six Labors license key is provisioned for CI and the image build. Every ImageSharp decode (Identify and Load, remote and local) passes `ImageSharpDecoding.Options(...)`, whose configuration registers every format `Configuration.Default` does EXCEPT TIFF; no code references the TIFF codec, ICC profile types or the histogram-equalization processors. `ImageSharpSurfaceTests` enforces both over the IL of every production assembly that can reach ImageSharp. Revisit trigger: a provisioned key — then move to the patched line, drop the Renovate hold and re-measure the #511 decoder behaviours. | 2026-10-27 | [link](records/security/imagesharp-3x-tiff-decoder-disabled.md) |
|
||||
| `security.imagesharp-3x-tiff-decoder-disabled` | SixLabors.ImageSharp stays on 3.1.12 until a Six Labors license key is provisioned for CI and the image build. Every ImageSharp decode (Identify and Load, remote and local) passes `ImageSharpDecoding.Options(...)`, whose configuration registers every format `Configuration.Default` does EXCEPT TIFF, and nothing outside `ImageSharpDecoding` assigns a decode configuration, reads `Configuration.Default` or touches an `ImageFormatManager`; no code references the TIFF codec, ICC profile types or the histogram-equalization processors. `ImageSharpSurfaceTests` enforces all of it over the IL of every production assembly that can reach ImageSharp. Revisit trigger: a provisioned key — then move to the patched line, drop the Renovate hold and re-measure the #511 decoder behaviours. | 2026-10-27 | [link](records/security/imagesharp-3x-tiff-decoder-disabled.md) |
|
||||
| `security.iptv-access-token-transport` | The `/iptv` `?access_token=` value is percent-encoded (`Uri.EscapeDataString`) everywhere it is interpolated into an M3U/HLS/XMLTV URL (XMLTV additionally XML-escapes the encoded value), so a structural character can't malform the manifest or guide; Serilog logs a scrubbed request path (`access_token` → `***` via `IncludeQueryInRequestPath = false` + a `RequestPathScrubbed` enricher), so a 5xx/Debug `/iptv` request never writes the token; and every dynamic token-bearing `/iptv` manifest (`channels.m3u`, `xmltv.xml`, the HLS multi-variant/media playlists) returns `Cache-Control: private, no-store`. | 2026-07-23 | [link](records/security/iptv-access-token-transport.md) |
|
||||
| `security.iptv-browser-token` | Under a JWT-enabled deployment (`JWT:IssuerSigningKey` set), the browser SPA obtains a short-lived, globally-scoped `/iptv/*` access token from an authenticated `GET /api/v1/auth/iptv-token` and appends it as `?access_token=`; the endpoint answers 204 when JWT is disabled (nothing to mint). Lifetime defaults to 60 min, configurable via `JWT:BrowserTokenLifetimeMinutes`. | 2026-07-22 | [link](records/security/iptv-browser-token.md) |
|
||||
| `security.session-auth-dual-credential` | `ApiAuthorizationFilter` accepts a request when a valid `X-Api-Key` matches OR the principal is an authenticated session (cookie `ctv-session`, `HttpOnly`/`SameSite=Lax`); session-authenticated mutations require the presence-only `X-CSRF` header or are rejected 403. This narrows the OIDC-inert sub-claim of `security.blazor-removal-auth-posture` (#206) — the rest of that record's auth-surface enumeration still holds. | 2026-07-12 | [link](records/security/session-auth-dual-credential.md) |
|
||||
|
||||
@@ -5,7 +5,7 @@ status: active
|
||||
since: '2026-10-27'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
rule: 'SixLabors.ImageSharp stays on 3.1.12 until a Six Labors license key is provisioned for CI and the image build. Every ImageSharp decode (Identify and Load, remote and local) passes `ImageSharpDecoding.Options(...)`, whose configuration registers every format `Configuration.Default` does EXCEPT TIFF; no code references the TIFF codec, ICC profile types or the histogram-equalization processors. `ImageSharpSurfaceTests` enforces both over the IL of every production assembly that can reach ImageSharp. Revisit trigger: a provisioned key — then move to the patched line, drop the Renovate hold and re-measure the #511 decoder behaviours.'
|
||||
rule: 'SixLabors.ImageSharp stays on 3.1.12 until a Six Labors license key is provisioned for CI and the image build. Every ImageSharp decode (Identify and Load, remote and local) passes `ImageSharpDecoding.Options(...)`, whose configuration registers every format `Configuration.Default` does EXCEPT TIFF, and nothing outside `ImageSharpDecoding` assigns a decode configuration, reads `Configuration.Default` or touches an `ImageFormatManager`; no code references the TIFF codec, ICC profile types or the histogram-equalization processors. `ImageSharpSurfaceTests` enforces all of it over the IL of every production assembly that can reach ImageSharp. Revisit trigger: a provisioned key — then move to the patched line, drop the Renovate hold and re-measure the #511 decoder behaviours.'
|
||||
signals: 'ImageSharp upgrade, ImageSharp 4, Six Labors license key, SixLaborsLicenseKey, No Six Labors license found, Release build fails, Renovate ImageSharp PR, vulnerable ImageSharp, TIFF decoder disabled, UnknownImageFormatException, ICC profile, dotnet list package --vulnerable, NuGetAuditSuppress · paths: `ErsatzTV.Infrastructure/Images/ImageSharpDecoding.cs`, `RemoteImageValidator.DecodeAndValidate`, `ImageElementBase.LoadImage`, `ImageSharpDecodingTests`, `ErsatzTV.Architecture.Tests/ImageSharpSurfaceTests.cs`, `Directory.Packages.props`, `renovate.json` · issues: #929, #923, #511, #525, #924'
|
||||
mechanics: '`ImageSharpDecoding.Configuration` is a fresh `Configuration` built from the format modules, NOT `Configuration.Default.Clone()` — Clone is shallow, so removing a detector from it would remove it from Default for the whole process'
|
||||
---
|
||||
@@ -46,15 +46,27 @@ ImageSharp, so each is enforced over the compiled code rather than left to revie
|
||||
`ImageSharpSurfaceTests` (`ErsatzTV.Architecture.Tests`) reads the IL of every non-test project in
|
||||
`ErsatzTV.sln` whose project-reference closure contains the ImageSharp package — the population is
|
||||
derived from the solution and csproj graph, and a project that joins it without being referenced
|
||||
by the test project fails the fixture. It rejects three shapes: an ImageSharp decode entry point
|
||||
(`Load*`, `Identify*`, `DetectFormat*`, `Decode*`) called without `DecoderOptions`, a
|
||||
`DecoderOptions` constructed outside `ImageSharpDecoding`, and any reference into the
|
||||
`Formats.Tiff`, `Metadata.Profiles.Icc` or `Processing.Processors.Normalization` namespaces, the
|
||||
`HistogramEqualizationExtensions` type, or an ImageSharp member whose name contains `Tiff` or
|
||||
`HistogramEqualization` (`ImageExtensions.SaveAsTiff` carries no TIFF type in its signature). Each
|
||||
check also asserts it sees something known to exist — the infrastructure and host assemblies in
|
||||
the population, the two decode sites, the one `DecoderOptions` construction, the `Resize` call
|
||||
inside a lambda — so an empty scan cannot pass.
|
||||
by the test project fails the fixture. It rejects four shapes:
|
||||
|
||||
- an ImageSharp decode entry point (`Load*`, `Identify*`, `DetectFormat*`, `Decode*`) called
|
||||
without `DecoderOptions`;
|
||||
- a `DecoderOptions` constructed outside `ImageSharpDecoding`;
|
||||
- outside `ImageSharpDecoding`, a call to `DecoderOptions.set_Configuration`, a read of
|
||||
`Configuration.Default`, or any reference to `ImageFormatManager`. A decoder fetched from a
|
||||
format registry decodes its own format whatever configuration the options it is handed carry, so
|
||||
passing the right options is not enough on its own. `DecoderOptions.Configuration` is init-only:
|
||||
reassigning it on a returned instance does not compile (CS8852, measured 2026-10-27), and an
|
||||
object initializer is caught twice, by this rule and the previous one;
|
||||
- any reference into the `Formats.Tiff`, `Metadata.Profiles.Icc` or
|
||||
`Processing.Processors.Normalization` namespaces, the `HistogramEqualizationExtensions` type, or an
|
||||
ImageSharp member whose name contains `Tiff` or `HistogramEqualization`
|
||||
(`ImageExtensions.SaveAsTiff` carries no TIFF type in its signature).
|
||||
|
||||
Each check also asserts it sees something known to exist — the infrastructure and host assemblies
|
||||
in the population, the two decode sites, the one `DecoderOptions` construction and its
|
||||
`Configuration` assignment, the `Resize` call inside a lambda — so an empty scan cannot pass.
|
||||
What an IL scan cannot see: reflection, and a format decoder's own static `Instance` (each decodes
|
||||
only its own format, and TIFF's is in the banned namespace).
|
||||
|
||||
`ImageSharpDecodingTests` derives the expected format list from `Configuration.Default` minus
|
||||
TIFF, so a package bump that adds a format reddens rather than silently narrowing the decoder; a
|
||||
|
||||
@@ -70,8 +70,8 @@ with have sat in these gaps. Treat anything not named here as unassessed, not as
|
||||
2. **C# and TypeScript guards** — none with a row. The list is hand-written, so it is a floor and
|
||||
not the population:
|
||||
`ErsatzTV.Architecture.Tests/ImageSharpSurfaceTests.cs` (ersatztv#929 — derives the assemblies
|
||||
that can reach ImageSharp from `ErsatzTV.sln` and the csproj graph and rejects decode, TIFF, ICC
|
||||
and histogram-equalization call shapes in their IL), its siblings
|
||||
that can reach ImageSharp from `ErsatzTV.sln` and the csproj graph and rejects decode,
|
||||
decode-configuration, TIFF, ICC and histogram-equalization call shapes in their IL), its siblings
|
||||
`ErsatzTV.Architecture.Tests/LayeringTests.cs` and `…/ProviderStaticsWiringTests.cs`,
|
||||
`ErsatzTV.Tests/Integration/SearchIndexMutationCoverageTests.cs` (ersatztv#824 — derives the
|
||||
`ISearchIndex` population from the declaring assembly and asserts set equality against a
|
||||
|
||||
+2
-1
@@ -18,7 +18,8 @@ before adding tests, not just `docs/contributing.md` §8 (which now just points
|
||||
| `ErsatzTV.Tests` | API controllers + MediatR handlers, plus the SkiaSharp text-overlay rasteriser (`Infrastructure/Graphics/`) | In-memory SQLite fixture: a shared `SqliteConnection("Data Source=:memory:;Foreign Keys=False")` kept open + `EnsureCreatedAsync()` (**not** full migration replay) + `PRAGMA foreign_keys=OFF`, then seed; a tiny `IDbContextFactory` wraps `new TvContext(...)`. ~1,870 tests (approximate on purpose — an exact count goes stale on every PR that adds one; the previous hardcoded 828 was off by over a thousand). |
|
||||
| `ErsatzTV.Core.Tests` | Domain logic, scheduling, IPTV/XMLTV generation | References `ErsatzTV.Application` directly — there is no separate `Application.Tests` project. ~650 tests + 1 skipped under `TZ=UTC` (the Block playout golden additionally skips under a non-UTC `TZ`; see Golden-file nets). |
|
||||
| `ErsatzTV.Scanner.Tests` | Library scanning: scan handlers, folder scanners, NFO readers | Handler tests substitute the folder scanners + `ILibraryRepository` and assert the resulting repository writes (e.g. `ScanLocalLibraryHandlerTests` pins which `LastScan` levels a scan records — ersatztv#264). Fakes/`Testably` back the file-system-facing scanners. ~1,485 tests (approximate on purpose — an exact count goes stale on every PR that adds one). Additionally contains `Core/FFmpeg/TranscodingTests` — `[Explicit]` + `[Combinatorial]`, so it never runs in CI or a plain `dotnet test` (it needs real ffmpeg/hardware) and contributes 0 to that count; run it by name when touching the transcoding pipeline. |
|
||||
| `ErsatzTV.Architecture.Tests` | Layering rules via NetArchTest.eNhancedEdition | Core↛Infra/App/EF; FFmpeg↛all; App↛concrete providers. 5 tests. See `docs/contributing.md` §1. |
|
||||
| `ErsatzTV.Infrastructure.Tests` | Infrastructure services tested without a host: remote image fetching, decode bounds and logo caching, the ImageSharp decode configuration, remote stream probing, the Jellyfin API client, entity locking, local statistics, search query parsing, sequential schedule validation, graphics opacity expressions | The HTTP-facing fixtures use the in-process handler and stream doubles in `Streaming/HttpTestDoubles.cs`, never the network. `ImageSharpDecodingTests` derives its expected format list from `Configuration.Default`, so an ImageSharp bump that adds a format reddens (`security.imagesharp-3x-tiff-decoder-disabled`). |
|
||||
| `ErsatzTV.Architecture.Tests` | Rules over compiled assemblies: layering (NetArchTest.eNhancedEdition), provider-statics wiring, and the ImageSharp call surface (Mono.Cecil IL scan) | `LayeringTests`: Core↛Infra/App/EF; FFmpeg↛all; App↛concrete providers — see `docs/contributing.md` §1. `ImageSharpSurfaceTests` reads the IL of every non-test assembly whose project graph reaches ImageSharp (derived from `ErsatzTV.sln` and the csproj references, so the project references the host `ErsatzTV` too) and rejects the call shapes `security.imagesharp-3x-tiff-decoder-disabled` names. No exact count on purpose — it goes stale on every PR that adds a rule. |
|
||||
| `ErsatzTV.FFmpeg.Tests` | FFmpeg command construction | Build a pipeline, assert the exact rendered arg string (`PipelineBuilderBaseTests.cs`). |
|
||||
| `web/` (vitest) | React SPA unit tests | Run alongside typecheck + build (see below). Collects `src/**`, `web/scripts/**` *and* `web/vite-plugins/**`, but deliberately **excludes** `web/e2e/**` (the Playwright specs — vitest's default `**/*.spec.*` glob would otherwise run them under jsdom). Some files have git prerequisites since ersatztv#819, and the set is not fixed — ersatztv#883 added one. `web/src/api/pageSizeCallSites.guard.test.ts` and `web/src/api/completeAnnotations.guard.test.ts` need a git **checkout** AND, through it, the **binary**: they derive their file population from `git ls-files` via `web/vite-plugins/trackedSourceFiles.ts` rather than a directory walk, and refuse rather than falling back. `web/vite-plugins/trackedSourceFiles.realgit.test.ts` needs the **binary** but no checkout — it builds its own temp repository to prove that derivation by executing it. So it is not checkout-versus-binary: supplying a `.git` alone would not let any of them run. **The suite therefore runs only where git is present, and `docker/Dockerfile` is not such a place** — its web-build stage builds the SPA and does not test it (ersatztv#887). Enumerating the git-dependent files as Docker `--exclude`s was tried and REVERSED: that list is a population nothing derives, it went stale the first time a guard was added, and the resulting red is unreachable on a PR — `Build & push image (amd64)` is `if: github.event_name != 'pull_request'` — so it landed on `main` and on the release tag instead. The image is gated on `docker-build.yml`'s `test` job running the whole suite on a real checkout, held by `scripts/tests/test_image_build_delegates_the_spa_suite.py`. |
|
||||
| `web/e2e/` (Playwright) | UI-interactive E2E flows against a **live** instance | Not a unit suite and **not** part of `npm test` — needs a running server, so it runs via `scripts/e2e-ui.sh` (boots its own fresh instance) and in CI as a step of the `functional-e2e` job. Headless Chromium, `serial`, `retries: 0`. Scope rule: assert only what the curl harness structurally cannot. See `docs/e2e-local.md` → "UI-E2E harness". |
|
||||
|
||||
Reference in New Issue
Block a user