cubeops: propagate database errors from the settings and user getters

CubeOps/internal/auth/handler.go

@@ -64,7 +64,7 @@ func (h *Handler) Login(c *gin.Context) {
 			return
 		}
 		logging.G(c.Request.Context()).Errorf("login failed: internal error: username=%q client_ip=%s error=%q", req.Username, c.ClientIP(), err.Error())
-		httputil.WriteError(c, http.StatusInternalServerError, err.Error())
+		httputil.WriteError(c, http.StatusInternalServerError, "internal server error")
 		return
 	}
 	httputil.WriteJSON(c, http.StatusOK, model.LoginResponse{
@@ -120,7 +120,7 @@ func (h *Handler) ChangePassword(c *gin.Context) {
 		logging.G(c.Request.Context()).Errorf("password change failed: operator=%q result=error error=%q", username, err.Error())
 		// Validation errors and DB errors share the 500 path here; finer
 		// mapping can be added by wrapping with sentinel errors if needed.
-		httputil.WriteError(c, http.StatusInternalServerError, err.Error())
+		httputil.WriteError(c, http.StatusInternalServerError, "internal server error")
 	}
 }
 

CubeOps/internal/auth/handler_error_leak_test.go

@@ -0,0 +1,78 @@
+// Copyright (c) 2026 Tencent Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package auth_test
+
+import (
+	"context"
+	"errors"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/gin-gonic/gin"
+	"github.com/tencentcloud/CubeSandbox/CubeOps/internal/auth"
+	"github.com/tencentcloud/CubeSandbox/CubeOps/internal/service"
+)
+
+const dbErrorDetail = "dial tcp 10.0.0.5:3306: connect: connection refused"
+
+type outageUserStore struct{ fakeUserStore }
+
+func (o *outageUserStore) GetUserPassword(_ context.Context, _ string) (string, error) {
+	return "", errors.New(dbErrorDetail)
+}
+
+func newOutageRouter(t *testing.T) *gin.Engine {
+	t.Helper()
+	jm := auth.NewJWTManager("test-secret-32-bytes-long-enough!", 15*time.Minute, 168*time.Hour)
+	svc := service.NewAuthService(&outageUserStore{}, jm)
+	h := auth.NewHandler(svc)
+
+	r := gin.New()
+	h.RegisterPublic(r.Group("/api/v1"))
+	h.RegisterAuthed(r.Group("/api/v1", auth.Middleware(jm)))
+	return r
+}
+
+func TestLoginOutageDoesNotLeakDatabaseDetailsToTheCaller(t *testing.T) {
+	r := newOutageRouter(t)
+
+	w := doRequest(t, r, "POST", "/api/v1/auth/login",
+		`{"username":"admin","password":"s3cret"}`, "")
+
+	if w.Code != 500 {
+		t.Fatalf("status = %d, want 500", w.Code)
+	}
+	body := w.Body.String()
+	for _, secret := range []string{dbErrorDetail, "10.0.0.5", "3306", "dial tcp", "connection refused"} {
+		if strings.Contains(body, secret) {
+			t.Errorf("500 body leaks %q to an unauthenticated caller: %s", secret, body)
+		}
+	}
+	if !strings.Contains(body, "internal server error") {
+		t.Errorf("500 body = %s, want a generic message", body)
+	}
+}
+
+func TestChangePasswordOutageDoesNotLeakDatabaseDetailsToTheCaller(t *testing.T) {
+	jm := auth.NewJWTManager("test-secret-32-bytes-long-enough!", 15*time.Minute, 168*time.Hour)
+	token, err := jm.GenerateAccessToken("admin")
+	if err != nil {
+		t.Fatalf("GenerateAccessToken: %v", err)
+	}
+
+	r := newOutageRouter(t)
+	w := doRequest(t, r, "POST", "/api/v1/auth/change-password",
+		`{"old_password":"s3cret","new_password":"n3wpass"}`, token)
+
+	if w.Code != 500 {
+		t.Fatalf("status = %d, want 500 (body: %s)", w.Code, w.Body.String())
+	}
+	body := w.Body.String()
+	for _, secret := range []string{dbErrorDetail, "10.0.0.5", "3306", "dial tcp", "connection refused"} {
+		if strings.Contains(body, secret) {
+			t.Errorf("500 body leaks %q to the caller: %s", secret, body)
+		}
+	}
+}

CubeOps/internal/service/auth.go

@@ -80,16 +80,9 @@ func (s *AuthService) Login(ctx context.Context, username, password string) (*Lo
 	}
 	stored, err := s.store.GetUserPassword(ctx, username)
 	if err != nil {
-		// Distinguish "user not found" (→ ErrInvalidCredentials, safe to
-		// expose) from infrastructure errors (→ return verbatim). We do this
-		// by checking whether stored is empty — the store layer returns "" +
-		// a "not found" error when the row is missing.
-		if stored == "" {
-			return nil, ErrInvalidCredentials
-		}
 		return nil, fmt.Errorf("failed to read user: %w", err)
 	}
-	if !crypto.VerifyPassword(stored, password) {
+	if stored == "" || !crypto.VerifyPassword(stored, password) {
 		return nil, ErrInvalidCredentials
 	}
 	accessToken, err := s.jm.GenerateAccessToken(username)
@@ -180,7 +173,7 @@ func (s *AuthService) ChangePassword(ctx context.Context, username, oldPassword,
 	if err != nil {
 		return fmt.Errorf("failed to read user: %w", err)
 	}
-	if !crypto.VerifyPassword(stored, oldPassword) {
+	if stored == "" || !crypto.VerifyPassword(stored, oldPassword) {
 		return ErrInvalidOldPassword
 	}
 	newHash, err := crypto.HashPassword(newPassword)

CubeOps/internal/service/auth_db_error_test.go

@@ -0,0 +1,81 @@
+// Copyright (c) 2026 Tencent Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package service
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+)
+
+type dbErrorStore struct {
+	err error
+}
+
+func (d dbErrorStore) GetUserPassword(context.Context, string) (string, error) {
+	return "", d.err
+}
+func (d dbErrorStore) SetUserPassword(context.Context, string, string) error       { return nil }
+func (d dbErrorStore) CreateRefreshToken(context.Context, string, string) error    { return nil }
+func (d dbErrorStore) IsRefreshTokenRevoked(context.Context, string) (bool, error) { return false, nil }
+func (d dbErrorStore) RevokeRefreshToken(context.Context, string) error            { return nil }
+func (d dbErrorStore) RevokeAllRefreshTokensForUser(context.Context, string) error { return nil }
+
+type stubIssuer struct{}
+
+func (stubIssuer) GenerateAccessToken(string) (string, error)          { return "a", nil }
+func (stubIssuer) GenerateRefreshToken(string) (string, string, error) { return "r", "t", nil }
+func (stubIssuer) VerifyRefreshToken(string) (*RefreshClaims, error)   { return &RefreshClaims{}, nil }
+func (stubIssuer) AccessTTL() time.Duration                            { return time.Minute }
+
+func TestLoginSurfacesInfrastructureError(t *testing.T) {
+	dbDown := errors.New("dial tcp 10.0.0.5:3306: connect: connection refused")
+	svc := NewAuthService(dbErrorStore{err: dbDown}, stubIssuer{})
+
+	_, err := svc.Login(context.Background(), "admin", "hunter2")
+	if err == nil {
+		t.Fatal("Login returned no error while the database was unreachable")
+	}
+	if errors.Is(err, ErrInvalidCredentials) {
+		t.Fatalf("Login masked a database outage as invalid credentials: %v", err)
+	}
+	if !errors.Is(err, dbDown) {
+		t.Fatalf("Login did not wrap the underlying error, got %v", err)
+	}
+}
+
+func TestLoginUnknownUserStillReportsInvalidCredentials(t *testing.T) {
+	svc := NewAuthService(dbErrorStore{err: nil}, stubIssuer{})
+
+	_, err := svc.Login(context.Background(), "nobody", "hunter2")
+	if !errors.Is(err, ErrInvalidCredentials) {
+		t.Fatalf("unknown user should report invalid credentials, got %v", err)
+	}
+}
+
+func TestChangePasswordSurfacesInfrastructureError(t *testing.T) {
+	dbDown := errors.New("dial tcp 10.0.0.5:3306: connect: connection refused")
+	svc := NewAuthService(dbErrorStore{err: dbDown}, stubIssuer{})
+
+	err := svc.ChangePassword(context.Background(), "admin", "old-pass", "new-pass")
+	if err == nil {
+		t.Fatal("ChangePassword returned no error while the database was unreachable")
+	}
+	if errors.Is(err, ErrInvalidOldPassword) {
+		t.Fatalf("ChangePassword masked a database outage as a bad old password: %v", err)
+	}
+	if !errors.Is(err, dbDown) {
+		t.Fatalf("ChangePassword discarded the underlying database error: %v", err)
+	}
+}
+
+func TestChangePasswordUnknownUserReportsBadOldPassword(t *testing.T) {
+	svc := NewAuthService(dbErrorStore{err: nil}, stubIssuer{})
+
+	err := svc.ChangePassword(context.Background(), "nobody", "old-pass", "new-pass")
+	if !errors.Is(err, ErrInvalidOldPassword) {
+		t.Fatalf("unknown user should report a bad old password, got %v", err)
+	}
+}

CubeOps/internal/service/auth_test.go

@@ -23,7 +23,7 @@ type fakeUserStore struct {
 func (f *fakeUserStore) GetUserPassword(_ context.Context, username string) (string, error) {
 	pw, ok := f.passwords[username]
 	if !ok {
-		return "", errors.New("user not found")
+		return "", nil
 	}
 	return pw, nil
 }

CubeOps/internal/store/db.go

@@ -87,9 +87,11 @@ func (s *Store) bootstrapMasterKey(ctx context.Context) error {
 
 	if b64 == "" {
 		// 2. Fallback: read from the old agenthub settings table.
-		//    This covers the upgrade window where the migration has run
-		//    (key copied to t_system_setting) but also the case where
-		//    CubeOps starts against a DB that hasn't been migrated yet.
+		//    This covers the upgrade window where the migration has run and
+		//    the key was copied to t_system_setting. An unmigrated DB no
+		//    longer reaches here: the read above now returns a real
+		//    table-not-found error and startup fails, as it would anyway at
+		//    seedDefaultAdmin.
 		b64, err = s.GetSetting(ctx, "secret_master_key")
 		if err != nil {
 			return fmt.Errorf("read master key from t_agenthub_setting: %w", err)

CubeOps/internal/store/setting.go

@@ -17,14 +17,17 @@ const settingMasterKey = "secret_master_key"
 
 // GetSystemSetting retrieves a system-level setting value by key.
 func (s *Store) GetSystemSetting(ctx context.Context, key string) (string, error) {
-	var val string
+	var val sql.NullString
 	err := s.db.WithContext(ctx).Raw(
 		"SELECT setting_value FROM t_system_setting WHERE setting_key = ? LIMIT 1", key,
-	).Scan(&val).Error
-	if errors.Is(err, sql.ErrNoRows) || val == "" {
-		return "", nil
+	).Row().Scan(&val)
+	if err != nil {
+		if errors.Is(err, sql.ErrNoRows) {
+			return "", nil
+		}
+		return "", err
 	}
-	return val, err
+	return val.String, nil
 }
 
 // GetOrCreateSystemSetting atomically gets an existing system setting or
@@ -52,14 +55,17 @@ func (s *Store) SetSystemSetting(ctx context.Context, key, value string) error {
 
 // GetSetting retrieves an AgentHub-level setting value by key.
 func (s *Store) GetSetting(ctx context.Context, key string) (string, error) {
-	var val string
+	var val sql.NullString
 	err := s.db.WithContext(ctx).Raw(
 		"SELECT setting_value FROM t_agenthub_setting WHERE setting_key = ? LIMIT 1", key,
-	).Scan(&val).Error
-	if errors.Is(err, sql.ErrNoRows) || val == "" {
-		return "", nil
+	).Row().Scan(&val)
+	if err != nil {
+		if errors.Is(err, sql.ErrNoRows) {
+			return "", nil
+		}
+		return "", err
 	}
-	return val, err
+	return val.String, nil
 }
 
 // GetOrCreateSetting atomically gets an existing setting or creates it with the given value.
@@ -104,14 +110,17 @@ func (s *Store) SetSettingsTx(ctx context.Context, kv map[string]string) error {
 
 // GetUserPassword retrieves the stored password hash for a user.
 func (s *Store) GetUserPassword(ctx context.Context, username string) (string, error) {
-	var pwd string
+	var pwd sql.NullString
 	err := s.db.WithContext(ctx).Raw(
 		"SELECT password FROM t_system_user WHERE username = ? LIMIT 1", username,
-	).Scan(&pwd).Error
-	if errors.Is(err, sql.ErrNoRows) || pwd == "" {
-		return "", nil
+	).Row().Scan(&pwd)
+	if err != nil {
+		if errors.Is(err, sql.ErrNoRows) {
+			return "", nil
+		}
+		return "", err
 	}
-	return pwd, err
+	return pwd.String, nil
 }
 
 // SetUserPassword updates the password hash for a user.