test: make the slowest unit and integration tests fast without weakening them

internal/api/openapi_403_test.go

@@ -197,55 +197,68 @@ func parseRouterEntries(routerPath string) ([]routeHandlerEntry, error) {
 	return entries, nil
 }
 
-// handlerFuncCallsRequirePermission returns true if any handler_*.go file in
-// srcDir contains a function whose name is fnName and whose body calls
+// permissionGatedHandlers scans every handler_*.go file in srcDir once and
+// returns the set of *Handler method names whose body calls
 // requirePermission.
-func handlerFuncCallsRequirePermission(srcDir, fnName string) (bool, error) {
+//
+// The scan reads each file a single time and records every gated method,
+// instead of re-reading all handler files for each route (which, with ~130
+// routes and ~75k handler lines, took about a minute under -race). The
+// per-method rule is the same: a method is gated when a line containing
+// requirePermission appears after its `func (h *Handler) name(` declaration
+// and before the next such declaration, or before its brace depth goes
+// negative.
+func permissionGatedHandlers(srcDir string) (map[string]bool, error) {
 	pattern := filepath.Join(srcDir, "handler_*.go")
 	files, err := filepath.Glob(pattern)
 	if err != nil {
-		return false, err
+		return nil, err
 	}
 
 	// Match the exact function name as declared on *Handler.
 	reFunc := regexp.MustCompile(`^func \(h \*Handler\) (\w+)\(`)
 
+	gated := make(map[string]bool)
 	for _, fpath := range files {
-		f, err := os.Open(fpath)
-		if err != nil {
-			return false, err
+		if err := scanHandlerFileForPermission(fpath, reFunc, gated); err != nil {
+			return nil, err
 		}
+	}
+	return gated, nil
+}
 
-		scanner := bufio.NewScanner(f)
-		inTarget := false
-		braceDepth := 0
+// scanHandlerFileForPermission adds to gated the name of every *Handler
+// method in fpath whose body calls requirePermission.
+func scanHandlerFileForPermission(fpath string, reFunc *regexp.Regexp, gated map[string]bool) error {
+	f, err := os.Open(fpath)
+	if err != nil {
+		return err
+	}
+	defer f.Close()
 
-		for scanner.Scan() {
-			line := scanner.Text()
+	scanner := bufio.NewScanner(f)
+	current := ""
+	braceDepth := 0
 
-			if m := reFunc.FindStringSubmatch(line); m != nil {
-				inTarget = m[1] == fnName
-				braceDepth = 0
-			}
+	for scanner.Scan() {
+		line := scanner.Text()
 
-			if inTarget {
-				braceDepth += strings.Count(line, "{") - strings.Count(line, "}")
-				if strings.Contains(line, "requirePermission") {
-					f.Close()
-					return true, nil
-				}
-				if braceDepth < 0 {
-					inTarget = false
-				}
-			}
+		if m := reFunc.FindStringSubmatch(line); m != nil {
+			current = m[1]
+			braceDepth = 0
 		}
-		if err := scanner.Err(); err != nil {
-			f.Close()
-			return false, err
+
+		if current != "" {
+			braceDepth += strings.Count(line, "{") - strings.Count(line, "}")
+			if strings.Contains(line, "requirePermission") {
+				gated[current] = true
+			}
+			if braceDepth < 0 {
+				current = ""
+			}
 		}
-		f.Close()
 	}
-	return false, nil
+	return scanner.Err()
 }
 
 // specPathsForRoute returns the set of spec routeKeys that the given router
@@ -299,6 +312,11 @@ func TestOpenAPI403OnPermissionGatedRoutes(t *testing.T) {
 	require.NoError(t, err, "failed to parse router.go")
 	require.NotEmpty(t, routerEntries, "parsed zero route entries from router.go")
 
+	// Index the permission-gated handlers once for all routes.
+	gatedHandlers, err := permissionGatedHandlers(pkgDir)
+	require.NoError(t, err, "failed to scan handler_*.go files")
+	require.NotEmpty(t, gatedHandlers, "found no handler that calls requirePermission")
+
 	var failures []string
 
 	for _, entry := range routerEntries {
@@ -308,12 +326,7 @@ func TestOpenAPI403OnPermissionGatedRoutes(t *testing.T) {
 		}
 
 		// Check if the delegate handler calls requirePermission.
-		callsPerm, err := handlerFuncCallsRequirePermission(pkgDir, entry.delegateFnName)
-		if err != nil {
-			t.Logf("warning: could not check handler %s: %v", entry.delegateFnName, err)
-			continue
-		}
-		if !callsPerm {
+		if !gatedHandlers[entry.delegateFnName] {
 			continue
 		}
 

internal/auth/service.go

@@ -71,6 +71,10 @@ type ServiceConfig struct {
 	DashboardURL     string
 	CSRFKey          []byte
 	SessionDuration  time.Duration
+	// BcryptCost lowers the bcrypt work factor for tests that build the
+	// service through other packages. Zero, the only value production
+	// uses, keeps bcryptCost (12) and the fixed cost-12 timing sentinel.
+	BcryptCost int
 }
 
 // NewService creates a new auth service.
@@ -124,6 +128,8 @@ func NewService(cfg ServiceConfig) *Service {
 		dashboardURL:     cfg.DashboardURL,
 		onPasswordChange: cfg.OnPasswordChange,
 		csrfKey:          csrfKey,
+		// Zero leaves the production cost in hashPassword and Login.
+		bcryptCostOverride: cfg.BcryptCost,
 	}
 }
 
@@ -162,7 +168,7 @@ func (s *Service) Login(ctx context.Context, req LoginRequest) (*LoginResponse,
 		// user is indistinguishable from a wrong-password attempt on a real
 		// account. Without this, the missing-row path returns immediately
 		// (no bcrypt), leaking account existence via timing (issue #416).
-		s.verifyPassword(req.Password, dummyPasswordHash)
+		s.verifyPassword(req.Password, s.dummyHashForCompare())
 		return nil, err
 	}
 

internal/auth/service_lockout_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/crypto/bcrypt"
 )
 
 func TestLogin_AccountLockout_Bookkeeping(t *testing.T) {
@@ -114,6 +115,26 @@ func TestLogin_BookkeepingErrors(t *testing.T) {
 	}
 }
 
+// Production (NewService with no BcryptCost) compares against the fixed
+// cost-12 sentinel. Only a test cost override switches the timing-equalizing
+// compare to a hash at that cost.
+func TestDummyHashForCompare_MatchesHashCost(t *testing.T) {
+	prod := NewService(ServiceConfig{DashboardURL: "https://dashboard.example.com", CSRFKey: testCSRFKey}).dummyHashForCompare()
+	assert.Equal(t, dummyPasswordHash, prod)
+	prodCost, err := bcrypt.Cost([]byte(prod))
+	require.NoError(t, err)
+	assert.Equal(t, bcryptCost, prodCost)
+
+	for name, svc := range map[string]*Service{
+		"override": newTestService(),
+		"config":   NewService(ServiceConfig{DashboardURL: "https://dashboard.example.com", CSRFKey: testCSRFKey, BcryptCost: bcrypt.MinCost}),
+	} {
+		testCost, err := bcrypt.Cost([]byte(svc.dummyHashForCompare()))
+		require.NoError(t, err, name)
+		assert.Equal(t, bcrypt.MinCost, testCost, name)
+	}
+}
+
 // TestLogin_OWASPEnumerationInvariant guards against regression that re-introduces
 // distinct error messages for the 6 authentication failure modes (OWASP ASVS V3.3.4
 // / V14.2). All 6 paths MUST return an identical string so the login endpoint cannot

internal/auth/service_password.go

@@ -71,6 +71,22 @@ func (s *Service) hashPassword(password string) (string, error) {
 	return string(hash), nil
 }
 
+// dummyHashForCompare returns the hash Login compares against when the
+// account cannot log in. Production always uses dummyPasswordHash, at the
+// same cost as real hashes. When a test lowers the cost with
+// bcryptCostOverride, the dummy hash uses that cost too, so the compare still
+// takes as long as a real one in that test instead of a full cost-12 compare.
+func (s *Service) dummyHashForCompare() string {
+	if s.bcryptCostOverride <= 0 {
+		return dummyPasswordHash
+	}
+	hash, err := bcrypt.GenerateFromPassword([]byte("timing-equalizer"), s.bcryptCostOverride)
+	if err != nil {
+		return dummyPasswordHash
+	}
+	return string(hash)
+}
+
 // verifyPassword verifies a password against a bcrypt hash.
 func (s *Service) verifyPassword(password, hash string) bool {
 	err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(password))

internal/auth/service_user_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
-	"golang.org/x/crypto/bcrypt"
 )
 
 func TestService_SetupAdmin(t *testing.T) {
@@ -1191,18 +1190,19 @@ func TestService_SetupAdmin_EdgeCases(t *testing.T) {
 // Test TOTP functions.
 func TestService_UpdateUserProfile(t *testing.T) {
 	ctx := context.Background()
+	// Hashed once at MinCost: the subtests only need a hash that
+	// verifyPassword accepts, not the production work factor.
+	oldHash := TestPasswordHash(t, "OldPassword123")
 
 	t.Run("update email and password successfully", func(t *testing.T) {
 		mockStore := new(MockStore)
 		mockEmail := new(MockEmailSender)
 		service := createTestService(mockStore, mockEmail)
 
-		// Create user with bcrypt hash for UpdateUserProfile test
-		hash, _ := bcrypt.GenerateFromPassword([]byte("OldPassword123"), bcrypt.DefaultCost)
 		testUser := &User{
 			ID:           "user-123",
 			Email:        "old@example.com",
-			PasswordHash: string(hash),
+			PasswordHash: oldHash,
 			Active:       true,
 			CreatedAt:    time.Now(),
 		}
@@ -1224,11 +1224,10 @@ func TestService_UpdateUserProfile(t *testing.T) {
 		mockEmail := new(MockEmailSender)
 		service := createTestService(mockStore, mockEmail)
 
-		hash, _ := bcrypt.GenerateFromPassword([]byte("OldPassword123"), bcrypt.DefaultCost)
 		testUser := &User{
 			ID:           "user-123",
 			Email:        "old@example.com",
-			PasswordHash: string(hash),
+			PasswordHash: oldHash,
 			Active:       true,
 		}
 
@@ -1252,11 +1251,10 @@ func TestService_UpdateUserProfile(t *testing.T) {
 		mockEmail := new(MockEmailSender)
 		service := createTestService(mockStore, mockEmail)
 
-		hash, _ := bcrypt.GenerateFromPassword([]byte("OldPassword123"), bcrypt.DefaultCost)
 		testUser := &User{
 			ID:           "user-123",
 			Email:        "old@example.com",
-			PasswordHash: string(hash),
+			PasswordHash: oldHash,
 			Active:       true,
 		}
 		otherUser := &User{
@@ -1280,11 +1278,10 @@ func TestService_UpdateUserProfile(t *testing.T) {
 		mockEmail := new(MockEmailSender)
 		service := createTestService(mockStore, mockEmail)
 
-		hash, _ := bcrypt.GenerateFromPassword([]byte("OldPassword123"), bcrypt.DefaultCost)
 		testUser := &User{
 			ID:           "user-123",
 			Email:        "old@example.com",
-			PasswordHash: string(hash),
+			PasswordHash: oldHash,
 			Active:       true,
 		}
 
@@ -1302,11 +1299,10 @@ func TestService_UpdateUserProfile(t *testing.T) {
 		mockEmail := new(MockEmailSender)
 		service := createTestService(mockStore, mockEmail)
 
-		hash, _ := bcrypt.GenerateFromPassword([]byte("OldPassword123"), bcrypt.DefaultCost)
 		testUser := &User{
 			ID:           "user-123",
 			Email:        "old@example.com",
-			PasswordHash: string(hash),
+			PasswordHash: oldHash,
 			Active:       true,
 		}
 
@@ -1337,11 +1333,10 @@ func TestService_UpdateUserProfile(t *testing.T) {
 		mockEmail := new(MockEmailSender)
 		service := createTestService(mockStore, mockEmail)
 
-		hash, _ := bcrypt.GenerateFromPassword([]byte("OldPassword123"), bcrypt.DefaultCost)
 		testUser := &User{
 			ID:           "user-123",
 			Email:        "old@example.com",
-			PasswordHash: string(hash),
+			PasswordHash: oldHash,
 			Active:       true,
 		}
 

internal/purchase/execute_detached_test.go

@@ -78,9 +78,19 @@ func newDetachFixture(t *testing.T) *detachFixture {
 	return f
 }
 
+// A run longer than the terminal save budget must still give the save its
+// full budget, and each provider call gets its own per-rec budget rather than
+// a share of one. The budgets are scaled down (production: 30s each, with 16s
+// calls) so the test takes about 2.5s instead of 32s; the production values
+// are pinned with literal durations in the ExpiredCallerDeadline test below.
 func TestExecuteAndFinalize_SlowRunGetsFreshTerminalSaveBudget(t *testing.T) {
+	const (
+		budget   = 2 * time.Second         // terminal save and per-rec cap
+		callTime = 1200 * time.Millisecond // each call fits its cap; two exceed the save budget
+	)
 	t.Setenv("CUDLY_MAX_ACCOUNT_PARALLELISM", "1")
 	f := newDetachFixture(t)
+	f.manager.budgets = purchaseBudgets{terminalSave: budget, perRec: budget}
 	f.exec.Recommendations = append(f.exec.Recommendations, f.exec.Recommendations[0])
 	f.exec.Recommendations[1].ResourceType = "m5.xlarge"
 	var active atomic.Int32
@@ -93,7 +103,7 @@ func TestExecuteAndFinalize_SlowRunGetsFreshTerminalSaveBudget(t *testing.T) {
 			}
 			defer active.Add(-1)
 			ctx := args.Get(0).(context.Context)
-			timer := time.NewTimer(16 * time.Second)
+			timer := time.NewTimer(callTime)
 			defer timer.Stop()
 			select {
 			case <-timer.C:
@@ -103,15 +113,15 @@ func TestExecuteAndFinalize_SlowRunGetsFreshTerminalSaveBudget(t *testing.T) {
 		}).Return(common.PurchaseResult{Success: true, CommitmentID: "ri-slow"}, nil).Twice()
 	started := time.Now()
 	err := f.manager.executeAndFinalize(context.Background(), f.exec)
-	assert.Greater(t, time.Since(started), 30*time.Second)
+	assert.Greater(t, time.Since(started), budget)
 	assert.False(t, overlapped.Load())
 	assert.False(t, providerCanceled.Load(), "provider call must fit its own budget")
 	f.svc.AssertNumberOfCalls(t, "PurchaseCommitment", 2)
 	assert.Equal(t, 2, f.history)
 	require.NoError(t, err)
 	assert.Equal(t, []string{"completed"}, f.saved)
-	assert.Greater(t, f.saveRemaining, 29*time.Second)
-	assert.LessOrEqual(t, f.saveRemaining, 30*time.Second)
+	assert.Greater(t, f.saveRemaining, budget-time.Second)
+	assert.LessOrEqual(t, f.saveRemaining, budget)
 }
 
 var stsOut = sts.GetCallerIdentityOutput{Account: aws.String("123456789012")}
@@ -153,6 +163,7 @@ func TestExecuteAndFinalize_ExpiredCallerDeadline_UsesOwnBudgets(t *testing.T) {
 
 	// per-rec cap (30s) still bounds the provider call inside the 4m run budget.
 	assert.WithinDuration(t, time.Now(), provDeadline, 30*time.Second)
+	assert.Greater(t, time.Until(provDeadline), 29*time.Second)
 	_, ok := f.saveCtx.Deadline()
 	require.True(t, ok, "terminal save needs a deadline")
 	// Literal durations on purpose: comparing against the constants would

internal/purchase/execution.go

@@ -1102,7 +1102,7 @@ func (m *Manager) executeSinglePurchase(ctx context.Context, rec config.Recommen
 	// The parent ctx governs the overall Lambda budget; this per-rec deadline
 	// is always the shorter of the two.
 	recTuple := fmt.Sprintf("%s/%s/%s/%s", rec.Provider, rec.Service, rec.Region, rec.ResourceType)
-	recCtx, recCancel := context.WithTimeout(ctx, 30*time.Second)
+	recCtx, recCancel := context.WithTimeout(ctx, m.budgets.perRecTimeout())
 	defer recCancel()
 
 	// Create the provider