Complete Phase 4: security hardening, player/playlist/admin UI, docs

Security: - New internal/netguard package: podcast feed/enclosure fetching now refuses to dial non-public IPs (loopback/RFC1918/link-local/cloud metadata/CGNAT), validated per-connection including across redirects, closing an SSRF gap where a malicious or compromised feed URL could probe the server's internal network. Configurable via VIBESONIC_PODCASTS_ALLOW_PRIVATE_NETWORKS for intentional LAN feeds. - Podcast episode downloads capped at 4GB (was unbounded, disk-exhaustion risk) - Cover art decoding now rejects images above a 40-megapixel budget before full decode (decompression-bomb guard) - HTTP server sets ReadHeaderTimeout (slowloris hardening) - Admin folder-scan redirect now validated as same-site instead of trusting the raw Referer header (open-redirect fix) - Legacy Subsonic token/salt auth now uses a constant-time comparison Web UI completion: - Persistent player: shuffle and repeat (off/all/one) controls, wired into queue advance/prev logic and persisted like other player state - Admin user-permissions form: adminRole toggle (with a guard against demoting the last remaining admin) and per-user music-folder access checkboxes, backed by existing schema/enforcement that was previously unreachable from the UI - Playlist M3U8 import/export Docs: added docs/client-compatibility.md (honest tracking template — no client testing has been performed), docs/deployment.md, docs/security.md, docs/backup.md; refreshed README and cross-links; added a footnote in docs/subsonic-compatibility.md distinguishing spec-standard from VibeSonic-original OpenSubsonic extensions. Also added Go fuzz tests for the external-auth CIDR/IP parser and the LRC lyrics parser. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014r98AtcBbevpscWLsfwVvv
2026-08-23 09:18:17 -05:00
parent 933ed31550
commit 789bf91eed
36 changed files with 1722 additions and 96 deletions
@@ -0,0 +1,147 @@
+// Package netguard builds HTTP clients that refuse to connect to
+// non-public IP addresses. It exists so features that fetch
+// externally-controlled URLs (podcast feeds and enclosures, most
+// notably) cannot be turned into a proxy onto the server's own
+// loopback interface, its LAN, or cloud-metadata endpoints
+// (169.254.169.254 and friends) via a malicious or compromised feed
+// URL, a redirect, or DNS rebinding.
+//
+// The check is enforced at dial time, on the specific IP address about
+// to be connected to, for every connection the client makes — including
+// ones made while following a redirect. Because the resolved IP is
+// validated immediately before dialing that exact address (rather than
+// re-resolving the hostname later), a DNS record that changes between
+// requests cannot rebind a validated hostname onto a blocked address.
+package netguard
+
+import (
+	"context"
+	"fmt"
+	"net"
+	"net/http"
+	"net/url"
+	"time"
+)
+
+// AllowedSchemes are the only URL schemes a guarded client will ever
+// dial; anything else (file://, ftp://, gopher://, ...) is rejected
+// before a connection is attempted.
+var AllowedSchemes = map[string]bool{"http": true, "https": true}
+
+// ValidateURL rejects an obviously-bad externally-supplied URL (wrong
+// scheme, missing host) before it's stored or fetched. This is a
+// fast, user-facing sanity check; the actual SSRF protection is
+// enforced at connection time by the client NewSafeHTTPClient returns,
+// regardless of what passes this check.
+func ValidateURL(rawURL string) error {
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		return fmt.Errorf("invalid URL: %w", err)
+	}
+	if !AllowedSchemes[u.Scheme] {
+		return fmt.Errorf("URL must be http or https")
+	}
+	if u.Host == "" {
+		return fmt.Errorf("URL must include a host")
+	}
+	return nil
+}
+
+// NewSafeHTTPClient returns an *http.Client that only connects to
+// public IP addresses, unless allowPrivateNetworks is true (for
+// deployments that intentionally point podcast feeds at a LAN/self-
+// hosted server). It also refuses to follow a redirect more than 5
+// times or onto a non-http(s) scheme.
+func NewSafeHTTPClient(timeout time.Duration, allowPrivateNetworks bool) *http.Client {
+	dialer := &net.Dialer{Timeout: 10 * time.Second}
+	transport := &http.Transport{
+		DialContext:           safeDialContext(dialer, allowPrivateNetworks),
+		TLSHandshakeTimeout:   10 * time.Second,
+		ResponseHeaderTimeout: 20 * time.Second,
+		ForceAttemptHTTP2:     true,
+	}
+	return &http.Client{
+		Timeout:   timeout,
+		Transport: transport,
+		CheckRedirect: func(req *http.Request, via []*http.Request) error {
+			if len(via) >= 5 {
+				return fmt.Errorf("stopped after 5 redirects")
+			}
+			if !AllowedSchemes[req.URL.Scheme] {
+				return fmt.Errorf("refusing to follow redirect to unsupported scheme %q", req.URL.Scheme)
+			}
+			return nil
+		},
+	}
+}
+
+func safeDialContext(base *net.Dialer, allowPrivateNetworks bool) func(ctx context.Context, network, addr string) (net.Conn, error) {
+	return func(ctx context.Context, network, addr string) (net.Conn, error) {
+		host, port, err := net.SplitHostPort(addr)
+		if err != nil {
+			return nil, err
+		}
+
+		// If the host is already a literal IP, LookupIP still handles
+		// it (returning it unchanged) without a real DNS round trip.
+		ips, err := net.DefaultResolver.LookupIP(ctx, "ip", host)
+		if err != nil {
+			return nil, err
+		}
+
+		var lastErr error
+		for _, ip := range ips {
+			if !allowPrivateNetworks && !IsPublicIP(ip) {
+				lastErr = fmt.Errorf("refusing to connect to non-public address %s", ip)
+				continue
+			}
+			conn, dialErr := base.DialContext(ctx, network, net.JoinHostPort(ip.String(), port))
+			if dialErr == nil {
+				return conn, nil
+			}
+			lastErr = dialErr
+		}
+		if lastErr == nil {
+			lastErr = fmt.Errorf("no addresses found for %s", host)
+		}
+		return nil, lastErr
+	}
+}
+
+// IsPublicIP reports whether ip is a globally routable unicast address
+// that a server-side fetcher should be allowed to connect to. It
+// rejects loopback, link-local (including the 169.254.169.254 cloud
+// metadata address), RFC1918/RFC4193 private ranges, carrier-grade NAT
+// (100.64.0.0/10), multicast, and unspecified addresses.
+func IsPublicIP(ip net.IP) bool {
+	ip = ip.To16()
+	if ip == nil {
+		return false
+	}
+	if v4 := ip.To4(); v4 != nil {
+		if cgnat.Contains(v4) {
+			return false
+		}
+	}
+	switch {
+	case ip.IsLoopback(),
+		ip.IsPrivate(),
+		ip.IsLinkLocalUnicast(),
+		ip.IsLinkLocalMulticast(),
+		ip.IsInterfaceLocalMulticast(),
+		ip.IsMulticast(),
+		ip.IsUnspecified():
+		return false
+	}
+	return true
+}
+
+var cgnat = mustParseCIDR("100.64.0.0/10")
+
+func mustParseCIDR(s string) *net.IPNet {
+	_, n, err := net.ParseCIDR(s)
+	if err != nil {
+		panic(err)
+	}
+	return n
+}
@@ -0,0 +1,53 @@
+package netguard
+
+import (
+	"net"
+	"testing"
+)
+
+// FuzzValidateURL guards against a crash in the podcast feed/enclosure
+// URL sanity check — the URLs it validates are admin-supplied but the
+// check must never panic regardless of input.
+func FuzzValidateURL(f *testing.F) {
+	f.Add("https://example.com/feed.xml")
+	f.Add("http://127.0.0.1/feed")
+	f.Add("ftp://example.com")
+	f.Add("not a url at all")
+	f.Add("")
+	f.Add("http://")
+	f.Add("javascript:alert(1)")
+	f.Add("http://[::1]:8080/feed")
+
+	f.Fuzz(func(t *testing.T, raw string) {
+		defer func() {
+			if r := recover(); r != nil {
+				t.Fatalf("ValidateURL panicked on %q: %v", raw, r)
+			}
+		}()
+		_ = ValidateURL(raw)
+	})
+}
+
+// FuzzIsPublicIP guards the address classifier that decides whether a
+// resolved IP is safe to connect to.
+func FuzzIsPublicIP(f *testing.F) {
+	f.Add("8.8.8.8")
+	f.Add("127.0.0.1")
+	f.Add("::1")
+	f.Add("169.254.169.254")
+	f.Add("100.64.0.1")
+	f.Add("::ffff:127.0.0.1")
+
+	f.Fuzz(func(t *testing.T, s string) {
+		ip := net.ParseIP(s)
+		if ip == nil {
+			return
+		}
+		defer func() {
+			if r := recover(); r != nil {
+				t.Fatalf("IsPublicIP panicked on %q: %v", s, r)
+			}
+		}()
+		IsPublicIP(ip)
+	})
+}
@@ -0,0 +1,85 @@
+package netguard
+
+import (
+	"context"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+)
+
+func TestIsPublicIP(t *testing.T) {
+	cases := map[string]bool{
+		"8.8.8.8":         true,
+		"1.1.1.1":         true,
+		"127.0.0.1":       false,
+		"10.0.0.1":        false,
+		"172.16.0.1":      false,
+		"192.168.1.1":     false,
+		"169.254.169.254": false, // cloud metadata
+		"100.64.0.1":      false, // CGNAT
+		"0.0.0.0":         false,
+		"::1":             false,
+		"fe80::1":         false,
+		"fc00::1":         false,
+		"2606:4700::1111": true,
+	}
+	for s, want := range cases {
+		ip := net.ParseIP(s)
+		if ip == nil {
+			t.Fatalf("bad test IP %q", s)
+		}
+		if got := IsPublicIP(ip); got != want {
+			t.Errorf("IsPublicIP(%q) = %v, want %v", s, got, want)
+		}
+	}
+}
+
+func TestSafeHTTPClientBlocksLoopback(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte("ok"))
+	}))
+	defer srv.Close()
+
+	client := NewSafeHTTPClient(5*time.Second, false)
+	_, err := client.Get(srv.URL)
+	if err == nil {
+		t.Fatal("expected request to loopback server to be blocked")
+	}
+}
+
+func TestSafeHTTPClientAllowsLoopbackWhenPermitted(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte("ok"))
+	}))
+	defer srv.Close()
+
+	client := NewSafeHTTPClient(5*time.Second, true)
+	resp, err := client.Get(srv.URL)
+	if err != nil {
+		t.Fatalf("expected request to loopback server to succeed: %v", err)
+	}
+	resp.Body.Close()
+}
+
+func TestSafeHTTPClientBlocksRedirectToPrivate(t *testing.T) {
+	blocked := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte("should never be reached"))
+	}))
+	defer blocked.Close()
+
+	// A "public-looking" server (still loopback in this test, but with
+	// AllowPrivateNetworks=false the initial dial to it would itself be
+	// blocked — so instead verify redirect scheme enforcement directly
+	// via CheckRedirect using a context that never dials at all).
+	client := NewSafeHTTPClient(5*time.Second, false)
+	req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, blocked.URL, nil)
+	if err := client.CheckRedirect(req, nil); err != nil {
+		t.Fatalf("unexpected error for allowed scheme: %v", err)
+	}
+	req.URL.Scheme = "file"
+	if err := client.CheckRedirect(req, nil); err == nil {
+		t.Fatal("expected file:// redirect to be rejected")
+	}
+}