Compare commits
11
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
324dbdbca0 | ||
|
|
bcf6f7e04d | ||
|
|
403bf011c1 | ||
|
|
00424bf090 | ||
|
|
8acba80a1a | ||
|
|
41d604f0ae | ||
|
|
e919b925f8 | ||
|
|
e61bbb63b5 | ||
|
|
e458630f48 | ||
|
|
bdd4ed7bad | ||
|
|
02aa05d75b |
@@ -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
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
using CliWrap;
|
||||
using ErsatzTV.Core.Scheduling.Engine;
|
||||
using ErsatzTV.Core.Scheduling.ScriptedScheduling;
|
||||
using NSubstitute;
|
||||
using NUnit.Framework;
|
||||
using Shouldly;
|
||||
|
||||
namespace ErsatzTV.Core.Tests.Scheduling.ScriptedScheduling;
|
||||
|
||||
/// <summary>
|
||||
/// The two Core halves of the scripted-build credential (ersatztv#913): the session store that mints and
|
||||
/// validates a build-scoped token, and the process launch that hands it to the user's program. The filter
|
||||
/// half — that the token is accepted on <c>ScriptedScheduleController</c> and nowhere else — is
|
||||
/// <c>ErsatzTV.Tests/Filters/ScriptedScheduleAuthorizationTests</c>.
|
||||
/// </summary>
|
||||
[TestFixture]
|
||||
public class ScriptedBuildTokenTests
|
||||
{
|
||||
[Test]
|
||||
public void StartSession_Mints_A_256_Bit_Token_Distinct_From_The_Build_Id_And_Per_Build()
|
||||
{
|
||||
var service = new ScriptedPlayoutBuilderService();
|
||||
|
||||
ScriptedBuildSession first = service.StartSession(Substitute.For<ISchedulingEngine>());
|
||||
ScriptedBuildSession second = service.StartSession(Substitute.For<ISchedulingEngine>());
|
||||
|
||||
first.Token.Length.ShouldBe(64);
|
||||
first.Token.ShouldAllBe(c => Uri.IsHexDigit(c));
|
||||
first.Token.ShouldNotContain(first.BuildId.ToString("N"), Case.Insensitive);
|
||||
second.Token.ShouldNotBe(first.Token);
|
||||
second.BuildId.ShouldNotBe(first.BuildId);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Token_Is_Valid_Only_For_Its_Own_Running_Build()
|
||||
{
|
||||
var service = new ScriptedPlayoutBuilderService();
|
||||
ScriptedBuildSession mine = service.StartSession(Substitute.For<ISchedulingEngine>());
|
||||
ScriptedBuildSession other = service.StartSession(Substitute.For<ISchedulingEngine>());
|
||||
|
||||
service.IsBuildTokenValid(mine.BuildId, mine.Token).ShouldBeTrue();
|
||||
service.IsBuildTokenValid(mine.BuildId, other.Token).ShouldBeFalse();
|
||||
service.IsBuildTokenValid(mine.BuildId, mine.Token.ToUpperInvariant()).ShouldBeFalse();
|
||||
service.IsBuildTokenValid(mine.BuildId, mine.Token[..^1]).ShouldBeFalse();
|
||||
service.IsBuildTokenValid(mine.BuildId, "").ShouldBeFalse();
|
||||
service.IsBuildTokenValid(mine.BuildId, null).ShouldBeFalse();
|
||||
service.IsBuildTokenValid(Guid.NewGuid(), mine.Token).ShouldBeFalse();
|
||||
|
||||
service.EndSession(mine.BuildId);
|
||||
|
||||
service.IsBuildTokenValid(mine.BuildId, mine.Token).ShouldBeFalse();
|
||||
service.IsBuildTokenValid(other.BuildId, other.Token).ShouldBeTrue();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void A_Mocked_Session_Has_No_Token_That_Validates()
|
||||
{
|
||||
// MockSession is the in-process test seam; it registers no token, so no presented value — the empty
|
||||
// string included — authenticates against it.
|
||||
var service = new ScriptedPlayoutBuilderService();
|
||||
var buildId = Guid.NewGuid();
|
||||
service.MockSession(Substitute.For<ISchedulingEngine>(), buildId).ShouldBeTrue();
|
||||
|
||||
service.GetEngine(buildId).ShouldNotBeNull();
|
||||
service.IsBuildTokenValid(buildId, "").ShouldBeFalse();
|
||||
service.IsBuildTokenValid(buildId, "anything").ShouldBeFalse();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Launch_Hands_The_Token_To_The_Program_In_The_Environment_And_Not_On_The_Command_Line()
|
||||
{
|
||||
const string Token = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef";
|
||||
string[] arguments = ["http://localhost:8409", Guid.NewGuid().ToString(), "reset", "user-arg"];
|
||||
|
||||
Command command = ScriptedPlayoutBuilder.CreateCommand("/scripts/run.py", arguments, Token);
|
||||
|
||||
command.EnvironmentVariables.ShouldContainKeyAndValue(ScriptedBuildToken.EnvironmentVariable, Token);
|
||||
command.Arguments.ShouldNotContain(Token);
|
||||
|
||||
// The published argument contract is unchanged: host, build id, mode, then the user's own arguments.
|
||||
command.Arguments.ShouldBe(string.Join(' ', arguments));
|
||||
command.TargetFilePath.ShouldBe("/scripts/run.py");
|
||||
command.Validation.ShouldBe(CommandResultValidation.None);
|
||||
}
|
||||
}
|
||||
@@ -1,11 +1,26 @@
|
||||
using ErsatzTV.Core.Scheduling.Engine;
|
||||
using ErsatzTV.Core.Scheduling.ScriptedScheduling;
|
||||
|
||||
namespace ErsatzTV.Core.Interfaces.Scheduling;
|
||||
|
||||
public interface IScriptedPlayoutBuilderService
|
||||
{
|
||||
/// <summary>
|
||||
/// Registers an engine under a caller-chosen build id with NO token, so
|
||||
/// <see cref="IsBuildTokenValid" /> is false for it. The in-process test seam (ersatztv#563).
|
||||
/// </summary>
|
||||
bool MockSession(ISchedulingEngine schedulingEngine, Guid buildId);
|
||||
Guid StartSession(ISchedulingEngine schedulingEngine);
|
||||
|
||||
/// <summary>Registers an engine under a fresh build id and mints the build-scoped token for it.</summary>
|
||||
ScriptedBuildSession StartSession(ISchedulingEngine schedulingEngine);
|
||||
|
||||
ISchedulingEngine GetEngine(Guid buildId);
|
||||
|
||||
/// <summary>
|
||||
/// True only when <paramref name="buildId" /> names a running build whose minted token equals
|
||||
/// <paramref name="token" /> (constant-time). An ended build, a mocked build and an empty token are all false.
|
||||
/// </summary>
|
||||
bool IsBuildTokenValid(Guid buildId, string token);
|
||||
|
||||
void EndSession(Guid buildId);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
namespace ErsatzTV.Core.Scheduling.ScriptedScheduling;
|
||||
|
||||
/// <summary>
|
||||
/// A running scripted build: the id the program addresses it by, and the build-scoped token it
|
||||
/// authenticates with (<see cref="ScriptedBuildToken" />). A class rather than a record so that its
|
||||
/// <c>ToString()</c> — and therefore a structured-log argument — never prints the token.
|
||||
/// </summary>
|
||||
public sealed class ScriptedBuildSession(Guid buildId, string token)
|
||||
{
|
||||
public Guid BuildId { get; } = buildId;
|
||||
|
||||
public string Token { get; } = token;
|
||||
}
|
||||
@@ -0,0 +1,25 @@
|
||||
namespace ErsatzTV.Core.Scheduling.ScriptedScheduling;
|
||||
|
||||
/// <summary>
|
||||
/// The credential a scripted-playout program presents when it calls back into
|
||||
/// <c>/api/v1/scripted/playout/build/{buildId}</c> (ersatztv#913, decision
|
||||
/// <c>api.scripted-build-token</c>). <see cref="ScriptedPlayoutBuilder" /> mints one per build, hands it to
|
||||
/// the program in <see cref="EnvironmentVariable" />, and the global API authorization filter accepts it in
|
||||
/// <see cref="HeaderName" /> — for that build's endpoints only, and only while the build is running.
|
||||
/// </summary>
|
||||
public static class ScriptedBuildToken
|
||||
{
|
||||
/// <summary>
|
||||
/// The request header the program sends the token in. A custom header, so — like the machine
|
||||
/// <c>X-Api-Key</c> — a browser cannot attach it cross-origin, and a token-authenticated mutation needs
|
||||
/// no CSRF header.
|
||||
/// </summary>
|
||||
public const string HeaderName = "X-Scripted-Build-Token";
|
||||
|
||||
/// <summary>
|
||||
/// The environment variable the token is delivered in. Not a command-line argument: the argument list is
|
||||
/// logged at Information level and is visible to every local user through the process table, and a new
|
||||
/// positional argument would shift the user's own arguments (the ones after <c>mode</c>) by one.
|
||||
/// </summary>
|
||||
public const string EnvironmentVariable = "ETV_SCRIPTED_BUILD_TOKEN";
|
||||
}
|
||||
@@ -27,7 +27,8 @@ public class ScriptedPlayoutBuilder(
|
||||
{
|
||||
var result = PlayoutBuildResult.Empty;
|
||||
|
||||
Guid buildId = scriptedPlayoutBuilderService.StartSession(schedulingEngine);
|
||||
ScriptedBuildSession session = scriptedPlayoutBuilderService.StartSession(schedulingEngine);
|
||||
Guid buildId = session.BuildId;
|
||||
|
||||
var timeoutSeconds = 30;
|
||||
|
||||
@@ -86,9 +87,7 @@ public class ScriptedPlayoutBuilder(
|
||||
using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(timeoutSeconds));
|
||||
using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, cts.Token);
|
||||
|
||||
Command command = Cli.Wrap(scriptFile)
|
||||
.WithArguments(arguments)
|
||||
.WithValidation(CommandResultValidation.None);
|
||||
Command command = CreateCommand(scriptFile, arguments, session.Token);
|
||||
|
||||
var commandResult = await command.ExecuteBufferedAsync(linkedCts.Token);
|
||||
if (!string.IsNullOrWhiteSpace(commandResult.StandardOutput))
|
||||
@@ -128,6 +127,18 @@ public class ScriptedPlayoutBuilder(
|
||||
return result;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The process launch for one build. The arguments are the published contract (host, build id, mode, then
|
||||
/// the user's own); the build-scoped token travels in <see cref="ScriptedBuildToken.EnvironmentVariable" />
|
||||
/// instead, because the global API authorization filter rejects an unauthenticated callback and the argument
|
||||
/// list is logged (ersatztv#913). CliWrap layers the variable over the inherited environment.
|
||||
/// </summary>
|
||||
internal static Command CreateCommand(string scriptFile, IEnumerable<string> arguments, string buildToken) =>
|
||||
Cli.Wrap(scriptFile)
|
||||
.WithArguments(arguments)
|
||||
.WithEnvironmentVariables(env => env.Set(ScriptedBuildToken.EnvironmentVariable, buildToken))
|
||||
.WithValidation(CommandResultValidation.None);
|
||||
|
||||
private async Task<int> GetDaysToBuild(CancellationToken cancellationToken) =>
|
||||
await configElementRepository
|
||||
.GetValue<int>(ConfigElementKey.PlayoutDaysToBuild, cancellationToken)
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
using System.Collections.Concurrent;
|
||||
using System.Security.Cryptography;
|
||||
using System.Text;
|
||||
using ErsatzTV.Core.Interfaces.Scheduling;
|
||||
using ErsatzTV.Core.Scheduling.Engine;
|
||||
|
||||
@@ -6,22 +8,36 @@ namespace ErsatzTV.Core.Scheduling.ScriptedScheduling;
|
||||
|
||||
public class ScriptedPlayoutBuilderService : IScriptedPlayoutBuilderService
|
||||
{
|
||||
private readonly ConcurrentDictionary<Guid, ISchedulingEngine> _sessions = new();
|
||||
private readonly ConcurrentDictionary<Guid, Session> _sessions = new();
|
||||
|
||||
public bool MockSession(ISchedulingEngine schedulingEngine, Guid buildId) =>
|
||||
_sessions.TryAdd(buildId, schedulingEngine);
|
||||
_sessions.TryAdd(buildId, new Session(schedulingEngine, []));
|
||||
|
||||
public Guid StartSession(ISchedulingEngine schedulingEngine)
|
||||
public ScriptedBuildSession StartSession(ISchedulingEngine schedulingEngine)
|
||||
{
|
||||
var buildId = Guid.NewGuid();
|
||||
_sessions[buildId] = schedulingEngine;
|
||||
return buildId;
|
||||
|
||||
// 256 bits from the CSPRNG, independent of the build id: the id is logged and passed on the command
|
||||
// line, so it must not double as the secret.
|
||||
string token = Convert.ToHexStringLower(RandomNumberGenerator.GetBytes(32));
|
||||
|
||||
_sessions[buildId] = new Session(schedulingEngine, Encoding.UTF8.GetBytes(token));
|
||||
return new ScriptedBuildSession(buildId, token);
|
||||
}
|
||||
|
||||
public ISchedulingEngine GetEngine(Guid buildId) => _sessions.GetValueOrDefault(buildId);
|
||||
public ISchedulingEngine GetEngine(Guid buildId) =>
|
||||
_sessions.TryGetValue(buildId, out Session session) ? session.Engine : null;
|
||||
|
||||
public bool IsBuildTokenValid(Guid buildId, string token) =>
|
||||
!string.IsNullOrEmpty(token)
|
||||
&& _sessions.TryGetValue(buildId, out Session session)
|
||||
&& session.Token.Length > 0
|
||||
&& CryptographicOperations.FixedTimeEquals(Encoding.UTF8.GetBytes(token), session.Token);
|
||||
|
||||
public void EndSession(Guid buildId)
|
||||
{
|
||||
_sessions.TryRemove(buildId, out _);
|
||||
}
|
||||
|
||||
private sealed record Session(ISchedulingEngine Engine, byte[] Token);
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
using System.Collections.Generic;
|
||||
using System.Security.Claims;
|
||||
using ErsatzTV.Core.Scheduling.ScriptedScheduling;
|
||||
using ErsatzTV.Filters;
|
||||
using ErsatzTV.Services;
|
||||
using Microsoft.AspNetCore.Http;
|
||||
@@ -68,7 +69,7 @@ public class ApiAuthorizationFilterTests
|
||||
}
|
||||
|
||||
private static ApiAuthorizationFilter MakeFilter(bool requireKeyForReads = true) =>
|
||||
new(new FakeApiKeyProvider(Key, requireKeyForReads));
|
||||
new(new FakeApiKeyProvider(Key, requireKeyForReads), new ScriptedPlayoutBuilderService());
|
||||
|
||||
private static void ShouldBeUnauthorized(AuthorizationFilterContext context)
|
||||
{
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using System.Collections.Generic;
|
||||
using ErsatzTV.Core.Scheduling.ScriptedScheduling;
|
||||
using ErsatzTV.Filters;
|
||||
using ErsatzTV.Services;
|
||||
using Microsoft.AspNetCore.Http;
|
||||
@@ -77,7 +78,8 @@ public class ApiKeyEndpointRequiresKeyTests
|
||||
// The filter, on an /api path with the header MISSING, produces a 401 exactly when the predicate
|
||||
// says the endpoint requires a key. Drive the real filter and compare its decision to the predicate.
|
||||
AuthorizationFilterContext context = MakeContext(method, requiresApiKey, skip);
|
||||
new ApiAuthorizationFilter(new FakeApiKeyProvider(requireKeyForReads)).OnAuthorization(context);
|
||||
new ApiAuthorizationFilter(new FakeApiKeyProvider(requireKeyForReads), new ScriptedPlayoutBuilderService())
|
||||
.OnAuthorization(context);
|
||||
|
||||
bool filterGated = context.Result is not null;
|
||||
filterGated.ShouldBe(expected);
|
||||
@@ -94,7 +96,8 @@ public class ApiKeyEndpointRequiresKeyTests
|
||||
.ShouldBeTrue();
|
||||
|
||||
AuthorizationFilterContext context = MakeContext("POST", requiresApiKey: false, skip: false, path: "/iptv/x.m3u");
|
||||
new ApiAuthorizationFilter(new FakeApiKeyProvider(requireKeyForReads: true)).OnAuthorization(context);
|
||||
new ApiAuthorizationFilter(new FakeApiKeyProvider(requireKeyForReads: true), new ScriptedPlayoutBuilderService())
|
||||
.OnAuthorization(context);
|
||||
|
||||
context.Result.ShouldBeNull();
|
||||
}
|
||||
|
||||
@@ -0,0 +1,306 @@
|
||||
using ErsatzTV.Controllers.Api;
|
||||
using ErsatzTV.Core.Scheduling.Engine;
|
||||
using ErsatzTV.Core.Scheduling.ScriptedScheduling;
|
||||
using ErsatzTV.Filters;
|
||||
using ErsatzTV.Services;
|
||||
using Microsoft.AspNetCore.Hosting;
|
||||
using Microsoft.AspNetCore.Http;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
using Microsoft.AspNetCore.Mvc.Abstractions;
|
||||
using Microsoft.AspNetCore.Mvc.Controllers;
|
||||
using Microsoft.AspNetCore.Mvc.Filters;
|
||||
using Microsoft.AspNetCore.Mvc.Infrastructure;
|
||||
using Microsoft.AspNetCore.Routing;
|
||||
using Microsoft.Extensions.Configuration;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.FileProviders;
|
||||
using NSubstitute;
|
||||
using NUnit.Framework;
|
||||
using Shouldly;
|
||||
|
||||
namespace ErsatzTV.Tests.Filters;
|
||||
|
||||
/// <summary>
|
||||
/// The authorization decision for the scripted-playout callback surface (ersatztv#913), driven through the
|
||||
/// REAL <see cref="ApiAuthorizationFilter" /> with the endpoint metadata MVC builds for
|
||||
/// <see cref="ScriptedScheduleController" /> under the production <c>Startup.ConfigureServices</c> — not a
|
||||
/// hand-assembled attribute list, so a controller-level attribute that is added, dropped or renamed shows up
|
||||
/// here exactly as it does at runtime. Before #913 a scripted program presented no credential and every
|
||||
/// callback was a 401; the program now presents the build-scoped token <c>ScriptedPlayoutBuilder</c> mints,
|
||||
/// and the tests below pin both halves: that token opens this controller for its own running build, and
|
||||
/// nothing about it opens anything else.
|
||||
/// </summary>
|
||||
[TestFixture]
|
||||
public class ScriptedScheduleAuthorizationTests
|
||||
{
|
||||
private const string MachineKey = "machine-key";
|
||||
|
||||
private static readonly IReadOnlyList<ControllerActionDescriptor> ScriptedActions = LoadScriptedActions();
|
||||
|
||||
private sealed class FakeApiKeyProvider(bool requireKeyForReads) : IApiKeyProvider
|
||||
{
|
||||
public string ApiKey => MachineKey;
|
||||
public bool RequireKeyForReads { get; } = requireKeyForReads;
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Scan_Found_Every_Scripted_Action()
|
||||
{
|
||||
// Guards the tests below against passing over an empty population (e.g. a part-discovery change that
|
||||
// stops MVC seeing the controller). The count is DERIVED from the controller's own public actions.
|
||||
int declared = typeof(ScriptedScheduleController)
|
||||
.GetMethods(System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance |
|
||||
System.Reflection.BindingFlags.DeclaredOnly)
|
||||
.Count(m => m.GetCustomAttributes(typeof(Microsoft.AspNetCore.Mvc.Routing.HttpMethodAttribute), true).Length > 0);
|
||||
|
||||
declared.ShouldBeGreaterThan(0);
|
||||
ScriptedActions.Count.ShouldBe(declared);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Scripted_Controller_Is_Gated_Not_Exempt_At_Every_Read_Setting()
|
||||
{
|
||||
// The #913 choice: the controller stays under the global filter (a credential is required) rather than
|
||||
// being made [SkipApiAuthorization]. Every action requires a credential at the production default
|
||||
// (Api:RequireKeyForReads=true), and every MUTATING action requires one even with reads opted open.
|
||||
foreach (ControllerActionDescriptor action in Actions())
|
||||
{
|
||||
action.EndpointMetadata.OfType<SkipApiAuthorizationAttribute>().ShouldBeEmpty(action.ActionName);
|
||||
action.EndpointMetadata.OfType<AcceptsScriptedBuildTokenAttribute>().ShouldNotBeEmpty(action.ActionName);
|
||||
|
||||
string method = MethodOf(action);
|
||||
ApiAuthorizationFilter.EndpointRequiresKey(method, action.EndpointMetadata, requireKeyForReads: true)
|
||||
.ShouldBeTrue(action.ActionName);
|
||||
ApiAuthorizationFilter.EndpointRequiresKey(method, action.EndpointMetadata, requireKeyForReads: false)
|
||||
.ShouldBe(!HttpMethods.IsGet(method), action.ActionName);
|
||||
}
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Token_Route_Value_Is_The_Controllers_Build_Id_Parameter()
|
||||
{
|
||||
// The filter reads the build id from the route value the marker names. A rename of the route parameter
|
||||
// would make every token silently fail to validate (back to a 401 per callback), so pin the coupling.
|
||||
foreach (ControllerActionDescriptor action in Actions())
|
||||
{
|
||||
action.AttributeRouteInfo.ShouldNotBeNull();
|
||||
action.AttributeRouteInfo!.Template.ShouldNotBeNull();
|
||||
action.AttributeRouteInfo.Template!.ShouldContain(
|
||||
"{" + AcceptsScriptedBuildTokenAttribute.RouteValueName + ":guid}",
|
||||
Case.Sensitive,
|
||||
action.ActionName);
|
||||
}
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void No_Credential_Is_A_401_On_Every_Scripted_Action()
|
||||
{
|
||||
// The deny branch at the production read setting: what every callback got before #913, and what a
|
||||
// program that ignores the token still gets.
|
||||
var service = new ScriptedPlayoutBuilderService();
|
||||
ScriptedBuildSession session = service.StartSession(Substitute.For<ISchedulingEngine>());
|
||||
|
||||
foreach (ControllerActionDescriptor action in Actions())
|
||||
{
|
||||
AuthorizationFilterContext context = MakeContext(action, session.BuildId);
|
||||
Filter(service).OnAuthorization(context);
|
||||
|
||||
context.Result.ShouldBeOfType<UnauthorizedObjectResult>(action.ActionName);
|
||||
}
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void With_Reads_Opted_Open_Only_The_Mutations_Still_Need_A_Credential()
|
||||
{
|
||||
// The opt-out value of Api:RequireKeyForReads: the filter's own deny branch, not just the predicate.
|
||||
var service = new ScriptedPlayoutBuilderService();
|
||||
ScriptedBuildSession session = service.StartSession(Substitute.For<ISchedulingEngine>());
|
||||
var filter = new ApiAuthorizationFilter(new FakeApiKeyProvider(requireKeyForReads: false), service);
|
||||
|
||||
foreach (ControllerActionDescriptor action in Actions())
|
||||
{
|
||||
AuthorizationFilterContext context = MakeContext(action, session.BuildId);
|
||||
filter.OnAuthorization(context);
|
||||
|
||||
if (HttpMethods.IsGet(MethodOf(action)))
|
||||
{
|
||||
context.Result.ShouldBeNull(action.ActionName);
|
||||
}
|
||||
else
|
||||
{
|
||||
context.Result.ShouldBeOfType<UnauthorizedObjectResult>(action.ActionName);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Running_Builds_Own_Token_Is_Accepted_On_Every_Scripted_Action()
|
||||
{
|
||||
var service = new ScriptedPlayoutBuilderService();
|
||||
ScriptedBuildSession session = service.StartSession(Substitute.For<ISchedulingEngine>());
|
||||
|
||||
foreach (ControllerActionDescriptor action in Actions())
|
||||
{
|
||||
AuthorizationFilterContext context = MakeContext(action, session.BuildId);
|
||||
context.HttpContext.Request.Headers[ScriptedBuildToken.HeaderName] = session.Token;
|
||||
Filter(service).OnAuthorization(context);
|
||||
|
||||
// No X-CSRF on the mutating actions: a custom-header credential is CSRF-immune, like the machine key.
|
||||
context.Result.ShouldBeNull(action.ActionName);
|
||||
}
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Token_Of_Another_Running_Build_Is_A_401()
|
||||
{
|
||||
var service = new ScriptedPlayoutBuilderService();
|
||||
ScriptedBuildSession mine = service.StartSession(Substitute.For<ISchedulingEngine>());
|
||||
ScriptedBuildSession other = service.StartSession(Substitute.For<ISchedulingEngine>());
|
||||
|
||||
foreach (ControllerActionDescriptor action in Actions())
|
||||
{
|
||||
AuthorizationFilterContext context = MakeContext(action, mine.BuildId);
|
||||
context.HttpContext.Request.Headers[ScriptedBuildToken.HeaderName] = other.Token;
|
||||
Filter(service).OnAuthorization(context);
|
||||
|
||||
context.Result.ShouldBeOfType<UnauthorizedObjectResult>(action.ActionName);
|
||||
}
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Token_Is_A_401_Once_Its_Build_Has_Ended()
|
||||
{
|
||||
var service = new ScriptedPlayoutBuilderService();
|
||||
ScriptedBuildSession session = service.StartSession(Substitute.For<ISchedulingEngine>());
|
||||
service.EndSession(session.BuildId);
|
||||
|
||||
foreach (ControllerActionDescriptor action in Actions())
|
||||
{
|
||||
AuthorizationFilterContext context = MakeContext(action, session.BuildId);
|
||||
context.HttpContext.Request.Headers[ScriptedBuildToken.HeaderName] = session.Token;
|
||||
Filter(service).OnAuthorization(context);
|
||||
|
||||
context.Result.ShouldBeOfType<UnauthorizedObjectResult>(action.ActionName);
|
||||
}
|
||||
}
|
||||
|
||||
[TestCase("")]
|
||||
[TestCase("not-the-token")]
|
||||
public void Wrong_Or_Empty_Token_Is_A_401(string presented)
|
||||
{
|
||||
var service = new ScriptedPlayoutBuilderService();
|
||||
ScriptedBuildSession session = service.StartSession(Substitute.For<ISchedulingEngine>());
|
||||
|
||||
foreach (ControllerActionDescriptor action in Actions())
|
||||
{
|
||||
AuthorizationFilterContext context = MakeContext(action, session.BuildId);
|
||||
context.HttpContext.Request.Headers[ScriptedBuildToken.HeaderName] = presented;
|
||||
Filter(service).OnAuthorization(context);
|
||||
|
||||
context.Result.ShouldBeOfType<UnauthorizedObjectResult>(action.ActionName);
|
||||
}
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Token_Opens_Nothing_Outside_The_Marked_Controller()
|
||||
{
|
||||
// The same valid token and the same build-id route value, on an endpoint that lacks the marker, is a 401:
|
||||
// the marker — not the shape of the route — is what scopes the credential.
|
||||
var service = new ScriptedPlayoutBuilderService();
|
||||
ScriptedBuildSession session = service.StartSession(Substitute.For<ISchedulingEngine>());
|
||||
|
||||
var httpContext = new DefaultHttpContext();
|
||||
httpContext.Request.Method = HttpMethods.Post;
|
||||
httpContext.Request.Path = "/api/v1/channels";
|
||||
httpContext.Request.Headers[ScriptedBuildToken.HeaderName] = session.Token;
|
||||
var routeData = new RouteData();
|
||||
routeData.Values[AcceptsScriptedBuildTokenAttribute.RouteValueName] = session.BuildId.ToString();
|
||||
var actionContext = new ActionContext(httpContext, routeData, new ActionDescriptor { EndpointMetadata = [] });
|
||||
var context = new AuthorizationFilterContext(actionContext, []);
|
||||
|
||||
Filter(service).OnAuthorization(context);
|
||||
|
||||
context.Result.ShouldBeOfType<UnauthorizedObjectResult>();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Machine_Key_Is_Still_Accepted_On_Every_Scripted_Action()
|
||||
{
|
||||
var service = new ScriptedPlayoutBuilderService();
|
||||
ScriptedBuildSession session = service.StartSession(Substitute.For<ISchedulingEngine>());
|
||||
|
||||
foreach (ControllerActionDescriptor action in Actions())
|
||||
{
|
||||
AuthorizationFilterContext context = MakeContext(action, session.BuildId);
|
||||
context.HttpContext.Request.Headers[ApiAuthorizationFilter.HeaderName] = MachineKey;
|
||||
Filter(service).OnAuthorization(context);
|
||||
|
||||
context.Result.ShouldBeNull(action.ActionName);
|
||||
}
|
||||
}
|
||||
|
||||
// Every per-action test iterates through this, so none of them can pass by looping over nothing.
|
||||
private static IReadOnlyList<ControllerActionDescriptor> Actions()
|
||||
{
|
||||
ScriptedActions.ShouldNotBeEmpty();
|
||||
return ScriptedActions;
|
||||
}
|
||||
|
||||
private static ApiAuthorizationFilter Filter(ScriptedPlayoutBuilderService service) =>
|
||||
new(new FakeApiKeyProvider(requireKeyForReads: true), service);
|
||||
|
||||
private static string MethodOf(ActionDescriptor action) =>
|
||||
action.EndpointMetadata.OfType<IHttpMethodMetadata>().SelectMany(m => m.HttpMethods).Single();
|
||||
|
||||
private static AuthorizationFilterContext MakeContext(ControllerActionDescriptor action, Guid buildId)
|
||||
{
|
||||
var httpContext = new DefaultHttpContext();
|
||||
httpContext.Request.Method = MethodOf(action);
|
||||
httpContext.Request.Path = "/" + action.AttributeRouteInfo!.Template!
|
||||
.TrimStart('/')
|
||||
.Replace("{" + AcceptsScriptedBuildTokenAttribute.RouteValueName + ":guid}", buildId.ToString(), StringComparison.Ordinal);
|
||||
|
||||
var routeData = new RouteData();
|
||||
routeData.Values[AcceptsScriptedBuildTokenAttribute.RouteValueName] = buildId.ToString();
|
||||
|
||||
var actionContext = new ActionContext(httpContext, routeData, action);
|
||||
return new AuthorizationFilterContext(actionContext, new List<IFilterMetadata>());
|
||||
}
|
||||
|
||||
// The action descriptors MVC builds from the production service registration — the same objects whose
|
||||
// EndpointMetadata the filter reads at runtime.
|
||||
private static List<ControllerActionDescriptor> LoadScriptedActions()
|
||||
{
|
||||
var settings = new Dictionary<string, string?>
|
||||
{
|
||||
["provider"] = "sqlite",
|
||||
["ConnectionStrings:Data"] = "Data Source=:memory:"
|
||||
};
|
||||
|
||||
IConfiguration configuration = new ConfigurationBuilder()
|
||||
.AddInMemoryCollection(settings)
|
||||
.Build();
|
||||
|
||||
var environment = Substitute.For<IWebHostEnvironment>();
|
||||
environment.ApplicationName.Returns("ErsatzTV");
|
||||
environment.EnvironmentName.Returns("Development");
|
||||
environment.ContentRootPath.Returns(TestContext.CurrentContext.TestDirectory);
|
||||
environment.WebRootPath.Returns(TestContext.CurrentContext.TestDirectory);
|
||||
environment.ContentRootFileProvider.Returns(new NullFileProvider());
|
||||
environment.WebRootFileProvider.Returns(new NullFileProvider());
|
||||
|
||||
// The host registers the environment before Startup runs; MVC reads ApplicationName from that
|
||||
// registration to discover the ErsatzTV assembly's controllers, so without it the scan finds none.
|
||||
var services = new ServiceCollection();
|
||||
services.AddSingleton(environment);
|
||||
new Startup(configuration, environment).ConfigureServices(services);
|
||||
|
||||
using ServiceProvider provider = services.BuildServiceProvider();
|
||||
return provider.GetRequiredService<IActionDescriptorCollectionProvider>()
|
||||
.ActionDescriptors
|
||||
.Items
|
||||
.OfType<ControllerActionDescriptor>()
|
||||
.Where(d => d.ControllerTypeInfo.AsType() == typeof(ScriptedScheduleController))
|
||||
.ToList();
|
||||
}
|
||||
}
|
||||
@@ -4,11 +4,19 @@ using ErsatzTV.Core.Domain.Filler;
|
||||
using ErsatzTV.Core.Extensions;
|
||||
using ErsatzTV.Core.Interfaces.Scheduling;
|
||||
using ErsatzTV.Core.Scheduling.Engine;
|
||||
using ErsatzTV.Filters;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
|
||||
namespace ErsatzTV.Controllers.Api;
|
||||
|
||||
/// <summary>
|
||||
/// The callback surface a scripted playout's program drives during a build. Under the global API
|
||||
/// authorization filter like every other controller — not <c>[SkipApiAuthorization]</c> — and additionally
|
||||
/// accepts the build-scoped token <c>ScriptedPlayoutBuilder</c> hands the program, so the program needs
|
||||
/// neither the machine key nor a session (ersatztv#913).
|
||||
/// </summary>
|
||||
[ApiController]
|
||||
[AcceptsScriptedBuildToken]
|
||||
[EndpointGroupName("scripted-schedule")]
|
||||
[Route("/api/v1/scripted/playout/build/{buildId:guid}")]
|
||||
public class ScriptedScheduleController(IScriptedPlayoutBuilderService scriptedPlayoutBuilderService) : ControllerBase
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
using ErsatzTV.Core.Scheduling.ScriptedScheduling;
|
||||
using Microsoft.AspNetCore.Mvc.Filters;
|
||||
|
||||
namespace ErsatzTV.Filters;
|
||||
|
||||
/// <summary>
|
||||
/// Lets <see cref="ApiAuthorizationFilter" /> accept a scripted build's own token
|
||||
/// (<see cref="ScriptedBuildToken.HeaderName" />) as a credential, in addition to the machine key and a
|
||||
/// session (ersatztv#913, decision <c>api.scripted-build-token</c>). The token is checked against the build
|
||||
/// named by the <see cref="RouteValueName" /> route value, so it authenticates only that build's callbacks,
|
||||
/// and only while that build runs. It does NOT exempt the endpoint: a request presenting none of the three
|
||||
/// credentials is still a 401. Carried by <c>ScriptedScheduleController</c> alone.
|
||||
/// </summary>
|
||||
[AttributeUsage(AttributeTargets.Class)]
|
||||
public sealed class AcceptsScriptedBuildTokenAttribute : Attribute, IFilterMetadata
|
||||
{
|
||||
/// <summary>The route parameter that names the build the token must belong to.</summary>
|
||||
public const string RouteValueName = "buildId";
|
||||
}
|
||||
@@ -1,5 +1,7 @@
|
||||
using System.Security.Cryptography;
|
||||
using System.Text;
|
||||
using ErsatzTV.Core.Interfaces.Scheduling;
|
||||
using ErsatzTV.Core.Scheduling.ScriptedScheduling;
|
||||
using ErsatzTV.Services;
|
||||
using Microsoft.AspNetCore.Http;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
@@ -15,6 +17,8 @@ namespace ErsatzTV.Filters;
|
||||
/// <item>a matching machine <c>X-Api-Key</c> header (MCP / external clients), or</item>
|
||||
/// <item>an authenticated session principal (browser cookie, from OIDC or local login).</item>
|
||||
/// </list>
|
||||
/// An endpoint carrying <see cref="AcceptsScriptedBuildTokenAttribute" /> also accepts a running scripted
|
||||
/// build's own token (<see cref="ScriptedBuildToken.HeaderName" />), for that build only (issue #913).
|
||||
/// Which endpoints require authentication is decided by <see cref="EndpointRequiresKey" /> — the
|
||||
/// single predicate shared with OpenAPI generation — so the spec can never drift from enforcement:
|
||||
/// mutating verbs are always fail-closed; reads are gated when <c>Api:RequireKeyForReads</c> is set
|
||||
@@ -28,7 +32,9 @@ namespace ErsatzTV.Filters;
|
||||
/// custom header — forces a CORS preflight that a cross-site attacker page cannot satisfy.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public class ApiAuthorizationFilter(IApiKeyProvider apiKeyProvider) : IAuthorizationFilter
|
||||
public class ApiAuthorizationFilter(
|
||||
IApiKeyProvider apiKeyProvider,
|
||||
IScriptedPlayoutBuilderService scriptedPlayoutBuilderService) : IAuthorizationFilter
|
||||
{
|
||||
public const string HeaderName = "X-Api-Key";
|
||||
|
||||
@@ -62,6 +68,13 @@ public class ApiAuthorizationFilter(IApiKeyProvider apiKeyProvider) : IAuthoriza
|
||||
return;
|
||||
}
|
||||
|
||||
// 1b. A scripted build's own token, on the one controller that opts in. Like the machine key it is a
|
||||
// custom header, so it is CSRF-immune.
|
||||
if (HasValidScriptedBuildToken(context))
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
// 2. Authenticated session (cookie principal from OIDC or local login).
|
||||
if (context.HttpContext.User?.Identity?.IsAuthenticated == true)
|
||||
{
|
||||
@@ -128,6 +141,23 @@ public class ApiAuthorizationFilter(IApiKeyProvider apiKeyProvider) : IAuthoriza
|
||||
|| endpointMetadata.OfType<RequiresAuthenticationAttribute>().Any();
|
||||
}
|
||||
|
||||
private bool HasValidScriptedBuildToken(AuthorizationFilterContext context)
|
||||
{
|
||||
if (!context.ActionDescriptor.EndpointMetadata.OfType<AcceptsScriptedBuildTokenAttribute>().Any())
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!context.HttpContext.Request.Headers.TryGetValue(ScriptedBuildToken.HeaderName, out StringValues provided))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
return context.RouteData.Values.TryGetValue(AcceptsScriptedBuildTokenAttribute.RouteValueName, out object value)
|
||||
&& Guid.TryParse(value?.ToString(), out Guid buildId)
|
||||
&& scriptedPlayoutBuilderService.IsBuildTokenValid(buildId, provided.ToString());
|
||||
}
|
||||
|
||||
private static bool IsMutating(string httpMethod) =>
|
||||
HttpMethods.IsPost(httpMethod)
|
||||
|| HttpMethods.IsPut(httpMethod)
|
||||
|
||||
@@ -1024,6 +1024,13 @@ OpenAPI generation, so the spec can't drift). When you add an endpoint:
|
||||
auth surface must be reachable before a caller is authenticated, and its one sensitive action
|
||||
(`POST /api/v1/auth/password`) self-checks the principal. `ApiControllerSecurityTests` asserts these two are the
|
||||
**only** auth-exempt controllers.
|
||||
- **The scripted-playout callback** (`ScriptedScheduleController`, `/api/v1/scripted/playout/build/{buildId}`) is
|
||||
**not** exempt. It carries `[AcceptsScriptedBuildToken]`, which adds a third accepted credential on that
|
||||
controller only: the per-build token `ScriptedPlayoutBuilder` passes the program in `ETV_SCRIPTED_BUILD_TOKEN`,
|
||||
sent back as `X-Scripted-Build-Token`, valid for the build named by the route's `buildId` until the build ends.
|
||||
The marker is not a general opt-in: its token is a scripted build's, so no other controller carries it. Why a
|
||||
scoped credential rather than a third `[SkipApiAuthorization]` controller: `api.scripted-build-token`; pinned by
|
||||
`ScriptedScheduleAuthorizationTests`.
|
||||
- **The `/api/v1/auth/*` surface** (`AuthController`, `[ApiExplorerSettings(IgnoreApi = true)]` → excluded from the
|
||||
OpenAPI doc, whose audience is machine clients): `GET config` (what auth options exist + `setupRequired`),
|
||||
`GET session`, `POST setup` (first-run claim), `POST login`, `POST logout`, `POST password`, `GET machine-key`
|
||||
|
||||
+41
-26
@@ -42,6 +42,7 @@ Upstream's final release was **`v26.3.0`** (archived). Our line continues from t
|
||||
| `v26.13.0` | **RuleBuilder maturation** — arbitrary-depth group nesting (#436), inline smart-query authoring in Channel Builder (#437), DB-sourced facet typeahead + relative-date operators + validation (#434/#435/#438), and an artist typeahead covering music-video/song credits with `album_artist` no longer 404ing (#578). **Per-channel On Now/Next transient overlay** (#74/#570) and **per-schedule clock-boundary padding** (#392); in-browser channel preview (#60); Auto-Tune per-source weight steppers + exclude/add-untagged (#440). Library-browse pickers now resolve by search instead of a 100-row window, closing several silent at-cap truncations (#644/#650/#651/#634). Correctness: one watermark resolver for all four attachment points, incl. `MiddleCenter` (#503/#510); QSV HDR tonemaps through OpenCL because `vpp_qsv=tonemap` is a silent no-op (#505); `LibraryFolder` unique index + concurrent-insert tolerance (#491); per-library music-video identity with soft trash (#496); Jellyfin Album/Track music-video projection (#177); metadata-collection dedup (#500); accented facet values via a registered Unicode fold on SQLite (#668); `WorkAheadSlots` atomic slot claim, never a negative count (#536/#539); on-demand guide rebuild on thaw (#68). Process/CI: the H10 review-verdict gate became a sha-bound **required** commit status and was hardened through its false-open chain (#622/#629/#632/#648/#649/#672/#698), the decision corpus split to one YAML-frontmatter record per file (#610/#620), and headless Playwright UI-E2E flows landed (#445/#533). Five dual-provider migrations. |
|
||||
| `v26.14.0` | **Live TV no longer starves on embedded bitmap subtitles** — `-readrate` paces an input off its *furthest-behind* stream, and a PGS/DVD subtitle read through the video's own `-i` is sparse enough to drag the whole process to **0.53x realtime** against the 1.0x a client consumes, draining the buffer until the channel stalls. Fixed with a capability-gated `-readrate_catchup` (ffmpeg 8.0+) on realtime inputs, keeping `-readrate` on the frame-producing path so the `ffmpeg.qsv-extra-hw-frames-floor` bound is untouched; measured 0.533x → 1.067x on QSV and software, with a 240s QSV soak clean of allocation errors (#726). Affects items carrying an embedded bitmap subtitle matching the channel's subtitle mode — 3,182 of 24,646 media versions on prod, and a property of the *item*, not the channel, which is why the stall presented as random. Process/CI: the H10 review-verdict gate's repair sentinel became a fixed point and its write is now fenced on the timeline retarget count, closing a raced-sentinel false-open (#706/#707/#711). **The decisions validator now cross-checks its dependency-free frontmatter parse against PyYAML** and reports both the truncating unquoted `` #`` and the scalar-closing bare apostrophe as errors, so a record whose `rule:` silently halves under PyYAML fails the local gate instead of CI (#674/#688) — the ceiling-calibration claim was also split so the suite pins what the derivation MEANS rather than live-corpus order statistics. Dependencies: CliWrap 3.10.4, JetBrains.ReSharper.GlobalTools 2025.3.5. |
|
||||
| `v26.15.0` | **`main` became PR-only, which is what makes `review-verdict/h10` load-bearing** — branch protection now carries `enable_push: false` *and* `block_admin_merge_override: true`, so a direct `git push origin HEAD:main` is refused at pre-receive for every account including a site admin, and an admin cannot force-merge past a missing or red required context (#743/#749). Gitea only evaluates `status_check_contexts` on the PR merge path, so before this the whole gate was skippable with no forgery needed. The gate was then hardened through the rest of its false-open chain: CI's registry credential scoped so a head-resolved workflow cannot forge the status (#697), a stray expression delimiter **in a comment** that killed the gate outright and is now banned on the release path itself by a `scan` job `build` depends on (#751/#767), a positive verdict inherited only from an allow-listed reviewer (#742), the PR-timeline `HEAD` alias fenced on the `pull_push` count (#803/#664), and both `/statuses/{sha}` reads paged to a validated terminator (#763). App-facing: **FFmpeg profile numeric fields are bounded with a 422 that names the bound and its consequence** instead of being silently rewritten, and readrate pacing is exposed as a profile field (#735 — one dual-provider migration, `Add_FFmpegProfile_ReadRatePacing`). **On Now / Next gets a boxed background and is on by default** (#732). Field-level progressive disclosure lands as a shared `FieldHelp` trigger + portalled panel, adopted on FFmpeg Profiles and documented as `docs/spa-conventions.md` §15 (#734). Auto-Tune proposal rows align on a grid (#721); `AddItemsDialog` resolves by search instead of windowing a whole media-library type, with stale-response and is-mounted guards (#685/#740). Correctness: a paged handler now counts **the same query it pages**, so a filtered page stops reporting the unfiltered total across playouts, rerun collections and six sibling handlers (#690/#758); nullable primitive collections are guarded at the read site rather than mutated on a tracked entity — `SongMetadata.Artists` in both search indexers (#701/#824) and the scheduling `DaysOfMonth`/`MonthsOfYear`/`DaysOfWeek` columns, where a runtime NULL reads as **unrestricted** (#823); SPA full-replace bodies are built as `Complete<T>` so an omitted optional DTO field is a type error (#807); and every `ErsatzTV.Mcp` tool is pinned to its OpenAPI contract, with `graphicsElementIds` and `padToNearestMinute` declared so an MCP channel update stops silently stripping the overlay (#754/#757). Process/CI: one rule for how a guard derives its population and one for the proof it ships with, both enforced (#774/#775) — populations come from the git index, never a filesystem walk (#806/#819), every `MUTATION` grade is executed each suite rather than asserted in prose (#790), and verification code is itself code under test (#796). Workflows declare their own per-job metadata and guard populations derive from it (#786/#789); all six declare `permissions:` (#748); `ci-image.yml` publishes from `main` only and the persisted checkout credential is gone (#744/#746/#835). Hooks report their own execution and the report is measured (#776), and `docs.no-session-narrative` was generalized to every durable artifact and the sites its sweep IDENTIFIED were remediated as judgements rather than by regex — no closure is claimed over a phrasing space (#784/#812). A ruff config is committed and enforced in CI (#780). Dependencies: Scriban.Signed 7.2.6. **Cut at `736649b3b`, not at `main`'s head**: `completeAnnotations.guard.test.ts` (merged in the very next commit) needs a git index the Dockerfile's `web-build` stage does not have, so every image build after `736649b3b` fails — including a tag build (#887). `736649b3b` is the newest commit whose `Build & push image (amd64)` actually ran green. The scheduling-recurrence fix #880 therefore slips to the next release. |
|
||||
| `v26.16.0` | **Channels carrying Jellyfin music videos play again** — the music-video scanner wrote the library-items projection's empty stream list straight through, so every Jellyfin music video was stored with no `MediaStream` rows (1520/1520 on prod) and `SelectVideoStream` threw on tune-in while the guide looked normal; it now reads statistics from per-item playback info under the movie scanner's conditions, writes nothing when the server cannot answer, and `SelectVideoStream` fails with the version, file and remedy instead of a bare `Sequence contains no matching element`. **Existing rows heal only on a deep scan** (`POST /api/v1/libraries/{id}/scan?deep=true`), since a regular scan skips unchanged etags (#924/#928, `scan.mediaserver-statistics-from-server`). **Alternate-schedule and template recurrence arrays: an absent array means unrestricted, an explicit `[]` is rejected with a 422 that names the consequence** — the three recurrence sets are read conjunctively, so `?? []` on an omitted array returned 200 while storing an item that could never apply; absent and null now normalize to the same `All*()` sets the read side substitutes, and an unchanged pre-existing empty set is still accepted so no other item in a whole-list PUT becomes uneditable (#880/#892 — slipped from v26.15.0). **Unknown channel `graphicsElementIds` are rejected with a 422** (and the same bounded id-list validation applies to Deco updates), and the built-in graphics elements are identified by the seeded path, not the filename (#568/#922). The channel preview says why it is not playing and only the preview opts into muted autoplay (#554/#910); a failed write in the Collections screen reports to a surface that outlives the dialog that started it (#830/#878). Correctness: a classifier-unreadable branch-protection rule (a glob rule with an empty `branch_name`) is no longer resolved to a rule that matches nothing, which had let the merge hook auto-grant against a rule Gitea may be applying (#858/#859/#897); an empty PR-timeline page before the 20-page cap is no longer read as exhaustion, which a PR author could use to hide an ABA force-push behind 50 comments — narrowed, not closed (#870/#896); a head-authored run holds no stored secret, so the PR route reads the registry and status API anonymously (#885/#919); `Complete<T>` annotations are now derived where applied, not only where defined (#820/#883). **Image builds fixed**: the Dockerfile's `web-build` stage built the SPA and then ran a vitest suite with a hand-written `--exclude` list, so `completeAnnotations.guard.test.ts` (needs a git index the stage lacks) failed every `main` and tag image build after `736649b3b`; the stage now builds the SPA and does not test it, the suite runs once unfiltered in the `test` job `build` depends on, and a guard pins that delegation (#887/#899). Process/CI: the verdict gate replaces every unknown state and proves the clauses that claim to (#849/#890), the verdict writer checks the gate will honour what it just posted (#845/#889), a sourced path is code so every hook resolves it from its own tree (#891/#903), a mutation-outcome claim is executed wherever it is written (#881/#914), `--depth` is never passed to a checkout that may be complete (#836/#884), session narrative is swept from hooks, workflows, scripts, tests and code comments (#876/#906), a scripted-engine build API is characterized in-process against a committed script fixture (#563/#915), orchestrated sessions get a mechanics page, standing prompt and pick/build/resume workflows (#907/#908) with the review loop and concurrency cap tightened (#911/#912/#916/#918), and CI claims dated at Gitea 1.25.4 are re-established on 1.27.1 (#869/#893/#905). No dual-provider migrations and no dependency bumps in this release. **The image moves to the .NET 10.0.14 runtime** (prod had 10.0.11), so it carries the October 2026 .NET out-of-band security fixes, including the Brotli decompression overflow (CVE-2026-111497) and the SocketAsyncEventArgs memory corruption (CVE-2026-70197). No code change was needed, because `docker/Dockerfile` follows the floating `aspnet`/`sdk:10.0-noble-amd64` tags. Release scan on the immutable `:26.16.0` (`sha256:6d698162…`), 2026-10-27: ZAP 0 FAIL / 120 PASS / 1 WARN (100001, the SPA catch-all's `302 → /app` HTML, by design); semgrep 2 rule classes, both false positives (a constant-SQL test helper; JSX inside `design-system/` demo HTML, which the app doesn't bundle). **Known advisories carried unchanged from v26.15.0**: `System.Security.Cryptography.Pkcs` 9.0.4 (High, transitive via the unused Oracle/SQL Server BulkExtensions providers) and `SixLabors.ImageSharp` 3.1.12 (Moderate, fix only in 4.x), tracked in #929. |
|
||||
|
||||
**Before cutting a release — sweep `docs/decisions.md` + `docs/decisions/`** (ersatztv#521, supersedes
|
||||
the ersatztv#303 H9 append-only ritual). Supersession/retirement is now a same-PR act (add the new
|
||||
@@ -106,10 +107,10 @@ commit in the range (see the `decisions.md` header) — routine lifecycle metada
|
||||
> tag-protection rule (`name_pattern: v*`, `whitelist_usernames: ["timothy"]`,
|
||||
> `release.tag-protection-v-star`), because a `v*` push builds and publishes `:prod`. A push from
|
||||
> any other account is expected to be refused — configured, not exercised, since only the
|
||||
> `timothy` credential is available here. **The positive half is UNVERIFIED too**: the rule was
|
||||
> applied 2026-09-04 and only its negative half was measured (a non-`v*` tag still pushes), because
|
||||
> pushing a `v*` tag outside a real cut would publish `:prod`. This cut is its first real
|
||||
> exercise. If the tag push is refused, unblock with
|
||||
> `timothy` credential is available here. The positive half is verified: the rule was
|
||||
> applied 2026-09-04, its negative half measured that day (a non-`v*` tag still pushes), and a real
|
||||
> `v*` cut by `timothy` succeeded with the rule in force on 2026-10-27 (v26.16.0). If the tag push
|
||||
> is refused, unblock with
|
||||
> `DELETE /api/v1/repos/timothy/ersatztv/tag_protections/1`, push, then re-`POST` the rule
|
||||
> (prefer a `PATCH` adding the account if the cut has moved to a different operator, so the
|
||||
> protection is never left off) — and record the outcome on `release.tag-protection-v-star`,
|
||||
@@ -1303,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
|
||||
@@ -1330,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 —
|
||||
@@ -1339,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.
|
||||
@@ -1360,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
|
||||
@@ -1462,10 +1475,10 @@ they are fail-open and `--no-verify` bypasses them. Tag pushes go through a sepa
|
||||
are meant to stay unaffected for the release operator: since ersatztv#885 `tag_protections` carries
|
||||
one rule, `v*` whitelisted to `timothy` (`release.tag-protection-v-star`), which is intended to leave
|
||||
the release cut in "Cutting a release" working unchanged while refusing a `v*` tag from the `renovate`
|
||||
bot — a push that would otherwise have published a `:prod` image. Both halves of that are EXPECTED,
|
||||
UNVERIFIED: only the `timothy` credential exists here, so neither a real release cut nor a refused bot
|
||||
push has been exercised. `release.tag-protection-v-star` carries the same caveat and waits on the
|
||||
next cut.
|
||||
bot — a push that would otherwise have published a `:prod` image. The release-cut half is verified
|
||||
(the v26.16.0 cut, 2026-10-27, pushed its `v*` tag as `timothy` with the rule in force); the refused
|
||||
bot push is still UNVERIFIED, because only the `timothy` credential exists here
|
||||
(`release.tag-protection-v-star`).
|
||||
|
||||
**A fourth arm: guard-scope freshness (ersatztv#787).** `scripts/tests/test_ci_dropped_step_guard.py`
|
||||
decides which CI jobs must carry per-step execution markers by DERIVING that scope from
|
||||
@@ -2647,8 +2660,10 @@ running product from outside our C#/review stack.
|
||||
`/api/v1` operation, injecting `X-Api-Key` on every request via a ZAP replacer rule so it reaches the
|
||||
`[RequiresAuthentication]` + `RequireKeyForReads` surface (not just the `/app` shell an unauthenticated
|
||||
spider sees). Then a **semgrep** SAST cross-check (`p/security-audit` + `p/secrets` + `p/csharp`). The
|
||||
container is torn down on exit.
|
||||
- **Where/when.** Runs on the **docker host** (jazz — the Mac has no docker), like
|
||||
container is torn down on exit. **semgrep runs only if it is installed on that host**, and otherwise the
|
||||
script prints a WARN and still exits 0. As of 2026-10-27 bumblebee has no semgrep, so run it separately
|
||||
against a `git archive` of the tagged commit (the Mac has it) and triage both halves.
|
||||
- **Where/when.** Runs on a **docker host** with registry access (bumblebee, as the script's own header names, or jazz), like
|
||||
`migration-smoke.sh`: `scripts/security-scan.sh [IMAGE] [PORT]` (defaults `…:latest` / `8411`). It is a
|
||||
**manual release-gate**, deliberately **not** a per-PR CI job — it needs docker + a booted image, takes
|
||||
several minutes, and is noisy (expect to tune, not take raw). The continuous layer is the per-PR
|
||||
|
||||
File diff suppressed because one or more lines are too long
+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,50 @@
|
||||
---
|
||||
key: api.scripted-build-token
|
||||
title: '2026-10-29 — A scripted playout''s program authenticates its callbacks with a build-scoped token, not by exempting the callback controller from API auth (#913)'
|
||||
status: active
|
||||
since: '2026-10-29'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
rule: '`ScriptedScheduleController` (`/api/v1/scripted/playout/build/{buildId:guid}`) stays under the global `ApiAuthorizationFilter` and is NOT `[SkipApiAuthorization]`. It carries `[AcceptsScriptedBuildToken]`, which lets the filter accept one more credential on that controller alone: the token `ScriptedPlayoutBuilderService.StartSession` mints for each build (256 bits from the CSPRNG, independent of the build id), presented in the `X-Scripted-Build-Token` header, compared in constant time, and valid only for the build named by the `buildId` route value and only until `EndSession`. `ScriptedPlayoutBuilder` hands it to the program in the `ETV_SCRIPTED_BUILD_TOKEN` environment variable — never on the command line, which is logged and visible in the process table, and whose positional contract (host, build id, mode, then the user''s own arguments) is unchanged. The machine key and a session are still accepted there; a request carrying none of the three is a 401. The token is a custom header, so like the machine key it needs no `X-CSRF`.'
|
||||
signals: 'scripted playout build 401 · scripted script gets no credential · build-scoped credential · ETV_SCRIPTED_BUILD_TOKEN · X-Scripted-Build-Token · AcceptsScriptedBuildToken · why not SkipApiAuthorization on the scripted controller · scripted build silently empty · paths: `ErsatzTV/Filters/ApiAuthorizationFilter.cs`, `ErsatzTV/Filters/AcceptsScriptedBuildTokenAttribute.cs`, `ErsatzTV/Controllers/Api/ScriptedScheduleController.cs`, `ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilderService.cs`, `ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilder.cs`, `scripts/scripted-schedules/entrypoint.py` · issues: #913, #563, #285, #197'
|
||||
mechanics: '`docs/api-conventions.md` §9 (the credential list); pinned by `ErsatzTV.Tests/Filters/ScriptedScheduleAuthorizationTests.cs` (the filter decision over the endpoint metadata MVC builds from the production `Startup`) and `ErsatzTV.Core.Tests/Scheduling/ScriptedScheduling/ScriptedBuildTokenTests.cs` (minting, validation, and the launch environment)'
|
||||
---
|
||||
|
||||
**The defect this settles.** The program a scripted playout runs calls back into
|
||||
`ScriptedScheduleController` over `http://localhost:{UiPort}`, and that controller has been under the
|
||||
fail-closed global filter since `security.fail-closed-api-auth`. The program was given no credential, so
|
||||
every callback was a 401. A program that exits 0 after that failure — `entrypoint.py` catches the
|
||||
client's `ApiException` and does — leaves a build that reports success with no items. Reproduced against
|
||||
a live instance on 2026-10-29; the evidence is on #913.
|
||||
|
||||
**Why not `[SkipApiAuthorization]`, the one-line fix.** It would restore upstream behaviour for every
|
||||
existing script, but it widens the auth-exempt set (`ApiControllerSecurityTests` pins it at the scanner
|
||||
and auth controllers) by a whole MUTATING controller, and what would then stand in for the credential is
|
||||
weaker than it looks:
|
||||
|
||||
- The host check in `Startup` does not make the endpoint localhost-only: a request on `Settings.UiPort`
|
||||
passes it from any LAN peer. `[LocalhostOnly]`, the scanner's mitigation, reads
|
||||
`RemoteIpAddress`, which the default trust-all `ForwardedHeaders` lets any peer set with
|
||||
`X-Forwarded-For` (`security.session-auth-dual-credential`).
|
||||
- The remaining gate would be the build id, which is logged at Information level on every build and is
|
||||
in the program's argv. #285 already rejected a GUID as the sole gate for the scanner callback.
|
||||
|
||||
**Why not the machine key.** Handing the program `X-Api-Key` would work with no filter change, and the
|
||||
program, which runs as the server's own user, could read `/config/api.key` anyway. It was rejected
|
||||
because a leak is unbounded: a key that a user script echoes to stdout is the whole API for as long as
|
||||
the key lives. A leaked build token opens one in-memory build's callbacks for at most the build timeout.
|
||||
|
||||
**Why the environment and not a fourth argument.** Arguments after the mode belong to the user: the
|
||||
fork's own driver takes its script module name there. Inserting a positional would shift them by one and
|
||||
break every existing script, and the argument list is the thing the server logs.
|
||||
|
||||
**What a script must now do.** Read `ETV_SCRIPTED_BUILD_TOKEN` and send it as `X-Scripted-Build-Token`
|
||||
on every callback. `entrypoint.py` does this with `ApiClient.set_default_header`, because the
|
||||
`scripted-schedule` OpenAPI document declares no security scheme, so the generated `etv_client` has no
|
||||
setting for it. A third-party script that ignores the variable gets the same 401 it got before this
|
||||
record; that compatibility cost is accepted, since that script never worked against this fork either.
|
||||
|
||||
**Residue, named.** The filter decision is tested in-process against the real MVC metadata; the Kestrel
|
||||
hop and the `Cli.Wrap` launch remain outside the automated suite (`testing.scripted-engine-in-process-net`),
|
||||
so `ScriptedPlayoutBuilder.Build` reaching `CreateCommand`, the one place the variable is set, is held by
|
||||
the live run on #913 rather than by a test.
|
||||
@@ -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
|
||||
|
||||
@@ -5,9 +5,9 @@ status: active
|
||||
since: '2026-09-04'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
rule: 'The repository carries one tag-protection rule: `name_pattern: v*`, `whitelist_usernames: ["timothy"]`. This matters because `docker-build.yml` triggers on `push.tags: [v*]` and a `v*` tag push BUILDS AND PUBLISHES the `:prod` image — the tag the `jazz-media` Komodo stack follows. `release.main-direct-push-disabled` closed every route to `main`, and left this one open: a tag push is a separate mechanism that branch protection does not govern, so it was the cheapest remaining single act that reaches production. It buys NOTHING against the admin actor (`timothy` is on the whitelist, and no self-administered control bounds an owner) and everything it buys is against `renovate`, the repo''s other write-capable account (`permission: write`, `is_admin: false` from `GET /repos/timothy/ersatztv/collaborators/renovate/permission`, measured in #853 2026-08-30 and re-read 2026-09-04): the H10 verdict gate and the Renovate exemption rule bound what that bot can get MERGED, and nothing bounded what it could TAG. The rule''s failure mode is a BROKEN RELEASE CUT, so it is verified in both directions before it is trusted: the negative half is done and recorded below; the POSITIVE half — a real `v*` release-cut tag push by `timothy` still succeeding — is DEFERRED to the operator''s next real cut, because pushing a `v*` tag from any working session publishes `:prod` and a release is the operator''s act, not a verification step. Treat that as OPEN until a cut lands. There is no committed mirror of this rule: it lives in Gitea, and this record''s `mechanics:` carries the exact call and the read-back, the same shape `release.main-direct-push-disabled` uses.'
|
||||
rule: 'The repository carries one tag-protection rule: `name_pattern: v*`, `whitelist_usernames: ["timothy"]`. This matters because `docker-build.yml` triggers on `push.tags: [v*]` and a `v*` tag push BUILDS AND PUBLISHES the `:prod` image — the tag the `jazz-media` Komodo stack follows. `release.main-direct-push-disabled` closed every route to `main`, and left this one open: a tag push is a separate mechanism that branch protection does not govern, so it was the cheapest remaining single act that reaches production. It buys NOTHING against the admin actor (`timothy` is on the whitelist, and no self-administered control bounds an owner) and everything it buys is against `renovate`, the repo''s other write-capable account (`permission: write`, `is_admin: false` from `GET /repos/timothy/ersatztv/collaborators/renovate/permission`, measured in #853 2026-08-30 and re-read 2026-09-04): the H10 verdict gate and the Renovate exemption rule bound what that bot can get MERGED, and nothing bounded what it could TAG. The rule''s failure mode is a BROKEN RELEASE CUT, so it is verified in both directions before it is trusted: both halves are done and recorded below: the negative half on 2026-09-04, and the POSITIVE half — a real `v*` release-cut tag push by `timothy` succeeding with the rule in force — on the v26.16.0 cut, 2026-10-27. There is no committed mirror of this rule: it lives in Gitea, and this record''s `mechanics:` carries the exact call and the read-back, the same shape `release.main-direct-push-disabled` uses.'
|
||||
signals: 'tag_protections empty, v* tag push publishes prod, release cut blocked, renovate can push a tag, tag protection whitelist, release operator whitelist, prod image published by tag, deferred positive verification · paths: `.gitea/workflows/docker-build.yml` · issues: #885, #853, #743, #698, #742'
|
||||
mechanics: 'Applied 2026-09-04 with `POST /api/v1/repos/timothy/ersatztv/tag_protections` body `{"name_pattern": "v*", "whitelist_usernames": ["timothy"]}`, which returned `{"id": 1, "name_pattern": "v*", "whitelist_usernames": ["timothy"], "whitelist_teams": [], "created_at": "2026-09-04T23:18:50+02:00"}`. Read back the same day with `GET .../tag_protections` -> the one rule, fields as echoed. It is reversible with `DELETE .../tag_protections/1`. NOTE THE ROUTE NAME: `/repos/{owner}/{repo}/tags/protection` is NOT it — measured 2026-09-04, that path resolves `protection` as a TAG NAME and answers 404 `{"message": "tag doesn''t exist: protection"}`. Before this the endpoint returned `[]` (measured #743 2026-08-24 and again 2026-09-04). NEGATIVE VERIFICATION available from this session and performed: a NON-`v*` tag (`probe-885-tagpush`) pushed by `timothy` succeeded and was deleted, confirmed gone by `git ls-remote --tags` — so tag pushes still work at all and the rule is not mis-scoped to `*`. NOT VERIFIED, and it cannot be from a working session: that a real `v*` release-cut push still succeeds, and that a push by `renovate` is refused (only the `timothy` credential is available here). The first is the operator''s next cut; the second would require the bot''s credential.'
|
||||
mechanics: 'Applied 2026-09-04 with `POST /api/v1/repos/timothy/ersatztv/tag_protections` body `{"name_pattern": "v*", "whitelist_usernames": ["timothy"]}`, which returned `{"id": 1, "name_pattern": "v*", "whitelist_usernames": ["timothy"], "whitelist_teams": [], "created_at": "2026-09-04T23:18:50+02:00"}`. Read back the same day with `GET .../tag_protections` -> the one rule, fields as echoed. It is reversible with `DELETE .../tag_protections/1`. NOTE THE ROUTE NAME: `/repos/{owner}/{repo}/tags/protection` is NOT it — measured 2026-09-04, that path resolves `protection` as a TAG NAME and answers 404 `{"message": "tag doesn''t exist: protection"}`. Before this the endpoint returned `[]` (measured #743 2026-08-24 and again 2026-09-04). NEGATIVE VERIFICATION available from this session and performed: a NON-`v*` tag (`probe-885-tagpush`) pushed by `timothy` succeeded and was deleted, confirmed gone by `git ls-remote --tags` — so tag pushes still work at all and the rule is not mis-scoped to `*`. POSITIVE VERIFICATION, 2026-10-27: the v26.16.0 release-cut push (`git push origin v26.16.0` as `timothy`, tag on `1f7b763a7`) succeeded and `git ls-remote --tags origin v26.16.0` read it back; `GET .../tag_protections` the same day returned rule `id: 1` unchanged (`updated_at` 2026-09-04T23:18:50+02:00), so the push ran WITH the rule in force, not past a deleted one. STILL NOT VERIFIED: that a push by `renovate` is refused, which needs the bot''s credential (only `timothy`''s is available here).'
|
||||
---
|
||||
|
||||
**What this closes.** `release.main-direct-push-disabled` made every change to `main` go through a PR
|
||||
|
||||
@@ -5,8 +5,8 @@ status: active
|
||||
since: '2026-09-05'
|
||||
supersedes: testing.scripted-playout-golden-deferred@2026-07-22
|
||||
superseded-by: none
|
||||
rule: 'Scripted playout is characterized at two in-process levels and no further. (1) `SchedulingEngineTests` drives the engine build API directly with substituted repositories — the same setup order `ScriptedPlayoutBuilder` uses (`WithPlayoutId`/`WithMode`/`WithSeed`/`BuildBetween`/`WithReferenceData`/`RestoreOrReset`, and `WithReferenceData` MUST precede the last two or `PlayoutReferenceData.PlayoutHistory` NREs). (2) `ScriptedScheduleControllerTests` replays a committed script fixture (`ErsatzTV.Tests/Controllers/Fixtures/scripted-build.json`) through the REAL `ScriptedScheduleController` + `ScriptedPlayoutBuilderService.MockSession` + `SchedulingEngine` and pins the resulting `PlayoutItem`s as a snapshot in the golden line format — so exactly ONE action-to-engine mapping exists in the repo, the production one. Fixture bodies in that replay are deserialized with `ApiJsonSettings`, the configuration `Startup` applies to `AddNewtonsoftJson`, invoked from the same function rather than mirrored — and two tests WITNESS that choice instead of asserting it, each binding a body that separates the production binder from one plausible replacement: `Production_Body_Binder_Ignores_Required_Members` omits a C# `required` member, which Newtonsoft defaults and System.Text.Json rejects; `Production_Body_Binder_Keeps_Declared_Defaults_Over_An_Explicit_Null` sends an explicit `"order": null`, which `NullValueHandling.Ignore` drops and a bare `JsonSerializerSettings` writes through into a 400. What is shared is the CONFIGURATION, never MVC''s settings OBJECT: `ApiJsonSettings.Create()` applies it to a bare settings object, so MVC''s `MaxDepth` of 32 and its `ProblemDetails`/`ValidationProblemDetails` converters are absent (measured 2026-09-05, pinned by `ApiJsonSettingsTests`); neither is reachable from a scripted request body, and no test may generalize from `Create()` to production past that. What no test observes is the `Startup` registration itself, so the extraction REMOVES the duplicate rather than detecting its drift. The `Cli.Wrap` launch of the user-authored program (exit code, timeout, stdout capture), the Kestrel/middleware/`ApiAuthorizationFilter` transport it calls back over, and MVC model binding as a WRAPPER (the input formatter, model validation and the `[ApiController]` automatic 400 either produces before an action runs — the serializer inside that wrapper is covered, the wrapper around it is not) are NOT tested and are not scheduled to be; that residue is named below rather than argued away. Both fixtures must aim every trimming instruction at an instant BETWEEN two content boundaries: a pad or duration that happens to land on one never enters the engine''s trim branch, so that action''s `trim` argument reaches nothing the snapshot can show. The controller is a thin adapter, NOT a 1:1 pass-through: an unparseable playback order is a 400, an unparseable filler kind SILENTLY degrades to `FillerKind.None`, an unknown build id is a 404, and the engine no-progress `InvalidOperationException` is translated to a 400 — each pinned by a test, because a prose claim of equivalence decays.'
|
||||
signals: 'why is there no scripted golden; scripted integration harness; scripted transport vs engine; SchedulingEngine is in-process testable; Cli.Wrap external process is transport only; shell-out scoped out; MockSession seam; in-process stand-in for a scripted script; engine build API characterization; committed script fixture replay; ScriptedScheduleController is an adapter not a pass-through; scripted filler kind silently falls back to None; scripted playout no-progress halt maps to 400; scripted build 401 blind spot; EnumeratorForContent · paths: `ErsatzTV.Core.Tests/Scheduling/Engine/SchedulingEngineTests.cs`, `ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs`, `ErsatzTV.Tests/Controllers/Fixtures/scripted-build.json`, `ErsatzTV.Tests/Serialization/ApiJsonSettingsTests.cs`, `ErsatzTV/Serialization/ApiJsonSettings.cs`, `ErsatzTV.Core/Scheduling/Engine/SchedulingEngine.cs`, `ErsatzTV/Controllers/Api/ScriptedScheduleController.cs`, `ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilder.cs`, `ErsatzTV.Core.Tests/Scheduling/Goldens/PlayoutBuildGoldenTests.cs` · issues: #563, #381, #395, #163'
|
||||
rule: 'Scripted playout is characterized at two in-process levels and no further. (1) `SchedulingEngineTests` drives the engine build API directly with substituted repositories — the same setup order `ScriptedPlayoutBuilder` uses (`WithPlayoutId`/`WithMode`/`WithSeed`/`BuildBetween`/`WithReferenceData`/`RestoreOrReset`, and `WithReferenceData` MUST precede the last two or `PlayoutReferenceData.PlayoutHistory` NREs). (2) `ScriptedScheduleControllerTests` replays a committed script fixture (`ErsatzTV.Tests/Controllers/Fixtures/scripted-build.json`) through the REAL `ScriptedScheduleController` + `ScriptedPlayoutBuilderService.MockSession` + `SchedulingEngine` and pins the resulting `PlayoutItem`s as a snapshot in the golden line format — so exactly ONE action-to-engine mapping exists in the repo, the production one. Fixture bodies in that replay are deserialized with `ApiJsonSettings`, the configuration `Startup` applies to `AddNewtonsoftJson`, invoked from the same function rather than mirrored — and two tests WITNESS that choice instead of asserting it, each binding a body that separates the production binder from one plausible replacement: `Production_Body_Binder_Ignores_Required_Members` omits a C# `required` member, which Newtonsoft defaults and System.Text.Json rejects; `Production_Body_Binder_Keeps_Declared_Defaults_Over_An_Explicit_Null` sends an explicit `"order": null`, which `NullValueHandling.Ignore` drops and a bare `JsonSerializerSettings` writes through into a 400. What is shared is the CONFIGURATION, never MVC''s settings OBJECT: `ApiJsonSettings.Create()` applies it to a bare settings object, so MVC''s `MaxDepth` of 32 and its `ProblemDetails`/`ValidationProblemDetails` converters are absent (measured 2026-09-05, pinned by `ApiJsonSettingsTests`); neither is reachable from a scripted request body, and no test may generalize from `Create()` to production past that. What no test observes is the `Startup` registration itself, so the extraction REMOVES the duplicate rather than detecting its drift. The `Cli.Wrap` launch of the user-authored program (exit code, timeout, stdout capture), the Kestrel/middleware transport it calls back over, and MVC model binding as a WRAPPER (the input formatter, model validation and the `[ApiController]` automatic 400 either produces before an action runs — the serializer inside that wrapper is covered, the wrapper around it is not) are NOT tested and are not scheduled to be; that residue is named below rather than argued away. The `ApiAuthorizationFilter` DECISION for this controller is not residue: `ScriptedScheduleAuthorizationTests` drives the real filter over the endpoint metadata MVC builds from the production `Startup`, which is how the build-scoped credential of `api.scripted-build-token` is pinned (#913). Both fixtures must aim every trimming instruction at an instant BETWEEN two content boundaries: a pad or duration that happens to land on one never enters the engine''s trim branch, so that action''s `trim` argument reaches nothing the snapshot can show. The controller is a thin adapter, NOT a 1:1 pass-through: an unparseable playback order is a 400, an unparseable filler kind SILENTLY degrades to `FillerKind.None`, an unknown build id is a 404, and the engine no-progress `InvalidOperationException` is translated to a 400 — each pinned by a test, because a prose claim of equivalence decays.'
|
||||
signals: 'why is there no scripted golden; scripted integration harness; scripted transport vs engine; SchedulingEngine is in-process testable; Cli.Wrap external process is transport only; shell-out scoped out; MockSession seam; in-process stand-in for a scripted script; engine build API characterization; committed script fixture replay; ScriptedScheduleController is an adapter not a pass-through; scripted filler kind silently falls back to None; scripted playout no-progress halt maps to 400; scripted build 401 blind spot; scripted build token; EnumeratorForContent · paths: `ErsatzTV.Core.Tests/Scheduling/Engine/SchedulingEngineTests.cs`, `ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs`, `ErsatzTV.Tests/Controllers/Fixtures/scripted-build.json`, `ErsatzTV.Tests/Serialization/ApiJsonSettingsTests.cs`, `ErsatzTV/Serialization/ApiJsonSettings.cs`, `ErsatzTV.Core/Scheduling/Engine/SchedulingEngine.cs`, `ErsatzTV/Controllers/Api/ScriptedScheduleController.cs`, `ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilder.cs`, `ErsatzTV.Core.Tests/Scheduling/Goldens/PlayoutBuildGoldenTests.cs`, `ErsatzTV.Tests/Filters/ScriptedScheduleAuthorizationTests.cs` · issues: #563, #381, #395, #163, #913'
|
||||
mechanics: docs/testing.md → "Scripted playout coverage" and "Golden-file nets"
|
||||
---
|
||||
|
||||
@@ -44,9 +44,10 @@ wrong — the four adapter mappings above are real behaviour that only a test ca
|
||||
forwarded-headers / compression / SPA-hosting middleware; MVC model binding as a *wrapper* — the input
|
||||
formatter (content-type selection, malformed-body handling), model validation (a non-nullable reference
|
||||
type picks up an implicit required check there) and the `[ApiController]` automatic 400 either produces
|
||||
before an action runs, since every test hands an action an already-bound object; and
|
||||
`ApiAuthorizationFilter`. The *serializer* inside that wrapper is deliberately not on this list — it is
|
||||
`ApiJsonSettings` itself, and two tests hold the two ways it can be replaced. `ApiJsonSettings` exists so
|
||||
before an action runs, since every test hands an action an already-bound object. Two things are
|
||||
deliberately NOT on this list. The `ApiAuthorizationFilter` *decision* is covered (see below). The
|
||||
*serializer* inside the binding wrapper is `ApiJsonSettings` itself, and two tests hold the two ways it
|
||||
can be replaced. `ApiJsonSettings` exists so
|
||||
that binder is *defined* once; it is not a drift **detector**, and the branch may not claim one, because
|
||||
nothing observes `Startup.ConfigureServices` and a byte-equal hand-copy of `Apply` is indistinguishable
|
||||
by construction. What the suites do separate is a mirror that has *lost* something. The mechanics doc
|
||||
@@ -62,13 +63,13 @@ ran them — dated. Durable prose carries
|
||||
the mechanism each test is built on instead — which serializer difference, which engine branch — because
|
||||
that is re-checkable by reading the code rather than by trusting a remembered outcome.
|
||||
|
||||
`ApiAuthorizationFilter` on that list is not hypothetical — `ScriptedScheduleController` carries no
|
||||
`[SkipApiAuthorization]` while
|
||||
`ScriptedPlayoutBuilder` passes the script only host, build id and mode, with no `X-Api-Key`, and
|
||||
`entrypoint.py` builds its client with no credential. Whether a scripted build therefore 401s on its first
|
||||
POST in this fork is unverified against a live instance and is tracked in ersatztv#913; it is recorded here
|
||||
as the concrete shape of what an engine-level net cannot see, so the scope-out is not read as a claim of
|
||||
equivalence.
|
||||
**The filter decision is covered; the hop to it is not.** An engine-level net cannot see
|
||||
`ApiAuthorizationFilter`, and that blind spot was real: the program was handed no credential, so every
|
||||
callback was a 401 and the build came back empty (reproduced live 2026-10-29, ersatztv#913). The fix,
|
||||
`api.scripted-build-token`, is held by `ScriptedScheduleAuthorizationTests`, which drives the real filter
|
||||
over the endpoint metadata MVC builds for `ScriptedScheduleController` from the production `Startup` —
|
||||
so a controller-level attribute added, dropped or renamed is seen as it is at runtime. Kestrel, the
|
||||
middleware and the `Cli.Wrap` launch that carries the token in its environment stay out, as above.
|
||||
|
||||
**Determinism constraints on the fixtures.** Both use Chronological order and only instant-preserving
|
||||
instructions (`AddCount`, `AddAll`, `AddDuration`, `PadUntilExact`). `WaitUntil(TimeOnly)` and
|
||||
|
||||
@@ -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** |
|
||||
|
||||
|
||||
+9
-5
@@ -74,6 +74,8 @@ at all (ersatztv#563, decision `testing.scripted-engine-in-process-net`).
|
||||
| `ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs` | A committed script fixture (`Controllers/Fixtures/scripted-build.json`), bound with the production body-binder configuration (`ApiJsonSettings`) and replayed through the real `ScriptedScheduleController` + `ScriptedPlayoutBuilderService.MockSession` + `SchedulingEngine`, with a pinned 13-item snapshot in the same line format the playout goldens use (both trimming actions target an instant *between* two content boundaries, so each one's `trim` argument reaches the engine's trim branch rather than sitting inert), plus the four adapter mappings: 404 on an unknown build id, 400 on an unparseable playback order, a silent fall back to `FillerKind.None` on an unparseable filler kind, and the engine's no-progress `InvalidOperationException` translated to a 400. |
|
||||
| `ErsatzTV.Core.Tests/Scheduling/ContentEnumeratorBuilderTests.cs` | The enumerator-construction helper the Scripted and Sequential/YAML engines share (ersatztv#395). |
|
||||
| `ErsatzTV.Tests/Serialization/ApiJsonSettingsTests.cs` | What the replay's standalone binder does and does not share with the one MVC runs, so the paragraph below stays a measurement rather than a claim. |
|
||||
| `ErsatzTV.Tests/Filters/ScriptedScheduleAuthorizationTests.cs` | The `ApiAuthorizationFilter` decision for every `ScriptedScheduleController` action, driven through the real filter over the endpoint metadata MVC builds from the production `Startup`: no credential is a 401 (with `Api:RequireKeyForReads` opted off, on the mutating actions only), the running build's own token is accepted (no `X-CSRF`), another build's token, an ended build's token, a wrong or empty token and the token on an unmarked endpoint are all 401, and the machine key still works (decision `api.scripted-build-token`, ersatztv#913). |
|
||||
| `ErsatzTV.Core.Tests/Scheduling/ScriptedScheduling/ScriptedBuildTokenTests.cs` | Minting and validating the build-scoped token in `ScriptedPlayoutBuilderService`, and `ScriptedPlayoutBuilder.CreateCommand` putting it in the program's environment and not on its command line. |
|
||||
|
||||
The expected snapshot is a string constant in the test rather than a fourth golden file: the golden
|
||||
harness lives in `ErsatzTV.Core.Tests`, which cannot reference a controller, and duplicating it would
|
||||
@@ -81,15 +83,17 @@ create a second action-to-engine mapping — the thing the single-mapping design
|
||||
|
||||
**Not covered, and not scheduled to be** (measured 2026-09-05): the `Cli.Wrap` launch of the user's own
|
||||
program — exit code, the `PlayoutScriptedScheduleTimeoutSeconds` timeout, stdout capture; Kestrel and the
|
||||
`Startup` middleware, including the host/`Settings.UiPort` check that 404s a foreign request;
|
||||
`ApiAuthorizationFilter`, which fail-closes every mutating verb for an endpoint without
|
||||
`[SkipApiAuthorization]`; and MVC model binding as a *wrapper* — the input formatter, model validation
|
||||
`Startup` middleware, including the host/`Settings.UiPort` check that 404s a foreign request; that
|
||||
`ScriptedPlayoutBuilder.Build` launches through `CreateCommand`, the one place the build token is put in
|
||||
the program's environment; and MVC model binding as a *wrapper* — the input formatter, model validation
|
||||
(a non-nullable reference type picks up an implicit required check there) and the `[ApiController]`
|
||||
automatic 400 either produces before the action runs, since the replay hands each action an
|
||||
already-bound object. Hosting the real `Startup` would drag in the whole DI graph, a
|
||||
hand-rolled minimal host would test a transport the product does not have, and the script is
|
||||
user-authored by definition — so any committed script is a stand-in either way. The decision record
|
||||
names the concrete blind spot this leaves, and it is tracked as ersatztv#913.
|
||||
user-authored by definition — so any committed script is a stand-in either way. The
|
||||
`ApiAuthorizationFilter` *decision* is the exception: it is a pure function of the request and the
|
||||
endpoint metadata, so `ScriptedScheduleAuthorizationTests` covers it without a host — the blind spot the
|
||||
decision record named, which was a real 401 on every callback (ersatztv#913).
|
||||
|
||||
The *serializer* inside that binding wrapper is covered rather than scoped out. The replay deserializes
|
||||
fixture bodies through `ErsatzTV.Serialization.ApiJsonSettings`, the same function `Startup` hands to
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import argparse
|
||||
import importlib
|
||||
import os
|
||||
import sys
|
||||
from uuid import UUID
|
||||
|
||||
@@ -26,7 +27,14 @@ def main():
|
||||
|
||||
configuration = etv_client.Configuration(host=known_args.host)
|
||||
|
||||
# ErsatzTV mints a token per build and passes it in the environment; the scripted callback endpoints
|
||||
# reject a request without it (ersatztv#913). A custom header, not the machine X-Api-Key.
|
||||
build_token = os.environ.get("ETV_SCRIPTED_BUILD_TOKEN")
|
||||
|
||||
with etv_client.ApiClient(configuration) as api_client:
|
||||
if build_token:
|
||||
api_client.set_default_header("X-Scripted-Build-Token", build_token)
|
||||
|
||||
try:
|
||||
define_content = script_module.define_content
|
||||
reset_playout = script_module.reset_playout
|
||||
|
||||
@@ -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