fix(security): harden HLS session IDs, /health disclosure, archive password handling

Phase 1 security audit follow-up:

- Reject HLS session IDs that aren't safe filesystem components
  (regex allowlist) to defend against path traversal via a buggy or
  compromised server. Applied at StartHLSSession and at the /hls URL
  handler; invalid IDs share the 404 of unknown sessions so the
  accepted format isn't enumerable.
- /health no longer leaks the active filename, taskID prefix or client
  IP to non-loopback callers. Uses net.IP.IsLoopback so IPv4-mapped
  IPv6 (::ffff:127.0.0.1) is recognised and the empty-string parse
  failure stops bypassing the boundary.
- unrar/7z passwords now travel through stdin instead of -p<password>
  in argv, removing /proc/<pid>/cmdline disclosure. Control characters
  in the password are rejected up front so a hostile NZB cannot feed
  extra prompt answers. Both invocations are bounded by a 30-minute
  context to stop indefinite hangs if the tool ever decides to prompt.

internal/engine/hls.go

@@ -241,6 +241,9 @@ func StartHLSSession(ctx context.Context, cfg HLSSessionConfig) (*HLSSession, er
 	if cfg.SessionID == "" {
 		return nil, errors.New("hls: empty session id")
 	}
+	if !validSessionID.MatchString(cfg.SessionID) {
+		return nil, errors.New("hls: invalid session id")
+	}
 	if cfg.SourcePath == "" {
 		return nil, errors.New("hls: empty source path")
 	}

internal/engine/hls_test.go

@@ -261,3 +261,33 @@ func TestCleanupHLSOrphanDirsMissingRoot(t *testing.T) {
 		t.Errorf("CleanupHLSOrphanDirs on missing root = %v, want nil", err)
 	}
 }
+
+func TestValidSessionID(t *testing.T) {
+	good := []string{
+		"abc",
+		"7b8c4f12-9d3e-4a1b-9c2f-aabbccddeeff",
+		"ABC_123-xyz",
+		strings.Repeat("a", 128),
+	}
+	bad := []string{
+		"",
+		"../etc/passwd",
+		"foo/bar",
+		"foo\\bar",
+		"foo.bar",
+		"with spaces",
+		"with\nnewline",
+		strings.Repeat("a", 129),
+		"héctor", // non-ascii
+	}
+	for _, id := range good {
+		if !validSessionID.MatchString(id) {
+			t.Errorf("validSessionID rejected good id %q", id)
+		}
+	}
+	for _, id := range bad {
+		if validSessionID.MatchString(id) {
+			t.Errorf("validSessionID accepted bad id %q", id)
+		}
+	}
+}

internal/engine/stream_server.go

@@ -303,6 +303,12 @@ func (ss *StreamServer) hlsHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	sessionID := parts[0]
+	// Reject malformed IDs with the same 404 we return for unknown sessions —
+	// no oracle for the accepted format.
+	if !validSessionID.MatchString(sessionID) {
+		http.Error(w, "hls session not found", http.StatusNotFound)
+		return
+	}
 	session := ss.hls.Get(sessionID)
 	if session == nil {
 		http.Error(w, "hls session not found", http.StatusNotFound)
@@ -392,6 +398,17 @@ func (ss *StreamServer) healthHandler(w http.ResponseWriter, r *http.Request) {
 	ss.mu.RUnlock()
 
 	clientIP, _, _ := net.SplitHostPort(r.RemoteAddr)
+	// Only expose filename/taskID/client to loopback callers (local diagnostics).
+	// Remote callers (LAN, Tailscale, UPnP public) get a minimal probe response
+	// so that scanners and unauthenticated peers cannot fingerprint the active
+	// download. The web stream-probe only checks HTTP 200 + Content-Type.
+	//
+	// Use net.IP.IsLoopback so we also accept ::ffff:127.0.0.1 (Linux dual-stack
+	// IPv4-mapped form) and reject the empty-string fallthrough when
+	// SplitHostPort fails on a malformed RemoteAddr — both would otherwise
+	// silently bypass the disclosure boundary.
+	parsedIP := net.ParseIP(clientIP)
+	isLocal := parsedIP != nil && parsedIP.IsLoopback()
 
 	type healthResponse struct {
 		Status    string `json:"status"`
@@ -399,21 +416,25 @@ func (ss *StreamServer) healthHandler(w http.ResponseWriter, r *http.Request) {
 		File      string `json:"file,omitempty"`
 		Task      string `json:"task,omitempty"`
 		Port      int    `json:"port"`
-		Client    string `json:"client"`
+		Client    string `json:"client,omitempty"`
 	}
 	resp := healthResponse{
 		Status: "ok",
 		Port:   ss.port,
-		Client: clientIP,
 	}
 	if provider != nil {
 		resp.Streaming = true
+	}
+	if isLocal {
+		resp.Client = clientIP
+		if provider != nil {
 			resp.File = provider.FileName()
 			resp.Task = taskID
 			if len(resp.Task) > 8 {
 				resp.Task = resp.Task[:8]
 			}
 		}
+	}
 
 	w.Header().Set("Content-Type", "application/json")
 	w.Header().Set("Cache-Control", "no-cache")

internal/engine/stream_server_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/http/httptest"
 	"strings"
 	"sync"
 	"testing"
@@ -379,6 +380,86 @@ func TestStreamServer_Health_WithFile(t *testing.T) {
 	}
 }
 
+// TestStreamServer_Health_NonLoopback_NoLeak verifica que /health no revela
+// nombre de fichero, taskID ni client IP cuando el caller no es loopback.
+// Protección contra reconnaissance vía LAN / UPnP / Tailscale.
+func TestStreamServer_Health_NonLoopback_NoLeak(t *testing.T) {
+	srv := NewStreamServer(0)
+	srv.disableUPnP = true
+	ctx := context.Background()
+	if err := srv.Listen(ctx); err != nil {
+		t.Fatalf("Listen() error: %v", err)
+	}
+	defer srv.Shutdown(ctx)
+
+	provider := newFakeProvider("secret.mkv", []byte("data"))
+	srv.SetFile(provider, "secret-task-id")
+
+	cases := []struct {
+		name       string
+		remoteAddr string
+	}{
+		{"lan_ipv4", "192.168.1.50:54321"},
+		{"empty_host_no_bypass", ":54321"},
+		{"public_ipv4", "203.0.113.10:443"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			rr := httptest.NewRecorder()
+			req := httptest.NewRequest(http.MethodGet, "/health", nil)
+			req.RemoteAddr = tc.remoteAddr
+			srv.healthHandler(rr, req)
+
+			body := rr.Body.String()
+			if !strings.Contains(body, `"status":"ok"`) {
+				t.Errorf("body missing status:ok: %q", body)
+			}
+			if !strings.Contains(body, `"streaming":true`) {
+				t.Errorf("body should report streaming bool: %q", body)
+			}
+			if strings.Contains(body, "secret.mkv") {
+				t.Errorf("body leaked filename: %q", body)
+			}
+			if strings.Contains(body, "secret-t") {
+				t.Errorf("body leaked task id: %q", body)
+			}
+			if strings.Contains(body, "192.168.1.50") || strings.Contains(body, "203.0.113.10") {
+				t.Errorf("body leaked client ip: %q", body)
+			}
+		})
+	}
+}
+
+// TestStreamServer_HLS_InvalidSessionID verifica que el hlsHandler rechaza
+// session IDs con caracteres ilegales devolviendo 404 (uniforme con sesión
+// inexistente) para no filtrar el formato aceptado a un attacker.
+func TestStreamServer_HLS_InvalidSessionID(t *testing.T) {
+	srv := NewStreamServer(0)
+	srv.disableUPnP = true
+	ctx := context.Background()
+	if err := srv.Listen(ctx); err != nil {
+		t.Fatalf("Listen() error: %v", err)
+	}
+	defer srv.Shutdown(ctx)
+
+	bad := []string{
+		"/hls/..%2Fetc%2Fpasswd/master.m3u8",
+		"/hls/foo.bar/master.m3u8",
+		"/hls/foo%20bar/master.m3u8",
+		"/hls/foo%2Fbar/master.m3u8",
+	}
+	for _, path := range bad {
+		t.Run(path, func(t *testing.T) {
+			rr := httptest.NewRecorder()
+			req := httptest.NewRequest(http.MethodGet, path, nil)
+			srv.hlsHandler(rr, req)
+			if rr.Code != http.StatusNotFound {
+				t.Errorf("path %q: status = %d, want 404", path, rr.Code)
+			}
+		})
+	}
+}
+
 // TestStreamServer_MKV_ContentType verifica que el Content-Type para .mkv
 // es el correcto.
 func TestStreamServer_MKV_ContentType(t *testing.T) {

internal/engine/validate.go

@@ -0,0 +1,12 @@
+// Package engine — validate.go centralises input validators used by the
+// stream/HLS HTTP handlers and the daemon glue. Keep new validators in this
+// file so a future reviewer can audit the trust boundary in one place.
+package engine
+
+import "regexp"
+
+// validSessionID restricts session IDs to characters safe for use as a single
+// filesystem path component. Server-issued UUIDs and hex strings match this;
+// anything containing slashes, dots, or path separators is rejected so a
+// compromised or buggy server cannot escape hlsTmpDirRoot via os.MkdirAll.
+var validSessionID = regexp.MustCompile(`^[a-zA-Z0-9_-]{1,128}$`)

internal/usenet/postprocess/extract.go

@@ -1,6 +1,7 @@
 package postprocess
 
 import (
+	"context"
 	"fmt"
 	"log"
 	"os"
@@ -8,8 +9,25 @@ import (
 	"path/filepath"
 	"regexp"
 	"strings"
+	"time"
 )
 
+// extractTimeout caps how long a single extractor invocation may run. Without
+// a cap, an encrypted archive that triggers a TTY-only prompt (or a corrupt
+// archive that confuses the tool) hangs the post-process pipeline forever.
+const extractTimeout = 30 * time.Minute
+
+// validatePassword rejects passwords containing control characters that could
+// inject extra answers into unrar/7z prompts via stdin (e.g. a newline lets an
+// attacker-controlled NZB password feed a second response to overwrite or
+// rename prompts).
+func validatePassword(password string) error {
+	if strings.ContainsAny(password, "\r\n\x00") {
+		return fmt.Errorf("invalid password: contains control characters")
+	}
+	return nil
+}
+
 // ExtractorType identifies which extraction tool is available.
 type ExtractorType string
 
@@ -50,18 +68,35 @@ func Extract(archivePath string, outputDir string, password string) ([]string, e
 }
 
 // extractUnrar extracts using unrar.
+//
+// Security: when a password is supplied it is sent via stdin rather than via
+// the `-p<password>` switch so it does not appear in `/proc/<pid>/cmdline`
+// (visible to any other process on the host). unrar prompts for the password
+// when no `-p` switch is given, and reads the prompt response from stdin when
+// no controlling TTY is attached (the usual case for a daemon-spawned child).
 func extractUnrar(unrarPath, archivePath, outputDir, password string) ([]string, error) {
+	if err := validatePassword(password); err != nil {
+		return nil, err
+	}
 	args := []string{"x", "-o+", "-y"}
-	if password != "" {
+	if password == "" {
-		args = append(args, "-p"+password)
+		// Tell unrar there is no password so it skips the prompt and fails
-	} else {
+		// fast on encrypted archives instead of hanging.
-		args = append(args, "-p-") // no password, skip asking
+		args = append(args, "-p-")
 	}
 	args = append(args, archivePath, outputDir+"/")
 
-	cmd := exec.Command(unrarPath, args...)
+	ctx, cancel := context.WithTimeout(context.Background(), extractTimeout)
+	defer cancel()
+	cmd := exec.CommandContext(ctx, unrarPath, args...)
 	cmd.Dir = outputDir
+	if password != "" {
+		cmd.Stdin = strings.NewReader(password + "\n")
+	}
 	output, err := cmd.CombinedOutput()
+	if ctx.Err() == context.DeadlineExceeded {
+		return nil, fmt.Errorf("unrar: timed out after %s", extractTimeout)
+	}
 	if err != nil {
 		// Check for password error
 		outStr := string(output)
@@ -75,18 +110,33 @@ func extractUnrar(unrarPath, archivePath, outputDir, password string) ([]string,
 }
 
 // extract7z extracts using 7z.
+//
+// Security: same rationale as extractUnrar — passwords go through stdin to
+// avoid `/proc/<pid>/cmdline` exposure. 7z reads the password from stdin when
+// no `-p` switch is given and the archive is encrypted.
 func extract7z(szPath, archivePath, outputDir, password string) ([]string, error) {
+	if err := validatePassword(password); err != nil {
+		return nil, err
+	}
 	args := []string{"x", "-y", "-o" + outputDir}
-	if password != "" {
+	if password == "" {
-		args = append(args, "-p"+password)
+		// `-p` with no value tells 7z the password is empty so encrypted
-	} else {
+		// archives fail fast instead of waiting for a prompt.
-		args = append(args, "-p") // empty password
+		args = append(args, "-p")
 	}
 	args = append(args, archivePath)
 
-	cmd := exec.Command(szPath, args...)
+	ctx, cancel := context.WithTimeout(context.Background(), extractTimeout)
+	defer cancel()
+	cmd := exec.CommandContext(ctx, szPath, args...)
 	cmd.Dir = outputDir
+	if password != "" {
+		cmd.Stdin = strings.NewReader(password + "\n")
+	}
 	output, err := cmd.CombinedOutput()
+	if ctx.Err() == context.DeadlineExceeded {
+		return nil, fmt.Errorf("7z: timed out after %s", extractTimeout)
+	}
 	if err != nil {
 		outStr := string(output)
 		if strings.Contains(outStr, "Wrong password") || strings.Contains(outStr, "incorrect password") {