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

feat(mcp): add read-only API server foundation #76

Closed
timothy wants to merge 5 commits from issue58-mcp into main
pull from: issue58-mcp
merge into: :main
Conversation 3 Commits 5 Files Changed 18
+1493
timothy commented 2026-07-02 00:41:24 +02:00
Owner
Copy Link
Copy Source

Summary

  • Add ErsatzTV.Mcp, a small stdio JSON-RPC MCP server over the current stable REST API.
  • Expose read-only v0 tools for channels, collections, smart collections, schedules/items, playout lookup, FFmpeg profiles/resolution, sessions, and version.
  • Add focused NUnit coverage for tool catalog scope, API client path/auth/error behavior, and MCP initialize/tools/list/tools/call behavior.
  • Document configuration, tool list, X-Api-Key behavior, and deferred #63-#68 redesign workflows in docs/mcp.md.

Verification

  • TZ=UTC dotnet restore ErsatzTV.sln -v minimal
  • TZ=UTC dotnet build ErsatzTV.sln --no-restore -v minimal
  • TZ=UTC dotnet test ErsatzTV.Mcp.Tests/ErsatzTV.Mcp.Tests.csproj -v minimal
  • TZ=UTC dotnet test ErsatzTV.sln --no-build -v minimal

Deferred

  • v0.1 write tools are not included in this PR.
  • Workflow-level tools are deferred until the backend gaps under #63-#68 exist.

Refs #58

## Summary - Add `ErsatzTV.Mcp`, a small stdio JSON-RPC MCP server over the current stable REST API. - Expose read-only v0 tools for channels, collections, smart collections, schedules/items, playout lookup, FFmpeg profiles/resolution, sessions, and version. - Add focused NUnit coverage for tool catalog scope, API client path/auth/error behavior, and MCP initialize/tools/list/tools/call behavior. - Document configuration, tool list, `X-Api-Key` behavior, and deferred #63-#68 redesign workflows in `docs/mcp.md`. ## Verification - `TZ=UTC dotnet restore ErsatzTV.sln -v minimal` - `TZ=UTC dotnet build ErsatzTV.sln --no-restore -v minimal` - `TZ=UTC dotnet test ErsatzTV.Mcp.Tests/ErsatzTV.Mcp.Tests.csproj -v minimal` - `TZ=UTC dotnet test ErsatzTV.sln --no-build -v minimal` ## Deferred - v0.1 write tools are not included in this PR. - Workflow-level tools are deferred until the backend gaps under #63-#68 exist. Refs #58
timothy added 1 commit 2026-07-02 00:41:24 +02:00
feat(mcp): add read-only API server foundation refs #58
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 4m5s
Details
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 5m16s
Details
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Details
190106237f
timothy force-pushed issue58-mcp from 190106237f to 945d108334 2026-07-07 11:45:07 +02:00 Compare
timothy added 1 commit 2026-07-07 11:58:31 +02:00
chore: retrigger CI (MySQL service port collision with concurrent run)
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 4m0s
Details
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 4m29s
Details
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Details
d1c04030af
timothy added 3 commits 2026-07-12 01:26:17 +02:00
security(#289): harden MCP server (read-only posture, JSON-DoS, response cap, arg validation) 55fc210385 
From the #197 cold review's MCP-consumer pass. Hardens `ErsatzTV.Mcp` (PR #76)
before it can go live against a soon-remote-exposed API.

- HIGH verb-guard: runtime read-only *posture* (ERSATZTV_ALLOW_WRITES, default
  false) enforced in the executor — a wrong catalog entry can't mutate/delete,
  and future write tools (#58) slot in behind the opt-in. Not a hardcoded
  GET-only clamp, so the non-read-only final design stays reachable.
- HIGH JSON-crash DoS: guard JsonDocument.Parse in HandleAsync (-32700, id null),
  handle missing/non-string method (-32600) instead of throwing, and wrap the
  Program.Main read loop so one bad stdin line can never kill the session.
- MED unbounded response / LOW no-timeout: cap the buffered body
  (ERSATZTV_MAX_RESPONSE_BYTES, truncation marker) + explicit HttpClient.Timeout.
- MED args unvalidated: ToolArgumentValidator checks caller args against each
  tool's InputSchema (required present, no unknown args, basic types) before a
  request is built.
- MED prompt injection: documented untrusted-data posture in docs/mcp.md.
- LOW base-URL prefix: preserve a reverse-proxy path prefix in the URL join.

Also documents that ERSATZTV_API_KEY is now effectively required (Bundle A gates
all /api reads behind X-Api-Key). 14 new tests (26 total); live stdin smoke
confirms malformed/no-method/non-object lines no longer crash the loop.

Refs #289 #197 #58

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
security(#289): review fixes — body-read timeout, transport-error response, bounded stdin, env clamps 4f68805d9a 
Folds in findings from the independent reviews of the initial hardening diff
(a cold adversarial pass + a Codex pass — complementary catches).

Fork (HIGH/MED):
- Body-read timeout regression: ResponseHeadersRead moved the body read outside
  HttpClient.Timeout and it used CancellationToken.None, so a slow-drip upstream
  hung the single-threaded session. Now a per-request linked CTS (options
  .EffectiveRequestTimeout) covers headers + body; HttpClient.Timeout set to
  Infinite so one timer owns it. Verified live against a black-hole upstream:
  1s timeout → -32603 in ~1s, no hang.
- Transport/timeout exceptions escaped HandleAsync's catch filter → no response
  for the id → client hangs. Added a broad final catch → -32603 when hasId.

Codex (env/input robustness):
- ERSATZTV_REQUEST_TIMEOUT_SECONDS / ERSATZTV_MAX_RESPONSE_BYTES at int.MaxValue
  crashed at startup / overflowed `cap+1` to a negative alloc. ParseInt now
  clamps to [min,max]; the client also hard-ceils the cap (MaxAllowedResponseBytes).
- A ".."/"." path param collapsed the URL onto another route under Uri
  canonicalization — rejected in BuildPath.

Both reviewers (stdin OOM): ReadLineAsync buffered an unbounded line before any
guard. New BoundedLineReader drains+drops oversized lines (memory-bounded),
keeping subsequent lines aligned.

Nits: case-insensitive ParseBool fallback; UTF-8 boundary backoff so a mid-
codepoint truncation doesn't emit U+FFFD; renamed a misleading test.

34 tests (was 26); +8 covering the transport-error path, dot-segment rejection,
cap-overflow fallback, UTF-8 seam, and the bounded reader.

Refs #289 #197
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test(#289): pin BoundedLineReader cap/CRLF boundaries + document per-segment path assumption
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 4m29s
Details
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 4m42s
Details
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Details
65d88b5167 
Non-blocking polish from the fix-commit re-review (verdict MERGEABLE):
- BuildPath: comment the assumption that each {param} is its own path segment
  (revalidate the whole path if a template ever concatenates adjacent params).
- Two additive BoundedLineReader tests: exactly-at-cap line returned intact
  (inclusive max), and \r\n carriage-return stripping. No logic change.

36 tests (was 34).

Refs #289
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
timothy commented 2026-07-12 01:27:21 +02:00
Author
Owner
Copy Link
Copy Source

Folded the #197 MCP-consumer security hardening (#289) into this branch — d1c04030..65d88b51 (3 commits, fast-forward):

  • Runtime read-only posture (ERSATZTV_ALLOW_WRITES, default false) enforced in the executor — the runtime backstop behind the read-only catalog, forward-compatible with future write tools.
  • JSON-crash DoS guard (malformed/no-method/non-object lines never crash the loop; -32700/-32600/-32603 responses).
  • Per-request timeout covering headers + streamed body (verified live: black-hole upstream → error in ~1s, no hang); response-size cap; arg validation vs InputSchema; bounded stdin reader; ./.. path rejection; reverse-proxy prefix preservation; env clamps.
  • docs/mcp.md updated, incl. that ERSATZTV_API_KEY is now effectively required (Bundle A on main gates all /api reads behind X-Api-Key).

Reviews: 2 cold adversarial passes + 1 Codex pass on the initial diff (complementary catches), 1 cold re-review of the fix commit = MERGEABLE. 36 tests (was 12), 0 warnings, format-clean.

⚠️ This branch is ~5 days behind main (pre-Bundle-A/B, pre-#91b). Before this PR merges: merge main in, regenerate any generated artifacts, and run the full-solution CI. MCP go-live is also gated on #197 (arc item 5). See #289 for the full close-out.

Folded the #197 MCP-consumer security hardening (#289) into this branch — `d1c04030..65d88b51` (3 commits, fast-forward): - Runtime **read-only posture** (`ERSATZTV_ALLOW_WRITES`, default false) enforced in the executor — the runtime backstop behind the read-only catalog, forward-compatible with future write tools. - **JSON-crash DoS** guard (malformed/no-method/non-object lines never crash the loop; `-32700`/`-32600`/`-32603` responses). - **Per-request timeout** covering headers + streamed body (verified live: black-hole upstream → error in ~1s, no hang); **response-size cap**; **arg validation** vs `InputSchema`; **bounded stdin reader**; `.`/`..` path rejection; reverse-proxy prefix preservation; env clamps. - `docs/mcp.md` updated, incl. that **`ERSATZTV_API_KEY` is now effectively required** (Bundle A on `main` gates all `/api` reads behind `X-Api-Key`). Reviews: 2 cold adversarial passes + 1 Codex pass on the initial diff (complementary catches), 1 cold re-review of the fix commit = **MERGEABLE**. 36 tests (was 12), 0 warnings, format-clean. ⚠️ **This branch is ~5 days behind `main`** (pre-Bundle-A/B, pre-#91b). Before this PR merges: merge `main` in, regenerate any generated artifacts, and run the full-solution CI. MCP go-live is also gated on #197 (arc item 5). See #289 for the full close-out.
timothy commented 2026-07-12 23:43:15 +02:00
Author
Owner
Copy Link
Copy Source

claiming — session started 2026-07-12 (night). Finalizing this branch for the go-live main-merge (owner-directed as the first #197 frontier step, ahead of #286 versioning).

Plan: rebase issue58-mcp onto current origin/main (branch is 427 commits behind — pre-Bundle-A auth, pre-#91b), resolve conflicts (expect ErsatzTV.sln / docs/README.md / docs/decisions.md), regen any generated artifacts, full-solution build+test, live-verify the MCP against current main's /api surface (Bundle-A X-Api-Key gating is new since this branch was written), independent re-review of the rebase delta, then merge. #286 (/api/v1 route sweep) is sequenced immediately after this lands.

claiming — session started 2026-07-12 (night). Finalizing this branch for the go-live main-merge (owner-directed as the first #197 frontier step, ahead of #286 versioning). Plan: rebase `issue58-mcp` onto current `origin/main` (branch is 427 commits behind — pre-Bundle-A auth, pre-#91b), resolve conflicts (expect `ErsatzTV.sln` / `docs/README.md` / `docs/decisions.md`), regen any generated artifacts, full-solution build+test, **live-verify the MCP against current main's `/api` surface** (Bundle-A `X-Api-Key` gating is new since this branch was written), independent re-review of the rebase delta, then merge. `#286` (`/api/v1` route sweep) is sequenced immediately after this lands.
timothy commented 2026-07-12 23:46:31 +02:00
Author
Owner
Copy Link
Copy Source

Superseded — closing without merge (owner-directed 2026-07-12)

Decision: not merging this branch. It's the v0 read-only foundation, written 427 commits / ~5 days ago against a pre-Bundle-A, pre-#91b API. Rather than rebase 427 commits and then immediately route-sweep it in #286 (/api/v1 versioning, the next #197 frontier item, which rewrites every route), we'll rebuild the MCP fresh under the existing epic #58 — read + cautious-write CRUD — against the frozen /api/v1 contract once #286 lands.

Why fresh rather than expand-in-place:

  • The /api write surface has matured enormously since this branch (full ChicoryTV CRUD over channels/collections/schedules/playouts/media-sources, auth Bundle A/B, ETag/If-Match optimistic concurrency, the #253/#265 replace-all contract) → a read+write MCP is now viable, so the read-only-only framing is outdated.
  • Generating the MCP tool contracts once, against the post-#286 versioned+frozen surface, avoids build-then-immediately-regenerate churn and lets the #197 security pass review the write MCP against a stable contract.

Not lost: the branch issue58-mcp stays as reference (not deleting it), and the #289 hardening (runtime read-only posture / ERSATZTV_ALLOW_WRITES, JSON-DoS guards, per-request timeout, bounded stdin, arg validation, injection-framing docs) is the security baseline the fresh build must carry forward. The v0 plan doc + docs/mcp.md remain in history.

Re-scoping #58 accordingly.

## Superseded — closing without merge (owner-directed 2026-07-12) Decision: **not** merging this branch. It's the v0 **read-only** foundation, written 427 commits / ~5 days ago against a pre-Bundle-A, pre-#91b API. Rather than rebase 427 commits and then immediately route-sweep it in **#286** (`/api/v1` versioning, the next #197 frontier item, which rewrites every route), we'll rebuild the MCP **fresh under the existing epic #58** — read **+** cautious-write CRUD — against the **frozen `/api/v1` contract** once #286 lands. Why fresh rather than expand-in-place: - The `/api` write surface has matured enormously since this branch (full ChicoryTV CRUD over channels/collections/schedules/playouts/media-sources, auth Bundle A/B, ETag/If-Match optimistic concurrency, the #253/#265 replace-all contract) → a read+write MCP is now viable, so the read-only-only framing is outdated. - Generating the MCP tool contracts once, against the post-#286 versioned+frozen surface, avoids build-then-immediately-regenerate churn and lets the #197 security pass review the write MCP against a *stable* contract. **Not lost**: the branch `issue58-mcp` stays as reference (not deleting it), and the #289 hardening (runtime read-only posture / `ERSATZTV_ALLOW_WRITES`, JSON-DoS guards, per-request timeout, bounded stdin, arg validation, injection-framing docs) is the security baseline the fresh build must carry forward. The v0 plan doc + `docs/mcp.md` remain in history. Re-scoping #58 accordingly.
timothy closed this pull request 2026-07-12 23:46:56 +02:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
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
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#76