fix(security): replace timing-unsafe token comparisons with hmac.Equal (#426)

Bearer and admin token comparisons used plain == / != operators, which are
not constant-time and leak timing information via early-exit string comparison.
Replace all four comparison sites with hmac.Equal([]byte(a), []byte(b)) as
required by issue #350. Add correctness guard tests for each site to ensure
accept/reject behavior is preserved.

Author: Alan-TheGentleman

internal/cloud/auth/auth.go

@@ -250,7 +250,7 @@ func (s *Service) Authorize(r *http.Request) error {
 	if token == "" {
 		return fmt.Errorf("bearer token is required")
 	}
-	if token != s.expectedToken {
+	if !hmac.Equal([]byte(token), []byte(s.expectedToken)) {
 		return fmt.Errorf("invalid bearer token")
 	}
 	return nil

internal/cloud/auth/auth_test.go

@@ -252,6 +252,44 @@ func TestDashboardSessionTokenRejectsWhenConfiguredBearerChanges(t *testing.T) {
 	}
 }
 
+// TestAuthorizeBearerTokenConstantTimeComparison is a correctness guard for the
+// constant-time token comparison at auth.go:253. A correct token must be accepted
+// and any wrong token must be rejected, preserving behavior after the
+// timing-safe hmac.Equal replacement (security issue #350).
+func TestAuthorizeBearerTokenConstantTimeComparison(t *testing.T) {
+	svc, err := NewService(&cloudstore.CloudStore{}, strings.Repeat("x", 32))
+	if err != nil {
+		t.Fatalf("new service: %v", err)
+	}
+	svc.SetBearerToken("correct-token")
+
+	correct := httptest.NewRequest("GET", "/sync/pull", nil)
+	correct.Header.Set("Authorization", "Bearer correct-token")
+	if err := svc.Authorize(correct); err != nil {
+		t.Fatalf("correct token must be accepted, got %v", err)
+	}
+
+	wrong := httptest.NewRequest("GET", "/sync/pull", nil)
+	wrong.Header.Set("Authorization", "Bearer wrong-token")
+	if err := svc.Authorize(wrong); err == nil {
+		t.Fatal("wrong token must be rejected")
+	}
+
+	// A token that is a prefix of the correct one must also be rejected.
+	prefix := httptest.NewRequest("GET", "/sync/pull", nil)
+	prefix.Header.Set("Authorization", "Bearer correct-toke")
+	if err := svc.Authorize(prefix); err == nil {
+		t.Fatal("prefix of correct token must be rejected")
+	}
+
+	// A token that is a superset of the correct one must also be rejected.
+	super := httptest.NewRequest("GET", "/sync/pull", nil)
+	super.Header.Set("Authorization", "Bearer correct-token-extra")
+	if err := svc.Authorize(super); err == nil {
+		t.Fatal("superset of correct token must be rejected")
+	}
+}
+
 func TestDashboardSessionTokenSupportsAdditionalDashboardCredential(t *testing.T) {
 	svc, err := NewService(&cloudstore.CloudStore{}, strings.Repeat("x", 32))
 	if err != nil {

internal/cloud/cloudserver/cloudserver.go

@@ -2,6 +2,7 @@ package cloudserver
 
 import (
 	"context"
+	"crypto/hmac"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -157,7 +158,7 @@ func (s *CloudServer) routes() {
 		if token == "" {
 			return fmt.Errorf("bearer token is required")
 		}
-		if adminToken := strings.TrimSpace(s.dashboardAdmin); adminToken != "" && token == adminToken {
+		if adminToken := strings.TrimSpace(s.dashboardAdmin); adminToken != "" && hmac.Equal([]byte(token), []byte(adminToken)) {
 			return nil
 		}
 		if s.auth == nil {
@@ -258,7 +259,7 @@ func (s *CloudServer) authorizeDashboardRequest(r *http.Request) error {
 	if strings.TrimSpace(bearerToken) == "" {
 		return fmt.Errorf("dashboard session token is empty")
 	}
-	if adminToken := strings.TrimSpace(s.dashboardAdmin); adminToken != "" && bearerToken == adminToken {
+	if adminToken := strings.TrimSpace(s.dashboardAdmin); adminToken != "" && hmac.Equal([]byte(bearerToken), []byte(adminToken)) {
 		return nil
 	}
 	req, _ := http.NewRequest(http.MethodGet, "/dashboard", nil)
@@ -317,7 +318,7 @@ func (s *CloudServer) isDashboardAdmin(r *http.Request) bool {
 	if err != nil {
 		return false
 	}
-	return token == adminToken
+	return hmac.Equal([]byte(token), []byte(adminToken))
 }
 
 func (s *CloudServer) handlePullManifest(w http.ResponseWriter, r *http.Request) {

internal/cloud/cloudserver/cloudserver_test.go

@@ -1497,6 +1497,105 @@ func TestAuditLogE2E_MutationPushPausedThenListRendered(t *testing.T) {
 	}
 }
 
+// TestValidateLoginTokenAdminComparisonGuard is a correctness guard for the
+// constant-time admin token comparison inside validateLoginToken (cloudserver.go:160).
+// A correct admin token must be accepted and a wrong one rejected, preserving
+// behavior after the timing-safe hmac.Equal replacement (security issue #350).
+func TestValidateLoginTokenAdminComparisonGuard(t *testing.T) {
+	authSvc, err := cloudauth.NewService(&cloudstore.CloudStore{}, strings.Repeat("x", 32))
+	if err != nil {
+		t.Fatalf("new auth service: %v", err)
+	}
+	authSvc.SetBearerToken("sync-token")
+	authSvc.SetDashboardSessionTokens([]string{"admin-token"})
+	srv := New(&fakeStore{}, authSvc, 0, WithDashboardAdminToken("admin-token"))
+
+	// Correct admin token must authenticate and redirect to dashboard.
+	goodLogin := httptest.NewRecorder()
+	goodReq := httptest.NewRequest(http.MethodPost, "/dashboard/login", strings.NewReader("token=admin-token"))
+	goodReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	srv.Handler().ServeHTTP(goodLogin, goodReq)
+	if goodLogin.Code != http.StatusSeeOther {
+		t.Fatalf("correct admin token must authenticate, got %d body=%q", goodLogin.Code, goodLogin.Body.String())
+	}
+
+	// Wrong token must be rejected (re-render login form).
+	badLogin := httptest.NewRecorder()
+	badReq := httptest.NewRequest(http.MethodPost, "/dashboard/login", strings.NewReader("token=wrong-token"))
+	badReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	srv.Handler().ServeHTTP(badLogin, badReq)
+	if badLogin.Code == http.StatusSeeOther {
+		t.Fatalf("wrong admin token must be rejected, got %d", badLogin.Code)
+	}
+	for _, cookie := range badLogin.Result().Cookies() {
+		if cookie.Name == dashboardSessionCookieName {
+			t.Fatal("wrong admin token must not set session cookie")
+		}
+	}
+}
+
+// TestIsDashboardAdminComparisonGuard is a correctness guard for the
+// constant-time admin token comparison inside isDashboardAdmin (cloudserver.go:320).
+// A valid admin session cookie must grant admin access and a wrong one must not,
+// preserving behavior after the timing-safe hmac.Equal replacement (security issue #350).
+func TestIsDashboardAdminComparisonGuard(t *testing.T) {
+	authSvc, err := cloudauth.NewService(&cloudstore.CloudStore{}, strings.Repeat("x", 32))
+	if err != nil {
+		t.Fatalf("new auth service: %v", err)
+	}
+	authSvc.SetBearerToken("sync-token")
+	authSvc.SetDashboardSessionTokens([]string{"admin-token"})
+	srv := New(&fakeStore{}, authSvc, 0, WithDashboardAdminToken("admin-token"))
+
+	// Establish a valid admin session cookie via login.
+	loginRec := httptest.NewRecorder()
+	loginReq := httptest.NewRequest(http.MethodPost, "/dashboard/login", strings.NewReader("token=admin-token"))
+	loginReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	srv.Handler().ServeHTTP(loginRec, loginReq)
+	if loginRec.Code != http.StatusSeeOther {
+		t.Fatalf("setup: expected admin login to succeed, got %d body=%q", loginRec.Code, loginRec.Body.String())
+	}
+	adminCookies := loginRec.Result().Cookies()
+
+	// Admin session must grant access to /dashboard/admin.
+	adminRec := httptest.NewRecorder()
+	adminReq := httptest.NewRequest(http.MethodGet, "/dashboard/admin", nil)
+	for _, c := range adminCookies {
+		adminReq.AddCookie(c)
+	}
+	srv.Handler().ServeHTTP(adminRec, adminReq)
+	if adminRec.Code != http.StatusOK {
+		t.Fatalf("valid admin session must access /dashboard/admin, got %d body=%q", adminRec.Code, adminRec.Body.String())
+	}
+
+	// Non-admin session (sync-token login) must not access /dashboard/admin.
+	authSvc2, err := cloudauth.NewService(&cloudstore.CloudStore{}, strings.Repeat("x", 32))
+	if err != nil {
+		t.Fatalf("new auth service 2: %v", err)
+	}
+	authSvc2.SetBearerToken("sync-token")
+	authSvc2.SetDashboardSessionTokens([]string{"admin-token"})
+	srv2 := New(&fakeStore{}, authSvc2, 0, WithDashboardAdminToken("admin-token"))
+
+	syncLoginRec := httptest.NewRecorder()
+	syncLoginReq := httptest.NewRequest(http.MethodPost, "/dashboard/login", strings.NewReader("token=sync-token"))
+	syncLoginReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	srv2.Handler().ServeHTTP(syncLoginRec, syncLoginReq)
+	if syncLoginRec.Code != http.StatusSeeOther {
+		t.Fatalf("setup: sync-token login must succeed, got %d body=%q", syncLoginRec.Code, syncLoginRec.Body.String())
+	}
+
+	nonAdminRec := httptest.NewRecorder()
+	nonAdminReq := httptest.NewRequest(http.MethodGet, "/dashboard/admin", nil)
+	for _, c := range syncLoginRec.Result().Cookies() {
+		nonAdminReq.AddCookie(c)
+	}
+	srv2.Handler().ServeHTTP(nonAdminRec, nonAdminReq)
+	if nonAdminRec.Code == http.StatusOK {
+		t.Fatalf("non-admin session must not access /dashboard/admin, got %d", nonAdminRec.Code)
+	}
+}
+
 // TestInsecureModeLoginRedirects asserts that GET /dashboard/login with auth==nil
 // returns 303 to /dashboard/ (login is a no-op in insecure mode). Satisfies REQ-110.
 func TestInsecureModeLoginRedirects(t *testing.T) {