Files
xTeVe/tasks/improvement-plan.md
T
nathan 51c7830067
continuous-integration/drone/push Build encountered an error
Phase 0: remove auto-updater, pin Go 1.27.1, fix vet warnings, tidy ignores
- Delete BinaryUpdate, internal/up2date, GitHub/Update structs and the
  xteveAutoUpdate / update.url settings (UI rows, en.json, defaults).
  Settings-schema migrations kept and moved to src/migrate.go.
- Drop kardianos/osext dependency.
- xteve.go version 0200 -> 0201 to match changelog; Drone now fails on drift.
- go.mod go 1.27.1; Dockerfile and Drone golang images pinned to 1.27.1.
- Fix four go vet unreachable-code warnings.
- .gitignore: .gocache/, agent.md, skill.md. .dockerignore: build context
  no longer includes caches, ts/, tasks/ or markdown except the changelog.
- Drone: publish :latest only from master; other branches publish a
  branch-named tag so a feature push cannot replace the deployed image.
- Add tasks/improvement-plan.md and tasks/todo.md.
- Regenerate src/webUI.go.
2026-09-26 12:38:02 +10:00

21 KiB

xTeVe fork: review and improvement plan

Date: 2026-09-25 Scope: whole repository (Go backend, TypeScript UI, Docker, CI, docs) Status: reviewed with maintainer 2026-09-25/26, all open questions resolved, ready to start Phase 0. Checklist lives in tasks/todo.md.

1. Where the fork stands

  • Upstream (xteve-project/xTeVe) has not moved since commit 0e999b8 (March 2021). There is nothing to merge. This fork is effectively its own project now, so decisions can be made without worrying about upstream compatibility.
  • The fork adds 26 commits, 57 files, roughly +4800/-1600 lines: UI redesign, Plex API refresh (src/plex_api.go), relaxed/strict XEPG mapping with auto-remap (findXEPGReplacementChannel), wizard-completed flag, config-dir migration in docker/entrypoint.sh, Drone CI, Go 1.25/1.26 modernisation.
  • go build ./... passes. go vet ./... reports four "unreachable code" warnings. One test file exists (src/internal/m3u-parser). staticcheck cannot run locally because the installed binary predates Go 1.25.
  • The README already lists four next steps (go:embed, CI, /healthz, websocket integration tests). This plan absorbs them.

The codebase works, and the fork's additions are sensible. The problems are mostly inherited from upstream: a security model that assumes a trusted LAN and a trusted playlist, data races in the stream buffer, and a build pipeline that depends on committed generated files. None of them show up in daily use until they do.

2. Guiding principles

  1. Security and crash bugs before features. Several findings allow code execution or process crashes from data you do not control (playlists, EPG feeds, any web page open on the LAN).
  2. Tests before refactors. Every phase below starts by adding tests around the code it touches, so the refactor can be checked against current behaviour.
  3. Small, reviewable PRs. Each checklist item should be one PR. Avoid a "big bang" rewrite.
  4. Keep vanilla TypeScript for now. A framework migration is a 3 to 6 week rewrite and would block everything else. Revisit after Phase 5.
  5. Every phase ends green: go vet, go test -race, tsc --noEmit, Docker build.

3. Phases

Phase 0: Hygiene (about half a day, no risk)

  • Commit the pending .gitignore change (.gocache/), delete .gocache/ from the working tree, and add .gocache/, ts/, tasks/, *.md (except what the Dockerfile needs) to .dockerignore.
  • Decide where agent.md and skill.md live. Recommendation: move to .claude/ or tasks/ and ignore them, so the build context and repo root stay clean.
  • Fix version drift: xteve.go says 2.2.0.0200, changelog-beta.md says 0201. Add the drift check that the changelog claims exists to .drone.yml (compare grep of both and fail on mismatch), or drop the hardcoded default and make the changelog the single source.
  • Delete the auto-updater (decided 2026-09-25: Docker is the only distribution channel). As shipped, a fork build will download and install an upstream binary over itself if upstream ever tags a higher build. Remove BinaryUpdate and the internal/up2date package, the calls at xteve.go:191 and src/maintenance.go:69, the GitHub struct and -X-style branch/user plumbing in xteve.go, config.go:62,189-190 and info.go:25, and the xteveAutoUpdate setting from struct-system.go:305, ts/base_ts.ts:26, ts/settings_ts.ts:290,484 and html/lang/en.json. Keep conditionalUpdateChanges, convertToNewFilter and setValueForUUID in src/update.go; they are settings-schema migrations, not binary updates. Rename that file to migrate.go so the distinction is obvious. This also removes the kardianos/osext dependency.
  • Pin Go to 1.27.1 in go.mod, the Dockerfile and the Drone golang image (decided 2026-09-26).
  • Fix the four go vet unreachable-code warnings (buffer.go:733, maintenance.go:78, webserver.go:413, xepg.go:356).
  • Install a current staticcheck (or use golangci-lint) and record the baseline.

Phase 1: Security (1 to 2 weeks, must do)

The threat model to design for: an attacker who can serve you a playlist or EPG feed, or who can get a browser on your LAN to load a page. Both are realistic for an IPTV proxy.

1a. Browser-side (small, do first)

  • Stored XSS: provider-controlled strings (channel name, group, file name, log lines, descriptions) are written with innerHTML. Replace with textContent at ts/menu_ts.ts:534, 589, 2362-2390, ts/logs_ts.ts:19, ts/configuration_ts.ts:91, ts/settings_ts.ts:591. The unauthenticated /stream/ endpoint logs the caller's User-Agent, so this is reachable without a malicious playlist.
  • Websocket origin: wsUpgrader.CheckOrigin in src/webserver.go:21 returns true. Compare Origin host to r.Host and reject mismatches. Move the token out of the query string into the cookie, and stop logging requests and responses (with tokens) to the console in ts/network_ts.ts.
  • Cookie flags: set HttpOnly, SameSite=Strict on the Token cookie (src/internal/authentication/authentication.go:601), and stop writing it from JS.
  • Serving the wizard page sets Settings.AuthenticationWEB = false (src/webserver.go:650). A GET must not mutate global settings. Gate the wizard on wizard.completed only.

1b. Server-side input handling (small to medium)

  • uploadLogo writes to a client-supplied filename with no sanitisation (src/images.go:19). Apply filepath.Base and an extension allow-list.
  • Backup restore has no zip-slip check (src/compression.go:96). Reject entries whose cleaned path escapes the target directory.
  • ffmpeg.path / vlc.path accept any existing file, and ffmpeg.options is free text. Combined with the open websocket this is remote code execution. Options: restrict paths to a configured allow-list (Docker image ships a fixed ffmpeg), or require authentication to change them, or both.
  • Put /download/ (backups containing authentication.json and the Plex token) behind authentication.
  • Stop deriving System.Domain from the request Host header on every request (setGlobalDomain). Use a configured base URL with Host as a fallback only when unset.
  • Provider URLs reach ffmpeg's -i unchanged; reject non-http(s) schemes so file: and concat: sources are not possible.

1c. Authentication (small to medium, needs migration)

  • SHA256(secret, salt) ignores the salt and computes an unsalted HMAC with the password as key. Replace with bcrypt (golang.org/x/crypto/bcrypt), migrate each user on next successful login, and use constant-time comparison.
  • Tokens minted for URL/Basic auth calls are never evicted (authentication.go:243, 575-585). Plex polls constantly, so memory grows for the life of the process. Add expiry and a periodic sweep.
  • Any authenticated web user can change any other user's credentials (src/data.go:606). Enforce that only admins can edit other users.
  • Plex token is stored in plaintext in settings.json (mode 0644) and pushed to every websocket client. Write settings with 0600 and redact the token in the config payload sent to the UI.
  • Web authentication stays off by default (LAN-only deployment, decided 2026-09-25). Document this in the README security note.

Phase 2: Stability and correctness in the streaming path (2 to 3 weeks)

This is where the "works well until it doesn't" risk sits. Start with go test -race tests that spin up two concurrent tuners against a local httptest server that streams a fake TS file, then fix:

  • Shared maps without locks. Playlist is stored by value in a sync.Map but contains maps, so every copy shares them. bufferingStream, connectToStreamingServer and killClientConnection write those maps from different goroutines, some under Lock, some not (src/buffer.go:101, 125, 189, 208, 462, 480, 575, 929). Give Playlist its own mutex, store *Playlist, and remove the store-back of stale copies at lines 756 and 930.
  • Tuner limit is a check-then-act race (buffer.go:152). Reserve the slot under the playlist lock.
  • force=true in killClientConnection deletes the stream but never cleans BufferClients.
  • Data.Cache.StreamingURLS is written by /lineup.json and /m3u while /stream/ reads it (src/system.go:338-390). Guard with an RWMutex.
  • Deferred closes inside read loops (buffer.go:880, 891, also 316, 707-737; compression.go:106, 112; imgcache/cache.go:106, 118). A multi-hour stream accumulates millions of deferred calls. Close explicitly or move the loop body into a function.
  • ffmpeg/VLC process management (buffer.go:1345-1641): timeout goroutine can block forever, the 20 s timeout cannot fire while Read blocks, cmd.Start() error ignored, panic inside a goroutine kills the process, double-open of the segment file leaks an fd. Rewrite around exec.CommandContext with the request context, which also replaces the deprecated http.CloseNotifier. Decided 2026-09-25: keep VLC. Structure the rewrite as one external-process buffer that takes a binary path plus an argument builder, with ffmpeg and VLC as two small builders. VLC then costs nothing to keep, the process-management fixes apply to both, and tests can cover the builders without VLC installed. The Docker image continues to ship ffmpeg only.
  • Fake mutexes in src/screen.go:55, 115, 130 (a new sync.RWMutex{} per call). Use one package-level mutex for the log.
  • Timeouts everywhere. Replace http.ListenAndServe with an http.Server with read/write/idle timeouts, and give every outbound http.Client a timeout (provider fetch, logo download, update check). A hung provider currently leaves ScanInProgress stuck.
  • imgcache holds its lock during every logo download, blocking /m3u/ and the XMLTV build. Download outside the lock.
  • Atomic file writes. Every save is a truncate-then-write. Write to a temp file in the same directory, fsync, rename. One helper, used by writeByteToFile and friends.
  • Concrete bugs to fix while there: xepg.go:249 appends instead of removing (Files[:i]); data.go:882 mutates a slice while ranging; screen.go:193 drops newest log lines when full; buffer.go:161-163 sets headers after WriteHeader and names one "Content-Length:"; screen.go:412 evicts random notifications; unchecked type assertions and indexing in data.go, backup.go, provider.go, authentication.go:89, screen.go:104; API double-write after responseAPIError (webserver.go:974-978); websocket request/response structs reused across iterations so error state and stale fields leak (webserver.go:334-335).
  • Error hygiene: the authenticationErr closures return from the closure not the caller; remove os.Exit/log.Fatal/panic outside main; exit non-zero on fatal errors; check ignored results of getProviderData, saveSettings, writeByteToFile.

Phase 3: Build, embed, CI, Docker (1 week, low risk, big quality-of-life win)

  • Replace src/webUI.go with //go:embed html. The generated file is 783 KB, map-ordered so it changes on every run, and appears in most UI commits. Serve via fs.Sub and http.FileServer; keep template substitution for HTML only. In -dev mode use os.DirFS("html"). Delete src/html-build.go and cmd/webui-gen. Add cache headers and ETags.
  • Drop i18n (decided 2026-09-26). The JS currently contains Go template placeholders ({{.settings.x.title}}) so it cannot be bundled, cached, or run outside the server. Only English exists. Inline the English strings from html/lang/en.json into the TypeScript, delete en.json and the template pass for JS and CSS, and keep parseTemplate only for the HTML pages that need server values. This is a prerequisite for the next item. Mechanical approach: a one-off script that reads en.json and rewrites every {{.path}} in ts/*.ts with the string literal, then a review pass.
  • Pinned TypeScript toolchain. Add package.json and tsconfig.json (strict, ES2020 target), bundle with esbuild to one unminified html/js/app.js, and run tsc --noEmit in CI. The bundle is committed (decided 2026-09-26) so go build works from a bare clone and the Dockerfile needs no Node stage; CI rebuilds it and fails on a diff. Delete the ten unreferenced legacy files in html/js/ (mapping-editor.js alone is 1465 lines) which are currently embedded in every binary.
  • CI hardening in .drone.yml: add go vet, golangci-lint, gofmt -l, go test -race, tsc --noEmit, and a check that committed build outputs are current. Publish images tagged with the version as well as latest and SHA. amd64 only (decided 2026-09-26); no multi-arch step.
  • Docker runtime: UID/GID are fixed at build time so prebuilt images cannot change them. Switch to the PUID/PGID + su-exec pattern in the entrypoint. Pin mwader/static-ffmpeg to a tag. Add VOLUME. Verify the healthcheck still passes with web auth enabled. Remove /xteve from LEGACY_CONFIG_DIRS (it is the parent of the default dir). Note in the README that SSDP needs host networking.
  • Add /healthz as the README suggests, so the healthcheck and monitoring do not depend on HDHomeRun endpoints.
  • Docs: write README-DEV.md (build, dev mode, UI toolchain, release and versioning process) and add a fork section to README.md (registry, compose files, env vars XTEVE_CONFIG/XTEVE_PORT/PUID/PGID, differences from upstream, security notes). Remove the upstream donation block.
  • Code cleanup (moved from Phase 4): deduplicate the image-caching goroutine (xepg.go:78-93 and 123-138), addErrorToStream (buffer.go:581, 1379), extractZIP/mapToJSON/randomString; delete dead code (Auto, getStreamByChannelID, updateXEPG, indexOfInt, commented-out blocks in struct-buffer.go and authentication.go); drop io/ioutil (12 files), kardianos/osext, rand.Seed. Do this after go:embed lands so the diffs stay readable.
  • Note: agent.md references docs/design-system/ and tasks/design-foundation.md, neither of which exist in this repo. Either add them or remove that section.

Phase 4: Data model and performance (NOT PLANNED)

Decided 2026-09-25: skipped. The real deployment is one M3U with 174 streams and 167 XEPG channels, and that is not expected to change. None of the quadratic passes matter at that size. The list below is kept for reference only; the non-performance cleanup items (dead code, duplicates, deprecated packages) have moved to Phase 3. Revisit only if a rebuild ever takes more than a few seconds.

  • Data.XEPG.Channels is map[string]any and every pass does json.MarshalIndent then Unmarshal per channel (xepg.go:370, 389, 405, 462, 659, 889, 1265). Type it as map[string]*XEPGChannelStruct.
  • getProgramData scans every programme for every channel (xepg.go:961). Index programmes by channel ID once per build.
  • xepg.json is written three times per rebuild; settings.json once per provider; urls.json on every /lineup.json request. Debounce and write once.
  • filterThisStream compiles regexps per stream per filter (m3u.go:65, 77). Precompile.
  • buildM3U uses string concatenation; use strings.Builder. The XMLTV output is built entirely in memory then copied; stream it to the gzip writer.
  • Web() re-parses the language JSON per request (webserver.go:614-634). Parse once.
  • Parsed XMLTV is cached forever (xepg.go:1221). Drop it after the build.
  • Deduplicate: image-caching goroutine (xepg.go:78-93 and 123-138), addErrorToStream (buffer.go:581, 1379), extractZIP/mapToJSON/randomString across packages. Delete dead code (Auto, getStreamByChannelID, updateXEPG, indexOfInt, commented-out blocks in struct-buffer.go and authentication.go).
  • Modernise: drop io/ioutil (12 files), kardianos/osext (use os.Executable), rand.Seed, http.CloseNotifier; adopt context, slog, errors.Is, slices, maps.

Phase 5: Frontend architecture (2 to 4 weeks, medium risk)

The UI redesign left a consistent visual layer. The problems are underneath it.

  • Websocket client: each request opens a new socket, and a global flag silently drops any request made while one is in flight (ts/network_ts.ts:9-11). Replace with one persistent socket, request IDs, a promise per request, a queue, and exponential-backoff reconnect. This is what "reconnection" should mean instead of a failure counter.
  • Stop rebuilding the whole UI on every response. createLayout() re-renders everything, losing focus and scroll position. Diff by section, or at minimum re-render only the active tab.
  • Mapping table: renders every channel with no virtualisation or pagination. Add windowed rendering; this is the single largest UX win for big lineups.
  • Code structure: menu_ts.ts is 2472 lines with 34 string onclick handlers (one typo'd javscript: at line 551). Split into modules, use addEventListener, make the settings table data-driven (about 20 copy-pasted blocks in settings_ts.ts:24-437).
  • Accessibility follow-ups: buttons instead of clickable <td>, <button> instead of <input type=button>, replace alert() with the existing announcer, drop deprecated language="javascript". Add a light theme via prefers-color-scheme using the existing :root tokens.
  • Merge base.css and screen.css into a layered structure (tokens, base, components, layout) so the boundary is clear.

Phase 6: Features worth borrowing (optional, after 1 to 3)

Threadfin is the most active successor fork. Its additions that fit this codebase without a rewrite:

  • Regex include/exclude filters (the current filter is substring based).
  • Per-playlist tuner limits and per-playlist buffer choice.
  • Bulk channel editing in the mapping table (the fork already has range-select checkboxes; this is the next step).
  • Working backup/restore with schema versioning (ties into the atomic-write work in Phase 2).
  • Dummy EPG with selectable durations.

4. Test strategy

There is one test today. Aim for the following minimum before each phase's refactor:

Area Test type Phase
Websocket command dispatch httptest + gorilla client, one test per cmd 1
Auth: login, token expiry, bcrypt migration, user edit permissions unit 1
Upload/restore path traversal unit with crafted names and zips 1
Buffer: two concurrent tuners, tuner limit, client disconnect cleanup integration with -race, fake TS server 2
Atomic write helper: crash simulation leaves old file intact unit 2
XEPG mapping: strict vs relaxed, auto-remap with 0/1/2 candidates table-driven unit on fixtures 2, 4
M3U/XMLTV parsing and filtering extend existing parser tests 4
XEPG build benchmark go test -bench on 2000-channel fixture 4
TypeScript tsc --noEmit in CI; consider Vitest for the websocket client once it is a module 3, 5

5. What this plan deliberately does not do

  • No framework migration now. A Preact/Svelte rewrite is the largest item on the list and blocks security and stability fixes. Phase 5 makes the vanilla code modular enough that a later migration can be incremental.
  • No upstream merge. There is nothing to merge; upstream is inactive.
  • No new features before Phase 3. The value is in stopping the process from crashing or being hijacked, and in making the next change cheap to ship.

6. Suggested order and rough timeline

Order Phase Effort Why this position
1 0 Hygiene 0.5 day Free, removes foot-guns (auto-update from upstream)
2 1a Browser security 1 to 2 days Smallest fix for the worst exposure
3 3 go:embed + TS toolchain + CI 1 week Every later PR gets smaller and safer to review
4 1b, 1c Server security + auth 1 week Needs the test harness from step 3's CI
5 2 Streaming stability 2 to 3 weeks Highest complexity; do with race tests in place
6 5 Frontend architecture 2 to 4 weeks Builds on the bundled TS from step 3
7 6 Features as desired Only once the base is solid

7. Open questions

These change the shape of the work and should be settled before starting:

  1. Is the deployment always Docker on a trusted LAN, or is xTeVe ever exposed beyond it? Decided 2026-09-25: trusted LAN only. Consequences: keep all of Phase 1a and 1b (a LAN browser or a hostile playlist is still in scope), keep bcrypt and token eviction in 1c, but do not change the auth-off default for fresh installs. Restrict ffmpeg.path to an allow-list rather than requiring auth to change it.
  2. How large are your playlists and EPG files? Decided 2026-09-25: 174 streams / 167 channels, one M3U, stable. Phase 4 dropped.
  3. Do you use the VLC buffer? Decided 2026-09-25: ffmpeg in practice, but keep VLC as an option. Implemented as a shared external-process buffer with per-tool argument builders (see Phase 2).
  4. Is the auto-updater used? Decided 2026-09-25: Docker only. Updater deleted in Phase 0; settings migrations in update.go are kept.
  5. Is multi-arch (arm64) needed for the image? Decided 2026-09-26: amd64 only.
  6. Keep the language layer? Decided 2026-09-26: drop it. English strings inlined in the TypeScript.
  7. Where does the TypeScript build run? Decided 2026-09-26: commit the esbuild bundle; CI verifies freshness; no Node in Docker.