Compare commits
12
Commits
bd24d21e23
...
71d660f213
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
71d660f213 | ||
|
|
bed50e2b77 | ||
|
|
59749babd9 | ||
|
|
f30a54172b | ||
|
|
52c383e832 | ||
|
|
e96e47a9d8 | ||
|
|
9eed552876 | ||
|
|
bcf6f7e04d | ||
|
|
403bf011c1 | ||
|
|
00424bf090 | ||
|
|
8acba80a1a | ||
|
|
41d604f0ae |
@@ -546,16 +546,19 @@ jobs:
|
||||
- name: Install test dependencies
|
||||
run: python3 -m pip install --disable-pip-version-check --quiet pytest pyyaml
|
||||
# jq gets its OWN step because its VERSION, not merely its presence, is load-bearing
|
||||
# (ersatztv#648). `--expect` makes this a TRIPWIRE: scripts/tests exercises the jq 1.6 code path
|
||||
# only because this runner ships 1.6, so an upgrade would silently delete that coverage — and
|
||||
# the three divergences found in ersatztv#643/#647 all lived exactly there. Going red forces an
|
||||
# explicit human decision instead of letting the coverage evaporate.
|
||||
# (ersatztv#648). `--expect` makes this a TRIPWIRE over an accepted SET of versions: the
|
||||
# set is exactly {1.6, 1.7} (ersatztv#935). Both `small` runners that took PR #933/#934 reported
|
||||
# 1.7 (2026-10-28), but both map to the floating runner-images:ubuntu-latest image with no digest
|
||||
# pin, so a host may hold an older pull and 1.6 stays in the set. Any other version goes red. No
|
||||
# known runner reports 1.6 today, so real-jq 1.6 coverage of scripts/tests is currently absent.
|
||||
# server-management#1080 proposes a fleet digest pin; when it settles one version, narrow this
|
||||
# to that version.
|
||||
#
|
||||
# The pin lives HERE and deliberately NOT in review-verdict.yml: that workflow writes the
|
||||
# branch-protection-required `review-verdict/h10` status, so pinning a version there would turn
|
||||
# any jq bump on the runner into a repo-wide merge deadlock. It gets the floor-only mode.
|
||||
# See docs/ci-cd.md -> "The jq contract".
|
||||
- name: Preflight jq version
|
||||
run: ./scripts/jq-preflight.sh --expect 1.6
|
||||
run: ./scripts/jq-preflight.sh --expect 1.6,1.7
|
||||
- name: Run scripts/tests
|
||||
run: PYTHONPATH=. python3 -m pytest scripts/tests -q
|
||||
|
||||
@@ -572,7 +572,7 @@ jobs:
|
||||
# verdict and leaves a FORGED one — and this job's own red status is not a required check.
|
||||
#
|
||||
# The empty case is checked EXPLICITLY, not left to jq's exit status: `jq -e` over empty input
|
||||
# exits 4 on jq >= 1.7 but 0 on jq 1.6, and THE RUNNER SHIPS 1.6 (ersatztv#647) — so on a
|
||||
# exits 4 on jq >= 1.7 but 0 on jq 1.6, and a runner may report 1.6 (the floor; ersatztv#647) — so on a
|
||||
# transient error this guard passed, the row came back "", and the job posted over a
|
||||
# possibly-existing human verdict.
|
||||
#
|
||||
@@ -1232,7 +1232,7 @@ jobs:
|
||||
# COUNTED, NOT `jq -e`, and that is this function's own standing rule rather than a
|
||||
# style preference. The `kind` read above says it in full: `jq -e` reports the
|
||||
# truthiness of its LAST OUTPUT, which is the exit-status subtlety that already bit
|
||||
# this workflow at jq 1.6 (ersatztv#647), and the runner still ships 1.6. A count read
|
||||
# this workflow at jq 1.6 (ersatztv#647), and a runner may still report 1.6. A count read
|
||||
# as a VALUE and then range-checked in shell cannot be read two ways, and it matches
|
||||
# every other tally here. (Using `jq -e` here would be safe by accident — the
|
||||
# array is known non-empty by the check above, so the empty-input
|
||||
|
||||
@@ -58,6 +58,7 @@
|
||||
<PackageVersion Include="Microsoft.Extensions.Logging.Debug" Version="10.0.7" />
|
||||
<PackageVersion Include="Microsoft.IO.RecyclableMemoryStream" Version="3.0.1" />
|
||||
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="18.0.1" />
|
||||
<PackageVersion Include="Mono.Cecil" Version="0.11.6" />
|
||||
<PackageVersion Include="Microsoft.VisualStudio.Threading.Analyzers" Version="17.14.15" />
|
||||
<PackageVersion Include="NCalcSync" Version="6.3.2" />
|
||||
<PackageVersion Include="NetArchTest.eNhancedEdition" Version="1.4.5" />
|
||||
@@ -87,6 +88,9 @@
|
||||
<PackageVersion Include="Serilog.Sinks.Debug" Version="3.0.0" />
|
||||
<PackageVersion Include="Serilog.Sinks.File" Version="7.0.0" />
|
||||
<PackageVersion Include="Shouldly" Version="4.3.0" />
|
||||
<!-- HELD at 3.1.12: every patched release (3.2.0, 4.1.2+) fails a Release build without a Six Labors
|
||||
license key. Do not bump until one is provisioned; the advisory bounds and the revisit steps are in
|
||||
security.imagesharp-3x-tiff-decoder-disabled, and renovate.json holds Renovate's update PRs (#929). -->
|
||||
<PackageVersion Include="SixLabors.ImageSharp" Version="3.1.12" />
|
||||
<PackageVersion Include="SkiaSharp" Version="3.119.1" />
|
||||
<PackageVersion Include="SkiaSharp.NativeAssets.Linux.NoDependencies" Version="3.119.1" />
|
||||
@@ -95,6 +99,12 @@
|
||||
bundled SQLite, GHSA-2m69-gcr7-jv3q). The 3.x line ships the patched native
|
||||
(lib.e_sqlite3 3.50.3); core 3.0.4 satisfies Microsoft.Data.Sqlite's `>= 2.1.10`. (#8) -->
|
||||
<PackageVersion Include="SQLitePCLRaw.bundle_e_sqlite3" Version="3.0.4" />
|
||||
<!-- Direct pin to override the transitive 9.0.4 that EFCore.BulkExtensions' Oracle and SqlServer
|
||||
providers pull in (via Oracle.ManagedDataAccess.Core / Microsoft.Data.SqlClient):
|
||||
GHSA-846f-f764-76mx (High) and GHSA-2fmv-rv2c-59x7 (Moderate), both fixed in 9.0.21.
|
||||
Referenced directly in ErsatzTV.Infrastructure.csproj, the only project that references
|
||||
EFCore.BulkExtensions, so the override flows to every project above it (#929). -->
|
||||
<PackageVersion Include="System.Security.Cryptography.Pkcs" Version="9.0.21" />
|
||||
<PackageVersion Include="System.CommandLine" Version="2.0.2" />
|
||||
<PackageVersion Include="TagLibSharp" Version="2.3.0" />
|
||||
<PackageVersion Include="Testably.Abstractions" Version="10.0.0" />
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
|
||||
<ItemGroup>
|
||||
<PackageReference Include="Microsoft.NET.Test.Sdk" />
|
||||
<!-- IL reads for ImageSharpSurfaceTests (#929); NetArchTest already depends on it -->
|
||||
<PackageReference Include="Mono.Cecil" />
|
||||
<PackageReference Include="NetArchTest.eNhancedEdition" />
|
||||
<PackageReference Include="NUnit" />
|
||||
<PackageReference Include="NUnit3TestAdapter" />
|
||||
@@ -33,6 +35,8 @@
|
||||
<ProjectReference Include="..\ErsatzTV.Infrastructure.Sqlite\ErsatzTV.Infrastructure.Sqlite.csproj" />
|
||||
<ProjectReference Include="..\ErsatzTV.Infrastructure.MySql\ErsatzTV.Infrastructure.MySql.csproj" />
|
||||
<ProjectReference Include="..\ErsatzTV.Scanner\ErsatzTV.Scanner.csproj" />
|
||||
<!-- the host: ImageSharpSurfaceTests scans every assembly that can reach ImageSharp (#929) -->
|
||||
<ProjectReference Include="..\ErsatzTV\ErsatzTV.csproj" />
|
||||
</ItemGroup>
|
||||
|
||||
</Project>
|
||||
|
||||
@@ -0,0 +1,406 @@
|
||||
using System.Xml.Linq;
|
||||
using Mono.Cecil;
|
||||
using Mono.Cecil.Cil;
|
||||
using NUnit.Framework;
|
||||
using Shouldly;
|
||||
|
||||
namespace ErsatzTV.Architecture.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// ersatztv#929, <c>security.imagesharp-3x-tiff-decoder-disabled</c>: SixLabors.ImageSharp is held at
|
||||
/// 3.1.12, and what keeps its advisories unreachable is how ErsatzTV CALLS it, not the package. Every
|
||||
/// decode must go through <c>ImageSharpDecoding</c>'s TIFF-blind configuration, and nothing may touch
|
||||
/// the TIFF codec, ICC profile entries or the histogram-equalization processors.
|
||||
/// <para>
|
||||
/// These rules read the IL of every production assembly that can reach ImageSharp, so a NEW call
|
||||
/// site fails here instead of quietly reopening an advisory. The population is derived from the
|
||||
/// solution and the csproj graph, not listed: a project that gains an ImageSharp reference joins
|
||||
/// it, and this fixture fails until the project is referenced here and therefore scanned.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
[TestFixture]
|
||||
public class ImageSharpSurfaceTests
|
||||
{
|
||||
private const string ImageSharpPackage = "SixLabors.ImageSharp";
|
||||
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.
|
||||
private static readonly string[] DecodeEntryPrefixes = ["Load", "Identify", "DetectFormat", "Decode"];
|
||||
private static readonly string[] NonDecodingEntries = ["LoadPixelData"];
|
||||
|
||||
// Where the 3.1.12 advisories live that no decoder configuration can bound: the TIFF codecs (the
|
||||
// encoders carry GHSA-jjfr / GHSA-j9gm), ICC profile parsing (GHSA-gwg2) and histogram equalization
|
||||
// (GHSA-j3p4). The whole Normalization namespace is banned, not just the one processor, because its
|
||||
// processors share the histogram code.
|
||||
private static readonly string[] ForbiddenTypePrefixes =
|
||||
[
|
||||
"SixLabors.ImageSharp.Formats.Tiff.",
|
||||
"SixLabors.ImageSharp.Metadata.Profiles.Icc.",
|
||||
"SixLabors.ImageSharp.Processing.Processors.Normalization.",
|
||||
"SixLabors.ImageSharp.Processing.HistogramEqualizationExtensions"
|
||||
];
|
||||
|
||||
// Extension methods such as ImageExtensions.SaveAsTiff carry no TIFF type in their signature.
|
||||
private static readonly string[] ForbiddenImageSharpMemberFragments = ["Tiff", "HistogramEqualization"];
|
||||
|
||||
private static readonly Lazy<IReadOnlyList<string>> Population = new(ImageSharpReachingAssemblies);
|
||||
|
||||
private static readonly Lazy<IReadOnlyList<Site>> Sites = new(() => Scan(Population.Value));
|
||||
|
||||
[Test]
|
||||
public void Every_production_assembly_that_can_reach_ImageSharp_is_scanned()
|
||||
{
|
||||
IReadOnlyList<string> population = Population.Value;
|
||||
|
||||
// sanity: the csproj walk found the package reference and followed project references upward
|
||||
population.ShouldContain("ErsatzTV.Infrastructure");
|
||||
population.ShouldContain("ErsatzTV");
|
||||
|
||||
List<string> missing = population
|
||||
.Where(name => !File.Exists(Path.Combine(AppContext.BaseDirectory, name + ".dll")))
|
||||
.ToList();
|
||||
|
||||
missing.ShouldBeEmpty(
|
||||
$"{string.Join(", ", missing)} can reach {ImageSharpPackage} but is not in this test's output, so "
|
||||
+ "its ImageSharp calls are not checked. Add a ProjectReference to ErsatzTV.Architecture.Tests.csproj.");
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Every_ImageSharp_decode_passes_DecoderOptions()
|
||||
{
|
||||
List<Site> decodes = Sites.Value
|
||||
.Where(s => s.Callee is not null && IsDecodeEntry(s.Callee))
|
||||
.ToList();
|
||||
|
||||
// sanity: the scan reaches the decodes we know exist, including the ones inside async state
|
||||
// machines (nested compiler-generated types), so an empty result cannot pass for a clean one
|
||||
decodes.ShouldContain(s => s.CallerType.StartsWith("ErsatzTV.Infrastructure.Images.RemoteImageValidator", StringComparison.Ordinal));
|
||||
decodes.ShouldContain(s => s.CallerType.StartsWith("ErsatzTV.Infrastructure.Streaming.Graphics.ImageElementBase", StringComparison.Ordinal));
|
||||
|
||||
List<string> violations = decodes
|
||||
.Where(s => !s.Callee!.Parameters.Any(p => TypeName(p.ParameterType) == DecoderOptionsType))
|
||||
.Select(s => s.Describe())
|
||||
.ToList();
|
||||
|
||||
violations.ShouldBeEmpty(
|
||||
"ImageSharp decode(s) without DecoderOptions, which fall back to Configuration.Default and so "
|
||||
+ "can detect and decode TIFF. Pass ImageSharpDecoding.Options(...):\n "
|
||||
+ string.Join("\n ", violations));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Only_ImageSharpDecoding_constructs_DecoderOptions()
|
||||
{
|
||||
List<Site> constructions = Sites.Value
|
||||
.Where(s => s.OpCode == OpCodes.Newobj
|
||||
&& s.Callee is not null
|
||||
&& TypeName(s.Callee.DeclaringType) == DecoderOptionsType)
|
||||
.ToList();
|
||||
|
||||
// sanity: the one legitimate construction is visible to the scan
|
||||
constructions.ShouldContain(s => IsInDecodingType(s.CallerType));
|
||||
|
||||
List<string> violations = constructions
|
||||
.Where(s => !IsInDecodingType(s.CallerType))
|
||||
.Select(s => s.Describe())
|
||||
.ToList();
|
||||
|
||||
violations.ShouldBeEmpty(
|
||||
"DecoderOptions constructed outside ImageSharpDecoding. Its Configuration defaults to "
|
||||
+ "Configuration.Default, which decodes TIFF. Use ImageSharpDecoding.Options(...):\n "
|
||||
+ 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()
|
||||
{
|
||||
List<Site> sites = Sites.Value.ToList();
|
||||
|
||||
// sanity: the type walk sees an extension method called from inside a lambda (Mutate's Resize),
|
||||
// which is the same shape a HistogramEqualization() call would have
|
||||
sites.ShouldContain(s => s.ReferencedTypes.Contains("SixLabors.ImageSharp.Processing.ResizeExtensions"));
|
||||
|
||||
List<string> violations = sites
|
||||
.Where(s => s.ReferencedTypes.Any(IsForbiddenType)
|
||||
|| s.Callee is not null
|
||||
&& TypeName(s.Callee.DeclaringType).StartsWith(ImageSharpNamespace, StringComparison.Ordinal)
|
||||
&& ForbiddenImageSharpMemberFragments.Any(f => s.Callee.Name.Contains(f, StringComparison.Ordinal)))
|
||||
.Select(s => s.Describe())
|
||||
.ToList();
|
||||
|
||||
violations.ShouldBeEmpty(
|
||||
"Code reaches an ImageSharp 3.1.12 path with an unpatched advisory (TIFF codec, ICC profile "
|
||||
+ "entries, histogram equalization). See security.imagesharp-3x-tiff-decoder-disabled:\n "
|
||||
+ string.Join("\n ", violations));
|
||||
}
|
||||
|
||||
private static bool IsDecodeEntry(MethodReference callee) =>
|
||||
TypeName(callee.DeclaringType).StartsWith(ImageSharpNamespace, StringComparison.Ordinal)
|
||||
&& 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);
|
||||
|
||||
private static bool IsForbiddenType(string typeName) =>
|
||||
ForbiddenTypePrefixes.Any(p => typeName.StartsWith(p, StringComparison.Ordinal));
|
||||
|
||||
private static List<Site> Scan(IReadOnlyList<string> assemblyNames)
|
||||
{
|
||||
var sites = new List<Site>();
|
||||
foreach (string name in assemblyNames)
|
||||
{
|
||||
string path = Path.Combine(AppContext.BaseDirectory, name + ".dll");
|
||||
if (!File.Exists(path))
|
||||
{
|
||||
// reported by Every_production_assembly_that_can_reach_ImageSharp_is_scanned
|
||||
continue;
|
||||
}
|
||||
|
||||
// read fully into memory and kept alive: Site holds MethodReferences whose parameters
|
||||
// are read lazily, after this loop
|
||||
AssemblyDefinition assembly = AssemblyDefinition.ReadAssembly(
|
||||
path,
|
||||
new ReaderParameters { InMemory = true, ReadingMode = ReadingMode.Immediate });
|
||||
foreach (TypeDefinition type in assembly.MainModule.GetTypes())
|
||||
{
|
||||
var signature = new HashSet<string>(StringComparer.Ordinal);
|
||||
if (type.BaseType is not null)
|
||||
{
|
||||
AddTypeNames(type.BaseType, signature);
|
||||
}
|
||||
|
||||
foreach (InterfaceImplementation implemented in type.Interfaces)
|
||||
{
|
||||
AddTypeNames(implemented.InterfaceType, signature);
|
||||
}
|
||||
|
||||
foreach (FieldDefinition field in type.Fields)
|
||||
{
|
||||
AddTypeNames(field.FieldType, signature);
|
||||
}
|
||||
|
||||
if (signature.Count > 0)
|
||||
{
|
||||
sites.Add(new Site(name, type.FullName, "(type signature)", OpCodes.Nop, null, signature));
|
||||
}
|
||||
|
||||
foreach (MethodDefinition method in type.Methods)
|
||||
{
|
||||
var methodTypes = new HashSet<string>(StringComparer.Ordinal);
|
||||
AddTypeNames(method.ReturnType, methodTypes);
|
||||
foreach (ParameterDefinition parameter in method.Parameters)
|
||||
{
|
||||
AddTypeNames(parameter.ParameterType, methodTypes);
|
||||
}
|
||||
|
||||
if (!method.HasBody)
|
||||
{
|
||||
sites.Add(new Site(name, type.FullName, method.Name, OpCodes.Nop, null, methodTypes));
|
||||
continue;
|
||||
}
|
||||
|
||||
foreach (VariableDefinition variable in method.Body.Variables)
|
||||
{
|
||||
AddTypeNames(variable.VariableType, methodTypes);
|
||||
}
|
||||
|
||||
sites.Add(new Site(name, type.FullName, method.Name, OpCodes.Nop, null, methodTypes));
|
||||
|
||||
foreach (Instruction instruction in method.Body.Instructions)
|
||||
{
|
||||
var referenced = new HashSet<string>(StringComparer.Ordinal);
|
||||
MethodReference? callee = null;
|
||||
switch (instruction.Operand)
|
||||
{
|
||||
case MethodReference methodReference:
|
||||
callee = methodReference;
|
||||
AddMethodTypeNames(methodReference, referenced);
|
||||
break;
|
||||
case FieldReference fieldReference:
|
||||
AddTypeNames(fieldReference.DeclaringType, referenced);
|
||||
AddTypeNames(fieldReference.FieldType, referenced);
|
||||
break;
|
||||
case TypeReference typeReference:
|
||||
AddTypeNames(typeReference, referenced);
|
||||
break;
|
||||
default:
|
||||
continue;
|
||||
}
|
||||
|
||||
sites.Add(new Site(name, type.FullName, method.Name, instruction.OpCode, callee, referenced));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return sites;
|
||||
}
|
||||
|
||||
private static void AddMethodTypeNames(MethodReference method, HashSet<string> names)
|
||||
{
|
||||
AddTypeNames(method.DeclaringType, names);
|
||||
AddTypeNames(method.ReturnType, names);
|
||||
foreach (ParameterDefinition parameter in method.Parameters)
|
||||
{
|
||||
AddTypeNames(parameter.ParameterType, names);
|
||||
}
|
||||
|
||||
if (method is GenericInstanceMethod generic)
|
||||
{
|
||||
foreach (TypeReference argument in generic.GenericArguments)
|
||||
{
|
||||
AddTypeNames(argument, names);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static void AddTypeNames(TypeReference type, HashSet<string> names)
|
||||
{
|
||||
switch (type)
|
||||
{
|
||||
case GenericParameter:
|
||||
return;
|
||||
case GenericInstanceType generic:
|
||||
AddTypeNames(generic.ElementType, names);
|
||||
foreach (TypeReference argument in generic.GenericArguments)
|
||||
{
|
||||
AddTypeNames(argument, names);
|
||||
}
|
||||
|
||||
return;
|
||||
case TypeSpecification specification:
|
||||
// arrays, by-ref, pointers, modifiers
|
||||
AddTypeNames(specification.ElementType, names);
|
||||
return;
|
||||
default:
|
||||
names.Add(TypeName(type));
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Namespace-qualified for nested types too: Cecil's FullName for a nested type is "Outer/Inner" and
|
||||
// carries the outer type's namespace, so prefix checks on the namespace still apply.
|
||||
private static string TypeName(TypeReference type) => type.GetElementType().FullName;
|
||||
|
||||
/// <summary>
|
||||
/// Non-test projects in <c>ErsatzTV.sln</c> whose ProjectReference closure contains a direct
|
||||
/// <c>SixLabors.ImageSharp</c> PackageReference, as assembly names.
|
||||
/// </summary>
|
||||
private static List<string> ImageSharpReachingAssemblies()
|
||||
{
|
||||
string root = RepoRoot();
|
||||
string solution = File.ReadAllText(Path.Combine(root, "ErsatzTV.sln"));
|
||||
|
||||
List<string> projects = System.Text.RegularExpressions.Regex
|
||||
.Matches(solution, "^Project\\(\"[^\"]+\"\\) = \"[^\"]+\", \"([^\"]+\\.csproj)\"", System.Text.RegularExpressions.RegexOptions.Multiline)
|
||||
.Select(m => Path.GetFullPath(Path.Combine(root, m.Groups[1].Value.Replace('\\', Path.DirectorySeparatorChar))))
|
||||
.ToList();
|
||||
|
||||
// sanity: the solution parse found the projects at all
|
||||
projects.Count.ShouldBeGreaterThan(5, "could not parse the project list out of ErsatzTV.sln");
|
||||
|
||||
var memo = new Dictionary<string, bool>(StringComparer.Ordinal);
|
||||
return projects
|
||||
.Where(p => !Path.GetFileNameWithoutExtension(p).EndsWith(".Tests", StringComparison.Ordinal))
|
||||
.Where(p => ReachesImageSharp(p, memo, []))
|
||||
.Select(AssemblyName)
|
||||
.OrderBy(n => n, StringComparer.Ordinal)
|
||||
.ToList();
|
||||
}
|
||||
|
||||
private static bool ReachesImageSharp(string csproj, Dictionary<string, bool> memo, HashSet<string> visiting)
|
||||
{
|
||||
if (memo.TryGetValue(csproj, out bool known))
|
||||
{
|
||||
return known;
|
||||
}
|
||||
|
||||
if (!visiting.Add(csproj))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
XDocument document = XDocument.Load(csproj);
|
||||
bool direct = document.Descendants("PackageReference")
|
||||
.Any(e => string.Equals((string?)e.Attribute("Include"), ImageSharpPackage, StringComparison.OrdinalIgnoreCase));
|
||||
|
||||
string directory = Path.GetDirectoryName(csproj)!;
|
||||
bool result = direct || document.Descendants("ProjectReference")
|
||||
.Select(e => (string?)e.Attribute("Include"))
|
||||
.OfType<string>()
|
||||
.Select(include => Path.GetFullPath(Path.Combine(directory, include.Replace('\\', Path.DirectorySeparatorChar))))
|
||||
.Any(reference => ReachesImageSharp(reference, memo, visiting));
|
||||
|
||||
memo[csproj] = result;
|
||||
return result;
|
||||
}
|
||||
|
||||
private static string AssemblyName(string csproj)
|
||||
{
|
||||
string? declared = XDocument.Load(csproj).Descendants("AssemblyName").Select(e => e.Value.Trim()).FirstOrDefault();
|
||||
return string.IsNullOrEmpty(declared) ? Path.GetFileNameWithoutExtension(csproj) : declared;
|
||||
}
|
||||
|
||||
private static string RepoRoot()
|
||||
{
|
||||
var directory = new DirectoryInfo(AppContext.BaseDirectory);
|
||||
while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "ErsatzTV.sln")))
|
||||
{
|
||||
directory = directory.Parent;
|
||||
}
|
||||
|
||||
directory.ShouldNotBeNull("could not locate the repository root (no ErsatzTV.sln above the test binary)");
|
||||
return directory!.FullName;
|
||||
}
|
||||
|
||||
private sealed record Site(
|
||||
string Assembly,
|
||||
string CallerType,
|
||||
string CallerMethod,
|
||||
OpCode OpCode,
|
||||
MethodReference? Callee,
|
||||
IReadOnlySet<string> ReferencedTypes)
|
||||
{
|
||||
public string Describe() =>
|
||||
$"{Assembly}: {CallerType}::{CallerMethod}"
|
||||
+ (Callee is null ? $" [{string.Join(", ", ReferencedTypes.Where(IsForbiddenType))}]" : $" -> {Callee.FullName}");
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,325 @@
|
||||
using System.Buffers.Binary;
|
||||
using ErsatzTV.Infrastructure.Images;
|
||||
using NUnit.Framework;
|
||||
using Shouldly;
|
||||
using SixLabors.ImageSharp;
|
||||
using SixLabors.ImageSharp.Formats;
|
||||
using SixLabors.ImageSharp.Formats.Bmp;
|
||||
using SixLabors.ImageSharp.Formats.Jpeg;
|
||||
using SixLabors.ImageSharp.Formats.Png;
|
||||
using SixLabors.ImageSharp.Formats.Tiff;
|
||||
using SixLabors.ImageSharp.Formats.Webp;
|
||||
using SixLabors.ImageSharp.Metadata.Profiles.Icc;
|
||||
using SixLabors.ImageSharp.PixelFormats;
|
||||
using SixLabors.ImageSharp.Processing;
|
||||
using Image = SixLabors.ImageSharp.Image;
|
||||
|
||||
namespace ErsatzTV.Infrastructure.Tests.Images;
|
||||
|
||||
/// <summary>
|
||||
/// ImageSharp 3.1.12's TIFF decoder carries GHSA-v76p-62qx-wwq2 and GHSA-wmxv-xphr-5c9g, and the
|
||||
/// patched releases need a Six Labors license key to build in Release, so every ImageSharp decode
|
||||
/// goes through a configuration that cannot detect TIFF. (ersatztv#929,
|
||||
/// security.imagesharp-3x-tiff-decoder-disabled)
|
||||
/// </summary>
|
||||
[TestFixture]
|
||||
public class ImageSharpDecodingTests
|
||||
{
|
||||
private static readonly Uri Uri = new("https://example.com/logo.tif");
|
||||
|
||||
// the list is derived from Configuration.Default, not restated: a package bump that adds or
|
||||
// drops a format reddens this instead of silently changing what ErsatzTV can decode
|
||||
[Test]
|
||||
public void Configuration_Registers_Every_Default_Format_Except_Tiff()
|
||||
{
|
||||
var expected = Configuration.Default.ImageFormats
|
||||
.Where(f => f is not TiffFormat)
|
||||
.Select(f => f.Name)
|
||||
.OrderBy(n => n, StringComparer.Ordinal)
|
||||
.ToList();
|
||||
|
||||
var actual = ImageSharpDecoding.Configuration.ImageFormats
|
||||
.Select(f => f.Name)
|
||||
.OrderBy(n => n, StringComparer.Ordinal)
|
||||
.ToList();
|
||||
|
||||
actual.ShouldBe(expected);
|
||||
}
|
||||
|
||||
// the exclusion above is only meaningful while Default really does carry TIFF, and building
|
||||
// the restricted configuration must not have removed it from the process-wide Default
|
||||
[Test]
|
||||
public void Configuration_Default_Still_Registers_Tiff()
|
||||
{
|
||||
_ = ImageSharpDecoding.Configuration;
|
||||
Configuration.Default.ImageFormats.ShouldContain(f => f is TiffFormat);
|
||||
}
|
||||
|
||||
// positive control for the rejections below: the fixture is a TIFF ImageSharp can decode,
|
||||
// so a rejection is the configuration's doing and not a broken fixture
|
||||
[Test]
|
||||
public async Task Tiff_Fixture_Decodes_Under_The_Default_Configuration()
|
||||
{
|
||||
await using MemoryStream stream = await RealTiff(64, 32);
|
||||
using Image image = await Image.LoadAsync(stream);
|
||||
image.Width.ShouldBe(64);
|
||||
image.Height.ShouldBe(32);
|
||||
}
|
||||
|
||||
// the validator as a whole (render path, save path and upload path). The Identify pass rejects
|
||||
// a TIFF before the Load pass runs, so this exercises Identify; that the Load pass takes the
|
||||
// same options is held by ErsatzTV.Architecture.Tests' ImageSharpSurfaceTests.
|
||||
[Test]
|
||||
public async Task DecodeAndValidate_Rejects_A_Tiff_As_An_Unknown_Format()
|
||||
{
|
||||
await using MemoryStream stream = await RealTiff(64, 32);
|
||||
await Should.ThrowAsync<UnknownImageFormatException>(
|
||||
() => RemoteImageValidator.DecodeAndValidate(stream, Uri, CancellationToken.None));
|
||||
}
|
||||
|
||||
// pins the IDENTIFY pass separately. The header declares 30000x30000: an Identify that could
|
||||
// read TIFF would report those dimensions and the budget check would throw
|
||||
// InvalidOperationException("pixel limit") instead, so only a TIFF-blind Identify yields
|
||||
// UnknownImageFormatException here. The IFD walk Identify performs is where the BigTIFF
|
||||
// non-progressing loop (GHSA-wmxv-xphr-5c9g) lives, so the header pass must not reach it.
|
||||
[Test]
|
||||
public async Task DecodeAndValidate_Rejects_A_Tiff_Before_Reading_Its_Header()
|
||||
{
|
||||
await using MemoryStream stream = await TiffDeclaring(30000, 30000);
|
||||
|
||||
ImageInfo info = await Image.IdentifyAsync(stream);
|
||||
info.Width.ShouldBe(30000);
|
||||
stream.Position = 0;
|
||||
|
||||
await Should.ThrowAsync<UnknownImageFormatException>(
|
||||
() => RemoteImageValidator.DecodeAndValidate(stream, Uri, CancellationToken.None));
|
||||
}
|
||||
|
||||
// --- GHSA-gwg2-r3hj-4w44: the ICC CLUT parser allocates from unvalidated grid dimensions ---
|
||||
//
|
||||
// ErsatzTV never reads ICC entries (ImageSharpSurfaceTests bans the namespace), but decode,
|
||||
// CloneFrame, Resize and CloneAs all carry the profile along. Whether any of them parses it is
|
||||
// ImageSharp's own runtime behaviour, which an IL scan of ErsatzTV cannot see, so it is pinned
|
||||
// here by allocation. A version that parses the profile on decode or clone reddens these
|
||||
// instead of reopening the advisory silently.
|
||||
|
||||
private const long HostileIccEntriesFloor = 64L * 1024 * 1024;
|
||||
internal const long ProductPathCeiling = 16L * 1024 * 1024;
|
||||
|
||||
internal static readonly string[] IccCarriers = ["bmp", "jpeg", "png", "webp"];
|
||||
|
||||
// the carrier list is checked, not trusted: every format the decode configuration registers is
|
||||
// round-tripped with the hostile profile through its default encoder, and the formats whose
|
||||
// decode still carries a profile must be exactly IccCarriers. A version that adds ICC support to
|
||||
// another format reddens here. A round trip cannot see a decoder that reads a profile its own
|
||||
// encoder never writes; on 3.1.12 the decoders that construct an IccProfile are BMP, JPEG, PNG,
|
||||
// WebP and TIFF (Cecil scan of the package, 2026-10-28), so none is missed today.
|
||||
[Test]
|
||||
public async Task Icc_Carriers_Are_Every_Configured_Format_That_Round_Trips_A_Profile()
|
||||
{
|
||||
var carriers = new List<string>();
|
||||
foreach (IImageFormat format in ImageSharpDecoding.Configuration.ImageFormats)
|
||||
{
|
||||
IImageEncoder encoder = Configuration.Default.ImageFormatsManager.GetEncoder(format);
|
||||
await using MemoryStream stream = await WithHostileIcc(encoder);
|
||||
using Image image = await Image.LoadAsync(ImageSharpDecoding.Options(), stream);
|
||||
if (image.Metadata.IccProfile is not null)
|
||||
{
|
||||
carriers.Add(format.Name.ToLowerInvariant());
|
||||
}
|
||||
}
|
||||
|
||||
carriers.Order(StringComparer.Ordinal).ShouldBe(IccCarriers.Order(StringComparer.Ordinal));
|
||||
}
|
||||
|
||||
// positive control: the fixture really is hostile. Reading Entries runs the CLUT reader on the
|
||||
// declared 255-point, 3-input grid (about 127 MiB on 3.1.12, measured 2026-10-28); the catch
|
||||
// only tolerates the truncated table throwing afterwards.
|
||||
[TestCaseSource(nameof(IccCarriers))]
|
||||
[NonParallelizable]
|
||||
public async Task Hostile_Icc_Fixture_Allocates_When_Its_Entries_Are_Parsed(string format)
|
||||
{
|
||||
await using MemoryStream stream = await WithHostileIcc(format);
|
||||
using Image image = await Image.LoadAsync(ImageSharpDecoding.Options(), stream);
|
||||
IccProfile profile = image.Metadata.IccProfile.ShouldNotBeNull();
|
||||
|
||||
long allocated = AllocatedDuring(() =>
|
||||
{
|
||||
try
|
||||
{
|
||||
_ = profile.Entries;
|
||||
}
|
||||
catch (Exception)
|
||||
{
|
||||
// the allocation is what is measured, not whether the truncated table throws
|
||||
}
|
||||
});
|
||||
|
||||
TestContext.Out.WriteLine($"{format}: reading Entries allocated {allocated / 1024} KiB");
|
||||
allocated.ShouldBeGreaterThan(HostileIccEntriesFloor);
|
||||
}
|
||||
|
||||
// the product path: the validator's two decodes, then the per-frame CloneFrame + Resize +
|
||||
// CloneAs that ImageElementBase runs. The profile must survive the path (so the bound is not
|
||||
// vacuous because a decoder dropped it) and nothing on it may size the CLUT.
|
||||
[TestCaseSource(nameof(IccCarriers))]
|
||||
[NonParallelizable]
|
||||
public async Task Decode_Clone_And_Resize_Do_Not_Parse_A_Hostile_Icc_Profile(string format)
|
||||
{
|
||||
await using MemoryStream stream = await WithHostileIcc(format);
|
||||
|
||||
long allocated = await AllocatedDuringAsync(async () =>
|
||||
{
|
||||
using Image image = await RemoteImageValidator.DecodeAndValidate(stream, Uri, CancellationToken.None);
|
||||
image.Metadata.IccProfile.ShouldNotBeNull();
|
||||
|
||||
using Image frame = image.Frames.CloneFrame(0);
|
||||
frame.Mutate(ctx => ctx.Resize(8, 8));
|
||||
frame.Metadata.IccProfile.ShouldNotBeNull();
|
||||
|
||||
using Image<Rgba32> rgba = frame.CloneAs<Rgba32>();
|
||||
rgba.Metadata.IccProfile.ShouldNotBeNull();
|
||||
});
|
||||
|
||||
TestContext.Out.WriteLine($"{format}: decode, clone and resize allocated {allocated / 1024} KiB");
|
||||
allocated.ShouldBeLessThan(ProductPathCeiling);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A 16x16 image in <paramref name="format" /> carrying an ICC profile whose A2B0 tag is an
|
||||
/// <c>mft2</c> (lut16) with 3 inputs, 15 outputs and 255 grid points, truncated before the
|
||||
/// CLUT: a 208-byte profile.
|
||||
/// </summary>
|
||||
internal static Task<MemoryStream> WithHostileIcc(string format) =>
|
||||
WithHostileIcc(
|
||||
format switch
|
||||
{
|
||||
"bmp" => new BmpEncoder(),
|
||||
"jpeg" => new JpegEncoder(),
|
||||
"png" => new PngEncoder(),
|
||||
"webp" => new WebpEncoder { FileFormat = WebpFileFormatType.Lossless },
|
||||
_ => throw new ArgumentOutOfRangeException(nameof(format), format, null)
|
||||
});
|
||||
|
||||
private static async Task<MemoryStream> WithHostileIcc(IImageEncoder encoder)
|
||||
{
|
||||
using var image = new Image<Rgba32>(16, 16);
|
||||
image.Metadata.IccProfile = new IccProfile(HostileIccBytes());
|
||||
|
||||
var stream = new MemoryStream();
|
||||
await image.SaveAsync(stream, encoder);
|
||||
stream.Position = 0;
|
||||
return stream;
|
||||
}
|
||||
|
||||
private static byte[] HostileIccBytes()
|
||||
{
|
||||
// mft2: type, reserved, in/out/grid/pad, 3x3 matrix, input/output table entry counts,
|
||||
// then the three 2-entry input tables, and no CLUT
|
||||
var tag = new List<byte>();
|
||||
tag.AddRange("mft2"u8.ToArray());
|
||||
tag.AddRange(new byte[4]);
|
||||
tag.AddRange([3, 15, 255, 0]);
|
||||
for (var i = 0; i < 9; i++)
|
||||
{
|
||||
tag.AddRange(BigEndian(i % 4 == 0 ? 0x00010000u : 0u));
|
||||
}
|
||||
|
||||
tag.AddRange([0, 2, 0, 2]);
|
||||
tag.AddRange(new byte[3 * 2 * 2]);
|
||||
|
||||
const int TagOffset = 128 + 4 + 12;
|
||||
int size = TagOffset + tag.Count;
|
||||
|
||||
var bytes = new byte[size];
|
||||
BinaryPrimitives.WriteUInt32BigEndian(bytes.AsSpan(0), (uint)size);
|
||||
BinaryPrimitives.WriteUInt32BigEndian(bytes.AsSpan(8), 0x02100000u);
|
||||
"mntr"u8.CopyTo(bytes.AsSpan(12));
|
||||
"RGB "u8.CopyTo(bytes.AsSpan(16));
|
||||
"XYZ "u8.CopyTo(bytes.AsSpan(20));
|
||||
"acsp"u8.CopyTo(bytes.AsSpan(36));
|
||||
BinaryPrimitives.WriteUInt32BigEndian(bytes.AsSpan(128), 1);
|
||||
"A2B0"u8.CopyTo(bytes.AsSpan(132));
|
||||
BinaryPrimitives.WriteUInt32BigEndian(bytes.AsSpan(136), TagOffset);
|
||||
BinaryPrimitives.WriteUInt32BigEndian(bytes.AsSpan(140), (uint)tag.Count);
|
||||
tag.CopyTo(bytes, TagOffset);
|
||||
return bytes;
|
||||
}
|
||||
|
||||
private static byte[] BigEndian(uint value)
|
||||
{
|
||||
var bytes = new byte[4];
|
||||
BinaryPrimitives.WriteUInt32BigEndian(bytes, value);
|
||||
return bytes;
|
||||
}
|
||||
|
||||
// process-wide and precise, because the async path may resume on another thread; the callers
|
||||
// are [NonParallelizable] so no other test's allocations land in the count
|
||||
private static long AllocatedDuring(Action action)
|
||||
{
|
||||
long before = GC.GetTotalAllocatedBytes(precise: true);
|
||||
action();
|
||||
return GC.GetTotalAllocatedBytes(precise: true) - before;
|
||||
}
|
||||
|
||||
private static async Task<long> AllocatedDuringAsync(Func<Task> action)
|
||||
{
|
||||
long before = GC.GetTotalAllocatedBytes(precise: true);
|
||||
await action();
|
||||
return GC.GetTotalAllocatedBytes(precise: true) - before;
|
||||
}
|
||||
|
||||
internal static async Task<MemoryStream> RealTiff(int width, int height)
|
||||
{
|
||||
using var image = new Image<Rgba32>(width, height);
|
||||
var stream = new MemoryStream();
|
||||
await image.SaveAsync(stream, new TiffEncoder());
|
||||
stream.Position = 0;
|
||||
return stream;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A real TIFF whose ImageWidth/ImageLength IFD entries are rewritten to declare
|
||||
/// <paramref name="width" /> x <paramref name="height" />, so Identify reports a size the
|
||||
/// pixel data does not back.
|
||||
/// </summary>
|
||||
private static async Task<MemoryStream> TiffDeclaring(int width, int height)
|
||||
{
|
||||
await using MemoryStream source = await RealTiff(8, 8);
|
||||
byte[] bytes = source.ToArray();
|
||||
|
||||
// TiffEncoder writes little-endian ("II"); the first IFD offset is at byte 4
|
||||
bytes[0].ShouldBe((byte)'I');
|
||||
var ifd = (int)BinaryPrimitives.ReadUInt32LittleEndian(bytes.AsSpan(4));
|
||||
int count = BinaryPrimitives.ReadUInt16LittleEndian(bytes.AsSpan(ifd));
|
||||
|
||||
var patched = 0;
|
||||
for (var i = 0; i < count; i++)
|
||||
{
|
||||
int entry = ifd + 2 + i * 12;
|
||||
ushort tag = BinaryPrimitives.ReadUInt16LittleEndian(bytes.AsSpan(entry));
|
||||
ushort type = BinaryPrimitives.ReadUInt16LittleEndian(bytes.AsSpan(entry + 2));
|
||||
if (tag is not (256 or 257))
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
int value = tag == 256 ? width : height;
|
||||
Span<byte> slot = bytes.AsSpan(entry + 8, 4);
|
||||
slot.Clear();
|
||||
if (type == 3)
|
||||
{
|
||||
BinaryPrimitives.WriteUInt16LittleEndian(slot, checked((ushort)value));
|
||||
}
|
||||
else
|
||||
{
|
||||
BinaryPrimitives.WriteUInt32LittleEndian(slot, (uint)value);
|
||||
}
|
||||
|
||||
patched++;
|
||||
}
|
||||
|
||||
patched.ShouldBe(2);
|
||||
return new MemoryStream(bytes);
|
||||
}
|
||||
}
|
||||
@@ -4,6 +4,7 @@ using ErsatzTV.Core.Interfaces.Streaming;
|
||||
using ErsatzTV.FFmpeg;
|
||||
using ErsatzTV.FFmpeg.State;
|
||||
using ErsatzTV.Infrastructure.Streaming.Graphics;
|
||||
using ErsatzTV.Infrastructure.Tests.Images;
|
||||
using LanguageExt;
|
||||
using SixLabors.ImageSharp;
|
||||
using SixLabors.ImageSharp.Formats.Png;
|
||||
@@ -129,6 +130,71 @@ public class WatermarkElementRemoteImageTests
|
||||
element.IsFinished.ShouldBeFalse();
|
||||
}
|
||||
|
||||
// --- TIFF is not decoded on either path (ersatztv#929, security.imagesharp-3x-tiff-decoder-disabled) ---
|
||||
//
|
||||
// Each TIFF case has a PNG twin that differs only in the encoder, so a disabled element is the
|
||||
// decoder configuration's doing, not a broken fixture or a path that never loads.
|
||||
|
||||
[TestCase(true, ExpectedResult = true)]
|
||||
[TestCase(false, ExpectedResult = false)]
|
||||
public async Task<bool> Should_Disable_The_Watermark_For_A_Remote_Tiff_Only(bool tiff)
|
||||
{
|
||||
MemoryStream source = tiff ? await ImageSharpDecodingTests.RealTiff(64, 64) : Apng(64, 64, 1);
|
||||
var element = new WatermarkElement(RemoteWatermarkOptions(), FetcherReturning(source), Substitute.For<ILogger>());
|
||||
|
||||
await element.InitializeAsync(Context(), CancellationToken.None);
|
||||
|
||||
return element.IsFinished;
|
||||
}
|
||||
|
||||
[TestCase(true, ExpectedResult = true)]
|
||||
[TestCase(false, ExpectedResult = false)]
|
||||
public async Task<bool> Should_Disable_The_Watermark_For_A_Local_Tiff_Only(bool tiff)
|
||||
{
|
||||
string path = Path.Combine(Path.GetTempPath(), $"etv-929-{Guid.NewGuid():N}.{(tiff ? "tif" : "png")}");
|
||||
try
|
||||
{
|
||||
await using (MemoryStream source = tiff ? await ImageSharpDecodingTests.RealTiff(64, 64) : Apng(64, 64, 1))
|
||||
{
|
||||
await File.WriteAllBytesAsync(path, source.ToArray());
|
||||
}
|
||||
|
||||
var options = new WatermarkOptions(Watermark(), path, Option<int>.None);
|
||||
var element = new WatermarkElement(options, Substitute.For<IRemoteImageFetcher>(), Substitute.For<ILogger>());
|
||||
|
||||
await element.InitializeAsync(Context(), CancellationToken.None);
|
||||
|
||||
return element.IsFinished;
|
||||
}
|
||||
finally
|
||||
{
|
||||
File.Delete(path);
|
||||
}
|
||||
}
|
||||
|
||||
// GHSA-gwg2-r3hj-4w44 over the element itself rather than a replica of its calls: decode,
|
||||
// CloneFrame, Resize and CloneAs carry a hostile ICC profile through without sizing its CLUT.
|
||||
// The element's images are private, so this case cannot assert the profile is still attached; it
|
||||
// shares its fixture with ImageSharpDecodingTests, whose
|
||||
// Decode_Clone_And_Resize_Do_Not_Parse_A_Hostile_Icc_Profile asserts attachment after each step and
|
||||
// whose Hostile_Icc_Fixture_Allocates_When_Its_Entries_Are_Parsed proves the fixture hostile.
|
||||
// (ersatztv#929, security.imagesharp-3x-tiff-decoder-disabled)
|
||||
[TestCaseSource(typeof(ImageSharpDecodingTests), nameof(ImageSharpDecodingTests.IccCarriers))]
|
||||
[NonParallelizable]
|
||||
public async Task Should_Render_A_Remote_Watermark_Without_Parsing_Its_Icc_Profile(string format)
|
||||
{
|
||||
MemoryStream source = await ImageSharpDecodingTests.WithHostileIcc(format);
|
||||
var element = new WatermarkElement(RemoteWatermarkOptions(), FetcherReturning(source), Substitute.For<ILogger>());
|
||||
|
||||
long before = GC.GetTotalAllocatedBytes(precise: true);
|
||||
await element.InitializeAsync(Context(), CancellationToken.None);
|
||||
long allocated = GC.GetTotalAllocatedBytes(precise: true) - before;
|
||||
|
||||
TestContext.Out.WriteLine($"{format}: watermark initialization allocated {allocated / 1024} KiB");
|
||||
element.IsFinished.ShouldBeFalse();
|
||||
allocated.ShouldBeLessThan(ImageSharpDecodingTests.ProductPathCeiling);
|
||||
}
|
||||
|
||||
private static IRemoteImageFetcher FetcherReturning(Stream stream)
|
||||
{
|
||||
var fetcher = Substitute.For<IRemoteImageFetcher>();
|
||||
|
||||
@@ -36,6 +36,10 @@
|
||||
<PackageReference Include="Scriban.Signed" />
|
||||
<PackageReference Include="SixLabors.ImageSharp" />
|
||||
<PackageReference Include="SkiaSharp" />
|
||||
<!-- Direct ref forces the patched System.Security.Cryptography.Pkcs over the vulnerable 9.0.4
|
||||
that EFCore.BulkExtensions' Oracle/SqlServer providers pull transitively (#929).
|
||||
Version lives in Directory.Packages.props (CPM). -->
|
||||
<PackageReference Include="System.Security.Cryptography.Pkcs" />
|
||||
<PackageReference Include="TagLibSharp" />
|
||||
<PackageReference Include="TimeZoneConverter" />
|
||||
</ItemGroup>
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
using SixLabors.ImageSharp;
|
||||
using SixLabors.ImageSharp.Formats;
|
||||
using SixLabors.ImageSharp.Formats.Bmp;
|
||||
using SixLabors.ImageSharp.Formats.Gif;
|
||||
using SixLabors.ImageSharp.Formats.Jpeg;
|
||||
using SixLabors.ImageSharp.Formats.Pbm;
|
||||
using SixLabors.ImageSharp.Formats.Png;
|
||||
using SixLabors.ImageSharp.Formats.Qoi;
|
||||
using SixLabors.ImageSharp.Formats.Tga;
|
||||
using SixLabors.ImageSharp.Formats.Webp;
|
||||
|
||||
namespace ErsatzTV.Infrastructure.Images;
|
||||
|
||||
/// <summary>
|
||||
/// The ImageSharp <see cref="Configuration" /> every ImageSharp decode in ErsatzTV goes through.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// It registers every format <see cref="Configuration.Default" /> registers EXCEPT TIFF, so a TIFF is
|
||||
/// rejected with <see cref="UnknownImageFormatException" /> before any TIFF decoder code runs.
|
||||
/// ImageSharp 3.1.12's TIFF decoder carries GHSA-v76p-62qx-wwq2 (heap out-of-bounds write from a tiled
|
||||
/// fax TIFF) and GHSA-wmxv-xphr-5c9g (non-progressing BigTIFF IFD loop). TIFF is outside the artwork
|
||||
/// allow-list (<c>ImageContentTypes.Accepted</c>), so what this removes is only a remote-URL or on-disk
|
||||
/// TIFF graphic. It is a fresh instance rather than a <c>Configuration.Default.Clone()</c> because
|
||||
/// Clone is shallow: removing a detector from the clone would remove it from Default too.
|
||||
/// <c>ImageSharpDecodingTests</c> pins the module list against <c>Configuration.Default</c>, so a
|
||||
/// package bump that adds a format fails a test instead of silently dropping it, and
|
||||
/// <c>ImageSharpSurfaceTests</c> (ErsatzTV.Architecture.Tests) fails any production decode that does not
|
||||
/// take these options.
|
||||
/// Rationale and revisit trigger: <c>security.imagesharp-3x-tiff-decoder-disabled</c>. (ersatztv#929)
|
||||
/// </remarks>
|
||||
public static class ImageSharpDecoding
|
||||
{
|
||||
public static Configuration Configuration { get; } = new(
|
||||
new PngConfigurationModule(),
|
||||
new JpegConfigurationModule(),
|
||||
new GifConfigurationModule(),
|
||||
new BmpConfigurationModule(),
|
||||
new PbmConfigurationModule(),
|
||||
new TgaConfigurationModule(),
|
||||
new WebpConfigurationModule(),
|
||||
new QoiConfigurationModule());
|
||||
|
||||
public static DecoderOptions Options() => new() { Configuration = Configuration };
|
||||
|
||||
public static DecoderOptions Options(uint maxFrames) =>
|
||||
new() { Configuration = Configuration, MaxFrames = maxFrames };
|
||||
}
|
||||
@@ -41,8 +41,11 @@ public class RemoteImageValidator : IRemoteImageValidator
|
||||
// PngEncoder wrote, which Image.Load then reads back perfectly (measured: 13 of 16 shapes).
|
||||
// Without this, adding the header pre-pass would silently disable every animated-PNG logo
|
||||
// that worked before this change. Only Width/Height are read below, and those stay correct.
|
||||
// Both passes use ImageSharpDecoding's configuration, which cannot detect TIFF (ersatztv#929).
|
||||
// This pass is the one that rejects a TIFF; the decode pass takes the same configuration so
|
||||
// the two always agree on which formats exist (held by ImageSharpSurfaceTests).
|
||||
ImageInfo info = await Image.IdentifyAsync(
|
||||
new DecoderOptions { MaxFrames = 1 },
|
||||
ImageSharpDecoding.Options(maxFrames: 1),
|
||||
stream,
|
||||
cancellationToken);
|
||||
|
||||
@@ -58,12 +61,12 @@ public class RemoteImageValidator : IRemoteImageValidator
|
||||
stream.Position = 0;
|
||||
|
||||
// MaxFrames is enforced BY THE DECODER, so it holds whatever the header claimed — measured
|
||||
// as honored by every animated decoder here (APNG, GIF, WebP, TIFF). Ask for two more than
|
||||
// as honored by every animated decoder here (APNG, GIF, WebP). Ask for two more than
|
||||
// the budget allows so that an animation exactly AT the limit still decodes in full, while
|
||||
// anything over it is present in the decoded image for the post-decode check below to
|
||||
// reject. Slop is at most two frames: MaxFrames = N yields N frames for GIF/WebP/TIFF but
|
||||
// reject. Slop is at most two frames: MaxFrames = N yields N frames for GIF/WebP but
|
||||
// N-1 for APNG, so the exact count varies by format and only the upper bound matters.
|
||||
var decoderOptions = new DecoderOptions { MaxFrames = (uint)(affordableFrames + 2) };
|
||||
DecoderOptions decoderOptions = ImageSharpDecoding.Options(maxFrames: (uint)(affordableFrames + 2));
|
||||
|
||||
Image image = await Image.LoadAsync(decoderOptions, stream, cancellationToken);
|
||||
|
||||
|
||||
@@ -67,7 +67,7 @@ public abstract class ImageElementBase(IRemoteImageFetcher remoteImageFetcher) :
|
||||
}
|
||||
else
|
||||
{
|
||||
_sourceImage = await Image.LoadAsync(image!, cancellationToken);
|
||||
_sourceImage = await Image.LoadAsync(ImageSharpDecoding.Options(), image!, cancellationToken);
|
||||
}
|
||||
|
||||
int scaledWidth = _sourceImage.Width;
|
||||
|
||||
+48
-22
@@ -1304,7 +1304,7 @@ minutes, and it would still be wrong to filter on `scripts/**`.
|
||||
stay unpinned on purpose: a pytest release does not add assertions to your suite, a ruff release
|
||||
adds rules to your lint.
|
||||
- Lint runs **before the jq preflight**, and after the `git` one. `Preflight jq version` is a hard
|
||||
`--expect` tripwire; a lint sitting behind it goes dark for as long as the jq contract is broken,
|
||||
`--expect` tripwire; a lint sitting behind it goes dark for as long as the jq version is outside the accepted set,
|
||||
under a red that says "jq". `Preflight external tools` stays ahead, because the lint steps consume
|
||||
`git` — without it a missing git reaches them as an empty population and they blame the glob.
|
||||
- Neither step is `ruff check .`. Both pass an **explicit population** from
|
||||
@@ -1331,7 +1331,7 @@ which is itself a small demonstration of why the suite needed to run in CI at al
|
||||
against a temp repo it creates itself, never this repository's history.
|
||||
|
||||
Two **preflight steps** run before the suite. The first asserts `git` is on PATH; the second runs
|
||||
`scripts/jq-preflight.sh --expect 1.6`, which checks jq's **version**, not merely its presence (see
|
||||
`scripts/jq-preflight.sh --expect 1.6,1.7`, which checks jq's **version**, not merely its presence (see
|
||||
"The jq contract" below). Those two tests exec the real shell scripts, which shell out to `jq` ~26
|
||||
times; the tests shim `curl` on PATH but not `jq`, so a runner image without it would surface as ~20
|
||||
opaque assertion failures instead of one diagnosis. Both deliberately **check** rather than install —
|
||||
@@ -1340,13 +1340,16 @@ the runner image.
|
||||
|
||||
### The jq contract (ersatztv#648)
|
||||
|
||||
> Full rationale: `docs/decisions/records/ci/jq-version-contract.md`.
|
||||
> Full rationale: `docs/decisions/records/ci/jq-accepted-version-set.md` (`ci.jq-accepted-version-set`).
|
||||
|
||||
Every shell gate in this repo — `decisions-guard`, `script-tests`'s own harness,
|
||||
`pretooluse-merge-consent.sh`, `review-verdict.yml`, `scripts/pr-changed-files.sh` — is authored and
|
||||
tested on a developer Mac shipping **jq 1.8.x**. The CI runner ships **jq 1.6**. Author to the
|
||||
1.6-compatible subset; three concrete constructs diverge between the two and each one produced a real
|
||||
bug when it hit CI for the first time:
|
||||
tested on a developer Mac shipping **jq 1.8.x**. The CI runners' jq is **not pinned**: both `small`
|
||||
runners map to the floating `runner-images:ubuntu-latest` image with no digest pin, so each host keeps
|
||||
whatever it last pulled. Measured 2026-10-28: jq 1.7 on both hosts (PR #933 job 12347, PR #934 job
|
||||
12360); no known runner reports 1.6, but an unpinned host could (server-management#1080). Author to the
|
||||
1.6-compatible subset, which is the **floor**; three concrete constructs diverge between 1.6 and
|
||||
later versions and each one produced a real bug when it hit CI for the first time:
|
||||
|
||||
- **`jq -e` over EMPTY input.** Exits 4 on jq >= 1.7, but **0** on jq 1.6. A guard that infers
|
||||
"transport failure" from that exit status silently passes an empty/failed page on 1.6.
|
||||
@@ -1361,16 +1364,25 @@ bug when it hit CI for the first time:
|
||||
the parsed version and asserts a floor of 1.6) so a future divergence can be diagnosed from the log
|
||||
alone instead of guessing at the runner image.
|
||||
|
||||
**Pin vs floor is deliberately asymmetric.** `scripts/jq-preflight.sh --expect 1.6` additionally pins
|
||||
the version and fails loudly if it drifts, but that mode is used **only** by `script-tests`
|
||||
(`.gitea/workflows/pr-checks.yml`) — advisory, not a required check. `review-verdict.yml` runs the
|
||||
no-args floor-only mode and never pins, because that workflow writes `review-verdict/h10`, the
|
||||
branch-protection-**required** status check on `main`: a hard pin there would mean the day the
|
||||
runner's jq version changes (a base-image bump, a host reimage — nothing this repo controls), every
|
||||
PR on `main` stops merging until someone notices and re-pins. A required merge gate cannot fail
|
||||
because an upstream package manager did its job. The narrower pin on `script-tests` exists precisely
|
||||
because that job is the suite's only 1.6 coverage — if the runner's jq silently changed, that coverage
|
||||
would evaporate with no signal, so failing loudly there forces a human decision instead.
|
||||
**Accepted set, not a single pin (2026-10-29, ersatztv#935).** `scripts/jq-preflight.sh --expect
|
||||
1.6,1.7` additionally asserts the version is a member of the comma-separated set and fails loudly
|
||||
(exit 1) on any other version; a malformed set (`1.6,`, `1.x`) is a usage error (exit 2). It is used
|
||||
**only** by `script-tests` (`.gitea/workflows/pr-checks.yml`). The set is exactly {1.6, 1.7}: a
|
||||
1.8 runner stays red so a person decides. As of 2026-10-28 no known runner provides
|
||||
jq 1.6, so **real-jq 1.6 coverage of `scripts/tests/` is currently absent**; 1.6 stays in the set only
|
||||
because the tag is unpinned. In-repo jq shims (`test_pr_changed_files.py`,
|
||||
`test_merge_consent_exemption.py`) reproduce the empty-input `-e` quirk only; the NUL-truncation and
|
||||
parse-error-exit-code quirks have no coverage on a 1.7 runner. The floor stays at 1.6. **server-management#1080** (a fleet digest
|
||||
pin) is what narrows the set: when it settles one version, change `--expect` to that single version.
|
||||
The tripwire is advisory in name only: `pretooluse-merge-consent.sh` reads the combined status, so a
|
||||
red `script-tests` blocks the hook-mediated merge path until the `--expect` value is fixed.
|
||||
|
||||
**Pin vs floor is deliberately asymmetric.** `review-verdict.yml` runs the no-args floor-only mode and
|
||||
never `--expect`, because that workflow writes `review-verdict/h10`, the branch-protection-**required**
|
||||
status check on `main`: a version assertion there would mean the day a runner's jq version changes (a
|
||||
base-image pull, a host reimage — nothing this repo controls), every PR on `main` stops merging until
|
||||
someone notices and re-pins. A required merge gate cannot fail because an upstream package manager did
|
||||
its job.
|
||||
|
||||
Baking a pinned jq into `docker/ci/Dockerfile` was considered and rejected: `review-verdict.yml` is
|
||||
`runs-on: small` with no toolchain-image pin, and per `ci.small-lane-git-only` the small lane is
|
||||
@@ -2594,7 +2606,16 @@ The advisories that prompted this were resolved in ersatztv#8 (NCalcSync→6.x;
|
||||
bundle 3.x) and ersatztv#314 (**Microsoft.OpenApi 2.0.0→2.7.5**, GHSA-v5pm-xwqc-g5wc High —
|
||||
direct-pinned in `ErsatzTV.csproj` over the 2.0.0 that `Microsoft.AspNetCore.OpenApi` +
|
||||
`Scalar.AspNetCore` pull transitively; the SQLitePCLRaw override pattern; regenerates the
|
||||
OpenAPI doc byte-identically). The NU1901-1903 demotion is **kept by design**: criticals (NU1904)
|
||||
OpenAPI doc byte-identically) and ersatztv#929 (**System.Security.Cryptography.Pkcs 9.0.4→9.0.21**,
|
||||
GHSA-846f-f764-76mx High — direct-referenced in `ErsatzTV.Infrastructure.csproj`, the only project
|
||||
that references `EFCore.BulkExtensions`, whose Oracle/SqlServer providers pull it in). **Override a
|
||||
transitive with a direct reference, not `CentralPackageTransitivePinningEnabled`**: the central
|
||||
transitive pin was measured 2026-10-27 to fail restore with NU1109 in every project, because it pins
|
||||
every transitive that has a `<PackageVersion>`, and the exact `Microsoft.Extensions.DependencyModel`
|
||||
`[8.0.2]` sits below the `>= 9.0.x` that EF Core requires. **SixLabors.ImageSharp is held at 3.1.12**:
|
||||
every patched release fails a Release build without a Six Labors license key — rationale, the
|
||||
per-advisory bounds and the TIFF decoder exclusion are in `security.imagesharp-3x-tiff-decoder-disabled`.
|
||||
The NU1901-1903 demotion is **kept by design**: criticals (NU1904)
|
||||
still hard-block, while low/moderate/high advisories surface as warnings + via the weekly scan and
|
||||
Renovate security PRs, rather than breaking unrelated PRs the moment a new transitive
|
||||
advisory drops.
|
||||
@@ -2607,9 +2628,11 @@ greps for the "has the following vulnerable packages" marker and fails the run i
|
||||
Detection only — it surfaces advisories on a schedule, a Gitea-native stand-in for
|
||||
Dependabot; it does **not** open update PRs (that's Renovate — server-management#484).
|
||||
Gitea registers `schedule` triggers only from the default branch, so the cron starts
|
||||
after merge to `main`; use `workflow_dispatch` to run on demand. It went **green** once
|
||||
ersatztv#8 cleared the NCalcSync/SQLitePCLRaw advisories — a red run now means a **new**
|
||||
advisory has appeared. (ersatztv#14, ersatztv#8)
|
||||
after merge to `main`; use `workflow_dispatch` to run on demand. Since 2026-10-27 it is **red by
|
||||
design on `SixLabors.ImageSharp` 3.1.12** until a Six Labors license key allows the patched line
|
||||
(`security.imagesharp-3x-tiff-decoder-disabled`); `dotnet list` ignores `NuGetAuditSuppress`, so
|
||||
the run cannot be greened by suppression. Read a red run's log for a package OTHER than ImageSharp:
|
||||
that is a new advisory. (ersatztv#14, ersatztv#8, ersatztv#929)
|
||||
|
||||
**Renovate (automated update PRs)** — `.gitea/workflows/renovate.yml` runs self-hosted
|
||||
[Renovate](https://docs.renovatebot.com) weekly (cron `0 3 * * 1`) + on `workflow_dispatch`,
|
||||
@@ -2627,8 +2650,11 @@ github.com PAT for changelogs — named `GH_`, **not** `GITHUB_`, a prefix Gitea
|
||||
**Patch** bumps to test/dev-only packages (NUnit\*, NSubstitute, Shouldly, coverlet,
|
||||
`Microsoft.NET.Test.Sdk`, `Testably.Abstractions*`, threading analyzer) **auto-merge** once
|
||||
the `Build & test (.NET)` check passes — branch protection on `main` requires that context;
|
||||
everything else is manual review (ersatztv is prod-bearing). Range-pinned packages (e.g. EF
|
||||
Core `[9.0.x,10)`) are respected — no v10 jump. PR volume is throttled (`prConcurrentLimit`
|
||||
everything else is manual review (ersatztv is prod-bearing). A version range in
|
||||
`Directory.Packages.props` does **not** hold Renovate back: the Dependency Dashboard (#22,
|
||||
2026-10-27) offers EF Core `[9.0.12,10)` → v10 and `MediatR [12.5.0]` → `[14.2.0]`. A package that
|
||||
must not move gets an `allowedVersions` packageRule in `renovate.json` (today: `SixLabors.ImageSharp`,
|
||||
`security.imagesharp-3x-tiff-decoder-disabled`). PR volume is throttled (`prConcurrentLimit`
|
||||
5 + `config:recommended`'s `prHourlyLimit` 2); tick a dashboard checkbox or raise the limits
|
||||
to drain faster. `workflow_dispatch` defaults to a safe **dry run**. Cross-repo rollout
|
||||
tracked in server-management#484. (server-management#484)
|
||||
|
||||
@@ -57,7 +57,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
|
||||
| `ci.grep-q-pipefail-inversion` | In any script running under `set -o pipefail`, a security or classification predicate of the form `producer \| grep -q…` is FORBIDDEN: `grep -q` exits at its first match, the producer then takes SIGPIPE and exits 141 once the data exceeds the pipe buffer (~64K), so `pipefail` reports the pipeline as FAILED even though grep MATCHED — inverting the predicate exactly when the input is large. A here-string (`grep -q… <<< "$data"`) is ALSO forbidden: bash materialises a large here-string via temporary storage, so it fails when temp space is full or unwritable, and inside an `if`/`!` that failure flips the predicate the same way. COUNT instead — `n=$(printf '%s\n' "$data" \| grep -cE "$re")` — because `grep -c` drains stdin (no early exit, no SIGPIPE) over an ordinary pipe (no temp file). Read grep's status honestly: exit 1 means a zero count and is a legitimate answer, anything >1 is a real error. Evaluate the counts ONCE at TOP LEVEL, never inline inside an `if`/`elif` condition: inside `$( )` an `exit` leaves only the subshell and `set -e` does not fire, so an error silently reads as "no match". Validate that each result is numeric and fail closed if not. This applies to both the enforced gate `.gitea/workflows/review-verdict.yml` and the advisory hook `.claude/hooks/pretooluse-merge-consent.sh`. | 2026-07-29 | [link](records/ci/grep-q-pipefail-inversion.md) |
|
||||
| `ci.image-build-delegates-the-spa-suite` | `docker/Dockerfile` runs no vitest suite. Its web-build stage lints, typechecks and BUILDS the SPA; the suite runs once, unfiltered, in `docker-build.yml`'s `test` job on a real checkout, and `build` carries `needs: [test, migrations, scan]` so no image is published past a red suite. THAT EDGE IS NOW THE ONLY LAYER, so the guard checks it is real and not merely present — and it does so by PINNING TEXT rather than parsing it. The commands each SPA-carrying Dockerfile stage runs, and the gating step's `run:` body and `if:`, are compared as strings against a declared pin; the step and its job must carry no `continue-on-error` in any spelling, the job no job-level `if:`, and the publish step must keep its own `docs_only` gate. A guard that asks what a command MEANS was wrong nine times in three review rounds. A pin is immune to a different SPELLING of the command, which is that whole class; it is not immune to the same text meaning something else, so the routes to that are pinned or refused too: `working-directory` and the step's own `shell:`, the workflow `defaults.run.shell` and the job-level `defaults` overriding it, a stage `SHELL`, `web/package.json`'s script map (pinned WHOLE — selecting on the literal `vitest` missed `npm run test`, `npm t` and the `prebuild`/`preinstall` lifecycle hooks), `web/vite.config.ts` PINNED WHOLE — it decides what the suite collects and what `vite build` loads, and pinning a BLOCK of it was defeated seven measured ways across three rounds, so the partial match was withdrawn rather than respelled an eighth time; and the ABSENCE of any config file that outranks it — `vitest.config.*` for vitest, and `vite.config.js`/`.mjs` for `vite build`, whose `DEFAULT_CONFIG_FILES` lists them BEFORE `.ts`. Pinning one file is worthless while a second can outrank it, and that family went short twice. The gating job's `container:` image is left to `ci.image-pin-population` rather than guarded twice. What is unmodelled is a LIST, not an "only": an `ENV` changing what a pinned `RUN` resolves, the plugin BODIES, and a publish through another action. The count of such routes lives in the `docs/guard-inventory.md` row and nowhere else — it briefly existed in three places with two different values, which is how a number stays wrong. It is a running total, wrong at every count so far. They share one shape, which is the transferable part: A PIN ASSUMES IT IS PINNING THE ARTIFACT THAT STILL DECIDES, and every route found so far is authority moving to another file, another occurrence, another workflow, or a hook the pinned command invokes. Do NOT restore a filtered in-image run by naming the specs that cannot run there: that list is a population nothing derives, and the red it produces when it goes stale is unreachable on a PR — `Build & push image (amd64)` is `if: github.event_name != 'pull_request'` — so it lands on `main` and on the `v*` tag path, where it fails the release cut. Held in both directions by `scripts/tests/test_image_build_delegates_the_spa_suite.py`. | 2026-08-30 | [link](records/ci/image-build-delegates-the-spa-suite.md) |
|
||||
| `ci.infra-shaped-red-under-load` | When a job dies inside a setup/cache step before your code compiles, check the runner host's load before diagnosing the diff, and never file a CI bug off one sample under pressure. | 2026-07-21 | [link](records/ci/infra-shaped-red-under-load.md) |
|
||||
| `ci.jq-version-contract` | Every shell gate that shells out to `jq` is authored to the jq 1.6-compatible subset, because the CI runner ships jq 1.6 while every developer Mac ships 1.8.x. `scripts/jq-preflight.sh` (no args) prints the parsed version and asserts a floor of 1.6 in every gate job's log; `scripts/jq-preflight.sh --expect 1.6` additionally pins and fails loudly, but ONLY in the `script-tests` job. `review-verdict.yml` never pins — it writes the branch-protection-required `review-verdict/h10` status, so a hard pin there would turn any jq bump into a repo-wide merge deadlock. | 2026-07-26 | [link](records/ci/jq-version-contract.md) |
|
||||
| `ci.jq-accepted-version-set` | Every shell gate that shells out to `jq` is authored to the jq 1.6-compatible subset (floor 1.6, unchanged). Both `small` runners map to the floating `runner-images:ubuntu-latest` image with no digest pin, so the fleet jq version is not guaranteed; measured 2026-10-28, both hosts report jq 1.7 and no known runner reports 1.6. `scripts/jq-preflight.sh --expect 1.6,1.7` in the `script-tests` job accepts exactly that comma-separated SET and fails loudly on any other version; 1.6 stays in the set only because the tag is unpinned, and real-jq 1.6 coverage of `scripts/tests/` is currently absent. server-management#1080 (fleet digest pin) is the event that narrows the set to one version. `review-verdict.yml` stays floor-only (no `--expect`): it writes the branch-protection-required `review-verdict/h10` status. | 2026-10-29 | [link](records/ci/jq-accepted-version-set.md) |
|
||||
| `ci.killed-job-triage` | Never trust a job's `conclusion` field alone — read the log tail and require an `❌ Failure - Main …` marker before treating a red as a real failure. | 2026-07-21 | [link](records/ci/killed-job-triage.md) |
|
||||
| `ci.monitor-armed-at-pr-open` | Arm a CI monitor on the PR head sha the moment the PR opens, polling the commit-status endpoint — not at the end of the work. | 2026-07-21 | [link](records/ci/monitor-armed-at-pr-open.md) |
|
||||
| `ci.no-host-health-gating` | Push when your work is validated — never SSH to bumblebee to sample load/RAM first, and never hand-schedule around other sessions' runs. | 2026-07-21 | [link](records/ci/no-host-health-gating.md) |
|
||||
@@ -184,6 +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, 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) |
|
||||
|
||||
+3
-3
@@ -1,11 +1,11 @@
|
||||
---
|
||||
key: ci.jq-version-contract
|
||||
title: '2026-07-26 — jq 1.6 is the FLOOR every shell gate must run on; `scripts/jq-preflight.sh` makes the version observable, and only `script-tests` pins it (#648)'
|
||||
status: active
|
||||
status: superseded
|
||||
since: '2026-07-26'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
rule: 'Every shell gate that shells out to `jq` is authored to the jq 1.6-compatible subset, because the CI runner ships jq 1.6 while every developer Mac ships 1.8.x. `scripts/jq-preflight.sh` (no args) prints the parsed version and asserts a floor of 1.6 in every gate job''s log; `scripts/jq-preflight.sh --expect 1.6` additionally pins and fails loudly, but ONLY in the `script-tests` job. `review-verdict.yml` never pins — it writes the branch-protection-required `review-verdict/h10` status, so a hard pin there would turn any jq bump into a repo-wide merge deadlock.'
|
||||
superseded-by: ci.jq-accepted-version-set@2026-10-29
|
||||
rule: '(superseded) Every shell gate that shells out to `jq` is authored to the jq 1.6-compatible subset, because the CI runner ships jq 1.6 while every developer Mac ships 1.8.x. `scripts/jq-preflight.sh` (no args) prints the parsed version and asserts a floor of 1.6 in every gate job''s log; `scripts/jq-preflight.sh --expect 1.6` additionally pins and fails loudly, but ONLY in the `script-tests` job. `review-verdict.yml` never pins — it writes the branch-protection-required `review-verdict/h10` status, so a hard pin there would turn any jq bump into a repo-wide merge deadlock.'
|
||||
signals: 'jq version divergence, jq 1.6 vs 1.8, jq -e exit code on empty input, contains NUL false positive, jq parse-error exit code collision, jq-preflight, script-tests --expect, review-verdict jq floor, merge deadlock from a pinned dependency · paths: `scripts/jq-preflight.sh`, `.gitea/workflows/review-verdict.yml`, `.gitea/workflows/pr-checks.yml`, `docs/ci-cd.md` · issues: #643, #647, #648, #649'
|
||||
mechanics: '`scripts/jq-preflight.sh` (no args) -> floor+observability in every gate job; `scripts/jq-preflight.sh --expect 1.6` -> tripwire, `script-tests` job only; `docs/ci-cd.md` -> "The jq contract"'
|
||||
---
|
||||
@@ -0,0 +1,80 @@
|
||||
---
|
||||
key: ci.jq-accepted-version-set
|
||||
title: '2026-10-29 — jq 1.6 stays the FLOOR; `script-tests` accepts exactly the SET {1.6, 1.7} until server-management#1080 pins the runner fleet (#935)'
|
||||
status: active
|
||||
since: '2026-10-29'
|
||||
supersedes: ci.jq-version-contract@2026-07-26
|
||||
superseded-by: none
|
||||
rule: 'Every shell gate that shells out to `jq` is authored to the jq 1.6-compatible subset (floor 1.6, unchanged). Both `small` runners map to the floating `runner-images:ubuntu-latest` image with no digest pin, so the fleet jq version is not guaranteed; measured 2026-10-28, both hosts report jq 1.7 and no known runner reports 1.6. `scripts/jq-preflight.sh --expect 1.6,1.7` in the `script-tests` job accepts exactly that comma-separated SET and fails loudly on any other version; 1.6 stays in the set only because the tag is unpinned, and real-jq 1.6 coverage of `scripts/tests/` is currently absent. server-management#1080 (fleet digest pin) is the event that narrows the set to one version. `review-verdict.yml` stays floor-only (no `--expect`): it writes the branch-protection-required `review-verdict/h10` status.'
|
||||
signals: 'jq version divergence, jq 1.6 vs 1.7, script-tests jq tripwire red, jq-1.7 (parsed 1.7; floor 1.6), --expect set, unpinned runner image, floating runner-images ubuntu-latest, server-management#1080, jq-preflight, merge blocked by combined status · paths: `scripts/jq-preflight.sh`, `scripts/tests/test_jq_preflight.py`, `.gitea/workflows/pr-checks.yml`, `.gitea/workflows/review-verdict.yml`, `docs/ci-cd.md` · issues: #935, #648, #643, #647'
|
||||
mechanics: '`scripts/jq-preflight.sh` (no args) -> floor+observability in every gate job; `scripts/jq-preflight.sh --expect 1.6,1.7` -> set tripwire, `script-tests` job only (`test_jq_preflight.py` pins the exact value); `docs/ci-cd.md` -> "The jq contract"'
|
||||
---
|
||||
|
||||
Supersedes `ci.jq-version-contract` (2026-07-26, archived), whose premise was that the CI runner
|
||||
ships jq 1.6. That is no longer a fact the repo can state.
|
||||
|
||||
**Measured 2026-10-28.** `script-tests`' tripwire (`--expect 1.6`) went red on jazz-small-runner
|
||||
(PR #933, job 12347) and on bumblebee's small-runner (PR #934, job 12360), each reporting
|
||||
`jq-1.7 (parsed 1.7; floor 1.6)`. Cause (server-management#1080): both `small` runners map to the
|
||||
floating `runner-images:ubuntu-latest` image with no digest pin or forced pull, so each host keeps
|
||||
whatever it last pulled and the fleet's jq version is a function of pull history. A digest pin across
|
||||
the fleet is proposed in server-management#1080 and not yet decided.
|
||||
|
||||
**The invariant.** `script-tests` accepts exactly the set `{1.6, 1.7}`:
|
||||
|
||||
- `scripts/jq-preflight.sh --expect 1.6,1.7` parses the argument as a comma-separated set of
|
||||
`<major>.<minor>` members; an empty or free-form member (`1.6,`, `1.6,,1.7`, `1.x`) is a usage
|
||||
error (exit 2), never a member that matches nothing or everything. Membership is exact on
|
||||
`major.minor`, not a prefix or range test (`1.10` and `1.71` do not match `1.7` / `1.6`).
|
||||
- A version outside the set (`1.8`, which every developer Mac ships, `2.x`) exits 1 with the tripwire
|
||||
message. That red is witnessed by `scripts/tests/test_jq_preflight.py`
|
||||
(`test_a_version_outside_the_accepted_set_is_LOUD`), and the exact `--expect 1.6,1.7` value in
|
||||
`pr-checks.yml` is pinned by `test_script_tests_pins_the_jq_version`.
|
||||
- The set is exactly {1.6, 1.7}, not "1.6 or newer": widening it to every version would delete the
|
||||
tripwire. Admitting 1.7 is evidence-driven (both measured hosts run it); admitting 1.8 is not, and
|
||||
a 1.8 runner should go red so a person decides.
|
||||
|
||||
**The floor stays at 1.6.** Raising it would assert that no host in the fleet can report 1.6, which
|
||||
the unpinned image does not guarantee, and which only the #1080 decision can establish. A gate
|
||||
authored to the 1.6 subset is harmless on 1.7, and the three divergences that motivated the contract
|
||||
(`jq -e` over empty input, `contains("\u0000")`, the parse-error exit code) remain real on whichever
|
||||
host still runs 1.6.
|
||||
|
||||
**What coverage is lost.** As of 2026-10-28 no known runner provides jq 1.6, so the real-jq 1.6 run
|
||||
of `scripts/tests/` is currently absent; it would happen only if a host held an older image. In-repo
|
||||
jq shims reproduce ONE of the three 1.6 divergences: `jq -e` over empty input exiting 0
|
||||
(`test_pr_changed_files.py`, `test_merge_consent_exemption.py`). The `contains("\u0000")` truncation
|
||||
and the parse-error exit code (4 vs 5) have no shim, so on a 1.7 run they are covered only by the
|
||||
constructs having been replaced with version-stable ones, and any NEW 1.6-only divergence is
|
||||
uncovered.
|
||||
|
||||
**What narrows the set.** server-management#1080. When it pins the fleet to one jq version, change
|
||||
the `--expect` value in `pr-checks.yml` to that single version (the script accepts a one-member set)
|
||||
and record the pin here by superseding this record. If the pin lands on 1.7, decide then whether the
|
||||
floor rises, with the fleet evidence in hand.
|
||||
|
||||
**Unchanged from the superseded record.**
|
||||
|
||||
- *Pin vs floor is asymmetric.* `review-verdict.yml` runs the no-args floor-only mode and never
|
||||
`--expect`: it writes `review-verdict/h10`, the branch-protection-required status on `main`, so a
|
||||
version assertion there would turn any runner jq change into a repo-wide merge deadlock.
|
||||
`test_review_verdict_never_pins_a_jq_version` holds that.
|
||||
- *Firing the tripwire blocks merges.* `.claude/hooks/pretooluse-merge-consent.sh` reads the combined
|
||||
commit status and denies on anything not `success`/`skipped` (ersatztv#598), and `script-tests` is a Gitea Actions job folded into it. Only the
|
||||
`merge_when_checks_succeed` path, which defers to Gitea's required checks, is spared. This is the
|
||||
cost of the tripwire, accepted because the remedy is a one-line edit to `--expect`, and the
|
||||
2026-10-28 firing is that cost being paid.
|
||||
- *Baking jq into `docker/ci/Dockerfile` cannot reach the gate.* `review-verdict.yml` is
|
||||
`runs-on: small` with no toolchain-image pin and `ci.small-lane-git-only` keeps that lane
|
||||
git-only, so it gets the host's jq whatever the toolchain image holds. The fleet pin in
|
||||
server-management#1080 is the only lever that reaches it.
|
||||
- *The parse is strictly fail-closed.* `jq-preflight.sh` accepts only a first line of the form
|
||||
`jq-<X>.<Y>` or `jq version <X>.<Y>`; anything else exits 1. If a wrapper banner ever breaks that
|
||||
in the required `review-verdict.yml` job, widen the accepted forms, never relax fail-closed: an
|
||||
unparsed version must not satisfy the floor.
|
||||
- *The three constructs to avoid:* never infer "empty input" from a jq exit status (check the string
|
||||
in shell first); never use `contains("\u0000")` (use `explode | index(0)`); never read `jq`'s
|
||||
exit code as a specific parse-error signal on ambiguous input (validate the response shape).
|
||||
|
||||
Full rationale for how the three divergences were found is in
|
||||
`docs/decisions/archive/ci/jq-version-contract.md` and `docs/decisions/records/ci/script-tests-job.md`.
|
||||
@@ -91,7 +91,7 @@ a judgement that type checking does not belong.
|
||||
|
||||
**The pin is the second half of the fix.** An unpinned `pip install ruff` re-introduces exactly the
|
||||
divergence the config closes, one layer up: the verdict becomes a function of *when* the job ran. Same
|
||||
argument as the `jq` pin in the same job (`ci.jq-version-contract`), and the same consequence — a bump
|
||||
argument as the `jq` pin in the same job (`ci.jq-accepted-version-set`), and the same consequence — a bump
|
||||
is a PR someone reads. `pytest` and `pyyaml` in the same job stay unpinned, and the
|
||||
asymmetry is the point rather than an oversight: a pytest release does not add assertions to your
|
||||
suite, a ruff release adds rules to your lint.
|
||||
|
||||
@@ -56,7 +56,7 @@ both reds were real:
|
||||
PyYAML). It passed locally only because the author's machine had PyYAML.
|
||||
2. A **fail-open in the merge-consent gate itself.** `jq -e` over EMPTY input exits 4 on jq ≥ 1.7
|
||||
but **0 on jq 1.6** (verified against both binaries), and the docs-only pagination guard leaned
|
||||
on that exit status to reject a transport failure. On jq 1.6 — which the CI runner ships — the
|
||||
on that exit status to reject a transport failure. On jq 1.6 — which the CI runner shipped on 2026-07-26 — the
|
||||
failed page passed the guard, the loop walked past it, the next page legitimately returned `[]`,
|
||||
and `files_complete=yes` was set over a PARTIAL list: the docs-only exemption firing over unread
|
||||
pages that may be pure code. Fixed by rejecting an empty body explicitly instead of inferring it
|
||||
@@ -68,7 +68,7 @@ on a developer Mac (jq 1.8) — only in CI, where the suite had never run. Two f
|
||||
the same shape (a `contains("\u0000")` false positive and a colliding parse-error exit code) turned
|
||||
up the same day; the durable contract that came out of all three — the exact constructs to avoid, and
|
||||
why `jq-preflight.sh` pins in `script-tests` but only floors the version in `review-verdict.yml` — is
|
||||
recorded once, in `ci.jq-version-contract` (`docs/decisions/records/ci/jq-version-contract.md`), and
|
||||
recorded once, in `ci.jq-accepted-version-set` (`docs/decisions/records/ci/jq-accepted-version-set.md`), and
|
||||
is not restated here.
|
||||
|
||||
An independent cross-family review of that fix then found **two further fail-opens in the same
|
||||
|
||||
@@ -131,7 +131,7 @@ Two reasons it read as deliberate rather than broken, both worth carrying forwar
|
||||
other.
|
||||
|
||||
The type is now read as a value (`case` over `jq -r 'type'`) rather than through `jq -e`, whose
|
||||
exit-status semantics already bit this workflow at jq 1.6 (`ci.jq-version-contract`), and both `null`
|
||||
exit-status semantics already bit this workflow at jq 1.6 (`ci.jq-accepted-version-set`), and both `null`
|
||||
and `[]` are read as the same EMPTY shape (which since #870 ends the walk only at page 1 or at the
|
||||
page cap, not in between). `test_the_fence_TRUSTS_the_count_and_POSTS_when_the_timeline_terminates`
|
||||
is parameterised over both shapes and asserts the POSTED STATUS rather than the log line — on the real
|
||||
|
||||
@@ -57,7 +57,8 @@ image descriptor exceeds its logical screen is clamped by the decoder, verified)
|
||||
frames of that size the budget affords; passes that to **`DecoderOptions.MaxFrames`**, which the
|
||||
decoder enforces regardless of what the header claimed; and then **re-verifies the real
|
||||
`image.Frames.Count`** after decoding, disposing and rejecting if it is over. `MaxFrames` was
|
||||
measured as honored by every animated decoder in play (APNG, GIF, WebP, TIFF), which is what makes
|
||||
measured on 3.1.12 as honored by every animated decoder then in play (APNG, GIF, WebP, TIFF; TIFF is
|
||||
no longer decodable at all — `security.imagesharp-3x-tiff-decoder-disabled`), which is what makes
|
||||
it a real bound rather than another advisory one. The code asks for `affordable + 2` so an
|
||||
animation exactly at the limit still decodes in full while anything over it is visible to the
|
||||
post-decode check; the slop is at most two frames, since `MaxFrames = N` yields `N` frames for
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
---
|
||||
key: security.imagesharp-3x-tiff-decoder-disabled
|
||||
title: 2026-10-27 — ImageSharp stays on 3.1.12 because every patched release needs a license key to build in Release; every ImageSharp decode goes through a TIFF-blind configuration (#929)
|
||||
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, 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'
|
||||
---
|
||||
|
||||
**Why 3.x is kept.** The ImageSharp releases that patch the October 2026 advisories — 3.2.0 on the
|
||||
3.x line, 4.1.2 and 4.1.3 on 4.x — ship an MSBuild target
|
||||
(`SixLabors_ValidateLicense`, before `CoreCompile`) that fails a non-Debug build unless
|
||||
`SixLaborsLicenseKey`, `SixLaborsLicenseFile` or a `sixlabors.lic` is present. Measured 2026-10-27: `dotnet build ErsatzTV.Infrastructure -c Release` errors with
|
||||
`No Six Labors license found` on 3.2.0 and on 4.1.3; a Debug build only warns. The 4.0.0 and 4.1.2
|
||||
packages carry the same `SixLabors.Licensing` task; 3.1.12 is the newest release without it. CI's `test`/`build`
|
||||
jobs and the Docker image all build Release, so a bare version bump (Renovate #923) turns every PR
|
||||
red. A key is application-based (Six Labors offers a free Community licence below USD 1M revenue),
|
||||
so obtaining one is an owner decision, not a code change. The licence TEXT is unchanged across
|
||||
3.1.12/3.2.0/4.1.3 (Six Labors Split License 1.0, byte-identical) — the build gate is what is new.
|
||||
|
||||
Disabling the target, or setting a placeholder key, is **rejected**: it defeats a licence check
|
||||
rather than satisfying it.
|
||||
|
||||
**What bounds each 3.1.12 advisory** (`dotnet list package --vulnerable --include-transitive`,
|
||||
2026-10-27; all fixed in 3.2.0):
|
||||
|
||||
| Advisory | Where the defect is | Why ErsatzTV does not reach it |
|
||||
|---|---|---|
|
||||
| GHSA-v76p-62qx-wwq2 (High) | TIFF decoder, tiled fax heap OOB write | TIFF is not detectable by `ImageSharpDecoding.Configuration`; a TIFF throws `UnknownImageFormatException` before any TIFF code runs |
|
||||
| GHSA-wmxv-xphr-5c9g (Moderate) | TIFF decoder, BigTIFF IFD loop | same — and the Identify pass, which walks the IFD, is the pass the tests pin |
|
||||
| GHSA-jjfr-hcj7-qf5w, GHSA-j9gm-c75j-xc9q (High) | TIFF CCITT T6/T4 **encoder** | production code never encodes TIFF (only test fixtures do) |
|
||||
| GHSA-j3p4-wp97-rph4 (High) | `HistogramEqualization` processor | never called; the only processor is `Resize` |
|
||||
| GHSA-gwg2-r3hj-4w44 (Moderate) | ICC CLUT parser | two halves. ErsatzTV never reads `IccProfile.Entries`, the accessor that runs the parser. And on 3.1.12 decode, `CloneFrame`, `Resize` and `CloneAs` carry a profile through without parsing it — ImageSharp's own runtime behaviour, measured 2026-10-28 |
|
||||
|
||||
The ImageSharp decode surface is exactly `RemoteImageValidator.DecodeAndValidate` (remote logos at
|
||||
render time, logo URLs saved via #525, uploaded artwork) and the local-path load in
|
||||
`ImageElementBase.LoadImage`. Artwork elsewhere goes through SkiaSharp, and uploads are already
|
||||
clamped to PNG/JPEG/GIF/WebP by `ImageContentTypes`, so the only input this rejects is a remote-URL
|
||||
or on-disk TIFF graphic, which now disables its element the same way an unreachable URL does.
|
||||
|
||||
**What holds the bounds.** Every bound in the table except one half of gwg2 is a property of how
|
||||
ErsatzTV calls ImageSharp, so each is enforced over the compiled code rather than left to review;
|
||||
the gwg2 runtime half is pinned by allocation (below).
|
||||
`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 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
|
||||
header-only TIFF declaring 30000x30000 exercises the Identify pass (a TIFF-aware Identify would
|
||||
report the size and fail the pixel budget instead). The watermark fixture exercises the local and
|
||||
remote element paths with PNG twins. Identify rejects a TIFF before Load runs, so these
|
||||
behavioural fixtures exercise the Identify pass; that the Load pass uses the same options is held
|
||||
by `ImageSharpSurfaceTests`.
|
||||
|
||||
The gwg2 runtime half is ImageSharp code an IL scan of ErsatzTV cannot see, so it is pinned by
|
||||
behaviour. A 208-byte ICC profile declares an `mft2` CLUT of 255 grid points over 3 inputs, and it is
|
||||
carried in every format whose decoder reads a profile: BMP, JPEG, PNG and WebP. That carrier list is
|
||||
checked rather than trusted: each format the decode configuration registers is round-tripped with
|
||||
the profile through its default encoder, and the formats that come back carrying it must be exactly
|
||||
the list, so a version that adds ICC to another format reddens. A round trip cannot see a decoder
|
||||
that reads a profile its own encoder never writes; on 3.1.12 the decoders that construct an
|
||||
`IccProfile` are BMP, JPEG, PNG, WebP and TIFF (Cecil scan of the package, 2026-10-28).
|
||||
`ImageSharpDecodingTests` sends each carrier through the validator plus `CloneFrame`/`Resize`/`CloneAs`,
|
||||
and the watermark fixture through a real remote `WatermarkElement`; both fail above 16 MiB allocated.
|
||||
The validator cases assert that the profile is still attached after decode, `CloneFrame`, `Resize`
|
||||
and `CloneAs`, so a decoder that drops it cannot pass them vacuously. The element case cannot see
|
||||
its private images and asserts only the allocation; it shares the validator cases' fixture. The
|
||||
positive control reads `Entries` on the same profile and must allocate over 64 MiB (about 127 MiB on
|
||||
3.1.12). So a version that starts parsing profiles on decode or clone reddens.
|
||||
|
||||
One shape is outside every check: a `Save(path)` that picks its
|
||||
encoder from the file extension. There is no ImageSharp encode in production code (2026-10-27).
|
||||
|
||||
**The scheduled scan stays red on ImageSharp until the revisit.** `NuGetAuditSuppress` silences the
|
||||
NU1902/NU1903 restore warnings but `dotnet list package --vulnerable` ignores it (measured
|
||||
2026-10-27: a suppressed GHSA still listed), so suppression would hide the advisory from build logs
|
||||
without making `dependency-scan.yml` green. None is added; the advisories stay visible.
|
||||
|
||||
**Renovate is held too.** A `renovate.json` packageRule limits `SixLabors.ImageSharp` to `3.1.x`
|
||||
for ordinary update PRs. A version range in `Directory.Packages.props` would not: the Dependency
|
||||
Dashboard (#22, 2026-10-27) offers `MediatR [12.5.0]` → `[14.2.0]` and EF Core `[9.0.12,10)` → v10
|
||||
for the existing range pins. Whether the rule also holds Renovate's OSV security PR (#923) is
|
||||
unmeasured — Renovate reads its config from `main`, so the rule cannot be probed before merge.
|
||||
|
||||
**On revisit.** With a key provisioned (a CI secret passed as `SixLaborsLicenseKey`, and the same
|
||||
for the Docker build), delete the Renovate rule and move to the patched line. The #511 measurements in
|
||||
`ffmpeg.remote-image-fetcher-bounded` (APNG `FrameMetadataCollection.Count == 0`, the default
|
||||
Identify throwing on APNGs, `MaxFrames` honoured per format) were taken on 3.1.12 and must be
|
||||
re-measured on the new version before the workaround comments are trusted. Whether TIFF comes back
|
||||
is a separate call: nothing in the product needs it.
|
||||
@@ -320,7 +320,7 @@ different fixes, and conflating them is why several of these recurred.
|
||||
| **B** | **Every guard ships with a proof it can go red: delete or disarm *that guard alone* and the suite must fail.** Not "a test exists" — a mutation. | Family B: 9 records (13%) | **rule-present-unenforced** — stated in #685's record and in project memory, enforced nowhere |
|
||||
| **C** | **Dedup by construction.** When two copies of one rule exist, delete one and have both callers invoke it — *unless* deriving the list would feed the judged input back into the judge, which is #711's reasoned exception. | Family C: 9 records (13%) | **rule-missing** |
|
||||
| **D** | **Anything read-then-written against live Gitea/remote state pins a version or uses compare-and-set.** See the honesty note below — this is a fix pattern, and its *detector* is detector A applied to an enumerated inventory of such sites. | Family D: 5 records (7%) | **rule-missing** (partially addressed for the merge path by the per-sha required check from #622) |
|
||||
| **E** | **Run the check under the interpreter/provider it will actually run under**, and preflight-log the version. | Environment divergence: 7 records | **rule-present** and working — `scripts/jq-preflight.sh` + `ci.jq-version-contract` closed the jq axis after #643/#647/#648. **Unclosed axes**: SQLite-vs-MySQL query semantics (#668), GPU generation (#505), CI-VM speed (#512) |
|
||||
| **E** | **Run the check under the interpreter/provider it will actually run under**, and preflight-log the version. | Environment divergence: 7 records | **rule-present** and working — `scripts/jq-preflight.sh` + `ci.jq-accepted-version-set` closed the jq axis after #643/#647/#648. **Unclosed axes**: SQLite-vs-MySQL query semantics (#668), GPU generation (#505), CI-VM speed (#512) |
|
||||
| **F** | **Test the DENY path with the production config value.** #756's lesson generalised: a fixture that omits a field tests only the default, so a fail-open in the production value stays invisible. Parametrise the whole matrix, including "config absent". | Fail-open-by-default: ~5 records | **rule-missing** |
|
||||
| **G** | **Full-replace endpoints assert their complete field list in a test**, and prefer reconcile-by-id over delete-and-reinsert where child state exists. | Destructive replace: ~3 records | **rule-missing** |
|
||||
|
||||
|
||||
@@ -67,7 +67,12 @@ with have sat in these gaps. Treat anything not named here as unassessed, not as
|
||||
the check proves each job was classified, never that the classification is CORRECT; and an
|
||||
`inline` assertion cannot be proven to work without running the job, so those rows carry
|
||||
`Proof: NONE` honestly instead of borrowing a neighbouring file's proof.
|
||||
2. **C# and TypeScript guards** — seven files, none with a row:
|
||||
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,
|
||||
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
|
||||
hand-written covered set),
|
||||
|
||||
+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; its hostile-ICC cases (and the watermark fixture's) pin by allocation that decode and clone do not parse an ICC profile (`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". |
|
||||
|
||||
+6
-1
@@ -41,7 +41,7 @@
|
||||
"groupName": "Microsoft.AspNetCore"
|
||||
},
|
||||
{
|
||||
"description": "Group Entity Framework Core + providers (held to v9 by the range pins in Directory.Packages.props)",
|
||||
"description": "Group Entity Framework Core + providers. The range pins in Directory.Packages.props hold the build to v9 but do not stop Renovate proposing v10 (docs/ci-cd.md, Renovate)",
|
||||
"matchPackageNames": [
|
||||
"/^Microsoft\\.EntityFrameworkCore/",
|
||||
"/^EFCore\\.BulkExtensions/",
|
||||
@@ -49,6 +49,11 @@
|
||||
],
|
||||
"groupName": "Entity Framework Core"
|
||||
},
|
||||
{
|
||||
"description": "Hold SixLabors.ImageSharp on 3.1.x: every patched release fails a Release build without a Six Labors license key. Delete this rule when the key is provisioned (security.imagesharp-3x-tiff-decoder-disabled, ersatztv#929)",
|
||||
"matchPackageNames": ["SixLabors.ImageSharp"],
|
||||
"allowedVersions": "/^3\\.1\\./"
|
||||
},
|
||||
{
|
||||
"description": "Group the Serilog family into one PR",
|
||||
"matchPackageNames": ["/^Serilog/"],
|
||||
|
||||
@@ -100,8 +100,8 @@ classifier="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib/branch-rule-classi
|
||||
# comparison was made", which is the truth for a non-array body.
|
||||
#
|
||||
# "To produce a finding" rather than "before the classifier sees it", and the distinction is the
|
||||
# `ci.jq-version-contract` gap (ersatztv#643): on jq 1.6 — the version `scripts/jq-preflight.sh`
|
||||
# pins for `script-tests` — `jq -e` over EMPTY input exits 0, so an empty 200 body is not diverted
|
||||
# `ci.jq-accepted-version-set` gap (ersatztv#643): on jq 1.6 — the floor `scripts/jq-preflight.sh`
|
||||
# admits for `script-tests` — `jq -e` over EMPTY input exits 0, so an empty 200 body is not diverted
|
||||
# here and does reach the classifier. Measured on both 1.6 and 1.8.2: the class word is `unreadable`
|
||||
# either way, because the classifier then produces nothing and the catch-all answers. The outcome is
|
||||
# version-independent; only the mechanism differs, so the comment states the outcome.
|
||||
|
||||
@@ -139,8 +139,8 @@ comments=$(cat)
|
||||
# NOTE the NUL test is `explode | index(0)`, NOT `contains("\u0000")` (ersatztv#647). On jq 1.6 the
|
||||
# escape truncates the literal to the EMPTY string, and every string contains "" — so that form
|
||||
# returns true for ALL input, making this guard reject every comment body as malformed. Verified
|
||||
# against both binaries: 1.6 says true for "hello", 1.7+ says false. The CI runner ships jq 1.6, so
|
||||
# the whole verdict classifier was inert there. `explode | index(0)` agrees on both.
|
||||
# against both binaries: 1.6 says true for "hello", 1.7+ says false. A CI runner reporting jq 1.6 (the floor)
|
||||
# had the whole verdict classifier inert. `explode | index(0)` agrees on both.
|
||||
#
|
||||
# `Review<NUL>-verdict: MERGEABLE @ <head>` would arrive at the matcher as a valid verdict line —
|
||||
# text that is not a verdict silently becoming one.
|
||||
|
||||
+47
-24
@@ -2,9 +2,10 @@
|
||||
# Make the jq version a job's shell gates run under OBSERVABLE, and any drift LOUD.
|
||||
#
|
||||
# ersatztv#648. Every shell gate in this repo is authored and tested on a developer Mac shipping
|
||||
# jq 1.8.x. The CI runner ships jq 1.6. Nothing pinned or checked that, and until ersatztv#631 the one
|
||||
# thing that could have noticed (scripts/tests/) never ran on the runner. Three independent divergences
|
||||
# surfaced in a single day:
|
||||
# jq 1.8.x. The CI runners shipped jq 1.6 when this was written (2026-07-26); on 2026-10-28 both
|
||||
# `small` runners reported jq 1.7 (server-management#1080). Nothing pinned or checked that, and until
|
||||
# ersatztv#631 the one thing that could have noticed (scripts/tests/) never ran on the runner. Three
|
||||
# independent divergences surfaced in a single day:
|
||||
#
|
||||
# ersatztv#643 `jq -e` over EMPTY input -> exit 4 on 1.8, exit 0 on 1.6 (a transport failure
|
||||
# passed the docs-only pagination guard)
|
||||
@@ -20,11 +21,15 @@
|
||||
# WHY A FLOOR AND NOT A PIN EVERYWHERE. The obvious fix — bake a pinned jq into the CI toolchain image
|
||||
# (docker/ci/Dockerfile) — provably does NOT cover the gate that actually broke. `.gitea/workflows/
|
||||
# review-verdict.yml` is `runs-on: small`, carries no toolchain-image pin, and per `ci.small-lane-git-only`
|
||||
# the small lane is git-only. It therefore gets the HOST's jq 1.6 no matter what the image contains.
|
||||
# the small lane is git-only. It therefore gets the HOST's jq, whatever the image contains.
|
||||
# That was checked, not assumed (ersatztv#648's first Done-when box).
|
||||
#
|
||||
# So the contract is the other way round: 1.6 is the FLOOR every gate must work on, and it is the
|
||||
# runner's own jq that provides the 1.6 coverage `scripts/tests/` runs under.
|
||||
# So the contract is the other way round: 1.6 is the FLOOR every gate must work on. Both `small`
|
||||
# runners map to the floating runner-images:ubuntu-latest image with no digest pin, so each host keeps
|
||||
# whatever it last pulled (2026-10-28: both measured hosts reported 1.7; a host may hold an older
|
||||
# pull). No known runner reports 1.6, so real-jq 1.6 coverage of `scripts/tests/` is currently absent;
|
||||
# 1.6 stays accepted only because the tag is unpinned. server-management#1080 proposes a fleet digest
|
||||
# pin; when it settles one version, narrow the `--expect` set in pr-checks.yml to that version.
|
||||
#
|
||||
# TWO MODES, deliberately asymmetric:
|
||||
#
|
||||
@@ -34,20 +39,21 @@
|
||||
# `main`, so a hard pin there would turn any jq upgrade on the runner into a
|
||||
# repo-wide merge deadlock. Observability without a deadlock risk.
|
||||
#
|
||||
# --expect X.Y Additionally assert the version is exactly X.Y, and FAIL if not. Used by the
|
||||
# `script-tests` job. This is the tripwire: `scripts/tests/` currently exercises
|
||||
# the 1.6 path only because the runner happens to ship 1.6. If the runner were
|
||||
# upgraded, that coverage would vanish SILENTLY and the whole class of bug above
|
||||
# would go untested again. Going red forces a human to decide — re-pin, or add a
|
||||
# real 1.6 matrix leg — rather than letting the coverage evaporate unnoticed.
|
||||
# --expect X.Y[,X.Y] Additionally assert the version is exactly one member of the comma-separated
|
||||
# set, and FAIL if not. Used by the `script-tests` job with `1.6,1.7` (since
|
||||
# 2026-10-29, until server-management#1080 pins the fleet). This is the
|
||||
# tripwire: a version outside the set means the runner image moved on and the
|
||||
# coverage the suite assumed has changed, so going red forces a human to decide
|
||||
# rather than letting it change unnoticed.
|
||||
#
|
||||
# Usage: jq-preflight.sh [--expect <major.minor>]
|
||||
# Usage: jq-preflight.sh [--expect <major.minor>[,<major.minor>...]]
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
# The lowest jq every shell gate in this repo must run correctly on. Do not raise this without
|
||||
# confirming the CI runner has actually been upgraded first — the runner, not the dev Mac, is the
|
||||
# binding constraint.
|
||||
# The lowest jq every shell gate in this repo must run correctly on. Do not raise this until every
|
||||
# runner that can take a job is confirmed above it — the runner fleet, not the dev Mac, is the binding
|
||||
# constraint, and while the runner image tag is unpinned (server-management#1080) a host may still hold
|
||||
# a 1.6 pull.
|
||||
MIN_VERSION="1.6"
|
||||
|
||||
expect=""
|
||||
@@ -156,18 +162,35 @@ if [ "$major" -lt "$min_major" ] || { [ "$major" -eq "$min_major" ] && [ "$minor
|
||||
fi
|
||||
|
||||
if [ -n "$expect" ]; then
|
||||
if [ "${major}.${minor}" != "$expect" ]; then
|
||||
echo "jq-preflight: expected jq ${expect}, found ${major}.${minor}." >&2
|
||||
# --expect is a comma-separated SET. Validate the WHOLE string first: every member must be a plain
|
||||
# <digits>.<digits>, so an empty member (`1.6,`, `,1.6`, `1.6,,1.7`) or a free-form one is a usage
|
||||
# error, never a member that silently matches nothing (or, via a later split, something it should not).
|
||||
member_re='[0-9]{1,9}\.[0-9]{1,9}'
|
||||
if [[ ! "$expect" =~ ^${member_re}(,${member_re})*$ ]]; then
|
||||
echo "jq-preflight: --expect must be <major.minor>[,<major.minor>...]; got '${expect}'" >&2
|
||||
exit 2
|
||||
fi
|
||||
matched=""
|
||||
IFS=',' read -r -a expect_set <<<"$expect"
|
||||
for member in "${expect_set[@]}"; do
|
||||
if [ "$member" = "${major}.${minor}" ]; then
|
||||
matched="yes"
|
||||
fi
|
||||
done
|
||||
if [ -z "$matched" ]; then
|
||||
echo "jq-preflight: expected jq in {${expect}}, found ${major}.${minor}." >&2
|
||||
echo "" >&2
|
||||
echo "This is a TRIPWIRE, not a defect in your change (ersatztv#648). scripts/tests/ was pinned to" >&2
|
||||
echo "jq ${expect} because that is what this runner shipped; it now reports ${major}.${minor}. The ${expect}" >&2
|
||||
echo "coverage the suite assumed has therefore just disappeared, silently — and jq 1.7 altered NUL" >&2
|
||||
echo "handling, exit codes, @base64d and number precision, every one of which a gate here depends on." >&2
|
||||
echo "This is a TRIPWIRE, not a defect in your change (ersatztv#648, #935). scripts/tests/ is run" >&2
|
||||
echo "under whichever jq the runner that took this job ships, and the accepted set is {${expect}}." >&2
|
||||
echo "This runner reports ${major}.${minor}, so the jq the suite assumed has changed — jq 1.7 altered" >&2
|
||||
echo "NUL handling, exit codes, @base64d and number precision, every one of which a gate here" >&2
|
||||
echo "depends on." >&2
|
||||
echo "" >&2
|
||||
echo "Decide explicitly, then update the --expect value in .gitea/workflows/pr-checks.yml:" >&2
|
||||
echo " * re-pin to the new version after re-reading docs/ci-cd.md -> 'The jq contract', or" >&2
|
||||
echo " * widen or re-pin the set after re-reading docs/ci-cd.md -> 'The jq contract'" >&2
|
||||
echo " (server-management#1080 is the fleet digest pin that narrows it to one version), or" >&2
|
||||
echo " * add a real matrix leg that runs the suite under ${MIN_VERSION} as well." >&2
|
||||
exit 1
|
||||
fi
|
||||
echo "jq-preflight: version matches the expected pin (${expect})."
|
||||
echo "jq-preflight: version ${major}.${minor} is a member of the expected set (${expect})."
|
||||
fi
|
||||
|
||||
@@ -79,7 +79,8 @@
|
||||
# ETV_GITEA_URL | GITEA_BASE_URL -> API base; defaults to the homelab Gitea. A value ending in
|
||||
# /api/v1 is used as-is, otherwise /api/v1 is appended.
|
||||
#
|
||||
# jq COMPATIBILITY (ersatztv#648). This runs on the CI runner, which ships **jq 1.6**, while it is
|
||||
# jq COMPATIBILITY (ersatztv#648). This runs on CI runners whose jq is unpinned (1.7 measured 2026-10-28;
|
||||
# floor 1.6), while it is
|
||||
# authored on Macs shipping 1.8.x. It is therefore written to the 1.6-compatible subset:
|
||||
# * never rely on `jq -e`'s exit status over EMPTY input — 1.6 exits 0 where >=1.7 exits 4, which is
|
||||
# precisely the fail-open that ersatztv#647 found live in the enforced gate. Emptiness is always
|
||||
@@ -180,7 +181,7 @@ while [ "$page" -le "$MAX_PAGES" ]; do
|
||||
raw=$(gq "repos/$owner/$repo/pulls/$pr/files?limit=${PAGE_SIZE}&page=${page}")
|
||||
|
||||
# An EMPTY body is rejected in SHELL, before jq sees it. `jq -e` over empty input exits 4 on
|
||||
# jq >= 1.7 but 0 on jq 1.6, and the runner ships 1.6 — leaving this to jq's exit status is the
|
||||
# jq >= 1.7 but 0 on jq 1.6, and a runner may still report 1.6 (unpinned image) — leaving this to jq's exit status is the
|
||||
# exact fail-open ersatztv#647 found in the enforced copy. A transport failure must never
|
||||
# masquerade as a legitimate short final page.
|
||||
if [ -z "${raw//[[:space:]]/}" ]; then
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
"""Tests for `scripts/jq-preflight.sh` — the jq version contract (ersatztv#648).
|
||||
|
||||
The axis this guards. Every shell gate in this repo is authored on a Mac shipping jq 1.8.x; the CI
|
||||
runner ships jq 1.6. Nothing pinned or checked that, and three independent divergences surfaced in a
|
||||
runners shipped jq 1.6 (2026-07-26) and both measured runners report 1.7 (2026-10-28). Nothing pinned
|
||||
or checked that, and three independent divergences surfaced in a
|
||||
single day — `jq -e` over empty input (exit 4 vs 0), `contains("<NUL>")` (false vs true for every
|
||||
string), and the parse-error exit code (5 vs 4, colliding with "no output"). Each was patched with a
|
||||
version-stable construct, but patching constructs one at a time leaves the AXIS untested.
|
||||
@@ -15,6 +16,7 @@ reproducible: it re-runs on every PR instead of living in one CI run's history.
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
@@ -132,13 +134,16 @@ def test_build_suffixes_still_parse_as_1_6(preflight, version_line):
|
||||
assert preflight.run("--expect", "1.6").returncode == 0, version_line
|
||||
|
||||
|
||||
ACCEPTED = "1.6,1.7" # the set pr-checks.yml passes until server-management#1080 pins the fleet
|
||||
|
||||
|
||||
def test_expect_mismatch_is_LOUD(preflight):
|
||||
"""THE TRIPWIRE. scripts/tests exercises the jq 1.6 path only because the runner ships 1.6. If
|
||||
the runner were upgraded that coverage would vanish silently, so the pin must go red instead."""
|
||||
"""THE TRIPWIRE. A version outside the accepted set must go red, so a runner image moving on
|
||||
changes the suite's jq visibly instead of silently."""
|
||||
preflight.with_jq("jq-1.7.1")
|
||||
r = preflight.run("--expect", "1.6")
|
||||
assert r.returncode == 1
|
||||
assert "expected jq 1.6, found 1.7" in r.stderr
|
||||
assert "expected jq in {1.6}, found 1.7" in r.stderr
|
||||
|
||||
|
||||
def test_expect_match_passes(preflight):
|
||||
@@ -146,6 +151,34 @@ def test_expect_match_passes(preflight):
|
||||
assert preflight.run("--expect", "1.6").returncode == 0
|
||||
|
||||
|
||||
@pytest.mark.parametrize("version_line", ["jq-1.6", "jq-1.7", "jq-1.7.1", "jq-1.6-dirty"])
|
||||
def test_the_accepted_set_passes_on_either_member(preflight, version_line):
|
||||
"""The unpinned fleet (server-management#1080): whichever version the runner holds must be green."""
|
||||
preflight.with_jq(version_line)
|
||||
r = preflight.run("--expect", ACCEPTED)
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert "member of the expected set" in r.stdout
|
||||
|
||||
|
||||
@pytest.mark.parametrize("version_line", ["jq-1.8.1", "jq-1.10", "jq-2.6", "jq-1.71"])
|
||||
def test_a_version_outside_the_accepted_set_is_LOUD(preflight, version_line):
|
||||
"""The witnessed red. 1.8 is what the dev Macs ship and 1.10/1.71 are string-prefix lookalikes of
|
||||
1.7/1.6 members: membership is exact on major.minor, not a prefix or numeric-range test."""
|
||||
preflight.with_jq(version_line)
|
||||
r = preflight.run("--expect", ACCEPTED)
|
||||
assert r.returncode == 1, version_line
|
||||
assert "expected jq in {1.6,1.7}" in r.stderr
|
||||
|
||||
|
||||
@pytest.mark.parametrize("bad", ["1.6,", ",1.6", "1.6,,1.7", "1.6 1.7", "1.x", "1", "1.6;1.7", "1.6,1.7,"])
|
||||
def test_a_malformed_expect_set_is_a_usage_error(preflight, bad):
|
||||
"""An empty or free-form member must not become a member that matches nothing (or everything)."""
|
||||
preflight.with_jq("jq-1.6")
|
||||
r = preflight.run("--expect", bad)
|
||||
assert r.returncode == 2, bad
|
||||
assert "--expect must be" in r.stderr
|
||||
|
||||
|
||||
def test_unknown_argument_is_a_usage_error(preflight):
|
||||
preflight.with_jq("jq-1.6")
|
||||
assert preflight.run("--pin", "1.6").returncode == 2
|
||||
@@ -345,6 +378,11 @@ def test_script_tests_pins_the_jq_version():
|
||||
"""
|
||||
pr_checks = (WORKFLOWS / "pr-checks.yml").read_text()
|
||||
assert "jq-preflight.sh --expect" in pr_checks, "script-tests must pin the jq version — that pin is the tripwire"
|
||||
# Exactly the accepted set until server-management#1080 settles one version (#935): not 1.6 alone
|
||||
# (red on a 1.7 runner), not a wider set (a 1.8 runner must stay red).
|
||||
assert re.search(rf"^\s*run: \./scripts/jq-preflight\.sh --expect {re.escape(ACCEPTED)}\s*$", pr_checks, re.M), (
|
||||
f"script-tests must run --expect {ACCEPTED}"
|
||||
)
|
||||
|
||||
|
||||
def test_review_verdict_never_pins_a_jq_version():
|
||||
|
||||
@@ -493,12 +493,12 @@ def test_a_SHORT_page_does_not_end_the_enumeration(enumerate_files):
|
||||
assert "ErsatzTV/Program.cs" in enumerate_files.paths()
|
||||
|
||||
|
||||
# --- The same guard, under the runner's jq 1.6 ------------------------------------------------
|
||||
# --- The same guard, under jq 1.6 ------------------------------------------------
|
||||
#
|
||||
# `test_transport_failure_mid_pagination_fails_closed` above does NOT isolate the explicit
|
||||
# `if [ -z "${raw//[[:space:]]/}" ]` clause — mutation confirms it: deleting that clause leaves
|
||||
# the whole suite green on a developer Mac, because jq 1.8 rejects empty
|
||||
# input on its own. jq 1.6 does not, and the runner ships 1.6 — so the one environment where the
|
||||
# input on its own. jq 1.6 does not, and a runner may report 1.6 — so the one environment where the
|
||||
# clause is load-bearing was the one environment with no coverage. That is the #643/#647 failure
|
||||
# class exactly, reproduced in the test suite meant to prevent it.
|
||||
#
|
||||
@@ -1840,7 +1840,7 @@ def _run_classify(
|
||||
curl.write_text(WORKFLOW_STUB_CURL)
|
||||
curl.chmod(0o755)
|
||||
if jq16:
|
||||
# Reproduce the runner's jq 1.6 (`-e` over EMPTY input exits 0, not 4). The shim is the one
|
||||
# Reproduce jq 1.6 (`-e` over EMPTY input exits 0, not 4). The shim is the one
|
||||
# already used for pr-changed-files.sh in this file, so its fidelity is covered by that
|
||||
# suite's own verify-the-verifier test.
|
||||
jq = bindir / "jq"
|
||||
@@ -2184,7 +2184,7 @@ def test_a_transport_failure_under_jq_1_6_STILL_refuses_to_classify(tmp_path):
|
||||
"""The ersatztv#647 fail-open, tested where it actually lives: jq 1.6.
|
||||
|
||||
The shell emptiness check exists because `jq -e` over EMPTY input exits 4 on jq >= 1.7 but **0 on
|
||||
jq 1.6, which is what the runner ships**. Remove that check and, on a dev Mac's 1.8, the guard
|
||||
jq 1.6, which a runner may report (the floor)**. Remove that check and, on a dev Mac's 1.8, the guard
|
||||
still fires and every test stays green — the bug is invisible locally.
|
||||
|
||||
Pinning the construct STRUCTURALLY instead, on the grounds that "no behavioural test can catch
|
||||
@@ -3345,8 +3345,8 @@ def test_count_pr_mutations_uses_NO_jq_e(tmp_path):
|
||||
|
||||
The function's own comment states the rule and the reason: `jq -e` reports the truthiness of its
|
||||
LAST OUTPUT, and over empty input jq 1.6 exits 0 where >= 1.7 exits 4 — the divergence that
|
||||
already took this workflow's enforced gate down once (ersatztv#647), on a runner that still ships
|
||||
1.6. Every tally here is therefore read with `jq -r` and range-checked in shell.
|
||||
already took this workflow's enforced gate down once (ersatztv#647), on a runner that may report
|
||||
1.6 (the floor). Every tally here is therefore read with `jq -r` and range-checked in shell.
|
||||
|
||||
This exists because the rule was BROKEN one commit after it was quoted (ersatztv#803 added a
|
||||
row-validation guard using `jq -e`, three lines below the comment forbidding it) and nothing
|
||||
@@ -3391,7 +3391,7 @@ def test_count_pr_mutations_uses_NO_jq_e(tmp_path):
|
||||
offenders = [ln.strip() for ln in joined if _uses_jq_exit_status(ln)]
|
||||
assert not offenders, (
|
||||
"`jq -e` appeared inside `count_pr_mutations`, whose own comment bans it because the runner "
|
||||
"ships jq 1.6 and `-e`'s exit status over empty input diverges there (ersatztv#647/#803). "
|
||||
"may report jq 1.6 and `-e`'s exit status over empty input diverges there (ersatztv#647/#803). "
|
||||
"Read the value with `jq -r` and range-check it in shell instead:\n " + "\n ".join(offenders)
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user