timothy/ersatztv
1
0
Fork 0
Code Issues 58 Pull Requests 8 Actions Packages Projects Releases 9 Wiki Activity

Scripted playout build almost certainly 401s: ScriptedScheduleController is under the global API auth filter and the script is handed no credential #913

New Issue
Open
opened 2026-09-05 07:15:40 +02:00 by timothy · 5 comments
timothy commented 2026-09-05 07:15:40 +02:00
Owner
Copy Link
Copy Source

Derived from source while closing #563 (measured 2026-09-05 against origin/main @ 4a56d54). Not yet reproduced against a live instance — that is step 1 below.

The chain

  1. Startup.ConfigureServices registers the filter globally: options.Filters.AddService<ApiAuthorizationFilter>().
  2. ApiAuthorizationFilter.EndpointRequiresKey (ErsatzTV/Filters/ApiAuthorizationFilter.cs) returns true for every mutating verb unconditionally — return IsMutating(httpMethod) || requireKeyForReads || ... — and for reads too, since Api:RequireKeyForReads defaults to true (ApiKeyProvider).
  3. ErsatzTV/Controllers/Api/ScriptedScheduleController.cs carries no [SkipApiAuthorization] (grep -n SkipApiAuthorization on that file exits 1). Its route is /api/v1/scripted/playout/build/{buildId:guid}, i.e. under /api, so the filter's path guard does not exempt it.
  4. ScriptedPlayoutBuilder (ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilder.cs) hands the user's program only three arguments: http://localhost:{Settings.UiPort}, the build id, and the mode. No X-Api-Key, no session.
  5. The fork's own driver, scripts/scripted-schedules/entrypoint.py, builds its client as etv_client.Configuration(host=known_args.host) — no credential either.

With neither credential presented, OnAuthorization falls through to the UnauthorizedObjectResult, so the script's first POST .../add_collection should get a 401 and the scripted build should produce nothing.

Why it is not covered by tests

#563 characterizes scripted playout in-process (decision testing.scripted-engine-in-process-net): ScriptedScheduleControllerTests invokes controller actions directly, so no filter, no middleware, no Kestrel. The record names ApiAuthorizationFilter as the concrete blind spot that scope-out leaves; this issue is that blind spot.

Scope

  1. Reproduce (or refute) against a live instance: run a scripted playout on test and read the API log for a 401 on /api/v1/scripted/playout/build/.
  2. If confirmed, decide the fix deliberately rather than by reflex — the endpoint is localhost-only and port-checked, but [SkipApiAuthorization] on a whole mutating controller is a widening, so weigh it against minting a build-scoped credential that ScriptedPlayoutBuilder passes to the script as a fourth argument.
  3. Whichever way it goes, pin it with a test that exercises the filter decision for this controller's metadata (EndpointRequiresKey is a pure static — it is testable without a host).

Done-when

  • Reproduced or refuted against a live instance, with the evidence recorded in a comment
  • If confirmed: fix landed, with the widening-vs-credential choice argued in the PR body
  • A test pins the filter decision for ScriptedScheduleController (not just the action)
  • docs/decisions/records/testing/scripted-engine-in-process-net.md updated so the 401 clause reflects the outcome
  • Adversarial review passed

Refs #563.

Derived from source while closing #563 (measured 2026-09-05 against `origin/main` @ `4a56d54`). Not yet reproduced against a live instance — that is step 1 below. ## The chain 1. `Startup.ConfigureServices` registers the filter globally: `options.Filters.AddService<ApiAuthorizationFilter>()`. 2. `ApiAuthorizationFilter.EndpointRequiresKey` (`ErsatzTV/Filters/ApiAuthorizationFilter.cs`) returns `true` for **every** mutating verb unconditionally — `return IsMutating(httpMethod) || requireKeyForReads || ...` — and for reads too, since `Api:RequireKeyForReads` defaults to `true` (`ApiKeyProvider`). 3. `ErsatzTV/Controllers/Api/ScriptedScheduleController.cs` carries no `[SkipApiAuthorization]` (`grep -n SkipApiAuthorization` on that file exits 1). Its route is `/api/v1/scripted/playout/build/{buildId:guid}`, i.e. under `/api`, so the filter's path guard does not exempt it. 4. `ScriptedPlayoutBuilder` (`ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilder.cs`) hands the user's program only three arguments: `http://localhost:{Settings.UiPort}`, the build id, and the mode. No `X-Api-Key`, no session. 5. The fork's own driver, `scripts/scripted-schedules/entrypoint.py`, builds its client as `etv_client.Configuration(host=known_args.host)` — no credential either. With neither credential presented, `OnAuthorization` falls through to the `UnauthorizedObjectResult`, so the script's first `POST .../add_collection` should get a 401 and the scripted build should produce nothing. ## Why it is not covered by tests #563 characterizes scripted playout **in-process** (decision `testing.scripted-engine-in-process-net`): `ScriptedScheduleControllerTests` invokes controller actions directly, so no filter, no middleware, no Kestrel. The record names `ApiAuthorizationFilter` as the concrete blind spot that scope-out leaves; this issue is that blind spot. ## Scope 1. Reproduce (or refute) against a live instance: run a scripted playout on test and read the API log for a 401 on `/api/v1/scripted/playout/build/`. 2. If confirmed, decide the fix deliberately rather than by reflex — the endpoint is localhost-only and port-checked, but `[SkipApiAuthorization]` on a whole mutating controller is a widening, so weigh it against minting a build-scoped credential that `ScriptedPlayoutBuilder` passes to the script as a fourth argument. 3. Whichever way it goes, pin it with a test that exercises the *filter* decision for this controller's metadata (`EndpointRequiresKey` is a pure static — it is testable without a host). ## Done-when - [ ] Reproduced or refuted against a live instance, with the evidence recorded in a comment - [ ] If confirmed: fix landed, with the widening-vs-credential choice argued in the PR body - [ ] A test pins the filter decision for `ScriptedScheduleController` (not just the action) - [ ] `docs/decisions/records/testing/scripted-engine-in-process-net.md` updated so the 401 clause reflects the outcome - [ ] Adversarial review passed Refs #563.
timothy added the priority: medium label 2026-09-05 18:09:59 +02:00
timothy added the in-progress label 2026-10-29 22:58:04 +01:00
timothy commented 2026-10-29 22:58:05 +01:00
Author
Owner
Copy Link
Copy Source

Claiming #913 (orchestrated slot, session f9ce6bc6). Branch 913-scripted-playout-401, worktree /Users/timothy/orca/workspaces/ersatztv/wt-913. Four-way check clean: no open PR references #913, no remote branch matching *913*, no prior comments; origin/main @ e919b925f.

Claiming #913 (orchestrated slot, session f9ce6bc6). Branch `913-scripted-playout-401`, worktree `/Users/timothy/orca/workspaces/ersatztv/wt-913`. Four-way check clean: no open PR references #913, no remote branch matching `*913*`, no prior comments; origin/main @ e919b925f.
timothy commented 2026-10-29 23:37:17 +01:00
Author
Owner
Copy Link
Copy Source

Reproduced (2026-10-29) — confirmed against a live instance

Instance: local live build of origin/main @ e919b925f, launched with ETV_UI_PORT=8510 scripts/e2e-local.sh <fresh config> (real Kestrel, real Startup pipeline, real ApiAuthorizationFilter, default Api:RequireKeyForReads=true). I used a local instance rather than ersatztv-test so no channel or playout had to be created on a shared instance.

Method: created a channel and a Scripted playout (POST /api/v1/playouts, scheduleKind: Scripted) whose scheduleFile was a small probe program. The probe makes the same calls entrypoint.py makes, at the HTTP level, with no credential (the same credential entrypoint.py sends: none), records each status code, and exits 0, as entrypoint.py does after catching ApiException.

Evidence:

  • Server log: Building scripted playout …/probe.py with arguments ["http://localhost:8510", "49a4b32b-2a88-4bfb-a0ea-dc22e4a99b66", "reset"], so the program got exactly host, build id and mode.
  • The program's calls during the build:
    • GET /api/v1/scripted/playout/build/49a4b32b-…/context → 401 {"title":"Unauthorized","status":401,"detail":"Authentication is required. Provide a valid 'X-Api-Key' header (machine clients) or sign in to obtain a session (browser)."}
    • POST …/add_collection → 401 (same body)
  • Control, after the build: the same GET …/context with X-Api-Key returned 404 Active build engine not found (auth passed; the session had ended), and 401 without the key. So the 401 comes from the auth filter and is not a missing route.
  • GET /api/v1/playouts/1 → "buildStatus":{"lastBuild":"…","success":true} with no items. The build reports success and is empty. That is the silent part: a program that exits 0 after the 401 looks like a clean build.

The chain in the issue body holds. Fix: branch 913-scripted-playout-401. The widening-vs-credential argument goes in the PR body.

## Reproduced (2026-10-29) — confirmed against a live instance **Instance:** local live build of `origin/main` @ `e919b925f`, launched with `ETV_UI_PORT=8510 scripts/e2e-local.sh <fresh config>` (real Kestrel, real `Startup` pipeline, real `ApiAuthorizationFilter`, default `Api:RequireKeyForReads=true`). I used a local instance rather than `ersatztv-test` so no channel or playout had to be created on a shared instance. **Method:** created a channel and a `Scripted` playout (`POST /api/v1/playouts`, `scheduleKind: Scripted`) whose `scheduleFile` was a small probe program. The probe makes the same calls `entrypoint.py` makes, at the HTTP level, with no credential (the same credential `entrypoint.py` sends: none), records each status code, and exits 0, as `entrypoint.py` does after catching `ApiException`. **Evidence:** - Server log: `Building scripted playout …/probe.py with arguments ["http://localhost:8510", "49a4b32b-2a88-4bfb-a0ea-dc22e4a99b66", "reset"]`, so the program got exactly host, build id and mode. - The program's calls during the build: - `GET /api/v1/scripted/playout/build/49a4b32b-…/context` → **401** `{"title":"Unauthorized","status":401,"detail":"Authentication is required. Provide a valid 'X-Api-Key' header (machine clients) or sign in to obtain a session (browser)."}` - `POST …/add_collection` → **401** (same body) - Control, after the build: the same `GET …/context` with `X-Api-Key` returned **404** `Active build engine not found` (auth passed; the session had ended), and **401** without the key. So the 401 comes from the auth filter and is not a missing route. - `GET /api/v1/playouts/1` → `"buildStatus":{"lastBuild":"…","success":true}` with no items. The build reports **success** and is empty. That is the silent part: a program that exits 0 after the 401 looks like a clean build. The chain in the issue body holds. Fix: branch `913-scripted-playout-401`. The widening-vs-credential argument goes in the PR body.
timothy commented 2026-10-29 23:38:21 +01:00
Author
Owner
Copy Link
Copy Source

Fix verified live (2026-10-29), branch 913-scripted-playout-401 @ 0721d5de2 (not pushed yet; under review)

Choice: a build-scoped credential, not [SkipApiAuthorization]. The argument is in the decision record api.scripted-build-token and will be in the PR body. The token is minted per build (256-bit), passed in the ETV_SCRIPTED_BUILD_TOKEN environment variable (not as a 4th argument: that would shift the user's own arguments, and argv is logged), and accepted as X-Scripted-Build-Token only on ScriptedScheduleController (marked [AcceptsScriptedBuildToken]), only for the route's buildId, and only until the build ends.

Ran on a fresh local instance (ETV_UI_PORT=8510 scripts/e2e-local.sh, fixed build):

  • The fork's real driver, end to end. I ran scripts/scripted-schedules/entrypoint.py (byte-identical copy, cmp clean) with the generated etv_client copied read-only from the ersatztv-test image, against an OtherVideos library of 3 scanned files and a collection holding them. A tiny user script did add_collection + add_count(3). Result: every call succeeded, GET /api/v1/playouts/1/items → totalCount: 3 (probe1/2/3, 20s each), and buildStatus.success: true.

  • Controls, run as the scripted program during its own build:

    Call Status
    own build, no header 401
    own build, wrong token 401
    own build, own token, GET context 200
    own build, own token, POST add_collection 200
    random other build id, own token 401
    GET /api/v1/channels with own token 401
    POST /api/v1/collections with own token 401
    own token after the build ended 401 (the machine key on the same URL: 404)

  • The token appears 0 times in the server log. The build id still appears in the INF argv line, as before; it is no longer the secret.

Tests: ErsatzTV.Tests/Filters/ScriptedScheduleAuthorizationTests.cs checks the filter decision over the endpoint metadata MVC builds from the production Startup, for all 28 controller actions. ErsatzTV.Core.Tests/Scheduling/ScriptedScheduling/ScriptedBuildTokenTests.cs covers token minting and validation and the launch environment. The clause-mutation results are in the commit message.

Follow-up filed: #937 (entrypoint.py exits 0 after an API error, so a failed build reports success).

## Fix verified live (2026-10-29), branch `913-scripted-playout-401` @ `0721d5de2` (not pushed yet; under review) **Choice:** a build-scoped credential, not `[SkipApiAuthorization]`. The argument is in the decision record `api.scripted-build-token` and will be in the PR body. The token is minted per build (256-bit), passed in the `ETV_SCRIPTED_BUILD_TOKEN` environment variable (not as a 4th argument: that would shift the user's own arguments, and argv is logged), and accepted as `X-Scripted-Build-Token` only on `ScriptedScheduleController` (marked `[AcceptsScriptedBuildToken]`), only for the route's `buildId`, and only until the build ends. Ran on a fresh local instance (`ETV_UI_PORT=8510 scripts/e2e-local.sh`, fixed build): - **The fork's real driver, end to end.** I ran `scripts/scripted-schedules/entrypoint.py` (byte-identical copy, `cmp` clean) with the generated `etv_client` copied read-only from the `ersatztv-test` image, against an OtherVideos library of 3 scanned files and a collection holding them. A tiny user script did `add_collection` + `add_count(3)`. Result: every call succeeded, `GET /api/v1/playouts/1/items` → `totalCount: 3` (probe1/2/3, 20s each), and `buildStatus.success: true`. - **Controls, run as the scripted program during its own build:** | Call | Status | |---|---| | own build, no header | 401 | | own build, wrong token | 401 | | own build, own token, `GET context` | 200 | | own build, own token, `POST add_collection` | 200 | | random other build id, own token | 401 | | `GET /api/v1/channels` with own token | 401 | | `POST /api/v1/collections` with own token | 401 | | own token after the build ended | 401 (the machine key on the same URL: 404) | - The token appears 0 times in the server log. The build id still appears in the INF argv line, as before; it is no longer the secret. Tests: `ErsatzTV.Tests/Filters/ScriptedScheduleAuthorizationTests.cs` checks the filter decision over the endpoint metadata MVC builds from the production `Startup`, for all 28 controller actions. `ErsatzTV.Core.Tests/Scheduling/ScriptedScheduling/ScriptedBuildTokenTests.cs` covers token minting and validation and the launch environment. The clause-mutation results are in the commit message. Follow-up filed: #937 (`entrypoint.py` exits 0 after an API error, so a failed build reports success).
timothy commented 2026-10-29 23:41:37 +01:00
Author
Owner
Copy Link
Copy Source

Adversarial review — 0721d5de214833ecc19099c48ec3c6fc65981d9c

Reviewed git diff origin/main...HEAD in wt-913 for correctness and security. No code changes made.

Blocking code findings: none identified. The credential is CSPRNG-generated independently of the logged build id, bound to the route's active build and the marked controller, compared with FixedTimeEquals, and removed by the builder's finally cleanup. Process arguments stay unchanged; entrypoint.py sends the environment-delivered token as a header. The credential choice avoids exposing the whole machine API key or exempting a mutating controller.

Should-fix / done-condition gap: no PR could be found by main...913-scripted-playout-401 or by HEAD commit. The required widening-vs-credential argument exists in docs/decisions/records/api/scripted-build-token.md but is not yet verifiable in a PR body. Create/push the PR with that argument before declaring #913 complete. This is a handoff gap, not an identified code defect.

Evidence: live reproduction comment #34584 and fixed-build verification #34586 were read; the latter records the real entrypoint/generated client producing three items plus no-token/wrong-token/cross-build/unmarked-endpoint/ended-token controls. ScriptedScheduleAuthorizationTests pins the production MVC metadata/filter decision; the requested testing decision record is updated. The existing silent API-error exit behavior is tracked separately as #937.

Verification: git diff --check origin/main...HEAD passed; working tree clean. No fresh .NET restore/build/test or live instance execution: this review session is read-only with approval disabled, while repository instructions require .NET verification outside the sandbox. Prior live and mutation results are recorded evidence, not runs performed by this reviewer.

Outcome: correctness/security source review passed; send-back for the missing PR-body acceptance evidence. Leave issue open until that handoff requirement is met.

## Adversarial review — 0721d5de214833ecc19099c48ec3c6fc65981d9c Reviewed `git diff origin/main...HEAD` in wt-913 for correctness and security. No code changes made. **Blocking code findings:** none identified. The credential is CSPRNG-generated independently of the logged build id, bound to the route's active build and the marked controller, compared with FixedTimeEquals, and removed by the builder's finally cleanup. Process arguments stay unchanged; entrypoint.py sends the environment-delivered token as a header. The credential choice avoids exposing the whole machine API key or exempting a mutating controller. **Should-fix / done-condition gap:** no PR could be found by main...913-scripted-playout-401 or by HEAD commit. The required widening-vs-credential argument exists in docs/decisions/records/api/scripted-build-token.md but is not yet verifiable in a PR body. Create/push the PR with that argument before declaring #913 complete. This is a handoff gap, not an identified code defect. **Evidence:** live reproduction comment #34584 and fixed-build verification #34586 were read; the latter records the real entrypoint/generated client producing three items plus no-token/wrong-token/cross-build/unmarked-endpoint/ended-token controls. ScriptedScheduleAuthorizationTests pins the production MVC metadata/filter decision; the requested testing decision record is updated. The existing silent API-error exit behavior is tracked separately as #937. **Verification:** git diff --check origin/main...HEAD passed; working tree clean. No fresh .NET restore/build/test or live instance execution: this review session is read-only with approval disabled, while repository instructions require .NET verification outside the sandbox. Prior live and mutation results are recorded evidence, not runs performed by this reviewer. **Outcome:** correctness/security source review passed; send-back for the missing PR-body acceptance evidence. Leave issue open until that handoff requirement is met.
timothy commented 2026-10-30 00:50:51 +01:00
Author
Owner
Copy Link
Copy Source

Closing record

Outcome: PR http://192.168.1.95:3000/timothy/ersatztv/pulls/939 (head 324dbdbca). Confirmed and fixed; merge closes this issue.
Root cause: ScriptedScheduleController is under the global ApiAuthorizationFilter, and the program a scripted playout launches received no credential (host, build id and mode only; entrypoint.py built its client without one), so every callback was a 401. #563 tests the engine in-process with no filter, so nothing saw it.
Decisions/conventions changed: added api.scripted-build-token (build-scoped token via ETV_SCRIPTED_BUILD_TOKEN / X-Scripted-Build-Token; [SkipApiAuthorization] on the controller rejected); testing.scripted-engine-in-process-net 401 clause updated.
Reusable knowledge: a program launched by the server needs its credential in the environment, not argv (argv is logged and in the process table). Live-check this path with a plain executable as the scheduleFile.
Verification: live E2E 2026-10-30 on port 8510: no credential 401; build token 200; same token on another build 401; machine key on unknown build 404; bogus token 401; launch args in the log carry no token. Full .NET suites, web (vitest 1371), scripts/tests (1615) green.
Deferred: none.
Docs updated: docs/api-conventions.md, docs/testing.md, docs/decisions/records/api/scripted-build-token.md, docs/decisions/records/testing/scripted-engine-in-process-net.md, regenerated docs/decisions/README.md.

Done-when evidence

  • Reproduced or refuted against a live instance: the confirmed behaviour is that a callback with no credential is a 401 (live: curl of /api/v1/scripted/playout/build/<id>/context without a header returned 401 on a scripted playout built via scripts/e2e-local.sh, port 8510, 2026-10-30). This comment is the recorded evidence.
  • Fix landed with the widening-vs-credential choice argued: PR #939 body, "Fix: a build-scoped credential".
  • Test pins the filter decision for the controller: ErsatzTV.Tests/Filters/ScriptedScheduleAuthorizationTests.cs (dotnet test ErsatzTV.Tests 2154 passed).
  • Decision record updated: docs/decisions/records/testing/scripted-engine-in-process-net.md in PR #939.
  • Adversarial review passed: round 1, 0 blocking / 0 should-fix / 12 nit (review history in PR #939 body); the review-verdict/h10 status is for the orchestrator to post.
## Closing record **Outcome:** PR http://192.168.1.95:3000/timothy/ersatztv/pulls/939 (head 324dbdbca). Confirmed and fixed; merge closes this issue. **Root cause:** `ScriptedScheduleController` is under the global `ApiAuthorizationFilter`, and the program a scripted playout launches received no credential (host, build id and mode only; `entrypoint.py` built its client without one), so every callback was a 401. #563 tests the engine in-process with no filter, so nothing saw it. **Decisions/conventions changed:** added `api.scripted-build-token` (build-scoped token via `ETV_SCRIPTED_BUILD_TOKEN` / `X-Scripted-Build-Token`; `[SkipApiAuthorization]` on the controller rejected); `testing.scripted-engine-in-process-net` 401 clause updated. **Reusable knowledge:** a program launched by the server needs its credential in the environment, not argv (argv is logged and in the process table). Live-check this path with a plain executable as the `scheduleFile`. **Verification:** live E2E 2026-10-30 on port 8510: no credential 401; build token 200; same token on another build 401; machine key on unknown build 404; bogus token 401; launch args in the log carry no token. Full .NET suites, web (vitest 1371), scripts/tests (1615) green. **Deferred:** none. **Docs updated:** docs/api-conventions.md, docs/testing.md, docs/decisions/records/api/scripted-build-token.md, docs/decisions/records/testing/scripted-engine-in-process-net.md, regenerated docs/decisions/README.md. ### Done-when evidence - Reproduced or refuted against a live instance: the confirmed behaviour is that a callback with no credential is a 401 (live: `curl` of `/api/v1/scripted/playout/build/<id>/context` without a header returned 401 on a scripted playout built via `scripts/e2e-local.sh`, port 8510, 2026-10-30). This comment is the recorded evidence. - Fix landed with the widening-vs-credential choice argued: PR #939 body, "Fix: a build-scoped credential". - Test pins the filter decision for the controller: `ErsatzTV.Tests/Filters/ScriptedScheduleAuthorizationTests.cs` (`dotnet test ErsatzTV.Tests` 2154 passed). - Decision record updated: `docs/decisions/records/testing/scripted-engine-in-process-net.md` in PR #939. - Adversarial review passed: round 1, 0 blocking / 0 should-fix / 12 nit (review history in PR #939 body); the `review-verdict/h10` status is for the orchestrator to post.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ad-hoc
One-off / ad-hoc work not tracked by a dedicated issue
 api
REST API / HTTP endpoints
 bug
Something isn't working
 ci-cd
Build, test, deploy pipeline
 content
Channel content / schedules / playlists
 dependencies
Dependency updates (Renovate)
 enhancement
New feature or improvement
 frontend
ChicoryTV React SPA frontend
 in-progress
Claimed by an active session — do not pick up
 jellyfin
Jellyfin tuner / IPTV integration
 parked
Excluded from automatic queue pickup; work only when explicitly selected
 priority: high
priority: low
priority: medium
review
Adversarial review finding
 security
Security / vulnerability fix
No labels in-progress priority: medium
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#913