diff --git a/ErsatzTV.Core.Tests/Scheduling/ScriptedScheduling/ScriptedBuildTokenTests.cs b/ErsatzTV.Core.Tests/Scheduling/ScriptedScheduling/ScriptedBuildTokenTests.cs new file mode 100644 index 000000000..31c91fa4f --- /dev/null +++ b/ErsatzTV.Core.Tests/Scheduling/ScriptedScheduling/ScriptedBuildTokenTests.cs @@ -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; + +/// +/// 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 ScriptedScheduleController and nowhere else — is +/// ErsatzTV.Tests/Filters/ScriptedScheduleAuthorizationTests. +/// +[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()); + ScriptedBuildSession second = service.StartSession(Substitute.For()); + + 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()); + ScriptedBuildSession other = service.StartSession(Substitute.For()); + + 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(), 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); + } +} diff --git a/ErsatzTV.Core/Interfaces/Scheduling/IScriptedPlayoutBuilderService.cs b/ErsatzTV.Core/Interfaces/Scheduling/IScriptedPlayoutBuilderService.cs index 20f095d79..eb8f57ef8 100644 --- a/ErsatzTV.Core/Interfaces/Scheduling/IScriptedPlayoutBuilderService.cs +++ b/ErsatzTV.Core/Interfaces/Scheduling/IScriptedPlayoutBuilderService.cs @@ -1,11 +1,26 @@ using ErsatzTV.Core.Scheduling.Engine; +using ErsatzTV.Core.Scheduling.ScriptedScheduling; namespace ErsatzTV.Core.Interfaces.Scheduling; public interface IScriptedPlayoutBuilderService { + /// + /// Registers an engine under a caller-chosen build id with NO token, so + /// is false for it. The in-process test seam (ersatztv#563). + /// bool MockSession(ISchedulingEngine schedulingEngine, Guid buildId); - Guid StartSession(ISchedulingEngine schedulingEngine); + + /// Registers an engine under a fresh build id and mints the build-scoped token for it. + ScriptedBuildSession StartSession(ISchedulingEngine schedulingEngine); + ISchedulingEngine GetEngine(Guid buildId); + + /// + /// True only when names a running build whose minted token equals + /// (constant-time). An ended build, a mocked build and an empty token are all false. + /// + bool IsBuildTokenValid(Guid buildId, string token); + void EndSession(Guid buildId); } diff --git a/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedBuildSession.cs b/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedBuildSession.cs new file mode 100644 index 000000000..4bf7e71fd --- /dev/null +++ b/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedBuildSession.cs @@ -0,0 +1,13 @@ +namespace ErsatzTV.Core.Scheduling.ScriptedScheduling; + +/// +/// A running scripted build: the id the program addresses it by, and the build-scoped token it +/// authenticates with (). A class rather than a record so that its +/// ToString() — and therefore a structured-log argument — never prints the token. +/// +public sealed class ScriptedBuildSession(Guid buildId, string token) +{ + public Guid BuildId { get; } = buildId; + + public string Token { get; } = token; +} diff --git a/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedBuildToken.cs b/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedBuildToken.cs new file mode 100644 index 000000000..4133fd853 --- /dev/null +++ b/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedBuildToken.cs @@ -0,0 +1,25 @@ +namespace ErsatzTV.Core.Scheduling.ScriptedScheduling; + +/// +/// The credential a scripted-playout program presents when it calls back into +/// /api/v1/scripted/playout/build/{buildId} (ersatztv#913, decision +/// api.scripted-build-token). mints one per build, hands it to +/// the program in , and the global API authorization filter accepts it in +/// — for that build's endpoints only, and only while the build is running. +/// +public static class ScriptedBuildToken +{ + /// + /// The request header the program sends the token in. A custom header, so — like the machine + /// X-Api-Key — a browser cannot attach it cross-origin, and a token-authenticated mutation needs + /// no CSRF header. + /// + public const string HeaderName = "X-Scripted-Build-Token"; + + /// + /// 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 mode) by one. + /// + public const string EnvironmentVariable = "ETV_SCRIPTED_BUILD_TOKEN"; +} diff --git a/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilder.cs b/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilder.cs index 2345d5344..df38ad1f0 100644 --- a/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilder.cs +++ b/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilder.cs @@ -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; } + /// + /// 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 + /// 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. + /// + internal static Command CreateCommand(string scriptFile, IEnumerable arguments, string buildToken) => + Cli.Wrap(scriptFile) + .WithArguments(arguments) + .WithEnvironmentVariables(env => env.Set(ScriptedBuildToken.EnvironmentVariable, buildToken)) + .WithValidation(CommandResultValidation.None); + private async Task GetDaysToBuild(CancellationToken cancellationToken) => await configElementRepository .GetValue(ConfigElementKey.PlayoutDaysToBuild, cancellationToken) diff --git a/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilderService.cs b/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilderService.cs index bd393529b..02f008743 100644 --- a/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilderService.cs +++ b/ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilderService.cs @@ -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 _sessions = new(); + private readonly ConcurrentDictionary _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); } diff --git a/ErsatzTV.Tests/Filters/ApiAuthorizationFilterTests.cs b/ErsatzTV.Tests/Filters/ApiAuthorizationFilterTests.cs index 4e508fdc6..ee1ee6f51 100644 --- a/ErsatzTV.Tests/Filters/ApiAuthorizationFilterTests.cs +++ b/ErsatzTV.Tests/Filters/ApiAuthorizationFilterTests.cs @@ -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) { diff --git a/ErsatzTV.Tests/Filters/ApiKeyEndpointRequiresKeyTests.cs b/ErsatzTV.Tests/Filters/ApiKeyEndpointRequiresKeyTests.cs index f665950bf..dcb114b2a 100644 --- a/ErsatzTV.Tests/Filters/ApiKeyEndpointRequiresKeyTests.cs +++ b/ErsatzTV.Tests/Filters/ApiKeyEndpointRequiresKeyTests.cs @@ -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(); } diff --git a/ErsatzTV.Tests/Filters/ScriptedScheduleAuthorizationTests.cs b/ErsatzTV.Tests/Filters/ScriptedScheduleAuthorizationTests.cs new file mode 100644 index 000000000..26cc65bec --- /dev/null +++ b/ErsatzTV.Tests/Filters/ScriptedScheduleAuthorizationTests.cs @@ -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; + +/// +/// The authorization decision for the scripted-playout callback surface (ersatztv#913), driven through the +/// REAL with the endpoint metadata MVC builds for +/// under the production Startup.ConfigureServices — 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 ScriptedPlayoutBuilder 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. +/// +[TestFixture] +public class ScriptedScheduleAuthorizationTests +{ + private const string MachineKey = "machine-key"; + + private static readonly IReadOnlyList 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().ShouldBeEmpty(action.ActionName); + action.EndpointMetadata.OfType().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()); + + foreach (ControllerActionDescriptor action in Actions()) + { + AuthorizationFilterContext context = MakeContext(action, session.BuildId); + Filter(service).OnAuthorization(context); + + context.Result.ShouldBeOfType(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()); + 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(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()); + + 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()); + ScriptedBuildSession other = service.StartSession(Substitute.For()); + + 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(action.ActionName); + } + } + + [Test] + public void Token_Is_A_401_Once_Its_Build_Has_Ended() + { + var service = new ScriptedPlayoutBuilderService(); + ScriptedBuildSession session = service.StartSession(Substitute.For()); + 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(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()); + + 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(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()); + + 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(); + } + + [Test] + public void Machine_Key_Is_Still_Accepted_On_Every_Scripted_Action() + { + var service = new ScriptedPlayoutBuilderService(); + ScriptedBuildSession session = service.StartSession(Substitute.For()); + + 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 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().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()); + } + + // The action descriptors MVC builds from the production service registration — the same objects whose + // EndpointMetadata the filter reads at runtime. + private static List LoadScriptedActions() + { + var settings = new Dictionary + { + ["provider"] = "sqlite", + ["ConnectionStrings:Data"] = "Data Source=:memory:" + }; + + IConfiguration configuration = new ConfigurationBuilder() + .AddInMemoryCollection(settings) + .Build(); + + var environment = Substitute.For(); + 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() + .ActionDescriptors + .Items + .OfType() + .Where(d => d.ControllerTypeInfo.AsType() == typeof(ScriptedScheduleController)) + .ToList(); + } +} diff --git a/ErsatzTV/Controllers/Api/ScriptedScheduleController.cs b/ErsatzTV/Controllers/Api/ScriptedScheduleController.cs index 342dc04a6..5ed9e954d 100644 --- a/ErsatzTV/Controllers/Api/ScriptedScheduleController.cs +++ b/ErsatzTV/Controllers/Api/ScriptedScheduleController.cs @@ -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; +/// +/// The callback surface a scripted playout's program drives during a build. Under the global API +/// authorization filter like every other controller — not [SkipApiAuthorization] — and additionally +/// accepts the build-scoped token ScriptedPlayoutBuilder hands the program, so the program needs +/// neither the machine key nor a session (ersatztv#913). +/// [ApiController] +[AcceptsScriptedBuildToken] [EndpointGroupName("scripted-schedule")] [Route("/api/v1/scripted/playout/build/{buildId:guid}")] public class ScriptedScheduleController(IScriptedPlayoutBuilderService scriptedPlayoutBuilderService) : ControllerBase diff --git a/ErsatzTV/Filters/AcceptsScriptedBuildTokenAttribute.cs b/ErsatzTV/Filters/AcceptsScriptedBuildTokenAttribute.cs new file mode 100644 index 000000000..f7ef7785f --- /dev/null +++ b/ErsatzTV/Filters/AcceptsScriptedBuildTokenAttribute.cs @@ -0,0 +1,19 @@ +using ErsatzTV.Core.Scheduling.ScriptedScheduling; +using Microsoft.AspNetCore.Mvc.Filters; + +namespace ErsatzTV.Filters; + +/// +/// Lets accept a scripted build's own token +/// () as a credential, in addition to the machine key and a +/// session (ersatztv#913, decision api.scripted-build-token). The token is checked against the build +/// named by the 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 ScriptedScheduleController alone. +/// +[AttributeUsage(AttributeTargets.Class)] +public sealed class AcceptsScriptedBuildTokenAttribute : Attribute, IFilterMetadata +{ + /// The route parameter that names the build the token must belong to. + public const string RouteValueName = "buildId"; +} diff --git a/ErsatzTV/Filters/ApiAuthorizationFilter.cs b/ErsatzTV/Filters/ApiAuthorizationFilter.cs index 4c2800be0..de6ba73c3 100644 --- a/ErsatzTV/Filters/ApiAuthorizationFilter.cs +++ b/ErsatzTV/Filters/ApiAuthorizationFilter.cs @@ -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; /// a matching machine X-Api-Key header (MCP / external clients), or /// an authenticated session principal (browser cookie, from OIDC or local login). /// +/// An endpoint carrying also accepts a running scripted +/// build's own token (), for that build only (issue #913). /// Which endpoints require authentication is decided by — 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 Api:RequireKeyForReads is set @@ -28,7 +32,9 @@ namespace ErsatzTV.Filters; /// custom header — forces a CORS preflight that a cross-site attacker page cannot satisfy. /// /// -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().Any(); } + private bool HasValidScriptedBuildToken(AuthorizationFilterContext context) + { + if (!context.ActionDescriptor.EndpointMetadata.OfType().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) diff --git a/docs/api-conventions.md b/docs/api-conventions.md index e49d98959..2d715b53f 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -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` diff --git a/docs/decisions/README.md b/docs/decisions/README.md index b4ebbf05e..cbad4d656 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -30,6 +30,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `api.response-dtos` | New REST response DTOs live in `ErsatzTV.Core/Api//*ResponseModel.cs` with a file-scoped `#nullable enable` pragma; controllers never expose Application VM types directly. | 2026-07 | [link](records/api/response-dtos.md) | | `api.schedule-item-flat-dto` | Schedule-item GET/POST/PUT use a flat, non-polymorphic `ScheduleItemResponseModel` (every subtype field promoted to a nullable top-level member) instead of the polymorphic Application VM hierarchy, with mutation field names matching `ScheduleItemRequest` 1:1 for a lossless round-trip. | 2026-07-10 | [link](records/api/schedule-item-flat-dto.md) | | `api.scheduling-hardening` | Create/Replace handlers guard against null/whitespace `name` (`IsNullOrWhiteSpace`, not just `Length`) to prevent NRE-500s, template-item overlap validation compares by index (not record value-equality) to catch exact-duplicate items, and unreachable 404 `ProducesResponseType` attributes on create-only actions are trimmed. | 2026-07-13 | [link](records/api/scheduling-hardening.md) | +| `api.scripted-build-token` | `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`. | 2026-10-29 | [link](records/api/scripted-build-token.md) | | `api.search-allitems-paging` | `GET /api/v1/search/all-items` is paginated (capped page size, `Totals` field) to bound DoS exposure; the SPA add-all flow pages to completeness instead of relying on an unbounded response. | 2026-07-18 | [link](records/api/search-allitems-paging.md) | | `api.search-field-values-sources` | `GET /api/v1/search/fields/{name}/values?q=&limit=` returns distinct WHOLE values from the database for a narrow allow-list of catalog fields (never the Lucene term dictionary — analyzed `TextField`s store lowercased word tokens, e.g. "Science Fiction" → `science`/`fiction`, useless as a suggestion), 404 for an unknown field, a non-`text` field, or a `text` field with no distinct-value source (`title`, `show_title` only); `limit` clamped to `[1, 50]` (default 50). The FINAL filter, dedup and ordering applied to the response are ORDINAL (`OrdinalIgnoreCase` / `StringComparer.Ordinal`), never current-culture, because `UseRequestLocalization` makes the culture caller-controlled — scoped to the in-memory stages on purpose: a field sourced by a plain EF query is filtered and truncated by the DATABASE collation first (SQLite's `LOWER()` is ASCII-only), which ordinal semantics downstream cannot undo (ersatztv#668). A field whose values live in an EF **primitive collection** (one JSON array per row in a single column: `SongMetadata.Artists`, `SongMetadata.AlbumArtists`) is served, not 404'd, as bounded best-effort, and its rows are read by a keyset page carrying **NO RESIDUAL predicate** — `SELECT Id, AS Payload FROM SongMetadata WHERE Id > @AfterId ORDER BY Id LIMIT @Batch`, no `LIKE`, no `LOWER`, not even `IS NOT NULL`. The cursor is itself a predicate, but a SEEKABLE one on the ordering key: it positions the scan and never discards a row. A RESIDUAL predicate discards rows the engine already produced, and `LIMIT` truncates only the survivors — so with one present it bounds the OUTPUT rather than the row count. All selectivity is in memory. The guarantee is scoped: **at most 20,000 LOGICAL rows returned/materialized and at most 10 round trips (11 for `artist`)** — NOT bounded physical work and NOT bounded bytes, because MySQL traverses deleted-but-unpurged index records and the `TEXT`/`longtext` payload width is unrestricted. The walk pages 2,000 rows at a time, stopping on the first of enough distinct matches, a short page, or the ceiling. | 2026-07-26 | [link](records/api/search-field-values-sources.md) | | `api.search-field-values-unicode-fold` | The EF-sourced facet fields (`genre`, `show_genre`, `studio`, `director`, `writer`, `actor`, `tag`, `network`, `collection`, `video_codec`, `album`, and `artist`'s entity half) reach stored values whose prefix carries an uppercase non-ASCII character, on BOTH providers, with no row budget and no accepted loss. The defect was SQLite-only and ONE-SIDED: SQLite's `LOWER()` folds ASCII only (`lower('Édith')` is `'Édith'` unchanged), so the predicate UNDER-matched, which no later stage can repair. MySQL was already correct — its `LOWER()` is Unicode-aware, so `LOWER('Édith')` really is `'édith'` and the existing predicate reaches the row unaided. The fix is a SECOND, ADDITIVE query taken only when `isSqlite && q contains a non-ASCII character`: raw Dapper SQL `SELECT DISTINCT AS Value FROM WHERE [ AND] etv_upper() LIKE @Pattern ESCAPE '\' ORDER BY LIMIT @Limit`, where `etv_upper` is a `SqliteConnection.CreateFunction` scalar implementing `ToUpperInvariant`. Every other case — all-ASCII `q`, and MySQL for all `q` — runs today's EF query BYTE-IDENTICALLY. Keeping selectivity in SQL here is NOT the refuted family from `api.search-field-values-sources`: those four attempts bounded a walk around a predicate that could not be made correct over JSON escape text, whereas this is a correct fold on a plain column in an ordinary `LIMIT`ed query. It narrows that record's "Known limitation inherited, not introduced" clause; everything else it settles still holds. | 2026-07-27 | [link](records/api/search-field-values-unicode-fold.md) | @@ -229,7 +230,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `testing.live-e2e-prepush-timing` | Run live-E2E via `scripts/e2e-local.sh` before pushing a write-path or UI change, and exercise download endpoints with curl, never a browser tab. | 2026-07-21 | [link](records/testing/live-e2e-prepush-timing.md) | | `testing.mutation-claims-are-executed` | A `MUTATION` row in `docs/guard-inventory.md` is not a statement that someone once witnessed a red. It carries a DECLARED clause mutation in `scripts/tests/mutation_manifest.py`, and `scripts/tests/test_mutation_harness.py` applies that mutation to an isolated copy of the repository on every run and requires the row's OWN named test to go red. The manifest and the MUTATION rows are compared for SET EQUALITY in both directions, so a row cannot claim the grade without a mutation and a mutation cannot outlive the grade it justifies. EXIT STATUS IS NOT THE VERDICT: each entry also declares the DIAGNOSTIC its red must carry, matched against pytest's exception output alone, because pytest reports a crashing test exactly as it reports a detecting one and a red for an unrelated reason is evidence about nothing. WHERE THE GUARD IS ITSELF A TEST, `target` may differ from `guard` and the exact-once check applies to the declared TARGET. Two shapes are admissible and the choice is not free. Where the guard's assertion IS the check — a completeness comparison against a Markdown inventory — the mutation goes into the guarded ARTIFACT, per `testing.guard-ships-with-mutation-proof`'s checker-guard exception, because mutating such a checker's own POPULATION demonstrates a false POSITIVE while proving nothing about the detection the row claims. Where the guard is a test module wrapping a separately mutable DETECTOR or helper, the clause may be in that detector, since disarming it is a real clause disarm and the module's own assertion is what notices. THE MUTATION IS DECLARED, NEVER INFERRED: a harness that guessed which clause of a 90-line hook is the guard would manufacture the confident-but-empty coverage this exists to prevent, which is why `testing.guard-ships-with-mutation-proof` rejected a generic runner. Where a proof test already names its clause in source, the manifest reuses THAT string, so a retarget in either place is caught by the other. COARSENESS IS RECORDED, NOT HIDDEN: each entry is graded `CLAUSE` or `DETECTOR`, and a `DETECTOR` entry — one whose detector accumulates faults from independent arms, so disarming any single arm leaves its proof test green — must CARRY the finer mutation that survived, which is re-run every time and required to keep surviving. Guards that are not graded `MUTATION` each carry a STATED reason in that same manifest, keyed on the guard and compared for SET EQUALITY against the inventory's `GUARD` rows in both directions — so a new guard cannot arrive without someone writing what a proof would need, and a reason cannot outlive the row it is about. Keying the reason on the row's GRADE instead is tautological (a new guard inherits one and nobody looks at it) and a pinned COUNT moves only on net change; both were tried and are rejected. The sandbox is a real git repository built from `git ls-files` with working-tree content, never a filesystem walk. THE POPULATION IS EVERY DECLARED MUTATION-OUTCOME CLAIM, NOT ONLY THE INVENTORY'S ROWS (ersatztv#881): a sentence asserting that a specific mutation reddens, or does not redden, a named test is the same kind of claim wherever it is written — a code comment, a test docstring, a decision record — and is either a `CLAIMS` entry in `scripts/tests/mutation_manifest.py` that executes every run, or it is not written. A claim entry is keyed on the PROSE rather than on a guard: it carries the tracked `site` and the verbatim `quote`, both checked every run, so a reworded sentence reports as a RETARGET instead of drifting away from the entry that justifies it. THE QUOTE SPANS THE MUTATION AND THE OUTCOME IT ASSERTS: the outcome half is the checkable part, so a quote stopping at the mutation binds the uncontroversial words and leaves the assertion, and the test it names, outside the binding entirely. It declares `RED` or `GREEN`, and GREEN is the half no `MUTATION` row can express — a claim that a mutation is NOT noticed is read by THREE separate clauses, because each alone certifies a run that proves nothing: the run exited 0, something actually PASSED, and the named proof was shown to REACH the mutated clause. THE TWO DIRECTIONS ARE NOT SYMMETRIC, WHICH IS WHY THE THIRD CLAUSE EXISTS ON ONLY ONE OF THEM. A RED claim checks its own relevance for free — a proof that never executes the mutated file stays green and is refused in the same words as a clause that has stopped being load-bearing — while a GREEN claim gets precisely the green it asserts from a proof that reads a different file entirely, so exit status and a passing test TOGETHER still certify a run that never executed the clause. A GREEN entry therefore also declares a SECOND mutation of the SAME clause, required to REDDEN the same proof and carrying its own declared diagnostic, executed through the same red-reading path the inventory rows use; only once that passes is the green evidence about this clause at all. It is worth what the `DETECTOR` survivor was worth: re-run, and required to KEEP surviving. THE REFERENT IS DECLARED, NEVER INFERRED, exactly as the clause is: a checker hunting for sentences that merely SOUND like mutation claims would be a string predicate over prose, which this repo refuses to make load-bearing, so what is enforced is a claim that named its own referent, and the direction that cannot be mechanised — prose that declares nothing — is stated rather than papered over. TWO NEIGHBOURING PROPOSALS ARE SETTLED HERE. A QUOTATION of another file is a claim about that file: adopted, and the `quote` field IS the check, wherever the referent is declared; a general quotation-checker is rejected for the same reason inference is, since a quotation whose referent is unnamed has nothing to resolve. ANCHORING PROSE TO A STATE THE COMMIT ITSELF MOVES ('the last edit to that path', 'the only X that'): rejected as a detector and kept as a phrasing rule. The rejection is measured, and the scan is PINNED IN THE BODY rather than summarised as a figure here, for the same reason the population scan is — a count quoted without the predicate that produced it is the defect this record is about. What it shows is that the shape is indistinguishable BY PATTERN from ordinary rationale ('X is the only thing that can Y'), which this rule carves out, so a detector would fire mostly on prose it must not flag, and an advisory check that does that stops being read. Anchor to a date or a sha under `docs.no-session-narrative`'s dated-measurement clause; inside this rule's own population the anchor is the manifest entry, which is re-executed and therefore cannot go stale. | 2026-08-22 | [link](records/testing/mutation-claims-are-executed.md) | | `testing.playwright-mcp-download-and-recovery` | In Playwright-MCP E2E, fetch file-download endpoints with curl — never a browser tab or `window.open` — and if browser tools stall repeatedly, `pkill -f ms-playwright-mcp` and drive a fresh session. | 2026-07-21 | [link](records/testing/playwright-mcp-download-and-recovery.md) | -| `testing.scripted-engine-in-process-net` | 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. | 2026-09-05 | [link](records/testing/scripted-engine-in-process-net.md) | +| `testing.scripted-engine-in-process-net` | 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. | 2026-09-05 | [link](records/testing/scripted-engine-in-process-net.md) | | `testing.suite-isolated-from-production-hook-fire-log` | THE SUITE MUST NOT WRITE TO `$HOME/.cache/ersatztv/hook-fire/`, AND THAT PROPERTY IS ESTABLISHED STRUCTURALLY RATHER THAN OBSERVED. Two layers in `scripts/tests/conftest.py`, which close different routes and must both stay. FIRST, `pytest_configure` sets `ETV_HOOK_FIRE_LOG_DIR` BEFORE COLLECTION. An autouse fixture alone runs at test setup — after every module has been imported — so it cannot reach a module that snapshots `{**os.environ}` at import time (the ersatztv#785 defect, 58 leaked records per run) and cannot reach a MODULE- or SESSION-SCOPED FIXTURE at all; measured on the pre-change tree by instrumenting `subprocess.Popen` for a full run and asking which launches resolve to `$HOME/.cache/ersatztv/hook-fire`, 83 did — 37 during collection, 44 inside module- or session-scoped fixtures, and 2 from an environment built from scratch. INSTRUMENT `Popen` ALONE: `subprocess.run`, `call` and `check_output` all reach it, so a census wrapping `run` as well counts every launch twice and every figure doubles. STATE THE PREDICATE WITH THE COUNT: this one also UNDERCOUNTS the from-scratch route, because such an environment usually carries no `HOME` either and lands in the sink's OTHER default, which this predicate cannot see. The launches that carry no `ETV_HOOK_FIRE_LOG_DIR` and are built from scratch number 83 as well, and 81 of those carry no `HOME` — two different sets of the same size, which is precisely the shape a later reader reconciles wrongly. The per-test fixture stays, because a test that inspects its own log dir must not see another test's records. SECOND, a `Popen` wrapper fails EVERY launch that does not CARRY the isolated directory — the route left open when an environment is built FROM SCRATCH rather than derived from `os.environ`, where no snapshot is involved and import ordering cannot help. THE RULE IS "CARRIES AN ISOLATED DIR", NOT "IS NOT THE PRODUCTION ONE", and that is the difference between a guard and a guard-shaped hole: `etv_hook_fire_log_dir` has TWO default branches — `$HOME/.cache/...` and, when `HOME` is unset or empty, `/tmp/.cache/...` — so a check comparing against one resolved production path models one branch and waves the other through, and `/tmp/.cache/ersatztv/hook-fire` is just as shared, just as persistent and just as readable with `hook-fire-log.sh report --dir`. Measured: of the 83 from-scratch launches, 81 carry no `HOME` either. Requiring the variable covers both branches and any a future sink adds, and is independent of what `HOME` happens to be on the machine running the suite — which is also what keeps it from firing on legitimate launches where `HOME` is unset or is itself `/tmp`. The corollary is that every from-scratch environment in the suite must carry the variable, one line each. NO COUNT OF THOSE SITES IS KEPT HERE: it is a hand-written population literal that nothing derives, and the commit introducing this rule already falsified its own, by adding further construction sites in the tests that prove the rule. The guard itself is the enumerator; a missing site is a red, not a stale sentence. Its population is every launch that actually happens, derived at runtime, so a new suite is covered by existing rather than by remembering. NEVER OBSERVE THE PRODUCTION DIRECTORY TO PROVE THIS. The withdrawn guard snapshotted `st_mtime_ns` across it and required no change, which makes the oracle state every concurrent session on the machine writes: an unrelated session firing a hook inside the window failed the suite with `a test run modified the production hook-fire log`, an accusation about the suite when the writer was another process. Seen on three separate branches, green on every immediate re-run. The misattribution is the expensive part — it aims the next reader at the suite, and a guard that cries wolf gets waved through. THE RESOLVER IS A REIMPLEMENTATION AND FAILS OPEN, so it is differential-tested against `etv_hook_fire_log_dir` every run: a resolver that disagrees clears a launch the sink then points at the real log. Shell's `:-` treats an EMPTY value as absent, which `dict.get(k, default)` does not — that case is in the matrix because it is where a Python transliteration goes wrong. AND BIND A VIOLATION BEFORE ASSERTING ON IT: pytest rewrites an assert expression and prints the repr of every sub-expression, so `assert isolation_violation(os.environ) is None` dumps the environment — API keys included — into the failure output and from there into a CI log. | 2026-08-28 | [link](records/testing/suite-isolated-from-production-hook-fire-log.md) | | `testing.troubleshoot-path-cannot-test-branding` | Verify logo/watermark/bug changes through a real channel playout — a green troubleshoot run proves nothing about branding. | 2026-07-21 | [link](records/testing/troubleshoot-path-cannot-test-branding.md) | | `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet several of them carry `MUTATION` rows — among them `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives it with no gitignored `.mcp.json` and no language server, which is how it is proven in `scripts/tests/test_mcp_smoke.py`. That coverage is a SUBSET and no count of it is kept here: the file's own docstring enumerates the cases it drives and names what it does not, so there is a list rather than a number to keep in step with it. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record's own threshold: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | diff --git a/docs/decisions/records/api/scripted-build-token.md b/docs/decisions/records/api/scripted-build-token.md new file mode 100644 index 000000000..7222153a4 --- /dev/null +++ b/docs/decisions/records/api/scripted-build-token.md @@ -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. diff --git a/docs/decisions/records/testing/scripted-engine-in-process-net.md b/docs/decisions/records/testing/scripted-engine-in-process-net.md index ee9b61bc5..04df19623 100644 --- a/docs/decisions/records/testing/scripted-engine-in-process-net.md +++ b/docs/decisions/records/testing/scripted-engine-in-process-net.md @@ -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 diff --git a/docs/testing.md b/docs/testing.md index 09ebf0827..17af43dcd 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -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 diff --git a/scripts/scripted-schedules/entrypoint.py b/scripts/scripted-schedules/entrypoint.py index 3728e5f5a..ccadfe347 100755 --- a/scripts/scripted-schedules/entrypoint.py +++ b/scripts/scripted-schedules/entrypoint.py @@ -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