fix(admin): harden console review findings

cmd/sower/proxy.go

@@ -106,6 +106,15 @@ func newTLSDialFn(dialer *net.Dialer, proxyHost string, tlsOptions upstreamtls.O
 	}, nil
 }
 
+// hostOnly strips the port from a host:port string, returning the input
+// unchanged when it has no port.
+func hostOnly(addr string) string {
+	if host, _, err := net.SplitHostPort(addr); err == nil {
+		return host
+	}
+	return addr
+}
+
 func upstreamDialAddr(addr, defaultPort string) (string, error) {
 	if host, port, err := net.SplitHostPort(addr); err == nil {
 		if host == "" || port == "" {
@@ -179,7 +188,7 @@ func handleHTTPConn(conn net.Conn, r *router.Router, stats *admin.Stats) {
 	}
 	_ = rereadConn.SetDeadline(time.Time{})
 
-	stats.BindConn(conn, req.Host)
+	stats.BindConn(conn, hostOnly(req.Host))
 	rc, err := r.DialProxyOnly("tcp", req.Host, 80)
 	if err != nil {
 		slog.Error("dial proxy", "error", err, "host", req.Host, "req", req.URL)

cmd/sower/proxy_test.go

@@ -57,6 +57,25 @@ func TestUpstreamDialAddrPreservesExplicitPort(t *testing.T) {
 	}
 }
 
+func TestHostOnly(t *testing.T) {
+	t.Parallel()
+
+	for _, test := range []struct {
+		addr string
+		want string
+	}{
+		{addr: "example.com:8080", want: "example.com"},
+		{addr: "example.com", want: "example.com"},
+		{addr: "[2001:db8::1]:443", want: "2001:db8::1"},
+	} {
+		t.Run(test.addr, func(t *testing.T) {
+			if got := hostOnly(test.addr); got != test.want {
+				t.Fatalf("hostOnly(%q) = %q, want %q", test.addr, got, test.want)
+			}
+		})
+	}
+}
+
 func TestHandleSocks5ConnReturnsForbiddenForBlockedConnect(t *testing.T) {
 	server, client := net.Pipe()
 	defer client.Close()

internal/admin/server.go

@@ -92,7 +92,10 @@ func NewServer(opts Options) *Server {
 	mux.HandleFunc("GET /api/traffic", s.mutateGuard(s.auth(s.handleTraffic)))
 	mux.HandleFunc("/", s.handleStatic)
 
-	s.http = &http.Server{Handler: mux}
+	s.http = &http.Server{
+		Handler:           mux,
+		ReadHeaderTimeout: 10 * time.Second,
+	}
 	return s
 }
 
@@ -123,7 +126,9 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
 		writeError(w, http.StatusUnauthorized, "invalid password")
 		return
 	}
-	s.issueSession(w, r)
+	if !s.issueSession(w, r) {
+		return
+	}
 	w.WriteHeader(http.StatusNoContent)
 }
 
@@ -152,12 +157,12 @@ func (s *Server) sessionCookie(value string, maxAge int, secure bool) *http.Cook
 	}
 }
 
-func (s *Server) issueSession(w http.ResponseWriter, r *http.Request) {
+func (s *Server) issueSession(w http.ResponseWriter, r *http.Request) bool {
 	var token [32]byte
 	if _, err := rand.Read(token[:]); err != nil {
 		slog.Error("generate session token", "error", err)
 		writeError(w, http.StatusInternalServerError, "failed to issue session")
-		return
+		return false
 	}
 	value := hex.EncodeToString(token[:])
 
@@ -169,6 +174,7 @@ func (s *Server) issueSession(w http.ResponseWriter, r *http.Request) {
 	s.mu.Unlock()
 
 	http.SetCookie(w, s.sessionCookie(value, int(sessionTTL.Seconds()), r.TLS != nil))
+	return true
 }
 
 func (s *Server) purgeExpiredLocked() {
@@ -296,12 +302,22 @@ func (s *Server) handleRulesRemove(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) handleTraffic(w http.ResponseWriter, r *http.Request) {
+	if s.opts.Stats == nil {
+		writeError(w, http.StatusInternalServerError, "stats unavailable")
+		return
+	}
 	writeJSON(w, http.StatusOK, s.opts.Stats.Snapshot())
 }
 
 // --- static frontend ---
 
 func (s *Server) handleStatic(w http.ResponseWriter, r *http.Request) {
+	// Unknown API paths must not fall through to the SPA shell.
+	if r.URL.Path == "/api" || strings.HasPrefix(r.URL.Path, "/api/") {
+		writeError(w, http.StatusNotFound, "unknown API endpoint")
+		return
+	}
+
 	fsys, err := web.Dist()
 	if err != nil {
 		slog.Error("load embedded web dist", "error", err)
@@ -315,6 +331,10 @@ func (s *Server) handleStatic(w http.ResponseWriter, r *http.Request) {
 	if p == "" {
 		p = "index.html"
 	}
+	if !fs.ValidPath(p) {
+		writeError(w, http.StatusNotFound, "not found")
+		return
+	}
 	if strings.HasPrefix(p, "assets/") {
 		w.Header().Set("Cache-Control", "public, max-age=31536000, immutable")
 	} else {
@@ -399,7 +419,9 @@ func decodeJSON(w http.ResponseWriter, r *http.Request, dst any) bool {
 func writeJSON(w http.ResponseWriter, status int, v any) {
 	w.Header().Set("Content-Type", "application/json; charset=utf-8")
 	w.WriteHeader(status)
-	_ = json.NewEncoder(w).Encode(v)
+	if err := json.NewEncoder(w).Encode(v); err != nil {
+		slog.Debug("write JSON response", "error", err)
+	}
 }
 
 func writeError(w http.ResponseWriter, status int, msg string) {

internal/admin/server_test.go

@@ -291,7 +291,16 @@ func TestLogoutInvalidatesSession(t *testing.T) {
 	if resp.StatusCode != http.StatusNoContent {
 		t.Fatalf("logout status: %d", resp.StatusCode)
 	}
+	cookieExpired := false
+	for _, c := range resp.Cookies() {
+		if c.Name == sessionCookieName && c.MaxAge < 0 {
+			cookieExpired = true
+		}
+	}
 	resp.Body.Close()
+	if !cookieExpired {
+		t.Fatal("logout did not expire the session cookie")
+	}
 
 	resp = authedRequest(t, ts, http.MethodGet, "/api/status", cookie, "")
 	if resp.StatusCode != http.StatusUnauthorized {
@@ -323,6 +332,22 @@ func TestTrafficEndpoint(t *testing.T) {
 	}
 }
 
+func TestTrafficEndpointRejectsMissingStats(t *testing.T) {
+	s := NewServer(Options{Password: "secret", Rules: newFakeRules()})
+	ts := httptest.NewServer(s.http.Handler)
+	t.Cleanup(ts.Close)
+	cookie := login(t, ts, "secret")
+
+	resp := authedRequest(t, ts, http.MethodGet, "/api/traffic", cookie, "")
+	if resp.StatusCode != http.StatusInternalServerError {
+		t.Fatalf("expected 500 when stats are unavailable, got %d", resp.StatusCode)
+	}
+	body := decodeBody(t, resp)
+	if body["error"] != "stats unavailable" {
+		t.Fatalf("unexpected error: %v", body)
+	}
+}
+
 func TestStaticServesIndexAndSPAFallback(t *testing.T) {
 	ts := newTestServer(t, newFakeRules())
 
@@ -346,6 +371,28 @@ func TestStaticServesIndexAndSPAFallback(t *testing.T) {
 	resp.Body.Close()
 }
 
+func TestUnknownAPIPathReturnsJSONNotFound(t *testing.T) {
+	ts := newTestServer(t, newFakeRules())
+	for _, path := range []string{"/api", "/api/missing"} {
+		resp, err := http.Get(ts.URL + path)
+		if err != nil {
+			t.Fatalf("get %s: %v", path, err)
+		}
+		if resp.StatusCode != http.StatusNotFound {
+			resp.Body.Close()
+			t.Fatalf("expected 404 for %s, got %d", path, resp.StatusCode)
+		}
+		if !strings.HasPrefix(resp.Header.Get("Content-Type"), "application/json") {
+			resp.Body.Close()
+			t.Fatalf("expected JSON response for %s, got %q", path, resp.Header.Get("Content-Type"))
+		}
+		body := decodeBody(t, resp)
+		if body["error"] != "unknown API endpoint" {
+			t.Fatalf("unexpected error for %s: %v", path, body)
+		}
+	}
+}
+
 func TestShutdown(t *testing.T) {
 	s := NewServer(Options{Password: "secret", Rules: newFakeRules(), Stats: NewStats()})
 	ts := httptest.NewServer(s.http.Handler)

internal/admin/stats.go

@@ -106,7 +106,7 @@ func (s *Stats) BindConn(conn net.Conn, domain string) {
 	if domain == "" {
 		return
 	}
-	cc.bind(s, domain)
+	cc.bind(domain)
 	s.record(domain, func(d *domainStat) { d.conns++ })
 }
 
@@ -209,8 +209,7 @@ func (c *countingConn) Write(p []byte) (int, error) {
 	return n, err
 }
 
-func (c *countingConn) bind(stats *Stats, domain string) {
-	c.stats = stats
+func (c *countingConn) bind(domain string) {
 	c.domain.Store(domain)
 }
 

internal/admin/stats_test.go

@@ -1,6 +1,7 @@
 package admin
 
 import (
+	"fmt"
 	"net"
 	"testing"
 	"time"
@@ -130,22 +131,36 @@ func TestStatsSnapshotEvictsStaleDomains(t *testing.T) {
 	}
 }
 
-func TestStatsDomainCapEvictsOldest(t *testing.T) {
+func TestStatsDomainCapIsEnforced(t *testing.T) {
 	s := NewStats()
 	for i := 0; i < maxDomainEntries+50; i++ {
-		s.record("host.example", func(d *domainStat) {
-			d.lastSeen = time.Now().Add(-time.Duration(i) * time.Minute)
-		})
+		s.record(fmt.Sprintf("host%d.example", i), func(d *domainStat) {})
 	}
-	snap := s.Snapshot()
-	if len(snap.Domains) > maxDomainEntries {
-		t.Fatalf("expected domain cap enforced, got %d entries", len(snap.Domains))
+	s.mu.Lock()
+	n := len(s.domains)
+	s.mu.Unlock()
+	if n > maxDomainEntries {
+		t.Fatalf("expected domain cap enforced, got %d entries", n)
 	}
-	if len(snap.Domains) == 0 {
+	if n == 0 {
 		t.Fatal("expected some domains to survive")
 	}
 }
 
+func TestEvictOldestLocked(t *testing.T) {
+	s := NewStats()
+	s.mu.Lock()
+	s.domains["old.example"] = &domainStat{lastSeen: time.Now().Add(-time.Hour)}
+	s.domains["new.example"] = &domainStat{lastSeen: time.Now()}
+	s.evictOldestLocked()
+	_, oldOK := s.domains["old.example"]
+	_, newOK := s.domains["new.example"]
+	s.mu.Unlock()
+	if oldOK || !newOK {
+		t.Fatalf("expected oldest evicted, old=%v new=%v", oldOK, newOK)
+	}
+}
+
 func TestStatsSnapshotSortsByTotalBytes(t *testing.T) {
 	s := NewStats()
 	s.record("small.example", func(d *domainStat) { d.bytesUp, d.bytesDown = 1, 1 })

web/dist/index.html

@@ -5,7 +5,7 @@
     <meta name="viewport" content="width=device-width, initial-scale=1.0" />
     <meta name="color-scheme" content="dark" />
     <title>Sower Admin</title>
-    <script type="module" crossorigin src="/assets/index-CZfLD4S6.js"></script>
+    <script type="module" crossorigin src="/assets/index-Ds2OrZNy.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-DBmMlH5L.css">
   </head>
   <body>

web/src/App.svelte

@@ -71,8 +71,20 @@
     }
   }
 
+  async function handleLogout() {
+    try {
+      await api.logout()
+    } catch {
+      // Local auth state must reset even if the session revoke request fails.
+    } finally {
+      onUnauthorized()
+    }
+  }
+
   function onUnauthorized() {
     authed = false
+    status = null
+    statusError = ''
     if (statusTimer) {
       clearInterval(statusTimer)
       statusTimer = undefined
@@ -96,7 +108,7 @@
     onSelectItem={(key) => (activeNav = key)}
     {status}
     {statusError}
-    onLogout={onUnauthorized}
+    onLogout={handleLogout}
   />
   <main>
     {#if activeNav === 'overview'}

web/src/views/Login.svelte

@@ -5,7 +5,7 @@
   import Input from '$lib/components/ui/input/input.svelte'
   import Label from '$lib/components/ui/label/label.svelte'
   import Button from '$lib/components/ui/button/button.svelte'
-  import { AlertCircle } from 'lucide-svelte'
+  import { CircleAlert } from 'lucide-svelte'
 
   let { onLogin }: { onLogin: () => void } = $props()
 
@@ -49,7 +49,7 @@
         </div>
         {#if error}
           <Alert.Alert variant="destructive">
-            <AlertCircle class="size-4" />
+            <CircleAlert class="size-4" />
             <Alert.AlertDescription>{error}</Alert.AlertDescription>
           </Alert.Alert>
         {/if}

web/src/views/Overview.svelte

@@ -5,7 +5,7 @@
   import * as Card from '$lib/components/ui/card'
   import * as Alert from '$lib/components/ui/alert'
   import Badge from '$lib/components/ui/badge/badge.svelte'
-  import { AlertCircle } from 'lucide-svelte'
+  import { CircleAlert } from 'lucide-svelte'
 
   let { onUnauthorized }: { onUnauthorized: () => void } = $props()
 
@@ -37,7 +37,7 @@
 
 {#if error}
   <Alert.Alert variant="destructive">
-    <AlertCircle class="size-4" />
+    <CircleAlert class="size-4" />
     <Alert.AlertDescription>{error}</Alert.AlertDescription>
   </Alert.Alert>
 {:else if !status || !traffic}

web/src/views/Rules.svelte

@@ -6,7 +6,7 @@
   import Badge from '$lib/components/ui/badge/badge.svelte'
   import Button from '$lib/components/ui/button/button.svelte'
   import Textarea from '$lib/components/ui/textarea/textarea.svelte'
-  import { AlertCircle } from 'lucide-svelte'
+  import { CircleAlert } from 'lucide-svelte'
 
   let { category, onUnauthorized }: { category: Category; onUnauthorized: () => void } = $props()
 
@@ -14,13 +14,22 @@
   let draft = $state('')
   let error = $state('')
   let busy = $state(false)
+  let requestID = 0
+
+  function requestIsCurrent(id: number, requestedCategory: Category) {
+    return id === requestID && requestedCategory === category
+  }
 
   async function load() {
+    const id = ++requestID
+    const requestedCategory = category
     try {
-      const resp: RulesResponse = await api.rules(category)
+      const resp: RulesResponse = await api.rules(requestedCategory)
+      if (!requestIsCurrent(id, requestedCategory)) return
       rules = resp.rules
       error = ''
     } catch (e) {
+      if (!requestIsCurrent(id, requestedCategory)) return
       if (e instanceof ApiError && e.status === 401) {
         onUnauthorized()
         return
@@ -35,38 +44,52 @@
       .map((s) => s.trim())
       .filter((s) => s.length > 0)
     if (items.length === 0 || busy) return
+
+    const id = ++requestID
+    const requestedCategory = category
     busy = true
     error = ''
     try {
-      const resp = await api.addRules(category, items)
+      const resp = await api.addRules(requestedCategory, items)
+      if (!requestIsCurrent(id, requestedCategory)) return
       rules = resp.rules
       draft = ''
     } catch (e) {
+      if (!requestIsCurrent(id, requestedCategory)) return
       if (e instanceof ApiError && e.status === 401) {
         onUnauthorized()
         return
       }
       error = e instanceof Error ? e.message : 'add failed'
     } finally {
-      busy = false
+      if (requestIsCurrent(id, requestedCategory)) {
+        busy = false
+      }
     }
   }
 
   async function removeRule(rule: string) {
     if (busy) return
+
+    const id = ++requestID
+    const requestedCategory = category
     busy = true
     error = ''
     try {
-      const resp = await api.removeRules(category, [rule])
+      const resp = await api.removeRules(requestedCategory, [rule])
+      if (!requestIsCurrent(id, requestedCategory)) return
       rules = resp.rules
     } catch (e) {
+      if (!requestIsCurrent(id, requestedCategory)) return
       if (e instanceof ApiError && e.status === 401) {
         onUnauthorized()
         return
       }
       error = e instanceof Error ? e.message : 'remove failed'
     } finally {
-      busy = false
+      if (requestIsCurrent(id, requestedCategory)) {
+        busy = false
+      }
     }
   }
 
@@ -74,13 +97,15 @@
   $effect(() => {
     category
     rules = null
+    error = ''
+    busy = false
     void load()
   })
 </script>
 
 {#if error}
   <Alert.Alert variant="destructive" class="mb-4">
-    <AlertCircle class="size-4" />
+    <CircleAlert class="size-4" />
     <Alert.AlertDescription>{error}</Alert.AlertDescription>
   </Alert.Alert>
 {/if}

web/src/views/Traffic.svelte

@@ -6,7 +6,7 @@
   import * as Card from '$lib/components/ui/card'
   import * as Table from '$lib/components/ui/table'
   import Badge from '$lib/components/ui/badge/badge.svelte'
-  import { AlertCircle } from 'lucide-svelte'
+  import { CircleAlert } from 'lucide-svelte'
 
   let { onUnauthorized }: { onUnauthorized: () => void } = $props()
 
@@ -35,7 +35,7 @@
 
 {#if error}
   <Alert.Alert variant="destructive" class="mb-4">
-    <AlertCircle class="size-4" />
+    <CircleAlert class="size-4" />
     <Alert.AlertDescription>{error}</Alert.AlertDescription>
   </Alert.Alert>
 {:else if !traffic}