Stop a .strm growing on every boot, and let TorrServer come back

CHANGELOG.md

@@ -9,6 +9,23 @@ Entries for 0.1.0 – 0.2.1 are backfilled from the published GitHub release not
 
 ## [Unreleased]
 
+### Fixed
+- **A legacy `.strm` grew on every restart.** The startup sweep decided a file was "already
+  signed" with `strings.Contains(url, "?t=")` — but the legacy `/proxy/stream` route spells its
+  token `&t=`, because that URL already has a query. So such a file never looked signed, was
+  re-signed on every boot, and the token was **appended** each time. One file on a live box had
+  the same token repeated nine times.
+
+  The check now parses the URL and looks for a `t` parameter, and signing **sets** rather than
+  appends — which also repairs a file that has already grown, collapsing the repeats back to
+  one on the next start.
+- **TorrServer could not recover from a clean exit.** Its unit used `Restart=on-failure`, and
+  TorrServer exposes an HTTP `/shutdown` that exits 0 — a success, as far as systemd is
+  concerned, so it was never restarted. Nothing noticed either: the VPN watchdog clears its
+  strike counter for a down TorrServer on the grounds that "systemd `Restart=` handles it".
+  It did not, and the box sat with no streaming engine. Now `Restart=always`; an explicit
+  `systemctl stop` is still respected.
+
 ## [0.7.4] - 2026-09-05
 
 ### Fixed

cmd/orchestrator/helpers.go

@@ -572,7 +572,7 @@ func sweepOrphanStrmTokens(db *store.Store, cfg *config.Config) {
 				return nil
 			}
 			cur := strings.TrimSpace(string(raw))
-			if cur == "" || strings.Contains(cur, "?t=") {
+			if cur == "" || hasPlayToken(cur) {
 				return nil // already carries a capability token
 			}
 			mediaType, tmdbID, season, episode, ok := parsePlayURL(cur)
@@ -594,10 +594,7 @@ func sweepOrphanStrmTokens(db *store.Store, cfg *config.Config) {
 				// closed. Signing keeps them working for exactly what they already point at.
 				if h, i, lok := parseLegacyStream(cur); lok {
 					if tok := streamToken(h, i); tok != "" {
-						signed := cur + "?t=" + tok
-						if strings.Contains(cur, "?") {
-							signed = cur + "&t=" + tok
-						}
+						signed := withPlayToken(cur, tok)
 						if werr := os.WriteFile(path, []byte(signed), 0o644); werr == nil {
 							rewritten++
 							return nil
@@ -658,6 +655,35 @@ func parsePlayURL(raw string) (mediaType string, tmdbID, season, episode int, ok
 	return "", 0, 0, 0, false
 }
 
+// hasPlayToken reports whether a .strm URL already carries a capability token.
+//
+// Parsed, not string-matched. The string test was `strings.Contains(cur, "?t=")`, and the
+// legacy route spells its token "&t=" because it already has a query (`?link=…&index=…`).
+// So a legacy file never looked signed, was re-signed on EVERY startup, and the token was
+// APPENDED each time — the file grew by one "&t=<43 chars>" per boot, indefinitely.
+func hasPlayToken(raw string) bool {
+	u, err := url.Parse(strings.TrimSpace(raw))
+	if err != nil {
+		return false
+	}
+	return u.Query().Get("t") != ""
+}
+
+// withPlayToken returns raw with exactly one t= parameter, whatever it had before.
+//
+// Set, never append. Appending is what grew the file; setting also REPAIRS one that has
+// already grown, because Query() collapses the repeats and Encode() writes a single value.
+func withPlayToken(raw, token string) string {
+	u, err := url.Parse(strings.TrimSpace(raw))
+	if err != nil {
+		return raw
+	}
+	q := u.Query()
+	q.Set("t", token)
+	u.RawQuery = q.Encode()
+	return u.String()
+}
+
 // parseLegacyStreamHash pulls the info hash out of a pre-resolve-at-play .strm.
 func parseLegacyStreamHash(raw string) string {
 	h, _, ok := parseLegacyStream(raw)

cmd/orchestrator/strmtoken_test.go

@@ -0,0 +1,64 @@
+package main
+
+import (
+	"net/url"
+	"strings"
+	"testing"
+)
+
+// The startup sweep decided "already signed" with strings.Contains(cur, "?t="). The legacy
+// route spells its token "&t=", because the URL already has a query (?link=…&index=…). So a
+// legacy .strm never looked signed, was re-signed on EVERY boot, and the token was APPENDED
+// each time — one "&t=<43 chars>" per restart, for as long as the box kept running.
+//
+// Observed on a live box: one file with the same token repeated nine times.
+func TestPlayTokenIsDetectedAndSetNotAppended(t *testing.T) {
+	const legacy = "http://box:1990/proxy/stream?link=2ef50cc10f6eb563314c2db47d6a4e04734312e1&index=0"
+	const tok = "UNhCvh-RuisST-JSN50Lq1M6aJwH_KA1o5H6l5iMjCM"
+
+	if hasPlayToken(legacy) {
+		t.Error("an unsigned legacy URL was reported as already carrying a token")
+	}
+
+	signed := withPlayToken(legacy, tok)
+	if !hasPlayToken(signed) {
+		t.Fatalf("a freshly signed URL was not detected as signed: %s", signed)
+	}
+	if got := strings.Count(signed, "t="); got != 1 {
+		t.Errorf("expected exactly one t= parameter, got %d: %s", got, signed)
+	}
+
+	// The actual regression: re-signing must be idempotent, not additive.
+	again := withPlayToken(signed, tok)
+	if again != signed {
+		t.Errorf("re-signing changed the URL — it will grow on every boot:\n  %s\n  %s", signed, again)
+	}
+
+	// And it must REPAIR a file that has already grown.
+	grown := legacy
+	for i := 0; i < 9; i++ {
+		grown += "&t=" + tok
+	}
+	repaired := withPlayToken(grown, tok)
+	if got := strings.Count(repaired, "t="); got != 1 {
+		t.Errorf("a grown URL was not collapsed to one token, got %d: %s", got, repaired)
+	}
+
+	// Repaired or not, it must still address the same torrent and file.
+	u, err := url.Parse(repaired)
+	if err != nil {
+		t.Fatalf("repaired URL does not parse: %v", err)
+	}
+	if u.Query().Get("link") != "2ef50cc10f6eb563314c2db47d6a4e04734312e1" ||
+		u.Query().Get("index") != "0" || u.Query().Get("t") != tok {
+		t.Errorf("repair lost or altered a parameter: %s", repaired)
+	}
+
+	// A modern identity URL spells it "?t=" and must still be recognised.
+	if !hasPlayToken("http://box:1990/play/movie/550?t=" + tok) {
+		t.Error("a normal /play URL was not detected as signed")
+	}
+	if hasPlayToken("http://box:1990/play/movie/550") {
+		t.Error("an unsigned /play URL was reported as signed")
+	}
+}

release/install.sh

@@ -930,7 +930,13 @@ User=$TS_USER
 NetworkNamespacePath=/var/run/netns/vpntorrent
 BindReadOnlyPaths=/etc/netns/vpntorrent/resolv.conf:/etc/resolv.conf
 ExecStart=/usr/local/bin/torrserver --port 8090 --path /var/lib/torrserver
-Restart=on-failure
+# always, NOT on-failure. TorrServer exposes an HTTP /shutdown route that exits 0, so a
+# clean exit is a SUCCESS as far as systemd is concerned and on-failure never restarts it.
+# The box then has no streaming engine until somebody notices — and nothing notices:
+# vpntorrent/watchdog.sh clears its strike counter for a down TorrServer on the grounds that
+# "systemd Restart= handles it", which it did not. Verified by a real outage.
+# An explicit `systemctl stop` is still respected; systemd does not fight a deliberate stop.
+Restart=always
 RestartSec=3
 StartLimitIntervalSec=300
 StartLimitBurst=5

tests/install/test_install.sh

@@ -144,6 +144,10 @@ assert_mode "$DEST/opt/jellyfreedom/web" 755
 # added afterwards were silently missing from Jellyfin. Privacy is not this knob's job — the
 # data dir, the VPN configs and the database each carry an explicit restrictive mode.
 assert_contains "$DEST/etc/systemd/system/jellyfreedom.service" 'UMask=0022'
+# TorrServer has an HTTP /shutdown that exits 0, so on-failure never restarts it and the box
+# is left with no streaming engine. This is not hypothetical — it happened.
+assert_contains "$DEST/etc/systemd/system/torrserver-netns.service" 'Restart=always'
+assert_not_contains "$DEST/etc/systemd/system/torrserver-netns.service" 'Restart=on-failure'
 assert_not_contains "$DEST/etc/systemd/system/jellyfreedom.service" 'UMask=0077'
 assert_mode "$DEST/var/lib/jellyfreedom/vpnconfigs" 700