From 55d6fdf766a7344ea832b00334f7ddf60717f702 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 02:44:42 +0000 Subject: [PATCH 1/6] test(api): scan handler files once in the OpenAPI 403 guard TestOpenAPI403OnPermissionGatedRoutes re-read and regex-scanned all 158 handler_*.go files (about 75k lines) once per router entry, about 130 times. Under -race that took 60-70s, almost all of it in regexp backtracking and GC. Build the set of permission-gated *Handler methods in one pass and look each route up in it. The per-method rule is unchanged, and a temporary comparison against the old function agreed on all 352 Handler methods (99 gated). With every '403' removed from openapi.yaml, main and this branch report the same 50 failing operations. Co-Authored-By: Claude Opus 5.5 --- internal/api/openapi_403_test.go | 87 ++++++++++++++++++-------------- 1 file changed, 50 insertions(+), 37 deletions(-) diff --git a/internal/api/openapi_403_test.go b/internal/api/openapi_403_test.go index e1007c064..5b7a5aca4 100644 --- a/internal/api/openapi_403_test.go +++ b/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 } From 4f0b83eaae7ce00c1d663666e9c9639a22097684 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 02:50:25 +0000 Subject: [PATCH 2/6] test(purchase): scale the slow-run budget test down to 2s budgets TestExecuteAndFinalize_SlowRunGetsFreshTerminalSaveBudget slept 16s in each of two provider calls to outlast the real 30s terminal save budget, so it took 32s. Add a zero-value purchaseBudgets field on Manager that overrides the terminal save and per-rec deadlines when set. NewManager leaves it zero, so production still uses 30s for both; the per-rec literal becomes the named constant perRecPurchaseTimeout with the same value. The test now sets 2s budgets with 1.2s calls and keeps every assertion, scaled. The ExpiredCallerDeadline test already pins the production save and run budgets with literal durations; it now also pins the per-rec cap from below (more than 29s), which the old 16s calls covered. Co-Authored-By: Claude Opus 5.5 --- internal/purchase/execute_detached_test.go | 19 +++++++++++--- internal/purchase/execution.go | 2 +- internal/purchase/manager.go | 29 +++++++++++++++++++++- 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/internal/purchase/execute_detached_test.go b/internal/purchase/execute_detached_test.go index 43f58083f..28f3ecfed 100644 --- a/internal/purchase/execute_detached_test.go +++ b/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 diff --git a/internal/purchase/execution.go b/internal/purchase/execution.go index 4e80d4671..5696de36d 100644 --- a/internal/purchase/execution.go +++ b/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 diff --git a/internal/purchase/manager.go b/internal/purchase/manager.go index adba19c9e..a299a190a 100644 --- a/internal/purchase/manager.go +++ b/internal/purchase/manager.go @@ -59,6 +59,9 @@ type Manager struct { oidcIssuerURL string notifyDays int resolver stepResolver + // budgets lets tests shorten purchase deadlines. Production leaves it + // zero, which means the default durations below. + budgets purchaseBudgets } // PurchaseDefaults holds default purchase settings. @@ -233,8 +236,32 @@ const ( // #913). purchaseExecutionTimeout = 4 * time.Minute terminalSaveTimeout = 30 * time.Second + // perRecPurchaseTimeout caps one recommendation's provider calls; see + // executeSinglePurchase. + perRecPurchaseTimeout = 30 * time.Second ) +// purchaseBudgets overrides deadlines for tests. A zero field means the +// production default, so a Manager built by NewManager uses the constants. +type purchaseBudgets struct { + terminalSave time.Duration + perRec time.Duration +} + +func (b purchaseBudgets) terminalSaveTimeout() time.Duration { + if b.terminalSave > 0 { + return b.terminalSave + } + return terminalSaveTimeout +} + +func (b purchaseBudgets) perRecTimeout() time.Duration { + if b.perRec > 0 { + return b.perRec + } + return perRecPurchaseTimeout +} + // executeAndFinalize runs a purchase and handles status updates, record saving, and progress. // // The root execution row is ALWAYS saved with its finalized status — including @@ -261,7 +288,7 @@ func (m *Manager) executeAndFinalize(ctx context.Context, exec *config.PurchaseE if execErr != nil { logging.Errorf("Failed to execute purchase %s: %v", exec.ExecutionID, execErr) } - saveCtx, cancelSave := ctxutil.DetachedWithTimeout(ctx, terminalSaveTimeout) + saveCtx, cancelSave := ctxutil.DetachedWithTimeout(ctx, m.budgets.terminalSaveTimeout()) defer cancelSave() // Operators hear about every failed or partial row (issue #714), even // when the terminal save below fails. A fan-out root is skipped: each From 09380917a342f063c910f650975f81e52fd0ed83 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 02:52:53 +0000 Subject: [PATCH 3/6] test(secrets): shrink SDK retry delay in the Azure metadata-block test TestNewAzureResolver_GetSecret_BlocksMetadata spent 15-19s in azcore's default retry backoff (3 retries, 800ms base delay, exponential with jitter) on a dial that the guard rejects instantly. Pass the GetSecret call a context carrying azcore's per-call retry override (policy.WithRetryOptions) with a 1ms delay. The resolver is still built by NewAzureResolver unchanged, the same 4 attempts still happen, and every assertion stays. No production change. Co-Authored-By: Claude Opus 5.5 --- internal/secrets/azure_resolver_httptest_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/secrets/azure_resolver_httptest_test.go b/internal/secrets/azure_resolver_httptest_test.go index 543b931d6..7ced375df 100644 --- a/internal/secrets/azure_resolver_httptest_test.go +++ b/internal/secrets/azure_resolver_httptest_test.go @@ -333,7 +333,14 @@ func TestNewAzureResolver_GetSecret_BlocksMetadata(t *testing.T) { resolver, err := NewAzureResolver(ctx, vaultURL) require.NoError(t, err) - secret, err := resolver.GetSecret(ctx, "test-secret") + // The SDK retries the blocked dial 3 times with exponential + // backoff (about 9s). Keep the retries but shrink the delay for + // this call only, via azcore's per-call override. + callCtx := policy.WithRetryOptions(ctx, policy.RetryOptions{ + RetryDelay: time.Millisecond, + MaxRetryDelay: time.Millisecond, + }) + secret, err := resolver.GetSecret(callCtx, "test-secret") require.Error(t, err) assert.Empty(t, secret) assert.Contains(t, err.Error(), "connection to metadata endpoint 169.254.169.254 is blocked") From 09ee8799c546a69a803831ddc776fefd7a2a9cf6 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 03:02:16 +0000 Subject: [PATCH 4/6] test(auth): keep bcrypt at the test cost in the slow auth tests Login runs a timing-equalizing compare against dummyPasswordHash, a fixed cost-12 hash, for missing, inactive and locked accounts. Under -race one cost-12 compare takes about 7s, so TestLogin_OWASPEnumerationInvariant (4 such paths) took 17-32s and TestIntegration_MFALifecycleWrongCodesLockAccount (2 locked-account logins) about 12s of its run, even though both services already set the existing bcryptCostOverride test hook to MinCost/4. Login now calls dummyHashForCompare. With bcryptCostOverride unset, which is the case for every Service built by NewService, it returns dummyPasswordHash unchanged. Only when a test sets the override does it hash a sentinel at that cost, so the dummy compare costs the same as the test's real hashes. TestDummyHashForCompare_MatchesHashCost pins both cases. TestService_UpdateUserProfile hashed its fixture at DefaultCost in every subtest (7 hashes plus compares, 17-25s). It now hashes once with the existing TestPasswordHash (MinCost) helper and every subtest builds its own User from that hash. Co-Authored-By: Claude Opus 5.5 --- internal/auth/service.go | 2 +- internal/auth/service_lockout_test.go | 15 +++++++++++++++ internal/auth/service_password.go | 16 ++++++++++++++++ internal/auth/service_user_test.go | 23 +++++++++-------------- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/internal/auth/service.go b/internal/auth/service.go index edaa3795d..8627fc4e2 100644 --- a/internal/auth/service.go +++ b/internal/auth/service.go @@ -162,7 +162,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 } diff --git a/internal/auth/service_lockout_test.go b/internal/auth/service_lockout_test.go index 5efc859b4..5e38e9875 100644 --- a/internal/auth/service_lockout_test.go +++ b/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,20 @@ func TestLogin_BookkeepingErrors(t *testing.T) { } } +// Production 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 := (&Service{}).dummyHashForCompare() + assert.Equal(t, dummyPasswordHash, prod) + prodCost, err := bcrypt.Cost([]byte(prod)) + require.NoError(t, err) + assert.Equal(t, bcryptCost, prodCost) + + testCost, err := bcrypt.Cost([]byte(newTestService().dummyHashForCompare())) + require.NoError(t, err) + assert.Equal(t, bcrypt.MinCost, testCost) +} + // 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 diff --git a/internal/auth/service_password.go b/internal/auth/service_password.go index 3be2d3027..30f7075bd 100644 --- a/internal/auth/service_password.go +++ b/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)) diff --git a/internal/auth/service_user_test.go b/internal/auth/service_user_test.go index 2207ab298..f38c71f52 100644 --- a/internal/auth/service_user_test.go +++ b/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, } From 406b7a04c780ca75ad8e0c9a7555be8b1e5805c6 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 03:16:06 +0000 Subject: [PATCH 5/6] test(server): run the rate limiter test at the test bcrypt cost TestApplicationRateLimiter sends 14 logins for a missing account (7 per mode) that reach Login, and each runs the cost-12 timing-equalizing compare: about 5s each under -race, 57-90s in all. The test checks rate-limit counters, not hashing. Add auth.ServiceConfig.BcryptCost, which NewService copies into the existing bcryptCostOverride hook, and an unexported ApplicationConfig.authBcryptCost that both auth.NewService call sites in server pass through. Production never sets either, so zero keeps cost 12 and the cost-12 sentinel; TestDummyHashForCompare_MatchesHashCost now pins that for NewService too. The test sets bcrypt.MinCost and keeps every request and assertion. Co-Authored-By: Claude Opus 5.5 --- internal/auth/service.go | 6 ++++++ internal/auth/service_lockout_test.go | 18 ++++++++++++------ internal/server/app.go | 5 +++++ .../app_rate_limiter_integration_test.go | 5 +++++ 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/internal/auth/service.go b/internal/auth/service.go index 8627fc4e2..ea8f587ed 100644 --- a/internal/auth/service.go +++ b/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, } } diff --git a/internal/auth/service_lockout_test.go b/internal/auth/service_lockout_test.go index 5e38e9875..5874ee924 100644 --- a/internal/auth/service_lockout_test.go +++ b/internal/auth/service_lockout_test.go @@ -115,18 +115,24 @@ func TestLogin_BookkeepingErrors(t *testing.T) { } } -// Production compares against the fixed cost-12 sentinel. Only a test cost -// override switches the timing-equalizing compare to a hash at that cost. +// 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 := (&Service{}).dummyHashForCompare() + 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) - testCost, err := bcrypt.Cost([]byte(newTestService().dummyHashForCompare())) - require.NoError(t, err) - assert.Equal(t, bcrypt.MinCost, testCost) + 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 diff --git a/internal/server/app.go b/internal/server/app.go index fd92d96bf..f0f1eabb6 100644 --- a/internal/server/app.go +++ b/internal/server/app.go @@ -145,6 +145,9 @@ type ApplicationConfig struct { // SQS event source mapping's function_response_types actually includes // "ReportBatchItemFailures" (issue #108 follow-up). ReportSQSBatchItemFailures bool + // authBcryptCost is passed to auth.ServiceConfig.BcryptCost. Only tests + // in this package set it; zero keeps the production bcrypt cost. + authBcryptCost int } // ExternalDeps holds pre-built external dependencies that require infrastructure. @@ -516,6 +519,7 @@ func NewApplicationFromDeps(ctx context.Context, cfg ApplicationConfig, deps Ext EmailSender: deps.EmailSender, SessionDuration: 24 * time.Hour, DashboardURL: cfg.DashboardURL, + BcryptCost: cfg.authBcryptCost, }) // Sensitive requests pass ensureDB, which replaces this temporary limiter @@ -764,6 +768,7 @@ func (app *Application) initAuthService(authStore *auth.PostgresStore, encKey [] DashboardURL: app.appConfig.DashboardURL, OnPasswordChange: buildAdminPasswordSyncCallback(authStore, app.secretResolver), CSRFKey: csrfKey, + BcryptCost: app.appConfig.authBcryptCost, }) if svc == nil { return nil, fmt.Errorf("failed to create auth service") diff --git a/internal/server/app_rate_limiter_integration_test.go b/internal/server/app_rate_limiter_integration_test.go index 2e344cfb6..d1ee54998 100644 --- a/internal/server/app_rate_limiter_integration_test.go +++ b/internal/server/app_rate_limiter_integration_test.go @@ -15,6 +15,7 @@ import ( "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/crypto/bcrypt" ) func TestApplicationRateLimiter(t *testing.T) { @@ -44,6 +45,10 @@ func TestApplicationRateLimiter(t *testing.T) { cfg := ApplicationConfig{ IsLambda: isLambda, DashboardURL: "https://rate-limit.example.test", DefaultTerm: 3, DefaultCoverage: 80, + // Each 401 login runs Login's timing-equalizing bcrypt + // compare, about 5s at cost 12 under -race. The limiter + // does not depend on the cost. + authBcryptCost: bcrypt.MinCost, } newApp := func() *Application { app, appErr := NewApplicationFromDeps(ctx, cfg, ExternalDeps{ From 49e1ebd81f0827fcb7aacecf6be679b508194923 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 03:33:58 +0000 Subject: [PATCH 6/6] test(server): build the adapter tests' auth service at the test bcrypt cost TestSetupAdminThenLogin_RoundTrip hashes and verifies a password at cost 12 and TestAuthServiceAdapter_Login hits Login's cost-12 timing-equalizing compare for an inactive user, 7-11s under -race in the full suite. Both now set the new auth.ServiceConfig.BcryptCost to bcrypt.MinCost; the round trip still hashes at setup and verifies at login through the real service, so the base64 regression it guards is still covered. Co-Authored-By: Claude Opus 5.5 --- internal/server/adapter_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/server/adapter_test.go b/internal/server/adapter_test.go index a1dfd4fee..2a90b0184 100644 --- a/internal/server/adapter_test.go +++ b/internal/server/adapter_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "golang.org/x/crypto/bcrypt" ) // hashSessionTokenForTest mirrors auth.hashSessionToken (private). Session @@ -36,6 +37,9 @@ func createMockAuthService(t *testing.T) (*authServiceAdapter, *auth.MockStore) // random ephemeral key, so any test asserting a specific derived CSRF // token (TestAuthServiceAdapter_ValidateCSRFToken) cannot reproduce it. CSRFKey: auth.TestCSRFKey(), + // Hashing and Login's timing-equalizing compare run at the test + // cost; cost 12 takes seconds per call under -race. + BcryptCost: bcrypt.MinCost, }) adapter := newAuthServiceAdapter(service) return adapter, mockStore @@ -582,6 +586,7 @@ func TestSetupAdminThenLogin_RoundTrip(t *testing.T) { SessionDuration: time.Hour, CSRFKey: auth.TestCSRFKey(), DashboardURL: "http://localhost:3000", + BcryptCost: bcrypt.MinCost, }) adapter := newAuthServiceAdapter(realAuth) handler := api.NewHandler(api.HandlerConfig{AuthService: adapter})